diff mbox series

wifi: rtw88: add RTW88_LEDS depends on LEDS_CLASS to Kconfig

Message ID 20250116054337.35723-1-pkshih@realtek.com (mailing list archive)
State New
Delegated to: Kalle Valo
Headers show
Series wifi: rtw88: add RTW88_LEDS depends on LEDS_CLASS to Kconfig | expand

Commit Message

Ping-Ke Shih Jan. 16, 2025, 5:43 a.m. UTC
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>
---
 drivers/net/wireless/realtek/rtw88/Kconfig  | 5 +++++
 drivers/net/wireless/realtek/rtw88/Makefile | 2 +-
 drivers/net/wireless/realtek/rtw88/led.h    | 2 +-
 3 files changed, 7 insertions(+), 2 deletions(-)

Comments

Ping-Ke Shih Jan. 16, 2025, 5:54 a.m. UTC | #1
Hi Kalle,

Ping-Ke Shih <pkshih@realtek.com> wrote:
> 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")

If this patch is good to you, please take it to wireless-next to fix the
problem caused by Commit 4b6652bc6d8d.

Assigned to you in patchwork. Thanks.

Ping-Ke
Kalle Valo Jan. 16, 2025, 10:17 a.m. UTC | #2
+ 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?

> --- 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
	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).
Ping-Ke Shih Jan. 16, 2025, 12:06 p.m. UTC | #3
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'
Kalle Valo Jan. 16, 2025, 12:21 p.m. UTC | #4
Ping-Ke Shih <pkshih@realtek.com> writes:

> 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.

Thanks.

>> > --- 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'.

Ok.

>
>>         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'

Thanks. I think the leds stuff is number one cause of build failures in
wireless so this is always tricky, let's hope this works.
diff mbox series

Patch

diff --git a/drivers/net/wireless/realtek/rtw88/Kconfig b/drivers/net/wireless/realtek/rtw88/Kconfig
index 733b3e58da51..5828162add6f 100644
--- 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
diff --git a/drivers/net/wireless/realtek/rtw88/Makefile b/drivers/net/wireless/realtek/rtw88/Makefile
index e8bad9d099a4..0cbbb366e393 100644
--- a/drivers/net/wireless/realtek/rtw88/Makefile
+++ b/drivers/net/wireless/realtek/rtw88/Makefile
@@ -20,7 +20,7 @@  rtw88_core-y += main.o \
 
 rtw88_core-$(CONFIG_PM) += wow.o
 
-rtw88_core-$(CONFIG_LEDS_CLASS) += led.o
+rtw88_core-$(CONFIG_RTW88_LEDS) += led.o
 
 obj-$(CONFIG_RTW88_8822B)	+= rtw88_8822b.o
 rtw88_8822b-objs		:= rtw8822b.o rtw8822b_table.o
diff --git a/drivers/net/wireless/realtek/rtw88/led.h b/drivers/net/wireless/realtek/rtw88/led.h
index c3bb6fe49b49..fa64002b0215 100644
--- a/drivers/net/wireless/realtek/rtw88/led.h
+++ b/drivers/net/wireless/realtek/rtw88/led.h
@@ -5,7 +5,7 @@ 
 #ifndef __RTW_LED_H
 #define __RTW_LED_H
 
-#ifdef CONFIG_LEDS_CLASS
+#ifdef CONFIG_RTW88_LEDS
 
 void rtw_led_init(struct rtw_dev *rtwdev);
 void rtw_led_deinit(struct rtw_dev *rtwdev);