[PATCH v8 00/14] add qca8084 ethernet phy driver

[PATCH v8 00/14] add qca8084 ethernet phy driver

[Luo Jie]

QCA8084 is four-port PHY with maximum link capability 2.5G,
which supports the interface mode qusgmii and sgmii mode,
there are two PCSs available to connected with ethernet port.

QCA8084 can work in switch mode or PHY mode.
For switch mode, both PCS0 and PCS1 work on sgmii mode.
For PHY mode, PCS1 works on qusgmii mode.
The fourth PHY connected with PCS0 works on sgmii mode.

Besides this PHY driver patches, the PCS driver is also needed
to bring up the qca8084 device, which mainly configurs PCS
and clocks.

The qca8084 PHY driver depends on the following clock controller
patchset, the initial clocks and resets are provided by the clock
controller driver below.
https://lore.kernel.org/lkml/20231104034858.9159-2-quic_luoj@quicinc.com/T/

Changes in v3:
	* pick the two patches to introduce the interface mode
	  10g-qxgmii from Vladimir Oltean(olteanv@gmail.com).
	* add the function phydev_id_is_qca808x to identify the
	  PHY qca8081 and qca8084.
	* update the interface mode name PHY_INTERFACE_MODE_QUSGMII
	  to PHY_INTERFACE_MODE_10G_QXGMII.

Changes in v4:
	* remove the following patch:
	  <net: phylink: move phylink_pcs_neg_mode() to phylink.c>.
	* split out 10g_qxgmii change of ethernet-controller.yaml.

Changes in v5:
	* update the author of the patch below.
	  <introduce core support for phy-mode = "10g-qxgmii">.

Changes in v6:
	* drop the "inline" keyword.
	* apply the patches with "--max-line-length=80".

Changes in v7:
	* add possible interfaces of phydev
	* customize phy address
	* add initialized clock & reset config
	* add the work mode config
	* update qca,ar803x.yaml for the new added properties

Changes in v7:
	* updated the patcheset based on the latest code

Luo Jie (12):
  net: phy: at803x: add QCA8084 ethernet phy support
  net: phy: at803x: add the function phydev_id_is_qca808x
  net: phy: at803x: Add qca8084_config_init function
  net: phy: at803x: add qca8084_link_change_notify
  net: phy: at803x: add the possible_interfaces
  net: phy: at803x: add qca8084 switch registe access
  net: phy: at803x: set MDIO address of qca8084 PHY
  net: phy: at803x: parse qca8084 clocks and resets
  net: phy: at803x: add qca808x initial config sequence
  net: phy: at803x: configure qca8084 common clocks
  net: phy: at803x: configure qca8084 work mode
  dt-bindings: net: ar803x: add qca8084 PHY properties

Vladimir Oltean (2):
  net: phy: introduce core support for phy-mode = "10g-qxgmii"
  dt-bindings: net: ethernet-controller: add 10g-qxgmii mode

 .../bindings/net/ethernet-controller.yaml     |   1 +
 .../devicetree/bindings/net/qca,ar803x.yaml   | 158 ++++-
 Documentation/networking/phy.rst              |   6 +
 drivers/net/phy/at803x.c                      | 577 +++++++++++++++++-
 drivers/net/phy/phy-core.c                    |   1 +
 drivers/net/phy/phylink.c                     |  11 +-
 include/linux/phy.h                           |   4 +
 include/linux/phylink.h                       |   2 +
 8 files changed, 752 insertions(+), 8 deletions(-)


base-commit: 17cb8a20bde66a520a2ca7aad1063e1ce7382240
-- 
2.42.0

[PATCH v8 01/14] net: phy: introduce core support for phy-mode = "10g-qxgmii"

[Luo Jie]

From: Vladimir Oltean <vladimir.oltean@nxp.com>

10G-QXGMII is a MAC-to-PHY interface defined by the USXGMII multiport
specification. It uses the same signaling as USXGMII, but it multiplexes
4 ports over the link, resulting in a maximum speed of 2.5G per port.

Some in-tree SoCs like the NXP LS1028A use "usxgmii" when they mean
either the single-port USXGMII or the quad-port 10G-QXGMII variant, and
they could get away just fine with that thus far. But there is a need to
distinguish between the 2 as far as SerDes drivers are concerned.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Signed-off-by: Luo Jie <quic_luoj@quicinc.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Reviewed-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 Documentation/networking/phy.rst |  6 ++++++
 drivers/net/phy/phy-core.c       |  1 +
 drivers/net/phy/phylink.c        | 11 +++++++++--
 include/linux/phy.h              |  4 ++++
 include/linux/phylink.h          |  2 ++
 5 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/Documentation/networking/phy.rst b/Documentation/networking/phy.rst
index 1283240d7620..f64641417c54 100644
--- a/Documentation/networking/phy.rst
+++ b/Documentation/networking/phy.rst
@@ -327,6 +327,12 @@ Some of the interface modes are described below:
     This is the Penta SGMII mode, it is similar to QSGMII but it combines 5
     SGMII lines into a single link compared to 4 on QSGMII.
 
+``PHY_INTERFACE_MODE_10G_QXGMII``
+    Represents the 10G-QXGMII PHY-MAC interface as defined by the Cisco USXGMII
+    Multiport Copper Interface document. It supports 4 ports over a 10.3125 GHz
+    SerDes lane, each port having speeds of 2.5G / 1G / 100M / 10M achieved
+    through symbol replication. The PCS expects the standard USXGMII code word.
+
 Pause frames / flow control
 ===========================
 
diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c
index 966c93cbe616..1cd58723d6d0 100644
--- a/drivers/net/phy/phy-core.c
+++ b/drivers/net/phy/phy-core.c
@@ -141,6 +141,7 @@ int phy_interface_num_ports(phy_interface_t interface)
 		return 1;
 	case PHY_INTERFACE_MODE_QSGMII:
 	case PHY_INTERFACE_MODE_QUSGMII:
+	case PHY_INTERFACE_MODE_10G_QXGMII:
 		return 4;
 	case PHY_INTERFACE_MODE_PSGMII:
 		return 5;
diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 48d3bd3e9fc7..938faac14930 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -231,6 +231,7 @@ static int phylink_interface_max_speed(phy_interface_t interface)
 		return SPEED_1000;
 
 	case PHY_INTERFACE_MODE_2500BASEX:
+	case PHY_INTERFACE_MODE_10G_QXGMII:
 		return SPEED_2500;
 
 	case PHY_INTERFACE_MODE_5GBASER:
@@ -500,7 +501,11 @@ static unsigned long phylink_get_capabilities(phy_interface_t interface,
 
 	switch (interface) {
 	case PHY_INTERFACE_MODE_USXGMII:
-		caps |= MAC_10000FD | MAC_5000FD | MAC_2500FD;
+		caps |= MAC_10000FD | MAC_5000FD;
+		fallthrough;
+
+	case PHY_INTERFACE_MODE_10G_QXGMII:
+		caps |= MAC_2500FD;
 		fallthrough;
 
 	case PHY_INTERFACE_MODE_RGMII_TXID:
@@ -941,6 +946,7 @@ static int phylink_parse_mode(struct phylink *pl,
 			phylink_set(pl->supported, 25000baseSR_Full);
 			fallthrough;
 		case PHY_INTERFACE_MODE_USXGMII:
+		case PHY_INTERFACE_MODE_10G_QXGMII:
 		case PHY_INTERFACE_MODE_10GKR:
 		case PHY_INTERFACE_MODE_10GBASER:
 			phylink_set(pl->supported, 10baseT_Half);
@@ -1837,7 +1843,8 @@ static int phylink_validate_phy(struct phylink *pl, struct phy_device *phy,
 	if (phy->is_c45 && state->rate_matching == RATE_MATCH_NONE &&
 	    state->interface != PHY_INTERFACE_MODE_RXAUI &&
 	    state->interface != PHY_INTERFACE_MODE_XAUI &&
-	    state->interface != PHY_INTERFACE_MODE_USXGMII)
+	    state->interface != PHY_INTERFACE_MODE_USXGMII &&
+	    state->interface != PHY_INTERFACE_MODE_10G_QXGMII)
 		state->interface = PHY_INTERFACE_MODE_NA;
 
 	return phylink_validate(pl, supported, state);
diff --git a/include/linux/phy.h b/include/linux/phy.h
index dbb5e13e3e1b..87859a32f6ef 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -125,6 +125,7 @@ extern const int phy_10gbit_features_array[1];
  * @PHY_INTERFACE_MODE_10GKR: 10GBASE-KR - with Clause 73 AN
  * @PHY_INTERFACE_MODE_QUSGMII: Quad Universal SGMII
  * @PHY_INTERFACE_MODE_1000BASEKX: 1000Base-KX - with Clause 73 AN
+ * @PHY_INTERFACE_MODE_10G_QXGMII: 10G-QXGMII - 4 ports over 10G USXGMII
  * @PHY_INTERFACE_MODE_MAX: Book keeping
  *
  * Describes the interface between the MAC and PHY.
@@ -165,6 +166,7 @@ typedef enum {
 	PHY_INTERFACE_MODE_10GKR,
 	PHY_INTERFACE_MODE_QUSGMII,
 	PHY_INTERFACE_MODE_1000BASEKX,
+	PHY_INTERFACE_MODE_10G_QXGMII,
 	PHY_INTERFACE_MODE_MAX,
 } phy_interface_t;
 
@@ -286,6 +288,8 @@ static inline const char *phy_modes(phy_interface_t interface)
 		return "100base-x";
 	case PHY_INTERFACE_MODE_QUSGMII:
 		return "qusgmii";
+	case PHY_INTERFACE_MODE_10G_QXGMII:
+		return "10g-qxgmii";
 	default:
 		return "unknown";
 	}
diff --git a/include/linux/phylink.h b/include/linux/phylink.h
index 875439ab45de..92bd2726cc8a 100644
--- a/include/linux/phylink.h
+++ b/include/linux/phylink.h
@@ -128,6 +128,7 @@ static inline unsigned int phylink_pcs_neg_mode(unsigned int mode,
 	case PHY_INTERFACE_MODE_QSGMII:
 	case PHY_INTERFACE_MODE_QUSGMII:
 	case PHY_INTERFACE_MODE_USXGMII:
+	case PHY_INTERFACE_MODE_10G_QXGMII:
 		/* These protocols are designed for use with a PHY which
 		 * communicates its negotiation result back to the MAC via
 		 * inband communication. Note: there exist PHYs that run
@@ -680,6 +681,7 @@ static inline int phylink_get_link_timer_ns(phy_interface_t interface)
 	case PHY_INTERFACE_MODE_SGMII:
 	case PHY_INTERFACE_MODE_QSGMII:
 	case PHY_INTERFACE_MODE_USXGMII:
+	case PHY_INTERFACE_MODE_10G_QXGMII:
 		return 1600000;
 
 	case PHY_INTERFACE_MODE_1000BASEX:
-- 
2.42.0

[PATCH v8 02/14] dt-bindings: net: ethernet-controller: add 10g-qxgmii mode

[Luo Jie]

From: Vladimir Oltean <vladimir.oltean@nxp.com>

Add the new interface mode 10g-qxgmii, which is similar to
usxgmii but extend to 4 channels to support maximum of 4
ports with the link speed 10M/100M/1G/2.5G.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Signed-off-by: Luo Jie <quic_luoj@quicinc.com>
Acked-by: Conor Dooley <conor.dooley@microchip.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
 Documentation/devicetree/bindings/net/ethernet-controller.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/net/ethernet-controller.yaml b/Documentation/devicetree/bindings/net/ethernet-controller.yaml
index d14d123ad7a0..0ef6103c5fd8 100644
--- a/Documentation/devicetree/bindings/net/ethernet-controller.yaml
+++ b/Documentation/devicetree/bindings/net/ethernet-controller.yaml
@@ -104,6 +104,7 @@ properties:
       - usxgmii
       - 10gbase-r
       - 25gbase-r
+      - 10g-qxgmii
 
   phy-mode:
     $ref: "#/properties/phy-connection-type"
-- 
2.42.0

[PATCH v8 03/14] net: phy: at803x: add QCA8084 ethernet phy support

[Luo Jie]

Add qca8084 PHY support, which is four-port PHY with maximum
link capability 2.5G, the features of each port is almost same
as QCA8081 and slave seed config is not needed.

Three kind of interface modes supported by qca8084.
PHY_INTERFACE_MODE_10G_QXGMII, PHY_INTERFACE_MODE_2500BASEX and
PHY_INTERFACE_MODE_SGMII.

The PCS(serdes) and clock are also needed to be configured to
bringup qca8084 PHY, which will be added in the pcs driver.

The additional CDT configurations used for qca8084.

Signed-off-by: Luo Jie <quic_luoj@quicinc.com>
---
 drivers/net/phy/at803x.c | 49 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index b9d3a26cf6dc..eacde4fa5d6c 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -176,6 +176,7 @@
 #define AT8030_PHY_ID_MASK			0xffffffef
 
 #define QCA8081_PHY_ID				0x004dd101
+#define QCA8084_PHY_ID				0x004dd180
 
 #define QCA8327_A_PHY_ID			0x004dd033
 #define QCA8327_B_PHY_ID			0x004dd034
@@ -1835,6 +1836,9 @@ static bool qca808x_is_prefer_master(struct phy_device *phydev)
 
 static bool qca808x_has_fast_retrain_or_slave_seed(struct phy_device *phydev)
 {
+	if (phydev_id_compare(phydev, QCA8084_PHY_ID))
+		return false;
+
 	return linkmode_test_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT, phydev->supported);
 }
 
@@ -1899,6 +1903,23 @@ static int qca808x_read_status(struct phy_device *phydev)
 		return ret;
 
 	if (phydev->link) {
+		/* There are two PCSs available for QCA8084, which support the
+		 * following interface modes.
+		 *
+		 * 1. PHY_INTERFACE_MODE_10G_QXGMII utilizes PCS1 for all
+		 * available 4 ports, which is for all link speeds.
+		 *
+		 * 2. PHY_INTERFACE_MODE_2500BASEX utilizes PCS0 for the
+		 * fourth port, which is only for the link speed 2500M same
+		 * as QCA8081.
+		 *
+		 * 3. PHY_INTERFACE_MODE_SGMII utilizes PCS0 for the fourth
+		 * port, which is for the link speed 10M, 100M and 1000M same
+		 * as QCA8081.
+		 */
+		if (phydev->interface == PHY_INTERFACE_MODE_10G_QXGMII)
+			return 0;
+
 		if (phydev->speed == SPEED_2500)
 			phydev->interface = PHY_INTERFACE_MODE_2500BASEX;
 		else
@@ -2033,6 +2054,14 @@ static int qca808x_cable_test_start(struct phy_device *phydev)
 	phy_write_mmd(phydev, MDIO_MMD_PCS, 0x807a, 0xc060);
 	phy_write_mmd(phydev, MDIO_MMD_PCS, 0x807e, 0xb060);
 
+	if (phydev_id_compare(phydev, QCA8084_PHY_ID)) {
+		/* Adjust the positive and negative pulse thereshold of CDT */
+		phy_write_mmd(phydev, MDIO_MMD_PCS, 0x8075, 0xa060);
+
+		/* Disable the near echo bypass */
+		phy_modify_mmd(phydev, MDIO_MMD_PCS, 0x807f, BIT(15), 0);
+	}
+
 	return 0;
 }
 
@@ -2304,6 +2333,25 @@ static struct phy_driver at803x_driver[] = {
 	.cable_test_start	= qca808x_cable_test_start,
 	.cable_test_get_status	= qca808x_cable_test_get_status,
 	.link_change_notify	= qca808x_link_change_notify,
+}, {
+	/* Qualcomm QCA8084 */
+	PHY_ID_MATCH_MODEL(QCA8084_PHY_ID),
+	.name			= "Qualcomm QCA8084",
+	.flags			= PHY_POLL_CABLE_TEST,
+	.config_intr		= at803x_config_intr,
+	.handle_interrupt	= at803x_handle_interrupt,
+	.get_tunable		= at803x_get_tunable,
+	.set_tunable		= at803x_set_tunable,
+	.set_wol		= at803x_set_wol,
+	.get_wol		= at803x_get_wol,
+	.get_features		= qca808x_get_features,
+	.config_aneg		= at803x_config_aneg,
+	.suspend		= genphy_suspend,
+	.resume			= genphy_resume,
+	.read_status		= qca808x_read_status,
+	.soft_reset		= qca808x_soft_reset,
+	.cable_test_start	= qca808x_cable_test_start,
+	.cable_test_get_status	= qca808x_cable_test_get_status,
 }, };
 
 module_phy_driver(at803x_driver);
@@ -2319,6 +2367,7 @@ static struct mdio_device_id __maybe_unused atheros_tbl[] = {
 	{ PHY_ID_MATCH_EXACT(QCA8327_B_PHY_ID) },
 	{ PHY_ID_MATCH_EXACT(QCA9561_PHY_ID) },
 	{ PHY_ID_MATCH_EXACT(QCA8081_PHY_ID) },
+	{ PHY_ID_MATCH_MODEL(QCA8084_PHY_ID) },
 	{ }
 };
 
-- 
2.42.0

[PATCH v8 04/14] net: phy: at803x: add the function phydev_id_is_qca808x

[Luo Jie]

The function phydev_id_is_qca808x is applicable to the
PHY qca8081 and qca8084.

Signed-off-by: Luo Jie <quic_luoj@quicinc.com>
---
 drivers/net/phy/at803x.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index eacde4fa5d6c..1030896dd546 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -922,6 +922,12 @@ static void at803x_link_change_notify(struct phy_device *phydev)
 	}
 }
 
