Re: [PATCH net-next v7 10/10] net: dsa: realtek: rtl8365mb: add support for RTL8367S-VB

[Mieczyslaw Nalewaj]

On Wed, Nov 04, 2026, Alvin Šipraga wrote:
> I haven't yet had the time to look closely at all the patches [...].
> But I'll try and take a closer look tomorrow and point out where else
> we could improve in this direction. I hope it's possible to remove
> most of the if (family is x) { do something } constructs and keep
> things flat.

Hi Alvin,

While waiting for your comments I went through the series myself with
that goal. Per-family constants and the L2 serializers are the cleanest
case, so I'm taking them one step further than a num_ports member.
Instead of adding more members to struct rtl8365mb_chip_info one by
one, I'm grouping everything that depends only on the family into a
single struct that chip_info points to:

  struct rtl8365mb_family_info {
    enum rtl8365mb_family family;
    unsigned int num_ports;
    const struct rtl8365mb_jam_tbl_entry *jam_table;
    size_t jam_size;
    size_t l2_entry_size;
    void (*l2_data_to_uc)(const u16 *data, struct rtl8365mb_l2_uc *uc);
    void (*l2_uc_to_data)(const struct rtl8365mb_l2_uc *uc, u16 *data);
    void (*l2_data_to_mc)(const u16 *data, struct rtl8365mb_l2_mc *mc);
    void (*l2_mc_to_data)(const struct rtl8365mb_l2_mc *mc, u16 *data);
  };

There is one instance for family C and one for family D, and the chip
table entries only say:

    .family_info = &rtl8365mb_family_d,

> The reason I'm saying this is that I saw quite a few places where you
> have a construct of the form:
>
> if (family is d)
>   // use family d constant
> else
>   // use family c constant

For the L2 code this removes the construct entirely: the four
rtl8365mb_l2_*() dispatchers and rtl8365mb_l2_entry_size() are gone,
and the callers use fi->l2_uc_to_data(), fi->l2_entry_size and so on.
The same goes for num_ports in detect() and the jam table in
switch_init().

Some places differ in sequence and not only in constants, e.g. the
SerDes configuration, the CVLAN entry packing and the PVID handling.
I'd rather not hide those behind callbacks without a clear gain, but
I'll go through them again and convert what reads better as
per-family data or ops. If you see specific spots when you review the
rest of the series, please point them out and I'll look at those
first.

Thanks,
Mieczyslaw