[PATCH v2 01/10] clk: shmobile: sh73a0 common clock framework implementation

[PATCH v2 01/10] clk: shmobile: sh73a0 common clock framework implementation

[Ulrich Hecht]

Driver for the SH73A0's clocks that are too specific to be supported by a
generic driver.

Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
---
 .../bindings/clock/renesas,sh73a0-cpg-clocks.txt   |  31 ++++
 drivers/clk/shmobile/Makefile                      |   1 +
 drivers/clk/shmobile/clk-sh73a0.c                  | 202 +++++++++++++++++++++
 3 files changed, 234 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/renesas,sh73a0-cpg-clocks.txt
 create mode 100644 drivers/clk/shmobile/clk-sh73a0.c

diff --git a/Documentation/devicetree/bindings/clock/renesas,sh73a0-cpg-clocks.txt b/Documentation/devicetree/bindings/clock/renesas,sh73a0-cpg-clocks.txt
new file mode 100644
index 0000000..6b8dece
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/renesas,sh73a0-cpg-clocks.txt
@@ -0,0 +1,31 @@
+These bindings should be considered EXPERIMENTAL for now.
+
+* Renesas SH73A0 Clock Pulse Generator (CPG)
+
+The CPG generates core clocks for the SH73A0 SoC. It includes four PLLs
+and several fixed ratio dividers.
+
+Required Properties:
+
+  - compatible: Must be "renesas,sh73a0-cpg-clocks"
+
+  - reg: Base address and length of the memory resource used by the CPG
+
+  - clocks: Reference to the parent clocks
+  - #clock-cells: Must be 1
+  - clock-output-names: The names of the clocks. Supported clocks are "main",
+    "pll0", "pll1", "pll2", "pll3", "z", "dsi0phy", "dsi1phy", and "twd"
+
+
+Example
+-------
+
+        cpg_clocks: cpg_clocks@e6150000 {
+                compatible = "renesas,sh73a0-cpg-clocks";
+                reg = <0 0xe6150000 0 0x10000>;
+                clocks = <&extal1_clk>, <&extal2_clk>;
+                #clock-cells = <1>;
+                clock-output-names = "main", "pll0", "pll1", "pll2",
+                                     "pll3", "z", "dsi0phy", "dsi1phy",
+                                     "twd";
+        };
diff --git a/drivers/clk/shmobile/Makefile b/drivers/clk/shmobile/Makefile
index 531d4f6..0c4fd11 100644
--- a/drivers/clk/shmobile/Makefile
+++ b/drivers/clk/shmobile/Makefile
@@ -5,6 +5,7 @@ obj-$(CONFIG_ARCH_R8A7779)		+= clk-r8a7779.o
 obj-$(CONFIG_ARCH_R8A7790)		+= clk-rcar-gen2.o
 obj-$(CONFIG_ARCH_R8A7791)		+= clk-rcar-gen2.o
 obj-$(CONFIG_ARCH_R8A7794)		+= clk-rcar-gen2.o
+obj-$(CONFIG_ARCH_SH73A0)		+= clk-sh73a0.o
 obj-$(CONFIG_ARCH_SHMOBILE_MULTI)	+= clk-div6.o
 obj-$(CONFIG_ARCH_SHMOBILE_MULTI)	+= clk-mstp.o
 # for emply built-in.o