+static bool phydev_id_is_qca808x(struct phy_device *phydev)
+{
+	return phydev_id_compare(phydev, QCA8081_PHY_ID) ||
+		phydev_id_compare(phydev, QCA8084_PHY_ID);
+}
+
 static int at803x_read_specific_status(struct phy_device *phydev)
 {
 	int ss;
@@ -941,8 +947,8 @@ static int at803x_read_specific_status(struct phy_device *phydev)
 		if (sfc < 0)
 			return sfc;
 
-		/* qca8081 takes the different bits for speed value from at803x */
-		if (phydev->drv->phy_id == QCA8081_PHY_ID)
+		/* qca808x takes the different bits for speed value from at803x */
+		if (phydev_id_is_qca808x(phydev))
 			speed = FIELD_GET(QCA808X_SS_SPEED_MASK, ss);
 		else
 			speed = FIELD_GET(AT803X_SS_SPEED_MASK, ss);
@@ -1073,7 +1079,7 @@ static int at803x_config_aneg(struct phy_device *phydev)
 	 */
 	ret = 0;
 
-	if (phydev->drv->phy_id == QCA8081_PHY_ID) {
+	if (phydev_id_is_qca808x(phydev)) {
 		int phy_ctrl = 0;
 
 		/* The reg MII_BMCR also needs to be configured for force mode, the
-- 
2.42.0

[PATCH v8 05/14] net: phy: at803x: Add qca8084_config_init function

[Luo Jie]

Configure MSE detect threshold and ADC clock edge invert.

Signed-off-by: Luo Jie <quic_luoj@quicinc.com>
---
 drivers/net/phy/at803x.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index 1030896dd546..252fae4329cc 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -280,6 +280,15 @@
 #define QCA8081_PHY_SERDES_MMD1_FIFO_CTRL	0x9072
 #define QCA8081_PHY_FIFO_RSTN			BIT(11)
 
+/* QCA8084 ADC clock edge */
+#define QCA8084_ADC_CLK_SEL			0x8b80
+#define QCA8084_ADC_CLK_SEL_ACLK		GENMASK(7, 4)
+#define QCA8084_ADC_CLK_SEL_ACLK_FALL		0xf
+#define QCA8084_ADC_CLK_SEL_ACLK_RISE		0x0
+
+#define QCA8084_MSE_THRESHOLD			0x800a
+#define QCA8084_MSE_THRESHOLD_2P5G_VAL		0x51c6
+
 MODULE_DESCRIPTION("Qualcomm Atheros AR803x and QCA808X PHY driver");
 MODULE_AUTHOR("Matus Ujhelyi");
 MODULE_LICENSE("GPL");
@@ -2163,6 +2172,26 @@ static void qca808x_link_change_notify(struct phy_device *phydev)
 			QCA8081_PHY_FIFO_RSTN, phydev->link ? QCA8081_PHY_FIFO_RSTN : 0);
 }
 
+static int qca8084_config_init(struct phy_device *phydev)
+{
+	int ret;
+
+	/* Invert ADC clock edge */
+	ret = at803x_debug_reg_mask(phydev, QCA8084_ADC_CLK_SEL,
+				    QCA8084_ADC_CLK_SEL_ACLK,
+				    FIELD_PREP(QCA8084_ADC_CLK_SEL_ACLK,
+					       QCA8084_ADC_CLK_SEL_ACLK_FALL));
+	if (ret < 0)
+		return ret;
+
+	/* Adjust MSE threshold value to avoid link issue with
+	 * some link partner.
+	 */
+	return phy_write_mmd(phydev, MDIO_MMD_PMAPMD,
+			     QCA8084_MSE_THRESHOLD,
+			     QCA8084_MSE_THRESHOLD_2P5G_VAL);
+}
+
 static struct phy_driver at803x_driver[] = {
 {
 	/* Qualcomm Atheros AR8035 */
@@ -2358,6 +2387,7 @@ static struct phy_driver at803x_driver[] = {
 	.soft_reset		= qca808x_soft_reset,
 	.cable_test_start	= qca808x_cable_test_start,
 	.cable_test_get_status	= qca808x_cable_test_get_status,
+	.config_init		= qca8084_config_init,
 }, };
 
 module_phy_driver(at803x_driver);
-- 
2.42.0

[PATCH v8 06/14] net: phy: at803x: add qca8084_link_change_notify

[Luo Jie]

When the link is changed, qca8084 needs to do the fifo reset and
adjust the IPG level for the qusgmii link speed 1000M.

Signed-off-by: Luo Jie <quic_luoj@quicinc.com>
---
 drivers/net/phy/at803x.c | 41 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index 252fae4329cc..bb382089ab77 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -289,6 +289,13 @@
 #define QCA8084_MSE_THRESHOLD			0x800a
 #define QCA8084_MSE_THRESHOLD_2P5G_VAL		0x51c6
 
+#define QCA8084_FIFO_CONTROL			0x19
+#define QCA8084_FIFO_MAC_2_PHY			BIT(1)
+#define QCA8084_FIFO_PHY_2_MAC			BIT(0)
+
+#define QCA8084_MMD7_IPG_OP			0x901d
+#define QCA8084_IPG_10_TO_11_EN			BIT(0)
+
 MODULE_DESCRIPTION("Qualcomm Atheros AR803x and QCA808X PHY driver");
 MODULE_AUTHOR("Matus Ujhelyi");
 MODULE_LICENSE("GPL");
@@ -2192,6 +2199,39 @@ static int qca8084_config_init(struct phy_device *phydev)
 			     QCA8084_MSE_THRESHOLD_2P5G_VAL);
 }
 
+static void qca8084_link_change_notify(struct phy_device *phydev)
+{
+	int ret;
+
+	ret = phy_modify(phydev, QCA8084_FIFO_CONTROL,
+			 QCA8084_FIFO_MAC_2_PHY | QCA8084_FIFO_PHY_2_MAC,
+			 0);
+	if (ret)
+		return;
+
+	/* If the PHY works on PHY_INTERFACE_MODE_10G_QXGMII mode, the fifo
+	 * needs to be kept as reset state in link down status.
+	 */
+	if (phydev->interface != PHY_INTERFACE_MODE_10G_QXGMII ||
+	    phydev->link) {
+		msleep(50);
+		ret = phy_modify(phydev, QCA8084_FIFO_CONTROL,
+				 QCA8084_FIFO_MAC_2_PHY |
+				 QCA8084_FIFO_PHY_2_MAC,
+				 QCA8084_FIFO_MAC_2_PHY |
+				 QCA8084_FIFO_PHY_2_MAC);
+		if (ret)
+			return;
+	}
+
+	/* Enable IPG 10 to 11 tuning on link speed 1000M of QUSGMII mode. */
+	if (phydev->interface == PHY_INTERFACE_MODE_10G_QXGMII)
+		phy_modify_mmd(phydev, MDIO_MMD_AN, QCA8084_MMD7_IPG_OP,
+			       QCA8084_IPG_10_TO_11_EN,
+			       phydev->speed == SPEED_1000 ?
+			       QCA8084_IPG_10_TO_11_EN : 0);
+}
+
 static struct phy_driver at803x_driver[] = {
 {
 	/* Qualcomm Atheros AR8035 */
@@ -2388,6 +2428,7 @@ static struct phy_driver at803x_driver[] = {
 	.cable_test_start	= qca808x_cable_test_start,
 	.cable_test_get_status	= qca808x_cable_test_get_status,
 	.config_init		= qca8084_config_init,
+	.link_change_notify	= qca8084_link_change_notify,
 }, };
 
 module_phy_driver(at803x_driver);
-- 
2.42.0

[PATCH v8 07/14] net: phy: at803x: add the possible_interfaces

[Luo Jie]

When qca808x works on the interface mode sgmii or
2500base-x, the interface mode can be switched according
to the PHY link speed.

Signed-off-by: Luo Jie <quic_luoj@quicinc.com>
---
 drivers/net/phy/at803x.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index bb382089ab77..cd1bdb61d122 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -2179,10 +2179,22 @@ static void qca808x_link_change_notify(struct phy_device *phydev)
 			QCA8081_PHY_FIFO_RSTN, phydev->link ? QCA8081_PHY_FIFO_RSTN : 0);
 }
 
