[PATCH v2] usb: gadget: at91_udc: move prepare clk into process context

[PATCH v2] usb: gadget: at91_udc: move prepare clk into process context

[Ronald Wahl]

Commit 7628083227b6bc4a7e33d7c381d7a4e558424b6b added clock preparation in
interrupt context. This is not allowed as it might sleep. Move clock
preparation and setting clock rate into process context (at91udc_probe).

Signed-off-by: Ronald Wahl <ronald.wahl@raritan.com>
---
 drivers/usb/gadget/udc/at91_udc.c | 44 ++++++++++++++++++++++++++++-----------
 1 file changed, 32 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/gadget/udc/at91_udc.c b/drivers/usb/gadget/udc/at91_udc.c
index cfd18bc..0d685d0 100644
--- a/drivers/usb/gadget/udc/at91_udc.c
+++ b/drivers/usb/gadget/udc/at91_udc.c
@@ -870,12 +870,10 @@ static void clk_on(struct at91_udc *udc)
 		return;
 	udc->clocked = 1;
 
-	if (IS_ENABLED(CONFIG_COMMON_CLK)) {
-		clk_set_rate(udc->uclk, 48000000);
-		clk_prepare_enable(udc->uclk);
-	}
-	clk_prepare_enable(udc->iclk);
-	clk_prepare_enable(udc->fclk);
+	if (IS_ENABLED(CONFIG_COMMON_CLK))
+		clk_enable(udc->uclk);
+	clk_enable(udc->iclk);
+	clk_enable(udc->fclk);
 }
 
 static void clk_off(struct at91_udc *udc)
@@ -884,10 +882,10 @@ static void clk_off(struct at91_udc *udc)
 		return;
 	udc->clocked = 0;
 	udc->gadget.speed = USB_SPEED_UNKNOWN;
-	clk_disable_unprepare(udc->fclk);
-	clk_disable_unprepare(udc->iclk);
+	clk_disable(udc->fclk);
+	clk_disable(udc->iclk);
 	if (IS_ENABLED(CONFIG_COMMON_CLK))
-		clk_disable_unprepare(udc->uclk);
+		clk_disable(udc->uclk);
 }
 
 /*
@@ -1780,14 +1778,24 @@ static int at91udc_probe(struct platform_device *pdev)
 	}
 
 	/* don't do anything until we have both gadget driver and VBUS */
+	if (IS_ENABLED(CONFIG_COMMON_CLK)) {
+		clk_set_rate(udc->uclk, 48000000);
+		retval = clk_prepare(udc->uclk);
+		if (retval)
+			goto fail1;
+	}
+	retval = clk_prepare(udc->fclk);
+	if (retval)
+		goto fail1a;
+
 	retval = clk_prepare_enable(udc->iclk);
 	if (retval)
-		goto fail1;
+		goto fail1b;
 	at91_udp_write(udc, AT91_UDP_TXVC, AT91_UDP_TXVC_TXVDIS);
 	at91_udp_write(udc, AT91_UDP_IDR, 0xffffffff);
 	/* Clear all pending interrupts - UDP may be used by bootloader. */
 	at91_udp_write(udc, AT91_UDP_ICR, 0xffffffff);
-	clk_disable_unprepare(udc->iclk);
+	clk_disable(udc->iclk);
 
 	/* request UDC and maybe VBUS irqs */
 	udc->udp_irq = platform_get_irq(pdev, 0);
@@ -1795,7 +1803,7 @@ static int at91udc_probe(struct platform_device *pdev)
 			0, driver_name, udc);
 	if (retval < 0) {
 		DBG("request irq %d failed\n", udc->udp_irq);
-		goto fail1;
+		goto fail1c;
 	}
 	if (gpio_is_valid(udc->board.vbus_pin)) {
 		retval = gpio_request(udc->board.vbus_pin, "udc_vbus");
@@ -1848,6 +1856,13 @@ fail3:
 		gpio_free(udc->board.vbus_pin);
 fail2:
 	free_irq(udc->udp_irq, udc);
+fail1c:
+	clk_unprepare(udc->iclk);
+fail1b:
+	clk_unprepare(udc->fclk);
+fail1a:
+	if (IS_ENABLED(CONFIG_COMMON_CLK))
+		clk_unprepare(udc->uclk);
 fail1:
 	if (IS_ENABLED(CONFIG_COMMON_CLK) && !IS_ERR(udc->uclk))
 		clk_put(udc->uclk);
@@ -1896,6 +1911,11 @@ static int __exit at91udc_remove(struct platform_device *pdev)
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	release_mem_region(res->start, resource_size(res));
 
+	if (IS_ENABLED(CONFIG_COMMON_CLK))
+		clk_unprepare(udc->uclk);
+	clk_unprepare(udc->fclk);
+	clk_unprepare(udc->iclk);
+
 	clk_put(udc->iclk);
 	clk_put(udc->fclk);
 	if (IS_ENABLED(CONFIG_COMMON_CLK))
-- 
1.9.3

[PATCH v2] usb: gadget: at91_udc: move prepare clk into process context

[Ronald Wahl]

Hi,

actually this should have been patch v3 and not v2. Anyway, the major 
difference is that I moved setting the clock rate into process context 
as well as it may also sleep.

- ron

On 07.08.2014 17:11, Ronald Wahl wrote:
> Commit 7628083227b6bc4a7e33d7c381d7a4e558424b6b added clock preparation in
> interrupt context. This is not allowed as it might sleep. Move clock
> preparation and setting clock rate into process context (at91udc_probe).
>
> Signed-off-by: Ronald Wahl <ronald.wahl@raritan.com>
> ---
>   drivers/usb/gadget/udc/at91_udc.c | 44 ++++++++++++++++++++++++++++-----------
>   1 file changed, 32 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/usb/gadget/udc/at91_udc.c b/drivers/usb/gadget/udc/at91_udc.c
> index cfd18bc..0d685d0 100644
> --- a/drivers/usb/gadget/udc/at91_udc.c
> +++ b/drivers/usb/gadget/udc/at91_udc.c
> @@ -870,12 +870,10 @@ static void clk_on(struct at91_udc *udc)
>   		return;
>   	udc->clocked = 1;
>
> -	if (IS_ENABLED(CONFIG_COMMON_CLK)) {
> -		clk_set_rate(udc->uclk, 48000000);
> -		clk_prepare_enable(udc->uclk);
> -	}
> -	clk_prepare_enable(udc->iclk);
> -	clk_prepare_enable(udc->fclk);
> +	if (IS_ENABLED(CONFIG_COMMON_CLK))
> +		clk_enable(udc->uclk);
> +	clk_enable(udc->iclk);
> +	clk_enable(udc->fclk);
>   }
>
>   static void clk_off(struct at91_udc *udc)
> @@ -884,10 +882,10 @@ static void clk_off(struct at91_udc *udc)
>   		return;
>   	udc->clocked = 0;
>   	udc->gadget.speed = USB_SPEED_UNKNOWN;
> -	clk_disable_unprepare(udc->fclk);
> -	clk_disable_unprepare(udc->iclk);
> +	clk_disable(udc->fclk);
> +	clk_disable(udc->iclk);
>   	if (IS_ENABLED(CONFIG_COMMON_CLK))
> -		clk_disable_unprepare(udc->uclk);
> +		clk_disable(udc->uclk);
>   }
>
>   /*
> @@ -1780,14 +1778,24 @@ static int at91udc_probe(struct platform_device *pdev)
>   	}
>
>   	/* don't do anything until we have both gadget driver and VBUS */
> +	if (IS_ENABLED(CONFIG_COMMON_CLK)) {
> +		clk_set_rate(udc->uclk, 48000000);
> +		retval = clk_prepare(udc->uclk);
> +		if (retval)
> +			goto fail1;
> +	}
> +	retval = clk_prepare(udc->fclk);
> +	if (retval)
> +		goto fail1a;
> +
>   	retval = clk_prepare_enable(udc->iclk);
>   	if (retval)
> -		goto fail1;
> +		goto fail1b;
>   	at91_udp_write(udc, AT91_UDP_TXVC, AT91_UDP_TXVC_TXVDIS);
>   	at91_udp_write(udc, AT91_UDP_IDR, 0xffffffff);
>   	/* Clear all pending interrupts - UDP may be used by bootloader. */
>   	at91_udp_write(udc, AT91_UDP_ICR, 0xffffffff);
> -	clk_disable_unprepare(udc->iclk);
> +	clk_disable(udc->iclk);
>
>   	/* request UDC and maybe VBUS irqs */
>   	udc->udp_irq = platform_get_irq(pdev, 0);
> @@ -1795,7 +1803,7 @@ static int at91udc_probe(struct platform_device *pdev)
>   			0, driver_name, udc);
>   	if (retval < 0) {
>   		DBG("request irq %d failed\n", udc->udp_irq);
> -		goto fail1;
> +		goto fail1c;
>   	}
>   	if (gpio_is_valid(udc->board.vbus_pin)) {
>   		retval = gpio_request(udc->board.vbus_pin, "udc_vbus");
> @@ -1848,6 +1856,13 @@ fail3:
>   		gpio_free(udc->board.vbus_pin);
>   fail2:
>   	free_irq(udc->udp_irq, udc);
> +fail1c:
> +	clk_unprepare(udc->iclk);
> +fail1b:
> +	clk_unprepare(udc->fclk);
> +fail1a:
> +	if (IS_ENABLED(CONFIG_COMMON_CLK))
> +		clk_unprepare(udc->uclk);
>   fail1:
>   	if (IS_ENABLED(CONFIG_COMMON_CLK) && !IS_ERR(udc->uclk))
>   		clk_put(udc->uclk);
> @@ -1896,6 +1911,11 @@ static int __exit at91udc_remove(struct platform_device *pdev)
>   	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>   	release_mem_region(res->start, resource_size(res));
>
> +	if (IS_ENABLED(CONFIG_COMMON_CLK))
> +		clk_unprepare(udc->uclk);
> +	clk_unprepare(udc->fclk);
> +	clk_unprepare(udc->iclk);
> +
>   	clk_put(udc->iclk);
>   	clk_put(udc->fclk);
>   	if (IS_ENABLED(CONFIG_COMMON_CLK))
>

[PATCH v2] usb: gadget: at91_udc: move prepare clk into process context

[Alexandre Belloni]

Hi,

For the next revision, please include the changelog after the --- marker
and before the diffstat. Else, this looks good to me, with the same
comment as Boris on the PLLB gating. We will take care of that later.

Regards

On 07/08/2014 at 17:23:58 +0200, Ronald Wahl wrote :
> Hi,
> 
> actually this should have been patch v3 and not v2. Anyway, the
> major difference is that I moved setting the clock rate into process
> context as well as it may also sleep.
> 
> - ron
> 
> On 07.08.2014 17:11, Ronald Wahl wrote:
> >Commit 7628083227b6bc4a7e33d7c381d7a4e558424b6b added clock preparation in
> >interrupt context. This is not allowed as it might sleep. Move clock
> >preparation and setting clock rate into process context (at91udc_probe).
> >
> >Signed-off-by: Ronald Wahl <ronald.wahl@raritan.com>
> >---
> >  drivers/usb/gadget/udc/at91_udc.c | 44 ++++++++++++++++++++++++++++-----------
> >  1 file changed, 32 insertions(+), 12 deletions(-)
> >
> >diff --git a/drivers/usb/gadget/udc/at91_udc.c b/drivers/usb/gadget/udc/at91_udc.c
> >index cfd18bc..0d685d0 100644
> >--- a/drivers/usb/gadget/udc/at91_udc.c
> >+++ b/drivers/usb/gadget/udc/at91_udc.c
> >@@ -870,12 +870,10 @@ static void clk_on(struct at91_udc *udc)
> >  		return;
> >  	udc->clocked = 1;
> >
> >-	if (IS_ENABLED(CONFIG_COMMON_CLK)) {
> >-		clk_set_rate(udc->uclk, 48000000);
> >-		clk_prepare_enable(udc->uclk);
> >-	}
> >-	clk_prepare_enable(udc->iclk);
> >-	clk_prepare_enable(udc->fclk);
> >+	if (IS_ENABLED(CONFIG_COMMON_CLK))
> >+		clk_enable(udc->uclk);
> >+	clk_enable(udc->iclk);
> >+	clk_enable(udc->fclk);
> >  }
> >
> >  static void clk_off(struct at91_udc *udc)
> >@@ -884,10 +882,10 @@ static void clk_off(struct at91_udc *udc)
> >  		return;
> >  	udc->clocked = 0;
> >  	udc->gadget.speed = USB_SPEED_UNKNOWN;
> >-	clk_disable_unprepare(udc->fclk);
> >-	clk_disable_unprepare(udc->iclk);
> >+	clk_disable(udc->fclk);
> >+	clk_disable(udc->iclk);
> >  	if (IS_ENABLED(CONFIG_COMMON_CLK))
> >-		clk_disable_unprepare(udc->uclk);
> >+		clk_disable(udc->uclk);
> >  }
> >
> >  /*
> >@@ -1780,14 +1778,24 @@ static int at91udc_probe(struct platform_device *pdev)
> >  	}
> >
> >  	/* don't do anything until we have both gadget driver and VBUS */
> >+	if (IS_ENABLED(CONFIG_COMMON_CLK)) {
> >+		clk_set_rate(udc->uclk, 48000000);
> >+		retval = clk_prepare(udc->uclk);
> >+		if (retval)
> >+			goto fail1;
> >+	}
> >+	retval = clk_prepare(udc->fclk);
> >+	if (retval)
> >+		goto fail1a;
> >+
> >  	retval = clk_prepare_enable(udc->iclk);
> >  	if (retval)
> >-		goto fail1;
> >+		goto fail1b;
> >  	at91_udp_write(udc, AT91_UDP_TXVC, AT91_UDP_TXVC_TXVDIS);
> >  	at91_udp_write(udc, AT91_UDP_IDR, 0xffffffff);
> >  	/* Clear all pending interrupts - UDP may be used by bootloader. */
> >  	at91_udp_write(udc, AT91_UDP_ICR, 0xffffffff);
> >-	clk_disable_unprepare(udc->iclk);
> >+	clk_disable(udc->iclk);
> >
> >  	/* request UDC and maybe VBUS irqs */
> >  	udc->udp_irq = platform_get_irq(pdev, 0);
> >@@ -1795,7 +1803,7 @@ static int at91udc_probe(struct platform_device *pdev)
> >  			0, driver_name, udc);
> >  	if (retval < 0) {
> >  		DBG("request irq %d failed\n", udc->udp_irq);
> >-		goto fail1;
> >+		goto fail1c;
> >  	}
> >  	if (gpio_is_valid(udc->board.vbus_pin)) {
> >  		retval = gpio_request(udc->board.vbus_pin, "udc_vbus");
> >@@ -1848,6 +1856,13 @@ fail3:
> >  		gpio_free(udc->board.vbus_pin);
> >  fail2:
> >  	free_irq(udc->udp_irq, udc);
> >+fail1c:
> >+	clk_unprepare(udc->iclk);
> >+fail1b:
> >+	clk_unprepare(udc->fclk);
> >+fail1a:
> >+	if (IS_ENABLED(CONFIG_COMMON_CLK))
> >+		clk_unprepare(udc->uclk);
> >  fail1:
> >  	if (IS_ENABLED(CONFIG_COMMON_CLK) && !IS_ERR(udc->uclk))
> >  		clk_put(udc->uclk);
> >@@ -1896,6 +1911,11 @@ static int __exit at91udc_remove(struct platform_device *pdev)
> >  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >  	release_mem_region(res->start, resource_size(res));
> >
> >+	if (IS_ENABLED(CONFIG_COMMON_CLK))
> >+		clk_unprepare(udc->uclk);
> >+	clk_unprepare(udc->fclk);
> >+	clk_unprepare(udc->iclk);
> >+
> >  	clk_put(udc->iclk);
> >  	clk_put(udc->fclk);
> >  	if (IS_ENABLED(CONFIG_COMMON_CLK))
> >

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[PATCH v2] usb: gadget: at91_udc: move prepare clk into process context

[Sergei Shtylyov]

Hello.

On 08/07/2014 07:11 PM, Ronald Wahl wrote:

> Commit 7628083227b6bc4a7e33d7c381d7a4e558424b6b added clock preparation in

    Please also specify that commit's summary in parens.

> interrupt context. This is not allowed as it might sleep. Move clock
> preparation and setting clock rate into process context (at91udc_probe).

> Signed-off-by: Ronald Wahl <ronald.wahl@raritan.com>

WBR, Sergei

[PATCH v2] usb: gadget: at91_udc: move prepare clk into process context

[Ronald Wahl]

On 2014-08-06 13:11, Ronald Wahl wrote:
> Commit 7628083227b6bc4a7e33d7c381d7a4e558424b6b added clock preparation in
> interrupt context. This is not allowed as it might sleep. Move clock
> preparation into process context (at91udc_probe).

Add Boris and Felipe to Cc: ...

- ron

[PATCH v2] usb: gadget: at91_udc: move prepare clk into process context

[Ronald Wahl]

Commit 7628083227b6bc4a7e33d7c381d7a4e558424b6b added clock preparation in
interrupt context. This is not allowed as it might sleep. Move clock
preparation into process context (at91udc_probe).
---
 drivers/usb/gadget/udc/at91_udc.c | 39 ++++++++++++++++++++++++++++++---------
 1 file changed, 30 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/gadget/udc/at91_udc.c b/drivers/usb/gadget/udc/at91_udc.c
index cfd18bc..0b347a0 100644
--- a/drivers/usb/gadget/udc/at91_udc.c
+++ b/drivers/usb/gadget/udc/at91_udc.c
@@ -872,10 +872,10 @@ static void clk_on(struct at91_udc *udc)
 
 	if (IS_ENABLED(CONFIG_COMMON_CLK)) {
 		clk_set_rate(udc->uclk, 48000000);
-		clk_prepare_enable(udc->uclk);
+		clk_enable(udc->uclk);
 	}
-	clk_prepare_enable(udc->iclk);
-	clk_prepare_enable(udc->fclk);
+	clk_enable(udc->iclk);
+	clk_enable(udc->fclk);
 }
 
 static void clk_off(struct at91_udc *udc)
@@ -884,10 +884,10 @@ static void clk_off(struct at91_udc *udc)
 		return;
 	udc->clocked = 0;
 	udc->gadget.speed = USB_SPEED_UNKNOWN;
-	clk_disable_unprepare(udc->fclk);
-	clk_disable_unprepare(udc->iclk);
+	clk_disable(udc->fclk);
+	clk_disable(udc->iclk);
 	if (IS_ENABLED(CONFIG_COMMON_CLK))
-		clk_disable_unprepare(udc->uclk);
+		clk_disable(udc->uclk);
 }
 
 /*
@@ -1780,14 +1780,23 @@ static int at91udc_probe(struct platform_device *pdev)
 	}
 
 	/* don't do anything until we have both gadget driver and VBUS */
+	if (IS_ENABLED(CONFIG_COMMON_CLK)) {
+		retval = clk_prepare(udc->uclk);
+		if (retval)
+			goto fail1;
+	}
+	retval = clk_prepare(udc->fclk);
+	if (retval)
+		goto fail1a;
+
 	retval = clk_prepare_enable(udc->iclk);
 	if (retval)
-		goto fail1;
+		goto fail1b;
 	at91_udp_write(udc, AT91_UDP_TXVC, AT91_UDP_TXVC_TXVDIS);
 	at91_udp_write(udc, AT91_UDP_IDR, 0xffffffff);
 	/* Clear all pending interrupts - UDP may be used by bootloader. */
 	at91_udp_write(udc, AT91_UDP_ICR, 0xffffffff);
-	clk_disable_unprepare(udc->iclk);
+	clk_disable(udc->iclk);
 
 	/* request UDC and maybe VBUS irqs */
 	udc->udp_irq = platform_get_irq(pdev, 0);
@@ -1795,7 +1804,7 @@ static int at91udc_probe(struct platform_device *pdev)
 			0, driver_name, udc);
 	if (retval < 0) {
 		DBG("request irq %d failed\n", udc->udp_irq);
-		goto fail1;
+		goto fail1c;
 	}
 	if (gpio_is_valid(udc->board.vbus_pin)) {
 		retval = gpio_request(udc->board.vbus_pin, "udc_vbus");
@@ -1848,6 +1857,13 @@ fail3:
 		gpio_free(udc->board.vbus_pin);
 fail2:
 	free_irq(udc->udp_irq, udc);
+fail1c:
+	clk_unprepare(udc->iclk);
+fail1b:
+	clk_unprepare(udc->fclk);
+fail1a:
+	if (IS_ENABLED(CONFIG_COMMON_CLK))
+		clk_unprepare(udc->uclk);
 fail1:
 	if (IS_ENABLED(CONFIG_COMMON_CLK) && !IS_ERR(udc->uclk))
 		clk_put(udc->uclk);
@@ -1896,6 +1912,11 @@ static int __exit at91udc_remove(struct platform_device *pdev)
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	release_mem_region(res->start, resource_size(res));
 
+	if (IS_ENABLED(CONFIG_COMMON_CLK))
+		clk_unprepare(udc->uclk);
+	clk_unprepare(udc->fclk);
+	clk_unprepare(udc->iclk);
+
 	clk_put(udc->iclk);
 	clk_put(udc->fclk);
 	if (IS_ENABLED(CONFIG_COMMON_CLK))
-- 
1.9.3

[PATCH v2] usb: gadget: at91_udc: move prepare clk into process context

[Boris BREZILLON]

Hi Ronald,

On Wed,  6 Aug 2014 15:11:42 +0200
Ronald Wahl <ronald.wahl@raritan.com> wrote:

> Commit 7628083227b6bc4a7e33d7c381d7a4e558424b6b added clock preparation in
> interrupt context. This is not allowed as it might sleep. Move clock
> preparation into process context (at91udc_probe).
> ---
>  drivers/usb/gadget/udc/at91_udc.c | 39 ++++++++++++++++++++++++++++++---------
>  1 file changed, 30 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/usb/gadget/udc/at91_udc.c b/drivers/usb/gadget/udc/at91_udc.c
> index cfd18bc..0b347a0 100644
> --- a/drivers/usb/gadget/udc/at91_udc.c
> +++ b/drivers/usb/gadget/udc/at91_udc.c
> @@ -872,10 +872,10 @@ static void clk_on(struct at91_udc *udc)
>  
>  	if (IS_ENABLED(CONFIG_COMMON_CLK)) {
>  		clk_set_rate(udc->uclk, 48000000);
> -		clk_prepare_enable(udc->uclk);
> +		clk_enable(udc->uclk);
>  	}
> -	clk_prepare_enable(udc->iclk);
> -	clk_prepare_enable(udc->fclk);
> +	clk_enable(udc->iclk);
> +	clk_enable(udc->fclk);
>  }
>  
>  static void clk_off(struct at91_udc *udc)
> @@ -884,10 +884,10 @@ static void clk_off(struct at91_udc *udc)
>  		return;
>  	udc->clocked = 0;
>  	udc->gadget.speed = USB_SPEED_UNKNOWN;
> -	clk_disable_unprepare(udc->fclk);
> -	clk_disable_unprepare(udc->iclk);
> +	clk_disable(udc->fclk);
> +	clk_disable(udc->iclk);
>  	if (IS_ENABLED(CONFIG_COMMON_CLK))
> -		clk_disable_unprepare(udc->uclk);
> +		clk_disable(udc->uclk);
>  }

As you stated prepare and unprepare should never be called in interrupt
context. 

My concern here is that PLLB (which is often used as USB clock
parent) will never be gated/disabled (because all this work is
done in its unprepare method), and thus your power consumption will be
higher (when entering suspend mode) than if you'd done a
disable_unprepare call.

How about leaving the clk_on/off unchanged and use a threaded irq
instead of a normal irq ?


Best Regards,

Boris

-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[PATCH v2] usb: gadget: at91_udc: move prepare clk into process context

[Boris BREZILLON]

