[PATCH] ARM: shmobile: r8a7779 CCF DTS update

[PATCH] ARM: shmobile: r8a7779 CCF DTS update

[Magnus Damm]

From: Magnus Damm <damm+renesas@opensource.se>

Update the r8a7779 CCF DTS with the following fixes:
 - Use MSTP0 for SCIF clock control
 - Use R8A7779_CLK_P as parent clock for SCIF (same as legacy code)
 - Use "clock-indicies" instead of "renesas,clock-indices"

Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
---

 Written on top of renesas-devel-20141202-v3.18-rc7

 arch/arm/boot/dts/r8a7779.dtsi |   30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

--- 0001/arch/arm/boot/dts/r8a7779.dtsi
+++ work/arch/arm/boot/dts/r8a7779.dtsi	2014-12-03 20:22:26.000000000 +0900
@@ -200,7 +200,7 @@
 		compatible = "renesas,scif-r8a7779", "renesas,scif";
 		reg = <0xffe40000 0x100>;
 		interrupts = <0 88 IRQ_TYPE_LEVEL_HIGH>;
-		clocks = <&cpg_clocks R8A7779_CLK_P>;
+		clocks = <&mstp0_clks R8A7779_CLK_SCIF0>;
 		clock-names = "sci_ick";
 		status = "disabled";
 	};
@@ -209,7 +209,7 @@
 		compatible = "renesas,scif-r8a7779", "renesas,scif";
 		reg = <0xffe41000 0x100>;
 		interrupts = <0 89 IRQ_TYPE_LEVEL_HIGH>;
-		clocks = <&cpg_clocks R8A7779_CLK_P>;
+		clocks = <&mstp0_clks R8A7779_CLK_SCIF1>;
 		clock-names = "sci_ick";
 		status = "disabled";
 	};
@@ -218,7 +218,7 @@
 		compatible = "renesas,scif-r8a7779", "renesas,scif";
 		reg = <0xffe42000 0x100>;
 		interrupts = <0 90 IRQ_TYPE_LEVEL_HIGH>;
-		clocks = <&cpg_clocks R8A7779_CLK_P>;
+		clocks = <&mstp0_clks R8A7779_CLK_SCIF2>;
 		clock-names = "sci_ick";
 		status = "disabled";
 	};
@@ -227,7 +227,7 @@
 		compatible = "renesas,scif-r8a7779", "renesas,scif";
 		reg = <0xffe43000 0x100>;
 		interrupts = <0 91 IRQ_TYPE_LEVEL_HIGH>;
-		clocks = <&cpg_clocks R8A7779_CLK_P>;
+		clocks = <&mstp0_clks R8A7779_CLK_SCIF3>;
 		clock-names = "sci_ick";
 		status = "disabled";
 	};
@@ -236,7 +236,7 @@
 		compatible = "renesas,scif-r8a7779", "renesas,scif";
 		reg = <0xffe44000 0x100>;
 		interrupts = <0 92 IRQ_TYPE_LEVEL_HIGH>;
-		clocks = <&cpg_clocks R8A7779_CLK_P>;
+		clocks = <&mstp0_clks R8A7779_CLK_SCIF4>;
 		clock-names = "sci_ick";
 		status = "disabled";
 	};
@@ -245,7 +245,7 @@
 		compatible = "renesas,scif-r8a7779", "renesas,scif";
 		reg = <0xffe45000 0x100>;
 		interrupts = <0 93 IRQ_TYPE_LEVEL_HIGH>;
-		clocks = <&cpg_clocks R8A7779_CLK_P>;
+		clocks = <&mstp0_clks R8A7779_CLK_SCIF5>;
 		clock-names = "sci_ick";
 		status = "disabled";
 	};
@@ -464,18 +464,18 @@
 				 <&cpg_clocks R8A7779_CLK_P>,
 				 <&cpg_clocks R8A7779_CLK_S>,
 				 <&cpg_clocks R8A7779_CLK_S>,
-				 <&cpg_clocks R8A7779_CLK_S1>,
-				 <&cpg_clocks R8A7779_CLK_S1>,
-				 <&cpg_clocks R8A7779_CLK_S1>,
-				 <&cpg_clocks R8A7779_CLK_S1>,
-				 <&cpg_clocks R8A7779_CLK_S1>,
-				 <&cpg_clocks R8A7779_CLK_S1>,
+				 <&cpg_clocks R8A7779_CLK_P>,
+				 <&cpg_clocks R8A7779_CLK_P>,
+				 <&cpg_clocks R8A7779_CLK_P>,
+				 <&cpg_clocks R8A7779_CLK_P>,
+				 <&cpg_clocks R8A7779_CLK_P>,
+				 <&cpg_clocks R8A7779_CLK_P>,
 				 <&cpg_clocks R8A7779_CLK_P>,
 				 <&cpg_clocks R8A7779_CLK_P>,
 				 <&cpg_clocks R8A7779_CLK_P>,
 				 <&cpg_clocks R8A7779_CLK_P>;
 			#clock-cells = <1>;
-			renesas,clock-indices = <
+			clock-indices = <
 				R8A7779_CLK_HSPI R8A7779_CLK_TMU2
 				R8A7779_CLK_TMU1 R8A7779_CLK_TMU0
 				R8A7779_CLK_HSCIF1 R8A7779_CLK_HSCIF0
@@ -506,7 +506,7 @@
 				 <&cpg_clocks R8A7779_CLK_P>,
 				 <&cpg_clocks R8A7779_CLK_S>;
 			#clock-cells = <1>;
-			renesas,clock-indices = <
+			clock-indices = <
 				R8A7779_CLK_USB01 R8A7779_CLK_USB2
 				R8A7779_CLK_DU R8A7779_CLK_VIN2
 				R8A7779_CLK_VIN1 R8A7779_CLK_VIN0
@@ -527,7 +527,7 @@
 			clocks = <&s4_clk>, <&s4_clk>, <&s4_clk>, <&s4_clk>,
 				 <&s4_clk>, <&s4_clk>;
 			#clock-cells = <1>;
-			renesas,clock-indices = <
+			clock-indices = <
 				R8A7779_CLK_SDHI3 R8A7779_CLK_SDHI2
 				R8A7779_CLK_SDHI1 R8A7779_CLK_SDHI0
 				R8A7779_CLK_MMC1 R8A7779_CLK_MMC0

Re: [PATCH] ARM: shmobile: r8a7779 CCF DTS update

[Geert Uytterhoeven]

Hi Magnus,

On Wed, Dec 3, 2014 at 12:51 PM, Magnus Damm <magnus.damm@gmail.com> wrote:
> Update the r8a7779 CCF DTS with the following fixes:
>  - Use MSTP0 for SCIF clock control
>  - Use R8A7779_CLK_P as parent clock for SCIF (same as legacy code)
>  - Use "clock-indicies" instead of "renesas,clock-indices"

We already have
"[PATCH v2 3/6] ARM: shmobile: r8a7779 dtsi: Change to using clock-indices"
(http://www.spinics.net/lists/linux-sh/msg37285.html) for the latter.

> Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
> ---
>
>  Written on top of renesas-devel-20141202-v3.18-rc7
>
>  arch/arm/boot/dts/r8a7779.dtsi |   30 +++++++++++++++---------------
>  1 file changed, 15 insertions(+), 15 deletions(-)
>
> --- 0001/arch/arm/boot/dts/r8a7779.dtsi
> +++ work/arch/arm/boot/dts/r8a7779.dtsi 2014-12-03 20:22:26.000000000 +0900
> @@ -200,7 +200,7 @@
>                 compatible = "renesas,scif-r8a7779", "renesas,scif";
>                 reg = <0xffe40000 0x100>;
>                 interrupts = <0 88 IRQ_TYPE_LEVEL_HIGH>;
> -               clocks = <&cpg_clocks R8A7779_CLK_P>;
> +               clocks = <&mstp0_clks R8A7779_CLK_SCIF0>;
>                 clock-names = "sci_ick";
>                 status = "disabled";
>         };

According to the datasheet, the SCIF can use both S1 and "SCIF_CLK" (I assume
that's the MSTP clock output for SCIF?) as clock input, selectable using the
XIN bit of the Clock Select Register (CKS).
Do you have more information?

> @@ -464,18 +464,18 @@
>                                  <&cpg_clocks R8A7779_CLK_P>,
>                                  <&cpg_clocks R8A7779_CLK_S>,
>                                  <&cpg_clocks R8A7779_CLK_S>,
> -                                <&cpg_clocks R8A7779_CLK_S1>,
> -                                <&cpg_clocks R8A7779_CLK_S1>,
> -                                <&cpg_clocks R8A7779_CLK_S1>,
> -                                <&cpg_clocks R8A7779_CLK_S1>,
> -                                <&cpg_clocks R8A7779_CLK_S1>,
> -                                <&cpg_clocks R8A7779_CLK_S1>,
> +                                <&cpg_clocks R8A7779_CLK_P>,
> +                                <&cpg_clocks R8A7779_CLK_P>,
> +                                <&cpg_clocks R8A7779_CLK_P>,
> +                                <&cpg_clocks R8A7779_CLK_P>,
> +                                <&cpg_clocks R8A7779_CLK_P>,
> +                                <&cpg_clocks R8A7779_CLK_P>,

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] ARM: shmobile: r8a7779 CCF DTS update

[Magnus Damm]

Hi Geert,

On Wed, Dec 3, 2014 at 9:25 PM, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> Hi Magnus,
>
> On Wed, Dec 3, 2014 at 12:51 PM, Magnus Damm <magnus.damm@gmail.com> wrote:
>> Update the r8a7779 CCF DTS with the following fixes:
>>  - Use MSTP0 for SCIF clock control
>>  - Use R8A7779_CLK_P as parent clock for SCIF (same as legacy code)
>>  - Use "clock-indicies" instead of "renesas,clock-indices"
>
> We already have
> "[PATCH v2 3/6] ARM: shmobile: r8a7779 dtsi: Change to using clock-indices"
> (http://www.spinics.net/lists/linux-sh/msg37285.html) for the latter.

Yes, I noticed that too late I'm afraid. =)

Are you aware of any outstanding issue for that series?

>> Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
>> ---
>>
>>  Written on top of renesas-devel-20141202-v3.18-rc7
>>
>>  arch/arm/boot/dts/r8a7779.dtsi |   30 +++++++++++++++---------------
>>  1 file changed, 15 insertions(+), 15 deletions(-)
>>
>> --- 0001/arch/arm/boot/dts/r8a7779.dtsi
>> +++ work/arch/arm/boot/dts/r8a7779.dtsi 2014-12-03 20:22:26.000000000 +0900
>> @@ -200,7 +200,7 @@
>>                 compatible = "renesas,scif-r8a7779", "renesas,scif";
>>                 reg = <0xffe40000 0x100>;
>>                 interrupts = <0 88 IRQ_TYPE_LEVEL_HIGH>;
>> -               clocks = <&cpg_clocks R8A7779_CLK_P>;
>> +               clocks = <&mstp0_clks R8A7779_CLK_SCIF0>;
>>                 clock-names = "sci_ick";
>>                 status = "disabled";
>>         };
>
> According to the datasheet, the SCIF can use both S1 and "SCIF_CLK" (I assume
> that's the MSTP clock output for SCIF?) as clock input, selectable using the
> XIN bit of the Clock Select Register (CKS).
> Do you have more information?

I suspect that you may look at the left side of the r8a7779 SCIF
overview page near the BRG where there is a "clks1" and the external
SCIF_CLK. I'm looking at the right side and the "baud rate generator"
where clkp is hooked up.

The SCIF driver today has relatively limited support for the BRG
(which confusingly enough is a second baud rate generator, how many do
one need?). With the "regular" baud rate generator and the BRG unit
some of the SCIF variants can be configured to use many different
clocks.

We currently lack code for the following:
A) Describe all the clocks hooked up to the SCIF
B) Select the best clock for any given baud rate

So with this patch we simply ignore the BRG.

>> @@ -464,18 +464,18 @@
>>                                  <&cpg_clocks R8A7779_CLK_P>,
>>                                  <&cpg_clocks R8A7779_CLK_S>,
>>                                  <&cpg_clocks R8A7779_CLK_S>,
>> -                                <&cpg_clocks R8A7779_CLK_S1>,
>> -                                <&cpg_clocks R8A7779_CLK_S1>,
>> -                                <&cpg_clocks R8A7779_CLK_S1>,
>> -                                <&cpg_clocks R8A7779_CLK_S1>,
>> -                                <&cpg_clocks R8A7779_CLK_S1>,
>> -                                <&cpg_clocks R8A7779_CLK_S1>,
>> +                                <&cpg_clocks R8A7779_CLK_P>,
>> +                                <&cpg_clocks R8A7779_CLK_P>,
>> +                                <&cpg_clocks R8A7779_CLK_P>,
>> +                                <&cpg_clocks R8A7779_CLK_P>,
>> +                                <&cpg_clocks R8A7779_CLK_P>,
>> +                                <&cpg_clocks R8A7779_CLK_P>,
>
> 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] ARM: shmobile: r8a7779 CCF DTS update

[Geert Uytterhoeven]

Hi Magnus,

On Wed, Dec 3, 2014 at 1:52 PM, Magnus Damm <magnus.damm@gmail.com> wrote:
> On Wed, Dec 3, 2014 at 9:25 PM, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>> On Wed, Dec 3, 2014 at 12:51 PM, Magnus Damm <magnus.damm@gmail.com> wrote:
>>> Update the r8a7779 CCF DTS with the following fixes:
>>>  - Use MSTP0 for SCIF clock control
>>>  - Use R8A7779_CLK_P as parent clock for SCIF (same as legacy code)
>>>  - Use "clock-indicies" instead of "renesas,clock-indices"
>>
>> We already have
>> "[PATCH v2 3/6] ARM: shmobile: r8a7779 dtsi: Change to using clock-indices"
>> (http://www.spinics.net/lists/linux-sh/msg37285.html) for the latter.
>
> Yes, I noticed that too late I'm afraid. =)
>
> Are you aware of any outstanding issue for that series?

Not that I'm aware of. As Mike merged the bindings update, I had pinged
Simon about the rest of the series, just before you sent your patch.

>>> Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
>>> ---
>>>
>>>  Written on top of renesas-devel-20141202-v3.18-rc7
>>>
>>>  arch/arm/boot/dts/r8a7779.dtsi |   30 +++++++++++++++---------------
>>>  1 file changed, 15 insertions(+), 15 deletions(-)
>>>
>>> --- 0001/arch/arm/boot/dts/r8a7779.dtsi
>>> +++ work/arch/arm/boot/dts/r8a7779.dtsi 2014-12-03 20:22:26.000000000 +0900
>>> @@ -200,7 +200,7 @@
>>>                 compatible = "renesas,scif-r8a7779", "renesas,scif";
>>>                 reg = <0xffe40000 0x100>;
>>>                 interrupts = <0 88 IRQ_TYPE_LEVEL_HIGH>;
>>> -               clocks = <&cpg_clocks R8A7779_CLK_P>;
>>> +               clocks = <&mstp0_clks R8A7779_CLK_SCIF0>;
>>>                 clock-names = "sci_ick";
>>>                 status = "disabled";
>>>         };
>>
>> According to the datasheet, the SCIF can use both S1 and "SCIF_CLK" (I assume
>> that's the MSTP clock output for SCIF?) as clock input, selectable using the
>> XIN bit of the Clock Select Register (CKS).
>> Do you have more information?
>
> I suspect that you may look at the left side of the r8a7779 SCIF
> overview page near the BRG where there is a "clks1" and the external
> SCIF_CLK. I'm looking at the right side and the "baud rate generator"
> where clkp is hooked up.

Oh right, that "clkp" is really "clkp through MSTP bit X"?

> The SCIF driver today has relatively limited support for the BRG
> (which confusingly enough is a second baud rate generator, how many do
> one need?). With the "regular" baud rate generator and the BRG unit
> some of the SCIF variants can be configured to use many different
> clocks.
>
> We currently lack code for the following:
> A) Describe all the clocks hooked up to the SCIF
> B) Select the best clock for any given baud rate
>
> So with this patch we simply ignore the BRG.

OK. Fair enough.

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] ARM: shmobile: r8a7779 CCF DTS update

[Magnus Damm]

Hi Geert,

On Wed, Dec 3, 2014 at 9:56 PM, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> Hi Magnus,
>
> On Wed, Dec 3, 2014 at 1:52 PM, Magnus Damm <magnus.damm@gmail.com> wrote:
>> On Wed, Dec 3, 2014 at 9:25 PM, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>>> On Wed, Dec 3, 2014 at 12:51 PM, Magnus Damm <magnus.damm@gmail.com> wrote:
>>>> Update the r8a7779 CCF DTS with the following fixes:
>>>>  - Use MSTP0 for SCIF clock control
>>>>  - Use R8A7779_CLK_P as parent clock for SCIF (same as legacy code)
>>>>  - Use "clock-indicies" instead of "renesas,clock-indices"
>>>
>>> We already have
>>> "[PATCH v2 3/6] ARM: shmobile: r8a7779 dtsi: Change to using clock-indices"
>>> (http://www.spinics.net/lists/linux-sh/msg37285.html) for the latter.
>>
>> Yes, I noticed that too late I'm afraid. =)
>>
>> Are you aware of any outstanding issue for that series?
>
> Not that I'm aware of. As Mike merged the bindings update, I had pinged
> Simon about the rest of the series, just before you sent your patch.

Thanks, sounds like I should make a V2 of this patch once Simon has
merged patch 3/6 above.

>>>> Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
>>>> ---
>>>>
>>>>  Written on top of renesas-devel-20141202-v3.18-rc7
>>>>
>>>>  arch/arm/boot/dts/r8a7779.dtsi |   30 +++++++++++++++---------------
>>>>  1 file changed, 15 insertions(+), 15 deletions(-)
>>>>
>>>> --- 0001/arch/arm/boot/dts/r8a7779.dtsi
>>>> +++ work/arch/arm/boot/dts/r8a7779.dtsi 2014-12-03 20:22:26.000000000 +0900
>>>> @@ -200,7 +200,7 @@
>>>>                 compatible = "renesas,scif-r8a7779", "renesas,scif";
>>>>                 reg = <0xffe40000 0x100>;
>>>>                 interrupts = <0 88 IRQ_TYPE_LEVEL_HIGH>;
>>>> -               clocks = <&cpg_clocks R8A7779_CLK_P>;
>>>> +               clocks = <&mstp0_clks R8A7779_CLK_SCIF0>;
>>>>                 clock-names = "sci_ick";
>>>>                 status = "disabled";
>>>>         };
>>>
>>> According to the datasheet, the SCIF can use both S1 and "SCIF_CLK" (I assume
>>> that's the MSTP clock output for SCIF?) as clock input, selectable using the
>>> XIN bit of the Clock Select Register (CKS).
>>> Do you have more information?
>>
>> I suspect that you may look at the left side of the r8a7779 SCIF
>> overview page near the BRG where there is a "clks1" and the external
>> SCIF_CLK. I'm looking at the right side and the "baud rate generator"
>> where clkp is hooked up.
>
> Oh right, that "clkp" is really "clkp through MSTP bit X"?

Yes, this is how we historically have hooked up the SCIF clocks. It is
however not entirely clear from the documentation how the MSTP bit is
connected, so the assumption may be wrong. And the SCIF driver has
support for interface clocks as well. The "clkp through MSTP bit X"
implementation seems better than the current code at least.

>> The SCIF driver today has relatively limited support for the BRG
>> (which confusingly enough is a second baud rate generator, how many do
>> one need?). With the "regular" baud rate generator and the BRG unit
>> some of the SCIF variants can be configured to use many different
>> clocks.
>>
>> We currently lack code for the following:
>> A) Describe all the clocks hooked up to the SCIF
>> B) Select the best clock for any given baud rate
>>
>> So with this patch we simply ignore the BRG.
>
> OK. Fair enough.
>
> Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>

Thanks!

/ magnus

Re: [PATCH] ARM: shmobile: r8a7779 CCF DTS update

[Simon Horman]

