chicken

Basic MSP430 GPIO Macros

21 posts in this topic

In my project, I use a few basic macros for GPIO. The goal is, that I can easily redefine pin assignment in a central location without compromising performance or code size.

 

The macros (gpiomacros.h):

// MSP430 gpio macros
#define GPIO_SEL(port) P ## port ## SEL
#define GPIO_DIR(port) P ## port ## DIR
#define GPIO_OUT(port) P ## port ## OUT
#define GPIO_IN(port) P ## port ## IN

#define GPIO_IS_INPUT(port,pin) { GPIO_SEL(port) &= ~(pin); GPIO_DIR(port) &= ~(pin); }
#define GPIO_IS_OUTPUT(port,pin) { GPIO_SEL(port) &= ~(pin); GPIO_DIR(port) |= (pin); }
#define GPIO_IS_PERIPHERAL_IN(port,pin) { GPIO_SEL(port) |= (pin); GPIO_DIR(port) &= ~(pin); }
#define GPIO_IS_PERIPHERAL_OUT(port,pin) { GPIO_SEL(port) |= (pin); GPIO_DIR(port) |= (pin); }

#define GPIO_SET(port,pin) { GPIO_OUT(port) |= (pin); }
#define GPIO_CLEAR(port,pin) { GPIO_OUT(port) &= ~(pin); }
#define GPIO_READ(port,pin)  ( GPIO_IN(port) & (pin) )

In a central configuration file (e.g. hardware.h) I assign pins like this:

// Pin assignment
#define LED1_PIN  BIT1
#define LED1_PORT 6
#define LED2_PIN  BIT0
#define LED2_PORT 1

And then in the code I interact with GPIO like this:

// Setup LEDs
GPIO_IS_OUTPUT(LED1_PORT, LED1_PIN);
GPIO_IS_OUTPUT(LED2_PORT, LED2_PIN);

// Turn off LEDs
GPIO_CLEAR(LED1_PORT, LED1_PIN);
GPIO_CLEAR(LED2_PORT, LED2_PIN);

The macros are resolved in two steps:

1. Higher level "functions" define the commands. E.g. GPIO_SET(), GPIO_IS_OUTPUT(), ..

2. Lower level macros used within those functions translate port, pin to a register. E.g. GPIO_IN(), GPIO_SEL(), ..

 

The end result is code like you would write when directly working with the GPIO registers. E.g. P2OUT &= ~BIT0; Note that this translation is done by the C pre-processor before the code is compiled.

 

This all works fine and dandy, with the exception of port J. Port J doesn't have a SEL register, which breaks the 1st half of the GPIO_IS_OUTPUT and GPIO_IS_INPUT macros. I currently work around this by adding special GPIO_IS_OUTPUT/INPUT_J macros, but then the main code needs to include some logic to invoke the proper macro.

#if (LED2_PORT == J)
GPIO_IS_OUTPUT_J(LED2_PORT, LED2_PIN);
#else
GPIO_IS_OUTPUT(LED2_PORT, LED2_PIN);
#endif

Any ideas, how I could include a condition inside macros, that checks whether the port is J, and if so excludes the GPIO_SEL command?

 

And yes, I could probably use C++ templates with identical results and an easy workaround for port J, but I'd like to avoid migrating my plain old C project.

 

Edit: Added a few missing parentheses, thanks to Rickta59 for spotting that

Fmilburn, zeke, spirilis and 4 others like this

Share this post


Link to post
Share on other sites

Hi Chicken,

 

So I thanked you for the first post. Because it think it's awesome when people share their code with others while offering up an explanation.

 

However, personally, I would  not do this. Do keep in mind when I say this, that it's purely a personal preference, based on my coding beliefs. But basically, I would not do this because it "obfuscates" how things are done. It adds more code than is necessary, and it makes the code less readable at a glance.

 

Readability, one cold really argue both ways. But when I say it makes it harder to see what's happening at a glance. I mean literally. Like, if we create a macro to clear a bit on a port, interrupt, whatever. How do we know that macro is correct, or what it actually does without going through an implementation file or two ? Additionally, sometimes it may not be clear where the actual macro is. When working with larger projects, and the source is not yours.

 

Passed all that I do have a strong dislike for #define directives, as well as macros in general. For numerical values, I prefer to use const.

 

Then somewhere in my past, I've read that creating function like macro's is not really a good thing. Whether I still believe that is true or not is mostly irrelevant. But I still prefer to use standard C functions.

 

So, when I need to set, or clear, or whatever a pin, interrupt, port, or whatever. I prefer to do it right where it needs to be done, and in plain C. This also keeps me from becoming lazy, and forgetting "things" which I already have a hard time with. Since I jump around between several languages a lot . . .

 

I have to say though, if this works for you. Then keep it up :)

chicken likes this

Share this post


Link to post
Share on other sites

You could consider using C++ template programming instead of C style defines.

I haven't tested this, but it would go something like this:

template <int port>
void GPIO_IS_INPUT(int pin)
{
  GPIO_SEL(port) &= ~(pin);
  GPIO_DIR(port) &= ~(pin);
}

template <>
void GPIO_IS_INPUT<J>(int pin)
{
  GPIO_DIR(J) &= ~(pin);
}

Now instead of passing port as a macro like function parameter, pass it as a template parameter

GPIO_IS_INPUT<LED1_PORT>(LED1_PIN);

I'm not entirely sure this will work though; the double-hash preprocessor operator is very tricky.

 

You could go all the way with these templates and get gone with the macros entirely.

If you really want no function call overhead, just make them inline.

yyrkoon, LiviuM and chicken like this

Share this post


Link to post
Share on other sites

I tend to use this style for many things, in particular single thread embedded applications like one tends to have on the smaller MSP430's, but it isn't always my preference.

 

The code tends to be more readable if done reasonably well, with full optimization available since it is expanded by the preprocessor, but inline functions can be just as efficient and are often less likely to lead to debugging issues, making them generally preferable when available, but with the drawback that expansion is not mandatory, so timing is not guaranteed. If forced, you lose some space optimization features.

chicken and yyrkoon like this

Share this post


Link to post
Share on other sites

Thank you all for your thoughtful comments.

 

@@yyrkoon I think being able to change pin assignments in once place reduces the likelihood of mistakes. Also reusing the same code or macro instead of repeating code in multiple places should reduce errors. But I agree that complex macros can be a pain to debug if there are issues.

 

@@roadrunner84 Yes, templates would probably the way to go if this were a C++ project.

 

@@enl Do you know, whether the compiler will optimize inline code when using constants as parameters in the function call? E.g. would if(port=='x') or switch(port) statements be removed if not applicable for the given invocation?

Share this post


Link to post
Share on other sites

@@enl Do you know, whether the compiler will optimize inline code when using constants as parameters in the function call? E.g. would if(port=='x') or switch(port) statements be removed if not applicable for the given invocation?

 

These all seem to generate the same sized code ... 

#include <msp430.h>
#include <stdint.h>

// MSP430 gpio macros
#define GPIO_SEL(port) P ## port ## SEL
#define GPIO_DIR(port) P ## port ## DIR
#define GPIO_OUT(port) P ## port ## OUT
#define GPIO_IN(port) P ## port ## IN

#define GPIO_IS_INPUT(port,pin) { GPIO_SEL(port) &= ~(pin); GPIO_DIR(port) &= ~(pin); }
#define GPIO_IS_OUTPUT(port,pin) { GPIO_SEL(port) &= ~(pin); GPIO_DIR(port) |= (pin); }
#define GPIO_IS_PERIPHERAL_IN(port,pin) { GPIO_SEL(port) |= (pin); GPIO_DIR(port) &= ~(pin); }
#define GPIO_IS_PERIPHERAL_OUT(port,pin) { GPIO_SEL(port) |= pin; GPIO_DIR(port) |= (pin); }

#define GPIO_SET(port,pin) { GPIO_OUT(port) |= (pin); }
#define GPIO_CLEAR(port,pin) { GPIO_OUT(port) &= ~(pin); }
#define GPIO_READ(port,pin)  ( GPIO_IN(port) & (pin) )

#define GPIO_PIN(pin) (1 << (pin))

__attribute__((always_inline)) 
static inline gpio_is_output(const unsigned port, const unsigned pinno)
{
  switch(port) {
  case 1:
    P1SEL &= ~(1 << pinno);
    P1DIR |= 1 << pinno;
    break;
  case 2:
    P1SEL &= ~(1 << pinno);
    P1DIR |= 1 << pinno;
    break;
  }
}

//#define USE_MACRO

void main(void) {
  WDTCTL = WDTPW|WDTHOLD;
  //DCOCTL -= 28;

#if defined(USE_MACRO)
  GPIO_IS_OUTPUT(1,4);
  GPIO_IS_OUTPUT(1,6);
  GPIO_IS_OUTPUT(1,0);
#elif defined(USE_INLINE_FUNC)
  gpio_is_output(1,4);
  gpio_is_output(1,6);
  gpio_is_output(1,0);
#else
  P1SEL &= ~(BIT4);
  P1DIR |= BIT4;
  P1SEL &= ~(BIT6);
  P1DIR |= BIT6;
  P1SEL &= ~(BIT0);
  P1DIR |= BIT0;

  P1OUT &= ~BIT0;
  P1OUT |= BIT6;

  P1SEL |= BIT4;
#endif

#if defined(USE_MACRO) || defined(USE_INLINE_FUNC)
  GPIO_CLEAR(1,0);
  GPIO_SET(1,6);
  GPIO_SEL(1) |= GPIO_PIN(4);
#endif

  for(;{
    P1OUT ^= (BIT0|BIT6);
    __delay_cycles(1000000>>3); // delay
  }
}

At least with msp430-gcc. I didn't try with msp430-elf-gcc

$ msp430-gcc -Os -g gpio_test.c -mmcu=msp430g2553 -DUSE_INLINE_FUNC; msp430-size a.out; msp430-objdump -CS a.out >/tmp/inline.txt
   text	   data	    bss	    dec	    hex	filename
    180	      0	      2	    182	     b6	a.out
$ msp430-gcc -Os -g gpio_test.c -mmcu=msp430g2553 -DUSE_MACRO; msp430-size a.out; msp430-objdump -CS a.out >/tmp/macro.txt
   text	   data	    bss	    dec	    hex	filenameD
    180	      0	      2	    182	     b6	a.out
$ msp430-gcc -Os -g gpio_test.c -mmcu=msp430g2553 ; msp430-size a.out; msp430-objdump -CS a.out >/tmp/reg.txt
   text	   data	    bss	    dec	    hex	filename
    180	      0	      2	    182	     b6	a.out

yyrkoon, chicken and Fmilburn like this

Share this post


Link to post
Share on other sites

I think that is probably a gcc only optimization (forcing always_inline). I'm not sure if you can do that with the ti version.

 

What I find interesting about discussions like this is that I'm often wrong about what feels true in my gut and what is actually true.  I'm sure I'm hampered by the fact I've been doing 'C' code now since 1982. I have a bag of personal biases that are often wrong with today's more capable compilers.  Lucky for all of you, optimization technology has come a long way since the days of the pdp 11 :)

 

-rick

yyrkoon and spirilis like this

Share this post


Link to post
Share on other sites

In my project, I use a few basic macros for GPIO. The goal is, that I can easily redefine pin assignment in a central location without compromising performance or code size.

 

The macros (gpiomacros.h):

// MSP430 gpio macros
..
#define GPIO_CLEAR(port,pin) { GPIO_OUT(port) &= ~pin; }
..

 

BTW:  that macro is missing parens around the ~pin ... should be ~(pin)

yyrkoon and chicken like this

Share this post


Link to post
Share on other sites

I think that Linus put it best, for me in this case. Something to the effect of "don't make your code hard for human eyes to parse . . . it should be readable enough that a grade school student can understand it . . .". Often times I do not agree with Linus's "coding standards", but this hit's pretty close to my own "mantra". Granted sometimes I ignore my own thinking, and I think we all know where that ends up. As I've demonstrated that( unintentionally ) on these forums once or twice. Once, was my CANBUS code, becoming too complex, and in the end I found, for no good reason. But I learned a lot through that whole process.

 

For me a macro == does not compute. Because in my mind, it's not legitimate C. Syntax, or notation, whatever, does not read like a C function, or any other "normal" code  statement.  That, and I think I've read that there can be a lot of assumption as to what's really going on within a macro. That can often times lead to undesirable side-effects.

 

Additionally. Bit manipulation was a concept that was mysterious to me. Often times still I will have to sit, pause for thought, to visualize in my head what's going on. So I actually prefer to see it more often in code, when it applies.

 

So, I guess one could say when I read code, I prefer to read it like a book - Cover to cover, top to bottom, etc. Not like a TI technical manual where one often has to bounce around through multiple sections, or even "books".

Share this post


Link to post
Share on other sites

@@yyrkoon I'd argue that

  GPIO_IS_OUTPUT(1,4);
  GPIO_IS_OUTPUT(1,6);
  GPIO_IS_OUTPUT(1,0);

or

  gpio_is_output(1,4);
  gpio_is_output(1,6);
  gpio_is_output(1,0);

are easier to read and maintain than

  P1SEL &= ~(BIT4);
  P1DIR |= BIT4;
  P1SEL &= ~(BIT6);
  P1DIR |= BIT6;
  P1SEL &= ~(BIT0);
  P1DIR |= BIT0;  

And to the be a total nit picker, the last piece of code actually still relies on macros defined in the MCU specific msp430xxxx.h file. Like with turtles, the embedded C world is macros all the way down :P

Share this post


Link to post
Share on other sites

@@yyrkoon I'd argue that

  GPIO_IS_OUTPUT(1,4);
  GPIO_IS_OUTPUT(1,6);
  GPIO_IS_OUTPUT(1,0);

or

  gpio_is_output(1,4);
  gpio_is_output(1,6);
  gpio_is_output(1,0);

are easier to read and maintain than

  P1SEL &= ~(BIT4);
  P1DIR |= BIT4;
  P1SEL &= ~(BIT6);
  P1DIR |= BIT6;
  P1SEL &= ~(BIT0);
  P1DIR |= BIT0;  

And to the be a total nit picker, the last piece of code actually still relies on macros defined in the MCU specific msp430xxxx.h file. Like with turtles, the embedded C world is macros all the way down :P

LOL ! I was actually coming back just now to say that I'm being sort of hypocritical. Because PxXXX and BITx are all macro's . . . so yes you're right. Still, these are macro's that are well documented all over the web, and seem to be consistent between the different MSP430 tool chains.

 

EDIT:

What I mean by readability though. I do not know what gpio_is_output() *is*, or does. I can guess what it is or does. But on the other hand P2DIR |= BIT4 is rather obvious. In my mind using macro's, or functions to do what you can in a single line is really not needed, and lends towards spaghetti code. Granted, it's not really a single line, if you include the macros code too. But it's pretty much "standardized". *shrug* As I said it's just a personal thing.

Share this post


Link to post
Share on other sites

I think that Linus put it best, for me in this case. Something to the effect of "don't make your code hard for human eyes to parse . . . it should be readable enough that a grade school student can understand it . . .". Often times I do not agree with Linus's "coding standards", but this hit's pretty close to my own "mantra". Granted sometimes I ignore my own thinking, and I think we all know where that ends up. As I've demonstrated that( unintentionally ) on these forums once or twice. Once, was my CANBUS code, becoming too complex, and in the end I found, for no good reason. But I learned a lot through that whole process.

 

For me a macro == does not compute. Because in my mind, it's not legitimate C. Syntax, or notation, whatever, does not read like a C function, or any other "normal" code  statement.  That, and I think I've read that there can be a lot of assumption as to what's really going on within a macro. That can often times lead to undesirable side-effects.

 

Additionally. Bit manipulation was a concept that was mysterious to me. Often times still I will have to sit, pause for thought, to visualize in my head what's going on. So I actually prefer to see it more often in code, when it applies.

 

So, I guess one could say when I read code, I prefer to read it like a book - Cover to cover, top to bottom, etc. Not like a TI technical manual where one often has to bounce around through multiple sections, or even "books".

I do agree that code should be readable. Bet then again, there's a trade off between readable low-level code and readable business logic.

I'd much rather see some unclear macros or templates to allow me to use cleaner calls in my "real" program, than have a murky program that reads like a book.

Things like set_pin_as_output() and set_high() are quite clear in their intention, it's okay to take a while to understand their inner workings, as long as the code using it is more readable thanks to using them.

Share this post


Link to post
Share on other sites

EDIT:

What I mean by readability though. I do not know what gpio_is_output() *is*, or does. I can guess what it is or does. But on the other hand P1DIR |= BIT4 is rather obvious.

Do you really know what I'm doing there?  I don't think so. I'm setting P1.4 as output and turning on the SEL bit for it also.  When those two registers are activated, it outputs the SMCLK on the P1.4 pin.  How is that obvious?

 

more obvious would be a function called:

__attribute__((always_inline))
static void CPU_output_SMCLK()
{
    P1DIR |= BIT4;
    P1SEL |= BIT4;
}

Oh yeah and that PORT/PIN/SEL setting combination is only for the msp430g2553, other chips output the clock on different ports and pins. How is just using the bare registers more readable?

 

-rick

Frida likes this

Share this post


Link to post
Share on other sites

Interesting stuff. I was thinking about this only yesterday, primarily because I have a project that I switch between a F5529LP dev board and a F5510 final PCB. That means it's helpful to be able to switch pins and ports with some sections like this

 

#if defined (__MSP430F5510__)
#define LED_RED_PORT	P2OUT
#define LED_RED_PIN 	BIT1
#endif

#if defined (__MSP430F5529__)
#define LED_RED_PORT P1OUT
#define LED_RED_PIN BIT0
#endif

Usage:
  LED_RED_PORT |= LED_RED_PIN;
This simple stuff is all fine, but gets messy when using interrupts and needing to define multiple PxDIR, PxIN, PxOUT, PxSELn, PxIE, etc.

 

Then I started doing this sort of thing, but it started to feel a bit wrong

#define IRQ_ON P2IE |= IRQ_PIN

Usage:
  if (something)
    IRQ_ON;
I might just use the parameterized macros with ## such as GPIO_SEL(x) so I only have to define the port once. I knew I'd seen something similar once, but couldn't remember where or when. Thanks for reminding me.

Share this post


Link to post
Share on other sites

Anyone know what I'm doing wrong here?

#define LED_PORT    1
#define PxOUT(port) P##port##OUT

PxOUT(LED_PORT) ^= 0x01;
I get a compilation error - identifier "PLED_PORTOUT" is undefined.

 

I can see that the value that I pass to the parameterized macro is taken literally rather than being itself defined as 1. How do I tell the preprocessor to make the substitution?

 

My code looks similar to that above. I've tried changing the ordering, whitespace, throwing in brackets. I can't seem to get it to work.

Share this post


Link to post
Share on other sites

Ah - Just after I posted I managed to find an answer myself although it's not elegant. It seems that to expand again you need another level of redirection.

http://stackoverflow.com/questions/12630548/c-expand-macro-with-token-pasting

 

This works because of the first rather pointless looking line:

#define PxOUT(port)   PxOUT_(port)
#define PxOUT_(port)  P##port##OUT
#define LED_PORT      1

PxOUT ( LED_PORT ) ^= 0x01;
If anyone knows a slightly neater solution then let me know, but this will do.

Share this post


Link to post
Share on other sites

 

Welcome to the preprocessor. I don't know of any more elegant solution as parameters are literal, though there may be one. The second level forces the parameter to be seen as a token itself and be substituted. The initial version did not let the token be seen on its own.

 

I learned the power, and limitations, or macro expansion with asmh on MVS, writing a 12 liner to expand to the twelve days of xmas for a class. The C/C++ preprocessor is more capable, but is still not doing full featured regeps.

 

 

If anyone knows a slightly neater solution then let me know, but this will do.

Share this post


Link to post
Share on other sites

As all gio registers start with port and then OUT, IN, IE etc, example: P1OUT, with some macro word scrambling you can do this:

#define read &           // words for common gpio interaction
#define clear &= ~
#define set |=
#define toggle ^=

#define Button1(y,x)    (P1 ##x y BIT0)       // PORT and PIN (ignore the ##x y)
#define BlueLED(y,x)    (P2 ##x y BIT5) 

To use them:
BlueLED(set,DIR);                                 
BlueLED(clear,OUT);

Button1(clear,DIR);                       // a input
Button1(set,OUT);                         // pull-up resistor
Button1(set,REN);                          
Button1(set,IES);                         // falling edge
button1(clear,IFG);
Button1(set,IE);                            
Fmilburn and Rickta59 like this

Share this post


Link to post
Share on other sites

This is how I do my GPIO initialization. It doesn't handle any later GPIO accesses, but it's a nice table, and smaller and faster than configuring the bits one by one.

/* use 0 or 1 to set the output level */
#define OUTPUT        0x00    /* default */
#define INPUT         0x02
#define PULL_DOWN     0x04
#define PULL_UP       0x05
#define REDUCED_DRIVE 0x00    /* default */
#define FULL_DRIVE    0x08
#define GPIO          0x00    /* default */
#define PERIPHERAL    0x10

struct digital_io_init_f5529 {
    u8 P1[8];
    u8 P2[8];
    u8 P3[8];
    u8 P4[8];
    u8 P5[8];
    u8 P6[8];
    u8 P7[8];
    u8 P8[3];
    u8 PJ[4];
};

static void init_port(const u8 *init, unsigned int count, uint16_t int base_address)
{
    uint16_t out = 0, dir = 0, ren = 0, ds = 0, sel = 0;
    uint16_t bit;

    for (bit = 1; count > 0; init++, bit <<= 1, count--) {
        if (*init & 1)          out |= bit;
        if (!(*init & INPUT))   dir |= bit;
        if (*init & PULL_DOWN)  ren |= bit;
        if (*init & FULL_DRIVE) ds  |= bit;
        if (*init & PERIPHERAL) sel |= bit;
    }
    HWREG16(base_address + OFS_PAOUT) = out;
    HWREG16(base_address + OFS_PADIR) = dir;
    HWREG16(base_address + OFS_PAREN) = ren;
    HWREG16(base_address + OFS_PADS ) = ds;
    HWREG16(base_address + OFS_PASEL) = sel;
}

static void init_ports(const struct digital_io_init_f5529 *init)
{
    init_port(init->P1, 8 + 8, PA_BASE);
    init_port(init->P3, 8 + 8, PB_BASE);
    init_port(init->P5, 8 + 8, PC_BASE);
    init_port(init->P7, 8 + 3, PD_BASE);
    init_port(init->PJ, 4,     PJ_BASE);
}

void init()
{
    init_ports(&(const struct digital_io_init_f5529){
           .P4[4] = PERIPHERAL | OUTPUT,
           .P4[5] = PERIPHERAL | INPUT,
           .P7[0] = GPIO | INPUT | PULL_UP,
           .P7[1] = GPIO | OUTPUT | FULL_DRIVE,
           /* ... */
    });
}
Fmilburn likes this

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!


Register a new account

Sign in

Already have an account? Sign in here.


Sign In Now