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

[John Stultz]

On Fri, Jun 10, 2016 at 12:32 AM, Lee Jones <lee.jones@xxxxxxxxxx> 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