From mboxrd@z Thu Jan 1 00:00:00 1970 From: Hans de Goede Date: Fri, 01 May 2015 10:26:25 +0200 Subject: [U-Boot] [PATCH 3/8] dm: usb: Store usb_device parent pointer in usb_device In-Reply-To: References: <1430404524-31413-1-git-send-email-hdegoede@redhat.com> <1430404524-31413-4-git-send-email-hdegoede@redhat.com> Message-ID: <554338B1.9010504@redhat.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi, On 01-05-15 06:11, Simon Glass wrote: > Hi Hans, > > On 30 April 2015 at 08:35, Hans de Goede wrote: >> When calling into the hcd code in usb_scan_device() we do not yet have >> the actual udevice for the device we are scanning, so we temporarily set >> usb_device.dev to the parent. >> >> This means that we cannot use usb_device.dev to accurately determine our >> place in the usb topology when reading descriptors, and this is necessary >> to correctly program the usb-2 hub tt settings when a usb-1 device is >> connected to a usb-2 hub. >> >> This commit (re)adds a direct parent usb_device pointer to struct usb_device >> so that the usb hcd code can get to the usb_device parent without needing to >> go through the dm. >> >> This fixes usb-1 devices plugged into usb-2 hubs not working with the driver >> model. >> >> Signed-off-by: Hans de Goede >> --- >> drivers/usb/host/ehci-hcd.c | 24 +----------------------- >> drivers/usb/host/usb-uclass.c | 5 ++--- >> include/usb.h | 2 +- >> 3 files changed, 4 insertions(+), 27 deletions(-) >> > > While I agree this is expedient, is there really no other way? How are > we going to move to driver model if we add back in the > non-driver-model pointers, etc.? We'll end up with a hybrid approach > and a real mess. > > Is it possible to bug-fix the driver model code (presumably below) instead? > That is what I tried first, and failed todo. But after implementing the fix as posted to the list I've been thinking about it some more and I think it should be possible so I'll try again. Regards, Hans >> diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c >> index 19f1e29..00c038c 100644 >> --- a/drivers/usb/host/ehci-hcd.c >> +++ b/drivers/usb/host/ehci-hcd.c >> @@ -292,7 +292,6 @@ static void ehci_update_endpt2_dev_n_port(struct usb_device *udev, >> struct QH *qh) >> { >> struct usb_device *ttdev; >> - int parent_devnum; >> >> if (udev->speed != USB_SPEED_LOW && udev->speed != USB_SPEED_FULL) >> return; >> @@ -302,35 +301,14 @@ static void ehci_update_endpt2_dev_n_port(struct usb_device *udev, >> * the tt, so of the first upstream usb-2 hub, there may be usb-1 hubs >> * in the tree before that one! >> */ >> -#ifdef CONFIG_DM_USB >> - struct udevice *parent; >> - >> - for (ttdev = udev; ; ) { >> - struct udevice *dev = ttdev->dev; >> - >> - if (dev->parent && >> - device_get_uclass_id(dev->parent) == UCLASS_USB_HUB) >> - parent = dev->parent; >> - else >> - parent = NULL; >> - if (!parent) >> - return; >> - ttdev = dev_get_parentdata(parent); >> - if (!ttdev->speed != USB_SPEED_HIGH) >> - break; >> - } >> - parent_devnum = ttdev->devnum; >> -#else >> ttdev = udev; >> while (ttdev->parent && ttdev->parent->speed != USB_SPEED_HIGH) >> ttdev = ttdev->parent; >> if (!ttdev->parent) >> return; >> - parent_devnum = ttdev->parent->devnum; >> -#endif >> >> qh->qh_endpt2 |= cpu_to_hc32(QH_ENDPT2_PORTNUM(ttdev->portnr) | >> - QH_ENDPT2_HUBADDR(parent_devnum)); >> + QH_ENDPT2_HUBADDR(ttdev->parent->devnum)); >> } >> >> static int >> diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c >> index 95e371d..0157bc6 100644 >> --- a/drivers/usb/host/usb-uclass.c >> +++ b/drivers/usb/host/usb-uclass.c >> @@ -470,7 +470,6 @@ int usb_scan_device(struct udevice *parent, int port, >> bool created = false; >> struct usb_dev_platdata *plat; >> struct usb_bus_priv *priv; >> - struct usb_device *parent_udev; >> int ret; >> ALLOC_CACHE_ALIGN_BUFFER(struct usb_device, udev, 1); >> struct usb_interface_descriptor *iface = &udev->config.if_desc[0].desc; >> @@ -515,9 +514,9 @@ int usb_scan_device(struct udevice *parent, int port, >> udev->devnum = priv->next_addr + 1; >> udev->portnr = port; >> debug("Calling usb_setup_device(), portnr=%d\n", udev->portnr); >> - parent_udev = device_get_uclass_id(parent) == UCLASS_USB_HUB ? >> + udev->parent = device_get_uclass_id(parent) == UCLASS_USB_HUB ? >> dev_get_parentdata(parent) : NULL; >> - ret = usb_setup_device(udev, priv->desc_before_addr, parent_udev, port); >> + ret = usb_setup_device(udev, priv->desc_before_addr, udev->parent, port); >> debug("read_descriptor for '%s': ret=%d\n", parent->name, ret); >> if (ret) >> return ret; >> diff --git a/include/usb.h b/include/usb.h >> index 1b667ff..7876df4 100644 >> --- a/include/usb.h >> +++ b/include/usb.h >> @@ -141,9 +141,9 @@ struct usb_device { >> int act_len; /* transfered bytes */ >> int maxchild; /* Number of ports if hub */ >> int portnr; /* Port number, 1=first */ >> -#ifndef CONFIG_DM_USB >> /* parent hub, or NULL if this is the root hub */ >> struct usb_device *parent; >> +#ifndef CONFIG_DM_USB >> struct usb_device *children[USB_MAXCHILDREN]; >> void *controller; /* hardware controller private data */ >> #endif >> -- >> 2.3.6 >>