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

John Stultz
2016-06-02T22:10:36Z

On Wed, Jun 1, 2016 at 7:10 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> Hi John,
>
> On Wed, Jun 01, 2016 at 02:27:39PM -0700, John Stultz wrote:
>> From: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>
>>
>> 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@huawei.com>
>> then basically rewritten by Jorge, but preserving the original
>> module author credits.
>>
>> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>> Cc: Rob Herring <robh+dt@kernel.org>
>> Cc: Pawel Moll <pawel.moll@arm.com>
>> Cc: Mark Rutland <mark.rutland@arm.com>
>> Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
>> Cc: Kumar Gala <galak@codeaurora.org>
>> Cc: Lee Jones <lee.jones@linaro.org>
>> Cc: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>
>> Cc: Wei Xu <xuwei5@hisilicon.com>
>> Cc: Guodong Xu <guodong.xu@linaro.org>
>> Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>
>> [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@linaro.org>
>> ---
>> 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