ARM: dts: ventana: fix eth1 pci dev node

Properly add the PCI device node for the 2nd GigE port so that the device driver can get its MAC from DT. Note that the Ventana bootloader uses the ethernet1 alias to populate the MAC address by adding the local-mac-address property. Also remove the unnecesssary 'sky2' alias.

This is based on Shawn's for-next branch

Signed-off-by Tim Harvey <[email protected]>

---
 arch/arm/boot/dts/imx6q-gw5400-a.dts  | 40 +++++++++++++++++++++++++++++++----
 arch/arm/boot/dts/imx6qdl-gw53xx.dtsi | 39 ++++++++++++++++++++++++++++++----
 arch/arm/boot/dts/imx6qdl-gw54xx.dtsi | 39 ++++++++++++++++++++++++++++++----
 3 files changed, 106 insertions(+), 12 deletions(-)

arch/arm/boot/dts/imx6q-gw5400-a.dts +1 -1

index 902f983..5d2b912 100644
@@ -16,7 +16,7 @@
 	model = "Gateworks Ventana GW5400-A";
 	compatible = "gw,imx6q-gw5400-a", "gw,ventana", "fsl,imx6q";
 
-	/* these are used by bootloader for disabling nodes */
+	/* these are used by bootloader for configuring nodes */
 	aliases {
 		ethernet0 = &fec;
 		ethernet1 = &eth1;

arch/arm/boot/dts/imx6q-gw5400-a.dts +8 -0

+		/* 00:00.0 host-bridge */
+		#address-cells = <3>;
+		#size-cells = <2>;
+		device_type = "pci";
+		reg = <0x0 0 0 0 0>;
+
+		/*
+		 * GigE PCI dev node needs to be defined so that enet driver

arch/arm/boot/dts/imx6q-gw5400-a.dts +15 -0

+				/* 02:08.0 PCIe switch port */
+				#address-cells = <3>;
+				#size-cells = <2>;
+				device_type = "pci";
+				reg = <0x4000 0 0 0 0>;
+				eth1: pcie at 0,0 {
+					/* 08:00.0 GigE */
+					#address-cells = <3>;
+					#size-cells = <2>;
+					device_type = "pci";
+					reg = <0x0 0 0 0 0>;
+					compatible = "marvell,sky2";
+				};
+			};
+		};
 	};
 };
 

arch/arm/boot/dts/imx6qdl-gw53xx.dtsi +35 -4

