[U-Boot] [PATCH 0/8] usb: driver-model fixes and dm support sunxi-ehci.c
Hi Simon and Marek,
This series completes my work on converting the sunxi usb/ehci code to the driver model. With this series everything works as it did before, and all the issues I've been seeing when switching to the driver model are resolved.
Please review / merge. I think it would be best to merge this through the dm tree.
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 --- 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 */
Hi Hans,
On 30 April 2015 at 08:35, Hans de Goede hdegoede@redhat.com wrote:
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:
- 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.
Didn't you have another patch that fixes that?
- It unnecessarily redoes a number of usb requests making usb probing slower,
and potentially upsetting some devices.
Does it actually upset anything?
The extra requests are in the second call to usb_select_config().
Do you think we could put the things we want to copy in a struct, and copy just those?
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
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 */ -- 2.3.6
Regards, Simon
Hi,
On 01-05-15 06:11, Simon Glass wrote:
Hi Hans,
On 30 April 2015 at 08:35, Hans de Goede hdegoede@redhat.com wrote:
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:
- 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.
Didn't you have another patch that fixes that?
Yes, but as explained in the coverletter of that patch I believe that this version is better. It would seem we disagree on that though :)
- It unnecessarily redoes a number of usb requests making usb probing slower,
and potentially upsetting some devices.
Does it actually upset anything?
Not to my knowledge, but I'm afraid that with some devices it may.
The extra requests are in the second call to usb_select_config().
Correct.
Do you think we could put the things we want to copy in a struct, and copy just those?
We would end up pretty much duplicating usb_device AFAICT, since we need the descriptors, the max packet sizes per endpoint parsed from them, etc.
Anyways since you clearly dislike this patch I'll drop it for v2, replacing it with my original fix for the ep0 maxpacket not being set issue, assuming that doing so does not cause any regressions during my testing.
Regards,
Hans
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
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;-- 2.3.6
Regards, Simon
Hi,
On 01-05-15 09:21, Hans de Goede wrote:
Hi,
On 01-05-15 06:11, Simon Glass wrote:
Hi Hans,
On 30 April 2015 at 08:35, Hans de Goede hdegoede@redhat.com wrote:
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:
- 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.
Didn't you have another patch that fixes that?
Yes, but as explained in the coverletter of that patch I believe that this version is better. It would seem we disagree on that though :)
- It unnecessarily redoes a number of usb requests making usb probing slower,
and potentially upsetting some devices.
Does it actually upset anything?
Not to my knowledge, but I'm afraid that with some devices it may.
The extra requests are in the second call to usb_select_config().
Correct.
Do you think we could put the things we want to copy in a struct, and copy just those?
We would end up pretty much duplicating usb_device AFAICT, since we need the descriptors, the max packet sizes per endpoint parsed from them, etc.
Anyways since you clearly dislike this patch I'll drop it for v2, replacing it with my original fix for the ep0 maxpacket not being set issue, assuming that doing so does not cause any regressions during my testing.
Ok, so my first test, which is hooking up a sunxi device to my dvi/usb kvm switch fails immediately when I use the maxpacketsize fix instead of my fix to avoid calling get_config twice. This is actually a pretty though test-case as the kvm presents itself + the keyboard and mouse as a complex hub hierarchy with both usb-2 and usb-1 hubs in there, which is what makes it a great test-case.
Now I could spend a couple of hours debugging this and maybe find a different fix, but this really shows that there is a reason why all usb stacks do the device probing / descriptor reading all in the exact same sequence, because usb devices are cheap and there qa consists of plug it into $random windows version running machine, does it work? Yes -> ship it.
So I really believe that my original fix for this is best. As for trying to pass all the bits we need through platdata rather then passing the struct usb_device itself. I can see value in that as part of a patch-set to get rid of usb_device, iow as part of a larger patchset but until then it just feels like make work to me.
So I'm going to stick with my original approach for v1 of this patch.
Regards,
Hans
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
Hi Hans,
On 30 April 2015 at 08:35, Hans de Goede hdegoede@redhat.com wrote:
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
My intent was to remove udev->controller_dev, since I was hoping to remove all pointers other than those maintained by driver model. Does this patch actually fix a bug?
Regards, Simon
Hi,
On 01-05-15 06:11, Simon Glass wrote:
Hi Hans,
On 30 April 2015 at 08:35, Hans de Goede hdegoede@redhat.com wrote:
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);My intent was to remove udev->controller_dev, since I was hoping to remove all pointers other than those maintained by driver model. Does this patch actually fix a bug?
I initially wrote this because my plan was to set usb_device.dev to NULL during the initial probing in usb_scan_device, as setting it to parent at that point is somewhat bogus.
I dropped the setting of usb_device.dev to NULL later because I was afraid it may cause regressions for other hcd code. But I kept this as it seemed like a nice cleanup.
I still think this needs a bit of cleanup, if we keep doing the lookup this way, at least it should use usb_get_bus rather then looping itself.
I'll put a patch for that in v2 of this set replacing this one.
Regards,
Hans
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
Hi Hans,
On 30 April 2015 at 08:35, Hans de Goede hdegoede@redhat.com wrote:
When calling into the hcd code in usb_scan_device() we do not yet have the actual udevice for the device we are scanning, so we temporarily set usb_device.dev to the parent.
This means that we cannot use usb_device.dev to accurately determine our place in the usb topology when reading descriptors, and this is necessary to correctly program the usb-2 hub tt settings when a usb-1 device is connected to a usb-2 hub.
This commit (re)adds a direct parent usb_device pointer to struct usb_device so that the usb hcd code can get to the usb_device parent without needing to go through the dm.
This fixes usb-1 devices plugged into usb-2 hubs not working with the driver model.
Signed-off-by: Hans de Goede hdegoede@redhat.com
drivers/usb/host/ehci-hcd.c | 24 +----------------------- drivers/usb/host/usb-uclass.c | 5 ++--- include/usb.h | 2 +- 3 files changed, 4 insertions(+), 27 deletions(-)
While I agree this is expedient, is there really no other way? How are we going to move to driver model if we add back in the non-driver-model pointers, etc.? We'll end up with a hybrid approach and a real mess.
Is it possible to bug-fix the driver model code (presumably below) instead?
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;elseparent = NULL;if (!parent)return;ttdev = dev_get_parentdata(parent);if (!ttdev->speed != USB_SPEED_HIGH)break;}parent_devnum = ttdev->devnum;-#else ttdev = udev; while (ttdev->parent && ttdev->parent->speed != USB_SPEED_HIGH) ttdev = ttdev->parent; if (!ttdev->parent) return;
parent_devnum = ttdev->parent->devnum;-#endif
qh->qh_endpt2 |= cpu_to_hc32(QH_ENDPT2_PORTNUM(ttdev->portnr) |
QH_ENDPT2_HUBADDR(parent_devnum));
QH_ENDPT2_HUBADDR(ttdev->parent->devnum));}
static int diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c index 95e371d..0157bc6 100644 --- a/drivers/usb/host/usb-uclass.c +++ b/drivers/usb/host/usb-uclass.c @@ -470,7 +470,6 @@ int usb_scan_device(struct udevice *parent, int port, bool created = false; struct usb_dev_platdata *plat; struct usb_bus_priv *priv;
struct usb_device *parent_udev; int ret; ALLOC_CACHE_ALIGN_BUFFER(struct usb_device, udev, 1); struct usb_interface_descriptor *iface = &udev->config.if_desc[0].desc;@@ -515,9 +514,9 @@ int usb_scan_device(struct udevice *parent, int port, udev->devnum = priv->next_addr + 1; udev->portnr = port; debug("Calling usb_setup_device(), portnr=%d\n", udev->portnr);
parent_udev = device_get_uclass_id(parent) == UCLASS_USB_HUB ?
udev->parent = device_get_uclass_id(parent) == UCLASS_USB_HUB ? dev_get_parentdata(parent) : NULL;
ret = usb_setup_device(udev, priv->desc_before_addr, parent_udev, port);
ret = usb_setup_device(udev, priv->desc_before_addr, udev->parent, port); debug("read_descriptor for '%s': ret=%d\n", parent->name, ret); if (ret) return ret;diff --git a/include/usb.h b/include/usb.h index 1b667ff..7876df4 100644 --- a/include/usb.h +++ b/include/usb.h @@ -141,9 +141,9 @@ struct usb_device { int act_len; /* transfered bytes */ int maxchild; /* Number of ports if hub */ int portnr; /* Port number, 1=first */ -#ifndef CONFIG_DM_USB /* parent hub, or NULL if this is the root hub */ struct usb_device *parent; +#ifndef CONFIG_DM_USB struct usb_device *children[USB_MAXCHILDREN]; void *controller; /* hardware controller private data */
#endif
2.3.6
Hi,
On 01-05-15 06:11, Simon Glass wrote:
Hi Hans,
On 30 April 2015 at 08:35, Hans de Goede hdegoede@redhat.com wrote:
When calling into the hcd code in usb_scan_device() we do not yet have the actual udevice for the device we are scanning, so we temporarily set usb_device.dev to the parent.
This means that we cannot use usb_device.dev to accurately determine our place in the usb topology when reading descriptors, and this is necessary to correctly program the usb-2 hub tt settings when a usb-1 device is connected to a usb-2 hub.
This commit (re)adds a direct parent usb_device pointer to struct usb_device so that the usb hcd code can get to the usb_device parent without needing to go through the dm.
This fixes usb-1 devices plugged into usb-2 hubs not working with the driver model.
Signed-off-by: Hans de Goede hdegoede@redhat.com
drivers/usb/host/ehci-hcd.c | 24 +----------------------- drivers/usb/host/usb-uclass.c | 5 ++--- include/usb.h | 2 +- 3 files changed, 4 insertions(+), 27 deletions(-)
While I agree this is expedient, is there really no other way? How are we going to move to driver model if we add back in the non-driver-model pointers, etc.? We'll end up with a hybrid approach and a real mess.
Is it possible to bug-fix the driver model code (presumably below) instead?
That is what I tried first, and failed todo. But after implementing the fix as posted to the list I've been thinking about it some more and I think it should be possible so I'll try again.
Regards,
Hans
diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c index 19f1e29..00c038c 100644 --- a/drivers/usb/host/ehci-hcd.c +++ b/drivers/usb/host/ehci-hcd.c @@ -292,7 +292,6 @@ static void ehci_update_endpt2_dev_n_port(struct usb_device *udev, struct QH *qh) { struct usb_device *ttdev;
int parent_devnum; if (udev->speed != USB_SPEED_LOW && udev->speed != USB_SPEED_FULL) return;@@ -302,35 +301,14 @@ static void ehci_update_endpt2_dev_n_port(struct usb_device *udev, * the tt, so of the first upstream usb-2 hub, there may be usb-1 hubs * in the tree before that one! */ -#ifdef CONFIG_DM_USB
struct udevice *parent;for (ttdev = udev; ; ) {struct udevice *dev = ttdev->dev;if (dev->parent &&device_get_uclass_id(dev->parent) == UCLASS_USB_HUB)parent = dev->parent;elseparent = NULL;if (!parent)return;ttdev = dev_get_parentdata(parent);if (!ttdev->speed != USB_SPEED_HIGH)break;}parent_devnum = ttdev->devnum;-#else ttdev = udev; while (ttdev->parent && ttdev->parent->speed != USB_SPEED_HIGH) ttdev = ttdev->parent; if (!ttdev->parent) return;
parent_devnum = ttdev->parent->devnum;-#endif
qh->qh_endpt2 |= cpu_to_hc32(QH_ENDPT2_PORTNUM(ttdev->portnr) |
QH_ENDPT2_HUBADDR(parent_devnum));
QH_ENDPT2_HUBADDR(ttdev->parent->devnum));}
static int
diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c index 95e371d..0157bc6 100644 --- a/drivers/usb/host/usb-uclass.c +++ b/drivers/usb/host/usb-uclass.c @@ -470,7 +470,6 @@ int usb_scan_device(struct udevice *parent, int port, bool created = false; struct usb_dev_platdata *plat; struct usb_bus_priv *priv;
struct usb_device *parent_udev; int ret; ALLOC_CACHE_ALIGN_BUFFER(struct usb_device, udev, 1); struct usb_interface_descriptor *iface = &udev->config.if_desc[0].desc;@@ -515,9 +514,9 @@ int usb_scan_device(struct udevice *parent, int port, udev->devnum = priv->next_addr + 1; udev->portnr = port; debug("Calling usb_setup_device(), portnr=%d\n", udev->portnr);
parent_udev = device_get_uclass_id(parent) == UCLASS_USB_HUB ?
udev->parent = device_get_uclass_id(parent) == UCLASS_USB_HUB ? dev_get_parentdata(parent) : NULL;
ret = usb_setup_device(udev, priv->desc_before_addr, parent_udev, port);
ret = usb_setup_device(udev, priv->desc_before_addr, udev->parent, port); debug("read_descriptor for '%s': ret=%d\n", parent->name, ret); if (ret) return ret;diff --git a/include/usb.h b/include/usb.h index 1b667ff..7876df4 100644 --- a/include/usb.h +++ b/include/usb.h @@ -141,9 +141,9 @@ struct usb_device { int act_len; /* transfered bytes */ int maxchild; /* Number of ports if hub */ int portnr; /* Port number, 1=first */ -#ifndef CONFIG_DM_USB /* parent hub, or NULL if this is the root hub */ struct usb_device *parent; +#ifndef CONFIG_DM_USB struct usb_device *children[USB_MAXCHILDREN]; void *controller; /* hardware controller private data */
#endif
2.3.6
Without this usb-1 device descriptors do not get read properly.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- drivers/usb/host/ehci-hcd.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c index 00c038c..9fc1e33 100644 --- a/drivers/usb/host/ehci-hcd.c +++ b/drivers/usb/host/ehci-hcd.c @@ -1547,12 +1547,15 @@ int ehci_register(struct udevice *dev, struct ehci_hccr *hccr, struct ehci_hcor *hcor, const struct ehci_ops *ops, uint tweaks, enum usb_init_type init) { + struct usb_bus_priv *priv = dev_get_uclass_priv(dev); struct ehci_ctrl *ctrl = dev_get_priv(dev); int ret;
debug("%s: dev='%s', ctrl=%p, hccr=%p, hcor=%p, init=%d\n", __func__, dev->name, ctrl, hccr, hcor, init);
+ priv->desc_before_addr = true; + ehci_setup_ops(ctrl, ops); ctrl->hccr = hccr; ctrl->hcor = hcor;
On 30 April 2015 at 08:35, Hans de Goede hdegoede@redhat.com wrote:
Without this usb-1 device descriptors do not get read properly.
Signed-off-by: Hans de Goede hdegoede@redhat.com
drivers/usb/host/ehci-hcd.c | 3 +++ 1 file changed, 3 insertions(+)
Acked-by: Simon Glass sjg@chromium.org
diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c index 00c038c..9fc1e33 100644 --- a/drivers/usb/host/ehci-hcd.c +++ b/drivers/usb/host/ehci-hcd.c @@ -1547,12 +1547,15 @@ int ehci_register(struct udevice *dev, struct ehci_hccr *hccr, struct ehci_hcor *hcor, const struct ehci_ops *ops, uint tweaks, enum usb_init_type init) {
struct usb_bus_priv *priv = dev_get_uclass_priv(dev); struct ehci_ctrl *ctrl = dev_get_priv(dev); int ret; debug("%s: dev='%s', ctrl=%p, hccr=%p, hcor=%p, init=%d\n", __func__, dev->name, ctrl, hccr, hcor, init);priv->desc_before_addr = true;ehci_setup_ops(ctrl, ops); ctrl->hccr = hccr; ctrl->hcor = hcor;-- 2.3.6
Interrupt endpoints typically are polled for a long time by the usb controller before they return anything, so calls to submit_int_msg() can take a long time to complete this.
To avoid this the u-boot code has the an interrupt queue mechanism / API, add support for this to the driver-model usb code.
See the added doc comments for more details.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- drivers/usb/host/usb-uclass.c | 36 ++++++++++++++++++++++++++++++++ include/usb.h | 48 ++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 83 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c index 0157bc6..9144f6b 100644 --- a/drivers/usb/host/usb-uclass.c +++ b/drivers/usb/host/usb-uclass.c @@ -65,6 +65,42 @@ int submit_bulk_msg(struct usb_device *udev, unsigned long pipe, void *buffer, return ops->bulk(bus, udev, pipe, buffer, length); }
+struct int_queue *create_int_queue(struct usb_device *udev, + unsigned long pipe, int queuesize, int elementsize, + void *buffer, int interval) +{ + struct udevice *bus = udev->controller_dev; + struct dm_usb_ops *ops = usb_get_ops(bus); + + if (!ops->create_int_queue) + return NULL; + + return ops->create_int_queue(bus, udev, pipe, queuesize, elementsize, + buffer, interval); +} + +void *poll_int_queue(struct usb_device *udev, struct int_queue *queue) +{ + struct udevice *bus = udev->controller_dev; + struct dm_usb_ops *ops = usb_get_ops(bus); + + if (!ops->poll_int_queue) + return NULL; + + return ops->poll_int_queue(bus, udev, queue); +} + +int destroy_int_queue(struct usb_device *udev, struct int_queue *queue) +{ + struct udevice *bus = udev->controller_dev; + struct dm_usb_ops *ops = usb_get_ops(bus); + + if (!ops->destroy_int_queue) + return -ENOSYS; + + return ops->destroy_int_queue(bus, udev, queue); +} + int usb_alloc_device(struct usb_device *udev) { struct udevice *bus = udev->controller_dev; diff --git a/include/usb.h b/include/usb.h index 7876df4..77bdfd9 100644 --- a/include/usb.h +++ b/include/usb.h @@ -198,7 +198,7 @@ int submit_control_msg(struct usb_device *dev, unsigned long pipe, void *buffer, int submit_int_msg(struct usb_device *dev, unsigned long pipe, void *buffer, int transfer_len, int interval);
-#if defined CONFIG_USB_EHCI || defined CONFIG_MUSB_HOST +#if defined CONFIG_USB_EHCI || defined CONFIG_MUSB_HOST || defined(CONFIG_DM_USB) struct int_queue *create_int_queue(struct usb_device *dev, unsigned long pipe, int queuesize, int elementsize, void *buffer, int interval); int destroy_int_queue(struct usb_device *dev, struct int_queue *queue); @@ -654,6 +654,52 @@ struct dm_usb_ops { int (*interrupt)(struct udevice *bus, struct usb_device *udev, unsigned long pipe, void *buffer, int length, int interval); + + /** + * create_int_queue() - Create and queue interrupt packets + * + * Create and queue @queuesize number of interrupt usb packets of + * @elementsize bytes each. @buffer must be atleast @queuesize * + * @elementsize bytes. + * + * Note some controllers only support a queuesize of 1. + * + * @interval: Interrupt interval + * + * @return A pointer to the created interrupt queue or NULL on error + */ + struct int_queue *(*create_int_queue)(struct udevice *bus, + struct usb_device *udev, unsigned long pipe, + int queuesize, int elementsize, void *buffer, + int interval); + + /** + * poll_int_queue() - Poll an interrupt queue for completed packets + * + * Poll an interrupt queue for completed packets. The return value + * points to the part of the buffer passed to create_int_queue() + * corresponding to the completed packet. + * + * @queue: queue to poll + * + * @return Pointer to the data of the first completed packet, or + * NULL if no packets are ready + */ + void *(*poll_int_queue)(struct udevice *bus, struct usb_device *udev, + struct int_queue *queue); + + /** + * destroy_int_queue() - Destroy an interrupt queue + * + * Destroy an interrupt queue created by create_int_queue(). + * + * @queue: queue to poll + * + * @return 0 if OK, -ve on error + */ + int (*destroy_int_queue)(struct udevice *bus, struct usb_device *udev, + struct int_queue *queue); + /** * alloc_device() - Allocate a new device context (XHCI) *
On 30 April 2015 at 08:35, Hans de Goede hdegoede@redhat.com wrote:
Interrupt endpoints typically are polled for a long time by the usb controller before they return anything, so calls to submit_int_msg() can take a long time to complete this.
To avoid this the u-boot code has the an interrupt queue mechanism / API, add support for this to the driver-model usb code.
See the added doc comments for more details.
Signed-off-by: Hans de Goede hdegoede@redhat.com
drivers/usb/host/usb-uclass.c | 36 ++++++++++++++++++++++++++++++++ include/usb.h | 48 ++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 83 insertions(+), 1 deletion(-)
Acked-by: Simon Glass sjg@chromium.org
This is a preparation patch for adding interrupt-queue support to the ehci dm code.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- drivers/usb/host/ehci-hcd.c | 31 +++++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c index 9fc1e33..12145ed 100644 --- a/drivers/usb/host/ehci-hcd.c +++ b/drivers/usb/host/ehci-hcd.c @@ -1220,9 +1220,9 @@ disable_periodic(struct ehci_ctrl *ctrl) return 0; }
-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 *_ehci_create_int_queue(struct usb_device *dev, + unsigned long pipe, int queuesize, int elementsize, + void *buffer, int interval) { struct ehci_ctrl *ctrl = ehci_get_ctrl(dev); struct int_queue *result = NULL; @@ -1378,7 +1378,8 @@ fail1: return NULL; }
-void *poll_int_queue(struct usb_device *dev, struct int_queue *queue) +static void *_ehci_poll_int_queue(struct usb_device *dev, + struct int_queue *queue) { struct QH *cur = queue->current; struct qTD *cur_td; @@ -1413,8 +1414,8 @@ void *poll_int_queue(struct usb_device *dev, struct int_queue *queue) }
/* Do not free buffers associated with QHs, they're owned by someone else */ -int -destroy_int_queue(struct usb_device *dev, struct int_queue *queue) +static int _ehci_destroy_int_queue(struct usb_device *dev, + struct int_queue *queue) { struct ehci_ctrl *ctrl = ehci_get_ctrl(dev); int result = -1; @@ -1515,6 +1516,24 @@ int submit_int_msg(struct usb_device *dev, unsigned long pipe, { return _ehci_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 _ehci_create_int_queue(dev, pipe, queuesize, elementsize, + buffer, interval); +} + +void *poll_int_queue(struct usb_device *dev, struct int_queue *queue) +{ + return _ehci_poll_int_queue(dev, queue); +} + +int destroy_int_queue(struct usb_device *dev, struct int_queue *queue) +{ + return _ehci_destroy_int_queue(dev, queue); +} #endif
#ifdef CONFIG_DM_USB
On 30 April 2015 at 08:35, Hans de Goede hdegoede@redhat.com wrote:
This is a preparation patch for adding interrupt-queue support to the ehci dm code.
Signed-off-by: Hans de Goede hdegoede@redhat.com
drivers/usb/host/ehci-hcd.c | 31 +++++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-)
Acked-by: Simon Glass sjg@chromium.org
Add support for interrupt queues to the dm ehci code.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- drivers/usb/host/ehci-hcd.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+)
diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c index 12145ed..9e8bacb 100644 --- a/drivers/usb/host/ehci-hcd.c +++ b/drivers/usb/host/ehci-hcd.c @@ -1562,6 +1562,29 @@ static int ehci_submit_int_msg(struct udevice *dev, struct usb_device *udev, return _ehci_submit_int_msg(udev, pipe, buffer, length, interval); }
+static struct int_queue *ehci_create_int_queue(struct udevice *dev, + struct usb_device *udev, unsigned long pipe, int queuesize, + int elementsize, void *buffer, int interval) +{ + debug("%s: dev='%s', udev=%p\n", __func__, dev->name, udev); + return _ehci_create_int_queue(udev, pipe, queuesize, elementsize, + buffer, interval); +} + +static void *ehci_poll_int_queue(struct udevice *dev, struct usb_device *udev, + struct int_queue *queue) +{ + debug("%s: dev='%s', udev=%p\n", __func__, dev->name, udev); + return _ehci_poll_int_queue(udev, queue); +} + +static int ehci_destroy_int_queue(struct udevice *dev, struct usb_device *udev, + struct int_queue *queue) +{ + debug("%s: dev='%s', udev=%p\n", __func__, dev->name, udev); + return _ehci_destroy_int_queue(udev, queue); +} + int ehci_register(struct udevice *dev, struct ehci_hccr *hccr, struct ehci_hcor *hcor, const struct ehci_ops *ops, uint tweaks, enum usb_init_type init) @@ -1610,6 +1633,9 @@ struct dm_usb_ops ehci_usb_ops = { .control = ehci_submit_control_msg, .bulk = ehci_submit_bulk_msg, .interrupt = ehci_submit_int_msg, + .create_int_queue = ehci_create_int_queue, + .poll_int_queue = ehci_poll_int_queue, + .destroy_int_queue = ehci_destroy_int_queue, };
#endif
On 30 April 2015 at 08:35, Hans de Goede hdegoede@redhat.com wrote:
Add support for interrupt queues to the dm ehci code.
Signed-off-by: Hans de Goede hdegoede@redhat.com
drivers/usb/host/ehci-hcd.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+)
Acked-by: Simon Glass sjg@chromium.org
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 Hans,
On 30 April 2015 at 08:35, Hans de Goede hdegoede@redhat.com wrote:
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(-)
A few nits, but otherwise:
Acked-by: Simon Glass sjg@chromium.org
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_SUNXIendif 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;
Comment for these two: ?
int ahb_gate_mask;int phy_index;+};
+static int ehci_usb_ofdata_to_platdata(struct udevice *dev) +{
return 0;
Maybe can drop this function if not used? Or do you plan to use it later?
+}
+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,+};
2.3.6
Regards, Simon
Hi,
On 01-05-15 06:12, Simon Glass wrote:
Hi Hans,
On 30 April 2015 at 08:35, Hans de Goede hdegoede@redhat.com wrote:
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(-)
A few nits, but otherwise:
Acked-by: Simon Glass sjg@chromium.org
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;Comment for these two: ?
int ahb_gate_mask;int phy_index;+};
+static int ehci_usb_ofdata_to_platdata(struct udevice *dev) +{
return 0;Maybe can drop this function if not used? Or do you plan to use it later?
Both are fixed in my sunxi tree now.
Regards,
Hans
+}
+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);
setbits_le32(&ccm->ahb_reset0_cfg, 1 << ahb_gate_offset);
setbits_le32(&ccm->ahb_reset0_cfg, priv->ahb_gate_mask);
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;clrbits_le32(&ccm->ahb_reset0_cfg, 1 << ahb_gate_offset);
clrbits_le32(&ccm->ahb_reset0_cfg, priv->ahb_gate_mask);
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,+};
2.3.6
Regards, Simon
On Thu, 2015-04-30 at 16:35 +0200, Hans de Goede wrote:
- if (hccr == (void *)SUNXI_USB1_BASE) {
priv->ahb_gate_mask = 1 << AHB_GATE_OFFSET_USB_EHCI0;priv->phy_index = 1;
Inferring these from the base address is a bit unfortunate, should we not get told this by the DT, or from something higher up?
I have a feeling the answer will be "this can go away when X, Y & X have happened", in which case perhaps a comment to that affect?
BTW I ignored patches 1..7 based on the diffstats, please let me know if I should take a look for some reason.
Ian.
On Sat, 2015-05-02 at 15:03 +0100, Ian Campbell wrote:
I see I missed v2, oh well, this bit of code looks the same AFAICT.
On Thu, 2015-04-30 at 16:35 +0200, Hans de Goede wrote:
- if (hccr == (void *)SUNXI_USB1_BASE) {
priv->ahb_gate_mask = 1 << AHB_GATE_OFFSET_USB_EHCI0;priv->phy_index = 1;Inferring these from the base address is a bit unfortunate, should we not get told this by the DT, or from something higher up?
I have a feeling the answer will be "this can go away when X, Y & X have happened", in which case perhaps a comment to that affect?
BTW I ignored patches 1..7 based on the diffstats, please let me know if I should take a look for some reason.
Ian.
Hi,
On 05/02/2015 04:03 PM, Ian Campbell wrote:
On Thu, 2015-04-30 at 16:35 +0200, Hans de Goede wrote:
- if (hccr == (void *)SUNXI_USB1_BASE) {
priv->ahb_gate_mask = 1 << AHB_GATE_OFFSET_USB_EHCI0;priv->phy_index = 1;Inferring these from the base address is a bit unfortunate, should we not get told this by the DT, or from something higher up?
I have a feeling the answer will be "this can go away when X, Y & X have happened", in which case perhaps a comment to that affect?
Yes getting rid of this this requires us to move to the driver-model for the phys reps. clocks. I'll add a comment to this extend.
Regards,
Hans
Hi,
On 30-04-15 16:35, Hans de Goede wrote:
Hi Simon and Marek,
This series completes my work on converting the sunxi usb/ehci code to the driver model. With this series everything works as it did before, and all the issues I've been seeing when switching to the driver model are resolved.
Please review / merge. I think it would be best to merge this through the dm tree.
Note that the last patch (the actual conversion of the ehci-sunxi.c code) depends on the 6 patch sunxi series starting with this patch:
http://patchwork.ozlabs.org/patch/465369/
So it is probably best if I take 8/8 upstream through the sunxi tree.
So assuming that the patch get a favorable review, the plan would be to merge 1-7 through the dm tree and once that is done I'll take care of the sunxi side if things.
Regards,
Hans
p.s.
Next stop: "Adding dm support to the ohci code and get it to work on sunxi"
Hi Hans,
On 30 April 2015 at 08:35, Hans de Goede hdegoede@redhat.com wrote:
Hi Simon and Marek,
This series completes my work on converting the sunxi usb/ehci code to the driver model. With this series everything works as it did before, and all the issues I've been seeing when switching to the driver model are resolved.
Please review / merge. I think it would be best to merge this through the dm tree.
Great to see this, thanks for all the work and fixes.
I am not too keen on the passing through of struct usb_device, in fact I went to some lengths to avoid that.
I'll read through the patches again, but I wonder if we can avoid heading in that direction? Conceptually in driver model the device does not exist until we find out what it is and can locate a device driver.
Regards, Simon
Hi,
On 30-04-15 16:48, Simon Glass wrote:
Hi Hans,
On 30 April 2015 at 08:35, Hans de Goede hdegoede@redhat.com wrote:
Hi Simon and Marek,
This series completes my work on converting the sunxi usb/ehci code to the driver model. With this series everything works as it did before, and all the issues I've been seeing when switching to the driver model are resolved.
Please review / merge. I think it would be best to merge this through the dm tree.
Great to see this, thanks for all the work and fixes.
I am not too keen on the passing through of struct usb_device, in fact I went to some lengths to avoid that.
I'll read through the patches again, but I wonder if we can avoid heading in that direction? Conceptually in driver model the device does not exist until we find out what it is and can locate a device driver.
Right, the problem is that AFAICT the way the driver-model works is that the driver needs to be known at device creation time, this probably is a result of the auto alloc mem on behalf of the device driver feature.
But for usb this is problematic as we need to read descriptors and whats not before we can find the right driver, and then we do not want to re-read those descriptors and the driver needs access to them too.
What you've been doing sofar is in essence passing through struct usb_device from usb_scan_device to the pre_child_probe method, with the difference that you've been doing it field by field. My patch which just adds the maxpacket size to the fields you're passing (via platdata) also works, but requires re-reading all the descriptors which is slow(ish) and may upset some devices, this can be avoided by stuffing more of usb_device into the plat_data passed from sb_scan_device to the pre_child_probe, but I decided it would be easier to just pass all of the usb_device
Another slightly cleaner (IMHO) option then copying over the entire usb_device would be to dynamically alloc usb_device in usb_scan_device, and keep using the same usb_device all the time, so remove the on stack copy, and do not auto-alloc the per child data for the uclass as it is already allocated manually in usb_scan_device.
This effectively gives us the kernel model of allocating the generic usb_device bits before looking for a driver.
Regards,
Hans
Hi Hans,
On 30 April 2015 at 13:38, Hans de Goede hdegoede@redhat.com wrote:
Hi,
On 30-04-15 16:48, Simon Glass wrote:
Hi Hans,
On 30 April 2015 at 08:35, Hans de Goede hdegoede@redhat.com wrote:
Hi Simon and Marek,
This series completes my work on converting the sunxi usb/ehci code to the driver model. With this series everything works as it did before, and all the issues I've been seeing when switching to the driver model are resolved.
Please review / merge. I think it would be best to merge this through the dm tree.
Great to see this, thanks for all the work and fixes.
I am not too keen on the passing through of struct usb_device, in fact I went to some lengths to avoid that.
I'll read through the patches again, but I wonder if we can avoid heading in that direction? Conceptually in driver model the device does not exist until we find out what it is and can locate a device driver.
Right, the problem is that AFAICT the way the driver-model works is that the driver needs to be known at device creation time, this probably is a result of the auto alloc mem on behalf of the device driver feature.
It's really a design decision.
But for usb this is problematic as we need to read descriptors and whats not before we can find the right driver, and then we do not want to re-read those descriptors and the driver needs access to them too.
What you've been doing sofar is in essence passing through struct usb_device from usb_scan_device to the pre_child_probe method, with the difference that you've been doing it field by field. My patch which just adds the maxpacket size to the fields you're passing (via platdata) also works, but requires re-reading all the descriptors which is slow(ish) and may upset some devices, this can be avoided by stuffing more of usb_device into the plat_data passed from sb_scan_device to the pre_child_probe, but I decided it would be easier to just pass all of the usb_device
It might take a few extra milliseconds (I have not timed it) but given the >1000ms it seems to take to start up USB I don't think it matters. It would be a strange device that refuses to let you read the descriptor twice.
While it is of course easier to pass all of usb_device, I am not keen. It is there not clear which bits are passed through and which are not. If you like we could put the relevant bits in a struct.
Another slightly cleaner (IMHO) option then copying over the entire usb_device would be to dynamically alloc usb_device in usb_scan_device, and keep using the same usb_device all the time, so remove the on stack copy, and do not auto-alloc the per child data for the uclass as it is already allocated manually in usb_scan_device.
But then we have a struct usb_device before a struct udevice. That just cements what I think is wrong with the code.
This effectively gives us the kernel model of allocating the generic usb_device bits before looking for a driver.
I'd be happier with that approach if we actually could split the generic bits into some sort of 'struct urb' or similar. It represents a destination for a request, and the request itself, not the device. I think it is unfortunate that U-Boot conflates the device and the request. About all we need to send a request to a device is its pipe word and packet sizes. The device is a U-Boot concept - we can after all fire requests all over the USB bus without a 'device'.
Regards, Simon
Hi,
On 01-05-15 00:04, Simon Glass wrote:
Hi Hans,
On 30 April 2015 at 13:38, Hans de Goede hdegoede@redhat.com wrote:
Hi,
On 30-04-15 16:48, Simon Glass wrote:
Hi Hans,
On 30 April 2015 at 08:35, Hans de Goede hdegoede@redhat.com wrote:
Hi Simon and Marek,
This series completes my work on converting the sunxi usb/ehci code to the driver model. With this series everything works as it did before, and all the issues I've been seeing when switching to the driver model are resolved.
Please review / merge. I think it would be best to merge this through the dm tree.
Great to see this, thanks for all the work and fixes.
I am not too keen on the passing through of struct usb_device, in fact I went to some lengths to avoid that.
I'll read through the patches again, but I wonder if we can avoid heading in that direction? Conceptually in driver model the device does not exist until we find out what it is and can locate a device driver.
Right, the problem is that AFAICT the way the driver-model works is that the driver needs to be known at device creation time, this probably is a result of the auto alloc mem on behalf of the device driver feature.
It's really a design decision.
TBH I'm not sure if it is the best decision (in hindsight) we may hit similar issues in other cases. Anyways this is how things work now, changing this is going to be a lot of work, so lets just roll with it.
But for usb this is problematic as we need to read descriptors and whats not before we can find the right driver, and then we do not want to re-read those descriptors and the driver needs access to them too.
What you've been doing sofar is in essence passing through struct usb_device from usb_scan_device to the pre_child_probe method, with the difference that you've been doing it field by field. My patch which just adds the maxpacket size to the fields you're passing (via platdata) also works, but requires re-reading all the descriptors which is slow(ish) and may upset some devices, this can be avoided by stuffing more of usb_device into the plat_data passed from sb_scan_device to the pre_child_probe, but I decided it would be easier to just pass all of the usb_device
It might take a few extra milliseconds (I have not timed it) but given the >1000ms it seems to take to start up USB I don't think it matters. It would be a strange device that refuses to let you read the descriptor twice.
I've done a lot of USB work (including redirection of USB devices into virtual machines for qemu) and I've seen stranger devices :)
But I must admit that I cannot actually name an example, it is just that I've a feeling that this may become a problem.
While it is of course easier to pass all of usb_device, I am not keen. It is there not clear which bits are passed through and which are not. If you like we could put the relevant bits in a struct.
Another slightly cleaner (IMHO) option then copying over the entire usb_device would be to dynamically alloc usb_device in usb_scan_device, and keep using the same usb_device all the time, so remove the on stack copy, and do not auto-alloc the per child data for the uclass as it is already allocated manually in usb_scan_device.
But then we have a struct usb_device before a struct udevice. That just cements what I think is wrong with the code.
We already have a struct usb_device before a struct udevice, just because the struct usb_device is hiding on the stack rather then sitting in the heap does not mean it is not there.
This effectively gives us the kernel model of allocating the generic usb_device bits before looking for a driver.
I'd be happier with that approach if we actually could split the generic bits into some sort of 'struct urb' or similar. It represents a destination for a request, and the request itself, not the device. I think it is unfortunate that U-Boot conflates the device and the request. About all we need to send a request to a device is its pipe word and packet sizes. The device is a U-Boot concept - we can after all fire requests all over the USB bus without a 'device'.
We do not just need the usb_device to send usb requests before we have a udevice (which a struct urb would fix) we also need it to store the descriptors read before we've a udevice.
Also see my reply to your review of patch 1/8.
Regards,
Hans
participants (3)
-
Hans de Goede -
Ian Campbell -
Simon Glass