[PATCH 0/2] Add Initial Device Tree for RZ/A2

[PATCH 0/2] Add Initial Device Tree for RZ/A2

[Chris Brandt]

Add a Device Tree for RZ/A2 and the existing eval board.

Once these get approved, I'll start piling on the other drivers in
another patch series.


Chris Brandt (2):
  ARM: dts: r7s9210: Initial SoC device tree
  ARM: dts: r7s9210-rza2mevb: Add support for RZ/A2M EVB

 Documentation/devicetree/bindings/arm/shmobile.txt |   2 +
 arch/arm/boot/dts/Makefile                         |   1 +
 arch/arm/boot/dts/r7s9210-rza2mevb.dts             | 133 +++++++++++++
 arch/arm/boot/dts/r7s9210.dtsi                     | 211 +++++++++++++++++++++
 4 files changed, 347 insertions(+)
 create mode 100644 arch/arm/boot/dts/r7s9210-rza2mevb.dts
 create mode 100644 arch/arm/boot/dts/r7s9210.dtsi

-- 
2.16.1

[PATCH 1/2] ARM: dts: r7s9210: Initial SoC device tree

[Chris Brandt]

Basic support for the RZ/A2 (R7S9210) SoC.

Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
---
 arch/arm/boot/dts/r7s9210.dtsi | 211 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 211 insertions(+)
 create mode 100644 arch/arm/boot/dts/r7s9210.dtsi

diff --git a/arch/arm/boot/dts/r7s9210.dtsi b/arch/arm/boot/dts/r7s9210.dtsi
new file mode 100644
index 000000000000..3c5373a5c7d2
--- /dev/null
+++ b/arch/arm/boot/dts/r7s9210.dtsi
@@ -0,0 +1,211 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Device Tree Source for the R7S9210 SoC
+ *
+ * Copyright (C) 2018 Renesas Electronics Corporation
+ *
+ */
+
+#include <dt-bindings/interrupt-controller/arm-gic.h>
+#include <dt-bindings/clock/r7s9210-cpg-mssr.h>
+
+/ {
+	compatible = "renesas,r7s9210";
+	interrupt-parent = <&gic>;
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	bsid: chipid@fcfe8004 {
+		compatible = "renesas,bsid";
+		reg = <0xfcfe8004 4>;
+	};
+
+	clocks {
+		ranges;
+		#address-cells = <1>;
+		#size-cells = <1>;
+
+		/* External clocks */
+		extal_clk: extal {
+			#clock-cells = <0>;
+			compatible = "fixed-clock";
+			/* Value must be set by board */
+			clock-frequency = <0>;
+		};
+
+		rtc_x1_clk: rtc_x1 {
+			#clock-cells = <0>;
+			compatible = "fixed-clock";
+			/* If clk present, value (32678) must be set by board */
+			clock-frequency = <0>;
+		};
+	};
+
+	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		cpu@0 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a9";
+			reg = <0>;
+			clock-frequency = <528000000>;
+			next-level-cache = <&L2>;
+		};
+	};
+
+	gic: interrupt-controller@e8221000 {
+		compatible = "arm,gic-400";
+		#interrupt-cells = <3>;
+		#address-cells = <0>;
+		interrupt-controller;
+		reg = <0xe8221000 0x1000>,
+		      <0xe8222000 0x1000>;
+	};
+
+	L2: cache-controller@1f003000 {
+		compatible = "arm,pl310-cache";
+		reg = <0x1f003000 0x1000>;
+		interrupts = <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>;
+		arm,early-bresp-disable;
+		arm,full-line-zero-disable;
+		cache-unified;
+		cache-level = <2>;
+	};
+
+	cpg: clock-controller@fcfe0020 {
+		compatible = "renesas,r7s9210-cpg-mssr";
+		reg = <0xfcfe0010 0x455>;
+		clocks = <&extal_clk>;
+		clock-names = "extal";
+		#clock-cells = <2>;
+		#power-domain-cells = <0>;
+		#reset-cells = <1>;
+	};
+
+	ostm0: timer@e803b000 {
+		compatible = "renesas,r7s9210-ostm", "renesas,ostm";
+		reg = <0xe803b000 0x30>;
+		interrupts = <GIC_SPI 56 IRQ_TYPE_EDGE_RISING>;
+		clocks = <&cpg CPG_MOD 36>;
+		clock-names = "ostm0";
+		power-domains = <&cpg>;
+		status = "disabled";
+	};
+
+	ostm1: timer@e803c000 {
+		compatible = "renesas,r7s9210-ostm", "renesas,ostm";
+		reg = <0xe803c000 0x30>;
+		interrupts = <GIC_SPI 57 IRQ_TYPE_EDGE_RISING>;
+		clocks = <&cpg CPG_MOD 35>;
+		clock-names = "ostm1";
+		power-domains = <&cpg>;
+		status = "disabled";
+	};
+
+	ostm2: timer@e803d000 {
+		compatible = "renesas,r7s9210-ostm", "renesas,ostm";
+		reg = <0xe803d000 0x30>;
+		interrupts = <GIC_SPI 58 IRQ_TYPE_EDGE_RISING>;
+		clocks = <&cpg CPG_MOD 34>;
+		clock-names = "ostm2";
+		power-domains = <&cpg>;
+		status = "disabled";
+	};
+
+	wdt: watchdog@fcfe7000 {
+		compatible = "renesas,r7s9210-wdt", "renesas,rza-wdt";
+		reg = <0xfcfe7000 0x26>;
+		interrupts = <GIC_SPI 61 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&cpg CPG_CORE R7S9210_CLK_P0>;
+	};
+
+	pinctrl: pin-controller@fcffe000 {
+		compatible = "renesas,r7s9210-pinctrl";
+		reg = <0xfcffe000 0x1000>;
+
+		gpio-controller;
+		#gpio-cells = <2>;
+		gpio-ranges = <&pinctrl 0 0 176>;
+	};
+
+	scif0: serial@e8007000 {
+		compatible = "renesas,scif-r7s9210";
+		reg = <0xe8007000 18>;
+		interrupts = <GIC_SPI 265 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 266 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 267 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 265 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 268 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 268 IRQ_TYPE_LEVEL_HIGH>;
+		interrupt-names = "eri", "rxi", "txi", "bri", "dri", "tei";
+		clocks = <&cpg CPG_MOD 47>;
+		clock-names = "fck";
+		power-domains = <&cpg>;
+		status = "disabled";
+	};
+
+	scif1: serial@e8007800 {
+		compatible = "renesas,scif-r7s9210";
+		reg = <0xe8007800 18>;
+		interrupts = <GIC_SPI 271 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 272 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 273 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 271 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 274 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 274 IRQ_TYPE_LEVEL_HIGH>;
+		interrupt-names = "eri", "rxi", "txi", "bri", "dri", "tei";
+		clocks = <&cpg CPG_MOD 46>;
+		clock-names = "fck";
+		power-domains = <&cpg>;
+		status = "disabled";
+	};
+
+	scif2: serial@e8008000 {
+		compatible = "renesas,scif-r7s9210";
+		reg = <0xe8008000 18>;
+		interrupts = <GIC_SPI 277 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 278 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 279 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 277 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 280 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 280 IRQ_TYPE_LEVEL_HIGH>;
+		interrupt-names = "eri", "rxi", "txi", "bri", "dri", "tei";
+		clocks = <&cpg CPG_MOD 45>;
+		clock-names = "fck";
+		power-domains = <&cpg>;
+		status = "disabled";
+	};
+
+	scif3: serial@e8008800 {
+		compatible = "renesas,scif-r7s9210";
+		reg = <0xe8008800 18>;
+		interrupts = <GIC_SPI 283 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 284 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 285 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 283 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 286 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 286 IRQ_TYPE_LEVEL_HIGH>;
+		interrupt-names = "eri", "rxi", "txi", "bri", "dri", "tei";
+		clocks = <&cpg CPG_MOD 44>;
+		clock-names = "fck";
+		power-domains = <&cpg>;
+		status = "disabled";
+	};
+
+	scif4: serial@e8009000 {
+		compatible = "renesas,scif-r7s9210";
+		reg = <0xe8009000 18>;
+		interrupts = <GIC_SPI 289 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 290 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 291 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 289 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 292 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 292 IRQ_TYPE_LEVEL_HIGH>;
+		interrupt-names = "eri", "rxi", "txi", "bri", "dri", "tei";
+		clocks = <&cpg CPG_MOD 43>;
+		clock-names = "fck";
+		power-domains = <&cpg>;
+		status = "disabled";
+	};
+};
-- 
2.16.1

Re: [PATCH 1/2] ARM: dts: r7s9210: Initial SoC device tree

[Simon Horman]

