Re: [RESEND PATCH] usb: gadget: at91_udc: move prepare clk into process context
On Fri, 2014-12-19 at 13:51 +, Luis Henriques wrote: Hi Felipe, On Thu, Nov 20, 2014 at 01:50:49PM -0600, Felipe Balbi wrote: On Wed, Nov 19, 2014 at 04:37:27PM +0100, Nicolas Ferre wrote: From: Ronald Wahl ronald.w...@raritan.com Commit 7628083227b6bc4a7e33d7c381d7a4e558424b6b (usb: gadget: at91_udc: prepare clk before calling enable) added clock preparation in interrupt context. This is not allowed as it might sleep. Also setting the clock rate is unsafe to call from there for the same reason. Move clock preparation and setting clock rate into process context (at91udc_probe). Signed-off-by: Ronald Wahl ronald.w...@raritan.com Acked-by: Alexandre Belloni alexandre.bell...@free-electrons.com Acked-by: Boris Brezillon boris.brezil...@free-electrons.com Acked-by: Nicolas Ferre nicolas.fe...@atmel.com Cc: Felipe Balbi ba...@ti.com Cc: sta...@vger.kernel.org # v3.17+ --- Hi Felipe, I forgot to answer you on this patch. So I resend it now with the proper stable tag. You can also queue it during this -rc phase if you feel it is still possible. I think it's late for v3.18, so it'll go on v3.19 and get backported to 3.17 and 3.18. Sorry :-s Although this commit (b2ba27a5c56f usb: gadget: at91_udc: move prepare clk into process context) is tagged for stable v3.17+, it seems like it could be applied to earlier kernels. 3.16, 3.13 and 3.12 seem to be affected by the same issue (and they all include commit 7628083227b6 usb: gadget: at91_udc: prepare clk before calling enable). Is there any reason for not applying it in these trees? I'll queue it up for 3.13-stable. Thanks all! -Kamal -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND PATCH] usb: gadget: at91_udc: move prepare clk into process context
Hi, On Fri, Dec 19, 2014 at 02:18:31PM +, Luis Henriques wrote: On Fri, Dec 19, 2014 at 03:08:15PM +0100, Nicolas Ferre wrote: Le 19/12/2014 15:02, Ronald Wahl a écrit : On 19.12.2014 14:51, Luis Henriques wrote: Hi Felipe, On Thu, Nov 20, 2014 at 01:50:49PM -0600, Felipe Balbi wrote: On Wed, Nov 19, 2014 at 04:37:27PM +0100, Nicolas Ferre wrote: From: Ronald Wahl ronald.w...@raritan.com Commit 7628083227b6bc4a7e33d7c381d7a4e558424b6b (usb: gadget: at91_udc: prepare clk before calling enable) added clock preparation in interrupt context. This is not allowed as it might sleep. Also setting the clock rate is unsafe to call from there for the same reason. Move clock preparation and setting clock rate into process context (at91udc_probe). Signed-off-by: Ronald Wahl ronald.w...@raritan.com Acked-by: Alexandre Belloni alexandre.bell...@free-electrons.com Acked-by: Boris Brezillon boris.brezil...@free-electrons.com Acked-by: Nicolas Ferre nicolas.fe...@atmel.com Cc: Felipe Balbi ba...@ti.com Cc: sta...@vger.kernel.org # v3.17+ --- Hi Felipe, I forgot to answer you on this patch. So I resend it now with the proper stable tag. You can also queue it during this -rc phase if you feel it is still possible. I think it's late for v3.18, so it'll go on v3.19 and get backported to 3.17 and 3.18. Sorry :-s Although this commit (b2ba27a5c56f usb: gadget: at91_udc: move prepare clk into process context) is tagged for stable v3.17+, it seems like it could be applied to earlier kernels. 3.16, 3.13 and 3.12 seem to be affected by the same issue (and they all include commit 7628083227b6 usb: gadget: at91_udc: prepare clk before calling enable). Is there any reason for not applying it in these trees? Not to forget 3.14 (LTS) which was the branch where I primarily found the issue... Yes, of course! Sorry. Well it's maybe an issue with the re-naming of the directory to drivers/usb/gadget/udc/ introduced by patch: 90fccb529d24 (usb: gadget: Gadget directory cleanup - group UDC drivers) The patch doesn't apply out of the box but it surely can be applied in those earlier kernels. Right, the file was renamed but the backport seems to be trivial. Thanks for confirming. can someone resend the patch with the correct Cc: stable tag so I can apply it ? -- balbi signature.asc Description: Digital signature
Re: [RESEND PATCH] usb: gadget: at91_udc: move prepare clk into process context
Hi Felipe, On Thu, Nov 20, 2014 at 01:50:49PM -0600, Felipe Balbi wrote: On Wed, Nov 19, 2014 at 04:37:27PM +0100, Nicolas Ferre wrote: From: Ronald Wahl ronald.w...@raritan.com Commit 7628083227b6bc4a7e33d7c381d7a4e558424b6b (usb: gadget: at91_udc: prepare clk before calling enable) added clock preparation in interrupt context. This is not allowed as it might sleep. Also setting the clock rate is unsafe to call from there for the same reason. Move clock preparation and setting clock rate into process context (at91udc_probe). Signed-off-by: Ronald Wahl ronald.w...@raritan.com Acked-by: Alexandre Belloni alexandre.bell...@free-electrons.com Acked-by: Boris Brezillon boris.brezil...@free-electrons.com Acked-by: Nicolas Ferre nicolas.fe...@atmel.com Cc: Felipe Balbi ba...@ti.com Cc: sta...@vger.kernel.org # v3.17+ --- Hi Felipe, I forgot to answer you on this patch. So I resend it now with the proper stable tag. You can also queue it during this -rc phase if you feel it is still possible. I think it's late for v3.18, so it'll go on v3.19 and get backported to 3.17 and 3.18. Sorry :-s Although this commit (b2ba27a5c56f usb: gadget: at91_udc: move prepare clk into process context) is tagged for stable v3.17+, it seems like it could be applied to earlier kernels. 3.16, 3.13 and 3.12 seem to be affected by the same issue (and they all include commit 7628083227b6 usb: gadget: at91_udc: prepare clk before calling enable). Is there any reason for not applying it in these trees? Cheers, -- Luís cheers -- balbi -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND PATCH] usb: gadget: at91_udc: move prepare clk into process context
On 19.12.2014 14:51, Luis Henriques wrote: Hi Felipe, On Thu, Nov 20, 2014 at 01:50:49PM -0600, Felipe Balbi wrote: On Wed, Nov 19, 2014 at 04:37:27PM +0100, Nicolas Ferre wrote: From: Ronald Wahl ronald.w...@raritan.com Commit 7628083227b6bc4a7e33d7c381d7a4e558424b6b (usb: gadget: at91_udc: prepare clk before calling enable) added clock preparation in interrupt context. This is not allowed as it might sleep. Also setting the clock rate is unsafe to call from there for the same reason. Move clock preparation and setting clock rate into process context (at91udc_probe). Signed-off-by: Ronald Wahl ronald.w...@raritan.com Acked-by: Alexandre Belloni alexandre.bell...@free-electrons.com Acked-by: Boris Brezillon boris.brezil...@free-electrons.com Acked-by: Nicolas Ferre nicolas.fe...@atmel.com Cc: Felipe Balbi ba...@ti.com Cc: sta...@vger.kernel.org # v3.17+ --- Hi Felipe, I forgot to answer you on this patch. So I resend it now with the proper stable tag. You can also queue it during this -rc phase if you feel it is still possible. I think it's late for v3.18, so it'll go on v3.19 and get backported to 3.17 and 3.18. Sorry :-s Although this commit (b2ba27a5c56f usb: gadget: at91_udc: move prepare clk into process context) is tagged for stable v3.17+, it seems like it could be applied to earlier kernels. 3.16, 3.13 and 3.12 seem to be affected by the same issue (and they all include commit 7628083227b6 usb: gadget: at91_udc: prepare clk before calling enable). Is there any reason for not applying it in these trees? Not to forget 3.14 (LTS) which was the branch where I primarily found the issue... - ron -- Ronald Wahl - ronald.w...@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 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND PATCH] usb: gadget: at91_udc: move prepare clk into process context
Le 19/12/2014 15:02, Ronald Wahl a écrit : On 19.12.2014 14:51, Luis Henriques wrote: Hi Felipe, On Thu, Nov 20, 2014 at 01:50:49PM -0600, Felipe Balbi wrote: On Wed, Nov 19, 2014 at 04:37:27PM +0100, Nicolas Ferre wrote: From: Ronald Wahl ronald.w...@raritan.com Commit 7628083227b6bc4a7e33d7c381d7a4e558424b6b (usb: gadget: at91_udc: prepare clk before calling enable) added clock preparation in interrupt context. This is not allowed as it might sleep. Also setting the clock rate is unsafe to call from there for the same reason. Move clock preparation and setting clock rate into process context (at91udc_probe). Signed-off-by: Ronald Wahl ronald.w...@raritan.com Acked-by: Alexandre Belloni alexandre.bell...@free-electrons.com Acked-by: Boris Brezillon boris.brezil...@free-electrons.com Acked-by: Nicolas Ferre nicolas.fe...@atmel.com Cc: Felipe Balbi ba...@ti.com Cc: sta...@vger.kernel.org # v3.17+ --- Hi Felipe, I forgot to answer you on this patch. So I resend it now with the proper stable tag. You can also queue it during this -rc phase if you feel it is still possible. I think it's late for v3.18, so it'll go on v3.19 and get backported to 3.17 and 3.18. Sorry :-s Although this commit (b2ba27a5c56f usb: gadget: at91_udc: move prepare clk into process context) is tagged for stable v3.17+, it seems like it could be applied to earlier kernels. 3.16, 3.13 and 3.12 seem to be affected by the same issue (and they all include commit 7628083227b6 usb: gadget: at91_udc: prepare clk before calling enable). Is there any reason for not applying it in these trees? Not to forget 3.14 (LTS) which was the branch where I primarily found the issue... Well it's maybe an issue with the re-naming of the directory to drivers/usb/gadget/udc/ introduced by patch: 90fccb529d24 (usb: gadget: Gadget directory cleanup - group UDC drivers) The patch doesn't apply out of the box but it surely can be applied in those earlier kernels. Thanks, bye. -- Nicolas Ferre -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND PATCH] usb: gadget: at91_udc: move prepare clk into process context
On Fri, Dec 19, 2014 at 03:08:15PM +0100, Nicolas Ferre wrote: Le 19/12/2014 15:02, Ronald Wahl a écrit : On 19.12.2014 14:51, Luis Henriques wrote: Hi Felipe, On Thu, Nov 20, 2014 at 01:50:49PM -0600, Felipe Balbi wrote: On Wed, Nov 19, 2014 at 04:37:27PM +0100, Nicolas Ferre wrote: From: Ronald Wahl ronald.w...@raritan.com Commit 7628083227b6bc4a7e33d7c381d7a4e558424b6b (usb: gadget: at91_udc: prepare clk before calling enable) added clock preparation in interrupt context. This is not allowed as it might sleep. Also setting the clock rate is unsafe to call from there for the same reason. Move clock preparation and setting clock rate into process context (at91udc_probe). Signed-off-by: Ronald Wahl ronald.w...@raritan.com Acked-by: Alexandre Belloni alexandre.bell...@free-electrons.com Acked-by: Boris Brezillon boris.brezil...@free-electrons.com Acked-by: Nicolas Ferre nicolas.fe...@atmel.com Cc: Felipe Balbi ba...@ti.com Cc: sta...@vger.kernel.org # v3.17+ --- Hi Felipe, I forgot to answer you on this patch. So I resend it now with the proper stable tag. You can also queue it during this -rc phase if you feel it is still possible. I think it's late for v3.18, so it'll go on v3.19 and get backported to 3.17 and 3.18. Sorry :-s Although this commit (b2ba27a5c56f usb: gadget: at91_udc: move prepare clk into process context) is tagged for stable v3.17+, it seems like it could be applied to earlier kernels. 3.16, 3.13 and 3.12 seem to be affected by the same issue (and they all include commit 7628083227b6 usb: gadget: at91_udc: prepare clk before calling enable). Is there any reason for not applying it in these trees? Not to forget 3.14 (LTS) which was the branch where I primarily found the issue... Yes, of course! Sorry. Well it's maybe an issue with the re-naming of the directory to drivers/usb/gadget/udc/ introduced by patch: 90fccb529d24 (usb: gadget: Gadget directory cleanup - group UDC drivers) The patch doesn't apply out of the box but it surely can be applied in those earlier kernels. Right, the file was renamed but the backport seems to be trivial. Thanks for confirming. Cheers, -- Luís Thanks, bye. -- Nicolas Ferre -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND PATCH] usb: gadget: at91_udc: move prepare clk into process context
On Wed, Nov 19, 2014 at 04:37:27PM +0100, Nicolas Ferre wrote: From: Ronald Wahl ronald.w...@raritan.com Commit 7628083227b6bc4a7e33d7c381d7a4e558424b6b (usb: gadget: at91_udc: prepare clk before calling enable) added clock preparation in interrupt context. This is not allowed as it might sleep. Also setting the clock rate is unsafe to call from there for the same reason. Move clock preparation and setting clock rate into process context (at91udc_probe). Signed-off-by: Ronald Wahl ronald.w...@raritan.com Acked-by: Alexandre Belloni alexandre.bell...@free-electrons.com Acked-by: Boris Brezillon boris.brezil...@free-electrons.com Acked-by: Nicolas Ferre nicolas.fe...@atmel.com Cc: Felipe Balbi ba...@ti.com Cc: sta...@vger.kernel.org # v3.17+ --- Hi Felipe, I forgot to answer you on this patch. So I resend it now with the proper stable tag. You can also queue it during this -rc phase if you feel it is still possible. I think it's late for v3.18, so it'll go on v3.19 and get backported to 3.17 and 3.18. Sorry :-s cheers -- balbi signature.asc Description: Digital signature
[RESEND PATCH] usb: gadget: at91_udc: move prepare clk into process context
From: Ronald Wahl ronald.w...@raritan.com
Commit 7628083227b6bc4a7e33d7c381d7a4e558424b6b (usb: gadget: at91_udc:
prepare clk before calling enable) added clock preparation in interrupt
context. This is not allowed as it might sleep. Also setting the clock
rate is unsafe to call from there for the same reason. Move clock
preparation and setting clock rate into process context (at91udc_probe).
Signed-off-by: Ronald Wahl ronald.w...@raritan.com
Acked-by: Alexandre Belloni alexandre.bell...@free-electrons.com
Acked-by: Boris Brezillon boris.brezil...@free-electrons.com
Acked-by: Nicolas Ferre nicolas.fe...@atmel.com
Cc: Felipe Balbi ba...@ti.com
Cc: sta...@vger.kernel.org # v3.17+
---
Hi Felipe,
I forgot to answer you on this patch. So I resend it now with the proper
stable tag. You can also queue it during this -rc phase if you feel it is
still possible.
Thanks, bye.
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 9968f5331fe4..0716c1994e28 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, 4800);
- 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, 4800);
+ 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, 0x);
/* Clear all pending interrupts - UDP may be used by bootloader. */
at91_udp_write(udc, AT91_UDP_ICR, 0x);
- 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))
--
2.1.3
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
