* [PATCH] ASoC: rt715:add Mic Mute LED control support
@ 2020-11-03 12:58 Perry Yuan
2020-11-03 13:12 ` Mark Brown
2020-11-03 19:03 ` kernel test robot
0 siblings, 2 replies; 10+ messages in thread
From: Perry Yuan @ 2020-11-03 12:58 UTC (permalink / raw)
To: oder_chiou, lgirdwood, broonie, perex, tiwai
Cc: alsa-devel, linux-kernel, perry_yuan, Limonciello Mario
From: perry_yuan <perry_yuan@dell.com>
Some new Dell system is going to support audio internal micphone
privacy setting from hardware level with micmute led state changing
This patch allow to change micmute led state through this micphone
led control interface like hda_generic provided.
Signed-off-by: Perry Yuan <perry_yuan@dell.com>
Signed-off-by: Limonciello Mario <mario_limonciello@dell.com>
---
sound/soc/codecs/rt715.c | 43 ++++++++++++++++++++++++++++++++++++++++
sound/soc/codecs/rt715.h | 1 +
2 files changed, 44 insertions(+)
diff --git a/sound/soc/codecs/rt715.c b/sound/soc/codecs/rt715.c
index 099c8bd20006..2df2895d0092 100644
--- a/sound/soc/codecs/rt715.c
+++ b/sound/soc/codecs/rt715.c
@@ -26,6 +26,7 @@
#include <linux/of.h>
#include <linux/of_gpio.h>
#include <linux/of_device.h>
+#include <linux/leds.h>
#include <sound/core.h>
#include <sound/pcm.h>
#include <sound/pcm_params.h>
@@ -213,6 +214,45 @@ static const DECLARE_TLV_DB_SCALE(mic_vol_tlv, 0, 1000, 0);
.private_value = SOC_DOUBLE_R_VALUE(reg_left, reg_right, xshift, \
xmax, xinvert) }
+static const char *rt715_micmute_led_mode[] = {
+ "Off", "On"
+};
+
+static const struct soc_enum rt715_micmute_led_mode_enum =
+ SOC_ENUM_SINGLE_EXT(ARRAY_SIZE(rt715_micmute_led_mode),
+ rt715_micmute_led_mode);
+
+static int rt715_mic_mute_led_mode_get(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
+ struct rt715_priv *rt715 = snd_soc_component_get_drvdata(component);
+ int led_mode = rt715->micmute_led;
+
+ ucontrol->value.integer.value[0] = led_mode;
+#if IS_ENABLED(CONFIG_LEDS_TRIGGER_AUDIO)
+ ledtrig_audio_set(LED_AUDIO_MICMUTE,
+ rt715->micmute_led ? LED_ON : LED_OFF);
+#endif
+ return 0;
+}
+
+static int rt715_micmute_led_mode_put(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
+ struct rt715_priv *rt715 = snd_soc_component_get_drvdata(component);
+ int led_mode = ucontrol->value.integer.value[0];
+
+ rt715->micmute_led = led_mode;
+#if IS_ENABLED(CONFIG_LEDS_TRIGGER_AUDIO)
+ ledtrig_audio_set(LED_AUDIO_MICMUTE,
+ rt715->micmute_led ? LED_ON : LED_OFF);
+#endif
+ return 0;
+}
+
+
static const struct snd_kcontrol_new rt715_snd_controls[] = {
/* Capture switch */
SOC_DOUBLE_R_EXT("ADC 07 Capture Switch", RT715_SET_GAIN_MIC_ADC_H,
@@ -277,6 +317,9 @@ static const struct snd_kcontrol_new rt715_snd_controls[] = {
RT715_SET_GAIN_LINE2_L, RT715_DIR_IN_SFT, 3, 0,
rt715_set_amp_gain_get, rt715_set_amp_gain_put,
mic_vol_tlv),
+ /*Micmute Led Control*/
+ SOC_ENUM_EXT("Micmute Led Mode", rt715_micmute_led_mode_enum,
+ rt715_mic_mute_led_mode_get, rt715_micmute_led_mode_put),
};
static int rt715_mux_get(struct snd_kcontrol *kcontrol,
diff --git a/sound/soc/codecs/rt715.h b/sound/soc/codecs/rt715.h
index df0f24f9bc0c..32917b7846b4 100644
--- a/sound/soc/codecs/rt715.h
+++ b/sound/soc/codecs/rt715.h
@@ -22,6 +22,7 @@ struct rt715_priv {
struct sdw_bus_params params;
bool hw_init;
bool first_hw_init;
+ int micmute_led;
};
struct sdw_stream_data {
--
2.25.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] ASoC: rt715:add Mic Mute LED control support
2020-11-03 12:58 [PATCH] ASoC: rt715:add Mic Mute LED control support Perry Yuan
@ 2020-11-03 13:12 ` Mark Brown
2020-11-03 16:13 ` Pierre-Louis Bossart
2020-11-03 19:03 ` kernel test robot
1 sibling, 1 reply; 10+ messages in thread
From: Mark Brown @ 2020-11-03 13:12 UTC (permalink / raw)
To: Perry Yuan
Cc: oder_chiou, lgirdwood, perex, tiwai, alsa-devel, linux-kernel,
Limonciello Mario
[-- Attachment #1: Type: text/plain, Size: 1424 bytes --]
On Tue, Nov 03, 2020 at 04:58:59AM -0800, Perry Yuan wrote:
> From: perry_yuan <perry_yuan@dell.com>
>
> Some new Dell system is going to support audio internal micphone
> privacy setting from hardware level with micmute led state changing
>
> This patch allow to change micmute led state through this micphone
> led control interface like hda_generic provided.
If this is useful it should be done at the subsystem level rather than
open coded in a specific CODEC driver, however I don't undersand why it
is.
> +static int rt715_micmute_led_mode_put(struct snd_kcontrol *kcontrol,
> + struct snd_ctl_elem_value *ucontrol)
> +{
> + struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
> + struct rt715_priv *rt715 = snd_soc_component_get_drvdata(component);
> + int led_mode = ucontrol->value.integer.value[0];
> +
> + rt715->micmute_led = led_mode;
> +#if IS_ENABLED(CONFIG_LEDS_TRIGGER_AUDIO)
> + ledtrig_audio_set(LED_AUDIO_MICMUTE,
> + rt715->micmute_led ? LED_ON : LED_OFF);
> +#endif
> + return 0;
> +}
This is just adding a userspace API to set a LED via the standard LED
APIs. Since the LED subsystem already has a perfectly good userspace
API why not use that? There is no visible value in this being in the
sound subsystem.
Please also follow the kernel coding style, your code doesn't visually
resemble the adjacent code.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] ASoC: rt715:add Mic Mute LED control support
2020-11-03 13:12 ` Mark Brown
@ 2020-11-03 16:13 ` Pierre-Louis Bossart
2020-11-03 17:51 ` Limonciello, Mario
2020-11-03 17:59 ` Mark Brown
0 siblings, 2 replies; 10+ messages in thread
From: Pierre-Louis Bossart @ 2020-11-03 16:13 UTC (permalink / raw)
To: Mark Brown, Perry Yuan
Cc: oder_chiou, alsa-devel, lgirdwood, Limonciello Mario,
linux-kernel, tiwai
Somehow this patch was filtered by alsa-devel servers?
On 11/3/20 7:12 AM, Mark Brown wrote:
> On Tue, Nov 03, 2020 at 04:58:59AM -0800, Perry Yuan wrote:
>> From: perry_yuan <perry_yuan@dell.com>
>>
>> Some new Dell system is going to support audio internal micphone
>> privacy setting from hardware level with micmute led state changing
>>
>> This patch allow to change micmute led state through this micphone
>> led control interface like hda_generic provided.
>
> If this is useful it should be done at the subsystem level rather than
> open coded in a specific CODEC driver, however I don't undersand why it
> is.
>
>> +static int rt715_micmute_led_mode_put(struct snd_kcontrol *kcontrol,
>> + struct snd_ctl_elem_value *ucontrol)
>> +{
>> + struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
>> + struct rt715_priv *rt715 = snd_soc_component_get_drvdata(component);
>> + int led_mode = ucontrol->value.integer.value[0];
>> +
>> + rt715->micmute_led = led_mode;
>> +#if IS_ENABLED(CONFIG_LEDS_TRIGGER_AUDIO)
>> + ledtrig_audio_set(LED_AUDIO_MICMUTE,
>> + rt715->micmute_led ? LED_ON : LED_OFF);
>> +#endif
>> + return 0;
>> +}
>
> This is just adding a userspace API to set a LED via the standard LED
> APIs. Since the LED subsystem already has a perfectly good userspace
> API why not use that? There is no visible value in this being in the
> sound subsystem.
I also don't quite follow. This looks as inspired from HDaudio code, but
with a lot of simplifications.
If the intent was that when userspace decides to mute the LED is turned
on, wouldn't it be enough to just track the state of a 'capture switch'
responsible for mute, i.e. when the capture Switch is 'off' the LED is
on. I don't see the point of having a new control, you would be adding
more work for PulseAudio/UCM whereas connecting the capture switch to a
led comes with zero work in userspace. See e.g. how the mute mic LED was
handled in the SOF code handling DMICs, we didn't add a new control but
turned the LED in the switch .put callback, see
https://elixir.bootlin.com/linux/latest/source/sound/soc/sof/control.c#L18
https://elixir.bootlin.com/linux/latest/source/sound/soc/sof/control.c#L153
Actually thinking more about it, having two controls for 'mute LED' and
'capture switch' could lead to inconsistent states where the LED is on
without mute being activated. we should really bolt the LED activation
to the capture switch, that way the mute and LED states are aligned.
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH] ASoC: rt715:add Mic Mute LED control support
2020-11-03 16:13 ` Pierre-Louis Bossart
@ 2020-11-03 17:51 ` Limonciello, Mario
2020-11-03 17:59 ` Mark Brown
1 sibling, 0 replies; 10+ messages in thread
From: Limonciello, Mario @ 2020-11-03 17:51 UTC (permalink / raw)
To: Pierre-Louis Bossart, Mark Brown, Yuan, Perry
Cc: oder_chiou, alsa-devel, lgirdwood, linux-kernel, tiwai
> -----Original Message-----
> From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> Sent: Tuesday, November 3, 2020 10:13
> To: Mark Brown; Yuan, Perry
> Cc: oder_chiou@realtek.com; alsa-devel@alsa-project.org; lgirdwood@gmail.com;
> Limonciello, Mario; linux-kernel@vger.kernel.org; tiwai@suse.com
> Subject: Re: [PATCH] ASoC: rt715:add Mic Mute LED control support
>
>
> [EXTERNAL EMAIL]
>
> Somehow this patch was filtered by alsa-devel servers?
>
> On 11/3/20 7:12 AM, Mark Brown wrote:
> > On Tue, Nov 03, 2020 at 04:58:59AM -0800, Perry Yuan wrote:
> >> From: perry_yuan <perry_yuan@dell.com>
> >>
> >> Some new Dell system is going to support audio internal micphone
> >> privacy setting from hardware level with micmute led state changing
> >>
> >> This patch allow to change micmute led state through this micphone
> >> led control interface like hda_generic provided.
> >
> > If this is useful it should be done at the subsystem level rather than
> > open coded in a specific CODEC driver, however I don't undersand why it
> > is.
> >
> >> +static int rt715_micmute_led_mode_put(struct snd_kcontrol *kcontrol,
> >> + struct snd_ctl_elem_value *ucontrol)
> >> +{
> >> + struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
> >> + struct rt715_priv *rt715 = snd_soc_component_get_drvdata(component);
> >> + int led_mode = ucontrol->value.integer.value[0];
> >> +
> >> + rt715->micmute_led = led_mode;
> >> +#if IS_ENABLED(CONFIG_LEDS_TRIGGER_AUDIO)
> >> + ledtrig_audio_set(LED_AUDIO_MICMUTE,
> >> + rt715->micmute_led ? LED_ON : LED_OFF);
> >> +#endif
> >> + return 0;
> >> +}
> >
> > This is just adding a userspace API to set a LED via the standard LED
> > APIs. Since the LED subsystem already has a perfectly good userspace
> > API why not use that? There is no visible value in this being in the
> > sound subsystem.
>
> I also don't quite follow. This looks as inspired from HDaudio code, but
> with a lot of simplifications.
>
> If the intent was that when userspace decides to mute the LED is turned
> on, wouldn't it be enough to just track the state of a 'capture switch'
> responsible for mute, i.e. when the capture Switch is 'off' the LED is
> on. I don't see the point of having a new control, you would be adding
> more work for PulseAudio/UCM whereas connecting the capture switch to a
> led comes with zero work in userspace. See e.g. how the mute mic LED was
> handled in the SOF code handling DMICs, we didn't add a new control but
> turned the LED in the switch .put callback, see
>
> https://elixir.bootlin.com/linux/latest/source/sound/soc/sof/control.c#L18
>
> https://elixir.bootlin.com/linux/latest/source/sound/soc/sof/control.c#L153
>
> Actually thinking more about it, having two controls for 'mute LED' and
> 'capture switch' could lead to inconsistent states where the LED is on
> without mute being activated. we should really bolt the LED activation
> to the capture switch, that way the mute and LED states are aligned.
>
After giving it some thought I agree. The UCM change that was opened
here https://github.com/alsa-project/alsa-ucm-conf/pull/60 wouldn't
be necessary at all if you just track capture switch directly like SOF does.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] ASoC: rt715:add Mic Mute LED control support
2020-11-03 16:13 ` Pierre-Louis Bossart
2020-11-03 17:51 ` Limonciello, Mario
@ 2020-11-03 17:59 ` Mark Brown
2020-11-03 18:04 ` Limonciello, Mario
1 sibling, 1 reply; 10+ messages in thread
From: Mark Brown @ 2020-11-03 17:59 UTC (permalink / raw)
To: Pierre-Louis Bossart
Cc: Perry Yuan, oder_chiou, alsa-devel, lgirdwood, Limonciello Mario,
linux-kernel, tiwai
[-- Attachment #1: Type: text/plain, Size: 935 bytes --]
On Tue, Nov 03, 2020 at 10:13:03AM -0600, Pierre-Louis Bossart wrote:
> Somehow this patch was filtered by alsa-devel servers?
It'll be a post by a non-subscriber I guess, in which case it will
appear later.
> Actually thinking more about it, having two controls for 'mute LED' and
> 'capture switch' could lead to inconsistent states where the LED is on
> without mute being activated. we should really bolt the LED activation to
> the capture switch, that way the mute and LED states are aligned.
Yeah, it's just asking for trouble and seems to defeat the point of
having the LED in the first place - aside from the general issues with
it being software controlled it'll require specific userspace support to
set it. Users won't be able to trust that the LED state accurately
reflects if they're muted or not. Your proposal is more what I'd expect
here, I'm not sure we can do much better with something software
controllable.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH] ASoC: rt715:add Mic Mute LED control support
2020-11-03 17:59 ` Mark Brown
@ 2020-11-03 18:04 ` Limonciello, Mario
2020-11-03 18:31 ` Mark Brown
2020-11-03 18:35 ` Pierre-Louis Bossart
0 siblings, 2 replies; 10+ messages in thread
From: Limonciello, Mario @ 2020-11-03 18:04 UTC (permalink / raw)
To: Mark Brown, Pierre-Louis Bossart
Cc: Yuan, Perry, oder_chiou, alsa-devel, lgirdwood, linux-kernel, tiwai
> -----Original Message-----
> From: Mark Brown <broonie@kernel.org>
> Sent: Tuesday, November 3, 2020 12:00
> To: Pierre-Louis Bossart
> Cc: Yuan, Perry; oder_chiou@realtek.com; alsa-devel@alsa-project.org;
> lgirdwood@gmail.com; Limonciello, Mario; linux-kernel@vger.kernel.org;
> tiwai@suse.com
> Subject: Re: [PATCH] ASoC: rt715:add Mic Mute LED control support
>
> On Tue, Nov 03, 2020 at 10:13:03AM -0600, Pierre-Louis Bossart wrote:
> > Somehow this patch was filtered by alsa-devel servers?
>
> It'll be a post by a non-subscriber I guess, in which case it will
> appear later.
>
> > Actually thinking more about it, having two controls for 'mute LED' and
> > 'capture switch' could lead to inconsistent states where the LED is on
> > without mute being activated. we should really bolt the LED activation to
> > the capture switch, that way the mute and LED states are aligned.
>
> Yeah, it's just asking for trouble and seems to defeat the point of
> having the LED in the first place - aside from the general issues with
> it being software controlled it'll require specific userspace support to
> set it. Users won't be able to trust that the LED state accurately
> reflects if they're muted or not. Your proposal is more what I'd expect
> here, I'm not sure we can do much better with something software
> controllable.
I don't think it came through in the commit message, but I wanted to mention
in the system that prompted this software does not control the LED. The LED
is actually controlled by hardware, but has circuitry to delay the hardware
mute until software mute is complete to avoid any "popping noises".
This patch along with the platform/x86 patch:
https://patchwork.kernel.org/project/platform-driver-x86/patch/20201103125542.8572-1-Perry_Yuan@Dell.com/
complete that loop.
The flow is:
User presses mute key, dell-wmi receives event, passes to dell-privacy-wmi.
This emits to userspace as KEY_MICMUTE. Userspace processes it and via UCM
switches get toggled. The codec driver (or subsystem perhaps) will use LED
trigger to notify to change LED. This gets picked up by dell-privacy-acpi.
dell-privacy-acpi doesn't actually change LED, but notifies that SW mute was
done.
If none of that flow was used the LED and mute function still work, but there
might be the popping noise.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] ASoC: rt715:add Mic Mute LED control support
2020-11-03 18:04 ` Limonciello, Mario
@ 2020-11-03 18:31 ` Mark Brown
2020-11-03 19:01 ` Limonciello, Mario
2020-11-03 18:35 ` Pierre-Louis Bossart
1 sibling, 1 reply; 10+ messages in thread
From: Mark Brown @ 2020-11-03 18:31 UTC (permalink / raw)
To: Limonciello, Mario
Cc: Pierre-Louis Bossart, Yuan, Perry, oder_chiou, alsa-devel,
lgirdwood, linux-kernel, tiwai
[-- Attachment #1: Type: text/plain, Size: 1256 bytes --]
On Tue, Nov 03, 2020 at 06:04:49PM +0000, Limonciello, Mario wrote:
> I don't think it came through in the commit message, but I wanted to mention
> in the system that prompted this software does not control the LED. The LED
> is actually controlled by hardware, but has circuitry to delay the hardware
> mute until software mute is complete to avoid any "popping noises".
Ah, this doesn't correspond to the description at all.
> The flow is:
> User presses mute key, dell-wmi receives event, passes to dell-privacy-wmi.
> This emits to userspace as KEY_MICMUTE. Userspace processes it and via UCM
> switches get toggled. The codec driver (or subsystem perhaps) will use LED
> trigger to notify to change LED. This gets picked up by dell-privacy-acpi.
> dell-privacy-acpi doesn't actually change LED, but notifies that SW mute was
> done.
> If none of that flow was used the LED and mute function still work, but there
> might be the popping noise.
With a timeout so that if things get lost somewhere then the mute button
is still functional, or can userspace block mute? Also what happens if
userspace tries to set the state without having done anything about
muting, will it trigger the hardware level mute as though the key had
been pressed?
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] ASoC: rt715:add Mic Mute LED control support
2020-11-03 18:04 ` Limonciello, Mario
2020-11-03 18:31 ` Mark Brown
@ 2020-11-03 18:35 ` Pierre-Louis Bossart
1 sibling, 0 replies; 10+ messages in thread
From: Pierre-Louis Bossart @ 2020-11-03 18:35 UTC (permalink / raw)
To: Limonciello, Mario, Mark Brown
Cc: oder_chiou, alsa-devel, lgirdwood, Yuan, Perry, linux-kernel, tiwai
> I don't think it came through in the commit message, but I wanted to mention
> in the system that prompted this software does not control the LED. The LED
> is actually controlled by hardware, but has circuitry to delay the hardware
> mute until software mute is complete to avoid any "popping noises".
>
> This patch along with the platform/x86 patch:
> https://patchwork.kernel.org/project/platform-driver-x86/patch/20201103125542.8572-1-Perry_Yuan@Dell.com/
> complete that loop.
>
> The flow is:
> User presses mute key, dell-wmi receives event, passes to dell-privacy-wmi.
> This emits to userspace as KEY_MICMUTE. Userspace processes it and via UCM
> switches get toggled. The codec driver (or subsystem perhaps) will use LED
> trigger to notify to change LED. This gets picked up by dell-privacy-acpi.
>
> dell-privacy-acpi doesn't actually change LED, but notifies that SW mute was
> done.
>
> If none of that flow was used the LED and mute function still work, but there
> might be the popping noise.
Side note that the existing UCM config for RT715 does not do what I
suggested, it seems we are using an incorrect configuration for
CaptureSwitch and CaptureVolume:
CaptureSwitch "PGA5.0 5 Master Capture Switch"
CaptureVolume "PGA5.0 5 Master Capture Volume"
CaptureVolume "PGA5.0 5 Master Capture Volume"
That should be an RT715 control, not an SOF one. This was brought to our
attention this morning. Probably a copy-paste from the DMIC case, likely
needs to be changed for both RT715 and RT715-sdca cases.
https://github.com/thesofproject/linux/issues/2544#issuecomment-721231103
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH] ASoC: rt715:add Mic Mute LED control support
2020-11-03 18:31 ` Mark Brown
@ 2020-11-03 19:01 ` Limonciello, Mario
0 siblings, 0 replies; 10+ messages in thread
From: Limonciello, Mario @ 2020-11-03 19:01 UTC (permalink / raw)
To: Mark Brown
Cc: Pierre-Louis Bossart, Yuan, Perry, oder_chiou, alsa-devel,
lgirdwood, linux-kernel, tiwai
> -----Original Message-----
> From: Mark Brown <broonie@kernel.org>
> Sent: Tuesday, November 3, 2020 12:32
> To: Limonciello, Mario
> Cc: Pierre-Louis Bossart; Yuan, Perry; oder_chiou@realtek.com; alsa-
> devel@alsa-project.org; lgirdwood@gmail.com; linux-kernel@vger.kernel.org;
> tiwai@suse.com
> Subject: Re: [PATCH] ASoC: rt715:add Mic Mute LED control support
>
> On Tue, Nov 03, 2020 at 06:04:49PM +0000, Limonciello, Mario wrote:
>
> > I don't think it came through in the commit message, but I wanted to mention
> > in the system that prompted this software does not control the LED. The LED
> > is actually controlled by hardware, but has circuitry to delay the hardware
> > mute until software mute is complete to avoid any "popping noises".
>
> Ah, this doesn't correspond to the description at all.
>
> > The flow is:
> > User presses mute key, dell-wmi receives event, passes to dell-privacy-wmi.
> > This emits to userspace as KEY_MICMUTE. Userspace processes it and via UCM
> > switches get toggled. The codec driver (or subsystem perhaps) will use LED
> > trigger to notify to change LED. This gets picked up by dell-privacy-acpi.
>
> > dell-privacy-acpi doesn't actually change LED, but notifies that SW mute was
> > done.
>
> > If none of that flow was used the LED and mute function still work, but
> there
> > might be the popping noise.
>
> With a timeout so that if things get lost somewhere then the mute button
> is still functional,
Exactly.
> or can userspace block mute? Also what happens if
> userspace tries to set the state without having done anything about
> muting, will it trigger the hardware level mute as though the key had
> been pressed?
No, the hardware level mute is tied to the actual key. If the software
mute is set, the LED won't change. In this case this is what would happen:
1) Kcontrol flips
2) Codec / Subsystem notifies dell-privacy-acpi
3) Dell-privacy-acpi notifies SW mute is done
4) Nothing happens to HW mute / LED
There is functionality in dell-privacy to track the state of the HW mute
so later on this scenario can be expanded upon to emit an event from
dell-privacy that userspace can see and show a message along the lines of
"SW was muted, but HW mute still isn't set, press FN+F2 to set it".
That's follow up stuff for after the initial patch series is landed.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] ASoC: rt715:add Mic Mute LED control support
2020-11-03 12:58 [PATCH] ASoC: rt715:add Mic Mute LED control support Perry Yuan
2020-11-03 13:12 ` Mark Brown
@ 2020-11-03 19:03 ` kernel test robot
1 sibling, 0 replies; 10+ messages in thread
From: kernel test robot @ 2020-11-03 19:03 UTC (permalink / raw)
To: Perry Yuan, oder_chiou, lgirdwood, broonie, perex, tiwai
Cc: kbuild-all, alsa-devel, linux-kernel, perry_yuan, Limonciello Mario
[-- Attachment #1: Type: text/plain, Size: 2532 bytes --]
Hi Perry,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on asoc/for-next]
[also build test ERROR on v5.10-rc2 next-20201103]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Perry-Yuan/ASoC-rt715-add-Mic-Mute-LED-control-support/20201103-210037
base: https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
config: m68k-randconfig-r021-20201103 (attached as .config)
compiler: m68k-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/7907f123e41159e71a0151518d1018dce3c5656d
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Perry-Yuan/ASoC-rt715-add-Mic-Mute-LED-control-support/20201103-210037
git checkout 7907f123e41159e71a0151518d1018dce3c5656d
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=m68k
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
m68k-linux-ld: sound/soc/codecs/rt715.o: in function `rt715_micmute_led_mode_put':
>> sound/soc/codecs/rt715.c:248: undefined reference to `ledtrig_audio_set'
m68k-linux-ld: sound/soc/codecs/rt715.o: in function `rt715_mic_mute_led_mode_get':
sound/soc/codecs/rt715.c:233: undefined reference to `ledtrig_audio_set'
vim +248 sound/soc/codecs/rt715.c
238
239 static int rt715_micmute_led_mode_put(struct snd_kcontrol *kcontrol,
240 struct snd_ctl_elem_value *ucontrol)
241 {
242 struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
243 struct rt715_priv *rt715 = snd_soc_component_get_drvdata(component);
244 int led_mode = ucontrol->value.integer.value[0];
245
246 rt715->micmute_led = led_mode;
247 #if IS_ENABLED(CONFIG_LEDS_TRIGGER_AUDIO)
> 248 ledtrig_audio_set(LED_AUDIO_MICMUTE,
249 rt715->micmute_led ? LED_ON : LED_OFF);
250 #endif
251 return 0;
252 }
253
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 17868 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-11-03 19:04 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-03 12:58 [PATCH] ASoC: rt715:add Mic Mute LED control support Perry Yuan
2020-11-03 13:12 ` Mark Brown
2020-11-03 16:13 ` Pierre-Louis Bossart
2020-11-03 17:51 ` Limonciello, Mario
2020-11-03 17:59 ` Mark Brown
2020-11-03 18:04 ` Limonciello, Mario
2020-11-03 18:31 ` Mark Brown
2020-11-03 19:01 ` Limonciello, Mario
2020-11-03 18:35 ` Pierre-Louis Bossart
2020-11-03 19:03 ` kernel test robot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).