[PATCH] arm64: dts: juno: Fix DMA address translations by adding SOC bus node

[PATCH] arm64: dts: juno: Fix DMA address translations by adding SOC bus node

[Sudeep Holla]

Commit 951d48855d86 ("of: Make of_dma_get_range() work on bus nodes")
reworked the logic such that of_dma_get_range() works correctly
starting from a bus node containing "dma-ranges".

Since on Juno we don't have a SoC level bus node and "dma-ranges" is
present only in the root node, we get the following error:

OF: translation of DMA address(0) to CPU address failed node(/sram@2e000000)
OF: translation of DMA address(0) to CPU address failed node(/uart@7ff80000)
...
OF: translation of DMA address(0) to CPU address failed node(/mhu@2b1f0000)
OF: translation of DMA address(0) to CPU address failed node(/iommu@2b600000)
OF: translation of DMA address(0) to CPU address failed node(/iommu@2b600000)
OF: translation of DMA address(0) to CPU address failed node(/iommu@2b600000)

Let's fix it by adding a SoC bus node and moving all the devices along
with the "dma-ranges" inside it.

Cc: Rob Herring <robh+dt@kernel.org>
Cc: Liviu Dudau <liviu.dudau@arm.com>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 arch/arm64/boot/dts/arm/juno-base.dtsi        | 162 +++++++++---------
 arch/arm64/boot/dts/arm/juno-clocks.dtsi      |   2 +
 arch/arm64/boot/dts/arm/juno-cs-r1r2.dtsi     |   2 +
 arch/arm64/boot/dts/arm/juno-motherboard.dtsi |   2 +
 4 files changed, 88 insertions(+), 80 deletions(-)

Hi Rob, Robin,

Let me know if this is correct fix for the issue I am seeing with linux-next
on Juno. This patch is generated with -b for ease of review. With lots of
indentation, actual delta is:

4 files changed, 1274 insertions(+), 1266 deletions(-)

Regards,
Sudeep

diff --git a/arch/arm64/boot/dts/arm/juno-base.dtsi b/arch/arm64/boot/dts/arm/juno-base.dtsi
index 9e3e8ce6adfe..ef21d7245e3b 100644
--- a/arch/arm64/boot/dts/arm/juno-base.dtsi
+++ b/arch/arm64/boot/dts/arm/juno-base.dtsi
@@ -3,6 +3,87 @@
 #include "juno-motherboard.dtsi"
 
 / {
+	gic: interrupt-controller@2c010000 {
+		compatible = "arm,gic-400", "arm,cortex-a15-gic";
+		reg = <0x0 0x2c010000 0 0x1000>,
+		      <0x0 0x2c02f000 0 0x2000>,
+		      <0x0 0x2c04f000 0 0x2000>,
+		      <0x0 0x2c06f000 0 0x2000>;
+		#address-cells = <2>;
+		#interrupt-cells = <3>;
+		#size-cells = <2>;
+		interrupt-controller;
+		interrupts = <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(6) | IRQ_TYPE_LEVEL_HIGH)>;
+		ranges = <0 0 0 0x2c1c0000 0 0x40000>;
+
+		v2m_0: v2m@0 {
+			compatible = "arm,gic-v2m-frame";
+			msi-controller;
+			reg = <0 0 0 0x10000>;
+		};
+
+		v2m@10000 {
+			compatible = "arm,gic-v2m-frame";
+			msi-controller;
+			reg = <0 0x10000 0 0x10000>;
+		};
+
+		v2m@20000 {
+			compatible = "arm,gic-v2m-frame";
+			msi-controller;
+			reg = <0 0x20000 0 0x10000>;
+		};
+
+		v2m@30000 {
+			compatible = "arm,gic-v2m-frame";
+			msi-controller;
+			reg = <0 0x30000 0 0x10000>;
+		};
+	};
+
+	timer {
+		compatible = "arm,armv8-timer";
+		interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(6) | IRQ_TYPE_LEVEL_LOW)>,
+			     <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(6) | IRQ_TYPE_LEVEL_LOW)>,
+			     <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(6) | IRQ_TYPE_LEVEL_LOW)>,
+			     <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(6) | IRQ_TYPE_LEVEL_LOW)>;
+	};
+
+	scpi {
+		compatible = "arm,scpi";
+		mboxes = <&mailbox 1>;
+		shmem = <&cpu_scp_hpri>;
+
+		clocks {
+			compatible = "arm,scpi-clocks";
+
+			scpi_dvfs: scpi-dvfs {
+				compatible = "arm,scpi-dvfs-clocks";
+				#clock-cells = <1>;
+				clock-indices = <0>, <1>, <2>;
+				clock-output-names = "atlclk", "aplclk","gpuclk";
+			};
+			scpi_clk: scpi-clk {
+				compatible = "arm,scpi-variable-clocks";
+				#clock-cells = <1>;
+				clock-indices = <3>;
+				clock-output-names = "pxlclk";
+			};
+		};
+
+		scpi_devpd: scpi-power-domains {
+			compatible = "arm,scpi-power-domains";
+			num-domains = <2>;
+			#power-domain-cells = <1>;
+		};
+
+		scpi_sensors0: sensors {
+			compatible = "arm,scpi-sensors";
+			#thermal-sensor-cells = <1>;
+		};
+	};
+
+	soc {
 		/*
 		 *  Devices shared by all Juno boards
 		 */
@@ -69,52 +150,6 @@
 			power-domains = <&scpi_devpd 0>;
 		};
 
-	gic: interrupt-controller@2c010000 {
-		compatible = "arm,gic-400", "arm,cortex-a15-gic";
-		reg = <0x0 0x2c010000 0 0x1000>,
-		      <0x0 0x2c02f000 0 0x2000>,
-		      <0x0 0x2c04f000 0 0x2000>,
-		      <0x0 0x2c06f000 0 0x2000>;
-		#address-cells = <2>;
-		#interrupt-cells = <3>;
-		#size-cells = <2>;
-		interrupt-controller;
-		interrupts = <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(6) | IRQ_TYPE_LEVEL_HIGH)>;
-		ranges = <0 0 0 0x2c1c0000 0 0x40000>;
-
-		v2m_0: v2m@0 {
-			compatible = "arm,gic-v2m-frame";
-			msi-controller;
-			reg = <0 0 0 0x10000>;
-		};
-
-		v2m@10000 {
-			compatible = "arm,gic-v2m-frame";
-			msi-controller;
-			reg = <0 0x10000 0 0x10000>;
-		};
-
-		v2m@20000 {
-			compatible = "arm,gic-v2m-frame";
-			msi-controller;
-			reg = <0 0x20000 0 0x10000>;
-		};
-
-		v2m@30000 {
-			compatible = "arm,gic-v2m-frame";
-			msi-controller;
-			reg = <0 0x30000 0 0x10000>;
-		};
-	};
-
-	timer {
-		compatible = "arm,armv8-timer";
-		interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(6) | IRQ_TYPE_LEVEL_LOW)>,
-			     <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(6) | IRQ_TYPE_LEVEL_LOW)>,
-			     <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(6) | IRQ_TYPE_LEVEL_LOW)>,
-			     <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(6) | IRQ_TYPE_LEVEL_LOW)>;
-	};
-
 		/*
 		 * Juno TRMs specify the size for these coresight components as 64K.
 		 * The actual size is just 4K though 64K is reserved. Access to the
@@ -557,40 +592,6 @@
 			iommu-map = <0x0 &smmu_pcie 0x0 0x1>;
 		};
 
-	scpi {
-		compatible = "arm,scpi";
-		mboxes = <&mailbox 1>;
-		shmem = <&cpu_scp_hpri>;
-
-		clocks {
-			compatible = "arm,scpi-clocks";
-
-			scpi_dvfs: scpi-dvfs {
-				compatible = "arm,scpi-dvfs-clocks";
-				#clock-cells = <1>;
-				clock-indices = <0>, <1>, <2>;
-				clock-output-names = "atlclk", "aplclk","gpuclk";
-			};
-			scpi_clk: scpi-clk {
-				compatible = "arm,scpi-variable-clocks";
-				#clock-cells = <1>;
-				clock-indices = <3>;
-				clock-output-names = "pxlclk";
-			};
-		};
-
-		scpi_devpd: scpi-power-domains {
-			compatible = "arm,scpi-power-domains";
-			num-domains = <2>;
-			#power-domain-cells = <1>;
-		};
-
-		scpi_sensors0: sensors {
-			compatible = "arm,scpi-sensors";
-			#thermal-sensor-cells = <1>;
-		};
-	};
-
 		thermal-zones {
 			pmic {
 				polling-delay = <1000>;
@@ -838,4 +839,5 @@
 			interrupt-map-mask = <0 0>;
 			interrupt-map = <0 0 &gic 0 0 GIC_SPI 168 IRQ_TYPE_LEVEL_HIGH>;
 		};
+	};
 };
diff --git a/arch/arm64/boot/dts/arm/juno-clocks.dtsi b/arch/arm64/boot/dts/arm/juno-clocks.dtsi
index e5e265dfa902..3ea934017b98 100644
--- a/arch/arm64/boot/dts/arm/juno-clocks.dtsi
+++ b/arch/arm64/boot/dts/arm/juno-clocks.dtsi
@@ -7,6 +7,7 @@
  *
  */
 / {
+	soc {
 		/* SoC fixed clocks */
 		soc_uartclk: refclk7273800hz {
 			compatible = "fixed-clock";
@@ -42,4 +43,5 @@
 			clock-frequency = <400000000>;
 			clock-output-names = "faxi_clk";
 		};
+	};
 };
diff --git a/arch/arm64/boot/dts/arm/juno-cs-r1r2.dtsi b/arch/arm64/boot/dts/arm/juno-cs-r1r2.dtsi
index eda3d9e18af6..3f6e607b450d 100644
--- a/arch/arm64/boot/dts/arm/juno-cs-r1r2.dtsi
+++ b/arch/arm64/boot/dts/arm/juno-cs-r1r2.dtsi
@@ -1,5 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0
 / {
+	soc {
 		funnel@20130000 { /* cssys1 */
 			compatible = "arm,coresight-dynamic-funnel", "arm,primecell";
 			reg = <0 0x20130000 0 0x1000>;
@@ -82,4 +83,5 @@
 
 			};
 		};
+	};
 };
diff --git a/arch/arm64/boot/dts/arm/juno-motherboard.dtsi b/arch/arm64/boot/dts/arm/juno-motherboard.dtsi
index 9f60dacb4f80..638c6b5b342a 100644
--- a/arch/arm64/boot/dts/arm/juno-motherboard.dtsi
+++ b/arch/arm64/boot/dts/arm/juno-motherboard.dtsi
@@ -8,6 +8,7 @@
  */
 
 / {
+	soc {
 		smb@8000000 {
 			mb_clk24mhz: clk24mhz {
 				compatible = "fixed-clock";
@@ -292,4 +293,5 @@
 				};
 			};
 		};
+	};
 };
-- 
2.17.1

Re: [PATCH] arm64: dts: juno: Fix DMA address translations by adding SOC bus node

[Robin Murphy]

Hi Sudeep,

On 2019-11-26 4:53 pm, Sudeep Holla wrote:
> Commit 951d48855d86 ("of: Make of_dma_get_range() work on bus nodes")
> reworked the logic such that of_dma_get_range() works correctly
> starting from a bus node containing "dma-ranges".
> 
> Since on Juno we don't have a SoC level bus node and "dma-ranges" is
> present only in the root node, we get the following error:
> 
> OF: translation of DMA address(0) to CPU address failed node(/sram@2e000000)
> OF: translation of DMA address(0) to CPU address failed node(/uart@7ff80000)
> ...
> OF: translation of DMA address(0) to CPU address failed node(/mhu@2b1f0000)
> OF: translation of DMA address(0) to CPU address failed node(/iommu@2b600000)
> OF: translation of DMA address(0) to CPU address failed node(/iommu@2b600000)
> OF: translation of DMA address(0) to CPU address failed node(/iommu@2b600000)
> 
> Let's fix it by adding a SoC bus node and moving all the devices along
> with the "dma-ranges" inside it.
> 
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Liviu Dudau <liviu.dudau@arm.com>
> Cc: Robin Murphy <robin.murphy@arm.com>
> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
>   arch/arm64/boot/dts/arm/juno-base.dtsi        | 162 +++++++++---------
>   arch/arm64/boot/dts/arm/juno-clocks.dtsi      |   2 +
>   arch/arm64/boot/dts/arm/juno-cs-r1r2.dtsi     |   2 +
>   arch/arm64/boot/dts/arm/juno-motherboard.dtsi |   2 +
>   4 files changed, 88 insertions(+), 80 deletions(-)
> 
> Hi Rob, Robin,
> 
> Let me know if this is correct fix for the issue I am seeing with linux-next
> on Juno. This patch is generated with -b for ease of review. With lots of
> indentation, actual delta is:
> 
> 4 files changed, 1274 insertions(+), 1266 deletions(-)

Other than a few nits - the GIC should probably be under the soc node as 
it's an MMIO device, while the clocks shouldn't; the dtsi's could 
probably avoid churn with a "&soc {...}" phandle - I think this is a 
reasonable thing to do, as it's generally the preferred structure.

The cruder but far simpler alternative would be to just drop the 
dma-ranges property - I'm not sure the effort to make all 64-bit 
platforms describe their dma-ranges has really panned out, and it isn't 
actually necessary for Juno (which is why it didn't seem like sufficient 
reason to do all this restructuring at the time, and instead I took a 
very liberal interpretation of the spec to sneak it into the root node).

Robin.

