| Message ID | 20200710151939.4894-1-grant.likely@arm.com (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | 4f57cace81438cc873a96f9f13f08298815c9b51 |
| Delegated to: | Jiri Kosina |
| Headers | show |
| Series | hid-input: Fix devices that return multiple bytes in battery report | expand |
On Fri, Jul 10, 2020 at 8:19 AM Grant Likely <grant.likely@secretlab.ca> wrote: > > Some devices, particularly the 3DConnexion Spacemouse wireless 3D > controllers, return more than just the battery capacity in the battery > report. The Spacemouse devices return an additional byte with a device > specific field. However, hidinput_query_battery_capacity() only > requests a 2 byte transfer. > > When a spacemouse is connected via USB (direct wire, no wireless dongle) > and it returns a 3 byte report instead of the assumed 2 byte battery > report the larger transfer confuses and frightens the USB subsystem > which chooses to ignore the transfer. Then after 2 seconds assume the > device has stopped responding and reset it. This can be reproduced > easily by using a wired connection with a wireless spacemouse. The > Spacemouse will enter a loop of resetting every 2 seconds which can be > observed in dmesg. > > This patch solves the problem by increasing the transfer request to 4 > bytes instead of 2. The fix isn't particularly elegant, but it is simple > and safe to backport to stable kernels. A further patch will follow to > more elegantly handle battery reports that contain additional data. > Applied and tested on 5.8.0-rc4+ (aa0c9086b40c) with a 3Dconnexion SpaceMouse Wireless (tested connected via USB). Observed the same behavior Grant reports before the patch. After the patch, the device stays connected successfully. Tested-by: Darren Hart <dvhart@infradead.org> Thanks Grant! > Signed-off-by: Grant Likely <grant.likely@secretlab.ca> > Cc: Darren Hart <darren@dvhart.com> > Cc: Jiri Kosina <jikos@kernel.org> > Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com> > Cc: stable@vger.kernel.org > --- > drivers/hid/hid-input.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c > index dea9cc65bf80..e8641ce677e4 100644 > --- a/drivers/hid/hid-input.c > +++ b/drivers/hid/hid-input.c > @@ -350,13 +350,13 @@ static int hidinput_query_battery_capacity(struct hid_device *dev) > u8 *buf; > int ret; > > - buf = kmalloc(2, GFP_KERNEL); > + buf = kmalloc(4, GFP_KERNEL); > if (!buf) > return -ENOMEM; > > - ret = hid_hw_raw_request(dev, dev->battery_report_id, buf, 2, > + ret = hid_hw_raw_request(dev, dev->battery_report_id, buf, 4, > dev->battery_report_type, HID_REQ_GET_REPORT); > - if (ret != 2) { > + if (ret < 2) { > kfree(buf); > return -ENODATA; > } > -- > 2.20.1 >
On Fri, 10 Jul 2020, Darren Hart wrote: > > Some devices, particularly the 3DConnexion Spacemouse wireless 3D > > controllers, return more than just the battery capacity in the battery > > report. The Spacemouse devices return an additional byte with a device > > specific field. However, hidinput_query_battery_capacity() only > > requests a 2 byte transfer. > > > > When a spacemouse is connected via USB (direct wire, no wireless dongle) > > and it returns a 3 byte report instead of the assumed 2 byte battery > > report the larger transfer confuses and frightens the USB subsystem > > which chooses to ignore the transfer. Then after 2 seconds assume the > > device has stopped responding and reset it. This can be reproduced > > easily by using a wired connection with a wireless spacemouse. The > > Spacemouse will enter a loop of resetting every 2 seconds which can be > > observed in dmesg. > > > > This patch solves the problem by increasing the transfer request to 4 > > bytes instead of 2. The fix isn't particularly elegant, but it is simple > > and safe to backport to stable kernels. A further patch will follow to > > more elegantly handle battery reports that contain additional data. > > > > Applied and tested on 5.8.0-rc4+ (aa0c9086b40c) with a 3Dconnexion > SpaceMouse Wireless (tested connected via USB). Observed the same > behavior Grant reports before the patch. After the patch, the device stays > connected successfully. > > Tested-by: Darren Hart <dvhart@infradead.org> Applied, thanks.
Hi
[This is an automated email]
This commit has been processed because it contains a -stable tag.
The stable tag indicates that it's relevant for the following trees: all
The bot has tested the following trees: v5.7.8, v5.4.51, v4.19.132, v4.14.188, v4.9.230, v4.4.230.
v5.7.8: Build OK!
v5.4.51: Build OK!
v4.19.132: Build OK!
v4.14.188: Build OK!
v4.9.230: Failed to apply! Possible dependencies:
581c4484769e6 ("HID: input: map digitizer battery usage")
v4.4.230: Failed to apply! Possible dependencies:
581c4484769e6 ("HID: input: map digitizer battery usage")
5d9374cf5f66e ("HID: input: ignore the battery in OKLICK Laser BTmouse")
NOTE: The patch will not be queued to stable trees until it is upstream.
How should we proceed with this patch?
diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c index dea9cc65bf80..e8641ce677e4 100644 --- a/drivers/hid/hid-input.c +++ b/drivers/hid/hid-input.c @@ -350,13 +350,13 @@ static int hidinput_query_battery_capacity(struct hid_device *dev) u8 *buf; int ret; - buf = kmalloc(2, GFP_KERNEL); + buf = kmalloc(4, GFP_KERNEL); if (!buf) return -ENOMEM; - ret = hid_hw_raw_request(dev, dev->battery_report_id, buf, 2, + ret = hid_hw_raw_request(dev, dev->battery_report_id, buf, 4, dev->battery_report_type, HID_REQ_GET_REPORT); - if (ret != 2) { + if (ret < 2) { kfree(buf); return -ENODATA; }
Some devices, particularly the 3DConnexion Spacemouse wireless 3D controllers, return more than just the battery capacity in the battery report. The Spacemouse devices return an additional byte with a device specific field. However, hidinput_query_battery_capacity() only requests a 2 byte transfer. When a spacemouse is connected via USB (direct wire, no wireless dongle) and it returns a 3 byte report instead of the assumed 2 byte battery report the larger transfer confuses and frightens the USB subsystem which chooses to ignore the transfer. Then after 2 seconds assume the device has stopped responding and reset it. This can be reproduced easily by using a wired connection with a wireless spacemouse. The Spacemouse will enter a loop of resetting every 2 seconds which can be observed in dmesg. This patch solves the problem by increasing the transfer request to 4 bytes instead of 2. The fix isn't particularly elegant, but it is simple and safe to backport to stable kernels. A further patch will follow to more elegantly handle battery reports that contain additional data. Signed-off-by: Grant Likely <grant.likely@secretlab.ca> Cc: Darren Hart <darren@dvhart.com> Cc: Jiri Kosina <jikos@kernel.org> Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com> Cc: stable@vger.kernel.org --- drivers/hid/hid-input.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)