Hi Jaroslav: > -----Original Message----- > From: Jaroslav Kysela <perex@xxxxxxxx> > Sent: Monday, March 22, 2021 10:38 PM > To: Yuan, Perry; Mark Brown; pierre-louis.bossart@xxxxxxxxxxxxxxx; > Limonciello, Mario; hdegoede@xxxxxxxxxx > Cc: pobrn@xxxxxxxxxxxxxx; oder_chiou@xxxxxxxxxxx; tiwai@xxxxxxxx; > mgross@xxxxxxxxxxxxxxx; lgirdwood@xxxxxxxxx; alsa-devel@alsa- > project.org; linux-kernel@xxxxxxxxxxxxxxx; platform-driver- > x86@xxxxxxxxxxxxxxx > Subject: Re: [PATCH v4 2/2] ASoC: rt715:add micmute led state control > supports > > > [EXTERNAL EMAIL] > > Dne 22. 03. 21 v 10:25 Yuan, Perry napsal(a): > > Hi Mark: > > > >> -----Original Message----- > >> From: Mark Brown <broonie@xxxxxxxxxx> > >> Sent: Tuesday, March 9, 2021 1:24 AM > >> To: Yuan, Perry > >> Cc: pobrn@xxxxxxxxxxxxxx; pierre-louis.bossart@xxxxxxxxxxxxxxx; > >> oder_chiou@xxxxxxxxxxx; perex@xxxxxxxx; tiwai@xxxxxxxx; > >> hdegoede@xxxxxxxxxx; mgross@xxxxxxxxxxxxxxx; Limonciello, Mario; > >> lgirdwood@xxxxxxxxx; alsa-devel@xxxxxxxxxxxxxxxx; linux- > >> kernel@xxxxxxxxxxxxxxx; platform-driver-x86@xxxxxxxxxxxxxxx > >> Subject: Re: [PATCH v4 2/2] ASoC: rt715:add micmute led state control > >> supports > >> > >> On Mon, Mar 01, 2021 at 05:38:34PM +0800, Perry Yuan wrote: > >> > >>> + /* Micmute LED state changed by muted/unmute switch */ > >>> + if (mc->invert) { > >>> + if (ucontrol->value.integer.value[0] || ucontrol- > >>> value.integer.value[1]) { > >>> + micmute_led = LED_OFF; > >>> + } else { > >>> + micmute_led = LED_ON; > >>> + } > >>> + ledtrig_audio_set(LED_AUDIO_MICMUTE, micmute_led); > >>> + } > >> > >> These conditionals on inversion seem weird and counterintuitive. If > >> we're going with this approach it would probably be clearer to define > >> a custom operation for the affected controls that wraps the standard > >> one and adds the LED setting rather than keying off invert like this. > > > > Currently the sof soundwire driver has no generic led control yet. > > This patch can handle the led control needs for MIC mute LED, definitely > the patch is a short term solution. > > There is a feature request discussion when we started to implement this > solution. > > https://github.com/thesofproject/linux/issues/2496#issuecomment- > 713892 > > 620 > > > > The workable way for now is that we put the LED mute control to the > codec driver. > > When there is new and full sound LED solution implemented, this part > will be also optimized. > > The Hardware privacy feature needs this patch to handle the Mic mute > led state change. > > Before that full solution ready in kernel, could we take this as short term > solution? > > Perry, it's about the machine detection. Your code is too much generic even > for the top-level LED trigger implementation. We need an extra check, if the > proper LED's are really controlled on the specific hardware. Other hardware > may use RT715 for a different purpose. Use DMI / ACPI checks to detect this > hardware and don't misuse the inversion flag to enable this code. > > Jaroslav > > -- > Jaroslav Kysela <perex@xxxxxxxx> > Linux Sound Maintainer; ALSA Project; Red Hat, Inc. In the V2 patch, I have added the machine detection, but some guys thought that I should remove the detection for it is harmless to other system So I remove it in the following patches. Is it Ok for you if I add below detection of Dell system which enable the privacy feature ? Then the mute led control will be called normally and Mic mute will be successfully configured. There is no any impaction to other systems. +#if IS_ENABLED(CONFIG_DELL_PRIVACY) ..... +#endif