Hi,
On Thu, Dec 15, 2011 at 02:56:51PM +0100, Sebastian Andrzej Siewior wrote:
> * Felipe Balbi | 2011-12-15 13:43:37 [+0200]:
>
> >--- 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;
> >+ }
> >+
> >+ req->num_mapped_sgs = mapped;
>
> I'm not very happy with this. First I would like to understand _why_ we
> map less than we asked for. Plus what happens with the outstanding
> bytes. Why do we drop them?
>
> >+ if (req->num_mapped_sgs) {
> >+ req->dma = DMA_ADDR_INVALID;
> >+ dma_unmap_sg(&gadget->dev, req->sg, req->num_sgs,
> >+ direction ? DMA_TO_DEVICE : DMA_FROM_DEVICE);
>
> No consistency here. Why you don't use num_mapped_sgs here? This looks
> wrong.
that's what was causing the Warning, did you not read the commit log ?
On unmap, we must give the total number of entries.
> >--- a/include/linux/usb/gadget.h
> >+++ b/include/linux/usb/gadget.h
> >@@ -950,6 +950,18 @@ static inline void usb_free_descriptors(struct usb_descriptor_header **v)
> >
> > /*-------------------------------------------------------------------------*/
> >
> >+/* utility to simplify map/unmap of usb_requests to/from DMA */
> >+
> >+#define DMA_ADDR_INVALID (~(dma_addr_t) 0)
>
> Please rip this define from each and every gadget out before introducing
> it here.
yeah, it's unfinished ;-)
--
balbi
Attachment:
signature.asc
Description: Digital signature