Adding USB and Atmel Maintainers in Cc.

On Thu, 7 Aug 2014 09:52:31 +0200
Boris BREZILLON <boris.brezillon@free-electrons.com> wrote:

> Hi Ronald,
> 
> On Wed,  6 Aug 2014 15:11:42 +0200
> Ronald Wahl <ronald.wahl@raritan.com> wrote:
> 
> > Commit 7628083227b6bc4a7e33d7c381d7a4e558424b6b added clock preparation in
> > interrupt context. This is not allowed as it might sleep. Move clock
> > preparation into process context (at91udc_probe).
> > ---
> >  drivers/usb/gadget/udc/at91_udc.c | 39 ++++++++++++++++++++++++++++++---------
> >  1 file changed, 30 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/usb/gadget/udc/at91_udc.c b/drivers/usb/gadget/udc/at91_udc.c
> > index cfd18bc..0b347a0 100644
> > --- a/drivers/usb/gadget/udc/at91_udc.c
> > +++ b/drivers/usb/gadget/udc/at91_udc.c
> > @@ -872,10 +872,10 @@ static void clk_on(struct at91_udc *udc)
> >  
> >  	if (IS_ENABLED(CONFIG_COMMON_CLK)) {
> >  		clk_set_rate(udc->uclk, 48000000);
> > -		clk_prepare_enable(udc->uclk);
> > +		clk_enable(udc->uclk);
> >  	}
> > -	clk_prepare_enable(udc->iclk);
> > -	clk_prepare_enable(udc->fclk);
> > +	clk_enable(udc->iclk);
> > +	clk_enable(udc->fclk);
> >  }
> >  
> >  static void clk_off(struct at91_udc *udc)
> > @@ -884,10 +884,10 @@ static void clk_off(struct at91_udc *udc)
> >  		return;
> >  	udc->clocked = 0;
> >  	udc->gadget.speed = USB_SPEED_UNKNOWN;
> > -	clk_disable_unprepare(udc->fclk);
> > -	clk_disable_unprepare(udc->iclk);
> > +	clk_disable(udc->fclk);
> > +	clk_disable(udc->iclk);
> >  	if (IS_ENABLED(CONFIG_COMMON_CLK))
> > -		clk_disable_unprepare(udc->uclk);
> > +		clk_disable(udc->uclk);
> >  }
> 
> As you stated prepare and unprepare should never be called in interrupt
> context. 
> 
> My concern here is that PLLB (which is often used as USB clock
> parent) will never be gated/disabled (because all this work is
> done in its unprepare method), and thus your power consumption will be
> higher (when entering suspend mode) than if you'd done a
> disable_unprepare call.
> 
> How about leaving the clk_on/off unchanged and use a threaded irq
> instead of a normal irq ?
> 
> 
> Best Regards,
> 
> Boris
> 



-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[PATCH v2] usb: gadget: at91_udc: move prepare clk into process context

[Ronald Wahl]

On 07.08.2014 09:59, Boris BREZILLON wrote:
> On Thu, 7 Aug 2014 09:52:31 +0200
> Boris BREZILLON <boris.brezillon@free-electrons.com> wrote:
>> On Wed,  6 Aug 2014 15:11:42 +0200
>> Ronald Wahl <ronald.wahl@raritan.com> wrote:
>>
>>> Commit 7628083227b6bc4a7e33d7c381d7a4e558424b6b added clock preparation in
>>> interrupt context. This is not allowed as it might sleep. Move clock
>>> preparation into process context (at91udc_probe).
>>> ---
>>>   drivers/usb/gadget/udc/at91_udc.c | 39 ++++++++++++++++++++++++++++++---------
>>>   1 file changed, 30 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/drivers/usb/gadget/udc/at91_udc.c b/drivers/usb/gadget/udc/at91_udc.c
>>> index cfd18bc..0b347a0 100644
>>> --- a/drivers/usb/gadget/udc/at91_udc.c
>>> +++ b/drivers/usb/gadget/udc/at91_udc.c
>>> @@ -872,10 +872,10 @@ static void clk_on(struct at91_udc *udc)
>>>
>>>   	if (IS_ENABLED(CONFIG_COMMON_CLK)) {
>>>   		clk_set_rate(udc->uclk, 48000000);
>>> -		clk_prepare_enable(udc->uclk);
>>> +		clk_enable(udc->uclk);
>>>   	}
>>> -	clk_prepare_enable(udc->iclk);
>>> -	clk_prepare_enable(udc->fclk);
>>> +	clk_enable(udc->iclk);
>>> +	clk_enable(udc->fclk);
>>>   }
>>>
>>>   static void clk_off(struct at91_udc *udc)
>>> @@ -884,10 +884,10 @@ static void clk_off(struct at91_udc *udc)
>>>   		return;
>>>   	udc->clocked = 0;
>>>   	udc->gadget.speed = USB_SPEED_UNKNOWN;
>>> -	clk_disable_unprepare(udc->fclk);
>>> -	clk_disable_unprepare(udc->iclk);
>>> +	clk_disable(udc->fclk);
>>> +	clk_disable(udc->iclk);
>>>   	if (IS_ENABLED(CONFIG_COMMON_CLK))
>>> -		clk_disable_unprepare(udc->uclk);
>>> +		clk_disable(udc->uclk);
>>>   }
>>
>> As you stated prepare and unprepare should never be called in interrupt
>> context.
>>
>> My concern here is that PLLB (which is often used as USB clock
>> parent) will never be gated/disabled (because all this work is
>> done in its unprepare method), and thus your power consumption will be
>> higher (when entering suspend mode) than if you'd done a
>> disable_unprepare call.
>>
>> How about leaving the clk_on/off unchanged and use a threaded irq
>> instead of a normal irq ?

Even with threaded interrupts things are still called while interrupts 
are disabled by one or more layers of spin_lock_irqsave. There are also 
different code paths from where clk_on() is called. So getting this 
right while saving power is probably not trivial and beyond my 
knowledge/experience with the udc code. It would be nice if someone with 
a deeper knowledge of the atmel udc code can get this right. My primary 
intention was to get the code working again.

thx & greets,
ron

