| Message ID | 1350989687-32102-2-git-send-email-gautam.vivek@samsung.com |
|---|---|
| State | RFC |
| Delegated to: | Marek Vasut |
| Headers | show |
Dear Marek, CCing Simon Glass. On Tue, Oct 23, 2012 at 5:10 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> >> Signedoff-by: Vivek Gautam <gautam.vivek@samsung.com> > > CCing Benoit, (help me! ;-) ) > > I'll be blunt and technical below, try not to take it personally please. > >> --- >> common/cmd_usb.c | 6 +- >> common/usb.c | 41 ++++++++- >> common/usb_hub.c | 26 +++++- >> common/usb_storage.c | 35 +++++---- >> include/common.h | 2 + >> include/linux/usb/ch9.h | 2 +- >> include/usb.h | 15 +++- >> include/usb_defs.h | 26 ++++++- >> include/usbdescriptors.h | 201 >> ++++++++++++++++++++++++++++++++++++++++++++++ 9 files changed, 322 >> insertions(+), 32 deletions(-) >> >> diff --git a/common/cmd_usb.c b/common/cmd_usb.c >> index c128455..013b2e8 100644 >> --- a/common/cmd_usb.c >> +++ b/common/cmd_usb.c >> @@ -262,7 +262,7 @@ 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; > > Fix, split into separate patch > Sure, will split these fixes in a separate patch. >> usb_display_ep_desc(epdesc); >> } >> } >> @@ -271,7 +271,9 @@ 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"; > > Can you convert this into switch please? > Sure, will make a switch for this. That whould be better. >> diff --git a/common/usb.c b/common/usb.c >> index 50b8175..691d9ac 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); >> @@ -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; >> @@ -401,21 +402,27 @@ static int usb_parse_config(struct usb_device *dev, >> 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].ep_desc, > > This change is irrelevant, it's a fix so put it into separate patch with proper > explanation. > Shall put all these fixes in a separate patch. >> &buffer[index], buffer[index]); >> ep_wMaxPacketSize = get_unaligned(&dev->config.\ >> if_desc[ifno].\ >> ep_desc[epno].\ >> - wMaxPacketSize); >> + ep_desc.wMaxPacketSize); > > What does this change do/fix ? > This comes as an affect of introduction of "struct usb_ep_desc" which eventually contains "struct usb_endpoint_descriptor" and "struct usb_ss_ep_comp_descriptor". >> 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), > > Do you need these braces in &(...) ? > True, we can remove these braces. >> + &buffer[index], buffer[index]); >> + break; >> default: >> if (head->bLength == 0) >> return 1; >> @@ -659,6 +666,18 @@ static int usb_get_string(struct usb_device *dev, >> unsigned short langid, return result; >> } >> >> +/* Allocate usb device */ >> +int usb_alloc_dev(struct usb_device *dev) >> +{ >> + int res; >> + >> + USB_PRINTF("alloc device\n"); > > this is a debug print. > Isn't USB_PRINTF itself a conditional debug print ? >> + res = usb_control_msg(dev, usb_sndctrlpipe(dev->parent, 0), >> + USB_REQ_ALLOC_DEV, 0, 0, 0, >> + NULL, 0, USB_CNTL_TIMEOUT); > > > How does this "allocate" a device? Please, do a proper documentation. Actually, > take a look at include/linker_lists.h > > Or see here: > http://git.denx.de/?p=u- > boot.git;a=blob;f=include/linker_lists.h;h=0b405d78ea34df1c528fbc4e24ed2aad756ac4a2;hb=HEAD > > And here (U-Boot Code Documentation): > http://www.denx.de/wiki/U-Boot/CodingStyle > > It'd be really nice if you could follow such pattern of documentation! > Yes, thanks for pointing out. Will document it properly to make things more understandable. >> + return res; >> +} >> >> static void usb_try_string_workarounds(unsigned char *buf, int *length) >> { >> @@ -852,7 +871,10 @@ int usb_new_device(struct usb_device *dev) >> struct usb_device_descriptor *desc; >> int port = -1; >> struct usb_device *parent = dev->parent; >> + >> +#ifndef CONFIG_USB_XHCI >> unsigned short portstatus; > > Use __maybe_unused instead so it's not poluted with these #ifdefs ? > Right. will change this accordingly. >> +#endif >> >> /* send 64-byte GET-DEVICE-DESCRIPTOR request. Since the descriptor is >> * only 18 bytes long, this will terminate with a short packet. But if >> @@ -889,12 +911,21 @@ int usb_new_device(struct usb_device *dev) >> return 1; >> } >> >> + /* >> + * Putting up the code here for slot assignment and initialization: >> + * xHCI spec sec 4.3.2, 4.3.3 >> + */ > > Misaligned comment? > Sure, will correct it. >> +#ifdef CONFIG_USB_XHCI >> + /* Call if and only if device is attached and detected */ >> + usb_alloc_dev(dev); >> +#else >> /* reset the port for the second time */ >> err = hub_port_reset(dev->parent, port, &portstatus); >> if (err < 0) { >> printf("\n Couldn't reset port %i\n", port); >> return 1; >> } >> +#endif >> } >> #endif >> >> diff --git a/common/usb_hub.c b/common/usb_hub.c >> index e4a1201..8a00894 100644 >> --- a/common/usb_hub.c >> +++ b/common/usb_hub.c >> @@ -204,7 +204,12 @@ int hub_port_reset(struct usb_device *dev, int port, >> } >> >> >> -void usb_hub_port_connect_change(struct usb_device *dev, int port) >> +/* >> + * Adding the flag do_port_reset: USB 2.0 device requires port reset >> + * while USB 3.0 device does not. >> + */ >> +void usb_hub_port_connect_change(struct usb_device *dev, int port, >> + int do_port_reset) > > Make it uint32_t flags so once usb4.0 (or MS's magic USB port where you can > connect joystick :D ) arrives and it needs two and half resets of a port, we > just add another flag. > Ok, will do this. >> { >> struct usb_device *usb; >> ALLOC_CACHE_ALIGN_BUFFER(struct usb_port_status, portsts, 1); >> @@ -235,11 +240,21 @@ void usb_hub_port_connect_change(struct usb_device >> *dev, int port) } >> mdelay(200); >> >> +#ifdef CONFIG_USB_XHCI >> + /* Reset the port */ >> + if (do_port_reset) { >> + if (hub_port_reset(dev, port, &portstatus) < 0) { >> + printf("cannot reset port %i!?\n", port + 1); >> + return; >> + } >> + } >> +#else >> /* Reset the port */ >> if (hub_port_reset(dev, port, &portstatus) < 0) { >> printf("cannot reset port %i!?\n", port + 1); >> return; >> } >> +#endif > > I'm sure > > #ifdef ... > do_port_reset = 1 > #endif > > would work just fine ... then you'd avoid such massive blocks of ifdef'd code. > Will try to fix this. >> mdelay(200); >> >> @@ -409,7 +424,10 @@ static int usb_hub_configure(struct usb_device *dev) >> >> if (portchange & USB_PORT_STAT_C_CONNECTION) { >> USB_HUB_PRINTF("port %d connection change\n", i + 1); >> - usb_hub_port_connect_change(dev, i); >> + usb_hub_port_connect_change(dev, i, 0); >> + } else if ((portstatus & 0x1) == 1) { > > Magic constant ... NAK > Sure, missed the proper macro here. ;) Will amend it. >> + USB_HUB_PRINTF("port %d connection change\n", i + 1); >> + usb_hub_port_connect_change(dev, i, 1); >> } >> if (portchange & USB_PORT_STAT_C_ENABLE) { >> USB_HUB_PRINTF("port %d enable change, status %x\n", > > [...] > >> @@ -278,10 +278,11 @@ 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++; >> + } >> } >> + >> } > > Can we fix this depth of indend somehow? > Ok, will try to fix this chunk. >> /* if storage device */ >> if (usb_max_devs == USB_MAX_STOR_DEV) { >> @@ -511,7 +512,7 @@ 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", > > Fixes should certainly be submitted as separate patches prior to feature > additions. > Sure, fixes in a separate patch :-) >> dir_in, srb->lun, srb->cmdlen, srb->cmd, srb->datalen, >> srb->pdata); >> if (srb->cmdlen) { >> @@ -1208,6 +1209,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; >> @@ -1290,24 +1292,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].ep_desc; >> /* is it an BULK endpoint? */ >> - if ((iface->ep_desc[i].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->bmAttributes & >> + USB_ENDPOINT_XFERTYPE_MASK) == USB_ENDPOINT_XFER_BULK) { >> + 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; > > Are you just introducing new variable here? > This is again a fix. As pointed here "ep_desc = &iface->ep_desc[i].ep_desc", fixes things around introduction of "struct usb_ep_desc". >> } >> } >> USB_STOR_PRINTF("Endpoints In %d Out %d Int %d\n", >> diff --git a/include/common.h b/include/common.h >> index b23e90b..ef5f687 100644 >> --- a/include/common.h >> +++ b/include/common.h >> @@ -211,6 +211,8 @@ typedef void (interrupt_handler_t)(void *); >> #define MIN(x, y) min(x, y) >> #define MAX(x, y) max(x, y) >> >> +#define min3(a, b, c) min(min(a, b), c) >> + > > Isn't this defined somewhere already? > Can you please guide me here, i can see a similar inline function in ehci-hcd only :( > [...] > > Rest is just great. Thanks for reviewing this Marek. I shall update the patchset soon. Thanks & Regards Vivek
Dear Vivek Gautam, [...] > > This comes as an affect of introduction of "struct usb_ep_desc" > which eventually contains "struct usb_endpoint_descriptor" and > "struct usb_ss_ep_comp_descriptor". I see, can we split this? I really enjoy small incremental patches, it's much easier to bisect issues then. > >> 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), > > > > Do you need these braces in &(...) ? > > True, we can remove these braces. > > >> + &buffer[index], buffer[index]); > >> + break; > >> > >> default: > >> if (head->bLength == 0) > >> > >> return 1; > >> > >> @@ -659,6 +666,18 @@ static int usb_get_string(struct usb_device *dev, > >> unsigned short langid, return result; > >> > >> } > >> > >> +/* Allocate usb device */ > >> +int usb_alloc_dev(struct usb_device *dev) > >> +{ > >> + int res; > >> + > >> + USB_PRINTF("alloc device\n"); > > > > this is a debug print. > > Isn't USB_PRINTF itself a conditional debug print ? Yes, but I'd prefer to kill USB_PRINTF altogether in favor of debug(). > >> + res = usb_control_msg(dev, usb_sndctrlpipe(dev->parent, 0), > >> + USB_REQ_ALLOC_DEV, 0, 0, 0, > >> + NULL, 0, USB_CNTL_TIMEOUT); > > > > How does this "allocate" a device? Please, do a proper documentation. > > Actually, take a look at include/linker_lists.h > > > > Or see here: > > http://git.denx.de/?p=u- > > boot.git;a=blob;f=include/linker_lists.h;h=0b405d78ea34df1c528fbc4e24ed2a > > ad756ac4a2;hb=HEAD > > > > And here (U-Boot Code Documentation): > > http://www.denx.de/wiki/U-Boot/CodingStyle > > > > It'd be really nice if you could follow such pattern of documentation! > > Yes, thanks for pointing out. Will document it properly to make things > more understandable. _AWESOME_ ! > >> + return res; > >> +} > >> [...] > >> diff --git a/include/common.h b/include/common.h > >> index b23e90b..ef5f687 100644 > >> --- a/include/common.h > >> +++ b/include/common.h > >> @@ -211,6 +211,8 @@ typedef void (interrupt_handler_t)(void *); > >> > >> #define MIN(x, y) min(x, y) > >> #define MAX(x, y) max(x, y) > >> > >> +#define min3(a, b, c) min(min(a, b), c) > >> + > > > > Isn't this defined somewhere already? > > Can you please guide me here, i can see a similar inline function in > ehci-hcd only :( ICK, so we have ad-hoc implementation of this :-( I'd say, put it into common.h and remove the ehci's ad-hoc implementation. > > [...] > > > > Rest is just great. > > Thanks for reviewing this Marek. I shall update the patchset soon. Welcome, it's pleasure to work with you ;-)
Hi Marek, On Fri, Oct 26, 2012 at 3:48 PM, Marek Vasut <marex@denx.de> wrote: > Dear Vivek Gautam, > > [...] >> >> This comes as an affect of introduction of "struct usb_ep_desc" >> which eventually contains "struct usb_endpoint_descriptor" and >> "struct usb_ss_ep_comp_descriptor". > > I see, can we split this? I really enjoy small incremental patches, it's much > easier to bisect issues then. > Sure, will split this small change. >> >> 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), >> > >> > Do you need these braces in &(...) ? >> >> True, we can remove these braces. >> >> >> + &buffer[index], buffer[index]); >> >> + break; >> >> >> >> default: >> >> if (head->bLength == 0) >> >> >> >> return 1; >> >> >> >> @@ -659,6 +666,18 @@ static int usb_get_string(struct usb_device *dev, >> >> unsigned short langid, return result; >> >> >> >> } >> >> >> >> +/* Allocate usb device */ >> >> +int usb_alloc_dev(struct usb_device *dev) >> >> +{ >> >> + int res; >> >> + >> >> + USB_PRINTF("alloc device\n"); >> > >> > this is a debug print. >> >> Isn't USB_PRINTF itself a conditional debug print ? > > Yes, but I'd prefer to kill USB_PRINTF altogether in favor of debug(). > Ok, will switch to debug(...). And may be in that case we may try to eliminate USB _PRINTF and other such macro altogether from sub framework ? >> >> + res = usb_control_msg(dev, usb_sndctrlpipe(dev->parent, 0), >> >> + USB_REQ_ALLOC_DEV, 0, 0, 0, >> >> + NULL, 0, USB_CNTL_TIMEOUT); >> > >> > How does this "allocate" a device? Please, do a proper documentation. >> > Actually, take a look at include/linker_lists.h >> > >> > Or see here: >> > http://git.denx.de/?p=u- >> > boot.git;a=blob;f=include/linker_lists.h;h=0b405d78ea34df1c528fbc4e24ed2a >> > ad756ac4a2;hb=HEAD >> > >> > And here (U-Boot Code Documentation): >> > http://www.denx.de/wiki/U-Boot/CodingStyle >> > >> > It'd be really nice if you could follow such pattern of documentation! >> >> Yes, thanks for pointing out. Will document it properly to make things >> more understandable. > > _AWESOME_ ! > >> >> + return res; >> >> +} >> >> > > [...] > >> >> diff --git a/include/common.h b/include/common.h >> >> index b23e90b..ef5f687 100644 >> >> --- a/include/common.h >> >> +++ b/include/common.h >> >> @@ -211,6 +211,8 @@ typedef void (interrupt_handler_t)(void *); >> >> >> >> #define MIN(x, y) min(x, y) >> >> #define MAX(x, y) max(x, y) >> >> >> >> +#define min3(a, b, c) min(min(a, b), c) >> >> + >> > >> > Isn't this defined somewhere already? >> >> Can you please guide me here, i can see a similar inline function in >> ehci-hcd only :( > > ICK, so we have ad-hoc implementation of this :-( I'd say, put it into common.h > and remove the ehci's ad-hoc implementation. > Yeah, sure will keep this in common.h and update ehci-hcd. >> > [...] >> > >> > Rest is just great. >> >> Thanks for reviewing this Marek. I shall update the patchset soon. > > Welcome, it's pleasure to work with you ;-)
Dear Vivek Gautam, [...] > >> > >> Isn't USB_PRINTF itself a conditional debug print ? > > > > Yes, but I'd prefer to kill USB_PRINTF altogether in favor of debug(). > > Ok, will switch to debug(...). > And may be in that case we may try to eliminate USB _PRINTF and > other such macro altogether from sub framework ? Yes, weed it out, be my guest ;-) [...] Thanks! Best regards, Marek Vasut
diff --git a/common/cmd_usb.c b/common/cmd_usb.c index c128455..013b2e8 100644 --- a/common/cmd_usb.c +++ b/common/cmd_usb.c @@ -262,7 +262,7 @@ 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 @@ 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 50b8175..691d9ac 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); @@ -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; @@ -401,21 +402,27 @@ static int usb_parse_config(struct usb_device *dev, 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].ep_desc, &buffer[index], buffer[index]); 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; @@ -659,6 +666,18 @@ static int usb_get_string(struct usb_device *dev, unsigned short langid, return result; } +/* Allocate usb device */ +int usb_alloc_dev(struct usb_device *dev) +{ + int res; + + USB_PRINTF("alloc device\n"); + res = usb_control_msg(dev, usb_sndctrlpipe(dev->parent, 0), + USB_REQ_ALLOC_DEV, 0, 0, 0, + NULL, 0, USB_CNTL_TIMEOUT); + + return res; +} static void usb_try_string_workarounds(unsigned char *buf, int *length) { @@ -852,7 +871,10 @@ int usb_new_device(struct usb_device *dev) struct usb_device_descriptor *desc; int port = -1; struct usb_device *parent = dev->parent; + +#ifndef CONFIG_USB_XHCI unsigned short portstatus; +#endif /* send 64-byte GET-DEVICE-DESCRIPTOR request. Since the descriptor is * only 18 bytes long, this will terminate with a short packet. But if @@ -889,12 +911,21 @@ int usb_new_device(struct usb_device *dev) return 1; } + /* + * Putting up the code here for slot assignment and initialization: + * xHCI spec sec 4.3.2, 4.3.3 + */ +#ifdef CONFIG_USB_XHCI + /* Call if and only if device is attached and detected */ + usb_alloc_dev(dev); +#else /* reset the port for the second time */ err = hub_port_reset(dev->parent, port, &portstatus); if (err < 0) { printf("\n Couldn't reset port %i\n", port); return 1; } +#endif } #endif diff --git a/common/usb_hub.c b/common/usb_hub.c index e4a1201..8a00894 100644 --- a/common/usb_hub.c +++ b/common/usb_hub.c @@ -204,7 +204,12 @@ int hub_port_reset(struct usb_device *dev, int port, } -void usb_hub_port_connect_change(struct usb_device *dev, int port) +/* + * Adding the flag do_port_reset: USB 2.0 device requires port reset + * while USB 3.0 device does not. + */ +void usb_hub_port_connect_change(struct usb_device *dev, int port, + int do_port_reset) { struct usb_device *usb; ALLOC_CACHE_ALIGN_BUFFER(struct usb_port_status, portsts, 1); @@ -235,11 +240,21 @@ void usb_hub_port_connect_change(struct usb_device *dev, int port) } mdelay(200); +#ifdef CONFIG_USB_XHCI + /* Reset the port */ + if (do_port_reset) { + if (hub_port_reset(dev, port, &portstatus) < 0) { + printf("cannot reset port %i!?\n", port + 1); + return; + } + } +#else /* Reset the port */ if (hub_port_reset(dev, port, &portstatus) < 0) { printf("cannot reset port %i!?\n", port + 1); return; } +#endif mdelay(200); @@ -409,7 +424,10 @@ static int usb_hub_configure(struct usb_device *dev) if (portchange & USB_PORT_STAT_C_CONNECTION) { USB_HUB_PRINTF("port %d connection change\n", i + 1); - usb_hub_port_connect_change(dev, i); + usb_hub_port_connect_change(dev, i, 0); + } else if ((portstatus & 0x1) == 1) { + USB_HUB_PRINTF("port %d connection change\n", i + 1); + usb_hub_port_connect_change(dev, i, 1); } if (portchange & USB_PORT_STAT_C_ENABLE) { USB_HUB_PRINTF("port %d enable change, status %x\n", @@ -426,7 +444,7 @@ static int usb_hub_configure(struct usb_device *dev) USB_HUB_PRINTF("already running port %i " \ "disabled by hub (EMI?), " \ "re-enabling...\n", i + 1); - usb_hub_port_connect_change(dev, i); + usb_hub_port_connect_change(dev, i, 0); } } if (portstatus & USB_PORT_STAT_SUSPEND) { @@ -470,7 +488,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 0c2a4c7..53d41d8 100644 --- a/common/usb_storage.c +++ b/common/usb_storage.c @@ -262,7 +262,7 @@ int usb_stor_scan(int mode) usb_max_devs = 0; for (i = 0; i < USB_MAX_DEVICE; i++) { dev = usb_get_dev_index(i); /* get device */ - USB_STOR_PRINTF("i=%d\n", i); + USB_STOR_PRINTF("i=%d, dev_num=%d\n", i, dev->devnum); if (dev == NULL) break; /* no more devices available */ @@ -278,10 +278,11 @@ 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 +512,7 @@ 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) { @@ -1208,6 +1209,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; @@ -1290,24 +1292,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].ep_desc; /* is it an BULK endpoint? */ - if ((iface->ep_desc[i].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->bmAttributes & + USB_ENDPOINT_XFERTYPE_MASK) == USB_ENDPOINT_XFER_BULK) { + 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/common.h b/include/common.h index b23e90b..ef5f687 100644 --- a/include/common.h +++ b/include/common.h @@ -211,6 +211,8 @@ typedef void (interrupt_handler_t)(void *); #define MIN(x, y) min(x, y) #define MAX(x, y) max(x, y) +#define min3(a, b, c) min(min(a, b), c) + /* * Return the absolute value of a number. * diff --git a/include/linux/usb/ch9.h b/include/linux/usb/ch9.h index ce1d1e1..33af2f4 100644 --- a/include/linux/usb/ch9.h +++ b/include/linux/usb/ch9.h @@ -122,7 +122,6 @@ #define USB_ENDPOINT_HALT 0 /* IN/OUT will STALL */ - /** * struct usb_ctrlrequest - SETUP data for a USB device control request * @bRequestType: matches the USB bmRequestType field @@ -491,6 +490,7 @@ enum usb_device_speed { USB_SPEED_UNKNOWN = 0, /* enumerating */ USB_SPEED_LOW, USB_SPEED_FULL, /* usb 1.1 */ USB_SPEED_HIGH, /* usb 2.0 */ + USB_SPEED_SUPER, /* usb 3.0 */ USB_SPEED_VARIABLE, /* wireless (usb 2.5) */ }; diff --git a/include/usb.h b/include/usb.h index 9dd8791..a7a0f22 100644 --- a/include/usb.h +++ b/include/usb.h @@ -73,6 +73,16 @@ struct usb_descriptor_header { unsigned char bDescriptorType; } __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; @@ -81,7 +91,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.. */ @@ -153,7 +163,8 @@ struct usb_device { defined(CONFIG_USB_SL811HS) || defined(CONFIG_USB_ISP116X_HCD) || \ defined(CONFIG_USB_R8A66597_HCD) || defined(CONFIG_USB_DAVINCI) || \ defined(CONFIG_USB_OMAP3) || defined(CONFIG_USB_DA8XX) || \ - defined(CONFIG_USB_BLACKFIN) || defined(CONFIG_USB_AM35X) + defined(CONFIG_USB_BLACKFIN) || defined(CONFIG_USB_AM35X) || \ + defined(CONFIG_USB_XHCI) int usb_lowlevel_init(int index, void **controller); int usb_lowlevel_stop(int index); diff --git a/include/usb_defs.h b/include/usb_defs.h index 8032e57..e681609 100644 --- a/include/usb_defs.h +++ b/include/usb_defs.h @@ -84,7 +84,8 @@ #define USB_SPEED_FULL 0x0 /* 12Mbps */ #define USB_SPEED_LOW 0x1 /* 1.5Mbps */ #define USB_SPEED_HIGH 0x2 /* 480Mbps */ -#define USB_SPEED_RESERVED 0x3 +#define USB_SPEED_SUPER 0x3 /* 5Gbps */ +#define USB_SPEED_RESERVED 0x4 /* Descriptor types */ #define USB_DT_DEVICE 0x01 @@ -93,6 +94,9 @@ #define USB_DT_INTERFACE 0x04 #define USB_DT_ENDPOINT 0x05 +/* From the USB 3.0 spec */ +#define USB_DT_SS_ENDPOINT_COMP 0x30 + #define USB_DT_HID (USB_TYPE_CLASS | 0x01) #define USB_DT_REPORT (USB_TYPE_CLASS | 0x02) #define USB_DT_PHYSICAL (USB_TYPE_CLASS | 0x03) @@ -156,6 +160,7 @@ #define USB_REQ_SET_IDLE 0x0A #define USB_REQ_SET_PROTOCOL 0x0B +#define USB_REQ_ALLOC_DEV 0xDE /* "pipe" definitions */ @@ -227,6 +232,23 @@ #define USB_PORT_STAT_SPEED \ (USB_PORT_STAT_LOW_SPEED | USB_PORT_STAT_HIGH_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 + +/* + * USB 3.0 wPortChange bit fields + * 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 + /* wPortChange bits */ #define USB_PORT_STAT_C_CONNECTION 0x0001 #define USB_PORT_STAT_C_ENABLE 0x0002 @@ -240,7 +262,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 diff --git a/include/usbdescriptors.h b/include/usbdescriptors.h index de1069f..527e895 100644 --- a/include/usbdescriptors.h +++ b/include/usbdescriptors.h @@ -192,6 +192,15 @@ * standard usb descriptor structures */ +/* USB_DT_SS_ENDPOINT_COMP: SuperSpeed Endpoint Companion descriptor */ +struct usb_ss_ep_comp_descriptor { + u8 bLength; + u8 bDescriptorType; /* 0x30 */ + u8 bMaxBurst; + u8 bmAttributes; + u16 wBytesPerInterval; +} __attribute__ ((packed)); + struct usb_endpoint_descriptor { u8 bLength; u8 bDescriptorType; /* 0x5 */ @@ -545,4 +554,196 @@ static inline void print_device_descriptor(struct usb_device_descriptor *d) #define print_device_descriptor(d) #endif /* DEBUG */ + +/*-------------------------------------------------------------------------*/ + +/** + * Get the endpoint's number + * + * @param epd Endpoint descriptor to be checked + * @return epd's number, 0 to 15. + */ +static inline int usb_endpoint_num(struct usb_endpoint_descriptor *epd) +{ + return epd->bEndpointAddress & USB_ENDPOINT_NUMBER_MASK; +} + +/** + * Get the endpoint's transfer type + * + * @param epd Endpoint to be checked + * @return One of USB_ENDPOINT_XFER_{CONTROL, ISOC, BULK, INT} according + * to @epd's transfer type. + */ +static inline int usb_endpoint_type(struct usb_endpoint_descriptor *epd) +{ + return epd->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK; +} + +/** + * usb_endpoint_dir_in - check if the endpoint has IN direction + * @epd: endpoint to be checked + * + * Returns true if the endpoint is of type IN, otherwise it returns false. + */ +static inline int usb_endpoint_dir_in(struct usb_endpoint_descriptor *epd) +{ + return (epd->bEndpointAddress & USB_ENDPOINT_DIR_MASK) == USB_DIR_IN; +} + +/** + * usb_endpoint_dir_out - check if the endpoint has OUT direction + * @epd: endpoint to be checked + * + * Returns true if the endpoint is of type OUT, otherwise it returns false. + */ +static inline int usb_endpoint_dir_out( + struct usb_endpoint_descriptor *epd) +{ + return (epd->bEndpointAddress & USB_ENDPOINT_DIR_MASK) == USB_DIR_OUT; +} + +/** + * usb_endpoint_xfer_bulk - check if the endpoint has bulk transfer type + * @epd: endpoint to be checked + * + * Returns true if the endpoint is of type bulk, otherwise it returns false. + */ +static inline int usb_endpoint_xfer_bulk( + struct usb_endpoint_descriptor *epd) +{ + return (epd->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK) == + USB_ENDPOINT_XFER_BULK; +} + +/** + * usb_endpoint_xfer_control - check if the endpoint has control transfer type + * @epd: endpoint to be checked + * + * Returns true if the endpoint is of type control, otherwise it returns false. + */ +static inline int usb_endpoint_xfer_control( + struct usb_endpoint_descriptor *epd) +{ + return (epd->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK) == + USB_ENDPOINT_XFER_CONTROL; +} + +/** + * usb_endpoint_xfer_int - check if the endpoint has interrupt transfer type + * @epd: endpoint to be checked + * + * Returns true if the endpoint is of type interrupt, otherwise it returns + * false. + */ +static inline int usb_endpoint_xfer_int( + struct usb_endpoint_descriptor *epd) +{ + return (epd->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK) == + USB_ENDPOINT_XFER_INT; +} + +/** + * usb_endpoint_xfer_isoc - check if the endpoint has isochronous transfer type + * @epd: endpoint to be checked + * + * Returns true if the endpoint is of type isochronous, otherwise it returns + * false. + */ +static inline int usb_endpoint_xfer_isoc( + struct usb_endpoint_descriptor *epd) +{ + return (epd->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK) == + USB_ENDPOINT_XFER_ISOC; +} + +/** + * usb_endpoint_is_bulk_in - check if the endpoint is bulk IN + * @epd: endpoint to be checked + * + * Returns true if the endpoint has bulk transfer type and IN direction, + * otherwise it returns false. + */ +static inline int usb_endpoint_is_bulk_in( + struct usb_endpoint_descriptor *epd) +{ + return usb_endpoint_xfer_bulk(epd) && usb_endpoint_dir_in(epd); +} + +/** + * usb_endpoint_is_bulk_out - check if the endpoint is bulk OUT + * @epd: endpoint to be checked + * + * Returns true if the endpoint has bulk transfer type and OUT direction, + * otherwise it returns false. + */ +static inline int usb_endpoint_is_bulk_out( + struct usb_endpoint_descriptor *epd) +{ + return usb_endpoint_xfer_bulk(epd) && usb_endpoint_dir_out(epd); +} + +/** + * usb_endpoint_is_int_in - check if the endpoint is interrupt IN + * @epd: endpoint to be checked + * + * Returns true if the endpoint has interrupt transfer type and IN direction, + * otherwise it returns false. + */ +static inline int usb_endpoint_is_int_in( + struct usb_endpoint_descriptor *epd) +{ + return usb_endpoint_xfer_int(epd) && usb_endpoint_dir_in(epd); +} + +/** + * usb_endpoint_is_int_out - check if the endpoint is interrupt OUT + * @epd: endpoint to be checked + * + * Returns true if the endpoint has interrupt transfer type and OUT direction, + * otherwise it returns false. + */ +static inline int usb_endpoint_is_int_out( + struct usb_endpoint_descriptor *epd) +{ + return usb_endpoint_xfer_int(epd) && usb_endpoint_dir_out(epd); +} + +/** + * usb_endpoint_is_isoc_in - check if the endpoint is isochronous IN + * @epd: endpoint to be checked + * + * Returns true if the endpoint has isochronous transfer type and IN direction, + * otherwise it returns false. + */ +static inline int usb_endpoint_is_isoc_in( + struct usb_endpoint_descriptor *epd) +{ + return usb_endpoint_xfer_isoc(epd) && usb_endpoint_dir_in(epd); +} + +/** + * usb_endpoint_is_isoc_out - check if the endpoint is isochronous OUT + * @epd: endpoint to be checked + * + * Returns true if the endpoint has isochronous transfer type and OUT direction, + * otherwise it returns false. + */ +static inline int usb_endpoint_is_isoc_out( + struct usb_endpoint_descriptor *epd) +{ + return usb_endpoint_xfer_isoc(epd) && usb_endpoint_dir_out(epd); +} + +/** + * usb_endpoint_maxp - get endpoint's max packet size + * @epd: endpoint to be checked + * + * Returns @epd's max packet + */ +static inline int usb_endpoint_maxp(struct usb_endpoint_descriptor *epd) +{ + return le16_to_cpu(epd->wMaxPacketSize); +} + #endif
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> Signedoff-by: Vivek Gautam <gautam.vivek@samsung.com> --- common/cmd_usb.c | 6 +- common/usb.c | 41 ++++++++- common/usb_hub.c | 26 +++++- common/usb_storage.c | 35 +++++---- include/common.h | 2 + include/linux/usb/ch9.h | 2 +- include/usb.h | 15 +++- include/usb_defs.h | 26 ++++++- include/usbdescriptors.h | 201 ++++++++++++++++++++++++++++++++++++++++++++++ 9 files changed, 322 insertions(+), 32 deletions(-)