Re: [PATCH] gpio: Add support for Intel SoC PMIC (Crystal Cove)

[Linus Walleij <linus.walleij@xxxxxxxxxx>]

On Sat, May 17, 2014 at 4:37 PM, Alexandre Courbot <gnurou@xxxxxxxxx> wrote:
> On Thu, May 15, 2014 at 12:44 AM, Zhu, Lejun <lejun.zhu@xxxxxxxxxxxxxxx> wrote:

>> +static void __crystalcove_irq_type(int gpio, int type)
>> +{
>> +       u8 ctli = gpio < 8 ? GPIO0P0CTLI + gpio : GPIO1P0CTLI + (gpio - 8);
>
> Not a big comment, but wouldn't it be safer to factorize this logic
> (which is repeated in many places) into some macro? e.g. something
> like:
>
> #define GPIOP0CTL(gpio, dir) ((gpio < 8 ? GPIO0P0CTL ## dir :
> GPIO1P0CTL ## dir) + (gpio & ~0x8))

static functions are preferred over macros, please :-)

If you want to be sure they are inlined, just mark them inline (but the
compiler will do it's job for sure).

Just split the function instead so it is more readable:

static void __crystalcove_irq_type(int gpio, int type)
{
      u8 ctli;

      if (gpio < 8)
                ctli = GPIO0P0CTLI + gpio;
      else
                ctli = GPIO1P0CTLI + (gpio - 8);

But I think this can be simplified with the % operator as stated
elsewhere.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html