linux-leds.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Pavel Machek <pavel@ucw.cz>
To: David Lin <dtwlin@google.com>
Cc: Jacek Anaszewski <jacek.anaszewski@gmail.com>,
	rpurdie@rpsys.net, robh@kernel.org,
	Rom Lemarchand <romlem@google.com>,
	Joel Fernandes <joelaf@google.com>,
	stable@vger.kernel.org, linux-leds@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] led: ledtrig-transient: replace timer_list with hrtimer
Date: Wed, 26 Apr 2017 00:34:55 +0200	[thread overview]
Message-ID: <20170425223455.GA24467@amd> (raw)
In-Reply-To: <CAPzFB+U3aLkM7Q2cubdMmtnJp-oCqGKGp2_Q-n2nkbyeh07JgA@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 3362 bytes --]

On Mon 2017-04-24 20:05:59, David Lin wrote:
> Hi Jacek,
> 
> On Mon, Apr 24, 2017 at 12:59 PM, Jacek Anaszewski
> <jacek.anaszewski@gmail.com> 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. 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

I believe we should fix the documentation. It is LED subsystem,
requirements are different, and we _already_ have haptic feedback
subsystem.
								Pavel

IOW, I suggest this: (hmm, and more led->LED is needed, and more
english fixes. Oh well.)

Signed-off-by: Pavel Machek <pavel@ucw.cz>

diff --git a/Documentation/leds/ledtrig-transient.txt b/Documentation/leds/ledtrig-transient.txt
index 3bd38b4..c5cf475 100644
--- a/Documentation/leds/ledtrig-transient.txt
+++ b/Documentation/leds/ledtrig-transient.txt
@@ -16,17 +16,11 @@ set a timer to hold a state, however when user space application crashes or
 goes away without deactivating the timer, the hardware will be left in that
 state permanently.
 
-As a specific example of this use-case, let's look at vibrate feature on
-phones. Vibrate function on phones is implemented using PWM pins on SoC or
-PMIC. There is a need to activate one shot timer to control the vibrate
-feature, to prevent user space crashes leaving the phone in vibrate mode
-permanently causing the battery to drain.
-
 Transient trigger addresses the need for one shot timer activation. The
 transient trigger can be enabled and disabled just like the other leds
 triggers.
 
-When an led class device driver registers itself, it can specify all leds
+When an LED class device driver registers itself, it can specify all leds
 triggers it supports and a default trigger. During registration, activation
 routine for the default trigger gets called. During registration of an led
 class device, the LED state does not change.
@@ -144,7 +138,6 @@ repeat the following step as needed:
 	echo none > trigger
 
 This trigger is intended to be used for for the following example use cases:
- - Control of vibrate (phones, tablets etc.) hardware by user space app.
  - Use of LED by user space app as activity indicator.
  - Use of LED by user space app as a kind of watchdog indicator -- as
        long as the app is alive, it can keep the LED illuminated, if it dies


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

  parent reply	other threads:[~2017-04-25 22:34 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-24  4:42 [PATCH] led: ledtrig-transient: replace timer_list with hrtimer David Lin
2017-04-24  7:44 ` Pavel Machek
2017-04-24 19:59 ` Jacek Anaszewski
2017-04-24 20:18   ` Pavel Machek
2017-04-25  3:05   ` David Lin
2017-04-25 20:15     ` Jacek Anaszewski
2017-04-27  3:48       ` David Lin
2017-04-27 18:37         ` Jacek Anaszewski
2017-04-25 22:34     ` Pavel Machek [this message]
2017-04-26 19:49       ` Jacek Anaszewski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170425223455.GA24467@amd \
    --to=pavel@ucw.cz \
    --cc=dtwlin@google.com \
    --cc=jacek.anaszewski@gmail.com \
    --cc=joelaf@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-leds@vger.kernel.org \
    --cc=robh@kernel.org \
    --cc=romlem@google.com \
    --cc=rpurdie@rpsys.net \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).