| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20201215163122.GM27410@pengutronix.de>
Date: Tue, 15 Dec 2020 17:31:22 +0100
From: Michael Grzeschik <mgr@...gutronix.de>
To: Tristram.Ha@...rochip.com
Cc: andrew@...n.ch, f.fainelli@...il.com, davem@...emloft.net,
kernel@...gutronix.de, netdev@...r.kernel.org,
matthias.schiffer@...tq-group.com, Woojung.Huh@...rochip.com,
UNGLinuxDriver@...rochip.com
Subject: Re: [PATCH v5 3/6] net: dsa: microchip: ksz8795: move register
offsets and shifts to separate struct
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@...rochip.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@...gutronix.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 |
Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)
Powered by blists - more mailing lists