Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1948643AbdDYUQP (ORCPT ); Tue, 25 Apr 2017 16:16:15 -0400 Received: from mail-wm0-f68.google.com ([74.125.82.68]:35235 "EHLO mail-wm0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1948304AbdDYUQD (ORCPT ); Tue, 25 Apr 2017 16:16:03 -0400 Subject: Re: [PATCH] led: ledtrig-transient: replace timer_list with hrtimer To: David Lin References: <20170424044254.145192-1-dtwlin@google.com> Cc: rpurdie@rpsys.net, pavel@ucw.cz, robh@kernel.org, Rom Lemarchand , Joel Fernandes , stable@vger.kernel.org, linux-leds@vger.kernel.org, linux-kernel@vger.kernel.org From: Jacek Anaszewski X-Enigmail-Draft-Status: N1110 Message-ID: <475626da-5fb2-c03c-cdf7-feb42ee6b672@gmail.com> Date: Tue, 25 Apr 2017 22:15:18 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Icedove/45.5.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2136 Lines: 63 Hi David, On 04/25/2017 05:05 AM, David Lin wrote: > Hi Jacek, > > On Mon, Apr 24, 2017 at 12:59 PM, Jacek Anaszewski > wrote: >> >> Hi David, >> >> Thanks for the patch. >> >> Unfortunately we cannot switch to using hr timers just like that >> without introducing side effects for many devices. We had similar >> attempt of increasing timer tirgger accuracy two years ago [0]. >> >> In short words, for drivers that can sleep while setting brightness >> and/or are using a bus like I2C you will not be able to enforce >> 1ms delay period. >> >> I recommend you to go through the thread [0] so that we had >> a well defined ground for the discussion on how to address this >> issue properly. >> > > I think I understand the background now, and would agree that not all > the LED driver require hrtimer as human eye can't probably tell > there's a 10ms variation in a blink. The main problem are side effects occurring when an event scheduled by hrtimer can't finish before the next one begins. We get warnings like in the example below (copied from [0]) then, and they have probably negative impact on the whole system performance. echo "timer" > trigger echo 1 > delay_on echo 1 > delay_off echo usec > delay_unit [ 178.584433] hrtimer: interrupt took 300747 ns > However, there's a need to > support hrtimer if the LED subsystem claims support the use case of > vibrator (please see Documentation/leds/ledtrig-transient.txt) as even > a 5ms of variation is perceivable to the user. I'm thinking if a > better interim solution is to introduce a > LEDS_TRIGGER_TRANSIENT_HRTIMER config to work with both timers in > compile time. Would you agree? I think that it would be better if LED class driver set a flag marking itself as capable of setting brightness with high rate. I'd limit that only to leds-gpio and devices driven through memory mapped registers. Having the flag e.g. LED_BRIGHTNESS_FAST, we could add support for hr timers also to ledtrig-timer. You can try also the other option mentioned by Pavel in [1]. [1] https://lkml.org/lkml/2017/4/24/881 -- Best regards, Jacek Anaszewski