* [RFC 1/4] dt-bindings: clock: qcom,dispcc: document power domain bindings
2020-09-11 13:09 [RFC 0/4] clk: qcom: handle power domains links for GDSC Dmitry Baryshkov
@ 2020-09-11 13:09 ` Dmitry Baryshkov
2020-09-22 22:56 ` Rob Herring
2020-09-11 13:09 ` [RFC 2/4] clk: qcom: gdsc: enable external switchable power domain Dmitry Baryshkov
` (3 subsequent siblings)
4 siblings, 1 reply; 8+ messages in thread
From: Dmitry Baryshkov @ 2020-09-11 13:09 UTC (permalink / raw)
To: Andy Gross, Bjorn Andersson, Rob Herring, Jonathan Marek,
Stephen Boyd, Michael Turquette
Cc: linux-arm-msm, Manivannan Sadhasivam, devicetree
SM8250 requires special power domain for accessing MMDS_GDSC registers.
Add bindings for the MMCX power domain.
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
.../bindings/clock/qcom,dispcc.yaml | 28 +++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/Documentation/devicetree/bindings/clock/qcom,dispcc.yaml b/Documentation/devicetree/bindings/clock/qcom,dispcc.yaml
index 0b905a4e9ada..6325d9969913 100644
--- a/Documentation/devicetree/bindings/clock/qcom,dispcc.yaml
+++ b/Documentation/devicetree/bindings/clock/qcom,dispcc.yaml
@@ -66,6 +66,16 @@ properties:
reg:
maxItems: 1
+ power-domains:
+ maxItems: 1
+
+ power-domain-names:
+ items:
+ - const: mmcx
+
+ required-opps:
+ maxItems: 1
+
required:
- compatible
- reg
@@ -81,6 +91,7 @@ examples:
- |
#include <dt-bindings/clock/qcom,gcc-sdm845.h>
#include <dt-bindings/clock/qcom,rpmh.h>
+ #include <dt-bindings/power/qcom-rpmpd.h>
clock-controller@af00000 {
compatible = "qcom,sdm845-dispcc";
reg = <0x0af00000 0x10000>;
@@ -105,5 +116,22 @@ examples:
#clock-cells = <1>;
#reset-cells = <1>;
#power-domain-cells = <1>;
+ /* this is a part of sm8250 setup the power domain example */
+ power-domains = <&rpmhpd SDM845_CX>;
+ power-domain-names = "mmcx";
+ required-opps = <&rpmhpd_opp_low_svs>;
+ };
+ rpmhpd: power-controller {
+ compatible = "qcom,sdm845-rpmhpd";
+ #power-domain-cells = <1>;
+ operating-points-v2 = <&rpmhpd_opp_table>;
+
+ rpmhpd_opp_table: opp-table {
+ compatible = "operating-points-v2";
+
+ rpmhpd_opp_low_svs: opp3 {
+ opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS>;
+ };
+ };
};
...
--
2.28.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [RFC 1/4] dt-bindings: clock: qcom,dispcc: document power domain bindings
2020-09-11 13:09 ` [RFC 1/4] dt-bindings: clock: qcom,dispcc: document power domain bindings Dmitry Baryshkov
@ 2020-09-22 22:56 ` Rob Herring
0 siblings, 0 replies; 8+ messages in thread
From: Rob Herring @ 2020-09-22 22:56 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Jonathan Marek, Manivannan Sadhasivam, Stephen Boyd, Andy Gross,
linux-arm-msm, devicetree, Bjorn Andersson, Michael Turquette,
Rob Herring
On Fri, 11 Sep 2020 16:09:47 +0300, Dmitry Baryshkov wrote:
> SM8250 requires special power domain for accessing MMDS_GDSC registers.
> Add bindings for the MMCX power domain.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
> .../bindings/clock/qcom,dispcc.yaml | 28 +++++++++++++++++++
> 1 file changed, 28 insertions(+)
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [RFC 2/4] clk: qcom: gdsc: enable external switchable power domain
2020-09-11 13:09 [RFC 0/4] clk: qcom: handle power domains links for GDSC Dmitry Baryshkov
2020-09-11 13:09 ` [RFC 1/4] dt-bindings: clock: qcom,dispcc: document power domain bindings Dmitry Baryshkov
@ 2020-09-11 13:09 ` Dmitry Baryshkov
2020-09-23 14:42 ` Bjorn Andersson
2020-09-11 13:09 ` [RFC 3/4] clk: qcom: dispcc-sm8250: handle MMCX " Dmitry Baryshkov
` (2 subsequent siblings)
4 siblings, 1 reply; 8+ messages in thread
From: Dmitry Baryshkov @ 2020-09-11 13:09 UTC (permalink / raw)
To: Andy Gross, Bjorn Andersson, Rob Herring, Jonathan Marek,
Stephen Boyd, Michael Turquette
Cc: linux-arm-msm, Manivannan Sadhasivam, devicetree
Some GDSCs (SM8250's MDSS_GDSC for example) need switchable power domain
to be on to be able to access hardware registers. Use dev_pm/opp to
enable corresponding power domain.
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
drivers/clk/qcom/gdsc.c | 56 ++++++++++++++++++++++++++++++++++++++---
drivers/clk/qcom/gdsc.h | 5 ++++
2 files changed, 57 insertions(+), 4 deletions(-)
diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c
index bfc4ac02f9ea..a522e062a79a 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_opp.h>
#include <linux/regmap.h>
#include <linux/regulator/consumer.h>
#include <linux/reset-controller.h>
@@ -110,13 +111,31 @@ static int gdsc_poll_status(struct gdsc *sc, enum gdsc_status status)
return -ETIMEDOUT;
}
+int gdsc_toggle_on(struct gdsc *sc)
+{
+ if (sc->rsupply)
+ return regulator_enable(sc->rsupply);
+ if (sc->pd_dev)
+ return dev_pm_genpd_set_performance_state(sc->pd_dev, sc->pd_opp);
+ return 0;
+}
+
+int gdsc_toggle_off(struct gdsc *sc)
+{
+ if (sc->pd_dev)
+ return dev_pm_genpd_set_performance_state(sc->pd_dev, 0);
+ if (sc->rsupply)
+ return regulator_disable(sc->rsupply);
+ return 0;
+}
+
static int gdsc_toggle_logic(struct gdsc *sc, enum gdsc_status status)
{
int ret;
u32 val = (status == GDSC_ON) ? 0 : SW_COLLAPSE_MASK;
- if (status == GDSC_ON && sc->rsupply) {
- ret = regulator_enable(sc->rsupply);
+ if (status == GDSC_ON) {
+ ret = gdsc_toggle_on(sc);
if (ret < 0)
return ret;
}
@@ -153,8 +172,8 @@ static int gdsc_toggle_logic(struct gdsc *sc, enum gdsc_status status)
ret = gdsc_poll_status(sc, status);
WARN(ret, "%s status stuck at 'o%s'", sc->pd.name, status ? "ff" : "n");
- if (!ret && status == GDSC_OFF && sc->rsupply) {
- ret = regulator_disable(sc->rsupply);
+ if (!ret && status == GDSC_OFF) {
+ ret = gdsc_toggle_off(sc);
if (ret < 0)
return ret;
}
@@ -407,6 +426,27 @@ int gdsc_register(struct gdsc_desc *desc,
return PTR_ERR(scs[i]->rsupply);
}
+ for (i = 0; i < num; i++) {
+ if (!scs[i] || !scs[i]->domain)
+ continue;
+
+ scs[i]->pd_opp = of_get_required_opp_performance_state(dev->of_node, scs[i]->perf_idx);
+ if (scs[i]->pd_opp < 0)
+ return scs[i]->pd_opp;
+
+ scs[i]->pd_dev = dev_pm_domain_attach_by_name(dev, scs[i]->domain);
+ if (IS_ERR(scs[i]->pd_dev)) {
+ ret = PTR_ERR(scs[i]->pd_dev);
+ /* Single domain has been already attached, so reuse dev */
+ if (ret == -EEXIST) {
+ scs[i]->pd_dev = dev;
+ } else {
+ scs[i]->pd_dev = NULL;
+ goto pm_detach;
+ }
+ }
+ }
+
data->num_domains = num;
for (i = 0; i < num; i++) {
if (!scs[i])
@@ -428,6 +468,12 @@ int gdsc_register(struct gdsc_desc *desc,
}
return of_genpd_add_provider_onecell(dev->of_node, data);
+
+pm_detach:
+ for (i = 0; i < num; i++)
+ if (scs[i]->pd_dev)
+ dev_pm_domain_detach(scs[i]->pd_dev, false);
+ return ret;
}
void gdsc_unregister(struct gdsc_desc *desc)
@@ -443,6 +489,8 @@ void gdsc_unregister(struct gdsc_desc *desc)
continue;
if (scs[i]->parent)
pm_genpd_remove_subdomain(scs[i]->parent, &scs[i]->pd);
+ if (scs[i]->pd_dev && scs[i]->pd_dev != dev)
+ dev_pm_domain_detach(scs[i]->pd_dev, true);
}
of_genpd_del_provider(dev->of_node);
}
diff --git a/drivers/clk/qcom/gdsc.h b/drivers/clk/qcom/gdsc.h
index bd537438c793..d58575f8f25f 100644
--- a/drivers/clk/qcom/gdsc.h
+++ b/drivers/clk/qcom/gdsc.h
@@ -57,6 +57,11 @@ struct gdsc {
const char *supply;
struct regulator *rsupply;
+
+ const char *domain;
+ unsigned int perf_idx;
+ struct device *pd_dev;
+ int pd_opp;
};
struct gdsc_desc {
--
2.28.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [RFC 2/4] clk: qcom: gdsc: enable external switchable power domain
2020-09-11 13:09 ` [RFC 2/4] clk: qcom: gdsc: enable external switchable power domain Dmitry Baryshkov
@ 2020-09-23 14:42 ` Bjorn Andersson
0 siblings, 0 replies; 8+ messages in thread
From: Bjorn Andersson @ 2020-09-23 14:42 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Stephen Boyd, Andy Gross, Rob Herring, Jonathan Marek,
Michael Turquette, linux-arm-msm, Manivannan Sadhasivam,
devicetree
On Fri 11 Sep 08:09 CDT 2020, Dmitry Baryshkov wrote:
> Some GDSCs (SM8250's MDSS_GDSC for example) need switchable power domain
> to be on to be able to access hardware registers. Use dev_pm/opp to
^
What you describe here ----+ sounds like the GDSC controller is part of
the power-domain specified and hence needs to be enabled in order to
control the GDSC.
But in contrast what the patch implements is a mechanism where the
GDSC power-domain is a child of some other power-domain. So the commit
message needs to better reflect what's implemented.
Then looking at the DT representation I think it says that the
controller sits in the specified power-domain, rather than the exposed
power-domain...
> enable corresponding power domain.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
> drivers/clk/qcom/gdsc.c | 56 ++++++++++++++++++++++++++++++++++++++---
> drivers/clk/qcom/gdsc.h | 5 ++++
> 2 files changed, 57 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c
> index bfc4ac02f9ea..a522e062a79a 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_opp.h>
> #include <linux/regmap.h>
> #include <linux/regulator/consumer.h>
> #include <linux/reset-controller.h>
> @@ -110,13 +111,31 @@ static int gdsc_poll_status(struct gdsc *sc, enum gdsc_status status)
> return -ETIMEDOUT;
> }
>
> +int gdsc_toggle_on(struct gdsc *sc)
This should be "static" and I think you should include "supply" in the
name to denote that it doesn't turn on the gdsc, but rather its supply.
> +{
> + if (sc->rsupply)
> + return regulator_enable(sc->rsupply);
> + if (sc->pd_dev)
> + return dev_pm_genpd_set_performance_state(sc->pd_dev, sc->pd_opp);
> + return 0;
> +}
> +
> +int gdsc_toggle_off(struct gdsc *sc)
> +{
> + if (sc->pd_dev)
> + return dev_pm_genpd_set_performance_state(sc->pd_dev, 0);
> + if (sc->rsupply)
> + return regulator_disable(sc->rsupply);
> + return 0;
> +}
> +
> static int gdsc_toggle_logic(struct gdsc *sc, enum gdsc_status status)
> {
> int ret;
> u32 val = (status == GDSC_ON) ? 0 : SW_COLLAPSE_MASK;
>
> - if (status == GDSC_ON && sc->rsupply) {
> - ret = regulator_enable(sc->rsupply);
> + if (status == GDSC_ON) {
> + ret = gdsc_toggle_on(sc);
> if (ret < 0)
> return ret;
> }
> @@ -153,8 +172,8 @@ static int gdsc_toggle_logic(struct gdsc *sc, enum gdsc_status status)
> ret = gdsc_poll_status(sc, status);
> WARN(ret, "%s status stuck at 'o%s'", sc->pd.name, status ? "ff" : "n");
>
> - if (!ret && status == GDSC_OFF && sc->rsupply) {
> - ret = regulator_disable(sc->rsupply);
> + if (!ret && status == GDSC_OFF) {
> + ret = gdsc_toggle_off(sc);
> if (ret < 0)
> return ret;
> }
> @@ -407,6 +426,27 @@ int gdsc_register(struct gdsc_desc *desc,
> return PTR_ERR(scs[i]->rsupply);
> }
>
> + for (i = 0; i < num; i++) {
> + if (!scs[i] || !scs[i]->domain)
> + continue;
> +
> + scs[i]->pd_opp = of_get_required_opp_performance_state(dev->of_node, scs[i]->perf_idx);
> + if (scs[i]->pd_opp < 0)
> + return scs[i]->pd_opp;
> +
> + scs[i]->pd_dev = dev_pm_domain_attach_by_name(dev, scs[i]->domain);
> + if (IS_ERR(scs[i]->pd_dev)) {
> + ret = PTR_ERR(scs[i]->pd_dev);
> + /* Single domain has been already attached, so reuse dev */
> + if (ret == -EEXIST) {
> + scs[i]->pd_dev = dev;
> + } else {
> + scs[i]->pd_dev = NULL;
> + goto pm_detach;
> + }
> + }
> + }
> +
> data->num_domains = num;
> for (i = 0; i < num; i++) {
> if (!scs[i])
> @@ -428,6 +468,12 @@ int gdsc_register(struct gdsc_desc *desc,
> }
>
> return of_genpd_add_provider_onecell(dev->of_node, data);
> +
> +pm_detach:
> + for (i = 0; i < num; i++)
> + if (scs[i]->pd_dev)
> + dev_pm_domain_detach(scs[i]->pd_dev, false);
I think that if dev_pm_domain_attach_by_name() returned -EEXIST you
will attempt to detach the main device's domain here.
> + return ret;
> }
>
> void gdsc_unregister(struct gdsc_desc *desc)
> @@ -443,6 +489,8 @@ void gdsc_unregister(struct gdsc_desc *desc)
> continue;
> if (scs[i]->parent)
> pm_genpd_remove_subdomain(scs[i]->parent, &scs[i]->pd);
> + if (scs[i]->pd_dev && scs[i]->pd_dev != dev)
> + dev_pm_domain_detach(scs[i]->pd_dev, true);
Ditto
Regards,
Bjorn
> }
> of_genpd_del_provider(dev->of_node);
> }
> diff --git a/drivers/clk/qcom/gdsc.h b/drivers/clk/qcom/gdsc.h
> index bd537438c793..d58575f8f25f 100644
> --- a/drivers/clk/qcom/gdsc.h
> +++ b/drivers/clk/qcom/gdsc.h
> @@ -57,6 +57,11 @@ struct gdsc {
>
> const char *supply;
> struct regulator *rsupply;
> +
> + const char *domain;
> + unsigned int perf_idx;
> + struct device *pd_dev;
> + int pd_opp;
> };
>
> struct gdsc_desc {
> --
> 2.28.0
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [RFC 3/4] clk: qcom: dispcc-sm8250: handle MMCX power domain
2020-09-11 13:09 [RFC 0/4] clk: qcom: handle power domains links for GDSC Dmitry Baryshkov
2020-09-11 13:09 ` [RFC 1/4] dt-bindings: clock: qcom,dispcc: document power domain bindings Dmitry Baryshkov
2020-09-11 13:09 ` [RFC 2/4] clk: qcom: gdsc: enable external switchable power domain Dmitry Baryshkov
@ 2020-09-11 13:09 ` Dmitry Baryshkov
2020-09-11 13:09 ` [RFC 4/4] arm64: dts: qcom: sm8250: pin MMCX to stop the board from crashing Dmitry Baryshkov
2020-10-01 0:23 ` [RFC 0/4] clk: qcom: handle power domains links for GDSC Stephen Boyd
4 siblings, 0 replies; 8+ messages in thread
From: Dmitry Baryshkov @ 2020-09-11 13:09 UTC (permalink / raw)
To: Andy Gross, Bjorn Andersson, Rob Herring, Jonathan Marek,
Stephen Boyd, Michael Turquette
Cc: linux-arm-msm, Manivannan Sadhasivam, devicetree
On SM8250 MMCX power domain is required to access MMDS_GDSC registers.
Enable using this power domain for the gdsc.
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
drivers/clk/qcom/dispcc-sm8250.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/clk/qcom/dispcc-sm8250.c b/drivers/clk/qcom/dispcc-sm8250.c
index 7c0f384a3a42..fc5fb2b2fe5e 100644
--- a/drivers/clk/qcom/dispcc-sm8250.c
+++ b/drivers/clk/qcom/dispcc-sm8250.c
@@ -970,6 +970,8 @@ static struct gdsc mdss_gdsc = {
},
.pwrsts = PWRSTS_OFF_ON,
.flags = HW_CTRL,
+ .domain = "mmcx",
+ .perf_idx = 0,
};
static struct clk_regmap *disp_cc_sm8250_clocks[] = {
--
2.28.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [RFC 4/4] arm64: dts: qcom: sm8250: pin MMCX to stop the board from crashing
2020-09-11 13:09 [RFC 0/4] clk: qcom: handle power domains links for GDSC Dmitry Baryshkov
` (2 preceding siblings ...)
2020-09-11 13:09 ` [RFC 3/4] clk: qcom: dispcc-sm8250: handle MMCX " Dmitry Baryshkov
@ 2020-09-11 13:09 ` Dmitry Baryshkov
2020-10-01 0:23 ` [RFC 0/4] clk: qcom: handle power domains links for GDSC Stephen Boyd
4 siblings, 0 replies; 8+ messages in thread
From: Dmitry Baryshkov @ 2020-09-11 13:09 UTC (permalink / raw)
To: Andy Gross, Bjorn Andersson, Rob Herring, Jonathan Marek,
Stephen Boyd, Michael Turquette
Cc: linux-arm-msm, Manivannan Sadhasivam, devicetree
Use MMCX domain to power up MDSS_GDSC.
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
arch/arm64/boot/dts/qcom/sm8250.dtsi | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
index 5045fe89a494..c42640228ceb 100644
--- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
@@ -1732,6 +1732,9 @@ dp_out: endpoint {
dispcc: clock-controller@af00000 {
compatible = "qcom,sm8250-dispcc";
reg = <0 0x0af00000 0 0x10000>;
+ power-domains = <&rpmhpd SM8250_MMCX>;
+ power-domain-names = "mmcx";
+ required-opps = <&rpmhpd_opp_low_svs>;
clocks = <&rpmhcc RPMH_CXO_CLK>,
<&dsi0_phy 0>,
<&dsi0_phy 1>,
--
2.28.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [RFC 0/4] clk: qcom: handle power domains links for GDSC
2020-09-11 13:09 [RFC 0/4] clk: qcom: handle power domains links for GDSC Dmitry Baryshkov
` (3 preceding siblings ...)
2020-09-11 13:09 ` [RFC 4/4] arm64: dts: qcom: sm8250: pin MMCX to stop the board from crashing Dmitry Baryshkov
@ 2020-10-01 0:23 ` Stephen Boyd
4 siblings, 0 replies; 8+ messages in thread
From: Stephen Boyd @ 2020-10-01 0:23 UTC (permalink / raw)
To: Andy Gross, Bjorn Andersson, Dmitry Baryshkov, Jonathan Marek,
Michael Turquette, Rob Herring
Cc: linux-arm-msm, Manivannan Sadhasivam, devicetree
Quoting Dmitry Baryshkov (2020-09-11 06:09:46)
> On SM8250 MDSS_GDSC requires MMCX domain to be powered to access GDSC
> registers. Handle this requirement in the gdsc code by binding the power
> domain via dts file. The example in the schema file demonstrates this
> binding (which is not required for SDM845).
>
Please Cc the linux-clk list on clk patches. It might be obvious now
that I don't always look at this email pile to fish out clk things.
^ permalink raw reply [flat|nested] 8+ messages in thread