On Thu, Dec 04, 2014 at 01:37:10PM +0900, Magnus Damm wrote:
> Hi Geert,
> 
> On Wed, Dec 3, 2014 at 9:56 PM, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > Hi Magnus,
> >
> > On Wed, Dec 3, 2014 at 1:52 PM, Magnus Damm <magnus.damm@gmail.com> wrote:
> >> On Wed, Dec 3, 2014 at 9:25 PM, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> >>> On Wed, Dec 3, 2014 at 12:51 PM, Magnus Damm <magnus.damm@gmail.com> wrote:
> >>>> Update the r8a7779 CCF DTS with the following fixes:
> >>>>  - Use MSTP0 for SCIF clock control
> >>>>  - Use R8A7779_CLK_P as parent clock for SCIF (same as legacy code)
> >>>>  - Use "clock-indicies" instead of "renesas,clock-indices"
> >>>
> >>> We already have
> >>> "[PATCH v2 3/6] ARM: shmobile: r8a7779 dtsi: Change to using clock-indices"
> >>> (http://www.spinics.net/lists/linux-sh/msg37285.html) for the latter.
> >>
> >> Yes, I noticed that too late I'm afraid. =)
> >>
> >> Are you aware of any outstanding issue for that series?
> >
> > Not that I'm aware of. As Mike merged the bindings update, I had pinged
> > Simon about the rest of the series, just before you sent your patch.
> 
> Thanks, sounds like I should make a V2 of this patch once Simon has
> merged patch 3/6 above.

In general I prefer one fix per patch.

So unless there is a dependency between the remaining two fixes
could you please make them separate patches when you drop the
clock-indicies changes?

> 
> >>>> Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
> >>>> ---
> >>>>
> >>>>  Written on top of renesas-devel-20141202-v3.18-rc7
> >>>>
> >>>>  arch/arm/boot/dts/r8a7779.dtsi |   30 +++++++++++++++---------------
> >>>>  1 file changed, 15 insertions(+), 15 deletions(-)
> >>>>
> >>>> --- 0001/arch/arm/boot/dts/r8a7779.dtsi
> >>>> +++ work/arch/arm/boot/dts/r8a7779.dtsi 2014-12-03 20:22:26.000000000 +0900
> >>>> @@ -200,7 +200,7 @@
> >>>>                 compatible = "renesas,scif-r8a7779", "renesas,scif";
> >>>>                 reg = <0xffe40000 0x100>;
> >>>>                 interrupts = <0 88 IRQ_TYPE_LEVEL_HIGH>;
> >>>> -               clocks = <&cpg_clocks R8A7779_CLK_P>;
> >>>> +               clocks = <&mstp0_clks R8A7779_CLK_SCIF0>;
> >>>>                 clock-names = "sci_ick";
> >>>>                 status = "disabled";
> >>>>         };
> >>>
> >>> According to the datasheet, the SCIF can use both S1 and "SCIF_CLK" (I assume
> >>> that's the MSTP clock output for SCIF?) as clock input, selectable using the
> >>> XIN bit of the Clock Select Register (CKS).
> >>> Do you have more information?
> >>
> >> I suspect that you may look at the left side of the r8a7779 SCIF
> >> overview page near the BRG where there is a "clks1" and the external
> >> SCIF_CLK. I'm looking at the right side and the "baud rate generator"
> >> where clkp is hooked up.
> >
> > Oh right, that "clkp" is really "clkp through MSTP bit X"?
> 
> Yes, this is how we historically have hooked up the SCIF clocks. It is
> however not entirely clear from the documentation how the MSTP bit is
> connected, so the assumption may be wrong. And the SCIF driver has
> support for interface clocks as well. The "clkp through MSTP bit X"
> implementation seems better than the current code at least.
> 
> >> The SCIF driver today has relatively limited support for the BRG
> >> (which confusingly enough is a second baud rate generator, how many do
> >> one need?). With the "regular" baud rate generator and the BRG unit
> >> some of the SCIF variants can be configured to use many different
> >> clocks.
> >>
> >> We currently lack code for the following:
> >> A) Describe all the clocks hooked up to the SCIF
> >> B) Select the best clock for any given baud rate
> >>
> >> So with this patch we simply ignore the BRG.
> >
> > OK. Fair enough.
> >
> > Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>
> 
> Thanks!
> 
> / magnus
> 

Re: [PATCH] ARM: shmobile: r8a7779 CCF DTS update

[Magnus Damm]

Hi Simon,