> 
> Regards,
> Sudeep
> 
> diff --git a/arch/arm64/boot/dts/arm/juno-base.dtsi b/arch/arm64/boot/dts/arm/juno-base.dtsi
> index 9e3e8ce6adfe..ef21d7245e3b 100644
> --- a/arch/arm64/boot/dts/arm/juno-base.dtsi
> +++ b/arch/arm64/boot/dts/arm/juno-base.dtsi
> @@ -3,6 +3,87 @@
>   #include "juno-motherboard.dtsi"
>   
>   / {
> +	gic: interrupt-controller@2c010000 {
> +		compatible = "arm,gic-400", "arm,cortex-a15-gic";
> +		reg = <0x0 0x2c010000 0 0x1000>,
> +		      <0x0 0x2c02f000 0 0x2000>,
> +		      <0x0 0x2c04f000 0 0x2000>,
> +		      <0x0 0x2c06f000 0 0x2000>;
> +		#address-cells = <2>;
> +		#interrupt-cells = <3>;
> +		#size-cells = <2>;
> +		interrupt-controller;
> +		interrupts = <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(6) | IRQ_TYPE_LEVEL_HIGH)>;
> +		ranges = <0 0 0 0x2c1c0000 0 0x40000>;
> +
> +		v2m_0: v2m@0 {
> +			compatible = "arm,gic-v2m-frame";
> +			msi-controller;
> +			reg = <0 0 0 0x10000>;
> +		};
> +
> +		v2m@10000 {
> +			compatible = "arm,gic-v2m-frame";
> +			msi-controller;
> +			reg = <0 0x10000 0 0x10000>;
> +		};
> +
> +		v2m@20000 {
> +			compatible = "arm,gic-v2m-frame";
> +			msi-controller;
> +			reg = <0 0x20000 0 0x10000>;
> +		};
> +
> +		v2m@30000 {
> +			compatible = "arm,gic-v2m-frame";
> +			msi-controller;
> +			reg = <0 0x30000 0 0x10000>;
> +		};
> +	};
> +
> +	timer {
> +		compatible = "arm,armv8-timer";
> +		interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(6) | IRQ_TYPE_LEVEL_LOW)>,
> +			     <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(6) | IRQ_TYPE_LEVEL_LOW)>,
> +			     <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(6) | IRQ_TYPE_LEVEL_LOW)>,
> +			     <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(6) | IRQ_TYPE_LEVEL_LOW)>;
> +	};
> +
> +	scpi {
> +		compatible = "arm,scpi";
> +		mboxes = <&mailbox 1>;
> +		shmem = <&cpu_scp_hpri>;
> +
> +		clocks {
> +			compatible = "arm,scpi-clocks";
> +
> +			scpi_dvfs: scpi-dvfs {
> +				compatible = "arm,scpi-dvfs-clocks";
> +				#clock-cells = <1>;
> +				clock-indices = <0>, <1>, <2>;
> +				clock-output-names = "atlclk", "aplclk","gpuclk";
> +			};
> +			scpi_clk: scpi-clk {
> +				compatible = "arm,scpi-variable-clocks";
> +				#clock-cells = <1>;
> +				clock-indices = <3>;
> +				clock-output-names = "pxlclk";
> +			};
> +		};
> +
> +		scpi_devpd: scpi-power-domains {
> +			compatible = "arm,scpi-power-domains";
> +			num-domains = <2>;
> +			#power-domain-cells = <1>;
> +		};
> +
> +		scpi_sensors0: sensors {
> +			compatible = "arm,scpi-sensors";
> +			#thermal-sensor-cells = <1>;
> +		};
> +	};
> +
> +	soc {
>   		/*
>   		 *  Devices shared by all Juno boards
>   		 */
> @@ -69,52 +150,6 @@
>   			power-domains = <&scpi_devpd 0>;
>   		};
>   
> -	gic: interrupt-controller@2c010000 {
> -		compatible = "arm,gic-400", "arm,cortex-a15-gic";
> -		reg = <0x0 0x2c010000 0 0x1000>,
> -		      <0x0 0x2c02f000 0 0x2000>,
> -		      <0x0 0x2c04f000 0 0x2000>,
> -		      <0x0 0x2c06f000 0 0x2000>;
> -		#address-cells = <2>;
> -		#interrupt-cells = <3>;
> -		#size-cells = <2>;
> -		interrupt-controller;
> -		interrupts = <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(6) | IRQ_TYPE_LEVEL_HIGH)>;
> -		ranges = <0 0 0 0x2c1c0000 0 0x40000>;
> -
> -		v2m_0: v2m@0 {
> -			compatible = "arm,gic-v2m-frame";
> -			msi-controller;
> -			reg = <0 0 0 0x10000>;
> -		};
> -
> -		v2m@10000 {
> -			compatible = "arm,gic-v2m-frame";
> -			msi-controller;
> -			reg = <0 0x10000 0 0x10000>;
> -		};
> -
> -		v2m@20000 {
> -			compatible = "arm,gic-v2m-frame";
> -			msi-controller;
> -			reg = <0 0x20000 0 0x10000>;
> -		};
> -
> -		v2m@30000 {
> -			compatible = "arm,gic-v2m-frame";
> -			msi-controller;
> -			reg = <0 0x30000 0 0x10000>;
> -		};
> -	};
> -
> -	timer {
> -		compatible = "arm,armv8-timer";
> -		interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(6) | IRQ_TYPE_LEVEL_LOW)>,
> -			     <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(6) | IRQ_TYPE_LEVEL_LOW)>,
> -			     <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(6) | IRQ_TYPE_LEVEL_LOW)>,
> -			     <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(6) | IRQ_TYPE_LEVEL_LOW)>;
> -	};
> -
>   		/*
>   		 * Juno TRMs specify the size for these coresight components as 64K.
>   		 * The actual size is just 4K though 64K is reserved. Access to the
> @@ -557,40 +592,6 @@
>   			iommu-map = <0x0 &smmu_pcie 0x0 0x1>;
>   		};
>   
> -	scpi {
> -		compatible = "arm,scpi";
> -		mboxes = <&mailbox 1>;
> -		shmem = <&cpu_scp_hpri>;
> -
> -		clocks {
> -			compatible = "arm,scpi-clocks";
> -
> -			scpi_dvfs: scpi-dvfs {
> -				compatible = "arm,scpi-dvfs-clocks";
> -				#clock-cells = <1>;
> -				clock-indices = <0>, <1>, <2>;
> -				clock-output-names = "atlclk", "aplclk","gpuclk";
> -			};
> -			scpi_clk: scpi-clk {
> -				compatible = "arm,scpi-variable-clocks";
> -				#clock-cells = <1>;
> -				clock-indices = <3>;
> -				clock-output-names = "pxlclk";
> -			};
> -		};
> -
> -		scpi_devpd: scpi-power-domains {
> -			compatible = "arm,scpi-power-domains";
> -			num-domains = <2>;
> -			#power-domain-cells = <1>;
> -		};
> -
> -		scpi_sensors0: sensors {
> -			compatible = "arm,scpi-sensors";
> -			#thermal-sensor-cells = <1>;
> -		};
> -	};
> -
>   		thermal-zones {
>   			pmic {
>   				polling-delay = <1000>;
> @@ -838,4 +839,5 @@
>   			interrupt-map-mask = <0 0>;
>   			interrupt-map = <0 0 &gic 0 0 GIC_SPI 168 IRQ_TYPE_LEVEL_HIGH>;
>   		};
> +	};
>   };
> diff --git a/arch/arm64/boot/dts/arm/juno-clocks.dtsi b/arch/arm64/boot/dts/arm/juno-clocks.dtsi
> index e5e265dfa902..3ea934017b98 100644
> --- a/arch/arm64/boot/dts/arm/juno-clocks.dtsi
> +++ b/arch/arm64/boot/dts/arm/juno-clocks.dtsi
> @@ -7,6 +7,7 @@
>    *
>    */
>   / {
> +	soc {
>   		/* SoC fixed clocks */
>   		soc_uartclk: refclk7273800hz {
>   			compatible = "fixed-clock";
> @@ -42,4 +43,5 @@
>   			clock-frequency = <400000000>;
>   			clock-output-names = "faxi_clk";
>   		};
> +	};
>   };
> diff --git a/arch/arm64/boot/dts/arm/juno-cs-r1r2.dtsi b/arch/arm64/boot/dts/arm/juno-cs-r1r2.dtsi
> index eda3d9e18af6..3f6e607b450d 100644
> --- a/arch/arm64/boot/dts/arm/juno-cs-r1r2.dtsi
> +++ b/arch/arm64/boot/dts/arm/juno-cs-r1r2.dtsi
> @@ -1,5 +1,6 @@
>   // SPDX-License-Identifier: GPL-2.0
>   / {
> +	soc {
>   		funnel@20130000 { /* cssys1 */
>   			compatible = "arm,coresight-dynamic-funnel", "arm,primecell";
>   			reg = <0 0x20130000 0 0x1000>;
> @@ -82,4 +83,5 @@
>   
>   			};
>   		};
> +	};
>   };
> diff --git a/arch/arm64/boot/dts/arm/juno-motherboard.dtsi b/arch/arm64/boot/dts/arm/juno-motherboard.dtsi
> index 9f60dacb4f80..638c6b5b342a 100644
> --- a/arch/arm64/boot/dts/arm/juno-motherboard.dtsi
> +++ b/arch/arm64/boot/dts/arm/juno-motherboard.dtsi
> @@ -8,6 +8,7 @@
>    */
>   
>   / {
> +	soc {
>   		smb@8000000 {
>   			mb_clk24mhz: clk24mhz {
>   				compatible = "fixed-clock";
> @@ -292,4 +293,5 @@
>   				};
>   			};
>   		};
> +	};
>   };
> 

Re: [PATCH] arm64: dts: juno: Fix DMA address translations by adding SOC bus node

[Sudeep Holla]

On Thu, Nov 28, 2019 at 11:50:54AM +0000, Robin Murphy wrote:
> Hi Sudeep,
> 
> On 2019-11-26 4:53 pm, Sudeep Holla wrote:
> > Commit 951d48855d86 ("of: Make of_dma_get_range() work on bus nodes")
> > reworked the logic such that of_dma_get_range() works correctly
> > starting from a bus node containing "dma-ranges".
> > 
> > Since on Juno we don't have a SoC level bus node and "dma-ranges" is
> > present only in the root node, we get the following error:
> > 
> > OF: translation of DMA address(0) to CPU address failed node(/sram@2e000000)
> > OF: translation of DMA address(0) to CPU address failed node(/uart@7ff80000)
> > ...
> > OF: translation of DMA address(0) to CPU address failed node(/mhu@2b1f0000)
> > OF: translation of DMA address(0) to CPU address failed node(/iommu@2b600000)
> > OF: translation of DMA address(0) to CPU address failed node(/iommu@2b600000)
> > OF: translation of DMA address(0) to CPU address failed node(/iommu@2b600000)
> > 
> > Let's fix it by adding a SoC bus node and moving all the devices along
> > with the "dma-ranges" inside it.
> > 
> > Cc: Rob Herring <robh+dt@kernel.org>
> > Cc: Liviu Dudau <liviu.dudau@arm.com>
> > Cc: Robin Murphy <robin.murphy@arm.com>
> > Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> > ---
> >   arch/arm64/boot/dts/arm/juno-base.dtsi        | 162 +++++++++---------
> >   arch/arm64/boot/dts/arm/juno-clocks.dtsi      |   2 +
> >   arch/arm64/boot/dts/arm/juno-cs-r1r2.dtsi     |   2 +
> >   arch/arm64/boot/dts/arm/juno-motherboard.dtsi |   2 +
> >   4 files changed, 88 insertions(+), 80 deletions(-)
> > 
> > Hi Rob, Robin,
> > 
> > Let me know if this is correct fix for the issue I am seeing with linux-next
> > on Juno. This patch is generated with -b for ease of review. With lots of
> > indentation, actual delta is:
> > 
> > 4 files changed, 1274 insertions(+), 1266 deletions(-)
> 
> Other than a few nits - the GIC should probably be under the soc node as
> it's an MMIO device, while the clocks shouldn't; the dtsi's could probably
> avoid churn with a "&soc {...}" phandle - I think this is a reasonable thing
> to do, as it's generally the preferred structure.
>

I agree and am still in confusion as what to put inside soc or not.

> The cruder but far simpler alternative would be to just drop the dma-ranges
> property - I'm not sure the effort to make all 64-bit platforms describe
> their dma-ranges has really panned out, and it isn't actually necessary for
> Juno (which is why it didn't seem like sufficient reason to do all this
> restructuring at the time, and instead I took a very liberal interpretation
> of the spec to sneak it into the root node).
>

I think I prefer that for v5.5 as a fix as this is much bigger churn. We
can do that for v5.6 if required. Let me know if you disagree. I can simply
revert your original patch adding dma-ranges for now and we can add it
later with all the soc structure.

--
Regards,
Sudeep

[PATCH] Revert "arm64: dts: juno: add dma-ranges property"

[Sudeep Holla]

This reverts commit 193d00a2b35ee3353813b4006a18131122087205.

Commit 951d48855d86 ("of: Make of_dma_get_range() work on bus nodes")
reworked the logic such that of_dma_get_range() works correctly
starting from a bus node containing "dma-ranges".

Since on Juno we don't have a SoC level bus node and "dma-ranges" is
present only in the root node, we get the following error:

OF: translation of DMA address(0) to CPU address failed node(/sram@2e000000)
OF: translation of DMA address(0) to CPU address failed node(/uart@7ff80000)
...
OF: translation of DMA address(0) to CPU address failed node(/mhu@2b1f0000)
OF: translation of DMA address(0) to CPU address failed node(/iommu@2b600000)
OF: translation of DMA address(0) to CPU address failed node(/iommu@2b600000)
OF: translation of DMA address(0) to CPU address failed node(/iommu@2b600000)

So let's fix it by dropping the "dma-ranges" property for now. We can
add it later with a proper SoC bus node and moving all the devices that
belong there along with the "dma-ranges" if required.

Cc: Rob Herring <robh+dt@kernel.org>
Cc: Liviu Dudau <liviu.dudau@arm.com>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 arch/arm64/boot/dts/arm/juno-base.dtsi | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm64/boot/dts/arm/juno-base.dtsi b/arch/arm64/boot/dts/arm/juno-base.dtsi
index 9e3e8ce6adfe..1f3c80aafbd7 100644
--- a/arch/arm64/boot/dts/arm/juno-base.dtsi
+++ b/arch/arm64/boot/dts/arm/juno-base.dtsi
@@ -6,7 +6,6 @@
 	/*
 	 *  Devices shared by all Juno boards
 	 */
