Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752479AbcKOLRH (ORCPT ); Tue, 15 Nov 2016 06:17:07 -0500 Received: from mx1.redhat.com ([209.132.183.28]:58740 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752007AbcKOLRE (ORCPT ); Tue, 15 Nov 2016 06:17:04 -0500 Subject: Re: LEDs that change brightness "itself" -- that's a trigger. Re: PM regression with LED changes in next-20161109 To: Jacek Anaszewski , Pavel Machek References: <4c31faef-144d-289c-0e32-83e76aff6178@gmail.com> <3eb60c78-d891-27e5-6b7b-a54a5b547a1c@redhat.com> <9b476f85-d45e-deb6-335d-fc56f6d90350@redhat.com> <127cdd42-6fd8-c671-60b7-3826b351577f@samsung.com> <15cafbf5-d842-e184-2fd4-65f8272f505a@redhat.com> <20161115103133.GA22860@amd> <4e392d5d-eb10-f285-517e-976a55c3e318@samsung.com> Cc: Jacek Anaszewski , Tony Lindgren , linux-leds@vger.kernel.org, linux-omap@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Darren Hart From: Hans de Goede Message-ID: <2f04d861-d2b9-8f53-6ebf-fe39bf73463a@redhat.com> Date: Tue, 15 Nov 2016 12:17:00 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <4e392d5d-eb10-f285-517e-976a55c3e318@samsung.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.29]); Tue, 15 Nov 2016 11:17:04 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2997 Lines: 84 HI, On 15-11-16 11:58, Jacek Anaszewski wrote: > On 11/15/2016 11:31 AM, Pavel Machek wrote: >> Hi! >> >>>> Hmm, v4 still calls led_notify_brightness_change(led_cdev) >>> >from both __led_set_brightness() and __led_set_brightness_blocking(). >>> >>> Ugh, I see I accidentally send a v4 twice, instead of >>> calling the version which dropped those called v5 as >>> I should have, sorry. >>> >>> The v4 which I would like to see merged, the one with >>> those calls dropped, is here: >>> >>> https://patchwork.kernel.org/patch/9423093/ >> >> Please, lets fix this properly. >> >> The LED you are talking about _has_ a trigger, implemented in >> hardware. That trigger can change LED brightness behind kernel's (and >> userspace's) back. Don't pretend the trigger does not exist, it does. >> >> And when you do that, you'll have nice place to report changes to >> userspace -- trigger can now export that information, and offer poll() >> interface. > > Well, that sounds interesting. It is logically justifiable. > I initially proposed exactly this solution, with recently > added userspace LED being a trigger listener. It seems a bit > awkward though. How would you listen to the trigger events? We could make the trigger sysfs attribute poll()-able, but only for select triggers, e.g.: Documentation/ABI/testing/sysfs-class-led What: /sys/class/leds//trigger Date: March 2006 KernelVersion: 2.6.17 Contact: Richard Purdie Description: Set the trigger for this LED. A trigger is a kernel based source of led events. You can change triggers in a similar manner to the way an IO scheduler is chosen. Trigger specific parameters can appear in /sys/class/leds/ once a given trigger is selected. For their documentation see sysfs-class-led-trigger-*. + + For some triggers userspace my poll() this file, watching for + POLL_PRI to detect when the trigger triggers. This is only + supported if this is explicitly mentioned as supported in + sysfs-class-led-trigger-* for the selected trigger. The reason for making this only supported for select triggers is to avoid getting the whole power-consumption issue from triggers which fire frequently again. And then we could add a new: Documentation/ABI/testing/sysfs-class-led-trigger-kbd-backlight-change File which documents that the new to be added kbd-backlight-change trigger is poll-able. We would also need: --- a/include/linux/leds.h +++ b/include/linux/leds.h @@ -47,6 +47,7 @@ struct led_classdev { #define LED_DEV_CAP_FLASH (1 << 18) #define LED_HW_PLUGGABLE (1 << 19) #define LED_PANIC_INDICATOR (1 << 20) +#define LED_TRIGGER_READ_ONLY (1 << 21) /* set_brightness_work / blink_timer flags, atomic, private. */ unsigned long work_flags; To allow led drivers to indicate that there trigger is hardwired. Regards, Hans