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

John Stultz
2016-06-10T07:36:09Z

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