* [PATCH v1 net] net: dsa: microchip: correct KSZ8795 static MAC table access
@ 2023-07-11 0:10 Tristram.Ha
2023-07-11 6:30 ` Horatiu Vultur
0 siblings, 1 reply; 3+ messages in thread
From: Tristram.Ha @ 2023-07-11 0:10 UTC (permalink / raw)
To: Woojung Huh, Andrew Lunn, Florian Fainelli, Vladimir Oltean,
David S. Miller
Cc: netdev, UNGLinuxDriver, Tristram Ha
From: Tristram Ha <Tristram.Ha@microchip.com>
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 <Tristram.Ha@microchip.com>
---
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
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v1 net] net: dsa: microchip: correct KSZ8795 static MAC table access
2023-07-11 0:10 [PATCH v1 net] net: dsa: microchip: correct KSZ8795 static MAC table access Tristram.Ha
@ 2023-07-11 6:30 ` Horatiu Vultur
2023-07-12 22:57 ` Jakub Kicinski
0 siblings, 1 reply; 3+ messages in thread
From: Horatiu Vultur @ 2023-07-11 6:30 UTC (permalink / raw)
To: Tristram.Ha
Cc: Woojung Huh, Andrew Lunn, Florian Fainelli, Vladimir Oltean,
David S. Miller, netdev, UNGLinuxDriver
The 07/10/2023 17:10, Tristram.Ha@microchip.com wrote:
> From: Tristram Ha <Tristram.Ha@microchip.com>
Hi Tristram,
It looks like you forgot again to add all the maintainers to the email
thread. Was it something wrong with the command
./scripts/get_maintainer.pl?
>
> 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 <Tristram.Ha@microchip.com>
Other than what I mentioned aboved, it looks OK.
Reviewed-by: Horatiu Vultur <horatiu.vultur@microchip.com>
> ---
> 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
>
--
/Horatiu
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v1 net] net: dsa: microchip: correct KSZ8795 static MAC table access
2023-07-11 6:30 ` Horatiu Vultur
@ 2023-07-12 22:57 ` Jakub Kicinski
0 siblings, 0 replies; 3+ messages in thread
From: Jakub Kicinski @ 2023-07-12 22:57 UTC (permalink / raw)
To: Tristram.Ha
Cc: Horatiu Vultur, Woojung Huh, Andrew Lunn, Florian Fainelli,
Vladimir Oltean, David S. Miller, netdev, UNGLinuxDriver
On Tue, 11 Jul 2023 08:30:20 +0200 Horatiu Vultur wrote:
> The 07/10/2023 17:10, Tristram.Ha@microchip.com wrote:
> > From: Tristram Ha <Tristram.Ha@microchip.com>
> >
> It looks like you forgot again to add all the maintainers to the email
> thread. Was it something wrong with the command
> ./scripts/get_maintainer.pl?
To be clear - you need to repost with the right CCs included.
CCing authors of the original patch (Oleksji, Michael in this case)
is an absolute must.
Please add Horatiu's review tag.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-07-12 22:57 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-07-11 0:10 [PATCH v1 net] net: dsa: microchip: correct KSZ8795 static MAC table access Tristram.Ha
2023-07-11 6:30 ` Horatiu Vultur
2023-07-12 22:57 ` Jakub Kicinski
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).