linux-mips
[Top] [All Lists]

Re: [PATCH] usb: gadget: bcm63xx UDC driver

To: Kevin Cernekee <cernekee@gmail.com>
Subject: Re: [PATCH] usb: gadget: bcm63xx UDC driver
From: Felipe Balbi <balbi@ti.com>
Date: Tue, 21 Aug 2012 21:08:19 +0300
Cc: balbi@ti.com, ralf@linux-mips.org, linux-mips@linux-mips.org, linux-usb@vger.kernel.org
In-reply-to: <CAJiQ=7BQz18s03du_Q33z45W+QrkVaPqgZSuUTU-x9v=48CGbA@mail.gmail.com>
List-archive: <http://www.linux-mips.org/archives/linux-mips/>
List-help: <mailto:ecartis@linux-mips.org?Subject=help>
List-id: linux-mips <linux-mips.eddie.linux-mips.org>
List-owner: <mailto:ralf@linux-mips.org>
List-post: <mailto:linux-mips@linux-mips.org>
List-software: Ecartis version 1.0.0
List-subscribe: <mailto:ecartis@linux-mips.org?subject=subscribe%20linux-mips>
List-unsubscribe: <mailto:ecartis@linux-mips.org?subject=unsubscribe%20linux-mips>
References: <97cb21b8063a02a9664baf8b749ae200@localhost> <20120820074041.GH17455@arwen.pp.htv.fi> <CAJiQ=7CB2w=aNwtU4f3di6c31tD-EWO9YLejESY5HsUaHY6s1A@mail.gmail.com> <20120821120418.GE10347@arwen.pp.htv.fi> <CAJiQ=7BQz18s03du_Q33z45W+QrkVaPqgZSuUTU-x9v=48CGbA@mail.gmail.com>
Reply-to: balbi@ti.com
Sender: linux-mips-bounce@linux-mips.org
User-agent: Mutt/1.5.21 (2010-09-15)
On Tue, Aug 21, 2012 at 08:20:43AM -0700, Kevin Cernekee wrote:
> On Tue, Aug 21, 2012 at 5:04 AM, Felipe Balbi <balbi@ti.com> wrote:
> > On Mon, Aug 20, 2012 at 08:48:11PM -0700, Kevin Cernekee wrote:
> >> On Mon, Aug 20, 2012 at 12:40 AM, Felipe Balbi <balbi@ti.com> wrote:
> >> > no workqueues, please either handle the IRQ here or use threaded_irqs.
> >> >
> >> > again, no workqueues.
> >>
> >> Felipe,
> >>
> >> I am seeing all sorts of deadlocks now, after removing the workqueue
> >> (patch V2).  Some have easy fixes, but for others it is not as
> >> obvious.  The code was much simpler when I could just trigger a
> >> deferred worker function.
> >>
> >> Workqueues are used in at91_udc, lpc32xx_udc, mv_udc_core, and
> >> pch_udc.  Could you please clarify why it is not OK to use one in
> >> bcm63xx_udc?
> >
> > Because threaded_irqs were added in order to drop such workqueues.
> > threaded_irqs also have the highest priority possible (only lower than
> > hardirq handlers), so you'll get scheduled much sooner.
> >
> > Could it be that most of your deadlocks is because you're not setting
> > IRQF_ONESHOT ?
> 
> The deadlocks involve ep0 processing that is triggered through
> bcm63xx_udc_queue().  e.g. gadget driver queues a new request, it's a
> reply to a spoofed SET_CONFIGURATION / SET_INTERFACE transaction, and
> the UDC driver calls the completion immediately.
> 
> So, not all of the ep0 work is being done in response to an IRQ from
> the controller HW, and I think that is where this driver diverges from
> most of the others.
> 
> Would it be OK to use a workqueue, or maybe a tasklet, given these
> circumstances?

Then stick to a workqueue... but could you let me know why exactly you
have to fake SET_CONFIGURATION/SET_INTERFACE requests ? Is this a
silicon bug or a silicon feature ? That's quite weird to me.

-- 
balbi

Attachment: signature.asc
Description: Digital signature

<Prev in Thread] Current Thread [Next in Thread>