Hi Jaroslav: > -----Original Message----- > From: Jaroslav Kysela <pe...@perex.cz> > Sent: Monday, March 22, 2021 10:38 PM > To: Yuan, Perry; Mark Brown; pierre-louis.boss...@linux.intel.com; > Limonciello, Mario; hdego...@redhat.com > Cc: po...@protonmail.com; oder_ch...@realtek.com; ti...@suse.com; > mgr...@linux.intel.com; lgirdw...@gmail.com; alsa-devel@alsa- > project.org; linux-kernel@vger.kernel.org; platform-driver- > x...@vger.kernel.org > 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 <broo...@kernel.org> > >> Sent: Tuesday, March 9, 2021 1:24 AM > >> To: Yuan, Perry > >> Cc: po...@protonmail.com; pierre-louis.boss...@linux.intel.com; > >> oder_ch...@realtek.com; pe...@perex.cz; ti...@suse.com; > >> hdego...@redhat.com; mgr...@linux.intel.com; Limonciello, Mario; > >> lgirdw...@gmail.com; alsa-de...@alsa-project.org; linux- > >> ker...@vger.kernel.org; platform-driver-...@vger.kernel.org > >> 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 <pe...@perex.cz> > 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