2023-07-14 01:35:38

by Tristram.Ha

[permalink] [raw]
Subject: [PATCH v2 net] net: dsa: microchip: correct KSZ8795 static MAC table access

From: Tristram Ha <[email protected]>

The KSZ8795 driver code was modified to use on KSZ8863/73, which has
different register definitions. Some of the new KSZ8795 register
information are wrong compared to previous code.

KSZ8795 also behaves differently in that the STATIC_MAC_TABLE_USE_FID
and STATIC_MAC_TABLE_FID bits are off by 1 when doing MAC table reading
than writing. To compensate that a special code was added to shift the
register value by 1 before applying those bits. This is wrong when the
code is running on KSZ8863, so this special code is only executed when
KSZ8795 is detected.

Fixes: 4b20a07e103f ("net: dsa: microchip: ksz8795: add support for ksz88xx chips")
Signed-off-by: Tristram Ha <[email protected]>
Reviewed-by: Horatiu Vultur <[email protected]>
---
v2
- Send to more maintainers. Add review from Horatiu Vultur

v1
- Use correct commit for fixes

drivers/net/dsa/microchip/ksz8795.c | 8 +++++++-
drivers/net/dsa/microchip/ksz_common.c | 8 ++++----
drivers/net/dsa/microchip/ksz_common.h | 7 +++++++
3 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
index 84d502589f8e..91aba470fb2f 100644
--- a/drivers/net/dsa/microchip/ksz8795.c
+++ b/drivers/net/dsa/microchip/ksz8795.c
@@ -506,7 +506,13 @@ static int ksz8_r_sta_mac_table(struct ksz_device *dev, u16 addr,
(data_hi & masks[STATIC_MAC_TABLE_FWD_PORTS]) >>
shifts[STATIC_MAC_FWD_PORTS];
alu->is_override = (data_hi & masks[STATIC_MAC_TABLE_OVERRIDE]) ? 1 : 0;
- data_hi >>= 1;
+
+ /* KSZ8795 family switches have STATIC_MAC_TABLE_USE_FID and
+ * STATIC_MAC_TABLE_FID definitions off by 1 when doing read on the
+ * static MAC table compared to doing write.
+ */
+ if (ksz_is_ksz87xx(dev))
+ data_hi >>= 1;
alu->is_static = true;
alu->is_use_fid = (data_hi & masks[STATIC_MAC_TABLE_USE_FID]) ? 1 : 0;
alu->fid = (data_hi & masks[STATIC_MAC_TABLE_FID]) >>
diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 813b91a816bb..b18cd170ec06 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -331,13 +331,13 @@ static const u32 ksz8795_masks[] = {
[STATIC_MAC_TABLE_VALID] = BIT(21),
[STATIC_MAC_TABLE_USE_FID] = BIT(23),
[STATIC_MAC_TABLE_FID] = GENMASK(30, 24),
- [STATIC_MAC_TABLE_OVERRIDE] = BIT(26),
- [STATIC_MAC_TABLE_FWD_PORTS] = GENMASK(24, 20),
+ [STATIC_MAC_TABLE_OVERRIDE] = BIT(22),
+ [STATIC_MAC_TABLE_FWD_PORTS] = GENMASK(20, 16),
[DYNAMIC_MAC_TABLE_ENTRIES_H] = GENMASK(6, 0),
- [DYNAMIC_MAC_TABLE_MAC_EMPTY] = BIT(8),
+ [DYNAMIC_MAC_TABLE_MAC_EMPTY] = BIT(7),
[DYNAMIC_MAC_TABLE_NOT_READY] = BIT(7),
[DYNAMIC_MAC_TABLE_ENTRIES] = GENMASK(31, 29),
- [DYNAMIC_MAC_TABLE_FID] = GENMASK(26, 20),
+ [DYNAMIC_MAC_TABLE_FID] = GENMASK(22, 16),
[DYNAMIC_MAC_TABLE_SRC_PORT] = GENMASK(26, 24),
[DYNAMIC_MAC_TABLE_TIMESTAMP] = GENMASK(28, 27),
[P_MII_TX_FLOW_CTRL] = BIT(5),
diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index 28444e5924f9..a4de58847dea 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -601,6 +601,13 @@ static inline void ksz_regmap_unlock(void *__mtx)
mutex_unlock(mtx);
}

+static inline bool ksz_is_ksz87xx(struct ksz_device *dev)
+{
+ return dev->chip_id == KSZ8795_CHIP_ID ||
+ dev->chip_id == KSZ8794_CHIP_ID ||
+ dev->chip_id == KSZ8765_CHIP_ID;
+}
+
static inline bool ksz_is_ksz88x3(struct ksz_device *dev)
{
return dev->chip_id == KSZ8830_CHIP_ID;
--
2.17.1



2023-07-15 11:11:15

by Simon Horman

[permalink] [raw]
Subject: Re: [PATCH v2 net] net: dsa: microchip: correct KSZ8795 static MAC table access

On Thu, Jul 13, 2023 at 05:46:22PM -0700, [email protected] wrote:
> From: Tristram Ha <[email protected]>
>
> The KSZ8795 driver code was modified to use on KSZ8863/73, which has
> different register definitions. Some of the new KSZ8795 register
> information are wrong compared to previous code.
>
> KSZ8795 also behaves differently in that the STATIC_MAC_TABLE_USE_FID
> and STATIC_MAC_TABLE_FID bits are off by 1 when doing MAC table reading
> than writing. To compensate that a special code was added to shift the
> register value by 1 before applying those bits. This is wrong when the
> code is running on KSZ8863, so this special code is only executed when
> KSZ8795 is detected.
>
> Fixes: 4b20a07e103f ("net: dsa: microchip: ksz8795: add support for ksz88xx chips")
> Signed-off-by: Tristram Ha <[email protected]>
> Reviewed-by: Horatiu Vultur <[email protected]>

Reviewed-by: Simon Horman <[email protected]>


2023-07-17 07:19:30

by patchwork-bot+netdevbpf

[permalink] [raw]
Subject: Re: [PATCH v2 net] net: dsa: microchip: correct KSZ8795 static MAC table access

Hello:

This patch was applied to netdev/net.git (main)
by David S. Miller <[email protected]>:

On Thu, 13 Jul 2023 17:46:22 -0700 you wrote:
> From: Tristram Ha <[email protected]>
>
> The KSZ8795 driver code was modified to use on KSZ8863/73, which has
> different register definitions. Some of the new KSZ8795 register
> information are wrong compared to previous code.
>
> KSZ8795 also behaves differently in that the STATIC_MAC_TABLE_USE_FID
> and STATIC_MAC_TABLE_FID bits are off by 1 when doing MAC table reading
> than writing. To compensate that a special code was added to shift the
> register value by 1 before applying those bits. This is wrong when the
> code is running on KSZ8863, so this special code is only executed when
> KSZ8795 is detected.
>
> [...]

Here is the summary with links:
- [v2,net] net: dsa: microchip: correct KSZ8795 static MAC table access
https://git.kernel.org/netdev/net/c/4bdf79d686b4

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html