[net-next PATCH v2 00/15] Multiple improvement for qca8337 switch

[net-next PATCH v2 00/15] Multiple improvement for qca8337 switch

[Ansuel Smith]

This series is the final step of a long process of porting 80+ devices
to use the new qca8k driver instead of the hacky qca one based on never
merged swconfig platform.
Some background to justify all these additions.
QCA used a special binding to declare raw initval to set the swich. I
made a script to convert all these magic values and convert 80+ dts and
scan all the needed "unsupported regs". We find a baseline where we
manage to find the common and used regs so in theory hopefully we don't
have to add anymore things.
We discovered lots of things with this, especially about how differently
qca8327 works compared to qca8337.

In short, we found that qca8327 have some problem with suspend/resume for
their internal phy. It instead sets some dedicated regs that suspend the
phy without setting the standard bit. First 4 patch are to fix this.
There is also a patch about preferring master. This is directly from the
original driver and it seems to be needed to prevent some problem with
the pause frame.

Every ipq806x target sets the mac power sel and this specific reg
regulates the output voltage of the regulator. Without this some
instability can occur.

Some configuration (for some reason) swap mac6 with mac0. We add support
for this.
Also, we discovered that some device doesn't work at all with pll enabled
for sgmii line. In the original code this was based on the switch
revision. In later revision the pll regs were decided based on the switch
type (disabled for qca8327 and enabled for qca8337) but still some
device had that disabled in the initval regs.
Considering we found at least one qca8337 device that required pll
disabled to work (no traffic problem) we decided to introduce a binding
to enable pll and set it only with that.

Lastly, we add support for led open drain that require the power-on-sel
to set. Also, some device have only the power-on-sel set in the initval
so we add also support for that. This is needed for the correct function
of the switch leds.
Qca8327 have a special reg in the pws regs that set it to a reduced
48pin layout. This is needed or the switch doesn't work.

These are all the special configuration we find on all these devices that
are from various targets. Mostly ath79, ipq806x and bcm53xx.

Changes v2:
- Reword Documentation patch to dt-bindings
- Propose first 2 phy patch to net
- Better describe and add hint on how to use all the new
  bindings 
- Rework delay scan function and move to phylink mac_config
- Drop package48 wrong binding
- Introduce support for qca8328 switch
- Fix wrong binding name power-on-sel
- Return error on wrong config with led open drain and 
  ignore-power-on-sel not set

Ansuel Smith (15):
  drivers: net: phy: at803x: fix resume for QCA8327 phy
  drivers: net: phy: at803x: add DAC amplitude fix for 8327 phy
  drivers: net: phy: at803x: enable prefer master for 83xx internal phy
  drivers: net: phy: at803x: better describe debug regs
  net: dsa: qca8k: add mac_power_sel support
  dt-bindings: net: dsa: qca8k: document rgmii_1_8v bindings
  net: dsa: qca8k: add support for mac6_exchange, sgmii falling edge
  dt-bindings: net: dsa: qca8k: Add MAC swap and clock phase properties
  net: dsa: qca8k: move rgmii delay detection to phylink mac_config
  net: dsa: qca8k: add explicit SGMII PLL enable
  dt-bindings: net: dsa: qca8k: Document qca,sgmii-enable-pll
  drivers: net: dsa: qca8k: add support for pws config reg
  dt-bindings: net: dsa: qca8k: document open drain binding
  drivers: net: dsa: qca8k: add support for QCA8328
  dt-bindings: net: dsa: qca8k: document support for qca8328

 .../devicetree/bindings/net/dsa/qca8k.txt     |  31 +++
 drivers/net/dsa/qca8k.c                       | 207 ++++++++++++------
 drivers/net/dsa/qca8k.h                       |  17 +-
 drivers/net/phy/at803x.c                      | 127 +++++++++--
 4 files changed, 302 insertions(+), 80 deletions(-)

-- 
2.32.0

[net PATCH v2 01/15] drivers: net: phy: at803x: fix resume for QCA8327 phy

[Ansuel Smith]

From Documentation phy resume triggers phy reset and restart
auto-negotiation. Add a dedicated function to wait reset to finish as
it was notice a regression where port sometime are not reliable after a
suspend/resume session. The reset wait logic is copied from phy_poll_reset.
Add dedicated suspend function to use genphy_suspend only with QCA8337
phy and set only additional debug settings for QCA8327. With more test
it was reported that QCA8327 doesn't proprely support this mode and
using this cause the unreliability of the switch ports, especially the
malfunction of the port0.

Fixes: 52a6cdbe43a3 ("net: phy: at803x: add resume/suspend function to qca83xx phy")
Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 drivers/net/phy/at803x.c | 69 ++++++++++++++++++++++++++++++++++++----
 1 file changed, 63 insertions(+), 6 deletions(-)

diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index 3feee4d59030..c6c87b82c95c 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -92,9 +92,14 @@
 #define AT803X_DEBUG_REG_5			0x05
 #define AT803X_DEBUG_TX_CLK_DLY_EN		BIT(8)
 
+#define AT803X_DEBUG_REG_HIB_CTRL		0x0b
+#define   AT803X_DEBUG_HIB_CTRL_SEL_RST_80U	BIT(10)
+#define   AT803X_DEBUG_HIB_CTRL_EN_ANY_CHANGE	BIT(13)
+
 #define AT803X_DEBUG_REG_3C			0x3C
 
 #define AT803X_DEBUG_REG_3D			0x3D
+#define   AT803X_DEBUG_GATE_CLK_IN1000		BIT(6)
 
 #define AT803X_DEBUG_REG_1F			0x1F
 #define AT803X_DEBUG_PLL_ON			BIT(2)
@@ -1312,6 +1317,58 @@ static int qca83xx_config_init(struct phy_device *phydev)
 	return 0;
 }
 
+static int qca83xx_resume(struct phy_device *phydev)
+{
+	int ret, val;
+
+	/* Skip reset if not suspended */
+	if (!phydev->suspended)
+		return 0;
+
+	/* Reinit the port, reset values set by suspend */
+	qca83xx_config_init(phydev);
+
+	/* Reset the port on port resume */
+	phy_set_bits(phydev, MII_BMCR, BMCR_RESET | BMCR_ANENABLE);
+
+	/* On resume from suspend the switch execute a reset and
+	 * restart auto-negotiation. Wait for reset to complete.
+	 */
+	ret = phy_read_poll_timeout(phydev, MII_BMCR, val, !(val & BMCR_RESET),
+				    50000, 600000, true);
+	if (ret)
+		return ret;
+
+	msleep(1);
+
+	return 0;
+}
+
+static int qca83xx_suspend(struct phy_device *phydev)
+{
+	u16 mask = 0;
+
+	/* Only QCA8337 support actual suspend.
+	 * QCA8327 cause port unreliability when phy suspend
+	 * is set.
+	 */
+	if (phydev->drv->phy_id == QCA8337_PHY_ID) {
+		genphy_suspend(phydev);
+	} else {
+		mask |= ~(BMCR_SPEED1000 | BMCR_FULLDPLX);
+		phy_modify(phydev, MII_BMCR, mask, 0);
+	}
+
+	at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_3D,
+			      AT803X_DEBUG_GATE_CLK_IN1000, 0);
+
+	at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_HIB_CTRL,
+			      AT803X_DEBUG_HIB_CTRL_EN_ANY_CHANGE |
+			      AT803X_DEBUG_HIB_CTRL_SEL_RST_80U, 0);
+
+	return 0;
+}
+
 static struct phy_driver at803x_driver[] = {
 {
 	/* Qualcomm Atheros AR8035 */
@@ -1421,8 +1478,8 @@ static struct phy_driver at803x_driver[] = {
 	.get_sset_count		= at803x_get_sset_count,
 	.get_strings		= at803x_get_strings,
 	.get_stats		= at803x_get_stats,
-	.suspend		= genphy_suspend,
-	.resume			= genphy_resume,
+	.suspend		= qca83xx_suspend,
+	.resume			= qca83xx_resume,
 }, {
 	/* QCA8327-A from switch QCA8327-AL1A */
 	.phy_id			= QCA8327_A_PHY_ID,
@@ -1436,8 +1493,8 @@ static struct phy_driver at803x_driver[] = {
 	.get_sset_count		= at803x_get_sset_count,
 	.get_strings		= at803x_get_strings,
 	.get_stats		= at803x_get_stats,
-	.suspend		= genphy_suspend,
-	.resume			= genphy_resume,
+	.suspend		= qca83xx_suspend,
+	.resume			= qca83xx_resume,
 }, {
 	/* QCA8327-B from switch QCA8327-BL1A */
 	.phy_id			= QCA8327_B_PHY_ID,
@@ -1451,8 +1508,8 @@ static struct phy_driver at803x_driver[] = {
 	.get_sset_count		= at803x_get_sset_count,
 	.get_strings		= at803x_get_strings,
 	.get_stats		= at803x_get_stats,
-	.suspend		= genphy_suspend,
-	.resume			= genphy_resume,
+	.suspend		= qca83xx_suspend,
+	.resume			= qca83xx_resume,
 }, };
 
 module_phy_driver(at803x_driver);
-- 
2.32.0

[net PATCH v2 02/15] drivers: net: phy: at803x: add DAC amplitude fix for 8327 phy

[Ansuel Smith]

QCA8327 internal phy require DAC amplitude adjustement set to +6% with
100m speed. Also add additional define to report a change of the same
reg in QCA8337. (different scope it does set 1000m voltage)
Add link_change_notify function to set the proper amplitude adjustement
on PHY_RUNNING state and disable on any other state.

Fixes: c6bcec0d6928 ("net: phy: at803x: add support for qca 8327 A variant internal phy")
Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 drivers/net/phy/at803x.c | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index c6c87b82c95c..5208ea8fdd69 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -87,6 +87,8 @@
 #define AT803X_PSSR_MR_AN_COMPLETE		0x0200
 
 #define AT803X_DEBUG_REG_0			0x00
+#define QCA8327_DEBUG_MANU_CTRL_EN		BIT(2)
+#define QCA8337_DEBUG_MANU_CTRL_EN		GENMASK(3, 2)
 #define AT803X_DEBUG_RX_CLK_DLY_EN		BIT(15)
 
 #define AT803X_DEBUG_REG_5			0x05
@@ -1314,9 +1316,37 @@ static int qca83xx_config_init(struct phy_device *phydev)
 		break;
 	}
 
+	/* QCA8327 require DAC amplitude adjustment for 100m set to +6%.
+	 * Disable on init and enable only with 100m speed following
+	 * qca original source code.
+	 */
+	if (phydev->drv->phy_id == QCA8327_A_PHY_ID ||
+	    phydev->drv->phy_id == QCA8327_B_PHY_ID)
+		at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_0,
+				      QCA8327_DEBUG_MANU_CTRL_EN, 0);
+
 	return 0;
 }
 
+static void qca83xx_link_change_notify(struct phy_device *phydev)
+{
+	/* QCA8337 doesn't require DAC Amplitude adjustement */
+	if (phydev->drv->phy_id == QCA8337_PHY_ID)
+		return;
+
+	/* Set DAC Amplitude adjustment to +6% for 100m on link running */
+	if (phydev->state == PHY_RUNNING) {
+		if (phydev->speed == SPEED_100)
+			at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_0,
+					      QCA8327_DEBUG_MANU_CTRL_EN,
+					      QCA8327_DEBUG_MANU_CTRL_EN);
+	} else {
+		/* Reset DAC Amplitude adjustment */
+		at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_0,
+				      QCA8327_DEBUG_MANU_CTRL_EN, 0);
+	}
+}
+
 static int qca83xx_resume(struct phy_device *phydev)
 {
 	int ret, val;
@@ -1471,6 +1501,7 @@ static struct phy_driver at803x_driver[] = {
 	.phy_id_mask		= QCA8K_PHY_ID_MASK,
 	.name			= "Qualcomm Atheros 8337 internal PHY",
 	/* PHY_GBIT_FEATURES */
+	.link_change_notify	= qca83xx_link_change_notify,
 	.probe			= at803x_probe,
 	.flags			= PHY_IS_INTERNAL,
 	.config_init		= qca83xx_config_init,
@@ -1486,6 +1517,7 @@ static struct phy_driver at803x_driver[] = {
 	.phy_id_mask		= QCA8K_PHY_ID_MASK,
 	.name			= "Qualcomm Atheros 8327-A internal PHY",
 	/* PHY_GBIT_FEATURES */
+	.link_change_notify	= qca83xx_link_change_notify,
 	.probe			= at803x_probe,
 	.flags			= PHY_IS_INTERNAL,
 	.config_init		= qca83xx_config_init,
@@ -1501,6 +1533,7 @@ static struct phy_driver at803x_driver[] = {
 	.phy_id_mask		= QCA8K_PHY_ID_MASK,
 	.name			= "Qualcomm Atheros 8327-B internal PHY",
 	/* PHY_GBIT_FEATURES */
+	.link_change_notify	= qca83xx_link_change_notify,
 	.probe			= at803x_probe,
 	.flags			= PHY_IS_INTERNAL,
 	.config_init		= qca83xx_config_init,
-- 
2.32.0

[net-next PATCH v2 03/15] drivers: net: phy: at803x: enable prefer master for 83xx internal phy

[Ansuel Smith]

From original QCA source code the port was set to prefer master as port
type in 1000BASE-T mode. Apply the same settings also here.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/phy/at803x.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index 5208ea8fdd69..402b2096f209 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -1325,6 +1325,9 @@ static int qca83xx_config_init(struct phy_device *phydev)
 		at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_0,
 				      QCA8327_DEBUG_MANU_CTRL_EN, 0);
 
+	/* Following original QCA sourcecode set port to prefer master */
+	phy_set_bits(phydev, MII_CTRL1000, CTL1000_PREFER_MASTER);
+
 	return 0;
 }
 
-- 
2.32.0

[net-next PATCH v2 04/15] drivers: net: phy: at803x: better describe debug regs

[Ansuel Smith]

Give a name to known debug regs from Documentation instead of using
unknown hex values.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 drivers/net/phy/at803x.c | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index 402b2096f209..f40f17a632ad 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -86,12 +86,12 @@
 #define AT803X_PSSR				0x11	/*PHY-Specific Status Register*/
 #define AT803X_PSSR_MR_AN_COMPLETE		0x0200
 
-#define AT803X_DEBUG_REG_0			0x00
+#define AT803X_DEBUG_ANALOG_TEST_CTRL		0x00
 #define QCA8327_DEBUG_MANU_CTRL_EN		BIT(2)
 #define QCA8337_DEBUG_MANU_CTRL_EN		GENMASK(3, 2)
 #define AT803X_DEBUG_RX_CLK_DLY_EN		BIT(15)
 
-#define AT803X_DEBUG_REG_5			0x05
+#define AT803X_DEBUG_SYSTEM_CTRL_MODE		0x05
 #define AT803X_DEBUG_TX_CLK_DLY_EN		BIT(8)
 
 #define AT803X_DEBUG_REG_HIB_CTRL		0x0b
@@ -100,7 +100,7 @@
 
 #define AT803X_DEBUG_REG_3C			0x3C
 
-#define AT803X_DEBUG_REG_3D			0x3D
+#define AT803X_DEBUG_REG_GREEN			0x3D
 #define   AT803X_DEBUG_GATE_CLK_IN1000		BIT(6)
 
 #define AT803X_DEBUG_REG_1F			0x1F
@@ -284,25 +284,25 @@ static int at803x_read_page(struct phy_device *phydev)
 
 static int at803x_enable_rx_delay(struct phy_device *phydev)
 {
-	return at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_0, 0,
+	return at803x_debug_reg_mask(phydev, AT803X_DEBUG_ANALOG_TEST_CTRL, 0,
 				     AT803X_DEBUG_RX_CLK_DLY_EN);
 }
 
 static int at803x_enable_tx_delay(struct phy_device *phydev)
 {
-	return at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_5, 0,
+	return at803x_debug_reg_mask(phydev, AT803X_DEBUG_SYSTEM_CTRL_MODE, 0,
 				     AT803X_DEBUG_TX_CLK_DLY_EN);
 }
 
 static int at803x_disable_rx_delay(struct phy_device *phydev)
 {
-	return at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_0,
+	return at803x_debug_reg_mask(phydev, AT803X_DEBUG_ANALOG_TEST_CTRL,
 				     AT803X_DEBUG_RX_CLK_DLY_EN, 0);
 }
 
 static int at803x_disable_tx_delay(struct phy_device *phydev)
 {
-	return at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_5,
+	return at803x_debug_reg_mask(phydev, AT803X_DEBUG_SYSTEM_CTRL_MODE,
 				     AT803X_DEBUG_TX_CLK_DLY_EN, 0);
 }
 
@@ -1300,9 +1300,9 @@ static int qca83xx_config_init(struct phy_device *phydev)
 	switch (switch_revision) {
 	case 1:
 		/* For 100M waveform */
-		at803x_debug_reg_write(phydev, AT803X_DEBUG_REG_0, 0x02ea);
+		at803x_debug_reg_write(phydev, AT803X_DEBUG_ANALOG_TEST_CTRL, 0x02ea);
 		/* Turn on Gigabit clock */
-		at803x_debug_reg_write(phydev, AT803X_DEBUG_REG_3D, 0x68a0);
+		at803x_debug_reg_write(phydev, AT803X_DEBUG_REG_GREEN, 0x68a0);
 		break;
 
 	case 2:
@@ -1310,8 +1310,8 @@ static int qca83xx_config_init(struct phy_device *phydev)
 		fallthrough;
 	case 4:
 		phy_write_mmd(phydev, MDIO_MMD_PCS, MDIO_AZ_DEBUG, 0x803f);
-		at803x_debug_reg_write(phydev, AT803X_DEBUG_REG_3D, 0x6860);
-		at803x_debug_reg_write(phydev, AT803X_DEBUG_REG_5, 0x2c46);
+		at803x_debug_reg_write(phydev, AT803X_DEBUG_REG_GREEN, 0x6860);
+		at803x_debug_reg_write(phydev, AT803X_DEBUG_SYSTEM_CTRL_MODE, 0x2c46);
 		at803x_debug_reg_write(phydev, AT803X_DEBUG_REG_3C, 0x6000);
 		break;
 	}
@@ -1322,7 +1322,7 @@ static int qca83xx_config_init(struct phy_device *phydev)
 	 */
 	if (phydev->drv->phy_id == QCA8327_A_PHY_ID ||
 	    phydev->drv->phy_id == QCA8327_B_PHY_ID)
-		at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_0,
+		at803x_debug_reg_mask(phydev, AT803X_DEBUG_ANALOG_TEST_CTRL,
 				      QCA8327_DEBUG_MANU_CTRL_EN, 0);
 
 	/* Following original QCA sourcecode set port to prefer master */
@@ -1340,12 +1340,12 @@ static void qca83xx_link_change_notify(struct phy_device *phydev)
 	/* Set DAC Amplitude adjustment to +6% for 100m on link running */
 	if (phydev->state == PHY_RUNNING) {
 		if (phydev->speed == SPEED_100)
-			at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_0,
+			at803x_debug_reg_mask(phydev, AT803X_DEBUG_ANALOG_TEST_CTRL,
 					      QCA8327_DEBUG_MANU_CTRL_EN,
 					      QCA8327_DEBUG_MANU_CTRL_EN);
 	} else {
 		/* Reset DAC Amplitude adjustment */
-		at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_0,
+		at803x_debug_reg_mask(phydev, AT803X_DEBUG_ANALOG_TEST_CTRL,
 				      QCA8327_DEBUG_MANU_CTRL_EN, 0);
 	}
 }
@@ -1392,7 +1392,7 @@ static int qca83xx_suspend(struct phy_device *phydev)
 		phy_modify(phydev, MII_BMCR, mask, 0);
 	}
 
-	at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_3D,
+	at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_GREEN,
 			      AT803X_DEBUG_GATE_CLK_IN1000, 0);
 
 	at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_HIB_CTRL,
-- 
2.32.0

[net-next PATCH v2 05/15] net: dsa: qca8k: add mac_power_sel support

[Ansuel Smith]

Add missing mac power sel support needed for some switch that requires
additional setup. ar8327 have a different setup than 8337.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 drivers/net/dsa/qca8k.c | 27 +++++++++++++++++++++++++++
 drivers/net/dsa/qca8k.h |  5 +++++
 2 files changed, 32 insertions(+)

diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
index bda5a9bf4f52..5bce7ac4dea7 100644
--- a/drivers/net/dsa/qca8k.c
+++ b/drivers/net/dsa/qca8k.c
@@ -950,6 +950,29 @@ qca8k_setup_of_rgmii_delay(struct qca8k_priv *priv)
 	return 0;
 }
 
+static int
+qca8k_setup_mac_pwr_sel(struct qca8k_priv *priv)
+{
+	struct device_node *node = priv->dev->of_node;
+	u32 mask = 0;
+	int ret = 0;
+
+	if (of_property_read_bool(node, "qca,rgmii0-1-8v"))
+		mask |= QCA8K_MAC_PWR_RGMII0_1_8V;
+
+	if (of_property_read_bool(node, "qca,rgmii56-1-8v"))
+		mask |= QCA8K_MAC_PWR_RGMII1_1_8V;
+
+	if (mask) {
+		ret = qca8k_rmw(priv, QCA8K_REG_MAC_PWR_SEL,
+				QCA8K_MAC_PWR_RGMII0_1_8V |
+				QCA8K_MAC_PWR_RGMII1_1_8V,
+				mask);
+	}
+
+	return ret;
+}
+
 static int
 qca8k_setup(struct dsa_switch *ds)
 {
@@ -979,6 +1002,10 @@ qca8k_setup(struct dsa_switch *ds)
 	if (ret)
 		return ret;
 
+	ret = qca8k_setup_mac_pwr_sel(priv);
+	if (ret)
+		return ret;
+
 	/* Enable CPU Port */
 	ret = qca8k_reg_set(priv, QCA8K_REG_GLOBAL_FW_CTRL0,
 			    QCA8K_GLOBAL_FW_CTRL0_CPU_PORT_EN);
diff --git a/drivers/net/dsa/qca8k.h b/drivers/net/dsa/qca8k.h
index ed3b05ad6745..fc7db94cc0c9 100644
--- a/drivers/net/dsa/qca8k.h
+++ b/drivers/net/dsa/qca8k.h
@@ -100,6 +100,11 @@
 #define   QCA8K_SGMII_MODE_CTRL_PHY			(1 << 22)
 #define   QCA8K_SGMII_MODE_CTRL_MAC			(2 << 22)
 
+/* MAC_PWR_SEL registers */
+#define QCA8K_REG_MAC_PWR_SEL				0x0e4
+#define   QCA8K_MAC_PWR_RGMII1_1_8V			BIT(18)
+#define   QCA8K_MAC_PWR_RGMII0_1_8V			BIT(19)
+
 /* EEE control registers */
 #define QCA8K_REG_EEE_CTRL				0x100
 #define  QCA8K_REG_EEE_CTRL_LPI_EN(_i)			((_i + 1) * 2)
-- 
2.32.0

[net-next PATCH v2 06/15] dt-bindings: net: dsa: qca8k: document rgmii_1_8v bindings

[Ansuel Smith]

Document new qca,rgmii0_1_8v and qca,rgmii56_1_8v needed to setup
mac_pwr_sel register for qca8337 switch. Specific the use of this binding
that is used only in qca8337 and not in qca8327.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 Documentation/devicetree/bindings/net/dsa/qca8k.txt | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/dsa/qca8k.txt b/Documentation/devicetree/bindings/net/dsa/qca8k.txt
index 8c73f67c43ca..9383d6bf2426 100644
--- a/Documentation/devicetree/bindings/net/dsa/qca8k.txt
+++ b/Documentation/devicetree/bindings/net/dsa/qca8k.txt
@@ -13,6 +13,14 @@ Required properties:
 Optional properties:
 
 - reset-gpios: GPIO to be used to reset the whole device
+- qca,rgmii0-1-8v: Set the internal regulator to supply 1.8v for MAC0 port.
+                   This is needed for qca8337 and toggles the supply voltage
+                   from 1.5v to 1.8v. For the specific regs it was observed
+                   that this is needed only for ipq8064 and ipq8065 target.
+- qca,rgmii56-1-8v: Set the internal regulator to supply 1.8v for MAC5/6 port.
+                    This is needed for qca8337 and toggles the supply voltage
+                    from 1.5v to 1.8v. For the specific regs it was observed
+                    that this is needed only for ipq8065 target.
 
 Subnodes:
 
-- 
2.32.0

[net-next PATCH v2 07/15] net: dsa: qca8k: add support for mac6_exchange, sgmii falling edge

[Ansuel Smith]

Some device set the switch to exchange the mac0 port with mac6 port. Add
support for this in the qca8k driver. Also add support for SGMII rx/tx
clock falling edge. This is only present for pad0, pad5 and pad6 have
these bit reserved from Documentation.

Signed-off-by: Matthew Hagan <mnhagan88@gmail.com>
Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 drivers/net/dsa/qca8k.c | 33 +++++++++++++++++++++++++++++++++
 drivers/net/dsa/qca8k.h |  3 +++
 2 files changed, 36 insertions(+)

diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
index 5bce7ac4dea7..3a040a3ed58e 100644
--- a/drivers/net/dsa/qca8k.c
+++ b/drivers/net/dsa/qca8k.c
@@ -973,6 +973,34 @@ qca8k_setup_mac_pwr_sel(struct qca8k_priv *priv)
 	return ret;
 }
 
+static int
+qca8k_setup_port0_pad_ctrl_reg(struct qca8k_priv *priv)
+{
+	struct device_node *node = priv->dev->of_node;
+	u32 mask = 0;
+	int ret = 0;
+
+	/* Swap MAC0-MAC6 */
+	if (of_property_read_bool(node, "qca,mac6-exchange"))
+		mask |= QCA8K_PORT0_PAD_CTRL_MAC06_EXCHG;
+
+	/* SGMII Clock phase configuration */
+	if (of_property_read_bool(node, "qca,sgmii-rxclk-falling-edge"))
+		mask |= QCA8K_PORT0_PAD_SGMII_RXCLK_FALLING_EDGE;
+
+	if (of_property_read_bool(node, "qca,sgmii-txclk-falling-edge"))
+		mask |= QCA8K_PORT0_PAD_SGMII_TXCLK_FALLING_EDGE;
+
+	if (mask)
+		ret = qca8k_rmw(priv, QCA8K_REG_PORT0_PAD_CTRL,
+				QCA8K_PORT0_PAD_CTRL_MAC06_EXCHG |
+				QCA8K_PORT0_PAD_SGMII_RXCLK_FALLING_EDGE |
+				QCA8K_PORT0_PAD_SGMII_TXCLK_FALLING_EDGE,
+				mask);
+
+	return ret;
+}
+
 static int
 qca8k_setup(struct dsa_switch *ds)
 {
@@ -1006,6 +1034,11 @@ qca8k_setup(struct dsa_switch *ds)
 	if (ret)
 		return ret;
 
+	/* Configure additional PORT0_PAD_CTRL properties */
+	ret = qca8k_setup_port0_pad_ctrl_reg(priv);
+	if (ret)
+		return ret;
+
 	/* Enable CPU Port */
 	ret = qca8k_reg_set(priv, QCA8K_REG_GLOBAL_FW_CTRL0,
 			    QCA8K_GLOBAL_FW_CTRL0_CPU_PORT_EN);
diff --git a/drivers/net/dsa/qca8k.h b/drivers/net/dsa/qca8k.h
index fc7db94cc0c9..3fded69a6839 100644
--- a/drivers/net/dsa/qca8k.h
+++ b/drivers/net/dsa/qca8k.h
@@ -35,6 +35,9 @@
 #define   QCA8K_MASK_CTRL_DEVICE_ID_MASK		GENMASK(15, 8)
 #define   QCA8K_MASK_CTRL_DEVICE_ID(x)			((x) >> 8)
 #define QCA8K_REG_PORT0_PAD_CTRL			0x004
+#define   QCA8K_PORT0_PAD_CTRL_MAC06_EXCHG		BIT(31)
+#define   QCA8K_PORT0_PAD_SGMII_RXCLK_FALLING_EDGE	BIT(19)
+#define   QCA8K_PORT0_PAD_SGMII_TXCLK_FALLING_EDGE	BIT(18)
 #define QCA8K_REG_PORT5_PAD_CTRL			0x008
 #define QCA8K_REG_PORT6_PAD_CTRL			0x00c
 #define   QCA8K_PORT_PAD_RGMII_EN			BIT(26)
-- 
2.32.0

[net-next PATCH v2 08/15] dt-bindings: net: dsa: qca8k: Add MAC swap and clock phase properties

[Ansuel Smith]

Add names and decriptions of additional PORT0_PAD_CTRL properties.
Document new binding qca,mac6_exchange that exchange the mac0 port
with mac6.
qca,sgmii-(rx|tx)clk-falling-edge are for setting the respective clock
phase to failling edge.

Signed-off-by: Matthew Hagan <mnhagan88@gmail.com>
Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 Documentation/devicetree/bindings/net/dsa/qca8k.txt | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/dsa/qca8k.txt b/Documentation/devicetree/bindings/net/dsa/qca8k.txt
index 9383d6bf2426..208ee5bc1bbb 100644
--- a/Documentation/devicetree/bindings/net/dsa/qca8k.txt
+++ b/Documentation/devicetree/bindings/net/dsa/qca8k.txt
@@ -13,6 +13,11 @@ Required properties:
 Optional properties:
 
 - reset-gpios: GPIO to be used to reset the whole device
+- qca,mac6-exchange: Internally swap MAC0 with MAC6.
+- qca,sgmii-rxclk-falling-edge: Set the receive clock phase to falling edge.
+                                Mostly used in qca8327 with CPU port 0 set to
+                                sgmii.
+- qca,sgmii-txclk-falling-edge: Set the transmit clock phase to falling edge.
 - qca,rgmii0-1-8v: Set the internal regulator to supply 1.8v for MAC0 port.
                    This is needed for qca8337 and toggles the supply voltage
                    from 1.5v to 1.8v. For the specific regs it was observed
-- 
2.32.0

[net-next PATCH v2 09/15] net: dsa: qca8k: move rgmii delay detection to phylink mac_config

[Ansuel Smith]

Future proof commit. This switch have 2 CPU port and one valid
configuration is first CPU port set to sgmii and second CPU port set to
regmii-id. The current implementation detects delay only for CPU port
zero set to rgmii and doesn't count any delay set in a secondary CPU
port. Drop the current delay scan function and move it to the phylink
mac_config to generilize and implicitly add support for secondary CPU
port set to rgmii-id.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 drivers/net/dsa/qca8k.c | 122 +++++++++++++++-------------------------
 drivers/net/dsa/qca8k.h |   2 -
 2 files changed, 45 insertions(+), 79 deletions(-)

diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
index 3a040a3ed58e..05ecec4ebc01 100644
--- a/drivers/net/dsa/qca8k.c
+++ b/drivers/net/dsa/qca8k.c
@@ -888,68 +888,6 @@ qca8k_setup_mdio_bus(struct qca8k_priv *priv)
 	return 0;
 }
 
-static int
-qca8k_setup_of_rgmii_delay(struct qca8k_priv *priv)
-{
-	struct device_node *port_dn;
-	phy_interface_t mode;
-	struct dsa_port *dp;
-	u32 val;
-
-	/* CPU port is already checked */
-	dp = dsa_to_port(priv->ds, 0);
-
-	port_dn = dp->dn;
-
-	/* Check if port 0 is set to the correct type */
-	of_get_phy_mode(port_dn, &mode);
-	if (mode != PHY_INTERFACE_MODE_RGMII_ID &&
-	    mode != PHY_INTERFACE_MODE_RGMII_RXID &&
-	    mode != PHY_INTERFACE_MODE_RGMII_TXID) {
-		return 0;
-	}
-
-	switch (mode) {
-	case PHY_INTERFACE_MODE_RGMII_ID:
-	case PHY_INTERFACE_MODE_RGMII_RXID:
-		if (of_property_read_u32(port_dn, "rx-internal-delay-ps", &val))
-			val = 2;
-		else
-			/* Switch regs accept value in ns, convert ps to ns */
-			val = val / 1000;
-
-		if (val > QCA8K_MAX_DELAY) {
-			dev_err(priv->dev, "rgmii rx delay is limited to a max value of 3ns, setting to the max value");
-			val = 3;
-		}
-
-		priv->rgmii_rx_delay = val;
-		/* Stop here if we need to check only for rx delay */
-		if (mode != PHY_INTERFACE_MODE_RGMII_ID)
-			break;
-
-		fallthrough;
-	case PHY_INTERFACE_MODE_RGMII_TXID:
-		if (of_property_read_u32(port_dn, "tx-internal-delay-ps", &val))
-			val = 1;
-		else
-			/* Switch regs accept value in ns, convert ps to ns */
-			val = val / 1000;
-
-		if (val > QCA8K_MAX_DELAY) {
-			dev_err(priv->dev, "rgmii tx delay is limited to a max value of 3ns, setting to the max value");
-			val = 3;
-		}
-
-		priv->rgmii_tx_delay = val;
-		break;
-	default:
-		return 0;
-	}
-
-	return 0;
-}
-
 static int
 qca8k_setup_mac_pwr_sel(struct qca8k_priv *priv)
 {
@@ -1026,10 +964,6 @@ qca8k_setup(struct dsa_switch *ds)
 	if (ret)
 		return ret;
 
-	ret = qca8k_setup_of_rgmii_delay(priv);
-	if (ret)
-		return ret;
-
 	ret = qca8k_setup_mac_pwr_sel(priv);
 	if (ret)
 		return ret;
@@ -1201,7 +1135,8 @@ qca8k_phylink_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
 			 const struct phylink_link_state *state)
 {
 	struct qca8k_priv *priv = ds->priv;
-	u32 reg, val;
+	struct dsa_port *dp;
+	u32 reg, val, delay;
 	int ret;
 
 	switch (port) {
@@ -1252,17 +1187,50 @@ qca8k_phylink_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
 	case PHY_INTERFACE_MODE_RGMII_ID:
 	case PHY_INTERFACE_MODE_RGMII_TXID:
 	case PHY_INTERFACE_MODE_RGMII_RXID:
-		/* RGMII_ID needs internal delay. This is enabled through
-		 * PORT5_PAD_CTRL for all ports, rather than individual port
-		 * registers
+		dp = dsa_to_port(ds, port);
+		val = QCA8K_PORT_PAD_RGMII_EN;
+
+		if (state->interface == PHY_INTERFACE_MODE_RGMII_ID ||
+		    state->interface == PHY_INTERFACE_MODE_RGMII_TXID) {
+			if (of_property_read_u32(dp->dn, "tx-internal-delay-ps", &delay))
+				delay = 1;
+			else
+				/* Switch regs accept value in ns, convert ps to ns */
+				delay = delay / 1000;
+
+			if (delay > QCA8K_MAX_DELAY) {
+				dev_err(priv->dev, "rgmii tx delay is limited to a max value of 3ns, setting to the max value");
+				delay = 3;
+			}
+
+			val |= QCA8K_PORT_PAD_RGMII_TX_DELAY(delay) |
+			       QCA8K_PORT_PAD_RGMII_TX_DELAY_EN;
+		}
+
+		if (state->interface == PHY_INTERFACE_MODE_RGMII_ID ||
+		    state->interface == PHY_INTERFACE_MODE_RGMII_RXID) {
+			if (of_property_read_u32(dp->dn, "rx-internal-delay-ps", &delay))
+				delay = 2;
+			else
+				/* Switch regs accept value in ns, convert ps to ns */
+				delay = delay / 1000;
+
+			if (delay > QCA8K_MAX_DELAY) {
+				dev_err(priv->dev, "rgmii rx delay is limited to a max value of 3ns, setting to the max value");
+				delay = 3;
+			}
+
+			val |= QCA8K_PORT_PAD_RGMII_RX_DELAY(delay) |
+			       QCA8K_PORT_PAD_RGMII_RX_DELAY_EN;
+		}
+
+		/* Set RGMII delay based on the selected values */
+		qca8k_write(priv, reg, val);
+
+		/* QCA8337 requires to set rgmii rx delay for all ports.
+		 * This is enabled through PORT5_PAD_CTRL for all ports,
+		 * rather than individual port registers.
 		 */
-		qca8k_write(priv, reg,
-			    QCA8K_PORT_PAD_RGMII_EN |
-			    QCA8K_PORT_PAD_RGMII_TX_DELAY(priv->rgmii_tx_delay) |
-			    QCA8K_PORT_PAD_RGMII_RX_DELAY(priv->rgmii_rx_delay) |
-			    QCA8K_PORT_PAD_RGMII_TX_DELAY_EN |
-			    QCA8K_PORT_PAD_RGMII_RX_DELAY_EN);
-		/* QCA8337 requires to set rgmii rx delay */
 		if (priv->switch_id == QCA8K_ID_QCA8337)
 			qca8k_write(priv, QCA8K_REG_PORT5_PAD_CTRL,
 				    QCA8K_PORT_PAD_RGMII_RX_DELAY_EN);
diff --git a/drivers/net/dsa/qca8k.h b/drivers/net/dsa/qca8k.h
index 3fded69a6839..a36ef43e3847 100644
--- a/drivers/net/dsa/qca8k.h
+++ b/drivers/net/dsa/qca8k.h
@@ -261,8 +261,6 @@ struct qca8k_match_data {
 struct qca8k_priv {
 	u8 switch_id;
 	u8 switch_revision;
-	u8 rgmii_tx_delay;
-	u8 rgmii_rx_delay;
 	bool legacy_phy_port_mapping;
 	struct regmap *regmap;
 	struct mii_bus *bus;
-- 
2.32.0

[net-next PATCH v2 10/15] net: dsa: qca8k: add explicit SGMII PLL enable

[Ansuel Smith]

Support enabling PLL on the SGMII CPU port. Some device require this
special configuration or no traffic is transmitted and the switch
doesn't work at all. A dedicated binding is added to the CPU node
port to apply the correct reg on mac config.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 drivers/net/dsa/qca8k.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
index 05ecec4ebc01..8917bb154e8f 100644
--- a/drivers/net/dsa/qca8k.c
+++ b/drivers/net/dsa/qca8k.c
@@ -1237,6 +1237,8 @@ qca8k_phylink_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
 		break;
 	case PHY_INTERFACE_MODE_SGMII:
 	case PHY_INTERFACE_MODE_1000BASEX:
+		dp = dsa_to_port(ds, port);
+
 		/* Enable SGMII on the port */
 		qca8k_write(priv, reg, QCA8K_PORT_PAD_SGMII_EN);
 
@@ -1255,8 +1257,11 @@ qca8k_phylink_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
 		if (ret)
 			return;
 
-		val |= QCA8K_SGMII_EN_PLL | QCA8K_SGMII_EN_RX |
-			QCA8K_SGMII_EN_TX | QCA8K_SGMII_EN_SD;
+		val |= QCA8K_SGMII_EN_SD;
+
+		if (of_property_read_bool(dp->dn, "qca,sgmii-enable-pll"))
+			val |= QCA8K_SGMII_EN_PLL | QCA8K_SGMII_EN_RX |
+			       QCA8K_SGMII_EN_TX;
 
 		if (dsa_is_cpu_port(ds, port)) {
 			/* CPU port, we're talking to the CPU MAC, be a PHY */
-- 
2.32.0

[net-next PATCH v2 11/15] dt-bindings: net: dsa: qca8k: Document qca,sgmii-enable-pll

[Ansuel Smith]

Document qca,sgmii-enable-pll binding used in the CPU nodes to
enable SGMII PLL on MAC config.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 Documentation/devicetree/bindings/net/dsa/qca8k.txt | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/dsa/qca8k.txt b/Documentation/devicetree/bindings/net/dsa/qca8k.txt
index 208ee5bc1bbb..b9cccb657373 100644
--- a/Documentation/devicetree/bindings/net/dsa/qca8k.txt
+++ b/Documentation/devicetree/bindings/net/dsa/qca8k.txt
@@ -50,6 +50,12 @@ A CPU port node has the following optional node:
                           managed entity. See
                           Documentation/devicetree/bindings/net/fixed-link.txt
                           for details.
+- qca,sgmii-enable-pll  : For SGMII CPU port, explicitly enable PLL, TX and RX
+                          chain along with Signal Detection.
+                          This should NOT be enabled for qca8327.
+                          This can be required for qca8337 switch with revision 2.
+                          With CPU port set to sgmii and qca8337 it is advised
+                          to set this unless a communication problem is observed.
 
 For QCA8K the 'fixed-link' sub-node supports only the following properties:
 
-- 
2.32.0

[net-next PATCH v2 12/15] drivers: net: dsa: qca8k: add support for pws config reg

[Ansuel Smith]

Some qca8327 switch require to force the ignore of power on sel
strapping. Some switch require to set the led open drain mode in regs
instead of using strapping. While most of the device implements this
using the correct way using pin strapping, there are still some broken
device that require to be set using sw regs.
Introduce a new binding and support these special configuration.
As led open drain require to ignore pin strapping to work, the probe
fails with EINVAL error with incorrect configuration.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 drivers/net/dsa/qca8k.c | 39 +++++++++++++++++++++++++++++++++++++++
 drivers/net/dsa/qca8k.h |  6 ++++++
 2 files changed, 45 insertions(+)

diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
index 8917bb154e8f..0dc921cfb8c6 100644
--- a/drivers/net/dsa/qca8k.c
+++ b/drivers/net/dsa/qca8k.c
@@ -939,6 +939,41 @@ qca8k_setup_port0_pad_ctrl_reg(struct qca8k_priv *priv)
 	return ret;
 }
 
+static int
+qca8k_setup_of_pws_reg(struct qca8k_priv *priv)
+{
+	struct device_node *node = priv->dev->of_node;
+	u32 val = 0;
+	int ret;
+
+	/* QCA8327 require to set to the correct mode.
+	 * His bigger brother QCA8328 have the 172 pin layout.
+	 * Should be applied by default but we set this just to make sure.
+	 */
+	if (priv->switch_id == QCA8K_ID_QCA8327) {
+		ret = qca8k_rmw(priv, QCA8K_REG_PWS, QCA8327_PWS_PACKAGE148_EN,
+				QCA8327_PWS_PACKAGE148_EN);
+		if (ret)
+			return ret;
+	}
+
+	if (of_property_read_bool(node, "qca,ignore-power-on-sel"))
+		val |= QCA8K_PWS_POWER_ON_SEL;
+
+	if (of_property_read_bool(node, "qca,led-open-drain")) {
+		if (!(val & QCA8K_PWS_POWER_ON_SEL)) {
+			dev_err(priv->dev, "qca,led-open-drain require qca,ignore-power-on-sel to be set.");
+			return -EINVAL;
+		}
+
+		val |= QCA8K_PWS_LED_OPEN_EN_CSR;
+	}
+
+	return qca8k_rmw(priv, QCA8K_REG_PWS,
+			QCA8K_PWS_LED_OPEN_EN_CSR | QCA8K_PWS_POWER_ON_SEL,
+			val);
+}
+
 static int
 qca8k_setup(struct dsa_switch *ds)
 {
@@ -964,6 +999,10 @@ qca8k_setup(struct dsa_switch *ds)
 	if (ret)
 		return ret;
 
+	ret = qca8k_setup_of_pws_reg(priv);
+	if (ret)
+		return ret;
+
 	ret = qca8k_setup_mac_pwr_sel(priv);
 	if (ret)
 		return ret;
diff --git a/drivers/net/dsa/qca8k.h b/drivers/net/dsa/qca8k.h
index a36ef43e3847..2c98b133ec4f 100644
--- a/drivers/net/dsa/qca8k.h
+++ b/drivers/net/dsa/qca8k.h
@@ -48,6 +48,12 @@
 #define   QCA8K_MAX_DELAY				3
 #define   QCA8K_PORT_PAD_SGMII_EN			BIT(7)
 #define QCA8K_REG_PWS					0x010
+#define   QCA8K_PWS_POWER_ON_SEL			BIT(31)
+/* This reg is only valid for QCA832x and toggle the package
+ * type from 176 pin (by default) to 148 pin used on QCA8327
+ */
+#define   QCA8327_PWS_PACKAGE148_EN			BIT(30)
+#define   QCA8K_PWS_LED_OPEN_EN_CSR			BIT(24)
 #define   QCA8K_PWS_SERDES_AEN_DIS			BIT(7)
 #define QCA8K_REG_MODULE_EN				0x030
 #define   QCA8K_MODULE_EN_MIB				BIT(0)
-- 
2.32.0

[net-next PATCH v2 13/15] dt-bindings: net: dsa: qca8k: document open drain binding

[Ansuel Smith]

Document new binding qca,power_on_sel used to enable Power-on-strapping
select reg and qca,led_open_drain to set led to open drain mode.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 Documentation/devicetree/bindings/net/dsa/qca8k.txt | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/dsa/qca8k.txt b/Documentation/devicetree/bindings/net/dsa/qca8k.txt
index b9cccb657373..9fb4db65907e 100644
--- a/Documentation/devicetree/bindings/net/dsa/qca8k.txt
+++ b/Documentation/devicetree/bindings/net/dsa/qca8k.txt
@@ -13,6 +13,17 @@ Required properties:
 Optional properties:
 
 - reset-gpios: GPIO to be used to reset the whole device
+- qca,ignore-power-on-sel: Ignore power on pin strapping to configure led open
+                           drain or eeprom presence. This is needed for broken
+                           device that have wrong configuration or when the oem
+                           decided to not use pin strapping and fallback to sw
+                           regs.
+- qca,led-open-drain: Set leds to open-drain mode. This require the
+                      qca,ignore-power-on-sel to be set or the driver will fail
+                      to probe. This is needed if the oem doesn't use pin
+                      strapping to set this mode and prefer to set it using sw
+                      regs. The pin strapping related to led open drain mode is
+                      the pin B68 for QCA832x and B49 for QCA833x
 - qca,mac6-exchange: Internally swap MAC0 with MAC6.
 - qca,sgmii-rxclk-falling-edge: Set the receive clock phase to falling edge.
                                 Mostly used in qca8327 with CPU port 0 set to
-- 
2.32.0

[net-next PATCH v2 14/15] drivers: net: dsa: qca8k: add support for QCA8328

[Ansuel Smith]

QCA8328 switch is the bigger brother of the qca8327. Same regs different
chip. Change the function to set the correct pin layout and introduce a
new match_data to differentiate the 2 switch as they have the same ID
and their internal PHY have the same ID.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 drivers/net/dsa/qca8k.c | 19 ++++++++++++++++---
 drivers/net/dsa/qca8k.h |  1 +
 2 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
index 0dc921cfb8c6..aae0cfcd0ce8 100644
--- a/drivers/net/dsa/qca8k.c
+++ b/drivers/net/dsa/qca8k.c
@@ -943,6 +943,7 @@ static int
 qca8k_setup_of_pws_reg(struct qca8k_priv *priv)
 {
 	struct device_node *node = priv->dev->of_node;
+	const struct qca8k_match_data *data;
 	u32 val = 0;
 	int ret;
 
@@ -951,8 +952,14 @@ qca8k_setup_of_pws_reg(struct qca8k_priv *priv)
 	 * Should be applied by default but we set this just to make sure.
 	 */
 	if (priv->switch_id == QCA8K_ID_QCA8327) {
+		data = of_device_get_match_data(priv->dev);
+
+		/* Set the correct package of 148 pin for QCA8327 */
+		if (data->reduced_package)
+			val |= QCA8327_PWS_PACKAGE148_EN;
+
 		ret = qca8k_rmw(priv, QCA8K_REG_PWS, QCA8327_PWS_PACKAGE148_EN,
-				QCA8327_PWS_PACKAGE148_EN);
+				val);
 		if (ret)
 			return ret;
 	}
@@ -1994,7 +2001,12 @@ static int qca8k_resume(struct device *dev)
 static SIMPLE_DEV_PM_OPS(qca8k_pm_ops,
 			 qca8k_suspend, qca8k_resume);
 
-static const struct qca8k_match_data qca832x = {
+static const struct qca8k_match_data qca8327 = {
+	.id = QCA8K_ID_QCA8327,
+	.reduced_package = true,
+};
+
+static const struct qca8k_match_data qca8328 = {
 	.id = QCA8K_ID_QCA8327,
 };
 
@@ -2003,7 +2015,8 @@ static const struct qca8k_match_data qca833x = {
 };
 
 static const struct of_device_id qca8k_of_match[] = {
-	{ .compatible = "qca,qca8327", .data = &qca832x },
+	{ .compatible = "qca,qca8327", .data = &qca8327 },
+	{ .compatible = "qca,qca8328", .data = &qca8328 },
 	{ .compatible = "qca,qca8334", .data = &qca833x },
 	{ .compatible = "qca,qca8337", .data = &qca833x },
 	{ /* sentinel */ },
diff --git a/drivers/net/dsa/qca8k.h b/drivers/net/dsa/qca8k.h
index 2c98b133ec4f..2d0c41e8cb75 100644
--- a/drivers/net/dsa/qca8k.h
+++ b/drivers/net/dsa/qca8k.h
@@ -262,6 +262,7 @@ struct ar8xxx_port_status {
 
 struct qca8k_match_data {
 	u8 id;
+	bool reduced_package;
 };
 
 struct qca8k_priv {
-- 
2.32.0

[net-next PATCH v2 15/15] dt-bindings: net: dsa: qca8k: document support for qca8328

[Ansuel Smith]

QCA8328 is the birrget brother of 8327. Document the new compatible
binding.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 Documentation/devicetree/bindings/net/dsa/qca8k.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/net/dsa/qca8k.txt b/Documentation/devicetree/bindings/net/dsa/qca8k.txt
index 9fb4db65907e..0e84500b8db2 100644
--- a/Documentation/devicetree/bindings/net/dsa/qca8k.txt
+++ b/Documentation/devicetree/bindings/net/dsa/qca8k.txt
@@ -3,6 +3,7 @@
 Required properties:
 
 - compatible: should be one of:
+    "qca,qca8328"
     "qca,qca8327"
     "qca,qca8334"
     "qca,qca8337"
-- 
2.32.0

Re: [net PATCH v2 01/15] drivers: net: phy: at803x: fix resume for QCA8327 phy

[Jakub Kicinski]

On Fri,  8 Oct 2021 02:22:11 +0200 Ansuel Smith wrote:
> From Documentation phy resume triggers phy reset and restart
> auto-negotiation. Add a dedicated function to wait reset to finish as
> it was notice a regression where port sometime are not reliable after a
> suspend/resume session. The reset wait logic is copied from phy_poll_reset.
> Add dedicated suspend function to use genphy_suspend only with QCA8337
> phy and set only additional debug settings for QCA8327. With more test
> it was reported that QCA8327 doesn't proprely support this mode and
> using this cause the unreliability of the switch ports, especially the
> malfunction of the port0.
> 
> Fixes: 52a6cdbe43a3 ("net: phy: at803x: add resume/suspend function to qca83xx phy")

Strange, checkpatch catches the wrong hash being used, but the
verify_fixes script doesn't. Did you mean:

Fixes: 15b9df4ece17 ("net: phy: at803x: add resume/suspend function to qca83xx phy")

Or is 52a6cdbe43a3 the correct commit hash? Same question for patch 2.


The fixes have to be a _separate_ series.

Re: [net PATCH v2 01/15] drivers: net: phy: at803x: fix resume for QCA8327 phy

[Ansuel Smith]

On Thu, Oct 07, 2021 at 07:23:04PM -0700, Jakub Kicinski wrote:
> On Fri,  8 Oct 2021 02:22:11 +0200 Ansuel Smith wrote:
> > From Documentation phy resume triggers phy reset and restart
> > auto-negotiation. Add a dedicated function to wait reset to finish as
> > it was notice a regression where port sometime are not reliable after a
> > suspend/resume session. The reset wait logic is copied from phy_poll_reset.
> > Add dedicated suspend function to use genphy_suspend only with QCA8337
> > phy and set only additional debug settings for QCA8327. With more test
> > it was reported that QCA8327 doesn't proprely support this mode and
> > using this cause the unreliability of the switch ports, especially the
> > malfunction of the port0.
> > 
> > Fixes: 52a6cdbe43a3 ("net: phy: at803x: add resume/suspend function to qca83xx phy")
> 
> Strange, checkpatch catches the wrong hash being used, but the
> verify_fixes script doesn't. Did you mean:
> 
> Fixes: 15b9df4ece17 ("net: phy: at803x: add resume/suspend function to qca83xx phy")
> 
> Or is 52a6cdbe43a3 the correct commit hash? Same question for patch 2.
> 
> 
> The fixes have to be a _separate_ series.

Hi,
this series contains changes that depends on the fixes. (the 4th patch
that rename the define is based on this 2 patch) How to handle that?
I know it was wrong to put net and net-next patch in the same series but
I don't know how to handle this strange situation. Any hint about that?

About the wrong hash, yes I wrongly took the hash from my local branch.

-- 
	Ansuel

Re: [net PATCH v2 01/15] drivers: net: phy: at803x: fix resume for QCA8327 phy

[Jakub Kicinski]

On Fri, 8 Oct 2021 10:45:51 +0200 Ansuel Smith wrote:
> On Thu, Oct 07, 2021 at 07:23:04PM -0700, Jakub Kicinski wrote:
> > On Fri,  8 Oct 2021 02:22:11 +0200 Ansuel Smith wrote:  
> > > From Documentation phy resume triggers phy reset and restart
> > > auto-negotiation. Add a dedicated function to wait reset to finish as
> > > it was notice a regression where port sometime are not reliable after a
> > > suspend/resume session. The reset wait logic is copied from phy_poll_reset.
> > > Add dedicated suspend function to use genphy_suspend only with QCA8337
> > > phy and set only additional debug settings for QCA8327. With more test
> > > it was reported that QCA8327 doesn't proprely support this mode and
> > > using this cause the unreliability of the switch ports, especially the
> > > malfunction of the port0.
> > > 
> > > Fixes: 52a6cdbe43a3 ("net: phy: at803x: add resume/suspend function to qca83xx phy")  
> > 
> > Strange, checkpatch catches the wrong hash being used, but the
> > verify_fixes script doesn't. Did you mean:
> > 
> > Fixes: 15b9df4ece17 ("net: phy: at803x: add resume/suspend function to qca83xx phy")
> > 
> > Or is 52a6cdbe43a3 the correct commit hash? Same question for patch 2.
> > 
> > 
> > The fixes have to be a _separate_ series.  
> 
> this series contains changes that depends on the fixes. (the 4th patch
> that rename the define is based on this 2 patch) How to handle that?
> I know it was wrong to put net and net-next patch in the same series but
> I don't know how to handle this strange situation. Any hint about that?

If there is a functional dependency you'll need to send the net changes
first and then wait until the trees are merged before sending net-next
changes. Merge usually happens Thursday afternoon (pacific). You can
post the net-next changes as RFC before the trees get merged to make
sure they are reviewed and ready to go in.

> About the wrong hash, yes I wrongly took the hash from my local branch.

Indeed, looks like our checker got broken hence my confusion.

Re: [net-next PATCH v2 04/15] drivers: net: phy: at803x: better describe debug regs

[Andrew Lunn]

On Fri, Oct 08, 2021 at 02:22:14AM +0200, Ansuel Smith wrote:
> Give a name to known debug regs from Documentation instead of using
> unknown hex values.
> 
> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [net-next PATCH v2 06/15] dt-bindings: net: dsa: qca8k: document rgmii_1_8v bindings

[Andrew Lunn]

On Fri, Oct 08, 2021 at 02:22:16AM +0200, Ansuel Smith wrote:
> Document new qca,rgmii0_1_8v and qca,rgmii56_1_8v needed to setup
> mac_pwr_sel register for qca8337 switch. Specific the use of this binding
> that is used only in qca8337 and not in qca8327.
> 
> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> ---
>  Documentation/devicetree/bindings/net/dsa/qca8k.txt | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/dsa/qca8k.txt b/Documentation/devicetree/bindings/net/dsa/qca8k.txt
> index 8c73f67c43ca..9383d6bf2426 100644
> --- a/Documentation/devicetree/bindings/net/dsa/qca8k.txt
> +++ b/Documentation/devicetree/bindings/net/dsa/qca8k.txt
> @@ -13,6 +13,14 @@ Required properties:
>  Optional properties:
>  
>  - reset-gpios: GPIO to be used to reset the whole device
> +- qca,rgmii0-1-8v: Set the internal regulator to supply 1.8v for MAC0 port.
> +                   This is needed for qca8337 and toggles the supply voltage
> +                   from 1.5v to 1.8v. For the specific regs it was observed
> +                   that this is needed only for ipq8064 and ipq8065 target.
> +- qca,rgmii56-1-8v: Set the internal regulator to supply 1.8v for MAC5/6 port.
> +                    This is needed for qca8337 and toggles the supply voltage
> +                    from 1.5v to 1.8v. For the specific regs it was observed
> +                    that this is needed only for ipq8065 target.

Are ipq8065 & ipq8064 SoCs which the switch is embedded into? So you
could look for the top level compatible and set these regulators based
on that. No DT property needed.

   Andrew

Re: [net-next PATCH v2 06/15] dt-bindings: net: dsa: qca8k: document rgmii_1_8v bindings

[Ansuel Smith]

On Sat, Oct 09, 2021 at 05:18:55PM +0200, Andrew Lunn wrote:
> On Fri, Oct 08, 2021 at 02:22:16AM +0200, Ansuel Smith wrote:
> > Document new qca,rgmii0_1_8v and qca,rgmii56_1_8v needed to setup
> > mac_pwr_sel register for qca8337 switch. Specific the use of this binding
> > that is used only in qca8337 and not in qca8327.
> > 
> > Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> > ---
> >  Documentation/devicetree/bindings/net/dsa/qca8k.txt | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/net/dsa/qca8k.txt b/Documentation/devicetree/bindings/net/dsa/qca8k.txt
> > index 8c73f67c43ca..9383d6bf2426 100644
> > --- a/Documentation/devicetree/bindings/net/dsa/qca8k.txt
> > +++ b/Documentation/devicetree/bindings/net/dsa/qca8k.txt
> > @@ -13,6 +13,14 @@ Required properties:
> >  Optional properties:
> >  
> >  - reset-gpios: GPIO to be used to reset the whole device
> > +- qca,rgmii0-1-8v: Set the internal regulator to supply 1.8v for MAC0 port.
> > +                   This is needed for qca8337 and toggles the supply voltage
> > +                   from 1.5v to 1.8v. For the specific regs it was observed
> > +                   that this is needed only for ipq8064 and ipq8065 target.
> > +- qca,rgmii56-1-8v: Set the internal regulator to supply 1.8v for MAC5/6 port.
> > +                    This is needed for qca8337 and toggles the supply voltage
> > +                    from 1.5v to 1.8v. For the specific regs it was observed
> > +                    that this is needed only for ipq8065 target.
> 
> Are ipq8065 & ipq8064 SoCs which the switch is embedded into? So you
> could look for the top level compatible and set these regulators based
> on that. No DT property needed.
> 
>    Andrew

The switch is still external for these 2 SoC. If we really want, yes we
can follow that route and sets only for the 2 SoC. (Considering ipq8065
is still not present, can I add it anyway in the qca8k code? Will for
sure propose the ipq8065 dtsi today)

-- 
	Ansuel

Re: [net-next PATCH v2 08/15] dt-bindings: net: dsa: qca8k: Add MAC swap and clock phase properties

[Andrew Lunn]

On Fri, Oct 08, 2021 at 02:22:18AM +0200, Ansuel Smith wrote:
> Add names and decriptions of additional PORT0_PAD_CTRL properties.
> Document new binding qca,mac6_exchange that exchange the mac0 port
> with mac6.
> qca,sgmii-(rx|tx)clk-falling-edge are for setting the respective clock
> phase to failling edge.
> 
> Signed-off-by: Matthew Hagan <mnhagan88@gmail.com>
> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> ---
>  Documentation/devicetree/bindings/net/dsa/qca8k.txt | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/dsa/qca8k.txt b/Documentation/devicetree/bindings/net/dsa/qca8k.txt
> index 9383d6bf2426..208ee5bc1bbb 100644
> --- a/Documentation/devicetree/bindings/net/dsa/qca8k.txt
> +++ b/Documentation/devicetree/bindings/net/dsa/qca8k.txt
> @@ -13,6 +13,11 @@ Required properties:
>  Optional properties:
>  
>  - reset-gpios: GPIO to be used to reset the whole device
> +- qca,mac6-exchange: Internally swap MAC0 with MAC6.
> +- qca,sgmii-rxclk-falling-edge: Set the receive clock phase to falling edge.
> +                                Mostly used in qca8327 with CPU port 0 set to
> +                                sgmii.
> +- qca,sgmii-txclk-falling-edge: Set the transmit clock phase to falling edge.
>  - qca,rgmii0-1-8v: Set the internal regulator to supply 1.8v for MAC0 port.
>                     This is needed for qca8337 and toggles the supply voltage
>                     from 1.5v to 1.8v. For the specific regs it was observed

The edge configuration is a port configuration. So it should be inside
the port DT node it applies to. That also gives a clean way forward
when a new switch appears with more SGMII interfaces, each with its
own edge configuration.

But that then leads into the MAC0/MAC6 swap mess. I need to think
about that some more, how do we cleanly describe that in DT.

      Andrew

Re: [net-next PATCH v2 11/15] dt-bindings: net: dsa: qca8k: Document qca,sgmii-enable-pll

[Andrew Lunn]

On Fri, Oct 08, 2021 at 02:22:21AM +0200, Ansuel Smith wrote:
> Document qca,sgmii-enable-pll binding used in the CPU nodes to
> enable SGMII PLL on MAC config.
> 
> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> ---
>  Documentation/devicetree/bindings/net/dsa/qca8k.txt | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/dsa/qca8k.txt b/Documentation/devicetree/bindings/net/dsa/qca8k.txt
> index 208ee5bc1bbb..b9cccb657373 100644
> --- a/Documentation/devicetree/bindings/net/dsa/qca8k.txt
> +++ b/Documentation/devicetree/bindings/net/dsa/qca8k.txt
> @@ -50,6 +50,12 @@ A CPU port node has the following optional node:
>                            managed entity. See
>                            Documentation/devicetree/bindings/net/fixed-link.txt
>                            for details.
> +- qca,sgmii-enable-pll  : For SGMII CPU port, explicitly enable PLL, TX and RX
> +                          chain along with Signal Detection.
> +                          This should NOT be enabled for qca8327.

So how about -EINVAL for qca8327, and document it is not valid then.

> +                          This can be required for qca8337 switch with revision 2.

Maybe add a warning if enabled with revision < 2? I would not make it
an error, because there could be devices manufactured with a mixture
or v1 and v2 silicon. Do you have any idea how wide spread v1 is?

> +                          With CPU port set to sgmii and qca8337 it is advised
> +                          to set this unless a communication problem is observed.

  Andrew

Re: [net-next PATCH v2 13/15] dt-bindings: net: dsa: qca8k: document open drain binding

[Andrew Lunn]

On Fri, Oct 08, 2021 at 02:22:23AM +0200, Ansuel Smith wrote:
> Document new binding qca,power_on_sel used to enable Power-on-strapping
> select reg and qca,led_open_drain to set led to open drain mode.
> 
> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> ---
>  Documentation/devicetree/bindings/net/dsa/qca8k.txt | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/dsa/qca8k.txt b/Documentation/devicetree/bindings/net/dsa/qca8k.txt
> index b9cccb657373..9fb4db65907e 100644
> --- a/Documentation/devicetree/bindings/net/dsa/qca8k.txt
> +++ b/Documentation/devicetree/bindings/net/dsa/qca8k.txt
> @@ -13,6 +13,17 @@ Required properties:
>  Optional properties:
>  
>  - reset-gpios: GPIO to be used to reset the whole device
> +- qca,ignore-power-on-sel: Ignore power on pin strapping to configure led open
> +                           drain or eeprom presence.

So strapping is only used for LEDs and EEPROM presence? Nothing else?
Seems link MAC0/MAC6 swap would be a good candidate for strapping?

I just want to make it clear that if you select this option, you need
to take care of X, Y and Z in DT.

	Andrew

Re: [net-next PATCH v2 15/15] dt-bindings: net: dsa: qca8k: document support for qca8328

[Andrew Lunn]

On Fri, Oct 08, 2021 at 02:22:25AM +0200, Ansuel Smith wrote:
> QCA8328 is the birrget brother of 8327. Document the new compatible

birrget?



> binding.
> 
> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> ---
>  Documentation/devicetree/bindings/net/dsa/qca8k.txt | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/dsa/qca8k.txt b/Documentation/devicetree/bindings/net/dsa/qca8k.txt
> index 9fb4db65907e..0e84500b8db2 100644
> --- a/Documentation/devicetree/bindings/net/dsa/qca8k.txt
> +++ b/Documentation/devicetree/bindings/net/dsa/qca8k.txt
> @@ -3,6 +3,7 @@
>  Required properties:
>  
>  - compatible: should be one of:
> +    "qca,qca8328"
>      "qca,qca8327"
>      "qca,qca8334"
>      "qca,qca8337"

This is much nice than the old DT property. But since the internal IDs
are the same, i think it would be good to add a little documentation
here about how the 8327 and 8328 differ, since most people are not
going to look at the commit message.

      Andrew

Re: [net-next PATCH v2 06/15] dt-bindings: net: dsa: qca8k: document rgmii_1_8v bindings

[Andrew Lunn]

> > Are ipq8065 & ipq8064 SoCs which the switch is embedded into? So you
> > could look for the top level compatible and set these regulators based
> > on that. No DT property needed.
> > 
> >    Andrew
> 
> The switch is still external for these 2 SoC. If we really want, yes we
> can follow that route and sets only for the 2 SoC. (Considering ipq8065
> is still not present, can I add it anyway in the qca8k code? Will for
> sure propose the ipq8065 dtsi today)

It seems like this is less error prone. If the properties really are
needed, because somebody creates a board with swapped SoC and Switch,
the properties can be added later.

    Andrew

Re: [net-next PATCH v2 08/15] dt-bindings: net: dsa: qca8k: Add MAC swap and clock phase properties

[Ansuel Smith]

On Sat, Oct 09, 2021 at 07:07:16PM +0200, Andrew Lunn wrote:
> On Fri, Oct 08, 2021 at 02:22:18AM +0200, Ansuel Smith wrote:
> > Add names and decriptions of additional PORT0_PAD_CTRL properties.
> > Document new binding qca,mac6_exchange that exchange the mac0 port
> > with mac6.
> > qca,sgmii-(rx|tx)clk-falling-edge are for setting the respective clock
> > phase to failling edge.
> > 
> > Signed-off-by: Matthew Hagan <mnhagan88@gmail.com>
> > Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> > ---
> >  Documentation/devicetree/bindings/net/dsa/qca8k.txt | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/net/dsa/qca8k.txt b/Documentation/devicetree/bindings/net/dsa/qca8k.txt
> > index 9383d6bf2426..208ee5bc1bbb 100644
> > --- a/Documentation/devicetree/bindings/net/dsa/qca8k.txt
> > +++ b/Documentation/devicetree/bindings/net/dsa/qca8k.txt
> > @@ -13,6 +13,11 @@ Required properties:
> >  Optional properties:
> >  
> >  - reset-gpios: GPIO to be used to reset the whole device
> > +- qca,mac6-exchange: Internally swap MAC0 with MAC6.
> > +- qca,sgmii-rxclk-falling-edge: Set the receive clock phase to falling edge.
> > +                                Mostly used in qca8327 with CPU port 0 set to
> > +                                sgmii.
> > +- qca,sgmii-txclk-falling-edge: Set the transmit clock phase to falling edge.
> >  - qca,rgmii0-1-8v: Set the internal regulator to supply 1.8v for MAC0 port.
> >                     This is needed for qca8337 and toggles the supply voltage
> >                     from 1.5v to 1.8v. For the specific regs it was observed
> 
> The edge configuration is a port configuration. So it should be inside
> the port DT node it applies to. That also gives a clean way forward
> when a new switch appears with more SGMII interfaces, each with its
> own edge configuration.
> 

Problem here is that from Documentation falling edge can be set only on
PAD0. PAD5 and PAD6 have the related bit reserved.
But anyway qca8k support only single sgmii and it's not supported a
config with multiple sgmii. Do we have standard binding for this?

> But that then leads into the MAC0/MAC6 swap mess. I need to think
> about that some more, how do we cleanly describe that in DT.
> 
>       Andrew

About the mac swap. Do we really need to implement a complex thing for
something that is really implemented internally to the switch? With this
option MAC6 is swapped with MAC0. But with the port configuration in DT
it doesn't change anything. Same reg, no change. It's really that some
OEM connect the secondary port instead of the primary (for some reason,
hw choice?) and swap them internally.

Anyway some question.
I will move the falling binding to the port DT node and move the
configuration to mac_config. Should I keep the
dedicated function to setup PAD0 swap or I can directly add the check in
the qca8k_setup for only the bit related to enable the swap?

-- 
	Ansuel

Re: [net-next PATCH v2 11/15] dt-bindings: net: dsa: qca8k: Document qca,sgmii-enable-pll

[Ansuel Smith]

On Sat, Oct 09, 2021 at 07:13:58PM +0200, Andrew Lunn wrote:
> On Fri, Oct 08, 2021 at 02:22:21AM +0200, Ansuel Smith wrote:
> > Document qca,sgmii-enable-pll binding used in the CPU nodes to
> > enable SGMII PLL on MAC config.
> > 
> > Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> > ---
> >  Documentation/devicetree/bindings/net/dsa/qca8k.txt | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/net/dsa/qca8k.txt b/Documentation/devicetree/bindings/net/dsa/qca8k.txt
> > index 208ee5bc1bbb..b9cccb657373 100644
> > --- a/Documentation/devicetree/bindings/net/dsa/qca8k.txt
> > +++ b/Documentation/devicetree/bindings/net/dsa/qca8k.txt
> > @@ -50,6 +50,12 @@ A CPU port node has the following optional node:
> >                            managed entity. See
> >                            Documentation/devicetree/bindings/net/fixed-link.txt
> >                            for details.
> > +- qca,sgmii-enable-pll  : For SGMII CPU port, explicitly enable PLL, TX and RX
> > +                          chain along with Signal Detection.
> > +                          This should NOT be enabled for qca8327.
> 
> So how about -EINVAL for qca8327, and document it is not valid then.
>

I would also add a warning. With all the ported device we found pll
needed only qca8337. I will add the error but also report the reason as
we really don't know if it does exist a qca8327 device that needs pll.
In theory not but who knows.

> > +                          This can be required for qca8337 switch with revision 2.
> 
> Maybe add a warning if enabled with revision < 2? I would not make it
> an error, because there could be devices manufactured with a mixture
> or v1 and v2 silicon. Do you have any idea how wide spread v1 is?
> 

No idea about the revision and can't be recovered from the switch data
print on the chip. Will add a warning and put in the documentation that
we warn when an uncorrect revision is detected.

> > +                          With CPU port set to sgmii and qca8337 it is advised
> > +                          to set this unless a communication problem is observed.
> 
>   Andrew

-- 
	Ansuel

Re: [net-next PATCH v2 15/15] dt-bindings: net: dsa: qca8k: document support for qca8328

[Ansuel Smith]

On Sat, Oct 09, 2021 at 07:24:45PM +0200, Andrew Lunn wrote:
> On Fri, Oct 08, 2021 at 02:22:25AM +0200, Ansuel Smith wrote:
> > QCA8328 is the birrget brother of 8327. Document the new compatible
> 
> birrget?
> 
> 
>

Me sending patch lat at night... it was brother.

> > binding.
> > 
> > Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> > ---
> >  Documentation/devicetree/bindings/net/dsa/qca8k.txt | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/net/dsa/qca8k.txt b/Documentation/devicetree/bindings/net/dsa/qca8k.txt
> > index 9fb4db65907e..0e84500b8db2 100644
> > --- a/Documentation/devicetree/bindings/net/dsa/qca8k.txt
> > +++ b/Documentation/devicetree/bindings/net/dsa/qca8k.txt
> > @@ -3,6 +3,7 @@
> >  Required properties:
> >  
> >  - compatible: should be one of:
> > +    "qca,qca8328"
> >      "qca,qca8327"
> >      "qca,qca8334"
> >      "qca,qca8337"
> 
> This is much nice than the old DT property. But since the internal IDs
> are the same, i think it would be good to add a little documentation
> here about how the 8327 and 8328 differ, since most people are not
> going to look at the commit message.
> 
>       Andrew

Ok will add some description on how to understand the correct compatible
to use.

-- 
	Ansuel

Re: [net-next PATCH v2 08/15] dt-bindings: net: dsa: qca8k: Add MAC swap and clock phase properties

[Andrew Lunn]

> Problem here is that from Documentation falling edge can be set only on
> PAD0. PAD5 and PAD6 have the related bit reserved.

Meaning in future, they could be used for this, if those ports get
support for SGMII.

> But anyway qca8k support only single sgmii and it's not supported a
> config with multiple sgmii.

Yet, until such hardware appears. We do see more support for SFPs. And
more support for multi-gigi ports. Both of which use a SERDES
interface which can support SGMII. So i would not be too surprised if
future versions of the switch have more ports like this.

> Do we have standard binding for this?

No, there is no standard binding for this. This seems specific to
these devices, maybe a proprietary extension to SGMII?

> About the mac swap. Do we really need to implement a complex thing for
> something that is really implemented internally to the switch?

If it was truly internal to the switch, no. But i don't think it
is. The DSA core has no idea the ports are swapped, and so i think
will put the names on the wrong ports. Does devlink understand the
ports are swapped? How about .ndo_get_phys_port_name? Will udev mix up
the ports?

The way you wanted to look in the other ports DT properties suggests
it is not internal to the switch.

I think to help my understanding, we need some examples of DTS files
with and without the swap, where the properties are read from, what
the interface names are, etc.

> I will move the falling binding to the port DT node and move the
> configuration to mac_config. Should I keep the
> dedicated function to setup PAD0 swap or I can directly add the check in
> the qca8k_setup for only the bit related to enable the swap?

That does not matter too much. DT is an ABI, we should not change it
later, so we need to look forward. C code is not ABI, it can be
changed if/when more SGMII ports actually arrive.

	Andrew

Re: [net-next PATCH v2 08/15] dt-bindings: net: dsa: qca8k: Add MAC swap and clock phase properties

[Ansuel Smith]

On Sat, Oct 09, 2021 at 09:47:57PM +0200, Andrew Lunn wrote:
> > Problem here is that from Documentation falling edge can be set only on
> > PAD0. PAD5 and PAD6 have the related bit reserved.
> 
> Meaning in future, they could be used for this, if those ports get
> support for SGMII.
>

Ok. Then I will move all to DT port. Consider falling is set to PAD0 for
both cpu port0 and cpu port6, should I make it hardcoded or should I add
a condition and force the reg to PAD0 only for the current supported
switch? Hope you understand what I mean.
(Yes we have some config with cpu port6 set to sgmii and that require
falling edge)

> > But anyway qca8k support only single sgmii and it's not supported a
> > config with multiple sgmii.
> 
> Yet, until such hardware appears. We do see more support for SFPs. And
> more support for multi-gigi ports. Both of which use a SERDES
> interface which can support SGMII. So i would not be too surprised if
> future versions of the switch have more ports like this.
> 
> > Do we have standard binding for this?
> 
> No, there is no standard binding for this. This seems specific to
> these devices, maybe a proprietary extension to SGMII?
> 

Then we are stuck to the special qca,... naming.

> > About the mac swap. Do we really need to implement a complex thing for
> > something that is really implemented internally to the switch?
> 
> If it was truly internal to the switch, no. But i don't think it
> is. The DSA core has no idea the ports are swapped, and so i think
> will put the names on the wrong ports. Does devlink understand the
> ports are swapped? How about .ndo_get_phys_port_name? Will udev mix up
> the ports?
> 
> The way you wanted to look in the other ports DT properties suggests
> it is not internal to the switch.
> 
> I think to help my understanding, we need some examples of DTS files
> with and without the swap, where the properties are read from, what
> the interface names are, etc.
> 

Here is 2 configuration one from an Netgear r7800 qca8337:

	switch@10 {
		compatible = "qca,qca8337";
		#address-cells = <1>;
		#size-cells = <0>;
		reg = <0x10>;

		qca8k,rgmii0_1_8v;
		qca8k,rgmii56_1_8v;

		ports {
			#address-cells = <1>;
			#size-cells = <0>;

			port@0 {
				reg = <0>;
				label = "cpu";
				ethernet = <&gmac1>;
				phy-mode = "rgmii-id";

				fixed-link {
					speed = <1000>;
					full-duplex;
				};
			};

			port@1 {
				reg = <1>;
				label = "lan1";
				phy-mode = "internal";
				phy-handle = <&phy_port1>;
			};

			port@2 {
				reg = <2>;
				label = "lan2";
				phy-mode = "internal";
				phy-handle = <&phy_port2>;
			};

			port@3 {
				reg = <3>;
				label = "lan3";
				phy-mode = "internal";
				phy-handle = <&phy_port3>;
			};

			port@4 {
				reg = <4>;
				label = "lan4";
				phy-mode = "internal";
				phy-handle = <&phy_port4>;
			};

			port@5 {
				reg = <5>;
				label = "wan";
				phy-mode = "internal";
				phy-handle = <&phy_port5>;
			};

			port@6 {
				reg = <6>;
				label = "cpu";
				ethernet = <&gmac2>;
				phy-mode = "sgmii";

				fixed-link {
					speed = <1000>;
					full-duplex;
				};
			};
		};

And here is one with mac swap Tp-Link Archer c7 v4 qca8327 

	switch0@10 {
		compatible = "qca,qca8337";
		#address-cells = <1>;
		#size-cells = <0>;

		reg = <0>;
		qca,sgmii-rxclk-falling-edge;
		qca,mac6-exchange;

		ports {
			#address-cells = <1>;
			#size-cells = <0>;

			port@0 {
				reg = <0>;
				label = "cpu";
				ethernet = <&eth0>;
				phy-mode = "sgmii";

				fixed-link {
					speed = <1000>;
					full-duplex;
				};
			};

			port@1 {
				reg = <1>;
				label = "wan";
				phy-mode = "internal";
				phy-handle = <&phy_port1>;
			};

			port@2 {
				reg = <2>;
				label = "lan1";
				phy-mode = "internal";
				phy-handle = <&phy_port2>;
			};

			port@3 {
				reg = <3>;
				label = "lan2";
				phy-mode = "internal";
				phy-handle = <&phy_port3>;
			};

			port@4 {
				reg = <4>;
				label = "lan3";
				phy-mode = "internal";
				phy-handle = <&phy_port4>;
			};

			port@5 {
				reg = <5>;
				label = "lan4";
				phy-mode = "internal";
				phy-handle = <&phy_port5>;
			};
		};

As you can see we use the mac06_exchange but we declare it as port0. DSA
see it as port0 (as it should as it's internall swapped). We also don't
need to apply some special function or stuff to apply the correct port
in the ACL/regs/VLAN. The switch will treat MAC6 as MAC0 and MAC6 as
MAC0. That is what we observed.
Someone would say... Considering this switch is 2 port... and currently
we use only one port. Why not drop this and use whatever is connected to
port 0. Problem is that some device have the secondary cpu port NOT
connected and they use mac6-exchange and that would result in no
connection since cpu port 0 is not connected and cpu port 6 is never
swapped.

> > I will move the falling binding to the port DT node and move the
> > configuration to mac_config. Should I keep the
> > dedicated function to setup PAD0 swap or I can directly add the check in
> > the qca8k_setup for only the bit related to enable the swap?
> 
> That does not matter too much. DT is an ABI, we should not change it
> later, so we need to look forward. C code is not ABI, it can be
> changed if/when more SGMII ports actually arrive.
> 
> 	Andrew

-- 
	Ansuel

Re: [net-next PATCH v2 08/15] dt-bindings: net: dsa: qca8k: Add MAC swap and clock phase properties

[Andrew Lunn]

> Here is 2 configuration one from an Netgear r7800 qca8337:
> 
> 	switch@10 {
> 		compatible = "qca,qca8337";
> 		#address-cells = <1>;
> 		#size-cells = <0>;
> 		reg = <0x10>;
> 
> 		qca8k,rgmii0_1_8v;
> 		qca8k,rgmii56_1_8v;
> 
> 		ports {
> 			#address-cells = <1>;
> 			#size-cells = <0>;
> 
> 			port@0 {
> 				reg = <0>;
> 				label = "cpu";
> 				ethernet = <&gmac1>;
> 				phy-mode = "rgmii-id";
> 
> 				fixed-link {
> 					speed = <1000>;
> 					full-duplex;
> 				};
> 			};
> 
> 			port@1 {
> 				reg = <1>;
> 				label = "lan1";
> 				phy-mode = "internal";
> 				phy-handle = <&phy_port1>;
> 			};
> 
> 			port@2 {
> 				reg = <2>;
> 				label = "lan2";
> 				phy-mode = "internal";
> 				phy-handle = <&phy_port2>;
> 			};
> 
> 			port@3 {
> 				reg = <3>;
> 				label = "lan3";
> 				phy-mode = "internal";
> 				phy-handle = <&phy_port3>;
> 			};
> 
> 			port@4 {
> 				reg = <4>;
> 				label = "lan4";
> 				phy-mode = "internal";
> 				phy-handle = <&phy_port4>;
> 			};
> 
> 			port@5 {
> 				reg = <5>;
> 				label = "wan";
> 				phy-mode = "internal";
> 				phy-handle = <&phy_port5>;
> 			};
> 
> 			port@6 {
> 				reg = <6>;
> 				label = "cpu";
> 				ethernet = <&gmac2>;
> 				phy-mode = "sgmii";
> 
> 				fixed-link {
> 					speed = <1000>;
> 					full-duplex;
> 				};

So here, it is a second CPU port.  But some other board could connect
an SGMII PHY, and call the port lan5. Or it could be connected to an
SFP cage, and used that way. Or are you forced to use it as a CPU
port, or not use it at all?

> And here is one with mac swap Tp-Link Archer c7 v4 qca8327 
> 
> 	switch0@10 {
> 		compatible = "qca,qca8337";
> 		#address-cells = <1>;
> 		#size-cells = <0>;
> 
> 		reg = <0>;
> 		qca,sgmii-rxclk-falling-edge;
> 		qca,mac6-exchange;
> 
> 		ports {
> 			#address-cells = <1>;
> 			#size-cells = <0>;
> 
> 			port@0 {
> 				reg = <0>;
> 				label = "cpu";
> 				ethernet = <&eth0>;
> 				phy-mode = "sgmii";
> 
> 				fixed-link {
> 					speed = <1000>;
> 					full-duplex;
> 				};

So when looking for SGMI properties, you need to look here. Where as
in the previous example, you would look in port 6. And the reverse is
true for RGMII delays.

> 			};
> 
> 			port@1 {
> 				reg = <1>;
> 				label = "wan";
> 				phy-mode = "internal";
> 				phy-handle = <&phy_port1>;
> 			};
> 
> 			port@2 {
> 				reg = <2>;
> 				label = "lan1";
> 				phy-mode = "internal";
> 				phy-handle = <&phy_port2>;
> 			};
> 
> 			port@3 {
> 				reg = <3>;
> 				label = "lan2";
> 				phy-mode = "internal";
> 				phy-handle = <&phy_port3>;
> 			};
> 
> 			port@4 {
> 				reg = <4>;
> 				label = "lan3";
> 				phy-mode = "internal";
> 				phy-handle = <&phy_port4>;
> 			};
> 
> 			port@5 {
> 				reg = <5>;
> 				label = "lan4";
> 				phy-mode = "internal";
> 				phy-handle = <&phy_port5>;
> 			};
> 		};

So here, port '6' is not used. But it could be connected to an RGMII
PHY and called lan5. Would the naming work out? What does devlink
think of it, etc. What about phy-handle? Is there an external MDIO
bus? What address would be used if there is no phy-handle?

      Andrew

Re: [net-next PATCH v2 08/15] dt-bindings: net: dsa: qca8k: Add MAC swap and clock phase properties

[Ansuel Smith]

On Sat, Oct 09, 2021 at 11:37:10PM +0200, Andrew Lunn wrote:
> > Here is 2 configuration one from an Netgear r7800 qca8337:
> > 
> > 	switch@10 {
> > 		compatible = "qca,qca8337";
> > 		#address-cells = <1>;
> > 		#size-cells = <0>;
> > 		reg = <0x10>;
> > 
> > 		qca8k,rgmii0_1_8v;
> > 		qca8k,rgmii56_1_8v;
> > 
> > 		ports {
> > 			#address-cells = <1>;
> > 			#size-cells = <0>;
> > 
> > 			port@0 {
> > 				reg = <0>;
> > 				label = "cpu";
> > 				ethernet = <&gmac1>;
> > 				phy-mode = "rgmii-id";
> > 
> > 				fixed-link {
> > 					speed = <1000>;
> > 					full-duplex;
> > 				};
> > 			};
> > 
> > 			port@1 {
> > 				reg = <1>;
> > 				label = "lan1";
> > 				phy-mode = "internal";
> > 				phy-handle = <&phy_port1>;
> > 			};
> > 
> > 			port@2 {
> > 				reg = <2>;
> > 				label = "lan2";
> > 				phy-mode = "internal";
> > 				phy-handle = <&phy_port2>;
> > 			};
> > 
> > 			port@3 {
> > 				reg = <3>;
> > 				label = "lan3";
> > 				phy-mode = "internal";
> > 				phy-handle = <&phy_port3>;
> > 			};
> > 
> > 			port@4 {
> > 				reg = <4>;
> > 				label = "lan4";
> > 				phy-mode = "internal";
> > 				phy-handle = <&phy_port4>;
> > 			};
> > 
> > 			port@5 {
> > 				reg = <5>;
> > 				label = "wan";
> > 				phy-mode = "internal";
> > 				phy-handle = <&phy_port5>;
> > 			};
> > 
> > 			port@6 {
> > 				reg = <6>;
> > 				label = "cpu";
> > 				ethernet = <&gmac2>;
> > 				phy-mode = "sgmii";
> > 
> > 				fixed-link {
> > 					speed = <1000>;
> > 					full-duplex;
> > 				};
> 
> So here, it is a second CPU port.  But some other board could connect
> an SGMII PHY, and call the port lan5. Or it could be connected to an
> SFP cage, and used that way. Or are you forced to use it as a CPU
> port, or not use it at all?
>

We have a bit to set the mode. So yes it can be used to different modes.
(base-x, phy and mac)

> > And here is one with mac swap Tp-Link Archer c7 v4 qca8327 
> > 
> > 	switch0@10 {
> > 		compatible = "qca,qca8337";
> > 		#address-cells = <1>;
> > 		#size-cells = <0>;
> > 
> > 		reg = <0>;
> > 		qca,sgmii-rxclk-falling-edge;
> > 		qca,mac6-exchange;
> > 
> > 		ports {
> > 			#address-cells = <1>;
> > 			#size-cells = <0>;
> > 
> > 			port@0 {
> > 				reg = <0>;
> > 				label = "cpu";
> > 				ethernet = <&eth0>;
> > 				phy-mode = "sgmii";
> > 
> > 				fixed-link {
> > 					speed = <1000>;
> > 					full-duplex;
> > 				};
> 
> So when looking for SGMI properties, you need to look here. Where as
> in the previous example, you would look in port 6. And the reverse is
> true for RGMII delays.
> 
> > 			};
> > 
> > 			port@1 {
> > 				reg = <1>;
> > 				label = "wan";
> > 				phy-mode = "internal";
> > 				phy-handle = <&phy_port1>;
> > 			};
> > 
> > 			port@2 {
> > 				reg = <2>;
> > 				label = "lan1";
> > 				phy-mode = "internal";
> > 				phy-handle = <&phy_port2>;
> > 			};
> > 
> > 			port@3 {
> > 				reg = <3>;
> > 				label = "lan2";
> > 				phy-mode = "internal";
> > 				phy-handle = <&phy_port3>;
> > 			};
> > 
> > 			port@4 {
> > 				reg = <4>;
> > 				label = "lan3";
> > 				phy-mode = "internal";
> > 				phy-handle = <&phy_port4>;
> > 			};
> > 
> > 			port@5 {
> > 				reg = <5>;
> > 				label = "lan4";
> > 				phy-mode = "internal";
> > 				phy-handle = <&phy_port5>;
> > 			};
> > 		};
> 
> So here, port '6' is not used. But it could be connected to an RGMII
> PHY and called lan5. Would the naming work out? What does devlink
> think of it, etc. What about phy-handle? Is there an external MDIO
> bus? What address would be used if there is no phy-handle?
> 
>       Andrew

In this case port6 is not used as it's not connected at all in hardware.
From the configuration list yes, it can be used as lan5 in phy mode and
it would have address 5 (internally the address are with an offset of
-1). Anyway I honestly think that we are putting too much effort in
something that can and should be handled differently. I agree that all
this mac exchange is bs and doesn't make much sense. I tried to
implement this as we currently qca8k is hardcoded to expect
the cpu port 0 for everything and doesn't actually found a valid cpu
port (aka it doesn't expect a configuration with cpu6)
I think the driver was writtent with the concept of mac exchange from
the start. That's why it's hardoced to port0.
I actually tied for fun running the switch using only the port6 cpu
port and it worked just right. So I think I will just drop the mac
exchange and fix the code to make it dynamic.

-- 
	Ansuel

Re: [net-next PATCH v2 13/15] dt-bindings: net: dsa: qca8k: document open drain binding

[Ansuel Smith]

On Sat, Oct 09, 2021 at 07:20:49PM +0200, Andrew Lunn wrote:
> On Fri, Oct 08, 2021 at 02:22:23AM +0200, Ansuel Smith wrote:
> > Document new binding qca,power_on_sel used to enable Power-on-strapping
> > select reg and qca,led_open_drain to set led to open drain mode.
> > 
> > Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> > ---
> >  Documentation/devicetree/bindings/net/dsa/qca8k.txt | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/net/dsa/qca8k.txt b/Documentation/devicetree/bindings/net/dsa/qca8k.txt
> > index b9cccb657373..9fb4db65907e 100644
> > --- a/Documentation/devicetree/bindings/net/dsa/qca8k.txt
> > +++ b/Documentation/devicetree/bindings/net/dsa/qca8k.txt
> > @@ -13,6 +13,17 @@ Required properties:
> >  Optional properties:
> >  
> >  - reset-gpios: GPIO to be used to reset the whole device
> > +- qca,ignore-power-on-sel: Ignore power on pin strapping to configure led open
> > +                           drain or eeprom presence.
> 
> So strapping is only used for LEDs and EEPROM presence? Nothing else?
> Seems link MAC0/MAC6 swap would be a good candidate for strapping?
> 
> I just want to make it clear that if you select this option, you need
> to take care of X, Y and Z in DT.
> 
> 	Andrew

Sorry I missed this. Yes strapping is used only for LEDs and EEPROM. No
reference in Documentation about mac swap. Other strapping are related
to voltage selection and other hardware stuff. Thing that can't be set
from sw.

-- 
	Ansuel