Hi Jaroslav:
-----Original Message----- From: Jaroslav Kysela perex@perex.cz Sent: Monday, March 22, 2021 10:38 PM To: Yuan, Perry; Mark Brown; pierre-louis.bossart@linux.intel.com; Limonciello, Mario; hdegoede@redhat.com Cc: pobrn@protonmail.com; oder_chiou@realtek.com; tiwai@suse.com; mgross@linux.intel.com; lgirdwood@gmail.com; alsa-devel@alsa- project.org; linux-kernel@vger.kernel.org; platform-driver- x86@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 broonie@kernel.org Sent: Tuesday, March 9, 2021 1:24 AM To: Yuan, Perry Cc: pobrn@protonmail.com; pierre-louis.bossart@linux.intel.com; oder_chiou@realtek.com; perex@perex.cz; tiwai@suse.com; hdegoede@redhat.com; mgross@linux.intel.com; Limonciello, Mario; lgirdwood@gmail.com; alsa-devel@alsa-project.org; linux- kernel@vger.kernel.org; platform-driver-x86@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 perex@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