[PATCH 0/3] MIPS: TXx9: Common Clock Framework Conversion

[PATCH 0/3] MIPS: TXx9: Common Clock Framework Conversion

[Geert Uytterhoeven]

	Hi Ralf, Nemoto-san, Mark, Wim, Günter,

This patch series converts the Toshiba TXx9 platforms from their own
custom minimal clock implementation to the Common Clock Framework.

  - Patches 1 and 2 add missing clock (un)prepare calls to TXx9-specific
    drivers,
  - Patch 3 replaces the custom clock implementation by a CCF-based one,
    providing a minimal set of clocks.

Patches 1 and 2 can be applied independently.
Patch 3 has a hard dependency on patches 1 and 2.
I don't know how you prefer to handle this?

This has been tested with the watchdog on RBTX4927.

Thanks!

Geert Uytterhoeven (3):
  spi: spi-txx9: Add missing clock (un)prepare calls for CCF
  watchdog: txx9wdt: Add missing clock (un)prepare calls for CCF
  MIPS: TXx9: Convert to Common Clock Framework

 arch/mips/txx9/Kconfig         |  2 +-
 arch/mips/txx9/generic/setup.c | 68 ++++++++++++++++++++----------------------
 drivers/spi/spi-txx9.c         |  6 ++--
 drivers/watchdog/txx9wdt.c     |  6 ++--
 4 files changed, 40 insertions(+), 42 deletions(-)

-- 
1.9.1

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

[PATCH 1/3] spi: spi-txx9: Add missing clock (un)prepare calls for CCF

[Geert Uytterhoeven]

While the custom minimal TXx9 clock implementation doesn't need or use
clock (un)prepare calls (they are dummies if !CONFIG_HAVE_CLK_PREPARE),
they are mandatory when using the Common Clock Framework.

Hence add them, to prepare for the advent of CCF.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
Untested due to lack of hardware.
---
 drivers/spi/spi-txx9.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/spi-txx9.c b/drivers/spi/spi-txx9.c
index 7492ea346b43ed5b..51759d3fd45f6b06 100644
--- a/drivers/spi/spi-txx9.c
+++ b/drivers/spi/spi-txx9.c
@@ -346,7 +346,7 @@ static int txx9spi_probe(struct platform_device *dev)
 		c->clk = NULL;
 		goto exit;
 	}
-	ret = clk_enable(c->clk);
+	ret = clk_prepare_enable(c->clk);
 	if (ret) {
 		c->clk = NULL;
 		goto exit;
@@ -395,7 +395,7 @@ static int txx9spi_probe(struct platform_device *dev)
 exit_busy:
 	ret = -EBUSY;
 exit:
-	clk_disable(c->clk);
+	clk_disable_unprepare(c->clk);
 	spi_master_put(master);
 	return ret;
 }
@@ -406,7 +406,7 @@ static int txx9spi_remove(struct platform_device *dev)
 	struct txx9spi *c = spi_master_get_devdata(master);
 
 	flush_work(&c->work);
-	clk_disable(c->clk);
+	clk_disable_unprepare(c->clk);
 	return 0;
 }
 
-- 
1.9.1

[PATCH 2/3] watchdog: txx9wdt: Add missing clock (un)prepare calls for CCF

[Geert Uytterhoeven]

While the custom minimal TXx9 clock implementation doesn't need or use
clock (un)prepare calls (they are dummies if !CONFIG_HAVE_CLK_PREPARE),
they are mandatory when using the Common Clock Framework.

Hence add them, to prepare for the advent of CCF.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
Tested on RBTX4927.
---
 drivers/watchdog/txx9wdt.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/watchdog/txx9wdt.c b/drivers/watchdog/txx9wdt.c
index c2da880292bc2f32..6f7a9deb27d05d25 100644
--- a/drivers/watchdog/txx9wdt.c
+++ b/drivers/watchdog/txx9wdt.c
@@ -112,7 +112,7 @@ static int __init txx9wdt_probe(struct platform_device *dev)
 		txx9_imclk = NULL;
 		goto exit;
 	}
-	ret = clk_enable(txx9_imclk);
+	ret = clk_prepare_enable(txx9_imclk);
 	if (ret) {
 		clk_put(txx9_imclk);
 		txx9_imclk = NULL;
@@ -144,7 +144,7 @@ static int __init txx9wdt_probe(struct platform_device *dev)
 	return 0;
 exit:
 	if (txx9_imclk) {
-		clk_disable(txx9_imclk);
+		clk_disable_unprepare(txx9_imclk);
 		clk_put(txx9_imclk);
 	}
 	return ret;
@@ -153,7 +153,7 @@ exit:
 static int __exit txx9wdt_remove(struct platform_device *dev)
 {
 	watchdog_unregister_device(&txx9wdt);
-	clk_disable(txx9_imclk);
+	clk_disable_unprepare(txx9_imclk);
 	clk_put(txx9_imclk);
 	return 0;
 }
-- 
1.9.1

Re: [PATCH 2/3] watchdog: txx9wdt: Add missing clock (un)prepare calls for CCF

[Guenter Roeck]

On Thu, Aug 18, 2016 at 07:34:26PM +0200, Geert Uytterhoeven wrote:
> While the custom minimal TXx9 clock implementation doesn't need or use
> clock (un)prepare calls (they are dummies if !CONFIG_HAVE_CLK_PREPARE),
> they are mandatory when using the Common Clock Framework.
> 
> Hence add them, to prepare for the advent of CCF.
> 
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
> Tested on RBTX4927.
> ---
>  drivers/watchdog/txx9wdt.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/watchdog/txx9wdt.c b/drivers/watchdog/txx9wdt.c
> index c2da880292bc2f32..6f7a9deb27d05d25 100644
> --- a/drivers/watchdog/txx9wdt.c
> +++ b/drivers/watchdog/txx9wdt.c
> @@ -112,7 +112,7 @@ static int __init txx9wdt_probe(struct platform_device *dev)
>  		txx9_imclk = NULL;
>  		goto exit;
>  	}
> -	ret = clk_enable(txx9_imclk);
> +	ret = clk_prepare_enable(txx9_imclk);
>  	if (ret) {
>  		clk_put(txx9_imclk);
>  		txx9_imclk = NULL;
> @@ -144,7 +144,7 @@ static int __init txx9wdt_probe(struct platform_device *dev)
>  	return 0;
>  exit:
>  	if (txx9_imclk) {
> -		clk_disable(txx9_imclk);
> +		clk_disable_unprepare(txx9_imclk);
>  		clk_put(txx9_imclk);
>  	}
>  	return ret;
> @@ -153,7 +153,7 @@ exit:
>  static int __exit txx9wdt_remove(struct platform_device *dev)
>  {
>  	watchdog_unregister_device(&txx9wdt);
> -	clk_disable(txx9_imclk);
> +	clk_disable_unprepare(txx9_imclk);
>  	clk_put(txx9_imclk);
>  	return 0;
>  }
> -- 
> 1.9.1
> 

Re: [PATCH 2/3] watchdog: txx9wdt: Add missing clock (un)prepare calls for CCF

[Guenter Roeck]

On 08/18/2016 10:34 AM, Geert Uytterhoeven wrote:
> While the custom minimal TXx9 clock implementation doesn't need or use
> clock (un)prepare calls (they are dummies if !CONFIG_HAVE_CLK_PREPARE),
> they are mandatory when using the Common Clock Framework.
>
> Hence add them, to prepare for the advent of CCF.
>
> Signed-off-by: Geert Uytterhoeven <geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org>

Reviewed-by: Guenter Roeck <linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>

> ---
> Tested on RBTX4927.
> ---
>  drivers/watchdog/txx9wdt.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/watchdog/txx9wdt.c b/drivers/watchdog/txx9wdt.c
> index c2da880292bc2f32..6f7a9deb27d05d25 100644
> --- a/drivers/watchdog/txx9wdt.c
> +++ b/drivers/watchdog/txx9wdt.c
> @@ -112,7 +112,7 @@ static int __init txx9wdt_probe(struct platform_device *dev)
>  		txx9_imclk = NULL;
>  		goto exit;
>  	}
> -	ret = clk_enable(txx9_imclk);
> +	ret = clk_prepare_enable(txx9_imclk);
>  	if (ret) {
>  		clk_put(txx9_imclk);
>  		txx9_imclk = NULL;
> @@ -144,7 +144,7 @@ static int __init txx9wdt_probe(struct platform_device *dev)
>  	return 0;
>  exit:
>  	if (txx9_imclk) {
> -		clk_disable(txx9_imclk);
> +		clk_disable_unprepare(txx9_imclk);
>  		clk_put(txx9_imclk);
>  	}
>  	return ret;
> @@ -153,7 +153,7 @@ exit:
>  static int __exit txx9wdt_remove(struct platform_device *dev)
>  {
>  	watchdog_unregister_device(&txx9wdt);
> -	clk_disable(txx9_imclk);
> +	clk_disable_unprepare(txx9_imclk);
>  	clk_put(txx9_imclk);
>  	return 0;
>  }
>

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/3] MIPS: TXx9: Convert to Common Clock Framework

[Geert Uytterhoeven]

Replace the custom minimal clock implementation for Toshiba TXx9 by a
basic implementation using the Common Clock Framework.