-	dma-ranges = <0 0 0 0 0x100 0>;
 
 	memtimer: timer@2a810000 {
 		compatible = "arm,armv7-timer-mem";
-- 
2.17.1

Re: [PATCH] Revert "arm64: dts: juno: add dma-ranges property"

[Robin Murphy]

On 28/11/2019 3:42 pm, Sudeep Holla wrote:
> This reverts commit 193d00a2b35ee3353813b4006a18131122087205.
> 
> Commit 951d48855d86 ("of: Make of_dma_get_range() work on bus nodes")
> reworked the logic such that of_dma_get_range() works correctly
> starting from a bus node containing "dma-ranges".
> 
> Since on Juno we don't have a SoC level bus node and "dma-ranges" is
> present only in the root node, we get the following error:
> 
> OF: translation of DMA address(0) to CPU address failed node(/sram@2e000000)
> OF: translation of DMA address(0) to CPU address failed node(/uart@7ff80000)
> ...
> OF: translation of DMA address(0) to CPU address failed node(/mhu@2b1f0000)
> OF: translation of DMA address(0) to CPU address failed node(/iommu@2b600000)
> OF: translation of DMA address(0) to CPU address failed node(/iommu@2b600000)
> OF: translation of DMA address(0) to CPU address failed node(/iommu@2b600000)
> 
> So let's fix it by dropping the "dma-ranges" property for now. We can
> add it later with a proper SoC bus node and moving all the devices that
> belong there along with the "dma-ranges" if required.

Acked-by: Robin Murphy <robin.murphy@arm.com>

As mentioned before, this is fine since it doesn't represent any kind of 
device-visible restriction; it was only there for completeness, and 
we've since given in to the assumption that missing "dma-ranges" implies 
a 1:1 mapping anyway.

Thanks,
Robin.

> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Liviu Dudau <liviu.dudau@arm.com>
> Cc: Robin Murphy <robin.murphy@arm.com>
> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
>   arch/arm64/boot/dts/arm/juno-base.dtsi | 1 -
>   1 file changed, 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/arm/juno-base.dtsi b/arch/arm64/boot/dts/arm/juno-base.dtsi
> index 9e3e8ce6adfe..1f3c80aafbd7 100644
> --- a/arch/arm64/boot/dts/arm/juno-base.dtsi
> +++ b/arch/arm64/boot/dts/arm/juno-base.dtsi
> @@ -6,7 +6,6 @@
>   	/*
>   	 *  Devices shared by all Juno boards
>   	 */
> -	dma-ranges = <0 0 0 0 0x100 0>;
>   
>   	memtimer: timer@2a810000 {
>   		compatible = "arm,armv7-timer-mem";
> 

Re: [PATCH] arm64: dts: juno: Fix DMA address translations by adding SOC bus node

[Robin Murphy]

On 28/11/2019 2:15 pm, Sudeep Holla wrote:
> On Thu, Nov 28, 2019 at 11:50:54AM +0000, Robin Murphy wrote:
>> Hi Sudeep,
>>
>> On 2019-11-26 4:53 pm, Sudeep Holla wrote:
>>> Commit 951d48855d86 ("of: Make of_dma_get_range() work on bus nodes")
>>> reworked the logic such that of_dma_get_range() works correctly
>>> starting from a bus node containing "dma-ranges".
>>>
>>> Since on Juno we don't have a SoC level bus node and "dma-ranges" is
>>> present only in the root node, we get the following error:
>>>
>>> OF: translation of DMA address(0) to CPU address failed node(/sram@2e000000)
>>> OF: translation of DMA address(0) to CPU address failed node(/uart@7ff80000)
>>> ...
>>> OF: translation of DMA address(0) to CPU address failed node(/mhu@2b1f0000)
>>> OF: translation of DMA address(0) to CPU address failed node(/iommu@2b600000)
>>> OF: translation of DMA address(0) to CPU address failed node(/iommu@2b600000)
>>> OF: translation of DMA address(0) to CPU address failed node(/iommu@2b600000)
>>>
>>> Let's fix it by adding a SoC bus node and moving all the devices along
>>> with the "dma-ranges" inside it.
>>>
>>> Cc: Rob Herring <robh+dt@kernel.org>
>>> Cc: Liviu Dudau <liviu.dudau@arm.com>
>>> Cc: Robin Murphy <robin.murphy@arm.com>
>>> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
>>> ---
>>>    arch/arm64/boot/dts/arm/juno-base.dtsi        | 162 +++++++++---------
>>>    arch/arm64/boot/dts/arm/juno-clocks.dtsi      |   2 +
>>>    arch/arm64/boot/dts/arm/juno-cs-r1r2.dtsi     |   2 +
>>>    arch/arm64/boot/dts/arm/juno-motherboard.dtsi |   2 +
>>>    4 files changed, 88 insertions(+), 80 deletions(-)
>>>
>>> Hi Rob, Robin,
>>>
>>> Let me know if this is correct fix for the issue I am seeing with linux-next
>>> on Juno. This patch is generated with -b for ease of review. With lots of
>>> indentation, actual delta is:
>>>
>>> 4 files changed, 1274 insertions(+), 1266 deletions(-)
>>
>> Other than a few nits - the GIC should probably be under the soc node as
>> it's an MMIO device, while the clocks shouldn't; the dtsi's could probably
>> avoid churn with a "&soc {...}" phandle - I think this is a reasonable thing
>> to do, as it's generally the preferred structure.
>>
> 
> I agree and am still in confusion as what to put inside soc or not.

FWIW my understanding is that the "soc" node is used to provide a 'bus' 
to represent on-chip MMIO - in principle we could nerd out and describe 
the ACE-lite/AXI/APB slave interconnects in full, but there's really no 
benefit to going into that much detail - so everything with a "reg" 
representing a physical address goes inside it, while CPUs, clocks, 
firmware, regulators etc. sit in the root node 'outside the PA space', 
regardless of whether they're physically on-chip or not.

Robin.

>> The cruder but far simpler alternative would be to just drop the dma-ranges
>> property - I'm not sure the effort to make all 64-bit platforms describe
>> their dma-ranges has really panned out, and it isn't actually necessary for
>> Juno (which is why it didn't seem like sufficient reason to do all this
>> restructuring at the time, and instead I took a very liberal interpretation
>> of the spec to sneak it into the root node).
>>
> 
> I think I prefer that for v5.5 as a fix as this is much bigger churn. We
> can do that for v5.6 if required. Let me know if you disagree. I can simply
> revert your original patch adding dma-ranges for now and we can add it
> later with all the soc structure.
> 
> --
> Regards,
> Sudeep
> 

Re: [PATCH] arm64: dts: juno: Fix DMA address translations by adding SOC bus node

[Sudeep Holla]

On Thu, Nov 28, 2019 at 04:15:43PM +0000, Robin Murphy wrote:
> On 28/11/2019 2:15 pm, Sudeep Holla wrote:
> > On Thu, Nov 28, 2019 at 11:50:54AM +0000, Robin Murphy wrote:
> > > Hi Sudeep,
> > > 
> > > On 2019-11-26 4:53 pm, Sudeep Holla wrote:
> > > > Commit 951d48855d86 ("of: Make of_dma_get_range() work on bus nodes")
> > > > reworked the logic such that of_dma_get_range() works correctly
> > > > starting from a bus node containing "dma-ranges".
> > > > 
> > > > Since on Juno we don't have a SoC level bus node and "dma-ranges" is
> > > > present only in the root node, we get the following error:
> > > > 
> > > > OF: translation of DMA address(0) to CPU address failed node(/sram@2e000000)
> > > > OF: translation of DMA address(0) to CPU address failed node(/uart@7ff80000)
> > > > ...
> > > > OF: translation of DMA address(0) to CPU address failed node(/mhu@2b1f0000)
> > > > OF: translation of DMA address(0) to CPU address failed node(/iommu@2b600000)
> > > > OF: translation of DMA address(0) to CPU address failed node(/iommu@2b600000)
> > > > OF: translation of DMA address(0) to CPU address failed node(/iommu@2b600000)
> > > > 
> > > > Let's fix it by adding a SoC bus node and moving all the devices along
> > > > with the "dma-ranges" inside it.
> > > > 
> > > > Cc: Rob Herring <robh+dt@kernel.org>
> > > > Cc: Liviu Dudau <liviu.dudau@arm.com>
> > > > Cc: Robin Murphy <robin.murphy@arm.com>
> > > > Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > > > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> > > > ---
> > > >    arch/arm64/boot/dts/arm/juno-base.dtsi        | 162 +++++++++---------
> > > >    arch/arm64/boot/dts/arm/juno-clocks.dtsi      |   2 +
> > > >    arch/arm64/boot/dts/arm/juno-cs-r1r2.dtsi     |   2 +
> > > >    arch/arm64/boot/dts/arm/juno-motherboard.dtsi |   2 +
> > > >    4 files changed, 88 insertions(+), 80 deletions(-)
> > > > 
> > > > Hi Rob, Robin,
> > > > 
> > > > Let me know if this is correct fix for the issue I am seeing with linux-next
> > > > on Juno. This patch is generated with -b for ease of review. With lots of
> > > > indentation, actual delta is:
> > > > 
> > > > 4 files changed, 1274 insertions(+), 1266 deletions(-)
> > > 
> > > Other than a few nits - the GIC should probably be under the soc node as
> > > it's an MMIO device, while the clocks shouldn't; the dtsi's could probably
> > > avoid churn with a "&soc {...}" phandle - I think this is a reasonable thing
> > > to do, as it's generally the preferred structure.
> > > 
> > 
> > I agree and am still in confusion as what to put inside soc or not.
> 
> FWIW my understanding is that the "soc" node is used to provide a 'bus' to
> represent on-chip MMIO - in principle we could nerd out and describe the
> ACE-lite/AXI/APB slave interconnects in full, but there's really no benefit
> to going into that much detail - so everything with a "reg" representing a
> physical address goes inside it, while CPUs, clocks, firmware, regulators
> etc. sit in the root node 'outside the PA space', regardless of whether
> they're physically on-chip or not.
>

I saw few other DTs and they all keep interrupt-controller and timer
outside this "soc" node. Timer is sysreg based, so makes sense. GIC has
both sysreg and MMIO components so I was not sure if it's done like that
due to some dependency on how interrupt-parent property is used.
e.g. arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi

I expect even the motherboard IO shouldn't go inside it but again to avoid
churn I kept it inside.

I agree on firmware, clocks, regulators,..etc. I wanted to keep the churn
minimum and it was not completely aligned to the principles you mentioned
above. Anyways it's better to fix it in right manner for next cycle.

--
Regards,
Sudeep

Re: [PATCH] Revert "arm64: dts: juno: add dma-ranges property"

[Sudeep Holla]

On Thu, Nov 28, 2019 at 03:58:28PM +0000, Robin Murphy wrote:
> On 28/11/2019 3:42 pm, Sudeep Holla wrote:
> > This reverts commit 193d00a2b35ee3353813b4006a18131122087205.
> >
> > Commit 951d48855d86 ("of: Make of_dma_get_range() work on bus nodes")
> > reworked the logic such that of_dma_get_range() works correctly
> > starting from a bus node containing "dma-ranges".
> >
> > Since on Juno we don't have a SoC level bus node and "dma-ranges" is
> > present only in the root node, we get the following error:
> >
> > OF: translation of DMA address(0) to CPU address failed node(/sram@2e000000)
> > OF: translation of DMA address(0) to CPU address failed node(/uart@7ff80000)
> > ...
> > OF: translation of DMA address(0) to CPU address failed node(/mhu@2b1f0000)
> > OF: translation of DMA address(0) to CPU address failed node(/iommu@2b600000)
> > OF: translation of DMA address(0) to CPU address failed node(/iommu@2b600000)
> > OF: translation of DMA address(0) to CPU address failed node(/iommu@2b600000)
> >
> > So let's fix it by dropping the "dma-ranges" property for now. We can
> > add it later with a proper SoC bus node and moving all the devices that
> > belong there along with the "dma-ranges" if required.
>
> Acked-by: Robin Murphy <robin.murphy@arm.com>
>

Thanks.

> As mentioned before, this is fine since it doesn't represent any kind of
> device-visible restriction; it was only there for completeness, and we've
> since given in to the assumption that missing "dma-ranges" implies a 1:1
> mapping anyway.
>

Agreed.

--
Regards,
Sudeep