lkml.org 
[lkml]   [2019]   [Apr]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 2/3] usb: gadget: atmel: support USB suspend
From
Date
Ping.  Any feedback on this at all?  It's been over two months without a 
single comment.

Thanks,
Jonas

On 20/02/2019 13:20, Jonas Bonn wrote:
> This patch adds support for USB suspend to the Atmel UDC.
>
> When suspended, the UDC clock can be stopped, resulting in some power
> savings. The "wake up" interrupt will fire irregardless of whether the
> clock is running or not, allowing the UDC clock to be restarted when the
> USB master wants to wake the device again.
>
> The IRQ state of this device is somewhat fiddly. The "wake up" IRQ
> seems to actually be a "bus activity" indicator; the IRQ is almost
> continuously asserted so enabling this IRQ should only be done after a
> suspend when the wake IRQ becomes relevant. Similarly, the "suspend"
> IRQ detects "bus inactivity" and may therefore fire together with a
> "wake" if the two types of activity coincide during the period between
> two IRQ handler invocations; therefore, it's important to ignore the
> "suspend" IRQ while waiting for a wake-up.
>
> This has been tested on a SAMA5D2 board.
>
> Signed-off-by: Jonas Bonn <jonas@norrbonn.se>
> CC: Cristian Birsan <cristian.birsan@microchip.com>
> CC: Felipe Balbi <balbi@kernel.org>
> CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> CC: Nicolas Ferre <nicolas.ferre@microchip.com>
> CC: Alexandre Belloni <alexandre.belloni@bootlin.com>
> CC: Ludovic Desroches <ludovic.desroches@microchip.com>
> CC: linux-arm-kernel@lists.infradead.org
> CC: linux-usb@vger.kernel.org
> ---
> drivers/usb/gadget/udc/atmel_usba_udc.c | 55 ++++++++++++++++++++++---
> drivers/usb/gadget/udc/atmel_usba_udc.h | 1 +
> 2 files changed, 50 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c b/drivers/usb/gadget/udc/atmel_usba_udc.c
> index 9d18fdddd9b2..740cb9308a86 100644
> --- a/drivers/usb/gadget/udc/atmel_usba_udc.c
> +++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
> @@ -1703,6 +1703,9 @@ static void usba_dma_irq(struct usba_udc *udc, struct usba_ep *ep)
> }
> }
>
> +static int start_clock(struct usba_udc *udc);
> +static void stop_clock(struct usba_udc *udc);
> +
> static irqreturn_t usba_udc_irq(int irq, void *devid)
> {
> struct usba_udc *udc = devid;
> @@ -1720,10 +1723,13 @@ static irqreturn_t usba_udc_irq(int irq, void *devid)
> DBG(DBG_INT, "irq, status=%#08x\n", status);
>
> if (status & USBA_DET_SUSPEND) {
> - toggle_bias(udc, 0);
> - usba_writel(udc, INT_CLR, USBA_DET_SUSPEND);
> + usba_writel(udc, INT_CLR, USBA_DET_SUSPEND|USBA_WAKE_UP);
> usba_int_enb_set(udc, USBA_WAKE_UP);
> + usba_int_enb_clear(udc, USBA_DET_SUSPEND);
> + udc->suspended = true;
> + toggle_bias(udc, 0);
> udc->bias_pulse_needed = true;
> + stop_clock(udc);
> DBG(DBG_BUS, "Suspend detected\n");
> if (udc->gadget.speed != USB_SPEED_UNKNOWN
> && udc->driver && udc->driver->suspend) {
> @@ -1734,14 +1740,17 @@ static irqreturn_t usba_udc_irq(int irq, void *devid)
> }
>
> if (status & USBA_WAKE_UP) {
> + start_clock(udc);
> toggle_bias(udc, 1);
> usba_writel(udc, INT_CLR, USBA_WAKE_UP);
> - usba_int_enb_clear(udc, USBA_WAKE_UP);
> DBG(DBG_BUS, "Wake Up CPU detected\n");
> }
>
> if (status & USBA_END_OF_RESUME) {
> + udc->suspended = false;
> usba_writel(udc, INT_CLR, USBA_END_OF_RESUME);
> + usba_int_enb_clear(udc, USBA_WAKE_UP);
> + usba_int_enb_set(udc, USBA_DET_SUSPEND);
> generate_bias_pulse(udc);
> DBG(DBG_BUS, "Resume detected\n");
> if (udc->gadget.speed != USB_SPEED_UNKNOWN
> @@ -1756,6 +1765,8 @@ static irqreturn_t usba_udc_irq(int irq, void *devid)
> if (dma_status) {
> int i;
>
> + usba_int_enb_set(udc, USBA_DET_SUSPEND);
> +
> for (i = 1; i <= USBA_NR_DMAS; i++)
> if (dma_status & (1 << i))
> usba_dma_irq(udc, &udc->usba_ep[i]);
> @@ -1765,6 +1776,8 @@ static irqreturn_t usba_udc_irq(int irq, void *devid)
> if (ep_status) {
> int i;
>
> + usba_int_enb_set(udc, USBA_DET_SUSPEND);
> +
> for (i = 0; i < udc->num_ep; i++)
> if (ep_status & (1 << i)) {
> if (ep_is_control(&udc->usba_ep[i]))
> @@ -1778,7 +1791,9 @@ static irqreturn_t usba_udc_irq(int irq, void *devid)
> struct usba_ep *ep0, *ep;
> int i, n;
>
> - usba_writel(udc, INT_CLR, USBA_END_OF_RESET);
> + usba_writel(udc, INT_CLR,
> + USBA_END_OF_RESET|USBA_END_OF_RESUME
> + |USBA_DET_SUSPEND|USBA_WAKE_UP);
> generate_bias_pulse(udc);
> reset_all_endpoints(udc);
>
> @@ -1805,6 +1820,11 @@ static irqreturn_t usba_udc_irq(int irq, void *devid)
> | USBA_BF(BK_NUMBER, USBA_BK_NUMBER_ONE)));
> usba_ep_writel(ep0, CTL_ENB,
> USBA_EPT_ENABLE | USBA_RX_SETUP);
> +
> + /* If we get reset while suspended... */
> + udc->suspended = false;
> + usba_int_enb_clear(udc, USBA_WAKE_UP);
> +
> usba_int_enb_set(udc, USBA_BF(EPT_INT, 1) |
> USBA_DET_SUSPEND | USBA_END_OF_RESUME);
>
> @@ -1872,9 +1892,19 @@ static int usba_start(struct usba_udc *udc)
> if (ret)
> return ret;
>
> + if (udc->suspended)
> + return 0;
> +
> spin_lock_irqsave(&udc->lock, flags);
> toggle_bias(udc, 1);
> usba_writel(udc, CTRL, USBA_ENABLE_MASK);
> + /* Clear all requested and pending interrupts... */
> + usba_writel(udc, INT_ENB, 0);
> + udc->int_enb_cache = 0;
> + usba_writel(udc, INT_CLR,
> + USBA_END_OF_RESET|USBA_END_OF_RESUME
> + |USBA_DET_SUSPEND|USBA_WAKE_UP);
> + /* ...and enable just 'reset' IRQ to get us started */
> usba_int_enb_set(udc, USBA_END_OF_RESET);
> spin_unlock_irqrestore(&udc->lock, flags);
>
> @@ -1885,6 +1915,9 @@ static void usba_stop(struct usba_udc *udc)
> {
> unsigned long flags;
>
> + if (udc->suspended)
> + return;
> +
> spin_lock_irqsave(&udc->lock, flags);
> udc->gadget.speed = USB_SPEED_UNKNOWN;
> reset_all_endpoints(udc);
> @@ -1912,6 +1945,7 @@ static irqreturn_t usba_vbus_irq_thread(int irq, void *devid)
> if (vbus) {
> usba_start(udc);
> } else {
> + udc->suspended = false;
> usba_stop(udc);
>
> if (udc->driver->disconnect)
> @@ -1975,6 +2009,7 @@ static int atmel_usba_stop(struct usb_gadget *gadget)
> if (fifo_mode == 0)
> udc->configured_ep = 1;
>
> + udc->suspended = false;
> usba_stop(udc);
>
> udc->driver = NULL;
> @@ -2288,6 +2323,7 @@ static int usba_udc_suspend(struct device *dev)
> mutex_lock(&udc->vbus_mutex);
>
> if (!device_may_wakeup(dev)) {
> + udc->suspended = false;
> usba_stop(udc);
> goto out;
> }
> @@ -2297,10 +2333,13 @@ static int usba_udc_suspend(struct device *dev)
> * to request vbus irq, assuming always on.
> */
> if (udc->vbus_pin) {
> + /* FIXME: right to stop here...??? */
> usba_stop(udc);
> enable_irq_wake(gpiod_to_irq(udc->vbus_pin));
> }
>
> + enable_irq_wake(udc->irq);
> +
> out:
> mutex_unlock(&udc->vbus_mutex);
> return 0;
> @@ -2314,8 +2353,12 @@ static int usba_udc_resume(struct device *dev)
> if (!udc->driver)
> return 0;
>
> - if (device_may_wakeup(dev) && udc->vbus_pin)
> - disable_irq_wake(gpiod_to_irq(udc->vbus_pin));
> + if (device_may_wakeup(dev)) {
> + if (udc->vbus_pin)
> + disable_irq_wake(gpiod_to_irq(udc->vbus_pin));
> +
> + disable_irq_wake(udc->irq);
> + }
>
> /* If Vbus is present, enable the controller and wait for reset */
> mutex_lock(&udc->vbus_mutex);
> diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.h b/drivers/usb/gadget/udc/atmel_usba_udc.h
> index 58c96730e32e..24e6fbd3bb99 100644
> --- a/drivers/usb/gadget/udc/atmel_usba_udc.h
> +++ b/drivers/usb/gadget/udc/atmel_usba_udc.h
> @@ -331,6 +331,7 @@ struct usba_udc {
> struct usba_ep *usba_ep;
> bool bias_pulse_needed;
> bool clocked;
> + bool suspended;
>
> u16 devstatus;
>
>

\
 
 \ /
  Last update: 2019-04-27 07:02    [W:0.113 / U:2.012 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site