public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Fabio Baltieri <fabio.baltieri@gmail.com>
To: Bryan Wu <bryan.wu@canonical.com>
Cc: 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, 6 Jun 2012 21:11:49 +0200	[thread overview]
Message-ID: <20120606191149.GA1376@gmail.com> (raw)
In-Reply-To: <CAK5ve-LrnZQqAi2tamFfdN1_upjGQ=TYgpU-xLXnL+8xRv=+Xw@mail.gmail.com>

Hi Bryan,

On Wed, Jun 06, 2012 at 10:10:23PM +0800, Bryan Wu wrote:
> Yeah, that's right. Rafal's patch was in my fixes-for-3.5 branch, I
> will send out by this week for Linus. So I have to rebase all the
> for-next patches on top of it. Although I got conflicts, I will fix
> that. I prepared a new branch merged for-next and fixes-for-3.5
> together name devel, please help to test.
> 
> http://git.kernel.org/?p=linux/kernel/git/cooloney/linux-leds.git;a=shortlog;h=refs/heads/devel

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.

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.
- moving the two led_cdev->blink_delay_xx = 0 only into
  led_brightness_set, as that's the only place when they are needed.

What you think about it?

I think the third option is the best, as it removes the pitfall and
simplify the code a bit.  I'll post that as a v3 "fix led_brightness_set
when soft-blinking", would you consider replacing the last patch you
applied in the new branch with the v3?

Fabio

  reply	other threads:[~2012-06-06 19:10 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 [this message]
2012-06-06 19:12                 ` [PATCH v3] " Fabio Baltieri
2012-06-06 20:30                 ` [PATCH v2] " Shuah Khan
2012-06-07 12:58                   ` 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=20120606191149.GA1376@gmail.com \
    --to=fabio.baltieri@gmail.com \
    --cc=bryan.wu@canonical.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