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).
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.