Hi,
> Hi,
>
> On Fri, Mar 13, 2015 at 01:14:01AM +0000, yoshihiro shimoda wrote:
> > > > > diff --git a/drivers/usb/renesas_usbhs/mod_gadget.c
> > > > > b/drivers/usb/renesas_usbhs/mod_gadget.c
> > > > > index e0384af77e56..e9d75d85be59 100644
> > > > > --- a/drivers/usb/renesas_usbhs/mod_gadget.c
> > > > > +++ b/drivers/usb/renesas_usbhs/mod_gadget.c
> > > > > @@ -119,7 +119,7 @@ struct usbhsg_recip_handle {
> > > > > /*
> > > > > * queue push/pop
> > > > > */
> > > > > -static void usbhsg_queue_pop(struct usbhsg_uep *uep,
> > > > > +static void __usbhsg_queue_pop(struct usbhsg_uep *uep,
> > > > > struct usbhsg_request *ureq,
> > > > > int status)
> > > > > {
> > > > > @@ -133,6 +133,15 @@ static void usbhsg_queue_pop(struct usbhsg_uep
> > > > > *uep,
> > > > > usb_gadget_giveback_request(&uep->ep, &ureq->req);
> > > > > }
> > > > >
> > > > > +static void usbhsg_queue_pop(struct usbhsg_uep *uep,
> > > > > + struct usbhsg_request *ureq,
> > > > > + int status)
> > > > > +{
> > > > > + usbhs_lock(priv, flags);
> > > > > + __usbhsg_queue_pop(uep, ureq, status);
> > > > > + usbhs_unlock(priv, flags);
> > > > > +}
> > > > > +
> > > > > static void usbhsg_queue_done(struct usbhs_priv *priv, struct
> > > > > usbhs_pkt *pkt)
> > > > > {
> > > > > struct usbhs_pipe *pipe = pkt->pipe;
> > > > >
> > > > >
> > > > > then, for cases where lock is already held you call
> > > > > __usbhsg_queue_pop()
> > > > > and for all other cases, call usbhsg_queue_pop().
> > > >
> > > > Thank you for the suggestion. However, we cannot use this
> > > > usbhsg_queue_pop() because a gadget driver might call usb_ep_queue()
> > > > in the "complete" function and this driver calls usbhs_lock() in the
> > > > usb_ep_queue().
> > >
> > > right, in that case just call __usbhs_queue_pop() directly.
> >
> > Yes. So, my understanding is that this driver always calls
> > __usbhs_queue_pop()
> > because this driver cannot know that a gadget driver calls usb_ep_queue()
> > or not
> > in the "complete" function.
>
> but you don't need to know that. All you have to do is spin_unlock()
> before calling usb_gadget_giveback_request(), look at
> drivers/usb/dwc3/gadget.c::dwc3_gadget_giveback()
Thank you for the suggestion. I understood it.
> > > > Perhaps my explanation was bad, but this issue was caused by the
> > > > following conditions:
> > > > - I use the renesas_usbhs driver as udc.
> > > > - I use an old usb-dmac driver that the callback runs on a kthread.
> > > > - I use the ncm driver. In this environment, the ncm driver might
> > > > cause a spinlock suspected.
> > > >
> > > > As an old solution, I fixed the renesas_usbhs driver by this patch.
> > > > However, if I use a new usb-dmac driver that the callback runs on a
> > > > tasklet, I don't need this patch. (This is a new solution.)
> > >
> > > then differentiate based on some revision register or something like
> > > that ?
> >
> > Sorry for the lack information. I am submitting this usb-dmac driver that
> > the callback runs on a tasklet now. This means that the driver is not
> > merged yet. So, I think that we don't need to differentiate.
>
> But as soon as your driver gets merged, you will need to differentiate,
> right ?
Right. So, I will submit v3 patch soon.
Best regards,
Yoshihiro Shimoda
> --
> balbi
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html