From mboxrd@z Thu Jan 1 00:00:00 1970 From: Hans de Goede Subject: Re: [PATCH v5 2/6] leds: triggers: Add a keyboard backlight trigger Date: Fri, 25 Nov 2016 12:14:56 +0100 Message-ID: References: <05766b18-026a-2af3-def8-9289ddb55234@samsung.com> <201611241751.27696@pali> <5238be1f-d669-07e6-c796-5bc0126cb456@gmail.com> <201611242245.00217@pali> <20161125100140.GC4062@amd> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Return-path: Received: from mx1.redhat.com ([209.132.183.28]:60882 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752992AbcKYLPB (ORCPT ); Fri, 25 Nov 2016 06:15:01 -0500 In-Reply-To: <20161125100140.GC4062@amd> Sender: linux-leds-owner@vger.kernel.org List-Id: linux-leds@vger.kernel.org To: Pavel Machek , Jacek Anaszewski Cc: =?UTF-8?Q?Pali_Roh=c3=a1r?= , Jacek Anaszewski , gdg@zplane.com, Darren Hart , Matthew Garrett , Henrique de Moraes Holschuh , Richard Purdie , ibm-acpi-devel@lists.sourceforge.net, platform-driver-x86@vger.kernel.org, linux-leds@vger.kernel.org Hi, On 25-11-16 11:01, Pavel Machek wrote: > Hi! > >> In view of the above we could report hw brightness changes with POLLPRI >> on brightness file, but unfortunately we can't because it is impossible >> to guarantee that readout of brightness file will return the brightness >> the POLLPRI was meant to notify about. > > Agreed here. > >> That's why a separate read only file seems to be the only proper >> solution. > > Yes please. And lets make self-changing leds into a trigger, as > proposed, and as Hans' patch should be already doing. > >> Moreover, the file should return the brightness from the time >> of last POLLPRI. > > Not sure I agree here. Normally, kernel returns current state for > variables, does not track "old" state. Agreed, storing the last state just unnecessarily complicates things. So do we have a consensus on implementing a new hw_brightness_change sysfs attribute now, which only some LEDs will have, can be polled to detect changed done autonomously by the hardware and returns the current / actual LED brightness when read ? As for the modeling how the hotkey controls the LED as a trigger, although I do like this from one pov, I can see Jacek's point that this is confusing as there really is nothing to configure here, where as normally a user could do "echo none > trigger" to break the link. So I think that is best (cleanest /minimal non confusing API) with just the hw_brightness_change sysfs-attribute and not model this as a trigger. That, or fall back to my latest patch-set as posted, I still like that one the most. Regards, Hans