Hi, On 11/24/21 20:13, Mark Pearson wrote: > > > On 2021-11-24 11:28, Hans de Goede wrote: >> Hi, >> >> On 11/24/21 16:53, Thomas Weißschuh wrote: >>> Hi, >>> >>> On 2021-11-23 22:05+0100, Hans de Goede wrote: >>>> There have been various bugs / forum threads about allowing control of >>>> the LED in the ThinkPad logo on the lid of various models. >>>> >>>> This seems to be something which users want to control and there really >>>> is no reason to require setting CONFIG_THINKPAD_ACPI_UNSAFE_LEDS for this. >>>> >>>> The lid-logo-led is LED number 10, so change the name of the 10th led >>>> from unknown_led2 to lid_logo_led and add it to the TPACPI_SAFE_LEDS mask. >>>> >>>> Link: https://www.reddit.com/r/thinkpad/comments/7n8eyu/thinkpad_led_control_under_gnulinux/>>>> BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1943318>>>> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx> >>>> --- >>>> drivers/platform/x86/thinkpad_acpi.c | 4 ++-- >>>> 1 file changed, 2 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c >>>> index 49fdf16b2db9..28f0299ecab0 100644 >>>> --- a/drivers/platform/x86/thinkpad_acpi.c >>>> +++ b/drivers/platform/x86/thinkpad_acpi.c >>>> @@ -5661,11 +5661,11 @@ static const char * const tpacpi_led_names[TPACPI_LED_NUMLEDS] = { >>>> "tpacpi::standby", >>>> "tpacpi::dock_status1", >>>> "tpacpi::dock_status2", >>>> - "tpacpi::unknown_led2", >>>> + "tpacpi::lid_logo_led", >>> >>> The suffix "_led" looks a bit redundant. Also non of the other LEDs have it. >> >> Hmm, good point, but without the _led to me it sounds as if it controls >> some backlight for the entire logo, where it really is just the dot of the i. >> >> So I'm not sure what to do here :) >> >>> Also currently the reported brightness is 0 before writing to it, although the >>> LED is powered on by default, not sure how this could be fixed though. >> >> Right, this is a known short-coming of the tpacpi LED interface, LEDs can be >> set but you cannot get the current status. >> >> And once set, the LED is now fully under usercontrol, until the next reboot >> (or maybe even power-cycle). >> > Apart from being vaguely fascinated that people want to play with the > LED (I assume because it's annoying and a small waste of power?) Some people just find the LED annoying so they just want to turn it off I believe. Others want to repurpose it for their own purposes. > is this > something that I should put in a request for an API to get the LED status? I don't believe not being able to read the status is really a big deal, with that said it would be nice to have, but definitely a low priority item. > I would like to get the FW teams point of view here too. We use the LED > to show if the system is suspended or not so I'm somewhat curious as to > what happens if a user overrides the setting. AFAIK once the user has overridden the value it stays at the user selected value until a reboot or power-cycle. Note that most thinkpads also make the power-button-led "glow" when suspended, and we already allow overriding this. The reason why thinkpad_acpi has the notion of safe LEDs is to disallow overriding some LEDs in older docks which indicate when it is safe to unplug the laptop. In that case overriding the LEDs could be quite bad but here we just loose "I'm suspended" notification which may be annoying if the user is waiting for the LED to start glowing after closing the lid (and before say bagging the laptop). But presumably a user deliberately overriding the LED knows it is not going to glow now. > I doubt it's a big deal > but I'd like to double check if there are any gotcha's. Sounds good, thanks. Regards, Hans
