| Message ID | 1687833188-3184-1-git-send-email-Tristram.Ha@microchip.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] net: dsa: microchip: correct KSZ8795 static MAC table access | expand |
The 06/26/2023 19:33, Tristram.Ha@microchip.com wrote: > From: Tristram Ha <Tristram.Ha@microchip.com> Hi Tristram, Please make sure you CC all the maintainers. You can find out the maintainers by running ./scripts/get_maintainer.pl on your patch file. > > 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 with 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 is only executed when > KSZ8795 is detected. > > Fixes: c8e04374f9e1 ("net: dsa: microchip: Make ksz8_w_sta_mac_table() static") Don't add a new line between Fixes and SoB tags. Also, are you sure that the blamed commit introduced the issue? Because that commit just makes ksz8_w_sta_mac_table to be static. > > Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com> > --- > 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 f56fca1b1a22..cc5b19a3d0df 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 a4428be5f483..a0ba2605bb62 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 8abecaf6089e..33d9a2f6af27 100644 > --- a/drivers/net/dsa/microchip/ksz_common.h > +++ b/drivers/net/dsa/microchip/ksz_common.h > @@ -569,6 +569,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 >
diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c index f56fca1b1a22..cc5b19a3d0df 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 a4428be5f483..a0ba2605bb62 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 8abecaf6089e..33d9a2f6af27 100644 --- a/drivers/net/dsa/microchip/ksz_common.h +++ b/drivers/net/dsa/microchip/ksz_common.h @@ -569,6 +569,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;