[U-Boot] [PATCH 0/7] usb: ss: Some fixes and cleanup for USB super-speed support

Based on 'u-boot-usb' master branch.
This patch-series includes majorly some clean-up, few fixes and then some basic super-speed usb infrastructure addition, to help put support for XHCI in near future.
Few patches here have been pulled from patch-series sent long back for XHCI support: [RFC PATCH 0/2] USB: XHCI: Add xHCI host controller stack driver
We plan to send further the xHCI controller's stack driver in near future.
Vivek Gautam (7): USB: Some cleanup prior to USB 3.0 interface addition usb: hub: Conditionally power on usb's root-hub ports usb: Update device class in usb device's descriptor usb: hub: Fix enumration timeout usb: hub: Increase device enumeration timeout for broken drives USB: SS: Add support for Super Speed USB interface usb: eth: Fix for updated usb interface descriptor structure
common/cmd_usb.c | 6 +++- common/usb.c | 34 ++++++++++++++++++--------- common/usb_hub.c | 54 +++++++++++++++++++++++++++++++++++-------- common/usb_storage.c | 30 +++++++++++++----------- drivers/usb/eth/asix.c | 14 ++++++----- drivers/usb/eth/smsc95xx.c | 24 +++++++++--------- include/usb.h | 12 +++++++++- include/usb_defs.h | 26 +++++++++++++++++++- 8 files changed, 142 insertions(+), 58 deletions(-)