index c8e5ae0..46a8582 100644
@@ -10,7 +10,7 @@
  */
 
 / {
-	/* these are used by bootloader for disabling nodes */
+	/* these are used by bootloader for configuring nodes */
 	aliases {
 		can0 = &can1;
 		ethernet0 = &fec;

@@ -19,7 +19,6 @@
 		led1 = &led1;
 		led2 = &led2;
 		nand = &gpmi;
-		sky2 = &eth1;
 		ssi0 = &ssi1;
 		usb0 = &usbh1;
 		usb1 = &usbotg;

@@ -503,8 +502,40 @@
 	reset-gpio = <&gpio1 29 0>;
 	status = "okay";
 
-	eth1: sky2 at 8 { /* MAC/PHY on bus 8 */
-		compatible = "marvell,sky2";
+	/*
+	 * GigE PCI dev node needs to be defined so that enet driver
+	 * can use it to obtain its boot-loader specified MAC
+	 */
+	pcie at 0,0 {
+		/* 00:00.0 root host-bridge */
+		#address-cells = <3>;
+		#size-cells = <2>;
+		device_type = "pci";
+		reg = <0x0 0 0 0 0>;
+
+		pcie at 0,0 {
+			/* 01:00.0 PCIe switch */
+			#address-cells = <3>;
+			#size-cells = <2>;
+			device_type = "pci";
+			reg = <0x0 0 0 0 0>;
+
+			pcie at 4,0 {
+				/* 02:04.0 PCIe switch port */
+				#address-cells = <3>;
+				#size-cells = <2>;
+				device_type = "pci";
+				reg = <0x2000 0 0 0 0>;
+				eth1: pcie at 0,0 {
+					/* 04:00.0 GigE */
+					#address-cells = <3>;
+					#size-cells = <2>;
+					device_type = "pci";
+					reg = <0x0 0 0 0 0>;
+					compatible = "marvell,sky2";
+				};
+			};
+		};
 	};
 };
 

arch/arm/boot/dts/imx6qdl-gw54xx.dtsi +35 -4

index 2795dfc..697aa67 100644
@@ -10,7 +10,7 @@
  */
 
 / {
-	/* these are used by bootloader for disabling nodes */
+	/* these are used by bootloader for configuring nodes */
 	aliases {
 		can0 = &can1;
 		ethernet0 = &fec;

@@ -19,7 +19,6 @@
 		led1 = &led1;
 		led2 = &led2;
 		nand = &gpmi;
-		sky2 = &eth1;
 		ssi0 = &ssi1;
 		usb0 = &usbh1;
 		usb1 = &usbotg;

@@ -525,8 +524,40 @@
 	reset-gpio = <&gpio1 29 0>;
 	status = "okay";
 
-	eth1: sky2 at 8 { /* MAC/PHY on bus 8 */
-		compatible = "marvell,sky2";
+	/*
+	 * GigE PCI dev node needs to be defined so that enet driver
+	 * can use it to obtain its boot-loader specified MAC
+	 */
+	pcie at 0,0 {
+		/* 00:00.0 root host-bridge */
+		#address-cells = <3>;
+		#size-cells = <2>;
+		device_type = "pci";
+		reg = <0x0 0 0 0 0>;
+
+		pcie at 0,0 {
+			/* 01:00.0 PCIe switch */
+			#address-cells = <3>;
+			#size-cells = <2>;
+			device_type = "pci";
+			reg = <0x0 0 0 0 0>;
+
+			pcie at 8,0 {
+				/* 02:08.0 PCIe switch port */
+				#address-cells = <3>;
+				#size-cells = <2>;
+				device_type = "pci";
+				reg = <0x4000 0 0 0 0>;
+				eth1: pcie at 0,0 {
+					/* 08:00.0 GigE */
+					#address-cells = <3>;
+					#size-cells = <2>;
+					device_type = "pci";
+					reg = <0x0 0 0 0 0>;
+					compatible = "marvell,sky2";
+				};
+			};
+		};
 	};
 };
 

Signature · 7 lines

1.8.3.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Quoted 6 lines · Tim Harvey

Properly add the PCI device node for the 2nd GigE port so that the device driver can get its MAC from DT. Note that the Ventana bootloader uses the ethernet1 alias to populate the MAC address by adding the local-mac-address property. Also remove the unnecesssary 'sky2' alias.

This is based on Shawn's for-next branch

This line shouldn't be necessarily in the commit log.

Quoted 17 lines

Signed-off-by Tim Harvey <[email protected]>

---
 arch/arm/boot/dts/imx6q-gw5400-a.dts  | 40 +++++++++++++++++++++++++++++++----
 arch/arm/boot/dts/imx6qdl-gw53xx.dtsi | 39 ++++++++++++++++++++++++++++++----
 arch/arm/boot/dts/imx6qdl-gw54xx.dtsi | 39 ++++++++++++++++++++++++++++++----
 3 files changed, 106 insertions(+), 12 deletions(-)

arch/arm/boot/dts/imx6q-gw5400-a.dts +0 -0

index 902f983..5d2b912 100644
@@ -16,7 +16,7 @@
      model = "Gateworks Ventana GW5400-A";
      compatible = "gw,imx6q-gw5400-a", "gw,ventana", "fsl,imx6q";
 

arch/arm/boot/dts/imx6q-gw5400-a.dts line 29

Quoted 11 lines

arch/arm/boot/dts/imx6q-gw5400-a.dts +2 -2

-     /* these are used by bootloader for disabling nodes */
+     /* these are used by bootloader for configuring nodes */
      aliases {
              ethernet0 = &fec;
              ethernet1 = &eth1;

@@ -26,7 +26,7 @@
              led0 = &led0;
              led1 = &led1;
              led2 = &led2;
-             sky2 = &eth1;
+

No need to add a new line.

Quoted 8 lines

arch/arm/boot/dts/imx6q-gw5400-a.dts +0 -0

              ssi0 = &ssi1;
              spi0 = &ecspi1;
              usb0 = &usbh1;

@@ -496,8 +496,40 @@
      reset-gpio = <&gpio1 29 0>;
      status = "okay";
 

arch/arm/boot/dts/imx6q-gw5400-a.dts line 500

Quoted 2 lines

arch/arm/boot/dts/imx6q-gw5400-a.dts +0 -2

-     eth1: sky2 at 8 { /* MAC/PHY on bus 8 */
-             compatible = "marvell,sky2";

So this was just a placeholder and did not actually work in any way, right?

arch/arm/boot/dts/imx6q-gw5400-a.dts line 499

Quoted 2 lines

arch/arm/boot/dts/imx6q-gw5400-a.dts +1 -0

+     pcie at 0,0 {

Is this whole bridge/switch hierarchy binding documented somewhere or is this just something that work for you?

Quoted 9 lines

arch/arm/boot/dts/imx6q-gw5400-a.dts +8 -0

+             /* 00:00.0 host-bridge */
+             #address-cells = <3>;
+             #size-cells = <2>;
+             device_type = "pci";
+             reg = <0x0 0 0 0 0>;
+
+             /*
+              * GigE PCI dev node needs to be defined so that enet driver

arch/arm/boot/dts/imx6q-gw5400-a.dts line 517

Quoted 10 lines

arch/arm/boot/dts/imx6q-gw5400-a.dts +10 -0

+              * can use it to obtain its boot-loader specified MAC
+              */
+             pcie at 0,0 {
+                     /* 01:00.0 PCIe switch */
+                     #address-cells = <3>;
+                     #size-cells = <2>;
+                     device_type = "pci";
+                     reg = <0x0 0 0 0 0>;
+
+                     pcie at 8,0 {

What's the naming schema for all these pcie nodes? Generally, we should have the numbers encoded in the node name coming from the address cells in 'reg' property.

Shawn

Quoted 19 lines

arch/arm/boot/dts/imx6q-gw5400-a.dts +15 -0

+                             /* 02:08.0 PCIe switch port */
+                             #address-cells = <3>;
+                             #size-cells = <2>;
+                             device_type = "pci";
+                             reg = <0x4000 0 0 0 0>;
+                             eth1: pcie at 0,0 {
+                                     /* 08:00.0 GigE */
+                                     #address-cells = <3>;
+                                     #size-cells = <2>;
+                                     device_type = "pci";
+                                     reg = <0x0 0 0 0 0>;
+                                     compatible = "marvell,sky2";
+                             };
+                     };
+             };
      };
 };
 

Quoted 2 lines

Is this whole bridge/switch hierarchy binding documented somewhere or is this just something that work for you?

I'm not sure where its 'best' documented, but it is the way the kernel works.

Quoted 9 lines

Quoted 8 lines

arch/arm/boot/dts/imx6q-gw5400-a.dts +8 -0

+             pcie at 0,0 {
+                     /* 01:00.0 PCIe switch */
+                     #address-cells = <3>;
+                     #size-cells = <2>;
+                     device_type = "pci";
+                     reg = <0x0 0 0 0 0>;
+
+                     pcie at 8,0 {

Quoted 3 lines

What's the naming schema for all these pcie nodes? Generally, we should have the numbers encoded in the node name coming from the address cells in 'reg' property.

I was hoping there was a way to reference PCI nodes by BDF values as I'm simply trying to define a marvell,sky2 device at 08:00.0. I found that the kernel's OF parsing code for PCI requires you to nest the nodes to match the bus hierarchy. In order to map a dt node to a PCI device, the bus the device is on must have a dt node itself, which is what creates the need for the nesting. Note that the bus topology here rc -> P2P bridge -> GigE.

Quoted 5 lines

Quoted 5 lines

arch/arm/boot/dts/imx6q-gw5400-a.dts +5 -0

+                             /* 02:08.0 PCIe switch port */
+                             #address-cells = <3>;
+                             #size-cells = <2>;
+                             device_type = "pci";
+                             reg = <0x4000 0 0 0 0>;

Quoted 8 lines

arch/arm/boot/dts/imx6q-gw5400-a.dts line 530

Quoted 8 lines

arch/arm/boot/dts/imx6q-gw5400-a.dts +8 -0

+                             eth1: pcie at 0,0 {
+                                     /* 08:00.0 GigE */
+                                     #address-cells = <3>;
+                                     #size-cells = <2>;
+                                     device_type = "pci";
+                                     reg = <0x0 0 0 0 0>;
+                                     compatible = "marvell,sky2";
+                             };

Quoted 5 lines · Tim Harvey

Quoted 2 lines

Is this whole bridge/switch hierarchy binding documented somewhere or is this just something that work for you?

I'm not sure where its 'best' documented, but it is the way the kernel works.

It is documented in the 'PCI Bus Binding to Open Firware' publication from IEEE.

Quoted 9 lines

Quoted 9 lines

Quoted 8 lines

arch/arm/boot/dts/imx6q-gw5400-a.dts +8 -0

+             pcie at 0,0 {
+                     /* 01:00.0 PCIe switch */
+                     #address-cells = <3>;
+                     #size-cells = <2>;
+                     device_type = "pci";
+                     reg = <0x0 0 0 0 0>;
+
+                     pcie at 8,0 {

Quoted 3 lines

Quoted 3 lines

What's the naming schema for all these pcie nodes? Generally, we should have the numbers encoded in the node name coming from the address cells in 'reg' property.

The 'reg' property for PCI encodes the device and function number, and the suffix in the device path is of the form @DEVICE,FUNCTION (see 2.2.1.3 of the spec)

So device=8, function=0 is @8,0 and reg = 0x4000.