Re: [PATCH v2] usb: fsl_udc: errata - postpone freeing current dTD

2012-06-12 Thread Christoph Fritz
Hi Fabio

On Sun, 2012-06-10 at 15:41 -0300, Fabio Estevam wrote:
 Hi Christoph,
 
 On Mon, Jun 4, 2012 at 8:37 AM, Christoph Fritz
 chf.fr...@googlemail.com wrote:
 
  After that, I stumbled upon this dmesg:
 
  Freescale High-Speed USB SOC Device Controller driver (Apr 20, 2007)
  fsl-usb2-udc fsl-usb2-udc: clk_get(usb) failed
  fsl-usb2-udc: probe of fsl-usb2-udc failed with error -2
 
  Sascha, could you give me a hint?
 
 Does the patch below fix your problem?

Thanks for your patch. It does indeed load
Freescale High-Speed USB SOC Device Controller driver (Apr 20, 2007)
fine - but now when I want to use it:

modprobe g_ether
[   17.099363] g_ether gadget: using random self ethernet address
[   17.105316] g_ether gadget: using random host ethernet address
[   17.111974] usb0: MAC 1a:c7:9e:76:cc:45
[   17.115866] usb0: HOST MAC 66:a2:4a:0a:46:17
[   17.120199] g_ether gadget: Ethernet Gadget, version: Memorial Day 2008
[   17.126861] g_ether gadget: g_ether ready

these are the last words: System hangs completely. Same behavior with
g_serial (these two I've tested).

Currently I can't investigate this further, not because of a bug - but a
flu.


Thanks,
 -- Christoph

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH v2] usb: fsl_udc: errata - postpone freeing current dTD

2012-06-12 Thread Fabio Estevam
On Tue, Jun 12, 2012 at 4:40 PM, Christoph Fritz
chf.fr...@googlemail.com wrote:

 Thanks for your patch. It does indeed load
 Freescale High-Speed USB SOC Device Controller driver (Apr 20, 2007)
 fine - but now when I want to use it:

 modprobe g_ether
 [   17.099363] g_ether gadget: using random self ethernet address
 [   17.105316] g_ether gadget: using random host ethernet address
 [   17.111974] usb0: MAC 1a:c7:9e:76:cc:45
 [   17.115866] usb0: HOST MAC 66:a2:4a:0a:46:17
 [   17.120199] g_ether gadget: Ethernet Gadget, version: Memorial Day 2008
 [   17.126861] g_ether gadget: g_ether ready

 these are the last words: System hangs completely. Same behavior with
 g_serial (these two I've tested).

Ok, I just sent a new patch to the linux-usb mailing list.

Tested it on a mx31 and mx51 and it probed g_ether fine. I don't have
a mx35 handy.


 Currently I can't investigate this further, not because of a bug - but a
 flu.

Hope you get better soon.

Regards,

Fabio Estevam
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH v2] usb: fsl_udc: errata - postpone freeing current dTD

2012-06-10 Thread Fabio Estevam
Hi Christoph,

On Mon, Jun 4, 2012 at 8:37 AM, Christoph Fritz
chf.fr...@googlemail.com wrote:

 After that, I stumbled upon this dmesg:

 Freescale High-Speed USB SOC Device Controller driver (Apr 20, 2007)
 fsl-usb2-udc fsl-usb2-udc: clk_get(usb) failed
 fsl-usb2-udc: probe of fsl-usb2-udc failed with error -2

 Sascha, could you give me a hint?

Does the patch below fix your problem?

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

diff --git a/drivers/usb/gadget/fsl_mxc_udc.c b/drivers/usb/gadget/fsl_mxc_udc.c
index dcbc0a2..c3ade6b 100644
--- a/drivers/usb/gadget/fsl_mxc_udc.c
+++ b/drivers/usb/gadget/fsl_mxc_udc.c
@@ -36,21 +36,21 @@ int fsl_udc_clk_init(struct platform_device *pdev)
pdata = pdev-dev.platform_data;

