2021-12-04 17:38:59

by Arnd Bergmann

[permalink] [raw]
Subject: [PATCH 1/3] iwlwifi: fix LED dependencies

From: Arnd Bergmann <[email protected]>

The dependencies for LED configuration are highly inconsistent and too
complicated at the moment. One of the results is a randconfig failure I
get very rarely when LEDS_CLASS is in a loadable module, but the wireless
core is built-in:

WARNING: unmet direct dependencies detected for MAC80211_LEDS
Depends on [n]: NET [=y] && WIRELESS [=y] && MAC80211 [=y] && (LEDS_CLASS [=m]=y || LEDS_CLASS [=m]=MAC80211 [=y])
Selected by [m]:
- IWLEGACY [=m] && NETDEVICES [=y] && WLAN [=y] && WLAN_VENDOR_INTEL [=y]
- IWLWIFI_LEDS [=y] && NETDEVICES [=y] && WLAN [=y] && WLAN_VENDOR_INTEL [=y] && IWLWIFI [=m] && (LEDS_CLASS [=m]=y || LEDS_CLASS [=m]=IWLWIFI [=m]) && (IWLMVM [=m] || IWLDVM [=m])

aarch64-linux-ld: drivers/net/wireless/ath/ath5k/led.o: in function `ath5k_register_led':
led.c:(.text+0x60): undefined reference to `led_classdev_register_ext'
aarch64-linux-ld: drivers/net/wireless/ath/ath5k/led.o: in function `ath5k_unregister_leds':
led.c:(.text+0x200): undefined reference to `led_classdev_unregister'

For iwlwifi, the dependency is wrong, since this config prevents the
MAC80211_LEDS code from being part of a built-in MAC80211 driver.

For iwlegacy, this is worse because the driver tries to force-enable
the other subsystems, which is both a layering violation and a bug
because it will still fail with MAC80211=y and IWLEGACY=m, leading
to LEDS_CLASS being a module as well.

The actual link failure in the ath5k driver is a result of MAC80211_LEDS
being enabled but not usable. With the Kconfig logic fixed in the
Intel drivers, the ath5k driver works as expected again.

Signed-off-by: Arnd Bergmann <[email protected]>
---
drivers/net/wireless/intel/iwlegacy/Kconfig | 4 ++--
drivers/net/wireless/intel/iwlwifi/Kconfig | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlegacy/Kconfig b/drivers/net/wireless/intel/iwlegacy/Kconfig
index 24fe3f63c321..7eacc8e58ee1 100644
--- a/drivers/net/wireless/intel/iwlegacy/Kconfig
+++ b/drivers/net/wireless/intel/iwlegacy/Kconfig
@@ -2,14 +2,13 @@
config IWLEGACY
tristate
select FW_LOADER
- select NEW_LEDS
- select LEDS_CLASS
select LEDS_TRIGGERS
select MAC80211_LEDS

config IWL4965
tristate "Intel Wireless WiFi 4965AGN (iwl4965)"
depends on PCI && MAC80211
+ depends on LEDS_CLASS=y || LEDS_CLASS=MAC80211
select IWLEGACY
help
This option enables support for
@@ -38,6 +37,7 @@ config IWL4965
config IWL3945
tristate "Intel PRO/Wireless 3945ABG/BG Network Connection (iwl3945)"
depends on PCI && MAC80211
+ depends on LEDS_CLASS=y || LEDS_CLASS=MAC80211
select IWLEGACY
help
Select to build the driver supporting the:
diff --git a/drivers/net/wireless/intel/iwlwifi/Kconfig b/drivers/net/wireless/intel/iwlwifi/Kconfig
index 337428ef2e67..cf1125d84929 100644
--- a/drivers/net/wireless/intel/iwlwifi/Kconfig
+++ b/drivers/net/wireless/intel/iwlwifi/Kconfig
@@ -47,7 +47,7 @@ if IWLWIFI

config IWLWIFI_LEDS
bool
- depends on LEDS_CLASS=y || LEDS_CLASS=IWLWIFI
+ depends on LEDS_CLASS=y || LEDS_CLASS=MAC80211
depends on IWLMVM || IWLDVM
select LEDS_TRIGGERS
select MAC80211_LEDS
--
2.29.2



2021-12-04 17:39:24

by Arnd Bergmann

[permalink] [raw]
Subject: [PATCH 2/3] brcmsmac: rework LED dependencies

From: Arnd Bergmann <[email protected]>

This is now the only driver that selects the LEDS_CLASS framework,
which is normally user-selectable. While it doesn't strictly cause
a bug, rework the Kconfig logic to be more consistent with what
other drivers do, and only enable LED support in brcmsmac if the
dependencies are all there, rather than using 'select' to enable
what it needs.

Signed-off-by: Arnd Bergmann <[email protected]>
---
drivers/net/wireless/broadcom/brcm80211/Kconfig | 14 +++++++++-----
.../wireless/broadcom/brcm80211/brcmsmac/Makefile | 2 +-
.../net/wireless/broadcom/brcm80211/brcmsmac/led.h | 2 +-
3 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/Kconfig b/drivers/net/wireless/broadcom/brcm80211/Kconfig
index 5bf2318763c5..3a1a35b5672f 100644
--- a/drivers/net/wireless/broadcom/brcm80211/Kconfig
+++ b/drivers/net/wireless/broadcom/brcm80211/Kconfig
@@ -7,16 +7,20 @@ config BRCMSMAC
depends on MAC80211
depends on BCMA_POSSIBLE
select BCMA
- select NEW_LEDS if BCMA_DRIVER_GPIO
- select LEDS_CLASS if BCMA_DRIVER_GPIO
select BRCMUTIL
select FW_LOADER
select CORDIC
help
This module adds support for PCIe wireless adapters based on Broadcom
- IEEE802.11n SoftMAC chipsets. It also has WLAN led support, which will
- be available if you select BCMA_DRIVER_GPIO. If you choose to build a
- module, the driver will be called brcmsmac.ko.
+ IEEE802.11n SoftMAC chipsets. If you choose to build a module, the
+ driver will be called brcmsmac.ko.
+
+config BRCMSMAC_LEDS
+ def_bool BRCMSMAC && BCMA_DRIVER_GPIO && MAC80211_LEDS
+ help
+ The brcmsmac LED support depends on the presence of the
+ BCMA_DRIVER_GPIO driver, and it only works if LED support
+ is enabled and reachable from the driver module.

source "drivers/net/wireless/broadcom/brcm80211/brcmfmac/Kconfig"

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmsmac/Makefile b/drivers/net/wireless/broadcom/brcm80211/brcmsmac/Makefile
index 482d7737764d..090757730ba6 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmsmac/Makefile
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmsmac/Makefile
@@ -42,6 +42,6 @@ brcmsmac-y := \
brcms_trace_events.o \
debug.o

-brcmsmac-$(CONFIG_BCMA_DRIVER_GPIO) += led.o
+brcmsmac-$(CONFIG_BRCMSMAC_LEDS) += led.o

obj-$(CONFIG_BRCMSMAC) += brcmsmac.o
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmsmac/led.h b/drivers/net/wireless/broadcom/brcm80211/brcmsmac/led.h
index d65f5c268fd7..2a5cbeb9e783 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmsmac/led.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmsmac/led.h
@@ -24,7 +24,7 @@ struct brcms_led {
struct gpio_desc *gpiod;
};

-#ifdef CONFIG_BCMA_DRIVER_GPIO
+#ifdef CONFIG_BRCMSMAC_LEDS
void brcms_led_unregister(struct brcms_info *wl);
int brcms_led_register(struct brcms_info *wl);
#else
--
2.29.2


2021-12-04 17:39:34

by Arnd Bergmann

[permalink] [raw]
Subject: [PATCH 3/3] mt76: mt7921: fix build regression

From: Arnd Bergmann <[email protected]>

After mt7921s got added, there are two possible build problems:

a) mt7921s does not get built at all if mt7921e is not also enabled

b) there is a link error when mt7921e is a loadable module, but mt7921s
configured as built-in:

ERROR: modpost: "mt7921_mac_sta_add" [drivers/net/wireless/mediatek/mt76/mt7921/mt7921e.ko] undefined!
ERROR: modpost: "mt7921_mac_sta_assoc" [drivers/net/wireless/mediatek/mt76/mt7921/mt7921e.ko] undefined!
ERROR: modpost: "mt7921_mac_sta_remove" [drivers/net/wireless/mediatek/mt76/mt7921/mt7921e.ko] undefined!
ERROR: modpost: "mt7921_mac_write_txwi" [drivers/net/wireless/mediatek/mt76/mt7921/mt7921e.ko] undefined!
ERROR: modpost: "mt7921_mcu_drv_pmctrl" [drivers/net/wireless/mediatek/mt76/mt7921/mt7921e.ko] undefined!
ERROR: modpost: "mt7921_mcu_fill_message" [drivers/net/wireless/mediatek/mt76/mt7921/mt7921e.ko] undefined!
ERROR: modpost: "mt7921_mcu_parse_response" [drivers/net/wireless/mediatek/mt76/mt7921/mt7921e.ko] undefined!
ERROR: modpost: "mt7921_ops" [drivers/net/wireless/mediatek/mt76/mt7921/mt7921e.ko] undefined!
ERROR: modpost: "mt7921_queue_rx_skb" [drivers/net/wireless/mediatek/mt76/mt7921/mt7921e.ko] undefined!
ERROR: modpost: "mt7921_update_channel" [drivers/net/wireless/mediatek/mt76/mt7921/mt7921e.ko] undefined!

Fix both by making sure that Kbuild enters the subdirectory when
either one is enabled.

Fixes: 48fab5bbef40 ("mt76: mt7921: introduce mt7921s support")
Signed-off-by: Arnd Bergmann <[email protected]>
---
drivers/net/wireless/mediatek/mt76/Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/mediatek/mt76/Makefile b/drivers/net/wireless/mediatek/mt76/Makefile
index 79ab850a45a2..c78ae4b89761 100644
--- a/drivers/net/wireless/mediatek/mt76/Makefile
+++ b/drivers/net/wireless/mediatek/mt76/Makefile
@@ -34,4 +34,4 @@ obj-$(CONFIG_MT76x2_COMMON) += mt76x2/
obj-$(CONFIG_MT7603E) += mt7603/
obj-$(CONFIG_MT7615_COMMON) += mt7615/
obj-$(CONFIG_MT7915E) += mt7915/
-obj-$(CONFIG_MT7921E) += mt7921/
+obj-$(CONFIG_MT7921_COMMON) += mt7921/
--
2.29.2


2021-12-04 17:52:19

by Felix Fietkau

[permalink] [raw]
Subject: Re: [PATCH 3/3] mt76: mt7921: fix build regression

On 2021-12-04 18:38, Arnd Bergmann wrote:
> From: Arnd Bergmann <[email protected]>
>
> After mt7921s got added, there are two possible build problems:
>
> a) mt7921s does not get built at all if mt7921e is not also enabled
>
> b) there is a link error when mt7921e is a loadable module, but mt7921s
> configured as built-in:
>
> ERROR: modpost: "mt7921_mac_sta_add" [drivers/net/wireless/mediatek/mt76/mt7921/mt7921e.ko] undefined!
> ERROR: modpost: "mt7921_mac_sta_assoc" [drivers/net/wireless/mediatek/mt76/mt7921/mt7921e.ko] undefined!
> ERROR: modpost: "mt7921_mac_sta_remove" [drivers/net/wireless/mediatek/mt76/mt7921/mt7921e.ko] undefined!
> ERROR: modpost: "mt7921_mac_write_txwi" [drivers/net/wireless/mediatek/mt76/mt7921/mt7921e.ko] undefined!
> ERROR: modpost: "mt7921_mcu_drv_pmctrl" [drivers/net/wireless/mediatek/mt76/mt7921/mt7921e.ko] undefined!
> ERROR: modpost: "mt7921_mcu_fill_message" [drivers/net/wireless/mediatek/mt76/mt7921/mt7921e.ko] undefined!
> ERROR: modpost: "mt7921_mcu_parse_response" [drivers/net/wireless/mediatek/mt76/mt7921/mt7921e.ko] undefined!
> ERROR: modpost: "mt7921_ops" [drivers/net/wireless/mediatek/mt76/mt7921/mt7921e.ko] undefined!
> ERROR: modpost: "mt7921_queue_rx_skb" [drivers/net/wireless/mediatek/mt76/mt7921/mt7921e.ko] undefined!
> ERROR: modpost: "mt7921_update_channel" [drivers/net/wireless/mediatek/mt76/mt7921/mt7921e.ko] undefined!
>
> Fix both by making sure that Kbuild enters the subdirectory when
> either one is enabled.
>
> Fixes: 48fab5bbef40 ("mt76: mt7921: introduce mt7921s support")
> Signed-off-by: Arnd Bergmann <[email protected]>
Acked-by: Felix Fietkau <[email protected]>

2021-12-07 07:05:08

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 1/3] iwlwifi: fix LED dependencies

Arnd Bergmann <[email protected]> writes:

> From: Arnd Bergmann <[email protected]>
>
> The dependencies for LED configuration are highly inconsistent and too
> complicated at the moment. One of the results is a randconfig failure I
> get very rarely when LEDS_CLASS is in a loadable module, but the wireless
> core is built-in:
>
> WARNING: unmet direct dependencies detected for MAC80211_LEDS
> Depends on [n]: NET [=y] && WIRELESS [=y] && MAC80211 [=y] && (LEDS_CLASS [=m]=y || LEDS_CLASS [=m]=MAC80211 [=y])
> Selected by [m]:
> - IWLEGACY [=m] && NETDEVICES [=y] && WLAN [=y] && WLAN_VENDOR_INTEL [=y]
> - IWLWIFI_LEDS [=y] && NETDEVICES [=y] && WLAN [=y] && WLAN_VENDOR_INTEL [=y] && IWLWIFI [=m] && (LEDS_CLASS [=m]=y || LEDS_CLASS [=m]=IWLWIFI [=m]) && (IWLMVM [=m] || IWLDVM [=m])
>
> aarch64-linux-ld: drivers/net/wireless/ath/ath5k/led.o: in function `ath5k_register_led':
> led.c:(.text+0x60): undefined reference to `led_classdev_register_ext'
> aarch64-linux-ld: drivers/net/wireless/ath/ath5k/led.o: in function `ath5k_unregister_leds':
> led.c:(.text+0x200): undefined reference to `led_classdev_unregister'
>
> For iwlwifi, the dependency is wrong, since this config prevents the
> MAC80211_LEDS code from being part of a built-in MAC80211 driver.
>
> For iwlegacy, this is worse because the driver tries to force-enable
> the other subsystems, which is both a layering violation and a bug
> because it will still fail with MAC80211=y and IWLEGACY=m, leading
> to LEDS_CLASS being a module as well.
>
> The actual link failure in the ath5k driver is a result of MAC80211_LEDS
> being enabled but not usable. With the Kconfig logic fixed in the
> Intel drivers, the ath5k driver works as expected again.
>
> Signed-off-by: Arnd Bergmann <[email protected]>

Luca, I would like to take this to wireless-drivers. Ack?

--
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

2021-12-07 10:15:04

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 1/3] iwlwifi: fix LED dependencies

On Sat, 2021-12-04 at 18:38 +0100, Arnd Bergmann wrote:
>
> config IWLWIFI_LEDS
> bool
> - depends on LEDS_CLASS=y || LEDS_CLASS=IWLWIFI
> + depends on LEDS_CLASS=y || LEDS_CLASS=MAC80211
>

Hm. Can we really not have this if LEDS_CLASS=n?

johannes

2021-12-07 10:16:22

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 1/3] iwlwifi: fix LED dependencies

On Tue, 2021-12-07 at 11:14 +0100, Johannes Berg wrote:
> On Sat, 2021-12-04 at 18:38 +0100, Arnd Bergmann wrote:
> >
> > config IWLWIFI_LEDS
> > bool
> > - depends on LEDS_CLASS=y || LEDS_CLASS=IWLWIFI
> > + depends on LEDS_CLASS=y || LEDS_CLASS=MAC80211
> >
>
> Hm. Can we really not have this if LEDS_CLASS=n?
>

Well, umm. That wouldn't make sense for IWLWIFI_LEDS, sorry.

Might be simpler to express this as "depends on MAC80211_LEDS" which has
the same condition, and it feels like that makes more sense than
referencing MAC80211 here?

johannes

2021-12-07 10:49:34

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 1/3] iwlwifi: fix LED dependencies

On Tue, 2021-12-07 at 11:16 +0100, Johannes Berg wrote:
> On Tue, 2021-12-07 at 11:14 +0100, Johannes Berg wrote:
> > On Sat, 2021-12-04 at 18:38 +0100, Arnd Bergmann wrote:
> > >
> > > config IWLWIFI_LEDS
> > > bool
> > > - depends on LEDS_CLASS=y || LEDS_CLASS=IWLWIFI
> > > + depends on LEDS_CLASS=y || LEDS_CLASS=MAC80211
> > >
> >
> > Hm. Can we really not have this if LEDS_CLASS=n?
> >
>
> Well, umm. That wouldn't make sense for IWLWIFI_LEDS, sorry.
>
> Might be simpler to express this as "depends on MAC80211_LEDS" which has
> the same condition, and it feels like that makes more sense than
> referencing MAC80211 here?
>

