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

[Linus Walleij]

On Fri, Apr 10, 2026 at 9:05 AM Luiz Angelo Daros de Luca
<luizluca@xxxxxxxxx> wrote:

> Thanks for the review, I took some time to evaluate the possibility of
> reusing rtl8366-core.

Thanks Luiz.

Even if we end up splitting out VLAN, I think my comments on the other
things that can be shared are still valid as well.

> While rtl8366-core is currently only used by RTL8366, I understand the
> goal of sharing VLAN handling across these drivers. However, RTL8365MB
> differs from RTL8366RB in how VLANs and PVIDs are implemented.

To give the whole picture, RTL8365MB wasn't even in the picture when
this abstraction was created.

The ASICs the driver was supposed to handle (and which were/is used
by OpenWrt) were:
- RTL8366RB
- RTL8366S
- RTL8367

The abstraction was part of the outoftree "swconfig" driver for a long
time before being migrated to DSA.

I haven't yet wrapped my head around if the RTL8367 (no extra letters)
is more RTL8366RB-ish or more RTL8365MB-ish... take a look at the
old code if you can figure it out from register maps etc:
https://github.com/openwrt/openwrt/tree/main/target/linux/generic/files/drivers/net/phy
(maybe Gabor knows, put him on cc)

If the VLAN is very dissimilar we can just as well have two different
implementations.

> 1) In RTL8366, VLAN4K, VLANMC and PVID are programmed together, and
> the helpers in rtl8366-core assume this 1:1 relationship. In
> RTL8365MB, the VLANMC is exclusively used as an indirect way to set
> the port VID used in the PVID (port pvid points to vlan MC table index
> and vlan MC entry has the VID number). In this patch, the VLANMC is
> conditionally allocated only when a port uses it as PVID and
> deallocated when no ports use it. VLAN MC membership is not checked by
> the HW but I still keep it in sync when it is allocated.
>
> As VLANMC, at least in RTL8365MB, is effectively only used for PVID,
> I'll put all vlan MC methods behind public PVID methods in the
> following v2. There is no need to expose them for now. For the next
> RTL8367D family, VLANMC is gone and there are dedicated registers to
> set the PVID VID number for each port.
>
> 2) In RTL8365MB, vlan is always enabled, controlling filtering by port
> (.port_vlan_filtering), while in RTL8366, it is enabled by vlan_add.
>
> 3) The abstraction doesn't set the framefilter in RTL8366 nor does it
> clear PVID status. I'm using framefilter as a way to disable PVID,
> dropping untagged frames, together with setting PVID VLAN MC index to
> 0 and keeping an empty (vid=0) vlan config at that index.
>
> Without clearing the PVID state in RTL8366, removing a PVID VLAN
> member and re-adding it as a non-PVID member will leave it configured
> as PVID.

OK seems we should split up the VLAN handling if there is nothing
to share in the abstraction here. We don't do abstraction for abstractions
sake.

I'd say make a patch moving all functions from realtek_ops
that can't be reused into the rtl8366rb driver and make them static.

But probably keep the shared core for other stuff. Though I guess it
might be empty then until we add more stuff :/

> 4) I don't know how IVL would be handled in RTL8366... but that's a
> topic for another patch.

OTOMH it seems half of it is generic but it's a helicopter view.
There is no question the ASICs are related, we just don't know
by how much...

> It is possible that the hardware
> behavior between RTL8366 and RTL8365MB is more similar than what the
> current drivers suggest. However, the existing rtl8366-core
> abstraction does not model this in a way that can be directly reused
> here. Aligning both drivers would likely require reworking that
> abstraction rather than adapting the RTL8365MB implementation to fit
> it.
>
> This could be explored in the future as a separate effort as it
> changes how RTL8366RB driver behave. For now, I would prefer to keep
> this implementation local to RTL8365MB and then follow up with a
> separate effort to reconcile both drivers and extract shared logic.
> Some of that work will likely become clearer while adding RTL8367D
> family support.

The reason I push back a bit is that any kind of things that are
"fix later" tend to never actually happen in my experience.
Either we add the abstraction now or never.

For VLAN I'm pretty much convinced that there is not much to
share. For port separation/isolation and possibly FDB I'm not as
convinced.

Yours,
Linus Walleij