On 4/21/25 6:29 PM, Boon Khai Ng wrote:
> Refactor VLAN implementation by moving common code for DWMAC4 and
> DWXGMAC IPs into a separate VLAN module. VLAN implementation for
> DWMAC4 and DWXGMAC differs only for CSR base address, the descriptor
> for the VLAN ID and VLAN VALID bit field.
>
> The descriptor format for VLAN is not moved to the common code due
> to hardware-specific differences between DWMAC4 and DWXGMAC.
>
> For the DWMAC4 IP, the Receive Normal Descriptor 0 (RDES0) is
> formatted as follows:
> 31 0
> ------------------------ -----------------------
> RDES0| Inner VLAN TAG [31:16] | Outer VLAN TAG [15:0] |
> ------------------------ -----------------------
>
> For the DWXGMAC IP, the RDES0 format varies based on the
> Tunneled Frame bit (TNP):
>
> a) For Non-Tunneled Frame (TNP=0)
>
> 31 0
> ------------------------ -----------------------
> RDES0| Inner VLAN TAG [31:16] | Outer VLAN TAG [15:0] |
> ------------------------ -----------------------
>
> b) For Tunneled Frame (TNP=1)
>
> 31 8 7 3 2 0
> --------------------- ------------------ -------
> RDES0| VNID/VSID | Reserved | OL2L3 |
> --------------------- ------------------ ------
>
> The logic for handling tunneled frames is not yet implemented
> in the dwxgmac2_wrback_get_rx_vlan_tci() function. Therefore,
> it is prudent to maintain separate functions within their
> respective descriptor driver files
> (dwxgmac2_descs.c and dwmac4_descs.c).
>
> Signed-off-by: Boon Khai Ng <boon.khai.ng@xxxxxxxxxx>
> Reviewed-by: Matthew Gerlach <matthew.gerlach@xxxxxxxxxx>
This patch does IMHO too many things together, and should be split in
several ones, i.e.:
- just moving the code in a separate file
- rename functions and simbols.
- other random changes...
> - ret = readl_poll_timeout(ioaddr + GMAC_VLAN_TAG, val,
> - !(val & GMAC_VLAN_TAG_CTRL_OB),
> - 1000, 500000);
> - if (ret) {
> - netdev_err(dev, "Timeout accessing MAC_VLAN_Tag_Filter\n");
> - return -EBUSY;
> - }
> + for (i = 0; i < timeout; i++) {
> + val = readl(ioaddr + VLAN_TAG);
> + if (!(val & VLAN_TAG_CTRL_OB))
> + return 0;
> + udelay(1);
> + }
> +
> + netdev_err(dev, "Timeout accessing MAC_VLAN_Tag_Filter\n");
> +
> + return -EBUSY;
... like the above on (which looks unnecessary?!?)
/P
