diff mbox

[v3,6/8] USB: SS: Add support for Super Speed USB interface

Message ID 1365764680-10917-7-git-send-email-gautam.vivek@samsung.com
State Accepted
Commit 6497c66704d03956e7ea49b54fcaa38740736416
Headers show

Commit Message

Vivek Gautam April 12, 2013, 11:04 a.m. UTC
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>
---

Changes from v2:
 - Replacing if-else with switch-case in portspeed() in cmd_usb.c

 common/cmd_usb.c   |   24 ++++++++++++++++++------
 common/usb.c       |    5 +++++
 common/usb_hub.c   |    8 ++++++--
 include/usb.h      |    6 ++++++
 include/usb_defs.h |   24 +++++++++++++++++++++++-
 5 files changed, 58 insertions(+), 9 deletions(-)

Comments

Julius Werner April 19, 2013, 6:22 p.m. UTC | #1
Migrating my comments here for public discussion.

> 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>
> 
> ---
> Changes from v2:
>  - Replacing if-else with switch-case in portspeed() in cmd_usb.c
> 
>  common/cmd_usb.c   |   24 ++++++++++++++++++------
>  common/usb.c       |    5 +++++
>  common/usb_hub.c   |    8 ++++++--
>  include/usb.h      |    6 ++++++
>  include/usb_defs.h |   24 +++++++++++++++++++++++-
>  5 files changed, 58 insertions(+), 9 deletions(-)
> 
> diff --git a/common/cmd_usb.c b/common/cmd_usb.c
> index dacdc2d..594385a 100644
> --- a/common/cmd_usb.c
> +++ b/common/cmd_usb.c
> @@ -271,12 +271,24 @@ static void usb_display_config(struct usb_device *dev)
>  
>  static inline char *portspeed(int speed)
>  {
> -	if (speed == USB_SPEED_HIGH)
> -		return "480 Mb/s";
> -	else if (speed == USB_SPEED_LOW)
> -		return "1.5 Mb/s";
> -	else
> -		return "12 Mb/s";
> +	char *speed_str;
> +
> +	switch (speed) {
> +	case USB_SPEED_SUPER:
> +		speed_str = "5 Gb/s";
> +		break;
> +	case USB_SPEED_HIGH:
> +		speed_str = "480 Mb/s";
> +		break;
> +	case USB_SPEED_LOW:
> +		speed_str = "1.5 Mb/s";
> +		break;
> +	default:
> +		speed_str = "12 Mb/s";
> +		break;
> +	}
> +
> +	return speed_str;
>  }
>  
>  /* shows the device tree recursively */
> diff --git a/common/usb.c b/common/usb.c
> index 3a96a34..55fff5b 100644
> --- a/common/usb.c
> +++ b/common/usb.c
> @@ -409,6 +409,11 @@ static int usb_parse_config(struct usb_device *dev,
>  					wMaxPacketSize);
>  			debug("if %d, ep %d\n", ifno, epno);
>  			break;
> +		case USB_DT_SS_ENDPOINT_COMP:
> +			if_desc = &dev->config.if_desc[ifno];
> +			memcpy(&if_desc->ss_ep_comp_desc[epno],
> +				&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 ab41943..1e225e6 100644
> --- a/common/usb_hub.c
> +++ b/common/usb_hub.c
> @@ -165,7 +165,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))

It doesn't make sense to use the USB_PORT_FEAT_ constants to read a port status
here. These should be USB_PORT_STAT_ instead (you could use a switch statement
if you mask and shift them correctly).

> +		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";
> @@ -268,7 +270,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;

Should change to switch statement after implementing my suggestion to
usb_defs.h

