mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
To: Jiri Kosina <jikos@kernel.org>,
	Benjamin Tissoires <bentiss@kernel.org>,
	 Gideon Heinz <gideonheinz02@gmail.com>
Cc: "Filipe Laíns" <lains@riseup.net>,
	"Bastien Nocera" <hadess@hadess.net>,
	"Hans de Goede" <hansg@kernel.org>,
	"Edwin Velds" <e.velds@gmail.com>,
	"Stuart Hayhurst" <stuart.a.hayhurst@gmail.com>,
	"Andrey Smirnov" <andrew.smirnov@gmail.com>,
	linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
	stable@vger.kernel.org
Subject: [PATCH v2 00/33] HID: logitech-hidpp: G920 force-feedback fixes, rework, and cleanups
Date: Tue, 10 Nov 2026 22:47:58 -0800	[thread overview]
Message-ID: <20261110-hid-logitech-hidpp-v2-0-30743e91d83b@gmail.com> (raw)

This series starts from Gideon Heinz's report of Logitech G920
force-feedback initialization timing out in v7.3 and addresses several
additional pre-existing bugs, lifetime races, and cleanup opportunities
found while auditing hid-logitech-hidpp.

The first 14 patches carry Fixes and stable tags. They fix the v7.3
G920 force-feedback initialization timeout by temporarily enabling
device I/O around g920_get_config() in hidpp_input_configured(), switch
hidpp_ff_queue_work() to GFP_ATOMIC since playback, gain, and autocenter
callbacks run under the input event spinlock, drain the G920 FF
workqueue before closing the HID transport on input close so queued
commands still receive their USB interrupt IN replies, synchronously
disable driver work items and disconnect input devices with device I/O
enabled before stopping the transport on removal and probe failure,
avoid storing an ERR_PTR in battery->ps on registration failure, avoid
NULL pointer subtraction in the HID++ 1.0 consumer keys report fixup,
and fix an off-by-one in G920 slot tracking that made the 63rd user
effect in hardware slot 64 unusable and leaked it across reopen.

Additionally in the bug fix section, hidpp_send_message_sync() is
updated to stop retrying non-busy errors (timeouts and transport
failures) so failed commands fail promptly without 15-second stalls,
and response buffers are initialized before publishing send_receive_buf
to eliminate races with concurrent or spurious packets. Battery fixes
correct HID++ 1.0 battery mileage event parsing to read capacity from
the correct report byte and update state, properly propagate negative
transport and timeout errors in ADC measurement (0x1f20) queries, mark
devices online when receiving unified battery (0x1004) events, fix K750
solar battery feature lookup and reconnect handling, and expose the
charge_type property for the battery voltage (0x1001) feature.

The next section prepares transport handling and reworks G920 force
feedback for next. Connect-event work is kept disabled in probe until
hid_connect() completes successfully. Transport error codes are
preserved in __hidpp_send_report(), and hidpp_send_message_sync()
is switched to return standard negative error codes on failure.
G920 capability discovery moves back into hidpp_probe() while device
I/O is active (keeping force feedback optional on failure and
excluding generic USB PID force feedback), device-level wheel settings
are separated from the input FF queue lifetime, the G920 range sysfs
attribute moves to driver dev_groups, and FF workqueue teardown moves
into ff->stop so open evdev handles across USB disconnect or unbind
no longer cause use-after-free.

Furthermore, G920 range store, effect upload, and effect erase are
converted to execute synchronously with proper transport lifetime
handling, avoiding asynchronous command queueing and work flushing
during device close and teardown. Effect slot lookup is streamlined by
indexing slot arrays directly by effect ID instead of performing linear
scans.

The final section modernizes the driver. Multi-byte packet fields are
switched to unaligned accessors, the device serial is stored as a byte
array, and resource management is updated to use guard(mutex),
__free(kfree), alloc_ordered_workqueue(), devm_mutex_init(), and
devm_device_add_group(). Lastly, unused kfifo fields and helpers
are removed.

Note that device removal teardown still has limitations due to how HID
core's hid_device_io_start() and hid_device_io_stop() (via
driver_input_lock) gate both driver raw command/response I/O and
subdriver event forwarding, disabling I/O before .remove() and
hid_disconnect(). For instance, HIDPP_QUIRK_DELAYED_INIT devm-managed
input devices are still unregistered after hid_hw_stop(), and
hidpp_connect_event() publishes hidpp->input before input registration
completes. Resolving these teardown edge cases cleanly will require
follow-up work in HID core and driver input registration.

Thanks.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
Changes in v2:
- Rebased the series on top of linux-next.
- Collected Tested-by tags from Gideon Heinz for 17 v1 patches (omitted
  from patch 4 where hidpp_remove() was significantly altered).
- Reorganized the series to place all 14 stable bug fixes upfront,
  followed by next refactorings and modernizations.
- Updated hidpp_remove() to enable device I/O while draining work
  items and disconnecting hidinput so in-flight transfers and
  force-feedback teardown can receive device replies (noting that
  complete resolution of delayed-init and transport teardown ordering
  requires follow-up work in HID core).
