lkml.org 
[lkml]   [2011]   [Dec]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH 4/9] usb: renesas: gadget: use generic map/unmap routines

Hi Felipe

Thank you for your hard work

> those routines have everything we need to map/unmap
> USB requests and it's better to use them.
>
> Signed-off-by: Felipe Balbi <balbi@ti.com>
> ---
> drivers/usb/renesas_usbhs/mod_gadget.c | 60 ++++----------------------------
> 1 files changed, 7 insertions(+), 53 deletions(-)
>
> diff --git a/drivers/usb/renesas_usbhs/mod_gadget.c b/drivers/usb/renesas_usbhs/mod_gadget.c
> index 528691d5f..cb92a1d 100644
> --- a/drivers/usb/renesas_usbhs/mod_gadget.c
> +++ b/drivers/usb/renesas_usbhs/mod_gadget.c
> @@ -165,69 +165,23 @@ 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;
>
> dir = usbhs_pipe_is_dir_in(pipe) ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
>
> - if (map)
> - return usbhsg_dma_map(dev, pkt, dir);
> - else
> - return usbhsg_dma_unmap(dev, pkt, dir);
> + if (map) {
> + return usb_gadget_map_request(&gpriv->gadget, req, dir);
> + } else {
> + usb_gadget_unmap_request(&gpriv->gadget, req, dir);
> + return 0;
> + }
> }

I tried this patch, but renesas_usbhs didn't work.
It seems have some bugs.

1. renesas_usbhs dma needs pkt->dma, but this patch didn't care it.
2. dma direction seems wrong
("dir" needs 0/1, not DMA_xxx_DEVICE for usb_gadget_xx_request())

And, renesas_usbhs can not use scatter/gather type dma

#
# this is not super important, but
#
# int usb_gadget_map_request(struct usb_gadget *gadget,
# struct usb_request *req, int direction)
#
# this "direction = 0/1" is a little bit un-understandable for me.
# it is difficult to understand "which direction ?" from code.
# if "direction = DMA_TO_DEVICE/DMA_FROM_DEVICE, it is understandable ;P
#

Can you please add below fix patch ?
------------------------------------------------
diff --git a/drivers/usb/renesas_usbhs/mod_gadget.c b/drivers/usb/renesas_usbhs/mod_gadget.c
index cb92a1d..c467067 100644
--- a/drivers/usb/renesas_usbhs/mod_gadget.c
+++ b/drivers/usb/renesas_usbhs/mod_gadget.c
@@ -173,15 +173,32 @@ static int usbhsg_dma_map_ctrl(struct usbhs_pkt *pkt, int map)
struct usbhsg_uep *uep = usbhsg_pipe_to_uep(pipe);
struct usbhsg_gpriv *gpriv = usbhsg_uep_to_gpriv(uep);
enum dma_data_direction dir;
+ int ret;

- dir = usbhs_pipe_is_dir_in(pipe) ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
+ dir = !usbhs_pipe_is_dir_in(pipe);

if (map) {
- return usb_gadget_map_request(&gpriv->gadget, req, dir);
+ if (req->num_sgs) /* it can not use scatter/gather */
+ return -EIO;
+
+ if (pkt->dma != DMA_ADDR_INVALID)
+ return -EIO;
+
+ ret = usb_gadget_map_request(&gpriv->gadget, req, dir);
+ if (ret < 0)
+ return ret;
+
+ pkt->dma = req->dma;
} else {
+ if (pkt->dma == DMA_ADDR_INVALID)
+ return -EIO;
+
usb_gadget_unmap_request(&gpriv->gadget, req, dir);
- return 0;
+ ret = 0;
+ pkt->dma = DMA_ADDR_INVALID;
}
+
+ return ret;
}

/*
------------------------------------------------
Best regards
---
Kuninori Morimoto

\
 
 \ /
  Last update: 2011-12-20 03:31    [from the cache]
©2003-2014 Jasper Spaans. Advertise on this site