linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: linus.walleij@linaro.org (Linus Walleij)
To: linux-arm-kernel@lists.infradead.org
Subject: [[PATCH v2]] drivers: leds/trigger: system cannot enter suspend
Date: Wed, 7 Jun 2017 11:46:10 +0200	[thread overview]
Message-ID: <CACRpkdacymETLuRmpn07HUKHvrjWc85Jdj0s0qk44zTcaALa9w@mail.gmail.com> (raw)
In-Reply-To: <20170607071256.GA3230@amd>

On Wed, Jun 7, 2017 at 9:12 AM, Pavel Machek <pavel@ucw.cz> wrote:

> Core reason is that 5ab92a7cb is wrong. (Very wrong; it papers over
> the issue for one trigger, but we have more than one... Even if
> special handling for heartbeat is warranted, that handling would be
> "turn the led off" not "unregister the trigger", which is
> userland-visible action.)

OK yeah I guess you're right.

I just couldn't think about anything better and didn't get any
review at the time, so mistakes were made.

> That one should be reverted, then maybe the
> driver should be fixed to turn the led off during suspend.

Do you mean that the heartbeat trigger driver should be fixed to
turn off the LED during sleep?

That is essentially what I was trying to achieve.

The reason it is done as it is, is that the trigger sets up
a timer to do its job, and the timer may trigger between
the point you turn off the LED and the system really goes
to suspend, again maybe turning the LED on and
again leaving the system with a glowing LED at suspend.

The patch also solves the following phenomenon, sorry
for not writing in the commit:

- Turn off LED
- Suspend I2C hardware
- Timer trigger
- Trying to blink the LED using I2C
- Crap in the console about the failed I2C transaction
- Actual suspen happens
- System comes online
- Trying to blink the LED using I2C
- Crap in the console about the failed I2C transaction
- Resume I2C hardware

It's just very fragile this trigger, turning off the LED from
the PM notifier is obviously not enough, we also need to
disable the timer, and then take it back online after resume.

That is the tricky part, which is simplified by just removing
and adding back the trigger.

Yours,
Linus Walleij

  parent reply	other threads:[~2017-06-07  9:46 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-07  2:50 [[PATCH v2]] drivers: leds/trigger: system cannot enter suspend Zhang Bo
2017-06-07  7:12 ` Pavel Machek
2017-06-07  9:27   ` Ulf Hansson
2017-06-07  9:46   ` Linus Walleij [this message]
2017-06-07 10:31     ` Pavel Machek
2017-06-08  2:57       ` Bruce Zhang
2017-06-08 20:27         ` Jacek Anaszewski
2017-06-09  2:57           ` Bruce Zhang
2017-06-09 20:47             ` Jacek Anaszewski
2017-06-09 22:21               ` Jacek Anaszewski
2017-06-11  4:43               ` Bruce Zhang

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=CACRpkdacymETLuRmpn07HUKHvrjWc85Jdj0s0qk44zTcaALa9w@mail.gmail.com \
    --to=linus.walleij@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.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).