On Sat, May 17, 2014 at 4:37 PM, Alexandre Courbot <[email protected]> wrote:
> On Thu, May 15, 2014 at 12:44 AM, Zhu, Lejun <[email protected]> 
> 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-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to