-- 
Ronald Wahl - ronald.wahl at raritan.com - Phone +49 375271349-0 Fax -99
Raritan Deutschland GmbH, Kornmarkt 7, 08056 Zwickau, Germany
USt-IdNr. DE813094160, Steuer-Nr. 227/117/01749
Amtsgericht Chemnitz HRB 23605
Gesch?ftsf?hrung: Stuart Hopper, Ralf Ploenes

[PATCH v2] usb: gadget: at91_udc: move prepare clk into process context

[Boris BREZILLON]

On Thu, 07 Aug 2014 14:43:32 +0200
Ronald Wahl <ronald.wahl@raritan.com> wrote:

> On 07.08.2014 09:59, Boris BREZILLON wrote:
> > On Thu, 7 Aug 2014 09:52:31 +0200
> > Boris BREZILLON <boris.brezillon@free-electrons.com> wrote:
> >> On Wed,  6 Aug 2014 15:11:42 +0200
> >> Ronald Wahl <ronald.wahl@raritan.com> wrote:
> >>
> >>> Commit 7628083227b6bc4a7e33d7c381d7a4e558424b6b added clock preparation in
> >>> interrupt context. This is not allowed as it might sleep. Move clock
> >>> preparation into process context (at91udc_probe).
> >>> ---
> >>>   drivers/usb/gadget/udc/at91_udc.c | 39 ++++++++++++++++++++++++++++++---------
> >>>   1 file changed, 30 insertions(+), 9 deletions(-)
> >>>
> >>> diff --git a/drivers/usb/gadget/udc/at91_udc.c b/drivers/usb/gadget/udc/at91_udc.c
> >>> index cfd18bc..0b347a0 100644
> >>> --- a/drivers/usb/gadget/udc/at91_udc.c
> >>> +++ b/drivers/usb/gadget/udc/at91_udc.c
> >>> @@ -872,10 +872,10 @@ static void clk_on(struct at91_udc *udc)
> >>>
> >>>   	if (IS_ENABLED(CONFIG_COMMON_CLK)) {
> >>>   		clk_set_rate(udc->uclk, 48000000);
> >>> -		clk_prepare_enable(udc->uclk);
> >>> +		clk_enable(udc->uclk);
> >>>   	}
> >>> -	clk_prepare_enable(udc->iclk);
> >>> -	clk_prepare_enable(udc->fclk);
> >>> +	clk_enable(udc->iclk);
> >>> +	clk_enable(udc->fclk);
> >>>   }
> >>>
> >>>   static void clk_off(struct at91_udc *udc)
> >>> @@ -884,10 +884,10 @@ static void clk_off(struct at91_udc *udc)
> >>>   		return;
> >>>   	udc->clocked = 0;
> >>>   	udc->gadget.speed = USB_SPEED_UNKNOWN;
> >>> -	clk_disable_unprepare(udc->fclk);
> >>> -	clk_disable_unprepare(udc->iclk);
> >>> +	clk_disable(udc->fclk);
> >>> +	clk_disable(udc->iclk);
> >>>   	if (IS_ENABLED(CONFIG_COMMON_CLK))
> >>> -		clk_disable_unprepare(udc->uclk);
> >>> +		clk_disable(udc->uclk);
> >>>   }
> >>
> >> As you stated prepare and unprepare should never be called in interrupt
> >> context.
> >>
> >> My concern here is that PLLB (which is often used as USB clock
> >> parent) will never be gated/disabled (because all this work is
> >> done in its unprepare method), and thus your power consumption will be
> >> higher (when entering suspend mode) than if you'd done a
> >> disable_unprepare call.
> >>
> >> How about leaving the clk_on/off unchanged and use a threaded irq
> >> instead of a normal irq ?
> 
> Even with threaded interrupts things are still called while interrupts 
> are disabled by one or more layers of spin_lock_irqsave. There are also 
> different code paths from where clk_on() is called.

You're right (I only had a quick look at it). Let's fix this as a first
step and we'll figure out how to optimize power consumption later.
BTW, clk_set_rate can sleep too (AFAIK clk_enable and clk_disable
are the only one that can be called in atomic context).

Best Regards,

Boris

-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[PATCH v2] usb: gadget: at91_udc: move prepare clk into process context

[Felipe Balbi]

Hi,

On Wed, Aug 06, 2014 at 03:11:42PM +0200, Ronald Wahl wrote:
> Commit 7628083227b6bc4a7e33d7c381d7a4e558424b6b added clock preparation in
> interrupt context. This is not allowed as it might sleep. Move clock
> preparation into process context (at91udc_probe).

missing Signed-off-by. Also, please resend with linux-usb in the loop.

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140807/fbff6d10/attachment.sig>

[PATCH v2] usb: gadget: at91_udc: move prepare clk into process context

[Mike Turquette]

