From mboxrd@z Thu Jan 1 00:00:00 1970 From: Pavel Machek Subject: Re: [PATCH v5 2/6] leds: triggers: Add a keyboard backlight trigger Date: Fri, 25 Nov 2016 10:29:05 +0100 Message-ID: <20161125092905.GA2304@amd> References: <20161117222441.31464-1-hdegoede@redhat.com> <55cdf83d-2233-151f-08e1-11d4619e8fd5@redhat.com> <7b8252c4-bb2a-01dd-2404-9b81c192fb6a@gmail.com> <201611201605.17631@pali> <20161120162116.GA15737@amd> <0c4e7840-064c-5d8a-c5eb-8afe71727fcd@samsung.com> <20161121114140.GA5094@amd> <3e31ec14-8eae-0a5e-0013-f30225b46e1a@samsung.com> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="h31gzZEtNLTqOjlF" Return-path: Received: from atrey.karlin.mff.cuni.cz ([195.113.26.193]:52250 "EHLO atrey.karlin.mff.cuni.cz" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750765AbcKYJ3I (ORCPT ); Fri, 25 Nov 2016 04:29:08 -0500 Content-Disposition: inline In-Reply-To: <3e31ec14-8eae-0a5e-0013-f30225b46e1a@samsung.com> Sender: linux-leds-owner@vger.kernel.org List-Id: linux-leds@vger.kernel.org To: Jacek Anaszewski Cc: Pali =?iso-8859-1?Q?Roh=E1r?= , Jacek Anaszewski , Hans de Goede , 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 --h31gzZEtNLTqOjlF Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Hi! > >>>>As pointed in other email, we do not know if HW really controls keybo= ard backlight, > >>>>so adding "fake" trigger on machines without HW control is not a good= idea. > >>> > >>>Well, if we know that hardware will not change the brightness on its > >>>own, yes, I'd avoid the trigger. If we don't know (as is common on > >>>ACPI machines, I'd keep the trigger). > >> > >>I'd drop the trigger approach due to the mess it can make in peoples' > >>minds due to the fact that LED class device handles trigger events > >>generated by itself. > > > >We can teach people. IMO the LED that changes itself is special, and > >trigger explains that nicely to the userspace. Plus, it allows us to > >keep this functionality out of the core. >=20 > Please refer to the downsides of this appraoch: >=20 > - lack of information if given LED class device supports hw > generated POLLPRI events Userspace can plainly see that a trigger is active, and knows to expect=20 > - impossible to apply other trigger while polling That's a good thing. We _don't_ want polling to be active when trigger such as "CPU active" is active. We want userspace to monitor hardware events but not software ones. > >>I'd add a file hw_brightness_change or async_brightness or something > >>similar and make it only readable/pollable. current_brightness is > >>ambiguous and questionable. > > > >Well, exact name is not too important... >=20 > The name should clearly explain the file purpose. I bet that we would > see many questions once the file appeared in the mainline. > Also, I'm afraid that I wouldn't be able to explain this name in > few simple words, without daunting the listener, or even triggering > the discussion on brightness shortcomings we've already gone through. Well, feel free to suggest non-confusing name. Unfortunately, yes, we'll need to do some explaining, as existing "brightness" behaviour is already pretty tricky / confusing / counterintuitive. Lets at least make sure new additions are clean and simple. Thanks, Pavel --=20 (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blo= g.html --h31gzZEtNLTqOjlF Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iEYEARECAAYFAlg4BGEACgkQMOfwapXb+vJVfwCgwo8WE4G2dBFkDfj9Viw+xcKw aDoAoKfjsRSrF+CYKtorjtOAB+Trm/uk =hIQB -----END PGP SIGNATURE----- --h31gzZEtNLTqOjlF--