| Message ID | 20220429143505.88208-1-clement.leger@bootlin.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | add support for Renesas RZ/N1 ethernet subsystem devices | expand |
On Fri, 29 Apr 2022 16:34:53 +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. Build's not happy (W=1 C=1): drivers/net/dsa/rzn1_a5psw.c:574:29: warning: symbol 'a5psw_switch_ops' was not declared. Should it be static? In file included from ../drivers/net/dsa/rzn1_a5psw.c:17: drivers/net/dsa/rzn1_a5psw.h:221:1: note: offset of packed bit-field ‘port_mask’ has changed in GCC 4.4 221 | } __packed; | ^ drivers/net/dsa/rzn1_a5psw.h:200: warning: Function parameter or member 'hclk' not described in 'a5psw' drivers/net/dsa/rzn1_a5psw.h:200: warning: Function parameter or member 'clk' not described in 'a5psw' Not sure how many of these are added by you but I think 2 at least.
Le Fri, 29 Apr 2022 12:32:35 -0700, Jakub Kicinski <kuba@kernel.org> a écrit : > On Fri, 29 Apr 2022 16:34:53 +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. > > Build's not happy (W=1 C=1): > > drivers/net/dsa/rzn1_a5psw.c:574:29: warning: symbol 'a5psw_switch_ops' was not declared. Should it be static? > In file included from ../drivers/net/dsa/rzn1_a5psw.c:17: > drivers/net/dsa/rzn1_a5psw.h:221:1: note: offset of packed bit-field ‘port_mask’ has changed in GCC 4.4 > 221 | } __packed; > | ^ > Hi Jakub, I only had this one (due to the lack of W=1 C=1 I guess) which I thought (wrongly) that it was due to my GCC version: CC net/dsa/switch.o CC net/dsa/tag_8021q.o In file included from ../drivers/net/dsa/rzn1_a5psw.c:17: ../drivers/net/dsa/rzn1_a5psw.h:221:1: note: offset of packed bit-field ‘port_mask’ has changed in GCC 4.4 221 | } __packed; | ^ CC kernel/module.o CC drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.o CC drivers/net/ethernet/stmicro/stmmac/dwmac100_core.o I'll fix the other errors which are more trivial, however, I did not found a way to fix the packed bit-field warning. Thanks > drivers/net/dsa/rzn1_a5psw.h:200: warning: Function parameter or member 'hclk' not described in 'a5psw' > drivers/net/dsa/rzn1_a5psw.h:200: warning: Function parameter or member 'clk' not described in 'a5psw' > > Not sure how many of these are added by you but I think 2 at least.
Hi Clément, On Mon, May 2, 2022 at 8:52 AM Clément Léger <clement.leger@bootlin.com> wrote: > Le Fri, 29 Apr 2022 12:32:35 -0700, > Jakub Kicinski <kuba@kernel.org> a écrit : > > > On Fri, 29 Apr 2022 16:34:53 +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. > > > > Build's not happy (W=1 C=1): > > > > drivers/net/dsa/rzn1_a5psw.c:574:29: warning: symbol 'a5psw_switch_ops' was not declared. Should it be static? > > In file included from ../drivers/net/dsa/rzn1_a5psw.c:17: > > drivers/net/dsa/rzn1_a5psw.h:221:1: note: offset of packed bit-field ‘port_mask’ has changed in GCC 4.4 > > 221 | } __packed; > > | ^ > > > > Hi Jakub, I only had this one (due to the lack of W=1 C=1 I guess) which > I thought (wrongly) that it was due to my GCC version: > > CC net/dsa/switch.o > CC net/dsa/tag_8021q.o > In file included from ../drivers/net/dsa/rzn1_a5psw.c:17: > ../drivers/net/dsa/rzn1_a5psw.h:221:1: note: offset of packed bit-field > ‘port_mask’ has changed in GCC 4.4 221 | } __packed; > | ^ > CC kernel/module.o > CC drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.o > CC drivers/net/ethernet/stmicro/stmmac/dwmac100_core.o > > I'll fix the other errors which are more trivial, however, I did not > found a way to fix the packed bit-field warning. The "port_mask" field is split across 2 u8s. What about using u16 instead, and adding explicit padding while at it? The below gets rid of the warning, while the generated code is the same. --- a/drivers/net/dsa/rzn1_a5psw.h +++ b/drivers/net/dsa/rzn1_a5psw.h @@ -169,10 +169,11 @@ struct fdb_entry { u8 mac[ETH_ALEN]; - u8 valid:1; - u8 is_static:1; - u8 prio:3; - u8 port_mask:5; + u16 valid:1; + u16 is_static:1; + u16 prio:3; + u16 port_mask:5; + u16 reserved:6; } __packed; union lk_data { 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
Le Mon, 2 May 2022 14:27:38 +0200, Geert Uytterhoeven <geert@linux-m68k.org> a écrit : > Hi Clément, > > On Mon, May 2, 2022 at 8:52 AM Clément Léger > <clement.leger@bootlin.com> wrote: > > Le Fri, 29 Apr 2022 12:32:35 -0700, > > Jakub Kicinski <kuba@kernel.org> a écrit : > > > > > On Fri, 29 Apr 2022 16:34:53 +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. > > > > > > Build's not happy (W=1 C=1): > > > > > > drivers/net/dsa/rzn1_a5psw.c:574:29: warning: symbol > > > 'a5psw_switch_ops' was not declared. Should it be static? In file > > > included from ../drivers/net/dsa/rzn1_a5psw.c:17: > > > drivers/net/dsa/rzn1_a5psw.h:221:1: note: offset of packed > > > bit-field ‘port_mask’ has changed in GCC 4.4 221 | } __packed; | ^ > > > > > > > Hi Jakub, I only had this one (due to the lack of W=1 C=1 I guess) > > which I thought (wrongly) that it was due to my GCC version: > > > > CC net/dsa/switch.o > > CC net/dsa/tag_8021q.o > > In file included from ../drivers/net/dsa/rzn1_a5psw.c:17: > > ../drivers/net/dsa/rzn1_a5psw.h:221:1: note: offset of packed > > bit-field ‘port_mask’ has changed in GCC 4.4 221 | } __packed; > > | ^ > > CC kernel/module.o > > CC drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.o > > CC drivers/net/ethernet/stmicro/stmmac/dwmac100_core.o > > > > I'll fix the other errors which are more trivial, however, I did not > > found a way to fix the packed bit-field warning. > > The "port_mask" field is split across 2 u8s. > What about using u16 instead, and adding explicit padding while > at it? The below gets rid of the warning, while the generated code > is the same. > > --- a/drivers/net/dsa/rzn1_a5psw.h > +++ b/drivers/net/dsa/rzn1_a5psw.h > @@ -169,10 +169,11 @@ > > struct fdb_entry { > u8 mac[ETH_ALEN]; > - u8 valid:1; > - u8 is_static:1; > - u8 prio:3; > - u8 port_mask:5; > + u16 valid:1; > + u16 is_static:1; > + u16 prio:3; > + u16 port_mask:5; > + u16 reserved:6; > } __packed; Hi Geert ! Indeed, while looking a bit more in depth at this error I found that using u16 avoids this error. I did switch to u16 but did not add any "reserved" field at the end and there is no more error. Do you think adding the "reserved" field would be preferable ? Thanks > > union lk_data { > > 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
Hi Clément, On Mon, May 2, 2022 at 5:10 PM Clément Léger <clement.leger@bootlin.com> wrote: > Le Mon, 2 May 2022 14:27:38 +0200, > Geert Uytterhoeven <geert@linux-m68k.org> a écrit : > > On Mon, May 2, 2022 at 8:52 AM Clément Léger > > <clement.leger@bootlin.com> wrote: > > > Le Fri, 29 Apr 2022 12:32:35 -0700, > > > Jakub Kicinski <kuba@kernel.org> a écrit : > > > > > > > On Fri, 29 Apr 2022 16:34:53 +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. > > > > > > > > Build's not happy (W=1 C=1): > > > > > > > > drivers/net/dsa/rzn1_a5psw.c:574:29: warning: symbol > > > > 'a5psw_switch_ops' was not declared. Should it be static? In file > > > > included from ../drivers/net/dsa/rzn1_a5psw.c:17: > > > > drivers/net/dsa/rzn1_a5psw.h:221:1: note: offset of packed > > > > bit-field ‘port_mask’ has changed in GCC 4.4 221 | } __packed; | ^ > > > > > > > > > > Hi Jakub, I only had this one (due to the lack of W=1 C=1 I guess) > > > which I thought (wrongly) that it was due to my GCC version: > > > > > > CC net/dsa/switch.o > > > CC net/dsa/tag_8021q.o > > > In file included from ../drivers/net/dsa/rzn1_a5psw.c:17: > > > ../drivers/net/dsa/rzn1_a5psw.h:221:1: note: offset of packed > > > bit-field ‘port_mask’ has changed in GCC 4.4 221 | } __packed; > > > | ^ > > > CC kernel/module.o > > > CC drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.o > > > CC drivers/net/ethernet/stmicro/stmmac/dwmac100_core.o > > > > > > I'll fix the other errors which are more trivial, however, I did not > > > found a way to fix the packed bit-field warning. > > > > The "port_mask" field is split across 2 u8s. > > What about using u16 instead, and adding explicit padding while > > at it? The below gets rid of the warning, while the generated code > > is the same. > > > > --- a/drivers/net/dsa/rzn1_a5psw.h > > +++ b/drivers/net/dsa/rzn1_a5psw.h > > @@ -169,10 +169,11 @@ > > > > struct fdb_entry { > > u8 mac[ETH_ALEN]; > > - u8 valid:1; > > - u8 is_static:1; > > - u8 prio:3; > > - u8 port_mask:5; > > + u16 valid:1; > > + u16 is_static:1; > > + u16 prio:3; > > + u16 port_mask:5; > > + u16 reserved:6; > > } __packed; > > Hi Geert ! Indeed, while looking a bit more in depth at this error I > found that using u16 avoids this error. I did switch to u16 but did not > add any "reserved" field at the end and there is no more error. Do you > think adding the "reserved" field would be preferable ? As this structure reflects a hardware definition, I think it is better to make this explicit. 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
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 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 (12): 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 ARM: dts: r9a06g032: describe MII converter ARM: dts: r9a06g032: describe GMAC2 ARM: dts: r9a06g032: describe switch MAINTAINERS: add Renesas RZ/N1 switch related driver entry .../bindings/net/dsa/renesas,rzn1-a5psw.yaml | 132 +++ .../bindings/net/pcs/renesas,rzn1-miic.yaml | 157 +++ MAINTAINERS | 11 + arch/arm/boot/dts/r9a06g032.dtsi | 63 + drivers/net/dsa/Kconfig | 9 + drivers/net/dsa/Makefile | 1 + drivers/net/dsa/rzn1_a5psw.c | 1055 +++++++++++++++++ drivers/net/dsa/rzn1_a5psw.h | 259 ++++ drivers/net/pcs/Kconfig | 7 + drivers/net/pcs/Makefile | 1 + drivers/net/pcs/pcs-rzn1-miic.c | 508 ++++++++ include/dt-bindings/net/pcs-rzn1-miic.h | 33 + include/linux/pcs-rzn1-miic.h | 18 + include/net/dsa.h | 5 + net/dsa/Kconfig | 7 + net/dsa/Makefile | 1 + net/dsa/slave.c | 13 + net/dsa/tag_rzn1_a5psw.c | 114 ++ 18 files changed, 2394 insertions(+) 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