Re: [net-next v2 00/12] add support for Renesas RZ/N1 ethernet subsystem devices

From: Geert Uytterhoeven
Date: Mon May 02 2022 - 08:28:04 EST


Hi Clément,

On Mon, May 2, 2022 at 8:52 AM Clément Léger <clement.leger@xxxxxxxxxxx> wrote:
> Le Fri, 29 Apr 2022 12:32:35 -0700,
> Jakub Kicinski <kuba@xxxxxxxxxx> 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@xxxxxxxxxxxxxx

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