Message-ID: <CAJq09z5EWps3j1jZPj2J+j=hmzmpvF8QdUJpKADz6nQ_jJEjGw@mail.gmail.com>
Date: Wed, 3 Jul 2024 17:31:10 -0300
From: Luiz Angelo Daros de Luca <luizluca@...il.com>
To: Chris Packham <chris.packham@...iedtelesis.co.nz>
Cc: Linus Walleij <linus.walleij@...aro.org>, "alsi@...g-olufsen.dk" <alsi@...g-olufsen.dk>, 
	Andrew Lunn <andrew@...n.ch>, Florian Fainelli <f.fainelli@...il.com>, 
	"olteanv@...il.com" <olteanv@...il.com>, Marek Behún <kabel@...nel.org>, 
	"ericwouds@...il.com" <ericwouds@...il.com>, David Miller <davem@...emloft.net>, 
	Eric Dumazet <edumazet@...gle.com>, "kuba@...nel.org" <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, 
	"justinstitt@...gle.com" <justinstitt@...gle.com>, 
	"rmk+kernel@...linux.org.uk" <rmk+kernel@...linux.org.uk>, netdev <netdev@...r.kernel.org>, 
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, 
	"sander@...nheule.net" <sander@...nheule.net>
Subject: Re: net: dsa: Realtek switch drivers

Em seg., 1 de jul. de 2024, 23:09, Chris Packham
<chris.packham@...iedtelesis.co.nz> escreveu:
>
>
> On 15/06/24 09:36, Luiz Angelo Daros de Luca wrote:
> > Hello Chris and Linus,
> >
> >>> I'm starting to look at some L2/L3 switches with Realtek silicon. I see
> >>> in the upstream kernel there are dsa drivers for a couple of simple L2
> >>> switches. While openwrt has support for a lot of the more advanced
> >>> silicon. I'm just wondering if there is a particular reason no-one has
> >>> attempted to upstream support for these switches?
> >> It began with the RTL8366RB ("RTL8366 revision B") which I think is
> >> equivalent to RTL8366S as well, but have not been able to test.
> >>
> >> Then Luiz and Alvin jumped in and fixed up the RTL8365MB family.
> >>
> >> So the support is pretty much what is stated in the DT bindings
> >> in Documentation/devicetree/bindings/net/dsa/realtek.yaml:
> >>
> >> properties:
> >>    compatible:
> >>      enum:
> >>        - realtek,rtl8365mb
> >>        - realtek,rtl8366rb
> >>      description: |
> >>        realtek,rtl8365mb:
> >>          Use with models RTL8363NB, RTL8363NB-VB, RTL8363SC, RTL8363SC-VB,
> >>          RTL8364NB, RTL8364NB-VB, RTL8365MB, RTL8366SC, RTL8367RB-VB, RTL8367S,
> >>          RTL8367SB, RTL8370MB, RTL8310SR
> >>        realtek,rtl8366rb:
> >>          Use with models RTL8366RB, RTL8366S
> >>
> >> It may look like just RTL8365 and RTL8366 on the surface but the sub-version
> >> is detected at runtime.
> >>
> >>> If I were to start
> >>> grabbing drivers from openwrt and trying to get them landed would that
> >>> be a problem?
> >> I think the base is there, when I started with RTL8366RB it was pretty
> >> uphill but the kernel DSA experts (Vladimir & Andrew especially) are super
> >> helpful so eventually we have arrived at something that works reasonably.
> >>
> >> The RTL8356MB-family driver is more advanced and has a lot more features,
> >> notably it supports all known RTL8367 variants.
> > I played with RTL8367R. It mostly works with rtl8365mb driver but I
> > wasn't able to enable the CPU tagging. Although
> >
> >> The upstream OpenWrt in target/linux/generic/files/drivers/net/phy
> >> has the following drivers for the old switchdev:
> >> -rw-r--r--. 1 linus linus 25382 Jun  7 21:44 rtl8306.c
> >> -rw-r--r--. 1 linus linus 40268 Jun  7 21:44 rtl8366rb.c
> >> -rw-r--r--. 1 linus linus 33681 Jun  7 21:44 rtl8366s.c
> >> -rw-r--r--. 1 linus linus 36324 Jun  7 21:44 rtl8366_smi.c
> >> -rw-r--r--. 1 linus linus  4838 Jun  7 21:44 rtl8366_smi.h
> >> -rw-r--r--. 1 linus linus 58021 Jun 12 18:50 rtl8367b.c
> >> -rw-r--r--. 1 linus linus 59612 Jun 12 18:50 rtl8367.c
> >>
> >> As far as I can tell we cover all but RTL8306 with the current in-tree
> >> drivers, the only reason these are still in OpenWrt would be that some
> >> boards are not migrated to DSA.
> > These drivers you listed are mostly found in old or low spec devices.
> > There is little incentive to invest too much time to migrate them. For
> > rtl8365mb, it still lacks support for vlan and forwarding offload. So,
> > the swconfig driver still makes sense.
> > There is also a performance problem with checksum offloading. These
> > switches are used with non-realtek SoC, which might lead to:
> >
> > "Checksum offload should work with category 1 and 2 taggers when the
> > DSA conduit driver declares NETIF_F_HW_CSUM in vlan_features and looks
> > at csum_start and csum_offset. For those cases, DSA will shift the
> > checksum start and offset by the tag size. If the DSA conduit driver
> > still uses the legacy NETIF_F_IP_CSUM or NETIF_F_IPV6_CSUM in
> > vlan_features, the offload might only work if the offload hardware
> > already expects that specific tag (perhaps due to matching vendors).
> > DSA user ports inherit those flags from the conduit, and it is up to
> > the driver to correctly fall back to software checksum when the IP
> > header is not where the hardware expects. If that check is
> > ineffective, the packets might go to the network without a proper
> > checksum (the checksum field will have the pseudo IP header sum). For
> > category 3, when the offload hardware does not already expect the
> > switch tag in use, the checksum must be calculated before any tag is
> > inserted (i.e. inside the tagger). Otherwise, the DSA conduit would
> > include the tail tag in the (software or hardware) checksum
> > calculation. Then, when the tag gets stripped by the switch during
> > transmission, it will leave an incorrect IP checksum in place."
> > See: https://docs.kernel.org/networking/dsa/dsa.html
> >
> >> But maybe I missed something?
> > I guess Chris is talking about the realtek target that uses Realtek
> > SoC (target/linux/realtek/files-5.15/). That is a completely different
> > beast. Although it might share some (or a lot) logic with current
> > upstream drivers, it is way more complex. It might require a
> > multi-function device driver. Anyway, the current realtek SoC/target
> > drivers need some love, like using regmap, implement functions using
> > an abstraction layer (and not if model a inside the code), get rid of
> > all magic numbers and replace them with meaningful macros, create a
> > proper tagger (and not translate a generic one just before forwarding
> > it). In OpenWrt, a code that gets things done might be acceptable but
> > the upstream kernel requires something more maintainable. So, if you
> > want to upstream those drivers, you can start by improving them in the
> > openwrt.
>
> So now got access to the Realtek docs and I've been pouring over them
> and the openwrt code (I'm avoiding looking at the Realtek SDK for now,
> just to make sure I don't submit something I don't have the rights to).
>
> If someone were to look at the block diagram in the brief datasheet
> they'd probably come away with the impression that it very much fits the
> DSA model. There's a SoC portion with the CPU, peripherals and a "NIC".
> That NIC is connected to the CPU MAC in the switch block. All seems like
> a pretty standard DSA type design and that's what the openwrt code
> implements a ethernet/rtl838x_eth.c driver for the Ethernet NIC and a
> dsa/rtl83xx driver for the DSA switch.
>
> But when you start digging into the detail you find that the registers
> for the NIC are scattered through the address space for the switch. Same
> for the MDIO related registers. There is a more detailed block diagram
> in the CPU and Peripherals datasheet that shows the NIC and switch as
> part of the same IP block. The openwrt implementation does things that I
> think would be frowned upon upstream like calling from the Ethernet
> driver into the switch driver to access registers.

Wouldn't that be a case for Multi-Function Device driver? From docs,
"A typical MFD can be:
- A mixed signal ASIC on an external bus, sometimes a PMIC (Power
Management Integrated Circuit) that is manufactured in a lower
technology node (rough silicon) that handles analog drivers for things
like audio amplifiers, LED drivers, level shifters, PHY (physical
interfaces to things like USB or ethernet), regulators etc.
- A range of memory registers containing "miscellaneous system
registers" also known as a system controller "syscon" or any other
memory range containing a mix of unrelated hardware devices."

Vladimir Oltean was the first to suggest to me the use of MFD for DSA
switches as they commonly need two drivers: DSA and the user mii bus
driver. The Realtek SoC just seems to make it more needed as we have
other functions registers scattered.
https://lore.kernel.org/netdev/20231211143513.n6ms3dlp6rrcqya6@skbuf/

The OpenWrt driver does work and I think it can be incrementally
improved up to upstream status. It is a bit messy but you could start
untangling it there, trying to decouple (the non-existing) tagger,
ethernet, phy and DSA drivers (and whatever else shares the same
register space). The upstream realtek drivers use a common
code+subdrivers model while the Realtek SoC DSA driver in OpenWrt uses
a single driver with lots of ifs inside each function. I don't know if
they can be merged into a single driver tree, sharing as much as
possible, or Realtek SoC will require a completely duplicated tree.

> This leads me to conclude that what Realtek call the "NIC" is actually
> just the DMA interface for packets sent from or trapped to the CPU.
> Rather than trying to make this fit the DSA model I should be looking at
> using switchdev directly (I can still probably leverage a lot of code
> from the openwrt drivers because the switch tables are the same either way).
>
> What would I be loosing if I don't use the DSA infrastructure? I got
> kind of hung up at the point where it really wanted a CPU port and I
> just couldn't provide a nice discrete NIC.

---
     Luiz Angelo Daros de Luca
            luizluca@...il.com

Powered by blists - more mailing lists