| Message ID | 20201207125627.30843-1-m.grzeschik@pengutronix.de |
|---|---|
| Headers | show |
| Series | microchip: add support for ksz88x3 driver family | expand |
Gentle Ping. Did you find time to look into my other patches of the series. I really would like to send the next version. Thanks! On Mon, Dec 07, 2020 at 10:44:15PM +0100, Michael Grzeschik wrote: >On Mon, Dec 07, 2020 at 08:02:57PM +0000, Tristram.Ha@microchip.com wrote: >>>In order to get this driver used with other switches the functions need >>>to use different offsets and register shifts. This patch changes the >>>direct use of the register defines to register description structures, >>>which can be set depending on the chips register layout. >>> >>>Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de> >>> >>>--- >>>v1 -> v4: - extracted this change from bigger previous patch >>>v4 -> v5: - added missing variables in ksz8_r_vlan_entries >>> - moved shifts, masks and registers to arrays indexed by enums >>> - using unsigned types where possible >>>--- >>> drivers/net/dsa/microchip/ksz8.h | 69 +++++++ >>> drivers/net/dsa/microchip/ksz8795.c | 261 +++++++++++++++++------- >>> drivers/net/dsa/microchip/ksz8795_reg.h | 85 -------- >>> 3 files changed, 253 insertions(+), 162 deletions(-) >>> create mode 100644 drivers/net/dsa/microchip/ksz8.h >> >>Sorry for not respond to these patches sooner. >> >>There are 3 older KSZ switch families: KSZ8863/73, KSZ8895/64, and KSZ8795/94. >>The newer KSZ8795 is not considered an upgrade for KSZ8895, so some of >>these switch registers are moved around and some features are dropped. >> >>It is best to have one driver to support all 3 switches, but some operations are >>Incompatible so it may be better to keep the drivers separate for now. >> >>For basic operations those issues may not occur so it seems simple to have >>one driver handling all 3 switches. I will come up with a list of those >>incompatibilities. > >Look into the next patch. I handled many special cases for the ksz8863 >in the "net: dsa: microchip: ksz8795: add support for ksz88xx chips". >These cases, including the VLAN, Tagging ... are handled by checking if >the feautre IS_88X3 is set. This can be extended to other types as well. > >My first version of the patches was an RFC series that was mentioning >that it is based on your RFC series for the ksz8895. > >8863 RFC: https://patchwork.ozlabs.org/project/netdev/cover/20190508211330.19328-1-m.grzeschik@pengutronix.de/ > >8895 RFC: https://patchwork.ozlabs.org/patch/822712/ > >I remember, that I was reading the datasheets of all three chips, >8895, 8863 and 8795. After the 8795 series was mainline, the >obvious next step was to get the 8863 into the 8795 code. The result >is this series. > >So the obvious question is, how far does your 8895 series differ >from the 8863 switches? > >>The tail tag format of KSZ8863 is different from KSZ8895 and KSZ8795, but >>because of the DSA driver implementation that issue never comes up. > >Right. In the first four series I kept an extra tail tag patch. But >after cleaning up I figured that the Implementation matched the >one for the KSZ9893. Therefor I reused the tag code. > >>>-static void ksz8_from_vlan(u16 vlan, u8 *fid, u8 *member, u8 *valid) >>>+static void ksz8_from_vlan(struct ksz_device *dev, u32 vlan, u8 *fid, >>>+ u8 *member, u8 *valid) >>> { >>>- *fid = vlan & VLAN_TABLE_FID; >>>- *member = (vlan & VLAN_TABLE_MEMBERSHIP) >> >>>VLAN_TABLE_MEMBERSHIP_S; >>>- *valid = !!(vlan & VLAN_TABLE_VALID); >>>+ struct ksz8 *ksz8 = dev->priv; >>>+ const u32 *masks = ksz8->masks; >>>+ const u8 *shifts = ksz8->shifts; >>>+ >>>+ *fid = vlan & masks[VLAN_TABLE_FID]; >>>+ *member = (vlan & masks[VLAN_TABLE_MEMBERSHIP]) >> >>>+ shifts[VLAN_TABLE_MEMBERSHIP_S]; >>>+ *valid = !!(vlan & masks[VLAN_TABLE_VALID]); >>> } >>> >>>-static void ksz8_to_vlan(u8 fid, u8 member, u8 valid, u16 *vlan) >>>+static void ksz8_to_vlan(struct ksz_device *dev, u8 fid, u8 member, u8 >>>valid, >>>+ u32 *vlan) >>> { >>>+ struct ksz8 *ksz8 = dev->priv; >>>+ const u32 *masks = ksz8->masks; >>>+ const u8 *shifts = ksz8->shifts; >>>+ >>> *vlan = fid; >>>- *vlan |= (u16)member << VLAN_TABLE_MEMBERSHIP_S; >>>+ *vlan |= (u16)member << shifts[VLAN_TABLE_MEMBERSHIP_S]; >>> if (valid) >>>- *vlan |= VLAN_TABLE_VALID; >>>+ *vlan |= masks[VLAN_TABLE_VALID]; >>> } >>> >>> static void ksz8_r_vlan_entries(struct ksz_device *dev, u16 addr) >>> { >>>+ struct ksz8 *ksz8 = dev->priv; >>>+ const u8 *shifts = ksz8->shifts; >>> u64 data; >>> int i; >>> >>>@@ -418,7 +509,7 @@ static void ksz8_r_vlan_entries(struct ksz_device >>>*dev, u16 addr) >>> addr *= dev->phy_port_cnt; >>> for (i = 0; i < dev->phy_port_cnt; i++) { >>> dev->vlan_cache[addr + i].table[0] = (u16)data; >>>- data >>= VLAN_TABLE_S; >>>+ data >>= shifts[VLAN_TABLE]; >>> } >>> } >>> >>>@@ -454,6 +545,8 @@ static void ksz8_w_vlan_table(struct ksz_device *dev, >>>u16 vid, u32 vlan) >>> >> >>The VLAN table operation in KSZ8863 is completely different from KSZ8795. >> >>>-/** >>>- * VLAN_TABLE_FID 00-007F007F-007F007F >>>- * VLAN_TABLE_MEMBERSHIP 00-0F800F80-0F800F80 >>>- * VLAN_TABLE_VALID 00-10001000-10001000 >>>- */ >>>- >>>-#define VLAN_TABLE_FID 0x007F >>>-#define VLAN_TABLE_MEMBERSHIP 0x0F80 >>>-#define VLAN_TABLE_VALID 0x1000 >>>- >>>-#define VLAN_TABLE_MEMBERSHIP_S 7 >>>-#define VLAN_TABLE_S 16 >>>- >> >>In KSZ8795 you can use all 4096 VLAN id. Each entry in the table contains >>4 VID. The FID is actually used for lookup and there is a limit, so you need >>to convert VID to FID when programming the VLAN table. >> >>The only effect of using FID is the same MAC address can be used in >>different VLANs. >> >>In KSZ8863 there are only 16 entries in the VLAN table. Just like static >>MAC table each entry is programmed using an index. The entry contains >>the VID, which does not have any relationship with the index unlike in >>KSZ8795. >> >>The number of FID is also limited to 16. So the maximum VLAN used is 16. >> >>> /** >>> * MIB_COUNTER_VALUE 00-00000000-3FFFFFFF >>> * MIB_TOTAL_BYTES 00-0000000F-FFFFFFFF >>>@@ -956,9 +874,6 @@ >>> * MIB_COUNTER_OVERFLOW 00-00000040-00000000 >>> */ >>> >>>-#define MIB_COUNTER_OVERFLOW BIT(6) >>>-#define MIB_COUNTER_VALID BIT(5) >>>- >>> #define MIB_COUNTER_VALUE 0x3FFFFFFF >> >>The MIB counters are also a little different in KSZ8863 and KSZ8795. >>KSZ8863 may have smaller total bytes. In normal operation this issue may >>not come up. > >As mentioned before, these cases are handled differently for both >types of drivers. > >-- >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 | -- 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 |