Quoting Ronald Wahl (2014-08-06 06:11:42)
> Commit 7628083227b6bc4a7e33d7c381d7a4e558424b6b added clock preparation in
> interrupt context. This is not allowed as it might sleep. Move clock
> preparation into process context (at91udc_probe).
> ---
>  drivers/usb/gadget/udc/at91_udc.c | 39 ++++++++++++++++++++++++++++++---------
>  1 file changed, 30 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/usb/gadget/udc/at91_udc.c b/drivers/usb/gadget/udc/at91_udc.c
> index cfd18bc..0b347a0 100644
> --- a/drivers/usb/gadget/udc/at91_udc.c
> +++ b/drivers/usb/gadget/udc/at91_udc.c
> @@ -872,10 +872,10 @@ static void clk_on(struct at91_udc *udc)
>  
>         if (IS_ENABLED(CONFIG_COMMON_CLK)) {

Why is this check necessary at all? Drivers shouldn't have to care at
all about the underlying clock framework implementation.

>                 clk_set_rate(udc->uclk, 48000000);
> -               clk_prepare_enable(udc->uclk);
> +               clk_enable(udc->uclk);
>         }
> -       clk_prepare_enable(udc->iclk);
> -       clk_prepare_enable(udc->fclk);
> +       clk_enable(udc->iclk);
> +       clk_enable(udc->fclk);
>  }
>  
>  static void clk_off(struct at91_udc *udc)
> @@ -884,10 +884,10 @@ static void clk_off(struct at91_udc *udc)
>                 return;
>         udc->clocked = 0;
>         udc->gadget.speed = USB_SPEED_UNKNOWN;
> -       clk_disable_unprepare(udc->fclk);
> -       clk_disable_unprepare(udc->iclk);
> +       clk_disable(udc->fclk);
> +       clk_disable(udc->iclk);
>         if (IS_ENABLED(CONFIG_COMMON_CLK))
> -               clk_disable_unprepare(udc->uclk);
> +               clk_disable(udc->uclk);
>  }
>  
>  /*
> @@ -1780,14 +1780,23 @@ static int at91udc_probe(struct platform_device *pdev)
>         }
>  
>         /* don't do anything until we have both gadget driver and VBUS */
> +       if (IS_ENABLED(CONFIG_COMMON_CLK)) {

Same question here. What does the clock framework implementation have to
do with uclk?

Regards,
Mike

[PATCH v2] usb: gadget: at91_udc: move prepare clk into process context

[Alexandre Belloni]

Hi Mike,

On 11/08/2014 at 20:34:56 -0700, Mike Turquette wrote :
> Quoting Ronald Wahl (2014-08-06 06:11:42)
> > Commit 7628083227b6bc4a7e33d7c381d7a4e558424b6b added clock preparation in
> > interrupt context. This is not allowed as it might sleep. Move clock
> > preparation into process context (at91udc_probe).
> > ---
> >  drivers/usb/gadget/udc/at91_udc.c | 39 ++++++++++++++++++++++++++++++---------
> >  1 file changed, 30 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/usb/gadget/udc/at91_udc.c b/drivers/usb/gadget/udc/at91_udc.c
> > index cfd18bc..0b347a0 100644
> > --- a/drivers/usb/gadget/udc/at91_udc.c
> > +++ b/drivers/usb/gadget/udc/at91_udc.c
> > @@ -872,10 +872,10 @@ static void clk_on(struct at91_udc *udc)
> >  
> >         if (IS_ENABLED(CONFIG_COMMON_CLK)) {
> 
> Why is this check necessary at all? Drivers shouldn't have to care at
> all about the underlying clock framework implementation.
> 

I believe it has been done because without the common clock framework,
usb_clk is not defined.

> >                 clk_set_rate(udc->uclk, 48000000);
> > -               clk_prepare_enable(udc->uclk);
> > +               clk_enable(udc->uclk);
> >         }
> > -       clk_prepare_enable(udc->iclk);
> > -       clk_prepare_enable(udc->fclk);
> > +       clk_enable(udc->iclk);
> > +       clk_enable(udc->fclk);
> >  }
> >  
> >  static void clk_off(struct at91_udc *udc)
> > @@ -884,10 +884,10 @@ static void clk_off(struct at91_udc *udc)
> >                 return;
> >         udc->clocked = 0;
> >         udc->gadget.speed = USB_SPEED_UNKNOWN;
> > -       clk_disable_unprepare(udc->fclk);
> > -       clk_disable_unprepare(udc->iclk);
> > +       clk_disable(udc->fclk);
> > +       clk_disable(udc->iclk);
> >         if (IS_ENABLED(CONFIG_COMMON_CLK))
> > -               clk_disable_unprepare(udc->uclk);
> > +               clk_disable(udc->uclk);
> >  }
> >  
> >  /*
> > @@ -1780,14 +1780,23 @@ static int at91udc_probe(struct platform_device *pdev)
> >         }
> >  
> >         /* don't do anything until we have both gadget driver and VBUS */
> > +       if (IS_ENABLED(CONFIG_COMMON_CLK)) {
> 
> Same question here. What does the clock framework implementation have to
> do with uclk?
> 
> Regards,
> Mike
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[PATCH v2] usb: gadget: at91_udc: move prepare clk into process context

[Boris BREZILLON]

On Wed, 13 Aug 2014 16:53:49 +0200
Alexandre Belloni <alexandre.belloni@free-electrons.com> wrote:

> Hi Mike,
> 
> On 11/08/2014 at 20:34:56 -0700, Mike Turquette wrote :
> > Quoting Ronald Wahl (2014-08-06 06:11:42)
> > > Commit 7628083227b6bc4a7e33d7c381d7a4e558424b6b added clock preparation in
> > > interrupt context. This is not allowed as it might sleep. Move clock
> > > preparation into process context (at91udc_probe).
> > > ---
> > >  drivers/usb/gadget/udc/at91_udc.c | 39 ++++++++++++++++++++++++++++++---------
> > >  1 file changed, 30 insertions(+), 9 deletions(-)
> > > 
> > > diff --git a/drivers/usb/gadget/udc/at91_udc.c b/drivers/usb/gadget/udc/at91_udc.c
> > > index cfd18bc..0b347a0 100644
> > > --- a/drivers/usb/gadget/udc/at91_udc.c
> > > +++ b/drivers/usb/gadget/udc/at91_udc.c
> > > @@ -872,10 +872,10 @@ static void clk_on(struct at91_udc *udc)
> > >  
> > >         if (IS_ENABLED(CONFIG_COMMON_CLK)) {
> > 
> > Why is this check necessary at all? Drivers shouldn't have to care at
> > all about the underlying clock framework implementation.
> > 
> 
> I believe it has been done because without the common clock framework,
> usb_clk is not defined.

Absolutely: uclk is only available when using the CCF. In the old at91
clock implementation the USB clock rate was configured during early boot
at registration time.
With the CCF implementation USB clk rate is no longer hardcoded at init
time and we have to configure it appropriately before using it (the
prepare and enable actions are useless though because uclk is the parent
of fclk, and thus will be prepared/enabled when fclk is
prepared/enabled).

What we could do here is test for CONFIG_COMMON_CLK_AT91 instead of
CONFIG_COMMON_CLK (so that there's no direct dependency on the CCF).

Another option is to implement determine_rate in the new at91 usb clk
implementation and then call clk_set_rate on fclk (without testing the
return code, because the old clk implementation will always return
-EINVAL).

> 
> > >                 clk_set_rate(udc->uclk, 48000000);
> > > -               clk_prepare_enable(udc->uclk);
> > > +               clk_enable(udc->uclk);
> > >         }
> > > -       clk_prepare_enable(udc->iclk);
> > > -       clk_prepare_enable(udc->fclk);
> > > +       clk_enable(udc->iclk);
> > > +       clk_enable(udc->fclk);
> > >  }
> > >  
> > >  static void clk_off(struct at91_udc *udc)
> > > @@ -884,10 +884,10 @@ static void clk_off(struct at91_udc *udc)
> > >                 return;
> > >         udc->clocked = 0;
> > >         udc->gadget.speed = USB_SPEED_UNKNOWN;
> > > -       clk_disable_unprepare(udc->fclk);
> > > -       clk_disable_unprepare(udc->iclk);
> > > +       clk_disable(udc->fclk);
> > > +       clk_disable(udc->iclk);
> > >         if (IS_ENABLED(CONFIG_COMMON_CLK))
> > > -               clk_disable_unprepare(udc->uclk);
> > > +               clk_disable(udc->uclk);
> > >  }
> > >  
> > >  /*
> > > @@ -1780,14 +1780,23 @@ static int at91udc_probe(struct platform_device *pdev)
> > >         }
> > >  
> > >         /* don't do anything until we have both gadget driver and VBUS */
> > > +       if (IS_ENABLED(CONFIG_COMMON_CLK)) {
> > 
> > Same question here. What does the clock framework implementation have to
> > do with uclk?
> > 
> > Regards,
> > Mike
> > 
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel at lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 



-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[PATCH v2] usb: gadget: at91_udc: move prepare clk into process context

[Nicolas Ferre]

On 13/08/2014 17:20, Boris BREZILLON :
> On Wed, 13 Aug 2014 16:53:49 +0200
> Alexandre Belloni <alexandre.belloni@free-electrons.com> wrote:
> 
>> Hi Mike,
>>
>> On 11/08/2014 at 20:34:56 -0700, Mike Turquette wrote :
>>> Quoting Ronald Wahl (2014-08-06 06:11:42)
>>>> Commit 7628083227b6bc4a7e33d7c381d7a4e558424b6b added clock preparation in
>>>> interrupt context. This is not allowed as it might sleep. Move clock
>>>> preparation into process context (at91udc_probe).
>>>> ---
>>>>  drivers/usb/gadget/udc/at91_udc.c | 39 ++++++++++++++++++++++++++++++---------
>>>>  1 file changed, 30 insertions(+), 9 deletions(-)
>>>>
>>>> diff --git a/drivers/usb/gadget/udc/at91_udc.c b/drivers/usb/gadget/udc/at91_udc.c
>>>> index cfd18bc..0b347a0 100644
>>>> --- a/drivers/usb/gadget/udc/at91_udc.c
>>>> +++ b/drivers/usb/gadget/udc/at91_udc.c
>>>> @@ -872,10 +872,10 @@ static void clk_on(struct at91_udc *udc)
>>>>  
>>>>         if (IS_ENABLED(CONFIG_COMMON_CLK)) {
>>>
>>> Why is this check necessary at all? Drivers shouldn't have to care at
>>> all about the underlying clock framework implementation.
>>>
>>
>> I believe it has been done because without the common clock framework,
>> usb_clk is not defined.
> 
> Absolutely: uclk is only available when using the CCF. In the old at91
> clock implementation the USB clock rate was configured during early boot
> at registration time.
> With the CCF implementation USB clk rate is no longer hardcoded at init
> time and we have to configure it appropriately before using it (the
> prepare and enable actions are useless though because uclk is the parent
> of fclk, and thus will be prepared/enabled when fclk is
> prepared/enabled).
> 
> What we could do here is test for CONFIG_COMMON_CLK_AT91 instead of
> CONFIG_COMMON_CLK (so that there's no direct dependency on the CCF).
> 
> Another option is to implement determine_rate in the new at91 usb clk
> implementation and then call clk_set_rate on fclk (without testing the
> return code, because the old clk implementation will always return
> -EINVAL).

I would recommend to keep it like that until we remove all need for the
old clock implementation. Then we will be able to remove this test and
simplify the code.
In the meantime, it is stated explicitly and I find it more obvious than
trying to hide this matter of fact.

Bye,

>>>>                 clk_set_rate(udc->uclk, 48000000);
>>>> -               clk_prepare_enable(udc->uclk);
>>>> +               clk_enable(udc->uclk);
>>>>         }
>>>> -       clk_prepare_enable(udc->iclk);
>>>> -       clk_prepare_enable(udc->fclk);
>>>> +       clk_enable(udc->iclk);
>>>> +       clk_enable(udc->fclk);
>>>>  }
>>>>  
>>>>  static void clk_off(struct at91_udc *udc)
>>>> @@ -884,10 +884,10 @@ static void clk_off(struct at91_udc *udc)
>>>>                 return;
>>>>         udc->clocked = 0;
>>>>         udc->gadget.speed = USB_SPEED_UNKNOWN;
>>>> -       clk_disable_unprepare(udc->fclk);
>>>> -       clk_disable_unprepare(udc->iclk);
>>>> +       clk_disable(udc->fclk);
>>>> +       clk_disable(udc->iclk);
>>>>         if (IS_ENABLED(CONFIG_COMMON_CLK))
>>>> -               clk_disable_unprepare(udc->uclk);
>>>> +               clk_disable(udc->uclk);
>>>>  }
>>>>  
>>>>  /*
>>>> @@ -1780,14 +1780,23 @@ static int at91udc_probe(struct platform_device *pdev)
>>>>         }
>>>>  
>>>>         /* don't do anything until we have both gadget driver and VBUS */
>>>> +       if (IS_ENABLED(CONFIG_COMMON_CLK)) {
>>>
>>> Same question here. What does the clock framework implementation have to
>>> do with uclk?
>>>
>>> Regards,
>>> Mike
>>>
>>> _______________________________________________
>>> linux-arm-kernel mailing list
>>> linux-arm-kernel at lists.infradead.org
>>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>
> 
> 
> 


-- 
Nicolas Ferre