Some cleanup in usb framework, nothing much on feature side.
Signed-off-by: Vikas C Sajjan vikas.sajjan@samsung.com Signed-off-by: Vivek Gautam gautam.vivek@samsung.com --- common/usb.c | 18 ++++++++++-------- common/usb_storage.c | 30 ++++++++++++++++-------------- include/usb_defs.h | 2 +- 3 files changed, 27 insertions(+), 23 deletions(-)
diff --git a/common/usb.c b/common/usb.c index 6fc0fc1..40c1547 100644 --- a/common/usb.c +++ b/common/usb.c @@ -360,6 +360,7 @@ static int usb_parse_config(struct usb_device *dev, int index, ifno, epno, curr_if_num; int i; u16 ep_wMaxPacketSize; + struct usb_interface *if_desc = NULL;
ifno = -1; epno = -1; @@ -387,23 +388,24 @@ static int usb_parse_config(struct usb_device *dev, &buffer[index])->bInterfaceNumber != curr_if_num) { /* this is a new interface, copy new desc */ ifno = dev->config.no_of_if; + if_desc = &dev->config.if_desc[ifno]; dev->config.no_of_if++; - memcpy(&dev->config.if_desc[ifno], - &buffer[index], buffer[index]); - dev->config.if_desc[ifno].no_of_ep = 0; - dev->config.if_desc[ifno].num_altsetting = 1; + memcpy(if_desc, &buffer[index], buffer[index]); + if_desc->no_of_ep = 0; + if_desc->num_altsetting = 1; curr_if_num = - dev->config.if_desc[ifno].desc.bInterfaceNumber; + if_desc->desc.bInterfaceNumber; } else { /* found alternate setting for the interface */ - dev->config.if_desc[ifno].num_altsetting++; + if_desc->num_altsetting++; } break; case USB_DT_ENDPOINT: epno = dev->config.if_desc[ifno].no_of_ep; + if_desc = &dev->config.if_desc[ifno]; /* found an endpoint */ - dev->config.if_desc[ifno].no_of_ep++; - memcpy(&dev->config.if_desc[ifno].ep_desc[epno], + if_desc->no_of_ep++; + memcpy(&if_desc->ep_desc[epno], &buffer[index], buffer[index]); ep_wMaxPacketSize = get_unaligned(&dev->config.\ if_desc[ifno].\ diff --git a/common/usb_storage.c b/common/usb_storage.c index fb322b4..475c218 100644 --- a/common/usb_storage.c +++ b/common/usb_storage.c @@ -278,10 +278,10 @@ int usb_stor_scan(int mode) lun++) { usb_dev_desc[usb_max_devs].lun = lun; if (usb_stor_get_info(dev, &usb_stor[start], - &usb_dev_desc[usb_max_devs]) == 1) { - usb_max_devs++; - } + &usb_dev_desc[usb_max_devs]) == 1) + usb_max_devs++; } + } /* if storage device */ if (usb_max_devs == USB_MAX_STOR_DEV) { @@ -511,7 +511,7 @@ static int usb_stor_BBB_comdat(ccb *srb, struct us_data *us) dir_in = US_DIRECTION(srb->cmd[0]);
#ifdef BBB_COMDAT_TRACE - printf("dir %d lun %d cmdlen %d cmd %p datalen %d pdata %p\n", + printf("dir %d lun %d cmdlen %d cmd %p datalen %lu pdata %p\n", dir_in, srb->lun, srb->cmdlen, srb->cmd, srb->datalen, srb->pdata); if (srb->cmdlen) { @@ -1218,6 +1218,7 @@ int usb_storage_probe(struct usb_device *dev, unsigned int ifnum, { struct usb_interface *iface; int i; + struct usb_endpoint_descriptor *ep_desc; unsigned int flags = 0;
int protocol = 0; @@ -1300,24 +1301,25 @@ int usb_storage_probe(struct usb_device *dev, unsigned int ifnum, * We will ignore any others. */ for (i = 0; i < iface->desc.bNumEndpoints; i++) { + ep_desc = &iface->ep_desc[i]; /* is it an BULK endpoint? */ - if ((iface->ep_desc[i].bmAttributes & + if ((ep_desc->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK) == USB_ENDPOINT_XFER_BULK) { - if (iface->ep_desc[i].bEndpointAddress & USB_DIR_IN) - ss->ep_in = iface->ep_desc[i].bEndpointAddress & - USB_ENDPOINT_NUMBER_MASK; + if (ep_desc->bEndpointAddress & USB_DIR_IN) + ss->ep_in = ep_desc->bEndpointAddress & + USB_ENDPOINT_NUMBER_MASK; else ss->ep_out = - iface->ep_desc[i].bEndpointAddress & + ep_desc->bEndpointAddress & USB_ENDPOINT_NUMBER_MASK; }
/* is it an interrupt endpoint? */ - if ((iface->ep_desc[i].bmAttributes & - USB_ENDPOINT_XFERTYPE_MASK) == USB_ENDPOINT_XFER_INT) { - ss->ep_int = iface->ep_desc[i].bEndpointAddress & - USB_ENDPOINT_NUMBER_MASK; - ss->irqinterval = iface->ep_desc[i].bInterval; + if ((ep_desc->bmAttributes & + USB_ENDPOINT_XFERTYPE_MASK) == USB_ENDPOINT_XFER_INT) { + ss->ep_int = ep_desc->bEndpointAddress & + USB_ENDPOINT_NUMBER_MASK; + ss->irqinterval = ep_desc->bInterval; } } USB_STOR_PRINTF("Endpoints In %d Out %d Int %d\n", diff --git a/include/usb_defs.h b/include/usb_defs.h index 9502544..0c78d9d 100644 --- a/include/usb_defs.h +++ b/include/usb_defs.h @@ -234,7 +234,7 @@ #define HUB_CHAR_OCPM 0x0018
/* - *Hub Status & Hub Change bit masks + * Hub Status & Hub Change bit masks */ #define HUB_STATUS_LOCAL_POWER 0x0001 #define HUB_STATUS_OVERCURRENT 0x0002

Dear Vivek Gautam,
Some cleanup in usb framework, nothing much on feature side.
Signed-off-by: Vikas C Sajjan vikas.sajjan@samsung.com Signed-off-by: Vivek Gautam gautam.vivek@samsung.com
common/usb.c | 18 ++++++++++-------- common/usb_storage.c | 30 ++++++++++++++++-------------- include/usb_defs.h | 2 +- 3 files changed, 27 insertions(+), 23 deletions(-)
diff --git a/common/usb.c b/common/usb.c index 6fc0fc1..40c1547 100644 --- a/common/usb.c +++ b/common/usb.c @@ -360,6 +360,7 @@ static int usb_parse_config(struct usb_device *dev, int index, ifno, epno, curr_if_num; int i; u16 ep_wMaxPacketSize;
struct usb_interface *if_desc = NULL;
ifno = -1; epno = -1;
@@ -387,23 +388,24 @@ static int usb_parse_config(struct usb_device *dev, &buffer[index])->bInterfaceNumber != curr_if_num) { /* this is a new interface, copy new desc */ ifno = dev->config.no_of_if;
if_desc = &dev->config.if_desc[ifno]; dev->config.no_of_if++;
memcpy(&dev->config.if_desc[ifno],
&buffer[index], buffer[index]);
dev->config.if_desc[ifno].no_of_ep = 0;
dev->config.if_desc[ifno].num_altsetting = 1;
memcpy(if_desc, &buffer[index], buffer[index]);
if_desc->no_of_ep = 0;
if_desc->num_altsetting = 1; curr_if_num =
dev-
config.if_desc[ifno].desc.bInterfaceNumber;
if_desc->desc.bInterfaceNumber; } else { /* found alternate setting for the interface */
dev->config.if_desc[ifno].num_altsetting++;
if_desc->num_altsetting++;
This will crash as if_desc is set in the if() branch above and it will be NULL in the else{} branch.
} break; case USB_DT_ENDPOINT: epno = dev->config.if_desc[ifno].no_of_ep;
if_desc = &dev->config.if_desc[ifno]; /* found an endpoint */
dev->config.if_desc[ifno].no_of_ep++;
memcpy(&dev->config.if_desc[ifno].ep_desc[epno],
if_desc->no_of_ep++;
memcpy(&if_desc->ep_desc[epno], &buffer[index], buffer[index]); ep_wMaxPacketSize = get_unaligned(&dev->config.\ if_desc[ifno].\
diff --git a/common/usb_storage.c b/common/usb_storage.c index fb322b4..475c218 100644 --- a/common/usb_storage.c +++ b/common/usb_storage.c @@ -278,10 +278,10 @@ int usb_stor_scan(int mode) lun++) { usb_dev_desc[usb_max_devs].lun = lun; if (usb_stor_get_info(dev, &usb_stor[start],
&usb_dev_desc[usb_max_devs]) == 1) {
usb_max_devs++;
}
&usb_dev_desc[usb_max_devs]) == 1)
usb_max_devs++; }
What is this change here doing?
} /* if storage device */ if (usb_max_devs == USB_MAX_STOR_DEV) {
[..]
Best regards, Marek Vasut

Hi Marek,
On Thu, Mar 28, 2013 at 7:56 PM, Marek Vasut marex@denx.de wrote:
Dear Vivek Gautam,
Some cleanup in usb framework, nothing much on feature side.
Signed-off-by: Vikas C Sajjan vikas.sajjan@samsung.com Signed-off-by: Vivek Gautam gautam.vivek@samsung.com
common/usb.c | 18 ++++++++++-------- common/usb_storage.c | 30 ++++++++++++++++-------------- include/usb_defs.h | 2 +- 3 files changed, 27 insertions(+), 23 deletions(-)
diff --git a/common/usb.c b/common/usb.c index 6fc0fc1..40c1547 100644 --- a/common/usb.c +++ b/common/usb.c @@ -360,6 +360,7 @@ static int usb_parse_config(struct usb_device *dev, int index, ifno, epno, curr_if_num; int i; u16 ep_wMaxPacketSize;
struct usb_interface *if_desc = NULL; ifno = -1; epno = -1;
@@ -387,23 +388,24 @@ static int usb_parse_config(struct usb_device *dev, &buffer[index])->bInterfaceNumber != curr_if_num) { /* this is a new interface, copy new desc */ ifno = dev->config.no_of_if;
if_desc = &dev->config.if_desc[ifno]; dev->config.no_of_if++;
memcpy(&dev->config.if_desc[ifno],
&buffer[index], buffer[index]);
dev->config.if_desc[ifno].no_of_ep = 0;
dev->config.if_desc[ifno].num_altsetting = 1;
memcpy(if_desc, &buffer[index], buffer[index]);
if_desc->no_of_ep = 0;
if_desc->num_altsetting = 1; curr_if_num =
dev-
config.if_desc[ifno].desc.bInterfaceNumber;
if_desc->desc.bInterfaceNumber; } else { /* found alternate setting for the interface */
dev->config.if_desc[ifno].num_altsetting++;
if_desc->num_altsetting++;
This will crash as if_desc is set in the if() branch above and it will be NULL in the else{} branch.
True, will add here if_desc = &dev->config.if_desc[ifno];
One more thing actually i could notice, "ifno = -1" in this else section. Is this what we want it here ?
} break; case USB_DT_ENDPOINT: epno = dev->config.if_desc[ifno].no_of_ep;
if_desc = &dev->config.if_desc[ifno]; /* found an endpoint */
dev->config.if_desc[ifno].no_of_ep++;
memcpy(&dev->config.if_desc[ifno].ep_desc[epno],
if_desc->no_of_ep++;
memcpy(&if_desc->ep_desc[epno], &buffer[index], buffer[index]); ep_wMaxPacketSize = get_unaligned(&dev->config.\ if_desc[ifno].\
diff --git a/common/usb_storage.c b/common/usb_storage.c index fb322b4..475c218 100644 --- a/common/usb_storage.c +++ b/common/usb_storage.c @@ -278,10 +278,10 @@ int usb_stor_scan(int mode) lun++) { usb_dev_desc[usb_max_devs].lun = lun; if (usb_stor_get_info(dev, &usb_stor[start],
&usb_dev_desc[usb_max_devs]) == 1) {
Do we really need this brace here? We just have one statement under this if.
usb_max_devs++;
}
&usb_dev_desc[usb_max_devs]) == 1)
usb_max_devs++; }
What is this change here doing?
There were these unaligned braces for if part, which we removed.
} /* if storage device */ if (usb_max_devs == USB_MAX_STOR_DEV) {
[..]
Best regards, Marek Vasut _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot

On Tue, Apr 2, 2013 at 11:24 AM, Vivek Gautam gautamvivek1987@gmail.com wrote:
Hi Marek,
On Thu, Mar 28, 2013 at 7:56 PM, Marek Vasut marex@denx.de wrote:
Dear Vivek Gautam,
Some cleanup in usb framework, nothing much on feature side.
Signed-off-by: Vikas C Sajjan vikas.sajjan@samsung.com Signed-off-by: Vivek Gautam gautam.vivek@samsung.com
common/usb.c | 18 ++++++++++-------- common/usb_storage.c | 30 ++++++++++++++++-------------- include/usb_defs.h | 2 +- 3 files changed, 27 insertions(+), 23 deletions(-)
diff --git a/common/usb.c b/common/usb.c index 6fc0fc1..40c1547 100644 --- a/common/usb.c +++ b/common/usb.c @@ -360,6 +360,7 @@ static int usb_parse_config(struct usb_device *dev, int index, ifno, epno, curr_if_num; int i; u16 ep_wMaxPacketSize;
struct usb_interface *if_desc = NULL; ifno = -1; epno = -1;
@@ -387,23 +388,24 @@ static int usb_parse_config(struct usb_device *dev, &buffer[index])->bInterfaceNumber != curr_if_num) { /* this is a new interface, copy new desc */ ifno = dev->config.no_of_if;
if_desc = &dev->config.if_desc[ifno]; dev->config.no_of_if++;
memcpy(&dev->config.if_desc[ifno],
&buffer[index], buffer[index]);
dev->config.if_desc[ifno].no_of_ep = 0;
dev->config.if_desc[ifno].num_altsetting = 1;
memcpy(if_desc, &buffer[index], buffer[index]);
if_desc->no_of_ep = 0;
if_desc->num_altsetting = 1; curr_if_num =
dev-
config.if_desc[ifno].desc.bInterfaceNumber;
if_desc->desc.bInterfaceNumber; } else { /* found alternate setting for the interface */
dev->config.if_desc[ifno].num_altsetting++;
if_desc->num_altsetting++;
This will crash as if_desc is set in the if() branch above and it will be NULL in the else{} branch.
True, will add here if_desc = &dev->config.if_desc[ifno];
One more thing actually i could notice, "ifno = -1" in this else section. Is this what we want it here ?
} break; case USB_DT_ENDPOINT: epno = dev->config.if_desc[ifno].no_of_ep;
if_desc = &dev->config.if_desc[ifno]; /* found an endpoint */
dev->config.if_desc[ifno].no_of_ep++;
memcpy(&dev->config.if_desc[ifno].ep_desc[epno],
if_desc->no_of_ep++;
memcpy(&if_desc->ep_desc[epno], &buffer[index], buffer[index]); ep_wMaxPacketSize = get_unaligned(&dev->config.\ if_desc[ifno].\
diff --git a/common/usb_storage.c b/common/usb_storage.c index fb322b4..475c218 100644 --- a/common/usb_storage.c +++ b/common/usb_storage.c @@ -278,10 +278,10 @@ int usb_stor_scan(int mode) lun++) { usb_dev_desc[usb_max_devs].lun = lun; if (usb_stor_get_info(dev, &usb_stor[start],
&usb_dev_desc[usb_max_devs]) == 1) {
Do we really need this brace here? We just have one statement under this if.
usb_max_devs++;
}
&usb_dev_desc[usb_max_devs]) == 1)
usb_max_devs++; }
What is this change here doing?
There were these unaligned braces for if part, which we removed.
Please ignore above comment. It was meant for change prior to this, above. Will remove this extra line here.
} /* if storage device */ if (usb_max_devs == USB_MAX_STOR_DEV) {
[..]
Best regards, Marek Vasut _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
-- Thanks & Regards Vivek

Dear Vivek Gautam,
Hi Marek,
On Thu, Mar 28, 2013 at 7:56 PM, Marek Vasut marex@denx.de wrote:
Dear Vivek Gautam,
Some cleanup in usb framework, nothing much on feature side.
Signed-off-by: Vikas C Sajjan vikas.sajjan@samsung.com Signed-off-by: Vivek Gautam gautam.vivek@samsung.com
common/usb.c | 18 ++++++++++-------- common/usb_storage.c | 30 ++++++++++++++++-------------- include/usb_defs.h | 2 +- 3 files changed, 27 insertions(+), 23 deletions(-)
diff --git a/common/usb.c b/common/usb.c index 6fc0fc1..40c1547 100644 --- a/common/usb.c +++ b/common/usb.c @@ -360,6 +360,7 @@ static int usb_parse_config(struct usb_device *dev,
int index, ifno, epno, curr_if_num; int i; u16 ep_wMaxPacketSize;
struct usb_interface *if_desc = NULL; ifno = -1; epno = -1;
@@ -387,23 +388,24 @@ static int usb_parse_config(struct usb_device *dev,
&buffer[index])->bInterfaceNumber != curr_if_num) { /* this is a new interface, copy new desc */ ifno = dev->config.no_of_if;
if_desc = &dev->config.if_desc[ifno]; dev->config.no_of_if++;
memcpy(&dev->config.if_desc[ifno],
&buffer[index], buffer[index]);
dev->config.if_desc[ifno].no_of_ep = 0;
dev->config.if_desc[ifno].num_altsetting =
1; + memcpy(if_desc, &buffer[index], buffer[index]); + if_desc->no_of_ep = 0;
if_desc->num_altsetting = 1; curr_if_num =
dev-
config.if_desc[ifno].desc.bInterfaceNumber;
if_desc->desc.bInterfaceNumber; } else { /* found alternate setting for the interface */
dev->config.if_desc[ifno].num_altsetting++; + if_desc->num_altsetting++;
This will crash as if_desc is set in the if() branch above and it will be NULL in the else{} branch.
True, will add here if_desc = &dev->config.if_desc[ifno];
Yea ;-)
One more thing actually i could notice, "ifno = -1" in this else section. Is this what we want it here ?
Maybe I don't see it ?
} break; case USB_DT_ENDPOINT: epno = dev->config.if_desc[ifno].no_of_ep;
if_desc = &dev->config.if_desc[ifno]; /* found an endpoint */
dev->config.if_desc[ifno].no_of_ep++;
memcpy(&dev->config.if_desc[ifno].ep_desc[epno],
if_desc->no_of_ep++;
memcpy(&if_desc->ep_desc[epno], &buffer[index], buffer[index]); ep_wMaxPacketSize = get_unaligned(&dev->config.\ if_desc[ifno].\
diff --git a/common/usb_storage.c b/common/usb_storage.c index fb322b4..475c218 100644 --- a/common/usb_storage.c +++ b/common/usb_storage.c @@ -278,10 +278,10 @@ int usb_stor_scan(int mode)
lun++) { usb_dev_desc[usb_max_devs].lun = lun; if (usb_stor_get_info(dev, &usb_stor[start],
&usb_dev_desc[usb_max_devs]) == 1) {
Do we really need this brace here? We just have one statement under this if.
If the condition is multi-line, then we use brace, yes.
usb_max_devs++;
}
&usb_dev_desc[usb_max_devs]) == 1)
usb_max_devs++; }
What is this change here doing?
There were these unaligned braces for if part, which we removed.
} /* if storage device */ if (usb_max_devs == USB_MAX_STOR_DEV) {
[..]
Best regards, Marek Vasut _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Best regards, Marek Vasut

On Tue, Apr 2, 2013 at 11:56 AM, Marek Vasut marex@denx.de wrote:
Dear Vivek Gautam,
Hi Marek,
On Thu, Mar 28, 2013 at 7:56 PM, Marek Vasut marex@denx.de wrote:
Dear Vivek Gautam,
Some cleanup in usb framework, nothing much on feature side.
Signed-off-by: Vikas C Sajjan vikas.sajjan@samsung.com Signed-off-by: Vivek Gautam gautam.vivek@samsung.com
common/usb.c | 18 ++++++++++-------- common/usb_storage.c | 30 ++++++++++++++++-------------- include/usb_defs.h | 2 +- 3 files changed, 27 insertions(+), 23 deletions(-)
diff --git a/common/usb.c b/common/usb.c index 6fc0fc1..40c1547 100644 --- a/common/usb.c +++ b/common/usb.c @@ -360,6 +360,7 @@ static int usb_parse_config(struct usb_device *dev,
int index, ifno, epno, curr_if_num; int i; u16 ep_wMaxPacketSize;
struct usb_interface *if_desc = NULL; ifno = -1; epno = -1;
@@ -387,23 +388,24 @@ static int usb_parse_config(struct usb_device *dev,
&buffer[index])->bInterfaceNumber != curr_if_num) { /* this is a new interface, copy new desc */ ifno = dev->config.no_of_if;
if_desc = &dev->config.if_desc[ifno]; dev->config.no_of_if++;
memcpy(&dev->config.if_desc[ifno],
&buffer[index], buffer[index]);
dev->config.if_desc[ifno].no_of_ep = 0;
dev->config.if_desc[ifno].num_altsetting =
1; + memcpy(if_desc, &buffer[index], buffer[index]); + if_desc->no_of_ep = 0;
if_desc->num_altsetting = 1; curr_if_num =
dev-
config.if_desc[ifno].desc.bInterfaceNumber;
if_desc->desc.bInterfaceNumber; } else { /* found alternate setting for the interface */
dev->config.if_desc[ifno].num_altsetting++; + if_desc->num_altsetting++;
This will crash as if_desc is set in the if() branch above and it will be NULL in the else{} branch.
True, will add here if_desc = &dev->config.if_desc[ifno];
Yea ;-)
One more thing actually i could notice, "ifno = -1" in this else section. Is this what we want it here ?
Maybe I don't see it ?
ifno is initailized to -1. Now when descriptor is 'Interface type' (case USB_DT_INTERFACE) ifno is update in if part only as below: ifno = dev->config.no_of_if;
This would not be reaching else part, right ?
} break; case USB_DT_ENDPOINT: epno = dev->config.if_desc[ifno].no_of_ep;
if_desc = &dev->config.if_desc[ifno]; /* found an endpoint */
dev->config.if_desc[ifno].no_of_ep++;
memcpy(&dev->config.if_desc[ifno].ep_desc[epno],
if_desc->no_of_ep++;
memcpy(&if_desc->ep_desc[epno], &buffer[index], buffer[index]); ep_wMaxPacketSize = get_unaligned(&dev->config.\ if_desc[ifno].\
diff --git a/common/usb_storage.c b/common/usb_storage.c index fb322b4..475c218 100644 --- a/common/usb_storage.c +++ b/common/usb_storage.c @@ -278,10 +278,10 @@ int usb_stor_scan(int mode)
lun++) { usb_dev_desc[usb_max_devs].lun = lun; if (usb_stor_get_info(dev, &usb_stor[start],
&usb_dev_desc[usb_max_devs]) == 1) {
Do we really need this brace here? We just have one statement under this if.
If the condition is multi-line, then we use brace, yes.
Alright, will amend this. :-)
usb_max_devs++;
}
&usb_dev_desc[usb_max_devs]) == 1)
usb_max_devs++; }
What is this change here doing?
There were these unaligned braces for if part, which we removed.
} /* if storage device */ if (usb_max_devs == USB_MAX_STOR_DEV) {
[..]

Dear Vivek Gautam,
On Tue, Apr 2, 2013 at 11:56 AM, Marek Vasut marex@denx.de wrote:
Dear Vivek Gautam,
Hi Marek,
On Thu, Mar 28, 2013 at 7:56 PM, Marek Vasut marex@denx.de wrote:
Dear Vivek Gautam,
Some cleanup in usb framework, nothing much on feature side.
Signed-off-by: Vikas C Sajjan vikas.sajjan@samsung.com Signed-off-by: Vivek Gautam gautam.vivek@samsung.com
common/usb.c | 18 ++++++++++-------- common/usb_storage.c | 30 ++++++++++++++++-------------- include/usb_defs.h | 2 +- 3 files changed, 27 insertions(+), 23 deletions(-)
diff --git a/common/usb.c b/common/usb.c index 6fc0fc1..40c1547 100644 --- a/common/usb.c +++ b/common/usb.c @@ -360,6 +360,7 @@ static int usb_parse_config(struct usb_device *dev,
int index, ifno, epno, curr_if_num; int i; u16 ep_wMaxPacketSize;
struct usb_interface *if_desc = NULL; ifno = -1; epno = -1;
@@ -387,23 +388,24 @@ static int usb_parse_config(struct usb_device *dev,
&buffer[index])->bInterfaceNumber != curr_if_num) { /* this is a new interface, copy new desc */ ifno = dev->config.no_of_if;
if_desc = &dev->config.if_desc[ifno]; dev->config.no_of_if++;
memcpy(&dev->config.if_desc[ifno],
&buffer[index], buffer[index]);
dev->config.if_desc[ifno].no_of_ep = 0;
dev->config.if_desc[ifno].num_altsetting = 1; + memcpy(if_desc, &buffer[index], buffer[index]); + if_desc->no_of_ep = 0; + if_desc->num_altsetting = 1;
curr_if_num =
dev-
config.if_desc[ifno].desc.bInterfaceNumber;
if_desc->desc.bInterfaceNumber; } else { /* found alternate setting for the interface */
dev->config.if_desc[ifno].num_altsetting++; +
if_desc->num_altsetting++;
This will crash as if_desc is set in the if() branch above and it will be NULL in the else{} branch.
True, will add here if_desc = &dev->config.if_desc[ifno];
Yea ;-)
One more thing actually i could notice, "ifno = -1" in this else section. Is this what we want it here ?
Maybe I don't see it ?
ifno is initailized to -1. Now when descriptor is 'Interface type' (case USB_DT_INTERFACE) ifno is update in if part only as below: ifno = dev->config.no_of_if;
This would not be reaching else part, right ?
Ah I see, yep, indexing the array with -1 is no good. You can add a safety-check there , but the -1 should never reach that place indeed.
Best regards, Marek Vasut

Hi,
On Tue, Apr 2, 2013 at 12:16 PM, Marek Vasut marex@denx.de wrote:
Dear Vivek Gautam,
On Tue, Apr 2, 2013 at 11:56 AM, Marek Vasut marex@denx.de wrote:
Dear Vivek Gautam,
Hi Marek,
On Thu, Mar 28, 2013 at 7:56 PM, Marek Vasut marex@denx.de wrote:
Dear Vivek Gautam,
Some cleanup in usb framework, nothing much on feature side.
Signed-off-by: Vikas C Sajjan vikas.sajjan@samsung.com Signed-off-by: Vivek Gautam gautam.vivek@samsung.com
common/usb.c | 18 ++++++++++-------- common/usb_storage.c | 30 ++++++++++++++++-------------- include/usb_defs.h | 2 +- 3 files changed, 27 insertions(+), 23 deletions(-)
diff --git a/common/usb.c b/common/usb.c index 6fc0fc1..40c1547 100644 --- a/common/usb.c +++ b/common/usb.c @@ -360,6 +360,7 @@ static int usb_parse_config(struct usb_device *dev,
int index, ifno, epno, curr_if_num; int i; u16 ep_wMaxPacketSize;
struct usb_interface *if_desc = NULL; ifno = -1; epno = -1;
@@ -387,23 +388,24 @@ static int usb_parse_config(struct usb_device *dev,
&buffer[index])->bInterfaceNumber != curr_if_num) { /* this is a new interface, copy new desc */ ifno = dev->config.no_of_if;
if_desc = &dev->config.if_desc[ifno]; dev->config.no_of_if++;
memcpy(&dev->config.if_desc[ifno],
&buffer[index], buffer[index]);
dev->config.if_desc[ifno].no_of_ep = 0;
dev->config.if_desc[ifno].num_altsetting = 1; + memcpy(if_desc, &buffer[index], buffer[index]); + if_desc->no_of_ep = 0; + if_desc->num_altsetting = 1;
curr_if_num =
dev-
config.if_desc[ifno].desc.bInterfaceNumber;
if_desc->desc.bInterfaceNumber; } else { /* found alternate setting for the interface */
dev->config.if_desc[ifno].num_altsetting++; +
if_desc->num_altsetting++;
This will crash as if_desc is set in the if() branch above and it will be NULL in the else{} branch.
True, will add here if_desc = &dev->config.if_desc[ifno];
Yea ;-)
One more thing actually i could notice, "ifno = -1" in this else section. Is this what we want it here ?
Maybe I don't see it ?
ifno is initailized to -1. Now when descriptor is 'Interface type' (case USB_DT_INTERFACE) ifno is update in if part only as below: ifno = dev->config.no_of_if;
This would not be reaching else part, right ?
Ah I see, yep, indexing the array with -1 is no good. You can add a safety-check there , but the -1 should never reach that place indeed.
Ok, will amend this as required and update.

Power on root hubs' ports only when they are not yet powered on. Its seen with USB 3.0 ports that they are powered on after a H/W reset, as also reflected in XHCI spec (sec 4.3): "After a Chip Hardware Reset, HCRST, or commanded to the PLS = RxDetect state, all Root Hub ports shall be in Disconnected state, i.e. the port is powered on (PP = 1)"
Signed-off-by: Amar amarendra.xt@samsung.com Signed-off-by: Vivek Gautam gautam.vivek@samsung.com --- common/usb_hub.c | 22 +++++++++++++++++++--- 1 files changed, 19 insertions(+), 3 deletions(-)
diff --git a/common/usb_hub.c b/common/usb_hub.c index b5eeb62..0677004 100644 --- a/common/usb_hub.c +++ b/common/usb_hub.c @@ -111,13 +111,29 @@ static void usb_hub_power_on(struct usb_hub_device *hub) int i; struct usb_device *dev; unsigned pgood_delay = hub->desc.bPwrOn2PwrGood * 2; + ALLOC_CACHE_ALIGN_BUFFER(struct usb_port_status, portsts, 1); + unsigned short portstatus; + int ret;
dev = hub->pusb_dev; - /* Enable power to the ports */ + + /* Enable power to ports whose power is not yet on */ USB_HUB_PRINTF("enabling power on all ports\n"); for (i = 0; i < dev->maxchild; i++) { - usb_set_port_feature(dev, i + 1, USB_PORT_FEAT_POWER); - USB_HUB_PRINTF("port %d returns %lX\n", i + 1, dev->status); + ret = usb_get_port_status(dev, i + 1, portsts); + if (ret < 0) { + USB_HUB_PRINTF("port %d: get_port_status failed\n", + i + 1); + return; + } + + portstatus = le16_to_cpu(portsts->wPortStatus); + + if (!(portstatus & (USB_PORT_STAT_POWER << 1))) { + usb_set_port_feature(dev, i + 1, USB_PORT_FEAT_POWER); + USB_HUB_PRINTF("port %d returns %lX\n", i + 1, + dev->status); + } }
/* Wait at least 100 msec for power to become stable */

Dear Vivek Gautam,
Power on root hubs' ports only when they are not yet powered on. Its seen with USB 3.0 ports that they are powered on after a H/W reset, as also reflected in XHCI spec (sec 4.3): "After a Chip Hardware Reset, HCRST, or commanded to the PLS = RxDetect state, all Root Hub ports shall be in Disconnected state, i.e. the port is powered on (PP = 1)"
Signed-off-by: Amar amarendra.xt@samsung.com Signed-off-by: Vivek Gautam gautam.vivek@samsung.com
Just wondering if we shouldn't power-cycle all the ports instead -- aka. turn them off and then turn all of them on again.
Best regards, Marek Vasut

Hi Marek,
Adding CC: Amarendra
On Thu, Mar 28, 2013 at 7:58 PM, Marek Vasut marex@denx.de wrote:
Dear Vivek Gautam,
Power on root hubs' ports only when they are not yet powered on. Its seen with USB 3.0 ports that they are powered on after a H/W reset, as also reflected in XHCI spec (sec 4.3): "After a Chip Hardware Reset, HCRST, or commanded to the PLS = RxDetect state, all Root Hub ports shall be in Disconnected state, i.e. the port is powered on (PP = 1)"
Signed-off-by: Amar amarendra.xt@samsung.com Signed-off-by: Vivek Gautam gautam.vivek@samsung.com
Just wondering if we shouldn't power-cycle all the ports instead -- aka. turn them off and then turn all of them on again.
Hmm, seems like a fine idea. Will it be fine to power-cycle even EHCI ports too ? Or we just check if ports are already powered-up (as in XHCI) we power-cycle them.

Dear Vivek Gautam,
Hi Marek,
Adding CC: Amarendra
On Thu, Mar 28, 2013 at 7:58 PM, Marek Vasut marex@denx.de wrote:
Dear Vivek Gautam,
Power on root hubs' ports only when they are not yet powered on. Its seen with USB 3.0 ports that they are powered on after a H/W reset, as also reflected in XHCI spec (sec 4.3): "After a Chip Hardware Reset, HCRST, or commanded to the PLS = RxDetect state, all Root Hub ports shall be in Disconnected state, i.e. the port is powered on (PP = 1)"
Signed-off-by: Amar amarendra.xt@samsung.com Signed-off-by: Vivek Gautam gautam.vivek@samsung.com
Just wondering if we shouldn't power-cycle all the ports instead -- aka. turn them off and then turn all of them on again.
Hmm, seems like a fine idea. Will it be fine to power-cycle even EHCI ports too ? Or we just check if ports are already powered-up (as in XHCI) we power-cycle them.
EHCI as well, all of them without differentiating the type. Why not , do you see a problem ?
Best regards, Marek Vasut

Hi,
On Tue, Apr 2, 2013 at 3:39 PM, Marek Vasut marex@denx.de wrote:
Dear Vivek Gautam,
Hi Marek,
Adding CC: Amarendra
On Thu, Mar 28, 2013 at 7:58 PM, Marek Vasut marex@denx.de wrote:
Dear Vivek Gautam,
Power on root hubs' ports only when they are not yet powered on. Its seen with USB 3.0 ports that they are powered on after a H/W reset, as also reflected in XHCI spec (sec 4.3): "After a Chip Hardware Reset, HCRST, or commanded to the PLS = RxDetect state, all Root Hub ports shall be in Disconnected state, i.e. the port is powered on (PP = 1)"
Signed-off-by: Amar amarendra.xt@samsung.com Signed-off-by: Vivek Gautam gautam.vivek@samsung.com
Just wondering if we shouldn't power-cycle all the ports instead -- aka. turn them off and then turn all of them on again.
Hmm, seems like a fine idea. Will it be fine to power-cycle even EHCI ports too ? Or we just check if ports are already powered-up (as in XHCI) we power-cycle them.
EHCI as well, all of them without differentiating the type. Why not , do you see a problem ?
Not of course, will amend this accordingly.

Dear Vivek Gautam,
Hi,
On Tue, Apr 2, 2013 at 3:39 PM, Marek Vasut marex@denx.de wrote:
Dear Vivek Gautam,
Hi Marek,
Adding CC: Amarendra
On Thu, Mar 28, 2013 at 7:58 PM, Marek Vasut marex@denx.de wrote:
Dear Vivek Gautam,
Power on root hubs' ports only when they are not yet powered on. Its seen with USB 3.0 ports that they are powered on after a H/W reset, as also reflected in XHCI spec (sec 4.3): "After a Chip Hardware Reset, HCRST, or commanded to the PLS = RxDetect state, all Root Hub ports shall be in Disconnected state, i.e. the port is powered on (PP = 1)"
Signed-off-by: Amar amarendra.xt@samsung.com Signed-off-by: Vivek Gautam gautam.vivek@samsung.com
Just wondering if we shouldn't power-cycle all the ports instead -- aka. turn them off and then turn all of them on again.
Hmm, seems like a fine idea. Will it be fine to power-cycle even EHCI ports too ? Or we just check if ports are already powered-up (as in XHCI) we power-cycle them.
EHCI as well, all of them without differentiating the type. Why not , do you see a problem ?
Not of course, will amend this accordingly.
Sounds great! Thank you!
Best regards, Marek Vasut

Fetch the device class into usb device's dwcriptors, so that the host controller's driver can use this info to differentiate between HUB and DEVICE.
Signed-off-by: Amar amarendra.xt@samsung.com --- common/usb.c | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/common/usb.c b/common/usb.c index 40c1547..39fcedd 100644 --- a/common/usb.c +++ b/common/usb.c @@ -888,6 +888,11 @@ int usb_new_device(struct usb_device *dev)
dev->descriptor.bMaxPacketSize0 = desc->bMaxPacketSize0;
+ /* Fetch the device class, driver can use this info + * to differentiate between HUB and DEVICE. + */ + dev->descriptor.bDeviceClass = desc->bDeviceClass; + /* find the port number we're at */ if (parent) { int j;

Dear Vivek Gautam,
Fetch the device class into usb device's dwcriptors, so that the host controller's driver can use this info to differentiate between HUB and DEVICE.
Signed-off-by: Amar amarendra.xt@samsung.com
Is this a full name? Otherwise this patch is OK.
common/usb.c | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/common/usb.c b/common/usb.c index 40c1547..39fcedd 100644 --- a/common/usb.c +++ b/common/usb.c @@ -888,6 +888,11 @@ int usb_new_device(struct usb_device *dev)
dev->descriptor.bMaxPacketSize0 = desc->bMaxPacketSize0;
- /* Fetch the device class, driver can use this info
* to differentiate between HUB and DEVICE.
*/
- dev->descriptor.bDeviceClass = desc->bDeviceClass;
- /* find the port number we're at */ if (parent) { int j;
Best regards, Marek Vasut

CC: Amarendra
On Thu, Mar 28, 2013 at 7:58 PM, Marek Vasut marex@denx.de wrote:
Dear Vivek Gautam,
Fetch the device class into usb device's dwcriptors, so that the host controller's driver can use this info to differentiate between HUB and DEVICE.
Signed-off-by: Amar amarendra.xt@samsung.com
Is this a full name? Otherwise this patch is OK.
common/usb.c | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/common/usb.c b/common/usb.c index 40c1547..39fcedd 100644 --- a/common/usb.c +++ b/common/usb.c @@ -888,6 +888,11 @@ int usb_new_device(struct usb_device *dev)
dev->descriptor.bMaxPacketSize0 = desc->bMaxPacketSize0;
/* Fetch the device class, driver can use this info
* to differentiate between HUB and DEVICE.
*/
dev->descriptor.bDeviceClass = desc->bDeviceClass;
/* find the port number we're at */ if (parent) { int j;
Best regards, Marek Vasut _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot

Dear Vivek Gautam,
CC: Amarendra
On Thu, Mar 28, 2013 at 7:58 PM, Marek Vasut marex@denx.de wrote:
Dear Vivek Gautam,
Fetch the device class into usb device's dwcriptors, so that the host controller's driver can use this info to differentiate between HUB and DEVICE.
Signed-off-by: Amar amarendra.xt@samsung.com
Is this a full name? Otherwise this patch is OK.
common/usb.c | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/common/usb.c b/common/usb.c index 40c1547..39fcedd 100644 --- a/common/usb.c +++ b/common/usb.c @@ -888,6 +888,11 @@ int usb_new_device(struct usb_device *dev)
dev->descriptor.bMaxPacketSize0 = desc->bMaxPacketSize0;
/* Fetch the device class, driver can use this info
* to differentiate between HUB and DEVICE.
*/
dev->descriptor.bDeviceClass = desc->bDeviceClass;
btw this really needs fixing, both the comment and the dangling newline.
/* find the port number we're at */ if (parent) { int j;
Best regards, Marek Vasut
Best regards, Marek Vasut

Hi,
On Tue, Apr 2, 2013 at 3:39 PM, Marek Vasut marex@denx.de wrote:
Dear Vivek Gautam,
CC: Amarendra
On Thu, Mar 28, 2013 at 7:58 PM, Marek Vasut marex@denx.de wrote:
Dear Vivek Gautam,
Fetch the device class into usb device's dwcriptors, so that the host controller's driver can use this info to differentiate between HUB and DEVICE.
Signed-off-by: Amar amarendra.xt@samsung.com
Is this a full name? Otherwise this patch is OK.
common/usb.c | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/common/usb.c b/common/usb.c index 40c1547..39fcedd 100644 --- a/common/usb.c +++ b/common/usb.c @@ -888,6 +888,11 @@ int usb_new_device(struct usb_device *dev)
dev->descriptor.bMaxPacketSize0 = desc->bMaxPacketSize0;
/* Fetch the device class, driver can use this info
* to differentiate between HUB and DEVICE.
*/
dev->descriptor.bDeviceClass = desc->bDeviceClass;
btw this really needs fixing, both the comment and the dangling newline.
Sure, will fix this.
/* find the port number we're at */ if (parent) { int j;

Patch b6d7852c increases timeout for enumeration, taking worst case to be 10 sec. get_timer() api returns timestamp in micro-seconds, which is what we are checking in the do-while() loop in usb_hub_configure() (get_timer(start) < CONFIG_SYS_HZ * 10). This should give us a required check for 10 seconds, and thereby we don't need to add additional mdelay of 100 microseconds in each cycle.
Signed-off-by: Vivek Gautam gautam.vivek@samsung.com CC: Vipin Kumar vipin.kumar@st.com --- common/usb_hub.c | 1 - 1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/common/usb_hub.c b/common/usb_hub.c index 0677004..d77f98d 100644 --- a/common/usb_hub.c +++ b/common/usb_hub.c @@ -439,7 +439,6 @@ static int usb_hub_configure(struct usb_device *dev) (portstatus & USB_PORT_STAT_CONNECTION)) break;
- mdelay(100); } while (get_timer(start) < CONFIG_SYS_HZ * 10);
if (ret < 0)

On 3/27/2013 2:59 PM, Vivek Gautam wrote:
Patch b6d7852c increases timeout for enumeration, taking worst case to be 10 sec. get_timer() api returns timestamp in micro-seconds, which is what we are checking in the do-while() loop in usb_hub_configure() (get_timer(start)< CONFIG_SYS_HZ * 10). This should give us a required check for 10 seconds, and thereby we don't need to add additional mdelay of 100 microseconds in each cycle.
Signed-off-by: Vivek Gautamgautam.vivek@samsung.com CC: Vipin Kumarvipin.kumar@st.com
common/usb_hub.c | 1 - 1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/common/usb_hub.c b/common/usb_hub.c index 0677004..d77f98d 100644 --- a/common/usb_hub.c +++ b/common/usb_hub.c @@ -439,7 +439,6 @@ static int usb_hub_configure(struct usb_device *dev) (portstatus& USB_PORT_STAT_CONNECTION)) break;
mdelay(100);
} while (get_timer(start)< CONFIG_SYS_HZ * 10);
if (ret< 0)
With this change, we are continuously reading the uhb status. Although this is also OK, but I feel 100 ms delay is better
Still, there is no harm even if this patch is added. So,
Reviewed-by: Vipin Kumar vipin.kumar@st.com

On Wed, Mar 27, 2013 at 02:59:00PM +0530, Vivek Gautam wrote:
Patch b6d7852c increases timeout for enumeration, taking worst case to be 10 sec. get_timer() api returns timestamp in micro-seconds, which is what we are checking in the do-while() loop in usb_hub_configure() (get_timer(start) < CONFIG_SYS_HZ * 10). This should give us a required check for 10 seconds, and thereby we don't need to add additional mdelay of 100 microseconds in each cycle.
The wording here is not correct. get_timer operates in milliseconds not microseconds.

Hi,
On Tue, Apr 2, 2013 at 1:43 AM, Tom Rini trini@ti.com wrote:
On Wed, Mar 27, 2013 at 02:59:00PM +0530, Vivek Gautam wrote:
Patch b6d7852c increases timeout for enumeration, taking worst case to be 10 sec. get_timer() api returns timestamp in micro-seconds, which is what we are checking in the do-while() loop in usb_hub_configure() (get_timer(start) < CONFIG_SYS_HZ * 10). This should give us a required check for 10 seconds, and thereby we don't need to add additional mdelay of 100 microseconds in each cycle.
The wording here is not correct. get_timer operates in milliseconds not microseconds.
Oops!! Yes of course, my mistake. Thanks
-- Tom
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot

Dear Vivek Gautam,
Hi,
On Tue, Apr 2, 2013 at 1:43 AM, Tom Rini trini@ti.com wrote:
On Wed, Mar 27, 2013 at 02:59:00PM +0530, Vivek Gautam wrote:
Patch b6d7852c increases timeout for enumeration, taking worst case to be 10 sec. get_timer() api returns timestamp in micro-seconds, which is what we are checking in the do-while() loop in usb_hub_configure() (get_timer(start) < CONFIG_SYS_HZ * 10). This should give us a required check for 10 seconds, and thereby we don't need to add additional mdelay of 100 microseconds in each cycle.
The wording here is not correct. get_timer operates in milliseconds not microseconds.
Oops!! Yes of course, my mistake. Thanks
I have the patches queued in usb/next, you can send me a fix and I'll squash it with what's already in there.
Best regards, Marek Vasut

Hi Marek,
On Tue, Apr 2, 2013 at 10:42 AM, Marek Vasut marex@denx.de wrote:
Dear Vivek Gautam,
Hi,
On Tue, Apr 2, 2013 at 1:43 AM, Tom Rini trini@ti.com wrote:
On Wed, Mar 27, 2013 at 02:59:00PM +0530, Vivek Gautam wrote:
Patch b6d7852c increases timeout for enumeration, taking worst case to be 10 sec. get_timer() api returns timestamp in micro-seconds, which is what we are checking in the do-while() loop in usb_hub_configure() (get_timer(start) < CONFIG_SYS_HZ * 10). This should give us a required check for 10 seconds, and thereby we don't need to add additional mdelay of 100 microseconds in each cycle.
The wording here is not correct. get_timer operates in milliseconds not microseconds.
Oops!! Yes of course, my mistake. Thanks
I have the patches queued in usb/next, you can send me a fix and I'll squash it with what's already in there.
Sure, i shall send a fix for this. Should it be good actually to respin the complete patch-series with the changes you have suggested ?? The patches i could see in the 'next' are the originally posted ones.
Best regards, Marek Vasut

On Tue, Apr 2, 2013 at 11:04 AM, Vivek Gautam gautamvivek1987@gmail.com wrote:
Hi Marek,
On Tue, Apr 2, 2013 at 10:42 AM, Marek Vasut marex@denx.de wrote:
Dear Vivek Gautam,
Hi,
On Tue, Apr 2, 2013 at 1:43 AM, Tom Rini trini@ti.com wrote:
On Wed, Mar 27, 2013 at 02:59:00PM +0530, Vivek Gautam wrote:
Patch b6d7852c increases timeout for enumeration, taking worst case to be 10 sec. get_timer() api returns timestamp in micro-seconds, which is what we are checking in the do-while() loop in usb_hub_configure() (get_timer(start) < CONFIG_SYS_HZ * 10). This should give us a required check for 10 seconds, and thereby we don't need to add additional mdelay of 100 microseconds in each cycle.
The wording here is not correct. get_timer operates in milliseconds not microseconds.
Oops!! Yes of course, my mistake. Thanks
I have the patches queued in usb/next, you can send me a fix and I'll squash it with what's already in there.
Sure, i shall send a fix for this. Should it be good actually to respin the complete patch-series with the changes you have suggested ??
I was out on weekend, so couldn't update the patch-series.
The patches i could see in the 'next' are the originally posted ones.
Best regards, Marek Vasut
-- Thanks & Regards Vivek

Dear Vivek Gautam,
On Tue, Apr 2, 2013 at 11:04 AM, Vivek Gautam gautamvivek1987@gmail.com
wrote:
Hi Marek,
On Tue, Apr 2, 2013 at 10:42 AM, Marek Vasut marex@denx.de wrote:
Dear Vivek Gautam,
Hi,
On Tue, Apr 2, 2013 at 1:43 AM, Tom Rini trini@ti.com wrote:
On Wed, Mar 27, 2013 at 02:59:00PM +0530, Vivek Gautam wrote:
Patch b6d7852c increases timeout for enumeration, taking worst case to be 10 sec. get_timer() api returns timestamp in micro-seconds, which is what we are checking in the do-while() loop in usb_hub_configure() (get_timer(start) < CONFIG_SYS_HZ * 10). This should give us a required check for 10 seconds, and thereby we don't need to add additional mdelay of 100 microseconds in each cycle.
The wording here is not correct. get_timer operates in milliseconds not microseconds.
Oops!! Yes of course, my mistake. Thanks
I have the patches queued in usb/next, you can send me a fix and I'll squash it with what's already in there.
Sure, i shall send a fix for this. Should it be good actually to respin the complete patch-series with the changes you have suggested ??
I was out on weekend, so couldn't update the patch-series.
No problem, I'll drop it from next then, please send the whole series.
Thanks!
Best regards, Marek Vasut

Few broken usb mass storage devices can take some time to set Current Connect Status (CCS) and Connect Status Change (CSC) in Port status register after an attach. So increasing some timeout when both CCS and CSC bits are not set.
Signed-off-by: Amar amarendra.xt@samsung.com Signed-off-by: Vivek Gautam gautam.vivek@samsung.com --- common/usb_hub.c | 21 ++++++++++++++++++--- 1 files changed, 18 insertions(+), 3 deletions(-)
diff --git a/common/usb_hub.c b/common/usb_hub.c index d77f98d..8ba7a0d 100644 --- a/common/usb_hub.c +++ b/common/usb_hub.c @@ -416,6 +416,7 @@ static int usb_hub_configure(struct usb_device *dev) unsigned short portstatus, portchange; int ret; ulong start = get_timer(0); + ulong ts = 0;
/* * Wait for (whichever finishes first) @@ -436,9 +437,23 @@ static int usb_hub_configure(struct usb_device *dev) portchange = le16_to_cpu(portsts->wPortChange);
if ((portchange & USB_PORT_STAT_C_CONNECTION) == - (portstatus & USB_PORT_STAT_CONNECTION)) - break; - + (portstatus & USB_PORT_STAT_CONNECTION)) { + /* + * Few brokern pen drives can take some + * time to even report CSC or CCS state after + * an attach. So giving this time lag so that + * the CCS or CSC state can be changed and + * reflected in Port status register. + * XXX: This approach however has a drawback, + * with this we are going to wait for 1 sec + * even for ports on which nothing is connected. + */ + if (ts == 0) + ts = get_timer(0); + if ((portchange & USB_PORT_STAT_C_CONNECTION) || + (get_timer(ts) > CONFIG_SYS_HZ * 1)) + break; + } } while (get_timer(start) < CONFIG_SYS_HZ * 10);
if (ret < 0)

