RE: [PATCH] Revert "firmware: arm_scmi: Add clock management to the SCMI power domain"

[Peng Fan]

> Subject: [PATCH] Revert "firmware: arm_scmi: Add clock management to
> the SCMI power domain"
> 
> This reverts commit a3b884cef873 ("firmware: arm_scmi: Add clock
> management to the SCMI power domain").
> 
> Using the GENPD_FLAG_PM_CLK tells genpd to gate/ungate the consumer
> device's clock(s) during runtime suspend/resume through the PM clock API.
> More precisely, in genpd_runtime_resume() the clock(s) for the consumer
> device would become ungated prior to the driver-level ->runtime_resume()
> callbacks gets invoked.
> 
> This behaviour isn't a good fit for all platforms/drivers. For example, a
> driver may need to make some preparations of its device in its
> ->runtime_resume() callback, like calling clk_set_rate() before the
> clock(s) should be ungated. In these cases, it's easier to let the clock(s) to be
> managed solely by the driver, rather than at the PM domain level.
> 
> For these reasons, let's drop the use GENPD_FLAG_PM_CLK for the SCMI PM
> domain, as to enable it to be more easily adopted across ARM platforms.
> 
> Fixes: a3b884cef873 ("firmware: arm_scmi: Add clock management to the
> SCMI power domain")
> Cc: Nicolas Pitre <npitre@xxxxxxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Ulf Hansson <ulf.hansson@xxxxxxxxxx>

Tested-by: Peng Fan <peng.fan@xxxxxxx>

Thanks,
Peng.

> ---
> 
> To get some more background to $subject patch, please have a look at the
> lore-link below.
> 
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.
> kernel.org%2Fall%2FDU0PR04MB94173B45A2CFEE3BF1BD313A88409%40D
> U0PR04MB9417.eurprd04.prod.outlook.com%2F&amp;data=05%7C01%7Cp
> eng.fan%40nxp.com%7C05437bc19cb14760450a08da9a3968c0%7C686ea1d
> 3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637991868616277658%7CUnkn
> own%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik
> 1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=QpQfK8lTApY
> qhYj80lUudQJMCujYyN9j1RNB4Q00wrM%3D&amp;reserved=0
> 
> Kind regards
> Ulf Hansson
> 
> ---
>  drivers/firmware/arm_scmi/scmi_pm_domain.c | 26 ----------------------
>  1 file changed, 26 deletions(-)
> 
> diff --git a/drivers/firmware/arm_scmi/scmi_pm_domain.c
> b/drivers/firmware/arm_scmi/scmi_pm_domain.c
> index 581d34c95769..d5dee625de78 100644
> --- a/drivers/firmware/arm_scmi/scmi_pm_domain.c
> +++ b/drivers/firmware/arm_scmi/scmi_pm_domain.c
> @@ -8,7 +8,6 @@
>  #include <linux/err.h>
>  #include <linux/io.h>
>  #include <linux/module.h>
> -#include <linux/pm_clock.h>
>  #include <linux/pm_domain.h>
>  #include <linux/scmi_protocol.h>
> 
> @@ -53,27 +52,6 @@ static int scmi_pd_power_off(struct
> generic_pm_domain *domain)
>  	return scmi_pd_power(domain, false);
>  }
> 
> -static int scmi_pd_attach_dev(struct generic_pm_domain *pd, struct device
> *dev) -{
> -	int ret;
> -
> -	ret = pm_clk_create(dev);
> -	if (ret)
> -		return ret;
> -
> -	ret = of_pm_clk_add_clks(dev);
> -	if (ret >= 0)
> -		return 0;
> -
> -	pm_clk_destroy(dev);
> -	return ret;
> -}
> -
> -static void scmi_pd_detach_dev(struct generic_pm_domain *pd, struct
> device *dev) -{
> -	pm_clk_destroy(dev);
> -}
> -
>  static int scmi_pm_domain_probe(struct scmi_device *sdev)  {
>  	int num_domains, i;
> @@ -124,10 +102,6 @@ static int scmi_pm_domain_probe(struct
> scmi_device *sdev)
>  		scmi_pd->genpd.name = scmi_pd->name;
>  		scmi_pd->genpd.power_off = scmi_pd_power_off;
>  		scmi_pd->genpd.power_on = scmi_pd_power_on;
> -		scmi_pd->genpd.attach_dev = scmi_pd_attach_dev;
> -		scmi_pd->genpd.detach_dev = scmi_pd_detach_dev;
> -		scmi_pd->genpd.flags = GENPD_FLAG_PM_CLK |
> -				       GENPD_FLAG_ACTIVE_WAKEUP;
> 
>  		pm_genpd_init(&scmi_pd->genpd, NULL,
>  			      state == SCMI_POWER_STATE_GENERIC_OFF);
> --
> 2.34.1