| Message ID | 1420418648-7815-1-git-send-email-horms+renesas@verge.net.au (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Simon Horman |
| Headers | show |
On Mon, Jan 05, 2015 at 09:44:08AM +0900, Simon Horman wrote: > Correct the SDHI clock node names to match their labels > and the documentation. > > This has no run-time affect as the clock nodes are not currently used. > > Reported-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> > Signed-off-by: Simon Horman <horms+renesas@verge.net.au> This patch is based on renesas-devel-20141231-v3.19-rc2 > --- > arch/arm/boot/dts/r8a7794.dtsi | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/arm/boot/dts/r8a7794.dtsi b/arch/arm/boot/dts/r8a7794.dtsi > index 8f78da5..9856e9c 100644 > --- a/arch/arm/boot/dts/r8a7794.dtsi > +++ b/arch/arm/boot/dts/r8a7794.dtsi > @@ -294,14 +294,14 @@ > "lb", "qspi", "sdh", "sd0", "z"; > }; > /* Variable factor clocks */ > - sd1_clk: sd2_clk@e6150078 { > + sd1_clk: sd1_clk@e6150078 { > compatible = "renesas,r8a7794-div6-clock", "renesas,cpg-div6-clock"; > reg = <0 0xe6150078 0 4>; > clocks = <&pll1_div2_clk>; > #clock-cells = <0>; > clock-output-names = "sd1"; > }; > - sd2_clk: sd3_clk@e615007c { > + sd2_clk: sd2_clk@e615007c { > compatible = "renesas,r8a7794-div6-clock", "renesas,cpg-div6-clock"; > reg = <0 0xe615007c 0 4>; > clocks = <&pll1_div2_clk>; > -- > 2.1.3 > -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jan 5, 2015 at 1:44 AM, Simon Horman <horms+renesas@verge.net.au> wrote: > diff --git a/arch/arm/boot/dts/r8a7794.dtsi b/arch/arm/boot/dts/r8a7794.dtsi > index 8f78da5..9856e9c 100644 > --- a/arch/arm/boot/dts/r8a7794.dtsi > +++ b/arch/arm/boot/dts/r8a7794.dtsi > @@ -294,14 +294,14 @@ > "lb", "qspi", "sdh", "sd0", "z"; > }; > /* Variable factor clocks */ > - sd1_clk: sd2_clk@e6150078 { > + sd1_clk: sd1_clk@e6150078 { > compatible = "renesas,r8a7794-div6-clock", "renesas,cpg-div6-clock"; > reg = <0 0xe6150078 0 4>; > clocks = <&pll1_div2_clk>; > #clock-cells = <0>; > clock-output-names = "sd1"; > }; > - sd2_clk: sd3_clk@e615007c { > + sd2_clk: sd2_clk@e615007c { > compatible = "renesas,r8a7794-div6-clock", "renesas,cpg-div6-clock"; > reg = <0 0xe615007c 0 4>; > clocks = <&pll1_div2_clk>; According to the documentation, SDCKCR is 0xe6150074, not ..78, SD2CKCR is 0xe6150078, not ..7c. For reference SD3CKCR is 0xe615026c. Note that r8a7791.dtsi also has the mismatches between the labels and node names. The discrepancy is caused by the different number of SDHI modules in the different SoCs of the R-Car Gen2 family. - r8a7790 has hardware modules SDHI0, SDHI1, SDHI2, and SDHI3. - r8a7791 and r8a7794 have hardware modules SDHI0, SDHI2, and SDHI3, but call the last two instances SDHI1 and SDHI2, while the clock registers are not renumbered. So instance SDHI1 uses SD2CKCR, and instance SDHI2 uses SD3CKCR, right? That means the node names are actually correct. But the register value and unit address for the second clock are still wrong, they should be (0x)e615007c, cfr. in r8a7791.dtsi. Note that IMHO the clock-output-names are wrong, for both r8a7791 and r8a7794, as the hardware documentation calls these "sd2" and "sd3", not "sd1" and "sd2". 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 -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jan 05, 2015 at 10:41:14AM +0100, Geert Uytterhoeven wrote: > On Mon, Jan 5, 2015 at 1:44 AM, Simon Horman <horms+renesas@verge.net.au> wrote: > > diff --git a/arch/arm/boot/dts/r8a7794.dtsi b/arch/arm/boot/dts/r8a7794.dtsi > > index 8f78da5..9856e9c 100644 > > --- a/arch/arm/boot/dts/r8a7794.dtsi > > +++ b/arch/arm/boot/dts/r8a7794.dtsi > > @@ -294,14 +294,14 @@ > > "lb", "qspi", "sdh", "sd0", "z"; > > }; > > /* Variable factor clocks */ > > - sd1_clk: sd2_clk@e6150078 { > > + sd1_clk: sd1_clk@e6150078 { > > compatible = "renesas,r8a7794-div6-clock", "renesas,cpg-div6-clock"; > > reg = <0 0xe6150078 0 4>; > > clocks = <&pll1_div2_clk>; > > #clock-cells = <0>; > > clock-output-names = "sd1"; > > }; > > - sd2_clk: sd3_clk@e615007c { > > + sd2_clk: sd2_clk@e615007c { > > compatible = "renesas,r8a7794-div6-clock", "renesas,cpg-div6-clock"; > > reg = <0 0xe615007c 0 4>; > > clocks = <&pll1_div2_clk>; > > According to the documentation, SDCKCR is 0xe6150074, not ..78, > SD2CKCR is 0xe6150078, not ..7c. > For reference SD3CKCR is 0xe615026c. > > Note that r8a7791.dtsi also has the mismatches between the labels and node > names. > > The discrepancy is caused by the different number of SDHI modules in the > different SoCs of the R-Car Gen2 family. > > - r8a7790 has hardware modules SDHI0, SDHI1, SDHI2, and SDHI3. > - r8a7791 and r8a7794 have hardware modules SDHI0, SDHI2, and SDHI3, > but call the last two instances SDHI1 and SDHI2, while the clock registers > are not renumbered. > So instance SDHI1 uses SD2CKCR, and instance SDHI2 uses SD3CKCR, right? I had not noticed, but yes that does seem to be the case. > That means the node names are actually correct. But the register value and > unit address for the second clock are still wrong, they should be (0x)e615007c, Ok, that makes sense. But in that the labels want updating, right? > cfr. in r8a7791.dtsi. > > Note that IMHO the clock-output-names are wrong, for both r8a7791 and r8a7794, > as the hardware documentation calls these "sd2" and "sd3", not "sd1" and "sd2". Excellent. I think we can easily change that for the r8a7794 as the nodes aren't being consumed nor have been part of a release. But I'm less sure about a path forward for the r8a7791. What are your thoughts? -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Simon, On Thu, Jan 8, 2015 at 1:38 AM, Simon Horman <horms@verge.net.au> wrote: > On Mon, Jan 05, 2015 at 10:41:14AM +0100, Geert Uytterhoeven wrote: >> On Mon, Jan 5, 2015 at 1:44 AM, Simon Horman <horms+renesas@verge.net.au> wrote: >> > diff --git a/arch/arm/boot/dts/r8a7794.dtsi b/arch/arm/boot/dts/r8a7794.dtsi >> > index 8f78da5..9856e9c 100644 >> > --- a/arch/arm/boot/dts/r8a7794.dtsi >> > +++ b/arch/arm/boot/dts/r8a7794.dtsi >> > @@ -294,14 +294,14 @@ >> > "lb", "qspi", "sdh", "sd0", "z"; >> > }; >> > /* Variable factor clocks */ >> > - sd1_clk: sd2_clk@e6150078 { >> > + sd1_clk: sd1_clk@e6150078 { >> > compatible = "renesas,r8a7794-div6-clock", "renesas,cpg-div6-clock"; >> > reg = <0 0xe6150078 0 4>; >> > clocks = <&pll1_div2_clk>; >> > #clock-cells = <0>; >> > clock-output-names = "sd1"; >> > }; >> > - sd2_clk: sd3_clk@e615007c { >> > + sd2_clk: sd2_clk@e615007c { >> > compatible = "renesas,r8a7794-div6-clock", "renesas,cpg-div6-clock"; >> > reg = <0 0xe615007c 0 4>; >> > clocks = <&pll1_div2_clk>; >> >> According to the documentation, SDCKCR is 0xe6150074, not ..78, >> SD2CKCR is 0xe6150078, not ..7c. >> For reference SD3CKCR is 0xe615026c. >> >> Note that r8a7791.dtsi also has the mismatches between the labels and node >> names. >> >> The discrepancy is caused by the different number of SDHI modules in the >> different SoCs of the R-Car Gen2 family. >> >> - r8a7790 has hardware modules SDHI0, SDHI1, SDHI2, and SDHI3. >> - r8a7791 and r8a7794 have hardware modules SDHI0, SDHI2, and SDHI3, >> but call the last two instances SDHI1 and SDHI2, while the clock registers >> are not renumbered. >> So instance SDHI1 uses SD2CKCR, and instance SDHI2 uses SD3CKCR, right? > > I had not noticed, but yes that does seem to be the case. > >> That means the node names are actually correct. But the register value and >> unit address for the second clock are still wrong, they should be (0x)e615007c, > > Ok, that makes sense. But in that the labels want updating, right? Yes, if the labels are supposed to refer to the clk instance (and not the SDHI instance). >> cfr. in r8a7791.dtsi. >> >> Note that IMHO the clock-output-names are wrong, for both r8a7791 and r8a7794, >> as the hardware documentation calls these "sd2" and "sd3", not "sd1" and "sd2". > > Excellent. I think we can easily change that for the r8a7794 as the > nodes aren't being consumed nor have been part of a release. But I'm > less sure about a path forward for the r8a7791. What are your thoughts? While the clock names are user-visible through /sysfs, I don't think anything relies on them. Their users (in DT) refer to the labels in DT. Or am I missing something? 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 -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/arm/boot/dts/r8a7794.dtsi b/arch/arm/boot/dts/r8a7794.dtsi index 8f78da5..9856e9c 100644 --- a/arch/arm/boot/dts/r8a7794.dtsi +++ b/arch/arm/boot/dts/r8a7794.dtsi @@ -294,14 +294,14 @@ "lb", "qspi", "sdh", "sd0", "z"; }; /* Variable factor clocks */ - sd1_clk: sd2_clk@e6150078 { + sd1_clk: sd1_clk@e6150078 { compatible = "renesas,r8a7794-div6-clock", "renesas,cpg-div6-clock"; reg = <0 0xe6150078 0 4>; clocks = <&pll1_div2_clk>; #clock-cells = <0>; clock-output-names = "sd1"; }; - sd2_clk: sd3_clk@e615007c { + sd2_clk: sd2_clk@e615007c { compatible = "renesas,r8a7794-div6-clock", "renesas,cpg-div6-clock"; reg = <0 0xe615007c 0 4>; clocks = <&pll1_div2_clk>;
Correct the SDHI clock node names to match their labels and the documentation. This has no run-time affect as the clock nodes are not currently used. Reported-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> Signed-off-by: Simon Horman <horms+renesas@verge.net.au> --- arch/arm/boot/dts/r8a7794.dtsi | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)