2019-02-26 08:27:50

by Sameer Pujar

[permalink] [raw]
Subject: [PATCH v2] ARM: tegra: enforce PM requirement

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 <[email protected]>
Reviewed-by: Thierry Reding <[email protected]>
Reviewed-by: Jonathan Hunter <[email protected]>
---
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



2019-02-26 09:14:11

by Russell King (Oracle)

[permalink] [raw]
Subject: Re: [PATCH v2] ARM: tegra: enforce PM requirement

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 <[email protected]>
> Reviewed-by: Thierry Reding <[email protected]>
> Reviewed-by: Jonathan Hunter <[email protected]>
> ---
> 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

2019-02-26 14:47:30

by Dmitry Osipenko

[permalink] [raw]
Subject: Re: [PATCH v2] ARM: tegra: enforce PM requirement

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/

2019-02-26 16:53:01

by Dmitry Osipenko

[permalink] [raw]
Subject: Re: [PATCH v2] ARM: tegra: enforce PM requirement

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".

2019-02-27 06:15:43

by Sameer Pujar

[permalink] [raw]
Subject: Re: [PATCH v2] ARM: tegra: enforce PM requirement


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.

2019-02-27 06:18:18

by Sameer Pujar

[permalink] [raw]
Subject: Re: [PATCH v2] ARM: tegra: enforce PM requirement


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.

2019-02-27 10:43:44

by Thierry Reding

[permalink] [raw]
Subject: Re: [PATCH v2] ARM: tegra: enforce PM requirement

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 <[email protected]>
> > Reviewed-by: Thierry Reding <[email protected]>
> > Reviewed-by: Jonathan Hunter <[email protected]>
> > ---
> > 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


Attachments:
(No filename) (2.60 kB)
signature.asc (849.00 B)
Download all attachments