Hi Clément,

On Mon, May 2, 2022 at 5:10 PM Clément Léger [off-list ref] wrote:

Le Mon, 2 May 2022 14:27:38 +0200,
Geert Uytterhoeven [off-list ref] a écrit :

quoted

On Mon, May 2, 2022 at 8:52 AM Clément Léger
[off-list ref] wrote:

quoted

Le Fri, 29 Apr 2022 12:32:35 -0700,
Jakub Kicinski [off-list ref] a écrit :

quoted

On Fri, 29 Apr 2022 16:34:53 +0200 Clément Léger wrote:

quoted

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