[PATCH 0/3] Add AM64x ICSSG Ethernet support

[PATCH 0/3] Add AM64x ICSSG Ethernet support

[MD Danish Anwar]

Hi All,

This series adds support for ICSSG ethernet on AM64x. 
This series is based on the latest next-20231207 linux-next.

AM64x EVM has three ethernet ports. One is dedicated to CPSW and one is
dedicated to ICSSG1. The remaining port is muxed between CPSW and ICSSG1
ICSSG1 ports. The ICSSG1 node is added in the k3-am642-evm.dts. By default
the muxed port is used by CPSW so 2nd ICSSG1 port is disabled in the
k3-am642-evm.dts. But overlay k3-am642-evm-icssg1-dualemac.dtso can be
applied to use muxed port as ICSSG1.

Thanks and Regards,
MD Danish Anwar

MD Danish Anwar (2):
  arm64: dts: ti: k3-am642-evm: add ICSSG1 Ethernet support
  arm64: dts: ti: k3-am642-evm: add overlay for icssg1 2nd port

Suman Anna (1):
  arm64: dts: ti: k3-am64-main: Add ICSSG IEP nodes

 arch/arm64/boot/dts/ti/Makefile               |   2 +
 arch/arm64/boot/dts/ti/k3-am64-main.dtsi      |  24 ++++
 .../dts/ti/k3-am642-evm-icssg1-dualemac.dtso  |  80 +++++++++++++
 arch/arm64/boot/dts/ti/k3-am642-evm.dts       | 107 +++++++++++++++++-
 4 files changed, 212 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm64/boot/dts/ti/k3-am642-evm-icssg1-dualemac.dtso

base-commit: 8e00ce02066e8f6f1ad5eab49a2ede7bf7a5ef64
-- 
2.34.1

[PATCH 1/3] arm64: dts: ti: k3-am64-main: Add ICSSG IEP nodes

[MD Danish Anwar]

From: Suman Anna <s-anna@ti.com>

The ICSSG IP on AM64x SoCs have two Industrial Ethernet Peripherals (IEPs)
to manage/generate Industrial Ethernet functions such as time stamping.
Each IEP sub-module is sourced from an internal clock mux that can be
derived from either of the IP instance's ICSSG_IEP_GCLK or from another
internal ICSSG CORE_CLK mux. Add both the IEP nodes for both the ICSSG
instances. The IEP clock is currently configured to be derived
indirectly from the ICSSG_ICLK running at 250 MHz.

Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com>
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
Signed-off-by: Suman Anna <s-anna@ti.com>
Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
---
 arch/arm64/boot/dts/ti/k3-am64-main.dtsi | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/arch/arm64/boot/dts/ti/k3-am64-main.dtsi b/arch/arm64/boot/dts/ti/k3-am64-main.dtsi
index c7be378492e2..055f6e3657c2 100644
--- a/arch/arm64/boot/dts/ti/k3-am64-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am64-main.dtsi
@@ -1234,6 +1234,18 @@ icssg0_iepclk_mux: iepclk-mux@30 {
 			};
 		};
 
+		icssg0_iep0: iep@2e000 {
+			compatible = "ti,am654-icss-iep";
+			reg = <0x2e000 0x1000>;
+			clocks = <&icssg0_iepclk_mux>;
+		};
+
+		icssg0_iep1: iep@2f000 {
+			compatible = "ti,am654-icss-iep";
+			reg = <0x2f000 0x1000>;
+			clocks = <&icssg0_iepclk_mux>;
+		};
+
 		icssg0_mii_rt: mii-rt@32000 {
 			compatible = "ti,pruss-mii", "syscon";
 			reg = <0x32000 0x100>;
@@ -1375,6 +1387,18 @@ icssg1_iepclk_mux: iepclk-mux@30 {
 			};
 		};
 
