RE: [PATCH v4 2/2] ASoC: rt715:add micmute led state control supports

[Yuan, Perry]

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