Re: [PATCH v5 4/6] net: dsa: microchip: ksz8795: add support for ksz88xx chips

[Michael Grzeschik]

Hi Tristam!

On Wed, Dec 16, 2020 at 06:33:06AM +0000, tristram...@microchip.com wrote:

 static void ksz8_r_vlan_table(struct ksz_device *dev, u16 vid, u32 *vlan)
 {
-       int index;
-       u16 *data;
-       u16 addr;
+       u16 addr = vid / dev->phy_port_cnt;
        u64 buf;

-       data = (u16 *)&buf;
-       addr = vid / dev->phy_port_cnt;
-       index = vid & 3;
        ksz8_r_table(dev, TABLE_VLAN, addr, &buf);
-       *vlan = data[index];
+       if (dev->features & IS_88X3) {
+               *vlan = (u32)buf;
+       } else {
+               u16 *data = (u16 *)&buf;
+
+               *vlan = data[vid & 3];
+       }
 }

 static void ksz8_w_vlan_table(struct ksz_device *dev, u16 vid, u32 vlan)
 {
-       int index;
-       u16 *data;
-       u16 addr;
+       u16 addr = vid / dev->phy_port_cnt;
        u64 buf;

-       data = (u16 *)&buf;
-       addr = vid / dev->phy_port_cnt;
-       index = vid & 3;
        ksz8_r_table(dev, TABLE_VLAN, addr, &buf);
-       data[index] = vlan;
+
+       if (dev->features & IS_88X3) {
+               buf = vlan;
+       } else {
+               u16 *data = (u16 *)&buf;
+
+               data[vid & 3] = vlan;
+       }
+
        dev->vlan_cache[vid].table[0] = vlan;
        ksz8_w_table(dev, TABLE_VLAN, addr, buf);
 }

I am confused about how the addr is derived.

In KSZ8795 vid is in range of 0-4095.  The addr is just (vid / 4) as there
are 4 entries in one access.  The data are lined up in 16-bit boundary
so that the VLAN information can be accessed using the array.

For KSZ8895 the VLAN data are not lined up so the 64-bit variable
needs to be shifted accordingly and masked.

For KSZ8863 the addr is a hard value from 0 to 15.  The data buffer is just
32-bit.  The vid value is contained in the entry.  You need to match that vid
with the input vid to return the right information.

You need a different VLAN read function to check if the VLAN is already
programmed in the VLAN table by searching all 16 entries.

For the VLAN write function you need to check if there is available space
to add a new entry.  The VID range is still 0-4095, but the FID range is 0-15.

Thanks for the clarification. Its possible that I missed that when
porting this from your first RFC.

Regarding the drivers for ksz88{6,7}3, did you also plan to mainline
some work? Do you have some code that could be shared? If so I would
not need to implement everything again. In case you have some work
pending, please let me know. Than we could colaborate to get this
chips finaly mainline.

Thanks,
Michael

--
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 |

signature.asc
Description: PGP signature