diff --git a/drivers/clk/shmobile/clk-sh73a0.c b/drivers/clk/shmobile/clk-sh73a0.c
new file mode 100644
index 0000000..9d45e9d
--- /dev/null
+++ b/drivers/clk/shmobile/clk-sh73a0.c
@@ -0,0 +1,202 @@
+/*
+ * sh73a0 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/init.h>
+#include <linux/kernel.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/spinlock.h>
+
+struct sh73a0_cpg {
+	struct clk_onecell_data data;
+	spinlock_t lock;
+	void __iomem *reg;
+};
+
+#define CPG_FRQCRA	0x00
+#define CPG_FRQCRB	0x04
+#define CPG_SD0CKCR	0x74
+#define CPG_SD1CKCR	0x78
+#define CPG_SD2CKCR	0x7c
+#define CPG_PLLECR	0xd0
+#define CPG_PLL0CR	0xd8
+#define CPG_PLL1CR	0x28
+#define CPG_PLL2CR	0x2c
+#define CPG_PLL3CR	0xdc
+#define CPG_CKSCR	0xc0
+#define CPG_DSI0PHYCR	0x6c
+#define CPG_DSI1PHYCR	0x70
+
+#define CLK_ENABLE_ON_INIT BIT(0)
+
+struct div4_clk {
+	const char *name;
+	const char *parent;
+	unsigned int reg;
+	unsigned int shift;
+	unsigned int mask;
+	int flags;
+};
+
+static struct div4_clk div4_clks[] = {
+	{ "zg", "pll0", CPG_FRQCRA, 16, 0x0d7f, CLK_ENABLE_ON_INIT },
+	{ "m3", "pll1", CPG_FRQCRA, 12, 0x1dff, CLK_ENABLE_ON_INIT },
+	{ "b",  "pll1", CPG_FRQCRA,  8, 0x0dff, CLK_ENABLE_ON_INIT },
+	{ "m1", "pll1", CPG_FRQCRA,  4, 0x1dff, 0 },
+	{ "m2", "pll1", CPG_FRQCRA,  0, 0x1dff, 0 },
+	{ "z",  "pll0", CPG_FRQCRB, 24, 0x097f, 0 },
+	{ "zx", "pll1", CPG_FRQCRB, 12, 0x0dff, 0 },
+	{ "hp", "pll1", CPG_FRQCRB,  4, 0x0dff, 0 },
+	{ NULL, 0, 0, 0, 0 },
+};
+
+static const struct clk_div_table div4_div_table[] = {
+	{ 0, 2 }, { 1, 3 }, { 2, 4 }, { 3, 6 }, { 4, 8 }, { 5, 12 },
+	{ 6, 16 }, { 7, 18 }, { 8, 24 }, { 10, 36 }, { 11, 48 },
+	{ 12, 7 }, { 0, 0 }
+};
+
+static struct clk * __init
+sh73a0_cpg_register_clock(struct device_node *np, struct sh73a0_cpg *cpg,
+			     const char *name)
+{
+	const struct clk_div_table *table = NULL;
+	const char *parent_name;
+	unsigned int shift, reg;
+	unsigned int mult = 1;
+	unsigned int div = 1;
+
+	if (!strcmp(name, "main")) {
+		/* extal1, extal1_div2, extal2, extal2_div2 */
+		parent_name = of_clk_get_parent_name(np,
+			(clk_readl(cpg->reg + CPG_CKSCR) >> 28) & 3);
+	} else if (!strncmp(name, "pll", 3)) {
+		void __iomem *enable_reg = cpg->reg;
+		u32 enable_bit = name[3] - '0';
+
+		parent_name = "main";
+		switch (enable_bit) {
+		case 0:
+			enable_reg += CPG_PLL0CR;
+			break;
+		case 1:
+			enable_reg += CPG_PLL1CR;
+			break;
+		case 2:
+			enable_reg += CPG_PLL2CR;
+			break;
+		case 3:
+			enable_reg += CPG_PLL3CR;
+			break;
+		default:
+			return ERR_PTR(-EINVAL);
+		}
+		if (clk_readl(cpg->reg + CPG_PLLECR) & BIT(enable_bit)) {
+			mult = ((clk_readl(enable_reg) >> 24) & 0x3f) + 1;
+			/* handle CFG bit for PLL1 and PLL2 */
+			if (enable_bit = 1 || enable_bit = 2)
+				if (clk_readl(enable_reg) & BIT(20))
+					mult *= 2;
+		}
+	} else if (!strcmp(name, "dsi0phy") || !strcmp(name, "dsi1phy")) {
+		u32 phy_no = name[3] - '0';
+		void __iomem *dsi_reg = cpg->reg +
+			(phy_no ? CPG_DSI1PHYCR : CPG_DSI0PHYCR);
+
+		parent_name = phy_no ? "dsi1pck" : "dsi0pck";
+		div = __raw_readl(dsi_reg);
+		if (!(div & 0xb8000))
+			div = 1;
+		else
+			div = (div & 0x3f) + 1;
+	} else {
+		struct div4_clk *c;
+
+		for (c = div4_clks; c->name; c++) {
+			if (!strcmp(name, c->name)) {
+				parent_name = c->parent;
+				table = div4_div_table;
+				reg = c->reg;
+				shift = c->shift;
+				break;
+			}
+		}
+		if (!c->name)
+			return ERR_PTR(-EINVAL);
+	}
+
+	if (!table) {
+		return clk_register_fixed_factor(NULL, name, parent_name, 0,
+						 mult, div);
+	} else {
+		return clk_register_divider_table(NULL, name, parent_name, 0,
+						  cpg->reg + reg, shift, 4, 0,
+						  table, &cpg->lock);
+	}
+}
+
+static void __init sh73a0_cpg_clocks_init(struct device_node *np)
+{
+	struct sh73a0_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;
+
+	/* Set SDHI clocks to a known state */
+	clk_writel(0x108, cpg->reg + CPG_SD0CKCR);
+	clk_writel(0x108, cpg->reg + CPG_SD1CKCR);
+	clk_writel(0x108, cpg->reg + CPG_SD2CKCR);
+
+	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 = sh73a0_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(sh73a0_cpg_clks, "renesas,sh73a0-cpg-clocks",
+	       sh73a0_cpg_clocks_init);
-- 
1.8.4.5

Re: [PATCH v2 01/10] clk: shmobile: sh73a0 common clock framework implementation

[Mike Turquette]

Quoting Ulrich Hecht (2014-09-02 02:12:57)
> diff --git a/drivers/clk/shmobile/Makefile b/drivers/clk/shmobile/Makefile
> index 531d4f6..0c4fd11 100644
> --- a/drivers/clk/shmobile/Makefile
> +++ b/drivers/clk/shmobile/Makefile
> @@ -5,6 +5,7 @@ obj-$(CONFIG_ARCH_R8A7779)              += clk-r8a7779.o
>  obj-$(CONFIG_ARCH_R8A7790)             += clk-rcar-gen2.o
>  obj-$(CONFIG_ARCH_R8A7791)             += clk-rcar-gen2.o
>  obj-$(CONFIG_ARCH_R8A7794)             += clk-rcar-gen2.o
> +obj-$(CONFIG_ARCH_SH73A0)              += clk-sh73a0.o
>  obj-$(CONFIG_ARCH_SHMOBILE_MULTI)      += clk-div6.o
>  obj-$(CONFIG_ARCH_SHMOBILE_MULTI)      += clk-mstp.o
>  # for emply built-in.o

So I don't have any of the CONFIG_ARCH_R8A7794 stuff in my tree, per
this email thread [0].

Do you want the clk patches in this series to go through Simon's tree
also?

Regards,
Mike

[0] http://lkml.kernel.org/r/<20140826234307.GC9189@verge.net.au>

Re: [PATCH v2 01/10] clk: shmobile: sh73a0 common clock framework implementation

[Ulrich Hecht]

On Wed, Sep 3, 2014 at 7:40 AM, Mike Turquette <mturquette@linaro.org> wrote:
> Quoting Ulrich Hecht (2014-09-02 02:12:57)
>> diff --git a/drivers/clk/shmobile/Makefile b/drivers/clk/shmobile/Makefile
>> index 531d4f6..0c4fd11 100644
>> --- a/drivers/clk/shmobile/Makefile
>> +++ b/drivers/clk/shmobile/Makefile
>> @@ -5,6 +5,7 @@ obj-$(CONFIG_ARCH_R8A7779)              += clk-r8a7779.o
>>  obj-$(CONFIG_ARCH_R8A7790)             += clk-rcar-gen2.o
>>  obj-$(CONFIG_ARCH_R8A7791)             += clk-rcar-gen2.o
>>  obj-$(CONFIG_ARCH_R8A7794)             += clk-rcar-gen2.o
>> +obj-$(CONFIG_ARCH_SH73A0)              += clk-sh73a0.o
>>  obj-$(CONFIG_ARCH_SHMOBILE_MULTI)      += clk-div6.o
>>  obj-$(CONFIG_ARCH_SHMOBILE_MULTI)      += clk-mstp.o
>>  # for emply built-in.o
>
> So I don't have any of the CONFIG_ARCH_R8A7794 stuff in my tree, per
> this email thread [0].
>
> Do you want the clk patches in this series to go through Simon's tree
> also?

That would probably work best.

CU
Uli

Re: [PATCH v2 01/10] clk: shmobile: sh73a0 common clock framework implementation

[Simon Horman]

On Wed, Sep 03, 2014 at 09:50:48AM +0200, Ulrich Hecht wrote:
> On Wed, Sep 3, 2014 at 7:40 AM, Mike Turquette <mturquette@linaro.org> wrote:
> > Quoting Ulrich Hecht (2014-09-02 02:12:57)
> >> diff --git a/drivers/clk/shmobile/Makefile b/drivers/clk/shmobile/Makefile
> >> index 531d4f6..0c4fd11 100644
> >> --- a/drivers/clk/shmobile/Makefile
> >> +++ b/drivers/clk/shmobile/Makefile
> >> @@ -5,6 +5,7 @@ obj-$(CONFIG_ARCH_R8A7779)              += clk-r8a7779.o
> >>  obj-$(CONFIG_ARCH_R8A7790)             += clk-rcar-gen2.o
> >>  obj-$(CONFIG_ARCH_R8A7791)             += clk-rcar-gen2.o
> >>  obj-$(CONFIG_ARCH_R8A7794)             += clk-rcar-gen2.o
> >> +obj-$(CONFIG_ARCH_SH73A0)              += clk-sh73a0.o
> >>  obj-$(CONFIG_ARCH_SHMOBILE_MULTI)      += clk-div6.o
> >>  obj-$(CONFIG_ARCH_SHMOBILE_MULTI)      += clk-mstp.o
> >>  # for emply built-in.o
> >
> > So I don't have any of the CONFIG_ARCH_R8A7794 stuff in my tree, per
> > this email thread [0].
> >
> > Do you want the clk patches in this series to go through Simon's tree
> > also?
> 
> That would probably work best.

Yes, I think so too.

Mike, could you provide an Ack?

Re: [PATCH v2 01/10] clk: shmobile: sh73a0 common clock framework implementation

[Simon Horman]

On Tue, Sep 02, 2014 at 11:12:57AM +0200, Ulrich Hecht wrote:
> Driver for the SH73A0's clocks that are too specific to be supported by a
> generic driver.
> 
> Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
> ---
>  .../bindings/clock/renesas,sh73a0-cpg-clocks.txt   |  31 ++++
>  drivers/clk/shmobile/Makefile                      |   1 +
>  drivers/clk/shmobile/clk-sh73a0.c                  | 202 +++++++++++++++++++++
>  3 files changed, 234 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/renesas,sh73a0-cpg-clocks.txt
>  create mode 100644 drivers/clk/shmobile/clk-sh73a0.c

Its not a big deal to me but there seems to be some precedence
for having separate patches for the driver and documenting its bindings.

Re: [PATCH v2 01/10] clk: shmobile: sh73a0 common clock framework implementation

[Laurent Pinchart]

On Thursday 04 September 2014 16:13:58 Simon Horman wrote:
> On Tue, Sep 02, 2014 at 11:12:57AM +0200, Ulrich Hecht wrote:
> > Driver for the SH73A0's clocks that are too specific to be supported by a
> > generic driver.
> > 
> > Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
> > ---
> > 
> >  .../bindings/clock/renesas,sh73a0-cpg-clocks.txt   |  31 ++++
> >  drivers/clk/shmobile/Makefile                      |   1 +
> >  drivers/clk/shmobile/clk-sh73a0.c                  | 202 ++++++++++++++++
> >  3 files changed, 234 insertions(+)
> >  create mode 100644
> >  Documentation/devicetree/bindings/clock/renesas,sh73a0-cpg-clocks.txt
> >  create mode 100644 drivers/clk/shmobile/clk-sh73a0.c
> 
> Its not a big deal to me but there seems to be some precedence
> for having separate patches for the driver and documenting its bindings.

devicetree@vger.kernel.org must be CC'ed on DT bindings patches. As a resend 
is thus needed, the patch could be split.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 01/10] clk: shmobile: sh73a0 common clock framework implementation

[Laurent Pinchart]

Hi Ulrich,

Thank you for the patch.

On Tuesday 02 September 2014 11:12:57 Ulrich Hecht wrote:
> Driver for the SH73A0's clocks that are too specific to be supported by a
> generic driver.
> 
> Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
> ---
>  .../bindings/clock/renesas,sh73a0-cpg-clocks.txt   |  31 ++++
>  drivers/clk/shmobile/Makefile                      |   1 +
>  drivers/clk/shmobile/clk-sh73a0.c                  | 202 ++++++++++++++++++
>  3 files changed, 234 insertions(+)
>  create mode 100644
> Documentation/devicetree/bindings/clock/renesas,sh73a0-cpg-clocks.txt
> create mode 100644 drivers/clk/shmobile/clk-sh73a0.c
> 
> diff --git
> a/Documentation/devicetree/bindings/clock/renesas,sh73a0-cpg-clocks.txt
> b/Documentation/devicetree/bindings/clock/renesas,sh73a0-cpg-clocks.txt new
> file mode 100644
> index 0000000..6b8dece
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/renesas,sh73a0-cpg-clocks.txt
> @@ -0,0 +1,31 @@
> +These bindings should be considered EXPERIMENTAL for now.
> +
> +* Renesas SH73A0 Clock Pulse Generator (CPG)
> +
> +The CPG generates core clocks for the SH73A0 SoC. It includes four PLLs
> +and several fixed ratio dividers.
> +
> +Required Properties:
> +
> +  - compatible: Must be "renesas,sh73a0-cpg-clocks"
> +
> +  - reg: Base address and length of the memory resource used by the CPG
> +
> +  - clocks: Reference to the parent clocks

The parent clocks should be named. Something like

"clocks: References to the parent clocks extal1 and extal2, in that order"

> +  - #clock-cells: Must be 1
> +  - clock-output-names: The names of the clocks. Supported clocks are
> "main",
> +    "pll0", "pll1", "pll2", "pll3", "z", "dsi0phy", "dsi1phy", and "twd"
> +
> +
> +Example
> +-------
> +
> +        cpg_clocks: cpg_clocks@e6150000 {
> +                compatible = "renesas,sh73a0-cpg-clocks";
> +                reg = <0 0xe6150000 0 0x10000>;
> +                clocks = <&extal1_clk>, <&extal2_clk>;
> +                #clock-cells = <1>;
> +                clock-output-names = "main", "pll0", "pll1", "pll2",
> +                                     "pll3", "z", "dsi0phy", "dsi1phy",
> +                                     "twd";
> +        };
> diff --git a/drivers/clk/shmobile/Makefile b/drivers/clk/shmobile/Makefile
> index 531d4f6..0c4fd11 100644
> --- a/drivers/clk/shmobile/Makefile
> +++ b/drivers/clk/shmobile/Makefile
> @@ -5,6 +5,7 @@ obj-$(CONFIG_ARCH_R8A7779)		+= clk-r8a7779.o
>  obj-$(CONFIG_ARCH_R8A7790)		+= clk-rcar-gen2.o
>  obj-$(CONFIG_ARCH_R8A7791)		+= clk-rcar-gen2.o
>  obj-$(CONFIG_ARCH_R8A7794)		+= clk-rcar-gen2.o
> +obj-$(CONFIG_ARCH_SH73A0)		+= clk-sh73a0.o
>  obj-$(CONFIG_ARCH_SHMOBILE_MULTI)	+= clk-div6.o
>  obj-$(CONFIG_ARCH_SHMOBILE_MULTI)	+= clk-mstp.o
>  # for emply built-in.o
> diff --git a/drivers/clk/shmobile/clk-sh73a0.c
> b/drivers/clk/shmobile/clk-sh73a0.c new file mode 100644
> index 0000000..9d45e9d
> --- /dev/null
> +++ b/drivers/clk/shmobile/clk-sh73a0.c
> @@ -0,0 +1,202 @@
> +/*
> + * sh73a0 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/init.h>
> +#include <linux/kernel.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/spinlock.h>
> +
> +struct sh73a0_cpg {
> +	struct clk_onecell_data data;
> +	spinlock_t lock;
> +	void __iomem *reg;
> +};
> +
> +#define CPG_FRQCRA	0x00
> +#define CPG_FRQCRB	0x04
> +#define CPG_SD0CKCR	0x74
> +#define CPG_SD1CKCR	0x78
> +#define CPG_SD2CKCR	0x7c
> +#define CPG_PLLECR	0xd0
> +#define CPG_PLL0CR	0xd8
> +#define CPG_PLL1CR	0x28
> +#define CPG_PLL2CR	0x2c
> +#define CPG_PLL3CR	0xdc
> +#define CPG_CKSCR	0xc0
> +#define CPG_DSI0PHYCR	0x6c
> +#define CPG_DSI1PHYCR	0x70
> +
> +#define CLK_ENABLE_ON_INIT BIT(0)
> +
> +struct div4_clk {
> +	const char *name;
> +	const char *parent;
> +	unsigned int reg;
> +	unsigned int shift;
> +	unsigned int mask;
> +	int flags;

You don't seem to use the mask and flags fields. As all the div4 clocks use a 
4 bits divisor you can most likely remove the mask. If you want to keep the 
flags you should turn them into an unsigned long, as that's what the clock API 
uses.

> +};
> +
> +static struct div4_clk div4_clks[] = {

I think you can make this array const.

> +	{ "zg", "pll0", CPG_FRQCRA, 16, 0x0d7f, CLK_ENABLE_ON_INIT },
> +	{ "m3", "pll1", CPG_FRQCRA, 12, 0x1dff, CLK_ENABLE_ON_INIT },
> +	{ "b",  "pll1", CPG_FRQCRA,  8, 0x0dff, CLK_ENABLE_ON_INIT },
> +	{ "m1", "pll1", CPG_FRQCRA,  4, 0x1dff, 0 },
> +	{ "m2", "pll1", CPG_FRQCRA,  0, 0x1dff, 0 },
> +	{ "z",  "pll0", CPG_FRQCRB, 24, 0x097f, 0 },
> +	{ "zx", "pll1", CPG_FRQCRB, 12, 0x0dff, 0 },
> +	{ "hp", "pll1", CPG_FRQCRB,  4, 0x0dff, 0 },
> +	{ NULL, 0, 0, 0, 0 },
> +};
> +
> +static const struct clk_div_table div4_div_table[] = {
> +	{ 0, 2 }, { 1, 3 }, { 2, 4 }, { 3, 6 }, { 4, 8 }, { 5, 12 },
> +	{ 6, 16 }, { 7, 18 }, { 8, 24 }, { 10, 36 }, { 11, 48 },
> +	{ 12, 7 }, { 0, 0 }
> +};
> +
> +static struct clk * __init
> +sh73a0_cpg_register_clock(struct device_node *np, struct sh73a0_cpg *cpg,
> +			     const char *name)
> +{
> +	const struct clk_div_table *table = NULL;
> +	const char *parent_name;
> +	unsigned int shift, reg;
> +	unsigned int mult = 1;
> +	unsigned int div = 1;
> +
> +	if (!strcmp(name, "main")) {
> +		/* extal1, extal1_div2, extal2, extal2_div2 */
> +		parent_name = of_clk_get_parent_name(np,
> +			(clk_readl(cpg->reg + CPG_CKSCR) >> 28) & 3);

That's four options, but your bindings example only define two input clocks 
and the bindings themselves don't document what the other two clocks should 
be. Unless I'm mistaken there are two possible input clocks and two possible 
divisors for them, you could implement it as

		reg = clk_readl(cpg->reg + CPG_CKSCR);
		parent_name = of_clk_get_parent_name(np, (reg >> 29) & 1);
		div = ((reg >> 28) & 1) + 1;

