All of lore.kernel.org
 help / color / mirror / Atom feed
* POWER_DOMAIN_ATTRIBUTES in SCMI
@ 2025-02-14  9:20 Peng Fan
  2025-02-18 10:41 ` Sudeep Holla
  0 siblings, 1 reply; 11+ messages in thread
From: Peng Fan @ 2025-02-14  9:20 UTC (permalink / raw)
  To: Sudeep Holla, cristian.marussi@arm.com,
	souvik.chakravarty@arm.com, Ulf Hansson, Dan Carpenter
  Cc: arm-scmi@vger.kernel.org, Chuck Cannon

All,

Previously I posted a patch to linux to set all power domains
with "GENPD_FLAG_ACTIVE_WAKEUP" set in
drivers/pmdomain/arm/scmi_pm_domain.c

But in the end we find that some power domains
could be in off state while it still could wakeup
the system. And we have a downstream patch
+               if (!strcmp(scmi_pd->name, "hsio_top"))
+                       scmi_pd->genpd.flags = 0;

So I am wondering to extend the attributes of SCMI spec,
Saying In SCMI spec(DEN0056E)
4.3.2.5 POWER_DOMAIN_ATTRIBUTES
Page 44 of 210:

Bit[26]: If set to 1, the power domain could
wakeup the system with power state
set as off.

This is just an idea in my mind, I not
write code to verify, just wanna to see
any comments from your side.

Thanks,
Peng.


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: POWER_DOMAIN_ATTRIBUTES in SCMI
  2025-02-14  9:20 POWER_DOMAIN_ATTRIBUTES in SCMI Peng Fan
@ 2025-02-18 10:41 ` Sudeep Holla
  2025-02-18 13:08   ` Peng Fan
  0 siblings, 1 reply; 11+ messages in thread
From: Sudeep Holla @ 2025-02-18 10:41 UTC (permalink / raw)
  To: Peng Fan
  Cc: cristian.marussi@arm.com, souvik.chakravarty@arm.com,
	Sudeep Holla, Ulf Hansson, Dan Carpenter,
	arm-scmi@vger.kernel.org, Chuck Cannon

On Fri, Feb 14, 2025 at 09:20:27AM +0000, Peng Fan wrote:
> All,
>
> Previously I posted a patch to linux to set all power domains
> with "GENPD_FLAG_ACTIVE_WAKEUP" set in
> drivers/pmdomain/arm/scmi_pm_domain.c

Yes, I remember ACK-ing that and now I am thinking why 😄.
The description of the flag GENPD_FLAG_ACTIVE_WAKEUP says:
"Instructs genpd to keep the PM domain powered on, in case any of its
attached devices is used in the wakeup path to serve system wakeups."

Does that mean all the SCMI power domains remain powered on with this
flag ? If so, that sounds wrong to me. I hope it is not the case and it
is effective only if the device attached is wakeup source.

> But in the end we find that some power domains
> could be in off state while it still could wakeup
> the system. And we have a downstream patch
> +               if (!strcmp(scmi_pd->name, "hsio_top"))
> +                       scmi_pd->genpd.flags = 0;
>

There you go, so you simply rushed some solution upstream to carry less
patches downstream but this time you got bitten again. Sorry, but I am
seeing a pattern from you here and I don't really like that.

> So I am wondering to extend the attributes of SCMI spec,
> Saying In SCMI spec(DEN0056E)
> 4.3.2.5 POWER_DOMAIN_ATTRIBUTES
> Page 44 of 210:
> 
> Bit[26]: If set to 1, the power domain could
> wakeup the system with power state
> set as off.
>

This is not what the above flag is all about. It just instructs genpd
to keep the power domain ON as the device attached to it could be a wakeup
source. They are not one and the same. If the device is marked wakeup in
the DT and it is both wakeup capable and is enabled, it shouldn't request
the power domain to be powered off when suspending. Why is that not
sufficient here ? What am I missing ? I am interested in knowing it as
it is important to fix the issue you are trying to address here.

> This is just an idea in my mind, I not
> write code to verify, just wanna to see
> any comments from your side.
> 

I wonder if we need to revert GENPD_FLAG_ACTIVE_WAKEUP flag enabling on
all SCMI power domains if that is not solving the problem you thought it
would.

-- 
Regards,
Sudeep

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: POWER_DOMAIN_ATTRIBUTES in SCMI
  2025-02-18 10:41 ` Sudeep Holla
@ 2025-02-18 13:08   ` Peng Fan
  2025-02-18 13:57     ` Vincent Guittot
  0 siblings, 1 reply; 11+ messages in thread
From: Peng Fan @ 2025-02-18 13:08 UTC (permalink / raw)
  To: Sudeep Holla
  Cc: Peng Fan, cristian.marussi@arm.com, souvik.chakravarty@arm.com,
	Ulf Hansson, Dan Carpenter, arm-scmi@vger.kernel.org,
	Chuck Cannon

On Tue, Feb 18, 2025 at 10:41:15AM +0000, Sudeep Holla wrote:
>On Fri, Feb 14, 2025 at 09:20:27AM +0000, Peng Fan wrote:
>> All,
>>
>> Previously I posted a patch to linux to set all power domains
>> with "GENPD_FLAG_ACTIVE_WAKEUP" set in
>> drivers/pmdomain/arm/scmi_pm_domain.c
>
>Yes, I remember ACK-ing that and now I am thinking why 😄.
>The description of the flag GENPD_FLAG_ACTIVE_WAKEUP says:
>"Instructs genpd to keep the PM domain powered on, in case any of its
>attached devices is used in the wakeup path to serve system wakeups."
>
>Does that mean all the SCMI power domains remain powered on with this
>flag ? If so, that sounds wrong to me. I hope it is not the case and it
>is effective only if the device attached is wakeup source.

Only when the device is setup wakeup source, the genpd framework
will take this flag as keeping power domain on.

Without this flag, even if the device setup as wakeup source, the
device's power domain will still be powered off.

>
>> But in the end we find that some power domains
>> could be in off state while it still could wakeup
>> the system. And we have a downstream patch
>> +               if (!strcmp(scmi_pd->name, "hsio_top"))
>> +                       scmi_pd->genpd.flags = 0;
>>
>
>There you go, so you simply rushed some solution upstream to carry less
>patches downstream but this time you got bitten again. Sorry, but I am
>seeing a pattern from you here and I don't really like that.

I not wanna to argue here.
I DO care reputation. If I do something wrong, I will improve.

>
>> So I am wondering to extend the attributes of SCMI spec,
>> Saying In SCMI spec(DEN0056E)
>> 4.3.2.5 POWER_DOMAIN_ATTRIBUTES
>> Page 44 of 210:
>> 
>> Bit[26]: If set to 1, the power domain could
>> wakeup the system with power state
>> set as off.
>>
>
>This is not what the above flag is all about. It just instructs genpd
>to keep the power domain ON as the device attached to it could be a wakeup
>source. They are not one and the same. If the device is marked wakeup in
>the DT and it is both wakeup capable and is enabled, it shouldn't request
>the power domain to be powered off when suspending. Why is that not
>sufficient here ? What am I missing ? I am interested in knowing it as
>it is important to fix the issue you are trying to address here.

The flag must be set if the power domain needs to keep power on to have
the wakeup capability.

But in some case, a power domain no need to keep power on and it still
have wakeup capability from hardware perspective.

For example usb phy in i.MX95, its power domain is off, but it could still
wakeup the SoC. But with the GENPD_FLAG_ACTIVE_WAKEUP set, the power domain
will not be powered off, so more power consumption in suspended state.

So I am requesting extending spec to give agents the information whether
the power domain could wakeup HW system in powered off state.

