2021-02-25 14:41:42

by Arnd Bergmann

[permalink] [raw]
Subject: [PATCH 1/3] net: mscc: ocelot: select NET_DEVLINK

From: Arnd Bergmann <[email protected]>

Without this option, the driver fails to link:

ld.lld: error: undefined symbol: devlink_sb_register
>>> referenced by ocelot_devlink.c
>>> net/ethernet/mscc/ocelot_devlink.o:(ocelot_devlink_sb_register) in archive drivers/built-in.a
>>> referenced by ocelot_devlink.c
>>> net/ethernet/mscc/ocelot_devlink.o:(ocelot_devlink_sb_register) in archive drivers/built-in.a

Fixes: f59fd9cab730 ("net: mscc: ocelot: configure watermarks using devlink-sb")
Signed-off-by: Arnd Bergmann <[email protected]>
---
drivers/net/ethernet/mscc/Kconfig | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/mscc/Kconfig b/drivers/net/ethernet/mscc/Kconfig
index c0ede0ca7115..05cb040c2677 100644
--- a/drivers/net/ethernet/mscc/Kconfig
+++ b/drivers/net/ethernet/mscc/Kconfig
@@ -13,6 +13,7 @@ if NET_VENDOR_MICROSEMI

# Users should depend on NET_SWITCHDEV, HAS_IOMEM
config MSCC_OCELOT_SWITCH_LIB
+ select NET_DEVLINK
select REGMAP_MMIO
select PACKING
select PHYLIB
--
2.29.2


2021-02-25 14:43:11

by Arnd Bergmann

[permalink] [raw]
Subject: [PATCH 2/3] net: dsa: tag_ocelot_8021q: fix driver dependency

From: Arnd Bergmann <[email protected]>

When the ocelot driver code is in a library, the dsa tag
code cannot be built-in:

ld.lld: error: undefined symbol: ocelot_can_inject
>>> referenced by tag_ocelot_8021q.c
>>> dsa/tag_ocelot_8021q.o:(ocelot_xmit) in archive net/built-in.a

ld.lld: error: undefined symbol: ocelot_port_inject_frame
>>> referenced by tag_ocelot_8021q.c
>>> dsa/tag_ocelot_8021q.o:(ocelot_xmit) in archive net/built-in.a

Building the tag support only really makes sense for compile-testing
when the driver is available, so add a Kconfig dependency that prevents
the broken configuration while allowing COMPILE_TEST alternative when
MSCC_OCELOT_SWITCH_LIB is disabled entirely. This case is handled
through the #ifdef check in include/soc/mscc/ocelot.h.

Fixes: 0a6f17c6ae21 ("net: dsa: tag_ocelot_8021q: add support for PTP timestamping")
Signed-off-by: Arnd Bergmann <[email protected]>
---
net/dsa/Kconfig | 2 ++
1 file changed, 2 insertions(+)

diff --git a/net/dsa/Kconfig b/net/dsa/Kconfig
index 3589224c8da9..58b8fc82cd3c 100644
--- a/net/dsa/Kconfig
+++ b/net/dsa/Kconfig
@@ -118,6 +118,8 @@ config NET_DSA_TAG_OCELOT

config NET_DSA_TAG_OCELOT_8021Q
tristate "Tag driver for Ocelot family of switches, using VLAN"
+ depends on MSCC_OCELOT_SWITCH_LIB || \
+ (MSCC_OCELOT_SWITCH_LIB=n && COMPILE_TEST)
select NET_DSA_TAG_8021Q
help
Say Y or M if you want to enable support for tagging frames with a
--
2.29.2

2021-02-25 14:44:45

by Arnd Bergmann

[permalink] [raw]
Subject: [PATCH 3/3] net: dsa: mt7530: add GPIOLIB dependency

From: Arnd Bergmann <[email protected]>

The new gpio support may be optional at runtime, but it requires
building against gpiolib:

ERROR: modpost: "gpiochip_get_data" [drivers/net/dsa/mt7530.ko] undefined!
ERROR: modpost: "devm_gpiochip_add_data_with_key" [drivers/net/dsa/mt7530.ko] undefined!

Add a Kconfig dependency to enforce this.

Fixes: 429a0edeefd8 ("net: dsa: mt7530: MT7530 optional GPIO support")
Signed-off-by: Arnd Bergmann <[email protected]>
---
drivers/net/dsa/Kconfig | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/net/dsa/Kconfig b/drivers/net/dsa/Kconfig
index 3af373e90806..07fc2a732597 100644
--- a/drivers/net/dsa/Kconfig
+++ b/drivers/net/dsa/Kconfig
@@ -37,6 +37,7 @@ config NET_DSA_LANTIQ_GSWIP
config NET_DSA_MT7530
tristate "MediaTek MT753x and MT7621 Ethernet switch support"
depends on NET_DSA
+ depends on GPIOLIB
select NET_DSA_TAG_MTK
help
This enables support for the MediaTek MT7530, MT7531, and MT7621
--
2.29.2

2021-02-25 14:46:02

by Vladimir Oltean

[permalink] [raw]
Subject: Re: [PATCH 2/3] net: dsa: tag_ocelot_8021q: fix driver dependency

On Thu, Feb 25, 2021 at 03:38:32PM +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann <[email protected]>
>
> When the ocelot driver code is in a library, the dsa tag
> code cannot be built-in:
>
> ld.lld: error: undefined symbol: ocelot_can_inject
> >>> referenced by tag_ocelot_8021q.c
> >>> dsa/tag_ocelot_8021q.o:(ocelot_xmit) in archive net/built-in.a
>
> ld.lld: error: undefined symbol: ocelot_port_inject_frame
> >>> referenced by tag_ocelot_8021q.c
> >>> dsa/tag_ocelot_8021q.o:(ocelot_xmit) in archive net/built-in.a
>
> Building the tag support only really makes sense for compile-testing
> when the driver is available, so add a Kconfig dependency that prevents
> the broken configuration while allowing COMPILE_TEST alternative when
> MSCC_OCELOT_SWITCH_LIB is disabled entirely. This case is handled
> through the #ifdef check in include/soc/mscc/ocelot.h.
>
> Fixes: 0a6f17c6ae21 ("net: dsa: tag_ocelot_8021q: add support for PTP timestamping")
> Signed-off-by: Arnd Bergmann <[email protected]>
> ---
> net/dsa/Kconfig | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/net/dsa/Kconfig b/net/dsa/Kconfig
> index 3589224c8da9..58b8fc82cd3c 100644
> --- a/net/dsa/Kconfig
> +++ b/net/dsa/Kconfig
> @@ -118,6 +118,8 @@ config NET_DSA_TAG_OCELOT
>
> config NET_DSA_TAG_OCELOT_8021Q
> tristate "Tag driver for Ocelot family of switches, using VLAN"
> + depends on MSCC_OCELOT_SWITCH_LIB || \
> + (MSCC_OCELOT_SWITCH_LIB=n && COMPILE_TEST)
> select NET_DSA_TAG_8021Q
> help
> Say Y or M if you want to enable support for tagging frames with a
> --
> 2.29.2
>

Why isn't this code in include/soc/mscc/ocelot.h enough?

#if IS_ENABLED(CONFIG_MSCC_OCELOT_SWITCH_LIB)

bool ocelot_can_inject(struct ocelot *ocelot, int grp);
void ocelot_port_inject_frame(struct ocelot *ocelot, int port, int grp,
u32 rew_op, struct sk_buff *skb);
int ocelot_xtr_poll_frame(struct ocelot *ocelot, int grp, struct sk_buff **skb);
void ocelot_drain_cpu_queue(struct ocelot *ocelot, int grp);

#else

static inline bool ocelot_can_inject(struct ocelot *ocelot, int grp)
{
return false;
}

static inline void ocelot_port_inject_frame(struct ocelot *ocelot, int port,
int grp, u32 rew_op,
struct sk_buff *skb)
{
}

static inline int ocelot_xtr_poll_frame(struct ocelot *ocelot, int grp,
struct sk_buff **skb)
{
return -EIO;
}

static inline void ocelot_drain_cpu_queue(struct ocelot *ocelot, int grp)
{
}

#endif

2021-02-25 14:50:59

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 2/3] net: dsa: tag_ocelot_8021q: fix driver dependency

On Thu, Feb 25, 2021 at 3:43 PM Vladimir Oltean <[email protected]> wrote:
>
> On Thu, Feb 25, 2021 at 03:38:32PM +0100, Arnd Bergmann wrote:
> > From: Arnd Bergmann <[email protected]>
> >
> > When the ocelot driver code is in a library, the dsa tag
> > code cannot be built-in:
> >
> > ld.lld: error: undefined symbol: ocelot_can_inject
> > >>> referenced by tag_ocelot_8021q.c
> > >>> dsa/tag_ocelot_8021q.o:(ocelot_xmit) in archive net/built-in.a
> >
> > ld.lld: error: undefined symbol: ocelot_port_inject_frame
> > >>> referenced by tag_ocelot_8021q.c
> > >>> dsa/tag_ocelot_8021q.o:(ocelot_xmit) in archive net/built-in.a
> >
> > Building the tag support only really makes sense for compile-testing
> > when the driver is available, so add a Kconfig dependency that prevents
> > the broken configuration while allowing COMPILE_TEST alternative when
> > MSCC_OCELOT_SWITCH_LIB is disabled entirely. This case is handled
> > through the #ifdef check in include/soc/mscc/ocelot.h.
> >
> > Fixes: 0a6f17c6ae21 ("net: dsa: tag_ocelot_8021q: add support for PTP timestamping")
> > Signed-off-by: Arnd Bergmann <[email protected]>
> > ---
> > net/dsa/Kconfig | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/net/dsa/Kconfig b/net/dsa/Kconfig
> > index 3589224c8da9..58b8fc82cd3c 100644
> > --- a/net/dsa/Kconfig
> > +++ b/net/dsa/Kconfig
> > @@ -118,6 +118,8 @@ config NET_DSA_TAG_OCELOT
> >
> > config NET_DSA_TAG_OCELOT_8021Q
> > tristate "Tag driver for Ocelot family of switches, using VLAN"
> > + depends on MSCC_OCELOT_SWITCH_LIB || \
> > + (MSCC_OCELOT_SWITCH_LIB=n && COMPILE_TEST)
> > select NET_DSA_TAG_8021Q
> > help
> > Say Y or M if you want to enable support for tagging frames with a
> > --
> > 2.29.2
> >
>
> Why isn't this code in include/soc/mscc/ocelot.h enough?
>
> #if IS_ENABLED(CONFIG_MSCC_OCELOT_SWITCH_LIB)
>
> bool ocelot_can_inject(struct ocelot *ocelot, int grp);
> void ocelot_port_inject_frame(struct ocelot *ocelot, int port, int grp,
> u32 rew_op, struct sk_buff *skb);
> int ocelot_xtr_poll_frame(struct ocelot *ocelot, int grp, struct sk_buff **skb);
> void ocelot_drain_cpu_queue(struct ocelot *ocelot, int grp);
>
> #else
>
> static inline bool ocelot_can_inject(struct ocelot *ocelot, int grp)
> {
> return false;
> }

That code is in include/soc/mscc/ocelot.h, it is what causes the
problem with CONFIG_MSCC_OCELOT_SWITCH_LIB=m
and NET_DSA_TAG_OCELOT_8021Q=y, as I tried to explain.

Arnd

2021-02-25 14:51:55

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 2/3] net: dsa: tag_ocelot_8021q: fix driver dependency

On Thu, Feb 25, 2021 at 3:47 PM Arnd Bergmann <[email protected]> wrote:
> On Thu, Feb 25, 2021 at 3:43 PM Vladimir Oltean <[email protected]> wrote:
> > On Thu, Feb 25, 2021 at 03:38:32PM +0100, Arnd Bergmann wrote:
> > > From: Arnd Bergmann <[email protected]>
> > >
> > > When the ocelot driver code is in a library, the dsa tag

I see the problem now, I should have written 'loadable module', not 'library'.
Let me know if I should resend with a fixed changelog text.

Arnd

2021-02-25 15:09:52

by Vladimir Oltean

[permalink] [raw]
Subject: Re: [PATCH 2/3] net: dsa: tag_ocelot_8021q: fix driver dependency

On Thu, Feb 25, 2021 at 03:49:08PM +0100, Arnd Bergmann wrote:
> On Thu, Feb 25, 2021 at 3:47 PM Arnd Bergmann <[email protected]> wrote:
> > On Thu, Feb 25, 2021 at 3:43 PM Vladimir Oltean <[email protected]> wrote:
> > > On Thu, Feb 25, 2021 at 03:38:32PM +0100, Arnd Bergmann wrote:
> > > > From: Arnd Bergmann <[email protected]>
> > > >
> > > > When the ocelot driver code is in a library, the dsa tag
>
> I see the problem now, I should have written 'loadable module', not 'library'.
> Let me know if I should resend with a fixed changelog text.

Ah, ok, things clicked into place now that you said 'module'.
So basically, your patch is the standard Kconfig incantation for 'if the
ocelot switch lib is built as module, build the tagger as module too',
plus some extra handling to allow NET_DSA_TAG_OCELOT_8021Q to still be y
or m when COMPILE_TEST is enabled, but it will be compiled in a
reduced-functionality mode, without MSCC_OCELOT_SWITCH_LIB, therefore
without PTP.

Do I get things right? Sorry, Kconfig is a very strange language.

2021-02-25 15:47:31

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 2/3] net: dsa: tag_ocelot_8021q: fix driver dependency

On Thu, Feb 25, 2021 at 4:07 PM Vladimir Oltean <[email protected]> wrote:
> On Thu, Feb 25, 2021 at 03:49:08PM +0100, Arnd Bergmann wrote:
> > On Thu, Feb 25, 2021 at 3:47 PM Arnd Bergmann <[email protected]> wrote:
> > > On Thu, Feb 25, 2021 at 3:43 PM Vladimir Oltean <[email protected]> wrote:
> > > > On Thu, Feb 25, 2021 at 03:38:32PM +0100, Arnd Bergmann wrote:
> > > > > From: Arnd Bergmann <[email protected]>
> > > > >
> > > > > When the ocelot driver code is in a library, the dsa tag
> >
> > I see the problem now, I should have written 'loadable module', not 'library'.
> > Let me know if I should resend with a fixed changelog text.
>
> Ah, ok, things clicked into place now that you said 'module'.
> So basically, your patch is the standard Kconfig incantation for 'if the
> ocelot switch lib is built as module, build the tagger as module too',
> plus some extra handling to allow NET_DSA_TAG_OCELOT_8021Q to still be y
> or m when COMPILE_TEST is enabled, but it will be compiled in a
> reduced-functionality mode, without MSCC_OCELOT_SWITCH_LIB, therefore
> without PTP.
>
> Do I get things right? Sorry, Kconfig is a very strange language.

Yes, that's basically correct. I tried to express it in Kconfig the way
I would explain it in English, which means it there are two options:

a) If MSCC_OCELOT_SWITCH_LIB is enabled (y or m) there is
a direct dependency, so NET_DSA_TAG_OCELOT_8021Q cannot
be built-in if MSCC_OCELOT_SWITCH_LIB=m
b) When compile-testing *and* MSCC_OCELOT_SWITCH_LIB is fully
disabled, NET_DSA_TAG_OCELOT_8021Q can be anything (y/m/n)

As a side-effect, this also means that if we are not compile-testing
and MSCC_OCELOT_SWITCH_LIB is disabled, the option is
hdden.

Arnd.

2021-02-25 15:55:20

by Qingfang Deng

[permalink] [raw]
Subject: Re: [PATCH 3/3] net: dsa: mt7530: add GPIOLIB dependency

Hi Arnd,

On Thu, Feb 25, 2021 at 10:40 PM Arnd Bergmann <[email protected]> wrote:
>
> From: Arnd Bergmann <[email protected]>
>
> The new gpio support may be optional at runtime, but it requires
> building against gpiolib:
>
> ERROR: modpost: "gpiochip_get_data" [drivers/net/dsa/mt7530.ko] undefined!
> ERROR: modpost: "devm_gpiochip_add_data_with_key" [drivers/net/dsa/mt7530.ko] undefined!
>
> Add a Kconfig dependency to enforce this.

I think wrapping the GPIO code block with #ifdef CONFIG_GPIOLIB ...
#endif may be a better idea.

>
> Fixes: 429a0edeefd8 ("net: dsa: mt7530: MT7530 optional GPIO support")
> Signed-off-by: Arnd Bergmann <[email protected]>

2021-02-25 16:21:39

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 3/3] net: dsa: mt7530: add GPIOLIB dependency

On Thu, Feb 25, 2021 at 4:52 PM DENG Qingfang <[email protected]> wrote:
>
> Hi Arnd,
>
> On Thu, Feb 25, 2021 at 10:40 PM Arnd Bergmann <[email protected]> wrote:
> >
> > From: Arnd Bergmann <[email protected]>
> >
> > The new gpio support may be optional at runtime, but it requires
> > building against gpiolib:
> >
> > ERROR: modpost: "gpiochip_get_data" [drivers/net/dsa/mt7530.ko] undefined!
> > ERROR: modpost: "devm_gpiochip_add_data_with_key" [drivers/net/dsa/mt7530.ko] undefined!
> >
> > Add a Kconfig dependency to enforce this.
>
> I think wrapping the GPIO code block with #ifdef CONFIG_GPIOLIB ...
> #endif may be a better idea.

In practice there is little difference, as most configurations have GPIOLIB
enabled anyway, in particular every configuration in which this driver is
used.

If you want to send an alternative patch to add the #ifdef, please include

Reported-by: Arnd Bergmann <[email protected]>

Arnd

2021-02-26 21:35:14

by Vladimir Oltean

[permalink] [raw]
Subject: Re: [PATCH 2/3] net: dsa: tag_ocelot_8021q: fix driver dependency

On Thu, Feb 25, 2021 at 03:38:32PM +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann <[email protected]>
>
> When the ocelot driver code is in a library, the dsa tag
> code cannot be built-in:
>
> ld.lld: error: undefined symbol: ocelot_can_inject
> >>> referenced by tag_ocelot_8021q.c
> >>> dsa/tag_ocelot_8021q.o:(ocelot_xmit) in archive net/built-in.a
>
> ld.lld: error: undefined symbol: ocelot_port_inject_frame
> >>> referenced by tag_ocelot_8021q.c
> >>> dsa/tag_ocelot_8021q.o:(ocelot_xmit) in archive net/built-in.a
>
> Building the tag support only really makes sense for compile-testing
> when the driver is available, so add a Kconfig dependency that prevents
> the broken configuration while allowing COMPILE_TEST alternative when
> MSCC_OCELOT_SWITCH_LIB is disabled entirely. This case is handled
> through the #ifdef check in include/soc/mscc/ocelot.h.
>
> Fixes: 0a6f17c6ae21 ("net: dsa: tag_ocelot_8021q: add support for PTP timestamping")
> Signed-off-by: Arnd Bergmann <[email protected]>
> ---

Acked-by: Vladimir Oltean <[email protected]>

2021-02-26 23:38:35

by Jakub Kicinski

[permalink] [raw]
Subject: Re: [PATCH 1/3] net: mscc: ocelot: select NET_DEVLINK

On Thu, 25 Feb 2021 15:38:31 +0100 Arnd Bergmann wrote:
> From: Arnd Bergmann <[email protected]>
>
> Without this option, the driver fails to link:
>
> ld.lld: error: undefined symbol: devlink_sb_register
> [...]
>
> Fixes: f59fd9cab730 ("net: mscc: ocelot: configure watermarks using devlink-sb")
> Signed-off-by: Arnd Bergmann <[email protected]>

Applied patch 1 and 2, I'll take Qingfang's patch for mt7530, thanks!