From: Shuah Khan <shuahkhan@gmail.com>
To: Fabio Baltieri <fabio.baltieri@gmail.com>
Cc: shuahkhan@gmail.com, Bryan Wu <bryan.wu@canonical.com>,
linux-leds@vger.kernel.org, linux-kernel@vger.kernel.org,
Richard Purdie <rpurdie@rpsys.net>
Subject: Re: [PATCH v2] leds: fix led_brightness_set when soft-blinking
Date: Wed, 06 Jun 2012 14:30:12 -0600 [thread overview]
Message-ID: <1339014612.2763.98.camel@lorien2> (raw)
In-Reply-To: <20120606191149.GA1376@gmail.com>
> I realize now that my last patch broke ledtrig-timer updates as that
> works by passing delay_{on,off} values by pointer, and moving
> led_stop_software_blink() earlier destroy the other value.
Fabio,
This type of interaction is what I am concerned about, and came up
during the transient trigger discussion as well. In the case of
transient trigger it was an easy decision to use separate namespace, but
that doesn't make sense in this case.
>
> That's a bit of a pitfall, as the old code was working because values
> were copied when entering led_set_software_blink.
>
> I see three options here:
> - reverting back my leds: "fix led_brightness_set when soft-blinking"
> (9b05cd0) to the first version I posted, wich should work as before.
> - modify ledtrig-timer to use two internal variables to store delay_on
> and delay_off instead of the led_cdev ones.
Don't this this is a viable option without restructuring the leds code.
These two variables are common to led_cdev and used by other drivers as
well. ledtrig-timer will still have to store it as these are used in the
led_timer_function() in led-class.c
> - moving the two led_cdev->blink_delay_xx = 0 only into
> led_brightness_set, as that's the only place when they are needed.
Sounds reasonable. I would like to pull your patches in and do some
testing, but very likely I won't be able to get to it until this
weekend. This is where use-cases will help as well for us go through
various transitions and see if it all comes together.
-- Shuah
next prev parent reply other threads:[~2012-06-06 20:30 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-06-05 21:38 [PATCH v2 1/2] led: add oneshot trigger Fabio Baltieri
2012-06-05 21:38 ` [PATCH 2/2] leds: fix led_brightness_set when soft-blinking Fabio Baltieri
2012-06-06 3:58 ` Bryan Wu
2012-06-06 7:00 ` Fabio Baltieri
2012-06-06 7:19 ` [PATCH v2] " Fabio Baltieri
2012-06-06 8:19 ` Bryan Wu
2012-06-06 11:10 ` Fabio Baltieri
2012-06-06 14:10 ` Bryan Wu
2012-06-06 19:11 ` Fabio Baltieri
2012-06-06 19:12 ` [PATCH v3] " Fabio Baltieri
2012-06-06 20:30 ` Shuah Khan [this message]
2012-06-07 12:58 ` [PATCH v2] " Bryan Wu
2012-06-11 3:06 ` Shuah Khan
2012-06-11 14:47 ` Bryan Wu
2012-06-06 0:51 ` [PATCH v2 1/2] led: add oneshot trigger Shuah Khan
2012-06-06 2:56 ` Bryan Wu
2012-06-06 6:04 ` Fabio Baltieri
2012-06-06 22:11 ` [PATCH v3] " Fabio Baltieri
2012-06-07 12:58 ` Bryan Wu
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=1339014612.2763.98.camel@lorien2 \
--to=shuahkhan@gmail.com \
--cc=bryan.wu@canonical.com \
--cc=fabio.baltieri@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-leds@vger.kernel.org \
--cc=rpurdie@rpsys.net \
/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