if (!cpu_is_mx35()  !cpu_is_mx25()) {
-   mxc_ahb_clk = clk_get(pdev-dev, usb_ahb);
+   mxc_ahb_clk = clk_get(pdev-dev, ipg);
if (IS_ERR(mxc_ahb_clk))
return PTR_ERR(mxc_ahb_clk);

-   ret = clk_enable(mxc_ahb_clk);
+   ret = clk_prepare_enable(mxc_ahb_clk);
if (ret  0) {
-   dev_err(pdev-dev, clk_enable(\usb_ahb\) failed\n);
+   dev_err(pdev-dev, clk_enable(\ipg\) failed\n);
goto eenahb;
}
}

/* make sure USB_CLK is running at 60 MHz +/- 1000 Hz */
-   mxc_usb_clk = clk_get(pdev-dev, usb);
+   mxc_usb_clk = clk_get(pdev-dev, per);
if (IS_ERR(mxc_usb_clk)) {
-   dev_err(pdev-dev, clk_get(\usb\) failed\n);
+   dev_err(pdev-dev, clk_get(\per\) failed\n);
ret = PTR_ERR(mxc_usb_clk);
goto egusb;
}
@@ -65,7 +65,7 @@ int fsl_udc_clk_init(struct platform_device *pdev)
}
}

-   ret = clk_enable(mxc_usb_clk);
+   ret = clk_prepare_enable(mxc_usb_clk);
if (ret  0) {
dev_err(pdev-dev, clk_enable(\usb_clk\) failed\n);
goto eenusb;
@@ -79,7 +79,7 @@ eclkrate:
mxc_usb_clk = NULL;
 egusb:
if (!cpu_is_mx35())
-   clk_disable(mxc_ahb_clk);
+   clk_disable_unprepare(mxc_ahb_clk);
 eenahb:
if (!cpu_is_mx35())
clk_put(mxc_ahb_clk);
@@ -104,7 +104,7 @@ void fsl_udc_clk_finalize(struct platform_device *pdev)

/* ULPI transceivers don't need usbpll */
if (pdata-phy_mode == FSL_USB2_PHY_ULPI) {
-   clk_disable(mxc_usb_clk);
+   clk_disable_unprepare(mxc_usb_clk);
clk_put(mxc_usb_clk);
mxc_usb_clk = NULL;
}
@@ -113,11 +113,11 @@ void fsl_udc_clk_finalize(struct platform_device *pdev)
 void fsl_udc_clk_release(void)
 {
if (mxc_usb_clk) {
-   clk_disable(mxc_usb_clk);
+   clk_disable_unprepare(mxc_usb_clk);
clk_put(mxc_usb_clk);
}
if (!cpu_is_mx35()) {
-   clk_disable(mxc_ahb_clk);
+   clk_disable_unprepare(mxc_ahb_clk);
clk_put(mxc_ahb_clk);
}
 }
--
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH v2] usb: fsl_udc: errata - postpone freeing current dTD

2012-06-04 Thread Christoph Fritz
Hi,