>
>> This is just an idea in my mind, I not
>> write code to verify, just wanna to see
>> any comments from your side.
>> 
>
>I wonder if we need to revert GENPD_FLAG_ACTIVE_WAKEUP flag enabling on
>all SCMI power domains if that is not solving the problem you thought it
>would.

Revert the flag will cause a power domain not able to wakeup SoC if the
power domain needs power state on to have wakeup capability.

I am not sure other vendor's SoC design. To i.MX95, without this flag set,
the NETC will lose wakeup on LAN capability.

Regards,
Peng.

>
>-- 
>Regards,
>Sudeep
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: POWER_DOMAIN_ATTRIBUTES in SCMI
  2025-02-18 13:08   ` Peng Fan
@ 2025-02-18 13:57     ` Vincent Guittot
  2025-02-18 15:02       ` Ulf Hansson
  2025-02-18 16:20       ` Peng Fan
  0 siblings, 2 replies; 11+ messages in thread
From: Vincent Guittot @ 2025-02-18 13:57 UTC (permalink / raw)
  To: Peng Fan
  Cc: Sudeep Holla, Peng Fan, cristian.marussi@arm.com,
	souvik.chakravarty@arm.com, Ulf Hansson, Dan Carpenter,
	arm-scmi@vger.kernel.org, Chuck Cannon

On Tue, 18 Feb 2025 at 13:03, Peng Fan <peng.fan@oss.nxp.com> wrote:
>
> On Tue, Feb 18, 2025 at 10:41:15AM +0000, Sudeep Holla wrote:
> >On Fri, Feb 14, 2025 at 09:20:27AM +0000, Peng Fan wrote:
> >> All,
> >>
> >> Previously I posted a patch to linux to set all power domains
> >> with "GENPD_FLAG_ACTIVE_WAKEUP" set in
> >> drivers/pmdomain/arm/scmi_pm_domain.c
> >
> >Yes, I remember ACK-ing that and now I am thinking why 😄.
> >The description of the flag GENPD_FLAG_ACTIVE_WAKEUP says:
> >"Instructs genpd to keep the PM domain powered on, in case any of its
> >attached devices is used in the wakeup path to serve system wakeups."
> >
> >Does that mean all the SCMI power domains remain powered on with this
> >flag ? If so, that sounds wrong to me. I hope it is not the case and it
> >is effective only if the device attached is wakeup source.
>
> Only when the device is setup wakeup source, the genpd framework
> will take this flag as keeping power domain on.
>
> Without this flag, even if the device setup as wakeup source, the
> device's power domain will still be powered off.
>
> >
> >> But in the end we find that some power domains
> >> could be in off state while it still could wakeup
> >> the system. And we have a downstream patch
> >> +               if (!strcmp(scmi_pd->name, "hsio_top"))
> >> +                       scmi_pd->genpd.flags = 0;
> >>
> >
> >There you go, so you simply rushed some solution upstream to carry less
> >patches downstream but this time you got bitten again. Sorry, but I am
> >seeing a pattern from you here and I don't really like that.
>
> I not wanna to argue here.
> I DO care reputation. If I do something wrong, I will improve.
>
> >
> >> So I am wondering to extend the attributes of SCMI spec,
> >> Saying In SCMI spec(DEN0056E)
> >> 4.3.2.5 POWER_DOMAIN_ATTRIBUTES
> >> Page 44 of 210:
> >>
> >> Bit[26]: If set to 1, the power domain could
> >> wakeup the system with power state
> >> set as off.
> >>
> >
> >This is not what the above flag is all about. It just instructs genpd
> >to keep the power domain ON as the device attached to it could be a wakeup
> >source. They are not one and the same. If the device is marked wakeup in
> >the DT and it is both wakeup capable and is enabled, it shouldn't request
> >the power domain to be powered off when suspending. Why is that not
> >sufficient here ? What am I missing ? I am interested in knowing it as
> >it is important to fix the issue you are trying to address here.
>
> The flag must be set if the power domain needs to keep power on to have
> the wakeup capability.
>
> But in some case, a power domain no need to keep power on and it still
> have wakeup capability from hardware perspective.
>
> For example usb phy in i.MX95, its power domain is off, but it could still
> wakeup the SoC. But with the GENPD_FLAG_ACTIVE_WAKEUP set, the power domain
> will not be powered off, so more power consumption in suspended state.

This probably means that the wakeup mechanism is not in the same power
domain but in another one that is kept on while the device is
suspended and the main power domain is off. Do you have more details
about this wakeup source that works with powerdomain being off ?
I vaguely remember some old discussions about inband/outband wakeup
source settings for devices which enable drivers to specify if the
wakeup source is in or out of the power domain and if we should keep
it on or not. But I can't remember the details; Ulf should have more
details

>
> So I am requesting extending spec to give agents the information whether
> the power domain could wakeup HW system in powered off state.

I tend to agree with Sudeep that it doesn't make sense to say that a
power domain can be off but the powered devices can still wakeup the
system. It usually means that the wakeup is not powered by the main
power domain

>
> >
> >> This is just an idea in my mind, I not
> >> write code to verify, just wanna to see
> >> any comments from your side.
> >>
> >
> >I wonder if we need to revert GENPD_FLAG_ACTIVE_WAKEUP flag enabling on
> >all SCMI power domains if that is not solving the problem you thought it
> >would.
>
> Revert the flag will cause a power domain not able to wakeup SoC if the
> power domain needs power state on to have wakeup capability.
>
> I am not sure other vendor's SoC design. To i.MX95, without this flag set,
> the NETC will lose wakeup on LAN capability.
>
> Regards,
> Peng.
>
> >
> >--
> >Regards,
> >Sudeep
> >
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: POWER_DOMAIN_ATTRIBUTES in SCMI
  2025-02-18 13:57     ` Vincent Guittot
@ 2025-02-18 15:02       ` Ulf Hansson
  2025-02-18 15:26         ` Sudeep Holla
  2025-02-18 16:31         ` Peng Fan
  2025-02-18 16:20       ` Peng Fan
  1 sibling, 2 replies; 11+ messages in thread
From: Ulf Hansson @ 2025-02-18 15:02 UTC (permalink / raw)
  To: Vincent Guittot
  Cc: Peng Fan, Sudeep Holla, Peng Fan, cristian.marussi@arm.com,
	souvik.chakravarty@arm.com, Dan Carpenter,
	arm-scmi@vger.kernel.org, Chuck Cannon

On Tue, 18 Feb 2025 at 14:57, Vincent Guittot
<vincent.guittot@linaro.org> wrote:
>
> On Tue, 18 Feb 2025 at 13:03, Peng Fan <peng.fan@oss.nxp.com> wrote:
> >
> > On Tue, Feb 18, 2025 at 10:41:15AM +0000, Sudeep Holla wrote:
> > >On Fri, Feb 14, 2025 at 09:20:27AM +0000, Peng Fan wrote:
> > >> All,
> > >>
> > >> Previously I posted a patch to linux to set all power domains
> > >> with "GENPD_FLAG_ACTIVE_WAKEUP" set in
> > >> drivers/pmdomain/arm/scmi_pm_domain.c
> > >
> > >Yes, I remember ACK-ing that and now I am thinking why 😄.
> > >The description of the flag GENPD_FLAG_ACTIVE_WAKEUP says:
> > >"Instructs genpd to keep the PM domain powered on, in case any of its
> > >attached devices is used in the wakeup path to serve system wakeups."
> > >
> > >Does that mean all the SCMI power domains remain powered on with this
> > >flag ? If so, that sounds wrong to me. I hope it is not the case and it
> > >is effective only if the device attached is wakeup source.
> >
> > Only when the device is setup wakeup source, the genpd framework
> > will take this flag as keeping power domain on.
> >
> > Without this flag, even if the device setup as wakeup source, the
> > device's power domain will still be powered off.
> >
> > >
> > >> But in the end we find that some power domains
> > >> could be in off state while it still could wakeup
> > >> the system. And we have a downstream patch
> > >> +               if (!strcmp(scmi_pd->name, "hsio_top"))
> > >> +                       scmi_pd->genpd.flags = 0;
> > >>
> > >
> > >There you go, so you simply rushed some solution upstream to carry less
> > >patches downstream but this time you got bitten again. Sorry, but I am
> > >seeing a pattern from you here and I don't really like that.
> >
> > I not wanna to argue here.
> > I DO care reputation. If I do something wrong, I will improve.
> >
> > >
> > >> So I am wondering to extend the attributes of SCMI spec,
> > >> Saying In SCMI spec(DEN0056E)
> > >> 4.3.2.5 POWER_DOMAIN_ATTRIBUTES
> > >> Page 44 of 210:
> > >>
> > >> Bit[26]: If set to 1, the power domain could
> > >> wakeup the system with power state
> > >> set as off.
> > >>
> > >
> > >This is not what the above flag is all about. It just instructs genpd
> > >to keep the power domain ON as the device attached to it could be a wakeup
> > >source. They are not one and the same. If the device is marked wakeup in
> > >the DT and it is both wakeup capable and is enabled, it shouldn't request
> > >the power domain to be powered off when suspending. Why is that not
> > >sufficient here ? What am I missing ? I am interested in knowing it as
> > >it is important to fix the issue you are trying to address here.
> >
> > The flag must be set if the power domain needs to keep power on to have
> > the wakeup capability.
> >
> > But in some case, a power domain no need to keep power on and it still
> > have wakeup capability from hardware perspective.
> >
> > For example usb phy in i.MX95, its power domain is off, but it could still
> > wakeup the SoC. But with the GENPD_FLAG_ACTIVE_WAKEUP set, the power domain
> > will not be powered off, so more power consumption in suspended state.
>
> This probably means that the wakeup mechanism is not in the same power
> domain but in another one that is kept on while the device is
> suspended and the main power domain is off. Do you have more details
> about this wakeup source that works with powerdomain being off ?
> I vaguely remember some old discussions about inband/outband wakeup
> source settings for devices which enable drivers to specify if the
> wakeup source is in or out of the power domain and if we should keep
> it on or not. But I can't remember the details; Ulf should have more
> details

Vincent, you have a great memory! :-)

Indeed I tried to post a series (I can dig it up, if you want?) that
allowed the driver to instruct upper layers, such as buses and PM
domains whether the wakeup is managed in-band or out-band.

An example could be an SDIO irq, being re-routed from a regular DATA
line that is managed by the SDIO/MMC controller to a GPIO pin during
system suspend. Another similar use-case with a potential similar
wake-up configuration, is waking up from a UART console.

My point is, the SDIO/MMC controller could be configured as a wakeup
source, while it's actually the GPIO pin that is managing the wakeup.
In this case, typically the GPIO irq can be managed from an always-on
logic/PMIC, which means there is no reason to keep the PM domain
powered-on for the SDIO/MMC controller during system suspend.

At the moment we don't have any way to express this kind of
configuration, I think.

>
> >
> > So I am requesting extending spec to give agents the information whether
> > the power domain could wakeup HW system in powered off state.
>
> I tend to agree with Sudeep that it doesn't make sense to say that a
> power domain can be off but the powered devices can still wakeup the
> system. It usually means that the wakeup is not powered by the main
> power domain

I tend to agree. In general, I think we should always use
GENPD_FLAG_ACTIVE_WAKEUP, but we are lacking a way to inform genpd
(and other upper layers) that the wakeup-irq is managed out-band,
which means genpd can still be powered-off.

>
> >
> > >
> > >> This is just an idea in my mind, I not
> > >> write code to verify, just wanna to see
> > >> any comments from your side.
> > >>
> > >
> > >I wonder if we need to revert GENPD_FLAG_ACTIVE_WAKEUP flag enabling on
> > >all SCMI power domains if that is not solving the problem you thought it
> > >would.
> >
> > Revert the flag will cause a power domain not able to wakeup SoC if the
> > power domain needs power state on to have wakeup capability.
> >
> > I am not sure other vendor's SoC design. To i.MX95, without this flag set,
> > the NETC will lose wakeup on LAN capability.
> >
> > Regards,
> > Peng.
> >
> > >
> > >--
> > >Regards,
> > >Sudeep
> > >
> >

Kind regards
Uffe

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: POWER_DOMAIN_ATTRIBUTES in SCMI
  2025-02-18 15:02       ` Ulf Hansson
@ 2025-02-18 15:26         ` Sudeep Holla
  2025-02-18 16:31         ` Peng Fan
  1 sibling, 0 replies; 11+ messages in thread
From: Sudeep Holla @ 2025-02-18 15:26 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Vincent Guittot, Sudeep Holla, Peng Fan, Peng Fan,
	cristian.marussi@arm.com, souvik.chakravarty@arm.com,
	Dan Carpenter, arm-scmi@vger.kernel.org, Chuck Cannon

On Tue, Feb 18, 2025 at 04:02:58PM +0100, Ulf Hansson wrote:
> On Tue, 18 Feb 2025 at 14:57, Vincent Guittot
> <vincent.guittot@linaro.org> wrote:
> >
> > On Tue, 18 Feb 2025 at 13:03, Peng Fan <peng.fan@oss.nxp.com> wrote:
> > >
> > > On Tue, Feb 18, 2025 at 10:41:15AM +0000, Sudeep Holla wrote:
> > > >On Fri, Feb 14, 2025 at 09:20:27AM +0000, Peng Fan wrote:
> > > >> All,
> > > >>
> > > >> Previously I posted a patch to linux to set all power domains
> > > >> with "GENPD_FLAG_ACTIVE_WAKEUP" set in
> > > >> drivers/pmdomain/arm/scmi_pm_domain.c
> > > >
> > > >Yes, I remember ACK-ing that and now I am thinking why 😄.
> > > >The description of the flag GENPD_FLAG_ACTIVE_WAKEUP says:
> > > >"Instructs genpd to keep the PM domain powered on, in case any of its
> > > >attached devices is used in the wakeup path to serve system wakeups."
> > > >
> > > >Does that mean all the SCMI power domains remain powered on with this
> > > >flag ? If so, that sounds wrong to me. I hope it is not the case and it
> > > >is effective only if the device attached is wakeup source.
> > >
> > > Only when the device is setup wakeup source, the genpd framework
> > > will take this flag as keeping power domain on.
> > >
> > > Without this flag, even if the device setup as wakeup source, the
> > > device's power domain will still be powered off.
> > >
> > > >
> > > >> But in the end we find that some power domains
> > > >> could be in off state while it still could wakeup
> > > >> the system. And we have a downstream patch
> > > >> +               if (!strcmp(scmi_pd->name, "hsio_top"))
> > > >> +                       scmi_pd->genpd.flags = 0;
> > > >>
> > > >
> > > >There you go, so you simply rushed some solution upstream to carry less
> > > >patches downstream but this time you got bitten again. Sorry, but I am
> > > >seeing a pattern from you here and I don't really like that.
> > >
> > > I not wanna to argue here.
> > > I DO care reputation. If I do something wrong, I will improve.
> > >
> > > >
> > > >> So I am wondering to extend the attributes of SCMI spec,
> > > >> Saying In SCMI spec(DEN0056E)
> > > >> 4.3.2.5 POWER_DOMAIN_ATTRIBUTES
> > > >> Page 44 of 210:
> > > >>
> > > >> Bit[26]: If set to 1, the power domain could
> > > >> wakeup the system with power state
> > > >> set as off.
> > > >>
> > > >
> > > >This is not what the above flag is all about. It just instructs genpd
> > > >to keep the power domain ON as the device attached to it could be a wakeup
> > > >source. They are not one and the same. If the device is marked wakeup in
> > > >the DT and it is both wakeup capable and is enabled, it shouldn't request
> > > >the power domain to be powered off when suspending. Why is that not
> > > >sufficient here ? What am I missing ? I am interested in knowing it as
> > > >it is important to fix the issue you are trying to address here.
> > >
> > > The flag must be set if the power domain needs to keep power on to have
> > > the wakeup capability.
> > >
> > > But in some case, a power domain no need to keep power on and it still
> > > have wakeup capability from hardware perspective.
> > >
> > > For example usb phy in i.MX95, its power domain is off, but it could still
> > > wakeup the SoC. But with the GENPD_FLAG_ACTIVE_WAKEUP set, the power domain
> > > will not be powered off, so more power consumption in suspended state.
> >
> > This probably means that the wakeup mechanism is not in the same power
> > domain but in another one that is kept on while the device is
> > suspended and the main power domain is off. Do you have more details
> > about this wakeup source that works with powerdomain being off ?
> > I vaguely remember some old discussions about inband/outband wakeup
> > source settings for devices which enable drivers to specify if the
> > wakeup source is in or out of the power domain and if we should keep
> > it on or not. But I can't remember the details; Ulf should have more
> > details