+static void qca808x_fill_possible_interfaces(struct phy_device *phydev)
+{
+	unsigned long *possible = phydev->possible_interfaces;
+
+	if (phydev->interface != PHY_INTERFACE_MODE_10G_QXGMII) {
+		__set_bit(PHY_INTERFACE_MODE_2500BASEX, possible);
+		__set_bit(PHY_INTERFACE_MODE_SGMII, possible);
+	}
+}
+
 static int qca8084_config_init(struct phy_device *phydev)
 {
 	int ret;
 
+	qca808x_fill_possible_interfaces(phydev);
+
 	/* Invert ADC clock edge */
 	ret = at803x_debug_reg_mask(phydev, QCA8084_ADC_CLK_SEL,
 				    QCA8084_ADC_CLK_SEL_ACLK,
-- 
2.42.0

[PATCH v8 08/14] net: phy: at803x: add qca8084 switch registe access

[Luo Jie]

For qca8084 chip, there are GCC, TLMM and security control
modules besides the PHY, these moudles are accessed with 32
bits value, which has the special MDIO sequences to read or
write this 32bit register.

There are initial configurations configured to make qca8084 PHY
probeable, and the MDIO address of qca8084 can be programmed for
the PHY device and PCS device.

Signed-off-by: Luo Jie <quic_luoj@quicinc.com>
---
 drivers/net/phy/at803x.c | 85 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 85 insertions(+)

diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index cd1bdb61d122..4982bde5a8a5 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -296,6 +296,13 @@
 #define QCA8084_MMD7_IPG_OP			0x901d
 #define QCA8084_IPG_10_TO_11_EN			BIT(0)
 
+/* QCA8084 includes secure control module, which supports customizing the
+ * MDIO address of PHY device and PCS device, the register of secure control
+ * is accessed by MDIO bus with the special MDIO sequences.
+ */
+#define QCA8084_HIGH_ADDR_PREFIX		0x18
+#define QCA8084_LOW_ADDR_PREFIX			0x10
+
 MODULE_DESCRIPTION("Qualcomm Atheros AR803x and QCA808X PHY driver");
 MODULE_AUTHOR("Matus Ujhelyi");
 MODULE_LICENSE("GPL");
@@ -408,6 +415,84 @@ static int at803x_read_page(struct phy_device *phydev)
 	return AT803X_PAGE_FIBER;
 }
 
+static void qca8084_split_addr(u32 regaddr, u16 *r1, u16 *r2,
+			       u16 *page, u16 *sw_addr)
+{
+	*r1 = regaddr & 0x1c;
+
+	regaddr >>= 5;
+	*r2 = regaddr & 0x7;
+
+	regaddr >>= 3;
+	*page = regaddr & 0xffff;
+
+	regaddr >>= 16;
+	*sw_addr = regaddr & 0xff;
+}
+
+static int __qca8084_set_page(struct mii_bus *bus, u16 sw_addr, u16 page)
+{
+	return __mdiobus_write(bus, QCA8084_HIGH_ADDR_PREFIX | (sw_addr >> 5),
+			       sw_addr & 0x1f, page);
+}
+
+static int __qca8084_mii_read(struct mii_bus *bus, u16 addr, u16 reg, u32 *val)
+{
+	int ret, data;
+
+	ret = __mdiobus_read(bus, addr, reg);
+	if (ret >= 0) {
+		data = ret;
+
+		ret = __mdiobus_read(bus, addr, reg | BIT(1));
+		if (ret >= 0)
+			*val =  data | ret << 16;
+	}
+
+	return ret < 0 ? ret : 0;
+}
+
+static int __qca8084_mii_write(struct mii_bus *bus, u16 addr, u16 reg, u32 val)
+{
+	int ret;
+
+	ret = __mdiobus_write(bus, addr, reg, lower_16_bits(val));
+	if (!ret)
+		ret = __mdiobus_write(bus, addr, reg | BIT(1), upper_16_bits(val));
+
+	return ret;
+}
+
+static int qca8084_mii_modify(struct phy_device *phydev, u32 regaddr,
+			      u32 clear, u32 set)
+{
+	struct mii_bus *bus;
+	u16 reg, addr, page, sw_addr;
+	u32 val;
+	int ret;
+
+	bus = phydev->mdio.bus;
+	mutex_lock(&bus->mdio_lock);
+
+	qca8084_split_addr(regaddr, &reg, &addr, &page, &sw_addr);
+	ret = __qca8084_set_page(bus, sw_addr, page);
+	if (ret < 0)
+		goto qca8084_mii_modify_exit;
+
+	ret = __qca8084_mii_read(bus, QCA8084_LOW_ADDR_PREFIX | addr,
+				 reg, &val);
+	if (ret < 0)
+		goto qca8084_mii_modify_exit;
+
+	val &= ~clear;
+	val |= set;
+	ret = __qca8084_mii_write(bus, QCA8084_LOW_ADDR_PREFIX | addr,
+				  reg, val);
+qca8084_mii_modify_exit:
+	mutex_unlock(&bus->mdio_lock);
+	return ret;
+};
+
 static int at803x_enable_rx_delay(struct phy_device *phydev)
 {
 	return at803x_debug_reg_mask(phydev, AT803X_DEBUG_ANALOG_TEST_CTRL, 0,
-- 
2.42.0

[PATCH v8 09/14] net: phy: at803x: set MDIO address of qca8084 PHY

[Luo Jie]

Program the MDIO address of qca8084 PHY and PCS device
in the PHY probe function.

The MDIO address of qca8084 device is configured according
to the property "qcom,phy-addr-fixup" of phy node, which
defines the MDIO address for 4 PHYs and 3 PCSes, each MDIO
address occupies 5 bits in the config register.

The MDIO address of qca8084 should be configured correctly
before doing the clock initialization in the PHY probe function,
so the property "reg" can't be used to configure the MDIO address
of phy device one by one, the clock initialization will be configured
with all 4 PHY devices in one PHY probe function.

Signed-off-by: Luo Jie <quic_luoj@quicinc.com>
---
 drivers/net/phy/at803x.c | 61 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 61 insertions(+)

diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index 4982bde5a8a5..c8830898ce2e 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -303,6 +303,18 @@
 #define QCA8084_HIGH_ADDR_PREFIX		0x18
 #define QCA8084_LOW_ADDR_PREFIX			0x10
 
+#define QCA8084_PCS_CFG				0xc90f014
+#define QCA8084_PCS_ADDR0_MASK			GENMASK(4, 0)
+#define QCA8084_PCS_ADDR1_MASK			GENMASK(9, 5)
+#define QCA8084_PCS_ADDR2_MASK			GENMASK(14, 10)
+
+#define QCA8084_EPHY_CFG			0xc90f018
+#define QCA8084_EPHY_ADDR0_MASK			GENMASK(4, 0)
+#define QCA8084_EPHY_ADDR1_MASK			GENMASK(9, 5)
+#define QCA8084_EPHY_ADDR2_MASK			GENMASK(14, 10)
+#define QCA8084_EPHY_ADDR3_MASK			GENMASK(19, 15)
+#define QCA8084_EPHY_LDO_EN			GENMASK(21, 20)
+
 MODULE_DESCRIPTION("Qualcomm Atheros AR803x and QCA808X PHY driver");
 MODULE_AUTHOR("Matus Ujhelyi");
 MODULE_LICENSE("GPL");
@@ -764,6 +776,51 @@ static int at803x_parse_dt(struct phy_device *phydev)
 	return 0;
 }
 
+static int qca8084_parse_and_set_phyaddr(struct phy_device *phydev)
+{
+	struct device_node *node;
+	u32 addr[7];
+	int ret;
+
+	node = phydev->mdio.dev.of_node;
+
+	/* The property "qcom,phy-addr-fixup" is only defined in one
+	 * PHY device tree node.
+	 */
+	ret = of_property_read_u32_array(node, "qcom,phy-addr-fixup",
+					 addr, ARRAY_SIZE(addr));
+	if (ret)
+		return ret == -EINVAL ? 0 : ret;
+
+	/* There are 4 PHYs and 3 PCSes on qca8084 chip, each device address
+	 * occupies 5 bits of the config register to customize the MDIO address.
+	 */
+	ret = qca8084_mii_modify(phydev, QCA8084_EPHY_CFG,
+				 QCA8084_EPHY_ADDR0_MASK |
+				 QCA8084_EPHY_ADDR1_MASK |
+				 QCA8084_EPHY_ADDR2_MASK |
+				 QCA8084_EPHY_ADDR3_MASK,
+				 FIELD_PREP(QCA8084_EPHY_ADDR0_MASK, addr[0]) |
+				 FIELD_PREP(QCA8084_EPHY_ADDR1_MASK, addr[1]) |
+				 FIELD_PREP(QCA8084_EPHY_ADDR2_MASK, addr[2]) |
+				 FIELD_PREP(QCA8084_EPHY_ADDR3_MASK, addr[3]));
+	if (ret)
+		return ret;
+
+	return qca8084_mii_modify(phydev, QCA8084_PCS_CFG,
+				  QCA8084_PCS_ADDR0_MASK |
+				  QCA8084_PCS_ADDR1_MASK |
+				  QCA8084_PCS_ADDR2_MASK,
+				  FIELD_PREP(QCA8084_PCS_ADDR0_MASK, addr[4]) |
+				  FIELD_PREP(QCA8084_PCS_ADDR1_MASK, addr[5]) |
+				  FIELD_PREP(QCA8084_PCS_ADDR2_MASK, addr[6]));
+}
+
+static int qca8084_probe(struct phy_device *phydev)
+{
+	return qca8084_parse_and_set_phyaddr(phydev);
+}
+
 static int at803x_probe(struct phy_device *phydev)
 {
 	struct device *dev = &phydev->mdio.dev;
@@ -776,6 +833,9 @@ static int at803x_probe(struct phy_device *phydev)
 
 	phydev->priv = priv;
 
+	if (phydev_id_compare(phydev, QCA8084_PHY_ID))
+		return qca8084_probe(phydev);
+
 	ret = at803x_parse_dt(phydev);
 	if (ret)
 		return ret;
@@ -2510,6 +2570,7 @@ static struct phy_driver at803x_driver[] = {
 	PHY_ID_MATCH_MODEL(QCA8084_PHY_ID),
 	.name			= "Qualcomm QCA8084",
 	.flags			= PHY_POLL_CABLE_TEST,
+	.probe			= at803x_probe,
 	.config_intr		= at803x_config_intr,
 	.handle_interrupt	= at803x_handle_interrupt,
 	.get_tunable		= at803x_get_tunable,
-- 
2.42.0

[PATCH v8 10/14] net: phy: at803x: parse qca8084 clocks and resets

[Luo Jie]

These clock and reset IDs are needed to bring up qca8084,
after the initializations with these clocks and resets,
the PHY function can be accessed correctly such as reading
the capabilities of PHY.

Signed-off-by: Luo Jie <quic_luoj@quicinc.com>
---
 drivers/net/phy/at803x.c | 87 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 87 insertions(+)

diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index c8830898ce2e..4c884d6b60bc 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -21,6 +21,8 @@
 #include <linux/phylink.h>
 #include <linux/sfp.h>
 #include <dt-bindings/net/qca-ar803x.h>
+#include <linux/clk.h>
+#include <linux/reset.h>
 
 #define AT803X_SPECIFIC_FUNCTION_CONTROL	0x10
 #define AT803X_SFC_ASSERT_CRS			BIT(11)
@@ -337,6 +339,52 @@ static struct at803x_hw_stat qca83xx_hw_stats[] = {
 	{ "eee_wake_errors", 0x16, GENMASK(15, 0), MMD},
 };
 
+enum {
+	SRDS0_SYS_CLK,
+	SRDS1_SYS_CLK,
+	GEPHY0_SYS_CLK,
+	GEPHY1_SYS_CLK,
+	GEPHY2_SYS_CLK,
+	GEPHY3_SYS_CLK,
+};
+
+enum {
+	SRDS0_SYS_RESET,
+	SRDS1_SYS_RESET,
+	GEPHY0_SYS_RESET,
+	GEPHY1_SYS_RESET,
+	GEPHY2_SYS_RESET,
+	GEPHY3_SYS_RESET,
+	GEPHY0_RESET,
+	GEPHY1_RESET,
+	GEPHY2_RESET,
+	GEPHY3_RESET,
+	GEPHY_DSP_RESET,
+};
+
+static const char *const qca8084_clock_name[] = {
+	"srds0_sys",
+	"srds1_sys",
+	"gephy0_sys",
+	"gephy1_sys",
+	"gephy2_sys",
+	"gephy3_sys",
+};
+
+static const char *const qca8084_reset_name[] = {
+	"srds0_sys",
+	"srds1_sys",
+	"gephy0_sys",
+	"gephy1_sys",
+	"gephy2_sys",
+	"gephy3_sys",
+	"gephy0_soft",
+	"gephy1_soft",
+	"gephy2_soft",
+	"gephy3_soft",
+	"gephy_dsp",
+};
+
 struct at803x_priv {
 	int flags;
 	u16 clk_25m_reg;
@@ -348,6 +396,8 @@ struct at803x_priv {
 	struct regulator_dev *vddio_rdev;
 	struct regulator_dev *vddh_rdev;
 	u64 stats[ARRAY_SIZE(qca83xx_hw_stats)];
+	struct clk *clk[ARRAY_SIZE(qca8084_clock_name)];
+	struct reset_control *reset[ARRAY_SIZE(qca8084_reset_name)];
 };
 
 struct at803x_context {
@@ -816,8 +866,45 @@ static int qca8084_parse_and_set_phyaddr(struct phy_device *phydev)
 				  FIELD_PREP(QCA8084_PCS_ADDR2_MASK, addr[6]));
 }
 
+static int qca8084_parse_dt(struct phy_device *phydev)
+{
+	struct at803x_priv *priv;
+	int i;
+
+	priv = phydev->priv;
+	for (i = 0; i < ARRAY_SIZE(qca8084_clock_name); i++) {
+		priv->clk[i] = devm_clk_get_optional(&phydev->mdio.dev,
+						     qca8084_clock_name[i]);
+		if (IS_ERR(priv->clk[i])) {
+			phydev_err(phydev, "failed to get the clock ID %s!\n",
+				   qca8084_clock_name[i]);
+
+			return PTR_ERR(priv->clk[i]);
+		}
+	}
+
+	for (i = 0; i < ARRAY_SIZE(qca8084_reset_name); i++) {
+		priv->reset[i] = devm_reset_control_get_optional_exclusive(&phydev->mdio.dev,
+									   qca8084_reset_name[i]);
+		if (IS_ERR(priv->reset[i])) {
+			phydev_err(phydev, "failed to get the reset ID %s!\n",
+				   qca8084_reset_name[i]);
+
+			return PTR_ERR(priv->reset[i]);
+		}
+	}
+
+	return 0;
+}
+
 static int qca8084_probe(struct phy_device *phydev)
 {
+	int ret;
+
+	ret = qca8084_parse_dt(phydev);
+	if (ret)
+		return ret;
+
 	return qca8084_parse_and_set_phyaddr(phydev);
 }
 
-- 
2.42.0

[PATCH v8 11/14] net: phy: at803x: add qca808x initial config sequence

[Luo Jie]

After GPIO reset, these Ethernet clock sequence needs to be
configured before reading the features of PHY, the Ethernet
system clock works on 25MHZ.

Signed-off-by: Luo Jie <quic_luoj@quicinc.com>
---
 drivers/net/phy/at803x.c | 84 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 83 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index 4c884d6b60bc..9885a728c72a 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -897,6 +897,84 @@ static int qca8084_parse_dt(struct phy_device *phydev)
 	return 0;
 }
 
+static int qca8084_clock_config(struct phy_device *phydev)
+{
+	struct at803x_priv *priv;
+	int ret = 0;
+
+	/* The ethernet clock IDs are only defined in one PHY device
+	 * tree node, and these ethernet clocks only needs to be configured
+	 * one time, which work on the clock rate 25MHZ.
+	 */
+	priv = phydev->priv;
+	if (!priv->clk[SRDS0_SYS_CLK])
+		return 0;
+
+	ret = clk_set_rate(priv->clk[SRDS0_SYS_CLK], 25000000);
+	if (ret)
+		return ret;
+
+	ret = clk_prepare_enable(priv->clk[SRDS0_SYS_CLK]);
+	if (ret)
+		return ret;
+
+	ret = clk_prepare_enable(priv->clk[SRDS1_SYS_CLK]);
+	if (ret)
+		return ret;
+
+	/* Reset PCS system clocks */
+	reset_control_assert(priv->reset[SRDS0_SYS_RESET]);
+	reset_control_assert(priv->reset[SRDS1_SYS_RESET]);
+	fsleep(20000);
+
+	reset_control_deassert(priv->reset[SRDS0_SYS_RESET]);
+	reset_control_deassert(priv->reset[SRDS1_SYS_RESET]);
+
+	ret = clk_prepare_enable(priv->clk[GEPHY0_SYS_CLK]);
+	if (ret)
+		return ret;
+
+	ret = clk_prepare_enable(priv->clk[GEPHY1_SYS_CLK]);
+	if (ret)
+		return ret;
+
+	ret = clk_prepare_enable(priv->clk[GEPHY2_SYS_CLK]);
+	if (ret)
+		return ret;
+
+	ret = clk_prepare_enable(priv->clk[GEPHY3_SYS_CLK]);
+	if (ret)
+		return ret;
+
+	/* Reset ethernet system clocks */
+	reset_control_assert(priv->reset[GEPHY0_SYS_RESET]);
+	reset_control_assert(priv->reset[GEPHY1_SYS_RESET]);
+	reset_control_assert(priv->reset[GEPHY2_SYS_RESET]);
+	reset_control_assert(priv->reset[GEPHY3_SYS_RESET]);
+	fsleep(20000);
+
+	reset_control_deassert(priv->reset[GEPHY0_SYS_RESET]);
+	reset_control_deassert(priv->reset[GEPHY1_SYS_RESET]);
+	reset_control_deassert(priv->reset[GEPHY2_SYS_RESET]);
+	reset_control_deassert(priv->reset[GEPHY3_SYS_RESET]);
+
+	/* Release ethernet DSP reset */
+	reset_control_deassert(priv->reset[GEPHY0_RESET]);
+	reset_control_deassert(priv->reset[GEPHY1_RESET]);
+	reset_control_deassert(priv->reset[GEPHY2_RESET]);
+	reset_control_deassert(priv->reset[GEPHY3_RESET]);
+	reset_control_deassert(priv->reset[GEPHY_DSP_RESET]);
+
+	/* Enable efuse loading into analog circuit */
+	ret = qca8084_mii_modify(phydev, QCA8084_EPHY_CFG,
+				 QCA8084_EPHY_LDO_EN, 0);
+	if (ret)
+		return ret;
+
+	fsleep(10000);
+	return 0;
+}
+
 static int qca8084_probe(struct phy_device *phydev)
 {
 	int ret;
@@ -905,7 +983,11 @@ static int qca8084_probe(struct phy_device *phydev)
 	if (ret)
 		return ret;
 
-	return qca8084_parse_and_set_phyaddr(phydev);
+	ret = qca8084_parse_and_set_phyaddr(phydev);
+	if (ret)
+		return ret;
+
+	return qca8084_clock_config(phydev);
 }
 
 static int at803x_probe(struct phy_device *phydev)
-- 
2.42.0

[PATCH v8 12/14] net: phy: at803x: configure qca8084 common clocks

[Luo Jie]

After initial clock sequence, the clock source 312.5MHZ is
available, the common clocks based on clock source 312.5MHZ
needs to be configured, which includes APB bridge clock tree
with rate 312.5MHZ, AHB clock tree with 104.17MHZ.

Signed-off-by: Luo Jie <quic_luoj@quicinc.com>
---
 drivers/net/phy/at803x.c | 69 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 68 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index 9885a728c72a..3ef4eacf40c7 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -340,6 +340,14 @@ static struct at803x_hw_stat qca83xx_hw_stats[] = {
 };
 
 enum {
+	APB_BRIDGE_CLK,
+	AHB_CLK,
+	SEC_CTRL_AHB_CLK,
+	TLMM_CLK,
+	TLMM_AHB_CLK,
+	CNOC_AHB_CLK,
+	MDIO_AHB_CLK,
+	MDIO_MASTER_AHB_CLK,
 	SRDS0_SYS_CLK,
 	SRDS1_SYS_CLK,
 	GEPHY0_SYS_CLK,
@@ -363,6 +371,14 @@ enum {
 };
 
 static const char *const qca8084_clock_name[] = {
+	"apb_bridge",
+	"ahb",
+	"sec_ctrl_ahb",
+	"tlmm",
+	"tlmm_ahb",
+	"cnoc_ahb",
+	"mdio_ahb",
+	"mdio_master_ahb",
 	"srds0_sys",
 	"srds1_sys",
 	"gephy0_sys",
@@ -975,6 +991,53 @@ static int qca8084_clock_config(struct phy_device *phydev)
 	return 0;
 }
 
+static int qca8084_common_clock_init(struct phy_device *phydev)
+{
+	struct at803x_priv *priv;
+	int ret = 0;
+
+	priv = phydev->priv;
+	/* Enable APB bridge tree clock */
+	ret = clk_set_rate(priv->clk[APB_BRIDGE_CLK], 312500000);
+	if (ret)
+		return ret;
+
+	ret = clk_prepare_enable(priv->clk[APB_BRIDGE_CLK]);
+	if (ret)
+		return ret;
+
+	/* Enable AHB tree clocks */
+	ret = clk_set_rate(priv->clk[AHB_CLK], 104170000);
+	if (ret)
+		return ret;
+
+	ret = clk_prepare_enable(priv->clk[AHB_CLK]);
+	if (ret)
+		return ret;
+
+	ret = clk_prepare_enable(priv->clk[SEC_CTRL_AHB_CLK]);
+	if (ret)
+		return ret;
+
+	ret = clk_prepare_enable(priv->clk[TLMM_CLK]);
+	if (ret)
+		return ret;
+
+	ret = clk_prepare_enable(priv->clk[TLMM_AHB_CLK]);
+	if (ret)
+		return ret;
+
+	ret = clk_prepare_enable(priv->clk[CNOC_AHB_CLK]);
+	if (ret)
+		return ret;
+
+	ret = clk_prepare_enable(priv->clk[MDIO_AHB_CLK]);
+	if (ret)
+		return ret;
+
+	return clk_prepare_enable(priv->clk[MDIO_MASTER_AHB_CLK]);
+}
+
 static int qca8084_probe(struct phy_device *phydev)
 {
 	int ret;
@@ -987,7 +1050,11 @@ static int qca8084_probe(struct phy_device *phydev)
 	if (ret)
 		return ret;
 
-	return qca8084_clock_config(phydev);
+	ret = qca8084_clock_config(phydev);
+	if (ret)
+		return ret;
+
+	return qca8084_common_clock_init(phydev);
 }
 
 static int at803x_probe(struct phy_device *phydev)
-- 
2.42.0

[PATCH v8 13/14] net: phy: at803x: configure qca8084 work mode

[Luo Jie]

There are four kind of work modes supported by qca8084.
1. Quad PHYs work on 10g-qxgmii.
2. PHY1, PHY2, PHY3 wors on 10g-qxgmii, PHY4 works on sgmii.
3. Quad PHYs connected with internal MACs by GMII, which works
   on switch mode.
4. PHY1, PHY2, PHY3 connected with internal MACs by GMII, PHY4
   works on sgmii.

Signed-off-by: Luo Jie <quic_luoj@quicinc.com>
---
 drivers/net/phy/at803x.c | 53 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 52 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index 3ef4eacf40c7..ac72b0551ed8 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -317,6 +317,13 @@
 #define QCA8084_EPHY_ADDR3_MASK			GENMASK(19, 15)
 #define QCA8084_EPHY_LDO_EN			GENMASK(21, 20)
 
+#define QCA8084_WORK_MODE_CFG			0xc90f030
+#define QCA8084_WORK_MODE_MASK			GENMASK(5, 0)
+#define QCA8084_WORK_MODE_QXGMII		(BIT(5) | GENMASK(3, 0))
+#define QCA8084_WORK_MODE_QXGMII_PORT4_SGMII	(BIT(5) | GENMASK(2, 0))
+#define QCA8084_WORK_MODE_SWITCH		BIT(4)
+#define QCA8084_WORK_MODE_SWITCH_PORT4_SGMII	BIT(5)
+
 MODULE_DESCRIPTION("Qualcomm Atheros AR803x and QCA808X PHY driver");
 MODULE_AUTHOR("Matus Ujhelyi");
 MODULE_LICENSE("GPL");
@@ -1038,6 +1045,46 @@ static int qca8084_common_clock_init(struct phy_device *phydev)
 	return clk_prepare_enable(priv->clk[MDIO_MASTER_AHB_CLK]);
 }
 
+static int qca8084_parse_and_set_work_mode(struct phy_device *phydev)
+{
+	struct device_node *node;
+	struct at803x_priv *priv;
+	u32 value, work_mode;
+	int ret;
+
+	node = phydev->mdio.dev.of_node;
+	priv = phydev->priv;
+
+	/* The property "qcom,phy-work-mode" is only defined in one
+	 * PHY device tree node.
+	 */
+	ret = of_property_read_u32(node, "qcom,phy-work-mode", &value);
+	if (ret)
+		return ret == -EINVAL ? 0 : ret;
+
+	switch (value) {
+	case 0:
+		work_mode = QCA8084_WORK_MODE_QXGMII;
+		break;
+	case 1:
+		work_mode = QCA8084_WORK_MODE_QXGMII_PORT4_SGMII;
+		break;
+	case 2:
+		work_mode = QCA8084_WORK_MODE_SWITCH;
+		break;
+	case 3:
+		work_mode = QCA8084_WORK_MODE_SWITCH_PORT4_SGMII;
+		break;
+	default:
+		phydev_err(phydev, "invalid qcom,phy-work-mode %d\n", value);
+		return -EINVAL;
+	}
+
+	return qca8084_mii_modify(phydev, QCA8084_WORK_MODE_CFG,
+				  QCA8084_WORK_MODE_MASK,
+				  FIELD_PREP(QCA8084_WORK_MODE_MASK, work_mode));
+}
+
 static int qca8084_probe(struct phy_device *phydev)
 {
 	int ret;
@@ -1054,7 +1101,11 @@ static int qca8084_probe(struct phy_device *phydev)
 	if (ret)
 		return ret;
 
-	return qca8084_common_clock_init(phydev);
+	ret = qca8084_common_clock_init(phydev);
+	if (ret)
+		return ret;
+
+	return qca8084_parse_and_set_work_mode(phydev);
 }
 
 static int at803x_probe(struct phy_device *phydev)
-- 
2.42.0

Re: [PATCH v8 13/14] net: phy: at803x: configure qca8084 work mode

[Simon Horman]

On Fri, Dec 15, 2023 at 03:40:03PM +0800, Luo Jie wrote:

...

> @@ -1038,6 +1045,46 @@ static int qca8084_common_clock_init(struct phy_device *phydev)
>  	return clk_prepare_enable(priv->clk[MDIO_MASTER_AHB_CLK]);
>  }
>  
> +static int qca8084_parse_and_set_work_mode(struct phy_device *phydev)
> +{
> +	struct device_node *node;
> +	struct at803x_priv *priv;
> +	u32 value, work_mode;
> +	int ret;
> +
> +	node = phydev->mdio.dev.of_node;
> +	priv = phydev->priv;

Hi Luo Jie,

a minor nit from my side: priv is set but otherwise unused in this function.

> +
> +	/* The property "qcom,phy-work-mode" is only defined in one
> +	 * PHY device tree node.
> +	 */
> +	ret = of_property_read_u32(node, "qcom,phy-work-mode", &value);
> +	if (ret)
> +		return ret == -EINVAL ? 0 : ret;
> +
> +	switch (value) {
> +	case 0:
> +		work_mode = QCA8084_WORK_MODE_QXGMII;
> +		break;
> +	case 1:
> +		work_mode = QCA8084_WORK_MODE_QXGMII_PORT4_SGMII;
> +		break;
> +	case 2:
> +		work_mode = QCA8084_WORK_MODE_SWITCH;
> +		break;
> +	case 3:
> +		work_mode = QCA8084_WORK_MODE_SWITCH_PORT4_SGMII;
> +		break;
> +	default:
> +		phydev_err(phydev, "invalid qcom,phy-work-mode %d\n", value);
> +		return -EINVAL;
> +	}
> +
> +	return qca8084_mii_modify(phydev, QCA8084_WORK_MODE_CFG,
> +				  QCA8084_WORK_MODE_MASK,
> +				  FIELD_PREP(QCA8084_WORK_MODE_MASK, work_mode));
> +}

...

Re: [PATCH v8 13/14] net: phy: at803x: configure qca8084 work mode

[Jie Luo]

On 12/17/2023 9:02 PM, Simon Horman wrote:
> On Fri, Dec 15, 2023 at 03:40:03PM +0800, Luo Jie wrote:
> 
> ...
> 
>> @@ -1038,6 +1045,46 @@ static int qca8084_common_clock_init(struct phy_device *phydev)
>>   	return clk_prepare_enable(priv->clk[MDIO_MASTER_AHB_CLK]);
>>   }
>>   
>> +static int qca8084_parse_and_set_work_mode(struct phy_device *phydev)
>> +{
>> +	struct device_node *node;
>> +	struct at803x_priv *priv;
>> +	u32 value, work_mode;
>> +	int ret;
>> +
>> +	node = phydev->mdio.dev.of_node;
>> +	priv = phydev->priv;
> 
> Hi Luo Jie,
> 
> a minor nit from my side: priv is set but otherwise unused in this function.

Thanks Simon, i will update it to remove it.

> 
>> +
>> +	/* The property "qcom,phy-work-mode" is only defined in one
>> +	 * PHY device tree node.
>> +	 */
>> +	ret = of_property_read_u32(node, "qcom,phy-work-mode", &value);
>> +	if (ret)
>> +		return ret == -EINVAL ? 0 : ret;
>> +
>> +	switch (value) {
>> +	case 0:
>> +		work_mode = QCA8084_WORK_MODE_QXGMII;
>> +		break;
>> +	case 1:
>> +		work_mode = QCA8084_WORK_MODE_QXGMII_PORT4_SGMII;
>> +		break;
>> +	case 2:
>> +		work_mode = QCA8084_WORK_MODE_SWITCH;
>> +		break;
>> +	case 3:
>> +		work_mode = QCA8084_WORK_MODE_SWITCH_PORT4_SGMII;
>> +		break;
>> +	default:
>> +		phydev_err(phydev, "invalid qcom,phy-work-mode %d\n", value);
>> +		return -EINVAL;
>> +	}
>> +
>> +	return qca8084_mii_modify(phydev, QCA8084_WORK_MODE_CFG,
>> +				  QCA8084_WORK_MODE_MASK,
>> +				  FIELD_PREP(QCA8084_WORK_MODE_MASK, work_mode));
>> +}
> 
> ...

[PATCH v8 14/14] dt-bindings: net: ar803x: add qca8084 PHY properties

[Luo Jie]

The following properties are added for qca8084 PHY.

1. add the compatible string "ethernet-phy-id004d.d180" since
   the PHY device is not accessible during MDIO bus register.
2. add property "qcom,phy-addr-fixup" for customizing MDIO address.
3. add property "qcom,phy-work-mode" for specifying qca8084 PHY
   work mode.
4. add the initial clocks and resets.

Signed-off-by: Luo Jie <quic_luoj@quicinc.com>
---
 .../devicetree/bindings/net/qca,ar803x.yaml   | 158 +++++++++++++++++-
 1 file changed, 155 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/qca,ar803x.yaml b/Documentation/devicetree/bindings/net/qca,ar803x.yaml
index 3acd09f0da86..febff039a44f 100644
--- a/Documentation/devicetree/bindings/net/qca,ar803x.yaml
+++ b/Documentation/devicetree/bindings/net/qca,ar803x.yaml
@@ -14,9 +14,6 @@ maintainers:
 description: |
   Bindings for Qualcomm Atheros AR803x PHYs
 
-allOf:
-  - $ref: ethernet-phy.yaml#
-
 properties:
   qca,clk-out-frequency:
     description: Clock output frequency in Hertz.
@@ -85,6 +82,161 @@ properties:
     $ref: /schemas/regulator/regulator.yaml
     unevaluatedProperties: false
 
+  qcom,phy-addr-fixup:
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    description:
+      MDIO address for 4 PHY devices and 3 PCS devices
+
+  qcom,phy-work-mode:
+    description: PHY device work mode.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [0, 1, 2, 3]
+
+  clocks:
+    items:
+      - description: APB bridge clock
+      - description: AHB clock
+      - description: Security control clock
+      - description: TLMM clock
+      - description: TLMM AHB clock
+      - description: CNOC AHB clock
+      - description: MDIO AHB clock
+      - description: MDIO master AHB clock
+      - description: PCS0 system clock
+      - description: PCS1 system clock
+      - description: EPHY0 system clock
+      - description: EPHY1 system clock
+      - description: EPHY2 system clock
+      - description: EPHY3 system clock
+    description: PHY initial common clock configs
+
+  clock-names:
+    items:
+      - const: apb_bridge
+      - const: ahb
+      - const: sec_ctrl_ahb
+      - const: tlmm
+      - const: tlmm_ahb
+      - const: cnoc_ahb
+      - const: mdio_ahb
+      - const: mdio_master_ahb
+      - const: srds0_sys
+      - const: srds1_sys
+      - const: gephy0_sys
+      - const: gephy1_sys
+      - const: gephy2_sys
+      - const: gephy3_sys
+
+  resets:
+    items:
+      - description: PCS0 system reset
+      - description: PCS1 system reset
+      - description: EPHY0 system reset
+      - description: EPHY1 system reset
+      - description: EPHY2 system reset
+      - description: EPHY3 system reset
+      - description: EPHY0 software reset
+      - description: EPHY1 software reset
+      - description: EPHY2 software reset
+      - description: EPHY3 software reset
+      - description: Ethernet DSP reset
+    description: PHY initial common reset configs
+
+  reset-names:
+    items:
+      - const: srds0_sys
+      - const: srds1_sys
+      - const: gephy0_sys
+      - const: gephy1_sys
+      - const: gephy2_sys
+      - const: gephy3_sys
+      - const: gephy0_soft
+      - const: gephy1_soft
+      - const: gephy2_soft
+      - const: gephy3_soft
+      - const: gephy_dsp
+
+allOf:
+  - $ref: ethernet-phy.yaml#
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - ethernet-phy-id004d.d180
+    then:
+      properties:
+        clocks:
+          items:
+            - description: APB bridge clock
+            - description: AHB clock
+            - description: Security control clock
+            - description: TLMM clock
+            - description: TLMM AHB clock
+            - description: CNOC AHB clock
+            - description: MDIO AHB clock
+            - description: MDIO master AHB clock
+            - description: PCS0 system clock
+            - description: PCS1 system clock
+            - description: EPHY0 system clock
+            - description: EPHY1 system clock
+            - description: EPHY2 system clock
+            - description: EPHY3 system clock
+        clock-names:
+          items:
+            - const: apb_bridge
+            - const: ahb
+            - const: sec_ctrl_ahb
+            - const: tlmm
+            - const: tlmm_ahb
+            - const: cnoc_ahb
+            - const: mdio_ahb
+            - const: mdio_master_ahb
+            - const: srds0_sys
+            - const: srds1_sys
+            - const: gephy0_sys
+            - const: gephy1_sys
+            - const: gephy2_sys
+            - const: gephy3_sys
+        resets:
+          items:
+            - description: PCS0 system reset
+            - description: PCS1 system reset
+            - description: EPHY0 system reset
+            - description: EPHY1 system reset
+            - description: EPHY2 system reset
+            - description: EPHY3 system reset
+            - description: EPHY0 software reset
+            - description: EPHY1 software reset
+            - description: EPHY2 software reset
+            - description: EPHY3 software reset
+            - description: Ethernet DSP reset
+        reset-names:
+          items:
+            - const: srds0_sys
+            - const: srds1_sys
+            - const: gephy0_sys
+            - const: gephy1_sys
+            - const: gephy2_sys
+            - const: gephy3_sys
+            - const: gephy0_soft
+            - const: gephy1_soft
+            - const: gephy2_soft
+            - const: gephy3_soft
+            - const: gephy_dsp
+      required:
+        - qcom,phy-addr-fixup
+        - qcom,phy-work-mode
+        - clocks
+        - clock-names
+        - resets
+        - reset-names
+    else:
+      properties:
+        qcom,phy-addr-fixup: false
+        qcom,phy-work-mode: false
+
 unevaluatedProperties: false
 
 examples:
-- 
2.42.0

Re: [PATCH v8 14/14] dt-bindings: net: ar803x: add qca8084 PHY properties

[Krzysztof Kozlowski]

On 15/12/2023 08:40, Luo Jie wrote:
> The following properties are added for qca8084 PHY.
> 
> 1. add the compatible string "ethernet-phy-id004d.d180" since
>    the PHY device is not accessible during MDIO bus register.
> 2. add property "qcom,phy-addr-fixup" for customizing MDIO address.
> 3. add property "qcom,phy-work-mode" for specifying qca8084 PHY
>    work mode.
> 4. add the initial clocks and resets.

All my previous comments (sent one minute before this patchset :) )
apply. Please respond to them or implement them in v5 (not earlier than
after 24h).


Best regards,
Krzysztof

Re: [PATCH v8 14/14] dt-bindings: net: ar803x: add qca8084 PHY properties

[Jie Luo]

On 12/15/2023 4:22 PM, Krzysztof Kozlowski wrote:
> On 15/12/2023 08:40, Luo Jie wrote:
>> The following properties are added for qca8084 PHY.
>>
>> 1. add the compatible string "ethernet-phy-id004d.d180" since
>>     the PHY device is not accessible during MDIO bus register.
>> 2. add property "qcom,phy-addr-fixup" for customizing MDIO address.
>> 3. add property "qcom,phy-work-mode" for specifying qca8084 PHY
>>     work mode.
>> 4. add the initial clocks and resets.
> 
> All my previous comments (sent one minute before this patchset :) )
> apply. Please respond to them or implement them in v5 (not earlier than
> after 24h).
> 
> 
> Best regards,
> Krzysztof
> 

Sure, will update the new version after the discussion completed.
actually i have query about the dt-bindings doc.

The "maxItems: 1" of the property resets is defined in ethernet-phy.yaml
that is referenced by qca,ar803x.yaml, but i have 11 reset instances
used for qca8084 PHY, it seems i can't overwrite maxItems of reset
property to 11 from 1 in qca,ar803x.yaml.

is there any method to overwrite the maxItems value?

Thanks,
Jie

Re: [PATCH v8 14/14] dt-bindings: net: ar803x: add qca8084 PHY properties

[Andrew Lunn]

> The "maxItems: 1" of the property resets is defined in ethernet-phy.yaml
> that is referenced by qca,ar803x.yaml, but i have 11 reset instances
> used for qca8084 PHY

11!?!?? Really? Why?

I assume the order and timer matters, otherwise why would you need
11? So the PHY driver needs to handle this, not phylib framework. So
you will be adding vendor properties to describe all 11 of them. So
ethernet-phy.yaml does not matter.

	Andrew

Re: [PATCH v8 14/14] dt-bindings: net: ar803x: add qca8084 PHY properties

[Jie Luo]

On 12/15/2023 7:25 PM, Andrew Lunn wrote:
>> The "maxItems: 1" of the property resets is defined in ethernet-phy.yaml
>> that is referenced by qca,ar803x.yaml, but i have 11 reset instances
>> used for qca8084 PHY
> 
> 11!?!?? Really? Why?
> 
> I assume the order and timer matters, otherwise why would you need
> 11? So the PHY driver needs to handle this, not phylib framework. So
> you will be adding vendor properties to describe all 11 of them. So
> ethernet-phy.yaml does not matter.
> 
> 	Andrew

Since these resets need to be configured in the special sequence, and
these clocks need to be configured with different clock rate.

But the clock instance get, the property name is fixed to "clock-names"
according to the function of_parse_clkspec, and the reset property name
is also fixed to "reset-names" from function __of_reset_control_get.

Re: [PATCH v8 14/14] dt-bindings: net: ar803x: add qca8084 PHY properties

[Russell King (Oracle)]

On Fri, Dec 15, 2023 at 08:16:53PM +0800, Jie Luo wrote:
> On 12/15/2023 7:25 PM, Andrew Lunn wrote:
> > > The "maxItems: 1" of the property resets is defined in ethernet-phy.yaml
> > > that is referenced by qca,ar803x.yaml, but i have 11 reset instances
> > > used for qca8084 PHY
> > 
> > 11!?!?? Really? Why?
> > 
> > I assume the order and timer matters, otherwise why would you need
> > 11? So the PHY driver needs to handle this, not phylib framework. So
> > you will be adding vendor properties to describe all 11 of them. So
> > ethernet-phy.yaml does not matter.
> > 
> > 	Andrew
> 
> Since these resets need to be configured in the special sequence, and
> these clocks need to be configured with different clock rate.
> 
> But the clock instance get, the property name is fixed to "clock-names"
> according to the function of_parse_clkspec, and the reset property name
> is also fixed to "reset-names" from function __of_reset_control_get.

I think you need to give more details about this.

Where are these 11 resets located? What is the sequence? Why does the
PHY driver need to deal with each individual reset?

IMHO, a PHY driver should _not_ be dealing with the resets outside of
the PHY device itself, and I find it hard to imagine that qca8084
would have 11 external resets.

If these are 11 internal resets (to qca8084) then why are you using the
reset subsystem, and why do you need to describe them in DT? Surely if
they are internal to the PHY, that can be encapsulated within the PHY
driver?

This is an example of why it is useful to have an _example_ of the use
of this binding, because it would answer some of the above questions.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH v8 14/14] dt-bindings: net: ar803x: add qca8084 PHY properties

[Jie Luo]

On 12/15/2023 9:42 PM, Russell King (Oracle) wrote:
> On Fri, Dec 15, 2023 at 08:16:53PM +0800, Jie Luo wrote:
>> On 12/15/2023 7:25 PM, Andrew Lunn wrote:
>>>> The "maxItems: 1" of the property resets is defined in ethernet-phy.yaml
>>>> that is referenced by qca,ar803x.yaml, but i have 11 reset instances
>>>> used for qca8084 PHY
>>>
>>> 11!?!?? Really? Why?
>>>
>>> I assume the order and timer matters, otherwise why would you need
>>> 11? So the PHY driver needs to handle this, not phylib framework. So
>>> you will be adding vendor properties to describe all 11 of them. So
>>> ethernet-phy.yaml does not matter.
>>>
>>> 	Andrew
>>
>> Since these resets need to be configured in the special sequence, and
>> these clocks need to be configured with different clock rate.
>>
>> But the clock instance get, the property name is fixed to "clock-names"
>> according to the function of_parse_clkspec, and the reset property name
>> is also fixed to "reset-names" from function __of_reset_control_get.
> 
> I think you need to give more details about this.
> 
> Where are these 11 resets located? What is the sequence? Why does the
> PHY driver need to deal with each individual reset?

All these resets and clocks are located in qca8084 chip that includes
the GCC module registered as the clock provider, all these clocks and
resets are from qca8084 clock provider driver, the clock and reset name
prefix with PCS or EPHY is for the individual MDIO device, others are
the qca8084 level, but these clocks and resets need to be initialized
completely, then the PHY probe function can be initialized correctly
with the PHY capabilities acquired after the qca8084 chip level GPIO
reset.

The sequence code is realized in the function qca8084_clock_config of
the patch <net: phy: at803x: add qca808x initial config sequence>, this
function is doing as below.
1. set the Ethernet system clock tree as 25M clock rate.
2. reset and enable PCS system clocks.
3. reset and enable the PHY system clocks.
4. loading the efuse, which is not related the clock and reset.

> 
> IMHO, a PHY driver should _not_ be dealing with the resets outside of
> the PHY device itself, and I find it hard to imagine that qca8084
> would have 11 external resets.

Indeed, these resets are not for a PHY device, some are the chip level
resets, and each PHY has the individual resets.

> 
> If these are 11 internal resets (to qca8084) then why are you using the
> reset subsystem, and why do you need to describe them in DT? Surely if
> they are internal to the PHY, that can be encapsulated within the PHY
> driver?

These resets and clocks are realized by the qca8k clock controller
driver, and i use the clock consumer APIs to initialize these clocks and
resets,
when qca8084 works on the PHY mode, there are only PHY driver
and PCS driver needed to be enabled to make PHY working, and these
clocks and resets need to be initialized before the PHY probe function
finished correctly, where the phy capabilities is read during the probe
function.

> 
> This is an example of why it is useful to have an _example_ of the use
> of this binding, because it would answer some of the above questions.
> 

Yes, Russell, i will add an example in the DT doc in the next patch set.
The following is the device node used for the current qca8084 PHY
code design.

mdio: mdio@90000 {
     ethernet-phy@0 { 

               compatible = "ethernet-phy-id004d.d180"; 

               reg = <1>; 

               qcom,phy-addr-fixup = <1 2 3 4 5 6 7>; 

               qcom,phy-work-mode = <2>; 

               clocks = <&qca8k_nsscc NSS_CC_APB_BRIDGE_CLK>, 

                      <&qca8k_nsscc NSS_CC_AHB_CLK>, 

                      <&qca8k_nsscc NSS_CC_SEC_CTRL_AHB_CLK>, 

                      <&qca8k_nsscc NSS_CC_TLMM_CLK>, 

                      <&qca8k_nsscc NSS_CC_TLMM_AHB_CLK>, 

                      <&qca8k_nsscc NSS_CC_CNOC_AHB_CLK>, 

                      <&qca8k_nsscc NSS_CC_MDIO_AHB_CLK>, 

                      <&qca8k_nsscc NSS_CC_MDIO_MASTER_AHB_CLK>, 

                      <&qca8k_nsscc NSS_CC_SRDS0_SYS_CLK>, 

                      <&qca8k_nsscc NSS_CC_SRDS1_SYS_CLK>, 

                      <&qca8k_nsscc NSS_CC_GEPHY0_SYS_CLK>, 

                      <&qca8k_nsscc NSS_CC_GEPHY1_SYS_CLK>, 

                      <&qca8k_nsscc NSS_CC_GEPHY2_SYS_CLK>, 

                      <&qca8k_nsscc NSS_CC_GEPHY3_SYS_CLK>; 

 

               clock-names = "apb_bridge", 

                       "ahb", 

                       "sec_ctrl_ahb", 

                       "tlmm", 

                       "tlmm_ahb", 

                       "cnoc_ahb", 

                       "mdio_ahb", 

                       "mdio_master_ahb", 

                       "srds0_sys", 

                       "srds1_sys", 

                       "gephy0_sys", 

                       "gephy1_sys", 

                       "gephy2_sys", 

                       "gephy3_sys";
               resets = <&qca8k_nsscc NSS_CC_SRDS0_SYS_ARES>, 

                      <&qca8k_nsscc NSS_CC_SRDS1_SYS_ARES>, 

                      <&qca8k_nsscc NSS_CC_GEPHY0_SYS_ARES>, 

                      <&qca8k_nsscc NSS_CC_GEPHY1_SYS_ARES>, 

                      <&qca8k_nsscc NSS_CC_GEPHY2_SYS_ARES>, 

                      <&qca8k_nsscc NSS_CC_GEPHY3_SYS_ARES>, 

                      <&qca8k_nsscc NSS_CC_GEPHY0_ARES>, 

                      <&qca8k_nsscc NSS_CC_GEPHY1_ARES>, 

                      <&qca8k_nsscc NSS_CC_GEPHY2_ARES>, 

                      <&qca8k_nsscc NSS_CC_GEPHY3_ARES>, 

                      <&qca8k_nsscc NSS_CC_DSP_ARES>; 

 

               reset-names = "srds0_sys", 

                       "srds1_sys", 

                       "gephy0_sys", 

                       "gephy1_sys", 

                       "gephy2_sys", 

                       "gephy3_sys", 

                       "gephy0_soft", 

                       "gephy1_soft", 

                       "gephy2_soft", 

                       "gephy3_soft", 

                       "gephy_dsp"; 

 

       }; 

       ethernet-phy@1 { 

               compatible = "ethernet-phy-id004d.d180"; 

               reg = <2>; 

       }; 

       ethernet-phy@2 { 

               compatible = "ethernet-phy-id004d.d180"; 

               reg = <3>; 

       }; 

 

       ethernet-phy@3 { 

               compatible = "ethernet-phy-id004d.d180"; 

               reg = <4>; 

       };
    };

Thanks.

Re: [PATCH v8 14/14] dt-bindings: net: ar803x: add qca8084 PHY properties

[Andrew Lunn]

> Yes, Russell, i will add an example in the DT doc in the next patch set.
> The following is the device node used for the current qca8084 PHY
> code design.

If you look at Christians work, this would be expressed differently:

