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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAK8P3a0_BJ4POV1rq4na5R0Wo0fTXqrU6-5vXutjwZ3Lx2f8nw@mail.gmail.com>
Date:   Sun, 30 May 2021 11:07:55 +0200
From:   Arnd Bergmann <arnd@...db.de>
To:     Randy Dunlap <rdunlap@...radead.org>
Cc:     Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        kernel test robot <lkp@...el.com>,
        Kalle Valo <kvalo@...eaurora.org>,
        Christian Lamparter <chunkeey@...glemail.com>,
        linux-wireless <linux-wireless@...r.kernel.org>
Subject: Re: [PATCH v3] wireless: carl9170: fix LEDS build errors & warnings

On Sun, May 30, 2021 at 5:14 AM Randy Dunlap <rdunlap@...radead.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@...radead.org>
> Reported-by: kernel test robot <lkp@...el.com>
> Cc: Kalle Valo <kvalo@...eaurora.org>
> Cc: Christian Lamparter <chunkeey@...glemail.com>
> Cc: linux-wireless@...r.kernel.org
> Cc: Arnd Bergmann <arnd@...db.de>
> Suggested-by: Christian Lamparter <chunkeey@...glemail.com>
> ---
> v2: modify as suggesed by Arnd
> v3: modify as suggested by Christian

Looks good to me,

Acked-by: Arnd Bergmann <arnd@...db.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