[PATCH v2 1/2] usb: hub: allow to increase HUB_DEBOUNCE_TIMEOUT

[Patrick DELAUNAY]

Hi,

On 9/9/22 14:24, Marek Vasut wrote:
> On 9/9/22 11:45, Patrick Delaunay wrote:
>> Add a new CONFIG_USB_HUB_DEBOUNCE_TIMEOUT to increase the
>> HUB_DEBOUNCE_TIMEOUT value, for example to 2s because some usb device
>> needs around 1.5s or more to make the hub port status to be
>> connected steadily after being powered off and powered on.
>>
>> This 2s value is aligned with Linux driver and avoids to configure
>> "usb_pgood_delay" as a workaround for connection timeout on
>> some USB device; normally the env variable "usb_pgood_delay" is used
>> to delay the first query after power ON and thus the device answer,
>> but this variable not used to increase the connection timeout delay.
>
> I realized this has one problem -- what happens if you have multiple 
> USB controllers in your system ? The answer is, all of them are 
> affected by the increased delay, possibly even those which do not 
> require the extra delay.
>
> Would it be possible to configure this per-controller (or should this 
> even be per-device?) in DT ? In fact, I wonder whether this is not 
> becoming a Vbus regulator ramp-up time kind of delay here ?


Yes, but I don't think, it is blocking.

This timeout will be common for all the USB HUB in the system, as it is 
done in Linux kernel.



And I don't thing the issue is linked to VBUS  regulator rampup, because

- with USB analyser we check that the answer for the problematic key are 
really slow,

   no answer in the 1s normal delay

- and any issue on vbus can be handled by query_delay = pgood delay =

    the power is stablized before the devices are queried.

    But For our use case, increasing pgood delay is not enought the 
problematic

    (low cost / poor quality / slow) USB keys.



So this CONFIG is used to support some slow USB devices on any USB HUB,

and it is not a workaround for HUB issue.



For my point of view 1s is a little penalty for usb start to support 
more USB devices on all

the USB HUB on the system, even when the 1s delay defined by the spec is 
not supported.


For kernel this delay is not managed by the device tree but hardcoded in 
the USB HUB driver

drivers/usb/core/hub.c

#define HUB_DEBOUNCE_TIMEOUT    2000

/* USB 2.0 spec, 7.1.7.3 / fig 7-29:
*
* Between connect detection and reset signaling there must be a delay
* of 100ms at least for debounce and power-settling. The corresponding
* timer shall restart whenever the downstream port detects a disconnect.
*
* Apparently there are some bluetooth and irda-dongles and a number of
* low-speed devices for which this debounce period may last over a second.
* Not covered by the spec - but easy to deal with.
*
* This implementation uses a 1500ms total debounce timeout; if the
* connection isn't stable by then it returns -ETIMEDOUT. It checks
* every 25ms for transient disconnects. When the port status has been
* unchanged for 100ms it returns the port status.
*/
inthub_port_debounce(structusb_hub *hub, intport1, boolmust_be_connected)


For me in U-Boot driver this debounce is managed as Linux kernel (with 
1s <=> 2s)

=> in U-Boot the connect_timeout is used in usb_scan_port():

1- the port is removed from the list when usb_get_port_status() return a 
error

     during the connect_timeout

2- the port is removed when the when usb_get_port_status() return no error

     and the connection change happened,


=> no issue when the PORT is become ready and connected

       the usb_get_port_status() will be return no error
       and the portstatus (answer of USB_REQ_GET_STATUS)

       is handled by usb_scan_port()


=> the potential issue for timeout is when the PORT is ready (no error 
for usb_get_port_status())
       and not connection detected portchange = 0 / portstatus = 0 after 
reset

       in usb_scan_port():


     /*
      * No connection change happened, wait a bit more.
      *
      * For some situation, the hub reports no connection change but a
      * device is connected to the port (eg: CCS bit is set but CSC is not
      * in the PORTSC register of a root hub), ignore such case.
      */
     if (!(portchange & USB_PORT_STAT_C_CONNECTION) &&
         !(portstatus & USB_PORT_STAT_CONNECTION)) {
         if (get_timer(0) >= hub->connect_timeout) {
             debug("devnum=%d port=%d: timeout\n",
                   dev->devnum, i + 1);
             /* Remove this device from scanning list */
             list_del(&usb_scan->list);
             free(usb_scan);
             return 0;
         }
         return 0;
     }


I think the "connect_timeout" could be not used here

as TIMEOUT for "No connection change happened, wait a bit more."


In linux kernel a other timeout is used: HUB_DEBOUNCE_STABLE = 100ms

for this condition...

when usb_get_port_status() return not error and the the connectio

is STABLE during this duration, it is enought.


Perhaps a optimization can be done here....

but I think it is a other subject and I am not enought expert on USB

to propose something


so I don't see the issue here: I just increase the CONNECTION TIMEOUT to 2s

with Kconfig, aligned on Linux value and only for STMicrolectronics 
defconfig

=> no impact on other boards

=> common for all USB HUB used on the STM32MP boards


Patrick