[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CACRpkdbVFT23h-BCpshE9w-eK5+sdXHB_utzxAffO+iUs9cPBA@mail.gmail.com>
Date: Tue, 27 May 2014 11:01:40 +0200
From: Linus Walleij <linus.walleij@...aro.org>
To: Alexandre Courbot <gnurou@...il.com>
Cc: "Zhu, Lejun" <lejun.zhu@...ux.intel.com>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
"linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
bin.yang@...el.com
Subject: Re: [PATCH] gpio: Add support for Intel SoC PMIC (Crystal Cove)
On Sat, May 17, 2014 at 4:37 PM, Alexandre Courbot <gnurou@...il.com> wrote:
> On Thu, May 15, 2014 at 12:44 AM, Zhu, Lejun <lejun.zhu@...ux.intel.com> 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 majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists