From mboxrd@z Thu Jan 1 00:00:00 1970 From: Hans de Goede Date: Fri, 18 Dec 2015 11:00:19 +0100 Subject: [U-Boot] [PATCH] usb: Move determination of TT hub address/port into seperate function In-Reply-To: <1450401101-22591-1-git-send-email-stefan.bruens@rwth-aachen.de> References: <3364487.y0b5h7sQeW@pebbles.site> <1450401101-22591-1-git-send-email-stefan.bruens@rwth-aachen.de> Message-ID: <5673D933.6070505@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 18-12-15 02:11, Stefan Br?ns wrote: > Start split and complete split tokens need the hub address and the > downstream port of the first HS hub (device view). > > The core of the function was duplicated in both host/ehci_hcd and > musb-new/usb-compat.h. > > Signed-off-by: Stefan Br?ns Thanks for working on this, I think I've spotted one small bug though, see comments inline. > --- > Obsoletes [PATCH 3/6] usb: dwc2: determine TT hub address and port for split transactions > > common/usb.c | 54 +++++++++++++++++++++++++++++++++++++++ > drivers/usb/host/ehci-hcd.c | 50 ++++-------------------------------- > drivers/usb/musb-new/musb_host.c | 8 +++--- > drivers/usb/musb-new/usb-compat.h | 53 -------------------------------------- > include/usb.h | 10 ++++++++ > 5 files changed, 74 insertions(+), 101 deletions(-) > > diff --git a/common/usb.c b/common/usb.c > index 700bfc3..1d0a151 100644 > --- a/common/usb.c > +++ b/common/usb.c > @@ -1200,4 +1200,58 @@ bool usb_device_has_child_on_port(struct usb_device *parent, int port) > #endif > } > > +#ifdef CONFIG_DM_USB > +void usb_find_hub_address_port(struct usb_device *udev, > + uint8_t *hub_address, uint8_t *hub_port) > +{ > + struct udevice *parent; > + struct usb_device *uparent, *ttdev; > + > + /* > + * When called from usb-uclass.c: usb_scan_device() udev->dev points > + * to the parent udevice, not the actual udevice belonging to the > + * udev as the device is not instantiated yet. So when searching > + * for the first usb-2 parent start with udev->dev not > + * udev->dev->parent . > + */ > + ttdev = udev; > + parent = udev->dev; > + uparent = dev_get_parent_priv(parent); > + > + while (uparent->speed != USB_SPEED_HIGH) { > + struct udevice *dev = parent; > + > + if (device_get_uclass_id(dev->parent) != UCLASS_USB_HUB) { > + printf("Error: Cannot find high speed parent of usb-1 device\n"); > + *hub_address = *hub_port = 0; > + return; > + } > + > + ttdev = dev_get_parent_priv(dev); > + parent = dev->parent; > + uparent = dev_get_parent_priv(parent); > + } > + *hub_address = uparent->devnum; > + *hub_port = ttdev->portnr; This is correct to replace the ehci-code, but lets take a closer look at the musb-new code below. > +} > +#else > +void usb_find_hub_address_port(struct usb_device *udev, > + uint8_t *hub_address, uint8_t *hub_port) > +{ > + /* Find out the nearest parent which is high speed */ > + while (udev->parent->parent != NULL) > + if (udev->parent->speed != USB_SPEED_HIGH) { > + udev = udev->parent; > + } else { > + *hub_address = udev->parent->devnum; > + *hub_port = udev->portnr; Same here. > + return; > + } > + > + printf("Error: Cannot find high speed parent of usb-1 device\n"); > + *hub_address = *hub_port = 0; > +} > +#endif > + > + > /* EOF */ > diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c > index c85dbce..0a279e7 100644 > --- a/drivers/usb/host/ehci-hcd.c > +++ b/drivers/usb/host/ehci-hcd.c > @@ -279,56 +279,16 @@ static inline u8 ehci_encode_speed(enum usb_device_speed speed) > static void ehci_update_endpt2_dev_n_port(struct usb_device *udev, > struct QH *qh) > { > - struct usb_device *ttdev; > - int parent_devnum; > + uint8_t portnr = 0; > + uint8_t hubaddr = 0; > > if (udev->speed != USB_SPEED_LOW && udev->speed != USB_SPEED_FULL) > return; > > - /* > - * For full / low speed devices we need to get the devnum and portnr of > - * 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 > - /* > - * When called from usb-uclass.c: usb_scan_device() udev->dev points > - * to the parent udevice, not the actual udevice belonging to the > - * udev as the device is not instantiated yet. So when searching > - * for the first usb-2 parent start with udev->dev not > - * udev->dev->parent . > - */ > - struct udevice *parent; > - struct usb_device *uparent; > - > - ttdev = udev; > - parent = udev->dev; > - uparent = dev_get_parent_priv(parent); > - > - while (uparent->speed != USB_SPEED_HIGH) { > - struct udevice *dev = parent; > - > - if (device_get_uclass_id(dev->parent) != UCLASS_USB_HUB) { > - printf("ehci: Error cannot find high-speed parent of usb-1 device\n"); > - return; > - } > - > - ttdev = dev_get_parent_priv(dev); > - parent = dev->parent; > - uparent = dev_get_parent_priv(parent); > - } > - parent_devnum = uparent->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 > + usb_find_hub_address_port(udev, &hubaddr, &portnr) > > - qh->qh_endpt2 |= cpu_to_hc32(QH_ENDPT2_PORTNUM(ttdev->portnr) | > - QH_ENDPT2_HUBADDR(parent_devnum)); > + qh->qh_endpt2 |= cpu_to_hc32(QH_ENDPT2_PORTNUM(portnr) | > + QH_ENDPT2_HUBADDR(hubaddr)); > } > > static int > diff --git a/drivers/usb/musb-new/usb-compat.h b/drivers/usb/musb-new/usb-compat.h > index 1c41e2a..760bd78 100644 > --- a/drivers/usb/musb-new/usb-compat.h > +++ b/drivers/usb/musb-new/usb-compat.h > @@ -68,38 +68,6 @@ static inline int usb_hcd_unmap_urb_for_dma(struct usb_hcd *hcd, > } > > #ifdef CONFIG_DM_USB > -static inline u16 find_tt(struct usb_device *udev) > -{ > - struct udevice *parent; > - struct usb_device *uparent, *ttdev; > - > - /* > - * When called from usb-uclass.c: usb_scan_device() udev->dev points > - * to the parent udevice, not the actual udevice belonging to the > - * udev as the device is not instantiated yet. So when searching > - * for the first usb-2 parent start with udev->dev not > - * udev->dev->parent . > - */ > - ttdev = udev; > - parent = udev->dev; > - uparent = dev_get_parent_priv(parent); > - > - while (uparent->speed != USB_SPEED_HIGH) { > - struct udevice *dev = parent; > - > - if (device_get_uclass_id(dev->parent) != UCLASS_USB_HUB) { > - printf("musb: Error cannot find high speed parent of usb-1 device\n"); > - return 0; > - } > - > - ttdev = dev_get_parent_priv(dev); > - parent = dev->parent; > - uparent = dev_get_parent_priv(parent); > - } > - > - return (uparent->devnum << 8) | (ttdev->portnr - 1); > -} > - Here in the original musb_new code "ttdev->portnr - 1" is used rather then "just "ttdev->portnr". > static inline struct usb_device *usb_dev_get_parent(struct usb_device *udev) > { > struct udevice *parent = udev->dev->parent; > @@ -129,27 +97,6 @@ static inline struct usb_device *usb_dev_get_parent(struct usb_device *udev) > return NULL; > } > #else > -static inline u16 find_tt(struct usb_device *dev) > -{ > - u8 chid; > - u8 hub; > - > - /* Find out the nearest parent which is high speed */ > - while (dev->parent->parent != NULL) > - if (dev->parent->speed != USB_SPEED_HIGH) > - dev = dev->parent; > - else > - break; > - > - /* determine the port address at that hub */ > - hub = dev->parent->devnum; > - for (chid = 0; chid < USB_MAXCHILDREN; chid++) > - if (dev->parent->children[chid] == dev) > - break; > - > - return (hub << 8) | chid; > -} > - And here for the non DM code the same in a convoluted way (the index is returned which starts at 0, where as portnr-s start at 1). > static inline struct usb_device *usb_dev_get_parent(struct usb_device *dev) > { > return dev->parent; > diff --git a/drivers/usb/musb-new/musb_host.c b/drivers/usb/musb-new/musb_host.c > index 40b9c66..352fa8c 100644 > --- a/drivers/usb/musb-new/musb_host.c > +++ b/drivers/usb/musb-new/musb_host.c > @@ -2092,9 +2092,11 @@ int musb_urb_enqueue( > } > #else > if (tt_needed(musb, urb->dev)) { > - u16 hub_port = find_tt(urb->dev); > - qh->h_addr_reg = (u8) (hub_port >> 8); > - qh->h_port_reg = (u8) (hub_port & 0xff); > + uint8_t portnr = 0; > + uint8_t hubaddr = 0; > + usb_find_hub_address_port(udev, &hubaddr, &portnr) > + qh->h_addr_reg = hubaddr; > + qh->h_port_reg = portnr; > } > #endif > } So here "qh->h_port_reg = portnr" should be "qh->h_port_reg = portnr - 1" to compensate for the portnr returned by the new usb_find_hub_address_port being one higher then the one returned by the old find_tt helper. > diff --git a/include/usb.h b/include/usb.h > index 55b9268..6e12876 100644 > --- a/include/usb.h > +++ b/include/usb.h > @@ -874,6 +874,16 @@ int legacy_hub_port_reset(struct usb_device *dev, int port, > > int hub_port_reset(struct udevice *dev, int port, unsigned short *portstat); > > +/* > + * Searches for the first HS hub above the given device. If a > + * HS hub is found, the hub address and the port the device is > + * connected to is return, as required for SPLIT transactions > + * > + * @param: udev full speed or low speed device > + */ > +void usb_find_hub_address_port(struct usb_device *udev, > + uint8_t *hub_address, uint8_t *hub_port); > + > /** > * usb_alloc_new_device() - Allocate a new device > * > This function is usb-2 controller / hub specific, maybe rename it to: "usb_find_usb2_hub_address_port" to reflect this ? Regards, Hans