Message-ID: <YzLybsJBIHtbQOwE@lunn.ch>
Date:   Tue, 27 Sep 2022 14:54:06 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Yoshihiro Shimoda <yoshihiro.shimoda.uh@...esas.com>
Cc:     "davem@...emloft.net" <davem@...emloft.net>,
        "edumazet@...gle.com" <edumazet@...gle.com>,
        "kuba@...nel.org" <kuba@...nel.org>,
        "pabeni@...hat.com" <pabeni@...hat.com>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>,
        "krzysztof.kozlowski+dt@...aro.org" 
        <krzysztof.kozlowski+dt@...aro.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-renesas-soc@...r.kernel.org" 
        <linux-renesas-soc@...r.kernel.org>
Subject: Re: [PATCH v3 2/3] net: ethernet: renesas: Add Ethernet Switch driver

> > How do the CPU ports differ to the other ports? When you mention CPU
> > ports, it makes me wonder if this should be a DSA driver?
> 
> I compared a DSA diagram [1] and this Ethernet Switch and then
> this switch differs than the DSA diagram:
> - This switch has a feature which accesses DRAM directly like an "ethernet controller".
>   I called this feature as "cpu port", but it might be incorrect.
> - This switch has doesn't have any "control path". Instead of that, this switch
>   is controled by registers via APB (internal bus) directly.
> 
> So, IIUC, this should not be a DSA driver.
> 
> [1] https://bootlin.com/blog/tag/dsa/
> 
> > Is there a public data sheet for this device?
> 
> Unfortunately, we have no public data sheet for this device.
> But, I tried to figure this switch diagram about control/data paths as below:
> 
> Control path:
> +--- R-Car S4-8 SoC -------------------------+
> |                                            |
> | CPU ---(APB bus)---+--- Ethernet Switch ---|---(MDIO)--------------+
> |                    |                       |                       |
> |                    +--- Ethernet SERDES    |              External Ethernet PHY --- RJ45
> |                                            |
> +--------------------------------------------+
> Notes: The switch and SERDES have 3 ports of MDIO and SGMII.
> 
> Data Path:
> +--- R-Car S4-8 SoC ---------------------------------------------------------+
> |                                                                            |
> | CPU ---(AXI bus)---+--- DRAM        +--------+                             |
> |                    +---(cpu port)---|        |---(ether port)--- SERDES ---|---(SGMII)--- PHY --- RJ45
> |                    |                | Switch |---(ether port)--- SERDES ---|---(SGMII)--- PHY --- RJ45
> |                    +---(cpu port)---|        |---(ether port)--- SERDES ---|---(SGMII)--- PHY --- RJ45
> |                                     +--------|                             |
> +----------------------------------------------------------------------------+

This device is somewhere between a DSA switch and a pure switchdev
switch. It probably could be modelled as a DSA switch.

For a pure switchdev switch, you have a set of DMA channels per user
port. So with 3 user ports, i would expect there to be three sets of
RX and TX DMA rings. Frames from the CPU would go directly to the user
ports, bypassing the switch.

For this hardware, how are the rings organised? Are the rings for the
CPU ports, or for the user ports? How do you direct a frame from the
CPU out a specific user port? Via the DMA ring you place it into, or
do you need a tag on the frame to indicate its egress port?

The memory mapping of the registers is not really an issue. The
B52/SF2 can have memory mapped registers.

> The current driver only supports one of MDIO, cpu port and ethernet port, and it acts as an ethernet device.
> 
> > > Perhaps, since the current driver supports 1 ethernet port and 1 CPU port only,
> > > I should modify this driver for the current condition strictly.
> > 
> > I would suggest you support all three user ports. For an initial
> > driver you don't need to support any sort of acceleration. You don't
> > need any hardware bridging etc. That can be added later. Just three
> > separated ports.
> 
> Thank you for your suggestion. However, supporting three user ports
> is required to modify an external ethernet PHY driver.
> (For now, a boot loader initialized the PHY, but one port only.)
> 
> The PHY is 88E2110 on my environment, so Linux has a driver in
> drivers/net/phy/marvell10g.c. However, I guess this is related to
> configuration of the PHY chip on the board, it needs to change
> the host 7interface mode, but the driver doesn't support it for now.

Please give us more details. The marvell10g driver will change its
host side depending on the result of the line side negotiation. It
changes the value of phydev->interface to indicate what is it doing on
its host side, and you have some control over what modes it will use
on the host side. You can probably define its initial host side mode
via phy-mode in DT.

Also, relying on the bootloader is a bad idea. People like to change
the bootloader, upgrade it for new features, etc. In the ARM world we
try to avoid any dependency on the bootloader.

> > In the DSA world we call these user ports.
> 
> I got it.
> However, when I read the dsa document of Linux kernel,
> it seems to call DSA ports. Perhaps, I could misunderstand the document though...
> https://docs.kernel.org/networking/dsa/dsa.html

DSA has four classes of ports:

User ports:   connected to the outside world
CPU ports:    connected to the SoC
DSA ports:    connecting between switches. If you have two switches and want
              to combine them into one cluster, you use DSA ports between
	      them.
Unused ports: Physically exist, but are unused in the board design.

       Andrew

Powered by blists - more mailing lists