On Sun, 21 Feb 2021 12:52:08 +0100, Hans de Goede wrote: > > Some 2-in-1s with a detachable (USB) keyboard(dock) have mute-LEDs in > the speaker- and/or mic-mute keys on the keyboard. > > Examples of this are the Lenovo Thinkpad10 tablet (with its USB kbd-dock) > and the HP x2 10 series. > > The detachable nature of these keyboards means that the keyboard and > thus the mute LEDs may show up after the user (or userspace restoring > old mixer settings) has muted the speaker and/or mic. > > Current LED-class devices with a default_trigger of "audio-mute" or > "audio-micmute" initialize the brightness member of led_classdev with > ledtrig_audio_get() before registering the LED. > > This makes the software state after attaching the keyboard match the > actual audio mute state, e.g. cat /sys/class/leds/foo/brightness will > show the right value. > > But before this commit nothing was actually calling the led_classdev's > brightness_set[_blocking] callback so the value returned by > ledtrig_audio_get() was never actually being sent to the hw, leading > to the mute LEDs staying in their default power-on state, after > attaching the keyboard, even if ledtrig_audio_get() returned a different > state. > > This could be fixed by having the individual LED drivers call > brightness_set[_blocking] themselves after registering the LED, > but this really is something which should be done by a led-trigger > activate callback. > > Add an activate callback for this, fixing the issue of the > mute LEDs being out of sync after (re)attaching the keyboard. > > Cc: Takashi Iwai <tiwai@xxxxxxx> > Fixes: faa2541f5b1a ("leds: trigger: Introduce audio mute LED trigger") > Reviewed-by: Marek Behún <kabel@xxxxxxxxxx> > Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx> > --- > Changes in v2: > - Fix a couple of grammar errors in the commit-message > - Add Marek's Reviewed-by (thank you) Looks good to me, thanks! Reviewed-by: Takashi Iwai <tiwai@xxxxxxx> Takashi > --- > drivers/leds/trigger/ledtrig-audio.c | 37 ++++++++++++++++++++++------ > 1 file changed, 29 insertions(+), 8 deletions(-) > > diff --git a/drivers/leds/trigger/ledtrig-audio.c b/drivers/leds/trigger/ledtrig-audio.c > index f76621e88482..c6b437e6369b 100644 > --- a/drivers/leds/trigger/ledtrig-audio.c > +++ b/drivers/leds/trigger/ledtrig-audio.c > @@ -6,10 +6,33 @@ > #include <linux/kernel.h> > #include <linux/leds.h> > #include <linux/module.h> > +#include "../leds.h" > > -static struct led_trigger *ledtrig_audio[NUM_AUDIO_LEDS]; > static enum led_brightness audio_state[NUM_AUDIO_LEDS]; > > +static int ledtrig_audio_mute_activate(struct led_classdev *led_cdev) > +{ > + led_set_brightness_nosleep(led_cdev, audio_state[LED_AUDIO_MUTE]); > + return 0; > +} > + > +static int ledtrig_audio_micmute_activate(struct led_classdev *led_cdev) > +{ > + led_set_brightness_nosleep(led_cdev, audio_state[LED_AUDIO_MICMUTE]); > + return 0; > +} > + > +static struct led_trigger ledtrig_audio[NUM_AUDIO_LEDS] = { > + [LED_AUDIO_MUTE] = { > + .name = "audio-mute", > + .activate = ledtrig_audio_mute_activate, > + }, > + [LED_AUDIO_MICMUTE] = { > + .name = "audio-micmute", > + .activate = ledtrig_audio_micmute_activate, > + }, > +}; > + > enum led_brightness ledtrig_audio_get(enum led_audio type) > { > return audio_state[type]; > @@ -19,24 +42,22 @@ EXPORT_SYMBOL_GPL(ledtrig_audio_get); > void ledtrig_audio_set(enum led_audio type, enum led_brightness state) > { > audio_state[type] = state; > - led_trigger_event(ledtrig_audio[type], state); > + led_trigger_event(&ledtrig_audio[type], state); > } > EXPORT_SYMBOL_GPL(ledtrig_audio_set); > > static int __init ledtrig_audio_init(void) > { > - led_trigger_register_simple("audio-mute", > - &ledtrig_audio[LED_AUDIO_MUTE]); > - led_trigger_register_simple("audio-micmute", > - &ledtrig_audio[LED_AUDIO_MICMUTE]); > + led_trigger_register(&ledtrig_audio[LED_AUDIO_MUTE]); > + led_trigger_register(&ledtrig_audio[LED_AUDIO_MICMUTE]); > return 0; > } > module_init(ledtrig_audio_init); > > static void __exit ledtrig_audio_exit(void) > { > - led_trigger_unregister_simple(ledtrig_audio[LED_AUDIO_MUTE]); > - led_trigger_unregister_simple(ledtrig_audio[LED_AUDIO_MICMUTE]); > + led_trigger_unregister(&ledtrig_audio[LED_AUDIO_MUTE]); > + led_trigger_unregister(&ledtrig_audio[LED_AUDIO_MICMUTE]); > } > module_exit(ledtrig_audio_exit); > > -- > 2.30.1 >