[PATCH v6 1/2] ARM: davinci: Export two clocks function

[PATCH v6 1/2] ARM: davinci: Export two clocks function

[Alexandre Bailon]

Rename and export __clk_enable() and __clk_disable() in order
to use them from usb-da8xx.c. This file implements the usb20 phy clock
that must be able to enable or disable usb20 clock.
To prevent a recurssive call to clk_enable() that would cause a recursive
locking issue, we must use __clk_enable() and __clk_disable().
Rename these methods in davinci_clk_enable() and davinci_clk_disable(),
and export them.

Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
Suggested-by: David Lechner <david@lechnology.com>
---
 arch/arm/mach-davinci/clock.c | 14 ++++++++------
 arch/arm/mach-davinci/clock.h |  2 ++
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/arch/arm/mach-davinci/clock.c b/arch/arm/mach-davinci/clock.c
index df42c93..0f967c3 100644
--- a/arch/arm/mach-davinci/clock.c
+++ b/arch/arm/mach-davinci/clock.c
@@ -31,10 +31,10 @@ static LIST_HEAD(clocks);
 static DEFINE_MUTEX(clocks_mutex);
 static DEFINE_SPINLOCK(clockfw_lock);
 
-static void __clk_enable(struct clk *clk)
+void davinci_clk_enable(struct clk *clk)
 {
 	if (clk->parent)
-		__clk_enable(clk->parent);
+		davinci_clk_enable(clk->parent);
 	if (clk->usecount++ == 0) {
 		if (clk->flags & CLK_PSC)
 			davinci_psc_config(clk->domain, clk->gpsc, clk->lpsc,
@@ -43,8 +43,9 @@ static void __clk_enable(struct clk *clk)
 			clk->clk_enable(clk);
 	}
 }
+EXPORT_SYMBOL(davinci_clk_enable);
 
-static void __clk_disable(struct clk *clk)
+void davinci_clk_disable(struct clk *clk)
 {
 	if (WARN_ON(clk->usecount == 0))
 		return;
@@ -56,8 +57,9 @@ static void __clk_disable(struct clk *clk)
 			clk->clk_disable(clk);
 	}
 	if (clk->parent)
-		__clk_disable(clk->parent);
+		davinci_clk_disable(clk->parent);
 }
+EXPORT_SYMBOL(davinci_clk_disable);
 
 int davinci_clk_reset(struct clk *clk, bool reset)
 {
@@ -103,7 +105,7 @@ int clk_enable(struct clk *clk)
 		return -EINVAL;
 
 	spin_lock_irqsave(&clockfw_lock, flags);
-	__clk_enable(clk);
+	davinci_clk_enable(clk);
 	spin_unlock_irqrestore(&clockfw_lock, flags);
 
 	return 0;
@@ -118,7 +120,7 @@ void clk_disable(struct clk *clk)
 		return;
 
 	spin_lock_irqsave(&clockfw_lock, flags);
-	__clk_disable(clk);
+	davinci_clk_disable(clk);
 	spin_unlock_irqrestore(&clockfw_lock, flags);
 }
 EXPORT_SYMBOL(clk_disable);
diff --git a/arch/arm/mach-davinci/clock.h b/arch/arm/mach-davinci/clock.h
index e2a5437..fa2b837 100644
--- a/arch/arm/mach-davinci/clock.h
+++ b/arch/arm/mach-davinci/clock.h
@@ -132,6 +132,8 @@ int davinci_set_sysclk_rate(struct clk *clk, unsigned long rate);
 int davinci_set_refclk_rate(unsigned long rate);
 int davinci_simple_set_rate(struct clk *clk, unsigned long rate);
 int davinci_clk_reset(struct clk *clk, bool reset);
+void davinci_clk_enable(struct clk *clk);
+void davinci_clk_disable(struct clk *clk);
 
 extern struct platform_device davinci_wdt_device;
 extern void davinci_watchdog_reset(struct platform_device *);
-- 
2.7.3

[PATCH v6 2/2] ARM: davinci: da8xx: Fix sleeping function called from invalid context

[Alexandre Bailon]

Everytime the usb20 phy is enabled, there is a
"sleeping function called from invalid context" BUG.
In addition, there is a recursive locking happening
because of the recurse call to clk_enable().

clk_enable() from arch/arm/mach-davinci/clock.c uses spin_lock_irqsave()
before to invoke the callback usb20_phy_clk_enable().
usb20_phy_clk_enable() uses clk_get() and clk_enable_prepapre()
which may sleep.
replace clk_prepare_enable() by davinci_clk_enable().

Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
Suggested-by: David Lechner <david@lechnology.com>
---
 arch/arm/mach-davinci/usb-da8xx.c | 33 +++++++++++++++------------------
 1 file changed, 15 insertions(+), 18 deletions(-)

diff --git a/arch/arm/mach-davinci/usb-da8xx.c b/arch/arm/mach-davinci/usb-da8xx.c
index c6feecf..9edd2d8 100644
--- a/arch/arm/mach-davinci/usb-da8xx.c
+++ b/arch/arm/mach-davinci/usb-da8xx.c
@@ -22,6 +22,8 @@
 #define DA8XX_USB0_BASE		0x01e00000
 #define DA8XX_USB1_BASE		0x01e25000
 
+static struct clk *usb20_clk;
+
 static struct platform_device da8xx_usb_phy = {
 	.name		= "da8xx-usb-phy",
 	.id		= -1,
@@ -158,26 +160,14 @@ int __init da8xx_register_usb_refclkin(int rate)
 
 static void usb20_phy_clk_enable(struct clk *clk)
 {
-	struct clk *usb20_clk;
 	int err;
 	u32 val;
 	u32 timeout = 500000; /* 500 msec */
 
 	val = readl(DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG));
 
-	usb20_clk = clk_get(&da8xx_usb20_dev.dev, "usb20");
-	if (IS_ERR(usb20_clk)) {
-		pr_err("could not get usb20 clk: %ld\n", PTR_ERR(usb20_clk));
-		return;
-	}
-
 	/* The USB 2.O PLL requires that the USB 2.O PSC is enabled as well. */
-	err = clk_prepare_enable(usb20_clk);
-	if (err) {
-		pr_err("failed to enable usb20 clk: %d\n", err);
-		clk_put(usb20_clk);
-		return;
-	}
+	davinci_clk_enable(usb20_clk);
 
 	/*
 	 * Turn on the USB 2.0 PHY, but just the PLL, and not OTG. The USB 1.1
@@ -197,8 +187,7 @@ static void usb20_phy_clk_enable(struct clk *clk)
 
 	pr_err("Timeout waiting for USB 2.0 PHY clock good\n");
 done:
-	clk_disable_unprepare(usb20_clk);
-	clk_put(usb20_clk);
+	davinci_clk_disable(usb20_clk);
 }
 
 static void usb20_phy_clk_disable(struct clk *clk)
@@ -285,11 +274,19 @@ static struct clk_lookup usb20_phy_clk_lookup =
 int __init da8xx_register_usb20_phy_clk(bool use_usb_refclkin)
 {
 	struct clk *parent;
-	int ret = 0;
+	int ret;
+
+	usb20_clk = clk_get(&da8xx_usb20_dev.dev, "usb20");
+	ret = PTR_ERR_OR_ZERO(usb20_clk);
+	if (ret)
+		return ret;
 
 	parent = clk_get(NULL, use_usb_refclkin ? "usb_refclkin" : "pll0_aux");
-	if (IS_ERR(parent))
-		return PTR_ERR(parent);
+	ret = PTR_ERR_OR_ZERO(parent);
+	if (ret) {
+		clk_put(usb20_clk);
+		return ret;
+	}
 
 	usb20_phy_clk.parent = parent;
 	ret = clk_register(&usb20_phy_clk);
-- 
2.7.3

[PATCH v6 1/2] ARM: davinci: Export two clocks function

[Sekhar Nori]

On Wednesday 07 December 2016 10:39 PM, Alexandre Bailon wrote:
> Rename and export __clk_enable() and __clk_disable() in order
> to use them from usb-da8xx.c. This file implements the usb20 phy clock
> that must be able to enable or disable usb20 clock.
> To prevent a recurssive call to clk_enable() that would cause a recursive
> locking issue, we must use __clk_enable() and __clk_disable().
> Rename these methods in davinci_clk_enable() and davinci_clk_disable(),
> and export them.
> 
> Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
> Suggested-by: David Lechner <david@lechnology.com>
> ---
>  arch/arm/mach-davinci/clock.c | 14 ++++++++------
>  arch/arm/mach-davinci/clock.h |  2 ++
>  2 files changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm/mach-davinci/clock.c b/arch/arm/mach-davinci/clock.c
> index df42c93..0f967c3 100644
> --- a/arch/arm/mach-davinci/clock.c
> +++ b/arch/arm/mach-davinci/clock.c
> @@ -31,10 +31,10 @@ static LIST_HEAD(clocks);
>  static DEFINE_MUTEX(clocks_mutex);
>  static DEFINE_SPINLOCK(clockfw_lock);
>  
> -static void __clk_enable(struct clk *clk)
> +void davinci_clk_enable(struct clk *clk)
>  {
>  	if (clk->parent)
> -		__clk_enable(clk->parent);
> +		davinci_clk_enable(clk->parent);
>  	if (clk->usecount++ == 0) {
>  		if (clk->flags & CLK_PSC)
>  			davinci_psc_config(clk->domain, clk->gpsc, clk->lpsc,
> @@ -43,8 +43,9 @@ static void __clk_enable(struct clk *clk)
>  			clk->clk_enable(clk);
>  	}
>  }
> +EXPORT_SYMBOL(davinci_clk_enable);

We don't want to export these as we dont want drivers to use this API.
This is to be used within mach-davinci only.

Thanks,
Sekhar

[PATCH v6 1/2] ARM: davinci: Export two clocks function

[Sekhar Nori]

On Thursday 08 December 2016 06:02 PM, Sekhar Nori wrote:
> On Wednesday 07 December 2016 10:39 PM, Alexandre Bailon wrote:
>> Rename and export __clk_enable() and __clk_disable() in order
>> to use them from usb-da8xx.c. This file implements the usb20 phy clock
>> that must be able to enable or disable usb20 clock.
>> To prevent a recurssive call to clk_enable() that would cause a recursive
>> locking issue, we must use __clk_enable() and __clk_disable().
>> Rename these methods in davinci_clk_enable() and davinci_clk_disable(),
>> and export them.
>>
>> Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
>> Suggested-by: David Lechner <david@lechnology.com>
>> ---
>>  arch/arm/mach-davinci/clock.c | 14 ++++++++------
>>  arch/arm/mach-davinci/clock.h |  2 ++
>>  2 files changed, 10 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/arm/mach-davinci/clock.c b/arch/arm/mach-davinci/clock.c
>> index df42c93..0f967c3 100644
>> --- a/arch/arm/mach-davinci/clock.c
>> +++ b/arch/arm/mach-davinci/clock.c
>> @@ -31,10 +31,10 @@ static LIST_HEAD(clocks);
>>  static DEFINE_MUTEX(clocks_mutex);
>>  static DEFINE_SPINLOCK(clockfw_lock);
>>  
>> -static void __clk_enable(struct clk *clk)
>> +void davinci_clk_enable(struct clk *clk)
>>  {
>>  	if (clk->parent)
>> -		__clk_enable(clk->parent);
>> +		davinci_clk_enable(clk->parent);
>>  	if (clk->usecount++ == 0) {
>>  		if (clk->flags & CLK_PSC)
>>  			davinci_psc_config(clk->domain, clk->gpsc, clk->lpsc,
>> @@ -43,8 +43,9 @@ static void __clk_enable(struct clk *clk)
>>  			clk->clk_enable(clk);
>>  	}
>>  }
>> +EXPORT_SYMBOL(davinci_clk_enable);
> 
> We don't want to export these as we dont want drivers to use this API.
> This is to be used within mach-davinci only.

Also, subject line is pretty vague. What about:

ARM: davinci: provide lock-less versions of clk_{enable|disable}

In the description too, please talk about on the main difference between
davinci_clk_{enable|disable}() (lockless) vs clk_enable() (locked). Use
USB case only as an example. Using them in usb-da8xx.c might be the
current motivation but this addresses similar needs in future too.

Thanks,
Sekhar