Re: [net-next PATCH 06/10] net: dsa: realtek: rtl8365mb: add VLAN support

[Luiz Angelo Daros de Luca]

> > +     vlan4k->vid = vid;
> > +     vlan4k->member =
> > +             FIELD_GET(RTL8365MB_CVLAN_ENTRY_D0_MBR_MASK, data[0]) |
> > +             (FIELD_GET(RTL8365MB_CVLAN_ENTRY_D2_MBR_EXT_MASK, data[2])
> > +              << FIELD_WIDTH(RTL8365MB_CVLAN_ENTRY_D0_MBR_MASK));
>
> This FIELD_GET() << FIELD_WIDTH() resembles FIELD_PREP(), except that
> you skip some checks. Is that intentional?

While it resembles FIELD_PREP(), the usage of FIELD_GET() <<
FIELD_WIDTH() is intentional and semantically distinct from
FIELD_PREP(). While FIELD_PREP() prepares a raw value to be written up
into a register mask, we are performing a read/unpack operation here:
extracting bits from hardware registers to populate the driver's
internal logical structure.

Using FIELD_PREP() for this would require defining a 'fake' software
mask (e.g., GENMASK(10, 8)) that does not correspond to any physical
register, which would be more confusing to future maintainers than the
current explicit reassembly.

The 11-bit member field is split across two non-contiguous 16-bit
words in the CVLAN entry:
- data[0] (D0): Contains the primary bits [7:0] via
RTL8365MB_CVLAN_ENTRY_D0_MBR_MASK.
- data[2] (D2): Contains the extension bits [10:8] via
RTL8365MB_CVLAN_ENTRY_D2_MBR_EXT_MASK.

This split layout suggests the hardware was originally designed for 8
ports and later expanded to 11, placing the extra bits in the
previously unused space in data[2].
Even if we concatenate them into a single 48-bit value, a single
bitfield macro would not handle the non-contiguous mask.

Regards,

Luiz