[PATCH 0/4 v3][RFC] arm64: Renesas Gen3 initial patch

[PATCH 0/4 v3][RFC] arm64: Renesas Gen3 initial patch

[Kuninori Morimoto]

Hi SH-ML

These are v3 patch set for Gen3 support. Almost all are same as v2.
v3 tidyuped defconfig patch which included unsupported SCIF.
It still has CGP patch which will be updated by Geert.

Gaku Inami (3):
      clk: shmobile: add Renesas R-Car Gen3 CPG support
      arm64: Add new Renesas R-Car Gen3 SoC Kconfig
      arm64: renesas: Add initial r8a7795 SoC support

Kuninori Morimoto (1):
      arm64: defconfig: Enable Renesas R-Car Gen3 SoC in defconfig

 Documentation/devicetree/bindings/arm/shmobile.txt                       |   2 +
 Documentation/devicetree/bindings/clock/renesas,cpg-mstp-clocks.txt      |   1 +
 Documentation/devicetree/bindings/clock/renesas,rcar-gen3-cpg-clocks.txt |  31 ++++++++++++
 arch/arm64/Kconfig                                                       |   9 ++++
 arch/arm64/boot/dts/Makefile                                             |   1 +
 arch/arm64/boot/dts/renesas/Makefile                                     |   5 ++
 arch/arm64/boot/dts/renesas/r8a7795.dtsi                                 |  93 +++++++++++++++++++++++++++++++++++
 arch/arm64/configs/defconfig                                             |   1 +
 drivers/clk/shmobile/Makefile                                            |   1 +
 drivers/clk/shmobile/clk-rcar-gen3.c                                     | 232 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 include/dt-bindings/clock/r8a7795-clock.h                                |  31 ++++++++++++
 11 files changed, 407 insertions(+)


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

[PATCH 3/4 v3][RFC] arm64: renesas: Add initial r8a7795 SoC support

[Kuninori Morimoto]

From: Gaku Inami <gaku.inami.xw@bp.renesas.com>

Signed-off-by: Gaku Inami <gaku.inami.xw@bp.renesas.com>
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
v2 -> v3

 - no change

 Documentation/devicetree/bindings/arm/shmobile.txt |  2 +
 .../bindings/clock/renesas,cpg-mstp-clocks.txt     |  1 +
 arch/arm64/boot/dts/Makefile                       |  1 +
 arch/arm64/boot/dts/renesas/Makefile               |  5 ++
 arch/arm64/boot/dts/renesas/r8a7795.dtsi           | 93 ++++++++++++++++++++++
 include/dt-bindings/clock/r8a7795-clock.h          | 31 ++++++++
 6 files changed, 133 insertions(+)
 create mode 100644 arch/arm64/boot/dts/renesas/Makefile
 create mode 100644 arch/arm64/boot/dts/renesas/r8a7795.dtsi
 create mode 100644 include/dt-bindings/clock/r8a7795-clock.h

diff --git a/Documentation/devicetree/bindings/arm/shmobile.txt b/Documentation/devicetree/bindings/arm/shmobile.txt
index c4f19b2..8d696a0 100644
--- a/Documentation/devicetree/bindings/arm/shmobile.txt
+++ b/Documentation/devicetree/bindings/arm/shmobile.txt
@@ -27,6 +27,8 @@ SoCs:
     compatible = "renesas,r8a7793"
   - R-Car E2 (R8A77940)
     compatible = "renesas,r8a7794"
+  - R-Car H3 (R8A77950)
+    compatible = "renesas,r8a7795"
 
 
 Boards:
diff --git a/Documentation/devicetree/bindings/clock/renesas,cpg-mstp-clocks.txt b/Documentation/devicetree/bindings/clock/renesas,cpg-mstp-clocks.txt
index 16ed181..4169c76 100644
--- a/Documentation/devicetree/bindings/clock/renesas,cpg-mstp-clocks.txt
+++ b/Documentation/devicetree/bindings/clock/renesas,cpg-mstp-clocks.txt
@@ -19,6 +19,7 @@ Required Properties:
     - "renesas,r8a7791-mstp-clocks" for R8A7791 (R-Car M2-W) MSTP gate clocks
     - "renesas,r8a7793-mstp-clocks" for R8A7793 (R-Car M2-N) MSTP gate clocks
     - "renesas,r8a7794-mstp-clocks" for R8A7794 (R-Car E2) MSTP gate clocks
+    - "renesas,r8a7795-mstp-clocks" for R8A7795 (R-Car H3) MSTP gate clocks
     - "renesas,sh73a0-mstp-clocks" for SH73A0 (SH-MobileAG5) MSTP gate clocks
     and "renesas,cpg-mstp-clocks" as a fallback.
   - reg: Base address and length of the I/O mapped registers used by the MSTP
diff --git a/arch/arm64/boot/dts/Makefile b/arch/arm64/boot/dts/Makefile
index 38913be2..5691ca4 100644
--- a/arch/arm64/boot/dts/Makefile
+++ b/arch/arm64/boot/dts/Makefile
@@ -7,6 +7,7 @@ dts-dirs += freescale
 dts-dirs += hisilicon
 dts-dirs += mediatek
 dts-dirs += qcom
+dts-dirs += renesas
 dts-dirs += sprd
 dts-dirs += xilinx
 
diff --git a/arch/arm64/boot/dts/renesas/Makefile b/arch/arm64/boot/dts/renesas/Makefile
new file mode 100644
index 0000000..6aeefd9
--- /dev/null
+++ b/arch/arm64/boot/dts/renesas/Makefile
@@ -0,0 +1,5 @@
+dtb-$(CONFIG_ARCH_RCAR_GEN3) ++
+always		:= $(dtb-y)
+subdir-y	:= $(dts-dirs)
+clean-files	:= *.dtb
diff --git a/arch/arm64/boot/dts/renesas/r8a7795.dtsi b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
new file mode 100644
index 0000000..0f298c3
--- /dev/null
+++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
@@ -0,0 +1,93 @@
+/*
+ * Device Tree Source for the r8a7795 SoC
+ *
+ * Copyright (C) 2015 Renesas Electronics Corp.
+ *
+ * This file is licensed under the terms of the GNU General Public License
+ * version 2.  This program is licensed "as is" without any warranty of any
+ * kind, whether express or implied.
+ */
+
+#include <dt-bindings/clock/r8a7795-clock.h>
+#include <dt-bindings/interrupt-controller/arm-gic.h>
+
+/ {
+	compatible = "renesas,r8a7795";
+	interrupt-parent = <&gic>;
+	#address-cells = <2>;
+	#size-cells = <2>;
+
+	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		/* 1core only at this point */
+		a57_0: cpu@0 {
+			compatible = "arm,cortex-a57", "arm,armv8";
+			reg = <0x0>;
+			device_type = "cpu";
+		};
+	};
+
+	gic: interrupt-controller@0xf1010000 {
+		compatible = "arm,gic-400";
+		#interrupt-cells = <3>;
+		#address-cells = <0>;
+		interrupt-controller;
+		reg = <0x0 0xf1010000 0 0x1000>,
+		      <0x0 0xf1020000 0 0x2000>;
+		interrupts = <GIC_PPI 9
+				(GIC_CPU_MASK_SIMPLE(1) | IRQ_TYPE_LEVEL_HIGH)>;
+	};
+
+	timer {
+		compatible = "arm,armv8-timer";
+		interrupts = <GIC_PPI 13
+				(GIC_CPU_MASK_SIMPLE(1) | IRQ_TYPE_LEVEL_LOW)>,
+			     <GIC_PPI 14
+				(GIC_CPU_MASK_SIMPLE(1) | IRQ_TYPE_LEVEL_LOW)>,
+			     <GIC_PPI 11
+				(GIC_CPU_MASK_SIMPLE(1) | IRQ_TYPE_LEVEL_LOW)>,
+			     <GIC_PPI 10
+				(GIC_CPU_MASK_SIMPLE(1) | IRQ_TYPE_LEVEL_LOW)>;
+	};
+
+	clocks {
+		#address-cells = <2>;
+		#size-cells = <2>;
+		ranges;
+
+		extal_clk: extal_clk {
+			compatible = "fixed-clock";
+			#clock-cells = <0>;
+			clock-frequency = <0>;
+			clock-output-names = "extal";
+		};
+		cpg_clocks: cpg_clocks@e6150000 {
+			compatible = "renesas,r8a7795-cpg-clocks",
+				     "renesas,rcar-gen3-cpg-clocks";
+			reg = <0 0xe6150000 0 0x1000>;
+			clocks = <&extal_clk>;
+			#clock-cells = <1>;
+			clock-output-names = "main", "pll0", "pll1","pll2",
+					     "pll3", "pll4";
+		};
+		p_clk: p_clk {
+			compatible = "fixed-factor-clock";
+			clocks = <&cpg_clocks RCAR_GEN3_CLK_PLL1>;
+			#clock-cells = <0>;
+			clock-div = <24>;
+			clock-mult = <1>;
+			clock-output-names = "p";
+		};
+		mstp3_clks: mstp3_clks@e615013c {
+			compatible = "renesas,r8a7795-mstp-clocks",
+				     "renesas,cpg-mstp-clocks";
+			reg = <0 0xe615013c 0 4>, <0 0xe6150048 0 4>;
+			clocks =  <&p_clk>;
+			#clock-cells = <1>;
+			renesas,clock-indices = <RCAR_GEN3_CLK_IRDA>;
+			clock-output-names = "irda";
+		};
+	};
+};
diff --git a/include/dt-bindings/clock/r8a7795-clock.h b/include/dt-bindings/clock/r8a7795-clock.h
new file mode 100644
index 0000000..fc1c4da
--- /dev/null
+++ b/include/dt-bindings/clock/r8a7795-clock.h
@@ -0,0 +1,31 @@
+#ifndef __DT_BINDINGS_CLOCK_RCAR_GEN3_H__
+#define __DT_BINDINGS_CLOCK_RCAR_GEN3_H__
+
+/* CPG */
+#define RCAR_GEN3_CLK_MAIN		0
+#define RCAR_GEN3_CLK_PLL0		1
+#define RCAR_GEN3_CLK_PLL1		2
+#define RCAR_GEN3_CLK_PLL2		3
+#define RCAR_GEN3_CLK_PLL3		4
+#define RCAR_GEN3_CLK_PLL4		5
+
+/* MSTP0 */
+
+/* MSTP1 */
+
+/* MSTP2 */
+
+/* MSTP3 */
+#define RCAR_GEN3_CLK_IRDA		10
+
+/* MSTP5 */
+
+/* MSTP7 */
+
+/* MSTP8 */
+
+/* MSTP9 */
+
+/* MSTP10 */
+
+#endif /* __DT_BINDINGS_CLOCK_RCAR_GEN3_H__ */
-- 
1.9.1

Re: [PATCH 0/4 v3][RFC] arm64: Renesas Gen3 initial patch

[Simon Horman]

Hi Morimoto-san,

I plan to queue these up in a topic branch, topic/arm64-rcar-gen3-v3,
of the renesas tree. I plan for the base of the branch to be v4.2-rc1,
currently the base for work I have queued up up for v4.2 (I am not
queuing up this series for v4.2 at this time). Please let me know
if a different base, e.g. v4.1, would be more appropriate.

On Mon, Aug 03, 2015 at 01:51:12AM +0000, Kuninori Morimoto wrote:
> 
> Hi SH-ML
> 
> These are v3 patch set for Gen3 support. Almost all are same as v2.
> v3 tidyuped defconfig patch which included unsupported SCIF.
> It still has CGP patch which will be updated by Geert.
> 
> Gaku Inami (3):
>       clk: shmobile: add Renesas R-Car Gen3 CPG support
>       arm64: Add new Renesas R-Car Gen3 SoC Kconfig
>       arm64: renesas: Add initial r8a7795 SoC support
> 
> Kuninori Morimoto (1):
>       arm64: defconfig: Enable Renesas R-Car Gen3 SoC in defconfig
> 
>  Documentation/devicetree/bindings/arm/shmobile.txt                       |   2 +
>  Documentation/devicetree/bindings/clock/renesas,cpg-mstp-clocks.txt      |   1 +
>  Documentation/devicetree/bindings/clock/renesas,rcar-gen3-cpg-clocks.txt |  31 ++++++++++++
>  arch/arm64/Kconfig                                                       |   9 ++++
>  arch/arm64/boot/dts/Makefile                                             |   1 +
>  arch/arm64/boot/dts/renesas/Makefile                                     |   5 ++
>  arch/arm64/boot/dts/renesas/r8a7795.dtsi                                 |  93 +++++++++++++++++++++++++++++++++++
>  arch/arm64/configs/defconfig                                             |   1 +
>  drivers/clk/shmobile/Makefile                                            |   1 +
>  drivers/clk/shmobile/clk-rcar-gen3.c                                     | 232 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/dt-bindings/clock/r8a7795-clock.h                                |  31 ++++++++++++
>  11 files changed, 407 insertions(+)
> 
> 
> --
> 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
> --
> 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
> 

Re: [PATCH 0/4 v3][RFC] arm64: Renesas Gen3 initial patch

[Simon Horman]

On Mon, Aug 03, 2015 at 11:44:14AM +0900, Simon Horman wrote:
> Hi Morimoto-san,
> 
> I plan to queue these up in a topic branch, topic/arm64-rcar-gen3-v3,
> of the renesas tree. I plan for the base of the branch to be v4.2-rc1,
> currently the base for work I have queued up up for v4.2 (I am not
> queuing up this series for v4.2 at this time). Please let me know
> if a different base, e.g. v4.1, would be more appropriate.

I have pushed the topic/arm64-rcar-gen3-v3 branch as described above.

Re: [PATCH 0/4 v3][RFC] arm64: Renesas Gen3 initial patch

[Kuninori Morimoto]

Hi Simon

> On Mon, Aug 03, 2015 at 11:44:14AM +0900, Simon Horman wrote:
> > Hi Morimoto-san,
> > 
> > I plan to queue these up in a topic branch, topic/arm64-rcar-gen3-v3,
> > of the renesas tree. I plan for the base of the branch to be v4.2-rc1,
> > currently the base for work I have queued up up for v4.2 (I am not
> > queuing up this series for v4.2 at this time). Please let me know
> > if a different base, e.g. v4.1, would be more appropriate.
> 
> I have pushed the topic/arm64-rcar-gen3-v3 branch as described above.

Thank you !!

Re: [PATCH 0/4 v3][RFC] arm64: Renesas Gen3 initial patch

[Geert Uytterhoeven]

Hi Simon et al,

On Mon, Aug 3, 2015 at 8:59 AM, Simon Horman <horms@verge.net.au> wrote:
> On Mon, Aug 03, 2015 at 11:44:14AM +0900, Simon Horman wrote:
>> I plan to queue these up in a topic branch, topic/arm64-rcar-gen3-v3,
>> of the renesas tree. I plan for the base of the branch to be v4.2-rc1,
>> currently the base for work I have queued up up for v4.2 (I am not
>> queuing up this series for v4.2 at this time). Please let me know
>> if a different base, e.g. v4.1, would be more appropriate.
>
> I have pushed the topic/arm64-rcar-gen3-v3 branch as described above.

That branch is not included in renesas-devel-20150803-v4.2-rc5.
Is that intentional?

Please note that arm-soc/for-next contains commits eed6b3eb20b97bef ("arm64:
Split out platform options to separate Kconfig", [1]) and fbac1c81e2591c5d
("arm64: add Rockchip architecture entry", [2]), causing merge conflicts.

Nothing I can't handle myself (for now), though...

[1] https://git.kernel.org/cgit/linux/kernel/git/arm/arm-soc.git/commit/?h=for-next&idîd6b3eb20b97beff66ca59bb610ac73532278ee
[2] https://git.kernel.org/cgit/linux/kernel/git/arm/arm-soc.git/commit/?h=for-next&idûac1c81e2591c5d1e5abd9a4477002f2afd0ab4

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 0/4 v3][RFC] arm64: Renesas Gen3 initial patch

[Simon Horman]

On Mon, Aug 03, 2015 at 06:56:59PM +0200, Geert Uytterhoeven wrote:
> Hi Simon et al,
> 
> On Mon, Aug 3, 2015 at 8:59 AM, Simon Horman <horms@verge.net.au> wrote:
> > On Mon, Aug 03, 2015 at 11:44:14AM +0900, Simon Horman wrote:
> >> I plan to queue these up in a topic branch, topic/arm64-rcar-gen3-v3,
> >> of the renesas tree. I plan for the base of the branch to be v4.2-rc1,
> >> currently the base for work I have queued up up for v4.2 (I am not
> >> queuing up this series for v4.2 at this time). Please let me know
> >> if a different base, e.g. v4.1, would be more appropriate.
> >
> > I have pushed the topic/arm64-rcar-gen3-v3 branch as described above.
> 
> That branch is not included in renesas-devel-20150803-v4.2-rc5.
> Is that intentional?

Yes, at least for now.

My intention was that:

renesas-next: is for code targeted at the forthcoming release, currently v4.3
renesas-devel: is for code not appropriate for next _yet_.
               e.g.: Code for the release after the forthcoming one,
	             currently v4.4. Or code queued up while next is
		     closed: for us that basically means between rc6 and rc1.

If we would like a merge branch that pulls together more alpha code, such as
topic/arm64-rcar-gen3-v3, then I would like to create a new merge branch.

> Please note that arm-soc/for-next contains commits eed6b3eb20b97bef ("arm64:
> Split out platform options to separate Kconfig", [1]) and fbac1c81e2591c5d
> ("arm64: add Rockchip architecture entry", [2]), causing merge conflicts.
> 
> Nothing I can't handle myself (for now), though...
> 
> [1] https://git.kernel.org/cgit/linux/kernel/git/arm/arm-soc.git/commit/?h=for-next&idîd6b3eb20b97beff66ca59bb610ac73532278ee
> [2] https://git.kernel.org/cgit/linux/kernel/git/arm/arm-soc.git/commit/?h=for-next&idûac1c81e2591c5d1e5abd9a4477002f2afd0ab4

At this point I think it would best if you handled that.

I can start using different bases, or merging arm-soc/for-next etc.. into
a merge branch if it seems useful.

Re: [PATCH 3/4 v3][RFC] arm64: renesas: Add initial r8a7795 SoC support

[Kuninori Morimoto]

Hi

> diff --git a/include/dt-bindings/clock/r8a7795-clock.h b/include/dt-bindings/clock/r8a7795-clock.h
> new file mode 100644
> index 0000000..fc1c4da
> --- /dev/null
> +++ b/include/dt-bindings/clock/r8a7795-clock.h
> @@ -0,0 +1,31 @@
> +#ifndef __DT_BINDINGS_CLOCK_RCAR_GEN3_H__
> +#define __DT_BINDINGS_CLOCK_RCAR_GEN3_H__
> +
> +/* CPG */
> +#define RCAR_GEN3_CLK_MAIN		0
> +#define RCAR_GEN3_CLK_PLL0		1
> +#define RCAR_GEN3_CLK_PLL1		2
> +#define RCAR_GEN3_CLK_PLL2		3
> +#define RCAR_GEN3_CLK_PLL3		4
> +#define RCAR_GEN3_CLK_PLL4		5
(snip)

About this, file name is r8a7795-clock.h, and it is using GEN3 for each definition.
I thought all Gen3 series (will) have compatible mapping, but maybe it can't.
So, maybe using SoC naming (same as before) in v4 is good idea.

Re: [PATCH 3/4 v3][RFC] arm64: renesas: Add initial r8a7795 SoC support

[Laurent Pinchart]

Hi Morimoto-san,

Thank you for the patch.

On Monday 03 August 2015 01:53:23 Kuninori Morimoto wrote:
> From: Gaku Inami <gaku.inami.xw@bp.renesas.com>
> 
> Signed-off-by: Gaku Inami <gaku.inami.xw@bp.renesas.com>
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---
> v2 -> v3
> 
>  - no change
> 
>  Documentation/devicetree/bindings/arm/shmobile.txt |  2 +
>  .../bindings/clock/renesas,cpg-mstp-clocks.txt     |  1 +
>  arch/arm64/boot/dts/Makefile                       |  1 +
>  arch/arm64/boot/dts/renesas/Makefile               |  5 ++
>  arch/arm64/boot/dts/renesas/r8a7795.dtsi           | 93 +++++++++++++++++++
>  include/dt-bindings/clock/r8a7795-clock.h          | 31 ++++++++
>  6 files changed, 133 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/renesas/Makefile
>  create mode 100644 arch/arm64/boot/dts/renesas/r8a7795.dtsi
>  create mode 100644 include/dt-bindings/clock/r8a7795-clock.h

[snip]

> diff --git a/arch/arm64/boot/dts/renesas/r8a7795.dtsi
> b/arch/arm64/boot/dts/renesas/r8a7795.dtsi new file mode 100644
> index 0000000..0f298c3
> --- /dev/null
> +++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
> @@ -0,0 +1,93 @@
> +/*
> + * Device Tree Source for the r8a7795 SoC
> + *
> + * Copyright (C) 2015 Renesas Electronics Corp.
> + *
> + * This file is licensed under the terms of the GNU General Public License
> + * version 2.  This program is licensed "as is" without any warranty of any
> + * kind, whether express or implied.
> + */
> +
> +#include <dt-bindings/clock/r8a7795-clock.h>
> +#include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +/ {
> +	compatible = "renesas,r8a7795";
> +	interrupt-parent = <&gic>;
> +	#address-cells = <2>;
> +	#size-cells = <2>;
> +
> +	cpus {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		/* 1core only at this point */
> +		a57_0: cpu@0 {
> +			compatible = "arm,cortex-a57", "arm,armv8";
> +			reg = <0x0>;
> +			device_type = "cpu";
> +		};
> +	};
> +
> +	gic: interrupt-controller@0xf1010000 {
> +		compatible = "arm,gic-400";
> +		#interrupt-cells = <3>;
> +		#address-cells = <0>;
> +		interrupt-controller;
> +		reg = <0x0 0xf1010000 0 0x1000>,
> +		      <0x0 0xf1020000 0 0x2000>;
> +		interrupts = <GIC_PPI 9
> +				(GIC_CPU_MASK_SIMPLE(1) | IRQ_TYPE_LEVEL_HIGH)>;
> +	};

Shouldn't the memory-mapped peripherals be put under a bus node instead of the 
root DT node ?

> +	timer {
> +		compatible = "arm,armv8-timer";
> +		interrupts = <GIC_PPI 13
> +				(GIC_CPU_MASK_SIMPLE(1) | IRQ_TYPE_LEVEL_LOW)>,
> +			     <GIC_PPI 14
> +				(GIC_CPU_MASK_SIMPLE(1) | IRQ_TYPE_LEVEL_LOW)>,
> +			     <GIC_PPI 11
> +				(GIC_CPU_MASK_SIMPLE(1) | IRQ_TYPE_LEVEL_LOW)>,
> +			     <GIC_PPI 10
> +				(GIC_CPU_MASK_SIMPLE(1) | IRQ_TYPE_LEVEL_LOW)>;
> +	};
> +
> +	clocks {

Let's try to make it right from the start on Gen3. The CPG node should be a 
direct child of the bus node mentioned above, and the MSTP clocks should be 
children of the CPG node.

I'm not sure where to put the non-memory-mapped clocks though, should they be 
directly under the root node ? It would make sense for extal_clk, but how 
about the fixed-factor clocks ? Should they be children of the CPG node too ?

> +		#address-cells = <2>;
> +		#size-cells = <2>;
> +		ranges;
> +
> +		extal_clk: extal_clk {
> +			compatible = "fixed-clock";
> +			#clock-cells = <0>;
> +			clock-frequency = <0>;
> +			clock-output-names = "extal";
> +		};
> +		cpg_clocks: cpg_clocks@e6150000 {
> +			compatible = "renesas,r8a7795-cpg-clocks",
> +				     "renesas,rcar-gen3-cpg-clocks";
> +			reg = <0 0xe6150000 0 0x1000>;
> +			clocks = <&extal_clk>;
> +			#clock-cells = <1>;
> +			clock-output-names = "main", "pll0", "pll1","pll2",
> +					     "pll3", "pll4";
> +		};
> +		p_clk: p_clk {
> +			compatible = "fixed-factor-clock";
> +			clocks = <&cpg_clocks RCAR_GEN3_CLK_PLL1>;
> +			#clock-cells = <0>;
> +			clock-div = <24>;
> +			clock-mult = <1>;
> +			clock-output-names = "p";
> +		};
> +		mstp3_clks: mstp3_clks@e615013c {
> +			compatible = "renesas,r8a7795-mstp-clocks",
> +				     "renesas,cpg-mstp-clocks";
> +			reg = <0 0xe615013c 0 4>, <0 0xe6150048 0 4>;
> +			clocks =  <&p_clk>;
> +			#clock-cells = <1>;
> +			renesas,clock-indices = <RCAR_GEN3_CLK_IRDA>;
> +			clock-output-names = "irda";
> +		};
> +	};
> +};

