lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <b4c98268-e436-46d5-8906-2fdaf6e89fed@arinc9.com>
Date: Tue, 30 Jul 2024 13:21:13 +0300
From: Arınç ÜNAL <arinc.unal@...nc9.com>
To: Lorenzo Bianconi <lorenzo@...nel.org>, netdev@...r.kernel.org
Cc: daniel@...rotopia.org, dqfext@...il.com, sean.wang@...iatek.com,
 andrew@...n.ch, f.fainelli@...il.com, olteanv@...il.com,
 davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
 pabeni@...hat.com, matthias.bgg@...il.com,
 angelogioacchino.delregno@...labora.com,
 linux-arm-kernel@...ts.infradead.org, linux-mediatek@...ts.infradead.org,
 lorenzo.bianconi83@...il.com, robh@...nel.org, krzk+dt@...nel.org,
 conor+dt@...nel.org, devicetree@...r.kernel.org, upstream@...oha.com
Subject: Re: [PATCH net-next 2/2] net: dsa: mt7530: Add EN7581 support

On 30/07/2024 10:46, Lorenzo Bianconi wrote:
> Introduce support for the DSA built-in switch available on the EN7581
> development board. EN7581 support is similar to MT7988 one except
> it requires to set MT7530_FORCE_MODE bit in MT753X_PMCR_P register
> for on cpu port.
> 
> Tested-by: Benjamin Larsson <benjamin.larsson@...exis.eu>
> Signed-off-by: Lorenzo Bianconi <lorenzo@...nel.org>
> ---
>   drivers/net/dsa/mt7530-mmio.c |  1 +
>   drivers/net/dsa/mt7530.c      | 38 +++++++++++++++++++++++++++++++----
>   drivers/net/dsa/mt7530.h      | 16 ++++++++++-----
>   3 files changed, 46 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/dsa/mt7530-mmio.c b/drivers/net/dsa/mt7530-mmio.c
> index b74a230a3f13..10dc49961f15 100644
> --- a/drivers/net/dsa/mt7530-mmio.c
> +++ b/drivers/net/dsa/mt7530-mmio.c
> @@ -11,6 +11,7 @@
>   #include "mt7530.h"
>   
>   static const struct of_device_id mt7988_of_match[] = {
> +	{ .compatible = "airoha,en7581-switch", .data = &mt753x_table[ID_EN7581], },
>   	{ .compatible = "mediatek,mt7988-switch", .data = &mt753x_table[ID_MT7988], },
>   	{ /* sentinel */ },
>   };
> diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
> index ec18e68bf3a8..8adc4561c5b2 100644
> --- a/drivers/net/dsa/mt7530.c
> +++ b/drivers/net/dsa/mt7530.c
> @@ -1152,7 +1152,8 @@ mt753x_cpu_port_enable(struct dsa_switch *ds, int port)
>   	 * the MT7988 SoC. Trapped frames will be forwarded to the CPU port that
>   	 * is affine to the inbound user port.
>   	 */
> -	if (priv->id == ID_MT7531 || priv->id == ID_MT7988)
> +	if (priv->id == ID_MT7531 || priv->id == ID_MT7988 ||
> +	    priv->id == ID_EN7581)
>   		mt7530_set(priv, MT7531_CFC, MT7531_CPU_PMAP(BIT(port)));
>   
>   	/* CPU port gets connected to all user ports of
> @@ -2207,7 +2208,7 @@ mt7530_setup_irq(struct mt7530_priv *priv)
>   		return priv->irq ? : -EINVAL;
>   	}
>   
> -	if (priv->id == ID_MT7988)
> +	if (priv->id == ID_MT7988 || priv->id == ID_EN7581)
>   		priv->irq_domain = irq_domain_add_linear(np, MT7530_NUM_PHYS,
>   							 &mt7988_irq_domain_ops,
>   							 priv);
> @@ -2766,7 +2767,7 @@ static void mt7988_mac_port_get_caps(struct dsa_switch *ds, int port,
>   {
>   	switch (port) {
>   	/* Ports which are connected to switch PHYs. There is no MII pinout. */
> -	case 0 ... 3:
> +	case 0 ... 4:

Please create a new function, such as en7581_mac_port_get_caps().

