From: Ana Bobillo <[email protected]> usb_select_config() ignores what usb_parse_config() returns, so a device whose configuration doesn't start with a valid configuration descriptor is set up anyway, with an empty configuration. Since the previous patches, this is the only case where usb_parse_config() fails: a device with more interfaces or endpoints than there is room for is parsed as far as there is room.
Skip such a device, as is done when its configuration can't be read. The hub then disables the device's port, and the next device found gets the same address. Have the sandbox hub emulator drop the address of a device on a disabled port, as such a device no longer answers, so that the emulator of the next device is the one found at that address. Add a test with a configuration descriptor of the wrong length, which also checks that the devices on the other ports are still found. Signed-off-by: Ana Bobillo <[email protected]> --- common/usb.c | 7 ++++++- drivers/usb/emul/sandbox_hub.c | 11 +++++++++++ test/dm/usb.c | 28 ++++++++++++++++++++++++++++ 3 files changed, 45 insertions(+), 1 deletion(-) diff --git a/common/usb.c b/common/usb.c index 619132512dd..87f0590bbd5 100644 --- a/common/usb.c +++ b/common/usb.c @@ -1232,8 +1232,13 @@ int usb_select_config(struct usb_device *dev) free(tmpbuf); return err; } - usb_parse_config(dev, tmpbuf, 0); + err = usb_parse_config(dev, tmpbuf, 0); free(tmpbuf); + if (err) { + printf("usb_new_device: Cannot parse configuration, skipping device %04x:%04x\n", + dev->descriptor.idVendor, dev->descriptor.idProduct); + return err; + } usb_set_maxpacket(dev); /* * we set the default configuration here diff --git a/drivers/usb/emul/sandbox_hub.c b/drivers/usb/emul/sandbox_hub.c index 066898f005e..c5a5470f9fd 100644 --- a/drivers/usb/emul/sandbox_hub.c +++ b/drivers/usb/emul/sandbox_hub.c @@ -190,6 +190,17 @@ static int clrset_post_state(struct udevice *hub, int port, int clear, int set) } } } + if (clear & USB_PORT_STAT_ENABLE) { + enum usb_device_speed speed; + struct udevice *dev = hub_find_device(hub, port, &speed); + + /* A device on a disabled port no longer answers at its address */ + if (dev) { + struct usb_dev_plat *plat = dev_get_parent_plat(dev); + + plat->devnum = 0; + } + } *change |= *status & clear; *change |= ~*status & set; *change &= 0x1f; diff --git a/test/dm/usb.c b/test/dm/usb.c index 4975a952d87..80763eb8e7b 100644 --- a/test/dm/usb.c +++ b/test/dm/usb.c @@ -690,3 +690,31 @@ static int dm_test_usb_config_too_many(struct unit_test_state *uts) return 0; } DM_TEST(dm_test_usb_config_too_many, UTF_SCAN_PDATA | UTF_SCAN_FDT); + +/* test that a device whose configuration cannot be parsed is not used */ +static int dm_test_usb_config_bad(struct unit_test_state *uts) +{ + static struct usb_config_descriptor config0; + static void *desc_list[] = { + &vendor_device_desc, + &config0, + &vendor_interface0, + &vendor_endpoint1_in, + NULL, + }; + struct usb_device *udev; + struct udevice *dev; + + config0 = vendor_config0; + config0.bLength = USB_DT_CONFIG_SIZE - 1; + ut_assertok(scan_port1(uts, desc_list, &udev)); + ut_assertnull(udev); + + /* The devices on the other ports are still there */ + ut_assertok(uclass_get_device(UCLASS_MASS_STORAGE, 0, &dev)); + ut_assertok(uclass_get_device(UCLASS_MASS_STORAGE, 1, &dev)); + ut_assertok(usb_stop()); + + return 0; +} +DM_TEST(dm_test_usb_config_bad, UTF_SCAN_PDATA | UTF_SCAN_FDT);
