Re: [U-Boot] [PATCH 4/4] USB:gadget:designware Fix memory nonalignment issue

[Amit Virdi]

Dear Marek,

On 3/5/2012 11:51 PM, Marek Vasut wrote:

Dear Amit Virdi,


From: Shiraz Hashimshiraz.has...@st.com

While receiving packets from FIFO sometimes the buffer provided was
nonaligned. Fix this by taking a temporary aligned buffer and then
copying the content to nonaligned buffer.

Signed-off-by: Shiraz Hashimshiraz.has...@st.com
Signed-off-by: Amit Virdiamit.vi...@st.com
---
  drivers/usb/gadget/designware_udc.c |   15 ++-
  1 files changed, 14 insertions(+), 1 deletions(-)

diff --git a/drivers/usb/gadget/designware_udc.c
b/drivers/usb/gadget/designware_udc.c index d4b53a2..878123c 100644
--- a/drivers/usb/gadget/designware_udc.c
+++ b/drivers/usb/gadget/designware_udc.c
@@ -202,6 +202,7 @@ static int usbgetpckfromfifo(int epNum, u8 *bufp, u32
len) u32 i, nw, nb;
u32 *wrdp;
u8 *bytp;
+   u32 tmp[128];

if (readl(udc_regs_p-dev_stat)  DEV_STAT_RXFIFO_EMPTY)
return -1;
@@ -209,7 +210,12 @@ static int usbgetpckfromfifo(int epNum, u8 *bufp, u32
len) nw = len / sizeof(u32);
nb = len % sizeof(u32);

-   wrdp = (u32 *)bufp;
+   /* use tmp buf if bufp is not word aligned */
+   if ((int)bufp  0x3)
+   wrdp = (u32 *)tmp[0];
+   else
+   wrdp = (u32 *)bufp;
+
for (i = 0; i  nw; i++) {
writel(readl(fifo_ptr), wrdp);
wrdp++;
@@ -223,6 +229,13 @@ static int usbgetpckfromfifo(int epNum, u8 *bufp, u32
len) }
readl(outep_regs_p[epNum].write_done);

+   /* copy back tmp buffer to bufp if bufp is not word aligned */
+   if ((int)bufp  0x3) {
+   bytp = (u8 *)tmp[0];
+   for (i = 0; i  len; i++)
+   bufp[i] = bytp[i];
+   }
+
return 0;
  }


This addresses EHCI cache problem, that's why you need bounce buffer, right?



No. The problem was we were copying data word-by-word to a non-word 
aligned memory in the USB gadget. So, this is different from the USB 
host controller issue.



Thanks n Regards
Amit Virdi


This is the patch from Puneet, can you give it a go and tell me if EHCI works
for you without 4/4 patch from your series or do you still see issues?

http://permalink.gmane.org/gmane.comp.boot-loaders.u-boot/126447

Thanks in advance!

Best regards,
Marek Vasut
.


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

Re: [U-Boot] [PATCH 4/4] USB:gadget:designware Fix memory nonalignment issue

[Marek Vasut]

Dear Amit Virdi,

 Dear Marek,
 
 On 3/5/2012 11:51 PM, Marek Vasut wrote:
  Dear Amit Virdi,
  
  From: Shiraz Hashimshiraz.has...@st.com
  
  While receiving packets from FIFO sometimes the buffer provided was
  nonaligned. Fix this by taking a temporary aligned buffer and then
  copying the content to nonaligned buffer.
  
  Signed-off-by: Shiraz Hashimshiraz.has...@st.com
  Signed-off-by: Amit Virdiamit.vi...@st.com
  ---
  
