From: Ronald Wahl <[email protected]>
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 <[email protected]>
Acked-by: Alexandre Belloni <[email protected]>
Acked-by: Boris Brezillon <[email protected]>
Acked-by: Nicolas Ferre <[email protected]>
Cc: Felipe Balbi <[email protected]>
Cc: <[email protected]> # 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, 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))
--
2.1.3
On Wed, Nov 19, 2014 at 04:37:27PM +0100, Nicolas Ferre wrote:
> From: Ronald Wahl <[email protected]>
>
> 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 <[email protected]>
> Acked-by: Alexandre Belloni <[email protected]>
> Acked-by: Boris Brezillon <[email protected]>
> Acked-by: Nicolas Ferre <[email protected]>
> Cc: Felipe Balbi <[email protected]>
> Cc: <[email protected]> # 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
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 <[email protected]>
> >
> > 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 <[email protected]>
> > Acked-by: Alexandre Belloni <[email protected]>
> > Acked-by: Boris Brezillon <[email protected]>
> > Acked-by: Nicolas Ferre <[email protected]>
> > Cc: Felipe Balbi <[email protected]>
> > Cc: <[email protected]> # 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
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 <[email protected]>
>>>>
>>>> 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 <[email protected]>
>>>> Acked-by: Alexandre Belloni <[email protected]>
>>>> Acked-by: Boris Brezillon <[email protected]>
>>>> Acked-by: Nicolas Ferre <[email protected]>
>>>> Cc: Felipe Balbi <[email protected]>
>>>> Cc: <[email protected]> # 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
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 <[email protected]>
>>>
>>> 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 <[email protected]>
>>> Acked-by: Alexandre Belloni <[email protected]>
>>> Acked-by: Boris Brezillon <[email protected]>
>>> Acked-by: Nicolas Ferre <[email protected]>
>>> Cc: Felipe Balbi <[email protected]>
>>> Cc: <[email protected]> # 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 - [email protected] - 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
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 <[email protected]>
> >>>>
> >>>> 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 <[email protected]>
> >>>> Acked-by: Alexandre Belloni <[email protected]>
> >>>> Acked-by: Boris Brezillon <[email protected]>
> >>>> Acked-by: Nicolas Ferre <[email protected]>
> >>>> Cc: Felipe Balbi <[email protected]>
> >>>> Cc: <[email protected]> # 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
Hi,
On Fri, Dec 19, 2014 at 02:18:31PM +0000, 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 <[email protected]>
> > >>>>
> > >>>> 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 <[email protected]>
> > >>>> Acked-by: Alexandre Belloni <[email protected]>
> > >>>> Acked-by: Boris Brezillon <[email protected]>
> > >>>> Acked-by: Nicolas Ferre <[email protected]>
> > >>>> Cc: Felipe Balbi <[email protected]>
> > >>>> Cc: <[email protected]> # 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