linux-kernel - Re: [PATCH 3/3] mfd: hi655x-pmic: Add powerkey device to hi655x PMIC driver

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] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 10 Jun 2016 00:36:04 -0700
From:	John Stultz <john.stultz@...aro.org>
To:	Lee Jones <lee.jones@...aro.org>
Cc:	lkml <linux-kernel@...r.kernel.org>,
	Dmitry Torokhov <dmitry.torokhov@...il.com>,
	Rob Herring <robh+dt@...nel.org>,
	Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@...aro.org>,
	Feng Chen <puck.chen@...ilicon.com>,
	Wei Xu <xuwei5@...ilicon.com>,
	Guodong Xu <guodong.xu@...aro.org>
Subject: Re: [PATCH 3/3] mfd: hi655x-pmic: Add powerkey device to hi655x PMIC driver

On Fri, Jun 10, 2016 at 12:32 AM, Lee Jones <lee.jones@...aro.org> wrote:
> On Thu, 09 Jun 2016, John Stultz wrote:
>
>> +static struct resource pwrkey_resources[] = {
>> +     {
>> +             .name   = "down",
>> +             .start  = 6,
>> +             .end    = 6,
>> +             .flags  = IORESOURCE_IRQ,
>> +     }, {
>> +             .name   = "up",
>> +             .start  = 5,
>> +             .end    = 5,
>> +             .flags  = IORESOURCE_IRQ,
>> +     }, {
>> +             .name   = "hold 4s",
>> +             .start  = 4,
>> +             .end    = 4,
>> +             .flags  = IORESOURCE_IRQ,
>> +     },
>> +};
>
> No idea what 4, 5 and 6 mean.  Please define them.

Fair enough. It already defines PWRON_D4SR_INT, PWRON_D20F_INT and
PWRON_D20R_INT, but those are unfortunately mask values. I'll add a
patch to rename those to be clearly mask values and generate them from
a proper define value as well.

>> +static const struct mfd_cell hi655x_pmic_devs[] = {
>> +     {
>> +             .name           = "hi655x-regulator",
>> +     },
>> +     {
>> +             .name           = "hi65xx-powerkey",
>> +             .num_resources  = ARRAY_SIZE(pwrkey_resources),
>> +             .resources      = &pwrkey_resources[0],
>> +     },
>> +};
>
> Nit: Place the -regulator entry at the bottom and turn it into a one
> liner:
>
>         { .name         = "hi655x-regulator", },

Sure.  Will do!

Thanks for the feedback!
-john