[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <5cad3045-7948-3282-c999-926095818d5f@redhat.com>
Date: Mon, 22 Mar 2021 16:01:41 +0100
From: Hans de Goede <hdegoede@...hat.com>
To: Jaroslav Kysela <perex@...ex.cz>,
"Yuan, Perry" <Perry.Yuan@...l.com>,
Mark Brown <broonie@...nel.org>,
"pierre-louis.bossart@...ux.intel.com"
<pierre-louis.bossart@...ux.intel.com>,
"Limonciello, Mario" <Mario.Limonciello@...l.com>
Cc: "pobrn@...tonmail.com" <pobrn@...tonmail.com>,
"oder_chiou@...ltek.com" <oder_chiou@...ltek.com>,
"tiwai@...e.com" <tiwai@...e.com>,
"mgross@...ux.intel.com" <mgross@...ux.intel.com>,
"lgirdwood@...il.com" <lgirdwood@...il.com>,
"alsa-devel@...a-project.org" <alsa-devel@...a-project.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"platform-driver-x86@...r.kernel.org"
<platform-driver-x86@...r.kernel.org>
Subject: Re: [PATCH v4 2/2] ASoC: rt715:add micmute led state control supports
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@...nel.org>
>>> Sent: Tuesday, March 9, 2021 1:24 AM
>>> To: Yuan, Perry
>>> Cc: pobrn@...tonmail.com; pierre-louis.bossart@...ux.intel.com;
>>> oder_chiou@...ltek.com; perex@...ex.cz; tiwai@...e.com;
>>> hdegoede@...hat.com; mgross@...ux.intel.com; Limonciello, Mario;
>>> lgirdwood@...il.com; alsa-devel@...a-project.org; linux-
>>> kernel@...r.kernel.org; platform-driver-x86@...r.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.
I think this would be a goo candidate for the new generic LED handling:
https://lore.kernel.org/alsa-devel/20210317172945.842280-1-perex@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
Powered by blists - more mailing lists