[RFC PATCH v2 0/5] Allow genpd providers to power off domains on sync state

[RFC PATCH v2 0/5] Allow genpd providers to power off domains on sync state

[Abel Vesa]

There have been already a couple of tries to make the genpd "disable
unused" late initcall skip the powering off of domains that might be
needed until later on (i.e. until some consumer probes). The conclusion
was that the provider could return -EBUSY from the power_off callback
until the provider's sync state has been reached. This patch series tries
to provide a proof-of-concept that is working on Qualcomm platforms.

I've been doing extensive testing on SM8450, but I've also spinned this
on my X13s (SC8280XP). Both patches that add the sync state callback to
the SC8280XP and SM8450 are here to provide context. Once we agree on
the form, I intend to add the sync state callback to all gdsc providers.

Currently, some of the gdsc providers might not reach sync state due to
list of consumers not probing yet (or at all). The sync state can be
enforced by writing 1 to the state_synced sysfs attribute of the
provider, thanks to Saravana's commit [1] which has been already merged.

[1] https://lore.kernel.org/r/20230304005355.746421-3-saravanak@google.com

V1 of this patchset was here:
https://lore.kernel.org/all/20230315132330.450877-1-abel.vesa@linaro.org/

Changes since v1:
 * Added the qcom_cc sync state callback which calls in turn the gdsc one
 * dropped extra semicolon from pm_domain.h

Abel Vesa (5):
  PM: domains: Allow power off queuing from providers
  soc: qcom: rpmhpd: Do proper power off when state synced
  clk: qcom: gdsc: Avoid actual power off until sync state
  clk: qcom: Add sync state callback to all SC8280XP providers
  clk: qcom: Add sync state callback to all SM8450 providers

 drivers/base/power/domain.c        |  3 ++-
 drivers/clk/qcom/camcc-sm8450.c    |  1 +
 drivers/clk/qcom/common.c          | 19 +++++++++++++++++++
 drivers/clk/qcom/common.h          |  2 ++
 drivers/clk/qcom/dispcc-sc8280xp.c |  1 +
 drivers/clk/qcom/dispcc-sm8450.c   |  1 +
 drivers/clk/qcom/gcc-sc8280xp.c    |  1 +
 drivers/clk/qcom/gcc-sm8450.c      |  1 +
 drivers/clk/qcom/gdsc.c            | 26 ++++++++++++++++++++++++++
 drivers/clk/qcom/gdsc.h            |  6 ++++++
 drivers/clk/qcom/gpucc-sc8280xp.c  |  1 +
 drivers/soc/qcom/rpmhpd.c          | 19 +++++++------------
 include/linux/pm_domain.h          |  6 ++++++
 13 files changed, 74 insertions(+), 13 deletions(-)

-- 
2.34.1

[RFC PATCH v2 1/5] PM: domains: Allow power off queuing from providers

[Abel Vesa]

In some cases, the providers might choose to refuse powering off some
domains until all of the consumer have had a chance to probe, that is,
until sync state callback has been called. Such providers might choose
to disable such domains on their on, from the sync state callback. So,
in order to do that, they need a way to queue up a power off request.
Since the generic genpd already has such API, make that available to
those providers.

Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
---
 drivers/base/power/domain.c | 3 ++-
 include/linux/pm_domain.h   | 6 ++++++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 32084e38b73d..97d4e2f2da91 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -649,10 +649,11 @@ static int _genpd_power_off(struct generic_pm_domain *genpd, bool timed)
  * Queue up the execution of genpd_power_off() unless it's already been done
  * before.
  */
-static void genpd_queue_power_off_work(struct generic_pm_domain *genpd)
+void genpd_queue_power_off_work(struct generic_pm_domain *genpd)
 {
 	queue_work(pm_wq, &genpd->power_off_work);
 }
+EXPORT_SYMBOL_GPL(genpd_queue_power_off_work);
 
 /**
  * genpd_power_off - Remove power from a given PM domain.
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index f776fb93eaa0..f9729640f87e 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -231,6 +231,7 @@ int pm_genpd_remove_subdomain(struct generic_pm_domain *genpd,
 int pm_genpd_init(struct generic_pm_domain *genpd,
 		  struct dev_power_governor *gov, bool is_off);
 int pm_genpd_remove(struct generic_pm_domain *genpd);
+void genpd_queue_power_off_work(struct generic_pm_domain *genpd);
 int dev_pm_genpd_set_performance_state(struct device *dev, unsigned int state);
 int dev_pm_genpd_add_notifier(struct device *dev, struct notifier_block *nb);
 int dev_pm_genpd_remove_notifier(struct device *dev);
@@ -278,6 +279,11 @@ static inline int pm_genpd_remove(struct generic_pm_domain *genpd)
 	return -EOPNOTSUPP;
 }
 
+void genpd_queue_power_off_work(struct generic_pm_domain *genpd)
+{
+	return -EOPNOTSUPP;
+}
+
 static inline int dev_pm_genpd_set_performance_state(struct device *dev,
 						     unsigned int state)
 {
-- 
2.34.1

Re: [RFC PATCH v2 1/5] PM: domains: Allow power off queuing from providers

[Ulf Hansson]

On Mon, 20 Mar 2023 at 14:42, Abel Vesa <abel.vesa@linaro.org> wrote:
>
> In some cases, the providers might choose to refuse powering off some
> domains until all of the consumer have had a chance to probe, that is,

/s/consumer/consumers

> until sync state callback has been called. Such providers might choose
> to disable such domains on their on, from the sync state callback. So,

/s/their on/their own

> in order to do that, they need a way to queue up a power off request.
> Since the generic genpd already has such API, make that available to
> those providers.
>
> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> ---
>  drivers/base/power/domain.c | 3 ++-
>  include/linux/pm_domain.h   | 6 ++++++
>  2 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 32084e38b73d..97d4e2f2da91 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -649,10 +649,11 @@ static int _genpd_power_off(struct generic_pm_domain *genpd, bool timed)
>   * Queue up the execution of genpd_power_off() unless it's already been done
>   * before.
>   */
> -static void genpd_queue_power_off_work(struct generic_pm_domain *genpd)

Please add a function description - and make sure to state that its
external use is explicitly intended for being called from genpd
providers ->sync_state callbacks.

> +void genpd_queue_power_off_work(struct generic_pm_domain *genpd)

As this becomes an exported function, we should also conform to
genpd's function naming rules, which is to use the "pm_genpd_*"
prefix.

While renaming it, perhaps it's sufficient with
"pm_genpd_queue_power_off" or maybe even better "pm_genpd_sync_state",
what do you think?

>  {
>         queue_work(pm_wq, &genpd->power_off_work);
>  }
> +EXPORT_SYMBOL_GPL(genpd_queue_power_off_work);
>
>  /**
>   * genpd_power_off - Remove power from a given PM domain.
> diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
> index f776fb93eaa0..f9729640f87e 100644
> --- a/include/linux/pm_domain.h
> +++ b/include/linux/pm_domain.h
> @@ -231,6 +231,7 @@ int pm_genpd_remove_subdomain(struct generic_pm_domain *genpd,
>  int pm_genpd_init(struct generic_pm_domain *genpd,
>                   struct dev_power_governor *gov, bool is_off);
>  int pm_genpd_remove(struct generic_pm_domain *genpd);
> +void genpd_queue_power_off_work(struct generic_pm_domain *genpd);
>  int dev_pm_genpd_set_performance_state(struct device *dev, unsigned int state);
>  int dev_pm_genpd_add_notifier(struct device *dev, struct notifier_block *nb);
>  int dev_pm_genpd_remove_notifier(struct device *dev);
> @@ -278,6 +279,11 @@ static inline int pm_genpd_remove(struct generic_pm_domain *genpd)
>         return -EOPNOTSUPP;
>  }
>
> +void genpd_queue_power_off_work(struct generic_pm_domain *genpd)
> +{
> +       return -EOPNOTSUPP;
> +}
> +
>  static inline int dev_pm_genpd_set_performance_state(struct device *dev,
>                                                      unsigned int state)
>  {
> --
> 2.34.1
>

Other than the minor things above, the approach looks reasonable to me!

Kind regards
Uffe

[RFC PATCH v2 2/5] soc: qcom: rpmhpd: Do proper power off when state synced

[Abel Vesa]

Instead of aggregating different corner values on sync state callback,
call the genpd API for queuing up the power off. This will also mark the
domain as powered off in the debugfs genpd summary. Also, until sync
state has been reached, return busy on power off request, in order to
allow genpd core to know that the actual domain hasn't been powered of
from the "disable unused" late initcall.

Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
---
 drivers/soc/qcom/rpmhpd.c | 19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/drivers/soc/qcom/rpmhpd.c b/drivers/soc/qcom/rpmhpd.c
index f20e2a49a669..abd999c74783 100644
--- a/drivers/soc/qcom/rpmhpd.c
+++ b/drivers/soc/qcom/rpmhpd.c
@@ -649,8 +649,12 @@ static int rpmhpd_power_off(struct generic_pm_domain *domain)
 	mutex_lock(&rpmhpd_lock);
 
 	ret = rpmhpd_aggregate_corner(pd, 0);
-	if (!ret)
-		pd->enabled = false;
+	if (!ret) {
+		if (!pd->state_synced)
+			ret = -EBUSY;
+		else
+			pd->enabled = false;
+	}
 
 	mutex_unlock(&rpmhpd_lock);
 
@@ -810,10 +814,8 @@ static void rpmhpd_sync_state(struct device *dev)
 {
 	const struct rpmhpd_desc *desc = of_device_get_match_data(dev);
 	struct rpmhpd **rpmhpds = desc->rpmhpds;
-	unsigned int corner;
 	struct rpmhpd *pd;
 	unsigned int i;
-	int ret;
 
 	mutex_lock(&rpmhpd_lock);
 	for (i = 0; i < desc->num_pds; i++) {
@@ -822,14 +824,7 @@ static void rpmhpd_sync_state(struct device *dev)
 			continue;
 
 		pd->state_synced = true;
-		if (pd->enabled)
-			corner = max(pd->corner, pd->enable_corner);
-		else
-			corner = 0;
-
-		ret = rpmhpd_aggregate_corner(pd, corner);
-		if (ret)
-			dev_err(dev, "failed to sync %s\n", pd->res_name);
+		genpd_queue_power_off_work(&pd->pd);
 	}
 	mutex_unlock(&rpmhpd_lock);
 }
-- 
2.34.1

[RFC PATCH v2 3/5] clk: qcom: gdsc: Avoid actual power off until sync state

[Abel Vesa]

In case there is a sync state callback registered for a provider,
do not actually power off any gdsc for that provider until sync state
has been reached and return busy instead. Since the qcom_cc is
private, add a helper that returns the gdsc_desc based on the device of
the provider. Finally, add the generic gdsc sync state callback to be
used by the platform specific providers.

Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
---
 drivers/clk/qcom/common.c | 19 +++++++++++++++++++
 drivers/clk/qcom/common.h |  2 ++
 drivers/clk/qcom/gdsc.c   | 26 ++++++++++++++++++++++++++
 drivers/clk/qcom/gdsc.h   |  6 ++++++
 4 files changed, 53 insertions(+)

diff --git a/drivers/clk/qcom/common.c b/drivers/clk/qcom/common.c
index 75f09e6e057e..d7fd1b170c1c 100644
--- a/drivers/clk/qcom/common.c
+++ b/drivers/clk/qcom/common.c
@@ -20,6 +20,7 @@
 struct qcom_cc {
 	struct qcom_reset_controller reset;
 	struct clk_regmap **rclks;
+	struct gdsc_desc *scd;
 	size_t num_rclks;
 };
 
@@ -234,6 +235,13 @@ static struct clk_hw *qcom_cc_clk_hw_get(struct of_phandle_args *clkspec,
 	return cc->rclks[idx] ? &cc->rclks[idx]->hw : NULL;
 }
 
+struct gdsc_desc *qcom_cc_get_gdsc_desc(struct device *dev)
+{
+	struct qcom_cc *cc = dev_get_drvdata(dev);
+
+	return cc->scd;
+}
+
 int qcom_cc_really_probe(struct platform_device *pdev,
 			 const struct qcom_cc_desc *desc, struct regmap *regmap)
 {
@@ -251,6 +259,8 @@ int qcom_cc_really_probe(struct platform_device *pdev,
 	if (!cc)
 		return -ENOMEM;
 
+	dev_set_drvdata(dev, cc);
+
 	reset = &cc->reset;
 	reset->rcdev.of_node = dev->of_node;
 	reset->rcdev.ops = &qcom_reset_ops;
@@ -267,6 +277,9 @@ int qcom_cc_really_probe(struct platform_device *pdev,
 		scd = devm_kzalloc(dev, sizeof(*scd), GFP_KERNEL);
 		if (!scd)
 			return -ENOMEM;
+
+		cc->scd = scd;
+
 		scd->dev = dev;
 		scd->scs = desc->gdscs;
 		scd->num = desc->num_gdscs;
@@ -319,6 +332,12 @@ int qcom_cc_probe(struct platform_device *pdev, const struct qcom_cc_desc *desc)
 }
 EXPORT_SYMBOL_GPL(qcom_cc_probe);
 
+void qcom_cc_sync_state(struct device *dev)
+{
+	gdsc_sync_state(dev);
+}
+EXPORT_SYMBOL_GPL(qcom_cc_sync_state);
+
 int qcom_cc_probe_by_index(struct platform_device *pdev, int index,
 			   const struct qcom_cc_desc *desc)
 {
diff --git a/drivers/clk/qcom/common.h b/drivers/clk/qcom/common.h
index 9c8f7b798d9f..1bea04da0a00 100644
--- a/drivers/clk/qcom/common.h
+++ b/drivers/clk/qcom/common.h
@@ -61,9 +61,11 @@ extern struct regmap *qcom_cc_map(struct platform_device *pdev,
 extern int qcom_cc_really_probe(struct platform_device *pdev,
 				const struct qcom_cc_desc *desc,
 				struct regmap *regmap);
+extern struct gdsc_desc *qcom_cc_get_gdsc_desc(struct device *dev);
 extern int qcom_cc_probe(struct platform_device *pdev,
 			 const struct qcom_cc_desc *desc);
 extern int qcom_cc_probe_by_index(struct platform_device *pdev, int index,
 				  const struct qcom_cc_desc *desc);
+extern void qcom_cc_sync_state(struct device *dev);
 
 #endif
diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c
index 5358e28122ab..af745907dc49 100644
--- a/drivers/clk/qcom/gdsc.c
+++ b/drivers/clk/qcom/gdsc.c
@@ -15,6 +15,8 @@
 #include <linux/regulator/consumer.h>
 #include <linux/reset-controller.h>
 #include <linux/slab.h>
+
+#include "common.h"
 #include "gdsc.h"
 
 #define PWR_ON_MASK		BIT(31)
@@ -319,6 +321,9 @@ static int gdsc_disable(struct generic_pm_domain *domain)
 	struct gdsc *sc = domain_to_gdsc(domain);
 	int ret;
 
+	if (!sc->state_synced)
+		return -EBUSY;
+
 	if (sc->pwrsts == PWRSTS_ON)
 		return gdsc_assert_reset(sc);
 
@@ -365,6 +370,7 @@ static int gdsc_disable(struct generic_pm_domain *domain)
 
 static int gdsc_init(struct gdsc *sc)
 {
+	struct device *dev = sc->dev;
 	u32 mask, val;
 	int on, ret;
 
@@ -452,6 +458,9 @@ static int gdsc_init(struct gdsc *sc)
 	if (!sc->pd.power_on)
 		sc->pd.power_on = gdsc_enable;
 
+	if (!dev_has_sync_state(dev))
+		sc->state_synced = true;
+
 	ret = pm_genpd_init(&sc->pd, NULL, !on);
 	if (ret)
 		goto err_disable_supply;
@@ -496,6 +505,7 @@ int gdsc_register(struct gdsc_desc *desc,
 	for (i = 0; i < num; i++) {
 		if (!scs[i])
 			continue;
+		scs[i]->dev = dev;
 		scs[i]->regmap = regmap;
 		scs[i]->rcdev = rcdev;
 		ret = gdsc_init(scs[i]);
@@ -536,6 +546,22 @@ void gdsc_unregister(struct gdsc_desc *desc)
 	of_genpd_del_provider(dev->of_node);
 }
 
+void gdsc_sync_state(struct device *dev)
+{
+	struct gdsc_desc *scd = qcom_cc_get_gdsc_desc(dev);
+	struct gdsc **scs = scd->scs;
+	size_t num = scd->num;
+	int i;
+
+	for (i = 0; i < num; i++) {
+		if (!scs[i])
+			continue;
+
+		scs[i]->state_synced = true;
+		genpd_queue_power_off_work(&scs[i]->pd);
+	}
+}
+
 /*
  * On SDM845+ the GPU GX domain is *almost* entirely controlled by the GMU
  * running in the CX domain so the CPU doesn't need to know anything about the
diff --git a/drivers/clk/qcom/gdsc.h b/drivers/clk/qcom/gdsc.h
index 803512688336..e1c902caecde 100644
--- a/drivers/clk/qcom/gdsc.h
+++ b/drivers/clk/qcom/gdsc.h
@@ -35,6 +35,7 @@ struct gdsc {
 	struct generic_pm_domain	pd;
 	struct generic_pm_domain	*parent;
 	struct regmap			*regmap;
+	struct device			*dev;
 	unsigned int			gdscr;
 	unsigned int			collapse_ctrl;
 	unsigned int			collapse_mask;
@@ -73,6 +74,8 @@ struct gdsc {
 
 	const char 			*supply;
 	struct regulator		*rsupply;
+
+	bool				state_synced;
 };
 
 struct gdsc_desc {
@@ -86,6 +89,7 @@ int gdsc_register(struct gdsc_desc *desc, struct reset_controller_dev *,
 		  struct regmap *);
 void gdsc_unregister(struct gdsc_desc *desc);
 int gdsc_gx_do_nothing_enable(struct generic_pm_domain *domain);
+void gdsc_sync_state(struct device *dev);
 #else
 static inline int gdsc_register(struct gdsc_desc *desc,
 				struct reset_controller_dev *rcdev,
@@ -94,6 +98,8 @@ static inline int gdsc_register(struct gdsc_desc *desc,
 	return -ENOSYS;
 }
 
+static inline void gdsc_sync_state(struct device *dev) { }
+
 static inline void gdsc_unregister(struct gdsc_desc *desc) {};
 #endif /* CONFIG_QCOM_GDSC */
 #endif /* __QCOM_GDSC_H__ */
-- 
2.34.1

[RFC PATCH v2 4/5] clk: qcom: Add sync state callback to all SC8280XP providers

[Abel Vesa]

Now that we have support for sync state delayed disabling of unused
power domains and a provided generic gdsc sync state callback, add it to
all the providers related to the SC8280XP platform.

Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
---
 drivers/clk/qcom/dispcc-sc8280xp.c | 1 +
 drivers/clk/qcom/gcc-sc8280xp.c    | 1 +
 drivers/clk/qcom/gpucc-sc8280xp.c  | 1 +
 3 files changed, 3 insertions(+)

diff --git a/drivers/clk/qcom/dispcc-sc8280xp.c b/drivers/clk/qcom/dispcc-sc8280xp.c
index 167470beb369..a64c396b9cc4 100644
--- a/drivers/clk/qcom/dispcc-sc8280xp.c
+++ b/drivers/clk/qcom/dispcc-sc8280xp.c
@@ -3199,6 +3199,7 @@ static struct platform_driver disp_cc_sc8280xp_driver = {
 	.driver = {
 		.name = "disp_cc-sc8280xp",
 		.of_match_table = disp_cc_sc8280xp_match_table,
+		.sync_state = qcom_cc_sync_state,
 	},
 };
 
diff --git a/drivers/clk/qcom/gcc-sc8280xp.c b/drivers/clk/qcom/gcc-sc8280xp.c
index b3198784e1c3..64d828ba07da 100644
--- a/drivers/clk/qcom/gcc-sc8280xp.c
+++ b/drivers/clk/qcom/gcc-sc8280xp.c
@@ -7441,6 +7441,7 @@ static struct platform_driver gcc_sc8280xp_driver = {
 	.driver = {
 		.name = "gcc-sc8280xp",
 		.of_match_table = gcc_sc8280xp_match_table,
+		.sync_state = qcom_cc_sync_state,
 	},
 };
 
diff --git a/drivers/clk/qcom/gpucc-sc8280xp.c b/drivers/clk/qcom/gpucc-sc8280xp.c
index ea1e9505c335..46ca242ba427 100644
--- a/drivers/clk/qcom/gpucc-sc8280xp.c
+++ b/drivers/clk/qcom/gpucc-sc8280xp.c
@@ -453,6 +453,7 @@ static struct platform_driver gpu_cc_sc8280xp_driver = {
 	.driver = {
 		.name = "gpu_cc-sc8280xp",
 		.of_match_table = gpu_cc_sc8280xp_match_table,
+		.sync_state = qcom_cc_sync_state,
 	},
 };
 module_platform_driver(gpu_cc_sc8280xp_driver);
-- 
2.34.1

[RFC PATCH v2 5/5] clk: qcom: Add sync state callback to all SM8450 providers

[Abel Vesa]

Now that we have support for sync state delayed disabling of unused
power domains and a provided generic gdsc sync state callback, add it to
all the providers related to the SM8450 platform.

Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
---
 drivers/clk/qcom/camcc-sm8450.c  | 1 +
 drivers/clk/qcom/dispcc-sm8450.c | 1 +
 drivers/clk/qcom/gcc-sm8450.c    | 1 +
 3 files changed, 3 insertions(+)

diff --git a/drivers/clk/qcom/camcc-sm8450.c b/drivers/clk/qcom/camcc-sm8450.c
index 51338a2884d2..b2c6109c7eba 100644
--- a/drivers/clk/qcom/camcc-sm8450.c
+++ b/drivers/clk/qcom/camcc-sm8450.c
@@ -2847,6 +2847,7 @@ static struct platform_driver cam_cc_sm8450_driver = {
 	.driver = {
 		.name = "camcc-sm8450",
 		.of_match_table = cam_cc_sm8450_match_table,
+		.sync_state = qcom_cc_sync_state,
 	},
 };
 
diff --git a/drivers/clk/qcom/dispcc-sm8450.c b/drivers/clk/qcom/dispcc-sm8450.c
index adbfd30bfc96..0ea719940a8e 100644
--- a/drivers/clk/qcom/dispcc-sm8450.c
+++ b/drivers/clk/qcom/dispcc-sm8450.c
@@ -1803,6 +1803,7 @@ static struct platform_driver disp_cc_sm8450_driver = {
 	.driver = {
 		.name = "disp_cc-sm8450",
 		.of_match_table = disp_cc_sm8450_match_table,
+		.sync_state = qcom_cc_sync_state,
 	},
 };
 
diff --git a/drivers/clk/qcom/gcc-sm8450.c b/drivers/clk/qcom/gcc-sm8450.c
index 84764cc3db4f..248709fb975e 100644
--- a/drivers/clk/qcom/gcc-sm8450.c
+++ b/drivers/clk/qcom/gcc-sm8450.c
@@ -3262,6 +3262,7 @@ static struct platform_driver gcc_sm8450_driver = {
 	.driver = {
 		.name = "gcc-sm8450",
 		.of_match_table = gcc_sm8450_match_table,
+		.sync_state = qcom_cc_sync_state,
 	},
 };
 
-- 
2.34.1

Re: [RFC PATCH v2 0/5] Allow genpd providers to power off domains on sync state

[Ulf Hansson]

On Mon, 20 Mar 2023 at 14:42, Abel Vesa <abel.vesa@linaro.org> wrote:
>
> There have been already a couple of tries to make the genpd "disable
> unused" late initcall skip the powering off of domains that might be
> needed until later on (i.e. until some consumer probes). The conclusion
> was that the provider could return -EBUSY from the power_off callback
> until the provider's sync state has been reached. This patch series tries
> to provide a proof-of-concept that is working on Qualcomm platforms.
>
> I've been doing extensive testing on SM8450, but I've also spinned this
> on my X13s (SC8280XP). Both patches that add the sync state callback to
> the SC8280XP and SM8450 are here to provide context. Once we agree on
> the form, I intend to add the sync state callback to all gdsc providers.
>
> Currently, some of the gdsc providers might not reach sync state due to
> list of consumers not probing yet (or at all). The sync state can be
> enforced by writing 1 to the state_synced sysfs attribute of the
> provider, thanks to Saravana's commit [1] which has been already merged.
>
> [1] https://lore.kernel.org/r/20230304005355.746421-3-saravanak@google.com
>
> V1 of this patchset was here:
> https://lore.kernel.org/all/20230315132330.450877-1-abel.vesa@linaro.org/
>
> Changes since v1:
>  * Added the qcom_cc sync state callback which calls in turn the gdsc one
>  * dropped extra semicolon from pm_domain.h
>
> Abel Vesa (5):
>   PM: domains: Allow power off queuing from providers
>   soc: qcom: rpmhpd: Do proper power off when state synced
>   clk: qcom: gdsc: Avoid actual power off until sync state
>   clk: qcom: Add sync state callback to all SC8280XP providers
>   clk: qcom: Add sync state callback to all SM8450 providers
>
>  drivers/base/power/domain.c        |  3 ++-
>  drivers/clk/qcom/camcc-sm8450.c    |  1 +
>  drivers/clk/qcom/common.c          | 19 +++++++++++++++++++
>  drivers/clk/qcom/common.h          |  2 ++
>  drivers/clk/qcom/dispcc-sc8280xp.c |  1 +
>  drivers/clk/qcom/dispcc-sm8450.c   |  1 +
>  drivers/clk/qcom/gcc-sc8280xp.c    |  1 +
>  drivers/clk/qcom/gcc-sm8450.c      |  1 +
>  drivers/clk/qcom/gdsc.c            | 26 ++++++++++++++++++++++++++
>  drivers/clk/qcom/gdsc.h            |  6 ++++++
>  drivers/clk/qcom/gpucc-sc8280xp.c  |  1 +
>  drivers/soc/qcom/rpmhpd.c          | 19 +++++++------------
>  include/linux/pm_domain.h          |  6 ++++++
>  13 files changed, 74 insertions(+), 13 deletions(-)
>

Besides the minor comments on patch1, this looks good to me! So, feel
free to add:

Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>

Kind regards
Uffe