[RFD] Devices that need full power for wakeup

[RFD] Devices that need full power for wakeup

[Rafael J. Wysocki]

Hi,

First, let me characterize the problem the way I understand it and if
my understanding of it is not correct, let me know why.

There are devices that can only signal wakeup if they are provided
with full "in-band" power (in contrast with devices that can signal
wakeup with the help of a special "wakeup" power source, possibly of
smaller capacity).  Suspending them via runtime PM is problematic
because of the general assumption that wakeup should always be enabled
for all runtime-suspended wakeup-capable devices which leads to sort
of a Catch 22 situation in this particular case.

Some drivers work around that by only allowing the device to be
suspended if they know that it is not expected by user space to signal
wakeup and they use the runtime PM usage counter to prevent the device
from being suspended otherwise.  That works for runtime PM, but if the
system goes to a sleep state, the runtime PM usage counter is not
taken into account and the handling of the device may not be adequate
and there is a question how to address that.

Assuming that the above description of the problem is accurate, here's
how I see it.

All depends on whether or not a device requiring full power for
signaling wakeup is allowed to wake up the system from sleeps states.
If it is not allowed to do that, it should be fully suspended during
system transitions to sleep states regardless of its previous runtime
PM status and (in the majority of cases) it may just be left in
suspend if already suspended.

The problematic case is when the device is allowed to wake up the
system from sleep states, because it has to be provided with full
power while the system is in a sleep state for the wakeup to work.
Clearly, checking the runtime PM usage counter alone is not sufficient
for that, because the wakeup counter may be nonzero for reasons other
than wakeup and the driver may not be able to arrange things properly
by itself during system-wide transitions, for example in the cases
when the device belongs to a power domain managed by a separate code
layer (like genpd).  This leads to the conclusion that it is necessary
to provide an additional mechanism allowing all of the layers of code
involved in the handling of the device to check what to do with it (or
with the power domain it belongs to etc) during system-wide
transitions to sleep state.

In principle, that mechanism might be a new driver flag saying that
power should not be removed from the device if device_may_wakeup()
returns "true" for it.  However, there already is a similar thing in
the kernel which already is taken into account by genpd_power_off()
and which is the PM_QOS_FLAG_NO_POWER_OFF PM QoS flag.

The existence of it means that drivers can use it for runtime PM in
the first place instead of preventing devices from being suspended if
they need to signal wakeup.  In principle, a driver may just set this
flag and then suspend the device normally whenever that makes sense
(of course, taking the flag into account if set) which may lead to
reduced power draw if (say) device cloks may be gated without
disturbing its wakeup capability.  Moreover, PM QoS requests may be
added and removed at any time, so the driver may just set the flag or
clear it whenever the device's wakeup settings change.

And if this flag is taken into account by genpd_sync_power_off() too,
using it may just be sufficient to address the concern at hand.

Thoughts?

Thanks,
Rafael

Re: [RFD] Devices that need full power for wakeup

[Geert Uytterhoeven]

Hi Rafael,

On Mon, Jan 8, 2018 at 12:51 PM, Rafael J. Wysocki <rafael@kernel.org> wrote:
> First, let me characterize the problem the way I understand it and if
> my understanding of it is not correct, let me know why.
>
> There are devices that can only signal wakeup if they are provided
> with full "in-band" power (in contrast with devices that can signal
> wakeup with the help of a special "wakeup" power source, possibly of
> smaller capacity).  Suspending them via runtime PM is problematic
> because of the general assumption that wakeup should always be enabled
> for all runtime-suspended wakeup-capable devices which leads to sort
> of a Catch 22 situation in this particular case.
>
> Some drivers work around that by only allowing the device to be
> suspended if they know that it is not expected by user space to signal
> wakeup and they use the runtime PM usage counter to prevent the device
> from being suspended otherwise.  That works for runtime PM, but if the
> system goes to a sleep state, the runtime PM usage counter is not
> taken into account and the handling of the device may not be adequate
> and there is a question how to address that.

Why would they play games with the runtime PM usage counter?
And why does "that work with runtime PM"?
Wake-up signals don't matter when a device is runtime resumed, as the
system is not in a system-wide sleep state in the first place?

Or do you consider normal interrupt-passing (e.g. pressing a key with
gpio-keys when the system is not asleep) too?

> Assuming that the above description of the problem is accurate, here's
> how I see it.
>
> All depends on whether or not a device requiring full power for
> signaling wakeup is allowed to wake up the system from sleeps states.
> If it is not allowed to do that, it should be fully suspended during
> system transitions to sleep states regardless of its previous runtime
> PM status and (in the majority of cases) it may just be left in
> suspend if already suspended.

OK.

> The problematic case is when the device is allowed to wake up the
> system from sleep states, because it has to be provided with full
> power while the system is in a sleep state for the wakeup to work.
> Clearly, checking the runtime PM usage counter alone is not sufficient
> for that, because the wakeup counter may be nonzero for reasons other
> than wakeup and the driver may not be able to arrange things properly
> by itself during system-wide transitions, for example in the cases
> when the device belongs to a power domain managed by a separate code
> layer (like genpd).  This leads to the conclusion that it is necessary
> to provide an additional mechanism allowing all of the layers of code
> involved in the handling of the device to check what to do with it (or
> with the power domain it belongs to etc) during system-wide
> transitions to sleep state.

OK.

> In principle, that mechanism might be a new driver flag saying that
> power should not be removed from the device if device_may_wakeup()
> returns "true" for it.  However, there already is a similar thing in
> the kernel which already is taken into account by genpd_power_off()
> and which is the PM_QOS_FLAG_NO_POWER_OFF PM QoS flag.

Reading genpd_power_off(), that indeed works, probably.

> The existence of it means that drivers can use it for runtime PM in
> the first place instead of preventing devices from being suspended if
> they need to signal wakeup.  In principle, a driver may just set this

Again, "runtime PM" for system wakeup???

> flag and then suspend the device normally whenever that makes sense
> (of course, taking the flag into account if set) which may lead to
> reduced power draw if (say) device cloks may be gated without
> disturbing its wakeup capability.  Moreover, PM QoS requests may be
> added and removed at any time, so the driver may just set the flag or
> clear it whenever the device's wakeup settings change.
>
> And if this flag is taken into account by genpd_sync_power_off() too,
> using it may just be sufficient to address the concern at hand.
>
> Thoughts?

Sounds reasonable.
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

Re: [RFD] Devices that need full power for wakeup

[Rafael J. Wysocki]

On Monday, January 8, 2018 1:12:49 PM CET Geert Uytterhoeven wrote:
> Hi Rafael,

Hi Geert,

> On Mon, Jan 8, 2018 at 12:51 PM, Rafael J. Wysocki <rafael@kernel.org> wrote:
> > First, let me characterize the problem the way I understand it and if
> > my understanding of it is not correct, let me know why.
> >
> > There are devices that can only signal wakeup if they are provided
> > with full "in-band" power (in contrast with devices that can signal
> > wakeup with the help of a special "wakeup" power source, possibly of
> > smaller capacity).  Suspending them via runtime PM is problematic
> > because of the general assumption that wakeup should always be enabled
> > for all runtime-suspended wakeup-capable devices which leads to sort
> > of a Catch 22 situation in this particular case.
> >
> > Some drivers work around that by only allowing the device to be
> > suspended if they know that it is not expected by user space to signal
> > wakeup and they use the runtime PM usage counter to prevent the device
> > from being suspended otherwise.  That works for runtime PM, but if the
> > system goes to a sleep state, the runtime PM usage counter is not
> > taken into account and the handling of the device may not be adequate
> > and there is a question how to address that.
> 
> Why would they play games with the runtime PM usage counter?
> And why does "that work with runtime PM"?
> Wake-up signals don't matter when a device is runtime resumed, as the
> system is not in a system-wide sleep state in the first place?

I'm talking about wakeup signals in general or "remote wakeup" if you will.

Devices in runtime suspend may produce wakeup signals too (which then mean
that the device needs to be resumed, because there's something going on
outside).

My usual example of this is attaching a network cable to an Ethernet adapter
in runtime suspend, which then may cause a WoL event and a resume of the
device which then can start sending/receiving packets.

> Or do you consider normal interrupt-passing (e.g. pressing a key with
> gpio-keys when the system is not asleep) too?

Yes, remote wakeup in general.

> > Assuming that the above description of the problem is accurate, here's
> > how I see it.
> >
> > All depends on whether or not a device requiring full power for
> > signaling wakeup is allowed to wake up the system from sleeps states.
> > If it is not allowed to do that, it should be fully suspended during
> > system transitions to sleep states regardless of its previous runtime
> > PM status and (in the majority of cases) it may just be left in
> > suspend if already suspended.
> 
> OK.
> 
> > The problematic case is when the device is allowed to wake up the
> > system from sleep states, because it has to be provided with full
> > power while the system is in a sleep state for the wakeup to work.
> > Clearly, checking the runtime PM usage counter alone is not sufficient
> > for that, because the wakeup counter may be nonzero for reasons other
> > than wakeup and the driver may not be able to arrange things properly
> > by itself during system-wide transitions, for example in the cases
> > when the device belongs to a power domain managed by a separate code
> > layer (like genpd).  This leads to the conclusion that it is necessary
> > to provide an additional mechanism allowing all of the layers of code
> > involved in the handling of the device to check what to do with it (or
> > with the power domain it belongs to etc) during system-wide
> > transitions to sleep state.
> 
> OK.
> 
> > In principle, that mechanism might be a new driver flag saying that
> > power should not be removed from the device if device_may_wakeup()
> > returns "true" for it.  However, there already is a similar thing in
> > the kernel which already is taken into account by genpd_power_off()
> > and which is the PM_QOS_FLAG_NO_POWER_OFF PM QoS flag.
> 
> Reading genpd_power_off(), that indeed works, probably.
> 
> > The existence of it means that drivers can use it for runtime PM in
> > the first place instead of preventing devices from being suspended if
> > they need to signal wakeup.  In principle, a driver may just set this
> 
> Again, "runtime PM" for system wakeup???
> 
> > flag and then suspend the device normally whenever that makes sense
> > (of course, taking the flag into account if set) which may lead to
> > reduced power draw if (say) device cloks may be gated without
> > disturbing its wakeup capability.  Moreover, PM QoS requests may be
> > added and removed at any time, so the driver may just set the flag or
> > clear it whenever the device's wakeup settings change.
> >
> > And if this flag is taken into account by genpd_sync_power_off() too,
> > using it may just be sufficient to address the concern at hand.
> >
> > Thoughts?
> 
> Sounds reasonable.
> Thanks!

OK, so something as simple as the patch below (untested) seems to be sufficient
to make that work.

---
 drivers/base/power/domain.c |    9 +++++++++
 1 file changed, 9 insertions(+)

Index: linux-pm/drivers/base/power/domain.c
===================================================================
--- linux-pm.orig/drivers/base/power/domain.c
+++ linux-pm/drivers/base/power/domain.c
@@ -880,6 +880,7 @@ static bool genpd_present(const struct g
 static void genpd_sync_power_off(struct generic_pm_domain *genpd, bool use_lock,
 				 unsigned int depth)
 {
+	struct pm_domain_data *pdd;
 	struct gpd_link *link;
 
 	if (!genpd_status_on(genpd) || genpd_is_always_on(genpd))
@@ -889,6 +890,14 @@ static void genpd_sync_power_off(struct
 	    || atomic_read(&genpd->sd_count) > 0)
 		return;
 
+	list_for_each_entry(pdd, &genpd->dev_list, list_node) {
+		enum pm_qos_flags_status stat;
+
+		stat = dev_pm_qos_flags(pdd->dev, PM_QOS_FLAG_NO_POWER_OFF);
+		if (stat > PM_QOS_FLAGS_NONE)
+			return;
+	}
+
 	/* Choose the deepest state when suspending */
 	genpd->state_idx = genpd->state_count - 1;
 	if (_genpd_power_off(genpd, false))

Re: [RFD] Devices that need full power for wakeup

[Ulf Hansson]

On 8 January 2018 at 12:51, Rafael J. Wysocki <rafael@kernel.org> wrote:
> Hi,
>
> First, let me characterize the problem the way I understand it and if
> my understanding of it is not correct, let me know why.
>
> There are devices that can only signal wakeup if they are provided
> with full "in-band" power (in contrast with devices that can signal
> wakeup with the help of a special "wakeup" power source, possibly of
> smaller capacity).  Suspending them via runtime PM is problematic
> because of the general assumption that wakeup should always be enabled
> for all runtime-suspended wakeup-capable devices which leads to sort
> of a Catch 22 situation in this particular case.
>
> Some drivers work around that by only allowing the device to be
> suspended if they know that it is not expected by user space to signal
> wakeup and they use the runtime PM usage counter to prevent the device
> from being suspended otherwise.  That works for runtime PM, but if the
> system goes to a sleep state, the runtime PM usage counter is not
> taken into account and the handling of the device may not be adequate
> and there is a question how to address that.

As far as I understand, I would describe the problem a bit
differently. Let me elaborate on that.

Genpd treats a device with the ->dev.power.wakeup_path flag set, as it
need to stay powered on during system suspend, thus also its PM
domain. There is nothing new around this, which make me think that we
should continue to treat that as being the default behavior. This is
also what Geert convinced to be the best, from earlier discussions.

However, there are a few related issues around system wakeups in
genpd, during system suspend.

1)
If the driver for the device, configures system wakeup as an
"out-band-wakeup", the device and its PM domain, don't need to stay
powered on during system suspend from the driver perspective. A very
common scenario when this is the case, is when a wakeup signal is
routed via GPIO IRQ.

In these cases, genpd still leaves the device powered on, as well as
its PM domain, thus wasting power during system suspend.

2)
The are cases when device_may_wakeup() returns true for a device, but
because there are no consumers registered for the system wakeup IRQ,
it doesn't make sense for the driver to configure system wakeup for
the device during system suspend. An example of this, exists in the
mmc subsystem, when an SDIO controller driver, during system suspend,
realizes that there are no SDIO functional driver (a WiFi driver for
example) that wants system wakeup signals (SDIO IRQs) to be delivered.

