[PATCH v7 0/2] clk: shmobile: DIV6 clock variable parent support

[PATCH v7 0/2] clk: shmobile: DIV6 clock variable parent support

[Ulrich Hecht]

Hi!

As Geert suggested, this revision fixes the number of parent clocks at one,
four, or eight to avoid ambiguity.

CU
Uli


Changes since v6:
- require exactly one, four, or eight parent clocks

Changes since v5:
- straighten types
- don't use hw->init outside cpg_div6_clock_init()
- drop superfluous allocation error messages
- drop renesas,src-* properties, can be inferred

Changes since v4:
- implement .get_parent, .set_parent
- improve bindings documentation

Changes since v3:
- note that renesas,src-shift and renesas,src-width depend on each other
- clarified description
- minor coding style fixes

Changes since v2:
- add r8a73a4 to bindings
- use u32 where appropriate
- don't split error message

Changes since v1:
- make sure unrelated register bits are preserved
- use the plural for the clocks property in bindings

Ulrich Hecht (2):
  clk: shmobile: div6: support selectable-input clocks
  clk: shmobile: document DIV6 clock parent bindings

 .../bindings/clock/renesas,cpg-div6-clocks.txt     |  18 ++--
 drivers/clk/shmobile/clk-div6.c                    | 113 ++++++++++++++++++---
 2 files changed, 113 insertions(+), 18 deletions(-)

-- 
1.8.4.5

[PATCH v7 1/2] clk: shmobile: div6: support selectable-input clocks

[Ulrich Hecht]

Support for setting the parent at initialization time based on the current
hardware configuration in DIV6 clocks with selectable parents as found in
the r8a73a4, r8a7740, sh73a0, and other SoCs.

Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
---
 drivers/clk/shmobile/clk-div6.c | 113 +++++++++++++++++++++++++++++++++++-----
 1 file changed, 101 insertions(+), 12 deletions(-)

diff --git a/drivers/clk/shmobile/clk-div6.c b/drivers/clk/shmobile/clk-div6.c
index f065f69..639241e 100644
--- a/drivers/clk/shmobile/clk-div6.c
+++ b/drivers/clk/shmobile/clk-div6.c
@@ -32,6 +32,9 @@ struct div6_clock {
 	struct clk_hw hw;
 	void __iomem *reg;
 	unsigned int div;
+	u32 src_shift;
+	u32 src_width;
+	u8 *parents;
 };
 
 #define to_div6_clock(_hw) container_of(_hw, struct div6_clock, hw)
@@ -39,8 +42,11 @@ struct div6_clock {
 static int cpg_div6_clock_enable(struct clk_hw *hw)
 {
 	struct div6_clock *clock = to_div6_clock(hw);
+	u32 val;
 
-	clk_writel(CPG_DIV6_DIV(clock->div - 1), clock->reg);
+	val = (clk_readl(clock->reg) & ~(CPG_DIV6_DIV_MASK | CPG_DIV6_CKSTP))
+	    | CPG_DIV6_DIV(clock->div - 1);
+	clk_writel(val, clock->reg);
 
 	return 0;
 }
@@ -52,7 +58,7 @@ static void cpg_div6_clock_disable(struct clk_hw *hw)
 	/* DIV6 clocks require the divisor field to be non-zero when stopping
 	 * the clock.
 	 */
-	clk_writel(CPG_DIV6_CKSTP | CPG_DIV6_DIV(CPG_DIV6_DIV_MASK),
+	clk_writel(clk_readl(clock->reg) | CPG_DIV6_CKSTP | CPG_DIV6_DIV_MASK,
 		   clock->reg);
 }
 
@@ -94,12 +100,53 @@ static int cpg_div6_clock_set_rate(struct clk_hw *hw, unsigned long rate,
 {
 	struct div6_clock *clock = to_div6_clock(hw);
 	unsigned int div = cpg_div6_clock_calc_div(rate, parent_rate);
+	u32 val;
 
 	clock->div = div;
 
+	val = clk_readl(clock->reg) & ~CPG_DIV6_DIV_MASK;
 	/* Only program the new divisor if the clock isn't stopped. */
-	if (!(clk_readl(clock->reg) & CPG_DIV6_CKSTP))
-		clk_writel(CPG_DIV6_DIV(clock->div - 1), clock->reg);
+	if (!(val & CPG_DIV6_CKSTP))
+		clk_writel(val | CPG_DIV6_DIV(clock->div - 1), clock->reg);
+
+	return 0;
+}
+
+static u8 cpg_div6_clock_get_parent(struct clk_hw *hw)
+{
+	struct div6_clock *clock = to_div6_clock(hw);
+	unsigned int i;
+	u8 hw_index;
+
+	if (clock->src_width == 0)
+		return 0;
+
+	hw_index = (clk_readl(clock->reg) >> clock->src_shift) &
+		   (BIT(clock->src_width) - 1);
+	for (i = 0; i < __clk_get_num_parents(hw->clk); i++) {
+		if (clock->parents[i] == hw_index)
+			return i;
+	}
+
+	pr_err("%s: %s DIV6 clock set to invalid parent %u\n",
+	       __func__, __clk_get_name(hw->clk), hw_index);
+	return 0;
+}
+
+static int cpg_div6_clock_set_parent(struct clk_hw *hw, u8 index)
+{
+	struct div6_clock *clock = to_div6_clock(hw);
+	u8 hw_index;
+	u32 mask;
+
+	if (index >= __clk_get_num_parents(hw->clk))
+		return -EINVAL;
+
+	mask = ~((BIT(clock->src_width) - 1) << clock->src_shift);
+	hw_index = clock->parents[index];
+
+	clk_writel((clk_readl(clock->reg) & mask) |
+		(hw_index << clock->src_shift), clock->reg);
 
 	return 0;
 }
@@ -108,6 +155,8 @@ static const struct clk_ops cpg_div6_clock_ops = {
 	.enable = cpg_div6_clock_enable,
 	.disable = cpg_div6_clock_disable,
 	.is_enabled = cpg_div6_clock_is_enabled,
+	.get_parent = cpg_div6_clock_get_parent,
+	.set_parent = cpg_div6_clock_set_parent,
 	.recalc_rate = cpg_div6_clock_recalc_rate,
 	.round_rate = cpg_div6_clock_round_rate,
 	.set_rate = cpg_div6_clock_set_rate,
@@ -115,20 +164,33 @@ static const struct clk_ops cpg_div6_clock_ops = {
 
 static void __init cpg_div6_clock_init(struct device_node *np)
 {
+	unsigned int num_parents, valid_parents;
+	const char **parent_names;
 	struct clk_init_data init;
 	struct div6_clock *clock;
-	const char *parent_name;
 	const char *name;
 	struct clk *clk;
+	unsigned int i;
 	int ret;
 
 	clock = kzalloc(sizeof(*clock), GFP_KERNEL);
-	if (!clock) {
-		pr_err("%s: failed to allocate %s DIV6 clock\n",
+	if (!clock)
+		return;
+
+	num_parents = of_clk_get_parent_count(np);
+	if (num_parents < 1) {
+		pr_err("%s: no parent found for %s DIV6 clock\n",
 		       __func__, np->name);
 		return;
 	}
 
+	clock->parents = kmalloc_array(num_parents, sizeof(*clock->parents),
+		GFP_KERNEL);
+	parent_names = kmalloc_array(num_parents, sizeof(*parent_names),
+				GFP_KERNEL);
+	if (!parent_names)
+		return;
+
 	/* Remap the clock register and read the divisor. Disabling the
 	 * clock overwrites the divisor, so we need to cache its value for the
 	 * enable operation.
@@ -150,9 +212,34 @@ static void __init cpg_div6_clock_init(struct device_node *np)
 		goto error;
 	}
 
-	parent_name = of_clk_get_parent_name(np, 0);
-	if (parent_name == NULL) {
-		pr_err("%s: failed to get %s DIV6 clock parent name\n",
+
+	for (i = 0, valid_parents = 0; i < num_parents; i++) {
+		const char *name = of_clk_get_parent_name(np, i);
+
+		if (name) {
+			parent_names[valid_parents] = name;
+			clock->parents[valid_parents] = i;
+			valid_parents++;
+		}
+	}
+
+	switch (num_parents) {
+	case 1:
+		/* fixed parent clock */
+		clock->src_shift = clock->src_width = 0;
+		break;
+	case 4:
+		/* clock with EXSRC bits 6-7 */
+		clock->src_shift = 6;
+		clock->src_width = 2;
+		break;
+	case 8:
+		/* VCLK with EXSRC bits 12-14 */
+		clock->src_shift = 12;
+		clock->src_width = 3;
+		break;
+	default:
+		pr_err("%s: invalid number of parents for DIV6 clock %s\n",
 		       __func__, np->name);
 		goto error;
 	}
@@ -161,8 +248,8 @@ static void __init cpg_div6_clock_init(struct device_node *np)
 	init.name = name;
 	init.ops = &cpg_div6_clock_ops;
 	init.flags = CLK_IS_BASIC;
-	init.parent_names = &parent_name;
-	init.num_parents = 1;
+	init.parent_names = parent_names;
+	init.num_parents = valid_parents;
 
 	clock->hw.init = &init;
 
@@ -175,11 +262,13 @@ static void __init cpg_div6_clock_init(struct device_node *np)
 
 	of_clk_add_provider(np, of_clk_src_simple_get, clk);
 
+	kfree(parent_names);
 	return;
 
 error:
 	if (clock->reg)
 		iounmap(clock->reg);
+	kfree(parent_names);
 	kfree(clock);
 }
 CLK_OF_DECLARE(cpg_div6_clk, "renesas,cpg-div6-clock", cpg_div6_clock_init);
-- 
1.8.4.5

[PATCH v7 2/2] clk: shmobile: document DIV6 clock parent bindings

[Ulrich Hecht]

Describes how to specify the parents for clocks with EXSRC bits.

Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
---
 .../bindings/clock/renesas,cpg-div6-clocks.txt         | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/clock/renesas,cpg-div6-clocks.txt b/Documentation/devicetree/bindings/clock/renesas,cpg-div6-clocks.txt
index 952e373..054f65f 100644
--- a/Documentation/devicetree/bindings/clock/renesas,cpg-div6-clocks.txt
+++ b/Documentation/devicetree/bindings/clock/renesas,cpg-div6-clocks.txt
@@ -7,11 +7,16 @@ to 64.
 Required Properties:
 
   - compatible: Must be one of the following
+    - "renesas,r8a73a4-div6-clock" for R8A73A4 (R-Mobile APE6) DIV6 clocks
+    - "renesas,r8a7740-div6-clock" for R8A7740 (R-Mobile A1) DIV6 clocks
     - "renesas,r8a7790-div6-clock" for R8A7790 (R-Car H2) DIV6 clocks
     - "renesas,r8a7791-div6-clock" for R8A7791 (R-Car M2) DIV6 clocks
+    - "renesas,sh73a0-div6-clock" for SH73A0 (SH-Mobile AG5) DIV6 clocks
     - "renesas,cpg-div6-clock" for generic DIV6 clocks
   - reg: Base address and length of the memory resource used by the DIV6 clock
-  - clocks: Reference to the parent clock
+  - clocks: Reference to the parent clock(s); either one, four, or eight
+    clocks must be specified.  For clocks with multiple parents, invalid
+    settings must be specified as "<0>".
   - #clock-cells: Must be 0
   - clock-output-names: The name of the clock as a free-form string
 
@@ -19,10 +24,11 @@ Required Properties:
 Example
 -------
 
-	sd2_clk: sd2_clk@e6150078 {
-		compatible = "renesas,r8a7790-div6-clock", "renesas,cpg-div6-clock";
-		reg = <0 0xe6150078 0 4>;
-		clocks = <&pll1_div2_clk>;
+	sdhi2_clk: sdhi2_clk@e615007c {
+		compatible = "renesas,r8a73a4-div6-clock", "renesas,cpg-div6-clock";
+		reg = <0 0xe615007c 0 4>;
+		clocks = <&pll1_div2_clk>, <&cpg_clocks R8A73A4_CLK_PLL2S>,
+			 <0>, <&extal2_clk>;
 		#clock-cells = <0>;
-		clock-output-names = "sd2";
+		clock-output-names = "sdhi2ck";
 	};
-- 
1.8.4.5

Re: [PATCH v7 0/2] clk: shmobile: DIV6 clock variable parent support

[Geert Uytterhoeven]

On Fri, Nov 7, 2014 at 4:51 PM, Ulrich Hecht
<ulrich.hecht+renesas@gmail.com> wrote:
>  .../bindings/clock/renesas,cpg-div6-clocks.txt     |  18 ++--
>  drivers/clk/shmobile/clk-div6.c                    | 113 ++++++++++++++++++---
>  2 files changed, 113 insertions(+), 18 deletions(-)

For the series:
Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>

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

Re: [PATCH v7 0/2] clk: shmobile: DIV6 clock variable parent support

[Magnus Damm]

On Mon, Nov 10, 2014 at 6:16 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> On Fri, Nov 7, 2014 at 4:51 PM, Ulrich Hecht
> <ulrich.hecht+renesas@gmail.com> wrote:
>>  .../bindings/clock/renesas,cpg-div6-clocks.txt     |  18 ++--
>>  drivers/clk/shmobile/clk-div6.c                    | 113 ++++++++++++++++++---
>>  2 files changed, 113 insertions(+), 18 deletions(-)
>
> For the series:
> Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>

Thanks, Geert!

>From my side this series "[PATCH v7 0/2] clk: shmobile: DIV6 clock
variable parent support" looks quite finalized. Would it be possible
for you to pick this up into the renesas-driver git repo? It would
also be great if you could lend a hand and make sure this series gets
picked up by Mike.

Cheers,

/ magnus

Re: [PATCH v7 0/2] clk: shmobile: DIV6 clock variable parent support

[Geert Uytterhoeven]

Hi Magnus,

On Tue, Nov 11, 2014 at 11:16 AM, Magnus Damm <magnus.damm@gmail.com> wrote:
> On Mon, Nov 10, 2014 at 6:16 PM, Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
>> On Fri, Nov 7, 2014 at 4:51 PM, Ulrich Hecht
>> <ulrich.hecht+renesas@gmail.com> wrote:
>>>  .../bindings/clock/renesas,cpg-div6-clocks.txt     |  18 ++--
>>>  drivers/clk/shmobile/clk-div6.c                    | 113 ++++++++++++++++++---
>>>  2 files changed, 113 insertions(+), 18 deletions(-)
>>
>> For the series:
>> Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>
>
> Thanks, Geert!
>
> From my side this series "[PATCH v7 0/2] clk: shmobile: DIV6 clock
> variable parent support" looks quite finalized. Would it be possible

Does this mean I can add your acked-by?

Thanks!

> for you to pick this up into the renesas-driver git repo? It would
> also be great if you could lend a hand and make sure this series gets
> picked up by Mike.

Will do.

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

Re: [PATCH v7 0/2] clk: shmobile: DIV6 clock variable parent support

[Magnus Damm]

Hi Geert,

On Wed, Nov 12, 2014 at 7:56 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> Hi Magnus,
>
> On Tue, Nov 11, 2014 at 11:16 AM, Magnus Damm <magnus.damm@gmail.com> wrote:
>> On Mon, Nov 10, 2014 at 6:16 PM, Geert Uytterhoeven
>> <geert@linux-m68k.org> wrote:
>>> On Fri, Nov 7, 2014 at 4:51 PM, Ulrich Hecht
>>> <ulrich.hecht+renesas@gmail.com> wrote:
>>>>  .../bindings/clock/renesas,cpg-div6-clocks.txt     |  18 ++--
>>>>  drivers/clk/shmobile/clk-div6.c                    | 113 ++++++++++++++++++---
>>>>  2 files changed, 113 insertions(+), 18 deletions(-)
>>>
>>> For the series:
>>> Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>
>>
>> Thanks, Geert!
>>
>> From my side this series "[PATCH v7 0/2] clk: shmobile: DIV6 clock
>> variable parent support" looks quite finalized. Would it be possible
>
> Does this mean I can add your acked-by?

My apologies for late reply and not being more clear. Feel free to add
my Acked-by for this series. No need to spend extra cycles and
reworking anything if you have queued up the patches already though.

> Thanks!
>
>> for you to pick this up into the renesas-driver git repo? It would
>> also be great if you could lend a hand and make sure this series gets
>> picked up by Mike.
>
> Will do.

Great, thanks!

/ magnus

Re: [PATCH v7 0/2] clk: shmobile: DIV6 clock variable parent support

[Geert Uytterhoeven]

Hi Magnus,

On Fri, Nov 21, 2014 at 6:54 AM, Magnus Damm <magnus.damm@gmail.com> wrote:
>>> From my side this series "[PATCH v7 0/2] clk: shmobile: DIV6 clock
>>> variable parent support" looks quite finalized. Would it be possible
>>
>> Does this mean I can add your acked-by?
>
> My apologies for late reply and not being more clear. Feel free to add
> my Acked-by for this series. No need to spend extra cycles and
> reworking anything if you have queued up the patches already though.

Too late. Pushed, and already pulled by Mike, but not yet visible in
clk-next.

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

Re: [PATCH v7 0/2] clk: shmobile: DIV6 clock variable parent support

[Mike Turquette]

Quoting Geert Uytterhoeven (2014-11-20 23:38:26)
> Hi Magnus,
> 
> On Fri, Nov 21, 2014 at 6:54 AM, Magnus Damm <magnus.damm@gmail.com> wrote:
> >>> From my side this series "[PATCH v7 0/2] clk: shmobile: DIV6 clock
> >>> variable parent support" looks quite finalized. Would it be possible
> >>
> >> Does this mean I can add your acked-by?
> >
> > My apologies for late reply and not being more clear. Feel free to add
> > my Acked-by for this series. No need to spend extra cycles and
> > reworking anything if you have queued up the patches already though.
> 
> Too late. Pushed, and already pulled by Mike, but not yet visible in
> clk-next.

I also saw this email too late. I would have fixed it up locally but now
it is out in the wild and I can't rebase it :-/

Regards,
Mike

> 
> 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

Re: [PATCH v7 0/2] clk: shmobile: DIV6 clock variable parent support

[Geert Uytterhoeven]

Hi Mike,

On Tue, Nov 25, 2014 at 2:49 AM, Mike Turquette <mturquette@linaro.org> wrote:
> Quoting Geert Uytterhoeven (2014-11-20 23:38:26)
>> On Fri, Nov 21, 2014 at 6:54 AM, Magnus Damm <magnus.damm@gmail.com> wrote:
>> >>> From my side this series "[PATCH v7 0/2] clk: shmobile: DIV6 clock
>> >>> variable parent support" looks quite finalized. Would it be possible
>> >>
>> >> Does this mean I can add your acked-by?
>> >
>> > My apologies for late reply and not being more clear. Feel free to add
>> > my Acked-by for this series. No need to spend extra cycles and
>> > reworking anything if you have queued up the patches already though.
>>
>> Too late. Pushed, and already pulled by Mike, but not yet visible in
>> clk-next.
>
> I also saw this email too late. I would have fixed it up locally but now
> it is out in the wild and I can't rebase it :-/

I don't think it's out in the wild yet, unless I'm looking at the wrong place?
https://git.linaro.org/people/mike.turquette/linux.git/shortlog/refs/heads/clk-next

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

Re: [PATCH v7 1/2] clk: shmobile: div6: support selectable-input clocks

[Laurent Pinchart]

Hi Ulrich,

Thank you for the patch.

On Friday 07 November 2014 16:51:07 Ulrich Hecht wrote:
> Support for setting the parent at initialization time based on the current
> hardware configuration in DIV6 clocks with selectable parents as found in
> the r8a73a4, r8a7740, sh73a0, and other SoCs.
> 
> Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
> ---
>  drivers/clk/shmobile/clk-div6.c | 113 ++++++++++++++++++++++++++++++++-----
>  1 file changed, 101 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/clk/shmobile/clk-div6.c
> b/drivers/clk/shmobile/clk-div6.c index f065f69..639241e 100644
> --- a/drivers/clk/shmobile/clk-div6.c
> +++ b/drivers/clk/shmobile/clk-div6.c

[snip]

> +static int cpg_div6_clock_set_parent(struct clk_hw *hw, u8 index)
> +{
> +	struct div6_clock *clock = to_div6_clock(hw);
> +	u8 hw_index;
> +	u32 mask;
> +
> +	if (index >= __clk_get_num_parents(hw->clk))
> +		return -EINVAL;

Should this check be moved to the CCF core ?

> +	mask = ~((BIT(clock->src_width) - 1) << clock->src_shift);
> +	hw_index = clock->parents[index];
> +
> +	clk_writel((clk_readl(clock->reg) & mask) |
> +		(hw_index << clock->src_shift), clock->reg);
> 
>  	return 0;
>  }

[snip]

-- 
Regards,

Laurent Pinchart