diff mbox

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

Message ID 1471541667-30689-4-git-send-email-geert@linux-m68k.org (mailing list archive)
State New, archived
Headers show

Commit Message

Geert Uytterhoeven Aug. 18, 2016, 5:34 p.m. UTC
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(-)

Comments

Atsushi Nemoto Aug. 19, 2016, 12:05 p.m. UTC | #1
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
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stephen Boyd Aug. 19, 2016, 7:17 p.m. UTC | #2
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?
Geert Uytterhoeven Aug. 22, 2016, 8:46 a.m. UTC | #3
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?

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
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stephen Boyd Aug. 22, 2016, 5:37 p.m. UTC | #4
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.
diff mbox

Patch

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;