On Sat, Mar 10, 2018 at 10:45 AM, Dmitry Torokhov
<dmitry.torokhov@xxxxxxxxx> wrote:
> On Mon, Mar 05, 2018 at 02:02:41PM -0800, Tim Harvey wrote:
>> Add support for dispatching Linux Input events for the various interrupts
>> the Gateworks System Controller provides.
>>
>> Cc: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>
>> Signed-off-by: Tim Harvey <tharvey@xxxxxxxxxxxxx>
>> ---
>> v2:
>> - reword Kconfig
>> - revise license comment block
>> - remove unnecessary read of status register
>> - remove unnecessary setting of input->dev.parent
>> - use dt bindings for irq to keycode mapping
>> - add support for platform-data
>> - remove work-queue
>>
>> drivers/input/misc/Kconfig | 9 ++
>> drivers/input/misc/Makefile | 1 +
>> drivers/input/misc/gsc-input.c | 180 ++++++++++++++++++++++++++++++++
>> include/linux/platform_data/gsc_input.h | 30 ++++++
>> 4 files changed, 220 insertions(+)
>> create mode 100644 drivers/input/misc/gsc-input.c
>> create mode 100644 include/linux/platform_data/gsc_input.h
>>
>> diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
>> index 9f082a3..e05f4fe 100644
>> --- a/drivers/input/misc/Kconfig
>> +++ b/drivers/input/misc/Kconfig
>> @@ -117,6 +117,15 @@ config INPUT_E3X0_BUTTON
>> To compile this driver as a module, choose M here: the
>> module will be called e3x0_button.
>>
>> +config INPUT_GSC
>> + tristate "Gateworks System Controller input support"
>> + depends on MFD_GSC
>> + help
>> + Support GSC events as Linux input events.
>> +
>> + To compile this driver as a module, choose M here: the
>> + module will be called gsc_input.
>> +
>> config INPUT_PCSPKR
>> tristate "PC Speaker support"
>> depends on PCSPKR_PLATFORM
>> diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
>> index 4b6118d..969debe 100644
>> --- a/drivers/input/misc/Makefile
>> +++ b/drivers/input/misc/Makefile
>> @@ -38,6 +38,7 @@ obj-$(CONFIG_INPUT_GP2A) += gp2ap002a00f.o
>> obj-$(CONFIG_INPUT_GPIO_BEEPER) += gpio-beeper.o
>> obj-$(CONFIG_INPUT_GPIO_TILT_POLLED) += gpio_tilt_polled.o
>> obj-$(CONFIG_INPUT_GPIO_DECODER) += gpio_decoder.o
>> +obj-$(CONFIG_INPUT_GSC) += gsc-input.o
>> obj-$(CONFIG_INPUT_HISI_POWERKEY) += hisi_powerkey.o
>> obj-$(CONFIG_HP_SDC_RTC) += hp_sdc_rtc.o
>> obj-$(CONFIG_INPUT_IMS_PCU) += ims-pcu.o
>> diff --git a/drivers/input/misc/gsc-input.c b/drivers/input/misc/gsc-input.c
>> new file mode 100644
>> index 0000000..27b5e93
>> --- /dev/null
>> +++ b/drivers/input/misc/gsc-input.c
>> @@ -0,0 +1,180 @@
>> +/* SPDX-License-Identifier: GPL-2.0
>> + *
>> + * Copyright (C) 2018 Gateworks Corporation
>> + *
>> + * This driver dispatches Linux input events for GSC interrupt events
>> + */
>> +#include <linux/input.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/of_irq.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/regmap.h>
>> +
>> +#include <linux/mfd/gsc.h>
>> +#include <linux/platform_data/gsc_input.h>
>> +
>> +struct gsc_input_button_priv {
>> + struct input_dev *input;
>> + const struct gsc_input_button *button;
>> +};
>> +
>> +struct gsc_input_data {
>> + struct gsc_dev *gsc;
>> + struct gsc_input_platform_data *pdata;
>> + struct input_dev *input;
>> + struct gsc_input_button_priv *buttons;
>> + int nbuttons;
>> + unsigned int irqs[];
>> +#if 0
>> + int irq;
>> + struct work_struct irq_work;
>> + struct mutex mutex;
>> +#endif
>> +};
>> +
>> +static irqreturn_t gsc_input_irq(int irq, void *data)
>> +{
>> + const struct gsc_input_button_priv *button_priv = data;
>> + struct input_dev *input = button_priv->input;
>> +
>> + dev_dbg(&input->dev, "irq%d code=%d\n", irq, button_priv->button->code);
>> + input_report_key(input, button_priv->button->code, 1);
>> + input_sync(input);
>> + input_report_key(input, button_priv->button->code, 0);
>> + input_sync(input);
>> +
>> + return IRQ_HANDLED;
>
> Hmm, so in the end this is just a bunch of buttons connected to
> interrupt lines. We already have a driver for that, called gpio-keys. It
> can work in pure interrupt mode (i.e. do not need real GPIO pin, just
> interrupt, and it will generate key down and up events when interrupt
> arrives, with possible delay for the up event).
>
Dmitry,
Yes that's what it does and yes your correct the gpio-keys driver will
work perfect for that. Thanks for calling me out on that - I had not
realized the gpio-keys driver could work from 'interrupts'.
I'll remove the input driver in my next submission. At least I know
how to properly write an input driver now from your previous review :)
Thanks,
Tim
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
