[PATCH net-next v1 1/1] net: dsa: microchip: add regmap_range for KSZ9563 chip

[PATCH net-next v1 1/1] net: dsa: microchip: add regmap_range for KSZ9563 chip

[Oleksij Rempel]

Add register validation for KSZ9563.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/net/dsa/microchip/ksz_common.c | 121 +++++++++++++++++++++++++
 1 file changed, 121 insertions(+)

diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 030b167764b39..2308be3bdc9d8 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -666,6 +666,125 @@ static const struct regmap_access_table ksz8563_register_set = {
 	.n_yes_ranges = ARRAY_SIZE(ksz8563_valid_regs),
 };
 
+static const struct regmap_range ksz9563_valid_regs[] = {
+	regmap_reg_range(0x0000, 0x0003),
+	regmap_reg_range(0x0006, 0x0006),
+	regmap_reg_range(0x000f, 0x000f),
+	regmap_reg_range(0x0010, 0x001f),
+	regmap_reg_range(0x0100, 0x0100),
+	regmap_reg_range(0x0104, 0x0107),
+	regmap_reg_range(0x010d, 0x010d),
+	regmap_reg_range(0x0110, 0x0113),
+	regmap_reg_range(0x0120, 0x012b),
+	regmap_reg_range(0x0201, 0x0201),
+	regmap_reg_range(0x0210, 0x0213),
+	regmap_reg_range(0x0300, 0x0300),
+	regmap_reg_range(0x0302, 0x030b),
+	regmap_reg_range(0x030e, 0x031b),
+	regmap_reg_range(0x0320, 0x032b),
+	regmap_reg_range(0x0330, 0x0336),
+	regmap_reg_range(0x0338, 0x033b),
+	regmap_reg_range(0x033e, 0x033e),
+	regmap_reg_range(0x0340, 0x035f),
+	regmap_reg_range(0x0370, 0x0370),
+	regmap_reg_range(0x0378, 0x0378),
+	regmap_reg_range(0x037c, 0x037d),
+	regmap_reg_range(0x0390, 0x0393),
+	regmap_reg_range(0x0400, 0x040e),
+	regmap_reg_range(0x0410, 0x042f),
+	regmap_reg_range(0x0500, 0x0519),
+	regmap_reg_range(0x0520, 0x054b),
+	regmap_reg_range(0x0550, 0x05b3),
+
+	/* port 1 */
+	regmap_reg_range(0x1000, 0x1001),
+	regmap_reg_range(0x1004, 0x100b),
+	regmap_reg_range(0x1013, 0x1013),
+	regmap_reg_range(0x1017, 0x1017),
+	regmap_reg_range(0x101b, 0x101b),
+	regmap_reg_range(0x101f, 0x1021),
+	regmap_reg_range(0x1030, 0x1030),
+	regmap_reg_range(0x1100, 0x1115),
+	regmap_reg_range(0x111a, 0x111f),
+	regmap_reg_range(0x1120, 0x112b),
+	regmap_reg_range(0x1134, 0x113b),
+	regmap_reg_range(0x113c, 0x113f),
+	regmap_reg_range(0x1400, 0x1401),
+	regmap_reg_range(0x1403, 0x1403),
+	regmap_reg_range(0x1410, 0x1417),
+	regmap_reg_range(0x1420, 0x1423),
+	regmap_reg_range(0x1500, 0x1507),
+	regmap_reg_range(0x1600, 0x1612),
+	regmap_reg_range(0x1800, 0x180f),
+	regmap_reg_range(0x1900, 0x1907),
+	regmap_reg_range(0x1914, 0x191b),
+	regmap_reg_range(0x1a00, 0x1a03),
+	regmap_reg_range(0x1a04, 0x1a07),
+	regmap_reg_range(0x1b00, 0x1b01),
+	regmap_reg_range(0x1b04, 0x1b04),
+	regmap_reg_range(0x1c00, 0x1c05),
+	regmap_reg_range(0x1c08, 0x1c1b),
+
+	/* port 2 */
+	regmap_reg_range(0x2000, 0x2001),
+	regmap_reg_range(0x2004, 0x200b),
+	regmap_reg_range(0x2013, 0x2013),
+	regmap_reg_range(0x2017, 0x2017),
+	regmap_reg_range(0x201b, 0x201b),
+	regmap_reg_range(0x201f, 0x2021),
+	regmap_reg_range(0x2030, 0x2030),
+	regmap_reg_range(0x2100, 0x2115),
+	regmap_reg_range(0x211a, 0x211f),
+	regmap_reg_range(0x2120, 0x212b),
+	regmap_reg_range(0x2134, 0x213b),
+	regmap_reg_range(0x213c, 0x213f),
+	regmap_reg_range(0x2400, 0x2401),
+	regmap_reg_range(0x2403, 0x2403),
+	regmap_reg_range(0x2410, 0x2417),
+	regmap_reg_range(0x2420, 0x2423),
+	regmap_reg_range(0x2500, 0x2507),
+	regmap_reg_range(0x2600, 0x2612),
+	regmap_reg_range(0x2800, 0x280f),
+	regmap_reg_range(0x2900, 0x2907),
+	regmap_reg_range(0x2914, 0x291b),
+	regmap_reg_range(0x2a00, 0x2a03),
+	regmap_reg_range(0x2a04, 0x2a07),
+	regmap_reg_range(0x2b00, 0x2b01),
+	regmap_reg_range(0x2b04, 0x2b04),
+	regmap_reg_range(0x2c00, 0x2c05),
+	regmap_reg_range(0x2c08, 0x2c1b),
+
+	/* port 3 */
+	regmap_reg_range(0x3000, 0x3001),
+	regmap_reg_range(0x3013, 0x3013),
+	regmap_reg_range(0x3017, 0x3017),
+	regmap_reg_range(0x301b, 0x301b),
+	regmap_reg_range(0x301f, 0x3020),
+	regmap_reg_range(0x3030, 0x3030),
+	regmap_reg_range(0x3300, 0x3301),
+	regmap_reg_range(0x3303, 0x3303),
+	regmap_reg_range(0x3400, 0x3401),
+	regmap_reg_range(0x3403, 0x3403),
+	regmap_reg_range(0x3410, 0x3417),
+	regmap_reg_range(0x3420, 0x3423),
+	regmap_reg_range(0x3500, 0x3507),
+	regmap_reg_range(0x3600, 0x3612),
+	regmap_reg_range(0x3800, 0x380f),
+	regmap_reg_range(0x3900, 0x3907),
+	regmap_reg_range(0x3914, 0x391b),
+	regmap_reg_range(0x3a00, 0x3a03),
+	regmap_reg_range(0x3a04, 0x3a07),
+	regmap_reg_range(0x3b00, 0x3b01),
+	regmap_reg_range(0x3b04, 0x3b04),
+	regmap_reg_range(0x3c00, 0x3c05),
+	regmap_reg_range(0x3c08, 0x3c1b),
+};
+
+static const struct regmap_access_table ksz9563_register_set = {
+	.yes_ranges = ksz9563_valid_regs,
+	.n_yes_ranges = ARRAY_SIZE(ksz9563_valid_regs),
+};
+
 static const struct regmap_range ksz9477_valid_regs[] = {
 	regmap_reg_range(0x0000, 0x0003),
 	regmap_reg_range(0x0006, 0x0006),
@@ -1475,6 +1594,8 @@ const struct ksz_chip_data ksz_switch_chips[] = {
 		.supports_rgmii = {false, false, true},
 		.internal_phy = {true, true, false},
 		.gbit_capable = {true, true, true},
+		.wr_table = &ksz9563_register_set,
+		.rd_table = &ksz9563_register_set,
 	},
 
 	[KSZ8567] = {
-- 
2.39.2

Re: [PATCH net-next v1 1/1] net: dsa: microchip: add regmap_range for KSZ9563 chip

[Simon Horman]

On Fri, Mar 08, 2024 at 11:50:21AM +0100, Oleksij Rempel wrote:
> Add register validation for KSZ9563.
> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>

Reviewed-by: Simon Horman <horms@kernel.org>

...

Re: [PATCH net-next v1 1/1] net: dsa: microchip: add regmap_range for KSZ9563 chip

[Florian Fainelli]

On 3/8/24 02:50, Oleksij Rempel wrote:
> Add register validation for KSZ9563.
> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
>   drivers/net/dsa/microchip/ksz_common.c | 121 +++++++++++++++++++++++++
>   1 file changed, 121 insertions(+)
> 
> diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
> index 030b167764b39..2308be3bdc9d8 100644
> --- a/drivers/net/dsa/microchip/ksz_common.c
> +++ b/drivers/net/dsa/microchip/ksz_common.c
> @@ -666,6 +666,125 @@ static const struct regmap_access_table ksz8563_register_set = {
>   	.n_yes_ranges = ARRAY_SIZE(ksz8563_valid_regs),
>   };
>   
> +static const struct regmap_range ksz9563_valid_regs[] = {

Missing comment to describe those are global registers, and not per-port 
registers?

> +	regmap_reg_range(0x0000, 0x0003),
> +	regmap_reg_range(0x0006, 0x0006),
> +	regmap_reg_range(0x000f, 0x000f),
> +	regmap_reg_range(0x0010, 0x001f),
> +	regmap_reg_range(0x0100, 0x0100),
> +	regmap_reg_range(0x0104, 0x0107),
> +	regmap_reg_range(0x010d, 0x010d),
> +	regmap_reg_range(0x0110, 0x0113),
> +	regmap_reg_range(0x0120, 0x012b),
> +	regmap_reg_range(0x0201, 0x0201),
> +	regmap_reg_range(0x0210, 0x0213),
> +	regmap_reg_range(0x0300, 0x0300),
> +	regmap_reg_range(0x0302, 0x030b),
> +	regmap_reg_range(0x030e, 0x031b),
> +	regmap_reg_range(0x0320, 0x032b),
> +	regmap_reg_range(0x0330, 0x0336),
> +	regmap_reg_range(0x0338, 0x033b),
> +	regmap_reg_range(0x033e, 0x033e),
> +	regmap_reg_range(0x0340, 0x035f),
> +	regmap_reg_range(0x0370, 0x0370),
> +	regmap_reg_range(0x0378, 0x0378),
> +	regmap_reg_range(0x037c, 0x037d),
> +	regmap_reg_range(0x0390, 0x0393),
> +	regmap_reg_range(0x0400, 0x040e),
> +	regmap_reg_range(0x0410, 0x042f),
> +	regmap_reg_range(0x0500, 0x0519),
> +	regmap_reg_range(0x0520, 0x054b),
> +	regmap_reg_range(0x0550, 0x05b3),
> +
> +	/* port 1 */
> +	regmap_reg_range(0x1000, 0x1001),

Seems like adding a macro that encapsulates all per-port register ranges 
would help a bit?

Other than that, LGTM!
-- 
Florian

[PATCH net-next v1 1/1] net: dsa: microchip: add regmap_range for KSZ9563 chip

[Oleksij Rempel]

Add register validation for KSZ9563.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/net/dsa/microchip/ksz_common.c | 121 +++++++++++++++++++++++++
 1 file changed, 121 insertions(+)

diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 030b167764b39..2308be3bdc9d8 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -666,6 +666,125 @@ static const struct regmap_access_table ksz8563_register_set = {
 	.n_yes_ranges = ARRAY_SIZE(ksz8563_valid_regs),
 };
 
+static const struct regmap_range ksz9563_valid_regs[] = {
+	regmap_reg_range(0x0000, 0x0003),
+	regmap_reg_range(0x0006, 0x0006),
+	regmap_reg_range(0x000f, 0x000f),
+	regmap_reg_range(0x0010, 0x001f),
+	regmap_reg_range(0x0100, 0x0100),
+	regmap_reg_range(0x0104, 0x0107),
+	regmap_reg_range(0x010d, 0x010d),
+	regmap_reg_range(0x0110, 0x0113),
+	regmap_reg_range(0x0120, 0x012b),
+	regmap_reg_range(0x0201, 0x0201),
+	regmap_reg_range(0x0210, 0x0213),
+	regmap_reg_range(0x0300, 0x0300),
+	regmap_reg_range(0x0302, 0x030b),
+	regmap_reg_range(0x030e, 0x031b),
+	regmap_reg_range(0x0320, 0x032b),
+	regmap_reg_range(0x0330, 0x0336),
+	regmap_reg_range(0x0338, 0x033b),
+	regmap_reg_range(0x033e, 0x033e),
+	regmap_reg_range(0x0340, 0x035f),
+	regmap_reg_range(0x0370, 0x0370),
+	regmap_reg_range(0x0378, 0x0378),
+	regmap_reg_range(0x037c, 0x037d),
+	regmap_reg_range(0x0390, 0x0393),
+	regmap_reg_range(0x0400, 0x040e),
+	regmap_reg_range(0x0410, 0x042f),
+	regmap_reg_range(0x0500, 0x0519),
+	regmap_reg_range(0x0520, 0x054b),
+	regmap_reg_range(0x0550, 0x05b3),
+
+	/* port 1 */
+	regmap_reg_range(0x1000, 0x1001),
+	regmap_reg_range(0x1004, 0x100b),
+	regmap_reg_range(0x1013, 0x1013),
+	regmap_reg_range(0x1017, 0x1017),
+	regmap_reg_range(0x101b, 0x101b),
+	regmap_reg_range(0x101f, 0x1021),
+	regmap_reg_range(0x1030, 0x1030),
+	regmap_reg_range(0x1100, 0x1115),
+	regmap_reg_range(0x111a, 0x111f),
+	regmap_reg_range(0x1120, 0x112b),
+	regmap_reg_range(0x1134, 0x113b),
+	regmap_reg_range(0x113c, 0x113f),
+	regmap_reg_range(0x1400, 0x1401),
+	regmap_reg_range(0x1403, 0x1403),
+	regmap_reg_range(0x1410, 0x1417),
+	regmap_reg_range(0x1420, 0x1423),
+	regmap_reg_range(0x1500, 0x1507),
+	regmap_reg_range(0x1600, 0x1612),
+	regmap_reg_range(0x1800, 0x180f),
+	regmap_reg_range(0x1900, 0x1907),
+	regmap_reg_range(0x1914, 0x191b),
+	regmap_reg_range(0x1a00, 0x1a03),
+	regmap_reg_range(0x1a04, 0x1a07),
+	regmap_reg_range(0x1b00, 0x1b01),
+	regmap_reg_range(0x1b04, 0x1b04),
+	regmap_reg_range(0x1c00, 0x1c05),
+	regmap_reg_range(0x1c08, 0x1c1b),
+
+	/* port 2 */
+	regmap_reg_range(0x2000, 0x2001),
+	regmap_reg_range(0x2004, 0x200b),
+	regmap_reg_range(0x2013, 0x2013),
+	regmap_reg_range(0x2017, 0x2017),
+	regmap_reg_range(0x201b, 0x201b),
+	regmap_reg_range(0x201f, 0x2021),
+	regmap_reg_range(0x2030, 0x2030),
+	regmap_reg_range(0x2100, 0x2115),
+	regmap_reg_range(0x211a, 0x211f),
+	regmap_reg_range(0x2120, 0x212b),
+	regmap_reg_range(0x2134, 0x213b),
+	regmap_reg_range(0x213c, 0x213f),
+	regmap_reg_range(0x2400, 0x2401),
+	regmap_reg_range(0x2403, 0x2403),
+	regmap_reg_range(0x2410, 0x2417),
+	regmap_reg_range(0x2420, 0x2423),
+	regmap_reg_range(0x2500, 0x2507),
+	regmap_reg_range(0x2600, 0x2612),
+	regmap_reg_range(0x2800, 0x280f),
+	regmap_reg_range(0x2900, 0x2907),
+	regmap_reg_range(0x2914, 0x291b),
+	regmap_reg_range(0x2a00, 0x2a03),
+	regmap_reg_range(0x2a04, 0x2a07),
+	regmap_reg_range(0x2b00, 0x2b01),
+	regmap_reg_range(0x2b04, 0x2b04),
+	regmap_reg_range(0x2c00, 0x2c05),
+	regmap_reg_range(0x2c08, 0x2c1b),
+
+	/* port 3 */
+	regmap_reg_range(0x3000, 0x3001),
+	regmap_reg_range(0x3013, 0x3013),
+	regmap_reg_range(0x3017, 0x3017),
+	regmap_reg_range(0x301b, 0x301b),
+	regmap_reg_range(0x301f, 0x3020),
+	regmap_reg_range(0x3030, 0x3030),
+	regmap_reg_range(0x3300, 0x3301),
+	regmap_reg_range(0x3303, 0x3303),
+	regmap_reg_range(0x3400, 0x3401),
+	regmap_reg_range(0x3403, 0x3403),
+	regmap_reg_range(0x3410, 0x3417),
+	regmap_reg_range(0x3420, 0x3423),
+	regmap_reg_range(0x3500, 0x3507),
+	regmap_reg_range(0x3600, 0x3612),
+	regmap_reg_range(0x3800, 0x380f),
+	regmap_reg_range(0x3900, 0x3907),
+	regmap_reg_range(0x3914, 0x391b),
+	regmap_reg_range(0x3a00, 0x3a03),
+	regmap_reg_range(0x3a04, 0x3a07),
+	regmap_reg_range(0x3b00, 0x3b01),
+	regmap_reg_range(0x3b04, 0x3b04),
+	regmap_reg_range(0x3c00, 0x3c05),
+	regmap_reg_range(0x3c08, 0x3c1b),
+};
+
+static const struct regmap_access_table ksz9563_register_set = {
+	.yes_ranges = ksz9563_valid_regs,
+	.n_yes_ranges = ARRAY_SIZE(ksz9563_valid_regs),
+};
+
 static const struct regmap_range ksz9477_valid_regs[] = {
 	regmap_reg_range(0x0000, 0x0003),
 	regmap_reg_range(0x0006, 0x0006),
@@ -1475,6 +1594,8 @@ const struct ksz_chip_data ksz_switch_chips[] = {
 		.supports_rgmii = {false, false, true},
 		.internal_phy = {true, true, false},
 		.gbit_capable = {true, true, true},
+		.wr_table = &ksz9563_register_set,
+		.rd_table = &ksz9563_register_set,
 	},
 
 	[KSZ8567] = {
-- 
2.39.2

[PATCH net-next v1 1/3] net: dsa: microchip: lan9372: add 100BaseTX PHY support

[Oleksij Rempel]

From: Lucas Stach <l.stach@pengutronix.de>

On the LAN9372 the 4th internal PHY is a 100BaseTX PHY instead of a 100BaseT1
PHY. The 100BaseTX PHYs have a different base register offset.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/net/dsa/microchip/ksz_common.h   | 1 +
 drivers/net/dsa/microchip/lan937x_main.c | 3 +++
 drivers/net/dsa/microchip/lan937x_reg.h  | 1 +
 3 files changed, 5 insertions(+)

diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index c784fd23a9937..f901cbe7cfdd5 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -22,6 +22,7 @@
 /* all KSZ switches count ports from 1 */
 #define KSZ_PORT_1 0
 #define KSZ_PORT_2 1
+#define KSZ_PORT_4 3
 
 struct ksz_device;
 struct ksz_port;
diff --git a/drivers/net/dsa/microchip/lan937x_main.c b/drivers/net/dsa/microchip/lan937x_main.c
index b479a628b1ae5..e907a5602035c 100644
--- a/drivers/net/dsa/microchip/lan937x_main.c
+++ b/drivers/net/dsa/microchip/lan937x_main.c
@@ -55,6 +55,9 @@ static int lan937x_vphy_ind_addr_wr(struct ksz_device *dev, int addr, int reg)
 	u16 addr_base = REG_PORT_T1_PHY_CTRL_BASE;
 	u16 temp;
 
+	if (dev->info->chip_id == LAN9372_CHIP_ID && addr == KSZ_PORT_4)
+		addr_base = REG_PORT_TX_PHY_CTRL_BASE;
+
 	/* get register address based on the logical port */
 	temp = PORT_CTRL_ADDR(addr, (addr_base + (reg << 2)));
 
diff --git a/drivers/net/dsa/microchip/lan937x_reg.h b/drivers/net/dsa/microchip/lan937x_reg.h
index 45b606b6429f6..7ecada9240233 100644
--- a/drivers/net/dsa/microchip/lan937x_reg.h
+++ b/drivers/net/dsa/microchip/lan937x_reg.h
@@ -147,6 +147,7 @@
 
 /* 1 - Phy */
 #define REG_PORT_T1_PHY_CTRL_BASE	0x0100
+#define REG_PORT_TX_PHY_CTRL_BASE	0x0280
 
 /* 3 - xMII */
 #define PORT_SGMII_SEL			BIT(7)
-- 
2.39.2

[PATCH net-next v1 2/3] net: dsa: microchip: lan937x: force RGMII interface into PHY mode

[Oleksij Rempel]

From: Lucas Stach <l.stach@pengutronix.de>

The register manual and datasheet documentation for the LAN937x series
disagree about the polarity of the MII mode strap. As a consequence
there are hardware designs that have the RGMII interface strapped into
MAC mode, which is a invalid configuration and will prevent the internal
clock from being fed into the port TX interface.

Force the MII mode to PHY for RGMII interfaces where this is the only
valid mode, to override the inproper strapping.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/net/dsa/microchip/lan937x_main.c | 11 +++++++++++
 drivers/net/dsa/microchip/lan937x_reg.h  |  3 +++
 2 files changed, 14 insertions(+)

diff --git a/drivers/net/dsa/microchip/lan937x_main.c b/drivers/net/dsa/microchip/lan937x_main.c
index e907a5602035c..b6ef48a655735 100644
--- a/drivers/net/dsa/microchip/lan937x_main.c
+++ b/drivers/net/dsa/microchip/lan937x_main.c
@@ -217,6 +217,17 @@ void lan937x_config_cpu_port(struct dsa_switch *ds)
 		if (dev->info->cpu_ports & (1 << dp->index)) {
 			dev->cpu_port = dp->index;
 
+			/*
+			 * Force RGMII interface into PHY mode, as that's the
+			 * only valid mode, but it may be in MAC mode due to
+			 * incorrect strapping.
+			 */
+			if (phy_interface_mode_is_rgmii(dev->ports[dp->index].interface)) {
+				lan937x_port_cfg(dev, dp->index,
+						 REG_PORT_XMII_CTRL_1,
+						 PORT_MII_MODE_MAC, false);
+			}
+
 			/* enable cpu port */
 			lan937x_port_setup(dev, dp->index, true);
 		}
diff --git a/drivers/net/dsa/microchip/lan937x_reg.h b/drivers/net/dsa/microchip/lan937x_reg.h
index 7ecada9240233..e36bcb155f545 100644
--- a/drivers/net/dsa/microchip/lan937x_reg.h
+++ b/drivers/net/dsa/microchip/lan937x_reg.h
@@ -150,6 +150,9 @@
 #define REG_PORT_TX_PHY_CTRL_BASE	0x0280
 
 /* 3 - xMII */
+#define REG_PORT_XMII_CTRL_1		0x0301
+#define PORT_MII_MODE_MAC		BIT(2)
+
 #define PORT_SGMII_SEL			BIT(7)
 #define PORT_GRXC_ENABLE		BIT(0)
 
-- 
2.39.2

[PATCH net-next v1 3/3] net: dsa: microchip: lan937x: disable VPHY output

[Oleksij Rempel]

From: Lucas Stach <l.stach@pengutronix.de>

While the documented VPHY out-of-reset configuration should autonegotiate
the maximum supported link speed on the CPU interface, that doesn't work
on RGMII links, where the VPHY negotiates 100MBit speed. This causes the
RGMII TX interface to run with a wrong clock rate.

Disable the VPHY output altogether, so it doesn't interfere with the
CPU interface configuration set via fixed-link. The VPHY is a compatibility
functionality to be able to attach network drivers without fixed-link
support to the switch, which generally should not be needed with linux
network drivers.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/net/dsa/microchip/lan937x_main.c | 3 +++
 drivers/net/dsa/microchip/lan937x_reg.h  | 4 ++++
 2 files changed, 7 insertions(+)

diff --git a/drivers/net/dsa/microchip/lan937x_main.c b/drivers/net/dsa/microchip/lan937x_main.c
index b6ef48a655735..9cd3ff38ed66b 100644
--- a/drivers/net/dsa/microchip/lan937x_main.c
+++ b/drivers/net/dsa/microchip/lan937x_main.c
@@ -400,6 +400,9 @@ int lan937x_setup(struct dsa_switch *ds)
 	lan937x_cfg(dev, REG_SW_GLOBAL_OUTPUT_CTRL__1,
 		    (SW_CLK125_ENB | SW_CLK25_ENB), true);
 
+	/* disable VPHY output*/
+	ksz_rmw32(dev, REG_SW_CFG_STRAP_OVR, SW_VPHY_DISABLE, SW_VPHY_DISABLE);
+
 	return 0;
 }
 
diff --git a/drivers/net/dsa/microchip/lan937x_reg.h b/drivers/net/dsa/microchip/lan937x_reg.h
index e36bcb155f545..be553e23a964e 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)
 
+/* 2 - PHY Control */
+#define REG_SW_CFG_STRAP_OVR		0x214
+#define SW_VPHY_DISABLE			BIT(31)
+
 /* 3 - Operation Control */
 #define REG_SW_OPERATION		0x0300
 
-- 
2.39.2

Re: [PATCH net-next v1 1/1] net: dsa: microchip: add regmap_range for KSZ9563 chip

[Oleksij Rempel]

Please ignore this patch, it was send by accident.

On Thu, Jun 27, 2024 at 02:39:08PM +0200, Oleksij Rempel wrote:
> Add register validation for KSZ9563.
> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
>  drivers/net/dsa/microchip/ksz_common.c | 121 +++++++++++++++++++++++++
>  1 file changed, 121 insertions(+)
> 
> diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
> index 030b167764b39..2308be3bdc9d8 100644
> --- a/drivers/net/dsa/microchip/ksz_common.c
> +++ b/drivers/net/dsa/microchip/ksz_common.c
> @@ -666,6 +666,125 @@ static const struct regmap_access_table ksz8563_register_set = {
>  	.n_yes_ranges = ARRAY_SIZE(ksz8563_valid_regs),
>  };
>  
> +static const struct regmap_range ksz9563_valid_regs[] = {
> +	regmap_reg_range(0x0000, 0x0003),
> +	regmap_reg_range(0x0006, 0x0006),
> +	regmap_reg_range(0x000f, 0x000f),
> +	regmap_reg_range(0x0010, 0x001f),
> +	regmap_reg_range(0x0100, 0x0100),
> +	regmap_reg_range(0x0104, 0x0107),
> +	regmap_reg_range(0x010d, 0x010d),
> +	regmap_reg_range(0x0110, 0x0113),
> +	regmap_reg_range(0x0120, 0x012b),
> +	regmap_reg_range(0x0201, 0x0201),
> +	regmap_reg_range(0x0210, 0x0213),
> +	regmap_reg_range(0x0300, 0x0300),
> +	regmap_reg_range(0x0302, 0x030b),
> +	regmap_reg_range(0x030e, 0x031b),
> +	regmap_reg_range(0x0320, 0x032b),
> +	regmap_reg_range(0x0330, 0x0336),
> +	regmap_reg_range(0x0338, 0x033b),
> +	regmap_reg_range(0x033e, 0x033e),
> +	regmap_reg_range(0x0340, 0x035f),
> +	regmap_reg_range(0x0370, 0x0370),
> +	regmap_reg_range(0x0378, 0x0378),
> +	regmap_reg_range(0x037c, 0x037d),
> +	regmap_reg_range(0x0390, 0x0393),
> +	regmap_reg_range(0x0400, 0x040e),
> +	regmap_reg_range(0x0410, 0x042f),
> +	regmap_reg_range(0x0500, 0x0519),
> +	regmap_reg_range(0x0520, 0x054b),
> +	regmap_reg_range(0x0550, 0x05b3),
> +
> +	/* port 1 */
> +	regmap_reg_range(0x1000, 0x1001),
> +	regmap_reg_range(0x1004, 0x100b),
> +	regmap_reg_range(0x1013, 0x1013),
> +	regmap_reg_range(0x1017, 0x1017),
> +	regmap_reg_range(0x101b, 0x101b),
> +	regmap_reg_range(0x101f, 0x1021),
> +	regmap_reg_range(0x1030, 0x1030),
> +	regmap_reg_range(0x1100, 0x1115),
> +	regmap_reg_range(0x111a, 0x111f),
> +	regmap_reg_range(0x1120, 0x112b),
> +	regmap_reg_range(0x1134, 0x113b),
> +	regmap_reg_range(0x113c, 0x113f),
> +	regmap_reg_range(0x1400, 0x1401),
> +	regmap_reg_range(0x1403, 0x1403),
> +	regmap_reg_range(0x1410, 0x1417),
> +	regmap_reg_range(0x1420, 0x1423),
> +	regmap_reg_range(0x1500, 0x1507),
> +	regmap_reg_range(0x1600, 0x1612),
> +	regmap_reg_range(0x1800, 0x180f),
> +	regmap_reg_range(0x1900, 0x1907),
> +	regmap_reg_range(0x1914, 0x191b),
> +	regmap_reg_range(0x1a00, 0x1a03),
> +	regmap_reg_range(0x1a04, 0x1a07),
> +	regmap_reg_range(0x1b00, 0x1b01),
> +	regmap_reg_range(0x1b04, 0x1b04),
> +	regmap_reg_range(0x1c00, 0x1c05),
> +	regmap_reg_range(0x1c08, 0x1c1b),
> +
> +	/* port 2 */
> +	regmap_reg_range(0x2000, 0x2001),
> +	regmap_reg_range(0x2004, 0x200b),
> +	regmap_reg_range(0x2013, 0x2013),
> +	regmap_reg_range(0x2017, 0x2017),
> +	regmap_reg_range(0x201b, 0x201b),
> +	regmap_reg_range(0x201f, 0x2021),
> +	regmap_reg_range(0x2030, 0x2030),
> +	regmap_reg_range(0x2100, 0x2115),
> +	regmap_reg_range(0x211a, 0x211f),
> +	regmap_reg_range(0x2120, 0x212b),
> +	regmap_reg_range(0x2134, 0x213b),
> +	regmap_reg_range(0x213c, 0x213f),
> +	regmap_reg_range(0x2400, 0x2401),
> +	regmap_reg_range(0x2403, 0x2403),
> +	regmap_reg_range(0x2410, 0x2417),
> +	regmap_reg_range(0x2420, 0x2423),
> +	regmap_reg_range(0x2500, 0x2507),
> +	regmap_reg_range(0x2600, 0x2612),
> +	regmap_reg_range(0x2800, 0x280f),
> +	regmap_reg_range(0x2900, 0x2907),
> +	regmap_reg_range(0x2914, 0x291b),
> +	regmap_reg_range(0x2a00, 0x2a03),
> +	regmap_reg_range(0x2a04, 0x2a07),
> +	regmap_reg_range(0x2b00, 0x2b01),
> +	regmap_reg_range(0x2b04, 0x2b04),
> +	regmap_reg_range(0x2c00, 0x2c05),
> +	regmap_reg_range(0x2c08, 0x2c1b),
> +
> +	/* port 3 */
> +	regmap_reg_range(0x3000, 0x3001),
> +	regmap_reg_range(0x3013, 0x3013),
> +	regmap_reg_range(0x3017, 0x3017),
> +	regmap_reg_range(0x301b, 0x301b),
> +	regmap_reg_range(0x301f, 0x3020),
> +	regmap_reg_range(0x3030, 0x3030),
> +	regmap_reg_range(0x3300, 0x3301),
> +	regmap_reg_range(0x3303, 0x3303),
> +	regmap_reg_range(0x3400, 0x3401),
> +	regmap_reg_range(0x3403, 0x3403),
> +	regmap_reg_range(0x3410, 0x3417),
> +	regmap_reg_range(0x3420, 0x3423),
> +	regmap_reg_range(0x3500, 0x3507),
> +	regmap_reg_range(0x3600, 0x3612),
> +	regmap_reg_range(0x3800, 0x380f),
> +	regmap_reg_range(0x3900, 0x3907),
> +	regmap_reg_range(0x3914, 0x391b),
> +	regmap_reg_range(0x3a00, 0x3a03),
> +	regmap_reg_range(0x3a04, 0x3a07),
> +	regmap_reg_range(0x3b00, 0x3b01),
> +	regmap_reg_range(0x3b04, 0x3b04),
> +	regmap_reg_range(0x3c00, 0x3c05),
> +	regmap_reg_range(0x3c08, 0x3c1b),
> +};
> +
> +static const struct regmap_access_table ksz9563_register_set = {
> +	.yes_ranges = ksz9563_valid_regs,
> +	.n_yes_ranges = ARRAY_SIZE(ksz9563_valid_regs),
> +};
> +
>  static const struct regmap_range ksz9477_valid_regs[] = {
>  	regmap_reg_range(0x0000, 0x0003),
>  	regmap_reg_range(0x0006, 0x0006),
> @@ -1475,6 +1594,8 @@ const struct ksz_chip_data ksz_switch_chips[] = {
>  		.supports_rgmii = {false, false, true},
>  		.internal_phy = {true, true, false},
>  		.gbit_capable = {true, true, true},
> +		.wr_table = &ksz9563_register_set,
> +		.rd_table = &ksz9563_register_set,
>  	},
>  
>  	[KSZ8567] = {
> -- 
> 2.39.2
> 
> 
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH net-next v1 2/3] net: dsa: microchip: lan937x: force RGMII interface into PHY mode

[Andrew Lunn]

On Thu, Jun 27, 2024 at 02:39:10PM +0200, Oleksij Rempel wrote:
> From: Lucas Stach <l.stach@pengutronix.de>
> 
> The register manual and datasheet documentation for the LAN937x series
> disagree about the polarity of the MII mode strap. As a consequence
> there are hardware designs that have the RGMII interface strapped into
> MAC mode, which is a invalid configuration and will prevent the internal
> clock from being fed into the port TX interface.

What i think is missing from this is that you are talking about the
CPU port. For a normal user point, RGMII MAC mode would make sense, if
there is an external RGMII PHY attached. And the code only does this
if the port is a CPU port.

So maybe:

... that have the CPU port RGMII interface ...

	Andrew

Re: [PATCH net-next v1 1/1] net: dsa: microchip: add regmap_range for KSZ9563 chip

[Jakub Kicinski]

On Thu, 27 Jun 2024 14:43:37 +0200 Oleksij Rempel wrote:
> Please ignore this patch, it was send by accident.

It fooled patchwork tho :(
-- 
pw-bot: cr

Re: [PATCH net-next v1 2/3] net: dsa: microchip: lan937x: force RGMII interface into PHY mode

[Vladimir Oltean]

On Thu, Jun 27, 2024 at 02:39:10PM +0200, Oleksij Rempel wrote:
> From: Lucas Stach <l.stach@pengutronix.de>
> 
> The register manual and datasheet documentation for the LAN937x series
> disagree about the polarity of the MII mode strap. As a consequence
> there are hardware designs that have the RGMII interface strapped into
> MAC mode, which is a invalid configuration and will prevent the internal
> clock from being fed into the port TX interface.
> 
> Force the MII mode to PHY for RGMII interfaces where this is the only
> valid mode, to override the inproper strapping.
> 
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---

What's the difference between MAC mode and PHY mode with RGMII for this switch?

Re: [PATCH net-next v1 3/3] net: dsa: microchip: lan937x: disable VPHY output

[Vladimir Oltean]

On Thu, Jun 27, 2024 at 02:39:11PM +0200, Oleksij Rempel wrote:
> The VPHY is a compatibility functionality to be able to attach network
> drivers without fixed-link support to the switch, which generally
> should not be needed with linux network drivers.

Sorry, I don't have much to base my judgement upon. I did search for the
"VPHY" string and found it to be accessed in the dev_ops->r_phy() and
dev_ops->w_phy() implementations, suggesting that it is more than just
that? These methods are used for accessing the registers of the embedded
PHYs for user ports. I don't see what is the connection with RGMII on
the CPU port.

Re: [PATCH net-next v1 1/3] net: dsa: microchip: lan9372: add 100BaseTX PHY support

[Arun.Ramadoss]

Hi Oleksij,

On Thu, 2024-06-27 at 14:39 +0200, Oleksij Rempel wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
> 
> From: Lucas Stach <l.stach@pengutronix.de>
> 
> On the LAN9372 the 4th internal PHY is a 100BaseTX PHY instead of a
> 100BaseT1
> PHY. The 100BaseTX PHYs have a different base register offset.

LAN9371 also has 4th internal phy as 100BaseTX Phy. Can you also add it
in this patch. 

Re: [PATCH net-next v1 3/3] net: dsa: microchip: lan937x: disable VPHY output

[Oleksij Rempel]

On Fri, Jun 28, 2024 at 01:38:18AM +0300, Vladimir Oltean wrote:
> On Thu, Jun 27, 2024 at 02:39:11PM +0200, Oleksij Rempel wrote:
> > The VPHY is a compatibility functionality to be able to attach network
> > drivers without fixed-link support to the switch, which generally
> > should not be needed with linux network drivers.
> 
> Sorry, I don't have much to base my judgement upon. I did search for the
> "VPHY" string and found it to be accessed in the dev_ops->r_phy() and
> dev_ops->w_phy() implementations, suggesting that it is more than just
> that? These methods are used for accessing the registers of the embedded
> PHYs for user ports. I don't see what is the connection with RGMII on
> the CPU port.

My understanding of the VPHY concept in the LAN937x switches is as
follows:

The VPHY in the LAN937x series provides an emulated MII management
interface (MDIO) per IEEE 802.3, allowing a MAC with an unmodified
driver to interact with the switch as if it is attached to a single port
PHY. This emulation includes a full bank of registers that comply with
IEEE 802.3 and supports auto-negotiation to configure link parameters.
At the same time, this functionality seems to be used to handle clock
crossings for integrated PHYs. To avoid a degradation of SPI_CLK, an
indirect mechanism has been added to the VPHY for accessing the PHY
registers.

However, when VPHY mode is enabled, it defaults to a 100Mbit link speed
during auto-negotiation, particularly affecting RGMII links. This
behavior overrides the MAC configuration set via the P_XMII_CTRL_1
register, which should allow for choosing between 10, 100, and 1000Mbit
speeds, as done similarly in the KSZ9477 variants.

The problem arises because, with VPHY enabled, the MAC configuration on
the CPU port is ignored, and the system is stuck at the default 100Mbit
speed. Disabling the VPHY output ensures that the MAC configuration set
via the P_XMII_CTRL_1 register is respected, allowing the CPU port to
operate at the desired speed (10, 100, or 1000Mbit).

I tried to configure the CPU MAC by using the VPHY interface but had no
luck with it (maybe I handled it wrong). This change was tested on my
system, and I do not see a visible degradation in the functionality of
the integrated PHYs. This might still work because the SPI clock on my
board is limited to 5MHz.

The following article seems to confirm the behavior I observed and
supports the proposed solution:
https://microchip.my.site.com/s/article/LAN937X-The-required-configuration-for-the-external-MAC-port-to-operate-at-RGMII-to-RGMII-1Gbps-link-speed

Regards,
Oleksij
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH net-next v1 2/3] net: dsa: microchip: lan937x: force RGMII interface into PHY mode

[Oleksij Rempel]

On Fri, Jun 28, 2024 at 01:25:43AM +0300, Vladimir Oltean wrote:
> On Thu, Jun 27, 2024 at 02:39:10PM +0200, Oleksij Rempel wrote:
> > From: Lucas Stach <l.stach@pengutronix.de>
> > 
> > The register manual and datasheet documentation for the LAN937x series
> > disagree about the polarity of the MII mode strap. As a consequence
> > there are hardware designs that have the RGMII interface strapped into
> > MAC mode, which is a invalid configuration and will prevent the internal
> > clock from being fed into the port TX interface.
> > 
> > Force the MII mode to PHY for RGMII interfaces where this is the only
> > valid mode, to override the inproper strapping.
> > 
> > Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> > ---
> 
> What's the difference between MAC mode and PHY mode with RGMII for this switch?

Let's take a step back. I'll describe first my initial findings, the symptoms,
and my new findings from today, so my argumentation and the patch itself should
be updated.

Initially, we identified that the RGMIIx_MODE[1,0] strap pins select between
RGMII, RMII, and MII. The MIIx_PHY_MODE pin configures PHY mode for MII or
clock direction in RMII but should have no effect in RGMII mode. However, if
MIIx_PHY_MODE = 1, RGMII exhibits the following symptoms:

- No signal on RGMII TXD[]
- No TX counters increase on the related MAC port.
- RX interface works, and data from the CPU through the switch is properly
  accounted for.

Due to the absence of TX counters even for broadcast traffic, we interpreted
this as a disabled MAC TX functionality or disabled TX clock for the MAC. This
issue was resolved by unsetting Bit 2 on register 0x301, which is undocumented
for RGMII.

Now, comparing LAN937x documentation with publicly available documentation for
other switches, for example KSZ9893R, may give some clue on the undocumented
part in the LAN937x datasheet:
RGMII Interface:
 1 = In-Band Status (IBS) enabled (requires IBS-capable PHY)
 0 = IBS disabled

The issue likely stems from active IBS mode, confirmed by an article
recommending IBS disablement via register 0x302.
https://microchip.my.site.com/s/article/LAN937X-The-required-configuration-for-the-external-MAC-port-to-operate-at-RGMII-to-RGMII-1Gbps-link-speed

The same effect seems to be achieved by toggling the undocumented 0x301 bit 2.
The ksz_set_xmii() function contains existing code to handle this:
	case PHY_INTERFACE_MODE_RGMII_RXID:
	    data8 |= bitval[P_RGMII_SEL];
	    /* On KSZ9893, disable RGMII in-band status support */
	    if (dev->chip_id == KSZ9893_CHIP_ID ||
		dev->chip_id == KSZ8563_CHIP_ID ||
		dev->chip_id == KSZ9563_CHIP_ID)
		data8 &= ~P_MII_MAC_MODE;
	    break;
	default:

Regards,
Oleksij
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH net-next v1 3/3] net: dsa: microchip: lan937x: disable VPHY output

[Lucas Stach]

Am Freitag, dem 28.06.2024 um 01:38 +0300 schrieb Vladimir Oltean:
> On Thu, Jun 27, 2024 at 02:39:11PM +0200, Oleksij Rempel wrote:
> > The VPHY is a compatibility functionality to be able to attach network
> > drivers without fixed-link support to the switch, which generally
> > should not be needed with linux network drivers.
> 
> Sorry, I don't have much to base my judgement upon. I did search for the
> "VPHY" string and found it to be accessed in the dev_ops->r_phy() and
> dev_ops->w_phy() implementations, suggesting that it is more than just
> that? These methods are used for accessing the registers of the embedded
> PHYs for user ports. I don't see what is the connection with RGMII on
> the CPU port.

There is a bit of a mixup with the names here. The VPHY (as in virtual
PHY) is a emulated PHY register space accessible via MDIO to allow
operating systems that don't support the concept of direct MAC to MAC
connections to work with the switch. However, it is buggy and the
emulated auto-negotiation does the wrong thing for RGMII interfaces. As
this part isn't needed for Linux we disable it with this patch, or to
be precise the VPHY itself isn't disabled, but rather the result of the
VPHY state machine isn't allowed to override explicit link
configurations in other registers anymore.

The VPHY used by the driver to access the registers of real PHYs is
described in the datasheet like this:

"Direct access to the PHY registers via SPI requires a reduced SPI_CLK
frequency. This is due to the latency incurred from clock crossings
from the internal bus and into the PHY. To avoid a degradation of
SPI_CLK, an indirect mechanism has been added to the VPHY for accessing
the PHY registers via Indirect Address Register, Indirect Data
Register, and Indirect Control Register."

This mechanism is located in the VPHY register range, but otherwise has
nothing to do with the VPHY state machine and is also not affected by
this patch.

Regards,
Lucas

Re: [PATCH net-next v1 2/3] net: dsa: microchip: lan937x: force RGMII interface into PHY mode

[Andrew Lunn]

> Now, comparing LAN937x documentation with publicly available documentation for
> other switches, for example KSZ9893R, may give some clue on the undocumented
> part in the LAN937x datasheet:
> RGMII Interface:
>  1 = In-Band Status (IBS) enabled (requires IBS-capable PHY)
>  0 = IBS disabled

That explains a lot.

With the updated commit message, this patch should be fine.

     Andrew