2023-03-27 19:45:26

by Abel Vesa

Subject: [PATCH v3 0/4] Allow genpd providers to power off domains on sync state

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/[email protected]

V2 (RFC) of this patchset was here:
https://lore.kernel.org/all/[email protected]/

Changes since v2:
* renamed genpd_queue_power_off_work to pm_genpd_queue_power_off and added
comment about its purpose w.r.t. it being exported.
* added the qcom_cc generic sync state callback to all providers that
register GDSCs, instead of SM8450 and SC8280XP

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 (4):
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 providers

drivers/base/power/domain.c | 18 ++++++++++--------
drivers/clk/qcom/apss-ipq6018.c | 1 +
drivers/clk/qcom/camcc-sc7180.c | 1 +
drivers/clk/qcom/camcc-sc7280.c | 1 +
drivers/clk/qcom/camcc-sdm845.c | 1 +
drivers/clk/qcom/camcc-sm6350.c | 1 +
drivers/clk/qcom/camcc-sm8250.c | 1 +
drivers/clk/qcom/camcc-sm8450.c | 1 +
drivers/clk/qcom/common.c | 19 +++++++++++++++++++
drivers/clk/qcom/common.h | 2 ++
drivers/clk/qcom/dispcc-qcm2290.c | 1 +
drivers/clk/qcom/dispcc-sc7180.c | 1 +
drivers/clk/qcom/dispcc-sc7280.c | 1 +
drivers/clk/qcom/dispcc-sc8280xp.c | 1 +
drivers/clk/qcom/dispcc-sdm845.c | 1 +
drivers/clk/qcom/dispcc-sm6115.c | 1 +
drivers/clk/qcom/dispcc-sm6125.c | 1 +
drivers/clk/qcom/dispcc-sm6350.c | 1 +
drivers/clk/qcom/dispcc-sm6375.c | 1 +
drivers/clk/qcom/dispcc-sm8250.c | 1 +
drivers/clk/qcom/dispcc-sm8450.c | 1 +
drivers/clk/qcom/dispcc-sm8550.c | 1 +
drivers/clk/qcom/gcc-apq8084.c | 1 +
drivers/clk/qcom/gcc-ipq806x.c | 1 +
drivers/clk/qcom/gcc-ipq8074.c | 1 +
drivers/clk/qcom/gcc-mdm9615.c | 1 +
drivers/clk/qcom/gcc-msm8660.c | 1 +
drivers/clk/qcom/gcc-msm8909.c | 1 +
drivers/clk/qcom/gcc-msm8916.c | 1 +
drivers/clk/qcom/gcc-msm8939.c | 1 +
drivers/clk/qcom/gcc-msm8953.c | 1 +
drivers/clk/qcom/gcc-msm8960.c | 1 +
drivers/clk/qcom/gcc-msm8974.c | 1 +
drivers/clk/qcom/gcc-msm8976.c | 1 +
drivers/clk/qcom/gcc-msm8994.c | 1 +
drivers/clk/qcom/gcc-msm8996.c | 1 +
drivers/clk/qcom/gcc-msm8998.c | 1 +
drivers/clk/qcom/gcc-qcm2290.c | 1 +
drivers/clk/qcom/gcc-qcs404.c | 1 +
drivers/clk/qcom/gcc-qdu1000.c | 1 +
drivers/clk/qcom/gcc-sa8775p.c | 1 +
drivers/clk/qcom/gcc-sc7180.c | 1 +
drivers/clk/qcom/gcc-sc7280.c | 1 +
drivers/clk/qcom/gcc-sc8180x.c | 1 +
drivers/clk/qcom/gcc-sc8280xp.c | 1 +
drivers/clk/qcom/gcc-sdm660.c | 1 +
drivers/clk/qcom/gcc-sdm845.c | 1 +
drivers/clk/qcom/gcc-sdx55.c | 1 +
drivers/clk/qcom/gcc-sdx65.c | 1 +
drivers/clk/qcom/gcc-sm6115.c | 1 +
drivers/clk/qcom/gcc-sm6125.c | 1 +
drivers/clk/qcom/gcc-sm6350.c | 1 +
drivers/clk/qcom/gcc-sm6375.c | 1 +
drivers/clk/qcom/gcc-sm7150.c | 1 +
drivers/clk/qcom/gcc-sm8150.c | 1 +
drivers/clk/qcom/gcc-sm8250.c | 1 +
drivers/clk/qcom/gcc-sm8350.c | 1 +
drivers/clk/qcom/gcc-sm8450.c | 1 +
drivers/clk/qcom/gcc-sm8550.c | 1 +
drivers/clk/qcom/gdsc.c | 26 ++++++++++++++++++++++++++
drivers/clk/qcom/gdsc.h | 6 ++++++
drivers/clk/qcom/gpucc-msm8998.c | 1 +
drivers/clk/qcom/gpucc-sc7180.c | 1 +
drivers/clk/qcom/gpucc-sc7280.c | 1 +
drivers/clk/qcom/gpucc-sc8280xp.c | 1 +
drivers/clk/qcom/gpucc-sdm660.c | 1 +
drivers/clk/qcom/gpucc-sdm845.c | 1 +
drivers/clk/qcom/gpucc-sm6115.c | 1 +
drivers/clk/qcom/gpucc-sm6125.c | 1 +
drivers/clk/qcom/gpucc-sm6350.c | 1 +
drivers/clk/qcom/gpucc-sm6375.c | 1 +
drivers/clk/qcom/gpucc-sm8150.c | 1 +
drivers/clk/qcom/gpucc-sm8250.c | 1 +
drivers/clk/qcom/gpucc-sm8350.c | 1 +
drivers/clk/qcom/lcc-ipq806x.c | 1 +
drivers/clk/qcom/lpassaudiocc-sc7280.c | 1 +
drivers/clk/qcom/lpasscc-sc7280.c | 1 +
drivers/clk/qcom/lpasscorecc-sc7180.c | 2 ++
drivers/clk/qcom/lpasscorecc-sc7280.c | 2 ++
drivers/clk/qcom/mmcc-apq8084.c | 1 +
drivers/clk/qcom/mmcc-msm8974.c | 1 +
drivers/clk/qcom/mmcc-msm8994.c | 1 +
drivers/clk/qcom/mmcc-msm8996.c | 1 +
drivers/clk/qcom/mmcc-msm8998.c | 1 +
drivers/clk/qcom/mmcc-sdm660.c | 1 +
drivers/clk/qcom/videocc-sc7180.c | 1 +
drivers/clk/qcom/videocc-sc7280.c | 1 +
drivers/clk/qcom/videocc-sdm845.c | 1 +
drivers/clk/qcom/videocc-sm8150.c | 1 +
drivers/clk/qcom/videocc-sm8250.c | 1 +
drivers/soc/qcom/rpmhpd.c | 19 +++++++------------
include/linux/pm_domain.h | 4 ++++
92 files changed, 161 insertions(+), 20 deletions(-)

--
2.34.1

2023-03-27 19:45:29

by Abel Vesa

Subject: [PATCH v3 2/4] soc: qcom: rpmhpd: Do proper power off when state synced

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 <[email protected]>
Reviewed-by: Ulf Hansson <[email protected]>
---
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..ec7926820772 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);
+ pm_genpd_queue_power_off(&pd->pd);
}
mutex_unlock(&rpmhpd_lock);
}
--
2.34.1

2023-03-27 19:45:42

by Abel Vesa

Subject: [PATCH v3 3/4] clk: qcom: gdsc: Avoid actual power off until sync state

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 <[email protected]>
Reviewed-by: Ulf Hansson <[email protected]>
---
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..ea7d753a38ef 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;
+ pm_genpd_queue_power_off(&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

2023-03-27 19:46:37

by Abel Vesa

Subject: [PATCH v3 4/4] clk: qcom: Add sync state callback to all providers

Now that we have support for sync state delayed disabling of unused
power domains and a provided generic qcom_cc sync state callback,
add it to all the providers for all platforms that actually registers
GDSCs.

Signed-off-by: Abel Vesa <[email protected]>
---
drivers/clk/qcom/apss-ipq6018.c | 1 +
drivers/clk/qcom/camcc-sc7180.c | 1 +
drivers/clk/qcom/camcc-sc7280.c | 1 +
drivers/clk/qcom/camcc-sdm845.c | 1 +
drivers/clk/qcom/camcc-sm6350.c | 1 +
drivers/clk/qcom/camcc-sm8250.c | 1 +
drivers/clk/qcom/camcc-sm8450.c | 1 +
drivers/clk/qcom/dispcc-qcm2290.c | 1 +
drivers/clk/qcom/dispcc-sc7180.c | 1 +
drivers/clk/qcom/dispcc-sc7280.c | 1 +
drivers/clk/qcom/dispcc-sc8280xp.c | 1 +
drivers/clk/qcom/dispcc-sdm845.c | 1 +
drivers/clk/qcom/dispcc-sm6115.c | 1 +
drivers/clk/qcom/dispcc-sm6125.c | 1 +
drivers/clk/qcom/dispcc-sm6350.c | 1 +
drivers/clk/qcom/dispcc-sm6375.c | 1 +
drivers/clk/qcom/dispcc-sm8250.c | 1 +
drivers/clk/qcom/dispcc-sm8450.c | 1 +
drivers/clk/qcom/dispcc-sm8550.c | 1 +
drivers/clk/qcom/gcc-apq8084.c | 1 +
drivers/clk/qcom/gcc-ipq806x.c | 1 +
drivers/clk/qcom/gcc-ipq8074.c | 1 +
drivers/clk/qcom/gcc-mdm9615.c | 1 +
drivers/clk/qcom/gcc-msm8660.c | 1 +
drivers/clk/qcom/gcc-msm8909.c | 1 +
drivers/clk/qcom/gcc-msm8916.c | 1 +
drivers/clk/qcom/gcc-msm8939.c | 1 +
drivers/clk/qcom/gcc-msm8953.c | 1 +
drivers/clk/qcom/gcc-msm8960.c | 1 +
drivers/clk/qcom/gcc-msm8974.c | 1 +
drivers/clk/qcom/gcc-msm8976.c | 1 +
drivers/clk/qcom/gcc-msm8994.c | 1 +
drivers/clk/qcom/gcc-msm8996.c | 1 +
drivers/clk/qcom/gcc-msm8998.c | 1 +
drivers/clk/qcom/gcc-qcm2290.c | 1 +
drivers/clk/qcom/gcc-qcs404.c | 1 +
drivers/clk/qcom/gcc-qdu1000.c | 1 +
drivers/clk/qcom/gcc-sa8775p.c | 1 +
drivers/clk/qcom/gcc-sc7180.c | 1 +
drivers/clk/qcom/gcc-sc7280.c | 1 +
drivers/clk/qcom/gcc-sc8180x.c | 1 +
drivers/clk/qcom/gcc-sc8280xp.c | 1 +
drivers/clk/qcom/gcc-sdm660.c | 1 +
drivers/clk/qcom/gcc-sdm845.c | 1 +
drivers/clk/qcom/gcc-sdx55.c | 1 +
drivers/clk/qcom/gcc-sdx65.c | 1 +
drivers/clk/qcom/gcc-sm6115.c | 1 +
drivers/clk/qcom/gcc-sm6125.c | 1 +
drivers/clk/qcom/gcc-sm6350.c | 1 +
drivers/clk/qcom/gcc-sm6375.c | 1 +
drivers/clk/qcom/gcc-sm7150.c | 1 +
drivers/clk/qcom/gcc-sm8150.c | 1 +
drivers/clk/qcom/gcc-sm8250.c | 1 +
drivers/clk/qcom/gcc-sm8350.c | 1 +
drivers/clk/qcom/gcc-sm8450.c | 1 +
drivers/clk/qcom/gcc-sm8550.c | 1 +
drivers/clk/qcom/gpucc-msm8998.c | 1 +
drivers/clk/qcom/gpucc-sc7180.c | 1 +
drivers/clk/qcom/gpucc-sc7280.c | 1 +
drivers/clk/qcom/gpucc-sc8280xp.c | 1 +
drivers/clk/qcom/gpucc-sdm660.c | 1 +
drivers/clk/qcom/gpucc-sdm845.c | 1 +
drivers/clk/qcom/gpucc-sm6115.c | 1 +
drivers/clk/qcom/gpucc-sm6125.c | 1 +
drivers/clk/qcom/gpucc-sm6350.c | 1 +
drivers/clk/qcom/gpucc-sm6375.c | 1 +
drivers/clk/qcom/gpucc-sm8150.c | 1 +
drivers/clk/qcom/gpucc-sm8250.c | 1 +
drivers/clk/qcom/gpucc-sm8350.c | 1 +
drivers/clk/qcom/lcc-ipq806x.c | 1 +
drivers/clk/qcom/lpassaudiocc-sc7280.c | 1 +
drivers/clk/qcom/lpasscc-sc7280.c | 1 +
drivers/clk/qcom/lpasscorecc-sc7180.c | 2 ++
drivers/clk/qcom/lpasscorecc-sc7280.c | 2 ++
drivers/clk/qcom/mmcc-apq8084.c | 1 +
drivers/clk/qcom/mmcc-msm8974.c | 1 +
drivers/clk/qcom/mmcc-msm8994.c | 1 +
drivers/clk/qcom/mmcc-msm8996.c | 1 +
drivers/clk/qcom/mmcc-msm8998.c | 1 +
drivers/clk/qcom/mmcc-sdm660.c | 1 +
drivers/clk/qcom/videocc-sc7180.c | 1 +
drivers/clk/qcom/videocc-sc7280.c | 1 +
drivers/clk/qcom/videocc-sdm845.c | 1 +
drivers/clk/qcom/videocc-sm8150.c | 1 +
drivers/clk/qcom/videocc-sm8250.c | 1 +
85 files changed, 87 insertions(+)

diff --git a/drivers/clk/qcom/apss-ipq6018.c b/drivers/clk/qcom/apss-ipq6018.c
index f2f502e2d5a4..bf450c82bcb3 100644
--- a/drivers/clk/qcom/apss-ipq6018.c
+++ b/drivers/clk/qcom/apss-ipq6018.c
@@ -96,6 +96,7 @@ static struct platform_driver apss_ipq6018_driver = {
.probe = apss_ipq6018_probe,
.driver = {
.name = "qcom,apss-ipq6018-clk",
+ .sync_state = qcom_cc_sync_state,
},
};

diff --git a/drivers/clk/qcom/camcc-sc7180.c b/drivers/clk/qcom/camcc-sc7180.c
index e2b4804695f3..fe3e4590aaa3 100644
--- a/drivers/clk/qcom/camcc-sc7180.c
+++ b/drivers/clk/qcom/camcc-sc7180.c
@@ -1695,6 +1695,7 @@ static struct platform_driver cam_cc_sc7180_driver = {
.name = "cam_cc-sc7180",
.of_match_table = cam_cc_sc7180_match_table,
.pm = &cam_cc_pm_ops,
+ .sync_state = qcom_cc_sync_state,
},
};

diff --git a/drivers/clk/qcom/camcc-sc7280.c b/drivers/clk/qcom/camcc-sc7280.c
index 4396fddba7a6..798adaf5ad39 100644
--- a/drivers/clk/qcom/camcc-sc7280.c
+++ b/drivers/clk/qcom/camcc-sc7280.c
@@ -2465,6 +2465,7 @@ static struct platform_driver cam_cc_sc7280_driver = {
.driver = {
.name = "cam_cc-sc7280",
.of_match_table = cam_cc_sc7280_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};

diff --git a/drivers/clk/qcom/camcc-sdm845.c b/drivers/clk/qcom/camcc-sdm845.c
index 27d44188a7ab..98cae0e24afc 100644
--- a/drivers/clk/qcom/camcc-sdm845.c
+++ b/drivers/clk/qcom/camcc-sdm845.c
@@ -1743,6 +1743,7 @@ static struct platform_driver cam_cc_sdm845_driver = {
.driver = {
.name = "sdm845-camcc",
.of_match_table = cam_cc_sdm845_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};

diff --git a/drivers/clk/qcom/camcc-sm6350.c b/drivers/clk/qcom/camcc-sm6350.c
index acba9f99d960..2536a1f94335 100644
--- a/drivers/clk/qcom/camcc-sm6350.c
+++ b/drivers/clk/qcom/camcc-sm6350.c
@@ -1887,6 +1887,7 @@ static struct platform_driver camcc_sm6350_driver = {
.driver = {
.name = "sm6350-camcc",
.of_match_table = camcc_sm6350_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};

diff --git a/drivers/clk/qcom/camcc-sm8250.c b/drivers/clk/qcom/camcc-sm8250.c
index 9b32c56a5bc5..6bc32e5441c0 100644
--- a/drivers/clk/qcom/camcc-sm8250.c
+++ b/drivers/clk/qcom/camcc-sm8250.c
@@ -2441,6 +2441,7 @@ static struct platform_driver cam_cc_sm8250_driver = {
.driver = {
.name = "cam_cc-sm8250",
.of_match_table = cam_cc_sm8250_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};

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-qcm2290.c b/drivers/clk/qcom/dispcc-qcm2290.c
index 2ebd9a02b895..bb35f79ce245 100644
--- a/drivers/clk/qcom/dispcc-qcm2290.c
+++ b/drivers/clk/qcom/dispcc-qcm2290.c
@@ -530,6 +530,7 @@ static struct platform_driver disp_cc_qcm2290_driver = {
.driver = {
.name = "dispcc-qcm2290",
.of_match_table = disp_cc_qcm2290_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};

diff --git a/drivers/clk/qcom/dispcc-sc7180.c b/drivers/clk/qcom/dispcc-sc7180.c
index 9536bfc72a43..118379921410 100644
--- a/drivers/clk/qcom/dispcc-sc7180.c
+++ b/drivers/clk/qcom/dispcc-sc7180.c
@@ -721,6 +721,7 @@ static struct platform_driver disp_cc_sc7180_driver = {
.driver = {
.name = "sc7180-dispcc",
.of_match_table = disp_cc_sc7180_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};

diff --git a/drivers/clk/qcom/dispcc-sc7280.c b/drivers/clk/qcom/dispcc-sc7280.c
index ad596d567f6a..374fcaf52514 100644
--- a/drivers/clk/qcom/dispcc-sc7280.c
+++ b/drivers/clk/qcom/dispcc-sc7280.c
@@ -892,6 +892,7 @@ static struct platform_driver disp_cc_sc7280_driver = {
.driver = {
.name = "disp_cc-sc7280",
.of_match_table = disp_cc_sc7280_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};

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/dispcc-sdm845.c b/drivers/clk/qcom/dispcc-sdm845.c
index 735adfefc379..9415a832d137 100644
--- a/drivers/clk/qcom/dispcc-sdm845.c
+++ b/drivers/clk/qcom/dispcc-sdm845.c
@@ -869,6 +869,7 @@ static struct platform_driver disp_cc_sdm845_driver = {
.driver = {
.name = "disp_cc-sdm845",
.of_match_table = disp_cc_sdm845_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};

diff --git a/drivers/clk/qcom/dispcc-sm6115.c b/drivers/clk/qcom/dispcc-sm6115.c
index 1937edf23f21..28699f7f4692 100644
--- a/drivers/clk/qcom/dispcc-sm6115.c
+++ b/drivers/clk/qcom/dispcc-sm6115.c
@@ -600,6 +600,7 @@ static struct platform_driver disp_cc_sm6115_driver = {
.driver = {
.name = "dispcc-sm6115",
.of_match_table = disp_cc_sm6115_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};

diff --git a/drivers/clk/qcom/dispcc-sm6125.c b/drivers/clk/qcom/dispcc-sm6125.c
index 87b27053ddb6..f607d1f60c05 100644
--- a/drivers/clk/qcom/dispcc-sm6125.c
+++ b/drivers/clk/qcom/dispcc-sm6125.c
@@ -690,6 +690,7 @@ static struct platform_driver disp_cc_sm6125_driver = {
.driver = {
.name = "disp_cc-sm6125",
.of_match_table = disp_cc_sm6125_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};

diff --git a/drivers/clk/qcom/dispcc-sm6350.c b/drivers/clk/qcom/dispcc-sm6350.c
index ea6f54ed846e..2a791b614dbb 100644
--- a/drivers/clk/qcom/dispcc-sm6350.c
+++ b/drivers/clk/qcom/dispcc-sm6350.c
@@ -778,6 +778,7 @@ static struct platform_driver disp_cc_sm6350_driver = {
.driver = {
.name = "disp_cc-sm6350",
.of_match_table = disp_cc_sm6350_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};

diff --git a/drivers/clk/qcom/dispcc-sm6375.c b/drivers/clk/qcom/dispcc-sm6375.c
index caa1b90a5ff2..963b1c0f9c2b 100644
--- a/drivers/clk/qcom/dispcc-sm6375.c
+++ b/drivers/clk/qcom/dispcc-sm6375.c
@@ -591,6 +591,7 @@ static struct platform_driver disp_cc_sm6375_driver = {
.driver = {
.name = "disp_cc-sm6375",
.of_match_table = disp_cc_sm6375_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};

diff --git a/drivers/clk/qcom/dispcc-sm8250.c b/drivers/clk/qcom/dispcc-sm8250.c
index e17bb8b543b5..6556404141af 100644
--- a/drivers/clk/qcom/dispcc-sm8250.c
+++ b/drivers/clk/qcom/dispcc-sm8250.c
@@ -1380,6 +1380,7 @@ static struct platform_driver disp_cc_sm8250_driver = {
.driver = {
.name = "disp_cc-sm8250",
.of_match_table = disp_cc_sm8250_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/dispcc-sm8550.c b/drivers/clk/qcom/dispcc-sm8550.c
index 1e5a11081860..60251884f655 100644
--- a/drivers/clk/qcom/dispcc-sm8550.c
+++ b/drivers/clk/qcom/dispcc-sm8550.c
@@ -1788,6 +1788,7 @@ static struct platform_driver disp_cc_sm8550_driver = {
.driver = {
.name = "disp_cc-sm8550",
.of_match_table = disp_cc_sm8550_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};

diff --git a/drivers/clk/qcom/gcc-apq8084.c b/drivers/clk/qcom/gcc-apq8084.c
index 7085d2ccae49..e89c2fd5ed2b 100644
--- a/drivers/clk/qcom/gcc-apq8084.c
+++ b/drivers/clk/qcom/gcc-apq8084.c
@@ -3642,6 +3642,7 @@ static struct platform_driver gcc_apq8084_driver = {
.driver = {
.name = "gcc-apq8084",
.of_match_table = gcc_apq8084_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};

diff --git a/drivers/clk/qcom/gcc-ipq806x.c b/drivers/clk/qcom/gcc-ipq806x.c
index 6447f3e81b55..53c14e476521 100644
--- a/drivers/clk/qcom/gcc-ipq806x.c
+++ b/drivers/clk/qcom/gcc-ipq806x.c
@@ -3428,6 +3428,7 @@ static struct platform_driver gcc_ipq806x_driver = {
.driver = {
.name = "gcc-ipq806x",
.of_match_table = gcc_ipq806x_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};

diff --git a/drivers/clk/qcom/gcc-ipq8074.c b/drivers/clk/qcom/gcc-ipq8074.c
index 6541d98c0348..f2b08ab865cb 100644
--- a/drivers/clk/qcom/gcc-ipq8074.c
+++ b/drivers/clk/qcom/gcc-ipq8074.c
@@ -4741,6 +4741,7 @@ static struct platform_driver gcc_ipq8074_driver = {
.driver = {
.name = "qcom,gcc-ipq8074",
.of_match_table = gcc_ipq8074_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};

diff --git a/drivers/clk/qcom/gcc-mdm9615.c b/drivers/clk/qcom/gcc-mdm9615.c
index 8bed02a748ab..5cdaac200378 100644
--- a/drivers/clk/qcom/gcc-mdm9615.c
+++ b/drivers/clk/qcom/gcc-mdm9615.c
@@ -1720,6 +1720,7 @@ static struct platform_driver gcc_mdm9615_driver = {
.driver = {
.name = "gcc-mdm9615",
.of_match_table = gcc_mdm9615_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};

diff --git a/drivers/clk/qcom/gcc-msm8660.c b/drivers/clk/qcom/gcc-msm8660.c
index a9eb6a9ac445..e5ff66cee197 100644
--- a/drivers/clk/qcom/gcc-msm8660.c
+++ b/drivers/clk/qcom/gcc-msm8660.c
@@ -2775,6 +2775,7 @@ static struct platform_driver gcc_msm8660_driver = {
.driver = {
.name = "gcc-msm8660",
.of_match_table = gcc_msm8660_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};

diff --git a/drivers/clk/qcom/gcc-msm8909.c b/drivers/clk/qcom/gcc-msm8909.c
index 2a00b11ce2cd..bde4992e0a7d 100644
--- a/drivers/clk/qcom/gcc-msm8909.c
+++ b/drivers/clk/qcom/gcc-msm8909.c
@@ -2711,6 +2711,7 @@ static struct platform_driver gcc_msm8909_driver = {
.driver = {
.name = "gcc-msm8909",
.of_match_table = gcc_msm8909_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};

diff --git a/drivers/clk/qcom/gcc-msm8916.c b/drivers/clk/qcom/gcc-msm8916.c
index 0c8fe19387a7..0fa2232f6f6b 100644
--- a/drivers/clk/qcom/gcc-msm8916.c
+++ b/drivers/clk/qcom/gcc-msm8916.c
@@ -3448,6 +3448,7 @@ static struct platform_driver gcc_msm8916_driver = {
.driver = {
.name = "gcc-msm8916",
.of_match_table = gcc_msm8916_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};

diff --git a/drivers/clk/qcom/gcc-msm8939.c b/drivers/clk/qcom/gcc-msm8939.c
index 7f8969a77974..ce1ea997b5ff 100644
--- a/drivers/clk/qcom/gcc-msm8939.c
+++ b/drivers/clk/qcom/gcc-msm8939.c
@@ -4013,6 +4013,7 @@ static struct platform_driver gcc_msm8939_driver = {
.driver = {
.name = "gcc-msm8939",
.of_match_table = gcc_msm8939_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};

diff --git a/drivers/clk/qcom/gcc-msm8953.c b/drivers/clk/qcom/gcc-msm8953.c
index 8aafa6591e84..313e279af413 100644
--- a/drivers/clk/qcom/gcc-msm8953.c
+++ b/drivers/clk/qcom/gcc-msm8953.c
@@ -4230,6 +4230,7 @@ static struct platform_driver gcc_msm8953_driver = {
.driver = {
.name = "gcc-msm8953",
.of_match_table = gcc_msm8953_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};

diff --git a/drivers/clk/qcom/gcc-msm8960.c b/drivers/clk/qcom/gcc-msm8960.c
index dbc7093ab9cc..07dc9ab735bd 100644
--- a/drivers/clk/qcom/gcc-msm8960.c
+++ b/drivers/clk/qcom/gcc-msm8960.c
@@ -3768,6 +3768,7 @@ static struct platform_driver gcc_msm8960_driver = {
.driver = {
.name = "gcc-msm8960",
.of_match_table = gcc_msm8960_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};

diff --git a/drivers/clk/qcom/gcc-msm8974.c b/drivers/clk/qcom/gcc-msm8974.c
index 0231c1efd286..93e81deafdee 100644
--- a/drivers/clk/qcom/gcc-msm8974.c
+++ b/drivers/clk/qcom/gcc-msm8974.c
@@ -2904,6 +2904,7 @@ static struct platform_driver gcc_msm8974_driver = {
.driver = {
.name = "gcc-msm8974",
.of_match_table = gcc_msm8974_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};

diff --git a/drivers/clk/qcom/gcc-msm8976.c b/drivers/clk/qcom/gcc-msm8976.c
index 8beb923c0e19..665f5165b066 100644
--- a/drivers/clk/qcom/gcc-msm8976.c
+++ b/drivers/clk/qcom/gcc-msm8976.c
@@ -4137,6 +4137,7 @@ static struct platform_driver gcc_msm8976_driver = {
.driver = {
.name = "qcom,gcc-msm8976",
.of_match_table = gcc_msm8976_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};

diff --git a/drivers/clk/qcom/gcc-msm8994.c b/drivers/clk/qcom/gcc-msm8994.c
index 0f52c48e89d8..09cda139a416 100644
--- a/drivers/clk/qcom/gcc-msm8994.c
+++ b/drivers/clk/qcom/gcc-msm8994.c
@@ -2712,6 +2712,7 @@ static struct platform_driver gcc_msm8994_driver = {
.driver = {
.name = "gcc-msm8994",
.of_match_table = gcc_msm8994_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};

diff --git a/drivers/clk/qcom/gcc-msm8996.c b/drivers/clk/qcom/gcc-msm8996.c
index 5e44d1bcca9e..7386026b08c1 100644
--- a/drivers/clk/qcom/gcc-msm8996.c
+++ b/drivers/clk/qcom/gcc-msm8996.c
@@ -3856,6 +3856,7 @@ static struct platform_driver gcc_msm8996_driver = {
.driver = {
.name = "gcc-msm8996",
.of_match_table = gcc_msm8996_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};

diff --git a/drivers/clk/qcom/gcc-msm8998.c b/drivers/clk/qcom/gcc-msm8998.c
index be024f8093c5..bf9ecda86946 100644
--- a/drivers/clk/qcom/gcc-msm8998.c
+++ b/drivers/clk/qcom/gcc-msm8998.c
@@ -3249,6 +3249,7 @@ static struct platform_driver gcc_msm8998_driver = {
.driver = {
.name = "gcc-msm8998",
.of_match_table = gcc_msm8998_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};

diff --git a/drivers/clk/qcom/gcc-qcm2290.c b/drivers/clk/qcom/gcc-qcm2290.c
index 096deff2ba25..5dcd4db13b69 100644
--- a/drivers/clk/qcom/gcc-qcm2290.c
+++ b/drivers/clk/qcom/gcc-qcm2290.c
@@ -3002,6 +3002,7 @@ static struct platform_driver gcc_qcm2290_driver = {
.driver = {
.name = "gcc-qcm2290",
.of_match_table = gcc_qcm2290_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};

diff --git a/drivers/clk/qcom/gcc-qcs404.c b/drivers/clk/qcom/gcc-qcs404.c
index a39c4990b29d..640ee59ca1b5 100644
--- a/drivers/clk/qcom/gcc-qcs404.c
+++ b/drivers/clk/qcom/gcc-qcs404.c
@@ -2832,6 +2832,7 @@ static struct platform_driver gcc_qcs404_driver = {
.driver = {
.name = "gcc-qcs404",
.of_match_table = gcc_qcs404_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};

diff --git a/drivers/clk/qcom/gcc-qdu1000.c b/drivers/clk/qcom/gcc-qdu1000.c
index 5051769ad90c..bea26044df30 100644
--- a/drivers/clk/qcom/gcc-qdu1000.c
+++ b/drivers/clk/qcom/gcc-qdu1000.c
@@ -2634,6 +2634,7 @@ static struct platform_driver gcc_qdu1000_driver = {
.driver = {
.name = "gcc-qdu1000",
.of_match_table = gcc_qdu1000_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};

diff --git a/drivers/clk/qcom/gcc-sa8775p.c b/drivers/clk/qcom/gcc-sa8775p.c
index bb94ff367abd..ff101e1f0163 100644
--- a/drivers/clk/qcom/gcc-sa8775p.c
+++ b/drivers/clk/qcom/gcc-sa8775p.c
@@ -4766,6 +4766,7 @@ static struct platform_driver gcc_sa8775p_driver = {
.driver = {
.name = "sa8775p-gcc",
.of_match_table = gcc_sa8775p_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};

diff --git a/drivers/clk/qcom/gcc-sc7180.c b/drivers/clk/qcom/gcc-sc7180.c
index cef3c77564cf..7f36d7017e14 100644
--- a/drivers/clk/qcom/gcc-sc7180.c
+++ b/drivers/clk/qcom/gcc-sc7180.c
@@ -2469,6 +2469,7 @@ static struct platform_driver gcc_sc7180_driver = {
.driver = {
.name = "gcc-sc7180",
.of_match_table = gcc_sc7180_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};

diff --git a/drivers/clk/qcom/gcc-sc7280.c b/drivers/clk/qcom/gcc-sc7280.c
index 1dc804154031..c6e10e174811 100644
--- a/drivers/clk/qcom/gcc-sc7280.c
+++ b/drivers/clk/qcom/gcc-sc7280.c
@@ -3480,6 +3480,7 @@ static struct platform_driver gcc_sc7280_driver = {
.driver = {
.name = "gcc-sc7280",
.of_match_table = gcc_sc7280_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};

diff --git a/drivers/clk/qcom/gcc-sc8180x.c b/drivers/clk/qcom/gcc-sc8180x.c
index c41b9f010585..61bcc73e2703 100644
--- a/drivers/clk/qcom/gcc-sc8180x.c
+++ b/drivers/clk/qcom/gcc-sc8180x.c
@@ -4610,6 +4610,7 @@ static struct platform_driver gcc_sc8180x_driver = {
.driver = {
.name = "gcc-sc8180x",
.of_match_table = gcc_sc8180x_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/gcc-sdm660.c b/drivers/clk/qcom/gcc-sdm660.c
index db918c92a522..a2839e893cb1 100644
--- a/drivers/clk/qcom/gcc-sdm660.c
+++ b/drivers/clk/qcom/gcc-sdm660.c
@@ -2485,6 +2485,7 @@ static struct platform_driver gcc_sdm660_driver = {
.driver = {
.name = "gcc-sdm660",
.of_match_table = gcc_sdm660_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};

diff --git a/drivers/clk/qcom/gcc-sdm845.c b/drivers/clk/qcom/gcc-sdm845.c
index 6af08e0ca847..2142e0c21d03 100644
--- a/drivers/clk/qcom/gcc-sdm845.c
+++ b/drivers/clk/qcom/gcc-sdm845.c
@@ -4020,6 +4020,7 @@ static struct platform_driver gcc_sdm845_driver = {
.driver = {
.name = "gcc-sdm845",
.of_match_table = gcc_sdm845_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};

diff --git a/drivers/clk/qcom/gcc-sdx55.c b/drivers/clk/qcom/gcc-sdx55.c
index d5e17122698c..4aa29c08459c 100644
--- a/drivers/clk/qcom/gcc-sdx55.c
+++ b/drivers/clk/qcom/gcc-sdx55.c
@@ -1628,6 +1628,7 @@ static struct platform_driver gcc_sdx55_driver = {
.driver = {
.name = "gcc-sdx55",
.of_match_table = gcc_sdx55_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};

diff --git a/drivers/clk/qcom/gcc-sdx65.c b/drivers/clk/qcom/gcc-sdx65.c
index b0c17043551d..50530f61225a 100644
--- a/drivers/clk/qcom/gcc-sdx65.c
+++ b/drivers/clk/qcom/gcc-sdx65.c
@@ -1591,6 +1591,7 @@ static struct platform_driver gcc_sdx65_driver = {
.driver = {
.name = "gcc-sdx65",
.of_match_table = gcc_sdx65_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};

diff --git a/drivers/clk/qcom/gcc-sm6115.c b/drivers/clk/qcom/gcc-sm6115.c
index 5b8222fea2f7..f6b9d560ec40 100644
--- a/drivers/clk/qcom/gcc-sm6115.c
+++ b/drivers/clk/qcom/gcc-sm6115.c
@@ -3512,6 +3512,7 @@ static struct platform_driver gcc_sm6115_driver = {
.driver = {
.name = "gcc-sm6115",
.of_match_table = gcc_sm6115_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};

diff --git a/drivers/clk/qcom/gcc-sm6125.c b/drivers/clk/qcom/gcc-sm6125.c
index 40ad062d1bf7..1825deb1f460 100644
--- a/drivers/clk/qcom/gcc-sm6125.c
+++ b/drivers/clk/qcom/gcc-sm6125.c
@@ -4170,6 +4170,7 @@ static struct platform_driver gcc_sm6125_driver = {
.driver = {
.name = "gcc-sm6125",
.of_match_table = gcc_sm6125_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};

diff --git a/drivers/clk/qcom/gcc-sm6350.c b/drivers/clk/qcom/gcc-sm6350.c
index 9b4e4bb05963..c30dd75372a0 100644
--- a/drivers/clk/qcom/gcc-sm6350.c
+++ b/drivers/clk/qcom/gcc-sm6350.c
@@ -2566,6 +2566,7 @@ static struct platform_driver gcc_sm6350_driver = {
.driver = {
.name = "gcc-sm6350",
.of_match_table = gcc_sm6350_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};

diff --git a/drivers/clk/qcom/gcc-sm6375.c b/drivers/clk/qcom/gcc-sm6375.c
index 417a0fd242ec..01954b8880c0 100644
--- a/drivers/clk/qcom/gcc-sm6375.c
+++ b/drivers/clk/qcom/gcc-sm6375.c
@@ -3902,6 +3902,7 @@ static struct platform_driver gcc_sm6375_driver = {
.driver = {
.name = "gcc-sm6375",
.of_match_table = gcc_sm6375_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};

diff --git a/drivers/clk/qcom/gcc-sm7150.c b/drivers/clk/qcom/gcc-sm7150.c
index 6b628178f62c..48c7d42d494b 100644
--- a/drivers/clk/qcom/gcc-sm7150.c
+++ b/drivers/clk/qcom/gcc-sm7150.c
@@ -3029,6 +3029,7 @@ static struct platform_driver gcc_sm7150_driver = {
.driver = {
.name = "gcc-sm7150",
.of_match_table = gcc_sm7150_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};

diff --git a/drivers/clk/qcom/gcc-sm8150.c b/drivers/clk/qcom/gcc-sm8150.c
index 70b067f3618c..feb22808a03d 100644
--- a/drivers/clk/qcom/gcc-sm8150.c
+++ b/drivers/clk/qcom/gcc-sm8150.c
@@ -3795,6 +3795,7 @@ static struct platform_driver gcc_sm8150_driver = {
.driver = {
.name = "gcc-sm8150",
.of_match_table = gcc_sm8150_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};

diff --git a/drivers/clk/qcom/gcc-sm8250.c b/drivers/clk/qcom/gcc-sm8250.c
index b6cf4bc88d4d..3508ddaaad67 100644
--- a/drivers/clk/qcom/gcc-sm8250.c
+++ b/drivers/clk/qcom/gcc-sm8250.c
@@ -3668,6 +3668,7 @@ static struct platform_driver gcc_sm8250_driver = {
.driver = {
.name = "gcc-sm8250",
.of_match_table = gcc_sm8250_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};

diff --git a/drivers/clk/qcom/gcc-sm8350.c b/drivers/clk/qcom/gcc-sm8350.c
index af4a1ea28421..f0eb2e804113 100644
--- a/drivers/clk/qcom/gcc-sm8350.c
+++ b/drivers/clk/qcom/gcc-sm8350.c
@@ -3855,6 +3855,7 @@ static struct platform_driver gcc_sm8350_driver = {
.driver = {
.name = "sm8350-gcc",
.of_match_table = gcc_sm8350_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,
},
};

diff --git a/drivers/clk/qcom/gcc-sm8550.c b/drivers/clk/qcom/gcc-sm8550.c
index 277cd4f020ff..6c65634dbe54 100644
--- a/drivers/clk/qcom/gcc-sm8550.c
+++ b/drivers/clk/qcom/gcc-sm8550.c
@@ -3368,6 +3368,7 @@ static struct platform_driver gcc_sm8550_driver = {
.driver = {
.name = "gcc-sm8550",
.of_match_table = gcc_sm8550_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};

diff --git a/drivers/clk/qcom/gpucc-msm8998.c b/drivers/clk/qcom/gpucc-msm8998.c
index f929e0f2333f..39a5924fb532 100644
--- a/drivers/clk/qcom/gpucc-msm8998.c
+++ b/drivers/clk/qcom/gpucc-msm8998.c
@@ -343,6 +343,7 @@ static struct platform_driver gpucc_msm8998_driver = {
.driver = {
.name = "gpucc-msm8998",
.of_match_table = gpucc_msm8998_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};
module_platform_driver(gpucc_msm8998_driver);
diff --git a/drivers/clk/qcom/gpucc-sc7180.c b/drivers/clk/qcom/gpucc-sc7180.c
index 3f92f0b43be6..a760d09c5385 100644
--- a/drivers/clk/qcom/gpucc-sc7180.c
+++ b/drivers/clk/qcom/gpucc-sc7180.c
@@ -249,6 +249,7 @@ static struct platform_driver gpu_cc_sc7180_driver = {
.driver = {
.name = "sc7180-gpucc",
.of_match_table = gpu_cc_sc7180_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};

diff --git a/drivers/clk/qcom/gpucc-sc7280.c b/drivers/clk/qcom/gpucc-sc7280.c
index 1490cd45a654..b466dcee81cb 100644
--- a/drivers/clk/qcom/gpucc-sc7280.c
+++ b/drivers/clk/qcom/gpucc-sc7280.c
@@ -473,6 +473,7 @@ static struct platform_driver gpu_cc_sc7280_driver = {
.driver = {
.name = "gpu_cc-sc7280",
.of_match_table = gpu_cc_sc7280_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);
diff --git a/drivers/clk/qcom/gpucc-sdm660.c b/drivers/clk/qcom/gpucc-sdm660.c
index d6b38a0b063d..9c5d5a3ce120 100644
--- a/drivers/clk/qcom/gpucc-sdm660.c
+++ b/drivers/clk/qcom/gpucc-sdm660.c
@@ -339,6 +339,7 @@ static struct platform_driver gpucc_sdm660_driver = {
.driver = {
.name = "gpucc-sdm660",
.of_match_table = gpucc_sdm660_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};
module_platform_driver(gpucc_sdm660_driver);
diff --git a/drivers/clk/qcom/gpucc-sdm845.c b/drivers/clk/qcom/gpucc-sdm845.c
index 970d7414bdf0..b293fe136fe4 100644
--- a/drivers/clk/qcom/gpucc-sdm845.c
+++ b/drivers/clk/qcom/gpucc-sdm845.c
@@ -200,6 +200,7 @@ static struct platform_driver gpu_cc_sdm845_driver = {
.driver = {
.name = "sdm845-gpucc",
.of_match_table = gpu_cc_sdm845_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};

diff --git a/drivers/clk/qcom/gpucc-sm6115.c b/drivers/clk/qcom/gpucc-sm6115.c
index c84727e8352d..f2ef128d4606 100644
--- a/drivers/clk/qcom/gpucc-sm6115.c
+++ b/drivers/clk/qcom/gpucc-sm6115.c
@@ -495,6 +495,7 @@ static struct platform_driver gpu_cc_sm6115_driver = {
.driver = {
.name = "sm6115-gpucc",
.of_match_table = gpu_cc_sm6115_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};
module_platform_driver(gpu_cc_sm6115_driver);
diff --git a/drivers/clk/qcom/gpucc-sm6125.c b/drivers/clk/qcom/gpucc-sm6125.c
index d4f1296a48ef..49981fcca06d 100644
--- a/drivers/clk/qcom/gpucc-sm6125.c
+++ b/drivers/clk/qcom/gpucc-sm6125.c
@@ -416,6 +416,7 @@ static struct platform_driver gpu_cc_sm6125_driver = {
.driver = {
.name = "gpucc-sm6125",
.of_match_table = gpu_cc_sm6125_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};
module_platform_driver(gpu_cc_sm6125_driver);
diff --git a/drivers/clk/qcom/gpucc-sm6350.c b/drivers/clk/qcom/gpucc-sm6350.c
index ef15185a99c3..f17138f787c8 100644
--- a/drivers/clk/qcom/gpucc-sm6350.c
+++ b/drivers/clk/qcom/gpucc-sm6350.c
@@ -502,6 +502,7 @@ static struct platform_driver gpu_cc_sm6350_driver = {
.driver = {
.name = "sm6350-gpucc",
.of_match_table = gpu_cc_sm6350_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};

diff --git a/drivers/clk/qcom/gpucc-sm6375.c b/drivers/clk/qcom/gpucc-sm6375.c
index d8f4c4b59f1b..839877c32947 100644
--- a/drivers/clk/qcom/gpucc-sm6375.c
+++ b/drivers/clk/qcom/gpucc-sm6375.c
@@ -449,6 +449,7 @@ static struct platform_driver gpucc_sm6375_driver = {
.driver = {
.name = "gpucc-sm6375",
.of_match_table = gpucc_sm6375_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};
module_platform_driver(gpucc_sm6375_driver);
diff --git a/drivers/clk/qcom/gpucc-sm8150.c b/drivers/clk/qcom/gpucc-sm8150.c
index 8422fd047493..dd502ffcbe05 100644
--- a/drivers/clk/qcom/gpucc-sm8150.c
+++ b/drivers/clk/qcom/gpucc-sm8150.c
@@ -312,6 +312,7 @@ static struct platform_driver gpu_cc_sm8150_driver = {
.driver = {
.name = "sm8150-gpucc",
.of_match_table = gpu_cc_sm8150_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};

diff --git a/drivers/clk/qcom/gpucc-sm8250.c b/drivers/clk/qcom/gpucc-sm8250.c
index 9c1f8ce32da4..ab407589c8f0 100644
--- a/drivers/clk/qcom/gpucc-sm8250.c
+++ b/drivers/clk/qcom/gpucc-sm8250.c
@@ -328,6 +328,7 @@ static struct platform_driver gpu_cc_sm8250_driver = {
.driver = {
.name = "sm8250-gpucc",
.of_match_table = gpu_cc_sm8250_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};

diff --git a/drivers/clk/qcom/gpucc-sm8350.c b/drivers/clk/qcom/gpucc-sm8350.c
index 5367ce654ac9..9c0ae16df813 100644
--- a/drivers/clk/qcom/gpucc-sm8350.c
+++ b/drivers/clk/qcom/gpucc-sm8350.c
@@ -618,6 +618,7 @@ static struct platform_driver gpu_cc_sm8350_driver = {
.driver = {
.name = "sm8350-gpucc",
.of_match_table = gpu_cc_sm8350_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};

diff --git a/drivers/clk/qcom/lcc-ipq806x.c b/drivers/clk/qcom/lcc-ipq806x.c
index 81a44a9a9abc..76f9483c7371 100644
--- a/drivers/clk/qcom/lcc-ipq806x.c
+++ b/drivers/clk/qcom/lcc-ipq806x.c
@@ -463,6 +463,7 @@ static struct platform_driver lcc_ipq806x_driver = {
.driver = {
.name = "lcc-ipq806x",
.of_match_table = lcc_ipq806x_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};
module_platform_driver(lcc_ipq806x_driver);
diff --git a/drivers/clk/qcom/lpassaudiocc-sc7280.c b/drivers/clk/qcom/lpassaudiocc-sc7280.c
index 1339f9211a14..30d7d7fb88ff 100644
--- a/drivers/clk/qcom/lpassaudiocc-sc7280.c
+++ b/drivers/clk/qcom/lpassaudiocc-sc7280.c
@@ -863,6 +863,7 @@ static struct platform_driver lpass_aon_cc_sc7280_driver = {
.name = "lpass_aon_cc-sc7280",
.of_match_table = lpass_aon_cc_sc7280_match_table,
.pm = &lpass_audio_cc_pm_ops,
+ .sync_state = qcom_cc_sync_state,
},
};

diff --git a/drivers/clk/qcom/lpasscc-sc7280.c b/drivers/clk/qcom/lpasscc-sc7280.c
index 48432010ce24..e48e7406e5b4 100644
--- a/drivers/clk/qcom/lpasscc-sc7280.c
+++ b/drivers/clk/qcom/lpasscc-sc7280.c
@@ -154,6 +154,7 @@ static struct platform_driver lpass_cc_sc7280_driver = {
.driver = {
.name = "sc7280-lpasscc",
.of_match_table = lpass_cc_sc7280_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};

diff --git a/drivers/clk/qcom/lpasscorecc-sc7180.c b/drivers/clk/qcom/lpasscorecc-sc7180.c
index 010867dcc2ef..27f10dcdba13 100644
--- a/drivers/clk/qcom/lpasscorecc-sc7180.c
+++ b/drivers/clk/qcom/lpasscorecc-sc7180.c
@@ -469,6 +469,7 @@ static struct platform_driver lpass_core_cc_sc7180_driver = {
.name = "lpass_core_cc-sc7180",
.of_match_table = lpass_core_cc_sc7180_match_table,
.pm = &lpass_pm_ops,
+ .sync_state = qcom_cc_sync_state,
},
};

@@ -478,6 +479,7 @@ static struct platform_driver lpass_hm_sc7180_driver = {
.name = "lpass_hm-sc7180",
.of_match_table = lpass_hm_sc7180_match_table,
.pm = &lpass_pm_ops,
+ .sync_state = qcom_cc_sync_state,
},
};

diff --git a/drivers/clk/qcom/lpasscorecc-sc7280.c b/drivers/clk/qcom/lpasscorecc-sc7280.c
index 6ad19b06b1ce..8da95d0d4345 100644
--- a/drivers/clk/qcom/lpasscorecc-sc7280.c
+++ b/drivers/clk/qcom/lpasscorecc-sc7280.c
@@ -413,6 +413,7 @@ static struct platform_driver lpass_core_cc_sc7280_driver = {
.driver = {
.name = "lpass_core_cc-sc7280",
.of_match_table = lpass_core_cc_sc7280_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};

@@ -438,6 +439,7 @@ static struct platform_driver lpass_hm_sc7280_driver = {
.driver = {
.name = "lpass_hm-sc7280",
.of_match_table = lpass_hm_sc7280_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};

diff --git a/drivers/clk/qcom/mmcc-apq8084.c b/drivers/clk/qcom/mmcc-apq8084.c
index 02fc21208dd1..0dc828d3831a 100644
--- a/drivers/clk/qcom/mmcc-apq8084.c
+++ b/drivers/clk/qcom/mmcc-apq8084.c
@@ -3145,6 +3145,7 @@ static struct platform_driver mmcc_apq8084_driver = {
.driver = {
.name = "mmcc-apq8084",
.of_match_table = mmcc_apq8084_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};
module_platform_driver(mmcc_apq8084_driver);
diff --git a/drivers/clk/qcom/mmcc-msm8974.c b/drivers/clk/qcom/mmcc-msm8974.c
index 4273fce9a4a4..19104a5ddae8 100644
--- a/drivers/clk/qcom/mmcc-msm8974.c
+++ b/drivers/clk/qcom/mmcc-msm8974.c
@@ -2801,6 +2801,7 @@ static struct platform_driver mmcc_msm8974_driver = {
.driver = {
.name = "mmcc-msm8974",
.of_match_table = mmcc_msm8974_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};
module_platform_driver(mmcc_msm8974_driver);
diff --git a/drivers/clk/qcom/mmcc-msm8994.c b/drivers/clk/qcom/mmcc-msm8994.c
index 89c5f5fa7d9a..f021406b13b9 100644
--- a/drivers/clk/qcom/mmcc-msm8994.c
+++ b/drivers/clk/qcom/mmcc-msm8994.c
@@ -2611,6 +2611,7 @@ static struct platform_driver mmcc_msm8994_driver = {
.driver = {
.name = "mmcc-msm8994",
.of_match_table = mmcc_msm8994_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};
module_platform_driver(mmcc_msm8994_driver);
diff --git a/drivers/clk/qcom/mmcc-msm8996.c b/drivers/clk/qcom/mmcc-msm8996.c
index 80330dab4d81..01ef542bc62c 100644
--- a/drivers/clk/qcom/mmcc-msm8996.c
+++ b/drivers/clk/qcom/mmcc-msm8996.c
@@ -3635,6 +3635,7 @@ static struct platform_driver mmcc_msm8996_driver = {
.driver = {
.name = "mmcc-msm8996",
.of_match_table = mmcc_msm8996_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};
module_platform_driver(mmcc_msm8996_driver);
diff --git a/drivers/clk/qcom/mmcc-msm8998.c b/drivers/clk/qcom/mmcc-msm8998.c
index 4490594bde69..cc6153490614 100644
--- a/drivers/clk/qcom/mmcc-msm8998.c
+++ b/drivers/clk/qcom/mmcc-msm8998.c
@@ -2881,6 +2881,7 @@ static struct platform_driver mmcc_msm8998_driver = {
.driver = {
.name = "mmcc-msm8998",
.of_match_table = mmcc_msm8998_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};
module_platform_driver(mmcc_msm8998_driver);
diff --git a/drivers/clk/qcom/mmcc-sdm660.c b/drivers/clk/qcom/mmcc-sdm660.c
index bc19a23e13f8..0afe71ec49fe 100644
--- a/drivers/clk/qcom/mmcc-sdm660.c
+++ b/drivers/clk/qcom/mmcc-sdm660.c
@@ -2859,6 +2859,7 @@ static struct platform_driver mmcc_660_driver = {
.driver = {
.name = "mmcc-sdm660",
.of_match_table = mmcc_660_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};
module_platform_driver(mmcc_660_driver);
diff --git a/drivers/clk/qcom/videocc-sc7180.c b/drivers/clk/qcom/videocc-sc7180.c
index 5b9b54f616b8..13aeb02a4b42 100644
--- a/drivers/clk/qcom/videocc-sc7180.c
+++ b/drivers/clk/qcom/videocc-sc7180.c
@@ -234,6 +234,7 @@ static struct platform_driver video_cc_sc7180_driver = {
.driver = {
.name = "sc7180-videocc",
.of_match_table = video_cc_sc7180_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};

diff --git a/drivers/clk/qcom/videocc-sc7280.c b/drivers/clk/qcom/videocc-sc7280.c
index 615695d82319..041317bfc702 100644
--- a/drivers/clk/qcom/videocc-sc7280.c
+++ b/drivers/clk/qcom/videocc-sc7280.c
@@ -306,6 +306,7 @@ static struct platform_driver video_cc_sc7280_driver = {
.driver = {
.name = "video_cc-sc7280",
.of_match_table = video_cc_sc7280_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};

diff --git a/drivers/clk/qcom/videocc-sdm845.c b/drivers/clk/qcom/videocc-sdm845.c
index c77a4dd5d39c..f14584800be0 100644
--- a/drivers/clk/qcom/videocc-sdm845.c
+++ b/drivers/clk/qcom/videocc-sdm845.c
@@ -337,6 +337,7 @@ static struct platform_driver video_cc_sdm845_driver = {
.driver = {
.name = "sdm845-videocc",
.of_match_table = video_cc_sdm845_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};

diff --git a/drivers/clk/qcom/videocc-sm8150.c b/drivers/clk/qcom/videocc-sm8150.c
index 1afdbe4a249d..3144e22dd660 100644
--- a/drivers/clk/qcom/videocc-sm8150.c
+++ b/drivers/clk/qcom/videocc-sm8150.c
@@ -253,6 +253,7 @@ static struct platform_driver video_cc_sm8150_driver = {
.driver = {
.name = "video_cc-sm8150",
.of_match_table = video_cc_sm8150_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};

diff --git a/drivers/clk/qcom/videocc-sm8250.c b/drivers/clk/qcom/videocc-sm8250.c
index ad46c4014a40..fea6caad8d1f 100644
--- a/drivers/clk/qcom/videocc-sm8250.c
+++ b/drivers/clk/qcom/videocc-sm8250.c
@@ -399,6 +399,7 @@ static struct platform_driver video_cc_sm8250_driver = {
.driver = {
.name = "sm8250-videocc",
.of_match_table = video_cc_sm8250_match_table,
+ .sync_state = qcom_cc_sync_state,
},
};

--
2.34.1

2023-03-28 00:48:55

by Saravana Kannan

Subject: Re: [PATCH v3 0/4] Allow genpd providers to power off domains on sync state

On Mon, Mar 27, 2023 at 12:38 PM Abel Vesa <[email protected]> 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'm giving my thoughts in the cover letter instead of spreading it
around all the patches so that there's context between the comments.

1) Why can't all the logic in this patch series be implemented at the
framework level? And then allow the drivers to opt into this behavior
by setting the sync_state() callback.

That way, you can land it only for QC drivers by setting up
sync_state() callback only for QC drivers, but actually have the same
code function correctly for non-QC drivers too. And then once we have
this functionality working properly for QC drivers for one kernel
version (or two), we'll just have the framework set the device's
driver's sync_state() if it doesn't have one already.

2) sync_state() is not just about power on/off. It's also about the
power domain level. Can you handle that too please?

3) In your GDSC drivers, it's not clear to me if you are preventing
power off until sync_state() only for GDSCs that were already on at
boot. So if an off-at-boot GDSC gets turned on, and then you attempt
to turn it off before all its consumers have probed, it'll fail to
power it off even though that wasn't necessary?

4) The returning -EBUSY when a power off is attempted seems to be
quite wasteful. The framework will go through the whole sequence of
trying to power down, send the notifications and then fail and then
send the undo notifications. Combined with point (2) I think this can
be handled better at the aggregation level in the framework to avoid
even going that far into the power off sequence.

-Saravana

>
> 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/[email protected]
>
> V2 (RFC) of this patchset was here:
> https://lore.kernel.org/all/[email protected]/
>
> Changes since v2:
> * renamed genpd_queue_power_off_work to pm_genpd_queue_power_off and added
> comment about its purpose w.r.t. it being exported.
> * added the qcom_cc generic sync state callback to all providers that
> register GDSCs, instead of SM8450 and SC8280XP
>
> 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 (4):
> 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 providers
>
> drivers/base/power/domain.c | 18 ++++++++++--------
> drivers/clk/qcom/apss-ipq6018.c | 1 +
> drivers/clk/qcom/camcc-sc7180.c | 1 +
> drivers/clk/qcom/camcc-sc7280.c | 1 +
> drivers/clk/qcom/camcc-sdm845.c | 1 +
> drivers/clk/qcom/camcc-sm6350.c | 1 +
> drivers/clk/qcom/camcc-sm8250.c | 1 +
> drivers/clk/qcom/camcc-sm8450.c | 1 +
> drivers/clk/qcom/common.c | 19 +++++++++++++++++++
> drivers/clk/qcom/common.h | 2 ++
> drivers/clk/qcom/dispcc-qcm2290.c | 1 +
> drivers/clk/qcom/dispcc-sc7180.c | 1 +
> drivers/clk/qcom/dispcc-sc7280.c | 1 +
> drivers/clk/qcom/dispcc-sc8280xp.c | 1 +
> drivers/clk/qcom/dispcc-sdm845.c | 1 +
> drivers/clk/qcom/dispcc-sm6115.c | 1 +
> drivers/clk/qcom/dispcc-sm6125.c | 1 +
> drivers/clk/qcom/dispcc-sm6350.c | 1 +
> drivers/clk/qcom/dispcc-sm6375.c | 1 +
> drivers/clk/qcom/dispcc-sm8250.c | 1 +
> drivers/clk/qcom/dispcc-sm8450.c | 1 +
> drivers/clk/qcom/dispcc-sm8550.c | 1 +
> drivers/clk/qcom/gcc-apq8084.c | 1 +
> drivers/clk/qcom/gcc-ipq806x.c | 1 +
> drivers/clk/qcom/gcc-ipq8074.c | 1 +
> drivers/clk/qcom/gcc-mdm9615.c | 1 +
> drivers/clk/qcom/gcc-msm8660.c | 1 +
> drivers/clk/qcom/gcc-msm8909.c | 1 +
> drivers/clk/qcom/gcc-msm8916.c | 1 +
> drivers/clk/qcom/gcc-msm8939.c | 1 +
> drivers/clk/qcom/gcc-msm8953.c | 1 +
> drivers/clk/qcom/gcc-msm8960.c | 1 +
> drivers/clk/qcom/gcc-msm8974.c | 1 +
> drivers/clk/qcom/gcc-msm8976.c | 1 +
> drivers/clk/qcom/gcc-msm8994.c | 1 +
> drivers/clk/qcom/gcc-msm8996.c | 1 +
> drivers/clk/qcom/gcc-msm8998.c | 1 +
> drivers/clk/qcom/gcc-qcm2290.c | 1 +
> drivers/clk/qcom/gcc-qcs404.c | 1 +
> drivers/clk/qcom/gcc-qdu1000.c | 1 +
> drivers/clk/qcom/gcc-sa8775p.c | 1 +
> drivers/clk/qcom/gcc-sc7180.c | 1 +
> drivers/clk/qcom/gcc-sc7280.c | 1 +
> drivers/clk/qcom/gcc-sc8180x.c | 1 +
> drivers/clk/qcom/gcc-sc8280xp.c | 1 +
> drivers/clk/qcom/gcc-sdm660.c | 1 +
> drivers/clk/qcom/gcc-sdm845.c | 1 +
> drivers/clk/qcom/gcc-sdx55.c | 1 +
> drivers/clk/qcom/gcc-sdx65.c | 1 +
> drivers/clk/qcom/gcc-sm6115.c | 1 +
> drivers/clk/qcom/gcc-sm6125.c | 1 +
> drivers/clk/qcom/gcc-sm6350.c | 1 +
> drivers/clk/qcom/gcc-sm6375.c | 1 +
> drivers/clk/qcom/gcc-sm7150.c | 1 +
> drivers/clk/qcom/gcc-sm8150.c | 1 +
> drivers/clk/qcom/gcc-sm8250.c | 1 +
> drivers/clk/qcom/gcc-sm8350.c | 1 +
> drivers/clk/qcom/gcc-sm8450.c | 1 +
> drivers/clk/qcom/gcc-sm8550.c | 1 +
> drivers/clk/qcom/gdsc.c | 26 ++++++++++++++++++++++++++
> drivers/clk/qcom/gdsc.h | 6 ++++++
> drivers/clk/qcom/gpucc-msm8998.c | 1 +
> drivers/clk/qcom/gpucc-sc7180.c | 1 +
> drivers/clk/qcom/gpucc-sc7280.c | 1 +
> drivers/clk/qcom/gpucc-sc8280xp.c | 1 +
> drivers/clk/qcom/gpucc-sdm660.c | 1 +
> drivers/clk/qcom/gpucc-sdm845.c | 1 +
> drivers/clk/qcom/gpucc-sm6115.c | 1 +
> drivers/clk/qcom/gpucc-sm6125.c | 1 +
> drivers/clk/qcom/gpucc-sm6350.c | 1 +
> drivers/clk/qcom/gpucc-sm6375.c | 1 +
> drivers/clk/qcom/gpucc-sm8150.c | 1 +
> drivers/clk/qcom/gpucc-sm8250.c | 1 +
> drivers/clk/qcom/gpucc-sm8350.c | 1 +
> drivers/clk/qcom/lcc-ipq806x.c | 1 +
> drivers/clk/qcom/lpassaudiocc-sc7280.c | 1 +
> drivers/clk/qcom/lpasscc-sc7280.c | 1 +
> drivers/clk/qcom/lpasscorecc-sc7180.c | 2 ++
> drivers/clk/qcom/lpasscorecc-sc7280.c | 2 ++
> drivers/clk/qcom/mmcc-apq8084.c | 1 +
> drivers/clk/qcom/mmcc-msm8974.c | 1 +
> drivers/clk/qcom/mmcc-msm8994.c | 1 +
> drivers/clk/qcom/mmcc-msm8996.c | 1 +
> drivers/clk/qcom/mmcc-msm8998.c | 1 +
> drivers/clk/qcom/mmcc-sdm660.c | 1 +
> drivers/clk/qcom/videocc-sc7180.c | 1 +
> drivers/clk/qcom/videocc-sc7280.c | 1 +
> drivers/clk/qcom/videocc-sdm845.c | 1 +
> drivers/clk/qcom/videocc-sm8150.c | 1 +
> drivers/clk/qcom/videocc-sm8250.c | 1 +
> drivers/soc/qcom/rpmhpd.c | 19 +++++++------------
> include/linux/pm_domain.h | 4 ++++
> 92 files changed, 161 insertions(+), 20 deletions(-)
>
> --
> 2.34.1
>

2023-03-30 11:38:18

by Abel Vesa

Subject: Re: [PATCH v3 0/4] Allow genpd providers to power off domains on sync state

On 23-03-27 17:17:28, Saravana Kannan wrote:
> On Mon, Mar 27, 2023 at 12:38 PM Abel Vesa <[email protected]> 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'm giving my thoughts in the cover letter instead of spreading it
> around all the patches so that there's context between the comments.
>
> 1) Why can't all the logic in this patch series be implemented at the
> framework level? And then allow the drivers to opt into this behavior
> by setting the sync_state() callback.
>
> That way, you can land it only for QC drivers by setting up
> sync_state() callback only for QC drivers, but actually have the same
> code function correctly for non-QC drivers too. And then once we have
> this functionality working properly for QC drivers for one kernel
> version (or two), we'll just have the framework set the device's
> driver's sync_state() if it doesn't have one already.

I think Ulf has already NACK'ed that approach here:
[1] https://lore.kernel.org/lkml/CAPDyKFon35wcQ+5kx3QZb-awN_S_q8y1Sir-G+GoxkCvpN=iiA@mail.gmail.com/

And suggested this new approach that this patch series proposes.
(Unless I missunderstood his point)

>
> 2) sync_state() is not just about power on/off. It's also about the
> power domain level. Can you handle that too please?

Well, this patchset only tries to delay the disabling of unused power
domains until all consumers have had a chance to probe. So we use sync
state only to queue up a power-off request to make sure those unused
ones get disabled.

>
> 3) In your GDSC drivers, it's not clear to me if you are preventing
> power off until sync_state() only for GDSCs that were already on at
> boot. So if an off-at-boot GDSC gets turned on, and then you attempt
> to turn it off before all its consumers have probed, it'll fail to
> power it off even though that wasn't necessary?

I think we can circumvent looking at a GDSC by knowing it there was ever
a power on request since boot. I'll try to come up with something in the
new version.

>
> 4) The returning -EBUSY when a power off is attempted seems to be
> quite wasteful. The framework will go through the whole sequence of
> trying to power down, send the notifications and then fail and then
> send the undo notifications. Combined with point (2) I think this can
> be handled better at the aggregation level in the framework to avoid
> even going that far into the power off sequence.

Again, have a look at [1] (above).

Ulf, any thoughts on this 4th point?

>
> -Saravana
>
> >
> > 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/[email protected]
> >
> > V2 (RFC) of this patchset was here:
> > https://lore.kernel.org/all/[email protected]/
> >
> > Changes since v2:
> > * renamed genpd_queue_power_off_work to pm_genpd_queue_power_off and added
> > comment about its purpose w.r.t. it being exported.
> > * added the qcom_cc generic sync state callback to all providers that
> > register GDSCs, instead of SM8450 and SC8280XP
> >
> > 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 (4):
> > 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 providers
> >
> > drivers/base/power/domain.c | 18 ++++++++++--------
> > drivers/clk/qcom/apss-ipq6018.c | 1 +
> > drivers/clk/qcom/camcc-sc7180.c | 1 +
> > drivers/clk/qcom/camcc-sc7280.c | 1 +
> > drivers/clk/qcom/camcc-sdm845.c | 1 +
> > drivers/clk/qcom/camcc-sm6350.c | 1 +
> > drivers/clk/qcom/camcc-sm8250.c | 1 +
> > drivers/clk/qcom/camcc-sm8450.c | 1 +
> > drivers/clk/qcom/common.c | 19 +++++++++++++++++++
> > drivers/clk/qcom/common.h | 2 ++
> > drivers/clk/qcom/dispcc-qcm2290.c | 1 +
> > drivers/clk/qcom/dispcc-sc7180.c | 1 +
> > drivers/clk/qcom/dispcc-sc7280.c | 1 +
> > drivers/clk/qcom/dispcc-sc8280xp.c | 1 +
> > drivers/clk/qcom/dispcc-sdm845.c | 1 +
> > drivers/clk/qcom/dispcc-sm6115.c | 1 +
> > drivers/clk/qcom/dispcc-sm6125.c | 1 +
> > drivers/clk/qcom/dispcc-sm6350.c | 1 +
> > drivers/clk/qcom/dispcc-sm6375.c | 1 +
> > drivers/clk/qcom/dispcc-sm8250.c | 1 +
> > drivers/clk/qcom/dispcc-sm8450.c | 1 +
> > drivers/clk/qcom/dispcc-sm8550.c | 1 +
> > drivers/clk/qcom/gcc-apq8084.c | 1 +
> > drivers/clk/qcom/gcc-ipq806x.c | 1 +
> > drivers/clk/qcom/gcc-ipq8074.c | 1 +
> > drivers/clk/qcom/gcc-mdm9615.c | 1 +
> > drivers/clk/qcom/gcc-msm8660.c | 1 +
> > drivers/clk/qcom/gcc-msm8909.c | 1 +
> > drivers/clk/qcom/gcc-msm8916.c | 1 +
> > drivers/clk/qcom/gcc-msm8939.c | 1 +
> > drivers/clk/qcom/gcc-msm8953.c | 1 +
> > drivers/clk/qcom/gcc-msm8960.c | 1 +
> > drivers/clk/qcom/gcc-msm8974.c | 1 +
> > drivers/clk/qcom/gcc-msm8976.c | 1 +
> > drivers/clk/qcom/gcc-msm8994.c | 1 +
> > drivers/clk/qcom/gcc-msm8996.c | 1 +
> > drivers/clk/qcom/gcc-msm8998.c | 1 +
> > drivers/clk/qcom/gcc-qcm2290.c | 1 +
> > drivers/clk/qcom/gcc-qcs404.c | 1 +
> > drivers/clk/qcom/gcc-qdu1000.c | 1 +
> > drivers/clk/qcom/gcc-sa8775p.c | 1 +
> > drivers/clk/qcom/gcc-sc7180.c | 1 +
> > drivers/clk/qcom/gcc-sc7280.c | 1 +
> > drivers/clk/qcom/gcc-sc8180x.c | 1 +
> > drivers/clk/qcom/gcc-sc8280xp.c | 1 +
> > drivers/clk/qcom/gcc-sdm660.c | 1 +
> > drivers/clk/qcom/gcc-sdm845.c | 1 +
> > drivers/clk/qcom/gcc-sdx55.c | 1 +
> > drivers/clk/qcom/gcc-sdx65.c | 1 +
> > drivers/clk/qcom/gcc-sm6115.c | 1 +
> > drivers/clk/qcom/gcc-sm6125.c | 1 +
> > drivers/clk/qcom/gcc-sm6350.c | 1 +
> > drivers/clk/qcom/gcc-sm6375.c | 1 +
> > drivers/clk/qcom/gcc-sm7150.c | 1 +
> > drivers/clk/qcom/gcc-sm8150.c | 1 +
> > drivers/clk/qcom/gcc-sm8250.c | 1 +
> > drivers/clk/qcom/gcc-sm8350.c | 1 +
> > drivers/clk/qcom/gcc-sm8450.c | 1 +
> > drivers/clk/qcom/gcc-sm8550.c | 1 +
> > drivers/clk/qcom/gdsc.c | 26 ++++++++++++++++++++++++++
> > drivers/clk/qcom/gdsc.h | 6 ++++++
> > drivers/clk/qcom/gpucc-msm8998.c | 1 +
> > drivers/clk/qcom/gpucc-sc7180.c | 1 +
> > drivers/clk/qcom/gpucc-sc7280.c | 1 +
> > drivers/clk/qcom/gpucc-sc8280xp.c | 1 +
> > drivers/clk/qcom/gpucc-sdm660.c | 1 +
> > drivers/clk/qcom/gpucc-sdm845.c | 1 +
> > drivers/clk/qcom/gpucc-sm6115.c | 1 +
> > drivers/clk/qcom/gpucc-sm6125.c | 1 +
> > drivers/clk/qcom/gpucc-sm6350.c | 1 +
> > drivers/clk/qcom/gpucc-sm6375.c | 1 +
> > drivers/clk/qcom/gpucc-sm8150.c | 1 +
> > drivers/clk/qcom/gpucc-sm8250.c | 1 +
> > drivers/clk/qcom/gpucc-sm8350.c | 1 +
> > drivers/clk/qcom/lcc-ipq806x.c | 1 +
> > drivers/clk/qcom/lpassaudiocc-sc7280.c | 1 +
> > drivers/clk/qcom/lpasscc-sc7280.c | 1 +
> > drivers/clk/qcom/lpasscorecc-sc7180.c | 2 ++
> > drivers/clk/qcom/lpasscorecc-sc7280.c | 2 ++
> > drivers/clk/qcom/mmcc-apq8084.c | 1 +
> > drivers/clk/qcom/mmcc-msm8974.c | 1 +
> > drivers/clk/qcom/mmcc-msm8994.c | 1 +
> > drivers/clk/qcom/mmcc-msm8996.c | 1 +
> > drivers/clk/qcom/mmcc-msm8998.c | 1 +
> > drivers/clk/qcom/mmcc-sdm660.c | 1 +
> > drivers/clk/qcom/videocc-sc7180.c | 1 +
> > drivers/clk/qcom/videocc-sc7280.c | 1 +
> > drivers/clk/qcom/videocc-sdm845.c | 1 +
> > drivers/clk/qcom/videocc-sm8150.c | 1 +
> > drivers/clk/qcom/videocc-sm8250.c | 1 +
> > drivers/soc/qcom/rpmhpd.c | 19 +++++++------------
> > include/linux/pm_domain.h | 4 ++++
> > 92 files changed, 161 insertions(+), 20 deletions(-)
> >
> > --
> > 2.34.1
> >

2023-03-30 19:54:02

by Saravana Kannan

Subject: Re: [PATCH v3 0/4] Allow genpd providers to power off domains on sync state

On Thu, Mar 30, 2023 at 4:27 AM Abel Vesa <[email protected]> wrote:
>
> On 23-03-27 17:17:28, Saravana Kannan wrote:
> > On Mon, Mar 27, 2023 at 12:38 PM Abel Vesa <[email protected]> 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'm giving my thoughts in the cover letter instead of spreading it
> > around all the patches so that there's context between the comments.
> >
> > 1) Why can't all the logic in this patch series be implemented at the
> > framework level? And then allow the drivers to opt into this behavior
> > by setting the sync_state() callback.
> >
> > That way, you can land it only for QC drivers by setting up
> > sync_state() callback only for QC drivers, but actually have the same
> > code function correctly for non-QC drivers too. And then once we have
> > this functionality working properly for QC drivers for one kernel
> > version (or two), we'll just have the framework set the device's
> > driver's sync_state() if it doesn't have one already.
>
> I think Ulf has already NACK'ed that approach here:
> [1] https://lore.kernel.org/lkml/CAPDyKFon35wcQ+5kx3QZb-awN_S_q8y1Sir-G+GoxkCvpN=iiA@mail.gmail.com/

I would have NACK'ed that too because that's an incomplete fix. As I
said further below, the fix needs to be at the aggregation level where
you aggregate all the current consumer requests. In there, you need to
add in the "state at boot" input that gets cleared out after a
sync_state() call is received for that power domain.

> And suggested this new approach that this patch series proposes.
> (Unless I missunderstood his point)
>
> >
> > 2) sync_state() is not just about power on/off. It's also about the
> > power domain level. Can you handle that too please?
>
> Well, this patchset only tries to delay the disabling of unused power
> domains until all consumers have had a chance to probe. So we use sync
> state only to queue up a power-off request to make sure those unused
> ones get disabled.

Sure, but the design is completely unusable for a more complete
sync_state() behavior. I'm okay if you want to improve the
sync_state() behavior in layers, but don't do it in a way where the
current design will definitely not work for what you want to add in
the future.

> >
> > 3) In your GDSC drivers, it's not clear to me if you are preventing
> > power off until sync_state() only for GDSCs that were already on at
> > boot. So if an off-at-boot GDSC gets turned on, and then you attempt
> > to turn it off before all its consumers have probed, it'll fail to
> > power it off even though that wasn't necessary?
>
> I think we can circumvent looking at a GDSC by knowing it there was ever
> a power on request since boot. I'll try to come up with something in the
> new version.

Please no. There's nothing wrong with reading the GDSC values. Please
read them and don't turn on GDSC's that weren't on at boot.

Otherwise you are making it a hassle for the case where there is a
consumer without a driver for a GDSC that was off at boot. You are now
forcing the use of timeouts or writing to state_synced file. Those
should be absolute last resorts, but you are making that a requirement
with your current implementation. If you implement it correctly by
reading the GDSC register, things will "just work". And it's not even
hard to do.

NACK'ed until this is handled correctly.

>
> >
> > 4) The returning -EBUSY when a power off is attempted seems to be
> > quite wasteful. The framework will go through the whole sequence of
> > trying to power down, send the notifications and then fail and then
> > send the undo notifications. Combined with point (2) I think this can
> > be handled better at the aggregation level in the framework to avoid
> > even going that far into the power off sequence.
>
> Again, have a look at [1] (above).

See my reply above. If you do it properly at the framework level, this
can be done in a clean way and will work for all power domains.

-Saravana

>
> Ulf, any thoughts on this 4th point?
>
> >
> > -Saravana
> >
> > >
> > > 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/[email protected]
> > >
> > > V2 (RFC) of this patchset was here:
> > > https://lore.kernel.org/all/[email protected]/
> > >
> > > Changes since v2:
> > > * renamed genpd_queue_power_off_work to pm_genpd_queue_power_off and added
> > > comment about its purpose w.r.t. it being exported.
> > > * added the qcom_cc generic sync state callback to all providers that
> > > register GDSCs, instead of SM8450 and SC8280XP
> > >
> > > 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 (4):
> > > 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 providers
> > >
> > > drivers/base/power/domain.c | 18 ++++++++++--------
> > > drivers/clk/qcom/apss-ipq6018.c | 1 +
> > > drivers/clk/qcom/camcc-sc7180.c | 1 +
> > > drivers/clk/qcom/camcc-sc7280.c | 1 +
> > > drivers/clk/qcom/camcc-sdm845.c | 1 +
> > > drivers/clk/qcom/camcc-sm6350.c | 1 +
> > > drivers/clk/qcom/camcc-sm8250.c | 1 +
> > > drivers/clk/qcom/camcc-sm8450.c | 1 +
> > > drivers/clk/qcom/common.c | 19 +++++++++++++++++++
> > > drivers/clk/qcom/common.h | 2 ++
> > > drivers/clk/qcom/dispcc-qcm2290.c | 1 +
> > > drivers/clk/qcom/dispcc-sc7180.c | 1 +
> > > drivers/clk/qcom/dispcc-sc7280.c | 1 +
> > > drivers/clk/qcom/dispcc-sc8280xp.c | 1 +
> > > drivers/clk/qcom/dispcc-sdm845.c | 1 +
> > > drivers/clk/qcom/dispcc-sm6115.c | 1 +
> > > drivers/clk/qcom/dispcc-sm6125.c | 1 +
> > > drivers/clk/qcom/dispcc-sm6350.c | 1 +
> > > drivers/clk/qcom/dispcc-sm6375.c | 1 +
> > > drivers/clk/qcom/dispcc-sm8250.c | 1 +
> > > drivers/clk/qcom/dispcc-sm8450.c | 1 +
> > > drivers/clk/qcom/dispcc-sm8550.c | 1 +
> > > drivers/clk/qcom/gcc-apq8084.c | 1 +
> > > drivers/clk/qcom/gcc-ipq806x.c | 1 +
> > > drivers/clk/qcom/gcc-ipq8074.c | 1 +
> > > drivers/clk/qcom/gcc-mdm9615.c | 1 +
> > > drivers/clk/qcom/gcc-msm8660.c | 1 +
> > > drivers/clk/qcom/gcc-msm8909.c | 1 +
> > > drivers/clk/qcom/gcc-msm8916.c | 1 +
> > > drivers/clk/qcom/gcc-msm8939.c | 1 +
> > > drivers/clk/qcom/gcc-msm8953.c | 1 +
> > > drivers/clk/qcom/gcc-msm8960.c | 1 +
> > > drivers/clk/qcom/gcc-msm8974.c | 1 +
> > > drivers/clk/qcom/gcc-msm8976.c | 1 +
> > > drivers/clk/qcom/gcc-msm8994.c | 1 +
> > > drivers/clk/qcom/gcc-msm8996.c | 1 +
> > > drivers/clk/qcom/gcc-msm8998.c | 1 +
> > > drivers/clk/qcom/gcc-qcm2290.c | 1 +
> > > drivers/clk/qcom/gcc-qcs404.c | 1 +
> > > drivers/clk/qcom/gcc-qdu1000.c | 1 +
> > > drivers/clk/qcom/gcc-sa8775p.c | 1 +
> > > drivers/clk/qcom/gcc-sc7180.c | 1 +
> > > drivers/clk/qcom/gcc-sc7280.c | 1 +
> > > drivers/clk/qcom/gcc-sc8180x.c | 1 +
> > > drivers/clk/qcom/gcc-sc8280xp.c | 1 +
> > > drivers/clk/qcom/gcc-sdm660.c | 1 +
> > > drivers/clk/qcom/gcc-sdm845.c | 1 +
> > > drivers/clk/qcom/gcc-sdx55.c | 1 +
> > > drivers/clk/qcom/gcc-sdx65.c | 1 +
> > > drivers/clk/qcom/gcc-sm6115.c | 1 +
> > > drivers/clk/qcom/gcc-sm6125.c | 1 +
> > > drivers/clk/qcom/gcc-sm6350.c | 1 +
> > > drivers/clk/qcom/gcc-sm6375.c | 1 +
> > > drivers/clk/qcom/gcc-sm7150.c | 1 +
> > > drivers/clk/qcom/gcc-sm8150.c | 1 +
> > > drivers/clk/qcom/gcc-sm8250.c | 1 +
> > > drivers/clk/qcom/gcc-sm8350.c | 1 +
> > > drivers/clk/qcom/gcc-sm8450.c | 1 +
> > > drivers/clk/qcom/gcc-sm8550.c | 1 +
> > > drivers/clk/qcom/gdsc.c | 26 ++++++++++++++++++++++++++
> > > drivers/clk/qcom/gdsc.h | 6 ++++++
> > > drivers/clk/qcom/gpucc-msm8998.c | 1 +
> > > drivers/clk/qcom/gpucc-sc7180.c | 1 +
> > > drivers/clk/qcom/gpucc-sc7280.c | 1 +
> > > drivers/clk/qcom/gpucc-sc8280xp.c | 1 +
> > > drivers/clk/qcom/gpucc-sdm660.c | 1 +
> > > drivers/clk/qcom/gpucc-sdm845.c | 1 +
> > > drivers/clk/qcom/gpucc-sm6115.c | 1 +
> > > drivers/clk/qcom/gpucc-sm6125.c | 1 +
> > > drivers/clk/qcom/gpucc-sm6350.c | 1 +
> > > drivers/clk/qcom/gpucc-sm6375.c | 1 +
> > > drivers/clk/qcom/gpucc-sm8150.c | 1 +
> > > drivers/clk/qcom/gpucc-sm8250.c | 1 +
> > > drivers/clk/qcom/gpucc-sm8350.c | 1 +
> > > drivers/clk/qcom/lcc-ipq806x.c | 1 +
> > > drivers/clk/qcom/lpassaudiocc-sc7280.c | 1 +
> > > drivers/clk/qcom/lpasscc-sc7280.c | 1 +
> > > drivers/clk/qcom/lpasscorecc-sc7180.c | 2 ++
> > > drivers/clk/qcom/lpasscorecc-sc7280.c | 2 ++
> > > drivers/clk/qcom/mmcc-apq8084.c | 1 +
> > > drivers/clk/qcom/mmcc-msm8974.c | 1 +
> > > drivers/clk/qcom/mmcc-msm8994.c | 1 +
> > > drivers/clk/qcom/mmcc-msm8996.c | 1 +
> > > drivers/clk/qcom/mmcc-msm8998.c | 1 +
> > > drivers/clk/qcom/mmcc-sdm660.c | 1 +
> > > drivers/clk/qcom/videocc-sc7180.c | 1 +
> > > drivers/clk/qcom/videocc-sc7280.c | 1 +
> > > drivers/clk/qcom/videocc-sdm845.c | 1 +
> > > drivers/clk/qcom/videocc-sm8150.c | 1 +
> > > drivers/clk/qcom/videocc-sm8250.c | 1 +
> > > drivers/soc/qcom/rpmhpd.c | 19 +++++++------------
> > > include/linux/pm_domain.h | 4 ++++
> > > 92 files changed, 161 insertions(+), 20 deletions(-)
> > >
> > > --
> > > 2.34.1
> > >

2023-03-31 05:04:49

by Abel Vesa

Subject: Re: [PATCH v3 0/4] Allow genpd providers to power off domains on sync state

On 23-03-30 12:50:44, Saravana Kannan wrote:
> On Thu, Mar 30, 2023 at 4:27 AM Abel Vesa <[email protected]> wrote:
> >
> > On 23-03-27 17:17:28, Saravana Kannan wrote:
> > > On Mon, Mar 27, 2023 at 12:38 PM Abel Vesa <[email protected]> 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'm giving my thoughts in the cover letter instead of spreading it
> > > around all the patches so that there's context between the comments.
> > >
> > > 1) Why can't all the logic in this patch series be implemented at the
> > > framework level? And then allow the drivers to opt into this behavior
> > > by setting the sync_state() callback.
> > >
> > > That way, you can land it only for QC drivers by setting up
> > > sync_state() callback only for QC drivers, but actually have the same
> > > code function correctly for non-QC drivers too. And then once we have
> > > this functionality working properly for QC drivers for one kernel
> > > version (or two), we'll just have the framework set the device's
> > > driver's sync_state() if it doesn't have one already.
> >
> > I think Ulf has already NACK'ed that approach here:
> > [1] https://lore.kernel.org/lkml/CAPDyKFon35wcQ+5kx3QZb-awN_S_q8y1Sir-G+GoxkCvpN=iiA@mail.gmail.com/
>
> I would have NACK'ed that too because that's an incomplete fix. As I
> said further below, the fix needs to be at the aggregation level where
> you aggregate all the current consumer requests. In there, you need to
> add in the "state at boot" input that gets cleared out after a
> sync_state() call is received for that power domain.
>

So, just to make sure I understand your point. You would rather have the
genpd_power_off check if 'state at boot' is 'on' and return busy and
then clear then, via a generic genpd sync state you would mark 'state at
boot' as 'off' and queue up a power off request for each PD from there.
And as for 'state at boot' it would check the enable bit through
provider.

Am I right so far?

> > And suggested this new approach that this patch series proposes.
> > (Unless I missunderstood his point)
> >
> > >
> > > 2) sync_state() is not just about power on/off. It's also about the
> > > power domain level. Can you handle that too please?
> >
> > Well, this patchset only tries to delay the disabling of unused power
> > domains until all consumers have had a chance to probe. So we use sync
> > state only to queue up a power-off request to make sure those unused
> > ones get disabled.
>
> Sure, but the design is completely unusable for a more complete
> sync_state() behavior. I'm okay if you want to improve the
> sync_state() behavior in layers, but don't do it in a way where the
> current design will definitely not work for what you want to add in
> the future.

But you would still be OK with the qcom_cc sync state wrapper, I guess,
right? Your concern is only about the sync state callback being not
genpd generic one, AFAIU.

>
> > >
> > > 3) In your GDSC drivers, it's not clear to me if you are preventing
> > > power off until sync_state() only for GDSCs that were already on at
> > > boot. So if an off-at-boot GDSC gets turned on, and then you attempt
> > > to turn it off before all its consumers have probed, it'll fail to
> > > power it off even though that wasn't necessary?
> >
> > I think we can circumvent looking at a GDSC by knowing it there was ever
> > a power on request since boot. I'll try to come up with something in the
> > new version.
>
> Please no. There's nothing wrong with reading the GDSC values. Please
> read them and don't turn on GDSC's that weren't on at boot.

Sorry for the typos above, I basically said that for this concern of
yours, we can add the 'state at boot' thing you mentioned above by
looking at the GDSC (as in reading reg).

>
> Otherwise you are making it a hassle for the case where there is a
> consumer without a driver for a GDSC that was off at boot. You are now
> forcing the use of timeouts or writing to state_synced file. Those
> should be absolute last resorts, but you are making that a requirement
> with your current implementation. If you implement it correctly by
> reading the GDSC register, things will "just work". And it's not even
> hard to do.
>
> NACK'ed until this is handled correctly.
>
> >
> > >
> > > 4) The returning -EBUSY when a power off is attempted seems to be
> > > quite wasteful. The framework will go through the whole sequence of
> > > trying to power down, send the notifications and then fail and then
> > > send the undo notifications. Combined with point (2) I think this can
> > > be handled better at the aggregation level in the framework to avoid
> > > even going that far into the power off sequence.
> >
> > Again, have a look at [1] (above).
>
> See my reply above. If you do it properly at the framework level, this
> can be done in a clean way and will work for all power domains.
>
> -Saravana
>
> >
> > Ulf, any thoughts on this 4th point?
> >
> > >
> > > -Saravana
> > >
> > > >
> > > > 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/[email protected]
> > > >
> > > > V2 (RFC) of this patchset was here:
> > > > https://lore.kernel.org/all/[email protected]/
> > > >
> > > > Changes since v2:
> > > > * renamed genpd_queue_power_off_work to pm_genpd_queue_power_off and added
> > > > comment about its purpose w.r.t. it being exported.
> > > > * added the qcom_cc generic sync state callback to all providers that
> > > > register GDSCs, instead of SM8450 and SC8280XP
> > > >
> > > > 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 (4):
> > > > 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 providers
> > > >
> > > > drivers/base/power/domain.c | 18 ++++++++++--------
> > > > drivers/clk/qcom/apss-ipq6018.c | 1 +
> > > > drivers/clk/qcom/camcc-sc7180.c | 1 +
> > > > drivers/clk/qcom/camcc-sc7280.c | 1 +
> > > > drivers/clk/qcom/camcc-sdm845.c | 1 +
> > > > drivers/clk/qcom/camcc-sm6350.c | 1 +
> > > > drivers/clk/qcom/camcc-sm8250.c | 1 +
> > > > drivers/clk/qcom/camcc-sm8450.c | 1 +
> > > > drivers/clk/qcom/common.c | 19 +++++++++++++++++++
> > > > drivers/clk/qcom/common.h | 2 ++
> > > > drivers/clk/qcom/dispcc-qcm2290.c | 1 +
> > > > drivers/clk/qcom/dispcc-sc7180.c | 1 +
> > > > drivers/clk/qcom/dispcc-sc7280.c | 1 +
> > > > drivers/clk/qcom/dispcc-sc8280xp.c | 1 +
> > > > drivers/clk/qcom/dispcc-sdm845.c | 1 +
> > > > drivers/clk/qcom/dispcc-sm6115.c | 1 +
> > > > drivers/clk/qcom/dispcc-sm6125.c | 1 +
> > > > drivers/clk/qcom/dispcc-sm6350.c | 1 +
> > > > drivers/clk/qcom/dispcc-sm6375.c | 1 +
> > > > drivers/clk/qcom/dispcc-sm8250.c | 1 +
> > > > drivers/clk/qcom/dispcc-sm8450.c | 1 +
> > > > drivers/clk/qcom/dispcc-sm8550.c | 1 +
> > > > drivers/clk/qcom/gcc-apq8084.c | 1 +
> > > > drivers/clk/qcom/gcc-ipq806x.c | 1 +
> > > > drivers/clk/qcom/gcc-ipq8074.c | 1 +
> > > > drivers/clk/qcom/gcc-mdm9615.c | 1 +
> > > > drivers/clk/qcom/gcc-msm8660.c | 1 +
> > > > drivers/clk/qcom/gcc-msm8909.c | 1 +
> > > > drivers/clk/qcom/gcc-msm8916.c | 1 +
> > > > drivers/clk/qcom/gcc-msm8939.c | 1 +
> > > > drivers/clk/qcom/gcc-msm8953.c | 1 +
> > > > drivers/clk/qcom/gcc-msm8960.c | 1 +
> > > > drivers/clk/qcom/gcc-msm8974.c | 1 +
> > > > drivers/clk/qcom/gcc-msm8976.c | 1 +
> > > > drivers/clk/qcom/gcc-msm8994.c | 1 +
> > > > drivers/clk/qcom/gcc-msm8996.c | 1 +
> > > > drivers/clk/qcom/gcc-msm8998.c | 1 +
> > > > drivers/clk/qcom/gcc-qcm2290.c | 1 +
> > > > drivers/clk/qcom/gcc-qcs404.c | 1 +
> > > > drivers/clk/qcom/gcc-qdu1000.c | 1 +
> > > > drivers/clk/qcom/gcc-sa8775p.c | 1 +
> > > > drivers/clk/qcom/gcc-sc7180.c | 1 +
> > > > drivers/clk/qcom/gcc-sc7280.c | 1 +
> > > > drivers/clk/qcom/gcc-sc8180x.c | 1 +
> > > > drivers/clk/qcom/gcc-sc8280xp.c | 1 +
> > > > drivers/clk/qcom/gcc-sdm660.c | 1 +
> > > > drivers/clk/qcom/gcc-sdm845.c | 1 +
> > > > drivers/clk/qcom/gcc-sdx55.c | 1 +
> > > > drivers/clk/qcom/gcc-sdx65.c | 1 +
> > > > drivers/clk/qcom/gcc-sm6115.c | 1 +
> > > > drivers/clk/qcom/gcc-sm6125.c | 1 +
> > > > drivers/clk/qcom/gcc-sm6350.c | 1 +
> > > > drivers/clk/qcom/gcc-sm6375.c | 1 +
> > > > drivers/clk/qcom/gcc-sm7150.c | 1 +
> > > > drivers/clk/qcom/gcc-sm8150.c | 1 +
> > > > drivers/clk/qcom/gcc-sm8250.c | 1 +
> > > > drivers/clk/qcom/gcc-sm8350.c | 1 +
> > > > drivers/clk/qcom/gcc-sm8450.c | 1 +
> > > > drivers/clk/qcom/gcc-sm8550.c | 1 +
> > > > drivers/clk/qcom/gdsc.c | 26 ++++++++++++++++++++++++++
> > > > drivers/clk/qcom/gdsc.h | 6 ++++++
> > > > drivers/clk/qcom/gpucc-msm8998.c | 1 +
> > > > drivers/clk/qcom/gpucc-sc7180.c | 1 +
> > > > drivers/clk/qcom/gpucc-sc7280.c | 1 +
> > > > drivers/clk/qcom/gpucc-sc8280xp.c | 1 +
> > > > drivers/clk/qcom/gpucc-sdm660.c | 1 +
> > > > drivers/clk/qcom/gpucc-sdm845.c | 1 +
> > > > drivers/clk/qcom/gpucc-sm6115.c | 1 +
> > > > drivers/clk/qcom/gpucc-sm6125.c | 1 +
> > > > drivers/clk/qcom/gpucc-sm6350.c | 1 +
> > > > drivers/clk/qcom/gpucc-sm6375.c | 1 +
> > > > drivers/clk/qcom/gpucc-sm8150.c | 1 +
> > > > drivers/clk/qcom/gpucc-sm8250.c | 1 +
> > > > drivers/clk/qcom/gpucc-sm8350.c | 1 +
> > > > drivers/clk/qcom/lcc-ipq806x.c | 1 +
> > > > drivers/clk/qcom/lpassaudiocc-sc7280.c | 1 +
> > > > drivers/clk/qcom/lpasscc-sc7280.c | 1 +
> > > > drivers/clk/qcom/lpasscorecc-sc7180.c | 2 ++
> > > > drivers/clk/qcom/lpasscorecc-sc7280.c | 2 ++
> > > > drivers/clk/qcom/mmcc-apq8084.c | 1 +
> > > > drivers/clk/qcom/mmcc-msm8974.c | 1 +
> > > > drivers/clk/qcom/mmcc-msm8994.c | 1 +
> > > > drivers/clk/qcom/mmcc-msm8996.c | 1 +
> > > > drivers/clk/qcom/mmcc-msm8998.c | 1 +
> > > > drivers/clk/qcom/mmcc-sdm660.c | 1 +
> > > > drivers/clk/qcom/videocc-sc7180.c | 1 +
> > > > drivers/clk/qcom/videocc-sc7280.c | 1 +
> > > > drivers/clk/qcom/videocc-sdm845.c | 1 +
> > > > drivers/clk/qcom/videocc-sm8150.c | 1 +
> > > > drivers/clk/qcom/videocc-sm8250.c | 1 +
> > > > drivers/soc/qcom/rpmhpd.c | 19 +++++++------------
> > > > include/linux/pm_domain.h | 4 ++++
> > > > 92 files changed, 161 insertions(+), 20 deletions(-)
> > > >
> > > > --
> > > > 2.34.1
> > > >

2023-04-05 14:14:05

by Ulf Hansson

Subject: Re: [PATCH v3 0/4] Allow genpd providers to power off domains on sync state

Abel, Saravana,

On Fri, 31 Mar 2023 at 06:59, Abel Vesa <[email protected]> wrote:
>
> On 23-03-30 12:50:44, Saravana Kannan wrote:
> > On Thu, Mar 30, 2023 at 4:27 AM Abel Vesa <[email protected]> wrote:
> > >
> > > On 23-03-27 17:17:28, Saravana Kannan wrote:
> > > > On Mon, Mar 27, 2023 at 12:38 PM Abel Vesa <[email protected]> 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'm giving my thoughts in the cover letter instead of spreading it
> > > > around all the patches so that there's context between the comments.
> > > >
> > > > 1) Why can't all the logic in this patch series be implemented at the
> > > > framework level? And then allow the drivers to opt into this behavior
> > > > by setting the sync_state() callback.
> > > >
> > > > That way, you can land it only for QC drivers by setting up
> > > > sync_state() callback only for QC drivers, but actually have the same
> > > > code function correctly for non-QC drivers too. And then once we have
> > > > this functionality working properly for QC drivers for one kernel
> > > > version (or two), we'll just have the framework set the device's
> > > > driver's sync_state() if it doesn't have one already.
> > >
> > > I think Ulf has already NACK'ed that approach here:
> > > [1] https://lore.kernel.org/lkml/CAPDyKFon35wcQ+5kx3QZb-awN_S_q8y1Sir-G+GoxkCvpN=iiA@mail.gmail.com/
> >
> > I would have NACK'ed that too because that's an incomplete fix. As I
> > said further below, the fix needs to be at the aggregation level where
> > you aggregate all the current consumer requests. In there, you need to
> > add in the "state at boot" input that gets cleared out after a
> > sync_state() call is received for that power domain.
> >
>
> So, just to make sure I understand your point. You would rather have the
> genpd_power_off check if 'state at boot' is 'on' and return busy and
> then clear then, via a generic genpd sync state you would mark 'state at
> boot' as 'off' and queue up a power off request for each PD from there.
> And as for 'state at boot' it would check the enable bit through
> provider.
>
> Am I right so far?

I am not sure I completely follow what you are suggesting here.

Although, let me point out that there is no requirement from the genpd
API point of view, that the provider needs to be a driver. This means
that the sync_state callback may not even be applicable for all genpd
providers.

In other words, it looks to me that we may need some new genpd helper
functions, no matter what. More importantly, it looks like we need an
opt-in behaviour, unless we can figure out a common way for genpd to
understand whether the sync_state thing is going to be applicable or
not. Maybe Saravana has some ideas around this?

Note that, I don't object to extending genpd to be more clever and to
share common code, of course. We could, for example, make
genpd_power_off() to bail out earlier, rather than calling the
->power_off() callback and waiting for it to return -EBUSY. Both of
you have pointed this out to me, in some of the earlier
replies/discussions too.

>
> > > And suggested this new approach that this patch series proposes.
> > > (Unless I missunderstood his point)
> > >
> > > >
> > > > 2) sync_state() is not just about power on/off. It's also about the
> > > > power domain level. Can you handle that too please?
> > >
> > > Well, this patchset only tries to delay the disabling of unused power
> > > domains until all consumers have had a chance to probe. So we use sync
> > > state only to queue up a power-off request to make sure those unused
> > > ones get disabled.
> >
> > Sure, but the design is completely unusable for a more complete
> > sync_state() behavior. I'm okay if you want to improve the
> > sync_state() behavior in layers, but don't do it in a way where the
> > current design will definitely not work for what you want to add in
> > the future.
>
> But you would still be OK with the qcom_cc sync state wrapper, I guess,
> right? Your concern is only about the sync state callback being not
> genpd generic one, AFAIU.
>
> >
> > > >
> > > > 3) In your GDSC drivers, it's not clear to me if you are preventing
> > > > power off until sync_state() only for GDSCs that were already on at
> > > > boot. So if an off-at-boot GDSC gets turned on, and then you attempt
> > > > to turn it off before all its consumers have probed, it'll fail to
> > > > power it off even though that wasn't necessary?
> > >
> > > I think we can circumvent looking at a GDSC by knowing it there was ever
> > > a power on request since boot. I'll try to come up with something in the
> > > new version.
> >
> > Please no. There's nothing wrong with reading the GDSC values. Please
> > read them and don't turn on GDSC's that weren't on at boot.
>
> Sorry for the typos above, I basically said that for this concern of
> yours, we can add the 'state at boot' thing you mentioned above by
> looking at the GDSC (as in reading reg).
>
> >
> > Otherwise you are making it a hassle for the case where there is a
> > consumer without a driver for a GDSC that was off at boot. You are now
> > forcing the use of timeouts or writing to state_synced file. Those
> > should be absolute last resorts, but you are making that a requirement
> > with your current implementation. If you implement it correctly by
> > reading the GDSC register, things will "just work". And it's not even
> > hard to do.
> >
> > NACK'ed until this is handled correctly.
> >
> > >
> > > >
> > > > 4) The returning -EBUSY when a power off is attempted seems to be
> > > > quite wasteful. The framework will go through the whole sequence of
> > > > trying to power down, send the notifications and then fail and then
> > > > send the undo notifications. Combined with point (2) I think this can
> > > > be handled better at the aggregation level in the framework to avoid
> > > > even going that far into the power off sequence.
> > >
> > > Again, have a look at [1] (above).
> >
> > See my reply above. If you do it properly at the framework level, this
> > can be done in a clean way and will work for all power domains.
> >
> > -Saravana
> >
> > >
> > > Ulf, any thoughts on this 4th point?

Please, see my reply above.

[...]

Kind regards
Uffe

2023-04-06 09:32:25

by Abel Vesa

Subject: Re: [PATCH v3 0/4] Allow genpd providers to power off domains on sync state

On 23-04-05 16:11:18, Ulf Hansson wrote:
> Abel, Saravana,
>
> On Fri, 31 Mar 2023 at 06:59, Abel Vesa <[email protected]> wrote:
> >
> > On 23-03-30 12:50:44, Saravana Kannan wrote:
> > > On Thu, Mar 30, 2023 at 4:27 AM Abel Vesa <[email protected]> wrote:
> > > >
> > > > On 23-03-27 17:17:28, Saravana Kannan wrote:
> > > > > On Mon, Mar 27, 2023 at 12:38 PM Abel Vesa <[email protected]> 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'm giving my thoughts in the cover letter instead of spreading it
> > > > > around all the patches so that there's context between the comments.
> > > > >
> > > > > 1) Why can't all the logic in this patch series be implemented at the
> > > > > framework level? And then allow the drivers to opt into this behavior
> > > > > by setting the sync_state() callback.
> > > > >
> > > > > That way, you can land it only for QC drivers by setting up
> > > > > sync_state() callback only for QC drivers, but actually have the same
> > > > > code function correctly for non-QC drivers too. And then once we have
> > > > > this functionality working properly for QC drivers for one kernel
> > > > > version (or two), we'll just have the framework set the device's
> > > > > driver's sync_state() if it doesn't have one already.
> > > >
> > > > I think Ulf has already NACK'ed that approach here:
> > > > [1] https://lore.kernel.org/lkml/CAPDyKFon35wcQ+5kx3QZb-awN_S_q8y1Sir-G+GoxkCvpN=iiA@mail.gmail.com/
> > >
> > > I would have NACK'ed that too because that's an incomplete fix. As I
> > > said further below, the fix needs to be at the aggregation level where
> > > you aggregate all the current consumer requests. In there, you need to
> > > add in the "state at boot" input that gets cleared out after a
> > > sync_state() call is received for that power domain.
> > >
> >
> > So, just to make sure I understand your point. You would rather have the
> > genpd_power_off check if 'state at boot' is 'on' and return busy and
> > then clear then, via a generic genpd sync state you would mark 'state at
> > boot' as 'off' and queue up a power off request for each PD from there.
> > And as for 'state at boot' it would check the enable bit through
> > provider.
> >
> > Am I right so far?
>
> I am not sure I completely follow what you are suggesting here.

Please have a look at this:
https://git.kernel.org/pub/scm/linux/kernel/git/abelvesa/linux.git/commit/?h=qcom/genpd/ignore_unused_until_sync_state&id=4f9e6140dfe77884012383f8ba2140cadb62ca4a

Keep in mind that is WIP for now. Once I have something, I'll post it on
mailing list. Right now, there is a missing piece mentioned in that
commit message.

>
> Although, let me point out that there is no requirement from the genpd
> API point of view, that the provider needs to be a driver. This means
> that the sync_state callback may not even be applicable for all genpd
> providers.

Yes, I'm considering that case too.

>
> In other words, it looks to me that we may need some new genpd helper
> functions, no matter what. More importantly, it looks like we need an
> opt-in behaviour, unless we can figure out a common way for genpd to
> understand whether the sync_state thing is going to be applicable or
> not. Maybe Saravana has some ideas around this?
>
> Note that, I don't object to extending genpd to be more clever and to
> share common code, of course. We could, for example, make
> genpd_power_off() to bail out earlier, rather than calling the
> ->power_off() callback and waiting for it to return -EBUSY. Both of
> you have pointed this out to me, in some of the earlier
> replies/discussions too.

The above link basically does this. I hope this is what Saravana has in
mind as well.

>
> >
> > > > And suggested this new approach that this patch series proposes.
> > > > (Unless I missunderstood his point)
> > > >
> > > > >
> > > > > 2) sync_state() is not just about power on/off. It's also about the
> > > > > power domain level. Can you handle that too please?
> > > >
> > > > Well, this patchset only tries to delay the disabling of unused power
> > > > domains until all consumers have had a chance to probe. So we use sync
> > > > state only to queue up a power-off request to make sure those unused
> > > > ones get disabled.
> > >
> > > Sure, but the design is completely unusable for a more complete
> > > sync_state() behavior. I'm okay if you want to improve the
> > > sync_state() behavior in layers, but don't do it in a way where the
> > > current design will definitely not work for what you want to add in
> > > the future.
> >
> > But you would still be OK with the qcom_cc sync state wrapper, I guess,
> > right? Your concern is only about the sync state callback being not
> > genpd generic one, AFAIU.
> >
> > >
> > > > >
> > > > > 3) In your GDSC drivers, it's not clear to me if you are preventing
> > > > > power off until sync_state() only for GDSCs that were already on at
> > > > > boot. So if an off-at-boot GDSC gets turned on, and then you attempt
> > > > > to turn it off before all its consumers have probed, it'll fail to
> > > > > power it off even though that wasn't necessary?
> > > >
> > > > I think we can circumvent looking at a GDSC by knowing it there was ever
> > > > a power on request since boot. I'll try to come up with something in the
> > > > new version.
> > >
> > > Please no. There's nothing wrong with reading the GDSC values. Please
> > > read them and don't turn on GDSC's that weren't on at boot.
> >
> > Sorry for the typos above, I basically said that for this concern of
> > yours, we can add the 'state at boot' thing you mentioned above by
> > looking at the GDSC (as in reading reg).
> >
> > >
> > > Otherwise you are making it a hassle for the case where there is a
> > > consumer without a driver for a GDSC that was off at boot. You are now
> > > forcing the use of timeouts or writing to state_synced file. Those
> > > should be absolute last resorts, but you are making that a requirement
> > > with your current implementation. If you implement it correctly by
> > > reading the GDSC register, things will "just work". And it's not even
> > > hard to do.
> > >
> > > NACK'ed until this is handled correctly.
> > >
> > > >
> > > > >
> > > > > 4) The returning -EBUSY when a power off is attempted seems to be
> > > > > quite wasteful. The framework will go through the whole sequence of
> > > > > trying to power down, send the notifications and then fail and then
> > > > > send the undo notifications. Combined with point (2) I think this can
> > > > > be handled better at the aggregation level in the framework to avoid
> > > > > even going that far into the power off sequence.
> > > >
> > > > Again, have a look at [1] (above).
> > >
> > > See my reply above. If you do it properly at the framework level, this
> > > can be done in a clean way and will work for all power domains.
> > >
> > > -Saravana
> > >
> > > >
> > > > Ulf, any thoughts on this 4th point?
>
> Please, see my reply above.
>
> [...]
>
> Kind regards
> Uffe

2023-04-06 11:11:50

by Ulf Hansson

Subject: Re: [PATCH v3 0/4] Allow genpd providers to power off domains on sync state

On Thu, 6 Apr 2023 at 11:26, Abel Vesa <[email protected]> wrote:
>
> On 23-04-05 16:11:18, Ulf Hansson wrote:
> > Abel, Saravana,
> >
> > On Fri, 31 Mar 2023 at 06:59, Abel Vesa <[email protected]> wrote:
> > >
> > > On 23-03-30 12:50:44, Saravana Kannan wrote:
> > > > On Thu, Mar 30, 2023 at 4:27 AM Abel Vesa <[email protected]> wrote:
> > > > >
> > > > > On 23-03-27 17:17:28, Saravana Kannan wrote:
> > > > > > On Mon, Mar 27, 2023 at 12:38 PM Abel Vesa <[email protected]> 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'm giving my thoughts in the cover letter instead of spreading it
> > > > > > around all the patches so that there's context between the comments.
> > > > > >
> > > > > > 1) Why can't all the logic in this patch series be implemented at the
> > > > > > framework level? And then allow the drivers to opt into this behavior
> > > > > > by setting the sync_state() callback.
> > > > > >
> > > > > > That way, you can land it only for QC drivers by setting up
> > > > > > sync_state() callback only for QC drivers, but actually have the same
> > > > > > code function correctly for non-QC drivers too. And then once we have
> > > > > > this functionality working properly for QC drivers for one kernel
> > > > > > version (or two), we'll just have the framework set the device's
> > > > > > driver's sync_state() if it doesn't have one already.
> > > > >
> > > > > I think Ulf has already NACK'ed that approach here:
> > > > > [1] https://lore.kernel.org/lkml/CAPDyKFon35wcQ+5kx3QZb-awN_S_q8y1Sir-G+GoxkCvpN=iiA@mail.gmail.com/
> > > >
> > > > I would have NACK'ed that too because that's an incomplete fix. As I
> > > > said further below, the fix needs to be at the aggregation level where
> > > > you aggregate all the current consumer requests. In there, you need to
> > > > add in the "state at boot" input that gets cleared out after a
> > > > sync_state() call is received for that power domain.
> > > >
> > >
> > > So, just to make sure I understand your point. You would rather have the
> > > genpd_power_off check if 'state at boot' is 'on' and return busy and
> > > then clear then, via a generic genpd sync state you would mark 'state at
> > > boot' as 'off' and queue up a power off request for each PD from there.
> > > And as for 'state at boot' it would check the enable bit through
> > > provider.
> > >
> > > Am I right so far?
> >
> > I am not sure I completely follow what you are suggesting here.
>
> Please have a look at this:
> https://git.kernel.org/pub/scm/linux/kernel/git/abelvesa/linux.git/commit/?h=qcom/genpd/ignore_unused_until_sync_state&id=4f9e6140dfe77884012383f8ba2140cadb62ca4a
>
> Keep in mind that is WIP for now. Once I have something, I'll post it on
> mailing list. Right now, there is a missing piece mentioned in that
> commit message.

I had a quick look and it seems rather promising, but I will have a
closer look when you post it.

>
> >
> > Although, let me point out that there is no requirement from the genpd
> > API point of view, that the provider needs to be a driver. This means
> > that the sync_state callback may not even be applicable for all genpd
> > providers.
>
> Yes, I'm considering that case too.

Good.

>
> >
> > In other words, it looks to me that we may need some new genpd helper
> > functions, no matter what. More importantly, it looks like we need an
> > opt-in behaviour, unless we can figure out a common way for genpd to
> > understand whether the sync_state thing is going to be applicable or
> > not. Maybe Saravana has some ideas around this?
> >
> > Note that, I don't object to extending genpd to be more clever and to
> > share common code, of course. We could, for example, make
> > genpd_power_off() to bail out earlier, rather than calling the
> > ->power_off() callback and waiting for it to return -EBUSY. Both of
> > you have pointed this out to me, in some of the earlier
> > replies/discussions too.
>
> The above link basically does this. I hope this is what Saravana has in
> mind as well.

Okay, let's see what Saravana thinks.

Maybe it's easier to post an RFC, based upon the above and continue
the discussion around that?

Kind regards
Uffe

2023-04-11 03:04:52

by Bjorn Andersson

Subject: Re: [PATCH v3 2/4] soc: qcom: rpmhpd: Do proper power off when state synced

On Mon, Mar 27, 2023 at 10:38:27PM +0300, Abel Vesa wrote:
> 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 <[email protected]>
> Reviewed-by: Ulf Hansson <[email protected]>
> ---
> 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..ec7926820772 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);

Note that the intent of this line is to lower the corner from max to
either a requested performance_state or the lowest non-disabled corner.
I don't think your solution maintains this behavior?

> - else
> - corner = 0;
> -
> - ret = rpmhpd_aggregate_corner(pd, corner);
> - if (ret)
> - dev_err(dev, "failed to sync %s\n", pd->res_name);
> + pm_genpd_queue_power_off(&pd->pd);

In the event that the power-domain has a single device attached, and no
subdomains, wouldn't pm_genpd_queue_power_off() pass straight through
all checks and turn off the power domain? Perhaps I'm just missing
something?

Regards,
Bjorn

> }
> mutex_unlock(&rpmhpd_lock);
> }
> --
> 2.34.1
>