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: Alan Stern <stern@rowland.harvard.edu>
Date: Tue, 21 Aug 2012 17:34:10 -0400 (EDT)
Cc: balbi@ti.com, <ralf@linux-mips.org>, <linux-mips@linux-mips.org>, <linux-usb@vger.kernel.org>
In-reply-to: <CAJiQ=7DefOV1daP5bfxmgPHseYvx8Yj1K7h=Kv3hc9iaKv0wXw@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>
Sender: linux-mips-bounce@linux-mips.org
On Tue, 21 Aug 2012, Kevin Cernekee wrote:

> On Tue, Aug 21, 2012 at 1:34 PM, Alan Stern <stern@rowland.harvard.edu> wrote:
> >> It is a silicon feature: the core will intercept SET_CONFIGURATION /
> >> SET_INTERFACE requests, store wValue/wIndex in the appropriate
> >> USBD_STATUS_REG field (cfg/intf/altintf), send an acknowledgement to
> >> the host, and raise a control interrupt.
> >
> > Your explanation is not clear.  The operations you listed are exactly
> > what any UDC should do when it receives any control request: It should
> > store the bRequestType, bRequest, wValue, wIndex, and wLength values in
> > appropriate registers, send an ACK back to the host, and generate an
> > IRQ.  What's special about Set-Config and Set-Interface?
> 
> For "most" control requests (such as GET_DESCRIPTOR), this core writes
> the raw packet data out to a buffer in DRAM and raises an IUDMA IRQ.
> The UDC driver passes this data on to the gadget driver and allows it
> to decide what to send back in subsequent phases.
> 
> But some requests are handled entirely in hardware, including the status 
> phase:
> 
> SET_ADDRESS
> SET_CONFIGURATION
> SET_INTERFACE
> SET_FEATURE
> CLEAR_FEATURE
> GET_FEATURE

I assume the features in question are endpoint-halts and
remote-wakeup-enable.  It's hard to think of any other features that 
could be handled in hardware.

> Where appropriate, the hardware block will update its registers to
> indicate the new settings and raise a control IRQ.

I see.  Didn't the manufacturer realize that, in the case of Set-Config
and Set-Interface, this violates the USB-2 specification?  The spec
explicitly says that the status stage must not be completed until the
operation is fully carried out.

(BTW, what happens if one of these requests arrives before a previous
control IRQ has been acknowledged by the CPU?  Does the old data in the
registers get overwritten and lost, or does the UDC hold off on 
carrying out the new request?)

I can't imagine how any gadget driver could hope to handle a mess like 
this.  For instance, suppose a Set-Config operation fails.  There's no 
way for the driver to tell the host, because the hardware has already 
said that the operation succeeded.

Anyway, you don't need to workqueue to manage this -- just a queue of 
pending control requests.  You should be able to do everything in 
interrupt context, especially since you will ignore any responses the 
driver submits on ep0 for these transfers.

Alan Stern


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