2022-08-23 08:11:53

by Oleksij Rempel

[permalink] [raw]
Subject: [PATCH net-next v3 00/17] net: dsa: microchip: add error handling and register access validation

changes v3:
- fix build error in the middle of the patch stack.

changes v2:
- add regmap_ranges for KSZ9477
- drop output clock devicetree in driver validation patches. DTs need
some more refactoring and can be done in a separate patch set.
- remove some unused variables.

This patch series adds error handling for the PHY read/write path and optional
register access validation.
After adding regmap_ranges for KSZ8563 some bugs was detected, so
critical bug fixes are sorted before ragmap_range patch.

Potentially this bug fixes can be ported to stable kernels, but need to be
reworked.

Oleksij Rempel (17):
net: dsa: microchip: add separate struct ksz_chip_data for KSZ8563
chip
net: dsa: microchip: do per-port Gbit detection instead of per-chip
net: dsa: microchip: don't announce extended register support on non
Gbit chips
net: dsa: microchip: allow to pass return values for PHY read/write
accesses
net: dsa: microchip: forward error value on all ksz_pread/ksz_pwrite
functions
net: dsa: microchip: ksz9477: add error handling to ksz9477_r/w_phy
net: dsa: microchip: ksz8795: add error handling to ksz8_r/w_phy
net: dsa: microchip: KSZ9893: do not write to not supported Output
Clock Control Register
net: dsa: microchip: add support for regmap_access_tables
net: dsa: microchip: add regmap_range for KSZ8563 chip
net: dsa: microchip: ksz9477: remove MII_CTRL1000 check from
ksz9477_w_phy()
net: dsa: microchip: add regmap_range for KSZ9477 chip
net: dsa: microchip: ksz9477: use internal_phy instead of phy_port_cnt
net: dsa: microchip: remove unused port phy variable
net: dsa: microchip: ksz9477: remove unused "on" variable
net: dsa: microchip: remove unused sgmii variable
net: dsa: microchip: remove IS_9893 flag

drivers/net/dsa/microchip/ksz8.h | 4 +-
drivers/net/dsa/microchip/ksz8795.c | 111 ++++--
drivers/net/dsa/microchip/ksz9477.c | 86 ++---
drivers/net/dsa/microchip/ksz9477.h | 4 +-
drivers/net/dsa/microchip/ksz_common.c | 450 ++++++++++++++++++++++-
drivers/net/dsa/microchip/ksz_common.h | 90 +++--
drivers/net/dsa/microchip/ksz_spi.c | 5 +-
drivers/net/dsa/microchip/lan937x.h | 4 +-
drivers/net/dsa/microchip/lan937x_main.c | 8 +-
9 files changed, 646 insertions(+), 116 deletions(-)

--
2.30.2


2022-08-23 08:45:15

by Oleksij Rempel

[permalink] [raw]
Subject: [PATCH net-next v3 12/17] net: dsa: microchip: add regmap_range for KSZ9477 chip

Add register validation for KSZ9477

Signed-off-by: Oleksij Rempel <[email protected]>
---
drivers/net/dsa/microchip/ksz_common.c | 272 +++++++++++++++++++++++++
1 file changed, 272 insertions(+)

diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index e2a698e16e023..a863d4feb4135 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -531,6 +531,276 @@ static const struct regmap_access_table ksz8563_register_set = {
.n_yes_ranges = ARRAY_SIZE(ksz8563_valid_regs),
};

+static const struct regmap_range ksz9477_valid_regs[] = {
+ regmap_reg_range(0x0000, 0x0003),
+ regmap_reg_range(0x0006, 0x0006),
+ regmap_reg_range(0x0010, 0x001f),
+ regmap_reg_range(0x0100, 0x0100),
+ regmap_reg_range(0x0103, 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, 0x031b),
+ regmap_reg_range(0x0320, 0x032b),
+ regmap_reg_range(0x0330, 0x0336),
+ regmap_reg_range(0x0338, 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(0x0444, 0x044b),
+ regmap_reg_range(0x0450, 0x046f),
+ regmap_reg_range(0x0500, 0x0519),
+ regmap_reg_range(0x0520, 0x054b),
+ regmap_reg_range(0x0550, 0x05b3),
+ regmap_reg_range(0x0604, 0x060b),
+ regmap_reg_range(0x0610, 0x0612),
+ regmap_reg_range(0x0614, 0x062c),
+ regmap_reg_range(0x0640, 0x0645),
+ regmap_reg_range(0x0648, 0x064d),
+
+ /* port 1 */
+ regmap_reg_range(0x1000, 0x1001),
+ regmap_reg_range(0x1013, 0x1013),
+ regmap_reg_range(0x1017, 0x1017),
+ regmap_reg_range(0x101b, 0x101b),
+ regmap_reg_range(0x101f, 0x1020),
+ regmap_reg_range(0x1030, 0x1030),
+ regmap_reg_range(0x1100, 0x1115),
+ regmap_reg_range(0x111a, 0x111f),
+ regmap_reg_range(0x1122, 0x1127),
+ regmap_reg_range(0x112a, 0x112b),
+ regmap_reg_range(0x1136, 0x1139),
+ regmap_reg_range(0x113e, 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, 0x1613),
+ regmap_reg_range(0x1800, 0x180f),
+ regmap_reg_range(0x1820, 0x1827),
+ regmap_reg_range(0x1830, 0x1837),
+ regmap_reg_range(0x1840, 0x184b),
+ regmap_reg_range(0x1900, 0x1907),
+ regmap_reg_range(0x1914, 0x191b),
+ regmap_reg_range(0x1920, 0x1920),
+ regmap_reg_range(0x1923, 0x1927),
+ 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(0x2013, 0x2013),
+ regmap_reg_range(0x2017, 0x2017),
+ regmap_reg_range(0x201b, 0x201b),
+ regmap_reg_range(0x201f, 0x2020),
+ regmap_reg_range(0x2030, 0x2030),
+ regmap_reg_range(0x2100, 0x2115),
+ regmap_reg_range(0x211a, 0x211f),
+ regmap_reg_range(0x2122, 0x2127),
+ regmap_reg_range(0x212a, 0x212b),
+ regmap_reg_range(0x2136, 0x2139),
+ regmap_reg_range(0x213e, 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, 0x2613),
+ regmap_reg_range(0x2800, 0x280f),
+ regmap_reg_range(0x2820, 0x2827),
+ regmap_reg_range(0x2830, 0x2837),
+ regmap_reg_range(0x2840, 0x284b),
+ regmap_reg_range(0x2900, 0x2907),
+ regmap_reg_range(0x2914, 0x291b),
+ regmap_reg_range(0x2920, 0x2920),
+ regmap_reg_range(0x2923, 0x2927),
+ 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(0x3100, 0x3115),
+ regmap_reg_range(0x311a, 0x311f),
+ regmap_reg_range(0x3122, 0x3127),
+ regmap_reg_range(0x312a, 0x312b),
+ regmap_reg_range(0x3136, 0x3139),
+ regmap_reg_range(0x313e, 0x313f),
+ 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, 0x3613),
+ regmap_reg_range(0x3800, 0x380f),
+ regmap_reg_range(0x3820, 0x3827),
+ regmap_reg_range(0x3830, 0x3837),
+ regmap_reg_range(0x3840, 0x384b),
+ regmap_reg_range(0x3900, 0x3907),
+ regmap_reg_range(0x3914, 0x391b),
+ regmap_reg_range(0x3920, 0x3920),
+ regmap_reg_range(0x3923, 0x3927),
+ 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),
+
+ /* port 4 */
+ regmap_reg_range(0x4000, 0x4001),
+ regmap_reg_range(0x4013, 0x4013),
+ regmap_reg_range(0x4017, 0x4017),
+ regmap_reg_range(0x401b, 0x401b),
+ regmap_reg_range(0x401f, 0x4020),
+ regmap_reg_range(0x4030, 0x4030),
+ regmap_reg_range(0x4100, 0x4115),
+ regmap_reg_range(0x411a, 0x411f),
+ regmap_reg_range(0x4122, 0x4127),
+ regmap_reg_range(0x412a, 0x412b),
+ regmap_reg_range(0x4136, 0x4139),
+ regmap_reg_range(0x413e, 0x413f),
+ regmap_reg_range(0x4400, 0x4401),
+ regmap_reg_range(0x4403, 0x4403),
+ regmap_reg_range(0x4410, 0x4417),
+ regmap_reg_range(0x4420, 0x4423),
+ regmap_reg_range(0x4500, 0x4507),
+ regmap_reg_range(0x4600, 0x4613),
+ regmap_reg_range(0x4800, 0x480f),
+ regmap_reg_range(0x4820, 0x4827),
+ regmap_reg_range(0x4830, 0x4837),
+ regmap_reg_range(0x4840, 0x484b),
+ regmap_reg_range(0x4900, 0x4907),
+ regmap_reg_range(0x4914, 0x491b),
+ regmap_reg_range(0x4920, 0x4920),
+ regmap_reg_range(0x4923, 0x4927),
+ regmap_reg_range(0x4a00, 0x4a03),
+ regmap_reg_range(0x4a04, 0x4a07),
+ regmap_reg_range(0x4b00, 0x4b01),
+ regmap_reg_range(0x4b04, 0x4b04),
+ regmap_reg_range(0x4c00, 0x4c05),
+ regmap_reg_range(0x4c08, 0x4c1b),
+
+ /* port 5 */
+ regmap_reg_range(0x5000, 0x5001),
+ regmap_reg_range(0x5013, 0x5013),
+ regmap_reg_range(0x5017, 0x5017),
+ regmap_reg_range(0x501b, 0x501b),
+ regmap_reg_range(0x501f, 0x5020),
+ regmap_reg_range(0x5030, 0x5030),
+ regmap_reg_range(0x5100, 0x5115),
+ regmap_reg_range(0x511a, 0x511f),
+ regmap_reg_range(0x5122, 0x5127),
+ regmap_reg_range(0x512a, 0x512b),
+ regmap_reg_range(0x5136, 0x5139),
+ regmap_reg_range(0x513e, 0x513f),
+ regmap_reg_range(0x5400, 0x5401),
+ regmap_reg_range(0x5403, 0x5403),
+ regmap_reg_range(0x5410, 0x5417),
+ regmap_reg_range(0x5420, 0x5423),
+ regmap_reg_range(0x5500, 0x5507),
+ regmap_reg_range(0x5600, 0x5613),
+ regmap_reg_range(0x5800, 0x580f),
+ regmap_reg_range(0x5820, 0x5827),
+ regmap_reg_range(0x5830, 0x5837),
+ regmap_reg_range(0x5840, 0x584b),
+ regmap_reg_range(0x5900, 0x5907),
+ regmap_reg_range(0x5914, 0x591b),
+ regmap_reg_range(0x5920, 0x5920),
+ regmap_reg_range(0x5923, 0x5927),
+ regmap_reg_range(0x5a00, 0x5a03),
+ regmap_reg_range(0x5a04, 0x5a07),
+ regmap_reg_range(0x5b00, 0x5b01),
+ regmap_reg_range(0x5b04, 0x5b04),
+ regmap_reg_range(0x5c00, 0x5c05),
+ regmap_reg_range(0x5c08, 0x5c1b),
+
+ /* port 6 */
+ regmap_reg_range(0x6000, 0x6001),
+ regmap_reg_range(0x6013, 0x6013),
+ regmap_reg_range(0x6017, 0x6017),
+ regmap_reg_range(0x601b, 0x601b),
+ regmap_reg_range(0x601f, 0x6020),
+ regmap_reg_range(0x6030, 0x6030),
+ regmap_reg_range(0x6300, 0x6301),
+ regmap_reg_range(0x6400, 0x6401),
+ regmap_reg_range(0x6403, 0x6403),
+ regmap_reg_range(0x6410, 0x6417),
+ regmap_reg_range(0x6420, 0x6423),
+ regmap_reg_range(0x6500, 0x6507),
+ regmap_reg_range(0x6600, 0x6613),
+ regmap_reg_range(0x6800, 0x680f),
+ regmap_reg_range(0x6820, 0x6827),
+ regmap_reg_range(0x6830, 0x6837),
+ regmap_reg_range(0x6840, 0x684b),
+ regmap_reg_range(0x6900, 0x6907),
+ regmap_reg_range(0x6914, 0x691b),
+ regmap_reg_range(0x6920, 0x6920),
+ regmap_reg_range(0x6923, 0x6927),
+ regmap_reg_range(0x6a00, 0x6a03),
+ regmap_reg_range(0x6a04, 0x6a07),
+ regmap_reg_range(0x6b00, 0x6b01),
+ regmap_reg_range(0x6b04, 0x6b04),
+ regmap_reg_range(0x6c00, 0x6c05),
+ regmap_reg_range(0x6c08, 0x6c1b),
+
+ /* port 7 */
+ regmap_reg_range(0x7000, 0x7001),
+ regmap_reg_range(0x7013, 0x7013),
+ regmap_reg_range(0x7017, 0x7017),
+ regmap_reg_range(0x701b, 0x701b),
+ regmap_reg_range(0x701f, 0x7020),
+ regmap_reg_range(0x7030, 0x7030),
+ regmap_reg_range(0x7200, 0x7203),
+ regmap_reg_range(0x7206, 0x7207),
+ regmap_reg_range(0x7300, 0x7301),
+ regmap_reg_range(0x7400, 0x7401),
+ regmap_reg_range(0x7403, 0x7403),
+ regmap_reg_range(0x7410, 0x7417),
+ regmap_reg_range(0x7420, 0x7423),
+ regmap_reg_range(0x7500, 0x7507),
+ regmap_reg_range(0x7600, 0x7613),
+ regmap_reg_range(0x7800, 0x780f),
+ regmap_reg_range(0x7820, 0x7827),
+ regmap_reg_range(0x7830, 0x7837),
+ regmap_reg_range(0x7840, 0x784b),
+ regmap_reg_range(0x7900, 0x7907),
+ regmap_reg_range(0x7914, 0x791b),
+ regmap_reg_range(0x7920, 0x7920),
+ regmap_reg_range(0x7923, 0x7927),
+ regmap_reg_range(0x7a00, 0x7a03),
+ regmap_reg_range(0x7a04, 0x7a07),
+ regmap_reg_range(0x7b00, 0x7b01),
+ regmap_reg_range(0x7b04, 0x7b04),
+ regmap_reg_range(0x7c00, 0x7c05),
+ regmap_reg_range(0x7c08, 0x7c1b),
+};
+
+static const struct regmap_access_table ksz9477_register_set = {
+ .yes_ranges = ksz9477_valid_regs,
+ .n_yes_ranges = ARRAY_SIZE(ksz9477_valid_regs),
+};
+
const struct ksz_chip_data ksz_switch_chips[] = {
[KSZ8563] = {
.chip_id = KSZ8563_CHIP_ID,
@@ -691,6 +961,8 @@ const struct ksz_chip_data ksz_switch_chips[] = {
.internal_phy = {true, true, true, true,
true, false, false},
.gbit_capable = {true, true, true, true, true, true, true},
+ .wr_table = &ksz9477_register_set,
+ .rd_table = &ksz9477_register_set,
},

[KSZ9897] = {
--
2.30.2

2022-08-23 09:00:18

by Oleksij Rempel

[permalink] [raw]
Subject: [PATCH net-next v3 08/17] net: dsa: microchip: KSZ9893: do not write to not supported Output Clock Control Register

This issue was detected after adding regmap register access validation.
KSZ9893 compatible chips do not have "Output Clock Control Register 0x0103".
So, avoid writing to it.

Signed-off-by: Oleksij Rempel <[email protected]>
---
drivers/net/dsa/microchip/ksz9477.c | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
index 313a17a989839..54514c9e6e003 100644
--- a/drivers/net/dsa/microchip/ksz9477.c
+++ b/drivers/net/dsa/microchip/ksz9477.c
@@ -193,6 +193,11 @@ int ksz9477_reset_switch(struct ksz_device *dev)
ksz_write32(dev, REG_SW_PORT_INT_MASK__4, 0x7F);
ksz_read32(dev, REG_SW_PORT_INT_STATUS__4, &data32);

+ /* KSZ9893 compatible chips do not support refclk configuration */
+ if (dev->chip_id == KSZ9893_CHIP_ID ||
+ dev->chip_id == KSZ8563_CHIP_ID)
+ return 0;
+
data8 = SW_ENABLE_REFCLKO;
if (dev->synclko_disable)
data8 = 0;
--
2.30.2

2022-08-23 09:02:00

by Oleksij Rempel

[permalink] [raw]
Subject: [PATCH net-next v3 15/17] net: dsa: microchip: ksz9477: remove unused "on" variable

This variable is not used on ksz9477 side. Remove it.

Signed-off-by: Oleksij Rempel <[email protected]>
---
drivers/net/dsa/microchip/ksz9477.c | 2 --
1 file changed, 2 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
index bfefb60ec91bf..609bd63f4cdb1 100644
--- a/drivers/net/dsa/microchip/ksz9477.c
+++ b/drivers/net/dsa/microchip/ksz9477.c
@@ -1070,7 +1070,6 @@ void ksz9477_config_cpu_port(struct dsa_switch *ds)

/* enable cpu port */
ksz9477_port_setup(dev, i, true);
- p->on = 1;
}
}

@@ -1080,7 +1079,6 @@ void ksz9477_config_cpu_port(struct dsa_switch *ds)
p = &dev->ports[i];

ksz_port_stp_state_set(ds, i, BR_STATE_DISABLED);
- p->on = 1;
if (dev->chip_id == 0x00947700 && i == 6) {
p->sgmii = 1;
}
--
2.30.2

2022-08-23 09:06:10

by Oleksij Rempel

[permalink] [raw]
Subject: [PATCH net-next v3 03/17] net: dsa: microchip: don't announce extended register support on non Gbit chips

This issue was detected after adding support of regmap_ranges for KSZ8563R
chip. This chip is reporting extended registers support without having
actual extended registers. This made PHYlib request not existing
registers.

Signed-off-by: Oleksij Rempel <[email protected]>
---
drivers/net/dsa/microchip/ksz9477.c | 12 ++++++++++++
1 file changed, 12 insertions(+)

diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
index 0f7f44358d7b3..acd4be196b69c 100644
--- a/drivers/net/dsa/microchip/ksz9477.c
+++ b/drivers/net/dsa/microchip/ksz9477.c
@@ -264,6 +264,17 @@ void ksz9477_port_init_cnt(struct ksz_device *dev, int port)
mutex_unlock(&mib->cnt_mutex);
}

+
+static void ksz9477_r_phy_quirks(struct ksz_device *dev, u16 addr, u16 reg,
+ u16 *data)
+{
+ /* KSZ8563R do not have extended registers but BMSR_ESTATEN and
+ * BMSR_ERCAP bits are set.
+ */
+ if (dev->chip_id == KSZ8563_CHIP_ID && reg == MII_BMSR)
+ *data &= ~(BMSR_ESTATEN | BMSR_ERCAP);
+}
+
void ksz9477_r_phy(struct ksz_device *dev, u16 addr, u16 reg, u16 *data)
{
u16 val = 0xffff;
@@ -308,6 +319,7 @@ void ksz9477_r_phy(struct ksz_device *dev, u16 addr, u16 reg, u16 *data)
}
} else {
ksz_pread16(dev, addr, 0x100 + (reg << 1), &val);
+ ksz9477_r_phy_quirks(dev, addr, reg, &val);
}

*data = val;
--
2.30.2

2022-08-23 09:06:20

by Oleksij Rempel

[permalink] [raw]
Subject: [PATCH net-next v3 16/17] net: dsa: microchip: remove unused sgmii variable

This variable is not used. So, remove it.

Signed-off-by: Oleksij Rempel <[email protected]>
---
drivers/net/dsa/microchip/ksz9477.c | 5 -----
drivers/net/dsa/microchip/ksz_common.h | 1 -
2 files changed, 6 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
index 609bd63f4cdb1..1dae75af8e1b9 100644
--- a/drivers/net/dsa/microchip/ksz9477.c
+++ b/drivers/net/dsa/microchip/ksz9477.c
@@ -1076,12 +1076,7 @@ void ksz9477_config_cpu_port(struct dsa_switch *ds)
for (i = 0; i < dev->info->port_cnt; i++) {
if (i == dev->cpu_port)
continue;
- p = &dev->ports[i];
-
ksz_port_stp_state_set(ds, i, BR_STATE_DISABLED);
- if (dev->chip_id == 0x00947700 && i == 6) {
- p->sgmii = 1;
- }
}
}

diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index 5b4970072c380..1b327bca1fb88 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -73,7 +73,6 @@ struct ksz_port {

u32 on:1; /* port is not disabled by hardware */
u32 fiber:1; /* port is fiber */
- u32 sgmii:1; /* port is SGMII */
u32 force:1;
u32 read:1; /* read MIB counters in background */
u32 freeze:1; /* MIB counter freeze is enabled */
--
2.30.2

2022-08-23 09:43:02

by Oleksij Rempel

[permalink] [raw]
Subject: [PATCH net-next v3 17/17] net: dsa: microchip: remove IS_9893 flag

Use chip_id as other places of this code do it

Signed-off-by: Oleksij Rempel <[email protected]>
---
drivers/net/dsa/microchip/ksz9477.c | 3 ---
drivers/net/dsa/microchip/ksz_common.c | 3 ++-
drivers/net/dsa/microchip/ksz_common.h | 4 ----
3 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
index 1dae75af8e1b9..6f857922eb3e2 100644
--- a/drivers/net/dsa/microchip/ksz9477.c
+++ b/drivers/net/dsa/microchip/ksz9477.c
@@ -1165,9 +1165,6 @@ int ksz9477_switch_init(struct ksz_device *dev)
if (ret)
return ret;

- if (dev->chip_id == KSZ9893_CHIP_ID)
- dev->features |= IS_9893;
-
return 0;
}

diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index a863d4feb4135..08491a67dc359 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -1865,7 +1865,8 @@ static void ksz_set_xmii(struct ksz_device *dev, int port,
case PHY_INTERFACE_MODE_RGMII_RXID:
data8 |= bitval[P_RGMII_SEL];
/* On KSZ9893, disable RGMII in-band status support */
- if (dev->features & IS_9893)
+ if (dev->chip_id == KSZ9893_CHIP_ID ||
+ dev->chip_id == KSZ8563_CHIP_ID)
data8 &= ~P_MII_MAC_MODE;
break;
default:
diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index 1b327bca1fb88..54c1cba35a9d8 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -118,7 +118,6 @@ struct ksz_device {
unsigned long mib_read_interval;
u16 mirror_rx;
u16 mirror_tx;
- u32 features; /* chip specific features */
u16 port_mask;
};

@@ -541,9 +540,6 @@ static inline int is_lan937x(struct ksz_device *dev)

#define SW_START 0x01

-/* Used with variable features to indicate capabilities. */
-#define IS_9893 BIT(2)
-
/* xMII configuration */
#define P_MII_DUPLEX_M BIT(6)
#define P_MII_100MBIT_M BIT(4)
--
2.30.2

2022-08-23 09:45:47

by Oleksij Rempel

[permalink] [raw]
Subject: [PATCH net-next v3 11/17] net: dsa: microchip: ksz9477: remove MII_CTRL1000 check from ksz9477_w_phy()

The reason why PHYlib may access MII_CTRL1000 on the chip without GBit
support is only if chip provides wrong information about extended caps
register. This issue is now handled by ksz9477_r_phy_quirks()

With proper regmap_ranges provided for all chips we will be able to catch this
kind of bugs any way. So, remove this sanity check.

Signed-off-by: Oleksij Rempel <[email protected]>
---
drivers/net/dsa/microchip/ksz9477.c | 4 ----
1 file changed, 4 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
index 54514c9e6e003..cb5bd0ceb8df4 100644
--- a/drivers/net/dsa/microchip/ksz9477.c
+++ b/drivers/net/dsa/microchip/ksz9477.c
@@ -342,10 +342,6 @@ int ksz9477_w_phy(struct ksz_device *dev, u16 addr, u16 reg, u16 val)
if (addr >= dev->phy_port_cnt)
return 0;

- /* No gigabit support. Do not write to this register. */
- if (!dev->info->gbit_capable[addr] && reg == MII_CTRL1000)
- return -ENXIO;
-
return ksz_pwrite16(dev, addr, 0x100 + (reg << 1), val);
}

--
2.30.2

2022-08-23 09:47:33

by Oleksij Rempel

[permalink] [raw]
Subject: [PATCH net-next v3 01/17] net: dsa: microchip: add separate struct ksz_chip_data for KSZ8563 chip

Add separate entry for the KSZ8563 chip. According to the documentation
it can support Gbit only on RGMII port. So, we will need to be able to
describe in the followup patch.

Signed-off-by: Oleksij Rempel <[email protected]>
---
drivers/net/dsa/microchip/ksz_common.c | 39 ++++++++++++++++++++++++--
drivers/net/dsa/microchip/ksz_common.h | 6 ++++
drivers/net/dsa/microchip/ksz_spi.c | 2 +-
3 files changed, 44 insertions(+), 3 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index ed7d137cba994..170c05cc0687a 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -413,6 +413,29 @@ static const u8 lan937x_shifts[] = {
};

const struct ksz_chip_data ksz_switch_chips[] = {
+ [KSZ8563] = {
+ .chip_id = KSZ8563_CHIP_ID,
+ .dev_name = "KSZ8563",
+ .num_vlans = 4096,
+ .num_alus = 4096,
+ .num_statics = 16,
+ .cpu_ports = 0x07, /* can be configured as cpu port */
+ .port_cnt = 3, /* total port count */
+ .ops = &ksz9477_dev_ops,
+ .mib_names = ksz9477_mib_names,
+ .mib_cnt = ARRAY_SIZE(ksz9477_mib_names),
+ .reg_mib_cnt = MIB_COUNTER_NUM,
+ .regs = ksz9477_regs,
+ .masks = ksz9477_masks,
+ .shifts = ksz9477_shifts,
+ .xmii_ctrl0 = ksz9477_xmii_ctrl0,
+ .xmii_ctrl1 = ksz8795_xmii_ctrl1, /* Same as ksz8795 */
+ .supports_mii = {false, false, true},
+ .supports_rmii = {false, false, true},
+ .supports_rgmii = {false, false, true},
+ .internal_phy = {true, true, false},
+ },
+
[KSZ8795] = {
.chip_id = KSZ8795_CHIP_ID,
.dev_name = "KSZ8795",
@@ -1319,6 +1342,7 @@ static enum dsa_tag_protocol ksz_get_tag_protocol(struct dsa_switch *ds,
proto = DSA_TAG_PROTO_KSZ8795;

if (dev->chip_id == KSZ8830_CHIP_ID ||
+ dev->chip_id == KSZ8563_CHIP_ID ||
dev->chip_id == KSZ9893_CHIP_ID)
proto = DSA_TAG_PROTO_KSZ9893;

@@ -1638,7 +1662,7 @@ static void ksz_phylink_mac_link_up(struct dsa_switch *ds, int port,

static int ksz_switch_detect(struct ksz_device *dev)
{
- u8 id1, id2;
+ u8 id1, id2, id4;
u16 id16;
u32 id32;
int ret;
@@ -1684,7 +1708,6 @@ static int ksz_switch_detect(struct ksz_device *dev)
switch (id32) {
case KSZ9477_CHIP_ID:
case KSZ9897_CHIP_ID:
- case KSZ9893_CHIP_ID:
case KSZ9567_CHIP_ID:
case LAN9370_CHIP_ID:
case LAN9371_CHIP_ID:
@@ -1692,6 +1715,18 @@ static int ksz_switch_detect(struct ksz_device *dev)
case LAN9373_CHIP_ID:
case LAN9374_CHIP_ID:
dev->chip_id = id32;
+ break;
+ case KSZ9893_CHIP_ID:
+ ret = ksz_read8(dev, REG_CHIP_ID4,
+ &id4);
+ if (ret)
+ return ret;
+
+ if (id4 == SKU_ID_KSZ8563)
+ dev->chip_id = KSZ8563_CHIP_ID;
+ else
+ dev->chip_id = KSZ9893_CHIP_ID;
+
break;
default:
dev_err(dev->dev,
diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index 764ada3a0f42a..2b3877d4ef46f 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -123,6 +123,7 @@ struct ksz_device {

/* List of supported models */
enum ksz_model {
+ KSZ8563,
KSZ8795,
KSZ8794,
KSZ8765,
@@ -139,6 +140,7 @@ enum ksz_model {
};

enum ksz_chip_id {
+ KSZ8563_CHIP_ID = 0x8563,
KSZ8795_CHIP_ID = 0x8795,
KSZ8794_CHIP_ID = 0x8794,
KSZ8765_CHIP_ID = 0x8765,
@@ -482,6 +484,10 @@ static inline int is_lan937x(struct ksz_device *dev)

#define SW_REV_ID_M GENMASK(7, 4)

+/* KSZ9893, KSZ9563, KSZ8563 specific register */
+#define REG_CHIP_ID4 0x0f
+#define SKU_ID_KSZ8563 0x3c
+
/* Driver set switch broadcast storm protection at 10% rate. */
#define BROADCAST_STORM_PROT_RATE 10

diff --git a/drivers/net/dsa/microchip/ksz_spi.c b/drivers/net/dsa/microchip/ksz_spi.c
index 05bd089795f83..746b725b09ec4 100644
--- a/drivers/net/dsa/microchip/ksz_spi.c
+++ b/drivers/net/dsa/microchip/ksz_spi.c
@@ -160,7 +160,7 @@ static const struct of_device_id ksz_dt_ids[] = {
},
{
.compatible = "microchip,ksz8563",
- .data = &ksz_switch_chips[KSZ9893]
+ .data = &ksz_switch_chips[KSZ8563]
},
{
.compatible = "microchip,ksz9567",
--
2.30.2

2022-08-25 09:12:44

by Paolo Abeni

[permalink] [raw]
Subject: Re: [PATCH net-next v3 00/17] net: dsa: microchip: add error handling and register access validation

On Tue, 2022-08-23 at 10:02 +0200, Oleksij Rempel wrote:
> changes v3:
> - fix build error in the middle of the patch stack.

The series looks reasonable to me, let's see the comments from the DSA
crew;)

The next time please additionally add the change informations in the
individual patch, after the '---' separator: that will made reviewers
life more easier.

It's hard to track what changed exactly since v2 from the above line.

Thanks!

Paolo


2022-08-25 21:04:29

by Vladimir Oltean

[permalink] [raw]
Subject: Re: [PATCH net-next v3 15/17] net: dsa: microchip: ksz9477: remove unused "on" variable

On Tue, Aug 23, 2022 at 10:02:29AM +0200, Oleksij Rempel wrote:
> This variable is not used on ksz9477 side. Remove it.
>
> Signed-off-by: Oleksij Rempel <[email protected]>
> ---
> drivers/net/dsa/microchip/ksz9477.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
> index bfefb60ec91bf..609bd63f4cdb1 100644
> --- a/drivers/net/dsa/microchip/ksz9477.c
> +++ b/drivers/net/dsa/microchip/ksz9477.c
> @@ -1070,7 +1070,6 @@ void ksz9477_config_cpu_port(struct dsa_switch *ds)
>
> /* enable cpu port */
> ksz9477_port_setup(dev, i, true);
> - p->on = 1;
> }
> }
>
> @@ -1080,7 +1079,6 @@ void ksz9477_config_cpu_port(struct dsa_switch *ds)
> p = &dev->ports[i];
>
> ksz_port_stp_state_set(ds, i, BR_STATE_DISABLED);
> - p->on = 1;
> if (dev->chip_id == 0x00947700 && i == 6) {
> p->sgmii = 1;
> }
> --
> 2.30.2
>

And it seems like it's not used on ksz8 either. The reason I'm saying
that is that ksz8_flush_dyn_mac_table() is the only apparent user of
p->on, and that only for the case where flushing the FDB of all ports is
requested (port > dev->info->port_cnt). But ksz8_flush_dyn_mac_table()
(through dev->dev_ops->flush_dyn_mac_table) is only called from DSA's
ds->ops->port_fast_age() method, and that will never be requested
"for all ports" (and to my knowledge never was in the past, either).
Badly ported SDK code would be my guess. So there are more
simplifications which could be done.

2022-08-25 22:13:14

by Vladimir Oltean

[permalink] [raw]
Subject: Re: [PATCH net-next v3 00/17] net: dsa: microchip: add error handling and register access validation

On Thu, Aug 25, 2022 at 11:00:44AM +0200, Paolo Abeni wrote:
> On Tue, 2022-08-23 at 10:02 +0200, Oleksij Rempel wrote:
> > changes v3:
> > - fix build error in the middle of the patch stack.
>
> The series looks reasonable to me, let's see the comments from the DSA
> crew;)

Patch set looks ok from my side, won't leave review tags for each of the
17 patches, just here

Reviewed-by: Vladimir Oltean <[email protected]>

2022-08-25 23:23:12

by Jakub Kicinski

[permalink] [raw]
Subject: Re: [PATCH net-next v3 00/17] net: dsa: microchip: add error handling and register access validation

On Fri, 26 Aug 2022 00:44:24 +0300 Vladimir Oltean wrote:
> On Thu, Aug 25, 2022 at 11:00:44AM +0200, Paolo Abeni wrote:
> > On Tue, 2022-08-23 at 10:02 +0200, Oleksij Rempel wrote:
> > > changes v3:
> > > - fix build error in the middle of the patch stack.
> >
> > The series looks reasonable to me, let's see the comments from the DSA
> > crew;)
>
> Patch set looks ok from my side, won't leave review tags for each of the
> 17 patches, just here
>
> Reviewed-by: Vladimir Oltean <[email protected]>

Thanks a lot!

Oleksij there are quite a few checkpatch annoyances here, please fix up:

-------------------------------------------------------------------------------------------
Commit 0ef2f8617176 ("net: dsa: microchip: do per-port Gbit detection instead of per-chip")
-------------------------------------------------------------------------------------------
WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#12:
ksz9477_switch_init() function. Which is using undocumented REG_GLOBAL_OPTIONS

total: 0 errors, 1 warnings, 0 checks, 144 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.

Commit 0ef2f8617176 ("net: dsa: microchip: do per-port Gbit detection instead of per-chip") has style problems, please review.
-------------------------------------------------------------------------------------------------------
Commit 06c98e1faaa7 ("net: dsa: microchip: don't announce extended register support on non Gbit chips")
-------------------------------------------------------------------------------------------------------
CHECK: Please don't use multiple blank lines
#29: FILE: drivers/net/dsa/microchip/ksz9477.c:267:

+

total: 0 errors, 0 warnings, 1 checks, 32 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.

Commit 06c98e1faaa7 ("net: dsa: microchip: don't announce extended register support on non Gbit chips") has style problems, please review.


------------------------------------------------------------------------------------------------------
Commit 872fff3049d3 ("net: dsa: microchip: forward error value on all ksz_pread/ksz_pwrite functions")
------------------------------------------------------------------------------------------------------
CHECK: Alignment should match open parenthesis
#28: FILE: drivers/net/dsa/microchip/ksz_common.h:397:
+static inline int ksz_pread8(struct ksz_device *dev, int port, int offset,
u8 *data)

CHECK: Alignment should match open parenthesis
#36: FILE: drivers/net/dsa/microchip/ksz_common.h:403:
+static inline int ksz_pread16(struct ksz_device *dev, int port, int offset,
u16 *data)

CHECK: Alignment should match open parenthesis
#44: FILE: drivers/net/dsa/microchip/ksz_common.h:409:
+static inline int ksz_pread32(struct ksz_device *dev, int port, int offset,
u32 *data)

CHECK: Alignment should match open parenthesis
#52: FILE: drivers/net/dsa/microchip/ksz_common.h:415:
+static inline int ksz_pwrite8(struct ksz_device *dev, int port, int offset,
u8 data)

CHECK: Alignment should match open parenthesis
#60: FILE: drivers/net/dsa/microchip/ksz_common.h:421:
+static inline int ksz_pwrite16(struct ksz_device *dev, int port, int offset,
u16 data)

CHECK: Alignment should match open parenthesis
#69: FILE: drivers/net/dsa/microchip/ksz_common.h:428:
+static inline int ksz_pwrite32(struct ksz_device *dev, int port, int offset,
u32 data)

total: 0 errors, 0 warnings, 6 checks, 58 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.

Commit 872fff3049d3 ("net: dsa: microchip: forward error value on all ksz_pread/ksz_pwrite functions") has style problems, please review.

-----------------------------------------------------------------------------------------------------------------
Commit a615f4ad0116 ("net: dsa: microchip: KSZ9893: do not write to not supported Output Clock Control Register")
-----------------------------------------------------------------------------------------------------------------
WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#8:
KSZ9893 compatible chips do not have "Output Clock Control Register 0x0103".

total: 0 errors, 1 warnings, 0 checks, 15 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.

Commit a615f4ad0116 ("net: dsa: microchip: KSZ9893: do not write to not supported Output Clock Control Register") has style problems, please review.
---------------------------------------------------------------------------------
Commit d1abab8b5762 ("net: dsa: microchip: add support for regmap_access_tables")
---------------------------------------------------------------------------------
WARNING: unnecessary whitespace before a quoted newline
#43: FILE: drivers/net/dsa/microchip/ksz_common.h:338:
+ dev_err(dev->dev, "can't read 8bit reg: 0x%x %pe \n", reg,

WARNING: unnecessary whitespace before a quoted newline
#56: FILE: drivers/net/dsa/microchip/ksz_common.h:351:
+ dev_err(dev->dev, "can't read 16bit reg: 0x%x %pe \n", reg,

WARNING: unnecessary whitespace before a quoted newline
#69: FILE: drivers/net/dsa/microchip/ksz_common.h:364:
+ dev_err(dev->dev, "can't read 32bit reg: 0x%x %pe \n", reg,

WARNING: unnecessary whitespace before a quoted newline
#84: FILE: drivers/net/dsa/microchip/ksz_common.h:378:
+ dev_err(dev->dev, "can't read 64bit reg: 0x%x %pe \n", reg,

WARNING: unnecessary whitespace before a quoted newline
#99: FILE: drivers/net/dsa/microchip/ksz_common.h:392:
+ dev_err(dev->dev, "can't write 8bit reg: 0x%x %pe \n", reg,

WARNING: unnecessary whitespace before a quoted newline
#112: FILE: drivers/net/dsa/microchip/ksz_common.h:404:
+ dev_err(dev->dev, "can't write 16bit reg: 0x%x %pe \n", reg,

WARNING: unnecessary whitespace before a quoted newline
#125: FILE: drivers/net/dsa/microchip/ksz_common.h:416:
+ dev_err(dev->dev, "can't write 32bit reg: 0x%x %pe \n", reg,

total: 0 errors, 7 warnings, 0 checks, 123 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.

Commit d1abab8b5762 ("net: dsa: microchip: add support for regmap_access_tables") has style problems, please review.

----------------------------------------------------------------------------------------------------
Commit 39a148ccc56c ("net: dsa: microchip: ksz9477: remove MII_CTRL1000 check from ksz9477_w_phy()")
----------------------------------------------------------------------------------------------------
WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#11:
With proper regmap_ranges provided for all chips we will be able to catch this

total: 0 errors, 1 warnings, 0 checks, 14 lines checked

2022-08-26 08:37:06

by Oleksij Rempel

[permalink] [raw]
Subject: Re: [PATCH net-next v3 15/17] net: dsa: microchip: ksz9477: remove unused "on" variable

On Thu, Aug 25, 2022 at 11:54:07PM +0300, Vladimir Oltean wrote:
> On Tue, Aug 23, 2022 at 10:02:29AM +0200, Oleksij Rempel wrote:
> > This variable is not used on ksz9477 side. Remove it.
> >
> > Signed-off-by: Oleksij Rempel <[email protected]>
> > ---
> > drivers/net/dsa/microchip/ksz9477.c | 2 --
> > 1 file changed, 2 deletions(-)
> >
> > diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
> > index bfefb60ec91bf..609bd63f4cdb1 100644
> > --- a/drivers/net/dsa/microchip/ksz9477.c
> > +++ b/drivers/net/dsa/microchip/ksz9477.c
> > @@ -1070,7 +1070,6 @@ void ksz9477_config_cpu_port(struct dsa_switch *ds)
> >
> > /* enable cpu port */
> > ksz9477_port_setup(dev, i, true);
> > - p->on = 1;
> > }
> > }
> >
> > @@ -1080,7 +1079,6 @@ void ksz9477_config_cpu_port(struct dsa_switch *ds)
> > p = &dev->ports[i];
> >
> > ksz_port_stp_state_set(ds, i, BR_STATE_DISABLED);
> > - p->on = 1;
> > if (dev->chip_id == 0x00947700 && i == 6) {
> > p->sgmii = 1;
> > }
> > --
> > 2.30.2
> >
>
> And it seems like it's not used on ksz8 either. The reason I'm saying
> that is that ksz8_flush_dyn_mac_table() is the only apparent user of
> p->on, and that only for the case where flushing the FDB of all ports is
> requested (port > dev->info->port_cnt). But ksz8_flush_dyn_mac_table()
> (through dev->dev_ops->flush_dyn_mac_table) is only called from DSA's
> ds->ops->port_fast_age() method, and that will never be requested
> "for all ports" (and to my knowledge never was in the past, either).
> Badly ported SDK code would be my guess. So there are more
> simplifications which could be done.

Ok, i'll take a look on it as soon as i get one of ksz8 board in my
fingers.

--
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 |