diff mbox

[1/3] ARM: shmobile: r8a7790: add MSTP10 support on DTSI

Message ID 87vbs9cx2q.wl%kuninori.morimoto.gx@renesas.com (mailing list archive)
State RFC
Headers show

Commit Message

Kuninori Morimoto June 10, 2014, 2:33 a.m. UTC
From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 arch/arm/boot/dts/r8a7790.dtsi            |   32 +++++++++++++++++++++++++++++
 include/dt-bindings/clock/r8a7790-clock.h |   26 +++++++++++++++++++++++
 2 files changed, 58 insertions(+)

Comments

Geert Uytterhoeven June 10, 2014, 7:32 a.m. UTC | #1
Hi Morimoto-san,

On Tue, Jun 10, 2014 at 4:33 AM, Kuninori Morimoto
<kuninori.morimoto.gx@renesas.com> wrote:
> --- a/arch/arm/boot/dts/r8a7790.dtsi
> +++ b/arch/arm/boot/dts/r8a7790.dtsi
> @@ -835,6 +835,38 @@
>                                 "rcan1", "rcan0", "qspi_mod", "iic3",
>                                 "i2c3", "i2c2", "i2c1", "i2c0";
>                 };
> +               mstp10_clks: mstp10_clks@e6150998 {
> +                       compatible = "renesas,r8a7790-mstp-clocks", "renesas,cpg-mstp-clocks";
> +                       reg = <0 0xe6150998 0 4>, <0 0xe61509a8 0 4>;
> +                       clocks = <&p_clk>, /* parent of SCU */
> +                               <&p_clk>,
> +                               <&p_clk>, <&p_clk>, <&p_clk>, <&p_clk>, <&p_clk>,
> +                               <&p_clk>, <&p_clk>, <&p_clk>, <&p_clk>, <&p_clk>,
> +                               <&mstp10_clks R8A7790_CLK_SCU>, <&mstp10_clks R8A7790_CLK_SCU>,
> +                               <&mstp10_clks R8A7790_CLK_SCU>, <&mstp10_clks R8A7790_CLK_SCU>,
> +                               <&mstp10_clks R8A7790_CLK_SCU>, <&mstp10_clks R8A7790_CLK_SCU>,
> +                               <&mstp10_clks R8A7790_CLK_SCU>, <&mstp10_clks R8A7790_CLK_SCU>,
> +                               <&mstp10_clks R8A7790_CLK_SCU>, <&mstp10_clks R8A7790_CLK_SCU>,
> +                               <&mstp10_clks R8A7790_CLK_SCU>, <&mstp10_clks R8A7790_CLK_SCU>;
> +
> +                       #clock-cells = <1>;
> +                       clock-indices = <
> +                               R8A7790_CLK_SCU
> +                               R8A7790_CLK_SSI
> +                               R8A7790_CLK_SSI9 R8A7790_CLK_SSI8 R8A7790_CLK_SSI7 R8A7790_CLK_SSI6 R8A7790_CLK_SSI5
> +                               R8A7790_CLK_SSI4 R8A7790_CLK_SSI3 R8A7790_CLK_SSI2 R8A7790_CLK_SSI1 R8A7790_CLK_SSI0
> +                               R8A7790_CLK_DVC1 R8A7790_CLK_DVC0
> +                               R8A7790_CLK_SRC9 R8A7790_CLK_SRC8 R8A7790_CLK_SRC7 R8A7790_CLK_SRC6 R8A7790_CLK_SRC5
> +                               R8A7790_CLK_SRC4 R8A7790_CLK_SRC3 R8A7790_CLK_SRC2 R8A7790_CLK_SRC1 R8A7790_CLK_SRC0

I think we usually put these in numerical order here.

> +                       >;
> +                       clock-output-names =
> +                               "scu", "ssi",
> +                               "ssi9", "ssi8", "ssi7", "ssi6", "ssi5",
> +                               "ssi4", "ssi3", "ssi2", "ssi1", "ssi0",
> +                               "dvc1", "dvc0",
> +                               "src9", "src8", "src7", "src6", "src5",
> +                               "src4", "src3", "src2", "src1", "src0";
> +               };
>         };
>
>         qspi: spi@e6b10000 {
> diff --git a/include/dt-bindings/clock/r8a7790-clock.h b/include/dt-bindings/clock/r8a7790-clock.h
> index 1118f7a..b4430b0 100644
> --- a/include/dt-bindings/clock/r8a7790-clock.h
> +++ b/include/dt-bindings/clock/r8a7790-clock.h
> @@ -107,4 +107,30 @@
>  #define R8A7790_CLK_I2C1               30
>  #define R8A7790_CLK_I2C0               31
>
> +/* MSTP10 */
> +#define R8A7790_CLK_SSI                        5

Should this be called R8A7790_CLK_SSI_ALL?

> +#define R8A7790_CLK_SSI9               6
> +#define R8A7790_CLK_SSI8               7
> +#define R8A7790_CLK_SSI7               8
> +#define R8A7790_CLK_SSI6               9
> +#define R8A7790_CLK_SSI5               10
> +#define R8A7790_CLK_SSI4               11
> +#define R8A7790_CLK_SSI3               12
> +#define R8A7790_CLK_SSI2               13
> +#define R8A7790_CLK_SSI1               14
> +#define R8A7790_CLK_SSI0               15
> +#define R8A7790_CLK_SCU                        17

Should this be called R8A7790_CLK_SCU_ALL?

> +#define R8A7790_CLK_DVC1               18
> +#define R8A7790_CLK_DVC0               19

"#define R8A7790_CLK_SCU_CTU1x_MIX1 20" and
"#define R8A7790_CLK_SCU_CTU0x_MIX0 21" are missing?

> +#define R8A7790_CLK_SRC9               22
> +#define R8A7790_CLK_SRC8               23
> +#define R8A7790_CLK_SRC7               24
> +#define R8A7790_CLK_SRC6               25
> +#define R8A7790_CLK_SRC5               26
> +#define R8A7790_CLK_SRC4               27
> +#define R8A7790_CLK_SRC3               28
> +#define R8A7790_CLK_SRC2               29
> +#define R8A7790_CLK_SRC1               30
> +#define R8A7790_CLK_SRC0               31

Should these be called R8A7790_CLK_SCU_SRC[0-9]?

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
Geert Uytterhoeven June 10, 2014, 7:35 a.m. UTC | #2
On Tue, Jun 10, 2014 at 9:32 AM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
>> +#define R8A7790_CLK_SSI                        5
>
> Should this be called R8A7790_CLK_SSI_ALL?

>> +#define R8A7790_CLK_SCU                        17
>
> Should this be called R8A7790_CLK_SCU_ALL?

>> +#define R8A7790_CLK_SRC9               22
>> +#define R8A7790_CLK_SRC8               23
>> +#define R8A7790_CLK_SRC7               24
>> +#define R8A7790_CLK_SRC6               25
>> +#define R8A7790_CLK_SRC5               26
>> +#define R8A7790_CLK_SRC4               27
>> +#define R8A7790_CLK_SRC3               28
>> +#define R8A7790_CLK_SRC2               29
>> +#define R8A7790_CLK_SRC1               30
>> +#define R8A7790_CLK_SRC0               31
>
> Should these be called R8A7790_CLK_SCU_SRC[0-9]?

If forgot: in case the above are changed, the values in "clock-output-names"
should be renamed, 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
--
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
Kuninori Morimoto June 10, 2014, 7:48 a.m. UTC | #3
Hi Geert

> > +               mstp10_clks: mstp10_clks@e6150998 {
> > +                       compatible = "renesas,r8a7790-mstp-clocks", "renesas,cpg-mstp-clocks";
> > +                       reg = <0 0xe6150998 0 4>, <0 0xe61509a8 0 4>;
> > +                       clocks = <&p_clk>, /* parent of SCU */
> > +                               <&p_clk>,
> > +                               <&p_clk>, <&p_clk>, <&p_clk>, <&p_clk>, <&p_clk>,
> > +                               <&p_clk>, <&p_clk>, <&p_clk>, <&p_clk>, <&p_clk>,
> > +                               <&mstp10_clks R8A7790_CLK_SCU>, <&mstp10_clks R8A7790_CLK_SCU>,
> > +                               <&mstp10_clks R8A7790_CLK_SCU>, <&mstp10_clks R8A7790_CLK_SCU>,
> > +                               <&mstp10_clks R8A7790_CLK_SCU>, <&mstp10_clks R8A7790_CLK_SCU>,
> > +                               <&mstp10_clks R8A7790_CLK_SCU>, <&mstp10_clks R8A7790_CLK_SCU>,
> > +                               <&mstp10_clks R8A7790_CLK_SCU>, <&mstp10_clks R8A7790_CLK_SCU>,
> > +                               <&mstp10_clks R8A7790_CLK_SCU>, <&mstp10_clks R8A7790_CLK_SCU>;
> > +
> > +                       #clock-cells = <1>;
> > +                       clock-indices = <
> > +                               R8A7790_CLK_SCU
> > +                               R8A7790_CLK_SSI
> > +                               R8A7790_CLK_SSI9 R8A7790_CLK_SSI8 R8A7790_CLK_SSI7 R8A7790_CLK_SSI6 R8A7790_CLK_SSI5
> > +                               R8A7790_CLK_SSI4 R8A7790_CLK_SSI3 R8A7790_CLK_SSI2 R8A7790_CLK_SSI1 R8A7790_CLK_SSI0
> > +                               R8A7790_CLK_DVC1 R8A7790_CLK_DVC0
> > +                               R8A7790_CLK_SRC9 R8A7790_CLK_SRC8 R8A7790_CLK_SRC7 R8A7790_CLK_SRC6 R8A7790_CLK_SRC5
> > +                               R8A7790_CLK_SRC4 R8A7790_CLK_SRC3 R8A7790_CLK_SRC2 R8A7790_CLK_SRC1 R8A7790_CLK_SRC0
> 
> I think we usually put these in numerical order here.

Do you mean R8A7790_CLK_SCU is out-of-order ?
If so, Unfortunately, we need put "R8A7790_CLK_SCU" before "R8A7790_CLK_SRCx",
because SCU(ALL) is parent clock of SRCn.

> > +#define R8A7790_CLK_DVC1               18
> > +#define R8A7790_CLK_DVC0               19
> 
> "#define R8A7790_CLK_SCU_CTU1x_MIX1 20" and
> "#define R8A7790_CLK_SCU_CTU0x_MIX0 21" are missing?

Yes, CTU/MIX is not supported at this point in sound driver.
I don't know I should add it or not

> > +/* MSTP10 */
> > +#define R8A7790_CLK_SSI                        5
> 
> Should this be called R8A7790_CLK_SSI_ALL?
(snip)
> > +#define R8A7790_CLK_SCU                        17
> 
> Should this be called R8A7790_CLK_SCU_ALL?
(snip)
> > +#define R8A7790_CLK_SRC9               22
> > +#define R8A7790_CLK_SRC8               23
> > +#define R8A7790_CLK_SRC7               24
> > +#define R8A7790_CLK_SRC6               25
> > +#define R8A7790_CLK_SRC5               26
> > +#define R8A7790_CLK_SRC4               27
> > +#define R8A7790_CLK_SRC3               28
> > +#define R8A7790_CLK_SRC2               29
> > +#define R8A7790_CLK_SRC1               30
> > +#define R8A7790_CLK_SRC0               31
> 
> Should these be called R8A7790_CLK_SCU_SRC[0-9]?

Using datasheet name will not create new confusion.
Thank you, will fix.
--
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
Geert Uytterhoeven June 10, 2014, 8:10 a.m. UTC | #4
Hi Morimoto-san,

On Tue, Jun 10, 2014 at 9:48 AM, Kuninori Morimoto
<kuninori.morimoto.gx@renesas.com> wrote:
>> > +               mstp10_clks: mstp10_clks@e6150998 {
>> > +                       compatible = "renesas,r8a7790-mstp-clocks", "renesas,cpg-mstp-clocks";
>> > +                       reg = <0 0xe6150998 0 4>, <0 0xe61509a8 0 4>;
>> > +                       clocks = <&p_clk>, /* parent of SCU */
>> > +                               <&p_clk>,
>> > +                               <&p_clk>, <&p_clk>, <&p_clk>, <&p_clk>, <&p_clk>,
>> > +                               <&p_clk>, <&p_clk>, <&p_clk>, <&p_clk>, <&p_clk>,
>> > +                               <&mstp10_clks R8A7790_CLK_SCU>, <&mstp10_clks R8A7790_CLK_SCU>,
>> > +                               <&mstp10_clks R8A7790_CLK_SCU>, <&mstp10_clks R8A7790_CLK_SCU>,
>> > +                               <&mstp10_clks R8A7790_CLK_SCU>, <&mstp10_clks R8A7790_CLK_SCU>,
>> > +                               <&mstp10_clks R8A7790_CLK_SCU>, <&mstp10_clks R8A7790_CLK_SCU>,
>> > +                               <&mstp10_clks R8A7790_CLK_SCU>, <&mstp10_clks R8A7790_CLK_SCU>,
>> > +                               <&mstp10_clks R8A7790_CLK_SCU>, <&mstp10_clks R8A7790_CLK_SCU>;
>> > +
>> > +                       #clock-cells = <1>;
>> > +                       clock-indices = <
>> > +                               R8A7790_CLK_SCU
>> > +                               R8A7790_CLK_SSI
>> > +                               R8A7790_CLK_SSI9 R8A7790_CLK_SSI8 R8A7790_CLK_SSI7 R8A7790_CLK_SSI6 R8A7790_CLK_SSI5
>> > +                               R8A7790_CLK_SSI4 R8A7790_CLK_SSI3 R8A7790_CLK_SSI2 R8A7790_CLK_SSI1 R8A7790_CLK_SSI0
>> > +                               R8A7790_CLK_DVC1 R8A7790_CLK_DVC0
>> > +                               R8A7790_CLK_SRC9 R8A7790_CLK_SRC8 R8A7790_CLK_SRC7 R8A7790_CLK_SRC6 R8A7790_CLK_SRC5
>> > +                               R8A7790_CLK_SRC4 R8A7790_CLK_SRC3 R8A7790_CLK_SRC2 R8A7790_CLK_SRC1 R8A7790_CLK_SRC0
>>
>> I think we usually put these in numerical order here.
>
> Do you mean R8A7790_CLK_SCU is out-of-order ?

I mean they should be in the same order as listed in
include/dt-bindings/clock/r8a7790-clock.h

> If so, Unfortunately, we need put "R8A7790_CLK_SCU" before "R8A7790_CLK_SRCx",
> because SCU(ALL) is parent clock of SRCn.

IC. I didn't look up the parent clock relations.
However, SCU_ALL == 17, while its children are 18-31, so numerical order
should be fine.

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
Kuninori Morimoto June 10, 2014, 8:23 a.m. UTC | #5
Hi Geert, Simon


> > If so, Unfortunately, we need put "R8A7790_CLK_SCU" before "R8A7790_CLK_SRCx",
> > because SCU(ALL) is parent clock of SRCn.
> 
> IC. I didn't look up the parent clock relations.
> However, SCU_ALL == 17, while its children are 18-31, so numerical order
> should be fine.

Hmm.. indeed
I had been confused about it.

I sent v2 patches before getting your feedback...
Before sending v3 patches, I want to get Simon's opinion.
I will send v3 patches if Simon can accept [1/3][2/3] (and [3/3] too?) now.
If not, I will say "Thank you for your feedback" to Geert,
but, not send v3. because this is [RFC] patches.

--
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 mbox

Patch

diff --git a/arch/arm/boot/dts/r8a7790.dtsi b/arch/arm/boot/dts/r8a7790.dtsi
index e990d3c..cb8a68d 100644
--- a/arch/arm/boot/dts/r8a7790.dtsi
+++ b/arch/arm/boot/dts/r8a7790.dtsi
@@ -835,6 +835,38 @@ 
 				"rcan1", "rcan0", "qspi_mod", "iic3",
 				"i2c3", "i2c2", "i2c1", "i2c0";
 		};
+		mstp10_clks: mstp10_clks@e6150998 {
+			compatible = "renesas,r8a7790-mstp-clocks", "renesas,cpg-mstp-clocks";
+			reg = <0 0xe6150998 0 4>, <0 0xe61509a8 0 4>;
+			clocks = <&p_clk>, /* parent of SCU */
+				<&p_clk>,
+				<&p_clk>, <&p_clk>, <&p_clk>, <&p_clk>, <&p_clk>,
+				<&p_clk>, <&p_clk>, <&p_clk>, <&p_clk>, <&p_clk>,
+				<&mstp10_clks R8A7790_CLK_SCU>, <&mstp10_clks R8A7790_CLK_SCU>,
+				<&mstp10_clks R8A7790_CLK_SCU>, <&mstp10_clks R8A7790_CLK_SCU>,
+				<&mstp10_clks R8A7790_CLK_SCU>, <&mstp10_clks R8A7790_CLK_SCU>,
+				<&mstp10_clks R8A7790_CLK_SCU>, <&mstp10_clks R8A7790_CLK_SCU>,
+				<&mstp10_clks R8A7790_CLK_SCU>, <&mstp10_clks R8A7790_CLK_SCU>,
+				<&mstp10_clks R8A7790_CLK_SCU>, <&mstp10_clks R8A7790_CLK_SCU>;
+
+			#clock-cells = <1>;
+			clock-indices = <
+				R8A7790_CLK_SCU
+				R8A7790_CLK_SSI
+				R8A7790_CLK_SSI9 R8A7790_CLK_SSI8 R8A7790_CLK_SSI7 R8A7790_CLK_SSI6 R8A7790_CLK_SSI5
+				R8A7790_CLK_SSI4 R8A7790_CLK_SSI3 R8A7790_CLK_SSI2 R8A7790_CLK_SSI1 R8A7790_CLK_SSI0
+				R8A7790_CLK_DVC1 R8A7790_CLK_DVC0
+				R8A7790_CLK_SRC9 R8A7790_CLK_SRC8 R8A7790_CLK_SRC7 R8A7790_CLK_SRC6 R8A7790_CLK_SRC5
+				R8A7790_CLK_SRC4 R8A7790_CLK_SRC3 R8A7790_CLK_SRC2 R8A7790_CLK_SRC1 R8A7790_CLK_SRC0
+			>;
+			clock-output-names =
+				"scu", "ssi",
+				"ssi9", "ssi8", "ssi7", "ssi6", "ssi5",
+				"ssi4", "ssi3", "ssi2", "ssi1", "ssi0",
+				"dvc1", "dvc0",
+				"src9", "src8", "src7", "src6", "src5",
+				"src4", "src3", "src2", "src1", "src0";
+		};
 	};
 
 	qspi: spi@e6b10000 {
diff --git a/include/dt-bindings/clock/r8a7790-clock.h b/include/dt-bindings/clock/r8a7790-clock.h
index 1118f7a..b4430b0 100644
--- a/include/dt-bindings/clock/r8a7790-clock.h
+++ b/include/dt-bindings/clock/r8a7790-clock.h
@@ -107,4 +107,30 @@ 
 #define R8A7790_CLK_I2C1		30
 #define R8A7790_CLK_I2C0		31
 
+/* MSTP10 */
+#define R8A7790_CLK_SSI			5
+#define R8A7790_CLK_SSI9		6
+#define R8A7790_CLK_SSI8		7
+#define R8A7790_CLK_SSI7		8
+#define R8A7790_CLK_SSI6		9
+#define R8A7790_CLK_SSI5		10
+#define R8A7790_CLK_SSI4		11
+#define R8A7790_CLK_SSI3		12
+#define R8A7790_CLK_SSI2		13
+#define R8A7790_CLK_SSI1		14
+#define R8A7790_CLK_SSI0		15
+#define R8A7790_CLK_SCU			17
+#define R8A7790_CLK_DVC1		18
+#define R8A7790_CLK_DVC0		19
+#define R8A7790_CLK_SRC9		22
+#define R8A7790_CLK_SRC8		23
+#define R8A7790_CLK_SRC7		24
+#define R8A7790_CLK_SRC6		25
+#define R8A7790_CLK_SRC5		26
+#define R8A7790_CLK_SRC4		27
+#define R8A7790_CLK_SRC3		28
+#define R8A7790_CLK_SRC2		29
+#define R8A7790_CLK_SRC1		30
+#define R8A7790_CLK_SRC0		31
+
 #endif /* __DT_BINDINGS_CLOCK_R8A7790_H__ */