All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christoph Fritz <chf.fritz@googlemail.com>
To: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Ben Dooks <ben-linux@fluff.org>,
	Chen Peter-B29397 <B29397@freescale.com>,
	Nicolas Ferre <nicolas.ferre@atmel.com>,
	"Hans J. Koch" <hjk@hansjkoch.de>,
	Fabio Estevam <festevam@gmail.com>,
	Kukjin Kim <kgene.kim@samsung.com>,
	Russell King <linux@arm.linux.org.uk>,
	Thomas Dahlmann <dahlmann.thomas@arcor.de>,
	Christian Hemp <c.hemp@phytec.de>,
	Haojian Zhuang <haojian.zhuang@gmail.com>,
	Daniel Mack <daniel@caiaq.de>, Neil Zhang <zhangwm@marvell.com>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
	Oliver Neukum <oneukum@suse.de>,
	Eric Miao <eric.y.miao@gmail.com>,
	Li Yang-R58472 <r58472@freescale.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	"linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
	Felipe Balbi <balbi@ti.com>, Ido Shayevitz <idos@codeaurora.org>,
	Estevam Fabio-R49496 <r49496@freescale.com>,
	"linuxppc-dev@lists.ozlabs.org" <linuxppc-dev@lists.ozlabs.org>
Subject: Re: [PATCH v2] usb: fsl_udc: errata - postpone freeing current dTD
Date: Mon, 04 Jun 2012 13:37:33 +0200	[thread overview]
Message-ID: <1338809853.3371.31.camel@mars> (raw)
In-Reply-To: <1338809424.3371.20.camel@mars>

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.fritz@googlemail.com>
> > > Signed-off-by: Christian Hemp <c.hemp@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.fritz@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

WARNING: multiple messages have this Message-ID (diff)
From: chf.fritz@googlemail.com (Christoph Fritz)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2] usb: fsl_udc: errata - postpone freeing current dTD
Date: Mon, 04 Jun 2012 13:37:33 +0200	[thread overview]
Message-ID: <1338809853.3371.31.camel@mars> (raw)
In-Reply-To: <1338809424.3371.20.camel@mars>

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.fritz@googlemail.com>
> > > Signed-off-by: Christian Hemp <c.hemp@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.fritz@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

  reply	other threads:[~2012-06-04 11:37 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <1333796272.3450.92.camel@lovely>
     [not found] ` <F281D0F91ED19E4D8E63A7504E8A64980AFFE3@039-SN2MPN1-022.039d.mgd.msft.net>
     [not found]   ` <CAOMZO5AjQsNw7_4ETkB38mmuBXj4pnCwJwAsi4zC4b_83=Enfw@mail.gmail.com>
     [not found]     ` <20120409200656.GD2640@local>
     [not found]       ` <CAOMZO5BTrA8G6wwQZ3e6bydJX7UtG7jj2c29Ln7u_3_jyUGB+w@mail.gmail.com>
     [not found]         ` <20120410001017.GF2640@local>
     [not found]           ` <CAOMZO5A=sHeyoQ0PCd=09qGGKb0NjXi-kF5qyG+3U-GhD7rhzg@mail.gmail.com>
     [not found]             ` <20120410021151.GB23044@lovely.krouter>
     [not found]               ` <20120411073918.GA9180@lovely.krouter>
     [not found]                 ` <CAOMZO5C729Ki0Bequ+s1nnrgt4NZWvwg3Wnjk4kHp=d9BtkeXw@mail.gmail.com>
2012-05-09  0:02                   ` [RFC] [PATCH] usb: gadget: fix dtd dma confusion Christoph Fritz
2012-05-09  1:50                     ` Neil Zhang
2012-05-09  2:11                       ` Chen Peter-B29397
2012-05-09  5:38                         ` Christoph Fritz
2012-05-09  5:43                           ` Chen Peter-B29397
2012-05-09  5:56                             ` Christoph Fritz
2012-05-09  5:45                           ` Neil Zhang
2012-05-09  6:04                             ` Christoph Fritz
2012-05-13 22:51                     ` Christoph Fritz
2012-05-14  1:11                       ` Chen Peter-B29397
2012-05-14  4:21                       ` Greg Kroah-Hartman
2012-05-20 23:17                         ` [PATCH] usb: fsl_udc: errata - postpone freeing current dTD Christoph Fritz
2012-05-21  1:05                           ` Chen Peter-B29397
2012-05-21  6:53                             ` Christoph Fritz
2012-05-21  6:57                               ` [PATCH v2] " Christoph Fritz
2012-05-21  7:25                                 ` Chen Peter-B29397
2012-05-21 19:04                                 ` Felipe Balbi
2012-06-04 11:30                                   ` Christoph Fritz
2012-06-04 11:30                                     ` Christoph Fritz
2012-06-04 11:37                                     ` Christoph Fritz [this message]
2012-06-04 11:37                                       ` Christoph Fritz
2012-06-10 18:41                                       ` Fabio Estevam
2012-06-10 18:41                                         ` Fabio Estevam
2012-06-12 19:40                                         ` Christoph Fritz
2012-06-12 19:40                                           ` Christoph Fritz
2012-06-13  1:17                                           ` Fabio Estevam
2012-06-13  1:17                                             ` Fabio Estevam
2012-06-04 14:59                                     ` Felipe Balbi
2012-06-04 14:59                                       ` Felipe Balbi
2012-06-04 15:24                                       ` [PATCH] usb: gadget: regression fix - useage of usb_ep Christoph Fritz
2012-06-04 15:24                                         ` Christoph Fritz
2012-10-19 10:22                                   ` [PATCH 0/7] update USB gadget driver fsl-usb2-udc Christoph Fritz
2012-10-19 10:24                                     ` [PATCH 1/7] usb: gadget: fsl_udc: simplify driver init Christoph Fritz
2012-10-19 10:24                                       ` Felipe Balbi
2012-10-19 10:24                                     ` [PATCH 2/7] usb: gadget: fsl_udc: protect fsl_pullup() with spin_lock Christoph Fritz
2012-10-19 10:25                                       ` Felipe Balbi
2012-10-19 10:24                                     ` [PATCH 3/7] usb: gadget: fsl_udc: convert to new ulc style Christoph Fritz
2012-10-19 10:27                                       ` Felipe Balbi
2012-10-19 10:24                                     ` [PATCH 4/7] usb: gadget: fsl_udc: drop ARCH dependency Christoph Fritz
2012-10-19 10:29                                       ` Felipe Balbi
2012-10-19 10:24                                     ` [PATCH 5/7] usb: gadget: fsl_udc: postpone freeing current dTD Christoph Fritz
2012-10-19 10:30                                       ` Felipe Balbi
2012-10-19 10:46                                         ` Christoph Fritz
2012-10-19 10:44                                           ` Felipe Balbi
2012-10-20 19:12                                             ` Christoph Fritz
2012-10-22  7:54                                               ` Felipe Balbi
2012-10-19 10:24                                     ` [PATCH 6/7] usb: gadget: fsl_udc: purge global pointer usb_sys_regs Christoph Fritz
2012-10-19 10:24                                     ` [PATCH 7/7] usb: gadget: fsl_udc: purge global pointer dr_regs Christoph Fritz
2012-10-19 15:36                                     ` [PATCH 0/7] update USB gadget driver fsl-usb2-udc Sascha Hauer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1338809853.3371.31.camel@mars \
    --to=chf.fritz@googlemail.com \
    --cc=B29397@freescale.com \
    --cc=balbi@ti.com \
    --cc=ben-linux@fluff.org \
    --cc=c.hemp@phytec.de \
    --cc=dahlmann.thomas@arcor.de \
    --cc=daniel@caiaq.de \
    --cc=eric.y.miao@gmail.com \
    --cc=festevam@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=haojian.zhuang@gmail.com \
    --cc=hjk@hansjkoch.de \
    --cc=idos@codeaurora.org \
    --cc=kgene.kim@samsung.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=linux@arm.linux.org.uk \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=nicolas.ferre@atmel.com \
    --cc=oneukum@suse.de \
    --cc=r49496@freescale.com \
    --cc=r58472@freescale.com \
    --cc=s.hauer@pengutronix.de \
    --cc=zhangwm@marvell.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.