Thanks Vincent for starting this discussion, much appreciated!

>
> Vincent, you have a great memory! :-)
>
> Indeed I tried to post a series (I can dig it up, if you want?) that
> allowed the driver to instruct upper layers, such as buses and PM
> domains whether the wakeup is managed in-band or out-band.
>

Right, this distinction must be clarified in i.MX case as I don't follow
their use case yet. Peng ?

> An example could be an SDIO irq, being re-routed from a regular DATA
> line that is managed by the SDIO/MMC controller to a GPIO pin during
> system suspend. Another similar use-case with a potential similar
> wake-up configuration, is waking up from a UART console.
>
> My point is, the SDIO/MMC controller could be configured as a wakeup
> source, while it's actually the GPIO pin that is managing the wakeup.
> In this case, typically the GPIO irq can be managed from an always-on
> logic/PMIC, which means there is no reason to keep the PM domain
> powered-on for the SDIO/MMC controller during system suspend.
>

Good explanation and example. Thanks again.

> At the moment we don't have any way to express this kind of
> configuration, I think.
>

Understood.

> >
> > >
> > > So I am requesting extending spec to give agents the information whether
> > > the power domain could wakeup HW system in powered off state.
> >
> > I tend to agree with Sudeep that it doesn't make sense to say that a
> > power domain can be off but the powered devices can still wakeup the
> > system. It usually means that the wakeup is not powered by the main
> > power domain
>
> I tend to agree. In general, I think we should always use
> GENPD_FLAG_ACTIVE_WAKEUP, but we are lacking a way to inform genpd
> (and other upper layers) that the wakeup-irq is managed out-band,
> which means genpd can still be powered-off.

Thanks Ulf, that's reassuring that SCMI is misusing or using incorrectly
some core genpd feature.

--
Regards,
Sudeep

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: POWER_DOMAIN_ATTRIBUTES in SCMI
  2025-02-18 13:57     ` Vincent Guittot
  2025-02-18 15:02       ` Ulf Hansson
@ 2025-02-18 16:20       ` Peng Fan
  1 sibling, 0 replies; 11+ messages in thread
From: Peng Fan @ 2025-02-18 16:20 UTC (permalink / raw)
  To: Vincent Guittot
  Cc: Sudeep Holla, Peng Fan, cristian.marussi@arm.com,
	souvik.chakravarty@arm.com, Ulf Hansson, Dan Carpenter,
	arm-scmi@vger.kernel.org, Chuck Cannon

On Tue, Feb 18, 2025 at 02:57:10PM +0100, Vincent Guittot wrote:
>On Tue, 18 Feb 2025 at 13:03, Peng Fan <peng.fan@oss.nxp.com> wrote:
>>
>> On Tue, Feb 18, 2025 at 10:41:15AM +0000, Sudeep Holla wrote:
>> >On Fri, Feb 14, 2025 at 09:20:27AM +0000, Peng Fan wrote:
>> >> All,
>> >>
>> >> Previously I posted a patch to linux to set all power domains
>> >> with "GENPD_FLAG_ACTIVE_WAKEUP" set in
>> >> drivers/pmdomain/arm/scmi_pm_domain.c
>> >
>> >Yes, I remember ACK-ing that and now I am thinking why 😄.
>> >The description of the flag GENPD_FLAG_ACTIVE_WAKEUP says:
>> >"Instructs genpd to keep the PM domain powered on, in case any of its
>> >attached devices is used in the wakeup path to serve system wakeups."
>> >
>> >Does that mean all the SCMI power domains remain powered on with this
>> >flag ? If so, that sounds wrong to me. I hope it is not the case and it
>> >is effective only if the device attached is wakeup source.
>>
>> Only when the device is setup wakeup source, the genpd framework
>> will take this flag as keeping power domain on.
>>
>> Without this flag, even if the device setup as wakeup source, the
>> device's power domain will still be powered off.
>>
>> >
>> >> But in the end we find that some power domains
>> >> could be in off state while it still could wakeup
>> >> the system. And we have a downstream patch
>> >> +               if (!strcmp(scmi_pd->name, "hsio_top"))
>> >> +                       scmi_pd->genpd.flags = 0;
>> >>
>> >
>> >There you go, so you simply rushed some solution upstream to carry less
>> >patches downstream but this time you got bitten again. Sorry, but I am
>> >seeing a pattern from you here and I don't really like that.
>>
>> I not wanna to argue here.
>> I DO care reputation. If I do something wrong, I will improve.
>>
>> >
>> >> So I am wondering to extend the attributes of SCMI spec,
>> >> Saying In SCMI spec(DEN0056E)
>> >> 4.3.2.5 POWER_DOMAIN_ATTRIBUTES
>> >> Page 44 of 210:
>> >>
>> >> Bit[26]: If set to 1, the power domain could
>> >> wakeup the system with power state
>> >> set as off.
>> >>
>> >
>> >This is not what the above flag is all about. It just instructs genpd
>> >to keep the power domain ON as the device attached to it could be a wakeup
>> >source. They are not one and the same. If the device is marked wakeup in
>> >the DT and it is both wakeup capable and is enabled, it shouldn't request
>> >the power domain to be powered off when suspending. Why is that not
>> >sufficient here ? What am I missing ? I am interested in knowing it as
>> >it is important to fix the issue you are trying to address here.
>>
>> The flag must be set if the power domain needs to keep power on to have
>> the wakeup capability.
>>
>> But in some case, a power domain no need to keep power on and it still
>> have wakeup capability from hardware perspective.
>>
>> For example usb phy in i.MX95, its power domain is off, but it could still
>> wakeup the SoC. But with the GENPD_FLAG_ACTIVE_WAKEUP set, the power domain
>> will not be powered off, so more power consumption in suspended state.
>
>This probably means that the wakeup mechanism is not in the same power
>domain but in another one that is kept on while the device is
>suspended and the main power domain is off. Do you have more details
>about this wakeup source that works with powerdomain being off ?

When power domain is off, some internal registers are retained and
not lost. And there is external always on power rail for always
on hardware logic.

Regards,
Peng

>I vaguely remember some old discussions about inband/outband wakeup
>source settings for devices which enable drivers to specify if the
>wakeup source is in or out of the power domain and if we should keep
>it on or not. But I can't remember the details; Ulf should have more
>details
>
>>
>> So I am requesting extending spec to give agents the information whether
>> the power domain could wakeup HW system in powered off state.
>
>I tend to agree with Sudeep that it doesn't make sense to say that a
>power domain can be off but the powered devices can still wakeup the
>system. It usually means that the wakeup is not powered by the main
>power domain
>
>>
>> >
>> >> This is just an idea in my mind, I not
>> >> write code to verify, just wanna to see
>> >> any comments from your side.
>> >>
>> >
>> >I wonder if we need to revert GENPD_FLAG_ACTIVE_WAKEUP flag enabling on
>> >all SCMI power domains if that is not solving the problem you thought it
>> >would.
>>
>> Revert the flag will cause a power domain not able to wakeup SoC if the
>> power domain needs power state on to have wakeup capability.
>>
>> I am not sure other vendor's SoC design. To i.MX95, without this flag set,
>> the NETC will lose wakeup on LAN capability.
>>
>> Regards,
>> Peng.
>>
>> >
>> >--
>> >Regards,
>> >Sudeep
>> >
>>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: POWER_DOMAIN_ATTRIBUTES in SCMI
  2025-02-18 15:02       ` Ulf Hansson
  2025-02-18 15:26         ` Sudeep Holla
@ 2025-02-18 16:31         ` Peng Fan
  2025-02-20  2:53           ` Peng Fan
  1 sibling, 1 reply; 11+ messages in thread
From: Peng Fan @ 2025-02-18 16:31 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Vincent Guittot, Sudeep Holla, Peng Fan, cristian.marussi@arm.com,
	souvik.chakravarty@arm.com, Dan Carpenter,
	arm-scmi@vger.kernel.org, Chuck Cannon

On Tue, Feb 18, 2025 at 04:02:58PM +0100, Ulf Hansson wrote:
>On Tue, 18 Feb 2025 at 14:57, Vincent Guittot
><vincent.guittot@linaro.org> wrote:
>>
>> On Tue, 18 Feb 2025 at 13:03, Peng Fan <peng.fan@oss.nxp.com> wrote:
>> >
>> > On Tue, Feb 18, 2025 at 10:41:15AM +0000, Sudeep Holla wrote:
>> > >On Fri, Feb 14, 2025 at 09:20:27AM +0000, Peng Fan wrote:
>> > >> All,
>> > >>
>> > >> Previously I posted a patch to linux to set all power domains
>> > >> with "GENPD_FLAG_ACTIVE_WAKEUP" set in
>> > >> drivers/pmdomain/arm/scmi_pm_domain.c
>> > >
>> > >Yes, I remember ACK-ing that and now I am thinking why 😄.
>> > >The description of the flag GENPD_FLAG_ACTIVE_WAKEUP says:
>> > >"Instructs genpd to keep the PM domain powered on, in case any of its
>> > >attached devices is used in the wakeup path to serve system wakeups."
>> > >
>> > >Does that mean all the SCMI power domains remain powered on with this
>> > >flag ? If so, that sounds wrong to me. I hope it is not the case and it
>> > >is effective only if the device attached is wakeup source.
>> >
>> > Only when the device is setup wakeup source, the genpd framework
>> > will take this flag as keeping power domain on.
>> >
>> > Without this flag, even if the device setup as wakeup source, the
>> > device's power domain will still be powered off.
>> >
>> > >
>> > >> But in the end we find that some power domains
>> > >> could be in off state while it still could wakeup
>> > >> the system. And we have a downstream patch
>> > >> +               if (!strcmp(scmi_pd->name, "hsio_top"))
>> > >> +                       scmi_pd->genpd.flags = 0;
>> > >>
>> > >
>> > >There you go, so you simply rushed some solution upstream to carry less
>> > >patches downstream but this time you got bitten again. Sorry, but I am
>> > >seeing a pattern from you here and I don't really like that.
>> >
>> > I not wanna to argue here.
>> > I DO care reputation. If I do something wrong, I will improve.
>> >
>> > >
>> > >> So I am wondering to extend the attributes of SCMI spec,
>> > >> Saying In SCMI spec(DEN0056E)
>> > >> 4.3.2.5 POWER_DOMAIN_ATTRIBUTES
>> > >> Page 44 of 210:
>> > >>
>> > >> Bit[26]: If set to 1, the power domain could
>> > >> wakeup the system with power state
>> > >> set as off.
>> > >>
>> > >
>> > >This is not what the above flag is all about. It just instructs genpd
>> > >to keep the power domain ON as the device attached to it could be a wakeup
>> > >source. They are not one and the same. If the device is marked wakeup in
>> > >the DT and it is both wakeup capable and is enabled, it shouldn't request
>> > >the power domain to be powered off when suspending. Why is that not
>> > >sufficient here ? What am I missing ? I am interested in knowing it as
>> > >it is important to fix the issue you are trying to address here.
>> >
>> > The flag must be set if the power domain needs to keep power on to have
>> > the wakeup capability.
>> >
>> > But in some case, a power domain no need to keep power on and it still
>> > have wakeup capability from hardware perspective.
>> >
>> > For example usb phy in i.MX95, its power domain is off, but it could still
>> > wakeup the SoC. But with the GENPD_FLAG_ACTIVE_WAKEUP set, the power domain
>> > will not be powered off, so more power consumption in suspended state.
>>
>> This probably means that the wakeup mechanism is not in the same power
>> domain but in another one that is kept on while the device is
>> suspended and the main power domain is off. Do you have more details
>> about this wakeup source that works with powerdomain being off ?
>> I vaguely remember some old discussions about inband/outband wakeup
>> source settings for devices which enable drivers to specify if the
>> wakeup source is in or out of the power domain and if we should keep
>> it on or not. But I can't remember the details; Ulf should have more
>> details
>
>Vincent, you have a great memory! :-)
>
>Indeed I tried to post a series (I can dig it up, if you want?) that
>allowed the driver to instruct upper layers, such as buses and PM
>domains whether the wakeup is managed in-band or out-band.
>
>An example could be an SDIO irq, being re-routed from a regular DATA
>line that is managed by the SDIO/MMC controller to a GPIO pin during
>system suspend. Another similar use-case with a potential similar
>wake-up configuration, is waking up from a UART console.
>
>My point is, the SDIO/MMC controller could be configured as a wakeup
>source, while it's actually the GPIO pin that is managing the wakeup.
>In this case, typically the GPIO irq can be managed from an always-on
>logic/PMIC, which means there is no reason to keep the PM domain
>powered-on for the SDIO/MMC controller during system suspend.
>
>At the moment we don't have any way to express this kind of
>configuration, I think.

Thanks for sharing detailed example.

typec chips using gpio to wakeup system should also be the same.

>
>>
>> >
>> > So I am requesting extending spec to give agents the information whether
>> > the power domain could wakeup HW system in powered off state.
>>
>> I tend to agree with Sudeep that it doesn't make sense to say that a
>> power domain can be off but the powered devices can still wakeup the
>> system. It usually means that the wakeup is not powered by the main
>> power domain
>
>I tend to agree. In general, I think we should always use
>GENPD_FLAG_ACTIVE_WAKEUP, but we are lacking a way to inform genpd
>(and other upper layers) that the wakeup-irq is managed out-band,
>which means genpd can still be powered-off.

ok. Would you plan to work on the feature? If you not have time,
I could do this, but need your guidance on the design. 

Thanks,
Peng.

>
>>
>> >
>> > >
>> > >> This is just an idea in my mind, I not
>> > >> write code to verify, just wanna to see
>> > >> any comments from your side.
>> > >>
>> > >
>> > >I wonder if we need to revert GENPD_FLAG_ACTIVE_WAKEUP flag enabling on
>> > >all SCMI power domains if that is not solving the problem you thought it
>> > >would.
>> >
>> > Revert the flag will cause a power domain not able to wakeup SoC if the
>> > power domain needs power state on to have wakeup capability.
>> >
>> > I am not sure other vendor's SoC design. To i.MX95, without this flag set,
>> > the NETC will lose wakeup on LAN capability.
>> >
>> > Regards,
>> > Peng.
>> >
>> > >
>> > >--
>> > >Regards,
>> > >Sudeep
>> > >
>> >
>
>Kind regards
>Uffe
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: POWER_DOMAIN_ATTRIBUTES in SCMI
  2025-02-18 16:31         ` Peng Fan
