Re: [PATCH v2 2/2] ASoC: rt715:add Mic Mute LED control support

From: Pierre-Louis Bossart
Date: Tue Jan 12 2021 - 21:43:43 EST



+#if IS_ENABLED(CONFIG_DELL_PRIVACY)
+ /* Privacy LED Trigger State Changed by muted/unmute switch */
+ if (mc->invert) {
+ val0 = ucontrol->value.integer.value[0];
+ val1 = ucontrol->value.integer.value[1];
+ if (val0 == 1 && val1 == 1) {
+ rt715->micmute_led = LED_OFF;
+ ledtrig_audio_set(LED_AUDIO_MICMUTE,
+ rt715->micmute_led ? LED_ON :
LED_OFF);
+ } else if (val0 == 0 && val1 == 0) {
+ rt715->micmute_led = LED_ON;
+ ledtrig_audio_set(LED_AUDIO_MICMUTE,
+ rt715->micmute_led ? LED_ON :
LED_OFF);
+ }
+ }
+#endif

Should this be activated for specific DMI quirks? This driver is
used in
non-Dell
platforms (I am thinking of Intel RVPs or Realtek daughterboards), I
am not sure if a build-time behavior change makes sense.

Or conversely could we just set the LEDs unconditionally if doing so
is harmless?

The current mic mute led setting path is not common used for other
vendors, just Dell platform support this mic mute led set operation.

Do you think that I need to add one DMI quirk in the next version ?
If so, I can add that.




In the HDA audio case this is modeled off of, the code runs whether or not a
vendor has support for a mic mute LED. The calls to ledtrig_audio_set should
be a no-op. I agree with @Pierre-Louis Bossart in this case, we should just be
running it whether or not dell-privacy is compiled in. If another vendor
chooses to add LED support they'll just need to set up their ledtrig supported
backend and not change codec code.

Hi @Pierre-Louis Bossart
Seems like that we have two way to go.
* DMI quirks,seems like that it needs to maintain the quirk list when vendors have new system to support.
* We just set the mic mute led state unconditionally .

Which way would you prefer for next patch review?

Maintaining quirks is a hassle, it's much simpler and consistent with HDaudio if the leds are set unconditionally. Thanks!