Dear Vivek Gautam,
Few broken usb mass storage devices can take some time to set Current Connect Status (CCS) and Connect Status Change (CSC) in Port status register after an attach. So increasing some timeout when both CCS and CSC bits are not set.
Signed-off-by: Amar amarendra.xt@samsung.com Signed-off-by: Vivek Gautam gautam.vivek@samsung.com
Can we not postpone checking of these CCS and CSC bits for such broken devices? This'd at least allow the "good" devices to be detected quickly and while these are handled, this would give some time for the "bad" ones to do their job too.
Best regards, Marek Vasut

CC: Amarendra
On Thu, Mar 28, 2013 at 8:02 PM, Marek Vasut marex@denx.de wrote:
Dear Vivek Gautam,
Few broken usb mass storage devices can take some time to set Current Connect Status (CCS) and Connect Status Change (CSC) in Port status register after an attach. So increasing some timeout when both CCS and CSC bits are not set.
Signed-off-by: Amar amarendra.xt@samsung.com Signed-off-by: Vivek Gautam gautam.vivek@samsung.com
Can we not postpone checking of these CCS and CSC bits for such broken devices? This'd at least allow the "good" devices to be detected quickly and while these are handled, this would give some time for the "bad" ones to do their job too.
Best regards, Marek Vasut _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot

Hi Marek,
On Thu, Mar 28, 2013 at 8:02 PM, Marek Vasut marex@denx.de wrote:
Dear Vivek Gautam,
Few broken usb mass storage devices can take some time to set Current Connect Status (CCS) and Connect Status Change (CSC) in Port status register after an attach. So increasing some timeout when both CCS and CSC bits are not set.
Signed-off-by: Amar amarendra.xt@samsung.com Signed-off-by: Vivek Gautam gautam.vivek@samsung.com
Can we not postpone checking of these CCS and CSC bits for such broken devices? This'd at least allow the "good" devices to be detected quickly and while these are handled, this would give some time for the "bad" ones to do their job too.
We are thinking of one approach here. Iterating over all the ports once, so that we find 'good' ports and 'bad' ports By 'bad' port we mean that either the device connected to it is bad or nothing is connected to this port. Once we are done with "usb_hub_port_connect_change()" and other jobs for 'good' ports, we iterate once again on bad ports. Not sure how good this approach :-(. Any suggestions ?

Dear Vivek Gautam,
Hi Marek,
On Thu, Mar 28, 2013 at 8:02 PM, Marek Vasut marex@denx.de wrote:
Dear Vivek Gautam,
Few broken usb mass storage devices can take some time to set Current Connect Status (CCS) and Connect Status Change (CSC) in Port status register after an attach. So increasing some timeout when both CCS and CSC bits are not set.
Signed-off-by: Amar amarendra.xt@samsung.com Signed-off-by: Vivek Gautam gautam.vivek@samsung.com
Can we not postpone checking of these CCS and CSC bits for such broken devices? This'd at least allow the "good" devices to be detected quickly and while these are handled, this would give some time for the "bad" ones to do their job too.
We are thinking of one approach here. Iterating over all the ports once, so that we find 'good' ports and 'bad' ports By 'bad' port we mean that either the device connected to it is bad or nothing is connected to this port. Once we are done with "usb_hub_port_connect_change()" and other jobs for 'good' ports, we iterate once again on bad ports. Not sure how good this approach :-(. Any suggestions ?
That might just work, but I wonder if it's worth it. It's a pity we can't make a list of those as they're not even probed yet :(
Best regards, Marek Vasut

Hi Marek,
On Thu, Apr 4, 2013 at 9:28 AM, Marek Vasut marex@denx.de wrote:
Dear Vivek Gautam,
Hi Marek,
On Thu, Mar 28, 2013 at 8:02 PM, Marek Vasut marex@denx.de wrote:
Dear Vivek Gautam,
Few broken usb mass storage devices can take some time to set Current Connect Status (CCS) and Connect Status Change (CSC) in Port status register after an attach. So increasing some timeout when both CCS and CSC bits are not set.
Signed-off-by: Amar amarendra.xt@samsung.com Signed-off-by: Vivek Gautam gautam.vivek@samsung.com
Can we not postpone checking of these CCS and CSC bits for such broken devices? This'd at least allow the "good" devices to be detected quickly and while these are handled, this would give some time for the "bad" ones to do their job too.
We are thinking of one approach here. Iterating over all the ports once, so that we find 'good' ports and 'bad' ports By 'bad' port we mean that either the device connected to it is bad or nothing is connected to this port. Once we are done with "usb_hub_port_connect_change()" and other jobs for 'good' ports, we iterate once again on bad ports. Not sure how good this approach :-(. Any suggestions ?
That might just work, but I wonder if it's worth it. It's a pity we can't make a list of those as they're not even probed yet :(
True, but at this point of time atleast the CSC and CCS bits tell us about whether something is connected or not. Right ? So lets make the ports on which nothing is connected 'bad' and probe them later so that if something is connected to these, job can be done.
If you are fine i may drop this patch from this series for now and come back with a solution. By the time i can update the patch-series.

Dear Vivek Gautam,
Hi Marek,
On Thu, Apr 4, 2013 at 9:28 AM, Marek Vasut marex@denx.de wrote:
Dear Vivek Gautam,
Hi Marek,
On Thu, Mar 28, 2013 at 8:02 PM, Marek Vasut marex@denx.de wrote:
Dear Vivek Gautam,
Few broken usb mass storage devices can take some time to set Current Connect Status (CCS) and Connect Status Change (CSC) in Port status register after an attach. So increasing some timeout when both CCS and CSC bits are not set.
Signed-off-by: Amar amarendra.xt@samsung.com Signed-off-by: Vivek Gautam gautam.vivek@samsung.com
Can we not postpone checking of these CCS and CSC bits for such broken devices? This'd at least allow the "good" devices to be detected quickly and while these are handled, this would give some time for the "bad" ones to do their job too.
We are thinking of one approach here. Iterating over all the ports once, so that we find 'good' ports and 'bad' ports By 'bad' port we mean that either the device connected to it is bad or nothing is connected to this port. Once we are done with "usb_hub_port_connect_change()" and other jobs for 'good' ports, we iterate once again on bad ports. Not sure how good this approach :-(. Any suggestions ?
That might just work, but I wonder if it's worth it. It's a pity we can't make a list of those as they're not even probed yet :(
True, but at this point of time atleast the CSC and CCS bits tell us about whether something is connected or not. Right ? So lets make the ports on which nothing is connected 'bad' and probe them later so that if something is connected to these, job can be done.
If you are fine i may drop this patch from this series for now and come back with a solution. By the time i can update the patch-series.
Sorry for the late reply again. I think the best way around here would be to make the delay configurable, possibly as an argument to "usb reset" with some reasonable default value, what do you think?
Best regards, Marek Vasut

This adds usb framework support for super-speed usb, which will further facilitate to add stack support for xHCI.
Signed-off-by: Vikas C Sajjan vikas.sajjan@samsung.com Signed-off-by: Vivek Gautam gautam.vivek@samsung.com --- common/cmd_usb.c | 6 ++++-- common/usb.c | 11 ++++++++--- common/usb_hub.c | 10 +++++++--- common/usb_storage.c | 2 +- include/usb.h | 12 +++++++++++- include/usb_defs.h | 24 +++++++++++++++++++++++- 6 files changed, 54 insertions(+), 11 deletions(-)
diff --git a/common/cmd_usb.c b/common/cmd_usb.c index dacdc2d..90c2cf1 100644 --- a/common/cmd_usb.c +++ b/common/cmd_usb.c @@ -262,7 +262,7 @@ static void usb_display_config(struct usb_device *dev) ifdesc = &config->if_desc[i]; usb_display_if_desc(&ifdesc->desc, dev); for (ii = 0; ii < ifdesc->no_of_ep; ii++) { - epdesc = &ifdesc->ep_desc[ii]; + epdesc = &ifdesc->ep_desc[ii].ep_desc; usb_display_ep_desc(epdesc); } } @@ -271,7 +271,9 @@ static void usb_display_config(struct usb_device *dev)
static inline char *portspeed(int speed) { - if (speed == USB_SPEED_HIGH) + if (speed == USB_SPEED_SUPER) + return "5 Gb/s"; + else if (speed == USB_SPEED_HIGH) return "480 Mb/s"; else if (speed == USB_SPEED_LOW) return "1.5 Mb/s"; diff --git a/common/usb.c b/common/usb.c index 39fcedd..a5b915e 100644 --- a/common/usb.c +++ b/common/usb.c @@ -304,7 +304,7 @@ usb_set_maxpacket_ep(struct usb_device *dev, int if_idx, int ep_idx) struct usb_endpoint_descriptor *ep; u16 ep_wMaxPacketSize;
- ep = &dev->config.if_desc[if_idx].ep_desc[ep_idx]; + ep = &dev->config.if_desc[if_idx].ep_desc[ep_idx].ep_desc;
b = ep->bEndpointAddress & USB_ENDPOINT_NUMBER_MASK; ep_wMaxPacketSize = get_unaligned(&ep->wMaxPacketSize); @@ -410,14 +410,19 @@ static int usb_parse_config(struct usb_device *dev, ep_wMaxPacketSize = get_unaligned(&dev->config.\ if_desc[ifno].\ ep_desc[epno].\ - wMaxPacketSize); + ep_desc.wMaxPacketSize); put_unaligned(le16_to_cpu(ep_wMaxPacketSize), &dev->config.\ if_desc[ifno].\ ep_desc[epno].\ - wMaxPacketSize); + ep_desc.wMaxPacketSize); USB_PRINTF("if %d, ep %d\n", ifno, epno); break; + case USB_DT_SS_ENDPOINT_COMP: + if_desc = &dev->config.if_desc[ifno]; + memcpy(&(if_desc->ep_desc[epno].ss_ep_comp), + &buffer[index], buffer[index]); + break; default: if (head->bLength == 0) return 1; diff --git a/common/usb_hub.c b/common/usb_hub.c index 8ba7a0d..9acaede 100644 --- a/common/usb_hub.c +++ b/common/usb_hub.c @@ -158,7 +158,9 @@ static struct usb_hub_device *usb_hub_allocate(void)
static inline char *portspeed(int portstatus) { - if (portstatus & (1 << USB_PORT_FEAT_HIGHSPEED)) + if (portstatus & (1 << USB_PORT_FEAT_SUPERSPEED)) + return "5 Gb/s"; + else if (portstatus & (1 << USB_PORT_FEAT_HIGHSPEED)) return "480 Mb/s"; else if (portstatus & (1 << USB_PORT_FEAT_LOWSPEED)) return "1.5 Mb/s"; @@ -262,7 +264,9 @@ void usb_hub_port_connect_change(struct usb_device *dev, int port) /* Allocate a new device struct for it */ usb = usb_alloc_new_device(dev->controller);
- if (portstatus & USB_PORT_STAT_HIGH_SPEED) + if (portstatus & USB_PORT_STAT_SUPER_SPEED) + usb->speed = USB_SPEED_SUPER; + else if (portstatus & USB_PORT_STAT_HIGH_SPEED) usb->speed = USB_SPEED_HIGH; else if (portstatus & USB_PORT_STAT_LOW_SPEED) usb->speed = USB_SPEED_LOW; @@ -525,7 +529,7 @@ int usb_hub_probe(struct usb_device *dev, int ifnum) /* Multiple endpoints? What kind of mutant ninja-hub is this? */ if (iface->desc.bNumEndpoints != 1) return 0; - ep = &iface->ep_desc[0]; + ep = &iface->ep_desc[0].ep_desc; /* Output endpoint? Curiousier and curiousier.. */ if (!(ep->bEndpointAddress & USB_DIR_IN)) return 0; diff --git a/common/usb_storage.c b/common/usb_storage.c index 475c218..c48da43 100644 --- a/common/usb_storage.c +++ b/common/usb_storage.c @@ -1301,7 +1301,7 @@ int usb_storage_probe(struct usb_device *dev, unsigned int ifnum, * We will ignore any others. */ for (i = 0; i < iface->desc.bNumEndpoints; i++) { - ep_desc = &iface->ep_desc[i]; + ep_desc = &iface->ep_desc[i].ep_desc; /* is it an BULK endpoint? */ if ((ep_desc->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK) == USB_ENDPOINT_XFER_BULK) { diff --git a/include/usb.h b/include/usb.h index d79c865..1d832ca 100644 --- a/include/usb.h +++ b/include/usb.h @@ -67,6 +67,16 @@ struct devrequest { unsigned short length; } __attribute__ ((packed));
+struct usb_ep_desc { + struct usb_endpoint_descriptor ep_desc; + /* + * Super Speed Device will have Super Speed Endpoint + * Companion Descriptor (section 9.6.7 of usb 3.0 spec) + * Revision 1.0 June 6th 2011 + */ + struct usb_ss_ep_comp_descriptor ss_ep_comp; +}; + /* Interface */ struct usb_interface { struct usb_interface_descriptor desc; @@ -75,7 +85,7 @@ struct usb_interface { unsigned char num_altsetting; unsigned char act_altsetting;
- struct usb_endpoint_descriptor ep_desc[USB_MAXENDPOINTS]; + struct usb_ep_desc ep_desc[USB_MAXENDPOINTS]; } __attribute__ ((packed));
/* Configuration information.. */ diff --git a/include/usb_defs.h b/include/usb_defs.h index 0c78d9d..e2aaef3 100644 --- a/include/usb_defs.h +++ b/include/usb_defs.h @@ -203,6 +203,8 @@ #define USB_PORT_FEAT_POWER 8 #define USB_PORT_FEAT_LOWSPEED 9 #define USB_PORT_FEAT_HIGHSPEED 10 +#define USB_PORT_FEAT_FULLSPEED 11 +#define USB_PORT_FEAT_SUPERSPEED 12 #define USB_PORT_FEAT_C_CONNECTION 16 #define USB_PORT_FEAT_C_ENABLE 17 #define USB_PORT_FEAT_C_SUSPEND 18 @@ -218,8 +220,20 @@ #define USB_PORT_STAT_POWER 0x0100 #define USB_PORT_STAT_LOW_SPEED 0x0200 #define USB_PORT_STAT_HIGH_SPEED 0x0400 /* support for EHCI */ +#define USB_PORT_STAT_FULL_SPEED 0x0800 +#define USB_PORT_STAT_SUPER_SPEED 0x1000 /* support for XHCI */ #define USB_PORT_STAT_SPEED \ - (USB_PORT_STAT_LOW_SPEED | USB_PORT_STAT_HIGH_SPEED) + (USB_PORT_STAT_LOW_SPEED | USB_PORT_STAT_HIGH_SPEED | \ + USB_PORT_STAT_FULL_SPEED | USB_PORT_STAT_SUPER_SPEED) + +/* + * Additions to wPortStatus bit field from USB 3.0 + * See USB 3.0 spec Table 10-10 + */ +#define USB_PORT_STAT_LINK_STATE 0x01e0 +#define USB_SS_PORT_STAT_POWER 0x0200 +#define USB_SS_PORT_STAT_SPEED 0x1c00 +#define USB_PORT_STAT_SPEED_5GBPS 0x0000
/* wPortChange bits */ #define USB_PORT_STAT_C_CONNECTION 0x0001 @@ -228,6 +242,14 @@ #define USB_PORT_STAT_C_OVERCURRENT 0x0008 #define USB_PORT_STAT_C_RESET 0x0010
+/* + * Addition to wPortChange bit fields form USB 3.0 + * See USB 3.0 spec Table 10-11 + */ +#define USB_PORT_STAT_C_BH_RESET 0x0020 +#define USB_PORT_STAT_C_LINK_STATE 0x0040 +#define USB_PORT_STAT_C_CONFIG_ERROR 0x0080 + /* wHubCharacteristics (masks) */ #define HUB_CHAR_LPSM 0x0003 #define HUB_CHAR_COMPOUND 0x0004

Dear Vivek Gautam,
This adds usb framework support for super-speed usb, which will further facilitate to add stack support for xHCI.
Signed-off-by: Vikas C Sajjan vikas.sajjan@samsung.com Signed-off-by: Vivek Gautam gautam.vivek@samsung.com
[...]
--- a/include/usb.h +++ b/include/usb.h @@ -67,6 +67,16 @@ struct devrequest { unsigned short length; } __attribute__ ((packed));
+struct usb_ep_desc {
- struct usb_endpoint_descriptor ep_desc;
- /*
* Super Speed Device will have Super Speed Endpoint
* Companion Descriptor (section 9.6.7 of usb 3.0 spec)
* Revision 1.0 June 6th 2011
*/
- struct usb_ss_ep_comp_descriptor ss_ep_comp;
+};
/* Interface */ struct usb_interface { struct usb_interface_descriptor desc; @@ -75,7 +85,7 @@ struct usb_interface { unsigned char num_altsetting; unsigned char act_altsetting;
- struct usb_endpoint_descriptor ep_desc[USB_MAXENDPOINTS];
- struct usb_ep_desc ep_desc[USB_MAXENDPOINTS];
Do we really need this struct usb_ep_desc? Can we not just store the usb_ss_ep_comp_descriptor here as well?
} __attribute__ ((packed));
/* Configuration information.. */ diff --git a/include/usb_defs.h b/include/usb_defs.h index 0c78d9d..e2aaef3 100644 --- a/include/usb_defs.h +++ b/include/usb_defs.h @@ -203,6 +203,8 @@ #define USB_PORT_FEAT_POWER 8 #define USB_PORT_FEAT_LOWSPEED 9 #define USB_PORT_FEAT_HIGHSPEED 10 +#define USB_PORT_FEAT_FULLSPEED 11 +#define USB_PORT_FEAT_SUPERSPEED 12 #define USB_PORT_FEAT_C_CONNECTION 16 #define USB_PORT_FEAT_C_ENABLE 17 #define USB_PORT_FEAT_C_SUSPEND 18 @@ -218,8 +220,20 @@ #define USB_PORT_STAT_POWER 0x0100 #define USB_PORT_STAT_LOW_SPEED 0x0200 #define USB_PORT_STAT_HIGH_SPEED 0x0400 /* support for EHCI */ +#define USB_PORT_STAT_FULL_SPEED 0x0800 +#define USB_PORT_STAT_SUPER_SPEED 0x1000 /* support for XHCI */ #define USB_PORT_STAT_SPEED \
- (USB_PORT_STAT_LOW_SPEED | USB_PORT_STAT_HIGH_SPEED)
- (USB_PORT_STAT_LOW_SPEED | USB_PORT_STAT_HIGH_SPEED | \
- USB_PORT_STAT_FULL_SPEED | USB_PORT_STAT_SUPER_SPEED)
+/*
- Additions to wPortStatus bit field from USB 3.0
- See USB 3.0 spec Table 10-10
- */
+#define USB_PORT_STAT_LINK_STATE 0x01e0 +#define USB_SS_PORT_STAT_POWER 0x0200 +#define USB_SS_PORT_STAT_SPEED 0x1c00 +#define USB_PORT_STAT_SPEED_5GBPS 0x0000
/* wPortChange bits */ #define USB_PORT_STAT_C_CONNECTION 0x0001 @@ -228,6 +242,14 @@ #define USB_PORT_STAT_C_OVERCURRENT 0x0008 #define USB_PORT_STAT_C_RESET 0x0010
+/*
- Addition to wPortChange bit fields form USB 3.0
- See USB 3.0 spec Table 10-11
- */
+#define USB_PORT_STAT_C_BH_RESET 0x0020 +#define USB_PORT_STAT_C_LINK_STATE 0x0040 +#define USB_PORT_STAT_C_CONFIG_ERROR 0x0080
/* wHubCharacteristics (masks) */ #define HUB_CHAR_LPSM 0x0003 #define HUB_CHAR_COMPOUND 0x0004

Hi Marek,
On Thu, Mar 28, 2013 at 8:05 PM, Marek Vasut marex@denx.de wrote:
Dear Vivek Gautam,
This adds usb framework support for super-speed usb, which will further facilitate to add stack support for xHCI.
Signed-off-by: Vikas C Sajjan vikas.sajjan@samsung.com Signed-off-by: Vivek Gautam gautam.vivek@samsung.com
[...]
--- a/include/usb.h +++ b/include/usb.h @@ -67,6 +67,16 @@ struct devrequest { unsigned short length; } __attribute__ ((packed));
+struct usb_ep_desc {
struct usb_endpoint_descriptor ep_desc;
/*
* Super Speed Device will have Super Speed Endpoint
* Companion Descriptor (section 9.6.7 of usb 3.0 spec)
* Revision 1.0 June 6th 2011
*/
struct usb_ss_ep_comp_descriptor ss_ep_comp;
+};
/* Interface */ struct usb_interface { struct usb_interface_descriptor desc; @@ -75,7 +85,7 @@ struct usb_interface { unsigned char num_altsetting; unsigned char act_altsetting;
struct usb_endpoint_descriptor ep_desc[USB_MAXENDPOINTS];
struct usb_ep_desc ep_desc[USB_MAXENDPOINTS];
Do we really need this struct usb_ep_desc? Can we not just store the usb_ss_ep_comp_descriptor here as well?
Yea, possibly we can add "usb_ss_ep_comp_descriptor" in 'usb_interface' itself only. Also quoting USB 3.0 specs: "SuperSpeed devices shall return Endpoint Companion descriptors for each of the endpoints in that interface to return additional information about its endpoint capabilities. The Endpoint Companion descriptor shall immediately follow the endpoint descriptor it is associated with in the configuration information."
Will update this accordingly.
} __attribute__ ((packed));
/* Configuration information.. */ diff --git a/include/usb_defs.h b/include/usb_defs.h index 0c78d9d..e2aaef3 100644 --- a/include/usb_defs.h +++ b/include/usb_defs.h @@ -203,6 +203,8 @@ #define USB_PORT_FEAT_POWER 8 #define USB_PORT_FEAT_LOWSPEED 9 #define USB_PORT_FEAT_HIGHSPEED 10 +#define USB_PORT_FEAT_FULLSPEED 11 +#define USB_PORT_FEAT_SUPERSPEED 12 #define USB_PORT_FEAT_C_CONNECTION 16 #define USB_PORT_FEAT_C_ENABLE 17 #define USB_PORT_FEAT_C_SUSPEND 18 @@ -218,8 +220,20 @@ #define USB_PORT_STAT_POWER 0x0100 #define USB_PORT_STAT_LOW_SPEED 0x0200 #define USB_PORT_STAT_HIGH_SPEED 0x0400 /* support for EHCI */ +#define USB_PORT_STAT_FULL_SPEED 0x0800 +#define USB_PORT_STAT_SUPER_SPEED 0x1000 /* support for XHCI */ #define USB_PORT_STAT_SPEED \
(USB_PORT_STAT_LOW_SPEED | USB_PORT_STAT_HIGH_SPEED)
(USB_PORT_STAT_LOW_SPEED | USB_PORT_STAT_HIGH_SPEED | \
USB_PORT_STAT_FULL_SPEED | USB_PORT_STAT_SUPER_SPEED)
+/*
- Additions to wPortStatus bit field from USB 3.0
- See USB 3.0 spec Table 10-10
- */
+#define USB_PORT_STAT_LINK_STATE 0x01e0 +#define USB_SS_PORT_STAT_POWER 0x0200 +#define USB_SS_PORT_STAT_SPEED 0x1c00 +#define USB_PORT_STAT_SPEED_5GBPS 0x0000
/* wPortChange bits */ #define USB_PORT_STAT_C_CONNECTION 0x0001 @@ -228,6 +242,14 @@ #define USB_PORT_STAT_C_OVERCURRENT 0x0008 #define USB_PORT_STAT_C_RESET 0x0010
+/*
- Addition to wPortChange bit fields form USB 3.0
- See USB 3.0 spec Table 10-11
- */
+#define USB_PORT_STAT_C_BH_RESET 0x0020 +#define USB_PORT_STAT_C_LINK_STATE 0x0040 +#define USB_PORT_STAT_C_CONFIG_ERROR 0x0080
/* wHubCharacteristics (masks) */ #define HUB_CHAR_LPSM 0x0003 #define HUB_CHAR_COMPOUND 0x0004

Now usb interface structure doesn't contain the Ep descriptor directly, in fact it contains a pointer to 'usb_ep_desc' structure which contains the Ep descriptor. So updating the usb-eth drivers accordingly.
Signed-off-by: Vivek Gautam gautam.vivek@samsung.com --- drivers/usb/eth/asix.c | 14 ++++++++------ drivers/usb/eth/smsc95xx.c | 24 ++++++++++++------------ 2 files changed, 20 insertions(+), 18 deletions(-)
diff --git a/drivers/usb/eth/asix.c b/drivers/usb/eth/asix.c index 75ec8f7..42e98f1 100644 --- a/drivers/usb/eth/asix.c +++ b/drivers/usb/eth/asix.c @@ -611,6 +611,7 @@ int asix_eth_probe(struct usb_device *dev, unsigned int ifnum, { struct usb_interface *iface; struct usb_interface_descriptor *iface_desc; + struct usb_endpoint_descriptor *ep_desc; int ep_in_found = 0, ep_out_found = 0; int i;
@@ -652,10 +653,11 @@ int asix_eth_probe(struct usb_device *dev, unsigned int ifnum, * int. We will ignore any others. */ for (i = 0; i < iface_desc->bNumEndpoints; i++) { + ep_desc = &iface->ep_desc[i].ep_desc; /* is it an BULK endpoint? */ - if ((iface->ep_desc[i].bmAttributes & + if ((ep_desc->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK) == USB_ENDPOINT_XFER_BULK) { - u8 ep_addr = iface->ep_desc[i].bEndpointAddress; + u8 ep_addr = ep_desc->bEndpointAddress; if (ep_addr & USB_DIR_IN) { if (!ep_in_found) { ss->ep_in = ep_addr & @@ -672,11 +674,11 @@ int asix_eth_probe(struct usb_device *dev, unsigned int ifnum, }
/* is it an interrupt endpoint? */ - if ((iface->ep_desc[i].bmAttributes & + if ((ep_desc->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK) == USB_ENDPOINT_XFER_INT) { - ss->ep_int = iface->ep_desc[i].bEndpointAddress & - USB_ENDPOINT_NUMBER_MASK; - ss->irqinterval = iface->ep_desc[i].bInterval; + ss->ep_int = ep_desc->bEndpointAddress & + USB_ENDPOINT_NUMBER_MASK; + ss->irqinterval = ep_desc->bInterval; } } debug("Endpoints In %d Out %d Int %d\n", diff --git a/drivers/usb/eth/smsc95xx.c b/drivers/usb/eth/smsc95xx.c index fd8f8a7..3f58c89 100644 --- a/drivers/usb/eth/smsc95xx.c +++ b/drivers/usb/eth/smsc95xx.c @@ -807,6 +807,7 @@ int smsc95xx_eth_probe(struct usb_device *dev, unsigned int ifnum, { struct usb_interface *iface; struct usb_interface_descriptor *iface_desc; + struct usb_endpoint_descriptor *ep_desc; int i;
/* let's examine the device now */ @@ -837,25 +838,24 @@ int smsc95xx_eth_probe(struct usb_device *dev, unsigned int ifnum, * We will ignore any others. */ for (i = 0; i < iface_desc->bNumEndpoints; i++) { + ep_desc = &iface->ep_desc[i].ep_desc; /* is it an BULK endpoint? */ - if ((iface->ep_desc[i].bmAttributes & + if ((ep_desc->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK) == USB_ENDPOINT_XFER_BULK) { - if (iface->ep_desc[i].bEndpointAddress & USB_DIR_IN) - ss->ep_in = - iface->ep_desc[i].bEndpointAddress & - USB_ENDPOINT_NUMBER_MASK; + if (ep_desc->bEndpointAddress & USB_DIR_IN) + ss->ep_in = ep_desc->bEndpointAddress & + USB_ENDPOINT_NUMBER_MASK; else - ss->ep_out = - iface->ep_desc[i].bEndpointAddress & - USB_ENDPOINT_NUMBER_MASK; + ss->ep_out = ep_desc->bEndpointAddress & + USB_ENDPOINT_NUMBER_MASK; }
/* is it an interrupt endpoint? */ - if ((iface->ep_desc[i].bmAttributes & + if ((ep_desc->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK) == USB_ENDPOINT_XFER_INT) { - ss->ep_int = iface->ep_desc[i].bEndpointAddress & - USB_ENDPOINT_NUMBER_MASK; - ss->irqinterval = iface->ep_desc[i].bInterval; + ss->ep_int = ep_desc->bEndpointAddress & + USB_ENDPOINT_NUMBER_MASK; + ss->irqinterval = ep_desc->bInterval; } } debug("Endpoints In %d Out %d Int %d\n",
participants (5)
-
Marek Vasut
-
Tom Rini
-
Vipin Kumar
-
Vivek Gautam
-
Vivek Gautam