[PATCH 0/6] Add PPE device tree node for Qualcomm IPQ SoC

[PATCH 0/6] Add PPE device tree node for Qualcomm IPQ SoC

[Luo Jie]

The PPE(packet process engine) hardware block is supported by Qualcomm
IPQ platforms, such as IPQ9574 and IPQ5332. The PPE includes the various
packet processing modules such as the routing and bridging flow engines,
L2 switch capability, VLAN and tunnels. Also included are integrated
ethernet MAC and PCS(uniphy), which is used to connect with the external
PHY devices by PCS.

This patch series enables support for the following DTSI functionality
for Qualcomm IPQ9574 and IPQ5332 chipsets. 

1. Add PPE (Packet Processing Engine) HW support

2. Add IPQ9574 RDP433 board support, where the PPE is connected
   with qca8075 PHY and AQ PHY.

3. Add IPQ5332 RDP441 board support, where the PPE is connected
   with qca8386 and SFP

PPE DTS depends on the NSSCC clock driver below, which provides the
clocks for the PPE driver.
https://lore.kernel.org/linux-arm-msm/20230825091234.32713-1-quic_devipriy@quicinc.com/
https://lore.kernel.org/linux-arm-msm/20231211-ipq5332-nsscc-v3-0-ad13bef9b137@quicinc.com/

Lei Wei (2):
  arm64: dts: qcom: ipq5332: Add RDP441 board device tree
  arm64: dts: qcom: ipq9574: Add RDP433 board device tree

Luo Jie (4):
  arm64: dts: qcom: ipq9574: Add PPE device tree node
  arm64: dts: qcom: ipq5332: Add PPE device tree node
  arm64: dts: qcom: ipq5332: Add MDIO device tree
  arm64: dts: qcom: ipq9574: Add MDIO device tree

 arch/arm64/boot/dts/qcom/ipq5332-rdp441.dts |  51 ++
 arch/arm64/boot/dts/qcom/ipq5332.dtsi       | 414 ++++++++++-
 arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts |  66 ++
 arch/arm64/boot/dts/qcom/ipq9574.dtsi       | 758 +++++++++++++++++++-
 4 files changed, 1279 insertions(+), 10 deletions(-)

-- 
2.42.0

[PATCH 1/6] arm64: dts: qcom: ipq9574: Add PPE device tree node

[Luo Jie]

The PPE device tree node includes the PPE initialization configurations
and UNIPHY instance configuration.

Ther are 3 UNIPHYs(PCS) on the platform ipq9574, which register the
clock provider to output the clock for PPE port to work on the different
link speed.

Signed-off-by: Luo Jie <quic_luoj@quicinc.com>
---
 arch/arm64/boot/dts/qcom/ipq9574.dtsi | 730 +++++++++++++++++++++++++-
 1 file changed, 724 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/ipq9574.dtsi b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
index 810cda4a850f..5fa241e27c8b 100644
--- a/arch/arm64/boot/dts/qcom/ipq9574.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
@@ -775,16 +775,734 @@ nsscc: nsscc@39b00000 {
 				 <&bias_pll_nss_noc_clk>,
 				 <&bias_pll_ubi_nc_clk>,
 				 <&gcc_gpll0_out_aux>,
-				 <0>,
-				 <0>,
-				 <0>,
-				 <0>,
-				 <0>,
-				 <0>,
+				 <&uniphys 0>,
+				 <&uniphys 1>,
+				 <&uniphys 2>,
+				 <&uniphys 3>,
+				 <&uniphys 4>,
+				 <&uniphys 5>,
 				 <&xo_board_clk>;
 			#clock-cells = <1>;
 			#reset-cells = <1>;
 		};
+
+		qcom_ppe: qcom-ppe@3a000000 {
+			compatible = "qcom,ipq9574-ppe";
+			reg = <0x3a000000 0xb00000>;
+			#address-cells = <1>;
+			#size-cells = <1>;
+			ranges;
+			status = "okay";
+
+			clocks = <&gcc GCC_CMN_12GPLL_AHB_CLK>,
+				 <&gcc GCC_CMN_12GPLL_SYS_CLK>,
+				 <&gcc GCC_UNIPHY0_SYS_CLK>,
+				 <&gcc GCC_UNIPHY1_SYS_CLK>,
+				 <&gcc GCC_UNIPHY2_SYS_CLK>,
+				 <&gcc GCC_UNIPHY0_AHB_CLK>,
+				 <&gcc GCC_UNIPHY1_AHB_CLK>,
+				 <&gcc GCC_UNIPHY2_AHB_CLK>,
+				 <&gcc GCC_NSSCC_CLK>,
+				 <&gcc GCC_NSSNOC_NSSCC_CLK>,
+				 <&gcc GCC_NSSNOC_SNOC_CLK>,
+				 <&gcc GCC_NSSNOC_SNOC_1_CLK>,
+				 <&nsscc NSS_CC_PPE_SWITCH_CLK>,
+				 <&nsscc NSS_CC_PPE_SWITCH_CFG_CLK>,
+				 <&nsscc NSS_CC_NSSNOC_PPE_CLK>,
+				 <&nsscc NSS_CC_NSSNOC_PPE_CFG_CLK>,
+				 <&nsscc NSS_CC_PPE_EDMA_CLK>,
+				 <&nsscc NSS_CC_PPE_EDMA_CFG_CLK>,
+				 <&nsscc NSS_CC_PPE_SWITCH_IPE_CLK>,
+				 <&nsscc NSS_CC_PPE_SWITCH_BTQ_CLK>,
+				 <&nsscc NSS_CC_PORT1_MAC_CLK>,
+				 <&nsscc NSS_CC_PORT2_MAC_CLK>,
+				 <&nsscc NSS_CC_PORT3_MAC_CLK>,
+				 <&nsscc NSS_CC_PORT4_MAC_CLK>,
+				 <&nsscc NSS_CC_PORT5_MAC_CLK>,
+				 <&nsscc NSS_CC_PORT6_MAC_CLK>,
+				 <&nsscc NSS_CC_PORT1_RX_CLK>,
+				 <&nsscc NSS_CC_PORT1_TX_CLK>,
+				 <&nsscc NSS_CC_PORT2_RX_CLK>,
+				 <&nsscc NSS_CC_PORT2_TX_CLK>,
+				 <&nsscc NSS_CC_PORT3_RX_CLK>,
+				 <&nsscc NSS_CC_PORT3_TX_CLK>,
+				 <&nsscc NSS_CC_PORT4_RX_CLK>,
+				 <&nsscc NSS_CC_PORT4_TX_CLK>,
+				 <&nsscc NSS_CC_PORT5_RX_CLK>,
+				 <&nsscc NSS_CC_PORT5_TX_CLK>,
+				 <&nsscc NSS_CC_PORT6_RX_CLK>,
+				 <&nsscc NSS_CC_PORT6_TX_CLK>,
+				 <&nsscc NSS_CC_UNIPHY_PORT1_RX_CLK>,
+				 <&nsscc NSS_CC_UNIPHY_PORT1_TX_CLK>,
+				 <&nsscc NSS_CC_UNIPHY_PORT2_RX_CLK>,
+				 <&nsscc NSS_CC_UNIPHY_PORT2_TX_CLK>,
+				 <&nsscc NSS_CC_UNIPHY_PORT3_RX_CLK>,
+				 <&nsscc NSS_CC_UNIPHY_PORT3_TX_CLK>,
+				 <&nsscc NSS_CC_UNIPHY_PORT4_RX_CLK>,
+				 <&nsscc NSS_CC_UNIPHY_PORT4_TX_CLK>,
+				 <&nsscc NSS_CC_UNIPHY_PORT5_RX_CLK>,
+				 <&nsscc NSS_CC_UNIPHY_PORT5_TX_CLK>,
+				 <&nsscc NSS_CC_UNIPHY_PORT6_RX_CLK>,
+				 <&nsscc NSS_CC_UNIPHY_PORT6_TX_CLK>,
+				 <&nsscc NSS_CC_PORT5_RX_CLK_SRC>,
+				 <&nsscc NSS_CC_PORT5_TX_CLK_SRC>;
+			clock-names = "cmn_ahb",
+				      "cmn_sys",
+				      "uniphy0_sys",
+				      "uniphy1_sys",
+				      "uniphy2_sys",
+				      "uniphy0_ahb",
+				      "uniphy1_ahb",
+				      "uniphy2_ahb",
+				      "gcc_nsscc",
+				      "gcc_nssnoc_nsscc",
+				      "gcc_nssnoc_snoc",
+				      "gcc_nssnoc_snoc_1",
+				      "nss_ppe",
+				      "nss_ppe_cfg",
+				      "nssnoc_ppe",
+				      "nssnoc_ppe_cfg",
+				      "nss_edma",
+				      "nss_edma_cfg",
+				      "nss_ppe_ipe",
+				      "nss_ppe_btq",
+				      "port1_mac",
+				      "port2_mac",
+				      "port3_mac",
+				      "port4_mac",
+				      "port5_mac",
+				      "port6_mac",
+				      "nss_port1_rx",
+				      "nss_port1_tx",
+				      "nss_port2_rx",
+				      "nss_port2_tx",
+				      "nss_port3_rx",
+				      "nss_port3_tx",
+				      "nss_port4_rx",
+				      "nss_port4_tx",
+				      "nss_port5_rx",
+				      "nss_port5_tx",
+				      "nss_port6_rx",
+				      "nss_port6_tx",
+				      "uniphy_port1_rx",
+				      "uniphy_port1_tx",
+				      "uniphy_port2_rx",
+				      "uniphy_port2_tx",
+				      "uniphy_port3_rx",
+				      "uniphy_port3_tx",
+				      "uniphy_port4_rx",
+				      "uniphy_port4_tx",
+				      "uniphy_port5_rx",
+				      "uniphy_port5_tx",
+				      "uniphy_port6_rx",
+				      "uniphy_port6_tx",
+				      "nss_port5_rx_clk_src",
+				      "nss_port5_tx_clk_src";
+
+			resets = <&nsscc PPE_FULL_RESET>,
+				 <&gcc GCC_UNIPHY0_SYS_RESET>,
+				 <&gcc GCC_UNIPHY1_SYS_RESET>,
+				 <&gcc GCC_UNIPHY2_SYS_RESET>,
+				 <&gcc GCC_UNIPHY0_AHB_RESET>,
+				 <&gcc GCC_UNIPHY1_AHB_RESET>,
+				 <&gcc GCC_UNIPHY2_AHB_RESET>,
+				 <&gcc GCC_UNIPHY0_XPCS_RESET>,
+				 <&gcc GCC_UNIPHY1_XPCS_RESET>,
+				 <&gcc GCC_UNIPHY2_XPCS_RESET>,
+				 <&nsscc UNIPHY0_SOFT_RESET>,
+				 <&nsscc UNIPHY_PORT5_ARES>,
+				 <&nsscc UNIPHY_PORT6_ARES>,
+				 <&nsscc UNIPHY_PORT1_ARES>,
+				 <&nsscc UNIPHY_PORT2_ARES>,
+				 <&nsscc UNIPHY_PORT3_ARES>,
+				 <&nsscc UNIPHY_PORT4_ARES>,
+				 <&nsscc NSSPORT1_RESET>,
+				 <&nsscc NSSPORT2_RESET>,
+				 <&nsscc NSSPORT3_RESET>,
+				 <&nsscc NSSPORT4_RESET>,
+				 <&nsscc NSSPORT5_RESET>,
+				 <&nsscc NSSPORT6_RESET>,
+				 <&nsscc PORT1_MAC_ARES>,
+				 <&nsscc PORT2_MAC_ARES>,
+				 <&nsscc PORT3_MAC_ARES>,
+				 <&nsscc PORT4_MAC_ARES>,
+				 <&nsscc PORT5_MAC_ARES>,
+				 <&nsscc PORT6_MAC_ARES>;
+			reset-names = "ppe",
+				      "uniphy0_sys",
+				      "uniphy1_sys",
+				      "uniphy2_sys",
+				      "uniphy0_ahb",
+				      "uniphy1_ahb",
+				      "uniphy2_ahb",
+				      "uniphy0_xpcs",
+				      "uniphy1_xpcs",
+				      "uniphy2_xpcs",
+				      "uniphy0_soft",
+				      "uniphy1_soft",
+				      "uniphy2_soft",
+				      "uniphy0_port1_dis",
+				      "uniphy0_port2_dis",
+				      "uniphy0_port3_dis",
+				      "uniphy0_port4_dis",
+				      "nss_port1",
+				      "nss_port2",
+				      "nss_port3",
+				      "nss_port4",
+				      "nss_port5",
+				      "nss_port6",
+				      "nss_port1_mac",
+				      "nss_port2_mac",
+				      "nss_port3_mac",
+				      "nss_port4_mac",
+				      "nss_port5_mac",
+				      "nss_port6_mac";
+
+			uniphys: qcom-uniphy@7a00000 {
+				reg = <0x7a00000 0x10000>,
+				      <0x7a10000 0x10000>,
+				      <0x7a20000 0x10000>;
+				#clock-cells = <0x1>;
+				clock-output-names = "uniphy0_gcc_rx_clk",
+						     "uniphy0_gcc_tx_clk",
+						     "uniphy1_gcc_rx_clk",
+						     "uniphy1_gcc_tx_clk",
+						     "uniphy2_gcc_rx_clk",
+						     "uniphy2_gcc_tx_clk";
+			};
+
+			tdm-config {
+				/*
+				 * qcom,tdm-bm-config = <valid egress port
+				 * second_valid second_port>;
+				 */
+				qcom,tdm-bm-config = <1 0 0 0 0>,
+						     <1 1 0 0 0>,
+						     <1 0 5 0 0>,
+						     <1 1 5 0 0>,
+						     <1 0 6 0 0>,
+						     <1 1 6 0 0>,
+						     <1 0 1 0 0>,
+						     <1 1 1 0 0>,
+						     <1 0 0 0 0>,
+						     <1 1 0 0 0>,
+						     <1 0 5 0 0>,
+						     <1 1 5 0 0>,
+						     <1 0 6 0 0>,
+						     <1 1 6 0 0>,
+						     <1 0 7 0 0>,
+						     <1 1 7 0 0>,
+						     <1 0 0 0 0>,
+						     <1 1 0 0 0>,
+						     <1 0 1 0 0>,
+						     <1 1 1 0 0>,
+						     <1 0 5 0 0>,
+						     <1 1 5 0 0>,
+						     <1 0 6 0 0>,
+						     <1 1 6 0 0>,
+						     <1 0 2 0 0>,
+						     <1 1 2 0 0>,
+						     <1 0 0 0 0>,
+						     <1 1 0 0 0>,
+						     <1 0 5 0 0>,
+						     <1 1 5 0 0>,
+						     <1 0 6 0 0>,
+						     <1 1 6 0 0>,
+						     <1 0 1 0 0>,
+						     <1 1 1 0 0>,
+						     <1 0 3 0 0>,
+						     <1 1 3 0 0>,
+						     <1 0 0 0 0>,
+						     <1 1 0 0 0>,
+						     <1 0 5 0 0>,
+						     <1 1 5 0 0>,
+						     <1 0 6 0 0>,
+						     <1 1 6 0 0>,
+						     <1 0 7 0 0>,
+						     <1 1 7 0 0>,
+						     <1 0 0 0 0>,
+						     <1 1 0 0 0>,
+						     <1 0 1 0 0>,
+						     <1 1 1 0 0>,
+						     <1 0 5 0 0>,
+						     <1 1 5 0 0>,
+						     <1 0 6 0 0>,
+						     <1 1 6 0 0>,
+						     <1 0 4 0 0>,
+						     <1 1 4 0 0>,
+						     <1 0 0 0 0>,
+						     <1 1 0 0 0>,
+						     <1 0 5 0 0>,
+						     <1 1 5 0 0>,
+						     <1 0 6 0 0>,
+						     <1 1 6 0 0>,
+						     <1 0 1 0 0>,
+						     <1 1 1 0 0>,
+						     <1 0 0 0 0>,
+						     <1 1 0 0 0>,
+						     <1 0 5 0 0>,
+						     <1 1 5 0 0>,
+						     <1 0 6 0 0>,
+						     <1 1 6 0 0>,
+						     <1 0 2 0 0>,
+						     <1 1 2 0 0>,
+						     <1 0 0 0 0>,
+						     <1 1 0 0 0>,
+						     <1 0 7 0 0>,
+						     <1 1 7 0 0>,
+						     <1 0 5 0 0>,
+						     <1 1 5 0 0>,
+						     <1 0 6 0 0>,
+						     <1 1 6 0 0>,
+						     <1 0 1 0 0>,
+						     <1 1 1 0 0>,
+						     <1 0 0 0 0>,
+						     <1 1 0 0 0>,
+						     <1 0 5 0 0>,
+						     <1 1 5 0 0>,
+						     <1 0 6 0 0>,
+						     <1 1 6 0 0>,
+						     <1 0 3 0 0>,
+						     <1 1 3 0 0>,
+						     <1 0 1 0 0>,
+						     <1 1 1 0 0>,
+						     <1 0 0 0 0>,
+						     <1 1 0 0 0>,
+						     <1 0 5 0 0>,
+						     <1 1 5 0 0>,
+						     <1 0 6 0 0>,
+						     <1 1 6 0 0>,
+						     <1 0 4 0 0>,
+						     <1 1 4 0 0>,
+						     <1 0 7 0 0>,
+						     <1 1 7 0 0>;
+
+			/*
+			 * qcom,tdm-port-scheduler-config = <ensch_bmp ensch_port
+			 * desch_port desch_second_valid desch_second_port>;
+			 */
+			qcom,tdm-port-scheduler-config = <0x98 6 0 1 1>,
+							 <0x94 5 6 1 3>,
+							 <0x86 0 5 1 4>,
+							 <0x8C 1 6 1 0>,
+							 <0x1C 7 5 1 1>,
+							 <0x98 2 6 1 0>,
+							 <0x1C 5 7 1 1>,
+							 <0x34 3 6 1 0>,
+							 <0x8C 4 5 1 1>,
+							 <0x98 2 6 1 0>,
+							 <0x8C 5 4 1 1>,
+							 <0xA8 0 6 1 2>,
+							 <0x98 5 1 1 0>,
+							 <0x98 6 5 1 2>,
+							 <0x89 1 6 1 4>,
+							 <0xA4 3 0 1 1>,
+							 <0x8C 5 6 1 4>,
+							 <0xA8 0 2 1 1>,
+							 <0x98 6 5 1 0>,
+							 <0xC4 4 3 1 1>,
+							 <0x94 6 5 1 0>,
+							 <0x1C 7 6 1 1>,
+							 <0x98 2 5 1 0>,
+							 <0x1C 6 7 1 1>,
+							 <0x1C 5 6 1 0>,
+							 <0x94 3 5 1 1>,
+							 <0x8C 4 6 1 0>,
+							 <0x94 1 5 1 3>,
+							 <0x94 6 1 1 0>,
+							 <0xD0 3 5 1 2>,
+							 <0x98 6 0 1 1>,
+							 <0x94 5 6 1 3>,
+							 <0x94 1 5 1 0>,
+							 <0x98 2 6 1 1>,
+							 <0x8C 4 5 1 0>,
+							 <0x1C 7 6 1 1>,
+							 <0x8C 0 5 1 4>,
+							 <0x89 1 6 1 2>,
+							 <0x98 5 0 1 1>,
+							 <0x94 6 5 1 3>,
+							 <0x92 0 6 1 2>,
+							 <0x98 1 5 1 0>,
+							 <0x98 6 2 1 1>,
+							 <0xD0 0 5 1 3>,
+							 <0x94 6 0 1 1>,
+							 <0x8C 5 6 1 4>,
+							 <0x8C 1 5 1 0>,
+							 <0x1C 6 7 1 1>,
+							 <0x1C 5 6 1 0>,
+							 <0xB0 2 3 1 1>,
+							 <0xC4 4 5 1 0>,
+							 <0x8C 6 4 1 1>,
+							 <0xA4 3 6 1 0>,
+							 <0x1C 5 7 1 1>,
+							 <0x4C 0 5 1 4>,
+							 <0x8C 6 0 1 1>,
+							 <0x34 7 6 1 3>,
+							 <0x94 5 0 1 1>,
+							 <0x98 6 5 1 2>;
+			};
+
+			buffer-management-config {
+				/* qcom,group-config = <group group_buf>; */
+				qcom,group-config = <0 1550>;
+				/*
+				 * qcom,port-config = <group port prealloc react
+				 * ceil weight res_off res_ceil dynamic>;
+				 */
+				qcom,port-config = <0 0 0 100 1146 7 8 0 1>,
+						   <0 1 0 100 250 4 36 0 1>,
+						   <0 2 0 100 250 4 36 0 1>,
+						   <0 3 0 100 250 4 36 0 1>,
+						   <0 4 0 100 250 4 36 0 1>,
+						   <0 5 0 100 250 4 36 0 1>,
+						   <0 6 0 100 250 4 36 0 1>,
+						   <0 7 0 100 250 4 36 0 1>,
+						   <0 8 0 128 250 4 36 0 1>,
+						   <0 9 0 128 250 4 36 0 1>,
+						   <0 10 0 128 250 4 36 0 1>,
+						   <0 11 0 128 250 4 36 0 1>,
+						   <0 12 0 128 250 4 36 0 1>,
+						   <0 13 0 128 250 4 36 0 1>,
+						   <0 14 0 40 250 4 36 0 1>;
+			};
+
+			queue-management-config {
+				/*
+				 * qcom,group-config = <group total prealloc
+				 * ceil resume_off>;
+				 */
+				qcom,group-config = <0 2000 0 0 0>;
+				/*
+				 * qcom,queue-config = <queue_base queue_num
+				 * group prealloc ceil weight resume_off dynamic>;
+				 */
+				qcom,queue-config = <0 256 0 0 400 4 36 1>,
+						    <256 44 0 0 250 0 36 0>;
+			};
+
+			port-scheduler-resource {
+				port0 {
+					port-id = <0>;
+					qcom,ucast-queue = <0 63>;
+					qcom,mcast-queue = <256 263>;
+					qcom,l0sp = <0 0>;
+					qcom,l0cdrr = <0 7>;
+					qcom,l0edrr = <0 7>;
+					qcom,l1cdrr = <0 0>;
+					qcom,l1edrr = <0 0>;
+				};
+
+				port1 {
+					port-id = <1>;
+					qcom,ucast-queue = <204 211>;
+					qcom,mcast-queue = <272 275>;
+					qcom,l0sp = <51 52>;
+					qcom,l0cdrr = <108 115>;
+					qcom,l0edrr = <108 115>;
+					qcom,l1cdrr = <23 24>;
+					qcom,l1edrr = <23 24>;
+				};
+
+				port2 {
+					port-id = <2>;
+					qcom,ucast-queue = <212 219>;
+					qcom,mcast-queue = <276 279>;
+					qcom,l0sp = <53 54>;
+					qcom,l0cdrr = <116 123>;
+					qcom,l0edrr = <116 123>;
+					qcom,l1cdrr = <25 26>;
+					qcom,l1edrr = <25 26>;
+				};
+
+				port3 {
+					port-id = <3>;
+					qcom,ucast-queue = <220 227>;
+					qcom,mcast-queue = <280 283>;
+					qcom,l0sp = <55 56>;
+					qcom,l0cdrr = <124 131>;
+					qcom,l0edrr = <124 131>;
+					qcom,l1cdrr = <27 28>;
+					qcom,l1edrr = <27 28>;
+				};
+
+				port4 {
+					port-id = <4>;
+					qcom,ucast-queue = <228 235>;
+					qcom,mcast-queue = <284 287>;
+					qcom,l0sp = <57 58>;
+					qcom,l0cdrr = <132 139>;
+					qcom,l0edrr = <132 139>;
+					qcom,l1cdrr = <29 30>;
+					qcom,l1edrr = <29 30>;
+				};
+
+				port5 {
+					port-id = <5>;
+					qcom,ucast-queue = <236 243>;
+					qcom,mcast-queue = <288 291>;
+					qcom,l0sp = <59 60>;
+					qcom,l0cdrr = <140 147>;
+					qcom,l0edrr = <140 147>;
+					qcom,l1cdrr = <31 32>;
+					qcom,l1edrr = <31 32>;
+				};
+
+				port6 {
+					port-id = <6>;
+					qcom,ucast-queue = <244 251>;
+					qcom,mcast-queue = <292 295>;
+					qcom,l0sp = <61 62>;
+					qcom,l0cdrr = <148 155>;
+					qcom,l0edrr = <148 155>;
+					qcom,l1cdrr = <33 34>;
+					qcom,l1edrr = <33 34>;
+				};
+
+				port7 {
+					port-id = <7>;
+					qcom,ucast-queue = <252 255>;
+					qcom,mcast-queue = <296 299>;
+					qcom,l0sp = <63 63>;
+					qcom,l0cdrr = <156 159>;
+					qcom,l0edrr = <156 159>;
+					qcom,l1cdrr = <35 35>;
+					qcom,l1edrr = <35 35>;
+				};
+			};
+
+			port-scheduler-config {
+				port0 {
+					port-id = <0>;
+					l1scheduler {
+						group0 {
+							/* flow ID from L0 SP */
+							qcom,flow = <0>;
+							/* sp cpri cdrr epri edrr */
+							qcom,scheduler-config = <0 0 0 0 0>;
+						};
+					};
+
+					l0scheduler {
+						group0 {
+							/* unicast queue */
+							qcom,ucast-queue = <0>;
+							qcom,ucast-loop-priority = <8>;
+							/* multicast queue */
+							qcom,mcast-queue = <256>;
+							/* sp cpri cdrr epri edrr */
+							qcom,scheduler-config = <0 0 0 0 0>;
+						};
+
+						group1 {
+							qcom,ucast-queue = <8>;
+							qcom,ucast-loop-priority = <8>;
+							qcom,mcast-queue = <257>;
+							qcom,scheduler-config = <0 0 0 0 0>;
+						};
+
+						group2 {
+							qcom,ucast-queue = <16>;
+							qcom,ucast-loop-priority = <8>;
+							qcom,mcast-queue = <258>;
+							qcom,scheduler-config = <0 0 0 0 0>;
+						};
+
+						group3 {
+							qcom,ucast-queue = <24>;
+							qcom,ucast-loop-priority = <8>;
+							qcom,mcast-queue = <259>;
+							qcom,scheduler-config = <0 0 0 0 0>;
+						};
+
+						group4 {
+							qcom,ucast-queue = <32>;
+							qcom,ucast-loop-priority = <8>;
+							qcom,mcast-queue = <260>;
+							qcom,scheduler-config = <0 0 0 0 0>;
+						};
+
+						group5 {
+							qcom,ucast-queue = <40>;
+							qcom,ucast-loop-priority = <8>;
+							qcom,mcast-queue = <261>;
+							qcom,scheduler-config = <0 0 0 0 0>;
+						};
+
+						group6 {
+							qcom,ucast-queue = <48>;
+							qcom,ucast-loop-priority = <8>;
+							qcom,mcast-queue = <262>;
+							qcom,scheduler-config = <0 0 0 0 0>;
+						};
+
+						group7 {
+							qcom,ucast-queue = <56>;
+							qcom,ucast-loop-priority = <8>;
+							qcom,mcast-queue = <263>;
+							qcom,scheduler-config = <0 0 0 0 0>;
+						};
+					};
+				};
+
+				port1 {
+					port-id = <1>;
+					l1scheduler {
+						group0 {
+							qcom,flow = <51>;
+							qcom,flow-loop-priority = <2>;
+							qcom,scheduler-config = <1 0 23 0 23>;
+						};
+					};
+
+					l0scheduler {
+						group0 {
+							qcom,ucast-queue = <204>;
+							qcom,ucast-loop-priority = <8>;
+							/* max priority per SP */
+							qcom,drr-max-priority = <4>;
+							qcom,mcast-queue = <272>;
+							qcom,mcast-loop-priority = <4>;
+							qcom,scheduler-config = <51 0 108 0 108>;
+						};
+					};
+				};
+
+				port2 {
+					port-id = <2>;
+					l1scheduler {
+						group0 {
+							qcom,flow = <53>;
+							qcom,flow-loop-priority = <2>;
+							qcom,scheduler-config = <2 0 25 0 25>;
+						};
+					};
+
+					l0scheduler {
+						group0 {
+							qcom,ucast-queue = <212>;
+							qcom,ucast-loop-priority = <8>;
+							/* max priority per SP */
+							qcom,drr-max-priority = <4>;
+							qcom,mcast-queue = <276>;
+							qcom,mcast-loop-priority = <4>;
+							qcom,scheduler-config = <53 0 116 0 116>;
+						};
+					};
+				};
+
+				port3 {
+					port-id = <3>;
+					l1scheduler {
+						group0 {
+							qcom,flow = <55>;
+							qcom,flow-loop-priority = <2>;
+							qcom,scheduler-config = <3 0 27 0 27>;
+						};
+					};
+
+					l0scheduler {
+						group0 {
+							qcom,ucast-queue = <220>;
+							qcom,ucast-loop-priority = <8>;
+							/* max priority per SP */
+							qcom,drr-max-priority = <4>;
+							qcom,mcast-queue = <280>;
+							qcom,mcast-loop-priority = <4>;
+							qcom,scheduler-config = <55 0 124 0 124>;
+						};
+					};
+				};
+
+				port4 {
+					port-id = <4>;
+					l1scheduler {
+						group0 {
+							qcom,flow = <57>;
+							qcom,flow-loop-priority = <2>;
+							qcom,scheduler-config = <4 0 29 0 29>;
+						};
+					};
+
+					l0scheduler {
+						group0 {
+							qcom,ucast-queue = <228>;
+							qcom,ucast-loop-priority = <8>;
+							/* max priority per SP */
+							qcom,drr-max-priority = <4>;
+							qcom,mcast-queue = <284>;
+							qcom,mcast-loop-priority = <4>;
+							qcom,scheduler-config = <57 0 132 0 132>;
+						};
+					};
+				};
+
+				port5 {
+					port-id = <5>;
+					l1scheduler {
+						group0 {
+							qcom,flow = <59>;
+							qcom,flow-loop-priority = <2>;
+							qcom,scheduler-config = <5 0 31 0 31>;
+						};
+					};
+
+					l0scheduler {
+						group0 {
+							qcom,ucast-queue = <236>;
+							qcom,ucast-loop-priority = <8>;
+							/* max priority per SP */
+							qcom,drr-max-priority = <4>;
+							qcom,mcast-queue = <288>;
+							qcom,mcast-loop-priority = <4>;
+							qcom,scheduler-config = <59 0 140 0 140>;
+						};
+					};
+				};
+
+				port6 {
+					port-id = <6>;
+					l1scheduler {
+						group0 {
+							qcom,flow = <61>;
+							qcom,flow-loop-priority = <2>;
+							qcom,scheduler-config = <6 0 33 0 33>;
+						};
+					};
+
+					l0scheduler {
+						group0 {
+							qcom,ucast-queue = <244>;
+							qcom,ucast-loop-priority = <8>;
+							/* max priority per SP */
+							qcom,drr-max-priority = <4>;
+							qcom,mcast-queue = <292>;
+							qcom,mcast-loop-priority = <4>;
+							qcom,scheduler-config = <61 0 148 0 148>;
+						};
+					};
+				};
+
+				port7 {
+					port-id = <7>;
+					l1scheduler {
+						group0 {
+							qcom,flow = <63>;
+							qcom,scheduler-config = <7 0 35 0 35>;
+						};
+					};
+
+					l0scheduler {
+						group0 {
+							qcom,ucast-queue = <252>;
+							qcom,ucast-loop-priority = <4>;
+							qcom,mcast-queue = <296>;
+							qcom,mcast-loop-priority = <4>;
+							qcom,scheduler-config = <63 0 156 0 156>;
+						};
+					};
+				};
+			};
+		};
 	};
 
 	thermal-zones {
-- 
2.42.0

Re: [PATCH 1/6] arm64: dts: qcom: ipq9574: Add PPE device tree node

[Krzysztof Kozlowski]

On 10/01/2024 12:20, Luo Jie wrote:
> The PPE device tree node includes the PPE initialization configurations
> and UNIPHY instance configuration.
> 
> Ther are 3 UNIPHYs(PCS) on the platform ipq9574, which register the
> clock provider to output the clock for PPE port to work on the different
> link speed.
> 
> Signed-off-by: Luo Jie <quic_luoj@quicinc.com>
> ---
>  arch/arm64/boot/dts/qcom/ipq9574.dtsi | 730 +++++++++++++++++++++++++-
>  1 file changed, 724 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/ipq9574.dtsi b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
> index 810cda4a850f..5fa241e27c8b 100644
> --- a/arch/arm64/boot/dts/qcom/ipq9574.dtsi
> +++ b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
> @@ -775,16 +775,734 @@ nsscc: nsscc@39b00000 {
>  				 <&bias_pll_nss_noc_clk>,
>  				 <&bias_pll_ubi_nc_clk>,
>  				 <&gcc_gpll0_out_aux>,
> -				 <0>,
> -				 <0>,
> -				 <0>,
> -				 <0>,
> -				 <0>,
> -				 <0>,
> +				 <&uniphys 0>,
> +				 <&uniphys 1>,
> +				 <&uniphys 2>,
> +				 <&uniphys 3>,
> +				 <&uniphys 4>,
> +				 <&uniphys 5>,
>  				 <&xo_board_clk>;
>  			#clock-cells = <1>;
>  			#reset-cells = <1>;
>  		};
> +
> +		qcom_ppe: qcom-ppe@3a000000 {

qcom is definitely not a generic name.

Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation


> +			compatible = "qcom,ipq9574-ppe";

I don't see this documented. I don't see reference to posted bindings.

Please run scripts/checkpatch.pl and fix reported warnings. Some
warnings can be ignored, but the code here looks like it needs a fix.
Feel free to get in touch if the warning is not clear.

Ignoring this warning is a sign you don't really check your patches
before sending.

> +			reg = <0x3a000000 0xb00000>;
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +			ranges;

Put after reg.

> +			status = "okay";

Drop

All of above comments apply to your entire patchset and all places.

Looking at code further, it does not look like suitable for mainline,
but copy of downstream code. That's not what we expect upstream. Please
go back to your bindings first. Also, I really insist you reaching out
to other folks to help you in this process.

Best regards,
Krzysztof

Re: [PATCH 1/6] arm64: dts: qcom: ipq9574: Add PPE device tree node

[Jie Luo]

On 1/10/2024 7:40 PM, Krzysztof Kozlowski wrote:
> On 10/01/2024 12:20, Luo Jie wrote:
>> The PPE device tree node includes the PPE initialization configurations
>> and UNIPHY instance configuration.
>>
>> Ther are 3 UNIPHYs(PCS) on the platform ipq9574, which register the
>> clock provider to output the clock for PPE port to work on the different
>> link speed.
>>
>> Signed-off-by: Luo Jie <quic_luoj@quicinc.com>
>> ---
>>   arch/arm64/boot/dts/qcom/ipq9574.dtsi | 730 +++++++++++++++++++++++++-
>>   1 file changed, 724 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/ipq9574.dtsi b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
>> index 810cda4a850f..5fa241e27c8b 100644
>> --- a/arch/arm64/boot/dts/qcom/ipq9574.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
>> @@ -775,16 +775,734 @@ nsscc: nsscc@39b00000 {
>>   				 <&bias_pll_nss_noc_clk>,
>>   				 <&bias_pll_ubi_nc_clk>,
>>   				 <&gcc_gpll0_out_aux>,
>> -				 <0>,
>> -				 <0>,
>> -				 <0>,
>> -				 <0>,
>> -				 <0>,
>> -				 <0>,
>> +				 <&uniphys 0>,
>> +				 <&uniphys 1>,
>> +				 <&uniphys 2>,
>> +				 <&uniphys 3>,
>> +				 <&uniphys 4>,
>> +				 <&uniphys 5>,
>>   				 <&xo_board_clk>;
>>   			#clock-cells = <1>;
>>   			#reset-cells = <1>;
>>   		};
>> +
>> +		qcom_ppe: qcom-ppe@3a000000 {
> 
> qcom is definitely not a generic name.
> 
> Node names should be generic. See also an explanation and list of
> examples (not exhaustive) in DT specification:
> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation

Ok, will update to use a generic name in the link, Thanks for the
guidance and the link.
> 
> 
>> +			compatible = "qcom,ipq9574-ppe";
> 
> I don't see this documented. I don't see reference to posted bindings.

The DT bindings patch was part of the driver series as below. This
property was documented in the DT bindings patch. Attaching it to DTSI 
series should make it more clear. If this is fine, I will update the 
DTSI series with the DT bindings patch.
https://lore.kernel.org/netdev/20240110142428.52026d9e@kernel.org/

> 
> Please run scripts/checkpatch.pl and fix reported warnings. Some
> warnings can be ignored, but the code here looks like it needs a fix.
> Feel free to get in touch if the warning is not clear.
> 
> Ignoring this warning is a sign you don't really check your patches
> before sending.

We have run the checkpatch.pl on the whole patch series including this
device tree patch set together with PPE driver patch set.
As mentioned above, I will add the DT bindings patch into the DTS
series. This should help with the checkpatch issue.

> 
>> +			reg = <0x3a000000 0xb00000>;
>> +			#address-cells = <1>;
>> +			#size-cells = <1>;
>> +			ranges;
> 
> Put after reg.
Ok.

> 
>> +			status = "okay";
> 
> Drop
Ok.

> 
> All of above comments apply to your entire patchset and all places.
> 
> Looking at code further, it does not look like suitable for mainline,
> but copy of downstream code. That's not what we expect upstream. Please
> go back to your bindings first. Also, I really insist you reaching out
> to other folks to help you in this process.
> 
> Best regards,
> Krzysztof
> 
We will do internal review of the gaps and update the patches as per
your comments.

Thanks for the review comments.

Re: [PATCH 1/6] arm64: dts: qcom: ipq9574: Add PPE device tree node

[Dmitry Baryshkov]

On Thu, 11 Jan 2024 at 17:31, Jie Luo <quic_luoj@quicinc.com> wrote:
>
>
>
> On 1/10/2024 7:40 PM, Krzysztof Kozlowski wrote:
> > On 10/01/2024 12:20, Luo Jie wrote:
> >> The PPE device tree node includes the PPE initialization configurations
> >> and UNIPHY instance configuration.
> >>
> >> Ther are 3 UNIPHYs(PCS) on the platform ipq9574, which register the
> >> clock provider to output the clock for PPE port to work on the different
> >> link speed.
> >>
> >> Signed-off-by: Luo Jie <quic_luoj@quicinc.com>
> >> ---
> >>   arch/arm64/boot/dts/qcom/ipq9574.dtsi | 730 +++++++++++++++++++++++++-
> >>   1 file changed, 724 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/arch/arm64/boot/dts/qcom/ipq9574.dtsi b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
> >> index 810cda4a850f..5fa241e27c8b 100644
> >> --- a/arch/arm64/boot/dts/qcom/ipq9574.dtsi
> >> +++ b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
> >> @@ -775,16 +775,734 @@ nsscc: nsscc@39b00000 {
> >>                               <&bias_pll_nss_noc_clk>,
> >>                               <&bias_pll_ubi_nc_clk>,
> >>                               <&gcc_gpll0_out_aux>,
> >> -                             <0>,
> >> -                             <0>,
> >> -                             <0>,
> >> -                             <0>,
> >> -                             <0>,
> >> -                             <0>,
> >> +                             <&uniphys 0>,
> >> +                             <&uniphys 1>,
> >> +                             <&uniphys 2>,
> >> +                             <&uniphys 3>,
> >> +                             <&uniphys 4>,
> >> +                             <&uniphys 5>,
> >>                               <&xo_board_clk>;
> >>                      #clock-cells = <1>;
> >>                      #reset-cells = <1>;
> >>              };
> >> +
> >> +            qcom_ppe: qcom-ppe@3a000000 {
> >
> > qcom is definitely not a generic name.
> >
> > Node names should be generic. See also an explanation and list of
> > examples (not exhaustive) in DT specification:
> > https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
>
> Ok, will update to use a generic name in the link, Thanks for the
> guidance and the link.
> >
> >
> >> +                    compatible = "qcom,ipq9574-ppe";
> >
> > I don't see this documented. I don't see reference to posted bindings.
>
> The DT bindings patch was part of the driver series as below. This
> property was documented in the DT bindings patch. Attaching it to DTSI
> series should make it more clear. If this is fine, I will update the
> DTSI series with the DT bindings patch.
> https://lore.kernel.org/netdev/20240110142428.52026d9e@kernel.org/
>
> >
> > Please run scripts/checkpatch.pl and fix reported warnings. Some
> > warnings can be ignored, but the code here looks like it needs a fix.
> > Feel free to get in touch if the warning is not clear.
> >
> > Ignoring this warning is a sign you don't really check your patches
> > before sending.
>
> We have run the checkpatch.pl on the whole patch series including this
> device tree patch set together with PPE driver patch set.
> As mentioned above, I will add the DT bindings patch into the DTS
> series. This should help with the checkpatch issue.

This will cause even more confusion, as there will be two instances of
the dt-bindings patch. One in the driver patchset, another one in the
DT changes. You just have to specify the dependencies in the cover
letter. Another option is to wait for the bindings + driver to be
accepted, then send the DTSI changes (and again, specify the
dependency).

>
> >
> >> +                    reg = <0x3a000000 0xb00000>;
> >> +                    #address-cells = <1>;
> >> +                    #size-cells = <1>;
> >> +                    ranges;
> >
> > Put after reg.
> Ok.
>
> >
> >> +                    status = "okay";
> >
> > Drop
> Ok.
>
> >
> > All of above comments apply to your entire patchset and all places.
> >
> > Looking at code further, it does not look like suitable for mainline,
> > but copy of downstream code. That's not what we expect upstream. Please
> > go back to your bindings first. Also, I really insist you reaching out
> > to other folks to help you in this process.
> >
> > Best regards,
> > Krzysztof
> >
> We will do internal review of the gaps and update the patches as per
> your comments.
>
> Thanks for the review comments.

From the first glance, the bindings do not follow upstream principles.
You have all the settings (tdm, port config, etc) in the DT, while
they should instead go to the driver. Well, unless you expect that the
board might need to override them.

-- 
With best wishes
Dmitry

Re: [PATCH 1/6] arm64: dts: qcom: ipq9574: Add PPE device tree node

[Jie Luo]

On 1/12/2024 12:06 AM, Dmitry Baryshkov wrote:
> On Thu, 11 Jan 2024 at 17:31, Jie Luo <quic_luoj@quicinc.com> wrote:
>>
>>
>>
>>
>> Ok, will update to use a generic name in the link, Thanks for the
>> guidance and the link.
>>>
>>>
>>>> +                    compatible = "qcom,ipq9574-ppe";
>>>
>>> I don't see this documented. I don't see reference to posted bindings.
>>
>> The DT bindings patch was part of the driver series as below. This
>> property was documented in the DT bindings patch. Attaching it to DTSI
>> series should make it more clear. If this is fine, I will update the
>> DTSI series with the DT bindings patch.
>> https://lore.kernel.org/netdev/20240110142428.52026d9e@kernel.org/
>>
>>>
>>> Please run scripts/checkpatch.pl and fix reported warnings. Some
>>> warnings can be ignored, but the code here looks like it needs a fix.
>>> Feel free to get in touch if the warning is not clear.
>>>
>>> Ignoring this warning is a sign you don't really check your patches
>>> before sending.
>>
>> We have run the checkpatch.pl on the whole patch series including this
>> device tree patch set together with PPE driver patch set.
>> As mentioned above, I will add the DT bindings patch into the DTS
>> series. This should help with the checkpatch issue.
> 
> This will cause even more confusion, as there will be two instances of
> the dt-bindings patch. One in the driver patchset, another one in the
> DT changes. You just have to specify the dependencies in the cover
> letter. Another option is to wait for the bindings + driver to be
> accepted, then send the DTSI changes (and again, specify the
> dependency).
> 

Thanks Dmitry for the suggestions.


As per the ongoing discussion on this series, we will hold off this DTS 
patch series for some time. We will update the cover letter of the DTSI 
series to point to the below driver series as a dependency, when we 
resume the series.

https://lore.kernel.org/netdev/20240110154414.GH9296@kernel.org/

>>
>>>
>>>> +                    reg = <0x3a000000 0xb00000>;
>>>> +                    #address-cells = <1>;
>>>> +                    #size-cells = <1>;
>>>> +                    ranges;
>>>
>>> Put after reg.
>> Ok.
>>
>>>
>>>> +                    status = "okay";
>>>
>>> Drop
>> Ok.
>>
>>>
>>> All of above comments apply to your entire patchset and all places.
>>>
>>> Looking at code further, it does not look like suitable for mainline,
>>> but copy of downstream code. That's not what we expect upstream. Please
>>> go back to your bindings first. Also, I really insist you reaching out
>>> to other folks to help you in this process.
>>>
>>> Best regards,
>>> Krzysztof
>>>
>> We will do internal review of the gaps and update the patches as per
>> your comments.
>>
>> Thanks for the review comments.
> 
>  From the first glance, the bindings do not follow upstream principles.
> You have all the settings (tdm, port config, etc) in the DT, while
> they should instead go to the driver. Well, unless you expect that the
> board might need to override them.
> 
Hi Dmitry,
The TDM configuration varies per SoC type, since the ethernet port 
capabilities of the SoCs vary. So we will have two different TDM 
configurations for IPQ5332 and IPQ9574 SoC. The driver also will
need to support future SoC, so we choose to configure this from the 
DTSI. The same reason applies to the port scheduler config as well.

Thanks for review comments.

Re: [PATCH 1/6] arm64: dts: qcom: ipq9574: Add PPE device tree node

[Krzysztof Kozlowski]

On 12/01/2024 15:40, Jie Luo wrote:
>>
>>  From the first glance, the bindings do not follow upstream principles.
>> You have all the settings (tdm, port config, etc) in the DT, while
>> they should instead go to the driver. Well, unless you expect that the
>> board might need to override them.
>>
> Hi Dmitry,
> The TDM configuration varies per SoC type, since the ethernet port 
> capabilities of the SoCs vary. So we will have two different TDM 
> configurations for IPQ5332 and IPQ9574 SoC. The driver also will
> need to support future SoC, so we choose to configure this from the 
> DTSI. The same reason applies to the port scheduler config as well.

Your statements here confirm Dmitry suggestion, so these are not board
specific and should go to the driver. Please read again Dmitry's sentences.

Best regards,
Krzysztof

Re: [PATCH 1/6] arm64: dts: qcom: ipq9574: Add PPE device tree node

[Jie Luo]

On 1/12/2024 10:51 PM, Krzysztof Kozlowski wrote:
> On 12/01/2024 15:40, Jie Luo wrote:
>>>
>>>   From the first glance, the bindings do not follow upstream principles.
>>> You have all the settings (tdm, port config, etc) in the DT, while
>>> they should instead go to the driver. Well, unless you expect that the
>>> board might need to override them.
>>>
>> Hi Dmitry,
>> The TDM configuration varies per SoC type, since the ethernet port
>> capabilities of the SoCs vary. So we will have two different TDM
>> configurations for IPQ5332 and IPQ9574 SoC. The driver also will
>> need to support future SoC, so we choose to configure this from the
>> DTSI. The same reason applies to the port scheduler config as well.
> 
> Your statements here confirm Dmitry suggestion, so these are not board
> specific and should go to the driver. Please read again Dmitry's sentences.
> 
> Best regards,
> Krzysztof
> 

Sure, we will update the driver to configure the TDM depending on the 
SoC need.

Re: [PATCH 1/6] arm64: dts: qcom: ipq9574: Add PPE device tree node

[Dmitry Baryshkov]

On 12 January 2024 16:40:02 EET, Jie Luo <quic_luoj@quicinc.com> wrote:
>
>
>On 1/12/2024 12:06 AM, Dmitry Baryshkov wrote:
>> On Thu, 11 Jan 2024 at 17:31, Jie Luo <quic_luoj@quicinc.com> wrote:
>
>>> 
>>>> 
>>>>> +                    reg = <0x3a000000 0xb00000>;
>>>>> +                    #address-cells = <1>;
>>>>> +                    #size-cells = <1>;
>>>>> +                    ranges;
>>>> 
>>>> Put after reg.
>>> Ok.
>>> 
>>>> 
>>>>> +                    status = "okay";
>>>> 
>>>> Drop
>>> Ok.
>>> 
>>>> 
>>>> All of above comments apply to your entire patchset and all places.
>>>> 
>>>> Looking at code further, it does not look like suitable for mainline,
>>>> but copy of downstream code. That's not what we expect upstream. Please
>>>> go back to your bindings first. Also, I really insist you reaching out
>>>> to other folks to help you in this process.
>>>> 
>>>> Best regards,
>>>> Krzysztof
>>>> 
>>> We will do internal review of the gaps and update the patches as per
>>> your comments.
>>> 
>>> Thanks for the review comments.
>> 
>>  From the first glance, the bindings do not follow upstream principles.
>> You have all the settings (tdm, port config, etc) in the DT, while
>> they should instead go to the driver. Well, unless you expect that the
>> board might need to override them.
>> 
>Hi Dmitry,
>The TDM configuration varies per SoC type, since the ethernet port capabilities of the SoCs vary. So we will have two different TDM configurations for IPQ5332 and IPQ9574 SoC. The driver also will
>need to support future SoC, so we choose to configure this from the DTSI. The same reason applies to the port scheduler config as well.

If it differs from SoC to SoC only, it goes to the driver. Point. No other options. Thank you.

>
>Thanks for review comments.

Re: [PATCH 1/6] arm64: dts: qcom: ipq9574: Add PPE device tree node

[Jie Luo]

On 1/12/2024 11:03 PM, Dmitry Baryshkov wrote:
> On 12 January 2024 16:40:02 EET, Jie Luo <quic_luoj@quicinc.com> wrote:
>>
>>
>> On 1/12/2024 12:06 AM, Dmitry Baryshkov wrote:
>>> On Thu, 11 Jan 2024 at 17:31, Jie Luo <quic_luoj@quicinc.com> wrote:
>>
>>>>
>>>>>
>>>>>> +                    reg = <0x3a000000 0xb00000>;
>>>>>> +                    #address-cells = <1>;
>>>>>> +                    #size-cells = <1>;
>>>>>> +                    ranges;
>>>>>
>>>>> Put after reg.
>>>> Ok.
>>>>
>>>>>
>>>>>> +                    status = "okay";
>>>>>
>>>>> Drop
>>>> Ok.
>>>>
>>>>>
>>>>> All of above comments apply to your entire patchset and all places.
>>>>>
>>>>> Looking at code further, it does not look like suitable for mainline,
>>>>> but copy of downstream code. That's not what we expect upstream. Please
>>>>> go back to your bindings first. Also, I really insist you reaching out
>>>>> to other folks to help you in this process.
>>>>>
>>>>> Best regards,
>>>>> Krzysztof
>>>>>
>>>> We will do internal review of the gaps and update the patches as per
>>>> your comments.
>>>>
>>>> Thanks for the review comments.
>>>
>>>   From the first glance, the bindings do not follow upstream principles.
>>> You have all the settings (tdm, port config, etc) in the DT, while
>>> they should instead go to the driver. Well, unless you expect that the
>>> board might need to override them.
>>>
>> Hi Dmitry,
>> The TuratDM configion varies per SoC type, since the ethernet port capabilities of the SoCs vary. So we will have two different TDM configurations for IPQ5332 and IPQ9574 SoC. The driver also will
>> need to support future SoC, so we choose to configure this from the DTSI. The same reason applies to the port scheduler config as well.
> 
> If it differs from SoC to SoC only, it goes to the driver. Point. No other options. Thank you.

Understand it, Thanks for the advise, will move it to the driver code.

> 
>>
>> Thanks for review comments.
> 

[PATCH 2/6] arm64: dts: qcom: ipq5332: Add PPE device tree node

[Luo Jie]

The PPE device tree node includes the PPE initialization configurations
and the UNIPHY(PCS) instance configurations.

There are two UNIPHYs in ipq5332 platform, which provide the root clock
to the PPE port to work on the different link speed.

Signed-off-by: Luo Jie <quic_luoj@quicinc.com>
---
 arch/arm64/boot/dts/qcom/ipq5332.dtsi | 370 +++++++++++++++++++++++++-
 1 file changed, 366 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/ipq5332.dtsi b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
index a1504f6c40c1..bc89480820cb 100644
--- a/arch/arm64/boot/dts/qcom/ipq5332.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
@@ -7,6 +7,7 @@
 
 #include <dt-bindings/clock/qcom,apss-ipq.h>
 #include <dt-bindings/clock/qcom,ipq5332-gcc.h>
+#include <dt-bindings/clock/qcom,ipq5332-nsscc.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 
 / {
@@ -492,15 +493,376 @@ nsscc: clock-controller@39b00000{
 			clocks = <&cmn_pll_nss_200m_clk>,
 				 <&cmn_pll_nss_300m_clk>,
 				 <&gcc GPLL0_OUT_AUX>,
-				 <0>,
-				 <0>,
-				 <0>,
-				 <0>,
+				 <&uniphys 0>,
+				 <&uniphys 1>,
+				 <&uniphys 2>,
+				 <&uniphys 3>,
 				 <&xo_board>;
 			#clock-cells = <1>;
 			#reset-cells = <1>;
 			#power-domain-cells = <1>;
 		};
+
+		qcom_ppe: qcom-ppe@3a000000 {
+			compatible = "qcom,ipq5332-ppe";
+			reg = <0x3a000000 0xb00000>;
+			#address-cells = <1>;
+			#size-cells = <1>;
+			ranges;
+			status = "okay";
+
+			clocks = <&gcc GCC_CMN_12GPLL_AHB_CLK>,
+				 <&gcc GCC_CMN_12GPLL_SYS_CLK>,
+				 <&gcc GCC_UNIPHY0_SYS_CLK>,
+				 <&gcc GCC_UNIPHY1_SYS_CLK>,
+				 <&gcc GCC_UNIPHY0_AHB_CLK>,
+				 <&gcc GCC_UNIPHY1_AHB_CLK>,
+				 <&gcc GCC_NSSCC_CLK>,
+				 <&gcc GCC_NSSNOC_SNOC_CLK>,
+				 <&gcc GCC_NSSNOC_SNOC_1_CLK>,
+				 <&gcc GCC_IM_SLEEP_CLK>,
+				 <&nsscc NSS_CC_PPE_SWITCH_CLK>,
+				 <&nsscc NSS_CC_PPE_SWITCH_CFG_CLK>,
+				 <&nsscc NSS_CC_NSSNOC_PPE_CLK>,
+				 <&nsscc NSS_CC_NSSNOC_PPE_CFG_CLK>,
+				 <&nsscc NSS_CC_PPE_EDMA_CLK>,
+				 <&nsscc NSS_CC_PPE_EDMA_CFG_CLK>,
+				 <&nsscc NSS_CC_PPE_SWITCH_IPE_CLK>,
+				 <&nsscc NSS_CC_PPE_SWITCH_BTQ_CLK>,
+				 <&nsscc NSS_CC_PORT1_MAC_CLK>,
+				 <&nsscc NSS_CC_PORT2_MAC_CLK>,
+				 <&nsscc NSS_CC_PORT1_RX_CLK>,
+				 <&nsscc NSS_CC_PORT1_TX_CLK>,
+				 <&nsscc NSS_CC_PORT2_RX_CLK>,
+				 <&nsscc NSS_CC_PORT2_TX_CLK>,
+				 <&nsscc NSS_CC_UNIPHY_PORT1_RX_CLK>,
+				 <&nsscc NSS_CC_UNIPHY_PORT1_TX_CLK>,
+				 <&nsscc NSS_CC_UNIPHY_PORT2_RX_CLK>,
+				 <&nsscc NSS_CC_UNIPHY_PORT2_TX_CLK>;
+			clock-names = "cmn_ahb",
+				      "cmn_sys",
+				      "uniphy0_sys",
+				      "uniphy1_sys",
+				      "uniphy0_ahb",
+				      "uniphy1_ahb",
+				      "gcc_nsscc",
+				      "gcc_nssnoc_snoc",
+				      "gcc_nssnoc_snoc_1",
+				      "gcc_im_sleep",
+				      "nss_ppe",
+				      "nss_ppe_cfg",
+				      "nssnoc_ppe",
+				      "nssnoc_ppe_cfg",
+				      "nss_edma",
+				      "nss_edma_cfg",
+				      "nss_ppe_ipe",
+				      "nss_ppe_btq",
+				      "port1_mac",
+				      "port2_mac",
+				      "nss_port1_rx",
+				      "nss_port1_tx",
+				      "nss_port2_rx",
+				      "nss_port2_tx",
+				      "uniphy_port1_rx",
+				      "uniphy_port1_tx",
+				      "uniphy_port2_rx",
+				      "uniphy_port2_tx";
+
+			resets = <&nsscc NSS_CC_PPE_BCR>,
+				 <&gcc GCC_UNIPHY0_SYS_CLK_ARES>,
+				 <&gcc GCC_UNIPHY1_SYS_CLK_ARES>,
+				 <&gcc GCC_UNIPHY0_AHB_CLK_ARES>,
+				 <&gcc GCC_UNIPHY1_AHB_CLK_ARES>,
+				 <&gcc GCC_UNIPHY0_XPCS_ARES>,
+				 <&gcc GCC_UNIPHY1_XPCS_ARES>,
+				 <&gcc GCC_UNIPHY0_BCR>,
+				 <&gcc GCC_UNIPHY1_BCR>,
+				 <&nsscc NSS_CC_UNIPHY_PORT1_RX_CLK_ARES>,
+				 <&nsscc NSS_CC_UNIPHY_PORT1_TX_CLK_ARES>,
+				 <&nsscc NSS_CC_UNIPHY_PORT2_RX_CLK_ARES>,
+				 <&nsscc NSS_CC_UNIPHY_PORT2_TX_CLK_ARES>,
+				 <&nsscc NSS_CC_PORT1_RX_CLK_ARES>,
+				 <&nsscc NSS_CC_PORT1_TX_CLK_ARES>,
+				 <&nsscc NSS_CC_PORT2_RX_CLK_ARES>,
+				 <&nsscc NSS_CC_PORT2_TX_CLK_ARES>,
+				 <&nsscc NSS_CC_PORT1_MAC_CLK_ARES>,
+				 <&nsscc NSS_CC_PORT2_MAC_CLK_ARES>;
+			reset-names = "ppe",
+				      "uniphy0_sys",
+				      "uniphy1_sys",
+				      "uniphy0_ahb",
+				      "uniphy1_ahb",
+				      "uniphy0_xpcs",
+				      "uniphy1_xpcs",
+				      "uniphy0_soft",
+				      "uniphy1_soft",
+				      "uniphy_port1_rx",
+				      "uniphy_port1_tx",
+				      "uniphy_port2_rx",
+				      "uniphy_port2_tx",
+				      "nss_port1_rx",
+				      "nss_port1_tx",
+				      "nss_port2_rx",
+				      "nss_port2_tx",
+				      "nss_port1_mac",
+				      "nss_port2_mac";
+
+			uniphys: qcom-uniphy@7a00000 {
+				reg = <0x7a00000 0x10000>,
+				      <0x7a10000 0x10000>;
+				#clock-cells = <0x1>;
+				clock-output-names = "uniphy0_gcc_rx_clk",
+						     "uniphy0_gcc_tx_clk",
+						     "uniphy1_gcc_rx_clk",
+						     "uniphy1_gcc_tx_clk";
+			};
+
+			tdm-config {
+				/*
+				 * qcom,tdm-bm-config =
+				 * <valid egress port second_valid second_port>;
+				 */
+				qcom,tdm-bm-config = <1 0 2 0 0>,
+						     <1 1 0 0 0>,
+						     <1 0 1 0 0>,
+						     <1 1 1 0 0>,
+						     <1 0 2 0 0>,
+						     <1 1 2 0 0>,
+						     <1 0 0 0 0>,
+						     <1 1 0 0 0>,
+						     <1 0 2 0 0>,
+						     <1 1 1 0 0>,
+						     <1 0 1 0 0>,
+						     <1 1 2 0 0>,
+						     <1 0 0 0 0>,
+						     <1 1 0 0 0>,
+						     <1 0 1 0 0>,
+						     <1 1 1 0 0>,
+						     <1 0 2 0 0>,
+						     <1 1 2 0 0>,
+						     <1 0 1 0 0>,
+						     <1 1 0 0 0>,
+						     <1 0 2 0 0>,
+						     <1 1 1 0 0>,
+						     <1 0 0 0 0>,
+						     <1 1 2 0 0>,
+						     <1 0 2 0 0>,
+						     <1 1 0 0 0>,
+						     <1 0 1 0 0>,
+						     <1 1 1 0 0>,
+						     <1 0 0 0 0>,
+						     <1 1 2 0 0>,
+						     <1 0 1 0 0>,
+						     <0 0 0 0 0>;
+
+				/*
+				 * qcom,tdm-port-scheduler-config =
+				 * <ensch_bmp ensch_port desch_port
+				 * desch_second_valid desch_second_port>;
+				 */
+				qcom,tdm-port-scheduler-config = <0x0 2 0 0 0>,
+								 <0x0 1 2 0 0>,
+								 <0x0 0 1 0 0>,
+								 <0x0 0 2 0 0>,
+								 <0x0 1 0 0 0>,
+								 <0x0 2 1 0 0>,
+								 <0x0 0 2 0 0>,
+								 <0x0 0 1 0 0>,
+								 <0x0 0 2 0 0>,
+								 <0x0 0 1 0 0>;
+			};
+
+			buffer-management-config {
+				/* qcom,group-config = <group group_buf>; */
+				qcom,group-config = <0 240>;
+				/*
+				 * qcom,port-config =
+				 * <group port prealloc react ceil weight
+				 * res_off res_ceil dynamic>;
+				 */
+				qcom,port-config = <0 0 12 40 30 7 5 20 1>,
+						   <0 1 12 40 30 7 5 20 1>,
+						   <0 2 12 40 30 7 5 20 1>,
+						   <0 3 12 40 30 7 5 20 1>,
+						   <0 4 12 40 30 7 5 20 1>,
+						   <0 5 12 40 30 7 5 20 1>,
+						   <0 6 12 40 30 7 5 20 1>,
+						   <0 7 12 40 30 7 5 20 1>,
+						   <0 8 12 128 48 7 5 20 1>,
+						   <0 9 12 128 48 7 5 20 1>;
+			};
+
+			queue-management-config {
+				/*
+				 * qcom,group-config =
+				 * <group total prealloc ceil resume_off>;
+				 */
+				qcom,group-config = <0 500 0 0 0>;
+				/*
+				 * qcom,queue-config =
+				 * <queue_base queue_num group prealloc ceil
+				 * weight resume_off dynamic>;
+				 */
+				qcom,queue-config = <0 256 0 0 50 5 18 1>,
+						    <256 44 0 0 50 0 18 0>;
+			};
+
+			port-scheduler-resource {
+				port0 {
+					port-id = <0>;
+					qcom,ucast-queue = <0 63>;
+					qcom,mcast-queue = <256 263>;
+					qcom,l0sp = <0 0>;
+					qcom,l0cdrr = <0 7>;
+					qcom,l0edrr = <0 7>;
+					qcom,l1cdrr = <0 0>;
+					qcom,l1edrr = <0 0>;
+				};
+
+				port1 {
+					port-id = <1>;
+					qcom,ucast-queue = <204 211>;
+					qcom,mcast-queue = <272 275>;
+					qcom,l0sp = <51 52>;
+					qcom,l0cdrr = <108 115>;
+					qcom,l0edrr = <108 115>;
+					qcom,l1cdrr = <23 24>;
+					qcom,l1edrr = <23 24>;
+				};
+
+				port2 {
+					port-id = <2>;
+					qcom,ucast-queue = <212 219>;
+					qcom,mcast-queue = <276 279>;
+					qcom,l0sp = <53 54>;
+					qcom,l0cdrr = <116 123>;
+					qcom,l0edrr = <116 123>;
+					qcom,l1cdrr = <25 26>;
+					qcom,l1edrr = <25 26>;
+				};
+			};
+
+			port-scheduler-config {
+				port0 {
+					port-id = <0>;
+					l1scheduler {
+						group0 {
+							/* flow ID from L0 SP */
+							qcom,flow = <0>;
+							/* sp cpri cdrr epri edrr */
+							qcom,scheduler-config = <0 0 0 0 0>;
+						};
+					};
+
+					l0scheduler {
+						group0 {
+							/* unicast queue */
+							qcom,ucast-queue = <0>;
+							qcom,ucast-loop-priority = <8>;
+							/* multicast queue */
+							qcom,mcast-queue = <256>;
+							/* sp cpri cdrr epri edrr */
+							qcom,scheduler-config = <0 0 0 0 0>;
+						};
+
+						group1 {
+							qcom,ucast-queue = <8>;
+							qcom,ucast-loop-priority = <8>;
+							qcom,mcast-queue = <257>;
+							qcom,scheduler-config = <0 0 0 0 0>;
+						};
+
+						group2 {
+							qcom,ucast-queue = <16>;
+							qcom,ucast-loop-priority = <8>;
+							qcom,mcast-queue = <258>;
+							qcom,scheduler-config = <0 0 0 0 0>;
+						};
+
+						group3 {
+							qcom,ucast-queue = <24>;
+							qcom,ucast-loop-priority = <8>;
+							qcom,mcast-queue = <259>;
+							qcom,scheduler-config = <0 0 0 0 0>;
+						};
+
+						group4 {
+							qcom,ucast-queue = <32>;
+							qcom,ucast-loop-priority = <8>;
+							qcom,mcast-queue = <260>;
+							qcom,scheduler-config = <0 0 0 0 0>;
+						};
+
+						group5 {
+							qcom,ucast-queue = <40>;
+							qcom,ucast-loop-priority = <8>;
+							qcom,mcast-queue = <261>;
+							qcom,scheduler-config = <0 0 0 0 0>;
+						};
+
+						group6 {
+							qcom,ucast-queue = <48>;
+							qcom,ucast-loop-priority = <8>;
+							qcom,mcast-queue = <262>;
+							qcom,scheduler-config = <0 0 0 0 0>;
+						};
+
+						group7 {
+							qcom,ucast-queue = <56>;
+							qcom,ucast-loop-priority = <8>;
+							qcom,mcast-queue = <263>;
+							qcom,scheduler-config = <0 0 0 0 0>;
+						};
+					};
+				};
+
+				port1 {
+					port-id = <1>;
+					l1scheduler {
+						group0 {
+							qcom,flow = <51>;
+							qcom,flow-loop-priority = <2>;
+							qcom,scheduler-config = <1 0 23 0 23>;
+						};
+					};
+
+					l0scheduler {
+						group0 {
+							qcom,ucast-queue = <204>;
+							qcom,ucast-loop-priority = <8>;
+							/* max priority per SP */
+							qcom,drr-max-priority = <4>;
+							qcom,mcast-queue = <272>;
+							qcom,mcast-loop-priority = <4>;
+							qcom,scheduler-config = <51 0 108 0 108>;
+						};
+					};
+				};
+
+				port2 {
+					port-id = <2>;
+					l1scheduler {
+						group0 {
+							qcom,flow = <53>;
+							qcom,flow-loop-priority = <2>;
+							qcom,scheduler-config = <2 0 25 0 25>;
+						};
+					};
+
+					l0scheduler {
+						group0 {
+							qcom,ucast-queue = <212>;
+							qcom,ucast-loop-priority = <8>;
+							/* max priority per SP */
+							qcom,drr-max-priority = <4>;
+							qcom,mcast-queue = <276>;
+							qcom,mcast-loop-priority = <4>;
+							qcom,scheduler-config = <53 0 116 0 116>;
+						};
+					};
+				};
+			};
+		};
 	};
 
 	timer {
-- 
2.42.0

[PATCH 3/6] arm64: dts: qcom: ipq5332: Add MDIO device tree

[Luo Jie]

Add the MDIO device tree of ipq5332.

Signed-off-by: Luo Jie <quic_luoj@quicinc.com>
---
 arch/arm64/boot/dts/qcom/ipq5332.dtsi | 44 +++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/ipq5332.dtsi b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
index bc89480820cb..e6c780e69d6e 100644
--- a/arch/arm64/boot/dts/qcom/ipq5332.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
@@ -214,6 +214,38 @@ serial_0_pins: serial0-state {
 				drive-strength = <8>;
 				bias-pull-up;
 			};
+
+			mdio0_pins: mdio0-state {
+				mux_0 {
+					pins = "gpio25";
+					function = "mdc0";
+					drive-strength = <8>;
+					bias-disable;
+				};
+
+				mux_1 {
+					pins = "gpio26";
+					function = "mdio0";
+					drive-strength = <8>;
+					bias-pull-up;
+				};
+			};
+
+			mdio1_pins: mdio1-state {
+				mux_0 {
+					pins = "gpio27";
+					function = "mdc1";
+					drive-strength = <8>;
+					bias-disable;
+				};
+
+				mux_1 {
+					pins = "gpio28";
+					function = "mdio1";
+					drive-strength = <8>;
+					bias-pull-up;
+				};
+			};
 		};
 
 		gcc: clock-controller@1800000 {
@@ -863,6 +895,18 @@ group0 {
 				};
 			};
 		};
+
+		mdio: mdio@90000 {
+			compatible = "qcom,ipq4019-mdio";
+			reg = <0x90000 0x64>;
+			pinctrl-0 = <&mdio1_pins &mdio0_pins>;
+			pinctrl-names = "default";
+			#address-cells = <1>;
+			#size-cells = <0>;
+			clocks = <&gcc GCC_MDIO_AHB_CLK>;
+			clock-names = "gcc_mdio_ahb_clk";
+			status = "disabled";
+		};
 	};
 
 	timer {
-- 
2.42.0

Re: [PATCH 3/6] arm64: dts: qcom: ipq5332: Add MDIO device tree

[Krzysztof Kozlowski]

On 10/01/2024 12:20, Luo Jie wrote:
> Add the MDIO device tree of ipq5332.

Subject: drop "device tree", it is obvious. Commit msg: say something
more instead of copying the subject. Or better squash the entire patch.
> 
> Signed-off-by: Luo Jie <quic_luoj@quicinc.com>
> ---
>  arch/arm64/boot/dts/qcom/ipq5332.dtsi | 44 +++++++++++++++++++++++++++
>  1 file changed, 44 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/ipq5332.dtsi b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
> index bc89480820cb..e6c780e69d6e 100644
> --- a/arch/arm64/boot/dts/qcom/ipq5332.dtsi
> +++ b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
> @@ -214,6 +214,38 @@ serial_0_pins: serial0-state {
>  				drive-strength = <8>;
>  				bias-pull-up;
>  			};
> +
> +			mdio0_pins: mdio0-state {
> +				mux_0 {

This wasn't tested...

It does not look like you tested the DTS against bindings. Please run
`make dtbs_check W=1` (see
Documentation/devicetree/bindings/writing-schema.rst or
https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
for instructions).


Best regards,
Krzysztof

Re: [PATCH 3/6] arm64: dts: qcom: ipq5332: Add MDIO device tree

[Jie Luo]

On 1/10/2024 7:56 PM, Krzysztof Kozlowski wrote:
> On 10/01/2024 12:20, Luo Jie wrote:
>> Add the MDIO device tree of ipq5332.
> 
> Subject: drop "device tree", it is obvious. Commit msg: say something
> more instead of copying the subject. Or better squash the entire patch.

Will update the commit message to improve the description when the 
series is updated.

>>
>> Signed-off-by: Luo Jie <quic_luoj@quicinc.com>
>> ---
>>   arch/arm64/boot/dts/qcom/ipq5332.dtsi | 44 +++++++++++++++++++++++++++
>>   1 file changed, 44 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/ipq5332.dtsi b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
>> index bc89480820cb..e6c780e69d6e 100644
>> --- a/arch/arm64/boot/dts/qcom/ipq5332.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
>> @@ -214,6 +214,38 @@ serial_0_pins: serial0-state {
>>   				drive-strength = <8>;
>>   				bias-pull-up;
>>   			};
>> +
>> +			mdio0_pins: mdio0-state {
>> +				mux_0 {
> 
> This wasn't tested...
> 
> It does not look like you tested the DTS against bindings. Please run
> `make dtbs_check W=1` (see
> Documentation/devicetree/bindings/writing-schema.rst or
> https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
> for instructions).
> 
> 
> Best regards,
> Krzysztof
> 

We will follow up the guidance mentioned in the link to validate the DTS 
patches.

Re: [PATCH 3/6] arm64: dts: qcom: ipq5332: Add MDIO device tree

[Andrew Lunn]

On Wed, Jan 10, 2024 at 07:20:56PM +0800, Luo Jie wrote:
> Add the MDIO device tree of ipq5332.
> 
> Signed-off-by: Luo Jie <quic_luoj@quicinc.com>
> ---
>  arch/arm64/boot/dts/qcom/ipq5332.dtsi | 44 +++++++++++++++++++++++++++
>  1 file changed, 44 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/ipq5332.dtsi b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
> index bc89480820cb..e6c780e69d6e 100644
> --- a/arch/arm64/boot/dts/qcom/ipq5332.dtsi
> +++ b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
> @@ -214,6 +214,38 @@ serial_0_pins: serial0-state {
>  				drive-strength = <8>;
>  				bias-pull-up;
>  			};
> +
> +			mdio0_pins: mdio0-state {
> +				mux_0 {
> +					pins = "gpio25";
> +					function = "mdc0";
> +					drive-strength = <8>;
> +					bias-disable;
> +				};
> +
> +				mux_1 {
> +					pins = "gpio26";
> +					function = "mdio0";
> +					drive-strength = <8>;
> +					bias-pull-up;
> +				};
> +			};
> +
> +			mdio1_pins: mdio1-state {
> +				mux_0 {
> +					pins = "gpio27";
> +					function = "mdc1";
> +					drive-strength = <8>;
> +					bias-disable;
> +				};
> +
> +				mux_1 {
> +					pins = "gpio28";
> +					function = "mdio1";
> +					drive-strength = <8>;
> +					bias-pull-up;
> +				};

I don't know why i'm asking this, because i don't really expect a
usable answer. What sort of MUX is this? Should you be using one of
the muxes in drivers/net/mdio/mdio-mux-* or something similar?

    Andrew

Re: [PATCH 3/6] arm64: dts: qcom: ipq5332: Add MDIO device tree

[Jie Luo]

On 1/10/2024 9:35 PM, Andrew Lunn wrote:
> On Wed, Jan 10, 2024 at 07:20:56PM +0800, Luo Jie wrote:
>> Add the MDIO device tree of ipq5332.
>>
>> Signed-off-by: Luo Jie <quic_luoj@quicinc.com>
>> ---
>>   arch/arm64/boot/dts/qcom/ipq5332.dtsi | 44 +++++++++++++++++++++++++++
>>   1 file changed, 44 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/ipq5332.dtsi b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
>> index bc89480820cb..e6c780e69d6e 100644
>> --- a/arch/arm64/boot/dts/qcom/ipq5332.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
>> @@ -214,6 +214,38 @@ serial_0_pins: serial0-state {
>>   				drive-strength = <8>;
>>   				bias-pull-up;
>>   			};
>> +
>> +			mdio0_pins: mdio0-state {
>> +				mux_0 {
>> +					pins = "gpio25";
>> +					function = "mdc0";
>> +					drive-strength = <8>;
>> +					bias-disable;
>> +				};
>> +
>> +				mux_1 {
>> +					pins = "gpio26";
>> +					function = "mdio0";
>> +					drive-strength = <8>;
>> +					bias-pull-up;
>> +				};
>> +			};
>> +
>> +			mdio1_pins: mdio1-state {
>> +				mux_0 {
>> +					pins = "gpio27";
>> +					function = "mdc1";
>> +					drive-strength = <8>;
>> +					bias-disable;
>> +				};
>> +
>> +				mux_1 {
>> +					pins = "gpio28";
>> +					function = "mdio1";
>> +					drive-strength = <8>;
>> +					bias-pull-up;
>> +				};
> 
> I don't know why i'm asking this, because i don't really expect a
> usable answer. What sort of MUX is this? Should you be using one of
> the muxes in drivers/net/mdio/mdio-mux-* or something similar?
> 
>      Andrew

Sorry for the confusion, the pin nodes are for the MDIO and MDC, these
PINs are used by the dedicated hardware MDIO block in the SoC. I will 
update the node name from mux_0 to MDC, mux_1 to MDIO, to make it clear. 
The driver for this node is drivers/net/mdio/mdio-ipq4019.c, it is not 
related to the mdio-mux-* code.

Re: [PATCH 3/6] arm64: dts: qcom: ipq5332: Add MDIO device tree

[Dmitry Baryshkov]

On Thu, 11 Jan 2024 at 18:00, Jie Luo <quic_luoj@quicinc.com> wrote:
>
>
>
> On 1/10/2024 9:35 PM, Andrew Lunn wrote:
> > On Wed, Jan 10, 2024 at 07:20:56PM +0800, Luo Jie wrote:
> >> Add the MDIO device tree of ipq5332.
> >>
> >> Signed-off-by: Luo Jie <quic_luoj@quicinc.com>
> >> ---
> >>   arch/arm64/boot/dts/qcom/ipq5332.dtsi | 44 +++++++++++++++++++++++++++
> >>   1 file changed, 44 insertions(+)
> >>
> >> diff --git a/arch/arm64/boot/dts/qcom/ipq5332.dtsi b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
> >> index bc89480820cb..e6c780e69d6e 100644
> >> --- a/arch/arm64/boot/dts/qcom/ipq5332.dtsi
> >> +++ b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
> >> @@ -214,6 +214,38 @@ serial_0_pins: serial0-state {
> >>                              drive-strength = <8>;
> >>                              bias-pull-up;
> >>                      };
> >> +
> >> +                    mdio0_pins: mdio0-state {
> >> +                            mux_0 {
> >> +                                    pins = "gpio25";
> >> +                                    function = "mdc0";
> >> +                                    drive-strength = <8>;
> >> +                                    bias-disable;
> >> +                            };
> >> +
> >> +                            mux_1 {
> >> +                                    pins = "gpio26";
> >> +                                    function = "mdio0";
> >> +                                    drive-strength = <8>;
> >> +                                    bias-pull-up;
> >> +                            };
> >> +                    };
> >> +
> >> +                    mdio1_pins: mdio1-state {
> >> +                            mux_0 {
> >> +                                    pins = "gpio27";
> >> +                                    function = "mdc1";
> >> +                                    drive-strength = <8>;
> >> +                                    bias-disable;
> >> +                            };
> >> +
> >> +                            mux_1 {
> >> +                                    pins = "gpio28";
> >> +                                    function = "mdio1";
> >> +                                    drive-strength = <8>;
> >> +                                    bias-pull-up;
> >> +                            };
> >
> > I don't know why i'm asking this, because i don't really expect a
> > usable answer. What sort of MUX is this? Should you be using one of
> > the muxes in drivers/net/mdio/mdio-mux-* or something similar?
> >
> >      Andrew
>
> Sorry for the confusion, the pin nodes are for the MDIO and MDC, these
> PINs are used by the dedicated hardware MDIO block in the SoC. I will
> update the node name from mux_0 to MDC, mux_1 to MDIO, to make it clear.
> The driver for this node is drivers/net/mdio/mdio-ipq4019.c, it is not
> related to the mdio-mux-* code.

Have you read Documentation/devicetree/bindings/pinctrl/qcom,ipq5332-tlmm.yaml
? Have you validated your DTSI files against DT schema? How many
warnings will you observe if you rename the mux_0 node to MDC?

-- 
With best wishes
Dmitry

Re: [PATCH 3/6] arm64: dts: qcom: ipq5332: Add MDIO device tree

[Jie Luo]

On 1/12/2024 12:13 AM, Dmitry Baryshkov wrote:
> On Thu, 11 Jan 2024 at 18:00, Jie Luo <quic_luoj@quicinc.com> wrote:
>>
>>
>>
>> On 1/10/2024 9:35 PM, Andrew Lunn wrote:
>>> On Wed, Jan 10, 2024 at 07:20:56PM +0800, Luo Jie wrote:
>>>> Add the MDIO device tree of ipq5332.
>>>>
>>>> Signed-off-by: Luo Jie <quic_luoj@quicinc.com>
>>>> ---
>>>>    arch/arm64/boot/dts/qcom/ipq5332.dtsi | 44 +++++++++++++++++++++++++++
>>>>    1 file changed, 44 insertions(+)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/qcom/ipq5332.dtsi b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
>>>> index bc89480820cb..e6c780e69d6e 100644
>>>> --- a/arch/arm64/boot/dts/qcom/ipq5332.dtsi
>>>> +++ b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
>>>> @@ -214,6 +214,38 @@ serial_0_pins: serial0-state {
>>>>                               drive-strength = <8>;
>>>>                               bias-pull-up;
>>>>                       };
>>>> +
>>>> +                    mdio0_pins: mdio0-state {
>>>> +                            mux_0 {
>>>> +                                    pins = "gpio25";
>>>> +                                    function = "mdc0";
>>>> +                                    drive-strength = <8>;
>>>> +                                    bias-disable;
>>>> +                            };
>>>> +
>>>> +                            mux_1 {
>>>> +                                    pins = "gpio26";
>>>> +                                    function = "mdio0";
>>>> +                                    drive-strength = <8>;
>>>> +                                    bias-pull-up;
>>>> +                            };
>>>> +                    };
>>>> +
>>>> +                    mdio1_pins: mdio1-state {
>>>> +                            mux_0 {
>>>> +                                    pins = "gpio27";
>>>> +                                    function = "mdc1";
>>>> +                                    drive-strength = <8>;
>>>> +                                    bias-disable;
>>>> +                            };
>>>> +
>>>> +                            mux_1 {
>>>> +                                    pins = "gpio28";
>>>> +                                    function = "mdio1";
>>>> +                                    drive-strength = <8>;
>>>> +                                    bias-pull-up;
>>>> +                            };
>>>
>>> I don't know why i'm asking this, because i don't really expect a
>>> usable answer. What sort of MUX is this? Should you be using one of
>>> the muxes in drivers/net/mdio/mdio-mux-* or something similar?
>>>
>>>       Andrew
>>
>> Sorry for the confusion, the pin nodes are for the MDIO and MDC, these
>> PINs are used by the dedicated hardware MDIO block in the SoC. I will
>> update the node name from mux_0 to MDC, mux_1 to MDIO, to make it clear.
>> The driver for this node is drivers/net/mdio/mdio-ipq4019.c, it is not
>> related to the mdio-mux-* code.
> 
> Have you read Documentation/devicetree/bindings/pinctrl/qcom,ipq5332-tlmm.yaml
> ? Have you validated your DTSI files against DT schema? How many
> warnings will you observe if you rename the mux_0 node to MDC?
> 
Sorry for this error, we will follow the DTSI validation process and 
update the patch with the right updates after validation, when the patch 
series resumes.

Thanks for correction and the pointer to the tlmm YAML file.

Re: [PATCH 3/6] arm64: dts: qcom: ipq5332: Add MDIO device tree

[Andrew Lunn]

> Sorry for the confusion, the pin nodes are for the MDIO and MDC, these
> PINs are used by the dedicated hardware MDIO block in the SoC. I will update
> the node name from mux_0 to MDC, mux_1 to MDIO, to make it clear. The driver
> for this node is drivers/net/mdio/mdio-ipq4019.c, it is not related to the
> mdio-mux-* code.

So these is all about pinmux.

When you say:
> PINs are used by the dedicated hardware MDIO block in the SoC

do you actually mean:

PINs are used by the two dedicated hardware MDIO blocks in the SoC.

You have two sets of mdio/mdc configurations here, so i assume there
are two MDIO hardware blocks, each being an MDIO bus master.

    Andrew

Re: [PATCH 3/6] arm64: dts: qcom: ipq5332: Add MDIO device tree

[Jie Luo]

On 1/12/2024 12:30 AM, Andrew Lunn wrote:
>> Sorry for the confusion, the pin nodes are for the MDIO and MDC, these
>> PINs are used by the dedicated hardware MDIO block in the SoC. I will update
>> the node name from mux_0 to MDC, mux_1 to MDIO, to make it clear. The driver
>> for this node is drivers/net/mdio/mdio-ipq4019.c, it is not related to the
>> mdio-mux-* code.
> 
> So these is all about pinmux.

Yes, it is about pinmux.

> 
> When you say:
>> PINs are used by the dedicated hardware MDIO block in the SoC
> 
> do you actually mean:
> 
> PINs are used by the two dedicated hardware MDIO blocks in the SoC.
> 
> You have two sets of mdio/mdc configurations here, so i assume there
> are two MDIO hardware blocks, each being an MDIO bus master.
> 
>      Andrew

There are two MDIO hardware blocks on IPQ5332 SoC. One is
for the MDIO bus master(gpio27, 28), which is for accessing the MDIO
slave devices(like PHY device). The mdio-ipq4019.c driver enables
this MDIO bus master.

Another one is the MDIO slave(gpio25, 26), which is dedicated
for receiving the back pressure signal from the connected Ethernet 
switch device QCA8386.

There is a MDIO master block integrated in QCA8386 switch device, this
integrated MDIO master is dedicated for generating the back
pressure signal to IPQ5332 SoC.

This MDIO slave block of IPQ5322 just needs to configure these PIN
mux for MDC and MDIO PINs. No additional driver is needed for this MDIO
slave block of IPQ5332.

Re: [PATCH 3/6] arm64: dts: qcom: ipq5332: Add MDIO device tree

[Andrew Lunn]

> Another one is the MDIO slave(gpio25, 26), which is dedicated
> for receiving the back pressure signal from the connected Ethernet switch
> device QCA8386.
> 
> There is a MDIO master block integrated in QCA8386 switch device, this
> integrated MDIO master is dedicated for generating the back
> pressure signal to IPQ5332 SoC.
> 
> This MDIO slave block of IPQ5322 just needs to configure these PIN
> mux for MDC and MDIO PINs. No additional driver is needed for this MDIO
> slave block of IPQ5332.

So there is a proprietary protocol running over the MDIO bus? And its
completely implemented in hardware in the slave block? Is this even
MDIO? Does it use c22 or c45 bus transactions? How is the slave
address configured, or is that also hard coded?

	Andrew

Re: [PATCH 3/6] arm64: dts: qcom: ipq5332: Add MDIO device tree

[Jie Luo]

On 1/17/2024 6:56 AM, Andrew Lunn wrote:
>> Another one is the MDIO slave(gpio25, 26), which is dedicated
>> for receiving the back pressure signal from the connected Ethernet switch
>> device QCA8386.
>>
>> There is a MDIO master block integrated in QCA8386 switch device, this
>> integrated MDIO master is dedicated for generating the back
>> pressure signal to IPQ5332 SoC.
>>
>> This MDIO slave block of IPQ5322 just needs to configure these PIN
>> mux for MDC and MDIO PINs. No additional driver is needed for this MDIO
>> slave block of IPQ5332.
> 
> So there is a proprietary protocol running over the MDIO bus? And its
> completely implemented in hardware in the slave block? Is this even
> MDIO? Does it use c22 or c45 bus transactions? How is the slave
> address configured, or is that also hard coded?
> 
> 	Andrew
> 

Hi Andrew,
Yes, this is a custom HW mechanism using the MDIO C22 frame, to enable 
back pressure from the QCA8386 switch to the IPQ5332 SoC. The slave 
block in the IPQ5332 SoC implements the back pressure function. There is 
no configuration for the MDIO slave address of IPQ5332 required, since 
the connection is one to one between slave and master.

However upon further review, we believe this node definition belongs to
the board DTS file, since the switch configuration is a board property.
We will move out this MDIO slave config from the patch series to
avoid the confusion. We will also rename the node from 'mdio0-state' to 
'backpressure-state' to make this clear.

Thanks.

Re: [PATCH 3/6] arm64: dts: qcom: ipq5332: Add MDIO device tree

[Krzysztof Kozlowski]

On 11/01/2024 16:59, Jie Luo wrote:
>>> +				mux_1 {
>>> +					pins = "gpio28";
>>> +					function = "mdio1";
>>> +					drive-strength = <8>;
>>> +					bias-pull-up;
>>> +				};
>>
>> I don't know why i'm asking this, because i don't really expect a
>> usable answer. What sort of MUX is this? Should you be using one of
>> the muxes in drivers/net/mdio/mdio-mux-* or something similar?
>>
>>      Andrew
> 
> Sorry for the confusion, the pin nodes are for the MDIO and MDC, these
> PINs are used by the dedicated hardware MDIO block in the SoC. I will 
> update the node name from mux_0 to MDC, mux_1 to MDIO, to make it clear. 
> The driver for this node is drivers/net/mdio/mdio-ipq4019.c, it is not 
> related to the mdio-mux-* code.

NAK. Run dtbs_check tests on your DTS first.

Best regards,
Krzysztof

[PATCH 4/6] arm64: dts: qcom: ipq9574: Add MDIO device tree

[Luo Jie]

Add MDIO device tree of ipq9574.

Signed-off-by: Luo Jie <quic_luoj@quicinc.com>
---
 arch/arm64/boot/dts/qcom/ipq9574.dtsi | 28 +++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/ipq9574.dtsi b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
index 5fa241e27c8b..cf21ce9bf756 100644
--- a/arch/arm64/boot/dts/qcom/ipq9574.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
@@ -316,6 +316,22 @@ uart2_pins: uart2-state {
 				drive-strength = <8>;
 				bias-disable;
 			};
+
+			mdio_pins: mdio-state {
+				mux_0 {
+					pins = "gpio38";
+					function = "mdc";
+					drive-strength = <8>;
+					bias-disable;
+				};
+
+				mux_1 {
+					pins = "gpio39";
+					function = "mdio";
+					drive-strength = <8>;
+					bias-pull-up;
+				};
+			};
 		};
 
 		gcc: clock-controller@1800000 {
@@ -1503,6 +1519,18 @@ group0 {
 				};
 			};
 		};
+
+		mdio: mdio@90000 {
+			compatible = "qcom,ipq4019-mdio";
+			reg = <0x90000 0x64>;
+			pinctrl-0 = <&mdio_pins>;
+			pinctrl-names = "default";
+			#address-cells = <1>;
+			#size-cells = <0>;
+			clocks = <&gcc GCC_MDIO_AHB_CLK>;
+			clock-names = "gcc_mdio_ahb_clk";
+			status = "disabled";
+		};
 	};
 
 	thermal-zones {
-- 
2.42.0

[PATCH 5/6] arm64: dts: qcom: ipq5332: Add RDP441 board device tree

[Luo Jie]

From: Lei Wei <quic_leiwei@quicinc.com>

RDP441 board has onboard QCA8386 switch and 10G SFP port.

Signed-off-by: Lei Wei <quic_leiwei@quicinc.com>
Signed-off-by: Luo Jie <quic_luoj@quicinc.com>
---
 arch/arm64/boot/dts/qcom/ipq5332-rdp441.dts | 51 +++++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/ipq5332-rdp441.dts b/arch/arm64/boot/dts/qcom/ipq5332-rdp441.dts
index 846413817e9a..d51968e9d601 100644
--- a/arch/arm64/boot/dts/qcom/ipq5332-rdp441.dts
+++ b/arch/arm64/boot/dts/qcom/ipq5332-rdp441.dts
@@ -12,6 +12,15 @@
 / {
 	model = "Qualcomm Technologies, Inc. IPQ5332 MI01.2";
 	compatible = "qcom,ipq5332-ap-mi01.2", "qcom,ipq5332";
+
+	soc@0 {
+		sfp1: sfp-1 {
+			compatible = "sff,sfp";
+			i2c-bus = <&blsp1_i2c1>;
+			los-gpios = <&tlmm 45 GPIO_ACTIVE_HIGH>;
+			tx-disable-gpios = <&tlmm 24 GPIO_ACTIVE_HIGH>;
+		};
+	};
 };
 
 &blsp1_i2c1 {
@@ -63,3 +72,45 @@ data-pins {
 		};
 	};
 };
+
+&qcom_ppe {
+	qcom,port_phyinfo {
+		ppe_port0: port@0 {
+			port_id = <1>;
+			phy-mode = "2500base-x";
+			fixed-link {
+				speed = <2500>;
+				full-duplex;
+				pause;
+			};
+		};
+
+		ppe_port1: port@1 {
+			port_id = <2>;
+			phy-mode = "10gbase-r";
+			sfp = <&sfp1>;
+			managed = "in-band-status";
+		};
+	};
+};
+
+&mdio {
+	status = "okay";
+	reset-gpios = <&tlmm 51 GPIO_ACTIVE_LOW>;
+
+	phy0: ethernet-phy@0 {
+		      reg = <1>;
+	      };
+
+	phy1: ethernet-phy@1 {
+		      reg = <2>;
+	      };
+
+	phy2: ethernet-phy@2 {
+		      reg = <3>;
+	      };
+
+	phy3: ethernet-phy@3 {
+		      reg = <4>;
+	      };
+};
-- 
2.42.0

Re: [PATCH 5/6] arm64: dts: qcom: ipq5332: Add RDP441 board device tree

[Krzysztof Kozlowski]

On 10/01/2024 12:20, Luo Jie wrote:
> From: Lei Wei <quic_leiwei@quicinc.com>
> 
> RDP441 board has onboard QCA8386 switch and 10G SFP port.
> 
> Signed-off-by: Lei Wei <quic_leiwei@quicinc.com>
> Signed-off-by: Luo Jie <quic_luoj@quicinc.com>
> ---
>  arch/arm64/boot/dts/qcom/ipq5332-rdp441.dts | 51 +++++++++++++++++++++
>  1 file changed, 51 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/ipq5332-rdp441.dts b/arch/arm64/boot/dts/qcom/ipq5332-rdp441.dts
> index 846413817e9a..d51968e9d601 100644
> --- a/arch/arm64/boot/dts/qcom/ipq5332-rdp441.dts
> +++ b/arch/arm64/boot/dts/qcom/ipq5332-rdp441.dts
> @@ -12,6 +12,15 @@
>  / {
>  	model = "Qualcomm Technologies, Inc. IPQ5332 MI01.2";
>  	compatible = "qcom,ipq5332-ap-mi01.2", "qcom,ipq5332";
> +
> +	soc@0 {

Nope, DTS does not define soc nodes.

> +		sfp1: sfp-1 {

Why is this soc? Where is the MMIO address?

> +			compatible = "sff,sfp";
> +			i2c-bus = <&blsp1_i2c1>;
> +			los-gpios = <&tlmm 45 GPIO_ACTIVE_HIGH>;
> +			tx-disable-gpios = <&tlmm 24 GPIO_ACTIVE_HIGH>;
> +		};
> +	};
>  };
>  
>  &blsp1_i2c1 {
> @@ -63,3 +72,45 @@ data-pins {
>  		};
>  	};
>  };
> +
> +&qcom_ppe {
> +	qcom,port_phyinfo {

Eh... We talk now about basics: please don't post downstream code, but
first clean it up from all the junk. All the basic issues which you have
in downstream and which we do not accept upstream.

I do not believe that this code passed internal review.

NAK.

Best regards,
Krzysztof

Re: [PATCH 5/6] arm64: dts: qcom: ipq5332: Add RDP441 board device tree

[Lei Wei]

On 1/10/2024 7:57 PM, Krzysztof Kozlowski wrote:
> On 10/01/2024 12:20, Luo Jie wrote:
>> From: Lei Wei <quic_leiwei@quicinc.com>
>>
>> RDP441 board has onboard QCA8386 switch and 10G SFP port.
>>
>> Signed-off-by: Lei Wei <quic_leiwei@quicinc.com>
>> Signed-off-by: Luo Jie <quic_luoj@quicinc.com>
>> ---
>>   arch/arm64/boot/dts/qcom/ipq5332-rdp441.dts | 51 +++++++++++++++++++++
>>   1 file changed, 51 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/ipq5332-rdp441.dts b/arch/arm64/boot/dts/qcom/ipq5332-rdp441.dts
>> index 846413817e9a..d51968e9d601 100644
>> --- a/arch/arm64/boot/dts/qcom/ipq5332-rdp441.dts
>> +++ b/arch/arm64/boot/dts/qcom/ipq5332-rdp441.dts
>> @@ -12,6 +12,15 @@
>>   / {
>>   	model = "Qualcomm Technologies, Inc. IPQ5332 MI01.2";
>>   	compatible = "qcom,ipq5332-ap-mi01.2", "qcom,ipq5332";
>> +
>> +	soc@0 {
> 
> Nope, DTS does not define soc nodes.
>
OK, I will remove the soc node in the DTS file. SFP node should not be 
inside the soc node.

>> +		sfp1: sfp-1 {
> 
> Why is this soc? Where is the MMIO address?

Sure, SoC node should not be required. I will remove the soc node.

> 
>> +			compatible = "sff,sfp";
>> +			i2c-bus = <&blsp1_i2c1>;
>> +			los-gpios = <&tlmm 45 GPIO_ACTIVE_HIGH>;
>> +			tx-disable-gpios = <&tlmm 24 GPIO_ACTIVE_HIGH>;
>> +		};
>> +	};
>>   };
>>   
>>   &blsp1_i2c1 {
>> @@ -63,3 +72,45 @@ data-pins {
>>   		};
>>   	};
>>   };
>> +
>> +&qcom_ppe {
>> +	qcom,port_phyinfo {
> 
> Eh... We talk now about basics: please don't post downstream code, but
> first clean it up from all the junk. All the basic issues which you have
> in downstream and which we do not accept upstream.
> 
> I do not believe that this code passed internal review.
> 
> NAK.

Sure, got it. I will follow the upstream principles to use generic node 
name.

> 
> Best regards,
> Krzysztof
> 

[PATCH 6/6] arm64: dts: qcom: ipq9574: Add RDP433 board device tree

[Luo Jie]

From: Lei Wei <quic_leiwei@quicinc.com>

RDP433 board has four QCA8075 PHYs and two Aquantia 10G PHY onboard.

Signed-off-by: Lei Wei <quic_leiwei@quicinc.com>
Signed-off-by: Luo Jie <quic_luoj@quicinc.com>
---
 arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts | 66 +++++++++++++++++++++
 1 file changed, 66 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts b/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts
index 1bb8d96c9a82..298c0853b4d2 100644
--- a/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts
+++ b/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts
@@ -60,3 +60,69 @@ rclk-pins {
 		};
 	};
 };
+
+&qcom_ppe {
+	qcom,port_phyinfo {
+		ppe_port0: port@0 {
+			port_id = <1>;
+			phy-mode = "qsgmii";
+			phy-handle = <&phy0>;
+		};
+		ppe_port1: port@1 {
+			port_id = <2>;
+			phy-mode = "qsgmii";
+			phy-handle = <&phy1>;
+		};
+		ppe_port2: port@2 {
+			port_id = <3>;
+			phy-mode = "qsgmii";
+			phy-handle = <&phy2>;
+		};
+		ppe_port3: port@3 {
+			port_id = <4>;
+			phy-mode = "qsgmii";
+			phy-handle = <&phy3>;
+		};
+		ppe_port4: port@4 {
+			port_id = <5>;
+			phy-mode = "usxgmii";
+			phy-handle = <&phy4>;
+		};
+		ppe_port5: port@5 {
+			port_id = <6>;
+			phy-mode = "usxgmii";
+			phy-handle = <&phy5>;
+		};
+	};
+};
+
+&mdio {
+	reset-gpios = <&tlmm 60 GPIO_ACTIVE_LOW>;
+	status = "okay";
+
+	phy0: ethernet-phy@0 {
+		      reg = <16>;
+	      };
+
+	phy1: ethernet-phy@1 {
+		      reg = <17>;
+	      };
+
+	phy2: ethernet-phy@2 {
+		      reg = <18>;
+	      };
+
+	phy3: ethernet-phy@3 {
+		      reg = <19>;
+	      };
+
+	phy4: ethernet-phy@4 {
+		      compatible ="ethernet-phy-ieee802.3-c45";
+		      reg = <8>;
+	      };
+
+	phy5: ethernet-phy@5 {
+		      compatible ="ethernet-phy-ieee802.3-c45";
+		      reg = <0>;
+	      };
+};
-- 
2.42.0

Re: [PATCH 6/6] arm64: dts: qcom: ipq9574: Add RDP433 board device tree

[Dmitry Baryshkov]

On 10 January 2024 13:20:59 EET, Luo Jie <quic_luoj@quicinc.com> wrote:
>From: Lei Wei <quic_leiwei@quicinc.com>
>
>RDP433 board has four QCA8075 PHYs and two Aquantia 10G PHY onboard.
>
>Signed-off-by: Lei Wei <quic_leiwei@quicinc.com>
>Signed-off-by: Luo Jie <quic_luoj@quicinc.com>
>---
> arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts | 66 +++++++++++++++++++++
> 1 file changed, 66 insertions(+)
>
>diff --git a/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts b/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts
>index 1bb8d96c9a82..298c0853b4d2 100644
>--- a/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts
>+++ b/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts
>@@ -60,3 +60,69 @@ rclk-pins {
> 		};
> 	};
> };
>+
>+&qcom_ppe {
>+	qcom,port_phyinfo {
>+		ppe_port0: port@0 {
>+			port_id = <1>;
>+			phy-mode = "qsgmii";
>+			phy-handle = <&phy0>;
>+		};
>+		ppe_port1: port@1 {
>+			port_id = <2>;
>+			phy-mode = "qsgmii";
>+			phy-handle = <&phy1>;
>+		};
>+		ppe_port2: port@2 {
>+			port_id = <3>;
>+			phy-mode = "qsgmii";
>+			phy-handle = <&phy2>;
>+		};
>+		ppe_port3: port@3 {
>+			port_id = <4>;
>+			phy-mode = "qsgmii";
>+			phy-handle = <&phy3>;
>+		};
>+		ppe_port4: port@4 {
>+			port_id = <5>;
>+			phy-mode = "usxgmii";
>+			phy-handle = <&phy4>;
>+		};
>+		ppe_port5: port@5 {
>+			port_id = <6>;
>+			phy-mode = "usxgmii";
>+			phy-handle = <&phy5>;
>+		};
>+	};
>+};
>+
>+&mdio {
>+	reset-gpios = <&tlmm 60 GPIO_ACTIVE_LOW>;
>+	status = "okay";
>+
>+	phy0: ethernet-phy@0 {
>+		      reg = <16>;
>+	      };

This part looks extremely wrong to me. If the reg is 16, then it should be @16 as well. You should have got a warning here.

>+
>+	phy1: ethernet-phy@1 {
>+		      reg = <17>;
>+	      };
>+
>+	phy2: ethernet-phy@2 {
>+		      reg = <18>;
>+	      };
>+
>+	phy3: ethernet-phy@3 {
>+		      reg = <19>;
>+	      };
>+
>+	phy4: ethernet-phy@4 {
>+		      compatible ="ethernet-phy-ieee802.3-c45";
>+		      reg = <8>;
>+	      };
>+
>+	phy5: ethernet-phy@5 {
>+		      compatible ="ethernet-phy-ieee802.3-c45";
>+		      reg = <0>;
>+	      };
>+};

Re: [PATCH 6/6] arm64: dts: qcom: ipq9574: Add RDP433 board device tree

[Lei Wei]

On 1/12/2024 11:05 PM, Dmitry Baryshkov wrote:
> On 10 January 2024 13:20:59 EET, Luo Jie <quic_luoj@quicinc.com> wrote:
>> From: Lei Wei <quic_leiwei@quicinc.com>
>>
>> RDP433 board has four QCA8075 PHYs and two Aquantia 10G PHY onboard.
>>
>> Signed-off-by: Lei Wei <quic_leiwei@quicinc.com>
>> Signed-off-by: Luo Jie <quic_luoj@quicinc.com>
>> ---
>> arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts | 66 +++++++++++++++++++++
>> 1 file changed, 66 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts b/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts
>> index 1bb8d96c9a82..298c0853b4d2 100644
>> --- a/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts
>> +++ b/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts
>> @@ -60,3 +60,69 @@ rclk-pins {
>> 		};
>> 	};
>> };
>> +
>> +&qcom_ppe {
>> +	qcom,port_phyinfo {
>> +		ppe_port0: port@0 {
>> +			port_id = <1>;
>> +			phy-mode = "qsgmii";
>> +			phy-handle = <&phy0>;
>> +		};
>> +		ppe_port1: port@1 {
>> +			port_id = <2>;
>> +			phy-mode = "qsgmii";
>> +			phy-handle = <&phy1>;
>> +		};
>> +		ppe_port2: port@2 {
>> +			port_id = <3>;
>> +			phy-mode = "qsgmii";
>> +			phy-handle = <&phy2>;
>> +		};
>> +		ppe_port3: port@3 {
>> +			port_id = <4>;
>> +			phy-mode = "qsgmii";
>> +			phy-handle = <&phy3>;
>> +		};
>> +		ppe_port4: port@4 {
>> +			port_id = <5>;
>> +			phy-mode = "usxgmii";
>> +			phy-handle = <&phy4>;
>> +		};
>> +		ppe_port5: port@5 {
>> +			port_id = <6>;
>> +			phy-mode = "usxgmii";
>> +			phy-handle = <&phy5>;
>> +		};
>> +	};
>> +};
>> +
>> +&mdio {
>> +	reset-gpios = <&tlmm 60 GPIO_ACTIVE_LOW>;
>> +	status = "okay";
>> +
>> +	phy0: ethernet-phy@0 {
>> +		      reg = <16>;
>> +	      };
> 
> This part looks extremely wrong to me. If the reg is 16, then it should be @16 as well. You should have got a warning here.
> 
Sure, I will fix and update it. Thanks.

>> +
>> +	phy1: ethernet-phy@1 {
>> +		      reg = <17>;
>> +	      };
>> +
>> +	phy2: ethernet-phy@2 {
>> +		      reg = <18>;
>> +	      };
>> +
>> +	phy3: ethernet-phy@3 {
>> +		      reg = <19>;
>> +	      };
>> +
>> +	phy4: ethernet-phy@4 {
>> +		      compatible ="ethernet-phy-ieee802.3-c45";
>> +		      reg = <8>;
>> +	      };
>> +
>> +	phy5: ethernet-phy@5 {
>> +		      compatible ="ethernet-phy-ieee802.3-c45";
>> +		      reg = <0>;
>> +	      };
>> +};
> 

Re: [PATCH 0/6] Add PPE device tree node for Qualcomm IPQ SoC

[Konrad Dybcio]

On 1/10/24 12:20, Luo Jie wrote:
> The PPE(packet process engine) hardware block is supported by Qualcomm
> IPQ platforms, such as IPQ9574 and IPQ5332. The PPE includes the various
> packet processing modules such as the routing and bridging flow engines,
> L2 switch capability, VLAN and tunnels. Also included are integrated
> ethernet MAC and PCS(uniphy), which is used to connect with the external
> PHY devices by PCS.
> 
> This patch series enables support for the following DTSI functionality
> for Qualcomm IPQ9574 and IPQ5332 chipsets.
> 
> 1. Add PPE (Packet Processing Engine) HW support
> 
> 2. Add IPQ9574 RDP433 board support, where the PPE is connected
>     with qca8075 PHY and AQ PHY.
> 
> 3. Add IPQ5332 RDP441 board support, where the PPE is connected
>     with qca8386 and SFP
> 
> PPE DTS depends on the NSSCC clock driver below, which provides the
> clocks for the PPE driver.
> https://lore.kernel.org/linux-arm-msm/20230825091234.32713-1-quic_devipriy@quicinc.com/
> https://lore.kernel.org/linux-arm-msm/20231211-ipq5332-nsscc-v3-0-ad13bef9b137@quicinc.com/

None of these describe (or even use) the compatible in the first
patch of this series ("qcom,ipq9574-ppe"). I didn't check the
subsequent ones, as I assume it's the same situtation, so this
is a NAK.

> Lei Wei (2):
>    arm64: dts: qcom: ipq5332: Add RDP441 board device tree
>    arm64: dts: qcom: ipq9574: Add RDP433 board device tree

These two look unrelated?

> 
> Luo Jie (4):
>    arm64: dts: qcom: ipq9574: Add PPE device tree node
>    arm64: dts: qcom: ipq5332: Add PPE device tree node
>    arm64: dts: qcom: ipq5332: Add MDIO device tree
>    arm64: dts: qcom: ipq9574: Add MDIO device tree

Konrad

Re: [PATCH 0/6] Add PPE device tree node for Qualcomm IPQ SoC

[Jie Luo]

On 1/10/2024 7:32 PM, Konrad Dybcio wrote:
> 
> 
> On 1/10/24 12:20, Luo Jie wrote:
>> The PPE(packet process engine) hardware block is supported by Qualcomm
>> IPQ platforms, such as IPQ9574 and IPQ5332. The PPE includes the various
>> packet processing modules such as the routing and bridging flow engines,
>> L2 switch capability, VLAN and tunnels. Also included are integrated
>> ethernet MAC and PCS(uniphy), which is used to connect with the external
>> PHY devices by PCS.
>>
>> This patch series enables support for the following DTSI functionality
>> for Qualcomm IPQ9574 and IPQ5332 chipsets.
>>
>> 1. Add PPE (Packet Processing Engine) HW support
>>
>> 2. Add IPQ9574 RDP433 board support, where the PPE is connected
>>     with qca8075 PHY and AQ PHY.
>>
>> 3. Add IPQ5332 RDP441 board support, where the PPE is connected
>>     with qca8386 and SFP
>>
>> PPE DTS depends on the NSSCC clock driver below, which provides the
>> clocks for the PPE driver.
>> https://lore.kernel.org/linux-arm-msm/20230825091234.32713-1-quic_devipriy@quicinc.com/
>> https://lore.kernel.org/linux-arm-msm/20231211-ipq5332-nsscc-v3-0-ad13bef9b137@quicinc.com/
> 
> None of these describe (or even use) the compatible in the first
> patch of this series ("qcom,ipq9574-ppe"). I didn't check the
> subsequent ones, as I assume it's the same situtation, so this
> is a NAK.

The DT binding file was included in the PPE driver series, which
documents the compatible string.

https://lore.kernel.org/netdev/20240110142428.52026d9e@kernel.org/

I will hold off this DTSI patch series for now as per discussion in the
series. When the series is resumed later, I will mention the link of PPE 
driver patch series in the cover letter, when updating this DTS patch 
series. Sorry for this confusion caused.



> 
>> Lei Wei (2):
>>    arm64: dts: qcom: ipq5332: Add RDP441 board device tree
>>    arm64: dts: qcom: ipq9574: Add RDP433 board device tree
> 
> These two look unrelated?

These two patches are for adding the PPE port related configuration 
nodes (such as port speed, interface mode and MDIO address) which are 
board specific. RDP441 and RDP433 are two different board types. Perhaps
the title of the patches are not clear enough. I will update the title
to make it clear when the patch series resumes.

> 
>>
>> Luo Jie (4):
>>    arm64: dts: qcom: ipq9574: Add PPE device tree node
>>    arm64: dts: qcom: ipq5332: Add PPE device tree node
>>    arm64: dts: qcom: ipq5332: Add MDIO device tree
>>    arm64: dts: qcom: ipq9574: Add MDIO device tree
> 
> Konrad

Re: [PATCH 0/6] Add PPE device tree node for Qualcomm IPQ SoC

[Krzysztof Kozlowski]

On 10/01/2024 12:20, Luo Jie wrote:
> The PPE(packet process engine) hardware block is supported by Qualcomm
> IPQ platforms, such as IPQ9574 and IPQ5332. The PPE includes the various
> packet processing modules such as the routing and bridging flow engines,
> L2 switch capability, VLAN and tunnels. Also included are integrated
> ethernet MAC and PCS(uniphy), which is used to connect with the external
> PHY devices by PCS.
> 
> This patch series enables support for the following DTSI functionality
> for Qualcomm IPQ9574 and IPQ5332 chipsets. 
> 
> 1. Add PPE (Packet Processing Engine) HW support
> 
> 2. Add IPQ9574 RDP433 board support, where the PPE is connected
>    with qca8075 PHY and AQ PHY.
> 
> 3. Add IPQ5332 RDP441 board support, where the PPE is connected
>    with qca8386 and SFP
> 
> PPE DTS depends on the NSSCC clock driver below, which provides the
> clocks for the PPE driver.

DTS cannot depend on clock drivers. Maybe you meant that it depends on
NSSCC clock controller DTS changes, which would be fine. However
depending on drivers is neither necessary nor allowed.

Best regards,
Krzysztof

Re: [PATCH 0/6] Add PPE device tree node for Qualcomm IPQ SoC

[Jie Luo]

On 1/10/2024 8:13 PM, Krzysztof Kozlowski wrote:
> On 10/01/2024 12:20, Luo Jie wrote:
>> The PPE(packet process engine) hardware block is supported by Qualcomm
>> IPQ platforms, such as IPQ9574 and IPQ5332. The PPE includes the various
>> packet processing modules such as the routing and bridging flow engines,
>> L2 switch capability, VLAN and tunnels. Also included are integrated
>> ethernet MAC and PCS(uniphy), which is used to connect with the external
>> PHY devices by PCS.
>>
>> This patch series enables support for the following DTSI functionality
>> for Qualcomm IPQ9574 and IPQ5332 chipsets.
>>
>> 1. Add PPE (Packet Processing Engine) HW support
>>
>> 2. Add IPQ9574 RDP433 board support, where the PPE is connected
>>     with qca8075 PHY and AQ PHY.
>>
>> 3. Add IPQ5332 RDP441 board support, where the PPE is connected
>>     with qca8386 and SFP
>>
>> PPE DTS depends on the NSSCC clock driver below, which provides the
>> clocks for the PPE driver.
> 
> DTS cannot depend on clock drivers. Maybe you meant that it depends on
> NSSCC clock controller DTS changes, which would be fine. However
> depending on drivers is neither necessary nor allowed.
> 
> Best regards,
> Krzysztof
> 

Yes, this DTSI series depends on the NSSCC clock controller DTS patches 
which are referred to in the cover letter. I will rectify the cover 
letter text when the series resumes later.