On Mon, 2012-05-21 at 22:04 +0300, Felipe Balbi wrote:
 On Mon, May 21, 2012 at 08:57:22AM +0200, Christoph Fritz wrote:
  USB controller may access a wrong address for the dTD (endpoint transfer
  descriptor) and then hang. This happens a lot when doing tests with
  g_ether module and iperf, a tool for measuring maximum TCP and UDP
  bandwidth.
  
  This hardware bug is explained in detail by errata number 2858 for i.MX23:
  http://cache.freescale.com/files/dsp/doc/errata/IMX23CE.pdf
  
  All (?) SOCs with an IP from chipidea suffer from this problem.
  mv_udc_core fixes this bug by commit daec765.  There still may be
  unfixed drivers.
  
  Signed-off-by: Christoph Fritz chf.fr...@googlemail.com
  Signed-off-by: Christian Hemp c.h...@phytec.de
  ---
   drivers/usb/gadget/fsl_udc_core.c |   15 ++-
   1 files changed, 14 insertions(+), 1 deletions(-)
  
  diff --git a/drivers/usb/gadget/fsl_udc_core.c 
  b/drivers/usb/gadget/fsl_udc_core.c
  index 55abfb6..72f2139 100644
  --- a/drivers/usb/gadget/fsl_udc_core.c
  +++ b/drivers/usb/gadget/fsl_udc_core.c
  @@ -65,6 +65,8 @@ static struct usb_sys_interface *usb_sys_regs;
   /* it is initialized in probe()  */
   static struct fsl_udc *udc_controller = NULL;
   
  +static struct ep_td_struct *last_free_td;
 
 I don't want to see global variables anymore. In fact, please convert
 this to the new udc_start()/udc_stop() calls and use the generic
 map/unmap routines.
 
 That'll help you get rid of a bunch of useless code on the driver. After
 that you should remove all asm/* header includes and drop the ARCH
 dependency.
 
 You can also drop the big-/little-endian helpers as you can make use of
 generic writel()/readl() routines.
 
 Please make sure these series comes in with enough time to reach v3.6
 merge window in about 3 months.
 
 You can put this fix together on that series after you drop the global.

Before I came to do the proposed changes, I stumbled upon this:

In file included from drivers/usb/gadget/fsl_udc_core.c:49:
drivers/usb/gadget/fsl_usb2_udc.h: In function ‘get_qh_by_ep’:
drivers/usb/gadget/fsl_usb2_udc.h:585: error: ‘struct fsl_ep’ has no member 
named ‘desc’
drivers/usb/gadget/fsl_udc_core.c: In function ‘done’:
drivers/usb/gadget/fsl_udc_core.c:187: error: ‘struct fsl_ep’ has no member 
named ‘desc’
drivers/usb/gadget/fsl_udc_core.c:187: error: ‘struct fsl_ep’ has no member 
named ‘desc’
snip

my proposed regression patch:
---
From: Christoph Fritz chf.fr...@googlemail.com
Date: Mon, 4 Jun 2012 12:58:21 +0200
Subject: [PATCH] usb: gadget: regression fix - useage of usb_ep

This patch removes redundant pointer to struct usb_endpoint_descriptor which
were missed in commit 79149b8:

 usb: gadget: Update fsl_udc_core to use usb_endpoint_descriptor inside the
 struct usb_ep

Signed-off-by: Christoph Fritz chf.fr...@googlemail.com
---
 drivers/usb/gadget/fsl_udc_core.c |2 +-
 drivers/usb/gadget/fsl_usb2_udc.h |4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/gadget/fsl_udc_core.c 
b/drivers/usb/gadget/fsl_udc_core.c
index 2831685..678ec4d 100644
--- a/drivers/usb/gadget/fsl_udc_core.c
+++ b/drivers/usb/gadget/fsl_udc_core.c
@@ -2575,7 +2575,7 @@ static int __init fsl_udc_probe(struct platform_device 
*pdev)
/* for ep0: the desc defined here;
 * for other eps, gadget layer called ep_enable with defined desc
 */
-   udc_controller-eps[0].desc = fsl_ep0_desc;
+   udc_controller-eps[0].ep.desc = fsl_ep0_desc;
udc_controller-eps[0].ep.maxpacket = USB_MAX_CTRL_PAYLOAD;
 
/* setup the udc-eps[] for non-control endpoints and link
diff --git a/drivers/usb/gadget/fsl_usb2_udc.h 
b/drivers/usb/gadget/fsl_usb2_udc.h
index 5cd7b7e..f61a967 100644
--- a/drivers/usb/gadget/fsl_usb2_udc.h
+++ b/drivers/usb/gadget/fsl_usb2_udc.h
@@ -568,10 +568,10 @@ static void dump_msg(const char *label, const u8 * buf, 
unsigned int length)
 /*
  * ### internal used help routines.
  */
