From mboxrd@z Thu Jan 1 00:00:00 1970 From: Hans de Goede Subject: Re: [PATCH v8 7/7] platform/x86/dell-*: Call led_classdev_notify_brightness_hw_changed on kbd brightness change Date: Mon, 6 Mar 2017 14:39:47 +0100 Message-ID: <595f88da-a0ee-005e-b22d-ee04cfe20628@redhat.com> References: <20170221170838.GO9795@pali> <7bb7c48f-789e-8b4c-b76b-1389d7fbbfb7@redhat.com> <20170222084935.GA21558@pali> <20170222120109.GC21558@pali> <035b56e6-b606-c632-35cf-00f880abcbf5@redhat.com> <20170301111558.GC3185@pali> <8c181bbc-e786-16c7-39f7-80d4be502066@redhat.com> <20170301125532.GF3185@pali> <20170303120020.GD25757@pali> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Return-path: In-Reply-To: <20170303120020.GD25757@pali> Sender: platform-driver-x86-owner@vger.kernel.org To: =?UTF-8?Q?Pali_Roh=c3=a1r?= Cc: Darren Hart , Andy Shevchenko , Henrique de Moraes Holschuh , Jacek Anaszewski , Pavel Machek , platform-driver-x86@vger.kernel.org, linux-leds@vger.kernel.org List-Id: linux-leds@vger.kernel.org Hi, On 03-03-17 13:00, Pali Rohár wrote: > On Wednesday 01 March 2017 14:58:45 Hans de Goede wrote: >>> In my idea I do not add any additional information to event. Which means >>> all received events are same in current driver state and I can swap >>> them. So I can decide to drop one event when driver is changing >>> backlight level. And because I can swap received events it does not >>> matter if I drop "correct" one (that which was really generated by my >>> backlight level change) or any other before or after (as they are same). >>> The only important is that kernel use correct number of firmware events. >>> >>> Idea for time interval of 2-3 seconds is that if event is delivered >>> later as after 3 seconds it is probably useless as driver application >>> code can be already out-of-sync. But this time interval is just another >>> idea and we do not need to use it. >> >> I really don't like the 2-3 seconds stuff, but I'm fine with going >> with your idea of just swallowing one event after a sysfs-write >> without the timeout stuff. > > Ok. I'm fine with just dropping exactly one event after setting new > value via sysfs. It is OK for you? Yes that will work for me. I will prepare a new version. Note I probably will not have time for this till the end of this week. One thing I was wondering, is what will happen if we write the same value to the sysfs brightness file twice, do we get events for both writes? One way to avoid this problem would be to compare the written value to the last known value and discard the write (avoiding an expensive SMM call) if they are equal. Discarding double writes does introduce a race between hotkey changes vs sysfs writes, if a hotkey change has been made but the event has not yet been handled then a sysfs write may be seen as being double and get discarded even though it is not double. OTOH if the hotkey press wins the race then the result would have been the write being discarded anyways, so I don't think this is a real problem. You're input on this would be appreciated. If writing the same value twice does not generate events then we must filter out the double writes. If it does drop events we can choose, the simplest would be to not filter in that case, avoiding any races (and userspace normally should not write the same value twice, but we cannot assume that). Regards, Hans