@ 2025-02-20  2:53           ` Peng Fan
  2025-02-20  9:59             ` Souvik Chakravarty
  0 siblings, 1 reply; 11+ messages in thread
From: Peng Fan @ 2025-02-20  2:53 UTC (permalink / raw)
  To: Ulf Hansson, Vincent Guittot
  Cc: Vincent Guittot, Sudeep Holla, Peng Fan, cristian.marussi@arm.com,
	souvik.chakravarty@arm.com, Dan Carpenter,
	arm-scmi@vger.kernel.org, Chuck Cannon

Hi Ulf, Vincent

On Wed, Feb 19, 2025 at 12:31:46AM +0800, Peng Fan wrote:
>On Tue, Feb 18, 2025 at 04:02:58PM +0100, Ulf Hansson wrote:
>>On Tue, 18 Feb 2025 at 14:57, Vincent Guittot
>><vincent.guittot@linaro.org> wrote:
>>>
>>> On Tue, 18 Feb 2025 at 13:03, Peng Fan <peng.fan@oss.nxp.com> wrote:
>>> >
>>> > On Tue, Feb 18, 2025 at 10:41:15AM +0000, Sudeep Holla wrote:
>>> > >On Fri, Feb 14, 2025 at 09:20:27AM +0000, Peng Fan wrote:
>>> > >> All,
>>> > >>
>>> > >> Previously I posted a patch to linux to set all power domains
>>> > >> with "GENPD_FLAG_ACTIVE_WAKEUP" set in
>>> > >> drivers/pmdomain/arm/scmi_pm_domain.c
>>> > >
>>> > >Yes, I remember ACK-ing that and now I am thinking why 😄.
>>> > >The description of the flag GENPD_FLAG_ACTIVE_WAKEUP says:
>>> > >"Instructs genpd to keep the PM domain powered on, in case any of its
>>> > >attached devices is used in the wakeup path to serve system wakeups."
>>> > >
>>> > >Does that mean all the SCMI power domains remain powered on with this
>>> > >flag ? If so, that sounds wrong to me. I hope it is not the case and it
>>> > >is effective only if the device attached is wakeup source.
>>> >
>>> > Only when the device is setup wakeup source, the genpd framework
>>> > will take this flag as keeping power domain on.
>>> >
>>> > Without this flag, even if the device setup as wakeup source, the
>>> > device's power domain will still be powered off.
>>> >
>>> > >
>>> > >> But in the end we find that some power domains
>>> > >> could be in off state while it still could wakeup
>>> > >> the system. And we have a downstream patch
>>> > >> +               if (!strcmp(scmi_pd->name, "hsio_top"))
>>> > >> +                       scmi_pd->genpd.flags = 0;
>>> > >>
>>> > >
>>> > >There you go, so you simply rushed some solution upstream to carry less
>>> > >patches downstream but this time you got bitten again. Sorry, but I am
>>> > >seeing a pattern from you here and I don't really like that.
>>> >
>>> > I not wanna to argue here.
>>> > I DO care reputation. If I do something wrong, I will improve.
>>> >
>>> > >
>>> > >> So I am wondering to extend the attributes of SCMI spec,
>>> > >> Saying In SCMI spec(DEN0056E)
>>> > >> 4.3.2.5 POWER_DOMAIN_ATTRIBUTES
>>> > >> Page 44 of 210:
>>> > >>
>>> > >> Bit[26]: If set to 1, the power domain could
>>> > >> wakeup the system with power state
>>> > >> set as off.
>>> > >>
>>> > >
>>> > >This is not what the above flag is all about. It just instructs genpd
>>> > >to keep the power domain ON as the device attached to it could be a wakeup
>>> > >source. They are not one and the same. If the device is marked wakeup in
>>> > >the DT and it is both wakeup capable and is enabled, it shouldn't request
>>> > >the power domain to be powered off when suspending. Why is that not
>>> > >sufficient here ? What am I missing ? I am interested in knowing it as
>>> > >it is important to fix the issue you are trying to address here.
>>> >
>>> > The flag must be set if the power domain needs to keep power on to have
>>> > the wakeup capability.
>>> >
>>> > But in some case, a power domain no need to keep power on and it still
>>> > have wakeup capability from hardware perspective.
>>> >
>>> > For example usb phy in i.MX95, its power domain is off, but it could still
>>> > wakeup the SoC. But with the GENPD_FLAG_ACTIVE_WAKEUP set, the power domain
>>> > will not be powered off, so more power consumption in suspended state.
>>>
>>> This probably means that the wakeup mechanism is not in the same power
>>> domain but in another one that is kept on while the device is
>>> suspended and the main power domain is off. Do you have more details
>>> about this wakeup source that works with powerdomain being off ?
>>> I vaguely remember some old discussions about inband/outband wakeup
>>> source settings for devices which enable drivers to specify if the
>>> wakeup source is in or out of the power domain and if we should keep
>>> it on or not. But I can't remember the details; Ulf should have more
>>> details
>>
>>Vincent, you have a great memory! :-)
>>
>>Indeed I tried to post a series (I can dig it up, if you want?) that
>>allowed the driver to instruct upper layers, such as buses and PM
>>domains whether the wakeup is managed in-band or out-band.
>>
>>An example could be an SDIO irq, being re-routed from a regular DATA
>>line that is managed by the SDIO/MMC controller to a GPIO pin during
>>system suspend. Another similar use-case with a potential similar
>>wake-up configuration, is waking up from a UART console.
>>
>>My point is, the SDIO/MMC controller could be configured as a wakeup
>>source, while it's actually the GPIO pin that is managing the wakeup.
>>In this case, typically the GPIO irq can be managed from an always-on
>>logic/PMIC, which means there is no reason to keep the PM domain
>>powered-on for the SDIO/MMC controller during system suspend.
>>
>>At the moment we don't have any way to express this kind of
>>configuration, I think.
>
>Thanks for sharing detailed example.
>
>typec chips using gpio to wakeup system should also be the same.
>
>>
>>>
>>> >
>>> > So I am requesting extending spec to give agents the information whether
>>> > the power domain could wakeup HW system in powered off state.
>>>
>>> I tend to agree with Sudeep that it doesn't make sense to say that a
>>> power domain can be off but the powered devices can still wakeup the
>>> system. It usually means that the wakeup is not powered by the main
>>> power domain
>>
>>I tend to agree. In general, I think we should always use
>>GENPD_FLAG_ACTIVE_WAKEUP, but we are lacking a way to inform genpd
>>(and other upper layers) that the wakeup-irq is managed out-band,
>>which means genpd can still be powered-off.
>
>ok. Would you plan to work on the feature? If you not have time,
>I could do this, but need your guidance on the design. 

Not sure whether you have time to work on this or not.

More info for i.MX case:
The i.MX95 usb wakeup logic is in an always on power domain. So when
usb controller is powered down, usb could still do remote wakeup.
There is no dedicated out band wakeup-irq in my case.

I draft a prototype. Would you please give a look whether this is feasible?
1. Add a new API device_set/get_out_band_wakeup with a new bool out_band_wakeup
   defined in dev_pm_info
2. Drivers could use device_set_out_band_wakeup to set the bool
3. pm core suspend/resume will check the bool to decide continue do
   power operation or bypass.

Thanks,
Peng.

Draft code below:

