Re: [PATCH v4 2/7] Input: use input_device_enabled()
On Tue, Dec 08, 2020 at 11:05:42AM +0100, Marek Szyprowski wrote:
> Hi Andrzej,
>
> On 07.12.2020 16:50, Andrzej Pietrasiewicz wrote:
> > Hi Marek,
> >
> > W dniu 07.12.2020 o 14:32, Marek Szyprowski pisze:
> >> Hi Andrzej,
> >>
> >> On 08.06.2020 13:22, Andrzej Pietrasiewicz wrote:
> >>> Use the newly added helper in relevant input drivers.
> >>>
> >>> Signed-off-by: Andrzej Pietrasiewicz
> >>
> >> This patch landed recently in linux-next as commit d69f0a43c677 ("Input:
> >> use input_device_enabled()"). Sadly it causes following warning during
> >> system suspend/resume cycle on ARM 32bit Samsung Exynos5250-based Snow
> >> Chromebook with kernel compiled from exynos_defconfig:
> >>
> >> [ cut here ]
> >> WARNING: CPU: 0 PID: 1777 at drivers/input/input.c:2230
> >> input_device_enabled+0x68/0x6c
> >> Modules linked in: cmac bnep mwifiex_sdio mwifiex sha256_generic
> >> libsha256 sha256_arm cfg80211 btmrvl_sdio btmrvl bluetooth s5p_mfc
> >> exynos_gsc v4l2_mem2mem videob
> >> CPU: 0 PID: 1777 Comm: rtcwake Not tainted
> >> 5.10.0-rc6-next-20201207-1-g49a0dc04c46d-dirty #9902
> >> Hardware name: Samsung Exynos (Flattened Device Tree)
> >> [] (unwind_backtrace) from [] (show_stack+0x10/0x14)
> >> [] (show_stack) from [] (dump_stack+0xb4/0xd4)
> >> [] (dump_stack) from [] (__warn+0xd8/0x11c)
> >> [] (__warn) from [] (warn_slowpath_fmt+0xb0/0xb8)
> >> [] (warn_slowpath_fmt) from []
> >> (input_device_enabled+0x68/0x6c)
> >> [] (input_device_enabled) from []
> >
> > Apparently you are hitting this line of code in drivers/input/input.c:
> >
> > lockdep_assert_held(&dev->mutex);
> >
> > Inspecting input device's "users" member should happen under dev's lock.
> >
> This check and warning has been introduced by this patch. I assume that
> the suspend/resume paths are correct, but it looks that they were not
> tested with this patch thus it has not been noticed that they are not
> called under the input's lock. This needs a fix. Dmitry: how would you
> like to handle this issue?
The check is proper and the warning is legit, cyapa should not be
checking this field without holding the lock. I think we can simply
remove this check from the power ops for gen3 and gen5, and this should
shut up the warning on suspend, but there other places in cyapa that do
check 'users', and they also need to be fixed.
Thanks.
--
Dmitry
Re: [PATCH v4 2/7] Input: use input_device_enabled()
Hi Andrzej,
On 07.12.2020 16:50, Andrzej Pietrasiewicz wrote:
> Hi Marek,
>
> W dniu 07.12.2020 o 14:32, Marek Szyprowski pisze:
>> Hi Andrzej,
>>
>> On 08.06.2020 13:22, Andrzej Pietrasiewicz wrote:
>>> Use the newly added helper in relevant input drivers.
>>>
>>> Signed-off-by: Andrzej Pietrasiewicz
>>
>> This patch landed recently in linux-next as commit d69f0a43c677 ("Input:
>> use input_device_enabled()"). Sadly it causes following warning during
>> system suspend/resume cycle on ARM 32bit Samsung Exynos5250-based Snow
>> Chromebook with kernel compiled from exynos_defconfig:
>>
>> [ cut here ]
>> WARNING: CPU: 0 PID: 1777 at drivers/input/input.c:2230
>> input_device_enabled+0x68/0x6c
>> Modules linked in: cmac bnep mwifiex_sdio mwifiex sha256_generic
>> libsha256 sha256_arm cfg80211 btmrvl_sdio btmrvl bluetooth s5p_mfc
>> exynos_gsc v4l2_mem2mem videob
>> CPU: 0 PID: 1777 Comm: rtcwake Not tainted
>> 5.10.0-rc6-next-20201207-1-g49a0dc04c46d-dirty #9902
>> Hardware name: Samsung Exynos (Flattened Device Tree)
>> [] (unwind_backtrace) from [] (show_stack+0x10/0x14)
>> [] (show_stack) from [] (dump_stack+0xb4/0xd4)
>> [] (dump_stack) from [] (__warn+0xd8/0x11c)
>> [] (__warn) from [] (warn_slowpath_fmt+0xb0/0xb8)
>> [] (warn_slowpath_fmt) from []
>> (input_device_enabled+0x68/0x6c)
>> [] (input_device_enabled) from []
>
> Apparently you are hitting this line of code in drivers/input/input.c:
>
> lockdep_assert_held(&dev->mutex);
>
> Inspecting input device's "users" member should happen under dev's lock.
>
This check and warning has been introduced by this patch. I assume that
the suspend/resume paths are correct, but it looks that they were not
tested with this patch thus it has not been noticed that they are not
called under the input's lock. This needs a fix. Dmitry: how would you
like to handle this issue?
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
Re: [PATCH v4 2/7] Input: use input_device_enabled()
Hi Marek,
W dniu 07.12.2020 o 14:32, Marek Szyprowski pisze:
Hi Andrzej,
On 08.06.2020 13:22, Andrzej Pietrasiewicz wrote:
Use the newly added helper in relevant input drivers.
Signed-off-by: Andrzej Pietrasiewicz
This patch landed recently in linux-next as commit d69f0a43c677 ("Input:
use input_device_enabled()"). Sadly it causes following warning during
system suspend/resume cycle on ARM 32bit Samsung Exynos5250-based Snow
Chromebook with kernel compiled from exynos_defconfig:
[ cut here ]
WARNING: CPU: 0 PID: 1777 at drivers/input/input.c:2230
input_device_enabled+0x68/0x6c
Modules linked in: cmac bnep mwifiex_sdio mwifiex sha256_generic
libsha256 sha256_arm cfg80211 btmrvl_sdio btmrvl bluetooth s5p_mfc
exynos_gsc v4l2_mem2mem videob
CPU: 0 PID: 1777 Comm: rtcwake Not tainted
5.10.0-rc6-next-20201207-1-g49a0dc04c46d-dirty #9902
Hardware name: Samsung Exynos (Flattened Device Tree)
[] (unwind_backtrace) from [] (show_stack+0x10/0x14)
[] (show_stack) from [] (dump_stack+0xb4/0xd4)
[] (dump_stack) from [] (__warn+0xd8/0x11c)
[] (__warn) from [] (warn_slowpath_fmt+0xb0/0xb8)
[] (warn_slowpath_fmt) from []
(input_device_enabled+0x68/0x6c)
[] (input_device_enabled) from []
Apparently you are hitting this line of code in drivers/input/input.c:
lockdep_assert_held(&dev->mutex);
Inspecting input device's "users" member should happen under dev's lock.
Regards,
Andrzej
Re: [PATCH v4 2/7] Input: use input_device_enabled()
Hi Andrzej,
On 08.06.2020 13:22, Andrzej Pietrasiewicz wrote:
> Use the newly added helper in relevant input drivers.
>
> Signed-off-by: Andrzej Pietrasiewicz
This patch landed recently in linux-next as commit d69f0a43c677 ("Input:
use input_device_enabled()"). Sadly it causes following warning during
system suspend/resume cycle on ARM 32bit Samsung Exynos5250-based Snow
Chromebook with kernel compiled from exynos_defconfig:
[ cut here ]
WARNING: CPU: 0 PID: 1777 at drivers/input/input.c:2230
input_device_enabled+0x68/0x6c
Modules linked in: cmac bnep mwifiex_sdio mwifiex sha256_generic
libsha256 sha256_arm cfg80211 btmrvl_sdio btmrvl bluetooth s5p_mfc
exynos_gsc v4l2_mem2mem videob
CPU: 0 PID: 1777 Comm: rtcwake Not tainted
5.10.0-rc6-next-20201207-1-g49a0dc04c46d-dirty #9902
Hardware name: Samsung Exynos (Flattened Device Tree)
[] (unwind_backtrace) from [] (show_stack+0x10/0x14)
[] (show_stack) from [] (dump_stack+0xb4/0xd4)
[] (dump_stack) from [] (__warn+0xd8/0x11c)
[] (__warn) from [] (warn_slowpath_fmt+0xb0/0xb8)
[] (warn_slowpath_fmt) from []
(input_device_enabled+0x68/0x6c)
[] (input_device_enabled) from []
(cyapa_gen3_set_power_mode+0x128/0x1cc)
[] (cyapa_gen3_set_power_mode) from []
(cyapa_suspend+0x74/0x114)
[] (cyapa_suspend) from [] (dpm_run_callback+0xb0/0x3c8)
[] (dpm_run_callback) from []
(__device_suspend+0x104/0x784)
[] (__device_suspend) from [] (dpm_suspend+0x184/0x540)
[] (dpm_suspend) from [] (dpm_suspend_start+0x98/0xa0)
[] (dpm_suspend_start) from []
(suspend_devices_and_enter+0xec/0xbd4)
[] (suspend_devices_and_enter) from []
(pm_suspend+0x314/0x42c)
[] (pm_suspend) from [] (state_store+0x6c/0xc8)
[] (state_store) from [] (kernfs_fop_write+0x10c/0x228)
[] (kernfs_fop_write) from [] (vfs_write+0xc8/0x530)
[] (vfs_write) from [] (ksys_write+0x60/0xd8)
[] (ksys_write) from [] (ret_fast_syscall+0x0/0x2c)
Exception stack(0xc45bffa8 to 0xc45bfff0)
...
irq event stamp: 14101
hardirqs last enabled at (14109): [] vprintk_emit+0x2d8/0x32c
hardirqs last disabled at (14116): [] vprintk_emit+0x29c/0x32c
softirqs last enabled at (13264): [] __do_softirq+0x528/0x684
softirqs last disabled at (13253): [] irq_exit+0x1ec/0x1f8
---[ end trace 6687a21e6b7e94a9 ]---
[ cut here ]
WARNING: CPU: 1 PID: 1777 at drivers/input/input.c:2230
input_device_enabled+0x68/0x6c
Modules linked in: cmac bnep mwifiex_sdio mwifiex sha256_generic
libsha256 sha256_arm cfg80211 btmrvl_sdio btmrvl bluetooth s5p_mfc
exynos_gsc v4l2_mem2mem videob
CPU: 1 PID: 1777 Comm: rtcwake Tainted: G W
5.10.0-rc6-next-20201207-1-g49a0dc04c46d-dirty #9902
Hardware name: Samsung Exynos (Flattened Device Tree)
[] (unwind_backtrace) from [] (show_stack+0x10/0x14)
[] (show_stack) from [] (dump_stack+0xb4/0xd4)
[] (dump_stack) from [] (__warn+0xd8/0x11c)
[] (__warn) from [] (warn_slowpath_fmt+0xb0/0xb8)
[] (warn_slowpath_fmt) from []
(input_device_enabled+0x68/0x6c)
[] (input_device_enabled) from []
(cyapa_gen3_set_power_mode+0x128/0x1cc)
[] (cyapa_gen3_set_power_mode) from []
(cyapa_reinitialize+0xcc/0x154)
[] (cyapa_reinitialize) from [] (cyapa_resume+0x48/0x98)
[] (cyapa_resume) from [] (dpm_run_callback+0xb0/0x3c8)
[] (dpm_run_callback) from [] (device_resume+0xbc/0x260)
[] (device_resume) from [] (dpm_resume+0x14c/0x51c)
[] (dpm_resume) from [] (dpm_resume_end+0xc/0x18)
[] (dpm_resume_end) from []
(suspend_devices_and_enter+0x1b4/0xbd4)
[] (suspend_devices_and_enter) from []
(pm_suspend+0x314/0x42c)
[] (pm_suspend) from [] (state_store+0x6c/0xc8)
[] (state_store) from [] (kernfs_fop_write+0x10c/0x228)
[] (kernfs_fop_write) from [] (vfs_write+0xc8/0x530)
[] (vfs_write) from [] (ksys_write+0x60/0xd8)
[] (ksys_write) from [] (ret_fast_syscall+0x0/0x2c)
Exception stack(0xc45bffa8 to 0xc45bfff0)
...
irq event stamp: 55479
hardirqs last enabled at (55487): [] vprintk_emit+0x2d8/0x32c
hardirqs last disabled at (55494): [] vprintk_emit+0x29c/0x32c
softirqs last enabled at (53552): [] __do_softirq+0x528/0x684
softirqs last disabled at (53541): [] irq_exit+0x1ec/0x1f8
---[ end trace 6687a21e6b7e94aa ]---
[ cut here ]
WARNING: CPU: 1 PID: 1777 at drivers/input/input.c:2230
input_device_enabled+0x68/0x6c
Modules linked in: cmac bnep mwifiex_sdio mwifiex sha256_generic
libsha256 sha256_arm cfg80211 btmrvl_sdio btmrvl bluetooth s5p_mfc
exynos_gsc v4l2_mem2mem videob
CPU: 1 PID: 1777 Comm: rtcwake Tainted: G W
5.10.0-rc6-next-20201207-1-g49a0dc04c46d-dirty #9902
Hardware name: Samsung Exynos (Flattened Device Tree)
[] (unwind_backtrace) from [] (show_stack+0x10/0x14)
[] (show_stack) from [] (dump_stack+0xb4/0xd4)
[] (dump_stack) from [] (__warn+0xd8/0x11c)
[] (__warn) from [] (warn_slowpath_fmt+0xb0/0xb8)
[] (warn_slowpath_fmt) from []
(input_device_enabled+0x68/0x6c)
[] (input_device_enabled) from []
(cyapa_reinitialize+0x4c/0x154)
[] (cyapa_reinitialize) from [] (cyapa_resume+0x48/0x98)
[] (cyapa_resume) from [] (dpm_r
Re: [PATCH v4 2/7] Input: use input_device_enabled()
On Mon, Jun 08, 2020 at 01:22:06PM +0200, Andrzej Pietrasiewicz wrote: > Use the newly added helper in relevant input drivers. > > Signed-off-by: Andrzej Pietrasiewicz Applied, thank you. -- Dmitry
[PATCH v4 2/7] Input: use input_device_enabled()
Use the newly added helper in relevant input drivers.
Signed-off-by: Andrzej Pietrasiewicz
---
drivers/input/joystick/xpad.c | 4 ++--
drivers/input/keyboard/ep93xx_keypad.c | 2 +-
drivers/input/keyboard/gpio_keys.c | 4 ++--
drivers/input/keyboard/imx_keypad.c | 4 ++--
drivers/input/keyboard/ipaq-micro-keys.c| 2 +-
drivers/input/keyboard/lpc32xx-keys.c | 4 ++--
drivers/input/keyboard/pmic8xxx-keypad.c| 4 ++--
drivers/input/keyboard/pxa27x_keypad.c | 2 +-
drivers/input/keyboard/samsung-keypad.c | 4 ++--
drivers/input/keyboard/spear-keyboard.c | 8
drivers/input/keyboard/st-keyscan.c | 4 ++--
drivers/input/keyboard/tegra-kbc.c | 4 ++--
drivers/input/misc/drv260x.c| 4 ++--
drivers/input/misc/drv2665.c| 4 ++--
drivers/input/misc/drv2667.c| 4 ++--
drivers/input/misc/gp2ap002a00f.c | 4 ++--
drivers/input/misc/kxtj9.c | 4 ++--
drivers/input/misc/sirfsoc-onkey.c | 2 +-
drivers/input/mouse/navpoint.c | 4 ++--
drivers/input/touchscreen/ad7879.c | 6 +++---
drivers/input/touchscreen/atmel_mxt_ts.c| 4 ++--
drivers/input/touchscreen/auo-pixcir-ts.c | 8
drivers/input/touchscreen/bu21029_ts.c | 4 ++--
drivers/input/touchscreen/chipone_icn8318.c | 4 ++--
drivers/input/touchscreen/cyttsp_core.c | 4 ++--
drivers/input/touchscreen/eeti_ts.c | 4 ++--
drivers/input/touchscreen/ektf2127.c| 4 ++--
drivers/input/touchscreen/imx6ul_tsc.c | 4 ++--
drivers/input/touchscreen/ipaq-micro-ts.c | 2 +-
drivers/input/touchscreen/iqs5xx.c | 4 ++--
drivers/input/touchscreen/lpc32xx_ts.c | 4 ++--
drivers/input/touchscreen/melfas_mip4.c | 4 ++--
drivers/input/touchscreen/mms114.c | 6 +++---
drivers/input/touchscreen/pixcir_i2c_ts.c | 8
drivers/input/touchscreen/ucb1400_ts.c | 4 ++--
drivers/input/touchscreen/wm97xx-core.c | 14 +-
drivers/input/touchscreen/zforce_ts.c | 8
37 files changed, 86 insertions(+), 82 deletions(-)
diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index c77cdb3b62b5..d8b6bc2d2171 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -1902,7 +1902,7 @@ static int xpad_suspend(struct usb_interface *intf,
pm_message_t message)
xpad360w_poweroff_controller(xpad);
} else {
mutex_lock(&input->mutex);
- if (input->users)
+ if (input_device_enabled(input))
xpad_stop_input(xpad);
mutex_unlock(&input->mutex);
}
@@ -1922,7 +1922,7 @@ static int xpad_resume(struct usb_interface *intf)
retval = xpad360w_start_input(xpad);
} else {
mutex_lock(&input->mutex);
- if (input->users) {
+ if (input_device_enabled(input)) {
retval = xpad_start_input(xpad);
} else if (xpad->xtype == XTYPE_XBOXONE) {
/*
diff --git a/drivers/input/keyboard/ep93xx_keypad.c
b/drivers/input/keyboard/ep93xx_keypad.c
index 7c70492d9d6b..8194e843d047 100644
--- a/drivers/input/keyboard/ep93xx_keypad.c
+++ b/drivers/input/keyboard/ep93xx_keypad.c
@@ -208,7 +208,7 @@ static int ep93xx_keypad_resume(struct device *dev)
mutex_lock(&input_dev->mutex);
- if (input_dev->users) {
+ if (input_device_enabled(input_dev)) {
if (!keypad->enabled) {
ep93xx_keypad_config(keypad);
clk_enable(keypad->clk);
diff --git a/drivers/input/keyboard/gpio_keys.c
b/drivers/input/keyboard/gpio_keys.c
index 53c9ff338dea..03ad27189553 100644
--- a/drivers/input/keyboard/gpio_keys.c
+++ b/drivers/input/keyboard/gpio_keys.c
@@ -966,7 +966,7 @@ static int __maybe_unused gpio_keys_suspend(struct device
*dev)
return error;
} else {
mutex_lock(&input->mutex);
- if (input->users)
+ if (input_device_enabled(input))
gpio_keys_close(input);
mutex_unlock(&input->mutex);
}
@@ -984,7 +984,7 @@ static int __maybe_unused gpio_keys_resume(struct device
*dev)
gpio_keys_disable_wakeup(ddata);
} else {
mutex_lock(&input->mutex);
- if (input->users)
+ if (input_device_enabled(input))
error = gpio_keys_open(input);
mutex_unlock(&input->mutex);
}
diff --git a/drivers/input/keyboard/imx_keypad.c
b/drivers/input/keyboard/imx_keypad.c
index 5a46d113e909..1f5c9ea5e9e5 100644
--- a/drivers/input/keyboard/imx_keypad.c
+++ b/drivers/input/keyboard/imx_keypad.c
@@ -532,7 +532,7 @@ static int __maybe_unused imx_kb