Again, the problem boils done to that the middle-layer/PM domain, only
can check the ->dev.power.wakeup_path flag to know if the device needs
to stay power on during system suspend, while in fact in both case 1)
and case 2), the device could have been powered off.

I think, to cope with these scenarios, we need a way for drivers to
inform the middle-layer/PM domain, that the driver is fine with
powering off the device and its PM domain during system suspend. Of
course, then its up to the middle-layer/PM domain to act on this.

>
> Assuming that the above description of the problem is accurate, here's
> how I see it.
>
> All depends on whether or not a device requiring full power for
> signaling wakeup is allowed to wake up the system from sleeps states.
> If it is not allowed to do that, it should be fully suspended during
> system transitions to sleep states regardless of its previous runtime
> PM status and (in the majority of cases) it may just be left in
> suspend if already suspended.
>
> The problematic case is when the device is allowed to wake up the
> system from sleep states, because it has to be provided with full
> power while the system is in a sleep state for the wakeup to work.
> Clearly, checking the runtime PM usage counter alone is not sufficient
> for that, because the wakeup counter may be nonzero for reasons other
> than wakeup and the driver may not be able to arrange things properly
> by itself during system-wide transitions, for example in the cases
> when the device belongs to a power domain managed by a separate code
> layer (like genpd).  This leads to the conclusion that it is necessary
> to provide an additional mechanism allowing all of the layers of code
> involved in the handling of the device to check what to do with it (or
> with the power domain it belongs to etc) during system-wide
> transitions to sleep state.
>
> In principle, that mechanism might be a new driver flag saying that
> power should not be removed from the device if device_may_wakeup()
> returns "true" for it.  However, there already is a similar thing in
> the kernel which already is taken into account by genpd_power_off()
> and which is the PM_QOS_FLAG_NO_POWER_OFF PM QoS flag.
>
> The existence of it means that drivers can use it for runtime PM in
> the first place instead of preventing devices from being suspended if
> they need to signal wakeup.  In principle, a driver may just set this
> flag and then suspend the device normally whenever that makes sense
> (of course, taking the flag into account if set) which may lead to
> reduced power draw if (say) device cloks may be gated without
> disturbing its wakeup capability.  Moreover, PM QoS requests may be
> added and removed at any time, so the driver may just set the flag or
> clear it whenever the device's wakeup settings change.
>
> And if this flag is taken into account by genpd_sync_power_off() too,
> using it may just be sufficient to address the concern at hand.
>
> Thoughts?

As I tried to describe above, I think we rather need an "opt-out
method", rather than opt-in. In other words, a flag telling that the
device can be powered off, from the driver's perspective.

I also think such flag needs to be propagated to parent devices,
similar to what we do for the wakeup_path flag.

If this makes some sense, I can share some patches which I already in my pipe!?

Kind regards
Uffe

Re: [RFD] Devices that need full power for wakeup

[Rafael J. Wysocki]

On Mon, Jan 8, 2018 at 5:33 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> On 8 January 2018 at 12:51, Rafael J. Wysocki <rafael@kernel.org> wrote:
>> Hi,
>>
>> First, let me characterize the problem the way I understand it and if
>> my understanding of it is not correct, let me know why.
>>
>> There are devices that can only signal wakeup if they are provided
>> with full "in-band" power (in contrast with devices that can signal
>> wakeup with the help of a special "wakeup" power source, possibly of
>> smaller capacity).  Suspending them via runtime PM is problematic
>> because of the general assumption that wakeup should always be enabled
>> for all runtime-suspended wakeup-capable devices which leads to sort
>> of a Catch 22 situation in this particular case.
>>
>> Some drivers work around that by only allowing the device to be
>> suspended if they know that it is not expected by user space to signal
>> wakeup and they use the runtime PM usage counter to prevent the device
>> from being suspended otherwise.  That works for runtime PM, but if the
>> system goes to a sleep state, the runtime PM usage counter is not
>> taken into account and the handling of the device may not be adequate
>> and there is a question how to address that.
>
> As far as I understand, I would describe the problem a bit
> differently. Let me elaborate on that.
>
> Genpd treats a device with the ->dev.power.wakeup_path flag set, as it
> need to stay powered on during system suspend, thus also its PM
> domain. There is nothing new around this, which make me think that we
> should continue to treat that as being the default behavior.

It actually does more than just preventing device (and domain) power
from being turned off, as it causes the entire genpd_finish_suspend()
to be skipped effectively in the genpd_is_active_wakeup() case.

Nothing wrong with that in principle, but let's be precise.

> This is also what Geert convinced to be the best, from earlier discussions.

I'm not sure about that.  Geert seems to need power to stay on,
nothing more, and that regardless of genpd_is_active_wakeup() AFAICS.

> However, there are a few related issues around system wakeups in
> genpd, during system suspend.
>
> 1)
> If the driver for the device, configures system wakeup as an
> "out-band-wakeup", the device and its PM domain, don't need to stay
> powered on during system suspend from the driver perspective.

That depends.

The wakeup signal has to be generated or asserted by something,
presumably the device itself.

Out-of-band usually only means that the signal is routed in a special
way different from the usual device's interrupt routing and not that
the device is not involved in wakeup signaling at all.

A wakeup signal usually means that something interesting happened
outside and that thing needs to be detected in the first place, most
likely by the device.

> A very common scenario when this is the case, is when a wakeup signal is
> routed via GPIO IRQ.
>
> In these cases, genpd still leaves the device powered on, as well as
> its PM domain, thus wasting power during system suspend.

Which probably is the right thing to do.

> 2)
> The are cases when device_may_wakeup() returns true for a device, but
> because there are no consumers registered for the system wakeup IRQ,
> it doesn't make sense for the driver to configure system wakeup for
> the device during system suspend. An example of this, exists in the
> mmc subsystem, when an SDIO controller driver, during system suspend,
> realizes that there are no SDIO functional driver (a WiFi driver for
> example) that wants system wakeup signals (SDIO IRQs) to be delivered.

This isn't genpd-specific, but general.

device_may_wakeup() currently means that the device can and is allowed
(by user space) to signal wakeup, but if the device is just a man in
the middle and merely passes wakeup signals from its children or
consumers, it is not necessary to configure it for wakeup when none of
them are allowed to wake up the system.

So far we have just enabled wakeup unconditionally for such "passive"
devices, although that indeed generally isn't particularly
energy-efficient, but it is simple and it allowed us to avoid quite
some headaches.

> Again, the problem boils done to that the middle-layer/PM domain, only
> can check the ->dev.power.wakeup_path flag to know if the device needs
> to stay power on during system suspend, while in fact in both case 1)
> and case 2), the device could have been powered off.
>
> I think, to cope with these scenarios, we need a way for drivers to
> inform the middle-layer/PM domain, that the driver is fine with
> powering off the device and its PM domain during system suspend. Of
> course, then its up to the middle-layer/PM domain to act on this.

I'm not sure why you want it this way and why it makes sense even.

The handling of "passive" devices requires two things: an indication
that the device is "passive" and the tracking of its "consumers".
wakeup_path only is a poor man's substitute for the latter.

>>
>> Assuming that the above description of the problem is accurate, here's
>> how I see it.
>>
>> All depends on whether or not a device requiring full power for
>> signaling wakeup is allowed to wake up the system from sleeps states.
>> If it is not allowed to do that, it should be fully suspended during
>> system transitions to sleep states regardless of its previous runtime
>> PM status and (in the majority of cases) it may just be left in
>> suspend if already suspended.
>>
>> The problematic case is when the device is allowed to wake up the
>> system from sleep states, because it has to be provided with full
>> power while the system is in a sleep state for the wakeup to work.
>> Clearly, checking the runtime PM usage counter alone is not sufficient
>> for that, because the wakeup counter may be nonzero for reasons other
>> than wakeup and the driver may not be able to arrange things properly
>> by itself during system-wide transitions, for example in the cases
>> when the device belongs to a power domain managed by a separate code
>> layer (like genpd).  This leads to the conclusion that it is necessary
>> to provide an additional mechanism allowing all of the layers of code
>> involved in the handling of the device to check what to do with it (or
>> with the power domain it belongs to etc) during system-wide
>> transitions to sleep state.
>>
>> In principle, that mechanism might be a new driver flag saying that
>> power should not be removed from the device if device_may_wakeup()
>> returns "true" for it.  However, there already is a similar thing in
>> the kernel which already is taken into account by genpd_power_off()
>> and which is the PM_QOS_FLAG_NO_POWER_OFF PM QoS flag.
>>
>> The existence of it means that drivers can use it for runtime PM in
>> the first place instead of preventing devices from being suspended if
>> they need to signal wakeup.  In principle, a driver may just set this
>> flag and then suspend the device normally whenever that makes sense
>> (of course, taking the flag into account if set) which may lead to
>> reduced power draw if (say) device cloks may be gated without
>> disturbing its wakeup capability.  Moreover, PM QoS requests may be
>> added and removed at any time, so the driver may just set the flag or
>> clear it whenever the device's wakeup settings change.
>>
>> And if this flag is taken into account by genpd_sync_power_off() too,
>> using it may just be sufficient to address the concern at hand.
>>
>> Thoughts?
>
> As I tried to describe above, I think we rather need an "opt-out
> method", rather than opt-in. In other words, a flag telling that the
> device can be powered off, from the driver's perspective.
>
> I also think such flag needs to be propagated to parent devices,
> similar to what we do for the wakeup_path flag.
>
> If this makes some sense, I can share some patches which I already in my pipe!?

I'd rather like to agree on general rules to start with.

Thanks,
Rafael

Re: [RFD] Devices that need full power for wakeup

[Ulf Hansson]

On 10 January 2018 at 02:22, Rafael J. Wysocki <rafael@kernel.org> wrote:
> On Mon, Jan 8, 2018 at 5:33 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>> On 8 January 2018 at 12:51, Rafael J. Wysocki <rafael@kernel.org> wrote:
>>> Hi,
>>>
>>> First, let me characterize the problem the way I understand it and if
>>> my understanding of it is not correct, let me know why.
>>>
>>> There are devices that can only signal wakeup if they are provided
>>> with full "in-band" power (in contrast with devices that can signal
>>> wakeup with the help of a special "wakeup" power source, possibly of
>>> smaller capacity).  Suspending them via runtime PM is problematic
>>> because of the general assumption that wakeup should always be enabled
>>> for all runtime-suspended wakeup-capable devices which leads to sort
>>> of a Catch 22 situation in this particular case.
>>>
>>> Some drivers work around that by only allowing the device to be
>>> suspended if they know that it is not expected by user space to signal
>>> wakeup and they use the runtime PM usage counter to prevent the device
>>> from being suspended otherwise.  That works for runtime PM, but if the
>>> system goes to a sleep state, the runtime PM usage counter is not
>>> taken into account and the handling of the device may not be adequate
>>> and there is a question how to address that.
>>
>> As far as I understand, I would describe the problem a bit
>> differently. Let me elaborate on that.
>>
>> Genpd treats a device with the ->dev.power.wakeup_path flag set, as it
>> need to stay powered on during system suspend, thus also its PM
>> domain. There is nothing new around this, which make me think that we
>> should continue to treat that as being the default behavior.
>
> It actually does more than just preventing device (and domain) power
> from being turned off, as it causes the entire genpd_finish_suspend()
> to be skipped effectively in the genpd_is_active_wakeup() case.
>
> Nothing wrong with that in principle, but let's be precise.

Actually this code in genpd isn't entirely correct, even if it doesn't
cause any problems for now.

I know you have pointed to this behavior in genpd before, so I am
going to send a patch to make sure the code in genpd gets correct and
consistent.

>
>> This is also what Geert convinced to be the best, from earlier discussions.
>
> I'm not sure about that.  Geert seems to need power to stay on,
> nothing more, and that regardless of genpd_is_active_wakeup() AFAICS.

That was the initial problem, which is taken care of by using the
->dev.power.wakeup_path flag and the new helper,
device_set_wakeup_path().

This in combination of enabling genpd to deal with wakeups, as that is
currently not the default setting, which is done via setting the
GENPD_FLAG_ACTIVE_WAKEUP configuration flag.

The next part of the problems Geert pointed out was exactly those I am
describing here. Please have a look at his reply at:
https://www.mail-archive.com/linux-renesas-soc@vger.kernel.org/msg21157.html

>
>> However, there are a few related issues around system wakeups in
>> genpd, during system suspend.
>>
>> 1)
>> If the driver for the device, configures system wakeup as an
>> "out-band-wakeup", the device and its PM domain, don't need to stay
>> powered on during system suspend from the driver perspective.
>
> That depends.
>
> The wakeup signal has to be generated or asserted by something,
> presumably the device itself.

The cases I looking at don't require the device to be powered - at
least from the driver's perspective.

A classic example is a SD card detect pin, which is being routed to a
GPIO pin. In these case, that GPIO pin has no logic whatsoever,
coupled with the device for SD controller. Still, it needs to be
manged by the same device driver.

Therefore, device_may_wakeup() may returns true, if userspace wants to
wakeup the system from sleep when there is an SD card inserted.
Although, that doesn't mean we have to keep the logic for the SD
controller device being powered during system suspend, since it would
waste power.

>
> Out-of-band usually only means that the signal is routed in a special
> way different from the usual device's interrupt routing and not that
> the device is not involved in wakeup signaling at all.

Okay, so it's a matter of terminology. I had that feeling, as I have
seen "out-of-band" being used in different context, not always meaning
the same thing.

>
> A wakeup signal usually means that something interesting happened
> outside and that thing needs to be detected in the first place, most
> likely by the device.
>
>> A very common scenario when this is the case, is when a wakeup signal is
>> routed via GPIO IRQ.
>>
>> In these cases, genpd still leaves the device powered on, as well as
>> its PM domain, thus wasting power during system suspend.
>
> Which probably is the right thing to do.

No, it isn't, simply because it would waste power.

Especially for those SoCs, which may have an external HW logic
(perhaps via co-processor) that can be informed from the irqchip
driver, to deal with wakeup GPIO IRQs during system sleep.

In these cases, keeping the device and PM domain powered would
potentially waste a lot of power, which is critical case for battery
driven platforms.

>
>> 2)
>> The are cases when device_may_wakeup() returns true for a device, but
>> because there are no consumers registered for the system wakeup IRQ,
>> it doesn't make sense for the driver to configure system wakeup for
>> the device during system suspend. An example of this, exists in the
>> mmc subsystem, when an SDIO controller driver, during system suspend,
>> realizes that there are no SDIO functional driver (a WiFi driver for
>> example) that wants system wakeup signals (SDIO IRQs) to be delivered.
>
> This isn't genpd-specific, but general.

I fully agree. I just wanted to give genpd as an example.

>
> device_may_wakeup() currently means that the device can and is allowed
> (by user space) to signal wakeup, but if the device is just a man in
> the middle and merely passes wakeup signals from its children or
> consumers, it is not necessary to configure it for wakeup when none of
> them are allowed to wake up the system.
>
> So far we have just enabled wakeup unconditionally for such "passive"
> devices, although that indeed generally isn't particularly
> energy-efficient, but it is simple and it allowed us to avoid quite
> some headaches.

I am not suggesting to change existing behavior, to for example, for
ACPI PM domain.

However, I am looking for how we can find a way for drivers to provide
additional information to the upper layers about its wakeup
configuration, since device_may_wakeup() don't give the full story.
Then, what upper layers decide to do with this information, have to be
decided on case by case.

Perhaps ACPI PM domain could benefit from this information, but if
not, there should be no harm.

>
>> Again, the problem boils done to that the middle-layer/PM domain, only
>> can check the ->dev.power.wakeup_path flag to know if the device needs
>> to stay power on during system suspend, while in fact in both case 1)
>> and case 2), the device could have been powered off.
>>
>> I think, to cope with these scenarios, we need a way for drivers to
>> inform the middle-layer/PM domain, that the driver is fine with
>> powering off the device and its PM domain during system suspend. Of
>> course, then its up to the middle-layer/PM domain to act on this.
>
> I'm not sure why you want it this way and why it makes sense even.

Because it's only the driver that has this information. Somehow it
needs to passed along, so upper layers can decide what to do with it.

>
> The handling of "passive" devices requires two things: an indication
> that the device is "passive" and the tracking of its "consumers".
> wakeup_path only is a poor man's substitute for the latter.

We don't have to use the wakeup_path flag for this, we can have a
separate flag/status bits for this.

The thing I was talking about above, was that currently the PM domain
only have the information in the ->dev.power.wakeup_path flag to act
on (and the device_may_wakeup()) but it's not sufficient to take the
best decision.

>
>>>
>>> Assuming that the above description of the problem is accurate, here's
>>> how I see it.
>>>
>>> All depends on whether or not a device requiring full power for
>>> signaling wakeup is allowed to wake up the system from sleeps states.
>>> If it is not allowed to do that, it should be fully suspended during
>>> system transitions to sleep states regardless of its previous runtime
>>> PM status and (in the majority of cases) it may just be left in
>>> suspend if already suspended.
>>>
>>> The problematic case is when the device is allowed to wake up the
>>> system from sleep states, because it has to be provided with full
>>> power while the system is in a sleep state for the wakeup to work.
>>> Clearly, checking the runtime PM usage counter alone is not sufficient
>>> for that, because the wakeup counter may be nonzero for reasons other
>>> than wakeup and the driver may not be able to arrange things properly
>>> by itself during system-wide transitions, for example in the cases
>>> when the device belongs to a power domain managed by a separate code
>>> layer (like genpd).  This leads to the conclusion that it is necessary
>>> to provide an additional mechanism allowing all of the layers of code
>>> involved in the handling of the device to check what to do with it (or
>>> with the power domain it belongs to etc) during system-wide
>>> transitions to sleep state.
>>>
>>> In principle, that mechanism might be a new driver flag saying that
>>> power should not be removed from the device if device_may_wakeup()
>>> returns "true" for it.  However, there already is a similar thing in
>>> the kernel which already is taken into account by genpd_power_off()
>>> and which is the PM_QOS_FLAG_NO_POWER_OFF PM QoS flag.
>>>
>>> The existence of it means that drivers can use it for runtime PM in
>>> the first place instead of preventing devices from being suspended if
>>> they need to signal wakeup.  In principle, a driver may just set this
>>> flag and then suspend the device normally whenever that makes sense
>>> (of course, taking the flag into account if set) which may lead to
>>> reduced power draw if (say) device cloks may be gated without
>>> disturbing its wakeup capability.  Moreover, PM QoS requests may be
>>> added and removed at any time, so the driver may just set the flag or
>>> clear it whenever the device's wakeup settings change.
>>>
>>> And if this flag is taken into account by genpd_sync_power_off() too,
>>> using it may just be sufficient to address the concern at hand.
>>>
>>> Thoughts?
>>
>> As I tried to describe above, I think we rather need an "opt-out
>> method", rather than opt-in. In other words, a flag telling that the
>> device can be powered off, from the driver's perspective.
>>
>> I also think such flag needs to be propagated to parent devices,
>> similar to what we do for the wakeup_path flag.
>>
>> If this makes some sense, I can share some patches which I already in my pipe!?
>
> I'd rather like to agree on general rules to start with.

Yeah, let's continue here for while, then we can move on to code.

Kind regards
Uffe

Re: [RFD] Devices that need full power for wakeup

[Rafael J. Wysocki]

On Wed, Jan 10, 2018 at 11:21 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> On 10 January 2018 at 02:22, Rafael J. Wysocki <rafael@kernel.org> wrote:
>> On Mon, Jan 8, 2018 at 5:33 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>>> On 8 January 2018 at 12:51, Rafael J. Wysocki <rafael@kernel.org> wrote:

[cut]

>>
>>> This is also what Geert convinced to be the best, from earlier discussions.
>>
>> I'm not sure about that.  Geert seems to need power to stay on,
>> nothing more, and that regardless of genpd_is_active_wakeup() AFAICS.
>
> That was the initial problem, which is taken care of by using the
> ->dev.power.wakeup_path flag and the new helper,
> device_set_wakeup_path().
>
> This in combination of enabling genpd to deal with wakeups, as that is
> currently not the default setting, which is done via setting the
> GENPD_FLAG_ACTIVE_WAKEUP configuration flag.

But that really isn't about wakeup, but rather about whether or not
power can be cut off things, right?

> The next part of the problems Geert pointed out was exactly those I am
> describing here. Please have a look at his reply at:
> https://www.mail-archive.com/linux-renesas-soc@vger.kernel.org/msg21157.html

OK

>>
>>> However, there are a few related issues around system wakeups in
>>> genpd, during system suspend.
>>>
>>> 1)
>>> If the driver for the device, configures system wakeup as an
>>> "out-band-wakeup", the device and its PM domain, don't need to stay
>>> powered on during system suspend from the driver perspective.
>>
>> That depends.
>>
>> The wakeup signal has to be generated or asserted by something,
>> presumably the device itself.
>
> The cases I looking at don't require the device to be powered - at
> least from the driver's perspective.
>
> A classic example is a SD card detect pin, which is being routed to a
> GPIO pin. In these case, that GPIO pin has no logic whatsoever,
> coupled with the device for SD controller. Still, it needs to be
> manged by the same device driver.

So technically it is a different device, but since it is functionally
related to the one originally handled by the driver in question, the
driver is expected to handle this wakeup thing too.

Fair enough, but certainly that is not a common setup for other subsystems.

[BTW, since the wakeup thing is a separate device technically, I would
try to model it as a separate device object in the kernel with its own
wakeup settings.  That would save you some complications you're trying
to deal with IMO.]

Also I'm sort of wondering why the problem is specific to system-wide
suspend.  It looks like if the card slot was empty, it would make
perfect sense to put the "master" device into runtime suspend with no
remote wakeup and then only resume it if the GPIO thing signaled card
insertion.  And this looks like the same problem in disguise.

Moreover, that is quite a departure from the direction of my original
message in this thread.

> Therefore, device_may_wakeup() may returns true, if userspace wants to
> wakeup the system from sleep when there is an SD card inserted.
> Although, that doesn't mean we have to keep the logic for the SD
> controller device being powered during system suspend, since it would
> waste power.

The SD controller need not be provided with power, but of course the
GPIO card detect pin won't work without power I suppose.  It
essentially is like a button attached to the device with its own power
and signal wiring.

Well, for devices like Ethernet or WiFi or even input (say mice and
keyboards, touch panels and so on) power has to be provided to the
device itself for detection of activity while suspended.

You seem to be considering a very special case which may have been
arranged entirely differently, so the usefulness of it as a general
example is quite limited in my view.

Yes, this case should be taken into account too somehow, but how to do
that is a good question and I'm not sure how relevant it is to the
general handling of wakeup to be honest.

>> Out-of-band usually only means that the signal is routed in a special
>> way different from the usual device's interrupt routing and not that
>> the device is not involved in wakeup signaling at all.
>
> Okay, so it's a matter of terminology. I had that feeling, as I have
> seen "out-of-band" being used in different context, not always meaning
> the same thing.

That's fair enough.

More generally, there are two aspects of wakeup: wakeup power and
wakeup signaling.  Wakeup power is what the device needs to be able to
detect activity while suspended and trigger a wakeup signal.  Wakeup
signaling is all about how the wakeup signal is generated (or
asserted), routed and handled.

Wakeup power may be provided in-band or out-of-band (eg. from a
special wakeup power source) and wakeup signaling may be in-band (eg.
via the IRQ normally used by the device when it is not suspended) or
sideband (out-of-band; when there is a special setup for wakeup
signals).  All combinations are generally possible, but in-band wakeup
signaling usually implies in-band wakeup power (although not
necessarily the other way around).

Note that in your example you effectively have two devices, the SD
controller with no wakeup capability of its own and the card detect
pin which can signal wakeup (in-band) and requires in-band (from its
perspective) wakeup power.  Also note that the card detect pin doesn't
take any input from the SD controller, so treating it as part of the
SD controller's wakeup signaling is technically questionable.

>>
>> A wakeup signal usually means that something interesting happened
>> outside and that thing needs to be detected in the first place, most
>> likely by the device.
>>
>>> A very common scenario when this is the case, is when a wakeup signal is
>>> routed via GPIO IRQ.

Routing via a GPIO IRQ doesn't mean that it always works like the card
detect pin given in the example.  Usually, some input from the device
is necessary to activate the IRQ pin.

>>> In these cases, genpd still leaves the device powered on, as well as
>>> its PM domain, thus wasting power during system suspend.
>>
>> Which probably is the right thing to do.
>
> No, it isn't, simply because it would waste power.

It would cause the system to draw more power than necessary.

[Power is the rate of the flow of energy and so it cannot be wasted.
Energy can be wasted, but only in the sense in which fresh water can
be wasted.]

Yes, it would, but that may not be because the domain handling is wrong.

> Especially for those SoCs, which may have an external HW logic
> (perhaps via co-processor) that can be informed from the irqchip
> driver, to deal with wakeup GPIO IRQs during system sleep.
>
> In these cases, keeping the device and PM domain powered would
> potentially waste a lot of power, which is critical case for battery
> driven platforms.

In the absence of any numbers it is hard to say what "a lot" means. :-)

Moreover, your argumentation goes like this:

"I have a case in which two different devices are handled as one by a
single driver.   They are functionally related, so they are
represented as a single entity.  Unfortunately, when remote wakeup
comes into play, that one entity is not handled adequately, because I
have no way to specify that part of it may be turned off completely,
while the other part has to stay on power for wakeup to work."

Fair enough, but the representation used here may be the source of the problem.

>>
>>> 2)
>>> The are cases when device_may_wakeup() returns true for a device, but
>>> because there are no consumers registered for the system wakeup IRQ,
>>> it doesn't make sense for the driver to configure system wakeup for
>>> the device during system suspend. An example of this, exists in the
>>> mmc subsystem, when an SDIO controller driver, during system suspend,
>>> realizes that there are no SDIO functional driver (a WiFi driver for
>>> example) that wants system wakeup signals (SDIO IRQs) to be delivered.
>>
>> This isn't genpd-specific, but general.
>
> I fully agree. I just wanted to give genpd as an example.
>
>>
>> device_may_wakeup() currently means that the device can and is allowed
>> (by user space) to signal wakeup, but if the device is just a man in
>> the middle and merely passes wakeup signals from its children or
>> consumers, it is not necessary to configure it for wakeup when none of
>> them are allowed to wake up the system.
>>
>> So far we have just enabled wakeup unconditionally for such "passive"
>> devices, although that indeed generally isn't particularly
>> energy-efficient, but it is simple and it allowed us to avoid quite
>> some headaches.
>
> I am not suggesting to change existing behavior, to for example, for
> ACPI PM domain.
>
> However, I am looking for how we can find a way for drivers to provide
> additional information to the upper layers about its wakeup
> configuration, since device_may_wakeup() don't give the full story.
> Then, what upper layers decide to do with this information, have to be
> decided on case by case.
>
> Perhaps ACPI PM domain could benefit from this information, but if
> not, there should be no harm.

I'm inclined to agree that the handling of "passive wakeup" devices
(that merely propagate wakeup signals from their "consumers") could be
improved (probably by something along the lines of wakeup_path, but
that also needs to work for runtime PM), so that they aren't
configured for wakeup unnecessarily (which may lead to excessive
energy usage).  Everybody might benefit from that, but for this
purpose it has to be consistent across the core, middle layers and
drivers.

The handling of devices that need in-band wakeup power (but not
in-band wakeup signaling) may be arranged with the help of
PM_QOS_FLAG_NO_POWER_OFF.  At least I haven't seen any arguments to
the contrary so far.

The handling of the cases like in your example seems to be possible to
arrange without any extra changes beyond the above.  At least I don't
see why it can't be done at this point.

>>
>>> Again, the problem boils done to that the middle-layer/PM domain, only
>>> can check the ->dev.power.wakeup_path flag to know if the device needs
>>> to stay power on during system suspend, while in fact in both case 1)
>>> and case 2), the device could have been powered off.
>>>
>>> I think, to cope with these scenarios, we need a way for drivers to
>>> inform the middle-layer/PM domain, that the driver is fine with
>>> powering off the device and its PM domain during system suspend. Of
>>> course, then its up to the middle-layer/PM domain to act on this.
>>
>> I'm not sure why you want it this way and why it makes sense even.
>
> Because it's only the driver that has this information.

If you mean something like "I use an unusual representation of things,
so please bear with me", then I may agree, but then I may not be quite
willing to make any changes in support of that.

> Somehow it needs to passed along, so upper layers can decide what to do with it.
>
>>
>> The handling of "passive" devices requires two things: an indication
>> that the device is "passive" and the tracking of its "consumers".
>> wakeup_path only is a poor man's substitute for the latter.
>
> We don't have to use the wakeup_path flag for this, we can have a
> separate flag/status bits for this.
>
> The thing I was talking about above, was that currently the PM domain
> only have the information in the ->dev.power.wakeup_path flag to act
> on (and the device_may_wakeup()) but it's not sufficient to take the
> best decision.

It may not be sufficient, yes.

That's why I sent the original message in this thread in the first place.

I asked specifically about devices needing in-band wakeup power and
you just started to talk about other things.

I guess we just need to make a list of issues to address, have a look
at the related code in detail and then try to design something
properly.

Thanks,
Rafael

Re: [RFD] Devices that need full power for wakeup

[Geert Uytterhoeven]

Hi Rafael,

On Thu, Jan 11, 2018 at 1:08 AM, Rafael J. Wysocki <rafael@kernel.org> wrote:
> On Wed, Jan 10, 2018 at 11:21 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>> On 10 January 2018 at 02:22, Rafael J. Wysocki <rafael@kernel.org> wrote:
>>> On Mon, Jan 8, 2018 at 5:33 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>>>> On 8 January 2018 at 12:51, Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> [cut]
>
>>>
>>>> This is also what Geert convinced to be the best, from earlier discussions.
>>>
>>> I'm not sure about that.  Geert seems to need power to stay on,
>>> nothing more, and that regardless of genpd_is_active_wakeup() AFAICS.
>>
>> That was the initial problem, which is taken care of by using the
>> ->dev.power.wakeup_path flag and the new helper,
>> device_set_wakeup_path().
>>
>> This in combination of enabling genpd to deal with wakeups, as that is
>> currently not the default setting, which is done via setting the
>> GENPD_FLAG_ACTIVE_WAKEUP configuration flag.
>
> But that really isn't about wakeup, but rather about whether or not
> power can be cut off things, right?

The flag indicates that power cannot be cut if the device is used for wake-up.

>> The next part of the problems Geert pointed out was exactly those I am
>> describing here. Please have a look at his reply at:
>> https://www.mail-archive.com/linux-renesas-soc@vger.kernel.org/msg21157.html
>
> OK
>
>>>
>>>> However, there are a few related issues around system wakeups in
>>>> genpd, during system suspend.
>>>>
>>>> 1)
>>>> If the driver for the device, configures system wakeup as an
>>>> "out-band-wakeup", the device and its PM domain, don't need to stay
>>>> powered on during system suspend from the driver perspective.
>>>
>>> That depends.
>>>
>>> The wakeup signal has to be generated or asserted by something,
>>> presumably the device itself.
>>
>> The cases I looking at don't require the device to be powered - at
>> least from the driver's perspective.
>>
>> A classic example is a SD card detect pin, which is being routed to a
>> GPIO pin. In these case, that GPIO pin has no logic whatsoever,
>> coupled with the device for SD controller. Still, it needs to be
>> manged by the same device driver.
>
> So technically it is a different device, but since it is functionally
> related to the one originally handled by the driver in question, the
> driver is expected to handle this wakeup thing too.
>
> Fair enough, but certainly that is not a common setup for other subsystems.
>
> [BTW, since the wakeup thing is a separate device technically, I would
> try to model it as a separate device object in the kernel with its own
> wakeup settings.  That would save you some complications you're trying
> to deal with IMO.]

Makes sense to me.

> Also I'm sort of wondering why the problem is specific to system-wide
> suspend.  It looks like if the card slot was empty, it would make
> perfect sense to put the "master" device into runtime suspend with no
> remote wakeup and then only resume it if the GPIO thing signaled card
> insertion.  And this looks like the same problem in disguise.

For SD card slots, it's indeed not just about system suspend, but also
about runtime suspend when no card is present.

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

Re: [RFD] Devices that need full power for wakeup

[Ulf Hansson]

[...]

>>>
>>>> However, there are a few related issues around system wakeups in
>>>> genpd, during system suspend.
>>>>
>>>> 1)
>>>> If the driver for the device, configures system wakeup as an
>>>> "out-band-wakeup", the device and its PM domain, don't need to stay
>>>> powered on during system suspend from the driver perspective.
>>>
>>> That depends.
>>>
>>> The wakeup signal has to be generated or asserted by something,
>>> presumably the device itself.
>>
>> The cases I looking at don't require the device to be powered - at
>> least from the driver's perspective.
>>
>> A classic example is a SD card detect pin, which is being routed to a
>> GPIO pin. In these case, that GPIO pin has no logic whatsoever,
>> coupled with the device for SD controller. Still, it needs to be
>> manged by the same device driver.
>
> So technically it is a different device, but since it is functionally
> related to the one originally handled by the driver in question, the
> driver is expected to handle this wakeup thing too.
>
> Fair enough, but certainly that is not a common setup for other subsystems.
>
> [BTW, since the wakeup thing is a separate device technically, I would
> try to model it as a separate device object in the kernel with its own
> wakeup settings.  That would save you some complications you're trying
> to deal with IMO.]
>
> Also I'm sort of wondering why the problem is specific to system-wide
> suspend.  It looks like if the card slot was empty, it would make
> perfect sense to put the "master" device into runtime suspend with no
> remote wakeup and then only resume it if the GPIO thing signaled card
> insertion.  And this looks like the same problem in disguise.
>
> Moreover, that is quite a departure from the direction of my original
> message in this thread.
>
>> Therefore, device_may_wakeup() may returns true, if userspace wants to
>> wakeup the system from sleep when there is an SD card inserted.
>> Although, that doesn't mean we have to keep the logic for the SD
>> controller device being powered during system suspend, since it would
>> waste power.
>
> The SD controller need not be provided with power, but of course the
> GPIO card detect pin won't work without power I suppose.  It
> essentially is like a button attached to the device with its own power
> and signal wiring.

Correct.

>
> Well, for devices like Ethernet or WiFi or even input (say mice and
> keyboards, touch panels and so on) power has to be provided to the
> device itself for detection of activity while suspended.

Actually no.

I realize that SD card detect GPIO was not the best example to
describe the real problem, even if it's related though.

Let me take another example from an embedded battery driven platform:

Assume user space has enabled the wakeup setting for the serial
console device. In a similar system as described earlier, the serial
console driver may be able to re-route the serial RX pin to a GPIO
wake IRQ at system suspend. This means that serial console device may
be completely powered off during system suspend. However, whether
powering off the serial device (and its attached PM domain) can be
done, is currently only known by the driver, as upper-layers (PM
domain) don't know if the RX pin is re-routed to a GPIO wake IRQ or
not.

>
> You seem to be considering a very special case which may have been
> arranged entirely differently, so the usefulness of it as a general
> example is quite limited in my view.
>
> Yes, this case should be taken into account too somehow, but how to do
> that is a good question and I'm not sure how relevant it is to the
> general handling of wakeup to be honest.

Again, the SD card detect GPIO was just one case.

There are several more, which are similar to the above described,
serial console wakeup case.

>
>>> Out-of-band usually only means that the signal is routed in a special
>>> way different from the usual device's interrupt routing and not that
>>> the device is not involved in wakeup signaling at all.
>>
>> Okay, so it's a matter of terminology. I had that feeling, as I have
>> seen "out-of-band" being used in different context, not always meaning
>> the same thing.
>
> That's fair enough.
>
> More generally, there are two aspects of wakeup: wakeup power and
> wakeup signaling.  Wakeup power is what the device needs to be able to
> detect activity while suspended and trigger a wakeup signal.  Wakeup
> signaling is all about how the wakeup signal is generated (or
> asserted), routed and handled.
>
> Wakeup power may be provided in-band or out-of-band (eg. from a
> special wakeup power source) and wakeup signaling may be in-band (eg.
> via the IRQ normally used by the device when it is not suspended) or
> sideband (out-of-band; when there is a special setup for wakeup
> signals).  All combinations are generally possible, but in-band wakeup
> signaling usually implies in-band wakeup power (although not
> necessarily the other way around).
>
> Note that in your example you effectively have two devices, the SD
> controller with no wakeup capability of its own and the card detect
> pin which can signal wakeup (in-band) and requires in-band (from its
> perspective) wakeup power.  Also note that the card detect pin doesn't
> take any input from the SD controller, so treating it as part of the
> SD controller's wakeup signaling is technically questionable.

Right, thanks for the clarifications!

>
>>>
>>> A wakeup signal usually means that something interesting happened
>>> outside and that thing needs to be detected in the first place, most
>>> likely by the device.
>>>
>>>> A very common scenario when this is the case, is when a wakeup signal is
>>>> routed via GPIO IRQ.
>
> Routing via a GPIO IRQ doesn't mean that it always works like the card
> detect pin given in the example.  Usually, some input from the device
> is necessary to activate the IRQ pin.

Well, not in those cases I am referring to.

The input and wakeup is completely driven via the (re-)routed GPIO pin.

>
>>>> In these cases, genpd still leaves the device powered on, as well as
>>>> its PM domain, thus wasting power during system suspend.
>>>
>>> Which probably is the right thing to do.
>>
>> No, it isn't, simply because it would waste power.
>
> It would cause the system to draw more power than necessary.
>
> [Power is the rate of the flow of energy and so it cannot be wasted.
> Energy can be wasted, but only in the sense in which fresh water can
> be wasted.]

Yes, of course. I will switch to "energy" to make it more clear in the future.

>
> Yes, it would, but that may not be because the domain handling is wrong.
>
>> Especially for those SoCs, which may have an external HW logic
>> (perhaps via co-processor) that can be informed from the irqchip
>> driver, to deal with wakeup GPIO IRQs during system sleep.
>>
>> In these cases, keeping the device and PM domain powered would
>> potentially waste a lot of power, which is critical case for battery
>> driven platforms.
>
> In the absence of any numbers it is hard to say what "a lot" means. :-)
>
> Moreover, your argumentation goes like this:
>
> "I have a case in which two different devices are handled as one by a
> single driver.   They are functionally related, so they are
> represented as a single entity.  Unfortunately, when remote wakeup
> comes into play, that one entity is not handled adequately, because I
> have no way to specify that part of it may be turned off completely,
> while the other part has to stay on power for wakeup to work."
>
> Fair enough, but the representation used here may be the source of the problem.

The example with the SD card detect GPIO pin was not the best one. :-(

>
>>>
>>>> 2)
>>>> The are cases when device_may_wakeup() returns true for a device, but
>>>> because there are no consumers registered for the system wakeup IRQ,
>>>> it doesn't make sense for the driver to configure system wakeup for
>>>> the device during system suspend. An example of this, exists in the
>>>> mmc subsystem, when an SDIO controller driver, during system suspend,
>>>> realizes that there are no SDIO functional driver (a WiFi driver for
>>>> example) that wants system wakeup signals (SDIO IRQs) to be delivered.
>>>
>>> This isn't genpd-specific, but general.
>>
>> I fully agree. I just wanted to give genpd as an example.
>>
>>>
>>> device_may_wakeup() currently means that the device can and is allowed
>>> (by user space) to signal wakeup, but if the device is just a man in
>>> the middle and merely passes wakeup signals from its children or
>>> consumers, it is not necessary to configure it for wakeup when none of
>>> them are allowed to wake up the system.
>>>
>>> So far we have just enabled wakeup unconditionally for such "passive"
>>> devices, although that indeed generally isn't particularly
>>> energy-efficient, but it is simple and it allowed us to avoid quite
>>> some headaches.
>>
>> I am not suggesting to change existing behavior, to for example, for
>> ACPI PM domain.
>>
>> However, I am looking for how we can find a way for drivers to provide
>> additional information to the upper layers about its wakeup
>> configuration, since device_may_wakeup() don't give the full story.
>> Then, what upper layers decide to do with this information, have to be
>> decided on case by case.
>>
>> Perhaps ACPI PM domain could benefit from this information, but if
>> not, there should be no harm.
>
> I'm inclined to agree that the handling of "passive wakeup" devices
> (that merely propagate wakeup signals from their "consumers") could be
> improved (probably by something along the lines of wakeup_path, but
> that also needs to work for runtime PM), so that they aren't
> configured for wakeup unnecessarily (which may lead to excessive
> energy usage).  Everybody might benefit from that, but for this
> purpose it has to be consistent across the core, middle layers and
> drivers.
>
> The handling of devices that need in-band wakeup power (but not
> in-band wakeup signaling) may be arranged with the help of
> PM_QOS_FLAG_NO_POWER_OFF.  At least I haven't seen any arguments to
> the contrary so far.
>
> The handling of the cases like in your example seems to be possible to
> arrange without any extra changes beyond the above.  At least I don't
> see why it can't be done at this point.

The new example with the serial console, can't. Can we move on to
discuss that instead?

>
>>>
>>>> Again, the problem boils done to that the middle-layer/PM domain, only
>>>> can check the ->dev.power.wakeup_path flag to know if the device needs
>>>> to stay power on during system suspend, while in fact in both case 1)
>>>> and case 2), the device could have been powered off.
>>>>
>>>> I think, to cope with these scenarios, we need a way for drivers to
>>>> inform the middle-layer/PM domain, that the driver is fine with
>>>> powering off the device and its PM domain during system suspend. Of
>>>> course, then its up to the middle-layer/PM domain to act on this.
>>>
>>> I'm not sure why you want it this way and why it makes sense even.
>>
>> Because it's only the driver that has this information.
>
> If you mean something like "I use an unusual representation of things,
> so please bear with me", then I may agree, but then I may not be quite
> willing to make any changes in support of that.
>
>> Somehow it needs to passed along, so upper layers can decide what to do with it.
>>
>>>
>>> The handling of "passive" devices requires two things: an indication
>>> that the device is "passive" and the tracking of its "consumers".
>>> wakeup_path only is a poor man's substitute for the latter.
>>
>> We don't have to use the wakeup_path flag for this, we can have a
>> separate flag/status bits for this.
>>
>> The thing I was talking about above, was that currently the PM domain
>> only have the information in the ->dev.power.wakeup_path flag to act
>> on (and the device_may_wakeup()) but it's not sufficient to take the
>> best decision.
>
> It may not be sufficient, yes.
>
> That's why I sent the original message in this thread in the first place.
>
> I asked specifically about devices needing in-band wakeup power and
> you just started to talk about other things.

I understand your frustration, apologize for the detour! Again, the SD
card detect was a bad example.

I hoping the new example about the serial console explains the problem better!

>
> I guess we just need to make a list of issues to address, have a look
> at the related code in detail and then try to design something
> properly.

Yes, but I need to describe the problem better, for your understanding
and so far I have failed with that. :-)

Kind regards
Uffe

Re: [RFD] Devices that need full power for wakeup

[Rafael J. Wysocki]

On Thu, Jan 11, 2018 at 3:45 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> [...]
>
>>>>
>>>>> However, there are a few related issues around system wakeups in
>>>>> genpd, during system suspend.
>>>>>
>>>>> 1)
>>>>> If the driver for the device, configures system wakeup as an
>>>>> "out-band-wakeup", the device and its PM domain, don't need to stay
>>>>> powered on during system suspend from the driver perspective.
>>>>
>>>> That depends.
>>>>
>>>> The wakeup signal has to be generated or asserted by something,
>>>> presumably the device itself.
>>>
>>> The cases I looking at don't require the device to be powered - at
>>> least from the driver's perspective.
>>>
>>> A classic example is a SD card detect pin, which is being routed to a
>>> GPIO pin. In these case, that GPIO pin has no logic whatsoever,
>>> coupled with the device for SD controller. Still, it needs to be
>>> manged by the same device driver.
>>
>> So technically it is a different device, but since it is functionally
>> related to the one originally handled by the driver in question, the
>> driver is expected to handle this wakeup thing too.
>>
>> Fair enough, but certainly that is not a common setup for other subsystems.
>>
>> [BTW, since the wakeup thing is a separate device technically, I would
>> try to model it as a separate device object in the kernel with its own
>> wakeup settings.  That would save you some complications you're trying
>> to deal with IMO.]
>>
>> Also I'm sort of wondering why the problem is specific to system-wide
>> suspend.  It looks like if the card slot was empty, it would make
>> perfect sense to put the "master" device into runtime suspend with no
>> remote wakeup and then only resume it if the GPIO thing signaled card
>> insertion.  And this looks like the same problem in disguise.
>>
>> Moreover, that is quite a departure from the direction of my original
>> message in this thread.
>>
>>> Therefore, device_may_wakeup() may returns true, if userspace wants to
>>> wakeup the system from sleep when there is an SD card inserted.
>>> Although, that doesn't mean we have to keep the logic for the SD
>>> controller device being powered during system suspend, since it would
>>> waste power.
>>
>> The SD controller need not be provided with power, but of course the
>> GPIO card detect pin won't work without power I suppose.  It
>> essentially is like a button attached to the device with its own power
>> and signal wiring.
>
> Correct.
>
>>
>> Well, for devices like Ethernet or WiFi or even input (say mice and
>> keyboards, touch panels and so on) power has to be provided to the
>> device itself for detection of activity while suspended.
>
> Actually no.

Why "no"?

The example below doesn't contradict what I said at all.

> I realize that SD card detect GPIO was not the best example to
> describe the real problem, even if it's related though.
>
> Let me take another example from an embedded battery driven platform:
>
> Assume user space has enabled the wakeup setting for the serial
> console device. In a similar system as described earlier, the serial
> console driver may be able to re-route the serial RX pin to a GPIO
> wake IRQ at system suspend.

OK

That's equivalent to switching over to an out-of-band wakeup power
source AFAICS.  There is no substantial practical difference between
that and being able to use a special separate power source for wakeup
for the whole device.

> This means that serial console device may
> be completely powered off during system suspend. However, whether
> powering off the serial device (and its attached PM domain) can be
> done, is currently only known by the driver, as upper-layers (PM
> domain) don't know if the RX pin is re-routed to a GPIO wake IRQ or
> not.

Well, then what does prevent you from defaulting to "turn off in-band
power on suspend" (which is the current state of affairs) and then
using PM_QOS_FLAG_NO_POWER_OFF for indicating the "no power off"
condition in case you've not rerouted the pin?

And I still don't see why this isn't relevant to runtime PM too, at
least in principle.

>>
>> You seem to be considering a very special case which may have been
>> arranged entirely differently, so the usefulness of it as a general
>> example is quite limited in my view.
>>
>> Yes, this case should be taken into account too somehow, but how to do
>> that is a good question and I'm not sure how relevant it is to the
>> general handling of wakeup to be honest.
>
> Again, the SD card detect GPIO was just one case.
>
> There are several more, which are similar to the above described,
> serial console wakeup case.

So far I still don't see a need for new infrastructure beyond the
"passive wakeup devices" case.

[cut]

>>
>>>>
>>>> A wakeup signal usually means that something interesting happened
>>>> outside and that thing needs to be detected in the first place, most
>>>> likely by the device.
>>>>
>>>>> A very common scenario when this is the case, is when a wakeup signal is
>>>>> routed via GPIO IRQ.
>>
>> Routing via a GPIO IRQ doesn't mean that it always works like the card
>> detect pin given in the example.  Usually, some input from the device
>> is necessary to activate the IRQ pin.
>
> Well, not in those cases I am referring to.
>
> The input and wakeup is completely driven via the (re-)routed GPIO pin.

So I should have said "often" instead of "usually" maybe (and that
depends a good deal on what is usual for whom).

The pin rerouting example is just a special case of switching over to
an out-of-band wakeup power source (as I said above), which in some
cases (eg. PCI/ACPI) can be done automatically and transparently with
respect to the driver and in some cases is done by the driver itself.
In the latter case it only is a matter of letting the other layers of
code know whether or not they can turn off the in-band power source
for the device and they really don't need to know that this is related
to wakeup in any way.

Again, genpd (for example) has a very "specific" understanding of
"wakeup".  From its perspective "wakeup" means "do not turn in-band
power off", but that is completely arbitrary and arguably it would be
sufficient to tell it directly "do not turn in-band power off" without
even saying that this is related to wakeup at all.

And in the genpd case in particular it doesn't even need to be
propagated do parents, because the hierarchy of domains should reflect
the power dependencies already anyway.

Thanks,
Rafael

Re: [RFD] Devices that need full power for wakeup

[Ulf Hansson]

[...]

>>>> Therefore, device_may_wakeup() may returns true, if userspace wants to
>>>> wakeup the system from sleep when there is an SD card inserted.
>>>> Although, that doesn't mean we have to keep the logic for the SD
>>>> controller device being powered during system suspend, since it would
>>>> waste power.
>>>
>>> The SD controller need not be provided with power, but of course the
>>> GPIO card detect pin won't work without power I suppose.  It
>>> essentially is like a button attached to the device with its own power
>>> and signal wiring.
>>
>> Correct.
>>
>>>
>>> Well, for devices like Ethernet or WiFi or even input (say mice and
>>> keyboards, touch panels and so on) power has to be provided to the
>>> device itself for detection of activity while suspended.
>>
>> Actually no.
>
> Why "no"?
>
> The example below doesn't contradict what I said at all.

The "no" meant, that the above statement isn't applicable as generic
statement. I meant there are cases when it isn't true, apologize if
that was unclear.

>
>> I realize that SD card detect GPIO was not the best example to
>> describe the real problem, even if it's related though.
>>
>> Let me take another example from an embedded battery driven platform:
>>
>> Assume user space has enabled the wakeup setting for the serial
>> console device. In a similar system as described earlier, the serial
>> console driver may be able to re-route the serial RX pin to a GPIO
>> wake IRQ at system suspend.
>
> OK
>
> That's equivalent to switching over to an out-of-band wakeup power
> source AFAICS.  There is no substantial practical difference between
> that and being able to use a special separate power source for wakeup
> for the whole device.

>From a wakup point of view, yes.

However, in case of the power source you refer to, this isn't
something the driver controls by itself, but rather the upper layers,
right!?

So, in principle for the cases you refer to, upper layers only need to
know that the device has wakeup enabled, in principle, as to be able
to take the correct decision during system suspend. Right?

While in those case I refer to, the driver explicitly needs to be
involved, dealing with the re-routing of the GPIO - *if* it can, as to
enable system wakeups. In these circumstances, upper layers have no
clue of what goes on. This is the big difference in my opinion.

>
>> This means that serial console device may
>> be completely powered off during system suspend. However, whether
>> powering off the serial device (and its attached PM domain) can be
>> done, is currently only known by the driver, as upper-layers (PM
>> domain) don't know if the RX pin is re-routed to a GPIO wake IRQ or
>> not.
>
> Well, then what does prevent you from defaulting to "turn off in-band
> power on suspend" (which is the current state of affairs) and then
> using PM_QOS_FLAG_NO_POWER_OFF for indicating the "no power off"
> condition in case you've not rerouted the pin?
>

Using the PM_QOS_FLAG_NO_POWER_OFF doesn't work, because of the
following reasons.

If genpd would default to power off the "in-band-power" at system
suspend, devices not supporting remote wakeups (keeping devices
runtime resumed), needs to start announcing that genpd needs to keep
the in-band-power on at system suspend. In other words, they need to
set the PM_QOS_FLAG_NO_POWER_OFF flag.

However, that wouldn't be sufficient, as the PM_QOS_FLAG_NO_POWER_OFF
flag isn't propagated to parents devices and other wakeup path
resources.

> And I still don't see why this isn't relevant to runtime PM too, at
> least in principle.

Regarding runtime PM; I don't see any issues, but perhaps you have
some cases for the ACPI PM domain in mind?

Anyway, for genpd and drivers that manages devices attached to it: If
the driver and the device can't be configured for remote wakeup (at
runtime suspend), runtime suspend must be prevented by the driver.
That's the generic behavior drivers must support, no matter if their
devices are attached to genpd or not, right!?

The point is, for remote wakeups, genpd and its users don't need the
PM_QOS_FLAG_NO_POWER_OFF flag, or anything else.

>
>>>
>>> You seem to be considering a very special case which may have been
>>> arranged entirely differently, so the usefulness of it as a general
>>> example is quite limited in my view.
>>>
>>> Yes, this case should be taken into account too somehow, but how to do
>>> that is a good question and I'm not sure how relevant it is to the
>>> general handling of wakeup to be honest.
>>
>> Again, the SD card detect GPIO was just one case.
>>
>> There are several more, which are similar to the above described,
>> serial console wakeup case.
>
> So far I still don't see a need for new infrastructure beyond the
> "passive wakeup devices" case.
>
> [cut]
>
>>>
>>>>>
>>>>> A wakeup signal usually means that something interesting happened
>>>>> outside and that thing needs to be detected in the first place, most
>>>>> likely by the device.
>>>>>
>>>>>> A very common scenario when this is the case, is when a wakeup signal is
>>>>>> routed via GPIO IRQ.
>>>
>>> Routing via a GPIO IRQ doesn't mean that it always works like the card
>>> detect pin given in the example.  Usually, some input from the device
>>> is necessary to activate the IRQ pin.
>>
>> Well, not in those cases I am referring to.
>>
>> The input and wakeup is completely driven via the (re-)routed GPIO pin.
>
> So I should have said "often" instead of "usually" maybe (and that
> depends a good deal on what is usual for whom).
>
> The pin rerouting example is just a special case of switching over to
> an out-of-band wakeup power source (as I said above), which in some
> cases (eg. PCI/ACPI) can be done automatically and transparently with
> respect to the driver and in some cases is done by the driver itself.
> In the latter case it only is a matter of letting the other layers of
> code know whether or not they can turn off the in-band power source
> for the device and they really don't need to know that this is related
> to wakeup in any way.

You have the principles correctly described here, so if we would start
from scratch, this is probably how we should have done it.

>
> Again, genpd (for example) has a very "specific" understanding of
> "wakeup".  From its perspective "wakeup" means "do not turn in-band
> power off", but that is completely arbitrary and arguably it would be
> sufficient to tell it directly "do not turn in-band power off" without
> even saying that this is related to wakeup at all.

Sorry, but there is more to it than this.

In regards to remote wakeup, the above is true, but not in regards to
system wakeups, as then we have two different types of scenarios to
consider.

1) A driver have configured its device for system wakeup and requires
its device+genpd to stay powered on, as to be able to have wakeup
signals delivered. These devices don't support remote wakeups.
2) A driver have configured its device for system wakeup, but allows
the in-band power for its device+genpd to be powered off, because the
driver managed to configure the system wakeup as out-band (GPIO IRQ).

>
> And in the genpd case in particular it doesn't even need to be
> propagated do parents, because the hierarchy of domains should reflect
> the power dependencies already anyway.

This we must not limit the solution to, as it depends on the device/PM
topology of the particular SoC.

I am sure I we already have cases where this is needed, but to me it
doesn't really matter. The complexity in regards to PM topology is
increasing, so I am sure it would be a mistake to accept that kind of
limitation.

Kind regards
Uffe

Re: [RFD] Devices that need full power for wakeup

[Rafael J. Wysocki]

On Friday, January 12, 2018 12:59:04 PM CET Ulf Hansson wrote:
> [...]
> 
> >>>> Therefore, device_may_wakeup() may returns true, if userspace wants to
> >>>> wakeup the system from sleep when there is an SD card inserted.
> >>>> Although, that doesn't mean we have to keep the logic for the SD
> >>>> controller device being powered during system suspend, since it would
> >>>> waste power.
> >>>
> >>> The SD controller need not be provided with power, but of course the
> >>> GPIO card detect pin won't work without power I suppose.  It
> >>> essentially is like a button attached to the device with its own power
> >>> and signal wiring.
> >>
> >> Correct.
> >>
> >>>
> >>> Well, for devices like Ethernet or WiFi or even input (say mice and
> >>> keyboards, touch panels and so on) power has to be provided to the
> >>> device itself for detection of activity while suspended.
> >>
> >> Actually no.
> >
> > Why "no"?
> >
> > The example below doesn't contradict what I said at all.
> 
> The "no" meant, that the above statement isn't applicable as generic
> statement. I meant there are cases when it isn't true, apologize if
> that was unclear.
> 
> >
> >> I realize that SD card detect GPIO was not the best example to
> >> describe the real problem, even if it's related though.
> >>
> >> Let me take another example from an embedded battery driven platform:
> >>
> >> Assume user space has enabled the wakeup setting for the serial
> >> console device. In a similar system as described earlier, the serial
> >> console driver may be able to re-route the serial RX pin to a GPIO
> >> wake IRQ at system suspend.
> >
> > OK
> >
> > That's equivalent to switching over to an out-of-band wakeup power
> > source AFAICS.  There is no substantial practical difference between
> > that and being able to use a special separate power source for wakeup
> > for the whole device.
> 
> From a wakup point of view, yes.
> 
> However, in case of the power source you refer to, this isn't
> something the driver controls by itself, but rather the upper layers,
> right!?

That may or may not be the case.

I guess your point is that the middle layer may control both the in-band and
out-of-band power sources and it needs to be told which one to use.

I'm not sure that this really is a problem, because the out-of-band wakeup
power source has to be represented somehow for the driver to be able to use it.
In your example, the driver can only reroute the pin to the GPIO if it knows
that the GPIO is there and then configure it for wakeup.  Then, the IRQ
chip driver responsible for it needs to figure out how to ensure sufficient
power for it to work while suspended (that's the Geert's use case or at least
part of it).

> So, in principle for the cases you refer to, upper layers only need to
> know that the device has wakeup enabled, in principle, as to be able
> to take the correct decision during system suspend. Right?

That depends on what the upper layer itself can do.

If there are facilities for dealing with wakeup in the upper layer, it should
be sufficient for it to know that the device can and is allowed to wake up the
system.  Then, it will just apply those facilities to the device.

OTOH, if the upper layer has no such facilities, the driver needs to give it
hints regarding what needs to be done in terms of what the upper layer *can*
do.

> While in those case I refer to, the driver explicitly needs to be
> involved, dealing with the re-routing of the GPIO - *if* it can, as to
> enable system wakeups. In these circumstances, upper layers have no
> clue of what goes on. This is the big difference in my opinion.

There is a difference if the driver needs to be involved in setting up the
device for wakeup (both system-wide and in runtime PM), but in that case the
driver knows (a) what wakeup IRQ it is going to use and (b) whether or not
in-band power may be turned off for it.  There are ways to indicate both of
these things to the other code layers today.

What else is needed, specifically?

> >
> >> This means that serial console device may
> >> be completely powered off during system suspend. However, whether
> >> powering off the serial device (and its attached PM domain) can be
> >> done, is currently only known by the driver, as upper-layers (PM
> >> domain) don't know if the RX pin is re-routed to a GPIO wake IRQ or
> >> not.
> >
> > Well, then what does prevent you from defaulting to "turn off in-band
> > power on suspend" (which is the current state of affairs) and then
> > using PM_QOS_FLAG_NO_POWER_OFF for indicating the "no power off"
> > condition in case you've not rerouted the pin?
> >
> 
> Using the PM_QOS_FLAG_NO_POWER_OFF doesn't work, because of the
> following reasons.
> 
> If genpd would default to power off the "in-band-power" at system
> suspend, devices not supporting remote wakeups (keeping devices
> runtime resumed), needs to start announcing that genpd needs to keep
> the in-band-power on at system suspend. In other words, they need to
> set the PM_QOS_FLAG_NO_POWER_OFF flag.
> 
> However, that wouldn't be sufficient, as the PM_QOS_FLAG_NO_POWER_OFF
> flag isn't propagated to parents devices and other wakeup path
> resources.

It only needs to be propagated to parents if the parents actively
route wakeup signals from the children or provide them with power.
In the genpd case the latter is not relevant at least.

> > And I still don't see why this isn't relevant to runtime PM too, at
> > least in principle.
> 
> Regarding runtime PM; I don't see any issues,

I wonder why.

How exactly do you tell the middle layer to avoid turning the in-band power
off for devices that are suspended if they need in-band power to signal
remote wakeup?

Your example with the pin rerouting is perfectly relevant for runtime PM too,
in particular, as the driver may very well want to reroute the pin for remote
wakeup and turn the in-band power off for the device then during runtime
suspend just like it does for system-wide suspend.

> but perhaps you have some cases for the ACPI PM domain in mind?
> 
> Anyway, for genpd and drivers that manages devices attached to it: If
> the driver and the device can't be configured for remote wakeup (at
> runtime suspend), runtime suspend must be prevented by the driver.

Why?  I don't see any technical reason for that whatever.

The "must" part seems to be totally artificial here.

> That's the generic behavior drivers must support, no matter if their
> devices are attached to genpd or not, right!?

Nope.

> The point is, for remote wakeups, genpd and its users don't need the
> PM_QOS_FLAG_NO_POWER_OFF flag, or anything else.

Yes, they do or at least they may, which is why it is already supported in
there.

I'll respond to the rest of this later, sorry for being time-constrained.

Thanks,
Rafael

Re: [RFD] Devices that need full power for wakeup

[Ulf Hansson]

On 13 January 2018 at 03:42, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> On Friday, January 12, 2018 12:59:04 PM CET Ulf Hansson wrote:
>> [...]
>>
>> >>>> Therefore, device_may_wakeup() may returns true, if userspace wants to
>> >>>> wakeup the system from sleep when there is an SD card inserted.
>> >>>> Although, that doesn't mean we have to keep the logic for the SD
>> >>>> controller device being powered during system suspend, since it would
>> >>>> waste power.
>> >>>
>> >>> The SD controller need not be provided with power, but of course the
>> >>> GPIO card detect pin won't work without power I suppose.  It
>> >>> essentially is like a button attached to the device with its own power
>> >>> and signal wiring.
>> >>
>> >> Correct.
>> >>
>> >>>
>> >>> Well, for devices like Ethernet or WiFi or even input (say mice and
>> >>> keyboards, touch panels and so on) power has to be provided to the
>> >>> device itself for detection of activity while suspended.
>> >>
>> >> Actually no.
>> >
>> > Why "no"?
>> >
>> > The example below doesn't contradict what I said at all.
>>
>> The "no" meant, that the above statement isn't applicable as generic
>> statement. I meant there are cases when it isn't true, apologize if
>> that was unclear.
>>
>> >
>> >> I realize that SD card detect GPIO was not the best example to
>> >> describe the real problem, even if it's related though.
>> >>
>> >> Let me take another example from an embedded battery driven platform:
>> >>
>> >> Assume user space has enabled the wakeup setting for the serial
>> >> console device. In a similar system as described earlier, the serial
>> >> console driver may be able to re-route the serial RX pin to a GPIO
>> >> wake IRQ at system suspend.
>> >
>> > OK
>> >
>> > That's equivalent to switching over to an out-of-band wakeup power
>> > source AFAICS.  There is no substantial practical difference between
>> > that and being able to use a special separate power source for wakeup
>> > for the whole device.
>>
>> From a wakup point of view, yes.
>>
>> However, in case of the power source you refer to, this isn't
>> something the driver controls by itself, but rather the upper layers,
>> right!?
>
> That may or may not be the case.
>
> I guess your point is that the middle layer may control both the in-band and
> out-of-band power sources and it needs to be told which one to use.
>
> I'm not sure that this really is a problem, because the out-of-band wakeup
> power source has to be represented somehow for the driver to be able to use it.
> In your example, the driver can only reroute the pin to the GPIO if it knows
> that the GPIO is there and then configure it for wakeup.  Then, the IRQ
> chip driver responsible for it needs to figure out how to ensure sufficient
> power for it to work while suspended (that's the Geert's use case or at least
> part of it).
>
>> So, in principle for the cases you refer to, upper layers only need to
>> know that the device has wakeup enabled, in principle, as to be able
>> to take the correct decision during system suspend. Right?
>
> That depends on what the upper layer itself can do.
>
> If there are facilities for dealing with wakeup in the upper layer, it should
> be sufficient for it to know that the device can and is allowed to wake up the
> system.  Then, it will just apply those facilities to the device.
>
> OTOH, if the upper layer has no such facilities, the driver needs to give it
> hints regarding what needs to be done in terms of what the upper layer *can*
> do.
>
>> While in those case I refer to, the driver explicitly needs to be
>> involved, dealing with the re-routing of the GPIO - *if* it can, as to
>> enable system wakeups. In these circumstances, upper layers have no
>> clue of what goes on. This is the big difference in my opinion.
>
> There is a difference if the driver needs to be involved in setting up the
> device for wakeup (both system-wide and in runtime PM), but in that case the
> driver knows (a) what wakeup IRQ it is going to use and (b) whether or not
> in-band power may be turned off for it.  There are ways to indicate both of
> these things to the other code layers today.
>
> What else is needed, specifically?

See at the end of my reply.

>
>> >
>> >> This means that serial console device may
>> >> be completely powered off during system suspend. However, whether
>> >> powering off the serial device (and its attached PM domain) can be
>> >> done, is currently only known by the driver, as upper-layers (PM
>> >> domain) don't know if the RX pin is re-routed to a GPIO wake IRQ or
>> >> not.
>> >
>> > Well, then what does prevent you from defaulting to "turn off in-band
>> > power on suspend" (which is the current state of affairs) and then
>> > using PM_QOS_FLAG_NO_POWER_OFF for indicating the "no power off"
>> > condition in case you've not rerouted the pin?
>> >
>>
>> Using the PM_QOS_FLAG_NO_POWER_OFF doesn't work, because of the
>> following reasons.
>>
>> If genpd would default to power off the "in-band-power" at system
>> suspend, devices not supporting remote wakeups (keeping devices
>> runtime resumed), needs to start announcing that genpd needs to keep
>> the in-band-power on at system suspend. In other words, they need to
>> set the PM_QOS_FLAG_NO_POWER_OFF flag.
>>
>> However, that wouldn't be sufficient, as the PM_QOS_FLAG_NO_POWER_OFF
>> flag isn't propagated to parents devices and other wakeup path
>> resources.
>
> It only needs to be propagated to parents if the parents actively
> route wakeup signals from the children or provide them with power.

That is correct, but how would the children driver know when to
propagate the PM_QOS_FLAG_NO_POWER_OFF flag from the child device to
the parent device?

Or maybe that is not what you are suggesting?

> In the genpd case the latter is not relevant at least.

Why not?

Especially we may have cases when the child device isn't attached to
genpd, while the parent device may be.

>
>> > And I still don't see why this isn't relevant to runtime PM too, at
>> > least in principle.
>>
>> Regarding runtime PM; I don't see any issues,
>
> I wonder why.
>
> How exactly do you tell the middle layer to avoid turning the in-band power
> off for devices that are suspended if they need in-band power to signal
> remote wakeup?

The driver simply don't allow its device to become runtime suspended.

>
> Your example with the pin rerouting is perfectly relevant for runtime PM too,
> in particular, as the driver may very well want to reroute the pin for remote
> wakeup and turn the in-band power off for the device then during runtime
> suspend just like it does for system-wide suspend.

Correct! In such case the driver allows its device to become runtime suspended.

>
>> but perhaps you have some cases for the ACPI PM domain in mind?
>>
>> Anyway, for genpd and drivers that manages devices attached to it: If
>> the driver and the device can't be configured for remote wakeup (at
>> runtime suspend), runtime suspend must be prevented by the driver.
>
> Why?  I don't see any technical reason for that whatever.
>
> The "must" part seems to be totally artificial here.
>
>> That's the generic behavior drivers must support, no matter if their
>> devices are attached to genpd or not, right!?
>
> Nope.

OK, this explains some of our disagreement. :-)

Artificial or not, I find it quite unlikely that this would work for
the general case. The reason is because of how runtime PM totally
respects for the device hierarchy.

For example, if a child driver allows it child device to enter runtime
suspend, while it still requires the in-band power to be provided to
its device - then how would the parent driver know how to behave in
this regards?

Especially in cases when the parent device provides power to the
child, then, when the parent device's child count drops to zero, the
parent device may also enter runtime suspend. In other words, what
happens to the child when the parent stops providing power to it,
under these circumstances?

I think the general assumption for these cases, is that it wouldn't
play well, which is why the child device is prevented to enter runtime
suspend, in case it needs in-band power as to deliver remote-wakeups.

>
>> The point is, for remote wakeups, genpd and its users don't need the
>> PM_QOS_FLAG_NO_POWER_OFF flag, or anything else.
>
> Yes, they do or at least they may, which is why it is already supported in
> there.

I did a quick investigation. The PM_QOS_FLAG_NO_POWER_OFF flag, added
in 2012, only have two drivers that use it today. These are:

1) drivers/pci/pci-acpi.c.
I assume this isn't a driver for devices that can be attached to genpd.

2) drivers/usb/core/port.c
This registers an internal struct device for the usb port, which it
deploys runtime PM support for and sets the PM_QOS_FLAG_NO_POWER_OFF
flag to. The internal created device can't be attached to genpd,
however it gets the usb hub device assigned to its parent, which is an
interesting part, as the PM_QOS_FLAG_NO_POWER_OFF doesn't become
propagated to it.

This made me realize that the code in genpd dealing with the
PM_QOS_FLAG_NO_POWER_OFF flag, is unused, unless I am missing another
use case, of course.

Another observation regarding the PM_QOS_FLAG_NO_POWER_OFF flag, is
that it's QOS thing, which may also be controlled from userspace via
sysfs. Using a QOS flag to prevent to turn of in-band power as to be
able to support wakeups correctly, just doesn't seems right to me. I
rather consider this as a HW configuration that must be set to support
a requested wakeup.

Going back to what I think is needed. I think we need a per device PM
flag that the driver can set, to instruct upper layers about how it
has configured the wakeup (in-band or out-band). That flag needs to be
propagated to parents as well. Moreover, I am questioning whether such
flag may be useful for runtime PM, maybe it could.

>
> I'll respond to the rest of this later, sorry for being time-constrained.

No worries, there is no rush!

Kind regards
Uffe

Re: [RFD] Devices that need full power for wakeup

[Rafael J. Wysocki]

On Mon, Jan 15, 2018 at 10:11 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> On 13 January 2018 at 03:42, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
>> On Friday, January 12, 2018 12:59:04 PM CET Ulf Hansson wrote:

[cut]

>>
>>> >
>>> >> This means that serial console device may
>>> >> be completely powered off during system suspend. However, whether
>>> >> powering off the serial device (and its attached PM domain) can be
>>> >> done, is currently only known by the driver, as upper-layers (PM
>>> >> domain) don't know if the RX pin is re-routed to a GPIO wake IRQ or
>>> >> not.
>>> >
>>> > Well, then what does prevent you from defaulting to "turn off in-band
>>> > power on suspend" (which is the current state of affairs) and then
>>> > using PM_QOS_FLAG_NO_POWER_OFF for indicating the "no power off"
>>> > condition in case you've not rerouted the pin?
>>> >
>>>
>>> Using the PM_QOS_FLAG_NO_POWER_OFF doesn't work, because of the
>>> following reasons.
>>>
>>> If genpd would default to power off the "in-band-power" at system
>>> suspend, devices not supporting remote wakeups (keeping devices
>>> runtime resumed), needs to start announcing that genpd needs to keep
>>> the in-band-power on at system suspend. In other words, they need to
>>> set the PM_QOS_FLAG_NO_POWER_OFF flag.
>>>
>>> However, that wouldn't be sufficient, as the PM_QOS_FLAG_NO_POWER_OFF
>>> flag isn't propagated to parents devices and other wakeup path
>>> resources.
>>
>> It only needs to be propagated to parents if the parents actively
>> route wakeup signals from the children or provide them with power.
>
> That is correct, but how would the children driver know when to
> propagate the PM_QOS_FLAG_NO_POWER_OFF flag from the child device to
> the parent device?
>
> Or maybe that is not what you are suggesting?

No, it isn't.

I see no valid reasons for propagating PM_QOS_FLAG_NO_POWER_OFF.  The
only case in which it could be potentially useful is when the parent
is explicitly providing the children with power, but in that case it
is the responsibility of the parent's driver to take the PM QoS flags
of the children into account and avoid turning off power at the parent
if need be.

OTOH, I see valid reasons for PM_QOS_FLAG_NO_POWER_OFF to not be
propagated (like when the parent does not provide the children with
power directly in which case there is no reason to avoid turning off
power at the parent when any of the children have that flag set).

>> In the genpd case the latter is not relevant at least.
>
> Why not?
>
> Especially we may have cases when the child device isn't attached to
> genpd, while the parent device may be.

That only matters if the parent provides power to the child directly
in which case, again, the parent's driver should (a) know that and (b)
take care of the child's PM QoS requirements as appropriate.

>>
>>> > And I still don't see why this isn't relevant to runtime PM too, at
>>> > least in principle.
>>>
>>> Regarding runtime PM; I don't see any issues,
>>
>> I wonder why.
>>
>> How exactly do you tell the middle layer to avoid turning the in-band power
>> off for devices that are suspended if they need in-band power to signal
>> remote wakeup?
>
> The driver simply don't allow its device to become runtime suspended.

Well, this is a strategy of dealing with the requirement, but it may
be sort of wasteful in some cases, like when you could disable clocks
for the suspended device while avoiding to remove in-band power from
it at the same time.

It arguably isn't the only strategy that can be used.

>>
>> Your example with the pin rerouting is perfectly relevant for runtime PM too,
>> in particular, as the driver may very well want to reroute the pin for remote
>> wakeup and turn the in-band power off for the device then during runtime
>> suspend just like it does for system-wide suspend.
>
> Correct! In such case the driver allows its device to become runtime suspended.

So instead of preventing runtime suspend from happening at all in the
other case the driver could indicate the requirement to supply in-band
power to the device via PM QoS and still allow runtime suspend to
work.

If that strategy is used, the runtime suspend and system-wide suspend
cases are not distinct any more from the driver's perspective.

>>
>>> but perhaps you have some cases for the ACPI PM domain in mind?
>>>
>>> Anyway, for genpd and drivers that manages devices attached to it: If
>>> the driver and the device can't be configured for remote wakeup (at
>>> runtime suspend), runtime suspend must be prevented by the driver.
>>
>> Why?  I don't see any technical reason for that whatever.
>>
>> The "must" part seems to be totally artificial here.
>>
>>> That's the generic behavior drivers must support, no matter if their
>>> devices are attached to genpd or not, right!?
>>
>> Nope.
>
> OK, this explains some of our disagreement. :-)
>
> Artificial or not, I find it quite unlikely that this would work for
> the general case. The reason is because of how runtime PM totally
> respects for the device hierarchy.
>
> For example, if a child driver allows it child device to enter runtime
> suspend, while it still requires the in-band power to be provided to
> its device - then how would the parent driver know how to behave in
> this regards?

Either the parent directly supplies in-band power to its children or
it doesn't.  In the former case the parent's driver needs to know that
this is the case and deal with it.  In the latter case it doesn't need
to care.

> Especially in cases when the parent device provides power to the
> child, then, when the parent device's child count drops to zero, the
> parent device may also enter runtime suspend.

Yes, it can enter the "suspended" metastate, but that metastate may be
mapped to multiple physical states.  Which of them is actually used
may depend on the power requirements of the children.

> In other words, what happens to the child when the parent stops providing power to it,
> under these circumstances?
>
> I think the general assumption for these cases, is that it wouldn't
> play well, which is why the child device is prevented to enter runtime
> suspend, in case it needs in-band power as to deliver remote-wakeups.

That is how it may be implemented, but it doesn't have to be done this way.

Again, please don't confuse a requirement with a strategy to meet it.

>>> The point is, for remote wakeups, genpd and its users don't need the
>>> PM_QOS_FLAG_NO_POWER_OFF flag, or anything else.
>>
>> Yes, they do or at least they may, which is why it is already supported in
>> there.
>
> I did a quick investigation. The PM_QOS_FLAG_NO_POWER_OFF flag, added
> in 2012, only have two drivers that use it today. These are:
>
> 1) drivers/pci/pci-acpi.c.
> I assume this isn't a driver for devices that can be attached to genpd.
>
> 2) drivers/usb/core/port.c
> This registers an internal struct device for the usb port, which it
> deploys runtime PM support for and sets the PM_QOS_FLAG_NO_POWER_OFF
> flag to. The internal created device can't be attached to genpd,
> however it gets the usb hub device assigned to its parent, which is an
> interesting part, as the PM_QOS_FLAG_NO_POWER_OFF doesn't become
> propagated to it.
>
> This made me realize that the code in genpd dealing with the
> PM_QOS_FLAG_NO_POWER_OFF flag, is unused, unless I am missing another
> use case, of course.

That's probably the case.

It was added in anticipation of future use in drivers.

> Another observation regarding the PM_QOS_FLAG_NO_POWER_OFF flag, is
> that it's QOS thing, which may also be controlled from userspace via
> sysfs. Using a QOS flag to prevent to turn of in-band power as to be
> able to support wakeups correctly, just doesn't seems right to me. I
> rather consider this as a HW configuration that must be set to support
> a requested wakeup.

User space may want in-band power to be provided to the device while
suspended too and I'm not sure what's the difference between that and
the wakeup requirement.

The point here, among other things, is that it doesn't really matter
*why* the device needs to be provided with in-band power while
suspended.  Potentially, there may be multiple valid reasons for that
and wakeup is only one of them, so why does it need to be special?

> Going back to what I think is needed. I think we need a per device PM
> flag that the driver can set, to instruct upper layers about how it
> has configured the wakeup (in-band or out-band). That flag needs to be
> propagated to parents as well. Moreover, I am questioning whether such
> flag may be useful for runtime PM, maybe it could.

Upper layers need not be informed about wakeup in general, like genpd
which only understands (a) whether or not to cut off in-band power and
(b) whether or not to invoke the "stop" callback.  It doesn't need to
care about why those two things may or may not be done.

My first conclusion from the above is that the runtime PM case and the
system-wide PM case are really the same from the wakeup setup
perspective and I don't see general reasons for them to be handled
differently.

The way to communicate the requirements related to that to upper
layers, parent drivers etc may be via PM QoS flags, but I'm not
insisting on that.  It has just occurred to me that it is one possible
way to do it and there was some work done in that direction.

Thanks,
Rafael

Re: [RFD] Devices that need full power for wakeup

[Ulf Hansson]

On 22 January 2018 at 09:00, Rafael J. Wysocki <rafael@kernel.org> wrote:
> On Mon, Jan 15, 2018 at 10:11 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>> On 13 January 2018 at 03:42, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
>>> On Friday, January 12, 2018 12:59:04 PM CET Ulf Hansson wrote:
>
> [cut]
>
>>>
>>>> >
>>>> >> This means that serial console device may
>>>> >> be completely powered off during system suspend. However, whether
>>>> >> powering off the serial device (and its attached PM domain) can be
>>>> >> done, is currently only known by the driver, as upper-layers (PM
>>>> >> domain) don't know if the RX pin is re-routed to a GPIO wake IRQ or
>>>> >> not.
>>>> >
>>>> > Well, then what does prevent you from defaulting to "turn off in-band
>>>> > power on suspend" (which is the current state of affairs) and then
>>>> > using PM_QOS_FLAG_NO_POWER_OFF for indicating the "no power off"
>>>> > condition in case you've not rerouted the pin?
>>>> >
>>>>
>>>> Using the PM_QOS_FLAG_NO_POWER_OFF doesn't work, because of the
>>>> following reasons.
>>>>
>>>> If genpd would default to power off the "in-band-power" at system
>>>> suspend, devices not supporting remote wakeups (keeping devices
>>>> runtime resumed), needs to start announcing that genpd needs to keep
>>>> the in-band-power on at system suspend. In other words, they need to
>>>> set the PM_QOS_FLAG_NO_POWER_OFF flag.
>>>>
>>>> However, that wouldn't be sufficient, as the PM_QOS_FLAG_NO_POWER_OFF
>>>> flag isn't propagated to parents devices and other wakeup path
>>>> resources.
>>>
>>> It only needs to be propagated to parents if the parents actively
>>> route wakeup signals from the children or provide them with power.
>>
>> That is correct, but how would the children driver know when to
>> propagate the PM_QOS_FLAG_NO_POWER_OFF flag from the child device to
>> the parent device?
>>
>> Or maybe that is not what you are suggesting?
>
> No, it isn't.
>
> I see no valid reasons for propagating PM_QOS_FLAG_NO_POWER_OFF.  The
> only case in which it could be potentially useful is when the parent
> is explicitly providing the children with power, but in that case it
> is the responsibility of the parent's driver to take the PM QoS flags
> of the children into account and avoid turning off power at the parent
> if need be.

Okay.

>
> OTOH, I see valid reasons for PM_QOS_FLAG_NO_POWER_OFF to not be
> propagated (like when the parent does not provide the children with
> power directly in which case there is no reason to avoid turning off
> power at the parent when any of the children have that flag set).

The idea with the propagation, is to provide the parent and upper
layers with information, which currently is missing. How they act on
the new information, must of course be decided on case by case.

Not doing the propagation, means that a parent needs to check all its
children, and those children needs to check their children, and so on.
It works, but perhaps not very efficient.

>
>>> In the genpd case the latter is not relevant at least.
>>
>> Why not?
>>
>> Especially we may have cases when the child device isn't attached to
>> genpd, while the parent device may be.
>
> That only matters if the parent provides power to the child directly
> in which case, again, the parent's driver should (a) know that and (b)
> take care of the child's PM QoS requirements as appropriate.
>
>>>
>>>> > And I still don't see why this isn't relevant to runtime PM too, at
>>>> > least in principle.
>>>>
>>>> Regarding runtime PM; I don't see any issues,
>>>
>>> I wonder why.
>>>
>>> How exactly do you tell the middle layer to avoid turning the in-band power
>>> off for devices that are suspended if they need in-band power to signal
>>> remote wakeup?
>>
>> The driver simply don't allow its device to become runtime suspended.
>
> Well, this is a strategy of dealing with the requirement, but it may
> be sort of wasteful in some cases, like when you could disable clocks
> for the suspended device while avoiding to remove in-band power from
> it at the same time.
>
> It arguably isn't the only strategy that can be used.

You have a point!

Anyway, as far as I know, this is the only strategy that currently
exists, hence why I didn't think runtime PM was something we needed to
consider.

>
>>>
>>> Your example with the pin rerouting is perfectly relevant for runtime PM too,
>>> in particular, as the driver may very well want to reroute the pin for remote
>>> wakeup and turn the in-band power off for the device then during runtime
>>> suspend just like it does for system-wide suspend.
>>
>> Correct! In such case the driver allows its device to become runtime suspended.
>
> So instead of preventing runtime suspend from happening at all in the
> other case the driver could indicate the requirement to supply in-band
> power to the device via PM QoS and still allow runtime suspend to
> work.

Right!

>
> If that strategy is used, the runtime suspend and system-wide suspend
> cases are not distinct any more from the driver's perspective.
>
>>>
>>>> but perhaps you have some cases for the ACPI PM domain in mind?
>>>>
>>>> Anyway, for genpd and drivers that manages devices attached to it: If
>>>> the driver and the device can't be configured for remote wakeup (at
>>>> runtime suspend), runtime suspend must be prevented by the driver.
>>>
>>> Why?  I don't see any technical reason for that whatever.
>>>
>>> The "must" part seems to be totally artificial here.
>>>
>>>> That's the generic behavior drivers must support, no matter if their
>>>> devices are attached to genpd or not, right!?
>>>
>>> Nope.
>>
>> OK, this explains some of our disagreement. :-)
>>
>> Artificial or not, I find it quite unlikely that this would work for
>> the general case. The reason is because of how runtime PM totally
>> respects for the device hierarchy.
>>
>> For example, if a child driver allows it child device to enter runtime
>> suspend, while it still requires the in-band power to be provided to
>> its device - then how would the parent driver know how to behave in
>> this regards?
>
> Either the parent directly supplies in-band power to its children or
> it doesn't.  In the former case the parent's driver needs to know that
> this is the case and deal with it.  In the latter case it doesn't need
> to care.

If I understand correctly, you are suggesting that the parent driver
then needs to check the PM_QOS_FLAG_NO_POWER_OFF flag for all its
children devices (and so on), each time the parent device is about to
be become runtime suspended, no?

>
>> Especially in cases when the parent device provides power to the
>> child, then, when the parent device's child count drops to zero, the
>> parent device may also enter runtime suspend.
>
> Yes, it can enter the "suspended" metastate, but that metastate may be
> mapped to multiple physical states.  Which of them is actually used
> may depend on the power requirements of the children.

Again, you are right.

However, what you are describing is to me a different (but related)
and more complex problem. As you say, this is about how to support
*multiple* device idle states.

My point is, trying to make use of PM_QOS_FLAG_NO_POWER_OFF, would in
principle enable support for *one* more device idle state. To me, that
doesn't sounds like a future proof solution. Don't we need something
more flexible?

>
>> In other words, what happens to the child when the parent stops providing power to it,
>> under these circumstances?
>>
>> I think the general assumption for these cases, is that it wouldn't
>> play well, which is why the child device is prevented to enter runtime
>> suspend, in case it needs in-band power as to deliver remote-wakeups.
>
> That is how it may be implemented, but it doesn't have to be done this way.
>
> Again, please don't confuse a requirement with a strategy to meet it.
>
>>>> The point is, for remote wakeups, genpd and its users don't need the
>>>> PM_QOS_FLAG_NO_POWER_OFF flag, or anything else.
>>>
>>> Yes, they do or at least they may, which is why it is already supported in
>>> there.
>>
>> I did a quick investigation. The PM_QOS_FLAG_NO_POWER_OFF flag, added
>> in 2012, only have two drivers that use it today. These are:
>>
>> 1) drivers/pci/pci-acpi.c.
>> I assume this isn't a driver for devices that can be attached to genpd.
>>
>> 2) drivers/usb/core/port.c
>> This registers an internal struct device for the usb port, which it
>> deploys runtime PM support for and sets the PM_QOS_FLAG_NO_POWER_OFF
>> flag to. The internal created device can't be attached to genpd,
>> however it gets the usb hub device assigned to its parent, which is an
>> interesting part, as the PM_QOS_FLAG_NO_POWER_OFF doesn't become
>> propagated to it.
>>
>> This made me realize that the code in genpd dealing with the
>> PM_QOS_FLAG_NO_POWER_OFF flag, is unused, unless I am missing another
>> use case, of course.
>
> That's probably the case.
>
> It was added in anticipation of future use in drivers.

Okay, I see.

>
>> Another observation regarding the PM_QOS_FLAG_NO_POWER_OFF flag, is
>> that it's QOS thing, which may also be controlled from userspace via
>> sysfs. Using a QOS flag to prevent to turn of in-band power as to be
>> able to support wakeups correctly, just doesn't seems right to me. I
>> rather consider this as a HW configuration that must be set to support
>> a requested wakeup.
>
> User space may want in-band power to be provided to the device while
> suspended too and I'm not sure what's the difference between that and
> the wakeup requirement.

I am not sure I understand, why user space need to care about what is
powered and not powered?

User space have wakeups to control and can set QoS latency
constraints, isn't that sufficient in this regards?

>
> The point here, among other things, is that it doesn't really matter
> *why* the device needs to be provided with in-band power while
> suspended.  Potentially, there may be multiple valid reasons for that
> and wakeup is only one of them, so why does it need to be special?

Sure, it doesn't needs to be special as long as it can cope with all
different scenarios.

>
>> Going back to what I think is needed. I think we need a per device PM
>> flag that the driver can set, to instruct upper layers about how it
>> has configured the wakeup (in-band or out-band). That flag needs to be
>> propagated to parents as well. Moreover, I am questioning whether such
>> flag may be useful for runtime PM, maybe it could.
>
> Upper layers need not be informed about wakeup in general, like genpd
> which only understands (a) whether or not to cut off in-band power and
> (b) whether or not to invoke the "stop" callback.  It doesn't need to
> care about why those two things may or may not be done.

That's true.

Still I am a bit worried about not doing the propagation, because we
then need to walk the hierarchy of children instead of just checking a
per device flag that has been propagated already.

>
> My first conclusion from the above is that the runtime PM case and the
> system-wide PM case are really the same from the wakeup setup
> perspective and I don't see general reasons for them to be handled
> differently.
>
> The way to communicate the requirements related to that to upper
> layers, parent drivers etc may be via PM QoS flags, but I'm not
> insisting on that.  It has just occurred to me that it is one possible
> way to do it and there was some work done in that direction.

Okay, I see.

Kind regards
Uffe

Re: [RFD] Devices that need full power for wakeup

[Ulf Hansson]

[...]

>>
>> Yes, it can enter the "suspended" metastate, but that metastate may be
>> mapped to multiple physical states.  Which of them is actually used
>> may depend on the power requirements of the children.
>
> Again, you are right.
>
> However, what you are describing is to me a different (but related)
> and more complex problem. As you say, this is about how to support
> *multiple* device idle states.
>
> My point is, trying to make use of PM_QOS_FLAG_NO_POWER_OFF, would in
> principle enable support for *one* more device idle state. To me, that
> doesn't sounds like a future proof solution. Don't we need something
> more flexible?

[...]

>>> Another observation regarding the PM_QOS_FLAG_NO_POWER_OFF flag, is
>>> that it's QOS thing, which may also be controlled from userspace via
>>> sysfs. Using a QOS flag to prevent to turn of in-band power as to be
>>> able to support wakeups correctly, just doesn't seems right to me. I
>>> rather consider this as a HW configuration that must be set to support
>>> a requested wakeup.
>>
>> User space may want in-band power to be provided to the device while
>> suspended too and I'm not sure what's the difference between that and
>> the wakeup requirement.
>
> I am not sure I understand, why user space need to care about what is
> powered and not powered?
>
> User space have wakeups to control and can set QoS latency
> constraints, isn't that sufficient in this regards?

Moreover, if we consider the runtime PM case, userspace can also force
a device to remain runtime resumed.

It sounds to me that userspace don't need yet another interface,
allowing it to control the in-band power for a device. Yeah, it's
already there for usb ports, but perhaps we can get rid of that!?

>
>>
>> The point here, among other things, is that it doesn't really matter
>> *why* the device needs to be provided with in-band power while
>> suspended.  Potentially, there may be multiple valid reasons for that
>> and wakeup is only one of them, so why does it need to be special?
>
> Sure, it doesn't needs to be special as long as it can cope with all
> different scenarios.

I have started to think a little bit more about how to support the
"multiple device idle state" thingy and I have started to prepare and
RFC. In fact it seems like quite limited amount of code, at least
initially.

In regards to that, it sure seems like it would be possible to find a
common way (both for system suspend and runtime suspend) to cope with
the out-band vs in-band wakeup problem.

In principle what I am thinking around that, is that we need way to
disable/enable the deepest supported idle state for a device, as that
seems to match turning off the in-band power for the device.

>
>>
>>> Going back to what I think is needed. I think we need a per device PM
>>> flag that the driver can set, to instruct upper layers about how it
>>> has configured the wakeup (in-band or out-band). That flag needs to be
>>> propagated to parents as well. Moreover, I am questioning whether such
>>> flag may be useful for runtime PM, maybe it could.
>>
>> Upper layers need not be informed about wakeup in general, like genpd
>> which only understands (a) whether or not to cut off in-band power and
>> (b) whether or not to invoke the "stop" callback.  It doesn't need to
>> care about why those two things may or may not be done.
>
> That's true.
>
> Still I am a bit worried about not doing the propagation, because we
> then need to walk the hierarchy of children instead of just checking a
> per device flag that has been propagated already.

Alright, even if I am still a bit concerned about this, let's start
simple and leave the parent driver to check the children.

>
>>
>> My first conclusion from the above is that the runtime PM case and the
>> system-wide PM case are really the same from the wakeup setup
>> perspective and I don't see general reasons for them to be handled
>> differently.
>>
>> The way to communicate the requirements related to that to upper
>> layers, parent drivers etc may be via PM QoS flags, but I'm not
>> insisting on that.  It has just occurred to me that it is one possible
>> way to do it and there was some work done in that direction.
>
> Okay, I see.

Unless you have some additional thoughts, I plan to continue to cook
the RFC for supporting multiple device idle states. When posted, we
can continue the discussion around that instead.

Do it sounds reasonable to you?

Kind regards
Uffe

Re: [RFD] Devices that need full power for wakeup

[Rafael J. Wysocki]

On Wed, Jan 24, 2018 at 12:40 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> [...]
>
>>>
>>> Yes, it can enter the "suspended" metastate, but that metastate may be
>>> mapped to multiple physical states.  Which of them is actually used
>>> may depend on the power requirements of the children.
>>
>> Again, you are right.
>>
>> However, what you are describing is to me a different (but related)
>> and more complex problem. As you say, this is about how to support
>> *multiple* device idle states.
>>
>> My point is, trying to make use of PM_QOS_FLAG_NO_POWER_OFF, would in
>> principle enable support for *one* more device idle state. To me, that
>> doesn't sounds like a future proof solution. Don't we need something
>> more flexible?
>
> [...]
>
>>>> Another observation regarding the PM_QOS_FLAG_NO_POWER_OFF flag, is
>>>> that it's QOS thing, which may also be controlled from userspace via
>>>> sysfs. Using a QOS flag to prevent to turn of in-band power as to be
>>>> able to support wakeups correctly, just doesn't seems right to me. I
>>>> rather consider this as a HW configuration that must be set to support
>>>> a requested wakeup.
>>>
>>> User space may want in-band power to be provided to the device while
>>> suspended too and I'm not sure what's the difference between that and
>>> the wakeup requirement.
>>
>> I am not sure I understand, why user space need to care about what is
>> powered and not powered?
>>
>> User space have wakeups to control and can set QoS latency
>> constraints, isn't that sufficient in this regards?
>
> Moreover, if we consider the runtime PM case, userspace can also force
> a device to remain runtime resumed.
>
> It sounds to me that userspace don't need yet another interface,
> allowing it to control the in-band power for a device. Yeah, it's
> already there for usb ports, but perhaps we can get rid of that!?

Well, what about devices with multiple power states where the deepest
one only is power-off and the in-band power is there in the remaining
states?

In those cases user space may very well still want the device to be
suspended when not in use, but not in the deepest (power-off) state.

>>> The point here, among other things, is that it doesn't really matter
>>> *why* the device needs to be provided with in-band power while
>>> suspended.  Potentially, there may be multiple valid reasons for that
>>> and wakeup is only one of them, so why does it need to be special?
>>
>> Sure, it doesn't needs to be special as long as it can cope with all
>> different scenarios.
>
> I have started to think a little bit more about how to support the
> "multiple device idle state" thingy and I have started to prepare and
> RFC. In fact it seems like quite limited amount of code, at least
> initially.
>
> In regards to that, it sure seems like it would be possible to find a
> common way (both for system suspend and runtime suspend) to cope with
> the out-band vs in-band wakeup problem.
>
> In principle what I am thinking around that, is that we need way to
> disable/enable the deepest supported idle state for a device, as that
> seems to match turning off the in-band power for the device.

That sounds reasonable.

We do some of that for PCI devices and in ACPI PM already.

>>
>>>
>>>> Going back to what I think is needed. I think we need a per device PM
>>>> flag that the driver can set, to instruct upper layers about how it
>>>> has configured the wakeup (in-band or out-band). That flag needs to be
>>>> propagated to parents as well. Moreover, I am questioning whether such
>>>> flag may be useful for runtime PM, maybe it could.
>>>
>>> Upper layers need not be informed about wakeup in general, like genpd
>>> which only understands (a) whether or not to cut off in-band power and
>>> (b) whether or not to invoke the "stop" callback.  It doesn't need to
>>> care about why those two things may or may not be done.
>>
>> That's true.
>>
>> Still I am a bit worried about not doing the propagation, because we
>> then need to walk the hierarchy of children instead of just checking a
>> per device flag that has been propagated already.
>
> Alright, even if I am still a bit concerned about this, let's start
> simple and leave the parent driver to check the children.

Cool. :-)

>>>
>>> My first conclusion from the above is that the runtime PM case and the
>>> system-wide PM case are really the same from the wakeup setup
>>> perspective and I don't see general reasons for them to be handled
>>> differently.
>>>
>>> The way to communicate the requirements related to that to upper
>>> layers, parent drivers etc may be via PM QoS flags, but I'm not
>>> insisting on that.  It has just occurred to me that it is one possible
>>> way to do it and there was some work done in that direction.
>>
>> Okay, I see.
>
> Unless you have some additional thoughts, I plan to continue to cook
> the RFC for supporting multiple device idle states. When posted, we
> can continue the discussion around that instead.
>
> Do it sounds reasonable to you?

Yes, it does, looking forward to seeing the patches. :-)

[Sorry for the delayed responses, I've been in a totally different
time zone lately.]

Thanks,
Rafael