Bug 244718 - [patch] WMT driver usb framelength
Summary: [patch] WMT driver usb framelength
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: usb (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Only Me
Assignee: Hans Petter Selasky
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-03-10 14:28 UTC by Oskar Holmlund
Modified: 2020-03-14 22:08 UTC (History)
4 users (show)

See Also:


Attachments
Patch for wmt driver. (370 bytes, text/plain)
2020-03-10 14:28 UTC, Oskar Holmlund
no flags Details
dont_restrict_transfer_size_to_wMaxPacketSize.patch (2.17 KB, patch)
2020-03-11 11:26 UTC, Vladimir Kondratyev
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Oskar Holmlund 2020-03-10 14:28:56 UTC
Created attachment 212306 [details]
Patch for wmt driver.

Attached patch diff from r358834

CiVUE ACB-C27003 with controller EETI EXC80W32/P80H32 (newer eGalax multitouch).

If usb frame length is set to 1024 bytes (WMT_BSIZE) the EETI controller will pack multiple touch events in the packet and the current code will only process the first touch event. (line 500: usbd_copy_out will only copy the first 64 bytes). As a result some important events are lost like releasing the finger from the touchscreen :)

Just let the hardware report its maximum packet size.
Comment 1 commit-hook freebsd_committer freebsd_triage 2020-03-11 08:16:58 UTC
A commit references this bug:

Author: hselasky
Date: Wed Mar 11 08:16:14 UTC 2020
New revision: 358872
URL: https://svnweb.freebsd.org/changeset/base/358872

Log:
  If the USB frame length is set to 1024 bytes, WMT_BSIZE, the EETI controller
  will pack multiple touch events in the packet and the current code will only
  process the first touch event.

  As a result some important events are lost like releasing the finger from the
  touchscreen.

  Use the maximum maximum packet size as buffer size instead.

  Submitted by:	Oskar Holmlund <oskar.holmlund@ohdata.se>
  PR:		244718
  MFC after:	3 days
  Sponsored by:	Mellanox Technologies

Changes:
  head/sys/dev/usb/input/wmt.c
Comment 2 Hans Petter Selasky freebsd_committer freebsd_triage 2020-03-11 08:17:31 UTC
Will be MFC'ed in a few days time.

Thank you for the patch!
Comment 3 Vladimir Kondratyev freebsd_committer freebsd_triage 2020-03-11 11:02:35 UTC
The patch breaks devices which have report size larger then packet size.

I think we should start with reverting r337289 and r338458. I'll submit patch to test soon.
Comment 4 Hans Petter Selasky freebsd_committer freebsd_triage 2020-03-11 11:05:31 UTC
Wulf: Are you sure about this?
Comment 5 Hans Petter Selasky freebsd_committer freebsd_triage 2020-03-11 11:10:38 UTC
Feel free to fix this (I.E. revert patches)

From my understand INTERRUPT endpoint data should fit within one wMaxPacketSize, but you know these devices better than I do.

I think we have a HID function which computes the worst-case descriptor length, and maybe this can be used together with .proxy_buffer = 1, to ensure the RX buffer is a multiple of wMaxPacketSize.

--HPS
Comment 6 Vladimir Kondratyev freebsd_committer freebsd_triage 2020-03-11 11:26:34 UTC
Created attachment 212329 [details]
dont_restrict_transfer_size_to_wMaxPacketSize.patch

Try this patch. If it don't help then try reduce bufsize to sc->isize.
Comment 7 Vladimir Kondratyev freebsd_committer freebsd_triage 2020-03-11 11:31:08 UTC
(In reply to Hans Petter Selasky from comment #5)

> From my understand INTERRUPT endpoint data should fit within one wMaxPacketSize

I have the device which reports data in 116 bytes transfers. Restricting bufsize to wMaxPacketSize (64) broke it.
Comment 8 Hans Petter Selasky freebsd_committer freebsd_triage 2020-03-11 11:37:24 UTC
Wulf: I see. Just make sure you check sc_isize <= WMT_BSIZE somewhere.
Comment 9 Oskar Holmlund 2020-03-11 13:10:19 UTC
(In reply to Vladimir Kondratyev from comment #6)

Your patch works with the EETI controller.

Thank you.
Comment 10 Hans Petter Selasky freebsd_committer freebsd_triage 2020-03-11 14:26:00 UTC
Wulf: Do you want me to submit your second patch or will you handle it?
Comment 11 Vladimir Kondratyev freebsd_committer freebsd_triage 2020-03-11 16:10:46 UTC
(In reply to Hans Petter Selasky from comment #10)

 I will handle it
Comment 12 commit-hook freebsd_committer freebsd_triage 2020-03-11 20:06:00 UTC
A commit references this bug:

Author: wulf
Date: Wed Mar 11 20:05:50 UTC 2020
New revision: 358895
URL: https://svnweb.freebsd.org/changeset/base/358895

Log:
  wmt(4): Reapply r358872 (by hselasky) modified to use
  maximal input report size instead of wMaxPacketSize.

  If the USB frame length is set to 1024 bytes, WMT_BSIZE, the EETI controller
  will pack multiple touch events in the packet and the current code will only
  process the first touch event.

  As a result some important events are lost like releasing the finger from the
  touchscreen.

  Use the maximal input report size as buffer size instead.

  PR:		244718
  Tested by:	Oskar Holmlund <oskar.holmlund@ohdata.se>, wulf
  MFC after:	3 days
  Discussed with:	hselasky

Changes:
  head/sys/dev/usb/input/wmt.c
Comment 13 commit-hook freebsd_committer freebsd_triage 2020-03-14 22:04:10 UTC
A commit references this bug:

Author: wulf
Date: Sat Mar 14 22:03:50 UTC 2020
New revision: 359003
URL: https://svnweb.freebsd.org/changeset/base/359003

Log:
  MFC r358895

  wmt(4): Reapply r358872 (by hselasky) modified to use
  maximal input report size instead of wMaxPacketSize.

  If the USB frame length is set to 1024 bytes, WMT_BSIZE, the EETI controller
  will pack multiple touch events in the packet and the current code will only
  process the first touch event.

  As a result some important events are lost like releasing the finger from the
  touchscreen.

  Use the maximal input report size as buffer size instead.

  PR:		244718
  Tested by:	Oskar Holmlund <oskar.holmlund@ohdata.se>, wulf
  Discussed with:	hselasky

Changes:
_U  stable/12/
  stable/12/sys/dev/usb/input/wmt.c
Comment 14 commit-hook freebsd_committer freebsd_triage 2020-03-14 22:08:12 UTC
A commit references this bug:

Author: wulf
Date: Sat Mar 14 22:07:11 UTC 2020
New revision: 359004
URL: https://svnweb.freebsd.org/changeset/base/359004

Log:
  MFC r358895

  wmt(4): Reapply r358872 (by hselasky) modified to use
  maximal input report size instead of wMaxPacketSize.

  If the USB frame length is set to 1024 bytes, WMT_BSIZE, the EETI controller
  will pack multiple touch events in the packet and the current code will only
  process the first touch event.

  As a result some important events are lost like releasing the finger from the
  touchscreen.

  Use the maximal input report size as buffer size instead.

  PR:		244718
  Tested by:	Oskar Holmlund <oskar.holmlund@ohdata.se>, wulf
  Discussed with:	hselasky

Changes:
_U  stable/11/
  stable/11/sys/dev/usb/input/wmt.c