[PATCH v2] ARM: tegra: enforce PM requirement

[PATCH v2] ARM: tegra: enforce PM requirement

[Sameer Pujar]

The requirement for this came while adding runtime PM support for HDA
driver. There were concerns about driver explicitly handling !PM case.
In general, drivers need to handle !PM case with work arounds for
managing clocks and power explicitly, which is not really necessary
when PM support on tegra is in good shape. In fact ARM 64-bit Tegra
platforms enforce PM support and there is no reason why this cannot be
done for 32-bit.

More details with regards to above can be found in following patch,
http://patchwork.ozlabs.org/patch/1036645/

This patch selects PM unconditionally and drivers can rely on runtime
PM framework for clock and power management.

Signed-off-by: Sameer Pujar <spujar@nvidia.com>
Reviewed-by: Thierry Reding <treding@nvidia.com>
Reviewed-by: Jonathan Hunter <jonathanh@nvidia.com>
---
 arch/arm/mach-tegra/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/mach-tegra/Kconfig b/arch/arm/mach-tegra/Kconfig
index 7f3b83e..51a8fa3 100644
--- a/arch/arm/mach-tegra/Kconfig
+++ b/arch/arm/mach-tegra/Kconfig
@@ -10,6 +10,7 @@ menuconfig ARCH_TEGRA
 	select HAVE_ARM_SCU if SMP
 	select HAVE_ARM_TWD if SMP
 	select PINCTRL
+	select PM
 	select PM_OPP
 	select ARCH_HAS_RESET_CONTROLLER
 	select RESET_CONTROLLER
-- 
2.7.4

Re: [PATCH v2] ARM: tegra: enforce PM requirement

[Russell King - ARM Linux admin]

On Tue, Feb 26, 2019 at 01:55:37PM +0530, Sameer Pujar wrote:
> The requirement for this came while adding runtime PM support for HDA
> driver. There were concerns about driver explicitly handling !PM case.
> In general, drivers need to handle !PM case with work arounds for
> managing clocks and power explicitly, which is not really necessary
> when PM support on tegra is in good shape. In fact ARM 64-bit Tegra
> platforms enforce PM support and there is no reason why this cannot be
> done for 32-bit.
> 
> More details with regards to above can be found in following patch,
> http://patchwork.ozlabs.org/patch/1036645/
> 
> This patch selects PM unconditionally and drivers can rely on runtime
> PM framework for clock and power management.

What if the drivers are re-used on another SoC IP?  Doesn't this lead
to unexpected failures?

If you want to do this, maybe also make those drivers depend on PM as
well?

> Signed-off-by: Sameer Pujar <spujar@nvidia.com>
> Reviewed-by: Thierry Reding <treding@nvidia.com>
> Reviewed-by: Jonathan Hunter <jonathanh@nvidia.com>
> ---
>  arch/arm/mach-tegra/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm/mach-tegra/Kconfig b/arch/arm/mach-tegra/Kconfig
> index 7f3b83e..51a8fa3 100644
> --- a/arch/arm/mach-tegra/Kconfig
> +++ b/arch/arm/mach-tegra/Kconfig
> @@ -10,6 +10,7 @@ menuconfig ARCH_TEGRA
>  	select HAVE_ARM_SCU if SMP
>  	select HAVE_ARM_TWD if SMP
>  	select PINCTRL
> +	select PM
>  	select PM_OPP
>  	select ARCH_HAS_RESET_CONTROLLER
>  	select RESET_CONTROLLER
> -- 
> 2.7.4
> 
> 

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

Re: [PATCH v2] ARM: tegra: enforce PM requirement

[Dmitry Osipenko]

26.02.2019 12:13, Russell King - ARM Linux admin пишет:
> On Tue, Feb 26, 2019 at 01:55:37PM +0530, Sameer Pujar wrote:
>> The requirement for this came while adding runtime PM support for HDA
>> driver. There were concerns about driver explicitly handling !PM case.
>> In general, drivers need to handle !PM case with work arounds for
>> managing clocks and power explicitly, which is not really necessary
>> when PM support on tegra is in good shape. In fact ARM 64-bit Tegra
>> platforms enforce PM support and there is no reason why this cannot be
>> done for 32-bit.
>>
>> More details with regards to above can be found in following patch,
>> http://patchwork.ozlabs.org/patch/1036645/
>>
>> This patch selects PM unconditionally and drivers can rely on runtime
>> PM framework for clock and power management.
> 
> What if the drivers are re-used on another SoC IP?  Doesn't this lead
> to unexpected failures?
> 
> If you want to do this, maybe also make those drivers depend on PM as
> well?

The commit message is inaccurate, it is intended for the Tegra HDA driver and not for some generic driver. The overall final intent is to remove dependency on the PM availability for all of Tegra drivers to "make Tegra maintainers life easier".

Re: [PATCH v2] ARM: tegra: enforce PM requirement

[Sameer Pujar]

On 2/26/2019 10:22 PM, Dmitry Osipenko wrote:
> 26.02.2019 12:13, Russell King - ARM Linux admin пишет:
>> On Tue, Feb 26, 2019 at 01:55:37PM +0530, Sameer Pujar wrote:
>>> The requirement for this came while adding runtime PM support for HDA
>>> driver. There were concerns about driver explicitly handling !PM case.
>>> In general, drivers need to handle !PM case with work arounds for
>>> managing clocks and power explicitly, which is not really necessary
>>> when PM support on tegra is in good shape. In fact ARM 64-bit Tegra
>>> platforms enforce PM support and there is no reason why this cannot be
>>> done for 32-bit.
>>>
>>> More details with regards to above can be found in following patch,
>>> http://patchwork.ozlabs.org/patch/1036645/
>>>
>>> This patch selects PM unconditionally and drivers can rely on runtime
>>> PM framework for clock and power management.
>> What if the drivers are re-used on another SoC IP?  Doesn't this lead
>> to unexpected failures?
>>
>> If you want to do this, maybe also make those drivers depend on PM as
>> well?
> The commit message is inaccurate, it is intended for the Tegra HDA driver and not for some generic driver. The overall final intent is to remove dependency on the PM availability for all of Tegra drivers to "make Tegra maintainers life easier".
Wanted to convey that finally it would be the case for all Tegra 
drivers. I will update commit message to make it more clear.

Re: [PATCH v2] ARM: tegra: enforce PM requirement

[Thierry Reding]

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

On Tue, Feb 26, 2019 at 09:13:08AM +0000, Russell King - ARM Linux admin wrote:
> On Tue, Feb 26, 2019 at 01:55:37PM +0530, Sameer Pujar wrote:
> > The requirement for this came while adding runtime PM support for HDA
> > driver. There were concerns about driver explicitly handling !PM case.
> > In general, drivers need to handle !PM case with work arounds for
> > managing clocks and power explicitly, which is not really necessary
> > when PM support on tegra is in good shape. In fact ARM 64-bit Tegra
> > platforms enforce PM support and there is no reason why this cannot be
> > done for 32-bit.
> > 
> > More details with regards to above can be found in following patch,
> > http://patchwork.ozlabs.org/patch/1036645/
> > 
> > This patch selects PM unconditionally and drivers can rely on runtime
> > PM framework for clock and power management.
> 
> What if the drivers are re-used on another SoC IP?  Doesn't this lead
> to unexpected failures?

I suppose it would if the configuration doesn't enable PM. In practice I
don't think this happens very often. I'm also not aware of any of the IP
blocks in Tegra being reused on other SoCs, so I think the risk of this
happening isn't very high.

> If you want to do this, maybe also make those drivers depend on PM as
> well?

That said, I think adding the dependency on PM is good documentation and
would prevent the cases that you're describing from happening, so I'll
see to it that we get it added where needed.

I think all drivers currently support running without PM, but the code
to do it is cumbersome and highly repetitive. I think we can add the PM
dependency at the same time that we remove the !PM boilerplate.

Thierry

> > Signed-off-by: Sameer Pujar <spujar@nvidia.com>
> > Reviewed-by: Thierry Reding <treding@nvidia.com>
> > Reviewed-by: Jonathan Hunter <jonathanh@nvidia.com>
> > ---
> >  arch/arm/mach-tegra/Kconfig | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/arch/arm/mach-tegra/Kconfig b/arch/arm/mach-tegra/Kconfig
> > index 7f3b83e..51a8fa3 100644
> > --- a/arch/arm/mach-tegra/Kconfig
> > +++ b/arch/arm/mach-tegra/Kconfig
> > @@ -10,6 +10,7 @@ menuconfig ARCH_TEGRA
> >  	select HAVE_ARM_SCU if SMP
> >  	select HAVE_ARM_TWD if SMP
> >  	select PINCTRL
> > +	select PM
> >  	select PM_OPP
> >  	select ARCH_HAS_RESET_CONTROLLER
> >  	select RESET_CONTROLLER
> > -- 
> > 2.7.4
> > 
> > 
> 
> -- 
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
> According to speedtest.net: 11.9Mbps down 500kbps up

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

Re: [PATCH v2] ARM: tegra: enforce PM requirement

[Dmitry Osipenko]

26.02.2019 11:25, Sameer Pujar пишет:
> The requirement for this came while adding runtime PM support for HDA
> driver. There were concerns about driver explicitly handling !PM case.
> In general, drivers need to handle !PM case with work arounds for
> managing clocks and power explicitly, which is not really necessary
> when PM support on tegra is in good shape. In fact ARM 64-bit Tegra
> platforms enforce PM support and there is no reason why this cannot be
> done for 32-bit.
> 
> More details with regards to above can be found in following patch,
> http://patchwork.ozlabs.org/patch/1036645/

I think you meant this: https://patchwork.ozlabs.org/patch/1031007/

Re: [PATCH v2] ARM: tegra: enforce PM requirement

[Sameer Pujar]

On 2/26/2019 8:16 PM, Dmitry Osipenko wrote:
> 26.02.2019 11:25, Sameer Pujar пишет:
>> The requirement for this came while adding runtime PM support for HDA
>> driver. There were concerns about driver explicitly handling !PM case.
>> In general, drivers need to handle !PM case with work arounds for
>> managing clocks and power explicitly, which is not really necessary
>> when PM support on tegra is in good shape. In fact ARM 64-bit Tegra
>> platforms enforce PM support and there is no reason why this cannot be
>> done for 32-bit.
>>
>> More details with regards to above can be found in following patch,
>> http://patchwork.ozlabs.org/patch/1036645/
> I think you meant this: https://patchwork.ozlabs.org/patch/1031007/
my bad! Thanks for pointing.