On Tue, Dec 20, 2011 at 06:56:19PM +0100, Sebastian Andrzej Siewior wrote:
> * Felipe Balbi | 2011-12-20 18:37:08 [+0200]:
>
> >diff --git a/drivers/usb/renesas_usbhs/mod_gadget.c b/drivers/usb/renesas_usbhs/mod_gadget.c
> >index 528691d5f..685c434 100644
> >--- a/drivers/usb/renesas_usbhs/mod_gadget.c
> >+++ b/drivers/usb/renesas_usbhs/mod_gadget.c
> >@@ -165,69 +165,32 @@ static void usbhsg_queue_push(struct usbhsg_uep *uep,
> > /*
> > * dma map/unmap
> > */
> >-static int usbhsg_dma_map(struct device *dev,
> >- struct usbhs_pkt *pkt,
> >- enum dma_data_direction dir)
> >-{
> >- struct usbhsg_request *ureq = usbhsg_pkt_to_ureq(pkt);
> >- struct usb_request *req = &ureq->req;
> >-
> >- if (pkt->dma != DMA_ADDR_INVALID) {
> >- dev_err(dev, "dma is already mapped\n");
> >- return -EIO;
> >- }
> >-
> >- if (req->dma == DMA_ADDR_INVALID) {
> >- pkt->dma = dma_map_single(dev, pkt->buf, pkt->length, dir);
> >- } else {
> >- dma_sync_single_for_device(dev, req->dma, req->length, dir);
> >- pkt->dma = req->dma;
> >- }
> >-
> >- if (dma_mapping_error(dev, pkt->dma)) {
> >- dev_err(dev, "dma mapping error %llx\n", (u64)pkt->dma);
> >- return -EIO;
> >- }
> >-
> >- return 0;
> >-}
> >-
> >-static int usbhsg_dma_unmap(struct device *dev,
> >- struct usbhs_pkt *pkt,
> >- enum dma_data_direction dir)
> >+static int usbhsg_dma_map_ctrl(struct usbhs_pkt *pkt, int map)
> > {
> > struct usbhsg_request *ureq = usbhsg_pkt_to_ureq(pkt);
> > struct usb_request *req = &ureq->req;
> >-
> >- if (pkt->dma == DMA_ADDR_INVALID) {
> >- dev_err(dev, "dma is not mapped\n");
> >- return -EIO;
> >- }
> >-
> >- if (req->dma == DMA_ADDR_INVALID)
> >- dma_unmap_single(dev, pkt->dma, pkt->length, dir);
> >- else
> >- dma_sync_single_for_cpu(dev, req->dma, req->length, dir);
> >-
> >- pkt->dma = DMA_ADDR_INVALID;
> >-
> >- return 0;
> >-}
> >-
> >-static int usbhsg_dma_map_ctrl(struct usbhs_pkt *pkt, int map)
> >-{
> > struct usbhs_pipe *pipe = pkt->pipe;
> > struct usbhsg_uep *uep = usbhsg_pipe_to_uep(pipe);
> > struct usbhsg_gpriv *gpriv = usbhsg_uep_to_gpriv(uep);
> >- struct device *dev = usbhsg_gpriv_to_dev(gpriv);
> > enum dma_data_direction dir;
> >+ int ret = 0;
> >
> >- dir = usbhs_pipe_is_dir_in(pipe) ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
> >+ dir = usbhs_pipe_is_dir_in(pipe);
> >
> >- if (map)
> >- return usbhsg_dma_map(dev, pkt, dir);
> >- else
> >- return usbhsg_dma_unmap(dev, pkt, dir);
> >+ if (map) {
> >+ /* it can not use scatter/gather */
> >+ WARN_ON(req->num_sgs);
>
> This does not matter. Could the mapping function do the right thing?
> UASP provides a contigues buffer atm. This can be improved but it does
> not explode :)
The way we have been using this API is that gadget driver should check
whether gadget->sg_supported is set before initializing the usb_request.
This is just a sanity check to catch wrongly converted gadget drivers.
I also didn't like it at first, but it does make sense to keep it. I
couldn't find a way to move this to the generic map/unmap API though.
> >+ ret = usb_gadget_map_request(&gpriv->gadget, req, dir);
> >+ if (ret < 0)
> >+ return ret;
> >+
> >+ pkt->dma = req->dma;
> You could remove pkt->dma whilete here
that would be a big rework of the driver and not subject to this patch.
--
balbi
Attachment:
signature.asc
Description: Digital signature
