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

Hi,

On 9/24/22 19:04, Marek Vasut wrote:

On 9/12/22 15:37, Patrick DELAUNAY wrote:

...

Hi,

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.

I just realized this is not the same delay as the usb_pgood_delay, right ?

Yes it is 2 different timeout in USB stack, usb_scan_port()

1/ pgood_delay used in

hub->query_delay = get_timer(0) + max(100, (int)pgood_delay);

=> time before the devices are queried

* Don't talk to the device before the query delay is expired.      * This is needed for voltages to stabalize.

2/ HUB_DEBOUNCE_TIMEOUT used in

hub->connect_timeout =  hub->query_delay + HUB_DEBOUNCE_TIMEOUT

=> timeout (max)   for usb_get_port_status() answer OR No connection change happened (portchange/portstatus)

This is actually the maximum wait for a device to start communicating, i.e. even if this timeout is set to a very high value, most devices will not take that long and will start communicating in shorter time anyway, so the time of completion for e.g. '=> usb reset' is not affected by this change on very much all systems, correct ?

Yes  as it is a max value, it is a protection timeout for problematic devices,

it should not occur for normal use case, but only when the device are correctly detected on the HUB port...

I think the the 'usb reset' sequence  is impacted:

usb_init => usb_new_device => usb_hub_probe => usb_hub_configure =

- usb_hub_power_on()

-usb_device_list_scan()

But as it is a CONFIG it is a platform choice.

Regards

---

Uboot-stm32 mailing list Uboot-stm32@st-md-mailman.stormreply.com https://st-md-mailman.stormreply.com/mailman/listinfo/uboot-stm32