+		icssg1_iep0: iep@2e000 {
+			compatible = "ti,am654-icss-iep";
+			reg = <0x2e000 0x1000>;
+			clocks = <&icssg1_iepclk_mux>;
+		};
+
+		icssg1_iep1: iep@2f000 {
+			compatible = "ti,am654-icss-iep";
+			reg = <0x2f000 0x1000>;
+			clocks = <&icssg1_iepclk_mux>;
+		};
+
 		icssg1_mii_rt: mii-rt@32000 {
 			compatible = "ti,pruss-mii", "syscon";
 			reg = <0x32000 0x100>;
-- 
2.34.1

[PATCH 2/3] arm64: dts: ti: k3-am642-evm: add ICSSG1 Ethernet support

[MD Danish Anwar]

ICSSG1 provides dual Gigabit Ethernet support with proper FW loaded.

The ICSSG1 MII0 (RGMII1) has DP83869 PHY attached to it. The ICSSG1 shares
MII1 (RGMII2) PHY DP83869 with CPSW3g and it's assigned by default to
CPSW3g. The MDIO access to MII1 (RGMII2) PHY DP83869 is controlled by MDIO
bus switch and also assigned to CPSW3g. Therefore the ICSSG1 MII1 (RGMII2)
port is kept disable and ICSSG1 is enabled in single MAC mode by
default.

Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
---
 arch/arm64/boot/dts/ti/k3-am642-evm.dts | 105 ++++++++++++++++++++++++
 1 file changed, 105 insertions(+)

diff --git a/arch/arm64/boot/dts/ti/k3-am642-evm.dts b/arch/arm64/boot/dts/ti/k3-am642-evm.dts
index 8c5651d2cf5d..04d1c0602d31 100644
--- a/arch/arm64/boot/dts/ti/k3-am642-evm.dts
+++ b/arch/arm64/boot/dts/ti/k3-am642-evm.dts
@@ -34,6 +34,11 @@ aliases {
 		ethernet1 = &cpsw_port2;
 	};
 
+	aliases {
+		ethernet2 = &icssg1_emac0;
+		ethernet3 = &icssg1_emac1;
+	};
+
 	memory@80000000 {
 		bootph-all;
 		device_type = "memory";
@@ -229,6 +234,70 @@ transceiver2: can-phy1 {
 		max-bitrate = <5000000>;
 		standby-gpios = <&exp1 9 GPIO_ACTIVE_HIGH>;
 	};
+
+	icssg1_eth: icssg1-eth {
+		compatible = "ti,am642-icssg-prueth";
+		pinctrl-names = "default";
+		pinctrl-0 = <&icssg1_rgmii1_pins_default>;
+
+		sram = <&oc_sram>;
+		ti,prus = <&pru1_0>, <&rtu1_0>, <&tx_pru1_0>, <&pru1_1>, <&rtu1_1>, <&tx_pru1_1>;
+		firmware-name = "ti-pruss/am65x-sr2-pru0-prueth-fw.elf",
+				"ti-pruss/am65x-sr2-rtu0-prueth-fw.elf",
+				"ti-pruss/am65x-sr2-txpru0-prueth-fw.elf",
+				"ti-pruss/am65x-sr2-pru1-prueth-fw.elf",
+				"ti-pruss/am65x-sr2-rtu1-prueth-fw.elf",
+				"ti-pruss/am65x-sr2-txpru1-prueth-fw.elf";
+
+		ti,pruss-gp-mux-sel = <2>,	/* MII mode */
+				      <2>,
+				      <2>,
+				      <2>,	/* MII mode */
+				      <2>,
+				      <2>;
+
+		ti,mii-g-rt = <&icssg1_mii_g_rt>;
+		ti,mii-rt = <&icssg1_mii_rt>;
+		ti,iep = <&icssg1_iep0>,  <&icssg1_iep1>;
+
+		interrupt-parent = <&icssg1_intc>;
+		interrupts = <24 0 2>, <25 1 3>;
+		interrupt-names = "tx_ts0", "tx_ts1";
+
+		dmas = <&main_pktdma 0xc200 15>, /* egress slice 0 */
+		       <&main_pktdma 0xc201 15>, /* egress slice 0 */
+		       <&main_pktdma 0xc202 15>, /* egress slice 0 */
+		       <&main_pktdma 0xc203 15>, /* egress slice 0 */
+		       <&main_pktdma 0xc204 15>, /* egress slice 1 */
+		       <&main_pktdma 0xc205 15>, /* egress slice 1 */
+		       <&main_pktdma 0xc206 15>, /* egress slice 1 */
+		       <&main_pktdma 0xc207 15>, /* egress slice 1 */
+		       <&main_pktdma 0x4200 15>, /* ingress slice 0 */
+		       <&main_pktdma 0x4201 15>; /* ingress slice 1 */
+		dma-names = "tx0-0", "tx0-1", "tx0-2", "tx0-3",
+			    "tx1-0", "tx1-1", "tx1-2", "tx1-3",
+			    "rx0", "rx1";
+
+		ethernet-ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			icssg1_emac0: port@0 {
+				reg = <0>;
+				phy-handle = <&icssg1_phy1>;
+				phy-mode = "rgmii-id";
+				ti,syscon-rgmii-delay = <&main_conf 0x4110>;
+				/* Filled in by bootloader */
+				local-mac-address = [00 00 00 00 00 00];
+			};
+			icssg1_emac1: port@1 {
+				reg = <1>;
+				ti,syscon-rgmii-delay = <&main_conf 0x4114>;
+				/* Filled in by bootloader */
+				local-mac-address = [00 00 00 00 00 00];
+				status = "disabled";
+			};
+		};
+	};
 };
 
 &main_pmx0 {
@@ -383,6 +452,30 @@ ddr_vtt_pins_default: ddr-vtt-default-pins {
 			AM64X_IOPAD(0x0030, PIN_OUTPUT_PULLUP, 7) /* (L18) OSPI0_CSN1.GPIO0_12 */
 		>;
 	};
+
+	icssg1_mdio1_pins_default: icssg1-mdio1-default-pins {
+		pinctrl-single,pins = <
+			AM64X_IOPAD(0x015c, PIN_OUTPUT, 0) /* (Y6) PRG1_MDIO0_MDC */
+			AM64X_IOPAD(0x0158, PIN_INPUT, 0) /* (AA6) PRG1_MDIO0_MDIO */
+		>;
+	};
+
+	icssg1_rgmii1_pins_default: icssg1-rgmii1-default-pins{
+		pinctrl-single,pins = <
+			AM64X_IOPAD(0x00b8, PIN_INPUT, 2) /* (Y7) PRG1_PRU0_GPO0.PRG1_RGMII1_RD0 */
+			AM64X_IOPAD(0x00bc, PIN_INPUT, 2) /* (U8) PRG1_PRU0_GPO1.PRG1_RGMII1_RD1 */
+			AM64X_IOPAD(0x00c0, PIN_INPUT, 2) /* (W8) PRG1_PRU0_GPO2.PRG1_RGMII1_RD2 */
+			AM64X_IOPAD(0x00c4, PIN_INPUT, 2) /* (V8) PRG1_PRU0_GPO3.PRG1_RGMII1_RD3 */
+			AM64X_IOPAD(0x00d0, PIN_INPUT, 2) /* (AA7) PRG1_PRU0_GPO6.PRG1_RGMII1_RXC */
+			AM64X_IOPAD(0x00c8, PIN_INPUT, 2) /* (Y8) PRG1_PRU0_GPO4.PRG1_RGMII1_RX_CTL */
+			AM64X_IOPAD(0x00e4, PIN_INPUT, 2) /* (AA8) PRG1_PRU0_GPO11.PRG1_RGMII1_TD0 */
+			AM64X_IOPAD(0x00e8, PIN_INPUT, 2) /* (U9) PRG1_PRU0_GPO12.PRG1_RGMII1_TD1 */
+			AM64X_IOPAD(0x00ec, PIN_INPUT, 2) /* (W9) PRG1_PRU0_GPO13.PRG1_RGMII1_TD2 */
+			AM64X_IOPAD(0x00f0, PIN_INPUT, 2) /* (AA9) PRG1_PRU0_GPO14.PRG1_RGMII1_TD3 */
+			AM64X_IOPAD(0x00f8, PIN_INPUT, 2) /* (V9) PRG1_PRU0_GPO16.PRG1_RGMII1_TXC */
+			AM64X_IOPAD(0x00f4, PIN_INPUT, 2) /* (Y9) PRG1_PRU0_GPO15.PRG1_RGMII1_TX_CTL */
+		>;
+	};
 };
 
 &main_uart0 {
@@ -731,3 +824,15 @@ &main_mcan1 {
 	pinctrl-0 = <&main_mcan1_pins_default>;
 	phys = <&transceiver2>;
 };
+
+&icssg1_mdio {
+	status = "okay";
+	pinctrl-names = "default";
+	pinctrl-0 = <&icssg1_mdio1_pins_default>;
+
+	icssg1_phy1: ethernet-phy@0 {
+		reg = <0xf>;
+		tx-internal-delay-ps = <250>;
+		rx-internal-delay-ps = <2000>;
+	};
+};
-- 
2.34.1

[PATCH 3/3] arm64: dts: ti: k3-am642-evm: add overlay for icssg1 2nd port

[MD Danish Anwar]

The am642-evm doesn't allow to enable 2 x CPSW3g ports and 2 x ICSSG1 ports
all together, so base k3-am642-evm.dts enables by default 2 x CPSW3g ports
and 1 x ICSSG1 ports, but it also possible to support 1 x CPSW3g ports and
2 x ICSSG1 ports configuration.

This patch adds overlay to support 1 x CPSW3g ports and 2 x ICSSG1 ports
configuration:
- disable 2nd CPSW3g port
- update CPSW3g pinmuxes to not use RGMII2
- disable mdio-mux-1 and define mdio-mux-2 to route ICSSG1 MDIO to the
  shared DP83869 PHY
- add and enable ICSSG1 RGMII2 pinmuxes
- enable ICSSG1 MII1 port

Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
---
 arch/arm64/boot/dts/ti/Makefile               |  2 +
 .../dts/ti/k3-am642-evm-icssg1-dualemac.dtso  | 80 +++++++++++++++++++
 arch/arm64/boot/dts/ti/k3-am642-evm.dts       |  2 +-
 3 files changed, 83 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm64/boot/dts/ti/k3-am642-evm-icssg1-dualemac.dtso

diff --git a/arch/arm64/boot/dts/ti/Makefile b/arch/arm64/boot/dts/ti/Makefile
index 5ef49b02c71f..99a4dce47f02 100644
--- a/arch/arm64/boot/dts/ti/Makefile
+++ b/arch/arm64/boot/dts/ti/Makefile
@@ -35,12 +35,14 @@ dtb-$(CONFIG_ARCH_K3) += k3-am62x-sk-csi2-imx219.dtbo
 dtb-$(CONFIG_ARCH_K3) += k3-am62x-sk-hdmi-audio.dtbo
 
 # Boards with AM64x SoC
+k3-am642-evm-icssg1-dtbs := k3-am642-evm.dtb k3-am642-evm-icssg1-dualemac.dtbo
 dtb-$(CONFIG_ARCH_K3) += k3-am642-evm.dtb
 dtb-$(CONFIG_ARCH_K3) += k3-am642-phyboard-electra-rdk.dtb
 dtb-$(CONFIG_ARCH_K3) += k3-am642-sk.dtb
 dtb-$(CONFIG_ARCH_K3) += k3-am642-tqma64xxl-mbax4xxl.dtb
 dtb-$(CONFIG_ARCH_K3) += k3-am64-tqma64xxl-mbax4xxl-sdcard.dtbo
 dtb-$(CONFIG_ARCH_K3) += k3-am64-tqma64xxl-mbax4xxl-wlan.dtbo
+dtb-$(CONFIG_ARCH_K3) += k3-am642-evm-icssg1.dtb
 
 # Boards with AM65x SoC
 k3-am654-gp-evm-dtbs := k3-am654-base-board.dtb k3-am654-base-board-rocktech-rk101-panel.dtbo
diff --git a/arch/arm64/boot/dts/ti/k3-am642-evm-icssg1-dualemac.dtso b/arch/arm64/boot/dts/ti/k3-am642-evm-icssg1-dualemac.dtso
new file mode 100644
index 000000000000..6f33290c1ad6
--- /dev/null
+++ b/arch/arm64/boot/dts/ti/k3-am642-evm-icssg1-dualemac.dtso
@@ -0,0 +1,80 @@
+// SPDX-License-Identifier: GPL-2.0
+/**
+ * DT overlay for enabling 2nd ICSSG1 port on AM642 EVM
+ *
+ * Copyright (C) 2023 Texas Instruments Incorporated - https://www.ti.com/
+ */
+
+/dts-v1/;
+/plugin/;
+
+#include <dt-bindings/gpio/gpio.h>
+#include "k3-pinctrl.h"
+
+&{/} {
+	aliases {
+		ethernet1 = "/icssg1-eth/ethernet-ports/port@0";
+		ethernet2 = "/icssg1-eth/ethernet-ports/port@1";
+	};
+
+	mdio-mux-2 {
+		compatible = "mdio-mux-multiplexer";
+		mux-controls = <&mdio_mux>;
+		mdio-parent-bus = <&icssg1_mdio>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		mdio@0 {
+			reg = <0x0>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			icssg1_phy2: ethernet-phy@3 {
+				reg = <3>;
+				tx-internal-delay-ps = <250>;
+				rx-internal-delay-ps = <2000>;
+			};
+		};
+	};
+};
+
+&main_pmx0 {
+	icssg1_rgmii2_pins_default: icssg1-rgmii2-default-pins {
+		pinctrl-single,pins = <
+			AM64X_IOPAD(0x0108, PIN_INPUT, 2) /* (W11) PRG1_PRU1_GPO0.RGMII2_RD0 */
+			AM64X_IOPAD(0x010c, PIN_INPUT, 2) /* (V11) PRG1_PRU1_GPO1.RGMII2_RD1 */
+			AM64X_IOPAD(0x0110, PIN_INPUT, 2) /* (AA12) PRG1_PRU1_GPO2.RGMII2_RD2 */
+			AM64X_IOPAD(0x0114, PIN_INPUT, 2) /* (Y12) PRG1_PRU1_GPO3.RGMII2_RD3 */
+			AM64X_IOPAD(0x0120, PIN_INPUT, 2) /* (U11) PRG1_PRU1_GPO6.RGMII2_RXC */
+			AM64X_IOPAD(0x0118, PIN_INPUT, 2) /* (W12) PRG1_PRU1_GPO4.RGMII2_RX_CTL */
+			AM64X_IOPAD(0x0134, PIN_OUTPUT, 2) /* (AA10) PRG1_PRU1_GPO11.RGMII2_TD0 */
+			AM64X_IOPAD(0x0138, PIN_OUTPUT, 2) /* (V10) PRG1_PRU1_GPO12.RGMII2_TD1 */
+			AM64X_IOPAD(0x013c, PIN_OUTPUT, 2) /* (U10) PRG1_PRU1_GPO13.RGMII2_TD2 */
+			AM64X_IOPAD(0x0140, PIN_OUTPUT, 2) /* (AA11) PRG1_PRU1_GPO14.RGMII2_TD3 */
+			AM64X_IOPAD(0x0148, PIN_OUTPUT, 2) /* (Y10) PRG1_PRU1_GPO16.RGMII2_TXC */
+			AM64X_IOPAD(0x0144, PIN_OUTPUT, 2) /* (Y11) PRG1_PRU1_GPO15.RGMII2_TX_CTL */
+		>;
+	};
+};
+
+&cpsw3g {
+	pinctrl-0 = <&rgmii1_pins_default>;
+};
+
+&cpsw_port2 {
+	status = "disabled";
+};
+
+&mdio_mux_1 {
+	status = "disabled";
+};
+
+&icssg1_eth {
+	pinctrl-0 = <&icssg1_rgmii1_pins_default &icssg1_rgmii2_pins_default>;
+};
+
+&icssg1_emac1 {
+	status = "okay";
+	phy-handle = <&icssg1_phy2>;
+	phy-mode = "rgmii-id";
+};
diff --git a/arch/arm64/boot/dts/ti/k3-am642-evm.dts b/arch/arm64/boot/dts/ti/k3-am642-evm.dts
index 04d1c0602d31..90867090e725 100644
--- a/arch/arm64/boot/dts/ti/k3-am642-evm.dts
+++ b/arch/arm64/boot/dts/ti/k3-am642-evm.dts
@@ -203,7 +203,7 @@ mdio_mux: mux-controller {
 		mux-gpios = <&exp1 12 GPIO_ACTIVE_HIGH>;
 	};
 
-	mdio-mux-1 {
+	mdio_mux_1: mdio-mux-1 {
 		compatible = "mdio-mux-multiplexer";
 		mux-controls = <&mdio_mux>;
 		mdio-parent-bus = <&cpsw3g_mdio>;
-- 
2.34.1

Re: [PATCH 2/3] arm64: dts: ti: k3-am642-evm: add ICSSG1 Ethernet support

[Nishanth Menon]

On 13:49-20231207, MD Danish Anwar wrote:
> ICSSG1 provides dual Gigabit Ethernet support with proper FW loaded.
> 
> The ICSSG1 MII0 (RGMII1) has DP83869 PHY attached to it. The ICSSG1 shares
> MII1 (RGMII2) PHY DP83869 with CPSW3g and it's assigned by default to
> CPSW3g. The MDIO access to MII1 (RGMII2) PHY DP83869 is controlled by MDIO
> bus switch and also assigned to CPSW3g. Therefore the ICSSG1 MII1 (RGMII2)
> port is kept disable and ICSSG1 is enabled in single MAC mode by
> default.
> 
> Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
> ---
>  arch/arm64/boot/dts/ti/k3-am642-evm.dts | 105 ++++++++++++++++++++++++
>  1 file changed, 105 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/ti/k3-am642-evm.dts b/arch/arm64/boot/dts/ti/k3-am642-evm.dts
> index 8c5651d2cf5d..04d1c0602d31 100644
> --- a/arch/arm64/boot/dts/ti/k3-am642-evm.dts
> +++ b/arch/arm64/boot/dts/ti/k3-am642-evm.dts
> @@ -34,6 +34,11 @@ aliases {
>  		ethernet1 = &cpsw_port2;
>  	};
>  
> +	aliases {
> +		ethernet2 = &icssg1_emac0;
> +		ethernet3 = &icssg1_emac1;
> +	};

Why another aliases section?

> +
>  	memory@80000000 {
>  		bootph-all;
>  		device_type = "memory";
> @@ -229,6 +234,70 @@ transceiver2: can-phy1 {
>  		max-bitrate = <5000000>;
>  		standby-gpios = <&exp1 9 GPIO_ACTIVE_HIGH>;
>  	};
> +
> +	icssg1_eth: icssg1-eth {
> +		compatible = "ti,am642-icssg-prueth";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&icssg1_rgmii1_pins_default>;
> +
> +		sram = <&oc_sram>;
> +		ti,prus = <&pru1_0>, <&rtu1_0>, <&tx_pru1_0>, <&pru1_1>, <&rtu1_1>, <&tx_pru1_1>;
> +		firmware-name = "ti-pruss/am65x-sr2-pru0-prueth-fw.elf",
> +				"ti-pruss/am65x-sr2-rtu0-prueth-fw.elf",
> +				"ti-pruss/am65x-sr2-txpru0-prueth-fw.elf",
> +				"ti-pruss/am65x-sr2-pru1-prueth-fw.elf",
> +				"ti-pruss/am65x-sr2-rtu1-prueth-fw.elf",
> +				"ti-pruss/am65x-sr2-txpru1-prueth-fw.elf";

Umm... am65x??? is that a typo? I'd rather keep it am64x here and drop
that sr2 thing. Tomorrow there will be a custom bug on am64 and then we
will have to respin this again.

> +
> +		ti,pruss-gp-mux-sel = <2>,	/* MII mode */
> +				      <2>,
> +				      <2>,
> +				      <2>,	/* MII mode */
> +				      <2>,
> +				      <2>;
> +
> +		ti,mii-g-rt = <&icssg1_mii_g_rt>;
> +		ti,mii-rt = <&icssg1_mii_rt>;
> +		ti,iep = <&icssg1_iep0>,  <&icssg1_iep1>;
> +
> +		interrupt-parent = <&icssg1_intc>;
> +		interrupts = <24 0 2>, <25 1 3>;
> +		interrupt-names = "tx_ts0", "tx_ts1";
> +
> +		dmas = <&main_pktdma 0xc200 15>, /* egress slice 0 */
> +		       <&main_pktdma 0xc201 15>, /* egress slice 0 */
> +		       <&main_pktdma 0xc202 15>, /* egress slice 0 */
> +		       <&main_pktdma 0xc203 15>, /* egress slice 0 */
> +		       <&main_pktdma 0xc204 15>, /* egress slice 1 */
> +		       <&main_pktdma 0xc205 15>, /* egress slice 1 */
> +		       <&main_pktdma 0xc206 15>, /* egress slice 1 */
> +		       <&main_pktdma 0xc207 15>, /* egress slice 1 */
> +		       <&main_pktdma 0x4200 15>, /* ingress slice 0 */
> +		       <&main_pktdma 0x4201 15>; /* ingress slice 1 */
> +		dma-names = "tx0-0", "tx0-1", "tx0-2", "tx0-3",
> +			    "tx1-0", "tx1-1", "tx1-2", "tx1-3",
> +			    "rx0", "rx1";
> +
> +		ethernet-ports {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			icssg1_emac0: port@0 {
> +				reg = <0>;
> +				phy-handle = <&icssg1_phy1>;
> +				phy-mode = "rgmii-id";
> +				ti,syscon-rgmii-delay = <&main_conf 0x4110>;
> +				/* Filled in by bootloader */
> +				local-mac-address = [00 00 00 00 00 00];
> +			};
> +			icssg1_emac1: port@1 {
> +				reg = <1>;
> +				ti,syscon-rgmii-delay = <&main_conf 0x4114>;
> +				/* Filled in by bootloader */
> +				local-mac-address = [00 00 00 00 00 00];
> +				status = "disabled";
> +			};
> +		};
> +	};
>  };
>  
>  &main_pmx0 {
> @@ -383,6 +452,30 @@ ddr_vtt_pins_default: ddr-vtt-default-pins {
>  			AM64X_IOPAD(0x0030, PIN_OUTPUT_PULLUP, 7) /* (L18) OSPI0_CSN1.GPIO0_12 */
>  		>;
>  	};
> +
> +	icssg1_mdio1_pins_default: icssg1-mdio1-default-pins {
> +		pinctrl-single,pins = <
> +			AM64X_IOPAD(0x015c, PIN_OUTPUT, 0) /* (Y6) PRG1_MDIO0_MDC */
> +			AM64X_IOPAD(0x0158, PIN_INPUT, 0) /* (AA6) PRG1_MDIO0_MDIO */
> +		>;
> +	};
> +
> +	icssg1_rgmii1_pins_default: icssg1-rgmii1-default-pins{
> +		pinctrl-single,pins = <
> +			AM64X_IOPAD(0x00b8, PIN_INPUT, 2) /* (Y7) PRG1_PRU0_GPO0.PRG1_RGMII1_RD0 */
> +			AM64X_IOPAD(0x00bc, PIN_INPUT, 2) /* (U8) PRG1_PRU0_GPO1.PRG1_RGMII1_RD1 */
> +			AM64X_IOPAD(0x00c0, PIN_INPUT, 2) /* (W8) PRG1_PRU0_GPO2.PRG1_RGMII1_RD2 */
> +			AM64X_IOPAD(0x00c4, PIN_INPUT, 2) /* (V8) PRG1_PRU0_GPO3.PRG1_RGMII1_RD3 */
> +			AM64X_IOPAD(0x00d0, PIN_INPUT, 2) /* (AA7) PRG1_PRU0_GPO6.PRG1_RGMII1_RXC */
> +			AM64X_IOPAD(0x00c8, PIN_INPUT, 2) /* (Y8) PRG1_PRU0_GPO4.PRG1_RGMII1_RX_CTL */
> +			AM64X_IOPAD(0x00e4, PIN_INPUT, 2) /* (AA8) PRG1_PRU0_GPO11.PRG1_RGMII1_TD0 */
> +			AM64X_IOPAD(0x00e8, PIN_INPUT, 2) /* (U9) PRG1_PRU0_GPO12.PRG1_RGMII1_TD1 */
> +			AM64X_IOPAD(0x00ec, PIN_INPUT, 2) /* (W9) PRG1_PRU0_GPO13.PRG1_RGMII1_TD2 */
> +			AM64X_IOPAD(0x00f0, PIN_INPUT, 2) /* (AA9) PRG1_PRU0_GPO14.PRG1_RGMII1_TD3 */
> +			AM64X_IOPAD(0x00f8, PIN_INPUT, 2) /* (V9) PRG1_PRU0_GPO16.PRG1_RGMII1_TXC */
> +			AM64X_IOPAD(0x00f4, PIN_INPUT, 2) /* (Y9) PRG1_PRU0_GPO15.PRG1_RGMII1_TX_CTL */
> +		>;
> +	};
>  };
>  
>  &main_uart0 {
> @@ -731,3 +824,15 @@ &main_mcan1 {
>  	pinctrl-0 = <&main_mcan1_pins_default>;
>  	phys = <&transceiver2>;
>  };
> +
> +&icssg1_mdio {
> +	status = "okay";
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&icssg1_mdio1_pins_default>;
> +
> +	icssg1_phy1: ethernet-phy@0 {
> +		reg = <0xf>;
> +		tx-internal-delay-ps = <250>;
> +		rx-internal-delay-ps = <2000>;
> +	};
> +};
> -- 
> 2.34.1
> 

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D

Re: [PATCH 3/3] arm64: dts: ti: k3-am642-evm: add overlay for icssg1 2nd port

[Nishanth Menon]

On 13:49-20231207, MD Danish Anwar wrote:
> The am642-evm doesn't allow to enable 2 x CPSW3g ports and 2 x ICSSG1 ports
> all together, so base k3-am642-evm.dts enables by default 2 x CPSW3g ports
> and 1 x ICSSG1 ports, but it also possible to support 1 x CPSW3g ports and
> 2 x ICSSG1 ports configuration.
> 
> This patch adds overlay to support 1 x CPSW3g ports and 2 x ICSSG1 ports
> configuration:
> - disable 2nd CPSW3g port
> - update CPSW3g pinmuxes to not use RGMII2
> - disable mdio-mux-1 and define mdio-mux-2 to route ICSSG1 MDIO to the
>   shared DP83869 PHY
> - add and enable ICSSG1 RGMII2 pinmuxes
> - enable ICSSG1 MII1 port
> 
> Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
> ---
>  arch/arm64/boot/dts/ti/Makefile               |  2 +
>  .../dts/ti/k3-am642-evm-icssg1-dualemac.dtso  | 80 +++++++++++++++++++
>  arch/arm64/boot/dts/ti/k3-am642-evm.dts       |  2 +-
>  3 files changed, 83 insertions(+), 1 deletion(-)
>  create mode 100644 arch/arm64/boot/dts/ti/k3-am642-evm-icssg1-dualemac.dtso
> 
> diff --git a/arch/arm64/boot/dts/ti/Makefile b/arch/arm64/boot/dts/ti/Makefile
> index 5ef49b02c71f..99a4dce47f02 100644
> --- a/arch/arm64/boot/dts/ti/Makefile
> +++ b/arch/arm64/boot/dts/ti/Makefile
> @@ -35,12 +35,14 @@ dtb-$(CONFIG_ARCH_K3) += k3-am62x-sk-csi2-imx219.dtbo
>  dtb-$(CONFIG_ARCH_K3) += k3-am62x-sk-hdmi-audio.dtbo
>  
>  # Boards with AM64x SoC
> +k3-am642-evm-icssg1-dtbs := k3-am642-evm.dtb k3-am642-evm-icssg1-dualemac.dtbo

Why not handle this for CONFIG_OF_ALL_DTBS alone? See commit
b0044823a6607e535fdb083c89f487fbf183b171

>  dtb-$(CONFIG_ARCH_K3) += k3-am642-evm.dtb
>  dtb-$(CONFIG_ARCH_K3) += k3-am642-phyboard-electra-rdk.dtb
>  dtb-$(CONFIG_ARCH_K3) += k3-am642-sk.dtb
>  dtb-$(CONFIG_ARCH_K3) += k3-am642-tqma64xxl-mbax4xxl.dtb
>  dtb-$(CONFIG_ARCH_K3) += k3-am64-tqma64xxl-mbax4xxl-sdcard.dtbo
>  dtb-$(CONFIG_ARCH_K3) += k3-am64-tqma64xxl-mbax4xxl-wlan.dtbo
> +dtb-$(CONFIG_ARCH_K3) += k3-am642-evm-icssg1.dtb
>  
>  # Boards with AM65x SoC
>  k3-am654-gp-evm-dtbs := k3-am654-base-board.dtb k3-am654-base-board-rocktech-rk101-panel.dtbo
> diff --git a/arch/arm64/boot/dts/ti/k3-am642-evm-icssg1-dualemac.dtso b/arch/arm64/boot/dts/ti/k3-am642-evm-icssg1-dualemac.dtso
> new file mode 100644
> index 000000000000..6f33290c1ad6
> --- /dev/null
> +++ b/arch/arm64/boot/dts/ti/k3-am642-evm-icssg1-dualemac.dtso
> @@ -0,0 +1,80 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/**
> + * DT overlay for enabling 2nd ICSSG1 port on AM642 EVM
> + *
> + * Copyright (C) 2023 Texas Instruments Incorporated - https://www.ti.com/
> + */
> +
> +/dts-v1/;
> +/plugin/;
> +
> +#include <dt-bindings/gpio/gpio.h>
> +#include "k3-pinctrl.h"
> +
> +&{/} {
> +	aliases {
> +		ethernet1 = "/icssg1-eth/ethernet-ports/port@0";
> +		ethernet2 = "/icssg1-eth/ethernet-ports/port@1";

I don't understand what you are overriding here. isn't patch #2 in your
series already introducing this in the base dts?

> +	};
> +
> +	mdio-mux-2 {
> +		compatible = "mdio-mux-multiplexer";
> +		mux-controls = <&mdio_mux>;
> +		mdio-parent-bus = <&icssg1_mdio>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		mdio@0 {
> +			reg = <0x0>;
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			icssg1_phy2: ethernet-phy@3 {
> +				reg = <3>;
> +				tx-internal-delay-ps = <250>;
> +				rx-internal-delay-ps = <2000>;
> +			};
> +		};
> +	};
> +};
> +
> +&main_pmx0 {
> +	icssg1_rgmii2_pins_default: icssg1-rgmii2-default-pins {
> +		pinctrl-single,pins = <
> +			AM64X_IOPAD(0x0108, PIN_INPUT, 2) /* (W11) PRG1_PRU1_GPO0.RGMII2_RD0 */
> +			AM64X_IOPAD(0x010c, PIN_INPUT, 2) /* (V11) PRG1_PRU1_GPO1.RGMII2_RD1 */
> +			AM64X_IOPAD(0x0110, PIN_INPUT, 2) /* (AA12) PRG1_PRU1_GPO2.RGMII2_RD2 */
> +			AM64X_IOPAD(0x0114, PIN_INPUT, 2) /* (Y12) PRG1_PRU1_GPO3.RGMII2_RD3 */
> +			AM64X_IOPAD(0x0120, PIN_INPUT, 2) /* (U11) PRG1_PRU1_GPO6.RGMII2_RXC */
> +			AM64X_IOPAD(0x0118, PIN_INPUT, 2) /* (W12) PRG1_PRU1_GPO4.RGMII2_RX_CTL */
> +			AM64X_IOPAD(0x0134, PIN_OUTPUT, 2) /* (AA10) PRG1_PRU1_GPO11.RGMII2_TD0 */
> +			AM64X_IOPAD(0x0138, PIN_OUTPUT, 2) /* (V10) PRG1_PRU1_GPO12.RGMII2_TD1 */
> +			AM64X_IOPAD(0x013c, PIN_OUTPUT, 2) /* (U10) PRG1_PRU1_GPO13.RGMII2_TD2 */
> +			AM64X_IOPAD(0x0140, PIN_OUTPUT, 2) /* (AA11) PRG1_PRU1_GPO14.RGMII2_TD3 */
> +			AM64X_IOPAD(0x0148, PIN_OUTPUT, 2) /* (Y10) PRG1_PRU1_GPO16.RGMII2_TXC */
> +			AM64X_IOPAD(0x0144, PIN_OUTPUT, 2) /* (Y11) PRG1_PRU1_GPO15.RGMII2_TX_CTL */
> +		>;
> +	};
> +};
> +
> +&cpsw3g {
> +	pinctrl-0 = <&rgmii1_pins_default>;
> +};
> +
> +&cpsw_port2 {
> +	status = "disabled";
> +};
> +
> +&mdio_mux_1 {
> +	status = "disabled";
> +};
> +
> +&icssg1_eth {
> +	pinctrl-0 = <&icssg1_rgmii1_pins_default &icssg1_rgmii2_pins_default>;

Grrr... No! I have been cleaning up after you folks and you folks should
take notice.

pinctrl-0 = <&icssg1_rgmii1_pins_default>, <&icssg1_rgmii2_pins_default>;


> +};
> +
> +&icssg1_emac1 {
> +	status = "okay";
> +	phy-handle = <&icssg1_phy2>;
> +	phy-mode = "rgmii-id";
> +};
> diff --git a/arch/arm64/boot/dts/ti/k3-am642-evm.dts b/arch/arm64/boot/dts/ti/k3-am642-evm.dts
> index 04d1c0602d31..90867090e725 100644
> --- a/arch/arm64/boot/dts/ti/k3-am642-evm.dts
> +++ b/arch/arm64/boot/dts/ti/k3-am642-evm.dts
> @@ -203,7 +203,7 @@ mdio_mux: mux-controller {
>  		mux-gpios = <&exp1 12 GPIO_ACTIVE_HIGH>;
>  	};
>  
> -	mdio-mux-1 {
> +	mdio_mux_1: mdio-mux-1 {

Commit message doesn't warn me for this change.
>  		compatible = "mdio-mux-multiplexer";
>  		mux-controls = <&mdio_mux>;
>  		mdio-parent-bus = <&cpsw3g_mdio>;
> -- 
> 2.34.1
> 

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D

Re: [PATCH 2/3] arm64: dts: ti: k3-am642-evm: add ICSSG1 Ethernet support

[Anwar, Md Danish]

On 12/7/2023 6:48 PM, Nishanth Menon wrote:
> On 13:49-20231207, MD Danish Anwar wrote:
>> ICSSG1 provides dual Gigabit Ethernet support with proper FW loaded.
>>
>> The ICSSG1 MII0 (RGMII1) has DP83869 PHY attached to it. The ICSSG1 shares
>> MII1 (RGMII2) PHY DP83869 with CPSW3g and it's assigned by default to
>> CPSW3g. The MDIO access to MII1 (RGMII2) PHY DP83869 is controlled by MDIO
>> bus switch and also assigned to CPSW3g. Therefore the ICSSG1 MII1 (RGMII2)
>> port is kept disable and ICSSG1 is enabled in single MAC mode by
>> default.
>>
>> Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
>> ---
>>  arch/arm64/boot/dts/ti/k3-am642-evm.dts | 105 ++++++++++++++++++++++++
>>  1 file changed, 105 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/ti/k3-am642-evm.dts b/arch/arm64/boot/dts/ti/k3-am642-evm.dts
>> index 8c5651d2cf5d..04d1c0602d31 100644
>> --- a/arch/arm64/boot/dts/ti/k3-am642-evm.dts
>> +++ b/arch/arm64/boot/dts/ti/k3-am642-evm.dts
>> @@ -34,6 +34,11 @@ aliases {
>>  		ethernet1 = &cpsw_port2;
>>  	};
>>  
>> +	aliases {
>> +		ethernet2 = &icssg1_emac0;
>> +		ethernet3 = &icssg1_emac1;
>> +	};
> 
> Why another aliases section?
> 

Sorry, My bad. I had created these patches a while back when there was
no alias section in this dts file, and applied them directly here. I
didn't notice the already existing alias section. I will remove this
aliases section and move these two aliases two the above section in v2.

>> +
>>  	memory@80000000 {
>>  		bootph-all;
>>  		device_type = "memory";
>> @@ -229,6 +234,70 @@ transceiver2: can-phy1 {
>>  		max-bitrate = <5000000>;
>>  		standby-gpios = <&exp1 9 GPIO_ACTIVE_HIGH>;
>>  	};
>> +
>> +	icssg1_eth: icssg1-eth {
>> +		compatible = "ti,am642-icssg-prueth";
>> +		pinctrl-names = "default";
>> +		pinctrl-0 = <&icssg1_rgmii1_pins_default>;
>> +
>> +		sram = <&oc_sram>;
>> +		ti,prus = <&pru1_0>, <&rtu1_0>, <&tx_pru1_0>, <&pru1_1>, <&rtu1_1>, <&tx_pru1_1>;
>> +		firmware-name = "ti-pruss/am65x-sr2-pru0-prueth-fw.elf",
>> +				"ti-pruss/am65x-sr2-rtu0-prueth-fw.elf",
>> +				"ti-pruss/am65x-sr2-txpru0-prueth-fw.elf",
>> +				"ti-pruss/am65x-sr2-pru1-prueth-fw.elf",
>> +				"ti-pruss/am65x-sr2-rtu1-prueth-fw.elf",
>> +				"ti-pruss/am65x-sr2-txpru1-prueth-fw.elf";
> 
> Umm... am65x??? is that a typo? I'd rather keep it am64x here and drop
> that sr2 thing. Tomorrow there will be a custom bug on am64 and then we
> will have to respin this again.
> 

No Nishant, this is not a typo. Both AM64x and AM65x use the same ICSSG
firmwares. We only have am65x-sr2-* firmwares and they are used by both
AM64x and AM65x and that is why I have kept the firmware-name here in dt
same as the files that we load on the pru cores.

>> +
>> +		ti,pruss-gp-mux-sel = <2>,	/* MII mode */
>> +				      <2>,
>> +				      <2>,
>> +				      <2>,	/* MII mode */
>> +				      <2>,
>> +				      <2>;
>> +
>> +		ti,mii-g-rt = <&icssg1_mii_g_rt>;
>> +		ti,mii-rt = <&icssg1_mii_rt>;
>> +		ti,iep = <&icssg1_iep0>,  <&icssg1_iep1>;
>> +
>> +		interrupt-parent = <&icssg1_intc>;
>> +		interrupts = <24 0 2>, <25 1 3>;
>> +		interrupt-names = "tx_ts0", "tx_ts1";
>> +
>> +		dmas = <&main_pktdma 0xc200 15>, /* egress slice 0 */
>> +		       <&main_pktdma 0xc201 15>, /* egress slice 0 */
>> +		       <&main_pktdma 0xc202 15>, /* egress slice 0 */
>> +		       <&main_pktdma 0xc203 15>, /* egress slice 0 */
>> +		       <&main_pktdma 0xc204 15>, /* egress slice 1 */
>> +		       <&main_pktdma 0xc205 15>, /* egress slice 1 */
>> +		       <&main_pktdma 0xc206 15>, /* egress slice 1 */
>> +		       <&main_pktdma 0xc207 15>, /* egress slice 1 */
>> +		       <&main_pktdma 0x4200 15>, /* ingress slice 0 */
>> +		       <&main_pktdma 0x4201 15>; /* ingress slice 1 */
>> +		dma-names = "tx0-0", "tx0-1", "tx0-2", "tx0-3",
>> +			    "tx1-0", "tx1-1", "tx1-2", "tx1-3",
>> +			    "rx0", "rx1";
>> +
>> +		ethernet-ports {
>> +			#address-cells = <1>;
>> +			#size-cells = <0>;
>> +			icssg1_emac0: port@0 {
>> +				reg = <0>;
>> +				phy-handle = <&icssg1_phy1>;
>> +				phy-mode = "rgmii-id";
>> +				ti,syscon-rgmii-delay = <&main_conf 0x4110>;
>> +				/* Filled in by bootloader */
>> +				local-mac-address = [00 00 00 00 00 00];
>> +			};
>> +			icssg1_emac1: port@1 {
>> +				reg = <1>;
>> +				ti,syscon-rgmii-delay = <&main_conf 0x4114>;
>> +				/* Filled in by bootloader */
>> +				local-mac-address = [00 00 00 00 00 00];
>> +				status = "disabled";
>> +			};
>> +		};
>> +	};
>>  };
>>  
>>  &main_pmx0 {
>> @@ -383,6 +452,30 @@ ddr_vtt_pins_default: ddr-vtt-default-pins {
>>  			AM64X_IOPAD(0x0030, PIN_OUTPUT_PULLUP, 7) /* (L18) OSPI0_CSN1.GPIO0_12 */
>>  		>;
>>  	};
>> +
>> +	icssg1_mdio1_pins_default: icssg1-mdio1-default-pins {
>> +		pinctrl-single,pins = <
>> +			AM64X_IOPAD(0x015c, PIN_OUTPUT, 0) /* (Y6) PRG1_MDIO0_MDC */
>> +			AM64X_IOPAD(0x0158, PIN_INPUT, 0) /* (AA6) PRG1_MDIO0_MDIO */
>> +		>;
>> +	};
>> +
>> +	icssg1_rgmii1_pins_default: icssg1-rgmii1-default-pins{
>> +		pinctrl-single,pins = <
>> +			AM64X_IOPAD(0x00b8, PIN_INPUT, 2) /* (Y7) PRG1_PRU0_GPO0.PRG1_RGMII1_RD0 */
>> +			AM64X_IOPAD(0x00bc, PIN_INPUT, 2) /* (U8) PRG1_PRU0_GPO1.PRG1_RGMII1_RD1 */
>> +			AM64X_IOPAD(0x00c0, PIN_INPUT, 2) /* (W8) PRG1_PRU0_GPO2.PRG1_RGMII1_RD2 */
>> +			AM64X_IOPAD(0x00c4, PIN_INPUT, 2) /* (V8) PRG1_PRU0_GPO3.PRG1_RGMII1_RD3 */
>> +			AM64X_IOPAD(0x00d0, PIN_INPUT, 2) /* (AA7) PRG1_PRU0_GPO6.PRG1_RGMII1_RXC */
>> +			AM64X_IOPAD(0x00c8, PIN_INPUT, 2) /* (Y8) PRG1_PRU0_GPO4.PRG1_RGMII1_RX_CTL */
>> +			AM64X_IOPAD(0x00e4, PIN_INPUT, 2) /* (AA8) PRG1_PRU0_GPO11.PRG1_RGMII1_TD0 */
>> +			AM64X_IOPAD(0x00e8, PIN_INPUT, 2) /* (U9) PRG1_PRU0_GPO12.PRG1_RGMII1_TD1 */
>> +			AM64X_IOPAD(0x00ec, PIN_INPUT, 2) /* (W9) PRG1_PRU0_GPO13.PRG1_RGMII1_TD2 */
>> +			AM64X_IOPAD(0x00f0, PIN_INPUT, 2) /* (AA9) PRG1_PRU0_GPO14.PRG1_RGMII1_TD3 */
>> +			AM64X_IOPAD(0x00f8, PIN_INPUT, 2) /* (V9) PRG1_PRU0_GPO16.PRG1_RGMII1_TXC */
>> +			AM64X_IOPAD(0x00f4, PIN_INPUT, 2) /* (Y9) PRG1_PRU0_GPO15.PRG1_RGMII1_TX_CTL */
>> +		>;
>> +	};
>>  };
>>  
>>  &main_uart0 {
>> @@ -731,3 +824,15 @@ &main_mcan1 {
>>  	pinctrl-0 = <&main_mcan1_pins_default>;
>>  	phys = <&transceiver2>;
>>  };
>> +
>> +&icssg1_mdio {
>> +	status = "okay";
>> +	pinctrl-names = "default";
>> +	pinctrl-0 = <&icssg1_mdio1_pins_default>;
>> +
>> +	icssg1_phy1: ethernet-phy@0 {
>> +		reg = <0xf>;
>> +		tx-internal-delay-ps = <250>;
>> +		rx-internal-delay-ps = <2000>;
>> +	};
>> +};
>> -- 
>> 2.34.1
>>
> 

-- 
Thanks and Regards,
Md Danish Anwar

Re: [PATCH 2/3] arm64: dts: ti: k3-am642-evm: add ICSSG1 Ethernet support

[Nishanth Menon]

On 18:58-20231207, Anwar, Md Danish wrote:
[...]
> >> +
> >>  	memory@80000000 {
> >>  		bootph-all;
> >>  		device_type = "memory";
> >> @@ -229,6 +234,70 @@ transceiver2: can-phy1 {
> >>  		max-bitrate = <5000000>;
> >>  		standby-gpios = <&exp1 9 GPIO_ACTIVE_HIGH>;
> >>  	};
> >> +
> >> +	icssg1_eth: icssg1-eth {
> >> +		compatible = "ti,am642-icssg-prueth";
> >> +		pinctrl-names = "default";
> >> +		pinctrl-0 = <&icssg1_rgmii1_pins_default>;
> >> +
> >> +		sram = <&oc_sram>;
> >> +		ti,prus = <&pru1_0>, <&rtu1_0>, <&tx_pru1_0>, <&pru1_1>, <&rtu1_1>, <&tx_pru1_1>;
> >> +		firmware-name = "ti-pruss/am65x-sr2-pru0-prueth-fw.elf",
> >> +				"ti-pruss/am65x-sr2-rtu0-prueth-fw.elf",
> >> +				"ti-pruss/am65x-sr2-txpru0-prueth-fw.elf",
> >> +				"ti-pruss/am65x-sr2-pru1-prueth-fw.elf",
> >> +				"ti-pruss/am65x-sr2-rtu1-prueth-fw.elf",
> >> +				"ti-pruss/am65x-sr2-txpru1-prueth-fw.elf";
> > 
> > Umm... am65x??? is that a typo? I'd rather keep it am64x here and drop
> > that sr2 thing. Tomorrow there will be a custom bug on am64 and then we
> > will have to respin this again.
> > 
> 
> No Nishant, this is not a typo. Both AM64x and AM65x use the same ICSSG
> firmwares. We only have am65x-sr2-* firmwares and they are used by both
> AM64x and AM65x and that is why I have kept the firmware-name here in dt
> same as the files that we load on the pru cores.
> 

SoCs are different. The hardware as a result is different as well. In
fact, you do have a different compatible to distinguish the two. Some
day, there will be an erratum that is different and we will be stuck
with abi breakage across distros. So, unless you can explain why this
scenario will never occur, I don't buy the argument this will survive
long term.

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D

Re: [PATCH 2/3] arm64: dts: ti: k3-am642-evm: add ICSSG1 Ethernet support

[Anwar, Md Danish]

On 12/7/2023 7:13 PM, Nishanth Menon wrote:
> On 18:58-20231207, Anwar, Md Danish wrote:
> [...]
>>>> +
>>>>  	memory@80000000 {
>>>>  		bootph-all;
>>>>  		device_type = "memory";
>>>> @@ -229,6 +234,70 @@ transceiver2: can-phy1 {
>>>>  		max-bitrate = <5000000>;
>>>>  		standby-gpios = <&exp1 9 GPIO_ACTIVE_HIGH>;
>>>>  	};
>>>> +
>>>> +	icssg1_eth: icssg1-eth {
>>>> +		compatible = "ti,am642-icssg-prueth";
>>>> +		pinctrl-names = "default";
>>>> +		pinctrl-0 = <&icssg1_rgmii1_pins_default>;
>>>> +
>>>> +		sram = <&oc_sram>;
>>>> +		ti,prus = <&pru1_0>, <&rtu1_0>, <&tx_pru1_0>, <&pru1_1>, <&rtu1_1>, <&tx_pru1_1>;
>>>> +		firmware-name = "ti-pruss/am65x-sr2-pru0-prueth-fw.elf",
>>>> +				"ti-pruss/am65x-sr2-rtu0-prueth-fw.elf",
>>>> +				"ti-pruss/am65x-sr2-txpru0-prueth-fw.elf",
>>>> +				"ti-pruss/am65x-sr2-pru1-prueth-fw.elf",
>>>> +				"ti-pruss/am65x-sr2-rtu1-prueth-fw.elf",
>>>> +				"ti-pruss/am65x-sr2-txpru1-prueth-fw.elf";
>>>
>>> Umm... am65x??? is that a typo? I'd rather keep it am64x here and drop
>>> that sr2 thing. Tomorrow there will be a custom bug on am64 and then we
>>> will have to respin this again.
>>>
>>
>> No Nishant, this is not a typo. Both AM64x and AM65x use the same ICSSG
>> firmwares. We only have am65x-sr2-* firmwares and they are used by both
>> AM64x and AM65x and that is why I have kept the firmware-name here in dt
>> same as the files that we load on the pru cores.
>>
> 
> SoCs are different. The hardware as a result is different as well. In
> fact, you do have a different compatible to distinguish the two. Some
> day, there will be an erratum that is different and we will be stuck
> with abi breakage across distros. So, unless you can explain why this
> scenario will never occur, I don't buy the argument this will survive
> long term.
> 

Agreed, this property was introduced for this purpose only. Today am65x
and am64x share the same firmware however in future the firmwares might
change and that is why we have this property. Currently this property is
not used in driver and firmware name is defined in the driver (with the
below structure) which is used for both am64x and am65x. I will rename
the firmware names here to am64x-sr2* in v2. In future when we have
different firmwares for different SoCs, we can stop using the below
structure and use the firmware-name property from dt.

static struct icssg_firmwares icssg_emac_firmwares[] = {
	{
		.pru = "ti-pruss/am65x-sr2-pru0-prueth-fw.elf",
		.rtu = "ti-pruss/am65x-sr2-rtu0-prueth-fw.elf",
		.txpru = "ti-pruss/am65x-sr2-txpru0-prueth-fw.elf",
	},
	{
		.pru = "ti-pruss/am65x-sr2-pru1-prueth-fw.elf",
		.rtu = "ti-pruss/am65x-sr2-rtu1-prueth-fw.elf",
		.txpru = "ti-pruss/am65x-sr2-txpru1-prueth-fw.elf",
	}
};

-- 
Thanks and Regards,
Md Danish Anwar

Re: [PATCH 3/3] arm64: dts: ti: k3-am642-evm: add overlay for icssg1 2nd port

[Anwar, Md Danish]

On 12/7/2023 6:57 PM, Nishanth Menon wrote:
> On 13:49-20231207, MD Danish Anwar wrote:
>> The am642-evm doesn't allow to enable 2 x CPSW3g ports and 2 x ICSSG1 ports
>> all together, so base k3-am642-evm.dts enables by default 2 x CPSW3g ports
>> and 1 x ICSSG1 ports, but it also possible to support 1 x CPSW3g ports and
>> 2 x ICSSG1 ports configuration.
>>
>> This patch adds overlay to support 1 x CPSW3g ports and 2 x ICSSG1 ports
>> configuration:
>> - disable 2nd CPSW3g port
>> - update CPSW3g pinmuxes to not use RGMII2
>> - disable mdio-mux-1 and define mdio-mux-2 to route ICSSG1 MDIO to the
>>   shared DP83869 PHY
>> - add and enable ICSSG1 RGMII2 pinmuxes
>> - enable ICSSG1 MII1 port
>>
>> Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
>> ---
>>  arch/arm64/boot/dts/ti/Makefile               |  2 +
>>  .../dts/ti/k3-am642-evm-icssg1-dualemac.dtso  | 80 +++++++++++++++++++
>>  arch/arm64/boot/dts/ti/k3-am642-evm.dts       |  2 +-
>>  3 files changed, 83 insertions(+), 1 deletion(-)
>>  create mode 100644 arch/arm64/boot/dts/ti/k3-am642-evm-icssg1-dualemac.dtso
>>
>> diff --git a/arch/arm64/boot/dts/ti/Makefile b/arch/arm64/boot/dts/ti/Makefile
>> index 5ef49b02c71f..99a4dce47f02 100644
>> --- a/arch/arm64/boot/dts/ti/Makefile
>> +++ b/arch/arm64/boot/dts/ti/Makefile
>> @@ -35,12 +35,14 @@ dtb-$(CONFIG_ARCH_K3) += k3-am62x-sk-csi2-imx219.dtbo
>>  dtb-$(CONFIG_ARCH_K3) += k3-am62x-sk-hdmi-audio.dtbo
>>  
>>  # Boards with AM64x SoC
>> +k3-am642-evm-icssg1-dtbs := k3-am642-evm.dtb k3-am642-evm-icssg1-dualemac.dtbo
> 
> Why not handle this for CONFIG_OF_ALL_DTBS alone? See commit
> b0044823a6607e535fdb083c89f487fbf183b171
> 

I'll have to look into this.

I am merging this k3-am642-evm-icssg1-dualemac.dtbo with
k3-am642-evm.dtb because when I had posted patches for AM65x DTS, Andrew
had commented [1] saying we should not leave orphan dtbos and every dtbo
should be applied over some dtb by using `dtb- +=`.

Due to this I am applying the overlay on k3-am642-evm.dtb and creating
new dtb k3-am642-evm-icssg1.dtb which can be used when both ICSSG1 ports
are needed to be enabled.

[1] https://lore.kernel.org/all/ca832fe3-d5cf-b075-324b-50da40794bb7@ti.com/

>>  dtb-$(CONFIG_ARCH_K3) += k3-am642-evm.dtb
>>  dtb-$(CONFIG_ARCH_K3) += k3-am642-phyboard-electra-rdk.dtb
>>  dtb-$(CONFIG_ARCH_K3) += k3-am642-sk.dtb
>>  dtb-$(CONFIG_ARCH_K3) += k3-am642-tqma64xxl-mbax4xxl.dtb
>>  dtb-$(CONFIG_ARCH_K3) += k3-am64-tqma64xxl-mbax4xxl-sdcard.dtbo
>>  dtb-$(CONFIG_ARCH_K3) += k3-am64-tqma64xxl-mbax4xxl-wlan.dtbo
>> +dtb-$(CONFIG_ARCH_K3) += k3-am642-evm-icssg1.dtb
>>  
>>  # Boards with AM65x SoC
>>  k3-am654-gp-evm-dtbs := k3-am654-base-board.dtb k3-am654-base-board-rocktech-rk101-panel.dtbo
>> diff --git a/arch/arm64/boot/dts/ti/k3-am642-evm-icssg1-dualemac.dtso b/arch/arm64/boot/dts/ti/k3-am642-evm-icssg1-dualemac.dtso
>> new file mode 100644
>> index 000000000000..6f33290c1ad6
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/ti/k3-am642-evm-icssg1-dualemac.dtso
>> @@ -0,0 +1,80 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/**
>> + * DT overlay for enabling 2nd ICSSG1 port on AM642 EVM
>> + *
>> + * Copyright (C) 2023 Texas Instruments Incorporated - https://www.ti.com/
>> + */
>> +
>> +/dts-v1/;
>> +/plugin/;
>> +
>> +#include <dt-bindings/gpio/gpio.h>
>> +#include "k3-pinctrl.h"
>> +
>> +&{/} {
>> +	aliases {
>> +		ethernet1 = "/icssg1-eth/ethernet-ports/port@0";
>> +		ethernet2 = "/icssg1-eth/ethernet-ports/port@1";
> 
> I don't understand what you are overriding here. isn't patch #2 in your
> series already introducing this in the base dts?
> 

Sorry, My bad. I will remove this in v2.

>> +	};
>> +
>> +	mdio-mux-2 {
>> +		compatible = "mdio-mux-multiplexer";
>> +		mux-controls = <&mdio_mux>;
>> +		mdio-parent-bus = <&icssg1_mdio>;
>> +		#address-cells = <1>;
>> +		#size-cells = <0>;
>> +
>> +		mdio@0 {
>> +			reg = <0x0>;
>> +			#address-cells = <1>;
>> +			#size-cells = <0>;
>> +
>> +			icssg1_phy2: ethernet-phy@3 {
>> +				reg = <3>;
>> +				tx-internal-delay-ps = <250>;
>> +				rx-internal-delay-ps = <2000>;
>> +			};
>> +		};
>> +	};
>> +};
>> +
>> +&main_pmx0 {
>> +	icssg1_rgmii2_pins_default: icssg1-rgmii2-default-pins {
>> +		pinctrl-single,pins = <
>> +			AM64X_IOPAD(0x0108, PIN_INPUT, 2) /* (W11) PRG1_PRU1_GPO0.RGMII2_RD0 */
>> +			AM64X_IOPAD(0x010c, PIN_INPUT, 2) /* (V11) PRG1_PRU1_GPO1.RGMII2_RD1 */
>> +			AM64X_IOPAD(0x0110, PIN_INPUT, 2) /* (AA12) PRG1_PRU1_GPO2.RGMII2_RD2 */
>> +			AM64X_IOPAD(0x0114, PIN_INPUT, 2) /* (Y12) PRG1_PRU1_GPO3.RGMII2_RD3 */
>> +			AM64X_IOPAD(0x0120, PIN_INPUT, 2) /* (U11) PRG1_PRU1_GPO6.RGMII2_RXC */
>> +			AM64X_IOPAD(0x0118, PIN_INPUT, 2) /* (W12) PRG1_PRU1_GPO4.RGMII2_RX_CTL */
>> +			AM64X_IOPAD(0x0134, PIN_OUTPUT, 2) /* (AA10) PRG1_PRU1_GPO11.RGMII2_TD0 */
>> +			AM64X_IOPAD(0x0138, PIN_OUTPUT, 2) /* (V10) PRG1_PRU1_GPO12.RGMII2_TD1 */
>> +			AM64X_IOPAD(0x013c, PIN_OUTPUT, 2) /* (U10) PRG1_PRU1_GPO13.RGMII2_TD2 */
>> +			AM64X_IOPAD(0x0140, PIN_OUTPUT, 2) /* (AA11) PRG1_PRU1_GPO14.RGMII2_TD3 */
>> +			AM64X_IOPAD(0x0148, PIN_OUTPUT, 2) /* (Y10) PRG1_PRU1_GPO16.RGMII2_TXC */
>> +			AM64X_IOPAD(0x0144, PIN_OUTPUT, 2) /* (Y11) PRG1_PRU1_GPO15.RGMII2_TX_CTL */
>> +		>;
>> +	};
>> +};
>> +
>> +&cpsw3g {
>> +	pinctrl-0 = <&rgmii1_pins_default>;
>> +};
>> +
>> +&cpsw_port2 {
>> +	status = "disabled";
>> +};
>> +
>> +&mdio_mux_1 {
>> +	status = "disabled";
>> +};
>> +
>> +&icssg1_eth {
>> +	pinctrl-0 = <&icssg1_rgmii1_pins_default &icssg1_rgmii2_pins_default>;
> 
> Grrr... No! I have been cleaning up after you folks and you folks should
> take notice.
> 
> pinctrl-0 = <&icssg1_rgmii1_pins_default>, <&icssg1_rgmii2_pins_default>;
> 

Sorry, I was not aware of this. I will change it in v2.

> 
>> +};
>> +
>> +&icssg1_emac1 {
>> +	status = "okay";
>> +	phy-handle = <&icssg1_phy2>;
>> +	phy-mode = "rgmii-id";
>> +};
>> diff --git a/arch/arm64/boot/dts/ti/k3-am642-evm.dts b/arch/arm64/boot/dts/ti/k3-am642-evm.dts
>> index 04d1c0602d31..90867090e725 100644
>> --- a/arch/arm64/boot/dts/ti/k3-am642-evm.dts
>> +++ b/arch/arm64/boot/dts/ti/k3-am642-evm.dts
>> @@ -203,7 +203,7 @@ mdio_mux: mux-controller {
>>  		mux-gpios = <&exp1 12 GPIO_ACTIVE_HIGH>;
>>  	};
>>  
>> -	mdio-mux-1 {
>> +	mdio_mux_1: mdio-mux-1 {
> 
> Commit message doesn't warn me for this change.

Sure. I will add details of this in commit message saying `Add label
name 'mdio_mux_1' for 'mdio-mux-1' node so that the node 'mdio-mux-1'
can be disabled in the overlay using the label name.`

>>  		compatible = "mdio-mux-multiplexer";
>>  		mux-controls = <&mdio_mux>;
>>  		mdio-parent-bus = <&cpsw3g_mdio>;
>> -- 
>> 2.34.1
>>
> 

-- 
Thanks and Regards,
Md Danish Anwar

Re: [PATCH 3/3] arm64: dts: ti: k3-am642-evm: add overlay for icssg1 2nd port

[Andrew Davis]

On 12/7/23 8:06 AM, Anwar, Md Danish wrote:
> On 12/7/2023 6:57 PM, Nishanth Menon wrote:
>> On 13:49-20231207, MD Danish Anwar wrote:
>>> The am642-evm doesn't allow to enable 2 x CPSW3g ports and 2 x ICSSG1 ports
>>> all together, so base k3-am642-evm.dts enables by default 2 x CPSW3g ports
>>> and 1 x ICSSG1 ports, but it also possible to support 1 x CPSW3g ports and
>>> 2 x ICSSG1 ports configuration.
>>>
>>> This patch adds overlay to support 1 x CPSW3g ports and 2 x ICSSG1 ports
>>> configuration:
>>> - disable 2nd CPSW3g port
>>> - update CPSW3g pinmuxes to not use RGMII2
>>> - disable mdio-mux-1 and define mdio-mux-2 to route ICSSG1 MDIO to the
>>>    shared DP83869 PHY
>>> - add and enable ICSSG1 RGMII2 pinmuxes
>>> - enable ICSSG1 MII1 port
>>>
>>> Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
>>> ---
>>>   arch/arm64/boot/dts/ti/Makefile               |  2 +
>>>   .../dts/ti/k3-am642-evm-icssg1-dualemac.dtso  | 80 +++++++++++++++++++
>>>   arch/arm64/boot/dts/ti/k3-am642-evm.dts       |  2 +-
>>>   3 files changed, 83 insertions(+), 1 deletion(-)
>>>   create mode 100644 arch/arm64/boot/dts/ti/k3-am642-evm-icssg1-dualemac.dtso
>>>
>>> diff --git a/arch/arm64/boot/dts/ti/Makefile b/arch/arm64/boot/dts/ti/Makefile
>>> index 5ef49b02c71f..99a4dce47f02 100644
>>> --- a/arch/arm64/boot/dts/ti/Makefile
>>> +++ b/arch/arm64/boot/dts/ti/Makefile
>>> @@ -35,12 +35,14 @@ dtb-$(CONFIG_ARCH_K3) += k3-am62x-sk-csi2-imx219.dtbo
>>>   dtb-$(CONFIG_ARCH_K3) += k3-am62x-sk-hdmi-audio.dtbo
>>>   
>>>   # Boards with AM64x SoC
>>> +k3-am642-evm-icssg1-dtbs := k3-am642-evm.dtb k3-am642-evm-icssg1-dualemac.dtbo
>>
>> Why not handle this for CONFIG_OF_ALL_DTBS alone? See commit
>> b0044823a6607e535fdb083c89f487fbf183b171
>>
> 
> I'll have to look into this.
> 
> I am merging this k3-am642-evm-icssg1-dualemac.dtbo with
> k3-am642-evm.dtb because when I had posted patches for AM65x DTS, Andrew
> had commented [1] saying we should not leave orphan dtbos and every dtbo
> should be applied over some dtb by using `dtb- +=`.
> 

And my comment on not having orphan DTBOs is still true. But that was back
in September, we now have a new way of accomplishing that without needing
to produce a bunch of temporary combined DTB files, which is what Nishanth
is pointing out with CONFIG_OF_ALL_DTBS.

So, no need for `k3-am642-evm-icssg1.dtb`, you can add the overlay target
directly, then create a fake target to test DTBO application by adding to
the `dtb- += ` lines below.

Andrew

> Due to this I am applying the overlay on k3-am642-evm.dtb and creating
> new dtb k3-am642-evm-icssg1.dtb which can be used when both ICSSG1 ports
> are needed to be enabled.
> 
> [1] https://lore.kernel.org/all/ca832fe3-d5cf-b075-324b-50da40794bb7@ti.com/
> 
>>>   dtb-$(CONFIG_ARCH_K3) += k3-am642-evm.dtb
>>>   dtb-$(CONFIG_ARCH_K3) += k3-am642-phyboard-electra-rdk.dtb
>>>   dtb-$(CONFIG_ARCH_K3) += k3-am642-sk.dtb
>>>   dtb-$(CONFIG_ARCH_K3) += k3-am642-tqma64xxl-mbax4xxl.dtb
>>>   dtb-$(CONFIG_ARCH_K3) += k3-am64-tqma64xxl-mbax4xxl-sdcard.dtbo
>>>   dtb-$(CONFIG_ARCH_K3) += k3-am64-tqma64xxl-mbax4xxl-wlan.dtbo
>>> +dtb-$(CONFIG_ARCH_K3) += k3-am642-evm-icssg1.dtb
>>>   
>>>   # Boards with AM65x SoC
>>>   k3-am654-gp-evm-dtbs := k3-am654-base-board.dtb k3-am654-base-board-rocktech-rk101-panel.dtbo
>>> diff --git a/arch/arm64/boot/dts/ti/k3-am642-evm-icssg1-dualemac.dtso b/arch/arm64/boot/dts/ti/k3-am642-evm-icssg1-dualemac.dtso
>>> new file mode 100644
>>> index 000000000000..6f33290c1ad6
>>> --- /dev/null
>>> +++ b/arch/arm64/boot/dts/ti/k3-am642-evm-icssg1-dualemac.dtso
>>> @@ -0,0 +1,80 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/**
>>> + * DT overlay for enabling 2nd ICSSG1 port on AM642 EVM
>>> + *
>>> + * Copyright (C) 2023 Texas Instruments Incorporated - https://www.ti.com/
>>> + */
>>> +
>>> +/dts-v1/;
>>> +/plugin/;
>>> +
>>> +#include <dt-bindings/gpio/gpio.h>
>>> +#include "k3-pinctrl.h"
>>> +
>>> +&{/} {
>>> +	aliases {
>>> +		ethernet1 = "/icssg1-eth/ethernet-ports/port@0";
>>> +		ethernet2 = "/icssg1-eth/ethernet-ports/port@1";
>>
>> I don't understand what you are overriding here. isn't patch #2 in your
>> series already introducing this in the base dts?
>>
> 
> Sorry, My bad. I will remove this in v2.
> 
>>> +	};
>>> +
>>> +	mdio-mux-2 {
>>> +		compatible = "mdio-mux-multiplexer";
>>> +		mux-controls = <&mdio_mux>;
>>> +		mdio-parent-bus = <&icssg1_mdio>;
>>> +		#address-cells = <1>;
>>> +		#size-cells = <0>;
>>> +
>>> +		mdio@0 {
>>> +			reg = <0x0>;
>>> +			#address-cells = <1>;
>>> +			#size-cells = <0>;
>>> +
>>> +			icssg1_phy2: ethernet-phy@3 {
>>> +				reg = <3>;
>>> +				tx-internal-delay-ps = <250>;
>>> +				rx-internal-delay-ps = <2000>;
>>> +			};
>>> +		};
>>> +	};
>>> +};
>>> +
>>> +&main_pmx0 {
>>> +	icssg1_rgmii2_pins_default: icssg1-rgmii2-default-pins {
>>> +		pinctrl-single,pins = <
>>> +			AM64X_IOPAD(0x0108, PIN_INPUT, 2) /* (W11) PRG1_PRU1_GPO0.RGMII2_RD0 */
>>> +			AM64X_IOPAD(0x010c, PIN_INPUT, 2) /* (V11) PRG1_PRU1_GPO1.RGMII2_RD1 */
>>> +			AM64X_IOPAD(0x0110, PIN_INPUT, 2) /* (AA12) PRG1_PRU1_GPO2.RGMII2_RD2 */
>>> +			AM64X_IOPAD(0x0114, PIN_INPUT, 2) /* (Y12) PRG1_PRU1_GPO3.RGMII2_RD3 */
>>> +			AM64X_IOPAD(0x0120, PIN_INPUT, 2) /* (U11) PRG1_PRU1_GPO6.RGMII2_RXC */
>>> +			AM64X_IOPAD(0x0118, PIN_INPUT, 2) /* (W12) PRG1_PRU1_GPO4.RGMII2_RX_CTL */
>>> +			AM64X_IOPAD(0x0134, PIN_OUTPUT, 2) /* (AA10) PRG1_PRU1_GPO11.RGMII2_TD0 */
>>> +			AM64X_IOPAD(0x0138, PIN_OUTPUT, 2) /* (V10) PRG1_PRU1_GPO12.RGMII2_TD1 */
>>> +			AM64X_IOPAD(0x013c, PIN_OUTPUT, 2) /* (U10) PRG1_PRU1_GPO13.RGMII2_TD2 */
>>> +			AM64X_IOPAD(0x0140, PIN_OUTPUT, 2) /* (AA11) PRG1_PRU1_GPO14.RGMII2_TD3 */
>>> +			AM64X_IOPAD(0x0148, PIN_OUTPUT, 2) /* (Y10) PRG1_PRU1_GPO16.RGMII2_TXC */
>>> +			AM64X_IOPAD(0x0144, PIN_OUTPUT, 2) /* (Y11) PRG1_PRU1_GPO15.RGMII2_TX_CTL */
>>> +		>;
>>> +	};
>>> +};
>>> +
>>> +&cpsw3g {
>>> +	pinctrl-0 = <&rgmii1_pins_default>;
>>> +};
>>> +
>>> +&cpsw_port2 {
>>> +	status = "disabled";
>>> +};
>>> +
>>> +&mdio_mux_1 {
>>> +	status = "disabled";
>>> +};
>>> +
>>> +&icssg1_eth {
>>> +	pinctrl-0 = <&icssg1_rgmii1_pins_default &icssg1_rgmii2_pins_default>;
>>
>> Grrr... No! I have been cleaning up after you folks and you folks should
>> take notice.
>>
>> pinctrl-0 = <&icssg1_rgmii1_pins_default>, <&icssg1_rgmii2_pins_default>;
>>
> 
> Sorry, I was not aware of this. I will change it in v2.
> 
>>
>>> +};
>>> +
>>> +&icssg1_emac1 {
>>> +	status = "okay";
>>> +	phy-handle = <&icssg1_phy2>;
>>> +	phy-mode = "rgmii-id";
>>> +};
>>> diff --git a/arch/arm64/boot/dts/ti/k3-am642-evm.dts b/arch/arm64/boot/dts/ti/k3-am642-evm.dts
>>> index 04d1c0602d31..90867090e725 100644
>>> --- a/arch/arm64/boot/dts/ti/k3-am642-evm.dts
>>> +++ b/arch/arm64/boot/dts/ti/k3-am642-evm.dts
>>> @@ -203,7 +203,7 @@ mdio_mux: mux-controller {
>>>   		mux-gpios = <&exp1 12 GPIO_ACTIVE_HIGH>;
>>>   	};
>>>   
>>> -	mdio-mux-1 {
>>> +	mdio_mux_1: mdio-mux-1 {
>>
>> Commit message doesn't warn me for this change.
> 
> Sure. I will add details of this in commit message saying `Add label
> name 'mdio_mux_1' for 'mdio-mux-1' node so that the node 'mdio-mux-1'
> can be disabled in the overlay using the label name.`
> 
>>>   		compatible = "mdio-mux-multiplexer";
>>>   		mux-controls = <&mdio_mux>;
>>>   		mdio-parent-bus = <&cpsw3g_mdio>;
>>> -- 
>>> 2.34.1
>>>
>>
> 

Re: [PATCH 2/3] arm64: dts: ti: k3-am642-evm: add ICSSG1 Ethernet support

[Andrew Lunn]

> +&icssg1_mdio {
> +	status = "okay";
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&icssg1_mdio1_pins_default>;
> +
> +	icssg1_phy1: ethernet-phy@0 {
> +		reg = <0xf>;

reg = 0xf, so this is ethernet-phy@f.

    Andrew

Re: [PATCH 3/3] arm64: dts: ti: k3-am642-evm: add overlay for icssg1 2nd port

[Andrew Lunn]

> +	mdio-mux-2 {
> +		compatible = "mdio-mux-multiplexer";
> +		mux-controls = <&mdio_mux>;
> +		mdio-parent-bus = <&icssg1_mdio>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		mdio@0 {
> +			reg = <0x0>;
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			icssg1_phy2: ethernet-phy@3 {
> +				reg = <3>;
> +				tx-internal-delay-ps = <250>;
> +				rx-internal-delay-ps = <2000>;
> +			};
> +		};

That looks odd. A mux generally has > 1 mdio bus. Otherwise its not
really a mux.

And this mux hardware exists all the time right? So it should be in
the .dtsi file.

	Andrew

Re: [PATCH 3/3] arm64: dts: ti: k3-am642-evm: add overlay for icssg1 2nd port

[Anwar, Md Danish]

On 12/7/2023 10:14 PM, Andrew Davis wrote:
> On 12/7/23 8:06 AM, Anwar, Md Danish wrote:
>> On 12/7/2023 6:57 PM, Nishanth Menon wrote:
>>> On 13:49-20231207, MD Danish Anwar wrote:
>>>> The am642-evm doesn't allow to enable 2 x CPSW3g ports and 2 x
>>>> ICSSG1 ports
>>>> all together, so base k3-am642-evm.dts enables by default 2 x CPSW3g
>>>> ports
>>>> and 1 x ICSSG1 ports, but it also possible to support 1 x CPSW3g
>>>> ports and
>>>> 2 x ICSSG1 ports configuration.
>>>>
>>>> This patch adds overlay to support 1 x CPSW3g ports and 2 x ICSSG1
>>>> ports
>>>> configuration:
>>>> - disable 2nd CPSW3g port
>>>> - update CPSW3g pinmuxes to not use RGMII2
>>>> - disable mdio-mux-1 and define mdio-mux-2 to route ICSSG1 MDIO to the
>>>>    shared DP83869 PHY
>>>> - add and enable ICSSG1 RGMII2 pinmuxes
>>>> - enable ICSSG1 MII1 port
>>>>
>>>> Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
>>>> ---
>>>>   arch/arm64/boot/dts/ti/Makefile               |  2 +
>>>>   .../dts/ti/k3-am642-evm-icssg1-dualemac.dtso  | 80
>>>> +++++++++++++++++++
>>>>   arch/arm64/boot/dts/ti/k3-am642-evm.dts       |  2 +-
>>>>   3 files changed, 83 insertions(+), 1 deletion(-)
>>>>   create mode 100644
>>>> arch/arm64/boot/dts/ti/k3-am642-evm-icssg1-dualemac.dtso
>>>>
>>>> diff --git a/arch/arm64/boot/dts/ti/Makefile
>>>> b/arch/arm64/boot/dts/ti/Makefile
>>>> index 5ef49b02c71f..99a4dce47f02 100644
>>>> --- a/arch/arm64/boot/dts/ti/Makefile
>>>> +++ b/arch/arm64/boot/dts/ti/Makefile
>>>> @@ -35,12 +35,14 @@ dtb-$(CONFIG_ARCH_K3) +=
>>>> k3-am62x-sk-csi2-imx219.dtbo
>>>>   dtb-$(CONFIG_ARCH_K3) += k3-am62x-sk-hdmi-audio.dtbo
>>>>     # Boards with AM64x SoC
>>>> +k3-am642-evm-icssg1-dtbs := k3-am642-evm.dtb
>>>> k3-am642-evm-icssg1-dualemac.dtbo
>>>
>>> Why not handle this for CONFIG_OF_ALL_DTBS alone? See commit
>>> b0044823a6607e535fdb083c89f487fbf183b171
>>>
>>
>> I'll have to look into this.
>>
>> I am merging this k3-am642-evm-icssg1-dualemac.dtbo with
>> k3-am642-evm.dtb because when I had posted patches for AM65x DTS, Andrew
>> had commented [1] saying we should not leave orphan dtbos and every dtbo
>> should be applied over some dtb by using `dtb- +=`.
>>
> 
> And my comment on not having orphan DTBOs is still true. But that was back
> in September, we now have a new way of accomplishing that without needing
> to produce a bunch of temporary combined DTB files, which is what Nishanth
> is pointing out with CONFIG_OF_ALL_DTBS.
> 
> So, no need for `k3-am642-evm-icssg1.dtb`, you can add the overlay target
> directly, then create a fake target to test DTBO application by adding to
> the `dtb- += ` lines below.
> 
> Andrew
> 

Sure Andrew. I'll do that and re-spin this series.

>> Due to this I am applying the overlay on k3-am642-evm.dtb and creating
>> new dtb k3-am642-evm-icssg1.dtb which can be used when both ICSSG1 ports
>> are needed to be enabled.
>>
>> [1]
>> https://lore.kernel.org/all/ca832fe3-d5cf-b075-324b-50da40794bb7@ti.com/
>>
>>>>   dtb-$(CONFIG_ARCH_K3) += k3-am642-evm.dtb
>>>>   dtb-$(CONFIG_ARCH_K3) += k3-am642-phyboard-electra-rdk.dtb
>>>>   dtb-$(CONFIG_ARCH_K3) += k3-am642-sk.dtb
>>>>   dtb-$(CONFIG_ARCH_K3) += k3-am642-tqma64xxl-mbax4xxl.dtb
>>>>   dtb-$(CONFIG_ARCH_K3) += k3-am64-tqma64xxl-mbax4xxl-sdcard.dtbo
>>>>   dtb-$(CONFIG_ARCH_K3) += k3-am64-tqma64xxl-mbax4xxl-wlan.dtbo
>>>> +dtb-$(CONFIG_ARCH_K3) += k3-am642-evm-icssg1.dtb
>>>>     # Boards with AM65x SoC
>>>>   k3-am654-gp-evm-dtbs := k3-am654-base-board.dtb
>>>> k3-am654-base-board-rocktech-rk101-panel.dtbo
>>>> diff --git
>>>> a/arch/arm64/boot/dts/ti/k3-am642-evm-icssg1-dualemac.dtso
>>>> b/arch/arm64/boot/dts/ti/k3-am642-evm-icssg1-dualemac.dtso
>>>> new file mode 100644
>>>> index 000000000000..6f33290c1ad6
>>>> --- /dev/null
>>>> +++ b/arch/arm64/boot/dts/ti/k3-am642-evm-icssg1-dualemac.dtso
>>>> @@ -0,0 +1,80 @@
>>>> +// SPDX-License-Identifier: GPL-2.0
>>>> +/**
>>>> + * DT overlay for enabling 2nd ICSSG1 port on AM642 EVM
>>>> + *
>>>> + * Copyright (C) 2023 Texas Instruments Incorporated -
>>>> https://www.ti.com/
>>>> + */
>>>> +
>>>> +/dts-v1/;
>>>> +/plugin/;
>>>> +
>>>> +#include <dt-bindings/gpio/gpio.h>
>>>> +#include "k3-pinctrl.h"
>>>> +
>>>> +&{/} {
>>>> +    aliases {
>>>> +        ethernet1 = "/icssg1-eth/ethernet-ports/port@0";
>>>> +        ethernet2 = "/icssg1-eth/ethernet-ports/port@1";
>>>
>>> I don't understand what you are overriding here. isn't patch #2 in your
>>> series already introducing this in the base dts?
>>>
>>
>> Sorry, My bad. I will remove this in v2.
>>
>>>> +    };
>>>> +
>>>> +    mdio-mux-2 {
>>>> +        compatible = "mdio-mux-multiplexer";
>>>> +        mux-controls = <&mdio_mux>;
>>>> +        mdio-parent-bus = <&icssg1_mdio>;
>>>> +        #address-cells = <1>;
>>>> +        #size-cells = <0>;
>>>> +
>>>> +        mdio@0 {
>>>> +            reg = <0x0>;
>>>> +            #address-cells = <1>;
>>>> +            #size-cells = <0>;
>>>> +
>>>> +            icssg1_phy2: ethernet-phy@3 {
>>>> +                reg = <3>;
>>>> +                tx-internal-delay-ps = <250>;
>>>> +                rx-internal-delay-ps = <2000>;
>>>> +            };
>>>> +        };
>>>> +    };
>>>> +};
>>>> +
>>>> +&main_pmx0 {
>>>> +    icssg1_rgmii2_pins_default: icssg1-rgmii2-default-pins {
>>>> +        pinctrl-single,pins = <
>>>> +            AM64X_IOPAD(0x0108, PIN_INPUT, 2) /* (W11)
>>>> PRG1_PRU1_GPO0.RGMII2_RD0 */
>>>> +            AM64X_IOPAD(0x010c, PIN_INPUT, 2) /* (V11)
>>>> PRG1_PRU1_GPO1.RGMII2_RD1 */
>>>> +            AM64X_IOPAD(0x0110, PIN_INPUT, 2) /* (AA12)
>>>> PRG1_PRU1_GPO2.RGMII2_RD2 */
>>>> +            AM64X_IOPAD(0x0114, PIN_INPUT, 2) /* (Y12)
>>>> PRG1_PRU1_GPO3.RGMII2_RD3 */
>>>> +            AM64X_IOPAD(0x0120, PIN_INPUT, 2) /* (U11)
>>>> PRG1_PRU1_GPO6.RGMII2_RXC */
>>>> +            AM64X_IOPAD(0x0118, PIN_INPUT, 2) /* (W12)
>>>> PRG1_PRU1_GPO4.RGMII2_RX_CTL */
>>>> +            AM64X_IOPAD(0x0134, PIN_OUTPUT, 2) /* (AA10)
>>>> PRG1_PRU1_GPO11.RGMII2_TD0 */
>>>> +            AM64X_IOPAD(0x0138, PIN_OUTPUT, 2) /* (V10)
>>>> PRG1_PRU1_GPO12.RGMII2_TD1 */
>>>> +            AM64X_IOPAD(0x013c, PIN_OUTPUT, 2) /* (U10)
>>>> PRG1_PRU1_GPO13.RGMII2_TD2 */
>>>> +            AM64X_IOPAD(0x0140, PIN_OUTPUT, 2) /* (AA11)
>>>> PRG1_PRU1_GPO14.RGMII2_TD3 */
>>>> +            AM64X_IOPAD(0x0148, PIN_OUTPUT, 2) /* (Y10)
>>>> PRG1_PRU1_GPO16.RGMII2_TXC */
>>>> +            AM64X_IOPAD(0x0144, PIN_OUTPUT, 2) /* (Y11)
>>>> PRG1_PRU1_GPO15.RGMII2_TX_CTL */
>>>> +        >;
>>>> +    };
>>>> +};
>>>> +
>>>> +&cpsw3g {
>>>> +    pinctrl-0 = <&rgmii1_pins_default>;
>>>> +};
>>>> +
>>>> +&cpsw_port2 {
>>>> +    status = "disabled";
>>>> +};
>>>> +
>>>> +&mdio_mux_1 {
>>>> +    status = "disabled";
>>>> +};
>>>> +
>>>> +&icssg1_eth {
>>>> +    pinctrl-0 = <&icssg1_rgmii1_pins_default
>>>> &icssg1_rgmii2_pins_default>;
>>>
>>> Grrr... No! I have been cleaning up after you folks and you folks should
>>> take notice.
>>>
>>> pinctrl-0 = <&icssg1_rgmii1_pins_default>,
>>> <&icssg1_rgmii2_pins_default>;
>>>
>>
>> Sorry, I was not aware of this. I will change it in v2.
>>
>>>
>>>> +};
>>>> +
>>>> +&icssg1_emac1 {
>>>> +    status = "okay";
>>>> +    phy-handle = <&icssg1_phy2>;
>>>> +    phy-mode = "rgmii-id";
>>>> +};
>>>> diff --git a/arch/arm64/boot/dts/ti/k3-am642-evm.dts
>>>> b/arch/arm64/boot/dts/ti/k3-am642-evm.dts
>>>> index 04d1c0602d31..90867090e725 100644
>>>> --- a/arch/arm64/boot/dts/ti/k3-am642-evm.dts
>>>> +++ b/arch/arm64/boot/dts/ti/k3-am642-evm.dts
>>>> @@ -203,7 +203,7 @@ mdio_mux: mux-controller {
>>>>           mux-gpios = <&exp1 12 GPIO_ACTIVE_HIGH>;
>>>>       };
>>>>   -    mdio-mux-1 {
>>>> +    mdio_mux_1: mdio-mux-1 {
>>>
>>> Commit message doesn't warn me for this change.
>>
>> Sure. I will add details of this in commit message saying `Add label
>> name 'mdio_mux_1' for 'mdio-mux-1' node so that the node 'mdio-mux-1'
>> can be disabled in the overlay using the label name.`
>>
>>>>           compatible = "mdio-mux-multiplexer";
>>>>           mux-controls = <&mdio_mux>;
>>>>           mdio-parent-bus = <&cpsw3g_mdio>;
>>>> -- 
>>>> 2.34.1
>>>>
>>>
>>

-- 
Thanks and Regards,
Md Danish Anwar

Re: [PATCH 2/3] arm64: dts: ti: k3-am642-evm: add ICSSG1 Ethernet support

[Anwar, Md Danish]

On 12/8/2023 3:04 AM, Andrew Lunn wrote:
>> +&icssg1_mdio {
>> +	status = "okay";
>> +	pinctrl-names = "default";
>> +	pinctrl-0 = <&icssg1_mdio1_pins_default>;
>> +
>> +	icssg1_phy1: ethernet-phy@0 {
>> +		reg = <0xf>;
> 
> reg = 0xf, so this is ethernet-phy@f.
> 
>     Andrew

Sure Andrew. I'll update this.

-- 
Thanks and Regards,
Md Danish Anwar

Re: [PATCH 3/3] arm64: dts: ti: k3-am642-evm: add overlay for icssg1 2nd port

[Anwar, Md Danish]

On 12/8/2023 3:10 AM, Andrew Lunn wrote:
>> +	mdio-mux-2 {
>> +		compatible = "mdio-mux-multiplexer";
>> +		mux-controls = <&mdio_mux>;
>> +		mdio-parent-bus = <&icssg1_mdio>;
>> +		#address-cells = <1>;
>> +		#size-cells = <0>;
>> +
>> +		mdio@0 {
>> +			reg = <0x0>;
>> +			#address-cells = <1>;
>> +			#size-cells = <0>;
>> +
>> +			icssg1_phy2: ethernet-phy@3 {
>> +				reg = <3>;
>> +				tx-internal-delay-ps = <250>;
>> +				rx-internal-delay-ps = <2000>;
>> +			};
>> +		};
> 
> That looks odd. A mux generally has > 1 mdio bus. Otherwise its not
> really a mux.
> 

We are disabling node `mdio-mux-1` which has the `cpsw3g_mdio` bus and
then adding a new node `mdio-mux-2` which has the `icssg1_mdio` bus. The
mux can actually have two different mdio buses. The patch actually
disables the mux1 node and creates a new node for icssg1_mdio bus so
that cpsw3g mdio bus is disabled properly.

We can modify the existing `mdio-mux-1` as well (added the code below)
instead of disabling mux1 and creating mux2 node.

&mdio_mux_1 {
	mdio-parent-bus = <&icssg1_mdio>;
	#address-cells = <1>;
	#size-cells = <0>;

	mdio@0 {
		reg = <0x0>;
		#address-cells = <1>;
		#size-cells = <0>;

		icssg1_phy2: ethernet-phy@3 {
			reg = <3>;
			tx-internal-delay-ps = <250>;
			rx-internal-delay-ps = <2000>;
		};
	};
};

Let me know what do you think. Is the approach in the patch correct or
should I modify existing mux node only?

> And this mux hardware exists all the time right? So it should be in
> the .dtsi file.
> 

Agreed. But the mdio-mux-1 node was added in k3-am642-evm.dts by the
commit 985204ecae1c37d55372874ff9146231d28fccc6. I did the same with
mdio-mux-2 node.

> 	Andrew

-- 
Thanks and Regards,
Md Danish Anwar

Re: [PATCH 3/3] arm64: dts: ti: k3-am642-evm: add overlay for icssg1 2nd port

[MD Danish Anwar]

Hi Andrew,

On 08/12/23 12:58 pm, Anwar, Md Danish wrote:
> On 12/8/2023 3:10 AM, Andrew Lunn wrote:
>>> +	mdio-mux-2 {
>>> +		compatible = "mdio-mux-multiplexer";
>>> +		mux-controls = <&mdio_mux>;
>>> +		mdio-parent-bus = <&icssg1_mdio>;
>>> +		#address-cells = <1>;
>>> +		#size-cells = <0>;
>>> +
>>> +		mdio@0 {
>>> +			reg = <0x0>;
>>> +			#address-cells = <1>;
>>> +			#size-cells = <0>;
>>> +
>>> +			icssg1_phy2: ethernet-phy@3 {
>>> +				reg = <3>;
>>> +				tx-internal-delay-ps = <250>;
>>> +				rx-internal-delay-ps = <2000>;
>>> +			};
>>> +		};
>>
>> That looks odd. A mux generally has > 1 mdio bus. Otherwise its not
>> really a mux.
>>
> 
> We are disabling node `mdio-mux-1` which has the `cpsw3g_mdio` bus and
> then adding a new node `mdio-mux-2` which has the `icssg1_mdio` bus. The
> mux can actually have two different mdio buses. The patch actually
> disables the mux1 node and creates a new node for icssg1_mdio bus so
> that cpsw3g mdio bus is disabled properly.
> 
> We can modify the existing `mdio-mux-1` as well (added the code below)
> instead of disabling mux1 and creating mux2 node.
> 
> &mdio_mux_1 {
> 	mdio-parent-bus = <&icssg1_mdio>;
> 	#address-cells = <1>;
> 	#size-cells = <0>;
> 
> 	mdio@0 {
> 		reg = <0x0>;
> 		#address-cells = <1>;
> 		#size-cells = <0>;
> 
> 		icssg1_phy2: ethernet-phy@3 {
> 			reg = <3>;
> 			tx-internal-delay-ps = <250>;
> 			rx-internal-delay-ps = <2000>;
> 		};
> 	};
> };
> 
> Let me know what do you think. Is the approach in the patch correct or
> should I modify existing mux node only?
> 

Can you please let me know which approach should I follow here?

>> And this mux hardware exists all the time right? So it should be in
>> the .dtsi file.
>>
> 
> Agreed. But the mdio-mux-1 node was added in k3-am642-evm.dts by the
> commit 985204ecae1c37d55372874ff9146231d28fccc6. I did the same with
> mdio-mux-2 node.
> 
>> 	Andrew
> 

-- 
Thanks and Regards,
Danish