Hi,
On Mon, Dec 19, 2011 at 05:46:56PM -0800, Kuninori Morimoto wrote:
> 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
I see. The thing is that we want usb_gadget_map/unmap_request() to
handle that. In the long run, we might want to have a direction flag on
the public struct usb_request and remove this extra parameter, but I'm
still considering that :-)
> Can you please add below fix patch ?
thanks, will do as soon as you fix below :-)
> ------------------------------------------------
> 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;
it would be better to through a big fat WARN() in this case. Gadget
drivers are supposed to check whether you support SGs (by looking into
gadget->sg_suported) before giving you a request with SGs.
> + if (pkt->dma != DMA_ADDR_INVALID)
> + return -EIO;
no no, we want to remove the whole DMA_ADDR_INVALID thing. What we're
doing now is that UDC is required to map the request buffer. So gadget
drivers must not do it. We shouldn't have this DMA_ADDR_INVALID macro
anymore :-)
--
balbi
Attachment:
signature.asc
Description: Digital signature