> +	} else if (!strncmp(name, "pll", 3)) {
> +		void __iomem *enable_reg = cpg->reg;
> +		u32 enable_bit = name[3] - '0';
> +
> +		parent_name = "main";
> +		switch (enable_bit) {
> +		case 0:
> +			enable_reg += CPG_PLL0CR;
> +			break;
> +		case 1:
> +			enable_reg += CPG_PLL1CR;
> +			break;
> +		case 2:
> +			enable_reg += CPG_PLL2CR;
> +			break;
> +		case 3:
> +			enable_reg += CPG_PLL3CR;
> +			break;
> +		default:
> +			return ERR_PTR(-EINVAL);
> +		}
> +		if (clk_readl(cpg->reg + CPG_PLLECR) & BIT(enable_bit)) {

If I understand the check correctly, it considers that a disabled PLL will 
work in pass-through mode and will output the input frequency at its output. 
Have you been able to test that ? I think it would at least deserve a comment 
explaining what's going on.

> +			mult = ((clk_readl(enable_reg) >> 24) & 0x3f) + 1;
> +			/* handle CFG bit for PLL1 and PLL2 */
> +			if (enable_bit = 1 || enable_bit = 2)
> +				if (clk_readl(enable_reg) & BIT(20))
> +					mult *= 2;

You could read the PLL enable register once only and use the cached value.

> +		}
> +	} else if (!strcmp(name, "dsi0phy") || !strcmp(name, "dsi1phy")) {
> +		u32 phy_no = name[3] - '0';
> +		void __iomem *dsi_reg = cpg->reg +
> +			(phy_no ? CPG_DSI1PHYCR : CPG_DSI0PHYCR);
> +
> +		parent_name = phy_no ? "dsi1pck" : "dsi0pck";
> +		div = __raw_readl(dsi_reg);

Any reason to use clk_readl above and __raw_readl here ?

> +		if (!(div & 0xb8000))
> +			div = 1;
> +		else
> +			div = (div & 0x3f) + 1;

To avoid assigning a different value to div and then reassigning 1, how about

		reg = __raw_readl(dsi_reg);
		if (reg & 0xb8000)
			div = (reg & 0x3f) + 1;

> +	} else {
> +		struct div4_clk *c;
> +
> +		for (c = div4_clks; c->name; c++) {
> +			if (!strcmp(name, c->name)) {
> +				parent_name = c->parent;
> +				table = div4_div_table;
> +				reg = c->reg;
> +				shift = c->shift;
> +				break;
> +			}
> +		}
> +		if (!c->name)
> +			return ERR_PTR(-EINVAL);
> +	}
> +
> +	if (!table) {
> +		return clk_register_fixed_factor(NULL, name, parent_name, 0,
> +						 mult, div);
> +	} else {
> +		return clk_register_divider_table(NULL, name, parent_name, 0,
> +						  cpg->reg + reg, shift, 4, 0,
> +						  table, &cpg->lock);
> +	}
> +}
> +
> +static void __init sh73a0_cpg_clocks_init(struct device_node *np)
> +{
> +	struct sh73a0_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;
> +
> +	/* Set SDHI clocks to a known state */
> +	clk_writel(0x108, cpg->reg + CPG_SD0CKCR);
> +	clk_writel(0x108, cpg->reg + CPG_SD1CKCR);
> +	clk_writel(0x108, cpg->reg + CPG_SD2CKCR);
> +
> +	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 = sh73a0_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(sh73a0_cpg_clks, "renesas,sh73a0-cpg-clocks",
> +	       sh73a0_cpg_clocks_init);

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 01/10] clk: shmobile: sh73a0 common clock framework implementation

[Laurent Pinchart]

Hi Ulrich,

One more comment.

On Tuesday 02 September 2014 11:12:57 Ulrich Hecht wrote:
> Driver for the SH73A0's clocks that are too specific to be supported by a
> generic driver.
> 
> Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
> ---
>  .../bindings/clock/renesas,sh73a0-cpg-clocks.txt   |  31 ++++
>  drivers/clk/shmobile/Makefile                      |   1 +
>  drivers/clk/shmobile/clk-sh73a0.c                  | 202 ++++++++++++++++++
>  3 files changed, 234 insertions(+)
>  create mode 100644
> Documentation/devicetree/bindings/clock/renesas,sh73a0-cpg-clocks.txt
> create mode 100644 drivers/clk/shmobile/clk-sh73a0.c
> 
> diff --git
> a/Documentation/devicetree/bindings/clock/renesas,sh73a0-cpg-clocks.txt
> b/Documentation/devicetree/bindings/clock/renesas,sh73a0-cpg-clocks.txt new
> file mode 100644
> index 0000000..6b8dece
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/renesas,sh73a0-cpg-clocks.txt
> @@ -0,0 +1,31 @@
> +These bindings should be considered EXPERIMENTAL for now.
> +
> +* Renesas SH73A0 Clock Pulse Generator (CPG)
> +
> +The CPG generates core clocks for the SH73A0 SoC. It includes four PLLs
> +and several fixed ratio dividers.
> +
> +Required Properties:
> +
> +  - compatible: Must be "renesas,sh73a0-cpg-clocks"
> +
> +  - reg: Base address and length of the memory resource used by the CPG
> +
> +  - clocks: Reference to the parent clocks
> +  - #clock-cells: Must be 1
> +  - clock-output-names: The names of the clocks. Supported clocks are
> "main", +    "pll0", "pll1", "pll2", "pll3", "z", "dsi0phy", "dsi1phy", and
> "twd" +
> +
> +Example
> +-------
> +
> +        cpg_clocks: cpg_clocks@e6150000 {
> +                compatible = "renesas,sh73a0-cpg-clocks";
> +                reg = <0 0xe6150000 0 0x10000>;
> +                clocks = <&extal1_clk>, <&extal2_clk>;
> +                #clock-cells = <1>;
> +                clock-output-names = "main", "pll0", "pll1", "pll2",
> +                                     "pll3", "z", "dsi0phy", "dsi1phy",
> +                                     "twd";
> +        };
> diff --git a/drivers/clk/shmobile/Makefile b/drivers/clk/shmobile/Makefile
> index 531d4f6..0c4fd11 100644
> --- a/drivers/clk/shmobile/Makefile
> +++ b/drivers/clk/shmobile/Makefile
> @@ -5,6 +5,7 @@ obj-$(CONFIG_ARCH_R8A7779)		+= clk-r8a7779.o
>  obj-$(CONFIG_ARCH_R8A7790)		+= clk-rcar-gen2.o
>  obj-$(CONFIG_ARCH_R8A7791)		+= clk-rcar-gen2.o
>  obj-$(CONFIG_ARCH_R8A7794)		+= clk-rcar-gen2.o
> +obj-$(CONFIG_ARCH_SH73A0)		+= clk-sh73a0.o
>  obj-$(CONFIG_ARCH_SHMOBILE_MULTI)	+= clk-div6.o
>  obj-$(CONFIG_ARCH_SHMOBILE_MULTI)	+= clk-mstp.o
>  # for emply built-in.o
> diff --git a/drivers/clk/shmobile/clk-sh73a0.c
> b/drivers/clk/shmobile/clk-sh73a0.c new file mode 100644
> index 0000000..9d45e9d
> --- /dev/null
> +++ b/drivers/clk/shmobile/clk-sh73a0.c
> @@ -0,0 +1,202 @@
> +/*
> + * sh73a0 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/init.h>
> +#include <linux/kernel.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/spinlock.h>
> +
> +struct sh73a0_cpg {
> +	struct clk_onecell_data data;
> +	spinlock_t lock;
> +	void __iomem *reg;
> +};
> +
> +#define CPG_FRQCRA	0x00
> +#define CPG_FRQCRB	0x04
> +#define CPG_SD0CKCR	0x74
> +#define CPG_SD1CKCR	0x78
> +#define CPG_SD2CKCR	0x7c
> +#define CPG_PLLECR	0xd0
> +#define CPG_PLL0CR	0xd8
> +#define CPG_PLL1CR	0x28
> +#define CPG_PLL2CR	0x2c
> +#define CPG_PLL3CR	0xdc
> +#define CPG_CKSCR	0xc0
> +#define CPG_DSI0PHYCR	0x6c
> +#define CPG_DSI1PHYCR	0x70
> +
> +#define CLK_ENABLE_ON_INIT BIT(0)
> +
> +struct div4_clk {
> +	const char *name;
> +	const char *parent;
> +	unsigned int reg;
> +	unsigned int shift;
> +	unsigned int mask;
> +	int flags;
> +};
> +
> +static struct div4_clk div4_clks[] = {
> +	{ "zg", "pll0", CPG_FRQCRA, 16, 0x0d7f, CLK_ENABLE_ON_INIT },
> +	{ "m3", "pll1", CPG_FRQCRA, 12, 0x1dff, CLK_ENABLE_ON_INIT },
> +	{ "b",  "pll1", CPG_FRQCRA,  8, 0x0dff, CLK_ENABLE_ON_INIT },
> +	{ "m1", "pll1", CPG_FRQCRA,  4, 0x1dff, 0 },
> +	{ "m2", "pll1", CPG_FRQCRA,  0, 0x1dff, 0 },
> +	{ "z",  "pll0", CPG_FRQCRB, 24, 0x097f, 0 },
> +	{ "zx", "pll1", CPG_FRQCRB, 12, 0x0dff, 0 },
> +	{ "hp", "pll1", CPG_FRQCRB,  4, 0x0dff, 0 },
> +	{ NULL, 0, 0, 0, 0 },
> +};
> +
> +static const struct clk_div_table div4_div_table[] = {
> +	{ 0, 2 }, { 1, 3 }, { 2, 4 }, { 3, 6 }, { 4, 8 }, { 5, 12 },
> +	{ 6, 16 }, { 7, 18 }, { 8, 24 }, { 10, 36 }, { 11, 48 },
> +	{ 12, 7 }, { 0, 0 }

Not all ratios are valid for all clocks, and not all clocks use the same 
numerical values for the same ratios. I suppose the first problem can be 
ignored for now if we assume that the registers are programmed with valid 
values. The second problem, however, needs to be handled at least for the zg 
clock that uses value 10 for a 1/5 divisor instead of 1/36 (unless this is a 
mistake in the datasheet).

> +};
> +
> +static struct clk * __init
> +sh73a0_cpg_register_clock(struct device_node *np, struct sh73a0_cpg *cpg,
> +			     const char *name)
> +{
> +	const struct clk_div_table *table = NULL;
> +	const char *parent_name;
> +	unsigned int shift, reg;
> +	unsigned int mult = 1;
> +	unsigned int div = 1;
> +
> +	if (!strcmp(name, "main")) {
> +		/* extal1, extal1_div2, extal2, extal2_div2 */
> +		parent_name = of_clk_get_parent_name(np,
> +			(clk_readl(cpg->reg + CPG_CKSCR) >> 28) & 3);
> +	} else if (!strncmp(name, "pll", 3)) {
> +		void __iomem *enable_reg = cpg->reg;
> +		u32 enable_bit = name[3] - '0';
> +
> +		parent_name = "main";
> +		switch (enable_bit) {
> +		case 0:
> +			enable_reg += CPG_PLL0CR;
> +			break;
> +		case 1:
> +			enable_reg += CPG_PLL1CR;
> +			break;
> +		case 2:
> +			enable_reg += CPG_PLL2CR;
> +			break;
> +		case 3:
> +			enable_reg += CPG_PLL3CR;
> +			break;
> +		default:
> +			return ERR_PTR(-EINVAL);
> +		}
> +		if (clk_readl(cpg->reg + CPG_PLLECR) & BIT(enable_bit)) {
> +			mult = ((clk_readl(enable_reg) >> 24) & 0x3f) + 1;
> +			/* handle CFG bit for PLL1 and PLL2 */
> +			if (enable_bit = 1 || enable_bit = 2)
> +				if (clk_readl(enable_reg) & BIT(20))
> +					mult *= 2;
> +		}
> +	} else if (!strcmp(name, "dsi0phy") || !strcmp(name, "dsi1phy")) {
> +		u32 phy_no = name[3] - '0';
> +		void __iomem *dsi_reg = cpg->reg +
> +			(phy_no ? CPG_DSI1PHYCR : CPG_DSI0PHYCR);
> +
> +		parent_name = phy_no ? "dsi1pck" : "dsi0pck";
> +		div = __raw_readl(dsi_reg);
> +		if (!(div & 0xb8000))
> +			div = 1;
> +		else
> +			div = (div & 0x3f) + 1;
> +	} else {
> +		struct div4_clk *c;
> +
> +		for (c = div4_clks; c->name; c++) {
> +			if (!strcmp(name, c->name)) {
> +				parent_name = c->parent;
> +				table = div4_div_table;
> +				reg = c->reg;
> +				shift = c->shift;
> +				break;
> +			}
> +		}
> +		if (!c->name)
> +			return ERR_PTR(-EINVAL);
> +	}
> +
> +	if (!table) {
> +		return clk_register_fixed_factor(NULL, name, parent_name, 0,
> +						 mult, div);
> +	} else {
> +		return clk_register_divider_table(NULL, name, parent_name, 0,
> +						  cpg->reg + reg, shift, 4, 0,
> +						  table, &cpg->lock);
> +	}
> +}
> +
> +static void __init sh73a0_cpg_clocks_init(struct device_node *np)
> +{
> +	struct sh73a0_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;
> +
> +	/* Set SDHI clocks to a known state */
> +	clk_writel(0x108, cpg->reg + CPG_SD0CKCR);
> +	clk_writel(0x108, cpg->reg + CPG_SD1CKCR);
> +	clk_writel(0x108, cpg->reg + CPG_SD2CKCR);
> +
> +	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 = sh73a0_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(sh73a0_cpg_clks, "renesas,sh73a0-cpg-clocks",
> +	       sh73a0_cpg_clocks_init);

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 01/10] clk: shmobile: sh73a0 common clock framework implementation

[Mike Turquette]

Quoting Simon Horman (2014-09-03 23:49:24)
> On Wed, Sep 03, 2014 at 09:50:48AM +0200, Ulrich Hecht wrote:
> > On Wed, Sep 3, 2014 at 7:40 AM, Mike Turquette <mturquette@linaro.org> wrote:
> > > Quoting Ulrich Hecht (2014-09-02 02:12:57)
> > >> diff --git a/drivers/clk/shmobile/Makefile b/drivers/clk/shmobile/Makefile
> > >> index 531d4f6..0c4fd11 100644
> > >> --- a/drivers/clk/shmobile/Makefile
> > >> +++ b/drivers/clk/shmobile/Makefile
> > >> @@ -5,6 +5,7 @@ obj-$(CONFIG_ARCH_R8A7779)              += clk-r8a7779.o
> > >>  obj-$(CONFIG_ARCH_R8A7790)             += clk-rcar-gen2.o
> > >>  obj-$(CONFIG_ARCH_R8A7791)             += clk-rcar-gen2.o
> > >>  obj-$(CONFIG_ARCH_R8A7794)             += clk-rcar-gen2.o
> > >> +obj-$(CONFIG_ARCH_SH73A0)              += clk-sh73a0.o
> > >>  obj-$(CONFIG_ARCH_SHMOBILE_MULTI)      += clk-div6.o
> > >>  obj-$(CONFIG_ARCH_SHMOBILE_MULTI)      += clk-mstp.o
> > >>  # for emply built-in.o
> > >
> > > So I don't have any of the CONFIG_ARCH_R8A7794 stuff in my tree, per
> > > this email thread [0].
> > >
> > > Do you want the clk patches in this series to go through Simon's tree
> > > also?
> > 
> > That would probably work best.
> 
> Yes, I think so too.
> 
> Mike, could you provide an Ack?

Sure, you have my Ack for patch #1 and #8. I think #9 is still
unresolved. Will that be converted to a mux clock?

Thanks,
Mike

Re: [PATCH v2 01/10] clk: shmobile: sh73a0 common clock framework implementation

[Laurent Pinchart]

Hi Mike,

On Tuesday 09 September 2014 15:19:10 Mike Turquette wrote:
> Quoting Simon Horman (2014-09-03 23:49:24)
> > On Wed, Sep 03, 2014 at 09:50:48AM +0200, Ulrich Hecht wrote:
> >> On Wed, Sep 3, 2014 at 7:40 AM, Mike Turquette wrote:
> >>> Quoting Ulrich Hecht (2014-09-02 02:12:57)
> >>> 
> >>>> diff --git a/drivers/clk/shmobile/Makefile
> >>>> b/drivers/clk/shmobile/Makefile
> >>>> index 531d4f6..0c4fd11 100644
> >>>> --- a/drivers/clk/shmobile/Makefile
> >>>> +++ b/drivers/clk/shmobile/Makefile
> >>>> @@ -5,6 +5,7 @@ obj-$(CONFIG_ARCH_R8A7779)              +> >>>> clk-r8a7779.o
> >>>> 
> >>>>  obj-$(CONFIG_ARCH_R8A7790)             += clk-rcar-gen2.o
> >>>>  obj-$(CONFIG_ARCH_R8A7791)             += clk-rcar-gen2.o
> >>>>  obj-$(CONFIG_ARCH_R8A7794)             += clk-rcar-gen2.o
> >>>> 
> >>>> +obj-$(CONFIG_ARCH_SH73A0)              += clk-sh73a0.o
> >>>> 
> >>>>  obj-$(CONFIG_ARCH_SHMOBILE_MULTI)      += clk-div6.o
> >>>>  obj-$(CONFIG_ARCH_SHMOBILE_MULTI)      += clk-mstp.o
> >>>>  # for emply built-in.o
> >>> 
> >>> So I don't have any of the CONFIG_ARCH_R8A7794 stuff in my tree, per
> >>> this email thread [0].
> >>> 
> >>> Do you want the clk patches in this series to go through Simon's tree
> >>> also?
> >> 
> >> That would probably work best.
> > 
> > Yes, I think so too.
> > 
> > Mike, could you provide an Ack?
> 
> Sure, you have my Ack for patch #1 and #8. I think #9 is still
> unresolved. Will that be converted to a mux clock?

As the DIV6 clocks support rate modification at runtime, so we can't turn them 
into plain mux clocks.

I don't have a strong preference between static and dynamic parent selection 
until someone provides use cases for dynamic parent selection. We need to iron 
out the DT bindings though, and regardless of the chosen implementation.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 01/10] clk: shmobile: sh73a0 common clock framework implementation

[Simon Horman]

On Tue, Sep 09, 2014 at 03:19:10PM -0700, Mike Turquette wrote:
> Quoting Simon Horman (2014-09-03 23:49:24)
> > On Wed, Sep 03, 2014 at 09:50:48AM +0200, Ulrich Hecht wrote:
> > > On Wed, Sep 3, 2014 at 7:40 AM, Mike Turquette <mturquette@linaro.org> wrote:
> > > > Quoting Ulrich Hecht (2014-09-02 02:12:57)
> > > >> diff --git a/drivers/clk/shmobile/Makefile b/drivers/clk/shmobile/Makefile
> > > >> index 531d4f6..0c4fd11 100644
> > > >> --- a/drivers/clk/shmobile/Makefile
> > > >> +++ b/drivers/clk/shmobile/Makefile
> > > >> @@ -5,6 +5,7 @@ obj-$(CONFIG_ARCH_R8A7779)              += clk-r8a7779.o
> > > >>  obj-$(CONFIG_ARCH_R8A7790)             += clk-rcar-gen2.o
> > > >>  obj-$(CONFIG_ARCH_R8A7791)             += clk-rcar-gen2.o
> > > >>  obj-$(CONFIG_ARCH_R8A7794)             += clk-rcar-gen2.o
> > > >> +obj-$(CONFIG_ARCH_SH73A0)              += clk-sh73a0.o
> > > >>  obj-$(CONFIG_ARCH_SHMOBILE_MULTI)      += clk-div6.o
> > > >>  obj-$(CONFIG_ARCH_SHMOBILE_MULTI)      += clk-mstp.o
> > > >>  # for emply built-in.o
> > > >
> > > > So I don't have any of the CONFIG_ARCH_R8A7794 stuff in my tree, per
> > > > this email thread [0].
> > > >
> > > > Do you want the clk patches in this series to go through Simon's tree
> > > > also?
> > > 
> > > That would probably work best.
> > 
> > Yes, I think so too.
> > 
> > Mike, could you provide an Ack?
> 
> Sure, you have my Ack for patch #1 and #8. I think #9 is still
> unresolved. Will that be converted to a mux clock?

Ulrich, please include Mike's ack when you repost this patch
to address Laurent's feedback (in other subthreads).

Re: [PATCH v2 01/10] clk: shmobile: sh73a0 common clock framework implementation

[Mike Turquette]

Quoting Laurent Pinchart (2014-09-09 15:27:27)
> Hi Mike,
> 
> On Tuesday 09 September 2014 15:19:10 Mike Turquette wrote:
> > Quoting Simon Horman (2014-09-03 23:49:24)
> > > On Wed, Sep 03, 2014 at 09:50:48AM +0200, Ulrich Hecht wrote:
> > >> On Wed, Sep 3, 2014 at 7:40 AM, Mike Turquette wrote:
> > >>> Quoting Ulrich Hecht (2014-09-02 02:12:57)
> > >>> 
> > >>>> diff --git a/drivers/clk/shmobile/Makefile
> > >>>> b/drivers/clk/shmobile/Makefile
> > >>>> index 531d4f6..0c4fd11 100644
> > >>>> --- a/drivers/clk/shmobile/Makefile
> > >>>> +++ b/drivers/clk/shmobile/Makefile
> > >>>> @@ -5,6 +5,7 @@ obj-$(CONFIG_ARCH_R8A7779)              +> > >>>> clk-r8a7779.o
> > >>>> 
> > >>>>  obj-$(CONFIG_ARCH_R8A7790)             += clk-rcar-gen2.o
> > >>>>  obj-$(CONFIG_ARCH_R8A7791)             += clk-rcar-gen2.o
> > >>>>  obj-$(CONFIG_ARCH_R8A7794)             += clk-rcar-gen2.o
> > >>>> 
> > >>>> +obj-$(CONFIG_ARCH_SH73A0)              += clk-sh73a0.o
> > >>>> 
> > >>>>  obj-$(CONFIG_ARCH_SHMOBILE_MULTI)      += clk-div6.o
> > >>>>  obj-$(CONFIG_ARCH_SHMOBILE_MULTI)      += clk-mstp.o
> > >>>>  # for emply built-in.o
> > >>> 
> > >>> So I don't have any of the CONFIG_ARCH_R8A7794 stuff in my tree, per
> > >>> this email thread [0].
> > >>> 
> > >>> Do you want the clk patches in this series to go through Simon's tree
> > >>> also?
> > >> 
> > >> That would probably work best.
> > > 
> > > Yes, I think so too.
> > > 
> > > Mike, could you provide an Ack?
> > 
> > Sure, you have my Ack for patch #1 and #8. I think #9 is still
> > unresolved. Will that be converted to a mux clock?
> 
> As the DIV6 clocks support rate modification at runtime, so we can't turn them 
> into plain mux clocks.

Nomenclature confusion ;-)

I don't actually mean to use common struct clk_mux. I mean, "support
.get_parent and .set_parent callbacks in your clock driver
implementation for DIV6 clocks".

> 
> I don't have a strong preference between static and dynamic parent selection 
> until someone provides use cases for dynamic parent selection. We need to iron 
> out the DT bindings though, and regardless of the chosen implementation.

Agreed.

Regards,
Mike

> 
> -- 
> Regards,
> 
> Laurent Pinchart
> 

Re: [PATCH v2 01/10] clk: shmobile: sh73a0 common clock framework implementation

[Ulrich Hecht]

On Wed, Sep 10, 2014 at 6:59 PM, Mike Turquette <mturquette@linaro.org> wrote:
> Quoting Laurent Pinchart (2014-09-09 15:27:27)
>> I don't have a strong preference between static and dynamic parent selection
>> until someone provides use cases for dynamic parent selection. We need to iron
>> out the DT bindings though, and regardless of the chosen implementation.
>
> Agreed.

I will be able to give this specific matter my undivided attention in
October and will look into it then.

CU
Uli