From: Perry Yuan <perry979106@gmail.com>
To: "Limonciello, Mario" <Mario.Limonciello@dell.com>,
"Yuan, Perry" <Perry.Yuan@dell.com>,
"oder_chiou@realtek.com" <oder_chiou@realtek.com>,
"perex@perex.cz" <perex@perex.cz>,
"tiwai@suse.com" <tiwai@suse.com>,
"hdegoede@redhat.com" <hdegoede@redhat.com>,
"mgross@linux.intel.com" <mgross@linux.intel.com>
Cc: "platform-driver-x86@vger.kernel.org"
<platform-driver-x86@vger.kernel.org>,
"alsa-devel@alsa-project.org" <alsa-devel@alsa-project.org>,
"broonie@kernel.org" <broonie@kernel.org>,
"lgirdwood@gmail.com" <lgirdwood@gmail.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v3 3/3] ASoC: rt715:add micmute led state control supports
Date: Sat, 16 Jan 2021 23:25:02 +0800 [thread overview]
Message-ID: <44d8e280-c8f3-4e3a-7ef7-b68224c96def@gmail.com> (raw)
In-Reply-To: <BY3PR19MB49131AA4EAE7C24D365FAC8DFAAA0@BY3PR19MB4913.namprd19.prod.outlook.com>
On 2021/1/13 1:50, Limonciello, Mario wrote:
>> -----Original Message-----
>> From: Yuan, Perry <Perry_Yuan@Dell.com>
>> Sent: Tuesday, January 12, 2021 11:18
>> To: oder_chiou@realtek.com; perex@perex.cz; tiwai@suse.com;
>> hdegoede@redhat.com; mgross@linux.intel.com
>> Cc: lgirdwood@gmail.com; broonie@kernel.org; alsa-devel@alsa-project.org;
>> linux-kernel@vger.kernel.org; platform-driver-x86@vger.kernel.org; Yuan,
>> Perry; Limonciello, Mario
>> Subject: [PATCH v3 3/3] ASoC: rt715:add micmute led state control supports
>>
>> 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
>> 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@dell.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
Pierre Louis suggested to just set the mic mute led state unconditionally .
It is more common interface to allow other platforms to change micmute led.
The discussion result is from "[PATCH v2 2/2] ASoC: rt715:add Mic Mute LED control support"
Here is the change compared to V3 ,the CONFIG_DELL_PRIVACY will be removed
-#if IS_ENABLED(CONFIG_DELL_PRIVACY)
- /* dell privacy LED trigger state changed by muted/unmute switch */
+ /* 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;
diff --git a/sound/soc/codecs/rt715.c b/sound/soc/codecs/rt715.c
index b4e480744c94..60bb3d98103e 100644
--- a/sound/soc/codecs/rt715.c
+++ b/sound/soc/codecs/rt715.c
@@ -97,8 +97,7 @@ 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 */
+ /* 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;
@@ -107,7 +106,6 @@ static int rt715_set_amp_gain_put(struct snd_kcontrol *kcontrol,
}
ledtrig_audio_set(LED_AUDIO_MICMUTE, rt715->micmute_led);
}
-#endif
next prev parent reply other threads:[~2021-01-18 8:05 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-12 17:18 [PATCH v3 3/3] ASoC: rt715:add micmute led state control supports Perry Yuan
2021-01-12 17:50 ` Limonciello, Mario
2021-01-16 15:25 ` Perry Yuan [this message]
2021-01-19 16:33 ` Limonciello, Mario
2021-01-19 20:33 ` Hans de Goede
2021-01-19 20:52 ` Takashi Iwai
2021-01-20 3:26 ` Yuan, Perry
2021-01-12 17:54 ` Mark Brown
2021-01-16 15:50 ` Perry Yuan
2021-01-18 12:03 ` Mark Brown
2021-02-14 6:42 ` Perry Yuan
2021-02-14 6:43 ` Perry Yuan
2021-02-14 18:51 ` Jaroslav Kysela
2021-01-15 17:45 ` Jaroslav Kysela
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=44d8e280-c8f3-4e3a-7ef7-b68224c96def@gmail.com \
--to=perry979106@gmail.com \
--cc=Mario.Limonciello@dell.com \
--cc=Perry.Yuan@dell.com \
--cc=alsa-devel@alsa-project.org \
--cc=broonie@kernel.org \
--cc=hdegoede@redhat.com \
--cc=lgirdwood@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mgross@linux.intel.com \
--cc=oder_chiou@realtek.com \
--cc=perex@perex.cz \
--cc=platform-driver-x86@vger.kernel.org \
--cc=tiwai@suse.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).