[PATCH] ALSA: hda - Fixes inverted Conexant GPIO mic mute led

[PATCH] ALSA: hda - Fixes inverted Conexant GPIO mic mute led

[jeronimo]

From: Jeronimo Borque <jeronimo@borque.com.ar>

"enabled" parameter historically referred to the device input or
output, not to the led indicator. After the changes added with the
led helper functions the mic mute led logic refers to the led and not
to the mic input which caused led indicator to be negated (Mic mute
led was on when the input enabled) Fixing it in the call to
cxt_update_gpio_led at the cxt_gpio_micmute_update hook.
Maybe more changes are required to be consistent everywhere.

Signed-off-by: Jeronimo Borque <jeronimo@borque.com.ar>
---
 sound/pci/hda/patch_conexant.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/sound/pci/hda/patch_conexant.c b/sound/pci/hda/patch_conexant.c
index f299f137eaea..8edf0d1290b5 100644
--- a/sound/pci/hda/patch_conexant.c
+++ b/sound/pci/hda/patch_conexant.c
@@ -636,6 +636,10 @@ static void cxt_update_gpio_led(struct hda_codec *codec, unsigned int mask,
 		spec->gpio_led &= ~mask;
 	else
 		spec->gpio_led |= mask;
+
+	codec_dbg(codec, "mask:%d enabled:%d gpio_led:%d\n",
+			    mask, enabled, spec->gpio_led);
+
 	if (spec->gpio_led != oldval)
 		snd_hda_codec_write(codec, 0x01, 0, AC_VERB_SET_GPIO_DATA,
 				    spec->gpio_led);
@@ -656,7 +660,7 @@ static void cxt_gpio_micmute_update(struct hda_codec *codec)
 	struct conexant_spec *spec = codec->spec;
 
 	cxt_update_gpio_led(codec, spec->gpio_mic_led_mask,
-			    spec->gen.micmute_led.led_value);
+			    !spec->gen.micmute_led.led_value);
 }
 
 
@@ -669,7 +673,6 @@ static void cxt_fixup_mute_led_gpio(struct hda_codec *codec,
 		{ 0x01, AC_VERB_SET_GPIO_DIRECTION, 0x03 },
 		{}
 	};
-	codec_info(codec, "action: %d gpio_led: %d\n", action, spec->gpio_led);
 
 	if (action == HDA_FIXUP_ACT_PRE_PROBE) {
 		spec->gen.vmaster_mute.hook = cxt_fixup_gpio_mute_hook;
-- 
2.21.0

Re: [PATCH] ALSA: hda - Fixes inverted Conexant GPIO mic mute led

[Takashi Iwai]

On Thu, 15 Aug 2019 03:38:24 +0200,
<jeronimo@borque.com.ar> wrote:
> 
> From: Jeronimo Borque <jeronimo@borque.com.ar>
> 
> "enabled" parameter historically referred to the device input or
> output, not to the led indicator. After the changes added with the
> led helper functions the mic mute led logic refers to the led and not
> to the mic input which caused led indicator to be negated (Mic mute
> led was on when the input enabled) Fixing it in the call to
> cxt_update_gpio_led at the cxt_gpio_micmute_update hook.
> Maybe more changes are required to be consistent everywhere.
> 
> Signed-off-by: Jeronimo Borque <jeronimo@borque.com.ar>

Could you check which value you have in "Mic Mute-LED Mode" mixer
element?  I guess it's "Follow Mute".  If so, change it to "Follow
Capture".

If this works, it means that the driver works as expected but the
problem is only about the default value.  The default value set in the
generic parser is based on other machine's standard (LED on at mic
off), while some machines might expect differently.  On such machines,
we need to set the different value initially in the quirk fixup.


thanks,

Takashi

Re: [PATCH] ALSA: hda - Fixes inverted Conexant GPIO mic mute led

[Jerónimo Borque]

Hi Takashi,
Modifying Mic Mute-LED Mode does indeed alter the behavior. The thing is
that this ends being confusing as in all machines I've been testing this
setting Mic Mute-LED Mode to "Follow Capture" actually makes it follow
mute, as setting it to "On" turns the LED off.
There is other setting called "mute_led_polarity" but this does not work,
as currently mic mute LED and mute LED do not follow the same logic.
What I think may be causing confusion is "cxt_update_gpio_led" "enabled"
parameter. Setting "enabled" to "true" sets the GPIO pin to 0 causing the
led to be turned off. I think "enabled" used to refer to the input capture
or output status and not to the LED being lit or not. Output or input not
enabled (enabled==false) caused the LED to be turned on.
This logic in the function negates it on the GPIO output.

if (enabled)
    spec->gpio_led &= ~mask;
else
    spec->gpio_led |= mask;

May be I can do a more comprehensive fix, reversing the behavior of
"cxt_update_gpio_led" "enabled" parameter to refer the GPIO output value (
enabled==true => GPIO pin output high )
Then also modify the call to "cxt_update_gpio_led" in
"cxt_fixup_gpio_mute_hook" to make it work consistently.

Thanks,
Jerónimo


El jue., 15 de ago. de 2019 a la(s) 02:58, Takashi Iwai (tiwai@suse.de)
escribió:

> On Thu, 15 Aug 2019 03:38:24 +0200,
> <jeronimo@borque.com.ar> wrote:
> >
> > From: Jeronimo Borque <jeronimo@borque.com.ar>
> >
> > "enabled" parameter historically referred to the device input or
> > output, not to the led indicator. After the changes added with the
> > led helper functions the mic mute led logic refers to the led and not
> > to the mic input which caused led indicator to be negated (Mic mute
> > led was on when the input enabled) Fixing it in the call to
> > cxt_update_gpio_led at the cxt_gpio_micmute_update hook.
> > Maybe more changes are required to be consistent everywhere.
> >
> > Signed-off-by: Jeronimo Borque <jeronimo@borque.com.ar>
>
> Could you check which value you have in "Mic Mute-LED Mode" mixer
> element?  I guess it's "Follow Mute".  If so, change it to "Follow
> Capture".
>
> If this works, it means that the driver works as expected but the
> problem is only about the default value.  The default value set in the
> generic parser is based on other machine's standard (LED on at mic
> off), while some machines might expect differently.  On such machines,
> we need to set the different value initially in the quirk fixup.
>
>
> thanks,
>
> Takashi
>
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH] ALSA: hda - Fixes inverted Conexant GPIO mic mute led

[Takashi Iwai]

On Thu, 15 Aug 2019 18:33:50 +0200,
Jerónimo Borque wrote:
> 
> Hi Takashi,
> Modifying Mic Mute-LED Mode does indeed alter the behavior. The thing is that
> this ends being confusing as in all machines I've been testing this setting
> Mic Mute-LED Mode to "Follow Capture" actually makes it follow mute, as
> setting it to "On" turns the LED off.
> There is other setting called "mute_led_polarity" but this does not work, as
> currently mic mute LED and mute LED do not follow the same logic.
> What I think may be causing confusion is "cxt_update_gpio_led" "enabled"
> parameter. Setting "enabled" to "true" sets the GPIO pin to 0 causing the led
> to be turned off. I think "enabled" used to refer to the input capture or
> output status and not to the LED being lit or not. Output or input not enabled
> (enabled==false) caused the LED to be turned on.
> This logic in the function negates it on the GPIO output.
> 
> if (enabled)
>     spec->gpio_led &= ~mask;
> else
>     spec->gpio_led |= mask;
> 
> May be I can do a more comprehensive fix, reversing the behavior of 
> "cxt_update_gpio_led" "enabled" parameter to refer the GPIO output value (
> enabled==true => GPIO pin output high )
> Then also modify the call to "cxt_update_gpio_led" in
> "cxt_fixup_gpio_mute_hook" to make it work consistently.

OK, if the "On" turns the LED off, it's indeed inverted.
Then we'd need to consider both fixing the inverted behavior and the
default mic-mute mode.

Could you confirm the following?

- Which models and codecs are checked?

- GPIO pin high = mic LED on or off?

- How is the expected behavior on Windows?
  Mute is on when mic is muted, or mute-on when mic is ready?


thanks,

Takashi

> 
> Thanks,
> Jerónimo
> 
> El jue., 15 de ago. de 2019 a la(s) 02:58, Takashi Iwai (tiwai@suse.de)
> escribió:
> 
>     On Thu, 15 Aug 2019 03:38:24 +0200,
>     <jeronimo@borque.com.ar> wrote:
>     >
>     > From: Jeronimo Borque <jeronimo@borque.com.ar>
>     >
>     > "enabled" parameter historically referred to the device input or
>     > output, not to the led indicator. After the changes added with the
>     > led helper functions the mic mute led logic refers to the led and not
>     > to the mic input which caused led indicator to be negated (Mic mute
>     > led was on when the input enabled) Fixing it in the call to
>     > cxt_update_gpio_led at the cxt_gpio_micmute_update hook.
>     > Maybe more changes are required to be consistent everywhere.
>     >
>     > Signed-off-by: Jeronimo Borque <jeronimo@borque.com.ar>
>    
>     Could you check which value you have in "Mic Mute-LED Mode" mixer
>     element?  I guess it's "Follow Mute".  If so, change it to "Follow
>     Capture".
>    
>     If this works, it means that the driver works as expected but the
>     problem is only about the default value.  The default value set in the
>     generic parser is based on other machine's standard (LED on at mic
>     off), while some machines might expect differently.  On such machines,
>     we need to set the different value initially in the quirk fixup.
> 
>     thanks,
>    
>     Takashi
> 
> 
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH] ALSA: hda - Fixes inverted Conexant GPIO mic mute led

[Jerónimo Borque]

El jue., 15 de ago. de 2019 a la(s) 14:06, Takashi Iwai (tiwai@suse.de)
escribió:

> On Thu, 15 Aug 2019 18:33:50 +0200,
> Jerónimo Borque wrote:
> >
> > Hi Takashi,
> > Modifying Mic Mute-LED Mode does indeed alter the behavior. The thing is
> that
> > this ends being confusing as in all machines I've been testing this
> setting
> > Mic Mute-LED Mode to "Follow Capture" actually makes it follow mute, as
> > setting it to "On" turns the LED off.
> > There is other setting called "mute_led_polarity" but this does not
> work, as
> > currently mic mute LED and mute LED do not follow the same logic.
> > What I think may be causing confusion is "cxt_update_gpio_led" "enabled"
> > parameter. Setting "enabled" to "true" sets the GPIO pin to 0 causing
> the led
> > to be turned off. I think "enabled" used to refer to the input capture or
> > output status and not to the LED being lit or not. Output or input not
> enabled
> > (enabled==false) caused the LED to be turned on.
> > This logic in the function negates it on the GPIO output.
> >
> > if (enabled)
> >     spec->gpio_led &= ~mask;
> > else
> >     spec->gpio_led |= mask;
> >
> > May be I can do a more comprehensive fix, reversing the behavior of
> > "cxt_update_gpio_led" "enabled" parameter to refer the GPIO output value
> (
> > enabled==true => GPIO pin output high )
> > Then also modify the call to "cxt_update_gpio_led" in
> > "cxt_fixup_gpio_mute_hook" to make it work consistently.
>
> OK, if the "On" turns the LED off, it's indeed inverted.
> Then we'd need to consider both fixing the inverted behavior and the
> default mic-mute mode.
>
> Could you confirm the following?
>
> - Which models and codecs are checked?
>

I've tested on HP ZBook 15U G3 (Conexant CX20724) and HP Probook 440 G4
(Conexant CX8200)


> - GPIO pin high = mic LED on or off?
>

 GPIO pin high = mic LED on


> - How is the expected behavior on Windows?
>    Mute is on when mic is muted, or mute-on when mic is ready?


Mute led is on when mic is muted.


Thanks,
Jerónimo


>
> thanks,
>
> Takashi
>
> >
> > Thanks,
> > Jerónimo
> >
> > El jue., 15 de ago. de 2019 a la(s) 02:58, Takashi Iwai (tiwai@suse.de)
> > escribió:
> >
> >     On Thu, 15 Aug 2019 03:38:24 +0200,
> >     <jeronimo@borque.com.ar> wrote:
> >     >
> >     > From: Jeronimo Borque <jeronimo@borque.com.ar>
> >     >
> >     > "enabled" parameter historically referred to the device input or
> >     > output, not to the led indicator. After the changes added with the
> >     > led helper functions the mic mute led logic refers to the led and
> not
> >     > to the mic input which caused led indicator to be negated (Mic mute
> >     > led was on when the input enabled) Fixing it in the call to
> >     > cxt_update_gpio_led at the cxt_gpio_micmute_update hook.
> >     > Maybe more changes are required to be consistent everywhere.
> >     >
> >     > Signed-off-by: Jeronimo Borque <jeronimo@borque.com.ar>
> >
> >     Could you check which value you have in "Mic Mute-LED Mode" mixer
> >     element?  I guess it's "Follow Mute".  If so, change it to "Follow
> >     Capture".
> >
> >     If this works, it means that the driver works as expected but the
> >     problem is only about the default value.  The default value set in
> the
> >     generic parser is based on other machine's standard (LED on at mic
> >     off), while some machines might expect differently.  On such
> machines,
> >     we need to set the different value initially in the quirk fixup.
> >
> >     thanks,
> >
> >     Takashi
> >
> >
>
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH] ALSA: hda - Fixes inverted Conexant GPIO mic mute led

[Takashi Iwai]

On Fri, 16 Aug 2019 01:08:34 +0200,
Jerónimo Borque wrote:
> 
> El jue., 15 de ago. de 2019 a la(s) 14:06, Takashi Iwai (tiwai@suse.de)
> escribió:
> 
>     On Thu, 15 Aug 2019 18:33:50 +0200,
>     Jerónimo Borque wrote:
>     >
>     > Hi Takashi,
>     > Modifying Mic Mute-LED Mode does indeed alter the behavior. The thing is
>     that
>     > this ends being confusing as in all machines I've been testing this
>     setting
>     > Mic Mute-LED Mode to "Follow Capture" actually makes it follow mute, as
>     > setting it to "On" turns the LED off.
>     > There is other setting called "mute_led_polarity" but this does not
>     work, as
>     > currently mic mute LED and mute LED do not follow the same logic.
>     > What I think may be causing confusion is "cxt_update_gpio_led" "enabled"
>     > parameter. Setting "enabled" to "true" sets the GPIO pin to 0 causing
>     the led
>     > to be turned off. I think "enabled" used to refer to the input capture
>     or
>     > output status and not to the LED being lit or not. Output or input not
>     enabled
>     > (enabled==false) caused the LED to be turned on.
>     > This logic in the function negates it on the GPIO output.
>     >
>     > if (enabled)
>     >     spec->gpio_led &= ~mask;
>     > else
>     >     spec->gpio_led |= mask;
>     >
>     > May be I can do a more comprehensive fix, reversing the behavior of 
>     > "cxt_update_gpio_led" "enabled" parameter to refer the GPIO output value
>     (
>     > enabled==true => GPIO pin output high )
>     > Then also modify the call to "cxt_update_gpio_led" in
>     > "cxt_fixup_gpio_mute_hook" to make it work consistently.
>    
>     OK, if the "On" turns the LED off, it's indeed inverted.
>     Then we'd need to consider both fixing the inverted behavior and the
>     default mic-mute mode.
>    
>     Could you confirm the following?
>    
>     - Which models and codecs are checked?
> 
> I've tested on HP ZBook 15U G3 (Conexant CX20724) and HP Probook 440 G4
> (Conexant CX8200)
> 
>     - GPIO pin high = mic LED on or off?
> 
>  GPIO pin high = mic LED on
> 
>     - How is the expected behavior on Windows?
>        Mute is on when mic is muted, or mute-on when mic is ready?
> 
> Mute led is on when mic is muted.

Thanks, it's clearer now.

Then I'd rather like to correct cxt_update_gpio_led(), i.e. taking the
argument led_on instead of enabled, and correct the caller in
cxt_fixup_gpio_mute_hook() to invert instead.  Something like below.
Could you retest and resubmit with that change?


thanks,

Takashi

--- a/sound/pci/hda/patch_conexant.c
+++ b/sound/pci/hda/patch_conexant.c
@@ -611,18 +611,18 @@ static void cxt_fixup_hp_gate_mic_jack(struct hda_codec *codec,
 
 /* update LED status via GPIO */
 static void cxt_update_gpio_led(struct hda_codec *codec, unsigned int mask,
-				bool enabled)
+				bool led_on)
 {
 	struct conexant_spec *spec = codec->spec;
 	unsigned int oldval = spec->gpio_led;
 
 	if (spec->mute_led_polarity)
-		enabled = !enabled;
+		led_on = !led_on;
 
-	if (enabled)
-		spec->gpio_led &= ~mask;
-	else
+	if (led_on)
 		spec->gpio_led |= mask;
+	else
+		spec->gpio_led &= ~mask;
 	if (spec->gpio_led != oldval)
 		snd_hda_codec_write(codec, 0x01, 0, AC_VERB_SET_GPIO_DATA,
 				    spec->gpio_led);
@@ -634,7 +634,8 @@ static void cxt_fixup_gpio_mute_hook(void *private_data, int enabled)
 	struct hda_codec *codec = private_data;
 	struct conexant_spec *spec = codec->spec;
 
-	cxt_update_gpio_led(codec, spec->gpio_mute_led_mask, enabled);
+	/* muted -> LED on */
+	cxt_update_gpio_led(codec, spec->gpio_mute_led_mask, !enabled);
 }
 
 /* turn on/off mic-mute LED via GPIO per capture hook */
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[PATCH v2] ALSA: hda - Fixes inverted Conexant GPIO mic mute led

[jeronimo]

From: Jeronimo Borque <jeronimo@borque.com.ar>

"enabled" parameter historically referred to the device input or
output, not to the led indicator. After the changes added with the led
helper functions the mic mute led logic refers to the led and not to
the mic input which caused led indicator to be negated.
Fixing logic in cxt_update_gpio_led and updated
cxt_fixup_gpio_mute_hook
Also updated debug messages to ease further debugging if necessary.

Suggested-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Jeronimo Borque <jeronimo@borque.com.ar>
---
 sound/pci/hda/patch_conexant.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/sound/pci/hda/patch_conexant.c b/sound/pci/hda/patch_conexant.c
index 14298ef45b21..d1c93dfa158b 100644
--- a/sound/pci/hda/patch_conexant.c
+++ b/sound/pci/hda/patch_conexant.c
@@ -611,18 +611,20 @@ static void cxt_fixup_hp_gate_mic_jack(struct hda_codec *codec,
 
 /* update LED status via GPIO */
 static void cxt_update_gpio_led(struct hda_codec *codec, unsigned int mask,
-				bool enabled)
+				bool led_on)
 {
 	struct conexant_spec *spec = codec->spec;
 	unsigned int oldval = spec->gpio_led;
 
 	if (spec->mute_led_polarity)
-		enabled = !enabled;
+		led_on = !led_on;
 
-	if (enabled)
-		spec->gpio_led &= ~mask;
-	else
+	if (led_on)
 		spec->gpio_led |= mask;
+	else
+		spec->gpio_led &= ~mask;
+	codec_dbg(codec, "mask:%d enabled:%d gpio_led:%d\n
+			mask, led_on, spec->gpio_led);
 	if (spec->gpio_led != oldval)
 		snd_hda_codec_write(codec, 0x01, 0, AC_VERB_SET_GPIO_DATA,
 				    spec->gpio_led);
@@ -633,8 +635,8 @@ static void cxt_fixup_gpio_mute_hook(void *private_data, int enabled)
 {
 	struct hda_codec *codec = private_data;
 	struct conexant_spec *spec = codec->spec;
-
-	cxt_update_gpio_led(codec, spec->gpio_mute_led_mask, enabled);
+	/* muted -> LED on */
+	cxt_update_gpio_led(codec, spec->gpio_mute_led_mask, !enabled);
 }
 
 /* turn on/off mic-mute LED via GPIO per capture hook */
@@ -656,7 +658,6 @@ static void cxt_fixup_mute_led_gpio(struct hda_codec *codec,
 		{ 0x01, AC_VERB_SET_GPIO_DIRECTION, 0x03 },
 		{}
 	};
-	codec_info(codec, "action: %d gpio_led: %d\n", action, spec->gpio_led);
 
 	if (action == HDA_FIXUP_ACT_PRE_PROBE) {
 		spec->gen.vmaster_mute.hook = cxt_fixup_gpio_mute_hook;
-- 
2.21.0

Re: [PATCH v2] ALSA: hda - Fixes inverted Conexant GPIO mic mute led

[Jerónimo Borque]

I'm sorry. I've sent a wrong patch file.
I'll resubmit the right patch shortly.
Thanks,
Jerónimo

El dom., 18 de ago. de 2019 a la(s) 18:23, <jeronimo@borque.com.ar>
escribió:

> From: Jeronimo Borque <jeronimo@borque.com.ar>
>
> "enabled" parameter historically referred to the device input or
> output, not to the led indicator. After the changes added with the led
> helper functions the mic mute led logic refers to the led and not to
> the mic input which caused led indicator to be negated.
> Fixing logic in cxt_update_gpio_led and updated
> cxt_fixup_gpio_mute_hook
> Also updated debug messages to ease further debugging if necessary.
>
> Suggested-by: Takashi Iwai <tiwai@suse.de>
> Signed-off-by: Jeronimo Borque <jeronimo@borque.com.ar>
> ---
>  sound/pci/hda/patch_conexant.c | 17 +++++++++--------
>  1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/sound/pci/hda/patch_conexant.c
> b/sound/pci/hda/patch_conexant.c
> index 14298ef45b21..d1c93dfa158b 100644
> --- a/sound/pci/hda/patch_conexant.c
> +++ b/sound/pci/hda/patch_conexant.c
> @@ -611,18 +611,20 @@ static void cxt_fixup_hp_gate_mic_jack(struct
> hda_codec *codec,
>
>  /* update LED status via GPIO */
>  static void cxt_update_gpio_led(struct hda_codec *codec, unsigned int
> mask,
> -                               bool enabled)
> +                               bool led_on)
>  {
>         struct conexant_spec *spec = codec->spec;
>         unsigned int oldval = spec->gpio_led;
>
>         if (spec->mute_led_polarity)
> -               enabled = !enabled;
> +               led_on = !led_on;
>
> -       if (enabled)
> -               spec->gpio_led &= ~mask;
> -       else
> +       if (led_on)
>                 spec->gpio_led |= mask;
> +       else
> +               spec->gpio_led &= ~mask;
> +       codec_dbg(codec, "mask:%d enabled:%d gpio_led:%d\n
> +                       mask, led_on, spec->gpio_led);
>         if (spec->gpio_led != oldval)
>                 snd_hda_codec_write(codec, 0x01, 0, AC_VERB_SET_GPIO_DATA,
>                                     spec->gpio_led);
> @@ -633,8 +635,8 @@ static void cxt_fixup_gpio_mute_hook(void
> *private_data, int enabled)
>  {
>         struct hda_codec *codec = private_data;
>         struct conexant_spec *spec = codec->spec;
> -
> -       cxt_update_gpio_led(codec, spec->gpio_mute_led_mask, enabled);
> +       /* muted -> LED on */
> +       cxt_update_gpio_led(codec, spec->gpio_mute_led_mask, !enabled);
>  }
>
>  /* turn on/off mic-mute LED via GPIO per capture hook */
> @@ -656,7 +658,6 @@ static void cxt_fixup_mute_led_gpio(struct hda_codec
> *codec,
>                 { 0x01, AC_VERB_SET_GPIO_DIRECTION, 0x03 },
>                 {}
>         };
> -       codec_info(codec, "action: %d gpio_led: %d\n", action,
> spec->gpio_led);
>
>         if (action == HDA_FIXUP_ACT_PRE_PROBE) {
>                 spec->gen.vmaster_mute.hook = cxt_fixup_gpio_mute_hook;
> --
> 2.21.0
>
>
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[PATCH v3] ALSA: hda - Fixes inverted Conexant GPIO mic mute led

[jeronimo]

From: Jeronimo Borque <jeronimo@borque.com.ar>

"enabled" parameter historically referred to the device input or
output, not to the led indicator. After the changes added with the led
helper functions the mic mute led logic refers to the led and not to
the mic input which caused led indicator to be negated.
Fixing logic in cxt_update_gpio_led and updated
cxt_fixup_gpio_mute_hook
Also updated debug messages to ease further debugging if necessary.

Suggested-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Jeronimo Borque <jeronimo@borque.com.ar>
---
 sound/pci/hda/patch_conexant.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/sound/pci/hda/patch_conexant.c b/sound/pci/hda/patch_conexant.c
index 14298ef45b21..968d3caab6ac 100644
--- a/sound/pci/hda/patch_conexant.c
+++ b/sound/pci/hda/patch_conexant.c
@@ -611,18 +611,20 @@ static void cxt_fixup_hp_gate_mic_jack(struct hda_codec *codec,
 
 /* update LED status via GPIO */
 static void cxt_update_gpio_led(struct hda_codec *codec, unsigned int mask,
-				bool enabled)
+				bool led_on)
 {
 	struct conexant_spec *spec = codec->spec;
 	unsigned int oldval = spec->gpio_led;
 
 	if (spec->mute_led_polarity)
-		enabled = !enabled;
+		led_on = !led_on;
 
-	if (enabled)
-		spec->gpio_led &= ~mask;
-	else
+	if (led_on)
 		spec->gpio_led |= mask;
+	else
+		spec->gpio_led &= ~mask;
+	codec_dbg(codec, "mask:%d enabled:%d gpio_led:%d\n",
+			mask, led_on, spec->gpio_led);
 	if (spec->gpio_led != oldval)
 		snd_hda_codec_write(codec, 0x01, 0, AC_VERB_SET_GPIO_DATA,
 				    spec->gpio_led);
@@ -633,8 +635,8 @@ static void cxt_fixup_gpio_mute_hook(void *private_data, int enabled)
 {
 	struct hda_codec *codec = private_data;
 	struct conexant_spec *spec = codec->spec;
-
-	cxt_update_gpio_led(codec, spec->gpio_mute_led_mask, enabled);
+	/* muted -> LED on */
+	cxt_update_gpio_led(codec, spec->gpio_mute_led_mask, !enabled);
 }
 
 /* turn on/off mic-mute LED via GPIO per capture hook */
@@ -656,7 +658,6 @@ static void cxt_fixup_mute_led_gpio(struct hda_codec *codec,
 		{ 0x01, AC_VERB_SET_GPIO_DIRECTION, 0x03 },
 		{}
 	};
-	codec_info(codec, "action: %d gpio_led: %d\n", action, spec->gpio_led);
 
 	if (action == HDA_FIXUP_ACT_PRE_PROBE) {
 		spec->gen.vmaster_mute.hook = cxt_fixup_gpio_mute_hook;
-- 
2.21.0

Re: [PATCH v3] ALSA: hda - Fixes inverted Conexant GPIO mic mute led

[Takashi Iwai]

On Mon, 19 Aug 2019 03:35:38 +0200,
jeronimo@borque.com.ar wrote:
> 
> From: Jeronimo Borque <jeronimo@borque.com.ar>
> 
> "enabled" parameter historically referred to the device input or
> output, not to the led indicator. After the changes added with the led
> helper functions the mic mute led logic refers to the led and not to
> the mic input which caused led indicator to be negated.
> Fixing logic in cxt_update_gpio_led and updated
> cxt_fixup_gpio_mute_hook
> Also updated debug messages to ease further debugging if necessary.
> 
> Suggested-by: Takashi Iwai <tiwai@suse.de>
> Signed-off-by: Jeronimo Borque <jeronimo@borque.com.ar>

Applied now with Cc to stable.  Thanks.


Takashi