From 84aa8536f0197e439832f56cc7b554af488fc3c8 Mon Sep 17 00:00:00 2001 From: Bin Meng Date: Sun, 1 Oct 2017 06:19:39 -0700 Subject: [PATCH] usb: sandbox: Fix emulator device select logic in usb_emul_find_devnum() Current emulator select logic in usb_emul_find_devnum() is to test the USB address. The USB address of the device being enumerated is initialized to zero at the beginning of the enumeration process in usb_setup_device(). At this point, the saved USB address in the platform data has not been assigned to any valid USB address either. This means: the logic will select an emulator device according to its sequence of declaring order in the device tree. Take test.dts for example, flash-stick@0 will be selected before flash-stick@1. But unfortunately such logic is wrong. In fact USB devices show up in a random order during the enumeration which means usb_emul_find_devnum() may be called on port 3 for keyb@3 before on port 0 for flash-stick@0. To fix this, we introduce a new emulator uclass specific platdata to store the USB device's port number on its parent hub, and update the logic to test the port number instead. Signed-off-by: Bin Meng --- drivers/usb/emul/sandbox_hub.c | 2 ++ drivers/usb/emul/usb-emul-uclass.c | 42 +++++++++++++++++++++++++++++++++----- drivers/usb/host/usb-sandbox.c | 6 +++--- include/usb.h | 18 ++++++++++++++-- 4 files changed, 58 insertions(+), 10 deletions(-) diff --git a/drivers/usb/emul/sandbox_hub.c b/drivers/usb/emul/sandbox_hub.c index 9ed7000..8ed7a0f 100644 --- a/drivers/usb/emul/sandbox_hub.c +++ b/drivers/usb/emul/sandbox_hub.c @@ -280,8 +280,10 @@ static int sandbox_hub_bind(struct udevice *dev) static int sandbox_child_post_bind(struct udevice *dev) { struct sandbox_hub_platdata *plat = dev_get_parent_platdata(dev); + struct usb_emul_platdata *emul = dev_get_uclass_platdata(dev); plat->port = dev_read_u32_default(dev, "reg", -1); + emul->port1 = plat->port + 1; return 0; } diff --git a/drivers/usb/emul/usb-emul-uclass.c b/drivers/usb/emul/usb-emul-uclass.c index e4a267b..1203eb2 100644 --- a/drivers/usb/emul/usb-emul-uclass.c +++ b/drivers/usb/emul/usb-emul-uclass.c @@ -107,7 +107,7 @@ static int usb_emul_get_descriptor(struct usb_dev_platdata *plat, int value, return upto ? upto : length ? -EIO : 0; } -static int usb_emul_find_devnum(int devnum, struct udevice **emulp) +static int usb_emul_find_devnum(int devnum, int port1, struct udevice **emulp) { struct udevice *dev; struct uclass *uc; @@ -120,7 +120,37 @@ static int usb_emul_find_devnum(int devnum, struct udevice **emulp) uclass_foreach_dev(dev, uc) { struct usb_dev_platdata *udev = dev_get_parent_platdata(dev); - if (udev->devnum == devnum) { + /* + * devnum is initialzied to zero at the beginning of the + * enumeration process in usb_setup_device(). At this + * point, udev->devnum has not been assigned to any valid + * USB address either, so we can't rely on the comparison + * result between udev->devnum and devnum to select an + * emulator device. + */ + if (!devnum) { + struct usb_emul_platdata *plat; + + /* + * If the parent is sandbox USB controller, we are + * the root hub. And there is only one root hub + * in the system. + */ + if (device_get_uclass_id(dev->parent) == UCLASS_USB) { + debug("%s: Found emulator '%s'\n", + __func__, dev->name); + *emulp = dev; + return 0; + } + + plat = dev_get_uclass_platdata(dev); + if (plat->port1 == port1) { + debug("%s: Found emulator '%s', port %d\n", + __func__, dev->name, port1); + *emulp = dev; + return 0; + } + } else if (udev->devnum == devnum) { debug("%s: Found emulator '%s', addr %d\n", __func__, dev->name, udev->devnum); *emulp = dev; @@ -132,18 +162,19 @@ static int usb_emul_find_devnum(int devnum, struct udevice **emulp) return -ENOENT; } -int usb_emul_find(struct udevice *bus, ulong pipe, struct udevice **emulp) +int usb_emul_find(struct udevice *bus, ulong pipe, int port1, + struct udevice **emulp) { int devnum = usb_pipedevice(pipe); - return usb_emul_find_devnum(devnum, emulp); + return usb_emul_find_devnum(devnum, port1, emulp); } int usb_emul_find_for_dev(struct udevice *dev, struct udevice **emulp) { struct usb_dev_platdata *udev = dev_get_parent_platdata(dev); - return usb_emul_find_devnum(udev->devnum, emulp); + return usb_emul_find_devnum(udev->devnum, 0, emulp); } int usb_emul_control(struct udevice *emul, struct usb_device *udev, @@ -276,6 +307,7 @@ UCLASS_DRIVER(usb_emul) = { .id = UCLASS_USB_EMUL, .name = "usb_emul", .post_bind = dm_scan_fdt_dev, + .per_device_platdata_auto_alloc_size = sizeof(struct usb_emul_platdata), .per_child_auto_alloc_size = sizeof(struct usb_device), .per_child_platdata_auto_alloc_size = sizeof(struct usb_dev_platdata), }; diff --git a/drivers/usb/host/usb-sandbox.c b/drivers/usb/host/usb-sandbox.c index f85d36b..15055b3 100644 --- a/drivers/usb/host/usb-sandbox.c +++ b/drivers/usb/host/usb-sandbox.c @@ -50,7 +50,7 @@ static int sandbox_submit_control(struct udevice *bus, /* Just use child of dev as emulator? */ debug("%s: bus=%s\n", __func__, bus->name); - ret = usb_emul_find(bus, pipe, &emul); + ret = usb_emul_find(bus, pipe, udev->portnr, &emul); usbmon_trace(bus, pipe, setup, emul); if (ret) return ret; @@ -83,7 +83,7 @@ static int sandbox_submit_bulk(struct udevice *bus, struct usb_device *udev, /* Just use child of dev as emulator? */ debug("%s: bus=%s\n", __func__, bus->name); - ret = usb_emul_find(bus, pipe, &emul); + ret = usb_emul_find(bus, pipe, udev->portnr, &emul); usbmon_trace(bus, pipe, NULL, emul); if (ret) return ret; @@ -109,7 +109,7 @@ static int sandbox_submit_int(struct udevice *bus, struct usb_device *udev, /* Just use child of dev as emulator? */ debug("%s: bus=%s\n", __func__, bus->name); - ret = usb_emul_find(bus, pipe, &emul); + ret = usb_emul_find(bus, pipe, udev->portnr, &emul); usbmon_trace(bus, pipe, NULL, emul); if (ret) return ret; diff --git a/include/usb.h b/include/usb.h index 3d51731..63edddd 100644 --- a/include/usb.h +++ b/include/usb.h @@ -653,6 +653,18 @@ struct usb_bus_priv { }; /** + * struct usb_emul_platdata - platform data about the USB emulator + * + * Given a USB emulator (UCLASS_USB_EMUL) 'dev', this is + * dev_get_uclass_platdata(dev). + * + * @port1: USB emulator device port number on the parent hub + */ +struct usb_emul_platdata { + int port1; /* Port number (numbered from 1) */ +}; + +/** * struct dm_usb_ops - USB controller operations * * This defines the operations supoorted on a USB controller. Common @@ -1023,14 +1035,16 @@ int usb_emul_int(struct udevice *emul, struct usb_device *udev, /** * usb_emul_find() - Find an emulator for a particular device * - * Check @pipe to find a device number on bus @bus and return it. + * Check @pipe and @port1 to find a device number on bus @bus and return it. * * @bus: USB bus (controller) * @pipe: Describes pipe being used, and includes the device number + * @port1: Describes port number on the parent hub * @emulp: Returns pointer to emulator, or NULL if not found * @return 0 if found, -ve on error */ -int usb_emul_find(struct udevice *bus, ulong pipe, struct udevice **emulp); +int usb_emul_find(struct udevice *bus, ulong pipe, int port1, + struct udevice **emulp); /** * usb_emul_find_for_dev() - Find an emulator for a particular device