-#define ep_index(EP)   ((EP)-desc-bEndpointAddress0xF)
+#define ep_index(EP)   ((EP)-ep.desc-bEndpointAddress0xF)
 #define ep_maxpacket(EP)   ((EP)-ep.maxpacket)
 #define ep_is_in(EP)   ( (ep_index(EP) == 0) ? (EP-udc-ep0_dir == \
-   USB_DIR_IN ):((EP)-desc-bEndpointAddress \
+   USB_DIR_IN) : ((EP)-ep.desc-bEndpointAddress \
 USB_DIR_IN)==USB_DIR_IN)
 #define get_ep_by_pipe(udc, pipe)  ((pipe == 1)? udc-eps[0]: \
udc-eps[pipe])
-- 
1.7.2.5





___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH v2] usb: fsl_udc: errata - postpone freeing current dTD

2012-06-04 Thread Christoph Fritz
On Mon, 2012-06-04 at 13:30 +0200, Christoph Fritz wrote:
 Hi,
 
 On Mon, 2012-05-21 at 22:04 +0300, Felipe Balbi wrote:
  On Mon, May 21, 2012 at 08:57:22AM +0200, Christoph Fritz wrote:
   USB controller may access a wrong address for the dTD (endpoint transfer
   descriptor) and then hang. This happens a lot when doing tests with
   g_ether module and iperf, a tool for measuring maximum TCP and UDP
   bandwidth.
   
   This hardware bug is explained in detail by errata number 2858 for i.MX23:
   http://cache.freescale.com/files/dsp/doc/errata/IMX23CE.pdf
   
   All (?) SOCs with an IP from chipidea suffer from this problem.
   mv_udc_core fixes this bug by commit daec765.  There still may be
   unfixed drivers.
   
   Signed-off-by: Christoph Fritz chf.fr...@googlemail.com
   Signed-off-by: Christian Hemp c.h...@phytec.de
   ---
drivers/usb/gadget/fsl_udc_core.c |   15 ++-
1 files changed, 14 insertions(+), 1 deletions(-)
   
   diff --git a/drivers/usb/gadget/fsl_udc_core.c 
   b/drivers/usb/gadget/fsl_udc_core.c
   index 55abfb6..72f2139 100644
   --- a/drivers/usb/gadget/fsl_udc_core.c
   +++ b/drivers/usb/gadget/fsl_udc_core.c
   @@ -65,6 +65,8 @@ static struct usb_sys_interface *usb_sys_regs;
/* it is initialized in probe()  */
static struct fsl_udc *udc_controller = NULL;

   +static struct ep_td_struct *last_free_td;
  
  I don't want to see global variables anymore. In fact, please convert
  this to the new udc_start()/udc_stop() calls and use the generic
  map/unmap routines.
  
  That'll help you get rid of a bunch of useless code on the driver. After
  that you should remove all asm/* header includes and drop the ARCH
  dependency.
  
  You can also drop the big-/little-endian helpers as you can make use of
  generic writel()/readl() routines.
  
  Please make sure these series comes in with enough time to reach v3.6
  merge window in about 3 months.
  
  You can put this fix together on that series after you drop the global.
 
 Before I came to do the proposed changes, I stumbled upon this:
 
 In file included from drivers/usb/gadget/fsl_udc_core.c:49:
 drivers/usb/gadget/fsl_usb2_udc.h: In function ‘get_qh_by_ep’:
 drivers/usb/gadget/fsl_usb2_udc.h:585: error: ‘struct fsl_ep’ has no member 
 named ‘desc’
 drivers/usb/gadget/fsl_udc_core.c: In function ‘done’:
 drivers/usb/gadget/fsl_udc_core.c:187: error: ‘struct fsl_ep’ has no member 
 named ‘desc’
 drivers/usb/gadget/fsl_udc_core.c:187: error: ‘struct fsl_ep’ has no member 
 named ‘desc’
 snip
 
 my proposed regression patch:
 ---
 From: Christoph Fritz chf.fr...@googlemail.com
 Date: Mon, 4 Jun 2012 12:58:21 +0200
 Subject: [PATCH] usb: gadget: regression fix - useage of usb_ep
snip

After that, I stumbled upon this dmesg:

Freescale High-Speed USB SOC Device Controller driver (Apr 20, 2007)
fsl-usb2-udc fsl-usb2-udc: clk_get(usb) failed
fsl-usb2-udc: probe of fsl-usb2-udc failed with error -2

Sascha, could you give me a hint?

 Thanks,
  -- Christoph


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH v2] usb: fsl_udc: errata - postpone freeing current dTD

2012-06-04 Thread Felipe Balbi
On Mon, Jun 04, 2012 at 01:30:24PM +0200, Christoph Fritz wrote:
 Hi,
 
 On Mon, 2012-05-21 at 22:04 +0300, Felipe Balbi wrote:
  On Mon, May 21, 2012 at 08:57:22AM +0200, Christoph Fritz wrote:
   USB controller may access a wrong address for the dTD (endpoint transfer
   descriptor) and then hang. This happens a lot when doing tests with
   g_ether module and iperf, a tool for measuring maximum TCP and UDP
   bandwidth.
   
   This hardware bug is explained in detail by errata number 2858 for i.MX23:
   http://cache.freescale.com/files/dsp/doc/errata/IMX23CE.pdf
   
   All (?) SOCs with an IP from chipidea suffer from this problem.
   mv_udc_core fixes this bug by commit daec765.  There still may be
   unfixed drivers.
   
   Signed-off-by: Christoph Fritz chf.fr...@googlemail.com
   Signed-off-by: Christian Hemp c.h...@phytec.de
   ---
drivers/usb/gadget/fsl_udc_core.c |   15 ++-
1 files changed, 14 insertions(+), 1 deletions(-)
   
   diff --git a/drivers/usb/gadget/fsl_udc_core.c 
   b/drivers/usb/gadget/fsl_udc_core.c
   index 55abfb6..72f2139 100644
   --- a/drivers/usb/gadget/fsl_udc_core.c
   +++ b/drivers/usb/gadget/fsl_udc_core.c
   @@ -65,6 +65,8 @@ static struct usb_sys_interface *usb_sys_regs;
/* it is initialized in probe()  */
static struct fsl_udc *udc_controller = NULL;

   +static struct ep_td_struct *last_free_td;
  
  I don't want to see global variables anymore. In fact, please convert
  this to the new udc_start()/udc_stop() calls and use the generic
  map/unmap routines.
  
  That'll help you get rid of a bunch of useless code on the driver. After
  that you should remove all asm/* header includes and drop the ARCH
  dependency.
  
  You can also drop the big-/little-endian helpers as you can make use of
  generic writel()/readl() routines.
  
  Please make sure these series comes in with enough time to reach v3.6
  merge window in about 3 months.
  
  You can put this fix together on that series after you drop the global.
 
 Before I came to do the proposed changes, I stumbled upon this:
 
 In file included from drivers/usb/gadget/fsl_udc_core.c:49:
 drivers/usb/gadget/fsl_usb2_udc.h: In function ‘get_qh_by_ep’:
 drivers/usb/gadget/fsl_usb2_udc.h:585: error: ‘struct fsl_ep’ has no member 
 named ‘desc’
 drivers/usb/gadget/fsl_udc_core.c: In function ‘done’:
 drivers/usb/gadget/fsl_udc_core.c:187: error: ‘struct fsl_ep’ has no member 
 named ‘desc’
 drivers/usb/gadget/fsl_udc_core.c:187: error: ‘struct fsl_ep’ has no member 
 named ‘desc’
 snip
 
 my proposed regression patch:
 ---
 From: Christoph Fritz chf.fr...@googlemail.com
 Date: Mon, 4 Jun 2012 12:58:21 +0200
 Subject: [PATCH] usb: gadget: regression fix - useage of usb_ep
 
 This patch removes redundant pointer to struct usb_endpoint_descriptor which
 were missed in commit 79149b8:
 
  usb: gadget: Update fsl_udc_core to use usb_endpoint_descriptor inside the
  struct usb_ep
 
 Signed-off-by: Christoph Fritz chf.fr...@googlemail.com
 ---
  drivers/usb/gadget/fsl_udc_core.c |2 +-
  drivers/usb/gadget/fsl_usb2_udc.h |4 ++--
  2 files changed, 3 insertions(+), 3 deletions(-)
 
 diff --git a/drivers/usb/gadget/fsl_udc_core.c 
 b/drivers/usb/gadget/fsl_udc_core.c
 index 2831685..678ec4d 100644
 --- a/drivers/usb/gadget/fsl_udc_core.c
 +++ b/drivers/usb/gadget/fsl_udc_core.c
 @@ -2575,7 +2575,7 @@ static int __init fsl_udc_probe(struct platform_device 
 *pdev)
   /* for ep0: the desc defined here;
* for other eps, gadget layer called ep_enable with defined desc
*/
 - udc_controller-eps[0].desc = fsl_ep0_desc;
 + udc_controller-eps[0].ep.desc = fsl_ep0_desc;
   udc_controller-eps[0].ep.maxpacket = USB_MAX_CTRL_PAYLOAD;
  
   /* setup the udc-eps[] for non-control endpoints and link
 diff --git a/drivers/usb/gadget/fsl_usb2_udc.h 
 b/drivers/usb/gadget/fsl_usb2_udc.h
 index 5cd7b7e..f61a967 100644
 --- a/drivers/usb/gadget/fsl_usb2_udc.h
 +++ b/drivers/usb/gadget/fsl_usb2_udc.h
 @@ -568,10 +568,10 @@ static void dump_msg(const char *label, const u8 * buf, 
 unsigned int length)
  /*
   * ### internal used help routines.
   */
 -#define ep_index(EP) ((EP)-desc-bEndpointAddress0xF)
 +#define ep_index(EP) ((EP)-ep.desc-bEndpointAddress0xF)
  #define ep_maxpacket(EP) ((EP)-ep.maxpacket)
  #define ep_is_in(EP) ( (ep_index(EP) == 0) ? (EP-udc-ep0_dir == \
 - USB_DIR_IN ):((EP)-desc-bEndpointAddress \
 + USB_DIR_IN) : ((EP)-ep.desc-bEndpointAddress \
USB_DIR_IN)==USB_DIR_IN)
  #define get_ep_by_pipe(udc, pipe)((pipe == 1)? udc-eps[0]: \
   udc-eps[pipe])
 -- 
 1.7.2.5

Please send as a proper patch so I can apply.

-- 
balbi


signature.asc
Description: Digital signature
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH v2] usb: fsl_udc: errata - postpone freeing current dTD

2012-05-21 Thread Christoph Fritz
USB controller may access a wrong address for the dTD (endpoint transfer
descriptor) and then hang. This happens a lot when doing tests with
g_ether module and iperf, a tool for measuring maximum TCP and UDP
bandwidth.

This hardware bug is explained in detail by errata number 2858 for i.MX23:
http://cache.freescale.com/files/dsp/doc/errata/IMX23CE.pdf

All (?) SOCs with an IP from chipidea suffer from this problem.
mv_udc_core fixes this bug by commit daec765.  There still may be
unfixed drivers.

Signed-off-by: Christoph Fritz chf.fr...@googlemail.com
Signed-off-by: Christian Hemp c.h...@phytec.de
---
 drivers/usb/gadget/fsl_udc_core.c |   15 ++-
 1 files changed, 14 insertions(+), 1 deletions(-)

diff --git a/drivers/usb/gadget/fsl_udc_core.c 
b/drivers/usb/gadget/fsl_udc_core.c
index 55abfb6..72f2139 100644
--- a/drivers/usb/gadget/fsl_udc_core.c
+++ b/drivers/usb/gadget/fsl_udc_core.c
@@ -65,6 +65,8 @@ static struct usb_sys_interface *usb_sys_regs;
 /* it is initialized in probe()  */
 static struct fsl_udc *udc_controller = NULL;
 
+static struct ep_td_struct *last_free_td;
+
 static const struct usb_endpoint_descriptor
 fsl_ep0_desc = {
.bLength =  USB_DT_ENDPOINT_SIZE,
@@ -180,8 +182,13 @@ static void done(struct fsl_ep *ep, struct fsl_req *req, 
int status)
curr_td = next_td;
if (j != req-dtd_count - 1) {
next_td = curr_td-next_td_virt;
+   dma_pool_free(udc-td_pool, curr_td, curr_td-td_dma);
+   } else {
+   if (last_free_td != NULL)
+   dma_pool_free(udc-td_pool, last_free_td,
+   last_free_td-td_dma);
+   last_free_td = curr_td;
}
-   dma_pool_free(udc-td_pool, curr_td, curr_td-td_dma);
}
 
if (req-mapped) {
@@ -2579,6 +2586,8 @@ static int __init fsl_udc_probe(struct platform_device 
*pdev)
goto err_unregister;
}
 
+   last_free_td = NULL;
+
ret = usb_add_gadget_udc(pdev-dev, udc_controller-gadget);
if (ret)
goto err_del_udc;
@@ -2633,6 +2642,10 @@ static int __exit fsl_udc_remove(struct platform_device 
*pdev)
kfree(udc_controller-status_req);
kfree(udc_controller-eps);
 
+   if (last_free_td != NULL)
+   dma_pool_free(udc_controller-td_pool, last_free_td,
+   last_free_td-td_dma);
+
dma_pool_destroy(udc_controller-td_pool);
free_irq(udc_controller-irq, udc_controller);
iounmap(dr_regs);
-- 
1.7.2.5


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


RE: [PATCH v2] usb: fsl_udc: errata - postpone freeing current dTD

2012-05-21 Thread Chen Peter-B29397
 
 
 USB controller may access a wrong address for the dTD (endpoint transfer
 descriptor) and then hang. This happens a lot when doing tests with
 g_ether module and iperf, a tool for measuring maximum TCP and UDP
 bandwidth.
 
 This hardware bug is explained in detail by errata number 2858 for i.MX23:
 http://cache.freescale.com/files/dsp/doc/errata/IMX23CE.pdf
 

 All (?) SOCs with an IP from chipidea suffer from this problem.
 mv_udc_core fixes this bug by commit daec765.  There still may be
 unfixed drivers.
 
 Signed-off-by: Christoph Fritz chf.fr...@googlemail.com
 Signed-off-by: Christian Hemp c.h...@phytec.de
 ---
  drivers/usb/gadget/fsl_udc_core.c |   15 ++-
  1 files changed, 14 insertions(+), 1 deletions(-)
 
 diff --git a/drivers/usb/gadget/fsl_udc_core.c
 b/drivers/usb/gadget/fsl_udc_core.c
 index 55abfb6..72f2139 100644
 --- a/drivers/usb/gadget/fsl_udc_core.c
 +++ b/drivers/usb/gadget/fsl_udc_core.c
 @@ -65,6 +65,8 @@ static struct usb_sys_interface *usb_sys_regs;
  /* it is initialized in probe()  */
  static struct fsl_udc *udc_controller = NULL;
 
 +static struct ep_td_struct *last_free_td;
 +
  static const struct usb_endpoint_descriptor
  fsl_ep0_desc = {
   .bLength =  USB_DT_ENDPOINT_SIZE,
 @@ -180,8 +182,13 @@ static void done(struct fsl_ep *ep, struct fsl_req
 *req, int status)
   curr_td = next_td;
   if (j != req-dtd_count - 1) {
   next_td = curr_td-next_td_virt;
 + dma_pool_free(udc-td_pool, curr_td, curr_td-td_dma);
 + } else {
 + if (last_free_td != NULL)
 + dma_pool_free(udc-td_pool, last_free_td,
 + last_free_td-td_dma);
 + last_free_td = curr_td;
   }
 - dma_pool_free(udc-td_pool, curr_td, curr_td-td_dma);
   }
 
   if (req-mapped) {
 @@ -2579,6 +2586,8 @@ static int __init fsl_udc_probe(struct
 platform_device *pdev)
   goto err_unregister;
   }
 
 + last_free_td = NULL;
 +
   ret = usb_add_gadget_udc(pdev-dev, udc_controller-gadget);
   if (ret)
   goto err_del_udc;
 @@ -2633,6 +2642,10 @@ static int __exit fsl_udc_remove(struct
 platform_device *pdev)
   kfree(udc_controller-status_req);
   kfree(udc_controller-eps);
 
 + if (last_free_td != NULL)
 + dma_pool_free(udc_controller-td_pool, last_free_td,
 + last_free_td-td_dma);
 +
   dma_pool_destroy(udc_controller-td_pool);
   free_irq(udc_controller-irq, udc_controller);
   iounmap(dr_regs);

Reviewed-by: Peter Chen peter.c...@freescale.com
 --
 1.7.2.5
 
 


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH v2] usb: fsl_udc: errata - postpone freeing current dTD

2012-05-21 Thread Felipe Balbi
Hi,

On Mon, May 21, 2012 at 08:57:22AM +0200, Christoph Fritz wrote:
 USB controller may access a wrong address for the dTD (endpoint transfer
 descriptor) and then hang. This happens a lot when doing tests with
 g_ether module and iperf, a tool for measuring maximum TCP and UDP
 bandwidth.
 
 This hardware bug is explained in detail by errata number 2858 for i.MX23:
 http://cache.freescale.com/files/dsp/doc/errata/IMX23CE.pdf
 
 All (?) SOCs with an IP from chipidea suffer from this problem.
 mv_udc_core fixes this bug by commit daec765.  There still may be
 unfixed drivers.
 
 Signed-off-by: Christoph Fritz chf.fr...@googlemail.com
 Signed-off-by: Christian Hemp c.h...@phytec.de
 ---
  drivers/usb/gadget/fsl_udc_core.c |   15 ++-
  1 files changed, 14 insertions(+), 1 deletions(-)
 
 diff --git a/drivers/usb/gadget/fsl_udc_core.c 
 b/drivers/usb/gadget/fsl_udc_core.c
 index 55abfb6..72f2139 100644
 --- a/drivers/usb/gadget/fsl_udc_core.c
 +++ b/drivers/usb/gadget/fsl_udc_core.c
 @@ -65,6 +65,8 @@ static struct usb_sys_interface *usb_sys_regs;
  /* it is initialized in probe()  */
  static struct fsl_udc *udc_controller = NULL;
  
 +static struct ep_td_struct *last_free_td;

I don't want to see global variables anymore. In fact, please convert
this to the new udc_start()/udc_stop() calls and use the generic
map/unmap routines.

That'll help you get rid of a bunch of useless code on the driver. After
that you should remove all asm/* header includes and drop the ARCH
dependency.

You can also drop the big-/little-endian helpers as you can make use of
generic writel()/readl() routines.

Please make sure these series comes in with enough time to reach v3.6
merge window in about 3 months.

You can put this fix together on that series after you drop the global.

-- 
balbi


signature.asc
Description: Digital signature
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev