On Mon, 25 Jan 2021 at 15:38, Arnd Bergmann <arnd@xxxxxxxxxx> wrote: > > On Mon, Jan 25, 2021 at 2:27 PM Krzysztof Kozlowski <krzk@xxxxxxxxxx> wrote: > > On Mon, 25 Jan 2021 at 14:09, Arnd Bergmann <arnd@xxxxxxxxxx> wrote: > > > On Mon, Jan 25, 2021 at 12:40 PM Krzysztof Kozlowski <krzk@xxxxxxxxxx> wrote: > > > > On Mon, 25 Jan 2021 at 12:36, Arnd Bergmann <arnd@xxxxxxxxxx> 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@xxxxxxxxxx/ > > > > 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
