| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <87aa6orn64.wl%kuninori.morimoto.gx@renesas.com>
Date: Mon, 19 Dec 2011 18:29:11 -0800 (PST)
From: Kuninori Morimoto <kuninori.morimoto.gx@...esas.com>
To: Felipe Balbi <balbi@...com>
Cc: Thomas Dahlmann <dahlmann.thomas@...or.de>,
Linux-USB <linux-usb@...r.kernel.org>, Greg KH <gregkh@...e.de>,
linux-geode@...ts.infradead.org, linux-kernel@...r.kernel.org,
linux-omap@...r.kernel.org,
Kuninori Morimoto <kuninori.morimoto.gx@...esas.com>
Subject: Re: [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@...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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists