Re: [U-Boot] [PATCH 3/8] dm: usb: Store usb_device parent pointer in usb_device

Hi,

On 01-05-15 06:11, Simon Glass wrote:

Hi Hans,

On 30 April 2015 at 08:35, Hans de Goede hdegoede@redhat.com 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 hdegoede@redhat.com

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