Hi,
On Thu, Dec 15, 2011 at 10:58:05AM -0500, Alan Stern wrote:
> On Thu, 15 Dec 2011, Felipe Balbi wrote:
>
> > such utilities are currently duplicated on all UDC
> > drivers basically with the same structure. Let's group
> > all implementations into one generic implementation
> > and get rid of that duplication.
> >
> > Signed-off-by: Felipe Balbi <balbi@xxxxxx>
> > ---
> > drivers/usb/gadget/udc-core.c | 58 +++++++++++++++++++++++++++++++++++++++++
> > include/linux/usb/gadget.h | 12 ++++++++
> > 2 files changed, 70 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/usb/gadget/udc-core.c b/drivers/usb/gadget/udc-core.c
> > index 0b0d12c..7e0bbfe 100644
> > --- a/drivers/usb/gadget/udc-core.c
> > +++ b/drivers/usb/gadget/udc-core.c
> > @@ -22,6 +22,7 @@
> > #include <linux/device.h>
> > #include <linux/list.h>
> > #include <linux/err.h>
> > +#include <linux/dma-mapping.h>
> >
> > #include <linux/usb/ch9.h>
> > #include <linux/usb/gadget.h>
> > @@ -49,6 +50,63 @@ static DEFINE_MUTEX(udc_lock);
> >
> > /* ------------------------------------------------------------------------- */
> >
> > +void usb_gadget_map_request(struct usb_gadget *gadget,
> > + struct usb_request *req, int direction)
> > +{
> > + if (req->length == 0)
> > + return;
> > +
> > + if (req->num_sgs) {
> > + int mapped;
> > +
> > + mapped = dma_map_sg(&gadget->dev, req->sg, req->num_sgs,
> > + direction ? DMA_TO_DEVICE
> > + : DMA_FROM_DEVICE);
> > + if (mapped < 0) {
> > + dev_err(&gadget->dev, "failed to map SGs\n");
> > + return;
>
> Why not return an error code here? Isn't this a fatal error?
can be done ;-)
> > + }
> > +
> > + req->num_mapped_sgs = mapped;
> > + return;
> > + }
> > +
> > + if (req->dma == DMA_ADDR_INVALID) {
>
> What's the purpose of this test? Just to catch UDC drivers that try to
> map a request twice in a row? If that's the case, then why not provide
> an error message when it happens?
The thing is that there has never been an agreement of who will do the
final mapping on the Gadget Framework. Look at all UDCs and you'll see
that they all have the same checks.
We can add such a requirement now, but I'm afraid we could have
out-of-tree gadget-drivers relying on dma_alloc_coherent() which would
already give a request with a valid dma address. Or gadget drivers
actually deciding to call dma_map_*() APIs.
If more people would be willing to accept that possibility, I'm very
eager to change the patch so that it would assume we have an unmapped
request always. What do you say ?
--
balbi
Attachment:
signature.asc
Description: Digital signature
