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

From: Aisheng Dong <aisheng.dong@nxp.com>
To: Ulf Hansson <ulf.hansson@linaro.org>,
	Shawn Guo <shawnguo@kernel.org>,
	Sascha Hauer <s.hauer@pengutronix.de>
Cc: Ying Liu <victor.liu@nxp.com>, Peng Fan <peng.fan@nxp.com>,
	dl-linux-imx <linux-imx@nxp.com>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH] firmware: imx: scu-pd: Update comments for single global power domain
Date: Fri, 19 Mar 2021 03:31:39 +0000	[thread overview]
Message-ID: <AM6PR04MB4966C6D0E08AB4C25B280CF480689@AM6PR04MB4966.eurprd04.prod.outlook.com> (raw)
In-Reply-To: <20210317093117.16091-1-ulf.hansson@linaro.org>

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