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

[Mieczyslaw Nalewaj <namiltd@yahoo.com>]

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