From patchwork Mon Jun 4 11:30:24 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Christoph Fritz X-Patchwork-Id: 162747 Return-Path: X-Original-To: incoming-imx@patchwork.ozlabs.org Delivered-To: patchwork-incoming-imx@bilbo.ozlabs.org Received: from merlin.infradead.org (merlin.infradead.org [IPv6:2001:4978:20e::2]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 0FDB8B7008 for ; Mon, 4 Jun 2012 21:34:34 +1000 (EST) Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.76 #1 (Red Hat Linux)) id 1SbVUa-0002Ji-VY; Mon, 04 Jun 2012 11:30:37 +0000 Received: from mail-bk0-f49.google.com ([209.85.214.49]) by merlin.infradead.org with esmtps (Exim 4.76 #1 (Red Hat Linux)) id 1SbVUX-0002JU-8O for linux-arm-kernel@lists.infradead.org; Mon, 04 Jun 2012 11:30:34 +0000 Received: by bkwj4 with SMTP id j4so4752741bkw.36 for ; Mon, 04 Jun 2012 04:30:30 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=googlemail.com; s=20120113; h=subject:from:to:cc:in-reply-to:references:content-type:date :message-id:mime-version:x-mailer:content-transfer-encoding; bh=Ko+MkkPoExTZl72SciK/W8bGGfK0JgyLPXADVHKIthk=; b=Abpt8YaHAW07qx3mByxyX1pOANeEKd5pxe1rYFbtOpK4nD0nC6pqvFmI26GqLVyE3S jlxOVdlotU5/NbDTznhhaZDQt9F7L3T0KZ18736ifwXdUKtriRVtm0ARDsdd5jlngldG 8uJ1pTyFbexr9wu6QH5vk+iXRNpGNGu8E/mrYLT/1kJJs0ERQhpqtVX0DvOh+oGohwsu hOjBVsV/4AYO48KBVEv5/rR9ib4owg4i2NXBicLpU1W7M8vtKGR8b2S9pbuNUcutgfNB sND6kwUTMIxxblCCzPu7Dzl7qYVWYDib/VFPXhkmYveG9Nx2UWaJbvc+35CJOVvLm2ER 9/iQ== Received: by 10.204.130.89 with SMTP id r25mr6431483bks.48.1338809430102; Mon, 04 Jun 2012 04:30:30 -0700 (PDT) Received: from [192.168.1.2] (p4FD8312A.dip.t-dialin.net. [79.216.49.42]) by mx.google.com with ESMTPS id ie3sm10025661bkc.1.2012.06.04.04.30.26 (version=SSLv3 cipher=OTHER); Mon, 04 Jun 2012 04:30:28 -0700 (PDT) Subject: Re: [PATCH v2] usb: fsl_udc: errata - postpone freeing current dTD From: Christoph Fritz To: Felipe Balbi In-Reply-To: <20120521190450.GA27492@arwen.pp.htv.fi> References: <20120410021151.GB23044@lovely.krouter> <20120411073918.GA9180@lovely.krouter> <20120509000221.GA19525@lovely.krouter> <20120513225126.GA3683@lovely.krouter> <20120514042142.GD9750@kroah.com> <20120520231724.GA7941@mars> <1337583221.3394.21.camel@mars> <20120521065722.GA4363@mars> <20120521190450.GA27492@arwen.pp.htv.fi> Date: Mon, 04 Jun 2012 13:30:24 +0200 Message-ID: <1338809424.3371.20.camel@mars> Mime-Version: 1.0 X-Mailer: Evolution 2.30.3 X-Spam-Note: CRM114 invocation failed X-Spam-Score: -2.7 (--) X-Spam-Report: SpamAssassin version 3.3.2 on merlin.infradead.org summary: Content analysis details: (-2.7 points) pts rule name description ---- ---------------------- -------------------------------------------------- 0.0 FREEMAIL_FROM Sender email is commonly abused enduser mail provider (chf.fritz[at]googlemail.com) -0.7 RCVD_IN_DNSWL_LOW RBL: Sender listed at http://www.dnswl.org/, low trust [209.85.214.49 listed in list.dnswl.org] -0.0 SPF_PASS SPF: sender matches SPF record -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] -0.1 DKIM_VALID_AU Message has a valid DKIM or DK signature from author's domain 0.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily valid -0.1 DKIM_VALID Message has at least one valid DKIM or DK signature Cc: Ben Dooks , Chen Peter-B29397 , Nicolas Ferre , "Hans J. Koch" , Fabio Estevam , Kukjin Kim , Russell King , Thomas Dahlmann , Sascha Hauer , Christian Hemp , Haojian Zhuang , Daniel Mack , Neil Zhang , linux-arm-kernel , Oliver Neukum , Eric Miao , Li Yang-R58472 , Greg Kroah-Hartman , "linux-usb@vger.kernel.org" , Ido Shayevitz , Estevam Fabio-R49496 , "linuxppc-dev@lists.ozlabs.org" X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.14 Precedence: list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-arm-kernel-bounces@lists.infradead.org Errors-To: linux-arm-kernel-bounces+incoming-imx=patchwork.ozlabs.org@lists.infradead.org List-Id: linux-imx-kernel.lists.patchwork.ozlabs.org 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 > > Signed-off-by: Christian Hemp > > --- > > 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 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’ my proposed regression patch: --- From: Christoph Fritz 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 --- 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->bEndpointAddress&0xF) +#define ep_index(EP) ((EP)->ep.desc->bEndpointAddress&0xF) #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])