diff mbox series

[net-next,v1,5/5] net: dsa: microchip: add support for side MDIO interface in LAN937x

Message ID 20241026063538.2506143-6-o.rempel@pengutronix.de (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series Side MDIO Support for LAN937x Switches | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 5 this patch: 5
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 10 of 10 maintainers
netdev/build_clang success Errors and warnings before: 3 this patch: 3
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 4 this patch: 4
netdev/checkpatch warning WARNING: line length of 83 exceeds 80 columns
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Oleksij Rempel Oct. 26, 2024, 6:35 a.m. UTC
Implement side MDIO channel support for LAN937x switches, providing an
alternative to SPI for PHY management alongside existing SPI-based
switch configuration. This is needed to reduce SPI load, as SPI can be
relatively expensive for small packets compared to MDIO support.

Also, implemented static mappings for PHY addresses for various LAN937x
models to support different internal PHY configurations. Since the PHY
address mappings are not equal to the port indexes, this patch also
provides PHY address calculation based on hardware strapping
configuration.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/net/dsa/microchip/ksz_common.c   |   7 ++
 drivers/net/dsa/microchip/lan937x.h      |   2 +
 drivers/net/dsa/microchip/lan937x_main.c | 139 ++++++++++++++++++++---
 drivers/net/dsa/microchip/lan937x_reg.h  |   4 +
 4 files changed, 136 insertions(+), 16 deletions(-)

Comments

Andrew Lunn Oct. 28, 2024, 12:15 p.m. UTC | #1
> +static const u8 lan9370_phy_addr[] = {
> +	[0] = 2, /* Port 1, T1 AFE0 */
> +	[1] = 3, /* Port 2, T1 AFE1 */
> +	[2] = 5, /* Port 3, T1 AFE3 */
> +	[3] = 6, /* Port 4, T1 AFE4 */
> +	[4] = U8_MAX, /* Port 5, RGMII 2 */
> +};

I think it would be good to add a #define for U8_MAX which gives a
hint at its meaning.


> +	for (i = 0; i < dev->info->port_cnt; i++) {
> +		if (phy_addr_map[i] == U8_MAX)
> +			dev->phy_addr_map[i] = phy_addr_map[i];
> +		else
> +			dev->phy_addr_map[i] = phy_addr_map[i] + offset;
> +	}

My first guess was that U8_MAX means the PHY is external, so could be
on any address depending on strapping. Looking at this code, i'm not
sure it does actually mean that.


    Andrew

---
pw-bot: cr
Oleksij Rempel Oct. 28, 2024, 1:43 p.m. UTC | #2
On Mon, Oct 28, 2024 at 01:15:42PM +0100, Andrew Lunn wrote:
> > +static const u8 lan9370_phy_addr[] = {
> > +	[0] = 2, /* Port 1, T1 AFE0 */
> > +	[1] = 3, /* Port 2, T1 AFE1 */
> > +	[2] = 5, /* Port 3, T1 AFE3 */
> > +	[3] = 6, /* Port 4, T1 AFE4 */
> > +	[4] = U8_MAX, /* Port 5, RGMII 2 */
> > +};
> 
> I think it would be good to add a #define for U8_MAX which gives a
> hint at its meaning.

ack
 
> > +	for (i = 0; i < dev->info->port_cnt; i++) {
> > +		if (phy_addr_map[i] == U8_MAX)
> > +			dev->phy_addr_map[i] = phy_addr_map[i];
> > +		else
> > +			dev->phy_addr_map[i] = phy_addr_map[i] + offset;
> > +	}
> 
> My first guess was that U8_MAX means the PHY is external, so could be
> on any address depending on strapping. Looking at this code, i'm not
> sure it does actually mean that.

Yes, the U8_MAX for ports without integrated PHYs. This code calculates address
for integrated PHYs, which can be different depending on strap
configuration.

I'll use some different define.
Arun Ramadoss Oct. 29, 2024, 3:39 a.m. UTC | #3
Hi Oleksij,

On Sat, 2024-10-26 at 08:35 +0200, Oleksij Rempel wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
> 
> diff --git a/drivers/net/dsa/microchip/lan937x_main.c
> b/drivers/net/dsa/microchip/lan937x_main.c
> index 824d9309a3d35..7dfd21d0d2843 100644
> --- a/drivers/net/dsa/microchip/lan937x_main.c
> +++ b/drivers/net/dsa/microchip/lan937x_main.c
> @@ -18,6 +18,47 @@
>  #include "ksz9477.h"
>  #include "lan937x.h"
> 
> +static const u8 lan9370_phy_addr[] = {
> +       [0] = 2, /* Port 1, T1 AFE0 */
> +       [1] = 3, /* Port 2, T1 AFE1 */
> +       [2] = 5, /* Port 3, T1 AFE3 */
> +       [3] = 6, /* Port 4, T1 AFE4 */
> +       [4] = U8_MAX, /* Port 5, RGMII 2 */
> +};
> +

Is it intentional to not to add support for lan9371 variant switch?

> +static const u8 lan9372_phy_addr[] = {
> +       [0] = 2, /* Port 1, T1 AFE0 */
> +       [1] = 3, /* Port 2, T1 AFE1 */
> +       [2] = 5, /* Port 3, T1 AFE3 */
> +       [3] = 8, /* Port 4, TX PHY */
> +       [4] = U8_MAX, /* Port 5, RGMII 2 */
> +       [5] = U8_MAX, /* Port 6, RGMII 1 */
> +       [6] = 6, /* Port 7, T1 AFE4 */
> +       [7] = 4, /* Port 8, T1 AFE2 */
> +};
> +
> +static const u8 lan9373_phy_addr[] = {
> +       [0] = 2, /* Port 1, T1 AFE0 */
> +       [1] = 3, /* Port 2, T1 AFE1 */
> +       [2] = 5, /* Port 3, T1 AFE3 */
> +       [3] = U8_MAX, /* Port 4, SGMII */
> +       [4] = U8_MAX, /* Port 5, RGMII 2 */
> +       [5] = U8_MAX, /* Port 6, RGMII 1 */
> +       [6] = 6, /* Port 7, T1 AFE4 */
> +       [7] = 4, /* Port 8, T1 AFE2 */
> +};
> +
> +static const u8 lan9374_phy_addr[] = {
> +       [0] = 2, /* Port 1, T1 AFE0 */
> +       [1] = 3, /* Port 2, T1 AFE1 */
> +       [2] = 5, /* Port 3, T1 AFE3 */
> +       [3] = 7, /* Port 4, T1 AFE5 */
> +       [4] = U8_MAX, /* Port 5, RGMII 2 */
> +       [5] = U8_MAX, /* Port 6, RGMII 1 */
> +       [6] = 6, /* Port 7, T1 AFE4 */
> +       [7] = 4, /* Port 8, T1 AFE2 */
> +};
> +
>  static int lan937x_cfg(struct ksz_device *dev, u32 addr, u8 bits,
> bool set)
>  {
>         return regmap_update_bits(ksz_regmap_8(dev), addr, bits, set
> ? bits : 0);
> @@ -30,24 +71,97 @@ static int lan937x_port_cfg(struct ksz_device
> *dev, int port, int offset,
>                                   bits, set ? bits : 0);
>  }
> 
> 
> 2.39.5
>
Oleksij Rempel Oct. 29, 2024, 5:15 a.m. UTC | #4
Hi Arun,

On Tue, Oct 29, 2024 at 03:39:33AM +0000, Arun.Ramadoss@microchip.com wrote:
> Hi Oleksij,
> 
> On Sat, 2024-10-26 at 08:35 +0200, Oleksij Rempel wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you
> > know the content is safe
> > 
> > diff --git a/drivers/net/dsa/microchip/lan937x_main.c
> > b/drivers/net/dsa/microchip/lan937x_main.c
> > index 824d9309a3d35..7dfd21d0d2843 100644
> > --- a/drivers/net/dsa/microchip/lan937x_main.c
> > +++ b/drivers/net/dsa/microchip/lan937x_main.c
> > @@ -18,6 +18,47 @@
> >  #include "ksz9477.h"
> >  #include "lan937x.h"
> > 
> > +static const u8 lan9370_phy_addr[] = {
> > +       [0] = 2, /* Port 1, T1 AFE0 */
> > +       [1] = 3, /* Port 2, T1 AFE1 */
> > +       [2] = 5, /* Port 3, T1 AFE3 */
> > +       [3] = 6, /* Port 4, T1 AFE4 */
> > +       [4] = U8_MAX, /* Port 5, RGMII 2 */
> > +};
> > +
> 
> Is it intentional to not to add support for lan9371 variant switch?

I forgot about this one, will add it too.

Regards,
Oleksij
diff mbox series

Patch

diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index f08fa52dd1387..303f1342e6cc2 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -411,6 +411,8 @@  static const struct ksz_dev_ops lan937x_dev_ops = {
 	.flush_dyn_mac_table = ksz9477_flush_dyn_mac_table,
 	.port_setup = lan937x_port_setup,
 	.set_ageing_time = lan937x_set_ageing_time,
+	.mdio_bus_preinit = lan937x_mdio_bus_preinit,
+	.create_phy_addr_map = lan937x_create_phy_addr_map,
 	.r_phy = lan937x_r_phy,
 	.w_phy = lan937x_w_phy,
 	.r_mib_cnt = ksz9477_r_mib_cnt,
@@ -1762,6 +1764,7 @@  const struct ksz_chip_data ksz_switch_chips[] = {
 		.num_tx_queues = 8,
 		.num_ipms = 8,
 		.tc_cbs_supported = true,
+		.phy_side_mdio_supported = true,
 		.ops = &lan937x_dev_ops,
 		.phylink_mac_ops = &lan937x_phylink_mac_ops,
 		.mib_names = ksz9477_mib_names,
@@ -1790,6 +1793,7 @@  const struct ksz_chip_data ksz_switch_chips[] = {
 		.num_tx_queues = 8,
 		.num_ipms = 8,
 		.tc_cbs_supported = true,
+		.phy_side_mdio_supported = true,
 		.ops = &lan937x_dev_ops,
 		.phylink_mac_ops = &lan937x_phylink_mac_ops,
 		.mib_names = ksz9477_mib_names,
@@ -1818,6 +1822,7 @@  const struct ksz_chip_data ksz_switch_chips[] = {
 		.num_tx_queues = 8,
 		.num_ipms = 8,
 		.tc_cbs_supported = true,
+		.phy_side_mdio_supported = true,
 		.ops = &lan937x_dev_ops,
 		.phylink_mac_ops = &lan937x_phylink_mac_ops,
 		.mib_names = ksz9477_mib_names,
@@ -1850,6 +1855,7 @@  const struct ksz_chip_data ksz_switch_chips[] = {
 		.num_tx_queues = 8,
 		.num_ipms = 8,
 		.tc_cbs_supported = true,
+		.phy_side_mdio_supported = true,
 		.ops = &lan937x_dev_ops,
 		.phylink_mac_ops = &lan937x_phylink_mac_ops,
 		.mib_names = ksz9477_mib_names,
@@ -1882,6 +1888,7 @@  const struct ksz_chip_data ksz_switch_chips[] = {
 		.num_tx_queues = 8,
 		.num_ipms = 8,
 		.tc_cbs_supported = true,
+		.phy_side_mdio_supported = true,
 		.ops = &lan937x_dev_ops,
 		.phylink_mac_ops = &lan937x_phylink_mac_ops,
 		.mib_names = ksz9477_mib_names,
diff --git a/drivers/net/dsa/microchip/lan937x.h b/drivers/net/dsa/microchip/lan937x.h
index 3388d91dbc44e..df13ebbd356f9 100644
--- a/drivers/net/dsa/microchip/lan937x.h
+++ b/drivers/net/dsa/microchip/lan937x.h
@@ -13,6 +13,8 @@  void lan937x_port_setup(struct ksz_device *dev, int port, bool cpu_port);
 void lan937x_config_cpu_port(struct dsa_switch *ds);
 int lan937x_switch_init(struct ksz_device *dev);
 void lan937x_switch_exit(struct ksz_device *dev);
+int lan937x_mdio_bus_preinit(struct ksz_device *dev, bool side_mdio);
+int lan937x_create_phy_addr_map(struct ksz_device *dev, bool side_mdio);
 int lan937x_r_phy(struct ksz_device *dev, u16 addr, u16 reg, u16 *data);
 int lan937x_w_phy(struct ksz_device *dev, u16 addr, u16 reg, u16 val);
 int lan937x_change_mtu(struct ksz_device *dev, int port, int new_mtu);
diff --git a/drivers/net/dsa/microchip/lan937x_main.c b/drivers/net/dsa/microchip/lan937x_main.c
index 824d9309a3d35..7dfd21d0d2843 100644
--- a/drivers/net/dsa/microchip/lan937x_main.c
+++ b/drivers/net/dsa/microchip/lan937x_main.c
@@ -18,6 +18,47 @@ 
 #include "ksz9477.h"
 #include "lan937x.h"
 
+static const u8 lan9370_phy_addr[] = {
+	[0] = 2, /* Port 1, T1 AFE0 */
+	[1] = 3, /* Port 2, T1 AFE1 */
+	[2] = 5, /* Port 3, T1 AFE3 */
+	[3] = 6, /* Port 4, T1 AFE4 */
+	[4] = U8_MAX, /* Port 5, RGMII 2 */
+};
+
+static const u8 lan9372_phy_addr[] = {
+	[0] = 2, /* Port 1, T1 AFE0 */
+	[1] = 3, /* Port 2, T1 AFE1 */
+	[2] = 5, /* Port 3, T1 AFE3 */
+	[3] = 8, /* Port 4, TX PHY */
+	[4] = U8_MAX, /* Port 5, RGMII 2 */
+	[5] = U8_MAX, /* Port 6, RGMII 1 */
+	[6] = 6, /* Port 7, T1 AFE4 */
+	[7] = 4, /* Port 8, T1 AFE2 */
+};
+
+static const u8 lan9373_phy_addr[] = {
+	[0] = 2, /* Port 1, T1 AFE0 */
+	[1] = 3, /* Port 2, T1 AFE1 */
+	[2] = 5, /* Port 3, T1 AFE3 */
+	[3] = U8_MAX, /* Port 4, SGMII */
+	[4] = U8_MAX, /* Port 5, RGMII 2 */
+	[5] = U8_MAX, /* Port 6, RGMII 1 */
+	[6] = 6, /* Port 7, T1 AFE4 */
+	[7] = 4, /* Port 8, T1 AFE2 */
+};
+
+static const u8 lan9374_phy_addr[] = {
+	[0] = 2, /* Port 1, T1 AFE0 */
+	[1] = 3, /* Port 2, T1 AFE1 */
+	[2] = 5, /* Port 3, T1 AFE3 */
+	[3] = 7, /* Port 4, T1 AFE5 */
+	[4] = U8_MAX, /* Port 5, RGMII 2 */
+	[5] = U8_MAX, /* Port 6, RGMII 1 */
+	[6] = 6, /* Port 7, T1 AFE4 */
+	[7] = 4, /* Port 8, T1 AFE2 */
+};
+
 static int lan937x_cfg(struct ksz_device *dev, u32 addr, u8 bits, bool set)
 {
 	return regmap_update_bits(ksz_regmap_8(dev), addr, bits, set ? bits : 0);
@@ -30,24 +71,97 @@  static int lan937x_port_cfg(struct ksz_device *dev, int port, int offset,
 				  bits, set ? bits : 0);
 }
 
-static int lan937x_enable_spi_indirect_access(struct ksz_device *dev)
+int lan937x_create_phy_addr_map(struct ksz_device *dev, bool side_mdio)
+{
+	static const u8 *phy_addr_map;
+	u32 strap_val;
+	u8 offset = 0;
+	size_t size;
+	int ret, i;
+
+	if (!side_mdio) {
+		/* simple direct mapping */
+		for (i = 0; i < dev->info->port_cnt; i++)
+			dev->phy_addr_map[i] = i;
+
+		return 0;
+	}
+
+	ret = ksz_read32(dev, REG_SW_CFG_STRAP_VAL, &strap_val);
+	if (ret < 0)
+		return ret;
+
+	if (!(strap_val & SW_CASCADE_ID_CFG) && !(strap_val & SW_VPHY_ADD_CFG))
+		offset = 0;
+	else if (!(strap_val & SW_CASCADE_ID_CFG) && (strap_val & SW_VPHY_ADD_CFG))
+		offset = 7;
+	else if ((strap_val & SW_CASCADE_ID_CFG) && !(strap_val & SW_VPHY_ADD_CFG))
+		offset = 15;
+	else
+		offset = 22;
+
+	switch (dev->info->chip_id) {
+	case LAN9370_CHIP_ID:
+		phy_addr_map = lan9370_phy_addr;
+		size = ARRAY_SIZE(lan9370_phy_addr);
+		break;
+	case LAN9372_CHIP_ID:
+		phy_addr_map = lan9372_phy_addr;
+		size = ARRAY_SIZE(lan9372_phy_addr);
+		break;
+	case LAN9373_CHIP_ID:
+		phy_addr_map = lan9373_phy_addr;
+		size = ARRAY_SIZE(lan9373_phy_addr);
+		break;
+	case LAN9374_CHIP_ID:
+		phy_addr_map = lan9374_phy_addr;
+		size = ARRAY_SIZE(lan9374_phy_addr);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	if (size < dev->info->port_cnt)
+		return -EINVAL;
+
+	for (i = 0; i < dev->info->port_cnt; i++) {
+		if (phy_addr_map[i] == U8_MAX)
+			dev->phy_addr_map[i] = phy_addr_map[i];
+		else
+			dev->phy_addr_map[i] = phy_addr_map[i] + offset;
+	}
+
+	return 0;
+}
+
+int lan937x_mdio_bus_preinit(struct ksz_device *dev, bool side_mdio)
 {
 	u16 data16;
 	int ret;
 
-	/* Enable Phy access through SPI */
+	/* Unlock access to the PHYs, needed for SPI and side MDIO access */
 	ret = lan937x_cfg(dev, REG_GLOBAL_CTRL_0, SW_PHY_REG_BLOCK, false);
 	if (ret < 0)
-		return ret;
+		goto print_error;
 
-	ret = ksz_read16(dev, REG_VPHY_SPECIAL_CTRL__2, &data16);
-	if (ret < 0)
-		return ret;
+	if (side_mdio)
+		/* Allow access to internal PHYs over MDIO bus */
+		data16 = VPHY_MDIO_INTERNAL_ENABLE;
+	else
+		/* Enable SPI indirect access to address clock domain crossing
+		 * issue
+		 */
+		data16 = VPHY_SPI_INDIRECT_ENABLE;
+
+	ret = ksz_rmw16(dev, REG_VPHY_SPECIAL_CTRL__2,
+			VPHY_SPI_INDIRECT_ENABLE | VPHY_MDIO_INTERNAL_ENABLE,
+			data16);
 
-	/* Allow SPI access */
-	data16 |= VPHY_SPI_INDIRECT_ENABLE;
+print_error:
+	if (ret < 0)
+		dev_err(dev->dev, "failed to enable spi indirect access");
 
-	return ksz_write16(dev, REG_VPHY_SPECIAL_CTRL__2, data16);
+	return ret;
 }
 
 static int lan937x_vphy_ind_addr_wr(struct ksz_device *dev, int addr, int reg)
@@ -363,13 +477,6 @@  int lan937x_setup(struct dsa_switch *ds)
 	struct ksz_device *dev = ds->priv;
 	int ret;
 
-	/* enable Indirect Access from SPI to the VPHY registers */
-	ret = lan937x_enable_spi_indirect_access(dev);
-	if (ret < 0) {
-		dev_err(dev->dev, "failed to enable spi indirect access");
-		return ret;
-	}
-
 	/* The VLAN aware is a global setting. Mixed vlan
 	 * filterings are not supported.
 	 */
diff --git a/drivers/net/dsa/microchip/lan937x_reg.h b/drivers/net/dsa/microchip/lan937x_reg.h
index 2f22a9d01de36..4ec93e421da45 100644
--- a/drivers/net/dsa/microchip/lan937x_reg.h
+++ b/drivers/net/dsa/microchip/lan937x_reg.h
@@ -37,6 +37,10 @@ 
 #define SW_CLK125_ENB			BIT(1)
 #define SW_CLK25_ENB			BIT(0)
 
+#define REG_SW_CFG_STRAP_VAL		0x0200
+#define SW_CASCADE_ID_CFG		BIT(15)
+#define SW_VPHY_ADD_CFG			BIT(0)
+
 /* 2 - PHY Control */
 #define REG_SW_CFG_STRAP_OVR		0x0214
 #define SW_VPHY_DISABLE			BIT(31)