> mdio: mdio@90000 {
>     ethernet-phy-package@1 {
> 
>         compatible = "qca,qca8084-package";
> 
>         qcom,phy-work-mode = <2>;
>         clocks = <&qca8k_nsscc NSS_CC_APB_BRIDGE_CLK>,
>                <&qca8k_nsscc NSS_CC_AHB_CLK>,
>                <&qca8k_nsscc NSS_CC_SEC_CTRL_AHB_CLK>,
>                <&qca8k_nsscc NSS_CC_TLMM_CLK>,
>                <&qca8k_nsscc NSS_CC_TLMM_AHB_CLK>,
>                <&qca8k_nsscc NSS_CC_CNOC_AHB_CLK>,
>                <&qca8k_nsscc NSS_CC_MDIO_AHB_CLK>,
>                <&qca8k_nsscc NSS_CC_MDIO_MASTER_AHB_CLK>,
>                <&qca8k_nsscc NSS_CC_SRDS0_SYS_CLK>,
>                <&qca8k_nsscc NSS_CC_SRDS1_SYS_CLK>;
>         clock-names = "apb_bridge",
>                 "ahb",
>                 "sec_ctrl_ahb",
>                 "tlmm",
>                 "tlmm_ahb",
>                 "cnoc_ahb",
>                 "mdio_ahb",
>                 "mdio_master_ahb",
>                 "srds0_sys",
>                 "srds1_sys";
>         resets = <&qca8k_nsscc NSS_CC_SRDS0_SYS_ARES>,
>                <&qca8k_nsscc NSS_CC_SRDS1_SYS_ARES>,
>                <&qca8k_nsscc NSS_CC_DSP_ARES>;
>         reset-names = "srds0_sys",
>                       "srds1_sys";
>

All the properties above are common to the package as a whole.

Then follow the four individual PHYs, and the properties which are
specific to each one.

> 
>         ethernet-phy@0 {
>             compatible = "ethernet-phy-id004d.d180";
>             reg = <0>;
>             clocks = <qca8k_nsscc NSS_CC_GEPHY0_SYS_CLK>,
>             clock-names = <"gephy_sys">;
>             resets = <&qca8k_nsscc NSS_CC_GEPHY0_SYS_ARES>,
>                      <&qca8k_nsscc NSS_CC_GEPHY0_ARES>;
>             reset-names = "gephy_sys", "gephy_soft";
>         };
>     
>     
>         ethernet-phy@1 {
>             compatible = "ethernet-phy-id004d.d180";
>             reg = <1>;
>             clocks = <qca8k_nsscc NSS_CC_GEPHY1_SYS_CLK>,
>             clock-names = <"gephy_sys">;
>             resets = <&qca8k_nsscc NSS_CC_GEPHY1_SYS_ARES>,
>                      <&qca8k_nsscc NSS_CC_GEPHY1_ARES>;
>             reset-names = "gephy_sys", "gephy_soft";
>     
>         };
>     
>         ethernet-phy@2 {
>             compatible = "ethernet-phy-id004d.d180";
>             reg = <2>;
>             clocks = <qca8k_nsscc NSS_CC_GEPHY2_SYS_CLK>,
>             clock-names = <"gephy_sys">;
>             resets = <&qca8k_nsscc NSS_CC_GEPHY2_SYS_ARES>,
>                      <&qca8k_nsscc NSS_CC_GEPHY2_ARES>;
>             reset-names = "gephy_sys", "gephy_soft";
>     
>         };
>     
>         ethernet-phy@3 {
>             compatible = "ethernet-phy-id004d.d180";
>             reg = <3>;
>             clocks = <qca8k_nsscc NSS_CC_GEPHY3_SYS_CLK>,
>             clock-names = <"gephy_sys">;
>             reset-names = "gephy_sys", "gephy_soft";
>         };
> };

	Andrew

Re: [PATCH v8 14/14] dt-bindings: net: ar803x: add qca8084 PHY properties

[Jie Luo]

On 12/17/2023 1:17 AM, Andrew Lunn wrote:
>> Yes, Russell, i will add an example in the DT doc in the next patch set.
>> The following is the device node used for the current qca8084 PHY
>> code design.
> 
> If you look at Christians work, this would be expressed differently:
> 
>> mdio: mdio@90000 {
>>      ethernet-phy-package@1 {
>>
>>          compatible = "qca,qca8084-package";
>>
>>          qcom,phy-work-mode = <2>;
>>          clocks = <&qca8k_nsscc NSS_CC_APB_BRIDGE_CLK>,
>>                 <&qca8k_nsscc NSS_CC_AHB_CLK>,
>>                 <&qca8k_nsscc NSS_CC_SEC_CTRL_AHB_CLK>,
>>                 <&qca8k_nsscc NSS_CC_TLMM_CLK>,
>>                 <&qca8k_nsscc NSS_CC_TLMM_AHB_CLK>,
>>                 <&qca8k_nsscc NSS_CC_CNOC_AHB_CLK>,
>>                 <&qca8k_nsscc NSS_CC_MDIO_AHB_CLK>,
>>                 <&qca8k_nsscc NSS_CC_MDIO_MASTER_AHB_CLK>,
>>                 <&qca8k_nsscc NSS_CC_SRDS0_SYS_CLK>,
>>                 <&qca8k_nsscc NSS_CC_SRDS1_SYS_CLK>;
>>          clock-names = "apb_bridge",
>>                  "ahb",
>>                  "sec_ctrl_ahb",
>>                  "tlmm",
>>                  "tlmm_ahb",
>>                  "cnoc_ahb",
>>                  "mdio_ahb",
>>                  "mdio_master_ahb",
>>                  "srds0_sys",
>>                  "srds1_sys";
>>          resets = <&qca8k_nsscc NSS_CC_SRDS0_SYS_ARES>,
>>                 <&qca8k_nsscc NSS_CC_SRDS1_SYS_ARES>,
>>                 <&qca8k_nsscc NSS_CC_DSP_ARES>;
>>          reset-names = "srds0_sys",
>>                        "srds1_sys";
>>
> 
> All the properties above are common to the package as a whole.
> 
> Then follow the four individual PHYs, and the properties which are
> specific to each one.

Thanks Andrew for the proposal.
For the pure PHY chip qca8084, there is no driver to parse the package
level device tree node for common clocks and resets.

For the common clocks and resets above, whether we can add a qca8084
common device tree node as the child node of MDIO bus node, and then
parse these common properties in the PHY probe function? since the DSA
driver is not enabled for the pure PHY chip.

> 
>>
>>          ethernet-phy@0 {
>>              compatible = "ethernet-phy-id004d.d180";
>>              reg = <0>;
>>              clocks = <qca8k_nsscc NSS_CC_GEPHY0_SYS_CLK>,
>>              clock-names = <"gephy_sys">;
>>              resets = <&qca8k_nsscc NSS_CC_GEPHY0_SYS_ARES>,
>>                       <&qca8k_nsscc NSS_CC_GEPHY0_ARES>;
>>              reset-names = "gephy_sys", "gephy_soft";
>>          };
>>      
>>      
>>          ethernet-phy@1 {
>>              compatible = "ethernet-phy-id004d.d180";
>>              reg = <1>;
>>              clocks = <qca8k_nsscc NSS_CC_GEPHY1_SYS_CLK>,
>>              clock-names = <"gephy_sys">;
>>              resets = <&qca8k_nsscc NSS_CC_GEPHY1_SYS_ARES>,
>>                       <&qca8k_nsscc NSS_CC_GEPHY1_ARES>;
>>              reset-names = "gephy_sys", "gephy_soft";
>>      
>>          };
>>      
>>          ethernet-phy@2 {
>>              compatible = "ethernet-phy-id004d.d180";
>>              reg = <2>;
>>              clocks = <qca8k_nsscc NSS_CC_GEPHY2_SYS_CLK>,
>>              clock-names = <"gephy_sys">;
>>              resets = <&qca8k_nsscc NSS_CC_GEPHY2_SYS_ARES>,
>>                       <&qca8k_nsscc NSS_CC_GEPHY2_ARES>;
>>              reset-names = "gephy_sys", "gephy_soft";
>>      
>>          };
>>      
>>          ethernet-phy@3 {
>>              compatible = "ethernet-phy-id004d.d180";
>>              reg = <3>;
>>              clocks = <qca8k_nsscc NSS_CC_GEPHY3_SYS_CLK>,
>>              clock-names = <"gephy_sys">;
>>              reset-names = "gephy_sys", "gephy_soft";
>>          };
>> };
> 
> 	Andrew

Re: [PATCH v8 14/14] dt-bindings: net: ar803x: add qca8084 PHY properties

[Andrew Lunn]

> Thanks Andrew for the proposal.
> For the pure PHY chip qca8084, there is no driver to parse the package
> level device tree node for common clocks and resets.

So you still have not look at the work Christian is doing. You must
work together with Christian. This driver is not going to be accepted
unless you do.

> > >          ethernet-phy@0 {
> > >              compatible = "ethernet-phy-id004d.d180";
> > >              reg = <0>;
> > >              clocks = <qca8k_nsscc NSS_CC_GEPHY0_SYS_CLK>,
> > >              clock-names = <"gephy_sys">;
> > >              resets = <&qca8k_nsscc NSS_CC_GEPHY0_SYS_ARES>,
> > >                       <&qca8k_nsscc NSS_CC_GEPHY0_ARES>;
> > >              reset-names = "gephy_sys", "gephy_soft";

Which of these properties exist for the Pure PHY device? Which exist
for the integrated switch? And by that, i mean which are actual pins
on the PHY device? We need the device tree binding to list which
properties are required for each use case.

	   Andrew

Re: [PATCH v8 14/14] dt-bindings: net: ar803x: add qca8084 PHY properties

[Jie Luo]

On 12/18/2023 5:34 PM, Andrew Lunn wrote:
>> Thanks Andrew for the proposal.
>> For the pure PHY chip qca8084, there is no driver to parse the package
>> level device tree node for common clocks and resets.
> 
> So you still have not look at the work Christian is doing. You must
> work together with Christian. This driver is not going to be accepted
> unless you do.
OK, Andrew, i am looking at Christian's patches on at803x.c, and i will
update qca8084 patches based on Christian's patch set.

> 
>>>>           ethernet-phy@0 {
>>>>               compatible = "ethernet-phy-id004d.d180";
>>>>               reg = <0>;
>>>>               clocks = <qca8k_nsscc NSS_CC_GEPHY0_SYS_CLK>,
>>>>               clock-names = <"gephy_sys">;
>>>>               resets = <&qca8k_nsscc NSS_CC_GEPHY0_SYS_ARES>,
>>>>                        <&qca8k_nsscc NSS_CC_GEPHY0_ARES>;
>>>>               reset-names = "gephy_sys", "gephy_soft";
> 
> Which of these properties exist for the Pure PHY device? Which exist
> for the integrated switch? And by that, i mean which are actual pins
> on the PHY device? We need the device tree binding to list which
> properties are required for each use case.
> 
> 	   Andrew

Hi Andrew,
For the clocks and resets listed here, only the clock "mdio_master_ahb"
is dedicated in qca8386, others are needed on the both chips qca8386
and qca8084.

Here is the DTS example for the clocks and resets working on the
devices, from the example below, we can get the dedicated clocks
and resets for each MDIO device and package level device.

The DTS properties in the "qcom,phy-common" should be initialized by
the first PHY probe function, and only being initialized one time.

phy0: ethernet-phy@0 { 

         compatible = "ethernet-phy-id004d.d180"; 

         reg = <1>;

         /* Package level configs, applicable on qca8386 and qca8081. */ 

         phy-common-config { 

                 qcom,phy-addr-fixup = <1 2 3 4 5 6 7>; 

                 qcom,phy-work-mode = <2>; 

                 clocks = <&qca8k_nsscc NSS_CC_APB_BRIDGE_CLK>, 

                        <&qca8k_nsscc NSS_CC_AHB_CLK>, 

                        <&qca8k_nsscc NSS_CC_SEC_CTRL_AHB_CLK>, 

                        <&qca8k_nsscc NSS_CC_TLMM_CLK>, 

                        <&qca8k_nsscc NSS_CC_TLMM_AHB_CLK>, 

                        <&qca8k_nsscc NSS_CC_CNOC_AHB_CLK>, 

                        <&qca8k_nsscc NSS_CC_MDIO_AHB_CLK>; 

 

                 clock-names = "apb_bridge", 

                         "ahb", 

                         "sec_ctrl_ahb", 

                         "tlmm", 

                         "tlmm_ahb", 

                         "cnoc_ahb", 

                         "mdio_ahb"; 

 

                 resets = <&qca8k_nsscc NSS_CC_DSP_ARES>; 

                 reset-names = "gephy_dsp"; 

 

                 ethernet-ports { 

                         /* clocks and resets for pcs0. */ 

                         pcs0 {
                                 clocks = <&qca8k_nsscc 
NSS_CC_SRDS0_SYS_CLK>;
                                 clock-names = "srds0_sys"; 

                                 resets = <&qca8k_nsscc 
NSS_CC_SRDS0_SYS_ARES>;
                                 reset-names = "srds0_sys"; 

                         };

                         /* clocks and resets for pcs1. */ 

                         pcs1 {
                                 clocks = <&qca8k_nsscc 
NSS_CC_SRDS1_SYS_CLK>;
                                 clock-names = "srds1_sys"; 

                                 resets = <&qca8k_nsscc 
NSS_CC_SRDS1_SYS_ARES>;
                                 reset-names = "srds1_sys"; 

                         };

			/* clocks and resets for first phy */
                         phy0 {
                                 clocks = <&qca8k_nsscc 
NSS_CC_GEPHY0_SYS_CLK>;
                                 clock-names = "gephy0_sys"; 

                                 resets = <&qca8k_nsscc 
NSS_CC_GEPHY0_SYS_ARES>,
                                        <&qca8k_nsscc 
NSS_CC_GEPHY0_ARES>;
                                 reset-names = "gephy0_sys", 

                                         "gephy0_soft"; 

                         }; 


			/* clocks and resets for second phy */ 

                         phy1 {
                                 clocks = <&qca8k_nsscc 
NSS_CC_GEPHY1_SYS_CLK>;
                                 clock-names = "gephy1_sys"; 

                                 resets = <&qca8k_nsscc 
NSS_CC_GEPHY1_SYS_ARES>,
                                        <&qca8k_nsscc 
NSS_CC_GEPHY1_ARES>;
                                 reset-names = "gephy1_sys", 

                                         "gephy1_soft"; 

                         }; 


			/* clocks and resets for third phy */ 

                         phy2 {
                                 clocks = <&qca8k_nsscc 
NSS_CC_GEPHY2_SYS_CLK>;
                                 clock-names = "gephy2_sys"; 

                                 resets = <&qca8k_nsscc 
NSS_CC_GEPHY2_SYS_ARES>,
                                        <&qca8k_nsscc 
NSS_CC_GEPHY2_ARES>;
                                 reset-names = "gephy2_sys", 

                                         "gephy2_soft"; 

                         }; 


			/* clocks and resets for fourth phy */ 

                         phy3 {
                                 clocks = <&qca8k_nsscc 
NSS_CC_GEPHY3_SYS_CLK>;
                                 clock-names = "gephy3_sys"; 

                                 resets = <&qca8k_nsscc 
NSS_CC_GEPHY3_SYS_ARES>,
                                        <&qca8k_nsscc 
NSS_CC_GEPHY3_ARES>;
                                 reset-names = "gephy3_sys", 

                                         "gephy3_soft"; 

                         }; 

 

                 }; 

         };

phy1: ethernet-phy@1 { 

         compatible = "ethernet-phy-id004d.d180"; 

         reg = <2>; 

};
 

phy2: ethernet-phy@2 { 

         compatible = "ethernet-phy-id004d.d180"; 

         reg = <3>; 

}; 

 

phy3: ethernet-phy@3 { 

         compatible = "ethernet-phy-id004d.d180"; 

         reg = <4>; 

};

Re: [PATCH v8 14/14] dt-bindings: net: ar803x: add qca8084 PHY properties

[Andrew Lunn]

> +  clocks:
> +    items:
> +      - description: APB bridge clock
> +      - description: AHB clock
> +      - description: Security control clock
> +      - description: TLMM clock
> +      - description: TLMM AHB clock
> +      - description: CNOC AHB clock
> +      - description: MDIO AHB clock
> +      - description: MDIO master AHB clock
> +      - description: PCS0 system clock
> +      - description: PCS1 system clock
> +      - description: EPHY0 system clock
> +      - description: EPHY1 system clock
> +      - description: EPHY2 system clock
> +      - description: EPHY3 system clock

What exactly are you describing here? A PHY, or a PHY package?

The ethernet-phy.yaml describes a PHY. So does each of your 4 PHYs
have 14 clocks? The PHY package as a whole has 14*4 clocks?

This seems unlikely. You have some clocks used by the package as a
whole, and you have some clocks used by one specific PHY within the
package. So you need a hierarchical description of the hardware in DT,
to match the actual hierarchical of the hardware.

This is exactly what Christian has been working on, and you have
persistently ignored what he is doing. You need to work with him.
Nothing is going to be merged until you and Christian have one
consistent design for the two PHYs you are working on.


    Andrew

---
pw-bot: cr

Re: [PATCH v8 14/14] dt-bindings: net: ar803x: add qca8084 PHY properties

[Jie Luo]

On 12/15/2023 8:12 PM, Andrew Lunn wrote:
>> +  clocks:
>> +    items:
>> +      - description: APB bridge clock
>> +      - description: AHB clock
>> +      - description: Security control clock
>> +      - description: TLMM clock
>> +      - description: TLMM AHB clock
>> +      - description: CNOC AHB clock
>> +      - description: MDIO AHB clock
>> +      - description: MDIO master AHB clock
>> +      - description: PCS0 system clock
>> +      - description: PCS1 system clock
>> +      - description: EPHY0 system clock
>> +      - description: EPHY1 system clock
>> +      - description: EPHY2 system clock
>> +      - description: EPHY3 system clock
> 
> What exactly are you describing here? A PHY, or a PHY package?
> 
> The ethernet-phy.yaml describes a PHY. So does each of your 4 PHYs
> have 14 clocks? The PHY package as a whole has 14*4 clocks?
> 
> This seems unlikely. You have some clocks used by the package as a
> whole, and you have some clocks used by one specific PHY within the
> package. So you need a hierarchical description of the hardware in DT,
> to match the actual hierarchical of the hardware.
> 
> This is exactly what Christian has been working on, and you have
> persistently ignored what he is doing. You need to work with him.
> Nothing is going to be merged until you and Christian have one
> consistent design for the two PHYs you are working on.
> 
> 
>      Andrew
> 
> ---
> pw-bot: cr

Hi Andrew,
These clocks are for the whole PHY package including quad PHYs, since
these clocks & resets need to be initialized at one point, i put it
the previous MDIO driver code, these clocks & resets are configured
after GPIO hardware reset, after these clocks and resets sequences
configured, each PHY capabilities can be acquired correctly in the PHY
probe function.

Sorry for missing Christian's patches, i will look his patches and
update qca8084 PHY driver correspondingly.

Re: [PATCH v8 14/14] dt-bindings: net: ar803x: add qca8084 PHY properties

[Andrew Lunn]

On Fri, Dec 15, 2023 at 08:33:00PM +0800, Jie Luo wrote:
> 
> 
> On 12/15/2023 8:12 PM, Andrew Lunn wrote:
> > > +  clocks:
> > > +    items:
> > > +      - description: APB bridge clock
> > > +      - description: AHB clock
> > > +      - description: Security control clock
> > > +      - description: TLMM clock
> > > +      - description: TLMM AHB clock
> > > +      - description: CNOC AHB clock
> > > +      - description: MDIO AHB clock
> > > +      - description: MDIO master AHB clock
> > > +      - description: PCS0 system clock
> > > +      - description: PCS1 system clock
> > > +      - description: EPHY0 system clock
> > > +      - description: EPHY1 system clock
> > > +      - description: EPHY2 system clock
> > > +      - description: EPHY3 system clock

> Hi Andrew,
> These clocks are for the whole PHY package including quad PHYs, since
> these clocks & resets need to be initialized at one point, i put it
> the previous MDIO driver code, these clocks & resets are configured
> after GPIO hardware reset, after these clocks and resets sequences
> configured, each PHY capabilities can be acquired correctly in the PHY
> probe function.

I really expect the hardware is hierarchical. Its unlikely that EPHY0
is connected to all four PHYs in the package. Its specific to one
PHY. So it should be in the DT properties for that one specific PHY. I
expect the resets are the same. It seems there is a soft and hard
reset per PHY, so i would expect these to be in the node for one PHY.

Do the two PCS instances take up two MDIO address? They can be
considered devices on the bus, so could have a DT node, and hence you
can place the PCS clocks on that node?

What exactly do the two MDIO clocks do? I assume these are not for the
MDIO bus master, but the MDIO slave block within the PHY package?
There is one MDIO slave block shared by the four PHYs. So these are
package properties and should be in the package node in DT.

Look at all the other clocks and decide, are they package clocks, or
specific to one block on the MDIO bus? Do the properties go in the
package node, or the per PHY node?

	Andrew

Re: [PATCH v8 14/14] dt-bindings: net: ar803x: add qca8084 PHY properties

[Jie Luo]

On 12/15/2023 9:31 PM, Andrew Lunn wrote:
> On Fri, Dec 15, 2023 at 08:33:00PM +0800, Jie Luo wrote:
>>
>>
>> On 12/15/2023 8:12 PM, Andrew Lunn wrote:
>>>> +  clocks:
>>>> +    items:
>>>> +      - description: APB bridge clock
>>>> +      - description: AHB clock
>>>> +      - description: Security control clock
>>>> +      - description: TLMM clock
>>>> +      - description: TLMM AHB clock
>>>> +      - description: CNOC AHB clock
>>>> +      - description: MDIO AHB clock
>>>> +      - description: MDIO master AHB clock
>>>> +      - description: PCS0 system clock
>>>> +      - description: PCS1 system clock
>>>> +      - description: EPHY0 system clock
>>>> +      - description: EPHY1 system clock
>>>> +      - description: EPHY2 system clock
>>>> +      - description: EPHY3 system clock
> 
>> Hi Andrew,
>> These clocks are for the whole PHY package including quad PHYs, since
>> these clocks & resets need to be initialized at one point, i put it
>> the previous MDIO driver code, these clocks & resets are configured
>> after GPIO hardware reset, after these clocks and resets sequences
>> configured, each PHY capabilities can be acquired correctly in the PHY
>> probe function.
> 
> I really expect the hardware is hierarchical. Its unlikely that EPHY0
> is connected to all four PHYs in the package. Its specific to one
> PHY. So it should be in the DT properties for that one specific PHY. I
> expect the resets are the same. It seems there is a soft and hard
> reset per PHY, so i would expect these to be in the node for one PHY.be 

Hi Andrew,
i understand your point, i tried putting the related clocks and resets
into each device node per PHY, which does not work, since these clocks
ans resets need to be initialized at one function pointer after GPIO 
reset on the qca8084 package.

Sorry for these confusions here, let me explain the qca8084 chip more
detail here, and i will also update this info on the cover letter.

The following is the chip package, the chip can work on the switch mode
like the existed upstream code qca8k, where PHY1-PHY4 is connected with
MAC1-MAC4 directly; The chip can also work on the PHY mode, where PHY1-
PHY4 is connected with PCS1 by 10g-qxgmii; Either switch mode or PHY 
mode, the PHY4 is optionally connected with PCS0 by SGMII, PCS0 and PCS1
are connected with the SoC(IPQ platform) PCSes.
+----------------------------------------------+
|          PCS1           PCS0                 |
|                                              |
|          MAC0           MAC5                 |
|                                              |
|                                              |
|                                              |
|      MAC1      MAC2      MAC3      MAC4      |
|                                              |
|      PHY1      PHY2      PHY3      PHY4      |
+----------------------------------------------+

After the GPIO reset on this package from the MDIO bus level, the chip
is in the cold hardware reset status, the clocks and resets mentioned
here need to be initialized before the capabilities of PHY can be
acquired correctly in the PHY probe function, that is why i put these
clocks and resets in the first PHY device tree node.
When the chip works on the PHY mode, the MAC listed in the block is
not used, the MAC is only used in the switch mode.

i know we can also put the related clocks and resets into the each
PHY and PCS device node to make the hardware hierarchical, then i can
read all initial clocks and resets in the first PHY probe function,
but there are still some package level clocks such as APB/AHB/CNOC
clocks needed for these initial configuration, so i put these clocks
and resets at one PHY device node and only needed to be called by one
time.

 From the clocks and resets name, EPHY0 is specific to the first PHY1,
which is providing clock and reset on the first PHY1, other EPHY clock
and reset is same for the corresponding PHY.

> 
> Do the two PCS instances take up two MDIO address? They can be
> considered devices on the bus, so could have a DT node, and hence you
> can place the PCS clocks on that node?

Yes, two PCS instances take up two MDIO device with different MDIO
address, which also have the DT nodes as the child node of MDIO bus node
there is also some specific clocks and resets defined in the PCS DT
node for the PCS driver.

> 
> What exactly do the two MDIO clocks do? I assume these are not for the
> MDIO bus master, but the MDIO slave block within the PHY package?
> There is one MDIO slave block shared by the four PHYs. So these are
> package properties and should be in the package node in DT.

The MDIO clocks are the qca8084 package level clock, since there
are other modules such as GCC and security control located in the
qca8084 chip, these module register is also accessed by the MDIO bus,
the MDIO AHB clock is this modules access.
there is also a MDIO master for the back pressure function in qca8084
chip, the MDIO master AHB clock is for this function, actually this
function is for the switch mode, but it is the package level clock,
so i put it together here.

No, these clocks are not for the IPQ4019 SoC MDIO bus master. Four PHYs 
are the independent MDIO slave devices.

For the switch mode, we can define these package level clocks and resets
in the DSA device node.

But for the PHY mode, All 4 PHYs is connected with PCS1 by 10g-qxgmii,
there is no package level device tree node defined.

> 
> Look at all the other clocks and decide, are they package clocks, or
> specific to one block on the MDIO bus? Do the properties go in the
> package node, or the per PHY node?
> 
> 	Andrew

The clocks and resets with prefix PCS or EPHY is per PHY/PCS node, other
clocks and resets are the qca8084 package level node.

Re: [PATCH v8 14/14] dt-bindings: net: ar803x: add qca8084 PHY properties

[Andrew Lunn]

> The following is the chip package, the chip can work on the switch mode
> like the existed upstream code qca8k, where PHY1-PHY4 is connected with
> MAC1-MAC4 directly;

Ah, that is new information, and has a big effect on the design.

Can the qca8K driver be extended to drive this hardware in switch
mode?

	Andrew

Re: [PATCH v8 14/14] dt-bindings: net: ar803x: add qca8084 PHY properties

[Jie Luo]

On 12/16/2023 6:21 PM, Andrew Lunn wrote:
>> The following is the chip package, the chip can work on the switch mode
>> like the existed upstream code qca8k, where PHY1-PHY4 is connected with
>> MAC1-MAC4 directly;
> 
> Ah, that is new information, and has a big effect on the design.
> 
> Can the qca8K driver be extended to drive this hardware in switch
> mode?
> 
> 	Andrew

Yes, Andrew, the qca8k driver can be extended to drive this hardware in
the switch mode, we will push it to the upstream in the near future.

Re: [PATCH v8 14/14] dt-bindings: net: ar803x: add qca8084 PHY properties

[Russell King (Oracle)]

On Sat, Dec 16, 2023 at 11:21:53AM +0100, Andrew Lunn wrote:
> > The following is the chip package, the chip can work on the switch mode
> > like the existed upstream code qca8k, where PHY1-PHY4 is connected with
> > MAC1-MAC4 directly;
> 
> Ah, that is new information, and has a big effect on the design.

This QCA8084 that's being proposed in these patches is not a PHY in
itself, but is a SoC. I came across this:

 https://www.rt-rk.com/android-tv-solution-tv-in-smartphone-pantsstb-based-on-qualcomm-soc-design/

It's sounding like what we have here is some PHY IP that is integrated
into a larger SoC, and the larger SoC needs to be configured so the
PHY IP can work correctly.

Given that this package of four PHYs seems to be rather unique, I think
we need Jie Luo to provide sufficient information so we can understand:

1) this package of four PHYs itself
2) how this package is integrated into the SoC

Specifically, what resets and clocks are controlled from within the
package's register space, which are external to the package
register space (and thus are provided by other IPs in the SoC).

As I've said previously, the lack of DT example doesn't help to further
our understanding. The lack of details of what the package encompases
also doesn't help us understand the hardware.

Unless we can gain that understanding, I feel that Jie Luo's patches
are effectively unreviewable and can't be accepted into mainline.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH v8 14/14] dt-bindings: net: ar803x: add qca8084 PHY properties

[Jie Luo]

On 12/16/2023 9:51 PM, Russell King (Oracle) wrote:
> On Sat, Dec 16, 2023 at 11:21:53AM +0100, Andrew Lunn wrote:
>>> The following is the chip package, the chip can work on the switch mode
>>> like the existed upstream code qca8k, where PHY1-PHY4 is connected with
>>> MAC1-MAC4 directly;
>>
>> Ah, that is new information, and has a big effect on the design.
> 
> This QCA8084 that's being proposed in these patches is not a PHY in
> itself, but is a SoC. I came across this:
> 
>   https://www.rt-rk.com/android-tv-solution-tv-in-smartphone-pantsstb-based-on-qualcomm-soc-design/

The chip mentioned in the link you mentioned is SoC, which is not the
chip that the qca8084 driver work for.

qca8084/qca8386 is just the Ethernet CHIP, not SoC, for the switch mode 
qca8386, which is most like qca8337 the dsa drive qca8k.c is already in 
upstream.

i qca8084 chip package includes 4 PHYs, 2 PCSs and the common chip level
modules such as GCC and security control modules, all these modules are
located in the qca8084 chip package, since qca8084 works on PHY mode, so
the MACs are not used.

qca8084 is connected with the SoC CHIP such as IPQ platform by PCS1
working on 10g-qxgmii mode and the fourth PHY can also optionally
be connected with the IPQ SoC PCS by sgmii mode, there is no more
interface on qca8084 to connect the external chips.

> It's sounding like what we have here is some PHY IP that is integrated
> into a larger SoC, and the larger SoC needs to be configured so the
> PHY IP can work correctly.

qca8084 is not a SoC, it is the Ethernet chip, in this qca8084 package,
there are GCC that is driving the PHY working on the various link speed.
that is the reason we need to do these package level common clocks and
resets initialization before probing PHY correctly.

> 
> Given that this package of four PHYs seems to be rather unique, I think
> we need Jie Luo to provide sufficient information so we can understand:
> 
> 1) this package of four PHYs itself

Yes, this chip package for all 4 PHYs itself, also including the PCSes
and common package level modules such as GCC.

> 2) how this package is integrated into the SoC

the qca8084 is connected with SoC by PCSes.

> 
> Specifically, what resets and clocks are controlled from within the
> package's register space, which are external to the package
> register space (and thus are provided by other IPs in the SoC).

All clocks and resets mentioned for qca8084 drive including package
level and PCS & PHY clocks and resets from the qca8084 internal GCC
modules register space,

> 
> As I've said previously, the lack of DT example doesn't help to further
> our understanding. The lack of details of what the package encompases
> also doesn't help us understand the hardware.

Indeed, i will add the qca8084 DT example in the next patch set.
BTW, i also replied your earlier comments by providing the DTS defined
for the current qca8084 drive code.

hope you can have a better understanding with the provided DTS code in
earlier reply of this email thread.
> 
> Unless we can gain that understanding, I feel that Jie Luo's patches
> are effectively unreviewable and can't be accepted into mainline.
> 

Re: [PATCH v8 14/14] dt-bindings: net: ar803x: add qca8084 PHY properties

[Christian Marangi]

On Sat, Dec 16, 2023 at 10:41:28PM +0800, Jie Luo wrote:
> 
> 
> On 12/16/2023 9:51 PM, Russell King (Oracle) wrote:
> > On Sat, Dec 16, 2023 at 11:21:53AM +0100, Andrew Lunn wrote:
> > > > The following is the chip package, the chip can work on the switch mode
> > > > like the existed upstream code qca8k, where PHY1-PHY4 is connected with
> > > > MAC1-MAC4 directly;
> > > 
> > > Ah, that is new information, and has a big effect on the design.
> > 
> > This QCA8084 that's being proposed in these patches is not a PHY in
> > itself, but is a SoC. I came across this:
> > 
> >   https://www.rt-rk.com/android-tv-solution-tv-in-smartphone-pantsstb-based-on-qualcomm-soc-design/
> 
> The chip mentioned in the link you mentioned is SoC, which is not the
> chip that the qca8084 driver work for.
> 
> qca8084/qca8386 is just the Ethernet CHIP, not SoC, for the switch mode
> qca8386, which is most like qca8337 the dsa drive qca8k.c is already in
> upstream.

Hi,
sorry for stepping in. I guess here there is a massive confusion with
naming and using qca8k.

Since it seems the same name is used for PHY and for Switch stuff, I
would add PHY and MAC prefix when referring to qca8064.

With the previous message I was a bit confused by the use of qca8k and
didn't know you were actually referring to the DSA driver.
Interesting... this is for the upcoming WiFi 7 platoform right? (ipq9574)

All these discussion comes for the problem of using this PHY as an
integrated PHY in the qca8386 switch and trying to select the mode in
the PHY driver.

Considering you would use the same logic of the current DSA qca8k driver
with the integrated PHY, the problem doesn't apply or a different
implementation should be used (and actually handled later when the
actual DSA code will come)

I would expect in the integrated mode, the switch to handle the PHY (as
it's done by qca8337) with the PHY defined in the switch node and qca8k
handling the PHY registration. With the following implementation flags
can be passed and PHY can be configured to integrated mode. (or virtual
PHY ID can be used for such scope with dedicated functions in the PHY
driver)

With this in mind the entire integrated problem can put on hold and
dropped to be later reimplemented when it's time. (assuming that all the
prereq are already here and the very same implementation of qca8k will
be used)

Anyway, I'm more or less the maintainer of the qca8k.c DSA driver and I
would be more than happy to help you guys internally or externally on
pushing and make this proceed further. (again assuming this is ipq9574
stuff, it would be good to finally have proper DSA driver instead of
leaving the thing unusable as it's the current situation with ipq8074)

> 
> i qca8084 chip package includes 4 PHYs, 2 PCSs and the common chip level
> modules such as GCC and security control modules, all these modules are
> located in the qca8084 chip package, since qca8084 works on PHY mode, so
> the MACs are not used.
> 
> qca8084 is connected with the SoC CHIP such as IPQ platform by PCS1
> working on 10g-qxgmii mode and the fourth PHY can also optionally
> be connected with the IPQ SoC PCS by sgmii mode, there is no more
> interface on qca8084 to connect the external chips.
> 
> > It's sounding like what we have here is some PHY IP that is integrated
> > into a larger SoC, and the larger SoC needs to be configured so the
> > PHY IP can work correctly.
> 
> qca8084 is not a SoC, it is the Ethernet chip, in this qca8084 package,
> there are GCC that is driving the PHY working on the various link speed.
> that is the reason we need to do these package level common clocks and
> resets initialization before probing PHY correctly.
> 
> > 
> > Given that this package of four PHYs seems to be rather unique, I think
> > we need Jie Luo to provide sufficient information so we can understand:
> > 
> > 1) this package of four PHYs itself
> 
> Yes, this chip package for all 4 PHYs itself, also including the PCSes
> and common package level modules such as GCC.
> 
> > 2) how this package is integrated into the SoC
> 
> the qca8084 is connected with SoC by PCSes.
> 
> > 
> > Specifically, what resets and clocks are controlled from within the
> > package's register space, which are external to the package
> > register space (and thus are provided by other IPs in the SoC).
> 
> All clocks and resets mentioned for qca8084 drive including package
> level and PCS & PHY clocks and resets from the qca8084 internal GCC
> modules register space,
> 
> > 
> > As I've said previously, the lack of DT example doesn't help to further
> > our understanding. The lack of details of what the package encompases
> > also doesn't help us understand the hardware.
> 
> Indeed, i will add the qca8084 DT example in the next patch set.
> BTW, i also replied your earlier comments by providing the DTS defined
> for the current qca8084 drive code.
> 
> hope you can have a better understanding with the provided DTS code in
> earlier reply of this email thread.
> > 
> > Unless we can gain that understanding, I feel that Jie Luo's patches
> > are effectively unreviewable and can't be accepted into mainline.
> > 

