Hi,
On Wed, Dec 21, 2011 at 04:40:10AM +0200, Felipe Balbi wrote:
> Hi,
>
> On Tue, Dec 20, 2011 at 04:51:21PM -0800, Kuninori Morimoto wrote:
> > Thank you for your rework
>
> np.
>
> > > those routines have everything we need to map/unmap
> > > USB requests and it's better to use them.
> > >
> > > Signed-off-by: Felipe Balbi <balbi@xxxxxx>
> > > ---
> > > drivers/usb/renesas_usbhs/mod_gadget.c | 73 +++++++------------------------
> > > 1 files changed, 17 insertions(+), 56 deletions(-)
> > >
> > > 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);
> >
> > Sorry for confusion, but this is wrong.
> > It mean "pipe" direction from renesas_usbhs point, not "endpoint" direction.
> > So, correct dir is
> >
> > dir = !usbhs_pipe_is_dir_in(pipe);
> >
> > or
> >
> > dir = usbhs_pipe_is_dir_host(pipe);
>
> So your driver threats the IN direction as "into the device" rather than
> "into the host" ? That's very confusing as the USB Specification states
> that direction is always from the point of view of the host. I will
> change to use usbhs_pipe_is_dir_host() and, if possible, I would like to
> get your Tested-by on the final set that I will send in few hours.
I have now fixed and pushed to my generic-map-unmap branch for reference
(it might take a while until kernel.org syncs up), if you can pull that
and give it a shot, I would be really glad.
Also, you might to add this very minor optimization:
diff --git a/drivers/usb/renesas_usbhs/pipe.c b/drivers/usb/renesas_usbhs/pipe.c
index c2559e8..488dc65 100644
--- a/drivers/usb/renesas_usbhs/pipe.c
+++ b/drivers/usb/renesas_usbhs/pipe.c
@@ -330,8 +330,7 @@ static u16 usbhsp_setup_pipecfg(struct usbhs_pipe *pipe,
if (dir_in)
usbhsp_flags_set(pipe, IS_DIR_HOST);
- if ((is_host && !dir_in) ||
- (!is_host && dir_in))
+ if (is_host ^ dir_in)
dir |= DIR_OUT;
if (!dir)
--
balbi
Attachment:
signature.asc
Description: Digital signature
