[U-Boot] [PATCH 00/22] Convert musb host mode code to the device-model
Hi Marek and Simon,
This series started out with the idea that it would be a nice small project for the weekend, but it turned out to be a bit more work...
The main purpose of this series is to convert the musb host mode code to the device-model this has also resulted in various usb fixes / cleanups / reworking to make this possible, both in the generic usb code as well as in the device model usb code.
Given that this touches both, I think it is probably best to merge the first 15 patches through Simon's tree like we did last time, then once those are place I can merge the sunxi bits. Note this is intended for v2015.10 (ofcourse).
Note that this series is useful for a bunch more boards then just the single one the last patch updates to use musb + ehci + ohci, but that is the one I've been testing with other defconfig-s will be updated with followup patches.
Please review.
Regards,
Hans
There is no harm in declaring the function prototypes even if nothing implements them, and when CONFIG_DM_USB=y the various usb functions are available regardless of any controller drivers being enabled.
This fixes compile warnings due to missing prototypes on ARCHs where the ARCH Kconfig always enables CONFIG_DM_USB and various usb drivers.
One could argue that in the case of no controllers CONFIG_DM_USB should not be set, but this problem is typically seen during bringup of boards which do actually have usb controllers.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- Changes in v2: -Simply always define the function prototypes instead of adding yet another condition to the already unwieldly #if def ... || def ... condition --- include/usb.h | 15 --------------- 1 file changed, 15 deletions(-)
diff --git a/include/usb.h b/include/usb.h index c709ce2..dca512d 100644 --- a/include/usb.h +++ b/include/usb.h @@ -171,17 +171,6 @@ enum usb_init_type { * this is how the lowlevel part communicate with the outer world */
-#if defined(CONFIG_USB_UHCI) || defined(CONFIG_USB_OHCI) || \ - defined(CONFIG_USB_EHCI) || defined(CONFIG_USB_OHCI_NEW) || \ - defined(CONFIG_USB_SL811HS) || defined(CONFIG_USB_ISP116X_HCD) || \ - defined(CONFIG_USB_R8A66597_HCD) || defined(CONFIG_USB_DAVINCI) || \ - defined(CONFIG_USB_OMAP3) || defined(CONFIG_USB_DA8XX) || \ - defined(CONFIG_USB_BLACKFIN) || defined(CONFIG_USB_AM35X) || \ - defined(CONFIG_USB_MUSB_DSPS) || defined(CONFIG_USB_MUSB_AM35X) || \ - defined(CONFIG_USB_MUSB_OMAP2PLUS) || defined(CONFIG_USB_MUSB_SUNXI) || \ - defined(CONFIG_USB_XHCI) || defined(CONFIG_USB_DWC2) || \ - defined(CONFIG_USB_EMUL) - int usb_lowlevel_init(int index, enum usb_init_type init, void **controller); int usb_lowlevel_stop(int index);
@@ -216,12 +205,8 @@ void *poll_int_queue(struct usb_device *dev, struct int_queue *queue); * in boards init functions e.g. udc_disconnect() used for * forced device disconnection from host. */ -#elif defined(CONFIG_USB_GADGET_PXA2XX) - extern void udc_disconnect(void);
-#endif - /* * board-specific hardware initialization, called by * usb drivers and u-boot commands
On 17 June 2015 at 13:33, Hans de Goede hdegoede@redhat.com wrote:
There is no harm in declaring the function prototypes even if nothing implements them, and when CONFIG_DM_USB=y the various usb functions are available regardless of any controller drivers being enabled.
This fixes compile warnings due to missing prototypes on ARCHs where the ARCH Kconfig always enables CONFIG_DM_USB and various usb drivers.
One could argue that in the case of no controllers CONFIG_DM_USB should not be set, but this problem is typically seen during bringup of boards which do actually have usb controllers.
Signed-off-by: Hans de Goede hdegoede@redhat.com
Changes in v2: -Simply always define the function prototypes instead of adding yet another condition to the already unwieldly #if def ... || def ... condition
include/usb.h | 15 --------------- 1 file changed, 15 deletions(-)
Acked-by: Simon Glass sjg@chromium.org
On 28 June 2015 at 21:44, Simon Glass sjg@chromium.org wrote:
On 17 June 2015 at 13:33, Hans de Goede hdegoede@redhat.com wrote:
There is no harm in declaring the function prototypes even if nothing implements them, and when CONFIG_DM_USB=y the various usb functions are available regardless of any controller drivers being enabled.
This fixes compile warnings due to missing prototypes on ARCHs where the ARCH Kconfig always enables CONFIG_DM_USB and various usb drivers.
One could argue that in the case of no controllers CONFIG_DM_USB should not be set, but this problem is typically seen during bringup of boards which do actually have usb controllers.
Signed-off-by: Hans de Goede hdegoede@redhat.com
Changes in v2: -Simply always define the function prototypes instead of adding yet another condition to the already unwieldly #if def ... || def ... condition
include/usb.h | 15 --------------- 1 file changed, 15 deletions(-)
Acked-by: Simon Glass sjg@chromium.org
Note: this was already applied to mainline.
The device-model usb_legacy_port_reset function calls the device-model usb_port_reset function which is a 1 on 1 copy of the non dm usb_legacy_port_reset and this is the only use of usb_port_reset in all of u-boot.
Drop both, and alway use the usb_legacy_port_reset() version in common/usb.c .
Also while at it make it static as it is only used in common/usb.c .
Signed-off-by: Hans de Goede hdegoede@redhat.com --- common/usb.c | 4 +--- drivers/usb/host/usb-uclass.c | 29 ----------------------------- include/usb.h | 8 -------- 3 files changed, 1 insertion(+), 40 deletions(-)
diff --git a/common/usb.c b/common/usb.c index 7ff8ac5..4ddf98f 100644 --- a/common/usb.c +++ b/common/usb.c @@ -911,8 +911,7 @@ __weak int usb_alloc_device(struct usb_device *udev) } #endif /* !CONFIG_DM_USB */
-#ifndef CONFIG_DM_USB -int usb_legacy_port_reset(struct usb_device *hub, int portnr) +static int usb_legacy_port_reset(struct usb_device *hub, int portnr) { if (hub) { unsigned short portstatus; @@ -930,7 +929,6 @@ int usb_legacy_port_reset(struct usb_device *hub, int portnr)
return 0; } -#endif
static int get_descriptor_len(struct usb_device *dev, int len, int expect_len) { diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c index 6e86f4a..10d4712 100644 --- a/drivers/usb/host/usb-uclass.c +++ b/drivers/usb/host/usb-uclass.c @@ -310,35 +310,6 @@ int usb_post_bind(struct udevice *dev) return dm_scan_fdt_node(dev, gd->fdt_blob, dev->of_offset, false); }
-int usb_port_reset(struct usb_device *parent, int portnr) -{ - unsigned short portstatus; - int ret; - - debug("%s: start\n", __func__); - - if (parent) { - /* reset the port for the second time */ - assert(portnr > 0); - debug("%s: reset %d\n", __func__, portnr - 1); - ret = legacy_hub_port_reset(parent, portnr - 1, &portstatus); - if (ret < 0) { - printf("\n Couldn't reset port %i\n", portnr); - return ret; - } - } else { - debug("%s: reset root\n", __func__); - usb_reset_root_port(); - } - - return 0; -} - -int usb_legacy_port_reset(struct usb_device *parent, int portnr) -{ - return usb_port_reset(parent, portnr); -} - int usb_setup_ehci_gadget(struct ehci_ctrl **ctlrp) { struct usb_platdata *plat; diff --git a/include/usb.h b/include/usb.h index dca512d..6b5d536 100644 --- a/include/usb.h +++ b/include/usb.h @@ -730,14 +730,6 @@ int usb_reset_root_port(void); struct usb_device *usb_get_dev_index(struct udevice *bus, int index);
/** - * usb_legacy_port_reset() - Legacy function to reset a hub port - * - * @hub: Hub device - * @portnr: Port number (1=first) - */ -int usb_legacy_port_reset(struct usb_device *hub, int portnr); - -/** * usb_setup_device() - set up a device ready for use * * @dev: USB device pointer. This need not be a real device - it is
On 17 June 2015 at 13:33, Hans de Goede hdegoede@redhat.com wrote:
The device-model usb_legacy_port_reset function calls the device-model usb_port_reset function which is a 1 on 1 copy of the non dm usb_legacy_port_reset and this is the only use of usb_port_reset in all of u-boot.
Drop both, and alway use the usb_legacy_port_reset() version in common/usb.c .
Also while at it make it static as it is only used in common/usb.c .
Signed-off-by: Hans de Goede hdegoede@redhat.com
common/usb.c | 4 +--- drivers/usb/host/usb-uclass.c | 29 ----------------------------- include/usb.h | 8 -------- 3 files changed, 1 insertion(+), 40 deletions(-)
Acked-by: Simon Glass sjg@chromium.org
On 28 June 2015 at 21:44, Simon Glass sjg@chromium.org wrote:
On 17 June 2015 at 13:33, Hans de Goede hdegoede@redhat.com wrote:
The device-model usb_legacy_port_reset function cApplied to u-boot-dm, thanks!
alls the device-model
usb_port_reset function which is a 1 on 1 copy of the non dm usb_legacy_port_reset and this is the only use of usb_port_reset in all of u-boot.
Drop both, and alway use the usb_legacy_port_reset() version in common/usb.c .
Also while at it make it static as it is only used in common/usb.c .
Signed-off-by: Hans de Goede hdegoede@redhat.com
common/usb.c | 4 +--- drivers/usb/host/usb-uclass.c | 29 ----------------------------- include/usb.h | 8 -------- 3 files changed, 1 insertion(+), 40 deletions(-)
Acked-by: Simon Glass sjg@chromium.org
Applied to u-boot-dm/next, thanks!
Drop the unneeded portnr function argument, the portnr is part of the usb_device struct which is passed via the dev argument.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- common/usb.c | 10 +++++----- drivers/usb/host/usb-uclass.c | 2 +- include/usb.h | 6 +++--- 3 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/common/usb.c b/common/usb.c index 4ddf98f..d237478 100644 --- a/common/usb.c +++ b/common/usb.c @@ -1030,7 +1030,7 @@ static int usb_setup_descriptor(struct usb_device *dev, bool do_read) }
static int usb_prepare_device(struct usb_device *dev, int addr, bool do_read, - struct usb_device *parent, int portnr) + struct usb_device *parent) { int err;
@@ -1048,7 +1048,7 @@ static int usb_prepare_device(struct usb_device *dev, int addr, bool do_read, err = usb_setup_descriptor(dev, do_read); if (err) return err; - err = usb_legacy_port_reset(parent, portnr); + err = usb_legacy_port_reset(parent, dev->portnr); if (err) return err;
@@ -1126,7 +1126,7 @@ int usb_select_config(struct usb_device *dev) }
int usb_setup_device(struct usb_device *dev, bool do_read, - struct usb_device *parent, int portnr) + struct usb_device *parent) { int addr; int ret; @@ -1135,7 +1135,7 @@ int usb_setup_device(struct usb_device *dev, bool do_read, addr = dev->devnum; dev->devnum = 0;
- ret = usb_prepare_device(dev, addr, do_read, parent, portnr); + ret = usb_prepare_device(dev, addr, do_read, parent); if (ret) return ret; ret = usb_select_config(dev); @@ -1165,7 +1165,7 @@ int usb_new_device(struct usb_device *dev) #ifdef CONFIG_USB_XHCI do_read = false; #endif - err = usb_setup_device(dev, do_read, dev->parent, dev->portnr); + err = usb_setup_device(dev, do_read, dev->parent); if (err) return err;
diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c index 10d4712..18680c9 100644 --- a/drivers/usb/host/usb-uclass.c +++ b/drivers/usb/host/usb-uclass.c @@ -565,7 +565,7 @@ int usb_scan_device(struct udevice *parent, int port, debug("Calling usb_setup_device(), portnr=%d\n", udev->portnr); parent_udev = 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, parent_udev); 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 6b5d536..8a71e28 100644 --- a/include/usb.h +++ b/include/usb.h @@ -734,14 +734,14 @@ struct usb_device *usb_get_dev_index(struct udevice *bus, int index); * * @dev: USB device pointer. This need not be a real device - it is * common for it to just be a local variable with its ->dev - * member (i.e. @dev->dev) set to the parent device + * member (i.e. @dev->dev) set to the parent device and + * dev->portnr set to the port number on the hub (1=first) * @do_read: true to read the device descriptor before an address is set * (should be false for XHCI buses, true otherwise) * @parent: Parent device (either UCLASS_USB or UCLASS_USB_HUB) - * @portnr: Port number on hub (1=first) or 0 for none * @return 0 if OK, -ve on error */ int usb_setup_device(struct usb_device *dev, bool do_read, - struct usb_device *parent, int portnr); + struct usb_device *parent);
/** * usb_hub_scan() - Scan a hub and find its devices
Hi Hans.
On 17 June 2015 at 13:33, Hans de Goede hdegoede@redhat.com wrote:
Drop the unneeded portnr function argument, the portnr is part of the usb_device struct which is passed via the dev argument.
Signed-off-by: Hans de Goede hdegoede@redhat.com
common/usb.c | 10 +++++----- drivers/usb/host/usb-uclass.c | 2 +- include/usb.h | 6 +++--- 3 files changed, 9 insertions(+), 9 deletions(-)
This was needed in the case where a fake usb_device was passed in. Has your previous refactoring changed that?
Regards, Simon
Hi,
On 29-06-15 05:44, Simon Glass wrote:
Hi Hans.
On 17 June 2015 at 13:33, Hans de Goede hdegoede@redhat.com wrote:
Drop the unneeded portnr function argument, the portnr is part of the usb_device struct which is passed via the dev argument.
Signed-off-by: Hans de Goede hdegoede@redhat.com
common/usb.c | 10 +++++----- drivers/usb/host/usb-uclass.c | 2 +- include/usb.h | 6 +++--- 3 files changed, 9 insertions(+), 9 deletions(-)
This was needed in the case where a fake usb_device was passed in. Has your previous refactoring changed that?
The portnr is still passed but it is padded via the usb_device struct's portnr member. When doing a CONFIG_DM_USB=y build the only call site of usb_setup_device() is usb_scan_device() from drivers/usb/host/usb-uclass.c which does:
udev->portnr = port; debug("Calling usb_setup_device(), portnr=%d\n", udev->portnr); parent_udev = device_get_uclass_id(parent) == UCLASS_USB_HUB ? dev_get_parentdata(parent) : NULL; ret = usb_setup_device(udev, priv->desc_before_addr, parent_udev);
So portnr is always set in the usb_device strict, and that is what gets used after this patch.
Regards,
Hans
Hi Hans,
On 30 June 2015 at 06:29, Hans de Goede hdegoede@redhat.com wrote:
Hi,
On 29-06-15 05:44, Simon Glass wrote:
Hi Hans.
On 17 June 2015 at 13:33, Hans de Goede hdegoede@redhat.com wrote:
Drop the unneeded portnr function argument, the portnr is part of the usb_device struct which is passed via the dev argument.
Signed-off-by: Hans de Goede hdegoede@redhat.com
common/usb.c | 10 +++++----- drivers/usb/host/usb-uclass.c | 2 +- include/usb.h | 6 +++--- 3 files changed, 9 insertions(+), 9 deletions(-)
This was needed in the case where a fake usb_device was passed in. Has your previous refactoring changed that?
The portnr is still passed but it is padded via the usb_device struct's portnr member. When doing a CONFIG_DM_USB=y build the only call site of usb_setup_device() is usb_scan_device() from drivers/usb/host/usb-uclass.c which does:
udev->portnr = port; debug("Calling usb_setup_device(), portnr=%d\n", udev->portnr); parent_udev = device_get_uclass_id(parent) == UCLASS_USB_HUB ? dev_get_parentdata(parent) : NULL; ret = usb_setup_device(udev, priv->desc_before_addr, parent_udev);So portnr is always set in the usb_device strict, and that is what gets used after this patch.
OK thanks for explaining that.
Acked-by: Simon Glass sjg@chromium.org
Regards, Simon
On 30 June 2015 at 08:51, Simon Glass sjg@chromium.org wrote:
Hi Hans,
On 30 June 2015 at 06:29, Hans de Goede hdegoede@redhat.com wrote:
Hi,
On 29-06-15 05:44, Simon Glass wrote:
Hi Hans.
On 17 June 2015 at 13:33, Hans de Goede hdegoede@redhat.com wrote:
Drop the unneeded portnr function argument, the portnr is part of the usb_device struct which is passed via the dev argument.
Signed-off-by: Hans de Goede hdegoede@redhat.com
common/usb.c | 10 +++++----- drivers/usb/host/usb-uclass.c | 2 +- include/usb.h | 6 +++--- 3 files changed, 9 insertions(+), 9 deletions(-)
This was needed in the case where a fake usb_device was passed in. Has your previous refactoring changed that?
The portnr is still passed but it is padded via the usb_device struct's portnr member. When doing a CONFIG_DM_USB=y build the only call site of usb_setup_device() is usb_scan_device() from drivers/usb/host/usb-uclass.c which does:
udev->portnr = port; debug("Calling usb_setup_device(), portnr=%d\n", udev->portnr); parent_udev = device_get_uclass_id(parent) == UCLASS_USB_HUB ? dev_get_parentdata(parent) : NULL; ret = usb_setup_device(udev, priv->desc_before_addr, parent_udev);So portnr is always set in the usb_device strict, and that is what gets used after this patch.
OK thanks for explaining that.
Acked-by: Simon Glass sjg@chromium.org
Applied to u-boot-dm/next, thanks!
Pass the usb_device instead of the portnr to usb_legacy_port_reset and rename it to usb_hub_port_reset as there is nothing legacy about it.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- common/usb.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/common/usb.c b/common/usb.c index d237478..d204ba2 100644 --- a/common/usb.c +++ b/common/usb.c @@ -911,16 +911,16 @@ __weak int usb_alloc_device(struct usb_device *udev) } #endif /* !CONFIG_DM_USB */
-static int usb_legacy_port_reset(struct usb_device *hub, int portnr) +static int usb_hub_port_reset(struct usb_device *dev, struct usb_device *hub) { if (hub) { unsigned short portstatus; int err;
/* reset the port for the second time */ - err = legacy_hub_port_reset(hub, portnr - 1, &portstatus); + err = legacy_hub_port_reset(hub, dev->portnr - 1, &portstatus); if (err < 0) { - printf("\n Couldn't reset port %i\n", portnr); + printf("\n Couldn't reset port %i\n", dev->portnr); return err; } } else { @@ -1048,7 +1048,7 @@ static int usb_prepare_device(struct usb_device *dev, int addr, bool do_read, err = usb_setup_descriptor(dev, do_read); if (err) return err; - err = usb_legacy_port_reset(parent, dev->portnr); + err = usb_hub_port_reset(dev, parent); if (err) return err;
Hi Hans,
On 17 June 2015 at 13:33, Hans de Goede hdegoede@redhat.com wrote:
Pass the usb_device instead of the portnr to usb_legacy_port_reset and rename it to usb_hub_port_reset as there is nothing legacy about it.
Signed-off-by: Hans de Goede hdegoede@redhat.com
common/usb.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
Legacy as in not driver model.
Regards, Simon
Hi,
On 29-06-15 05:44, Simon Glass wrote:
Hi Hans,
On 17 June 2015 at 13:33, Hans de Goede hdegoede@redhat.com wrote:
Pass the usb_device instead of the portnr to usb_legacy_port_reset and rename it to usb_hub_port_reset as there is nothing legacy about it.
Signed-off-by: Hans de Goede hdegoede@redhat.com
common/usb.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
Legacy as in not driver model.
Except that it gets used in both device-model and non device model builds of the usb-stack.
Regards,
Hans
p.s.
Thanks for reviewing this largish series!
Hi Hans,
On 30 June 2015 at 06:31, Hans de Goede hdegoede@redhat.com wrote:
Hi,
On 29-06-15 05:44, Simon Glass wrote:
Hi Hans,
On 17 June 2015 at 13:33, Hans de Goede hdegoede@redhat.com wrote:
Pass the usb_device instead of the portnr to usb_legacy_port_reset and rename it to usb_hub_port_reset as there is nothing legacy about it.
Signed-off-by: Hans de Goede hdegoede@redhat.com
common/usb.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
Legacy as in not driver model.
Except that it gets used in both device-model and non device model builds of the usb-stack.
Yes, and the non-driver-model stuff can presumably be considered 'legacy' at this point?
Acked-by: Simon Glass sjg@chromium.org
Regards,
Hans
p.s.
Thanks for reviewing this largish series!
You're welcome. It's a lot easier than writing it :-)
Regards, Simon
On 30 June 2015 at 08:58, Simon Glass sjg@chromium.org wrote:
Hi Hans,
On 30 June 2015 at 06:31, Hans de Goede hdegoede@redhat.com wrote:
Hi,
On 29-06-15 05:44, Simon Glass wrote:
Hi Hans,
On 17 June 2015 at 13:33, Hans de Goede hdegoede@redhat.com wrote:
Pass the usb_device instead of the portnr to usb_legacy_port_reset and rename it to usb_hub_port_reset as there is nothing legacy about it.
Signed-off-by: Hans de Goede hdegoede@redhat.com
common/usb.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
Legacy as in not driver model.
Except that it gets used in both device-model and non device model builds of the usb-stack.
Yes, and the non-driver-model stuff can presumably be considered 'legacy' at this point?
Acked-by: Simon Glass sjg@chromium.org
Regards,
Hans
p.s.
Thanks for reviewing this largish series!
You're welcome. It's a lot easier than writing it :-)
Regards, Simon
Applied to u-boot-dm/next, thanks!
Add an usb_device parameter to usb_reset_root_port so that it knows which root-port it is resetting. This is necessary for proper device-model support for usb_reset_root_port.
Also remove a duplicate declaration of usb_reset_root_port() from usb.h .
Signed-off-by: Hans de Goede hdegoede@redhat.com --- common/usb.c | 2 +- drivers/usb/host/usb-uclass.c | 2 +- drivers/usb/musb-new/musb_uboot.c | 4 ++-- include/usb.h | 8 ++------ 4 files changed, 6 insertions(+), 10 deletions(-)
diff --git a/common/usb.c b/common/usb.c index d204ba2..fbaf8ec 100644 --- a/common/usb.c +++ b/common/usb.c @@ -924,7 +924,7 @@ static int usb_hub_port_reset(struct usb_device *dev, struct usb_device *hub) return err; } } else { - usb_reset_root_port(); + usb_reset_root_port(dev); }
return 0; diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c index 18680c9..bce6cec 100644 --- a/drivers/usb/host/usb-uclass.c +++ b/drivers/usb/host/usb-uclass.c @@ -265,7 +265,7 @@ int usb_init(void) return usb_started ? 0 : -1; }
-int usb_reset_root_port(void) +int usb_reset_root_port(struct usb_device *udev) { return -ENOSYS; } diff --git a/drivers/usb/musb-new/musb_uboot.c b/drivers/usb/musb-new/musb_uboot.c index d1ee5f8..1bf676c 100644 --- a/drivers/usb/musb-new/musb_uboot.c +++ b/drivers/usb/musb-new/musb_uboot.c @@ -180,7 +180,7 @@ void *poll_int_queue(struct usb_device *dev, struct int_queue *queue) return NULL; /* URB still pending */ }
-int usb_reset_root_port(void) +int usb_reset_root_port(struct usb_device *dev) { void *mbase = host->mregs; u8 power; @@ -232,7 +232,7 @@ int usb_lowlevel_init(int index, enum usb_init_type init, void **controller) if (get_timer(0) >= timeout) return -ENODEV;
- usb_reset_root_port(); + usb_reset_root_port(NULL); host->is_active = 1; hcd.hcd_priv = host;
diff --git a/include/usb.h b/include/usb.h index 8a71e28..2bb6a06 100644 --- a/include/usb.h +++ b/include/usb.h @@ -175,9 +175,9 @@ int usb_lowlevel_init(int index, enum usb_init_type init, void **controller); int usb_lowlevel_stop(int index);
#if defined(CONFIG_MUSB_HOST) || defined(CONFIG_DM_USB) -int usb_reset_root_port(void); +int usb_reset_root_port(struct usb_device *dev); #else -#define usb_reset_root_port() +#define usb_reset_root_port(dev) #endif
int submit_bulk_msg(struct usb_device *dev, unsigned long pipe, @@ -710,10 +710,6 @@ struct dm_usb_ops { #define usb_get_ops(dev) ((struct dm_usb_ops *)(dev)->driver->ops) #define usb_get_emul_ops(dev) ((struct dm_usb_ops *)(dev)->driver->ops)
-#ifdef CONFIG_MUSB_HOST -int usb_reset_root_port(void); -#endif - /** * usb_get_dev_index() - look up a device index number *
On 17 June 2015 at 13:33, Hans de Goede hdegoede@redhat.com wrote:
Add an usb_device parameter to usb_reset_root_port so that it knows which root-port it is resetting. This is necessary for proper device-model support for usb_reset_root_port.
Also remove a duplicate declaration of usb_reset_root_port() from usb.h .
Signed-off-by: Hans de Goede hdegoede@redhat.com
common/usb.c | 2 +- drivers/usb/host/usb-uclass.c | 2 +- drivers/usb/musb-new/musb_uboot.c | 4 ++-- include/usb.h | 8 ++------ 4 files changed, 6 insertions(+), 10 deletions(-)
Acked-by: Simon Glass sjg@chromium.org
On 28 June 2015 at 21:45, Simon Glass sjg@chromium.org wrote:
On 17 June 2015 at 13:33, Hans de Goede hdegoede@redhat.com wrote:
Add an usb_device parameter to usb_reset_root_port so that it knows which root-port it is resetting. This is necessary for proper device-model support for usb_reset_root_port.
Also remove a duplicate declaration of usb_reset_root_port() from usb.h .
Signed-off-by: Hans de Goede hdegoede@redhat.com
common/usb.c | 2 +- drivers/usb/host/usb-uclass.c | 2 +- drivers/usb/musb-new/musb_uboot.c | 4 ++-- include/usb.h | 8 ++------ 4 files changed, 6 insertions(+), 10 deletions(-)
Acked-by: Simon Glass sjg@chromium.org
Applied to u-boot-dm/next, thanks!
These functions are useful to remove all children from an usb bus before rescanning the bus.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- drivers/core/device-remove.c | 18 ++---------------- include/dm/device-internal.h | 26 ++++++++++++++++++++++++++ 2 files changed, 28 insertions(+), 16 deletions(-)
diff --git a/drivers/core/device-remove.c b/drivers/core/device-remove.c index 6a16b4f..06de7e3 100644 --- a/drivers/core/device-remove.c +++ b/drivers/core/device-remove.c @@ -18,16 +18,7 @@ #include <dm/uclass-internal.h> #include <dm/util.h>
-/** - * device_chld_unbind() - Unbind all device's children from the device - * - * On error, the function continues to unbind all children, and reports the - * first error. - * - * @dev: The device that is to be stripped of its children - * @return 0 on success, -ve on error - */ -static int device_chld_unbind(struct udevice *dev) +int device_chld_unbind(struct udevice *dev) { struct udevice *pos, *n; int ret, saved_ret = 0; @@ -43,12 +34,7 @@ static int device_chld_unbind(struct udevice *dev) return saved_ret; }
-/** - * device_chld_remove() - Stop all device's children - * @dev: The device whose children are to be removed - * @return 0 on success, -ve on error - */ -static int device_chld_remove(struct udevice *dev) +int device_chld_remove(struct udevice *dev) { struct udevice *pos, *n; int ret; diff --git a/include/dm/device-internal.h b/include/dm/device-internal.h index 687462b..6c8fe23 100644 --- a/include/dm/device-internal.h +++ b/include/dm/device-internal.h @@ -107,6 +107,32 @@ int device_unbind(struct udevice *dev); static inline int device_unbind(struct udevice *dev) { return 0; } #endif
+/** + * device_chld_remove() - Stop all device's children + * @dev: The device whose children are to be removed + * @return 0 on success, -ve on error + */ +#ifdef CONFIG_DM_DEVICE_REMOVE +int device_chld_remove(struct udevice *dev); +#else +static inline int device_chld_remove(struct udevice *dev) { return 0; } +#endif + +/** + * device_chld_unbind() - Unbind all device's children from the device + * + * On error, the function continues to unbind all children, and reports the + * first error. + * + * @dev: The device that is to be stripped of its children + * @return 0 on success, -ve on error + */ +#ifdef CONFIG_DM_DEVICE_REMOVE +int device_chld_unbind(struct udevice *dev); +#else +static inline int device_chld_unbind(struct udevice *dev) { return 0; } +#endif + #ifdef CONFIG_DM_DEVICE_REMOVE void device_free(struct udevice *dev); #else
Hi Hans,
On 17 June 2015 at 13:33, Hans de Goede hdegoede@redhat.com wrote:
These functions are useful to remove all children from an usb bus before rescanning the bus.
Signed-off-by: Hans de Goede hdegoede@redhat.com
drivers/core/device-remove.c | 18 ++---------------- include/dm/device-internal.h | 26 ++++++++++++++++++++++++++ 2 files changed, 28 insertions(+), 16 deletions(-)
diff --git a/drivers/core/device-remove.c b/drivers/core/device-remove.c index 6a16b4f..06de7e3 100644 --- a/drivers/core/device-remove.c +++ b/drivers/core/device-remove.c @@ -18,16 +18,7 @@ #include <dm/uclass-internal.h> #include <dm/util.h>
-/**
- device_chld_unbind() - Unbind all device's children from the device
- On error, the function continues to unbind all children, and reports the
- first error.
- @dev: The device that is to be stripped of its children
- @return 0 on success, -ve on error
- */
-static int device_chld_unbind(struct udevice *dev) +int device_chld_unbind(struct udevice *dev)
If we are exporting this can we give it a better name? Maybe device_unbind_children()?
{ struct udevice *pos, *n; int ret, saved_ret = 0; @@ -43,12 +34,7 @@ static int device_chld_unbind(struct udevice *dev) return saved_ret; }
-/**
- device_chld_remove() - Stop all device's children
- @dev: The device whose children are to be removed
- @return 0 on success, -ve on error
- */
-static int device_chld_remove(struct udevice *dev) +int device_chld_remove(struct udevice *dev) { struct udevice *pos, *n; int ret; diff --git a/include/dm/device-internal.h b/include/dm/device-internal.h index 687462b..6c8fe23 100644 --- a/include/dm/device-internal.h +++ b/include/dm/device-internal.h @@ -107,6 +107,32 @@ int device_unbind(struct udevice *dev); static inline int device_unbind(struct udevice *dev) { return 0; } #endif
+/**
- device_chld_remove() - Stop all device's children
- @dev: The device whose children are to be removed
- @return 0 on success, -ve on error
- */
+#ifdef CONFIG_DM_DEVICE_REMOVE +int device_chld_remove(struct udevice *dev); +#else +static inline int device_chld_remove(struct udevice *dev) { return 0; } +#endif
+/**
- device_chld_unbind() - Unbind all device's children from the device
- On error, the function continues to unbind all children, and reports the
- first error.
- @dev: The device that is to be stripped of its children
- @return 0 on success, -ve on error
- */
+#ifdef CONFIG_DM_DEVICE_REMOVE +int device_chld_unbind(struct udevice *dev); +#else +static inline int device_chld_unbind(struct udevice *dev) { return 0; } +#endif
#ifdef CONFIG_DM_DEVICE_REMOVE void device_free(struct udevice *dev);
#else
2.4.3
Regards, Simon
Hi,
On 29-06-15 05:45, Simon Glass wrote:
Hi Hans,
On 17 June 2015 at 13:33, Hans de Goede hdegoede@redhat.com wrote:
These functions are useful to remove all children from an usb bus before rescanning the bus.
Signed-off-by: Hans de Goede hdegoede@redhat.com
drivers/core/device-remove.c | 18 ++---------------- include/dm/device-internal.h | 26 ++++++++++++++++++++++++++ 2 files changed, 28 insertions(+), 16 deletions(-)
diff --git a/drivers/core/device-remove.c b/drivers/core/device-remove.c index 6a16b4f..06de7e3 100644 --- a/drivers/core/device-remove.c +++ b/drivers/core/device-remove.c @@ -18,16 +18,7 @@ #include <dm/uclass-internal.h> #include <dm/util.h>
-/**
- device_chld_unbind() - Unbind all device's children from the device
- On error, the function continues to unbind all children, and reports the
- first error.
- @dev: The device that is to be stripped of its children
- @return 0 on success, -ve on error
- */
-static int device_chld_unbind(struct udevice *dev) +int device_chld_unbind(struct udevice *dev)
If we are exporting this can we give it a better name? Maybe device_unbind_children()?
Will do for v2 of this set.
Regards,
Hans
{ struct udevice *pos, *n; int ret, saved_ret = 0; @@ -43,12 +34,7 @@ static int device_chld_unbind(struct udevice *dev) return saved_ret; }
-/**
- device_chld_remove() - Stop all device's children
- @dev: The device whose children are to be removed
- @return 0 on success, -ve on error
- */
-static int device_chld_remove(struct udevice *dev) +int device_chld_remove(struct udevice *dev) { struct udevice *pos, *n; int ret; diff --git a/include/dm/device-internal.h b/include/dm/device-internal.h index 687462b..6c8fe23 100644 --- a/include/dm/device-internal.h +++ b/include/dm/device-internal.h @@ -107,6 +107,32 @@ int device_unbind(struct udevice *dev); static inline int device_unbind(struct udevice *dev) { return 0; } #endif
+/**
- device_chld_remove() - Stop all device's children
- @dev: The device whose children are to be removed
- @return 0 on success, -ve on error
- */
+#ifdef CONFIG_DM_DEVICE_REMOVE +int device_chld_remove(struct udevice *dev); +#else +static inline int device_chld_remove(struct udevice *dev) { return 0; } +#endif
+/**
- device_chld_unbind() - Unbind all device's children from the device
- On error, the function continues to unbind all children, and reports the
- first error.
- @dev: The device that is to be stripped of its children
- @return 0 on success, -ve on error
- */
+#ifdef CONFIG_DM_DEVICE_REMOVE +int device_chld_unbind(struct udevice *dev); +#else +static inline int device_chld_unbind(struct udevice *dev) { return 0; } +#endif
- #ifdef CONFIG_DM_DEVICE_REMOVE void device_free(struct udevice *dev); #else
-- 2.4.3
Regards, Simon
last_child was abused by the old usb code to first store 1 if the usb_device was not the root of the usb tree, and then later on re-used to store whether or not the usb_device is actually the last child.
The dm-usb code was always setting it to actually reflect the last-child status which is wrong for the last child leading to output like this:
USB device tree: 1 Hub (12 Mb/s, 100mA) | ALCOR USB Hub 2.0 | | 2 Mass Storage (12 Mb/s, 100mA) | USB Flash Disk 4C0E960F | +-3 Human Interface (1.5 Mb/s, 100mA) SINO WEALTH USB Composite Device
Instead of this:
USB device tree: 1 Hub (12 Mb/s, 100mA) | ALCOR USB Hub 2.0 | +-2 Mass Storage (12 Mb/s, 100mA) | USB Flash Disk 4C0E960F | +-3 Human Interface (1.5 Mb/s, 100mA) SINO WEALTH USB Composite Device
This commit fixes this by first checking that the device is not root, and then setting last_child. This commit also updates the old code to not abuse the last_child variable to store the root check result.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- common/cmd_usb.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/common/cmd_usb.c b/common/cmd_usb.c index eab55cd..ca06826 100644 --- a/common/cmd_usb.c +++ b/common/cmd_usb.c @@ -355,12 +355,12 @@ static void usb_show_tree_graph(struct usb_device *dev, char *pre) #endif /* check if we are the last one */ #ifdef CONFIG_DM_USB - last_child = device_is_last_sibling(dev->dev); + /* Not the root of the usb tree? */ + if (device_get_uclass_id(dev->dev->parent) != UCLASS_USB) { + last_child = device_is_last_sibling(dev->dev); #else - last_child = (dev->parent != NULL); -#endif - if (last_child) { -#ifndef CONFIG_DM_USB + if (dev->parent != NULL) { /* not root? */ + last_child = 1; for (i = 0; i < dev->parent->maxchild; i++) { /* search for children */ if (dev->parent->children[i] == dev) {
On 17 June 2015 at 13:33, Hans de Goede hdegoede@redhat.com wrote:
last_child was abused by the old usb code to first store 1 if the usb_device was not the root of the usb tree, and then later on re-used to store whether or not the usb_device is actually the last child.
The dm-usb code was always setting it to actually reflect the last-child status which is wrong for the last child leading to output like this:
USB device tree: 1 Hub (12 Mb/s, 100mA) | ALCOR USB Hub 2.0 | | 2 Mass Storage (12 Mb/s, 100mA) | USB Flash Disk 4C0E960F | +-3 Human Interface (1.5 Mb/s, 100mA) SINO WEALTH USB Composite Device
Instead of this:
USB device tree: 1 Hub (12 Mb/s, 100mA) | ALCOR USB Hub 2.0 | +-2 Mass Storage (12 Mb/s, 100mA) | USB Flash Disk 4C0E960F | +-3 Human Interface (1.5 Mb/s, 100mA) SINO WEALTH USB Composite Device
This commit fixes this by first checking that the device is not root, and then setting last_child. This commit also updates the old code to not abuse the last_child variable to store the root check result.
Signed-off-by: Hans de Goede hdegoede@redhat.com
common/cmd_usb.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
Acked-by: Simon Glass sjg@chromium.org
On 28 June 2015 at 21:45, Simon Glass sjg@chromium.org wrote:
On 17 June 2015 at 13:33, Hans de Goede hdegoede@redhat.com wrote:
last_child was abused by the old usb code to first store 1 if the usb_device was not the root of the usb tree, and then later on re-used to store whether or not the usb_device is actually the last child.
The dm-usb code was always setting it to actually reflect the last-child status which is wrong for the last child leading to output like this:
USB device tree: 1 Hub (12 Mb/s, 100mA) | ALCOR USB Hub 2.0 | | 2 Mass Storage (12 Mb/s, 100mA) | USB Flash Disk 4C0E960F | +-3 Human Interface (1.5 Mb/s, 100mA) SINO WEALTH USB Composite Device
Instead of this:
USB device tree: 1 Hub (12 Mb/s, 100mA) | ALCOR USB Hub 2.0 | +-2 Mass Storage (12 Mb/s, 100mA) | USB Flash Disk 4C0E960F | +-3 Human Interface (1.5 Mb/s, 100mA) SINO WEALTH USB Composite Device
This commit fixes this by first checking that the device is not root, and then setting last_child. This commit also updates the old code to not abuse the last_child variable to store the root check result.
Signed-off-by: Hans de Goede hdegoede@redhat.com
common/cmd_usb.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
Acked-by: Simon Glass sjg@chromium.org
Applied to u-boot-dm/next, thanks!
On an usb stop instead of leaving orphan usb devices behind simply remove them. This requires CONFIG_DM_DEVICE_REMOVE to be set, so only build usb_stop() when that is set.
The result of this commit is best seen in the output of "dm tree" after plugging out an usb hub with 2 devices plugges in and plugging in a keyb. instead, before this commit the output would be:
usb [ + ] `-- sunxi-musb usb_hub [ ] |-- usb_hub usb_mass_st [ ] | |-- usb_mass_storage usb_dev_gen [ ] | `-- generic_bus_0_dev_3 usb_dev_gen [ + ] `-- generic_bus_0_dev_1
Notice the non active usb_hub child and its 2 non active children. The first child being non-active as in this example also causes usb_get_dev_index to return NULL when probing the first child, which results in the usb kbd code not binding to the keyboard.
With this commit in place the output after swapping and "usb reset" is:
usb [ + ] `-- sunxi-musb usb_dev_gen [ + ] `-- generic_bus_0_dev_1
As expected, and usb_get_dev_index works properly and the keyboard works.
After this commit usb_find_child() is only necessary for emulated usb devices, so make its body #ifdef CONFIG_USB_EMUL.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- drivers/usb/host/usb-uclass.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c index bce6cec..8f26e35 100644 --- a/drivers/usb/host/usb-uclass.c +++ b/drivers/usb/host/usb-uclass.c @@ -128,6 +128,7 @@ int usb_alloc_device(struct usb_device *udev) return ops->alloc_device(bus, udev); }
+#ifdef CONFIG_DM_DEVICE_REMOVE int usb_stop(void) { struct udevice *bus; @@ -143,6 +144,12 @@ int usb_stop(void) uc_priv = uc->priv;
uclass_foreach_dev(bus, uc) { + ret = device_chld_remove(bus); + if (ret && !err) + err = ret; + ret = device_chld_unbind(bus); + if (ret && !err) + err = ret; ret = device_remove(bus); if (ret && !err) err = ret; @@ -166,6 +173,7 @@ int usb_stop(void)
return err; } +#endif
static void usb_scan_bus(struct udevice *bus, bool recurse) { @@ -491,6 +499,7 @@ static int usb_find_child(struct udevice *parent, struct usb_interface_descriptor *iface, struct udevice **devp) { +#ifdef CONFIG_USB_EMUL /* Emulated devices are explictily bound */ struct udevice *dev;
*devp = NULL; @@ -509,6 +518,7 @@ static int usb_find_child(struct udevice *parent, return 0; } } +#endif
return -ENOENT; }
Hi Hans,
On 17 June 2015 at 13:33, Hans de Goede hdegoede@redhat.com wrote:
On an usb stop instead of leaving orphan usb devices behind simply remove
On a usb_stop()
or
On a 'usb stop' command
?
them. This requires CONFIG_DM_DEVICE_REMOVE to be set, so only build usb_stop() when that is set.
This seems a little unfortunate. I can see the reasoning, but do you think this is necessary? I suspect people chasing code size may remove that option and still want to use USB properly.
The result of this commit is best seen in the output of "dm tree" after plugging out an usb hub with 2 devices plugges in and plugging in a keyb. instead, before this commit the output would be:
usb [ + ] `-- sunxi-musb usb_hub [ ] |-- usb_hub usb_mass_st [ ] | |-- usb_mass_storage usb_dev_gen [ ] | `-- generic_bus_0_dev_3 usb_dev_gen [ + ] `-- generic_bus_0_dev_1
Notice the non active usb_hub child and its 2 non active children. The first child being non-active as in this example also causes usb_get_dev_index to return NULL when probing the first child, which results in the usb kbd code not binding to the keyboard.
Although I suspect that could be fixed.
With this commit in place the output after swapping and "usb reset" is:
usb [ + ] `-- sunxi-musb usb_dev_gen [ + ] `-- generic_bus_0_dev_1
As expected, and usb_get_dev_index works properly and the keyboard works.
After this commit usb_find_child() is only necessary for emulated usb devices, so make its body #ifdef CONFIG_USB_EMUL.
Signed-off-by: Hans de Goede hdegoede@redhat.com
drivers/usb/host/usb-uclass.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c index bce6cec..8f26e35 100644 --- a/drivers/usb/host/usb-uclass.c +++ b/drivers/usb/host/usb-uclass.c @@ -128,6 +128,7 @@ int usb_alloc_device(struct usb_device *udev) return ops->alloc_device(bus, udev); }
+#ifdef CONFIG_DM_DEVICE_REMOVE int usb_stop(void) { struct udevice *bus; @@ -143,6 +144,12 @@ int usb_stop(void) uc_priv = uc->priv;
uclass_foreach_dev(bus, uc) {
ret = device_chld_remove(bus);if (ret && !err)err = ret;ret = device_chld_unbind(bus);if (ret && !err)err = ret; ret = device_remove(bus); if (ret && !err) err = ret;@@ -166,6 +173,7 @@ int usb_stop(void)
return err;} +#endif
static void usb_scan_bus(struct udevice *bus, bool recurse) { @@ -491,6 +499,7 @@ static int usb_find_child(struct udevice *parent, struct usb_interface_descriptor *iface, struct udevice **devp) { +#ifdef CONFIG_USB_EMUL /* Emulated devices are explictily bound */
explicitly
Can you add a comment about this? It seems that we should rename this function to usb_find_emul_child() and have it present only when CONFIG_USB_EMUL is around?
Also, why bother with the #ifdef if this function is never called outside sandbox?
struct udevice *dev; *devp = NULL;@@ -509,6 +518,7 @@ static int usb_find_child(struct udevice *parent, return 0; } } +#endif
return -ENOENT;}
2.4.3
Regards, Simon
Hi,
On 29-06-15 05:45, Simon Glass wrote:
Hi Hans,
On 17 June 2015 at 13:33, Hans de Goede hdegoede@redhat.com wrote:
On an usb stop instead of leaving orphan usb devices behind simply remove
On a usb_stop()
or
On a 'usb stop' command ?
My intention was for both, since I was under the assumption that "usb stop" on the cmdline, was the only caller of usb_stop(), but a quick grep to the sources show that I'm wrong ...
them. This requires CONFIG_DM_DEVICE_REMOVE to be set, so only build usb_stop() when that is set.
This seems a little unfortunate. I can see the reasoning, but do you think this is necessary? I suspect people chasing code size may remove that option and still want to use USB properly.
This was mostly a result of my thinking that usb_stop() is only used on "usb stop" at the cmdline, which I know realize is wrong.
However my quick grep has learned that we do really need CONFIG_DM_DEVICE_REMOVE to properly implement usb_stop():
From common/bootm.c :
#if defined(CONFIG_CMD_USB) /* * turn off USB to prevent the host controller from writing to the * SDRAM while Linux is booting. This could happen (at least for OHCI * controller), because the HCCA (Host Controller Communication Area) * lies within the SDRAM and the host controller writes continously to * this area (as busmaster!). The HccaFrameNumber is for example * updated every 1 ms within the HCCA structure in SDRAM! For more * details see the OpenHCI specification. */ usb_stop(); #endif
And without CONFIG_DM_DEVICE_REMOVE we end up never calling the hcd's remove callback and thus do not properly stop the usb controller.
So this problem of usb_stop() needing CONFIG_DM_DEVICE_REMOVE already exists before this patch. If you want I can split out the adding of the #ifdef in a separate commit, spelling out why usb_stop() MUST have CONFIG_DM_DEVICE_REMOVE in the commit message. Or maybe just move this all to Kconfig and make DM_USB conflict with CONFIG_DM_DEVICE_REMOVE?
The result of this commit is best seen in the output of "dm tree" after plugging out an usb hub with 2 devices plugges in and plugging in a keyb. instead, before this commit the output would be:
usb [ + ] `-- sunxi-musb usb_hub [ ] |-- usb_hub usb_mass_st [ ] | |-- usb_mass_storage usb_dev_gen [ ] | `-- generic_bus_0_dev_3 usb_dev_gen [ + ] `-- generic_bus_0_dev_1
Notice the non active usb_hub child and its 2 non active children. The first child being non-active as in this example also causes usb_get_dev_index to return NULL when probing the first child, which results in the usb kbd code not binding to the keyboard.
Although I suspect that could be fixed.
Right, but just removing the children is a much cleaner solution, and also makes the output of "dm tree" properly reflect reality.
With this commit in place the output after swapping and "usb reset" is:
usb [ + ] `-- sunxi-musb usb_dev_gen [ + ] `-- generic_bus_0_dev_1
As expected, and usb_get_dev_index works properly and the keyboard works.
After this commit usb_find_child() is only necessary for emulated usb devices, so make its body #ifdef CONFIG_USB_EMUL.
Signed-off-by: Hans de Goede hdegoede@redhat.com
drivers/usb/host/usb-uclass.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c index bce6cec..8f26e35 100644 --- a/drivers/usb/host/usb-uclass.c +++ b/drivers/usb/host/usb-uclass.c @@ -128,6 +128,7 @@ int usb_alloc_device(struct usb_device *udev) return ops->alloc_device(bus, udev); }
+#ifdef CONFIG_DM_DEVICE_REMOVE int usb_stop(void) { struct udevice *bus; @@ -143,6 +144,12 @@ int usb_stop(void) uc_priv = uc->priv;
uclass_foreach_dev(bus, uc) {
ret = device_chld_remove(bus);if (ret && !err)err = ret;ret = device_chld_unbind(bus);if (ret && !err)err = ret; ret = device_remove(bus); if (ret && !err) err = ret;@@ -166,6 +173,7 @@ int usb_stop(void)
return err;} +#endif
static void usb_scan_bus(struct udevice *bus, bool recurse) { @@ -491,6 +499,7 @@ static int usb_find_child(struct udevice *parent, struct usb_interface_descriptor *iface, struct udevice **devp) { +#ifdef CONFIG_USB_EMUL /* Emulated devices are explictily bound */
explicitly
Ack.
Can you add a comment about this? It seems that we should rename this function to usb_find_emul_child() and have it present only when CONFIG_USB_EMUL is around?
Renaming it to usb_find_emul_child() and only defining the function when CONFIG_USB_EMUL works for me I will do that for v2.
Also, why bother with the #ifdef if this function is never called outside sandbox?
Because its call site does not have a #ifdef, I'll add an #ifdef at its call site to make it more clear that this is only used for emulated devices.
struct udevice *dev; *devp = NULL;@@ -509,6 +518,7 @@ static int usb_find_child(struct udevice *parent, return 0; } } +#endif
return -ENOENT;}
2.4.3
Regards, Simon
Regards,
Hans
Hi Hans,
On 30 June 2015 at 06:54, Hans de Goede hdegoede@redhat.com wrote:
Hi,
On 29-06-15 05:45, Simon Glass wrote:
Hi Hans,
On 17 June 2015 at 13:33, Hans de Goede hdegoede@redhat.com wrote:
On an usb stop instead of leaving orphan usb devices behind simply remove
On a usb_stop()
or
On a 'usb stop' command ?
My intention was for both, since I was under the assumption that "usb stop" on the cmdline, was the only caller of usb_stop(), but a quick grep to the sources show that I'm wrong ...
them. This requires CONFIG_DM_DEVICE_REMOVE to be set, so only build usb_stop() when that is set.
This seems a little unfortunate. I can see the reasoning, but do you think this is necessary? I suspect people chasing code size may remove that option and still want to use USB properly.
This was mostly a result of my thinking that usb_stop() is only used on "usb stop" at the cmdline, which I know realize is wrong.
However my quick grep has learned that we do really need CONFIG_DM_DEVICE_REMOVE to properly implement usb_stop():
From common/bootm.c :
#if defined(CONFIG_CMD_USB) /* * turn off USB to prevent the host controller from writing to the * SDRAM while Linux is booting. This could happen (at least for OHCI * controller), because the HCCA (Host Controller Communication Area) * lies within the SDRAM and the host controller writes continously to * this area (as busmaster!). The HccaFrameNumber is for example * updated every 1 ms within the HCCA structure in SDRAM! For more * details see the OpenHCI specification. */ usb_stop(); #endif
And without CONFIG_DM_DEVICE_REMOVE we end up never calling the hcd's remove callback and thus do not properly stop the usb controller.
So this problem of usb_stop() needing CONFIG_DM_DEVICE_REMOVE already exists before this patch. If you want I can split out the adding of the #ifdef in a separate commit, spelling out why usb_stop() MUST have CONFIG_DM_DEVICE_REMOVE in the commit message. Or maybe just move this all to Kconfig and make DM_USB conflict with CONFIG_DM_DEVICE_REMOVE?
I don't think that is necessary, it feels a bit too inflexible. But perhaps you could add a comment to the Kconfig help for CONFIG_DM_DEVICE_REMOVE?
It is remove() that is needed, not unbind(). Actually I think it is quite unfortunate to make usb_stop() call unbind. It is a waste of time to do this just before booting the kernel - the current design leaves all devices bound (but I hope we can remove() them at some point).
Instead, I wonder if we can remove the children when we probe the bus? Also, what happens to children that are in the device tree - i.e. static USB devices like WiFi? The device tree might have parameters for them. Still, that might not matter as I'm not sure that case is handled correctly today.
The result of this commit is best seen in the output of "dm tree" after plugging out an usb hub with 2 devices plugges in and plugging in a keyb. instead, before this commit the output would be:
usb [ + ] `-- sunxi-musb usb_hub [ ] |-- usb_hub usb_mass_st [ ] | |-- usb_mass_storage usb_dev_gen [ ] | `-- generic_bus_0_dev_3 usb_dev_gen [ + ] `-- generic_bus_0_dev_1
Notice the non active usb_hub child and its 2 non active children. The first child being non-active as in this example also causes usb_get_dev_index to return NULL when probing the first child, which results in the usb kbd code not binding to the keyboard.
Although I suspect that could be fixed.
Right, but just removing the children is a much cleaner solution, and also makes the output of "dm tree" properly reflect reality.
True, although you also have 'usb tree' for that. Another option would be to mark devices that were found and remove the others after the scan.
With this commit in place the output after swapping and "usb reset" is:
usb [ + ] `-- sunxi-musb usb_dev_gen [ + ] `-- generic_bus_0_dev_1
As expected, and usb_get_dev_index works properly and the keyboard works.
After this commit usb_find_child() is only necessary for emulated usb devices, so make its body #ifdef CONFIG_USB_EMUL.
Signed-off-by: Hans de Goede hdegoede@redhat.com
drivers/usb/host/usb-uclass.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c index bce6cec..8f26e35 100644 --- a/drivers/usb/host/usb-uclass.c +++ b/drivers/usb/host/usb-uclass.c @@ -128,6 +128,7 @@ int usb_alloc_device(struct usb_device *udev) return ops->alloc_device(bus, udev); }
+#ifdef CONFIG_DM_DEVICE_REMOVE int usb_stop(void) { struct udevice *bus; @@ -143,6 +144,12 @@ int usb_stop(void) uc_priv = uc->priv;
uclass_foreach_dev(bus, uc) {
ret = device_chld_remove(bus);if (ret && !err)err = ret;ret = device_chld_unbind(bus);if (ret && !err)err = ret; ret = device_remove(bus); if (ret && !err) err = ret;@@ -166,6 +173,7 @@ int usb_stop(void)
return err;} +#endif
static void usb_scan_bus(struct udevice *bus, bool recurse) { @@ -491,6 +499,7 @@ static int usb_find_child(struct udevice *parent, struct usb_interface_descriptor *iface, struct udevice **devp) { +#ifdef CONFIG_USB_EMUL /* Emulated devices are explictily bound */
explicitly
Ack.
Can you add a comment about this? It seems that we should rename this function to usb_find_emul_child() and have it present only when CONFIG_USB_EMUL is around?
Renaming it to usb_find_emul_child() and only defining the function when CONFIG_USB_EMUL works for me I will do that for v2.
Also, why bother with the #ifdef if this function is never called outside sandbox?
Because its call site does not have a #ifdef, I'll add an #ifdef at its call site to make it more clear that this is only used for emulated devices.
struct udevice *dev; *devp = NULL;@@ -509,6 +518,7 @@ static int usb_find_child(struct udevice *parent, return 0; } } +#endif
return -ENOENT;}
2.4.3
Regards, Simon
Hi,
On 06/30/2015 04:58 PM, Simon Glass wrote:
Hi Hans,
On 30 June 2015 at 06:54, Hans de Goede hdegoede@redhat.com wrote:
Hi,
On 29-06-15 05:45, Simon Glass wrote:
Hi Hans,
On 17 June 2015 at 13:33, Hans de Goede hdegoede@redhat.com wrote:
On an usb stop instead of leaving orphan usb devices behind simply remove
On a usb_stop()
or
On a 'usb stop' command ?
My intention was for both, since I was under the assumption that "usb stop" on the cmdline, was the only caller of usb_stop(), but a quick grep to the sources show that I'm wrong ...
them. This requires CONFIG_DM_DEVICE_REMOVE to be set, so only build usb_stop() when that is set.
This seems a little unfortunate. I can see the reasoning, but do you think this is necessary? I suspect people chasing code size may remove that option and still want to use USB properly.
This was mostly a result of my thinking that usb_stop() is only used on "usb stop" at the cmdline, which I know realize is wrong.
However my quick grep has learned that we do really need CONFIG_DM_DEVICE_REMOVE to properly implement usb_stop():
From common/bootm.c :
#if defined(CONFIG_CMD_USB) /* * turn off USB to prevent the host controller from writing to the * SDRAM while Linux is booting. This could happen (at least for OHCI * controller), because the HCCA (Host Controller Communication Area) * lies within the SDRAM and the host controller writes continously to * this area (as busmaster!). The HccaFrameNumber is for example * updated every 1 ms within the HCCA structure in SDRAM! For more * details see the OpenHCI specification. */ usb_stop(); #endif
And without CONFIG_DM_DEVICE_REMOVE we end up never calling the hcd's remove callback and thus do not properly stop the usb controller.
So this problem of usb_stop() needing CONFIG_DM_DEVICE_REMOVE already exists before this patch. If you want I can split out the adding of the #ifdef in a separate commit, spelling out why usb_stop() MUST have CONFIG_DM_DEVICE_REMOVE in the commit message. Or maybe just move this all to Kconfig and make DM_USB conflict with CONFIG_DM_DEVICE_REMOVE?
I don't think that is necessary, it feels a bit too inflexible. But perhaps you could add a comment to the Kconfig help for CONFIG_DM_DEVICE_REMOVE?
Ok will do.
It is remove() that is needed, not unbind(). Actually I think it is quite unfortunate to make usb_stop() call unbind. It is a waste of time to do this just before booting the kernel - the current design leaves all devices bound (but I hope we can remove() them at some point).
Instead, I wonder if we can remove the children when we probe the bus?
That should work, but I do not really see any advantage in that, removing the children is not that expensive and it feels like a kludge.
Also, what happens to children that are in the device tree - i.e. static USB devices like WiFi? The device tree might have parameters for them. Still, that might not matter as I'm not sure that case is handled correctly today.
AFAIK there is no such thing as usb devices in devicetree, which makes sense as usb is a fully discoverable bus.
The result of this commit is best seen in the output of "dm tree" after plugging out an usb hub with 2 devices plugges in and plugging in a keyb. instead, before this commit the output would be:
usb [ + ] `-- sunxi-musb usb_hub [ ] |-- usb_hub usb_mass_st [ ] | |-- usb_mass_storage usb_dev_gen [ ] | `-- generic_bus_0_dev_3 usb_dev_gen [ + ] `-- generic_bus_0_dev_1
Notice the non active usb_hub child and its 2 non active children. The first child being non-active as in this example also causes usb_get_dev_index to return NULL when probing the first child, which results in the usb kbd code not binding to the keyboard.
Although I suspect that could be fixed.
Right, but just removing the children is a much cleaner solution, and also makes the output of "dm tree" properly reflect reality.
True, although you also have 'usb tree' for that. Another option would be to mark devices that were found and remove the others after the scan.
That seems like needless complexity. I believe that simply removing + unbinding the children on usb_stop is the right thing to do, and it also is the KISS solution.
Regards,
Hans
Hi Hans,
On 30 June 2015 at 09:54, Hans de Goede hdegoede@redhat.com wrote:
Hi,
On 06/30/2015 04:58 PM, Simon Glass wrote:
Hi Hans,
On 30 June 2015 at 06:54, Hans de Goede hdegoede@redhat.com wrote:
Hi,
On 29-06-15 05:45, Simon Glass wrote:
Hi Hans,
On 17 June 2015 at 13:33, Hans de Goede hdegoede@redhat.com wrote:
On an usb stop instead of leaving orphan usb devices behind simply remove
On a usb_stop()
or
On a 'usb stop' command ?
My intention was for both, since I was under the assumption that "usb stop" on the cmdline, was the only caller of usb_stop(), but a quick grep to the sources show that I'm wrong ...
them. This requires CONFIG_DM_DEVICE_REMOVE to be set, so only build usb_stop() when that is set.
This seems a little unfortunate. I can see the reasoning, but do you think this is necessary? I suspect people chasing code size may remove that option and still want to use USB properly.
This was mostly a result of my thinking that usb_stop() is only used on "usb stop" at the cmdline, which I know realize is wrong.
However my quick grep has learned that we do really need CONFIG_DM_DEVICE_REMOVE to properly implement usb_stop():
From common/bootm.c :
#if defined(CONFIG_CMD_USB) /* * turn off USB to prevent the host controller from writing to the * SDRAM while Linux is booting. This could happen (at least for OHCI * controller), because the HCCA (Host Controller Communication Area) * lies within the SDRAM and the host controller writes continously to * this area (as busmaster!). The HccaFrameNumber is for example * updated every 1 ms within the HCCA structure in SDRAM! For more * details see the OpenHCI specification. */ usb_stop(); #endif
And without CONFIG_DM_DEVICE_REMOVE we end up never calling the hcd's remove callback and thus do not properly stop the usb controller.
So this problem of usb_stop() needing CONFIG_DM_DEVICE_REMOVE already exists before this patch. If you want I can split out the adding of the #ifdef in a separate commit, spelling out why usb_stop() MUST have CONFIG_DM_DEVICE_REMOVE in the commit message. Or maybe just move this all to Kconfig and make DM_USB conflict with CONFIG_DM_DEVICE_REMOVE?
I don't think that is necessary, it feels a bit too inflexible. But perhaps you could add a comment to the Kconfig help for CONFIG_DM_DEVICE_REMOVE?
Ok will do.
It is remove() that is needed, not unbind(). Actually I think it is quite unfortunate to make usb_stop() call unbind. It is a waste of time to do this just before booting the kernel - the current design leaves all devices bound (but I hope we can remove() them at some point).
Instead, I wonder if we can remove the children when we probe the bus?
That should work, but I do not really see any advantage in that, removing the children is not that expensive and it feels like a kludge.
That's how it currently works, from what I can see in the code. But since there is a 'usb_started' boolean this is irrelevant.
Also, what happens to children that are in the device tree - i.e. static USB devices like WiFi? The device tree might have parameters for them. Still, that might not matter as I'm not sure that case is handled correctly today.
AFAIK there is no such thing as usb devices in devicetree, which makes sense as usb is a fully discoverable bus.
Sort-of. But as with PCI it is useful to be able to add settings for the devices in some cases. You can match them using vendor/device or interface IDs. Then the driver can access its settings.
That's why I'm suggesting we unbind the devices that are no-longer present.
The result of this commit is best seen in the output of "dm tree" after plugging out an usb hub with 2 devices plugges in and plugging in a keyb. instead, before this commit the output would be:
usb [ + ] `-- sunxi-musb usb_hub [ ] |-- usb_hub usb_mass_st [ ] | |-- usb_mass_storage usb_dev_gen [ ] | `-- generic_bus_0_dev_3 usb_dev_gen [ + ] `-- generic_bus_0_dev_1
Notice the non active usb_hub child and its 2 non active children. The first child being non-active as in this example also causes usb_get_dev_index to return NULL when probing the first child, which results in the usb kbd code not binding to the keyboard.
Although I suspect that could be fixed.
Right, but just removing the children is a much cleaner solution, and also makes the output of "dm tree" properly reflect reality.
True, although you also have 'usb tree' for that. Another option would be to mark devices that were found and remove the others after the scan.
That seems like needless complexity. I believe that simply removing + unbinding the children on usb_stop is the right thing to do, and it also is the KISS solution.
I'm good with the remove(), but less sure about the unbind(). The state of 'dm tree' does not bother me, and I worry that we then limit our ability to use usb_find_child() to locate a device's parameters (i.e. support for more complex devices which need settings might be harder).
For now, can we just leave this alone? I really don't want to re-visit this later.
Regards, Simon
Hi,
On 06/30/2015 06:07 PM, Simon Glass wrote:
<snip>
Instead, I wonder if we can remove the children when we probe the bus?
That should work, but I do not really see any advantage in that, removing the children is not that expensive and it feels like a kludge.
That's how it currently works, from what I can see in the code. But since there is a 'usb_started' boolean this is irrelevant.
Also, what happens to children that are in the device tree - i.e. static USB devices like WiFi? The device tree might have parameters for them. Still, that might not matter as I'm not sure that case is handled correctly today.
AFAIK there is no such thing as usb devices in devicetree, which makes sense as usb is a fully discoverable bus.
Sort-of. But as with PCI it is useful to be able to add settings for the devices in some cases. You can match them using vendor/device or interface IDs. Then the driver can access its settings.
AFAIK there is not a single example of having settings in devicetree for an usb device, since usb-devices are always 100% self describing since usb is a bus designed for hot(un)plug from the outset.
That's why I'm suggesting we unbind the devices that are no-longer present.
You're asking to make the code more complicated here using a what if reasoning with a "what if" which is likely to never happen.
The result of this commit is best seen in the output of "dm tree" after plugging out an usb hub with 2 devices plugges in and plugging in a keyb. instead, before this commit the output would be:
usb [ + ] `-- sunxi-musb usb_hub [ ] |-- usb_hub usb_mass_st [ ] | |-- usb_mass_storage usb_dev_gen [ ] | `-- generic_bus_0_dev_3 usb_dev_gen [ + ] `-- generic_bus_0_dev_1Notice the non active usb_hub child and its 2 non active children. The first child being non-active as in this example also causes usb_get_dev_index to return NULL when probing the first child, which results in the usb kbd code not binding to the keyboard.
Although I suspect that could be fixed.
Right, but just removing the children is a much cleaner solution, and also makes the output of "dm tree" properly reflect reality.
True, although you also have 'usb tree' for that. Another option would be to mark devices that were found and remove the others after the scan.
That seems like needless complexity. I believe that simply removing + unbinding the children on usb_stop is the right thing to do, and it also is the KISS solution.
I'm good with the remove(), but less sure about the unbind().
The unbind is necessary for usb_get_dev_index() to work properly, which is necessary for proper output of "usb tree" and for driver binding to work properly, without the unbind usb-keyboards will e.g. not work in certain circumstances.
The state of 'dm tree' does not bother me,
The state if dm tree is what usb_get_dev_index() works from, so if it is not in a good state, then usb_get_dev_index() will not work.
and I worry that we then limit our ability to use usb_find_child() to locate a device's parameters (i.e. support for more complex devices which need settings might be harder).
Again this is a what if reasoning for a hypothetical future problem which will likely never happen, where as the broken state of the dm tree after a "usb reset" is causing real problems.
For now, can we just leave this alone? I really don't want to re-visit this later.
Nope we cannot leave this alone, without unbinding usb devices which no longer exist, the dm tree will be broken and with it usb_get_dev_index() and through usb_get_dev_index() the keyboard driver.
Regards,
Hans
HI Hans,
On 30 June 2015 at 14:20, Hans de Goede hdegoede@redhat.com wrote:
Hi,
On 06/30/2015 06:07 PM, Simon Glass wrote:
<snip>
Instead, I wonder if we can remove the children when we probe the bus?
That should work, but I do not really see any advantage in that, removing the children is not that expensive and it feels like a kludge.
That's how it currently works, from what I can see in the code. But since there is a 'usb_started' boolean this is irrelevant.
Also, what happens to children that are in the device tree - i.e. static USB devices like WiFi? The device tree might have parameters for them. Still, that might not matter as I'm not sure that case is handled correctly today.
AFAIK there is no such thing as usb devices in devicetree, which makes sense as usb is a fully discoverable bus.
Sort-of. But as with PCI it is useful to be able to add settings for the devices in some cases. You can match them using vendor/device or interface IDs. Then the driver can access its settings.
AFAIK there is not a single example of having settings in devicetree for an usb device, since usb-devices are always 100% self describing since usb is a bus designed for hot(un)plug from the outset.
That's why I'm suggesting we unbind the devices that are no-longer present.
You're asking to make the code more complicated here using a what if reasoning with a "what if" which is likely to never happen.
> > The result of this commit is best seen in the output of "dm tree" > after > plugging out an usb hub with 2 devices plugges in and plugging in a > keyb. > instead, before this commit the output would be: > > usb [ + ] `-- sunxi-musb > usb_hub [ ] |-- usb_hub > usb_mass_st [ ] | |-- usb_mass_storage > usb_dev_gen [ ] | `-- generic_bus_0_dev_3 > usb_dev_gen [ + ] `-- generic_bus_0_dev_1 > > Notice the non active usb_hub child and its 2 non active children. > The > first child being non-active as in this example also causes > usb_get_dev_index > to return NULL when probing the first child, which results in the usb > kbd > code not binding to the keyboard.
Although I suspect that could be fixed.
Right, but just removing the children is a much cleaner solution, and also makes the output of "dm tree" properly reflect reality.
True, although you also have 'usb tree' for that. Another option would be to mark devices that were found and remove the others after the scan.
That seems like needless complexity. I believe that simply removing + unbinding the children on usb_stop is the right thing to do, and it also is the KISS solution.
I'm good with the remove(), but less sure about the unbind().
The unbind is necessary for usb_get_dev_index() to work properly, which is necessary for proper output of "usb tree" and for driver binding to work properly, without the unbind usb-keyboards will e.g. not work in certain circumstances.
The state of 'dm tree' does not bother me,
The state if dm tree is what usb_get_dev_index() works from, so if it is not in a good state, then usb_get_dev_index() will not work.
and I worry that we then limit our ability to use usb_find_child() to locate a device's parameters (i.e. support for more complex devices which need settings might be harder).
Again this is a what if reasoning for a hypothetical future problem which will likely never happen, where as the broken state of the dm tree after a "usb reset" is causing real problems.
For now, can we just leave this alone? I really don't want to re-visit this later.
Nope we cannot leave this alone, without unbinding usb devices which no longer exist, the dm tree will be broken and with it usb_get_dev_index() and through usb_get_dev_index() the keyboard driver.
We really should not be calling usb_get_dev_index() from the keyboard driver - that function is an interim port-helper. If it were ported fully to driver model it could use USB_DEVICE() like mass storage. We could fairly easily change the devnum to 0 for all devices instead of deleting them, and I suspect that would solve the problem.
But we would have to port usb ethernet which uses the same technique.
Anyway I think you've persuaded me that I might be worrying too much about what is to come. USB is not really the same as PCI in the sense that settings don't seem to be added to the device like PCI. Let's go with your approach. We still have the USB emulation stuff working, and that's the only real use case today.
Regards, Simon
Allow usb uclass host drivers to implement usb_reset_root_port, this is used by single port usb hosts which do not emulate a hub, such as otg controllers.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- drivers/usb/host/usb-uclass.c | 16 +++++++++++----- include/usb.h | 5 +++++ 2 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c index 8f26e35..afa8655 100644 --- a/drivers/usb/host/usb-uclass.c +++ b/drivers/usb/host/usb-uclass.c @@ -128,6 +128,17 @@ int usb_alloc_device(struct usb_device *udev) return ops->alloc_device(bus, udev); }
+int usb_reset_root_port(struct usb_device *udev) +{ + struct udevice *bus = udev->controller_dev; + struct dm_usb_ops *ops = usb_get_ops(bus); + + if (!ops->reset_root_port) + return -ENOSYS; + + return ops->reset_root_port(bus, udev); +} + #ifdef CONFIG_DM_DEVICE_REMOVE int usb_stop(void) { @@ -273,11 +284,6 @@ int usb_init(void) return usb_started ? 0 : -1; }
-int usb_reset_root_port(struct usb_device *udev) -{ - return -ENOSYS; -} - static struct usb_device *find_child_devnum(struct udevice *parent, int devnum) { struct usb_device *udev; diff --git a/include/usb.h b/include/usb.h index 2bb6a06..25f8543 100644 --- a/include/usb.h +++ b/include/usb.h @@ -705,6 +705,11 @@ struct dm_usb_ops { * is read). This should be NULL for EHCI, which does not need this. */ int (*alloc_device)(struct udevice *bus, struct usb_device *udev); + + /** + * reset_root_port() - Reset usb root port + */ + int (*reset_root_port)(struct udevice *bus, struct usb_device *udev); };
#define usb_get_ops(dev) ((struct dm_usb_ops *)(dev)->driver->ops)
On 17 June 2015 at 13:33, Hans de Goede hdegoede@redhat.com wrote:
Allow usb uclass host drivers to implement usb_reset_root_port, this is used by single port usb hosts which do not emulate a hub, such as otg controllers.
Signed-off-by: Hans de Goede hdegoede@redhat.com
drivers/usb/host/usb-uclass.c | 16 +++++++++++----- include/usb.h | 5 +++++ 2 files changed, 16 insertions(+), 5 deletions(-)
Acked-by: Simon Glass sjg@chromium.org
On 28 June 2015 at 21:45, Simon Glass sjg@chromium.org wrote:
On 17 June 2015 at 13:33, Hans de Goede hdegoede@redhat.com wrote:
Allow usb uclass host drivers to implement usb_reset_root_port, this is used by single port usb hosts which do not emulate a hub, such as otg controllers.
Signed-off-by: Hans de Goede hdegoede@redhat.com
drivers/usb/host/usb-uclass.c | 16 +++++++++++----- include/usb.h | 5 +++++ 2 files changed, 16 insertions(+), 5 deletions(-)
Acked-by: Simon Glass sjg@chromium.org
Applied to u-boot-dm/next, thanks!
On some single port (otg) controllers there is no emulated root hub, so the first child (if any) may be one of: UCLASS_MASS_STORAGE, UCLASS_USB_DEV_GENERIC or UCLASS_USB_HUB.
All three of these (and in the future others) are suitable for our purposes, remove the check for the device being a hub, and add a check to deal with the fact that there may be no child-dev.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- common/cmd_usb.c | 9 ++++----- drivers/usb/host/usb-uclass.c | 10 +++++----- 2 files changed, 9 insertions(+), 10 deletions(-)
diff --git a/common/cmd_usb.c b/common/cmd_usb.c index ca06826..2ed4cb4 100644 --- a/common/cmd_usb.c +++ b/common/cmd_usb.c @@ -630,12 +630,11 @@ static int do_usb(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) bus; uclass_next_device(&bus)) { struct usb_device *udev; - struct udevice *hub; + struct udevice *dev;
- device_find_first_child(bus, &hub); - if (device_get_uclass_id(hub) == UCLASS_USB_HUB && - device_active(hub)) { - udev = dev_get_parentdata(hub); + device_find_first_child(bus, &dev); + if (dev && device_active(dev)) { + udev = dev_get_parentdata(dev); usb_show_tree(udev); } } diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c index afa8655..5fcbde9 100644 --- a/drivers/usb/host/usb-uclass.c +++ b/drivers/usb/host/usb-uclass.c @@ -308,14 +308,14 @@ static struct usb_device *find_child_devnum(struct udevice *parent, int devnum)
struct usb_device *usb_get_dev_index(struct udevice *bus, int index) { - struct udevice *hub; + struct udevice *dev; int devnum = index + 1; /* Addresses are allocated from 1 on USB */
- device_find_first_child(bus, &hub); - if (device_get_uclass_id(hub) == UCLASS_USB_HUB) - return find_child_devnum(hub, devnum); + device_find_first_child(bus, &dev); + if (!dev) + return NULL;
- return NULL; + return find_child_devnum(dev, devnum); }
int usb_post_bind(struct udevice *dev)
On 17 June 2015 at 13:33, Hans de Goede hdegoede@redhat.com wrote:
On some single port (otg) controllers there is no emulated root hub, so the first child (if any) may be one of: UCLASS_MASS_STORAGE, UCLASS_USB_DEV_GENERIC or UCLASS_USB_HUB.
All three of these (and in the future others) are suitable for our purposes, remove the check for the device being a hub, and add a check to deal with the fact that there may be no child-dev.
Signed-off-by: Hans de Goede hdegoede@redhat.com
common/cmd_usb.c | 9 ++++----- drivers/usb/host/usb-uclass.c | 10 +++++----- 2 files changed, 9 insertions(+), 10 deletions(-)
Acked-by: Simon Glass sjg@chromium.org
On 28 June 2015 at 21:45, Simon Glass sjg@chromium.org wrote:
On 17 June 2015 at 13:33, Hans de Goede hdegoede@redhat.com wrote:
On some single port (otg) controllers there is no emulated root hub, so the first child (if any) may be one of: UCLASS_MASS_STORAGE, UCLASS_USB_DEV_GENERIC or UCLASS_USB_HUB.
All three of these (and in the future others) are suitable for our purposes, remove the check for the device being a hub, and add a check to deal with the fact that there may be no child-dev.
Signed-off-by: Hans de Goede hdegoede@redhat.com
common/cmd_usb.c | 9 ++++----- drivers/usb/host/usb-uclass.c | 10 +++++----- 2 files changed, 9 insertions(+), 10 deletions(-)
Acked-by: Simon Glass sjg@chromium.org
Applied to u-boot-dm/next, thanks!
Allow musb_platform_enable to return an error code and propagate it up to usb_lowlevel_init().
This allows moving the checks for an external vbus being present to be moved from platform_init to platform_enable, so that the user can unplug a charger, plug in a host adapter with a usb-device, do a "usb reset" and have things working.
This also allows adding a check for the id-pin to platform_enable, so that it can short circuit the 1s delay in usb_lowlevel_init() when no host cable is plugged in and thus waiting for a device to show up is useless.
Note that all the changes to code shared with the kernel are wrapped in the kernel.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- drivers/usb/musb-new/am35x.c | 7 +++++++ drivers/usb/musb-new/musb_core.c | 20 ++++++++++++++++++++ drivers/usb/musb-new/musb_core.h | 18 ++++++++++++++++++ drivers/usb/musb-new/musb_dsps.c | 6 ++++++ drivers/usb/musb-new/musb_uboot.c | 6 +++++- drivers/usb/musb-new/omap2430.c | 5 +++++ drivers/usb/musb-new/sunxi.c | 5 +++-- 7 files changed, 64 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/musb-new/am35x.c b/drivers/usb/musb-new/am35x.c index 857d7eb..d158454 100644 --- a/drivers/usb/musb-new/am35x.c +++ b/drivers/usb/musb-new/am35x.c @@ -100,7 +100,11 @@ struct am35x_glue { /* * am35x_musb_enable - enable interrupts */ +#ifndef __UBOOT__ static void am35x_musb_enable(struct musb *musb) +#else +static int am35x_musb_enable(struct musb *musb) +#endif { void __iomem *reg_base = musb->ctrl_base; u32 epmask; @@ -116,6 +120,9 @@ static void am35x_musb_enable(struct musb *musb) if (is_otg_enabled(musb)) musb_writel(reg_base, CORE_INTR_SRC_SET_REG, AM35X_INTR_DRVVBUS << AM35X_INTR_USB_SHIFT); +#ifdef __UBOOT__ + return 0; +#endif }
/* diff --git a/drivers/usb/musb-new/musb_core.c b/drivers/usb/musb-new/musb_core.c index 242cc30..f530af4 100644 --- a/drivers/usb/musb-new/musb_core.c +++ b/drivers/usb/musb-new/musb_core.c @@ -926,10 +926,17 @@ b_host: /* * Program the HDRC to start (enable interrupts, dma, etc.). */ +#ifndef __UBOOT__ void musb_start(struct musb *musb) +#else +int musb_start(struct musb *musb) +#endif { void __iomem *regs = musb->mregs; u8 devctl = musb_readb(regs, MUSB_DEVCTL); +#ifdef __UBOOT__ + int ret; +#endif
dev_dbg(musb->controller, "<== devctl %02x\n", devctl);
@@ -972,8 +979,21 @@ void musb_start(struct musb *musb) if ((devctl & MUSB_DEVCTL_VBUS) == MUSB_DEVCTL_VBUS) musb->is_active = 1; } + +#ifndef __UBOOT__ musb_platform_enable(musb); +#else + ret = musb_platform_enable(musb); + if (ret) { + musb->is_active = 0; + return ret; + } +#endif musb_writeb(regs, MUSB_DEVCTL, devctl); + +#ifdef __UBOOT__ + return 0; +#endif }
diff --git a/drivers/usb/musb-new/musb_core.h b/drivers/usb/musb-new/musb_core.h index 2695742..8727f64 100644 --- a/drivers/usb/musb-new/musb_core.h +++ b/drivers/usb/musb-new/musb_core.h @@ -231,7 +231,11 @@ struct musb_platform_ops { int (*init)(struct musb *musb); int (*exit)(struct musb *musb);
+#ifndef __UBOOT__ void (*enable)(struct musb *musb); +#else + int (*enable)(struct musb *musb); +#endif void (*disable)(struct musb *musb);
int (*set_mode)(struct musb *musb, u8 mode); @@ -546,7 +550,11 @@ static inline void musb_configure_ep0(struct musb *musb)
extern const char musb_driver_name[];
+#ifndef __UBOOT__ extern void musb_start(struct musb *musb); +#else +extern int musb_start(struct musb *musb); +#endif extern void musb_stop(struct musb *musb);
extern void musb_write_fifo(struct musb_hw_ep *ep, u16 len, const u8 *src); @@ -564,11 +572,21 @@ static inline void musb_platform_set_vbus(struct musb *musb, int is_on) musb->ops->set_vbus(musb, is_on); }
+#ifndef __UBOOT__ static inline void musb_platform_enable(struct musb *musb) { if (musb->ops->enable) musb->ops->enable(musb); } +#else +static inline int musb_platform_enable(struct musb *musb) +{ + if (!musb->ops->enable) + return 0; + + return musb->ops->enable(musb); +} +#endif
static inline void musb_platform_disable(struct musb *musb) { diff --git a/drivers/usb/musb-new/musb_dsps.c b/drivers/usb/musb-new/musb_dsps.c index 17ed224..8959397 100644 --- a/drivers/usb/musb-new/musb_dsps.c +++ b/drivers/usb/musb-new/musb_dsps.c @@ -156,7 +156,11 @@ struct dsps_glue { /** * dsps_musb_enable - enable interrupts */ +#ifndef __UBOOT__ static void dsps_musb_enable(struct musb *musb) +#else +static int dsps_musb_enable(struct musb *musb) +#endif { #ifndef __UBOOT__ struct device *dev = musb->controller; @@ -181,6 +185,8 @@ static void dsps_musb_enable(struct musb *musb) if (is_otg_enabled(musb)) dsps_writel(reg_base, wrp->coreintr_set, (1 << wrp->drvvbus) << wrp->usb_shift); +#else + return 0; #endif }
diff --git a/drivers/usb/musb-new/musb_uboot.c b/drivers/usb/musb-new/musb_uboot.c index 1bf676c..70e87c9 100644 --- a/drivers/usb/musb-new/musb_uboot.c +++ b/drivers/usb/musb-new/musb_uboot.c @@ -217,13 +217,17 @@ int usb_lowlevel_init(int index, enum usb_init_type init, void **controller) void *mbase; /* USB spec says it may take up to 1 second for a device to connect */ unsigned long timeout = get_timer(0) + 1000; + int ret;
if (!host) { printf("MUSB host is not registered\n"); return -ENODEV; }
- musb_start(host); + ret = musb_start(host); + if (ret) + return ret; + mbase = host->mregs; do { if (musb_readb(mbase, MUSB_DEVCTL) & MUSB_DEVCTL_HM) diff --git a/drivers/usb/musb-new/omap2430.c b/drivers/usb/musb-new/omap2430.c index 31a280e..77273a4 100644 --- a/drivers/usb/musb-new/omap2430.c +++ b/drivers/usb/musb-new/omap2430.c @@ -400,7 +400,11 @@ err1: return status; }
+#ifndef __UBOOT__ static void omap2430_musb_enable(struct musb *musb) +#else +static int omap2430_musb_enable(struct musb *musb) +#endif { #ifndef __UBOOT__ u8 devctl; @@ -445,6 +449,7 @@ static void omap2430_musb_enable(struct musb *musb) __PRETTY_FUNCTION__); } #endif + return 0; #endif }
diff --git a/drivers/usb/musb-new/sunxi.c b/drivers/usb/musb-new/sunxi.c index 052e065..c123d61 100644 --- a/drivers/usb/musb-new/sunxi.c +++ b/drivers/usb/musb-new/sunxi.c @@ -199,12 +199,12 @@ static irqreturn_t sunxi_musb_interrupt(int irq, void *__hci) /* musb_core does not call enable / disable in a balanced manner <sigh> */ static bool enabled = false;
-static void sunxi_musb_enable(struct musb *musb) +static int sunxi_musb_enable(struct musb *musb) { pr_debug("%s():\n", __func__);
if (enabled) - return; + return 0;
/* select PIO mode */ musb_writeb(musb->mregs, USBC_REG_o_VEND0, 0); @@ -215,6 +215,7 @@ static void sunxi_musb_enable(struct musb *musb) USBC_ForceVbusValidToHigh(musb->mregs);
enabled = true; + return 0; }
static void sunxi_musb_disable(struct musb *musb)
On 17 June 2015 at 13:33, Hans de Goede hdegoede@redhat.com wrote:
Allow musb_platform_enable to return an error code and propagate it up to usb_lowlevel_init().
This allows moving the checks for an external vbus being present to be moved from platform_init to platform_enable, so that the user can unplug a charger, plug in a host adapter with a usb-device, do a "usb reset" and have things working.
This also allows adding a check for the id-pin to platform_enable, so that it can short circuit the 1s delay in usb_lowlevel_init() when no host cable is plugged in and thus waiting for a device to show up is useless.
Note that all the changes to code shared with the kernel are wrapped in the kernel.
Signed-off-by: Hans de Goede hdegoede@redhat.com
drivers/usb/musb-new/am35x.c | 7 +++++++ drivers/usb/musb-new/musb_core.c | 20 ++++++++++++++++++++ drivers/usb/musb-new/musb_core.h | 18 ++++++++++++++++++ drivers/usb/musb-new/musb_dsps.c | 6 ++++++ drivers/usb/musb-new/musb_uboot.c | 6 +++++- drivers/usb/musb-new/omap2430.c | 5 +++++ drivers/usb/musb-new/sunxi.c | 5 +++-- 7 files changed, 64 insertions(+), 3 deletions(-)
Acked-by: Simon Glass sjg@chromium.org
On 28 June 2015 at 21:45, Simon Glass sjg@chromium.org wrote:
On 17 June 2015 at 13:33, Hans de Goede hdegoede@redhat.com wrote:
Allow musb_platform_enable to return an error code and propagate it up to usb_lowlevel_init().
This allows moving the checks for an external vbus being present to be moved from platform_init to platform_enable, so that the user can unplug a charger, plug in a host adapter with a usb-device, do a "usb reset" and have things working.
This also allows adding a check for the id-pin to platform_enable, so that it can short circuit the 1s delay in usb_lowlevel_init() when no host cable is plugged in and thus waiting for a device to show up is useless.
Note that all the changes to code shared with the kernel are wrapped in the kernel.
Signed-off-by: Hans de Goede hdegoede@redhat.com
drivers/usb/musb-new/am35x.c | 7 +++++++ drivers/usb/musb-new/musb_core.c | 20 ++++++++++++++++++++ drivers/usb/musb-new/musb_core.h | 18 ++++++++++++++++++ drivers/usb/musb-new/musb_dsps.c | 6 ++++++ drivers/usb/musb-new/musb_uboot.c | 6 +++++- drivers/usb/musb-new/omap2430.c | 5 +++++ drivers/usb/musb-new/sunxi.c | 5 +++-- 7 files changed, 64 insertions(+), 3 deletions(-)
Acked-by: Simon Glass sjg@chromium.org
Applied to u-boot-dm/next, thanks!
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(-)
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. + * + * 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__ */
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
Hi,
On 29-06-15 05:45, Simon Glass wrote:
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().
Right, I think that may end up being cleaner in the long term.
Regards,
Hans
On 1 July 2015 at 08:57, Hans de Goede hdegoede@redhat.com wrote:
Hi,
On 29-06-15 05:45, Simon Glass wrote:
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->devpoints
* 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 parentof 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->devpoints
* 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().
Right, I think that may end up being cleaner in the long term.
Regards,
Hans
Applied to u-boot-dm/next, thanks!
Rename and wrap the usb host API public functions, this is a preparation patch for adding device-model support.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- drivers/usb/musb-new/musb_uboot.c | 70 +++++++++++++++++++++++++++++++++------ 1 file changed, 59 insertions(+), 11 deletions(-)
diff --git a/drivers/usb/musb-new/musb_uboot.c b/drivers/usb/musb-new/musb_uboot.c index a96e8d2..10c7cc4 100644 --- a/drivers/usb/musb-new/musb_uboot.c +++ b/drivers/usb/musb-new/musb_uboot.c @@ -90,7 +90,7 @@ static int submit_urb(struct usb_hcd *hcd, struct urb *urb) return urb->status; }
-int submit_control_msg(struct usb_device *dev, unsigned long pipe, +static int _musb_submit_control_msg(struct usb_device *dev, unsigned long pipe, void *buffer, int len, struct devrequest *setup) { construct_urb(&urb, &hep, dev, USB_ENDPOINT_XFER_CONTROL, pipe, @@ -103,8 +103,7 @@ int submit_control_msg(struct usb_device *dev, unsigned long pipe, return submit_urb(&hcd, &urb); }
- -int submit_bulk_msg(struct usb_device *dev, unsigned long pipe, +static int _musb_submit_bulk_msg(struct usb_device *dev, unsigned long pipe, void *buffer, int len) { construct_urb(&urb, &hep, dev, USB_ENDPOINT_XFER_BULK, pipe, @@ -112,7 +111,7 @@ int submit_bulk_msg(struct usb_device *dev, unsigned long pipe, return submit_urb(&hcd, &urb); }
-int submit_int_msg(struct usb_device *dev, unsigned long pipe, +static int _musb_submit_int_msg(struct usb_device *dev, unsigned long pipe, void *buffer, int len, int interval) { construct_urb(&urb, &hep, dev, USB_ENDPOINT_XFER_INT, pipe, @@ -120,8 +119,9 @@ int submit_int_msg(struct usb_device *dev, unsigned long pipe, return submit_urb(&hcd, &urb); }
-struct int_queue *create_int_queue(struct usb_device *dev, unsigned long pipe, - int queuesize, int elementsize, void *buffer, int interval) +static struct int_queue *_musb_create_int_queue(struct usb_device *dev, + unsigned long pipe, int queuesize, int elementsize, + void *buffer, int interval) { struct int_queue *queue; int ret, index = usb_pipein(pipe) * 16 + usb_pipeendpoint(pipe); @@ -154,7 +154,8 @@ struct int_queue *create_int_queue(struct usb_device *dev, unsigned long pipe, return queue; }
-int destroy_int_queue(struct usb_device *dev, struct int_queue *queue) +static int _musb_destroy_int_queue(struct usb_device *dev, + struct int_queue *queue) { int index = usb_pipein(queue->urb.pipe) * 16 + usb_pipeendpoint(queue->urb.pipe); @@ -167,7 +168,8 @@ int destroy_int_queue(struct usb_device *dev, struct int_queue *queue) return 0; }
-void *poll_int_queue(struct usb_device *dev, struct int_queue *queue) +static void *_musb_poll_int_queue(struct usb_device *dev, + struct int_queue *queue) { if (queue->urb.status != -EINPROGRESS) return NULL; /* URB has already completed in a prev. poll */ @@ -180,7 +182,7 @@ void *poll_int_queue(struct usb_device *dev, struct int_queue *queue) return NULL; /* URB still pending */ }
-int usb_reset_root_port(struct usb_device *dev) +static int _musb_reset_root_port(struct usb_device *dev) { void *mbase = host->mregs; u8 power; @@ -212,7 +214,7 @@ int usb_reset_root_port(struct usb_device *dev) return 0; }
-int usb_lowlevel_init(int index, enum usb_init_type init, void **controller) +int musb_lowlevel_init(void) { void *mbase; /* USB spec says it may take up to 1 second for a device to connect */ @@ -236,7 +238,7 @@ int usb_lowlevel_init(int index, enum usb_init_type init, void **controller) if (get_timer(0) >= timeout) return -ENODEV;
- usb_reset_root_port(NULL); + _musb_reset_root_port(NULL); host->is_active = 1; hcd.hcd_priv = host;
@@ -253,6 +255,52 @@ int usb_lowlevel_stop(int index) musb_stop(host); return 0; } + +int submit_bulk_msg(struct usb_device *dev, unsigned long pipe, + void *buffer, int length) +{ + return _musb_submit_bulk_msg(dev, pipe, buffer, length); +} + +int submit_control_msg(struct usb_device *dev, unsigned long pipe, + void *buffer, int length, struct devrequest *setup) +{ + return _musb_submit_control_msg(dev, pipe, buffer, length, setup); +} + +int submit_int_msg(struct usb_device *dev, unsigned long pipe, + void *buffer, int length, int interval) +{ + return _musb_submit_int_msg(dev, pipe, buffer, length, interval); +} + +struct int_queue *create_int_queue(struct usb_device *dev, + unsigned long pipe, int queuesize, int elementsize, + void *buffer, int interval) +{ + return _musb_create_int_queue(dev, pipe, queuesize, elementsize, + buffer, interval); +} + +void *poll_int_queue(struct usb_device *dev, struct int_queue *queue) +{ + return _musb_poll_int_queue(dev, queue); +} + +int destroy_int_queue(struct usb_device *dev, struct int_queue *queue) +{ + return _musb_destroy_int_queue(dev, queue); +} + +int usb_reset_root_port(struct usb_device *dev) +{ + return _musb_reset_root_port(dev); +} + +int usb_lowlevel_init(int index, enum usb_init_type init, void **controller) +{ + return musb_lowlevel_init(); +} #endif /* CONFIG_MUSB_HOST */
#ifdef CONFIG_MUSB_GADGET
On 17 June 2015 at 13:33, Hans de Goede hdegoede@redhat.com wrote:
Rename and wrap the usb host API public functions, this is a preparation patch for adding device-model support.
Signed-off-by: Hans de Goede hdegoede@redhat.com
drivers/usb/musb-new/musb_uboot.c | 70 +++++++++++++++++++++++++++++++++------ 1 file changed, 59 insertions(+), 11 deletions(-)
Acked-by: Simon Glass sjg@chromium.org
On 28 June 2015 at 21:45, Simon Glass sjg@chromium.org wrote:
On 17 June 2015 at 13:33, Hans de Goede hdegoede@redhat.com wrote:
Rename and wrap the usb host API public functions, this is a preparation patch for adding device-model support.
Signed-off-by: Hans de Goede hdegoede@redhat.com
drivers/usb/musb-new/musb_uboot.c | 70 +++++++++++++++++++++++++++++++++------ 1 file changed, 59 insertions(+), 11 deletions(-)
Acked-by: Simon Glass sjg@chromium.org
Applied to u-boot-dm/next, thanks!
Add a musb_host_data struct to hold all the global data host related musb data. This is a preparation patch for adding device-model support.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- drivers/usb/musb-new/musb_uboot.c | 107 +++++++++++++++++++------------------- drivers/usb/musb-new/musb_uboot.h | 24 +++++++++ 2 files changed, 77 insertions(+), 54 deletions(-) create mode 100644 drivers/usb/musb-new/musb_uboot.h
diff --git a/drivers/usb/musb-new/musb_uboot.c b/drivers/usb/musb-new/musb_uboot.c index 10c7cc4..9875100 100644 --- a/drivers/usb/musb-new/musb_uboot.c +++ b/drivers/usb/musb-new/musb_uboot.c @@ -13,6 +13,7 @@ #include "musb_core.h" #include "musb_host.h" #include "musb_gadget.h" +#include "musb_uboot.h"
#ifdef CONFIG_MUSB_HOST struct int_queue { @@ -20,9 +21,7 @@ struct int_queue { struct urb urb; };
-static struct musb *host; -static struct usb_hcd hcd; -static enum usb_device_speed host_speed; +struct musb_host_data musb_host;
static void musb_host_complete_urb(struct urb *urb) { @@ -30,9 +29,6 @@ static void musb_host_complete_urb(struct urb *urb) urb->dev->act_len = urb->actual_length; }
-static struct usb_host_endpoint hep; -static struct urb urb; - static void construct_urb(struct urb *urb, struct usb_host_endpoint *hep, struct usb_device *dev, int endpoint_type, unsigned long pipe, void *buffer, int len, @@ -90,38 +86,40 @@ static int submit_urb(struct usb_hcd *hcd, struct urb *urb) return urb->status; }
-static int _musb_submit_control_msg(struct usb_device *dev, unsigned long pipe, - void *buffer, int len, struct devrequest *setup) +static int _musb_submit_control_msg(struct musb_host_data *host, + struct usb_device *dev, unsigned long pipe, + void *buffer, int len, struct devrequest *setup) { - construct_urb(&urb, &hep, dev, USB_ENDPOINT_XFER_CONTROL, pipe, - buffer, len, setup, 0); + construct_urb(&host->urb, &host->hep, dev, USB_ENDPOINT_XFER_CONTROL, + pipe, buffer, len, setup, 0);
/* Fix speed for non hub-attached devices */ if (!usb_dev_get_parent(dev)) - dev->speed = host_speed; + dev->speed = host->host_speed;
- return submit_urb(&hcd, &urb); + return submit_urb(&host->hcd, &host->urb); }
-static int _musb_submit_bulk_msg(struct usb_device *dev, unsigned long pipe, - void *buffer, int len) +static int _musb_submit_bulk_msg(struct musb_host_data *host, + struct usb_device *dev, unsigned long pipe, void *buffer, int len) { - construct_urb(&urb, &hep, dev, USB_ENDPOINT_XFER_BULK, pipe, - buffer, len, NULL, 0); - return submit_urb(&hcd, &urb); + construct_urb(&host->urb, &host->hep, dev, USB_ENDPOINT_XFER_BULK, + pipe, buffer, len, NULL, 0); + return submit_urb(&host->hcd, &host->urb); }
-static int _musb_submit_int_msg(struct usb_device *dev, unsigned long pipe, - void *buffer, int len, int interval) +static int _musb_submit_int_msg(struct musb_host_data *host, + struct usb_device *dev, unsigned long pipe, + void *buffer, int len, int interval) { - construct_urb(&urb, &hep, dev, USB_ENDPOINT_XFER_INT, pipe, + construct_urb(&host->urb, &host->hep, dev, USB_ENDPOINT_XFER_INT, pipe, buffer, len, NULL, interval); - return submit_urb(&hcd, &urb); + return submit_urb(&host->hcd, &host->urb); }
-static struct int_queue *_musb_create_int_queue(struct usb_device *dev, - unsigned long pipe, int queuesize, int elementsize, - void *buffer, int interval) +static struct int_queue *_musb_create_int_queue(struct musb_host_data *host, + struct usb_device *dev, unsigned long pipe, int queuesize, + int elementsize, void *buffer, int interval) { struct int_queue *queue; int ret, index = usb_pipein(pipe) * 16 + usb_pipeendpoint(pipe); @@ -143,7 +141,7 @@ static struct int_queue *_musb_create_int_queue(struct usb_device *dev, construct_urb(&queue->urb, &queue->hep, dev, USB_ENDPOINT_XFER_INT, pipe, buffer, elementsize, NULL, interval);
- ret = musb_urb_enqueue(&hcd, &queue->urb, 0); + ret = musb_urb_enqueue(&host->hcd, &queue->urb, 0); if (ret < 0) { printf("Failed to enqueue URB to controller\n"); free(queue); @@ -154,27 +152,27 @@ static struct int_queue *_musb_create_int_queue(struct usb_device *dev, return queue; }
-static int _musb_destroy_int_queue(struct usb_device *dev, - struct int_queue *queue) +static int _musb_destroy_int_queue(struct musb_host_data *host, + struct usb_device *dev, struct int_queue *queue) { int index = usb_pipein(queue->urb.pipe) * 16 + usb_pipeendpoint(queue->urb.pipe);
if (queue->urb.status == -EINPROGRESS) - musb_urb_dequeue(&hcd, &queue->urb, -ETIME); + musb_urb_dequeue(&host->hcd, &queue->urb, -ETIME);
dev->int_pending &= ~(1 << index); free(queue); return 0; }
-static void *_musb_poll_int_queue(struct usb_device *dev, - struct int_queue *queue) +static void *_musb_poll_int_queue(struct musb_host_data *host, + struct usb_device *dev, struct int_queue *queue) { if (queue->urb.status != -EINPROGRESS) return NULL; /* URB has already completed in a prev. poll */
- host->isr(0, host); + host->host->isr(0, host->host);
if (queue->urb.status != -EINPROGRESS) return queue->urb.transfer_buffer; /* Done */ @@ -182,9 +180,10 @@ static void *_musb_poll_int_queue(struct usb_device *dev, return NULL; /* URB still pending */ }
-static int _musb_reset_root_port(struct usb_device *dev) +static int _musb_reset_root_port(struct musb_host_data *host, + struct usb_device *dev) { - void *mbase = host->mregs; + void *mbase = host->host->mregs; u8 power;
power = musb_readb(mbase, MUSB_POWER); @@ -204,33 +203,33 @@ static int _musb_reset_root_port(struct usb_device *dev) #ifdef CONFIG_ARCH_SUNXI sunxi_usb_phy_enable_squelch_detect(0, 1); #endif - host->isr(0, host); - host_speed = (musb_readb(mbase, MUSB_POWER) & MUSB_POWER_HSMODE) ? + host->host->isr(0, host->host); + host->host_speed = (musb_readb(mbase, MUSB_POWER) & MUSB_POWER_HSMODE) ? USB_SPEED_HIGH : (musb_readb(mbase, MUSB_DEVCTL) & MUSB_DEVCTL_FSDEV) ? USB_SPEED_FULL : USB_SPEED_LOW; - mdelay((host_speed == USB_SPEED_LOW) ? 200 : 50); + mdelay((host->host_speed == USB_SPEED_LOW) ? 200 : 50);
return 0; }
-int musb_lowlevel_init(void) +int musb_lowlevel_init(struct musb_host_data *host) { void *mbase; /* USB spec says it may take up to 1 second for a device to connect */ unsigned long timeout = get_timer(0) + 1000; int ret;
- if (!host) { + if (!host->host) { printf("MUSB host is not registered\n"); return -ENODEV; }
- ret = musb_start(host); + ret = musb_start(host->host); if (ret) return ret;
- mbase = host->mregs; + mbase = host->host->mregs; do { if (musb_readb(mbase, MUSB_DEVCTL) & MUSB_DEVCTL_HM) break; @@ -238,68 +237,68 @@ int musb_lowlevel_init(void) if (get_timer(0) >= timeout) return -ENODEV;
- _musb_reset_root_port(NULL); - host->is_active = 1; - hcd.hcd_priv = host; + _musb_reset_root_port(host, NULL); + host->host->is_active = 1; + host->hcd.hcd_priv = host->host;
return 0; }
int usb_lowlevel_stop(int index) { - if (!host) { + if (!musb_host.host) { printf("MUSB host is not registered\n"); return -ENODEV; }
- musb_stop(host); + musb_stop(musb_host.host); return 0; }
int submit_bulk_msg(struct usb_device *dev, unsigned long pipe, void *buffer, int length) { - return _musb_submit_bulk_msg(dev, pipe, buffer, length); + return _musb_submit_bulk_msg(&musb_host, dev, pipe, buffer, length); }
int submit_control_msg(struct usb_device *dev, unsigned long pipe, void *buffer, int length, struct devrequest *setup) { - return _musb_submit_control_msg(dev, pipe, buffer, length, setup); + return _musb_submit_control_msg(&musb_host, dev, pipe, buffer, length, setup); }
int submit_int_msg(struct usb_device *dev, unsigned long pipe, void *buffer, int length, int interval) { - return _musb_submit_int_msg(dev, pipe, buffer, length, interval); + return _musb_submit_int_msg(&musb_host, dev, pipe, buffer, length, interval); }
struct int_queue *create_int_queue(struct usb_device *dev, unsigned long pipe, int queuesize, int elementsize, void *buffer, int interval) { - return _musb_create_int_queue(dev, pipe, queuesize, elementsize, + return _musb_create_int_queue(&musb_host, dev, pipe, queuesize, elementsize, buffer, interval); }
void *poll_int_queue(struct usb_device *dev, struct int_queue *queue) { - return _musb_poll_int_queue(dev, queue); + return _musb_poll_int_queue(&musb_host, dev, queue); }
int destroy_int_queue(struct usb_device *dev, struct int_queue *queue) { - return _musb_destroy_int_queue(dev, queue); + return _musb_destroy_int_queue(&musb_host, dev, queue); }
int usb_reset_root_port(struct usb_device *dev) { - return _musb_reset_root_port(dev); + return _musb_reset_root_port(&musb_host, dev); }
int usb_lowlevel_init(int index, enum usb_init_type init, void **controller) { - return musb_lowlevel_init(); + return musb_lowlevel_init(&musb_host); } #endif /* CONFIG_MUSB_HOST */
@@ -363,7 +362,7 @@ int musb_register(struct musb_hdrc_platform_data *plat, void *bdata, switch (plat->mode) { #ifdef CONFIG_MUSB_HOST case MUSB_HOST: - musbp = &host; + musbp = &musb_host.host; break; #endif #ifdef CONFIG_MUSB_GADGET diff --git a/drivers/usb/musb-new/musb_uboot.h b/drivers/usb/musb-new/musb_uboot.h new file mode 100644 index 0000000..69b7977 --- /dev/null +++ b/drivers/usb/musb-new/musb_uboot.h @@ -0,0 +1,24 @@ +/* + * MUSB OTG driver u-boot specific functions + * + * Copyright © 2015 Hans de Goede hdegoede@redhat.com + * + * SPDX-License-Identifier: GPL-2.0+ + */ +#ifndef __MUSB_UBOOT_H__ +#define __MUSB_UBOOT_H__ + +#include <usb.h> +#include "linux-compat.h" +#include "usb-compat.h" +#include "musb_core.h" + +struct musb_host_data { + struct musb *host; + struct usb_hcd hcd; + enum usb_device_speed host_speed; + struct usb_host_endpoint hep; + struct urb urb; +}; + +#endif
Hi Hans,
On 17 June 2015 at 13:33, Hans de Goede hdegoede@redhat.com wrote:
Add a musb_host_data struct to hold all the global data host related musb data. This is a preparation patch for adding device-model support.
Signed-off-by: Hans de Goede hdegoede@redhat.com
drivers/usb/musb-new/musb_uboot.c | 107 +++++++++++++++++++------------------- drivers/usb/musb-new/musb_uboot.h | 24 +++++++++ 2 files changed, 77 insertions(+), 54 deletions(-) create mode 100644 drivers/usb/musb-new/musb_uboot.h
Acked-by: Simon Glass sjg@chromium.org
Regards, Simon
On 28 June 2015 at 21:45, Simon Glass sjg@chromium.org wrote:
Hi Hans,
On 17 June 2015 at 13:33, Hans de Goede hdegoede@redhat.com wrote:
Add a musb_host_data struct to hold all the global data host related musb data. This is a preparation patch for adding device-model support.
Signed-off-by: Hans de Goede hdegoede@redhat.com
drivers/usb/musb-new/musb_uboot.c | 107 +++++++++++++++++++------------------- drivers/usb/musb-new/musb_uboot.h | 24 +++++++++ 2 files changed, 77 insertions(+), 54 deletions(-) create mode 100644 drivers/usb/musb-new/musb_uboot.h
Acked-by: Simon Glass sjg@chromium.org
Regards, Simon
Applied to u-boot-dm/next, thanks!
Add device-model support to the musb-host u-boot glue, note this only adds device-model support to the musb-core glue code, it does not add support for device-model to any of the SoC specific musb glue code.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- drivers/usb/musb-new/musb_uboot.c | 70 ++++++++++++++++++++++++++++++++++++++- drivers/usb/musb-new/musb_uboot.h | 4 +++ 2 files changed, 73 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/musb-new/musb_uboot.c b/drivers/usb/musb-new/musb_uboot.c index 9875100..9b56e90 100644 --- a/drivers/usb/musb-new/musb_uboot.c +++ b/drivers/usb/musb-new/musb_uboot.c @@ -21,7 +21,9 @@ struct int_queue { struct urb urb; };
+#ifndef CONFIG_DM_USB struct musb_host_data musb_host; +#endif
static void musb_host_complete_urb(struct urb *urb) { @@ -244,6 +246,7 @@ int musb_lowlevel_init(struct musb_host_data *host) return 0; }
+#ifndef CONFIG_DM_USB int usb_lowlevel_stop(int index) { if (!musb_host.host) { @@ -300,6 +303,71 @@ int usb_lowlevel_init(int index, enum usb_init_type init, void **controller) { return musb_lowlevel_init(&musb_host); } +#endif /* !CONFIG_DM_USB */ + +#ifdef CONFIG_DM_USB +static int musb_submit_control_msg(struct udevice *dev, struct usb_device *udev, + unsigned long pipe, void *buffer, int length, + struct devrequest *setup) +{ + struct musb_host_data *host = dev_get_priv(dev); + return _musb_submit_control_msg(host, udev, pipe, buffer, length, setup); +} + +static int musb_submit_bulk_msg(struct udevice *dev, struct usb_device *udev, + unsigned long pipe, void *buffer, int length) +{ + struct musb_host_data *host = dev_get_priv(dev); + return _musb_submit_bulk_msg(host, udev, pipe, buffer, length); +} + +static int musb_submit_int_msg(struct udevice *dev, struct usb_device *udev, + unsigned long pipe, void *buffer, int length, + int interval) +{ + struct musb_host_data *host = dev_get_priv(dev); + return _musb_submit_int_msg(host, udev, pipe, buffer, length, interval); +} + +static struct int_queue *musb_create_int_queue(struct udevice *dev, + struct usb_device *udev, unsigned long pipe, int queuesize, + int elementsize, void *buffer, int interval) +{ + struct musb_host_data *host = dev_get_priv(dev); + return _musb_create_int_queue(host, udev, pipe, queuesize, elementsize, + buffer, interval); +} + +static void *musb_poll_int_queue(struct udevice *dev, struct usb_device *udev, + struct int_queue *queue) +{ + struct musb_host_data *host = dev_get_priv(dev); + return _musb_poll_int_queue(host, udev, queue); +} + +static int musb_destroy_int_queue(struct udevice *dev, struct usb_device *udev, + struct int_queue *queue) +{ + struct musb_host_data *host = dev_get_priv(dev); + return _musb_destroy_int_queue(host, udev, queue); +} + +static int musb_reset_root_port(struct udevice *dev, struct usb_device *udev) +{ + struct musb_host_data *host = dev_get_priv(dev); + return _musb_reset_root_port(host, udev); +} + +struct dm_usb_ops musb_usb_ops = { + .control = musb_submit_control_msg, + .bulk = musb_submit_bulk_msg, + .interrupt = musb_submit_int_msg, + .create_int_queue = musb_create_int_queue, + .poll_int_queue = musb_poll_int_queue, + .destroy_int_queue = musb_destroy_int_queue, + .reset_root_port = musb_reset_root_port, +}; +#endif /* CONFIG_DM_USB */ #endif /* CONFIG_MUSB_HOST */
#ifdef CONFIG_MUSB_GADGET @@ -360,7 +428,7 @@ int musb_register(struct musb_hdrc_platform_data *plat, void *bdata, struct musb **musbp;
switch (plat->mode) { -#ifdef CONFIG_MUSB_HOST +#if defined(CONFIG_MUSB_HOST) && !defined(CONFIG_DM_USB) case MUSB_HOST: musbp = &musb_host.host; break; diff --git a/drivers/usb/musb-new/musb_uboot.h b/drivers/usb/musb-new/musb_uboot.h index 69b7977..6312cd2 100644 --- a/drivers/usb/musb-new/musb_uboot.h +++ b/drivers/usb/musb-new/musb_uboot.h @@ -21,4 +21,8 @@ struct musb_host_data { struct urb urb; };
+extern struct dm_usb_ops musb_usb_ops; + +int musb_lowlevel_init(struct musb_host_data *host); + #endif
Hi Hans,
On 17 June 2015 at 13:33, Hans de Goede hdegoede@redhat.com wrote:
Add device-model support to the musb-host u-boot glue, note this only adds device-model support to the musb-core glue code, it does not add support for device-model to any of the SoC specific musb glue code.
Signed-off-by: Hans de Goede hdegoede@redhat.com
drivers/usb/musb-new/musb_uboot.c | 70 ++++++++++++++++++++++++++++++++++++++- drivers/usb/musb-new/musb_uboot.h | 4 +++ 2 files changed, 73 insertions(+), 1 deletion(-)
Acked-by: Simon Glass sjg@chromium.org
Nit below.
diff --git a/drivers/usb/musb-new/musb_uboot.c b/drivers/usb/musb-new/musb_uboot.c index 9875100..9b56e90 100644 --- a/drivers/usb/musb-new/musb_uboot.c +++ b/drivers/usb/musb-new/musb_uboot.c @@ -21,7 +21,9 @@ struct int_queue { struct urb urb; };
+#ifndef CONFIG_DM_USB struct musb_host_data musb_host; +#endif
static void musb_host_complete_urb(struct urb *urb) { @@ -244,6 +246,7 @@ int musb_lowlevel_init(struct musb_host_data *host) return 0; }
+#ifndef CONFIG_DM_USB int usb_lowlevel_stop(int index) { if (!musb_host.host) { @@ -300,6 +303,71 @@ int usb_lowlevel_init(int index, enum usb_init_type init, void **controller) { return musb_lowlevel_init(&musb_host); } +#endif /* !CONFIG_DM_USB */
+#ifdef CONFIG_DM_USB +static int musb_submit_control_msg(struct udevice *dev, struct usb_device *udev,
unsigned long pipe, void *buffer, int length,struct devrequest *setup)+{
struct musb_host_data *host = dev_get_priv(dev);return _musb_submit_control_msg(host, udev, pipe, buffer, length, setup);+}
+static int musb_submit_bulk_msg(struct udevice *dev, struct usb_device *udev,
unsigned long pipe, void *buffer, int length)+{
struct musb_host_data *host = dev_get_priv(dev);return _musb_submit_bulk_msg(host, udev, pipe, buffer, length);+}
+static int musb_submit_int_msg(struct udevice *dev, struct usb_device *udev,
unsigned long pipe, void *buffer, int length,int interval)+{
struct musb_host_data *host = dev_get_priv(dev);return _musb_submit_int_msg(host, udev, pipe, buffer, length, interval);+}
+static struct int_queue *musb_create_int_queue(struct udevice *dev,
struct usb_device *udev, unsigned long pipe, int queuesize,int elementsize, void *buffer, int interval)+{
struct musb_host_data *host = dev_get_priv(dev);
Can we have newlines after declarations?
return _musb_create_int_queue(host, udev, pipe, queuesize, elementsize,buffer, interval);+}
+static void *musb_poll_int_queue(struct udevice *dev, struct usb_device *udev,
struct int_queue *queue)+{
struct musb_host_data *host = dev_get_priv(dev);return _musb_poll_int_queue(host, udev, queue);+}
+static int musb_destroy_int_queue(struct udevice *dev, struct usb_device *udev,
struct int_queue *queue)+{
struct musb_host_data *host = dev_get_priv(dev);return _musb_destroy_int_queue(host, udev, queue);+}
+static int musb_reset_root_port(struct udevice *dev, struct usb_device *udev) +{
struct musb_host_data *host = dev_get_priv(dev);return _musb_reset_root_port(host, udev);+}
+struct dm_usb_ops musb_usb_ops = {
.control = musb_submit_control_msg,.bulk = musb_submit_bulk_msg,.interrupt = musb_submit_int_msg,.create_int_queue = musb_create_int_queue,.poll_int_queue = musb_poll_int_queue,.destroy_int_queue = musb_destroy_int_queue,.reset_root_port = musb_reset_root_port,+}; +#endif /* CONFIG_DM_USB */ #endif /* CONFIG_MUSB_HOST */
#ifdef CONFIG_MUSB_GADGET @@ -360,7 +428,7 @@ int musb_register(struct musb_hdrc_platform_data *plat, void *bdata, struct musb **musbp;
switch (plat->mode) {-#ifdef CONFIG_MUSB_HOST +#if defined(CONFIG_MUSB_HOST) && !defined(CONFIG_DM_USB) case MUSB_HOST: musbp = &musb_host.host; break; diff --git a/drivers/usb/musb-new/musb_uboot.h b/drivers/usb/musb-new/musb_uboot.h index 69b7977..6312cd2 100644 --- a/drivers/usb/musb-new/musb_uboot.h +++ b/drivers/usb/musb-new/musb_uboot.h @@ -21,4 +21,8 @@ struct musb_host_data { struct urb urb; };
+extern struct dm_usb_ops musb_usb_ops;
+int musb_lowlevel_init(struct musb_host_data *host);
#endif
2.4.3
Regards, Simon
On 28 June 2015 at 21:45, Simon Glass sjg@chromium.org wrote:
Hi Hans,
On 17 June 2015 at 13:33, Hans de Goede hdegoede@redhat.com wrote:
Add device-model support to the musb-host u-boot glue, note this only adds device-model support to the musb-core glue code, it does not add support for device-model to any of the SoC specific musb glue code.
Signed-off-by: Hans de Goede hdegoede@redhat.com
drivers/usb/musb-new/musb_uboot.c | 70 ++++++++++++++++++++++++++++++++++++++- drivers/usb/musb-new/musb_uboot.h | 4 +++ 2 files changed, 73 insertions(+), 1 deletion(-)
Acked-by: Simon Glass sjg@chromium.org
Nit below.
diff --git a/drivers/usb/musb-new/musb_uboot.c b/drivers/usb/musb-new/musb_uboot.c index 9875100..9b56e90 100644 --- a/drivers/usb/musb-new/musb_uboot.c +++ b/drivers/usb/musb-new/musb_uboot.c @@ -21,7 +21,9 @@ struct int_queue { struct urb urb; };
+#ifndef CONFIG_DM_USB struct musb_host_data musb_host; +#endif
static void musb_host_complete_urb(struct urb *urb) { @@ -244,6 +246,7 @@ int musb_lowlevel_init(struct musb_host_data *host) return 0; }
+#ifndef CONFIG_DM_USB int usb_lowlevel_stop(int index) { if (!musb_host.host) { @@ -300,6 +303,71 @@ int usb_lowlevel_init(int index, enum usb_init_type init, void **controller) { return musb_lowlevel_init(&musb_host); } +#endif /* !CONFIG_DM_USB */
+#ifdef CONFIG_DM_USB +static int musb_submit_control_msg(struct udevice *dev, struct usb_device *udev,
unsigned long pipe, void *buffer, int length,struct devrequest *setup)+{
struct musb_host_data *host = dev_get_priv(dev);return _musb_submit_control_msg(host, udev, pipe, buffer, length, setup);+}
+static int musb_submit_bulk_msg(struct udevice *dev, struct usb_device *udev,
unsigned long pipe, void *buffer, int length)+{
struct musb_host_data *host = dev_get_priv(dev);return _musb_submit_bulk_msg(host, udev, pipe, buffer, length);+}
+static int musb_submit_int_msg(struct udevice *dev, struct usb_device *udev,
unsigned long pipe, void *buffer, int length,int interval)+{
struct musb_host_data *host = dev_get_priv(dev);return _musb_submit_int_msg(host, udev, pipe, buffer, length, interval);+}
+static struct int_queue *musb_create_int_queue(struct udevice *dev,
struct usb_device *udev, unsigned long pipe, int queuesize,int elementsize, void *buffer, int interval)+{
struct musb_host_data *host = dev_get_priv(dev);Can we have newlines after declarations?
Added these, and applied to u-boot-dm/next, thanks!
return _musb_create_int_queue(host, udev, pipe, queuesize, elementsize,buffer, interval);+}
+static void *musb_poll_int_queue(struct udevice *dev, struct usb_device *udev,
struct int_queue *queue)+{
struct musb_host_data *host = dev_get_priv(dev);return _musb_poll_int_queue(host, udev, queue);+}
+static int musb_destroy_int_queue(struct udevice *dev, struct usb_device *udev,
struct int_queue *queue)+{
struct musb_host_data *host = dev_get_priv(dev);return _musb_destroy_int_queue(host, udev, queue);+}
+static int musb_reset_root_port(struct udevice *dev, struct usb_device *udev) +{
struct musb_host_data *host = dev_get_priv(dev);return _musb_reset_root_port(host, udev);+}
+struct dm_usb_ops musb_usb_ops = {
.control = musb_submit_control_msg,.bulk = musb_submit_bulk_msg,.interrupt = musb_submit_int_msg,.create_int_queue = musb_create_int_queue,.poll_int_queue = musb_poll_int_queue,.destroy_int_queue = musb_destroy_int_queue,.reset_root_port = musb_reset_root_port,+}; +#endif /* CONFIG_DM_USB */ #endif /* CONFIG_MUSB_HOST */
#ifdef CONFIG_MUSB_GADGET @@ -360,7 +428,7 @@ int musb_register(struct musb_hdrc_platform_data *plat, void *bdata, struct musb **musbp;
switch (plat->mode) {-#ifdef CONFIG_MUSB_HOST +#if defined(CONFIG_MUSB_HOST) && !defined(CONFIG_DM_USB) case MUSB_HOST: musbp = &musb_host.host; break; diff --git a/drivers/usb/musb-new/musb_uboot.h b/drivers/usb/musb-new/musb_uboot.h index 69b7977..6312cd2 100644 --- a/drivers/usb/musb-new/musb_uboot.h +++ b/drivers/usb/musb-new/musb_uboot.h @@ -21,4 +21,8 @@ struct musb_host_data { struct urb urb; };
+extern struct dm_usb_ops musb_usb_ops;
+int musb_lowlevel_init(struct musb_host_data *host);
#endif
2.4.3
Regards, Simon
Add support for reading the id pin value of the otg connector to the usb phy code.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- arch/arm/cpu/armv7/sunxi/usb_phy.c | 34 +++++++++++++++++++++++++++++++ arch/arm/include/asm/arch-sunxi/usb_phy.h | 1 + board/sunxi/Kconfig | 7 +++++++ 3 files changed, 42 insertions(+)
diff --git a/arch/arm/cpu/armv7/sunxi/usb_phy.c b/arch/arm/cpu/armv7/sunxi/usb_phy.c index b07d67f..5e82ddc 100644 --- a/arch/arm/cpu/armv7/sunxi/usb_phy.c +++ b/arch/arm/cpu/armv7/sunxi/usb_phy.c @@ -44,6 +44,7 @@ static struct sunxi_usb_phy { int usb_rst_mask; int gpio_vbus; int gpio_vbus_det; + int gpio_id_det; int id; int init_count; int power_on_count; @@ -82,6 +83,14 @@ static int get_vbus_detect_gpio(int index) return -EINVAL; }
+static int get_id_detect_gpio(int index) +{ + switch (index) { + case 0: return sunxi_name_to_gpio(CONFIG_USB0_ID_DET); + } + return -EINVAL; +} + static void usb_phy_write(struct sunxi_usb_phy *phy, int addr, int data, int len) { @@ -247,6 +256,16 @@ int sunxi_usb_phy_vbus_detect(int index) return err; }
+int sunxi_usb_phy_id_detect(int index) +{ + struct sunxi_usb_phy *phy = &sunxi_usb_phy[index]; + + if (phy->gpio_id_det < 0) + return phy->gpio_id_det; + + return gpio_get_value(phy->gpio_id_det); +} + int sunxi_usb_phy_probe(void) { struct sunxi_ccm_reg *ccm = (struct sunxi_ccm_reg *)SUNXI_CCM_BASE; @@ -275,6 +294,18 @@ int sunxi_usb_phy_probe(void) if (ret) return ret; } + + phy->gpio_id_det = get_id_detect_gpio(i); + if (phy->gpio_id_det >= 0) { + ret = gpio_request(phy->gpio_id_det, "usb_id_det"); + if (ret) + return ret; + ret = gpio_direction_input(phy->gpio_id_det); + if (ret) + return ret; + sunxi_gpio_set_pull(phy->gpio_id_det, + SUNXI_GPIO_PULL_UP); + } }
setbits_le32(&ccm->usb_clk_cfg, CCM_USB_CTRL_PHYGATE); @@ -298,6 +329,9 @@ int sunxi_usb_phy_remove(void)
if (phy->gpio_vbus_det >= 0) gpio_free(phy->gpio_vbus_det); + + if (phy->gpio_id_det >= 0) + gpio_free(phy->gpio_id_det); }
return 0; diff --git a/arch/arm/include/asm/arch-sunxi/usb_phy.h b/arch/arm/include/asm/arch-sunxi/usb_phy.h index b7b831e..5a9cacb 100644 --- a/arch/arm/include/asm/arch-sunxi/usb_phy.h +++ b/arch/arm/include/asm/arch-sunxi/usb_phy.h @@ -17,4 +17,5 @@ void sunxi_usb_phy_exit(int index); void sunxi_usb_phy_power_on(int index); void sunxi_usb_phy_power_off(int index); int sunxi_usb_phy_vbus_detect(int index); +int sunxi_usb_phy_id_detect(int index); void sunxi_usb_phy_enable_squelch_detect(int index, int enable); diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig index b2eca51..4311c3e 100644 --- a/board/sunxi/Kconfig +++ b/board/sunxi/Kconfig @@ -306,6 +306,13 @@ config USB0_VBUS_DET Set the Vbus detect pin for usb0 (otg). This takes a string in the format understood by sunxi_name_to_gpio, e.g. PH1 for pin 1 of port H.
+config USB0_ID_DET + string "ID detect pin for usb0 (otg)" + default "" + ---help--- + Set the ID detect pin for usb0 (otg). This takes a string in the + format understood by sunxi_name_to_gpio, e.g. PH1 for pin 1 of port H. + config USB1_VBUS_PIN string "Vbus enable pin for usb1 (ehci0)" default "PH6" if MACH_SUN4I || MACH_SUN7I
On Wed, 2015-06-17 at 21:33 +0200, Hans de Goede wrote:
Add support for reading the id pin value of the otg connector to the usb phy code.
Signed-off-by: Hans de Goede hdegoede@redhat.com
Acked-by: Ian Campbell ijc@hellion.org.uk
This way it can be re-checked on "usb reset".
Signed-off-by: Hans de Goede hdegoede@redhat.com --- drivers/usb/musb-new/sunxi.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/drivers/usb/musb-new/sunxi.c b/drivers/usb/musb-new/sunxi.c index c123d61..ee018c7 100644 --- a/drivers/usb/musb-new/sunxi.c +++ b/drivers/usb/musb-new/sunxi.c @@ -201,6 +201,8 @@ static bool enabled = false;
static int sunxi_musb_enable(struct musb *musb) { + int ret; + pr_debug("%s():\n", __func__);
if (enabled) @@ -209,8 +211,14 @@ static int sunxi_musb_enable(struct musb *musb) /* select PIO mode */ musb_writeb(musb->mregs, USBC_REG_o_VEND0, 0);
- if (is_host_enabled(musb)) + if (is_host_enabled(musb)) { + ret = sunxi_usb_phy_vbus_detect(0); + if (ret) { + printf("A charger is plugged into the OTG: "); + return -ENODEV; + } sunxi_usb_phy_power_on(0); /* port power on */ + }
USBC_ForceVbusValidToHigh(musb->mregs);
@@ -237,18 +245,9 @@ static void sunxi_musb_disable(struct musb *musb) static int sunxi_musb_init(struct musb *musb) { struct sunxi_ccm_reg *ccm = (struct sunxi_ccm_reg *)SUNXI_CCM_BASE; - int err;
pr_debug("%s():\n", __func__);
- if (is_host_enabled(musb)) { - err = sunxi_usb_phy_vbus_detect(0); - if (err) { - eprintf("Error: A charger is plugged into the OTG\n"); - return -EIO; - } - } - musb->isr = sunxi_musb_interrupt;
setbits_le32(&ccm->ahb_gate0, 1 << AHB_GATE_OFFSET_USB0);
On Wed, 2015-06-17 at 21:34 +0200, Hans de Goede wrote:
This way it can be re-checked on "usb reset".
Signed-off-by: Hans de Goede hdegoede@redhat.com
Acked-by: Ian Campbell ijc@hellion.org.uk
When in host mode check if there is a host cable inserted into the otg port by checking the id pin. If there is no host cable return an error to make usb_lowlevel_init() exit early, rather then waiting for 1 second for a device which will never show up.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- configs/Ippo_q8h_v1_2_a33_1024x600_defconfig | 1 + configs/Ippo_q8h_v1_2_defconfig | 1 + configs/Ippo_q8h_v5_defconfig | 1 + drivers/usb/musb-new/sunxi.c | 5 +++++ 4 files changed, 8 insertions(+)
diff --git a/configs/Ippo_q8h_v1_2_a33_1024x600_defconfig b/configs/Ippo_q8h_v1_2_a33_1024x600_defconfig index 5b1080f..51d75c9 100644 --- a/configs/Ippo_q8h_v1_2_a33_1024x600_defconfig +++ b/configs/Ippo_q8h_v1_2_a33_1024x600_defconfig @@ -5,6 +5,7 @@ CONFIG_DRAM_CLK=480 CONFIG_DRAM_ZQ=15291 CONFIG_USB0_VBUS_PIN="AXP0-VBUS-ENABLE" CONFIG_USB0_VBUS_DET="AXP0-VBUS-DETECT" +CONFIG_USB0_ID_DET="PH8" CONFIG_AXP_GPIO=y CONFIG_VIDEO_LCD_MODE="x:1024,y:600,depth:18,pclk_khz:51000,le:159,ri:160,up:22,lo:12,hs:1,vs:1,sync:3,vmode:0" CONFIG_VIDEO_LCD_DCLK_PHASE=0 diff --git a/configs/Ippo_q8h_v1_2_defconfig b/configs/Ippo_q8h_v1_2_defconfig index 8d03300..20fc311 100644 --- a/configs/Ippo_q8h_v1_2_defconfig +++ b/configs/Ippo_q8h_v1_2_defconfig @@ -5,6 +5,7 @@ CONFIG_DRAM_CLK=432 CONFIG_DRAM_ZQ=63306 CONFIG_USB0_VBUS_PIN="AXP0-VBUS-ENABLE" CONFIG_USB0_VBUS_DET="AXP0-VBUS-DETECT" +CONFIG_USB0_ID_DET="PH8" CONFIG_AXP_GPIO=y CONFIG_VIDEO_LCD_MODE="x:800,y:480,depth:18,pclk_khz:33000,le:87,ri:167,up:31,lo:13,hs:1,vs:1,sync:3,vmode:0" CONFIG_VIDEO_LCD_DCLK_PHASE=0 diff --git a/configs/Ippo_q8h_v5_defconfig b/configs/Ippo_q8h_v5_defconfig index 1a07064..3264a20 100644 --- a/configs/Ippo_q8h_v5_defconfig +++ b/configs/Ippo_q8h_v5_defconfig @@ -5,6 +5,7 @@ CONFIG_DRAM_CLK=480 CONFIG_DRAM_ZQ=63351 CONFIG_USB0_VBUS_PIN="AXP0-VBUS-ENABLE" CONFIG_USB0_VBUS_DET="AXP0-VBUS-DETECT" +CONFIG_USB0_ID_DET="PH8" CONFIG_AXP_GPIO=y CONFIG_VIDEO_LCD_MODE="x:800,y:480,depth:18,pclk_khz:33000,le:87,ri:168,up:31,lo:13,hs:1,vs:1,sync:3,vmode:0" CONFIG_VIDEO_LCD_DCLK_PHASE=0 diff --git a/drivers/usb/musb-new/sunxi.c b/drivers/usb/musb-new/sunxi.c index ee018c7..cafb480 100644 --- a/drivers/usb/musb-new/sunxi.c +++ b/drivers/usb/musb-new/sunxi.c @@ -217,6 +217,11 @@ static int sunxi_musb_enable(struct musb *musb) printf("A charger is plugged into the OTG: "); return -ENODEV; } + ret = sunxi_usb_phy_id_detect(0); + if (ret == 1) { + printf("No host cable detected: "); + return -ENODEV; + } sunxi_usb_phy_power_on(0); /* port power on */ }
On Wed, 2015-06-17 at 21:34 +0200, Hans de Goede wrote:
When in host mode check if there is a host cable inserted into the otg port by checking the id pin. If there is no host cable return an error to make usb_lowlevel_init() exit early, rather then waiting for 1 second for a device which will never show up.
Signed-off-by: Hans de Goede hdegoede@redhat.com
I guess the defconfig fiddling is here because this is the culmination of these three patches which makes it useful to set this value? In which case:
Acked-by: Ian Campbell ijc@hellion.org.uk
Although I'd have been inclined to do it where the Kconfig option was added (ack to that too if you want to move).
Ian.
Hi,
On 19-06-15 09:40, Ian Campbell wrote:
On Wed, 2015-06-17 at 21:34 +0200, Hans de Goede wrote:
When in host mode check if there is a host cable inserted into the otg port by checking the id pin. If there is no host cable return an error to make usb_lowlevel_init() exit early, rather then waiting for 1 second for a device which will never show up.
Signed-off-by: Hans de Goede hdegoede@redhat.com
I guess the defconfig fiddling is here because this is the culmination of these three patches which makes it useful to set this value?
Right.
In which case:
Acked-by: Ian Campbell ijc@hellion.org.uk
Thanks,
Hans
Although I'd have been inclined to do it where the Kconfig option was added (ack to that too if you want to move).
Ian.
Move the musb config and platdata to the sunxi-musb glue, which is where it really belongs. This is preparation patch for adding device-model support for the sunxi-musb-host code.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- arch/arm/include/asm/arch-sunxi/usb_phy.h | 7 +++++++ board/sunxi/board.c | 28 ++----------------------- drivers/usb/musb-new/sunxi.c | 35 ++++++++++++++++++++++--------- 3 files changed, 34 insertions(+), 36 deletions(-)
diff --git a/arch/arm/include/asm/arch-sunxi/usb_phy.h b/arch/arm/include/asm/arch-sunxi/usb_phy.h index 5a9cacb..17d31b8 100644 --- a/arch/arm/include/asm/arch-sunxi/usb_phy.h +++ b/arch/arm/include/asm/arch-sunxi/usb_phy.h @@ -19,3 +19,10 @@ void sunxi_usb_phy_power_off(int index); int sunxi_usb_phy_vbus_detect(int index); int sunxi_usb_phy_id_detect(int index); void sunxi_usb_phy_enable_squelch_detect(int index, int enable); + +/* Not really phy related, but we have to declare this somewhere ... */ +#if defined(CONFIG_MUSB_HOST) || defined(CONFIG_MUSB_GADGET) +void sunxi_musb_board_init(void); +#else +#define sunxi_musb_board_init() +#endif diff --git a/board/sunxi/board.c b/board/sunxi/board.c index 2638bc1..e14725f 100644 --- a/board/sunxi/board.c +++ b/board/sunxi/board.c @@ -34,7 +34,6 @@ #include <asm/arch/usb_phy.h> #include <asm/gpio.h> #include <asm/io.h> -#include <linux/usb/musb.h> #include <net.h>
#if defined CONFIG_VIDEO_LCD_PANEL_I2C && !(defined CONFIG_SPL_BUILD) @@ -467,28 +466,6 @@ void sunxi_board_init(void) } #endif
-#if defined(CONFIG_MUSB_HOST) || defined(CONFIG_MUSB_GADGET) -extern const struct musb_platform_ops sunxi_musb_ops; - -static struct musb_hdrc_config musb_config = { - .multipoint = 1, - .dyn_fifo = 1, - .num_eps = 6, - .ram_bits = 11, -}; - -static struct musb_hdrc_platform_data musb_plat = { -#if defined(CONFIG_MUSB_HOST) - .mode = MUSB_HOST, -#else - .mode = MUSB_PERIPHERAL, -#endif - .config = &musb_config, - .power = 250, - .platform_ops = &sunxi_musb_ops, -}; -#endif - #ifdef CONFIG_USB_GADGET int g_dnl_board_usb_cable_connected(void) { @@ -551,9 +528,8 @@ int misc_init_r(void) if (ret) return ret; #endif -#if defined(CONFIG_MUSB_HOST) || defined(CONFIG_MUSB_GADGET) - musb_register(&musb_plat, NULL, (void *)SUNXI_USB0_BASE); -#endif + sunxi_musb_board_init(); + return 0; } #endif diff --git a/drivers/usb/musb-new/sunxi.c b/drivers/usb/musb-new/sunxi.c index cafb480..cbd2954 100644 --- a/drivers/usb/musb-new/sunxi.c +++ b/drivers/usb/musb-new/sunxi.c @@ -26,17 +26,9 @@ #include <asm/arch/gpio.h> #include <asm/arch/usb_phy.h> #include <asm-generic/gpio.h> +#include <linux/usb/musb.h> #include "linux-compat.h" #include "musb_core.h" -#ifdef CONFIG_AXP152_POWER -#include <axp152.h> -#endif -#ifdef CONFIG_AXP209_POWER -#include <axp209.h> -#endif -#ifdef CONFIG_AXP221_POWER -#include <axp221.h> -#endif
/****************************************************************************** ****************************************************************************** @@ -277,8 +269,31 @@ static int sunxi_musb_init(struct musb *musb) return 0; }
-const struct musb_platform_ops sunxi_musb_ops = { +static const struct musb_platform_ops sunxi_musb_ops = { .init = sunxi_musb_init, .enable = sunxi_musb_enable, .disable = sunxi_musb_disable, }; + +static struct musb_hdrc_config musb_config = { + .multipoint = 1, + .dyn_fifo = 1, + .num_eps = 6, + .ram_bits = 11, +}; + +static struct musb_hdrc_platform_data musb_plat = { +#if defined(CONFIG_MUSB_HOST) + .mode = MUSB_HOST, +#else + .mode = MUSB_PERIPHERAL, +#endif + .config = &musb_config, + .power = 250, + .platform_ops = &sunxi_musb_ops, +}; + +void sunxi_musb_board_init(void) +{ + musb_register(&musb_plat, NULL, (void *)SUNXI_USB0_BASE); +}
On Wed, 2015-06-17 at 21:34 +0200, Hans de Goede wrote:
Move the musb config and platdata to the sunxi-musb glue, which is where it really belongs. This is preparation patch for adding device-model support for the sunxi-musb-host code.
Signed-off-by: Hans de Goede hdegoede@redhat.com
arch/arm/include/asm/arch-sunxi/usb_phy.h | 7 +++++++ board/sunxi/board.c | 28 ++----------------------- drivers/usb/musb-new/sunxi.c | 35 ++++++++++++++++++++++--------- 3 files changed, 34 insertions(+), 36 deletions(-)
diff --git a/arch/arm/include/asm/arch-sunxi/usb_phy.h b/arch/arm/include/asm/arch-sunxi/usb_phy.h index 5a9cacb..17d31b8 100644 --- a/arch/arm/include/asm/arch-sunxi/usb_phy.h +++ b/arch/arm/include/asm/arch-sunxi/usb_phy.h @@ -19,3 +19,10 @@ void sunxi_usb_phy_power_off(int index); int sunxi_usb_phy_vbus_detect(int index); int sunxi_usb_phy_id_detect(int index); void sunxi_usb_phy_enable_squelch_detect(int index, int enable);
+/* Not really phy related, but we have to declare this somewhere ... */
I guess arch/arm/include/asm/arch-sunxi/usbc.h isn't any better?
With it here or there:
Acked-by: Ian Campbell ijc@hellion.org.uk
+#if defined(CONFIG_MUSB_HOST) || defined(CONFIG_MUSB_GADGET) +void sunxi_musb_board_init(void); +#else +#define sunxi_musb_board_init() +#endif
Ian.
Hi,
On 19-06-15 09:43, Ian Campbell wrote:
On Wed, 2015-06-17 at 21:34 +0200, Hans de Goede wrote:
Move the musb config and platdata to the sunxi-musb glue, which is where it really belongs. This is preparation patch for adding device-model support for the sunxi-musb-host code.
Signed-off-by: Hans de Goede hdegoede@redhat.com
arch/arm/include/asm/arch-sunxi/usb_phy.h | 7 +++++++ board/sunxi/board.c | 28 ++----------------------- drivers/usb/musb-new/sunxi.c | 35 ++++++++++++++++++++++--------- 3 files changed, 34 insertions(+), 36 deletions(-)
diff --git a/arch/arm/include/asm/arch-sunxi/usb_phy.h b/arch/arm/include/asm/arch-sunxi/usb_phy.h index 5a9cacb..17d31b8 100644 --- a/arch/arm/include/asm/arch-sunxi/usb_phy.h +++ b/arch/arm/include/asm/arch-sunxi/usb_phy.h @@ -19,3 +19,10 @@ void sunxi_usb_phy_power_off(int index); int sunxi_usb_phy_vbus_detect(int index); int sunxi_usb_phy_id_detect(int index); void sunxi_usb_phy_enable_squelch_detect(int index, int enable);
+/* Not really phy related, but we have to declare this somewhere ... */
I guess arch/arm/include/asm/arch-sunxi/usbc.h isn't any better?
Well we do not have that, and I did not feel it was worth adding yet another .h file for just this one function.
With it here or there:
Acked-by: Ian Campbell ijc@hellion.org.uk
Regards,
Hans
+#if defined(CONFIG_MUSB_HOST) || defined(CONFIG_MUSB_GADGET) +void sunxi_musb_board_init(void); +#else +#define sunxi_musb_board_init() +#endif
Ian.
On Fri, 2015-06-19 at 11:35 +0200, Hans de Goede wrote:
Hi,
On 19-06-15 09:43, Ian Campbell wrote:
On Wed, 2015-06-17 at 21:34 +0200, Hans de Goede wrote:
Move the musb config and platdata to the sunxi-musb glue, which is where it really belongs. This is preparation patch for adding device-model support for the sunxi-musb-host code.
Signed-off-by: Hans de Goede hdegoede@redhat.com
arch/arm/include/asm/arch-sunxi/usb_phy.h | 7 +++++++ board/sunxi/board.c | 28 ++----------------------- drivers/usb/musb-new/sunxi.c | 35 ++++++++++++++++++++++--------- 3 files changed, 34 insertions(+), 36 deletions(-)
diff --git a/arch/arm/include/asm/arch-sunxi/usb_phy.h b/arch/arm/include/asm/arch-sunxi/usb_phy.h index 5a9cacb..17d31b8 100644 --- a/arch/arm/include/asm/arch-sunxi/usb_phy.h +++ b/arch/arm/include/asm/arch-sunxi/usb_phy.h @@ -19,3 +19,10 @@ void sunxi_usb_phy_power_off(int index); int sunxi_usb_phy_vbus_detect(int index); int sunxi_usb_phy_id_detect(int index); void sunxi_usb_phy_enable_squelch_detect(int index, int enable);
+/* Not really phy related, but we have to declare this somewhere ... */
I guess arch/arm/include/asm/arch-sunxi/usbc.h isn't any better?
Well we do not have that, and I did not feel it was worth adding yet another .h file for just this one function.
Ah, my tree had:
u-boot.git$ wc -l arch/arm/include/asm/arch-sunxi/usbc.h 24 arch/arm/include/asm/arch-sunxi/usbc.h
But it was stale and the file has now gone in sunxi#next.
Ian.
Modify the sunxi musb glue to use the device-model for musb host mode.
This allows using musb in host mode together with other host drivers such as ehci / ohci, which is esp. useful on boards which use the musb controller in host-only mode, these boards have e.g. an usb-a receptacle or an usb to sata converter attached to the musb controller.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- board/sunxi/Kconfig | 2 +- drivers/usb/musb-new/sunxi.c | 57 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 1 deletion(-)
diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig index 4311c3e..6a19f85 100644 --- a/board/sunxi/Kconfig +++ b/board/sunxi/Kconfig @@ -586,7 +586,7 @@ config DM_SERIAL default y
config DM_USB - default y if !USB_MUSB_SUNXI + default y
config CMD_SETEXPR default y diff --git a/drivers/usb/musb-new/sunxi.c b/drivers/usb/musb-new/sunxi.c index cbd2954..d1cb8e0 100644 --- a/drivers/usb/musb-new/sunxi.c +++ b/drivers/usb/musb-new/sunxi.c @@ -26,9 +26,12 @@ #include <asm/arch/gpio.h> #include <asm/arch/usb_phy.h> #include <asm-generic/gpio.h> +#include <dm/lists.h> +#include <dm/root.h> #include <linux/usb/musb.h> #include "linux-compat.h" #include "musb_core.h" +#include "musb_uboot.h"
/****************************************************************************** ****************************************************************************** @@ -293,7 +296,61 @@ static struct musb_hdrc_platform_data musb_plat = { .platform_ops = &sunxi_musb_ops, };
+#ifdef CONFIG_MUSB_HOST +int musb_usb_probe(struct udevice *dev) +{ + struct musb_host_data *host = dev_get_priv(dev); + struct usb_bus_priv *priv = dev_get_uclass_priv(dev); + + priv->desc_before_addr = true; + + if (!host->host) { + host->host = musb_init_controller(&musb_plat, NULL, + (void *)SUNXI_USB0_BASE); + if (!host->host) { + printf("Failed to init the controller\n"); + return -EIO; + } + } + + printf("MUSB OTG in host-mode\n"); + + return musb_lowlevel_init(host); +} + +int musb_usb_remove(struct udevice *dev) +{ + struct musb_host_data *host = dev_get_priv(dev); + + musb_stop(host->host); + + return 0; +} + +U_BOOT_DRIVER(usb_musb) = { + .name = "sunxi-musb", + .id = UCLASS_USB, + .probe = musb_usb_probe, + .remove = musb_usb_remove, + .ops = &musb_usb_ops, + .platdata_auto_alloc_size = sizeof(struct usb_platdata), + .priv_auto_alloc_size = sizeof(struct musb_host_data), +}; +#endif + void sunxi_musb_board_init(void) { +#ifdef CONFIG_MUSB_HOST + struct udevice *dev; + + /* + * Bind the driver directly for now as musb linux kernel support is + * still pending upstream so our dts files do not have the necessay + * nodes yet. TODO: Remove this as soon as the dts nodes are in place + * and bind by compatible instead. + */ + device_bind_driver(dm_root(), "sunxi-musb", "sunxi-musb", &dev); +#else musb_register(&musb_plat, NULL, (void *)SUNXI_USB0_BASE); +#endif }
On Wed, 2015-06-17 at 21:34 +0200, Hans de Goede wrote:
[...]
* Bind the driver directly for now as musb linux kernel support is* still pending upstream so our dts files do not have the necessay
"necessary". Otherwise:
Acked-by: Ian Campbell ijc@hellion.org.uk
So far as this being a sunxi change goes, although I think this probably needs other acks from the CC line too.
Ian.
* nodes yet. TODO: Remove this as soon as the dts nodes are in place* and bind by compatible instead.*/- device_bind_driver(dm_root(), "sunxi-musb", "sunxi-musb", &dev);
+#else musb_register(&musb_plat, NULL, (void *)SUNXI_USB0_BASE); +#endif }
Start using the new Kconfig options which are available for most of the USB settings now.
This also allows us to use "CONFIG_USB_EHCI_HCD=y" in defconfig files for new boards instead of CONFIG_SYS_EXTRA_OPTIONS="USB_EHCI".
Signed-off-by: Hans de Goede hdegoede@redhat.com --- board/sunxi/Kconfig | 9 +++++++++ include/configs/sunxi-common.h | 5 ----- 2 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig index 6a19f85..8d55cd6 100644 --- a/board/sunxi/Kconfig +++ b/board/sunxi/Kconfig @@ -594,4 +594,13 @@ config CMD_SETEXPR config CMD_NET default y
+config CMD_USB + default y + +config USB + default y + +config USB_STORAGE + default y + endif diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h index 063abd5..e2371a1 100644 --- a/include/configs/sunxi-common.h +++ b/include/configs/sunxi-common.h @@ -338,11 +338,6 @@ extern int soft_i2c_gpio_scl; #define CONFIG_MUSB_PIO_ONLY #endif
-#if defined CONFIG_USB_EHCI || defined CONFIG_USB_MUSB_SUNXI -#define CONFIG_CMD_USB -#define CONFIG_USB_STORAGE -#endif - #ifdef CONFIG_USB_KEYBOARD #define CONFIG_CONSOLE_MUX #define CONFIG_PREBOOT
On Wed, 2015-06-17 at 21:34 +0200, Hans de Goede wrote:
Start using the new Kconfig options which are available for most of the USB settings now.
This also allows us to use "CONFIG_USB_EHCI_HCD=y" in defconfig files for new boards instead of CONFIG_SYS_EXTRA_OPTIONS="USB_EHCI".
Should there not be a raft of defconfig changes at the same time? Or is that nice but not necessary at this stage?
Signed-off-by: Hans de Goede hdegoede@redhat.com
board/sunxi/Kconfig | 9 +++++++++ include/configs/sunxi-common.h | 5 ----- 2 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig index 6a19f85..8d55cd6 100644 --- a/board/sunxi/Kconfig +++ b/board/sunxi/Kconfig @@ -594,4 +594,13 @@ config CMD_SETEXPR config CMD_NET default y
+config CMD_USB
- default y
+config USB
- default y
+config USB_STORAGE
- default y
endif diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h index 063abd5..e2371a1 100644 --- a/include/configs/sunxi-common.h +++ b/include/configs/sunxi-common.h @@ -338,11 +338,6 @@ extern int soft_i2c_gpio_scl; #define CONFIG_MUSB_PIO_ONLY #endif
-#if defined CONFIG_USB_EHCI || defined CONFIG_USB_MUSB_SUNXI -#define CONFIG_CMD_USB -#define CONFIG_USB_STORAGE -#endif
#ifdef CONFIG_USB_KEYBOARD #define CONFIG_CONSOLE_MUX #define CONFIG_PREBOOT
Hi,
On 19-06-15 09:46, Ian Campbell wrote:
On Wed, 2015-06-17 at 21:34 +0200, Hans de Goede wrote:
Start using the new Kconfig options which are available for most of the USB settings now.
This also allows us to use "CONFIG_USB_EHCI_HCD=y" in defconfig files for new boards instead of CONFIG_SYS_EXTRA_OPTIONS="USB_EHCI".
Should there not be a raft of defconfig changes at the same time? Or is that nice but not necessary at this stage?
There could be a of defconfig changes at the same time, but it is not necessary, so I decided to wait with cleaning this up till later.
I would like to first see all the other stuff from CONFIG_SYS_EXTRA_OPTIONS by Kconfig-ified too, and then we can cleanup all the CONFIG_SYS_EXTRA_OPTIONS usage in one go.
Regards,
Hans
Signed-off-by: Hans de Goede hdegoede@redhat.com
board/sunxi/Kconfig | 9 +++++++++ include/configs/sunxi-common.h | 5 ----- 2 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig index 6a19f85..8d55cd6 100644 --- a/board/sunxi/Kconfig +++ b/board/sunxi/Kconfig @@ -594,4 +594,13 @@ config CMD_SETEXPR config CMD_NET default y
+config CMD_USB
- default y
+config USB
- default y
+config USB_STORAGE
- default y
- endif
diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h index 063abd5..e2371a1 100644 --- a/include/configs/sunxi-common.h +++ b/include/configs/sunxi-common.h @@ -338,11 +338,6 @@ extern int soft_i2c_gpio_scl; #define CONFIG_MUSB_PIO_ONLY #endif
-#if defined CONFIG_USB_EHCI || defined CONFIG_USB_MUSB_SUNXI -#define CONFIG_CMD_USB -#define CONFIG_USB_STORAGE -#endif
- #ifdef CONFIG_USB_KEYBOARD #define CONFIG_CONSOLE_MUX #define CONFIG_PREBOOT
On Fri, 2015-06-19 at 11:37 +0200, Hans de Goede wrote:
Hi,
On 19-06-15 09:46, Ian Campbell wrote:
On Wed, 2015-06-17 at 21:34 +0200, Hans de Goede wrote:
Start using the new Kconfig options which are available for most of the USB settings now.
This also allows us to use "CONFIG_USB_EHCI_HCD=y" in defconfig files for new boards instead of CONFIG_SYS_EXTRA_OPTIONS="USB_EHCI".
Should there not be a raft of defconfig changes at the same time? Or is that nice but not necessary at this stage?
There could be a of defconfig changes at the same time, but it is not necessary, so I decided to wait with cleaning this up till later.
OK: Acked-by: Ian Campbell ijc@hellion.org.uk
I would like to first see all the other stuff from CONFIG_SYS_EXTRA_OPTIONS by Kconfig-ified too, and then we can cleanup all the CONFIG_SYS_EXTRA_OPTIONS usage in one go.
This allows using devices plugged into both ports of the tablet.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- arch/arm/dts/sun8i-a23-a33.dtsi | 18 ++++++++++++++++++ arch/arm/dts/sun8i-a33-ga10h-v1.1.dts | 8 ++++++++ configs/ga10h_v1_1_defconfig | 2 ++ 3 files changed, 28 insertions(+)
diff --git a/arch/arm/dts/sun8i-a23-a33.dtsi b/arch/arm/dts/sun8i-a23-a33.dtsi index faea94e..eeb6655 100644 --- a/arch/arm/dts/sun8i-a23-a33.dtsi +++ b/arch/arm/dts/sun8i-a23-a33.dtsi @@ -332,6 +332,24 @@ #size-cells = <0>; };
+ ehci0: usb@01c1a000 { + compatible = "allwinner,sun8i-a23-ehci", "generic-ehci"; + reg = <0x01c1a000 0x100>; + interrupts = <GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&ahb1_gates 26>; + resets = <&ahb1_rst 26>; + status = "disabled"; + }; + + ohci0: usb@01c1a400 { + compatible = "allwinner,sun8i-a23-ohci", "generic-ohci"; + reg = <0x01c1a400 0x100>; + interrupts = <GIC_SPI 73 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&ahb1_gates 29>, <&usb_clk 16>; + resets = <&ahb1_rst 29>; + status = "disabled"; + }; + pio: pinctrl@01c20800 { /* compatible gets set in SoC specific dtsi file */ reg = <0x01c20800 0x400>; diff --git a/arch/arm/dts/sun8i-a33-ga10h-v1.1.dts b/arch/arm/dts/sun8i-a33-ga10h-v1.1.dts index 8667033..da16343 100644 --- a/arch/arm/dts/sun8i-a33-ga10h-v1.1.dts +++ b/arch/arm/dts/sun8i-a33-ga10h-v1.1.dts @@ -61,6 +61,10 @@ }; };
+&ehci0 { + status = "okay"; +}; + &i2c0 { pinctrl-names = "default"; pinctrl-0 = <&i2c0_pins_a>; @@ -109,6 +113,10 @@ status = "okay"; };
+&ohci0 { + status = "okay"; +}; + &pio { mmc0_cd_pin_q8h: mmc0_cd_pin@0 { allwinner,pins = "PB4"; diff --git a/configs/ga10h_v1_1_defconfig b/configs/ga10h_v1_1_defconfig index 67b40c2..8d3063b 100644 --- a/configs/ga10h_v1_1_defconfig +++ b/configs/ga10h_v1_1_defconfig @@ -6,6 +6,7 @@ CONFIG_DRAM_ZQ=15291 CONFIG_DRAM_ODT_EN=y CONFIG_USB0_VBUS_PIN="AXP0-VBUS-ENABLE" CONFIG_USB0_VBUS_DET="AXP0-VBUS-DETECT" +CONFIG_USB0_ID_DET="PH8" CONFIG_AXP_GPIO=y CONFIG_VIDEO_LCD_MODE="x:1024,y:600,depth:18,pclk_khz:52000,le:138,ri:162,up:22,lo:10,hs:20,vs:3,sync:3,vmode:0" CONFIG_VIDEO_LCD_DCLK_PHASE=0 @@ -19,3 +20,4 @@ CONFIG_SPL=y CONFIG_SYS_EXTRA_OPTIONS="CONS_INDEX=5" CONFIG_AXP221_DLDO1_VOLT=3300 CONFIG_AXP221_ALDO1_VOLT=3000 +CONFIG_USB_EHCI_HCD=y
On Wed, 2015-06-17 at 21:34 +0200, Hans de Goede wrote:
This allows using devices plugged into both ports of the tablet.
Signed-off-by: Hans de Goede hdegoede@redhat.com
Acked-by: Ian Campbell ijc@hellion.org.uk
Hi Hans,
On 17 June 2015 at 13:33, Hans de Goede hdegoede@redhat.com wrote:
Hi Marek and Simon,
This series started out with the idea that it would be a nice small project for the weekend, but it turned out to be a bit more work...
The main purpose of this series is to convert the musb host mode code to the device-model this has also resulted in various usb fixes / cleanups / reworking to make this possible, both in the generic usb code as well as in the device model usb code.
Given that this touches both, I think it is probably best to merge the first 15 patches through Simon's tree like we did last time, then once those are place I can merge the sunxi bits. Note this is intended for v2015.10 (ofcourse).
Note that this series is useful for a bunch more boards then just the single one the last patch updates to use musb + ehci + ohci, but that is the one I've been testing with other defconfig-s will be updated with followup patches.
Please review.
Thanks for putting this together. Actually I'm not sure what musb is. Can you explain what it is used for - it seems to be referred to as a new version of something else. Why do we have musb and non-musb? Is there a README somewhere I could read?
Regards, Simon
Hi,
On 19-06-15 15:10, Simon Glass wrote:
Hi Hans,
On 17 June 2015 at 13:33, Hans de Goede hdegoede@redhat.com wrote:
Hi Marek and Simon,
This series started out with the idea that it would be a nice small project for the weekend, but it turned out to be a bit more work...
The main purpose of this series is to convert the musb host mode code to the device-model this has also resulted in various usb fixes / cleanups / reworking to make this possible, both in the generic usb code as well as in the device model usb code.
Given that this touches both, I think it is probably best to merge the first 15 patches through Simon's tree like we did last time, then once those are place I can merge the sunxi bits. Note this is intended for v2015.10 (ofcourse).
Note that this series is useful for a bunch more boards then just the single one the last patch updates to use musb + ehci + ohci, but that is the one I've been testing with other defconfig-s will be updated with followup patches.
Please review.
Thanks for putting this together. Actually I'm not sure what musb is. Can you explain what it is used for - it seems to be referred to as a new version of something else. Why do we have musb and non-musb? Is there a README somewhere I could read?
musb stands for mentor-graphics usb it is an otg usb2 ip-block used in various SoCs as otg controller. As an otg controller it can be used in both host or peripheral mode. This series converts the code for using it in host-mode to the device-model.
Regards,
Hans
Hi,
On 19-06-15 15:10, Simon Glass wrote:
Hi Hans,
On 17 June 2015 at 13:33, Hans de Goede hdegoede@redhat.com wrote:
Hi Marek and Simon,
This series started out with the idea that it would be a nice small project for the weekend, but it turned out to be a bit more work...
The main purpose of this series is to convert the musb host mode code to the device-model this has also resulted in various usb fixes / cleanups / reworking to make this possible, both in the generic usb code as well as in the device model usb code.
Given that this touches both, I think it is probably best to merge the first 15 patches through Simon's tree like we did last time, then once those are place I can merge the sunxi bits. Note this is intended for v2015.10 (ofcourse).
Note that this series is useful for a bunch more boards then just the single one the last patch updates to use musb + ehci + ohci, but that is the one I've been testing with other defconfig-s will be updated with followup patches.
Please review.
Thanks for putting this together. Actually I'm not sure what musb is. Can you explain what it is used for - it seems to be referred to as a new version of something else. Why do we have musb and non-musb? Is there a README somewhere I could read?
p.s.
As for there being both a drivers/usb/musb and a driver/usb/musb-new directoyry, I believe that they both are drivers for the same hardware, but not all boards have been ported to musb-new yet. musb-new is derived from recent kernel code for the musb controller.
Regards,
Hans
Regards, Simon
participants (3)
-
Hans de Goede -
Ian Campbell -
Simon Glass