diff --git a/drivers/pmdomain/core.c b/drivers/pmdomain/core.c
index 9b2f28b34bb5..fa0d93c9078e 100644
--- a/drivers/pmdomain/core.c
+++ b/drivers/pmdomain/core.c
@@ -1450,7 +1450,8 @@ static int genpd_finish_suspend(struct device *dev,
        if (ret)
                return ret;
			 
-       if (device_wakeup_path(dev) && genpd_is_active_wakeup(genpd))
+       if (device_wakeup_path(dev) && genpd_is_active_wakeup(genpd) &&
+           !device_get_out_band_wakeup(dev))
                return 0;
					  
         if (genpd->dev_ops.stop && genpd->dev_ops.start &&
@@ -1505,7 +1506,8 @@ static int genpd_finish_resume(struct device *dev,
	 if (IS_ERR(genpd))
	        return -EINVAL;
				   
-       if (device_wakeup_path(dev) && genpd_is_active_wakeup(genpd))
+       if (device_wakeup_path(dev) && genpd_is_active_wakeup(genpd) &&
+           !device_get_out_band_wakeup(dev))
               return resume_noirq(dev);

In include/linux/pm_wakeup.h

+static inline void device_set_out_band_wakeup(struct device *dev, bool capable)
+{
+       dev->power.out_band_wakeup = capable;
+}
+
+static inline bool device_get_out_band_wakeup(struct device *dev)
+{
+       return dev->power.out_band_wakeup;
+}

>
>Thanks,
>Peng.
>
>>
>>>
>>> >
>>> > >
>>> > >> This is just an idea in my mind, I not
>>> > >> write code to verify, just wanna to see
>>> > >> any comments from your side.
>>> > >>
>>> > >
>>> > >I wonder if we need to revert GENPD_FLAG_ACTIVE_WAKEUP flag enabling on
>>> > >all SCMI power domains if that is not solving the problem you thought it
>>> > >would.
>>> >
>>> > Revert the flag will cause a power domain not able to wakeup SoC if the
>>> > power domain needs power state on to have wakeup capability.
>>> >
>>> > I am not sure other vendor's SoC design. To i.MX95, without this flag set,
>>> > the NETC will lose wakeup on LAN capability.
>>> >
>>> > Regards,
>>> > Peng.
>>> >
>>> > >
>>> > >--
>>> > >Regards,
>>> > >Sudeep
>>> > >
>>> >
>>
>>Kind regards
>>Uffe
>>

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: POWER_DOMAIN_ATTRIBUTES in SCMI
  2025-02-20  2:53           ` Peng Fan
@ 2025-02-20  9:59             ` Souvik Chakravarty
  2025-04-08  0:23               ` Kevin Hilman
  0 siblings, 1 reply; 11+ messages in thread
From: Souvik Chakravarty @ 2025-02-20  9:59 UTC (permalink / raw)
  To: Peng Fan, Ulf Hansson, Vincent Guittot
  Cc: Sudeep Holla, Peng Fan, cristian.marussi@arm.com, Dan Carpenter,
	arm-scmi@vger.kernel.org, Chuck Cannon

Some quick thoughts below to keep this architecturally consistent.

On 20/02/2025 02:53, Peng Fan wrote:
> Hi Ulf, Vincent
>
> On Wed, Feb 19, 2025 at 12:31:46AM +0800, Peng Fan wrote:
>> On Tue, Feb 18, 2025 at 04:02:58PM +0100, Ulf Hansson wrote:
>>> On Tue, 18 Feb 2025 at 14:57, Vincent Guittot
>>> <vincent.guittot@linaro.org> wrote:
>>>>
>>>> On Tue, 18 Feb 2025 at 13:03, Peng Fan <peng.fan@oss.nxp.com> wrote:
>>>>>
>>>>> On Tue, Feb 18, 2025 at 10:41:15AM +0000, Sudeep Holla wrote:
>>>>>> On Fri, Feb 14, 2025 at 09:20:27AM +0000, Peng Fan wrote:
>>>>>>> All,
>>>>>>>
>>>>>>> Previously I posted a patch to linux to set all power domains
>>>>>>> with "GENPD_FLAG_ACTIVE_WAKEUP" set in
>>>>>>> drivers/pmdomain/arm/scmi_pm_domain.c
>>>>>>
>>>>>> Yes, I remember ACK-ing that and now I am thinking why 😄.
>>>>>> The description of the flag GENPD_FLAG_ACTIVE_WAKEUP says:
>>>>>> "Instructs genpd to keep the PM domain powered on, in case any of its
>>>>>> attached devices is used in the wakeup path to serve system wakeups."
>>>>>>
>>>>>> Does that mean all the SCMI power domains remain powered on with this
>>>>>> flag ? If so, that sounds wrong to me. I hope it is not the case and it
>>>>>> is effective only if the device attached is wakeup source.
>>>>>
>>>>> Only when the device is setup wakeup source, the genpd framework
>>>>> will take this flag as keeping power domain on.
>>>>>
>>>>> Without this flag, even if the device setup as wakeup source, the
>>>>> device's power domain will still be powered off.
>>>>>
>>>>>>
>>>>>>> But in the end we find that some power domains
>>>>>>> could be in off state while it still could wakeup
>>>>>>> the system. And we have a downstream patch
>>>>>>> +               if (!strcmp(scmi_pd->name, "hsio_top"))
>>>>>>> +                       scmi_pd->genpd.flags = 0;
>>>>>>>
>>>>>>
>>>>>> There you go, so you simply rushed some solution upstream to carry less
>>>>>> patches downstream but this time you got bitten again. Sorry, but I am
>>>>>> seeing a pattern from you here and I don't really like that.
>>>>>
>>>>> I not wanna to argue here.
>>>>> I DO care reputation. If I do something wrong, I will improve.
>>>>>
>>>>>>
>>>>>>> So I am wondering to extend the attributes of SCMI spec,
>>>>>>> Saying In SCMI spec(DEN0056E)
>>>>>>> 4.3.2.5 POWER_DOMAIN_ATTRIBUTES
>>>>>>> Page 44 of 210:
>>>>>>>
>>>>>>> Bit[26]: If set to 1, the power domain could
>>>>>>> wakeup the system with power state
>>>>>>> set as off.
>>>>>>>
>>>>>>
>>>>>> This is not what the above flag is all about. It just instructs genpd
>>>>>> to keep the power domain ON as the device attached to it could be a wakeup
>>>>>> source. They are not one and the same. If the device is marked wakeup in
>>>>>> the DT and it is both wakeup capable and is enabled, it shouldn't request
>>>>>> the power domain to be powered off when suspending. Why is that not
>>>>>> sufficient here ? What am I missing ? I am interested in knowing it as
>>>>>> it is important to fix the issue you are trying to address here.
>>>>>
>>>>> The flag must be set if the power domain needs to keep power on to have
>>>>> the wakeup capability.
>>>>>
>>>>> But in some case, a power domain no need to keep power on and it still
>>>>> have wakeup capability from hardware perspective.
>>>>>
>>>>> For example usb phy in i.MX95, its power domain is off, but it could still
>>>>> wakeup the SoC. But with the GENPD_FLAG_ACTIVE_WAKEUP set, the power domain
>>>>> will not be powered off, so more power consumption in suspended state.
>>>>
>>>> This probably means that the wakeup mechanism is not in the same power
>>>> domain but in another one that is kept on while the device is
>>>> suspended and the main power domain is off. Do you have more details
>>>> about this wakeup source that works with powerdomain being off ?
>>>> I vaguely remember some old discussions about inband/outband wakeup
>>>> source settings for devices which enable drivers to specify if the
>>>> wakeup source is in or out of the power domain and if we should keep
>>>> it on or not. But I can't remember the details; Ulf should have more
>>>> details
>>>
>>> Vincent, you have a great memory! :-)
>>>
>>> Indeed I tried to post a series (I can dig it up, if you want?) that
>>> allowed the driver to instruct upper layers, such as buses and PM
>>> domains whether the wakeup is managed in-band or out-band.
>>>
>>> An example could be an SDIO irq, being re-routed from a regular DATA
>>> line that is managed by the SDIO/MMC controller to a GPIO pin during
>>> system suspend. Another similar use-case with a potential similar
>>> wake-up configuration, is waking up from a UART console.
>>>
>>> My point is, the SDIO/MMC controller could be configured as a wakeup
>>> source, while it's actually the GPIO pin that is managing the wakeup.
>>> In this case, typically the GPIO irq can be managed from an always-on
>>> logic/PMIC, which means there is no reason to keep the PM domain
>>> powered-on for the SDIO/MMC controller during system suspend.
>>>
>>> At the moment we don't have any way to express this kind of
>>> configuration, I think.
>>
>> Thanks for sharing detailed example.
>>
>> typec chips using gpio to wakeup system should also be the same.
>>
>>>
>>>>
>>>>>
>>>>> So I am requesting extending spec to give agents the information whether
>>>>> the power domain could wakeup HW system in powered off state.
>>>>
>>>> I tend to agree with Sudeep that it doesn't make sense to say that a
>>>> power domain can be off but the powered devices can still wakeup the
>>>> system. It usually means that the wakeup is not powered by the main
>>>> power domain
>>>
>>> I tend to agree. In general, I think we should always use
>>> GENPD_FLAG_ACTIVE_WAKEUP, but we are lacking a way to inform genpd
>>> (and other upper layers) that the wakeup-irq is managed out-band,
>>> which means genpd can still be powered-off.
>>
>> ok. Would you plan to work on the feature? If you not have time,
>> I could do this, but need your guidance on the design.
>
> Not sure whether you have time to work on this or not.
>
> More info for i.MX case:
> The i.MX95 usb wakeup logic is in an always on power domain. So when
> usb controller is powered down, usb could still do remote wakeup.
> There is no dedicated out band wakeup-irq in my case.

There are 2 ways to wakeup a device:
a) The device is powered by one or more PDs, one or more of which needs
to be kept ON to trigger the wakeup.
b) The device is powered by one or more PDs, but none of them are
required to trigger a wakeup. The device wake is derived from an out of
band logic which is kept powered ON (either transparently to OS, or via
another PD which does NOT belong to the device).

It would be good to ensure that whatever solution we craft here
acknowledges and works for both these cases, as they are subtly different.

Regards,
Souvik


>
> I draft a prototype. Would you please give a look whether this is feasible?
> 1. Add a new API device_set/get_out_band_wakeup with a new bool out_band_wakeup
>     defined in dev_pm_info
> 2. Drivers could use device_set_out_band_wakeup to set the bool
> 3. pm core suspend/resume will check the bool to decide continue do
>     power operation or bypass.
>
> Thanks,
> Peng.
>
> Draft code below:
>
> diff --git a/drivers/pmdomain/core.c b/drivers/pmdomain/core.c
> index 9b2f28b34bb5..fa0d93c9078e 100644
> --- a/drivers/pmdomain/core.c
> +++ b/drivers/pmdomain/core.c
> @@ -1450,7 +1450,8 @@ static int genpd_finish_suspend(struct device *dev,
>          if (ret)
>                  return ret;
>
> -       if (device_wakeup_path(dev) && genpd_is_active_wakeup(genpd))
> +       if (device_wakeup_path(dev) && genpd_is_active_wakeup(genpd) &&
> +           !device_get_out_band_wakeup(dev))
>                  return 0;
>
>           if (genpd->dev_ops.stop && genpd->dev_ops.start &&
> @@ -1505,7 +1506,8 @@ static int genpd_finish_resume(struct device *dev,
>        if (IS_ERR(genpd))
>               return -EINVAL;
>
> -       if (device_wakeup_path(dev) && genpd_is_active_wakeup(genpd))
> +       if (device_wakeup_path(dev) && genpd_is_active_wakeup(genpd) &&
> +           !device_get_out_band_wakeup(dev))
>                 return resume_noirq(dev);
>
> In include/linux/pm_wakeup.h
>
> +static inline void device_set_out_band_wakeup(struct device *dev, bool capable)
> +{
> +       dev->power.out_band_wakeup = capable;
> +}
> +
> +static inline bool device_get_out_band_wakeup(struct device *dev)
> +{
> +       return dev->power.out_band_wakeup;
> +}
>
>>
>> Thanks,
>> Peng.
>>
>>>
>>>>
>>>>>
>>>>>>
>>>>>>> This is just an idea in my mind, I not
>>>>>>> write code to verify, just wanna to see
>>>>>>> any comments from your side.
>>>>>>>
>>>>>>
>>>>>> I wonder if we need to revert GENPD_FLAG_ACTIVE_WAKEUP flag enabling on
>>>>>> all SCMI power domains if that is not solving the problem you thought it
>>>>>> would.
>>>>>
>>>>> Revert the flag will cause a power domain not able to wakeup SoC if the
>>>>> power domain needs power state on to have wakeup capability.
>>>>>
>>>>> I am not sure other vendor's SoC design. To i.MX95, without this flag set,
>>>>> the NETC will lose wakeup on LAN capability.
>>>>>
>>>>> Regards,
>>>>> Peng.
>>>>>
>>>>>>
>>>>>> --
>>>>>> Regards,
>>>>>> Sudeep
>>>>>>
>>>>>
>>>
>>> Kind regards
>>> Uffe
>>>
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: POWER_DOMAIN_ATTRIBUTES in SCMI
  2025-02-20  9:59             ` Souvik Chakravarty
@ 2025-04-08  0:23               ` Kevin Hilman
  0 siblings, 0 replies; 11+ messages in thread
From: Kevin Hilman @ 2025-04-08  0:23 UTC (permalink / raw)
  To: Souvik Chakravarty, Peng Fan, Ulf Hansson, Vincent Guittot
  Cc: Sudeep Holla, Peng Fan, cristian.marussi@arm.com, Dan Carpenter,
	arm-scmi@vger.kernel.org, Chuck Cannon

Souvik Chakravarty <souvik.chakravarty@arm.com> writes:

> Some quick thoughts below to keep this architecturally consistent.

[...]

> There are 2 ways to wakeup a device:
> a) The device is powered by one or more PDs, one or more of which needs
> to be kept ON to trigger the wakeup.
> b) The device is powered by one or more PDs, but none of them are
> required to trigger a wakeup. The device wake is derived from an out of
> band logic which is kept powered ON (either transparently to OS, or via
> another PD which does NOT belong to the device).

In the case of TI SoCs, it's (b).  In fact, most of the IO pads on the
SoC can be enabled as wakeup capable when the SoC hits certain low-power
states.  The bits to control this are part of the pinctrl settings for
the each driver on these SoCs.  So technically it's neither the device
nor its power domain that cause the wakeup, but the external pad, aided
by some dedicated, always-on hardware, called "IO daisy chain" in the TI
documentation[1].

Kevin

[1] c.f Section 6.2.3.11 I/O Power Management and Daisy Chaining
    in the AM62L TRM for b bit more details: https://www.ti.com/lit/pdf/sprujb4





^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2025-04-08  0:23 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-14  9:20 POWER_DOMAIN_ATTRIBUTES in SCMI Peng Fan
2025-02-18 10:41 ` Sudeep Holla
2025-02-18 13:08   ` Peng Fan
2025-02-18 13:57     ` Vincent Guittot
2025-02-18 15:02       ` Ulf Hansson
2025-02-18 15:26         ` Sudeep Holla
2025-02-18 16:31         ` Peng Fan
2025-02-20  2:53           ` Peng Fan
2025-02-20  9:59             ` Souvik Chakravarty
2025-04-08  0:23               ` Kevin Hilman
2025-02-18 16:20       ` Peng Fan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.