On Thu, Nov 29, 2018 at 08:05:58AM -0500, Chris Brandt wrote:
> Basic support for the RZ/A2 (R7S9210) SoC.
> 
> Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
> ---
>  arch/arm/boot/dts/r7s9210.dtsi | 211 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 211 insertions(+)
>  create mode 100644 arch/arm/boot/dts/r7s9210.dtsi
> 
> diff --git a/arch/arm/boot/dts/r7s9210.dtsi b/arch/arm/boot/dts/r7s9210.dtsi
> new file mode 100644
> index 000000000000..3c5373a5c7d2
> --- /dev/null
> +++ b/arch/arm/boot/dts/r7s9210.dtsi
> @@ -0,0 +1,211 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Device Tree Source for the R7S9210 SoC
> + *
> + * Copyright (C) 2018 Renesas Electronics Corporation
> + *
> + */
> +
> +#include <dt-bindings/interrupt-controller/arm-gic.h>
> +#include <dt-bindings/clock/r7s9210-cpg-mssr.h>
> +
> +/ {
> +	compatible = "renesas,r7s9210";
> +	interrupt-parent = <&gic>;
> +	#address-cells = <1>;
> +	#size-cells = <1>;
> +
> +	bsid: chipid@fcfe8004 {
> +		compatible = "renesas,bsid";
> +		reg = <0xfcfe8004 4>;
> +	};
> +
> +	clocks {
> +		ranges;
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +
> +		/* External clocks */
> +		extal_clk: extal {
> +			#clock-cells = <0>;
> +			compatible = "fixed-clock";
> +			/* Value must be set by board */
> +			clock-frequency = <0>;
> +		};
> +
> +		rtc_x1_clk: rtc_x1 {
> +			#clock-cells = <0>;
> +			compatible = "fixed-clock";
> +			/* If clk present, value (32678) must be set by board */
> +			clock-frequency = <0>;
> +		};
> +	};
> +
> +	cpus {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		cpu@0 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a9";
> +			reg = <0>;
> +			clock-frequency = <528000000>;
> +			next-level-cache = <&L2>;
> +		};
> +	};
> +
> +	gic: interrupt-controller@e8221000 {
> +		compatible = "arm,gic-400";
> +		#interrupt-cells = <3>;
> +		#address-cells = <0>;
> +		interrupt-controller;
> +		reg = <0xe8221000 0x1000>,
> +		      <0xe8222000 0x1000>;
> +	};
> +
> +	L2: cache-controller@1f003000 {
> +		compatible = "arm,pl310-cache";
> +		reg = <0x1f003000 0x1000>;
> +		interrupts = <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>;
> +		arm,early-bresp-disable;
> +		arm,full-line-zero-disable;
> +		cache-unified;
> +		cache-level = <2>;
> +	};
> +
> +	cpg: clock-controller@fcfe0020 {
> +		compatible = "renesas,r7s9210-cpg-mssr";
> +		reg = <0xfcfe0010 0x455>;

There is a discrepancy here between the base address, fcfe0020
and the start address of the register range, 0xfcfe0010.

> +		clocks = <&extal_clk>;
> +		clock-names = "extal";
> +		#clock-cells = <2>;
> +		#power-domain-cells = <0>;
> +		#reset-cells = <1>;
> +	};
> +
> +	ostm0: timer@e803b000 {
> +		compatible = "renesas,r7s9210-ostm", "renesas,ostm";
> +		reg = <0xe803b000 0x30>;


Its not clear to me why the size of the register range is 0x30.

> +		interrupts = <GIC_SPI 56 IRQ_TYPE_EDGE_RISING>;
> +		clocks = <&cpg CPG_MOD 36>;
> +		clock-names = "ostm0";
> +		power-domains = <&cpg>;
> +		status = "disabled";
> +	};
> +
> +	ostm1: timer@e803c000 {
> +		compatible = "renesas,r7s9210-ostm", "renesas,ostm";
> +		reg = <0xe803c000 0x30>;
> +		interrupts = <GIC_SPI 57 IRQ_TYPE_EDGE_RISING>;
> +		clocks = <&cpg CPG_MOD 35>;
> +		clock-names = "ostm1";
> +		power-domains = <&cpg>;
> +		status = "disabled";
> +	};
> +
> +	ostm2: timer@e803d000 {
> +		compatible = "renesas,r7s9210-ostm", "renesas,ostm";
> +		reg = <0xe803d000 0x30>;
> +		interrupts = <GIC_SPI 58 IRQ_TYPE_EDGE_RISING>;
> +		clocks = <&cpg CPG_MOD 34>;
> +		clock-names = "ostm2";
> +		power-domains = <&cpg>;
> +		status = "disabled";
> +	};
> +
> +	wdt: watchdog@fcfe7000 {
> +		compatible = "renesas,r7s9210-wdt", "renesas,rza-wdt";
> +		reg = <0xfcfe7000 0x26>;
> +		interrupts = <GIC_SPI 61 IRQ_TYPE_LEVEL_HIGH>;
> +		clocks = <&cpg CPG_CORE R7S9210_CLK_P0>;
> +	};
> +
> +	pinctrl: pin-controller@fcffe000 {
> +		compatible = "renesas,r7s9210-pinctrl";
> +		reg = <0xfcffe000 0x1000>;
> +
> +		gpio-controller;
> +		#gpio-cells = <2>;
> +		gpio-ranges = <&pinctrl 0 0 176>;
> +	};
> +
> +	scif0: serial@e8007000 {
> +		compatible = "renesas,scif-r7s9210";
> +		reg = <0xe8007000 18>;
> +		interrupts = <GIC_SPI 265 IRQ_TYPE_LEVEL_HIGH>,
> +			     <GIC_SPI 266 IRQ_TYPE_LEVEL_HIGH>,
> +			     <GIC_SPI 267 IRQ_TYPE_LEVEL_HIGH>,
> +			     <GIC_SPI 265 IRQ_TYPE_LEVEL_HIGH>,
> +			     <GIC_SPI 268 IRQ_TYPE_LEVEL_HIGH>,
> +			     <GIC_SPI 268 IRQ_TYPE_LEVEL_HIGH>;
> +		interrupt-names = "eri", "rxi", "txi", "bri", "dri", "tei";
> +		clocks = <&cpg CPG_MOD 47>;
> +		clock-names = "fck";
> +		power-domains = <&cpg>;
> +		status = "disabled";
> +	};
> +
> +	scif1: serial@e8007800 {
> +		compatible = "renesas,scif-r7s9210";
> +		reg = <0xe8007800 18>;
> +		interrupts = <GIC_SPI 271 IRQ_TYPE_LEVEL_HIGH>,
> +			     <GIC_SPI 272 IRQ_TYPE_LEVEL_HIGH>,
> +			     <GIC_SPI 273 IRQ_TYPE_LEVEL_HIGH>,
> +			     <GIC_SPI 271 IRQ_TYPE_LEVEL_HIGH>,
> +			     <GIC_SPI 274 IRQ_TYPE_LEVEL_HIGH>,
> +			     <GIC_SPI 274 IRQ_TYPE_LEVEL_HIGH>;
> +		interrupt-names = "eri", "rxi", "txi", "bri", "dri", "tei";
> +		clocks = <&cpg CPG_MOD 46>;
> +		clock-names = "fck";
> +		power-domains = <&cpg>;
> +		status = "disabled";
> +	};
> +
> +	scif2: serial@e8008000 {
> +		compatible = "renesas,scif-r7s9210";
> +		reg = <0xe8008000 18>;
> +		interrupts = <GIC_SPI 277 IRQ_TYPE_LEVEL_HIGH>,
> +			     <GIC_SPI 278 IRQ_TYPE_LEVEL_HIGH>,
> +			     <GIC_SPI 279 IRQ_TYPE_LEVEL_HIGH>,
> +			     <GIC_SPI 277 IRQ_TYPE_LEVEL_HIGH>,
> +			     <GIC_SPI 280 IRQ_TYPE_LEVEL_HIGH>,
> +			     <GIC_SPI 280 IRQ_TYPE_LEVEL_HIGH>;
> +		interrupt-names = "eri", "rxi", "txi", "bri", "dri", "tei";
> +		clocks = <&cpg CPG_MOD 45>;
> +		clock-names = "fck";
> +		power-domains = <&cpg>;
> +		status = "disabled";
> +	};
> +
> +	scif3: serial@e8008800 {
> +		compatible = "renesas,scif-r7s9210";
> +		reg = <0xe8008800 18>;
> +		interrupts = <GIC_SPI 283 IRQ_TYPE_LEVEL_HIGH>,
> +			     <GIC_SPI 284 IRQ_TYPE_LEVEL_HIGH>,
> +			     <GIC_SPI 285 IRQ_TYPE_LEVEL_HIGH>,
> +			     <GIC_SPI 283 IRQ_TYPE_LEVEL_HIGH>,
> +			     <GIC_SPI 286 IRQ_TYPE_LEVEL_HIGH>,
> +			     <GIC_SPI 286 IRQ_TYPE_LEVEL_HIGH>;
> +		interrupt-names = "eri", "rxi", "txi", "bri", "dri", "tei";
> +		clocks = <&cpg CPG_MOD 44>;
> +		clock-names = "fck";
> +		power-domains = <&cpg>;
> +		status = "disabled";
> +	};
> +
> +	scif4: serial@e8009000 {
> +		compatible = "renesas,scif-r7s9210";
> +		reg = <0xe8009000 18>;
> +		interrupts = <GIC_SPI 289 IRQ_TYPE_LEVEL_HIGH>,
> +			     <GIC_SPI 290 IRQ_TYPE_LEVEL_HIGH>,
> +			     <GIC_SPI 291 IRQ_TYPE_LEVEL_HIGH>,
> +			     <GIC_SPI 289 IRQ_TYPE_LEVEL_HIGH>,
> +			     <GIC_SPI 292 IRQ_TYPE_LEVEL_HIGH>,
> +			     <GIC_SPI 292 IRQ_TYPE_LEVEL_HIGH>;
> +		interrupt-names = "eri", "rxi", "txi", "bri", "dri", "tei";
> +		clocks = <&cpg CPG_MOD 43>;
> +		clock-names = "fck";
> +		power-domains = <&cpg>;
> +		status = "disabled";
> +	};
> +};
> -- 
> 2.16.1
> 

RE: [PATCH 1/2] ARM: dts: r7s9210: Initial SoC device tree

[Chris Brandt]

Hi Simon,


On Friday, November 30, 2018, Simon Horman wrote:
> > +	cpg: clock-controller@fcfe0020 {
> > +		compatible = "renesas,r7s9210-cpg-mssr";
> > +		reg = <0xfcfe0010 0x455>;
> 
> There is a discrepancy here between the base address, fcfe0020
> and the start address of the register range, 0xfcfe0010.

Good catch! Thank you.


> > +	ostm0: timer@e803b000 {
> > +		compatible = "renesas,r7s9210-ostm", "renesas,ostm";
> > +		reg = <0xe803b000 0x30>;
> 
> 
> Its not clear to me why the size of the register range is 0x30.

Because when reviewing the bindings docs, I was putting in 'exact' 
values, but Geert said "those are ugly, just round up to make it look
nicer".
So instead of 0x21, I put 0x30.

You know...the standard answer: Throw Geert under the bus ;)


Chris

Re: [PATCH 1/2] ARM: dts: r7s9210: Initial SoC device tree

[Simon Horman]

On Fri, Nov 30, 2018 at 12:04:57PM +0000, Chris Brandt wrote:
> Hi Simon,
> 
> 
> On Friday, November 30, 2018, Simon Horman wrote:
> > > +	cpg: clock-controller@fcfe0020 {
> > > +		compatible = "renesas,r7s9210-cpg-mssr";
> > > +		reg = <0xfcfe0010 0x455>;
> > 
> > There is a discrepancy here between the base address, fcfe0020
> > and the start address of the register range, 0xfcfe0010.
> 
> Good catch! Thank you.
> 
> 
> > > +	ostm0: timer@e803b000 {
> > > +		compatible = "renesas,r7s9210-ostm", "renesas,ostm";
> > > +		reg = <0xe803b000 0x30>;
> > 
> > 
> > Its not clear to me why the size of the register range is 0x30.
> 
> Because when reviewing the bindings docs, I was putting in 'exact' 
> values, but Geert said "those are ugly, just round up to make it look
> nicer".
> So instead of 0x21, I put 0x30.
> 
> You know...the standard answer: Throw Geert under the bus ;)

Lets leave 0x30 :)

Re: [PATCH 1/2] ARM: dts: r7s9210: Initial SoC device tree

[Geert Uytterhoeven]

Hi Simon,

On Fri, Nov 30, 2018 at 1:23 PM Simon Horman <horms@verge.net.au> wrote:
> On Fri, Nov 30, 2018 at 12:04:57PM +0000, Chris Brandt wrote:
> > On Friday, November 30, 2018, Simon Horman wrote:
> > > > + ostm0: timer@e803b000 {
> > > > +         compatible = "renesas,r7s9210-ostm", "renesas,ostm";
> > > > +         reg = <0xe803b000 0x30>;
> > >
> > >
> > > Its not clear to me why the size of the register range is 0x30.
> >
> > Because when reviewing the bindings docs, I was putting in 'exact'
> > values, but Geert said "those are ugly, just round up to make it look
> > nicer".
> > So instead of 0x21, I put 0x30.
> >
> > You know...the standard answer: Throw Geert under the bus ;)
>
> Lets leave 0x30 :)

Pfew...

BTW, I'd be surprised the hardware address decoder would route all
addresses in the range 0xe803b000..0xe803b02f to the OSTM.
0xe803b000..0xe803b03f sounds more logical to me, as it requires less gates.
But hey, who said hardware designers are logical? ;-)

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 1/2] ARM: dts: r7s9210: Initial SoC device tree

[Chris Brandt]

From: Geert Uytterhoeven [mailto:geert@linux-m68k.org]
Sent: Friday, November 30, 2018 11:03 AM
> BTW, I'd be surprised the hardware address decoder would route all
> addresses in the range 0xe803b000..0xe803b02f to the OSTM.
> 0xe803b000..0xe803b03f sounds more logical to me, as it requires less
> gates.

Gates??? I thought everything was magical RTL now a days???


Chris

Re: [PATCH 1/2] ARM: dts: r7s9210: Initial SoC device tree

[Geert Uytterhoeven]

Hi Chris,

On Thu, Nov 29, 2018 at 2:07 PM Chris Brandt <chris.brandt@renesas.com> wrote:
> Basic support for the RZ/A2 (R7S9210) SoC.
>
> Signed-off-by: Chris Brandt <chris.brandt@renesas.com>

Thanks for your patch!

> --- /dev/null
> +++ b/arch/arm/boot/dts/r7s9210.dtsi
> @@ -0,0 +1,211 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Device Tree Source for the R7S9210 SoC
> + *
> + * Copyright (C) 2018 Renesas Electronics Corporation
> + *
> + */
> +
> +#include <dt-bindings/interrupt-controller/arm-gic.h>
> +#include <dt-bindings/clock/r7s9210-cpg-mssr.h>
> +
> +/ {

> +       clocks {

Please remove the clocks subnode.
These days clocks live at the root node.

> +       gic: interrupt-controller@e8221000 {
> +               compatible = "arm,gic-400";
> +               #interrupt-cells = <3>;
> +               #address-cells = <0>;
> +               interrupt-controller;
> +               reg = <0xe8221000 0x1000>,
> +                     <0xe8222000 0x1000>;
> +       };
> +
> +       L2: cache-controller@1f003000 {

Please sort nodes by address (per group of devices).

> +               compatible = "arm,pl310-cache";
> +               reg = <0x1f003000 0x1000>;
> +               interrupts = <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>;
> +               arm,early-bresp-disable;
> +               arm,full-line-zero-disable;
> +               cache-unified;
> +               cache-level = <2>;
> +       };
> +
> +       cpg: clock-controller@fcfe0020 {

fcfe0010, as pointed out by Simon.

> +               compatible = "renesas,r7s9210-cpg-mssr";
> +               reg = <0xfcfe0010 0x455>;
> +               clocks = <&extal_clk>;
> +               clock-names = "extal";
> +               #clock-cells = <2>;
> +               #power-domain-cells = <0>;
> +               #reset-cells = <1>;

Note that resets are not yet supported by the driver.
But probably they will use #reset-cells = <1>, if ever implemented.

> +       };
> +
> +       ostm0: timer@e803b000 {
> +               compatible = "renesas,r7s9210-ostm", "renesas,ostm";
> +               reg = <0xe803b000 0x30>;
> +               interrupts = <GIC_SPI 56 IRQ_TYPE_EDGE_RISING>;
> +               clocks = <&cpg CPG_MOD 36>;
> +               clock-names = "ostm0";

The clock-names property is not documented in the DT bindings.
Moreover, using different names for the clock inputs of the different
channels is strange.

> +       scif0: serial@e8007000 {
> +               compatible = "renesas,scif-r7s9210";
> +               reg = <0xe8007000 18>;

0x18 (for all scif instances)


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

[PATCH 2/2] ARM: dts: r7s9210-rza2mevb: Add support for RZ/A2M EVB

[Chris Brandt]

Add support for Renesas RZ/A2M evaluation board.

Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
---
 Documentation/devicetree/bindings/arm/shmobile.txt |   2 +
 arch/arm/boot/dts/Makefile                         |   1 +
 arch/arm/boot/dts/r7s9210-rza2mevb.dts             | 133 +++++++++++++++++++++
 3 files changed, 136 insertions(+)
 create mode 100644 arch/arm/boot/dts/r7s9210-rza2mevb.dts

diff --git a/Documentation/devicetree/bindings/arm/shmobile.txt b/Documentation/devicetree/bindings/arm/shmobile.txt
index a94db59d8e2d..67a6810c4345 100644
--- a/Documentation/devicetree/bindings/arm/shmobile.txt
+++ b/Documentation/devicetree/bindings/arm/shmobile.txt
@@ -125,6 +125,8 @@ Boards:
     compatible = "renesas,porter", "renesas,r8a7791"
   - RSKRZA1 (YR0K77210C000BE)
     compatible = "renesas,rskrza1", "renesas,r7s72100"
+  - RZ/A2M Eval Board (RTK7921053S00000BE)
+    compatible = "renesas,rza2mevb", "renesas,r7s9210"
   - RZN1D-DB (RZ/N1D Demo Board for the RZ/N1D 400 pins package)
     compatible = "renesas,rzn1d400-db", "renesas,r9a06g032"
   - Salvator-X (RTP0RC7795SIPB0010S)
diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index b0e966d625b9..fbdd068fd8af 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -824,6 +824,7 @@ dtb-$(CONFIG_ARCH_RENESAS) += \
 	r7s72100-genmai.dtb \
 	r7s72100-gr-peach.dtb \
 	r7s72100-rskrza1.dtb \
+	r7s9210-rza2mevb.dtb \
 	r8a73a4-ape6evm.dtb \
 	r8a7740-armadillo800eva.dtb \
 	r8a7743-iwg20d-q7.dtb \
diff --git a/arch/arm/boot/dts/r7s9210-rza2mevb.dts b/arch/arm/boot/dts/r7s9210-rza2mevb.dts
new file mode 100644
index 000000000000..9570aeb8f1ce
--- /dev/null
+++ b/arch/arm/boot/dts/r7s9210-rza2mevb.dts
@@ -0,0 +1,133 @@
+/*
+ * Device Tree Source for the RZA2MEVB board
+ *
+ * Copyright (C) 2018 Renesas Electronics
+ *
+ * This file is dual-licensed: you can use it either under the terms
+ * of the GPL or the X11 license, at your option. Note that this dual
+ * licensing only applies to this file, and not this project as a
+ * whole.
+ *
+ *  a) This file is free software; you can redistribute it and/or
+ *     modify it under the terms of the GNU General Public License as
+ *     published by the Free Software Foundation; either version 2 of the
+ *     License, or (at your option) any later version.
+ *
+ *     This file is distributed in the hope that it will be useful,
+ *     but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *     MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *     GNU General Public License for more details.
+ *
+ * Or, alternatively,
+ *
+ *  b) Permission is hereby granted, free of charge, to any person
+ *     obtaining a copy of this software and associated documentation
+ *     files (the "Software"), to deal in the Software without
+ *     restriction, including without limitation the rights to use,
+ *     copy, modify, merge, publish, distribute, sublicense, and/or
+ *     sell copies of the Software, and to permit persons to whom the
+ *     Software is furnished to do so, subject to the following
+ *     conditions:
+ *
+ *     The above copyright notice and this permission notice shall be
+ *     included in all copies or substantial portions of the Software.
+ *
+ *     THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+ *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+ *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ *     OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+/dts-v1/;
+#include "r7s9210.dtsi"
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/pinctrl/r7s9210-pinctrl.h>
+
+/ {
+	model = "RZA2MEVB";
+	compatible = "renesas,rza2mevb", "renesas,r7s9210";
+
+	aliases {
+		serial0 = &scif4;
+	};
+
+	chosen {
+		bootargs = "ignore_loglevel earlycon rootfstype=cramfs root=mtd:rootfs";
+		stdout-path = "serial0:115200n8";
+	};
+
+	memory@80000000 {
+		device_type = "memory";
+		reg = <0x40000000 0x00800000>;	 /* HyperRAM */
+	};
+
+	lbsc {
+		#address-cells = <1>;
+		#size-cells = <1>;
+	};
+
+	leds {
+		status = "okay";
+		compatible = "gpio-leds";
+
+		led0 {
+			gpios = <&pinctrl RZA2_PIN(PORT6, 0) GPIO_ACTIVE_HIGH>;
+		};
+	};
+
+	/* Cramfs XIP File System in QSPI */
+	qspi@20000000 {
+		compatible = "mtd-rom";
+		probe-type = "direct-mapped";	/* XIP from QSPI */
+		reg = <0x20000000 0x4000000>;	/* 64 MB*/
+		bank-width = <4>;
+		device-width = <1>;
+		#address-cells = <1>;
+		#size-cells = <1>;
+		partition@800000 {
+			label ="rootfs";
+			reg = <0x0800000 0x1000000>; /* 16MB @ 0x20800000 */
+			read-only;
+		};
+	};
+};
+
+/* EXTAL */
+&extal_clk {
+	clock-frequency = <24000000>;	/* 24MHz */
+};
+
+/* RTC_X1 */
+&rtc_x1_clk {
+	clock-frequency = <32768>;
+};
+
+&pinctrl {
+
+	/* Serial Console */
+	scif4_pins: serial4 {
+		pinmux = <RZA2_PINMUX(PORT9, 0, 4)>,	/* TxD4 */
+			 <RZA2_PINMUX(PORT9, 1, 4)>;	/* RxD4 */
+	};
+};
+
+/* High resolution System tick timers */
+&ostm0 {
+	status = "okay";
+};
+
+&ostm1 {
+	status = "okay";
+};
+
+/* Serial Console */
+&scif4 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&scif4_pins>;
+
+	status = "okay";
+};
-- 
2.16.1

Re: [PATCH 2/2] ARM: dts: r7s9210-rza2mevb: Add support for RZ/A2M EVB

[Simon Horman]

On Thu, Nov 29, 2018 at 08:05:59AM -0500, Chris Brandt wrote:
> Add support for Renesas RZ/A2M evaluation board.
> 
> Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
> ---
>  Documentation/devicetree/bindings/arm/shmobile.txt |   2 +
>  arch/arm/boot/dts/Makefile                         |   1 +
>  arch/arm/boot/dts/r7s9210-rza2mevb.dts             | 133 +++++++++++++++++++++
>  3 files changed, 136 insertions(+)
>  create mode 100644 arch/arm/boot/dts/r7s9210-rza2mevb.dts
> 
> diff --git a/Documentation/devicetree/bindings/arm/shmobile.txt b/Documentation/devicetree/bindings/arm/shmobile.txt
> index a94db59d8e2d..67a6810c4345 100644
> --- a/Documentation/devicetree/bindings/arm/shmobile.txt
> +++ b/Documentation/devicetree/bindings/arm/shmobile.txt
> @@ -125,6 +125,8 @@ Boards:
>      compatible = "renesas,porter", "renesas,r8a7791"
>    - RSKRZA1 (YR0K77210C000BE)
>      compatible = "renesas,rskrza1", "renesas,r7s72100"
> +  - RZ/A2M Eval Board (RTK7921053S00000BE)
> +    compatible = "renesas,rza2mevb", "renesas,r7s9210"
>    - RZN1D-DB (RZ/N1D Demo Board for the RZ/N1D 400 pins package)
>      compatible = "renesas,rzn1d400-db", "renesas,r9a06g032"
>    - Salvator-X (RTP0RC7795SIPB0010S)

Pleas separate the change above into a separate patch.
Such changes typically go upstream via a separate branch.

> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index b0e966d625b9..fbdd068fd8af 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -824,6 +824,7 @@ dtb-$(CONFIG_ARCH_RENESAS) += \
>  	r7s72100-genmai.dtb \
>  	r7s72100-gr-peach.dtb \
>  	r7s72100-rskrza1.dtb \
> +	r7s9210-rza2mevb.dtb \
>  	r8a73a4-ape6evm.dtb \
>  	r8a7740-armadillo800eva.dtb \
>  	r8a7743-iwg20d-q7.dtb \
> diff --git a/arch/arm/boot/dts/r7s9210-rza2mevb.dts b/arch/arm/boot/dts/r7s9210-rza2mevb.dts
> new file mode 100644
> index 000000000000..9570aeb8f1ce
> --- /dev/null
> +++ b/arch/arm/boot/dts/r7s9210-rza2mevb.dts
> @@ -0,0 +1,133 @@
> +/*
> + * Device Tree Source for the RZA2MEVB board
> + *
> + * Copyright (C) 2018 Renesas Electronics
> + *
> + * This file is dual-licensed: you can use it either under the terms
> + * of the GPL or the X11 license, at your option. Note that this dual
> + * licensing only applies to this file, and not this project as a
> + * whole.
> + *
> + *  a) This file is free software; you can redistribute it and/or
> + *     modify it under the terms of the GNU General Public License as
> + *     published by the Free Software Foundation; either version 2 of the
> + *     License, or (at your option) any later version.
> + *
> + *     This file is distributed in the hope that it will be useful,
> + *     but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *     MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *     GNU General Public License for more details.
> + *
> + * Or, alternatively,
> + *
> + *  b) Permission is hereby granted, free of charge, to any person
> + *     obtaining a copy of this software and associated documentation
> + *     files (the "Software"), to deal in the Software without
> + *     restriction, including without limitation the rights to use,
> + *     copy, modify, merge, publish, distribute, sublicense, and/or
> + *     sell copies of the Software, and to permit persons to whom the
> + *     Software is furnished to do so, subject to the following
> + *     conditions:
> + *
> + *     The above copyright notice and this permission notice shall be
> + *     included in all copies or substantial portions of the Software.
> + *
> + *     THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> + *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
> + *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> + *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
> + *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
> + *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> + *     OTHER DEALINGS IN THE SOFTWARE.
> + */

I am wondering about the motivation for dual-licensing this file.
It does not seem to be something Renesas has done before with
upstream DT.

I am also wondering if the dual licence, if it remains, can be
described using SPDX.

> +
> +/dts-v1/;
> +#include "r7s9210.dtsi"
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/pinctrl/r7s9210-pinctrl.h>
> +
> +/ {
> +	model = "RZA2MEVB";
> +	compatible = "renesas,rza2mevb", "renesas,r7s9210";
> +
> +	aliases {
> +		serial0 = &scif4;
> +	};
> +
> +	chosen {
> +		bootargs = "ignore_loglevel earlycon rootfstype=cramfs root=mtd:rootfs";
> +		stdout-path = "serial0:115200n8";
> +	};
> +
> +	memory@80000000 {
> +		device_type = "memory";
> +		reg = <0x40000000 0x00800000>;	 /* HyperRAM */
> +	};
> +
> +	lbsc {
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +	};
> +
> +	leds {
> +		status = "okay";
> +		compatible = "gpio-leds";
> +
> +		led0 {
> +			gpios = <&pinctrl RZA2_PIN(PORT6, 0) GPIO_ACTIVE_HIGH>;
> +		};
> +	};
> +
> +	/* Cramfs XIP File System in QSPI */
> +	qspi@20000000 {
> +		compatible = "mtd-rom";
> +		probe-type = "direct-mapped";	/* XIP from QSPI */
> +		reg = <0x20000000 0x4000000>;	/* 64 MB*/
> +		bank-width = <4>;
> +		device-width = <1>;
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		partition@800000 {
> +			label ="rootfs";
> +			reg = <0x0800000 0x1000000>; /* 16MB @ 0x20800000 */
> +			read-only;
> +		};
> +	};
> +};
> +
> +/* EXTAL */
> +&extal_clk {
> +	clock-frequency = <24000000>;	/* 24MHz */
> +};
> +
> +/* RTC_X1 */
> +&rtc_x1_clk {
> +	clock-frequency = <32768>;
> +};
> +
> +&pinctrl {
> +
> +	/* Serial Console */
> +	scif4_pins: serial4 {
> +		pinmux = <RZA2_PINMUX(PORT9, 0, 4)>,	/* TxD4 */
> +			 <RZA2_PINMUX(PORT9, 1, 4)>;	/* RxD4 */
> +	};
> +};
> +
> +/* High resolution System tick timers */
> +&ostm0 {
> +	status = "okay";
> +};
> +
> +&ostm1 {
> +	status = "okay";
> +};
> +
> +/* Serial Console */
> +&scif4 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&scif4_pins>;
> +
> +	status = "okay";
> +};
> -- 
> 2.16.1
> 

RE: [PATCH 2/2] ARM: dts: r7s9210-rza2mevb: Add support for RZ/A2M EVB

[Chris Brandt]

Hi Simon,

On Friday, November 30, 2018, Simon Horman wrote:
> > +  - RZ/A2M Eval Board (RTK7921053S00000BE)
> > +    compatible = "renesas,rza2mevb", "renesas,r7s9210"
> >    - RZN1D-DB (RZ/N1D Demo Board for the RZ/N1D 400 pins package)
> >      compatible = "renesas,rzn1d400-db", "renesas,r9a06g032"
> >    - Salvator-X (RTP0RC7795SIPB0010S)
> 
> Pleas separate the change above into a separate patch.
> Such changes typically go upstream via a separate branch.

OK. Looking through the history log, I saw it different ways, so I was 
not sure.


> I am wondering about the motivation for dual-licensing this file.
> It does not seem to be something Renesas has done before with
> upstream DT.
> 
> I am also wondering if the dual licence, if it remains, can be
> described using SPDX.

A while back, I was reading/hearing about how board DT file do not have 
to be GPL and the user should not have to be forced to make this GPL. 
(Maybe at some ELC conference or on LWN or something)
So in our RZ/A BSP that I release to customers I would use this dual 
license. You can see the exact same license in a number of dts files in 
mainline.

Since there is no SPDX for this, I figure Rob might have some opinion on
the matter.
And, if I have to make it GLP for the mainline version, then I will just
replace the license when I release the customer BSP (since I wrote it, 
I can do that). But, I would be nice to keep the BSP version as close to
mainline as I can.

Chris

Re: [PATCH 2/2] ARM: dts: r7s9210-rza2mevb: Add support for RZ/A2M EVB

[Geert Uytterhoeven]

Hi Chris,

On Fri, Nov 30, 2018 at 1:20 PM Chris Brandt <Chris.Brandt@renesas.com> wrote:
> On Friday, November 30, 2018, Simon Horman wrote:
> > I am wondering about the motivation for dual-licensing this file.
> > It does not seem to be something Renesas has done before with
> > upstream DT.
> >
> > I am also wondering if the dual licence, if it remains, can be
> > described using SPDX.
>
> A while back, I was reading/hearing about how board DT file do not have
> to be GPL and the user should not have to be forced to make this GPL.
> (Maybe at some ELC conference or on LWN or something)

Indeed. Recently there have been some attempts to fix this.

> So in our RZ/A BSP that I release to customers I would use this dual
> license. You can see the exact same license in a number of dts files in
> mainline.

Note that your file includes

    #include <dt-bindings/gpio/gpio.h>
    #include <dt-bindings/pinctrl/r7s9210-pinctrl.h>

both of which are

   include/dt-bindings/gpio/gpio.h:/* SPDX-License-Identifier: GPL-2.0 */
   include/dt-bindings/pinctrl/r7s9210-pinctrl.h:/*
SPDX-License-Identifier: GPL-2.0 */

> Since there is no SPDX for this, I figure Rob might have some opinion on
> the matter.
> And, if I have to make it GLP for the mainline version, then I will just
> replace the license when I release the customer BSP (since I wrote it,
> I can do that). But, I would be nice to keep the BSP version as close to
> mainline as I can.

Documentation/process/license-rules.rst does have SPDX identifiers for
dual-licensed files.  Perhaps they match the license of your file?

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 2/2] ARM: dts: r7s9210-rza2mevb: Add support for RZ/A2M EVB

[Chris Brandt]

Hi Geert,

On Friday, November 30, 2018 1, Geert wrote:
> > So in our RZ/A BSP that I release to customers I would use this dual
> > license. You can see the exact same license in a number of dts files in
> > mainline.
> 
> Note that your file includes
> 
>     #include <dt-bindings/gpio/gpio.h>
>     #include <dt-bindings/pinctrl/r7s9210-pinctrl.h>
> 
> both of which are
> 
>    include/dt-bindings/gpio/gpio.h:/* SPDX-License-Identifier: GPL-2.0 */
>    include/dt-bindings/pinctrl/r7s9210-pinctrl.h:/*
> SPDX-License-Identifier: GPL-2.0 */

After I wrote that, I realized that we'll always have:
	#include "r7s9210.dtsi"
Which I have as GPL-2.0....so the dual license idea in pointless.

So to make things easy, I'll just drop the dual license thing and just 
use SPDX GPL-2.0


Chris

Re: [PATCH 2/2] ARM: dts: r7s9210-rza2mevb: Add support for RZ/A2M EVB

[Geert Uytterhoeven]

Hi Chris,

On Fri, Nov 30, 2018 at 5:10 PM Chris Brandt <Chris.Brandt@renesas.com> wrote:
> On Friday, November 30, 2018 1, Geert wrote:
> > > So in our RZ/A BSP that I release to customers I would use this dual
> > > license. You can see the exact same license in a number of dts files in
> > > mainline.
> >
> > Note that your file includes
> >
> >     #include <dt-bindings/gpio/gpio.h>
> >     #include <dt-bindings/pinctrl/r7s9210-pinctrl.h>
> >
> > both of which are
> >
> >    include/dt-bindings/gpio/gpio.h:/* SPDX-License-Identifier: GPL-2.0 */
> >    include/dt-bindings/pinctrl/r7s9210-pinctrl.h:/*
> > SPDX-License-Identifier: GPL-2.0 */
>
> After I wrote that, I realized that we'll always have:
>         #include "r7s9210.dtsi"
> Which I have as GPL-2.0....so the dual license idea in pointless.
>
> So to make things easy, I'll just drop the dual license thing and just
> use SPDX GPL-2.0

That's one solution.

Another solution would be to relicense all DT binding definitions and DTS files.
Cfr. e.g. commit d061864b89c3234b ("ARM: dt: relicense two DT binding IRQ
headers").

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 2/2] ARM: dts: r7s9210-rza2mevb: Add support for RZ/A2M EVB

[Geert Uytterhoeven]

Hi Chris,

On Thu, Nov 29, 2018 at 2:07 PM Chris Brandt <chris.brandt@renesas.com> wrote:
> Add support for Renesas RZ/A2M evaluation board.
>
> Signed-off-by: Chris Brandt <chris.brandt@renesas.com>

Thanks for your patch!

> index 000000000000..9570aeb8f1ce
> --- /dev/null
> +++ b/arch/arm/boot/dts/r7s9210-rza2mevb.dts
> @@ -0,0 +1,133 @@
> +/*
> + * Device Tree Source for the RZA2MEVB board
> + *
> + * Copyright (C) 2018 Renesas Electronics
> + *
> + * This file is dual-licensed: you can use it either under the terms
> + * of the GPL or the X11 license, at your option. Note that this dual
> + * licensing only applies to this file, and not this project as a
> + * whole.

Let's see about the license...

> +/ {
> +       model = "RZA2MEVB";
> +       compatible = "renesas,rza2mevb", "renesas,r7s9210";
> +
> +       aliases {
> +               serial0 = &scif4;
> +       };
> +
> +       chosen {
> +               bootargs = "ignore_loglevel earlycon rootfstype=cramfs root=mtd:rootfs";

I wouldn't put "earlycon" in the default bootargs.
It's meant for debugging.

> +               stdout-path = "serial0:115200n8";
> +       };
> +
> +       memory@80000000 {

40000000

> +               device_type = "memory";
> +               reg = <0x40000000 0x00800000>;   /* HyperRAM */
> +       };
> +
> +       lbsc {
> +               #address-cells = <1>;
> +               #size-cells = <1>;
> +       };
> +
> +       leds {
> +               status = "okay";

Please drop this line, "okay" is the default.

> +               compatible = "gpio-leds";
> +
> +               led0 {
> +                       gpios = <&pinctrl RZA2_PIN(PORT6, 0) GPIO_ACTIVE_HIGH>;

There's a second (yellow green) LED connected to PC_1.
Hence

        red {
                gpios = <&pinctrl RZA2_PIN(PORT6, 0) GPIO_ACTIVE_HIGH>;
        };
        green {
                gpios = <&pinctrl RZA2_PIN(PORTC, 1) GPIO_ACTIVE_HIGH>;
        }

> +               };
> +       };

> +&pinctrl {
> +

Please drop this blank line.

> +       /* Serial Console */
> +       scif4_pins: serial4 {
> +               pinmux = <RZA2_PINMUX(PORT9, 0, 4)>,    /* TxD4 */
> +                        <RZA2_PINMUX(PORT9, 1, 4)>;    /* RxD4 */
> +       };
> +};

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 2/2] ARM: dts: r7s9210-rza2mevb: Add support for RZ/A2M EVB

[Chris Brandt]

Hi Geert,

Thanks for your review.