> diff --git a/include/usb.h b/include/usb.h
> index d79c865..d7b082d 100644
> --- a/include/usb.h
> +++ b/include/usb.h
> @@ -76,6 +76,12 @@ struct usb_interface {
>  	unsigned char	act_altsetting;
>  
>  	struct usb_endpoint_descriptor ep_desc[USB_MAXENDPOINTS];
> +	/*
> +	 * 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_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

Speed is never set as a feature anyway. We should just get rid of all of these
and always use USB_PORT_STAT_ for them.

>  #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 */

These two new values are *not* correct. 0x800 is actually PORT_TEST, and 0x1000
is PORT_INDICATOR. Full speed has always been reported by having both the
LOW_SPEED and the HIGH_SPEED bits zero, and you should define it that way (for
use in switch statements).

If you want to fake SuperSpeed into the USB 2.0 hub descriptor (see my comment
on xhci_port_speed in the unreleased CL), you could represent it with 0xc00,
which should be a reserved value for all existing 1.0 and 2.0 hubs.

>  #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)

Maybe this should be renamed to USB_PORT_STAT_SPEED_MASK to make its purpose
clearer.

> +
> +/*
> + * Additions to wPortStatus bit field from USB 3.0

I would reword "Additions" to "Changes" and explicitly state that these fields
concern the new USB 3.0 hub descriptor format, which is different and
exclusively applies to SuperSpeed hubs.

> + * See USB 3.0 spec Table 10-10

In my spec it's Table 10-11... are you sure you have the most recent version?

> + */
> +#define USB_PORT_STAT_LINK_STATE	0x01e0

For consistency, I would prefix all values in here with USB_SS_PORT_STAT_ (to
make extra clear that they belong to something different than the ones above).

> +#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

Same here. New incompatible port status format for SuperSpeed hubs, please make
the comment clearer and prefix as USB_SS_PORT_STAT_C_

> +#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
Marek Vasut April 19, 2013, 6:38 p.m. UTC | #2
Hi Julius,

> Migrating my comments here for public discussion.

Maybe you can make a patch(set) against u-boot-usb/next ?

Best regards,
Marek Vasut
Julius Werner April 19, 2013, 8:32 p.m. UTC | #3
These patches haven't gone in yet, right? I think Vivek wants to
discuss/update them himself, he just asked me to move my reviews to
this thread.
Marek Vasut April 20, 2013, 11:57 a.m. UTC | #4
Dear Julius Werner,

> These patches haven't gone in yet, right? I think Vivek wants to
> discuss/update them himself, he just asked me to move my reviews to
> this thread.

They're not in, but they're already pushed in my tree. It'd be also easier to 
review diff instead of full patches again.

Best regards,
Marek Vasut
Vivek Gautam April 22, 2013, 6:43 a.m. UTC | #5
Hi Julius,


On Fri, Apr 19, 2013 at 11:52 PM, Julius Werner <jwerner@chromium.org> wrote:
> Migrating my comments here for public discussion.

Thanks for your valuable comments here.

>
>> 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>
>>
>> ---
>> Changes from v2:
>>  - Replacing if-else with switch-case in portspeed() in cmd_usb.c
>>
>>  common/cmd_usb.c   |   24 ++++++++++++++++++------
>>  common/usb.c       |    5 +++++
>>  common/usb_hub.c   |    8 ++++++--
>>  include/usb.h      |    6 ++++++
>>  include/usb_defs.h |   24 +++++++++++++++++++++++-
>>  5 files changed, 58 insertions(+), 9 deletions(-)
>>
>> diff --git a/common/cmd_usb.c b/common/cmd_usb.c
>> index dacdc2d..594385a 100644
>> --- a/common/cmd_usb.c
>> +++ b/common/cmd_usb.c
>> @@ -271,12 +271,24 @@ static void usb_display_config(struct usb_device *dev)
>>
>>  static inline char *portspeed(int speed)
>>  {
>> -     if (speed == USB_SPEED_HIGH)
>> -             return "480 Mb/s";
>> -     else if (speed == USB_SPEED_LOW)
>> -             return "1.5 Mb/s";
>> -     else
>> -             return "12 Mb/s";
>> +     char *speed_str;
>> +
>> +     switch (speed) {
>> +     case USB_SPEED_SUPER:
>> +             speed_str = "5 Gb/s";
>> +             break;
>> +     case USB_SPEED_HIGH:
>> +             speed_str = "480 Mb/s";
>> +             break;
>> +     case USB_SPEED_LOW:
>> +             speed_str = "1.5 Mb/s";
>> +             break;
>> +     default:
>> +             speed_str = "12 Mb/s";
>> +             break;
>> +     }
>> +
>> +     return speed_str;
>>  }
>>
>>  /* shows the device tree recursively */
>> diff --git a/common/usb.c b/common/usb.c
>> index 3a96a34..55fff5b 100644
>> --- a/common/usb.c
>> +++ b/common/usb.c
>> @@ -409,6 +409,11 @@ static int usb_parse_config(struct usb_device *dev,
>>                                       wMaxPacketSize);
>>                       debug("if %d, ep %d\n", ifno, epno);
>>                       break;
>> +             case USB_DT_SS_ENDPOINT_COMP:
>> +                     if_desc = &dev->config.if_desc[ifno];
>> +                     memcpy(&if_desc->ss_ep_comp_desc[epno],
>> +                             &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 ab41943..1e225e6 100644
>> --- a/common/usb_hub.c
>> +++ b/common/usb_hub.c
>> @@ -165,7 +165,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))
>
> It doesn't make sense to use the USB_PORT_FEAT_ constants to read a port status
> here. These should be USB_PORT_STAT_ instead (you could use a switch statement
> if you mask and shift them correctly).

