Netdev List
 help / color / mirror / Atom feed
From: Mieczyslaw Nalewaj <namiltd@yahoo.com>
To: "Alvin Šipraga" <alvin.sipraga@analog.com>
Cc: linusw@kernel.org, andrew@lunn.ch, olteanv@gmail.com,
	kuba@kernel.org, davem@davemloft.net, edumazet@google.com,
	pabeni@redhat.com, linux@armlinux.org.uk, luizluca@gmail.com,
	maxime.chevallier@bootlin.com, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH net-next v7 10/10] net: dsa: realtek: rtl8365mb: add support for RTL8367S-VB
Date: Wed, 4 Nov 2026 17:37:56 +0100	[thread overview]
Message-ID: <0dcbc902-30c8-41f0-ab45-992c8fdefe13@yahoo.com> (raw)
In-Reply-To: <ausr461gzl1pxF29@analog.com>

On 11/4/2026 2:14 PM, Alvin Šipraga wrote:
> On Wed, Nov 04, 2026 at 10:40:11AM +0100, Mieczyslaw Nalewaj wrote:
>> There is one instance for family C and one for family D, and the chip
>> table entries only say:
>>
>>     .family_info = &rtl8365mb_family_d,
> Yes, sounds reasonable. Starts to resemble how mv88e6xxx does it.
Thanks. I'm glad we are aligned on the family_info approach. Since I already
have a working draft of this refactoring, I’ve pushed the current pre-v9 code
to GitHub for early visibility. If you'd like to see how the family_info
looks in practice while reviewing, you can check it out here: https://github.com/namiltd/rtl8367d/pull/11/commits

> It seemed to me that VLAN would also benefit from being split
> out somehow. I already tried to structure the VLAN/L2 APIs into somewhat
> high- and low-level implementations, the former being accessed by
> _main.c and the latter implemented in _{vlan,l2}.c. In the cases where
> the binary format/address the chip expects is different, it seems
> logical to then have different low-level implementations depending on
> family. There might be some code duplication, but to the extent that
> it's just incidental (e.g. 50% of the stuff is the same), it can still
> improve the maintainability of the code IMO.
> But if there's also extra steps in the sequence, then perhaps
> it's better to avoid an if (family D) { do this extra thing }, and
> instead implement a whole different operation with a little code
> duplication. It's a matter of taste though.

Agreed. I will look into splitting the low-level VLAN operations into distinct
family C and D functions inside rtl8365mb_vlan.c to avoid internal if/else
checks. A bit of duplication is definitely worth it for better maintainability.

> I would encourage you not to send a v9 straight away but to discuss some
> things on the list first. Otherwise it's easy to lose track of the
> conversation.

Understood. I will hold off on sending the official v9 patchset until we finish
discussing the remaining parts here on the list.

Kind regards,
Mieczyslaw

      reply	other threads:[~2026-11-04 16:38 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <3bf4dd70-9771-497b-8c27-e925e0ef81eb.ref@yahoo.com>
2026-11-02 14:34 ` [PATCH net-next v7 0/10] net: dsa: realtek: rtl8365mb: add support for RTL8367S-VB Mieczyslaw Nalewaj
2026-11-02 14:36   ` [PATCH net-next v7 01/10] dt-bindings: net: dsa: realtek: add RTL8367S-VB Mieczyslaw Nalewaj
2026-11-02 14:37   ` [PATCH net-next v7 02/10] net: dsa: realtek: rtl8365mb: add chip family infrastructure Mieczyslaw Nalewaj
2026-11-02 14:38   ` [PATCH net-next v7 03/10] net: dsa: realtek: rtl8365mb: set speed for family D Mieczyslaw Nalewaj
2026-11-04 12:20     ` Alvin Šipraga
2026-11-02 14:39   ` [PATCH net-next v7 04/10] net: dsa: realtek: rtl8365mb: set RGMII mode " Mieczyslaw Nalewaj
2026-11-04 12:39     ` Alvin Šipraga
2026-11-02 14:40   ` [PATCH net-next v7 05/10] net: dsa: realtek: rtl8365mb: set and get vlan 4k " Mieczyslaw Nalewaj
2026-11-04 12:45     ` Alvin Šipraga
2026-11-02 14:41   ` [PATCH net-next v7 06/10] net: dsa: realtek: rtl8365mb: use raw VID for PVID on " Mieczyslaw Nalewaj
2026-11-02 14:42   ` [PATCH net-next v7 07/10] net: dsa: realtek: rtl8365mb: add family D SDS13 PCS support Mieczyslaw Nalewaj
2026-11-02 14:43   ` [PATCH net-next v7 08/10] net: dsa: realtek: rtl8365mb: re-latch the family D SerDes Mieczyslaw Nalewaj
2026-11-04 12:57     ` Alvin Šipraga
2026-11-02 14:44   ` [PATCH net-next v7 09/10] net: dsa: realtek: rtl8365mb: make L2/LUT access work on family D Mieczyslaw Nalewaj
2026-11-03 15:36     ` Alvin Šipraga
2026-11-03 16:44       ` Mieczyslaw Nalewaj
2026-11-02 14:45   ` [PATCH net-next v7 10/10] net: dsa: realtek: rtl8365mb: add support for RTL8367S-VB Mieczyslaw Nalewaj
2026-11-03 22:52     ` Alvin Šipraga
2026-11-03 23:41       ` Mieczyslaw Nalewaj
2026-11-04  9:40         ` Mieczyslaw Nalewaj
2026-11-04 13:14           ` Alvin Šipraga
2026-11-04 16:37             ` Mieczyslaw Nalewaj [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0dcbc902-30c8-41f0-ab45-992c8fdefe13@yahoo.com \
    --to=namiltd@yahoo.com \
    --cc=alvin.sipraga@analog.com \
    --cc=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=kuba@kernel.org \
    --cc=linusw@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=luizluca@gmail.com \
    --cc=maxime.chevallier@bootlin.com \
    --cc=netdev@vger.kernel.org \
    --cc=olteanv@gmail.com \
    --cc=pabeni@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox