[PATCH v3] wireless: carl9170: fix LEDS build errors & warnings

[PATCH v3] wireless: carl9170: fix LEDS build errors & warnings

[Randy Dunlap]

kernel test robot reports over 200 build errors and warnings
that are due to this Kconfig problem when CARL9170=m,
MAC80211=y, and LEDS_CLASS=m.

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]:
  - CARL9170_LEDS [=y] && NETDEVICES [=y] && WLAN [=y] && WLAN_VENDOR_ATH [=y] && CARL9170 [=m]

CARL9170_LEDS selects MAC80211_LEDS even though its kconfig
dependencies are not met. This happens because 'select' does not follow
any Kconfig dependency chains.

Fix this by making CARL9170_LEDS depend on MAC80211_LEDS, where
the latter supplies any needed dependencies on LEDS_CLASS.

Fixes: 1d7e1e6b1b8ed ("carl9170: Makefile, Kconfig files and MAINTAINERS")
Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
Reported-by: kernel test robot <lkp@intel.com>
Cc: Kalle Valo <kvalo@codeaurora.org>
Cc: Christian Lamparter <chunkeey@googlemail.com>
Cc: linux-wireless@vger.kernel.org
Cc: Arnd Bergmann <arnd@arndb.de>
Suggested-by: Christian Lamparter <chunkeey@googlemail.com>
---
v2: modify as suggesed by Arnd
v3: modify as suggested by Christian

 drivers/net/wireless/ath/carl9170/Kconfig |    8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

--- linux-next-20210528.orig/drivers/net/wireless/ath/carl9170/Kconfig
+++ linux-next-20210528/drivers/net/wireless/ath/carl9170/Kconfig
@@ -16,13 +16,11 @@ config CARL9170
 
 config CARL9170_LEDS
 	bool "SoftLED Support"
-	depends on CARL9170
-	select MAC80211_LEDS
-	select LEDS_CLASS
-	select NEW_LEDS
 	default y
+	depends on CARL9170
+	depends on MAC80211_LEDS
 	help
-	  This option is necessary, if you want your device' LEDs to blink
+	  This option is necessary, if you want your device's LEDs to blink.
 
 	  Say Y, unless you need the LEDs for firmware debugging.
 

Re: [PATCH v3] wireless: carl9170: fix LEDS build errors & warnings

[Arnd Bergmann]

On Sun, May 30, 2021 at 5:14 AM Randy Dunlap <rdunlap@infradead.org> wrote:
>
> kernel test robot reports over 200 build errors and warnings
> that are due to this Kconfig problem when CARL9170=m,
> MAC80211=y, and LEDS_CLASS=m.
>
> 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]:
>   - CARL9170_LEDS [=y] && NETDEVICES [=y] && WLAN [=y] && WLAN_VENDOR_ATH [=y] && CARL9170 [=m]
>
> CARL9170_LEDS selects MAC80211_LEDS even though its kconfig
> dependencies are not met. This happens because 'select' does not follow
> any Kconfig dependency chains.
>
> Fix this by making CARL9170_LEDS depend on MAC80211_LEDS, where
> the latter supplies any needed dependencies on LEDS_CLASS.
>
> Fixes: 1d7e1e6b1b8ed ("carl9170: Makefile, Kconfig files and MAINTAINERS")
> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
> Reported-by: kernel test robot <lkp@intel.com>
> Cc: Kalle Valo <kvalo@codeaurora.org>
> Cc: Christian Lamparter <chunkeey@googlemail.com>
> Cc: linux-wireless@vger.kernel.org
> Cc: Arnd Bergmann <arnd@arndb.de>
> Suggested-by: Christian Lamparter <chunkeey@googlemail.com>
> ---
> v2: modify as suggesed by Arnd
> v3: modify as suggested by Christian

Looks good to me,

Acked-by: Arnd Bergmann <arnd@arndb.de>

> --- linux-next-20210528.orig/drivers/net/wireless/ath/carl9170/Kconfig
> +++ linux-next-20210528/drivers/net/wireless/ath/carl9170/Kconfig
> @@ -16,13 +16,11 @@ config CARL9170
>
>  config CARL9170_LEDS
>         bool "SoftLED Support"
> -       depends on CARL9170
> -       select MAC80211_LEDS
> -       select LEDS_CLASS
> -       select NEW_LEDS
>         default y
> +       depends on CARL9170
> +       depends on MAC80211_LEDS
>         help
> -         This option is necessary, if you want your device' LEDs to blink
> +         This option is necessary, if you want your device's LEDs to blink.
>

I see some other drivers using a similar approach, but then making the
symbol silent. This also makes sense to me, but more importantly it would
be good to be consistent across drivers, and eventually move them all
to one model. Ideally that would avoid the 'select NEW_LEDS' entirely.

       Arnd

Re: [PATCH v3] wireless: carl9170: fix LEDS build errors & warnings

[Christian Lamparter]

On 30/05/2021 05:11, Randy Dunlap wrote:
> kernel test robot reports over 200 build errors and warnings
> that are due to this Kconfig problem when CARL9170=m,
> MAC80211=y, and LEDS_CLASS=m.
> 
> 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]:
>    - CARL9170_LEDS [=y] && NETDEVICES [=y] && WLAN [=y] && WLAN_VENDOR_ATH [=y] && CARL9170 [=m]
> 
> CARL9170_LEDS selects MAC80211_LEDS even though its kconfig
> dependencies are not met. This happens because 'select' does not follow
> any Kconfig dependency chains.
> 
> Fix this by making CARL9170_LEDS depend on MAC80211_LEDS, where
> the latter supplies any needed dependencies on LEDS_CLASS.

Ok, this is not what I was expecting... I though you would just
add a "depends on / imply MAC80211_LEDS" on your v2. (this was
based on the assumption of what mac80211,  ath9k/_htc and mt76
solutions of the same problem looked like).

But since (I assuming here) this patch passed the build-bots
testing with flying colors in the different config permutations.

Acked-by: Christian Lamparter <chunkeey@gmail.com>

> Fixes: 1d7e1e6b1b8ed ("carl9170: Makefile, Kconfig files and MAINTAINERS")
> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
> Reported-by: kernel test robot <lkp@intel.com>
> Cc: Kalle Valo <kvalo@codeaurora.org>
> Cc: Christian Lamparter <chunkeey@gmail.com>
> Cc: linux-wireless@vger.kernel.org
> Cc: Arnd Bergmann <arnd@arndb.de>
> Suggested-by: Christian Lamparter <chunkeey@gmail.com>
> ---
> v2: modify as suggesed by Arnd
> v3: modify as suggested by Christian
> 
>   drivers/net/wireless/ath/carl9170/Kconfig |    8 +++-----
>   1 file changed, 3 insertions(+), 5 deletions(-)
> 
> --- linux-next-20210528.orig/drivers/net/wireless/ath/carl9170/Kconfig
> +++ linux-next-20210528/drivers/net/wireless/ath/carl9170/Kconfig
> @@ -16,13 +16,11 @@ config CARL9170
>   
>   config CARL9170_LEDS
>   	bool "SoftLED Support"
> -	depends on CARL9170
> -	select MAC80211_LEDS
> -	select LEDS_CLASS
> -	select NEW_LEDS
>   	default y
> +	depends on CARL9170
> +	depends on MAC80211_LEDS
>   	help
> -	  This option is necessary, if you want your device' LEDs to blink
> +	  This option is necessary, if you want your device's LEDs to blink.
>   
>   	  Say Y, unless you need the LEDs for firmware debugging.

Re: [PATCH v3] wireless: carl9170: fix LEDS build errors & warnings

[Randy Dunlap]

On 5/30/21 2:31 AM, Christian Lamparter wrote:
> On 30/05/2021 05:11, Randy Dunlap wrote:
>> kernel test robot reports over 200 build errors and warnings
>> that are due to this Kconfig problem when CARL9170=m,
>> MAC80211=y, and LEDS_CLASS=m.
>>
>> 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]:
>>    - CARL9170_LEDS [=y] && NETDEVICES [=y] && WLAN [=y] && WLAN_VENDOR_ATH [=y] && CARL9170 [=m]
>>
>> CARL9170_LEDS selects MAC80211_LEDS even though its kconfig
>> dependencies are not met. This happens because 'select' does not follow
>> any Kconfig dependency chains.
>>
>> Fix this by making CARL9170_LEDS depend on MAC80211_LEDS, where
>> the latter supplies any needed dependencies on LEDS_CLASS.
> 
> Ok, this is not what I was expecting... I though you would just
> add a "depends on / imply MAC80211_LEDS" on your v2. (this was
> based on the assumption of what mac80211,  ath9k/_htc and mt76
> solutions of the same problem looked like).

Do you want the user choice/prompt removed, like MT76 is?

> But since (I assuming here) this patch passed the build-bots
> testing with flying colors in the different config permutations.

It hasn't passed any build-bots testing that I know of.
I did 8 combinations of kconfigs (well, 2 of them were invalid),
but they all passed my own build testing.

> Acked-by: Christian Lamparter <chunkeey@gmail.com>
> 
>> Fixes: 1d7e1e6b1b8ed ("carl9170: Makefile, Kconfig files and MAINTAINERS")
>> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
>> Reported-by: kernel test robot <lkp@intel.com>
>> Cc: Kalle Valo <kvalo@codeaurora.org>
>> Cc: Christian Lamparter <chunkeey@gmail.com>
>> Cc: linux-wireless@vger.kernel.org
>> Cc: Arnd Bergmann <arnd@arndb.de>
>> Suggested-by: Christian Lamparter <chunkeey@gmail.com>
>> ---
>> v2: modify as suggesed by Arnd
>> v3: modify as suggested by Christian
>>
>>   drivers/net/wireless/ath/carl9170/Kconfig |    8 +++-----
>>   1 file changed, 3 insertions(+), 5 deletions(-)
>>
>> --- linux-next-20210528.orig/drivers/net/wireless/ath/carl9170/Kconfig
>> +++ linux-next-20210528/drivers/net/wireless/ath/carl9170/Kconfig
>> @@ -16,13 +16,11 @@ config CARL9170
>>     config CARL9170_LEDS
>>       bool "SoftLED Support"
>> -    depends on CARL9170
>> -    select MAC80211_LEDS
>> -    select LEDS_CLASS
>> -    select NEW_LEDS
>>       default y
>> +    depends on CARL9170
>> +    depends on MAC80211_LEDS
>>       help
>> -      This option is necessary, if you want your device' LEDs to blink
>> +      This option is necessary, if you want your device's LEDs to blink.
>>           Say Y, unless you need the LEDs for firmware debugging.
> 

thanks.
-- 
~Randy

Re: [PATCH v3] wireless: carl9170: fix LEDS build errors & warnings

[Kalle Valo]

Randy Dunlap <rdunlap@infradead.org> writes:

> On 5/30/21 2:31 AM, Christian Lamparter wrote:
>> On 30/05/2021 05:11, Randy Dunlap wrote:
>>> kernel test robot reports over 200 build errors and warnings
>>> that are due to this Kconfig problem when CARL9170=m,
>>> MAC80211=y, and LEDS_CLASS=m.
>>>
>>> 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]:
>>>    - CARL9170_LEDS [=y] && NETDEVICES [=y] && WLAN [=y] &&
>>> WLAN_VENDOR_ATH [=y] && CARL9170 [=m]
>>>
>>> CARL9170_LEDS selects MAC80211_LEDS even though its kconfig
>>> dependencies are not met. This happens because 'select' does not follow
>>> any Kconfig dependency chains.
>>>
>>> Fix this by making CARL9170_LEDS depend on MAC80211_LEDS, where
>>> the latter supplies any needed dependencies on LEDS_CLASS.
>> 
>> Ok, this is not what I was expecting... I though you would just
>> add a "depends on / imply MAC80211_LEDS" on your v2. (this was
>> based on the assumption of what mac80211,  ath9k/_htc and mt76
>> solutions of the same problem looked like).
>
> Do you want the user choice/prompt removed, like MT76 is?
>
>> But since (I assuming here) this patch passed the build-bots
>> testing with flying colors in the different config permutations.
>
> It hasn't passed any build-bots testing that I know of.
> I did 8 combinations of kconfigs (well, 2 of them were invalid),
> but they all passed my own build testing.

So is this ok to take now? Or will there be v4?

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

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

Re: [PATCH v3] wireless: carl9170: fix LEDS build errors & warnings

[Randy Dunlap]

On 6/3/21 2:46 AM, Kalle Valo wrote:
> Randy Dunlap <rdunlap@infradead.org> writes:
> 
>> On 5/30/21 2:31 AM, Christian Lamparter wrote:
>>> On 30/05/2021 05:11, Randy Dunlap wrote:
>>>> kernel test robot reports over 200 build errors and warnings
>>>> that are due to this Kconfig problem when CARL9170=m,
>>>> MAC80211=y, and LEDS_CLASS=m.
>>>>
>>>> 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]:
>>>>    - CARL9170_LEDS [=y] && NETDEVICES [=y] && WLAN [=y] &&
>>>> WLAN_VENDOR_ATH [=y] && CARL9170 [=m]
>>>>
>>>> CARL9170_LEDS selects MAC80211_LEDS even though its kconfig
>>>> dependencies are not met. This happens because 'select' does not follow
>>>> any Kconfig dependency chains.
>>>>
>>>> Fix this by making CARL9170_LEDS depend on MAC80211_LEDS, where
>>>> the latter supplies any needed dependencies on LEDS_CLASS.
>>>
>>> Ok, this is not what I was expecting... I though you would just
>>> add a "depends on / imply MAC80211_LEDS" on your v2. (this was
>>> based on the assumption of what mac80211,  ath9k/_htc and mt76
>>> solutions of the same problem looked like).
>>
>> Do you want the user choice/prompt removed, like MT76 is?
>>
>>> But since (I assuming here) this patch passed the build-bots
>>> testing with flying colors in the different config permutations.
>>
>> It hasn't passed any build-bots testing that I know of.
>> I did 8 combinations of kconfigs (well, 2 of them were invalid),
>> but they all passed my own build testing.
> 
> So is this ok to take now? Or will there be v4?

