[PATCH 1/1] clk: renesas: cpg-mssr: r8a7796: add IMR clocks

[PATCH 1/1] clk: renesas: cpg-mssr: r8a7796: add IMR clocks

[Sergei Shtylyov]

Add the IMR[0-1] clocks to the R8A7796 CPG/MSSR driver.

Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

---
This patch is against the 'clk-next' branch of CLK group's 'linux.git' repo.

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

Index: linux/drivers/clk/renesas/r8a7796-cpg-mssr.c
===================================================================
--- linux.orig/drivers/clk/renesas/r8a7796-cpg-mssr.c
+++ linux/drivers/clk/renesas/r8a7796-cpg-mssr.c
@@ -179,6 +179,8 @@ static const struct mssr_mod_clk r8a7796
 	DEF_MOD("vin1",			 810,	R8A7796_CLK_S0D2),
 	DEF_MOD("vin0",			 811,	R8A7796_CLK_S0D2),
 	DEF_MOD("etheravb",		 812,	R8A7796_CLK_S0D6),
+	DEF_MOD("imr1",			 822,	R8A7796_CLK_S2D1),
+	DEF_MOD("imr0",			 823,	R8A7796_CLK_S2D1),
 	DEF_MOD("gpio7",		 905,	R8A7796_CLK_S3D4),
 	DEF_MOD("gpio6",		 906,	R8A7796_CLK_S3D4),
 	DEF_MOD("gpio5",		 907,	R8A7796_CLK_S3D4),

Re: [PATCH 1/1] clk: renesas: cpg-mssr: r8a7796: add IMR clocks

[Geert Uytterhoeven]

Hi Sergei,

On Sat, Feb 18, 2017 at 10:39 PM, Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
> Add the IMR[0-1] clocks to the R8A7796 CPG/MSSR driver.
>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

> --- linux.orig/drivers/clk/renesas/r8a7796-cpg-mssr.c
> +++ linux/drivers/clk/renesas/r8a7796-cpg-mssr.c
> @@ -179,6 +179,8 @@ static const struct mssr_mod_clk r8a7796
>         DEF_MOD("vin1",                  810,   R8A7796_CLK_S0D2),
>         DEF_MOD("vin0",                  811,   R8A7796_CLK_S0D2),
>         DEF_MOD("etheravb",              812,   R8A7796_CLK_S0D6),
> +       DEF_MOD("imr1",                  822,   R8A7796_CLK_S2D1),
> +       DEF_MOD("imr0",                  823,   R8A7796_CLK_S2D1),

According to R-Car Gen3 Hardware Users Manual Rev.0.53E, the parent
clock is S0D2, like on H3 ES2.0.

Will queue in clk-renesas-for-v4.12 with corrected parent clock.

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 1/1] clk: renesas: cpg-mssr: r8a7796: add IMR clocks

[Sergei Shtylyov]

On 02/20/2017 11:38 AM, Geert Uytterhoeven wrote:

>> Add the IMR[0-1] clocks to the R8A7796 CPG/MSSR driver.
>>
>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
>
>> --- linux.orig/drivers/clk/renesas/r8a7796-cpg-mssr.c
>> +++ linux/drivers/clk/renesas/r8a7796-cpg-mssr.c
>> @@ -179,6 +179,8 @@ static const struct mssr_mod_clk r8a7796
>>         DEF_MOD("vin1",                  810,   R8A7796_CLK_S0D2),
>>         DEF_MOD("vin0",                  811,   R8A7796_CLK_S0D2),
>>         DEF_MOD("etheravb",              812,   R8A7796_CLK_S0D6),
>> +       DEF_MOD("imr1",                  822,   R8A7796_CLK_S2D1),
>> +       DEF_MOD("imr0",                  823,   R8A7796_CLK_S2D1),
>
> According to R-Car Gen3 Hardware Users Manual Rev.0.53E, the parent
> clock is S0D2, like on H3 ES2.0.

    I only have 0.51E here, doesn't seem to contain much of the clock info...

> Will queue in clk-renesas-for-v4.12 with corrected parent clock.

    Thank you!