On Tuesday, December 04, 2018 1, Geert Uytterhoeven wrote:
> > + * This file is dual-licensed: you can use it either under the terms
> > + * of the GPL or the X11 license, at your option. Note that this dual
> > + * licensing only applies to this file, and not this project as a
> > + * whole.
> 
> Let's see about the license...

I did find that I could use "SPDX-License-Identifier: (GPL-2.0+ OR MIT)",
but, for the mainline version I will probably just make it GPL. The
customer BSP version is going to be different anyway so I'll make that one
a dual license.

Besides, this board will only boot as XIP_KERNEL which is an impossible 
configuration in the mainline kernel, so this .dts file is mostly for 
checking the .dtsi file.

Thanks,
Chris

RE: [PATCH 2/2] ARM: dts: r7s9210-rza2mevb: Add support for RZ/A2M EVB

[Chris Brandt]

Hi Geert,

Thanks for your review.

On Tuesday, December 04, 2018 1, Geert Uytterhoeven wrote:
> > + * This file is dual-licensed: you can use it either under the terms
> > + * of the GPL or the X11 license, at your option. Note that this dual
> > + * licensing only applies to this file, and not this project as a
> > + * whole.
> 
> Let's see about the license...

I did find that I could use "SPDX-License-Identifier: (GPL-2.0+ OR MIT)",
but, for the mainline version I will probably just make it GPL. The
customer BSP version is going to be different anyway so I'll make that one
a dual license.

Besides, this board will only boot as XIP_KERNEL which is an impossible 
configuration in the mainline kernel, so this .dts file is mostly for 
checking the .dtsi file.

Thanks,
Chris

Re: [PATCH 2/2] ARM: dts: r7s9210-rza2mevb: Add support for RZ/A2M EVB

[Geert Uytterhoeven]

Hi Chris,

On Tue, Dec 4, 2018 at 5:25 PM Chris Brandt <Chris.Brandt@renesas.com> wrote:
> Besides, this board will only boot as XIP_KERNEL which is an impossible
> configuration in the mainline kernel, so this .dts file is mostly for
> checking the .dtsi file.

Let's hope we can fix that anytime soon.

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 2/2] ARM: dts: r7s9210-rza2mevb: Add support for RZ/A2M EVB

[Rob Herring]

