[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <BY3PR19MB49131AA4EAE7C24D365FAC8DFAAA0@BY3PR19MB4913.namprd19.prod.outlook.com>
Date: Tue, 12 Jan 2021 17:50:11 +0000
From: "Limonciello, Mario" <Mario.Limonciello@...l.com>
To: "Yuan, Perry" <Perry.Yuan@...l.com>,
"oder_chiou@...ltek.com" <oder_chiou@...ltek.com>,
"perex@...ex.cz" <perex@...ex.cz>,
"tiwai@...e.com" <tiwai@...e.com>,
"hdegoede@...hat.com" <hdegoede@...hat.com>,
"mgross@...ux.intel.com" <mgross@...ux.intel.com>
CC: "lgirdwood@...il.com" <lgirdwood@...il.com>,
"broonie@...nel.org" <broonie@...nel.org>,
"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 v3 3/3] ASoC: rt715:add micmute led state control supports
> -----Original Message-----
> From: Yuan, Perry <Perry_Yuan@...l.com>
> Sent: Tuesday, January 12, 2021 11:18
> To: oder_chiou@...ltek.com; perex@...ex.cz; tiwai@...e.com;
> hdegoede@...hat.com; mgross@...ux.intel.com
> Cc: lgirdwood@...il.com; broonie@...nel.org; alsa-devel@...a-project.org;
> linux-kernel@...r.kernel.org; platform-driver-x86@...r.kernel.org; Yuan,
> Perry; Limonciello, Mario
> Subject: [PATCH v3 3/3] ASoC: rt715:add micmute led state control supports
>
> From: Perry Yuan <perry_yuan@...l.com>
>
> Some new Dell system is going to support audio internal micphone
> privacy setting from hardware level with micmute led state changing
> When micmute hotkey pressed by user, soft mute will need to be enabled
> firstly in case of pop noise, and codec driver need to react to mic
> mute event to EC(embedded controller) notifying that SW mute is completed
> Then EC will do the hardware mute physically within the timeout reached
>
> This patch allow codec rt715 driver to ack EC when micmute key pressed
> through this micphone led control interface like hda_generic provided
> ACPI method defined in dell-privacy micmute led trigger will be called
> for notifying the EC that software mute has been completed
>
> Signed-off-by: Perry Yuan <perry_yuan@...l.com>
>
> --------
> v2 -> v3
> * simplify the patch to reuse some val value
> * add more detail to the commit info
>
> v1 -> v2:
> * fix some format issue
> --------
> ---
> sound/soc/codecs/rt715-sdca.c | 16 ++++++++++++++++
> sound/soc/codecs/rt715-sdca.h | 1 +
> sound/soc/codecs/rt715.c | 14 ++++++++++++++
> sound/soc/codecs/rt715.h | 1 +
> 4 files changed, 32 insertions(+)
>
> diff --git a/sound/soc/codecs/rt715-sdca.c b/sound/soc/codecs/rt715-sdca.c
> index b43ac8559e45..861a0d2a8957 100644
> --- a/sound/soc/codecs/rt715-sdca.c
> +++ b/sound/soc/codecs/rt715-sdca.c
> @@ -12,6 +12,7 @@
> #include <linux/version.h>
> #include <linux/kernel.h>
> #include <linux/init.h>
> +#include <linux/leds.h>
> #include <linux/pm_runtime.h>
> #include <linux/pm.h>
> #include <linux/soundwire/sdw.h>
> @@ -244,6 +245,7 @@ static int rt715_sdca_get_volsw(struct snd_kcontrol
> *kcontrol,
> unsigned int max = mc->max;
> int val;
>
> + pr_err("++++++rt715_sdca_get_volsw++\n");
> val = snd_soc_component_read(component, mc->reg);
> if (val < 0)
> return -EINVAL;
> @@ -261,6 +263,7 @@ static int rt715_sdca_put_volsw(struct snd_kcontrol
> *kcontrol,
> struct snd_ctl_elem_value *ucontrol)
> {
> struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
> + struct rt715_sdca_priv *rt715 =
> snd_soc_component_get_drvdata(component);
> struct soc_mixer_control *mc =
> (struct soc_mixer_control *)kcontrol->private_value;
> unsigned int val, val2, loop_cnt = 2, i;
> @@ -268,6 +271,7 @@ static int rt715_sdca_put_volsw(struct snd_kcontrol
> *kcontrol,
> unsigned int reg2 = mc->rreg;
> unsigned int reg = mc->reg;
> unsigned int max = mc->max;
> + unsigned int val0, val1;
> int err;
>
> val = ucontrol->value.integer.value[0];
> @@ -287,6 +291,18 @@ static int rt715_sdca_put_volsw(struct snd_kcontrol
> *kcontrol,
> return err;
> }
>
> +#if IS_ENABLED(CONFIG_DELL_PRIVACY)
> + /* dell privacy LED trigger state changed by muted/unmute switch */
> + if (mc->invert) {
> + if (ucontrol->value.integer.value[0] || ucontrol-
> >value.integer.value[1]) {
> + rt715->micmute_led = LED_OFF;
> + } else {
> + rt715->micmute_led = LED_ON;
> + }
> + ledtrig_audio_set(LED_AUDIO_MICMUTE, rt715->micmute_led);
> + }
> +#endif
> +
> return 0;
> }
>
> diff --git a/sound/soc/codecs/rt715-sdca.h b/sound/soc/codecs/rt715-sdca.h
> index 840c237895dd..f8988ab88f80 100644
> --- a/sound/soc/codecs/rt715-sdca.h
> +++ b/sound/soc/codecs/rt715-sdca.h
> @@ -31,6 +31,7 @@ struct rt715_sdca_priv {
> int l_is_unmute;
> int r_is_unmute;
> int hw_sdw_ver;
> + bool micmute_led;
> };
>
> struct rt715_sdw_stream_data {
> diff --git a/sound/soc/codecs/rt715.c b/sound/soc/codecs/rt715.c
> index cdcba70146da..b4e480744c94 100644
> --- a/sound/soc/codecs/rt715.c
> +++ b/sound/soc/codecs/rt715.c
> @@ -13,6 +13,7 @@
> #include <linux/init.h>
> #include <linux/delay.h>
> #include <linux/i2c.h>
> +#include <linux/leds.h>
> #include <linux/pm_runtime.h>
> #include <linux/pm.h>
> #include <linux/soundwire/sdw.h>
> @@ -88,6 +89,7 @@ static int rt715_set_amp_gain_put(struct snd_kcontrol
> *kcontrol,
> RT715_SET_GAIN_MIX_ADC2_L};
> unsigned int addr_h, addr_l, val_h, val_ll, val_lr;
> unsigned int read_ll, read_rl, i, j, loop_cnt;
> + unsigned int val0, val1;
>
> if (strstr(ucontrol->id.name, "Main Capture Switch") ||
> strstr(ucontrol->id.name, "Main Capture Volume"))
> @@ -95,6 +97,18 @@ static int rt715_set_amp_gain_put(struct snd_kcontrol
> *kcontrol,
> else
> loop_cnt = 1;
>
> +#if IS_ENABLED(CONFIG_DELL_PRIVACY)
> + /* Micmute LED state changed by muted/unmute switch */
> + if (mc->invert) {
> + if (ucontrol->value.integer.value[0] || ucontrol-
> >value.integer.value[1]) {
> + rt715->micmute_led = LED_OFF;
> + } else {
> + rt715->micmute_led = LED_ON;
> + }
> + ledtrig_audio_set(LED_AUDIO_MICMUTE, rt715->micmute_led);
> + }
> +#endif
> +
You might have missed my other comment on v2 feedback, but is there a reason
to keep it behind a compile time flag for dell privacy module? In practice
any other future led backend provider should work too. Another way to think
about it - if dell privacy wasn't enabled would this cause a problem to run
this code? I think it would just be a no-op.
> for (j = 0; j < loop_cnt; j++) {
> /* Can't use update bit function, so read the original value first
> */
> if (loop_cnt == 1) {
> diff --git a/sound/soc/codecs/rt715.h b/sound/soc/codecs/rt715.h
> index 009a8266f606..57c9af041181 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;
> + bool micmute_led;
> };
>
> struct sdw_stream_data {
> --
> 2.25.1
Powered by blists - more mailing lists