Re: [RFC][PATCH 4/5] drivers: input: powerkey for HISI 65xx SoC

[John Stultz]

On Wed, Jun 1, 2016 at 7:10 PM, Dmitry Torokhov
<dmitry.torokhov@xxxxxxxxx> wrote:
> Hi John,
>
> On Wed, Jun 01, 2016 at 02:27:39PM -0700, John Stultz wrote:
>> From: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@xxxxxxxxxx>
>>
>> This driver provides a input driver for the power button on the
>> HiSi 65xx SoC for boards like HiKey.
>>
>> This driver was originally by Zhiliang Xue <xuezhiliang@xxxxxxxxxx>
>> then basically rewritten by Jorge, but preserving the original
>> module author credits.
>>
>> Cc: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>
>> Cc: Rob Herring <robh+dt@xxxxxxxxxx>
>> Cc: Pawel Moll <pawel.moll@xxxxxxx>
>> Cc: Mark Rutland <mark.rutland@xxxxxxx>
>> Cc: Ian Campbell <ijc+devicetree@xxxxxxxxxxxxxx>
>> Cc: Kumar Gala <galak@xxxxxxxxxxxxxx>
>> Cc: Lee Jones <lee.jones@xxxxxxxxxx>
>> Cc: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@xxxxxxxxxx>
>> Cc: Wei Xu <xuwei5@xxxxxxxxxxxxx>
>> Cc: Guodong Xu <guodong.xu@xxxxxxxxxx>
>> Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@xxxxxxxxxx>
>> [jstultz: Reworked commit message, folded in other fixes/cleanups
>> from Jorge, and made a few small fixes and cleanups of my own]
>> Signed-off-by: John Stultz <john.stultz@xxxxxxxxxx>
>> ---
>>  drivers/input/misc/Kconfig         |   8 ++
>>  drivers/input/misc/Makefile        |   1 +
>>  drivers/input/misc/hisi_powerkey.c | 228 +++++++++++++++++++++++++++++++++++++
>>  3 files changed, 237 insertions(+)
>>  create mode 100644 drivers/input/misc/hisi_powerkey.c
>>
>> diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
>> index 1f2337a..2e57bbd 100644
>> --- a/drivers/input/misc/Kconfig
>> +++ b/drivers/input/misc/Kconfig
>> @@ -796,4 +796,12 @@ config INPUT_DRV2667_HAPTICS
>>         To compile this driver as a module, choose M here: the
>>         module will be called drv2667-haptics.
>>
>> +config HISI_POWERKEY
>> +     tristate "Hisilicon PMIC ONKEY support"
>
> Any depends on? Something MACH_XX || COMPILE_TEST?

Hey Dmitry!

Thanks so much for the review! I've got almost all your suggestions
integrated (and it greatly simplifies things) and will resend
tomorrow.

One comment on your question below...

>> +             ret = devm_request_threaded_irq(dev, irq, NULL,
>> +                                     irq_info[i].handler, IRQF_ONESHOT,
>> +                                     irq_info[i].name, priv);
>
> Why threaded irq? Seems wasteful to have 3 threads for this.

As this is a nested interrupt, devm_request_irq was failing unless it
was threaded.
Any ideas for something better?

thanks
-john