* Re: [PATCH v2 1/1] Docs/subsystem-apis: Remove '[The ]Linux' prefixes from titles of listed documents
From: Jonathan Corbet @ 2023-01-24 22:27 UTC (permalink / raw)
To: SeongJae Park
Cc: SeongJae Park, Andrew Morton, Bjorn Helgaas, Rafael J. Wysocki,
Viresh Kumar, Herbert Xu, David S. Miller, David Airlie,
Daniel Vetter, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Jean Delvare, Guenter Roeck, Dmitry Torokhov,
Iwona Winiarska, Jaroslav Kysela, Takashi Iwai, Wim Van Sebroeck,
linux-pci, linux-doc, linux-kernel, linux-pm, linux-crypto,
linux-mm, dri-devel, linux-hwmon, linux-input, openbmc,
alsa-devel, linux-watchdog
In-Reply-To: <20230122184834.181977-1-sj@kernel.org>
SeongJae Park <sj@kernel.org> writes:
> Some documents that listed on subsystem-apis have 'Linux' or 'The Linux'
> title prefixes. It's duplicated information, and makes finding the
> document of interest with human eyes not easy. Remove the prefixes from
> the titles.
>
> Signed-off-by: SeongJae Park <sj@kernel.org>
> ---
> Changes from v1
> (https://lore.kernel.org/lkml/20230114194741.115855-1-sj@kernel.org/)
> - Drop second patch (will post later for each subsystem)
>
> Documentation/PCI/index.rst | 6 +++---
> Documentation/cpu-freq/index.rst | 6 +++---
> Documentation/crypto/index.rst | 6 +++---
> Documentation/driver-api/index.rst | 6 +++---
> Documentation/gpu/index.rst | 6 +++---
> Documentation/hwmon/index.rst | 6 +++---
> Documentation/input/index.rst | 6 +++---
> Documentation/mm/index.rst | 6 +++---
> Documentation/peci/index.rst | 6 +++---
> Documentation/scheduler/index.rst | 6 +++---
> Documentation/scsi/index.rst | 6 +++---
> Documentation/sound/index.rst | 6 +++---
> Documentation/virt/index.rst | 6 +++---
> Documentation/watchdog/index.rst | 6 +++---
> 14 files changed, 42 insertions(+), 42 deletions(-)
Applied, thanks.
jon
^ permalink raw reply
* Re: [PATCH v2 0/8] Docs: Add missing SPDX license identifiers of
From: Jonathan Corbet @ 2023-01-24 22:31 UTC (permalink / raw)
To: SeongJae Park
Cc: SeongJae Park, Alex Deucher, Daniel Vetter, David Airlie,
David S. Miller, Dmitry Torokhov, Guenter Roeck, Herbert Xu,
Jaroslav Kysela, Jean Delvare, Maarten Lankhorst, Maxime Ripard,
Takashi Iwai, Thomas Zimmermann, Andrew Morton, linux-mm,
alsa-devel, dri-devel, linux-crypto, linux-doc, linux-hwmon,
linux-input, linux-kernel
In-Reply-To: <20230122213650.187710-1-sj@kernel.org>
SeongJae Park <sj@kernel.org> writes:
> Some subsystem documents are missing SPDX license identifiers on index
> files. This patchset adds those.
>
> Changes from v1
> (https://lore.kernel.org/lkml/20230114194741.115855-2-sj@kernel.org/)
> - Separate from index file content changes
> - Separate patch for each subsystem doc (Alex Deucher)
> - Use MIT license for gpu (Alex Deucher)
>
> SeongJae Park (8):
> Docs/crypto/index: Add missing SPDX License Identifier
> Docs/driver-api/index: Add missing SPDX License Identifier
> Docs/gpu/index: Add missing SPDX License Identifier
> Docs/hwmon/index: Add missing SPDX License Identifier
> Docs/input/index: Add missing SPDX License Identifier
> Docs/mm/index: Add missing SPDX License Identifier
> Docs/scheduler/index: Add missing SPDX License Identifier
> Docs/sound/index: Add missing SPDX License Identifier
So I've applied patches 2 (since I wrote the initial file) and 8 (with
Takashi's ack). The others are also fine, I think, but I hesitate to
apply license texts to files without knowing that they match the
author's intent. I hate to say it, but I think the best approach is to
send each of the remaining patches to the appropriate maintainers for
the subsystem involved.
Thanks,
jon
^ permalink raw reply
* Re: [PATCH v2 0/8] Docs: Add missing SPDX license identifiers of
From: SeongJae Park @ 2023-01-24 23:03 UTC (permalink / raw)
To: Jonathan Corbet
Cc: SeongJae Park, Alex Deucher, Daniel Vetter, David Airlie,
David S. Miller, Dmitry Torokhov, Guenter Roeck, Herbert Xu,
Jaroslav Kysela, Jean Delvare, Maarten Lankhorst, Maxime Ripard,
Takashi Iwai, Thomas Zimmermann, Andrew Morton, linux-mm,
alsa-devel, dri-devel, linux-crypto, linux-doc, linux-hwmon,
linux-input, linux-kernel
In-Reply-To: <87a627ftuf.fsf@meer.lwn.net>
On Tue, 24 Jan 2023 15:31:04 -0700 Jonathan Corbet <corbet@lwn.net> wrote:
> SeongJae Park <sj@kernel.org> writes:
>
> > Some subsystem documents are missing SPDX license identifiers on index
> > files. This patchset adds those.
> >
> > Changes from v1
> > (https://lore.kernel.org/lkml/20230114194741.115855-2-sj@kernel.org/)
> > - Separate from index file content changes
> > - Separate patch for each subsystem doc (Alex Deucher)
> > - Use MIT license for gpu (Alex Deucher)
> >
> > SeongJae Park (8):
> > Docs/crypto/index: Add missing SPDX License Identifier
> > Docs/driver-api/index: Add missing SPDX License Identifier
> > Docs/gpu/index: Add missing SPDX License Identifier
> > Docs/hwmon/index: Add missing SPDX License Identifier
> > Docs/input/index: Add missing SPDX License Identifier
> > Docs/mm/index: Add missing SPDX License Identifier
> > Docs/scheduler/index: Add missing SPDX License Identifier
> > Docs/sound/index: Add missing SPDX License Identifier
>
> So I've applied patches 2 (since I wrote the initial file) and 8 (with
> Takashi's ack). The others are also fine, I think, but I hesitate to
> apply license texts to files without knowing that they match the
> author's intent. I hate to say it, but I think the best approach is to
> send each of the remaining patches to the appropriate maintainers for
> the subsystem involved.
That makes 100% sense, thank you, Jon!
Thanks,
SJ
>
> Thanks,
>
> jon
^ permalink raw reply
* [PATCH] input: keyboard: fix a Kconfig spelling mistake & hyphenation
From: Randy Dunlap @ 2023-01-24 23:34 UTC (permalink / raw)
To: linux-kernel; +Cc: Randy Dunlap, Dmitry Torokhov, linux-input
Correct a spelling mistake (reported by codespell).
Also hyphenate "matrix-based".
Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: linux-input@vger.kernel.org
---
drivers/input/keyboard/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff -- a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
--- a/drivers/input/keyboard/Kconfig
+++ b/drivers/input/keyboard/Kconfig
@@ -557,7 +557,7 @@ config KEYBOARD_PMIC8XXX
help
Say Y here if you want to enable the driver for the PMIC8XXX
keypad provided as a reference design from Qualcomm. This is intended
- to support upto 18x8 matrix based keypad design.
+ to support up to 18x8 matrix-based keypad design.
To compile this driver as a module, choose M here: the module will
be called pmic8xxx-keypad.
^ permalink raw reply
* Re: [PATCH 2/2] HID: hid-steam: Add rumble on Deck
From: Benjamin Tissoires @ 2023-01-25 8:41 UTC (permalink / raw)
To: Lyude Paul; +Cc: Vicki Pfau, Jiri Kosina, linux-input
In-Reply-To: <c53fc0632b16a7b04bc59ffcd642f3bad4ee864a.camel@redhat.com>
On Wed, Jan 18, 2023 at 12:16 AM Lyude Paul <lyude@redhat.com> wrote:
>
> On Tue, 2023-01-10 at 17:23 -0800, Vicki Pfau wrote:
> > The Steam Deck includes a new report that allows for emulating XInput-style
> > rumble motors with the Deck's actuators. This adds support for passing these
> > values directly to the Deck.
> >
> > Signed-off-by: Vicki Pfau <vi@endrift.com>
> > ---
> > drivers/hid/Kconfig | 8 ++++++
> > drivers/hid/hid-steam.c | 55 +++++++++++++++++++++++++++++++++++++++++
> > 2 files changed, 63 insertions(+)
> >
> > diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> > index e2a5d30c8895..e9de0a2d3cd3 100644
> > --- a/drivers/hid/Kconfig
> > +++ b/drivers/hid/Kconfig
> > @@ -1025,6 +1025,14 @@ config HID_STEAM
> > without running the Steam Client. It supports both the wired and
> > the wireless adaptor.
> >
> > +config STEAM_FF
> > + bool "Steam Deck force feedback support"
> > + depends on HID_STEAM
> > + select INPUT_FF_MEMLESS
> > + help
> > + Say Y here if you want to enable force feedback support for the Steam
> > + Deck.
> > +
> > config HID_STEELSERIES
> > tristate "Steelseries SRW-S1 steering wheel support"
> > help
> > diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
> > index efd192d6c51a..788b5baaf145 100644
> > --- a/drivers/hid/hid-steam.c
> > +++ b/drivers/hid/hid-steam.c
> > @@ -91,6 +91,7 @@ static LIST_HEAD(steam_devices);
> > #define STEAM_CMD_FORCEFEEDBAK 0x8f
> > #define STEAM_CMD_REQUEST_COMM_STATUS 0xb4
> > #define STEAM_CMD_GET_SERIAL 0xae
> > +#define STEAM_CMD_HAPTIC_RUMBLE 0xeb
> >
> > /* Some useful register ids */
> > #define STEAM_REG_LPAD_MODE 0x07
> > @@ -134,6 +135,9 @@ struct steam_device {
> > u8 battery_charge;
> > u16 voltage;
> > struct delayed_work heartbeat;
> > + struct work_struct rumble_work;
> > + u16 rumble_left;
> > + u16 rumble_right;
> > };
> >
> > static int steam_recv_report(struct steam_device *steam,
> > @@ -290,6 +294,45 @@ static inline int steam_request_conn_status(struct steam_device *steam)
> > return steam_send_report_byte(steam, STEAM_CMD_REQUEST_COMM_STATUS);
> > }
> >
> > +static inline int steam_haptic_rumble(struct steam_device *steam,
> > + u16 intensity, u16 left_speed, u16 right_speed,
> > + u8 left_gain, u8 right_gain)
> > +{
> > + u8 report[11] = {STEAM_CMD_HAPTIC_RUMBLE, 9};
> > +
> > + report[3] = intensity & 0xFF;
> > + report[4] = intensity >> 8;
> > + report[5] = left_speed & 0xFF;
> > + report[6] = left_speed >> 8;
> > + report[7] = right_speed & 0xFF;
> > + report[8] = right_speed >> 8;
> > + report[9] = left_gain;
> > + report[10] = right_gain;
> > +
> > + return steam_send_report(steam, report, sizeof(report));
> > +}
> > +
> > +static void steam_haptic_rumble_cb(struct work_struct *work)
> > +{
> > + struct steam_device *steam = container_of(work, struct steam_device,
> > + rumble_work);
> > + steam_haptic_rumble(steam, 0, steam->rumble_left,
> > + steam->rumble_right, 2, 0);
> > +}
> > +
> > +#ifdef CONFIG_STEAM_FF
> > +static int steam_play_effect(struct input_dev *dev, void *data,
> > + struct ff_effect *effect)
> > +{
> > + struct steam_device *steam = input_get_drvdata(dev);
> > +
> > + steam->rumble_left = effect->u.rumble.strong_magnitude;
> > + steam->rumble_right = effect->u.rumble.weak_magnitude;
> > +
> > + return schedule_work(&steam->rumble_work);
> > +}
> > +#endif
> > +
> > static void steam_set_lizard_mode(struct steam_device *steam, bool enable)
> > {
> > if (enable) {
> > @@ -541,6 +584,15 @@ static int steam_input_register(struct steam_device *steam)
> > input_abs_set_res(input, ABS_HAT0X, STEAM_PAD_RESOLUTION);
> > input_abs_set_res(input, ABS_HAT0Y, STEAM_PAD_RESOLUTION);
> >
> > +#ifdef CONFIG_STEAM_FF
> > + if (steam->quirks & STEAM_QUIRK_DECK) {
> > + input_set_capability(input, EV_FF, FF_RUMBLE);
> > + ret = input_ff_create_memless(input, NULL, steam_play_effect);
> > + if (ret)
> > + goto init_ff_fail;
> > + }
> > +#endif
> > +
> > ret = input_register_device(input);
> > if (ret)
> > goto input_register_fail;
> > @@ -549,6 +601,7 @@ static int steam_input_register(struct steam_device *steam)
> > return 0;
> >
> > input_register_fail:
> > +init_ff_fail:
>
> JFYI, this actually causes a compilation warning with CONFIG_STEAM_FF
> disabled:
>
> drivers/hid/hid-steam.c: In function ‘steam_input_register’:
> drivers/hid/hid-steam.c:604:1: warning: label ‘init_ff_fail’ defined but not
> used [-Wunused-label]
> 604 | init_ff_fail:
> | ^~~~~~~~~~~~
>
> TBH I think we should be fine just reusing the input_register_fail: jump label
> for this instead of adding another label.
>
> FWIW as well if you want: you could just drop the Kconfig option for this
> entirely, which bentiss may or may not want. It would at least leave a little
> less chance for compilation warnings like this, since the more Kconfig options
> you have for a module the higher the chance you'll leave a warning by mistake
> in some random kernel config.
I agree with Lyude here. However the whole HID tree is crippled with
those "if input_ff" and that would mean a bigger policy change.
So I would suggest keeping the Kconfig option for now, and if you
want, Vicki (or anybody else) maybe it's time to get rid of those
input_ff Kconfigs and make them slightly more evident for users.
Whether the first driver to use it selects input_ff or they all depend
on input_ff without the ability to disable it is entirely the
responsibility of the submitter :)
Cheers,
Benjamin
>
> If you end up deciding to leave the Kconfig in I'd at least update the commit
> message to mention explicitly you added it so people notice it even if they
> don't look at the diff (e.g. maintainers just merging reviewed patches).
>
> I have no hard opinion either way as long as we fix the compilation warning
> :). With the issues mentioned here addressed, this patch is:
>
> Reviewed-by: Lyude Paul <lyude@redhat.com>
>
> > input_free_device(input);
> > return ret;
> > }
> > @@ -842,6 +895,7 @@ static int steam_probe(struct hid_device *hdev,
> > INIT_WORK(&steam->work_connect, steam_work_connect_cb);
> > INIT_LIST_HEAD(&steam->list);
> > INIT_DEFERRABLE_WORK(&steam->heartbeat, steam_lizard_mode_heartbeat);
> > + INIT_WORK(&steam->rumble_work, steam_haptic_rumble_cb);
> >
> > steam->client_hdev = steam_create_client_hid(hdev);
> > if (IS_ERR(steam->client_hdev)) {
> > @@ -898,6 +952,7 @@ static int steam_probe(struct hid_device *hdev,
> > client_hdev_fail:
> > cancel_work_sync(&steam->work_connect);
> > cancel_delayed_work_sync(&steam->heartbeat);
> > + cancel_work_sync(&steam->rumble_work);
> > steam_alloc_fail:
> > hid_err(hdev, "%s: failed with error %d\n",
> > __func__, ret);
>
> --
> Cheers,
> Lyude Paul (she/her)
> Software Engineer at Red Hat
>
^ permalink raw reply
* Re: [PATCH 2/3] HID: logitech-hidpp: Don't restart communication if not necessary
From: Benjamin Tissoires @ 2023-01-25 10:18 UTC (permalink / raw)
To: Bastien Nocera
Cc: linux-input, linux-kernel, Jiri Kosina, Peter F . Patel-Schneider,
Filipe Laíns, Nestor Lopez Casado
In-Reply-To: <a75e34efce22ab1de8f0a2e247294a441e710193.camel@hadess.net>
On Tue, Jan 24, 2023 at 6:20 PM Bastien Nocera <hadess@hadess.net> wrote:
>
> On Tue, 2022-12-20 at 10:22 +0100, Bastien Nocera wrote:
> > Don't stop and restart communication with the device unless we need
> > to
> > modify the connect flags used because of a device quirk.
>
> FIWW, Andreas Bergmeier told me off-list that this fixed their problem
> with the Litra Glow not connecting properly.
>
> Would be great to have reviews on this and my other HID++ patches.
Sigh. I reviewed the patches just now (well, v2 at least), and thought
I better give a shot at it before merging, and it turns out that this
patch breaks the Unifying receivers.
Without it, each device presented to the user space has a proper name:
logitech-hidpp-device 0003:046D:4041.001C: input,hidraw15: USB HID
v1.11 Keyboard [Logitech MX Master] on usb-0000:01:00.0-4/input2:5
But with it, I get:
logitech-hidpp-device 0003:046D:4041.0024: input,hidraw8: USB HID
v1.11 Keyboard [Logitech Wireless Device PID:4041] on
usb-0000:00:14.0-8.2.4/input2:5
This is because we present the device to the userspace before being
able to fetch the name from the receiver.
I think we should make that connect/disconnect a special case of the
receivers too. Or maybe if the bus is not Bluetooth or USB, do the
disconnect/reconnect.
Cheers,
Benjamin
>
> Cheers
>
> > ---
> > drivers/hid/hid-logitech-hidpp.c | 31 +++++++++++++++++++-----------
> > -
> > 1 file changed, 19 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-
> > logitech-hidpp.c
> > index 7f9187201913..b4e4a8c79c75 100644
> > --- a/drivers/hid/hid-logitech-hidpp.c
> > +++ b/drivers/hid/hid-logitech-hidpp.c
> > @@ -4310,6 +4310,7 @@ static int hidpp_probe(struct hid_device *hdev,
> > const struct hid_device_id *id)
> > bool connected;
> > unsigned int connect_mask = HID_CONNECT_DEFAULT;
> > struct hidpp_ff_private_data data;
> > + bool will_restart = false;
> >
> > /* report_fixup needs drvdata to be set before we call
> > hid_parse */
> > hidpp = devm_kzalloc(&hdev->dev, sizeof(*hidpp), GFP_KERNEL);
> > @@ -4360,6 +4361,9 @@ static int hidpp_probe(struct hid_device *hdev,
> > const struct hid_device_id *id)
> > return ret;
> > }
> >
> > + if (hidpp->quirks & HIDPP_QUIRK_DELAYED_INIT)
> > + will_restart = true;
> > +
> > INIT_WORK(&hidpp->work, delayed_work_cb);
> > mutex_init(&hidpp->send_mutex);
> > init_waitqueue_head(&hidpp->wait);
> > @@ -4374,7 +4378,7 @@ static int hidpp_probe(struct hid_device *hdev,
> > const struct hid_device_id *id)
> > * Plain USB connections need to actually call start and open
> > * on the transport driver to allow incoming data.
> > */
> > - ret = hid_hw_start(hdev, 0);
> > + ret = hid_hw_start(hdev, will_restart ? 0 : connect_mask);
> > if (ret) {
> > hid_err(hdev, "hw start failed\n");
> > goto hid_hw_start_fail;
> > @@ -4411,6 +4415,7 @@ static int hidpp_probe(struct hid_device *hdev,
> > const struct hid_device_id *id)
> > hidpp->wireless_feature_index = 0;
> > else if (ret)
> > goto hid_hw_init_fail;
> > + ret = 0;
> > }
> >
> > if (connected && (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP)) {
> > @@ -4425,19 +4430,21 @@ static int hidpp_probe(struct hid_device
> > *hdev, const struct hid_device_id *id)
> >
> > hidpp_connect_event(hidpp);
> >
> > - /* Reset the HID node state */
> > - hid_device_io_stop(hdev);
> > - hid_hw_close(hdev);
> > - hid_hw_stop(hdev);
> > + if (will_restart) {
> > + /* Reset the HID node state */
> > + hid_device_io_stop(hdev);
> > + hid_hw_close(hdev);
> > + hid_hw_stop(hdev);
> >
> > - if (hidpp->quirks & HIDPP_QUIRK_NO_HIDINPUT)
> > - connect_mask &= ~HID_CONNECT_HIDINPUT;
> > + if (hidpp->quirks & HIDPP_QUIRK_NO_HIDINPUT)
> > + connect_mask &= ~HID_CONNECT_HIDINPUT;
> >
> > - /* Now export the actual inputs and hidraw nodes to the world
> > */
> > - ret = hid_hw_start(hdev, connect_mask);
> > - if (ret) {
> > - hid_err(hdev, "%s:hid_hw_start returned error\n",
> > __func__);
> > - goto hid_hw_start_fail;
> > + /* Now export the actual inputs and hidraw nodes to
> > the world */
> > + ret = hid_hw_start(hdev, connect_mask);
> > + if (ret) {
> > + hid_err(hdev, "%s:hid_hw_start returned
> > error\n", __func__);
> > + goto hid_hw_start_fail;
> > + }
> > }
> >
> > if (hidpp->quirks & HIDPP_QUIRK_CLASS_G920) {
>
^ permalink raw reply
* Re: [PATCH] HID: logitech-hidpp: Hard-code HID++ 1.0 fast scroll support
From: Benjamin Tissoires @ 2023-01-25 10:21 UTC (permalink / raw)
To: Bastien Nocera
Cc: linux-input, linux-kernel, Jiri Kosina, Peter F . Patel-Schneider,
Filipe Laíns, Nestor Lopez Casado
In-Reply-To: <20230116130937.391441-1-hadess@hadess.net>
On Mon, Jan 16, 2023 at 2:09 PM Bastien Nocera <hadess@hadess.net> wrote:
>
> HID++ 1.0 devices only export whether Fast Scrolling is enabled, not
> whether they are capable of it. Reinstate the original quirks for the 3
> supported mice so fast scrolling works again on those devices.
>
> Fixes: 908d325 ("HID: logitech-hidpp: Detect hi-res scrolling support")
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=216903
> Signed-off-by: Bastien Nocera <hadess@hadess.net>
> ---
Applied to for-6.3/logitech
Thanks!
Cheers,
Benjamin
> drivers/hid/hid-logitech-hidpp.c | 17 +++++++++--------
> 1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
> index abf2c95e4d0b..fa026e9107c5 100644
> --- a/drivers/hid/hid-logitech-hidpp.c
> +++ b/drivers/hid/hid-logitech-hidpp.c
> @@ -77,6 +77,7 @@ MODULE_PARM_DESC(disable_tap_to_click,
> #define HIDPP_QUIRK_HIDPP_WHEELS BIT(26)
> #define HIDPP_QUIRK_HIDPP_EXTRA_MOUSE_BTNS BIT(27)
> #define HIDPP_QUIRK_HIDPP_CONSUMER_VENDOR_KEYS BIT(28)
> +#define HIDPP_QUIRK_HI_RES_SCROLL_1P0 BIT(29)
>
> /* These are just aliases for now */
> #define HIDPP_QUIRK_KBD_SCROLL_WHEEL HIDPP_QUIRK_HIDPP_WHEELS
> @@ -3472,14 +3473,8 @@ static int hidpp_initialize_hires_scroll(struct hidpp_device *hidpp)
> hid_dbg(hidpp->hid_dev, "Detected HID++ 2.0 hi-res scrolling\n");
> }
> } else {
> - struct hidpp_report response;
> -
> - ret = hidpp_send_rap_command_sync(hidpp,
> - REPORT_ID_HIDPP_SHORT,
> - HIDPP_GET_REGISTER,
> - HIDPP_ENABLE_FAST_SCROLL,
> - NULL, 0, &response);
> - if (!ret) {
> + /* We cannot detect fast scrolling support on HID++ 1.0 devices */
> + if (hidpp->quirks & HIDPP_QUIRK_HI_RES_SCROLL_1P0) {
> hidpp->capabilities |= HIDPP_CAPABILITY_HIDPP10_FAST_SCROLL;
> hid_dbg(hidpp->hid_dev, "Detected HID++ 1.0 fast scroll\n");
> }
> @@ -4296,9 +4291,15 @@ static const struct hid_device_id hidpp_devices[] = {
> HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH,
> USB_DEVICE_ID_LOGITECH_T651),
> .driver_data = HIDPP_QUIRK_CLASS_WTP },
> + { /* Mouse Logitech Anywhere MX */
> + LDJ_DEVICE(0x1017), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_1P0 },
> { /* Mouse logitech M560 */
> LDJ_DEVICE(0x402d),
> .driver_data = HIDPP_QUIRK_DELAYED_INIT | HIDPP_QUIRK_CLASS_M560 },
> + { /* Mouse Logitech M705 (firmware RQM17) */
> + LDJ_DEVICE(0x101b), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_1P0 },
> + { /* Mouse Logitech Performance MX */
> + LDJ_DEVICE(0x101a), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_1P0 },
> { /* Keyboard logitech K400 */
> LDJ_DEVICE(0x4024),
> .driver_data = HIDPP_QUIRK_CLASS_K400 },
> --
> 2.39.0
>
^ permalink raw reply
* Re: [PATCH v2 1/3] Revert "HID: logitech-hidpp: add a module parameter to keep firmware gestures"
From: Benjamin Tissoires @ 2023-01-25 10:22 UTC (permalink / raw)
To: Bastien Nocera
Cc: linux-input, linux-kernel, Jiri Kosina, Peter F . Patel-Schneider,
Filipe Laíns, Nestor Lopez Casado
In-Reply-To: <20221220154345.474596-1-hadess@hadess.net>
On Tue, Dec 20, 2022 at 4:43 PM Bastien Nocera <hadess@hadess.net> wrote:
>
> Now that we're in 2022, and the majority of desktop environments can and
> should support touchpad gestures through libinput, remove the legacy
> module parameter that made it possible to use gestures implemented in
> firmware.
>
> This will eventually allow simplifying the driver's initialisation code.
>
> This reverts commit 9188dbaed68a4b23dc96eba165265c08caa7dc2a.
>
> Signed-off-by: Bastien Nocera <hadess@hadess.net>
Applied just this one to for-6.3/logitech
Cheers,
Benjamin
> ---
> drivers/hid/hid-logitech-hidpp.c | 10 ----------
> 1 file changed, 10 deletions(-)
>
> diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
> index 08ad19097e9e..7f9187201913 100644
> --- a/drivers/hid/hid-logitech-hidpp.c
> +++ b/drivers/hid/hid-logitech-hidpp.c
> @@ -32,11 +32,6 @@ MODULE_LICENSE("GPL");
> MODULE_AUTHOR("Benjamin Tissoires <benjamin.tissoires@gmail.com>");
> MODULE_AUTHOR("Nestor Lopez Casado <nlopezcasad@logitech.com>");
>
> -static bool disable_raw_mode;
> -module_param(disable_raw_mode, bool, 0644);
> -MODULE_PARM_DESC(disable_raw_mode,
> - "Disable Raw mode reporting for touchpads and keep firmware gestures.");
> -
> static bool disable_tap_to_click;
> module_param(disable_tap_to_click, bool, 0644);
> MODULE_PARM_DESC(disable_tap_to_click,
> @@ -4355,11 +4350,6 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
> hidpp_application_equals(hdev, HID_GD_KEYBOARD))
> hidpp->quirks |= HIDPP_QUIRK_HIDPP_CONSUMER_VENDOR_KEYS;
>
> - if (disable_raw_mode) {
> - hidpp->quirks &= ~HIDPP_QUIRK_CLASS_WTP;
> - hidpp->quirks &= ~HIDPP_QUIRK_NO_HIDINPUT;
> - }
> -
> if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP) {
> ret = wtp_allocate(hdev, id);
> if (ret)
> --
> 2.38.1
>
^ permalink raw reply
* [PATCH resend 0/3] Input: touchscreen - settings module-param support
From: Hans de Goede @ 2023-01-25 10:54 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Hans de Goede, Bastien Nocera, Gregor Riepl, linux-input
Hi Dmitry, et al,
This is a resend, because the discussion surrounding this patch-set
seems to have dried up without really coming to any conclusion
(summary of previous discussion below).
Original cover-letter (edited for already merged bugfix):
"""
This series adds support for overriding various silead and
goodix touchscreen related device-properties from the kernel cmdline.
This is a reposting of an earlier version which had the following
cover letter:
On x86/ACPI platforms touchscreens mostly just work without needing any
device/model specific configuration. But in some cases (mostly with Silead
and Goodix touchscreens) it is still necessary to manually specify various
touchscreen-properties on a per model basis.
This is handled by drivers/platform/x86/touchscreen_dmi.c which contains
a large list of per-model touchscreen properties which it attaches to the
(i2c)device before the touchscreen driver's probe() method gets called.
This means that ATM changing these settings requires recompiling the
kernel. This makes figuring out what settings/properties a specific
touchscreen needs very hard for normal users to do.
This new settings parameter support is especially useful for helping
users to get their Silead touchscreens to work. These need to have
all of their settings (min-x, width, min-y, height, swapping, inverting)
hardcoded in touchscreen_dmi.c. The settings mod-param allows users
to find the right setting without them needing to be capable of
building their own kernel (and without a lot of rebooting) after which
they can ask someone with some kernel-dev experience to turn this into
a touchscreen_dmi.c patch for them (and yes the way this hw works sucks,
but we cannot change that).
An example of a user who was stuck getting their touchscreen to work and
who was helped by providing a kernel with the settings mod-param added:
https://github.com/onitake/gsl-firmware/discussions/193
"""
Dmitry, you replied to the first patch of this series with:
"""
I totally understand the motivation for this, but I do not think that
having special handling for only touchscreen properties is the right
thing to do. I would very much prefer is we had a more generic approach
of adding/overriding properties (via an swnode?).
"""
To which I wrote the following long reply:
"""
I understand where you are coming from, but I suspect the devicetree
folks are going to not like any generic solution for 2 reasons:
1. Allowing overriding devicetree properties like regulator voltage is a bad idea,
granted users can already do this with a custom DTB, but that is a higher
threshold to pass for a user then just adding something on the kernel cmdline
2. Devicetree supports devicetree-overlays and I expect the devicetree folks
to steer people who want to override random devicetree properties in that
direction (or in the direction of using a custom DTB)
So the ACPI/x86 case really is somewhat special here and especially the
silead touchscreens are special here. Normally all the settings we are
talking here come from ACPI tables (or can directly be read from the
touchscreen controller) and then messing with these settings would be
a case of using an initrd with a custom ACPI DSDT, just like how
on devicetree I think we would expect people to use a custom DTB and
or a devicetree overlay.
but because of this info lacking from the ACPI tables we have it
hardcoded per 2-in-1/tablet model in:
drivers/platform/x86/touchscreen_dmi.c
The downside of this hardcoding is that testing new settings requires
building a custom kernel, which is both not helpful for having
a quick change settings -> test -> adjust settings cycle when trying
to find the right settings for a new model as well as quite a steep
hill to climb for novice users who want to get things to work on
a new model.
So I do believe that because of this the touchscreen properties
or special in this case and a somewhat custom solution to allow
setting just the touchscreen properties from the cmdline thus
is justified.
Also:
1. Having a mechanism specific to touchscreen properties is
simpler (more KISS) then having to come up with some more
complicated generic property override mechanism.
2. A touchscreen property specific mechanism is much less
susceptible to being misused. Setting the touchscreen properties
wrong cannot really result in any harm. OTOH setting the
max / end-of charging voltage of a lipo cell to 4.6 volt
(this is a real world example) is very much harmful.
The lipo-cell max charge voltage is something which we
in the sysfs interface deliberately disallow to be set any higher
then the boot-time value (lower is allowed). Adding a generic
cmdline mechanism for setting properties would allow
overriding this.
"""
And then the discussion stopped.
Regards,
Hans
Hans de Goede (3):
Input: touchscreen - Extend touchscreen_parse_properties() to allow
overriding settings with a module option
Input: silead - Add a settings module-parameter
Input: goodix - Add a settings module-parameter
drivers/input/touchscreen.c | 103 ++++++++++++++++++++++++++---
drivers/input/touchscreen/goodix.c | 7 +-
drivers/input/touchscreen/silead.c | 26 +++++---
include/linux/input/touchscreen.h | 19 +++++-
4 files changed, 131 insertions(+), 24 deletions(-)
--
2.39.0
^ permalink raw reply
* [PATCH resend 3/3] Input: goodix - Add a settings module-parameter
From: Hans de Goede @ 2023-01-25 10:54 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Hans de Goede, Bastien Nocera, Gregor Riepl, linux-input
In-Reply-To: <20230125105416.17406-1-hdegoede@redhat.com>
Add a settings module-parameter which can be allowed to specify/override
various device-properties.
Usuually Goodix touchscreens on x86 devices (where there is no devicetree
to specify the properties) just work. But in some cases the touchscreen is
mounted 90 or 180 degrees rotated vs the display, requiring setting the
touchscreen-x/y-inverted or touchscreen-swapped-x-y properties.
Being able to specify the necessary properties through a module parameter,
allows users of new device-models to help us with bringing up support for
new devices without them needing to do a local kernel-build just to modify
these settings.
Reviewed-by: Bastien Nocera <hadess@hadess.net>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/input/touchscreen/goodix.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
index 620f5ec0962f..a105dee04156 100644
--- a/drivers/input/touchscreen/goodix.c
+++ b/drivers/input/touchscreen/goodix.c
@@ -53,6 +53,10 @@
#define ACPI_GPIO_SUPPORT
#endif
+static char *settings;
+module_param(settings, charp, 0444);
+MODULE_PARM_DESC(settings, "Override touchscreen settings using a ; separated key=value list, e.g. \"touchscreen-fuzz-x=5;touchscreen-fuzz-y=5;touchscreen-swapped-x-y\"");
+
struct goodix_chip_id {
const char *id;
const struct goodix_chip_data *data;
@@ -1197,7 +1201,8 @@ static int goodix_configure_dev(struct goodix_ts_data *ts)
goodix_read_config(ts);
/* Try overriding touchscreen parameters via device properties */
- touchscreen_parse_properties(ts->input_dev, true, &ts->prop);
+ touchscreen_parse_properties_with_settings(ts->input_dev, true,
+ &ts->prop, settings);
if (!ts->prop.max_x || !ts->prop.max_y || !ts->max_touch_num) {
if (!ts->reset_controller_at_probe &&
--
2.39.0
^ permalink raw reply related
* [PATCH resend 2/3] Input: silead - Add a settings module-parameter
From: Hans de Goede @ 2023-01-25 10:54 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Hans de Goede, Bastien Nocera, Gregor Riepl, linux-input
In-Reply-To: <20230125105416.17406-1-hdegoede@redhat.com>
Add a settings module-parameter which can be allowed to specify/override
various device-properties.
Unlike most other touchscreen controllers Silead touchscreens don't tell us
the ranges of the reported x and y coordinates and Silead touchscreens also
often need to have their axis inverted and/or swapped to match the display
coordinates.
Being able to specify the necessary properties through a module parameter,
allows users of new device-models to help us with bringing up support for
new devices without them needing to do a local kernel-build just to modify
these settings.
Reviewed-by: Bastien Nocera <hadess@hadess.net>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/input/touchscreen/silead.c | 26 ++++++++++++++++----------
1 file changed, 16 insertions(+), 10 deletions(-)
diff --git a/drivers/input/touchscreen/silead.c b/drivers/input/touchscreen/silead.c
index 8a7351c4414c..22cee8d1bb22 100644
--- a/drivers/input/touchscreen/silead.c
+++ b/drivers/input/touchscreen/silead.c
@@ -58,6 +58,10 @@
#define SILEAD_MAX_FINGERS 10
+static char *settings;
+module_param(settings, charp, 0444);
+MODULE_PARM_DESC(settings, "Override touchscreen settings using a ; separated key=value list, e.g. \"touchscreen-size-x=1665;touchscreen-size-y=1140;touchscreen-swapped-x-y\"");
+
enum silead_ts_power {
SILEAD_POWER_ON = 1,
SILEAD_POWER_OFF = 0
@@ -133,14 +137,15 @@ static int silead_ts_request_input_dev(struct silead_ts_data *data)
input_set_abs_params(data->input, ABS_MT_POSITION_X, 0, 4095, 0, 0);
input_set_abs_params(data->input, ABS_MT_POSITION_Y, 0, 4095, 0, 0);
- touchscreen_parse_properties(data->input, true, &data->prop);
+ touchscreen_parse_properties_with_settings(data->input, true,
+ &data->prop, settings);
silead_apply_efi_fw_min_max(data);
input_mt_init_slots(data->input, data->max_fingers,
INPUT_MT_DIRECT | INPUT_MT_DROP_UNUSED |
INPUT_MT_TRACK);
- if (device_property_read_bool(dev, "silead,home-button"))
+ if (touchscreen_property_read_bool(dev, "silead,home-button", settings))
input_set_capability(data->input, EV_KEY, KEY_LEFTMETA);
data->input->name = SILEAD_TS_NAME;
@@ -173,7 +178,8 @@ static int silead_ts_request_pen_input_dev(struct silead_ts_data *data)
input_set_capability(data->pen_input, EV_KEY, BTN_TOUCH);
input_set_capability(data->pen_input, EV_KEY, BTN_TOOL_PEN);
set_bit(INPUT_PROP_DIRECT, data->pen_input->propbit);
- touchscreen_parse_properties(data->pen_input, false, &data->prop);
+ touchscreen_parse_properties_with_settings(data->pen_input, false,
+ &data->prop, settings);
input_abs_set_res(data->pen_input, ABS_X, data->pen_x_res);
input_abs_set_res(data->pen_input, ABS_Y, data->pen_y_res);
@@ -523,8 +529,8 @@ static int silead_ts_setup(struct i2c_client *client)
* this.
*/
- if (device_property_read_bool(&client->dev,
- "silead,stuck-controller-bug")) {
+ if (touchscreen_property_read_bool(&client->dev, "silead,stuck-controller-bug",
+ settings)) {
pm_runtime_set_active(&client->dev);
pm_runtime_enable(&client->dev);
pm_runtime_allow(&client->dev);
@@ -591,8 +597,8 @@ static void silead_ts_read_props(struct i2c_client *client)
const char *str;
int error;
- error = device_property_read_u32(dev, "silead,max-fingers",
- &data->max_fingers);
+ error = touchscreen_property_read_u32(dev, "silead,max-fingers", settings,
+ &data->max_fingers);
if (error) {
dev_dbg(dev, "Max fingers read error %d\n", error);
data->max_fingers = 5; /* Most devices handle up-to 5 fingers */
@@ -605,9 +611,9 @@ static void silead_ts_read_props(struct i2c_client *client)
else
dev_dbg(dev, "Firmware file name read error. Using default.");
- data->pen_supported = device_property_read_bool(dev, "silead,pen-supported");
- device_property_read_u32(dev, "silead,pen-resolution-x", &data->pen_x_res);
- device_property_read_u32(dev, "silead,pen-resolution-y", &data->pen_y_res);
+ data->pen_supported = touchscreen_property_read_bool(dev, "silead,pen-supported", settings);
+ touchscreen_property_read_u32(dev, "silead,pen-resolution-x", settings, &data->pen_x_res);
+ touchscreen_property_read_u32(dev, "silead,pen-resolution-y", settings, &data->pen_y_res);
}
#ifdef CONFIG_ACPI
--
2.39.0
^ permalink raw reply related
* [PATCH resend 1/3] Input: touchscreen - Extend touchscreen_parse_properties() to allow overriding settings with a module option
From: Hans de Goede @ 2023-01-25 10:54 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Hans de Goede, Bastien Nocera, Gregor Riepl, linux-input
In-Reply-To: <20230125105416.17406-1-hdegoede@redhat.com>
On x86/ACPI platforms touchscreens mostly just work without needing any
device/model specific configuration. But in some cases (mostly with Silead
and Goodix touchscreens) it is still necessary to manually specify various
touchscreen-properties on a per model basis.
This is handled by drivers/platform/x86/touchscreen_dmi.c which contains
a large list of per-model touchscreen properties which it attaches to the
(i2c)device before the touchscreen driver's probe() method gets called.
This means that ATM changing these settings requires recompiling the
kernel. This makes figuring out what settings/properties a specific
touchscreen needs very hard for normal users to do.
Add a new, optional, settings_override string argument to
touchscreen_parse_properties(), which takes a list of ; separated
property-name=value pairs, e.g. :
"touchscreen-size-x=1665;touchscreen-size-y=1140;touchscreen-swapped-x-y".
This new argument can be used by drivers to implement a module option which
allows users to easily specify alternative settings for testing.
The 2 new touchscreen_property_read_u32() and
touchscreen_property_read_bool() helpers are also exported so that
drivers can use these to add settings-override support to the code
for driver-specific properties.
Reviewed-by: Bastien Nocera <hadess@hadess.net>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
- Instead of patching all drivers rename touchscreen_parse_properties()
to touchscreen_parse_properties_with_override() and add
a static inline wrapper which passes NULL.
---
drivers/input/touchscreen.c | 103 ++++++++++++++++++++++++++----
include/linux/input/touchscreen.h | 19 +++++-
2 files changed, 109 insertions(+), 13 deletions(-)
diff --git a/drivers/input/touchscreen.c b/drivers/input/touchscreen.c
index 4620e20d0190..3b9505d5468d 100644
--- a/drivers/input/touchscreen.c
+++ b/drivers/input/touchscreen.c
@@ -12,15 +12,80 @@
#include <linux/input/touchscreen.h>
#include <linux/module.h>
+static int touchscreen_get_prop_from_settings_string(const char *settings,
+ const char *propname,
+ bool is_boolean,
+ u32 *val_ret)
+{
+ char *begin, *end;
+ u32 val;
+
+ if (!settings)
+ return -ENOENT;
+
+ begin = strstr(settings, propname);
+ if (!begin)
+ return -ENOENT;
+
+ /* begin must be either the begin of settings, or be preceded by a ';' */
+ if (begin != settings && begin[-1] != ';')
+ return -EINVAL;
+
+ end = begin + strlen(propname);
+ if (*end != '=') {
+ if (is_boolean && (*end == '\0' || *end == ';')) {
+ *val_ret = true;
+ return 0;
+ }
+ return -EINVAL;
+ }
+
+ val = simple_strtoul(end + 1, &end, 0);
+ if (*end != '\0' && *end != ';')
+ return -EINVAL;
+
+ *val_ret = val;
+ return 0;
+}
+
+int touchscreen_property_read_u32(struct device *dev, const char *propname,
+ const char *settings, u32 *val)
+{
+ int error;
+
+ error = device_property_read_u32(dev, propname, val);
+
+ if (touchscreen_get_prop_from_settings_string(settings, propname,
+ false, val) == 0)
+ error = 0;
+
+ return error;
+}
+EXPORT_SYMBOL(touchscreen_property_read_u32);
+
+bool touchscreen_property_read_bool(struct device *dev, const char *propname,
+ const char *settings)
+{
+ u32 val;
+
+ val = device_property_read_bool(dev, propname);
+
+ touchscreen_get_prop_from_settings_string(settings, propname, true, &val);
+
+ return val;
+}
+EXPORT_SYMBOL(touchscreen_property_read_bool);
+
static bool touchscreen_get_prop_u32(struct device *dev,
const char *property,
+ const char *settings,
unsigned int default_value,
unsigned int *value)
{
u32 val;
int error;
- error = device_property_read_u32(dev, property, &val);
+ error = touchscreen_property_read_u32(dev, property, settings, &val);
if (error) {
*value = default_value;
return false;
@@ -50,20 +115,28 @@ static void touchscreen_set_params(struct input_dev *dev,
}
/**
- * touchscreen_parse_properties - parse common touchscreen properties
+ * touchscreen_parse_properties_with_settings - parse common touchscreen properties
* @input: input device that should be parsed
* @multitouch: specifies whether parsed properties should be applied to
* single-touch or multi-touch axes
* @prop: pointer to a struct touchscreen_properties into which to store
* axis swap and invert info for use with touchscreen_report_x_y();
* or %NULL
+ * @settings: string with ; separated name=value pairs overriding
+ * the device-properties or %NULL.
*
* This function parses common properties for touchscreens and sets up the
* input device accordingly. The function keeps previously set up default
* values if no value is specified.
+ *
+ * Callers can optional specify a settings string overriding the
+ * device-properties, this can be used to implement a module option which
+ * allows users to easily specify alternative settings for testing.
*/
-void touchscreen_parse_properties(struct input_dev *input, bool multitouch,
- struct touchscreen_properties *prop)
+void touchscreen_parse_properties_with_settings(struct input_dev *input,
+ bool multitouch,
+ struct touchscreen_properties *prop,
+ const char *settings)
{
struct device *dev = input->dev.parent;
struct input_absinfo *absinfo;
@@ -79,26 +152,32 @@ void touchscreen_parse_properties(struct input_dev *input, bool multitouch,
axis_y = multitouch ? ABS_MT_POSITION_Y : ABS_Y;
data_present = touchscreen_get_prop_u32(dev, "touchscreen-min-x",
+ settings,
input_abs_get_min(input, axis_x),
&minimum);
data_present |= touchscreen_get_prop_u32(dev, "touchscreen-size-x",
+ settings,
input_abs_get_max(input,
axis_x) + 1,
&maximum);
data_present |= touchscreen_get_prop_u32(dev, "touchscreen-fuzz-x",
+ settings,
input_abs_get_fuzz(input, axis_x),
&fuzz);
if (data_present)
touchscreen_set_params(input, axis_x, minimum, maximum - 1, fuzz);
data_present = touchscreen_get_prop_u32(dev, "touchscreen-min-y",
+ settings,
input_abs_get_min(input, axis_y),
&minimum);
data_present |= touchscreen_get_prop_u32(dev, "touchscreen-size-y",
+ settings,
input_abs_get_max(input,
axis_y) + 1,
&maximum);
data_present |= touchscreen_get_prop_u32(dev, "touchscreen-fuzz-y",
+ settings,
input_abs_get_fuzz(input, axis_y),
&fuzz);
if (data_present)
@@ -107,10 +186,12 @@ void touchscreen_parse_properties(struct input_dev *input, bool multitouch,
axis = multitouch ? ABS_MT_PRESSURE : ABS_PRESSURE;
data_present = touchscreen_get_prop_u32(dev,
"touchscreen-max-pressure",
+ settings,
input_abs_get_max(input, axis),
&maximum);
data_present |= touchscreen_get_prop_u32(dev,
"touchscreen-fuzz-pressure",
+ settings,
input_abs_get_fuzz(input, axis),
&fuzz);
if (data_present)
@@ -122,28 +203,28 @@ void touchscreen_parse_properties(struct input_dev *input, bool multitouch,
prop->max_x = input_abs_get_max(input, axis_x);
prop->max_y = input_abs_get_max(input, axis_y);
- prop->invert_x =
- device_property_read_bool(dev, "touchscreen-inverted-x");
+ prop->invert_x = touchscreen_property_read_bool(dev, "touchscreen-inverted-x",
+ settings);
if (prop->invert_x) {
absinfo = &input->absinfo[axis_x];
absinfo->maximum -= absinfo->minimum;
absinfo->minimum = 0;
}
- prop->invert_y =
- device_property_read_bool(dev, "touchscreen-inverted-y");
+ prop->invert_y = touchscreen_property_read_bool(dev, "touchscreen-inverted-y",
+ settings);
if (prop->invert_y) {
absinfo = &input->absinfo[axis_y];
absinfo->maximum -= absinfo->minimum;
absinfo->minimum = 0;
}
- prop->swap_x_y =
- device_property_read_bool(dev, "touchscreen-swapped-x-y");
+ prop->swap_x_y = touchscreen_property_read_bool(dev, "touchscreen-swapped-x-y",
+ settings);
if (prop->swap_x_y)
swap(input->absinfo[axis_x], input->absinfo[axis_y]);
}
-EXPORT_SYMBOL(touchscreen_parse_properties);
+EXPORT_SYMBOL(touchscreen_parse_properties_with_settings);
static void
touchscreen_apply_prop_to_x_y(const struct touchscreen_properties *prop,
diff --git a/include/linux/input/touchscreen.h b/include/linux/input/touchscreen.h
index fe66e2b58f62..0023c6e368ba 100644
--- a/include/linux/input/touchscreen.h
+++ b/include/linux/input/touchscreen.h
@@ -17,8 +17,23 @@ struct touchscreen_properties {
bool swap_x_y;
};
-void touchscreen_parse_properties(struct input_dev *input, bool multitouch,
- struct touchscreen_properties *prop);
+void touchscreen_parse_properties_with_settings(struct input_dev *input,
+ bool multitouch,
+ struct touchscreen_properties *prop,
+ const char *settings);
+
+static inline void touchscreen_parse_properties(struct input_dev *input,
+ bool multitouch,
+ struct touchscreen_properties *prop)
+{
+ touchscreen_parse_properties_with_settings(input, multitouch, prop, NULL);
+}
+
+int touchscreen_property_read_u32(struct device *dev, const char *propname,
+ const char *settings, u32 *val);
+
+bool touchscreen_property_read_bool(struct device *dev, const char *propname,
+ const char *settings);
void touchscreen_set_mt_pos(struct input_mt_pos *pos,
const struct touchscreen_properties *prop,
--
2.39.0
^ permalink raw reply related
* Re: [PATCH 2/3] HID: logitech-hidpp: Don't restart communication if not necessary
From: Bastien Nocera @ 2023-01-25 11:52 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: linux-input, linux-kernel, Jiri Kosina, Peter F . Patel-Schneider,
Filipe Laíns, Nestor Lopez Casado
In-Reply-To: <CAO-hwJJb+hkCpqbiF0Zw8Ot4aCJDpgvMXpVS6rCoMe7QWkhiCg@mail.gmail.com>
On Wed, 2023-01-25 at 11:18 +0100, Benjamin Tissoires wrote:
> On Tue, Jan 24, 2023 at 6:20 PM Bastien Nocera <hadess@hadess.net>
> wrote:
> >
> > On Tue, 2022-12-20 at 10:22 +0100, Bastien Nocera wrote:
> > > Don't stop and restart communication with the device unless we
> > > need
> > > to
> > > modify the connect flags used because of a device quirk.
> >
> > FIWW, Andreas Bergmeier told me off-list that this fixed their
> > problem
> > with the Litra Glow not connecting properly.
> >
> > Would be great to have reviews on this and my other HID++ patches.
>
> Sigh. I reviewed the patches just now (well, v2 at least), and
> thought
> I better give a shot at it before merging, and it turns out that this
> patch breaks the Unifying receivers.
>
> Without it, each device presented to the user space has a proper
> name:
>
> logitech-hidpp-device 0003:046D:4041.001C: input,hidraw15: USB HID
> v1.11 Keyboard [Logitech MX Master] on usb-0000:01:00.0-4/input2:5
>
> But with it, I get:
>
> logitech-hidpp-device 0003:046D:4041.0024: input,hidraw8: USB HID
> v1.11 Keyboard [Logitech Wireless Device PID:4041] on
> usb-0000:00:14.0-8.2.4/input2:5
>
> This is because we present the device to the userspace before being
> able to fetch the name from the receiver.
>
> I think we should make that connect/disconnect a special case of the
> receivers too. Or maybe if the bus is not Bluetooth or USB, do the
> disconnect/reconnect.
From what I can tell, this would mean restarting the connection in case
hidpp_unifying_init() did anything, right?
I'll test this out and update the patch.
diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 4547e9580101..e0c28257f598 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -4392,8 +4392,10 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
/* Allow incoming packets */
hid_device_io_start(hdev);
- if (hidpp->quirks & HIDPP_QUIRK_UNIFYING)
- hidpp_unifying_init(hidpp);
+ if (hidpp->quirks & HIDPP_QUIRK_UNIFYING) {
+ if (hidpp_unifying_init(hidpp) == 0)
+ will_restart = true;
+ }
connected = hidpp_root_get_protocol_version(hidpp) == 0;
atomic_set(&hidpp->connected, connected);
^ permalink raw reply related
* [PATCH v2 1/2] HID: logitech-hidpp: Don't restart communication if not necessary
From: Bastien Nocera @ 2023-01-25 12:17 UTC (permalink / raw)
To: linux-input
Cc: linux-kernel, Jiri Kosina, Benjamin Tissoires,
Peter F . Patel-Schneider, Filipe Laíns, Nestor Lopez Casado
Don't stop and restart communication with the device unless we need to
modify the connect flags used because of a device quirk.
Signed-off-by: Bastien Nocera <hadess@hadess.net>
---
drivers/hid/hid-logitech-hidpp.c | 32 ++++++++++++++++++++------------
1 file changed, 20 insertions(+), 12 deletions(-)
diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index abf2c95e4d0b..31d1fc23a435 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -4106,6 +4106,7 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
bool connected;
unsigned int connect_mask = HID_CONNECT_DEFAULT;
struct hidpp_ff_private_data data;
+ bool will_restart = false;
/* report_fixup needs drvdata to be set before we call hid_parse */
hidpp = devm_kzalloc(&hdev->dev, sizeof(*hidpp), GFP_KERNEL);
@@ -4161,6 +4162,10 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
return ret;
}
+ if (hidpp->quirks & HIDPP_QUIRK_DELAYED_INIT ||
+ hidpp->quirks & HIDPP_QUIRK_UNIFYING)
+ will_restart = true;
+
INIT_WORK(&hidpp->work, delayed_work_cb);
mutex_init(&hidpp->send_mutex);
init_waitqueue_head(&hidpp->wait);
@@ -4175,7 +4180,7 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
* Plain USB connections need to actually call start and open
* on the transport driver to allow incoming data.
*/
- ret = hid_hw_start(hdev, 0);
+ ret = hid_hw_start(hdev, will_restart ? 0 : connect_mask);
if (ret) {
hid_err(hdev, "hw start failed\n");
goto hid_hw_start_fail;
@@ -4212,6 +4217,7 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
hidpp->wireless_feature_index = 0;
else if (ret)
goto hid_hw_init_fail;
+ ret = 0;
}
if (connected && (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP)) {
@@ -4226,19 +4232,21 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
hidpp_connect_event(hidpp);
- /* Reset the HID node state */
- hid_device_io_stop(hdev);
- hid_hw_close(hdev);
- hid_hw_stop(hdev);
+ if (will_restart) {
+ /* Reset the HID node state */
+ hid_device_io_stop(hdev);
+ hid_hw_close(hdev);
+ hid_hw_stop(hdev);
- if (hidpp->quirks & HIDPP_QUIRK_NO_HIDINPUT)
- connect_mask &= ~HID_CONNECT_HIDINPUT;
+ if (hidpp->quirks & HIDPP_QUIRK_NO_HIDINPUT)
+ connect_mask &= ~HID_CONNECT_HIDINPUT;
- /* Now export the actual inputs and hidraw nodes to the world */
- ret = hid_hw_start(hdev, connect_mask);
- if (ret) {
- hid_err(hdev, "%s:hid_hw_start returned error\n", __func__);
- goto hid_hw_start_fail;
+ /* Now export the actual inputs and hidraw nodes to the world */
+ ret = hid_hw_start(hdev, connect_mask);
+ if (ret) {
+ hid_err(hdev, "%s:hid_hw_start returned error\n", __func__);
+ goto hid_hw_start_fail;
+ }
}
if (hidpp->quirks & HIDPP_QUIRK_CLASS_G920) {
--
2.39.1
^ permalink raw reply related
* [PATCH v2 2/2] HID: logitech-hidpp: Remove HIDPP_QUIRK_NO_HIDINPUT quirk
From: Bastien Nocera @ 2023-01-25 12:17 UTC (permalink / raw)
To: linux-input
Cc: linux-kernel, Jiri Kosina, Benjamin Tissoires,
Peter F . Patel-Schneider, Filipe Laíns, Nestor Lopez Casado
In-Reply-To: <20230125121723.3122-1-hadess@hadess.net>
HIDPP_QUIRK_NO_HIDINPUT isn't used by any devices but still happens to
work as HIDPP_QUIRK_DELAYED_INIT is defined to the same value. Remove
HIDPP_QUIRK_NO_HIDINPUT and use HIDPP_QUIRK_DELAYED_INIT everywhere
instead.
Tested on a T650 which requires that quirk, and a number of unifying and
Bluetooth devices that don't.
Signed-off-by: Bastien Nocera <hadess@hadess.net>
---
drivers/hid/hid-logitech-hidpp.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 31d1fc23a435..ec388e436b6a 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -71,7 +71,7 @@ MODULE_PARM_DESC(disable_tap_to_click,
/* bits 2..20 are reserved for classes */
/* #define HIDPP_QUIRK_CONNECT_EVENTS BIT(21) disabled */
#define HIDPP_QUIRK_WTP_PHYSICAL_BUTTONS BIT(22)
-#define HIDPP_QUIRK_NO_HIDINPUT BIT(23)
+#define HIDPP_QUIRK_DELAYED_INIT BIT(23)
#define HIDPP_QUIRK_FORCE_OUTPUT_REPORTS BIT(24)
#define HIDPP_QUIRK_UNIFYING BIT(25)
#define HIDPP_QUIRK_HIDPP_WHEELS BIT(26)
@@ -87,8 +87,6 @@ MODULE_PARM_DESC(disable_tap_to_click,
HIDPP_CAPABILITY_HIDPP20_HI_RES_SCROLL | \
HIDPP_CAPABILITY_HIDPP20_HI_RES_WHEEL)
-#define HIDPP_QUIRK_DELAYED_INIT HIDPP_QUIRK_NO_HIDINPUT
-
#define HIDPP_CAPABILITY_HIDPP10_BATTERY BIT(0)
#define HIDPP_CAPABILITY_HIDPP20_BATTERY BIT(1)
#define HIDPP_CAPABILITY_BATTERY_MILEAGE BIT(2)
@@ -4001,7 +3999,7 @@ static void hidpp_connect_event(struct hidpp_device *hidpp)
if (hidpp->capabilities & HIDPP_CAPABILITY_HI_RES_SCROLL)
hi_res_scroll_enable(hidpp);
- if (!(hidpp->quirks & HIDPP_QUIRK_NO_HIDINPUT) || hidpp->delayed_input)
+ if (!(hidpp->quirks & HIDPP_QUIRK_DELAYED_INIT) || hidpp->delayed_input)
/* if the input nodes are already created, we can stop now */
return;
@@ -4238,7 +4236,7 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
hid_hw_close(hdev);
hid_hw_stop(hdev);
- if (hidpp->quirks & HIDPP_QUIRK_NO_HIDINPUT)
+ if (hidpp->quirks & HIDPP_QUIRK_DELAYED_INIT)
connect_mask &= ~HID_CONNECT_HIDINPUT;
/* Now export the actual inputs and hidraw nodes to the world */
--
2.39.1
^ permalink raw reply related
* Re: [PATCH 1/1] HID: evision: Add preliminary support for EVision keyboards
From: Jiri Kosina @ 2023-01-25 13:27 UTC (permalink / raw)
To: Benjamin Tissoires; +Cc: Philippe Valembois, linux-kernel, linux-input
In-Reply-To: <CAO-hwJJwUQJueutZ5z_4qwBb1Y4+-9h3ta0Xran=s6pJ_e3itQ@mail.gmail.com>
On Mon, 23 Jan 2023, Benjamin Tissoires wrote:
> > From: Philippe Valembois <lephilousophe@users.noreply.github.com>
>
> Jiri, I have a doubt. Do we accept emails from users.noreply.github.com?
From quick search in github documentation, this seems to be a blackhole
email address, right?
If so, I'd much prefer to have a real e-mail address there, in order to
make sure the author / signoff holders can be reached in the future if
there are any concerns regarding the patch.
The e-mail address disclosure concern is moot as soon as the emaill with
the patch hits LKML anyway.
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] HID: Ignore battery for Elan touchscreen on Asus TP420IA
From: Jiri Kosina @ 2023-01-25 14:41 UTC (permalink / raw)
To: Marco Rodolfi; +Cc: Benjamin Tissoires, Linux Input, Linux Kernel
In-Reply-To: <NLR9TEh--3-9@tuta.io>
On Tue, 10 Jan 2023, marco.rodolfi@tuta.io wrote:
>
> This device has a touchscreen thats report a battery even if it doesn't
> have one.
> Ask Linux to ignore the battery so it will not always report it as low.
>
> Signed-off-by: Marco Rodolfi <marco.rodolfi@tuta.io>
Hi Marco,
your patch has been malformed by your mail client. I have fixed it up and
applied, but please make sure you fix that for your further submissions.
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* [PATCH v3 1/1] HID: evision: Add preliminary support for EVision keyboards
From: Philippe Valembois @ 2023-01-25 21:15 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, linux-kernel, linux-input
Cc: Philippe Valembois
In-Reply-To: <20230123211843.10028-1-lephilousophe@gmail.com>
For now only supports one model and only filters out bogus reports sent
when the keyboard has been configured through hidraw.
Without this, as events are not released, soft repeat floods userspace
with unknown key events.
Signed-off-by: Philippe Valembois <lephilousophe@gmail.com>
---
drivers/hid/Kconfig | 7 ++++++
drivers/hid/Makefile | 1 +
drivers/hid/hid-evision.c | 53 +++++++++++++++++++++++++++++++++++++++
drivers/hid/hid-ids.h | 3 +++
4 files changed, 64 insertions(+)
create mode 100644 drivers/hid/hid-evision.c
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index e2a5d30c8..35f214773 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -329,6 +329,13 @@ config HID_ELO
Support for the ELO USB 4000/4500 touchscreens. Note that this is for
different devices than those handled by CONFIG_TOUCHSCREEN_USB_ELO.
+config HID_EVISION
+ tristate "EVision Keyboards Support"
+ depends on HID
+ help
+ Support for some EVision keyboards. Note that this is needed only when
+ applying customization using userspace programs.
+
config HID_EZKEY
tristate "Ezkey BTC 8193 keyboard"
default !EXPERT
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index e8014c1a2..bd01571dd 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -45,6 +45,7 @@ obj-$(CONFIG_HID_EMS_FF) += hid-emsff.o
obj-$(CONFIG_HID_ELAN) += hid-elan.o
obj-$(CONFIG_HID_ELECOM) += hid-elecom.o
obj-$(CONFIG_HID_ELO) += hid-elo.o
+obj-$(CONFIG_HID_EVISION) += hid-evision.o
obj-$(CONFIG_HID_EZKEY) += hid-ezkey.o
obj-$(CONFIG_HID_FT260) += hid-ft260.o
obj-$(CONFIG_HID_GEMBIRD) += hid-gembird.o
diff --git a/drivers/hid/hid-evision.c b/drivers/hid/hid-evision.c
new file mode 100644
index 000000000..ef6b4b435
--- /dev/null
+++ b/drivers/hid/hid-evision.c
@@ -0,0 +1,53 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * HID driver for EVision devices
+ * For now, only ignore bogus consumer reports
+ * sent after the keyboard has been configured
+ *
+ * Copyright (c) 2022 Philippe Valembois
+ */
+
+#include <linux/device.h>
+#include <linux/input.h>
+#include <linux/hid.h>
+#include <linux/module.h>
+
+#include "hid-ids.h"
+
+static int evision_input_mapping(struct hid_device *hdev, struct hid_input *hi,
+ struct hid_field *field, struct hid_usage *usage,
+ unsigned long **bit, int *max)
+{
+ if ((usage->hid & HID_USAGE_PAGE) != HID_UP_CONSUMER)
+ return 0;
+
+ /* Ignore key down event */
+ if ((usage->hid & HID_USAGE) >> 8 == 0x05)
+ return -1;
+ /* Ignore key up event */
+ if ((usage->hid & HID_USAGE) >> 8 == 0x06)
+ return -1;
+
+ switch (usage->hid & HID_USAGE) {
+ /* Ignore configuration saved event */
+ case 0x0401: return -1;
+ /* Ignore reset event */
+ case 0x0402: return -1;
+ }
+ return 0;
+}
+
+static const struct hid_device_id evision_devices[] = {
+ { HID_USB_DEVICE(USB_VENDOR_ID_EVISION, USB_DEVICE_ID_EVISION_ICL01) },
+ { }
+};
+MODULE_DEVICE_TABLE(hid, evision_devices);
+
+static struct hid_driver evision_driver = {
+ .name = "evision",
+ .id_table = evision_devices,
+ .input_mapping = evision_input_mapping,
+};
+module_hid_driver(evision_driver);
+
+MODULE_LICENSE("GPL");
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 0f8c11842..7ae89aebd 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -445,6 +445,9 @@
#define USB_VENDOR_ID_EMS 0x2006
#define USB_DEVICE_ID_EMS_TRIO_LINKER_PLUS_II 0x0118
+#define USB_VENDOR_ID_EVISION 0x320f
+#define USB_DEVICE_ID_EVISION_ICL01 0x5041
+
#define USB_VENDOR_ID_FLATFROG 0x25b5
#define USB_DEVICE_ID_MULTITOUCH_3200 0x0002
--
2.39.1
^ permalink raw reply related
* Re: [PATCH 1/1] HID: evision: Add preliminary support for EVision keyboards
From: Philippe Valembois @ 2023-01-25 21:19 UTC (permalink / raw)
To: Jiri Kosina; +Cc: Benjamin Tissoires, linux-kernel, linux-input
In-Reply-To: <nycvar.YFH.7.76.2301251425190.1734@cbobk.fhfr.pm>
Hello,
It is a blackhole address indeed.
I just sent a v3 with my corrected email address.
Thanks,
Regards,
Philippe Valembois
Le mer. 25 janv. 2023 à 14:27, Jiri Kosina <jikos@kernel.org> a écrit :
>
> On Mon, 23 Jan 2023, Benjamin Tissoires wrote:
>
> > > From: Philippe Valembois <lephilousophe@users.noreply.github.com>
> >
> > Jiri, I have a doubt. Do we accept emails from users.noreply.github.com?
>
> From quick search in github documentation, this seems to be a blackhole
> email address, right?
>
> If so, I'd much prefer to have a real e-mail address there, in order to
> make sure the author / signoff holders can be reached in the future if
> there are any concerns regarding the patch.
>
> The e-mail address disclosure concern is moot as soon as the emaill with
> the patch hits LKML anyway.
>
> Thanks,
>
> --
> Jiri Kosina
> SUSE Labs
>
^ permalink raw reply
* Linux 6.1 and 6.2-rc make mousewheel on Logitech G903 (046d:c091) report too many non-hires events
From: Tobias Klausmann @ 2023-01-25 22:01 UTC (permalink / raw)
To: linux-input
Hi!
As it says in the subject.
At some point between 6.0 and 6.1, the kernel (if HID_LOGITECH_HIDPP was
m or y) started reporting a full event for every hires event on a
Logitech G903.
In my quest of finding out what the root cause is, ive filed these bugs:
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1029584
https://gitlab.freedesktop.org/libinput/libinput/-/issues/852
The libinput one has the most detail. The most relevant one is this log
from libinput record, showing what happens if I turn the mousewheel by
one notch:
events:
# Current time is 20:37:11
- evdev:
- [ 3, 63476, 2, 8, -1] # EV_REL / REL_WHEEL -1
- [ 3, 63476, 2, 11, -120] # EV_REL / REL_WHEEL_HI_RES -120
- [ 3, 63476, 0, 0, 0] # ------------ SYN_REPORT (0) ---------- +3063ms
- evdev:
- [ 3, 139476, 2, 8, -1] # EV_REL / REL_WHEEL -1
- [ 3, 139476, 2, 11, -120] # EV_REL / REL_WHEEL_HI_RES -120
- [ 3, 139476, 0, 0, 0] # ------------ SYN_REPORT (0) ---------- +76ms
- evdev:
- [ 3, 197482, 2, 8, -1] # EV_REL / REL_WHEEL -1
- [ 3, 197482, 2, 11, -120] # EV_REL / REL_WHEEL_HI_RES -120
- [ 3, 197482, 0, 0, 0] # ------------ SYN_REPORT (0) ---------- +58ms
- evdev:
- [ 3, 227485, 2, 8, -1] # EV_REL / REL_WHEEL -1
- [ 3, 227485, 2, 11, -120] # EV_REL / REL_WHEEL_HI_RES -120
- [ 3, 227485, 0, 0, 0] # ------------ SYN_REPORT (0) ---------- +30ms
- evdev:
- [ 3, 240484, 2, 8, -1] # EV_REL / REL_WHEEL -1
- [ 3, 240484, 2, 11, -120] # EV_REL / REL_WHEEL_HI_RES -120
- [ 3, 240484, 0, 0, 0] # ------------ SYN_REPORT (0) ---------- +13ms
- evdev:
- [ 3, 250470, 2, 8, -1] # EV_REL / REL_WHEEL -1
- [ 3, 250470, 2, 11, -120] # EV_REL / REL_WHEEL_HI_RES -120
- [ 3, 250470, 0, 0, 0] # ------------ SYN_REPORT (0) ---------- +10ms
- evdev:
- [ 3, 256486, 2, 8, -1] # EV_REL / REL_WHEEL -1
- [ 3, 256486, 2, 11, -120] # EV_REL / REL_WHEEL_HI_RES -120
- [ 3, 256486, 0, 0, 0] # ------------ SYN_REPORT (0) ---------- +6ms
- evdev:
- [ 3, 264472, 2, 8, -1] # EV_REL / REL_WHEEL -1
- [ 3, 264472, 2, 11, -120] # EV_REL / REL_WHEEL_HI_RES -120
- [ 3, 264472, 0, 0, 0] # ------------ SYN_REPORT (0) ---------- +8ms
# Current time is 20:37:15
Note how it reports both REL_WHEEL and REL_WHEEL_HI_RES at every
instance.
I've tried assorted kernels, both Debian as well as hand-compiled
vanilla ones, and the problem only shows up if hid_logitech_hidpp.ko is
loaded, so I am blacklisting it for now. Also note that even unloading
the module didn't fix the behavior, though that may be because X11 was
still runing and so libinput's state ight have been still mangled.
The most recent kernels I have tested with are 6.1.4 and 6.2-rc5.
Best,
Tobias
--
printk(KERN_ERR "BIGMAC: Would you like that for here or to go?\n");
linux-2.6.19/drivers/net/sunbmac.c
^ permalink raw reply
* [PATCH] dt-bindings: input: azoteq: Fix differing types
From: Rob Herring @ 2023-01-25 22:14 UTC (permalink / raw)
To: Dmitry Torokhov, Krzysztof Kozlowski, Jeff LaBundy
Cc: linux-input, devicetree, linux-kernel
'azoteq,ati-base' and 'azoteq,thresh' properties are defined in multiple
bindings, but have differing types defined. Both 'uint32' and
'uint32-array' are used. Unify these to use 'uint32-array' everywhere.
Signed-off-by: Rob Herring <robh@kernel.org>
---
.../bindings/input/azoteq,iqs7222.yaml | 12 ++++---
.../devicetree/bindings/input/iqs269a.yaml | 34 +++++++++++--------
.../devicetree/bindings/input/iqs626a.yaml | 12 ++++---
3 files changed, 33 insertions(+), 25 deletions(-)
diff --git a/Documentation/devicetree/bindings/input/azoteq,iqs7222.yaml b/Documentation/devicetree/bindings/input/azoteq,iqs7222.yaml
index 9ddba7f2e7aa..f2382a56884d 100644
--- a/Documentation/devicetree/bindings/input/azoteq,iqs7222.yaml
+++ b/Documentation/devicetree/bindings/input/azoteq,iqs7222.yaml
@@ -354,10 +354,11 @@ patternProperties:
description: Specifies the channel's ATI target.
azoteq,ati-base:
- $ref: /schemas/types.yaml#/definitions/uint32
- multipleOf: 16
- minimum: 0
- maximum: 496
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+ items:
+ - multipleOf: 16
+ minimum: 0
+ maximum: 496
description: Specifies the channel's ATI base.
azoteq,ati-mode:
@@ -440,7 +441,8 @@ patternProperties:
slider gesture).
azoteq,thresh:
- $ref: /schemas/types.yaml#/definitions/uint32
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+ maxItems: 1
description:
Specifies the threshold for the event. Valid entries range from
0-127 and 0-255 for proximity and touch events, respectively.
diff --git a/Documentation/devicetree/bindings/input/iqs269a.yaml b/Documentation/devicetree/bindings/input/iqs269a.yaml
index 3c430d38594f..4fa20f0f6847 100644
--- a/Documentation/devicetree/bindings/input/iqs269a.yaml
+++ b/Documentation/devicetree/bindings/input/iqs269a.yaml
@@ -334,9 +334,10 @@ patternProperties:
3: Full
azoteq,ati-base:
- $ref: /schemas/types.yaml#/definitions/uint32
- enum: [75, 100, 150, 200]
- default: 100
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+ items:
+ - enum: [75, 100, 150, 200]
+ default: 100
description: Specifies the channel's ATI base.
azoteq,ati-target:
@@ -391,10 +392,11 @@ patternProperties:
properties:
azoteq,thresh:
- $ref: /schemas/types.yaml#/definitions/uint32
- minimum: 0
- maximum: 255
- default: 10
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+ items:
+ - minimum: 0
+ maximum: 255
+ default: 10
description: Specifies the threshold for the event.
linux,code: true
@@ -408,10 +410,11 @@ patternProperties:
properties:
azoteq,thresh:
- $ref: /schemas/types.yaml#/definitions/uint32
- minimum: 0
- maximum: 255
- default: 8
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+ items:
+ - minimum: 0
+ maximum: 255
+ default: 8
description: Specifies the threshold for the event.
azoteq,hyst:
@@ -432,10 +435,11 @@ patternProperties:
properties:
azoteq,thresh:
- $ref: /schemas/types.yaml#/definitions/uint32
- minimum: 0
- maximum: 255
- default: 26
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+ items:
+ - minimum: 0
+ maximum: 255
+ default: 26
description: Specifies the threshold for the event.
azoteq,hyst:
diff --git a/Documentation/devicetree/bindings/input/iqs626a.yaml b/Documentation/devicetree/bindings/input/iqs626a.yaml
index 7a27502095f3..dbd63d48605c 100644
--- a/Documentation/devicetree/bindings/input/iqs626a.yaml
+++ b/Documentation/devicetree/bindings/input/iqs626a.yaml
@@ -234,8 +234,9 @@ patternProperties:
about the available RUI options.
azoteq,ati-base:
- $ref: /schemas/types.yaml#/definitions/uint32
- enum: [75, 100, 150, 200]
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+ items:
+ - enum: [75, 100, 150, 200]
description:
Specifies the channel's ATI base. The default value is a function
of the channel and the device's RUI.
@@ -475,9 +476,10 @@ patternProperties:
properties:
azoteq,thresh:
- $ref: /schemas/types.yaml#/definitions/uint32
- minimum: 0
- maximum: 255
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+ items:
+ - minimum: 0
+ maximum: 255
description: Specifies the threshold for the event.
azoteq,hyst:
--
2.39.0
^ permalink raw reply related
* [PATCH 1/5] HID: bigben_remove: manually unregister leds
From: Pietro Borrello @ 2023-01-26 0:24 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Hanno Zulla, Carlo Caione,
Pavel Machek, Lee Jones, Roderick Colenbrander, Sven Eckelmann
Cc: linux-leds, Cristiano Giuffrida, Bos, H.J., Jakob Koschel,
linux-input, linux-kernel, Jiri Kosina, Roderick Colenbrander,
Pietro Borrello
In-Reply-To: <20230125-hid-unregister-leds-v1-0-9a5192dcef16@diag.uniroma1.it>
Unregister the LED controllers before device removal, as
bigben_set_led() may schedule bigben->worker after the structure has
been freed, causing a use-after-free.
Fixes: 4eb1b01de5b9 ("HID: hid-bigbenff: fix race condition for scheduled work during removal")
Signed-off-by: Pietro Borrello <borrello@diag.uniroma1.it>
---
drivers/hid/hid-bigbenff.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/hid/hid-bigbenff.c b/drivers/hid/hid-bigbenff.c
index e8b16665860d..d3201b755595 100644
--- a/drivers/hid/hid-bigbenff.c
+++ b/drivers/hid/hid-bigbenff.c
@@ -306,9 +306,14 @@ static enum led_brightness bigben_get_led(struct led_classdev *led)
static void bigben_remove(struct hid_device *hid)
{
+ int n;
struct bigben_device *bigben = hid_get_drvdata(hid);
bigben->removed = true;
+ for (n = 0; n < NUM_LEDS; n++) {
+ if (bigben->leds[n])
+ devm_led_classdev_unregister(&hid->dev, bigben->leds[n]);
+ }
cancel_work_sync(&bigben->worker);
hid_hw_stop(hid);
}
--
2.25.1
^ permalink raw reply related
* [PATCH 0/5] HID: manually unregister leds on device removal to prevent UAFs
From: Pietro Borrello @ 2023-01-26 0:24 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Hanno Zulla, Carlo Caione,
Pavel Machek, Lee Jones, Roderick Colenbrander, Sven Eckelmann
Cc: linux-leds, Cristiano Giuffrida, Bos, H.J., Jakob Koschel,
linux-input, linux-kernel, Jiri Kosina, Roderick Colenbrander,
Pietro Borrello
I noticed a recurring pattern is present in multiple hid devices in the
Linux tree, where the LED controller of a device schedules a work_struct
to interact with the hardware.
The work_struct is embedded in the device structure and thus, is freed
at device removal.
The issue is that a LED worker may be scheduled by a timer concurrently
with device removal, causing the work_struct to be accessed after having
been freed.
I was able to trigger the issue in hid-bigbenff.c and hid-asus.c
where the work_structs may be scheduled by the LED controller
while the device is disconnecting, triggering use-after-frees.
I can attach the reproducer, but it's very simple USB configuration,
using the /dev/raw-gadget interface with some more USB interactions
to manage LEDs configuration and pass checks in asus_kbd_init()
and asus_kbd_get_functions() in case of hid-asus.c.
I triggered the issue by connecting a device and immediately
disconnecting it, so that the remove function runs before the LED one
which remains pending.
More drivers have the same pattern (hid-lg-g15.c, hid-playstation.c,
hid-sony.c) but I wasn't able to properly pass the right descriptors
to trigger the led configurations needed to trigger the workers.
Some other drivers manually unregister at removal (hid-corsair.c,
hid-gt683r.c, hid-lenovo.c) since they do not use the managed
interface, which is safe, to my understanding.
Also, a similar pattern is present with callbacks which schedule
a worker originating from input_ff_create_memless() (e.g.,
in hid-bigbenff.c) but in these cases, I wasn't able to trigger
the race condition with the event handling to schedule the worker
during device removal. However, I have no experience with the USB
protocol and I'm not able to say that they cannot be triggered.
I am currently wondering if this is due to some emulation of the
/dev/raw-gadget interface or if it's effectively an issue with how each
device manages resource removal.
But I wonder why syzkaller didn't find any crash while fuzzing the
interface with upstream-usb.config, as they seem pretty
straightforward to trigger.
Configuring the kernel with CONFIG_DEBUG_OBJECTS, it emits
a warning in debug_check_no_obj_freed, which makes it clear that
device removal is freeing resources in use.
KASAN detects them as use-after-free.
I am attaching multiple patches for all the drivers I suspect the bug
is present.
The proposed patches unregister the LED controllers before removing the
device itself.
I attach the (partial for brevity) ODEBUG dumps:
```hid-bigbenff.c
[ 37.803135][ T1170] usb 1-1: USB disconnect, device number 2
[ 37.827979][ T1170] ODEBUG: free active (active state 0) object
type: work_struct hint: bigben_worker+0x0/0x860
[ 37.829634][ T1170] WARNING: CPU: 0 PID: 1170 at
lib/debugobjects.c:505 debug_check_no_obj_freed+0x43a/0x630
[ 37.830904][ T1170] Modules linked in:
[ 37.831413][ T1170] CPU: 0 PID: 1170 Comm: kworker/0:3 Not tainted
6.1.0-rc4-dirty #43
[ 37.832465][ T1170] Hardware name: QEMU Standard PC (i440FX + PIIX,
1996), BIOS 1.13.0-1ubuntu1.1 04/01/2014
[ 37.833751][ T1170] Workqueue: usb_hub_wq hub_event
[ 37.834409][ T1170] RIP: 0010:debug_check_no_obj_freed+0x43a/0x630
[ 37.835218][ T1170] Code: 48 89 ef e8 28 82 58 ff 49 8b 14 24 4c 8b
45 00 48 c7 c7 40 5f 09 87 48 c7 c6 60 5b 09 87 89 d9 4d 89 f9 31 c0
e8 46 25 ef fe <0f> 0b 4c 8b 64 24 20 48 ba 00 00 00 00 00 fc ff df ff
05 4f 7c 17
[ 37.837667][ T1170] RSP: 0018:ffffc900006fee60 EFLAGS: 00010246
[ 37.838503][ T1170] RAX: 0d2d19ffcded3d00 RBX: 0000000000000000
RCX: ffff888117fc9b00
[ 37.839519][ T1170] RDX: 0000000000000000 RSI: 0000000000000000
RDI: 0000000000000000
[ 37.840570][ T1170] RBP: ffffffff86e88380 R08: ffffffff8130793b
R09: fffff520000dfd85
[ 37.841618][ T1170] R10: fffff520000dfd85 R11: 0000000000000000
R12: ffffffff87095fb8
[ 37.842649][ T1170] R13: ffff888117770ad8 R14: ffff888117770acc
R15: ffffffff852b7420
[ 37.843728][ T1170] FS: 0000000000000000(0000)
GS:ffff8881f6600000(0000) knlGS:0000000000000000
[ 37.844877][ T1170] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 37.845749][ T1170] CR2: 00007f992eaab380 CR3: 000000011834b000
CR4: 00000000001006f0
[ 37.846794][ T1170] Call Trace:
[ 37.847245][ T1170] <TASK>
[ 37.847643][ T1170] slab_free_freelist_hook+0x89/0x160
[ 37.848409][ T1170] ? devres_release_all+0x262/0x350
[ 37.849156][ T1170] __kmem_cache_free+0x71/0x110
[ 37.849829][ T1170] devres_release_all+0x262/0x350
[ 37.850478][ T1170] ? devres_release+0x90/0x90
[ 37.851118][ T1170] device_release_driver_internal+0x5e5/0x8a0
[ 37.851944][ T1170] bus_remove_device+0x2ea/0x400
[ 37.852611][ T1170] device_del+0x64f/0xb40
[ 37.853212][ T1170] ? kill_device+0x150/0x150
[ 37.853831][ T1170] ? print_irqtrace_events+0x1f0/0x1f0
[ 37.854564][ T1170] hid_destroy_device+0x66/0x100
[ 37.855226][ T1170] usbhid_disconnect+0x9a/0xc0
[ 37.855887][ T1170] usb_unbind_interface+0x1e1/0x890
```
``` hid-asus.c
[ 77.409878][ T1169] usb 1-1: USB disconnect, device number 2
[ 77.423606][ T1169] ODEBUG: free active (active state 0) object
type: work_struct hint: asus_kbd_backlight_work+0x0/0x2c0
[ 77.425222][ T1169] WARNING: CPU: 0 PID: 1169 at
lib/debugobjects.c:505 debug_check_no_obj_freed+0x43a/0x630
[ 77.426599][ T1169] Modules linked in:
[ 77.427322][ T1169] CPU: 0 PID: 1169 Comm: kworker/0:3 Not tainted
6.1.0-rc4-dirty #43
[ 77.428404][ T1169] Hardware name: QEMU Standard PC (i440FX + PIIX,
1996), BIOS 1.13.0-1ubuntu1.1 04/01/2014
[ 77.429644][ T1169] Workqueue: usb_hub_wq hub_event
[ 77.430296][ T1169] RIP: 0010:debug_check_no_obj_freed+0x43a/0x630
[ 77.431142][ T1169] Code: 48 89 ef e8 28 82 58 ff 49 8b 14 24 4c 8b
45 00 48 c7 c7 40 5f 09 87 48 c7 c6 60 5b 09 87 89 d9 4d 89 f9 31 c0
e8 46 25 ef fe <0f> 0b 4c 8b 64 24 20 48 ba 00 00 00 00 00 fc ff df ff
05 4f 7c 17
[ 77.433691][ T1169] RSP: 0018:ffffc9000069ee60 EFLAGS: 00010246
[ 77.434470][ T1169] RAX: b85d2b40c12d7600 RBX: 0000000000000000
RCX: ffff888117a78000
[ 77.435507][ T1169] RDX: 0000000000000000 RSI: 0000000080000000
RDI: 0000000000000000
[ 77.436521][ T1169] RBP: ffffffff86e88380 R08: ffffffff8130793b
R09: ffffed103ecc4ed6
[ 77.437582][ T1169] R10: ffffed103ecc4ed6 R11: 0000000000000000
R12: ffffffff87095fb8
[ 77.438593][ T1169] R13: ffff88810e348fe0 R14: ffff88810e348fd4
R15: ffffffff852b5780
[ 77.439667][ T1169] FS: 0000000000000000(0000)
GS:ffff8881f6600000(0000) knlGS:0000000000000000
[ 77.440842][ T1169] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 77.441688][ T1169] CR2: 00007ffc05495ff0 CR3: 000000010cdf0000
CR4: 00000000001006f0
[ 77.442720][ T1169] Call Trace:
[ 77.443167][ T1169] <TASK>
[ 77.443555][ T1169] slab_free_freelist_hook+0x89/0x160
[ 77.444302][ T1169] ? devres_release_all+0x262/0x350
[ 77.444990][ T1169] __kmem_cache_free+0x71/0x110
[ 77.445638][ T1169] devres_release_all+0x262/0x350
[ 77.446309][ T1169] ? devres_release+0x90/0x90
[ 77.446978][ T1169] device_release_driver_internal+0x5e5/0x8a0
[ 77.447748][ T1169] bus_remove_device+0x2ea/0x400
[ 77.448421][ T1169] device_del+0x64f/0xb40
[ 77.448976][ T1169] ? kill_device+0x150/0x150
[ 77.449577][ T1169] ? print_irqtrace_events+0x1f0/0x1f0
[ 77.450307][ T1169] hid_destroy_device+0x66/0x100
[ 77.450938][ T1169] usbhid_disconnect+0x9a/0xc0
```
To: Jiri Kosina <jikos@kernel.org>
To: Benjamin Tissoires <benjamin.tissoires@redhat.com>
To: Hanno Zulla <kontakt@hanno.de>
To: Carlo Caione <carlo@endlessm.com>
To: Pavel Machek <pavel@ucw.cz>
To: Lee Jones <lee@kernel.org>
To: Roderick Colenbrander <roderick.colenbrander@sony.com>
To: Sven Eckelmann <sven@narfation.org>
Cc: linux-leds@vger.kernel.org
Cc: Cristiano Giuffrida <c.giuffrida@vu.nl>
Cc: "Bos, H.J." <h.j.bos@vu.nl>
Cc: Jakob Koschel <jkl820.git@gmail.com>
Cc: linux-input@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: Jiri Kosina <jkosina@suse.cz>
Cc: Roderick Colenbrander <roderick@gaikai.com>
Signed-off-by: Pietro Borrello <borrello@diag.uniroma1.it>
---
Pietro Borrello (5):
HID: bigben_remove: manually unregister leds
HID: asus_remove: manually unregister led
HID: dualsense_remove: manually unregister leds
HID: dualshock4_remove: manually unregister leds
HID: sony_remove: manually unregister leds
drivers/hid/hid-asus.c | 1 +
drivers/hid/hid-bigbenff.c | 5 +++++
drivers/hid/hid-playstation.c | 8 ++++++++
drivers/hid/hid-sony.c | 8 ++++++++
4 files changed, 22 insertions(+)
---
base-commit: 2241ab53cbb5cdb08a6b2d4688feb13971058f65
change-id: 20230125-hid-unregister-leds-4cbf67099e1d
Best regards,
--
Pietro Borrello <borrello@diag.uniroma1.it>
^ permalink raw reply
* [PATCH 2/5] HID: asus_remove: manually unregister led
From: Pietro Borrello @ 2023-01-26 0:24 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Hanno Zulla, Carlo Caione,
Pavel Machek, Lee Jones, Roderick Colenbrander, Sven Eckelmann
Cc: linux-leds, Cristiano Giuffrida, Bos, H.J., Jakob Koschel,
linux-input, linux-kernel, Jiri Kosina, Roderick Colenbrander,
Pietro Borrello
In-Reply-To: <20230125-hid-unregister-leds-v1-0-9a5192dcef16@diag.uniroma1.it>
Unregister the LED controller before device removal, as
asus_kbd_backlight_set() may schedule led->work after the structure
has been freed, causing a use-after-free.
Fixes: af22a610bc38 ("HID: asus: support backlight on USB keyboards")
Signed-off-by: Pietro Borrello <borrello@diag.uniroma1.it>
---
drivers/hid/hid-asus.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index f99752b998f3..0f274c8d1bef 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -1122,6 +1122,7 @@ static void asus_remove(struct hid_device *hdev)
if (drvdata->kbd_backlight) {
drvdata->kbd_backlight->removed = true;
+ devm_led_classdev_unregister(&hdev->dev, &drvdata->kbd_backlight->cdev);
cancel_work_sync(&drvdata->kbd_backlight->work);
}
--
2.25.1
^ permalink raw reply related
* [PATCH 3/5] HID: dualsense_remove: manually unregister leds
From: Pietro Borrello @ 2023-01-26 0:24 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Hanno Zulla, Carlo Caione,
Pavel Machek, Lee Jones, Roderick Colenbrander, Sven Eckelmann
Cc: linux-leds, Cristiano Giuffrida, Bos, H.J., Jakob Koschel,
linux-input, linux-kernel, Jiri Kosina, Roderick Colenbrander,
Pietro Borrello
In-Reply-To: <20230125-hid-unregister-leds-v1-0-9a5192dcef16@diag.uniroma1.it>
Unregister the LED controller before device removal, as
dualsense_player_led_set_brightness() may schedule output_worker
after the structure has been freed, causing a use-after-free.
Fixes: 8c0ab553b072 ("HID: playstation: expose DualSense player LEDs through LED class.")
Signed-off-by: Pietro Borrello <borrello@diag.uniroma1.it>
---
drivers/hid/hid-playstation.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
index 27c40894acab..9e23860b7e95 100644
--- a/drivers/hid/hid-playstation.c
+++ b/drivers/hid/hid-playstation.c
@@ -1503,11 +1503,15 @@ static void dualsense_remove(struct ps_device *ps_dev)
{
struct dualsense *ds = container_of(ps_dev, struct dualsense, base);
unsigned long flags;
+ int i;
spin_lock_irqsave(&ds->base.lock, flags);
ds->output_worker_initialized = false;
spin_unlock_irqrestore(&ds->base.lock, flags);
+ for (i = 0; i < ARRAY_SIZE(ds->player_leds); i++)
+ devm_led_classdev_unregister(&ps_dev->hdev->dev, &ds->player_leds[i]);
+
cancel_work_sync(&ds->output_worker);
}
--
2.25.1
^ 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