All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.