[U-Boot] [RFC 0/4] dm-usb fixes + dm conversion for sunxi-ehci

Hi All,
Here are a couple of dm-usb fixes and the dm conversion for sunxi-ehci, note that this series is currently still RFC only as I'm still having some issues with USB-1 devices not working which I need to investigate further.
I guess that some of the dm-usb fixed can be applied regardless as they are an improvement over the current status quo.
Regards,
Hans

Currently we copy over a number of usb_device values stored in the on stack struct usb_device probed in usb_scan_device() to the final driver-model managed struct usb_device in usb_child_pre_probe() through usb_device_platdata, and then call usb_select_config() to fill in the rest.
There are 2 problems with this approach:
1) It does not fill in enough fields before calling usb_select_config(), specifically it does not fill in ep0's maxpacketsize causing a div by zero exception in the ehci driver.
2) It unnecessarily redoes a number of usb requests making usb probing slower, and potentially upsetting some devices.
This commit fixes both issues by removing the usb_select_config() call from usb_child_pre_probe(), and instead of copying over things field by field through usb_device_platdata, store a pointer to the in stack usb_device (which is still valid when usb_child_pre_probe() gets called) and copy over the entire struct.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- Changes in v2: -Remove code setting controller_dev from usb_child_pre_probe as that is already done in usb_scan_device() and copied over with the rest --- drivers/usb/host/usb-uclass.c | 27 ++++++--------------------- include/usb.h | 5 +---- 2 files changed, 7 insertions(+), 25 deletions(-)
diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c index 714bc0e..95e371d 100644 --- a/drivers/usb/host/usb-uclass.c +++ b/drivers/usb/host/usb-uclass.c @@ -535,12 +535,7 @@ int usb_scan_device(struct udevice *parent, int port, } plat = dev_get_parent_platdata(dev); debug("%s: Probing '%s', plat=%p\n", __func__, dev->name, plat); - plat->devnum = udev->devnum; - plat->speed = udev->speed; - plat->slot_id = udev->slot_id; - plat->portnr = port; - debug("** device '%s': stashing slot_id=%d\n", dev->name, - plat->slot_id); + plat->udev = udev; priv->next_addr++; ret = device_probe(dev); if (ret) { @@ -599,25 +594,15 @@ int usb_get_bus(struct udevice *dev, struct udevice **busp)
int usb_child_pre_probe(struct udevice *dev) { - struct udevice *bus; struct usb_device *udev = dev_get_parentdata(dev); struct usb_dev_platdata *plat = dev_get_parent_platdata(dev); - int ret;
- ret = usb_get_bus(dev, &bus); - if (ret) - return ret; - udev->controller_dev = bus; + /* + * Copy over all the values set in the on stack struct usb_device in + * usb_scan_device() to our final struct usb_device for this dev. + */ + *udev = *(plat->udev); udev->dev = dev; - udev->devnum = plat->devnum; - udev->slot_id = plat->slot_id; - udev->portnr = plat->portnr; - udev->speed = plat->speed; - debug("** device '%s': getting slot_id=%d\n", dev->name, plat->slot_id); - - ret = usb_select_config(udev); - if (ret) - return ret;
return 0; } diff --git a/include/usb.h b/include/usb.h index 1984e8f..1b667ff 100644 --- a/include/usb.h +++ b/include/usb.h @@ -581,10 +581,7 @@ struct usb_platdata { */ struct usb_dev_platdata { struct usb_device_id id; - enum usb_device_speed speed; - int devnum; - int slot_id; - int portnr; /* Hub port number, 1..n */ + struct usb_device *udev; #ifdef CONFIG_SANDBOX struct usb_string *strings; /* NULL-terminated list of descriptor pointers */

Use the controller_dev pointer in the ehci hcd code rather then going up the tree till we find the first UCLASS_USB device.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- drivers/usb/host/ehci-hcd.c | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c index bd9861d..19f1e29 100644 --- a/drivers/usb/host/ehci-hcd.c +++ b/drivers/usb/host/ehci-hcd.c @@ -125,14 +125,7 @@ static struct descriptor { static struct ehci_ctrl *ehci_get_ctrl(struct usb_device *udev) { #ifdef CONFIG_DM_USB - struct udevice *dev; - - /* Find the USB controller */ - for (dev = udev->dev; - device_get_uclass_id(dev) != UCLASS_USB; - dev = dev->parent) - ; - return dev_get_priv(dev); + return dev_get_priv(udev->controller_dev); #else return udev->controller; #endif

When calling into the hcd code in usb_scan_device() we do not yet have the actual udevice for the device we are scanning, so we temporarily set usb_device.dev to the parent.
This means that we cannot use usb_device.dev to accurately determine our place in the usb topology when reading descriptors, and this is necessary to correctly program the usb-2 hub tt settings when a usb-1 device is connected to a usb-2 hub.
This commit (re)adds a direct parent usb_device pointer to struct usb_device so that the usb hcd code can get to the usb_device parent without needing to go through the dm.
This fixes usb-1 devices plugged into usb-2 hubs not working with the driver model.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- drivers/usb/host/ehci-hcd.c | 24 +----------------------- drivers/usb/host/usb-uclass.c | 5 ++--- include/usb.h | 2 +- 3 files changed, 4 insertions(+), 27 deletions(-)
diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c index 19f1e29..00c038c 100644 --- a/drivers/usb/host/ehci-hcd.c +++ b/drivers/usb/host/ehci-hcd.c @@ -292,7 +292,6 @@ static void ehci_update_endpt2_dev_n_port(struct usb_device *udev, struct QH *qh) { struct usb_device *ttdev; - int parent_devnum;
if (udev->speed != USB_SPEED_LOW && udev->speed != USB_SPEED_FULL) return; @@ -302,35 +301,14 @@ static void ehci_update_endpt2_dev_n_port(struct usb_device *udev, * the tt, so of the first upstream usb-2 hub, there may be usb-1 hubs * in the tree before that one! */ -#ifdef CONFIG_DM_USB - struct udevice *parent; - - for (ttdev = udev; ; ) { - struct udevice *dev = ttdev->dev; - - if (dev->parent && - device_get_uclass_id(dev->parent) == UCLASS_USB_HUB) - parent = dev->parent; - else - parent = NULL; - if (!parent) - return; - ttdev = dev_get_parentdata(parent); - if (!ttdev->speed != USB_SPEED_HIGH) - break; - } - parent_devnum = ttdev->devnum; -#else ttdev = udev; while (ttdev->parent && ttdev->parent->speed != USB_SPEED_HIGH) ttdev = ttdev->parent; if (!ttdev->parent) return; - parent_devnum = ttdev->parent->devnum; -#endif
qh->qh_endpt2 |= cpu_to_hc32(QH_ENDPT2_PORTNUM(ttdev->portnr) | - QH_ENDPT2_HUBADDR(parent_devnum)); + QH_ENDPT2_HUBADDR(ttdev->parent->devnum)); }
static int diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c index 95e371d..0157bc6 100644 --- a/drivers/usb/host/usb-uclass.c +++ b/drivers/usb/host/usb-uclass.c @@ -470,7 +470,6 @@ int usb_scan_device(struct udevice *parent, int port, bool created = false; struct usb_dev_platdata *plat; struct usb_bus_priv *priv; - struct usb_device *parent_udev; int ret; ALLOC_CACHE_ALIGN_BUFFER(struct usb_device, udev, 1); struct usb_interface_descriptor *iface = &udev->config.if_desc[0].desc; @@ -515,9 +514,9 @@ int usb_scan_device(struct udevice *parent, int port, udev->devnum = priv->next_addr + 1; udev->portnr = port; debug("Calling usb_setup_device(), portnr=%d\n", udev->portnr); - parent_udev = device_get_uclass_id(parent) == UCLASS_USB_HUB ? + udev->parent = device_get_uclass_id(parent) == UCLASS_USB_HUB ? dev_get_parentdata(parent) : NULL; - ret = usb_setup_device(udev, priv->desc_before_addr, parent_udev, port); + ret = usb_setup_device(udev, priv->desc_before_addr, udev->parent, port); debug("read_descriptor for '%s': ret=%d\n", parent->name, ret); if (ret) return ret; diff --git a/include/usb.h b/include/usb.h index 1b667ff..7876df4 100644 --- a/include/usb.h +++ b/include/usb.h @@ -141,9 +141,9 @@ struct usb_device { int act_len; /* transfered bytes */ int maxchild; /* Number of ports if hub */ int portnr; /* Port number, 1=first */ -#ifndef CONFIG_DM_USB /* parent hub, or NULL if this is the root hub */ struct usb_device *parent; +#ifndef CONFIG_DM_USB struct usb_device *children[USB_MAXCHILDREN]; void *controller; /* hardware controller private data */ #endif

Convert sunxi-boards which use the sunxi-ehci code to the driver-model.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- board/sunxi/Kconfig | 3 ++ drivers/usb/host/ehci-sunxi.c | 95 ++++++++++++++++++++++++++++++------------- 2 files changed, 69 insertions(+), 29 deletions(-)
diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig index 18e5561..6dcbff3 100644 --- a/board/sunxi/Kconfig +++ b/board/sunxi/Kconfig @@ -559,4 +559,7 @@ config DM_ETH config DM_SERIAL default y
+config DM_USB + default y if !USB_MUSB_SUNXI + endif diff --git a/drivers/usb/host/ehci-sunxi.c b/drivers/usb/host/ehci-sunxi.c index 0edb643..9c6703c 100644 --- a/drivers/usb/host/ehci-sunxi.c +++ b/drivers/usb/host/ehci-sunxi.c @@ -14,53 +14,90 @@ #include <asm/arch/clock.h> #include <asm/arch/usb_phy.h> #include <asm/io.h> +#include <dm.h> #include "ehci.h"
-int ehci_hcd_init(int index, enum usb_init_type init, struct ehci_hccr **hccr, - struct ehci_hcor **hcor) +struct ehci_sunxi_priv { + struct ehci_ctrl ehci; + int ahb_gate_mask; + int phy_index; +}; + +static int ehci_usb_ofdata_to_platdata(struct udevice *dev) +{ + return 0; +} + +static int ehci_usb_probe(struct udevice *dev) { struct sunxi_ccm_reg *ccm = (struct sunxi_ccm_reg *)SUNXI_CCM_BASE; - int ahb_gate_offset; + struct usb_platdata *plat = dev_get_platdata(dev); + struct ehci_sunxi_priv *priv = dev_get_priv(dev); + struct ehci_hccr *hccr = (struct ehci_hccr *)dev_get_addr(dev); + struct ehci_hcor *hcor; + + if (hccr == (void *)SUNXI_USB1_BASE) { + priv->ahb_gate_mask = 1 << AHB_GATE_OFFSET_USB_EHCI0; + priv->phy_index = 1; + } else { + priv->ahb_gate_mask = 1 << AHB_GATE_OFFSET_USB_EHCI1; + priv->phy_index = 2; + }
- ahb_gate_offset = index ? AHB_GATE_OFFSET_USB_EHCI1 : - AHB_GATE_OFFSET_USB_EHCI0; - setbits_le32(&ccm->ahb_gate0, 1 << ahb_gate_offset); + setbits_le32(&ccm->ahb_gate0, priv->ahb_gate_mask); #ifdef CONFIG_SUNXI_GEN_SUN6I - setbits_le32(&ccm->ahb_reset0_cfg, 1 << ahb_gate_offset); + setbits_le32(&ccm->ahb_reset0_cfg, priv->ahb_gate_mask); #endif
- sunxi_usb_phy_init(index + 1); - sunxi_usb_phy_power_on(index + 1); + sunxi_usb_phy_init(priv->phy_index); + sunxi_usb_phy_power_on(priv->phy_index);
- if (index == 0) - *hccr = (void *)SUNXI_USB1_BASE; - else - *hccr = (void *)SUNXI_USB2_BASE; + hcor = (struct ehci_hcor *)((uint32_t)hccr + + HC_LENGTH(ehci_readl(&hccr->cr_capbase)));
- *hcor = (struct ehci_hcor *)((uint32_t) *hccr - + HC_LENGTH(ehci_readl(&(*hccr)->cr_capbase))); - - debug("sunxi-ehci: init hccr %x and hcor %x hc_length %d\n", - (uint32_t)*hccr, (uint32_t)*hcor, - (uint32_t)HC_LENGTH(ehci_readl(&(*hccr)->cr_capbase))); - - return 0; + return ehci_register(dev, hccr, hcor, NULL, 0, plat->init_type); }
-int ehci_hcd_stop(int index) +static int ehci_usb_remove(struct udevice *dev) { struct sunxi_ccm_reg *ccm = (struct sunxi_ccm_reg *)SUNXI_CCM_BASE; - int ahb_gate_offset; + struct ehci_sunxi_priv *priv = dev_get_priv(dev); + int ret;
- sunxi_usb_phy_power_off(index + 1); - sunxi_usb_phy_exit(index + 1); + ret = ehci_deregister(dev); + if (ret) + return ret; + + sunxi_usb_phy_power_off(priv->phy_index); + sunxi_usb_phy_exit(priv->phy_index);
- ahb_gate_offset = index ? AHB_GATE_OFFSET_USB_EHCI1 : - AHB_GATE_OFFSET_USB_EHCI0; #ifdef CONFIG_SUNXI_GEN_SUN6I - clrbits_le32(&ccm->ahb_reset0_cfg, 1 << ahb_gate_offset); + clrbits_le32(&ccm->ahb_reset0_cfg, priv->ahb_gate_mask); #endif - clrbits_le32(&ccm->ahb_gate0, 1 << ahb_gate_offset); + clrbits_le32(&ccm->ahb_gate0, priv->ahb_gate_mask);
return 0; } + +static const struct udevice_id ehci_usb_ids[] = { + { .compatible = "allwinner,sun4i-a10-ehci", }, + { .compatible = "allwinner,sun5i-a13-ehci", }, + { .compatible = "allwinner,sun6i-a31-ehci", }, + { .compatible = "allwinner,sun7i-a20-ehci", }, + { .compatible = "allwinner,sun8i-a23-ehci", }, + { .compatible = "allwinner,sun9i-a80-ehci", }, + { } +}; + +U_BOOT_DRIVER(usb_ehci) = { + .name = "ehci_sunxi", + .id = UCLASS_USB, + .of_match = ehci_usb_ids, + .ofdata_to_platdata = ehci_usb_ofdata_to_platdata, + .probe = ehci_usb_probe, + .remove = ehci_usb_remove, + .ops = &ehci_usb_ops, + .platdata_auto_alloc_size = sizeof(struct usb_platdata), + .priv_auto_alloc_size = sizeof(struct ehci_sunxi_priv), + .flags = DM_FLAG_ALLOC_PRIV_DMA, +};

Hi,
On 29-04-15 21:18, Hans de Goede wrote:
Hi All,
Here are a couple of dm-usb fixes and the dm conversion for sunxi-ehci, note that this series is currently still RFC only as I'm still having some issues with USB-1 devices not working which I need to investigate further.
I guess that some of the dm-usb fixed can be applied regardless as they are an improvement over the current status quo.
Regards,
Hans
p.s.
This series also fixes the dm usb reset issue I was seeing, that was an issue in the dm conversion of ehci-sunxi.c and thus entirely my fault.
Regards,
Hans
participants (1)
-
Hans de Goede