True, we should be 'ANDing' here with respectve PORT_STAT_ constants,
will modify this as required.

>
>> +             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";
>> @@ -268,7 +270,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;
>
> Should change to switch statement after implementing my suggestion to
> usb_defs.h

Sure,

>
>> diff --git a/include/usb.h b/include/usb.h
>> index d79c865..d7b082d 100644
>> --- a/include/usb.h
>> +++ b/include/usb.h
>> @@ -76,6 +76,12 @@ struct usb_interface {
>>       unsigned char   act_altsetting;
>>
>>       struct usb_endpoint_descriptor ep_desc[USB_MAXENDPOINTS];
>> +     /*
>> +      * 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_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
>
> Speed is never set as a feature anyway. We should just get rid of all of these
> and always use USB_PORT_STAT_ for them.

These are simply 'Hub class feature numbers' given in USB 2.0 and 3.0 specs.
Why don't we just keep them like that (I shall be removing the
USB_PORT_FEAT_FULLSPEED
and USB_PORT_FEAT_SUPERSPEED definitely from here). In case we want to
check the port status field,
we would check against PORT_STAT_ constants.

>
>>  #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 */
>
> These two new values are *not* correct. 0x800 is actually PORT_TEST, and 0x1000
> is PORT_INDICATOR. Full speed has always been reported by having both the
> LOW_SPEED and the HIGH_SPEED bits zero, and you should define it that way (for
> use in switch statements).

Sure, will remove these two constants.

>
> If you want to fake SuperSpeed into the USB 2.0 hub descriptor (see my comment
> on xhci_port_speed in the unreleased CL), you could represent it with 0xc00,
> which should be a reserved value for all existing 1.0 and 2.0 hubs.
>
>>  #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)
>
> Maybe this should be renamed to USB_PORT_STAT_SPEED_MASK to make its purpose
> clearer.

Alright, shall rename it to USB_PORT_STAT_SPEED_MASK.

>
>> +
>> +/*
>> + * Additions to wPortStatus bit field from USB 3.0
>
> I would reword "Additions" to "Changes" and explicitly state that these fields
> concern the new USB 3.0 hub descriptor format, which is different and
> exclusively applies to SuperSpeed hubs.

This bit fields are actually additions to USB 3.0 framework. These
fields are not defined in
USB 2.0 specs, or else they are reserved there. If you insist i can
modify this as suggested.

>
>> + * See USB 3.0 spec Table 10-10
>
> In my spec it's Table 10-11... are you sure you have the most recent version?

Yea, my mistake, typo !!

>
>> + */
>> +#define USB_PORT_STAT_LINK_STATE     0x01e0
>
> For consistency, I would prefix all values in here with USB_SS_PORT_STAT_ (to
> make extra clear that they belong to something different than the ones above).

True, shall change this to make things consistent.

>
>> +#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
>
> Same here. New incompatible port status format for SuperSpeed hubs, please make
> the comment clearer and prefix as USB_SS_PORT_STAT_C_

Sure, will add the relavant prefix, and write proper comment.

>
>> +#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
Vivek Gautam April 22, 2013, 6:46 a.m. UTC | #6
Hi Marek,


On Sat, Apr 20, 2013 at 5:27 PM, Marek Vasut <marex@denx.de> wrote:
> Dear Julius Werner,
>
>> These patches haven't gone in yet, right? I think Vivek wants to
>> discuss/update them himself, he just asked me to move my reviews to
>> this thread.
>
> They're not in, but they're already pushed in my tree. It'd be also easier to
> review diff instead of full patches again.

I shall send a diff patchset for these changes, but do we have a
possibility of squashing the changes
to earler commits at some point of time later, so that clean changes
get to u-boot ?
Or we shall go as you suggest.
Julius Werner April 22, 2013, 10:14 p.m. UTC | #7
>> I would reword "Additions" to "Changes" and explicitly state that these fields
>> concern the new USB 3.0 hub descriptor format, which is different and
>> exclusively applies to SuperSpeed hubs.
>
> This bit fields are actually additions to USB 3.0 framework. These
> fields are not defined in
> USB 2.0 specs, or else they are reserved there. If you insist i can
> modify this as suggested.

Yes, okay, I think we mean the same thing here. I just wanted to point
out that the USB 3.0 port status is actually a completely different
thing from the USB 2.0 port status (which is still valid for
non-SuperSpeed hubs in USB 3.0 environments, even though the spec
hasn't printed it out again). They did not just *add* a few bits to
the old status bit field, they actually *changed* existing bits (not
just reserved ones)... e.g. bit 9 means LOW_SPEED for USB 2.0 hubs but
PORT_POWER for SuperSpeed hubs. I found this very confusing when I
first read it, so I thought it might be worth making it extra obvious
through comments.
Marek Vasut April 23, 2013, 2:24 a.m. UTC | #8
Dear Vivek Gautam,

> Hi Marek,
> 
> On Sat, Apr 20, 2013 at 5:27 PM, Marek Vasut <marex@denx.de> wrote:
> > Dear Julius Werner,
> > 
> >> These patches haven't gone in yet, right? I think Vivek wants to
> >> discuss/update them himself, he just asked me to move my reviews to
> >> this thread.
> > 
> > They're not in, but they're already pushed in my tree. It'd be also
> > easier to review diff instead of full patches again.
> 
> I shall send a diff patchset for these changes, but do we have a
> possibility of squashing the changes
> to earler commits at some point of time later, so that clean changes
> get to u-boot ?
> Or we shall go as you suggest.

Just send a subsequent patch ;-)

Best regards,
Marek Vasut
Vivek Gautam April 23, 2013, 4:39 a.m. UTC | #9
Hi Julius,


On Tue, Apr 23, 2013 at 3:44 AM, Julius Werner <jwerner@chromium.org> wrote:
>>> I would reword "Additions" to "Changes" and explicitly state that these fields
>>> concern the new USB 3.0 hub descriptor format, which is different and
>>> exclusively applies to SuperSpeed hubs.
>>
>> This bit fields are actually additions to USB 3.0 framework. These
>> fields are not defined in
>> USB 2.0 specs, or else they are reserved there. If you insist i can
>> modify this as suggested.
>
> Yes, okay, I think we mean the same thing here. I just wanted to point
> out that the USB 3.0 port status is actually a completely different
> thing from the USB 2.0 port status (which is still valid for
> non-SuperSpeed hubs in USB 3.0 environments, even though the spec
> hasn't printed it out again). They did not just *add* a few bits to
> the old status bit field, they actually *changed* existing bits (not
> just reserved ones)... e.g. bit 9 means LOW_SPEED for USB 2.0 hubs but
> PORT_POWER for SuperSpeed hubs. I found this very confusing when I
> first read it, so I thought it might be worth making it extra obvious
> through comments.

Fair enough, i shall change this accordingly, Thanks !!
Vivek Gautam April 23, 2013, 6:46 a.m. UTC | #10
Hi Marek,


On Tue, Apr 23, 2013 at 7:54 AM, Marek Vasut <marex@denx.de> wrote:
> Dear Vivek Gautam,
>
>> Hi Marek,
>>
>> On Sat, Apr 20, 2013 at 5:27 PM, Marek Vasut <marex@denx.de> wrote:
>> > Dear Julius Werner,
>> >
>> >> These patches haven't gone in yet, right? I think Vivek wants to
>> >> discuss/update them himself, he just asked me to move my reviews to
>> >> this thread.
>> >
>> > They're not in, but they're already pushed in my tree. It'd be also
>> > easier to review diff instead of full patches again.
>>
>> I shall send a diff patchset for these changes, but do we have a
>> possibility of squashing the changes
>> to earler commits at some point of time later, so that clean changes
>> get to u-boot ?
>> Or we shall go as you suggest.
>
> Just send a subsequent patch ;-)

Sure, will send subsequent patches on u-boot-usb/next

>
> Best regards,
> Marek Vasut
diff mbox

Patch

diff --git a/common/cmd_usb.c b/common/cmd_usb.c
index dacdc2d..594385a 100644
--- a/common/cmd_usb.c
+++ b/common/cmd_usb.c
@@ -271,12 +271,24 @@  static void usb_display_config(struct usb_device *dev)
 
 static inline char *portspeed(int speed)
 {
-	if (speed == USB_SPEED_HIGH)
-		return "480 Mb/s";
-	else if (speed == USB_SPEED_LOW)
-		return "1.5 Mb/s";
-	else
-		return "12 Mb/s";
+	char *speed_str;
+
+	switch (speed) {
+	case USB_SPEED_SUPER:
+		speed_str = "5 Gb/s";
+		break;
+	case USB_SPEED_HIGH:
+		speed_str = "480 Mb/s";
+		break;
+	case USB_SPEED_LOW:
+		speed_str = "1.5 Mb/s";
+		break;
+	default:
+		speed_str = "12 Mb/s";
+		break;
+	}
+
+	return speed_str;
 }
 
 /* shows the device tree recursively */
diff --git a/common/usb.c b/common/usb.c
index 3a96a34..55fff5b 100644
--- a/common/usb.c
+++ b/common/usb.c
@@ -409,6 +409,11 @@  static int usb_parse_config(struct usb_device *dev,
 					wMaxPacketSize);
 			debug("if %d, ep %d\n", ifno, epno);
 			break;
+		case USB_DT_SS_ENDPOINT_COMP:
+			if_desc = &dev->config.if_desc[ifno];
+			memcpy(&if_desc->ss_ep_comp_desc[epno],
+				&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 ab41943..1e225e6 100644
--- a/common/usb_hub.c
+++ b/common/usb_hub.c
@@ -165,7 +165,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";
@@ -268,7 +270,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;
diff --git a/include/usb.h b/include/usb.h
index d79c865..d7b082d 100644
--- a/include/usb.h
+++ b/include/usb.h
@@ -76,6 +76,12 @@  struct usb_interface {
 	unsigned char	act_altsetting;
 
 	struct usb_endpoint_descriptor ep_desc[USB_MAXENDPOINTS];
+	/*
+	 * 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_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