(avoid top-posting. If you want to know the reasons, google is your friend)
On Tue, Sep 29, 2015 at 10:38:05PM +0300, Alexey Khoroshilov wrote:
> Looks good, but
> 1. We still have
> spin_lock(&dev->lock);
> dev->driver->disconnect(&dev->gadget);
> spin_unlock(&dev->lock);
> in
> pch_udc_vbus_session()
> pch_udc_pcd_pullup().
both are fine.
> And also there is lock around dev->driver->setup() in
> pch_udc_svc_control_out()
> pch_udc_svc_intf_interrupt()
> pch_udc_svc_cfg_interrupt().
> Is it ok?
yes
> 2. Deadlocks mentioned in my original report are still there:
> pch_udc_isr()
> spin_lock(&dev->lock);
> pch_udc_dev_isr(dev, dev_intr);
> pch_udc_svc_ur_interrupt(dev);
> empty_req_queue(ep);
> complete_req(ep, req, -ESHUTDOWN);
> spin_lock(&dev->lock); <--- deadlock
no they're not. this is now an unlock.
> if (dev->driver) { spin_lock(&dev->lock); } <--- deadlock
>
>
> --
> Alexey
>
>
> On 28.09.2015 18:49, Felipe Balbi wrote:
> > gadget methods should be called without
> > spinlocks held.
> >
> > Reported-by: Alexey Khoroshilov <khoroshilov@xxxxxxxxx>
> > Signed-off-by: Felipe Balbi <balbi@xxxxxx>
> > ---
> > drivers/usb/gadget/udc/pch_udc.c | 8 ++++----
> > 1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/usb/gadget/udc/pch_udc.c b/drivers/usb/gadget/udc/pch_udc.c
> > index e5f4c5274298..3181fc9c1c49 100644
> > --- a/drivers/usb/gadget/udc/pch_udc.c
> > +++ b/drivers/usb/gadget/udc/pch_udc.c
> > @@ -2747,18 +2747,18 @@ static void pch_udc_dev_isr(struct pch_udc_dev *dev, u32 dev_intr)
> > if (dev_intr & UDC_DEVINT_US) {
> > if (dev->driver
> > && dev->driver->suspend) {
> > - spin_lock(&dev->lock);
> > - dev->driver->suspend(&dev->gadget);
> > spin_unlock(&dev->lock);
> > + dev->driver->suspend(&dev->gadget);
> > + spin_lock(&dev->lock);
> > }
> >
> > vbus = pch_vbus_gpio_get_value(dev);
> > if ((dev->vbus_session == 0)
> > && (vbus != 1)) {
> > if (dev->driver && dev->driver->disconnect) {
> > - spin_lock(&dev->lock);
> > - dev->driver->disconnect(&dev->gadget);
> > spin_unlock(&dev->lock);
> > + dev->driver->disconnect(&dev->gadget);
> > + spin_lock(&dev->lock);
> > }
> > pch_udc_reconnect(dev);
> > } else if ((dev->vbus_session == 0)
> >
>
--
balbi
Attachment:
signature.asc
Description: PGP signature