-- 
	Ansuel

Re: [PATCH v8 14/14] dt-bindings: net: ar803x: add qca8084 PHY properties

[Jie Luo]

On 12/17/2023 12:01 AM, Christian Marangi wrote:
> On Sat, Dec 16, 2023 at 10:41:28PM +0800, Jie Luo wrote:
>>
>>
>> On 12/16/2023 9:51 PM, Russell King (Oracle) wrote:
>>> On Sat, Dec 16, 2023 at 11:21:53AM +0100, Andrew Lunn wrote:
>>>>> The following is the chip package, the chip can work on the switch mode
>>>>> like the existed upstream code qca8k, where PHY1-PHY4 is connected with
>>>>> MAC1-MAC4 directly;
>>>>
>>>> Ah, that is new information, and has a big effect on the design.
>>>
>>> This QCA8084 that's being proposed in these patches is not a PHY in
>>> itself, but is a SoC. I came across this:
>>>
>>>    https://www.rt-rk.com/android-tv-solution-tv-in-smartphone-pantsstb-based-on-qualcomm-soc-design/
>>
>> The chip mentioned in the link you mentioned is SoC, which is not the
>> chip that the qca8084 driver work for.
>>
>> qca8084/qca8386 is just the Ethernet CHIP, not SoC, for the switch mode
>> qca8386, which is most like qca8337 the dsa drive qca8k.c is already in
>> upstream.
> 
> Hi,
> sorry for stepping in. I guess here there is a massive confusion with
> naming and using qca8k.
> 
> Since it seems the same name is used for PHY and for Switch stuff, I
> would add PHY and MAC prefix when referring to qca8064.

Hi Christian,
Welcome to join the discussion.

Sorry for the confusion, since the switch and PHY are the different
chips, the switch chip is named as qca8386, the pure PHY chip is named
as qca8084, the pure PHY chip qca8084 is connected with the SoC by PCS
working on 10g-qxgmii, the switch chip qca8386 is connected with SoC by
PCS working on sgmii.

> 
> With the previous message I was a bit confused by the use of qca8k and
> didn't know you were actually referring to the DSA driver.
> Interesting... this is for the upcoming WiFi 7 platoform right? (ipq9574)

For qca8386, it is the switch chip and leverages the qca8k DSA driver,
it is connected on the ipq5332 platform currently.

But for qca8084, it is the pure PHY chip, which works on 10g-qxgmii for
quad-phy, which is connected on the ipq9574.

> 
> All these discussion comes for the problem of using this PHY as an
> integrated PHY in the qca8386 switch and trying to select the mode in
> the PHY driver.

Yes, for qca8386, the PHY is integrated to switch, since same PHY needs
to work with qca8386 and qca8084, so the work mode needs to be
configured to distinguish the different work mode as the patch
<[PATCH v8 13/14] net: phy: at803x: configure qca8084 work mode>.

> 
> Considering you would use the same logic of the current DSA qca8k driver
> with the integrated PHY, the problem doesn't apply or a different
> implementation should be used (and actually handled later when the
> actual DSA code will come)

qca8386 has some differences from qca8337(qca8k dsa driver), qca8337
integrated PHY can work by the general PHY, but the PHY in qca8386 needs
the extra PHY driver, and the GCC clocks introduced.

> 
> I would expect in the integrated mode, the switch to handle the PHY (as
> it's done by qca8337) with the PHY defined in the switch node and qca8k
> handling the PHY registration. With the following implementation flags
> can be passed and PHY can be configured to integrated mode. (or virtual
> PHY ID can be used for such scope with dedicated functions in the PHY
> driver)
> 
Since there is the pure PHY chip qca8084 using the same PHY, which does
not enable the DSA driver.

So the PHY driver should be standalone, and the common clocks and resets
is better to put in the PHY driver, since these are the common configs
for the qca8386 and qca8084.

> With this in mind the entire integrated problem can put on hold and
> dropped to be later reimplemented when it's time. (assuming that all the
> prereq are already here and the very same implementation of qca8k will
> be used)

For the qca8386, it is true very same implementation of qca8k, besides
the 2.5G capability and GCC clock involved.

> 
> Anyway, I'm more or less the maintainer of the qca8k.c DSA driver and I
> would be more than happy to help you guys internally or externally on
> pushing and make this proceed further. (again assuming this is ipq9574
> stuff, it would be good to finally have proper DSA driver instead of
> leaving the thing unusable as it's the current situation with ipq8074)

Yes, the pure PHY chip qca8084 is connected on ipq9574 platform,
and the switch chip qca8386 is connected on ipq5332 platform.

Many thanks Christian again, will add you when pushing the qca8386 DSA
patches for upstream review.

> 
>>
>> i qca8084 chip package includes 4 PHYs, 2 PCSs and the common chip level
>> modules such as GCC and security control modules, all these modules are
>> located in the qca8084 chip package, since qca8084 works on PHY mode, so
>> the MACs are not used.
>>
>> qca8084 is connected with the SoC CHIP such as IPQ platform by PCS1
>> working on 10g-qxgmii mode and the fourth PHY can also optionally
>> be connected with the IPQ SoC PCS by sgmii mode, there is no more
>> interface on qca8084 to connect the external chips.
>>
>>> It's sounding like what we have here is some PHY IP that is integrated
>>> into a larger SoC, and the larger SoC needs to be configured so the
>>> PHY IP can work correctly.
>>
>> qca8084 is not a SoC, it is the Ethernet chip, in this qca8084 package,
>> there are GCC that is driving the PHY working on the various link speed.
>> that is the reason we need to do these package level common clocks and
>> resets initialization before probing PHY correctly.
>>
>>>
>>> Given that this package of four PHYs seems to be rather unique, I think
>>> we need Jie Luo to provide sufficient information so we can understand:
>>>
>>> 1) this package of four PHYs itself
>>
>> Yes, this chip package for all 4 PHYs itself, also including the PCSes
>> and common package level modules such as GCC.
>>
>>> 2) how this package is integrated into the SoC
>>
>> the qca8084 is connected with SoC by PCSes.
>>
>>>
>>> Specifically, what resets and clocks are controlled from within the
>>> package's register space, which are external to the package
>>> register space (and thus are provided by other IPs in the SoC).
>>
>> All clocks and resets mentioned for qca8084 drive including package
>> level and PCS & PHY clocks and resets from the qca8084 internal GCC
>> modules register space,
>>
>>>
>>> As I've said previously, the lack of DT example doesn't help to further
>>> our understanding. The lack of details of what the package encompases
>>> also doesn't help us understand the hardware.
>>
>> Indeed, i will add the qca8084 DT example in the next patch set.
>> BTW, i also replied your earlier comments by providing the DTS defined
>> for the current qca8084 drive code.
>>
>> hope you can have a better understanding with the provided DTS code in
>> earlier reply of this email thread.
>>>
>>> Unless we can gain that understanding, I feel that Jie Luo's patches
>>> are effectively unreviewable and can't be accepted into mainline.
>>>
> 

Re: [PATCH v8 14/14] dt-bindings: net: ar803x: add qca8084 PHY properties

[Russell King (Oracle)]

On Sat, Dec 16, 2023 at 10:41:28PM +0800, Jie Luo wrote:
> 
> 
> On 12/16/2023 9:51 PM, Russell King (Oracle) wrote:
> > On Sat, Dec 16, 2023 at 11:21:53AM +0100, Andrew Lunn wrote:
> > > > The following is the chip package, the chip can work on the switch mode
> > > > like the existed upstream code qca8k, where PHY1-PHY4 is connected with
> > > > MAC1-MAC4 directly;
> > > 
> > > Ah, that is new information, and has a big effect on the design.
> > 
> > This QCA8084 that's being proposed in these patches is not a PHY in
> > itself, but is a SoC. I came across this:
> > 
> >   https://www.rt-rk.com/android-tv-solution-tv-in-smartphone-pantsstb-based-on-qualcomm-soc-design/
> 
> The chip mentioned in the link you mentioned is SoC, which is not the
> chip that the qca8084 driver work for.

So there's two chips called QCA8084 both produced by Qualcomm? I find
that hard to believe.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH v8 14/14] dt-bindings: net: ar803x: add qca8084 PHY properties

[Jie Luo]

On 12/17/2023 12:09 AM, Russell King (Oracle) wrote:
> On Sat, Dec 16, 2023 at 10:41:28PM +0800, Jie Luo wrote:
>>
>>
>> On 12/16/2023 9:51 PM, Russell King (Oracle) wrote:
>>> On Sat, Dec 16, 2023 at 11:21:53AM +0100, Andrew Lunn wrote:
>>>>> The following is the chip package, the chip can work on the switch mode
>>>>> like the existed upstream code qca8k, where PHY1-PHY4 is connected with
>>>>> MAC1-MAC4 directly;
>>>>
>>>> Ah, that is new information, and has a big effect on the design.
>>>
>>> This QCA8084 that's being proposed in these patches is not a PHY in
>>> itself, but is a SoC. I came across this:
>>>
>>>    https://www.rt-rk.com/android-tv-solution-tv-in-smartphone-pantsstb-based-on-qualcomm-soc-design/
>>
>> The chip mentioned in the link you mentioned is SoC, which is not the
>> chip that the qca8084 driver work for.
> 
> So there's two chips called QCA8084 both produced by Qualcomm? I find
> that hard to believe.
> 

The SoC mentioned in the link you provided is the APQ8084 that is 
introduced in the link below:
https://www.qualcomm.com/products/mobile/snapdragon/smartphones/snapdragon-8-series-mobile-platforms/snapdragon-processors-805

https://hwbot.org/hardware/processor/snapdragon_805_apq8084_pro__(8084_fusion_4.5)_2700mhz/

The driver here is for qca8084, which has the different prefix,
and qca8084 is the Ethernet CHIP like qca8081, but qca8084 is
multiple ports(quad-phy).

Re: [PATCH v8 14/14] dt-bindings: net: ar803x: add qca8084 PHY properties

[Russell King (Oracle)]

On Mon, Dec 18, 2023 at 11:01:03AM +0800, Jie Luo wrote:
> 
> 
> On 12/17/2023 12:09 AM, Russell King (Oracle) wrote:
> > On Sat, Dec 16, 2023 at 10:41:28PM +0800, Jie Luo wrote:
> > > 
> > > 
> > > On 12/16/2023 9:51 PM, Russell King (Oracle) wrote:
> > > > On Sat, Dec 16, 2023 at 11:21:53AM +0100, Andrew Lunn wrote:
> > > > > > The following is the chip package, the chip can work on the switch mode
> > > > > > like the existed upstream code qca8k, where PHY1-PHY4 is connected with
> > > > > > MAC1-MAC4 directly;
> > > > > 
> > > > > Ah, that is new information, and has a big effect on the design.
> > > > 
> > > > This QCA8084 that's being proposed in these patches is not a PHY in
> > > > itself, but is a SoC. I came across this:
> > > > 
> > > >    https://www.rt-rk.com/android-tv-solution-tv-in-smartphone-pantsstb-based-on-qualcomm-soc-design/
> > > 
> > > The chip mentioned in the link you mentioned is SoC, which is not the
> > > chip that the qca8084 driver work for.
> > 
> > So there's two chips called QCA8084 both produced by Qualcomm? I find
> > that hard to believe.
> > 
> 
> The SoC mentioned in the link you provided is the APQ8084 that is introduced
> in the link below:
> https://www.qualcomm.com/products/mobile/snapdragon/smartphones/snapdragon-8-series-mobile-platforms/snapdragon-processors-805

So the one mentioned in the rt-rk article and a load of CVEs is _not_
QCA8084 but is APQ8084. Sounds like a lot of people are getting stuff
wrong - which is hardly surprising as there are people that seem to
_enjoy_ getting the technical details wrong. I haven't worked out if
it's intentional malace, or they're just fundamentally lazy individuals
who just like to screw with other people.

Sigh.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH v8 14/14] dt-bindings: net: ar803x: add qca8084 PHY properties

[Christian Marangi]

On Tue, Jan 02, 2024 at 09:57:48AM +0000, Russell King (Oracle) wrote:
> On Mon, Dec 18, 2023 at 11:01:03AM +0800, Jie Luo wrote:
> > 
> > 
> > On 12/17/2023 12:09 AM, Russell King (Oracle) wrote:
> > > On Sat, Dec 16, 2023 at 10:41:28PM +0800, Jie Luo wrote:
> > > > 
> > > > 
> > > > On 12/16/2023 9:51 PM, Russell King (Oracle) wrote:
> > > > > On Sat, Dec 16, 2023 at 11:21:53AM +0100, Andrew Lunn wrote:
> > > > > > > The following is the chip package, the chip can work on the switch mode
> > > > > > > like the existed upstream code qca8k, where PHY1-PHY4 is connected with
> > > > > > > MAC1-MAC4 directly;
> > > > > > 
> > > > > > Ah, that is new information, and has a big effect on the design.
> > > > > 
> > > > > This QCA8084 that's being proposed in these patches is not a PHY in
> > > > > itself, but is a SoC. I came across this:
> > > > > 
> > > > >    https://www.rt-rk.com/android-tv-solution-tv-in-smartphone-pantsstb-based-on-qualcomm-soc-design/
> > > > 
> > > > The chip mentioned in the link you mentioned is SoC, which is not the
> > > > chip that the qca8084 driver work for.
> > > 
> > > So there's two chips called QCA8084 both produced by Qualcomm? I find
> > > that hard to believe.
> > > 
> > 
> > The SoC mentioned in the link you provided is the APQ8084 that is introduced
> > in the link below:
> > https://www.qualcomm.com/products/mobile/snapdragon/smartphones/snapdragon-8-series-mobile-platforms/snapdragon-processors-805
> 
> So the one mentioned in the rt-rk article and a load of CVEs is _not_
> QCA8084 but is APQ8084. Sounds like a lot of people are getting stuff
> wrong - which is hardly surprising as there are people that seem to
> _enjoy_ getting the technical details wrong. I haven't worked out if
> it's intentional malace, or they're just fundamentally lazy individuals
> who just like to screw with other people.
> 
> Sigh.
>

Hoping to give some clarification with the naming.
- APQ8084 ("Application" SoC for 8084 family)
- IPQ8084 ("Internet" SoC version of APQ8084)
- QCA8084 (Integrated PHYs in the IPQ8084 SoC)

I guess?

Considering QCA8084 is only in in IPQ8084 SoC, the confusion with
referring to it is in the fact that it's all the same thing, and
everything related to APQ is also related to IPQ since they are the same
SoC with minor difference (different DSP, presence of NSS cores)

I can totally see sencente like "The IPQ8084 PHYs..." referencing the
QCA8084 PHY.

(Just to put how the naming is confusing there are PMIC with the
same exact naming)

-- 
	Ansuel

Re: [PATCH v8 14/14] dt-bindings: net: ar803x: add qca8084 PHY properties

[Jie Luo]

On 1/2/2024 6:08 PM, Christian Marangi wrote:
> On Tue, Jan 02, 2024 at 09:57:48AM +0000, Russell King (Oracle) wrote:
>> On Mon, Dec 18, 2023 at 11:01:03AM +0800, Jie Luo wrote:
>>>
>>>
>>> On 12/17/2023 12:09 AM, Russell King (Oracle) wrote:
>>>> On Sat, Dec 16, 2023 at 10:41:28PM +0800, Jie Luo wrote:
>>>>>
>>>>>
>>>>> On 12/16/2023 9:51 PM, Russell King (Oracle) wrote:
>>>>>> On Sat, Dec 16, 2023 at 11:21:53AM +0100, Andrew Lunn wrote:
>>>>>>>> The following is the chip package, the chip can work on the switch mode
>>>>>>>> like the existed upstream code qca8k, where PHY1-PHY4 is connected with
>>>>>>>> MAC1-MAC4 directly;
>>>>>>>
>>>>>>> Ah, that is new information, and has a big effect on the design.
>>>>>>
>>>>>> This QCA8084 that's being proposed in these patches is not a PHY in
>>>>>> itself, but is a SoC. I came across this:
>>>>>>
>>>>>>     https://www.rt-rk.com/android-tv-solution-tv-in-smartphone-pantsstb-based-on-qualcomm-soc-design/
>>>>>
>>>>> The chip mentioned in the link you mentioned is SoC, which is not the
>>>>> chip that the qca8084 driver work for.
>>>>
>>>> So there's two chips called QCA8084 both produced by Qualcomm? I find
>>>> that hard to believe.
>>>>
>>>
>>> The SoC mentioned in the link you provided is the APQ8084 that is introduced
>>> in the link below:
>>> https://www.qualcomm.com/products/mobile/snapdragon/smartphones/snapdragon-8-series-mobile-platforms/snapdragon-processors-805
>>
>> So the one mentioned in the rt-rk article and a load of CVEs is _not_
>> QCA8084 but is APQ8084. Sounds like a lot of people are getting stuff
>> wrong - which is hardly surprising as there are people that seem to
>> _enjoy_ getting the technical details wrong. I haven't worked out if
>> it's intentional malace, or they're just fundamentally lazy individuals
>> who just like to screw with other people.
>>
>> Sigh.
>>
> 
> Hoping to give some clarification with the naming.
> - APQ8084 ("Application" SoC for 8084 family)
> - IPQ8084 ("Internet" SoC version of APQ8084)
> - QCA8084 (Integrated PHYs in the IPQ8084 SoC)
> 
> I guess? >
> Considering QCA8084 is only in in IPQ8084 SoC, the confusion with
> referring to it is in the fact that it's all the same thing, and
> everything related to APQ is also related to IPQ since they are the same
> SoC with minor difference (different DSP, presence of NSS cores)
> 
> I can totally see sencente like "The IPQ8084 PHYs..." referencing the
> QCA8084 PHY.
> 
> (Just to put how the naming is confusing there are PMIC with the
> same exact naming)
> 

There should be NO IPQ8084.
Yes, APQ8084 is the application SoC.
QCA8084 is the pure PHY chip which has quad-phy.

The prefix QCA is the Ethernet device, like qca8081(PHY chip), qca8386(
switch chip) and qca8084(PHY chip).
The prefix IPQ is the internet processor SoC, like ipq5332.

Re: [PATCH v8 14/14] dt-bindings: net: ar803x: add qca8084 PHY properties

[Andrew Lunn]

> Yes, APQ8084 is the application SoC.
> QCA8084 is the pure PHY chip which has quad-phy.

I think everybody agrees these are terrible names, being so close
together but being very different devices.

You have the issues of not giving clear explanations of your
hardware. This is resulting in a lot of wasted tome for everybody. So
please make your explanations very clear. I personally would avoid
using APQ8084 or QCA8084 on there own. Always say the application SoC
APQ8084, or the PHY chip QCA8084, or the switch embedded within the
application processor APQ8084, or the PHYs embedded within the
Application processor etc. This is particularly important when talking
about clocks and resets, since the PHYs embedded within the
application processor are likely to have different clocks and reset
controllers to the PHY chip QCA8084.

	Andrew

Re: [PATCH v8 14/14] dt-bindings: net: ar803x: add qca8084 PHY properties

[Jie Luo]

On 1/3/2024 10:22 PM, Andrew Lunn wrote:
>> Yes, APQ8084 is the application SoC.
>> QCA8084 is the pure PHY chip which has quad-phy.
> 
> I think everybody agrees these are terrible names, being so close
> together but being very different devices.
> 
> You have the issues of not giving clear explanations of your
> hardware. This is resulting in a lot of wasted tome for everybody. S
> please make your explanations very clear. I personally would avoid
> using APQ8084 or QCA8084 on there own. Always say the application SoC
> APQ8084, or the PHY chip QCA8084, or the switch embedded within the
> application processor APQ8084, or the PHYs embedded within the
> Application processor etc. This is particularly important when talking
> about clocks and resets, since the PHYs embedded within the
> application processor are likely to have different clocks and reset
> controllers to the PHY chip QCA8084.
> 
> 	Andrew

Let me explain it more.
APQ8084 is the Snapdragon SoC(for smart phone or other applicaiton)
according to the link below.
https://www.qualcomm.com/products/mobile/snapdragon/smartphones/snapdragon-8-series-mobile-platforms/snapdragon-processors-805.
which has nothing to do with QCA8084 or IPQ SoC we are discussing here.
let's remove out the APQ SoC from the discussion here.

1. For IPQ SoC series, there are only ipq4019, ipq5018, ipq6018,
ipq8074 documented in the current dt-bindings doc qcom,ipq4019-mdio.yaml
and ipq9574, ipq5332 that are being added by the MDIO patch, and one
more ipq8064 whose MDIO driver is mdio-ipq8064.c, on more others.

2. For qca8084(pure PHY chip), which is the quad-phy chip, which is just
    like qca8081 PHY(single port PHY), each port can be linked to maximum
    speed 2.5G.

    For qca8386(switch chip), which includes the same PHY CHIP as qca8084
    (4 physical ports and two CPU ports), qca8386 switch can work with
    the current qca8k.c DSA driver with the supplement patches.

    Both qca8084 and qca8386 includes same network clock controller(let's
    call it NSSCC, since this clock controller is located in the
    Ethernet chip qca8084 and qca8386), they have the same clock initial
    configuration sequence to initialize the Ethernet chip.

   The Ethernet chip qca8084 and qca8386 are only connected with IPQ SoC,
   Currently qca8084 is connected with IPQ SoC by 10G-QXGMII mode.
   the 4 PHYs of qca8386 are connected with the internal MAC of qca8386
   by GMII, the maximum speed is also 2.5G.
   The port4 of qca8084 or qca8386 is optionally be able to connected
   with IPQ SoC by sgmii.

Re: [PATCH v8 14/14] dt-bindings: net: ar803x: add qca8084 PHY properties

[Andrew Lunn]

> 1. For IPQ SoC series, there are only ipq4019, ipq5018, ipq6018,
> ipq8074 documented in the current dt-bindings doc qcom,ipq4019-mdio.yaml
> and ipq9574, ipq5332 that are being added by the MDIO patch, and one
> more ipq8064 whose MDIO driver is mdio-ipq8064.c, on more others.
> 
> 2. For qca8084(pure PHY chip), which is the quad-phy chip, which is just
>    like qca8081 PHY(single port PHY), each port can be linked to maximum
>    speed 2.5G.
> 
>    For qca8386(switch chip), which includes the same PHY CHIP as qca8084
>    (4 physical ports and two CPU ports), qca8386 switch can work with
>    the current qca8k.c DSA driver with the supplement patches.

Is the qca8386 purely a switch plus integrated PHYs? There is no CPU
on it? What is the management path? MDIO?

> 
>    Both qca8084 and qca8386 includes same network clock controller(let's
>    call it NSSCC, since this clock controller is located in the
>    Ethernet chip qca8084 and qca8386), they have the same clock initial
>    configuration sequence to initialize the Ethernet chip.

You said For "qca8084(pure PHY chip)". Here you just called it an
Ethernet chip? To me, and Ethernet chip is a MAC, Intel e1000e etc.
Do you now see how your explanations are confusing. Is it s pure PHY,
or is it an Ethernet chip?

O.K. Since we are getting nowhere at the moment, lets take just the
pure PHY chip, and ignore the rest for the moment.

For any pure PHY, there is generally one clock input, which might be a
crystal, or an actual clock. If you look at other DT bindings for
PHYs, it is only listed if the clock is expected to come from
somewhere else, like a SoC, and it needs to be turned on before the
PHY will work. And generally, a pure PHY has one defined clock
frequency input. If that is true, there is no need to specify the
clock. If multiple clock input frequencies are supported, then you do
need to specify the clock, so its possible to work out what frequency
it is using. How that clock input is then used internally in the PHY
is not described in DT, but the driver can set any dividers, PLLs
needed etc.

So, for the pure PHY chip, what is the pinout? Is there one clock
input? Or 4 clock inputs, one per PHY in the quad package? Typically,
where does this/these clocks come from? Is the frequency fixed by the
design, or are a number of input frequencies supported?

>   The Ethernet chip qca8084 and qca8386 are only connected with IPQ SoC,
>   Currently qca8084 is connected with IPQ SoC by 10G-QXGMII mode.
>   the 4 PHYs of qca8386 are connected with the internal MAC of qca8386
>   by GMII, the maximum speed is also 2.5G.
>   The port4 of qca8084 or qca8386 is optionally be able to connected
>   with IPQ SoC by sgmii.

To some extent, this does not matter. The DT binding and the driver
should not care what the pure PHY is connected to. It has standardised
ports, so in theory it could be connected to any vendors MAC.

Please be very careful with your wording. Because computers
instructions should be unambiguous, it does what it is told, we also
expect computer scientists to be unambiguous. Wording is very
important.

       Andrew

Re: [PATCH v8 14/14] dt-bindings: net: ar803x: add qca8084 PHY properties

[Jie Luo]

On 1/4/2024 9:57 PM, Andrew Lunn wrote:
>> 1. For IPQ SoC series, there are only ipq4019, ipq5018, ipq6018,
>> ipq8074 documented in the current dt-bindings doc qcom,ipq4019-mdio.yaml
>> and ipq9574, ipq5332 that are being added by the MDIO patch, and one
>> more ipq8064 whose MDIO driver is mdio-ipq8064.c, on more others.
>>
>> 2. For qca8084(pure PHY chip), which is the quad-phy chip, which is just
>>     like qca8081 PHY(single port PHY), each port can be linked to maximum
>>     speed 2.5G.
>>
>>     For qca8386(switch chip), which includes the same PHY CHIP as qca8084
>>     (4 physical ports and two CPU ports), qca8386 switch can work with
>>     the current qca8k.c DSA driver with the supplement patches.
> 
> Is the qca8386 purely a switch plus integrated PHYs? There is no CPU
> on it? What is the management path? MDIO?

Yes, qca8386 is a pure switch plus integrated PHYs(same PHY type as
qca8084), there is no CPU on qca8386, the management path is MDIO.
the access of switch register is by the multiple MDIO operations.

> 
>>
>>     Both qca8084 and qca8386 includes same network clock controller(let's
>>     call it NSSCC, since this clock controller is located in the
>>     Ethernet chip qca8084 and qca8386), they have the same clock initial
>>     configuration sequence to initialize the Ethernet chip.
> 
> You said For "qca8084(pure PHY chip)". Here you just called it an
> Ethernet chip? To me, and Ethernet chip is a MAC, Intel e1000e etc.
> Do you now see how your explanations are confusing. Is it s pure PHY,
> or is it an Ethernet chip?

My bad, sorry for this confusion.
qca8084 is a pure PHY, there is no MAC in qca8084.

> 
> O.K. Since we are getting nowhere at the moment, lets take just the
> pure PHY chip, and ignore the rest for the moment.
> 
> For any pure PHY, there is generally one clock input, which might be a
> crystal, or an actual clock. If you look at other DT bindings for
> PHYs, it is only listed if the clock is expected to come from
> somewhere else, like a SoC, and it needs to be turned on before the
> PHY will work. And generally, a pure PHY has one defined clock
> frequency input. If that is true, there is no need to specify the
> clock. If multiple clock input frequencies are supported, then you do
> need to specify the clock, so its possible to work out what frequency
> it is using. How that clock input is then used internally in the PHY
> is not described in DT, but the driver can set any dividers, PLLs
> needed etc.

Yes, Andrew, there is only one clock input to qca8084(same as qca8386),
this input clock rate is 50MHZ, which is from the output clock of CMN
PLL block that is configured by the MDIO bus driver patch under review.

In qca8084(same as qca8386), there is a clock controller, let's call it
as NSSCC, the logic of NSSCC is same as qualcomm GCC(located in SoC),
the NSSCC provides the clocks to the quad PHYs, the initial clocks for
quad PHYs need to be configured before PHY to work.

These clocks and resets are provided by the NSSCC provider driver,
i need to define these clocks and resets in DT to use it.

> 
> So, for the pure PHY chip, what is the pinout? Is there one clock
> input? Or 4 clock inputs, one per PHY in the quad package? Typically,
> where does this/these clocks come from? Is the frequency fixed by the
> design, or are a number of input frequencies supported?

There is one 50M clock input for qca8084(same as qca8386), the input
clock is generated from the CMN PLL block that is configured by MDIO
driver patch of mdio-ipq4019.c.
The frequency of input clock is fixed to 50MHZ.

> 
>>    The Ethernet chip qca8084 and qca8386 are only connected with IPQ SoC,
>>    Currently qca8084 is connected with IPQ SoC by 10G-QXGMII mode.
>>    the 4 PHYs of qca8386 are connected with the internal MAC of qca8386
>>    by GMII, the maximum speed is also 2.5G.
>>    The port4 of qca8084 or qca8386 is optionally be able to connected
>>    with IPQ SoC by sgmii.
> 
> To some extent, this does not matter. The DT binding and the driver
> should not care what the pure PHY is connected to. It has standardised
> ports, so in theory it could be connected to any vendors MAC.

Yes, it can be connected with any vendors MAC with the interface mode
supported.

> 
> Please be very careful with your wording. Because computers
> instructions should be unambiguous, it does what it is told, we also
> expect computer scientists to be unambiguous. Wording is very
> important.
> 
>         Andrew
Got it. Thanks Andrew for the comments and suggestions.

Re: [PATCH v8 14/14] dt-bindings: net: ar803x: add qca8084 PHY properties

[Andrew Lunn]

> > O.K. Since we are getting nowhere at the moment, lets take just the
> > pure PHY chip, and ignore the rest for the moment.
> > 
> > For any pure PHY, there is generally one clock input, which might be a
> > crystal, or an actual clock. If you look at other DT bindings for
> > PHYs, it is only listed if the clock is expected to come from
> > somewhere else, like a SoC, and it needs to be turned on before the
> > PHY will work. And generally, a pure PHY has one defined clock
> > frequency input. If that is true, there is no need to specify the
> > clock. If multiple clock input frequencies are supported, then you do
> > need to specify the clock, so its possible to work out what frequency
> > it is using. How that clock input is then used internally in the PHY
> > is not described in DT, but the driver can set any dividers, PLLs
> > needed etc.
> 
> Yes, Andrew, there is only one clock input to qca8084(same as qca8386),
> this input clock rate is 50MHZ, which is from the output clock of CMN
> PLL block that is configured by the MDIO bus driver patch under review.

Lets concentrate on the pure PHY. All it sees is a clock. It does not
care where it come from. All you need in the device tree for the pure
PHY is a clock consumer.

There is one clock input, so its shared by all four instances in the
pure PHY package. So you need to use Christians code which extends the
PHY DT bindings to allow DT properties for a package of PHYs.

What about resets. Is there one reset pin for the pure PHY package, or
one per PHY?

Go find Christians code, understand it, and propose a DT binding for
the pure PHY. Include the clock provider and the reset
provider. Forget about the MDIO controller, and the PHY integrated
into the switch, etc. Baby steps...

> In qca8084(same as qca8386), there is a clock controller, let's call it
> as NSSCC, the logic of NSSCC is same as qualcomm GCC(located in SoC),
> the NSSCC provides the clocks to the quad PHYs, the initial clocks for
> quad PHYs need to be configured before PHY to work.

You said above, there is one clock input to the qca8084. Here you use
the word clocks, plural. Is there one clock, or multiple clocks?

    Andrew

Re: [PATCH v8 14/14] dt-bindings: net: ar803x: add qca8084 PHY properties

[Jie Luo]

On 1/5/2024 9:37 PM, Andrew Lunn wrote:
>>> O.K. Since we are getting nowhere at the moment, lets take just the
>>> pure PHY chip, and ignore the rest for the moment.
>>>
>>> For any pure PHY, there is generally one clock input, which might be a
>>> crystal, or an actual clock. If you look at other DT bindings for
>>> PHYs, it is only listed if the clock is expected to come from
>>> somewhere else, like a SoC, and it needs to be turned on before the
>>> PHY will work. And generally, a pure PHY has one defined clock
>>> frequency input. If that is true, there is no need to specify the
>>> clock. If multiple clock input frequencies are supported, then you do
>>> need to specify the clock, so its possible to work out what frequency
>>> it is using. How that clock input is then used internally in the PHY
>>> is not described in DT, but the driver can set any dividers, PLLs
>>> needed etc.
>>
>> Yes, Andrew, there is only one clock input to qca8084(same as qca8386),
>> this input clock rate is 50MHZ, which is from the output clock of CMN
>> PLL block that is configured by the MDIO bus driver patch under review.
> 
> Lets concentrate on the pure PHY. All it sees is a clock. It does not
> care where it come from. All you need in the device tree for the pure
> PHY is a clock consumer.
Yes.

> 
> There is one clock input, so its shared by all four instances in the
> pure PHY package. So you need to use Christians code which extends the
> PHY DT bindings to allow DT properties for a package of PHYs.

OK, will

> 
> What about resets. Is there one reset pin for the pure PHY package, or
> one per PHY?

There is only one GPIO hardware reset PIN for the chip qca8084 including
all 4 PHYs.

> 
> Go find Christians code, understand it, and propose a DT binding for
> the pure PHY. Include the clock provider and the reset
> provider. Forget about the MDIO controller, and the PHY integrated
> into the switch, etc. Baby steps...

Thanks Andrew for pointing me the Christians code, i will keep the
driver of qca8084 synced with Christian's code before pushing the
next patch set.

> 
>> In qca8084(same as qca8386), there is a clock controller, let's call it
>> as NSSCC, the logic of NSSCC is same as qualcomm GCC(located in SoC),
>> the NSSCC provides the clocks to the quad PHYs, the initial clocks for
>> quad PHYs need to be configured before PHY to work.
> 
> You said above, there is one clock input to the qca8084. Here you use
> the word clocks, plural. Is there one clock, or multiple clocks?
> 
>      Andrew

Yes, Andrew, it is multiple clocks.
These multiple clocks are generated(PLL, divider) and used internally by
qca8084 CHIP, these clocks are generated by the clock controller of
qca8084, let's call the clock controller of qca8084 as NSSCC provider,
which generates the clocks to the PHYs, this NSSCC is located in
qca8084.

The only one input clock of qca8084 is the clock source of the chip
qca8084, which is fixed to 50MHZ.

The NSSCC of qca8084 generates different clock rates for the different
link speed of the PHY, which is the internal block of qca8084.

Re: [PATCH v8 14/14] dt-bindings: net: ar803x: add qca8084 PHY properties

[Andrew Lunn]

> The following is the chip package, the chip can work on the switch mode
> like the existed upstream code qca8k, where PHY1-PHY4 is connected with
> MAC1-MAC4 directly; The chip can also work on the PHY mode, where PHY1-
> PHY4 is connected with PCS1 by 10g-qxgmii; Either switch mode or PHY mode,
> the PHY4 is optionally connected with PCS0 by SGMII, PCS0 and PCS1
> are connected with the SoC(IPQ platform) PCSes.

I don't really understand. Are you saying the hardware is actually :


+----------------------------------------------+
|          PCS1           PCS0                 |
|                                              |
|          MAC0           MAC5                 |
|           |              |                   |
|     +-----+--------------+-------------+     |
|     |                                  |     |
|     |                Switch            |     |
|     |                                  |     |
|     +-+---------+---------+---------+--+     |
|       |         |         |         |        |
|      MAC1      MAC2      MAC3      MAC4      |
|                                              |
|      PHY1      PHY2      PHY3      PHY4      |
+----------------------------------------------+

When in PHY mode, the switch is hard coded to map the 4 PCS1 channels
straight to MAC1-MAC4 and all switch functionality is disabled. But
then in switch mode, the switch can be controlled as a DSA switch? The
10G PCS1 is then a single 10G port, not 4x 2.5G?

Is there a product brief for this PHY? That might help us understand
this hardware?

    Andrew

Re: [PATCH v8 14/14] dt-bindings: net: ar803x: add qca8084 PHY properties

[Russell King (Oracle)]

On Sat, Dec 16, 2023 at 06:30:00PM +0100, Andrew Lunn wrote:
> > The following is the chip package, the chip can work on the switch mode
> > like the existed upstream code qca8k, where PHY1-PHY4 is connected with
> > MAC1-MAC4 directly; The chip can also work on the PHY mode, where PHY1-
> > PHY4 is connected with PCS1 by 10g-qxgmii; Either switch mode or PHY mode,
> > the PHY4 is optionally connected with PCS0 by SGMII, PCS0 and PCS1
> > are connected with the SoC(IPQ platform) PCSes.
> 
> I don't really understand. Are you saying the hardware is actually :
> 
> 
> +----------------------------------------------+
> |          PCS1           PCS0                 |
> |                                              |
> |          MAC0           MAC5                 |
> |           |              |                   |
> |     +-----+--------------+-------------+     |
> |     |                                  |     |
> |     |                Switch            |     |
> |     |                                  |     |
> |     +-+---------+---------+---------+--+     |
> |       |         |         |         |        |
> |      MAC1      MAC2      MAC3      MAC4      |
> |                                              |
> |      PHY1      PHY2      PHY3      PHY4      |
> +----------------------------------------------+
> 
> When in PHY mode, the switch is hard coded to map the 4 PCS1 channels
> straight to MAC1-MAC4 and all switch functionality is disabled. But
> then in switch mode, the switch can be controlled as a DSA switch? The
> 10G PCS1 is then a single 10G port, not 4x 2.5G?
> 
> Is there a product brief for this PHY? That might help us understand
> this hardware?

Not even digikey give any clues what "QCA8084" is - they list it as
"unclassified" and give no documentation and no photo. Basically it
seems to be a super secret device.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH v8 14/14] dt-bindings: net: ar803x: add qca8084 PHY properties

[Jie Luo]

On 12/17/2023 3:08 AM, Russell King (Oracle) wrote:
> On Sat, Dec 16, 2023 at 06:30:00PM +0100, Andrew Lunn wrote:
>>> The following is the chip package, the chip can work on the switch mode
>>> like the existed upstream code qca8k, where PHY1-PHY4 is connected with
>>> MAC1-MAC4 directly; The chip can also work on the PHY mode, where PHY1-
>>> PHY4 is connected with PCS1 by 10g-qxgmii; Either switch mode or PHY mode,
>>> the PHY4 is optionally connected with PCS0 by SGMII, PCS0 and PCS1
>>> are connected with the SoC(IPQ platform) PCSes.
>>
>> I don't really understand. Are you saying the hardware is actually :
>>
>>
>> +----------------------------------------------+
>> |          PCS1           PCS0                 |
>> |                                              |
>> |          MAC0           MAC5                 |
>> |           |              |                   |
>> |     +-----+--------------+-------------+     |
>> |     |                                  |     |
>> |     |                Switch            |     |
>> |     |                                  |     |
>> |     +-+---------+---------+---------+--+     |
>> |       |         |         |         |        |
>> |      MAC1      MAC2      MAC3      MAC4      |
>> |                                              |
>> |      PHY1      PHY2      PHY3      PHY4      |
>> +----------------------------------------------+
>>
>> When in PHY mode, the switch is hard coded to map the 4 PCS1 channels
>> straight to MAC1-MAC4 and all switch functionality is disabled. But
>> then in switch mode, the switch can be controlled as a DSA switch? The
>> 10G PCS1 is then a single 10G port, not 4x 2.5G?
>>
>> Is there a product brief for this PHY? That might help us understand
>> this hardware?
> 
> Not even digikey give any clues what "QCA8084" is - they list it as
> "unclassified" and give no documentation and no photo. Basically it
> seems to be a super secret device.
> 
Sorry for the confusion here, maybe the chip is developed recently,
which leads to the Doc or introduction is not released in time.

Re: [PATCH v8 14/14] dt-bindings: net: ar803x: add qca8084 PHY properties

[Jie Luo]

On 12/17/2023 1:30 AM, Andrew Lunn wrote:
>> The following is the chip package, the chip can work on the switch mode
>> like the existed upstream code qca8k, where PHY1-PHY4 is connected with
>> MAC1-MAC4 directly; The chip can also work on the PHY mode, where PHY1-
>> PHY4 is connected with PCS1 by 10g-qxgmii; Either switch mode or PHY mode,
>> the PHY4 is optionally connected with PCS0 by SGMII, PCS0 and PCS1
>> are connected with the SoC(IPQ platform) PCSes.
> 
> I don't really understand. Are you saying the hardware is actually :
> 
> 
> +----------------------------------------------+
> |          PCS1           PCS0                 |
> |                                              |
> |          MAC0           MAC5                 |
> |           |              |                   |
> |     +-----+--------------+-------------+     |
> |     |                                  |     |
> |     |                Switch            |     |
> |     |                                  |     |
> |     +-+---------+---------+---------+--+     |
> |       |         |         |         |        |
> |      MAC1      MAC2      MAC3      MAC4      |
> |                                              |
> |      PHY1      PHY2      PHY3      PHY4      |
> +----------------------------------------------+
> 

Actually there are two CHIP types, ,let me explain to be more clear.

1. The diagram you describe is actually the switch work mode, which has
the different chip name called qca8386, the DSA driver and PHY driver
are used, since the general PHY driver can't work for the PHY here.

  +----------------------------------------------+
  | +-----+
  | | GCC |
  | +-----+  PCS1           PCS0                 |
  |                                              |
  |          MAC0           MAC5                 |
  |           |              |                   |
  |     +-----+--------------+-------------+     |
  |     |                                  |     |
  |     |                Switch            |     |
  |     |                                  |     |
  |     +-+---------+---------+---------+--+     |
  |       |         |         |         |        |
  |      MAC1      MAC2      MAC3      MAC4      |
  |                                              |
  |      PHY1      PHY2      PHY3      PHY4      |
  +----------------------------------------------+

2. The pure PHY chip called by qca8084 works on the PHY mode 10-qxgmii
on quad-phy, or the sgmii mode can be configured on PHY4 optionally.
The qca8084 is below, there is no MAC involved on qca8084.

  +----------------------------------------------+
  |          PCS1           PCS0                 |
  |                                              |
  |   +-----+
  |   | GCC |
  |   +-----+
  |                                              |
  |      PHY1      PHY2      PHY3      PHY4      |
  +----------------------------------------------+

On qca8386, the same qca8084 PHY is used, but the qca8084 PHY is
connected with internal MAC directly same as qca8337(qca8k dsa driver).

On both Ethernet chips qca8386 and qca8084, GCC block is same and
with the same clock controller driver that provides the clocks and
resets used by the qca8084 PHY driver and qca8386 DSA driver(leverage
the existed DSA driver qca8k.c).

> When in PHY mode, the switch is hard coded to map the 4 PCS1 channels
> straight to MAC1-MAC4 and all switch functionality is disabled. But
> then in switch mode, the switch can be controlled as a DSA switch? The
> 10G PCS1 is then a single 10G port, not 4x 2.5G?

For the qca8084 PHY chip, there is no MAC involved, the PHY is connected 
with the PCS with 10g-qxgmii, PHY4 is optional connected with sgmii.

For the qca8386 switch chip, it is controlled as DSA, the PCS is
connected with the SOC(such as IPQ5332) PCS.
> 
> Is there a product brief for this PHY? That might help us understand
> this hardware?
> 
Sorry, i also searched it on the internet and Qualcomm website, there is
no Doc found, the CHIP is developed recently 1-2 year before, the Doc is
not updated to the website.

>      Andrew