diff mbox series

[RFC,net-next,v3,10/20] net: dsa: qca8k: add priority tweak to qca8337 switch

Message ID 20210504222915.17206-10-ansuelsmth@gmail.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series [RFC,net-next,v3,01/20] net: mdio: ipq8064: clean whitespaces in define | expand

Checks

Context Check Description
netdev/apply fail Patch does not apply to net-next
netdev/tree_selection success Clearly marked for net-next

Commit Message

Christian Marangi May 4, 2021, 10:29 p.m. UTC
The port 5 of the ar8337 have some problem in flood condition. The
original legacy driver had some specific buffer and priority settings
for the different port suggested by the QCA switch team. Add this
missing settings to improve switch stability under load condition.
The packet priority tweak and the rx delay is specific to qca8337.
Limit this changes to qca8337 as now we also support 8327 switch.

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

Comments

Andrew Lunn May 5, 2021, 12:53 a.m. UTC | #1
On Wed, May 05, 2021 at 12:29:04AM +0200, Ansuel Smith wrote:
> The port 5 of the ar8337 have some problem in flood condition. The
> original legacy driver had some specific buffer and priority settings
> for the different port suggested by the QCA switch team. Add this
> missing settings to improve switch stability under load condition.
> The packet priority tweak and the rx delay is specific to qca8337.
> Limit this changes to qca8337 as now we also support 8327 switch.
> 
> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> ---
>  drivers/net/dsa/qca8k.c | 54 +++++++++++++++++++++++++++++++++++++++--
>  drivers/net/dsa/qca8k.h | 24 ++++++++++++++++++
>  2 files changed, 76 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
> index 17c6fd4afa7d..9e034c445085 100644
> --- a/drivers/net/dsa/qca8k.c
> +++ b/drivers/net/dsa/qca8k.c
> @@ -783,7 +783,12 @@ static int
>  qca8k_setup(struct dsa_switch *ds)
>  {
>  	struct qca8k_priv *priv = (struct qca8k_priv *)ds->priv;
> +	const struct qca8k_match_data *data;
>  	int ret, i;
> +	u32 mask;
> +
> +	/* get the switches ID from the compatible */
> +	data = of_device_get_match_data(priv->dev);

You have already done this once in probe. Either store data in qca8k_priv,
or add the id to qca8K_priv.

   Andrew
Vladimir Oltean May 6, 2021, 11:16 a.m. UTC | #2
On Wed, May 05, 2021 at 12:29:04AM +0200, Ansuel Smith wrote:
> The port 5 of the ar8337 have some problem in flood condition. The
> original legacy driver had some specific buffer and priority settings
> for the different port suggested by the QCA switch team. Add this
> missing settings to improve switch stability under load condition.
> The packet priority tweak and the rx delay is specific to qca8337.
> Limit this changes to qca8337 as now we also support 8327 switch.
> 
> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> ---
>  drivers/net/dsa/qca8k.c | 54 +++++++++++++++++++++++++++++++++++++++--
>  drivers/net/dsa/qca8k.h | 24 ++++++++++++++++++
>  2 files changed, 76 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
> index 17c6fd4afa7d..9e034c445085 100644
> --- a/drivers/net/dsa/qca8k.c
> +++ b/drivers/net/dsa/qca8k.c
> @@ -783,7 +783,12 @@ static int
>  qca8k_setup(struct dsa_switch *ds)
>  {
>  	struct qca8k_priv *priv = (struct qca8k_priv *)ds->priv;
> +	const struct qca8k_match_data *data;
>  	int ret, i;
> +	u32 mask;
> +
> +	/* get the switches ID from the compatible */
> +	data = of_device_get_match_data(priv->dev);
>  
>  	/* Make sure that port 0 is the cpu port */
>  	if (!dsa_is_cpu_port(ds, 0)) {
> @@ -889,6 +894,45 @@ qca8k_setup(struct dsa_switch *ds)
>  		}
>  	}
>  
> +	if (data->id == QCA8K_ID_QCA8337) {
> +		for (i = 0; i < QCA8K_NUM_PORTS; i++) {
> +			switch (i) {
> +			/* The 2 CPU port and port 5 requires some different
> +			 * priority than any other ports.
> +			 */
> +			case 0:
> +			case 5:
> +			case 6:
> +				mask = QCA8K_PORT_HOL_CTRL0_EG_PRI0(0x3) |
> +					QCA8K_PORT_HOL_CTRL0_EG_PRI1(0x4) |
> +					QCA8K_PORT_HOL_CTRL0_EG_PRI2(0x4) |
> +					QCA8K_PORT_HOL_CTRL0_EG_PRI3(0x4) |
> +					QCA8K_PORT_HOL_CTRL0_EG_PRI4(0x6) |
> +					QCA8K_PORT_HOL_CTRL0_EG_PRI5(0x8) |
> +					QCA8K_PORT_HOL_CTRL0_EG_PORT(0x1e);
> +				break;
> +			default:
> +				mask = QCA8K_PORT_HOL_CTRL0_EG_PRI0(0x3) |
> +					QCA8K_PORT_HOL_CTRL0_EG_PRI1(0x4) |
> +					QCA8K_PORT_HOL_CTRL0_EG_PRI2(0x6) |
> +					QCA8K_PORT_HOL_CTRL0_EG_PRI3(0x8) |
> +					QCA8K_PORT_HOL_CTRL0_EG_PORT(0x19);
> +			}
> +			qca8k_write(priv, QCA8K_REG_PORT_HOL_CTRL0(i), mask);
> +
> +			mask = QCA8K_PORT_HOL_CTRL1_ING(0x6) |
> +			QCA8K_PORT_HOL_CTRL1_EG_PRI_BUF_EN |
> +			QCA8K_PORT_HOL_CTRL1_EG_PORT_BUF_EN |
> +			QCA8K_PORT_HOL_CTRL1_WRED_EN;
> +			qca8k_rmw(priv, QCA8K_REG_PORT_HOL_CTRL1(i),
> +				  QCA8K_PORT_HOL_CTRL1_ING_BUF |
> +				  QCA8K_PORT_HOL_CTRL1_EG_PRI_BUF_EN |
> +				  QCA8K_PORT_HOL_CTRL1_EG_PORT_BUF_EN |
> +				  QCA8K_PORT_HOL_CTRL1_WRED_EN,
> +				  mask);
> +		}

What is this actually doing and why is it needed?

> +	}
> +
>  	/* Setup our port MTUs to match power on defaults */
>  	for (i = 0; i < QCA8K_NUM_PORTS; i++)
>  		priv->port_mtu[i] = ETH_FRAME_LEN + ETH_FCS_LEN;
> @@ -909,9 +953,13 @@ static void
>  qca8k_phylink_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
>  			 const struct phylink_link_state *state)
>  {
> +	const struct qca8k_match_data *data;
>  	struct qca8k_priv *priv = ds->priv;
>  	u32 reg, val;
>  
> +	/* get the switches ID from the compatible */
> +	data = of_device_get_match_data(priv->dev);
> +
>  	switch (port) {
>  	case 0: /* 1st CPU port */
>  		if (state->interface != PHY_INTERFACE_MODE_RGMII &&
> @@ -962,8 +1010,10 @@ qca8k_phylink_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
>  			    QCA8K_PORT_PAD_RGMII_EN |
>  			    QCA8K_PORT_PAD_RGMII_TX_DELAY(QCA8K_MAX_DELAY) |
>  			    QCA8K_PORT_PAD_RGMII_RX_DELAY(QCA8K_MAX_DELAY));
> -		qca8k_write(priv, QCA8K_REG_PORT5_PAD_CTRL,
> -			    QCA8K_PORT_PAD_RGMII_RX_DELAY_EN);
> +		/* QCA8337 requires to set rgmii rx delay */
> +		if (data->id == QCA8K_ID_QCA8337)
> +			qca8k_write(priv, QCA8K_REG_PORT5_PAD_CTRL,
> +				    QCA8K_PORT_PAD_RGMII_RX_DELAY_EN);

Why are we looking at an RGMII delay change in a patch about "priority
tweaks"? This patch is very confusing.

>  		break;
>  	case PHY_INTERFACE_MODE_SGMII:
>  	case PHY_INTERFACE_MODE_1000BASEX:
> diff --git a/drivers/net/dsa/qca8k.h b/drivers/net/dsa/qca8k.h
> index 86e8d479c9f9..34c5522e7202 100644
> --- a/drivers/net/dsa/qca8k.h
> +++ b/drivers/net/dsa/qca8k.h
> @@ -166,6 +166,30 @@
>  #define   QCA8K_PORT_LOOKUP_STATE			GENMASK(18, 16)
>  #define   QCA8K_PORT_LOOKUP_LEARN			BIT(20)
>  
> +#define QCA8K_REG_PORT_HOL_CTRL0(_i)			(0x970 + (_i) * 0x8)
> +#define   QCA8K_PORT_HOL_CTRL0_EG_PRI0_BUF		GENMASK(3, 0)
> +#define   QCA8K_PORT_HOL_CTRL0_EG_PRI0(x)		((x) << 0)
> +#define   QCA8K_PORT_HOL_CTRL0_EG_PRI1_BUF		GENMASK(7, 4)
> +#define   QCA8K_PORT_HOL_CTRL0_EG_PRI1(x)		((x) << 4)
> +#define   QCA8K_PORT_HOL_CTRL0_EG_PRI2_BUF		GENMASK(11, 8)
> +#define   QCA8K_PORT_HOL_CTRL0_EG_PRI2(x)		((x) << 8)
> +#define   QCA8K_PORT_HOL_CTRL0_EG_PRI3_BUF		GENMASK(15, 12)
> +#define   QCA8K_PORT_HOL_CTRL0_EG_PRI3(x)		((x) << 12)
> +#define   QCA8K_PORT_HOL_CTRL0_EG_PRI4_BUF		GENMASK(19, 16)
> +#define   QCA8K_PORT_HOL_CTRL0_EG_PRI4(x)		((x) << 16)
> +#define   QCA8K_PORT_HOL_CTRL0_EG_PRI5_BUF		GENMASK(23, 20)
> +#define   QCA8K_PORT_HOL_CTRL0_EG_PRI5(x)		((x) << 20)
> +#define   QCA8K_PORT_HOL_CTRL0_EG_PORT_BUF		GENMASK(29, 24)
> +#define   QCA8K_PORT_HOL_CTRL0_EG_PORT(x)		((x) << 24)
> +
> +#define QCA8K_REG_PORT_HOL_CTRL1(_i)			(0x974 + (_i) * 0x8)
> +#define   QCA8K_PORT_HOL_CTRL1_ING_BUF			GENMASK(3, 0)
> +#define   QCA8K_PORT_HOL_CTRL1_ING(x)			((x) << 0)
> +#define   QCA8K_PORT_HOL_CTRL1_EG_PRI_BUF_EN		BIT(6)
> +#define   QCA8K_PORT_HOL_CTRL1_EG_PORT_BUF_EN		BIT(7)
> +#define   QCA8K_PORT_HOL_CTRL1_WRED_EN			BIT(8)
> +#define   QCA8K_PORT_HOL_CTRL1_EG_MIRROR_EN		BIT(16)
> +
>  /* Pkt edit registers */
>  #define QCA8K_EGRESS_VLAN(x)				(0x0c70 + (4 * (x / 2)))
>  
> -- 
> 2.30.2
>
diff mbox series

Patch

diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
index 17c6fd4afa7d..9e034c445085 100644
--- a/drivers/net/dsa/qca8k.c
+++ b/drivers/net/dsa/qca8k.c
@@ -783,7 +783,12 @@  static int
 qca8k_setup(struct dsa_switch *ds)
 {
 	struct qca8k_priv *priv = (struct qca8k_priv *)ds->priv;
+	const struct qca8k_match_data *data;
 	int ret, i;
+	u32 mask;
+
+	/* get the switches ID from the compatible */
+	data = of_device_get_match_data(priv->dev);
 
 	/* Make sure that port 0 is the cpu port */
 	if (!dsa_is_cpu_port(ds, 0)) {
@@ -889,6 +894,45 @@  qca8k_setup(struct dsa_switch *ds)
 		}
 	}
 
+	if (data->id == QCA8K_ID_QCA8337) {
+		for (i = 0; i < QCA8K_NUM_PORTS; i++) {
+			switch (i) {
+			/* The 2 CPU port and port 5 requires some different
+			 * priority than any other ports.
+			 */
+			case 0:
+			case 5:
+			case 6:
+				mask = QCA8K_PORT_HOL_CTRL0_EG_PRI0(0x3) |
+					QCA8K_PORT_HOL_CTRL0_EG_PRI1(0x4) |
+					QCA8K_PORT_HOL_CTRL0_EG_PRI2(0x4) |
+					QCA8K_PORT_HOL_CTRL0_EG_PRI3(0x4) |
+					QCA8K_PORT_HOL_CTRL0_EG_PRI4(0x6) |
+					QCA8K_PORT_HOL_CTRL0_EG_PRI5(0x8) |
+					QCA8K_PORT_HOL_CTRL0_EG_PORT(0x1e);
+				break;
+			default:
+				mask = QCA8K_PORT_HOL_CTRL0_EG_PRI0(0x3) |
+					QCA8K_PORT_HOL_CTRL0_EG_PRI1(0x4) |
+					QCA8K_PORT_HOL_CTRL0_EG_PRI2(0x6) |
+					QCA8K_PORT_HOL_CTRL0_EG_PRI3(0x8) |
+					QCA8K_PORT_HOL_CTRL0_EG_PORT(0x19);
+			}
+			qca8k_write(priv, QCA8K_REG_PORT_HOL_CTRL0(i), mask);
+
+			mask = QCA8K_PORT_HOL_CTRL1_ING(0x6) |
+			QCA8K_PORT_HOL_CTRL1_EG_PRI_BUF_EN |
+			QCA8K_PORT_HOL_CTRL1_EG_PORT_BUF_EN |
+			QCA8K_PORT_HOL_CTRL1_WRED_EN;
+			qca8k_rmw(priv, QCA8K_REG_PORT_HOL_CTRL1(i),
+				  QCA8K_PORT_HOL_CTRL1_ING_BUF |
+				  QCA8K_PORT_HOL_CTRL1_EG_PRI_BUF_EN |
+				  QCA8K_PORT_HOL_CTRL1_EG_PORT_BUF_EN |
+				  QCA8K_PORT_HOL_CTRL1_WRED_EN,
+				  mask);
+		}
+	}
+
 	/* Setup our port MTUs to match power on defaults */
 	for (i = 0; i < QCA8K_NUM_PORTS; i++)
 		priv->port_mtu[i] = ETH_FRAME_LEN + ETH_FCS_LEN;
@@ -909,9 +953,13 @@  static void
 qca8k_phylink_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
 			 const struct phylink_link_state *state)
 {
+	const struct qca8k_match_data *data;
 	struct qca8k_priv *priv = ds->priv;
 	u32 reg, val;
 
+	/* get the switches ID from the compatible */
+	data = of_device_get_match_data(priv->dev);
+
 	switch (port) {
 	case 0: /* 1st CPU port */
 		if (state->interface != PHY_INTERFACE_MODE_RGMII &&
@@ -962,8 +1010,10 @@  qca8k_phylink_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
 			    QCA8K_PORT_PAD_RGMII_EN |
 			    QCA8K_PORT_PAD_RGMII_TX_DELAY(QCA8K_MAX_DELAY) |
 			    QCA8K_PORT_PAD_RGMII_RX_DELAY(QCA8K_MAX_DELAY));
-		qca8k_write(priv, QCA8K_REG_PORT5_PAD_CTRL,
-			    QCA8K_PORT_PAD_RGMII_RX_DELAY_EN);
+		/* QCA8337 requires to set rgmii rx delay */
+		if (data->id == QCA8K_ID_QCA8337)
+			qca8k_write(priv, QCA8K_REG_PORT5_PAD_CTRL,
+				    QCA8K_PORT_PAD_RGMII_RX_DELAY_EN);
 		break;
 	case PHY_INTERFACE_MODE_SGMII:
 	case PHY_INTERFACE_MODE_1000BASEX:
diff --git a/drivers/net/dsa/qca8k.h b/drivers/net/dsa/qca8k.h
index 86e8d479c9f9..34c5522e7202 100644
--- a/drivers/net/dsa/qca8k.h
+++ b/drivers/net/dsa/qca8k.h
@@ -166,6 +166,30 @@ 
 #define   QCA8K_PORT_LOOKUP_STATE			GENMASK(18, 16)
 #define   QCA8K_PORT_LOOKUP_LEARN			BIT(20)
 
+#define QCA8K_REG_PORT_HOL_CTRL0(_i)			(0x970 + (_i) * 0x8)
+#define   QCA8K_PORT_HOL_CTRL0_EG_PRI0_BUF		GENMASK(3, 0)
+#define   QCA8K_PORT_HOL_CTRL0_EG_PRI0(x)		((x) << 0)
+#define   QCA8K_PORT_HOL_CTRL0_EG_PRI1_BUF		GENMASK(7, 4)
+#define   QCA8K_PORT_HOL_CTRL0_EG_PRI1(x)		((x) << 4)
+#define   QCA8K_PORT_HOL_CTRL0_EG_PRI2_BUF		GENMASK(11, 8)
+#define   QCA8K_PORT_HOL_CTRL0_EG_PRI2(x)		((x) << 8)
+#define   QCA8K_PORT_HOL_CTRL0_EG_PRI3_BUF		GENMASK(15, 12)
+#define   QCA8K_PORT_HOL_CTRL0_EG_PRI3(x)		((x) << 12)
+#define   QCA8K_PORT_HOL_CTRL0_EG_PRI4_BUF		GENMASK(19, 16)
+#define   QCA8K_PORT_HOL_CTRL0_EG_PRI4(x)		((x) << 16)
+#define   QCA8K_PORT_HOL_CTRL0_EG_PRI5_BUF		GENMASK(23, 20)
+#define   QCA8K_PORT_HOL_CTRL0_EG_PRI5(x)		((x) << 20)
+#define   QCA8K_PORT_HOL_CTRL0_EG_PORT_BUF		GENMASK(29, 24)
+#define   QCA8K_PORT_HOL_CTRL0_EG_PORT(x)		((x) << 24)
+
+#define QCA8K_REG_PORT_HOL_CTRL1(_i)			(0x974 + (_i) * 0x8)
+#define   QCA8K_PORT_HOL_CTRL1_ING_BUF			GENMASK(3, 0)
+#define   QCA8K_PORT_HOL_CTRL1_ING(x)			((x) << 0)
+#define   QCA8K_PORT_HOL_CTRL1_EG_PRI_BUF_EN		BIT(6)
+#define   QCA8K_PORT_HOL_CTRL1_EG_PORT_BUF_EN		BIT(7)
+#define   QCA8K_PORT_HOL_CTRL1_WRED_EN			BIT(8)
+#define   QCA8K_PORT_HOL_CTRL1_EG_MIRROR_EN		BIT(16)
+
 /* Pkt edit registers */
 #define QCA8K_EGRESS_VLAN(x)				(0x0c70 + (4 * (x / 2)))