All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 04/11] ARM: shmobile: r8a7778: common clock framework CPG driver
@ 2015-01-21 16:20 Ulrich Hecht
  2015-01-21 23:37 ` Laurent Pinchart
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Ulrich Hecht @ 2015-01-21 16:20 UTC (permalink / raw)
  To: linux-sh

Driver for the r8a7778's clocks that depend on the mode bits.

Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
---
 .../bindings/clock/renesas,r8a7778-cpg-clocks.txt  |  24 ++++
 arch/arm/mach-shmobile/board-bockw-reference.c     |   2 +
 arch/arm/mach-shmobile/setup-r8a7778.c             |  19 +++
 drivers/clk/shmobile/Makefile                      |   1 +
 drivers/clk/shmobile/clk-r8a7778.c                 | 146 +++++++++++++++++++++
 include/linux/clk/shmobile.h                       |   1 +
 6 files changed, 193 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/renesas,r8a7778-cpg-clocks.txt
 create mode 100644 drivers/clk/shmobile/clk-r8a7778.c

diff --git a/Documentation/devicetree/bindings/clock/renesas,r8a7778-cpg-clocks.txt b/Documentation/devicetree/bindings/clock/renesas,r8a7778-cpg-clocks.txt
new file mode 100644
index 0000000..c9a9544
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/renesas,r8a7778-cpg-clocks.txt
@@ -0,0 +1,24 @@
+* Renesas R8A7778 Clock Pulse Generator (CPG)
+
+The CPG generates core clocks for the R8A7778. It includes two PLLs and
+several fixed ratio dividers
+
+Required Properties:
+
+  - compatible: Must be "renesas,r8a7778-cpg-clocks"
+  - reg: Base address and length of the memory resource used by the CPG
+  - #clock-cells: Must be 1
+  - clock-output-names: The names of the clocks. Supported clocks are
+    "extal", "plla", "pllb", "b", "out", "p", "s", and "s1".
+
+
+Example
+-------
+
+	cpg_clocks: cpg_clocks@ffc80000 {
+		compatible = "renesas,r8a7778-cpg-clocks";
+		reg = <0xffc80000 0x80>;
+		#clock-cells = <1>;
+		clock-output-names = "extal", "plla", "pllb", "b",
+				     "out", "p", "s", "s1";
+	};
diff --git a/arch/arm/mach-shmobile/board-bockw-reference.c b/arch/arm/mach-shmobile/board-bockw-reference.c
index d649ade..9a74efd 100644
--- a/arch/arm/mach-shmobile/board-bockw-reference.c
+++ b/arch/arm/mach-shmobile/board-bockw-reference.c
@@ -36,7 +36,9 @@ static void __init bockw_init(void)
 	void __iomem *fpga;
 	void __iomem *pfc;
 
+#ifndef CONFIG_COMMON_CLK
 	r8a7778_clock_init();
+#endif
 	r8a7778_init_irq_extpin_dt(1);
 	r8a7778_add_dt_devices();
 
diff --git a/arch/arm/mach-shmobile/setup-r8a7778.c b/arch/arm/mach-shmobile/setup-r8a7778.c
index cef8895..c49aa09 100644
--- a/arch/arm/mach-shmobile/setup-r8a7778.c
+++ b/arch/arm/mach-shmobile/setup-r8a7778.c
@@ -15,6 +15,7 @@
  * GNU General Public License for more details.
  */
 
+#include <linux/clk/shmobile.h>
 #include <linux/kernel.h>
 #include <linux/io.h>
 #include <linux/irqchip/arm-gic.h>
@@ -41,6 +42,21 @@
 #include "irqs.h"
 #include "r8a7778.h"
 
+#define MODEMR 0xffcc0020
+
+#ifdef CONFIG_COMMON_CLK
+static void __init r8a7778_timer_init(void)
+{
+	u32 mode;
+	void __iomem *modemr = ioremap_nocache(MODEMR, 4);
+
+	BUG_ON(!modemr);
+	mode = ioread32(modemr);
+	iounmap(modemr);
+	r8a7778_clocks_init(mode);
+}
+#endif
+
 /* SCIF */
 #define R8A7778_SCIF(index, baseaddr, irq)			\
 static struct plat_sci_port scif##index##_platform_data = {	\
@@ -608,6 +624,9 @@ DT_MACHINE_START(R8A7778_DT, "Generic R8A7778 (Flattened Device Tree)")
 	.init_early	= shmobile_init_delay,
 	.init_irq	= r8a7778_init_irq_dt,
 	.init_late	= shmobile_init_late,
+#ifdef CONFIG_COMMON_CLK
+	.init_time	= r8a7778_timer_init,
+#endif
 	.dt_compat	= r8a7778_compat_dt,
 MACHINE_END
 
diff --git a/drivers/clk/shmobile/Makefile b/drivers/clk/shmobile/Makefile
index 0689d7f..97c71c8 100644
--- a/drivers/clk/shmobile/Makefile
+++ b/drivers/clk/shmobile/Makefile
@@ -2,6 +2,7 @@ obj-$(CONFIG_ARCH_EMEV2)		+= clk-emev2.o
 obj-$(CONFIG_ARCH_R7S72100)		+= clk-rz.o
 obj-$(CONFIG_ARCH_R8A73A4)		+= clk-r8a73a4.o
 obj-$(CONFIG_ARCH_R8A7740)		+= clk-r8a7740.o
+obj-$(CONFIG_ARCH_R8A7778)		+= clk-r8a7778.o
 obj-$(CONFIG_ARCH_R8A7779)		+= clk-r8a7779.o
 obj-$(CONFIG_ARCH_R8A7790)		+= clk-rcar-gen2.o
 obj-$(CONFIG_ARCH_R8A7791)		+= clk-rcar-gen2.o
diff --git a/drivers/clk/shmobile/clk-r8a7778.c b/drivers/clk/shmobile/clk-r8a7778.c
new file mode 100644
index 0000000..30c5c14
--- /dev/null
+++ b/drivers/clk/shmobile/clk-r8a7778.c
@@ -0,0 +1,146 @@
+/*
+ * r8a7778 Core CPG Clocks
+ *
+ * Copyright (C) 2014  Ulrich Hecht
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/clkdev.h>
+#include <linux/clk/shmobile.h>
+#include <linux/of_address.h>
+
+struct r8a7778_cpg {
+	struct clk_onecell_data data;
+	spinlock_t lock;
+	void __iomem *reg;
+};
+
+/* EXTAL rate and PLL multipliers per bits 11, 12, and 18 of MODEMR */
+struct {
+	unsigned long extal_rate;
+	unsigned long plla_mult;
+	unsigned long pllb_mult;
+} r8a7778_rates[] __initdata = {
+	[0] = { 38000000, 21, 21},
+	[1] = { 33333333, 24, 24},
+	[2] = { 28500000, 28, 28},
+	[3] = { 25000000, 32, 32},
+	[5] = { 33333333, 24, 21},
+	[6] = { 28500000, 28, 21},
+	[7] = { 25000000, 32, 24},
+};
+
+/* Clock dividers per bits 1 and 2 of MODEMR */
+struct {
+	const char *name;
+	unsigned int div[4];
+} r8a7778_divs[6] __initdata = {
+	{ "b",   { 12, 12, 16, 18 } },
+	{ "out", { 12, 12, 16, 18 } },
+	{ "p",   { 16, 12, 16, 12 } },
+	{ "s",   { 4,  3,  4,  3  } },
+	{ "s1",  { 8,  6,  8,  6  } },
+	{ NULL },
+};
+
+static u32 cpg_mode_rates __initdata;
+static u32 cpg_mode_divs __initdata;
+
+static struct clk * __init
+r8a7778_cpg_register_clock(struct device_node *np, struct r8a7778_cpg *cpg,
+			     const char *name)
+{
+	if (!strcmp(name, "extal")) {
+		return clk_register_fixed_rate(NULL, "extal", NULL,
+			CLK_IS_ROOT, r8a7778_rates[cpg_mode_rates].extal_rate);
+	} else if (!strcmp(name, "plla")) {
+		return clk_register_fixed_factor(NULL, "plla", "extal", 0,
+			r8a7778_rates[cpg_mode_rates].plla_mult, 1);
+	} else if (!strcmp(name, "pllb")) {
+		return clk_register_fixed_factor(NULL, "pllb", "extal", 0,
+			r8a7778_rates[cpg_mode_rates].pllb_mult, 1);
+	} else {
+		unsigned int i;
+
+		for (i = 0; r8a7778_divs[i].name; i++) {
+			if (!strcmp(name, r8a7778_divs[i].name)) {
+				return clk_register_fixed_factor(NULL,
+					r8a7778_divs[i].name,
+					"plla", 0, 1,
+					r8a7778_divs[i].div[cpg_mode_divs]);
+			}
+		}
+	}
+
+	return ERR_PTR(-EINVAL);
+}
+
+
+static void __init r8a7778_cpg_clocks_init(struct device_node *np)
+{
+	struct r8a7778_cpg *cpg;
+	struct clk **clks;
+	unsigned int i;
+	int num_clks;
+
+	num_clks = of_property_count_strings(np, "clock-output-names");
+	if (num_clks < 0) {
+		pr_err("%s: failed to count clocks\n", __func__);
+		return;
+	}
+
+	cpg = kzalloc(sizeof(*cpg), GFP_KERNEL);
+	clks = kcalloc(num_clks, sizeof(*clks), GFP_KERNEL);
+	if (cpg = NULL || clks = NULL) {
+		/* We're leaking memory on purpose, there's no point in cleaning
+		 * up as the system won't boot anyway.
+		 */
+		return;
+	}
+
+	spin_lock_init(&cpg->lock);
+
+	cpg->data.clks = clks;
+	cpg->data.clk_num = num_clks;
+
+	cpg->reg = of_iomap(np, 0);
+	if (WARN_ON(cpg->reg = NULL))
+		return;
+
+	for (i = 0; i < num_clks; ++i) {
+		const char *name;
+		struct clk *clk;
+
+		of_property_read_string_index(np, "clock-output-names", i,
+					      &name);
+
+		clk = r8a7778_cpg_register_clock(np, cpg, name);
+		if (IS_ERR(clk))
+			pr_err("%s: failed to register %s %s clock (%ld)\n",
+			       __func__, np->name, name, PTR_ERR(clk));
+		else
+			cpg->data.clks[i] = clk;
+	}
+
+	of_clk_add_provider(np, of_clk_src_onecell_get, &cpg->data);
+}
+
+CLK_OF_DECLARE(r8a7778_cpg_clks, "renesas,r8a7778-cpg-clocks",
+	       r8a7778_cpg_clocks_init);
+
+void __init r8a7778_clocks_init(u32 mode)
+{
+	if (!(mode & BIT(19)))
+		BUG();
+	cpg_mode_rates = (!!(mode & BIT(18)) << 2) |
+			 (!!(mode & BIT(12)) << 1) |
+			 (!!(mode & BIT(11)));
+	cpg_mode_divs = (!!(mode & BIT(2)) << 1) |
+			(!!(mode & BIT(1)));
+
+	of_clk_init(NULL);
+}
diff --git a/include/linux/clk/shmobile.h b/include/linux/clk/shmobile.h
index 9f8a140..63a8159 100644
--- a/include/linux/clk/shmobile.h
+++ b/include/linux/clk/shmobile.h
@@ -16,6 +16,7 @@
 
 #include <linux/types.h>
 
+void r8a7778_clocks_init(u32 mode);
 void r8a7779_clocks_init(u32 mode);
 void rcar_gen2_clocks_init(u32 mode);
 
-- 
2.2.2


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

* Re: [PATCH 04/11] ARM: shmobile: r8a7778: common clock framework CPG driver
  2015-01-21 16:20 [PATCH 04/11] ARM: shmobile: r8a7778: common clock framework CPG driver Ulrich Hecht
@ 2015-01-21 23:37 ` Laurent Pinchart
  2015-01-23  9:12 ` Geert Uytterhoeven
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Laurent Pinchart @ 2015-01-21 23:37 UTC (permalink / raw)
  To: linux-sh

Hi Ulrich,

Thank you for the patch.

On Wednesday 21 January 2015 17:20:55 Ulrich Hecht wrote:
> Driver for the r8a7778's clocks that depend on the mode bits.
> 
> Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
> ---
>  .../bindings/clock/renesas,r8a7778-cpg-clocks.txt  |  24 ++++
>  arch/arm/mach-shmobile/board-bockw-reference.c     |   2 +
>  arch/arm/mach-shmobile/setup-r8a7778.c             |  19 +++
>  drivers/clk/shmobile/Makefile                      |   1 +
>  drivers/clk/shmobile/clk-r8a7778.c                 | 146 ++++++++++++++++++
>  include/linux/clk/shmobile.h                       |   1 +
>  6 files changed, 193 insertions(+)
>  create mode 100644
> Documentation/devicetree/bindings/clock/renesas,r8a7778-cpg-clocks.txt
> create mode 100644 drivers/clk/shmobile/clk-r8a7778.c

[snip]

> diff --git a/drivers/clk/shmobile/clk-r8a7778.c
> b/drivers/clk/shmobile/clk-r8a7778.c new file mode 100644
> index 0000000..30c5c14
> --- /dev/null
> +++ b/drivers/clk/shmobile/clk-r8a7778.c
> @@ -0,0 +1,146 @@
> +/*
> + * r8a7778 Core CPG Clocks
> + *
> + * Copyright (C) 2014  Ulrich Hecht
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; version 2 of the License.
> + */
> +
> +#include <linux/clk-provider.h>
> +#include <linux/clkdev.h>
> +#include <linux/clk/shmobile.h>
> +#include <linux/of_address.h>
> +
> +struct r8a7778_cpg {
> +	struct clk_onecell_data data;
> +	spinlock_t lock;
> +	void __iomem *reg;
> +};
> +
> +/* EXTAL rate and PLL multipliers per bits 11, 12, and 18 of MODEMR */
> +struct {
> +	unsigned long extal_rate;
> +	unsigned long plla_mult;
> +	unsigned long pllb_mult;
> +} r8a7778_rates[] __initdata = {
> +	[0] = { 38000000, 21, 21},
> +	[1] = { 33333333, 24, 24},
> +	[2] = { 28500000, 28, 28},
> +	[3] = { 25000000, 32, 32},
> +	[5] = { 33333333, 24, 21},
> +	[6] = { 28500000, 28, 21},
> +	[7] = { 25000000, 32, 24},
> +};
> +
> +/* Clock dividers per bits 1 and 2 of MODEMR */
> +struct {
> +	const char *name;
> +	unsigned int div[4];
> +} r8a7778_divs[6] __initdata = {
> +	{ "b",   { 12, 12, 16, 18 } },
> +	{ "out", { 12, 12, 16, 18 } },
> +	{ "p",   { 16, 12, 16, 12 } },
> +	{ "s",   { 4,  3,  4,  3  } },
> +	{ "s1",  { 8,  6,  8,  6  } },
> +	{ NULL },
> +};
> +
> +static u32 cpg_mode_rates __initdata;
> +static u32 cpg_mode_divs __initdata;
> +
> +static struct clk * __init
> +r8a7778_cpg_register_clock(struct device_node *np, struct r8a7778_cpg *cpg,
> +			     const char *name)
> +{
> +	if (!strcmp(name, "extal")) {
> +		return clk_register_fixed_rate(NULL, "extal", NULL,
> +			CLK_IS_ROOT, r8a7778_rates[cpg_mode_rates].extal_rate);

As the external clock is an input to the CPG, shouldn't it be specified by a 
separate fixed-frequency clock DT node, and referenced by the CPG using the 
clocks attributes ? You could then remove the extal_rate field from the 
r8a7778_rates array above, and possibly rename the array to r8a7778_pll_mults 
or similar.

> +	} else if (!strcmp(name, "plla")) {
> +		return clk_register_fixed_factor(NULL, "plla", "extal", 0,
> +			r8a7778_rates[cpg_mode_rates].plla_mult, 1);
> +	} else if (!strcmp(name, "pllb")) {
> +		return clk_register_fixed_factor(NULL, "pllb", "extal", 0,
> +			r8a7778_rates[cpg_mode_rates].pllb_mult, 1);
> +	} else {
> +		unsigned int i;
> +
> +		for (i = 0; r8a7778_divs[i].name; i++) {

You could use i < ARRAY_SIZE(r8a7778_divs) here and avoid the NULL entry at 
the end of the array.

> +			if (!strcmp(name, r8a7778_divs[i].name)) {
> +				return clk_register_fixed_factor(NULL,
> +					r8a7778_divs[i].name,
> +					"plla", 0, 1,
> +					r8a7778_divs[i].div[cpg_mode_divs]);
> +			}
> +		}
> +	}
> +
> +	return ERR_PTR(-EINVAL);
> +}
> +
> +
> +static void __init r8a7778_cpg_clocks_init(struct device_node *np)
> +{
> +	struct r8a7778_cpg *cpg;
> +	struct clk **clks;
> +	unsigned int i;
> +	int num_clks;
> +
> +	num_clks = of_property_count_strings(np, "clock-output-names");
> +	if (num_clks < 0) {
> +		pr_err("%s: failed to count clocks\n", __func__);
> +		return;
> +	}
> +
> +	cpg = kzalloc(sizeof(*cpg), GFP_KERNEL);
> +	clks = kcalloc(num_clks, sizeof(*clks), GFP_KERNEL);
> +	if (cpg = NULL || clks = NULL) {
> +		/* We're leaking memory on purpose, there's no point in cleaning
> +		 * up as the system won't boot anyway.
> +		 */
> +		return;
> +	}
> +
> +	spin_lock_init(&cpg->lock);
> +
> +	cpg->data.clks = clks;
> +	cpg->data.clk_num = num_clks;
> +
> +	cpg->reg = of_iomap(np, 0);
> +	if (WARN_ON(cpg->reg = NULL))
> +		return;
> +
> +	for (i = 0; i < num_clks; ++i) {
> +		const char *name;
> +		struct clk *clk;
> +
> +		of_property_read_string_index(np, "clock-output-names", i,
> +					      &name);
> +
> +		clk = r8a7778_cpg_register_clock(np, cpg, name);
> +		if (IS_ERR(clk))
> +			pr_err("%s: failed to register %s %s clock (%ld)\n",
> +			       __func__, np->name, name, PTR_ERR(clk));
> +		else
> +			cpg->data.clks[i] = clk;
> +	}
> +
> +	of_clk_add_provider(np, of_clk_src_onecell_get, &cpg->data);
> +}
> +
> +CLK_OF_DECLARE(r8a7778_cpg_clks, "renesas,r8a7778-cpg-clocks",
> +	       r8a7778_cpg_clocks_init);
> +
> +void __init r8a7778_clocks_init(u32 mode)
> +{
> +	if (!(mode & BIT(19)))
> +		BUG();

Nitpicking here, you could use BUG_ON().

> +	cpg_mode_rates = (!!(mode & BIT(18)) << 2) |
> +			 (!!(mode & BIT(12)) << 1) |
> +			 (!!(mode & BIT(11)));
> +	cpg_mode_divs = (!!(mode & BIT(2)) << 1) |
> +			(!!(mode & BIT(1)));
> +
> +	of_clk_init(NULL);
> +}

-- 
Regards,

Laurent Pinchart


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

* Re: [PATCH 04/11] ARM: shmobile: r8a7778: common clock framework CPG driver
  2015-01-21 16:20 [PATCH 04/11] ARM: shmobile: r8a7778: common clock framework CPG driver Ulrich Hecht
  2015-01-21 23:37 ` Laurent Pinchart
@ 2015-01-23  9:12 ` Geert Uytterhoeven
  2015-01-23 17:00 ` Ulrich Hecht
  2015-01-24 23:16 ` Laurent Pinchart
  3 siblings, 0 replies; 5+ messages in thread
From: Geert Uytterhoeven @ 2015-01-23  9:12 UTC (permalink / raw)
  To: linux-sh

Hi Ulrich,

On Wed, Jan 21, 2015 at 5:20 PM, Ulrich Hecht
<ulrich.hecht+renesas@gmail.com> wrote:
> Driver for the r8a7778's clocks that depend on the mode bits.
>
> Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
> ---
>  .../bindings/clock/renesas,r8a7778-cpg-clocks.txt  |  24 ++++
>  arch/arm/mach-shmobile/board-bockw-reference.c     |   2 +
>  arch/arm/mach-shmobile/setup-r8a7778.c             |  19 +++
>  drivers/clk/shmobile/Makefile                      |   1 +
>  drivers/clk/shmobile/clk-r8a7778.c                 | 146 +++++++++++++++++++++
>  include/linux/clk/shmobile.h                       |   1 +

Shouldn't this be split in two parts: clk and arch/arm/mach-shmobile?

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

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

* Re: [PATCH 04/11] ARM: shmobile: r8a7778: common clock framework CPG driver
  2015-01-21 16:20 [PATCH 04/11] ARM: shmobile: r8a7778: common clock framework CPG driver Ulrich Hecht
  2015-01-21 23:37 ` Laurent Pinchart
  2015-01-23  9:12 ` Geert Uytterhoeven
@ 2015-01-23 17:00 ` Ulrich Hecht
  2015-01-24 23:16 ` Laurent Pinchart
  3 siblings, 0 replies; 5+ messages in thread
From: Ulrich Hecht @ 2015-01-23 17:00 UTC (permalink / raw)
  To: linux-sh

On Thu, Jan 22, 2015 at 12:37 AM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>> +     if (!strcmp(name, "extal")) {
>> +             return clk_register_fixed_rate(NULL, "extal", NULL,
>> +                     CLK_IS_ROOT, r8a7778_rates[cpg_mode_rates].extal_rate);
>
> As the external clock is an input to the CPG, shouldn't it be specified by a
> separate fixed-frequency clock DT node, and referenced by the CPG using the
> clocks attributes ? You could then remove the extal_rate field from the
> r8a7778_rates array above, and possibly rename the array to r8a7778_pll_mults
> or similar.
>

This is a translation of what the legacy clock driver does.  These
frequencies seem to be assumptions as to what input clocks would be
reasonable given the current mode bits.  If I interpret the schematics
correctly, the Bock-W board has a socketed oscillator, and the mode
bits are switchable, so it's up to the user to configure the board to
a sane setting.  To me, neither a fixed frequency nor this list of
educated guesses seems quite "right" under these circumstances...

CU
Uli

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

* Re: [PATCH 04/11] ARM: shmobile: r8a7778: common clock framework CPG driver
  2015-01-21 16:20 [PATCH 04/11] ARM: shmobile: r8a7778: common clock framework CPG driver Ulrich Hecht
                   ` (2 preceding siblings ...)
  2015-01-23 17:00 ` Ulrich Hecht
@ 2015-01-24 23:16 ` Laurent Pinchart
  3 siblings, 0 replies; 5+ messages in thread