On Thu, Dec 4, 2014 at 4:15 PM, Simon Horman <horms@verge.net.au> wrote:
> On Thu, Dec 04, 2014 at 01:37:10PM +0900, Magnus Damm wrote:
>> Hi Geert,
>>
>> On Wed, Dec 3, 2014 at 9:56 PM, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>> > Hi Magnus,
>> >
>> > On Wed, Dec 3, 2014 at 1:52 PM, Magnus Damm <magnus.damm@gmail.com> wrote:
>> >> On Wed, Dec 3, 2014 at 9:25 PM, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>> >>> On Wed, Dec 3, 2014 at 12:51 PM, Magnus Damm <magnus.damm@gmail.com> wrote:
>> >>>> Update the r8a7779 CCF DTS with the following fixes:
>> >>>>  - Use MSTP0 for SCIF clock control
>> >>>>  - Use R8A7779_CLK_P as parent clock for SCIF (same as legacy code)
>> >>>>  - Use "clock-indicies" instead of "renesas,clock-indices"
>> >>>
>> >>> We already have
>> >>> "[PATCH v2 3/6] ARM: shmobile: r8a7779 dtsi: Change to using clock-indices"
>> >>> (http://www.spinics.net/lists/linux-sh/msg37285.html) for the latter.
>> >>
>> >> Yes, I noticed that too late I'm afraid. =)
>> >>
>> >> Are you aware of any outstanding issue for that series?
>> >
>> > Not that I'm aware of. As Mike merged the bindings update, I had pinged
>> > Simon about the rest of the series, just before you sent your patch.
>>
>> Thanks, sounds like I should make a V2 of this patch once Simon has
>> merged patch 3/6 above.
>
> In general I prefer one fix per patch.
>
> So unless there is a dependency between the remaining two fixes
> could you please make them separate patches when you drop the
> clock-indicies changes?

I think both are needed to make it work, but I don't mind breaking
them out into two separate commits. I agree that it is cleaner.

/ magnus

Re: [PATCH] ARM: shmobile: r8a7779 CCF DTS update

[Simon Horman]

On Thu, Dec 04, 2014 at 05:57:19PM +0900, Magnus Damm wrote:
> Hi Simon,
> 
> On Thu, Dec 4, 2014 at 4:15 PM, Simon Horman <horms@verge.net.au> wrote:
> > On Thu, Dec 04, 2014 at 01:37:10PM +0900, Magnus Damm wrote:
> >> Hi Geert,
> >>
> >> On Wed, Dec 3, 2014 at 9:56 PM, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> >> > Hi Magnus,
> >> >
> >> > On Wed, Dec 3, 2014 at 1:52 PM, Magnus Damm <magnus.damm@gmail.com> wrote:
> >> >> On Wed, Dec 3, 2014 at 9:25 PM, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> >> >>> On Wed, Dec 3, 2014 at 12:51 PM, Magnus Damm <magnus.damm@gmail.com> wrote:
> >> >>>> Update the r8a7779 CCF DTS with the following fixes:
> >> >>>>  - Use MSTP0 for SCIF clock control
> >> >>>>  - Use R8A7779_CLK_P as parent clock for SCIF (same as legacy code)
> >> >>>>  - Use "clock-indicies" instead of "renesas,clock-indices"
> >> >>>
> >> >>> We already have
> >> >>> "[PATCH v2 3/6] ARM: shmobile: r8a7779 dtsi: Change to using clock-indices"
> >> >>> (http://www.spinics.net/lists/linux-sh/msg37285.html) for the latter.
> >> >>
> >> >> Yes, I noticed that too late I'm afraid. =)
> >> >>
> >> >> Are you aware of any outstanding issue for that series?
> >> >
> >> > Not that I'm aware of. As Mike merged the bindings update, I had pinged
> >> > Simon about the rest of the series, just before you sent your patch.
> >>
> >> Thanks, sounds like I should make a V2 of this patch once Simon has
> >> merged patch 3/6 above.
> >
> > In general I prefer one fix per patch.
> >
> > So unless there is a dependency between the remaining two fixes
> > could you please make them separate patches when you drop the
> > clock-indicies changes?
> 
> I think both are needed to make it work, but I don't mind breaking
> them out into two separate commits. I agree that it is cleaner.

Thanks, I think that would be best.

Re: [PATCH] ARM: shmobile: r8a7779 CCF DTS update

[Laurent Pinchart]

Hi Magnus,

On Thursday 04 December 2014 13:37:10 Magnus Damm wrote:
> On Wed, Dec 3, 2014 at 9:56 PM, Geert Uytterhoeven wrote:
> > On Wed, Dec 3, 2014 at 1:52 PM, Magnus Damm <magnus.damm@gmail.com> wrote:
> >> On Wed, Dec 3, 2014 at 9:25 PM, Geert Uytterhoeven wrote:
> >>> On Wed, Dec 3, 2014 at 12:51 PM, Magnus Damm wrote:
> >>>> Update the r8a7779 CCF DTS with the following fixes:
> >>>>  - Use MSTP0 for SCIF clock control
> >>>>  - Use R8A7779_CLK_P as parent clock for SCIF (same as legacy code)
> >>>>  - Use "clock-indicies" instead of "renesas,clock-indices"
> >>> 
> >>> We already have
> >>> "[PATCH v2 3/6] ARM: shmobile: r8a7779 dtsi: Change to using
> >>> clock-indices"
> >>> (http://www.spinics.net/lists/linux-sh/msg37285.html) for the latter.
> >> 
> >> Yes, I noticed that too late I'm afraid. =)
> >> 
> >> Are you aware of any outstanding issue for that series?
> > 
> > Not that I'm aware of. As Mike merged the bindings update, I had pinged
> > Simon about the rest of the series, just before you sent your patch.
> 
> Thanks, sounds like I should make a V2 of this patch once Simon has
> merged patch 3/6 above.
> 
> >>>> Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
> >>>> ---
> >>>> 
> >>>>  Written on top of renesas-devel-20141202-v3.18-rc7
> >>>>  
> >>>>  arch/arm/boot/dts/r8a7779.dtsi |   30 +++++++++++++++---------------
> >>>>  1 file changed, 15 insertions(+), 15 deletions(-)
> >>>> 
> >>>> --- 0001/arch/arm/boot/dts/r8a7779.dtsi
> >>>> +++ work/arch/arm/boot/dts/r8a7779.dtsi 2014-12-03 20:22:26.000000000
> >>>> +0900
> >>>> @@ -200,7 +200,7 @@
> >>>>                 compatible = "renesas,scif-r8a7779", "renesas,scif";
> >>>>                 reg = <0xffe40000 0x100>;
> >>>>                 interrupts = <0 88 IRQ_TYPE_LEVEL_HIGH>;
> >>>> -               clocks = <&cpg_clocks R8A7779_CLK_P>;
> >>>> +               clocks = <&mstp0_clks R8A7779_CLK_SCIF0>;
> >>>>                 clock-names = "sci_ick";
> >>>>                 status = "disabled";
> >>>>         };
> >>> 
> >>> According to the datasheet, the SCIF can use both S1 and "SCIF_CLK" (I
> >>> assume that's the MSTP clock output for SCIF?) as clock input,
> >>> selectable using the XIN bit of the Clock Select Register (CKS).
> >>> Do you have more information?
> >> 
> >> I suspect that you may look at the left side of the r8a7779 SCIF
> >> overview page near the BRG where there is a "clks1" and the external
> >> SCIF_CLK. I'm looking at the right side and the "baud rate generator"
> >> where clkp is hooked up.
> > 
> > Oh right, that "clkp" is really "clkp through MSTP bit X"?
> 
> Yes, this is how we historically have hooked up the SCIF clocks. It is
> however not entirely clear from the documentation how the MSTP bit is
> connected, so the assumption may be wrong. And the SCIF driver has
> support for interface clocks as well.

I've studied this extensively over the SH and ARM SoCs and my conclusion was 
that the SCIF has a functional clock and several optional baud rate clocks, 
but no interface clock. References to the interface clock in the driver and DT 
bindings are leftovers of historical mistakes.

> The "clkp through MSTP bit X" implementation seems better than the current
> code at least.
> 
> >> The SCIF driver today has relatively limited support for the BRG
> >> (which confusingly enough is a second baud rate generator, how many do
> >> one need?). With the "regular" baud rate generator and the BRG unit
> >> some of the SCIF variants can be configured to use many different
> >> clocks.
> >> 
> >> We currently lack code for the following:
> >> A) Describe all the clocks hooked up to the SCIF
> >> B) Select the best clock for any given baud rate
> >> 
> >> So with this patch we simply ignore the BRG.
> > 
> > OK. Fair enough.
> > 
> > Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>

-- 
Regards,

Laurent Pinchart