[alsa-devel] [PATCH] ALSA: hda - Fix inconsistent Mic mute LED

[Takashi Iwai]

At Fri, 31 Jan 2014 18:28:35 +0100,
David Henningsson wrote:
> 
> On 01/31/2014 06:02 PM, Takashi Iwai wrote:
> > At Fri, 31 Jan 2014 17:52:55 +0100,
> > David Henningsson wrote:
> >>
> >> On 01/30/2014 06:04 PM, Takashi Iwai wrote:
> >>> The current code for controlling mic mute LED in patch_sigmatel.c
> >>> blindly assumes that there is a single capture switch.  But, there can
> >>> be multiple multiple ones, and each of them flips the state, ended up
> >>> in an inconsistent state.
> >>>
> >>> For fixing this problem, this patch adds kcontrol to be passed to the
> >>> hook function so that the callee can check which switch is being
> >>> accessed.  In stac_capture_led_hook(), the state is checked as a
> >>> bitmask, and turns on the LED when all capture switches are off.
> >>
> >> Is capture switch with non-zero indices turned off by default (and not
> >> turned on by e g alsactl init)? Otherwise it sounds like this could be
> >> regressing OOTB behaviour, i e, the mic mute LED worked with
> >> people/PulseAudio just toggling Capture Switch 0, but now requires
> >> manual toggling of the other capture switches...?
> > 
> > I thought alsactl init doesn't take all captures on but only the
> > first capture.
> > 
> > I don't mind to change the behavior only to check the first capture,
> > and ignore the rest.  Although it sounds a bit inconsistent, it might
> > be slightly better aligned to the existing behavior.
> 
> From a security perspective the bitmask would be better. I think we just
> need to double check that the Capture Switch 1 and 2 are not enabled by
> default (in both kernel code and alsactl init).

Well, what if it were default on?  Then we should take a security
risk?  So, here is the question.  If we regard this security risk more
serious, we should take the first one.  If we regard the compatibility
more important, we need to take the second.

> >> Anyway, the same reasoning would potentially apply to Conexant and
> >> Realtek, so perhaps it's time to move some more of mic mute LED handling
> >> into hda_generic?
> > 
> > The hook itself can be used not only for the LED.  So, filtering the
> > action in the generic parser side doesn't fit, IMO.
> 
> My thought were more on the lines of moving the bitmask check to the
> generic parser, and add a new "toggle_mic_mute_led(bool on)" hook. In
> order to minimise the duplication of code between
> conexant/sigmatel/realtek codecs.

Yes, but I prefer keeping simplicity and flexibility there.  The
callback is still simple.  Now it receives kcontrol in addition, and
that's all.


Takashi