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

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

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

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 <arnd@arndb.de>
---
 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

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

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

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 <arnd@arndb.de>
---
 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

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

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

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 <arnd@arndb.de>
---
 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

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

[Vladimir Oltean]

On Thu, Feb 25, 2021 at 03:38:32PM +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> 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 <arnd@arndb.de>
> ---
>  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

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

[Arnd Bergmann]

On Thu, Feb 25, 2021 at 3:43 PM Vladimir Oltean <olteanv@gmail.com> wrote:
>
> On Thu, Feb 25, 2021 at 03:38:32PM +0100, Arnd Bergmann wrote:
> > From: Arnd Bergmann <arnd@arndb.de>
> >
> > 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 <arnd@arndb.de>
> > ---
> >  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

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

[Arnd Bergmann]

On Thu, Feb 25, 2021 at 3:47 PM Arnd Bergmann <arnd@kernel.org> wrote:
> On Thu, Feb 25, 2021 at 3:43 PM Vladimir Oltean <olteanv@gmail.com> wrote:
> > On Thu, Feb 25, 2021 at 03:38:32PM +0100, Arnd Bergmann wrote:
> > > From: Arnd Bergmann <arnd@arndb.de>
> > >
> > > 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

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

[Vladimir Oltean]

On Thu, Feb 25, 2021 at 03:49:08PM +0100, Arnd Bergmann wrote:
> On Thu, Feb 25, 2021 at 3:47 PM Arnd Bergmann <arnd@kernel.org> wrote:
> > On Thu, Feb 25, 2021 at 3:43 PM Vladimir Oltean <olteanv@gmail.com> wrote:
> > > On Thu, Feb 25, 2021 at 03:38:32PM +0100, Arnd Bergmann wrote:
> > > > From: Arnd Bergmann <arnd@arndb.de>
> > > >
> > > > 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.

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

[Arnd Bergmann]

On Thu, Feb 25, 2021 at 4:07 PM Vladimir Oltean <olteanv@gmail.com> 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 <arnd@kernel.org> wrote:
> > > On Thu, Feb 25, 2021 at 3:43 PM Vladimir Oltean <olteanv@gmail.com> wrote:
> > > > On Thu, Feb 25, 2021 at 03:38:32PM +0100, Arnd Bergmann wrote:
> > > > > From: Arnd Bergmann <arnd@arndb.de>
> > > > >
> > > > > 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.

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

[DENG Qingfang]

Hi Arnd,

On Thu, Feb 25, 2021 at 10:40 PM Arnd Bergmann <arnd@kernel.org> wrote:
>
> From: Arnd Bergmann <arnd@arndb.de>
>
> 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 <arnd@arndb.de>

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

[Arnd Bergmann]

On Thu, Feb 25, 2021 at 4:52 PM DENG Qingfang <dqfext@gmail.com> wrote:
>
> Hi Arnd,
>
> On Thu, Feb 25, 2021 at 10:40 PM Arnd Bergmann <arnd@kernel.org> wrote:
> >
> > From: Arnd Bergmann <arnd@arndb.de>
> >
> > 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 <arnd@arndb.de>

      Arnd

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

[Vladimir Oltean]

On Thu, Feb 25, 2021 at 03:38:32PM +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> 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 <arnd@arndb.de>
> ---

Acked-by: Vladimir Oltean <vladimir.oltean@nxp.com>

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

[Jakub Kicinski]

On Thu, 25 Feb 2021 15:38:31 +0100 Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> 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 <arnd@arndb.de>

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