USB-CDC: correct wrong alignment in ether.c

PATCH Superseded Stefano Babic 3 participants 9 messages Aug 14, 2010 – Aug 15, 2010 USB

Share & tools

Stefano Babic

author

2010-08-14 08:19 UTC #0 lore raw

PATCH v2 1/2 USB-CDC: correct wrong alignment in ether.c

The buffer for the status request must be word aligned because it is accessed with 32 bit pointer in the eth_status_complete function.

Signed-off-by Stefano Babic <[email protected]>

---
 drivers/usb/gadget/ether.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

drivers/usb/gadget/ether.c +1 -1

index 313f15f..0b4ed18 100644
@@ -684,7 +684,7 @@ static struct usb_gadget_strings	stringtab = {
 
 /*============================================================================*/
 static u8 control_req[USB_BUFSIZ];
-static u8 status_req[STATUS_BYTECOUNT];
+static u8 status_req[STATUS_BYTECOUNT] __attribute__ ((aligned(32)));

Signature · 1 line

1.6.3.3

Sergei Shtylyov

2010-08-14 12:23 UTC #1 lore raw

PATCH v2 1/2 USB-CDC: correct wrong alignment in ether.c

Hello.

Quoted 5 lines · Stefano Babic

The buffer for the status request must be word aligned because it is accessed with 32 bit pointer in the eth_status_complete function.
Signed-off-by Stefano Babic <[email protected]>

[...]

drivers/usb/gadget/ether.c static struct usb_gadget_strings stringtab = { · line 687

Quoted 10 lines

drivers/usb/gadget/ether.c +1 -1

index 313f15f..0b4ed18 100644
@@ -684,7 +684,7 @@ static struct usb_gadget_strings	stringtab = {
 
 /*============================================================================*/
 static u8 control_req[USB_BUFSIZ];
-static u8 status_req[STATUS_BYTECOUNT];
+static u8 status_req[STATUS_BYTECOUNT] __attribute__ ((aligned(32)));

You're aligning to 32 *bytes* -- is that what you meant by "32 bit pointer"?

WBR, Sergei

Remy Bohmer

2010-08-14 15:44 UTC #2 lore raw

PATCH v2 1/2 USB-CDC: correct wrong alignment in ether.c

Hi,

2010/8/14 Sergei Shtylyov <[email protected]>:

Quoted 18 lines

Hello.
Quoted 5 lines · Stefano Babic
The buffer for the status request must be word aligned because it is accessed with 32 bit pointer in the eth_status_complete function.
Signed-off-by Stefano Babic <[email protected]>
[...]
Quoted 7 lines
drivers/usb/gadget/ether.c +0 -0
index 313f15f..0b4ed18 100644
@@ -684,7 +684,7 @@ static struct usb_gadget_strings ? ?stringtab = {
 

Quoted 7 lines

drivers/usb/gadget/ether.c static struct usb_gadget_strings stringtab = { · line 687
Quoted 4 lines
?/*============================================================================*/ ?static u8 control_req[USB_BUFSIZ];
drivers/usb/gadget/ether.c +1 -1
-static u8 status_req[STATUS_BYTECOUNT];
+static u8 status_req[STATUS_BYTECOUNT] __attribute__ ((aligned(32)));
? You're aligning to 32 *bytes* -- is that what you meant by "32 bit pointer"?

Whoops... Good remark... Overlooked it myself...
Unapplied it...

Kind regards,

Remy

Stefano Babic

author

2010-08-15 12:16 UTC #3 lore raw

PATCH v2 1/2 USB-CDC: correct wrong alignment in ether.c

In reply to Sergei Shtylyov

Quoted 16 lines · Sergei Shtylyov

Hello.
Quoted 5 lines · Stefano Babic
The buffer for the status request must be word aligned because it is accessed with 32 bit pointer in the eth_status_complete function.
Signed-off-by Stefano Babic <[email protected]>
[...]
Quoted 6 lines
drivers/usb/gadget/ether.c +0 -0
index 313f15f..0b4ed18 100644
@@ -684,7 +684,7 @@ static struct usb_gadget_strings    stringtab = {
 

Quoted 8 lines

drivers/usb/gadget/ether.c static struct usb_gadget_strings stringtab = { · line 687
Quoted 5 lines
drivers/usb/gadget/ether.c +1 -1
 /*============================================================================*/
 
 static u8 control_req[USB_BUFSIZ];
-static u8 status_req[STATUS_BYTECOUNT];
+static u8 status_req[STATUS_BYTECOUNT] __attribute__ ((aligned(32)));
You're aligning to 32 *bytes* -- is that what you meant by "32 bit
pointer"?

Oooopsss ! Thanks, I fix it

Best regards,
Stefano

Signature · 5 lines

=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

Remy Bohmer

2010-08-14 15:22 UTC #4 lore raw

PATCH v2 1/2 USB-CDC: correct wrong alignment in ether.c

In reply to Stefano Babic

Hi,

2010/8/14 Stefano Babic <[email protected]>:

Quoted 8 lines

The buffer for the status request must be word aligned because it is accessed with 32 bit pointer in the eth_status_complete function.
Signed-off-by Stefano Babic <[email protected]>
---
?drivers/usb/gadget/ether.c | ? ?2 +-
?1 files changed, 1 insertions(+), 1 deletions(-)

Applied to u-boot-usb/cdc

Thanks.

Remy

Stefano Babic

author

2010-08-15 12:18 UTC #5 lore raw

PATCH v3 1/2 USB-CDC: correct wrong alignment in ether.c

In reply to Stefano Babic

The buffer for the status request must be word aligned because it is accessed with 32 bit pointer in the eth_status_complete function.

Signed-off-by Stefano Babic <[email protected]>

---
 drivers/usb/gadget/ether.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

drivers/usb/gadget/ether.c +1 -1

index c601d4a..9fc6a36 100644
@@ -677,7 +677,7 @@ static struct usb_gadget_strings	stringtab = {
 
 /*============================================================================*/
 static u8 control_req[USB_BUFSIZ];
-static u8 status_req[STATUS_BYTECOUNT];
+static u8 status_req[STATUS_BYTECOUNT] __attribute__ ((aligned(4)));

Signature · 1 line

1.6.3.3

Stefano Babic

author

2010-08-15 12:19 UTC #6 lore raw

PATCH v3 2/2 USB-CDC: called handle_interrupts inside usb_eth_send

The patch removes an endless loop in the usb_eth_send if the tx_complete is not called before going in the loop. The driver interrupt routine is called allowing the driver to check if the TX is completed.

Signed-off-by Stefano Babic <[email protected]>

---
 drivers/usb/gadget/ether.c |   10 +++++++++-
 1 files changed, 9 insertions(+), 1 deletions(-)

drivers/usb/gadget/ether.c +9 -1

index 9fc6a36..2965299 100644
@@ -1801,6 +1801,8 @@ static int usb_eth_send(struct eth_device* netdev, volatile void* packet, int le
 	int			retval;
 	struct usb_request	*req = NULL;
 	struct eth_dev		*dev = &l_ethdev;
+ 	unsigned long ts;
+ 	unsigned long timeout = USB_CONNECT_TIMEOUT;
 
 	debug("%s:...\n", __func__);

@@ -1826,6 +1828,8 @@ static int usb_eth_send(struct eth_device* netdev, volatile void* packet, int le
 			? ((dev->tx_qlen % qmult) != 0) : 0;
 #endif
 	dev->tx_qlen=1;
+ 	ts = get_timer(0);
+ 	packet_sent = 0;
 
 	retval = usb_ep_queue (dev->in_ep, req, GFP_ATOMIC);

@@ -1833,7 +1837,11 @@ static int usb_eth_send(struct eth_device* netdev, volatile void* packet, int le
 		debug("%s: packet queued\n", __func__);
 	while(!packet_sent)
 	{
-		packet_sent=0;
+ 		if (get_timer(ts) > timeout) {
+ 			printf("timeout sending packets to usb ethernet\n");
+ 			return -1;
+ 		}
+ 		usb_gadget_handle_interrupts();
 	}
 
 	return 0;

Signature · 1 line

1.6.3.3

Remy Bohmer

2010-08-15 16:17 UTC #7 lore raw

PATCH v3 2/2 USB-CDC: called handle_interrupts inside usb_eth_send

Hi,

2010/8/15 Stefano Babic <[email protected]>:

Quoted 9 lines

The patch removes an endless loop ?in the usb_eth_send if the tx_complete is not called before going in the loop. The driver interrupt routine is called allowing the driver to check if the TX is completed.
Signed-off-by Stefano Babic <[email protected]>
---
?drivers/usb/gadget/ether.c | ? 10 +++++++++- ?1 files changed, 9 insertions(+), 1 deletions(-)

Applied to u-boot-usb/cdc
Thanks.

Remy

Remy Bohmer

2010-08-15 16:18 UTC #8 lore raw

PATCH v3 1/2 USB-CDC: correct wrong alignment in ether.c

In reply to Stefano Babic

Hi,

2010/8/15 Stefano Babic <[email protected]>:

Quoted 8 lines

The buffer for the status request must be word aligned because it is accessed with 32 bit pointer in the eth_status_complete function.
Signed-off-by Stefano Babic <[email protected]>
---
?drivers/usb/gadget/ether.c | ? ?2 +-
?1 files changed, 1 insertions(+), 1 deletions(-)

Applied to u-boot-usb/cdc
Thanks.

Remy