Re: [PATCH] wifi: rtw88: add RTW88_LEDS depends on LEDS_CLASS to Kconfig

[Ping-Ke Shih <pkshih@realtek.com>]

Kalle Valo <kvalo@kernel.org> wrote:
> + stephen
> 
> Ping-Ke Shih <pkshih@realtek.com> writes:
> 
> > When using allmodconfig, .config has CONFIG_LEDS_CLASS=m but
> > autoconf.h has CONFIG_LEDS_CLASS_MODULE (additional suffix _MODULE)
> > instead of CONFIG_LEDS_CLASS, which condition CONFIG_LEDS_CLASS in
> > rtw88/led.h can't work properly.
> >
> > Add RTW88_LEDS to Kconfig, and use it as condition to fix this problem.
> >
> > drivers/net/wireless/realtek/rtw88/led.c:19:6: error: redefinition of 'rtw_led_init'
> >    19 | void rtw_led_init(struct rtw_dev *rtwdev)
> >       |      ^~~~~~~~~~~~
> > In file included from drivers/net/wireless/realtek/rtw88/led.c:7:
> > drivers/net/wireless/realtek/rtw88/led.h:15:20: note: previous
> > definition of 'rtw_led_init' with type 'void(struct rtw_dev *)'
> >    15 | static inline void rtw_led_init(struct rtw_dev *rtwdev)
> >       |                    ^~~~~~~~~~~~
> > drivers/net/wireless/realtek/rtw88/led.c:64:6: error: redefinition of 'rtw_led_deinit'
> >    64 | void rtw_led_deinit(struct rtw_dev *rtwdev)
> >       |      ^~~~~~~~~~~~~~
> > drivers/net/wireless/realtek/rtw88/led.h:19:20: note: previous
> > definition of 'rtw_led_deinit' with type 'void(struct rtw_dev *)'
> >    19 | static inline void rtw_led_deinit(struct rtw_dev *rtwdev)
> >       |                    ^~~~~~~~~~~~~~
> >
> > Fixes: 4b6652bc6d8d ("wifi: rtw88: Add support for LED blinking")
> > Cc: Bitterblue Smith <rtl8821cerfe2@gmail.com>
> > Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
> 
> Maybe add Reported-by and Closes pointing to Stephen's report?

add these by v2.

> 
> > --- a/drivers/net/wireless/realtek/rtw88/Kconfig
> > +++ b/drivers/net/wireless/realtek/rtw88/Kconfig
> > @@ -238,4 +238,9 @@ config RTW88_DEBUGFS
> >
> >         If unsure, say Y to simplify debug problems
> >
> > +config RTW88_LEDS
> > +     bool
> > +     depends on LEDS_CLASS
> > +     default y
> > +
> >  endif
> 
> As led.c uses ieee80211_create_tpt_led_trigger() should we depend on
> mac80211? For example ath10k has:
> 
> config ATH10K_LEDS
>         bool
>         depends on ATH10K

Kconfig of rtw89 uses a big 'if RTW88', so I will not add 'depdens on RTW88'.

>         depends on LEDS_CLASS=y || LEDS_CLASS=MAC80211
>         default y
> 
> I can't recall the details but I suspect that's handling the case where
> one of the modules is 'm' and other one 'y' (or something like that).
> 

Since ieee80211_create_tpt_led_trigger() depends on CONFIG_MAC80211_LEDS,
which looks like

config MAC80211_LEDS
        bool "Enable LED triggers"
        depends on MAC80211
        depends on LEDS_CLASS=y || LEDS_CLASS=MAC80211

So I will imitate ath10k to use 'depends on LEDS_CLASS=y || LEDS_CLASS=MAC80211'