[PATCH] arm64: dts: imx8mm-venice: Fix PCI bus nodes

[PATCH] arm64: dts: imx8mm-venice: Fix PCI bus nodes

[Rob Herring]

The imx8mm-venice boards PCI bus nodes are a complete mess. The
unit-addresses are wrong. The PCI bridge nodes are incomplete missing
"device_type" and "ranges" and just wrong for "#address-cells" and
"#size-cells" values.

All of these issues are reported warnings if anyone bothered to pay
attention. Sigh.

The kernel may have happened to work because it only looks at "reg"
to assign the DT nodes to PCI devices. Based on that, I'm assuming "reg"
is correctly matching the devices present, and the unit-addresses are
wrong. Presumably the bootloader fills in "local-mac-address" with
something valid. Hopefully it too uses "reg" rather than the path.

Fixes: afb424b99e0f ("arm64: dts: imx8mm-venice*: add PCIe support")
Signed-off-by: Rob Herring <robh@kernel.org>
---
 .../dts/freescale/imx8mm-venice-gw72xx.dtsi   | 28 +++++++++++--------
 .../dts/freescale/imx8mm-venice-gw73xx.dtsi   | 28 +++++++++++--------
 .../dts/freescale/imx8mm-venice-gw7902.dts    | 12 ++++----
 3 files changed, 38 insertions(+), 30 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8mm-venice-gw72xx.dtsi b/arch/arm64/boot/dts/freescale/imx8mm-venice-gw72xx.dtsi
index 3a0a10e835a2..ff461b004dc5 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm-venice-gw72xx.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm-venice-gw72xx.dtsi
@@ -150,25 +150,29 @@ &pcie0 {
 				 <&clk IMX8MM_SYS_PLL2_250M>;
 	status = "okay";
 
-	pcie@0,0 {
+	pcie@0 {
+		device_type = "pci";
 		reg = <0x0000 0 0 0 0>;
-		#address-cells = <1>;
-		#size-cells = <0>;
+		#address-cells = <3>;
+		#size-cells = <2>;
+		ranges;
 
-		pcie@1,0 {
+		pcie@0 {
+			device_type = "pci";
 			reg = <0x0000 0 0 0 0>;
-			#address-cells = <1>;
-			#size-cells = <0>;
+			#address-cells = <3>;
+			#size-cells = <2>;
+			ranges;
 
-			pcie@2,3 {
+			pcie@3 {
+				device_type = "pci";
 				reg = <0x1800 0 0 0 0>;
-				#address-cells = <1>;
-				#size-cells = <0>;
+				#address-cells = <3>;
+				#size-cells = <2>;
+				ranges;
 
-				eth1: pcie@5,0 {
+				eth1: ethernet@0 {
 					reg = <0x0000 0 0 0 0>;
-					#address-cells = <1>;
-					#size-cells = <0>;
 
 					local-mac-address = [00 00 00 00 00 00];
 				};
diff --git a/arch/arm64/boot/dts/freescale/imx8mm-venice-gw73xx.dtsi b/arch/arm64/boot/dts/freescale/imx8mm-venice-gw73xx.dtsi
index d79fe9f62b95..6f5a6d91c95e 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm-venice-gw73xx.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm-venice-gw73xx.dtsi
@@ -177,25 +177,29 @@ &pcie0 {
 				 <&clk IMX8MM_SYS_PLL2_250M>;
 	status = "okay";
 
-	pcie@0,0 {
+	pcie@0 {
+		device_type = "pci";
 		reg = <0x0000 0 0 0 0>;
-		#address-cells = <1>;
-		#size-cells = <0>;
+		#address-cells = <3>;
+		#size-cells = <2>;
+		ranges;
 
-		pcie@1,0 {
+		pcie@0 {
+			device_type = "pci";
 			reg = <0x0000 0 0 0 0>;
-			#address-cells = <1>;
-			#size-cells = <0>;
+			#address-cells = <3>;
+			#size-cells = <2>;
+			ranges;
 
-			pcie@2,4 {
+			pcie@4 {
+				device_type = "pci";
 				reg = <0x2000 0 0 0 0>;
-				#address-cells = <1>;
-				#size-cells = <0>;
+				#address-cells = <3>;
+				#size-cells = <2>;
+				ranges;
 
-				eth1: pcie@6,0 {
+				eth1: ethernet@0 {
 					reg = <0x0000 0 0 0 0>;
-					#address-cells = <1>;
-					#size-cells = <0>;
 
 					local-mac-address = [00 00 00 00 00 00];
 				};
diff --git a/arch/arm64/boot/dts/freescale/imx8mm-venice-gw7902.dts b/arch/arm64/boot/dts/freescale/imx8mm-venice-gw7902.dts
index 06a394a41d7c..4bb22fdc5d2e 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm-venice-gw7902.dts
+++ b/arch/arm64/boot/dts/freescale/imx8mm-venice-gw7902.dts
@@ -633,15 +633,15 @@ &pcie0 {
 				 <&clk IMX8MM_SYS_PLL2_250M>;
 	status = "okay";
 
-	pcie@0,0 {
+	pcie@0 {
+		device_type = "pci";
 		reg = <0x0000 0 0 0 0>;
-		#address-cells = <1>;
-		#size-cells = <0>;
+		#address-cells = <3>;
+		#size-cells = <2>;
+		ranges;
 
-		eth1: pcie@1,0 {
+		eth1: ethernet@0 {
 			reg = <0x0000 0 0 0 0>;
-			#address-cells = <1>;
-			#size-cells = <0>;
 
 			local-mac-address = [00 00 00 00 00 00];
 		};
-- 
2.42.0

Re: [PATCH] arm64: dts: imx8mm-venice: Fix PCI bus nodes

[Fabio Estevam]

Hi Rob,

On Thu, Nov 30, 2023 at 4:18 PM Rob Herring <robh@kernel.org> wrote:
>
> The imx8mm-venice boards PCI bus nodes are a complete mess. The
> unit-addresses are wrong. The PCI bridge nodes are incomplete missing
> "device_type" and "ranges" and just wrong for "#address-cells" and
> "#size-cells" values.
>
> All of these issues are reported warnings if anyone bothered to pay
> attention. Sigh.

The warnings are gone in linux-next:

https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/arch/arm64/boot/dts/freescale?h=next-20231130&id=d61c5068729a76a6183a897bcad4bf26e8d3d674

Re: [PATCH] arm64: dts: imx8mm-venice: Fix PCI bus nodes

[Rob Herring]

On Thu, Nov 30, 2023 at 1:28 PM Fabio Estevam <festevam@gmail.com> wrote:
>
> Hi Rob,
>
> On Thu, Nov 30, 2023 at 4:18 PM Rob Herring <robh@kernel.org> wrote:
> >
> > The imx8mm-venice boards PCI bus nodes are a complete mess. The
> > unit-addresses are wrong. The PCI bridge nodes are incomplete missing
> > "device_type" and "ranges" and just wrong for "#address-cells" and
> > "#size-cells" values.
> >
> > All of these issues are reported warnings if anyone bothered to pay
> > attention. Sigh.
>
> The warnings are gone in linux-next:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/arch/arm64/boot/dts/freescale?h=next-20231130&id=d61c5068729a76a6183a897bcad4bf26e8d3d674

Linux-next is wrong. The ethernet device should have a node name of
'ethernet'. The 'pcie' node name and 'device_type = "pci"' is for PCI
buses/bridges only.

Rob

Re: [PATCH] arm64: dts: imx8mm-venice: Fix PCI bus nodes

[Tim Harvey]

On Thu, Nov 30, 2023 at 2:33 PM Rob Herring <robh@kernel.org> wrote:
>
> On Thu, Nov 30, 2023 at 1:28 PM Fabio Estevam <festevam@gmail.com> wrote:
> >
> > Hi Rob,
> >
> > On Thu, Nov 30, 2023 at 4:18 PM Rob Herring <robh@kernel.org> wrote:
> > >
> > > The imx8mm-venice boards PCI bus nodes are a complete mess. The
> > > unit-addresses are wrong. The PCI bridge nodes are incomplete missing
> > > "device_type" and "ranges" and just wrong for "#address-cells" and
> > > "#size-cells" values.
> > >
> > > All of these issues are reported warnings if anyone bothered to pay
> > > attention. Sigh.

Rob,

Sorry about that. At the time the dt was submitted there were still so
many dt warnings it wasn't very clear what was a legitimate issue and
the PCI bindings are not that easy to understand.

> >
> > The warnings are gone in linux-next:
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/arch/arm64/boot/dts/freescale?h=next-20231130&id=d61c5068729a76a6183a897bcad4bf26e8d3d674
>
> Linux-next is wrong. The ethernet device should have a node name of
> 'ethernet'. The 'pcie' node name and 'device_type = "pci"' is for PCI
> buses/bridges only.

So as Fabio has tried to fix this with a patch that landed in
linux-next this patch won't apply. I'll submit one that covers your
changes.

It's always been unfortunate to have to have this level of detail in a
device-tree just to allow boot firmware to populate the mac address of
a PCI ethernet device.

Best Regards,

Tim

Re: [PATCH] arm64: dts: imx8mm-venice: Fix PCI bus nodes

[Rob Herring]

On Tue, Dec 5, 2023 at 12:13 PM Tim Harvey <tharvey@gateworks.com> wrote:
>
> On Thu, Nov 30, 2023 at 2:33 PM Rob Herring <robh@kernel.org> wrote:
> >
> > On Thu, Nov 30, 2023 at 1:28 PM Fabio Estevam <festevam@gmail.com> wrote:
> > >
> > > Hi Rob,
> > >
> > > On Thu, Nov 30, 2023 at 4:18 PM Rob Herring <robh@kernel.org> wrote:
> > > >
> > > > The imx8mm-venice boards PCI bus nodes are a complete mess. The
> > > > unit-addresses are wrong. The PCI bridge nodes are incomplete missing
> > > > "device_type" and "ranges" and just wrong for "#address-cells" and
> > > > "#size-cells" values.
> > > >
> > > > All of these issues are reported warnings if anyone bothered to pay
> > > > attention. Sigh.
>
> Rob,
>
> Sorry about that. At the time the dt was submitted there were still so
> many dt warnings it wasn't very clear what was a legitimate issue and
> the PCI bindings are not that easy to understand.
>
> > >
> > > The warnings are gone in linux-next:
> > >
> > > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/arch/arm64/boot/dts/freescale?h=next-20231130&id=d61c5068729a76a6183a897bcad4bf26e8d3d674
> >
> > Linux-next is wrong. The ethernet device should have a node name of
> > 'ethernet'. The 'pcie' node name and 'device_type = "pci"' is for PCI
> > buses/bridges only.
>
> So as Fabio has tried to fix this with a patch that landed in
> linux-next this patch won't apply. I'll submit one that covers your
> changes.

Thanks.

> It's always been unfortunate to have to have this level of detail in a
> device-tree just to allow boot firmware to populate the mac address of
> a PCI ethernet device.

More unfortunate are incomplete h/w designs lacking the MAC address. :(

Not really any way around it I think if you want something that works
for any device and any number of devices.

Rob