linux-kernel - Re: [PATCH] ath9k: fix build error with LEDS

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Wed, 27 Jan 2021 11:36:34 +0100
From:   Johannes Berg <johannes@...solutions.net>
To:     Kalle Valo <kvalo@...eaurora.org>, Arnd Bergmann <arnd@...nel.org>
Cc:     Krzysztof Kozlowski <krzk@...nel.org>,
        QCA ath9k Development <ath9k-devel@....qualcomm.com>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Arnd Bergmann <arnd@...db.de>,
        Masahiro Yamada <masahiroy@...nel.org>,
        Flavio Suligoi <f.suligoi@...m.it>,
        linux-wireless <linux-wireless@...r.kernel.org>,
        Networking <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] ath9k: fix build error with LEDS_CLASS=m

On Wed, 2021-01-27 at 12:35 +0200, Kalle Valo wrote:
> Arnd Bergmann <arnd@...nel.org> writes:
> 
> > On Mon, Jan 25, 2021 at 4:04 PM Krzysztof Kozlowski <krzk@...nel.org> wrote:
> > > On Mon, 25 Jan 2021 at 15:38, Arnd Bergmann <arnd@...nel.org> wrote:
> > > > On Mon, Jan 25, 2021 at 2:27 PM Krzysztof Kozlowski <krzk@...nel.org> wrote:
> > > 
> > > I meant that having MAC80211_LEDS selected causes the ath9k driver to
> > > toggle on/off the WiFi LED. Every second, regardless whether it's
> > > doing something or not. In my setup, I have problems with a WiFi
> > > dongle somehow crashing (WiFi disappears, nothing comes from the
> > > dongle... maybe it's Atheros FW, maybe some HW problem) and I found
> > > this LED on/off slightly increases the chances of this dongle-crash.
> > > That was the actual reason behind my commits.
> > > 
> > > Second reason is that I don't want to send USB commands every second
> > > when the device is idle. It unnecessarily consumes power on my
> > > low-power device.
> > 
> > Ok, I see.
> > 
> > > Of course another solution is to just disable the trigger via sysfs
> > > LED API. It would also work but my patch allows entire code to be
> > > compiled-out (which was conditional in ath9k already).
> > > 
> > > Therefore the patch I sent allows the ath9k LED option to be fully
> > > choosable. Someone wants every-second-LED-blink, sure, enable
> > > ATH9K_LEDS and you have it. Someone wants to reduce the kernel size,
> > > don't enable ATH9K_LEDS.
> > 
> > Originally, I think this is what CONFIG_MAC80211_LEDS was meant
> > for, but it seems that this is not actually practical, since this also
> > gets selected by half of the drivers using it, while the other half have
> > a dependency on it. Out of the ones that select it, some in turn
> > select LEDS_CLASS, while some depend on it.
> > 
> > I think this needs a larger-scale cleanup for consistency between
> > (at least) all the wireless drivers using LEDs.
> 
> I agree, this needs cleanup.
> 
> > Either your patch or mine should get applied in the meantime, and I
> > don't care much which one in this case, as we still have the remaining
> > inconsistency.
> 
> My problem with Krzysztof's patch[1] is that it adds a new Kconfig
> option for ath9k, is that really necessary? Like Arnd said, we should
> fix drivers to use CONFIG_MAC80211_LEDS instead of having driver
> specific options.
> 
> So I would prefer take this Arnd's patch instead and queue it for v5.11.
> But as it modifies mac80211 I'll need an ack from Johannes, what do you
> think?

Sure, that seems fine.

Acked-by: Johannes Berg <johannes@...solutions.net>

johannes