>   		__set_bit(PHY_INTERFACE_MODE_INTERNAL,
>   			  config->supported_interfaces);
>   
> @@ -2850,6 +2851,23 @@ mt7531_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
>   	}
>   }
>   
> +static void
> +en7581_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
> +		  phy_interface_t interface)
> +{
> +	/* BIT(31-27): reserved
> +	 * BIT(26): TX_CRC_EN: enable(0)/disable(1) CRC insertion
> +	 * BIT(25): RX_CRC_EN: enable(0)/disable(1) CRC insertion
> +	 * Since the bits above have a different meaning with respect to the
> +	 * one described in mt7530.h, set default values.
> +	 */
> +	mt7530_clear(ds->priv, MT753X_PMCR_P(port), MT7531_FORCE_MODE_MASK);
> +	if (dsa_is_cpu_port(ds, port)) {
> +		/* enable MT7530_FORCE_MODE on cpu port */
> +		mt7530_set(ds->priv, MT753X_PMCR_P(port), MT7530_FORCE_MODE);
> +	}
> +}

This seems to undo "Clear link settings and enable force mode to force link
down on all ports until they're enabled later." on mt7531_setup_common()
and redo it only for the CPU port. It should be so that force mode is
enabled on all ports. You could position the diff below as a patch before
this patch. It introduces the MT753X_FORCE_MODE() macro to choose the
correct constant for the switch model.

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index ec18e68bf3a8..4915264c460f 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -2438,8 +2438,10 @@ mt7530_setup(struct dsa_switch *ds)
  		/* Clear link settings and enable force mode to force link down
  		 * on all ports until they're enabled later.
  		 */
-		mt7530_rmw(priv, MT753X_PMCR_P(i), PMCR_LINK_SETTINGS_MASK |
-			   MT7530_FORCE_MODE, MT7530_FORCE_MODE);
+		mt7530_rmw(priv, MT753X_PMCR_P(i),
+			   PMCR_LINK_SETTINGS_MASK |
+				   MT753X_FORCE_MODE(priv->id),
+			   MT753X_FORCE_MODE(priv->id));
  
  		/* Disable forwarding by default on all ports */
  		mt7530_rmw(priv, MT7530_PCR_P(i), PCR_MATRIX_MASK,
@@ -2550,8 +2552,10 @@ mt7531_setup_common(struct dsa_switch *ds)
  		/* Clear link settings and enable force mode to force link down
  		 * on all ports until they're enabled later.
  		 */
-		mt7530_rmw(priv, MT753X_PMCR_P(i), PMCR_LINK_SETTINGS_MASK |
-			   MT7531_FORCE_MODE_MASK, MT7531_FORCE_MODE_MASK);
+		mt7530_rmw(priv, MT753X_PMCR_P(i),
+			   PMCR_LINK_SETTINGS_MASK |
+				   MT753X_FORCE_MODE(priv->id),
+			   MT753X_FORCE_MODE(priv->id));
  
  		/* Disable forwarding by default on all ports */
  		mt7530_rmw(priv, MT7530_PCR_P(i), PCR_MATRIX_MASK,
diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
index 28592123070b..d47d1ce511ba 100644
--- a/drivers/net/dsa/mt7530.h
+++ b/drivers/net/dsa/mt7530.h
@@ -355,6 +355,10 @@ enum mt7530_vlan_port_acc_frm {
  					 MT7531_FORCE_MODE_TX_FC | \
  					 MT7531_FORCE_MODE_EEE100 | \
  					 MT7531_FORCE_MODE_EEE1G)
+#define  MT753X_FORCE_MODE(id)		((id == ID_MT7531 || \
+					  id == ID_MT7988) ? \
+					 MT7531_FORCE_MODE_MASK : \
+					 MT7530_FORCE_MODE)
  #define  PMCR_LINK_SETTINGS_MASK	(PMCR_MAC_TX_EN | PMCR_MAC_RX_EN | \
  					 PMCR_FORCE_EEE1G | \
  					 PMCR_FORCE_EEE100 | \

> +
>   static struct phylink_pcs *
>   mt753x_phylink_mac_select_pcs(struct phylink_config *config,
>   			      phy_interface_t interface)
> @@ -2880,7 +2898,8 @@ mt753x_phylink_mac_config(struct phylink_config *config, unsigned int mode,
>   
>   	priv = ds->priv;
>   
> -	if ((port == 5 || port == 6) && priv->info->mac_port_config)
> +	if ((port == 5 || port == 6 || priv->id == ID_EN7581) &&
> +	    priv->info->mac_port_config)
>   		priv->info->mac_port_config(ds, port, mode, state->interface);
>   
>   	/* Are we connected to external phy */
> @@ -3220,6 +3239,17 @@ const struct mt753x_info mt753x_table[] = {
>   		.phy_write_c45 = mt7531_ind_c45_phy_write,
>   		.mac_port_get_caps = mt7988_mac_port_get_caps,
>   	},
> +	[ID_EN7581] = {
> +		.id = ID_EN7581,
> +		.pcs_ops = &mt7530_pcs_ops,
> +		.sw_setup = mt7988_setup,
> +		.phy_read_c22 = mt7531_ind_c22_phy_read,
> +		.phy_write_c22 = mt7531_ind_c22_phy_write,
> +		.phy_read_c45 = mt7531_ind_c45_phy_read,
> +		.phy_write_c45 = mt7531_ind_c45_phy_write,
> +		.mac_port_get_caps = mt7988_mac_port_get_caps,
> +		.mac_port_config = en7581_mac_config,
> +	},

Let me lend a hand; you can apply this diff on top of this patch.

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index b18b98a53a7d..f5766d8ae360 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -2768,6 +2768,28 @@ static void mt7531_mac_port_get_caps(struct dsa_switch *ds, int port,
  
  static void mt7988_mac_port_get_caps(struct dsa_switch *ds, int port,
  				     struct phylink_config *config)
+{
+	switch (port) {
+	/* Ports which are connected to switch PHYs. There is no MII pinout. */
+	case 0 ... 3:
+		__set_bit(PHY_INTERFACE_MODE_INTERNAL,
+			  config->supported_interfaces);
+
+		config->mac_capabilities |= MAC_10 | MAC_100 | MAC_1000FD;
+		break;
+
+	/* Port 6 is connected to SoC's XGMII MAC. There is no MII pinout. */
+	case 6:
+		__set_bit(PHY_INTERFACE_MODE_INTERNAL,
+			  config->supported_interfaces);
+
+		config->mac_capabilities |= MAC_10000FD;
+		break;
+	}
+}
+
+static void en7581_mac_port_get_caps(struct dsa_switch *ds, int port,
+				     struct phylink_config *config)
  {
  	switch (port) {
  	/* Ports which are connected to switch PHYs. There is no MII pinout. */
@@ -2855,23 +2877,6 @@ mt7531_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
  	}
  }
  
-static void
-en7581_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
-		  phy_interface_t interface)
-{
-	/* BIT(31-27): reserved
-	 * BIT(26): TX_CRC_EN: enable(0)/disable(1) CRC insertion
-	 * BIT(25): RX_CRC_EN: enable(0)/disable(1) CRC insertion
-	 * Since the bits above have a different meaning with respect to the
-	 * one described in mt7530.h, set default values.
-	 */
-	mt7530_clear(ds->priv, MT753X_PMCR_P(port), MT7531_FORCE_MODE_MASK);
-	if (dsa_is_cpu_port(ds, port)) {
-		/* enable MT7530_FORCE_MODE on cpu port */
-		mt7530_set(ds->priv, MT753X_PMCR_P(port), MT7530_FORCE_MODE);
-	}
-}
-
  static struct phylink_pcs *
  mt753x_phylink_mac_select_pcs(struct phylink_config *config,
  			      phy_interface_t interface)
@@ -2902,8 +2907,7 @@ mt753x_phylink_mac_config(struct phylink_config *config, unsigned int mode,
  
  	priv = ds->priv;
  
-	if ((port == 5 || port == 6 || priv->id == ID_EN7581) &&
-	    priv->info->mac_port_config)
+	if ((port == 5 || port == 6) && priv->info->mac_port_config)
  		priv->info->mac_port_config(ds, port, mode, state->interface);
  
  	/* Are we connected to external phy */
@@ -3251,8 +3255,7 @@ const struct mt753x_info mt753x_table[] = {
  		.phy_write_c22 = mt7531_ind_c22_phy_write,
  		.phy_read_c45 = mt7531_ind_c45_phy_read,
  		.phy_write_c45 = mt7531_ind_c45_phy_write,
-		.mac_port_get_caps = mt7988_mac_port_get_caps,
-		.mac_port_config = en7581_mac_config,
+		.mac_port_get_caps = en7581_mac_port_get_caps,
  	},
  };
  EXPORT_SYMBOL_GPL(mt753x_table);

I don't know this hardware so please make sure the comments on
en7581_mac_port_get_caps() are correct. I didn't compile this so please
make sure it works.

By the way, is this supposed to be AN7581? There's EN7580 but no EN7581 on
the Airoha website.

https://www.airoha.com/products/y1cQz8EpjIKhbK61

Arınç

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