From: Arnd Bergmann <arnd@arndb.de>
When CONFIG_ATH9K is built-in but LED support is in a loadable
module, both ath9k drivers fails to link:
x86_64-linux-ld: drivers/net/wireless/ath/ath9k/gpio.o: in function `ath_deinit_leds':
gpio.c:(.text+0x36): undefined reference to `led_classdev_unregister'
x86_64-linux-ld: drivers/net/wireless/ath/ath9k/gpio.o: in function `ath_init_leds':
gpio.c:(.text+0x179): undefined reference to `led_classdev_register_ext'
The problem is that the 'imply' keyword does not enforce any dependency
but is only a weak hint to Kconfig to enable another symbol from a
defconfig file.
Change imply to a 'depends on LEDS_CLASS' that prevents the incorrect
configuration but still allows building the driver without LED support.
The 'select MAC80211_LEDS' is now ensures that the LED support is
actually used if it is present, and the added Kconfig dependency
on MAC80211_LEDS ensures that it cannot be enabled manually when it
has no effect.
Fixes: 197f466e93f5 ("ath9k_htc: Do not select MAC80211_LEDS by default")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/net/wireless/ath/ath9k/Kconfig | 8 ++------
net/mac80211/Kconfig | 2 +-
2 files changed, 3 insertions(+), 7 deletions(-)
On Mon, Jan 25, 2021 at 4:04 PM Krzysztof Kozlowski [off-list ref] wrote:
On Mon, 25 Jan 2021 at 15:38, Arnd Bergmann [off-list ref] wrote:
quoted
On Mon, Jan 25, 2021 at 2:27 PM Krzysztof Kozlowski [off-list ref] wrote:
I meant that having MAC80211_LEDS selected causes the ath9k driver to
toggle on/off the WiFi LED. Every second, regardless whether it's
doing something or not. In my setup, I have problems with a WiFi
dongle somehow crashing (WiFi disappears, nothing comes from the
dongle... maybe it's Atheros FW, maybe some HW problem) and I found
this LED on/off slightly increases the chances of this dongle-crash.
That was the actual reason behind my commits.
Second reason is that I don't want to send USB commands every second
when the device is idle. It unnecessarily consumes power on my
low-power device.
Ok, I see.
Of course another solution is to just disable the trigger via sysfs
LED API. It would also work but my patch allows entire code to be
compiled-out (which was conditional in ath9k already).
Therefore the patch I sent allows the ath9k LED option to be fully
choosable. Someone wants every-second-LED-blink, sure, enable
ATH9K_LEDS and you have it. Someone wants to reduce the kernel size,
don't enable ATH9K_LEDS.
Originally, I think this is what CONFIG_MAC80211_LEDS was meant
for, but it seems that this is not actually practical, since this also
gets selected by half of the drivers using it, while the other half have
a dependency on it. Out of the ones that select it, some in turn
select LEDS_CLASS, while some depend on it.
I think this needs a larger-scale cleanup for consistency between
(at least) all the wireless drivers using LEDs. Either your patch
or mine should get applied in the meantime, and I don't care much
which one in this case, as we still have the remaining inconsistency.
Arnd
On Mon, Jan 25, 2021 at 12:40 PM Krzysztof Kozlowski [off-list ref] wrote:
On Mon, 25 Jan 2021 at 12:36, Arnd Bergmann [off-list ref] wrote:
quoted
From: Arnd Bergmann <arnd@arndb.de>
When CONFIG_ATH9K is built-in but LED support is in a loadable
module, both ath9k drivers fails to link:
x86_64-linux-ld: drivers/net/wireless/ath/ath9k/gpio.o: in function `ath_deinit_leds':
gpio.c:(.text+0x36): undefined reference to `led_classdev_unregister'
x86_64-linux-ld: drivers/net/wireless/ath/ath9k/gpio.o: in function `ath_init_leds':
gpio.c:(.text+0x179): undefined reference to `led_classdev_register_ext'
The problem is that the 'imply' keyword does not enforce any dependency
but is only a weak hint to Kconfig to enable another symbol from a
defconfig file.
Change imply to a 'depends on LEDS_CLASS' that prevents the incorrect
configuration but still allows building the driver without LED support.
The 'select MAC80211_LEDS' is now ensures that the LED support is
actually used if it is present, and the added Kconfig dependency
on MAC80211_LEDS ensures that it cannot be enabled manually when it
has no effect.
Generally speaking, I don't like to have a device driver specific Kconfig
setting 'select' a subsystem', for two reasons:
- you suddenly get asked for tons of new LED specific options when
enabling seemingly benign options
- Mixing 'depends on' and 'select' leads to bugs with circular
dependencies that usually require turning some other 'select'
into 'depends on'.
The problem with LEDS_CLASS in particular is that there is a mix of drivers
using one vs the other roughly 50:50.
Arnd
From: Krzysztof Kozlowski <krzk@kernel.org> Date: 2021-01-26 06:23:14
On Mon, 25 Jan 2021 at 14:09, Arnd Bergmann [off-list ref] wrote:
On Mon, Jan 25, 2021 at 12:40 PM Krzysztof Kozlowski [off-list ref] wrote:
quoted
On Mon, 25 Jan 2021 at 12:36, Arnd Bergmann [off-list ref] wrote:
quoted
From: Arnd Bergmann <arnd@arndb.de>
When CONFIG_ATH9K is built-in but LED support is in a loadable
module, both ath9k drivers fails to link:
x86_64-linux-ld: drivers/net/wireless/ath/ath9k/gpio.o: in function `ath_deinit_leds':
gpio.c:(.text+0x36): undefined reference to `led_classdev_unregister'
x86_64-linux-ld: drivers/net/wireless/ath/ath9k/gpio.o: in function `ath_init_leds':
gpio.c:(.text+0x179): undefined reference to `led_classdev_register_ext'
The problem is that the 'imply' keyword does not enforce any dependency
but is only a weak hint to Kconfig to enable another symbol from a
defconfig file.
Change imply to a 'depends on LEDS_CLASS' that prevents the incorrect
configuration but still allows building the driver without LED support.
The 'select MAC80211_LEDS' is now ensures that the LED support is
actually used if it is present, and the added Kconfig dependency
on MAC80211_LEDS ensures that it cannot be enabled manually when it
has no effect.
Generally speaking, I don't like to have a device driver specific Kconfig
setting 'select' a subsystem', for two reasons:
- you suddenly get asked for tons of new LED specific options when
enabling seemingly benign options
- Mixing 'depends on' and 'select' leads to bugs with circular
dependencies that usually require turning some other 'select'
into 'depends on'.
The problem with LEDS_CLASS in particular is that there is a mix of drivers
using one vs the other roughly 50:50.
Yes, you are right, I also don't like it. However it was like this
before my commit so I am not introducing a new issue. The point is
that in your choice the MAC80211_LEDS will be selected if LEDS_CLASS
is present, which is exactly what I was trying to fix/remove. My WiFi
dongle does not have a LED and it causes a periodic (every second)
event. However I still have LEDS_CLASS for other LEDS in the system.
Best regards,
Krzysztof
On Mon, 25 Jan 2021 at 14:09, Arnd Bergmann [off-list ref] wrote:
On Mon, Jan 25, 2021 at 12:40 PM Krzysztof Kozlowski [off-list ref] wrote:
quoted
On Mon, 25 Jan 2021 at 12:36, Arnd Bergmann [off-list ref] wrote:
quoted
From: Arnd Bergmann <arnd@arndb.de>
When CONFIG_ATH9K is built-in but LED support is in a loadable
module, both ath9k drivers fails to link:
x86_64-linux-ld: drivers/net/wireless/ath/ath9k/gpio.o: in function `ath_deinit_leds':
gpio.c:(.text+0x36): undefined reference to `led_classdev_unregister'
x86_64-linux-ld: drivers/net/wireless/ath/ath9k/gpio.o: in function `ath_init_leds':
gpio.c:(.text+0x179): undefined reference to `led_classdev_register_ext'
The problem is that the 'imply' keyword does not enforce any dependency
but is only a weak hint to Kconfig to enable another symbol from a
defconfig file.
Change imply to a 'depends on LEDS_CLASS' that prevents the incorrect
configuration but still allows building the driver without LED support.
The 'select MAC80211_LEDS' is now ensures that the LED support is
actually used if it is present, and the added Kconfig dependency
on MAC80211_LEDS ensures that it cannot be enabled manually when it
has no effect.
Generally speaking, I don't like to have a device driver specific Kconfig
setting 'select' a subsystem', for two reasons:
- you suddenly get asked for tons of new LED specific options when
enabling seemingly benign options
- Mixing 'depends on' and 'select' leads to bugs with circular
dependencies that usually require turning some other 'select'
into 'depends on'.
The problem with LEDS_CLASS in particular is that there is a mix of drivers
using one vs the other roughly 50:50.
Arnd
Generally speaking, I don't like to have a device driver specific Kconfig
setting 'select' a subsystem', for two reasons:
- you suddenly get asked for tons of new LED specific options when
enabling seemingly benign options
- Mixing 'depends on' and 'select' leads to bugs with circular
dependencies that usually require turning some other 'select'
into 'depends on'.
The problem with LEDS_CLASS in particular is that there is a mix of drivers
using one vs the other roughly 50:50.
Yes, you are right, I also don't like it. However it was like this
before my commit so I am not introducing a new issue. The point is
that in your choice the MAC80211_LEDS will be selected if LEDS_CLASS
is present, which is exactly what I was trying to fix/remove. My WiFi
dongle does not have a LED and it causes a periodic (every second)
event. However I still have LEDS_CLASS for other LEDS in the system.
What is the effect of this lost event every second? If it causes some
runtime warning or other problem, then neither of our fixes would
solve it completely, because someone with a distro kernel would
see the same issue when they have the symbol enabled but no
physical LED in the device.
Arnd
Generally speaking, I don't like to have a device driver specific Kconfig
setting 'select' a subsystem', for two reasons:
- you suddenly get asked for tons of new LED specific options when
enabling seemingly benign options
- Mixing 'depends on' and 'select' leads to bugs with circular
dependencies that usually require turning some other 'select'
into 'depends on'.
The problem with LEDS_CLASS in particular is that there is a mix of drivers
using one vs the other roughly 50:50.
Yes, you are right, I also don't like it. However it was like this
before my commit so I am not introducing a new issue. The point is
that in your choice the MAC80211_LEDS will be selected if LEDS_CLASS
is present, which is exactly what I was trying to fix/remove. My WiFi
dongle does not have a LED and it causes a periodic (every second)
event. However I still have LEDS_CLASS for other LEDS in the system.
What is the effect of this lost event every second? If it causes some
runtime warning or other problem, then neither of our fixes would
solve it completely, because someone with a distro kernel would
see the same issue when they have the symbol enabled but no
physical LED in the device.
I meant that having MAC80211_LEDS selected causes the ath9k driver to
toggle on/off the WiFi LED. Every second, regardless whether it's
doing something or not. In my setup, I have problems with a WiFi
dongle somehow crashing (WiFi disappears, nothing comes from the
dongle... maybe it's Atheros FW, maybe some HW problem) and I found
this LED on/off slightly increases the chances of this dongle-crash.
That was the actual reason behind my commits.
Second reason is that I don't want to send USB commands every second
when the device is idle. It unnecessarily consumes power on my
low-power device.
Of course another solution is to just disable the trigger via sysfs
LED API. It would also work but my patch allows entire code to be
compiled-out (which was conditional in ath9k already).
Therefore the patch I sent allows the ath9k LED option to be fully
choosable. Someone wants every-second-LED-blink, sure, enable
ATH9K_LEDS and you have it. Someone wants to reduce the kernel size,
don't enable ATH9K_LEDS.
Best regards,
Krzysztof
From: Krzysztof Kozlowski <krzk@kernel.org> Date: 2021-01-26 19:23:00
On Mon, 25 Jan 2021 at 12:36, Arnd Bergmann [off-list ref] wrote:
From: Arnd Bergmann <arnd@arndb.de>
When CONFIG_ATH9K is built-in but LED support is in a loadable
module, both ath9k drivers fails to link:
x86_64-linux-ld: drivers/net/wireless/ath/ath9k/gpio.o: in function `ath_deinit_leds':
gpio.c:(.text+0x36): undefined reference to `led_classdev_unregister'
x86_64-linux-ld: drivers/net/wireless/ath/ath9k/gpio.o: in function `ath_init_leds':
gpio.c:(.text+0x179): undefined reference to `led_classdev_register_ext'
The problem is that the 'imply' keyword does not enforce any dependency
but is only a weak hint to Kconfig to enable another symbol from a
defconfig file.
Change imply to a 'depends on LEDS_CLASS' that prevents the incorrect
configuration but still allows building the driver without LED support.
The 'select MAC80211_LEDS' is now ensures that the LED support is
actually used if it is present, and the added Kconfig dependency
on MAC80211_LEDS ensures that it cannot be enabled manually when it
has no effect.
From: Kalle Valo <hidden> Date: 2021-01-28 07:34:12
Arnd Bergmann [off-list ref] wrote:
From: Arnd Bergmann <arnd@arndb.de>
When CONFIG_ATH9K is built-in but LED support is in a loadable
module, both ath9k drivers fails to link:
x86_64-linux-ld: drivers/net/wireless/ath/ath9k/gpio.o: in function `ath_deinit_leds':
gpio.c:(.text+0x36): undefined reference to `led_classdev_unregister'
x86_64-linux-ld: drivers/net/wireless/ath/ath9k/gpio.o: in function `ath_init_leds':
gpio.c:(.text+0x179): undefined reference to `led_classdev_register_ext'
The problem is that the 'imply' keyword does not enforce any dependency
but is only a weak hint to Kconfig to enable another symbol from a
defconfig file.
Change imply to a 'depends on LEDS_CLASS' that prevents the incorrect
configuration but still allows building the driver without LED support.
The 'select MAC80211_LEDS' is now ensures that the LED support is
actually used if it is present, and the added Kconfig dependency
on MAC80211_LEDS ensures that it cannot be enabled manually when it
has no effect.
Fixes: 197f466e93f5 ("ath9k_htc: Do not select MAC80211_LEDS by default")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Acked-by: Johannes Berg <johannes@sipsolutions.net>