diff mbox series

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

Message ID 20210225143910.3964364-2-arnd@kernel.org (mailing list archive)
State Accepted
Delegated to: Netdev Maintainers
Headers show
Series [1/3] net: mscc: ocelot: select NET_DEVLINK | expand

Checks

Context Check Description
netdev/cover_letter warning Series does not have a cover letter
netdev/fixes_present success Link
netdev/patch_count success Link
netdev/tree_selection success Guessed tree name to be net-next
netdev/subject_prefix warning Target tree name not specified in the subject
netdev/cc_maintainers success CCed 7 of 7 maintainers
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success Link
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 8 lines checked
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/header_inline success Link
netdev/stable success Stable not CCed

Commit Message

Arnd Bergmann Feb. 25, 2021, 2:38 p.m. UTC 
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(+)

Comments

Vladimir Oltean Feb. 25, 2021, 2:43 p.m. UTC | #1 
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
Arnd Bergmann Feb. 25, 2021, 2:47 p.m. UTC | #2 
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
Arnd Bergmann Feb. 25, 2021, 2:49 p.m. UTC | #3 
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
Vladimir Oltean Feb. 25, 2021, 3:07 p.m. UTC | #4 
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.
Arnd Bergmann Feb. 25, 2021, 3:43 p.m. UTC | #5 
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.
Vladimir Oltean Feb. 26, 2021, 9:31 p.m. UTC | #6 
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>
diff mbox series

Patch

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