| Message ID | 1421592681-28317-3-git-send-email-sylvain.rochet@finsecur.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sun, 18 Jan 2015 15:51:21 +0100 Sylvain Rochet <sylvain.rochet@finsecur.com> wrote: > Prepare_enable on rising edge, disable_unprepare on falling edge. Reduce > power consumption if USB PLL is not already necessary for OHCI or EHCI. > If USB host is not connected we can sleep with USB PLL stopped. > > This driver does not support suspend/resume yet, not suspending if we > are still attached to an USB host is fine for what I need, this patch > allow suspending with USB PLL stopped when USB device is not currently > used. > > Signed-off-by: Sylvain Rochet <sylvain.rochet@finsecur.com> > --- > drivers/usb/gadget/udc/atmel_usba_udc.c | 95 ++++++++++++++++++++++++--------- > drivers/usb/gadget/udc/atmel_usba_udc.h | 4 ++ > 2 files changed, 73 insertions(+), 26 deletions(-) > > diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c b/drivers/usb/gadget/udc/atmel_usba_udc.c > index e207d75..986677b 100644 > --- a/drivers/usb/gadget/udc/atmel_usba_udc.c > +++ b/drivers/usb/gadget/udc/atmel_usba_udc.c > @@ -315,6 +315,38 @@ static inline void usba_cleanup_debugfs(struct usba_udc *udc) > } > #endif > > +static int start_clock(struct usba_udc *udc) > +{ > + int ret; > + > + if (udc->clocked) > + return 0; > + > + ret = clk_prepare_enable(udc->pclk); > + if (ret) > + return ret; > + ret = clk_prepare_enable(udc->hclk); > + if (ret) { > + clk_disable_unprepare(udc->pclk); > + return ret; > + } > + > + udc->clocked = true; > + return ret; > +} > + > +static int stop_clock(struct usba_udc *udc) > +{ > + if (!udc->clocked) > + return 0; > + > + clk_disable_unprepare(udc->hclk); > + clk_disable_unprepare(udc->pclk); > + > + udc->clocked = false; > + return 0; > +} > + > static int vbus_is_present(struct usba_udc *udc) > { > if (gpio_is_valid(udc->vbus_pin)) > @@ -1719,42 +1751,55 @@ static irqreturn_t usba_udc_irq(int irq, void *devid) > return IRQ_HANDLED; > } > > -static irqreturn_t usba_vbus_irq(int irq, void *devid) > +static irqreturn_t usba_vbus_irq_thread(int irq, void *devid) > { > struct usba_udc *udc = devid; > int vbus; > + int ret; > > /* debounce */ > udelay(10); > > - spin_lock(&udc->lock); > + mutex_lock(&udc->vbus_mutex); > > /* May happen if Vbus pin toggles during probe() */ > - if (!udc->driver) > + spin_lock(&udc->lock); Since this lock is taken in irq context (usba_udc_irq) and this handler is not called in irq context anymore you should use spin_lock_irqsave/unlock_irqrestore here.
Prepare_enable on rising edge, disable_unprepare on falling edge. Reduce
power consumption if USB PLL is not already necessary for OHCI or EHCI.
If USB host is not connected we can sleep with USB PLL stopped.
This driver does not support suspend/resume yet, not suspending if we
are still attached to an USB host is fine for what I need, this patch
allow suspending with USB PLL stopped when USB device is not currently
used.
Changes since v2:
* Use spin_lock_irqsave/unlock_irqrestore instead of spin_lock/unlock in
threaded interrupt because we are not in irq context anymore
* Removed useless and probably harmful IRQF_NO_SUSPEND from
devm_request_threaded_irq() flags
Changes since v1:
* Using a threaded irq and mutex instead of spinclock as suggested
* Moved a silently fixed bug in a separate patch (1/2)
Sylvain Rochet (2):
USB: gadget: atmel_usba_udc: Fixed vbus_prev initial state
USB: gadget: atmel_usba_udc: Enable/disable USB PLL on Vbus change
drivers/usb/gadget/udc/atmel_usba_udc.c | 98 ++++++++++++++++++++++++---------
drivers/usb/gadget/udc/atmel_usba_udc.h | 4 ++
2 files changed, 76 insertions(+), 26 deletions(-)
On Sun, 18 Jan 2015 15:51:21 +0100 Sylvain Rochet <sylvain.rochet@finsecur.com> wrote: > Prepare_enable on rising edge, disable_unprepare on falling edge. Reduce > power consumption if USB PLL is not already necessary for OHCI or EHCI. > If USB host is not connected we can sleep with USB PLL stopped. > > This driver does not support suspend/resume yet, not suspending if we > are still attached to an USB host is fine for what I need, this patch > allow suspending with USB PLL stopped when USB device is not currently > used. > > Signed-off-by: Sylvain Rochet <sylvain.rochet@finsecur.com> > --- > drivers/usb/gadget/udc/atmel_usba_udc.c | 95 ++++++++++++++++++++++++--------- > drivers/usb/gadget/udc/atmel_usba_udc.h | 4 ++ > 2 files changed, 73 insertions(+), 26 deletions(-) > > diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c b/drivers/usb/gadget/udc/atmel_usba_udc.c > index e207d75..986677b 100644 > --- a/drivers/usb/gadget/udc/atmel_usba_udc.c > +++ b/drivers/usb/gadget/udc/atmel_usba_udc.c > @@ -315,6 +315,38 @@ static inline void usba_cleanup_debugfs(struct usba_udc *udc) > } > #endif > [...] > > if (gpio_is_valid(udc->vbus_pin)) { > if (!devm_gpio_request(&pdev->dev, udc->vbus_pin, "atmel_usba_udc")) { > - ret = devm_request_irq(&pdev->dev, > - gpio_to_irq(udc->vbus_pin), > - usba_vbus_irq, 0, > + ret = devm_request_threaded_irq(&pdev->dev, > + gpio_to_irq(udc->vbus_pin), NULL, > + usba_vbus_irq_thread, IRQF_NO_SUSPEND | IRQF_ONESHOT, You should drop the IRQF_NO_SUSPEND flag. If you want to wakeup on VBUS change you should use enable_irq_wake instead.
diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c b/drivers/usb/gadget/udc/atmel_usba_udc.c index e207d75..986677b 100644 --- a/drivers/usb/gadget/udc/atmel_usba_udc.c +++ b/drivers/usb/gadget/udc/atmel_usba_udc.c @@ -315,6 +315,38 @@ static inline void usba_cleanup_debugfs(struct usba_udc *udc) } #endif +static int start_clock(struct usba_udc *udc) +{ + int ret; + + if (udc->clocked) + return 0; + + ret = clk_prepare_enable(udc->pclk); + if (ret) + return ret; + ret = clk_prepare_enable(udc->hclk); + if (ret) { + clk_disable_unprepare(udc->pclk); + return ret; + } + + udc->clocked = true; + return ret; +} + +static int stop_clock(struct usba_udc *udc) +{ + if (!udc->clocked) + return 0; + + clk_disable_unprepare(udc->hclk); + clk_disable_unprepare(udc->pclk); + + udc->clocked = false; + return 0; +} + static int vbus_is_present(struct usba_udc *udc) { if (gpio_is_valid(udc->vbus_pin)) @@ -1719,42 +1751,55 @@ static irqreturn_t usba_udc_irq(int irq, void *devid) return IRQ_HANDLED; } -static irqreturn_t usba_vbus_irq(int irq, void *devid) +static irqreturn_t usba_vbus_irq_thread(int irq, void *devid) { struct usba_udc *udc = devid; int vbus; + int ret; /* debounce */ udelay(10); - spin_lock(&udc->lock); + mutex_lock(&udc->vbus_mutex); /* May happen if Vbus pin toggles during probe() */ - if (!udc->driver) + spin_lock(&udc->lock); + if (!udc->driver) { + spin_unlock(&udc->lock); goto out; + } + spin_unlock(&udc->lock); vbus = vbus_is_present(udc); if (vbus != udc->vbus_prev) { if (vbus) { + ret = start_clock(udc); + if (ret) + goto out; + + spin_lock(&udc->lock); toggle_bias(1); usba_writel(udc, CTRL, USBA_ENABLE_MASK); usba_writel(udc, INT_ENB, USBA_END_OF_RESET); + spin_unlock(&udc->lock); } else { + spin_lock(&udc->lock); udc->gadget.speed = USB_SPEED_UNKNOWN; reset_all_endpoints(udc); toggle_bias(0); usba_writel(udc, CTRL, USBA_DISABLE_MASK); - if (udc->driver->disconnect) { - spin_unlock(&udc->lock); + spin_unlock(&udc->lock); + + stop_clock(udc); + + if (udc->driver->disconnect) udc->driver->disconnect(&udc->gadget); - spin_lock(&udc->lock); - } } udc->vbus_prev = vbus; } out: - spin_unlock(&udc->lock); + mutex_unlock(&udc->vbus_mutex); return IRQ_HANDLED; } @@ -1762,7 +1807,7 @@ out: static int atmel_usba_start(struct usb_gadget *gadget, struct usb_gadget_driver *driver) { - int ret; + int ret = 0; struct usba_udc *udc = container_of(gadget, struct usba_udc, gadget); unsigned long flags; @@ -1772,31 +1817,29 @@ static int atmel_usba_start(struct usb_gadget *gadget, udc->driver = driver; spin_unlock_irqrestore(&udc->lock, flags); - ret = clk_prepare_enable(udc->pclk); - if (ret) - return ret; - ret = clk_prepare_enable(udc->hclk); - if (ret) { - clk_disable_unprepare(udc->pclk); - return ret; - } - + mutex_lock(&udc->vbus_mutex); udc->vbus_prev = 0; if (gpio_is_valid(udc->vbus_pin)) enable_irq(gpio_to_irq(udc->vbus_pin)); /* If Vbus is present, enable the controller and wait for reset */ - spin_lock_irqsave(&udc->lock, flags); if (vbus_is_present(udc) && udc->vbus_prev == 0) { + ret = start_clock(udc); + if (ret) + goto out; + + spin_lock_irqsave(&udc->lock, flags); toggle_bias(1); usba_writel(udc, CTRL, USBA_ENABLE_MASK); usba_writel(udc, INT_ENB, USBA_END_OF_RESET); + spin_unlock_irqrestore(&udc->lock, flags); udc->vbus_prev = 1; } - spin_unlock_irqrestore(&udc->lock, flags); - return 0; +out: + mutex_unlock(&udc->vbus_mutex); + return ret; } static int atmel_usba_stop(struct usb_gadget *gadget) @@ -1816,8 +1859,7 @@ static int atmel_usba_stop(struct usb_gadget *gadget) toggle_bias(0); usba_writel(udc, CTRL, USBA_DISABLE_MASK); - clk_disable_unprepare(udc->hclk); - clk_disable_unprepare(udc->pclk); + stop_clock(udc); udc->driver = NULL; @@ -1997,6 +2039,7 @@ static int usba_udc_probe(struct platform_device *pdev) return PTR_ERR(hclk); spin_lock_init(&udc->lock); + mutex_init(&udc->vbus_mutex); udc->pdev = pdev; udc->pclk = pclk; udc->hclk = hclk; @@ -2049,9 +2092,9 @@ static int usba_udc_probe(struct platform_device *pdev) if (gpio_is_valid(udc->vbus_pin)) { if (!devm_gpio_request(&pdev->dev, udc->vbus_pin, "atmel_usba_udc")) { - ret = devm_request_irq(&pdev->dev, - gpio_to_irq(udc->vbus_pin), - usba_vbus_irq, 0, + ret = devm_request_threaded_irq(&pdev->dev, + gpio_to_irq(udc->vbus_pin), NULL, + usba_vbus_irq_thread, IRQF_NO_SUSPEND | IRQF_ONESHOT, "atmel_usba_udc", udc); if (ret) { udc->vbus_pin = -ENODEV; diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.h b/drivers/usb/gadget/udc/atmel_usba_udc.h index a70706e..3ceed76 100644 --- a/drivers/usb/gadget/udc/atmel_usba_udc.h +++ b/drivers/usb/gadget/udc/atmel_usba_udc.h @@ -308,6 +308,9 @@ struct usba_udc { /* Protect hw registers from concurrent modifications */ spinlock_t lock; + /* Mutex to prevent concurrent start or stop */ + struct mutex vbus_mutex; + void __iomem *regs; void __iomem *fifo; @@ -321,6 +324,7 @@ struct usba_udc { struct clk *pclk; struct clk *hclk; struct usba_ep *usba_ep; + bool clocked; u16 devstatus;
Prepare_enable on rising edge, disable_unprepare on falling edge. Reduce power consumption if USB PLL is not already necessary for OHCI or EHCI. If USB host is not connected we can sleep with USB PLL stopped. This driver does not support suspend/resume yet, not suspending if we are still attached to an USB host is fine for what I need, this patch allow suspending with USB PLL stopped when USB device is not currently used. Signed-off-by: Sylvain Rochet <sylvain.rochet@finsecur.com> --- drivers/usb/gadget/udc/atmel_usba_udc.c | 95 ++++++++++++++++++++++++--------- drivers/usb/gadget/udc/atmel_usba_udc.h | 4 ++ 2 files changed, 73 insertions(+), 26 deletions(-)