- Stopped retrying non-busy errors (timeouts and negative transport
  failures) in hidpp_send_message_sync().
- Initialized response buffers before publishing send_receive_buf to
  prevent stale response reads on concurrent or spurious events.
- Fixed HID++ 1.0 battery mileage event parsing to read capacity from
  the correct report byte and update battery state.
- Handled negative transport and timeout errors properly in ADC
  measurement (0x1f20) queries.
- Marked devices online when receiving unified battery (0x1004) events.
- Fixed K750 solar battery feature lookup and reconnect handling.
- Exposed charge_type property for battery voltage (0x1001) feature.
- Kept connect work disabled during probe until hid_connect() succeeds.
- Switched hidpp_send_message_sync() to translate RAP and FAP protocol
  error codes into negative Linux error codes (-EBUSY, -EIO, -EPROTO)
  instead of returning positive protocol values, and preserved transport
  error codes in __hidpp_send_report().
- Converted G920 range store, effect upload, and effect erase to
  synchronous execution with slot indexing by effect ID.
- Added G920 force-feedback helper cleanups, removed unused kfifo and
  struct wtp_data fields, and cleaned up minor helpers.
- Link to v1: https://patch.msgid.link/20261109061103.2887636-1-dmitry.torokhov@gmail.com

---
Dmitry Torokhov (33):
      HID: logitech-hidpp: fix G920 force-feedback initialization timeout
      HID: logitech-hidpp: use GFP_ATOMIC in hidpp_ff_queue_work()
      HID: logitech-hidpp: drain G920 FF workqueue before closing transport
      HID: logitech-hidpp: disable works before stopping transport
      HID: logitech-hidpp: do not store ERR_PTR in battery->ps
      HID: logitech-hidpp: avoid NULL subtraction in consumer key report fixup
      HID: logitech-hidpp: fix off-by-one in G920 force-feedback slot tracking
      HID: logitech-hidpp: do not retry timeouts and transport errors in hidpp_send_message_sync()
      HID: logitech-hidpp: initialize response buffer before publishing send_receive_buf
      HID: logitech-hidpp: fix HID++ 1.0 battery mileage event parsing
      HID: logitech-hidpp: handle negative errors in hidpp20_get_adc_measurement_1f20()
      HID: logitech-hidpp: mark device online on unified battery 0x1004 events
      HID: logitech-hidpp: fix K750 solar battery feature lookup and reconnect
      HID: logitech-hidpp: expose charge_type property for battery voltage feature
      HID: logitech-hidpp: keep connect work disabled until hid_connect() completes
      HID: logitech-hidpp: preserve transport error codes in __hidpp_send_report()
      HID: logitech-hidpp: return negative error codes from hidpp_send_message_sync()
      HID: logitech-hidpp: rework G920 force-feedback initialization
      HID: logitech-hidpp: use sysfs_emit() in hidpp_ff_range_show()
      HID: logitech-hidpp: manage G920 range sysfs attribute via dev_groups
      HID: logitech-hidpp: execute G920 range sysfs store synchronously
      HID: logitech-hidpp: shut down G920 force feedback on input unregister
      HID: logitech-hidpp: index G920 force-feedback slots by effect ID
      HID: logitech-hidpp: execute G920 effect upload and erase synchronously
      HID: logitech-hidpp: clean up G920 force-feedback helpers
      HID: logitech-hidpp: use unaligned accessors for multi-byte values
      HID: logitech-hidpp: store device serial as a byte array
      HID: logitech-hidpp: use guard() for send_mutex
      HID: logitech-hidpp: use __free(kfree) for temporary allocations
      HID: logitech-hidpp: use alloc_ordered_workqueue() for FF queue
      HID: logitech-hidpp: manage send_mutex with devm_mutex_init()
      HID: logitech-hidpp: manage ps_attribute_group with devm_device_add_group()
      HID: logitech-hidpp: remove unused kfifo and clean up minor helpers

 drivers/hid/hid-logitech-hidpp.c | 897 ++++++++++++++++++---------------------
 1 file changed, 411 insertions(+), 486 deletions(-)
---
base-commit: 630059a5a632377b9b7b487baccde92b7ae1dd47
change-id: 20261110-hid-logitech-hidpp-6884bfb6a348

Thanks.

-- 
Dmitry


             reply	other threads:[~2026-11-11  6:56 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-11  6:47 Dmitry Torokhov [this message]
2026-11-11  6:47 ` [PATCH v2 01/33] HID: logitech-hidpp: fix G920 force-feedback initialization timeout Dmitry Torokhov
2026-11-11  6:48 ` [PATCH v2 02/33] HID: logitech-hidpp: use GFP_ATOMIC in hidpp_ff_queue_work() Dmitry Torokhov
2026-11-11  6:48 ` [PATCH v2 03/33] HID: logitech-hidpp: drain G920 FF workqueue before closing transport Dmitry Torokhov
2026-11-11  6:48 ` [PATCH v2 04/33] HID: logitech-hidpp: disable works before stopping transport Dmitry Torokhov
2026-11-11  6:48 ` [PATCH v2 05/33] HID: logitech-hidpp: do not store ERR_PTR in battery->ps Dmitry Torokhov
2026-11-11  6:48 ` [PATCH v2 06/33] HID: logitech-hidpp: avoid NULL subtraction in consumer key report fixup Dmitry Torokhov
2026-11-11  6:48 ` [PATCH v2 07/33] HID: logitech-hidpp: fix off-by-one in G920 force-feedback slot tracking Dmitry Torokhov
2026-11-11  6:48 ` [PATCH v2 08/33] HID: logitech-hidpp: do not retry timeouts and transport errors in hidpp_send_message_sync() Dmitry Torokhov
2026-11-11  6:48 ` [PATCH v2 09/33] HID: logitech-hidpp: initialize response buffer before publishing send_receive_buf Dmitry Torokhov
2026-11-11  6:48 ` [PATCH v2 10/33] HID: logitech-hidpp: fix HID++ 1.0 battery mileage event parsing Dmitry Torokhov
2026-11-11  6:48 ` [PATCH v2 11/33] HID: logitech-hidpp: handle negative errors in hidpp20_get_adc_measurement_1f20() Dmitry Torokhov
2026-11-11  6:48 ` [PATCH v2 12/33] HID: logitech-hidpp: mark device online on unified battery 0x1004 events Dmitry Torokhov
2026-11-11  6:48 ` [PATCH v2 13/33] HID: logitech-hidpp: fix K750 solar battery feature lookup and reconnect Dmitry Torokhov
2026-11-11  6:48 ` [PATCH v2 14/33] HID: logitech-hidpp: expose charge_type property for battery voltage feature Dmitry Torokhov
2026-11-11  6:48 ` [PATCH v2 15/33] HID: logitech-hidpp: keep connect work disabled until hid_connect() completes Dmitry Torokhov
2026-11-11  6:48 ` [PATCH v2 16/33] HID: logitech-hidpp: preserve transport error codes in __hidpp_send_report() Dmitry Torokhov
2026-11-11  6:48 ` [PATCH v2 17/33] HID: logitech-hidpp: return negative error codes from hidpp_send_message_sync() Dmitry Torokhov
2026-11-11  6:48 ` [PATCH v2 18/33] HID: logitech-hidpp: rework G920 force-feedback initialization Dmitry Torokhov
2026-11-11  6:48 ` [PATCH v2 19/33] HID: logitech-hidpp: use sysfs_emit() in hidpp_ff_range_show() Dmitry Torokhov
2026-11-11  6:48 ` [PATCH v2 20/33] HID: logitech-hidpp: manage G920 range sysfs attribute via dev_groups Dmitry Torokhov
2026-11-11  6:48 ` [PATCH v2 21/33] HID: logitech-hidpp: execute G920 range sysfs store synchronously Dmitry Torokhov
2026-11-11  6:48 ` [PATCH v2 22/33] HID: logitech-hidpp: shut down G920 force feedback on input unregister Dmitry Torokhov
2026-11-11  6:48 ` [PATCH v2 23/33] HID: logitech-hidpp: index G920 force-feedback slots by effect ID Dmitry Torokhov
2026-11-11  6:48 ` [PATCH v2 24/33] HID: logitech-hidpp: execute G920 effect upload and erase synchronously Dmitry Torokhov
2026-11-11  6:48 ` [PATCH v2 25/33] HID: logitech-hidpp: clean up G920 force-feedback helpers Dmitry Torokhov
2026-11-11  6:48 ` [PATCH v2 26/33] HID: logitech-hidpp: use unaligned accessors for multi-byte values Dmitry Torokhov
2026-11-11  6:48 ` [PATCH v2 27/33] HID: logitech-hidpp: store device serial as a byte array Dmitry Torokhov
2026-11-11  6:48 ` [PATCH v2 28/33] HID: logitech-hidpp: use guard() for send_mutex Dmitry Torokhov
2026-11-11  6:48 ` [PATCH v2 29/33] HID: logitech-hidpp: use __free(kfree) for temporary allocations Dmitry Torokhov
2026-11-11  6:48 ` [PATCH v2 30/33] HID: logitech-hidpp: use alloc_ordered_workqueue() for FF queue Dmitry Torokhov
2026-11-11  6:48 ` [PATCH v2 31/33] HID: logitech-hidpp: manage send_mutex with devm_mutex_init() Dmitry Torokhov
2026-11-11  6:48 ` [PATCH v2 32/33] HID: logitech-hidpp: manage ps_attribute_group with devm_device_add_group() Dmitry Torokhov
2026-11-11  6:48 ` [PATCH v2 33/33] HID: logitech-hidpp: remove unused kfifo and clean up minor helpers Dmitry Torokhov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261110-hid-logitech-hidpp-v2-0-30743e91d83b@gmail.com \
    --to=dmitry.torokhov@gmail.com \
    --cc=andrew.smirnov@gmail.com \
    --cc=bentiss@kernel.org \
    --cc=e.velds@gmail.com \
    --cc=gideonheinz02@gmail.com \
    --cc=hadess@hadess.net \
    --cc=hansg@kernel.org \
    --cc=jikos@kernel.org \
    --cc=lains@riseup.net \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=stuart.a.hayhurst@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®