public inbox for linux-arm-kernel@lists.infradead.org
 help / color / mirror / Atom feed
* [PATCH] USB: ohci-at91: add usb_clk for transition to common clk framework
@ 2013-08-01  6:16 Boris BREZILLON
  2013-08-01 14:39 ` Alan Stern
  0 siblings, 1 reply; 6+ messages in thread
From: Boris BREZILLON @ 2013-08-01  6:16 UTC (permalink / raw)
  To: linux-arm-kernel

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 adds 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 it).

Signed-off-by: Boris BREZILLON <b.brezillon@overkiz.com>
---
 drivers/usb/host/ohci-at91.c |   17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c
index 9677f68..913d293 100644
--- a/drivers/usb/host/ohci-at91.c
+++ b/drivers/usb/host/ohci-at91.c
@@ -31,8 +31,8 @@
 #define at91_for_each_port(index)	\
 		for ((index) = 0; (index) < AT91_MAX_USBH_PORTS; (index)++)
 
-/* interface and function clocks; sometimes also an AHB clock */
-static struct clk *iclk, *fclk, *hclk;
+/* interface, function and usb clocks; sometimes also an AHB clock */
+static struct clk *iclk, *fclk, *uclk, *hclk;
 static int clocked;
 
 extern int usb_disabled(void);
@@ -41,6 +41,10 @@ extern int usb_disabled(void);
 
 static void at91_start_clock(void)
 {
+	if (!IS_ERR(uclk)) {
+		clk_set_rate(uclk, 48000000);
+		clk_prepare_enable(uclk);
+	}
 	clk_prepare_enable(hclk);
 	clk_prepare_enable(iclk);
 	clk_prepare_enable(fclk);
@@ -52,6 +56,8 @@ static void at91_stop_clock(void)
 	clk_disable_unprepare(fclk);
 	clk_disable_unprepare(iclk);
 	clk_disable_unprepare(hclk);
+	if (!IS_ERR(uclk))
+		clk_disable_unprepare(uclk);
 	clocked = 0;
 }
 
@@ -163,6 +169,8 @@ static int usb_hcd_at91_probe(const struct hc_driver *driver,
 		goto err5;
 	}
 
+	uclk = clk_get(&pdev->dev, "usb_clk");
+
 	at91_start_hc(pdev);
 	ohci_hcd_init(hcd_to_ohci(hcd));
 
@@ -173,6 +181,9 @@ static int usb_hcd_at91_probe(const struct hc_driver *driver,
 	/* Error handling */
 	at91_stop_hc(pdev);
 
+	if (!IS_ERR(uclk))
+		clk_put(uclk);
+
 	clk_put(hclk);
  err5:
 	clk_put(fclk);
@@ -212,6 +223,8 @@ static void usb_hcd_at91_remove(struct usb_hcd *hcd,
 	release_mem_region(hcd->rsrc_start, hcd->rsrc_len);
 	usb_put_hcd(hcd);
 
+	if (!IS_ERR(uclk))
+		clk_put(uclk);
 	clk_put(hclk);
 	clk_put(fclk);
 	clk_put(iclk);
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH] USB: ohci-at91: add usb_clk for transition to common clk framework
  2013-08-01  6:16 [PATCH] USB: ohci-at91: add usb_clk for transition to common clk framework Boris BREZILLON
@ 2013-08-01 14:39 ` Alan Stern
  2013-08-01 15:14   ` boris brezillon
  0 siblings, 1 reply; 6+ messages in thread
From: Alan Stern @ 2013-08-01 14:39 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, 1 Aug 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 adds 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 it).
> 
> Signed-off-by: Boris BREZILLON <b.brezillon@overkiz.com>

> @@ -163,6 +169,8 @@ static int usb_hcd_at91_probe(const struct hc_driver *driver,
>  		goto err5;
>  	}
>  
> +	uclk = clk_get(&pdev->dev, "usb_clk");

What happens if uclk isn't found but it is needed?

Alan Stern

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] USB: ohci-at91: add usb_clk for transition to common clk framework
  2013-08-01 14:39 ` Alan Stern
@ 2013-08-01 15:14   ` boris brezillon
  2013-08-01 15:56     ` Alan Stern
  0 siblings, 1 reply; 6+ messages in thread
From: boris brezillon @ 2013-08-01 15:14 UTC (permalink / raw)
  To: linux-arm-kernel

On 01/08/2013 16:39, Alan Stern wrote:
> On Thu, 1 Aug 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 adds 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 it).
>>
>> Signed-off-by: Boris BREZILLON <b.brezillon@overkiz.com>
>> @@ -163,6 +169,8 @@ static int usb_hcd_at91_probe(const struct hc_driver *driver,
>>   		goto err5;
>>   	}
>>   
>> +	uclk = clk_get(&pdev->dev, "usb_clk");
> What happens if uclk isn't found but it is needed?
It will not fail but I guess the usb host won't work as the clock won't 
be correctly
configured.

I agree with you: this is not a good solution.

Another option is to keep the previous version of this patch and put 
uclk handling
in "#ifdef CONFIG_COMMON_CLK #endif" sections.

Would this be acceptable ?

>
> Alan Stern
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] USB: ohci-at91: add usb_clk for transition to common clk framework
  2013-08-01 15:14   ` boris brezillon
@ 2013-08-01 15:56     ` Alan Stern
  2013-08-01 17:30       ` Sergei Shtylyov
  0 siblings, 1 reply; 6+ messages in thread
From: Alan Stern @ 2013-08-01 15:56 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, 1 Aug 2013, boris brezillon wrote:

> > What happens if uclk isn't found but it is needed?
> It will not fail but I guess the usb host won't work as the clock won't 
> be correctly
> configured.
> 
> I agree with you: this is not a good solution.
> 
> Another option is to keep the previous version of this patch and put 
> uclk handling
> in "#ifdef CONFIG_COMMON_CLK #endif" sections.
> 
> Would this be acceptable ?

Yes.  However, a more elegant approach is to protect the relevant 
statements with: if (defined(CONFIG_COMMON_CLK)).  This preprocessor 
evaluates this to a 0 or 1, so the compiler will not generate output 
for the protected source code if the symbol isn't defined.

Alan Stern

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] USB: ohci-at91: add usb_clk for transition to common clk framework
  2013-08-01 15:56     ` Alan Stern
@ 2013-08-01 17:30       ` Sergei Shtylyov
  2013-08-01 17:38         ` Alan Stern
  0 siblings, 1 reply; 6+ messages in thread
From: Sergei Shtylyov @ 2013-08-01 17:30 UTC (permalink / raw)
  To: linux-arm-kernel

Hello.

On 08/01/2013 07:56 PM, Alan Stern wrote:

>>> What happens if uclk isn't found but it is needed?

>> It will not fail but I guess the usb host won't work as the clock won't
>> be correctly
>> configured.

>> I agree with you: this is not a good solution.

>> Another option is to keep the previous version of this patch and put
>> uclk handling
>> in "#ifdef CONFIG_COMMON_CLK #endif" sections.

>> Would this be acceptable ?

> Yes.  However, a more elegant approach is to protect the relevant
> statements with: if (defined(CONFIG_COMMON_CLK)).  This preprocessor

    You probably meant IS_ENABLED() or IS_BUIILTIN()? Or is there something I 
don't know about gcc preprocessor?

> evaluates this to a 0 or 1, so the compiler will not generate output
> for the protected source code if the symbol isn't defined.

> Alan Stern

WBR, Sergei

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] USB: ohci-at91: add usb_clk for transition to common clk framework
  2013-08-01 17:30       ` Sergei Shtylyov
@ 2013-08-01 17:38         ` Alan Stern
  0 siblings, 0 replies; 6+ messages in thread
From: Alan Stern @ 2013-08-01 17:38 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, 1 Aug 2013, Sergei Shtylyov wrote:

> > Yes.  However, a more elegant approach is to protect the relevant
> > statements with: if (defined(CONFIG_COMMON_CLK)).  This preprocessor
> 
>     You probably meant IS_ENABLED() or IS_BUIILTIN()? Or is there something I 
> don't know about gcc preprocessor?

Yes, sorry, I meant IS_ENABLED().  Boris got it right.

Alan Stern

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2013-08-01 17:38 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-08-01  6:16 [PATCH] USB: ohci-at91: add usb_clk for transition to common clk framework Boris BREZILLON
2013-08-01 14:39 ` Alan Stern
2013-08-01 15:14   ` boris brezillon
2013-08-01 15:56     ` Alan Stern
2013-08-01 17:30       ` Sergei Shtylyov
2013-08-01 17:38         ` Alan Stern

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox