[PATCH net-next] dsa: sja1105: fix reverse dependency

Subsystems: networking [dsa], networking [general], the rest

STALE1833d

7 messages, 2 authors, 2021-08-05 · open the first message on its own page

[PATCH net-next] dsa: sja1105: fix reverse dependency

From: Arnd Bergmann <arnd@kernel.org>
Date: 2021-08-05 11:00:58

From: Arnd Bergmann <arnd@arndb.de>

The DSA driver and the tag driver for sja1105 are closely linked,
and recently the dependency started becoming visible in the form
of the sja1110_process_meta_tstamp() that gets exported by one
and used by the other.

This causes a rare build failure with CONFIG_NET_DSA_TAG_SJA1105=y
and CONFIG_NET_DSA_SJA1105=m, as the 'select' statement only
prevents the opposite configuration:

aarch64-linux-ld: net/dsa/tag_sja1105.o: in function `sja1110_rcv':
tag_sja1105.c:(.text.sja1110_rcv+0x164): undefined reference to `sja1110_process_meta_tstamp'

Add a stricter dependency for the CONFIG_NET_DSA_TAG_SJA110y to
prevent it from being built-in when the other one is not.

Fixes: 566b18c8b752 ("net: dsa: sja1105: implement TX timestamping for SJA1110")
Fixes: 227d07a07ef1 ("net: dsa: sja1105: Add support for traffic through standalone ports")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
Not sure if there is a more logical way to deal with this,
but the added dependency does help avoid the build failure.

I found this one while verifying the PTP dependency patch, but
it's really a separate issue.
---
 net/dsa/Kconfig | 1 +
 1 file changed, 1 insertion(+)
diff --git a/net/dsa/Kconfig b/net/dsa/Kconfig
index bca1b5d66df2..548285539752 100644
--- a/net/dsa/Kconfig
+++ b/net/dsa/Kconfig
@@ -138,6 +138,7 @@ config NET_DSA_TAG_LAN9303
 
 config NET_DSA_TAG_SJA1105
 	tristate "Tag driver for NXP SJA1105 switches"
+	depends on NET_DSA_SJA1105 || !NET_DSA_SJA1105
 	select PACKING
 	help
 	  Say Y or M if you want to enable support for tagging frames with the
-- 
2.29.2

Re: [PATCH net-next] dsa: sja1105: fix reverse dependency

From: Vladimir Oltean <olteanv@gmail.com>
Date: 2021-08-05 11:25:58

Hi Arnd,

On Thu, Aug 05, 2021 at 01:00:28PM +0200, Arnd Bergmann wrote:
From: Arnd Bergmann <arnd@arndb.de>

The DSA driver and the tag driver for sja1105 are closely linked,
and recently the dependency started becoming visible in the form
of the sja1110_process_meta_tstamp() that gets exported by one
and used by the other.

This causes a rare build failure with CONFIG_NET_DSA_TAG_SJA1105=y
and CONFIG_NET_DSA_SJA1105=m, as the 'select' statement only
prevents the opposite configuration:

aarch64-linux-ld: net/dsa/tag_sja1105.o: in function `sja1110_rcv':
tag_sja1105.c:(.text.sja1110_rcv+0x164): undefined reference to `sja1110_process_meta_tstamp'

Add a stricter dependency for the CONFIG_NET_DSA_TAG_SJA110y to
prevent it from being built-in when the other one is not.

Fixes: 566b18c8b752 ("net: dsa: sja1105: implement TX timestamping for SJA1110")
Fixes: 227d07a07ef1 ("net: dsa: sja1105: Add support for traffic through standalone ports")
The second Fixes: tag makes no sense.
quoted hunk
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
Not sure if there is a more logical way to deal with this,
but the added dependency does help avoid the build failure.

I found this one while verifying the PTP dependency patch, but
it's really a separate issue.
---
 net/dsa/Kconfig | 1 +
 1 file changed, 1 insertion(+)
diff --git a/net/dsa/Kconfig b/net/dsa/Kconfig
index bca1b5d66df2..548285539752 100644
--- a/net/dsa/Kconfig
+++ b/net/dsa/Kconfig
@@ -138,6 +138,7 @@ config NET_DSA_TAG_LAN9303

 config NET_DSA_TAG_SJA1105
 	tristate "Tag driver for NXP SJA1105 switches"
+	depends on NET_DSA_SJA1105 || !NET_DSA_SJA1105
I think I would prefer an optional "build as module if NET_DSA_SJA1105 is a module"
dependency only if NET_DSA_SJA1105_PTP is enabled. I think this is how that is
expressed:

	depends on (NET_DSA_SJA1105 && NET_DSA_SJA1105_PTP) || !NET_DSA_SJA1105 || !NET_DSA_SJA1105_PTP
 	select PACKING
 	help
 	  Say Y or M if you want to enable support for tagging frames with the
--
2.29.2

Re: [PATCH net-next] dsa: sja1105: fix reverse dependency

From: Arnd Bergmann <arnd@kernel.org>
Date: 2021-08-05 11:39:55

On Thu, Aug 5, 2021 at 1:25 PM Vladimir Oltean [off-list ref] wrote:
On Thu, Aug 05, 2021 at 01:00:28PM +0200, Arnd Bergmann wrote:
quoted
Fixes: 566b18c8b752 ("net: dsa: sja1105: implement TX timestamping for SJA1110")
Fixes: 227d07a07ef1 ("net: dsa: sja1105: Add support for traffic through standalone ports")
The second Fixes: tag makes no sense.
Fair enough. I added this because that was when the original 'select' got added,
but of course it was not wrong at the time.
quoted
diff --git a/net/dsa/Kconfig b/net/dsa/Kconfig
index bca1b5d66df2..548285539752 100644
--- a/net/dsa/Kconfig
+++ b/net/dsa/Kconfig
@@ -138,6 +138,7 @@ config NET_DSA_TAG_LAN9303

 config NET_DSA_TAG_SJA1105
      tristate "Tag driver for NXP SJA1105 switches"
+     depends on NET_DSA_SJA1105 || !NET_DSA_SJA1105
I think I would prefer an optional "build as module if NET_DSA_SJA1105 is a module"
dependency only if NET_DSA_SJA1105_PTP is enabled. I think this is how that is
expressed:

        depends on (NET_DSA_SJA1105 && NET_DSA_SJA1105_PTP) || !NET_DSA_SJA1105 || !NET_DSA_SJA1105_PTP
Ah, I had not realized this dependency is only there when NET_DSA_SJA1105_PTP
is also enabled. I will give this a little more testing and resend
later with that change.

Do you have any opinion on whether that 'select' going the other way is still
relevant?

      Arnd

Re: [PATCH net-next] dsa: sja1105: fix reverse dependency

From: Vladimir Oltean <olteanv@gmail.com>
Date: 2021-08-05 11:49:53

On Thu, Aug 05, 2021 at 01:39:34PM +0200, Arnd Bergmann wrote:
Do you have any opinion on whether that 'select' going the other way is still
relevant?
Yes, of course it is. It also has nothing to do with build dependencies.
With the original DSA design from 2008, an Ethernet switch has separate
drivers for
(a) accessing its registers
(b) manipulating the packets that the switch sends towards a host
    Ethernet controller ("DSA master")

The register access drivers are in drivers/net/dsa/*, the packet
manipulation ("tagging protocol") drivers are in net/dsa/tag_*.c.

[ This is because it was originally thought that a "tagging protocol" is
  completely stateless and you should never need to access a hardware
  register when manipulating a packet. ]

When you enable a driver for a switch, you absolutely want to ping
through it too, so all register access drivers enable the tagging
protocol driver specific to their hardware as well, using 'select'.
This works just fine because tagging protocol drivers generally have no
dependencies, or if they do, the register access driver inherits them too.
So a user does not need to manually enable the tagging protocol driver.

Re: [PATCH net-next] dsa: sja1105: fix reverse dependency

From: Arnd Bergmann <arnd@kernel.org>
Date: 2021-08-05 12:05:31

On Thu, Aug 5, 2021 at 1:49 PM Vladimir Oltean [off-list ref] wrote:
On Thu, Aug 05, 2021 at 01:39:34PM +0200, Arnd Bergmann wrote:
quoted
Do you have any opinion on whether that 'select' going the other way is still
relevant?
Yes, of course it is. It also has nothing to do with build dependencies.
With the original DSA design from 2008, an Ethernet switch has separate
drivers for
(a) accessing its registers
(b) manipulating the packets that the switch sends towards a host
    Ethernet controller ("DSA master")

The register access drivers are in drivers/net/dsa/*, the packet
manipulation ("tagging protocol") drivers are in net/dsa/tag_*.c.

[ This is because it was originally thought that a "tagging protocol" is
  completely stateless and you should never need to access a hardware
  register when manipulating a packet. ]

When you enable a driver for a switch, you absolutely want to ping
through it too, so all register access drivers enable the tagging
protocol driver specific to their hardware as well, using 'select'.
This works just fine because tagging protocol drivers generally have no
dependencies, or if they do, the register access driver inherits them too.
So a user does not need to manually enable the tagging protocol driver.
Got it, thanks for the explanation.

      Arnd

Re: [PATCH net-next] dsa: sja1105: fix reverse dependency

From: Vladimir Oltean <olteanv@gmail.com>
Date: 2021-08-05 12:18:01

On Thu, Aug 05, 2021 at 01:39:34PM +0200, Arnd Bergmann wrote:
I will give this a little more testing and resend
later with that change.
Btw, not sure if you noticed but I did send that out already:
https://patchwork.kernel.org/project/netdevbpf/patch/20210805113612.2174148-1-vladimir.oltean@nxp.com/

Re: [PATCH net-next] dsa: sja1105: fix reverse dependency

From: Arnd Bergmann <arnd@kernel.org>
Date: 2021-08-05 12:22:24

On Thu, Aug 5, 2021 at 2:17 PM Vladimir Oltean [off-list ref] wrote:
On Thu, Aug 05, 2021 at 01:39:34PM +0200, Arnd Bergmann wrote:
quoted
I will give this a little more testing and resend
later with that change.
Btw, not sure if you noticed but I did send that out already:
https://patchwork.kernel.org/project/netdevbpf/patch/20210805113612.2174148-1-vladimir.oltean@nxp.com/
Yes, saw it, but only after my reply. I don't expect any problems with it, but
it's in my test tree now and I'll let you know if something comes up.

      Arnd
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help