[PATCH] Provide r8a7795 FDP1 clocks

[PATCH] Provide r8a7795 FDP1 clocks

[Kieran Bingham]

Hi All,

I've now got the FDP1 powered up and reading registers, and the cpg-mssr
is updated to support this. However, we did not have a mapping to identify
the correct clock parent. For now I have used the R8A7795_CLK_S2D1, but
does anyone have a clock tree diagram to verify this, or identify the
correct parent please?

Kieran Bingham (1):
  clk: renesas: Provide r8a7795 FDP1 clocks

 drivers/clk/renesas/r8a7795-cpg-mssr.c | 3 +++
 1 file changed, 3 insertions(+)

-- 
2.5.0

[PATCH] clk: renesas: Provide r8a7795 FDP1 clocks

[Kieran Bingham]

Add the clocks with a parent of S2D1 for now, until the correct
parentage is identified
---
 drivers/clk/renesas/r8a7795-cpg-mssr.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/clk/renesas/r8a7795-cpg-mssr.c b/drivers/clk/renesas/r8a7795-cpg-mssr.c
index 6af7f5b6e824..5fdc6bd840a4 100644
--- a/drivers/clk/renesas/r8a7795-cpg-mssr.c
+++ b/drivers/clk/renesas/r8a7795-cpg-mssr.c
@@ -128,6 +128,9 @@ static const struct cpg_core_clk r8a7795_core_clks[] __initconst = {
 };
 
 static const struct mssr_mod_clk r8a7795_mod_clks[] __initconst = {
+	DEF_MOD("fdp1-ch2",		 117,	R8A7795_CLK_S2D1),
+	DEF_MOD("fdp1-ch1",		 118,	R8A7795_CLK_S2D1),
+	DEF_MOD("fdp1-ch0",		 119,	R8A7795_CLK_S2D1),
 	DEF_MOD("scif5",		 202,	R8A7795_CLK_S3D4),
 	DEF_MOD("scif4",		 203,	R8A7795_CLK_S3D4),
 	DEF_MOD("scif3",		 204,	R8A7795_CLK_S3D4),
-- 
2.5.0

Re: [PATCH] clk: renesas: Provide r8a7795 FDP1 clocks

[Geert Uytterhoeven]

Hi Kieran,

On Tue, May 17, 2016 at 12:03 PM, Kieran Bingham <kieran@ksquared.org.uk> wrote:
> Add the clocks with a parent of S2D1 for now, until the correct
> parentage is identified

Thanks for your patch!

> --- a/drivers/clk/renesas/r8a7795-cpg-mssr.c
> +++ b/drivers/clk/renesas/r8a7795-cpg-mssr.c
> @@ -128,6 +128,9 @@ static const struct cpg_core_clk r8a7795_core_clks[] __initconst = {
>  };
>
>  static const struct mssr_mod_clk r8a7795_mod_clks[] __initconst = {
> +       DEF_MOD("fdp1-ch2",              117,   R8A7795_CLK_S2D1),
> +       DEF_MOD("fdp1-ch1",              118,   R8A7795_CLK_S2D1),
> +       DEF_MOD("fdp1-ch0",              119,   R8A7795_CLK_S2D1),

I would call them "fdp1-2" etc., to match the documentation for the various
Module Stop registers.

Apart from that:
Reviewed-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] clk: renesas: Provide r8a7795 FDP1 clocks

[Kieran Bingham]

Thanks Geert,

On 17/05/16 11:25, Geert Uytterhoeven wrote:
> Hi Kieran,
> 
> On Tue, May 17, 2016 at 12:03 PM, Kieran Bingham <kieran@ksquared.org.uk> wrote:
>> Add the clocks with a parent of S2D1 for now, until the correct
>> parentage is identified
> 
> Thanks for your patch!
> 
>> --- a/drivers/clk/renesas/r8a7795-cpg-mssr.c
>> +++ b/drivers/clk/renesas/r8a7795-cpg-mssr.c
>> @@ -128,6 +128,9 @@ static const struct cpg_core_clk r8a7795_core_clks[] __initconst = {
>>  };
>>
>>  static const struct mssr_mod_clk r8a7795_mod_clks[] __initconst = {
>> +       DEF_MOD("fdp1-ch2",              117,   R8A7795_CLK_S2D1),
>> +       DEF_MOD("fdp1-ch1",              118,   R8A7795_CLK_S2D1),
>> +       DEF_MOD("fdp1-ch0",              119,   R8A7795_CLK_S2D1),
> 
> I would call them "fdp1-2" etc., to match the documentation for the various
> Module Stop registers.

Ok. I've adjusted the names locally, and I'll await
confirmation/correction on the clock parents before a resubmit with
updated commit-log/sign-off.

I presume patches can be integrated as soon as they are wholly
independent? i.e. this update, and the DTS updates for the FDP1, then
the FDP1 driver itself? or would it be preferred to batch the whole lot
up into one set?

Possibly just as well to keep the DT, and driver close together...

> Apart from that:
> Reviewed-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
> 

-- 
Regards

Kieran Bingham

Re: [PATCH] clk: renesas: Provide r8a7795 FDP1 clocks

[Laurent Pinchart]

Hello Kieran,

On Tuesday 17 May 2016 12:25:37 Geert Uytterhoeven wrote:
> On Tue, May 17, 2016 at 12:03 PM, Kieran Bingham wrote:
> > Add the clocks with a parent of S2D1 for now, until the correct
> > parentage is identified
> 
> Thanks for your patch!
> 
> > --- a/drivers/clk/renesas/r8a7795-cpg-mssr.c
> > +++ b/drivers/clk/renesas/r8a7795-cpg-mssr.c
> > @@ -128,6 +128,9 @@ static const struct cpg_core_clk r8a7795_core_clks[]
> > __initconst = {> 
> >  };
> >  
> >  static const struct mssr_mod_clk r8a7795_mod_clks[] __initconst = {
> > 
> > +       DEF_MOD("fdp1-ch2",              117,   R8A7795_CLK_S2D1),
> > +       DEF_MOD("fdp1-ch1",              118,   R8A7795_CLK_S2D1),
> > +       DEF_MOD("fdp1-ch0",              119,   R8A7795_CLK_S2D1),
> 
> I would call them "fdp1-2" etc., to match the documentation for the various
> Module Stop registers.
> 
> Apart from that:
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

After fixing that (and double-checking the clock parent),

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

-- 
Regards,

Laurent Pinchart

Re: [PATCH] clk: renesas: Provide r8a7795 FDP1 clocks

[Geert Uytterhoeven]

Hi Kieran,

On Tue, May 17, 2016 at 3:03 PM, Kieran Bingham <kieran@bingham.xyz> wrote:
> On 17/05/16 11:25, Geert Uytterhoeven wrote:
>> On Tue, May 17, 2016 at 12:03 PM, Kieran Bingham <kieran@ksquared.org.uk> wrote:
>>> Add the clocks with a parent of S2D1 for now, until the correct
>>> parentage is identified
>>
>> Thanks for your patch!
>>
>>> --- a/drivers/clk/renesas/r8a7795-cpg-mssr.c
>>> +++ b/drivers/clk/renesas/r8a7795-cpg-mssr.c
>>> @@ -128,6 +128,9 @@ static const struct cpg_core_clk r8a7795_core_clks[] __initconst = {
>>>  };
>>>
>>>  static const struct mssr_mod_clk r8a7795_mod_clks[] __initconst = {
>>> +       DEF_MOD("fdp1-ch2",              117,   R8A7795_CLK_S2D1),
>>> +       DEF_MOD("fdp1-ch1",              118,   R8A7795_CLK_S2D1),
>>> +       DEF_MOD("fdp1-ch0",              119,   R8A7795_CLK_S2D1),
>>
>> I would call them "fdp1-2" etc., to match the documentation for the various
>> Module Stop registers.
>
> Ok. I've adjusted the names locally, and I'll await
> confirmation/correction on the clock parents before a resubmit with
> updated commit-log/sign-off.

OK.

> I presume patches can be integrated as soon as they are wholly
> independent? i.e. this update, and the DTS updates for the FDP1, then
> the FDP1 driver itself? or would it be preferred to batch the whole lot
> up into one set?
>
> Possibly just as well to keep the DT, and driver close together...

It's easier to apply if pieces destined for different maintainers are posted
separately.

I can take the clock update as soon as the parent clock has been determined.
The FDP driver and its bindings should be submitted to the media maintainer
(please CC linux-renesas-soc).
Simon will take the DT updates as soon as the FDP bindings have been accepted.

If needed, I can include things earlier in renesas-drivers.

Thanks!

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] clk: renesas: Provide r8a7795 FDP1 clocks

[Laurent Pinchart]

Hi Kieran,

On Tuesday 17 May 2016 14:03:41 Kieran Bingham wrote:
> On 17/05/16 11:25, Geert Uytterhoeven wrote:
> > On Tue, May 17, 2016 at 12:03 PM, Kieran Bingham wrote:
> >> Add the clocks with a parent of S2D1 for now, until the correct
> >> parentage is identified
> > 
> > Thanks for your patch!
> > 
> >> --- a/drivers/clk/renesas/r8a7795-cpg-mssr.c
> >> +++ b/drivers/clk/renesas/r8a7795-cpg-mssr.c
> >> @@ -128,6 +128,9 @@ static const struct cpg_core_clk r8a7795_core_clks[]
> >> __initconst = {
> >>  };
> >>  
> >>  static const struct mssr_mod_clk r8a7795_mod_clks[] __initconst = {
> >> +       DEF_MOD("fdp1-ch2",              117,   R8A7795_CLK_S2D1),
> >> +       DEF_MOD("fdp1-ch1",              118,   R8A7795_CLK_S2D1),
> >> +       DEF_MOD("fdp1-ch0",              119,   R8A7795_CLK_S2D1),
> > 
> > I would call them "fdp1-2" etc., to match the documentation for the
> > various Module Stop registers.
> 
> Ok. I've adjusted the names locally, and I'll await
> confirmation/correction on the clock parents before a resubmit with
> updated commit-log/sign-off.
> 
> I presume patches can be integrated as soon as they are wholly
> independent? i.e. this update, and the DTS updates for the FDP1, then
> the FDP1 driver itself? or would it be preferred to batch the whole lot
> up into one set?

You can get the pieces merged separately, but the DTS updates need the 
corresponding DT bindings patch to be at least acked first.

> Possibly just as well to keep the DT, and driver close together...

-- 
Regards,

Laurent Pinchart