| Message ID | 1505742050-5697-2-git-send-email-bmeng.cn@gmail.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Marek Vasut |
| Headers | show |
| Series | usb: xhci: Add interrupt transfer support and full speed device support | expand |
On 18.09.2017 15:40, Bin Meng wrote: > At present we only do device_remove() during usb stop. The DM API > device_remove() only marks the device state as inactivated, but > still keeps its USB topology (eg: parent, children, etc) in the DM > device structure. There is no issue if we only start USB subsystem > once and never stop it. But a big issue occurs when we do 'usb stop' > and 'usb start' multiple times. > > Strange things may be observed with current implementation, like: > - the enumeration may report only 1 mass storage device is detected, > but the total number of USB devices is correct. > - USB keyboard does not work anymore after a bunch of 'usb reset' > even if 'usb tree' shows it is correctly identified. > - read/write flash drive via 'fatload usb' may complain "Bad device" > > In fact, every time when USB host controller starts the enumeration > process, it takes random time for each USB port to show up online, > hence each USB device may appear in a different order from previous > enumeration, and gets assigned to a totally different USB address. > As a result, we end up using a stale USB topology in the DM device > structure which still reflects the previous enumeration result, and > it may create an exact same DM device name like generic_bus_0_dev_7 > that is already in the DM device structure. And since the DM device > structure is there, there is no device_bind() call to bind driver to > the device during current enumeration process, eventually creating > an inconsistent software representation of the hardware topology, a > non-working USB subsystem. > > The fix is to clear the unused USB topology in the usb_stop(), by > calling device_unbind() on each controller's root hub device, and > the unbinding will unbind all of its children automatically. > > Signed-off-by: Bin Meng <bmeng.cn@gmail.com> Reviewed-by: Stefan Roese <sr@denx.de> Tested-by: Stefan Roese <sr@denx.de> Thanks, Stefan
diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c index bc44fc3..e90614f 100644 --- a/drivers/usb/host/usb-uclass.c +++ b/drivers/usb/host/usb-uclass.c @@ -164,6 +164,7 @@ int usb_get_max_xfer_size(struct usb_device *udev, size_t *size) int usb_stop(void) { struct udevice *bus; + struct udevice *rh; struct uclass *uc; struct usb_uclass_priv *uc_priv; int err = 0, ret; @@ -179,6 +180,18 @@ int usb_stop(void) ret = device_remove(bus, DM_REMOVE_NORMAL); if (ret && !err) err = ret; + + /* Locate root hub device */ + device_find_first_child(bus, &rh); + if (rh) { + /* + * All USB devices are children of root hub. + * Unbinding root hub will unbind all of its children. + */ + ret = device_unbind(rh); + if (ret && !err) + err = ret; + } } #ifdef CONFIG_BLK ret = blk_unbind_all(IF_TYPE_USB);
At present we only do device_remove() during usb stop. The DM API device_remove() only marks the device state as inactivated, but still keeps its USB topology (eg: parent, children, etc) in the DM device structure. There is no issue if we only start USB subsystem once and never stop it. But a big issue occurs when we do 'usb stop' and 'usb start' multiple times. Strange things may be observed with current implementation, like: - the enumeration may report only 1 mass storage device is detected, but the total number of USB devices is correct. - USB keyboard does not work anymore after a bunch of 'usb reset' even if 'usb tree' shows it is correctly identified. - read/write flash drive via 'fatload usb' may complain "Bad device" In fact, every time when USB host controller starts the enumeration process, it takes random time for each USB port to show up online, hence each USB device may appear in a different order from previous enumeration, and gets assigned to a totally different USB address. As a result, we end up using a stale USB topology in the DM device structure which still reflects the previous enumeration result, and it may create an exact same DM device name like generic_bus_0_dev_7 that is already in the DM device structure. And since the DM device structure is there, there is no device_bind() call to bind driver to the device during current enumeration process, eventually creating an inconsistent software representation of the hardware topology, a non-working USB subsystem. The fix is to clear the unused USB topology in the usb_stop(), by calling device_unbind() on each controller's root hub device, and the unbinding will unbind all of its children automatically. Signed-off-by: Bin Meng <bmeng.cn@gmail.com> --- drivers/usb/host/usb-uclass.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)