It's all good AFAIK unless Christian wants something changed.

Christian?

-- 
~Randy

Re: [PATCH v3] wireless: carl9170: fix LEDS build errors & warnings

[Christian Lamparter]

On 03/06/2021 17:20, Randy Dunlap wrote:
> On 6/3/21 2:46 AM, Kalle Valo wrote:
>> Randy Dunlap <rdunlap@infradead.org> writes:
>>
>>> On 5/30/21 2:31 AM, Christian Lamparter wrote:
>>>> On 30/05/2021 05:11, Randy Dunlap wrote:
>>>>> kernel test robot reports over 200 build errors and warnings
>>>>> that are due to this Kconfig problem when CARL9170=m,
>>>>> MAC80211=y, and LEDS_CLASS=m.
>>>>>
>>>>> 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]:
>>>>>     - CARL9170_LEDS [=y] && NETDEVICES [=y] && WLAN [=y] &&
>>>>> WLAN_VENDOR_ATH [=y] && CARL9170 [=m]
>>>>>
>>>>> CARL9170_LEDS selects MAC80211_LEDS even though its kconfig
>>>>> dependencies are not met. This happens because 'select' does not follow
>>>>> any Kconfig dependency chains.
>>>>>
>>>>> Fix this by making CARL9170_LEDS depend on MAC80211_LEDS, where
>>>>> the latter supplies any needed dependencies on LEDS_CLASS.
>>>>
>>>> Ok, this is not what I was expecting... I though you would just
>>>> add a "depends on / imply MAC80211_LEDS" on your v2. (this was
>>>> based on the assumption of what mac80211,  ath9k/_htc and mt76
>>>> solutions of the same problem looked like).
>>>
>>> Do you want the user choice/prompt removed, like MT76 is?
>>>
>>>> But since (I assuming here) this patch passed the build-bots
>>>> testing with flying colors in the different config permutations.
>>>
>>> It hasn't passed any build-bots testing that I know of.
>>> I did 8 combinations of kconfigs (well, 2 of them were invalid),
>>> but they all passed my own build testing.
>>
>> So is this ok to take now? Or will there be v4?
> 
> It's all good AFAIK unless Christian wants something changed.
> 
> Christian?
> 
I think it's good. It's probably just that Kalle is busy.
 From what I know, if something was wrong there the build-bots
would have already sent a letter.

Cheers,
Christian

Re: [PATCH v3] wireless: carl9170: fix LEDS build errors & warnings

[Kalle Valo]

Randy Dunlap <rdunlap@infradead.org> wrote:

> kernel test robot reports over 200 build errors and warnings
> that are due to this Kconfig problem when CARL9170=m,
> MAC80211=y, and LEDS_CLASS=m.
> 
> 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]:
>   - CARL9170_LEDS [=y] && NETDEVICES [=y] && WLAN [=y] && WLAN_VENDOR_ATH [=y] && CARL9170 [=m]
> 
> CARL9170_LEDS selects MAC80211_LEDS even though its kconfig
> dependencies are not met. This happens because 'select' does not follow
> any Kconfig dependency chains.
> 
> Fix this by making CARL9170_LEDS depend on MAC80211_LEDS, where
> the latter supplies any needed dependencies on LEDS_CLASS.
> 
> Fixes: 1d7e1e6b1b8ed ("carl9170: Makefile, Kconfig files and MAINTAINERS")
> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
> Reported-by: kernel test robot <lkp@intel.com>
> Cc: Kalle Valo <kvalo@codeaurora.org>
> Cc: Christian Lamparter <chunkeey@googlemail.com>
> Cc: linux-wireless@vger.kernel.org
> Cc: Arnd Bergmann <arnd@arndb.de>
> Suggested-by: Christian Lamparter <chunkeey@googlemail.com>
> Acked-by: Arnd Bergmann <arnd@arndb.de>
> Acked-by: Christian Lamparter <chunkeey@gmail.com>
> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>

Patch applied to ath-next branch of ath.git, thanks.

272fdc0c4542 wireless: carl9170: fix LEDS build errors & warnings

-- 
https://patchwork.kernel.org/project/linux-wireless/patch/20210530031134.23274-1-rdunlap@infradead.org/

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