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

[Yuan, Perry]

Hi Hans

> -----Original Message-----
> From: Hans de Goede <hdegoede at redhat.com>
> Sent: Monday, March 22, 2021 11:02 PM
> To: Jaroslav Kysela; Yuan, Perry; Mark Brown; pierre-
> louis.bossart at linux.intel.com; Limonciello, Mario
> Cc: pobrn at protonmail.com; oder_chiou at realtek.com; tiwai at suse.com;
> mgross at linux.intel.com; lgirdwood at gmail.com; alsa-devel at alsa-project.org;
> linux-kernel at vger.kernel.org; platform-driver-x86 at vger.kernel.org
> Subject: Re: [PATCH v4 2/2] ASoC: rt715:add micmute led state control
> supports
> 
> 
> [EXTERNAL EMAIL]
> 
> Hi,
> 
> On 3/22/21 3:37 PM, Jaroslav Kysela wrote:
> > Dne 22. 03. 21 v 10:25 Yuan, Perry napsal(a):
> >> Hi Mark:
> >>
> >>> -----Original Message-----
> >>> From: Mark Brown <broonie at kernel.org>
> >>> Sent: Tuesday, March 9, 2021 1:24 AM
> >>> To: Yuan, Perry
> >>> Cc: pobrn at protonmail.com; pierre-louis.bossart at linux.intel.com;
> >>> oder_chiou at realtek.com; perex at perex.cz; tiwai at suse.com;
> >>> hdegoede at redhat.com; mgross at linux.intel.com; Limonciello, Mario;
> >>> lgirdwood at gmail.com; alsa-devel at alsa-project.org; linux-
> >>> kernel at vger.kernel.org; platform-driver-x86 at 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-
> 71389
> >> 2620
> >>
> >> 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.
> 
> I think this would be a goo candidate for the new generic LED handling:
> 
> https://lore.kernel.org/alsa-devel/20210317172945.842280-1-
> perex at perex.cz/
> 
> And then use a udev-rule + hwdb and/or UCM profiles to configure the LED
> trigger for specific models from userspace ?
> 
> Regards,
> 
> Hans
> 
> 
> 
Because the SOF SDW design has no mic mute led control yet.
So I add one short term solution to make Dell privacy working for now 
Definitely , that is new solution I can add my patch on that to test as one user case .
We really need to take the short term solution work for some system which support new SOF soundwire hardware which have  dependence on the MIC mute feature
Meanwhile we can continue working on the new  "udev-rule + hwdb and/or UCM profiles" solution as to replace this one.
If we agree that, I will submit another V6 addressing new feedback. 

Perry