On Tue, Feb 27, 2018 at 8:59 PM, Dmitry Torokhov
<[email protected]> wrote:
> Hi Andrey,
>
> On Mon, Feb 26, 2018 at 07:41:29AM -0800, Andrey Smirnov wrote:
>> Add driver that properly handles input event emitted by RAVE SP
>> devices.
>>
>> Cc: Dmitry Torokhov <[email protected]>
>> Cc: [email protected]
>> Cc: [email protected]
>> Cc: Rob Herring <[email protected]>
>> Cc: Mark Rutland <[email protected]>
>> Cc: [email protected]
>> Cc: Guenter Roeck <[email protected]>
>> Cc: Chris Healy <[email protected]>
>> Cc: Lucas Stach <[email protected]>
>> Signed-off-by: Andrey Smirnov <[email protected]>
>> ---
>> drivers/input/misc/Kconfig | 9 ++++
>> drivers/input/misc/Makefile | 1 +
>> drivers/input/misc/rave-sp-pwrbutton.c | 92
>> ++++++++++++++++++++++++++++++++++
>> 3 files changed, 102 insertions(+)
>> create mode 100644 drivers/input/misc/rave-sp-pwrbutton.c
>>
>> diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
>> index 62a1312a7387..6a3c753b093b 100644
>> --- a/drivers/input/misc/Kconfig
>> +++ b/drivers/input/misc/Kconfig
>> @@ -841,4 +841,13 @@ config INPUT_HISI_POWERKEY
>> To compile this driver as a module, choose M here: the
>> module will be called hisi_powerkey.
>>
>> +config INPUT_RAVE_SP_PWRBUTTON
>> + tristate "RAVE SP Power button Driver"
>> + depends on RAVE_SP_CORE
>> + help
>> + Say Y here if you want to enable power key reporting from RAVE SP
>> +
>> + To compile this driver as a module, choose M here: the
>> + module will be called rave-sp-pwrbutton.
>> +
>> endif
>> diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
>> index a8f61af865aa..6ebeec1f4dd0 100644
>> --- a/drivers/input/misc/Makefile
>> +++ b/drivers/input/misc/Makefile
>> @@ -80,3 +80,4 @@ obj-$(CONFIG_INPUT_WM831X_ON) += wm831x-on.o
>> obj-$(CONFIG_INPUT_XEN_KBDDEV_FRONTEND) += xen-kbdfront.o
>> obj-$(CONFIG_INPUT_YEALINK) += yealink.o
>> obj-$(CONFIG_INPUT_IDEAPAD_SLIDEBAR) += ideapad_slidebar.o
>> +obj-$(CONFIG_INPUT_RAVE_SP_PWRBUTTON) += rave-sp-pwrbutton.o
>
> Please put it in [somewhat] alphabetical order (even though ideapad is
> out of order).
>
OK, will do in v2.
>
>> diff --git a/drivers/input/misc/rave-sp-pwrbutton.c
>> b/drivers/input/misc/rave-sp-pwrbutton.c
>> new file mode 100644
>> index 000000000000..7511062c724b
>> --- /dev/null
>> +++ b/drivers/input/misc/rave-sp-pwrbutton.c
>> @@ -0,0 +1,92 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +
>> +/*
>> + * Power Button driver for RAVE SP
>> + *
>> + * Copyright (C) 2017 Zodiac Inflight Innovations
>> + *
>> + */
>
> Let's keep SPDX and copyright in the same // comment block.
>
This goes against kernel coding style, but sure.
>> +
>> +#include <linux/input.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/mfd/rave-sp.h>
>> +#include <linux/platform_device.h>
>> +
>> +#define RAVE_SP_EVNT_BUTTON_PRESS (RAVE_SP_EVNT_BASE + 0x00)
>> +
>> +struct rave_sp_power_button {
>> + struct input_dev *idev;
>> + struct notifier_block nb;
>> +};
>> +
>> +static int rave_sp_power_button_event(struct notifier_block *nb,
>> + unsigned long action, void *data)
>> +{
>> + struct rave_sp_power_button *pb =
>> + container_of(nb, struct rave_sp_power_button, nb);
>> + const u8 event = rave_sp_action_unpack_event(action);
>> + const u8 value = rave_sp_action_unpack_value(action);
>> + struct input_dev *idev = pb->idev;
>> +
>> + if (event == RAVE_SP_EVNT_BUTTON_PRESS) {
>> + input_report_key(idev, KEY_POWER, value);
>> + input_sync(idev);
>> +
>> + return NOTIFY_STOP;
>> + }
>> +
>> + return NOTIFY_DONE;
>> +}
>> +
>> +static int rave_sp_pwrbutton_probe(struct platform_device *pdev)
>> +{
>> + struct device *dev = &pdev->dev;
>> + struct rave_sp_power_button *pb;
>> + struct input_dev *idev;
>> + int ret;
>> +
>> + pb = devm_kzalloc(dev, sizeof(*pb), GFP_KERNEL);
>> + if (!pb)
>> + return -ENOMEM;
>> +
>> + idev = devm_input_allocate_device(dev);
>> + if (!idev)
>> + return -ENOMEM;
>> +
>> + idev->name = pdev->name;
>> + idev->dev.parent = dev;
>
> Not needed - done by devm_input_allocate_device().
>
Good point, will remove.
>> +
>> + input_set_capability(idev, EV_KEY, KEY_POWER);
>> +
>> + ret = input_register_device(idev);
>> + if (ret)
>> + return ret;
>
> Please call this "error".
Sure, will do.
>
>> +
>> + pb->idev = idev;
>> + pb->nb.notifier_call = rave_sp_power_button_event;
>> + pb->nb.priority = 128;
>> +
>> + return devm_rave_sp_register_event_notifier(dev, &pb->nb);
>
> Prefer
>
> error = devm_rave...();
> if (error)
> return error;
>
> return 0;
>
OK, will do in v2.
Thanks,
Andrey Smirnov
