| Message ID | 1414750354-19571-1-git-send-email-m.szyprowski@samsung.com |
|---|---|
| State | New |
| Headers | show |
> From: Marek Szyprowski [mailto:m.szyprowski@samsung.com] > Sent: Friday, October 31, 2014 3:13 AM > > This patch adds mutex, which protects initialization and > deinitialization procedures against suspend/resume methods. > > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> > --- > drivers/usb/dwc2/core.h | 1 + > drivers/usb/dwc2/gadget.c | 20 ++++++++++++++++++++ > 2 files changed, 21 insertions(+) > > diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h > index 9f77b4d1c5ff..58732a9a0019 100644 > --- a/drivers/usb/dwc2/core.h > +++ b/drivers/usb/dwc2/core.h > @@ -187,6 +187,7 @@ struct s3c_hsotg { > struct s3c_hsotg_plat *plat; > > spinlock_t lock; > + struct mutex init_mutex; > > void __iomem *regs; > int irq; > diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c > index d8dda39c9e16..a2e4272a904e 100644 > --- a/drivers/usb/dwc2/gadget.c > +++ b/drivers/usb/dwc2/gadget.c > @@ -21,6 +21,7 @@ > #include <linux/platform_device.h> > #include <linux/dma-mapping.h> > #include <linux/debugfs.h> > +#include <linux/mutex.h> > #include <linux/seq_file.h> > #include <linux/delay.h> > #include <linux/io.h> > @@ -2908,6 +2909,7 @@ static int s3c_hsotg_udc_start(struct usb_gadget *gadget, > return -EINVAL; > } > > + mutex_lock(&hsotg->init_mutex); > WARN_ON(hsotg->driver); > > driver->driver.bus = NULL; > @@ -2933,9 +2935,12 @@ static int s3c_hsotg_udc_start(struct usb_gadget *gadget, > > dev_info(hsotg->dev, "bound driver %s\n", driver->driver.name); > > + mutex_unlock(&hsotg->init_mutex); > + > return 0; > > err: > + mutex_unlock(&hsotg->init_mutex); > hsotg->driver = NULL; > return ret; > } > @@ -2957,6 +2962,8 @@ static int s3c_hsotg_udc_stop(struct usb_gadget *gadget, > if (!hsotg) > return -ENODEV; > > + mutex_lock(&hsotg->init_mutex); > + > /* all endpoints should be shutdown */ > for (ep = 1; ep < hsotg->num_of_eps; ep++) > s3c_hsotg_ep_disable(&hsotg->eps[ep].ep); > @@ -2974,6 +2981,8 @@ static int s3c_hsotg_udc_stop(struct usb_gadget *gadget, > > clk_disable(hsotg->clk); > > + mutex_unlock(&hsotg->init_mutex); > + > return 0; > } > > @@ -3002,6 +3011,7 @@ static int s3c_hsotg_pullup(struct usb_gadget *gadget, int is_on) > > dev_dbg(hsotg->dev, "%s: is_on: %d\n", __func__, is_on); > > + mutex_lock(&hsotg->init_mutex); > spin_lock_irqsave(&hsotg->lock, flags); > if (is_on) { > clk_enable(hsotg->clk); > @@ -3013,6 +3023,7 @@ static int s3c_hsotg_pullup(struct usb_gadget *gadget, int is_on) > > hsotg->gadget.speed = USB_SPEED_UNKNOWN; > spin_unlock_irqrestore(&hsotg->lock, flags); > + mutex_unlock(&hsotg->init_mutex); > > return 0; > } > @@ -3507,6 +3518,7 @@ static int s3c_hsotg_probe(struct platform_device *pdev) > } > > spin_lock_init(&hsotg->lock); > + mutex_init(&hsotg->init_mutex); > > hsotg->irq = ret; > > @@ -3652,6 +3664,8 @@ static int s3c_hsotg_suspend(struct platform_device *pdev, pm_message_t state) > unsigned long flags; > int ret = 0; > > + mutex_lock(&hsotg->init_mutex); > + > if (hsotg->driver) > dev_info(hsotg->dev, "suspending usb gadget %s\n", > hsotg->driver->driver.name); > @@ -3674,6 +3688,8 @@ static int s3c_hsotg_suspend(struct platform_device *pdev, pm_message_t state) > clk_disable(hsotg->clk); > } > > + mutex_unlock(&hsotg->init_mutex); > + > return ret; > } > > @@ -3683,7 +3699,9 @@ static int s3c_hsotg_resume(struct platform_device *pdev) > unsigned long flags; > int ret = 0; > > + mutex_lock(&hsotg->init_mutex); > if (hsotg->driver) { > + > dev_info(hsotg->dev, "resuming usb gadget %s\n", > hsotg->driver->driver.name); > > @@ -3699,6 +3717,8 @@ static int s3c_hsotg_resume(struct platform_device *pdev) > s3c_hsotg_core_connect(hsotg); > spin_unlock_irqrestore(&hsotg->lock, flags); > > + mutex_unlock(&hsotg->init_mutex); > + > return ret; > } > Hmm. I can't find any other UDC driver that uses a mutex in its suspend/resume functions. Can you explain why this is needed only for dwc2?
Hello, On 2014-10-31 19:46, Paul Zimmerman wrote: >> From: Marek Szyprowski [mailto:m.szyprowski@samsung.com] >> Sent: Friday, October 31, 2014 3:13 AM >> >> This patch adds mutex, which protects initialization and >> deinitialization procedures against suspend/resume methods. >> >> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> >> --- >> drivers/usb/dwc2/core.h | 1 + >> drivers/usb/dwc2/gadget.c | 20 ++++++++++++++++++++ >> 2 files changed, 21 insertions(+) >> >> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h >> index 9f77b4d1c5ff..58732a9a0019 100644 >> --- a/drivers/usb/dwc2/core.h >> +++ b/drivers/usb/dwc2/core.h >> @@ -187,6 +187,7 @@ struct s3c_hsotg { >> struct s3c_hsotg_plat *plat; >> >> spinlock_t lock; >> + struct mutex init_mutex; >> >> void __iomem *regs; >> int irq; >> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c >> index d8dda39c9e16..a2e4272a904e 100644 >> --- a/drivers/usb/dwc2/gadget.c >> +++ b/drivers/usb/dwc2/gadget.c >> @@ -21,6 +21,7 @@ >> #include <linux/platform_device.h> >> #include <linux/dma-mapping.h> >> #include <linux/debugfs.h> >> +#include <linux/mutex.h> >> #include <linux/seq_file.h> >> #include <linux/delay.h> >> #include <linux/io.h> >> @@ -2908,6 +2909,7 @@ static int s3c_hsotg_udc_start(struct usb_gadget *gadget, >> return -EINVAL; >> } >> >> + mutex_lock(&hsotg->init_mutex); >> WARN_ON(hsotg->driver); >> >> driver->driver.bus = NULL; >> @@ -2933,9 +2935,12 @@ static int s3c_hsotg_udc_start(struct usb_gadget *gadget, >> >> dev_info(hsotg->dev, "bound driver %s\n", driver->driver.name); >> >> + mutex_unlock(&hsotg->init_mutex); >> + >> return 0; >> >> err: >> + mutex_unlock(&hsotg->init_mutex); >> hsotg->driver = NULL; >> return ret; >> } >> @@ -2957,6 +2962,8 @@ static int s3c_hsotg_udc_stop(struct usb_gadget *gadget, >> if (!hsotg) >> return -ENODEV; >> >> + mutex_lock(&hsotg->init_mutex); >> + >> /* all endpoints should be shutdown */ >> for (ep = 1; ep < hsotg->num_of_eps; ep++) >> s3c_hsotg_ep_disable(&hsotg->eps[ep].ep); >> @@ -2974,6 +2981,8 @@ static int s3c_hsotg_udc_stop(struct usb_gadget *gadget, >> >> clk_disable(hsotg->clk); >> >> + mutex_unlock(&hsotg->init_mutex); >> + >> return 0; >> } >> >> @@ -3002,6 +3011,7 @@ static int s3c_hsotg_pullup(struct usb_gadget *gadget, int is_on) >> >> dev_dbg(hsotg->dev, "%s: is_on: %d\n", __func__, is_on); >> >> + mutex_lock(&hsotg->init_mutex); >> spin_lock_irqsave(&hsotg->lock, flags); >> if (is_on) { >> clk_enable(hsotg->clk); >> @@ -3013,6 +3023,7 @@ static int s3c_hsotg_pullup(struct usb_gadget *gadget, int is_on) >> >> hsotg->gadget.speed = USB_SPEED_UNKNOWN; >> spin_unlock_irqrestore(&hsotg->lock, flags); >> + mutex_unlock(&hsotg->init_mutex); >> >> return 0; >> } >> @@ -3507,6 +3518,7 @@ static int s3c_hsotg_probe(struct platform_device *pdev) >> } >> >> spin_lock_init(&hsotg->lock); >> + mutex_init(&hsotg->init_mutex); >> >> hsotg->irq = ret; >> >> @@ -3652,6 +3664,8 @@ static int s3c_hsotg_suspend(struct platform_device *pdev, pm_message_t state) >> unsigned long flags; >> int ret = 0; >> >> + mutex_lock(&hsotg->init_mutex); >> + >> if (hsotg->driver) >> dev_info(hsotg->dev, "suspending usb gadget %s\n", >> hsotg->driver->driver.name); >> @@ -3674,6 +3688,8 @@ static int s3c_hsotg_suspend(struct platform_device *pdev, pm_message_t state) >> clk_disable(hsotg->clk); >> } >> >> + mutex_unlock(&hsotg->init_mutex); >> + >> return ret; >> } >> >> @@ -3683,7 +3699,9 @@ static int s3c_hsotg_resume(struct platform_device *pdev) >> unsigned long flags; >> int ret = 0; >> >> + mutex_lock(&hsotg->init_mutex); >> if (hsotg->driver) { >> + >> dev_info(hsotg->dev, "resuming usb gadget %s\n", >> hsotg->driver->driver.name); >> >> @@ -3699,6 +3717,8 @@ static int s3c_hsotg_resume(struct platform_device *pdev) >> s3c_hsotg_core_connect(hsotg); >> spin_unlock_irqrestore(&hsotg->lock, flags); >> >> + mutex_unlock(&hsotg->init_mutex); >> + >> return ret; >> } >> > Hmm. I can't find any other UDC driver that uses a mutex in its > suspend/resume functions. Can you explain why this is needed only > for dwc2? I've posted this version because I thought you were not convinced that the patch "usb: dwc2/gadget: rework suspend/resume code to correctly restore gadget state" can add code for initialization and deinitialization in suspend/resume paths. Best regards
> From: Marek Szyprowski [mailto:m.szyprowski@samsung.com] > Sent: Thursday, November 13, 2014 7:18 AM > > On 2014-10-31 19:46, Paul Zimmerman wrote: > >> From: Marek Szyprowski [mailto:m.szyprowski@samsung.com] > >> Sent: Friday, October 31, 2014 3:13 AM > >> > >> This patch adds mutex, which protects initialization and > >> deinitialization procedures against suspend/resume methods. > >> > >> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> > >> --- > >> drivers/usb/dwc2/core.h | 1 + > >> drivers/usb/dwc2/gadget.c | 20 ++++++++++++++++++++ > >> 2 files changed, 21 insertions(+) > >> > >> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h > >> index 9f77b4d1c5ff..58732a9a0019 100644 > >> --- a/drivers/usb/dwc2/core.h > >> +++ b/drivers/usb/dwc2/core.h > >> @@ -187,6 +187,7 @@ struct s3c_hsotg { > >> struct s3c_hsotg_plat *plat; > >> > >> spinlock_t lock; > >> + struct mutex init_mutex; > >> > >> void __iomem *regs; > >> int irq; > >> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c > >> index d8dda39c9e16..a2e4272a904e 100644 > >> --- a/drivers/usb/dwc2/gadget.c > >> +++ b/drivers/usb/dwc2/gadget.c > >> @@ -21,6 +21,7 @@ > >> #include <linux/platform_device.h> > >> #include <linux/dma-mapping.h> > >> #include <linux/debugfs.h> > >> +#include <linux/mutex.h> > >> #include <linux/seq_file.h> > >> #include <linux/delay.h> > >> #include <linux/io.h> > >> @@ -2908,6 +2909,7 @@ static int s3c_hsotg_udc_start(struct usb_gadget *gadget, > >> return -EINVAL; > >> } > >> > >> + mutex_lock(&hsotg->init_mutex); > >> WARN_ON(hsotg->driver); > >> > >> driver->driver.bus = NULL; > >> @@ -2933,9 +2935,12 @@ static int s3c_hsotg_udc_start(struct usb_gadget *gadget, > >> > >> dev_info(hsotg->dev, "bound driver %s\n", driver->driver.name); > >> > >> + mutex_unlock(&hsotg->init_mutex); > >> + > >> return 0; > >> > >> err: > >> + mutex_unlock(&hsotg->init_mutex); > >> hsotg->driver = NULL; > >> return ret; > >> } > >> @@ -2957,6 +2962,8 @@ static int s3c_hsotg_udc_stop(struct usb_gadget *gadget, > >> if (!hsotg) > >> return -ENODEV; > >> > >> + mutex_lock(&hsotg->init_mutex); > >> + > >> /* all endpoints should be shutdown */ > >> for (ep = 1; ep < hsotg->num_of_eps; ep++) > >> s3c_hsotg_ep_disable(&hsotg->eps[ep].ep); > >> @@ -2974,6 +2981,8 @@ static int s3c_hsotg_udc_stop(struct usb_gadget *gadget, > >> > >> clk_disable(hsotg->clk); > >> > >> + mutex_unlock(&hsotg->init_mutex); > >> + > >> return 0; > >> } > >> > >> @@ -3002,6 +3011,7 @@ static int s3c_hsotg_pullup(struct usb_gadget *gadget, int is_on) > >> > >> dev_dbg(hsotg->dev, "%s: is_on: %d\n", __func__, is_on); > >> > >> + mutex_lock(&hsotg->init_mutex); > >> spin_lock_irqsave(&hsotg->lock, flags); > >> if (is_on) { > >> clk_enable(hsotg->clk); > >> @@ -3013,6 +3023,7 @@ static int s3c_hsotg_pullup(struct usb_gadget *gadget, int is_on) > >> > >> hsotg->gadget.speed = USB_SPEED_UNKNOWN; > >> spin_unlock_irqrestore(&hsotg->lock, flags); > >> + mutex_unlock(&hsotg->init_mutex); > >> > >> return 0; > >> } > >> @@ -3507,6 +3518,7 @@ static int s3c_hsotg_probe(struct platform_device *pdev) > >> } > >> > >> spin_lock_init(&hsotg->lock); > >> + mutex_init(&hsotg->init_mutex); > >> > >> hsotg->irq = ret; > >> > >> @@ -3652,6 +3664,8 @@ static int s3c_hsotg_suspend(struct platform_device *pdev, pm_message_t > state) > >> unsigned long flags; > >> int ret = 0; > >> > >> + mutex_lock(&hsotg->init_mutex); > >> + > >> if (hsotg->driver) > >> dev_info(hsotg->dev, "suspending usb gadget %s\n", > >> hsotg->driver->driver.name); > >> @@ -3674,6 +3688,8 @@ static int s3c_hsotg_suspend(struct platform_device *pdev, pm_message_t > state) > >> clk_disable(hsotg->clk); > >> } > >> > >> + mutex_unlock(&hsotg->init_mutex); > >> + > >> return ret; > >> } > >> > >> @@ -3683,7 +3699,9 @@ static int s3c_hsotg_resume(struct platform_device *pdev) > >> unsigned long flags; > >> int ret = 0; > >> > >> + mutex_lock(&hsotg->init_mutex); > >> if (hsotg->driver) { > >> + > >> dev_info(hsotg->dev, "resuming usb gadget %s\n", > >> hsotg->driver->driver.name); > >> > >> @@ -3699,6 +3717,8 @@ static int s3c_hsotg_resume(struct platform_device *pdev) > >> s3c_hsotg_core_connect(hsotg); > >> spin_unlock_irqrestore(&hsotg->lock, flags); > >> > >> + mutex_unlock(&hsotg->init_mutex); > >> + > >> return ret; > >> } > >> > > Hmm. I can't find any other UDC driver that uses a mutex in its > > suspend/resume functions. Can you explain why this is needed only > > for dwc2? > > I've posted this version because I thought you were not convinced that > the patch > "usb: dwc2/gadget: rework suspend/resume code to correctly restore > gadget state" > can add code for initialization and deinitialization in suspend/resume > paths. My problem with that patch was that you were checking the ->enabled flag outside of the spinlock. To address that, you only need to move the check inside of the spinlock. I don't see why a mutex is needed. -- Paul
Hello, On 2014-11-13 21:55, Paul Zimmerman wrote: >> From: Marek Szyprowski [mailto:m.szyprowski@samsung.com] >> Sent: Thursday, November 13, 2014 7:18 AM >> >> On 2014-10-31 19:46, Paul Zimmerman wrote: >>>> From: Marek Szyprowski [mailto:m.szyprowski@samsung.com] >>>> Sent: Friday, October 31, 2014 3:13 AM >>>> >>>> This patch adds mutex, which protects initialization and >>>> deinitialization procedures against suspend/resume methods. >>>> >>>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> >>>> --- >>>> drivers/usb/dwc2/core.h | 1 + >>>> drivers/usb/dwc2/gadget.c | 20 ++++++++++++++++++++ >>>> 2 files changed, 21 insertions(+) >>>> >>>> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h >>>> index 9f77b4d1c5ff..58732a9a0019 100644 >>>> --- a/drivers/usb/dwc2/core.h >>>> +++ b/drivers/usb/dwc2/core.h >>>> @@ -187,6 +187,7 @@ struct s3c_hsotg { >>>> struct s3c_hsotg_plat *plat; >>>> >>>> spinlock_t lock; >>>> + struct mutex init_mutex; >>>> >>>> void __iomem *regs; >>>> int irq; >>>> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c >>>> index d8dda39c9e16..a2e4272a904e 100644 >>>> --- a/drivers/usb/dwc2/gadget.c >>>> +++ b/drivers/usb/dwc2/gadget.c >>>> @@ -21,6 +21,7 @@ >>>> #include <linux/platform_device.h> >>>> #include <linux/dma-mapping.h> >>>> #include <linux/debugfs.h> >>>> +#include <linux/mutex.h> >>>> #include <linux/seq_file.h> >>>> #include <linux/delay.h> >>>> #include <linux/io.h> >>>> @@ -2908,6 +2909,7 @@ static int s3c_hsotg_udc_start(struct usb_gadget *gadget, >>>> return -EINVAL; >>>> } >>>> >>>> + mutex_lock(&hsotg->init_mutex); >>>> WARN_ON(hsotg->driver); >>>> >>>> driver->driver.bus = NULL; >>>> @@ -2933,9 +2935,12 @@ static int s3c_hsotg_udc_start(struct usb_gadget *gadget, >>>> >>>> dev_info(hsotg->dev, "bound driver %s\n", driver->driver.name); >>>> >>>> + mutex_unlock(&hsotg->init_mutex); >>>> + >>>> return 0; >>>> >>>> err: >>>> + mutex_unlock(&hsotg->init_mutex); >>>> hsotg->driver = NULL; >>>> return ret; >>>> } >>>> @@ -2957,6 +2962,8 @@ static int s3c_hsotg_udc_stop(struct usb_gadget *gadget, >>>> if (!hsotg) >>>> return -ENODEV; >>>> >>>> + mutex_lock(&hsotg->init_mutex); >>>> + >>>> /* all endpoints should be shutdown */ >>>> for (ep = 1; ep < hsotg->num_of_eps; ep++) >>>> s3c_hsotg_ep_disable(&hsotg->eps[ep].ep); >>>> @@ -2974,6 +2981,8 @@ static int s3c_hsotg_udc_stop(struct usb_gadget *gadget, >>>> >>>> clk_disable(hsotg->clk); >>>> >>>> + mutex_unlock(&hsotg->init_mutex); >>>> + >>>> return 0; >>>> } >>>> >>>> @@ -3002,6 +3011,7 @@ static int s3c_hsotg_pullup(struct usb_gadget *gadget, int is_on) >>>> >>>> dev_dbg(hsotg->dev, "%s: is_on: %d\n", __func__, is_on); >>>> >>>> + mutex_lock(&hsotg->init_mutex); >>>> spin_lock_irqsave(&hsotg->lock, flags); >>>> if (is_on) { >>>> clk_enable(hsotg->clk); >>>> @@ -3013,6 +3023,7 @@ static int s3c_hsotg_pullup(struct usb_gadget *gadget, int is_on) >>>> >>>> hsotg->gadget.speed = USB_SPEED_UNKNOWN; >>>> spin_unlock_irqrestore(&hsotg->lock, flags); >>>> + mutex_unlock(&hsotg->init_mutex); >>>> >>>> return 0; >>>> } >>>> @@ -3507,6 +3518,7 @@ static int s3c_hsotg_probe(struct platform_device *pdev) >>>> } >>>> >>>> spin_lock_init(&hsotg->lock); >>>> + mutex_init(&hsotg->init_mutex); >>>> >>>> hsotg->irq = ret; >>>> >>>> @@ -3652,6 +3664,8 @@ static int s3c_hsotg_suspend(struct platform_device *pdev, pm_message_t >> state) >>>> unsigned long flags; >>>> int ret = 0; >>>> >>>> + mutex_lock(&hsotg->init_mutex); >>>> + >>>> if (hsotg->driver) >>>> dev_info(hsotg->dev, "suspending usb gadget %s\n", >>>> hsotg->driver->driver.name); >>>> @@ -3674,6 +3688,8 @@ static int s3c_hsotg_suspend(struct platform_device *pdev, pm_message_t >> state) >>>> clk_disable(hsotg->clk); >>>> } >>>> >>>> + mutex_unlock(&hsotg->init_mutex); >>>> + >>>> return ret; >>>> } >>>> >>>> @@ -3683,7 +3699,9 @@ static int s3c_hsotg_resume(struct platform_device *pdev) >>>> unsigned long flags; >>>> int ret = 0; >>>> >>>> + mutex_lock(&hsotg->init_mutex); >>>> if (hsotg->driver) { >>>> + >>>> dev_info(hsotg->dev, "resuming usb gadget %s\n", >>>> hsotg->driver->driver.name); >>>> >>>> @@ -3699,6 +3717,8 @@ static int s3c_hsotg_resume(struct platform_device *pdev) >>>> s3c_hsotg_core_connect(hsotg); >>>> spin_unlock_irqrestore(&hsotg->lock, flags); >>>> >>>> + mutex_unlock(&hsotg->init_mutex); >>>> + >>>> return ret; >>>> } >>>> >>> Hmm. I can't find any other UDC driver that uses a mutex in its >>> suspend/resume functions. Can you explain why this is needed only >>> for dwc2? >> I've posted this version because I thought you were not convinced that >> the patch >> "usb: dwc2/gadget: rework suspend/resume code to correctly restore >> gadget state" >> can add code for initialization and deinitialization in suspend/resume >> paths. > My problem with that patch was that you were checking the ->enabled > flag outside of the spinlock. To address that, you only need to move > the check inside of the spinlock. I don't see why a mutex is needed. It is not that simple. I can add spin_lock() before checking enabled, but then I would need to spin_unlock() to call regulator_bulk_enable() and phy_enable(), because both cannot be called from atomic context. This means that the spinlock in such case will not protect anything and is simply useless. Best regards
PiBGcm9tOiBNYXJlayBTenlwcm93c2tpIFttYWlsdG86bS5zenlwcm93c2tpQHNhbXN1bmcuY29t XQ0KPiBTZW50OiBGcmlkYXksIE5vdmVtYmVyIDE0LCAyMDE0IDE6MTkgQU0NCj4gDQo+IE9uIDIw MTQtMTEtMTMgMjE6NTUsIFBhdWwgWmltbWVybWFuIHdyb3RlOg0KPiA+PiBGcm9tOiBNYXJlayBT enlwcm93c2tpIFttYWlsdG86bS5zenlwcm93c2tpQHNhbXN1bmcuY29tXQ0KPiA+PiBTZW50OiBU aHVyc2RheSwgTm92ZW1iZXIgMTMsIDIwMTQgNzoxOCBBTQ0KPiA+Pg0KPiA+PiBPbiAyMDE0LTEw LTMxIDE5OjQ2LCBQYXVsIFppbW1lcm1hbiB3cm90ZToNCj4gPj4+PiBGcm9tOiBNYXJlayBTenlw cm93c2tpIFttYWlsdG86bS5zenlwcm93c2tpQHNhbXN1bmcuY29tXQ0KPiA+Pj4+IFNlbnQ6IEZy aWRheSwgT2N0b2JlciAzMSwgMjAxNCAzOjEzIEFNDQo+ID4+Pj4NCj4gPj4+PiBUaGlzIHBhdGNo IGFkZHMgbXV0ZXgsIHdoaWNoIHByb3RlY3RzIGluaXRpYWxpemF0aW9uIGFuZA0KPiA+Pj4+IGRl aW5pdGlhbGl6YXRpb24gcHJvY2VkdXJlcyBhZ2FpbnN0IHN1c3BlbmQvcmVzdW1lIG1ldGhvZHMu DQo+ID4+Pj4NCj4gPj4+PiBTaWduZWQtb2ZmLWJ5OiBNYXJlayBTenlwcm93c2tpIDxtLnN6eXBy b3dza2lAc2Ftc3VuZy5jb20+DQo+ID4+Pj4gLS0tDQo+ID4+Pj4gICAgZHJpdmVycy91c2IvZHdj Mi9jb3JlLmggICB8ICAxICsNCj4gPj4+PiAgICBkcml2ZXJzL3VzYi9kd2MyL2dhZGdldC5jIHwg MjAgKysrKysrKysrKysrKysrKysrKysNCj4gPj4+PiAgICAyIGZpbGVzIGNoYW5nZWQsIDIxIGlu c2VydGlvbnMoKykNCj4gPj4+Pg0KPiA+Pj4+IGRpZmYgLS1naXQgYS9kcml2ZXJzL3VzYi9kd2My L2NvcmUuaCBiL2RyaXZlcnMvdXNiL2R3YzIvY29yZS5oDQo+ID4+Pj4gaW5kZXggOWY3N2I0ZDFj NWZmLi41ODczMmE5YTAwMTkgMTAwNjQ0DQo+ID4+Pj4gLS0tIGEvZHJpdmVycy91c2IvZHdjMi9j b3JlLmgNCj4gPj4+PiArKysgYi9kcml2ZXJzL3VzYi9kd2MyL2NvcmUuaA0KPiA+Pj4+IEBAIC0x ODcsNiArMTg3LDcgQEAgc3RydWN0IHMzY19oc290ZyB7DQo+ID4+Pj4gICAgCXN0cnVjdCBzM2Nf aHNvdGdfcGxhdCAgICAqcGxhdDsNCj4gPj4+Pg0KPiA+Pj4+ICAgIAlzcGlubG9ja190ICAgICAg ICAgICAgICBsb2NrOw0KPiA+Pj4+ICsJc3RydWN0IG11dGV4CQlpbml0X211dGV4Ow0KPiA+Pj4+ DQo+ID4+Pj4gICAgCXZvaWQgX19pb21lbSAgICAgICAgICAgICpyZWdzOw0KPiA+Pj4+ICAgIAlp bnQgICAgICAgICAgICAgICAgICAgICBpcnE7DQo+ID4+Pj4gZGlmZiAtLWdpdCBhL2RyaXZlcnMv dXNiL2R3YzIvZ2FkZ2V0LmMgYi9kcml2ZXJzL3VzYi9kd2MyL2dhZGdldC5jDQo+ID4+Pj4gaW5k ZXggZDhkZGEzOWM5ZTE2Li5hMmU0MjcyYTkwNGUgMTAwNjQ0DQo+ID4+Pj4gLS0tIGEvZHJpdmVy cy91c2IvZHdjMi9nYWRnZXQuYw0KPiA+Pj4+ICsrKyBiL2RyaXZlcnMvdXNiL2R3YzIvZ2FkZ2V0 LmMNCj4gPj4+PiBAQCAtMjEsNiArMjEsNyBAQA0KPiA+Pj4+ICAgICNpbmNsdWRlIDxsaW51eC9w bGF0Zm9ybV9kZXZpY2UuaD4NCj4gPj4+PiAgICAjaW5jbHVkZSA8bGludXgvZG1hLW1hcHBpbmcu aD4NCj4gPj4+PiAgICAjaW5jbHVkZSA8bGludXgvZGVidWdmcy5oPg0KPiA+Pj4+ICsjaW5jbHVk ZSA8bGludXgvbXV0ZXguaD4NCj4gPj4+PiAgICAjaW5jbHVkZSA8bGludXgvc2VxX2ZpbGUuaD4N Cj4gPj4+PiAgICAjaW5jbHVkZSA8bGludXgvZGVsYXkuaD4NCj4gPj4+PiAgICAjaW5jbHVkZSA8 bGludXgvaW8uaD4NCj4gPj4+PiBAQCAtMjkwOCw2ICsyOTA5LDcgQEAgc3RhdGljIGludCBzM2Nf aHNvdGdfdWRjX3N0YXJ0KHN0cnVjdCB1c2JfZ2FkZ2V0ICpnYWRnZXQsDQo+ID4+Pj4gICAgCQly ZXR1cm4gLUVJTlZBTDsNCj4gPj4+PiAgICAJfQ0KPiA+Pj4+DQo+ID4+Pj4gKwltdXRleF9sb2Nr KCZoc290Zy0+aW5pdF9tdXRleCk7DQo+ID4+Pj4gICAgCVdBUk5fT04oaHNvdGctPmRyaXZlcik7 DQo+ID4+Pj4NCj4gPj4+PiAgICAJZHJpdmVyLT5kcml2ZXIuYnVzID0gTlVMTDsNCj4gPj4+PiBA QCAtMjkzMyw5ICsyOTM1LDEyIEBAIHN0YXRpYyBpbnQgczNjX2hzb3RnX3VkY19zdGFydChzdHJ1 Y3QgdXNiX2dhZGdldCAqZ2FkZ2V0LA0KPiA+Pj4+DQo+ID4+Pj4gICAgCWRldl9pbmZvKGhzb3Rn LT5kZXYsICJib3VuZCBkcml2ZXIgJXNcbiIsIGRyaXZlci0+ZHJpdmVyLm5hbWUpOw0KPiA+Pj4+ DQo+ID4+Pj4gKwltdXRleF91bmxvY2soJmhzb3RnLT5pbml0X211dGV4KTsNCj4gPj4+PiArDQo+ ID4+Pj4gICAgCXJldHVybiAwOw0KPiA+Pj4+DQo+ID4+Pj4gICAgZXJyOg0KPiA+Pj4+ICsJbXV0 ZXhfdW5sb2NrKCZoc290Zy0+aW5pdF9tdXRleCk7DQo+ID4+Pj4gICAgCWhzb3RnLT5kcml2ZXIg PSBOVUxMOw0KPiA+Pj4+ICAgIAlyZXR1cm4gcmV0Ow0KPiA+Pj4+ICAgIH0NCj4gPj4+PiBAQCAt Mjk1Nyw2ICsyOTYyLDggQEAgc3RhdGljIGludCBzM2NfaHNvdGdfdWRjX3N0b3Aoc3RydWN0IHVz Yl9nYWRnZXQgKmdhZGdldCwNCj4gPj4+PiAgICAJaWYgKCFoc290ZykNCj4gPj4+PiAgICAJCXJl dHVybiAtRU5PREVWOw0KPiA+Pj4+DQo+ID4+Pj4gKwltdXRleF9sb2NrKCZoc290Zy0+aW5pdF9t dXRleCk7DQo+ID4+Pj4gKw0KPiA+Pj4+ICAgIAkvKiBhbGwgZW5kcG9pbnRzIHNob3VsZCBiZSBz aHV0ZG93biAqLw0KPiA+Pj4+ICAgIAlmb3IgKGVwID0gMTsgZXAgPCBoc290Zy0+bnVtX29mX2Vw czsgZXArKykNCj4gPj4+PiAgICAJCXMzY19oc290Z19lcF9kaXNhYmxlKCZoc290Zy0+ZXBzW2Vw XS5lcCk7DQo+ID4+Pj4gQEAgLTI5NzQsNiArMjk4MSw4IEBAIHN0YXRpYyBpbnQgczNjX2hzb3Rn X3VkY19zdG9wKHN0cnVjdCB1c2JfZ2FkZ2V0ICpnYWRnZXQsDQo+ID4+Pj4NCj4gPj4+PiAgICAJ Y2xrX2Rpc2FibGUoaHNvdGctPmNsayk7DQo+ID4+Pj4NCj4gPj4+PiArCW11dGV4X3VubG9jaygm aHNvdGctPmluaXRfbXV0ZXgpOw0KPiA+Pj4+ICsNCj4gPj4+PiAgICAJcmV0dXJuIDA7DQo+ID4+ Pj4gICAgfQ0KPiA+Pj4+DQo+ID4+Pj4gQEAgLTMwMDIsNiArMzAxMSw3IEBAIHN0YXRpYyBpbnQg czNjX2hzb3RnX3B1bGx1cChzdHJ1Y3QgdXNiX2dhZGdldCAqZ2FkZ2V0LCBpbnQgaXNfb24pDQo+ ID4+Pj4NCj4gPj4+PiAgICAJZGV2X2RiZyhoc290Zy0+ZGV2LCAiJXM6IGlzX29uOiAlZFxuIiwg X19mdW5jX18sIGlzX29uKTsNCj4gPj4+Pg0KPiA+Pj4+ICsJbXV0ZXhfbG9jaygmaHNvdGctPmlu aXRfbXV0ZXgpOw0KPiA+Pj4+ICAgIAlzcGluX2xvY2tfaXJxc2F2ZSgmaHNvdGctPmxvY2ssIGZs YWdzKTsNCj4gPj4+PiAgICAJaWYgKGlzX29uKSB7DQo+ID4+Pj4gICAgCQljbGtfZW5hYmxlKGhz b3RnLT5jbGspOw0KPiA+Pj4+IEBAIC0zMDEzLDYgKzMwMjMsNyBAQCBzdGF0aWMgaW50IHMzY19o c290Z19wdWxsdXAoc3RydWN0IHVzYl9nYWRnZXQgKmdhZGdldCwgaW50IGlzX29uKQ0KPiA+Pj4+ DQo+ID4+Pj4gICAgCWhzb3RnLT5nYWRnZXQuc3BlZWQgPSBVU0JfU1BFRURfVU5LTk9XTjsNCj4g Pj4+PiAgICAJc3Bpbl91bmxvY2tfaXJxcmVzdG9yZSgmaHNvdGctPmxvY2ssIGZsYWdzKTsNCj4g Pj4+PiArCW11dGV4X3VubG9jaygmaHNvdGctPmluaXRfbXV0ZXgpOw0KPiA+Pj4+DQo+ID4+Pj4g ICAgCXJldHVybiAwOw0KPiA+Pj4+ICAgIH0NCj4gPj4+PiBAQCAtMzUwNyw2ICszNTE4LDcgQEAg c3RhdGljIGludCBzM2NfaHNvdGdfcHJvYmUoc3RydWN0IHBsYXRmb3JtX2RldmljZSAqcGRldikN Cj4gPj4+PiAgICAJfQ0KPiA+Pj4+DQo+ID4+Pj4gICAgCXNwaW5fbG9ja19pbml0KCZoc290Zy0+ bG9jayk7DQo+ID4+Pj4gKwltdXRleF9pbml0KCZoc290Zy0+aW5pdF9tdXRleCk7DQo+ID4+Pj4N Cj4gPj4+PiAgICAJaHNvdGctPmlycSA9IHJldDsNCj4gPj4+Pg0KPiA+Pj4+IEBAIC0zNjUyLDYg KzM2NjQsOCBAQCBzdGF0aWMgaW50IHMzY19oc290Z19zdXNwZW5kKHN0cnVjdCBwbGF0Zm9ybV9k ZXZpY2UgKnBkZXYsIHBtX21lc3NhZ2VfdA0KPiA+PiBzdGF0ZSkNCj4gPj4+PiAgICAJdW5zaWdu ZWQgbG9uZyBmbGFnczsNCj4gPj4+PiAgICAJaW50IHJldCA9IDA7DQo+ID4+Pj4NCj4gPj4+PiAr CW11dGV4X2xvY2soJmhzb3RnLT5pbml0X211dGV4KTsNCj4gPj4+PiArDQo+ID4+Pj4gICAgCWlm IChoc290Zy0+ZHJpdmVyKQ0KPiA+Pj4+ICAgIAkJZGV2X2luZm8oaHNvdGctPmRldiwgInN1c3Bl bmRpbmcgdXNiIGdhZGdldCAlc1xuIiwNCj4gPj4+PiAgICAJCQkgaHNvdGctPmRyaXZlci0+ZHJp dmVyLm5hbWUpOw0KPiA+Pj4+IEBAIC0zNjc0LDYgKzM2ODgsOCBAQCBzdGF0aWMgaW50IHMzY19o c290Z19zdXNwZW5kKHN0cnVjdCBwbGF0Zm9ybV9kZXZpY2UgKnBkZXYsIHBtX21lc3NhZ2VfdA0K PiA+PiBzdGF0ZSkNCj4gPj4+PiAgICAJCWNsa19kaXNhYmxlKGhzb3RnLT5jbGspOw0KPiA+Pj4+ ICAgIAl9DQo+ID4+Pj4NCj4gPj4+PiArCW11dGV4X3VubG9jaygmaHNvdGctPmluaXRfbXV0ZXgp Ow0KPiA+Pj4+ICsNCj4gPj4+PiAgICAJcmV0dXJuIHJldDsNCj4gPj4+PiAgICB9DQo+ID4+Pj4N Cj4gPj4+PiBAQCAtMzY4Myw3ICszNjk5LDkgQEAgc3RhdGljIGludCBzM2NfaHNvdGdfcmVzdW1l KHN0cnVjdCBwbGF0Zm9ybV9kZXZpY2UgKnBkZXYpDQo+ID4+Pj4gICAgCXVuc2lnbmVkIGxvbmcg ZmxhZ3M7DQo+ID4+Pj4gICAgCWludCByZXQgPSAwOw0KPiA+Pj4+DQo+ID4+Pj4gKwltdXRleF9s b2NrKCZoc290Zy0+aW5pdF9tdXRleCk7DQo+ID4+Pj4gICAgCWlmIChoc290Zy0+ZHJpdmVyKSB7 DQo+ID4+Pj4gKw0KPiA+Pj4+ICAgIAkJZGV2X2luZm8oaHNvdGctPmRldiwgInJlc3VtaW5nIHVz YiBnYWRnZXQgJXNcbiIsDQo+ID4+Pj4gICAgCQkJIGhzb3RnLT5kcml2ZXItPmRyaXZlci5uYW1l KTsNCj4gPj4+Pg0KPiA+Pj4+IEBAIC0zNjk5LDYgKzM3MTcsOCBAQCBzdGF0aWMgaW50IHMzY19o c290Z19yZXN1bWUoc3RydWN0IHBsYXRmb3JtX2RldmljZSAqcGRldikNCj4gPj4+PiAgICAJczNj X2hzb3RnX2NvcmVfY29ubmVjdChoc290Zyk7DQo+ID4+Pj4gICAgCXNwaW5fdW5sb2NrX2lycXJl c3RvcmUoJmhzb3RnLT5sb2NrLCBmbGFncyk7DQo+ID4+Pj4NCj4gPj4+PiArCW11dGV4X3VubG9j aygmaHNvdGctPmluaXRfbXV0ZXgpOw0KPiA+Pj4+ICsNCj4gPj4+PiAgICAJcmV0dXJuIHJldDsN Cj4gPj4+PiAgICB9DQo+ID4+Pj4NCj4gPj4+IEhtbS4gSSBjYW4ndCBmaW5kIGFueSBvdGhlciBV REMgZHJpdmVyIHRoYXQgdXNlcyBhIG11dGV4IGluIGl0cw0KPiA+Pj4gc3VzcGVuZC9yZXN1bWUg ZnVuY3Rpb25zLiBDYW4geW91IGV4cGxhaW4gd2h5IHRoaXMgaXMgbmVlZGVkIG9ubHkNCj4gPj4+ IGZvciBkd2MyPw0KPiA+PiBJJ3ZlIHBvc3RlZCB0aGlzIHZlcnNpb24gYmVjYXVzZSBJIHRob3Vn aHQgeW91IHdlcmUgbm90IGNvbnZpbmNlZCB0aGF0DQo+ID4+IHRoZSBwYXRjaA0KPiA+PiAidXNi OiBkd2MyL2dhZGdldDogcmV3b3JrIHN1c3BlbmQvcmVzdW1lIGNvZGUgdG8gY29ycmVjdGx5IHJl c3RvcmUNCj4gPj4gZ2FkZ2V0IHN0YXRlIg0KPiA+PiBjYW4gYWRkIGNvZGUgZm9yIGluaXRpYWxp emF0aW9uIGFuZCBkZWluaXRpYWxpemF0aW9uIGluIHN1c3BlbmQvcmVzdW1lDQo+ID4+IHBhdGhz Lg0KPiA+IE15IHByb2JsZW0gd2l0aCB0aGF0IHBhdGNoIHdhcyB0aGF0IHlvdSB3ZXJlIGNoZWNr aW5nIHRoZSAtPmVuYWJsZWQNCj4gPiBmbGFnIG91dHNpZGUgb2YgdGhlIHNwaW5sb2NrLiBUbyBh ZGRyZXNzIHRoYXQsIHlvdSBvbmx5IG5lZWQgdG8gbW92ZQ0KPiA+IHRoZSBjaGVjayBpbnNpZGUg b2YgdGhlIHNwaW5sb2NrLiBJIGRvbid0IHNlZSB3aHkgYSBtdXRleCBpcyBuZWVkZWQuDQo+IA0K PiBJdCBpcyBub3QgdGhhdCBzaW1wbGUuIEkgY2FuIGFkZCBzcGluX2xvY2soKSBiZWZvcmUgY2hl Y2tpbmcgZW5hYmxlZCwNCj4gYnV0IHRoZW4NCj4gSSB3b3VsZCBuZWVkIHRvIHNwaW5fdW5sb2Nr KCkgdG8gY2FsbCByZWd1bGF0b3JfYnVsa19lbmFibGUoKSBhbmQNCj4gcGh5X2VuYWJsZSgpLA0K PiBiZWNhdXNlIGJvdGggY2Fubm90IGJlIGNhbGxlZCBmcm9tIGF0b21pYyBjb250ZXh0LiBUaGlz IG1lYW5zIHRoYXQgdGhlDQo+IHNwaW5sb2NrDQo+IGluIHN1Y2ggY2FzZSB3aWxsIG5vdCBwcm90 ZWN0IGFueXRoaW5nIGFuZCBpcyBzaW1wbHkgdXNlbGVzcy4NCg0KQWgsIE9LLiBTbyB5b3UncmUg dXNpbmcgdGhlIG11dGV4IGluc3RlYWQgb2YgdGhlIC0+ZW5hYmxlZCBmbGFnIHRoYXQgeW91DQpw cm9wb3NlZCBpbiB0aGUgInJld29yayBzdXNwZW5kL3Jlc3VtZSBjb2RlIiBwYXRjaC4gU28gdGhp cyBwYXRjaCBpcyBhDQpyZXBsYWNlbWVudCBmb3IgdGhhdCBvbmUuIFNvbWVob3cgSSB3YXMgdGhp bmtpbmcgdGhpcyBwYXRjaCB3YXMgb24gdG9wDQpvZiB0aGF0IG9uZS4NCg0KU28gSSBndWVzcyB0 aGlzIGlzIE9LLCBidXQgSSB3b3VsZCBsaWtlIHRvIGdldCBGZWxpcGUncyBvcGluaW9uIGFib3V0 DQppdCBiZWZvcmUgd2UgYXBwbHkgdGhpcy4NCg0KRmVsaXBlPw0KDQotLSANClBhdWwNCg0K -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On Fri, Nov 14, 2014 at 07:43:23PM +0000, Paul Zimmerman wrote: > > >>>> @@ -3699,6 +3717,8 @@ static int s3c_hsotg_resume(struct platform_device *pdev) > > >>>> s3c_hsotg_core_connect(hsotg); > > >>>> spin_unlock_irqrestore(&hsotg->lock, flags); > > >>>> > > >>>> + mutex_unlock(&hsotg->init_mutex); > > >>>> + > > >>>> return ret; > > >>>> } > > >>>> > > >>> Hmm. I can't find any other UDC driver that uses a mutex in its > > >>> suspend/resume functions. Can you explain why this is needed only > > >>> for dwc2? > > >> I've posted this version because I thought you were not convinced that > > >> the patch > > >> "usb: dwc2/gadget: rework suspend/resume code to correctly restore > > >> gadget state" > > >> can add code for initialization and deinitialization in suspend/resume > > >> paths. > > > My problem with that patch was that you were checking the ->enabled > > > flag outside of the spinlock. To address that, you only need to move > > > the check inside of the spinlock. I don't see why a mutex is needed. > > > > It is not that simple. I can add spin_lock() before checking enabled, > > but then > > I would need to spin_unlock() to call regulator_bulk_enable() and > > phy_enable(), > > because both cannot be called from atomic context. This means that the > > spinlock > > in such case will not protect anything and is simply useless. > > Ah, OK. So you're using the mutex instead of the ->enabled flag that you > proposed in the "rework suspend/resume code" patch. So this patch is a > replacement for that one. Somehow I was thinking this patch was on top > of that one. > > So I guess this is OK, but I would like to get Felipe's opinion about > it before we apply this. > > Felipe? I can't think of a better way, I'm afraid :-(
> From: Marek Szyprowski [mailto:m.szyprowski@samsung.com] > Sent: Friday, October 31, 2014 3:13 AM > > This patch adds mutex, which protects initialization and > deinitialization procedures against suspend/resume methods. > > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> > --- > drivers/usb/dwc2/core.h | 1 + > drivers/usb/dwc2/gadget.c | 20 ++++++++++++++++++++ > 2 files changed, 21 insertions(+) > > diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h > index 9f77b4d1c5ff..58732a9a0019 100644 > --- a/drivers/usb/dwc2/core.h > +++ b/drivers/usb/dwc2/core.h > @@ -187,6 +187,7 @@ struct s3c_hsotg { > struct s3c_hsotg_plat *plat; > > spinlock_t lock; > + struct mutex init_mutex; > > void __iomem *regs; > int irq; > diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c > index d8dda39c9e16..a2e4272a904e 100644 > --- a/drivers/usb/dwc2/gadget.c > +++ b/drivers/usb/dwc2/gadget.c > @@ -21,6 +21,7 @@ > #include <linux/platform_device.h> > #include <linux/dma-mapping.h> > #include <linux/debugfs.h> > +#include <linux/mutex.h> > #include <linux/seq_file.h> > #include <linux/delay.h> > #include <linux/io.h> > @@ -2908,6 +2909,7 @@ static int s3c_hsotg_udc_start(struct usb_gadget *gadget, > return -EINVAL; > } > > + mutex_lock(&hsotg->init_mutex); > WARN_ON(hsotg->driver); > > driver->driver.bus = NULL; > @@ -2933,9 +2935,12 @@ static int s3c_hsotg_udc_start(struct usb_gadget *gadget, > > dev_info(hsotg->dev, "bound driver %s\n", driver->driver.name); > > + mutex_unlock(&hsotg->init_mutex); > + > return 0; > > err: > + mutex_unlock(&hsotg->init_mutex); > hsotg->driver = NULL; > return ret; > } > @@ -2957,6 +2962,8 @@ static int s3c_hsotg_udc_stop(struct usb_gadget *gadget, > if (!hsotg) > return -ENODEV; > > + mutex_lock(&hsotg->init_mutex); > + > /* all endpoints should be shutdown */ > for (ep = 1; ep < hsotg->num_of_eps; ep++) > s3c_hsotg_ep_disable(&hsotg->eps[ep].ep); > @@ -2974,6 +2981,8 @@ static int s3c_hsotg_udc_stop(struct usb_gadget *gadget, > > clk_disable(hsotg->clk); > > + mutex_unlock(&hsotg->init_mutex); > + > return 0; > } > > @@ -3002,6 +3011,7 @@ static int s3c_hsotg_pullup(struct usb_gadget *gadget, int is_on) > > dev_dbg(hsotg->dev, "%s: is_on: %d\n", __func__, is_on); > > + mutex_lock(&hsotg->init_mutex); > spin_lock_irqsave(&hsotg->lock, flags); > if (is_on) { > clk_enable(hsotg->clk); > @@ -3013,6 +3023,7 @@ static int s3c_hsotg_pullup(struct usb_gadget *gadget, int is_on) > > hsotg->gadget.speed = USB_SPEED_UNKNOWN; > spin_unlock_irqrestore(&hsotg->lock, flags); > + mutex_unlock(&hsotg->init_mutex); > > return 0; > } > @@ -3507,6 +3518,7 @@ static int s3c_hsotg_probe(struct platform_device *pdev) > } > > spin_lock_init(&hsotg->lock); > + mutex_init(&hsotg->init_mutex); > > hsotg->irq = ret; > > @@ -3652,6 +3664,8 @@ static int s3c_hsotg_suspend(struct platform_device *pdev, pm_message_t state) > unsigned long flags; > int ret = 0; > > + mutex_lock(&hsotg->init_mutex); > + > if (hsotg->driver) > dev_info(hsotg->dev, "suspending usb gadget %s\n", > hsotg->driver->driver.name); > @@ -3674,6 +3688,8 @@ static int s3c_hsotg_suspend(struct platform_device *pdev, pm_message_t state) > clk_disable(hsotg->clk); > } > > + mutex_unlock(&hsotg->init_mutex); > + > return ret; > } > > @@ -3683,7 +3699,9 @@ static int s3c_hsotg_resume(struct platform_device *pdev) > unsigned long flags; > int ret = 0; > > + mutex_lock(&hsotg->init_mutex); > if (hsotg->driver) { > + > dev_info(hsotg->dev, "resuming usb gadget %s\n", > hsotg->driver->driver.name); > > @@ -3699,6 +3717,8 @@ static int s3c_hsotg_resume(struct platform_device *pdev) > s3c_hsotg_core_connect(hsotg); > spin_unlock_irqrestore(&hsotg->lock, flags); > > + mutex_unlock(&hsotg->init_mutex); > + > return ret; > } > Acked-by: Paul Zimmerman <paulz@synopsys.com> -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Oct 31, 2014 at 11:12:33AM +0100, Marek Szyprowski wrote: > This patch adds mutex, which protects initialization and > deinitialization procedures against suspend/resume methods. > > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> doesn't apply either: checking file drivers/usb/dwc2/core.h Hunk #1 FAILED at 187. 1 out of 1 hunk FAILED checking file drivers/usb/dwc2/gadget.c Hunk #2 succeeded at 2863 (offset -46 lines). Hunk #3 succeeded at 2889 (offset -46 lines). Hunk #4 succeeded at 2915 (offset -47 lines). Hunk #5 succeeded at 2934 (offset -47 lines). Hunk #6 succeeded at 2964 (offset -47 lines). Hunk #7 succeeded at 2976 (offset -47 lines). Hunk #8 FAILED at 3518. Hunk #9 succeeded at 3579 (offset -84 lines). Hunk #10 succeeded at 3603 with fuzz 1 (offset -84 lines). Hunk #11 succeeded at 3614 (offset -84 lines). Hunk #12 succeeded at 3632 with fuzz 1 (offset -84 lines). 1 out of 12 hunks FAILED please rebase on testing/next. Also, add Paul's Ack when doing so ;-) thanks
diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h index 9f77b4d1c5ff..58732a9a0019 100644 --- a/drivers/usb/dwc2/core.h +++ b/drivers/usb/dwc2/core.h @@ -187,6 +187,7 @@ struct s3c_hsotg { struct s3c_hsotg_plat *plat; spinlock_t lock; + struct mutex init_mutex; void __iomem *regs; int irq; diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c index d8dda39c9e16..a2e4272a904e 100644 --- a/drivers/usb/dwc2/gadget.c +++ b/drivers/usb/dwc2/gadget.c @@ -21,6 +21,7 @@ #include <linux/platform_device.h> #include <linux/dma-mapping.h> #include <linux/debugfs.h> +#include <linux/mutex.h> #include <linux/seq_file.h> #include <linux/delay.h> #include <linux/io.h> @@ -2908,6 +2909,7 @@ static int s3c_hsotg_udc_start(struct usb_gadget *gadget, return -EINVAL; } + mutex_lock(&hsotg->init_mutex); WARN_ON(hsotg->driver); driver->driver.bus = NULL; @@ -2933,9 +2935,12 @@ static int s3c_hsotg_udc_start(struct usb_gadget *gadget, dev_info(hsotg->dev, "bound driver %s\n", driver->driver.name); + mutex_unlock(&hsotg->init_mutex); + return 0; err: + mutex_unlock(&hsotg->init_mutex); hsotg->driver = NULL; return ret; } @@ -2957,6 +2962,8 @@ static int s3c_hsotg_udc_stop(struct usb_gadget *gadget, if (!hsotg) return -ENODEV; + mutex_lock(&hsotg->init_mutex); + /* all endpoints should be shutdown */ for (ep = 1; ep < hsotg->num_of_eps; ep++) s3c_hsotg_ep_disable(&hsotg->eps[ep].ep); @@ -2974,6 +2981,8 @@ static int s3c_hsotg_udc_stop(struct usb_gadget *gadget, clk_disable(hsotg->clk); + mutex_unlock(&hsotg->init_mutex); + return 0; } @@ -3002,6 +3011,7 @@ static int s3c_hsotg_pullup(struct usb_gadget *gadget, int is_on) dev_dbg(hsotg->dev, "%s: is_on: %d\n", __func__, is_on); + mutex_lock(&hsotg->init_mutex); spin_lock_irqsave(&hsotg->lock, flags); if (is_on) { clk_enable(hsotg->clk); @@ -3013,6 +3023,7 @@ static int s3c_hsotg_pullup(struct usb_gadget *gadget, int is_on) hsotg->gadget.speed = USB_SPEED_UNKNOWN; spin_unlock_irqrestore(&hsotg->lock, flags); + mutex_unlock(&hsotg->init_mutex); return 0; } @@ -3507,6 +3518,7 @@ static int s3c_hsotg_probe(struct platform_device *pdev) } spin_lock_init(&hsotg->lock); + mutex_init(&hsotg->init_mutex); hsotg->irq = ret; @@ -3652,6 +3664,8 @@ static int s3c_hsotg_suspend(struct platform_device *pdev, pm_message_t state) unsigned long flags; int ret = 0; + mutex_lock(&hsotg->init_mutex); + if (hsotg->driver) dev_info(hsotg->dev, "suspending usb gadget %s\n", hsotg->driver->driver.name); @@ -3674,6 +3688,8 @@ static int s3c_hsotg_suspend(struct platform_device *pdev, pm_message_t state) clk_disable(hsotg->clk); } + mutex_unlock(&hsotg->init_mutex); + return ret; } @@ -3683,7 +3699,9 @@ static int s3c_hsotg_resume(struct platform_device *pdev) unsigned long flags; int ret = 0; + mutex_lock(&hsotg->init_mutex); if (hsotg->driver) { + dev_info(hsotg->dev, "resuming usb gadget %s\n", hsotg->driver->driver.name); @@ -3699,6 +3717,8 @@ static int s3c_hsotg_resume(struct platform_device *pdev) s3c_hsotg_core_connect(hsotg); spin_unlock_irqrestore(&hsotg->lock, flags); + mutex_unlock(&hsotg->init_mutex); + return ret; }
This patch adds mutex, which protects initialization and deinitialization procedures against suspend/resume methods. Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> --- drivers/usb/dwc2/core.h | 1 + drivers/usb/dwc2/gadget.c | 20 ++++++++++++++++++++ 2 files changed, 21 insertions(+)