mbox series

[net-next,v9,00/16] add support for Renesas RZ/N1 ethernet subsystem devices

Message ID 20220624144001.95518-1-clement.leger@bootlin.com
Headers show
Series add support for Renesas RZ/N1 ethernet subsystem devices | expand

Message

Clément Léger June 24, 2022, 2:39 p.m. UTC
The Renesas RZ/N1 SoCs features an ethernet subsystem which contains
(most notably) a switch, two GMACs, and a MII converter [1]. This
series adds support for the switch and the MII converter.

The MII converter present on this SoC has been represented as a PCS
which sit between the MACs and the PHY. This PCS driver is probed from
the device-tree since it requires to be configured. Indeed the MII
converter also contains the registers that are handling the muxing of
ports (Switch, MAC, HSR, RTOS, etc) internally to the SoC.

The switch driver is based on DSA and exposes 4 ports + 1 CPU
management port. It include basic bridging support as well as FDB and
statistics support.

Link: [1] https://www.renesas.com/us/en/document/mah/rzn1d-group-rzn1s-group-rzn1l-group-users-manual-r-engine-and-ethernet-peripherals

-----
Changes in V9:
- Cover letter:
  - Remove comment about RZN1 patches that are now in the master branch.
- Commits:
  - Add Vladimir Oltean Reviewed-by
- PCS:
  - Add "Depends on OF" for PCS_RZN1_MIIC due to error found by intel
    kernel test robot <lkp@intel.com>.
  - Check return value of of_property_read_u32() for
    "renesas,miic-switch-portin" property before setting conf.
  - Return miic_parse_dt() return value in miic_probe() on error
- Switch:
  - Add "Depends on OF" for NET_DSA_RZN1_A5PSW due to errors found by
    intel kernel test robot <lkp@intel.com>.
- DT:
  - Add spaces between switch port and '{'

Changes in V8:
- Commits:
  - Reorder Acked-By and Rewieved-by chronologically and add Florian
    Fainelli Reviewed-By.
- PCS:
  - Fix of node leaks
- MAINTAINERS:
  - Reorder L: before S:

Resent V7 due to messed up cover letter.

Changes in V7:
- Commits:
  - Add Rob Herring Acked-by for commit "dt-bindings: net: snps,dwmac: add
    "power-domains" property"
  - Rebased on net-next/master
- MAINTAINERS:
  - Add renesas-soc and netdev mailing lists

Changes in V6:
- Commits:
  - Add commit which enable ethernet switch on RZ/N1D-DB board
  - Add commit which adds "renesas,rzn1-gmac" compatible t
    "snps,dwmac" bindings
  - Fix mutex change being done in FDB feature commit
  - Add commit which  adds"power-domains" to "snps,dwmac" bindings
- Bindings and DT
  - Add clock-names to MII converter and make it required
  - Added Reviewed-by Geert on MII converter binding
  - Added "power-domains" to switch bindings and to switch description
  - Use new compatible "renesas,rzn1-gmac" for GMAC2
  - Describe all switch ports in ethernet switch node
  - Add phy-mode = "internal" to cpu port
- PCS:
  - use phy_interface_mode_is_rgmii() instead of open coded check
  - Add device_link_add() call in miic_create()
- Switch:
  - Fix missing of_node_put(port) in case of loop break.
  - Fix comment alignment for statistics defines
  - Move lk_lock mutex locking outside of the fdb_dump loop

Changes in V5:
- MAINTAINERS:
  - Add Florian Fainelli Reviewed-by
- Switch:
  - Switch Lookup table lock to a mutex instead of a spinlock
  - Only handle "ethernet-ports" property for switch ports
  - Handle RGMII_ID/RXID/TXID
  - Add check for pdata to be non null in remove
  - Add missing of_node_put() for mdio and ports
  - Applied Florian Fainelli patch which makes stats description
    shorter
  - Add Kconfig dependency on ARCH_RZN1 to avoid Kconfig "unmet direct
    dependency"
- PCS:
  - Handle RGMII_ID/RXID/TXID
  - Use value instead of BIT() for speed/mode
- Tag driver:
  - Add Florian Fainelli Reviewed-by

Changes in V4:
- Add ETH_P_DSA_A5PSW in uapi/linux/if_ether.h
- PCS:
  - Use devm_pm_runtime_enable() instead of pm_runtime_enable()
- Switch:
  - Return -EOPNOTSUPP and set extack when multiple bridges are created
  - Remove error messages in fdb_del if entry does not exists
  - Add compatibility with "ethernet-ports" device-tree property
- Tag driver:
  - Use ETH_ZLEN as padding len

Changes in V3:
- PCS:
  - Fixed reverse christmas tree declaration
  - Remove spurious pr_err
  - Use pm_runtime functions
- Tag driver:
  - Remove packed attribute from the tag struct
- Switch:
  - Fix missing spin_unlock in fdb_dump in case of error
  - Add static qualifier to dsa_switch_ops
  - Add missing documentation for hclk and clk members of struct a5psw
  - Changed types of fdb_entry to u16 to discard GCC note on char
    packed bitfields and add reserved field
- Added Reviewed-by tag from Florian Fainelli

Changes in V2:
- PCS:
  - Fix Reverse Christmas tree declaration
  - Removed stray newline
  - Add PCS remove function and disable clocks in them
  - Fix miic_validate function to return correct values
  - Split PCS CONV_MODE definition
  - Reordered phylink_pcs_ops in definition order
  - Remove interface setting in miic_link_up
  - Remove useless checks for invalid interface/speed and error prints
  - Replace phylink_pcs_to_miic_port macro by a static function
  - Add comment in miic_probe about platform_set_drvdata
- Bindings:
 - Fix wrong path for mdio.yaml $ref
 - Fix yamllint errors
- Tag driver:
  - Squashed commit that added tag value with tag driver
  - Add BUILD_BUG_ON for tag size
  - Split control_data2 in 2 16bits values
- Switch:
  - Use .phylink_get_caps instead of .phylink_validate and fill
    supported_interface correctly
  - Use fixed size (ETH_GSTRING_LEN) string for stats and use memcpy
  - Remove stats access locking since RTNL lock is used in upper layers
  - Check for non C45 addresses in mdio_read/write and return
    -EOPNOTSUPP
  - Add get_eth_mac_stats, get_eth_mac_ctrl_stat, get_rmon_stats
  - Fix a few indentation problems
  - Remove reset callback from MDIO bus operation
  - Add phy/mac/rmon stats
- Add get_rmon_stat to dsa_ops

Clément Léger (16):
  net: dsa: allow port_bridge_join() to override extack message
  net: dsa: add support for ethtool get_rmon_stats()
  net: dsa: add Renesas RZ/N1 switch tag driver
  dt-bindings: net: pcs: add bindings for Renesas RZ/N1 MII converter
  net: pcs: add Renesas MII converter driver
  dt-bindings: net: dsa: add bindings for Renesas RZ/N1 Advanced 5 port
    switch
  net: dsa: rzn1-a5psw: add Renesas RZ/N1 advanced 5 port switch driver
  net: dsa: rzn1-a5psw: add statistics support
  net: dsa: rzn1-a5psw: add FDB support
  dt-bindings: net: snps,dwmac: add "power-domains" property
  dt-bindings: net: snps,dwmac: add "renesas,rzn1" compatible
  ARM: dts: r9a06g032: describe MII converter
  ARM: dts: r9a06g032: describe GMAC2
  ARM: dts: r9a06g032: describe switch
  ARM: dts: r9a06g032-rzn1d400-db: add switch description
  MAINTAINERS: add Renesas RZ/N1 switch related driver entry

 .../bindings/net/dsa/renesas,rzn1-a5psw.yaml  |  134 +++
 .../bindings/net/pcs/renesas,rzn1-miic.yaml   |  171 +++
 .../devicetree/bindings/net/snps,dwmac.yaml   |    5 +
 MAINTAINERS                                   |   13 +
 arch/arm/boot/dts/r9a06g032-rzn1d400-db.dts   |  117 ++
 arch/arm/boot/dts/r9a06g032.dtsi              |  108 ++
 drivers/net/dsa/Kconfig                       |    9 +
 drivers/net/dsa/Makefile                      |    1 +
 drivers/net/dsa/rzn1_a5psw.c                  | 1062 +++++++++++++++++
 drivers/net/dsa/rzn1_a5psw.h                  |  259 ++++
 drivers/net/pcs/Kconfig                       |    8 +
 drivers/net/pcs/Makefile                      |    1 +
 drivers/net/pcs/pcs-rzn1-miic.c               |  520 ++++++++
 include/dt-bindings/net/pcs-rzn1-miic.h       |   33 +
 include/linux/pcs-rzn1-miic.h                 |   18 +
 include/net/dsa.h                             |    5 +
 include/uapi/linux/if_ether.h                 |    1 +
 net/dsa/Kconfig                               |    7 +
 net/dsa/Makefile                              |    1 +
 net/dsa/slave.c                               |   18 +-
 net/dsa/tag_rzn1_a5psw.c                      |  113 ++
 21 files changed, 2602 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/dsa/renesas,rzn1-a5psw.yaml
 create mode 100644 Documentation/devicetree/bindings/net/pcs/renesas,rzn1-miic.yaml
 create mode 100644 drivers/net/dsa/rzn1_a5psw.c
 create mode 100644 drivers/net/dsa/rzn1_a5psw.h
 create mode 100644 drivers/net/pcs/pcs-rzn1-miic.c
 create mode 100644 include/dt-bindings/net/pcs-rzn1-miic.h
 create mode 100644 include/linux/pcs-rzn1-miic.h
 create mode 100644 net/dsa/tag_rzn1_a5psw.c

Comments

Vladimir Oltean June 24, 2022, 9:43 p.m. UTC | #1
On Fri, Jun 24, 2022 at 04:39:45PM +0200, Clément Léger wrote:
> The Renesas RZ/N1 SoCs features an ethernet subsystem which contains
> (most notably) a switch, two GMACs, and a MII converter [1]. This
> series adds support for the switch and the MII converter.
> 
> The MII converter present on this SoC has been represented as a PCS
> which sit between the MACs and the PHY. This PCS driver is probed from
> the device-tree since it requires to be configured. Indeed the MII
> converter also contains the registers that are handling the muxing of
> ports (Switch, MAC, HSR, RTOS, etc) internally to the SoC.
> 
> The switch driver is based on DSA and exposes 4 ports + 1 CPU
> management port. It include basic bridging support as well as FDB and
> statistics support.
> 
> Link: [1] https://www.renesas.com/us/en/document/mah/rzn1d-group-rzn1s-group-rzn1l-group-users-manual-r-engine-and-ethernet-peripherals
> 
> -----
> Changes in V9:
> - Cover letter:
>   - Remove comment about RZN1 patches that are now in the master branch.
> - Commits:
>   - Add Vladimir Oltean Reviewed-by
> - PCS:
>   - Add "Depends on OF" for PCS_RZN1_MIIC due to error found by intel
>     kernel test robot <lkp@intel.com>.
>   - Check return value of of_property_read_u32() for
>     "renesas,miic-switch-portin" property before setting conf.
>   - Return miic_parse_dt() return value in miic_probe() on error
> - Switch:
>   - Add "Depends on OF" for NET_DSA_RZN1_A5PSW due to errors found by
>     intel kernel test robot <lkp@intel.com>.
> - DT:
>   - Add spaces between switch port and '{'

I took one more look through the series and this looks good, thanks.

Reviewed-by: Vladimir Oltean <olteanv@gmail.com>
Florian Fainelli June 25, 2022, 2:35 a.m. UTC | #2
On 6/24/2022 7:39 AM, Clément Léger wrote:
> Add a PCS driver for the MII converter that is present on the Renesas
> RZ/N1 SoC. This MII converter is reponsible for converting MII to
> RMII/RGMII or act as a MII pass-trough. Exposing it as a PCS allows to
> reuse it in both the switch driver and the stmmac driver. Currently,
> this driver only allows the PCS to be used by the dual Cortex-A7
> subsystem since the register locking system is not used.
> 
> Signed-off-by: Clément Léger <clement.leger@bootlin.com>
> Reviewed-by: Vladimir Oltean <olteanv@gmail.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Florian Fainelli June 25, 2022, 2:37 a.m. UTC | #3
On 6/24/2022 7:39 AM, Clément Léger wrote:
> Add Renesas RZ/N1 advanced 5 port switch driver. This switch handles 5
> ports including 1 CPU management port. A MDIO bus is also exposed by
> this switch and allows to communicate with PHYs connected to the ports.
> Each switch port (except for the CPU management ports) is connected to
> the MII converter.
> 
> This driver includes basic bridging support, more support will be added
> later (vlan, etc).
> 
> Suggested-by: Jean-Pierre Geslin <jean-pierre.geslin@non.se.com>
> Suggested-by: Phil Edworthy <phil.edworthy@renesas.com>
> Signed-off-by: Clément Léger <clement.leger@bootlin.com>
> Reviewed-by: Vladimir Oltean <olteanv@gmail.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Florian Fainelli June 25, 2022, 2:37 a.m. UTC | #4
On 6/24/2022 7:39 AM, Clément Léger wrote:
> This commits add forwarding database support to the driver. It
> implements fdb_add(), fdb_del() and fdb_dump().
> 
> Signed-off-by: Clément Léger <clement.leger@bootlin.com>
> Reviewed-by: Vladimir Oltean <olteanv@gmail.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Florian Fainelli June 25, 2022, 2:38 a.m. UTC | #5
On 6/24/2022 7:39 AM, Clément Léger wrote:
> Add the MII converter node which describes the MII converter that is
> present on the RZ/N1 SoC.
> 
> Signed-off-by: Clément Léger <clement.leger@bootlin.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Florian Fainelli June 25, 2022, 2:38 a.m. UTC | #6
On 6/24/2022 7:39 AM, Clément Léger wrote:
> RZ/N1 SoC includes two MAC named GMACx that are compatible with the
> "snps,dwmac" driver. GMAC1 is connected directly to the MII converter
> port 1. GMAC2 however can be used as the MAC for the switch CPU
> management port or can be muxed to be connected directly to the MII
> converter port 2. This commit add description for the GMAC2 which will
> be used by the switch description.
> 
> Signed-off-by: Clément Léger <clement.leger@bootlin.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Florian Fainelli June 25, 2022, 2:39 a.m. UTC | #7
On 6/24/2022 7:39 AM, Clément Léger wrote:
> Add description of the switch that is present on the RZ/N1 SoC. This
> description includes ethernet-ports description for all the ports that
> are present on the switch along with their connection to the MII
> converter ports and to the GMAC for the CPU port.
> 
> Signed-off-by: Clément Léger <clement.leger@bootlin.com>
> Reviewed-by: Vladimir Oltean <olteanv@gmail.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Florian Fainelli June 25, 2022, 2:40 a.m. UTC | #8
On 6/24/2022 7:40 AM, Clément Léger wrote:
> Add description for the switch, GMAC2 and MII converter. With these
> definitions, the switch port 0 and 1 (MII port 5 and 4) are working on
> RZ/N1D-DB board.
> 
> Signed-off-by: Clément Léger <clement.leger@bootlin.com>
> Reviewed-by: Vladimir Oltean <olteanv@gmail.com>
> ---

[snip]

> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pins_mdio1>, <&pins_eth3>, <&pins_eth4>;
> +
> +	dsa,member = <0 0>;

Does not hurt to have it, but not required at this point. Not a reson to 
spin a v10 though:

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Clément Léger June 27, 2022, 7:38 a.m. UTC | #9
Le Sat, 25 Jun 2022 00:43:30 +0300,
Vladimir Oltean <olteanv@gmail.com> a écrit :

> On Fri, Jun 24, 2022 at 04:39:45PM +0200, Clément Léger wrote:
> > The Renesas RZ/N1 SoCs features an ethernet subsystem which contains
> > (most notably) a switch, two GMACs, and a MII converter [1]. This
> > series adds support for the switch and the MII converter.
> > 
> > The MII converter present on this SoC has been represented as a PCS
> > which sit between the MACs and the PHY. This PCS driver is probed from
> > the device-tree since it requires to be configured. Indeed the MII
> > converter also contains the registers that are handling the muxing of
> > ports (Switch, MAC, HSR, RTOS, etc) internally to the SoC.
> > 
> > The switch driver is based on DSA and exposes 4 ports + 1 CPU
> > management port. It include basic bridging support as well as FDB and
> > statistics support.
> > 
> > Link: [1] https://www.renesas.com/us/en/document/mah/rzn1d-group-rzn1s-group-rzn1l-group-users-manual-r-engine-and-ethernet-peripherals
> > 
> > -----
> > Changes in V9:
> > - Cover letter:
> >   - Remove comment about RZN1 patches that are now in the master branch.
> > - Commits:
> >   - Add Vladimir Oltean Reviewed-by
> > - PCS:
> >   - Add "Depends on OF" for PCS_RZN1_MIIC due to error found by intel
> >     kernel test robot <lkp@intel.com>.
> >   - Check return value of of_property_read_u32() for
> >     "renesas,miic-switch-portin" property before setting conf.
> >   - Return miic_parse_dt() return value in miic_probe() on error
> > - Switch:
> >   - Add "Depends on OF" for NET_DSA_RZN1_A5PSW due to errors found by
> >     intel kernel test robot <lkp@intel.com>.
> > - DT:
> >   - Add spaces between switch port and '{'  
> 
> I took one more look through the series and this looks good, thanks.

Hi Vladimir,

Thanks for the review.

Clément

> 
> Reviewed-by: Vladimir Oltean <olteanv@gmail.com>
patchwork-bot+netdevbpf@kernel.org June 27, 2022, 10:50 a.m. UTC | #10
Hello:

This series was applied to netdev/net-next.git (master)
by David S. Miller <davem@davemloft.net>:

On Fri, 24 Jun 2022 16:39:45 +0200 you wrote:
> The Renesas RZ/N1 SoCs features an ethernet subsystem which contains
> (most notably) a switch, two GMACs, and a MII converter [1]. This
> series adds support for the switch and the MII converter.
> 
> The MII converter present on this SoC has been represented as a PCS
> which sit between the MACs and the PHY. This PCS driver is probed from
> the device-tree since it requires to be configured. Indeed the MII
> converter also contains the registers that are handling the muxing of
> ports (Switch, MAC, HSR, RTOS, etc) internally to the SoC.
> 
> [...]

Here is the summary with links:
  - [net-next,v9,01/16] net: dsa: allow port_bridge_join() to override extack message
    https://git.kernel.org/netdev/net-next/c/1c6e8088d9a7
  - [net-next,v9,02/16] net: dsa: add support for ethtool get_rmon_stats()
    https://git.kernel.org/netdev/net-next/c/67f38b1c7324
  - [net-next,v9,03/16] net: dsa: add Renesas RZ/N1 switch tag driver
    https://git.kernel.org/netdev/net-next/c/a08d6a6dc820
  - [net-next,v9,04/16] dt-bindings: net: pcs: add bindings for Renesas RZ/N1 MII converter
    https://git.kernel.org/netdev/net-next/c/c823c2bf9156
  - [net-next,v9,05/16] net: pcs: add Renesas MII converter driver
    https://git.kernel.org/netdev/net-next/c/7dc54d3b8d91
  - [net-next,v9,06/16] dt-bindings: net: dsa: add bindings for Renesas RZ/N1 Advanced 5 port switch
    https://git.kernel.org/netdev/net-next/c/8956e96c1d4d
  - [net-next,v9,07/16] net: dsa: rzn1-a5psw: add Renesas RZ/N1 advanced 5 port switch driver
    https://git.kernel.org/netdev/net-next/c/888cdb892b61
  - [net-next,v9,08/16] net: dsa: rzn1-a5psw: add statistics support
    https://git.kernel.org/netdev/net-next/c/c7243fd4a62f
  - [net-next,v9,09/16] net: dsa: rzn1-a5psw: add FDB support
    https://git.kernel.org/netdev/net-next/c/5edf246c6869
  - [net-next,v9,10/16] dt-bindings: net: snps,dwmac: add "power-domains" property
    https://git.kernel.org/netdev/net-next/c/955fe312a9d2
  - [net-next,v9,11/16] dt-bindings: net: snps,dwmac: add "renesas,rzn1" compatible
    https://git.kernel.org/netdev/net-next/c/d7cc14bc9802
  - [net-next,v9,12/16] ARM: dts: r9a06g032: describe MII converter
    https://git.kernel.org/netdev/net-next/c/066c3bd35835
  - [net-next,v9,13/16] ARM: dts: r9a06g032: describe GMAC2
    https://git.kernel.org/netdev/net-next/c/3f5261f1c2a8
  - [net-next,v9,14/16] ARM: dts: r9a06g032: describe switch
    https://git.kernel.org/netdev/net-next/c/cf9695d8a7e9
  - [net-next,v9,15/16] ARM: dts: r9a06g032-rzn1d400-db: add switch description
    https://git.kernel.org/netdev/net-next/c/9aab31d66ec9
  - [net-next,v9,16/16] MAINTAINERS: add Renesas RZ/N1 switch related driver entry
    https://git.kernel.org/netdev/net-next/c/717a5c56deec

You are awesome, thank you!
Geert Uytterhoeven June 27, 2022, 11:11 a.m. UTC | #11
Hi David,

On Mon, Jun 27, 2022 at 12:50 PM <patchwork-bot+netdevbpf@kernel.org> wrote:
> This series was applied to netdev/net-next.git (master)
> by David S. Miller <davem@davemloft.net>:
>
> On Fri, 24 Jun 2022 16:39:45 +0200 you wrote:
> > The Renesas RZ/N1 SoCs features an ethernet subsystem which contains
> > (most notably) a switch, two GMACs, and a MII converter [1]. This
> > series adds support for the switch and the MII converter.
> >
> > The MII converter present on this SoC has been represented as a PCS
> > which sit between the MACs and the PHY. This PCS driver is probed from
> > the device-tree since it requires to be configured. Indeed the MII
> > converter also contains the registers that are handling the muxing of
> > ports (Switch, MAC, HSR, RTOS, etc) internally to the SoC.
> >
> > [...]
>
> Here is the summary with links:

>   - [net-next,v9,12/16] ARM: dts: r9a06g032: describe MII converter
>     https://git.kernel.org/netdev/net-next/c/066c3bd35835
>   - [net-next,v9,13/16] ARM: dts: r9a06g032: describe GMAC2
>     https://git.kernel.org/netdev/net-next/c/3f5261f1c2a8
>   - [net-next,v9,14/16] ARM: dts: r9a06g032: describe switch
>     https://git.kernel.org/netdev/net-next/c/cf9695d8a7e9
>   - [net-next,v9,15/16] ARM: dts: r9a06g032-rzn1d400-db: add switch description
>     https://git.kernel.org/netdev/net-next/c/9aab31d66ec9

Please do not apply DTS patches to the netdev tree.
These should go in through the platform and soc trees instead.

Thanks for reverting!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Geert Uytterhoeven June 28, 2022, 3:28 p.m. UTC | #12
On Fri, Jun 24, 2022 at 4:41 PM Clément Léger <clement.leger@bootlin.com> wrote:
> Add the MII converter node which describes the MII converter that is
> present on the RZ/N1 SoC.
>
> Signed-off-by: Clément Léger <clement.leger@bootlin.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
i.e. will queue in renesas-devel for v5.20.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Geert Uytterhoeven June 28, 2022, 3:30 p.m. UTC | #13
On Fri, Jun 24, 2022 at 4:42 PM Clément Léger <clement.leger@bootlin.com> wrote:
> RZ/N1 SoC includes two MAC named GMACx that are compatible with the
> "snps,dwmac" driver. GMAC1 is connected directly to the MII converter
> port 1. GMAC2 however can be used as the MAC for the switch CPU
> management port or can be muxed to be connected directly to the MII
> converter port 2. This commit add description for the GMAC2 which will
> be used by the switch description.
>
> Signed-off-by: Clément Léger <clement.leger@bootlin.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
i.e. will queue in renesas-devel for v5.20.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Geert Uytterhoeven June 28, 2022, 3:31 p.m. UTC | #14
On Fri, Jun 24, 2022 at 4:42 PM Clément Léger <clement.leger@bootlin.com> wrote:
> Add description of the switch that is present on the RZ/N1 SoC. This
> description includes ethernet-ports description for all the ports that
> are present on the switch along with their connection to the MII
> converter ports and to the GMAC for the CPU port.
>
> Signed-off-by: Clément Léger <clement.leger@bootlin.com>
> Reviewed-by: Vladimir Oltean <olteanv@gmail.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
i.e. will queue in renesas-devel for v5.20.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Geert Uytterhoeven June 28, 2022, 3:34 p.m. UTC | #15
Hi Clément,

On Fri, Jun 24, 2022 at 4:42 PM Clément Léger <clement.leger@bootlin.com> wrote:
> Add description for the switch, GMAC2 and MII converter. With these
> definitions, the switch port 0 and 1 (MII port 5 and 4) are working on
> RZ/N1D-DB board.
>
> Signed-off-by: Clément Léger <clement.leger@bootlin.com>
> Reviewed-by: Vladimir Oltean <olteanv@gmail.com>

Thanks for your patch!

> --- a/arch/arm/boot/dts/r9a06g032-rzn1d400-db.dts
> +++ b/arch/arm/boot/dts/r9a06g032-rzn1d400-db.dts
> @@ -31,3 +33,118 @@ &wdt0 {
>         timeout-sec = <60>;
>         status = "okay";
>  };
> +
> +&gmac2 {

Please keep the nodes sorted (everywhere).

> +&pinctrl{
> +       pins_mdio1: pins_mdio1 {
> +               pinmux = <
> +                       RZN1_PINMUX(152, RZN1_FUNC_MDIO1_SWITCH)
> +                       RZN1_PINMUX(153, RZN1_FUNC_MDIO1_SWITCH)
> +               >;

This is not a single value, but an array of 2 values.  Hence they
should be grouped using angular brackets, to enable automatic
validation.

I will fix the above while applying, so no need to resend.

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
i.e. will queue in renesas-devel for v5.20.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Clément Léger June 28, 2022, 4:20 p.m. UTC | #16
Le Tue, 28 Jun 2022 17:34:31 +0200,
Geert Uytterhoeven <geert@linux-m68k.org> a écrit :

> > +&gmac2 {  
> 
> Please keep the nodes sorted (everywhere).

Arg sorry, again, the previous nodes seems not to be ordered.
I'll be more careful next time.

> 
> > +&pinctrl{
> > +       pins_mdio1: pins_mdio1 {
> > +               pinmux = <
> > +                       RZN1_PINMUX(152, RZN1_FUNC_MDIO1_SWITCH)
> > +                       RZN1_PINMUX(153, RZN1_FUNC_MDIO1_SWITCH)
> > +               >;  
> 
> This is not a single value, but an array of 2 values.  Hence they
> should be grouped using angular brackets, to enable automatic
> validation.

Good to know, noted for next time.

> 
> I will fix the above while applying, so no need to resend.

Thanks Geert,

Clément

> 
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> i.e. will queue in renesas-devel for v5.20.
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
Russell King (Oracle) June 28, 2022, 4:42 p.m. UTC | #17
On Fri, Jun 24, 2022 at 04:39:50PM +0200, Clément Léger wrote:
> Add a PCS driver for the MII converter that is present on the Renesas
> RZ/N1 SoC. This MII converter is reponsible for converting MII to
> RMII/RGMII or act as a MII pass-trough. Exposing it as a PCS allows to
> reuse it in both the switch driver and the stmmac driver. Currently,
> this driver only allows the PCS to be used by the dual Cortex-A7
> subsystem since the register locking system is not used.
> 
> Signed-off-by: Clément Léger <clement.leger@bootlin.com>
> Reviewed-by: Vladimir Oltean <olteanv@gmail.com>

Looks good to me, thanks.

The only issue I haven't brought up is:

> +static int miic_config(struct phylink_pcs *pcs, unsigned int mode,
> +		       phy_interface_t interface,
> +		       const unsigned long *advertising, bool permit)
> +{
> +	struct miic_port *miic_port = phylink_pcs_to_miic_port(pcs);
> +	struct miic *miic = miic_port->miic;
> +	int port = miic_port->port;
> +	u32 speed, conv_mode, val;
> +
> +	switch (interface) {
> +	case PHY_INTERFACE_MODE_RMII:
> +		conv_mode = CONV_MODE_RMII;
> +		speed = CONV_MODE_100MBPS;
> +		break;
> +	case PHY_INTERFACE_MODE_RGMII:
> +	case PHY_INTERFACE_MODE_RGMII_ID:
> +	case PHY_INTERFACE_MODE_RGMII_TXID:
> +	case PHY_INTERFACE_MODE_RGMII_RXID:
> +		conv_mode = CONV_MODE_RGMII;
> +		speed = CONV_MODE_1000MBPS;
> +		break;
> +	case PHY_INTERFACE_MODE_MII:
> +		conv_mode = CONV_MODE_MII;
> +		/* When in MII mode, speed should be set to 0 (which is actually
> +		 * CONV_MODE_10MBPS)
> +		 */
> +		speed = CONV_MODE_10MBPS;
> +		break;
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +
> +	val = FIELD_PREP(MIIC_CONVCTRL_CONV_MODE, conv_mode) |
> +	      FIELD_PREP(MIIC_CONVCTRL_CONV_SPEED, speed);
> +
> +	miic_reg_rmw(miic, MIIC_CONVCTRL(port),
> +		     MIIC_CONVCTRL_CONV_MODE | MIIC_CONVCTRL_CONV_SPEED, val);
> +	miic_converter_enable(miic_port->miic, miic_port->port, 1);
> +
> +	return 0;
> +}

the stting of the speed here. As this function can be called as a result
of ethtool setting the configuration while the link is up, this could
have disasterous effects on the link. This will only happen if there is
no PHY present and we aren't using fixed-link mode.

Therefore, I'm willing to get this pass, but I think it would be better
if the speed was only updated if the interface setting is actually
being changed. So:

Reviewed-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
Clément Léger June 28, 2022, 4:49 p.m. UTC | #18
Le Tue, 28 Jun 2022 17:42:58 +0100,
"Russell King (Oracle)" <linux@armlinux.org.uk> a écrit :

> > +		break;
> > +	default:
> > +		return -EOPNOTSUPP;
> > +	}
> > +
> > +	val = FIELD_PREP(MIIC_CONVCTRL_CONV_MODE, conv_mode) |
> > +	      FIELD_PREP(MIIC_CONVCTRL_CONV_SPEED, speed);
> > +
> > +	miic_reg_rmw(miic, MIIC_CONVCTRL(port),
> > +		     MIIC_CONVCTRL_CONV_MODE | MIIC_CONVCTRL_CONV_SPEED, val);
> > +	miic_converter_enable(miic_port->miic, miic_port->port, 1);
> > +
> > +	return 0;
> > +}  
> 
> the stting of the speed here. As this function can be called as a result
> of ethtool setting the configuration while the link is up, this could
> have disasterous effects on the link. This will only happen if there is
> no PHY present and we aren't using fixed-link mode.
> 
> Therefore, I'm willing to get this pass, but I think it would be better
> if the speed was only updated if the interface setting is actually
> being changed. So:

Hi Russell,

Ok, I'll make a follow-up patch to handle that properly.