The only clocks that are provided are those needed by TXx9-specific
drivers ("imbus" and "spi" (TX4938 only)), and their common parent
clock "gbus". Other clocks can be added when needed.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
Tested on RBTX4927.
---
 arch/mips/txx9/Kconfig         |  2 +-
 arch/mips/txx9/generic/setup.c | 68 ++++++++++++++++++++----------------------
 2 files changed, 34 insertions(+), 36 deletions(-)

diff --git a/arch/mips/txx9/Kconfig b/arch/mips/txx9/Kconfig
index 8c337d60f790db9f..42923478d45ca363 100644
--- a/arch/mips/txx9/Kconfig
+++ b/arch/mips/txx9/Kconfig
@@ -20,7 +20,7 @@ config MACH_TXX9
 	select SYS_SUPPORTS_32BIT_KERNEL
 	select SYS_SUPPORTS_LITTLE_ENDIAN
 	select SYS_SUPPORTS_BIG_ENDIAN
-	select HAVE_CLK
+	select COMMON_CLK
 
 config TOSHIBA_JMR3927
 	bool "Toshiba JMR-TX3927 board"
diff --git a/arch/mips/txx9/generic/setup.c b/arch/mips/txx9/generic/setup.c
index ada92db92f87d91a..2fdbcf91b2cc472c 100644
--- a/arch/mips/txx9/generic/setup.c
+++ b/arch/mips/txx9/generic/setup.c
@@ -15,7 +15,8 @@
 #include <linux/interrupt.h>
 #include <linux/string.h>
 #include <linux/module.h>
-#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/clkdev.h>
 #include <linux/err.h>
 #include <linux/gpio/driver.h>
 #include <linux/platform_device.h>
@@ -83,40 +84,6 @@ int txx9_ccfg_toeon __initdata;
 int txx9_ccfg_toeon __initdata = 1;
 #endif
 
-/* Minimum CLK support */
-
-struct clk *clk_get(struct device *dev, const char *id)
-{
-	if (!strcmp(id, "spi-baseclk"))
-		return (struct clk *)((unsigned long)txx9_gbus_clock / 2 / 2);
-	if (!strcmp(id, "imbus_clk"))
-		return (struct clk *)((unsigned long)txx9_gbus_clock / 2);
-	return ERR_PTR(-ENOENT);
-}
-EXPORT_SYMBOL(clk_get);
-
-int clk_enable(struct clk *clk)
-{
-	return 0;
-}
-EXPORT_SYMBOL(clk_enable);
-
-void clk_disable(struct clk *clk)
-{
-}
-EXPORT_SYMBOL(clk_disable);
-
-unsigned long clk_get_rate(struct clk *clk)
-{
-	return (unsigned long)clk;
-}
-EXPORT_SYMBOL(clk_get_rate);
-
-void clk_put(struct clk *clk)
-{
-}
-EXPORT_SYMBOL(clk_put);
-
 #define BOARD_VEC(board)	extern struct txx9_board_vec board;
 #include <asm/txx9/boards.h>
 #undef BOARD_VEC
@@ -560,8 +527,39 @@ void __init plat_time_init(void)
 	txx9_board_vec->time_init();
 }
 
+static void txx9_clk_init(void)
+{
+	struct clk *clk;
+	int error;
+
+	clk = clk_register_fixed_rate(NULL, "gbus", NULL, 0, txx9_gbus_clock);
+	if (IS_ERR(clk)) {
+		error = PTR_ERR(clk);
+		goto fail;
+	}
+
+	clk = clk_register_fixed_factor(NULL, "imbus", "gbus", 0, 1, 2);
+	error = clk_register_clkdev(clk, "imbus_clk", NULL);
+	if (error)
+		goto fail;
+
+	if (TX4938_REV_PCODE() == 0x4938) {
+		clk = clk_register_fixed_factor(NULL, "spi", "gbus", 0, 1, 4);
+		error = clk_register_clkdev(clk, "spi-baseclk", NULL);
+		if (error)
+			goto fail;
+	}
+
+	return;
+
+fail:
+	pr_err("Failed to register clocks: %d\n", error);
+}
+
 static int __init _txx9_arch_init(void)
 {
+	txx9_clk_init();
+
 	if (txx9_board_vec->arch_init)
 		txx9_board_vec->arch_init();
 	return 0;
-- 
1.9.1

Re: [PATCH 3/3] MIPS: TXx9: Convert to Common Clock Framework

[Atsushi Nemoto]

On Thu, 18 Aug 2016 19:34:27 +0200, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> Replace the custom minimal clock implementation for Toshiba TXx9 by a
> basic implementation using the Common Clock Framework.

Thank you for good cleanup.

> +	if (TX4938_REV_PCODE() == 0x4938) {
> +		clk = clk_register_fixed_factor(NULL, "spi", "gbus", 0, 1, 4);
> +		error = clk_register_clkdev(clk, "spi-baseclk", NULL);
> +		if (error)
> +			goto fail;
> +	}

Unfortunately, TX4938_REV_PCODE() can be used only on TX49 based
platforms.  Please enclose this block with #ifdef CONFIG_CPU_TX49XX.

Or, while registering unused clkdev will not bloat kernel so much,
just remove this TX4938 checking.

---
Atsushi Nemoto

Re: [PATCH 3/3] MIPS: TXx9: Convert to Common Clock Framework

[Stephen Boyd]

On 08/18, Geert Uytterhoeven wrote:
> diff --git a/arch/mips/txx9/generic/setup.c b/arch/mips/txx9/generic/setup.c
> index ada92db92f87d91a..2fdbcf91b2cc472c 100644
> --- a/arch/mips/txx9/generic/setup.c
> +++ b/arch/mips/txx9/generic/setup.c
> @@ -560,8 +527,39 @@ void __init plat_time_init(void)
>  	txx9_board_vec->time_init();
>  }
>  
> +static void txx9_clk_init(void)
> +{
> +	struct clk *clk;
> +	int error;
> +
> +	clk = clk_register_fixed_rate(NULL, "gbus", NULL, 0, txx9_gbus_clock);

Can we use the clk_hw_*() based variants instead please?

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] MIPS: TXx9: Convert to Common Clock Framework

[Geert Uytterhoeven]

Hi Stephen,

On Fri, Aug 19, 2016 at 9:17 PM, Stephen Boyd <sboyd-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> wrote:
> On 08/18, Geert Uytterhoeven wrote:
>> diff --git a/arch/mips/txx9/generic/setup.c b/arch/mips/txx9/generic/setup.c
>> index ada92db92f87d91a..2fdbcf91b2cc472c 100644
>> --- a/arch/mips/txx9/generic/setup.c
>> +++ b/arch/mips/txx9/generic/setup.c
>> @@ -560,8 +527,39 @@ void __init plat_time_init(void)
>>       txx9_board_vec->time_init();
>>  }
>>
>> +static void txx9_clk_init(void)
>> +{
>> +     struct clk *clk;
>> +     int error;
>> +
>> +     clk = clk_register_fixed_rate(NULL, "gbus", NULL, 0, txx9_gbus_clock);
>
> Can we use the clk_hw_*() based variants instead please?

Yes we can.

BTW, is it intentional that clk_hw_register_clkdev() doesn't detect errors
from a previous registration call, like clk_register_clkdev() does?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] MIPS: TXx9: Convert to Common Clock Framework

[Stephen Boyd]

On 08/22, Geert Uytterhoeven wrote:
> Hi Stephen,
> 
> On Fri, Aug 19, 2016 at 9:17 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> > On 08/18, Geert Uytterhoeven wrote:
> >> diff --git a/arch/mips/txx9/generic/setup.c b/arch/mips/txx9/generic/setup.c
> >> index ada92db92f87d91a..2fdbcf91b2cc472c 100644
> >> --- a/arch/mips/txx9/generic/setup.c
> >> +++ b/arch/mips/txx9/generic/setup.c
> >> @@ -560,8 +527,39 @@ void __init plat_time_init(void)
> >>       txx9_board_vec->time_init();
> >>  }
> >>
> >> +static void txx9_clk_init(void)
> >> +{
> >> +     struct clk *clk;
> >> +     int error;
> >> +
> >> +     clk = clk_register_fixed_rate(NULL, "gbus", NULL, 0, txx9_gbus_clock);
> >
> > Can we use the clk_hw_*() based variants instead please?
> 
> Yes we can.
> 
> BTW, is it intentional that clk_hw_register_clkdev() doesn't detect errors
> from a previous registration call, like clk_register_clkdev() does?
> 

Slightly intentional, because the assumption is providers already
have a clk_hw structure that they've created themselves when they
register a clkdev, whereas a struct clk is more likely to be an
error pointer because of clk_get(), etc. But I suppose we can add
the error check to ease registration in cases where providers are
using the basic types.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH 0/3] MIPS: TXx9: Common Clock Framework Conversion

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 282 bytes --]

On Thu, Aug 18, 2016 at 07:34:24PM +0200, Geert Uytterhoeven wrote:

> Patches 1 and 2 can be applied independently.
> Patch 3 has a hard dependency on patches 1 and 2.
> I don't know how you prefer to handle this?

I can sign a tag for pulling in easily enough for the SPI change.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]