[RFD] PM_GENERIC_DOMAINS && !PM

[RFD] PM_GENERIC_DOMAINS && !PM

[Jon Hunter]

Currently, if PM is disabled in the kernel then so is
PM_GENERIC_DOMAINS. Although this makes sense, I can see a scenario
where having minimal genpd support could be advantageous.

I am looking at enabling genpd for Tegra and ideally we would populate
the power domains when the platform device is probed for the power
management controller (PMC) as this device contains the registers for
enabling the power domains.

This works fine for the case where PM is enabled, but I am concerned
about the case where we don't have PM enabled in the kernel. In this
case, because domains are not populated early during the boot process, I
am concerned that there is a chance for a device dependent on a power
domain to be probed before the PMC device has been probed and had chance
to turn on any power domains.

Obviously, we could try and play with the ordering of devices, but it
seems to me that even if PM is not enabled, it would still be nice to
register power domains with the kernel and allow devices to attach to
them and only probe the device if they are powered.

Does this sound reasonable or is there a better approach?

Cheers
Jon

Re: [RFD] PM_GENERIC_DOMAINS && !PM

[Rafael J. Wysocki]

On Monday, November 30, 2015 04:42:46 PM Jon Hunter wrote:
> Currently, if PM is disabled in the kernel then so is
> PM_GENERIC_DOMAINS. Although this makes sense, I can see a scenario
> where having minimal genpd support could be advantageous.
> 
> I am looking at enabling genpd for Tegra and ideally we would populate
> the power domains when the platform device is probed for the power
> management controller (PMC) as this device contains the registers for
> enabling the power domains.
> 
> This works fine for the case where PM is enabled, but I am concerned
> about the case where we don't have PM enabled in the kernel. In this
> case, because domains are not populated early during the boot process, I
> am concerned that there is a chance for a device dependent on a power
> domain to be probed before the PMC device has been probed and had chance
> to turn on any power domains.

So make the !PM case invalid for that platform.

Seriously, either PM is mandatory, or it isn't.  If it is mandatory, make it so
instead of pretending that you can live without it.

Thanks,
Rafael

Re: [RFD] PM_GENERIC_DOMAINS && !PM

[Jon Hunter]

On 30/11/15 22:42, Rafael J. Wysocki wrote:
> On Monday, November 30, 2015 04:42:46 PM Jon Hunter wrote:
>> Currently, if PM is disabled in the kernel then so is
>> PM_GENERIC_DOMAINS. Although this makes sense, I can see a scenario
>> where having minimal genpd support could be advantageous.
>>
>> I am looking at enabling genpd for Tegra and ideally we would populate
>> the power domains when the platform device is probed for the power
>> management controller (PMC) as this device contains the registers for
>> enabling the power domains.
>>
>> This works fine for the case where PM is enabled, but I am concerned
>> about the case where we don't have PM enabled in the kernel. In this
>> case, because domains are not populated early during the boot process, I
>> am concerned that there is a chance for a device dependent on a power
>> domain to be probed before the PMC device has been probed and had chance
>> to turn on any power domains.
> 
> So make the !PM case invalid for that platform.

Thierry, I know that we have not reviewed GPD for tegra recently, but
what are your thoughts on the above in principle? At least for Tegra 64-bit?

> Seriously, either PM is mandatory, or it isn't.  If it is mandatory, make it so
> instead of pretending that you can live without it.

I understand your point and I do agree, however, this means that for any
device with power-domains that PM is already mandatory (unless they
don't use genpd at all).

I see some platforms getting around this by enabling all the
power-domains early during the platform code and if PM is not enabled
then they won't be turned off and so all is ok (d438462c20a3 "ARM: imx6:
gpc: always enable PU domain if CONFIG_PM is not set"). However, this
assumes that power-domains will never fail to power on and this seems a
little fragile to me.

One thought I had was to make the PM_GENERIC_DOMAINS_OF code independent
of PM (ie. moved to domain_of.c) and with a little tweaking of the
genpd_dev_pm_attach() function it could be made to work whether PM is
enable or not. In other words, if PM is not enabled, then
genpd_dev_pm_attach() would only return success if:
1. The device is not dependent on a power domain or
2. The device is dependent on a power domain and it is present and
   already powered on.

Alternatively, may be genpd_dev_pm_attach() should always return
-EPROBE_DEFER (to prevent probing a device) if !PM and the device has a
"power-domains" node defined.

Cheers
Jon

Re: [RFD] PM_GENERIC_DOMAINS && !PM

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 3129 bytes --]

On Tue, Dec 01, 2015 at 03:38:14PM +0000, Jon Hunter wrote:
> 
> On 30/11/15 22:42, Rafael J. Wysocki wrote:
> > On Monday, November 30, 2015 04:42:46 PM Jon Hunter wrote:
> >> Currently, if PM is disabled in the kernel then so is
> >> PM_GENERIC_DOMAINS. Although this makes sense, I can see a scenario
> >> where having minimal genpd support could be advantageous.
> >>
> >> I am looking at enabling genpd for Tegra and ideally we would populate
> >> the power domains when the platform device is probed for the power
> >> management controller (PMC) as this device contains the registers for
> >> enabling the power domains.
> >>
> >> This works fine for the case where PM is enabled, but I am concerned
> >> about the case where we don't have PM enabled in the kernel. In this
> >> case, because domains are not populated early during the boot process, I
> >> am concerned that there is a chance for a device dependent on a power
> >> domain to be probed before the PMC device has been probed and had chance
> >> to turn on any power domains.
> > 
> > So make the !PM case invalid for that platform.
> 
> Thierry, I know that we have not reviewed GPD for tegra recently, but
> what are your thoughts on the above in principle? At least for Tegra 64-bit?
> 
> > Seriously, either PM is mandatory, or it isn't.  If it is mandatory, make it so
> > instead of pretending that you can live without it.
> 
> I understand your point and I do agree, however, this means that for any
> device with power-domains that PM is already mandatory (unless they
> don't use genpd at all).
> 
> I see some platforms getting around this by enabling all the
> power-domains early during the platform code and if PM is not enabled
> then they won't be turned off and so all is ok (d438462c20a3 "ARM: imx6:
> gpc: always enable PU domain if CONFIG_PM is not set"). However, this
> assumes that power-domains will never fail to power on and this seems a
> little fragile to me.
> 
> One thought I had was to make the PM_GENERIC_DOMAINS_OF code independent
> of PM (ie. moved to domain_of.c) and with a little tweaking of the
> genpd_dev_pm_attach() function it could be made to work whether PM is
> enable or not. In other words, if PM is not enabled, then
> genpd_dev_pm_attach() would only return success if:
> 1. The device is not dependent on a power domain or
> 2. The device is dependent on a power domain and it is present and
>    already powered on.
> 
> Alternatively, may be genpd_dev_pm_attach() should always return
> -EPROBE_DEFER (to prevent probing a device) if !PM and the device has a
> "power-domains" node defined.

That all sounds very complicated for little gain. Disabling PM doesn't
make much sense these days, so I'd personally be fine to unconditonally
enable PM on Tegra. I'm not sure what the correct way would be. Since
the PM symbol is user-visible, selecting it doesn't sound like a very
good solution, though a quick grep shows that some architectures do
this.

Rafael, do you know what the canonical way is to make !PM invalid for a
platform?

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [RFD] PM_GENERIC_DOMAINS && !PM

[Rafael J. Wysocki]

On Tuesday, December 01, 2015 05:18:09 PM Thierry Reding wrote:
> 

[cut]

> 
> That all sounds very complicated for little gain. Disabling PM doesn't
> make much sense these days, so I'd personally be fine to unconditonally
> enable PM on Tegra. I'm not sure what the correct way would be. Since
> the PM symbol is user-visible, selecting it doesn't sound like a very
> good solution, though a quick grep shows that some architectures do
> this.
> 
> Rafael, do you know what the canonical way is to make !PM invalid for a
> platform?

Well, good question.

I can't really suggest anything cleaner than selecting it from the top-level
Kconfig at the moment.

Thanks,
Rafael

Re: [RFD] PM_GENERIC_DOMAINS && !PM

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 855 bytes --]

On Wed, Dec 02, 2015 at 02:35:20AM +0100, Rafael J. Wysocki wrote:
> On Tuesday, December 01, 2015 05:18:09 PM Thierry Reding wrote:
> > 
> 
> [cut]
> 
> > 
> > That all sounds very complicated for little gain. Disabling PM doesn't
> > make much sense these days, so I'd personally be fine to unconditonally
> > enable PM on Tegra. I'm not sure what the correct way would be. Since
> > the PM symbol is user-visible, selecting it doesn't sound like a very
> > good solution, though a quick grep shows that some architectures do
> > this.
> > 
> > Rafael, do you know what the canonical way is to make !PM invalid for a
> > platform?
> 
> Well, good question.
> 
> I can't really suggest anything cleaner than selecting it from the top-level
> Kconfig at the moment.

Alright, sounds like something we should do.

Thanks,
Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]