* Re: [PATCH] ARM: shmobile: r8a7779 CCF DTS update
2014-12-03 11:51 [PATCH] ARM: shmobile: r8a7779 CCF DTS update Magnus Damm
@ 2014-12-03 12:25 ` Geert Uytterhoeven
2014-12-03 12:52 ` Magnus Damm
` (6 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Geert Uytterhoeven @ 2014-12-03 12:25 UTC (permalink / raw)
To: linux-sh
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
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] ARM: shmobile: r8a7779 CCF DTS update
2014-12-03 11:51 [PATCH] ARM: shmobile: r8a7779 CCF DTS update Magnus Damm
2014-12-03 12:25 ` Geert Uytterhoeven
@ 2014-12-03 12:52 ` Magnus Damm
2014-12-03 12:56 ` Geert Uytterhoeven
` (5 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Magnus Damm @ 2014-12-03 12:52 UTC (permalink / raw)
To: linux-sh
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
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] ARM: shmobile: r8a7779 CCF DTS update
2014-12-03 11:51 [PATCH] ARM: shmobile: r8a7779 CCF DTS update Magnus Damm
2014-12-03 12:25 ` Geert Uytterhoeven
2014-12-03 12:52 ` Magnus Damm
@ 2014-12-03 12:56 ` Geert Uytterhoeven
2014-12-04 4:37 ` Magnus Damm
` (4 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Geert Uytterhoeven @ 2014-12-03 12:56 UTC (permalink / raw)
To: linux-sh
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
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] ARM: shmobile: r8a7779 CCF DTS update
2014-12-03 11:51 [PATCH] ARM: shmobile: r8a7779 CCF DTS update Magnus Damm
` (2 preceding siblings ...)
2014-12-03 12:56 ` Geert Uytterhoeven
@ 2014-12-04 4:37 ` Magnus Damm
2014-12-04 7:15 ` Simon Horman
` (3 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Magnus Damm @ 2014-12-04 4:37 UTC (permalink / raw)
To: linux-sh
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
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] ARM: shmobile: r8a7779 CCF DTS update
2014-12-03 11:51 [PATCH] ARM: shmobile: r8a7779 CCF DTS update Magnus Damm
` (3 preceding siblings ...)
2014-12-04 4:37 ` Magnus Damm
@ 2014-12-04 7:15 ` Simon Horman
2014-12-04 8:57 ` Magnus Damm
` (2 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Simon Horman @ 2014-12-04 7:15 UTC (permalink / raw)
To: linux-sh
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
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] ARM: shmobile: r8a7779 CCF DTS update
2014-12-03 11:51 [PATCH] ARM: shmobile: r8a7779 CCF DTS update Magnus Damm
` (4 preceding siblings ...)
2014-12-04 7:15 ` Simon Horman
@ 2014-12-04 8:57 ` Magnus Damm
2014-12-04 11:45 ` Simon Horman
2014-12-12 19:32 ` Laurent Pinchart
7 siblings, 0 replies; 9+ messages in thread
From: Magnus Damm @ 2014-12-04 8:57 UTC (permalink / raw)
To: linux-sh
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
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] ARM: shmobile: r8a7779 CCF DTS update
2014-12-03 11:51 [PATCH] ARM: shmobile: r8a7779 CCF DTS update Magnus Damm
` (5 preceding siblings ...)
2014-12-04 8:57 ` Magnus Damm
@ 2014-12-04 11:45 ` Simon Horman
2014-12-12 19:32 ` Laurent Pinchart
7 siblings, 0 replies; 9+ messages in thread
From: Simon Horman @ 2014-12-04 11:45 UTC (permalink / raw)
To: linux-sh
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.
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] ARM: shmobile: r8a7779 CCF DTS update
2014-12-03 11:51 [PATCH] ARM: shmobile: r8a7779 CCF DTS update Magnus Damm
` (6 preceding siblings ...)
2014-12-04 11:45 ` Simon Horman
@ 2014-12-12 19:32 ` Laurent Pinchart
7 siblings, 0 replies; 9+ messages in thread
From: Laurent Pinchart @ 2014-12-12 19:32 UTC (permalink / raw)
To: linux-sh
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
^ permalink raw reply [flat|nested] 9+ messages in thread