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-713892620
>
> 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.
