[PATCH] firmware: imx: scu-pd: Update comments for single global power domain

[PATCH] firmware: imx: scu-pd: Update comments for single global power domain

[Ulf Hansson]

Since the introduction of the PM domain support for the scu-pd, the genpd
framework has been continuously improved. More preciously, using a single
global power domain can quite easily be deployed for imx platforms.

To avoid confusions, let's therefore make an update to the comments about
the missing pieces.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/firmware/imx/scu-pd.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/firmware/imx/scu-pd.c b/drivers/firmware/imx/scu-pd.c
index 08533ee67626..6e178a63442d 100644
--- a/drivers/firmware/imx/scu-pd.c
+++ b/drivers/firmware/imx/scu-pd.c
@@ -29,6 +29,10 @@
  *    The framework needs some proper extension to support multi power
  *    domain cases.
  *
+ *    Update: Genpd assigns the ->of_node for the virtual device before it
+ *    invokes ->attach_dev() callback, hence parsing for device resources via
+ *    DT should work fine.
+ *
  * 2. It also breaks most of current drivers as the driver probe sequence
  *    behavior changed if removing ->power_on|off() callback and use
  *    ->start() and ->stop() instead. genpd_dev_pm_attach will only power
@@ -39,8 +43,11 @@
  *    domain enabled will trigger a HW access error. That means we need fix
  *    most drivers probe sequence with proper runtime pm.
  *
- * In summary, we need fix above two issue before being able to switch to
- * the "single global power domain" way.
+ *    Update: Runtime PM support isn't necessary. Instead, this can easily be
+ *    fixed in drivers by adding a call to dev_pm_domain_start() during probe.
+ *
+ * In summary, the second part needs to be addressed via minor updates to the
+ * relevant drivers, before the "single global power domain" model can be used.
  *
  */
 
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

RE: [PATCH] firmware: imx: scu-pd: Update comments for single global power domain

[Aisheng Dong]

Hi Ulf,

> From: Ulf Hansson <ulf.hansson@linaro.org>
> Sent: Wednesday, March 17, 2021 5:31 PM
> 
> Since the introduction of the PM domain support for the scu-pd, the genpd
> framework has been continuously improved. More preciously, using a single
> global power domain can quite easily be deployed for imx platforms.
> 
> To avoid confusions, let's therefore make an update to the comments about
> the missing pieces.
> 
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>

Thanks for the update.
Reviewed-by: Dong Aisheng <aisheng.dong@nxp.com>

BTW, I'm still uncertain if the new approach can finally work well for i.MX as SCU PD
also supports multiple low power state.
I could investigate it more when I adding multi low power states support.

Regards
Aisheng

> ---
>  drivers/firmware/imx/scu-pd.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/firmware/imx/scu-pd.c b/drivers/firmware/imx/scu-pd.c
> index 08533ee67626..6e178a63442d 100644
> --- a/drivers/firmware/imx/scu-pd.c
> +++ b/drivers/firmware/imx/scu-pd.c
> @@ -29,6 +29,10 @@
>   *    The framework needs some proper extension to support multi power
>   *    domain cases.
>   *
> + *    Update: Genpd assigns the ->of_node for the virtual device before it
> + *    invokes ->attach_dev() callback, hence parsing for device resources via
> + *    DT should work fine.
> + *
>   * 2. It also breaks most of current drivers as the driver probe sequence
>   *    behavior changed if removing ->power_on|off() callback and use
>   *    ->start() and ->stop() instead. genpd_dev_pm_attach will only power
> @@ -39,8 +43,11 @@
>   *    domain enabled will trigger a HW access error. That means we need fix
>   *    most drivers probe sequence with proper runtime pm.
>   *
> - * In summary, we need fix above two issue before being able to switch to
> - * the "single global power domain" way.
> + *    Update: Runtime PM support isn't necessary. Instead, this can easily be
> + *    fixed in drivers by adding a call to dev_pm_domain_start() during
> probe.
> + *
> + * In summary, the second part needs to be addressed via minor updates
> + to the
> + * relevant drivers, before the "single global power domain" model can be
> used.
>   *
>   */
> 
> --
> 2.25.1

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] firmware: imx: scu-pd: Update comments for single global power domain

[Ulf Hansson]

On Fri, 19 Mar 2021 at 04:31, Aisheng Dong <aisheng.dong@nxp.com> wrote:
>
> Hi Ulf,
>
> > From: Ulf Hansson <ulf.hansson@linaro.org>
> > Sent: Wednesday, March 17, 2021 5:31 PM
> >
> > Since the introduction of the PM domain support for the scu-pd, the genpd
> > framework has been continuously improved. More preciously, using a single
> > global power domain can quite easily be deployed for imx platforms.
> >
> > To avoid confusions, let's therefore make an update to the comments about
> > the missing pieces.
> >
> > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>
> Thanks for the update.
> Reviewed-by: Dong Aisheng <aisheng.dong@nxp.com>
>
> BTW, I'm still uncertain if the new approach can finally work well for i.MX as SCU PD
> also supports multiple low power state.
> I could investigate it more when I adding multi low power states support.

The multiple low power states are currently only supported per genpd
and not per device. So, yes, in principle you could have one genpd per
device as you currently model it, to support this.

Although, thinking long term wise, we probably want something else
that doesn't rely on the device to be attached to a genpd to support
this use case.

In the past, I have been talking to different people from various SoC
vendors and it looks like the use case is there, but it's kind of
messy to support it. I would certainly be very interested to hear
about your use case, would you mind sharing some more about this?

Moreover, note that, there is a limitation in the genpd infrastructure
when you build a hierarchy with parent/childs genpds, when each genpd
has multiple idle states. That is, a parent-genpd may be allowed to
enter any of its idle states, no matter what idle state that has been
selected for the child-genpd. As a matter of fact, I am about to fix
this problem quite soon. Is perhaps this something that could be
valuable for your platforms too?

[...]

Kind regards
Uffe

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] firmware: imx: scu-pd: Update comments for single global power domain

[Shawn Guo]

On Wed, Mar 17, 2021 at 10:31:17AM +0100, Ulf Hansson wrote:
> Since the introduction of the PM domain support for the scu-pd, the genpd
> framework has been continuously improved. More preciously, using a single
> global power domain can quite easily be deployed for imx platforms.
> 
> To avoid confusions, let's therefore make an update to the comments about
> the missing pieces.
> 
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>

Applied, thanks.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel