Re: [PATCH V4 16/16] ARM64: tegra: select PM_GENERIC_DOMAINS

[Jon Hunter]

Hi Arnd, Ulf,

On 14/01/16 17:16, Jon Hunter wrote:
> 
> On 14/01/16 09:21, Arnd Bergmann wrote:
>> On Thursday 14 January 2016 09:57:14 Ulf Hansson wrote:
>>> On 13 January 2016 at 21:43, Arnd Bergmann <arnd@xxxxxxxx> wrote:
>>>> On Wednesday 13 January 2016 18:03:24 Thierry Reding wrote:
>>>>> On Fri, Dec 04, 2015 at 02:57:17PM +0000, Jon Hunter wrote:
>>>>>> Enable PM_GENERIC_DOMAINS for tegra 64-bit devices. To ensure that devices
>>>>>> dependent upon a particular power-domain are only probed when that power
>>>>>> domain has been powered up, requires that PM is made mandatory for tegra
>>>>>> 64-bit devices and so select this option for tegra as well.
>>>>>>
>>>>>> Signed-off-by: Jon Hunter <jonathanh@xxxxxxxxxx>
>>>>>> ---
>>>>>>  arch/arm64/Kconfig.platforms | 2 ++
>>>>>>  1 file changed, 2 insertions(+)
>>>>>>
>>>>>> diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
>>>>>> index 9806324fa215..e0b5bd0aff0f 100644
>>>>>> --- a/arch/arm64/Kconfig.platforms
>>>>>> +++ b/arch/arm64/Kconfig.platforms
>>>>>> @@ -93,6 +93,8 @@ config ARCH_TEGRA
>>>>>>       select GENERIC_CLOCKEVENTS
>>>>>>       select HAVE_CLK
>>>>>>       select PINCTRL
>>>>>> +     select PM
>>>>>> +     select PM_GENERIC_DOMAINS
>>>>>>       select RESET_CONTROLLER
>>>>>>       help
>>>>>>         This enables support for the NVIDIA Tegra SoC family.
>>>>>
>>>>> This has potential consequences for multi-platform builds, doesn't it?
>>>>> All of a sudden any combination of builds that includes Tegra won't be
>>>>> possible to build without PM support.
>>>>>
>>>>> Adding linux-arm-kernel@xxxxxxxxxxxxxxxxxxx for visibility.
>>>>>
>>>>>
>>>>
>>>> Agreed, it would be better to add 'depends on PM_GENERIC_DOMAINS'
>>>> dependencies in the drivers that require it.
>>>>
>>>
>>> The problem with that approach is that if those drivers are cross SoC
>>> drivers. In some cases PM isn't needed and it is.
>>>
>>> Of course I don't have the in depth knowledge about the drivers being
>>> used in Tegra which may need PM, perhaps it's not that many?
>>>
>>> Anyway, to me it seems like ARCH_TEGRA should depend on PM instead.
>>> Would that work?
>>
>> That seems a little over-restrictive, as it prevents you from
>> building a tegra kernel even if none of the drivers that rely
>> on the pm domains are used, but it would work.
>>
>> I've looked again at how other platforms (on arm32) do it, and
>> a lot of them use "select PM_GENERIC_DOMAINS if PM", so they don't
>> automatically enable PM, but they enable the pmdomain code if
>> PM is already set. No driver really "depends on PM_GENERIC_DOMAINS",
>> so we shouldn't really start that now or we end up with circular
>> dependencies in the long run.
> 
> What I am not a fan of in the current gen-pd implementation, is if we
> have !PM but the platform has power-domains, then there is no way to
> determine if a device within a power-domain can be probed safely. Some
> arm platforms force all the power-domains on during early init in the
> case of !PM. IMO this is still not ideal, because if a power-domain
> failed to turn on during early init, then you should probably call
> BUG(). Ideally the kernel should be able to boot and only probe the
> devices you know that can be probed safely.
> 
> So for platforms have use PM_GENERIC_DOMAINS, I think really they should
> select PM and not "select PM_GENERIC_DOMAINS if PM". IMO, "select
> PM_GENERIC_DOMAINS if PM" seems fragile.

Any more thoughts on this?

I have been discussing with Thierry and we think that selecting PM for
tegra still makes the most sense. The question is, is this ok for
multi-configs?

The only other suggestion/thought I have is to allow
PM_GENERIC_DOMAINS_OF to be selected independently of PM so that we can
have minimal support for PM domains that allows you to register PM
domains with the kernel and their current state, but does not allow you
to control them, etc. This way tegra could always select
PM_GENERIC_DOMAINS_OF regardless of PM, and we would be able to
determine if we can probe a device safely.

I am not sure that Rafael is too keen on this approach but that is the
only alternative I have come up with.

I have a rough outline of a patch for this here [0] FWIW.

Cheers
Jon

[0] https://github.com/jonhunter/linux/commits/gpd