Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change LED state reader to consume vents, as soon as "gadget state connected to host" could be determined #33

Closed
mame82 opened this issue Sep 21, 2018 · 2 comments

Comments

@mame82
Copy link
Collaborator

mame82 commented Sep 21, 2018

Currently the internal LED state reader only consumes a new state, if a listener is present (at least one HIDScript with a running waitLED or waitLEDRepeat command pending).

This design decision was taken, to assure that no LED state change is missed if no listener is running. This is because Windows sends a LED state change ONE TIME in case a keyboard driver is ready. This event mustn't be missed, if the intention is to use it as trigger (for "ready to type"). It couldn't be guaranteed that a HIDScript with such a trigger, is already running when this initial state change occurs, thus ALL LED state changes are preserved until at least one consumer is present (relies on the FIFO queue of the kernel, by not reading from the HID device file till needed).

This behavior by far isn't optimal, as not consuming new LED states means: The current LED state is undefined, till all state changes are consumed.

The situation changes, when reliable trigger for the following USB gadget states (dual role mode) are available:

  • USB gadget mode, not connected to host
  • USB gadget mode, connected to host
  • OTG mode, no device connected (OTG adapter attached, but no device on OTG adapter)
  • OTG mode, device connected (OTG adapter attached and device attached to OTG adapter)

If a state change to USB gadget mode, connected to host is used to trigger a HIDScript (and assuming the delay between firing the trigger and starting the script is negligible), there is no need to preserve past LED state changes which haven't been consumed. Instead, a waitLED command on the start of such a HIDScript would reliably catch the first LED state change, which is issued from the keyboard driver once ready).

@mame82
Copy link
Collaborator Author

mame82 commented Sep 21, 2018

Notes on trigger

while $true; do cat /sys/kernel/debug/20980000.usb/regdump | grep HPRT0; sleep 1; done

Host Port Control and Status register
=====================================
Reg value is dumpable via debugfs for DesignWare USB 2.0 host controller (dwc2)
The dump could be triggered via debugff by reading `/sys/kernel/debug/20980000.usb/regdump` which holds the value for the register in `HPRT0`.
The problem with the approach: The register dump has to be triggered by polling /sys/kernel/debug/20980000.usb/regdump and dumps ALL registers.
This seems to crash the UDC from time to time, in contrast to the comment of the kernel source (https://github.com/raspberrypi/linux/blob/rpi-4.14.y/drivers/usb/dwc2/debugfs.c#L402).

HPRT --> Host Port Control and Status Register, details on data struct:
https://www.cl.cam.ac.uk/~atm26/ephemeral/rpi/dwc_otg/doc/html/unionhprt0__data.html#a964274b5d22e89ca4490f66dff3c763


Meanings of struct fields:
https://www.intel.com/content/www/us/en/programmable/documentation/sfo1410144425160/sfo1410067646785/sfo1410069362932/sfo1410069623936/sfo1410069628257/sfo1410069409998.html

01741 typedef union hprt0_data {
01743         uint32_t d32;
01745         struct {
01746                 unsigned prtconnsts:1;
01747                 unsigned prtconndet:1;
01748                 unsigned prtena:1;
01749                 unsigned prtenchng:1;

01750                 unsigned prtovrcurract:1;
01751                 unsigned prtovrcurrchng:1;
01752                 unsigned prtres:1;
01753                 unsigned prtsusp:1;

01754                 unsigned prtrst:1;
01755                 unsigned reserved9:1;
01756                 unsigned prtlnsts:2;

01757                 unsigned prtpwr:1;
01758                 unsigned prttstctl:4;
01759                 unsigned prtspd:2;
01760 #define DWC_HPRT0_PRTSPD_HIGH_SPEED 0
01761 #define DWC_HPRT0_PRTSPD_FULL_SPEED 1
01762 #define DWC_HPRT0_PRTSPD_LOW_SPEED      2
01763                 unsigned reserved19_31:13;
01764         } b;
01765 } hprt0_data_t;

Test results (polling /sys/kernel/debug/20980000.usb/regdump and parsing "HPRT0")
=================================================================================

USB gadget running, but OTG adapter attached, NO device connected to the OTG adapter
--> prtlnsts = 0 (No data line on positive logic level)
--> prtpwr = 1 (the port is powered ... at least if there's no overcorrency)
--> prtconnsts = 0 (No device attached)

USB gadget running, but OTG adapter attached, device connected to the OTG adapter
--> prtlnsts = 0,1 or 2 (changes)
--> prtpwr = 1 (the port is powered ... at least if there's no overcorrency)
--> prtconnsts = 1 (A device is attached)
--> prtspd = 1 or 2 (for non high speed devices)

USB gadget running, no OTG adapter attached, but NOT connected to host as a peripheral
--> prtlnsts = 1 (Logic Level of D+ is 1, logic level of D- is 0)
--> prtpwr = 0
--> prtconnsts = 0 (No device attached)

USB gadget running, no OTG adapter attached, connected to host as a peripheral
--> prtlnsts = 0 (both, D+ and D-, are indicated as low)
--> prtpwr = 0
--> prtconnsts = 0 (No device attached)
--> in fact the whole HPRT0 is set to 0x00000000 if connected to a host in device mode

WARNING: Constant reading from the regdump file sometimes crashes UDC

@mame82
Copy link
Collaborator Author

mame82 commented Nov 27, 2018

The debugfs based Trigger has been replaced by a customized dwc2 driver, which sends connection events to userspace via gennetlink (directly from the ISR of the USB softreset interrupt)

@mame82 mame82 closed this as completed Nov 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant