Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754596AbcKON2t (ORCPT ); Tue, 15 Nov 2016 08:28:49 -0500 Received: from mailout3.w1.samsung.com ([210.118.77.13]:27997 "EHLO mailout3.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752214AbcKON2p (ORCPT ); Tue, 15 Nov 2016 08:28:45 -0500 X-AuditID: cbfec7f1-f79f46d0000008eb-58-582b0d892957 Subject: Re: LEDs that change brightness "itself" -- that's a trigger. Re: PM regression with LED changes in next-20161109 To: Hans de Goede , Pavel Machek 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: Jacek Anaszewski Message-id: <9e7d5e0f-c4ca-6930-63b9-83dc28517f33@samsung.com> Date: Tue, 15 Nov 2016 14:28:39 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-version: 1.0 In-reply-to: Content-type: text/plain; charset=windows-1252; format=flowed Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFlrAKsWRmVeSWpSXmKPExsWy7djP87qdvNoRBi33DSy6FhpYvDk+ncni 9tYNLBabHl9jtbi8aw6bxdY36xgtZi/pZ7G4e+oom8X+K14OnB7fvk5i8dg56y67x+YVWh6b l9R7vN93lc1jxerv7B6fN8kFsEdx2aSk5mSWpRbp2yVwZaxfe5utoEOi4s6ZqcwNjKuEuxg5 OSQETCSWbj/MCGGLSVy4t56ti5GLQ0hgKaPEkZWTGCGcz4wSqzY+YIfpeDRlLitEYhmjxJxz 61lBEkICzxglXrz2AbGFBWokLm1uZu5i5OAQEXCRaG/2BqlnFvjJKNG7vosNpIZNwFDi54vX TCA2r4CdxLXXp8DmsAioSvw9c5QNpFdUIEJi991UiBJBiR+T77GA2JxA5esmXQAbwyzgKPFg 0U5WCFteYvOat8wQdx5il1jzKAZkjISArMSmA1BhF4nmE2ehHhaWeHV8C9RbMhKdHQeZQM6U EJjMKHHx2E1WCGc1o8TGzk4WiCpriYb/v1gglvFJTNo2nRliAa9ER5sQRImHxK09Z6CGOkq0 brgLDcSbLBLLbk5knsAoPwvJP7OQ/DALyQ8LGJlXMYqklhbnpqcWG+kVJ+YWl+al6yXn525i BKab0/+Of9zB+P6E1SFGAQ5GJR7eHUc1I4RYE8uKK3MPMUpwMCuJ8FqyakcI8aYkVlalFuXH F5XmpBYfYpTmYFES592z4Eq4kEB6YklqdmpqQWoRTJaJg1OqgZH7n9SWp8wJ4al7Ip/E2mxh umnoWXXZZE5UpZSsdJZk3ln5BQ6zb3h1Xt/cI25toh/UseSf2y7bvFccZWlFLHd+7fSoKAo7 Hbz3uAW/WNC/2lV57+qY/88O+VD0cs4dplr5HTPzlzmxvV50vUuuzcM7dFpitPJX77bpFrqS T2dt2aiRd5ytQ4mlOCPRUIu5qDgRAIbVzJMzAwAA X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFjrEIsWRmVeSWpSXmKPExsVy+t/xa7p9vNoRBgcnS1h0LTSweHN8OpPF 7a0bWCw2Pb7GanF51xw2i61v1jFazF7Sz2Jx99RRNov9V7wcOD2+fZ3E4rFz1l12j80rtDw2 L6n3eL/vKpvHitXf2T0+b5ILYI9ys8lITUxJLVJIzUvOT8nMS7dVCg1x07VQUshLzE21VYrQ 9Q0JUlIoS8wpBfKMDNCAg3OAe7CSvl2CW8b6tbfZCjokKu6cmcrcwLhKuIuRk0NCwETi0ZS5 rBC2mMSFe+vZuhi5OIQEljBKPLu7mBHCecYo8fn4ZmaQKmGBGonT078AJTg4RARcJNqbvSFq rrJIPOp8wQTiMAv8ZJT482oLC0gDm4ChxM8Xr5lAbF4BO4lrr0+BrWMRUJX4e+YoG4gtKhAh cWvVR0aIGkGJH5PvgfVyAtWvm3QBrIZZwFZiwft1LBC2vMTmNW+ZJzAKzELSMgtJ2SwkZQsY mVcxiqSWFuem5xYb6hUn5haX5qXrJefnbmIExt+2Yz8372C8tDH4EKMAB6MSD++Oo5oRQqyJ ZcWVuYcYJTiYlUR4LVm1I4R4UxIrq1KL8uOLSnNSiw8xmgI9MZFZSjQ5H5ga8kriDU0MzS0N jYwtLMyNjJTEeUs+XAkXEkhPLEnNTk0tSC2C6WPi4JRqYGS5EvOHNfuWHUfV2nfbHc7H6WV3 idioLmib9nyDmEnx9x3ZMxjSdmWuVuAOWV51rCFNxTvj4Z2s5XP2dvotX3P96P33CaseK4Rf W5ccy2gT+W/ZvC8PDv7lOfvho6gKr6iPianUCd7tq74ebPnebHct5mXRn7AINpGN3/8sP1Ow Y5qZVkvH5XVKLMUZiYZazEXFiQBagrE21QIAAA== X-MTR: 20000000000000000@CPGS X-CMS-MailID: 20161115132841eucas1p1f5313b8bd43b7237141c0851c59d6b37 X-Msg-Generator: CA X-Sender-IP: 182.198.249.179 X-Local-Sender: =?UTF-8?B?SmFjZWsgQW5hc3pld3NraRtTUlBPTC1TeXN0ZW0gRlcgIChN?= =?UTF-8?B?Qikb7IK87ISx7KCE7J6QG1NlbmlvciBTb2Z0d2FyZSBFbmdpbmVlcg==?= X-Global-Sender: =?UTF-8?B?SmFjZWsgQW5hc3pld3NraRtTUlBPTC1TeXN0ZW0gRlcgIChN?= =?UTF-8?B?QikbU2Ftc3VuZyBFbGVjdHJvbmljcxtTZW5pb3IgU29mdHdhcmUgRW5naW5l?= =?UTF-8?B?ZXI=?= X-Sender-Code: =?UTF-8?B?QzEwG0VIURtDMTBDRDAyQ0QwMjc1MjY=?= CMS-TYPE: 201P X-HopCount: 7 X-CMS-RootMailID: 20161115103141epcas2p251f2521354a8cd7b9dfcfe55bfa482f0 X-RootMTR: 20161115103141epcas2p251f2521354a8cd7b9dfcfe55bfa482f0 References: <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> <20161115111154.GA5482@amd> <128aae59-b790-42f1-7d66-81391c9330c3@redhat.com> <20161115114859.GA7018@amd> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2997 Lines: 83 On 11/15/2016 01:06 PM, Hans de Goede wrote: > Hi, > > On 15-11-16 12:48, Pavel Machek wrote: >> Hi! >> >>>>>> 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. >>>> >>>> Thanks. >>>> >>>>> 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? >>>> >>>> Trigger exposes a file in sysfs, with poll() working on that file >>> >>> Hmm, a new file would give the advantage of making it easy for >>> userspace to see if the trigger is poll-able, this is likely >>> better then my own proposal I just send. >> >> Good. >> >>>> (and >>>> probably read exposing the current brightness). >>> >>> If we do this, can we please make it mirror brightness, iow >>> also make it writable, that will make it easier for userspace >>> to deal with it. We can simply re-use the existing show / store >>> methods for brightness for this. >> >> Actually, echo 0 > brightness disables the trigger, IIRC. I'd avoid >> that here, you want to be able to turn off the backlight but still >> keep the trigger (and be notified of future changes). > > True, that is easy to do the store method will just need to call > led_set_brightness_nosleep instead of led_set_brightness, this > will skip the checks to stop blinking in led_set_brightness and > otherwise is equivalent. > >>> I suggest we call it: >>> >>> trigger_brightness >>> >>> And only register it when a poll-able trigger is present. >> >> I'd call it 'current_brightness', but that's no big deal. Yes, only >> registering it for poll-able triggers makes sense. > > current_brightness works for me. I will take a shot a patch-set > implementing this. Word "current" is not precise here. It can be thought of as either last brightness set by the user or the brightness currently written to the device (returned by brightness file). There is a semantic discrepancy in our requirements - we want the file representing both permanent brightness set by the user and brightness set by the hardware. The two stand in contradiction to each other since brightness set by the user can be adjusted by the hardware. Reading the file shouldn't update brightness property of struct led_classdev, so it shouldn't call led_update_brightness() but it still should allow reading brightness set by the hardware, as a result of each POLLPRI event. So in fact in the same time it should report both according to our requirements which is impossible. Do we need three brightness files ? -- Best regards, Jacek Anaszewski