Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1308638 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH V4 16/16] ARM64: tegra: select PM_GENERIC_DOMAINS Thierry Reding <thierry.reding@gmail.com> - 2016-01-13 18:10 +0100
    Re: [PATCH V4 16/16] ARM64: tegra: select PM_GENERIC_DOMAINS Arnd Bergmann <arnd@arndb.de> - 2016-01-13 21:50 +0100
      Re: [PATCH V4 16/16] ARM64: tegra: select PM_GENERIC_DOMAINS Ulf Hansson <ulf.hansson@linaro.org> - 2016-01-14 10:00 +0100
        Re: [PATCH V4 16/16] ARM64: tegra: select PM_GENERIC_DOMAINS Arnd Bergmann <arnd@arndb.de> - 2016-01-14 10:30 +0100
          Re: [PATCH V4 16/16] ARM64: tegra: select PM_GENERIC_DOMAINS Thierry Reding <thierry.reding@gmail.com> - 2016-01-14 11:30 +0100
            Re: [PATCH V4 16/16] ARM64: tegra: select PM_GENERIC_DOMAINS Arnd Bergmann <arnd@arndb.de> - 2016-01-14 12:20 +0100
          Re: [PATCH V4 16/16] ARM64: tegra: select PM_GENERIC_DOMAINS Jon Hunter <jonathanh@nvidia.com> - 2016-01-14 18:20 +0100

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

[Multipart message — attachments visible in raw view] — view raw

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@nvidia.com>
> ---
>  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@lists.infradead.org for visibility.

Thierry

[toc] | [next] | [standalone]

#1308803

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@nvidia.com>
> > ---
> >  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@lists.infradead.org for visibility.
> 
> 

Agreed, it would be better to add 'depends on PM_GENERIC_DOMAINS'
dependencies in the drivers that require it.

	Arnd

[toc] | [prev] | [next] | [standalone]

#1309088

On 13 January 2016 at 21:43, Arnd Bergmann <arnd@arndb.de> 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@nvidia.com>
>> > ---
>> >  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@lists.infradead.org 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?

Kind regards
Uffe

[toc] | [prev] | [next] | [standalone]

#1309113

On Thursday 14 January 2016 09:57:14 Ulf Hansson wrote:
> On 13 January 2016 at 21:43, Arnd Bergmann <arnd@arndb.de> 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@nvidia.com>
> >> > ---
> >> >  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@lists.infradead.org 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.

	Arnd

[toc] | [prev] | [next] | [standalone]

#1309158

[Multipart message — attachments visible in raw view] — view raw

On Thu, Jan 14, 2016 at 10:21:06AM +0100, Arnd Bergmann wrote:
> On Thursday 14 January 2016 09:57:14 Ulf Hansson wrote:
> > On 13 January 2016 at 21:43, Arnd Bergmann <arnd@arndb.de> 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@nvidia.com>
> > >> > ---
> > >> >  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@lists.infradead.org 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.

It just occurred to me that none of these options really make much of a
difference. As Jon mentioned once we merge this series a lot of features
on Tegra will start to rely on PM_GENERIC_DOMAINS and hence PM. So if we
do want to build a kernel with a maximum of Tegra features enabled (and
I think a multi_v7_defconfig should include that) we'll end up with a PM
dependency anyway, whether forced via select or implied via depends on.

I'm beginning to wonder if PM really should be an option these days. The
disadvantages of making it optional do outweigh the advantages in my
opinion. I'm not saying that, in general, it's totally useless to build
a kernel that has no PM support, but for the more specific case where
you would want to enable multi-platform support I don't think there's
much practical advantage in allowing !PM. One of the most common build
warnings are triggered because of this option. Also multi-platform
kernels are really big already, so much so that I doubt it would make a
significant difference if we unconditionally built PM support. Also the
chances are that we'll be seeing more and more SoCs support PM and rely
on it, much like Tegra would with the addition of this series.

I imagine that we could save ourselves a lot of headaches by simply
enabling PM by default, whether that be via the PM Kconfig option or by
selecting it from ARCH_TEGRA and any other architectures that may come
to rely on it. Doing so would also reduce the amount of test coverage
that we need to do, both at compile- and runtime.

Thierry

[toc] | [prev] | [next] | [standalone]

#1309185

On Thursday 14 January 2016 11:29:24 Thierry Reding wrote:
> 
> It just occurred to me that none of these options really make much of a
> difference. As Jon mentioned once we merge this series a lot of features
> on Tegra will start to rely on PM_GENERIC_DOMAINS and hence PM. So if we
> do want to build a kernel with a maximum of Tegra features enabled (and
> I think a multi_v7_defconfig should include that) we'll end up with a PM
> dependency anyway, whether forced via select or implied via depends on.
> 
> I'm beginning to wonder if PM really should be an option these days. The
> disadvantages of making it optional do outweigh the advantages in my
> opinion. I'm not saying that, in general, it's totally useless to build
> a kernel that has no PM support, but for the more specific case where
> you would want to enable multi-platform support I don't think there's
> much practical advantage in allowing !PM. One of the most common build
> warnings are triggered because of this option. Also multi-platform
> kernels are really big already, so much so that I doubt it would make a
> significant difference if we unconditionally built PM support. Also the
> chances are that we'll be seeing more and more SoCs support PM and rely
> on it, much like Tegra would with the addition of this series.
> 
> I imagine that we could save ourselves a lot of headaches by simply
> enabling PM by default, whether that be via the PM Kconfig option or by
> selecting it from ARCH_TEGRA and any other architectures that may come
> to rely on it. Doing so would also reduce the amount of test coverage
> that we need to do, both at compile- and runtime.

I think this needs some investigation. As a general policy, we should
not grow the kernel image size when moving from a traditional ARM
platform to an ARCH_MULTIPLATFORM one.

This is somewhat contradicted by how we already require CONFIG_OF
to be set for multiplatform kernels, and that adds around 80kb
to the image size.

Looking at just the defconfig files, these are the ones that currently
do not set CONFIG_PM:

build/acs5k_defconfig/.config:# CONFIG_PM is not set
build/acs5k_tiny_defconfig/.config:# CONFIG_PM is not set
build/axm55xx_defconfig/.config:# CONFIG_PM is not set
build/bcm2835_defconfig/.config:# CONFIG_PM is not set
build/clps711x_defconfig/.config:# CONFIG_PM is not set
build/ebsa110_defconfig/.config:# CONFIG_PM is not set
build/footbridge_defconfig/.config:# CONFIG_PM is not set
build/ks8695_defconfig/.config:# CONFIG_PM is not set
build/netwinder_defconfig/.config:# CONFIG_PM is not set
build/rpc_defconfig/.config:# CONFIG_PM is not set
build/u300_defconfig/.config:# CONFIG_PM is not set
build/vf610m4_defconfig/.config:# CONFIG_PM is not set

The only ones among these are are actually multiplatform are axm55xx,
bcm2835, and u300. I see no downsides of force-enabling PM for
any of those, so we could decide to 'select PM' from
CONFIG_ARCH_MULTIPLATFORM.

The one usecase where we may want to have a modern machine without
CONFIG_PM is a minimal MACH_VIRT kernel for running in a virtual
machine or QEMU with minimal memory requirements, e.g. trying to
squeeze a large number of guests on a single host system.

	Arnd

[toc] | [prev] | [next] | [standalone]

#1309491

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@arndb.de> 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@nvidia.com>
>>>>> ---
>>>>>  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@lists.infradead.org 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.

Cheers
Jon

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web