* [PATCH v4 0/6] clk: qcom: use power-domain for sm8250's clock controllers
@ 2021-07-10 1:32 Dmitry Baryshkov
2021-07-10 1:32 ` [PATCH v4 1/6] dt-bindings: clock: qcom,dispcc-sm8x50: add mmcx power domain Dmitry Baryshkov
` (5 more replies)
0 siblings, 6 replies; 11+ messages in thread
From: Dmitry Baryshkov @ 2021-07-10 1:32 UTC (permalink / raw)
To: Andy Gross, Bjorn Andersson, Rob Herring, Stephen Boyd,
Taniya Das, Jonathan Marek, Michael Turquette
Cc: linux-arm-msm, devicetree, linux-clk, Bryan O'Donoghue,
Mark Brown, Ulf Hansson, linux-kernel
On SM8250 both the display and video clock controllers are powered up by
the MMCX power domain. Handle this by linking clock controllers to the
proper power domain, and using runtime power management to enable and
disable the MMCX power domain.
Dependencies:
- https://lore.kernel.org/linux-pm/20210603093438.138705-1-ulf.hansson@linaro.org/ (merged in 5.14)
- https://lore.kernel.org/linux-arm-msm/20210703005416.2668319-1-bjorn.andersson@linaro.org/
(pending)
Changes since v3:
- Wrap gdsc_enable/gdsc_disable into pm_runtime_get/put calls rather
than calling pm_runtime_get in gdsc_enabled and _put in gdsc_disable
- Squash gdsc patches together to remove possible dependencies between
two patches.
Changes since v2:
- Move pm_runtime calls from generic genpd code to the gdsc code for
now (as suggested by Ulf & Bjorn)
Changes since v1:
- Rebase on top of Bjorn's patches, removing the need for setting
performance state directly.
- Move runtime PM calls from GDSC code to generic genpd code.
- Always call pm_runtime_enable in the Qualcomm generic clock
controller code.
- Register GDSC power domains as subdomains of the domain powering the
clock controller if there is one.
----------------------------------------------------------------
Dmitry Baryshkov (6):
dt-bindings: clock: qcom,dispcc-sm8x50: add mmcx power domain
dt-bindings: clock: qcom,videocc: add mmcx power domain
clk: qcom: gdsc: enable optional power domain support
arm64: dts: qcom: sm8250: remove mmcx regulator
clk: qcom: dispcc-sm8250: stop using mmcx regulator
clk: qcom: videocc-sm8250: stop using mmcx regulator
.../bindings/clock/qcom,dispcc-sm8x50.yaml | 7 +++
.../devicetree/bindings/clock/qcom,videocc.yaml | 7 +++
arch/arm64/boot/dts/qcom/sm8250.dtsi | 11 +---
drivers/clk/qcom/common.c | 37 ++++++++++--
drivers/clk/qcom/dispcc-sm8250.c | 1 -
drivers/clk/qcom/gdsc.c | 67 +++++++++++++++++++++-
drivers/clk/qcom/gdsc.h | 2 +
drivers/clk/qcom/videocc-sm8250.c | 4 --
8 files changed, 113 insertions(+), 23 deletions(-)
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v4 1/6] dt-bindings: clock: qcom,dispcc-sm8x50: add mmcx power domain
2021-07-10 1:32 [PATCH v4 0/6] clk: qcom: use power-domain for sm8250's clock controllers Dmitry Baryshkov
@ 2021-07-10 1:32 ` Dmitry Baryshkov
2021-07-15 16:17 ` Rob Herring
2021-07-10 1:32 ` [PATCH v4 2/6] dt-bindings: clock: qcom,videocc: " Dmitry Baryshkov
` (4 subsequent siblings)
5 siblings, 1 reply; 11+ messages in thread
From: Dmitry Baryshkov @ 2021-07-10 1:32 UTC (permalink / raw)
To: Andy Gross, Bjorn Andersson, Rob Herring, Stephen Boyd,
Taniya Das, Jonathan Marek, Michael Turquette
Cc: linux-arm-msm, devicetree, linux-clk, Bryan O'Donoghue,
Mark Brown, Ulf Hansson, linux-kernel
On sm8250 dispcc requires MMCX power domain to be powered up before
clock controller's registers become available. For now sm8250 was using
external regulator driven by the power domain to describe this
relationship. Switch into specifying power-domain and required opp-state
directly.
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
.../devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/Documentation/devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml
index 0cdf53f41f84..d5c4fed56b6e 100644
--- a/Documentation/devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml
+++ b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml
@@ -55,6 +55,11 @@ properties:
reg:
maxItems: 1
+ power-domains:
+ description:
+ A phandle and PM domain specifier for the MMCX power domain.
+ maxItems: 1
+
required:
- compatible
- reg
@@ -69,6 +74,7 @@ additionalProperties: false
examples:
- |
#include <dt-bindings/clock/qcom,rpmh.h>
+ #include <dt-bindings/power/qcom-rpmpd.h>
clock-controller@af00000 {
compatible = "qcom,sm8250-dispcc";
reg = <0x0af00000 0x10000>;
@@ -89,5 +95,6 @@ examples:
#clock-cells = <1>;
#reset-cells = <1>;
#power-domain-cells = <1>;
+ power-domains = <&rpmhpd SM8250_MMCX>;
};
...
--
2.30.2
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v4 2/6] dt-bindings: clock: qcom,videocc: add mmcx power domain
2021-07-10 1:32 [PATCH v4 0/6] clk: qcom: use power-domain for sm8250's clock controllers Dmitry Baryshkov
2021-07-10 1:32 ` [PATCH v4 1/6] dt-bindings: clock: qcom,dispcc-sm8x50: add mmcx power domain Dmitry Baryshkov
@ 2021-07-10 1:32 ` Dmitry Baryshkov
2021-07-15 16:18 ` Rob Herring
2021-07-10 1:32 ` [PATCH v4 3/6] clk: qcom: gdsc: enable optional power domain support Dmitry Baryshkov
` (3 subsequent siblings)
5 siblings, 1 reply; 11+ messages in thread
From: Dmitry Baryshkov @ 2021-07-10 1:32 UTC (permalink / raw)
To: Andy Gross, Bjorn Andersson, Rob Herring, Stephen Boyd,
Taniya Das, Jonathan Marek, Michael Turquette
Cc: linux-arm-msm, devicetree, linux-clk, Bryan O'Donoghue,
Mark Brown, Ulf Hansson, linux-kernel
On sm8250 videocc requires MMCX power domain to be powered up before
clock controller's registers become available. For now sm8250 was using
external regulator driven by the power domain to describe this
relationship. Switch into specifying power-domain and required opp-state
directly.
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
Documentation/devicetree/bindings/clock/qcom,videocc.yaml | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/Documentation/devicetree/bindings/clock/qcom,videocc.yaml b/Documentation/devicetree/bindings/clock/qcom,videocc.yaml
index 567202942b88..db4ada6acf27 100644
--- a/Documentation/devicetree/bindings/clock/qcom,videocc.yaml
+++ b/Documentation/devicetree/bindings/clock/qcom,videocc.yaml
@@ -47,6 +47,11 @@ properties:
reg:
maxItems: 1
+ power-domains:
+ description:
+ A phandle and PM domain specifier for the MMCX power domain.
+ maxItems: 1
+
required:
- compatible
- reg
@@ -61,6 +66,7 @@ additionalProperties: false
examples:
- |
#include <dt-bindings/clock/qcom,rpmh.h>
+ #include <dt-bindings/power/qcom-rpmpd.h>
clock-controller@ab00000 {
compatible = "qcom,sdm845-videocc";
reg = <0x0ab00000 0x10000>;
@@ -69,5 +75,6 @@ examples:
#clock-cells = <1>;
#reset-cells = <1>;
#power-domain-cells = <1>;
+ power-domains = <&rpmhpd SM8250_MMCX>;
};
...
--
2.30.2
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v4 3/6] clk: qcom: gdsc: enable optional power domain support
2021-07-10 1:32 [PATCH v4 0/6] clk: qcom: use power-domain for sm8250's clock controllers Dmitry Baryshkov
2021-07-10 1:32 ` [PATCH v4 1/6] dt-bindings: clock: qcom,dispcc-sm8x50: add mmcx power domain Dmitry Baryshkov
2021-07-10 1:32 ` [PATCH v4 2/6] dt-bindings: clock: qcom,videocc: " Dmitry Baryshkov
@ 2021-07-10 1:32 ` Dmitry Baryshkov
2021-07-10 3:44 ` Bjorn Andersson
2021-07-10 1:32 ` [PATCH v4 4/6] arm64: dts: qcom: sm8250: remove mmcx regulator Dmitry Baryshkov
` (2 subsequent siblings)
5 siblings, 1 reply; 11+ messages in thread
From: Dmitry Baryshkov @ 2021-07-10 1:32 UTC (permalink / raw)
To: Andy Gross, Bjorn Andersson, Rob Herring, Stephen Boyd,
Taniya Das, Jonathan Marek, Michael Turquette
Cc: linux-arm-msm, devicetree, linux-clk, Bryan O'Donoghue,
Mark Brown, Ulf Hansson, linux-kernel
On sm8250 dispcc and videocc registers are powered up by the MMCX power
domain. Currently we use a regulator to enable this domain on demand,
however this has some consequences, as genpd code is not reentrant.
Teach Qualcomm clock controller code about setting up runtime PM and
using specified genpd for gdsc powerup. Clock core will use runtime_pm
calls to ensure that clock registers are accessible. The genpd code will
powerup the parent domain when gdsc is powered, thus ensuring that the
power stays on. Make gdsc code also use pm_runtime calls to ensure that
registers are accessible during the gdsc_enable/gdsc_disable operations.
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
drivers/clk/qcom/common.c | 37 +++++++++++++++++----
drivers/clk/qcom/gdsc.c | 67 +++++++++++++++++++++++++++++++++++++--
drivers/clk/qcom/gdsc.h | 2 ++
3 files changed, 97 insertions(+), 9 deletions(-)
diff --git a/drivers/clk/qcom/common.c b/drivers/clk/qcom/common.c
index 60d2a78d1395..43d8f8feeb3c 100644
--- a/drivers/clk/qcom/common.c
+++ b/drivers/clk/qcom/common.c
@@ -10,6 +10,7 @@
#include <linux/clk-provider.h>
#include <linux/reset-controller.h>
#include <linux/of.h>
+#include <linux/pm_runtime.h>
#include "common.h"
#include "clk-rcg.h"
@@ -224,6 +225,11 @@ static struct clk_hw *qcom_cc_clk_hw_get(struct of_phandle_args *clkspec,
return cc->rclks[idx] ? &cc->rclks[idx]->hw : NULL;
}
+static void qcom_cc_pm_runtime_disable(void *data)
+{
+ pm_runtime_disable(data);
+}
+
int qcom_cc_really_probe(struct platform_device *pdev,
const struct qcom_cc_desc *desc, struct regmap *regmap)
{
@@ -241,6 +247,18 @@ int qcom_cc_really_probe(struct platform_device *pdev,
if (!cc)
return -ENOMEM;
+ pm_runtime_enable(dev);
+ ret = pm_runtime_get_sync(dev);
+ if (ret < 0) {
+ pm_runtime_put(dev);
+ pm_runtime_disable(dev);
+ return ret;
+ }
+
+ ret = devm_add_action_or_reset(dev, qcom_cc_pm_runtime_disable, dev);
+ if (ret)
+ goto err;
+
reset = &cc->reset;
reset->rcdev.of_node = dev->of_node;
reset->rcdev.ops = &qcom_reset_ops;
@@ -251,7 +269,7 @@ int qcom_cc_really_probe(struct platform_device *pdev,
ret = devm_reset_controller_register(dev, &reset->rcdev);
if (ret)
- return ret;
+ goto err;
if (desc->gdscs && desc->num_gdscs) {
scd = devm_kzalloc(dev, sizeof(*scd), GFP_KERNEL);
@@ -262,11 +280,11 @@ int qcom_cc_really_probe(struct platform_device *pdev,
scd->num = desc->num_gdscs;
ret = gdsc_register(scd, &reset->rcdev, regmap);
if (ret)
- return ret;
+ goto err;
ret = devm_add_action_or_reset(dev, qcom_cc_gdsc_unregister,
scd);
if (ret)
- return ret;
+ goto err;
}
cc->rclks = rclks;
@@ -277,7 +295,7 @@ int qcom_cc_really_probe(struct platform_device *pdev,
for (i = 0; i < num_clk_hws; i++) {
ret = devm_clk_hw_register(dev, clk_hws[i]);
if (ret)
- return ret;
+ goto err;
}
for (i = 0; i < num_clks; i++) {
@@ -286,14 +304,21 @@ int qcom_cc_really_probe(struct platform_device *pdev,
ret = devm_clk_register_regmap(dev, rclks[i]);
if (ret)
- return ret;
+ goto err;
}
ret = devm_of_clk_add_hw_provider(dev, qcom_cc_clk_hw_get, cc);
if (ret)
- return ret;
+ goto err;
+
+ pm_runtime_put(dev);
return 0;
+
+err:
+ pm_runtime_put(dev);
+
+ return ret;
}
EXPORT_SYMBOL_GPL(qcom_cc_really_probe);
diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c
index 51ed640e527b..cfe875f87efe 100644
--- a/drivers/clk/qcom/gdsc.c
+++ b/drivers/clk/qcom/gdsc.c
@@ -11,6 +11,7 @@
#include <linux/kernel.h>
#include <linux/ktime.h>
#include <linux/pm_domain.h>
+#include <linux/pm_runtime.h>
#include <linux/regmap.h>
#include <linux/regulator/consumer.h>
#include <linux/reset-controller.h>
@@ -50,6 +51,30 @@ enum gdsc_status {
GDSC_ON
};
+static int gdsc_pm_runtime_get(struct gdsc *sc)
+{
+ int ret;
+
+ if (!sc->dev)
+ return 0;
+
+ ret = pm_runtime_get_sync(sc->dev);
+ if (ret < 0) {
+ pm_runtime_put_noidle(sc->dev);
+ return ret;
+ }
+
+ return 0;
+}
+
+static int gdsc_pm_runtime_put(struct gdsc *sc)
+{
+ if (!sc->dev)
+ return 0;
+
+ return pm_runtime_put_sync(sc->dev);
+}
+
/* Returns 1 if GDSC status is status, 0 if not, and < 0 on error */
static int gdsc_check_status(struct gdsc *sc, enum gdsc_status status)
{
@@ -232,9 +257,8 @@ static void gdsc_retain_ff_on(struct gdsc *sc)
regmap_update_bits(sc->regmap, sc->gdscr, mask, mask);
}
-static int gdsc_enable(struct generic_pm_domain *domain)
+static int _gdsc_enable(struct gdsc *sc)
{
- struct gdsc *sc = domain_to_gdsc(domain);
int ret;
if (sc->pwrsts == PWRSTS_ON)
@@ -290,11 +314,26 @@ static int gdsc_enable(struct generic_pm_domain *domain)
return 0;
}
-static int gdsc_disable(struct generic_pm_domain *domain)
+static int gdsc_enable(struct generic_pm_domain *domain)
{
struct gdsc *sc = domain_to_gdsc(domain);
int ret;
+ ret = gdsc_pm_runtime_get(sc);
+ if (ret)
+ return ret;
+
+ ret = _gdsc_enable(sc);
+
+ gdsc_pm_runtime_put(sc);
+
+ return ret;
+}
+
+static int _gdsc_disable(struct gdsc *sc)
+{
+ int ret;
+
if (sc->pwrsts == PWRSTS_ON)
return gdsc_assert_reset(sc);
@@ -329,6 +368,22 @@ static int gdsc_disable(struct generic_pm_domain *domain)
return 0;
}
+static int gdsc_disable(struct generic_pm_domain *domain)
+{
+ struct gdsc *sc = domain_to_gdsc(domain);
+ int ret;
+
+ ret = gdsc_pm_runtime_get(sc);
+ if (ret)
+ return ret;
+
+ ret = _gdsc_disable(sc);
+
+ gdsc_pm_runtime_put(sc);
+
+ return ret;
+}
+
static int gdsc_init(struct gdsc *sc)
{
u32 mask, val;
@@ -425,6 +480,8 @@ int gdsc_register(struct gdsc_desc *desc,
for (i = 0; i < num; i++) {
if (!scs[i])
continue;
+ if (pm_runtime_enabled(dev))
+ scs[i]->dev = dev;
scs[i]->regmap = regmap;
scs[i]->rcdev = rcdev;
ret = gdsc_init(scs[i]);
@@ -439,6 +496,8 @@ int gdsc_register(struct gdsc_desc *desc,
continue;
if (scs[i]->parent)
pm_genpd_add_subdomain(scs[i]->parent, &scs[i]->pd);
+ else if (!IS_ERR_OR_NULL(dev->pm_domain))
+ pm_genpd_add_subdomain(pd_to_genpd(dev->pm_domain), &scs[i]->pd);
}
return of_genpd_add_provider_onecell(dev->of_node, data);
@@ -457,6 +516,8 @@ void gdsc_unregister(struct gdsc_desc *desc)
continue;
if (scs[i]->parent)
pm_genpd_remove_subdomain(scs[i]->parent, &scs[i]->pd);
+ else if (!IS_ERR_OR_NULL(dev->pm_domain))
+ pm_genpd_remove_subdomain(pd_to_genpd(dev->pm_domain), &scs[i]->pd);
}
of_genpd_del_provider(dev->of_node);
}
diff --git a/drivers/clk/qcom/gdsc.h b/drivers/clk/qcom/gdsc.h
index 5bb396b344d1..702d47a87af6 100644
--- a/drivers/clk/qcom/gdsc.h
+++ b/drivers/clk/qcom/gdsc.h
@@ -25,6 +25,7 @@ struct reset_controller_dev;
* @resets: ids of resets associated with this gdsc
* @reset_count: number of @resets
* @rcdev: reset controller
+ * @dev: the device holding the GDSC, used for pm_runtime calls
*/
struct gdsc {
struct generic_pm_domain pd;
@@ -58,6 +59,7 @@ struct gdsc {
const char *supply;
struct regulator *rsupply;
+ struct device *dev;
};
struct gdsc_desc {
--
2.30.2
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v4 4/6] arm64: dts: qcom: sm8250: remove mmcx regulator
2021-07-10 1:32 [PATCH v4 0/6] clk: qcom: use power-domain for sm8250's clock controllers Dmitry Baryshkov
` (2 preceding siblings ...)
2021-07-10 1:32 ` [PATCH v4 3/6] clk: qcom: gdsc: enable optional power domain support Dmitry Baryshkov
@ 2021-07-10 1:32 ` Dmitry Baryshkov
2021-07-10 1:32 ` [PATCH v4 5/6] clk: qcom: dispcc-sm8250: stop using " Dmitry Baryshkov
2021-07-10 1:32 ` [PATCH v4 6/6] clk: qcom: videocc-sm8250: " Dmitry Baryshkov
5 siblings, 0 replies; 11+ messages in thread
From: Dmitry Baryshkov @ 2021-07-10 1:32 UTC (permalink / raw)
To: Andy Gross, Bjorn Andersson, Rob Herring, Stephen Boyd,
Taniya Das, Jonathan Marek, Michael Turquette
Cc: linux-arm-msm, devicetree, linux-clk, Bryan O'Donoghue,
Mark Brown, Ulf Hansson, linux-kernel
Switch dispcc and videocc into using MMCX domain directly. Drop the now
unused mmcx regulator.
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
arch/arm64/boot/dts/qcom/sm8250.dtsi | 11 ++---------
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
index 4c0de12aaba6..2a468b85dc09 100644
--- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
@@ -271,13 +271,6 @@ memory@80000000 {
reg = <0x0 0x80000000 0x0 0x0>;
};
- mmcx_reg: mmcx-reg {
- compatible = "regulator-fixed-domain";
- power-domains = <&rpmhpd SM8250_MMCX>;
- required-opps = <&rpmhpd_opp_low_svs>;
- regulator-name = "MMCX";
- };
-
pmu {
compatible = "arm,armv8-pmuv3";
interrupts = <GIC_PPI 7 IRQ_TYPE_LEVEL_LOW>;
@@ -2362,7 +2355,7 @@ videocc: clock-controller@abf0000 {
clocks = <&gcc GCC_VIDEO_AHB_CLK>,
<&rpmhcc RPMH_CXO_CLK>,
<&rpmhcc RPMH_CXO_CLK_A>;
- mmcx-supply = <&mmcx_reg>;
+ power-domains = <&rpmhpd SM8250_MMCX>;
clock-names = "iface", "bi_tcxo", "bi_tcxo_ao";
#clock-cells = <1>;
#reset-cells = <1>;
@@ -2627,7 +2620,7 @@ opp-358000000 {
dispcc: clock-controller@af00000 {
compatible = "qcom,sm8250-dispcc";
reg = <0 0x0af00000 0 0x10000>;
- mmcx-supply = <&mmcx_reg>;
+ power-domains = <&rpmhpd SM8250_MMCX>;
clocks = <&rpmhcc RPMH_CXO_CLK>,
<&dsi0_phy 0>,
<&dsi0_phy 1>,
--
2.30.2
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v4 5/6] clk: qcom: dispcc-sm8250: stop using mmcx regulator
2021-07-10 1:32 [PATCH v4 0/6] clk: qcom: use power-domain for sm8250's clock controllers Dmitry Baryshkov
` (3 preceding siblings ...)
2021-07-10 1:32 ` [PATCH v4 4/6] arm64: dts: qcom: sm8250: remove mmcx regulator Dmitry Baryshkov
@ 2021-07-10 1:32 ` Dmitry Baryshkov
2021-07-10 1:32 ` [PATCH v4 6/6] clk: qcom: videocc-sm8250: " Dmitry Baryshkov
5 siblings, 0 replies; 11+ messages in thread
From: Dmitry Baryshkov @ 2021-07-10 1:32 UTC (permalink / raw)
To: Andy Gross, Bjorn Andersson, Rob Herring, Stephen Boyd,
Taniya Das, Jonathan Marek, Michael Turquette
Cc: linux-arm-msm, devicetree, linux-clk, Bryan O'Donoghue,
Mark Brown, Ulf Hansson, linux-kernel
Now as the common qcom clock controller code has been taught about power
domains, stop mentioning mmcx supply as a way to power up the clock
controller's gdsc.
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
drivers/clk/qcom/dispcc-sm8250.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/clk/qcom/dispcc-sm8250.c b/drivers/clk/qcom/dispcc-sm8250.c
index de09cd5c209f..dfbfe64b12f6 100644
--- a/drivers/clk/qcom/dispcc-sm8250.c
+++ b/drivers/clk/qcom/dispcc-sm8250.c
@@ -955,7 +955,6 @@ static struct gdsc mdss_gdsc = {
},
.pwrsts = PWRSTS_OFF_ON,
.flags = HW_CTRL,
- .supply = "mmcx",
};
static struct clk_regmap *disp_cc_sm8250_clocks[] = {
--
2.30.2
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v4 6/6] clk: qcom: videocc-sm8250: stop using mmcx regulator
2021-07-10 1:32 [PATCH v4 0/6] clk: qcom: use power-domain for sm8250's clock controllers Dmitry Baryshkov
` (4 preceding siblings ...)
2021-07-10 1:32 ` [PATCH v4 5/6] clk: qcom: dispcc-sm8250: stop using " Dmitry Baryshkov
@ 2021-07-10 1:32 ` Dmitry Baryshkov
5 siblings, 0 replies; 11+ messages in thread
From: Dmitry Baryshkov @ 2021-07-10 1:32 UTC (permalink / raw)
To: Andy Gross, Bjorn Andersson, Rob Herring, Stephen Boyd,
Taniya Das, Jonathan Marek, Michael Turquette
Cc: linux-arm-msm, devicetree, linux-clk, Bryan O'Donoghue,
Mark Brown, Ulf Hansson, linux-kernel
Now as the common qcom clock controller code has been taught about power
domains, stop mentioning mmcx supply as a way to power up the clock
controller's gdscs.
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
drivers/clk/qcom/videocc-sm8250.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/drivers/clk/qcom/videocc-sm8250.c b/drivers/clk/qcom/videocc-sm8250.c
index 7b435a1c2c4b..eedef85d90e5 100644
--- a/drivers/clk/qcom/videocc-sm8250.c
+++ b/drivers/clk/qcom/videocc-sm8250.c
@@ -276,7 +276,6 @@ static struct gdsc mvs0c_gdsc = {
},
.flags = 0,
.pwrsts = PWRSTS_OFF_ON,
- .supply = "mmcx",
};
static struct gdsc mvs1c_gdsc = {
@@ -286,7 +285,6 @@ static struct gdsc mvs1c_gdsc = {
},
.flags = 0,
.pwrsts = PWRSTS_OFF_ON,
- .supply = "mmcx",
};
static struct gdsc mvs0_gdsc = {
@@ -296,7 +294,6 @@ static struct gdsc mvs0_gdsc = {
},
.flags = HW_CTRL,
.pwrsts = PWRSTS_OFF_ON,
- .supply = "mmcx",
};
static struct gdsc mvs1_gdsc = {
@@ -306,7 +303,6 @@ static struct gdsc mvs1_gdsc = {
},
.flags = HW_CTRL,
.pwrsts = PWRSTS_OFF_ON,
- .supply = "mmcx",
};
static struct clk_regmap *video_cc_sm8250_clocks[] = {
--
2.30.2
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v4 3/6] clk: qcom: gdsc: enable optional power domain support
2021-07-10 1:32 ` [PATCH v4 3/6] clk: qcom: gdsc: enable optional power domain support Dmitry Baryshkov
@ 2021-07-10 3:44 ` Bjorn Andersson
2021-07-10 11:50 ` Dmitry Baryshkov
0 siblings, 1 reply; 11+ messages in thread
From: Bjorn Andersson @ 2021-07-10 3:44 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Andy Gross, Rob Herring, Stephen Boyd, Taniya Das,
Jonathan Marek, Michael Turquette, linux-arm-msm, devicetree,
linux-clk, Bryan O'Donoghue, Mark Brown, Ulf Hansson,
linux-kernel
On Fri 09 Jul 20:32 CDT 2021, Dmitry Baryshkov wrote:
> On sm8250 dispcc and videocc registers are powered up by the MMCX power
> domain. Currently we use a regulator to enable this domain on demand,
> however this has some consequences, as genpd code is not reentrant.
>
> Teach Qualcomm clock controller code about setting up runtime PM and
> using specified genpd for gdsc powerup. Clock core will use runtime_pm
> calls to ensure that clock registers are accessible. The genpd code will
> powerup the parent domain when gdsc is powered, thus ensuring that the
> power stays on. Make gdsc code also use pm_runtime calls to ensure that
> registers are accessible during the gdsc_enable/gdsc_disable operations.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
> drivers/clk/qcom/common.c | 37 +++++++++++++++++----
> drivers/clk/qcom/gdsc.c | 67 +++++++++++++++++++++++++++++++++++++--
> drivers/clk/qcom/gdsc.h | 2 ++
> 3 files changed, 97 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/clk/qcom/common.c b/drivers/clk/qcom/common.c
> index 60d2a78d1395..43d8f8feeb3c 100644
> --- a/drivers/clk/qcom/common.c
> +++ b/drivers/clk/qcom/common.c
> @@ -10,6 +10,7 @@
> #include <linux/clk-provider.h>
> #include <linux/reset-controller.h>
> #include <linux/of.h>
> +#include <linux/pm_runtime.h>
>
> #include "common.h"
> #include "clk-rcg.h"
> @@ -224,6 +225,11 @@ static struct clk_hw *qcom_cc_clk_hw_get(struct of_phandle_args *clkspec,
> return cc->rclks[idx] ? &cc->rclks[idx]->hw : NULL;
> }
>
> +static void qcom_cc_pm_runtime_disable(void *data)
> +{
> + pm_runtime_disable(data);
> +}
> +
> int qcom_cc_really_probe(struct platform_device *pdev,
> const struct qcom_cc_desc *desc, struct regmap *regmap)
> {
> @@ -241,6 +247,18 @@ int qcom_cc_really_probe(struct platform_device *pdev,
> if (!cc)
> return -ENOMEM;
>
> + pm_runtime_enable(dev);
As I said in v3, this will result in double pm_runtime_enable(), which
is not allowed. What I didn't remember is that we have 6 drivers that
rely on pm_runtime to keep their iface clock enabled during clock
operations, so it's not only isolated to the turingcc driver.
> + ret = pm_runtime_get_sync(dev);
And I still think that you should add this to gdsc_init(), like you do
in gdsc_enable() and gdsc_disable.
Combined, there are no longer any changes to common.c
The rest of this looks good now!
Regards,
Bjorn
> + if (ret < 0) {
> + pm_runtime_put(dev);
> + pm_runtime_disable(dev);
> + return ret;
> + }
> +
> + ret = devm_add_action_or_reset(dev, qcom_cc_pm_runtime_disable, dev);
> + if (ret)
> + goto err;
> +
> reset = &cc->reset;
> reset->rcdev.of_node = dev->of_node;
> reset->rcdev.ops = &qcom_reset_ops;
> @@ -251,7 +269,7 @@ int qcom_cc_really_probe(struct platform_device *pdev,
>
> ret = devm_reset_controller_register(dev, &reset->rcdev);
> if (ret)
> - return ret;
> + goto err;
>
> if (desc->gdscs && desc->num_gdscs) {
> scd = devm_kzalloc(dev, sizeof(*scd), GFP_KERNEL);
> @@ -262,11 +280,11 @@ int qcom_cc_really_probe(struct platform_device *pdev,
> scd->num = desc->num_gdscs;
> ret = gdsc_register(scd, &reset->rcdev, regmap);
> if (ret)
> - return ret;
> + goto err;
> ret = devm_add_action_or_reset(dev, qcom_cc_gdsc_unregister,
> scd);
> if (ret)
> - return ret;
> + goto err;
> }
>
> cc->rclks = rclks;
> @@ -277,7 +295,7 @@ int qcom_cc_really_probe(struct platform_device *pdev,
> for (i = 0; i < num_clk_hws; i++) {
> ret = devm_clk_hw_register(dev, clk_hws[i]);
> if (ret)
> - return ret;
> + goto err;
> }
>
> for (i = 0; i < num_clks; i++) {
> @@ -286,14 +304,21 @@ int qcom_cc_really_probe(struct platform_device *pdev,
>
> ret = devm_clk_register_regmap(dev, rclks[i]);
> if (ret)
> - return ret;
> + goto err;
> }
>
> ret = devm_of_clk_add_hw_provider(dev, qcom_cc_clk_hw_get, cc);
> if (ret)
> - return ret;
> + goto err;
> +
> + pm_runtime_put(dev);
>
> return 0;
> +
> +err:
> + pm_runtime_put(dev);
> +
> + return ret;
> }
> EXPORT_SYMBOL_GPL(qcom_cc_really_probe);
>
> diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c
> index 51ed640e527b..cfe875f87efe 100644
> --- a/drivers/clk/qcom/gdsc.c
> +++ b/drivers/clk/qcom/gdsc.c
> @@ -11,6 +11,7 @@
> #include <linux/kernel.h>
> #include <linux/ktime.h>
> #include <linux/pm_domain.h>
> +#include <linux/pm_runtime.h>
> #include <linux/regmap.h>
> #include <linux/regulator/consumer.h>
> #include <linux/reset-controller.h>
> @@ -50,6 +51,30 @@ enum gdsc_status {
> GDSC_ON
> };
>
> +static int gdsc_pm_runtime_get(struct gdsc *sc)
> +{
> + int ret;
> +
> + if (!sc->dev)
> + return 0;
> +
> + ret = pm_runtime_get_sync(sc->dev);
> + if (ret < 0) {
> + pm_runtime_put_noidle(sc->dev);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int gdsc_pm_runtime_put(struct gdsc *sc)
> +{
> + if (!sc->dev)
> + return 0;
> +
> + return pm_runtime_put_sync(sc->dev);
> +}
> +
> /* Returns 1 if GDSC status is status, 0 if not, and < 0 on error */
> static int gdsc_check_status(struct gdsc *sc, enum gdsc_status status)
> {
> @@ -232,9 +257,8 @@ static void gdsc_retain_ff_on(struct gdsc *sc)
> regmap_update_bits(sc->regmap, sc->gdscr, mask, mask);
> }
>
> -static int gdsc_enable(struct generic_pm_domain *domain)
> +static int _gdsc_enable(struct gdsc *sc)
> {
> - struct gdsc *sc = domain_to_gdsc(domain);
> int ret;
>
> if (sc->pwrsts == PWRSTS_ON)
> @@ -290,11 +314,26 @@ static int gdsc_enable(struct generic_pm_domain *domain)
> return 0;
> }
>
> -static int gdsc_disable(struct generic_pm_domain *domain)
> +static int gdsc_enable(struct generic_pm_domain *domain)
> {
> struct gdsc *sc = domain_to_gdsc(domain);
> int ret;
>
> + ret = gdsc_pm_runtime_get(sc);
> + if (ret)
> + return ret;
> +
> + ret = _gdsc_enable(sc);
> +
> + gdsc_pm_runtime_put(sc);
> +
> + return ret;
> +}
> +
> +static int _gdsc_disable(struct gdsc *sc)
> +{
> + int ret;
> +
> if (sc->pwrsts == PWRSTS_ON)
> return gdsc_assert_reset(sc);
>
> @@ -329,6 +368,22 @@ static int gdsc_disable(struct generic_pm_domain *domain)
> return 0;
> }
>
> +static int gdsc_disable(struct generic_pm_domain *domain)
> +{
> + struct gdsc *sc = domain_to_gdsc(domain);
> + int ret;
> +
> + ret = gdsc_pm_runtime_get(sc);
> + if (ret)
> + return ret;
> +
> + ret = _gdsc_disable(sc);
> +
> + gdsc_pm_runtime_put(sc);
> +
> + return ret;
> +}
> +
> static int gdsc_init(struct gdsc *sc)
> {
> u32 mask, val;
> @@ -425,6 +480,8 @@ int gdsc_register(struct gdsc_desc *desc,
> for (i = 0; i < num; i++) {
> if (!scs[i])
> continue;
> + if (pm_runtime_enabled(dev))
> + scs[i]->dev = dev;
> scs[i]->regmap = regmap;
> scs[i]->rcdev = rcdev;
> ret = gdsc_init(scs[i]);
> @@ -439,6 +496,8 @@ int gdsc_register(struct gdsc_desc *desc,
> continue;
> if (scs[i]->parent)
> pm_genpd_add_subdomain(scs[i]->parent, &scs[i]->pd);
> + else if (!IS_ERR_OR_NULL(dev->pm_domain))
> + pm_genpd_add_subdomain(pd_to_genpd(dev->pm_domain), &scs[i]->pd);
> }
>
> return of_genpd_add_provider_onecell(dev->of_node, data);
> @@ -457,6 +516,8 @@ void gdsc_unregister(struct gdsc_desc *desc)
> continue;
> if (scs[i]->parent)
> pm_genpd_remove_subdomain(scs[i]->parent, &scs[i]->pd);
> + else if (!IS_ERR_OR_NULL(dev->pm_domain))
> + pm_genpd_remove_subdomain(pd_to_genpd(dev->pm_domain), &scs[i]->pd);
> }
> of_genpd_del_provider(dev->of_node);
> }
> diff --git a/drivers/clk/qcom/gdsc.h b/drivers/clk/qcom/gdsc.h
> index 5bb396b344d1..702d47a87af6 100644
> --- a/drivers/clk/qcom/gdsc.h
> +++ b/drivers/clk/qcom/gdsc.h
> @@ -25,6 +25,7 @@ struct reset_controller_dev;
> * @resets: ids of resets associated with this gdsc
> * @reset_count: number of @resets
> * @rcdev: reset controller
> + * @dev: the device holding the GDSC, used for pm_runtime calls
> */
> struct gdsc {
> struct generic_pm_domain pd;
> @@ -58,6 +59,7 @@ struct gdsc {
>
> const char *supply;
> struct regulator *rsupply;
> + struct device *dev;
> };
>
> struct gdsc_desc {
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v4 3/6] clk: qcom: gdsc: enable optional power domain support
2021-07-10 3:44 ` Bjorn Andersson
@ 2021-07-10 11:50 ` Dmitry Baryshkov
0 siblings, 0 replies; 11+ messages in thread
From: Dmitry Baryshkov @ 2021-07-10 11:50 UTC (permalink / raw)
To: Bjorn Andersson
Cc: Andy Gross, Rob Herring, Stephen Boyd, Taniya Das,
Jonathan Marek, Michael Turquette,
open list:DRM DRIVER FOR MSM ADRENO GPU,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list:COMMON CLK FRAMEWORK, Bryan O'Donoghue, Mark Brown,
Ulf Hansson, open list
On Sat, 10 Jul 2021 at 06:44, Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
>
> On Fri 09 Jul 20:32 CDT 2021, Dmitry Baryshkov wrote:
>
> > On sm8250 dispcc and videocc registers are powered up by the MMCX power
> > domain. Currently we use a regulator to enable this domain on demand,
> > however this has some consequences, as genpd code is not reentrant.
> >
> > Teach Qualcomm clock controller code about setting up runtime PM and
> > using specified genpd for gdsc powerup. Clock core will use runtime_pm
> > calls to ensure that clock registers are accessible. The genpd code will
> > powerup the parent domain when gdsc is powered, thus ensuring that the
> > power stays on. Make gdsc code also use pm_runtime calls to ensure that
> > registers are accessible during the gdsc_enable/gdsc_disable operations.
> >
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > ---
> > drivers/clk/qcom/common.c | 37 +++++++++++++++++----
> > drivers/clk/qcom/gdsc.c | 67 +++++++++++++++++++++++++++++++++++++--
> > drivers/clk/qcom/gdsc.h | 2 ++
> > 3 files changed, 97 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/clk/qcom/common.c b/drivers/clk/qcom/common.c
> > index 60d2a78d1395..43d8f8feeb3c 100644
> > --- a/drivers/clk/qcom/common.c
> > +++ b/drivers/clk/qcom/common.c
> > @@ -10,6 +10,7 @@
> > #include <linux/clk-provider.h>
> > #include <linux/reset-controller.h>
> > #include <linux/of.h>
> > +#include <linux/pm_runtime.h>
> >
> > #include "common.h"
> > #include "clk-rcg.h"
> > @@ -224,6 +225,11 @@ static struct clk_hw *qcom_cc_clk_hw_get(struct of_phandle_args *clkspec,
> > return cc->rclks[idx] ? &cc->rclks[idx]->hw : NULL;
> > }
> >
> > +static void qcom_cc_pm_runtime_disable(void *data)
> > +{
> > + pm_runtime_disable(data);
> > +}
> > +
> > int qcom_cc_really_probe(struct platform_device *pdev,
> > const struct qcom_cc_desc *desc, struct regmap *regmap)
> > {
> > @@ -241,6 +247,18 @@ int qcom_cc_really_probe(struct platform_device *pdev,
> > if (!cc)
> > return -ENOMEM;
> >
> > + pm_runtime_enable(dev);
>
> As I said in v3, this will result in double pm_runtime_enable(), which
> is not allowed. What I didn't remember is that we have 6 drivers that
> rely on pm_runtime to keep their iface clock enabled during clock
> operations, so it's not only isolated to the turingcc driver.
This quickly becomes more than just mmcx.
As we have 6 drivers using pm_runtime, let's do it another way. I'd
send a patch moving
pm_runtime_enable/pm_clk_add to qcom_cc_map(). If it is accepted, we
can proceed with mmcx.
Bonus points if it would become logical to rewrite lpass-gfm-sm8250 to
use qcom_cc framework
>
> > + ret = pm_runtime_get_sync(dev);
>
> And I still think that you should add this to gdsc_init(), like you do
> in gdsc_enable() and gdsc_disable.
I do not think so. clk_FOO_pll_configure, clk gating, setting up
always-on CLKs would not use pm_runtime calls, but will do register
writes. So I think it simpler to get the device for the whole _probe
time rather than micro-managing it for this and that events.
In the end (see above), I think we will pm_runtime_get in the
qcom_cc_map and pm_runtime_put in qcom_cc_really_probe.
>
> Combined, there are no longer any changes to common.c
>
>
> The rest of this looks good now!
>
> Regards,
> Bjorn
>
> > + if (ret < 0) {
> > + pm_runtime_put(dev);
> > + pm_runtime_disable(dev);
> > + return ret;
> > + }
> > +
> > + ret = devm_add_action_or_reset(dev, qcom_cc_pm_runtime_disable, dev);
> > + if (ret)
> > + goto err;
> > +
> > reset = &cc->reset;
> > reset->rcdev.of_node = dev->of_node;
> > reset->rcdev.ops = &qcom_reset_ops;
> > @@ -251,7 +269,7 @@ int qcom_cc_really_probe(struct platform_device *pdev,
> >
> > ret = devm_reset_controller_register(dev, &reset->rcdev);
> > if (ret)
> > - return ret;
> > + goto err;
> >
> > if (desc->gdscs && desc->num_gdscs) {
> > scd = devm_kzalloc(dev, sizeof(*scd), GFP_KERNEL);
> > @@ -262,11 +280,11 @@ int qcom_cc_really_probe(struct platform_device *pdev,
> > scd->num = desc->num_gdscs;
> > ret = gdsc_register(scd, &reset->rcdev, regmap);
> > if (ret)
> > - return ret;
> > + goto err;
> > ret = devm_add_action_or_reset(dev, qcom_cc_gdsc_unregister,
> > scd);
> > if (ret)
> > - return ret;
> > + goto err;
> > }
> >
> > cc->rclks = rclks;
> > @@ -277,7 +295,7 @@ int qcom_cc_really_probe(struct platform_device *pdev,
> > for (i = 0; i < num_clk_hws; i++) {
> > ret = devm_clk_hw_register(dev, clk_hws[i]);
> > if (ret)
> > - return ret;
> > + goto err;
> > }
> >
> > for (i = 0; i < num_clks; i++) {
> > @@ -286,14 +304,21 @@ int qcom_cc_really_probe(struct platform_device *pdev,
> >
> > ret = devm_clk_register_regmap(dev, rclks[i]);
> > if (ret)
> > - return ret;
> > + goto err;
> > }
> >
> > ret = devm_of_clk_add_hw_provider(dev, qcom_cc_clk_hw_get, cc);
> > if (ret)
> > - return ret;
> > + goto err;
> > +
> > + pm_runtime_put(dev);
> >
> > return 0;
> > +
> > +err:
> > + pm_runtime_put(dev);
> > +
> > + return ret;
> > }
> > EXPORT_SYMBOL_GPL(qcom_cc_really_probe);
> >
> > diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c
> > index 51ed640e527b..cfe875f87efe 100644
> > --- a/drivers/clk/qcom/gdsc.c
> > +++ b/drivers/clk/qcom/gdsc.c
> > @@ -11,6 +11,7 @@
> > #include <linux/kernel.h>
> > #include <linux/ktime.h>
> > #include <linux/pm_domain.h>
> > +#include <linux/pm_runtime.h>
> > #include <linux/regmap.h>
> > #include <linux/regulator/consumer.h>
> > #include <linux/reset-controller.h>
> > @@ -50,6 +51,30 @@ enum gdsc_status {
> > GDSC_ON
> > };
> >
> > +static int gdsc_pm_runtime_get(struct gdsc *sc)
> > +{
> > + int ret;
> > +
> > + if (!sc->dev)
> > + return 0;
> > +
> > + ret = pm_runtime_get_sync(sc->dev);
> > + if (ret < 0) {
> > + pm_runtime_put_noidle(sc->dev);
> > + return ret;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int gdsc_pm_runtime_put(struct gdsc *sc)
> > +{
> > + if (!sc->dev)
> > + return 0;
> > +
> > + return pm_runtime_put_sync(sc->dev);
> > +}
> > +
> > /* Returns 1 if GDSC status is status, 0 if not, and < 0 on error */
> > static int gdsc_check_status(struct gdsc *sc, enum gdsc_status status)
> > {
> > @@ -232,9 +257,8 @@ static void gdsc_retain_ff_on(struct gdsc *sc)
> > regmap_update_bits(sc->regmap, sc->gdscr, mask, mask);
> > }
> >
> > -static int gdsc_enable(struct generic_pm_domain *domain)
> > +static int _gdsc_enable(struct gdsc *sc)
> > {
> > - struct gdsc *sc = domain_to_gdsc(domain);
> > int ret;
> >
> > if (sc->pwrsts == PWRSTS_ON)
> > @@ -290,11 +314,26 @@ static int gdsc_enable(struct generic_pm_domain *domain)
> > return 0;
> > }
> >
> > -static int gdsc_disable(struct generic_pm_domain *domain)
> > +static int gdsc_enable(struct generic_pm_domain *domain)
> > {
> > struct gdsc *sc = domain_to_gdsc(domain);
> > int ret;
> >
> > + ret = gdsc_pm_runtime_get(sc);
> > + if (ret)
> > + return ret;
> > +
> > + ret = _gdsc_enable(sc);
> > +
> > + gdsc_pm_runtime_put(sc);
> > +
> > + return ret;
> > +}
> > +
> > +static int _gdsc_disable(struct gdsc *sc)
> > +{
> > + int ret;
> > +
> > if (sc->pwrsts == PWRSTS_ON)
> > return gdsc_assert_reset(sc);
> >
> > @@ -329,6 +368,22 @@ static int gdsc_disable(struct generic_pm_domain *domain)
> > return 0;
> > }
> >
> > +static int gdsc_disable(struct generic_pm_domain *domain)
> > +{
> > + struct gdsc *sc = domain_to_gdsc(domain);
> > + int ret;
> > +
> > + ret = gdsc_pm_runtime_get(sc);
> > + if (ret)
> > + return ret;
> > +
> > + ret = _gdsc_disable(sc);
> > +
> > + gdsc_pm_runtime_put(sc);
> > +
> > + return ret;
> > +}
> > +
> > static int gdsc_init(struct gdsc *sc)
> > {
> > u32 mask, val;
> > @@ -425,6 +480,8 @@ int gdsc_register(struct gdsc_desc *desc,
> > for (i = 0; i < num; i++) {
> > if (!scs[i])
> > continue;
> > + if (pm_runtime_enabled(dev))
> > + scs[i]->dev = dev;
> > scs[i]->regmap = regmap;
> > scs[i]->rcdev = rcdev;
> > ret = gdsc_init(scs[i]);
> > @@ -439,6 +496,8 @@ int gdsc_register(struct gdsc_desc *desc,
> > continue;
> > if (scs[i]->parent)
> > pm_genpd_add_subdomain(scs[i]->parent, &scs[i]->pd);
> > + else if (!IS_ERR_OR_NULL(dev->pm_domain))
> > + pm_genpd_add_subdomain(pd_to_genpd(dev->pm_domain), &scs[i]->pd);
> > }
> >
> > return of_genpd_add_provider_onecell(dev->of_node, data);
> > @@ -457,6 +516,8 @@ void gdsc_unregister(struct gdsc_desc *desc)
> > continue;
> > if (scs[i]->parent)
> > pm_genpd_remove_subdomain(scs[i]->parent, &scs[i]->pd);
> > + else if (!IS_ERR_OR_NULL(dev->pm_domain))
> > + pm_genpd_remove_subdomain(pd_to_genpd(dev->pm_domain), &scs[i]->pd);
> > }
> > of_genpd_del_provider(dev->of_node);
> > }
> > diff --git a/drivers/clk/qcom/gdsc.h b/drivers/clk/qcom/gdsc.h
> > index 5bb396b344d1..702d47a87af6 100644
> > --- a/drivers/clk/qcom/gdsc.h
> > +++ b/drivers/clk/qcom/gdsc.h
> > @@ -25,6 +25,7 @@ struct reset_controller_dev;
> > * @resets: ids of resets associated with this gdsc
> > * @reset_count: number of @resets
> > * @rcdev: reset controller
> > + * @dev: the device holding the GDSC, used for pm_runtime calls
> > */
> > struct gdsc {
> > struct generic_pm_domain pd;
> > @@ -58,6 +59,7 @@ struct gdsc {
> >
> > const char *supply;
> > struct regulator *rsupply;
> > + struct device *dev;
> > };
> >
> > struct gdsc_desc {
> > --
> > 2.30.2
> >
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v4 1/6] dt-bindings: clock: qcom,dispcc-sm8x50: add mmcx power domain
2021-07-10 1:32 ` [PATCH v4 1/6] dt-bindings: clock: qcom,dispcc-sm8x50: add mmcx power domain Dmitry Baryshkov
@ 2021-07-15 16:17 ` Rob Herring
0 siblings, 0 replies; 11+ messages in thread
From: Rob Herring @ 2021-07-15 16:17 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Michael Turquette, Bjorn Andersson, Rob Herring, Ulf Hansson,
Bryan O'Donoghue, linux-kernel, devicetree, Mark Brown,
Jonathan Marek, linux-clk, Stephen Boyd, Andy Gross, Taniya Das,
linux-arm-msm
On Sat, 10 Jul 2021 04:32:48 +0300, Dmitry Baryshkov wrote:
> On sm8250 dispcc requires MMCX power domain to be powered up before
> clock controller's registers become available. For now sm8250 was using
> external regulator driven by the power domain to describe this
> relationship. Switch into specifying power-domain and required opp-state
> directly.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
> .../devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml | 7 +++++++
> 1 file changed, 7 insertions(+)
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v4 2/6] dt-bindings: clock: qcom,videocc: add mmcx power domain
2021-07-10 1:32 ` [PATCH v4 2/6] dt-bindings: clock: qcom,videocc: " Dmitry Baryshkov
@ 2021-07-15 16:18 ` Rob Herring
0 siblings, 0 replies; 11+ messages in thread
From: Rob Herring @ 2021-07-15 16:18 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Bjorn Andersson, Ulf Hansson, Taniya Das, Michael Turquette,
Bryan O'Donoghue, linux-clk, Mark Brown, linux-kernel,
Andy Gross, Rob Herring, Jonathan Marek, devicetree,
linux-arm-msm, Stephen Boyd
On Sat, 10 Jul 2021 04:32:49 +0300, Dmitry Baryshkov wrote:
> On sm8250 videocc requires MMCX power domain to be powered up before
> clock controller's registers become available. For now sm8250 was using
> external regulator driven by the power domain to describe this
> relationship. Switch into specifying power-domain and required opp-state
> directly.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
> Documentation/devicetree/bindings/clock/qcom,videocc.yaml | 7 +++++++
> 1 file changed, 7 insertions(+)
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2021-07-15 16:18 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-10 1:32 [PATCH v4 0/6] clk: qcom: use power-domain for sm8250's clock controllers Dmitry Baryshkov
2021-07-10 1:32 ` [PATCH v4 1/6] dt-bindings: clock: qcom,dispcc-sm8x50: add mmcx power domain Dmitry Baryshkov
2021-07-15 16:17 ` Rob Herring
2021-07-10 1:32 ` [PATCH v4 2/6] dt-bindings: clock: qcom,videocc: " Dmitry Baryshkov
2021-07-15 16:18 ` Rob Herring
2021-07-10 1:32 ` [PATCH v4 3/6] clk: qcom: gdsc: enable optional power domain support Dmitry Baryshkov
2021-07-10 3:44 ` Bjorn Andersson
2021-07-10 11:50 ` Dmitry Baryshkov
2021-07-10 1:32 ` [PATCH v4 4/6] arm64: dts: qcom: sm8250: remove mmcx regulator Dmitry Baryshkov
2021-07-10 1:32 ` [PATCH v4 5/6] clk: qcom: dispcc-sm8250: stop using " Dmitry Baryshkov
2021-07-10 1:32 ` [PATCH v4 6/6] clk: qcom: videocc-sm8250: " Dmitry Baryshkov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).