* Re: [PATCH v6] Fix modifier keys for Redragon Asura Keyboard
From: Benjamin Tissoires @ 2018-04-17 8:50 UTC (permalink / raw)
To: Robert Munteanu; +Cc: Jiri Kosina, lkml, open list:HID CORE LAYER
In-Reply-To: <20180416213824.14688-1-rombert@apache.org>
On Mon, Apr 16, 2018 at 11:38 PM, Robert Munteanu <rombert@apache.org> wrote:
> This adds a new driver for the Redragon Asura keyboard. The Asura
> keyboard contains an error in the HID descriptor which causes all
> modifier keys to be mapped to left shift. Additionally, we suppress
> the creation of a second, not working, keyboard device.
>
> Signed-off-by: Robert Munteanu <rombert@apache.org>
> ---
Looks good to me now:
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Thanks Robert for respinning the series!
Cheers,
Benjamin
> drivers/hid/Kconfig | 7 ++++
> drivers/hid/Makefile | 1 +
> drivers/hid/hid-ids.h | 1 +
> drivers/hid/hid-redragon.c | 86 ++++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 95 insertions(+)
> create mode 100644 drivers/hid/hid-redragon.c
>
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index 19c499f5623d..1125e4813716 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -560,6 +560,13 @@ config HID_MAYFLASH
> Say Y here if you have HJZ Mayflash PS3 game controller adapters
> and want to enable force feedback support.
>
> +config HID_REDRAGON
> + tristate "Redragon keyboards"
> + depends on HID
> + default !EXPERT
> + ---help---
> + Support for Redragon keyboards that need fix-ups to work properly.
> +
> config HID_MICROSOFT
> tristate "Microsoft non-fully HID-compliant devices"
> depends on HID
> diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
> index eb13b9e92d85..a36f3f40ba63 100644
> --- a/drivers/hid/Makefile
> +++ b/drivers/hid/Makefile
> @@ -84,6 +84,7 @@ hid-picolcd-$(CONFIG_DEBUG_FS) += hid-picolcd_debugfs.o
>
> obj-$(CONFIG_HID_PLANTRONICS) += hid-plantronics.o
> obj-$(CONFIG_HID_PRIMAX) += hid-primax.o
> +obj-$(CONFIG_HID_REDRAGON) += hid-redragon.o
> obj-$(CONFIG_HID_RETRODE) += hid-retrode.o
> obj-$(CONFIG_HID_ROCCAT) += hid-roccat.o hid-roccat-common.o \
> hid-roccat-arvo.o hid-roccat-isku.o hid-roccat-kone.o \
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index 9454ac134ce2..41a64d0e91f9 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -599,6 +599,7 @@
> #define USB_VENDOR_ID_JESS 0x0c45
> #define USB_DEVICE_ID_JESS_YUREX 0x1010
> #define USB_DEVICE_ID_ASUS_MD_5112 0x5112
> +#define USB_DEVICE_ID_REDRAGON_ASURA 0x760b
>
> #define USB_VENDOR_ID_JESS2 0x0f30
> #define USB_DEVICE_ID_JESS2_COLOR_RUMBLE_PAD 0x0111
> diff --git a/drivers/hid/hid-redragon.c b/drivers/hid/hid-redragon.c
> new file mode 100644
> index 000000000000..daf59578bf93
> --- /dev/null
> +++ b/drivers/hid/hid-redragon.c
> @@ -0,0 +1,86 @@
> +/*
> + * HID driver for Redragon keyboards
> + *
> + * Copyright (c) 2017 Robert Munteanu
> + * SPDX-License-Identifier: GPL-2.0+
> + */
> +
> +/*
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the Free
> + * Software Foundation; either version 2 of the License, or (at your option)
> + * any later version.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/hid.h>
> +#include <linux/module.h>
> +
> +#include "hid-ids.h"
> +
> +
> +/*
> + * The Redragon Asura keyboard sends an incorrect HID descriptor.
> + * At byte 100 it contains
> + *
> + * 0x81, 0x00
> + *
> + * which is Input (Data, Arr, Abs), but it should be
> + *
> + * 0x81, 0x02
> + *
> + * which is Input (Data, Var, Abs), which is consistent with the way
> + * key codes are generated.
> + */
> +
> +static __u8 *redragon_report_fixup(struct hid_device *hdev, __u8 *rdesc,
> + unsigned int *rsize)
> +{
> + if (*rsize >= 102 && rdesc[100] == 0x81 && rdesc[101] == 0x00) {
> + dev_info(&hdev->dev, "Fixing Redragon ASURA report descriptor.\n");
> + rdesc[101] = 0x02;
> + }
> +
> + return rdesc;
> +}
> +
> +static int redragon_probe(struct hid_device *dev,
> + const struct hid_device_id *id)
> +{
> + int ret;
> +
> + ret = hid_parse(dev);
> + if (ret) {
> + hid_err(dev, "parse failed\n");
> + return ret;
> + }
> +
> + /* do not register unused input device */
> + if (dev->maxapplication == 1)
> + return 0;
> +
> + ret = hid_hw_start(dev, HID_CONNECT_DEFAULT);
> + if (ret) {
> + hid_err(dev, "hw start failed\n");
> + return ret;
> + }
> +
> + return 0;
> +}
> +static const struct hid_device_id redragon_devices[] = {
> + {HID_USB_DEVICE(USB_VENDOR_ID_JESS, USB_DEVICE_ID_REDRAGON_ASURA)},
> + {}
> +};
> +
> +MODULE_DEVICE_TABLE(hid, redragon_devices);
> +
> +static struct hid_driver redragon_driver = {
> + .name = "redragon",
> + .id_table = redragon_devices,
> + .report_fixup = redragon_report_fixup,
> + .probe = redragon_probe
> +};
> +
> +module_hid_driver(redragon_driver);
> +
> +MODULE_LICENSE("GPL");
> --
> 2.16.3
>
^ permalink raw reply
* RE: [PATCH v2 0/9] Input: support for latest Lenovo thinkpads (series 80)
From: 廖崇榮 @ 2018-04-17 8:40 UTC (permalink / raw)
To: 'Benjamin Tissoires'
Cc: 'Oliver Haessler', 'Benjamin Berg',
'Rob Herring', devicetree,
'open list:HID CORE LAYER', 'lkml',
'Dmitry Torokhov', 'Aaron Ma'
In-Reply-To: <CAO-hwJJnHPEQ+R+8qWb1Dn+otqgmbcKgiNDOyNkVBD+xdzY_zw@mail.gmail.com>
Hi Benjamin,
I agree this series.
Thanks
KT
-----Original Message-----
From: Benjamin Tissoires [mailto:benjamin.tissoires@redhat.com]
Sent: Tuesday, April 17, 2018 4:34 PM
To: 廖崇榮
Cc: Oliver Haessler; Benjamin Berg; Rob Herring; devicetree@vger.kernel.org; open list:HID CORE LAYER; lkml; Dmitry Torokhov; Aaron Ma
Subject: Re: [PATCH v2 0/9] Input: support for latest Lenovo thinkpads (series 80)
Hi KT,
gentle ping :)
Could you ACK/NACK this series?
Dmitry, the first patch could go without KT's approval. Also I realized that Aaron submitted a similar patch for the X1 Carbon last
October: https://patchwork.kernel.org/patch/10008513/
So I think the first one should go ASAP now that the laptops are shipping from Lenovo.
Cheers,
Benjamin
On Mon, Apr 9, 2018 at 11:10 AM, Benjamin Tissoires <benjamin.tissoires@redhat.com> wrote:
> Hi Dmitry,
>
> Here is the v2 of the Lenovo 80 series.
> Changes from v1:
> - included patch to convert a function to static from build bot
> - use of device property instead of platform data (thus the new device tree
> binding)
>
> BTW, KT, if you want to add your Signed-off-by on the patches, feel
> free to do so. You were of tremendous help :)
>
> Cheers,
> Benjamin
>
> Benjamin Tissoires (9):
> Input: synaptics - add Lenovo 80 series ids to SMBus
> input: elan_i2c_smbus - fix corrupted stack
> Input: elan_i2c - add trackstick report
> Input: elantech - split device info into a separate structure
> Input: elantech_query_info() can be static
> Input: elantech - query the resolution in query_info
> Input: elantech - add support for SMBus devices
> Input: elantech - detect new ICs and setup Host Notify for them
> input: psmouse-smbus: allow to control psmouse_deactivate
>
> .../devicetree/bindings/input/elan_i2c.txt | 1 +
> drivers/input/mouse/Kconfig | 12 +
> drivers/input/mouse/elan_i2c_core.c | 90 +++-
> drivers/input/mouse/elan_i2c_smbus.c | 22 +-
> drivers/input/mouse/elantech.c | 479 +++++++++++++++------
> drivers/input/mouse/elantech.h | 69 ++-
> drivers/input/mouse/psmouse-base.c | 21 +-
> drivers/input/mouse/psmouse-smbus.c | 24 +-
> drivers/input/mouse/psmouse.h | 2 +
> drivers/input/mouse/synaptics.c | 5 +-
> 10 files changed, 565 insertions(+), 160 deletions(-)
>
> --
> 2.14.3
>
^ permalink raw reply
* Re: [PATCH v2 0/9] Input: support for latest Lenovo thinkpads (series 80)
From: Benjamin Tissoires @ 2018-04-17 8:34 UTC (permalink / raw)
To: 廖崇榮
Cc: Oliver Haessler, Benjamin Berg, Rob Herring, devicetree,
open list:HID CORE LAYER, lkml, Dmitry Torokhov, Aaron Ma
In-Reply-To: <20180409091051.2944-1-benjamin.tissoires@redhat.com>
Hi KT,
gentle ping :)
Could you ACK/NACK this series?
Dmitry, the first patch could go without KT's approval. Also I
realized that Aaron submitted a similar patch for the X1 Carbon last
October: https://patchwork.kernel.org/patch/10008513/
So I think the first one should go ASAP now that the laptops are
shipping from Lenovo.
Cheers,
Benjamin
On Mon, Apr 9, 2018 at 11:10 AM, Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
> Hi Dmitry,
>
> Here is the v2 of the Lenovo 80 series.
> Changes from v1:
> - included patch to convert a function to static from build bot
> - use of device property instead of platform data (thus the new device tree
> binding)
>
> BTW, KT, if you want to add your Signed-off-by on the patches, feel free to
> do so. You were of tremendous help :)
>
> Cheers,
> Benjamin
>
> Benjamin Tissoires (9):
> Input: synaptics - add Lenovo 80 series ids to SMBus
> input: elan_i2c_smbus - fix corrupted stack
> Input: elan_i2c - add trackstick report
> Input: elantech - split device info into a separate structure
> Input: elantech_query_info() can be static
> Input: elantech - query the resolution in query_info
> Input: elantech - add support for SMBus devices
> Input: elantech - detect new ICs and setup Host Notify for them
> input: psmouse-smbus: allow to control psmouse_deactivate
>
> .../devicetree/bindings/input/elan_i2c.txt | 1 +
> drivers/input/mouse/Kconfig | 12 +
> drivers/input/mouse/elan_i2c_core.c | 90 +++-
> drivers/input/mouse/elan_i2c_smbus.c | 22 +-
> drivers/input/mouse/elantech.c | 479 +++++++++++++++------
> drivers/input/mouse/elantech.h | 69 ++-
> drivers/input/mouse/psmouse-base.c | 21 +-
> drivers/input/mouse/psmouse-smbus.c | 24 +-
> drivers/input/mouse/psmouse.h | 2 +
> drivers/input/mouse/synaptics.c | 5 +-
> 10 files changed, 565 insertions(+), 160 deletions(-)
>
> --
> 2.14.3
>
^ permalink raw reply
* Re: [PATCH v2 1/3] Input: ti_am335x_tsc - Mark IRQ as wakeup capable
From: Vignesh R @ 2018-04-17 8:20 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Strashko, Grygorii, linux-input@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-omap@vger.kernel.org,
Tony Lindgren
In-Reply-To: <20180416174542.GB77055@dtor-ws>
Hi,
On Monday 16 April 2018 11:15 PM, Dmitry Torokhov wrote:
> On Sat, Apr 14, 2018 at 03:21:51PM +0530, Vignesh R wrote:
>> On AM335x, ti_am335x_tsc can wake up the system from suspend, mark the
>> IRQ as wakeup capable, so that device irq is not disabled during system
>> suspend.
>>
>> Signed-off-by: Vignesh R <vigneshr@ti.com>
>> ---
>>
>> v2: No changes
>>
>> drivers/input/touchscreen/ti_am335x_tsc.c | 9 +++++++++
>> 1 file changed, 9 insertions(+)
>>
>> diff --git a/drivers/input/touchscreen/ti_am335x_tsc.c b/drivers/input/touchscreen/ti_am335x_tsc.c
>> index f1043ae71dcc..810e05c9c4f5 100644
>> --- a/drivers/input/touchscreen/ti_am335x_tsc.c
>> +++ b/drivers/input/touchscreen/ti_am335x_tsc.c
>> @@ -27,6 +27,7 @@
>> #include <linux/of.h>
>> #include <linux/of_device.h>
>> #include <linux/sort.h>
>> +#include <linux/pm_wakeirq.h>
>>
>> #include <linux/mfd/ti_am335x_tscadc.h>
>>
>> @@ -432,6 +433,12 @@ static int titsc_probe(struct platform_device *pdev)
>> goto err_free_mem;
>> }
>>
>> + if (device_may_wakeup(tscadc_dev->dev)) {
>> + err = dev_pm_set_wake_irq(tscadc_dev->dev, ts_dev->irq);
>
> Hmm, most of the drivers simply use enable_irq_wake()/disable_irq_wake()
> in suspend/resume paths
dev_pm_*_wake_irq() function are alternative to above
[1]:
For most drivers, we should be able to drop the following
boilerplate code from runtime_suspend and runtime_resume
functions:
...
device_init_wakeup(dev, true);
...
if (device_may_wakeup(dev))
enable_irq_wake(irq);
...
if (device_may_wakeup(dev))
disable_irq_wake(irq);
...
device_init_wakeup(dev, false);
...
We can replace it with just the following init and exit
time code:
...
device_init_wakeup(dev, true);
dev_pm_set_wake_irq(dev, irq);
...
dev_pm_clear_wake_irq(dev);
device_init_wakeup(dev, false);
[1]https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/base/power/wakeirq.c?id=4990d4fe327b9d9a7a3be7103a82699406fdde69
I see device_may_wakeup() check is not required. Will drop that in next version.
> and use dev_pm_set_wake_irq() only for dedicated
> and distinct wake interrupts. Why do we not follow the same pattern
> here?
Thats dev_pm_*_dedicated_wake_irq()
Thanks for the review!
--
Regards
Vignesh
^ permalink raw reply
* Re: [PATCH v2 3/3] Input: ti_am335x_tsc - Prevent system suspend when TSC is in use
From: Vignesh R @ 2018-04-17 8:19 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Strashko, Grygorii, linux-input@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-omap@vger.kernel.org,
Tony Lindgren
In-Reply-To: <20180416180148.GD77055@dtor-ws>
On Monday 16 April 2018 11:31 PM, Dmitry Torokhov wrote:
> On Sat, Apr 14, 2018 at 03:21:53PM +0530, Vignesh R wrote:
>> From: Grygorii Strashko <grygorii.strashko@ti.com>
>>
>> Prevent system suspend while user has finger on touch screen,
>> because TSC is wakeup source and suspending device while in use will
>> result in failure to disable the module.
>> This patch uses pm_stay_awake() and pm_relax() APIs to prevent and
>> resume system suspend as required.
>
> This looks like common behavior for all touchscreens and many other
> input devices, but other systems seem to cope without having to add
> pm_stay_awake() and pm_relax(). I wonder why your system requires it and
> whether we can generalize this somehow.
>
Not sure if other touch drivers with IP level wakeup, have been tested
in the same way as above scenario. But, if user has finger on
touchscreen, its better not to enter suspend until user stops
interacting with wakeup source.
I see pm_stay_awake()/pm_relax() pair as the only way to stop system
suspend when wakeup source is active. Also, one other touchscreen
driver(zforce_ts.c) and gpio_keys.c use pm_stay_awake()/pm_relax() pair.
Regards
Vignesh
>>
>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>> Signed-off-by: Vignesh R <vigneshr@ti.com>
>> ---
>>
>> v2: No changes.
>>
>> drivers/input/touchscreen/ti_am335x_tsc.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/input/touchscreen/ti_am335x_tsc.c b/drivers/input/touchscreen/ti_am335x_tsc.c
>> index dcd9db768169..43b22e071842 100644
>> --- a/drivers/input/touchscreen/ti_am335x_tsc.c
>> +++ b/drivers/input/touchscreen/ti_am335x_tsc.c
>> @@ -275,6 +275,7 @@ static irqreturn_t titsc_irq(int irq, void *dev)
>> if (status & IRQENB_HW_PEN) {
>> ts_dev->pen_down = true;
>> irqclr |= IRQENB_HW_PEN;
>> + pm_stay_awake(ts_dev->mfd_tscadc->dev);
>> }
>>
>> if (status & IRQENB_PENUP) {
>> @@ -284,6 +285,7 @@ static irqreturn_t titsc_irq(int irq, void *dev)
>> input_report_key(input_dev, BTN_TOUCH, 0);
>> input_report_abs(input_dev, ABS_PRESSURE, 0);
>> input_sync(input_dev);
>> + pm_relax(ts_dev->mfd_tscadc->dev);
>> } else {
>> ts_dev->pen_down = true;
>> }
>> @@ -524,6 +526,7 @@ static int __maybe_unused titsc_resume(struct device *dev)
>> titsc_writel(ts_dev, REG_IRQWAKEUP,
>> 0x00);
>> titsc_writel(ts_dev, REG_IRQCLR, IRQENB_HW_PEN);
>> + pm_relax(ts_dev->mfd_tscadc->dev);
>> }
>> titsc_step_config(ts_dev);
>> titsc_writel(ts_dev, REG_FIFO0THR,
>> --
>> 2.17.0
>>
>
> Thanks.
>
> --
> Dmitry
^ permalink raw reply
* Re: [PATCH v2 2/3] Input: ti_am335x_tsc - Ack pending IRQs at probe and before suspend
From: Vignesh R @ 2018-04-17 8:19 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Strashko, Grygorii, linux-input@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-omap@vger.kernel.org,
Tony Lindgren
In-Reply-To: <20180416175909.GC77055@dtor-ws>
On Monday 16 April 2018 11:29 PM, Dmitry Torokhov wrote:
> On Sat, Apr 14, 2018 at 03:21:52PM +0530, Vignesh R wrote:
>> From: Grygorii Strashko <grygorii.strashko@ti.com>
>>
>> It is seen that just enabling the TSC module triggers a HW_PEN IRQ
>> without any interaction with touchscreen by user. This results in first
>> suspend/resume sequence to fail as system immediately wakes up from
>> suspend as soon as HW_PEN IRQ is enabled in suspend handler due to the
>> pending IRQ. Therefore clear all IRQs at probe and also in suspend
>
> Are the interrupts configured as edge?
>
No, its a level interrupt.
>> callback for sanity.
>>
>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>> Signed-off-by: Vignesh R <vigneshr@ti.com>
>> Acked-by: Lee Jones <lee.jones@linaro.org>
>> ---
>>
>> v2: Add Acks from v1.
>>
>> drivers/input/touchscreen/ti_am335x_tsc.c | 2 ++
>> include/linux/mfd/ti_am335x_tscadc.h | 1 +
>> 2 files changed, 3 insertions(+)
>>
>> diff --git a/drivers/input/touchscreen/ti_am335x_tsc.c b/drivers/input/touchscreen/ti_am335x_tsc.c
>> index 810e05c9c4f5..dcd9db768169 100644
>> --- a/drivers/input/touchscreen/ti_am335x_tsc.c
>> +++ b/drivers/input/touchscreen/ti_am335x_tsc.c
>> @@ -439,6 +439,7 @@ static int titsc_probe(struct platform_device *pdev)
>> dev_err(&pdev->dev, "irq wake enable failed.\n");
>> }
>>
>> + titsc_writel(ts_dev, REG_IRQSTATUS, IRQENB_MASK);
>> titsc_writel(ts_dev, REG_IRQENABLE, IRQENB_FIFO0THRES);
>> titsc_writel(ts_dev, REG_IRQENABLE, IRQENB_EOS);
>
> Out of curiosity, should this be:
>
> titsc_writel(ts_dev, REG_IRQENABLE,
> IRQENB_FIFO0THRES | IRQENB_EOS);
>
> ?
>
> Because 2nd titsc_writel() overwrites the first? Or separate writes to
> the same register are distinct?
>
As per TRM, writing 0 to any bit of REG_IRQENABLE has no effect(IRQs are
cleared by writing to REG_IRQCLR). Therefore, second write does not
overwrite the first. I agree that there is nothing that prevents us from
enabling both IRQs in a single write. That can be a separate cleanup patch.
>> err = titsc_config_wires(ts_dev);
>> @@ -504,6 +505,7 @@ static int __maybe_unused titsc_suspend(struct device *dev)
>>
>> tscadc_dev = ti_tscadc_dev_get(to_platform_device(dev));
>> if (device_may_wakeup(tscadc_dev->dev)) {
>> + titsc_writel(ts_dev, REG_IRQSTATUS, IRQENB_MASK);
>
> The comment in titsc_irq() says that we should not be touching
> IRQENB_FIFO0THRES as it is used by another driver, but here we are
> whacking it. Can you elaborate why this is safe?
>
> You might need to rework the interrupt handling since you have several
> drivers...
>
I guess you meant IRQENB_FIFO1THRES(FIFO0 is used for TSC and FIFO1 is
ADC). Yes, this driver must not touch FIFO1 related IRQs, I will fix
that up in next version.
>> idle = titsc_readl(ts_dev, REG_IRQENABLE);
>> titsc_writel(ts_dev, REG_IRQENABLE,
>> (idle | IRQENB_HW_PEN));
>> diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
>> index b9a53e013bff..1a6a34f726cc 100644
>> --- a/include/linux/mfd/ti_am335x_tscadc.h
>> +++ b/include/linux/mfd/ti_am335x_tscadc.h
>> @@ -63,6 +63,7 @@
>> #define IRQENB_FIFO1OVRRUN BIT(6)
>> #define IRQENB_FIFO1UNDRFLW BIT(7)
>> #define IRQENB_PENUP BIT(9)
>> +#define IRQENB_MASK (0x7FF)
>>
>> /* Step Configuration */
>> #define STEPCONFIG_MODE_MASK (3 << 0)
>> --
>> 2.17.0
>>
>
> Thanks.
>
> --
> Dmitry
--
Regards
Vignesh
^ permalink raw reply
* Re: [PATCH 2/2] input: misc: Add Spreadtrum vibrator driver
From: Baolin Wang @ 2018-04-17 7:49 UTC (permalink / raw)
To: Marcus Folkesson
Cc: Dmitry Torokhov, Rob Herring, Mark Rutland, Orson Zhai,
Chunyan Zhang, open list:HID CORE LAYER, DTML, LKML, xiaotong.lu
In-Reply-To: <20180417072530.GA4629@gmail.com>
Hi Marcus,
On 17 April 2018 at 15:25, Marcus Folkesson <marcus.folkesson@gmail.com> wrote:
> Hi Xiaotong,
>
> On Tue, Apr 17, 2018 at 11:18:24AM +0800, Baolin Wang wrote:
>> From: Xiaotong Lu <xiaotong.lu@spreadtrum.com>
>
> [snip]
>
>> +static int sc27xx_vibra_probe(struct platform_device *pdev)
>> +{
>> + struct device_node *node = pdev->dev.of_node;
>> + struct vibra_info *info;
>> + int ret;
>> +
>> + info = devm_kzalloc(&pdev->dev, sizeof(*info), GFP_KERNEL);
>> + if (!info)
>> + return -ENOMEM;
>> +
>> + info->regmap = dev_get_regmap(pdev->dev.parent, NULL);
>> + if (!info->regmap) {
>> + dev_err(&pdev->dev, "failed to get vibrator regmap.\n");
>> + return -ENODEV;
>> + }
>> +
>> + ret = of_property_read_u32(node, "reg", &info->base);
>> + if (ret) {
>> + dev_err(&pdev->dev, "failed to get vibrator base address.\n");
>> + return ret;
>> + }
>> +
>> + info->input_dev = devm_input_allocate_device(&pdev->dev);
>> + if (!info->input_dev) {
>> + dev_err(&pdev->dev, "failed to allocate input device.\n");
>> + return -ENOMEM;
>> + }
>> +
>> + info->input_dev->name = "sc27xx:vibrator";
>> + info->input_dev->id.version = 0;
>> + info->input_dev->dev.parent = pdev->dev.parent;
>> + info->input_dev->close = sc27xx_vibra_close;
>> +
>> + input_set_drvdata(info->input_dev, info);
>> + input_set_capability(info->input_dev, EV_FF, FF_RUMBLE);
>> + INIT_WORK(&info->play_work, sc27xx_vibra_play_work);
>> + info->enabled = false;
>> +
>> + ret = input_ff_create_memless(info->input_dev, NULL, sc27xx_vibra_play);
>> + if (ret) {
>> + dev_err(&pdev->dev, "failed to register vibrator to FF.\n");
>> + return ret;
>> + }
>> +
>> + ret = input_register_device(info->input_dev);
>> + if (ret) {
>> + dev_err(&pdev->dev, "failed to register input device.\n");
>> + input_ff_destroy(info->input_dev);
>
> I'm not sure how the input_ff is freed for managed devices.
>
> Either you don't have to destroy it here, or you also need to destroy it
> in a release() function.
I checked again, we do not need to destroy it manually. Will remove in
next version.
>
>> + return ret;
>> + }
>> +
>> + ret = sc27xx_vibra_hw_init(info);
>> + if (ret) {
>> + dev_err(&pdev->dev, "failed to initialize the vibrator.\n");
>> + input_ff_destroy(info->input_dev);
>> + input_unregister_device(info->input_dev);
>
> No need to unregister managed input devices.
You are correct. Will remove these in next version. Thanks for your comments.
--
Baolin.wang
Best Regards
^ permalink raw reply
* Re: [PATCH v3 06/11] iio: inkern: add module put/get on iio dev module when requesting channels
From: Eugen Hristev @ 2018-04-17 7:39 UTC (permalink / raw)
To: Dmitry Torokhov, Jonathan Cameron
Cc: ludovic.desroches, alexandre.belloni, linux-arm-kernel,
devicetree, linux-kernel, linux-iio, linux-input, nicolas.ferre,
robh
In-Reply-To: <20180416235821.GF77055@dtor-ws>
On 17.04.2018 02:58, Dmitry Torokhov wrote:
> On Sun, Apr 15, 2018 at 08:33:21PM +0100, Jonathan Cameron wrote:
>> On Tue, 10 Apr 2018 11:57:52 +0300
>> Eugen Hristev <eugen.hristev@microchip.com> wrote:
>>
>>> When requesting channels for a particular consumer device,
>>> besides requesting the device (incrementing the reference counter), also
>>> do it for the driver module of the iio dev. This will avoid the situation
>>> where the producer IIO device can be removed and the consumer is still
>>> present in the kernel.
>>>
>>> Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
>>> ---
>>> drivers/iio/inkern.c | 8 +++++++-
>>> 1 file changed, 7 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
>>> index ec98790..68d9b87 100644
>>> --- a/drivers/iio/inkern.c
>>> +++ b/drivers/iio/inkern.c
>>> @@ -11,6 +11,7 @@
>>> #include <linux/slab.h>
>>> #include <linux/mutex.h>
>>> #include <linux/of.h>
>>> +#include <linux/module.h>
>>>
>>> #include <linux/iio/iio.h>
>>> #include "iio_core.h"
>>> @@ -152,6 +153,7 @@ static int __of_iio_channel_get(struct iio_channel *channel,
>>> if (index < 0)
>>> goto err_put;
>>> channel->channel = &indio_dev->channels[index];
>>> + try_module_get(channel->indio_dev->driver_module);
>>
>> And if it fails? (the module we are trying to get is going away...)
>>
>> We should try and handle it I think. Be it by just erroring out of here.
>
> Even more, this has nothing to do with modules. A device can go away for
> any number of reasons (we unbind it manually via sysfs, we pull the USB
> plug from the host in case it is USB-connected device, we unload I2C
> adapter for the bus device resides on, we kick underlying PCI device)
> and we should be able to handle this in some fashion. Handling errors
> from reads and ignoring garbage is one of methods.
>
> FWIW this is a NACK from me.
>
> Thanks.
Hello,
This patch is actually a "best effort attempt" for the consumer driver
(touch driver) to get a reference to the producer of the data (the IIO
device), when it requests the specific channels.
As of this moment, there is no attempt whatsoever for the consumer to
have a reference on the producer driver. Thus, the producer can be
removed at any time, and the consumer will fail ungraciously.
I can change the perspective from "best effort" to "mandatory" to get a
reference to the producer, or you wish to stop trying to get any
reference at all (remove this patch completely) ?
Thanks,
Eugen
>
^ permalink raw reply
* atmel_mxt_ts on droid4: powersave seems to break the driver
From: Pavel Machek @ 2018-04-17 7:38 UTC (permalink / raw)
To: mutt, kernel list, linux-arm-kernel, linux-omap, tony, sre,
nekit1000, mpartap, merlijn, nick, dmitry.torokhov, rydberg,
linux-input
[-- Attachment #1: Type: text/plain, Size: 637 bytes --]
HI!
v4.17-rc1 on motorola droid 4.
If I disable/reenable touschreen with xinput, it fails, with this in
the logs:
[ 1633.749450] cpcap-usb-phy cpcap-usb-phy.0: connected to USB host
[ 1655.938751] atmel_mxt_ts 1-004a: __mxt_read_reg: i2c transfer
failed (-121)
[ 1655.945800] atmel_mxt_ts 1-004a: Failed to read T44 and T5 (-121)
[ 1663.829498] cpcap-usb-phy cpcap-usb-phy.0: connected to USB host
Unfortunately, xscreensaver seems to break it in a similar way.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* Re: [PATCH v3 03/11] iio: Add channel for Position Relative
From: Eugen Hristev @ 2018-04-17 7:30 UTC (permalink / raw)
To: Jonathan Cameron
Cc: ludovic.desroches, alexandre.belloni, linux-arm-kernel,
devicetree, linux-kernel, linux-iio, linux-input, nicolas.ferre,
dmitry.torokhov, robh
In-Reply-To: <20180415202942.5c7d50a8@archlinux>
On 15.04.2018 22:29, Jonathan Cameron wrote:
> On Tue, 10 Apr 2018 11:57:49 +0300
> Eugen Hristev <eugen.hristev@microchip.com> wrote:
>
>> Add new channel type for relative position on a pad.
>>
>> These type of analog sensor offers the position of a pen
>> on a touchpad, and is represented as a voltage, which can be
>> converted to a position on X and Y axis on the pad.
>> The channel will hand the relative position on the pad in both directions.
>>
>> The channel can then be consumed by a touchscreen driver or
>> read as-is for a raw indication of the touchpen on a touchpad.
>>
>> Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
>> ---
>> Changes in v2:
>> - modified channel name to relative position as suggested.
>> - modified kernel version to 4.18 (presumable)
>>
>> Documentation/ABI/testing/sysfs-bus-iio | 12 ++++++++++++
>> drivers/iio/industrialio-core.c | 1 +
>> include/uapi/linux/iio/types.h | 1 +
>> tools/iio/iio_event_monitor.c | 2 ++
>> 4 files changed, 16 insertions(+)
>>
>> diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
>> index 6a5f34b..42a9287 100644
>> --- a/Documentation/ABI/testing/sysfs-bus-iio
>> +++ b/Documentation/ABI/testing/sysfs-bus-iio
>> @@ -190,6 +190,18 @@ Description:
>> but should match other such assignments on device).
>> Units after application of scale and offset are m/s^2.
>>
>> +What: /sys/bus/iio/devices/iio:deviceX/in_positionrelative_x_raw
>> +What: /sys/bus/iio/devices/iio:deviceX/in_positionrelative_y_raw
>> +KernelVersion: 4.18
>> +Contact: linux-iio@vger.kernel.org
>> +Description:
>> + Relative position in direction x or y on a pad (may be
>> + arbitrarily assigned but should match other such assignments on
>> + device).
>> + Units after application of scale and offset are milli percents
>> + from the pad's size in both directions. Should be calibrated by
>> + the consumer.
> I know the milli percent comes form the humidity equivalent, but I wonder
> if we are right to follow that. 10^-5 is a pretty random base unit (though
> I got argued into it being a standard choice for humidity sensors IIRC...
>
> What do people think? We could go with 1 for full range or just percent perhaps?
>
> I'm not that fussed about staying consistent with humidity - we are unlikely
> to end up with sensors doing both anytime soon so there shouldn't be
> any confusion...
>
> Jonathan
Hello Jonathan,
For my specific use case, percents are not enough, as this would mean
the resolution will be pretty low. If we have a touchpanel of a higher
resolution, having the position only ranged 0 to 100 is pretty bad.
Having millipercents means we can have a resolution up to 100,000 points
in range, which is fine for the hardware as of this moment.
Of course, specific drivers reporting these channels can define a
specific range if they desire (report just in the first percent if you
wish (0-1000) or so, which would mean a lower resolution.
Centi-percent would work fine as well I believe (0-10,000 range), but to
keep consistent with your suggestion regarding relative humidity, I
picked this larger scale.
In my specific case, since we have a 12 bit ADC, range is 0-4096 for
this value.
Calibrating these values to the actual size of the touchpanel is left
for the consumer to do (map 4096x4096 to the actual resolution)
If you have a better idea than milli-percents I can change it, no problem.
Eugen
>> +
>> [...]
^ permalink raw reply
* Re: [PATCH 2/2] input: misc: Add Spreadtrum vibrator driver
From: Marcus Folkesson @ 2018-04-17 7:25 UTC (permalink / raw)
To: Baolin Wang
Cc: dmitry.torokhov, robh+dt, mark.rutland, orsonzhai, zhang.lyra,
linux-input, devicetree, linux-kernel, xiaotong.lu
In-Reply-To: <b5580f22aaed9845af9b981319d6cefa04fddd35.1523934640.git.baolin.wang@linaro.org>
Hi Xiaotong,
On Tue, Apr 17, 2018 at 11:18:24AM +0800, Baolin Wang wrote:
> From: Xiaotong Lu <xiaotong.lu@spreadtrum.com>
[snip]
> +static int sc27xx_vibra_probe(struct platform_device *pdev)
> +{
> + struct device_node *node = pdev->dev.of_node;
> + struct vibra_info *info;
> + int ret;
> +
> + info = devm_kzalloc(&pdev->dev, sizeof(*info), GFP_KERNEL);
> + if (!info)
> + return -ENOMEM;
> +
> + info->regmap = dev_get_regmap(pdev->dev.parent, NULL);
> + if (!info->regmap) {
> + dev_err(&pdev->dev, "failed to get vibrator regmap.\n");
> + return -ENODEV;
> + }
> +
> + ret = of_property_read_u32(node, "reg", &info->base);
> + if (ret) {
> + dev_err(&pdev->dev, "failed to get vibrator base address.\n");
> + return ret;
> + }
> +
> + info->input_dev = devm_input_allocate_device(&pdev->dev);
> + if (!info->input_dev) {
> + dev_err(&pdev->dev, "failed to allocate input device.\n");
> + return -ENOMEM;
> + }
> +
> + info->input_dev->name = "sc27xx:vibrator";
> + info->input_dev->id.version = 0;
> + info->input_dev->dev.parent = pdev->dev.parent;
> + info->input_dev->close = sc27xx_vibra_close;
> +
> + input_set_drvdata(info->input_dev, info);
> + input_set_capability(info->input_dev, EV_FF, FF_RUMBLE);
> + INIT_WORK(&info->play_work, sc27xx_vibra_play_work);
> + info->enabled = false;
> +
> + ret = input_ff_create_memless(info->input_dev, NULL, sc27xx_vibra_play);
> + if (ret) {
> + dev_err(&pdev->dev, "failed to register vibrator to FF.\n");
> + return ret;
> + }
> +
> + ret = input_register_device(info->input_dev);
> + if (ret) {
> + dev_err(&pdev->dev, "failed to register input device.\n");
> + input_ff_destroy(info->input_dev);
I'm not sure how the input_ff is freed for managed devices.
Either you don't have to destroy it here, or you also need to destroy it
in a release() function.
> + return ret;
> + }
> +
> + ret = sc27xx_vibra_hw_init(info);
> + if (ret) {
> + dev_err(&pdev->dev, "failed to initialize the vibrator.\n");
> + input_ff_destroy(info->input_dev);
> + input_unregister_device(info->input_dev);
No need to unregister managed input devices.
> + return ret;
> + }
Cheers,
Marcus Folkesson
^ permalink raw reply
* Re: [PATCH] Input: i8042 - Fix KBD port cannot wake up system from suspend-to-idle
From: Kai-Heng Feng @ 2018-04-17 6:48 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, linux-kernel
In-Reply-To: <20180413215746.GA27465@dtor-ws>
at 5:57 AM, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> On Wed, Apr 11, 2018 at 04:59:05PM +0800, Kai-Heng Feng wrote:
>> Commit f13b2065de81 ("Input: i8042 - allow KBD and AUX ports to wake up
>> from suspend-to-idle") make system in s2idle can be woken up by i8042
>> keyboard, but it's disabled by default.
>>
>> In commit 3e6e15a862a0 ("Input: enable remote wakeup for PNP i8042
>> keyboard ports") states that "Keyboard ports are always supposed to be
>> wakeup-enabled", it should be enabled by default. Keyboard wakeup from
>> s2idles is also the default behavior for other OSes.
>>
>> But right now we can't wake up the system by keyboard, from s2idle.
>>
>> In i8042_probe(), device_set_wakeup_enable(), which gets called by
>> i8042_pnp_kbd_probe(), runs before device_set_wakeup_capable(), which
>> gets called by i8042_register_ports(). So device_set_wakeup_enable()
>> doesn't really enable wakeup for keyboard.
>
> You are talking about 2 different devices here, one representing PNP and
> another KBD serio port. Unfortunately there is not really a link between
> the 2.
Thanks for the clarification. I can see the difference now.
Enable wakeup for the serio device is what we want here.
So does the device_set_wakeup_enable() in i8042_pnp_kbd_probe() need
another device_set_wakeup_capable()?
Otherwise I think device_set_wakeup_enable() here never works.
>
>> We can enable keyboard wakeup in i8042_register_ports() directly.
>
> No, the world is not all x86, what makes sense for x86 does not
> necessarily work for other architectures. We need to come up with a way
> for tying PNP devices and i8042 ports for x86 case.
Maybe wrap it around with "#ifdef CONFIG_X86"? I can see there are several
occurrences in the i8042.c.
Kai-Heng
>
>> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
>> ---
>> drivers/input/serio/i8042-x86ia64io.h | 3 ---
>> drivers/input/serio/i8042.c | 4 ++++
>> 2 files changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/input/serio/i8042-x86ia64io.h
>> b/drivers/input/serio/i8042-x86ia64io.h
>> index b353d494ad40..e3def9195c2a 100644
>> --- a/drivers/input/serio/i8042-x86ia64io.h
>> +++ b/drivers/input/serio/i8042-x86ia64io.h
>> @@ -925,9 +925,6 @@ static int i8042_pnp_kbd_probe(struct pnp_dev *dev,
>> const struct pnp_device_id *
>> i8042_pnp_id_to_string(dev->id, i8042_kbd_firmware_id,
>> sizeof(i8042_kbd_firmware_id));
>>
>> - /* Keyboard ports are always supposed to be wakeup-enabled */
>> - device_set_wakeup_enable(&dev->dev, true);
>> -
>> i8042_pnp_kbd_devices++;
>> return 0;
>> }
>> diff --git a/drivers/input/serio/i8042.c b/drivers/input/serio/i8042.c
>> index 824f4c1c1f31..21a16b757931 100644
>> --- a/drivers/input/serio/i8042.c
>> +++ b/drivers/input/serio/i8042.c
>> @@ -1400,6 +1400,10 @@ static void __init i8042_register_ports(void)
>> i8042_ports[i].irq);
>> serio_register_port(serio);
>> device_set_wakeup_capable(&serio->dev, true);
>> +
>> + /* Keyboard ports are always supposed to be wakeup-enabled */
>> + if (i == I8042_KBD_PORT_NO)
>> + device_wakeup_enable(&serio->dev);
>> }
>> }
>> }
>> --
>> 2.17.0
>
> Thanks.
>
> --
> Dmitry
^ permalink raw reply
* Re: EXT: Re: [PATCHv1] Input: atmel_mxt_ts - fix the firmware update
From: Nandor Han @ 2018-04-17 6:37 UTC (permalink / raw)
To: Nick Dyer, Sebastian Reichel
Cc: Sebastian Reichel, Dmitry Torokhov, linux-input, Henrik Rydberg,
linux-kernel, kernel
In-Reply-To: <20180323194736.7fhgb6qjkwbivsvx@hairyalien>
On 23/03/18 21:47, Nick Dyer wrote:
> On Thu, Mar 22, 2018 at 05:43:30PM +0100, Sebastian Reichel wrote:
>> The automatic update mechanism will trigger an update if the
>> info block CRCs are different between maxtouch configuration
>> file (maxtouch.cfg) and chip.
>>
>> The driver compared the CRCs without retrieving the chip CRC,
>> resulting always in a failure and firmware flashing action
>> triggered. The patch will fix this issue by retrieving the
>> chip info block CRC before the check.
>
> Thanks for raising this, I agree it's definitely something we want to
> fix.
>
> However, I'm not convinced you're solving the problem in the best way.
> You've attached it to the read_t9_resolution() code path, whereas the
> info block is common between T9 and T100 and works in the same way.
>
> Would you mind trying the below patch? I've dusted it off from some
> work that I did back in 2012 and it should solve your issue.
>
> It also has the benefit that by reading the information block and the
> object table into a contiguous region of memory, we can verify the
> checksum at probe time. This means we make sure that we are indeed
> talking to a chip that supports object protocol correctly.
>
Thanks Nick. This looks like a better solution.
Sebastian will test this and we can see the results.
Nandor
^ permalink raw reply
* [PATCH 2/2] input: misc: Add Spreadtrum vibrator driver
From: Baolin Wang @ 2018-04-17 3:18 UTC (permalink / raw)
To: dmitry.torokhov, robh+dt, mark.rutland, orsonzhai, zhang.lyra
Cc: baolin.wang, linux-input, devicetree, linux-kernel, xiaotong.lu
In-Reply-To: <0bfdc811c2b3da3aa1cde2feef278a86503e3804.1523934640.git.baolin.wang@linaro.org>
From: Xiaotong Lu <xiaotong.lu@spreadtrum.com>
This patch adds the Spreadtrum vibrator driver, which embedded in the
Spreadtrum SC27xx series PMICs.
Signed-off-by: Xiaotong Lu <xiaotong.lu@spreadtrum.com>
Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
drivers/input/misc/Kconfig | 10 +++
drivers/input/misc/Makefile | 1 +
drivers/input/misc/sc27xx-vibra.c | 159 +++++++++++++++++++++++++++++++++++++
3 files changed, 170 insertions(+)
create mode 100644 drivers/input/misc/sc27xx-vibra.c
diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index 572b15f..c761c0c 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -841,4 +841,14 @@ config INPUT_RAVE_SP_PWRBUTTON
To compile this driver as a module, choose M here: the
module will be called rave-sp-pwrbutton.
+config INPUT_SC27XX_VIBRA
+ tristate "Spreadtrum sc27xx vibrator support"
+ depends on MFD_SC27XX_PMIC || COMPILE_TEST
+ select INPUT_FF_MEMLESS
+ help
+ This option enables support for Spreadtrum sc27xx vibrator driver.
+
+ To compile this driver as a module, choose M here. The module will
+ be called sc27xx_vibra.
+
endif
diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
index 72cde28..9d0f9d1 100644
--- a/drivers/input/misc/Makefile
+++ b/drivers/input/misc/Makefile
@@ -66,6 +66,7 @@ obj-$(CONFIG_INPUT_RETU_PWRBUTTON) += retu-pwrbutton.o
obj-$(CONFIG_INPUT_AXP20X_PEK) += axp20x-pek.o
obj-$(CONFIG_INPUT_GPIO_ROTARY_ENCODER) += rotary_encoder.o
obj-$(CONFIG_INPUT_RK805_PWRKEY) += rk805-pwrkey.o
+obj-$(CONFIG_INPUT_SC27XX_VIBRA) += sc27xx-vibra.o
obj-$(CONFIG_INPUT_SGI_BTNS) += sgi_btns.o
obj-$(CONFIG_INPUT_SIRFSOC_ONKEY) += sirfsoc-onkey.o
obj-$(CONFIG_INPUT_SOC_BUTTON_ARRAY) += soc_button_array.o
diff --git a/drivers/input/misc/sc27xx-vibra.c b/drivers/input/misc/sc27xx-vibra.c
new file mode 100644
index 0000000..a34a315
--- /dev/null
+++ b/drivers/input/misc/sc27xx-vibra.c
@@ -0,0 +1,159 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2018 Spreadtrum Communications Inc.
+ */
+
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/input.h>
+#include <linux/workqueue.h>
+
+#define CUR_DRV_CAL_SEL GENMASK(13, 12)
+#define SLP_LDOVIBR_PD_EN BIT(9)
+#define LDO_VIBR_PD BIT(8)
+
+struct vibra_info {
+ struct input_dev *input_dev;
+ struct work_struct play_work;
+ struct regmap *regmap;
+ u32 base;
+ u32 strength;
+ bool enabled;
+};
+
+static void sc27xx_vibra_set(struct vibra_info *info, bool on)
+{
+ if (on) {
+ regmap_update_bits(info->regmap, info->base, LDO_VIBR_PD, 0);
+ regmap_update_bits(info->regmap, info->base,
+ SLP_LDOVIBR_PD_EN, 0);
+ info->enabled = true;
+ } else {
+ regmap_update_bits(info->regmap, info->base, LDO_VIBR_PD,
+ LDO_VIBR_PD);
+ regmap_update_bits(info->regmap, info->base,
+ SLP_LDOVIBR_PD_EN, SLP_LDOVIBR_PD_EN);
+ info->enabled = false;
+ }
+}
+
+static int sc27xx_vibra_hw_init(struct vibra_info *info)
+{
+ return regmap_update_bits(info->regmap, info->base, CUR_DRV_CAL_SEL, 0);
+}
+
+static void sc27xx_vibra_play_work(struct work_struct *work)
+{
+ struct vibra_info *info = container_of(work, struct vibra_info,
+ play_work);
+
+ if (info->strength && !info->enabled)
+ sc27xx_vibra_set(info, true);
+ else if (info->enabled)
+ sc27xx_vibra_set(info, false);
+}
+
+static int sc27xx_vibra_play(struct input_dev *input, void *data,
+ struct ff_effect *effect)
+{
+ struct vibra_info *info = input_get_drvdata(input);
+
+ info->strength = effect->u.rumble.weak_magnitude;
+ schedule_work(&info->play_work);
+
+ return 0;
+}
+
+static void sc27xx_vibra_close(struct input_dev *input)
+{
+ struct vibra_info *info = input_get_drvdata(input);
+
+ cancel_work_sync(&info->play_work);
+ if (info->enabled)
+ sc27xx_vibra_set(info, false);
+}
+
+static int sc27xx_vibra_probe(struct platform_device *pdev)
+{
+ struct device_node *node = pdev->dev.of_node;
+ struct vibra_info *info;
+ int ret;
+
+ info = devm_kzalloc(&pdev->dev, sizeof(*info), GFP_KERNEL);
+ if (!info)
+ return -ENOMEM;
+
+ info->regmap = dev_get_regmap(pdev->dev.parent, NULL);
+ if (!info->regmap) {
+ dev_err(&pdev->dev, "failed to get vibrator regmap.\n");
+ return -ENODEV;
+ }
+
+ ret = of_property_read_u32(node, "reg", &info->base);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to get vibrator base address.\n");
+ return ret;
+ }
+
+ info->input_dev = devm_input_allocate_device(&pdev->dev);
+ if (!info->input_dev) {
+ dev_err(&pdev->dev, "failed to allocate input device.\n");
+ return -ENOMEM;
+ }
+
+ info->input_dev->name = "sc27xx:vibrator";
+ info->input_dev->id.version = 0;
+ info->input_dev->dev.parent = pdev->dev.parent;
+ info->input_dev->close = sc27xx_vibra_close;
+
+ input_set_drvdata(info->input_dev, info);
+ input_set_capability(info->input_dev, EV_FF, FF_RUMBLE);
+ INIT_WORK(&info->play_work, sc27xx_vibra_play_work);
+ info->enabled = false;
+
+ ret = input_ff_create_memless(info->input_dev, NULL, sc27xx_vibra_play);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to register vibrator to FF.\n");
+ return ret;
+ }
+
+ ret = input_register_device(info->input_dev);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to register input device.\n");
+ input_ff_destroy(info->input_dev);
+ return ret;
+ }
+
+ ret = sc27xx_vibra_hw_init(info);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to initialize the vibrator.\n");
+ input_ff_destroy(info->input_dev);
+ input_unregister_device(info->input_dev);
+ return ret;
+ }
+
+ platform_set_drvdata(pdev, info);
+ return 0;
+}
+
+static const struct of_device_id sc27xx_vibra_of_match[] = {
+ { .compatible = "sprd,sc27xx-vibrator", },
+ {}
+};
+MODULE_DEVICE_TABLE(of, sc27xx_vibra_of_match);
+
+static struct platform_driver sc27xx_vibra_driver = {
+ .driver = {
+ .name = "sc27xx-vibrator",
+ .of_match_table = sc27xx_vibra_of_match,
+ },
+ .probe = sc27xx_vibra_probe,
+};
+
+module_platform_driver(sc27xx_vibra_driver);
+
+MODULE_DESCRIPTION("Spreadtrum SC27xx Vibrator Driver");
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Xiaotong Lu <xiaotong.lu@spreadtrum.com>");
--
1.7.9.5
^ permalink raw reply related
* [PATCH 1/2] dt-bindings: input: Add Add Spreadtrum SC27xx vibrator documentation
From: Baolin Wang @ 2018-04-17 3:18 UTC (permalink / raw)
To: dmitry.torokhov, robh+dt, mark.rutland, orsonzhai, zhang.lyra
Cc: baolin.wang, linux-input, devicetree, linux-kernel, xiaotong.lu
From: Xiaotong Lu <xiaotong.lu@spreadtrum.com>
This patch adds the binding documentation for Spreadtrum SC27xx series
vibrator device.
Signed-off-by: Xiaotong Lu <xiaotong.lu@spreadtrum.com>
Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
.../bindings/input/sprd,sc27xx-vibra.txt | 12 ++++++++++++
1 file changed, 12 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/sprd,sc27xx-vibra.txt
diff --git a/Documentation/devicetree/bindings/input/sprd,sc27xx-vibra.txt b/Documentation/devicetree/bindings/input/sprd,sc27xx-vibra.txt
new file mode 100644
index 0000000..92ead29
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/sprd,sc27xx-vibra.txt
@@ -0,0 +1,12 @@
+Spreadtrum SC27xx PMIC Vibrator
+
+Required properties:
+- compatible: should be "sprd,sc27xx-vibrator".
+- reg: address of vibrator control register.
+
+Example :
+
+vibrator@eb4 {
+ compatible = "sprd,sc27xx-vibrator";
+ reg = <0xeb4>;
+};
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH v3 06/11] iio: inkern: add module put/get on iio dev module when requesting channels
From: Dmitry Torokhov @ 2018-04-16 23:58 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Eugen Hristev, ludovic.desroches, alexandre.belloni,
linux-arm-kernel, devicetree, linux-kernel, linux-iio,
linux-input, nicolas.ferre, robh
In-Reply-To: <20180415203321.1aaaf91e@archlinux>
On Sun, Apr 15, 2018 at 08:33:21PM +0100, Jonathan Cameron wrote:
> On Tue, 10 Apr 2018 11:57:52 +0300
> Eugen Hristev <eugen.hristev@microchip.com> wrote:
>
> > When requesting channels for a particular consumer device,
> > besides requesting the device (incrementing the reference counter), also
> > do it for the driver module of the iio dev. This will avoid the situation
> > where the producer IIO device can be removed and the consumer is still
> > present in the kernel.
> >
> > Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
> > ---
> > drivers/iio/inkern.c | 8 +++++++-
> > 1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
> > index ec98790..68d9b87 100644
> > --- a/drivers/iio/inkern.c
> > +++ b/drivers/iio/inkern.c
> > @@ -11,6 +11,7 @@
> > #include <linux/slab.h>
> > #include <linux/mutex.h>
> > #include <linux/of.h>
> > +#include <linux/module.h>
> >
> > #include <linux/iio/iio.h>
> > #include "iio_core.h"
> > @@ -152,6 +153,7 @@ static int __of_iio_channel_get(struct iio_channel *channel,
> > if (index < 0)
> > goto err_put;
> > channel->channel = &indio_dev->channels[index];
> > + try_module_get(channel->indio_dev->driver_module);
>
> And if it fails? (the module we are trying to get is going away...)
>
> We should try and handle it I think. Be it by just erroring out of here.
Even more, this has nothing to do with modules. A device can go away for
any number of reasons (we unbind it manually via sysfs, we pull the USB
plug from the host in case it is USB-connected device, we unload I2C
adapter for the bus device resides on, we kick underlying PCI device)
and we should be able to handle this in some fashion. Handling errors
from reads and ignoring garbage is one of methods.
FWIW this is a NACK from me.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v2 2/2] HID: input: do not increment usages when a duplicate is found
From: Dmitry Torokhov @ 2018-04-16 23:51 UTC (permalink / raw)
To: Peter Hutterer; +Cc: Benjamin Tissoires, Jiri Kosina, linux-input, linux-kernel
In-Reply-To: <20171210222926.GB11027@jelly>
On Mon, Dec 11, 2017 at 08:29:26AM +1000, Peter Hutterer wrote:
> On Fri, Dec 08, 2017 at 03:28:18PM +0100, Benjamin Tissoires wrote:
> > This is something that bothered us from a long time. When hid-input
> > doesn't know how to map a usage, it uses *_MISC. But there is something
> > else which increments the usage if the evdev code is already used.
> >
> > This leads to few issues:
> > - some devices may have their ABS_X mapped to ABS_Y if they export a bad
> > set of usages (see the DragonRise joysticks IIRC -> fixed in a specific
> > HID driver)
> > - *_MISC + N might (will) conflict with other defined axes (my Logitech
> > H800 exports some multitouch axes because of that)
> > - this prevents to freely add some new evdev usages, because "hey, my
> > headset will now report ABS_COFFEE, and it's not coffee capable".
> >
> > So let's try to kill this nonsense, and hope we won't break too many
> > devices.
> >
> > I my headset case, the ABS_MISC axes are created because of some
> > proprietary usages, so we might not break that many devices.
> >
> > For backward compatibility, a quirk HID_QUIRK_INCREMENT_USAGE_ON_DUPLICATE
> > is created and can be applied to any device that needs this behavior.
> >
> > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
>
> Acked-by: Peter Hutterer <peter.hutterer@who-t.net>
So what is happening with this series? I think we should get it them
in; there is really no reason for bumping ABS_MISC till it gets into
ABS_MT_* range on some devices that are out there.
FWIW
Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v2] Input: add bu21029 touch driver
From: Rob Herring @ 2018-04-16 21:44 UTC (permalink / raw)
To: Mark Jonas
Cc: Dmitry Torokhov, Mark Rutland, linux-input, devicetree,
linux-kernel, hs, Zhu Yi
In-Reply-To: <1523897378-23207-1-git-send-email-mark.jonas@de.bosch.com>
On Mon, Apr 16, 2018 at 06:49:38PM +0200, Mark Jonas wrote:
> From: Zhu Yi <yi.zhu5@cn.bosch.com>
>
> Add Rohm BU21029 resistive touch panel controller support with I2C
> interface.
>
> Signed-off-by: Zhu Yi <yi.zhu5@cn.bosch.com>
> Signed-off-by: Mark Jonas <mark.jonas@de.bosch.com>
> Reviewed-by: Heiko Schocher <hs@denx.de>
> ---
> Changes in v2:
> - make ABS_PRESSURE proportionally rising with finger pressure
> - fix race between interrupt and timer during shutdown
> - use infrastructure from include/linux/input/touchscreen.h
> - add SPDX tag for the driver
> - improve binding documentation
> - fix multi-line comments
> ---
> .../bindings/input/touchscreen/bu21029.txt | 34 ++
Reviewed-by: Rob Herring <robh@kernel.org>
> drivers/input/touchscreen/Kconfig | 12 +
> drivers/input/touchscreen/Makefile | 1 +
> drivers/input/touchscreen/bu21029_ts.c | 485 +++++++++++++++++++++
> 4 files changed, 532 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/input/touchscreen/bu21029.txt
> create mode 100644 drivers/input/touchscreen/bu21029_ts.c
^ permalink raw reply
* [PATCH v6] Fix modifier keys for Redragon Asura Keyboard
From: Robert Munteanu @ 2018-04-16 21:38 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: linux-kernel, linux-input, Robert Munteanu
This adds a new driver for the Redragon Asura keyboard. The Asura
keyboard contains an error in the HID descriptor which causes all
modifier keys to be mapped to left shift. Additionally, we suppress
the creation of a second, not working, keyboard device.
Signed-off-by: Robert Munteanu <rombert@apache.org>
---
drivers/hid/Kconfig | 7 ++++
drivers/hid/Makefile | 1 +
drivers/hid/hid-ids.h | 1 +
drivers/hid/hid-redragon.c | 86 ++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 95 insertions(+)
create mode 100644 drivers/hid/hid-redragon.c
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 19c499f5623d..1125e4813716 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -560,6 +560,13 @@ config HID_MAYFLASH
Say Y here if you have HJZ Mayflash PS3 game controller adapters
and want to enable force feedback support.
+config HID_REDRAGON
+ tristate "Redragon keyboards"
+ depends on HID
+ default !EXPERT
+ ---help---
+ Support for Redragon keyboards that need fix-ups to work properly.
+
config HID_MICROSOFT
tristate "Microsoft non-fully HID-compliant devices"
depends on HID
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index eb13b9e92d85..a36f3f40ba63 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -84,6 +84,7 @@ hid-picolcd-$(CONFIG_DEBUG_FS) += hid-picolcd_debugfs.o
obj-$(CONFIG_HID_PLANTRONICS) += hid-plantronics.o
obj-$(CONFIG_HID_PRIMAX) += hid-primax.o
+obj-$(CONFIG_HID_REDRAGON) += hid-redragon.o
obj-$(CONFIG_HID_RETRODE) += hid-retrode.o
obj-$(CONFIG_HID_ROCCAT) += hid-roccat.o hid-roccat-common.o \
hid-roccat-arvo.o hid-roccat-isku.o hid-roccat-kone.o \
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 9454ac134ce2..41a64d0e91f9 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -599,6 +599,7 @@
#define USB_VENDOR_ID_JESS 0x0c45
#define USB_DEVICE_ID_JESS_YUREX 0x1010
#define USB_DEVICE_ID_ASUS_MD_5112 0x5112
+#define USB_DEVICE_ID_REDRAGON_ASURA 0x760b
#define USB_VENDOR_ID_JESS2 0x0f30
#define USB_DEVICE_ID_JESS2_COLOR_RUMBLE_PAD 0x0111
diff --git a/drivers/hid/hid-redragon.c b/drivers/hid/hid-redragon.c
new file mode 100644
index 000000000000..daf59578bf93
--- /dev/null
+++ b/drivers/hid/hid-redragon.c
@@ -0,0 +1,86 @@
+/*
+ * HID driver for Redragon keyboards
+ *
+ * Copyright (c) 2017 Robert Munteanu
+ * SPDX-License-Identifier: GPL-2.0+
+ */
+
+/*
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the Free
+ * Software Foundation; either version 2 of the License, or (at your option)
+ * any later version.
+ */
+
+#include <linux/device.h>
+#include <linux/hid.h>
+#include <linux/module.h>
+
+#include "hid-ids.h"
+
+
+/*
+ * The Redragon Asura keyboard sends an incorrect HID descriptor.
+ * At byte 100 it contains
+ *
+ * 0x81, 0x00
+ *
+ * which is Input (Data, Arr, Abs), but it should be
+ *
+ * 0x81, 0x02
+ *
+ * which is Input (Data, Var, Abs), which is consistent with the way
+ * key codes are generated.
+ */
+
+static __u8 *redragon_report_fixup(struct hid_device *hdev, __u8 *rdesc,
+ unsigned int *rsize)
+{
+ if (*rsize >= 102 && rdesc[100] == 0x81 && rdesc[101] == 0x00) {
+ dev_info(&hdev->dev, "Fixing Redragon ASURA report descriptor.\n");
+ rdesc[101] = 0x02;
+ }
+
+ return rdesc;
+}
+
+static int redragon_probe(struct hid_device *dev,
+ const struct hid_device_id *id)
+{
+ int ret;
+
+ ret = hid_parse(dev);
+ if (ret) {
+ hid_err(dev, "parse failed\n");
+ return ret;
+ }
+
+ /* do not register unused input device */
+ if (dev->maxapplication == 1)
+ return 0;
+
+ ret = hid_hw_start(dev, HID_CONNECT_DEFAULT);
+ if (ret) {
+ hid_err(dev, "hw start failed\n");
+ return ret;
+ }
+
+ return 0;
+}
+static const struct hid_device_id redragon_devices[] = {
+ {HID_USB_DEVICE(USB_VENDOR_ID_JESS, USB_DEVICE_ID_REDRAGON_ASURA)},
+ {}
+};
+
+MODULE_DEVICE_TABLE(hid, redragon_devices);
+
+static struct hid_driver redragon_driver = {
+ .name = "redragon",
+ .id_table = redragon_devices,
+ .report_fixup = redragon_report_fixup,
+ .probe = redragon_probe
+};
+
+module_hid_driver(redragon_driver);
+
+MODULE_LICENSE("GPL");
--
2.16.3
^ permalink raw reply related
* Re: [PATCH v5] Fix modifier keys for Redragon Asura Keyboard
From: Robert Munteanu @ 2018-04-16 21:35 UTC (permalink / raw)
To: Benjamin Tissoires; +Cc: Jiri Kosina, lkml, open list:HID CORE LAYER
In-Reply-To: <CAO-hwJKF-=5jXN__csRJRTLBHt3cbwHWM+eTm4i8UXknuZzfHA@mail.gmail.com>
On Mon, 2018-04-16 at 15:36 +0200, Benjamin Tissoires wrote:
> > > > +MODULE_LICENSE("GPL");
> > >
> > > The SPDX header says GPL-v2. And IIRC if there is the SPDX header
> > > you
> > > can drop the MODULE_LICENSE (not entirely sure though).
> >
> > Ack, will adjust and re-test.
>
> Actually, you might be correct. I just read Mauro's blog:
> https://blogs.s-osg.org/linux-kernel-license-practices-revisited-spdx
> /
> and it says "Types of licenses for MODULE_LICENSE() macro -> 'GPL' ->
> GNU Public License v2 or later" (in the second table).
Currently the header says 'SPDX-License-Identifier: GPL-2.0', which is
'GNU General Public License v2.0 only' according to
https://spdx.org/licenses/
So we can't use that with MODULE_LICENSE("GPL") which means 'GNU
General Public License v2.0 or later' . I don't have a strong
preference for either, but I'll go with the 'or later' part since that
seems to be more in favour ( shorter argument for the MODULE_LICENSE
macro ).
Robert
^ permalink raw reply
* Re: [PATCH v6 24/30] drm/rockchip: Disable PSR on input events
From: Ezequiel Garcia @ 2018-04-16 19:42 UTC (permalink / raw)
To: Enric Balletbo i Serra, architt, inki.dae, thierry.reding, hjc,
seanpaul, airlied, tfiga, heiko
Cc: dri-devel, dianders, a.hajda, ykk, kernel, m.szyprowski,
linux-samsung-soc, jy0922.shim, rydberg, krzk, linux-rockchip,
kgene, linux-input, orjan.eide, wxt, jeffy.chen, linux-arm-kernel,
mark.yao, wzz, hl, jingoohan1, sw0312.kim, linux-kernel,
kyungmin.park, Laurent.pinchart, kuankuan.y, hshi,
Kristian H. Kristensen
In-Reply-To: <20180405095000.9756-25-enric.balletbo@collabora.com>
On Thu, 2018-04-05 at 11:49 +0200, Enric Balletbo i Serra wrote:
> From: "Kristian H. Kristensen" <hoegsberg@google.com>
>
> To improve PSR exit latency, we speculatively start exiting when we
> receive input events. Occasionally, this may lead to false positives,
> but most of the time we get a head start on coming out of PSR.
> Depending
> on how userspace takes to produce a new frame in response to the
> event,
> this can completely hide the exit latency. In case of Chrome OS, we
> typically get the input notifier 50ms or more before the dirty_fb
> triggered exit.
>
Why is this rockchip-specific? It sounds more like something
we'd want to integrate generically for drivers to leverage.
Regards,
Eze
^ permalink raw reply
* Re: [PATCH v2 3/3] Input: ti_am335x_tsc - Prevent system suspend when TSC is in use
From: Dmitry Torokhov @ 2018-04-16 18:01 UTC (permalink / raw)
To: Vignesh R
Cc: Grygorii Strashko, linux-input, linux-kernel, linux-omap,
Tony Lindgren
In-Reply-To: <20180414095153.32060-4-vigneshr@ti.com>
On Sat, Apr 14, 2018 at 03:21:53PM +0530, Vignesh R wrote:
> From: Grygorii Strashko <grygorii.strashko@ti.com>
>
> Prevent system suspend while user has finger on touch screen,
> because TSC is wakeup source and suspending device while in use will
> result in failure to disable the module.
> This patch uses pm_stay_awake() and pm_relax() APIs to prevent and
> resume system suspend as required.
This looks like common behavior for all touchscreens and many other
input devices, but other systems seem to cope without having to add
pm_stay_awake() and pm_relax(). I wonder why your system requires it and
whether we can generalize this somehow.
>
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> Signed-off-by: Vignesh R <vigneshr@ti.com>
> ---
>
> v2: No changes.
>
> drivers/input/touchscreen/ti_am335x_tsc.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/input/touchscreen/ti_am335x_tsc.c b/drivers/input/touchscreen/ti_am335x_tsc.c
> index dcd9db768169..43b22e071842 100644
> --- a/drivers/input/touchscreen/ti_am335x_tsc.c
> +++ b/drivers/input/touchscreen/ti_am335x_tsc.c
> @@ -275,6 +275,7 @@ static irqreturn_t titsc_irq(int irq, void *dev)
> if (status & IRQENB_HW_PEN) {
> ts_dev->pen_down = true;
> irqclr |= IRQENB_HW_PEN;
> + pm_stay_awake(ts_dev->mfd_tscadc->dev);
> }
>
> if (status & IRQENB_PENUP) {
> @@ -284,6 +285,7 @@ static irqreturn_t titsc_irq(int irq, void *dev)
> input_report_key(input_dev, BTN_TOUCH, 0);
> input_report_abs(input_dev, ABS_PRESSURE, 0);
> input_sync(input_dev);
> + pm_relax(ts_dev->mfd_tscadc->dev);
> } else {
> ts_dev->pen_down = true;
> }
> @@ -524,6 +526,7 @@ static int __maybe_unused titsc_resume(struct device *dev)
> titsc_writel(ts_dev, REG_IRQWAKEUP,
> 0x00);
> titsc_writel(ts_dev, REG_IRQCLR, IRQENB_HW_PEN);
> + pm_relax(ts_dev->mfd_tscadc->dev);
> }
> titsc_step_config(ts_dev);
> titsc_writel(ts_dev, REG_FIFO0THR,
> --
> 2.17.0
>
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v2 2/3] Input: ti_am335x_tsc - Ack pending IRQs at probe and before suspend
From: Dmitry Torokhov @ 2018-04-16 17:59 UTC (permalink / raw)
To: Vignesh R
Cc: Grygorii Strashko, linux-input, linux-kernel, linux-omap,
Tony Lindgren
In-Reply-To: <20180414095153.32060-3-vigneshr@ti.com>
On Sat, Apr 14, 2018 at 03:21:52PM +0530, Vignesh R wrote:
> From: Grygorii Strashko <grygorii.strashko@ti.com>
>
> It is seen that just enabling the TSC module triggers a HW_PEN IRQ
> without any interaction with touchscreen by user. This results in first
> suspend/resume sequence to fail as system immediately wakes up from
> suspend as soon as HW_PEN IRQ is enabled in suspend handler due to the
> pending IRQ. Therefore clear all IRQs at probe and also in suspend
Are the interrupts configured as edge?
> callback for sanity.
>
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> Signed-off-by: Vignesh R <vigneshr@ti.com>
> Acked-by: Lee Jones <lee.jones@linaro.org>
> ---
>
> v2: Add Acks from v1.
>
> drivers/input/touchscreen/ti_am335x_tsc.c | 2 ++
> include/linux/mfd/ti_am335x_tscadc.h | 1 +
> 2 files changed, 3 insertions(+)
>
> diff --git a/drivers/input/touchscreen/ti_am335x_tsc.c b/drivers/input/touchscreen/ti_am335x_tsc.c
> index 810e05c9c4f5..dcd9db768169 100644
> --- a/drivers/input/touchscreen/ti_am335x_tsc.c
> +++ b/drivers/input/touchscreen/ti_am335x_tsc.c
> @@ -439,6 +439,7 @@ static int titsc_probe(struct platform_device *pdev)
> dev_err(&pdev->dev, "irq wake enable failed.\n");
> }
>
> + titsc_writel(ts_dev, REG_IRQSTATUS, IRQENB_MASK);
> titsc_writel(ts_dev, REG_IRQENABLE, IRQENB_FIFO0THRES);
> titsc_writel(ts_dev, REG_IRQENABLE, IRQENB_EOS);
Out of curiosity, should this be:
titsc_writel(ts_dev, REG_IRQENABLE,
IRQENB_FIFO0THRES | IRQENB_EOS);
?
Because 2nd titsc_writel() overwrites the first? Or separate writes to
the same register are distinct?
> err = titsc_config_wires(ts_dev);
> @@ -504,6 +505,7 @@ static int __maybe_unused titsc_suspend(struct device *dev)
>
> tscadc_dev = ti_tscadc_dev_get(to_platform_device(dev));
> if (device_may_wakeup(tscadc_dev->dev)) {
> + titsc_writel(ts_dev, REG_IRQSTATUS, IRQENB_MASK);
The comment in titsc_irq() says that we should not be touching
IRQENB_FIFO0THRES as it is used by another driver, but here we are
whacking it. Can you elaborate why this is safe?
You might need to rework the interrupt handling since you have several
drivers...
> idle = titsc_readl(ts_dev, REG_IRQENABLE);
> titsc_writel(ts_dev, REG_IRQENABLE,
> (idle | IRQENB_HW_PEN));
> diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
> index b9a53e013bff..1a6a34f726cc 100644
> --- a/include/linux/mfd/ti_am335x_tscadc.h
> +++ b/include/linux/mfd/ti_am335x_tscadc.h
> @@ -63,6 +63,7 @@
> #define IRQENB_FIFO1OVRRUN BIT(6)
> #define IRQENB_FIFO1UNDRFLW BIT(7)
> #define IRQENB_PENUP BIT(9)
> +#define IRQENB_MASK (0x7FF)
>
> /* Step Configuration */
> #define STEPCONFIG_MODE_MASK (3 << 0)
> --
> 2.17.0
>
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v2 1/3] Input: ti_am335x_tsc - Mark IRQ as wakeup capable
From: Dmitry Torokhov @ 2018-04-16 17:45 UTC (permalink / raw)
To: Vignesh R
Cc: Grygorii Strashko, linux-input, linux-kernel, linux-omap,
Tony Lindgren
In-Reply-To: <20180414095153.32060-2-vigneshr@ti.com>
On Sat, Apr 14, 2018 at 03:21:51PM +0530, Vignesh R wrote:
> On AM335x, ti_am335x_tsc can wake up the system from suspend, mark the
> IRQ as wakeup capable, so that device irq is not disabled during system
> suspend.
>
> Signed-off-by: Vignesh R <vigneshr@ti.com>
> ---
>
> v2: No changes
>
> drivers/input/touchscreen/ti_am335x_tsc.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/drivers/input/touchscreen/ti_am335x_tsc.c b/drivers/input/touchscreen/ti_am335x_tsc.c
> index f1043ae71dcc..810e05c9c4f5 100644
> --- a/drivers/input/touchscreen/ti_am335x_tsc.c
> +++ b/drivers/input/touchscreen/ti_am335x_tsc.c
> @@ -27,6 +27,7 @@
> #include <linux/of.h>
> #include <linux/of_device.h>
> #include <linux/sort.h>
> +#include <linux/pm_wakeirq.h>
>
> #include <linux/mfd/ti_am335x_tscadc.h>
>
> @@ -432,6 +433,12 @@ static int titsc_probe(struct platform_device *pdev)
> goto err_free_mem;
> }
>
> + if (device_may_wakeup(tscadc_dev->dev)) {
> + err = dev_pm_set_wake_irq(tscadc_dev->dev, ts_dev->irq);
Hmm, most of the drivers simply use enable_irq_wake()/disable_irq_wake()
in suspend/resume paths and use dev_pm_set_wake_irq() only for dedicated
and distinct wake interrupts. Why do we not follow the same pattern
here?
Thanks.
> + if (err)
> + dev_err(&pdev->dev, "irq wake enable failed.\n");
> + }
> +
> titsc_writel(ts_dev, REG_IRQENABLE, IRQENB_FIFO0THRES);
> titsc_writel(ts_dev, REG_IRQENABLE, IRQENB_EOS);
> err = titsc_config_wires(ts_dev);
> @@ -462,6 +469,7 @@ static int titsc_probe(struct platform_device *pdev)
> return 0;
>
> err_free_irq:
> + dev_pm_clear_wake_irq(tscadc_dev->dev);
> free_irq(ts_dev->irq, ts_dev);
> err_free_mem:
> input_free_device(input_dev);
> @@ -474,6 +482,7 @@ static int titsc_remove(struct platform_device *pdev)
> struct titsc *ts_dev = platform_get_drvdata(pdev);
> u32 steps;
>
> + dev_pm_clear_wake_irq(ts_dev->mfd_tscadc->dev);
> free_irq(ts_dev->irq, ts_dev);
>
> /* total steps followed by the enable mask */
> --
> 2.17.0
>
--
Dmitry
^ permalink raw reply
* Re: [PATCH v6 24/30] drm/rockchip: Disable PSR on input events
From: Dmitry Torokhov @ 2018-04-16 17:41 UTC (permalink / raw)
To: Andrzej Hajda
Cc: Enric Balletbo i Serra, architt, inki.dae, thierry.reding, hjc,
seanpaul, airlied, tfiga, heiko, dri-devel, dianders, ykk, kernel,
m.szyprowski, linux-samsung-soc, jy0922.shim, rydberg, krzk,
linux-rockchip, kgene, linux-input, orjan.eide, wxt, jeffy.chen,
linux-arm-kernel, mark.yao, wzz, hl, jingoohan1, sw0312.kim,
linux-kernel, kyungmin.
In-Reply-To: <20d2aa03-63e3-0072-5ca7-5f3ca42c3c9e@samsung.com>
On Mon, Apr 16, 2018 at 09:19:21AM +0200, Andrzej Hajda wrote:
>
> +CC: linux-input list and maintainer
>
>
> On 05.04.2018 11:49, Enric Balletbo i Serra wrote:
> > From: "Kristian H. Kristensen" <hoegsberg@google.com>
> >
> > To improve PSR exit latency, we speculatively start exiting when we
> > receive input events. Occasionally, this may lead to false positives,
> > but most of the time we get a head start on coming out of PSR. Depending
> > on how userspace takes to produce a new frame in response to the event,
> > this can completely hide the exit latency. In case of Chrome OS, we
> > typically get the input notifier 50ms or more before the dirty_fb
> > triggered exit.
>
> As I see from the code below, you just need notification from input
> subsystem on user activity.
> Maybe there is some simpler notification mechanism for such things?
> If not, maybe such helper should be created in input subsystem, I
> suppose it could be reused in other places as well.
Creating an input_handler is how you get user activity. It allows you to
decide what devices you are interested in and you get events so you
decide what to do with them.
I am pretty sure using something like atomic notifier is not that much
simpler, but much less flexible.
I wonder though we we really need to open devices when we connect to
them, or if we can leave it as is and only receive events if someone
else opened the device and is consuming events.
Thanks.
>
> Regards
> Andrzej
>
> >
> > Signed-off-by: Kristian H. Kristensen <hoegsberg@google.com>
> > Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
> > Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> > Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
> > ---
> >
> > drivers/gpu/drm/rockchip/rockchip_drm_psr.c | 134 ++++++++++++++++++++++++++++
> > 1 file changed, 134 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_psr.c b/drivers/gpu/drm/rockchip/rockchip_drm_psr.c
> > index 9376f4396b6b..a107845ba97c 100644
> > --- a/drivers/gpu/drm/rockchip/rockchip_drm_psr.c
> > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_psr.c
> > @@ -12,6 +12,8 @@
> > * GNU General Public License for more details.
> > */
> >
> > +#include <linux/input.h>
> > +
> > #include <drm/drmP.h>
> > #include <drm/drm_crtc_helper.h>
> >
> > @@ -35,6 +37,9 @@ struct psr_drv {
> > enum psr_state state;
> >
> > struct delayed_work flush_work;
> > + struct work_struct disable_work;
> > +
> > + struct input_handler input_handler;
> >
> > int (*set)(struct drm_encoder *encoder, bool enable);
> > };
> > @@ -133,6 +138,18 @@ static void psr_flush_handler(struct work_struct *work)
> > mutex_unlock(&psr->lock);
> > }
> >
> > +static void psr_disable_handler(struct work_struct *work)
> > +{
> > + struct psr_drv *psr = container_of(work, struct psr_drv, disable_work);
> > +
> > + /* If the state has changed since we initiated the flush, do nothing */
> > + mutex_lock(&psr->lock);
> > + if (psr->state == PSR_ENABLE)
> > + psr_set_state_locked(psr, PSR_FLUSH);
> > + mutex_unlock(&psr->lock);
> > + mod_delayed_work(system_wq, &psr->flush_work, PSR_FLUSH_TIMEOUT_MS);
> > +}
> > +
> > /**
> > * rockchip_drm_psr_activate - activate PSR on the given pipe
> > * @encoder: encoder to obtain the PSR encoder
> > @@ -173,6 +190,7 @@ int rockchip_drm_psr_deactivate(struct drm_encoder *encoder)
> > psr->active = false;
> > mutex_unlock(&psr->lock);
> > cancel_delayed_work_sync(&psr->flush_work);
> > + cancel_work_sync(&psr->disable_work);
> >
> > return 0;
> > }
> > @@ -226,6 +244,95 @@ void rockchip_drm_psr_flush_all(struct drm_device *dev)
> > }
> > EXPORT_SYMBOL(rockchip_drm_psr_flush_all);
> >
> > +static void psr_input_event(struct input_handle *handle,
> > + unsigned int type, unsigned int code,
> > + int value)
> > +{
> > + struct psr_drv *psr = handle->handler->private;
> > +
> > + schedule_work(&psr->disable_work);
> > +}
> > +
> > +static int psr_input_connect(struct input_handler *handler,
> > + struct input_dev *dev,
> > + const struct input_device_id *id)
> > +{
> > + struct input_handle *handle;
> > + int error;
> > +
> > + handle = kzalloc(sizeof(struct input_handle), GFP_KERNEL);
> > + if (!handle)
> > + return -ENOMEM;
> > +
> > + handle->dev = dev;
> > + handle->handler = handler;
> > + handle->name = "rockchip-psr";
> > +
> > + error = input_register_handle(handle);
> > + if (error)
> > + goto err2;
> > +
> > + error = input_open_device(handle);
> > + if (error)
> > + goto err1;
> > +
> > + return 0;
> > +
> > +err1:
> > + input_unregister_handle(handle);
> > +err2:
> > + kfree(handle);
> > + return error;
> > +}
> > +
> > +static void psr_input_disconnect(struct input_handle *handle)
> > +{
> > + input_close_device(handle);
> > + input_unregister_handle(handle);
> > + kfree(handle);
> > +}
> > +
> > +/* Same device ids as cpu-boost */
> > +static const struct input_device_id psr_ids[] = {
> > + {
> > + .flags = INPUT_DEVICE_ID_MATCH_EVBIT |
> > + INPUT_DEVICE_ID_MATCH_ABSBIT,
> > + .evbit = { BIT_MASK(EV_ABS) },
> > + .absbit = { [BIT_WORD(ABS_MT_POSITION_X)] =
> > + BIT_MASK(ABS_MT_POSITION_X) |
> > + BIT_MASK(ABS_MT_POSITION_Y) },
> > + }, /* multi-touch touchscreen */
> > + {
> > + .flags = INPUT_DEVICE_ID_MATCH_EVBIT,
> > + .evbit = { BIT_MASK(EV_ABS) },
> > + .absbit = { [BIT_WORD(ABS_X)] = BIT_MASK(ABS_X) }
> > +
> > + }, /* stylus or joystick device */
> > + {
> > + .flags = INPUT_DEVICE_ID_MATCH_EVBIT,
> > + .evbit = { BIT_MASK(EV_KEY) },
> > + .keybit = { [BIT_WORD(BTN_LEFT)] = BIT_MASK(BTN_LEFT) },
> > + }, /* pointer (e.g. trackpad, mouse) */
> > + {
> > + .flags = INPUT_DEVICE_ID_MATCH_EVBIT,
> > + .evbit = { BIT_MASK(EV_KEY) },
> > + .keybit = { [BIT_WORD(KEY_ESC)] = BIT_MASK(KEY_ESC) },
> > + }, /* keyboard */
> > + {
> > + .flags = INPUT_DEVICE_ID_MATCH_EVBIT |
> > + INPUT_DEVICE_ID_MATCH_KEYBIT,
> > + .evbit = { BIT_MASK(EV_KEY) },
> > + .keybit = {[BIT_WORD(BTN_JOYSTICK)] = BIT_MASK(BTN_JOYSTICK) },
> > + }, /* joysticks not caught by ABS_X above */
> > + {
> > + .flags = INPUT_DEVICE_ID_MATCH_EVBIT |
> > + INPUT_DEVICE_ID_MATCH_KEYBIT,
> > + .evbit = { BIT_MASK(EV_KEY) },
> > + .keybit = { [BIT_WORD(BTN_GAMEPAD)] = BIT_MASK(BTN_GAMEPAD) },
> > + }, /* gamepad */
> > + { },
> > +};
> > +
> > /**
> > * rockchip_drm_psr_register - register encoder to psr driver
> > * @encoder: encoder that obtain the PSR function
> > @@ -239,6 +346,7 @@ int rockchip_drm_psr_register(struct drm_encoder *encoder,
> > {
> > struct rockchip_drm_private *drm_drv = encoder->dev->dev_private;
> > struct psr_drv *psr;
> > + int error;
> >
> > if (!encoder || !psr_set)
> > return -EINVAL;
> > @@ -248,6 +356,7 @@ int rockchip_drm_psr_register(struct drm_encoder *encoder,
> > return -ENOMEM;
> >
> > INIT_DELAYED_WORK(&psr->flush_work, psr_flush_handler);
> > + INIT_WORK(&psr->disable_work, psr_disable_handler);
> > mutex_init(&psr->lock);
> >
> > psr->active = true;
> > @@ -255,11 +364,33 @@ int rockchip_drm_psr_register(struct drm_encoder *encoder,
> > psr->encoder = encoder;
> > psr->set = psr_set;
> >
> > + psr->input_handler.event = psr_input_event;
> > + psr->input_handler.connect = psr_input_connect;
> > + psr->input_handler.disconnect = psr_input_disconnect;
> > + psr->input_handler.name =
> > + kasprintf(GFP_KERNEL, "rockchip-psr-%s", encoder->name);
> > + if (!psr->input_handler.name) {
> > + error = -ENOMEM;
> > + goto err2;
> > + }
> > + psr->input_handler.id_table = psr_ids;
> > + psr->input_handler.private = psr;
> > +
> > + error = input_register_handler(&psr->input_handler);
> > + if (error)
> > + goto err1;
> > +
> > mutex_lock(&drm_drv->psr_list_lock);
> > list_add_tail(&psr->list, &drm_drv->psr_list);
> > mutex_unlock(&drm_drv->psr_list_lock);
> >
> > return 0;
> > +
> > + err1:
> > + kfree(psr->input_handler.name);
> > + err2:
> > + kfree(psr);
> > + return error;
> > }
> > EXPORT_SYMBOL(rockchip_drm_psr_register);
> >
> > @@ -279,8 +410,11 @@ void rockchip_drm_psr_unregister(struct drm_encoder *encoder)
> > mutex_lock(&drm_drv->psr_list_lock);
> > list_for_each_entry_safe(psr, n, &drm_drv->psr_list, list) {
> > if (psr->encoder == encoder) {
> > + input_unregister_handler(&psr->input_handler);
> > cancel_delayed_work_sync(&psr->flush_work);
> > + cancel_work_sync(&psr->disable_work);
> > list_del(&psr->list);
> > + kfree(psr->input_handler.name);
> > kfree(psr);
> > }
> > }
>
>
--
Dmitry
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox