All of lore.kernel.org
 help / color / mirror / Atom feed
From: Anton Vorontsov <avorontsov@ru.mvista.com>
To: Li Yang <leoli@freescale.com>
Cc: dbrownell@users.sourceforge.net, greg@kroah.com,
	linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
	linuxppc-dev@ozlabs.org, Robert Jarzmik <rjarzmik@free.fr>
Subject: Re: [PATCH] usb: add Freescale QE/CPM USB peripheral controller driver
Date: Mon, 1 Sep 2008 20:34:53 +0400	[thread overview]
Message-ID: <20080901163453.GA6178@oksana.dev.rtsoft.ru> (raw)
In-Reply-To: <1219916613-28827-1-git-send-email-leoli@freescale.com>

On Thu, Aug 28, 2008 at 05:43:33PM +0800, Li Yang wrote:
> Some of Freescale SoC chips have a QE or CPM co-processor which
> supports full speed USB.  The driver adds device mode support
> of both QE and CPM USB controller to Linux USB gadget.  The
> driver is tested with MPC8360 and MPC8272, and should work with
> other models having QE/CPM given minor tweaks.
> 
> Signed-off-by: Xie Xiaobo <X.Xie@freescale.com>
> Signed-off-by: Li Yang <leoli@freescale.com>
> ---

Just found a recursive locking bug:

[...]
> +static int reset_queues(struct qe_udc *udc)
> +{

Note: this function is called from the IRQ, the IRQ handler
grabs udc->lock spinlock..

> +	u8 pipe;
> +
> +	for (pipe = 0; pipe < USB_MAX_ENDPOINTS; pipe++)
> +		udc_reset_ep_queue(udc, pipe);
> +
> +	/* report disconnect; the driver is already quiesced */
> +	udc->driver->disconnect(&udc->gadget);

In the disconnect(), g_ether driver will immediately call
qe_ep_disable() function which will try to grab &udc->lock
spinlock once again..

Not sure how to fix this properly... :-/

p.s. the same bug exists in omap_udc.c, pxa27x_udc.c and probably
other drivers as well... The only reason why it does not exploit
in most cases is that the spin_lock_irqsave for !SMP case turns
into simple local_irq_save().

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

WARNING: multiple messages have this Message-ID (diff)
From: Anton Vorontsov <avorontsov@ru.mvista.com>
To: Li Yang <leoli@freescale.com>
Cc: greg@kroah.com, linux-usb@vger.kernel.org,
	dbrownell@users.sourceforge.net, linuxppc-dev@ozlabs.org,
	linux-kernel@vger.kernel.org, Robert Jarzmik <rjarzmik@free.fr>
Subject: Re: [PATCH] usb: add Freescale QE/CPM USB peripheral controller driver
Date: Mon, 1 Sep 2008 20:34:53 +0400	[thread overview]
Message-ID: <20080901163453.GA6178@oksana.dev.rtsoft.ru> (raw)
In-Reply-To: <1219916613-28827-1-git-send-email-leoli@freescale.com>

On Thu, Aug 28, 2008 at 05:43:33PM +0800, Li Yang wrote:
> Some of Freescale SoC chips have a QE or CPM co-processor which
> supports full speed USB.  The driver adds device mode support
> of both QE and CPM USB controller to Linux USB gadget.  The
> driver is tested with MPC8360 and MPC8272, and should work with
> other models having QE/CPM given minor tweaks.
> 
> Signed-off-by: Xie Xiaobo <X.Xie@freescale.com>
> Signed-off-by: Li Yang <leoli@freescale.com>
> ---

Just found a recursive locking bug:

[...]
> +static int reset_queues(struct qe_udc *udc)
> +{

Note: this function is called from the IRQ, the IRQ handler
grabs udc->lock spinlock..

> +	u8 pipe;
> +
> +	for (pipe = 0; pipe < USB_MAX_ENDPOINTS; pipe++)
> +		udc_reset_ep_queue(udc, pipe);
> +
> +	/* report disconnect; the driver is already quiesced */
> +	udc->driver->disconnect(&udc->gadget);

In the disconnect(), g_ether driver will immediately call
qe_ep_disable() function which will try to grab &udc->lock
spinlock once again..

Not sure how to fix this properly... :-/

p.s. the same bug exists in omap_udc.c, pxa27x_udc.c and probably
other drivers as well... The only reason why it does not exploit
in most cases is that the spin_lock_irqsave for !SMP case turns
into simple local_irq_save().

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

  parent reply	other threads:[~2008-09-01 16:34 UTC|newest]

Thread overview: 61+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-08-28  9:43 [PATCH] usb: add Freescale QE/CPM USB peripheral controller driver Li Yang
2008-08-28  9:43 ` Li Yang
2008-08-28 15:04 ` Arnd Bergmann
2008-08-28 15:04   ` Arnd Bergmann
2008-08-28 17:22   ` Alan Stern
2008-08-28 17:22     ` Alan Stern
2008-08-28 17:53     ` Scott Wood
2008-08-28 17:53       ` Scott Wood
2008-08-28 20:16       ` Alan Stern
2008-08-28 20:16         ` Alan Stern
2008-08-28 22:27         ` Arnd Bergmann
2008-08-28 22:27           ` Arnd Bergmann
2008-08-29 16:05           ` Alan Stern
2008-08-29 16:05             ` Alan Stern
2008-08-29 16:29             ` Arnd Bergmann
2008-08-29 16:29               ` Arnd Bergmann
2008-08-29 17:19               ` Alan Stern
2008-08-29 17:19                 ` Alan Stern
2008-08-29 17:38                 ` Arnd Bergmann
2008-08-29 17:38                   ` Arnd Bergmann
2008-08-29 21:22                   ` Alan Stern
2008-08-29 21:22                     ` Alan Stern
2008-09-24 20:15           ` David Brownell
2008-09-24 20:15             ` David Brownell
2008-08-29  8:57   ` Li Yang
2008-08-29  8:57     ` Li Yang
2008-08-29  8:57     ` Arnd Bergmann
2008-08-29  8:57       ` Arnd Bergmann
2008-09-24 20:10   ` David Brownell
2008-09-24 20:10     ` David Brownell
2008-08-28 16:39 ` Scott Wood
2008-08-28 16:39   ` Scott Wood
2008-08-29  9:35   ` Li Yang
2008-08-29  9:35     ` Li Yang
2008-09-01 13:52 ` Anton Vorontsov
2008-09-01 13:52   ` Anton Vorontsov
2008-09-02  7:08   ` Li Yang
2008-09-02  7:08     ` Li Yang
2008-09-01 16:34 ` Anton Vorontsov [this message]
2008-09-01 16:34   ` Anton Vorontsov
2008-09-01 17:11 ` Anton Vorontsov
2008-09-01 17:11   ` Anton Vorontsov
2008-09-02  7:35   ` Li Yang
2008-09-02  7:35     ` Li Yang
2008-09-02  7:57     ` Joakim Tjernlund
2008-09-02  7:57       ` Joakim Tjernlund
2008-09-02  8:12       ` [PATCH] usb: add Freescale QE/CPM USB peripheral controllerdriver Li Yang-R58472
2008-09-02  8:12         ` Li Yang-R58472
2008-09-02  8:15         ` Joakim Tjernlund
2008-09-02  8:15           ` Joakim Tjernlund
2008-09-24 20:28           ` David Brownell
2008-09-24 20:28             ` David Brownell
2008-09-24 21:30             ` Joakim Tjernlund
2008-09-24 21:42               ` David Brownell
2008-09-24 21:42                 ` David Brownell
2008-09-24 20:26         ` David Brownell
2008-09-24 20:26           ` David Brownell
  -- strict thread matches above, loose matches on Subject: below --
2008-08-06  7:16 [PATCH] usb: add Freescale QE/CPM USB peripheral controller driver Li Yang
2008-08-06 15:15 ` Anton Vorontsov
2008-08-06 15:47 ` Timur Tabi
2008-08-15 14:16 ` Anton Vorontsov

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=20080901163453.GA6178@oksana.dev.rtsoft.ru \
    --to=avorontsov@ru.mvista.com \
    --cc=dbrownell@users.sourceforge.net \
    --cc=greg@kroah.com \
    --cc=leoli@freescale.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=linuxppc-dev@ozlabs.org \
    --cc=rjarzmik@free.fr \
    /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.