drivers/usb/gadget/designware_udc.c |   15 ++-
1 files changed, 14 insertions(+), 1 deletions(-)
  
  diff --git a/drivers/usb/gadget/designware_udc.c
  b/drivers/usb/gadget/designware_udc.c index d4b53a2..878123c 100644
  --- a/drivers/usb/gadget/designware_udc.c
  +++ b/drivers/usb/gadget/designware_udc.c
  @@ -202,6 +202,7 @@ static int usbgetpckfromfifo(int epNum, u8 *bufp,
  u32 len) u32 i, nw, nb;
  
 u32 *wrdp;
 u8 *bytp;
  
  +  u32 tmp[128];
  
 if (readl(udc_regs_p-dev_stat)  DEV_STAT_RXFIFO_EMPTY)
 
 return -1;
  
  @@ -209,7 +210,12 @@ static int usbgetpckfromfifo(int epNum, u8 *bufp,
  u32 len) nw = len / sizeof(u32);
  
 nb = len % sizeof(u32);
  
  -  wrdp = (u32 *)bufp;
  +  /* use tmp buf if bufp is not word aligned */
  +  if ((int)bufp  0x3)
  +  wrdp = (u32 *)tmp[0];
  +  else
  +  wrdp = (u32 *)bufp;
  +
  
 for (i = 0; i  nw; i++) {
 
 writel(readl(fifo_ptr), wrdp);
 wrdp++;
  
  @@ -223,6 +229,13 @@ static int usbgetpckfromfifo(int epNum, u8 *bufp,
  u32 len) }
  
 readl(outep_regs_p[epNum].write_done);
  
  +  /* copy back tmp buffer to bufp if bufp is not word aligned */
  +  if ((int)bufp  0x3) {
  +  bytp = (u8 *)tmp[0];
  +  for (i = 0; i  len; i++)
  +  bufp[i] = bytp[i];
  +  }
  +
  
 return 0;

}
  
  This addresses EHCI cache problem, that's why you need bounce buffer,
  right?
 
 No. The problem was we were copying data word-by-word to a non-word
 aligned memory in the USB gadget. So, this is different from the USB
 host controller issue.
 

I see ... why isn't buffer aligned by the usb stack then?

Thanks!

 
 Thanks n Regards
 Amit Virdi
 
  This is the patch from Puneet, can you give it a go and tell me if EHCI
  works for you without 4/4 patch from your series or do you still see
  issues?
  
  http://permalink.gmane.org/gmane.comp.boot-loaders.u-boot/126447
  
  Thanks in advance!
  
  Best regards,
  Marek Vasut
  .
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 4/4] USB:gadget:designware Fix memory nonalignment issue

[Mike Frysinger]

On Tuesday 06 March 2012 04:51:57 Marek Vasut wrote:
  On 3/5/2012 11:51 PM, Marek Vasut wrote:
   Amit Virdi wrote:
   While receiving packets from FIFO sometimes the buffer provided was
   nonaligned. Fix this by taking a temporary aligned buffer and then
   copying the content to nonaligned buffer.
   
   --- a/drivers/usb/gadget/designware_udc.c
   +++ b/drivers/usb/gadget/designware_udc.c
   @@ -202,6 +202,7 @@ static int usbgetpckfromfifo(int epNum, u8 *bufp,
   u32 len) u32 i, nw, nb;
   
u32 *wrdp;
u8 *bytp;
   
   +u32 tmp[128];
   
if (readl(udc_regs_p-dev_stat)  DEV_STAT_RXFIFO_EMPTY)

return -1;
   
   @@ -209,7 +210,12 @@ static int usbgetpckfromfifo(int epNum, u8 *bufp,
   u32 len) nw = len / sizeof(u32);
   
nb = len % sizeof(u32);
   
   -wrdp = (u32 *)bufp;
   +/* use tmp buf if bufp is not word aligned */
   +if ((int)bufp  0x3)
   +wrdp = (u32 *)tmp[0];
   +else
   +wrdp = (u32 *)bufp;
   +
   
for (i = 0; i  nw; i++) {

writel(readl(fifo_ptr), wrdp);
wrdp++;
   
   @@ -223,6 +229,13 @@ static int usbgetpckfromfifo(int epNum, u8 *bufp,
   u32 len) }
   
readl(outep_regs_p[epNum].write_done);
   
   +/* copy back tmp buffer to bufp if bufp is not word aligned */
   +if ((int)bufp  0x3) {
   +bytp = (u8 *)tmp[0];
   +for (i = 0; i  len; i++)
   +bufp[i] = bytp[i];
   +}
   +
   
return 0;
 
 }
   
   This addresses EHCI cache problem, that's why you need bounce buffer,
   right?
  
  No. The problem was we were copying data word-by-word to a non-word
  aligned memory in the USB gadget. So, this is different from the USB
  host controller issue.
 
 I see ... why isn't buffer aligned by the usb stack then?

because it might not be a requirement higher up.  i.e. it's dealing with a 
data byte stream.  forcing all higher layers to use 32bit alignment because 
this host controller requires 32bit alignment in its FIFOs is incorrect.
-mike


signature.asc
Description: This is a digitally signed message part.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 4/4] USB:gadget:designware Fix memory nonalignment issue

[Mike Frysinger]

On Thursday 16 February 2012 07:03:38 Amit Virdi wrote:
 --- a/drivers/usb/gadget/designware_udc.c
 +++ b/drivers/usb/gadget/designware_udc.c
 
 + /* copy back tmp buffer to bufp if bufp is not word aligned */
 + if ((int)bufp  0x3) {
 + bytp = (u8 *)tmp[0];
 + for (i = 0; i  len; i++)
 + bufp[i] = bytp[i];
 + }

memcpy(bufp, tmp, len) ?
-mike


signature.asc
Description: This is a digitally signed message part.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 4/4] USB:gadget:designware Fix memory nonalignment issue

[Amit Virdi]

Hey Mike,

On 3/6/2012 9:39 PM, Mike Frysinger wrote:

On Thursday 16 February 2012 07:03:38 Amit Virdi wrote:

--- a/drivers/usb/gadget/designware_udc.c
+++ b/drivers/usb/gadget/designware_udc.c

+   /* copy back tmp buffer to bufp if bufp is not word aligned */
+   if ((int)bufp  0x3) {
+   bytp = (u8 *)tmp[0];
+   for (i = 0; i  len; i++)
+   bufp[i] = bytp[i];
+   }


memcpy(bufp, tmp, len) ?


Yes, memcpy can be used. I'll amend the code.

Thanks
Amit Virdi
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 4/4] USB:gadget:designware Fix memory nonalignment issue

[Amit Virdi]

Dear Marek,


   }


This addresses EHCI cache problem, that's why you need bounce buffer,
right?


No. The problem was we were copying data word-by-word to a non-word
aligned memory in the USB gadget. So, this is different from the USB
host controller issue.


I see ... why isn't buffer aligned by the usb stack then?


because it might not be a requirement higher up.  i.e. it's dealing with a
data byte stream.  forcing all higher layers to use 32bit alignment because
this host controller requires 32bit alignment in its FIFOs is incorrect.


The usb gadget is used by the usbtty so we can receive any number of 
bytes and, as Mike explains, we cannot force 32-bit alignment.


Thanks
Amit Virdi
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 4/4] USB:gadget:designware Fix memory nonalignment issue

[Marek Vasut]

