> >> -----Original Message----- > >> From: Yuan, Perry <Perry_Yuan@xxxxxxxx> > >> Sent: Tuesday, January 12, 2021 11:18 > >> To: oder_chiou@xxxxxxxxxxx; perex@xxxxxxxx; tiwai@xxxxxxxx; > >> hdegoede@xxxxxxxxxx; mgross@xxxxxxxxxxxxxxx > >> Cc: lgirdwood@xxxxxxxxx; broonie@xxxxxxxxxx; alsa-devel@xxxxxxxxxxxxxxxx; > >> linux-kernel@xxxxxxxxxxxxxxx; platform-driver-x86@xxxxxxxxxxxxxxx; Yuan, > >> Perry; Limonciello, Mario > >> Subject: [PATCH v3 3/3] ASoC: rt715:add micmute led state control supports > >> > >> From: Perry Yuan <perry_yuan@xxxxxxxx> > >> > >> 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@xxxxxxxx> > >> > >> -------- > >> 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 I'm not sure which thread, but I believe there was still also another suggestion that this should be made more "generic" and to work for all codecs. So that when we have lets say a hypothetical rt717 we don't need the same in that module.