On Wed, Sep 14, 2016 at 8:11 PM, Anton Lindqvist <anton.lindqv...@gmail.com> wrote: > On Tue, Sep 13, 2016 at 10:48:26AM -0600, Theo de Raadt wrote: >> > Anton Lindqvist wrote: >> > > I'm trying to fix a minor annoyance on my x240: the speaker mute key >> > > LED-state is not respected at boot. Pressing the mute key will mute the >> > > speaker while the expected behavior is to unmute. The LED-state will >> > > remain out-of-sync until I run `mixerctl -t outputs.master.mute`. >> > > >> > > I've managed to determine if the speaker is muted in the acpithinkpad(4) >> > > attach function by reading from the embedded controller. However, >> > > calling wskbd_set_mixervolume at this stage returns ENODEV. I assume the >> > > audio device has not been attached yet. I'm new to kernel development >> > > and therefore wonder if this approach makes sense. If true, is it >> > > possible to postpone a task to run once a certain device has attached? >> > >> > The function you're looking for is startuphook_establish. >> > >> > But this requires crazy amounts of testing, since many thinkpads will be >> > different. It's well known that the mute button, hardware state, and >> > software >> > state can became desynced, but the specifics vary by model. >> > >> >> Actually, I think Anton might be onto something here. The desyncronization >> may be due to the audio driver + BIOS not recognizing the other's state. > > By using startuphook_establish, as proposed by tedu@, I managed to get > it working. Booting with the volume muted before reboot and the volume > not muted before reboot both behaves correctly. > > A few comments regarding the patch: > > - At first, I tried sticking all code in the startuphook function > because it felt like a good approach to keep all the logic in one > place and reduce the number of ifdef-conditionals. However, that > caused my kernel to halt since the EC is busy at this point and the > assertion in acpiec.c:337 failed. The only other usage of acpiec_read > I can find is related to polling the sensors. I therefore kept volume > reading from the EC inside the attached function and moved it prior > attaching the sensors. The kernel then continued successfully, but I'm > still worried a that potential race still can occur. Or is the action > taken enough to make the volume EC read atomic? > > - I didn't bother checking the return value of startuphook_establish > since I couldn't find any other invocation in the kernel doing so. > > - The VOLUME_MUTE_MASK is taken from the FreeBSD ThinkPad ACPI driver. > > If the patch looks good, then I would like to add some logging prior > asking people to try it out.
Works as intended on my X220. Thanks! David