Hm, maybe not. Sorry for the monologue here - but MAC80211_LEDS is user
selectable, and so I guess that's a different thing.

johannes

2021-12-07 10:52:12

by Luciano Coelho

[permalink] [raw]
Subject: Re: [PATCH 1/3] iwlwifi: fix LED dependencies

On Tue, 2021-12-07 at 11:49 +0100, Johannes Berg wrote:
> On Tue, 2021-12-07 at 11:16 +0100, Johannes Berg wrote:
> > On Tue, 2021-12-07 at 11:14 +0100, Johannes Berg wrote:
> > > On Sat, 2021-12-04 at 18:38 +0100, Arnd Bergmann wrote:
> > > >
> > > > config IWLWIFI_LEDS
> > > > bool
> > > > - depends on LEDS_CLASS=y || LEDS_CLASS=IWLWIFI
> > > > + depends on LEDS_CLASS=y || LEDS_CLASS=MAC80211
> > > >
> > >
> > > Hm. Can we really not have this if LEDS_CLASS=n?
> > >
> >
> > Well, umm. That wouldn't make sense for IWLWIFI_LEDS, sorry.
> >
> > Might be simpler to express this as "depends on MAC80211_LEDS" which has
> > the same condition, and it feels like that makes more sense than
> > referencing MAC80211 here?
> >
>
> Hm, maybe not. Sorry for the monologue here - but MAC80211_LEDS is user
> selectable, and so I guess that's a different thing.

Thanks for checking this, Johannes!

So, if you think we can take Arnd's patch, Kalle has my ack:

Acked-by: Luca Coelho <[email protected]>


--
Cheers,
Luca.

2021-12-08 18:17:49

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 1/3] iwlwifi: fix LED dependencies

Arnd Bergmann <[email protected]> wrote:

> From: Arnd Bergmann <[email protected]>
>
> The dependencies for LED configuration are highly inconsistent and too
> complicated at the moment. One of the results is a randconfig failure I
> get very rarely when LEDS_CLASS is in a loadable module, but the wireless
> core is built-in:
>
> WARNING: unmet direct dependencies detected for MAC80211_LEDS
> Depends on [n]: NET [=y] && WIRELESS [=y] && MAC80211 [=y] && (LEDS_CLASS [=m]=y || LEDS_CLASS [=m]=MAC80211 [=y])
> Selected by [m]:
> - IWLEGACY [=m] && NETDEVICES [=y] && WLAN [=y] && WLAN_VENDOR_INTEL [=y]
> - IWLWIFI_LEDS [=y] && NETDEVICES [=y] && WLAN [=y] && WLAN_VENDOR_INTEL [=y] && IWLWIFI [=m] && (LEDS_CLASS [=m]=y || LEDS_CLASS [=m]=IWLWIFI [=m]) && (IWLMVM [=m] || IWLDVM [=m])
>
> aarch64-linux-ld: drivers/net/wireless/ath/ath5k/led.o: in function `ath5k_register_led':
> led.c:(.text+0x60): undefined reference to `led_classdev_register_ext'
> aarch64-linux-ld: drivers/net/wireless/ath/ath5k/led.o: in function `ath5k_unregister_leds':
> led.c:(.text+0x200): undefined reference to `led_classdev_unregister'
>
> For iwlwifi, the dependency is wrong, since this config prevents the
> MAC80211_LEDS code from being part of a built-in MAC80211 driver.
>
> For iwlegacy, this is worse because the driver tries to force-enable
> the other subsystems, which is both a layering violation and a bug
> because it will still fail with MAC80211=y and IWLEGACY=m, leading
> to LEDS_CLASS being a module as well.
>
> The actual link failure in the ath5k driver is a result of MAC80211_LEDS
> being enabled but not usable. With the Kconfig logic fixed in the
> Intel drivers, the ath5k driver works as expected again.
>
> Signed-off-by: Arnd Bergmann <[email protected]>
> Acked-by: Luca Coelho <[email protected]>

3 patches applied to wireless-drivers.git, thanks.

efdbfa0ad03e iwlwifi: fix LED dependencies
c68115fc5375 brcmsmac: rework LED dependencies
f7d55d2e439f mt76: mt7921: fix build regression

--
https://patchwork.kernel.org/project/linux-wireless/patch/[email protected]/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches