From: ronald.wahl@raritan•com (Ronald Wahl)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH v2] usb: gadget: at91_udc: move prepare clk into process context
Date: Thu, 07 Aug 2014 14:43:32 +0200 [thread overview]
Message-ID: <53E37474.6060501@raritan.com> (raw)
In-Reply-To: <20140807095912.7648de46@bbrezillon>
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
next prev parent reply other threads:[~2014-08-07 12:43 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-08-06 13:11 [PATCH v2] usb: gadget: at91_udc: move prepare clk into process context Ronald Wahl
2014-08-07 7:52 ` Boris BREZILLON
2014-08-07 7:59 ` Boris BREZILLON
2014-08-07 12:43 ` Ronald Wahl [this message]
2014-08-07 13:46 ` Boris BREZILLON
2014-08-07 15:24 ` Felipe Balbi
2014-08-12 3:34 ` Mike Turquette
2014-08-13 14:53 ` Alexandre Belloni
2014-08-13 15:20 ` Boris BREZILLON
2014-08-13 15:55 ` Nicolas Ferre
-- strict thread matches above, loose matches on Subject: below --
2014-08-06 23:45 Ronald Wahl
2014-08-07 15:11 Ronald Wahl
2014-08-07 15:23 ` Ronald Wahl
2014-08-07 22:32 ` Alexandre Belloni
2014-08-07 16:27 ` Sergei Shtylyov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=53E37474.6060501@raritan.com \
--to=ronald.wahl@raritan$(echo .)com \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox