From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jacek Anaszewski Subject: Re: PM regression with LED changes in next-20161109 Date: Mon, 14 Nov 2016 10:12:55 +0100 Message-ID: <127cdd42-6fd8-c671-60b7-3826b351577f@samsung.com> References: <20161109192301.GS26979@atomide.com> <28234714-3994-6747-9cf8-1ff0b3257f7a@gmail.com>

<5bd5333e-0dbb-6333-0a48-ca4d3a990f9c@samsung.com> <20161110162925.GA28832@amd> <20161110175537.GF27724@atomide.com> <20161110202910.GE28832@amd> <80b645e7-c3fa-8001-d9b1-c3c8c40394fd@gmail.com> <20161111120114.GA1076@amd>

<4c31faef-144d-289c-0e32-83e76aff6178@gmail.com> <3eb60c78-d891-27e5-6b7b-a54a5b547a1c@redhat.com>

<9b476f85-d45e-deb6-335d-fc56f6d90350@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-reply-to: <9b476f85-d45e-deb6-335d-fc56f6d90350@redhat.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=m.gmane.org@lists.infradead.org To: Hans de Goede , Jacek Anaszewski , Pavel Machek Cc: Tony Lindgren , linux-kernel@vger.kernel.org, linux-leds@vger.kernel.org, Darren Hart , linux-omap@vger.kernel.org, linux-arm-kernel@lists.infradead.org List-Id: linux-leds@vger.kernel.org Hi, On 11/13/2016 02:52 PM, Hans de Goede wrote: > Hi, > > On 13-11-16 12:44, Jacek Anaszewski wrote: >> Hi, >> >> On 11/12/2016 10:14 PM, Hans de Goede wrote: > > > >>>>> So I would like to propose creating a new read-write >>>>> user_brightness file. >>>>> >>>>> The write behavior would be 100% identical to the brightness >>>>> file (in code terms it will call the same store function). >>>>> >>>>> The the read behavior otoh will be different: it will shows >>>>> the last brightness as set by the user, this would show the >>>>> read behavior we really want of brightness: show the real >>>>> brightness when not blinking / triggers are active and show >>>>> the brightness used when on when blinking / triggers are active. >>>>> >>>>> We could then add poll support on this new user_brightness >>>>> file, thus avoiding the problem with the extra cpu-load on >>>>> notifications on blinking / triggers. >>>> >>>> I agree that user_brightness allows to solve the issues you raised >>>> about inconsistent write and read brightness' semantics >>>> (which is not that painful IMHO). >>>> >>>> Reporting non-user brightness changes on user_brightness file >>>> doesn't sound reasonable though. >>> >>> The changes I'm interested in are user brightness changes they >>> are just not done through sysfs, but through a hardwired hotkey, >>> they are however very much done by the user. >> >> Ah, so this file name would be misleading especially taking into account >> the context in which "user" is used in kernel, which predominantly >> means "userspace", e.g. copy_to_user(), copy_from_user(). >> >>>> Also, how would we read the >>>> brightness set by the firmware? We'd have to read brightness >>>> file, so still two files would have to be opened which is >>>> a second drawback of this approach. >>> >>> No, look carefully at the definition of the read behavior >>> I plan to put in the ABI doc: >> >> OK, "user" was what confused me. So in this case changes made >> by the firmware even if in a result of user activity >> (pressing hardware key) obviously cannot be treated similarly >> to the changes made from the userspace context. > > In the end both result on the brightness of the device > changing, so any userspace process interested in monitoring > the brightness will want to know about both type of changes. > >> Unless you're able to give references to the kernel code which >> contradict my judgement. > > AFAIK the audio code will signal volume changes done by > hardwired buttons the same way as audio changes done > by userspace calling into the kernel. This also makes > sense because in the end, what is interesting for a > mixer app, is that the volume changed, and what the > new volume is. OK, so it is indeed similar to your LED use case. Nonetheless in case of LED controllers it is also possible that hardware adjusts LED brightness in case of low battery voltage. If a device is able e.g. to generate an interrupt to notify this kind of event, then we would like also to be able to notify the client about that. It wouldn't be user generated brightness change though. >>> "Reading this file will return the actual led brightness >>> when not blinking and no triggers are active; reading this >>> file will return the brightness used when the led is on >>> when blinking or triggers are active." >> >> This is unnecessarily entangled. Blinking means timer trigger >> is active. > > Ok. > >>> So for e.g. the backlit keyboard case reading this single >>> file will return the actual brightness of the backlight, >>> since this does not involve blinking or triggers. >>> >>> Basically the idea is that the user_brightness file >>> will have the semantics which IMHO the brightness file >>> itself should have had from the beginning, but which >>> we can't change now due to ABI reasons. >> >> And in fact introducing user_brightness file would indeed >> fix that shortcoming. However without providing notifications >> of hw brightness changes on it. > > See above, I believe such a file should report any > changes in brightness, except those caused by triggers, > so it would report hw brightness changes. > > Anyways if you're not interested in fixing the > shortcomings of the current read behavior on the > brightness file (I'm fine with that, I can live > with the shortcomings) I suggest that we simply go > with v2 of my poll() patch. v2 entails power consumption related issues. Generally I think that we could add the file you proposed, however it would be good to devise a name which will cover also the cases when brightness is changed by firmware without user interaction. >>>> Having no difference in this area between the two approaches >>>> I'm still in favour of the read-only file for notifying >>>> brightness changes procured by hardware. >>> >>> That brings back the needing 2 fds problem; and does >>> not solve userspace not being able to reliably read >>> the led on brightness when blinking or using triggers. >>> >>> And this also has the issue that one is doing poll() on >>> one fd to detect changes on another fd, >> >> It is not necessarily true. We can treat the polling on >> hw_brightness_change file as a means to detect brightness >> changes procured by hardware and we can read that brightness >> by executing read on this same fd. It could return -ENODATA >> if no such an event has occurred so far. > > That would still require 2 fds as userspace also wants to > be able to set the keyboard backlight, but allowing read() > on the hw_brightness_change file at least fixes the weirdness > where userspace gets woken from poll() without being able to > read. So if you insist on going the hw_brightness_change file > route, then I can live with that (and upower will simply > need to open 2 fds, that is doable). > > But, BUT, I would greatly prefer to just go for v4 of my > patch, which fixes the only real problem we've seen with > my patch as original merged without adding a new, somewhat > convoluted sysfs attribute. Hmm, v4 still calls led_notify_brightness_change(led_cdev) from both __led_set_brightness() and __led_set_brightness_blocking(). -- Best regards, Jacek Anaszewski From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754723AbcKNJNg (ORCPT ); Mon, 14 Nov 2016 04:13:36 -0500 Received: from mailout1.w1.samsung.com ([210.118.77.11]:20756 "EHLO mailout1.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753915AbcKNJNC (ORCPT ); Mon, 14 Nov 2016 04:13:02 -0500 X-AuditID: cbfec7f2-f79556d000002c42-63-5829801a80c1 Subject: Re: PM regression with LED changes in next-20161109 To: Hans de Goede , Jacek Anaszewski , Pavel Machek Cc: 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: <127cdd42-6fd8-c671-60b7-3826b351577f@samsung.com> Date: Mon, 14 Nov 2016 10:12:55 +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: <9b476f85-d45e-deb6-335d-fc56f6d90350@redhat.com> Content-type: text/plain; charset=windows-1252; format=flowed Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFlrEKsWRmVeSWpSXmKPExsWy7djP87pSDZoRBud/s1p0LTSweHN8OpPF 7a0bWCw2Pb7GanF51xw2i61v1jFazF7Sz2Jx99RRNov9V7wcOD2+fZ3E4rFz1l12j80rtDw2 L6n3eL/vKpvHitXf2T0+b5ILYI/isklJzcksSy3St0vgypi5q4W5YLNhxfI/M9kbGM+odzFy ckgImEgcmTWDBcIWk7hwbz1bFyMXh5DAUkaJCV1vWSGcz4wSv7tuM8J07Fq5EyqxjFHi1Y2/ 7CAJIYFnjBIfj9iA2MICNhLTp3wEGysiUCrxev8bRpAGZoETjBLTzs8Fa2ATMJT4+eI1E4jN K2An8WbqVLAGFgFVibdn9gNt4OAQFYiQ2H03FaJEUOLH5HtgJZxA5bcfNDOD2MwCjhIPFoEc BGLLS2xe85YZZJeEwD52iWtXN7CDzJEQkJXYdIAZ4gEXie2P50K9LCzx6vgWdghbRuLy5G4W iN7JjBIXj91khXBWM0ps7OyE6rCWaPj/iwViG5/EpG3TmSEW8Ep0tAlBlHhILJ7/ixXCdpSY Pn8OIyS03rFJbLw9lWUCo/wsJA/NQvLELCRPLGBkXsUoklpanJueWmysV5yYW1yal66XnJ+7 iRGYck7/O/5pB+PXE1aHGAU4GJV4eDvyNSKEWBPLiitzDzFKcDArifAK1mlGCPGmJFZWpRbl xxeV5qQWH2KU5mBREufds+BKuJBAemJJanZqakFqEUyWiYNTqoFRx3VpGscrzyvqk5neRYlz JMiIce02/vXjyYQnm+5eEZaybA68e/xeek1l1Le/AvU7Qp+5yl9cbbboQtqTU/p/RX48FdtR vX9yTttTzikXa+3e7daz3Jdh2JYxqysmQUtxxd7TuxmPu0w009y55OWh/77pO818ej4kbXmy hovTmvF8Fc/XW3piSizFGYmGWsxFxYkAYvS13TUDAAA= X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFjrMIsWRmVeSWpSXmKPExsVy+t/xa7r/6jUjDP6d4rPoWmhg8eb4dCaL 21s3sFhsenyN1eLyrjlsFlvfrGO0mL2kn8Xi7qmjbBb7r3g5cHp8+zqJxWPnrLvsHptXaHls XlLv8X7fVTaPFau/s3t83iQXwB7lZpORmpiSWqSQmpecn5KZl26rFBripmuhpJCXmJtqqxSh 6xsSpKRQlphTCuQZGaABB+cA92AlfbsEt4yZu1qYCzYbViz/M5O9gfGMehcjJ4eEgInErpU7 WSFsMYkL99azdTFycQgJLGGUOPdnCzOE84xR4smpdUwgVcICNhLTp3xkAbFFBEolXrUeY4Qo esMm8eP+QrB2ZoETjBIdtw4xglSxCRhK/HzxGqybV8BO4s3UqWDdLAKqEm/P7AfbLSoQIXFr 1UdGiBpBiR+T74HVcALV337QzAxiMwvYSix4v44FwpaX2LzmLfMERoFZSFpmISmbhaRsASPz KkaR1NLi3PTcYiO94sTc4tK8dL3k/NxNjMAI3Hbs55YdjF3vgg8xCnAwKvHwduRrRAixJpYV V+YeYpTgYFYS4RWs04wQ4k1JrKxKLcqPLyrNSS0+xGgK9MREZinR5HxgcsgriTc0MTS3NDQy trAwNzJSEued+uFKuJBAemJJanZqakFqEUwfEwenVAPjvIOHLZverVra7MIpeWamK8fDiZN8 7qS9WXFEiWtDIPfTsxt4Y/OjLcQCm8PklS6HXWdJNT7NvLwqg8nauljodvOUO8k9k85WyobG rY4N/bK+zmVNeM/quANJF+7onFwwrc0zwDJsqe3B8gjFdTly/0+7vasxnZpket98Q6CGtsvh qmA2cSElluKMREMt5qLiRADTr7jj1gIAAA== X-MTR: 20000000000000000@CPGS X-CMS-MailID: 20161114091257eucas1p2df82242c58fb12dc6a8335caa082f729 X-Msg-Generator: CA X-Sender-IP: 182.198.249.180 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: 20161110084919epcas3p2e8fbdd2905cabe15c5301b7a8e22416c X-RootMTR: 20161110084919epcas3p2e8fbdd2905cabe15c5301b7a8e22416c References: <20161109192301.GS26979@atomide.com> <28234714-3994-6747-9cf8-1ff0b3257f7a@gmail.com>

<4c31faef-144d-289c-0e32-83e76aff6178@gmail.com> <3eb60c78-d891-27e5-6b7b-a54a5b547a1c@redhat.com>

<9b476f85-d45e-deb6-335d-fc56f6d90350@redhat.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, On 11/13/2016 02:52 PM, Hans de Goede wrote: > Hi, > > On 13-11-16 12:44, Jacek Anaszewski wrote: >> Hi, >> >> On 11/12/2016 10:14 PM, Hans de Goede wrote: > > > >>>>> So I would like to propose creating a new read-write >>>>> user_brightness file. >>>>> >>>>> The write behavior would be 100% identical to the brightness >>>>> file (in code terms it will call the same store function). >>>>> >>>>> The the read behavior otoh will be different: it will shows >>>>> the last brightness as set by the user, this would show the >>>>> read behavior we really want of brightness: show the real >>>>> brightness when not blinking / triggers are active and show >>>>> the brightness used when on when blinking / triggers are active. >>>>> >>>>> We could then add poll support on this new user_brightness >>>>> file, thus avoiding the problem with the extra cpu-load on >>>>> notifications on blinking / triggers. >>>> >>>> I agree that user_brightness allows to solve the issues you raised >>>> about inconsistent write and read brightness' semantics >>>> (which is not that painful IMHO). >>>> >>>> Reporting non-user brightness changes on user_brightness file >>>> doesn't sound reasonable though. >>> >>> The changes I'm interested in are user brightness changes they >>> are just not done through sysfs, but through a hardwired hotkey, >>> they are however very much done by the user. >> >> Ah, so this file name would be misleading especially taking into account >> the context in which "user" is used in kernel, which predominantly >> means "userspace", e.g. copy_to_user(), copy_from_user(). >> >>>> Also, how would we read the >>>> brightness set by the firmware? We'd have to read brightness >>>> file, so still two files would have to be opened which is >>>> a second drawback of this approach. >>> >>> No, look carefully at the definition of the read behavior >>> I plan to put in the ABI doc: >> >> OK, "user" was what confused me. So in this case changes made >> by the firmware even if in a result of user activity >> (pressing hardware key) obviously cannot be treated similarly >> to the changes made from the userspace context. > > In the end both result on the brightness of the device > changing, so any userspace process interested in monitoring > the brightness will want to know about both type of changes. > >> Unless you're able to give references to the kernel code which >> contradict my judgement. > > AFAIK the audio code will signal volume changes done by > hardwired buttons the same way as audio changes done > by userspace calling into the kernel. This also makes > sense because in the end, what is interesting for a > mixer app, is that the volume changed, and what the > new volume is. OK, so it is indeed similar to your LED use case. Nonetheless in case of LED controllers it is also possible that hardware adjusts LED brightness in case of low battery voltage. If a device is able e.g. to generate an interrupt to notify this kind of event, then we would like also to be able to notify the client about that. It wouldn't be user generated brightness change though. >>> "Reading this file will return the actual led brightness >>> when not blinking and no triggers are active; reading this >>> file will return the brightness used when the led is on >>> when blinking or triggers are active." >> >> This is unnecessarily entangled. Blinking means timer trigger >> is active. > > Ok. > >>> So for e.g. the backlit keyboard case reading this single >>> file will return the actual brightness of the backlight, >>> since this does not involve blinking or triggers. >>> >>> Basically the idea is that the user_brightness file >>> will have the semantics which IMHO the brightness file >>> itself should have had from the beginning, but which >>> we can't change now due to ABI reasons. >> >> And in fact introducing user_brightness file would indeed >> fix that shortcoming. However without providing notifications >> of hw brightness changes on it. > > See above, I believe such a file should report any > changes in brightness, except those caused by triggers, > so it would report hw brightness changes. > > Anyways if you're not interested in fixing the > shortcomings of the current read behavior on the > brightness file (I'm fine with that, I can live > with the shortcomings) I suggest that we simply go > with v2 of my poll() patch. v2 entails power consumption related issues. Generally I think that we could add the file you proposed, however it would be good to devise a name which will cover also the cases when brightness is changed by firmware without user interaction. >>>> Having no difference in this area between the two approaches >>>> I'm still in favour of the read-only file for notifying >>>> brightness changes procured by hardware. >>> >>> That brings back the needing 2 fds problem; and does >>> not solve userspace not being able to reliably read >>> the led on brightness when blinking or using triggers. >>> >>> And this also has the issue that one is doing poll() on >>> one fd to detect changes on another fd, >> >> It is not necessarily true. We can treat the polling on >> hw_brightness_change file as a means to detect brightness >> changes procured by hardware and we can read that brightness >> by executing read on this same fd. It could return -ENODATA >> if no such an event has occurred so far. > > That would still require 2 fds as userspace also wants to > be able to set the keyboard backlight, but allowing read() > on the hw_brightness_change file at least fixes the weirdness > where userspace gets woken from poll() without being able to > read. So if you insist on going the hw_brightness_change file > route, then I can live with that (and upower will simply > need to open 2 fds, that is doable). > > But, BUT, I would greatly prefer to just go for v4 of my > patch, which fixes the only real problem we've seen with > my patch as original merged without adding a new, somewhat > convoluted sysfs attribute. Hmm, v4 still calls led_notify_brightness_change(led_cdev) from both __led_set_brightness() and __led_set_brightness_blocking(). -- Best regards, Jacek Anaszewski From mboxrd@z Thu Jan 1 00:00:00 1970 From: j.anaszewski@samsung.com (Jacek Anaszewski) Date: Mon, 14 Nov 2016 10:12:55 +0100 Subject: PM regression with LED changes in next-20161109 In-Reply-To: <9b476f85-d45e-deb6-335d-fc56f6d90350@redhat.com> References: <20161109192301.GS26979@atomide.com> <28234714-3994-6747-9cf8-1ff0b3257f7a@gmail.com>

<4c31faef-144d-289c-0e32-83e76aff6178@gmail.com> <3eb60c78-d891-27e5-6b7b-a54a5b547a1c@redhat.com>

<9b476f85-d45e-deb6-335d-fc56f6d90350@redhat.com> Message-ID: <127cdd42-6fd8-c671-60b7-3826b351577f@samsung.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi, On 11/13/2016 02:52 PM, Hans de Goede wrote: > Hi, > > On 13-11-16 12:44, Jacek Anaszewski wrote: >> Hi, >> >> On 11/12/2016 10:14 PM, Hans de Goede wrote: > > > >>>>> So I would like to propose creating a new read-write >>>>> user_brightness file. >>>>> >>>>> The write behavior would be 100% identical to the brightness >>>>> file (in code terms it will call the same store function). >>>>> >>>>> The the read behavior otoh will be different: it will shows >>>>> the last brightness as set by the user, this would show the >>>>> read behavior we really want of brightness: show the real >>>>> brightness when not blinking / triggers are active and show >>>>> the brightness used when on when blinking / triggers are active. >>>>> >>>>> We could then add poll support on this new user_brightness >>>>> file, thus avoiding the problem with the extra cpu-load on >>>>> notifications on blinking / triggers. >>>> >>>> I agree that user_brightness allows to solve the issues you raised >>>> about inconsistent write and read brightness' semantics >>>> (which is not that painful IMHO). >>>> >>>> Reporting non-user brightness changes on user_brightness file >>>> doesn't sound reasonable though. >>> >>> The changes I'm interested in are user brightness changes they >>> are just not done through sysfs, but through a hardwired hotkey, >>> they are however very much done by the user. >> >> Ah, so this file name would be misleading especially taking into account >> the context in which "user" is used in kernel, which predominantly >> means "userspace", e.g. copy_to_user(), copy_from_user(). >> >>>> Also, how would we read the >>>> brightness set by the firmware? We'd have to read brightness >>>> file, so still two files would have to be opened which is >>>> a second drawback of this approach. >>> >>> No, look carefully at the definition of the read behavior >>> I plan to put in the ABI doc: >> >> OK, "user" was what confused me. So in this case changes made >> by the firmware even if in a result of user activity >> (pressing hardware key) obviously cannot be treated similarly >> to the changes made from the userspace context. > > In the end both result on the brightness of the device > changing, so any userspace process interested in monitoring > the brightness will want to know about both type of changes. > >> Unless you're able to give references to the kernel code which >> contradict my judgement. > > AFAIK the audio code will signal volume changes done by > hardwired buttons the same way as audio changes done > by userspace calling into the kernel. This also makes > sense because in the end, what is interesting for a > mixer app, is that the volume changed, and what the > new volume is. OK, so it is indeed similar to your LED use case. Nonetheless in case of LED controllers it is also possible that hardware adjusts LED brightness in case of low battery voltage. If a device is able e.g. to generate an interrupt to notify this kind of event, then we would like also to be able to notify the client about that. It wouldn't be user generated brightness change though. >>> "Reading this file will return the actual led brightness >>> when not blinking and no triggers are active; reading this >>> file will return the brightness used when the led is on >>> when blinking or triggers are active." >> >> This is unnecessarily entangled. Blinking means timer trigger >> is active. > > Ok. > >>> So for e.g. the backlit keyboard case reading this single >>> file will return the actual brightness of the backlight, >>> since this does not involve blinking or triggers. >>> >>> Basically the idea is that the user_brightness file >>> will have the semantics which IMHO the brightness file >>> itself should have had from the beginning, but which >>> we can't change now due to ABI reasons. >> >> And in fact introducing user_brightness file would indeed >> fix that shortcoming. However without providing notifications >> of hw brightness changes on it. > > See above, I believe such a file should report any > changes in brightness, except those caused by triggers, > so it would report hw brightness changes. > > Anyways if you're not interested in fixing the > shortcomings of the current read behavior on the > brightness file (I'm fine with that, I can live > with the shortcomings) I suggest that we simply go > with v2 of my poll() patch. v2 entails power consumption related issues. Generally I think that we could add the file you proposed, however it would be good to devise a name which will cover also the cases when brightness is changed by firmware without user interaction. >>>> Having no difference in this area between the two approaches >>>> I'm still in favour of the read-only file for notifying >>>> brightness changes procured by hardware. >>> >>> That brings back the needing 2 fds problem; and does >>> not solve userspace not being able to reliably read >>> the led on brightness when blinking or using triggers. >>> >>> And this also has the issue that one is doing poll() on >>> one fd to detect changes on another fd, >> >> It is not necessarily true. We can treat the polling on >> hw_brightness_change file as a means to detect brightness >> changes procured by hardware and we can read that brightness >> by executing read on this same fd. It could return -ENODATA >> if no such an event has occurred so far. > > That would still require 2 fds as userspace also wants to > be able to set the keyboard backlight, but allowing read() > on the hw_brightness_change file at least fixes the weirdness > where userspace gets woken from poll() without being able to > read. So if you insist on going the hw_brightness_change file > route, then I can live with that (and upower will simply > need to open 2 fds, that is doable). > > But, BUT, I would greatly prefer to just go for v4 of my > patch, which fixes the only real problem we've seen with > my patch as original merged without adding a new, somewhat > convoluted sysfs attribute. Hmm, v4 still calls led_notify_brightness_change(led_cdev) from both __led_set_brightness() and __led_set_brightness_blocking(). -- Best regards, Jacek Anaszewski