-- 
Regards,

Laurent Pinchart

Re: [PATCH 3/4 v3][RFC] arm64: renesas: Add initial r8a7795 SoC support

[Geert Uytterhoeven]

On Tue, Aug 4, 2015 at 2:22 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Monday 03 August 2015 01:53:23 Kuninori Morimoto wrote:
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
>> @@ -0,0 +1,93 @@
>> +/ {

>> +     clocks {
>
> Let's try to make it right from the start on Gen3. The CPG node should be a
> direct child of the bus node mentioned above, and the MSTP clocks should be
> children of the CPG node.

Agreed.

> I'm not sure where to put the non-memory-mapped clocks though, should they be
> directly under the root node ? It would make sense for extal_clk, but how
> about the fixed-factor clocks ? Should they be children of the CPG node too ?

I believe the current trend is to put clocks like "extal_clk" under the root
node.
As the fixed-factor clocks are generated by the CPG module, and we do have
a device node for it, I'd make them children of the CPG node, too.

Any comments from the clk+dt experts?

Thanks!

>> +             #address-cells = <2>;
>> +             #size-cells = <2>;
>> +             ranges;
>> +
>> +             extal_clk: extal_clk {
>> +                     compatible = "fixed-clock";
>> +                     #clock-cells = <0>;
>> +                     clock-frequency = <0>;
>> +                     clock-output-names = "extal";
>> +             };
>> +             cpg_clocks: cpg_clocks@e6150000 {
>> +                     compatible = "renesas,r8a7795-cpg-clocks",
>> +                                  "renesas,rcar-gen3-cpg-clocks";
>> +                     reg = <0 0xe6150000 0 0x1000>;
>> +                     clocks = <&extal_clk>;
>> +                     #clock-cells = <1>;
>> +                     clock-output-names = "main", "pll0", "pll1","pll2",
>> +                                          "pll3", "pll4";
>> +             };
>> +             p_clk: p_clk {
>> +                     compatible = "fixed-factor-clock";
>> +                     clocks = <&cpg_clocks RCAR_GEN3_CLK_PLL1>;
>> +                     #clock-cells = <0>;
>> +                     clock-div = <24>;
>> +                     clock-mult = <1>;
>> +                     clock-output-names = "p";
>> +             };
>> +             mstp3_clks: mstp3_clks@e615013c {
>> +                     compatible = "renesas,r8a7795-mstp-clocks",
>> +                                  "renesas,cpg-mstp-clocks";
>> +                     reg = <0 0xe615013c 0 4>, <0 0xe6150048 0 4>;
>> +                     clocks =  <&p_clk>;
>> +                     #clock-cells = <1>;
>> +                     renesas,clock-indices = <RCAR_GEN3_CLK_IRDA>;
>> +                     clock-output-names = "irda";
>> +             };
>> +     };
>> +};

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 3/4 v3][RFC] arm64: renesas: Add initial r8a7795 SoC support

[Geert Uytterhoeven]

On Tue, Aug 4, 2015 at 2:22 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Monday 03 August 2015 01:53:23 Kuninori Morimoto wrote:
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
>> @@ -0,0 +1,93 @@
>> +/ {

>> +     clocks {
>
> Let's try to make it right from the start on Gen3. The CPG node should be a
> direct child of the bus node mentioned above, and the MSTP clocks should be
> children of the CPG node.

Agreed.

> I'm not sure where to put the non-memory-mapped clocks though, should they be
> directly under the root node ? It would make sense for extal_clk, but how
> about the fixed-factor clocks ? Should they be children of the CPG node too ?

I believe the current trend is to put clocks like "extal_clk" under the root
node.
As the fixed-factor clocks are generated by the CPG module, and we do have
a device node for it, I'd make them children of the CPG node, too.

Any comments from the clk+dt experts?

Thanks!

>> +             #address-cells = <2>;
>> +             #size-cells = <2>;
>> +             ranges;
>> +
>> +             extal_clk: extal_clk {
>> +                     compatible = "fixed-clock";
>> +                     #clock-cells = <0>;
>> +                     clock-frequency = <0>;
>> +                     clock-output-names = "extal";
>> +             };
>> +             cpg_clocks: cpg_clocks@e6150000 {
>> +                     compatible = "renesas,r8a7795-cpg-clocks",
>> +                                  "renesas,rcar-gen3-cpg-clocks";
>> +                     reg = <0 0xe6150000 0 0x1000>;
>> +                     clocks = <&extal_clk>;
>> +                     #clock-cells = <1>;
>> +                     clock-output-names = "main", "pll0", "pll1","pll2",
>> +                                          "pll3", "pll4";
>> +             };
>> +             p_clk: p_clk {
>> +                     compatible = "fixed-factor-clock";
>> +                     clocks = <&cpg_clocks RCAR_GEN3_CLK_PLL1>;
>> +                     #clock-cells = <0>;
>> +                     clock-div = <24>;
>> +                     clock-mult = <1>;
>> +                     clock-output-names = "p";
>> +             };
>> +             mstp3_clks: mstp3_clks@e615013c {
>> +                     compatible = "renesas,r8a7795-mstp-clocks",
>> +                                  "renesas,cpg-mstp-clocks";
>> +                     reg = <0 0xe615013c 0 4>, <0 0xe6150048 0 4>;
>> +                     clocks =  <&p_clk>;
>> +                     #clock-cells = <1>;
>> +                     renesas,clock-indices = <RCAR_GEN3_CLK_IRDA>;
>> +                     clock-output-names = "irda";
>> +             };
>> +     };
>> +};

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 3/4 v3][RFC] arm64: renesas: Add initial r8a7795 SoC support

[Michael Turquette]

Quoting Geert Uytterhoeven (2015-08-04 05:34:06)
> On Tue, Aug 4, 2015 at 2:22 PM, Laurent Pinchart
> <laurent.pinchart@ideasonboard.com> wrote:
> > On Monday 03 August 2015 01:53:23 Kuninori Morimoto wrote:
> >> --- /dev/null
> >> +++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
> >> @@ -0,0 +1,93 @@
> >> +/ {
> =

> >> +     clocks {
> >
> > Let's try to make it right from the start on Gen3. The CPG node should =
be a
> > direct child of the bus node mentioned above, and the MSTP clocks shoul=
d be
> > children of the CPG node.
> =

> Agreed.
> =

> > I'm not sure where to put the non-memory-mapped clocks though, should t=
hey be
> > directly under the root node ? It would make sense for extal_clk, but h=
ow
> > about the fixed-factor clocks ? Should they be children of the CPG node=
 too ?
> =

> I believe the current trend is to put clocks like "extal_clk" under the r=
oot
> node.
> As the fixed-factor clocks are generated by the CPG module, and we do have
> a device node for it, I'd make them children of the CPG node, too.
> =

> Any comments from the clk+dt experts?

I don't know if anyone is an expert ;-)

extal_clk should be under the root node. That is true for all
board-level clocks and clock controllers.

Within the SoC we want to model the clock controller as a node in DT,
not necessarily all of the individual clocks. So you definitely need a
"cpg" node in DT with #clock-cells > 0.

Whether or not you enumerate the individual clocks in DT is up to you. I
do not like the data-driven approach of putting the clock definition
data into DT. It makes it awkward to do things like set a flag on a
single clock later on. Simply using the clock controller phandle plus
one or more offsets is preferred over a per-clock phandle.

Stephen and I have been discussing what a formal clock-controller
binding would look like, and one item we came up with is that any
sub-nodes of the controller would not be allowed to have a #clock-cells
property.

Also, while you're thinking about the perfect clock binding, please do
consider dropping clock-output-names if you can. Specifying clock-names
alongside the clocks property inside of the consumer node is a bit more
elegant in my opinion. This is also a bit easier if you think about
expressing your clock data with C inside of your provider driver.

Regards,
Mike

> =

> Thanks!
> =

> >> +             #address-cells =3D <2>;
> >> +             #size-cells =3D <2>;
> >> +             ranges;
> >> +
> >> +             extal_clk: extal_clk {
> >> +                     compatible =3D "fixed-clock";
> >> +                     #clock-cells =3D <0>;
> >> +                     clock-frequency =3D <0>;
> >> +                     clock-output-names =3D "extal";
> >> +             };
> >> +             cpg_clocks: cpg_clocks@e6150000 {
> >> +                     compatible =3D "renesas,r8a7795-cpg-clocks",
> >> +                                  "renesas,rcar-gen3-cpg-clocks";
> >> +                     reg =3D <0 0xe6150000 0 0x1000>;
> >> +                     clocks =3D <&extal_clk>;
> >> +                     #clock-cells =3D <1>;
> >> +                     clock-output-names =3D "main", "pll0", "pll1","p=
ll2",
> >> +                                          "pll3", "pll4";
> >> +             };
> >> +             p_clk: p_clk {
> >> +                     compatible =3D "fixed-factor-clock";
> >> +                     clocks =3D <&cpg_clocks RCAR_GEN3_CLK_PLL1>;
> >> +                     #clock-cells =3D <0>;
> >> +                     clock-div =3D <24>;
> >> +                     clock-mult =3D <1>;
> >> +                     clock-output-names =3D "p";
> >> +             };
> >> +             mstp3_clks: mstp3_clks@e615013c {
> >> +                     compatible =3D "renesas,r8a7795-mstp-clocks",
> >> +                                  "renesas,cpg-mstp-clocks";
> >> +                     reg =3D <0 0xe615013c 0 4>, <0 0xe6150048 0 4>;
> >> +                     clocks =3D  <&p_clk>;
> >> +                     #clock-cells =3D <1>;
> >> +                     renesas,clock-indices =3D <RCAR_GEN3_CLK_IRDA>;
> >> +                     clock-output-names =3D "irda";
> >> +             };
> >> +     };
> >> +};
> =

> Gr{oetje,eeting}s,
> =

>                         Geert
> =

> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m6=
8k.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-clk" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/4 v3][RFC] arm64: renesas: Add initial r8a7795 SoC support

[Michael Turquette]

Quoting Geert Uytterhoeven (2015-08-04 05:34:06)
> On Tue, Aug 4, 2015 at 2:22 PM, Laurent Pinchart
> <laurent.pinchart@ideasonboard.com> wrote:
> > On Monday 03 August 2015 01:53:23 Kuninori Morimoto wrote:
> >> --- /dev/null
> >> +++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
> >> @@ -0,0 +1,93 @@
> >> +/ {
> 
> >> +     clocks {
> >
> > Let's try to make it right from the start on Gen3. The CPG node should be a
> > direct child of the bus node mentioned above, and the MSTP clocks should be
> > children of the CPG node.
> 
> Agreed.
> 
> > I'm not sure where to put the non-memory-mapped clocks though, should they be
> > directly under the root node ? It would make sense for extal_clk, but how
> > about the fixed-factor clocks ? Should they be children of the CPG node too ?
> 
> I believe the current trend is to put clocks like "extal_clk" under the root
> node.
> As the fixed-factor clocks are generated by the CPG module, and we do have
> a device node for it, I'd make them children of the CPG node, too.
> 
> Any comments from the clk+dt experts?

I don't know if anyone is an expert ;-)

extal_clk should be under the root node. That is true for all
board-level clocks and clock controllers.

Within the SoC we want to model the clock controller as a node in DT,
not necessarily all of the individual clocks. So you definitely need a
"cpg" node in DT with #clock-cells > 0.

Whether or not you enumerate the individual clocks in DT is up to you. I
do not like the data-driven approach of putting the clock definition
data into DT. It makes it awkward to do things like set a flag on a
single clock later on. Simply using the clock controller phandle plus
one or more offsets is preferred over a per-clock phandle.

Stephen and I have been discussing what a formal clock-controller
binding would look like, and one item we came up with is that any
sub-nodes of the controller would not be allowed to have a #clock-cells
property.

Also, while you're thinking about the perfect clock binding, please do
consider dropping clock-output-names if you can. Specifying clock-names
alongside the clocks property inside of the consumer node is a bit more
elegant in my opinion. This is also a bit easier if you think about
expressing your clock data with C inside of your provider driver.

Regards,
Mike

> 
> Thanks!
> 
> >> +             #address-cells = <2>;
> >> +             #size-cells = <2>;
> >> +             ranges;
> >> +
> >> +             extal_clk: extal_clk {
> >> +                     compatible = "fixed-clock";
> >> +                     #clock-cells = <0>;
> >> +                     clock-frequency = <0>;
> >> +                     clock-output-names = "extal";
> >> +             };
> >> +             cpg_clocks: cpg_clocks@e6150000 {
> >> +                     compatible = "renesas,r8a7795-cpg-clocks",
> >> +                                  "renesas,rcar-gen3-cpg-clocks";
> >> +                     reg = <0 0xe6150000 0 0x1000>;
> >> +                     clocks = <&extal_clk>;
> >> +                     #clock-cells = <1>;
> >> +                     clock-output-names = "main", "pll0", "pll1","pll2",
> >> +                                          "pll3", "pll4";
> >> +             };
> >> +             p_clk: p_clk {
> >> +                     compatible = "fixed-factor-clock";
> >> +                     clocks = <&cpg_clocks RCAR_GEN3_CLK_PLL1>;
> >> +                     #clock-cells = <0>;
> >> +                     clock-div = <24>;
> >> +                     clock-mult = <1>;
> >> +                     clock-output-names = "p";
> >> +             };
> >> +             mstp3_clks: mstp3_clks@e615013c {
> >> +                     compatible = "renesas,r8a7795-mstp-clocks",
> >> +                                  "renesas,cpg-mstp-clocks";
> >> +                     reg = <0 0xe615013c 0 4>, <0 0xe6150048 0 4>;
> >> +                     clocks =  <&p_clk>;
> >> +                     #clock-cells = <1>;
> >> +                     renesas,clock-indices = <RCAR_GEN3_CLK_IRDA>;
> >> +                     clock-output-names = "irda";
> >> +             };
> >> +     };
> >> +};
> 
> 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-clk" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/4 v3][RFC] arm64: renesas: Add initial r8a7795 SoC support

[Michael Turquette]

Quoting Geert Uytterhoeven (2015-08-04 05:34:06)
> On Tue, Aug 4, 2015 at 2:22 PM, Laurent Pinchart
> <laurent.pinchart@ideasonboard.com> wrote:
> > On Monday 03 August 2015 01:53:23 Kuninori Morimoto wrote:
> >> --- /dev/null
> >> +++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
> >> @@ -0,0 +1,93 @@
> >> +/ {
> 
> >> +     clocks {
> >
> > Let's try to make it right from the start on Gen3. The CPG node should be a
> > direct child of the bus node mentioned above, and the MSTP clocks should be
> > children of the CPG node.
> 
> Agreed.
> 
> > I'm not sure where to put the non-memory-mapped clocks though, should they be
> > directly under the root node ? It would make sense for extal_clk, but how
> > about the fixed-factor clocks ? Should they be children of the CPG node too ?
> 
> I believe the current trend is to put clocks like "extal_clk" under the root
> node.
> As the fixed-factor clocks are generated by the CPG module, and we do have
> a device node for it, I'd make them children of the CPG node, too.
> 
> Any comments from the clk+dt experts?

I don't know if anyone is an expert ;-)

extal_clk should be under the root node. That is true for all
board-level clocks and clock controllers.

Within the SoC we want to model the clock controller as a node in DT,
not necessarily all of the individual clocks. So you definitely need a
"cpg" node in DT with #clock-cells > 0.

Whether or not you enumerate the individual clocks in DT is up to you. I
do not like the data-driven approach of putting the clock definition
data into DT. It makes it awkward to do things like set a flag on a
single clock later on. Simply using the clock controller phandle plus
one or more offsets is preferred over a per-clock phandle.

Stephen and I have been discussing what a formal clock-controller
binding would look like, and one item we came up with is that any
sub-nodes of the controller would not be allowed to have a #clock-cells
property.

Also, while you're thinking about the perfect clock binding, please do
consider dropping clock-output-names if you can. Specifying clock-names
alongside the clocks property inside of the consumer node is a bit more
elegant in my opinion. This is also a bit easier if you think about
expressing your clock data with C inside of your provider driver.

Regards,
Mike

> 
> Thanks!
> 
> >> +             #address-cells = <2>;
> >> +             #size-cells = <2>;
> >> +             ranges;
> >> +
> >> +             extal_clk: extal_clk {
> >> +                     compatible = "fixed-clock";
> >> +                     #clock-cells = <0>;
> >> +                     clock-frequency = <0>;
> >> +                     clock-output-names = "extal";
> >> +             };
> >> +             cpg_clocks: cpg_clocks@e6150000 {
> >> +                     compatible = "renesas,r8a7795-cpg-clocks",
> >> +                                  "renesas,rcar-gen3-cpg-clocks";
> >> +                     reg = <0 0xe6150000 0 0x1000>;
> >> +                     clocks = <&extal_clk>;
> >> +                     #clock-cells = <1>;
> >> +                     clock-output-names = "main", "pll0", "pll1","pll2",
> >> +                                          "pll3", "pll4";
> >> +             };
> >> +             p_clk: p_clk {
> >> +                     compatible = "fixed-factor-clock";
> >> +                     clocks = <&cpg_clocks RCAR_GEN3_CLK_PLL1>;
> >> +                     #clock-cells = <0>;
> >> +                     clock-div = <24>;
> >> +                     clock-mult = <1>;
> >> +                     clock-output-names = "p";
> >> +             };
> >> +             mstp3_clks: mstp3_clks@e615013c {
> >> +                     compatible = "renesas,r8a7795-mstp-clocks",
> >> +                                  "renesas,cpg-mstp-clocks";
> >> +                     reg = <0 0xe615013c 0 4>, <0 0xe6150048 0 4>;
> >> +                     clocks =  <&p_clk>;
> >> +                     #clock-cells = <1>;
> >> +                     renesas,clock-indices = <RCAR_GEN3_CLK_IRDA>;
> >> +                     clock-output-names = "irda";
> >> +             };
> >> +     };
> >> +};
> 
> 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-clk" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/4 v3][RFC] arm64: renesas: Add initial r8a7795 SoC support

[Geert Uytterhoeven]

Hi Mike,

On Tue, Aug 18, 2015 at 2:20 AM, Michael Turquette
<mturquette@baylibre.com> wrote:
> Quoting Geert Uytterhoeven (2015-08-04 05:34:06)
>> On Tue, Aug 4, 2015 at 2:22 PM, Laurent Pinchart
>> <laurent.pinchart@ideasonboard.com> wrote:
>> > On Monday 03 August 2015 01:53:23 Kuninori Morimoto wrote:
>> >> --- /dev/null
>> >> +++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
>> >> @@ -0,0 +1,93 @@
>> >> +/ {
>>
>> >> +     clocks {
>> >
>> > Let's try to make it right from the start on Gen3. The CPG node should be a
>> > direct child of the bus node mentioned above, and the MSTP clocks should be
>> > children of the CPG node.
>>
>> Agreed.
>>
>> > I'm not sure where to put the non-memory-mapped clocks though, should they be
>> > directly under the root node ? It would make sense for extal_clk, but how
>> > about the fixed-factor clocks ? Should they be children of the CPG node too ?
>>
>> I believe the current trend is to put clocks like "extal_clk" under the root
>> node.
>> As the fixed-factor clocks are generated by the CPG module, and we do have
>> a device node for it, I'd make them children of the CPG node, too.
>>
>> Any comments from the clk+dt experts?
>
> I don't know if anyone is an expert ;-)
>
> extal_clk should be under the root node. That is true for all
> board-level clocks and clock controllers.

OK.

> Within the SoC we want to model the clock controller as a node in DT,
> not necessarily all of the individual clocks. So you definitely need a
> "cpg" node in DT with #clock-cells > 0.

OK.

> Whether or not you enumerate the individual clocks in DT is up to you. I
> do not like the data-driven approach of putting the clock definition
> data into DT. It makes it awkward to do things like set a flag on a
> single clock later on. Simply using the clock controller phandle plus
> one or more offsets is preferred over a per-clock phandle.
>
> Stephen and I have been discussing what a formal clock-controller
> binding would look like, and one item we came up with is that any
> sub-nodes of the controller would not be allowed to have a #clock-cells
> property.

Does that mean #clock-cells is inherited from the parent (cpg) node, or does
that preclude having any "fixed-factor-clock" or "renesas,cpg-div6-clock"
(both use #clock-cells = <0>), or "renesas,cpg-mstp-clocks" subnodes?

> Also, while you're thinking about the perfect clock binding, please do
> consider dropping clock-output-names if you can. Specifying clock-names
> alongside the clocks property inside of the consumer node is a bit more
> elegant in my opinion. This is also a bit easier if you think about
> expressing your clock data with C inside of your provider driver.

Makes sense. I don't think anything relies on the "clock-output-names"
... currently.

I was going to use it for identifying the GIC clock, though:

--- a/drivers/clk/shmobile/clk-mstp.c
+++ b/drivers/clk/shmobile/clk-mstp.c
@@ -144,6 +144,9 @@ cpg_mstp_clock_register(const char *name, const char *paren
        init.name = name;
        init.ops = &cpg_mstp_clock_ops;
        init.flags = CLK_IS_BASIC | CLK_SET_RATE_PARENT;
+       /* INTC-SYS is the module clock of the GIC, and must not be disabled */
+       if (!strcmp(name, "intc-sys"))
+               init.flags |= CLK_ENABLE_HAND_OFF;
        init.parent_names = &parent_name;
        init.num_parents = 1;

(FTR, on R-Car Gen3 the clock is called "intc-ap", not "intc-sys", doh...).

This does put some reliance on (undocumented) naming in DT, though, so not
having the "clock-output-names" would solve that.

However, while "intc-sys"/"intc-ap" (if existent) is always MSTP408, MSTP408
is used for other modules on R-Car Gen1 and most older SoCs. so it would
complicate the driver code.

>> >> +             #address-cells = <2>;
>> >> +             #size-cells = <2>;
>> >> +             ranges;
>> >> +
>> >> +             extal_clk: extal_clk {
>> >> +                     compatible = "fixed-clock";
>> >> +                     #clock-cells = <0>;
>> >> +                     clock-frequency = <0>;
>> >> +                     clock-output-names = "extal";
>> >> +             };
>> >> +             cpg_clocks: cpg_clocks@e6150000 {
>> >> +                     compatible = "renesas,r8a7795-cpg-clocks",
>> >> +                                  "renesas,rcar-gen3-cpg-clocks";
>> >> +                     reg = <0 0xe6150000 0 0x1000>;
>> >> +                     clocks = <&extal_clk>;
>> >> +                     #clock-cells = <1>;
>> >> +                     clock-output-names = "main", "pll0", "pll1","pll2",
>> >> +                                          "pll3", "pll4";
>> >> +             };
>> >> +             p_clk: p_clk {
>> >> +                     compatible = "fixed-factor-clock";
>> >> +                     clocks = <&cpg_clocks RCAR_GEN3_CLK_PLL1>;
>> >> +                     #clock-cells = <0>;
>> >> +                     clock-div = <24>;
>> >> +                     clock-mult = <1>;
>> >> +                     clock-output-names = "p";
>> >> +             };
>> >> +             mstp3_clks: mstp3_clks@e615013c {
>> >> +                     compatible = "renesas,r8a7795-mstp-clocks",
>> >> +                                  "renesas,cpg-mstp-clocks";
>> >> +                     reg = <0 0xe615013c 0 4>, <0 0xe6150048 0 4>;
>> >> +                     clocks =  <&p_clk>;
>> >> +                     #clock-cells = <1>;
>> >> +                     renesas,clock-indices = <RCAR_GEN3_CLK_IRDA>;
>> >> +                     clock-output-names = "irda";
>> >> +             };
>> >> +     };
>> >> +};

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 3/4 v3][RFC] arm64: renesas: Add initial r8a7795 SoC support

[Geert Uytterhoeven]

Hi Mike,

On Tue, Aug 18, 2015 at 2:20 AM, Michael Turquette
<mturquette@baylibre.com> wrote:
> Quoting Geert Uytterhoeven (2015-08-04 05:34:06)
>> On Tue, Aug 4, 2015 at 2:22 PM, Laurent Pinchart
>> <laurent.pinchart@ideasonboard.com> wrote:
>> > On Monday 03 August 2015 01:53:23 Kuninori Morimoto wrote:
>> >> --- /dev/null
>> >> +++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
>> >> @@ -0,0 +1,93 @@
>> >> +/ {
>>
>> >> +     clocks {
>> >
>> > Let's try to make it right from the start on Gen3. The CPG node should be a
>> > direct child of the bus node mentioned above, and the MSTP clocks should be
>> > children of the CPG node.
>>
>> Agreed.
>>
>> > I'm not sure where to put the non-memory-mapped clocks though, should they be
>> > directly under the root node ? It would make sense for extal_clk, but how
>> > about the fixed-factor clocks ? Should they be children of the CPG node too ?
>>
>> I believe the current trend is to put clocks like "extal_clk" under the root
>> node.
>> As the fixed-factor clocks are generated by the CPG module, and we do have
>> a device node for it, I'd make them children of the CPG node, too.
>>
>> Any comments from the clk+dt experts?
>
> I don't know if anyone is an expert ;-)
>
> extal_clk should be under the root node. That is true for all
> board-level clocks and clock controllers.

OK.

> Within the SoC we want to model the clock controller as a node in DT,
> not necessarily all of the individual clocks. So you definitely need a
> "cpg" node in DT with #clock-cells > 0.

OK.

> Whether or not you enumerate the individual clocks in DT is up to you. I
> do not like the data-driven approach of putting the clock definition
> data into DT. It makes it awkward to do things like set a flag on a
> single clock later on. Simply using the clock controller phandle plus
> one or more offsets is preferred over a per-clock phandle.
>
> Stephen and I have been discussing what a formal clock-controller
> binding would look like, and one item we came up with is that any
> sub-nodes of the controller would not be allowed to have a #clock-cells
> property.

Does that mean #clock-cells is inherited from the parent (cpg) node, or does
that preclude having any "fixed-factor-clock" or "renesas,cpg-div6-clock"
(both use #clock-cells = <0>), or "renesas,cpg-mstp-clocks" subnodes?

> Also, while you're thinking about the perfect clock binding, please do
> consider dropping clock-output-names if you can. Specifying clock-names
> alongside the clocks property inside of the consumer node is a bit more
> elegant in my opinion. This is also a bit easier if you think about
> expressing your clock data with C inside of your provider driver.

Makes sense. I don't think anything relies on the "clock-output-names"
... currently.

I was going to use it for identifying the GIC clock, though:

--- a/drivers/clk/shmobile/clk-mstp.c
+++ b/drivers/clk/shmobile/clk-mstp.c
@@ -144,6 +144,9 @@ cpg_mstp_clock_register(const char *name, const char *paren
        init.name = name;
        init.ops = &cpg_mstp_clock_ops;
        init.flags = CLK_IS_BASIC | CLK_SET_RATE_PARENT;
+       /* INTC-SYS is the module clock of the GIC, and must not be disabled */
+       if (!strcmp(name, "intc-sys"))
+               init.flags |= CLK_ENABLE_HAND_OFF;
        init.parent_names = &parent_name;
        init.num_parents = 1;

(FTR, on R-Car Gen3 the clock is called "intc-ap", not "intc-sys", doh...).

This does put some reliance on (undocumented) naming in DT, though, so not
having the "clock-output-names" would solve that.

However, while "intc-sys"/"intc-ap" (if existent) is always MSTP408, MSTP408
is used for other modules on R-Car Gen1 and most older SoCs. so it would
complicate the driver code.

>> >> +             #address-cells = <2>;
>> >> +             #size-cells = <2>;
>> >> +             ranges;
>> >> +
>> >> +             extal_clk: extal_clk {
>> >> +                     compatible = "fixed-clock";
>> >> +                     #clock-cells = <0>;
>> >> +                     clock-frequency = <0>;
>> >> +                     clock-output-names = "extal";
>> >> +             };
>> >> +             cpg_clocks: cpg_clocks@e6150000 {
>> >> +                     compatible = "renesas,r8a7795-cpg-clocks",
>> >> +                                  "renesas,rcar-gen3-cpg-clocks";
>> >> +                     reg = <0 0xe6150000 0 0x1000>;
>> >> +                     clocks = <&extal_clk>;
>> >> +                     #clock-cells = <1>;
>> >> +                     clock-output-names = "main", "pll0", "pll1","pll2",
>> >> +                                          "pll3", "pll4";
>> >> +             };
>> >> +             p_clk: p_clk {
>> >> +                     compatible = "fixed-factor-clock";
>> >> +                     clocks = <&cpg_clocks RCAR_GEN3_CLK_PLL1>;
>> >> +                     #clock-cells = <0>;
>> >> +                     clock-div = <24>;
>> >> +                     clock-mult = <1>;
>> >> +                     clock-output-names = "p";
>> >> +             };
>> >> +             mstp3_clks: mstp3_clks@e615013c {
>> >> +                     compatible = "renesas,r8a7795-mstp-clocks",
>> >> +                                  "renesas,cpg-mstp-clocks";
>> >> +                     reg = <0 0xe615013c 0 4>, <0 0xe6150048 0 4>;
>> >> +                     clocks =  <&p_clk>;
>> >> +                     #clock-cells = <1>;
>> >> +                     renesas,clock-indices = <RCAR_GEN3_CLK_IRDA>;
>> >> +                     clock-output-names = "irda";
>> >> +             };
>> >> +     };
>> >> +};

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 3/4 v3][RFC] arm64: renesas: Add initial r8a7795 SoC support

[Laurent Pinchart]

Hello,

On Wednesday 19 August 2015 09:49:28 Geert Uytterhoeven wrote:
> On Tue, Aug 18, 2015 at 2:20 AM, Michael Turquette wrote:
> > Quoting Geert Uytterhoeven (2015-08-04 05:34:06)
> >> On Tue, Aug 4, 2015 at 2:22 PM, Laurent Pinchart wrote:
> >>> On Monday 03 August 2015 01:53:23 Kuninori Morimoto wrote:
> >>>> --- /dev/null
> >>>> +++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
> >>>> @@ -0,0 +1,93 @@
> >>>> +/ {
> >>>> +     clocks {
> >>> 
> >>> Let's try to make it right from the start on Gen3. The CPG node should
> >>> be a direct child of the bus node mentioned above, and the MSTP clocks
> >>> should be children of the CPG node.
> >> 
> >> Agreed.
> >> 
> >>> I'm not sure where to put the non-memory-mapped clocks though, should
> >>> they be directly under the root node ? It would make sense for
> >>> extal_clk, but how about the fixed-factor clocks ? Should they be
> >>> children of the CPG node too ?
> >>
> >> I believe the current trend is to put clocks like "extal_clk" under the
> >> root node. As the fixed-factor clocks are generated by the CPG module,
> >> and we do have a device node for it, I'd make them children of the CPG
> >> node, too.
> >> 
> >> Any comments from the clk+dt experts?
> > 
> > I don't know if anyone is an expert ;-)
> > 
> > extal_clk should be under the root node. That is true for all
> > board-level clocks and clock controllers.
> 
> OK.

I agree too.

> > Within the SoC we want to model the clock controller as a node in DT,
> > not necessarily all of the individual clocks. So you definitely need a
> > "cpg" node in DT with #clock-cells > 0.
> 
> OK.

Ditto.

> > Whether or not you enumerate the individual clocks in DT is up to you. I
> > do not like the data-driven approach of putting the clock definition
> > data into DT. It makes it awkward to do things like set a flag on a
> > single clock later on. Simply using the clock controller phandle plus
> > one or more offsets is preferred over a per-clock phandle.
> > 
> > Stephen and I have been discussing what a formal clock-controller
> > binding would look like, and one item we came up with is that any
> > sub-nodes of the controller would not be allowed to have a #clock-cells
> > property.
> 
> Does that mean #clock-cells is inherited from the parent (cpg) node, or does
> that preclude having any "fixed-factor-clock" or "renesas,cpg-div6-clock"
> (both use #clock-cells = <0>), or "renesas,cpg-mstp-clocks" subnodes?

We were thinking about moving the fixed factor clocks and the gate clocks as 
subnodes as the CPG DT node, as those clocks are provided by the CPG. This 
would require setting #clock-cells to 1 in the CPG node and to 0 in some of 
the subnodes. If that's not allowed, how should the fixed factor clocks and 
gate clocks be declared ?

> > Also, while you're thinking about the perfect clock binding, please do
> > consider dropping clock-output-names if you can. Specifying clock-names
> > alongside the clocks property inside of the consumer node is a bit more
> > elegant in my opinion. This is also a bit easier if you think about
> > expressing your clock data with C inside of your provider driver.
> 
> Makes sense. I don't think anything relies on the "clock-output-names"
> ... currently.
> 
> I was going to use it for identifying the GIC clock, though:
> 
> --- a/drivers/clk/shmobile/clk-mstp.c
> +++ b/drivers/clk/shmobile/clk-mstp.c
> @@ -144,6 +144,9 @@ cpg_mstp_clock_register(const char *name, const char
> *paren init.name = name;
>         init.ops = &cpg_mstp_clock_ops;
>         init.flags = CLK_IS_BASIC | CLK_SET_RATE_PARENT;
> +       /* INTC-SYS is the module clock of the GIC, and must not be disabled
> */
> +       if (!strcmp(name, "intc-sys"))
> +               init.flags |= CLK_ENABLE_HAND_OFF;
>         init.parent_names = &parent_name;
>         init.num_parents = 1;
> 
> (FTR, on R-Car Gen3 the clock is called "intc-ap", not "intc-sys", doh...).
> 
> This does put some reliance on (undocumented) naming in DT, though, so not
> having the "clock-output-names" would solve that.

Using the clock name is indeed very messy, let's avoid that.

> However, while "intc-sys"/"intc-ap" (if existent) is always MSTP408, MSTP408
> is used for other modules on R-Car Gen1 and most older SoCs. so it would
> complicate the driver code.

How about setting the flag based on information provided in DT ?

> >>>> +             #address-cells = <2>;
> >>>> +             #size-cells = <2>;
> >>>> +             ranges;
> >>>> +
> >>>> +             extal_clk: extal_clk {
> >>>> +                     compatible = "fixed-clock";
> >>>> +                     #clock-cells = <0>;
> >>>> +                     clock-frequency = <0>;
> >>>> +                     clock-output-names = "extal";
> >>>> +             };
> >>>> +             cpg_clocks: cpg_clocks@e6150000 {
> >>>> +                     compatible = "renesas,r8a7795-cpg-clocks",
> >>>> +                                  "renesas,rcar-gen3-cpg-clocks";
> >>>> +                     reg = <0 0xe6150000 0 0x1000>;
> >>>> +                     clocks = <&extal_clk>;
> >>>> +                     #clock-cells = <1>;
> >>>> +                     clock-output-names = "main", "pll0",
> >>>> "pll1","pll2",
> >>>> +                                          "pll3", "pll4";
> >>>> +             };
> >>>> +             p_clk: p_clk {
> >>>> +                     compatible = "fixed-factor-clock";
> >>>> +                     clocks = <&cpg_clocks RCAR_GEN3_CLK_PLL1>;
> >>>> +                     #clock-cells = <0>;
> >>>> +                     clock-div = <24>;
> >>>> +                     clock-mult = <1>;
> >>>> +                     clock-output-names = "p";
> >>>> +             };
> >>>> +             mstp3_clks: mstp3_clks@e615013c {
> >>>> +                     compatible = "renesas,r8a7795-mstp-clocks",
> >>>> +                                  "renesas,cpg-mstp-clocks";
> >>>> +                     reg = <0 0xe615013c 0 4>, <0 0xe6150048 0 4>;
> >>>> +                     clocks =  <&p_clk>;
> >>>> +                     #clock-cells = <1>;
> >>>> +                     renesas,clock-indices = <RCAR_GEN3_CLK_IRDA>;
> >>>> +                     clock-output-names = "irda";
> >>>> +             };
> >>>> +     };
> >>>> +};

-- 
Regards,

Laurent Pinchart

Re: [PATCH 3/4 v3][RFC] arm64: renesas: Add initial r8a7795 SoC support

[Laurent Pinchart]

Hello,

On Wednesday 19 August 2015 09:49:28 Geert Uytterhoeven wrote:
> On Tue, Aug 18, 2015 at 2:20 AM, Michael Turquette wrote:
> > Quoting Geert Uytterhoeven (2015-08-04 05:34:06)
> >> On Tue, Aug 4, 2015 at 2:22 PM, Laurent Pinchart wrote:
> >>> On Monday 03 August 2015 01:53:23 Kuninori Morimoto wrote:
> >>>> --- /dev/null
> >>>> +++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
> >>>> @@ -0,0 +1,93 @@
> >>>> +/ {
> >>>> +     clocks {
> >>> 
> >>> Let's try to make it right from the start on Gen3. The CPG node should
> >>> be a direct child of the bus node mentioned above, and the MSTP clocks
> >>> should be children of the CPG node.
> >> 
> >> Agreed.
> >> 
> >>> I'm not sure where to put the non-memory-mapped clocks though, should
> >>> they be directly under the root node ? It would make sense for
> >>> extal_clk, but how about the fixed-factor clocks ? Should they be
> >>> children of the CPG node too ?
> >>
> >> I believe the current trend is to put clocks like "extal_clk" under the
> >> root node. As the fixed-factor clocks are generated by the CPG module,
> >> and we do have a device node for it, I'd make them children of the CPG
> >> node, too.
> >> 
> >> Any comments from the clk+dt experts?
> > 
> > I don't know if anyone is an expert ;-)
> > 
> > extal_clk should be under the root node. That is true for all
> > board-level clocks and clock controllers.
> 
> OK.

I agree too.

> > Within the SoC we want to model the clock controller as a node in DT,
> > not necessarily all of the individual clocks. So you definitely need a
> > "cpg" node in DT with #clock-cells > 0.
> 
> OK.

Ditto.

> > Whether or not you enumerate the individual clocks in DT is up to you. I
> > do not like the data-driven approach of putting the clock definition
> > data into DT. It makes it awkward to do things like set a flag on a
> > single clock later on. Simply using the clock controller phandle plus
> > one or more offsets is preferred over a per-clock phandle.
> > 
> > Stephen and I have been discussing what a formal clock-controller
> > binding would look like, and one item we came up with is that any
> > sub-nodes of the controller would not be allowed to have a #clock-cells
> > property.
> 
> Does that mean #clock-cells is inherited from the parent (cpg) node, or does
> that preclude having any "fixed-factor-clock" or "renesas,cpg-div6-clock"
> (both use #clock-cells = <0>), or "renesas,cpg-mstp-clocks" subnodes?

We were thinking about moving the fixed factor clocks and the gate clocks as 
subnodes as the CPG DT node, as those clocks are provided by the CPG. This 
would require setting #clock-cells to 1 in the CPG node and to 0 in some of 
the subnodes. If that's not allowed, how should the fixed factor clocks and 
gate clocks be declared ?

> > Also, while you're thinking about the perfect clock binding, please do
> > consider dropping clock-output-names if you can. Specifying clock-names
> > alongside the clocks property inside of the consumer node is a bit more
> > elegant in my opinion. This is also a bit easier if you think about
> > expressing your clock data with C inside of your provider driver.
> 
> Makes sense. I don't think anything relies on the "clock-output-names"
> ... currently.
> 
> I was going to use it for identifying the GIC clock, though:
> 
> --- a/drivers/clk/shmobile/clk-mstp.c
> +++ b/drivers/clk/shmobile/clk-mstp.c
> @@ -144,6 +144,9 @@ cpg_mstp_clock_register(const char *name, const char
> *paren init.name = name;
>         init.ops = &cpg_mstp_clock_ops;
>         init.flags = CLK_IS_BASIC | CLK_SET_RATE_PARENT;
> +       /* INTC-SYS is the module clock of the GIC, and must not be disabled
> */
> +       if (!strcmp(name, "intc-sys"))
> +               init.flags |= CLK_ENABLE_HAND_OFF;
>         init.parent_names = &parent_name;
>         init.num_parents = 1;
> 
> (FTR, on R-Car Gen3 the clock is called "intc-ap", not "intc-sys", doh...).
> 
> This does put some reliance on (undocumented) naming in DT, though, so not
> having the "clock-output-names" would solve that.

Using the clock name is indeed very messy, let's avoid that.

> However, while "intc-sys"/"intc-ap" (if existent) is always MSTP408, MSTP408
> is used for other modules on R-Car Gen1 and most older SoCs. so it would
> complicate the driver code.

How about setting the flag based on information provided in DT ?

> >>>> +             #address-cells = <2>;
> >>>> +             #size-cells = <2>;
> >>>> +             ranges;
> >>>> +
> >>>> +             extal_clk: extal_clk {
> >>>> +                     compatible = "fixed-clock";
> >>>> +                     #clock-cells = <0>;
> >>>> +                     clock-frequency = <0>;
> >>>> +                     clock-output-names = "extal";
> >>>> +             };
> >>>> +             cpg_clocks: cpg_clocks@e6150000 {
> >>>> +                     compatible = "renesas,r8a7795-cpg-clocks",
> >>>> +                                  "renesas,rcar-gen3-cpg-clocks";
> >>>> +                     reg = <0 0xe6150000 0 0x1000>;
> >>>> +                     clocks = <&extal_clk>;
> >>>> +                     #clock-cells = <1>;
> >>>> +                     clock-output-names = "main", "pll0",
> >>>> "pll1","pll2",
> >>>> +                                          "pll3", "pll4";
> >>>> +             };
> >>>> +             p_clk: p_clk {
> >>>> +                     compatible = "fixed-factor-clock";
> >>>> +                     clocks = <&cpg_clocks RCAR_GEN3_CLK_PLL1>;
> >>>> +                     #clock-cells = <0>;
> >>>> +                     clock-div = <24>;
> >>>> +                     clock-mult = <1>;
> >>>> +                     clock-output-names = "p";
> >>>> +             };
> >>>> +             mstp3_clks: mstp3_clks@e615013c {
> >>>> +                     compatible = "renesas,r8a7795-mstp-clocks",
> >>>> +                                  "renesas,cpg-mstp-clocks";
> >>>> +                     reg = <0 0xe615013c 0 4>, <0 0xe6150048 0 4>;
> >>>> +                     clocks =  <&p_clk>;
> >>>> +                     #clock-cells = <1>;
> >>>> +                     renesas,clock-indices = <RCAR_GEN3_CLK_IRDA>;
> >>>> +                     clock-output-names = "irda";
> >>>> +             };
> >>>> +     };
> >>>> +};

-- 
Regards,

Laurent Pinchart

Re: [PATCH 3/4 v3][RFC] arm64: renesas: Add initial r8a7795 SoC support

[Geert Uytterhoeven]

Hi Laurent,

On Wed, Aug 19, 2015 at 11:29 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>> > Also, while you're thinking about the perfect clock binding, please do
>> > consider dropping clock-output-names if you can. Specifying clock-names
>> > alongside the clocks property inside of the consumer node is a bit more
>> > elegant in my opinion. This is also a bit easier if you think about
>> > expressing your clock data with C inside of your provider driver.
>>
>> Makes sense. I don't think anything relies on the "clock-output-names"
>> ... currently.
>>
>> I was going to use it for identifying the GIC clock, though:
>>
>> --- a/drivers/clk/shmobile/clk-mstp.c
>> +++ b/drivers/clk/shmobile/clk-mstp.c
>> @@ -144,6 +144,9 @@ cpg_mstp_clock_register(const char *name, const char
>> *paren init.name = name;
>>         init.ops = &cpg_mstp_clock_ops;
>>         init.flags = CLK_IS_BASIC | CLK_SET_RATE_PARENT;
>> +       /* INTC-SYS is the module clock of the GIC, and must not be disabled
>> */
>> +       if (!strcmp(name, "intc-sys"))
>> +               init.flags |= CLK_ENABLE_HAND_OFF;
>>         init.parent_names = &parent_name;
>>         init.num_parents = 1;
>>
>> (FTR, on R-Car Gen3 the clock is called "intc-ap", not "intc-sys", doh...).
>>
>> This does put some reliance on (undocumented) naming in DT, though, so not
>> having the "clock-output-names" would solve that.
>
> Using the clock name is indeed very messy, let's avoid that.
>
>> However, while "intc-sys"/"intc-ap" (if existent) is always MSTP408, MSTP408
>> is used for other modules on R-Car Gen1 and most older SoCs. so it would
>> complicate the driver code.
>
> How about setting the flag based on information provided in DT ?

Like scanning DT for the GIC, and looking for its clock?

I don't want to add an explicit flag in DT, as it's not a hardware property,
but a deficiency of the current GIC driver/subsystem.

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 3/4 v3][RFC] arm64: renesas: Add initial r8a7795 SoC support

[Geert Uytterhoeven]

Hi Laurent,

On Wed, Aug 19, 2015 at 11:29 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>> > Also, while you're thinking about the perfect clock binding, please do
>> > consider dropping clock-output-names if you can. Specifying clock-names
>> > alongside the clocks property inside of the consumer node is a bit more
>> > elegant in my opinion. This is also a bit easier if you think about
>> > expressing your clock data with C inside of your provider driver.
>>
>> Makes sense. I don't think anything relies on the "clock-output-names"
>> ... currently.
>>
>> I was going to use it for identifying the GIC clock, though:
>>
>> --- a/drivers/clk/shmobile/clk-mstp.c
>> +++ b/drivers/clk/shmobile/clk-mstp.c
>> @@ -144,6 +144,9 @@ cpg_mstp_clock_register(const char *name, const char
>> *paren init.name = name;
>>         init.ops = &cpg_mstp_clock_ops;
>>         init.flags = CLK_IS_BASIC | CLK_SET_RATE_PARENT;
>> +       /* INTC-SYS is the module clock of the GIC, and must not be disabled
>> */
>> +       if (!strcmp(name, "intc-sys"))
>> +               init.flags |= CLK_ENABLE_HAND_OFF;
>>         init.parent_names = &parent_name;
>>         init.num_parents = 1;
>>
>> (FTR, on R-Car Gen3 the clock is called "intc-ap", not "intc-sys", doh...).
>>
>> This does put some reliance on (undocumented) naming in DT, though, so not
>> having the "clock-output-names" would solve that.
>
> Using the clock name is indeed very messy, let's avoid that.
>
>> However, while "intc-sys"/"intc-ap" (if existent) is always MSTP408, MSTP408
>> is used for other modules on R-Car Gen1 and most older SoCs. so it would
>> complicate the driver code.
>
> How about setting the flag based on information provided in DT ?

Like scanning DT for the GIC, and looking for its clock?

I don't want to add an explicit flag in DT, as it's not a hardware property,
but a deficiency of the current GIC driver/subsystem.

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 3/4 v3][RFC] arm64: renesas: Add initial r8a7795 SoC support

[Laurent Pinchart]

Hi Geert,

On Thursday 20 August 2015 09:43:34 Geert Uytterhoeven wrote:
> On Wed, Aug 19, 2015 at 11:29 PM, Laurent Pinchart wrote:
> >> > Also, while you're thinking about the perfect clock binding, please do
> >> > consider dropping clock-output-names if you can. Specifying clock-names
> >> > alongside the clocks property inside of the consumer node is a bit more
> >> > elegant in my opinion. This is also a bit easier if you think about
> >> > expressing your clock data with C inside of your provider driver.
> >> 
> >> Makes sense. I don't think anything relies on the "clock-output-names"
> >> ... currently.
> >> 
> >> I was going to use it for identifying the GIC clock, though:
> >> 
> >> --- a/drivers/clk/shmobile/clk-mstp.c
> >> +++ b/drivers/clk/shmobile/clk-mstp.c
> >> @@ -144,6 +144,9 @@ cpg_mstp_clock_register(const char *name, const char
> >> *paren init.name = name;
> >> 
> >>         init.ops = &cpg_mstp_clock_ops;
> >>         init.flags = CLK_IS_BASIC | CLK_SET_RATE_PARENT;
> >> 
> >> +       /* INTC-SYS is the module clock of the GIC, and must not be
> >> disabled */
> >> +       if (!strcmp(name, "intc-sys"))
> >> +               init.flags |= CLK_ENABLE_HAND_OFF;
> >> 
> >>         init.parent_names = &parent_name;
> >>         init.num_parents = 1;
> >> 
> >> (FTR, on R-Car Gen3 the clock is called "intc-ap", not "intc-sys",
> >> doh...).
> >> 
> >> This does put some reliance on (undocumented) naming in DT, though, so
> >> not
> >> having the "clock-output-names" would solve that.
> > 
> > Using the clock name is indeed very messy, let's avoid that.
> > 
> >> However, while "intc-sys"/"intc-ap" (if existent) is always MSTP408,
> >> MSTP408 is used for other modules on R-Car Gen1 and most older SoCs. so
> >> it would complicate the driver code.
> > 
> > How about setting the flag based on information provided in DT ?
> 
> Like scanning DT for the GIC, and looking for its clock?
> 
> I don't want to add an explicit flag in DT, as it's not a hardware property,
> but a deficiency of the current GIC driver/subsystem.

What are the changes we can fix the GIC driver in a reasonable time frame ?

-- 
Regards,

Laurent Pinchart

Re: [PATCH 3/4 v3][RFC] arm64: renesas: Add initial r8a7795 SoC support

[Laurent Pinchart]

Hi Geert,

On Thursday 20 August 2015 09:43:34 Geert Uytterhoeven wrote:
> On Wed, Aug 19, 2015 at 11:29 PM, Laurent Pinchart wrote:
> >> > Also, while you're thinking about the perfect clock binding, please do
> >> > consider dropping clock-output-names if you can. Specifying clock-names
> >> > alongside the clocks property inside of the consumer node is a bit more
> >> > elegant in my opinion. This is also a bit easier if you think about
> >> > expressing your clock data with C inside of your provider driver.
> >> 
> >> Makes sense. I don't think anything relies on the "clock-output-names"
> >> ... currently.
> >> 
> >> I was going to use it for identifying the GIC clock, though:
> >> 
> >> --- a/drivers/clk/shmobile/clk-mstp.c
> >> +++ b/drivers/clk/shmobile/clk-mstp.c
> >> @@ -144,6 +144,9 @@ cpg_mstp_clock_register(const char *name, const char
> >> *paren init.name = name;
> >> 
> >>         init.ops = &cpg_mstp_clock_ops;
> >>         init.flags = CLK_IS_BASIC | CLK_SET_RATE_PARENT;
> >> 
> >> +       /* INTC-SYS is the module clock of the GIC, and must not be
> >> disabled */
> >> +       if (!strcmp(name, "intc-sys"))
> >> +               init.flags |= CLK_ENABLE_HAND_OFF;
> >> 
> >>         init.parent_names = &parent_name;
> >>         init.num_parents = 1;
> >> 
> >> (FTR, on R-Car Gen3 the clock is called "intc-ap", not "intc-sys",
> >> doh...).
> >> 
> >> This does put some reliance on (undocumented) naming in DT, though, so
> >> not
> >> having the "clock-output-names" would solve that.
> > 
> > Using the clock name is indeed very messy, let's avoid that.
> > 
> >> However, while "intc-sys"/"intc-ap" (if existent) is always MSTP408,
> >> MSTP408 is used for other modules on R-Car Gen1 and most older SoCs. so
> >> it would complicate the driver code.
> > 
> > How about setting the flag based on information provided in DT ?
> 
> Like scanning DT for the GIC, and looking for its clock?
> 
> I don't want to add an explicit flag in DT, as it's not a hardware property,
> but a deficiency of the current GIC driver/subsystem.

What are the changes we can fix the GIC driver in a reasonable time frame ?

-- 
Regards,

Laurent Pinchart

Re: [PATCH 3/4 v3][RFC] arm64: renesas: Add initial r8a7795 SoC support

[Geert Uytterhoeven]

Hi Laurent,

On Thu, Aug 20, 2015 at 9:48 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Thursday 20 August 2015 09:43:34 Geert Uytterhoeven wrote:
>> On Wed, Aug 19, 2015 at 11:29 PM, Laurent Pinchart wrote:
>> >> > Also, while you're thinking about the perfect clock binding, please do
>> >> > consider dropping clock-output-names if you can. Specifying clock-names
>> >> > alongside the clocks property inside of the consumer node is a bit more
>> >> > elegant in my opinion. This is also a bit easier if you think about
>> >> > expressing your clock data with C inside of your provider driver.
>> >>
>> >> Makes sense. I don't think anything relies on the "clock-output-names"
>> >> ... currently.
>> >>
>> >> I was going to use it for identifying the GIC clock, though:
>> >>
>> >> --- a/drivers/clk/shmobile/clk-mstp.c
>> >> +++ b/drivers/clk/shmobile/clk-mstp.c
>> >> @@ -144,6 +144,9 @@ cpg_mstp_clock_register(const char *name, const char
>> >> *paren init.name = name;
>> >>
>> >>         init.ops = &cpg_mstp_clock_ops;
>> >>         init.flags = CLK_IS_BASIC | CLK_SET_RATE_PARENT;
>> >>
>> >> +       /* INTC-SYS is the module clock of the GIC, and must not be
>> >> disabled */
>> >> +       if (!strcmp(name, "intc-sys"))
>> >> +               init.flags |= CLK_ENABLE_HAND_OFF;
>> >>
>> >>         init.parent_names = &parent_name;
>> >>         init.num_parents = 1;
>> >>
>> >> (FTR, on R-Car Gen3 the clock is called "intc-ap", not "intc-sys",
>> >> doh...).
>> >>
>> >> This does put some reliance on (undocumented) naming in DT, though, so
>> >> not
>> >> having the "clock-output-names" would solve that.
>> >
>> > Using the clock name is indeed very messy, let's avoid that.
>> >
>> >> However, while "intc-sys"/"intc-ap" (if existent) is always MSTP408,
>> >> MSTP408 is used for other modules on R-Car Gen1 and most older SoCs. so
>> >> it would complicate the driver code.
>> >
>> > How about setting the flag based on information provided in DT ?
>>
>> Like scanning DT for the GIC, and looking for its clock?
>>
>> I don't want to add an explicit flag in DT, as it's not a hardware property,
>> but a deficiency of the current GIC driver/subsystem.
>
> What are the changes we can fix the GIC driver in a reasonable time frame ?

Very low.

The GIC driver uses IRQCHIP_DECLARE(), hence it's initialized from
of_irq_init() <- irqchip_init() <- init_IRQ() <- start_kernel().

The MSTP clock driver uses CLK_OF_DECLARE(), hence it's initialized from
of_clk_init() <- time_init() <- start_kernel(), but 7 calls later.

IRQCHIP_DECLARE() doesn't support deferred probing.

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 Torvald

Re: [PATCH 3/4 v3][RFC] arm64: renesas: Add initial r8a7795 SoC support

[Geert Uytterhoeven]

Hi Laurent,

On Thu, Aug 20, 2015 at 9:48 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Thursday 20 August 2015 09:43:34 Geert Uytterhoeven wrote:
>> On Wed, Aug 19, 2015 at 11:29 PM, Laurent Pinchart wrote:
>> >> > Also, while you're thinking about the perfect clock binding, please do
>> >> > consider dropping clock-output-names if you can. Specifying clock-names
>> >> > alongside the clocks property inside of the consumer node is a bit more
>> >> > elegant in my opinion. This is also a bit easier if you think about
>> >> > expressing your clock data with C inside of your provider driver.
>> >>
>> >> Makes sense. I don't think anything relies on the "clock-output-names"
>> >> ... currently.
>> >>
>> >> I was going to use it for identifying the GIC clock, though:
>> >>
>> >> --- a/drivers/clk/shmobile/clk-mstp.c
>> >> +++ b/drivers/clk/shmobile/clk-mstp.c
>> >> @@ -144,6 +144,9 @@ cpg_mstp_clock_register(const char *name, const char
>> >> *paren init.name = name;
>> >>
>> >>         init.ops = &cpg_mstp_clock_ops;
>> >>         init.flags = CLK_IS_BASIC | CLK_SET_RATE_PARENT;
>> >>
>> >> +       /* INTC-SYS is the module clock of the GIC, and must not be
>> >> disabled */
>> >> +       if (!strcmp(name, "intc-sys"))
>> >> +               init.flags |= CLK_ENABLE_HAND_OFF;
>> >>
>> >>         init.parent_names = &parent_name;
>> >>         init.num_parents = 1;
>> >>
>> >> (FTR, on R-Car Gen3 the clock is called "intc-ap", not "intc-sys",
>> >> doh...).
>> >>
>> >> This does put some reliance on (undocumented) naming in DT, though, so
>> >> not
>> >> having the "clock-output-names" would solve that.
>> >
>> > Using the clock name is indeed very messy, let's avoid that.
>> >
>> >> However, while "intc-sys"/"intc-ap" (if existent) is always MSTP408,
>> >> MSTP408 is used for other modules on R-Car Gen1 and most older SoCs. so
>> >> it would complicate the driver code.
>> >
>> > How about setting the flag based on information provided in DT ?
>>
>> Like scanning DT for the GIC, and looking for its clock?
>>
>> I don't want to add an explicit flag in DT, as it's not a hardware property,
>> but a deficiency of the current GIC driver/subsystem.
>
> What are the changes we can fix the GIC driver in a reasonable time frame ?

Very low.

The GIC driver uses IRQCHIP_DECLARE(), hence it's initialized from
of_irq_init() <- irqchip_init() <- init_IRQ() <- start_kernel().

The MSTP clock driver uses CLK_OF_DECLARE(), hence it's initialized from
of_clk_init() <- time_init() <- start_kernel(), but 7 calls later.

IRQCHIP_DECLARE() doesn't support deferred probing.

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 Torvald

Re: [PATCH 3/4 v3][RFC] arm64: renesas: Add initial r8a7795 SoC support

[Geert Uytterhoeven]

Hi Mike,

On Wed, Aug 19, 2015 at 9:49 AM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> On Tue, Aug 18, 2015 at 2:20 AM, Michael Turquette
> <mturquette@baylibre.com> wrote:
>> Also, while you're thinking about the perfect clock binding, please do
>> consider dropping clock-output-names if you can. Specifying clock-names
>> alongside the clocks property inside of the consumer node is a bit more
>> elegant in my opinion. This is also a bit easier if you think about
>> expressing your clock data with C inside of your provider driver.
>
> Makes sense. I don't think anything relies on the "clock-output-names"
> ... currently.

BTW, was "dropping clock-output-names" a general comment for all clock
drivers, or specific to the SoC's Clock Pulse Generator?

For e.g. "fixed-factor-clock", the driver falls back to using the node's name
if  "clock-output-names" is not present.

But e.g. for MSTP clocks, that can't be done, as the clocks wouldn't have
names in that case (single node with multiple clocks). Unless we start
fabricating them from the node name and the indices.

> I was going to use it for identifying the GIC clock, though:

That applies to MSTP clocks ("intc-sys"), not the main CPG block 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

Re: [PATCH 3/4 v3][RFC] arm64: renesas: Add initial r8a7795 SoC support

[Geert Uytterhoeven]

Hi Mike,

On Wed, Aug 19, 2015 at 9:49 AM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> On Tue, Aug 18, 2015 at 2:20 AM, Michael Turquette
> <mturquette@baylibre.com> wrote:
>> Also, while you're thinking about the perfect clock binding, please do
>> consider dropping clock-output-names if you can. Specifying clock-names
>> alongside the clocks property inside of the consumer node is a bit more
>> elegant in my opinion. This is also a bit easier if you think about
>> expressing your clock data with C inside of your provider driver.
>
> Makes sense. I don't think anything relies on the "clock-output-names"
> ... currently.

BTW, was "dropping clock-output-names" a general comment for all clock
drivers, or specific to the SoC's Clock Pulse Generator?

For e.g. "fixed-factor-clock", the driver falls back to using the node's name
if  "clock-output-names" is not present.

But e.g. for MSTP clocks, that can't be done, as the clocks wouldn't have
names in that case (single node with multiple clocks). Unless we start
fabricating them from the node name and the indices.

> I was going to use it for identifying the GIC clock, though:

That applies to MSTP clocks ("intc-sys"), not the main CPG block 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