Re: [PATCH] wifi: rtw89: fix LED dependencies

[Arnd Bergmann]

On Wed, Sep 30, 2026, at 02:28, Ping-Ke Shih wrote:
> Arnd Bergmann <arnd@xxxxxxxxxx> wrote:
>> The problem is a misunderstanding of how Kconfig dependencies
>> work, as the 'imply' keyword is not sufficient to enable a
>> a user-visible dependency, and the boolean 'RTW89_LEDS_MC'
>> symbol cannot determine whether linking against the MC code is
>> valid.
>
> People intend to weakly select the dependency. I think keeping
> 'imply' is harmless.

Whoever those 'people' are, please tell them to stop using 'imply'.

It's obviously harmless in the sense that it doesn't do enforce
anything, it just make it more error-prone:

 - the first 'imply' only works because MAC80211_LEDS has
   the same dependency as RTW89_LEDS, so it works as a 'select'
   as long as the dependencies don't change. If the dependency
   were to change, the only difference is that imply makes it
   harder to debug because it skips the helpful message from
   kconfig.

- the second 'imply' turns on a random symbol from another
  subsystem, which is discouraged.

- we already have a mix of 'depends on' and 'select'
  for the LED support, which can lead to circular dependencies
  and other problems. Adding a third way can only make it
  worse.

>> Address this by using the correct construct to determing whether
>> linking agains the MAC80211_LEDS and LEDS_CLASS_MULTICOLOR
>> code is possible, respectively.
>> 
>> Fixes: d910631ff352 ("wifi: rtw89: add LED support to reflect the wireless association status")
>> Fixes: 721d90c8509a ("wifi: rtw89: add multicolor LED support for RTL8852CU valve board")
>> Signed-off-by: Arnd Bergmann <arnd@xxxxxxxx>
>> ---
>>  drivers/net/wireless/realtek/rtw89/Kconfig | 6 ++----
>>  1 file changed, 2 insertions(+), 4 deletions(-)
>> 
>> diff --git a/drivers/net/wireless/realtek/rtw89/Kconfig
>> b/drivers/net/wireless/realtek/rtw89/Kconfig
>> index 7c678dd1f6b3..4000ae344544 100644
>> --- a/drivers/net/wireless/realtek/rtw89/Kconfig
>> +++ b/drivers/net/wireless/realtek/rtw89/Kconfig
>> @@ -208,15 +208,13 @@ config RTW89_DEBUGFS
>>  config RTW89_LEDS
>>         bool
>>         depends on RTW89_CORE
>> -       depends on LEDS_CLASS=y || LEDS_CLASS=MAC80211
>
> I remember we fixed to this style years ago by imitating ath10k and iwlwifi,
> which they look like that still. 

ath10k doesn't use ieee80211_led but seems to just duplicate that code.

I do see that my version also got it wrong, as the 

+	depends on MAC80211_LEDS=y || MAC80211_LEDS=RTW89_CORE

line is nonsense with MAC80211_LEDS being a 'bool' symbol.

The way this was meant to be used is to have

config RTW89_LEDS
      def_bool RTW89_CORE && MAC80211_LEDS

or you can skip the symbol entirely and replace all the
CONFIG_RTW89_LEDS checks in source code and Makefile with
'#ifdef CONFIG_MAC80211_LEDS', see ath5k and ath9k for instance.

     Arnd