ibm-acpi-devel — ibm-acpi Linux driver development

[ibm-acpi-devel] hotkey_tablet_mode

[Christian S. <chr...@gm...>]

Hello list,

I'm running a recent linux kernel (v5.1.2) on a Lenovo X380 Yoga with the thinkpad_acpi kernel module active:

bash-4.4# dmesg | grep think
[    1.912994] thinkpad_acpi: ThinkPad ACPI Extras v0.26
[    1.913000] thinkpad_acpi: http://ibm-acpi.sf.net/
[    1.913003] thinkpad_acpi: ThinkPad BIOS R0SET39W (1.23 ), EC unknown
[    1.913006] thinkpad_acpi: Lenovo ThinkPad X380 Yoga, model 20LJS43M00
[    1.913506] thinkpad_acpi: radio switch found; radios are enabled
[    1.913571] thinkpad_acpi: Tablet mode switch found (type: GMMS), currently in laptop mode
[    1.913801] thinkpad_acpi: This ThinkPad has standard ACPI backlight brightness control, supported by the ACPI video driver
[    1.913808] thinkpad_acpi: Disabling thinkpad-acpi brightness events by default...
[    1.916093] thinkpad_acpi: rfkill switch tpacpi_bluetooth_sw: radio is unblocked
[    1.921443] thinkpad_acpi: Standard ACPI backlight interface available, not loading native one
[    1.921451] thinkpad_acpi: volume: disabled as there is no ALSA support in this kernel
[    1.926063] thinkpad_acpi: battery 1 registered (start 128, stop 128)
[    1.926126] input: ThinkPad Extra Buttons as /devices/platform/thinkpad_acpi/input/input7
[  130.702563] thinkpad_acpi: setting the hotkey mask to 0xffffffff is likely not the best way to go about it
[  130.702568] thinkpad_acpi: please consider using the driver defaults, and refer to up-to-date thinkpad-acpi documentation
[  130.703930] thinkpad_acpi: asked for hotkey mask 0xffffffff, but firmware forced it to 0xfffffffb

My problem is, that the state of the tablet mode switch - which can be determined by reading /sys/bus/platform/drivers/thinkpad_acpi/thinkpad_acpi/hotkey_tablet_mode - never changes, no matter how far I open or close the Yoga's lid.

Also, in a dual boot setup (Windows 10 / Linux), when I switch the Yoga to tablet mode in Windows, reboot it in this state to linux and then "fold" it back to laptop mode, the "hotkey_tablet_mode" still tells me that the Yoga is in tablet mode (which means that the keyboard does not work...); and I did not find any way to switch it back to laptop mode under linux. Only after booting Windows again and rebooting in laptop mode, hotkey_tablet_mode holds the correct value.

I suppose that there's something wrong with my kernel config, but I have no clue what...

Maybe someone can give me a hint?

Thanks and best regards,
Christian

Re: [ibm-acpi-devel] [PATCH] Add Lenovo PrivacyGuard feature found in T480s, T490, T490s

[Andy S. <and...@gm...>]

On Tue, May 21, 2019 at 3:56 PM Alexander Schremmer
<al...@al...> wrote:
> I wonder whether you have received my kernel patch, referenced below. It
> might have been caught by the spam filter.

Yes, you may always consult with patchwork [1]. If it's they, it won't be lost.

[1]: https://patchwork.kernel.org/project/platform-driver-x86/list/

-- 
With Best Regards,
Andy Shevchenko

[ibm-acpi-devel] Volume/Mic Mute Light Does Not Function on ThinkPad L420

[Mingcong B. <jef...@gm...>]

Greetings,

Thanks for all your great work for making ThinkPads usable on Linux!

I have recently acquired a ThinkPad L420, but I have been running into 
some issues - specifically, the Mic Mute button and the Volume Mute button:

- The Mic Mute button does not function, with no event registered on 
acpi_listen. However, if I turn down the input volume to 0 in a 
PulseAudio mixer, the Mic Mute LED lights up (red).
- The Volume Mute button *works*, but the LED does not light up/turn off 
with the volume mute event. It does register an event on acpi_listen, 
"button/mute MUTE 00000080 00000000 K".

In Windows, both mute LED only works when the Hot Key Integration driver 
is installed, much like other ThinkPad models of the same period (T420, 
X220, etc.).

I have attached acpidump and dmidecode outputs.

Best Regards,
Mingcong Bai

Re: [ibm-acpi-devel] [PATCH] leds: tpacpi: cleanup for Thinkpad ACPI led

[Andy S. <and...@gm...>]

On Mon, Apr 29, 2019 at 6:22 PM Pavel Machek <pa...@uc...> wrote:
>
>
> Make error returns more consistent... no behaviour change intended.
>

Pushed to my review and testing queue, thanks!

P.S. I fixed prefix accordingly.

> Signed-off-by: Pavel Machek <pa...@uc...>
>
> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
> index 726341f..7008a7f 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -5808,7 +5808,7 @@ static int led_set_status(const unsigned int led,
>                         return -EPERM;
>                 if (!acpi_evalf(led_handle, NULL, NULL, "vdd",
>                                 (1 << led), led_sled_arg1[ledstatus]))
> -                       rc = -EIO;
> +                       return -EIO;
>                 break;
>         case TPACPI_LED_OLD:
>                 /* 600e/x, 770e, 770x, A21e, A2xm/p, T20-22, X20 */
> @@ -5832,10 +5832,10 @@ static int led_set_status(const unsigned int led,
>                         return -EPERM;
>                 if (!acpi_evalf(led_handle, NULL, NULL, "vdd",
>                                 led, led_led_arg1[ledstatus]))
> -                       rc = -EIO;
> +                       return -EIO;
>                 break;
>         default:
> -               rc = -ENXIO;
> +               return -ENXIO;
>         }
>
>         if (!rc)
>
> --
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html



-- 
With Best Regards,
Andy Shevchenko

Re: [ibm-acpi-devel] [PATCH] platform/x86: thinkpad_acpi: Mark expected switch fall-throughs

[Andy S. <and...@gm...>]

On Wed, May 1, 2019 at 11:44 PM Gustavo A. R. Silva
<gu...@em...> wrote:
>
>
>
> On 4/24/19 7:55 PM, ibm...@hm... wrote:
> > On Wed, Apr 24, 2019, at 16:05, Kees Cook wrote:
> >> On Wed, Apr 24, 2019 at 11:15 AM Gustavo A. R. Silva
> >> <gu...@em...> wrote:
> >>>
> >>> In preparation to enabling -Wimplicit-fallthrough, mark switch
> >>> cases where we are expecting to fall through.
> >>>
> >>> This patch fixes the following warnings:
> >>>
>
> [..]
>
> >>>
> >>> Warning level 3 was used: -Wimplicit-fallthrough=3
> >>>
> >>> Notice that, in this particular case, the code comments are modified
> >>> in accordance with what GCC is expecting to find.
> >>>
> >>> This patch is part of the ongoing efforts to enable
> >>> -Wimplicit-fallthrough.
> >>>
> >>> Signed-off-by: Gustavo A. R. Silva <gu...@em...>
> >>
> >> Reviewed-by: Kees Cook <kee...@ch...>
> >
> > Acked-by: Henrique de Moraes Holschuh <hm...@hm...>
> >
>
> Thank you both, Kees and Henrique.
>
> Friendly ping:
>
> Who can take this?

Pushed to my review and testing queue, thanks!

>
> Thanks
> --
> Gustavo



--
With Best Regards,
Andy Shevchenko

Re: [ibm-acpi-devel] [PATCH] platform/x86: thinkpad_acpi: fix spelling mistake "capabilites" -> "capabilities"

[Andy S. <and...@gm...>]

On Tue, Apr 16, 2019 at 9:58 AM Mukesh Ojha <mo...@co...> wrote:
>
>
> On 4/15/2019 6:22 PM, Colin King wrote:
> > From: Colin Ian King <col...@ca...>
> >
> > There is a spelling mistake in a module parameter description. Fix it.
> >

Pushed to my review and testing queue, thanks!

> > Signed-off-by: Colin Ian King <col...@ca...>
> Reviewed-by: Mukesh Ojha <mo...@co...>
>
> Cheers,
> -Mukesh
>
> > ---
> >   drivers/platform/x86/thinkpad_acpi.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
> > index 57d9ae9d8e56..9192b686e9a6 100644
> > --- a/drivers/platform/x86/thinkpad_acpi.c
> > +++ b/drivers/platform/x86/thinkpad_acpi.c
> > @@ -10269,7 +10269,7 @@ MODULE_PARM_DESC(volume_mode,
> >
> >   module_param_named(volume_capabilities, volume_capabilities, uint, 0444);
> >   MODULE_PARM_DESC(volume_capabilities,
> > -              "Selects the mixer capabilites: 0=auto, 1=volume and mute, 2=mute only");
> > +              "Selects the mixer capabilities: 0=auto, 1=volume and mute, 2=mute only");
> >
> >   module_param_named(volume_control, volume_control_allowed, bool, 0444);
> >   MODULE_PARM_DESC(volume_control,



-- 
With Best Regards,
Andy Shevchenko

Re: [ibm-acpi-devel] [PATCH] leds: avoid races with workqueue

[Pavel M. <pa...@uc...>]

Hi!

> >Yes, that looks right. If you can add fixes: and cc: stable tags, the
> >rest should happen automagically.
> 
> Cc: st...@ke... is exactly for what it claims - sending a copy
> to that list.
> 
> "Fixes:" seems to be always enough for the bots to pick a patch.
> 
> Tag added.

Well, docs says Cc: stable is right way to request inclusion, and it
does not talk about Fixes:... but I guess that will work too.

Thanks,
									Pavel

(
To have the patch automatically included in the stable tree, add the
tag

.. code-block:: none

     Cc: st...@vg...

in the sign-off area. Once the patch is merged it will be applied to
the stable tree without anything else needing to be done by the author
or subsystem maintainer.
)

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

Re: [ibm-acpi-devel] [PATCH] leds: avoid races with workqueue

[Jacek A. <jac...@gm...>]

On 5/2/19 10:06 PM, Pavel Machek wrote:
> On Thu 2019-05-02 21:28:06, Jacek Anaszewski wrote:
>> On 5/2/19 9:13 PM, Pavel Machek wrote:
>>> Hi!
>>>
>>>>>>> +++ b/drivers/leds/led-class.c
>>>>>>> @@ -57,6 +57,7 @@ static ssize_t brightness_store(struct device *dev,
>>>>>>>   	if (state == LED_OFF)
>>>>>>>   		led_trigger_remove(led_cdev);
>>>>>>>   	led_set_brightness(led_cdev, state);
>>>>>>> +	flush_work(&led_cdev->set_brightness_work);
>>>>>>
>>>>>> Is this really required here? It creates non-uniform brightness
>>>>>> setting behavior depending on whether it is set from sysfs or
>>>>>> by in-kernel call to led_set_brightness().
>>>>>
>>>>> This fixes the echo 0 > brightness; echo 1 > brightness. It has to be
>>>>> at a place where we can sleep.
>>>>>
>>>>> If you have better idea, it is welcome, but it would be good to fix
>>>>> the bug.
>>>>
>>>> Currently not, so I applied the patch in this shape.
>>>
>>> Thanks!
>>>
>>> This is actually something that makes sense for stable.. perhaps the
>>> bots can pick it up.
>>
>> I was thinking of it, but finally decided to submit this patch
>> to linux-stable when it will prove not having side effects.
>>
>> But if you think it is ready for stable then I can add
>> relevant "Fixes" tag. Do you think that below will be an appropriate
>> base to refer to?
>>
>> Fixes 1afcadfcd184 ("leds: core: Use set_brightness_work for the blocking
>> op")
> 
> Yes, that looks right. If you can add fixes: and cc: stable tags, the
> rest should happen automagically.

Cc: st...@ke... is exactly for what it claims - sending a copy
to that list.

"Fixes:" seems to be always enough for the bots to pick a patch.

Tag added.

-- 
Best regards,
Jacek Anaszewski

Re: [ibm-acpi-devel] [PATCH] leds: avoid races with workqueue

[Pavel M. <pa...@uc...>]

On Thu 2019-05-02 21:28:06, Jacek Anaszewski wrote:
> On 5/2/19 9:13 PM, Pavel Machek wrote:
> >Hi!
> >
> >>>>>+++ b/drivers/leds/led-class.c
> >>>>>@@ -57,6 +57,7 @@ static ssize_t brightness_store(struct device *dev,
> >>>>>  	if (state == LED_OFF)
> >>>>>  		led_trigger_remove(led_cdev);
> >>>>>  	led_set_brightness(led_cdev, state);
> >>>>>+	flush_work(&led_cdev->set_brightness_work);
> >>>>
> >>>>Is this really required here? It creates non-uniform brightness
> >>>>setting behavior depending on whether it is set from sysfs or
> >>>>by in-kernel call to led_set_brightness().
> >>>
> >>>This fixes the echo 0 > brightness; echo 1 > brightness. It has to be
> >>>at a place where we can sleep.
> >>>
> >>>If you have better idea, it is welcome, but it would be good to fix
> >>>the bug.
> >>
> >>Currently not, so I applied the patch in this shape.
> >
> >Thanks!
> >
> >This is actually something that makes sense for stable.. perhaps the
> >bots can pick it up.
> 
> I was thinking of it, but finally decided to submit this patch
> to linux-stable when it will prove not having side effects.
> 
> But if you think it is ready for stable then I can add
> relevant "Fixes" tag. Do you think that below will be an appropriate
> base to refer to?
> 
> Fixes 1afcadfcd184 ("leds: core: Use set_brightness_work for the blocking
> op")

Yes, that looks right. If you can add fixes: and cc: stable tags, the
rest should happen automagically.

Thanks!
									Pavel



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

Re: [ibm-acpi-devel] [PATCH] leds: avoid races with workqueue

[Jacek A. <jac...@gm...>]

On 5/2/19 9:13 PM, Pavel Machek wrote:
> Hi!
> 
>>>>> +++ b/drivers/leds/led-class.c
>>>>> @@ -57,6 +57,7 @@ static ssize_t brightness_store(struct device *dev,
>>>>>   	if (state == LED_OFF)
>>>>>   		led_trigger_remove(led_cdev);
>>>>>   	led_set_brightness(led_cdev, state);
>>>>> +	flush_work(&led_cdev->set_brightness_work);
>>>>
>>>> Is this really required here? It creates non-uniform brightness
>>>> setting behavior depending on whether it is set from sysfs or
>>>> by in-kernel call to led_set_brightness().
>>>
>>> This fixes the echo 0 > brightness; echo 1 > brightness. It has to be
>>> at a place where we can sleep.
>>>
>>> If you have better idea, it is welcome, but it would be good to fix
>>> the bug.
>>
>> Currently not, so I applied the patch in this shape.
> 
> Thanks!
> 
> This is actually something that makes sense for stable.. perhaps the
> bots can pick it up.

I was thinking of it, but finally decided to submit this patch
to linux-stable when it will prove not having side effects.

But if you think it is ready for stable then I can add
relevant "Fixes" tag. Do you think that below will be an appropriate
base to refer to?

Fixes 1afcadfcd184 ("leds: core: Use set_brightness_work for the 
blocking op")

?
-- 
Best regards,
Jacek Anaszewski

Re: [ibm-acpi-devel] [PATCH] leds: avoid races with workqueue

[Pavel M. <pa...@uc...>]

Hi!

> >>>+++ b/drivers/leds/led-class.c
> >>>@@ -57,6 +57,7 @@ static ssize_t brightness_store(struct device *dev,
> >>>  	if (state == LED_OFF)
> >>>  		led_trigger_remove(led_cdev);
> >>>  	led_set_brightness(led_cdev, state);
> >>>+	flush_work(&led_cdev->set_brightness_work);
> >>
> >>Is this really required here? It creates non-uniform brightness
> >>setting behavior depending on whether it is set from sysfs or
> >>by in-kernel call to led_set_brightness().
> >
> >This fixes the echo 0 > brightness; echo 1 > brightness. It has to be
> >at a place where we can sleep.
> >
> >If you have better idea, it is welcome, but it would be good to fix
> >the bug.
> 
> Currently not, so I applied the patch in this shape.

Thanks!

This is actually something that makes sense for stable.. perhaps the
bots can pick it up.
									Pavel


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

Re: [ibm-acpi-devel] [PATCH] leds: avoid races with workqueue

[Jacek A. <jac...@gm...>]

Hi Pavel,

On 5/1/19 8:36 PM, Pavel Machek wrote:
> Hi!
> 
>>> There are races between "main" thread and workqueue. They manifest
>>> themselves on Thinkpad X60:
>>> This should result in LED blinking, but it turns it off instead:
>>>      root@amd:/data/pavel# cd /sys/class/leds/tpacpi\:\:power
>>>      root@amd:/sys/class/leds/tpacpi::power# echo timer > trigger
>>>      root@amd:/sys/class/leds/tpacpi::power# echo timer > trigger
>>>      root@amd:/sys/class/leds/tpacpi::power#

I believe this line is redundant, so I removed it.

>>> It should be possible to transition from blinking to solid on by echo
>>> 0 > brightness; echo 1 > brightness... but that does not work, either,
>>> if done too quickly.
>>> Synchronization of the workqueue fixes both.
>>> Signed-off-by: Pavel Machek <pa...@uc...>
>>>
>>> diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
>>> index 68aa923..dcb59c8 100644
>>> --- a/drivers/leds/led-class.c
>>> +++ b/drivers/leds/led-class.c
>>> @@ -57,6 +57,7 @@ static ssize_t brightness_store(struct device *dev,
>>>   	if (state == LED_OFF)
>>>   		led_trigger_remove(led_cdev);
>>>   	led_set_brightness(led_cdev, state);
>>> +	flush_work(&led_cdev->set_brightness_work);
>>
>> Is this really required here? It creates non-uniform brightness
>> setting behavior depending on whether it is set from sysfs or
>> by in-kernel call to led_set_brightness().
> 
> This fixes the echo 0 > brightness; echo 1 > brightness. It has to be
> at a place where we can sleep.
> 
> If you have better idea, it is welcome, but it would be good to fix
> the bug.

Currently not, so I applied the patch in this shape.

-- 
Best regards,
Jacek Anaszewski

Re: [ibm-acpi-devel] [PATCH] leds: avoid races with workqueue

[Pavel M. <pa...@uc...>]

Hi!

> >There are races between "main" thread and workqueue. They manifest
> >themselves on Thinkpad X60:
> >This should result in LED blinking, but it turns it off instead:
> >     root@amd:/data/pavel# cd /sys/class/leds/tpacpi\:\:power
> >     root@amd:/sys/class/leds/tpacpi::power# echo timer > trigger
> >     root@amd:/sys/class/leds/tpacpi::power# echo timer > trigger
> >     root@amd:/sys/class/leds/tpacpi::power#
> >It should be possible to transition from blinking to solid on by echo
> >0 > brightness; echo 1 > brightness... but that does not work, either,
> >if done too quickly.
> >Synchronization of the workqueue fixes both.
> >Signed-off-by: Pavel Machek <pa...@uc...>
> >
> >diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
> >index 68aa923..dcb59c8 100644
> >--- a/drivers/leds/led-class.c
> >+++ b/drivers/leds/led-class.c
> >@@ -57,6 +57,7 @@ static ssize_t brightness_store(struct device *dev,
> >  	if (state == LED_OFF)
> >  		led_trigger_remove(led_cdev);
> >  	led_set_brightness(led_cdev, state);
> >+	flush_work(&led_cdev->set_brightness_work);
> 
> Is this really required here? It creates non-uniform brightness
> setting behavior depending on whether it is set from sysfs or
> by in-kernel call to led_set_brightness().

This fixes the echo 0 > brightness; echo 1 > brightness. It has to be
at a place where we can sleep.

If you have better idea, it is welcome, but it would be good to fix
the bug.

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

Re: [ibm-acpi-devel] [PATCH] leds: avoid races with workqueue

[Jacek A. <jac...@gm...>]

Hi Pavel,

Thank you for the patch.

On 4/29/19 5:22 PM, Pavel Machek wrote:
> 
> There are races between "main" thread and workqueue. They manifest
> themselves on Thinkpad X60:
>      
> This should result in LED blinking, but it turns it off instead:
>      
>      root@amd:/data/pavel# cd /sys/class/leds/tpacpi\:\:power
>      root@amd:/sys/class/leds/tpacpi::power# echo timer > trigger
>      root@amd:/sys/class/leds/tpacpi::power# echo timer > trigger
>      root@amd:/sys/class/leds/tpacpi::power#
>      
> It should be possible to transition from blinking to solid on by echo
> 0 > brightness; echo 1 > brightness... but that does not work, either,
> if done too quickly.
>      
> Synchronization of the workqueue fixes both.
>      
> Signed-off-by: Pavel Machek <pa...@uc...>
> 
> diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
> index 68aa923..dcb59c8 100644
> --- a/drivers/leds/led-class.c
> +++ b/drivers/leds/led-class.c
> @@ -57,6 +57,7 @@ static ssize_t brightness_store(struct device *dev,
>   	if (state == LED_OFF)
>   		led_trigger_remove(led_cdev);
>   	led_set_brightness(led_cdev, state);
> +	flush_work(&led_cdev->set_brightness_work);

Is this really required here? It creates non-uniform brightness
setting behavior depending on whether it is set from sysfs or
by in-kernel call to led_set_brightness().

>   	ret = size;
>   unlock:
> diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
> index 9f8da39..aefac4d 100644
> --- a/drivers/leds/led-core.c
> +++ b/drivers/leds/led-core.c
> @@ -166,6 +166,11 @@ static void led_blink_setup(struct led_classdev *led_cdev,
>   		     unsigned long *delay_on,
>   		     unsigned long *delay_off)
>   {
> +	/*
> +	 * If "set brightness to 0" is pending in workqueue, we don't
> +	 * want that to be reordered after blink_set()
> +	 */
> +	flush_work(&led_cdev->set_brightness_work);
>   	if (!test_bit(LED_BLINK_ONESHOT, &led_cdev->work_flags) &&
>   	    led_cdev->blink_set &&
>   	    !led_cdev->blink_set(led_cdev, delay_on, delay_off))
> 

-- 
Best regards,
Jacek Anaszewski

[ibm-acpi-devel] [PATCH] leds: avoid races with workqueue

[Pavel M. <pa...@uc...>]

There are races between "main" thread and workqueue. They manifest
themselves on Thinkpad X60:
    
This should result in LED blinking, but it turns it off instead:
    
    root@amd:/data/pavel# cd /sys/class/leds/tpacpi\:\:power
    root@amd:/sys/class/leds/tpacpi::power# echo timer > trigger
    root@amd:/sys/class/leds/tpacpi::power# echo timer > trigger
    root@amd:/sys/class/leds/tpacpi::power#
    
It should be possible to transition from blinking to solid on by echo
0 > brightness; echo 1 > brightness... but that does not work, either,
if done too quickly.
    
Synchronization of the workqueue fixes both.
    
Signed-off-by: Pavel Machek <pa...@uc...>

diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index 68aa923..dcb59c8 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -57,6 +57,7 @@ static ssize_t brightness_store(struct device *dev,
 	if (state == LED_OFF)
 		led_trigger_remove(led_cdev);
 	led_set_brightness(led_cdev, state);
+	flush_work(&led_cdev->set_brightness_work);
 
 	ret = size;
 unlock:
diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
index 9f8da39..aefac4d 100644
--- a/drivers/leds/led-core.c
+++ b/drivers/leds/led-core.c
@@ -166,6 +166,11 @@ static void led_blink_setup(struct led_classdev *led_cdev,
 		     unsigned long *delay_on,
 		     unsigned long *delay_off)
 {
+	/*
+	 * If "set brightness to 0" is pending in workqueue, we don't
+	 * want that to be reordered after blink_set()
+	 */
+	flush_work(&led_cdev->set_brightness_work);
 	if (!test_bit(LED_BLINK_ONESHOT, &led_cdev->work_flags) &&
 	    led_cdev->blink_set &&
 	    !led_cdev->blink_set(led_cdev, delay_on, delay_off))

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

[ibm-acpi-devel] [PATCH] leds: tpacpi: cleanup for Thinkpad ACPI led

[Pavel M. <pa...@uc...>]

Make error returns more consistent... no behaviour change intended.
    
Signed-off-by: Pavel Machek <pa...@uc...>

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index 726341f..7008a7f 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -5808,7 +5808,7 @@ static int led_set_status(const unsigned int led,
 			return -EPERM;
 		if (!acpi_evalf(led_handle, NULL, NULL, "vdd",
 				(1 << led), led_sled_arg1[ledstatus]))
-			rc = -EIO;
+			return -EIO;
 		break;
 	case TPACPI_LED_OLD:
 		/* 600e/x, 770e, 770x, A21e, A2xm/p, T20-22, X20 */
@@ -5832,10 +5832,10 @@ static int led_set_status(const unsigned int led,
 			return -EPERM;
 		if (!acpi_evalf(led_handle, NULL, NULL, "vdd",
 				led, led_led_arg1[ledstatus]))
-			rc = -EIO;
+			return -EIO;
 		break;
 	default:
-		rc = -ENXIO;
+		return -ENXIO;
 	}
 
 	if (!rc)

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

Re: [ibm-acpi-devel] Thinkpad ACPI led -- it keeps blinking

[Pavel M. <pa...@uc...>]

Hi!

> >This fixes one problem:

> >Signed-off-by: Pavel Machek <pa...@uc...>
> >
> >diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
> >index e3da7c0..d795d8f 100644
> >--- a/drivers/leds/led-core.c
> >+++ b/drivers/leds/led-core.c
> >@@ -164,8 +164,14 @@ static void led_blink_setup(struct led_classdev *led_cdev,
> >  		     unsigned long *delay_on,
> >  		     unsigned long *delay_off)
> >  {
> >+	while (work_pending(&led_cdev->set_brightness_work)) {
> >+		printk("Waiting for brightness set to finish\n");
> >+		schedule();
> >+	}
> 
> Or even better:
> 
> flush_work(&led_cdev->set_brightness_work);

Yup, thanks for a hint.

I should have acceptable patch, soon; but no promises I catched all
similar bugs, that code is quite ... tricky.

So far I have this:

diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index 3c7e348..85848c5 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -57,6 +57,7 @@ static ssize_t brightness_store(struct device *dev,
 	if (state == LED_OFF)
 		led_trigger_remove(led_cdev);
 	led_set_brightness(led_cdev, state);
+	flush_work(&led_cdev->set_brightness_work);
 
 	ret = size;
 unlock:
diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
index e3da7c0..e9ae7f8 100644
--- a/drivers/leds/led-core.c
+++ b/drivers/leds/led-core.c
@@ -164,6 +164,11 @@ static void led_blink_setup(struct led_classdev *led_cdev,
 		     unsigned long *delay_on,
 		     unsigned long *delay_off)
 {
+	/*
+	 * If "set brightness to 0" is pending in workqueue, we don't
+	 * want that to be reordered after blink_set()
+	 */
+	flush_work(&led_cdev->set_brightness_work);
 	if (!test_bit(LED_BLINK_ONESHOT, &led_cdev->work_flags) &&
 	    led_cdev->blink_set &&
 	    !led_cdev->blink_set(led_cdev, delay_on, delay_off))
diff --git a/drivers/leds/led-triggers.c b/drivers/leds/led-triggers.c
index 2d451b6..ddfd2dd 100644
--- a/drivers/leds/led-triggers.c
+++ b/drivers/leds/led-triggers.c
@@ -65,6 +65,7 @@ ssize_t led_trigger_store(struct device *dev, struct device_attribute *attr,
 	up_read(&triggers_list_lock);
 
 unlock:
+	flush_work(&led_cdev->set_brightness_work);
 	mutex_unlock(&led_cdev->led_access);
 	return ret;
 }
diff --git a/drivers/leds/trigger/ledtrig-timer.c b/drivers/leds/trigger/ledtrig-timer.c
index ca898c1..0b061bb5 100644
--- a/drivers/leds/trigger/ledtrig-timer.c
+++ b/drivers/leds/trigger/ledtrig-timer.c
@@ -104,6 +104,7 @@ static void pattern_init(struct led_classdev *led_cdev)
 
 static int timer_trig_activate(struct led_classdev *led_cdev)
 {
+	printk("timer_trig_activate\n");
 	if (led_cdev->flags & LED_INIT_DEFAULT_TRIGGER) {
 		pattern_init(led_cdev);
 		/*
@@ -115,14 +116,18 @@ static int timer_trig_activate(struct led_classdev *led_cdev)
 
 	led_blink_set(led_cdev, &led_cdev->blink_delay_on,
 		      &led_cdev->blink_delay_off);
+	printk("timer_trig_activate done\n");
 
 	return 0;
 }
 
 static void timer_trig_deactivate(struct led_classdev *led_cdev)
 {
+	printk("timer_trig_deactivate\n");
 	/* Stop blinking */
 	led_set_brightness(led_cdev, LED_OFF);
+
+	printk("timer_trig_deactivate done\n");
 }
 
 static struct led_trigger timer_led_trigger = {
diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index 3a4402a..b3fa9c9 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -363,11 +363,11 @@ int mmc_of_parse_voltage(struct device_node *np, u32 *mask)
 	int num_ranges, i;
 
 	voltage_ranges = of_get_property(np, "voltage-ranges", &num_ranges);
-	num_ranges = num_ranges / sizeof(*voltage_ranges) / 2;
 	if (!voltage_ranges) {
 		pr_debug("%pOF: voltage-ranges unspecified\n", np);
 		return 0;
 	}
+	num_ranges = num_ranges / sizeof(*voltage_ranges) / 2;
 	if (!num_ranges) {
 		pr_err("%pOF: voltage-ranges empty\n", np);
 		return -EINVAL;
diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index 57d9ae9..3580bab 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -5867,6 +5867,8 @@ static int led_set_status(const unsigned int led,
 
 	int rc = 0;
 
+	printk("LED set %d to %d\n", led, ledstatus);
+
 	switch (led_supported) {
 	case TPACPI_LED_570:
 		/* 570 */
@@ -5876,7 +5878,7 @@ static int led_set_status(const unsigned int led,
 			return -EPERM;
 		if (!acpi_evalf(led_handle, NULL, NULL, "vdd",
 				(1 << led), led_sled_arg1[ledstatus]))
-			rc = -EIO;
+			return -EIO;
 		break;
 	case TPACPI_LED_OLD:
 		/* 600e/x, 770e, 770x, A21e, A2xm/p, T20-22, X20 */
@@ -5900,12 +5902,14 @@ static int led_set_status(const unsigned int led,
 			return -EPERM;
 		if (!acpi_evalf(led_handle, NULL, NULL, "vdd",
 				led, led_led_arg1[ledstatus]))
-			rc = -EIO;
+			return -EIO;
 		break;
 	default:
-		rc = -ENXIO;
+		return -ENXIO;
 	}
 
+	printk("LED set %d to %d... %d\n", led, ledstatus, rc);
+
 	if (!rc)
 		tpacpi_led_state_cache[led] = ledstatus;
 
@@ -5919,6 +5923,8 @@ static int led_sysfs_set(struct led_classdev *led_cdev,
 			     struct tpacpi_led_classdev, led_classdev);
 	enum led_status_t new_state;
 
+	printk("sysfs_set %d\n", brightness);
+
 	if (brightness == LED_OFF)
 		new_state = TPACPI_LED_OFF;
 	else if (tpacpi_led_state_cache[data->led] != TPACPI_LED_BLINK)
@@ -5935,6 +5941,8 @@ static int led_sysfs_blink_set(struct led_classdev *led_cdev,
 	struct tpacpi_led_classdev *data = container_of(led_cdev,
 			     struct tpacpi_led_classdev, led_classdev);
 
+	printk("sysfs_blink_set\n");
+
 	/* Can we choose the flash rate? */
 	if (*delay_on == 0 && *delay_off == 0) {
 		/* yes. set them to the hardware blink rate (1 Hz) */
@@ -5943,6 +5951,7 @@ static int led_sysfs_blink_set(struct led_classdev *led_cdev,
 	} else if ((*delay_on != 500) || (*delay_off != 500))
 		return -EINVAL;
 
+	printk("sysfs_blink_set: hardware can do it\n");
 	return led_set_status(data->led, TPACPI_LED_BLINK);
 }
 

								Pavel

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

Re: [ibm-acpi-devel] Thinkpad ACPI led -- it keeps blinking

[Jacek A. <jac...@gm...>]

On 4/28/19 12:32 AM, Pavel Machek wrote:
> Hi!
> 
> This fixes one problem:
> 
> 									Pavel
> 
> Signed-off-by: Pavel Machek <pa...@uc...>
> 
> diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
> index e3da7c0..d795d8f 100644
> --- a/drivers/leds/led-core.c
> +++ b/drivers/leds/led-core.c
> @@ -164,8 +164,14 @@ static void led_blink_setup(struct led_classdev *led_cdev,
>   		     unsigned long *delay_on,
>   		     unsigned long *delay_off)
>   {
> +	while (work_pending(&led_cdev->set_brightness_work)) {
> +		printk("Waiting for brightness set to finish\n");
> +		schedule();
> +	}

Or even better:

flush_work(&led_cdev->set_brightness_work);

>   	if (!test_bit(LED_BLINK_ONESHOT, &led_cdev->work_flags) &&
>   	    led_cdev->blink_set &&
> +	    /* This can sleep */
>   	    !led_cdev->blink_set(led_cdev, delay_on, delay_off))
>   		return;
>   
> 

-- 
Best regards,
Jacek Anaszewski

Re: [ibm-acpi-devel] Thinkpad ACPI led -- it keeps blinking

[Pavel M. <pa...@uc...>]

Hi!

This fixes one problem:

									Pavel

Signed-off-by: Pavel Machek <pa...@uc...>

diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
index e3da7c0..d795d8f 100644
--- a/drivers/leds/led-core.c
+++ b/drivers/leds/led-core.c
@@ -164,8 +164,14 @@ static void led_blink_setup(struct led_classdev *led_cdev,
 		     unsigned long *delay_on,
 		     unsigned long *delay_off)
 {
+	while (work_pending(&led_cdev->set_brightness_work)) {
+		printk("Waiting for brightness set to finish\n");
+		schedule();
+	}
+		       
 	if (!test_bit(LED_BLINK_ONESHOT, &led_cdev->work_flags) &&
 	    led_cdev->blink_set &&
+	    /* This can sleep */
 	    !led_cdev->blink_set(led_cdev, delay_on, delay_off))
 		return;
 

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

Re: [ibm-acpi-devel] Thinkpad ACPI led -- it keeps blinking

[Pavel M. <pa...@uc...>]

Hi!

It seems quite clear:

static int timer_trig_activate(struct led_classdev *led_cdev)
{
        printk("timer_trig_activate\n");
	if (led_cdev->flags & LED_INIT_DEFAULT_TRIGGER) {
	
        led_blink_set(led_cdev, &led_cdev->blink_delay_on, &led_cdev->blink_delay_off);

	printk("timer_trig_activate done\n");

        return 0;
}

static void timer_trig_deactivate(struct led_classdev *led_cdev)
{
        printk("timer_trig_deactivate\n");

	/* Stop blinking */
	led_set_brightness(led_cdev, LED_OFF);

        printk("timer_trig_deactivate done\n");
}

We get timer_trig_deactivate() immediately followed by
timer_trig_activate().

set_brightness goes to workqueue because it would block. That means
that blink_set() happens before set_brightness...

								Pavel

[   16.194141] e1000e 0000:02:00.0 eth1: 10/100 speed: disabling TSO
[  145.887931] timer_trig_activate
[  145.888011] LED set 0 to 2
[  145.888879] LED set 0 to 2... 0
[  145.888893] timer_trig_activate done
[  149.977138] timer_trig_deactivate
[  149.977169] timer_trig_deactivate done
[  149.977479] timer_trig_activate
[  149.977497] LED set 0 to 2
[  149.978281] LED set 0 to 2... 0
[  149.978295] timer_trig_activate done
[  149.978415] LED set 0 to 0
[  149.979851] LED set 0 to 0... 0
[  184.839252] timer_trig_deactivate
[  184.839282] timer_trig_deactivate done
[  184.839319] timer_trig_activate
[  184.839337] LED set 0 to 2
[  184.839907] LED set 0 to 0
[  184.840369] LED set 0 to 2... 0
[  184.840383] timer_trig_activate done
[  184.843318] LED set 0 to 0... 0
root@amd:/sys/class/leds/tpacpi::power#

diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
index e3da7c0..ca1f69b 100644
--- a/drivers/leds/led-core.c
+++ b/drivers/leds/led-core.c
@@ -166,6 +166,7 @@ static void led_blink_setup(struct led_classdev *led_cdev,
 {
 	if (!test_bit(LED_BLINK_ONESHOT, &led_cdev->work_flags) &&
 	    led_cdev->blink_set &&
+	    /* This can sleep */
 	    !led_cdev->blink_set(led_cdev, delay_on, delay_off))
 		return;
 
diff --git a/drivers/leds/trigger/ledtrig-timer.c b/drivers/leds/trigger/ledtrig-timer.c
index ca898c1..0b061bb5 100644
--- a/drivers/leds/trigger/ledtrig-timer.c
+++ b/drivers/leds/trigger/ledtrig-timer.c
@@ -104,6 +104,7 @@ static void pattern_init(struct led_classdev *led_cdev)
 
 static int timer_trig_activate(struct led_classdev *led_cdev)
 {
+	printk("timer_trig_activate\n");
 	if (led_cdev->flags & LED_INIT_DEFAULT_TRIGGER) {
 		pattern_init(led_cdev);
 		/*
@@ -115,14 +116,18 @@ static int timer_trig_activate(struct led_classdev *led_cdev)
 
 	led_blink_set(led_cdev, &led_cdev->blink_delay_on,
 		      &led_cdev->blink_delay_off);
+	printk("timer_trig_activate done\n");
 
 	return 0;
 }
 
 static void timer_trig_deactivate(struct led_classdev *led_cdev)
 {
+	printk("timer_trig_deactivate\n");
 	/* Stop blinking */
 	led_set_brightness(led_cdev, LED_OFF);
+
+	printk("timer_trig_deactivate done\n");
 }
 
 static struct led_trigger timer_led_trigger = {
diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index 3a4402a..b3fa9c9 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -363,11 +363,11 @@ int mmc_of_parse_voltage(struct device_node *np, u32 *mask)
 	int num_ranges, i;
 
 	voltage_ranges = of_get_property(np, "voltage-ranges", &num_ranges);
-	num_ranges = num_ranges / sizeof(*voltage_ranges) / 2;
 	if (!voltage_ranges) {
 		pr_debug("%pOF: voltage-ranges unspecified\n", np);
 		return 0;
 	}
+	num_ranges = num_ranges / sizeof(*voltage_ranges) / 2;
 	if (!num_ranges) {
 		pr_err("%pOF: voltage-ranges empty\n", np);
 		return -EINVAL;
diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index 57d9ae9..3ec6636 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -5867,6 +5867,8 @@ static int led_set_status(const unsigned int led,
 
 	int rc = 0;
 
+	printk("LED set %d to %d\n", led, ledstatus);
+
 	switch (led_supported) {
 	case TPACPI_LED_570:
 		/* 570 */
@@ -5876,7 +5878,7 @@ static int led_set_status(const unsigned int led,
 			return -EPERM;
 		if (!acpi_evalf(led_handle, NULL, NULL, "vdd",
 				(1 << led), led_sled_arg1[ledstatus]))
-			rc = -EIO;
+			return -EIO;
 		break;
 	case TPACPI_LED_OLD:
 		/* 600e/x, 770e, 770x, A21e, A2xm/p, T20-22, X20 */
@@ -5900,12 +5902,14 @@ static int led_set_status(const unsigned int led,
 			return -EPERM;
 		if (!acpi_evalf(led_handle, NULL, NULL, "vdd",
 				led, led_led_arg1[ledstatus]))
-			rc = -EIO;
+			return -EIO;
 		break;
 	default:
-		rc = -ENXIO;
+		return -ENXIO;
 	}
 
+	printk("LED set %d to %d... %d\n", led, ledstatus, rc);
+
 	if (!rc)
 		tpacpi_led_state_cache[led] = ledstatus;
 

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

Re: [ibm-acpi-devel] Thinkpad ACPI led -- it keeps blinking

[Pavel M. <pa...@uc...>]

On Sat 2019-04-27 22:35:35, Jacek Anaszewski wrote:
> On 4/27/19 9:34 PM, Pavel Machek wrote:
> >On Sat 2019-04-27 18:55:37, Jacek Anaszewski wrote:
> >>On 4/26/19 11:42 PM, Pavel Machek wrote:
> >>>Hi!
> >>>
> >>>>Kernel 5.1.0-rc1 + some unrelated bits.
> >>>
> >>>I tried adding
> >>>https://marc.info/?l=linux-kernel&m=151622365715313&q=raw as Jacek
> >>>suggested, and it is still broken.
> >>>
> >>>Test code is this: ledtest actually works as expected on first try,
> >>>but keeps blinking on second run. Strange.
> >>
> >>Did it work for previous releases? If yes, then bisect should help here.
> >
> >Absolutely no idea :-(. I assume "no". Capslock LED on the same system
> >works as expected.
> >
> >I still hope thinkpad people will speak up, notice it does not work
> >for them, and debug it :-).
> 
> I see this driver implements blink_set:
> 
> tpacpi_leds[led].led_classdev.blink_set = &led_sysfs_blink_set;
> 
> and also applies some internal logic related to the type
> of supported LEDs, and the way how the hardware is accessed.
> 
> To eliminate the problem on the LED core side you could
> disable initialization of blink_set op in the driver.

Yep, software blinking will very likely work ok.

[  226.949924] LED set 0 to 2
[  226.950766] LED set 0 to 2... 0
[  232.613577] LED set 0 to 2
[  232.613991] LED set 0 to 0
[  232.614467] LED set 0 to 2... 0
[  232.616442] LED set 0 to 0... 0

Thinkpad ACPI driver is being asked to turn led to blink [232.613577]
LED set 0 to 2 and turn it off [  232.613991] LED set 0 to 0
simultaneously. It has no internal locking and ACPI is slow. That
can't end well.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [ibm-acpi-devel] Thinkpad ACPI led -- it keeps blinking

[Jacek A. <jac...@gm...>]

On 4/27/19 9:34 PM, Pavel Machek wrote:
> On Sat 2019-04-27 18:55:37, Jacek Anaszewski wrote:
>> On 4/26/19 11:42 PM, Pavel Machek wrote:
>>> Hi!
>>>
>>>> Kernel 5.1.0-rc1 + some unrelated bits.
>>>
>>> I tried adding
>>> https://marc.info/?l=linux-kernel&m=151622365715313&q=raw as Jacek
>>> suggested, and it is still broken.
>>>
>>> Test code is this: ledtest actually works as expected on first try,
>>> but keeps blinking on second run. Strange.
>>
>> Did it work for previous releases? If yes, then bisect should help here.
> 
> Absolutely no idea :-(. I assume "no". Capslock LED on the same system
> works as expected.
> 
> I still hope thinkpad people will speak up, notice it does not work
> for them, and debug it :-).

I see this driver implements blink_set:

tpacpi_leds[led].led_classdev.blink_set = &led_sysfs_blink_set;

and also applies some internal logic related to the type
of supported LEDs, and the way how the hardware is accessed.

To eliminate the problem on the LED core side you could
disable initialization of blink_set op in the driver.

-- 
Best regards,
Jacek Anaszewski

Re: [ibm-acpi-devel] Thinkpad ACPI led -- it keeps blinking

[Pavel M. <pa...@uc...>]

On Sat 2019-04-27 18:55:37, Jacek Anaszewski wrote:
> On 4/26/19 11:42 PM, Pavel Machek wrote:
> >Hi!
> >
> >>Kernel 5.1.0-rc1 + some unrelated bits.
> >
> >I tried adding
> >https://marc.info/?l=linux-kernel&m=151622365715313&q=raw as Jacek
> >suggested, and it is still broken.
> >
> >Test code is this: ledtest actually works as expected on first try,
> >but keeps blinking on second run. Strange.
> 
> Did it work for previous releases? If yes, then bisect should help here.

Absolutely no idea :-(. I assume "no". Capslock LED on the same system
works as expected.

I still hope thinkpad people will speak up, notice it does not work
for them, and debug it :-).
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [ibm-acpi-devel] Thinkpad ACPI led -- it keeps blinking

[Jacek A. <jac...@gm...>]

On 4/26/19 11:42 PM, Pavel Machek wrote:
> Hi!
> 
>> Kernel 5.1.0-rc1 + some unrelated bits.
> 
> I tried adding
> https://marc.info/?l=linux-kernel&m=151622365715313&q=raw as Jacek
> suggested, and it is still broken.
> 
> Test code is this: ledtest actually works as expected on first try,
> but keeps blinking on second run. Strange.

Did it work for previous releases? If yes, then bisect should help here.

-- 
Best regards,
Jacek Anaszewski

Re: [ibm-acpi-devel] Thinkpad ACPI led -- it keeps blinking

[Pavel M. <pa...@uc...>]

Hi!

> Kernel 5.1.0-rc1 + some unrelated bits.

I tried adding
https://marc.info/?l=linux-kernel&m=151622365715313&q=raw as Jacek
suggested, and it is still broken.

Test code is this: ledtest actually works as expected on first try,
but keeps blinking on second run. Strange.

								Pavel

#!/usr/bin/python
# -*- python -*-
# Copyright Bluez project, GPLv2
# Adapted from test/monitor-bluetooth

from __future__ import absolute_import, print_function, unicode_literals

import sys
import time
import os

import dbus
import dbus.mainloop.glib
try:
  from gi.repository import GObject
except ImportError:
  import gobject as GObject

relevant_ifaces = [ "org.bluez.Device1" ]

class LED:
	def __init__(self):
        	self.path = "/sys/class/leds/tpacpi::power/"
        	#self.path = "/sys/class/leds/input5::capslock/"
                self.brightness = self.path + "brightness"
                self.trigger = self.path + "trigger"

        def set(self, name, val):
        	f = open(name, "w")
                f.write(val)
                f.close()

        def solid(self, b):
        	self.set(self.trigger, "none")
        	self.set(self.brightness, "0")
        	self.set(self.brightness, str(b))

        def blink(self):
        	self.set(self.trigger, "timer")

        def as_root(self):
        	os.chmod(self.trigger, 0777)
        	os.chmod(self.brightness, 0777)

def led_test():
	l = LED()
        l.solid(0)
        time.sleep(2)
	l.solid(1)
        time.sleep(2)
        l.blink()
        time.sleep(2)
	l.solid(1)

def handle_params(args):
        if len(args) < 2 or args[1] == "run":
                LED().blink()
        	run()
                return
        if args[1] == "ledtest":
        	led_test()
                return
        if args[1] == "stop":
                LED().solid(0)
        if args[1] == "startup":
        	LED().as_root()
                return

        print("Unknown parameters", args)

handle_params(sys.argv)




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