From: b.brezillon@overkiz.com (boris brezillon)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v3 6/7] USB: ohci-at91: add usb_clk for transition to common clk framework
Date: Tue, 16 Jul 2013 19:08:12 +0200 [thread overview]
Message-ID: <51E57DFC.5030302@overkiz.com> (raw)
In-Reply-To: <Pine.LNX.4.44L0.1307161246130.1032-100000@iolanthe.rowland.org>
Hello Alan,
On 16/07/2013 18:48, Alan Stern wrote:
> On Tue, 16 Jul 2013, Boris BREZILLON wrote:
>
>> The AT91 PMC (Power Management Controller) provides an USB clock used by
>> USB Full Speed host (ohci) and USB Full Speed device (udc).
>> The usb drivers (ohci and udc) must configure this clock to 48Mhz.
>> This configuration was formely done in mach-at91/clock.c, but this
>> implementation will be removed when moving to common clk framework.
>>
>> This patch add support for usb clock retrieval and configuration, and is
>> backward compatible with the current at91 clk implementation (if usb clk
>> is not found, it does not configure/enable the usb clk).
> But it does print a warning in the system log, right?
Yes it does.
>
>> @@ -144,6 +150,11 @@ static int usb_hcd_at91_probe(const struct hc_driver *driver,
>> goto err2;
>> }
>>
>> + uclk = clk_get(&pdev->dev, "usb_clk");
>> + if (IS_ERR(uclk)) {
>> + uclk = NULL;
>> + dev_warn(&pdev->dev, "failed to get usb_clk\n");
>> + }
> Is this really what you want for backward compatibility?
Here are some proposition to remove the warning message:
1) replace it with a dev_info and change the message:
dev_info(&pdev->dev, "failed to get usb_clk (most likely using old
at91 clk implementation)\n");
2) drop the log and silently ignore the missing clk (I'm not a big fan
of this solution
as it may lead to some errors if we're using new clk implem and the
clock is really missing)
3) rework the current clk_set_rate function to accept clk_set_rate on
usb clk and add clk_lookup entries
for the usb clk (I'm not a big fan of this solution neither as this
modifications will only be used for a short time
until the transition to common clk framework is completed).
> Alan Stern
>
Thanks for your review.
Best Regards,
Boris
WARNING: multiple messages have this Message-ID (diff)
From: boris brezillon <b.brezillon@overkiz.com>
To: Alan Stern <stern@rowland.harvard.edu>
Cc: Nicolas Ferre <nicolas.ferre@atmel.com>,
Ludovic Desroches <ludovic.desroches@atmel.com>,
Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org
Subject: Re: [PATCH v3 6/7] USB: ohci-at91: add usb_clk for transition to common clk framework
Date: Tue, 16 Jul 2013 19:08:12 +0200 [thread overview]
Message-ID: <51E57DFC.5030302@overkiz.com> (raw)
In-Reply-To: <Pine.LNX.4.44L0.1307161246130.1032-100000@iolanthe.rowland.org>
Hello Alan,
On 16/07/2013 18:48, Alan Stern wrote:
> On Tue, 16 Jul 2013, Boris BREZILLON wrote:
>
>> The AT91 PMC (Power Management Controller) provides an USB clock used by
>> USB Full Speed host (ohci) and USB Full Speed device (udc).
>> The usb drivers (ohci and udc) must configure this clock to 48Mhz.
>> This configuration was formely done in mach-at91/clock.c, but this
>> implementation will be removed when moving to common clk framework.
>>
>> This patch add support for usb clock retrieval and configuration, and is
>> backward compatible with the current at91 clk implementation (if usb clk
>> is not found, it does not configure/enable the usb clk).
> But it does print a warning in the system log, right?
Yes it does.
>
>> @@ -144,6 +150,11 @@ static int usb_hcd_at91_probe(const struct hc_driver *driver,
>> goto err2;
>> }
>>
>> + uclk = clk_get(&pdev->dev, "usb_clk");
>> + if (IS_ERR(uclk)) {
>> + uclk = NULL;
>> + dev_warn(&pdev->dev, "failed to get usb_clk\n");
>> + }
> Is this really what you want for backward compatibility?
Here are some proposition to remove the warning message:
1) replace it with a dev_info and change the message:
dev_info(&pdev->dev, "failed to get usb_clk (most likely using old
at91 clk implementation)\n");
2) drop the log and silently ignore the missing clk (I'm not a big fan
of this solution
as it may lead to some errors if we're using new clk implem and the
clock is really missing)
3) rework the current clk_set_rate function to accept clk_set_rate on
usb clk and add clk_lookup entries
for the usb clk (I'm not a big fan of this solution neither as this
modifications will only be used for a short time
until the transition to common clk framework is completed).
> Alan Stern
>
Thanks for your review.
Best Regards,
Boris
next prev parent reply other threads:[~2013-07-16 17:08 UTC|newest]
Thread overview: 43+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-07-16 15:03 [PATCH v3 0/7] ARM: at91: prepare transition to common clk framework Boris BREZILLON
2013-07-16 15:03 ` Boris BREZILLON
2013-07-16 15:05 ` [PATCH v3 1/7] ARM: at91/tc/clocksource: replace clk_enable/disable with clk_prepare_enable/disable_unprepare Boris BREZILLON
2013-07-16 15:05 ` Boris BREZILLON
2013-07-16 15:48 ` Daniel Lezcano
2013-07-16 15:48 ` Daniel Lezcano
2013-07-16 15:06 ` [PATCH v3 2/7] mmc: atmel-mci: prepare clk before calling enable Boris BREZILLON
2013-07-16 15:06 ` Boris BREZILLON
2013-07-16 15:13 ` Thomas Petazzoni
2013-07-16 15:13 ` Thomas Petazzoni
2013-07-16 15:49 ` boris brezillon
2013-07-16 15:49 ` boris brezillon
2013-07-16 17:13 ` Russell King - ARM Linux
2013-07-16 17:13 ` Russell King - ARM Linux
2013-07-16 15:08 ` [PATCH v3 3/7] at91/avr32/atmel_lcdfb: " Boris BREZILLON
2013-07-16 15:08 ` Boris BREZILLON
2013-07-16 15:08 ` Boris BREZILLON
2013-07-16 15:10 ` [PATCH v3 4/7] USB: gadget: atmel_usba: " Boris BREZILLON
2013-07-16 15:10 ` Boris BREZILLON
2013-07-16 15:16 ` [PATCH v3 5/7] spi: atmel: " Boris BREZILLON
2013-07-16 15:16 ` Boris BREZILLON
2013-07-17 8:10 ` Mark Brown
2013-07-17 8:10 ` Mark Brown
2013-07-16 15:22 ` [PATCH v3 6/7] USB: ohci-at91: add usb_clk for transition to common clk framework Boris BREZILLON
2013-07-16 15:22 ` Boris BREZILLON
2013-07-16 16:48 ` Alan Stern
2013-07-16 16:48 ` Alan Stern
2013-07-16 17:08 ` boris brezillon [this message]
2013-07-16 17:08 ` boris brezillon
2013-07-16 18:47 ` Alan Stern
2013-07-16 18:47 ` Alan Stern
2013-07-16 19:06 ` boris brezillon
2013-07-16 19:06 ` boris brezillon
2013-07-16 17:07 ` Russell King - ARM Linux
2013-07-16 17:07 ` Russell King - ARM Linux
2013-07-17 15:33 ` Alan Stern
2013-07-17 15:33 ` Alan Stern
2013-07-17 15:45 ` boris brezillon
2013-07-17 15:45 ` boris brezillon
2013-07-17 16:17 ` Alan Stern
2013-07-17 16:17 ` Alan Stern
2013-07-16 15:25 ` [PATCH v3 7/7] usb: gadget: at91_udc: " Boris BREZILLON
2013-07-16 15:25 ` Boris BREZILLON
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=51E57DFC.5030302@overkiz.com \
--to=b.brezillon@overkiz.com \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.