> Gr{oetje,eeting}s,
>
>                         Geert

MBR, Sergei

Re: [PATCH 1/1] clk: renesas: cpg-mssr: r8a7796: add IMR clocks

[Sergei Shtylyov]

On 02/20/2017 11:38 AM, Geert Uytterhoeven wrote:

>> Add the IMR[0-1] clocks to the R8A7796 CPG/MSSR driver.
>>
>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
>
>> --- linux.orig/drivers/clk/renesas/r8a7796-cpg-mssr.c
>> +++ linux/drivers/clk/renesas/r8a7796-cpg-mssr.c
>> @@ -179,6 +179,8 @@ static const struct mssr_mod_clk r8a7796
>>         DEF_MOD("vin1",                  810,   R8A7796_CLK_S0D2),
>>         DEF_MOD("vin0",                  811,   R8A7796_CLK_S0D2),
>>         DEF_MOD("etheravb",              812,   R8A7796_CLK_S0D6),
>> +       DEF_MOD("imr1",                  822,   R8A7796_CLK_S2D1),
>> +       DEF_MOD("imr0",                  823,   R8A7796_CLK_S2D1),
>
> According to R-Car Gen3 Hardware Users Manual Rev.0.53E, the parent
> clock is S0D2, like on H3 ES2.0.

    Not S0D4? I'd expect that what they call "register access" to be 
controlled by the MSSR bits...

> Will queue in clk-renesas-for-v4.12 with corrected parent clock.

    So, what have you used for 7795 and 7796?

> Gr{oetje,eeting}s,
>
>                         Geert

MBR, Sergei

Re: [PATCH 1/1] clk: renesas: cpg-mssr: r8a7796: add IMR clocks

[Geert Uytterhoeven]

Hi Sergei,

On Tue, Feb 21, 2017 at 7:54 PM, Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
> On 02/20/2017 11:38 AM, Geert Uytterhoeven wrote:
>>> Add the IMR[0-1] clocks to the R8A7796 CPG/MSSR driver.
>>>
>>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>>
>> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
>>
>>> --- linux.orig/drivers/clk/renesas/r8a7796-cpg-mssr.c
>>> +++ linux/drivers/clk/renesas/r8a7796-cpg-mssr.c
>>> @@ -179,6 +179,8 @@ static const struct mssr_mod_clk r8a7796
>>>         DEF_MOD("vin1",                  810,   R8A7796_CLK_S0D2),
>>>         DEF_MOD("vin0",                  811,   R8A7796_CLK_S0D2),
>>>         DEF_MOD("etheravb",              812,   R8A7796_CLK_S0D6),
>>> +       DEF_MOD("imr1",                  822,   R8A7796_CLK_S2D1),
>>> +       DEF_MOD("imr0",                  823,   R8A7796_CLK_S2D1),
>>
>>
>> According to R-Car Gen3 Hardware Users Manual Rev.0.53E, the parent
>> clock is S0D2, like on H3 ES2.0.
>
>    Not S0D4? I'd expect that what they call "register access" to be
> controlled by the MSSR bits...

Good question...

>> Will queue in clk-renesas-for-v4.12 with corrected parent clock.
>
>    So, what have you used for 7795 and 7796?

S2D1 for r8a7795 (H3 ES1.0)
S0D2 for r8a7796 (M3-W)

And I planned to use S0D2 for H3 ES2.0, too.

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 1/1] clk: renesas: cpg-mssr: r8a7796: add IMR clocks

[Sergei Shtylyov]

On 02/21/2017 10:39 PM, Geert Uytterhoeven wrote:

>> On 02/20/2017 11:38 AM, Geert Uytterhoeven wrote:
>>>> Add the IMR[0-1] clocks to the R8A7796 CPG/MSSR driver.
>>>>
>>>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>>>
>>> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
>>>
>>>> --- linux.orig/drivers/clk/renesas/r8a7796-cpg-mssr.c
>>>> +++ linux/drivers/clk/renesas/r8a7796-cpg-mssr.c
>>>> @@ -179,6 +179,8 @@ static const struct mssr_mod_clk r8a7796
>>>>         DEF_MOD("vin1",                  810,   R8A7796_CLK_S0D2),
>>>>         DEF_MOD("vin0",                  811,   R8A7796_CLK_S0D2),
>>>>         DEF_MOD("etheravb",              812,   R8A7796_CLK_S0D6),
>>>> +       DEF_MOD("imr1",                  822,   R8A7796_CLK_S2D1),
>>>> +       DEF_MOD("imr0",                  823,   R8A7796_CLK_S2D1),
>>>
>>>
>>> According to R-Car Gen3 Hardware Users Manual Rev.0.53E, the parent
>>> clock is S0D2, like on H3 ES2.0.
>>
>>    Not S0D4? I'd expect that what they call "register access" to be
>> controlled by the MSSR bits...
>
> Good question...
>
>>> Will queue in clk-renesas-for-v4.12 with corrected parent clock.
>>
>>    So, what have you used for 7795 and 7796?
>
> S2D1 for r8a7795 (H3 ES1.0)
> S0D2 for r8a7796 (M3-W)
>
> And I planned to use S0D2 for H3 ES2.0, too.

    Looks to me we should be using S0D4 for both. And perhaps the 2nd 
specifier for S0D2.

> Gr{oetje,eeting}s,
>
>                         Geert

MBR, Sergei

Re: [PATCH 1/1] clk: renesas: cpg-mssr: r8a7796: add IMR clocks

[Geert Uytterhoeven]

Hi Sergei,

On Tue, Feb 21, 2017 at 8:49 PM, Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
> On 02/21/2017 10:39 PM, Geert Uytterhoeven wrote:
>>> On 02/20/2017 11:38 AM, Geert Uytterhoeven wrote:
>>>>> Add the IMR[0-1] clocks to the R8A7796 CPG/MSSR driver.
>>>>>
>>>>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>>>>
>>>> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
>>>>> --- linux.orig/drivers/clk/renesas/r8a7796-cpg-mssr.c
>>>>> +++ linux/drivers/clk/renesas/r8a7796-cpg-mssr.c
>>>>> @@ -179,6 +179,8 @@ static const struct mssr_mod_clk r8a7796
>>>>>         DEF_MOD("vin1",                  810,   R8A7796_CLK_S0D2),
>>>>>         DEF_MOD("vin0",                  811,   R8A7796_CLK_S0D2),
>>>>>         DEF_MOD("etheravb",              812,   R8A7796_CLK_S0D6),
>>>>> +       DEF_MOD("imr1",                  822,   R8A7796_CLK_S2D1),
>>>>> +       DEF_MOD("imr0",                  823,   R8A7796_CLK_S2D1),
>>>>
>>>> According to R-Car Gen3 Hardware Users Manual Rev.0.53E, the parent
>>>> clock is S0D2, like on H3 ES2.0.
>>>
>>>    Not S0D4? I'd expect that what they call "register access" to be
>>> controlled by the MSSR bits...

I think in reality, both are gated by the same MSSR bit ;-)

However, if the driver needs to know the frequency of the clock, or control it
(e.g. it's driving some external interface), it's always interested in the
module's internal clock, not the clock used for register access.  The latter
is just an artifact of synchronous hardware.

>> Good question...

I've just finished going over all modules and their (known) parents on R-Car
H3 (ES1.x and ES2.0), and M3-W.
If multiple clock inputs are known, we always use the internal clock, which
is consistent with my comment above.

>>>> Will queue in clk-renesas-for-v4.12 with corrected parent clock.
>>>
>>>    So, what have you used for 7795 and 7796?
>>
>> S2D1 for r8a7795 (H3 ES1.0)
>> S0D2 for r8a7796 (M3-W)
>>
>> And I planned to use S0D2 for H3 ES2.0, too.
>
>    Looks to me we should be using S0D4 for both. And perhaps the 2nd
> specifier for S0D2.

We typically don't list all other parents clocks, unless we need to
query/control them (especially if they can't be gated from Linux, like the
internal SxDy clocks).

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