| Message ID | 1449241037-22193-17-git-send-email-jonathanh@nvidia.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On 4 December 2015 at 15:57, Jon Hunter <jonathanh@nvidia.com> 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 If you still want to allow ARCH_TEGRA to run without PM, you should probably change to: select PM_GENERIC_DOMAINS if PM > select RESET_CONTROLLER > help > This enables support for the NVIDIA Tegra SoC family. > -- > 2.1.4 > Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Ulf, On 15/12/15 19:54, Ulf Hansson wrote: > On 4 December 2015 at 15:57, Jon Hunter <jonathanh@nvidia.com> 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 > > If you still want to allow ARCH_TEGRA to run without PM, you should > probably change to: > > select PM_GENERIC_DOMAINS if PM Per the changelog this is deliberate. If we allow !PM, then there is a potential that you could probe a device when the power domain is not powered on. I understand that some SoCs turn on all the power-domains when !PM but this will not work for tegra because we don't register the power domain until later in the boot and so we are relying upon probe deferral to defer the probe of devices that use power-domains. Cheers Jon -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 16 December 2015 at 10:40, Jon Hunter <jonathanh@nvidia.com> wrote: > Hi Ulf, > > On 15/12/15 19:54, Ulf Hansson wrote: >> On 4 December 2015 at 15:57, Jon Hunter <jonathanh@nvidia.com> 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 >> >> If you still want to allow ARCH_TEGRA to run without PM, you should >> probably change to: >> >> select PM_GENERIC_DOMAINS if PM > > Per the changelog this is deliberate. If we allow !PM, then there is a > potential that you could probe a device when the power domain is not > powered on. I understand that some SoCs turn on all the power-domains > when !PM but this will not work for tegra because we don't register the > power domain until later in the boot and so we are relying upon probe > deferral to defer the probe of devices that use power-domains. So what you are saying is that adding the PM domain support, will fix some devices to become successfully probed as they were broken before? If that's the case, this makes all good sense! Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 16/12/15 09:47, Ulf Hansson wrote: > On 16 December 2015 at 10:40, Jon Hunter <jonathanh@nvidia.com> wrote: >> Hi Ulf, >> >> On 15/12/15 19:54, Ulf Hansson wrote: >>> On 4 December 2015 at 15:57, Jon Hunter <jonathanh@nvidia.com> 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 >>> >>> If you still want to allow ARCH_TEGRA to run without PM, you should >>> probably change to: >>> >>> select PM_GENERIC_DOMAINS if PM >> >> Per the changelog this is deliberate. If we allow !PM, then there is a >> potential that you could probe a device when the power domain is not >> powered on. I understand that some SoCs turn on all the power-domains >> when !PM but this will not work for tegra because we don't register the >> power domain until later in the boot and so we are relying upon probe >> deferral to defer the probe of devices that use power-domains. > > So what you are saying is that adding the PM domain support, will fix > some devices to become successfully probed as they were broken before? Not exactly. There is a legacy tegra_powergate_sequence_power_up() that has been used to date to get around this. However, by migrating to GENPD we really need to make PM mandatory, otherwise you could attempt to probe a device in a power domain that is not powered. In other words, you are probing blindly. I know some SoCs do this, but that does not seem very robust. Cheers Jon -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 16 December 2015 at 12:40, Jon Hunter <jonathanh@nvidia.com> wrote: > > On 16/12/15 09:47, Ulf Hansson wrote: >> On 16 December 2015 at 10:40, Jon Hunter <jonathanh@nvidia.com> wrote: >>> Hi Ulf, >>> >>> On 15/12/15 19:54, Ulf Hansson wrote: >>>> On 4 December 2015 at 15:57, Jon Hunter <jonathanh@nvidia.com> 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 >>>> >>>> If you still want to allow ARCH_TEGRA to run without PM, you should >>>> probably change to: >>>> >>>> select PM_GENERIC_DOMAINS if PM >>> >>> Per the changelog this is deliberate. If we allow !PM, then there is a >>> potential that you could probe a device when the power domain is not >>> powered on. I understand that some SoCs turn on all the power-domains >>> when !PM but this will not work for tegra because we don't register the >>> power domain until later in the boot and so we are relying upon probe >>> deferral to defer the probe of devices that use power-domains. >> >> So what you are saying is that adding the PM domain support, will fix >> some devices to become successfully probed as they were broken before? > > Not exactly. There is a legacy tegra_powergate_sequence_power_up() that > has been used to date to get around this. However, by migrating to GENPD > we really need to make PM mandatory, otherwise you could attempt to > probe a device in a power domain that is not powered. In other words, > you are probing blindly. I know some SoCs do this, but that does not > seem very robust. Thank for the clarification. I agree. You may add my: Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org> Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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
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 -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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
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 -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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@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. 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 -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jan 14, 2016 at 12:11:39PM +0100, Arnd Bergmann wrote: > 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. If we make ARCH_TEGRA select PM, then moving to a multi-platform kernel isn't automatically going to increase the image size. The image size is only going to increase if you select ARCH_TEGRA to be part of the multi platform image. > 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. Yeah, there's also a fair amount of per-SoC code that can't be built as a module and which will be included in multi-platform images when the corresponding ARCH_* symbol is enabled. But I think that's inevitable given the purpose of multi-platform images. > 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. ARCH_MULTIPLATFORM selecting PM would include PM unconditionally, even if none of the selected platforms require it. In my opinion an explicit select from platforms that require PM would be cleaner. It could be that once we start doing that for a single platform others might follow. When this becomes common place it might be worth moving it up a level, but I think explicit dependencies would be better for starters. > 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. Right, but those won't be multi-platform kernels, right? If you know exactly that the kernel will run in a virtual machine and that you want to run lots of virtual machines, you probably want to build a custom kernel. Thierry
Thierry Reding <thierry.reding@gmail.com> writes: > On Thu, Jan 14, 2016 at 12:11:39PM +0100, Arnd Bergmann wrote: >> 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. > > If we make ARCH_TEGRA select PM, then moving to a multi-platform kernel > isn't automatically going to increase the image size. The image size is > only going to increase if you select ARCH_TEGRA to be part of the multi > platform image. > >> 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. > > Yeah, there's also a fair amount of per-SoC code that can't be built as > a module and which will be included in multi-platform images when the > corresponding ARCH_* symbol is enabled. But I think that's inevitable > given the purpose of multi-platform images. > >> 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. > > ARCH_MULTIPLATFORM selecting PM would include PM unconditionally, even > if none of the selected platforms require it. In my opinion an explicit > select from platforms that require PM would be cleaner. I agree. Doing it this way also points you exactly at which arch(es) needs to be disabled if you want to build a !PM multi-plaform kernel. > It could be that once we start doing that for a single platform others > might follow. I suspect so as well. The main reason we're not there already is that full PM support for most platforms remains out of tree. > When this becomes common place it might be worth moving it up a level, > but I think explicit dependencies would be better for starters. +1 Kevin -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[...] > > 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? Yes, this is too me the best option. Additionally, for those tegra SoCs/platforms that needs PM_GENERIC_DOMAINS, it can safely select that as well. > > 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 think Rafael already made his point around this approach, which is a clear no. And I fully agree with it. Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.
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(+)