On Thu, Nov 29, 2018 at 08:05:59AM -0500, Chris Brandt wrote:
> Add support for Renesas RZ/A2M evaluation board.
> 
> Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
> ---
>  Documentation/devicetree/bindings/arm/shmobile.txt |   2 +
>  arch/arm/boot/dts/Makefile                         |   1 +
>  arch/arm/boot/dts/r7s9210-rza2mevb.dts             | 133 +++++++++++++++++++++
>  3 files changed, 136 insertions(+)
>  create mode 100644 arch/arm/boot/dts/r7s9210-rza2mevb.dts
> 
> diff --git a/Documentation/devicetree/bindings/arm/shmobile.txt b/Documentation/devicetree/bindings/arm/shmobile.txt
> index a94db59d8e2d..67a6810c4345 100644
> --- a/Documentation/devicetree/bindings/arm/shmobile.txt
> +++ b/Documentation/devicetree/bindings/arm/shmobile.txt
> @@ -125,6 +125,8 @@ Boards:
>      compatible = "renesas,porter", "renesas,r8a7791"
>    - RSKRZA1 (YR0K77210C000BE)
>      compatible = "renesas,rskrza1", "renesas,r7s72100"
> +  - RZ/A2M Eval Board (RTK7921053S00000BE)
> +    compatible = "renesas,rza2mevb", "renesas,r7s9210"
>    - RZN1D-DB (RZ/N1D Demo Board for the RZ/N1D 400 pins package)
>      compatible = "renesas,rzn1d400-db", "renesas,r9a06g032"
>    - Salvator-X (RTP0RC7795SIPB0010S)
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index b0e966d625b9..fbdd068fd8af 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -824,6 +824,7 @@ dtb-$(CONFIG_ARCH_RENESAS) += \
>  	r7s72100-genmai.dtb \
>  	r7s72100-gr-peach.dtb \
>  	r7s72100-rskrza1.dtb \
> +	r7s9210-rza2mevb.dtb \
>  	r8a73a4-ape6evm.dtb \
>  	r8a7740-armadillo800eva.dtb \
>  	r8a7743-iwg20d-q7.dtb \
> diff --git a/arch/arm/boot/dts/r7s9210-rza2mevb.dts b/arch/arm/boot/dts/r7s9210-rza2mevb.dts
> new file mode 100644
> index 000000000000..9570aeb8f1ce
> --- /dev/null
> +++ b/arch/arm/boot/dts/r7s9210-rza2mevb.dts
> @@ -0,0 +1,133 @@
> +/*
> + * Device Tree Source for the RZA2MEVB board
> + *
> + * Copyright (C) 2018 Renesas Electronics
> + *
> + * This file is dual-licensed: you can use it either under the terms
> + * of the GPL or the X11 license, at your option. Note that this dual
> + * licensing only applies to this file, and not this project as a
> + * whole.
> + *
> + *  a) This file is free software; you can redistribute it and/or
> + *     modify it under the terms of the GNU General Public License as
> + *     published by the Free Software Foundation; either version 2 of the
> + *     License, or (at your option) any later version.
> + *
> + *     This file is distributed in the hope that it will be useful,
> + *     but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *     MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *     GNU General Public License for more details.
> + *
> + * Or, alternatively,
> + *
> + *  b) Permission is hereby granted, free of charge, to any person
> + *     obtaining a copy of this software and associated documentation
> + *     files (the "Software"), to deal in the Software without
> + *     restriction, including without limitation the rights to use,
> + *     copy, modify, merge, publish, distribute, sublicense, and/or
> + *     sell copies of the Software, and to permit persons to whom the
> + *     Software is furnished to do so, subject to the following
> + *     conditions:
> + *
> + *     The above copyright notice and this permission notice shall be
> + *     included in all copies or substantial portions of the Software.
> + *
> + *     THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> + *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
> + *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> + *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
> + *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
> + *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> + *     OTHER DEALINGS IN THE SOFTWARE.
> + */
> +
> +/dts-v1/;
> +#include "r7s9210.dtsi"
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/pinctrl/r7s9210-pinctrl.h>
> +
> +/ {
> +	model = "RZA2MEVB";
> +	compatible = "renesas,rza2mevb", "renesas,r7s9210";
> +
> +	aliases {
> +		serial0 = &scif4;
> +	};
> +
> +	chosen {
> +		bootargs = "ignore_loglevel earlycon rootfstype=cramfs root=mtd:rootfs";
> +		stdout-path = "serial0:115200n8";
> +	};
> +
> +	memory@80000000 {

Wrong unit-address.

> +		device_type = "memory";
> +		reg = <0x40000000 0x00800000>;	 /* HyperRAM */
> +	};
> +
> +	lbsc {
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +	};
> +
> +	leds {
> +		status = "okay";

gpio-leds shouldn't need this as it shouldn't be in the SoC file.

> +		compatible = "gpio-leds";
> +
> +		led0 {
> +			gpios = <&pinctrl RZA2_PIN(PORT6, 0) GPIO_ACTIVE_HIGH>;
> +		};
> +	};
> +
> +	/* Cramfs XIP File System in QSPI */
> +	qspi@20000000 {


> +		compatible = "mtd-rom";

This should be the actual Quad-SPI controller and then a flash device 
underneath it. It may work as-is initially, but eventually won't you 
need the flash details?

> +		probe-type = "direct-mapped";	/* XIP from QSPI */
> +		reg = <0x20000000 0x4000000>;	/* 64 MB*/
> +		bank-width = <4>;
> +		device-width = <1>;
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		partition@800000 {

Best practice is grouping under a 'partitions' node.

> +			label ="rootfs";
> +			reg = <0x0800000 0x1000000>; /* 16MB @ 0x20800000 */
> +			read-only;
> +		};
> +	};

RE: [PATCH 2/2] ARM: dts: r7s9210-rza2mevb: Add support for RZ/A2M EVB

[Chris Brandt]

On Tuesday, December 11, 2018, Rob Herring wrote:
> > +	/* Cramfs XIP File System in QSPI */
> > +	qspi@20000000 {
> 
> 
> > +		compatible = "mtd-rom";
> 
> This should be the actual Quad-SPI controller and then a flash device
> underneath it. It may work as-is initially, but eventually won't you
> need the flash details?

Not really.
The "QSPI" actually works as a memory mapped SPI (as in, the CPU sees 
the QSPI flash as a linear read-only address range).
Basically, u-boot sets up the QSPI to look like a ROM area, and that's 
it. Everything is running directly from that QSPI flash, so you can't 
make any modifications to it while running, otherwise the whole system will
go down. Therefore, there is no need to know anything about the actually
Flash that is connected, because you are never allowed to interact with it
(or even query it on boot).

This board in particular only has 8MB of RAM. So, it really only works 
well as an XIP system (meaning both kernel and file system are XIP from 
Flash).


Chris

RE: [PATCH 2/2] ARM: dts: r7s9210-rza2mevb: Add support for RZ/A2M EVB

[Chris Brandt]

On Tuesday, December 11, 2018, Rob Herring wrote:
> > +	/* Cramfs XIP File System in QSPI */
> > +	qspi@20000000 {
> 
> 
> > +		compatible = "mtd-rom";
> 
> This should be the actual Quad-SPI controller and then a flash device
> underneath it. It may work as-is initially, but eventually won't you
> need the flash details?

Not really.
The "QSPI" actually works as a memory mapped SPI (as in, the CPU sees 
the QSPI flash as a linear read-only address range).
Basically, u-boot sets up the QSPI to look like a ROM area, and that's 
it. Everything is running directly from that QSPI flash, so you can't 
make any modifications to it while running, otherwise the whole system will
go down. Therefore, there is no need to know anything about the actually
Flash that is connected, because you are never allowed to interact with it
(or even query it on boot).

This board in particular only has 8MB of RAM. So, it really only works 
well as an XIP system (meaning both kernel and file system are XIP from 
Flash).


Chris

Re: [PATCH 2/2] ARM: dts: r7s9210-rza2mevb: Add support for RZ/A2M EVB

[Rob Herring]

+Kumar who was just asking me about this exact problem.

On Wed, Dec 12, 2018 at 7:59 AM Chris Brandt <Chris.Brandt@renesas.com> wrote:
>
> On Tuesday, December 11, 2018, Rob Herring wrote:
> > > +   /* Cramfs XIP File System in QSPI */
> > > +   qspi@20000000 {
> >
> >
> > > +           compatible = "mtd-rom";
> >
> > This should be the actual Quad-SPI controller and then a flash device
> > underneath it. It may work as-is initially, but eventually won't you
> > need the flash details?
>
> Not really.
> The "QSPI" actually works as a memory mapped SPI (as in, the CPU sees
> the QSPI flash as a linear read-only address range).

Yes, those controllers are becoming pretty standard.

> Basically, u-boot sets up the QSPI to look like a ROM area, and that's
> it. Everything is running directly from that QSPI flash, so you can't
> make any modifications to it while running, otherwise the whole system will
> go down. Therefore, there is no need to know anything about the actually
> Flash that is connected, because you are never allowed to interact with it
> (or even query it on boot).

Okay, but these days u-boot uses DT itself. So how does u-boot know
how to set things up? It's going to need to know what controller and
flash details. Or how does one do a flash update (in the field)?

I think it would be better to have a property to say the flash is
already setup and just use it. Or maybe it can be done with
compatibles:

spi@1234 {
  compatible = "vendor,soc-quadspi", "mmio-spi";
  flash@0 {
    compatible = "some-flash-part", "mtd-rom";
  };
};

Now the problem is how to define the address range as SPI flash
devices are already defined to use SPI chip-select numbers. The
easiest option from a DT standpoint is just look at the parent reg
property and figure out the memory range (typically there are 2 and
you want the big one). Or you could have some simple driver that knows
which reg range to get for specific compatibles.

Rob

Re: [PATCH 2/2] ARM: dts: r7s9210-rza2mevb: Add support for RZ/A2M EVB

[Rob Herring]

+Kumar who was just asking me about this exact problem.

On Wed, Dec 12, 2018 at 7:59 AM Chris Brandt <Chris.Brandt@renesas.com> wrote:
>
> On Tuesday, December 11, 2018, Rob Herring wrote:
> > > +   /* Cramfs XIP File System in QSPI */
> > > +   qspi@20000000 {
> >
> >
> > > +           compatible = "mtd-rom";
> >
> > This should be the actual Quad-SPI controller and then a flash device
> > underneath it. It may work as-is initially, but eventually won't you
> > need the flash details?
>
> Not really.
> The "QSPI" actually works as a memory mapped SPI (as in, the CPU sees
> the QSPI flash as a linear read-only address range).

Yes, those controllers are becoming pretty standard.

> Basically, u-boot sets up the QSPI to look like a ROM area, and that's
> it. Everything is running directly from that QSPI flash, so you can't
> make any modifications to it while running, otherwise the whole system will
> go down. Therefore, there is no need to know anything about the actually
> Flash that is connected, because you are never allowed to interact with it
> (or even query it on boot).

Okay, but these days u-boot uses DT itself. So how does u-boot know
how to set things up? It's going to need to know what controller and
flash details. Or how does one do a flash update (in the field)?

I think it would be better to have a property to say the flash is
already setup and just use it. Or maybe it can be done with
compatibles:

spi@1234 {
  compatible = "vendor,soc-quadspi", "mmio-spi";
  flash@0 {
    compatible = "some-flash-part", "mtd-rom";
  };
};

Now the problem is how to define the address range as SPI flash
devices are already defined to use SPI chip-select numbers. The
easiest option from a DT standpoint is just look at the parent reg
property and figure out the memory range (typically there are 2 and
you want the big one). Or you could have some simple driver that knows
which reg range to get for specific compatibles.

Rob

RE: [PATCH 2/2] ARM: dts: r7s9210-rza2mevb: Add support for RZ/A2M EVB

[Chris Brandt]

On Wednesday, December 12, 2018 1, Rob Herring wrote:
> > Not really.
> > The "QSPI" actually works as a memory mapped SPI (as in, the CPU sees
> > the QSPI flash as a linear read-only address range).
> 
> Yes, those controllers are becoming pretty standard.

Ya, my SoCs are not so special any more :(

But, XIP_KERNEL does not seem to be mainstream either.


> > Therefore, there is no need to know anything about the actually
> > Flash that is connected, because you are never allowed to interact with
> it
> > (or even query it on boot).
> 
> Okay, but these days u-boot uses DT itself. So how does u-boot know
> how to set things up? It's going to need to know what controller and
> flash details.

That's all hand coded at the moment. As these Quad-SPI chips came on the
market, each vendor had their own way to get in (and out) of these 
modes, and even the protocols were not the same. Sometimes even the same 
flash vendor would do things different form chip to chip. At this point, 
(at least for Renesas) I have u-boot code that at least points you in the 
right direction depending on what flash vendor you go with....but I 
can't guarantee you won't need to do some tweaking.


> Or how does one do a flash update (in the field)?
Well....for that there's a "special" driver that I wrote. Basically, 
going back to the ways we've done it with MCU for years: Copy code to RAM, 
shut off interrupts, jump to RAM, do your business, jump back to flash.

Again, tweaking is needed depending on what SPI flash you use, so there 
is no magic 'one driver fits all'.


> I think it would be better to have a property to say the flash is
> already setup and just use it. Or maybe it can be done with
> compatibles:
> 
> spi@1234 {
>   compatible = "vendor,soc-quadspi", "mmio-spi";
>   flash@0 {
>     compatible = "some-flash-part", "mtd-rom";
>   };
> };
> 
> Now the problem is how to define the address range as SPI flash
> devices are already defined to use SPI chip-select numbers. The
> easiest option from a DT standpoint is just look at the parent reg
> property and figure out the memory range (typically there are 2 and
> you want the big one). Or you could have some simple driver that knows
> which reg range to get for specific compatibles.


I'm still confused on why I would want to do this as I want the kernel 
to not know nothing about the underlying HW so it never tries to access
it any way.
At the end of the day, all I want is for the kernel to do an ioremap of 
this area for me so my file system (XIP cramfs in this case) can access 
it. "mtd-rom" does this for me today.

Just FYI, early in boot, the kernel already mapped part of my QSPI into 
virtual memory since we're most likely running an XIP_KERNEL.

While these XIP Linux systems seem to function quiet well, I might need 
to continue to hide some of the inner details from mainline because they
don't always fit into the direction of the kernel systems (ie, monster 
quad core systems with lots of DDR memory).

The other XIP file system that we use more often (AXFS, not in mainline)
does not require the partitions in DT at all, so it's really only 
needed when using cramfs. So for now, maybe it would be better to just remove
this confusing portion of the dts.


Chris

RE: [PATCH 2/2] ARM: dts: r7s9210-rza2mevb: Add support for RZ/A2M EVB

[Chris Brandt]

On Wednesday, December 12, 2018 1, Rob Herring wrote:
> > Not really.
> > The "QSPI" actually works as a memory mapped SPI (as in, the CPU sees
> > the QSPI flash as a linear read-only address range).
> 
> Yes, those controllers are becoming pretty standard.

Ya, my SoCs are not so special any more :(

But, XIP_KERNEL does not seem to be mainstream either.


> > Therefore, there is no need to know anything about the actually
> > Flash that is connected, because you are never allowed to interact with
> it
> > (or even query it on boot).
> 
> Okay, but these days u-boot uses DT itself. So how does u-boot know
> how to set things up? It's going to need to know what controller and
> flash details.

That's all hand coded at the moment. As these Quad-SPI chips came on the
market, each vendor had their own way to get in (and out) of these 
modes, and even the protocols were not the same. Sometimes even the same 
flash vendor would do things different form chip to chip. At this point, 
(at least for Renesas) I have u-boot code that at least points you in the 
right direction depending on what flash vendor you go with....but I 
can't guarantee you won't need to do some tweaking.


> Or how does one do a flash update (in the field)?
Well....for that there's a "special" driver that I wrote. Basically, 
going back to the ways we've done it with MCU for years: Copy code to RAM, 
shut off interrupts, jump to RAM, do your business, jump back to flash.

Again, tweaking is needed depending on what SPI flash you use, so there 
is no magic 'one driver fits all'.


> I think it would be better to have a property to say the flash is
> already setup and just use it. Or maybe it can be done with
> compatibles:
> 
> spi@1234 {
>   compatible = "vendor,soc-quadspi", "mmio-spi";
>   flash@0 {
>     compatible = "some-flash-part", "mtd-rom";
>   };
> };
> 
> Now the problem is how to define the address range as SPI flash
> devices are already defined to use SPI chip-select numbers. The
> easiest option from a DT standpoint is just look at the parent reg
> property and figure out the memory range (typically there are 2 and
> you want the big one). Or you could have some simple driver that knows
> which reg range to get for specific compatibles.


I'm still confused on why I would want to do this as I want the kernel 
to not know nothing about the underlying HW so it never tries to access
it any way.
At the end of the day, all I want is for the kernel to do an ioremap of 
this area for me so my file system (XIP cramfs in this case) can access 
it. "mtd-rom" does this for me today.

Just FYI, early in boot, the kernel already mapped part of my QSPI into 
virtual memory since we're most likely running an XIP_KERNEL.

While these XIP Linux systems seem to function quiet well, I might need 
to continue to hide some of the inner details from mainline because they
don't always fit into the direction of the kernel systems (ie, monster 
quad core systems with lots of DDR memory).

The other XIP file system that we use more often (AXFS, not in mainline)
does not require the partitions in DT at all, so it's really only 
needed when using cramfs. So for now, maybe it would be better to just remove
this confusing portion of the dts.


Chris

Re: [PATCH 2/2] ARM: dts: r7s9210-rza2mevb: Add support for RZ/A2M EVB

[Rob Herring]

On Wed, Dec 12, 2018 at 12:03 PM Chris Brandt <Chris.Brandt@renesas.com> wrote:
>
> On Wednesday, December 12, 2018 1, Rob Herring wrote:
> > > Not really.
> > > The "QSPI" actually works as a memory mapped SPI (as in, the CPU sees
> > > the QSPI flash as a linear read-only address range).
> >
> > Yes, those controllers are becoming pretty standard.
>
> Ya, my SoCs are not so special any more :(
>
> But, XIP_KERNEL does not seem to be mainstream either.

Part of the problem there is XIP_KERNEL doesn't work with
multi-platform. I think that is mainly due to the dependency on
run-time phys2virt patching. We should fix that such that either we
can set a default phys_offset or possibly we could do some offline
patching.

> > > Therefore, there is no need to know anything about the actually
> > > Flash that is connected, because you are never allowed to interact with
> > it
> > > (or even query it on boot).
> >
> > Okay, but these days u-boot uses DT itself. So how does u-boot know
> > how to set things up? It's going to need to know what controller and
> > flash details.
>
> That's all hand coded at the moment. As these Quad-SPI chips came on the
> market, each vendor had their own way to get in (and out) of these
> modes, and even the protocols were not the same. Sometimes even the same
> flash vendor would do things different form chip to chip. At this point,
> (at least for Renesas) I have u-boot code that at least points you in the
> right direction depending on what flash vendor you go with....but I
> can't guarantee you won't need to do some tweaking.

Seems like DT would help you there...

> > Or how does one do a flash update (in the field)?
> Well....for that there's a "special" driver that I wrote. Basically,
> going back to the ways we've done it with MCU for years: Copy code to RAM,
> shut off interrupts, jump to RAM, do your business, jump back to flash.
>
> Again, tweaking is needed depending on what SPI flash you use, so there
> is no magic 'one driver fits all'.
>
>
> > I think it would be better to have a property to say the flash is
> > already setup and just use it. Or maybe it can be done with
> > compatibles:
> >
> > spi@1234 {
> >   compatible = "vendor,soc-quadspi", "mmio-spi";
> >   flash@0 {
> >     compatible = "some-flash-part", "mtd-rom";
> >   };
> > };
> >
> > Now the problem is how to define the address range as SPI flash
> > devices are already defined to use SPI chip-select numbers. The
> > easiest option from a DT standpoint is just look at the parent reg
> > property and figure out the memory range (typically there are 2 and
> > you want the big one). Or you could have some simple driver that knows
> > which reg range to get for specific compatibles.
>
>
> I'm still confused on why I would want to do this as I want the kernel
> to not know nothing about the underlying HW so it never tries to access
> it any way.
> At the end of the day, all I want is for the kernel to do an ioremap of
> this area for me so my file system (XIP cramfs in this case) can access
> it. "mtd-rom" does this for me today.

And partition layout? If it was only what to map, we could add a
property in chosen or perhaps a reserved-memory node. But I doubt it
will remain that simple. What happens when you have clock management
and need to keep the SPI clock enabled for example.

The other option is just have your platform code instantiate an
mtd-rom device. Either describe the h/w with DT or don't use DT.

> Just FYI, early in boot, the kernel already mapped part of my QSPI into
> virtual memory since we're most likely running an XIP_KERNEL.
>
> While these XIP Linux systems seem to function quiet well, I might need
> to continue to hide some of the inner details from mainline because they
> don't always fit into the direction of the kernel systems (ie, monster
> quad core systems with lots of DDR memory).
>
> The other XIP file system that we use more often (AXFS, not in mainline)
> does not require the partitions in DT at all, so it's really only
> needed when using cramfs. So for now, maybe it would be better to just remove
> this confusing portion of the dts.

What does AXFS have that cramfs doesn't making it not need partitions?

Rob

Re: [PATCH 2/2] ARM: dts: r7s9210-rza2mevb: Add support for RZ/A2M EVB

[Rob Herring]

On Wed, Dec 12, 2018 at 12:03 PM Chris Brandt <Chris.Brandt@renesas.com> wrote:
>
> On Wednesday, December 12, 2018 1, Rob Herring wrote:
> > > Not really.
> > > The "QSPI" actually works as a memory mapped SPI (as in, the CPU sees
> > > the QSPI flash as a linear read-only address range).
> >
> > Yes, those controllers are becoming pretty standard.
>
> Ya, my SoCs are not so special any more :(
>
> But, XIP_KERNEL does not seem to be mainstream either.

Part of the problem there is XIP_KERNEL doesn't work with
multi-platform. I think that is mainly due to the dependency on
run-time phys2virt patching. We should fix that such that either we
can set a default phys_offset or possibly we could do some offline
patching.

> > > Therefore, there is no need to know anything about the actually
> > > Flash that is connected, because you are never allowed to interact with
> > it
> > > (or even query it on boot).
> >
> > Okay, but these days u-boot uses DT itself. So how does u-boot know
> > how to set things up? It's going to need to know what controller and
> > flash details.
>
> That's all hand coded at the moment. As these Quad-SPI chips came on the
> market, each vendor had their own way to get in (and out) of these
> modes, and even the protocols were not the same. Sometimes even the same
> flash vendor would do things different form chip to chip. At this point,
> (at least for Renesas) I have u-boot code that at least points you in the
> right direction depending on what flash vendor you go with....but I
> can't guarantee you won't need to do some tweaking.

Seems like DT would help you there...

> > Or how does one do a flash update (in the field)?
> Well....for that there's a "special" driver that I wrote. Basically,
> going back to the ways we've done it with MCU for years: Copy code to RAM,
> shut off interrupts, jump to RAM, do your business, jump back to flash.
>
> Again, tweaking is needed depending on what SPI flash you use, so there
> is no magic 'one driver fits all'.
>
>
> > I think it would be better to have a property to say the flash is
> > already setup and just use it. Or maybe it can be done with
> > compatibles:
> >
> > spi@1234 {
> >   compatible = "vendor,soc-quadspi", "mmio-spi";
> >   flash@0 {
> >     compatible = "some-flash-part", "mtd-rom";
> >   };
> > };
> >
> > Now the problem is how to define the address range as SPI flash
> > devices are already defined to use SPI chip-select numbers. The
> > easiest option from a DT standpoint is just look at the parent reg
> > property and figure out the memory range (typically there are 2 and
> > you want the big one). Or you could have some simple driver that knows
> > which reg range to get for specific compatibles.
>
>
> I'm still confused on why I would want to do this as I want the kernel
> to not know nothing about the underlying HW so it never tries to access
> it any way.
> At the end of the day, all I want is for the kernel to do an ioremap of
> this area for me so my file system (XIP cramfs in this case) can access
> it. "mtd-rom" does this for me today.

And partition layout? If it was only what to map, we could add a
property in chosen or perhaps a reserved-memory node. But I doubt it
will remain that simple. What happens when you have clock management
and need to keep the SPI clock enabled for example.

The other option is just have your platform code instantiate an
mtd-rom device. Either describe the h/w with DT or don't use DT.

> Just FYI, early in boot, the kernel already mapped part of my QSPI into
> virtual memory since we're most likely running an XIP_KERNEL.
>
> While these XIP Linux systems seem to function quiet well, I might need
> to continue to hide some of the inner details from mainline because they
> don't always fit into the direction of the kernel systems (ie, monster
> quad core systems with lots of DDR memory).
>
> The other XIP file system that we use more often (AXFS, not in mainline)
> does not require the partitions in DT at all, so it's really only
> needed when using cramfs. So for now, maybe it would be better to just remove
> this confusing portion of the dts.

What does AXFS have that cramfs doesn't making it not need partitions?

Rob

RE: [PATCH 2/2] ARM: dts: r7s9210-rza2mevb: Add support for RZ/A2M EVB

[Chris Brandt]

On Monday, December 17, 2018 1, Rob Herring wrote:
> > But, XIP_KERNEL does not seem to be mainstream either.
> 
> Part of the problem there is XIP_KERNEL doesn't work with
> multi-platform. I think that is mainly due to the dependency on
> run-time phys2virt patching. We should fix that such that either we
> can set a default phys_offset or possibly we could do some offline
> patching.

Not really. I fixed all of that back in 4.6.

The issue is more of the 'theoretical' kind.

The ARM maintainer defines "multiplatform" as being able to build 1 
binary image that can be run on multiple SoCs.
But...that won't work when you have to define the ROM/RAM area at 
compile time.

I prefer to think of "multiplatform" as a way of grouping like SoCs 
together (if you need to) and use DT to sort out any differences.

Basically, I just need this simple patch in order to build and boot so I
can test kernel releases:

[arch/arm/Kconfig]

config XIP_KERNEL
 	bool "Kernel Execute-In-Place from ROM"
-	depends on !ARM_LPAE && !ARCH_MULTIPLATFORM
+	depends on !ARM_LPAE

However...I've given up on trying to get XIP_KERNEL enabled in Kconfig. 
But, of course still keep an eye on all the 'code' in the ARM core to 
make sure no one breaks it.



> > At the end of the day, all I want is for the kernel to do an ioremap of
> > this area for me so my file system (XIP cramfs in this case) can access
> > it. "mtd-rom" does this for me today.
> 
> And partition layout? If it was only what to map, we could add a
> property in chosen or perhaps a reserved-memory node. But I doubt it
> will remain that simple. What happens when you have clock management
> and need to keep the SPI clock enabled for example.

Yup...there's the killer!
You'll notice that in mainline RZ/A devices, there is no QSPI driver, 
and also the QSPI clock is left out of the list of HW clocks. Otherwise, 
as you mentioned, that 'unused' clock would get shut off at the end of 
boot...and you're whole system would stop immediately.

> The other option is just have your platform code instantiate an
> mtd-rom device. Either describe the h/w with DT or don't use DT.

You mean in a 'board' file?

BTW, I still use board files for things that are too complicated for DT.


> > The other XIP file system that we use more often (AXFS, not in mainline)
> > does not require the partitions in DT at all, so it's really only
> > needed when using cramfs. So for now, maybe it would be better to just
> remove
> > this confusing portion of the dts.
> 
> What does AXFS have that cramfs doesn't making it not need partitions?

For AXFS, you pass in the physical address to your rootfs on the kernel 
command line and then AXFS does the ioreapping itself inside the driver.

When Nicolas Pitre started adding XIP mode to cramfs last year, he also 
started out that way. But, that was rejected (as it was also rejected 
when DAX was first started). The file system people said 'file system 
drives should not have the ability to map physical memory themselves'.

Another thing AXFS can do that Cramfs can't do (yet), is AXFS can within
*each file* select what pages will be XIP, and what ones will be 
compressed. This saves a lot on Flash space when only your executable and 
const pages are left as uncompressed XIP pages.


Chris

RE: [PATCH 2/2] ARM: dts: r7s9210-rza2mevb: Add support for RZ/A2M EVB

[Chris Brandt]

On Monday, December 17, 2018 1, Rob Herring wrote:
> > But, XIP_KERNEL does not seem to be mainstream either.
> 
> Part of the problem there is XIP_KERNEL doesn't work with
> multi-platform. I think that is mainly due to the dependency on
> run-time phys2virt patching. We should fix that such that either we
> can set a default phys_offset or possibly we could do some offline
> patching.

Not really. I fixed all of that back in 4.6.

The issue is more of the 'theoretical' kind.

The ARM maintainer defines "multiplatform" as being able to build 1 
binary image that can be run on multiple SoCs.
But...that won't work when you have to define the ROM/RAM area at 
compile time.

I prefer to think of "multiplatform" as a way of grouping like SoCs 
together (if you need to) and use DT to sort out any differences.

Basically, I just need this simple patch in order to build and boot so I
can test kernel releases:

[arch/arm/Kconfig]

config XIP_KERNEL
 	bool "Kernel Execute-In-Place from ROM"
-	depends on !ARM_LPAE && !ARCH_MULTIPLATFORM
+	depends on !ARM_LPAE

However...I've given up on trying to get XIP_KERNEL enabled in Kconfig. 
But, of course still keep an eye on all the 'code' in the ARM core to 
make sure no one breaks it.



> > At the end of the day, all I want is for the kernel to do an ioremap of
> > this area for me so my file system (XIP cramfs in this case) can access
> > it. "mtd-rom" does this for me today.
> 
> And partition layout? If it was only what to map, we could add a
> property in chosen or perhaps a reserved-memory node. But I doubt it
> will remain that simple. What happens when you have clock management
> and need to keep the SPI clock enabled for example.

Yup...there's the killer!
You'll notice that in mainline RZ/A devices, there is no QSPI driver, 
and also the QSPI clock is left out of the list of HW clocks. Otherwise, 
as you mentioned, that 'unused' clock would get shut off at the end of 
boot...and you're whole system would stop immediately.

> The other option is just have your platform code instantiate an
> mtd-rom device. Either describe the h/w with DT or don't use DT.

You mean in a 'board' file?

BTW, I still use board files for things that are too complicated for DT.


> > The other XIP file system that we use more often (AXFS, not in mainline)
> > does not require the partitions in DT at all, so it's really only
> > needed when using cramfs. So for now, maybe it would be better to just
> remove
> > this confusing portion of the dts.
> 
> What does AXFS have that cramfs doesn't making it not need partitions?

For AXFS, you pass in the physical address to your rootfs on the kernel 
command line and then AXFS does the ioreapping itself inside the driver.

When Nicolas Pitre started adding XIP mode to cramfs last year, he also 
started out that way. But, that was rejected (as it was also rejected 
when DAX was first started). The file system people said 'file system 
drives should not have the ability to map physical memory themselves'.

Another thing AXFS can do that Cramfs can't do (yet), is AXFS can within
*each file* select what pages will be XIP, and what ones will be 
compressed. This saves a lot on Flash space when only your executable and 
const pages are left as uncompressed XIP pages.


Chris

Re: [PATCH 2/2] ARM: dts: r7s9210-rza2mevb: Add support for RZ/A2M EVB

[Rob Herring]

On Mon, Dec 17, 2018 at 10:22 AM Chris Brandt <Chris.Brandt@renesas.com> wrote:
>
> On Monday, December 17, 2018 1, Rob Herring wrote:
> > > But, XIP_KERNEL does not seem to be mainstream either.
> >
> > Part of the problem there is XIP_KERNEL doesn't work with
> > multi-platform. I think that is mainly due to the dependency on
> > run-time phys2virt patching. We should fix that such that either we
> > can set a default phys_offset or possibly we could do some offline
> > patching.
>
> Not really. I fixed all of that back in 4.6.
>
> The issue is more of the 'theoretical' kind.
>
> The ARM maintainer defines "multiplatform" as being able to build 1
> binary image that can be run on multiple SoCs.
> But...that won't work when you have to define the ROM/RAM area at
> compile time.
>
> I prefer to think of "multiplatform" as a way of grouping like SoCs
> together (if you need to) and use DT to sort out any differences.

Yes, I'm aware of all that. I did a bunch of the work to enable it.

> Basically, I just need this simple patch in order to build and boot so I
> can test kernel releases:
>
> [arch/arm/Kconfig]
>
> config XIP_KERNEL
>         bool "Kernel Execute-In-Place from ROM"
> -       depends on !ARM_LPAE && !ARCH_MULTIPLATFORM
> +       depends on !ARM_LPAE

Okay, I guess it's not that hard to enable if one wants. Perhaps all

> However...I've given up on trying to get XIP_KERNEL enabled in Kconfig.
> But, of course still keep an eye on all the 'code' in the ARM core to
> make sure no one breaks it.


> > > At the end of the day, all I want is for the kernel to do an ioremap of
> > > this area for me so my file system (XIP cramfs in this case) can access
> > > it. "mtd-rom" does this for me today.
> >
> > And partition layout? If it was only what to map, we could add a
> > property in chosen or perhaps a reserved-memory node. But I doubt it
> > will remain that simple. What happens when you have clock management
> > and need to keep the SPI clock enabled for example.
>
> Yup...there's the killer!
> You'll notice that in mainline RZ/A devices, there is no QSPI driver,
> and also the QSPI clock is left out of the list of HW clocks. Otherwise,
> as you mentioned, that 'unused' clock would get shut off at the end of
> boot...and you're whole system would stop immediately.

If you had a simple driver with the binding as I proposed, then it
could do the ioremap and clock management. And you could also have
specific SPI and flash drivers for when you need to flashing without
changing the DT.

> > The other option is just have your platform code instantiate an
> > mtd-rom device. Either describe the h/w with DT or don't use DT.
>
> You mean in a 'board' file?

Yes.

> BTW, I still use board files for things that are too complicated for DT.

As you should.

> > > The other XIP file system that we use more often (AXFS, not in mainline)
> > > does not require the partitions in DT at all, so it's really only
> > > needed when using cramfs. So for now, maybe it would be better to just
> > remove
> > > this confusing portion of the dts.
> >
> > What does AXFS have that cramfs doesn't making it not need partitions?
>
> For AXFS, you pass in the physical address to your rootfs on the kernel
> command line and then AXFS does the ioreapping itself inside the driver.
>
> When Nicolas Pitre started adding XIP mode to cramfs last year, he also
> started out that way. But, that was rejected (as it was also rejected
> when DAX was first started). The file system people said 'file system
> drives should not have the ability to map physical memory themselves'.

Right, I remember that now.

> Another thing AXFS can do that Cramfs can't do (yet), is AXFS can within
> *each file* select what pages will be XIP, and what ones will be
> compressed. This saves a lot on Flash space when only your executable and
> const pages are left as uncompressed XIP pages.
>
>
> Chris
>

Re: [PATCH 2/2] ARM: dts: r7s9210-rza2mevb: Add support for RZ/A2M EVB

[Rob Herring]

On Mon, Dec 17, 2018 at 10:22 AM Chris Brandt <Chris.Brandt@renesas.com> wrote:
>
> On Monday, December 17, 2018 1, Rob Herring wrote:
> > > But, XIP_KERNEL does not seem to be mainstream either.
> >
> > Part of the problem there is XIP_KERNEL doesn't work with
> > multi-platform. I think that is mainly due to the dependency on
> > run-time phys2virt patching. We should fix that such that either we
> > can set a default phys_offset or possibly we could do some offline
> > patching.
>
> Not really. I fixed all of that back in 4.6.
>
> The issue is more of the 'theoretical' kind.
>
> The ARM maintainer defines "multiplatform" as being able to build 1
> binary image that can be run on multiple SoCs.
> But...that won't work when you have to define the ROM/RAM area at
> compile time.
>
> I prefer to think of "multiplatform" as a way of grouping like SoCs
> together (if you need to) and use DT to sort out any differences.

Yes, I'm aware of all that. I did a bunch of the work to enable it.

> Basically, I just need this simple patch in order to build and boot so I
> can test kernel releases:
>
> [arch/arm/Kconfig]
>
> config XIP_KERNEL
>         bool "Kernel Execute-In-Place from ROM"
> -       depends on !ARM_LPAE && !ARCH_MULTIPLATFORM
> +       depends on !ARM_LPAE

Okay, I guess it's not that hard to enable if one wants. Perhaps all

> However...I've given up on trying to get XIP_KERNEL enabled in Kconfig.
> But, of course still keep an eye on all the 'code' in the ARM core to
> make sure no one breaks it.


> > > At the end of the day, all I want is for the kernel to do an ioremap of
> > > this area for me so my file system (XIP cramfs in this case) can access
> > > it. "mtd-rom" does this for me today.
> >
> > And partition layout? If it was only what to map, we could add a
> > property in chosen or perhaps a reserved-memory node. But I doubt it
> > will remain that simple. What happens when you have clock management
> > and need to keep the SPI clock enabled for example.
>
> Yup...there's the killer!
> You'll notice that in mainline RZ/A devices, there is no QSPI driver,
> and also the QSPI clock is left out of the list of HW clocks. Otherwise,
> as you mentioned, that 'unused' clock would get shut off at the end of
> boot...and you're whole system would stop immediately.

If you had a simple driver with the binding as I proposed, then it
could do the ioremap and clock management. And you could also have
specific SPI and flash drivers for when you need to flashing without
changing the DT.

> > The other option is just have your platform code instantiate an
> > mtd-rom device. Either describe the h/w with DT or don't use DT.
>
> You mean in a 'board' file?

Yes.

> BTW, I still use board files for things that are too complicated for DT.

As you should.

> > > The other XIP file system that we use more often (AXFS, not in mainline)
> > > does not require the partitions in DT at all, so it's really only
> > > needed when using cramfs. So for now, maybe it would be better to just
> > remove
> > > this confusing portion of the dts.
> >
> > What does AXFS have that cramfs doesn't making it not need partitions?
>
> For AXFS, you pass in the physical address to your rootfs on the kernel
> command line and then AXFS does the ioreapping itself inside the driver.
>
> When Nicolas Pitre started adding XIP mode to cramfs last year, he also
> started out that way. But, that was rejected (as it was also rejected
> when DAX was first started). The file system people said 'file system
> drives should not have the ability to map physical memory themselves'.

Right, I remember that now.

> Another thing AXFS can do that Cramfs can't do (yet), is AXFS can within
> *each file* select what pages will be XIP, and what ones will be
> compressed. This saves a lot on Flash space when only your executable and
> const pages are left as uncompressed XIP pages.
>
>
> Chris
>

RE: [PATCH 2/2] ARM: dts: r7s9210-rza2mevb: Add support for RZ/A2M EVB

[Chris Brandt]

On Monday, December 17, 2018 1, linux-renesas-soc-owner@vger.kernel.org wrote:
> > Yup...there's the killer!
> > You'll notice that in mainline RZ/A devices, there is no QSPI driver,
> > and also the QSPI clock is left out of the list of HW clocks. Otherwise,
> > as you mentioned, that 'unused' clock would get shut off at the end of
> > boot...and you're whole system would stop immediately.
> 
> If you had a simple driver with the binding as I proposed, then it
> could do the ioremap and clock management. And you could also have
> specific SPI and flash drivers for when you need to flashing without
> changing the DT.

I agree.

At the same time, this particular XIP-QSPI HW is now used in the latest 
Renesas R-Car devices, and there has been some talk of upstreaming the 
driver for it (as in, using it in SPI mode instead of XIP mode).
I'm waiting to see where that ends up, and if I should just use that 
SPI mode driver for the RZ/A series but simply put in a new property like
"use-xip-mode;" that basically tells the driver to do nothing except enable 
the clock (again) so it doesn't get shut off at the end of boot.

However, with your idea, I could also add in the io-remapping into that 
driver as well.

The only thing I can't remember is if the cramfs-xip driver really 
needed to have an mtd partition with 'probe-type = "direct-mapped";' 
underneath it so it could more closely map different regions during run-time. 
And in that case, the QSPI driver should not be doing an io-remapping 
because it's not its responsibility (and might screw things up).


Chris

RE: [PATCH 2/2] ARM: dts: r7s9210-rza2mevb: Add support for RZ/A2M EVB

[Chris Brandt]

On Monday, December 17, 2018 1, linux-renesas-soc-owner@vger.kernel.org wrote:
> > Yup...there's the killer!
> > You'll notice that in mainline RZ/A devices, there is no QSPI driver,
> > and also the QSPI clock is left out of the list of HW clocks. Otherwise,
> > as you mentioned, that 'unused' clock would get shut off at the end of
> > boot...and you're whole system would stop immediately.
> 
> If you had a simple driver with the binding as I proposed, then it
> could do the ioremap and clock management. And you could also have
> specific SPI and flash drivers for when you need to flashing without
> changing the DT.

I agree.

At the same time, this particular XIP-QSPI HW is now used in the latest 
Renesas R-Car devices, and there has been some talk of upstreaming the 
driver for it (as in, using it in SPI mode instead of XIP mode).
I'm waiting to see where that ends up, and if I should just use that 
SPI mode driver for the RZ/A series but simply put in a new property like
"use-xip-mode;" that basically tells the driver to do nothing except enable 
the clock (again) so it doesn't get shut off at the end of boot.

However, with your idea, I could also add in the io-remapping into that 
driver as well.

The only thing I can't remember is if the cramfs-xip driver really 
needed to have an mtd partition with 'probe-type = "direct-mapped";' 
underneath it so it could more closely map different regions during run-time. 
And in that case, the QSPI driver should not be doing an io-remapping 
because it's not its responsibility (and might screw things up).


Chris

Re: [PATCH 2/2] ARM: dts: r7s9210-rza2mevb: Add support for RZ/A2M EVB

[Geert Uytterhoeven]

Hi Chris,

On Mon, Dec 17, 2018 at 8:40 PM Chris Brandt <Chris.Brandt@renesas.com> wrote:
> On Monday, December 17, 2018 1, linux-renesas-soc-owner@vger.kernel.org wrote:
> > > Yup...there's the killer!
> > > You'll notice that in mainline RZ/A devices, there is no QSPI driver,
> > > and also the QSPI clock is left out of the list of HW clocks. Otherwise,
> > > as you mentioned, that 'unused' clock would get shut off at the end of
> > > boot...and you're whole system would stop immediately.
> >
> > If you had a simple driver with the binding as I proposed, then it
> > could do the ioremap and clock management. And you could also have
> > specific SPI and flash drivers for when you need to flashing without
> > changing the DT.
>
> I agree.
>
> At the same time, this particular XIP-QSPI HW is now used in the latest
> Renesas R-Car devices, and there has been some talk of upstreaming the
> driver for it (as in, using it in SPI mode instead of XIP mode).
> I'm waiting to see where that ends up, and if I should just use that
> SPI mode driver for the RZ/A series but simply put in a new property like
> "use-xip-mode;" that basically tells the driver to do nothing except enable
> the clock (again) so it doesn't get shut off at the end of boot.

The "clock management" should simply be calls to pm_runtime_enable()
and pm_runtime_get_sync().

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 2/2] ARM: dts: r7s9210-rza2mevb: Add support for RZ/A2M EVB

[Geert Uytterhoeven]

Hi Chris,

On Mon, Dec 17, 2018 at 8:40 PM Chris Brandt <Chris.Brandt@renesas.com> wrote:
> On Monday, December 17, 2018 1, linux-renesas-soc-owner@vger.kernel.org wrote:
> > > Yup...there's the killer!
> > > You'll notice that in mainline RZ/A devices, there is no QSPI driver,
> > > and also the QSPI clock is left out of the list of HW clocks. Otherwise,
> > > as you mentioned, that 'unused' clock would get shut off at the end of
> > > boot...and you're whole system would stop immediately.
> >
> > If you had a simple driver with the binding as I proposed, then it
> > could do the ioremap and clock management. And you could also have
> > specific SPI and flash drivers for when you need to flashing without
> > changing the DT.
>
> I agree.
>
> At the same time, this particular XIP-QSPI HW is now used in the latest
> Renesas R-Car devices, and there has been some talk of upstreaming the
> driver for it (as in, using it in SPI mode instead of XIP mode).
> I'm waiting to see where that ends up, and if I should just use that
> SPI mode driver for the RZ/A series but simply put in a new property like
> "use-xip-mode;" that basically tells the driver to do nothing except enable
> the clock (again) so it doesn't get shut off at the end of boot.

The "clock management" should simply be calls to pm_runtime_enable()
and pm_runtime_get_sync().

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