> -----Original Message----- > From: Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx> > Sent: 2021年1月13日 10:42 > To: Yuan, Perry; Limonciello, Mario; oder_chiou@xxxxxxxxxxx; > perex@xxxxxxxx; tiwai@xxxxxxxx > Cc: alsa-devel@xxxxxxxxxxxxxxxx; broonie@xxxxxxxxxx; lgirdwood@xxxxxxxxx; > linux-kernel@xxxxxxxxxxxxxxx > Subject: Re: [PATCH v2 2/2] ASoC: rt715:add Mic Mute LED control support > > > [EXTERNAL EMAIL] > > > >>>>> +#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! Thank you for your confirm. I will take this to next patch V4. Perry Yuan Dell | Client Software Group | CDC Linux OS