On Tue, Aug 21, 2012 at 08:20:43AM -0700, Kevin Cernekee wrote:

On Tue, Aug 21, 2012 at 5:04 AM, Felipe Balbi [off-list ref] wrote:

quoted

On Mon, Aug 20, 2012 at 08:48:11PM -0700, Kevin Cernekee wrote:

quoted

On Mon, Aug 20, 2012 at 12:40 AM, Felipe Balbi [off-list ref] wrote:

quoted

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