4
\$\begingroup\$

I have a program that accesses a certain I/O controller using memory-mapped I/O. I currently store the base address of this controller in a const variable.

This leads to a code structure something like this:

gpio.h:

int gpioFunc (unsigned int pin, unsigned int func);

int setGpio (unsigned int pin, unsigned int val);

gpio.c:

#include "gpio.h"
unsigned volatile int* const gpioAddr = (unsigned int*) 0x20200000;

int gpioFunc (unsigned int pin, unsigned int func)
{
    //code that uses the variable
}

int setGpio (unsigned int pin, unsigned int val)
{
    //code that uses the variable
}

main.c:

#include "gpio.h"
void main ()
{
    gpioFunc (16,1);
    setGpio(16,0);
    hlt();
}

I have always been uncertain of the best practices in C as to where to put constants. Does this way make sense, or would it be better practice to put it somewhere else? If so, where?

\$\endgroup\$

2 Answers 2

2
\$\begingroup\$

You should make a small driver layer, so that the main doesn't know about the low level details of what is going on - and here in that .c file that you have implemented the driver in, the constants goes as well.

Then you call your driver and all is good :)

\$\endgroup\$
2
  • \$\begingroup\$ I think this may what I have currently... how would this be different from my current implementation? Thank you for your help. \$\endgroup\$ Mar 16, 2014 at 12:18
  • \$\begingroup\$ No, this is not a driver. This is calling the bare metal layer from your main. You want to make a small driver layer, so that you get calls like "<driver_name>_get<data_pin_name>" and "<driver_name>_init". This also allows you to easily change the setup in the future, when you need to. \$\endgroup\$ Mar 16, 2014 at 12:20
2
\$\begingroup\$

I recommend you to declare gpioAddr variable as static. That way the program can only access it from gpio.c and nowhere else (unless you somehow pass the address of the variable to somewhere, which is a poor design, IMO). Making it static also helps the compiler to optimize the variable and just generates code for reading from/writing to the particular memory address.

Also, 0x20200000 is a magic number, you should consider to define it somewhere appropriate.

Lastly, casting as (unsigned volatile int*) may be better for readability, although it makes no difference.

Edit: The appropriate place for defining constants depend on the context. If these definitions are needed somewhere else, you must put them in header file. Defining them in source file is more appropriate otherwise, as unnecessary information would be hidden. Unfortunately, #define preprocessor directives in C are in the global namespace, so appropriate prefixes should be added to the name regardless of the definition place.

\$\endgroup\$
2
  • \$\begingroup\$ I am aware that I should consider to define it somewhere appropriate. The question was where would be considered appropriate? \$\endgroup\$ Mar 17, 2014 at 19:06
  • \$\begingroup\$ It is the source file if it does not needed to know from somewhere else, i.e. including a header file, header file if otherwise. I think source file is more appropriate in this occasion. \$\endgroup\$
    – obareey
    Mar 17, 2014 at 19:07

Not the answer you're looking for? Browse other questions tagged or ask your own question.