* [PATCH v2 0/6] HID: input cleanups and mt additions
From: Benjamin Tissoires @ 2018-04-24 8:04 UTC (permalink / raw)
To: Jiri Kosina
Cc: Dmitry Torokhov, Peter Hutterer, Mario.Limonciello, linux-input,
linux-kernel, Benjamin Tissoires
Hi Jiri,
following the thread about the 'not incrementing ABS_MISC', here is the
actual submission of the series.
Compared to the 2 patches I sent last week, there are a few more:
- patch 1 needed to be added or some (all?) Advanced Silicon touchscreen
would fail the tests with HID_QUIRK_INPUT_PER_APPLICATION
- patch 3 is extracted from v1-2/2, as it has grown significantly, still
because of the same issue with Advanced Silicon panels
- patch 4 is something I had in mind while debugging some other
not-so-talkative touchpads, because of patch 5
- patch 5 should put us at the same level than the Windows driver now.
Note that we could unset the features (button/surface reporting, and
latency during suspend, but not sure it helps that much. We should probably
test the current consumption with and without)
The series has been tested against regressions thanks to my new hid test suite
https://github.com/bentiss/hid-tools (I rewrote hid-replay in python and hook
up python tests). I also used the loaner from Dell (thanks Mario!), and XPS
9360 that has both touchscreen and touchpad, and on which the touchpad has the
new features I implemented here.
Cheers,
Benjamin
Benjamin Tissoires (6):
HID: store the full list of reports in the hidinput
HID: generic: create one input report per application type
HID: input: append a suffix matching the application
HID: multitouch: make use of HID_QUIRK_INPUT_PER_APP
HID: multitouch: simplify the settings of the various features
HID: multitouch: implement precision touchpad latency and switches
drivers/hid/hid-core.c | 19 ++--
drivers/hid/hid-generic.c | 15 +++
drivers/hid/hid-gfrm.c | 2 +-
drivers/hid/hid-input.c | 90 +++++++++++++++--
drivers/hid/hid-magicmouse.c | 6 +-
drivers/hid/hid-multitouch.c | 227 ++++++++++++++++++++++---------------------
include/linux/hid.h | 16 ++-
7 files changed, 244 insertions(+), 131 deletions(-)
--
2.14.3
^ permalink raw reply
* [PATCH] HID: wacom: Release device resource data obtained by devres_alloc()
From: Arvind Yadav @ 2018-04-24 8:03 UTC (permalink / raw)
To: jikos, benjamin.tissoires, jkosina; +Cc: linux-kernel, linux-input
Free device resource data, if __wacom_devm_sysfs_create_group
is not successful.
Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
---
drivers/hid/wacom_sys.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index b54ef1f..ee7a37e 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -1213,8 +1213,10 @@ static int __wacom_devm_sysfs_create_group(struct wacom *wacom,
devres->root = root;
error = sysfs_create_group(devres->root, group);
- if (error)
+ if (error) {
+ devres_free(devres);
return error;
+ }
devres_add(&wacom->hdev->dev, devres);
--
1.9.1
^ permalink raw reply related
* [PATCH] xen/kbdif: Add frontend's virtual device options
From: Oleksandr Andrushchenko @ 2018-04-24 7:31 UTC (permalink / raw)
To: linux-input, xen-devel, konrad.wilk, dmitry.torokhov
Cc: andr2000, Oleksandr Andrushchenko, jandryuk
From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
It is now not possible to control if and which virtual devices
are created by the frontend, e.g. keyboard and pointer devices
are always created and multi-touch device is created if the
backend advertises multi-touch support. In some cases this
behavior is not desirable and better control over the frontend's
configuration is required.
Add new frontend options, so it is possible to individually
control set of exposed virtual devices for each guest OS:
- set opt-no-pointer if no pointer device needs to be created
- set opt-no-keyboard if no keyboard device needs to be created
- set opt-no-multi-touch if no multi-touch device needs to be created
Keep old behavior by default.
Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
---
xen/include/public/io/kbdif.h | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/xen/include/public/io/kbdif.h b/xen/include/public/io/kbdif.h
index 3ce54e9a44c1..703433156c1f 100644
--- a/xen/include/public/io/kbdif.h
+++ b/xen/include/public/io/kbdif.h
@@ -132,6 +132,29 @@
* OBSOLETE, not recommended for use.
* PFN of the shared page.
*
+ *--------------------------- Frontend Options -------------------------------
+ *
+ * For the below options if corresponding XenStore entry omitted,
+ * then the value must be treated as set to 0.
+ *
+ * opt-no-pointer
+ * Values: <uint>
+ *
+ * If no virtual pointer device to be exposed by the frontend then
+ * this must be set to 1.
+ *
+ * opt-no-keyboard
+ * Values: <uint>
+ *
+ * If no virtual keyboard device to be exposed by the frontend then
+ * this must be set to 1.
+ *
+ * opt-no-multi-touch
+ * Values: <uint>
+ *
+ * If no virtual multi-touch device to be exposed by the frontend then
+ * this must be set to 1.
+ *
*----------------------- Multi-touch Device Parameters -----------------------
*
* multi-touch-num-contacts
@@ -181,6 +204,9 @@
#define XENKBD_FIELD_FEAT_MTOUCH "feature-multi-touch"
#define XENKBD_FIELD_REQ_ABS_POINTER "request-abs-pointer"
#define XENKBD_FIELD_REQ_MTOUCH "request-multi-touch"
+#define XENKBD_FIELD_OPT_NO_PTR "opt-no-pointer"
+#define XENKBD_FIELD_OPT_NO_KBD "opt-no-keyboard"
+#define XENKBD_FIELD_OPT_NO_MTOUCH "opt-no-multi-touch"
#define XENKBD_FIELD_RING_GREF "page-gref"
#define XENKBD_FIELD_EVT_CHANNEL "event-channel"
#define XENKBD_FIELD_WIDTH "width"
--
2.17.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related
* [PATCH v3 3/3] Input: ti_am335x_tsc - Prevent system suspend when TSC is in use
From: Vignesh R @ 2018-04-24 6:27 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Vignesh R, Grygorii Strashko, linux-input, linux-kernel,
linux-omap, Tony Lindgren
In-Reply-To: <20180424062741.7891-1-vigneshr@ti.com>
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.
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
Signed-off-by: Vignesh R <vigneshr@ti.com>
---
v3: No change
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 a548b0dbe320..ae6fc9099636 100644
--- a/drivers/input/touchscreen/ti_am335x_tsc.c
+++ b/drivers/input/touchscreen/ti_am335x_tsc.c
@@ -277,6 +277,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) {
@@ -286,6 +287,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
^ permalink raw reply related
* [PATCH v3 2/3] Input: ti_am335x_tsc - Ack pending IRQs at probe and before suspend
From: Vignesh R @ 2018-04-24 6:27 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Vignesh R, Grygorii Strashko, linux-input, linux-kernel,
linux-omap, Tony Lindgren
In-Reply-To: <20180424062741.7891-1-vigneshr@ti.com>
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
callback for sanity.
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
Signed-off-by: Vignesh R <vigneshr@ti.com>
---
v3: Clear IRQs related to TSC module only
drivers/input/touchscreen/ti_am335x_tsc.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/input/touchscreen/ti_am335x_tsc.c b/drivers/input/touchscreen/ti_am335x_tsc.c
index 3908c21862c7..a548b0dbe320 100644
--- a/drivers/input/touchscreen/ti_am335x_tsc.c
+++ b/drivers/input/touchscreen/ti_am335x_tsc.c
@@ -35,6 +35,8 @@
#define SEQ_SETTLE 275
#define MAX_12BIT ((1 << 12) - 1)
+#define TSC_IRQENB_MASK (IRQENB_FIFO0THRES | IRQENB_EOS | IRQENB_HW_PEN)
+
static const int config_pins[] = {
STEPCONFIG_XPP,
STEPCONFIG_XNN,
@@ -437,6 +439,7 @@ static int titsc_probe(struct platform_device *pdev)
if (err)
dev_err(&pdev->dev, "irq wake enable failed.\n");
+ titsc_writel(ts_dev, REG_IRQSTATUS, TSC_IRQENB_MASK);
titsc_writel(ts_dev, REG_IRQENABLE, IRQENB_FIFO0THRES);
titsc_writel(ts_dev, REG_IRQENABLE, IRQENB_EOS);
err = titsc_config_wires(ts_dev);
@@ -502,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, TSC_IRQENB_MASK);
idle = titsc_readl(ts_dev, REG_IRQENABLE);
titsc_writel(ts_dev, REG_IRQENABLE,
(idle | IRQENB_HW_PEN));
--
2.17.0
^ permalink raw reply related
* [PATCH v3 1/3] Input: ti_am335x_tsc - Mark IRQ as wakeup capable
From: Vignesh R @ 2018-04-24 6:27 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Vignesh R, Grygorii Strashko, linux-input, linux-kernel,
linux-omap, Tony Lindgren
In-Reply-To: <20180424062741.7891-1-vigneshr@ti.com>
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>
---
v3: Drop unneeded device_may_wakeup() check
drivers/input/touchscreen/ti_am335x_tsc.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/input/touchscreen/ti_am335x_tsc.c b/drivers/input/touchscreen/ti_am335x_tsc.c
index f1043ae71dcc..3908c21862c7 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,10 @@ static int titsc_probe(struct platform_device *pdev)
goto err_free_mem;
}
+ err = dev_pm_set_wake_irq(tscadc_dev->dev, ts_dev->irq);
+ 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 +467,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 +480,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
^ permalink raw reply related
* [PATCH v3 0/3] ti_am335x_tsc: Fix suspend/resume
From: Vignesh R @ 2018-04-24 6:27 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Vignesh R, Grygorii Strashko, linux-input, linux-kernel,
linux-omap, Tony Lindgren
This patch series fixes couple of issues wrt suspend/resume with TI AM335x
TSC driver. Disable and clear any pending IRQs before suspend, and
handle case where TSC wakeup would fail, if there were touch events
during suspend.
Grygorii Strashko (2):
Input: ti_am335x_tsc - Ack pending IRQs at probe and before suspend
Input: ti_am335x_tsc - Prevent system suspend when TSC is in use
Vignesh R (1):
Input: ti_am335x_tsc - Mark IRQ as wakeup capable
drivers/input/touchscreen/ti_am335x_tsc.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
--
2.17.0
^ permalink raw reply
* Re: [PATCH v2] Input: xen-kbdfront - allow better run-time configuration
From: Oleksandr Andrushchenko @ 2018-04-24 6:05 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Juergen Gross, xen-devel, linux-input, linux-kernel, lyan,
boris.ostrovsky, andrii_chepurnyi, Oleksandr Andrushchenko
In-Reply-To: <20180423185530.GC66646@dtor-ws>
On 04/23/2018 09:55 PM, Dmitry Torokhov wrote:
> On Mon, Apr 23, 2018 at 11:29:45AM +0300, Oleksandr Andrushchenko wrote:
>> On 04/23/2018 11:23 AM, Juergen Gross wrote:
>>> On 23/04/18 10:02, Oleksandr Andrushchenko wrote:
>>>> Juergen, Jason, Dmitry
>>>> any comment on this?
>>> Oleksandr, please give us some time. I can't speak for others, but I am
>>> not sitting here idling and hoping that some work (e.g. patches to
>>> review) might appear.
>>>
>>> I have a lot of other stuff to do and will respond when I find some time
>>> to look at your patches.
>>>
>>> Pinging others on Monday when having sent out the patch only on Thursday
>>> is rather unfriendly.
>> Really sorry about this, my bad.
>> I had an impression that the only change we need to
>> discuss was uint -> bool change which seemed rather trivial
> I am sorry I did not respond to the previous version until today, but I
> do not think that we should be extending module parameters for this.
> Protocol features are already there for absolute pointers and
> multi-touch, keyboard and relative pointers should use the same
> mechanism.
Ok, then I'll start from extending the protocol
> Thanks.
>
^ permalink raw reply
* Re: [PATCH] Input: xen-kbdfront - allow better run-time configuration
From: Oleksandr Andrushchenko @ 2018-04-24 5:55 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Juergen Gross, xen-devel, linux-input, linux-kernel, lyan,
boris.ostrovsky, andrii_chepurnyi, Oleksandr Andrushchenko
In-Reply-To: <20180423185325.GB66646@dtor-ws>
On 04/23/2018 09:53 PM, Dmitry Torokhov wrote:
> On Thu, Apr 19, 2018 at 02:44:19PM +0300, Oleksandr Andrushchenko wrote:
>> On 04/19/2018 02:25 PM, Juergen Gross wrote:
>>> On 18/04/18 17:04, Oleksandr Andrushchenko wrote:
>>>> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
>>>>
>>>> It is now only possible to control if multi-touch virtual device
>>>> is created or not (via the corresponding XenStore entries),
>>>> but keyboard and pointer devices are always created.
>>> Why don't you want to go that route for keyboard and mouse, too?
>>> Or does this really make no sense?
>> Well, I would prefer not to touch anything outside Linux and
>> this driver. And these settings seem to be implementation specific.
>> So, this is why introduce Linux module parameters and don't extend
>> the kbdif protocol.
> Why do you consider this implementation specific? How other guests
> decide to forego creation of relative pointer device or keyboard-like
> device?
>
> You already have "features" for absolute pointing device and multitouch,
> so please extend the protocol properly so you indeed do not code
> something implementation-specific (i.e. module parameters).
Ok, but in order to preserve the default behavior, e.g.
pointer and keyboard devices are always created now, I'll have
to have reverse features in the protocol:
- feature-no-pointer
- feature-no-keyboard
The above may be set as a part of frontend's configuration and
if missed are considered to be set to false.
>
> Thanks.
>
^ permalink raw reply
* Re: [PATCH v2 2/2] input: misc: Add Spreadtrum vibrator driver
From: Baolin Wang @ 2018-04-24 2:44 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Rob Herring, Mark Rutland, Orson Zhai, Chunyan Zhang,
open list:HID CORE LAYER, DTML, LKML, xiaotong.lu
In-Reply-To: <20180423172937.GA66646@dtor-ws>
Hi Dmitry,
On 24 April 2018 at 01:29, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> Hi Xiaotong,
>
> On Mon, Apr 23, 2018 at 10:33:36AM +0800, Baolin Wang wrote:
>> 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>
>> ---
>> Changes since v1:
>> - Remove input_ff_destroy() and input_unregister_device()
>> ---
>> drivers/input/misc/Kconfig | 10 +++
>> drivers/input/misc/Makefile | 1 +
>> drivers/input/misc/sc27xx-vibra.c | 156 +++++++++++++++++++++++++++++++++++++
>> 3 files changed, 167 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..f78e70f
>> --- /dev/null
>> +++ b/drivers/input/misc/sc27xx-vibra.c
>> @@ -0,0 +1,156 @@
>> +// 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);
>
> I do not think this is correct. If you issue 2 play requests with
> info->strength that is not 0 then you'll end up disabling the vibrator.
>
> I think you want the 2nd condition to be:
>
> else if (info->strength == 0 && info->enabled)
> sc27xx_vibra_set(info, false);
>
Yes, you are right. Will fix in next patch.
>
>> +}
>> +
>> +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;
>
> Can we please call this variable "error"?
Sure.
>
>> +
>> + 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);
>
> I am fan of generic device properties, please change to
> device_property_read_u32().
OK.
>
>> + 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;
>
> Why is device reparented to the parent of platform device? This breaks
> devm implementation for the input device. If you really need this,
> you'll have to switch to unmanaged API.
Sorry, this is one mistake and will fix it in next version.
>
>> + 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");
>> + return ret;
>> + }
>> +
>> + ret = sc27xx_vibra_hw_init(info);
>> + if (ret) {
>> + dev_err(&pdev->dev, "failed to initialize the vibrator.\n");
>> + return ret;
>> + }
>
> It is too late to initialize hardware after registering the input
> device, as once registered it should be fully functional. I'd recommend
> calling this before calling input_register_device(), or (maybe even
> better) doing this as part of open() method.
OK.
>
>> +
>> + platform_set_drvdata(pdev, info);
>
> I do not think you are using platform device's driver data anywhere.
We can remove it.
>
>> + 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,
>
> Do you need suspend support by chance? To shut off the vibrator when
> system transitions to sleep?
We do not need the suspend support, disabling the vibrator is enough.
Thanks for your comments.
--
Baolin.wang
Best Regards
^ permalink raw reply
* Re: [PATCH] Input: alps - Demystify trackstick initialization for v3 and v6 protocols
From: Dmitry Torokhov @ 2018-04-23 23:40 UTC (permalink / raw)
To: Pali Rohár
Cc: linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
Masaki Ota
In-Reply-To: <20180418114030.5xkvynzeqf63ir7j@pali>
On Wed, Apr 18, 2018 at 01:40:30PM +0200, Pali Rohár wrote:
> Dmitry, ping
Applied, thank you. Can you tell me if I am missing anything else for
ALPS?
Thanks!
>
> On Wednesday 21 March 2018 17:41:26 Pali Rohár wrote:
> > That is pity, but OK.
> >
> > Anyway, as wrote patch which I sent in the first email matches this
> > documentation.
> >
> > Dmitry, can you review/comment/accept/reject this patch?
> >
> > On Monday 19 March 2018 08:41:19 Masaki Ota wrote:
> > > Hi, Pali,
> > >
> > > v3/v6 devices are T3 type, and it can use only Method 2.
> > > P36 (At this time GLIDEPOINT_T3 uses the Method 2 ....)
> > > T3 has the potential that uses Method2, but I think it needs to change Firmware.
> > >
> > > Best Regards,
> > > Masaki Ota
> > > -----Original Message-----
> > > From: Pali Rohár [mailto:pali.rohar@gmail.com]
> > > Sent: Friday, March 16, 2018 7:58 PM
> > > To: 太田 真喜 Masaki Ota <masaki.ota@jp.alps.com>
> > > Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>; linux-input@vger.kernel.org; linux-kernel@vger.kernel.org
> > > Subject: Re: [PATCH] Input: alps - Demystify trackstick initialization for v3 and v6 protocols
> > >
> > > Great, thank you. Enabling that RAW/extended/SP4 mode is done by sequence E6, E6, E6, F3, C8, F3, 14 (written in page 35) and it matches what function alps_trackstick_enter_extended_mode_v3_v6() in my patch is doing. So is correct.
> > >
> > > On page 36 I see that there is described Method 1 for reporting stick data which prevents cursor jumps. Seems that kernel uses Method 2.
> > > Method 1 depends on some prioritization.
> > >
> > > Do you have some information how to activate Method 1? Sometimes I observe that problem with "cursor jumps" and from Method 1 could prevent it. So I would like to try experimenting...
> > >
> > > On Wednesday 14 March 2018 23:56:46 Masaki Ota wrote:
> > > > Hi, Pali,
> > > >
> > > > I have added Appendix.
> > > > According to this spec documents, SP raw mode is SP 4 byte mode.
> > > > I think Extended mode meaning is almost all the same as Raw mode.
> > > > The description of how to set is written in Page 35.
> > > >
> > > > Best Regards,
> > > > Masaki Ota
> > > > -----Original Message-----
> > > > From: Pali Rohár [mailto:pali.rohar@gmail.com]
> > > > Sent: Thursday, March 15, 2018 7:58 AM
> > > > To: 太田 真喜 Masaki Ota <masaki.ota@jp.alps.com>
> > > > Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>;
> > > > linux-input@vger.kernel.org; linux-kernel@vger.kernel.org
> > > > Subject: Re: [PATCH] Input: alps - Demystify trackstick initialization
> > > > for v3 and v6 protocols
> > > >
> > > > Hi! Thank you for information.
> > > >
> > > > Your PS/2 Aux Port Control description seems to really matches. Just there is reverse order of bits. Bit 0 in description is highest, therefore matches BIT(7) macro.
> > > >
> > > > Bit 6 in description (BIT(1) in code) describes SP Extended Mode which alps.c enabled. And in that description is written:
> > > >
> > > > "If 1 SP is extended packet format (driver must set SP raw mode and GP absolute mode)."
> > > >
> > > > Do you have any idea what "SP raw mode" is? How to set it? For me it looks like it could be that extended mode of trackstick itself.
> > > >
> > > > "GP absolute mode" I guess is GlidePoint absolute mode, therefore
> > > > enable
> > > > 6 byte absolute mode for touchpad.
> > > >
> > > > And for Bit 7 (BIT(0)) is written:
> > > >
> > > > This bit is used with the PS/2 Aux port to use the Pass-Thru mode ( see appendix A ). Do you have some information about this appendix A?
> > > >
> > > > On Wednesday 14 March 2018 10:21:43 Masaki Ota wrote:
> > > > > Hi, Pali,
> > > > >
> > > > > I just picked up the spec which relates with trackstic.
> > > > >
> > > > > Best Regards,
> > > > > Masaki Ota
> > > > > -----Original Message-----
> > > > > From: Pali Rohár [mailto:pali.rohar@gmail.com]
> > > > > Sent: Tuesday, March 13, 2018 8:14 AM
> > > > > To: 太田 真喜 Masaki Ota <masaki.ota@jp.alps.com>; Dmitry Torokhov
> > > > > <dmitry.torokhov@gmail.com>
> > > > > Cc: linux-input@vger.kernel.org; linux-kernel@vger.kernel.org
> > > > > Subject: Re: [PATCH] Input: alps - Demystify trackstick
> > > > > initialization for v3 and v6 protocols
> > > > >
> > > > > Masaki, if you have access to the internal ALPS v3 / Rushmore
> > > > > documentation, I would like to have a review of this patch or
> > > > > confirmation of those information :-)
> > > >
> > > > --
> > > > Pali Rohár
> > > > pali.rohar@gmail.com
> > >
> > > --
> > > Pali Rohár
> > > pali.rohar@gmail.com
> >
>
> --
> Pali Rohár
> pali.rohar@gmail.com
--
Dmitry
^ permalink raw reply
* Re: [PATCH] Input: alps - Fix reporting pressure of v3 trackstick
From: Dmitry Torokhov @ 2018-04-23 23:26 UTC (permalink / raw)
To: Pali Rohár; +Cc: Masaki Ota, linux-input, linux-kernel
In-Reply-To: <20180408155321.10433-1-pali.rohar@gmail.com>
On Sun, Apr 08, 2018 at 05:53:21PM +0200, Pali Rohár wrote:
> According to documentation, all 7 lower bits represents trackpoint pressure.
>
> Fixes: 4621c9660459 ("Input: alps - report pressure of v3 and v7 trackstick")
> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
Applied, thank you.
> ---
> drivers/input/mouse/alps.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
> index 0a9e6a3a2f9f..1e28d9fc5492 100644
> --- a/drivers/input/mouse/alps.c
> +++ b/drivers/input/mouse/alps.c
> @@ -583,7 +583,7 @@ static void alps_process_trackstick_packet_v3(struct psmouse *psmouse)
>
> x = (s8)(((packet[0] & 0x20) << 2) | (packet[1] & 0x7f));
> y = (s8)(((packet[0] & 0x10) << 3) | (packet[2] & 0x7f));
> - z = packet[4] & 0x7c;
> + z = packet[4] & 0x7f;
>
> /*
> * The x and y values tend to be quite large, and when used
> --
> 2.11.0
>
--
Dmitry
^ permalink raw reply
* Re: [PATCH v3 08/11] input: touchscreen: resistive-adc-touch: add generic resistive ADC touchscreen
From: Dmitry Torokhov @ 2018-04-23 23:24 UTC (permalink / raw)
To: Eugen Hristev
Cc: jic23, ludovic.desroches, alexandre.belloni, linux-arm-kernel,
devicetree, linux-kernel, linux-iio, linux-input, nicolas.ferre,
robh
In-Reply-To: <20180423232251.GF66646@dtor-ws>
On Mon, Apr 23, 2018 at 04:22:51PM -0700, Dmitry Torokhov wrote:
> Hi Eugen,
>
> On Tue, Apr 10, 2018 at 11:57:54AM +0300, Eugen Hristev wrote:
> > This adds a generic resistive touchscreen (GRTS) driver, which is based
> > on an IIO device (an ADC). It must be connected to the channels of an ADC
> > to receive touch data. Then it will feed the data into the input subsystem
> > where it registers an input device.
> > It uses an IIO callback buffer to register to the IIO device
> >
> > Some parts of this patch are based on initial original work by
> > Mohamed Jamsheeth Hajanajubudeen and Bandaru Venkateswara Swamy
> >
> > Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
> > ---
> > Changes in v3:
> > - pressure now reported naturally growing down-up
> > - using helpers from touchscreen.h to parse DT properties
> > - added devm_add_action_or_reset to handle callback buffer clean on exit
> > - changed compatible and threshold property to adapt to changed bindings
> >
> > Changes in v2:
> > - this is now a generic resistive adc touchscreen driver
> >
> > drivers/input/touchscreen/Kconfig | 13 ++
> > drivers/input/touchscreen/Makefile | 1 +
> > drivers/input/touchscreen/resistive-adc-touch.c | 200 ++++++++++++++++++++++++
> > 3 files changed, 214 insertions(+)
> > create mode 100644 drivers/input/touchscreen/resistive-adc-touch.c
> >
> > diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
> > index 4f15496..8f85d3a 100644
> > --- a/drivers/input/touchscreen/Kconfig
> > +++ b/drivers/input/touchscreen/Kconfig
> > @@ -92,6 +92,19 @@ config TOUCHSCREEN_AD7879_SPI
> > To compile this driver as a module, choose M here: the
> > module will be called ad7879-spi.
> >
> > +config TOUCHSCREEN_ADC
> > + tristate "Generic ADC based resistive touchscreen"
> > + depends on IIO
> > + select IIO_BUFFER_CB
> > + help
> > + Say Y here if you want to use the generic ADC
> > + resistive touchscreen driver.
> > +
> > + If unsure, say N (but it's safe to say "Y").
> > +
> > + To compile this driver as a module, choose M here: the
> > + module will be called resistive-adc-touch.ko.
> > +
> > config TOUCHSCREEN_AR1021_I2C
> > tristate "Microchip AR1020/1021 i2c touchscreen"
> > depends on I2C && OF
> > diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
> > index dddae79..843c7f9 100644
> > --- a/drivers/input/touchscreen/Makefile
> > +++ b/drivers/input/touchscreen/Makefile
> > @@ -13,6 +13,7 @@ obj-$(CONFIG_TOUCHSCREEN_AD7877) += ad7877.o
> > obj-$(CONFIG_TOUCHSCREEN_AD7879) += ad7879.o
> > obj-$(CONFIG_TOUCHSCREEN_AD7879_I2C) += ad7879-i2c.o
> > obj-$(CONFIG_TOUCHSCREEN_AD7879_SPI) += ad7879-spi.o
> > +obj-$(CONFIG_TOUCHSCREEN_ADC) += resistive-adc-touch.o
> > obj-$(CONFIG_TOUCHSCREEN_ADS7846) += ads7846.o
> > obj-$(CONFIG_TOUCHSCREEN_AR1021_I2C) += ar1021_i2c.o
> > obj-$(CONFIG_TOUCHSCREEN_ATMEL_MXT) += atmel_mxt_ts.o
> > diff --git a/drivers/input/touchscreen/resistive-adc-touch.c b/drivers/input/touchscreen/resistive-adc-touch.c
> > new file mode 100644
> > index 0000000..7e201ee
> > --- /dev/null
> > +++ b/drivers/input/touchscreen/resistive-adc-touch.c
> > @@ -0,0 +1,200 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * ADC generic resistive touchscreen (GRTS)
> > + *
> > + * Copyright (C) 2017,2018 Microchip Technology,
> > + * Author: Eugen Hristev <eugen.hristev@microchip.com>
> > + *
> > + */
> > +#include <linux/input.h>
> > +#include <linux/input/touchscreen.h>
> > +#include <linux/iio/consumer.h>
> > +#include <linux/iio/iio.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/of_device.h>
> > +#include <linux/platform_device.h>
> > +
> > +#define DRIVER_NAME "resistive-adc-touch"
> > +#define GRTS_DEFAULT_PRESSURE_THRESHOLD 50000
> > +#define MAX_POS_MASK GENMASK(11, 0)
> > +
> > +/**
> > + * grts_state - generic resistive touch screen information struct
> > + * @pressure_threshold: number representing the threshold for the pressure
> > + * @pressure: are we getting pressure info or not
> > + * @iio_chans: list of channels acquired
> > + * @iio_cb: iio_callback buffer for the data
> > + * @input: the input device structure that we register
> > + * @prop: touchscreen properties struct
> > + */
> > +struct grts_state {
> > + u32 pressure_threshold;
> > + bool pressure;
> > + struct iio_channel *iio_chans;
> > + struct iio_cb_buffer *iio_cb;
> > + struct input_dev *input;
> > + struct touchscreen_properties prop;
> > +};
> > +
> > +static int grts_cb(const void *data, void *private)
> > +{
> > + const u16 *touch_info = data;
> > + struct grts_state *st = private;
> > +
>
> Extra blank line.
>
> > + unsigned int x, y, press = 0xFFFF;
>
> Why to you need to initialize "press"?
>
> > +
> > + /* channel data coming in buffer in the order below */
> > + x = touch_info[0];
> > + y = touch_info[1];
> > + if (st->pressure)
> > + press = touch_info[2];
> > +
> > + if ((!x && !y) || (st->pressure && (press < st->pressure_threshold))) {
> > + /* report end of touch */
>
> Hm, so this makes "press" not pressure but resistance...
Sorry, ignore this and below. I misread the code.
>
> > + input_report_key(st->input, BTN_TOUCH, 0);
> > + input_sync(st->input);
> > + return 0;
> > + }
> > +
> > + /* report proper touch to subsystem*/
> > + touchscreen_report_pos(st->input, &st->prop, x, y, false);
> > + if (st->pressure)
> > + input_report_abs(st->input, ABS_PRESSURE, press);
>
> and if this is resistance, then you want to have:
>
> input_report_abs(st->input, ABS_PRESSURE,
> st->resistance_threshold - resistance);
>
> > + input_report_key(st->input, BTN_TOUCH, 1);
> > + input_sync(st->input);
> > +
> > + return 0;
> > +}
> > +
> > +static int grts_open(struct input_dev *dev)
> > +{
> > + int error;
> > + struct grts_state *st = input_get_drvdata(dev);
> > +
> > + error = iio_channel_start_all_cb(st->iio_cb);
> > + if (error) {
> > + dev_err(dev->dev.parent, "failed to start callback buffer.\n");
> > + return error;
> > + }
> > + return 0;
> > +}
> > +
> > +static void grts_close(struct input_dev *dev)
> > +{
> > + struct grts_state *st = input_get_drvdata(dev);
> > +
> > + iio_channel_stop_all_cb(st->iio_cb);
> > +}
> > +
> > +static void grts_disable(void *data)
> > +{
> > + iio_channel_release_all_cb(data);
> > +}
> > +
> > +static int grts_probe(struct platform_device *pdev)
> > +{
> > + struct grts_state *st;
> > + struct input_dev *input;
> > + struct device *dev = &pdev->dev;
> > + struct device_node *node = dev->of_node;
> > + struct iio_channel *chan;
> > + int ret = 0;
>
> Let's call this variable "error". There is also no need to initialize it
> to 0.
>
> > +
> > + st = devm_kzalloc(dev, sizeof(struct grts_state), GFP_KERNEL);
> > + if (!st)
> > + return -ENOMEM;
> > +
> > + ret = of_property_read_u32(node, "touchscreen-threshold-pressure",
> > + &st->pressure_threshold);
>
> Let's use generic device property API (device_property_read_u32).
>
>
> > + if (ret < 0) {
>
> I do not think of_ or device_ property API returns positive on success.
> So simply
>
> if (error) {
> ...
> }
>
> > + dev_dbg(dev, "can't get touchscreen pressure threshold property.\n");
> > + st->pressure_threshold = GRTS_DEFAULT_PRESSURE_THRESHOLD;
> > + }
> > +
> > + /* get the channels from IIO device */
> > + st->iio_chans = devm_iio_channel_get_all(dev);
> > +
>
> Extra blank line.
>
> > + if (IS_ERR(st->iio_chans)) {
> > + if (PTR_ERR(st->iio_chans) != -EPROBE_DEFER)
> > + dev_err(dev, "can't get iio channels.\n");
> > + return PTR_ERR(st->iio_chans);
> > + }
> > +
> > + chan = &st->iio_chans[0];
> > + st->pressure = false;
> > + while (chan && chan->indio_dev) {
> > + if (!strcmp(chan->channel->datasheet_name, "pressure"))
> > + st->pressure = true;
> > + chan++;
> > + }
> > +
> > + input = devm_input_allocate_device(dev);
> > + if (!input) {
> > + dev_err(dev, "failed to allocate input device.\n");
> > + return -ENOMEM;
> > + }
> > +
> > + input->name = DRIVER_NAME;
> > + input->id.bustype = BUS_HOST;
> > + input->open = grts_open;
> > + input->close = grts_close;
> > +
> > + input_set_abs_params(input, ABS_X, 0, MAX_POS_MASK - 1, 0, 0);
> > + input_set_abs_params(input, ABS_Y, 0, MAX_POS_MASK - 1, 0, 0);
> > + if (st->pressure)
> > + input_set_abs_params(input, ABS_PRESSURE, 0, 0xffffff, 0, 0);
>
> I am sure we can figure the range given the resistance threshold.
>
> > +
> > + input->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
> > + input->keybit[BIT_WORD(BTN_TOUCH)] = BIT_MASK(BTN_TOUCH);
>
> Replace this 2 with
>
> input_set_capability(input, EV_KEY, BTN_TOUCH);
>
> > +
> > + /* parse optional device tree properties */
> > + touchscreen_parse_properties(input, false, &st->prop);
> > +
> > + st->input = input;
> > + input_set_drvdata(input, st);
> > +
> > + ret = input_register_device(input);
> > + if (ret) {
> > + dev_err(dev, "failed to register input device.");
> > + return ret;
> > + }
> > +
> > + st->iio_cb = iio_channel_get_all_cb(dev, grts_cb, st);
> > +
> > + if (IS_ERR(st->iio_cb)) {
> > + dev_err(dev, "failed to allocate callback buffer.\n");
> > + ret = PTR_ERR(st->iio_cb);
> > + return ret;
> > + }
> > +
> > + ret = devm_add_action_or_reset(dev, grts_disable, st->iio_cb);
>
> if (error)
> return error;
>
> > +
> > + platform_set_drvdata(pdev, st);
>
> I do not think you are using platform device driver data anywhere.
>
> > +
> > + return ret;
>
> Prefer explicit success returns, so
>
> return 0;
>
> > +}
> > +
> > +static const struct of_device_id grts_of_match[] = {
> > + {
> > + .compatible = "resistive-adc-touch",
> > + }, {
> > + /* sentinel */
> > + },
> > +};
> > +
> > +MODULE_DEVICE_TABLE(of, grts_of_match);
> > +
> > +static struct platform_driver grts_driver = {
> > + .probe = grts_probe,
> > + .driver = {
> > + .name = DRIVER_NAME,
>
> Keep indenting with tabs please.
>
> > + .of_match_table = of_match_ptr(grts_of_match),
> > + },
> > +};
> > +
> > +module_platform_driver(grts_driver);
> > +
> > +MODULE_AUTHOR("Eugen Hristev <eugen.hristev@microchip.com>");
> > +MODULE_DESCRIPTION("Generic ADC Resistive Touch Driver");
> > +MODULE_LICENSE("GPL v2");
> > --
> > 2.7.4
> >
>
> --
> Dmitry
--
Dmitry
^ permalink raw reply
* Re: [PATCH v3 08/11] input: touchscreen: resistive-adc-touch: add generic resistive ADC touchscreen
From: Dmitry Torokhov @ 2018-04-23 23:22 UTC (permalink / raw)
To: Eugen Hristev
Cc: jic23, ludovic.desroches, alexandre.belloni, linux-arm-kernel,
devicetree, linux-kernel, linux-iio, linux-input, nicolas.ferre,
robh
In-Reply-To: <1523350677-27106-9-git-send-email-eugen.hristev@microchip.com>
Hi Eugen,
On Tue, Apr 10, 2018 at 11:57:54AM +0300, Eugen Hristev wrote:
> This adds a generic resistive touchscreen (GRTS) driver, which is based
> on an IIO device (an ADC). It must be connected to the channels of an ADC
> to receive touch data. Then it will feed the data into the input subsystem
> where it registers an input device.
> It uses an IIO callback buffer to register to the IIO device
>
> Some parts of this patch are based on initial original work by
> Mohamed Jamsheeth Hajanajubudeen and Bandaru Venkateswara Swamy
>
> Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
> ---
> Changes in v3:
> - pressure now reported naturally growing down-up
> - using helpers from touchscreen.h to parse DT properties
> - added devm_add_action_or_reset to handle callback buffer clean on exit
> - changed compatible and threshold property to adapt to changed bindings
>
> Changes in v2:
> - this is now a generic resistive adc touchscreen driver
>
> drivers/input/touchscreen/Kconfig | 13 ++
> drivers/input/touchscreen/Makefile | 1 +
> drivers/input/touchscreen/resistive-adc-touch.c | 200 ++++++++++++++++++++++++
> 3 files changed, 214 insertions(+)
> create mode 100644 drivers/input/touchscreen/resistive-adc-touch.c
>
> diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
> index 4f15496..8f85d3a 100644
> --- a/drivers/input/touchscreen/Kconfig
> +++ b/drivers/input/touchscreen/Kconfig
> @@ -92,6 +92,19 @@ config TOUCHSCREEN_AD7879_SPI
> To compile this driver as a module, choose M here: the
> module will be called ad7879-spi.
>
> +config TOUCHSCREEN_ADC
> + tristate "Generic ADC based resistive touchscreen"
> + depends on IIO
> + select IIO_BUFFER_CB
> + help
> + Say Y here if you want to use the generic ADC
> + resistive touchscreen driver.
> +
> + If unsure, say N (but it's safe to say "Y").
> +
> + To compile this driver as a module, choose M here: the
> + module will be called resistive-adc-touch.ko.
> +
> config TOUCHSCREEN_AR1021_I2C
> tristate "Microchip AR1020/1021 i2c touchscreen"
> depends on I2C && OF
> diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
> index dddae79..843c7f9 100644
> --- a/drivers/input/touchscreen/Makefile
> +++ b/drivers/input/touchscreen/Makefile
> @@ -13,6 +13,7 @@ obj-$(CONFIG_TOUCHSCREEN_AD7877) += ad7877.o
> obj-$(CONFIG_TOUCHSCREEN_AD7879) += ad7879.o
> obj-$(CONFIG_TOUCHSCREEN_AD7879_I2C) += ad7879-i2c.o
> obj-$(CONFIG_TOUCHSCREEN_AD7879_SPI) += ad7879-spi.o
> +obj-$(CONFIG_TOUCHSCREEN_ADC) += resistive-adc-touch.o
> obj-$(CONFIG_TOUCHSCREEN_ADS7846) += ads7846.o
> obj-$(CONFIG_TOUCHSCREEN_AR1021_I2C) += ar1021_i2c.o
> obj-$(CONFIG_TOUCHSCREEN_ATMEL_MXT) += atmel_mxt_ts.o
> diff --git a/drivers/input/touchscreen/resistive-adc-touch.c b/drivers/input/touchscreen/resistive-adc-touch.c
> new file mode 100644
> index 0000000..7e201ee
> --- /dev/null
> +++ b/drivers/input/touchscreen/resistive-adc-touch.c
> @@ -0,0 +1,200 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * ADC generic resistive touchscreen (GRTS)
> + *
> + * Copyright (C) 2017,2018 Microchip Technology,
> + * Author: Eugen Hristev <eugen.hristev@microchip.com>
> + *
> + */
> +#include <linux/input.h>
> +#include <linux/input/touchscreen.h>
> +#include <linux/iio/consumer.h>
> +#include <linux/iio/iio.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +
> +#define DRIVER_NAME "resistive-adc-touch"
> +#define GRTS_DEFAULT_PRESSURE_THRESHOLD 50000
> +#define MAX_POS_MASK GENMASK(11, 0)
> +
> +/**
> + * grts_state - generic resistive touch screen information struct
> + * @pressure_threshold: number representing the threshold for the pressure
> + * @pressure: are we getting pressure info or not
> + * @iio_chans: list of channels acquired
> + * @iio_cb: iio_callback buffer for the data
> + * @input: the input device structure that we register
> + * @prop: touchscreen properties struct
> + */
> +struct grts_state {
> + u32 pressure_threshold;
> + bool pressure;
> + struct iio_channel *iio_chans;
> + struct iio_cb_buffer *iio_cb;
> + struct input_dev *input;
> + struct touchscreen_properties prop;
> +};
> +
> +static int grts_cb(const void *data, void *private)
> +{
> + const u16 *touch_info = data;
> + struct grts_state *st = private;
> +
Extra blank line.
> + unsigned int x, y, press = 0xFFFF;
Why to you need to initialize "press"?
> +
> + /* channel data coming in buffer in the order below */
> + x = touch_info[0];
> + y = touch_info[1];
> + if (st->pressure)
> + press = touch_info[2];
> +
> + if ((!x && !y) || (st->pressure && (press < st->pressure_threshold))) {
> + /* report end of touch */
Hm, so this makes "press" not pressure but resistance...
> + input_report_key(st->input, BTN_TOUCH, 0);
> + input_sync(st->input);
> + return 0;
> + }
> +
> + /* report proper touch to subsystem*/
> + touchscreen_report_pos(st->input, &st->prop, x, y, false);
> + if (st->pressure)
> + input_report_abs(st->input, ABS_PRESSURE, press);
and if this is resistance, then you want to have:
input_report_abs(st->input, ABS_PRESSURE,
st->resistance_threshold - resistance);
> + input_report_key(st->input, BTN_TOUCH, 1);
> + input_sync(st->input);
> +
> + return 0;
> +}
> +
> +static int grts_open(struct input_dev *dev)
> +{
> + int error;
> + struct grts_state *st = input_get_drvdata(dev);
> +
> + error = iio_channel_start_all_cb(st->iio_cb);
> + if (error) {
> + dev_err(dev->dev.parent, "failed to start callback buffer.\n");
> + return error;
> + }
> + return 0;
> +}
> +
> +static void grts_close(struct input_dev *dev)
> +{
> + struct grts_state *st = input_get_drvdata(dev);
> +
> + iio_channel_stop_all_cb(st->iio_cb);
> +}
> +
> +static void grts_disable(void *data)
> +{
> + iio_channel_release_all_cb(data);
> +}
> +
> +static int grts_probe(struct platform_device *pdev)
> +{
> + struct grts_state *st;
> + struct input_dev *input;
> + struct device *dev = &pdev->dev;
> + struct device_node *node = dev->of_node;
> + struct iio_channel *chan;
> + int ret = 0;
Let's call this variable "error". There is also no need to initialize it
to 0.
> +
> + st = devm_kzalloc(dev, sizeof(struct grts_state), GFP_KERNEL);
> + if (!st)
> + return -ENOMEM;
> +
> + ret = of_property_read_u32(node, "touchscreen-threshold-pressure",
> + &st->pressure_threshold);
Let's use generic device property API (device_property_read_u32).
> + if (ret < 0) {
I do not think of_ or device_ property API returns positive on success.
So simply
if (error) {
...
}
> + dev_dbg(dev, "can't get touchscreen pressure threshold property.\n");
> + st->pressure_threshold = GRTS_DEFAULT_PRESSURE_THRESHOLD;
> + }
> +
> + /* get the channels from IIO device */
> + st->iio_chans = devm_iio_channel_get_all(dev);
> +
Extra blank line.
> + if (IS_ERR(st->iio_chans)) {
> + if (PTR_ERR(st->iio_chans) != -EPROBE_DEFER)
> + dev_err(dev, "can't get iio channels.\n");
> + return PTR_ERR(st->iio_chans);
> + }
> +
> + chan = &st->iio_chans[0];
> + st->pressure = false;
> + while (chan && chan->indio_dev) {
> + if (!strcmp(chan->channel->datasheet_name, "pressure"))
> + st->pressure = true;
> + chan++;
> + }
> +
> + input = devm_input_allocate_device(dev);
> + if (!input) {
> + dev_err(dev, "failed to allocate input device.\n");
> + return -ENOMEM;
> + }
> +
> + input->name = DRIVER_NAME;
> + input->id.bustype = BUS_HOST;
> + input->open = grts_open;
> + input->close = grts_close;
> +
> + input_set_abs_params(input, ABS_X, 0, MAX_POS_MASK - 1, 0, 0);
> + input_set_abs_params(input, ABS_Y, 0, MAX_POS_MASK - 1, 0, 0);
> + if (st->pressure)
> + input_set_abs_params(input, ABS_PRESSURE, 0, 0xffffff, 0, 0);
I am sure we can figure the range given the resistance threshold.
> +
> + input->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
> + input->keybit[BIT_WORD(BTN_TOUCH)] = BIT_MASK(BTN_TOUCH);
Replace this 2 with
input_set_capability(input, EV_KEY, BTN_TOUCH);
> +
> + /* parse optional device tree properties */
> + touchscreen_parse_properties(input, false, &st->prop);
> +
> + st->input = input;
> + input_set_drvdata(input, st);
> +
> + ret = input_register_device(input);
> + if (ret) {
> + dev_err(dev, "failed to register input device.");
> + return ret;
> + }
> +
> + st->iio_cb = iio_channel_get_all_cb(dev, grts_cb, st);
> +
> + if (IS_ERR(st->iio_cb)) {
> + dev_err(dev, "failed to allocate callback buffer.\n");
> + ret = PTR_ERR(st->iio_cb);
> + return ret;
> + }
> +
> + ret = devm_add_action_or_reset(dev, grts_disable, st->iio_cb);
if (error)
return error;
> +
> + platform_set_drvdata(pdev, st);
I do not think you are using platform device driver data anywhere.
> +
> + return ret;
Prefer explicit success returns, so
return 0;
> +}
> +
> +static const struct of_device_id grts_of_match[] = {
> + {
> + .compatible = "resistive-adc-touch",
> + }, {
> + /* sentinel */
> + },
> +};
> +
> +MODULE_DEVICE_TABLE(of, grts_of_match);
> +
> +static struct platform_driver grts_driver = {
> + .probe = grts_probe,
> + .driver = {
> + .name = DRIVER_NAME,
Keep indenting with tabs please.
> + .of_match_table = of_match_ptr(grts_of_match),
> + },
> +};
> +
> +module_platform_driver(grts_driver);
> +
> +MODULE_AUTHOR("Eugen Hristev <eugen.hristev@microchip.com>");
> +MODULE_DESCRIPTION("Generic ADC Resistive Touch Driver");
> +MODULE_LICENSE("GPL v2");
> --
> 2.7.4
>
--
Dmitry
^ permalink raw reply
* Re: [PATCH] MAINTAINERS: Rakesh Iyer can't be reached anymore
From: Dmitry Torokhov @ 2018-04-23 19:03 UTC (permalink / raw)
To: Jon Hunter
Cc: Wolfram Sang, linux-kernel, Laxman Dewangan, Thierry Reding,
linux-input, linux-tegra
In-Reply-To: <1f70f124-53e9-9cca-c045-ff919a577e4e@nvidia.com>
On Fri, Apr 20, 2018 at 08:03:26AM +0100, Jon Hunter wrote:
>
> On 19/04/18 18:42, Wolfram Sang wrote:
> > The current mail address is rejected, last activity (with a different
> > address) in git-history is from 2012. Remove this.
> >
> > Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
> > ---
> >
> > If somebody knows a recent address, then we can simply update, of course.
> >
> > MAINTAINERS | 1 -
> > 1 file changed, 1 deletion(-)
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index c4b6d379423e..0307426c2564 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -13833,7 +13833,6 @@ S: Supported
> > F: drivers/iommu/tegra*
> >
> > TEGRA KBC DRIVER
> > -M: Rakesh Iyer <riyer@nvidia.com>
> > M: Laxman Dewangan <ldewangan@nvidia.com>
> > S: Supported
> > F: drivers/input/keyboard/tegra-kbc.c
> >
>
> Acked-by: Jon Hunter <jonathanh@nvidia.com>
Applied, thank you.
>
> Cheers!
> Jon
--
Dmitry
^ permalink raw reply
* Re: [PATCH] Input: touchscreen: Fix a typo in Kconfig
From: Dmitry Torokhov @ 2018-04-23 19:02 UTC (permalink / raw)
To: Masanari Iida; +Cc: linux-kernel, trivial, linux-input
In-Reply-To: <20180419184514.9668-1-standby24x7@gmail.com>
On Fri, Apr 20, 2018 at 03:45:14AM +0900, Masanari Iida wrote:
> This patch fix a spelling typo found in Kconfig.
>
> Signed-off-by: Masanari Iida <standby24x7@gmail.com>
Applied, thank you.
> ---
> drivers/input/touchscreen/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
> index 4f15496fec8b..3e613afa10b4 100644
> --- a/drivers/input/touchscreen/Kconfig
> +++ b/drivers/input/touchscreen/Kconfig
> @@ -362,7 +362,7 @@ config TOUCHSCREEN_HIDEEP
>
> If unsure, say N.
>
> - To compile this driver as a moudle, choose M here : the
> + To compile this driver as a module, choose M here : the
> module will be called hideep_ts.
>
> config TOUCHSCREEN_ILI210X
> --
> 2.17.0.140.g0b0cc9f86731
>
--
Dmitry
^ permalink raw reply
* Re: [PATCH v2] Input: xen-kbdfront - allow better run-time configuration
From: Dmitry Torokhov @ 2018-04-23 18:55 UTC (permalink / raw)
To: Oleksandr Andrushchenko
Cc: Juergen Gross, xen-devel, linux-input, linux-kernel, lyan,
boris.ostrovsky, andrii_chepurnyi, Oleksandr Andrushchenko
In-Reply-To: <7b4c1f8a-6317-1f17-5950-64a1b7183dba@gmail.com>
On Mon, Apr 23, 2018 at 11:29:45AM +0300, Oleksandr Andrushchenko wrote:
> On 04/23/2018 11:23 AM, Juergen Gross wrote:
> > On 23/04/18 10:02, Oleksandr Andrushchenko wrote:
> > > Juergen, Jason, Dmitry
> > > any comment on this?
> > Oleksandr, please give us some time. I can't speak for others, but I am
> > not sitting here idling and hoping that some work (e.g. patches to
> > review) might appear.
> >
> > I have a lot of other stuff to do and will respond when I find some time
> > to look at your patches.
> >
> > Pinging others on Monday when having sent out the patch only on Thursday
> > is rather unfriendly.
> Really sorry about this, my bad.
> I had an impression that the only change we need to
> discuss was uint -> bool change which seemed rather trivial
I am sorry I did not respond to the previous version until today, but I
do not think that we should be extending module parameters for this.
Protocol features are already there for absolute pointers and
multi-touch, keyboard and relative pointers should use the same
mechanism.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] Input: xen-kbdfront - allow better run-time configuration
From: Dmitry Torokhov @ 2018-04-23 18:53 UTC (permalink / raw)
To: Oleksandr Andrushchenko
Cc: Juergen Gross, xen-devel, linux-input, linux-kernel, lyan,
boris.ostrovsky, andrii_chepurnyi, Oleksandr Andrushchenko
In-Reply-To: <2bff035e-303e-d644-5f51-5e64150c097c@gmail.com>
On Thu, Apr 19, 2018 at 02:44:19PM +0300, Oleksandr Andrushchenko wrote:
> On 04/19/2018 02:25 PM, Juergen Gross wrote:
> > On 18/04/18 17:04, Oleksandr Andrushchenko wrote:
> > > From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
> > >
> > > It is now only possible to control if multi-touch virtual device
> > > is created or not (via the corresponding XenStore entries),
> > > but keyboard and pointer devices are always created.
> > Why don't you want to go that route for keyboard and mouse, too?
> > Or does this really make no sense?
> Well, I would prefer not to touch anything outside Linux and
> this driver. And these settings seem to be implementation specific.
> So, this is why introduce Linux module parameters and don't extend
> the kbdif protocol.
Why do you consider this implementation specific? How other guests
decide to forego creation of relative pointer device or keyboard-like
device?
You already have "features" for absolute pointing device and multitouch,
so please extend the protocol properly so you indeed do not code
something implementation-specific (i.e. module parameters).
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [Xen-devel] [PATCH v2] Input: xen-kbdfront - allow better run-time configuration
From: Jason Andryuk @ 2018-04-23 18:40 UTC (permalink / raw)
To: Oleksandr Andrushchenko
Cc: xen-devel, linux-input, open list, Dmitry Torokhov, Juergen Gross,
lyan, Boris Ostrovsky, andrii_chepurnyi, Oleksandr Andrushchenko
In-Reply-To: <20180419133934.31306-1-andr2000@gmail.com>
Hi, Oleksandr.
On Thu, Apr 19, 2018 at 9:39 AM, Oleksandr Andrushchenko
<andr2000@gmail.com> wrote:
<snip>
> @@ -241,60 +242,84 @@ static int xenkbd_probe(struct xenbus_device *dev,
> }
>
> /* keyboard */
> - kbd = input_allocate_device();
> - if (!kbd)
> - goto error_nomem;
> - kbd->name = "Xen Virtual Keyboard";
> - kbd->phys = info->phys;
> - kbd->id.bustype = BUS_PCI;
> - kbd->id.vendor = 0x5853;
> - kbd->id.product = 0xffff;
> -
> - __set_bit(EV_KEY, kbd->evbit);
> - for (i = KEY_ESC; i < KEY_UNKNOWN; i++)
> - __set_bit(i, kbd->keybit);
> - for (i = KEY_OK; i < KEY_MAX; i++)
> - __set_bit(i, kbd->keybit);
> -
> - ret = input_register_device(kbd);
> - if (ret) {
> - input_free_device(kbd);
> - xenbus_dev_fatal(dev, ret, "input_register_device(kbd)");
> - goto error;
> + if (!no_kbd_dev) {
My earlier suggestion on the option name was aimed at replacing the above with:
if (kbd_dev) {
I find it easier to read then the double negative !no_kbd_dev. But
it's only used once, so it's not a big deal either way.
<snip>
>
> - __set_bit(EV_KEY, ptr->evbit);
> - for (i = BTN_LEFT; i <= BTN_TASK; i++)
> - __set_bit(i, ptr->keybit);
> + ptr = input_allocate_device();
> + if (!ptr)
> + goto error_nomem;
> + ptr->name = "Xen Virtual Pointer";
> + ptr->phys = info->phys;
> + ptr->id.bustype = BUS_PCI;
> + ptr->id.vendor = 0x5853;
> + ptr->id.product = 0xfffe;
> +
> + if (abs) {
> + __set_bit(EV_ABS, ptr->evbit);
> + input_set_abs_params(ptr, ABS_X, 0, ptr_size[KPARAM_X], 0, 0);
> + input_set_abs_params(ptr, ABS_Y, 0, ptr_size[KPARAM_Y], 0, 0);
Just noticed these lines now exceed 80 columns.
Otherwise it's just code motion and fine by me.
Regards,
Jason
^ permalink raw reply
* Re: WARNING: HARDIRQ-safe -> HARDIRQ-unsafe lock order detected
From: Dmitry Torokhov @ 2018-04-23 17:49 UTC (permalink / raw)
To: syzbot, Theodore Ts'o
Cc: linux-input@vger.kernel.org, lkml, Henrik Rydberg, syzkaller-bugs
In-Reply-To: <0000000000008bce26056a7a6c23@google.com>
On Sun, Apr 22, 2018 at 7:02 PM, syzbot
<syzbot+e1670f554caa60fb147b@syzkaller.appspotmail.com> wrote:
>
> Hello,
>
> syzbot hit the following crash on upstream commit
> 285848b0f4074f04ab606f1e5dca296482033d54 (Sun Apr 22 04:20:48 2018 +0000)
> Merge tag 'random_for_linus_stable' of git://git.kernel.org/pub/scm/linux/kernel/git/tytso/random
> syzbot dashboard link: https://syzkaller.appspot.com/bug?extid=e1670f554caa60fb147b
Ted,
input_add_randomness() (that ends up calling crng_reseed() and the new
numa_crng_init()) is called (and has been called ever since inception)
from an interrupt context and thus may not sleep. The following commit
breaks this:
ommit 8ef35c866f8862df074a49a93b0309725812dea8
Author: Theodore Ts'o <tytso@mit.edu>
Date: Wed Apr 11 15:23:56 2018 -0400
random: set up the NUMA crng instances after the CRNG is fully initialized
Until the primary_crng is fully initialized, don't initialize the NUMA
crng nodes. Otherwise users of /dev/urandom on NUMA systems before
the CRNG is fully initialized can get very bad quality randomness. Of
course everyone should move to getrandom(2) where this won't be an
issue, but there's a lot of legacy code out there. This related to
CVE-2018-1108.
Reported-by: Jann Horn <jannh@google.com>
Fixes: 1e7f583af67b ("random: make /dev/urandom scalable for silly...")
Cc: stable@kernel.org # 4.8+
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Thanks!
>
>
>
> So far this crash happened 398 times on upstream.
> C reproducer: https://syzkaller.appspot.com/x/repro.c?id=6457007586410496
> syzkaller reproducer: https://syzkaller.appspot.com/x/repro.syz?id=5576436211515392
> Raw console output: https://syzkaller.appspot.com/x/log.txt?id=6327380104708096
> Kernel config: https://syzkaller.appspot.com/x/.config?id=1808800213120130118
> compiler: gcc (GCC) 8.0.1 20180413 (experimental)
>
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+e1670f554caa60fb147b@syzkaller.appspotmail.com
> It will help syzbot understand when the bug is fixed. See footer for details.
> If you forward the report, please keep this part and the footer.
>
>
> =====================================================
> WARNING: HARDIRQ-safe -> HARDIRQ-unsafe lock order detected
> 4.17.0-rc1+ #12 Not tainted
> -----------------------------------------------------
> syzkaller880831/4534 [HC0[0]:SC0[0]:HE0:SE1] is trying to acquire:
> (ptrval) (fs_reclaim){+.+.}, at: fs_reclaim_acquire.part.82+0x0/0x30 mm/page_alloc.c:463
>
> and this task is already holding:
> (ptrval) (&(&dev->event_lock)->rlock){-.-.}, at: input_inject_event+0xe0/0x3ed drivers/input/input.c:461
> which would create a new lock dependency:
> (&(&dev->event_lock)->rlock){-.-.} -> (fs_reclaim){+.+.}
>
> but this new dependency connects a HARDIRQ-irq-safe lock:
> (&(&dev->event_lock)->rlock){-.-.}
>
> ... which became HARDIRQ-irq-safe at:
> lock_acquire+0x1dc/0x520 kernel/locking/lockdep.c:3920
> __raw_spin_lock_irqsave include/linux/spinlock_api_smp.h:110 [inline]
> _raw_spin_lock_irqsave+0x96/0xc0 kernel/locking/spinlock.c:152
> input_event+0x67/0xa0 drivers/input/input.c:435
> input_report_key include/linux/input.h:393 [inline]
> psmouse_report_standard_buttons+0x31/0x90 drivers/input/mouse/psmouse-base.c:127
> psmouse_report_standard_packet drivers/input/mouse/psmouse-base.c:145 [inline]
> psmouse_process_byte+0x1ef/0x710 drivers/input/mouse/psmouse-base.c:236
> psmouse_handle_byte+0x4a/0x570 drivers/input/mouse/psmouse-base.c:278
> psmouse_interrupt+0x38a/0x1420 drivers/input/mouse/psmouse-base.c:428
> serio_interrupt+0x98/0x160 drivers/input/serio/serio.c:1018
> i8042_interrupt+0x385/0x5e0 drivers/input/serio/i8042.c:586
> __handle_irq_event_percpu+0x1c0/0xad0 kernel/irq/handle.c:149
> handle_irq_event_percpu+0x98/0x1c0 kernel/irq/handle.c:189
> handle_irq_event+0xa7/0x135 kernel/irq/handle.c:206
> handle_edge_irq+0x20f/0x870 kernel/irq/chip.c:791
> generic_handle_irq_desc include/linux/irqdesc.h:159 [inline]
> handle_irq+0x18c/0x2e7 arch/x86/kernel/irq_64.c:77
> do_IRQ+0x78/0x190 arch/x86/kernel/irq.c:245
> ret_from_intr+0x0/0x1e
> arch_local_irq_enable arch/x86/include/asm/paravirt.h:793 [inline]
> __do_softirq+0x298/0xaf5 kernel/softirq.c:269
> invoke_softirq kernel/softirq.c:365 [inline]
> irq_exit+0x1d1/0x200 kernel/softirq.c:405
> exiting_irq arch/x86/include/asm/apic.h:525 [inline]
> smp_apic_timer_interrupt+0x17e/0x710 arch/x86/kernel/apic/apic.c:1052
> apic_timer_interrupt+0xf/0x20 arch/x86/entry/entry_64.S:863
> arch_local_irq_restore arch/x86/include/asm/paravirt.h:783 [inline]
> lock_release+0x4d4/0xa10 kernel/locking/lockdep.c:3942
> fs_reclaim_release.part.83+0x1c/0x20 mm/page_alloc.c:3746
> fs_reclaim_release+0x14/0x20 mm/page_alloc.c:3747
> slab_pre_alloc_hook mm/slab.h:419 [inline]
> slab_alloc mm/slab.c:3378 [inline]
> kmem_cache_alloc+0x30/0x760 mm/slab.c:3552
> kmem_cache_zalloc include/linux/slab.h:691 [inline]
> __kernfs_new_node+0xe7/0x580 fs/kernfs/dir.c:633
> kernfs_new_node+0x80/0xf0 fs/kernfs/dir.c:679
> __kernfs_create_file+0x4d/0x330 fs/kernfs/file.c:989
> sysfs_add_file_mode_ns+0x21a/0x560 fs/sysfs/file.c:305
> create_files fs/sysfs/group.c:62 [inline]
> internal_create_group+0x282/0x970 fs/sysfs/group.c:132
> sysfs_create_group fs/sysfs/group.c:154 [inline]
> sysfs_create_groups+0x9b/0x150 fs/sysfs/group.c:181
> device_add_groups drivers/base/core.c:1033 [inline]
> device_add_attrs drivers/base/core.c:1181 [inline]
> device_add+0x84d/0x16d0 drivers/base/core.c:1813
> netdev_register_kobject+0x180/0x380 net/core/net-sysfs.c:1604
> register_netdevice+0x997/0x11c0 net/core/dev.c:7961
> register_netdev+0x30/0x50 net/core/dev.c:8076
> sit_init_net+0x445/0xc50 net/ipv6/sit.c:1857
> ops_init+0xff/0x550 net/core/net_namespace.c:128
> __register_pernet_operations net/core/net_namespace.c:912 [inline]
> register_pernet_operations+0x49a/0x9f0 net/core/net_namespace.c:987
> register_pernet_device+0x2a/0x80 net/core/net_namespace.c:1074
> sit_init+0x22/0x175 net/ipv6/sit.c:1914
> do_one_initcall+0x127/0x913 init/main.c:883
> do_initcall_level init/main.c:951 [inline]
> do_initcalls init/main.c:959 [inline]
> do_basic_setup init/main.c:977 [inline]
> kernel_init_freeable+0x49b/0x58e init/main.c:1127
> kernel_init+0x11/0x1b3 init/main.c:1053
> ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:412
>
> to a HARDIRQ-irq-unsafe lock:
> (fs_reclaim){+.+.}
>
> ... which became HARDIRQ-irq-unsafe at:
> ...
> lock_acquire+0x1dc/0x520 kernel/locking/lockdep.c:3920
> fs_reclaim_acquire.part.82+0x24/0x30 mm/page_alloc.c:3739
> fs_reclaim_acquire+0x14/0x20 mm/page_alloc.c:3740
> slab_pre_alloc_hook mm/slab.h:418 [inline]
> slab_alloc_node mm/slab.c:3299 [inline]
> kmem_cache_alloc_node_trace+0x39/0x770 mm/slab.c:3661
> kmalloc_node include/linux/slab.h:550 [inline]
> kzalloc_node include/linux/slab.h:712 [inline]
> alloc_worker+0xbd/0x2e0 kernel/workqueue.c:1704
> init_rescuer.part.25+0x1f/0x190 kernel/workqueue.c:4000
> init_rescuer kernel/workqueue.c:3997 [inline]
> workqueue_init+0x51f/0x7d0 kernel/workqueue.c:5732
> kernel_init_freeable+0x2ad/0x58e init/main.c:1115
> kernel_init+0x11/0x1b3 init/main.c:1053
> ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:412
>
> other info that might help us debug this:
>
> Possible interrupt unsafe locking scenario:
>
> CPU0 CPU1
> ---- ----
> lock(fs_reclaim);
> local_irq_disable();
> lock(&(&dev->event_lock)->rlock);
> lock(fs_reclaim);
> <Interrupt>
> lock(&(&dev->event_lock)->rlock);
>
> *** DEADLOCK ***
>
> 3 locks held by syzkaller880831/4534:
> #0: (ptrval) (&evdev->mutex){+.+.}, at: evdev_write+0x1cc/0x860 drivers/input/evdev.c:543
> #1: (ptrval) (&(&dev->event_lock)->rlock){-.-.}, at: input_inject_event+0xe0/0x3ed drivers/input/input.c:461
> #2: (ptrval) (rcu_read_lock){....}, at: is_event_supported drivers/input/input.c:56 [inline]
> #2: (ptrval) (rcu_read_lock){....}, at: input_inject_event+0xc5/0x3ed drivers/input/input.c:460
>
> the dependencies between HARDIRQ-irq-safe lock and the holding lock:
> -> (&(&dev->event_lock)->rlock){-.-.} ops: 1797 {
> IN-HARDIRQ-W at:
> lock_acquire+0x1dc/0x520 kernel/locking/lockdep.c:3920
> __raw_spin_lock_irqsave include/linux/spinlock_api_smp.h:110 [inline]
> _raw_spin_lock_irqsave+0x96/0xc0 kernel/locking/spinlock.c:152
> input_event+0x67/0xa0 drivers/input/input.c:435
> input_report_key include/linux/input.h:393 [inline]
> psmouse_report_standard_buttons+0x31/0x90 drivers/input/mouse/psmouse-base.c:127
> psmouse_report_standard_packet drivers/input/mouse/psmouse-base.c:145 [inline]
> psmouse_process_byte+0x1ef/0x710 drivers/input/mouse/psmouse-base.c:236
> psmouse_handle_byte+0x4a/0x570 drivers/input/mouse/psmouse-base.c:278
> psmouse_interrupt+0x38a/0x1420 drivers/input/mouse/psmouse-base.c:428
> serio_interrupt+0x98/0x160 drivers/input/serio/serio.c:1018
> i8042_interrupt+0x385/0x5e0 drivers/input/serio/i8042.c:586
> __handle_irq_event_percpu+0x1c0/0xad0 kernel/irq/handle.c:149
> handle_irq_event_percpu+0x98/0x1c0 kernel/irq/handle.c:189
> handle_irq_event+0xa7/0x135 kernel/irq/handle.c:206
> handle_edge_irq+0x20f/0x870 kernel/irq/chip.c:791
> generic_handle_irq_desc include/linux/irqdesc.h:159 [inline]
> handle_irq+0x18c/0x2e7 arch/x86/kernel/irq_64.c:77
> do_IRQ+0x78/0x190 arch/x86/kernel/irq.c:245
> ret_from_intr+0x0/0x1e
> arch_local_irq_enable arch/x86/include/asm/paravirt.h:793 [inline]
> __do_softirq+0x298/0xaf5 kernel/softirq.c:269
> invoke_softirq kernel/softirq.c:365 [inline]
> irq_exit+0x1d1/0x200 kernel/softirq.c:405
> exiting_irq arch/x86/include/asm/apic.h:525 [inline]
> smp_apic_timer_interrupt+0x17e/0x710 arch/x86/kernel/apic/apic.c:1052
> apic_timer_interrupt+0xf/0x20 arch/x86/entry/entry_64.S:863
> arch_local_irq_restore arch/x86/include/asm/paravirt.h:783 [inline]
> lock_release+0x4d4/0xa10 kernel/locking/lockdep.c:3942
> fs_reclaim_release.part.83+0x1c/0x20 mm/page_alloc.c:3746
> fs_reclaim_release+0x14/0x20 mm/page_alloc.c:3747
> slab_pre_alloc_hook mm/slab.h:419 [inline]
> slab_alloc mm/slab.c:3378 [inline]
> kmem_cache_alloc+0x30/0x760 mm/slab.c:3552
> kmem_cache_zalloc include/linux/slab.h:691 [inline]
> __kernfs_new_node+0xe7/0x580 fs/kernfs/dir.c:633
> kernfs_new_node+0x80/0xf0 fs/kernfs/dir.c:679
> __kernfs_create_file+0x4d/0x330 fs/kernfs/file.c:989
> sysfs_add_file_mode_ns+0x21a/0x560 fs/sysfs/file.c:305
> create_files fs/sysfs/group.c:62 [inline]
> internal_create_group+0x282/0x970 fs/sysfs/group.c:132
> sysfs_create_group fs/sysfs/group.c:154 [inline]
> sysfs_create_groups+0x9b/0x150 fs/sysfs/group.c:181
> device_add_groups drivers/base/core.c:1033 [inline]
> device_add_attrs drivers/base/core.c:1181 [inline]
> device_add+0x84d/0x16d0 drivers/base/core.c:1813
> netdev_register_kobject+0x180/0x380 net/core/net-sysfs.c:1604
> register_netdevice+0x997/0x11c0 net/core/dev.c:7961
> register_netdev+0x30/0x50 net/core/dev.c:8076
> sit_init_net+0x445/0xc50 net/ipv6/sit.c:1857
> ops_init+0xff/0x550 net/core/net_namespace.c:128
> __register_pernet_operations net/core/net_namespace.c:912 [inline]
> register_pernet_operations+0x49a/0x9f0 net/core/net_namespace.c:987
> register_pernet_device+0x2a/0x80 net/core/net_namespace.c:1074
> sit_init+0x22/0x175 net/ipv6/sit.c:1914
> do_one_initcall+0x127/0x913 init/main.c:883
> do_initcall_level init/main.c:951 [inline]
> do_initcalls init/main.c:959 [inline]
> do_basic_setup init/main.c:977 [inline]
> kernel_init_freeable+0x49b/0x58e init/main.c:1127
> kernel_init+0x11/0x1b3 init/main.c:1053
> ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:412
> IN-SOFTIRQ-W at:
> lock_acquire+0x1dc/0x520 kernel/locking/lockdep.c:3920
> __raw_spin_lock_irqsave include/linux/spinlock_api_smp.h:110 [inline]
> _raw_spin_lock_irqsave+0x96/0xc0 kernel/locking/spinlock.c:152
> input_event+0x67/0xa0 drivers/input/input.c:435
> input_report_key include/linux/input.h:393 [inline]
> psmouse_report_standard_buttons+0x31/0x90 drivers/input/mouse/psmouse-base.c:127
> psmouse_report_standard_packet drivers/input/mouse/psmouse-base.c:145 [inline]
> psmouse_process_byte+0x1ef/0x710 drivers/input/mouse/psmouse-base.c:236
> psmouse_handle_byte+0x4a/0x570 drivers/input/mouse/psmouse-base.c:278
> psmouse_interrupt+0x38a/0x1420 drivers/input/mouse/psmouse-base.c:428
> serio_interrupt+0x98/0x160 drivers/input/serio/serio.c:1018
> i8042_interrupt+0x385/0x5e0 drivers/input/serio/i8042.c:586
> __handle_irq_event_percpu+0x1c0/0xad0 kernel/irq/handle.c:149
> handle_irq_event_percpu+0x98/0x1c0 kernel/irq/handle.c:189
> handle_irq_event+0xa7/0x135 kernel/irq/handle.c:206
> handle_edge_irq+0x20f/0x870 kernel/irq/chip.c:791
> generic_handle_irq_desc include/linux/irqdesc.h:159 [inline]
> handle_irq+0x18c/0x2e7 arch/x86/kernel/irq_64.c:77
> do_IRQ+0x78/0x190 arch/x86/kernel/irq.c:245
> ret_from_intr+0x0/0x1e
> arch_local_irq_enable arch/x86/include/asm/paravirt.h:793 [inline]
> __do_softirq+0x298/0xaf5 kernel/softirq.c:269
> invoke_softirq kernel/softirq.c:365 [inline]
> irq_exit+0x1d1/0x200 kernel/softirq.c:405
> exiting_irq arch/x86/include/asm/apic.h:525 [inline]
> smp_apic_timer_interrupt+0x17e/0x710 arch/x86/kernel/apic/apic.c:1052
> apic_timer_interrupt+0xf/0x20 arch/x86/entry/entry_64.S:863
> arch_local_irq_restore arch/x86/include/asm/paravirt.h:783 [inline]
> lock_release+0x4d4/0xa10 kernel/locking/lockdep.c:3942
> fs_reclaim_release.part.83+0x1c/0x20 mm/page_alloc.c:3746
> fs_reclaim_release+0x14/0x20 mm/page_alloc.c:3747
> slab_pre_alloc_hook mm/slab.h:419 [inline]
> slab_alloc mm/slab.c:3378 [inline]
> kmem_cache_alloc+0x30/0x760 mm/slab.c:3552
> kmem_cache_zalloc include/linux/slab.h:691 [inline]
> __kernfs_new_node+0xe7/0x580 fs/kernfs/dir.c:633
> kernfs_new_node+0x80/0xf0 fs/kernfs/dir.c:679
> __kernfs_create_file+0x4d/0x330 fs/kernfs/file.c:989
> sysfs_add_file_mode_ns+0x21a/0x560 fs/sysfs/file.c:305
> create_files fs/sysfs/group.c:62 [inline]
> internal_create_group+0x282/0x970 fs/sysfs/group.c:132
> sysfs_create_group fs/sysfs/group.c:154 [inline]
> sysfs_create_groups+0x9b/0x150 fs/sysfs/group.c:181
> device_add_groups drivers/base/core.c:1033 [inline]
> device_add_attrs drivers/base/core.c:1181 [inline]
> device_add+0x84d/0x16d0 drivers/base/core.c:1813
> netdev_register_kobject+0x180/0x380 net/core/net-sysfs.c:1604
> register_netdevice+0x997/0x11c0 net/core/dev.c:7961
> register_netdev+0x30/0x50 net/core/dev.c:8076
> sit_init_net+0x445/0xc50 net/ipv6/sit.c:1857
> ops_init+0xff/0x550 net/core/net_namespace.c:128
> __register_pernet_operations net/core/net_namespace.c:912 [inline]
> register_pernet_operations+0x49a/0x9f0 net/core/net_namespace.c:987
> register_pernet_device+0x2a/0x80 net/core/net_namespace.c:1074
> sit_init+0x22/0x175 net/ipv6/sit.c:1914
> do_one_initcall+0x127/0x913 init/main.c:883
> do_initcall_level init/main.c:951 [inline]
> do_initcalls init/main.c:959 [inline]
> do_basic_setup init/main.c:977 [inline]
> kernel_init_freeable+0x49b/0x58e init/main.c:1127
> kernel_init+0x11/0x1b3 init/main.c:1053
> ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:412
> INITIAL USE at:
> lock_acquire+0x1dc/0x520 kernel/locking/lockdep.c:3920
> __raw_spin_lock_irqsave include/linux/spinlock_api_smp.h:110 [inline]
> _raw_spin_lock_irqsave+0x96/0xc0 kernel/locking/spinlock.c:152
> input_inject_event+0xe0/0x3ed drivers/input/input.c:461
> input_leds_brightness_set+0x81/0xb0 drivers/input/input-leds.c:66
> __led_set_brightness drivers/leds/led-core.c:34 [inline]
> led_set_brightness_nopm+0x4c/0xe0 drivers/leds/led-core.c:261
> led_set_brightness_nosleep drivers/leds/led-core.c:278 [inline]
> led_set_brightness+0x113/0x220 drivers/leds/led-core.c:253
> led_trigger_event+0x77/0xd0 drivers/leds/led-triggers.c:292
> kbd_led_trigger_activate+0xed/0x120 drivers/tty/vt/keyboard.c:969
> led_trigger_set+0x668/0x930 drivers/leds/led-triggers.c:138
> led_trigger_set_default+0x10a/0x180 drivers/leds/led-triggers.c:171
> of_led_classdev_register+0x485/0x640 drivers/leds/led-class.c:302
> input_leds_connect+0x410/0x7c0 drivers/input/input-leds.c:143
> input_attach_handler+0x1b1/0x210 drivers/input/input.c:996
> input_register_device.cold.22+0xe8/0x297 drivers/input/input.c:2152
> atkbd_connect+0x6fe/0x930 drivers/input/keyboard/atkbd.c:1200
> serio_connect_driver+0x4f/0x70 drivers/input/serio/serio.c:63
> serio_driver_probe+0x47/0x60 drivers/input/serio/serio.c:794
> really_probe drivers/base/dd.c:448 [inline]
> driver_probe_device+0x69b/0x960 drivers/base/dd.c:590
> __driver_attach+0x1b2/0x1f0 drivers/base/dd.c:824
> bus_for_each_dev+0x151/0x1d0 drivers/base/bus.c:311
> driver_attach+0x3d/0x50 drivers/base/dd.c:843
> serio_attach_driver drivers/input/serio/serio.c:824 [inline]
> serio_handle_event+0x70a/0xb20 drivers/input/serio/serio.c:243
> process_one_work+0xc1e/0x1b50 kernel/workqueue.c:2145
> worker_thread+0x1cc/0x1440 kernel/workqueue.c:2279
> kthread+0x345/0x410 kernel/kthread.c:238
> ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:412
> }
> ... key at: [<ffffffff8b147da0>] __key.33448+0x0/0x40
> ... acquired at:
> lock_acquire+0x1dc/0x520 kernel/locking/lockdep.c:3920
> fs_reclaim_acquire.part.82+0x24/0x30 mm/page_alloc.c:3739
> fs_reclaim_acquire+0x14/0x20 mm/page_alloc.c:3740
> slab_pre_alloc_hook mm/slab.h:418 [inline]
> slab_alloc mm/slab.c:3378 [inline]
> __do_kmalloc mm/slab.c:3716 [inline]
> __kmalloc+0x45/0x760 mm/slab.c:3727
> kmalloc_array include/linux/slab.h:631 [inline]
> kcalloc include/linux/slab.h:642 [inline]
> numa_crng_init drivers/char/random.c:798 [inline]
> crng_reseed+0x427/0x920 drivers/char/random.c:923
> credit_entropy_bits+0x98d/0xa30 drivers/char/random.c:708
> add_timer_randomness+0x26b/0x320 drivers/char/random.c:1133
> add_input_randomness+0xce/0x3e0 drivers/char/random.c:1148
> input_handle_event+0xb3/0x1210 drivers/input/input.c:375
> input_inject_event+0x367/0x3ed drivers/input/input.c:466
> evdev_write+0x4d1/0x860 drivers/input/evdev.c:560
> __vfs_write+0x10b/0x960 fs/read_write.c:485
> vfs_write+0x1f8/0x560 fs/read_write.c:549
> ksys_write+0xf9/0x250 fs/read_write.c:598
> __do_sys_write fs/read_write.c:610 [inline]
> __se_sys_write fs/read_write.c:607 [inline]
> __x64_sys_write+0x73/0xb0 fs/read_write.c:607
> do_syscall_64+0x1b1/0x800 arch/x86/entry/common.c:287
> entry_SYSCALL_64_after_hwframe+0x49/0xbe
>
>
> the dependencies between the lock to be acquired
> and HARDIRQ-irq-unsafe lock:
> -> (fs_reclaim){+.+.} ops: 1058989 {
> HARDIRQ-ON-W at:
> lock_acquire+0x1dc/0x520 kernel/locking/lockdep.c:3920
> fs_reclaim_acquire.part.82+0x24/0x30 mm/page_alloc.c:3739
> fs_reclaim_acquire+0x14/0x20 mm/page_alloc.c:3740
> slab_pre_alloc_hook mm/slab.h:418 [inline]
> slab_alloc_node mm/slab.c:3299 [inline]
> kmem_cache_alloc_node_trace+0x39/0x770 mm/slab.c:3661
> kmalloc_node include/linux/slab.h:550 [inline]
> kzalloc_node include/linux/slab.h:712 [inline]
> alloc_worker+0xbd/0x2e0 kernel/workqueue.c:1704
> init_rescuer.part.25+0x1f/0x190 kernel/workqueue.c:4000
> init_rescuer kernel/workqueue.c:3997 [inline]
> workqueue_init+0x51f/0x7d0 kernel/workqueue.c:5732
> kernel_init_freeable+0x2ad/0x58e init/main.c:1115
> kernel_init+0x11/0x1b3 init/main.c:1053
> ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:412
> SOFTIRQ-ON-W at:
> lock_acquire+0x1dc/0x520 kernel/locking/lockdep.c:3920
> fs_reclaim_acquire.part.82+0x24/0x30 mm/page_alloc.c:3739
> fs_reclaim_acquire+0x14/0x20 mm/page_alloc.c:3740
> slab_pre_alloc_hook mm/slab.h:418 [inline]
> slab_alloc_node mm/slab.c:3299 [inline]
> kmem_cache_alloc_node_trace+0x39/0x770 mm/slab.c:3661
> kmalloc_node include/linux/slab.h:550 [inline]
> kzalloc_node include/linux/slab.h:712 [inline]
> alloc_worker+0xbd/0x2e0 kernel/workqueue.c:1704
> init_rescuer.part.25+0x1f/0x190 kernel/workqueue.c:4000
> init_rescuer kernel/workqueue.c:3997 [inline]
> workqueue_init+0x51f/0x7d0 kernel/workqueue.c:5732
> kernel_init_freeable+0x2ad/0x58e init/main.c:1115
> kernel_init+0x11/0x1b3 init/main.c:1053
> ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:412
> INITIAL USE at:
> lock_acquire+0x1dc/0x520 kernel/locking/lockdep.c:3920
> fs_reclaim_acquire.part.82+0x24/0x30 mm/page_alloc.c:3739
> fs_reclaim_acquire+0x14/0x20 mm/page_alloc.c:3740
> slab_pre_alloc_hook mm/slab.h:418 [inline]
> slab_alloc_node mm/slab.c:3299 [inline]
> kmem_cache_alloc_node_trace+0x39/0x770 mm/slab.c:3661
> kmalloc_node include/linux/slab.h:550 [inline]
> kzalloc_node include/linux/slab.h:712 [inline]
> alloc_worker+0xbd/0x2e0 kernel/workqueue.c:1704
> init_rescuer.part.25+0x1f/0x190 kernel/workqueue.c:4000
> init_rescuer kernel/workqueue.c:3997 [inline]
> workqueue_init+0x51f/0x7d0 kernel/workqueue.c:5732
> kernel_init_freeable+0x2ad/0x58e init/main.c:1115
> kernel_init+0x11/0x1b3 init/main.c:1053
> ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:412
> }
> ... key at: [<ffffffff88df4620>] __fs_reclaim_map+0x0/0x40
> ... acquired at:
> lock_acquire+0x1dc/0x520 kernel/locking/lockdep.c:3920
> fs_reclaim_acquire.part.82+0x24/0x30 mm/page_alloc.c:3739
> fs_reclaim_acquire+0x14/0x20 mm/page_alloc.c:3740
> slab_pre_alloc_hook mm/slab.h:418 [inline]
> slab_alloc mm/slab.c:3378 [inline]
> __do_kmalloc mm/slab.c:3716 [inline]
> __kmalloc+0x45/0x760 mm/slab.c:3727
> kmalloc_array include/linux/slab.h:631 [inline]
> kcalloc include/linux/slab.h:642 [inline]
> numa_crng_init drivers/char/random.c:798 [inline]
> crng_reseed+0x427/0x920 drivers/char/random.c:923
> credit_entropy_bits+0x98d/0xa30 drivers/char/random.c:708
> add_timer_randomness+0x26b/0x320 drivers/char/random.c:1133
> add_input_randomness+0xce/0x3e0 drivers/char/random.c:1148
> input_handle_event+0xb3/0x1210 drivers/input/input.c:375
> input_inject_event+0x367/0x3ed drivers/input/input.c:466
> evdev_write+0x4d1/0x860 drivers/input/evdev.c:560
> __vfs_write+0x10b/0x960 fs/read_write.c:485
> vfs_write+0x1f8/0x560 fs/read_write.c:549
> ksys_write+0xf9/0x250 fs/read_write.c:598
> __do_sys_write fs/read_write.c:610 [inline]
> __se_sys_write fs/read_write.c:607 [inline]
> __x64_sys_write+0x73/0xb0 fs/read_write.c:607
> do_syscall_64+0x1b1/0x800 arch/x86/entry/common.c:287
> entry_SYSCALL_64_after_hwframe+0x49/0xbe
>
>
> stack backtrace:
> CPU: 0 PID: 4534 Comm: syzkaller880831 Not tainted 4.17.0-rc1+ #12
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> Call Trace:
> __dump_stack lib/dump_stack.c:77 [inline]
> dump_stack+0x1b9/0x294 lib/dump_stack.c:113
> print_bad_irq_dependency kernel/locking/lockdep.c:1570 [inline]
> check_usage.cold.58+0x6d5/0xac7 kernel/locking/lockdep.c:1602
> check_irq_usage kernel/locking/lockdep.c:1658 [inline]
> check_prev_add_irq kernel/locking/lockdep_states.h:7 [inline]
> check_prev_add kernel/locking/lockdep.c:1868 [inline]
> check_prevs_add kernel/locking/lockdep.c:1976 [inline]
> validate_chain kernel/locking/lockdep.c:2417 [inline]
> __lock_acquire+0x2417/0x5140 kernel/locking/lockdep.c:3431
> lock_acquire+0x1dc/0x520 kernel/locking/lockdep.c:3920
> fs_reclaim_acquire.part.82+0x24/0x30 mm/page_alloc.c:3739
> fs_reclaim_acquire+0x14/0x20 mm/page_alloc.c:3740
> slab_pre_alloc_hook mm/slab.h:418 [inline]
> slab_alloc mm/slab.c:3378 [inline]
> __do_kmalloc mm/slab.c:3716 [inline]
> __kmalloc+0x45/0x760 mm/slab.c:3727
> kmalloc_array include/linux/slab.h:631 [inline]
> kcalloc include/linux/slab.h:642 [inline]
> numa_crng_init drivers/char/random.c:798 [inline]
> crng_reseed+0x427/0x920 drivers/char/random.c:923
> credit_entropy_bits+0x98d/0xa30 drivers/char/random.c:708
> add_timer_randomness+0x26b/0x320 drivers/char/random.c:1133
> add_input_randomness+0xce/0x3e0 drivers/char/random.c:1148
> input_handle_event+0xb3/0x1210 drivers/input/input.c:375
> input_inject_event+0x367/0x3ed drivers/input/input.c:466
> evdev_write+0x4d1/0x860 drivers/input/evdev.c:560
> __vfs_write+0x10b/0x960 fs/read_write.c:485
> vfs_write+0x1f8/0x560 fs/read_write.c:549
> ksys_write+0xf9/0x250 fs/read_write.c:598
> __do_sys_write fs/read_write.c:610 [inline]
> __se_sys_write fs/read_write.c:607 [inline]
> __x64_sys_write+0x73/0xb0 fs/read_write.c:607
> do_syscall_64+0x1b1/0x800 arch/x86/entry/common.c:287
> entry_SYSCALL_64_after_hwframe+0x49/0xbe
> RIP: 0033:0x443db9
> RSP: 002b:00007ffd62c88e88 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
> RAX: ffffffffffffffda RBX: 0008000040000002 RCX: 0000000000443db9
> RDX: 0000000000000030 RSI: 00000000200000c0 RDI: 00000000000003ff
> RBP: 746e6576652f7475 R08: 00000000004002e0 R09: 00000000004002e0
> R10: 0000000000000000 R11: 0000000000000246 R12: 706e692f7665642f
> R13: 0000000000401af0 R14: 0000000000000000 R15: 0000000000000000
> BUG: sleeping function called from invalid context at mm/slab.h:421
> in_atomic(): 1, irqs_disabled(): 1, pid: 4534, name: syzkaller880831
> INFO: lockdep is turned off.
> irq event stamp: 74430
> hardirqs last enabled at (74429): [<ffffffff8100c172>] do_syscall_64+0x92/0x800 arch/x86/entry/common.c:274
> hardirqs last disabled at (74430): [<ffffffff876eada4>] __raw_spin_lock_irqsave include/linux/spinlock_api_smp.h:108 [inline]
> hardirqs last disabled at (74430): [<ffffffff876eada4>] _raw_spin_lock_irqsave+0x74/0xc0 kernel/locking/spinlock.c:152
> softirqs last enabled at (74408): [<ffffffff87a00778>] __do_softirq+0x778/0xaf5 kernel/softirq.c:311
> softirqs last disabled at (74401): [<ffffffff81475041>] invoke_softirq kernel/softirq.c:365 [inline]
> softirqs last disabled at (74401): [<ffffffff81475041>] irq_exit+0x1d1/0x200 kernel/softirq.c:405
> CPU: 0 PID: 4534 Comm: syzkaller880831 Not tainted 4.17.0-rc1+ #12
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> Call Trace:
> __dump_stack lib/dump_stack.c:77 [inline]
> dump_stack+0x1b9/0x294 lib/dump_stack.c:113
> ___might_sleep.cold.87+0x11f/0x13a kernel/sched/core.c:6188
> __might_sleep+0x95/0x190 kernel/sched/core.c:6141
> slab_pre_alloc_hook mm/slab.h:421 [inline]
> slab_alloc mm/slab.c:3378 [inline]
> __do_kmalloc mm/slab.c:3716 [inline]
> __kmalloc+0x2b9/0x760 mm/slab.c:3727
> kmalloc_array include/linux/slab.h:631 [inline]
> kcalloc include/linux/slab.h:642 [inline]
> numa_crng_init drivers/char/random.c:798 [inline]
> crng_reseed+0x427/0x920 drivers/char/random.c:923
> credit_entropy_bits+0x98d/0xa30 drivers/char/random.c:708
> add_timer_randomness+0x26b/0x320 drivers/char/random.c:1133
> add_input_randomness+0xce/0x3e0 drivers/char/random.c:1148
> input_handle_event+0xb3/0x1210 drivers/input/input.c:375
> input_inject_event+0x367/0x3ed drivers/input/input.c:466
> evdev_write+0x4d1/0x860 drivers/input/evdev.c:560
> __vfs_write+0x10b/0x960 fs/read_write.c:485
> vfs_write+0x1f8/0x560 fs/read_write.c:549
> ksys_write+0xf9/0x250 fs/read_write.c:598
> __do_sys_write fs/read_write.c:610 [inline]
> __se_sys_write fs/read_write.c:607 [inline]
> __x64_sys_write+0x73/0xb0 fs/read_write.c:607
> do_syscall_64+0x1b1/0x800 arch/x86/entry/common.c:287
> entry_SYSCALL_64_after_hwframe+0x49/0xbe
> RIP: 0033:0x443db9
> RSP: 002b:00007ffd62c88e88 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
> RAX: ffffffffffffffda RBX: 0008000040000002 RCX: 0000000000443db9
> RDX: 0000000000000030 RSI: 00000000200000c0 RDI: 00000000000003ff
> RBP: 746e6576652f7475 R08: 00000000004002e0 R09: 00000000004002e0
> R10: 0000000000000000 R11: 0000000000000246 R12: 706e692f7665642f
> R13: 0000000000401af0 R14: 0000000000000000 R15: 0000000000000000
> random: crng init done
>
>
> ---
> This bug is generated by a dumb bot. It may contain errors.
> See https://goo.gl/tpsmEJ for details.
> Direct all questions to syzkaller@googlegroups.com.
>
> syzbot will keep track of this bug report.
> If you forgot to add the Reported-by tag, once the fix for this bug is merged
> into any tree, please reply to this email with:
> #syz fix: exact-commit-title
> If you want to test a patch for this bug, please reply with:
> #syz test: git://repo/address.git branch
> and provide the patch inline or as an attachment.
> To mark this as a duplicate of another syzbot report, please reply with:
> #syz dup: exact-subject-of-another-report
> If it's a one-off invalid bug report, please reply with:
> #syz invalid
> Note: if the crash happens again, it will cause creation of a new bug report.
> Note: all commands must start from beginning of the line in the email body.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v2 2/2] input: misc: Add Spreadtrum vibrator driver
From: Dmitry Torokhov @ 2018-04-23 17:29 UTC (permalink / raw)
To: Baolin Wang
Cc: robh+dt, mark.rutland, orsonzhai, zhang.lyra, linux-input,
devicetree, linux-kernel, xiaotong.lu
In-Reply-To: <dc7af2705ab659a7cf579145b7e1eda9697b0ef2.1524450536.git.baolin.wang@linaro.org>
Hi Xiaotong,
On Mon, Apr 23, 2018 at 10:33:36AM +0800, Baolin Wang wrote:
> 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>
> ---
> Changes since v1:
> - Remove input_ff_destroy() and input_unregister_device()
> ---
> drivers/input/misc/Kconfig | 10 +++
> drivers/input/misc/Makefile | 1 +
> drivers/input/misc/sc27xx-vibra.c | 156 +++++++++++++++++++++++++++++++++++++
> 3 files changed, 167 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..f78e70f
> --- /dev/null
> +++ b/drivers/input/misc/sc27xx-vibra.c
> @@ -0,0 +1,156 @@
> +// 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);
I do not think this is correct. If you issue 2 play requests with
info->strength that is not 0 then you'll end up disabling the vibrator.
I think you want the 2nd condition to be:
else if (info->strength == 0 && 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;
Can we please call this variable "error"?
> +
> + 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);
I am fan of generic device properties, please change to
device_property_read_u32().
> + 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;
Why is device reparented to the parent of platform device? This breaks
devm implementation for the input device. If you really need this,
you'll have to switch to unmanaged API.
> + 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");
> + return ret;
> + }
> +
> + ret = sc27xx_vibra_hw_init(info);
> + if (ret) {
> + dev_err(&pdev->dev, "failed to initialize the vibrator.\n");
> + return ret;
> + }
It is too late to initialize hardware after registering the input
device, as once registered it should be fully functional. I'd recommend
calling this before calling input_register_device(), or (maybe even
better) doing this as part of open() method.
> +
> + platform_set_drvdata(pdev, info);
I do not think you are using platform device's driver data anywhere.
> + 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,
Do you need suspend support by chance? To shut off the vibrator when
system transitions to sleep?
> + },
> + .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
>
Thanks.
--
Dmitry
^ permalink raw reply
* [PATCHv2] Input: atmel_mxt_ts - fix reset-gpio for level based irqs
From: Sebastian Reichel @ 2018-04-23 11:58 UTC (permalink / raw)
To: Nick Dyer, Dmitry Torokhov, linux-input
Cc: linux-kernel, Ezequiel Garcia, kernel, Sebastian Reichel
In-Reply-To: <20180420172408.26164-1-sebastian.reichel@collabora.co.uk>
The current reset-gpio support triggers an interrupt storm on platforms
using the maxtouch with level based interrupt. The Motorola Droid 4,
which I used for some of the tests is not affected, since it uses a edge
based interrupt.
This change avoids the interrupt storm by enabling the device while its
interrupt is disabled. Afterwards we wait 100ms. This is important for
two reasons: The device is unresponsive for some time (~22ms for
mxt224E) and the CHG (interrupt) line is not working properly for 100ms.
We don't need to wait for any following interrupts, since the following
mxt_initialize() checks for bootloader mode anyways.
This fixes a boot issue on GE PPD (watchdog kills device due to
interrupt storm) and does not cause regression on Motorola Droid 4.
Fixes: f657b00df22e ("Input: atmel_mxt_ts - add support for reset line")
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
---
Changes since PATCHv2:
* fix patch description
* use same timeout value for soft and gpio-reset
---
drivers/input/touchscreen/atmel_mxt_ts.c | 18 +++++++-----------
1 file changed, 7 insertions(+), 11 deletions(-)
diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 5d9699fe1b55..f97fbb0af020 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -194,6 +194,8 @@ enum t100_type {
/* Delay times */
#define MXT_BACKUP_TIME 50 /* msec */
+#define MXT_RESET_GPIO_TIME 20 /* msec */
+#define MXT_RESET_INVALID_CHG 100 /* msec */
#define MXT_RESET_TIME 200 /* msec */
#define MXT_RESET_TIMEOUT 3000 /* msec */
#define MXT_CRC_TIMEOUT 1000 /* msec */
@@ -1206,7 +1208,7 @@ static int mxt_soft_reset(struct mxt_data *data)
return ret;
/* Ignore CHG line for 100ms after reset */
- msleep(100);
+ msleep(MXT_RESET_INVALID_CHG);
mxt_acquire_irq(data);
@@ -3167,20 +3169,14 @@ static int mxt_probe(struct i2c_client *client, const struct i2c_device_id *id)
return error;
}
+ disable_irq(client->irq);
+
if (data->reset_gpio) {
- data->in_bootloader = true;
- msleep(MXT_RESET_TIME);
- reinit_completion(&data->bl_completion);
+ msleep(MXT_RESET_GPIO_TIME);
gpiod_set_value(data->reset_gpio, 1);
- error = mxt_wait_for_completion(data, &data->bl_completion,
- MXT_RESET_TIMEOUT);
- if (error)
- return error;
- data->in_bootloader = false;
+ msleep(MXT_RESET_INVALID_CHG);
}
- disable_irq(client->irq);
-
error = mxt_initialize(data);
if (error)
return error;
--
2.17.0
^ permalink raw reply related
* Re: [PATCH] Input: atmel_mxt_ts - fix reset-gpio for level based irqs
From: Sebastian Reichel @ 2018-04-23 11:43 UTC (permalink / raw)
To: Nick Dyer
Cc: Ezequiel Garcia, Dmitry Torokhov, linux-input, linux-kernel,
kernel
In-Reply-To: <20180421201137.fyr6eqzjuob765ie@hairyalien>
[-- Attachment #1: Type: text/plain, Size: 2784 bytes --]
Hi,
On Sat, Apr 21, 2018 at 09:11:37PM +0100, Nick Dyer wrote:
> On Fri, Apr 20, 2018 at 09:42:07PM +0200, Sebastian Reichel wrote:
> > On Fri, Apr 20, 2018 at 02:44:02PM -0300, Ezequiel Garcia wrote:
> > > Hi Sebastian,
> > >
> > > On Fri, 2018-04-20 at 19:24 +0200, Sebastian Reichel wrote:
> > > > The current reset-gpio support triggers an interrupt storm on platforms
> > > > using the maxtouch with level based interrupt. The Motorola Droid 4,
> > > > which I used for some of the tests is not affected, since it uses a level
> > > > based interrupt.
> > > >
> > >
> > > I found this confusing. Interrupt storm happen with level-based interrupts,
> > > but the Droid4 is not affected?
>
> Can I ask what happens during the interrupt storm. Are you getting lots
> of the "failed to read T44 and T5" message, or something else?
I don't get any message (with DEBUG enabled for the driver).
The boot process hangs and after a few seconds the watchdog kicks in.
> > > > This change avoids the interrupt storm by enabling the device while
> > > > its interrupt is disabled. The following mxt_initialize() requires,
> > > > that the device is responsive (at least mxt224E is unresponsive for
> > > > ~22ms), so we wait some time. We don't wait for leaving bootloader
> > > > mode anymore, since mxt_initialize() checks for it anyways.
> > > >
> > >
> > > IMHO, having some more or less arbritrary sleeps is almost
> > > always a problem. This value might be enough for some platform,
> > > might be too short for some other, and then it might get too large
> > > for someone else.
> >
> > The 22ms chip-being-unresponsive are not newly introduced. The
> > same 22ms are also required for soft-reset. I did introduce a
> > new time (MXT_RESET_GPIO_TIME) for the "chip being reset" state,
> > since my randomly chosen 200ms from before were exaggerated
> > considering all mxt datasheets I checked stated only a few nano
> > seconds.
>
> According to the data sheets there is a period after a reset where the
> CHG line is temporarily set as an input, during which the host should
> ignore it. If you don't, you might get a stray interrupt and try and
> communicate with the device, which might leave it in a bad state. I
> think you mentioned that later in your email.
>
> The reset time varies per chip, but the 100ms in mxt_soft_reset() was
> based on discussions with app support at Atmel, so should be correct in
> most cases.
Right. This sleep time is missing for the gpio based reset. The
other checks are not required, since the chip initialization
routine is called directly afterwards. I will send a PATCHv2,
that fixes the patch description and uses the same timeout value
for both resets.
-- Sebastian
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v2] Input: xen-kbdfront - allow better run-time configuration
From: Oleksandr Andrushchenko @ 2018-04-23 8:29 UTC (permalink / raw)
To: Juergen Gross, xen-devel, linux-input, linux-kernel,
dmitry.torokhov, lyan, boris.ostrovsky
Cc: andrii_chepurnyi, Oleksandr Andrushchenko
In-Reply-To: <a2189ec9-ef4d-2c23-28c6-a6ff8525e8e5@suse.com>
On 04/23/2018 11:23 AM, Juergen Gross wrote:
> On 23/04/18 10:02, Oleksandr Andrushchenko wrote:
>> Juergen, Jason, Dmitry
>> any comment on this?
> Oleksandr, please give us some time. I can't speak for others, but I am
> not sitting here idling and hoping that some work (e.g. patches to
> review) might appear.
>
> I have a lot of other stuff to do and will respond when I find some time
> to look at your patches.
>
> Pinging others on Monday when having sent out the patch only on Thursday
> is rather unfriendly.
Really sorry about this, my bad.
I had an impression that the only change we need to
discuss was uint -> bool change which seemed rather trivial
>
> Juergen
^ permalink raw reply
* Re: [PATCH v2] Input: xen-kbdfront - allow better run-time configuration
From: Juergen Gross @ 2018-04-23 8:23 UTC (permalink / raw)
To: Oleksandr Andrushchenko, xen-devel, linux-input, linux-kernel,
dmitry.torokhov, lyan, boris.ostrovsky
Cc: andrii_chepurnyi, Oleksandr Andrushchenko
In-Reply-To: <998533bb-8042-b07e-cb79-661750e035d9@gmail.com>
On 23/04/18 10:02, Oleksandr Andrushchenko wrote:
> Juergen, Jason, Dmitry
> any comment on this?
Oleksandr, please give us some time. I can't speak for others, but I am
not sitting here idling and hoping that some work (e.g. patches to
review) might appear.
I have a lot of other stuff to do and will respond when I find some time
to look at your patches.
Pinging others on Monday when having sent out the patch only on Thursday
is rather unfriendly.
Juergen
^ 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