Re: [PATCH 7/12] drivers: PMC MSP71xx GPIO char driver

[Andrew Morton <akpm@linux-foundation.org>]

On Tue, 19 Jun 2007 17:27:55 -0700 Marc St-Jean <Marc_St-Jean@pmc-sierra.com> wrote:

> Andrew Morton wrote:
> > On Fri, 15 Jun 2007 14:46:03 -0600
> > Marc St-Jean <stjeanma@pmc-sierra.com> wrote:
> > 
> >  > [PATCH 7/12] drivers: PMC MSP71xx GPIO char driver
> >  >
> >  > Patch to add a GPIO char driver for the PMC-Sierra MSP71xx devices.
> > 
> 
> >  > ...
> >  >
> >  > +/* Maps 'basic' pins to relative offset from 0 per register */
> >  > +static int const MSP_GPIO_OFFSET[] = {
> >  > +     /* GPIO 0 and 1 on the first register */
> >  > +     0, 0,
> >  > +     /* GPIO 2, 3, 4, and 5 on the second register */
> >  > +     2, 2, 2, 2,
> >  > +     /* GPIO 6, 7, 8, and 9 on the third register */
> >  > +     6, 6, 6, 6,
> >  > +     /* GPIO 10, 11, 12, 13, 14, and 15 on the fourth register */
> >  > +     10, 10, 10, 10, 10, 10,
> >  > +};
> > 
> > This shouldn't be in a header file.  Because each compilation unit which
> > includes this header will (potentially) get its own copy of the data.
> > 
> > That includes any userspace apps which include this header(!)
> 
> There are other drivers which use these macros irrespective of the char
> driver being compiled in or not. I can't move this to the driver .c file
> as all the macros will become useless.

In that case this storage should be placed into a separate .c file which
the others can link against.  Creating a separate copy of this table
per-driver is bad practice.

> > inlined functions are preferred over macros.  Only use macros when for some
> > reason you *must* use macros.
> 
> Even for simple macros that have a single +, - or << ?

Sure, why not?  There's rarely any reason to use macros.

> I thought there was an advantage to using macros, allowing the compiler to
> combine a series of simple macro calls into a single constant.

It will easily do that with inlines too.