All of lore.kernel.org
 help / color / mirror / Atom feed
From: Geert Uytterhoeven <geert@linux-m68k.org>
To: "Rafael J. Wysocki" <rjw@rjwysocki.net>,
	Ulf Hansson <ulf.hansson@linaro.org>
Cc: Linux PM list <linux-pm@vger.kernel.org>,
	Kevin Hilman <khilman@kernel.org>,
	Viresh Kumar <viresh.kumar@linaro.org>,
	Geert Uytterhoeven <geert+renesas@glider.be>,
	Simon Horman <horms@verge.net.au>,
	Niklas Soderlund <niklas.soderlund+renesas@ragnatech.se>,
	Linux-Renesas <linux-renesas-soc@vger.kernel.org>
Subject: Re: [PATCH v2 2/3] PM / core: Add IN_BAND_WAKEUP driver flag
Date: Sun, 10 Dec 2017 11:16:40 +0100	[thread overview]
Message-ID: <CAMuHMdUE+fdbS+HDxbcWNHaTZubnrQ7ai4bkOsBpLriG1y-gZg@mail.gmail.com> (raw)
In-Reply-To: <1926914.PsNMcYBlGW@aspire.rjw.lan>

Hi Rafael, Ulf,

On Sun, Dec 10, 2017 at 3:30 AM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> On Monday, November 13, 2017 4:46:42 PM CET Ulf Hansson wrote:
>> For some bus types and PM domains, it's not sufficient to only check the
>> return value from device_may_wakeup(), to fully understand how to configure
>> wakeup settings for the device during system suspend.
>>
>> In particular, sometimes the device may need to remain in its power state,
>> in case the driver has configured it for in band IRQs, as to be able to
>> generate wakeup signals. Therefore, define and document an IN_BAND_WAKEUP
>> driver flag, to enable drivers to instruct bus types and PM domains about
>> this setting.
>>
>> Of course, in case the bus type and PM domain has additional information
>> about wakeup settings of the device, they may override the behaviour.
>>
>> Using the IN_BAND_WAKEUP driver flag for a device, may also affect how bus
>> types and PM domains should treat the device's parent during system
>> suspend. Therefore, in __device_suspend(), let the PM core propagate the
>> wakeup setting by using a status flag in the struct dev_pm_info for the
>> parent. This also makes it consistent with how the existing "wakeup_path"
>> status flag is being assigned.
>
> I've been thinking about this quite a bit recently and my conclusion is that
> the flag makes perfect sence (as it covers a valid use case), but I would
> define it and design the handling of it a bit differently.

I'm also still thinking about this, cfr. my recent silence w.r.t.
these matters...

On Renesas ARM SoCs (and at least some SH SoCs, but these are stuck in the
pre-DT legacy clock domain), a device needs to be kept running if it is a
wake-up source (e.g. WoL), or if it's part of the wake-up patch (e.g.
irqchip). So in the absence of the GENPD_FLAG_ACTIVE_WAKEUP flag in the PM
Domain driver, all individual drivers would need to set the IN_BAND_WAKEUP
flag (but see the exception below)
Hence for this class of SoCs, this would duplicate the existing
dev->power.wakeup and dev->power.wakeup_path flags, so that's why I would
prefer using GENPD_FLAG_ACTIVE_WAKEUP instead (like we already do for the
older SH-Mobile SoCs, but not for newer R-Car and RZ series).
For other SoC families, the situation may be different.

For us, the only exception are devices where the wakeup-source is not the
device itself, but a GPIO, e.g. SDHI card detect. In such a case, only the
device driver knows if the device is to be woken up through a dedicated CD
line, or through a GPIO CD. So that would call for a method to opt-out,
e.g. OUT_BAND_WAKEUP.

To complicate matters, some drivers may be used on SoCs where the device
needs to be kept running (clock and/or power domain), and on SoCs where the
device is always running. This difference is typically handled by genpd,
and the device driver may not even be aware. Of course the driver can just
set IN_BAND_WAKEUP regardless, (else it has to check for the presence of
clocks and/or power-domains properties itself, duplicating genpd
core/driver code).

So what about

         if (IN_BAND_WAKEUP ||
            (GENPD_FLAG_ACTIVE_WAKEUP && !OUT_BAND_WAKEUP)) {
                ... suspend device...
        }

?

Thanks!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

  parent reply	other threads:[~2017-12-10 10:16 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-13 15:46 [PATCH v2 0/3] PM / core: Invent a WAKEUP_POWERED driver flag Ulf Hansson
2017-11-13 15:46 ` [PATCH v2 1/3] PM / core: Re-factor some code dealing with parents in __device_suspend() Ulf Hansson
2017-12-06  1:01   ` Rafael J. Wysocki
2017-11-13 15:46 ` [PATCH v2 2/3] PM / core: Add IN_BAND_WAKEUP driver flag Ulf Hansson
2017-12-01 11:17   ` Vincent Guittot
2017-12-10  2:30   ` Rafael J. Wysocki
2017-12-10  9:18     ` Ulf Hansson
2017-12-10 10:16     ` Geert Uytterhoeven [this message]
2017-12-11 10:24       ` Ulf Hansson
2017-12-11 10:48         ` Geert Uytterhoeven
2017-12-11 20:59           ` Ulf Hansson
2017-12-12  8:16             ` Geert Uytterhoeven
2017-12-12 14:20               ` Ulf Hansson
2017-12-14 10:52             ` Geert Uytterhoeven
2017-12-14 14:13               ` Ulf Hansson
2017-12-14 14:27                 ` Geert Uytterhoeven
2017-11-13 15:46 ` [PATCH v2 3/3] PM / Domains: Take wakeup_path_in_band status flag into account Ulf Hansson
2017-11-13 15:50 ` [PATCH v2 0/3] PM / core: Invent a WAKEUP_POWERED driver flag Ulf Hansson

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=CAMuHMdUE+fdbS+HDxbcWNHaTZubnrQ7ai4bkOsBpLriG1y-gZg@mail.gmail.com \
    --to=geert@linux-m68k.org \
    --cc=geert+renesas@glider.be \
    --cc=horms@verge.net.au \
    --cc=khilman@kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=linux-renesas-soc@vger.kernel.org \
    --cc=niklas.soderlund+renesas@ragnatech.se \
    --cc=rjw@rjwysocki.net \
    --cc=ulf.hansson@linaro.org \
    --cc=viresh.kumar@linaro.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.