From: Laurent Pinchart @ 2015-01-24 23:16 UTC (permalink / raw)
  To: linux-sh

Hi Ulrich,

On Friday 23 January 2015 18:00:02 Ulrich Hecht wrote:
> On Thu, Jan 22, 2015 at 12:37 AM, Laurent Pinchart wrote:
> >> +     if (!strcmp(name, "extal")) {
> >> +             return clk_register_fixed_rate(NULL, "extal", NULL,
> >> +                     CLK_IS_ROOT,
> >> r8a7778_rates[cpg_mode_rates].extal_rate);
> >
> > As the external clock is an input to the CPG, shouldn't it be specified by
> > a separate fixed-frequency clock DT node, and referenced by the CPG using
> > the clocks attributes ? You could then remove the extal_rate field from
> > the r8a7778_rates array above, and possibly rename the array to
> > r8a7778_pll_mults or similar.
> 
> This is a translation of what the legacy clock driver does.  These
> frequencies seem to be assumptions as to what input clocks would be
> reasonable given the current mode bits.  If I interpret the schematics
> correctly, the Bock-W board has a socketed oscillator, and the mode
> bits are switchable, so it's up to the user to configure the board to
> a sane setting.  To me, neither a fixed frequency nor this list of
> educated guesses seems quite "right" under these circumstances...

A separate fixed frequency clock DT matches the board configuration, so I 
think that's what should be used. It's true that the user could replace the 
oscillator, and in that case the DT will need to be updated. There's not much 
we can do for non-discoverable devices. Even if that solution isn't perfect, I 
think it's better than making the CPG node provide the external clock source, 
as it clearly doesn't from a hardware point of view.

-- 
Regards,

Laurent Pinchart


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

end of thread, other threads:[~2015-01-24 23:16 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-21 16:20 [PATCH 04/11] ARM: shmobile: r8a7778: common clock framework CPG driver Ulrich Hecht
2015-01-21 23:37 ` Laurent Pinchart
2015-01-23  9:12 ` Geert Uytterhoeven
2015-01-23 17:00 ` Ulrich Hecht
2015-01-24 23:16 ` Laurent Pinchart

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.