Skip to contentSkip to conversation

usb : musb : Add high speed field in usb_device structure

PATCH Superseded Thomas Abraham 4 participants 5 messages Dec 16, 2008 – Dec 18, 2008

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Thomas Abraham

author
#0 lore raw

PATCH 1/6 usb : musb : Add high speed field in usb_device structure

This patch adds support for identification of high speed devices. For high spped devices that are connected via hubs, the information that the device is high speed is recorded. This is required by Mentor USB Host controller driver.

Signed-off-by Ravi Babu <[email protected]>
Signed-off-by Swaminathan S <[email protected]>
Signed-off-by Thomas Abraham <[email protected]>
Signed-off-by Ajay Kumar Gupta <[email protected]>
---
 common/usb.c       |    1 +
 include/usb.h      |    1 +
 include/usb_defs.h |    1 +
 3 files changed, 3 insertions(+), 0 deletions(-)
common/usb.c +1 -0
index ee18152..f8379c9 100644
@@ -1137,6 +1137,7 @@ void usb_hub_port_connect_change(struct usb_device *dev, int port)
 	/* Allocate a new device struct for it */
 	usb = usb_alloc_new_device();
 	usb->slow = (portstatus & USB_PORT_STAT_LOW_SPEED) ? 1 : 0;
+	usb->high = (portstatus & USB_PORT_STAT_HIGH_SPEED) ? 1 : 0;
 
 	dev->children[port] = usb;
 	usb->parent = dev;
include/usb.h +1 -0
index 510df95..e6aa551 100644
@@ -139,6 +139,7 @@ enum {
 struct usb_device {
 	int	devnum;			/* Device number on USB bus */
 	int	slow;			/* Slow device? */
+	int     high;			/* High speed device? */
 	char	mf[32];			/* manufacturer */
 	char	prod[32];		/* product */
 	char	serial[32];		/* serial number */
include/usb_defs.h +1 -0
index 353019f..500a0ad 100644
@@ -216,6 +216,7 @@
 #define USB_PORT_STAT_RESET         0x0010
 #define USB_PORT_STAT_POWER         0x0100
 #define USB_PORT_STAT_LOW_SPEED     0x0200
+#define USB_PORT_STAT_HIGH_SPEED    0x0400
 
 /* wPortChange bits */
 #define USB_PORT_STAT_C_CONNECTION  0x0001
Signature · 1 line
1.5.6
#1 lore raw

PATCH 1/6 usb : musb : Add high speed field in usb_device structure

Hello Thomas,

Nice work, but I have a few comments.

2008/12/16 Thomas Abraham <[email protected]>:

Quoted 20 lines

This patch adds support for identification of high speed devices. For high spped devices that are connected via hubs, the information that the device is high speed is recorded. This is required by Mentor USB Host controller driver.

Signed-off-by Ravi Babu <[email protected]>
Signed-off-by Swaminathan S <[email protected]>
Signed-off-by Thomas Abraham <[email protected]>
Signed-off-by Ajay Kumar Gupta <[email protected]>
---
 common/usb.c       |    1 +
 include/usb.h      |    1 +
 include/usb_defs.h |    1 +
 3 files changed, 3 insertions(+), 0 deletions(-)
common/usb.c +0 -0
index ee18152..f8379c9 100644

common/usb.c void usb_hub_port_connect_change(struct usb_device *dev, int port) · line 1140

Quoted 4 lines
common/usb.c +1 -0
@@ -1137,6 +1137,7 @@ void usb_hub_port_connect_change(struct usb_device *dev, int port)
       /* Allocate a new device struct for it */
       usb = usb_alloc_new_device();
       usb->slow = (portstatus & USB_PORT_STAT_LOW_SPEED) ? 1 : 0;
+       usb->high = (portstatus & USB_PORT_STAT_HIGH_SPEED) ? 1 : 0;

This patch clashes with the work of Michael Trimarchi who added high speed support somewhat different.
Look in the U-boot-usb git tree (next branch) where his patches are already pending for the next release.
(http://git.denx.de/?p=u-boot/u-boot-usb.git;a=shortlog;h=refs/heads/next)

include/usb.h enum { · line 142

Quoted 11 lines
common/usb.c +0 -0
       dev->children[port] = usb;
       usb->parent = dev;
include/usb.h +1 -0
index 510df95..e6aa551 100644
@@ -139,6 +139,7 @@ enum {
 struct usb_device {
       int     devnum;                 /* Device number on USB bus */
       int     slow;                   /* Slow device? */
+       int     high;                   /* High speed device? */

Same, Michael combined slow+high into 1 single element called 'speed', what I would prefer here.

Can you please rebase your work on the U-boot-usb tree, 'next' branch? And streamline these changes with the work Michael already has done?

Kind Regards,

Remy

Quoted 23 lines
include/usb.h +0 -0
       char    mf[32];                 /* manufacturer */
       char    prod[32];               /* product */
       char    serial[32];             /* serial number */
include/usb_defs.h +1 -1
index 353019f..500a0ad 100644
@@ -216,6 +216,7 @@
 #define USB_PORT_STAT_RESET         0x0010
 #define USB_PORT_STAT_POWER         0x0100
 #define USB_PORT_STAT_LOW_SPEED     0x0200
+#define USB_PORT_STAT_HIGH_SPEED    0x0400
 
 /* wPortChange bits */
 #define USB_PORT_STAT_C_CONNECTION  0x0001
--

1.5.6

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

#2 lore raw

PATCH 1/6 usb : musb : Add high speed field in usb_device structure

Hi Remy,

Quoted 6 lines

-----Original Message-----
From: l.pinguin at gmail.com [mailto:l.pinguin at gmail.com] On Behalf Of Remy Bohmer
Sent: Wednesday, December 17, 2008 1:48 AM
To: Abraham, Thomas

<snip>

Quoted 6 lines

Same, Michael combined slow+high into 1 single element called 'speed', what I would prefer here.

Can you please rebase your work on the U-boot-usb tree, 'next' branch? And streamline these changes with the work Michael already has done?

Ok. I will rebase with u-boot-usb next branch.

For mentor usb controller, it would still be necessary to record the speed of the device when it is found connected to a hub port. So the update of usb->speed would have to be done as below in 'usb_hub_port_connect_change' function. Would this be acceptable?

/* Allocate a new device struct for it */
usb = usb_alloc_new_device();
usb->speed = (portstatus & USB_PORT_STAT_LOW_SPEED) ? 1 : 0;
+usb->speed = (portstatus & USB_PORT_STAT_HIGH_SPEED) ? 1 : 0;

Quoted 4 lines

Kind Regards,

Remy

Thanks,
Thomas.

#3 lore raw

PATCH 1/6 usb : musb : Add high speed field in usb_device structure

Hello Michael,

Quoted 6 lines

For mentor usb controller, it would still be necessary to record the speed of the device when it is found connected to a hub port. So the update of usb->speed would have to be done as below in 'usb_hub_port_connect_change' function. Would this be acceptable?

/* Allocate a new device struct for it */
usb = usb_alloc_new_device();
usb->speed = (portstatus & USB_PORT_STAT_LOW_SPEED) ? 1 : 0;
+usb->speed = (portstatus & USB_PORT_STAT_HIGH_SPEED) ? 1 : 0;

Would this change break anything you are working on? Or do you have a better suggestion?

Kind Regards,

Remy

#4 lore raw

PATCH 1/6 usb : musb : Add high speed field in usb_device structure

In reply to Abraham
Quoted 21 lines · Thomas

Hi Remy,

Quoted 7 lines

-----Original Message-----
From: l.pinguin at gmail.com [mailto:l.pinguin at gmail.com] On Behalf Of Remy Bohmer
Sent: Wednesday, December 17, 2008 1:48 AM
To: Abraham, Thomas

<snip>

Quoted 6 lines

Same, Michael combined slow+high into 1 single element called 'speed', what I would prefer here.

Can you please rebase your work on the U-boot-usb tree, 'next' branch? And streamline these changes with the work Michael already has done?

Quoted 10 lines

Ok. I will rebase with u-boot-usb next branch.

For mentor usb controller, it would still be necessary to record the speed of the device when it is found connected to a hub port. So the update of usb->speed would have to be done as below in 'usb_hub_port_connect_change' function. Would this be acceptable?

/* Allocate a new device struct for it */
usb = usb_alloc_new_device();
usb->speed = (portstatus & USB_PORT_STAT_LOW_SPEED) ? 1 : 0;
+usb->speed = (portstatus & USB_PORT_STAT_HIGH_SPEED) ? 1 : 0;

I'll send a patch. I think that the correct way to do it is:
+
+ 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;
+ else
+ usb->speed = USB_SPEED_FULL;

Like linux does.

Quoted 10 lines
Quoted 4 lines

Kind Regards,

Remy

Thanks,
Thomas.

Regards Michael