Dear Amit Virdi,

 From: Shiraz Hashim shiraz.has...@st.com
 
 While receiving packets from FIFO sometimes the buffer provided was
 nonaligned. Fix this by taking a temporary aligned buffer and then
 copying the content to nonaligned buffer.
 
 Signed-off-by: Shiraz Hashim shiraz.has...@st.com
 Signed-off-by: Amit Virdi amit.vi...@st.com
 ---
  drivers/usb/gadget/designware_udc.c |   15 ++-
  1 files changed, 14 insertions(+), 1 deletions(-)
 
 diff --git a/drivers/usb/gadget/designware_udc.c
 b/drivers/usb/gadget/designware_udc.c index d4b53a2..878123c 100644
 --- a/drivers/usb/gadget/designware_udc.c
 +++ b/drivers/usb/gadget/designware_udc.c
 @@ -202,6 +202,7 @@ static int usbgetpckfromfifo(int epNum, u8 *bufp, u32
 len) u32 i, nw, nb;
   u32 *wrdp;
   u8 *bytp;
 + u32 tmp[128];
 
   if (readl(udc_regs_p-dev_stat)  DEV_STAT_RXFIFO_EMPTY)
   return -1;
 @@ -209,7 +210,12 @@ static int usbgetpckfromfifo(int epNum, u8 *bufp, u32
 len) nw = len / sizeof(u32);
   nb = len % sizeof(u32);
 
 - wrdp = (u32 *)bufp;
 + /* use tmp buf if bufp is not word aligned */
 + if ((int)bufp  0x3)
 + wrdp = (u32 *)tmp[0];
 + else
 + wrdp = (u32 *)bufp;
 +
   for (i = 0; i  nw; i++) {
   writel(readl(fifo_ptr), wrdp);
   wrdp++;
 @@ -223,6 +229,13 @@ static int usbgetpckfromfifo(int epNum, u8 *bufp, u32
 len) }
   readl(outep_regs_p[epNum].write_done);
 
 + /* copy back tmp buffer to bufp if bufp is not word aligned */
 + if ((int)bufp  0x3) {
 + bytp = (u8 *)tmp[0];
 + for (i = 0; i  len; i++)
 + bufp[i] = bytp[i];
 + }
 +
   return 0;
  }

This addresses EHCI cache problem, that's why you need bounce buffer, right? 

This is the patch from Puneet, can you give it a go and tell me if EHCI works 
for you without 4/4 patch from your series or do you still see issues?

http://permalink.gmane.org/gmane.comp.boot-loaders.u-boot/126447

Thanks in advance!

Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 4/4] USB:gadget:designware Fix memory nonalignment issue

[Amit Virdi]

From: Shiraz Hashim shiraz.has...@st.com

While receiving packets from FIFO sometimes the buffer provided was
nonaligned. Fix this by taking a temporary aligned buffer and then
copying the content to nonaligned buffer.

Signed-off-by: Shiraz Hashim shiraz.has...@st.com
Signed-off-by: Amit Virdi amit.vi...@st.com
---
 drivers/usb/gadget/designware_udc.c |   15 ++-
 1 files changed, 14 insertions(+), 1 deletions(-)

diff --git a/drivers/usb/gadget/designware_udc.c 
b/drivers/usb/gadget/designware_udc.c
index d4b53a2..878123c 100644
--- a/drivers/usb/gadget/designware_udc.c
+++ b/drivers/usb/gadget/designware_udc.c
@@ -202,6 +202,7 @@ static int usbgetpckfromfifo(int epNum, u8 *bufp, u32 len)
u32 i, nw, nb;
u32 *wrdp;
u8 *bytp;
+   u32 tmp[128];
 
if (readl(udc_regs_p-dev_stat)  DEV_STAT_RXFIFO_EMPTY)
return -1;
@@ -209,7 +210,12 @@ static int usbgetpckfromfifo(int epNum, u8 *bufp, u32 len)
nw = len / sizeof(u32);
nb = len % sizeof(u32);
 
-   wrdp = (u32 *)bufp;
+   /* use tmp buf if bufp is not word aligned */
+   if ((int)bufp  0x3)
+   wrdp = (u32 *)tmp[0];
+   else
+   wrdp = (u32 *)bufp;
+
for (i = 0; i  nw; i++) {
writel(readl(fifo_ptr), wrdp);
wrdp++;
@@ -223,6 +229,13 @@ static int usbgetpckfromfifo(int epNum, u8 *bufp, u32 len)
}
readl(outep_regs_p[epNum].write_done);
 
+   /* copy back tmp buffer to bufp if bufp is not word aligned */
+   if ((int)bufp  0x3) {
+   bytp = (u8 *)tmp[0];
+   for (i = 0; i  len; i++)
+   bufp[i] = bytp[i];
+   }
+
return 0;
 }
 
-- 
1.7.2.2

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