[PATCH 0/2] clk: shmobile rcar-gen2 fixes

[PATCH 0/2] clk: shmobile rcar-gen2 fixes

[Laurent Pinchart]

Hello,

These two patches fix the clock parents and qspi clock divisor in the
rcar-gen2 clock driver.

Geert, could you please verify that the series fixes your QSPI clock issues
with the Koelsch board ?

Laurent Pinchart (2):
  clk: shmobile: rcar-gen2: Fix clock parent all non-PLL clocks
  clk: shmobile: rcar-gen2: Fix qspi divisor

 drivers/clk/shmobile/clk-rcar-gen2.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

-- 
Regards,

Laurent Pinchart

[PATCH 1/2] clk: shmobile: rcar-gen2: Fix clock parent all non-PLL clocks

[Laurent Pinchart]

The lb, qspi, sdh, sd0 and sd1 clocks have the PLL1 (divided by 2) as
their parent, not the main clock. Fix it.

Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/clk/shmobile/clk-rcar-gen2.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/shmobile/clk-rcar-gen2.c b/drivers/clk/shmobile/clk-rcar-gen2.c
index a59ec21..8c7bcbd 100644
--- a/drivers/clk/shmobile/clk-rcar-gen2.c
+++ b/drivers/clk/shmobile/clk-rcar-gen2.c
@@ -186,7 +186,7 @@ rcar_gen2_cpg_register_clock(struct device_node *np, struct rcar_gen2_cpg *cpg,
 			     const char *name)
 {
 	const struct clk_div_table *table = NULL;
-	const char *parent_name = "main";
+	const char *parent_name;
 	unsigned int shift;
 	unsigned int mult = 1;
 	unsigned int div = 1;
@@ -201,23 +201,31 @@ rcar_gen2_cpg_register_clock(struct device_node *np, struct rcar_gen2_cpg *cpg,
 		 * the multiplier value.
 		 */
 		u32 value = clk_readl(cpg->reg + CPG_PLL0CR);
+		parent_name = "main";
 		mult = ((value >> 24) & ((1 << 7) - 1)) + 1;
 	} else if (!strcmp(name, "pll1")) {
+		parent_name = "main";
 		mult = config->pll1_mult / 2;
 	} else if (!strcmp(name, "pll3")) {
+		parent_name = "main";
 		mult = config->pll3_mult;
 	} else if (!strcmp(name, "lb")) {
+		parent_name = "pll1_div2";
 		div = cpg_mode & BIT(18) ? 36 : 24;
 	} else if (!strcmp(name, "qspi")) {
+		parent_name = "pll1_div2";
 		div = (cpg_mode & (BIT(3) | BIT(2) | BIT(1))) = BIT(2)
 		    ? 16 : 20;
 	} else if (!strcmp(name, "sdh")) {
+		parent_name = "pll1_div2";
 		table = cpg_sdh_div_table;
 		shift = 8;
 	} else if (!strcmp(name, "sd0")) {
+		parent_name = "pll1_div2";
 		table = cpg_sd01_div_table;
 		shift = 4;
 	} else if (!strcmp(name, "sd1")) {
+		parent_name = "pll1_div2";
 		table = cpg_sd01_div_table;
 		shift = 0;
 	} else if (!strcmp(name, "z")) {
-- 
1.8.3.2

[PATCH 2/2] clk: shmobile: rcar-gen2: Fix qspi divisor

[Laurent Pinchart]

The qspi clock divisor is incorrectly set to twice the value it should
have, possibly because it has been computed based on PLL1 as the clock
parent instead of PLL1 / 2 (the datasheets specifies the qspi nominal
frequencies, not the divisor values). Fix it.

Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/clk/shmobile/clk-rcar-gen2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/shmobile/clk-rcar-gen2.c b/drivers/clk/shmobile/clk-rcar-gen2.c
index 8c7bcbd..dd272a0 100644
--- a/drivers/clk/shmobile/clk-rcar-gen2.c
+++ b/drivers/clk/shmobile/clk-rcar-gen2.c
@@ -215,7 +215,7 @@ rcar_gen2_cpg_register_clock(struct device_node *np, struct rcar_gen2_cpg *cpg,
 	} else if (!strcmp(name, "qspi")) {
 		parent_name = "pll1_div2";
 		div = (cpg_mode & (BIT(3) | BIT(2) | BIT(1))) = BIT(2)
-		    ? 16 : 20;
+		    ? 8 : 10;
 	} else if (!strcmp(name, "sdh")) {
 		parent_name = "pll1_div2";
 		table = cpg_sdh_div_table;
-- 
1.8.3.2

Re: [PATCH 0/2] clk: shmobile rcar-gen2 fixes

[Geert Uytterhoeven]

On Tue, Jan 7, 2014 at 5:47 PM, Laurent Pinchart
<laurent.pinchart+renesas@ideasonboard.com> wrote:
> Geert, could you please verify that the series fixes your QSPI clock issues
> with the Koelsch board ?
>
> Laurent Pinchart (2):
>   clk: shmobile: rcar-gen2: Fix clock parent all non-PLL clocks
>   clk: shmobile: rcar-gen2: Fix qspi divisor

Thanks, both:

Tested-by: Geert Uytterhoeven <geert@linux-m68k.org>

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 0/2] clk: shmobile rcar-gen2 fixes

[Laurent Pinchart]

Hi Geert,

On Tuesday 07 January 2014 17:59:01 Geert Uytterhoeven wrote:
> On Tue, Jan 7, 2014 at 5:47 PM, Laurent Pinchart wrote:
> > Geert, could you please verify that the series fixes your QSPI clock
> > issues with the Koelsch board ?
> > 
> > Laurent Pinchart (2):
> >   clk: shmobile: rcar-gen2: Fix clock parent all non-PLL clocks
> >   clk: shmobile: rcar-gen2: Fix qspi divisor
> 
> Thanks, both:
> 
> Tested-by: Geert Uytterhoeven <geert@linux-m68k.org>

Thank you.

Mike, could you please pick those patches up for v3.14 ?

-- 
Regards,

Laurent Pinchart

Re: [PATCH 0/2] clk: shmobile rcar-gen2 fixes

[Simon Horman]

On Tue, Jan 07, 2014 at 05:59:01PM +0100, Geert Uytterhoeven wrote:
> On Tue, Jan 7, 2014 at 5:47 PM, Laurent Pinchart
> <laurent.pinchart+renesas@ideasonboard.com> wrote:
> > Geert, could you please verify that the series fixes your QSPI clock issues
> > with the Koelsch board ?
> >
> > Laurent Pinchart (2):
> >   clk: shmobile: rcar-gen2: Fix clock parent all non-PLL clocks
> >   clk: shmobile: rcar-gen2: Fix qspi divisor
> 
> Thanks, both:
> 
> Tested-by: Geert Uytterhoeven <geert@linux-m68k.org>

Acked-by: Simon Horman <horms+renesas@verge.net.au>

Re: [PATCH 0/2] clk: shmobile rcar-gen2 fixes

[Laurent Pinchart]

Hi Mike,

On Wednesday 08 January 2014 00:06:39 Laurent Pinchart wrote:
> On Tuesday 07 January 2014 17:59:01 Geert Uytterhoeven wrote:
> > On Tue, Jan 7, 2014 at 5:47 PM, Laurent Pinchart wrote:
> > > Geert, could you please verify that the series fixes your QSPI clock
> > > issues with the Koelsch board ?
> > > 
> > > Laurent Pinchart (2):
> > >   clk: shmobile: rcar-gen2: Fix clock parent all non-PLL clocks
> > >   clk: shmobile: rcar-gen2: Fix qspi divisor
> > 
> > Thanks, both:
> > 
> > Tested-by: Geert Uytterhoeven <geert@linux-m68k.org>
> 
> Thank you.
> 
> Mike, could you please pick those patches up for v3.14 ?

Ping ?

-- 
Regards,

Laurent Pinchart

Re: [PATCH 0/2] clk: shmobile rcar-gen2 fixes

[Laurent Pinchart]

Mike,

Ping ?

v3.14-rc1 is out, and those are bug fixes.

On Wednesday 08 January 2014 00:06:39 Laurent Pinchart wrote:
> On Tuesday 07 January 2014 17:59:01 Geert Uytterhoeven wrote:
> > On Tue, Jan 7, 2014 at 5:47 PM, Laurent Pinchart wrote:
> > > Geert, could you please verify that the series fixes your QSPI clock
> > > issues with the Koelsch board ?
> > > 
> > > Laurent Pinchart (2):
> > >   clk: shmobile: rcar-gen2: Fix clock parent all non-PLL clocks
> > >   clk: shmobile: rcar-gen2: Fix qspi divisor
> > 
> > Thanks, both:
> > 
> > Tested-by: Geert Uytterhoeven <geert@linux-m68k.org>
> 
> Thank you.
> 
> Mike, could you please pick those patches up for v3.14 ?

-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/2] clk: shmobile: rcar-gen2: Fix clock parent all non-PLL clocks

[Ben Dooks]

On 07/01/14 16:47, Laurent Pinchart wrote:
> The lb, qspi, sdh, sd0 and sd1 clocks have the PLL1 (divided by 2) as
> their parent, not the main clock. Fix it.

William Towle has already sent a patch to move this to device tree
which I think is a better solution for this. Any chance of commenting
on which to take please.

> Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>   drivers/clk/shmobile/clk-rcar-gen2.c | 10 +++++++++-
>   1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/clk/shmobile/clk-rcar-gen2.c b/drivers/clk/shmobile/clk-rcar-gen2.c
> index a59ec21..8c7bcbd 100644
> --- a/drivers/clk/shmobile/clk-rcar-gen2.c
> +++ b/drivers/clk/shmobile/clk-rcar-gen2.c
> @@ -186,7 +186,7 @@ rcar_gen2_cpg_register_clock(struct device_node *np, struct rcar_gen2_cpg *cpg,
>   			     const char *name)
>   {
>   	const struct clk_div_table *table = NULL;
> -	const char *parent_name = "main";
> +	const char *parent_name;
>   	unsigned int shift;
>   	unsigned int mult = 1;
>   	unsigned int div = 1;
> @@ -201,23 +201,31 @@ rcar_gen2_cpg_register_clock(struct device_node *np, struct rcar_gen2_cpg *cpg,
>   		 * the multiplier value.
>   		 */
>   		u32 value = clk_readl(cpg->reg + CPG_PLL0CR);
> +		parent_name = "main";
>   		mult = ((value >> 24) & ((1 << 7) - 1)) + 1;
>   	} else if (!strcmp(name, "pll1")) {
> +		parent_name = "main";
>   		mult = config->pll1_mult / 2;
>   	} else if (!strcmp(name, "pll3")) {
> +		parent_name = "main";
>   		mult = config->pll3_mult;
>   	} else if (!strcmp(name, "lb")) {
> +		parent_name = "pll1_div2";
>   		div = cpg_mode & BIT(18) ? 36 : 24;
>   	} else if (!strcmp(name, "qspi")) {
> +		parent_name = "pll1_div2";
>   		div = (cpg_mode & (BIT(3) | BIT(2) | BIT(1))) = BIT(2)
>   		    ? 16 : 20;
>   	} else if (!strcmp(name, "sdh")) {
> +		parent_name = "pll1_div2";
>   		table = cpg_sdh_div_table;
>   		shift = 8;
>   	} else if (!strcmp(name, "sd0")) {
> +		parent_name = "pll1_div2";
>   		table = cpg_sd01_div_table;
>   		shift = 4;
>   	} else if (!strcmp(name, "sd1")) {
> +		parent_name = "pll1_div2";
>   		table = cpg_sd01_div_table;
>   		shift = 0;
>   	} else if (!strcmp(name, "z")) {
>


-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

Re: [PATCH 1/2] clk: shmobile: rcar-gen2: Fix clock parent all non-PLL clocks

[Laurent Pinchart]

Hi Ben,

On Wednesday 05 February 2014 10:34:49 Ben Dooks wrote:
> On 07/01/14 16:47, Laurent Pinchart wrote:
> > The lb, qspi, sdh, sd0 and sd1 clocks have the PLL1 (divided by 2) as
> > their parent, not the main clock. Fix it.
> 
> William Towle has already sent a patch to move this to device tree
> which I think is a better solution for this.

I actually disagree. The CPG is an IP core that generates a bunch of clocks 
from a single external parent. The fact that clocks are not flat but organized 
as a tree internally is an internal property of the CPG, and I prefer keeping 
it that way instead of exposing it in the device tree.

> Any chance of commenting on which to take please.
> 
> > Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
> > Signed-off-by: Laurent Pinchart
> > <laurent.pinchart+renesas@ideasonboard.com>
> > ---
> > 
> >   drivers/clk/shmobile/clk-rcar-gen2.c | 10 +++++++++-
> >   1 file changed, 9 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/clk/shmobile/clk-rcar-gen2.c
> > b/drivers/clk/shmobile/clk-rcar-gen2.c index a59ec21..8c7bcbd 100644
> > --- a/drivers/clk/shmobile/clk-rcar-gen2.c
> > +++ b/drivers/clk/shmobile/clk-rcar-gen2.c
> > @@ -186,7 +186,7 @@ rcar_gen2_cpg_register_clock(struct device_node *np,
> > struct rcar_gen2_cpg *cpg,> 
> >   			     const char *name)
> >   {
> >   	const struct clk_div_table *table = NULL;
> > -	const char *parent_name = "main";
> > +	const char *parent_name;
> >   	unsigned int shift;
> >   	unsigned int mult = 1;
> >   	unsigned int div = 1;
> > @@ -201,23 +201,31 @@ rcar_gen2_cpg_register_clock(struct device_node *np,
> > struct rcar_gen2_cpg *cpg,> 
> >   		 * the multiplier value.
> >   		 */
> >   		u32 value = clk_readl(cpg->reg + CPG_PLL0CR);
> > +		parent_name = "main";
> >   		mult = ((value >> 24) & ((1 << 7) - 1)) + 1;
> >   	} else if (!strcmp(name, "pll1")) {
> > +		parent_name = "main";
> >   		mult = config->pll1_mult / 2;
> >   	} else if (!strcmp(name, "pll3")) {
> > +		parent_name = "main";
> >   		mult = config->pll3_mult;
> >   	} else if (!strcmp(name, "lb")) {
> > +		parent_name = "pll1_div2";
> >   		div = cpg_mode & BIT(18) ? 36 : 24;
> >   	} else if (!strcmp(name, "qspi")) {
> > +		parent_name = "pll1_div2";
> >   		div = (cpg_mode & (BIT(3) | BIT(2) | BIT(1))) = BIT(2)
> >   		    ? 16 : 20;
> >   	} else if (!strcmp(name, "sdh")) {
> > +		parent_name = "pll1_div2";
> >   		table = cpg_sdh_div_table;
> >   		shift = 8;
> >   	} else if (!strcmp(name, "sd0")) {
> > +		parent_name = "pll1_div2";
> >   		table = cpg_sd01_div_table;
> >   		shift = 4;
> >   	} else if (!strcmp(name, "sd1")) {
> > +		parent_name = "pll1_div2";
> >   		table = cpg_sd01_div_table;
> >   		shift = 0;
> >   	} else if (!strcmp(name, "z")) {

-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/2] clk: shmobile: rcar-gen2: Fix clock parent all non-PLL clocks

[Ben Dooks]

On 05/02/14 10:51, Laurent Pinchart wrote:
> Hi Ben,
>
> On Wednesday 05 February 2014 10:34:49 Ben Dooks wrote:
>> On 07/01/14 16:47, Laurent Pinchart wrote:
>>> The lb, qspi, sdh, sd0 and sd1 clocks have the PLL1 (divided by 2) as
>>> their parent, not the main clock. Fix it.
>>
>> William Towle has already sent a patch to move this to device tree
>> which I think is a better solution for this.
>
> I actually disagree. The CPG is an IP core that generates a bunch of clocks
> from a single external parent. The fact that clocks are not flat but organized
> as a tree internally is an internal property of the CPG, and I prefer keeping
> it that way instead of exposing it in the device tree.

If you feel that the block is unlikely to change if it gets
re-used then that'll be fine. We can always revisit the changes
if needed.

-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius