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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

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/