[U-Boot] [PATCH 12/22] musb: Update usb-compat to work with struct usb_device without a parent ptr

Simon Glass
2015-06-29T03:45:25Z

Hi Hans,

On 17 June 2015 at 13:33, Hans de Goede <hdegoede@redhat.com> wrote:
> When building with CONFIG_DM_USB=y struct usb_device does not have a parent
> pointer. This commit adds support to the musb code to deal with this.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> drivers/usb/musb-new/musb_host.c | 4 +++
> drivers/usb/musb-new/musb_uboot.c | 2 +-
> drivers/usb/musb-new/usb-compat.h | 70 +++++++++++++++++++++++++++++++++++++++
> 3 files changed, 75 insertions(+), 1 deletion(-)
>

Acked-by: Simon Glass <sjg@chromium.org>

See note below.

> diff --git a/drivers/usb/musb-new/musb_host.c b/drivers/usb/musb-new/musb_host.c
> index 437309c..40b9c66 100644
> --- a/drivers/usb/musb-new/musb_host.c
> +++ b/drivers/usb/musb-new/musb_host.c
> @@ -2067,7 +2067,11 @@ int musb_urb_enqueue(
>
> /* precompute addressing for external hub/tt ports */
> if (musb->is_multipoint) {
> +#ifndef __UBOOT__
> struct usb_device *parent = urb->dev->parent;
> +#else
> + struct usb_device *parent = usb_dev_get_parent(urb->dev);
> +#endif
>
> #ifndef __UBOOT__
> if (parent != hcd->self.root_hub) {
> diff --git a/drivers/usb/musb-new/musb_uboot.c b/drivers/usb/musb-new/musb_uboot.c
> index 70e87c9..a96e8d2 100644
> --- a/drivers/usb/musb-new/musb_uboot.c
> +++ b/drivers/usb/musb-new/musb_uboot.c
> @@ -97,7 +97,7 @@ int submit_control_msg(struct usb_device *dev, unsigned long pipe,
> buffer, len, setup, 0);
>
> /* Fix speed for non hub-attached devices */
> - if (!dev->parent)
> + if (!usb_dev_get_parent(dev))
> dev->speed = host_speed;
>
> return submit_urb(&hcd, &urb);
> diff --git a/drivers/usb/musb-new/usb-compat.h b/drivers/usb/musb-new/usb-compat.h
> index 50bad37..53fe4ff 100644
> --- a/drivers/usb/musb-new/usb-compat.h
> +++ b/drivers/usb/musb-new/usb-compat.h
> @@ -1,6 +1,7 @@
> #ifndef __USB_COMPAT_H__
> #define __USB_COMPAT_H__
>
> +#include <dm.h>
> #include "usb.h"
>
> struct usb_hcd {
> @@ -66,6 +67,68 @@ static inline int usb_hcd_unmap_urb_for_dma(struct usb_hcd *hcd,
> return 0;
> }
>
> +#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_parentdata(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_parentdata(dev);
> + parent = dev->parent;
> + uparent = dev_get_parentdata(parent);
> + }
> +
> + return (uparent->devnum << 8) | (ttdev->portnr - 1);
> +}
> +
> +static inline struct usb_device *usb_dev_get_parent(struct usb_device *udev)
> +{
> + struct udevice *parent = udev->dev->parent;
> +
> + /*
> + * 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.

Another option here is to somehow allow devices to be added before we
know what they are. In this case we could bind a 'generic' USB device
(UCLASS_USB_DEV_GENERIC). Then when we work out what it is, we could
unbind it (without throwing away the udevice and usb_device) and have
it bind again as the correct device. Something like
device_morph_child().

Just a thought. I'm not sure which is worse yet.

> + *
> + * If dev is an usb-bus, then we are called from usb_scan_device() for
> + * an usb-device plugged directly into the root port, return NULL.
> + */
> + if (device_get_uclass_id(udev->dev) == UCLASS_USB)
> + return NULL;
> +
> + /*
> + * If these 2 are not the same we are being called from
> + * usb_scan_device() and udev itself is the parent.
> + */
> + if (dev_get_parentdata(udev->dev) != udev)
> + return udev;
> +
> + /* We are being called normally, use the parent pointer */
> + if (device_get_uclass_id(parent) == UCLASS_USB_HUB)
> + return dev_get_parentdata(parent);
> +
> + return NULL;
> +}
> +#else
> static inline u16 find_tt(struct usb_device *dev)
> {
> u8 chid;
> @@ -86,4 +149,11 @@ static inline u16 find_tt(struct usb_device *dev)
>
> return (hub << 8) | chid;
> }
> +
> +static inline struct usb_device *usb_dev_get_parent(struct usb_device *dev)
> +{
> + return dev->parent;
> +}
> +#endif
> +
> #endif /* __USB_COMPAT_H__ */
> --
> 2.4.3
>

Regards,
Simon