ThinkPad T480s & LED_MUTE, LED_MICMUTE

ThinkPad T480s & LED_MUTE, LED_MICMUTE

[Pali Rohár]

Hi! With up-to-date thinkpad_acpi.ko driver on ThinkPad T480s I'm seeing
a strange behavior of LEDs which are integrated into mic mute (Fn+F4)
and mute (Fn+F1) keys.

When thinkpad_acpi.ko is not loaded, then mute key is working fine. When
pressed, it correctly generates KEY_MUTE on AT Translated Set 2 keyboard
input device and also turn on/of mute led. But when micmute key is
pressed then, nothing happen. No key event is reported and also led is
not turned on/off.

On the other hand, when thinkpad_acpi.ko is loaded, then both buttons
mute and micmute correctly generates input events; mute via AT keyboard
and micmute via ThinkPad Extra Buttons. But led is not changed. When
thinkpad_acpi.ko is loaded it turn off both leds (mute and micmute) and
leds after pressing any of those buttons, leds are not turned on again.

When thinkpad_acpi.ko is unloaded, then pressing mute button again start
switching led on/off.

So it seems that some init sequence of thinkpad_acpi.ko breaks mute led.
And fini sequence of thinkpad_acpi.ko makes mute led working again.

-- 
Pali Rohár
pali.rohar@gmail.com

Re: ThinkPad T480s & LED_MUTE, LED_MICMUTE

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 1340 bytes --]

Hi!

> Hi! With up-to-date thinkpad_acpi.ko driver on ThinkPad T480s I'm seeing
> a strange behavior of LEDs which are integrated into mic mute (Fn+F4)
> and mute (Fn+F1) keys.
> 
> When thinkpad_acpi.ko is not loaded, then mute key is working fine. When
> pressed, it correctly generates KEY_MUTE on AT Translated Set 2 keyboard
> input device and also turn on/of mute led. But when micmute key is
> pressed then, nothing happen. No key event is reported and also led is
> not turned on/off.
> 
> On the other hand, when thinkpad_acpi.ko is loaded, then both buttons
> mute and micmute correctly generates input events; mute via AT keyboard
> and micmute via ThinkPad Extra Buttons. But led is not changed. When
> thinkpad_acpi.ko is loaded it turn off both leds (mute and micmute) and
> leds after pressing any of those buttons, leds are not turned on again.

With thinkpad_acpi.ko loaded, kernel should handle the LEDs, right? Do
we have a support for that? Do they have directories in
/sys/class/leds? What are the triggers there?

With thinkpad_acpi.ko unloaded, hardware drives the LEDs, so nothing
for us to do...

       	     	      	     	   	    	     	 	   Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: ThinkPad T480s & LED_MUTE, LED_MICMUTE

[Pali Rohár]

On Friday 15 June 2018 13:26:06 Pavel Machek wrote:
> Hi!
> 
> > Hi! With up-to-date thinkpad_acpi.ko driver on ThinkPad T480s I'm seeing
> > a strange behavior of LEDs which are integrated into mic mute (Fn+F4)
> > and mute (Fn+F1) keys.
> > 
> > When thinkpad_acpi.ko is not loaded, then mute key is working fine. When
> > pressed, it correctly generates KEY_MUTE on AT Translated Set 2 keyboard
> > input device and also turn on/of mute led. But when micmute key is
> > pressed then, nothing happen. No key event is reported and also led is
> > not turned on/off.
> > 
> > On the other hand, when thinkpad_acpi.ko is loaded, then both buttons
> > mute and micmute correctly generates input events; mute via AT keyboard
> > and micmute via ThinkPad Extra Buttons. But led is not changed. When
> > thinkpad_acpi.ko is loaded it turn off both leds (mute and micmute) and
> > leds after pressing any of those buttons, leds are not turned on again.
> 
> With thinkpad_acpi.ko loaded, kernel should handle the LEDs, right?

I suppose. But I would be happy even with working "hardware" controlling
(which is working fine when thinkpad_acpi.ko is unloaded).

> Do we have a support for that?

In thinkpad_acpi.c there are some TPACPI_LED_MUTE and TPACPI_LED_MICMUTE
keywords. And also function static int mute_led_on_off(). So some kind
of support there is.

> Do they have directories in /sys/class/leds? What are the triggers there?

No.

$ ll /sys/class/leds
total 0
drwxr-xr-x  2 root root 0 Jun 15 13:27 ./
drwxr-xr-x 51 root root 0 Jun 15 13:27 ../
lrwxrwxrwx  1 root root 0 Jun 15 13:27 input0::capslock -> ../../devices/platform/i8042/serio0/input/input0/input0::capslock/
lrwxrwxrwx  1 root root 0 Jun 15 13:27 input0::numlock -> ../../devices/platform/i8042/serio0/input/input0/input0::numlock/
lrwxrwxrwx  1 root root 0 Jun 15 13:27 input0::scrolllock -> ../../devices/platform/i8042/serio0/input/input0/input0::scrolllock/
lrwxrwxrwx  1 root root 0 Jun 15 13:27 phy0-led -> ../../devices/pci0000:00/0000:00:1c.6/0000:3d:00.0/leds/phy0-led/
lrwxrwxrwx  1 root root 0 Jun 15 13:27 tpacpi::kbd_backlight -> ../../devices/platform/thinkpad_acpi/leds/tpacpi::kbd_backlight/
lrwxrwxrwx  1 root root 0 Jun 15 13:27 tpacpi::power -> ../../devices/platform/thinkpad_acpi/leds/tpacpi::power/
lrwxrwxrwx  1 root root 0 Jun 15 13:27 tpacpi::standby -> ../../devices/platform/thinkpad_acpi/leds/tpacpi::standby/
lrwxrwxrwx  1 root root 0 Jun 15 13:27 tpacpi::thinklight -> ../../devices/platform/thinkpad_acpi/leds/tpacpi::thinklight/
lrwxrwxrwx  1 root root 0 Jun 15 13:27 tpacpi::thinkvantage -> ../../devices/platform/thinkpad_acpi/leds/tpacpi::thinkvantage/

But looks like that thinkpad_acpi.c does not define any struct
led_classdev nor struct tpacpi_led_classdev for MUTE or MICMUTE.

Henrique, any idea why there are no exported led classes for mute and
micmute? And how are suppose to be controlled?

> With thinkpad_acpi.ko unloaded, hardware drives the LEDs, so nothing
> for us to do...

So somehow tell thinkpad_acpi.ko to let hardware control those LEDs when
thinkpad_acpi.ko is loaded?

-- 
Pali Rohár
pali.rohar@gmail.com

Re: ThinkPad T480s & LED_MUTE, LED_MICMUTE

[Henrique de Moraes Holschuh]

On Fri, 15 Jun 2018, Pali Rohár wrote:
> Henrique, any idea why there are no exported led classes for mute and
> micmute? And how are suppose to be controlled?

I have to look into the code, it was contributed by someone who had
access to the proper hardware to test it.

But the way *I* would like it to work is this:

1. When implemented in *hardware* or *EC*, let the hardware and EC take
   full control, and never allow the operating system to mess with it.
   So, it becomes much harder for that LED to "lie".

2. Otherwise implement it in-kernel, so that userspace cannot unmute
   when the human has activated the "mute" switch, and the LED cannot be
   controlled by userspace to lie (report mute when it is not mute).

It might, or might not be possible to achieve the above.

> > With thinkpad_acpi.ko unloaded, hardware drives the LEDs, so nothing
> > for us to do...
> 
> So somehow tell thinkpad_acpi.ko to let hardware control those LEDs when
> thinkpad_acpi.ko is loaded?

I.e. look into the DSDT and XSDT, to find out what it is doing.  It will
be there: it is very rare for the thinkpad EC itself to implement these
behavior changes.

-- 
  Henrique Holschuh

Re: ThinkPad T480s & LED_MUTE, LED_MICMUTE

[Pali Rohár]

[-- Attachment #1: Type: text/plain, Size: 4670 bytes --]

On Friday 15 June 2018 09:30:07 Henrique de Moraes Holschuh wrote:
> On Fri, 15 Jun 2018, Pali Rohár wrote:
> > Henrique, any idea why there are no exported led classes for mute and
> > micmute? And how are suppose to be controlled?
> 
> I have to look into the code, it was contributed by someone who had
> access to the proper hardware to test it.
> 
> But the way *I* would like it to work is this:
> 
> 1. When implemented in *hardware* or *EC*, let the hardware and EC take
>    full control, and never allow the operating system to mess with it.
>    So, it becomes much harder for that LED to "lie".

This means that kernel should not export any led class device. Or when
exported, then "set" operation should always fail.

> 2. Otherwise implement it in-kernel, so that userspace cannot unmute
>    when the human has activated the "mute" switch, and the LED cannot be
>    controlled by userspace to lie (report mute when it is not mute).

This looks like a good candidate to use led "trigger" interface. Create
a mute trigger and attach it to that led device.

I hope that this is what Pavel means.

> It might, or might not be possible to achieve the above.
> 
> > > With thinkpad_acpi.ko unloaded, hardware drives the LEDs, so nothing
> > > for us to do...
> > 
> > So somehow tell thinkpad_acpi.ko to let hardware control those LEDs when
> > thinkpad_acpi.ko is loaded?
> 
> I.e. look into the DSDT and XSDT, to find out what it is doing.  It will
> be there: it is very rare for the thinkpad EC itself to implement these
> behavior changes.

DSDT helped. Thanks!

Function with "EC.LED" name is self explaining and also "EC.HKEY". I
used acpi_call.ko for debugging and here are results how to control
these two leds on ThinkPad T480s:

        Scope (\_SB.PCI0.LPCB.EC.HKEY)
        {
            Method (MMTG, 0, NotSerialized)
            {
                Local0 = 0x0101
                If (HDMC)
                {
                    Local0 |= 0x00010000
                }

                Return (Local0)
            }

            Method (MMTS, 1, NotSerialized)
            {
                If (HDMC)
                {
                    Noop
                }
                ElseIf (Arg0 == 0x02)
                {
                    \_SB.PCI0.LPCB.EC.LED (0x0E, 0x80)
                }
                ElseIf (Arg0 == 0x03)
                {
                    \_SB.PCI0.LPCB.EC.LED (0x0E, 0xC0)
                }
                Else
                {
                    \_SB.PCI0.LPCB.EC.LED (0x0E, 0x00)
                }
            }
        }

        Scope (\_SB.PCI0.LPCB.EC.HKEY)
        {
            Method (GSMS, 1, NotSerialized)
            {
                Return (\AUDC (0x00, 0x00))
            }

            Method (SSMS, 1, NotSerialized)
            {
                Return (\AUDC (0x01, (Arg0 & 0x01)))
            }

            Method (SHDA, 1, NotSerialized)
            {
                Local0 = Arg0
                If ((OSYS >= 0x07DF) && (Local0 == 0x01))
                {
                    Local0 = 0x02
                }

                Return (\AUDC (0x02, (Local0 & 0x03)))
            }
        }

    Method (AUDC, 2, NotSerialized)
    {
        Return (SMI (0x14, 0x07, Arg0, Arg1, 0x00))
    }

MMTS controls mic mute led. When Arg0 is 0x02 then mic mute led is
turned on. When it is 0x03 then it starts blinking. And otherwise is
turned off. MM in name probably means MicMute and S as set. I guess that
MMTG (G as get) would return capabilities as it returns constant.

blink: echo "\_SB.PCI0.LPCB.EC.HKEY.MMTS 3" > /proc/acpi/call
on:    echo "\_SB.PCI0.LPCB.EC.HKEY.MMTS 2" > /proc/acpi/call
off:   echo "\_SB.PCI0.LPCB.EC.HKEY.MMTS 0" > /proc/acpi/call

SSMS controls mute led. It calls some SMI method via AUDC. When Arg0 has
first bit set to one then mute led is turned on. When set to zero then
is turned off. GSMS takes one parameter, but ignores it. When mute led
is turned off then it returns 0x100. When mute led is turned on then it
return 0x101. So looks like that "G" in GSMS means "get" and "S" in SSMS
means set. What is SHDA doing, I have not figured out. It does not
change GSMS result nor led status. When called with argument 1..10 then
it returns always returned 0x0 except for 3 and 7 it returned
0x80000000. There is no blinking support (or at least I have not figured
out).

on:    echo "\_SB.PCI0.LPCB.EC.HKEY.SSMS 1" > /proc/acpi/call
off:   echo "\_SB.PCI0.LPCB.EC.HKEY.SSMS 0" > /proc/acpi/call

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re: ThinkPad T480s & LED_MUTE, LED_MICMUTE

[Henrique de Moraes Holschuh]

On Fri, 15 Jun 2018, Pali Rohár wrote:
> This means that kernel should not export any led class device. Or when
> exported, then "set" operation should always fail.

"not export" is right.

> > 2. Otherwise implement it in-kernel, so that userspace cannot unmute
> >    when the human has activated the "mute" switch, and the LED cannot be
> >    controlled by userspace to lie (report mute when it is not mute).
> 
> This looks like a good candidate to use led "trigger" interface. Create
> a mute trigger and attach it to that led device.

Maybe, as long as done in-kernel and not possible to mess with from
userspace.

> means set. What is SHDA doing, I have not figured out. It does not

Look at the HDA mixer state, SHDA is likely to be directly messing with
it.

-- 
  Henrique Holschuh

Re: ThinkPad T480s & LED_MUTE, LED_MICMUTE

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 1280 bytes --]

Hi!

On Fri 2018-06-15 20:36:28, Henrique de Moraes Holschuh wrote:
> On Fri, 15 Jun 2018, Pali Rohár wrote:
> > This means that kernel should not export any led class device. Or when
> > exported, then "set" operation should always fail.
> 
> "not export" is right.
> 
> > > 2. Otherwise implement it in-kernel, so that userspace cannot unmute
> > >    when the human has activated the "mute" switch, and the LED cannot be
> > >    controlled by userspace to lie (report mute when it is not mute).
> > 
> > This looks like a good candidate to use led "trigger" interface. Create
> > a mute trigger and attach it to that led device.
> 
> Maybe, as long as done in-kernel and not possible to mess with from
> userspace.

Question is if we want flexibility or security.

If we want security, going through LED subsystem makes no sense, just
control the LED as hardware would, or let hardware do it.

For full flexibility, just export the LED and use normal mechanisms we
have (such as triggers). root should be allowed to configure the LEDs,
and he can change the kernel, too, so...

Best regards,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: ThinkPad T480s & LED_MUTE, LED_MICMUTE

[Henrique de Moraes Holschuh]

On Sat, 16 Jun 2018, Pavel Machek wrote:
> Question is if we want flexibility or security.

For thinkpad-acpi, it is security by default.  Flexibility is allowed as
*compile*-time (Kconfig) option, and only as long as it defaults to
secure *and* the help text is very explicit at instructing distros to
NOT enable this option.

That said, if mic-mute is under HDA mixer control, it will only be
"secure" if ALSA is also blocking userspace access to the relevant bits,
and if that is not possible to do properly, might as well go for
flexibility.  But only in that case.

That would be why I use phisical shutters on embedded webcams unless the
hardware actually airgaps them from the USB bus when the privacy switch
is active.

-- 
  Henrique Holschuh

Re: ThinkPad T480s & LED_MUTE, LED_MICMUTE

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 976 bytes --]

Hi!

> > Question is if we want flexibility or security.
> 
> For thinkpad-acpi, it is security by default.  Flexibility is allowed as
> *compile*-time (Kconfig) option, and only as long as it defaults to
> secure *and* the help text is very explicit at instructing distros to
> NOT enable this option.

Yep, no problem with that.

> That said, if mic-mute is under HDA mixer control, it will only be
> "secure" if ALSA is also blocking userspace access to the relevant bits,
> and if that is not possible to do properly, might as well go for
> flexibility.  But only in that case.

That also makes sense.

> That would be why I use phisical shutters on embedded webcams unless the
> hardware actually airgaps them from the USB bus when the privacy switch
> is active.

And yes, I use them, too.
									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: ThinkPad T480s & LED_MUTE, LED_MICMUTE

[Pali Rohár]

On Friday 15 June 2018 20:36:28 Henrique de Moraes Holschuh wrote:
> On Fri, 15 Jun 2018, Pali Rohár wrote:
> > means set. What is SHDA doing, I have not figured out. It does not
> 
> Look at the HDA mixer state, SHDA is likely to be directly messing with
> it.

Now I found following email thread between you and Alex with patch which
configures HDA mixer via that SHDA method:

https://www.spinics.net/lists/platform-driver-x86/msg03206.html

But looks like that patch was never included into mainline kernel...

Seems it would be needed in some part (to not break older Thinkpads) for
that LED support in alsa mixer about which Takashi Iwai wrote.

-- 
Pali Rohár
pali.rohar@gmail.com

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel

Re: ThinkPad T480s & LED_MUTE, LED_MICMUTE

[Pali Rohár]

On Friday 15 June 2018 20:36:28 Henrique de Moraes Holschuh wrote:
> On Fri, 15 Jun 2018, Pali Rohár wrote:
> > means set. What is SHDA doing, I have not figured out. It does not
> 
> Look at the HDA mixer state, SHDA is likely to be directly messing with
> it.

Now I found following email thread between you and Alex with patch which
configures HDA mixer via that SHDA method:

https://www.spinics.net/lists/platform-driver-x86/msg03206.html

But looks like that patch was never included into mainline kernel...

Seems it would be needed in some part (to not break older Thinkpads) for
that LED support in alsa mixer about which Takashi Iwai wrote.

-- 
Pali Rohár
pali.rohar@gmail.com

Re: ThinkPad T480s & LED_MUTE, LED_MICMUTE

[Pali Rohár]

On Monday 18 June 2018 12:11:44 Pali Rohár wrote:
> On Friday 15 June 2018 20:36:28 Henrique de Moraes Holschuh wrote:
> > On Fri, 15 Jun 2018, Pali Rohár wrote:
> > > means set. What is SHDA doing, I have not figured out. It does not
> > 
> > Look at the HDA mixer state, SHDA is likely to be directly messing with
> > it.
> 
> Now I found following email thread between you and Alex with patch which
> configures HDA mixer via that SHDA method:
> 
> https://www.spinics.net/lists/platform-driver-x86/msg03206.html
> 
> But looks like that patch was never included into mainline kernel...
> 
> Seems it would be needed in some part (to not break older Thinkpads) for
> that LED support in alsa mixer about which Takashi Iwai wrote.

Hi Alex, do you have other details or documentation for your patch
"thinkpad_acpi: added BIOS mute interfaces for volume" (linked above)
which you sent to platform-driver-x86 list 6 years ago?

It calls SHDA thinkpad ACPI function and I would like to know more what
is this function doing...

-- 
Pali Rohár
pali.rohar@gmail.com

Re: ThinkPad T480s & LED_MUTE, LED_MICMUTE

[Takashi Iwai]

On Fri, 08 Jun 2018 13:10:57 +0200,
Pali Rohár wrote:
> 
> Hi! With up-to-date thinkpad_acpi.ko driver on ThinkPad T480s I'm seeing
> a strange behavior of LEDs which are integrated into mic mute (Fn+F4)
> and mute (Fn+F1) keys.
> 
> When thinkpad_acpi.ko is not loaded, then mute key is working fine. When
> pressed, it correctly generates KEY_MUTE on AT Translated Set 2 keyboard
> input device and also turn on/of mute led. But when micmute key is
> pressed then, nothing happen. No key event is reported and also led is
> not turned on/off.
> 
> On the other hand, when thinkpad_acpi.ko is loaded, then both buttons
> mute and micmute correctly generates input events; mute via AT keyboard
> and micmute via ThinkPad Extra Buttons. But led is not changed. When
> thinkpad_acpi.ko is loaded it turn off both leds (mute and micmute) and
> leds after pressing any of those buttons, leds are not turned on again.
> 
> When thinkpad_acpi.ko is unloaded, then pressing mute button again start
> switching led on/off.
> 
> So it seems that some init sequence of thinkpad_acpi.ko breaks mute led.
> And fini sequence of thinkpad_acpi.ko makes mute led working again.

Usually the mute LED on Thinkpad is triggered from HD-audio driver
(sound/pci/hda/thinkpad_helper.c), and it's a soft-bound via
symbol_request(tpacpi_led_set).  I thought thinkpad_acpi is
auto-loaded when the module gets bound.

A possible explanation would be that TPT480s has neither IBM0068,
LEN0068 nor LEN0268 ACPI HIDs, hence the driver is not auto-loaded.
(In HD-audio driver side, the ACPI ID is checked and the mute LED
 control is applied only to these three IDs, too.)
Meanwhile, when you load thinkpad_acpi, it does still recognize some
device and initialize it.  By the initialization, it goes out of BIOS
control, and the OS control is expected...   This is my wild guess.


BTW, the reason we have no LED class for these is that we don't want
to confuse users by providing multiple ways to access to the single
stuff.  We've had already the mute LED control from the audio driver
since long time ago, we don't want to drop and enforce the user-space
solution (that is anyway flakier than in kernel in most cases).


thanks,

Takashi

Re: ThinkPad T480s & LED_MUTE, LED_MICMUTE

[Pali Rohár]

[-- Attachment #1: Type: text/plain, Size: 3636 bytes --]

On Friday 15 June 2018 14:51:47 Takashi Iwai wrote:
> On Fri, 08 Jun 2018 13:10:57 +0200,
> Pali Rohár wrote:
> > 
> > Hi! With up-to-date thinkpad_acpi.ko driver on ThinkPad T480s I'm seeing
> > a strange behavior of LEDs which are integrated into mic mute (Fn+F4)
> > and mute (Fn+F1) keys.
> > 
> > When thinkpad_acpi.ko is not loaded, then mute key is working fine. When
> > pressed, it correctly generates KEY_MUTE on AT Translated Set 2 keyboard
> > input device and also turn on/of mute led. But when micmute key is
> > pressed then, nothing happen. No key event is reported and also led is
> > not turned on/off.
> > 
> > On the other hand, when thinkpad_acpi.ko is loaded, then both buttons
> > mute and micmute correctly generates input events; mute via AT keyboard
> > and micmute via ThinkPad Extra Buttons. But led is not changed. When
> > thinkpad_acpi.ko is loaded it turn off both leds (mute and micmute) and
> > leds after pressing any of those buttons, leds are not turned on again.
> > 
> > When thinkpad_acpi.ko is unloaded, then pressing mute button again start
> > switching led on/off.
> > 
> > So it seems that some init sequence of thinkpad_acpi.ko breaks mute led.
> > And fini sequence of thinkpad_acpi.ko makes mute led working again.
> 
> Usually the mute LED on Thinkpad is triggered from HD-audio driver
> (sound/pci/hda/thinkpad_helper.c), and it's a soft-bound via
> symbol_request(tpacpi_led_set).  I thought thinkpad_acpi is
> auto-loaded when the module gets bound.
> 
> A possible explanation would be that TPT480s has neither IBM0068,
> LEN0068 nor LEN0268 ACPI HIDs, hence the driver is not auto-loaded.

I have Debian Stretch kernel (4.9) which does not have LEN0268 alias for
thinkpad_acpi.ko. So thinkpad_acpi.ko is not loaded automatically. But I
have put thinkpad_acpi into /etc/modules and it is now automatically
loaded at boot.

I also compiled upstream version of thinkpad_acpi.ko, loaded it in
Stretch kernel, but it behaves in same way.

Maybe... there could be a problem that thinkpad_acpi.ko must be already
loaded when sound subsystem is doing initialization? If yes, this could
explain it as /etc/modules is loaded at later stage and manually loading
of new version of thinkpad_acpi.ko at runtime does not help when sound
subsystem is already running.

> (In HD-audio driver side, the ACPI ID is checked and the mute LED
>  control is applied only to these three IDs, too.)
> Meanwhile, when you load thinkpad_acpi, it does still recognize some
> device and initialize it.  By the initialization, it goes out of BIOS
> control, and the OS control is expected...   This is my wild guess.
> 
> 
> BTW, the reason we have no LED class for these is that we don't want
> to confuse users by providing multiple ways to access to the single
> stuff.  We've had already the mute LED control from the audio driver
> since long time ago, we don't want to drop and enforce the user-space
> solution (that is anyway flakier than in kernel in most cases).

If possible... I would prefer swapped LED behavior: turn led on when
microphone is turned on AND turn led off when microphone is turned off.
Because current behavior is to have turned led off when microphone is
on which seems a bit strange. Also microphone is in majority of time not
used and when is not used it is (or should be) turned off. On the other
hand e.g. CAPSLOCK led is turned on when CAPSLOCK is enabled (and not
opposite) -- which matches uses, it is not used in majority of time.

> 
> thanks,
> 
> Takashi

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re: ThinkPad T480s & LED_MUTE, LED_MICMUTE

[Takashi Iwai]

On Fri, 15 Jun 2018 21:09:59 +0200,
Pali Rohár wrote:
> 
> On Friday 15 June 2018 14:51:47 Takashi Iwai wrote:
> > On Fri, 08 Jun 2018 13:10:57 +0200,
> > Pali Rohár wrote:
> > > 
> > > Hi! With up-to-date thinkpad_acpi.ko driver on ThinkPad T480s I'm seeing
> > > a strange behavior of LEDs which are integrated into mic mute (Fn+F4)
> > > and mute (Fn+F1) keys.
> > > 
> > > When thinkpad_acpi.ko is not loaded, then mute key is working fine. When
> > > pressed, it correctly generates KEY_MUTE on AT Translated Set 2 keyboard
> > > input device and also turn on/of mute led. But when micmute key is
> > > pressed then, nothing happen. No key event is reported and also led is
> > > not turned on/off.
> > > 
> > > On the other hand, when thinkpad_acpi.ko is loaded, then both buttons
> > > mute and micmute correctly generates input events; mute via AT keyboard
> > > and micmute via ThinkPad Extra Buttons. But led is not changed. When
> > > thinkpad_acpi.ko is loaded it turn off both leds (mute and micmute) and
> > > leds after pressing any of those buttons, leds are not turned on again.
> > > 
> > > When thinkpad_acpi.ko is unloaded, then pressing mute button again start
> > > switching led on/off.
> > > 
> > > So it seems that some init sequence of thinkpad_acpi.ko breaks mute led.
> > > And fini sequence of thinkpad_acpi.ko makes mute led working again.
> > 
> > Usually the mute LED on Thinkpad is triggered from HD-audio driver
> > (sound/pci/hda/thinkpad_helper.c), and it's a soft-bound via
> > symbol_request(tpacpi_led_set).  I thought thinkpad_acpi is
> > auto-loaded when the module gets bound.
> > 
> > A possible explanation would be that TPT480s has neither IBM0068,
> > LEN0068 nor LEN0268 ACPI HIDs, hence the driver is not auto-loaded.
> 
> I have Debian Stretch kernel (4.9) which does not have LEN0268 alias for
> thinkpad_acpi.ko. So thinkpad_acpi.ko is not loaded automatically. But I
> have put thinkpad_acpi into /etc/modules and it is now automatically
> loaded at boot.

That's odd.  It's exposed via
  MODULE_DEVICE_TABLE(acpi, ibm_htk_device_ids);

It's been already in 4.9.  At this point, something is fishy.

> I also compiled upstream version of thinkpad_acpi.ko, loaded it in
> Stretch kernel, but it behaves in same way.
>
> Maybe... there could be a problem that thinkpad_acpi.ko must be already
> loaded when sound subsystem is doing initialization? If yes, this could
> explain it as /etc/modules is loaded at later stage and manually loading
> of new version of thinkpad_acpi.ko at runtime does not help when sound
> subsystem is already running.

Not really.  The HD-audio driver tries to bind with tpacpi_led_set()
via symbol_request().  i.e. if it's not present, it tries to load a
module.

Check whether hda_fixup_thinkpad_acpi() is called and the symbol gets
loaded or not.

But, I don't think it's worth to debug such an old kernel primarily.
Could you test the latest Linus tree or 4.17.x at least as a test
basis?

> > (In HD-audio driver side, the ACPI ID is checked and the mute LED
> >  control is applied only to these three IDs, too.)
> > Meanwhile, when you load thinkpad_acpi, it does still recognize some
> > device and initialize it.  By the initialization, it goes out of BIOS
> > control, and the OS control is expected...   This is my wild guess.
> > 
> > 
> > BTW, the reason we have no LED class for these is that we don't want
> > to confuse users by providing multiple ways to access to the single
> > stuff.  We've had already the mute LED control from the audio driver
> > since long time ago, we don't want to drop and enforce the user-space
> > solution (that is anyway flakier than in kernel in most cases).
> 
> If possible... I would prefer swapped LED behavior: turn led on when
> microphone is turned on AND turn led off when microphone is turned off.
> Because current behavior is to have turned led off when microphone is
> on which seems a bit strange. Also microphone is in majority of time not
> used and when is not used it is (or should be) turned off. On the other
> hand e.g. CAPSLOCK led is turned on when CAPSLOCK is enabled (and not
> opposite) -- which matches uses, it is not used in majority of time.

Currently the mixer control to change such LED behavior is provided
only for Dell laptops.  It'd be trivial to port to Thinkpad, though.
I can provide a patch once after you test with the recent kernels.


thanks,

Takashi

Re: ThinkPad T480s & LED_MUTE, LED_MICMUTE

[Pali Rohár]

[-- Attachment #1: Type: text/plain, Size: 5119 bytes --]

On Saturday 16 June 2018 09:05:41 Takashi Iwai wrote:
> On Fri, 15 Jun 2018 21:09:59 +0200,
> Pali Rohár wrote:
> > 
> > On Friday 15 June 2018 14:51:47 Takashi Iwai wrote:
> > > On Fri, 08 Jun 2018 13:10:57 +0200,
> > > Pali Rohár wrote:
> > > > 
> > > > Hi! With up-to-date thinkpad_acpi.ko driver on ThinkPad T480s I'm seeing
> > > > a strange behavior of LEDs which are integrated into mic mute (Fn+F4)
> > > > and mute (Fn+F1) keys.
> > > > 
> > > > When thinkpad_acpi.ko is not loaded, then mute key is working fine. When
> > > > pressed, it correctly generates KEY_MUTE on AT Translated Set 2 keyboard
> > > > input device and also turn on/of mute led. But when micmute key is
> > > > pressed then, nothing happen. No key event is reported and also led is
> > > > not turned on/off.
> > > > 
> > > > On the other hand, when thinkpad_acpi.ko is loaded, then both buttons
> > > > mute and micmute correctly generates input events; mute via AT keyboard
> > > > and micmute via ThinkPad Extra Buttons. But led is not changed. When
> > > > thinkpad_acpi.ko is loaded it turn off both leds (mute and micmute) and
> > > > leds after pressing any of those buttons, leds are not turned on again.
> > > > 
> > > > When thinkpad_acpi.ko is unloaded, then pressing mute button again start
> > > > switching led on/off.
> > > > 
> > > > So it seems that some init sequence of thinkpad_acpi.ko breaks mute led.
> > > > And fini sequence of thinkpad_acpi.ko makes mute led working again.
> > > 
> > > Usually the mute LED on Thinkpad is triggered from HD-audio driver
> > > (sound/pci/hda/thinkpad_helper.c), and it's a soft-bound via
> > > symbol_request(tpacpi_led_set).  I thought thinkpad_acpi is
> > > auto-loaded when the module gets bound.
> > > 
> > > A possible explanation would be that TPT480s has neither IBM0068,
> > > LEN0068 nor LEN0268 ACPI HIDs, hence the driver is not auto-loaded.
> > 
> > I have Debian Stretch kernel (4.9) which does not have LEN0268 alias for
> > thinkpad_acpi.ko. So thinkpad_acpi.ko is not loaded automatically. But I
> > have put thinkpad_acpi into /etc/modules and it is now automatically
> > loaded at boot.
> 
> That's odd.  It's exposed via
>   MODULE_DEVICE_TABLE(acpi, ibm_htk_device_ids);
> 
> It's been already in 4.9.  At this point, something is fishy.

$ /sbin/modinfo thinkpad_acpi | grep alias
alias:          dmi:bvnIBM:bvrI[MU]ET??WW*
alias:          tpacpi
alias:          acpi*:LEN0068:*
alias:          acpi*:IBM0068:*

No there is no LEN0268 on 4.9.

> > I also compiled upstream version of thinkpad_acpi.ko, loaded it in
> > Stretch kernel, but it behaves in same way.
> >
> > Maybe... there could be a problem that thinkpad_acpi.ko must be already
> > loaded when sound subsystem is doing initialization? If yes, this could
> > explain it as /etc/modules is loaded at later stage and manually loading
> > of new version of thinkpad_acpi.ko at runtime does not help when sound
> > subsystem is already running.
> 
> Not really.  The HD-audio driver tries to bind with tpacpi_led_set()
> via symbol_request().  i.e. if it's not present, it tries to load a
> module.
> 
> Check whether hda_fixup_thinkpad_acpi() is called and the symbol gets
> loaded or not.
> 
> But, I don't think it's worth to debug such an old kernel primarily.

It is default one used by the last released Debian stable version.

> Could you test the latest Linus tree or 4.17.x at least as a test
> basis?

Ok, will do that later.

> > > (In HD-audio driver side, the ACPI ID is checked and the mute LED
> > >  control is applied only to these three IDs, too.)
> > > Meanwhile, when you load thinkpad_acpi, it does still recognize some
> > > device and initialize it.  By the initialization, it goes out of BIOS
> > > control, and the OS control is expected...   This is my wild guess.
> > > 
> > > 
> > > BTW, the reason we have no LED class for these is that we don't want
> > > to confuse users by providing multiple ways to access to the single
> > > stuff.  We've had already the mute LED control from the audio driver
> > > since long time ago, we don't want to drop and enforce the user-space
> > > solution (that is anyway flakier than in kernel in most cases).
> > 
> > If possible... I would prefer swapped LED behavior: turn led on when
> > microphone is turned on AND turn led off when microphone is turned off.
> > Because current behavior is to have turned led off when microphone is
> > on which seems a bit strange. Also microphone is in majority of time not
> > used and when is not used it is (or should be) turned off. On the other
> > hand e.g. CAPSLOCK led is turned on when CAPSLOCK is enabled (and not
> > opposite) -- which matches uses, it is not used in majority of time.
> 
> Currently the mixer control to change such LED behavior is provided
> only for Dell laptops.  It'd be trivial to port to Thinkpad, though.
> I can provide a patch once after you test with the recent kernels.
> 
> 
> thanks,
> 
> Takashi

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re: ThinkPad T480s & LED_MUTE, LED_MICMUTE

[Takashi Iwai]

On Sat, 16 Jun 2018 17:43:09 +0200,
Pali Rohár wrote:
> 
> On Saturday 16 June 2018 09:05:41 Takashi Iwai wrote:
> > On Fri, 15 Jun 2018 21:09:59 +0200,
> > Pali Rohár wrote:
> > > 
> > > On Friday 15 June 2018 14:51:47 Takashi Iwai wrote:
> > > > On Fri, 08 Jun 2018 13:10:57 +0200,
> > > > Pali Rohár wrote:
> > > > > 
> > > > > Hi! With up-to-date thinkpad_acpi.ko driver on ThinkPad T480s I'm seeing
> > > > > a strange behavior of LEDs which are integrated into mic mute (Fn+F4)
> > > > > and mute (Fn+F1) keys.
> > > > > 
> > > > > When thinkpad_acpi.ko is not loaded, then mute key is working fine. When
> > > > > pressed, it correctly generates KEY_MUTE on AT Translated Set 2 keyboard
> > > > > input device and also turn on/of mute led. But when micmute key is
> > > > > pressed then, nothing happen. No key event is reported and also led is
> > > > > not turned on/off.
> > > > > 
> > > > > On the other hand, when thinkpad_acpi.ko is loaded, then both buttons
> > > > > mute and micmute correctly generates input events; mute via AT keyboard
> > > > > and micmute via ThinkPad Extra Buttons. But led is not changed. When
> > > > > thinkpad_acpi.ko is loaded it turn off both leds (mute and micmute) and
> > > > > leds after pressing any of those buttons, leds are not turned on again.
> > > > > 
> > > > > When thinkpad_acpi.ko is unloaded, then pressing mute button again start
> > > > > switching led on/off.
> > > > > 
> > > > > So it seems that some init sequence of thinkpad_acpi.ko breaks mute led.
> > > > > And fini sequence of thinkpad_acpi.ko makes mute led working again.
> > > > 
> > > > Usually the mute LED on Thinkpad is triggered from HD-audio driver
> > > > (sound/pci/hda/thinkpad_helper.c), and it's a soft-bound via
> > > > symbol_request(tpacpi_led_set).  I thought thinkpad_acpi is
> > > > auto-loaded when the module gets bound.
> > > > 
> > > > A possible explanation would be that TPT480s has neither IBM0068,
> > > > LEN0068 nor LEN0268 ACPI HIDs, hence the driver is not auto-loaded.
> > > 
> > > I have Debian Stretch kernel (4.9) which does not have LEN0268 alias for
> > > thinkpad_acpi.ko. So thinkpad_acpi.ko is not loaded automatically. But I
> > > have put thinkpad_acpi into /etc/modules and it is now automatically
> > > loaded at boot.
> > 
> > That's odd.  It's exposed via
> >   MODULE_DEVICE_TABLE(acpi, ibm_htk_device_ids);
> > 
> > It's been already in 4.9.  At this point, something is fishy.
> 
> $ /sbin/modinfo thinkpad_acpi | grep alias
> alias:          dmi:bvnIBM:bvrI[MU]ET??WW*
> alias:          tpacpi
> alias:          acpi*:LEN0068:*
> alias:          acpi*:IBM0068:*
> 
> No there is no LEN0268 on 4.9.

OK, that's the cause.  It's really old.

The commit a3c42a467a25 ("platform/x86: thinkpad_acpi: Adding new
hotkey ID for Lenovo thinkpad") has to be backported.
Also, in the HD-audio side, the commit 2ecb704a1290 ("ALSA: hda - add
a new condition to check if it is thinkpad") is needed, too.

> > > I also compiled upstream version of thinkpad_acpi.ko, loaded it in
> > > Stretch kernel, but it behaves in same way.
> > >
> > > Maybe... there could be a problem that thinkpad_acpi.ko must be already
> > > loaded when sound subsystem is doing initialization? If yes, this could
> > > explain it as /etc/modules is loaded at later stage and manually loading
> > > of new version of thinkpad_acpi.ko at runtime does not help when sound
> > > subsystem is already running.
> > 
> > Not really.  The HD-audio driver tries to bind with tpacpi_led_set()
> > via symbol_request().  i.e. if it's not present, it tries to load a
> > module.
> > 
> > Check whether hda_fixup_thinkpad_acpi() is called and the symbol gets
> > loaded or not.
> > 
> > But, I don't think it's worth to debug such an old kernel primarily.
> 
> It is default one used by the last released Debian stable version.

Heh, that explains :)

And there was a recent regression in HD-audio that was addressed in
4.9.104.  If you're using some earlier 4.9.x, you might hit the
problem regarding HD-audio thinkpad_acpi binding.
(But I guess it doesn't work in anyway without the backport of the
 commit above.)

> > Could you test the latest Linus tree or 4.17.x at least as a test
> > basis?
> 
> Ok, will do that later.

If my analysis above is correct, everything should work with the
recent upstream kernel as is.

Once after you confirm it, I can cook a patch to add the mixer enum to
change LED behavior as you wanted.


Takashi

Re: ThinkPad T480s & LED_MUTE, LED_MICMUTE

[Pali Rohár]

On Saturday 16 June 2018 18:02:14 Takashi Iwai wrote:
> On Sat, 16 Jun 2018 17:43:09 +0200,
> Pali Rohár wrote:
> > 
> > On Saturday 16 June 2018 09:05:41 Takashi Iwai wrote:
> > > On Fri, 15 Jun 2018 21:09:59 +0200,
> > > Pali Rohár wrote:
> > > > 
> > > > On Friday 15 June 2018 14:51:47 Takashi Iwai wrote:
> > > > > On Fri, 08 Jun 2018 13:10:57 +0200,
> > > > > Pali Rohár wrote:
> > > > > > 
> > > > > > Hi! With up-to-date thinkpad_acpi.ko driver on ThinkPad T480s I'm seeing
> > > > > > a strange behavior of LEDs which are integrated into mic mute (Fn+F4)
> > > > > > and mute (Fn+F1) keys.
> > > > > > 
> > > > > > When thinkpad_acpi.ko is not loaded, then mute key is working fine. When
> > > > > > pressed, it correctly generates KEY_MUTE on AT Translated Set 2 keyboard
> > > > > > input device and also turn on/of mute led. But when micmute key is
> > > > > > pressed then, nothing happen. No key event is reported and also led is
> > > > > > not turned on/off.
> > > > > > 
> > > > > > On the other hand, when thinkpad_acpi.ko is loaded, then both buttons
> > > > > > mute and micmute correctly generates input events; mute via AT keyboard
> > > > > > and micmute via ThinkPad Extra Buttons. But led is not changed. When
> > > > > > thinkpad_acpi.ko is loaded it turn off both leds (mute and micmute) and
> > > > > > leds after pressing any of those buttons, leds are not turned on again.
> > > > > > 
> > > > > > When thinkpad_acpi.ko is unloaded, then pressing mute button again start
> > > > > > switching led on/off.
> > > > > > 
> > > > > > So it seems that some init sequence of thinkpad_acpi.ko breaks mute led.
> > > > > > And fini sequence of thinkpad_acpi.ko makes mute led working again.
> > > > > 
> > > > > Usually the mute LED on Thinkpad is triggered from HD-audio driver
> > > > > (sound/pci/hda/thinkpad_helper.c), and it's a soft-bound via
> > > > > symbol_request(tpacpi_led_set).  I thought thinkpad_acpi is
> > > > > auto-loaded when the module gets bound.
> > > > > 
> > > > > A possible explanation would be that TPT480s has neither IBM0068,
> > > > > LEN0068 nor LEN0268 ACPI HIDs, hence the driver is not auto-loaded.
> > > > 
> > > > I have Debian Stretch kernel (4.9) which does not have LEN0268 alias for
> > > > thinkpad_acpi.ko. So thinkpad_acpi.ko is not loaded automatically. But I
> > > > have put thinkpad_acpi into /etc/modules and it is now automatically
> > > > loaded at boot.
> > > 
> > > That's odd.  It's exposed via
> > >   MODULE_DEVICE_TABLE(acpi, ibm_htk_device_ids);
> > > 
> > > It's been already in 4.9.  At this point, something is fishy.
> > 
> > $ /sbin/modinfo thinkpad_acpi | grep alias
> > alias:          dmi:bvnIBM:bvrI[MU]ET??WW*
> > alias:          tpacpi
> > alias:          acpi*:LEN0068:*
> > alias:          acpi*:IBM0068:*
> > 
> > No there is no LEN0268 on 4.9.
> 
> OK, that's the cause.  It's really old.
> 
> The commit a3c42a467a25 ("platform/x86: thinkpad_acpi: Adding new
> hotkey ID for Lenovo thinkpad") has to be backported.

This commit just autoloads thinkpad_acpi driver, right? So manual
modprobe (for now) is OK too?

> Also, in the HD-audio side, the commit 2ecb704a1290 ("ALSA: hda - add
> a new condition to check if it is thinkpad") is needed, too.

This commit was introduced in 4.9 and Debian kernel has it. I looked
into Debian source code and there is check for LEN0268 in file
sound/pci/hda/thinkpad_helper.c

> > > > I also compiled upstream version of thinkpad_acpi.ko, loaded it in
> > > > Stretch kernel, but it behaves in same way.
> > > >
> > > > Maybe... there could be a problem that thinkpad_acpi.ko must be already
> > > > loaded when sound subsystem is doing initialization? If yes, this could
> > > > explain it as /etc/modules is loaded at later stage and manually loading
> > > > of new version of thinkpad_acpi.ko at runtime does not help when sound
> > > > subsystem is already running.
> > > 
> > > Not really.  The HD-audio driver tries to bind with tpacpi_led_set()
> > > via symbol_request().  i.e. if it's not present, it tries to load a
> > > module.
> > > 
> > > Check whether hda_fixup_thinkpad_acpi() is called and the symbol gets
> > > loaded or not.
> > > 
> > > But, I don't think it's worth to debug such an old kernel primarily.
> > 
> > It is default one used by the last released Debian stable version.
> 
> Heh, that explains :)
> 
> And there was a recent regression in HD-audio that was addressed in
> 4.9.104.  If you're using some earlier 4.9.x, you might hit the
> problem regarding HD-audio thinkpad_acpi binding.

Debian has currently 4.9.88.

> (But I guess it doesn't work in anyway without the backport of the
>  commit above.)

Which change/regression it is? As 2ecb704a1290 is already part of 4.9 I
think this is a problem why it is not working...

> > > Could you test the latest Linus tree or 4.17.x at least as a test
> > > basis?
> > 
> > Ok, will do that later.
> 
> If my analysis above is correct, everything should work with the
> recent upstream kernel as is.
> 
> Once after you confirm it, I can cook a patch to add the mixer enum to
> change LED behavior as you wanted.
> 
> 
> Takashi

-- 
Pali Rohár
pali.rohar@gmail.com

Re: ThinkPad T480s & LED_MUTE, LED_MICMUTE

[Takashi Iwai]

On Mon, 18 Jun 2018 12:28:06 +0200,
Pali Rohár wrote:
> 
> On Saturday 16 June 2018 18:02:14 Takashi Iwai wrote:
> > On Sat, 16 Jun 2018 17:43:09 +0200,
> > Pali Rohár wrote:
> > > 
> > > On Saturday 16 June 2018 09:05:41 Takashi Iwai wrote:
> > > > On Fri, 15 Jun 2018 21:09:59 +0200,
> > > > Pali Rohár wrote:
> > > > > 
> > > > > On Friday 15 June 2018 14:51:47 Takashi Iwai wrote:
> > > > > > On Fri, 08 Jun 2018 13:10:57 +0200,
> > > > > > Pali Rohár wrote:
> > > > > > > 
> > > > > > > Hi! With up-to-date thinkpad_acpi.ko driver on ThinkPad T480s I'm seeing
> > > > > > > a strange behavior of LEDs which are integrated into mic mute (Fn+F4)
> > > > > > > and mute (Fn+F1) keys.
> > > > > > > 
> > > > > > > When thinkpad_acpi.ko is not loaded, then mute key is working fine. When
> > > > > > > pressed, it correctly generates KEY_MUTE on AT Translated Set 2 keyboard
> > > > > > > input device and also turn on/of mute led. But when micmute key is
> > > > > > > pressed then, nothing happen. No key event is reported and also led is
> > > > > > > not turned on/off.
> > > > > > > 
> > > > > > > On the other hand, when thinkpad_acpi.ko is loaded, then both buttons
> > > > > > > mute and micmute correctly generates input events; mute via AT keyboard
> > > > > > > and micmute via ThinkPad Extra Buttons. But led is not changed. When
> > > > > > > thinkpad_acpi.ko is loaded it turn off both leds (mute and micmute) and
> > > > > > > leds after pressing any of those buttons, leds are not turned on again.
> > > > > > > 
> > > > > > > When thinkpad_acpi.ko is unloaded, then pressing mute button again start
> > > > > > > switching led on/off.
> > > > > > > 
> > > > > > > So it seems that some init sequence of thinkpad_acpi.ko breaks mute led.
> > > > > > > And fini sequence of thinkpad_acpi.ko makes mute led working again.
> > > > > > 
> > > > > > Usually the mute LED on Thinkpad is triggered from HD-audio driver
> > > > > > (sound/pci/hda/thinkpad_helper.c), and it's a soft-bound via
> > > > > > symbol_request(tpacpi_led_set).  I thought thinkpad_acpi is
> > > > > > auto-loaded when the module gets bound.
> > > > > > 
> > > > > > A possible explanation would be that TPT480s has neither IBM0068,
> > > > > > LEN0068 nor LEN0268 ACPI HIDs, hence the driver is not auto-loaded.
> > > > > 
> > > > > I have Debian Stretch kernel (4.9) which does not have LEN0268 alias for
> > > > > thinkpad_acpi.ko. So thinkpad_acpi.ko is not loaded automatically. But I
> > > > > have put thinkpad_acpi into /etc/modules and it is now automatically
> > > > > loaded at boot.
> > > > 
> > > > That's odd.  It's exposed via
> > > >   MODULE_DEVICE_TABLE(acpi, ibm_htk_device_ids);
> > > > 
> > > > It's been already in 4.9.  At this point, something is fishy.
> > > 
> > > $ /sbin/modinfo thinkpad_acpi | grep alias
> > > alias:          dmi:bvnIBM:bvrI[MU]ET??WW*
> > > alias:          tpacpi
> > > alias:          acpi*:LEN0068:*
> > > alias:          acpi*:IBM0068:*
> > > 
> > > No there is no LEN0268 on 4.9.
> > 
> > OK, that's the cause.  It's really old.
> > 
> > The commit a3c42a467a25 ("platform/x86: thinkpad_acpi: Adding new
> > hotkey ID for Lenovo thinkpad") has to be backported.
> 
> This commit just autoloads thinkpad_acpi driver, right? So manual
> modprobe (for now) is OK too?

Well, the manual modprobe is superfluous if it's properly bound with
HD-audio driver.  The HD-audio driver calls symbol_request(), so it
should do modprobe by itself.

> > Also, in the HD-audio side, the commit 2ecb704a1290 ("ALSA: hda - add
> > a new condition to check if it is thinkpad") is needed, too.
> 
> This commit was introduced in 4.9 and Debian kernel has it. I looked
> into Debian source code and there is check for LEN0268 in file
> sound/pci/hda/thinkpad_helper.c

Then check whether the function gets really called.


> > > > > I also compiled upstream version of thinkpad_acpi.ko, loaded it in
> > > > > Stretch kernel, but it behaves in same way.
> > > > >
> > > > > Maybe... there could be a problem that thinkpad_acpi.ko must be already
> > > > > loaded when sound subsystem is doing initialization? If yes, this could
> > > > > explain it as /etc/modules is loaded at later stage and manually loading
> > > > > of new version of thinkpad_acpi.ko at runtime does not help when sound
> > > > > subsystem is already running.
> > > > 
> > > > Not really.  The HD-audio driver tries to bind with tpacpi_led_set()
> > > > via symbol_request().  i.e. if it's not present, it tries to load a
> > > > module.
> > > > 
> > > > Check whether hda_fixup_thinkpad_acpi() is called and the symbol gets
> > > > loaded or not.
> > > > 
> > > > But, I don't think it's worth to debug such an old kernel primarily.
> > > 
> > > It is default one used by the last released Debian stable version.
> > 
> > Heh, that explains :)
> > 
> > And there was a recent regression in HD-audio that was addressed in
> > 4.9.104.  If you're using some earlier 4.9.x, you might hit the
> > problem regarding HD-audio thinkpad_acpi binding.
> 
> Debian has currently 4.9.88.
> 
> > (But I guess it doesn't work in anyway without the backport of the
> >  commit above.)
> 
> Which change/regression it is? As 2ecb704a1290 is already part of 4.9 I
> think this is a problem why it is not working...

It's 71bff398b0d4 that is in 4.9.104.

But you should really try the latest upstream kernel before spending
too much time (not only yours but also others).


Takashi


> > > > Could you test the latest Linus tree or 4.17.x at least as a test
> > > > basis?
> > > 
> > > Ok, will do that later.
> > 
> > If my analysis above is correct, everything should work with the
> > recent upstream kernel as is.
> > 
> > Once after you confirm it, I can cook a patch to add the mixer enum to
> > change LED behavior as you wanted.
> > 
> > 
> > Takashi
> 
> -- 
> Pali Rohár
> pali.rohar@gmail.com
> 

Re: ThinkPad T480s & LED_MUTE, LED_MICMUTE

[Pali Rohár]

On Monday 18 June 2018 12:36:31 Takashi Iwai wrote:
> On Mon, 18 Jun 2018 12:28:06 +0200,
> Pali Rohár wrote:
> > 
> > On Saturday 16 June 2018 18:02:14 Takashi Iwai wrote:
> > > On Sat, 16 Jun 2018 17:43:09 +0200,
> > > Pali Rohár wrote:
> > > > 
> > > > On Saturday 16 June 2018 09:05:41 Takashi Iwai wrote:
> > > > > On Fri, 15 Jun 2018 21:09:59 +0200,
> > > > > Pali Rohár wrote:
> > > > > > 
> > > > > > On Friday 15 June 2018 14:51:47 Takashi Iwai wrote:
> > > > > > > On Fri, 08 Jun 2018 13:10:57 +0200,
> > > > > > > Pali Rohár wrote:
> > > > > > > > 
> > > > > > > > Hi! With up-to-date thinkpad_acpi.ko driver on ThinkPad T480s I'm seeing
> > > > > > > > a strange behavior of LEDs which are integrated into mic mute (Fn+F4)
> > > > > > > > and mute (Fn+F1) keys.
> > > > > > > > 
> > > > > > > > When thinkpad_acpi.ko is not loaded, then mute key is working fine. When
> > > > > > > > pressed, it correctly generates KEY_MUTE on AT Translated Set 2 keyboard
> > > > > > > > input device and also turn on/of mute led. But when micmute key is
> > > > > > > > pressed then, nothing happen. No key event is reported and also led is
> > > > > > > > not turned on/off.
> > > > > > > > 
> > > > > > > > On the other hand, when thinkpad_acpi.ko is loaded, then both buttons
> > > > > > > > mute and micmute correctly generates input events; mute via AT keyboard
> > > > > > > > and micmute via ThinkPad Extra Buttons. But led is not changed. When
> > > > > > > > thinkpad_acpi.ko is loaded it turn off both leds (mute and micmute) and
> > > > > > > > leds after pressing any of those buttons, leds are not turned on again.
> > > > > > > > 
> > > > > > > > When thinkpad_acpi.ko is unloaded, then pressing mute button again start
> > > > > > > > switching led on/off.
> > > > > > > > 
> > > > > > > > So it seems that some init sequence of thinkpad_acpi.ko breaks mute led.
> > > > > > > > And fini sequence of thinkpad_acpi.ko makes mute led working again.
> > > > > > > 
> > > > > > > Usually the mute LED on Thinkpad is triggered from HD-audio driver
> > > > > > > (sound/pci/hda/thinkpad_helper.c), and it's a soft-bound via
> > > > > > > symbol_request(tpacpi_led_set).  I thought thinkpad_acpi is
> > > > > > > auto-loaded when the module gets bound.
> > > > > > > 
> > > > > > > A possible explanation would be that TPT480s has neither IBM0068,
> > > > > > > LEN0068 nor LEN0268 ACPI HIDs, hence the driver is not auto-loaded.
> > > > > > 
> > > > > > I have Debian Stretch kernel (4.9) which does not have LEN0268 alias for
> > > > > > thinkpad_acpi.ko. So thinkpad_acpi.ko is not loaded automatically. But I
> > > > > > have put thinkpad_acpi into /etc/modules and it is now automatically
> > > > > > loaded at boot.
> > > > > 
> > > > > That's odd.  It's exposed via
> > > > >   MODULE_DEVICE_TABLE(acpi, ibm_htk_device_ids);
> > > > > 
> > > > > It's been already in 4.9.  At this point, something is fishy.
> > > > 
> > > > $ /sbin/modinfo thinkpad_acpi | grep alias
> > > > alias:          dmi:bvnIBM:bvrI[MU]ET??WW*
> > > > alias:          tpacpi
> > > > alias:          acpi*:LEN0068:*
> > > > alias:          acpi*:IBM0068:*
> > > > 
> > > > No there is no LEN0268 on 4.9.
> > > 
> > > OK, that's the cause.  It's really old.
> > > 
> > > The commit a3c42a467a25 ("platform/x86: thinkpad_acpi: Adding new
> > > hotkey ID for Lenovo thinkpad") has to be backported.
> > 
> > This commit just autoloads thinkpad_acpi driver, right? So manual
> > modprobe (for now) is OK too?
> 
> Well, the manual modprobe is superfluous if it's properly bound with
> HD-audio driver.  The HD-audio driver calls symbol_request(), so it
> should do modprobe by itself.
> 
> > > Also, in the HD-audio side, the commit 2ecb704a1290 ("ALSA: hda - add
> > > a new condition to check if it is thinkpad") is needed, too.
> > 
> > This commit was introduced in 4.9 and Debian kernel has it. I looked
> > into Debian source code and there is check for LEN0268 in file
> > sound/pci/hda/thinkpad_helper.c
> 
> Then check whether the function gets really called.

Checked, no it is not called.

Now I see that snd_hda_codec_generic is visible in lsmod and thinkpad
helper is compiled only into snd-hda-codec-realtek and
snd-hda-codec-conexant... Therefore nobody is even trying to use that
function.

And yes, sound playback and microphone recording is working.

> 
> > > > > > I also compiled upstream version of thinkpad_acpi.ko, loaded it in
> > > > > > Stretch kernel, but it behaves in same way.
> > > > > >
> > > > > > Maybe... there could be a problem that thinkpad_acpi.ko must be already
> > > > > > loaded when sound subsystem is doing initialization? If yes, this could
> > > > > > explain it as /etc/modules is loaded at later stage and manually loading
> > > > > > of new version of thinkpad_acpi.ko at runtime does not help when sound
> > > > > > subsystem is already running.
> > > > > 
> > > > > Not really.  The HD-audio driver tries to bind with tpacpi_led_set()
> > > > > via symbol_request().  i.e. if it's not present, it tries to load a
> > > > > module.
> > > > > 
> > > > > Check whether hda_fixup_thinkpad_acpi() is called and the symbol gets
> > > > > loaded or not.
> > > > > 
> > > > > But, I don't think it's worth to debug such an old kernel primarily.
> > > > 
> > > > It is default one used by the last released Debian stable version.
> > > 
> > > Heh, that explains :)
> > > 
> > > And there was a recent regression in HD-audio that was addressed in
> > > 4.9.104.  If you're using some earlier 4.9.x, you might hit the
> > > problem regarding HD-audio thinkpad_acpi binding.
> > 
> > Debian has currently 4.9.88.
> > 
> > > (But I guess it doesn't work in anyway without the backport of the
> > >  commit above.)
> > 
> > Which change/regression it is? As 2ecb704a1290 is already part of 4.9 I
> > think this is a problem why it is not working...
> 
> It's 71bff398b0d4 that is in 4.9.104.
> 
> But you should really try the latest upstream kernel before spending
> too much time (not only yours but also others).

Now I see that in Debian backports is some 4.16.12 kernel version. I can
try this one (and later compile one from Linus tree).

> 
> Takashi
> 
> 
> > > > > Could you test the latest Linus tree or 4.17.x at least as a test
> > > > > basis?
> > > > 
> > > > Ok, will do that later.
> > > 
> > > If my analysis above is correct, everything should work with the
> > > recent upstream kernel as is.
> > > 
> > > Once after you confirm it, I can cook a patch to add the mixer enum to
> > > change LED behavior as you wanted.
> > > 
> > > 
> > > Takashi
> > 
> > -- 
> > Pali Rohár
> > pali.rohar@gmail.com
> > 

-- 
Pali Rohár
pali.rohar@gmail.com

Re: ThinkPad T480s & LED_MUTE, LED_MICMUTE

[Pali Rohár]

On Monday 18 June 2018 13:21:21 Pali Rohár wrote:
> Now I see that in Debian backports is some 4.16.12 kernel version. I can
> try this one (and later compile one from Linus tree).

Tested, this version is working fine and leds works as expected. And now
also snd_hda_codec_realtek is loaded.

-- 
Pali Rohár
pali.rohar@gmail.com

Re: ThinkPad T480s & LED_MUTE, LED_MICMUTE

[Takashi Iwai]

[-- Attachment #1: Type: text/plain, Size: 553 bytes --]

On Mon, 18 Jun 2018 13:26:18 +0200,
Pali Rohár wrote:
> 
> On Monday 18 June 2018 13:21:21 Pali Rohár wrote:
> > Now I see that in Debian backports is some 4.16.12 kernel version. I can
> > try this one (and later compile one from Linus tree).
> 
> Tested, this version is working fine and leds works as expected. And now
> also snd_hda_codec_realtek is loaded.

Good to hear :)

Below are test patches to allow user choosing the mic mute LED
behavior on Thinkpad, too.  Note: totally untested!

Let me know if these work for you.


thanks,

Takashi


[-- Attachment #2: 0001-ALSA-hda-Move-mic-mute-LED-helper-to-the-generic-par.patch --]
[-- Type: application/octet-stream, Size: 10421 bytes --]

From 1cd2cac7e1fb62a733bcc3e202d5e68de8001b61 Mon Sep 17 00:00:00 2001
From: Takashi Iwai <tiwai@suse.de>
Date: Mon, 18 Jun 2018 17:26:12 +0200
Subject: [PATCH 1/2] ALSA: hda - Move mic mute LED helper to the generic
 parser

Move the code for setting up and controlling the mic mute LED hook
from dell-wmi helper to the generic parser, so that it can be referred
from the multiple driver codes.

No functional change.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/pci/hda/dell_wmi_helper.c | 115 ++------------------------
 sound/pci/hda/hda_generic.c     | 141 ++++++++++++++++++++++++++++++++
 sound/pci/hda/hda_generic.h     |  16 ++++
 3 files changed, 163 insertions(+), 109 deletions(-)

diff --git a/sound/pci/hda/dell_wmi_helper.c b/sound/pci/hda/dell_wmi_helper.c
index 1b48a8c19d28..28780772008e 100644
--- a/sound/pci/hda/dell_wmi_helper.c
+++ b/sound/pci/hda/dell_wmi_helper.c
@@ -6,107 +6,15 @@
 #if IS_ENABLED(CONFIG_DELL_LAPTOP)
 #include <linux/dell-led.h>
 
-enum {
-	MICMUTE_LED_ON,
-	MICMUTE_LED_OFF,
-	MICMUTE_LED_FOLLOW_CAPTURE,
-	MICMUTE_LED_FOLLOW_MUTE,
-};
-
-static int dell_led_mode = MICMUTE_LED_FOLLOW_MUTE;
-static int dell_capture;
-static int dell_led_value;
 static int (*dell_micmute_led_set_func)(int);
-static void (*dell_old_cap_hook)(struct hda_codec *,
-			         struct snd_kcontrol *,
-				 struct snd_ctl_elem_value *);
-
-static void call_micmute_led_update(void)
-{
-	int val;
-
-	switch (dell_led_mode) {
-	case MICMUTE_LED_ON:
-		val = 1;
-		break;
-	case MICMUTE_LED_OFF:
-		val = 0;
-		break;
-	case MICMUTE_LED_FOLLOW_CAPTURE:
-		val = dell_capture;
-		break;
-	case MICMUTE_LED_FOLLOW_MUTE:
-	default:
-		val = !dell_capture;
-		break;
-	}
-
-	if (val == dell_led_value)
-		return;
-	dell_led_value = val;
-	dell_micmute_led_set_func(dell_led_value);
-}
-
-static void update_dell_wmi_micmute_led(struct hda_codec *codec,
-				        struct snd_kcontrol *kcontrol,
-					struct snd_ctl_elem_value *ucontrol)
-{
-	if (dell_old_cap_hook)
-		dell_old_cap_hook(codec, kcontrol, ucontrol);
-
-	if (!ucontrol || !dell_micmute_led_set_func)
-		return;
-	if (strcmp("Capture Switch", ucontrol->id.name) == 0 && ucontrol->id.index == 0) {
-		/* TODO: How do I verify if it's a mono or stereo here? */
-		dell_capture = (ucontrol->value.integer.value[0] ||
-				ucontrol->value.integer.value[1]);
-		call_micmute_led_update();
-	}
-}
 
-static int dell_mic_mute_led_mode_info(struct snd_kcontrol *kcontrol,
-				       struct snd_ctl_elem_info *uinfo)
+static void dell_micmute_update(struct hda_codec *codec)
 {
-	static const char * const texts[] = {
-		"On", "Off", "Follow Capture", "Follow Mute",
-	};
-
-	return snd_ctl_enum_info(uinfo, 1, ARRAY_SIZE(texts), texts);
-}
+	struct hda_gen_spec *spec = codec->spec;
 
-static int dell_mic_mute_led_mode_get(struct snd_kcontrol *kcontrol,
-				      struct snd_ctl_elem_value *ucontrol)
-{
-	ucontrol->value.enumerated.item[0] = dell_led_mode;
-	return 0;
+	dell_micmute_led_set_func(spec->micmute_led.led_value);
 }
 
-static int dell_mic_mute_led_mode_put(struct snd_kcontrol *kcontrol,
-				      struct snd_ctl_elem_value *ucontrol)
-{
-	unsigned int mode;
-
-	mode = ucontrol->value.enumerated.item[0];
-	if (mode > MICMUTE_LED_FOLLOW_MUTE)
-		mode = MICMUTE_LED_FOLLOW_MUTE;
-	if (mode == dell_led_mode)
-		return 0;
-	dell_led_mode = mode;
-	call_micmute_led_update();
-	return 1;
-}
-
-static const struct snd_kcontrol_new dell_mic_mute_mode_ctls[] = {
-	{
-		.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
-		.name = "Mic Mute-LED Mode",
-		.info = dell_mic_mute_led_mode_info,
-		.get = dell_mic_mute_led_mode_get,
-		.put = dell_mic_mute_led_mode_put,
-	},
-	{}
-};
-
 static void alc_fixup_dell_wmi(struct hda_codec *codec,
 			       const struct hda_fixup *fix, int action)
 {
@@ -121,25 +29,14 @@ static void alc_fixup_dell_wmi(struct hda_codec *codec,
 			return;
 		}
 
-		removefunc = true;
-		if (dell_micmute_led_set_func(false) >= 0) {
-			dell_led_value = 0;
-			if (spec->gen.num_adc_nids > 1 && !spec->gen.dyn_adc_switch)
-				codec_dbg(codec, "Skipping micmute LED control due to several ADCs");
-			else {
-				dell_old_cap_hook = spec->gen.cap_sync_hook;
-				spec->gen.cap_sync_hook = update_dell_wmi_micmute_led;
-				removefunc = false;
-				add_mixer(spec, dell_mic_mute_mode_ctls);
-			}
-		}
-
+		removefunc = (dell_micmute_led_set_func(false) < 0) ||
+			(snd_hda_gen_add_micmute_led(codec,
+						     dell_micmute_update) <= 0);
 	}
 
 	if (dell_micmute_led_set_func && (action == HDA_FIXUP_ACT_FREE || removefunc)) {
 		symbol_put(dell_micmute_led_set);
 		dell_micmute_led_set_func = NULL;
-		dell_old_cap_hook = NULL;
 	}
 }
 
diff --git a/sound/pci/hda/hda_generic.c b/sound/pci/hda/hda_generic.c
index db773e219aaa..707614e38aeb 100644
--- a/sound/pci/hda/hda_generic.c
+++ b/sound/pci/hda/hda_generic.c
@@ -3899,6 +3899,147 @@ static int parse_mic_boost(struct hda_codec *codec)
 	return 0;
 }
 
+/*
+ * mic mute LED hook helpers
+ */
+enum {
+	MICMUTE_LED_ON,
+	MICMUTE_LED_OFF,
+	MICMUTE_LED_FOLLOW_CAPTURE,
+	MICMUTE_LED_FOLLOW_MUTE,
+};
+
+static void call_micmute_led_update(struct hda_codec *codec)
+{
+	struct hda_gen_spec *spec = codec->spec;
+	unsigned int val;
+
+	switch (spec->micmute_led.led_mode) {
+	case MICMUTE_LED_ON:
+		val = 1;
+		break;
+	case MICMUTE_LED_OFF:
+		val = 0;
+		break;
+	case MICMUTE_LED_FOLLOW_CAPTURE:
+		val = spec->micmute_led.capture;
+		break;
+	case MICMUTE_LED_FOLLOW_MUTE:
+	default:
+		val = !spec->micmute_led.capture;
+		break;
+	}
+
+	if (val == spec->micmute_led.led_value)
+		return;
+	spec->micmute_led.led_value = val;
+	if (spec->micmute_led.update)
+		spec->micmute_led.update(codec);
+}
+
+
+static void update_micmute_led(struct hda_codec *codec,
+			       struct snd_kcontrol *kcontrol,
+			       struct snd_ctl_elem_value *ucontrol)
+{
+	struct hda_gen_spec *spec = codec->spec;
+
+	if (spec->micmute_led.old_hook)
+		spec->micmute_led.old_hook(codec, kcontrol, ucontrol);
+
+	if (!ucontrol)
+		return;
+	if (!strcmp("Capture Switch", ucontrol->id.name) &&
+	    !ucontrol->id.index) {
+		/* TODO: How do I verify if it's a mono or stereo here? */
+		spec->micmute_led.capture = (ucontrol->value.integer.value[0] ||
+					     ucontrol->value.integer.value[1]);
+		call_micmute_led_update(codec);
+	}
+}
+
+static int micmute_led_mode_info(struct snd_kcontrol *kcontrol,
+				 struct snd_ctl_elem_info *uinfo)
+{
+	static const char * const texts[] = {
+		"On", "Off", "Follow Capture", "Follow Mute",
+	};
+
+	return snd_ctl_enum_info(uinfo, 1, ARRAY_SIZE(texts), texts);
+}
+
+static int micmute_led_mode_get(struct snd_kcontrol *kcontrol,
+				struct snd_ctl_elem_value *ucontrol)
+{
+	struct hda_codec *codec = snd_kcontrol_chip(kcontrol);
+	struct hda_gen_spec *spec = codec->spec;
+
+	ucontrol->value.enumerated.item[0] = spec->micmute_led.led_mode;
+	return 0;
+}
+
+static int micmute_led_mode_put(struct snd_kcontrol *kcontrol,
+				struct snd_ctl_elem_value *ucontrol)
+{
+	struct hda_codec *codec = snd_kcontrol_chip(kcontrol);
+	struct hda_gen_spec *spec = codec->spec;
+	unsigned int mode;
+
+	mode = ucontrol->value.enumerated.item[0];
+	if (mode > MICMUTE_LED_FOLLOW_MUTE)
+		mode = MICMUTE_LED_FOLLOW_MUTE;
+	if (mode == spec->micmute_led.led_mode)
+		return 0;
+	spec->micmute_led.led_mode = mode;
+	call_micmute_led_update(codec);
+	return 1;
+}
+
+static const struct snd_kcontrol_new micmute_led_mode_ctl = {
+	.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
+	.name = "Mic Mute-LED Mode",
+	.info = micmute_led_mode_info,
+	.get = micmute_led_mode_get,
+	.put = micmute_led_mode_put,
+};
+
+/**
+ * snd_hda_gen_add_micmute_led - helper for setting up mic mute LED hook
+ * @codec: the HDA codec
+ * @hook: the callback for updating LED
+ *
+ * Called from the codec drivers for offering the mic mute LED controls.
+ * Only valid for a single ADC (or a single input).  When established, it
+ * sets up cap_sync_hook and triggers the callback at each time when the
+ * capture mixer switch changes.  The callback is supposed to update the LED
+ * accordingly.
+ *
+ * Returns 1 if the hook is established, 0 if skipped (no valid config), or
+ * a negative error code.
+ */
+int snd_hda_gen_add_micmute_led(struct hda_codec *codec,
+				void (*hook)(struct hda_codec *))
+{
+	struct hda_gen_spec *spec = codec->spec;
+
+	if (spec->num_adc_nids > 1 && !spec->dyn_adc_switch) {
+		codec_dbg(codec,
+			  "Skipping micmute LED control due to several ADCs");
+		return 0;
+	}
+
+	spec->micmute_led.led_mode = MICMUTE_LED_FOLLOW_MUTE;
+	spec->micmute_led.capture = 0;
+	spec->micmute_led.led_value = 0;
+	spec->micmute_led.old_hook = spec->cap_sync_hook;
+	spec->micmute_led.update = hook;
+	spec->cap_sync_hook = update_micmute_led;
+	if (!snd_hda_gen_add_kctl(spec, NULL, &micmute_led_mode_ctl))
+		return -ENOMEM;
+	return 1;
+}
+EXPORT_SYMBOL_GPL(snd_hda_gen_add_micmute_led);
+
 /*
  * parse digital I/Os and set up NIDs in BIOS auto-parse mode
  */
diff --git a/sound/pci/hda/hda_generic.h b/sound/pci/hda/hda_generic.h
index 61772317de46..10123664fa61 100644
--- a/sound/pci/hda/hda_generic.h
+++ b/sound/pci/hda/hda_generic.h
@@ -86,6 +86,16 @@ struct badness_table {
 extern const struct badness_table hda_main_out_badness;
 extern const struct badness_table hda_extra_out_badness;
 
+struct hda_micmute_hook {
+	unsigned int led_mode;
+	unsigned int capture;
+	unsigned int led_value;
+	void (*update)(struct hda_codec *codec);
+	void (*old_hook)(struct hda_codec *codec,
+			 struct snd_kcontrol *kcontrol,
+			 struct snd_ctl_elem_value *ucontrol);
+};
+
 struct hda_gen_spec {
 	char stream_name_analog[32];	/* analog PCM stream */
 	const struct hda_pcm_stream *stream_analog_playback;
@@ -276,6 +286,9 @@ struct hda_gen_spec {
 			      struct snd_kcontrol *kcontrol,
 			      struct snd_ctl_elem_value *ucontrol);
 
+	/* mic mute LED hook; called via cap_sync_hook */
+	struct hda_micmute_hook micmute_led;
+
 	/* PCM hooks */
 	void (*pcm_playback_hook)(struct hda_pcm_stream *hinfo,
 				  struct hda_codec *codec,
@@ -342,4 +355,7 @@ unsigned int snd_hda_gen_path_power_filter(struct hda_codec *codec,
 void snd_hda_gen_stream_pm(struct hda_codec *codec, hda_nid_t nid, bool on);
 int snd_hda_gen_fix_pin_power(struct hda_codec *codec, hda_nid_t pin);
 
+int snd_hda_gen_add_micmute_led(struct hda_codec *codec,
+				void (*hook)(struct hda_codec *));
+
 #endif /* __SOUND_HDA_GENERIC_H */
-- 
2.17.1


[-- Attachment #3: 0002-ALSA-hda-Use-the-common-helper-for-thinkpad_acpi-mic.patch --]
[-- Type: application/octet-stream, Size: 2513 bytes --]

From 2730426826f8dbc702efb0f4723abe7795d8e4a2 Mon Sep 17 00:00:00 2001
From: Takashi Iwai <tiwai@suse.de>
Date: Mon, 18 Jun 2018 17:28:45 +0200
Subject: [PATCH 2/2] ALSA: hda - Use the common helper for thinkpad_acpi mic
 mute LED handling

Use the new common helper for setting up and controlling the mic mute
LED over thinkpad_acpi.  This also provides a new mixer enum "Mic
Mute-LED Mode" (that was present only for Dell models), which allows
user to choose the mic mute LED behavior.  For example, if you want
the mic mute LED turned on only while mic is on, choose "Follow
Capture" there.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/pci/hda/thinkpad_helper.c | 27 ++++++++-------------------
 1 file changed, 8 insertions(+), 19 deletions(-)

diff --git a/sound/pci/hda/thinkpad_helper.c b/sound/pci/hda/thinkpad_helper.c
index 65bb3ac6af4c..97f49b751e6e 100644
--- a/sound/pci/hda/thinkpad_helper.c
+++ b/sound/pci/hda/thinkpad_helper.c
@@ -27,17 +27,11 @@ static void update_tpacpi_mute_led(void *private_data, int enabled)
 		led_set_func(TPACPI_LED_MUTE, !enabled);
 }
 
-static void update_tpacpi_micmute_led(struct hda_codec *codec,
-				      struct snd_kcontrol *kcontrol,
-				      struct snd_ctl_elem_value *ucontrol)
+static void update_tpacpi_micmute(struct hda_codec *codec)
 {
-	if (!ucontrol || !led_set_func)
-		return;
-	if (strcmp("Capture Switch", ucontrol->id.name) == 0 && ucontrol->id.index == 0) {
-		/* TODO: How do I verify if it's a mono or stereo here? */
-		bool val = ucontrol->value.integer.value[0] || ucontrol->value.integer.value[1];
-		led_set_func(TPACPI_LED_MICMUTE, !val);
-	}
+	struct hda_gen_spec *spec = codec->spec;
+
+	led_set_func(TPACPI_LED_MICMUTE, spec->micmute_led.led_value);
 }
 
 static void hda_fixup_thinkpad_acpi(struct hda_codec *codec,
@@ -63,15 +57,10 @@ static void hda_fixup_thinkpad_acpi(struct hda_codec *codec,
 			spec->vmaster_mute.hook = update_tpacpi_mute_led;
 			removefunc = false;
 		}
-		if (led_set_func(TPACPI_LED_MICMUTE, false) >= 0) {
-			if (spec->num_adc_nids > 1 && !spec->dyn_adc_switch)
-				codec_dbg(codec,
-					  "Skipping micmute LED control due to several ADCs");
-			else {
-				spec->cap_sync_hook = update_tpacpi_micmute_led;
-				removefunc = false;
-			}
-		}
+		if (led_set_func(TPACPI_LED_MICMUTE, false) >= 0 &&
+		    snd_hda_gen_add_micmute_led(codec,
+						update_tpacpi_micmute) > 0)
+			removefunc = false;
 	}
 
 	if (led_set_func && (action == HDA_FIXUP_ACT_FREE || removefunc)) {
-- 
2.17.1

Re: ThinkPad T480s & LED_MUTE, LED_MICMUTE

[Pali Rohár]

On Monday 18 June 2018 17:35:18 Takashi Iwai wrote:
> On Mon, 18 Jun 2018 13:26:18 +0200,
> Pali Rohár wrote:
> > 
> > On Monday 18 June 2018 13:21:21 Pali Rohár wrote:
> > > Now I see that in Debian backports is some 4.16.12 kernel version. I can
> > > try this one (and later compile one from Linus tree).
> > 
> > Tested, this version is working fine and leds works as expected. And now
> > also snd_hda_codec_realtek is loaded.
> 
> Good to hear :)
> 
> Below are test patches to allow user choosing the mic mute LED
> behavior on Thinkpad, too.  Note: totally untested!
> 
> Let me know if these work for you.

I applied them to Debian's kernel (with slightly modification) and they
are working fine. Via "alsamixer -c 0" I can change "Mic Mute-LED Mode"
to "Follow Capture" and then led is ON only when microphone is ON.
Recording from microphone is working in both configurations.

If testing with Debian's kernel is enough, you can add my:

Tested-by: Pali Rohár <pali.rohar@gmail.com>

Anyway, (this question is also for Henrique), what can we do with MUTE
led to work? Support for controlling it was there, but was revered in
commit 3530febb5c7636f6b26d15637f68296804d26491.

-- 
Pali Rohár
pali.rohar@gmail.com

Re: [ibm-acpi-devel] ThinkPad T480s & LED_MUTE, LED_MICMUTE

[Damjan Georgievski]

> I applied them to Debian's kernel (with slightly modification) and they
> are working fine. Via "alsamixer -c 0" I can change "Mic Mute-LED Mode"
> to "Follow Capture" and then led is ON only when microphone is ON.
> Recording from microphone is working in both configurations.

hm this is the opposite of how it works on my T450s and X1, where the
led is ON when microphone is muted.
(I do wish the led would be ON when the microphone is not muted ie. listening)



-- 
damjan

Re: [ibm-acpi-devel] ThinkPad T480s & LED_MUTE, LED_MICMUTE

[Takashi Iwai]

On Thu, 21 Jun 2018 13:35:21 +0200,
Damjan Georgievski wrote:
> 
> > I applied them to Debian's kernel (with slightly modification) and they
> > are working fine. Via "alsamixer -c 0" I can change "Mic Mute-LED Mode"
> > to "Follow Capture" and then led is ON only when microphone is ON.
> > Recording from microphone is working in both configurations.
> 
> hm this is the opposite of how it works on my T450s and X1, where the
> led is ON when microphone is muted.
> (I do wish the led would be ON when the microphone is not muted ie. listening)

Don't worry, it's still the default behavior ("Follow Mute").
Only when user chooses another mode (e.g. "Follow Capture"), the
behavior changes.


Takashi

Re: [ibm-acpi-devel] ThinkPad T480s & LED_MUTE, LED_MICMUTE

[Damjan Georgievski]

On 21 June 2018 at 13:39, Takashi Iwai <tiwai@suse.de> wrote:
> On Thu, 21 Jun 2018 13:35:21 +0200,
> Damjan Georgievski wrote:
>>
>> > I applied them to Debian's kernel (with slightly modification) and they
>> > are working fine. Via "alsamixer -c 0" I can change "Mic Mute-LED Mode"
>> > to "Follow Capture" and then led is ON only when microphone is ON.
>> > Recording from microphone is working in both configurations.
>>
>> hm this is the opposite of how it works on my T450s and X1, where the
>> led is ON when microphone is muted.
>> (I do wish the led would be ON when the microphone is not muted ie. listening)
>
> Don't worry, it's still the default behavior ("Follow Mute").
> Only when user chooses another mode (e.g. "Follow Capture"), the
> behavior changes.

Yeah, sorry, I didn't notice that the topic changed in the thread.

Anyways, I was intrigued and applied the patches on top of 4.18.0-rc1
and I can report it works fine on X1 Carbon (5th gen).



-- 
damjan

Re: [ibm-acpi-devel] ThinkPad T480s & LED_MUTE, LED_MICMUTE

[Pali Rohár]

On Thursday 21 June 2018 13:35:21 Damjan Georgievski wrote:
> > I applied them to Debian's kernel (with slightly modification) and they
> > are working fine. Via "alsamixer -c 0" I can change "Mic Mute-LED Mode"
> > to "Follow Capture" and then led is ON only when microphone is ON.
> > Recording from microphone is working in both configurations.
> 
> hm this is the opposite of how it works on my T450s and X1, where the
> led is ON when microphone is muted.

Purpose of this patch it to allow user to configure that LED. As I wrote
above, when I *changed* "Mic Mute-LED Mode" to "Follow Capture" in
alsamixer, then led is ON when mic is ON. Default configuration (prior
changing anything) in alsamixer is "Follow Mute" when led is ON when mic
is OFF. That patch allows you to change how LED behave.

> (I do wish the led would be ON when the microphone is not muted ie. listening)

-- 
Pali Rohár
pali.rohar@gmail.com

Re: ThinkPad T480s & LED_MUTE, LED_MICMUTE

[Pali Rohár]

On Monday 18 June 2018 13:21:21 Pali Rohár wrote:
> On Monday 18 June 2018 12:36:31 Takashi Iwai wrote:
> > On Mon, 18 Jun 2018 12:28:06 +0200,
> > Pali Rohár wrote:
> > > 
> > > On Saturday 16 June 2018 18:02:14 Takashi Iwai wrote:
> > > > On Sat, 16 Jun 2018 17:43:09 +0200,
> > > > Pali Rohár wrote:
> > > > > 
> > > > > On Saturday 16 June 2018 09:05:41 Takashi Iwai wrote:
> > > > > > On Fri, 15 Jun 2018 21:09:59 +0200,
> > > > > > Pali Rohár wrote:
> > > > > > > 
> > > > > > > On Friday 15 June 2018 14:51:47 Takashi Iwai wrote:
> > > > > > > > On Fri, 08 Jun 2018 13:10:57 +0200,
> > > > > > > > Pali Rohár wrote:
> > > > > > > > > 
> > > > > > > > > Hi! With up-to-date thinkpad_acpi.ko driver on ThinkPad T480s I'm seeing
> > > > > > > > > a strange behavior of LEDs which are integrated into mic mute (Fn+F4)
> > > > > > > > > and mute (Fn+F1) keys.
> > > > > > > > > 
> > > > > > > > > When thinkpad_acpi.ko is not loaded, then mute key is working fine. When
> > > > > > > > > pressed, it correctly generates KEY_MUTE on AT Translated Set 2 keyboard
> > > > > > > > > input device and also turn on/of mute led. But when micmute key is
> > > > > > > > > pressed then, nothing happen. No key event is reported and also led is
> > > > > > > > > not turned on/off.
> > > > > > > > > 
> > > > > > > > > On the other hand, when thinkpad_acpi.ko is loaded, then both buttons
> > > > > > > > > mute and micmute correctly generates input events; mute via AT keyboard
> > > > > > > > > and micmute via ThinkPad Extra Buttons. But led is not changed. When
> > > > > > > > > thinkpad_acpi.ko is loaded it turn off both leds (mute and micmute) and
> > > > > > > > > leds after pressing any of those buttons, leds are not turned on again.
> > > > > > > > > 
> > > > > > > > > When thinkpad_acpi.ko is unloaded, then pressing mute button again start
> > > > > > > > > switching led on/off.
> > > > > > > > > 
> > > > > > > > > So it seems that some init sequence of thinkpad_acpi.ko breaks mute led.
> > > > > > > > > And fini sequence of thinkpad_acpi.ko makes mute led working again.
> > > > > > > > 
> > > > > > > > Usually the mute LED on Thinkpad is triggered from HD-audio driver
> > > > > > > > (sound/pci/hda/thinkpad_helper.c), and it's a soft-bound via
> > > > > > > > symbol_request(tpacpi_led_set).  I thought thinkpad_acpi is
> > > > > > > > auto-loaded when the module gets bound.
> > > > > > > > 
> > > > > > > > A possible explanation would be that TPT480s has neither IBM0068,
> > > > > > > > LEN0068 nor LEN0268 ACPI HIDs, hence the driver is not auto-loaded.
> > > > > > > 
> > > > > > > I have Debian Stretch kernel (4.9) which does not have LEN0268 alias for
> > > > > > > thinkpad_acpi.ko. So thinkpad_acpi.ko is not loaded automatically. But I
> > > > > > > have put thinkpad_acpi into /etc/modules and it is now automatically
> > > > > > > loaded at boot.
> > > > > > 
> > > > > > That's odd.  It's exposed via
> > > > > >   MODULE_DEVICE_TABLE(acpi, ibm_htk_device_ids);
> > > > > > 
> > > > > > It's been already in 4.9.  At this point, something is fishy.
> > > > > 
> > > > > $ /sbin/modinfo thinkpad_acpi | grep alias
> > > > > alias:          dmi:bvnIBM:bvrI[MU]ET??WW*
> > > > > alias:          tpacpi
> > > > > alias:          acpi*:LEN0068:*
> > > > > alias:          acpi*:IBM0068:*
> > > > > 
> > > > > No there is no LEN0268 on 4.9.
> > > > 
> > > > OK, that's the cause.  It's really old.
> > > > 
> > > > The commit a3c42a467a25 ("platform/x86: thinkpad_acpi: Adding new
> > > > hotkey ID for Lenovo thinkpad") has to be backported.
> > > 
> > > This commit just autoloads thinkpad_acpi driver, right? So manual
> > > modprobe (for now) is OK too?
> > 
> > Well, the manual modprobe is superfluous if it's properly bound with
> > HD-audio driver.  The HD-audio driver calls symbol_request(), so it
> > should do modprobe by itself.
> > 
> > > > Also, in the HD-audio side, the commit 2ecb704a1290 ("ALSA: hda - add
> > > > a new condition to check if it is thinkpad") is needed, too.
> > > 
> > > This commit was introduced in 4.9 and Debian kernel has it. I looked
> > > into Debian source code and there is check for LEN0268 in file
> > > sound/pci/hda/thinkpad_helper.c
> > 
> > Then check whether the function gets really called.
> 
> Checked, no it is not called.
> 
> Now I see that snd_hda_codec_generic is visible in lsmod and thinkpad
> helper is compiled only into snd-hda-codec-realtek and
> snd-hda-codec-conexant... Therefore nobody is even trying to use that
> function.

Interesting is this part:

$ head /proc/asound/card0/codec#0
Codec: Realtek ALC257
Address: 0
AFG Function Id: 0x1 (unsol 1)
Vendor Id: 0x10ec0257
Subsystem Id: 0x17aa2258
Revision Id: 0x100001
No Modem Function Group found
Default PCM:
    rates [0x560]: 44100 48000 96000 192000
        bits [0xe]: 16 20 24

It uses vendor id 0x10ec0257 and support for it was added into
snd-hda-codec-realtek in commit f429e7e494 which was introduced in
version v4.15.

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f429e7e494afaded76e62c6f98211a635aa03098

I applied this patch to Debian's 4.9 kernel and mute & mic mute leds
started working. This one patch was enough.

Takashi, it is possible to backport this patch into -stable trees?

-- 
Pali Rohár
pali.rohar@gmail.com

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel

Re: ThinkPad T480s & LED_MUTE, LED_MICMUTE

[Pali Rohár]

On Monday 18 June 2018 13:21:21 Pali Rohár wrote:
> On Monday 18 June 2018 12:36:31 Takashi Iwai wrote:
> > On Mon, 18 Jun 2018 12:28:06 +0200,
> > Pali Rohár wrote:
> > > 
> > > On Saturday 16 June 2018 18:02:14 Takashi Iwai wrote:
> > > > On Sat, 16 Jun 2018 17:43:09 +0200,
> > > > Pali Rohár wrote:
> > > > > 
> > > > > On Saturday 16 June 2018 09:05:41 Takashi Iwai wrote:
> > > > > > On Fri, 15 Jun 2018 21:09:59 +0200,
> > > > > > Pali Rohár wrote:
> > > > > > > 
> > > > > > > On Friday 15 June 2018 14:51:47 Takashi Iwai wrote:
> > > > > > > > On Fri, 08 Jun 2018 13:10:57 +0200,
> > > > > > > > Pali Rohár wrote:
> > > > > > > > > 
> > > > > > > > > Hi! With up-to-date thinkpad_acpi.ko driver on ThinkPad T480s I'm seeing
> > > > > > > > > a strange behavior of LEDs which are integrated into mic mute (Fn+F4)
> > > > > > > > > and mute (Fn+F1) keys.
> > > > > > > > > 
> > > > > > > > > When thinkpad_acpi.ko is not loaded, then mute key is working fine. When
> > > > > > > > > pressed, it correctly generates KEY_MUTE on AT Translated Set 2 keyboard
> > > > > > > > > input device and also turn on/of mute led. But when micmute key is
> > > > > > > > > pressed then, nothing happen. No key event is reported and also led is
> > > > > > > > > not turned on/off.
> > > > > > > > > 
> > > > > > > > > On the other hand, when thinkpad_acpi.ko is loaded, then both buttons
> > > > > > > > > mute and micmute correctly generates input events; mute via AT keyboard
> > > > > > > > > and micmute via ThinkPad Extra Buttons. But led is not changed. When
> > > > > > > > > thinkpad_acpi.ko is loaded it turn off both leds (mute and micmute) and
> > > > > > > > > leds after pressing any of those buttons, leds are not turned on again.
> > > > > > > > > 
> > > > > > > > > When thinkpad_acpi.ko is unloaded, then pressing mute button again start
> > > > > > > > > switching led on/off.
> > > > > > > > > 
> > > > > > > > > So it seems that some init sequence of thinkpad_acpi.ko breaks mute led.
> > > > > > > > > And fini sequence of thinkpad_acpi.ko makes mute led working again.
> > > > > > > > 
> > > > > > > > Usually the mute LED on Thinkpad is triggered from HD-audio driver
> > > > > > > > (sound/pci/hda/thinkpad_helper.c), and it's a soft-bound via
> > > > > > > > symbol_request(tpacpi_led_set).  I thought thinkpad_acpi is
> > > > > > > > auto-loaded when the module gets bound.
> > > > > > > > 
> > > > > > > > A possible explanation would be that TPT480s has neither IBM0068,
> > > > > > > > LEN0068 nor LEN0268 ACPI HIDs, hence the driver is not auto-loaded.
> > > > > > > 
> > > > > > > I have Debian Stretch kernel (4.9) which does not have LEN0268 alias for
> > > > > > > thinkpad_acpi.ko. So thinkpad_acpi.ko is not loaded automatically. But I
> > > > > > > have put thinkpad_acpi into /etc/modules and it is now automatically
> > > > > > > loaded at boot.
> > > > > > 
> > > > > > That's odd.  It's exposed via
> > > > > >   MODULE_DEVICE_TABLE(acpi, ibm_htk_device_ids);
> > > > > > 
> > > > > > It's been already in 4.9.  At this point, something is fishy.
> > > > > 
> > > > > $ /sbin/modinfo thinkpad_acpi | grep alias
> > > > > alias:          dmi:bvnIBM:bvrI[MU]ET??WW*
> > > > > alias:          tpacpi
> > > > > alias:          acpi*:LEN0068:*
> > > > > alias:          acpi*:IBM0068:*
> > > > > 
> > > > > No there is no LEN0268 on 4.9.
> > > > 
> > > > OK, that's the cause.  It's really old.
> > > > 
> > > > The commit a3c42a467a25 ("platform/x86: thinkpad_acpi: Adding new
> > > > hotkey ID for Lenovo thinkpad") has to be backported.
> > > 
> > > This commit just autoloads thinkpad_acpi driver, right? So manual
> > > modprobe (for now) is OK too?
> > 
> > Well, the manual modprobe is superfluous if it's properly bound with
> > HD-audio driver.  The HD-audio driver calls symbol_request(), so it
> > should do modprobe by itself.
> > 
> > > > Also, in the HD-audio side, the commit 2ecb704a1290 ("ALSA: hda - add
> > > > a new condition to check if it is thinkpad") is needed, too.
> > > 
> > > This commit was introduced in 4.9 and Debian kernel has it. I looked
> > > into Debian source code and there is check for LEN0268 in file
> > > sound/pci/hda/thinkpad_helper.c
> > 
> > Then check whether the function gets really called.
> 
> Checked, no it is not called.
> 
> Now I see that snd_hda_codec_generic is visible in lsmod and thinkpad
> helper is compiled only into snd-hda-codec-realtek and
> snd-hda-codec-conexant... Therefore nobody is even trying to use that
> function.

Interesting is this part:

$ head /proc/asound/card0/codec#0
Codec: Realtek ALC257
Address: 0
AFG Function Id: 0x1 (unsol 1)
Vendor Id: 0x10ec0257
Subsystem Id: 0x17aa2258
Revision Id: 0x100001
No Modem Function Group found
Default PCM:
    rates [0x560]: 44100 48000 96000 192000
        bits [0xe]: 16 20 24

It uses vendor id 0x10ec0257 and support for it was added into
snd-hda-codec-realtek in commit f429e7e494 which was introduced in
version v4.15.

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f429e7e494afaded76e62c6f98211a635aa03098

I applied this patch to Debian's 4.9 kernel and mute & mic mute leds
started working. This one patch was enough.

Takashi, it is possible to backport this patch into -stable trees?

-- 
Pali Rohár
pali.rohar@gmail.com

Re: ThinkPad T480s & LED_MUTE, LED_MICMUTE

[Takashi Iwai]

On Tue, 19 Jun 2018 10:37:12 +0200,
Pali Rohár wrote:
> 
> On Monday 18 June 2018 13:21:21 Pali Rohár wrote:
> > On Monday 18 June 2018 12:36:31 Takashi Iwai wrote:
> > > On Mon, 18 Jun 2018 12:28:06 +0200,
> > > Pali Rohár wrote:
> > > > 
> > > > On Saturday 16 June 2018 18:02:14 Takashi Iwai wrote:
> > > > > On Sat, 16 Jun 2018 17:43:09 +0200,
> > > > > Pali Rohár wrote:
> > > > > > 
> > > > > > On Saturday 16 June 2018 09:05:41 Takashi Iwai wrote:
> > > > > > > On Fri, 15 Jun 2018 21:09:59 +0200,
> > > > > > > Pali Rohár wrote:
> > > > > > > > 
> > > > > > > > On Friday 15 June 2018 14:51:47 Takashi Iwai wrote:
> > > > > > > > > On Fri, 08 Jun 2018 13:10:57 +0200,
> > > > > > > > > Pali Rohár wrote:
> > > > > > > > > > 
> > > > > > > > > > Hi! With up-to-date thinkpad_acpi.ko driver on ThinkPad T480s I'm seeing
> > > > > > > > > > a strange behavior of LEDs which are integrated into mic mute (Fn+F4)
> > > > > > > > > > and mute (Fn+F1) keys.
> > > > > > > > > > 
> > > > > > > > > > When thinkpad_acpi.ko is not loaded, then mute key is working fine. When
> > > > > > > > > > pressed, it correctly generates KEY_MUTE on AT Translated Set 2 keyboard
> > > > > > > > > > input device and also turn on/of mute led. But when micmute key is
> > > > > > > > > > pressed then, nothing happen. No key event is reported and also led is
> > > > > > > > > > not turned on/off.
> > > > > > > > > > 
> > > > > > > > > > On the other hand, when thinkpad_acpi.ko is loaded, then both buttons
> > > > > > > > > > mute and micmute correctly generates input events; mute via AT keyboard
> > > > > > > > > > and micmute via ThinkPad Extra Buttons. But led is not changed. When
> > > > > > > > > > thinkpad_acpi.ko is loaded it turn off both leds (mute and micmute) and
> > > > > > > > > > leds after pressing any of those buttons, leds are not turned on again.
> > > > > > > > > > 
> > > > > > > > > > When thinkpad_acpi.ko is unloaded, then pressing mute button again start
> > > > > > > > > > switching led on/off.
> > > > > > > > > > 
> > > > > > > > > > So it seems that some init sequence of thinkpad_acpi.ko breaks mute led.
> > > > > > > > > > And fini sequence of thinkpad_acpi.ko makes mute led working again.
> > > > > > > > > 
> > > > > > > > > Usually the mute LED on Thinkpad is triggered from HD-audio driver
> > > > > > > > > (sound/pci/hda/thinkpad_helper.c), and it's a soft-bound via
> > > > > > > > > symbol_request(tpacpi_led_set).  I thought thinkpad_acpi is
> > > > > > > > > auto-loaded when the module gets bound.
> > > > > > > > > 
> > > > > > > > > A possible explanation would be that TPT480s has neither IBM0068,
> > > > > > > > > LEN0068 nor LEN0268 ACPI HIDs, hence the driver is not auto-loaded.
> > > > > > > > 
> > > > > > > > I have Debian Stretch kernel (4.9) which does not have LEN0268 alias for
> > > > > > > > thinkpad_acpi.ko. So thinkpad_acpi.ko is not loaded automatically. But I
> > > > > > > > have put thinkpad_acpi into /etc/modules and it is now automatically
> > > > > > > > loaded at boot.
> > > > > > > 
> > > > > > > That's odd.  It's exposed via
> > > > > > >   MODULE_DEVICE_TABLE(acpi, ibm_htk_device_ids);
> > > > > > > 
> > > > > > > It's been already in 4.9.  At this point, something is fishy.
> > > > > > 
> > > > > > $ /sbin/modinfo thinkpad_acpi | grep alias
> > > > > > alias:          dmi:bvnIBM:bvrI[MU]ET??WW*
> > > > > > alias:          tpacpi
> > > > > > alias:          acpi*:LEN0068:*
> > > > > > alias:          acpi*:IBM0068:*
> > > > > > 
> > > > > > No there is no LEN0268 on 4.9.
> > > > > 
> > > > > OK, that's the cause.  It's really old.
> > > > > 
> > > > > The commit a3c42a467a25 ("platform/x86: thinkpad_acpi: Adding new
> > > > > hotkey ID for Lenovo thinkpad") has to be backported.
> > > > 
> > > > This commit just autoloads thinkpad_acpi driver, right? So manual
> > > > modprobe (for now) is OK too?
> > > 
> > > Well, the manual modprobe is superfluous if it's properly bound with
> > > HD-audio driver.  The HD-audio driver calls symbol_request(), so it
> > > should do modprobe by itself.
> > > 
> > > > > Also, in the HD-audio side, the commit 2ecb704a1290 ("ALSA: hda - add
> > > > > a new condition to check if it is thinkpad") is needed, too.
> > > > 
> > > > This commit was introduced in 4.9 and Debian kernel has it. I looked
> > > > into Debian source code and there is check for LEN0268 in file
> > > > sound/pci/hda/thinkpad_helper.c
> > > 
> > > Then check whether the function gets really called.
> > 
> > Checked, no it is not called.
> > 
> > Now I see that snd_hda_codec_generic is visible in lsmod and thinkpad
> > helper is compiled only into snd-hda-codec-realtek and
> > snd-hda-codec-conexant... Therefore nobody is even trying to use that
> > function.
> 
> Interesting is this part:
> 
> $ head /proc/asound/card0/codec#0
> Codec: Realtek ALC257
> Address: 0
> AFG Function Id: 0x1 (unsol 1)
> Vendor Id: 0x10ec0257
> Subsystem Id: 0x17aa2258
> Revision Id: 0x100001
> No Modem Function Group found
> Default PCM:
>     rates [0x560]: 44100 48000 96000 192000
>         bits [0xe]: 16 20 24
> 
> It uses vendor id 0x10ec0257 and support for it was added into
> snd-hda-codec-realtek in commit f429e7e494 which was introduced in
> version v4.15.
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f429e7e494afaded76e62c6f98211a635aa03098
> 
> I applied this patch to Debian's 4.9 kernel and mute & mic mute leds
> started working. This one patch was enough.
> 
> Takashi, it is possible to backport this patch into -stable trees?

It has already a Cc-to-stable, so it should have been backported.
Maybe it wasn't applicable at that time due to lack of other dependent
patches...

In anyway, feel free to ping Greg for including it to 4.9.x if it
works for you now.


Takashi

Re: ThinkPad T480s & LED_MUTE, LED_MICMUTE

[Pali Rohár]

On Tuesday 19 June 2018 10:42:12 Takashi Iwai wrote:
> On Tue, 19 Jun 2018 10:37:12 +0200,
> Pali Rohár wrote:
> > Interesting is this part:
> > 
> > $ head /proc/asound/card0/codec#0
> > Codec: Realtek ALC257
> > Address: 0
> > AFG Function Id: 0x1 (unsol 1)
> > Vendor Id: 0x10ec0257
> > Subsystem Id: 0x17aa2258
> > Revision Id: 0x100001
> > No Modem Function Group found
> > Default PCM:
> >     rates [0x560]: 44100 48000 96000 192000
> >         bits [0xe]: 16 20 24
> > 
> > It uses vendor id 0x10ec0257 and support for it was added into
> > snd-hda-codec-realtek in commit f429e7e494 which was introduced in
> > version v4.15.
> > 
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f429e7e494afaded76e62c6f98211a635aa03098
> > 
> > I applied this patch to Debian's 4.9 kernel and mute & mic mute leds
> > started working. This one patch was enough.
> > 
> > Takashi, it is possible to backport this patch into -stable trees?
> 
> It has already a Cc-to-stable, so it should have been backported.
> Maybe it wasn't applicable at that time due to lack of other dependent
> patches...
> 
> In anyway, feel free to ping Greg for including it to 4.9.x if it
> works for you now.

I sent email to Greg, but it was eaten by "email-bot of Greg
Kroah-Hartman's inbox". So I sent another patch email to stable ML.

-- 
Pali Rohár
pali.rohar@gmail.com

Re: ThinkPad T480s & LED_MUTE, LED_MICMUTE

[Takashi Iwai]

On Thu, 21 Jun 2018 13:24:07 +0200,
Pali Rohár wrote:
> 
> On Tuesday 19 June 2018 10:42:12 Takashi Iwai wrote:
> > On Tue, 19 Jun 2018 10:37:12 +0200,
> > Pali Rohár wrote:
> > > Interesting is this part:
> > > 
> > > $ head /proc/asound/card0/codec#0
> > > Codec: Realtek ALC257
> > > Address: 0
> > > AFG Function Id: 0x1 (unsol 1)
> > > Vendor Id: 0x10ec0257
> > > Subsystem Id: 0x17aa2258
> > > Revision Id: 0x100001
> > > No Modem Function Group found
> > > Default PCM:
> > >     rates [0x560]: 44100 48000 96000 192000
> > >         bits [0xe]: 16 20 24
> > > 
> > > It uses vendor id 0x10ec0257 and support for it was added into
> > > snd-hda-codec-realtek in commit f429e7e494 which was introduced in
> > > version v4.15.
> > > 
> > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f429e7e494afaded76e62c6f98211a635aa03098
> > > 
> > > I applied this patch to Debian's 4.9 kernel and mute & mic mute leds
> > > started working. This one patch was enough.
> > > 
> > > Takashi, it is possible to backport this patch into -stable trees?
> > 
> > It has already a Cc-to-stable, so it should have been backported.
> > Maybe it wasn't applicable at that time due to lack of other dependent
> > patches...
> > 
> > In anyway, feel free to ping Greg for including it to 4.9.x if it
> > works for you now.
> 
> I sent email to Greg, but it was eaten by "email-bot of Greg
> Kroah-Hartman's inbox". So I sent another patch email to stable ML.

Better to put '[PATCH 4.9.y]' prefix in the subject to make clear that
it's only for 4.9.y.

Also, the line indicating the upstream commit id is:

  commit $ID upstream.

Last but not least, if you resubmit the patch, you must give your own
sign-off, too.


thanks,

Takashi