Re: [PATCH] ath9k: fix build error with LEDS_CLASS=m

[Krzysztof Kozlowski <krzk@kernel.org>]

On Mon, 25 Jan 2021 at 15:38, Arnd Bergmann <arnd@kernel.org> wrote:
>
> On Mon, Jan 25, 2021 at 2:27 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> > On Mon, 25 Jan 2021 at 14:09, Arnd Bergmann <arnd@kernel.org> wrote:
> > > On Mon, Jan 25, 2021 at 12:40 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> > > > On Mon, 25 Jan 2021 at 12:36, Arnd Bergmann <arnd@kernel.org> wrote:
> > > > But we do not want to have this dependency (selecting MAC80211_LEDS).
> > > > I fixed this problem here:
> > > > https://lore.kernel.org/lkml/20201227143034.1134829-1-krzk@kernel.org/
> > > > Maybe let's take this approach?
> > >
> > > Generally speaking, I don't like to have a device driver specific Kconfig
> > > setting 'select' a subsystem', for two reasons:
> > >
> > > - you suddenly get asked for tons of new LED specific options when
> > >   enabling seemingly benign options
> > >
> > > - Mixing 'depends on' and 'select' leads to bugs with circular
> > >   dependencies that usually require turning some other 'select'
> > >   into 'depends on'.
> > >
> > > The problem with LEDS_CLASS in particular is that there is a mix of drivers
> > > using one vs the other roughly 50:50.
> >
> > Yes, you are right, I also don't like it. However it was like this
> > before my commit so I am not introducing a new issue. The point is
> > that in your choice the MAC80211_LEDS will be selected if LEDS_CLASS
> > is present, which is exactly what I was trying to fix/remove. My WiFi
> > dongle does not have a LED and it causes a periodic (every second)
> > event. However I still have LEDS_CLASS for other LEDS in the system.
>
> What is the effect of this lost event every second? If it causes some
> runtime warning or other problem, then neither of our fixes would
> solve it completely, because someone with a distro kernel would
> see the same issue when they have the symbol enabled but no
> physical LED in the device.

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.

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.

Best regards,
Krzysztof