* Re: [PATCH] Input: cros_ec_keyb: Add back missing mask for event_type
From: Nicolas Boichat @ 2019-08-13 9:47 UTC (permalink / raw)
To: Fei Shao
Cc: linux-arm Mailing List, Dmitry Torokhov, Benson Leung,
Enric Balletbo i Serra, Guenter Roeck, Ting Shen, Brian Norris,
open list:HID CORE LAYER, lkml
In-Reply-To: <20190813093821.74158-1-fshao@chromium.org>
On Tue, Aug 13, 2019 at 5:38 PM Fei Shao <fshao@chromium.org> wrote:
>
> In the previous patch we didn't mask out event_type in case statement,
> so switches are always picked instead of buttons, which results in
> ChromeOS devices misbehaving when power button is pressed.
> This patch adds back the missing mask.
>
> Fixes: d096aa3eb604 ("Input: cros_ec_keyb: mask out extra flags in event_type")
> Signed-off-by: Fei Shao <fshao@chromium.org>
Reviewed-by: Nicolas Boichat <drinkcat@chromium.org>
> ---
> drivers/input/keyboard/cros_ec_keyb.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
> index 38cb6d82d8fe..bef7bee6f05e 100644
> --- a/drivers/input/keyboard/cros_ec_keyb.c
> +++ b/drivers/input/keyboard/cros_ec_keyb.c
> @@ -226,6 +226,8 @@ static int cros_ec_keyb_work(struct notifier_block *nb,
> {
> struct cros_ec_keyb *ckdev = container_of(nb, struct cros_ec_keyb,
> notifier);
> + uint8_t mkbp_event_type = ckdev->ec->event_data.event_type &
> + EC_MKBP_EVENT_TYPE_MASK;
> u32 val;
> unsigned int ev_type;
>
> @@ -237,7 +239,7 @@ static int cros_ec_keyb_work(struct notifier_block *nb,
> if (queued_during_suspend && !device_may_wakeup(ckdev->dev))
> return NOTIFY_OK;
>
> - switch (ckdev->ec->event_data.event_type & EC_MKBP_EVENT_TYPE_MASK) {
> + switch (mkbp_event_type) {
> case EC_MKBP_EVENT_KEY_MATRIX:
> pm_wakeup_event(ckdev->dev, 0);
>
> @@ -264,7 +266,7 @@ static int cros_ec_keyb_work(struct notifier_block *nb,
> case EC_MKBP_EVENT_SWITCH:
> pm_wakeup_event(ckdev->dev, 0);
>
> - if (ckdev->ec->event_data.event_type == EC_MKBP_EVENT_BUTTON) {
> + if (mkbp_event_type == EC_MKBP_EVENT_BUTTON) {
> val = get_unaligned_le32(
> &ckdev->ec->event_data.data.buttons);
> ev_type = EV_KEY;
> --
> 2.23.0.rc1.153.gdeed80330f-goog
^ permalink raw reply
* Re: [PATCH v4 3/9] nvmem: core: add nvmem_device_find
From: Srinivas Kandagatla @ 2019-08-13 9:40 UTC (permalink / raw)
To: Thomas Bogendoerfer, Ralf Baechle, Paul Burton, James Hogan,
Dmitry Torokhov, Lee Jones, David S. Miller, Alessandro Zummo,
Alexandre Belloni, Greg Kroah-Hartman, Jiri Slaby,
Evgeniy Polyakov, linux-mips, linux-kernel, linux-input, netdev,
linux-rtc, linux-serial
In-Reply-To: <20190809103235.16338-4-tbogendoerfer@suse.de>
On 09/08/2019 11:32, Thomas Bogendoerfer wrote:
> nvmem_device_find provides a way to search for nvmem devices with
> the help of a match function simlair to bus_find_device.
>
> Signed-off-by: Thomas Bogendoerfer <tbogendoerfer@suse.de>
> ---
> drivers/nvmem/core.c | 62 ++++++++++++++++++++++--------------------
> include/linux/nvmem-consumer.h | 9 ++++++
> 2 files changed, 41 insertions(+), 30 deletions(-)
Have you considered using nvmem_register_notifier() ?
--srini
^ permalink raw reply
* [PATCH] Input: cros_ec_keyb: Add back missing mask for event_type
From: Fei Shao @ 2019-08-13 9:38 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Dmitry Torokhov, Benson Leung, Enric Balletbo i Serra,
Guenter Roeck, Ting Shen, Brian Norris, Fei Shao, linux-input,
linux-kernel
In the previous patch we didn't mask out event_type in case statement,
so switches are always picked instead of buttons, which results in
ChromeOS devices misbehaving when power button is pressed.
This patch adds back the missing mask.
Fixes: d096aa3eb604 ("Input: cros_ec_keyb: mask out extra flags in event_type")
Signed-off-by: Fei Shao <fshao@chromium.org>
---
drivers/input/keyboard/cros_ec_keyb.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
index 38cb6d82d8fe..bef7bee6f05e 100644
--- a/drivers/input/keyboard/cros_ec_keyb.c
+++ b/drivers/input/keyboard/cros_ec_keyb.c
@@ -226,6 +226,8 @@ static int cros_ec_keyb_work(struct notifier_block *nb,
{
struct cros_ec_keyb *ckdev = container_of(nb, struct cros_ec_keyb,
notifier);
+ uint8_t mkbp_event_type = ckdev->ec->event_data.event_type &
+ EC_MKBP_EVENT_TYPE_MASK;
u32 val;
unsigned int ev_type;
@@ -237,7 +239,7 @@ static int cros_ec_keyb_work(struct notifier_block *nb,
if (queued_during_suspend && !device_may_wakeup(ckdev->dev))
return NOTIFY_OK;
- switch (ckdev->ec->event_data.event_type & EC_MKBP_EVENT_TYPE_MASK) {
+ switch (mkbp_event_type) {
case EC_MKBP_EVENT_KEY_MATRIX:
pm_wakeup_event(ckdev->dev, 0);
@@ -264,7 +266,7 @@ static int cros_ec_keyb_work(struct notifier_block *nb,
case EC_MKBP_EVENT_SWITCH:
pm_wakeup_event(ckdev->dev, 0);
- if (ckdev->ec->event_data.event_type == EC_MKBP_EVENT_BUTTON) {
+ if (mkbp_event_type == EC_MKBP_EVENT_BUTTON) {
val = get_unaligned_le32(
&ckdev->ec->event_data.data.buttons);
ev_type = EV_KEY;
--
2.23.0.rc1.153.gdeed80330f-goog
^ permalink raw reply related
* Re: [PATCH v4 8/9] MIPS: SGI-IP27: fix readb/writeb addressing
From: Philippe Mathieu-Daudé @ 2019-08-13 8:47 UTC (permalink / raw)
To: Greg Kroah-Hartman, Andy Shevchenko
Cc: Thomas Bogendoerfer, Ralf Baechle, Paul Burton, James Hogan,
Dmitry Torokhov, Lee Jones, David S. Miller, Srinivas Kandagatla,
Alessandro Zummo, Alexandre Belloni, Jiri Slaby, Evgeniy Polyakov,
linux-mips, Linux Kernel Mailing List, linux-input, netdev,
open list:REAL TIME CLOCK (RTC) SUBSYSTEM,
open list:SERIAL DRIVERS
In-Reply-To: <20190811072907.GA1416@kroah.com>
Hi Thomas,
On 8/11/19 9:29 AM, Greg Kroah-Hartman wrote:
> On Sat, Aug 10, 2019 at 04:22:23PM +0300, Andy Shevchenko wrote:
>> On Fri, Aug 9, 2019 at 1:34 PM Thomas Bogendoerfer
>> <tbogendoerfer@suse.de> wrote:
>>>
>>> Our chosen byte swapping, which is what firmware already uses, is to
>>> do readl/writel by normal lw/sw intructions (data invariance). This
>>> also means we need to mangle addresses for u8 and u16 accesses. The
>>> mangling for 16bit has been done aready, but 8bit one was missing.
>>> Correcting this causes different addresses for accesses to the
>>> SuperIO and local bus of the IOC3 chip. This is fixed by changing
>>> byte order in ioc3 and m48rtc_rtc structs.
>>
>>> /* serial port register map */
>>> struct ioc3_serialregs {
>>> - uint32_t sscr;
>>> - uint32_t stpir;
>>> - uint32_t stcir;
>>> - uint32_t srpir;
>>> - uint32_t srcir;
>>> - uint32_t srtr;
>>> - uint32_t shadow;
>>> + u32 sscr;
>>> + u32 stpir;
>>> + u32 stcir;
>>> + u32 srpir;
>>> + u32 srcir;
>>> + u32 srtr;
>>> + u32 shadow;
>>> };
>>
>> Isn't it a churn? AFAIU kernel documentation the uint32_t is okay to
>> use, just be consistent inside one module / driver.
>> Am I mistaken?
>
> No, but really it uint* shouldn't be used anywhere in the kernel source
> as it does not make sense.
If you respin your series, please send this cleanup as a separate patch.
Thanks,
Phil.
^ permalink raw reply
* Re: WARNING in usbhid_raw_request/usb_submit_urb
From: Oliver Neukum @ 2019-08-13 8:14 UTC (permalink / raw)
To: Hillf Danton, syzbot
Cc: gustavo, Jiri Slaby, andreyknvl, syzkaller-bugs, gregkh, stern,
Jiri Kosina, linux-input, linux-kernel, linux-usb
In-Reply-To: <20190813042649.888-1-hdanton@sina.com>
Am Dienstag, den 13.08.2019, 12:26 +0800 schrieb Hillf Danton:
> [respin with the mess in Cc list cleaned up]
> Followup of commit e3e14de50dff ("HID: fix start/stop cycle in usbhid driver")
>
> --- a/drivers/hid/usbhid/hid-core.c
> +++ b/drivers/hid/usbhid/hid-core.c
> @@ -1214,6 +1214,8 @@ static void usbhid_stop(struct hid_devic
>
> hid->claimed = 0;
>
> + if (!usbhid->urbin) /* freeing buffers only once */
> + return;
> usb_free_urb(usbhid->urbin);
> usb_free_urb(usbhid->urbctrl);
> usb_free_urb(usbhid->urbout);
This looks rather suspicious. Why is stop() called multiple times?
Do we have a refcounting issue? If not, what controls locking?
Regards
Oliver
^ permalink raw reply
* Re: [PATCH 1/2] HID: do not call hid_set_drvdata(hdev, NULL) in drivers
From: Bruno Prémont @ 2019-08-13 5:53 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Jonathan Cameron, Srinivas Pandruvada, Ping Cheng, Jason Gerecke,
Jiri Kosina, linux-input, linux-kernel
In-Reply-To: <20190812162740.15898-2-benjamin.tissoires@redhat.com>
On Mon, 12 Aug 2019 18:27:39 +0200 Benjamin Tissoires wrote:
> This is a common pattern in the HID drivers to reset the drvdata. Some
> do it properly, some do it only in case of failure.
>
> But, this is actually already handled by driver core, so there is no need
> to do it manually.
>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
For hid-picolcd_core.c:
Acked-by: Bruno Prémont <bonbons@linux-vserver.org>
> ---
> drivers/hid/hid-cougar.c | 6 ++----
> drivers/hid/hid-gfrm.c | 7 -------
> drivers/hid/hid-lenovo.c | 2 --
> drivers/hid/hid-picolcd_core.c | 7 +------
> drivers/hid/hid-sensor-hub.c | 1 -
> 5 files changed, 3 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/hid/hid-cougar.c b/drivers/hid/hid-cougar.c
> index e0bb7b34f3a4..4ff3bc1d25e2 100644
> --- a/drivers/hid/hid-cougar.c
> +++ b/drivers/hid/hid-cougar.c
> @@ -207,7 +207,7 @@ static int cougar_probe(struct hid_device *hdev,
> error = hid_parse(hdev);
> if (error) {
> hid_err(hdev, "parse failed\n");
> - goto fail;
> + return error;
> }
>
> if (hdev->collection->usage == COUGAR_VENDOR_USAGE) {
> @@ -219,7 +219,7 @@ static int cougar_probe(struct hid_device *hdev,
> error = hid_hw_start(hdev, connect_mask);
> if (error) {
> hid_err(hdev, "hw start failed\n");
> - goto fail;
> + return error;
> }
>
> error = cougar_bind_shared_data(hdev, cougar);
> @@ -249,8 +249,6 @@ static int cougar_probe(struct hid_device *hdev,
>
> fail_stop_and_cleanup:
> hid_hw_stop(hdev);
> -fail:
> - hid_set_drvdata(hdev, NULL);
> return error;
> }
>
> diff --git a/drivers/hid/hid-gfrm.c b/drivers/hid/hid-gfrm.c
> index 86c317320bf2..699186ff2349 100644
> --- a/drivers/hid/hid-gfrm.c
> +++ b/drivers/hid/hid-gfrm.c
> @@ -123,12 +123,6 @@ static int gfrm_probe(struct hid_device *hdev, const struct hid_device_id *id)
> return ret;
> }
>
> -static void gfrm_remove(struct hid_device *hdev)
> -{
> - hid_hw_stop(hdev);
> - hid_set_drvdata(hdev, NULL);
> -}
> -
> static const struct hid_device_id gfrm_devices[] = {
> { HID_BLUETOOTH_DEVICE(0x58, 0x2000),
> .driver_data = GFRM100 },
> @@ -142,7 +136,6 @@ static struct hid_driver gfrm_driver = {
> .name = "gfrm",
> .id_table = gfrm_devices,
> .probe = gfrm_probe,
> - .remove = gfrm_remove,
> .input_mapping = gfrm_input_mapping,
> .raw_event = gfrm_raw_event,
> .input_configured = gfrm_input_configured,
> diff --git a/drivers/hid/hid-lenovo.c b/drivers/hid/hid-lenovo.c
> index 364bc7f11d9d..96fa2a2c2cd3 100644
> --- a/drivers/hid/hid-lenovo.c
> +++ b/drivers/hid/hid-lenovo.c
> @@ -866,8 +866,6 @@ static void lenovo_remove_tpkbd(struct hid_device *hdev)
>
> led_classdev_unregister(&data_pointer->led_micmute);
> led_classdev_unregister(&data_pointer->led_mute);
> -
> - hid_set_drvdata(hdev, NULL);
> }
>
> static void lenovo_remove_cptkbd(struct hid_device *hdev)
> diff --git a/drivers/hid/hid-picolcd_core.c b/drivers/hid/hid-picolcd_core.c
> index 5f7a39a5d4af..1b5c63241af0 100644
> --- a/drivers/hid/hid-picolcd_core.c
> +++ b/drivers/hid/hid-picolcd_core.c
> @@ -534,8 +534,7 @@ static int picolcd_probe(struct hid_device *hdev,
> data = kzalloc(sizeof(struct picolcd_data), GFP_KERNEL);
> if (data == NULL) {
> hid_err(hdev, "can't allocate space for Minibox PicoLCD device data\n");
> - error = -ENOMEM;
> - goto err_no_cleanup;
> + return -ENOMEM;
> }
>
> spin_lock_init(&data->lock);
> @@ -597,9 +596,6 @@ static int picolcd_probe(struct hid_device *hdev,
> hid_hw_stop(hdev);
> err_cleanup_data:
> kfree(data);
> -err_no_cleanup:
> - hid_set_drvdata(hdev, NULL);
> -
> return error;
> }
>
> @@ -635,7 +631,6 @@ static void picolcd_remove(struct hid_device *hdev)
> picolcd_exit_cir(data);
> picolcd_exit_keys(data);
>
> - hid_set_drvdata(hdev, NULL);
> mutex_destroy(&data->mutex);
> /* Finally, clean up the picolcd data itself */
> kfree(data);
> diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c
> index be92a6f79687..94c7398b5c27 100644
> --- a/drivers/hid/hid-sensor-hub.c
> +++ b/drivers/hid/hid-sensor-hub.c
> @@ -742,7 +742,6 @@ static void sensor_hub_remove(struct hid_device *hdev)
> }
> spin_unlock_irqrestore(&data->lock, flags);
> mfd_remove_devices(&hdev->dev);
> - hid_set_drvdata(hdev, NULL);
> mutex_destroy(&data->mutex);
> }
>
^ permalink raw reply
* Re: [PATCH v4 1/3] dt-bindings: Add pixart vendor
From: Rob Herring @ 2019-08-12 22:46 UTC (permalink / raw)
Cc: robh+dt, mark.rutland, jic23, linux-kernel, linux-iio,
baylibre-upstreaming, dmitry.torokhov, linux-input, devicetree,
Alexandre Mergnat
In-Reply-To: <20190713080455.17513-2-amergnat@baylibre.com>
On Sat, 13 Jul 2019 10:04:53 +0200, Alexandre Mergnat wrote:
> PixArt Imaging Inc. is expertized in CMOS image sensors (CIS),
> capacitive touch controllers and related imaging application development.
>
> Signed-off-by: Alexandre Mergnat <amergnat@baylibre.com>
> ---
> Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
> 1 file changed, 2 insertions(+)
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH v4 7/9] mfd: ioc3: Add driver for SGI IOC3 chip
From: Jakub Kicinski @ 2019-08-12 19:52 UTC (permalink / raw)
To: Thomas Bogendoerfer
Cc: Ralf Baechle, Paul Burton, James Hogan, Dmitry Torokhov,
Lee Jones, David S. Miller, Srinivas Kandagatla, Alessandro Zummo,
Alexandre Belloni, Greg Kroah-Hartman, Jiri Slaby,
Evgeniy Polyakov, linux-mips, linux-kernel, linux-input, netdev,
linux-rtc, linux-serial
In-Reply-To: <20190811093212.88635fb1a6c796a073ec71ff@suse.de>
On Sun, 11 Aug 2019 09:32:12 +0200, Thomas Bogendoerfer wrote:
> > Also please don't use stdint types in the kernel, please try checkpatch
> > to catch coding style issues.
>
> my patch already reduces them and checkpatch only warns about usage of printk
> for the network part. Changing that to dev_warn/dev_err in the mfd patch didn't
> seem the right thing to do. As I'm splitting the conversion patch into a few
> steps I could also replace the printks.
Thanks for looking into it. I was referring to the use of uint32_t
instead of u32. Perhaps checkpatch has to be motivated with the --strict
option to point those out?
^ permalink raw reply
* Re: KMSAN: uninit-value in gtco_probe
From: syzbot @ 2019-08-12 18:21 UTC (permalink / raw)
To: dmitry.torokhov, glider, granthernandez, linux-input,
linux-kernel, syzkaller-bugs
In-Reply-To: <0000000000007a3d3b058fea6016@google.com>
syzbot has found a reproducer for the following crash on:
HEAD commit: 61ccdad1 Revert "drm/bochs: Use shadow buffer for bochs fr..
git tree: kmsan
console output: https://syzkaller.appspot.com/x/log.txt?x=106e8536600000
kernel config: https://syzkaller.appspot.com/x/.config?x=27abc558ecb16a3b
dashboard link: https://syzkaller.appspot.com/bug?extid=bb54195a43a54b1e5e5e
compiler: clang version 9.0.0 (/home/glider/llvm/clang
80fee25776c2fb61e74c1ecb1a523375c2500b69)
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1766194a600000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=13d5879a600000
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+bb54195a43a54b1e5e5e@syzkaller.appspotmail.com
usb 1-1: config 0 has no interface number 0
usb 1-1: New USB device found, idVendor=078c, idProduct=1002,
bcdDevice=e6.47
usb 1-1: New USB device strings: Mfr=0, Product=0, SerialNumber=0
usb 1-1: config 0 descriptor??
gtco 1-1:0.219: Collection level already at zero
==================================================================
BUG: KMSAN: uninit-value in parse_hid_report_descriptor
drivers/input/tablet/gtco.c:297 [inline]
BUG: KMSAN: uninit-value in gtco_probe+0x18c7/0x3520
drivers/input/tablet/gtco.c:938
CPU: 1 PID: 621 Comm: kworker/1:1 Not tainted 5.3.0-rc3+ #17
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Workqueue: usb_hub_wq hub_event
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x191/0x1f0 lib/dump_stack.c:113
kmsan_report+0x162/0x2d0 mm/kmsan/kmsan_report.c:109
__msan_warning+0x75/0xe0 mm/kmsan/kmsan_instr.c:294
parse_hid_report_descriptor drivers/input/tablet/gtco.c:297 [inline]
gtco_probe+0x18c7/0x3520 drivers/input/tablet/gtco.c:938
usb_probe_interface+0xd19/0x1310 drivers/usb/core/driver.c:361
really_probe+0x1373/0x1dc0 drivers/base/dd.c:552
driver_probe_device+0x1ba/0x510 drivers/base/dd.c:709
__device_attach_driver+0x5b8/0x790 drivers/base/dd.c:816
bus_for_each_drv+0x28e/0x3b0 drivers/base/bus.c:454
__device_attach+0x489/0x750 drivers/base/dd.c:882
device_initial_probe+0x4a/0x60 drivers/base/dd.c:929
bus_probe_device+0x131/0x390 drivers/base/bus.c:514
device_add+0x25b5/0x2df0 drivers/base/core.c:2114
usb_set_configuration+0x309f/0x3710 drivers/usb/core/message.c:2027
generic_probe+0xe7/0x280 drivers/usb/core/generic.c:210
usb_probe_device+0x146/0x200 drivers/usb/core/driver.c:266
really_probe+0x1373/0x1dc0 drivers/base/dd.c:552
driver_probe_device+0x1ba/0x510 drivers/base/dd.c:709
__device_attach_driver+0x5b8/0x790 drivers/base/dd.c:816
bus_for_each_drv+0x28e/0x3b0 drivers/base/bus.c:454
__device_attach+0x489/0x750 drivers/base/dd.c:882
device_initial_probe+0x4a/0x60 drivers/base/dd.c:929
bus_probe_device+0x131/0x390 drivers/base/bus.c:514
device_add+0x25b5/0x2df0 drivers/base/core.c:2114
usb_new_device+0x23e5/0x2fb0 drivers/usb/core/hub.c:2536
hub_port_connect drivers/usb/core/hub.c:5098 [inline]
hub_port_connect_change drivers/usb/core/hub.c:5213 [inline]
port_event drivers/usb/core/hub.c:5359 [inline]
hub_event+0x581d/0x72f0 drivers/usb/core/hub.c:5441
process_one_work+0x1572/0x1ef0 kernel/workqueue.c:2269
worker_thread+0x111b/0x2460 kernel/workqueue.c:2415
kthread+0x4b5/0x4f0 kernel/kthread.c:256
ret_from_fork+0x35/0x40 arch/x86/entry/entry_64.S:355
Uninit was stored to memory at:
kmsan_save_stack_with_flags mm/kmsan/kmsan.c:187 [inline]
kmsan_internal_chain_origin+0xcc/0x150 mm/kmsan/kmsan.c:345
__msan_chain_origin+0x6b/0xe0 mm/kmsan/kmsan_instr.c:190
parse_hid_report_descriptor drivers/input/tablet/gtco.c:298 [inline]
gtco_probe+0x1a7c/0x3520 drivers/input/tablet/gtco.c:938
usb_probe_interface+0xd19/0x1310 drivers/usb/core/driver.c:361
really_probe+0x1373/0x1dc0 drivers/base/dd.c:552
driver_probe_device+0x1ba/0x510 drivers/base/dd.c:709
__device_attach_driver+0x5b8/0x790 drivers/base/dd.c:816
bus_for_each_drv+0x28e/0x3b0 drivers/base/bus.c:454
__device_attach+0x489/0x750 drivers/base/dd.c:882
device_initial_probe+0x4a/0x60 drivers/base/dd.c:929
bus_probe_device+0x131/0x390 drivers/base/bus.c:514
device_add+0x25b5/0x2df0 drivers/base/core.c:2114
usb_set_configuration+0x309f/0x3710 drivers/usb/core/message.c:2027
generic_probe+0xe7/0x280 drivers/usb/core/generic.c:210
usb_probe_device+0x146/0x200 drivers/usb/core/driver.c:266
really_probe+0x1373/0x1dc0 drivers/base/dd.c:552
driver_probe_device+0x1ba/0x510 drivers/base/dd.c:709
__device_attach_driver+0x5b8/0x790 drivers/base/dd.c:816
bus_for_each_drv+0x28e/0x3b0 drivers/base/bus.c:454
__device_attach+0x489/0x750 drivers/base/dd.c:882
device_initial_probe+0x4a/0x60 drivers/base/dd.c:929
bus_probe_device+0x131/0x390 drivers/base/bus.c:514
device_add+0x25b5/0x2df0 drivers/base/core.c:2114
usb_new_device+0x23e5/0x2fb0 drivers/usb/core/hub.c:2536
hub_port_connect drivers/usb/core/hub.c:5098 [inline]
hub_port_connect_change drivers/usb/core/hub.c:5213 [inline]
port_event drivers/usb/core/hub.c:5359 [inline]
hub_event+0x581d/0x72f0 drivers/usb/core/hub.c:5441
process_one_work+0x1572/0x1ef0 kernel/workqueue.c:2269
worker_thread+0x111b/0x2460 kernel/workqueue.c:2415
kthread+0x4b5/0x4f0 kernel/kthread.c:256
ret_from_fork+0x35/0x40 arch/x86/entry/entry_64.S:355
Local variable description: ----globalval.i@gtco_probe
Variable was created at:
parse_hid_report_descriptor drivers/input/tablet/gtco.c:221 [inline]
gtco_probe+0xcd6/0x3520 drivers/input/tablet/gtco.c:938
usb_probe_interface+0xd19/0x1310 drivers/usb/core/driver.c:361
==================================================================
^ permalink raw reply
* Re: [PATCH] Input: add support for polling to input devices
From: Dmitry Torokhov @ 2019-08-12 17:11 UTC (permalink / raw)
To: Benjamin Tissoires; +Cc: open list:HID CORE LAYER, lkml, Rob Herring
In-Reply-To: <CAO-hwJL1Jq5XjqV32fD7+_nMpi3PhUbrB5QQ+EEs3N6=mBy-1g@mail.gmail.com>
Hi Benjamin,
On Mon, Aug 12, 2019 at 06:50:38PM +0200, Benjamin Tissoires wrote:
> Hi Dmitry,
>
> On Fri, Aug 9, 2019 at 7:35 PM Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> >
> > Separating "normal" and "polled" input devices was a mistake, as often we
> > want to allow the very same device work on both interrupt-driven and
> > polled mode, depending on the board on which the device is used.
> >
> > This introduces new APIs:
> >
> > - input_setup_polling
> > - input_set_poll_interval
> > - input_set_min_poll_interval
> > - input_set_max_poll_interval
> >
> > These new APIs allow switching an input device into polled mode with sysfs
> > attributes matching drivers using input_polled_dev APIs that will be
> > eventually removed.
>
> Are you sure that using sysfs is the correct way here?
> I would think using generic properties that can be overwritten by the
> DSDT or by the device tree would make things easier from a driver
> point of view.
Couple of points: I wanted it to be compatible with input-polldev.c so
the sysfs attributes must match (so that we can convert existing drivers
and zap input-polldev). I also am not sure if polling parameters are
property of board, or it is either fundamental hardware limitation or
simply desired system behavior. If Rob is OK with adding device
properties I'd be fine adding them as a followup, otherwise we can have
udev adjust the behavior as needed for given box shortly after boot.
>
> Also, checkpatch complains about a few octal permissions that are
> preferred over symbolic ones, and there is a possible 'out of memory'
> nessage at drivers/input/input-poller.c:75.
Yes, there is. It is there so we would know what device we were trying
to set up when OOM happened. You can probable decipher the driver from
the stack trace, but figuring particular device instance is harder.
Thanks.
>
> Cheers,
> Benjamin
>
> >
> > Tested-by: Michal Vokáč <michal.vokac@ysoft.com>
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > ---
> > drivers/input/Makefile | 2 +-
> > drivers/input/input-poller.c | 208 +++++++++++++++++++++++++++++++++++
> > drivers/input/input-poller.h | 18 +++
> > drivers/input/input.c | 36 ++++--
> > include/linux/input.h | 12 ++
> > 5 files changed, 268 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/input/Makefile b/drivers/input/Makefile
> > index 40de6a7be641..e35650930371 100644
> > --- a/drivers/input/Makefile
> > +++ b/drivers/input/Makefile
> > @@ -6,7 +6,7 @@
> > # Each configuration option enables a list of files.
> >
> > obj-$(CONFIG_INPUT) += input-core.o
> > -input-core-y := input.o input-compat.o input-mt.o ff-core.o
> > +input-core-y := input.o input-compat.o input-mt.o input-poller.o ff-core.o
> >
> > obj-$(CONFIG_INPUT_FF_MEMLESS) += ff-memless.o
> > obj-$(CONFIG_INPUT_POLLDEV) += input-polldev.o
> > diff --git a/drivers/input/input-poller.c b/drivers/input/input-poller.c
> > new file mode 100644
> > index 000000000000..e041adb04f5a
> > --- /dev/null
> > +++ b/drivers/input/input-poller.c
> > @@ -0,0 +1,208 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Support for polling mode for input devices.
> > + */
> > +
> > +#include <linux/device.h>
> > +#include <linux/input.h>
> > +#include <linux/jiffies.h>
> > +#include <linux/mutex.h>
> > +#include <linux/slab.h>
> > +#include <linux/types.h>
> > +#include <linux/workqueue.h>
> > +#include "input-poller.h"
> > +
> > +struct input_dev_poller {
> > + void (*poll)(struct input_dev *dev);
> > +
> > + unsigned int poll_interval; /* msec */
> > + unsigned int poll_interval_max; /* msec */
> > + unsigned int poll_interval_min; /* msec */
> > +
> > + struct input_dev *input;
> > + struct delayed_work work;
> > +};
> > +
> > +static void input_dev_poller_queue_work(struct input_dev_poller *poller)
> > +{
> > + unsigned long delay;
> > +
> > + delay = msecs_to_jiffies(poller->poll_interval);
> > + if (delay >= HZ)
> > + delay = round_jiffies_relative(delay);
> > +
> > + queue_delayed_work(system_freezable_wq, &poller->work, delay);
> > +}
> > +
> > +static void input_dev_poller_work(struct work_struct *work)
> > +{
> > + struct input_dev_poller *poller =
> > + container_of(work, struct input_dev_poller, work.work);
> > +
> > + poller->poll(poller->input);
> > + input_dev_poller_queue_work(poller);
> > +}
> > +
> > +void input_dev_poller_finalize(struct input_dev_poller *poller)
> > +{
> > + if (!poller->poll_interval)
> > + poller->poll_interval = 500;
> > + if (!poller->poll_interval_max)
> > + poller->poll_interval_max = poller->poll_interval;
> > +}
> > +
> > +void input_dev_poller_start(struct input_dev_poller *poller)
> > +{
> > + /* Only start polling if polling is enabled */
> > + if (poller->poll_interval > 0) {
> > + poller->poll(poller->input);
> > + input_dev_poller_queue_work(poller);
> > + }
> > +}
> > +
> > +void input_dev_poller_stop(struct input_dev_poller *poller)
> > +{
> > + cancel_delayed_work_sync(&poller->work);
> > +}
> > +
> > +int input_setup_polling(struct input_dev *dev,
> > + void (*poll_fn)(struct input_dev *dev))
> > +{
> > + struct input_dev_poller *poller;
> > +
> > + poller = kzalloc(sizeof(*poller), GFP_KERNEL);
> > + if (!poller) {
> > + dev_err(dev->dev.parent ?: &dev->dev,
> > + "%s: unable to allocate poller structure\n", __func__);
> > + return -ENOMEM;
> > + }
> > +
> > + INIT_DELAYED_WORK(&poller->work, input_dev_poller_work);
> > + poller->input = dev;
> > + poller->poll = poll_fn;
> > +
> > + dev->poller = poller;
> > + return 0;
> > +}
> > +EXPORT_SYMBOL(input_setup_polling);
> > +
> > +static bool input_dev_ensure_poller(struct input_dev *dev)
> > +{
> > + if (!dev->poller) {
> > + dev_err(dev->dev.parent ?: &dev->dev,
> > + "poller structure has not been set up\n");
> > + return false;
> > + }
> > +
> > + return true;
> > +}
> > +
> > +void input_set_poll_interval(struct input_dev *dev, unsigned int interval)
> > +{
> > + if (input_dev_ensure_poller(dev))
> > + dev->poller->poll_interval = interval;
> > +}
> > +EXPORT_SYMBOL(input_set_poll_interval);
> > +
> > +void input_set_min_poll_interval(struct input_dev *dev, unsigned int interval)
> > +{
> > + if (input_dev_ensure_poller(dev))
> > + dev->poller->poll_interval_min = interval;
> > +}
> > +EXPORT_SYMBOL(input_set_min_poll_interval);
> > +
> > +void input_set_max_poll_interval(struct input_dev *dev, unsigned int interval)
> > +{
> > + if (input_dev_ensure_poller(dev))
> > + dev->poller->poll_interval_max = interval;
> > +}
> > +EXPORT_SYMBOL(input_set_max_poll_interval);
> > +
> > +/* SYSFS interface */
> > +
> > +static ssize_t input_dev_get_poll_interval(struct device *dev,
> > + struct device_attribute *attr,
> > + char *buf)
> > +{
> > + struct input_dev *input = to_input_dev(dev);
> > +
> > + return sprintf(buf, "%d\n", input->poller->poll_interval);
> > +}
> > +
> > +static ssize_t input_dev_set_poll_interval(struct device *dev,
> > + struct device_attribute *attr,
> > + const char *buf, size_t count)
> > +{
> > + struct input_dev *input = to_input_dev(dev);
> > + struct input_dev_poller *poller = input->poller;
> > + unsigned int interval;
> > + int err;
> > +
> > + err = kstrtouint(buf, 0, &interval);
> > + if (err)
> > + return err;
> > +
> > + if (interval < poller->poll_interval_min)
> > + return -EINVAL;
> > +
> > + if (interval > poller->poll_interval_max)
> > + return -EINVAL;
> > +
> > + mutex_lock(&input->mutex);
> > +
> > + poller->poll_interval = interval;
> > +
> > + if (input->users) {
> > + cancel_delayed_work_sync(&poller->work);
> > + if (poller->poll_interval > 0)
> > + input_dev_poller_queue_work(poller);
> > + }
> > +
> > + mutex_unlock(&input->mutex);
> > +
> > + return count;
> > +}
> > +
> > +static DEVICE_ATTR(poll, S_IRUGO | S_IWUSR,
> > + input_dev_get_poll_interval, input_dev_set_poll_interval);
> > +
> > +static ssize_t input_dev_get_poll_max(struct device *dev,
> > + struct device_attribute *attr, char *buf)
> > +{
> > + struct input_dev *input = to_input_dev(dev);
> > +
> > + return sprintf(buf, "%d\n", input->poller->poll_interval_max);
> > +}
> > +
> > +static DEVICE_ATTR(max, S_IRUGO, input_dev_get_poll_max, NULL);
> > +
> > +static ssize_t input_dev_get_poll_min(struct device *dev,
> > + struct device_attribute *attr, char *buf)
> > +{
> > + struct input_dev *input = to_input_dev(dev);
> > +
> > + return sprintf(buf, "%d\n", input->poller->poll_interval_min);
> > +}
> > +
> > +static DEVICE_ATTR(min, S_IRUGO, input_dev_get_poll_min, NULL);
> > +
> > +static umode_t input_poller_attrs_visible(struct kobject *kobj,
> > + struct attribute *attr, int n)
> > +{
> > + struct device *dev = kobj_to_dev(kobj);
> > + struct input_dev *input = to_input_dev(dev);
> > +
> > + return input->poller ? attr->mode : 0;
> > +}
> > +
> > +static struct attribute *input_poller_attrs[] = {
> > + &dev_attr_poll.attr,
> > + &dev_attr_max.attr,
> > + &dev_attr_min.attr,
> > + NULL
> > +};
> > +
> > +struct attribute_group input_poller_attribute_group = {
> > + .is_visible = input_poller_attrs_visible,
> > + .attrs = input_poller_attrs,
> > +};
> > diff --git a/drivers/input/input-poller.h b/drivers/input/input-poller.h
> > new file mode 100644
> > index 000000000000..e3fca0be1d32
> > --- /dev/null
> > +++ b/drivers/input/input-poller.h
> > @@ -0,0 +1,18 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +#ifndef _INPUT_POLLER_H
> > +#define _INPUT_POLLER_H
> > +
> > +/*
> > + * Support for polling mode for input devices.
> > + */
> > +#include <linux/sysfs.h>
> > +
> > +struct input_dev_poller;
> > +
> > +void input_dev_poller_finalize(struct input_dev_poller *poller);
> > +void input_dev_poller_start(struct input_dev_poller *poller);
> > +void input_dev_poller_stop(struct input_dev_poller *poller);
> > +
> > +extern struct attribute_group input_poller_attribute_group;
> > +
> > +#endif /* _INPUT_POLLER_H */
> > diff --git a/drivers/input/input.c b/drivers/input/input.c
> > index 7494a0dede79..c08aa3596144 100644
> > --- a/drivers/input/input.c
> > +++ b/drivers/input/input.c
> > @@ -24,6 +24,7 @@
> > #include <linux/mutex.h>
> > #include <linux/rcupdate.h>
> > #include "input-compat.h"
> > +#include "input-poller.h"
> >
> > MODULE_AUTHOR("Vojtech Pavlik <vojtech@suse.cz>");
> > MODULE_DESCRIPTION("Input core");
> > @@ -603,20 +604,31 @@ int input_open_device(struct input_handle *handle)
> >
> > handle->open++;
> >
> > - if (!dev->users++ && dev->open)
> > - retval = dev->open(dev);
> > + if (dev->users++) {
> > + /*
> > + * Device is already opened, so we can exit immediately and
> > + * report success.
> > + */
> > + goto out;
> > + }
> >
> > - if (retval) {
> > - dev->users--;
> > - if (!--handle->open) {
> > + if (dev->open) {
> > + retval = dev->open(dev);
> > + if (retval) {
> > + dev->users--;
> > + handle->open--;
> > /*
> > * Make sure we are not delivering any more events
> > * through this handle
> > */
> > synchronize_rcu();
> > + goto out;
> > }
> > }
> >
> > + if (dev->poller)
> > + input_dev_poller_start(dev->poller);
> > +
> > out:
> > mutex_unlock(&dev->mutex);
> > return retval;
> > @@ -655,8 +667,13 @@ void input_close_device(struct input_handle *handle)
> >
> > __input_release_device(handle);
> >
> > - if (!--dev->users && dev->close)
> > - dev->close(dev);
> > + if (!--dev->users) {
> > + if (dev->poller)
> > + input_dev_poller_stop(dev->poller);
> > +
> > + if (dev->close)
> > + dev->close(dev);
> > + }
> >
> > if (!--handle->open) {
> > /*
> > @@ -1502,6 +1519,7 @@ static const struct attribute_group *input_dev_attr_groups[] = {
> > &input_dev_attr_group,
> > &input_dev_id_attr_group,
> > &input_dev_caps_attr_group,
> > + &input_poller_attribute_group,
> > NULL
> > };
> >
> > @@ -1511,6 +1529,7 @@ static void input_dev_release(struct device *device)
> >
> > input_ff_destroy(dev);
> > input_mt_destroy_slots(dev);
> > + kfree(dev->poller);
> > kfree(dev->absinfo);
> > kfree(dev->vals);
> > kfree(dev);
> > @@ -2175,6 +2194,9 @@ int input_register_device(struct input_dev *dev)
> > if (!dev->setkeycode)
> > dev->setkeycode = input_default_setkeycode;
> >
> > + if (dev->poller)
> > + input_dev_poller_finalize(dev->poller);
> > +
> > error = device_add(&dev->dev);
> > if (error)
> > goto err_free_vals;
> > diff --git a/include/linux/input.h b/include/linux/input.h
> > index e95a439d8bd5..94f277cd806a 100644
> > --- a/include/linux/input.h
> > +++ b/include/linux/input.h
> > @@ -21,6 +21,8 @@
> > #include <linux/timer.h>
> > #include <linux/mod_devicetable.h>
> >
> > +struct input_dev_poller;
> > +
> > /**
> > * struct input_value - input value representation
> > * @type: type of value (EV_KEY, EV_ABS, etc)
> > @@ -71,6 +73,8 @@ enum input_clock_type {
> > * not sleep
> > * @ff: force feedback structure associated with the device if device
> > * supports force feedback effects
> > + * @poller: poller structure associated with the device if device is
> > + * set up to use polling mode
> > * @repeat_key: stores key code of the last key pressed; used to implement
> > * software autorepeat
> > * @timer: timer for software autorepeat
> > @@ -156,6 +160,8 @@ struct input_dev {
> >
> > struct ff_device *ff;
> >
> > + struct input_dev_poller *poller;
> > +
> > unsigned int repeat_key;
> > struct timer_list timer;
> >
> > @@ -372,6 +378,12 @@ void input_unregister_device(struct input_dev *);
> >
> > void input_reset_device(struct input_dev *);
> >
> > +int input_setup_polling(struct input_dev *dev,
> > + void (*poll_fn)(struct input_dev *dev));
> > +void input_set_poll_interval(struct input_dev *dev, unsigned int interval);
> > +void input_set_min_poll_interval(struct input_dev *dev, unsigned int interval);
> > +void input_set_max_poll_interval(struct input_dev *dev, unsigned int interval);
> > +
> > int __must_check input_register_handler(struct input_handler *);
> > void input_unregister_handler(struct input_handler *);
> >
> > --
> > 2.23.0.rc1.153.gdeed80330f-goog
> >
> >
> > --
> > Dmitry
--
Dmitry
^ permalink raw reply
* Re: [PATCH] HID: apple: Fix stuck function keys when using FN
From: Benjamin Tissoires @ 2019-08-12 16:57 UTC (permalink / raw)
To: João Moreno
Cc: Benjamin Tissoires, Jiri Kosina, open list:HID CORE LAYER, lkml
In-Reply-To: <CAHxFc3TjZu7_u0U3ZoB466WGNzbfYLe4ZB7C4LuKdBAwkRum5Q@mail.gmail.com>
Hi João,
On Thu, Aug 8, 2019 at 10:35 PM João Moreno <mail@joaomoreno.com> wrote:
>
> Hi Benjamin,
>
> On Mon, 8 Jul 2019 at 22:35, João Moreno <mail@joaomoreno.com> wrote:
> >
> > Hi Benjamin,
> >
> > No worries, also pretty busy over here. Didn't mean to press.
> >
> > On Mon, 1 Jul 2019 at 10:32, Benjamin Tissoires
> > <benjamin.tissoires@redhat.com> wrote:
> > >
> > > Hi João,
> > >
> > > On Sun, Jun 30, 2019 at 10:15 PM João Moreno <mail@joaomoreno.com> wrote:
> > > >
> > > > Hi Jiri & Benjamin,
> > > >
> > > > Let me know if you need something else to get this patch moving forward. This
> > > > fixes an issue I hit daily, it would be great to get it fixed.
> > >
> > > Sorry for the delay, I am very busy with internal corporate stuff, and
> > > I tried setting up a new CI system at home, and instead of spending a
> > > couple of ours, I am down to 2 weeks of hard work, without possibility
> > > to switch to the new right now :(
> > > Anyway.
> > >
> > > >
> > > > Thanks.
> > > >
> > > > On Mon, 10 Jun 2019 at 23:31, Joao Moreno <mail@joaomoreno.com> wrote:
> > > > >
> > > > > This fixes an issue in which key down events for function keys would be
> > > > > repeatedly emitted even after the user has raised the physical key. For
> > > > > example, the driver fails to emit the F5 key up event when going through
> > > > > the following steps:
> > > > > - fnmode=1: hold FN, hold F5, release FN, release F5
> > > > > - fnmode=2: hold F5, hold FN, release F5, release FN
> > >
> > > Ouch :/
> > >
> >
> > Right?!
> >
> > > > >
> > > > > The repeated F5 key down events can be easily verified using xev.
> > > > >
> > > > > Signed-off-by: Joao Moreno <mail@joaomoreno.com>
> > > > > ---
> > > > > drivers/hid/hid-apple.c | 21 +++++++++++----------
> > > > > 1 file changed, 11 insertions(+), 10 deletions(-)
> > > > >
> > > > > diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c
> > > > > index 1cb41992aaa1..81867a6fa047 100644
> > > > > --- a/drivers/hid/hid-apple.c
> > > > > +++ b/drivers/hid/hid-apple.c
> > > > > @@ -205,20 +205,21 @@ static int hidinput_apple_event(struct hid_device *hid, struct input_dev *input,
> > > > > trans = apple_find_translation (table, usage->code);
> > > > >
> > > > > if (trans) {
> > > > > - if (test_bit(usage->code, asc->pressed_fn))
> > > > > - do_translate = 1;
> > > > > - else if (trans->flags & APPLE_FLAG_FKEY)
> > > > > - do_translate = (fnmode == 2 && asc->fn_on) ||
> > > > > - (fnmode == 1 && !asc->fn_on);
> > > > > + int fn_on = value ? asc->fn_on :
> > > > > + test_bit(usage->code, asc->pressed_fn);
> > > > > +
> > > > > + if (!value)
> > > > > + clear_bit(usage->code, asc->pressed_fn);
> > > > > + else if (asc->fn_on)
> > > > > + set_bit(usage->code, asc->pressed_fn);
> > >
> > > I have the feeling that this is not the correct fix here.
> > >
> > > I might be wrong, but the following sequence might also mess up the
> > > driver state, depending on how the reports are emitted:
> > > - hold FN, hold F4, hold F5, release F4, release FN, release F5
> > >
> >
> > I believe this should be fine. Following the code:
> >
> > - hold FN, sets asc->fn_on to true
> > - hold F4, in the trans block fn_on will be true and we'll set the F4
> > bit in the bitmap
> > - hold F5, in the trans block fn_on will be true and we'll set the F5 bit
> > - release F4, in the trans block fn_on will be true (because of the bitmap) and
> > we'll clear the F4 bit
> > - release FN, asc->fn_on will be false, but it doesn't matter since...
> > - release F5, in the trans block we'll look into the bitmap (instead
> > of asc->fn_on),
> > so fn_on will be true and we'll clear the F5 bit
> >
> > I tested it in practice using my changes:
> >
> > Interestingly the Apple keyboard doesn't seem to emit an even for F5 when F4 is
> > pressed, seems like a hardware limitation. But F6 does work. So, when I execute
> > these events in that order, everything works as it should: xev reports
> > the following:
> >
> > KeyPress F4
> > KeyPress F6
> > KeyRelease F4
> > KeyRelease F6
> >
> > > The reason is that the driver only considers you have one key pressed
> > > with the modifier, and as the code changed its state based on the last
> > > value.
> > >
> >
> > I believe the bitmap takes care of storing the FN state per key press. The
> > trick I did was to check on the global `asc->fn_on` state only when a key
> > is pressed, but check on the bitmap instead when it's released.
> >
> > Let me know what you think. Am I missing something here?
> >
> > Cheers,
> > João.
> >
> > > IMO a better fix would:
> > >
> > > - keep the existing `trans` mapping lookout
> > > - whenever a `trans` mapping gets found:
> > > * get both translated and non-translated currently reported values
> > > (`test_bit(keycode, input_dev->key)`)
> > > * if one of them is set to true, then consider the keycode to be the
> > > one of the key (no matter fn_on)
> > > -> deal with `value` with the corrected keycode
> > > * if the key was not pressed:
> > > -> chose the keycode based on `fn_on` and `fnmode` states
> > > and report the key press event
> > >
> > > This should remove the nasty pressed_fn state which depends on the
> > > other pressed keys.
> > >
> > > Cheers,
> > > Benjamin
> > >
> > > > > +
> > > > > + if (trans->flags & APPLE_FLAG_FKEY)
> > > > > + do_translate = (fnmode == 2 && fn_on) ||
> > > > > + (fnmode == 1 && !fn_on);
> > > > > else
> > > > > do_translate = asc->fn_on;
> > > > >
> > > > > if (do_translate) {
> > > > > - if (value)
> > > > > - set_bit(usage->code, asc->pressed_fn);
> > > > > - else
> > > > > - clear_bit(usage->code, asc->pressed_fn);
> > > > > -
> > > > > input_event(input, usage->type, trans->to,
> > > > > value);
> > > > >
> > > > > --
> > > > > 2.19.1
> > > > >
>
> Gave this another look and I still haven't found any issues, let me
> know if you still
> think I'm missing something. Thanks!
>
OK, I am tempted to take the patch, but I'll need to add this as a
regression test in my test-suite. This is too complex that it can
easily break next time someone looks at it.
Can you send me the report descriptors of the device using
hid-recorder from hid-tools[0].
Ideally, if you can put the faulty sequence in the logs, that would
help writing down the use case.
Cheers,
Benjamin
[0] https://gitlab.freedesktop.org/libevdev/hid-tools
^ permalink raw reply
* Re: [PATCH] Input: add support for polling to input devices
From: Benjamin Tissoires @ 2019-08-12 16:50 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: open list:HID CORE LAYER, lkml
In-Reply-To: <20190809173548.GA32693@dtor-ws>
Hi Dmitry,
On Fri, Aug 9, 2019 at 7:35 PM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>
> Separating "normal" and "polled" input devices was a mistake, as often we
> want to allow the very same device work on both interrupt-driven and
> polled mode, depending on the board on which the device is used.
>
> This introduces new APIs:
>
> - input_setup_polling
> - input_set_poll_interval
> - input_set_min_poll_interval
> - input_set_max_poll_interval
>
> These new APIs allow switching an input device into polled mode with sysfs
> attributes matching drivers using input_polled_dev APIs that will be
> eventually removed.
Are you sure that using sysfs is the correct way here?
I would think using generic properties that can be overwritten by the
DSDT or by the device tree would make things easier from a driver
point of view.
Also, checkpatch complains about a few octal permissions that are
preferred over symbolic ones, and there is a possible 'out of memory'
nessage at drivers/input/input-poller.c:75.
Cheers,
Benjamin
>
> Tested-by: Michal Vokáč <michal.vokac@ysoft.com>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
> drivers/input/Makefile | 2 +-
> drivers/input/input-poller.c | 208 +++++++++++++++++++++++++++++++++++
> drivers/input/input-poller.h | 18 +++
> drivers/input/input.c | 36 ++++--
> include/linux/input.h | 12 ++
> 5 files changed, 268 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/input/Makefile b/drivers/input/Makefile
> index 40de6a7be641..e35650930371 100644
> --- a/drivers/input/Makefile
> +++ b/drivers/input/Makefile
> @@ -6,7 +6,7 @@
> # Each configuration option enables a list of files.
>
> obj-$(CONFIG_INPUT) += input-core.o
> -input-core-y := input.o input-compat.o input-mt.o ff-core.o
> +input-core-y := input.o input-compat.o input-mt.o input-poller.o ff-core.o
>
> obj-$(CONFIG_INPUT_FF_MEMLESS) += ff-memless.o
> obj-$(CONFIG_INPUT_POLLDEV) += input-polldev.o
> diff --git a/drivers/input/input-poller.c b/drivers/input/input-poller.c
> new file mode 100644
> index 000000000000..e041adb04f5a
> --- /dev/null
> +++ b/drivers/input/input-poller.c
> @@ -0,0 +1,208 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Support for polling mode for input devices.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/input.h>
> +#include <linux/jiffies.h>
> +#include <linux/mutex.h>
> +#include <linux/slab.h>
> +#include <linux/types.h>
> +#include <linux/workqueue.h>
> +#include "input-poller.h"
> +
> +struct input_dev_poller {
> + void (*poll)(struct input_dev *dev);
> +
> + unsigned int poll_interval; /* msec */
> + unsigned int poll_interval_max; /* msec */
> + unsigned int poll_interval_min; /* msec */
> +
> + struct input_dev *input;
> + struct delayed_work work;
> +};
> +
> +static void input_dev_poller_queue_work(struct input_dev_poller *poller)
> +{
> + unsigned long delay;
> +
> + delay = msecs_to_jiffies(poller->poll_interval);
> + if (delay >= HZ)
> + delay = round_jiffies_relative(delay);
> +
> + queue_delayed_work(system_freezable_wq, &poller->work, delay);
> +}
> +
> +static void input_dev_poller_work(struct work_struct *work)
> +{
> + struct input_dev_poller *poller =
> + container_of(work, struct input_dev_poller, work.work);
> +
> + poller->poll(poller->input);
> + input_dev_poller_queue_work(poller);
> +}
> +
> +void input_dev_poller_finalize(struct input_dev_poller *poller)
> +{
> + if (!poller->poll_interval)
> + poller->poll_interval = 500;
> + if (!poller->poll_interval_max)
> + poller->poll_interval_max = poller->poll_interval;
> +}
> +
> +void input_dev_poller_start(struct input_dev_poller *poller)
> +{
> + /* Only start polling if polling is enabled */
> + if (poller->poll_interval > 0) {
> + poller->poll(poller->input);
> + input_dev_poller_queue_work(poller);
> + }
> +}
> +
> +void input_dev_poller_stop(struct input_dev_poller *poller)
> +{
> + cancel_delayed_work_sync(&poller->work);
> +}
> +
> +int input_setup_polling(struct input_dev *dev,
> + void (*poll_fn)(struct input_dev *dev))
> +{
> + struct input_dev_poller *poller;
> +
> + poller = kzalloc(sizeof(*poller), GFP_KERNEL);
> + if (!poller) {
> + dev_err(dev->dev.parent ?: &dev->dev,
> + "%s: unable to allocate poller structure\n", __func__);
> + return -ENOMEM;
> + }
> +
> + INIT_DELAYED_WORK(&poller->work, input_dev_poller_work);
> + poller->input = dev;
> + poller->poll = poll_fn;
> +
> + dev->poller = poller;
> + return 0;
> +}
> +EXPORT_SYMBOL(input_setup_polling);
> +
> +static bool input_dev_ensure_poller(struct input_dev *dev)
> +{
> + if (!dev->poller) {
> + dev_err(dev->dev.parent ?: &dev->dev,
> + "poller structure has not been set up\n");
> + return false;
> + }
> +
> + return true;
> +}
> +
> +void input_set_poll_interval(struct input_dev *dev, unsigned int interval)
> +{
> + if (input_dev_ensure_poller(dev))
> + dev->poller->poll_interval = interval;
> +}
> +EXPORT_SYMBOL(input_set_poll_interval);
> +
> +void input_set_min_poll_interval(struct input_dev *dev, unsigned int interval)
> +{
> + if (input_dev_ensure_poller(dev))
> + dev->poller->poll_interval_min = interval;
> +}
> +EXPORT_SYMBOL(input_set_min_poll_interval);
> +
> +void input_set_max_poll_interval(struct input_dev *dev, unsigned int interval)
> +{
> + if (input_dev_ensure_poller(dev))
> + dev->poller->poll_interval_max = interval;
> +}
> +EXPORT_SYMBOL(input_set_max_poll_interval);
> +
> +/* SYSFS interface */
> +
> +static ssize_t input_dev_get_poll_interval(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct input_dev *input = to_input_dev(dev);
> +
> + return sprintf(buf, "%d\n", input->poller->poll_interval);
> +}
> +
> +static ssize_t input_dev_set_poll_interval(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct input_dev *input = to_input_dev(dev);
> + struct input_dev_poller *poller = input->poller;
> + unsigned int interval;
> + int err;
> +
> + err = kstrtouint(buf, 0, &interval);
> + if (err)
> + return err;
> +
> + if (interval < poller->poll_interval_min)
> + return -EINVAL;
> +
> + if (interval > poller->poll_interval_max)
> + return -EINVAL;
> +
> + mutex_lock(&input->mutex);
> +
> + poller->poll_interval = interval;
> +
> + if (input->users) {
> + cancel_delayed_work_sync(&poller->work);
> + if (poller->poll_interval > 0)
> + input_dev_poller_queue_work(poller);
> + }
> +
> + mutex_unlock(&input->mutex);
> +
> + return count;
> +}
> +
> +static DEVICE_ATTR(poll, S_IRUGO | S_IWUSR,
> + input_dev_get_poll_interval, input_dev_set_poll_interval);
> +
> +static ssize_t input_dev_get_poll_max(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct input_dev *input = to_input_dev(dev);
> +
> + return sprintf(buf, "%d\n", input->poller->poll_interval_max);
> +}
> +
> +static DEVICE_ATTR(max, S_IRUGO, input_dev_get_poll_max, NULL);
> +
> +static ssize_t input_dev_get_poll_min(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct input_dev *input = to_input_dev(dev);
> +
> + return sprintf(buf, "%d\n", input->poller->poll_interval_min);
> +}
> +
> +static DEVICE_ATTR(min, S_IRUGO, input_dev_get_poll_min, NULL);
> +
> +static umode_t input_poller_attrs_visible(struct kobject *kobj,
> + struct attribute *attr, int n)
> +{
> + struct device *dev = kobj_to_dev(kobj);
> + struct input_dev *input = to_input_dev(dev);
> +
> + return input->poller ? attr->mode : 0;
> +}
> +
> +static struct attribute *input_poller_attrs[] = {
> + &dev_attr_poll.attr,
> + &dev_attr_max.attr,
> + &dev_attr_min.attr,
> + NULL
> +};
> +
> +struct attribute_group input_poller_attribute_group = {
> + .is_visible = input_poller_attrs_visible,
> + .attrs = input_poller_attrs,
> +};
> diff --git a/drivers/input/input-poller.h b/drivers/input/input-poller.h
> new file mode 100644
> index 000000000000..e3fca0be1d32
> --- /dev/null
> +++ b/drivers/input/input-poller.h
> @@ -0,0 +1,18 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +#ifndef _INPUT_POLLER_H
> +#define _INPUT_POLLER_H
> +
> +/*
> + * Support for polling mode for input devices.
> + */
> +#include <linux/sysfs.h>
> +
> +struct input_dev_poller;
> +
> +void input_dev_poller_finalize(struct input_dev_poller *poller);
> +void input_dev_poller_start(struct input_dev_poller *poller);
> +void input_dev_poller_stop(struct input_dev_poller *poller);
> +
> +extern struct attribute_group input_poller_attribute_group;
> +
> +#endif /* _INPUT_POLLER_H */
> diff --git a/drivers/input/input.c b/drivers/input/input.c
> index 7494a0dede79..c08aa3596144 100644
> --- a/drivers/input/input.c
> +++ b/drivers/input/input.c
> @@ -24,6 +24,7 @@
> #include <linux/mutex.h>
> #include <linux/rcupdate.h>
> #include "input-compat.h"
> +#include "input-poller.h"
>
> MODULE_AUTHOR("Vojtech Pavlik <vojtech@suse.cz>");
> MODULE_DESCRIPTION("Input core");
> @@ -603,20 +604,31 @@ int input_open_device(struct input_handle *handle)
>
> handle->open++;
>
> - if (!dev->users++ && dev->open)
> - retval = dev->open(dev);
> + if (dev->users++) {
> + /*
> + * Device is already opened, so we can exit immediately and
> + * report success.
> + */
> + goto out;
> + }
>
> - if (retval) {
> - dev->users--;
> - if (!--handle->open) {
> + if (dev->open) {
> + retval = dev->open(dev);
> + if (retval) {
> + dev->users--;
> + handle->open--;
> /*
> * Make sure we are not delivering any more events
> * through this handle
> */
> synchronize_rcu();
> + goto out;
> }
> }
>
> + if (dev->poller)
> + input_dev_poller_start(dev->poller);
> +
> out:
> mutex_unlock(&dev->mutex);
> return retval;
> @@ -655,8 +667,13 @@ void input_close_device(struct input_handle *handle)
>
> __input_release_device(handle);
>
> - if (!--dev->users && dev->close)
> - dev->close(dev);
> + if (!--dev->users) {
> + if (dev->poller)
> + input_dev_poller_stop(dev->poller);
> +
> + if (dev->close)
> + dev->close(dev);
> + }
>
> if (!--handle->open) {
> /*
> @@ -1502,6 +1519,7 @@ static const struct attribute_group *input_dev_attr_groups[] = {
> &input_dev_attr_group,
> &input_dev_id_attr_group,
> &input_dev_caps_attr_group,
> + &input_poller_attribute_group,
> NULL
> };
>
> @@ -1511,6 +1529,7 @@ static void input_dev_release(struct device *device)
>
> input_ff_destroy(dev);
> input_mt_destroy_slots(dev);
> + kfree(dev->poller);
> kfree(dev->absinfo);
> kfree(dev->vals);
> kfree(dev);
> @@ -2175,6 +2194,9 @@ int input_register_device(struct input_dev *dev)
> if (!dev->setkeycode)
> dev->setkeycode = input_default_setkeycode;
>
> + if (dev->poller)
> + input_dev_poller_finalize(dev->poller);
> +
> error = device_add(&dev->dev);
> if (error)
> goto err_free_vals;
> diff --git a/include/linux/input.h b/include/linux/input.h
> index e95a439d8bd5..94f277cd806a 100644
> --- a/include/linux/input.h
> +++ b/include/linux/input.h
> @@ -21,6 +21,8 @@
> #include <linux/timer.h>
> #include <linux/mod_devicetable.h>
>
> +struct input_dev_poller;
> +
> /**
> * struct input_value - input value representation
> * @type: type of value (EV_KEY, EV_ABS, etc)
> @@ -71,6 +73,8 @@ enum input_clock_type {
> * not sleep
> * @ff: force feedback structure associated with the device if device
> * supports force feedback effects
> + * @poller: poller structure associated with the device if device is
> + * set up to use polling mode
> * @repeat_key: stores key code of the last key pressed; used to implement
> * software autorepeat
> * @timer: timer for software autorepeat
> @@ -156,6 +160,8 @@ struct input_dev {
>
> struct ff_device *ff;
>
> + struct input_dev_poller *poller;
> +
> unsigned int repeat_key;
> struct timer_list timer;
>
> @@ -372,6 +378,12 @@ void input_unregister_device(struct input_dev *);
>
> void input_reset_device(struct input_dev *);
>
> +int input_setup_polling(struct input_dev *dev,
> + void (*poll_fn)(struct input_dev *dev));
> +void input_set_poll_interval(struct input_dev *dev, unsigned int interval);
> +void input_set_min_poll_interval(struct input_dev *dev, unsigned int interval);
> +void input_set_max_poll_interval(struct input_dev *dev, unsigned int interval);
> +
> int __must_check input_register_handler(struct input_handler *);
> void input_unregister_handler(struct input_handler *);
>
> --
> 2.23.0.rc1.153.gdeed80330f-goog
>
>
> --
> Dmitry
^ permalink raw reply
* Re: [PATCH 1/2] HID: do not call hid_set_drvdata(hdev, NULL) in drivers
From: Srinivas Pandruvada @ 2019-08-12 16:39 UTC (permalink / raw)
To: Benjamin Tissoires, Bruno Prémont, Jonathan Cameron,
Ping Cheng, Jason Gerecke, Jiri Kosina
Cc: linux-input, linux-kernel
In-Reply-To: <20190812162740.15898-2-benjamin.tissoires@redhat.com>
On Mon, 2019-08-12 at 18:27 +0200, Benjamin Tissoires wrote:
> This is a common pattern in the HID drivers to reset the drvdata.
> Some
> do it properly, some do it only in case of failure.
>
> But, this is actually already handled by driver core, so there is no
> need
> to do it manually.
>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
for hid-sensor-hub.c
Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> ---
> drivers/hid/hid-cougar.c | 6 ++----
> drivers/hid/hid-gfrm.c | 7 -------
> drivers/hid/hid-lenovo.c | 2 --
> drivers/hid/hid-picolcd_core.c | 7 +------
> drivers/hid/hid-sensor-hub.c | 1 -
> 5 files changed, 3 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/hid/hid-cougar.c b/drivers/hid/hid-cougar.c
> index e0bb7b34f3a4..4ff3bc1d25e2 100644
> --- a/drivers/hid/hid-cougar.c
> +++ b/drivers/hid/hid-cougar.c
> @@ -207,7 +207,7 @@ static int cougar_probe(struct hid_device *hdev,
> error = hid_parse(hdev);
> if (error) {
> hid_err(hdev, "parse failed\n");
> - goto fail;
> + return error;
> }
>
> if (hdev->collection->usage == COUGAR_VENDOR_USAGE) {
> @@ -219,7 +219,7 @@ static int cougar_probe(struct hid_device *hdev,
> error = hid_hw_start(hdev, connect_mask);
> if (error) {
> hid_err(hdev, "hw start failed\n");
> - goto fail;
> + return error;
> }
>
> error = cougar_bind_shared_data(hdev, cougar);
> @@ -249,8 +249,6 @@ static int cougar_probe(struct hid_device *hdev,
>
> fail_stop_and_cleanup:
> hid_hw_stop(hdev);
> -fail:
> - hid_set_drvdata(hdev, NULL);
> return error;
> }
>
> diff --git a/drivers/hid/hid-gfrm.c b/drivers/hid/hid-gfrm.c
> index 86c317320bf2..699186ff2349 100644
> --- a/drivers/hid/hid-gfrm.c
> +++ b/drivers/hid/hid-gfrm.c
> @@ -123,12 +123,6 @@ static int gfrm_probe(struct hid_device *hdev,
> const struct hid_device_id *id)
> return ret;
> }
>
> -static void gfrm_remove(struct hid_device *hdev)
> -{
> - hid_hw_stop(hdev);
> - hid_set_drvdata(hdev, NULL);
> -}
> -
> static const struct hid_device_id gfrm_devices[] = {
> { HID_BLUETOOTH_DEVICE(0x58, 0x2000),
> .driver_data = GFRM100 },
> @@ -142,7 +136,6 @@ static struct hid_driver gfrm_driver = {
> .name = "gfrm",
> .id_table = gfrm_devices,
> .probe = gfrm_probe,
> - .remove = gfrm_remove,
> .input_mapping = gfrm_input_mapping,
> .raw_event = gfrm_raw_event,
> .input_configured = gfrm_input_configured,
> diff --git a/drivers/hid/hid-lenovo.c b/drivers/hid/hid-lenovo.c
> index 364bc7f11d9d..96fa2a2c2cd3 100644
> --- a/drivers/hid/hid-lenovo.c
> +++ b/drivers/hid/hid-lenovo.c
> @@ -866,8 +866,6 @@ static void lenovo_remove_tpkbd(struct hid_device
> *hdev)
>
> led_classdev_unregister(&data_pointer->led_micmute);
> led_classdev_unregister(&data_pointer->led_mute);
> -
> - hid_set_drvdata(hdev, NULL);
> }
>
> static void lenovo_remove_cptkbd(struct hid_device *hdev)
> diff --git a/drivers/hid/hid-picolcd_core.c b/drivers/hid/hid-
> picolcd_core.c
> index 5f7a39a5d4af..1b5c63241af0 100644
> --- a/drivers/hid/hid-picolcd_core.c
> +++ b/drivers/hid/hid-picolcd_core.c
> @@ -534,8 +534,7 @@ static int picolcd_probe(struct hid_device *hdev,
> data = kzalloc(sizeof(struct picolcd_data), GFP_KERNEL);
> if (data == NULL) {
> hid_err(hdev, "can't allocate space for Minibox PicoLCD
> device data\n");
> - error = -ENOMEM;
> - goto err_no_cleanup;
> + return -ENOMEM;
> }
>
> spin_lock_init(&data->lock);
> @@ -597,9 +596,6 @@ static int picolcd_probe(struct hid_device *hdev,
> hid_hw_stop(hdev);
> err_cleanup_data:
> kfree(data);
> -err_no_cleanup:
> - hid_set_drvdata(hdev, NULL);
> -
> return error;
> }
>
> @@ -635,7 +631,6 @@ static void picolcd_remove(struct hid_device
> *hdev)
> picolcd_exit_cir(data);
> picolcd_exit_keys(data);
>
> - hid_set_drvdata(hdev, NULL);
> mutex_destroy(&data->mutex);
> /* Finally, clean up the picolcd data itself */
> kfree(data);
> diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-
> hub.c
> index be92a6f79687..94c7398b5c27 100644
> --- a/drivers/hid/hid-sensor-hub.c
> +++ b/drivers/hid/hid-sensor-hub.c
> @@ -742,7 +742,6 @@ static void sensor_hub_remove(struct hid_device
> *hdev)
> }
> spin_unlock_irqrestore(&data->lock, flags);
> mfd_remove_devices(&hdev->dev);
> - hid_set_drvdata(hdev, NULL);
> mutex_destroy(&data->mutex);
> }
>
^ permalink raw reply
* [PATCH 2/2] HID: wacom: do not call hid_set_drvdata(hdev, NULL)
From: Benjamin Tissoires @ 2019-08-12 16:27 UTC (permalink / raw)
To: Bruno Prémont, Jonathan Cameron, Srinivas Pandruvada,
Ping Cheng, Jason Gerecke, Jiri Kosina
Cc: linux-input, linux-kernel, Benjamin Tissoires
In-Reply-To: <20190812162740.15898-1-benjamin.tissoires@redhat.com>
This is a common pattern in the HID drivers to reset the drvdata.
However, this is actually already handled by driver core, so there
is no need to do it manually.
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
drivers/hid/wacom_sys.c | 18 +++++-------------
1 file changed, 5 insertions(+), 13 deletions(-)
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 53bddb50aeba..69ccfdd51a6f 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -2718,14 +2718,12 @@ static int wacom_probe(struct hid_device *hdev,
wacom_wac->features = *((struct wacom_features *)id->driver_data);
features = &wacom_wac->features;
- if (features->check_for_hid_type && features->hid_type != hdev->type) {
- error = -ENODEV;
- goto fail;
- }
+ if (features->check_for_hid_type && features->hid_type != hdev->type)
+ return -ENODEV;
error = kfifo_alloc(&wacom_wac->pen_fifo, WACOM_PKGLEN_MAX, GFP_KERNEL);
if (error)
- goto fail;
+ return error;
wacom_wac->hid_data.inputmode = -1;
wacom_wac->mode_report = -1;
@@ -2743,12 +2741,12 @@ static int wacom_probe(struct hid_device *hdev,
error = hid_parse(hdev);
if (error) {
hid_err(hdev, "parse failed\n");
- goto fail;
+ return error;
}
error = wacom_parse_and_register(wacom, false);
if (error)
- goto fail;
+ return error;
if (hdev->bus == BUS_BLUETOOTH) {
error = device_create_file(&hdev->dev, &dev_attr_speed);
@@ -2759,10 +2757,6 @@ static int wacom_probe(struct hid_device *hdev,
}
return 0;
-
-fail:
- hid_set_drvdata(hdev, NULL);
- return error;
}
static void wacom_remove(struct hid_device *hdev)
@@ -2791,8 +2785,6 @@ static void wacom_remove(struct hid_device *hdev)
wacom_release_resources(wacom);
kfifo_free(&wacom_wac->pen_fifo);
-
- hid_set_drvdata(hdev, NULL);
}
#ifdef CONFIG_PM
--
2.19.2
^ permalink raw reply related
* [PATCH 1/2] HID: do not call hid_set_drvdata(hdev, NULL) in drivers
From: Benjamin Tissoires @ 2019-08-12 16:27 UTC (permalink / raw)
To: Bruno Prémont, Jonathan Cameron, Srinivas Pandruvada,
Ping Cheng, Jason Gerecke, Jiri Kosina
Cc: linux-input, linux-kernel, Benjamin Tissoires
In-Reply-To: <20190812162740.15898-1-benjamin.tissoires@redhat.com>
This is a common pattern in the HID drivers to reset the drvdata. Some
do it properly, some do it only in case of failure.
But, this is actually already handled by driver core, so there is no need
to do it manually.
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
drivers/hid/hid-cougar.c | 6 ++----
drivers/hid/hid-gfrm.c | 7 -------
drivers/hid/hid-lenovo.c | 2 --
drivers/hid/hid-picolcd_core.c | 7 +------
drivers/hid/hid-sensor-hub.c | 1 -
5 files changed, 3 insertions(+), 20 deletions(-)
diff --git a/drivers/hid/hid-cougar.c b/drivers/hid/hid-cougar.c
index e0bb7b34f3a4..4ff3bc1d25e2 100644
--- a/drivers/hid/hid-cougar.c
+++ b/drivers/hid/hid-cougar.c
@@ -207,7 +207,7 @@ static int cougar_probe(struct hid_device *hdev,
error = hid_parse(hdev);
if (error) {
hid_err(hdev, "parse failed\n");
- goto fail;
+ return error;
}
if (hdev->collection->usage == COUGAR_VENDOR_USAGE) {
@@ -219,7 +219,7 @@ static int cougar_probe(struct hid_device *hdev,
error = hid_hw_start(hdev, connect_mask);
if (error) {
hid_err(hdev, "hw start failed\n");
- goto fail;
+ return error;
}
error = cougar_bind_shared_data(hdev, cougar);
@@ -249,8 +249,6 @@ static int cougar_probe(struct hid_device *hdev,
fail_stop_and_cleanup:
hid_hw_stop(hdev);
-fail:
- hid_set_drvdata(hdev, NULL);
return error;
}
diff --git a/drivers/hid/hid-gfrm.c b/drivers/hid/hid-gfrm.c
index 86c317320bf2..699186ff2349 100644
--- a/drivers/hid/hid-gfrm.c
+++ b/drivers/hid/hid-gfrm.c
@@ -123,12 +123,6 @@ static int gfrm_probe(struct hid_device *hdev, const struct hid_device_id *id)
return ret;
}
-static void gfrm_remove(struct hid_device *hdev)
-{
- hid_hw_stop(hdev);
- hid_set_drvdata(hdev, NULL);
-}
-
static const struct hid_device_id gfrm_devices[] = {
{ HID_BLUETOOTH_DEVICE(0x58, 0x2000),
.driver_data = GFRM100 },
@@ -142,7 +136,6 @@ static struct hid_driver gfrm_driver = {
.name = "gfrm",
.id_table = gfrm_devices,
.probe = gfrm_probe,
- .remove = gfrm_remove,
.input_mapping = gfrm_input_mapping,
.raw_event = gfrm_raw_event,
.input_configured = gfrm_input_configured,
diff --git a/drivers/hid/hid-lenovo.c b/drivers/hid/hid-lenovo.c
index 364bc7f11d9d..96fa2a2c2cd3 100644
--- a/drivers/hid/hid-lenovo.c
+++ b/drivers/hid/hid-lenovo.c
@@ -866,8 +866,6 @@ static void lenovo_remove_tpkbd(struct hid_device *hdev)
led_classdev_unregister(&data_pointer->led_micmute);
led_classdev_unregister(&data_pointer->led_mute);
-
- hid_set_drvdata(hdev, NULL);
}
static void lenovo_remove_cptkbd(struct hid_device *hdev)
diff --git a/drivers/hid/hid-picolcd_core.c b/drivers/hid/hid-picolcd_core.c
index 5f7a39a5d4af..1b5c63241af0 100644
--- a/drivers/hid/hid-picolcd_core.c
+++ b/drivers/hid/hid-picolcd_core.c
@@ -534,8 +534,7 @@ static int picolcd_probe(struct hid_device *hdev,
data = kzalloc(sizeof(struct picolcd_data), GFP_KERNEL);
if (data == NULL) {
hid_err(hdev, "can't allocate space for Minibox PicoLCD device data\n");
- error = -ENOMEM;
- goto err_no_cleanup;
+ return -ENOMEM;
}
spin_lock_init(&data->lock);
@@ -597,9 +596,6 @@ static int picolcd_probe(struct hid_device *hdev,
hid_hw_stop(hdev);
err_cleanup_data:
kfree(data);
-err_no_cleanup:
- hid_set_drvdata(hdev, NULL);
-
return error;
}
@@ -635,7 +631,6 @@ static void picolcd_remove(struct hid_device *hdev)
picolcd_exit_cir(data);
picolcd_exit_keys(data);
- hid_set_drvdata(hdev, NULL);
mutex_destroy(&data->mutex);
/* Finally, clean up the picolcd data itself */
kfree(data);
diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c
index be92a6f79687..94c7398b5c27 100644
--- a/drivers/hid/hid-sensor-hub.c
+++ b/drivers/hid/hid-sensor-hub.c
@@ -742,7 +742,6 @@ static void sensor_hub_remove(struct hid_device *hdev)
}
spin_unlock_irqrestore(&data->lock, flags);
mfd_remove_devices(&hdev->dev);
- hid_set_drvdata(hdev, NULL);
mutex_destroy(&data->mutex);
}
--
2.19.2
^ permalink raw reply related
* [PATCH 0/2] HID: do not call hid_set_drvdata(hdev, NULL) in drivers
From: Benjamin Tissoires @ 2019-08-12 16:27 UTC (permalink / raw)
To: Bruno Prémont, Jonathan Cameron, Srinivas Pandruvada,
Ping Cheng, Jason Gerecke, Jiri Kosina
Cc: linux-input, linux-kernel, Benjamin Tissoires
Hi,
the wacom one is split dfrom the other drivers as the change is slightly
bigger.
Cheers,
Benjamin
Benjamin Tissoires (2):
HID: do not call hid_set_drvdata(hdev, NULL) in drivers
HID: wacom: do not call hid_set_drvdata(hdev, NULL)
drivers/hid/hid-cougar.c | 6 ++----
drivers/hid/hid-gfrm.c | 7 -------
drivers/hid/hid-lenovo.c | 2 --
drivers/hid/hid-picolcd_core.c | 7 +------
drivers/hid/hid-sensor-hub.c | 1 -
drivers/hid/wacom_sys.c | 18 +++++-------------
6 files changed, 8 insertions(+), 33 deletions(-)
--
2.19.2
^ permalink raw reply
* Re: [PATCH] HID: logitech-dj: add support of the G700(s) receiver
From: Hans de Goede @ 2019-08-12 16:26 UTC (permalink / raw)
To: Benjamin Tissoires, Jiri Kosina; +Cc: linux-input, linux-kernel
In-Reply-To: <20190812160804.11803-1-benjamin.tissoires@redhat.com>
Hi,
On 12-08-19 18:08, Benjamin Tissoires wrote:
> Both the G700 and the G700s are sharing the same receiver.
> Include support for this receiver in hid-logitech-dj so that userspace
> can differentiate both.
>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Nice, looks good to me:
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Regards,
Hans
> ---
> drivers/hid/hid-logitech-dj.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
> index c547cba05fbb..d6250b0cb9f8 100644
> --- a/drivers/hid/hid-logitech-dj.c
> +++ b/drivers/hid/hid-logitech-dj.c
> @@ -959,6 +959,7 @@ static void logi_hidpp_recv_queue_notif(struct hid_device *hdev,
> break;
> case 0x07:
> device_type = "eQUAD step 4 Gaming";
> + logi_hidpp_dev_conn_notif_equad(hdev, hidpp_report, &workitem);
> break;
> case 0x08:
> device_type = "eQUAD step 4 for gamepads";
> @@ -1832,6 +1833,10 @@ static const struct hid_device_id logi_dj_receivers[] = {
> HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
> USB_DEVICE_ID_LOGITECH_NANO_RECEIVER_2),
> .driver_data = recvr_type_hidpp},
> + { /* Logitech G700(s) receiver (0xc531) */
> + HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
> + 0xc531),
> + .driver_data = recvr_type_gaming_hidpp},
> { /* Logitech lightspeed receiver (0xc539) */
> HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
> USB_DEVICE_ID_LOGITECH_NANO_RECEIVER_LIGHTSPEED),
>
^ permalink raw reply
* [PATCH 2/2] HID: multitouch: add support for the Smart Tech panel
From: Benjamin Tissoires @ 2019-08-12 16:23 UTC (permalink / raw)
To: Matthias Fend, Jiri Kosina; +Cc: linux-input, linux-kernel, Benjamin Tissoires
In-Reply-To: <20190812162326.14253-1-benjamin.tissoires@redhat.com>
This panel is not very friendly to us:
it exposes multiple multitouch collections, some of them being of
logical application stylus.
Usually, a device has only one report per application, and that is
what I assumed in commit 8dfe14b3b47f ("HID: multitouch: ditch mt_report_id")
To avoid breaking all working device, add a new class and a new quirk
for that situation.
Reported-and-tested-by: Matthias Fend <Matthias.Fend@wolfvision.net>
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
drivers/hid/hid-multitouch.c | 34 ++++++++++++++++++++++++++++------
1 file changed, 28 insertions(+), 6 deletions(-)
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 0d190d93ca7c..3cfeb1629f79 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -68,6 +68,7 @@ MODULE_LICENSE("GPL");
#define MT_QUIRK_STICKY_FINGERS BIT(16)
#define MT_QUIRK_ASUS_CUSTOM_UP BIT(17)
#define MT_QUIRK_WIN8_PTP_BUTTONS BIT(18)
+#define MT_QUIRK_SEPARATE_APP_REPORT BIT(19)
#define MT_INPUTMODE_TOUCHSCREEN 0x02
#define MT_INPUTMODE_TOUCHPAD 0x03
@@ -103,6 +104,7 @@ struct mt_usages {
struct mt_application {
struct list_head list;
unsigned int application;
+ unsigned int report_id;
struct list_head mt_usages; /* mt usages list */
__s32 quirks;
@@ -203,6 +205,7 @@ static void mt_post_parse(struct mt_device *td, struct mt_application *app);
#define MT_CLS_VTL 0x0110
#define MT_CLS_GOOGLE 0x0111
#define MT_CLS_RAZER_BLADE_STEALTH 0x0112
+#define MT_CLS_SMART_TECH 0x0113
#define MT_DEFAULT_MAXCONTACT 10
#define MT_MAX_MAXCONTACT 250
@@ -354,6 +357,12 @@ static const struct mt_class mt_classes[] = {
MT_QUIRK_CONTACT_CNT_ACCURATE |
MT_QUIRK_WIN8_PTP_BUTTONS,
},
+ { .name = MT_CLS_SMART_TECH,
+ .quirks = MT_QUIRK_ALWAYS_VALID |
+ MT_QUIRK_IGNORE_DUPLICATES |
+ MT_QUIRK_CONTACT_CNT_ACCURATE |
+ MT_QUIRK_SEPARATE_APP_REPORT,
+ },
{ }
};
@@ -510,8 +519,9 @@ static struct mt_usages *mt_allocate_usage(struct hid_device *hdev,
}
static struct mt_application *mt_allocate_application(struct mt_device *td,
- unsigned int application)
+ struct hid_report *report)
{
+ unsigned int application = report->application;
struct mt_application *mt_application;
mt_application = devm_kzalloc(&td->hdev->dev, sizeof(*mt_application),
@@ -536,6 +546,7 @@ static struct mt_application *mt_allocate_application(struct mt_device *td,
mt_application->scantime = DEFAULT_ZERO;
mt_application->raw_cc = DEFAULT_ZERO;
mt_application->quirks = td->mtclass.quirks;
+ mt_application->report_id = report->id;
list_add_tail(&mt_application->list, &td->applications);
@@ -543,19 +554,23 @@ static struct mt_application *mt_allocate_application(struct mt_device *td,
}
static struct mt_application *mt_find_application(struct mt_device *td,
- unsigned int application)
+ struct hid_report *report)
{
+ unsigned int application = report->application;
struct mt_application *tmp, *mt_application = NULL;
list_for_each_entry(tmp, &td->applications, list) {
if (application == tmp->application) {
- mt_application = tmp;
- break;
+ if (!(td->mtclass.quirks & MT_QUIRK_SEPARATE_APP_REPORT) ||
+ tmp->report_id == report->id) {
+ mt_application = tmp;
+ break;
+ }
}
}
if (!mt_application)
- mt_application = mt_allocate_application(td, application);
+ mt_application = mt_allocate_application(td, report);
return mt_application;
}
@@ -572,7 +587,7 @@ static struct mt_report_data *mt_allocate_report_data(struct mt_device *td,
return NULL;
rdata->report = report;
- rdata->application = mt_find_application(td, report->application);
+ rdata->application = mt_find_application(td, report);
if (!rdata->application) {
devm_kfree(&td->hdev->dev, rdata);
@@ -1562,6 +1577,9 @@ static int mt_input_configured(struct hid_device *hdev, struct hid_input *hi)
case HID_VD_ASUS_CUSTOM_MEDIA_KEYS:
suffix = "Custom Media Keys";
break;
+ case HID_DG_PEN:
+ suffix = "Stylus";
+ break;
default:
suffix = "UNKNOWN";
break;
@@ -2023,6 +2041,10 @@ static const struct hid_device_id mt_devices[] = {
HID_DEVICE(BUS_I2C, HID_GROUP_MULTITOUCH_WIN_8,
USB_VENDOR_ID_SYNAPTICS, 0x8323) },
+ /* Smart Tech panels */
+ { .driver_data = MT_CLS_SMART_TECH,
+ MT_USB_DEVICE(0x0b8c, 0x0092)},
+
/* Stantum panels */
{ .driver_data = MT_CLS_CONFIDENCE,
MT_USB_DEVICE(USB_VENDOR_ID_STANTUM_STM,
--
2.19.2
^ permalink raw reply related
* [PATCH 1/2] HID: multitouch: do not filter mice nodes
From: Benjamin Tissoires @ 2019-08-12 16:23 UTC (permalink / raw)
To: Matthias Fend, Jiri Kosina; +Cc: linux-input, linux-kernel, Benjamin Tissoires
In-Reply-To: <20190812162326.14253-1-benjamin.tissoires@redhat.com>
It was a good idea at the time to not create a mouse node for the
multitouch touchscreens, but:
- touchscreens following the Win 8 protocol should not have this
disturbing mouse node anymore, or if they have, it should be
used for something else (like a joystick attached to the screen)
- touchpads have it, and they should not use it unless there is a bug,
but when the laptop has a trackstick, the data are reported through this
mouse node.
So instead of whitelisting all of the devices that have a need for the
mouse node, just export it.
hid-input.c will append a suffix to it ('Mouse'), so users will eventually
see if something goes wrong.
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
drivers/hid/hid-multitouch.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index b603c14d043b..0d190d93ca7c 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -263,7 +263,8 @@ static const struct mt_class mt_classes[] = {
MT_QUIRK_HOVERING |
MT_QUIRK_CONTACT_CNT_ACCURATE |
MT_QUIRK_STICKY_FINGERS |
- MT_QUIRK_WIN8_PTP_BUTTONS },
+ MT_QUIRK_WIN8_PTP_BUTTONS,
+ .export_all_inputs = true },
{ .name = MT_CLS_EXPORT_ALL_INPUTS,
.quirks = MT_QUIRK_ALWAYS_VALID |
MT_QUIRK_CONTACT_CNT_ACCURATE,
--
2.19.2
^ permalink raw reply related
* [PATCH 0/2] HID multitouch: small fixes
From: Benjamin Tissoires @ 2019-08-12 16:23 UTC (permalink / raw)
To: Matthias Fend, Jiri Kosina; +Cc: linux-input, linux-kernel, Benjamin Tissoires
First one should prevent us to add more quirks for Win8 devices
that have a trackstick.
Second one is a weird device that doesnt work properly in recent
kernels.
Cheers,
Benjamin
Benjamin Tissoires (2):
HID: multitouch: do not filter mice nodes
HID: multitouch: add support for the Smart Tech panel
drivers/hid/hid-multitouch.c | 37 +++++++++++++++++++++++++++++-------
1 file changed, 30 insertions(+), 7 deletions(-)
--
2.19.2
^ permalink raw reply
* [PATCH] HID: uhid: actually use the err number from userspace
From: Benjamin Tissoires @ 2019-08-12 16:21 UTC (permalink / raw)
To: David Rheinsberg, Jiri Kosina
Cc: linux-input, linux-kernel, Benjamin Tissoires
This can help debugging the situation
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
Hi,
not entirely sure if we can use this in a such simple way.
However, this is useful to mimic device behaviour from userspace.
Cheers,
Benjamin
drivers/hid/uhid.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hid/uhid.c b/drivers/hid/uhid.c
index fa0cc0899827..2fa32e7fc733 100644
--- a/drivers/hid/uhid.c
+++ b/drivers/hid/uhid.c
@@ -284,7 +284,7 @@ static int uhid_hid_set_report(struct hid_device *hid, unsigned char rnum,
goto unlock;
if (uhid->report_buf.u.set_report_reply.err)
- ret = -EIO;
+ ret = -uhid->report_buf.u.set_report_reply.err;
else
ret = count;
--
2.19.2
^ permalink raw reply related
* [PATCH] HID: logitech-dj: add support of the G700(s) receiver
From: Benjamin Tissoires @ 2019-08-12 16:08 UTC (permalink / raw)
To: Hans de Goede, Jiri Kosina; +Cc: linux-input, linux-kernel, Benjamin Tissoires
Both the G700 and the G700s are sharing the same receiver.
Include support for this receiver in hid-logitech-dj so that userspace
can differentiate both.
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
drivers/hid/hid-logitech-dj.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
index c547cba05fbb..d6250b0cb9f8 100644
--- a/drivers/hid/hid-logitech-dj.c
+++ b/drivers/hid/hid-logitech-dj.c
@@ -959,6 +959,7 @@ static void logi_hidpp_recv_queue_notif(struct hid_device *hdev,
break;
case 0x07:
device_type = "eQUAD step 4 Gaming";
+ logi_hidpp_dev_conn_notif_equad(hdev, hidpp_report, &workitem);
break;
case 0x08:
device_type = "eQUAD step 4 for gamepads";
@@ -1832,6 +1833,10 @@ static const struct hid_device_id logi_dj_receivers[] = {
HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
USB_DEVICE_ID_LOGITECH_NANO_RECEIVER_2),
.driver_data = recvr_type_hidpp},
+ { /* Logitech G700(s) receiver (0xc531) */
+ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
+ 0xc531),
+ .driver_data = recvr_type_gaming_hidpp},
{ /* Logitech lightspeed receiver (0xc539) */
HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
USB_DEVICE_ID_LOGITECH_NANO_RECEIVER_LIGHTSPEED),
--
2.19.2
^ permalink raw reply related
* [PATCH] HID: cp2112: prevent sleeping function called from invalid context
From: Benjamin Tissoires @ 2019-08-12 16:04 UTC (permalink / raw)
To: David Barksdale, Jiri Kosina
Cc: linux-input, linux-kernel, Benjamin Tissoires
When calling request_threaded_irq() with a CP2112, the function
cp2112_gpio_irq_startup() is called in a IRQ context.
Therefore we can not sleep, and we can not call
cp2112_gpio_direction_input() there.
Move the call to cp2112_gpio_direction_input() earlier to have a working
driver.
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
drivers/hid/hid-cp2112.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/hid-cp2112.c b/drivers/hid/hid-cp2112.c
index 2310c96ccf4a..db1b55df0d13 100644
--- a/drivers/hid/hid-cp2112.c
+++ b/drivers/hid/hid-cp2112.c
@@ -1153,8 +1153,6 @@ static unsigned int cp2112_gpio_irq_startup(struct irq_data *d)
INIT_DELAYED_WORK(&dev->gpio_poll_worker, cp2112_gpio_poll_callback);
- cp2112_gpio_direction_input(gc, d->hwirq);
-
if (!dev->gpio_poll) {
dev->gpio_poll = true;
schedule_delayed_work(&dev->gpio_poll_worker, 0);
@@ -1204,6 +1202,12 @@ static int __maybe_unused cp2112_allocate_irq(struct cp2112_device *dev,
return PTR_ERR(dev->desc[pin]);
}
+ ret = cp2112_gpio_direction_input(&dev->gc, pin);
+ if (ret < 0) {
+ dev_err(dev->gc.parent, "Failed to set GPIO to input dir\n");
+ goto err_desc;
+ }
+
ret = gpiochip_lock_as_irq(&dev->gc, pin);
if (ret) {
dev_err(dev->gc.parent, "Failed to lock GPIO as interrupt\n");
--
2.19.2
^ permalink raw reply related
* [PATCH v3 3/3] HID: core: fix dmesg flooding if report field larger than 32bit
From: stillcompiling @ 2019-08-12 15:20 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, open list:HID CORE LAYER,
open list
Cc: Joe Perches, Joshua Clayton
In-Reply-To: <20190812152022.27963-1-stillcompiling@gmail.com>
From: Joshua Clayton <stillcompiling@gmail.com>
Only warn once of oversize hid report value field
On HP spectre x360 convertible the message:
hid-sensor-hub 001F:8087:0AC2.0002: hid_field_extract() called with n (192) > 32! (kworker/1:2)
is continually printed many times per second, crowding out all else.
Protect dmesg by printing the warning only one time.
The size of the hid report field data structure should probably be increased.
The data structure is treated as a u32 in Linux, but an unlimited number
of bits in the USB hid spec, so there is some rearchitecture needed now that
devices are sending more than 32 bits.
Signed-off-by: Joshua Clayton <stillcompiling@gmail.com>
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 210b81a56e1a..3eaee2c37931 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1311,8 +1311,8 @@ u32 hid_field_extract(const struct hid_device *hid, u8 *report,
unsigned offset, unsigned n)
{
if (n > 32) {
- hid_warn(hid, "hid_field_extract() called with n (%d) > 32! (%s)\n",
- n, current->comm);
+ hid_warn_once(hid, "%s() called with n (%d) > 32! (%s)\n",
+ __func__, n, current->comm);
n = 32;
}
--
2.21.0
^ permalink raw reply related
* [PATCH v3 2/3] HID: core: Add printk_once variants to hid_warn() etc
From: stillcompiling @ 2019-08-12 15:20 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, open list:HID CORE LAYER,
open list
Cc: Joe Perches, Joshua Clayton
In-Reply-To: <20190812152022.27963-1-stillcompiling@gmail.com>
From: Joshua Clayton <stillcompiling@gmail.com>
hid_warn_once() is needed. Add the others as part of the block.
Signed-off-by: Joshua Clayton <stillcompiling@gmail.com>
diff --git a/include/linux/hid.h b/include/linux/hid.h
index e6c7efdb0458..cd41f209043f 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -1171,4 +1171,15 @@ do { \
#define hid_dbg(hid, fmt, ...) \
dev_dbg(&(hid)->dev, fmt, ##__VA_ARGS__)
+#define hid_err_once(hid, fmt, ...) \
+ dev_err_once(&(hid)->dev, fmt, ##__VA_ARGS__)
+#define hid_notice_once(hid, fmt, ...) \
+ dev_notice_once(&(hid)->dev, fmt, ##__VA_ARGS__)
+#define hid_warn_once(hid, fmt, ...) \
+ dev_warn_once(&(hid)->dev, fmt, ##__VA_ARGS__)
+#define hid_info_once(hid, fmt, ...) \
+ dev_info_once(&(hid)->dev, fmt, ##__VA_ARGS__)
+#define hid_dbg_once(hid, fmt, ...) \
+ dev_dbg_once(&(hid)->dev, fmt, ##__VA_ARGS__)
+
#endif
--
2.21.0
^ permalink raw reply related
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