From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754112Ab1LTBrA (ORCPT ); Mon, 19 Dec 2011 20:47:00 -0500 Received: from mail-gx0-f174.google.com ([209.85.161.174]:50424 "EHLO mail-gx0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753039Ab1LTBq6 (ORCPT ); Mon, 19 Dec 2011 20:46:58 -0500 Date: Mon, 19 Dec 2011 17:46:56 -0800 (PST) Message-ID: <87d3bkrp4n.wl%kuninori.morimoto.gx@renesas.com> From: Kuninori Morimoto To: Felipe Balbi Cc: Linux USB Mailing List , Greg Kroah-Hartman (supporter:USB SUBSYSTEM ,commit_signer:28/50=56%), Thomas Dahlmann (supporter:AMD GEODE CS5536...), linux-omap@vger.kernel.org (open list:DESIGNWARE USB3 D...), linux-kernel@vger.kernel.org (open list), linux-geode@lists.infradead.org (open list:AMD GEODE CS5536...) Subject: Re: [PATCH 4/9] usb: renesas: gadget: use generic map/unmap routines In-Reply-To: <1324290632-23758-5-git-send-email-balbi@ti.com> References: <1324290632-23758-1-git-send-email-balbi@ti.com> <1324290632-23758-5-git-send-email-balbi@ti.com> User-Agent: Wanderlust/2.14.0 (Africa) SEMI/1.14.6 (Maruoka) FLIM/1.14.9 (=?ISO-2022-JP-2?B?R29qGyQoRCtXGyhC?=) APEL/10.8 Emacs/23.3 (i686-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > --- > 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 From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kuninori Morimoto Subject: Re: [PATCH 4/9] usb: renesas: gadget: use generic map/unmap routines Date: Mon, 19 Dec 2011 17:46:56 -0800 (PST) Message-ID: <87d3bkrp4n.wl%kuninori.morimoto.gx@renesas.com> References: <1324290632-23758-1-git-send-email-balbi@ti.com> <1324290632-23758-5-git-send-email-balbi@ti.com> Mime-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset=US-ASCII Return-path: In-Reply-To: <1324290632-23758-5-git-send-email-balbi-l0cyMroinI0@public.gmane.org> Sender: linux-usb-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Felipe Balbi Cc: Linux USB Mailing List , Greg Kroah-Hartman , Thomas Dahlmann , "open list:DESIGNWARE USB3 D..." , open list , "open list:AMD GEODE CS5536..." List-Id: linux-omap@vger.kernel.org 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 > --- > 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-usb" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html