[PATCH RFT 00/20] SMD RPMCC sleep preparations

[PATCH RFT 00/20] SMD RPMCC sleep preparations

[Konrad Dybcio]

This series brings support for a couple of things necessary for the full
system idle on SMD RPM SoCs, namely unused clk shutdown and keepalive
votes (permanent active votes that are required on certain clocks for the
platform to function).

Tested on MSM8996 and SM6375, does not seem to introduce any additional
regressions.

Keepalive clocks for other platforms were gathered by digging in old
downstream kernels, please give them a test.

Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
Konrad Dybcio (18):
      clk: qcom: smd-rpm: Add support for keepalive votes
      clk: qcom: smd-rpm: Add keepalive_clks for SM6375
      clk: qcom: smd-rpm: Add keepalive_clks for MSM8996
      clk: qcom: smd-rpm: Add keepalive_clks for MSM8909
      clk: qcom: smd-rpm: Add keepalive_clks for MSM8916
      clk: qcom: smd-rpm: Add keepalive_clks for MSM8936
      clk: qcom: smd-rpm: Add keepalive_clks for MSM8974
      clk: qcom: smd-rpm: Add keepalive_clks for MSM8976
      clk: qcom: smd-rpm: Add keepalive_clks for MSM8992
      clk: qcom: smd-rpm: Add keepalive_clks for MSM8994
      clk: qcom: smd-rpm: Add keepalive_clks for MSM8998
      clk: qcom: smd-rpm: Add keepalive_clks for SDM660
      clk: qcom: smd-rpm: Add keepalive_clks for MDM9607
      clk: qcom: smd-rpm: Add keepalive_clks for MSM8953
      clk: qcom: smd-rpm: Add keepalive_clks for SM6125
      clk: qcom: smd-rpm: Add keepalive_clks for SM6115
      clk: qcom: smd-rpm: Add keepalive_clks for QCM2290
      clk: qcom: smd-rpm: Add keepalive_clks for QCS404

Shawn Guo (2):
      clk: qcom: smd-rpm: Add .is_enabled hook
      clk: qcom: smd-rpm: Add .is_prepared hook

 drivers/clk/qcom/clk-smd-rpm.c | 85 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 83 insertions(+), 2 deletions(-)
---
base-commit: 1acf39ef8f1425cd105f630dc2c7c1d8fff27ed1
change-id: 20230303-topic-rpmcc_sleep-d67aad9f3012

Best regards,
-- 
Konrad Dybcio <konrad.dybcio@linaro.org>

[PATCH RFT 01/20] clk: qcom: smd-rpm: Add .is_enabled hook

[Konrad Dybcio]

From: Shawn Guo <shawn.guo@linaro.org>

The RPM clock enabling state can be found with 'enabled' in struct
clk_smd_rpm.  Add .is_enabled hook so that clk_summary in debugfs can
show a correct enabling state for RPM clocks.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
[Konrad: rebase]
Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
 drivers/clk/qcom/clk-smd-rpm.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/clk/qcom/clk-smd-rpm.c b/drivers/clk/qcom/clk-smd-rpm.c
index 198886c1b6c8..ecacfbc4a16c 100644
--- a/drivers/clk/qcom/clk-smd-rpm.c
+++ b/drivers/clk/qcom/clk-smd-rpm.c
@@ -424,18 +424,27 @@ static int clk_smd_rpm_enable_scaling(struct qcom_smd_rpm *rpm)
 	return 0;
 }
 
+static int clk_smd_rpm_is_enabled(struct clk_hw *hw)
+{
+	struct clk_smd_rpm *r = to_clk_smd_rpm(hw);
+
+	return r->enabled;
+}
+
 static const struct clk_ops clk_smd_rpm_ops = {
 	.prepare	= clk_smd_rpm_prepare,
 	.unprepare	= clk_smd_rpm_unprepare,
 	.set_rate	= clk_smd_rpm_set_rate,
 	.round_rate	= clk_smd_rpm_round_rate,
 	.recalc_rate	= clk_smd_rpm_recalc_rate,
+	.is_enabled	= clk_smd_rpm_is_enabled,
 };
 
 static const struct clk_ops clk_smd_rpm_branch_ops = {
 	.prepare	= clk_smd_rpm_prepare,
 	.unprepare	= clk_smd_rpm_unprepare,
 	.recalc_rate	= clk_smd_rpm_recalc_rate,
+	.is_enabled	= clk_smd_rpm_is_enabled,
 };
 
 DEFINE_CLK_SMD_RPM_BRANCH_A(bi_tcxo, QCOM_SMD_RPM_MISC_CLK, 0, 19200000);

-- 
2.39.2

[PATCH RFT 02/20] clk: qcom: smd-rpm: Add .is_prepared hook

[Konrad Dybcio]

From: Shawn Guo <shawn.guo@linaro.org>

The RPM clocks are enabled/disabled through clk framework prepare/unprepare
hooks.  Without .is_prepared hook, those unused RPM clocks will not be
disabled by core function clk_unprepare_unused_subtree(), because
clk_core_is_prepared() always returns 0.

Add .is_prepared hook to clk_ops and return the clock prepare (enable)
state, so that those unused RPM clocks can be disabled by clk framework.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
[Konrad: rebase, don't duplicate the enable func]
Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
 drivers/clk/qcom/clk-smd-rpm.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/clk/qcom/clk-smd-rpm.c b/drivers/clk/qcom/clk-smd-rpm.c
index ecacfbc4a16c..cce7daa97c1e 100644
--- a/drivers/clk/qcom/clk-smd-rpm.c
+++ b/drivers/clk/qcom/clk-smd-rpm.c
@@ -438,6 +438,7 @@ static const struct clk_ops clk_smd_rpm_ops = {
 	.round_rate	= clk_smd_rpm_round_rate,
 	.recalc_rate	= clk_smd_rpm_recalc_rate,
 	.is_enabled	= clk_smd_rpm_is_enabled,
+	.is_prepared	= clk_smd_rpm_is_enabled,
 };
 
 static const struct clk_ops clk_smd_rpm_branch_ops = {
@@ -445,6 +446,7 @@ static const struct clk_ops clk_smd_rpm_branch_ops = {
 	.unprepare	= clk_smd_rpm_unprepare,
 	.recalc_rate	= clk_smd_rpm_recalc_rate,
 	.is_enabled	= clk_smd_rpm_is_enabled,
+	.is_prepared	= clk_smd_rpm_is_enabled,
 };
 
 DEFINE_CLK_SMD_RPM_BRANCH_A(bi_tcxo, QCOM_SMD_RPM_MISC_CLK, 0, 19200000);

-- 
2.39.2

[PATCH RFT 03/20] clk: qcom: smd-rpm: Add support for keepalive votes

[Konrad Dybcio]

Some bus clock should always have a minimum (19.2 MHz) vote cast on
them, otherwise the platform will fall apart, hang and reboot.

Add support for specifying which clocks should be kept alive and
always keep a vote on XO_A to make sure the clock tree doesn't
collapse. This removes the need to keep a maximum vote that was
previously guaranteed by clk_smd_rpm_handoff.

This commit is a combination of existing (not-exactly-upstream) work
by Taniya Das, Shawn Guo and myself.

Co-developed-by: Shawn Guo <shawn.guo@linaro.org>
Co-developed-by: Taniya Das <quic_tdas@quicinc.com>
Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
 drivers/clk/qcom/clk-smd-rpm.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/drivers/clk/qcom/clk-smd-rpm.c b/drivers/clk/qcom/clk-smd-rpm.c
index cce7daa97c1e..8e017c575361 100644
--- a/drivers/clk/qcom/clk-smd-rpm.c
+++ b/drivers/clk/qcom/clk-smd-rpm.c
@@ -4,6 +4,7 @@
  * Copyright (c) 2014, The Linux Foundation. All rights reserved.
  */
 
+#include <linux/clk.h>
 #include <linux/clk-provider.h>
 #include <linux/err.h>
 #include <linux/export.h>
@@ -178,6 +179,8 @@ struct clk_smd_rpm_req {
 struct rpm_smd_clk_desc {
 	struct clk_smd_rpm **clks;
 	size_t num_clks;
+	struct clk_hw **keepalive_clks;
+	size_t num_keepalive_clks;
 };
 
 static DEFINE_MUTEX(rpm_smd_clk_lock);
@@ -1278,6 +1281,7 @@ static int rpm_smd_clk_probe(struct platform_device *pdev)
 	struct qcom_smd_rpm *rpm;
 	struct clk_smd_rpm **rpm_smd_clks;
 	const struct rpm_smd_clk_desc *desc;
+	struct clk_hw **keepalive_clks;
 
 	rpm = dev_get_drvdata(pdev->dev.parent);
 	if (!rpm) {
@@ -1291,6 +1295,7 @@ static int rpm_smd_clk_probe(struct platform_device *pdev)
 
 	rpm_smd_clks = desc->clks;
 	num_clks = desc->num_clks;
+	keepalive_clks = desc->keepalive_clks;
 
 	for (i = 0; i < num_clks; i++) {
 		if (!rpm_smd_clks[i])
@@ -1321,6 +1326,24 @@ static int rpm_smd_clk_probe(struct platform_device *pdev)
 	if (ret)
 		goto err;
 
+	/* Leave a permanent active vote on clocks that require it. */
+	for (i = 0; i < desc->num_keepalive_clks; i++) {
+		if (WARN_ON(!keepalive_clks[i]))
+			continue;
+
+		ret = clk_prepare_enable(keepalive_clks[i]->clk);
+		if (ret)
+			return ret;
+
+		ret = clk_set_rate(keepalive_clks[i]->clk, 19200000);
+		if (ret)
+			return ret;
+	}
+
+	/* Keep an active vote on CXO in case no other driver votes for it. */
+	if (rpm_smd_clks[RPM_SMD_XO_A_CLK_SRC])
+		return clk_prepare_enable(rpm_smd_clks[RPM_SMD_XO_A_CLK_SRC]->hw.clk);
+
 	return 0;
 err:
 	dev_err(&pdev->dev, "Error registering SMD clock driver (%d)\n", ret);

-- 
2.39.2

[PATCH RFT 04/20] clk: qcom: smd-rpm: Add keepalive_clks for SM6375

[Konrad Dybcio]

SM6375 requires that CNoC and SNoC clocks always get an active vote.
Guarantee that.

Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
 drivers/clk/qcom/clk-smd-rpm.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/clk/qcom/clk-smd-rpm.c b/drivers/clk/qcom/clk-smd-rpm.c
index 8e017c575361..8ca301c58909 100644
--- a/drivers/clk/qcom/clk-smd-rpm.c
+++ b/drivers/clk/qcom/clk-smd-rpm.c
@@ -521,6 +521,11 @@ DEFINE_CLK_SMD_RPM_XO_BUFFER(div_clk1, 11, 19200000);
 DEFINE_CLK_SMD_RPM_XO_BUFFER(div_clk2, 12, 19200000);
 DEFINE_CLK_SMD_RPM_XO_BUFFER(div_clk3, 13, 19200000);
 
+static struct clk_hw *cnoc_snoc_keepalive_clks[] = {
+	&clk_smd_rpm_bus_1_cnoc_a_clk.hw,
+	&clk_smd_rpm_bus_2_snoc_a_clk.hw,
+};
+
 static struct clk_smd_rpm *msm8909_clks[] = {
 	[RPM_SMD_PCNOC_CLK]		= &clk_smd_rpm_bus_0_pcnoc_clk,
 	[RPM_SMD_PCNOC_A_CLK]		= &clk_smd_rpm_bus_0_pcnoc_a_clk,
@@ -1189,6 +1194,8 @@ static struct clk_smd_rpm *sm6375_clks[] = {
 static const struct rpm_smd_clk_desc rpm_clk_sm6375 = {
 	.clks = sm6375_clks,
 	.num_clks = ARRAY_SIZE(sm6375_clks),
+	.keepalive_clks = cnoc_snoc_keepalive_clks,
+	.num_keepalive_clks = ARRAY_SIZE(cnoc_snoc_keepalive_clks),
 };
 
 static struct clk_smd_rpm *qcm2290_clks[] = {

-- 
2.39.2

[PATCH RFT 05/20] clk: qcom: smd-rpm: Add keepalive_clks for MSM8996

[Konrad Dybcio]

MSM8996 requires that the PCNoC clock always gets an active vote.
Guarantee that.

Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
 drivers/clk/qcom/clk-smd-rpm.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/clk/qcom/clk-smd-rpm.c b/drivers/clk/qcom/clk-smd-rpm.c
index 8ca301c58909..72d06d9d2619 100644
--- a/drivers/clk/qcom/clk-smd-rpm.c
+++ b/drivers/clk/qcom/clk-smd-rpm.c
@@ -526,6 +526,10 @@ static struct clk_hw *cnoc_snoc_keepalive_clks[] = {
 	&clk_smd_rpm_bus_2_snoc_a_clk.hw,
 };
 
+static struct clk_hw *pnoc_keepalive_clks[] = {
+	&clk_smd_rpm_bus_0_pcnoc_a_clk.hw,
+};
+
 static struct clk_smd_rpm *msm8909_clks[] = {
 	[RPM_SMD_PCNOC_CLK]		= &clk_smd_rpm_bus_0_pcnoc_clk,
 	[RPM_SMD_PCNOC_A_CLK]		= &clk_smd_rpm_bus_0_pcnoc_a_clk,
@@ -882,6 +886,8 @@ static struct clk_smd_rpm *msm8996_clks[] = {
 static const struct rpm_smd_clk_desc rpm_clk_msm8996 = {
 	.clks = msm8996_clks,
 	.num_clks = ARRAY_SIZE(msm8996_clks),
+	.keepalive_clks = pnoc_keepalive_clks,
+	.num_keepalive_clks = ARRAY_SIZE(pnoc_keepalive_clks),
 };
 
 static struct clk_smd_rpm *qcs404_clks[] = {

-- 
2.39.2

[PATCH RFT 06/20] clk: qcom: smd-rpm: Add keepalive_clks for MSM8909

[Konrad Dybcio]

MSM8909 requires that the PCNoC clock always gets an active vote.
Guarantee that.

Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
 drivers/clk/qcom/clk-smd-rpm.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/clk/qcom/clk-smd-rpm.c b/drivers/clk/qcom/clk-smd-rpm.c
index 72d06d9d2619..1a678a962770 100644
--- a/drivers/clk/qcom/clk-smd-rpm.c
+++ b/drivers/clk/qcom/clk-smd-rpm.c
@@ -562,6 +562,8 @@ static struct clk_smd_rpm *msm8909_clks[] = {
 static const struct rpm_smd_clk_desc rpm_clk_msm8909 = {
 	.clks = msm8909_clks,
 	.num_clks = ARRAY_SIZE(msm8909_clks),
+	.keepalive_clks = pnoc_keepalive_clks,
+	.num_keepalive_clks = ARRAY_SIZE(pnoc_keepalive_clks),
 };
 
 static struct clk_smd_rpm *msm8916_clks[] = {

-- 
2.39.2

[PATCH RFT 07/20] clk: qcom: smd-rpm: Add keepalive_clks for MSM8916

[Konrad Dybcio]

MSM8916 requires that the PCNoC clock always gets an active vote.
Guarantee that.

Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
 drivers/clk/qcom/clk-smd-rpm.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/clk/qcom/clk-smd-rpm.c b/drivers/clk/qcom/clk-smd-rpm.c
index 1a678a962770..a93b4575f44f 100644
--- a/drivers/clk/qcom/clk-smd-rpm.c
+++ b/drivers/clk/qcom/clk-smd-rpm.c
@@ -596,6 +596,8 @@ static struct clk_smd_rpm *msm8916_clks[] = {
 static const struct rpm_smd_clk_desc rpm_clk_msm8916 = {
 	.clks = msm8916_clks,
 	.num_clks = ARRAY_SIZE(msm8916_clks),
+	.keepalive_clks = pnoc_keepalive_clks,
+	.num_keepalive_clks = ARRAY_SIZE(pnoc_keepalive_clks),
 };
 
 static struct clk_smd_rpm *msm8936_clks[] = {

-- 
2.39.2

[PATCH RFT 08/20] clk: qcom: smd-rpm: Add keepalive_clks for MSM8936

[Konrad Dybcio]

MSM8936 requires that the PCNoC clock always gets an active vote.
Guarantee that.

Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
 drivers/clk/qcom/clk-smd-rpm.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/qcom/clk-smd-rpm.c b/drivers/clk/qcom/clk-smd-rpm.c
index a93b4575f44f..6db78aeae9dd 100644
--- a/drivers/clk/qcom/clk-smd-rpm.c
+++ b/drivers/clk/qcom/clk-smd-rpm.c
@@ -632,8 +632,10 @@ static struct clk_smd_rpm *msm8936_clks[] = {
 };
 
 static const struct rpm_smd_clk_desc rpm_clk_msm8936 = {
-		.clks = msm8936_clks,
-		.num_clks = ARRAY_SIZE(msm8936_clks),
+	.clks = msm8936_clks,
+	.num_clks = ARRAY_SIZE(msm8936_clks),
+	.keepalive_clks = pnoc_keepalive_clks,
+	.num_keepalive_clks = ARRAY_SIZE(pnoc_keepalive_clks),
 };
 
 static struct clk_smd_rpm *msm8974_clks[] = {

-- 
2.39.2

[PATCH RFT 09/20] clk: qcom: smd-rpm: Add keepalive_clks for MSM8974

[Konrad Dybcio]

MSM8974 requires that the PCNoC clock always gets an active vote.
Guarantee that.

Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
 drivers/clk/qcom/clk-smd-rpm.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/clk/qcom/clk-smd-rpm.c b/drivers/clk/qcom/clk-smd-rpm.c
index 6db78aeae9dd..a390ec3d6c01 100644
--- a/drivers/clk/qcom/clk-smd-rpm.c
+++ b/drivers/clk/qcom/clk-smd-rpm.c
@@ -686,6 +686,8 @@ static struct clk_smd_rpm *msm8974_clks[] = {
 static const struct rpm_smd_clk_desc rpm_clk_msm8974 = {
 	.clks = msm8974_clks,
 	.num_clks = ARRAY_SIZE(msm8974_clks),
+	.keepalive_clks = pnoc_keepalive_clks,
+	.num_keepalive_clks = ARRAY_SIZE(pnoc_keepalive_clks),
 };
 
 static struct clk_smd_rpm *msm8976_clks[] = {

-- 
2.39.2

[PATCH RFT 10/20] clk: qcom: smd-rpm: Add keepalive_clks for MSM8976

[Konrad Dybcio]

MSM8976 requires that the PCNoC clock always gets an active vote.
Guarantee that.

Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
 drivers/clk/qcom/clk-smd-rpm.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/clk/qcom/clk-smd-rpm.c b/drivers/clk/qcom/clk-smd-rpm.c
index a390ec3d6c01..f82576180fd4 100644
--- a/drivers/clk/qcom/clk-smd-rpm.c
+++ b/drivers/clk/qcom/clk-smd-rpm.c
@@ -722,6 +722,8 @@ static struct clk_smd_rpm *msm8976_clks[] = {
 static const struct rpm_smd_clk_desc rpm_clk_msm8976 = {
 	.clks = msm8976_clks,
 	.num_clks = ARRAY_SIZE(msm8976_clks),
+	.keepalive_clks = pnoc_keepalive_clks,
+	.num_keepalive_clks = ARRAY_SIZE(pnoc_keepalive_clks),
 };
 
 static struct clk_smd_rpm *msm8992_clks[] = {

-- 
2.39.2

[PATCH RFT 11/20] clk: qcom: smd-rpm: Add keepalive_clks for MSM8992

[Konrad Dybcio]

MSM8992 requires that the PCNoC clock always gets an active vote.
Guarantee that.

Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
 drivers/clk/qcom/clk-smd-rpm.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/clk/qcom/clk-smd-rpm.c b/drivers/clk/qcom/clk-smd-rpm.c
index f82576180fd4..c7c623b258ce 100644
--- a/drivers/clk/qcom/clk-smd-rpm.c
+++ b/drivers/clk/qcom/clk-smd-rpm.c
@@ -782,6 +782,8 @@ static struct clk_smd_rpm *msm8992_clks[] = {
 static const struct rpm_smd_clk_desc rpm_clk_msm8992 = {
 	.clks = msm8992_clks,
 	.num_clks = ARRAY_SIZE(msm8992_clks),
+	.keepalive_clks = pnoc_keepalive_clks,
+	.num_keepalive_clks = ARRAY_SIZE(pnoc_keepalive_clks),
 };
 
 static struct clk_smd_rpm *msm8994_clks[] = {

-- 
2.39.2

[PATCH RFT 12/20] clk: qcom: smd-rpm: Add keepalive_clks for MSM8994

[Konrad Dybcio]

MSM8994 requires that the PCNoC clock always gets an active vote.
Guarantee that.

Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
 drivers/clk/qcom/clk-smd-rpm.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/clk/qcom/clk-smd-rpm.c b/drivers/clk/qcom/clk-smd-rpm.c
index c7c623b258ce..4b6037e804ae 100644
--- a/drivers/clk/qcom/clk-smd-rpm.c
+++ b/drivers/clk/qcom/clk-smd-rpm.c
@@ -844,6 +844,8 @@ static struct clk_smd_rpm *msm8994_clks[] = {
 static const struct rpm_smd_clk_desc rpm_clk_msm8994 = {
 	.clks = msm8994_clks,
 	.num_clks = ARRAY_SIZE(msm8994_clks),
+	.keepalive_clks = pnoc_keepalive_clks,
+	.num_keepalive_clks = ARRAY_SIZE(pnoc_keepalive_clks),
 };
 
 static struct clk_smd_rpm *msm8996_clks[] = {

-- 
2.39.2

[PATCH RFT 13/20] clk: qcom: smd-rpm: Add keepalive_clks for MSM8998

[Konrad Dybcio]

MSM8998 requires that the PCNoC clock always gets an active vote.
Guarantee that.

Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
 drivers/clk/qcom/clk-smd-rpm.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/clk/qcom/clk-smd-rpm.c b/drivers/clk/qcom/clk-smd-rpm.c
index 4b6037e804ae..6649f13c741f 100644
--- a/drivers/clk/qcom/clk-smd-rpm.c
+++ b/drivers/clk/qcom/clk-smd-rpm.c
@@ -521,6 +521,10 @@ DEFINE_CLK_SMD_RPM_XO_BUFFER(div_clk1, 11, 19200000);
 DEFINE_CLK_SMD_RPM_XO_BUFFER(div_clk2, 12, 19200000);
 DEFINE_CLK_SMD_RPM_XO_BUFFER(div_clk3, 13, 19200000);
 
+static struct clk_hw *cnoc_keepalive_clks[] = {
+	&clk_smd_rpm_bus_2_cnoc_a_clk.hw,
+};
+
 static struct clk_hw *cnoc_snoc_keepalive_clks[] = {
 	&clk_smd_rpm_bus_1_cnoc_a_clk.hw,
 	&clk_smd_rpm_bus_2_snoc_a_clk.hw,
@@ -990,6 +994,8 @@ static struct clk_smd_rpm *msm8998_clks[] = {
 static const struct rpm_smd_clk_desc rpm_clk_msm8998 = {
 	.clks = msm8998_clks,
 	.num_clks = ARRAY_SIZE(msm8998_clks),
+	.keepalive_clks = cnoc_keepalive_clks,
+	.num_keepalive_clks = ARRAY_SIZE(cnoc_keepalive_clks),
 };
 
 static struct clk_smd_rpm *sdm660_clks[] = {

-- 
2.39.2

[PATCH RFT 14/20] clk: qcom: smd-rpm: Add keepalive_clks for SDM660

[Konrad Dybcio]

SDM660 requires that the PCNoC clock always gets an active vote.
Guarantee that.

Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
 drivers/clk/qcom/clk-smd-rpm.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/clk/qcom/clk-smd-rpm.c b/drivers/clk/qcom/clk-smd-rpm.c
index 6649f13c741f..7e1814f606e9 100644
--- a/drivers/clk/qcom/clk-smd-rpm.c
+++ b/drivers/clk/qcom/clk-smd-rpm.c
@@ -1042,6 +1042,8 @@ static struct clk_smd_rpm *sdm660_clks[] = {
 static const struct rpm_smd_clk_desc rpm_clk_sdm660 = {
 	.clks = sdm660_clks,
 	.num_clks = ARRAY_SIZE(sdm660_clks),
+	.keepalive_clks = cnoc_keepalive_clks,
+	.num_keepalive_clks = ARRAY_SIZE(cnoc_keepalive_clks),
 };
 
 static struct clk_smd_rpm *mdm9607_clks[] = {

-- 
2.39.2

[PATCH RFT 15/20] clk: qcom: smd-rpm: Add keepalive_clks for MDM9607

[Konrad Dybcio]

MDM9607 requires that the PCNoC clock always gets an active vote.
Guarantee that.

Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
 drivers/clk/qcom/clk-smd-rpm.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/clk/qcom/clk-smd-rpm.c b/drivers/clk/qcom/clk-smd-rpm.c
index 7e1814f606e9..a098327dc851 100644
--- a/drivers/clk/qcom/clk-smd-rpm.c
+++ b/drivers/clk/qcom/clk-smd-rpm.c
@@ -1066,6 +1066,8 @@ static struct clk_smd_rpm *mdm9607_clks[] = {
 static const struct rpm_smd_clk_desc rpm_clk_mdm9607 = {
 	.clks = mdm9607_clks,
 	.num_clks = ARRAY_SIZE(mdm9607_clks),
+	.keepalive_clks = pnoc_keepalive_clks,
+	.num_keepalive_clks = ARRAY_SIZE(pnoc_keepalive_clks),
 };
 
 static struct clk_smd_rpm *msm8953_clks[] = {

-- 
2.39.2

[PATCH RFT 16/20] clk: qcom: smd-rpm: Add keepalive_clks for MSM8953

[Konrad Dybcio]

MSM8953 requires that the PCNoC clock always gets an active vote.
Guarantee that.

Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
 drivers/clk/qcom/clk-smd-rpm.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/clk/qcom/clk-smd-rpm.c b/drivers/clk/qcom/clk-smd-rpm.c
index a098327dc851..dfcb2020b7b1 100644
--- a/drivers/clk/qcom/clk-smd-rpm.c
+++ b/drivers/clk/qcom/clk-smd-rpm.c
@@ -1104,6 +1104,8 @@ static struct clk_smd_rpm *msm8953_clks[] = {
 static const struct rpm_smd_clk_desc rpm_clk_msm8953 = {
 	.clks = msm8953_clks,
 	.num_clks = ARRAY_SIZE(msm8953_clks),
+	.keepalive_clks = pnoc_keepalive_clks,
+	.num_keepalive_clks = ARRAY_SIZE(pnoc_keepalive_clks),
 };
 
 static struct clk_smd_rpm *sm6125_clks[] = {

-- 
2.39.2

[PATCH RFT 17/20] clk: qcom: smd-rpm: Add keepalive_clks for SM6125

[Konrad Dybcio]

SM6125 requires that the CNoC&SNoC clocks always get an active vote.
Guarantee that.

Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
 drivers/clk/qcom/clk-smd-rpm.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/clk/qcom/clk-smd-rpm.c b/drivers/clk/qcom/clk-smd-rpm.c
index dfcb2020b7b1..a5171b818d15 100644
--- a/drivers/clk/qcom/clk-smd-rpm.c
+++ b/drivers/clk/qcom/clk-smd-rpm.c
@@ -1148,6 +1148,8 @@ static struct clk_smd_rpm *sm6125_clks[] = {
 static const struct rpm_smd_clk_desc rpm_clk_sm6125 = {
 	.clks = sm6125_clks,
 	.num_clks = ARRAY_SIZE(sm6125_clks),
+	.keepalive_clks = cnoc_snoc_keepalive_clks,
+	.num_keepalive_clks = ARRAY_SIZE(cnoc_snoc_keepalive_clks),
 };
 
 /* SM6115 */

-- 
2.39.2

[PATCH RFT 18/20] clk: qcom: smd-rpm: Add keepalive_clks for SM6115

[Konrad Dybcio]

SM6115 requires that the CNoC&SNoC clocks always get an active vote.
Guarantee that.

Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
 drivers/clk/qcom/clk-smd-rpm.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/clk/qcom/clk-smd-rpm.c b/drivers/clk/qcom/clk-smd-rpm.c
index a5171b818d15..c3ab882de7ff 100644
--- a/drivers/clk/qcom/clk-smd-rpm.c
+++ b/drivers/clk/qcom/clk-smd-rpm.c
@@ -1191,6 +1191,8 @@ static struct clk_smd_rpm *sm6115_clks[] = {
 static const struct rpm_smd_clk_desc rpm_clk_sm6115 = {
 	.clks = sm6115_clks,
 	.num_clks = ARRAY_SIZE(sm6115_clks),
+	.keepalive_clks = cnoc_snoc_keepalive_clks,
+	.num_keepalive_clks = ARRAY_SIZE(cnoc_snoc_keepalive_clks),
 };
 
 static struct clk_smd_rpm *sm6375_clks[] = {

-- 
2.39.2

[PATCH RFT 19/20] clk: qcom: smd-rpm: Add keepalive_clks for QCM2290

[Konrad Dybcio]

QCM2290 requires that the CNoC&SNoC clocks always get an active vote.
Guarantee that.

Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
 drivers/clk/qcom/clk-smd-rpm.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/clk/qcom/clk-smd-rpm.c b/drivers/clk/qcom/clk-smd-rpm.c
index c3ab882de7ff..2d0efe2cb1f5 100644
--- a/drivers/clk/qcom/clk-smd-rpm.c
+++ b/drivers/clk/qcom/clk-smd-rpm.c
@@ -1278,6 +1278,8 @@ static struct clk_smd_rpm *qcm2290_clks[] = {
 static const struct rpm_smd_clk_desc rpm_clk_qcm2290 = {
 	.clks = qcm2290_clks,
 	.num_clks = ARRAY_SIZE(qcm2290_clks),
+	.keepalive_clks = cnoc_snoc_keepalive_clks,
+	.num_keepalive_clks = ARRAY_SIZE(cnoc_snoc_keepalive_clks),
 };
 
 static const struct of_device_id rpm_smd_clk_match_table[] = {

-- 
2.39.2

[PATCH RFT 20/20] clk: qcom: smd-rpm: Add keepalive_clks for QCS404

[Konrad Dybcio]

QCS404 requires that the PCNoC clock always gets an active vote.
Guarantee that.

Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
 drivers/clk/qcom/clk-smd-rpm.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/clk/qcom/clk-smd-rpm.c b/drivers/clk/qcom/clk-smd-rpm.c
index 2d0efe2cb1f5..af8a43394349 100644
--- a/drivers/clk/qcom/clk-smd-rpm.c
+++ b/drivers/clk/qcom/clk-smd-rpm.c
@@ -934,6 +934,8 @@ static struct clk_smd_rpm *qcs404_clks[] = {
 static const struct rpm_smd_clk_desc rpm_clk_qcs404 = {
 	.clks = qcs404_clks,
 	.num_clks = ARRAY_SIZE(qcs404_clks),
+	.keepalive_clks = pnoc_keepalive_clks,
+	.num_keepalive_clks = ARRAY_SIZE(pnoc_keepalive_clks),
 };
 
 static struct clk_smd_rpm *msm8998_clks[] = {

-- 
2.39.2

Re: [PATCH RFT 01/20] clk: qcom: smd-rpm: Add .is_enabled hook

[Dmitry Baryshkov]

On Sat, 4 Mar 2023 at 15:27, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>
> From: Shawn Guo <shawn.guo@linaro.org>
>
> The RPM clock enabling state can be found with 'enabled' in struct
> clk_smd_rpm.  Add .is_enabled hook so that clk_summary in debugfs can
> show a correct enabling state for RPM clocks.

There is a small problem with this patch: now if the bootloader leaves
a clock enabled for some reason and there are no Linux users for that
clock, clk_disable_unused_subtree() will consider this clock disabled
and will not disable the clock on its own.
And at this stage of smd-rpm platforms support I think we better off
finding missing clock consumers rather than depending on the
bootloader for some of the setup.

>
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> [Konrad: rebase]
> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> ---
>  drivers/clk/qcom/clk-smd-rpm.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/drivers/clk/qcom/clk-smd-rpm.c b/drivers/clk/qcom/clk-smd-rpm.c
> index 198886c1b6c8..ecacfbc4a16c 100644
> --- a/drivers/clk/qcom/clk-smd-rpm.c
> +++ b/drivers/clk/qcom/clk-smd-rpm.c
> @@ -424,18 +424,27 @@ static int clk_smd_rpm_enable_scaling(struct qcom_smd_rpm *rpm)
>         return 0;
>  }
>
> +static int clk_smd_rpm_is_enabled(struct clk_hw *hw)
> +{
> +       struct clk_smd_rpm *r = to_clk_smd_rpm(hw);
> +
> +       return r->enabled;
> +}
> +
>  static const struct clk_ops clk_smd_rpm_ops = {
>         .prepare        = clk_smd_rpm_prepare,
>         .unprepare      = clk_smd_rpm_unprepare,
>         .set_rate       = clk_smd_rpm_set_rate,
>         .round_rate     = clk_smd_rpm_round_rate,
>         .recalc_rate    = clk_smd_rpm_recalc_rate,
> +       .is_enabled     = clk_smd_rpm_is_enabled,
>  };
>
>  static const struct clk_ops clk_smd_rpm_branch_ops = {
>         .prepare        = clk_smd_rpm_prepare,
>         .unprepare      = clk_smd_rpm_unprepare,
>         .recalc_rate    = clk_smd_rpm_recalc_rate,
> +       .is_enabled     = clk_smd_rpm_is_enabled,
>  };
>
>  DEFINE_CLK_SMD_RPM_BRANCH_A(bi_tcxo, QCOM_SMD_RPM_MISC_CLK, 0, 19200000);
>
> --
> 2.39.2
>


-- 
With best wishes
Dmitry

Re: [PATCH RFT 01/20] clk: qcom: smd-rpm: Add .is_enabled hook

[Dmitry Baryshkov]

On 06/03/2023 02:21, Dmitry Baryshkov wrote:
> On Sat, 4 Mar 2023 at 15:27, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>>
>> From: Shawn Guo <shawn.guo@linaro.org>
>>
>> The RPM clock enabling state can be found with 'enabled' in struct
>> clk_smd_rpm.  Add .is_enabled hook so that clk_summary in debugfs can
>> show a correct enabling state for RPM clocks.
> 
> There is a small problem with this patch: now if the bootloader leaves
> a clock enabled for some reason and there are no Linux users for that
> clock, clk_disable_unused_subtree() will consider this clock disabled
> and will not disable the clock on its own.
> And at this stage of smd-rpm platforms support I think we better off
> finding missing clock consumers rather than depending on the
> bootloader for some of the setup.

Argh. And then I understood that without the patch clk_core_is_enabled 
will shortcut to core->enable_count, which also means that the clock 
will be skipped.

But now I have another question: since r->enabled follows 
core->enabled_count, can we get rid of r->enabled and switch to 
core->enable_count? This might mean adding clk_software_is_enabled() 
function for your patch.

-- 
With best wishes
Dmitry

Re: [PATCH RFT 02/20] clk: qcom: smd-rpm: Add .is_prepared hook

[Dmitry Baryshkov]

On Sat, 4 Mar 2023 at 15:27, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>
> From: Shawn Guo <shawn.guo@linaro.org>
>
> The RPM clocks are enabled/disabled through clk framework prepare/unprepare
> hooks.  Without .is_prepared hook, those unused RPM clocks will not be
> disabled by core function clk_unprepare_unused_subtree(), because
> clk_core_is_prepared() always returns 0.
>
> Add .is_prepared hook to clk_ops and return the clock prepare (enable)
> state, so that those unused RPM clocks can be disabled by clk framework.

Does it make any difference? Without this patch the clock framework
depends on core->prepare_count to judge whether the clock should be
unprepared. With this patch in place, it will depend on r->enabled,
which is more or less the same thing.
Probably there is one category, the prepared-but-not-enabled clocks.

>
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> [Konrad: rebase, don't duplicate the enable func]
> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> ---
>  drivers/clk/qcom/clk-smd-rpm.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/clk/qcom/clk-smd-rpm.c b/drivers/clk/qcom/clk-smd-rpm.c
> index ecacfbc4a16c..cce7daa97c1e 100644
> --- a/drivers/clk/qcom/clk-smd-rpm.c
> +++ b/drivers/clk/qcom/clk-smd-rpm.c
> @@ -438,6 +438,7 @@ static const struct clk_ops clk_smd_rpm_ops = {
>         .round_rate     = clk_smd_rpm_round_rate,
>         .recalc_rate    = clk_smd_rpm_recalc_rate,
>         .is_enabled     = clk_smd_rpm_is_enabled,
> +       .is_prepared    = clk_smd_rpm_is_enabled,
>  };
>
>  static const struct clk_ops clk_smd_rpm_branch_ops = {
> @@ -445,6 +446,7 @@ static const struct clk_ops clk_smd_rpm_branch_ops = {
>         .unprepare      = clk_smd_rpm_unprepare,
>         .recalc_rate    = clk_smd_rpm_recalc_rate,
>         .is_enabled     = clk_smd_rpm_is_enabled,
> +       .is_prepared    = clk_smd_rpm_is_enabled,
>  };
>
>  DEFINE_CLK_SMD_RPM_BRANCH_A(bi_tcxo, QCOM_SMD_RPM_MISC_CLK, 0, 19200000);
>
> --
> 2.39.2
>


-- 
With best wishes
Dmitry

Re: [PATCH RFT 03/20] clk: qcom: smd-rpm: Add support for keepalive votes

[Dmitry Baryshkov]

On Sat, 4 Mar 2023 at 15:27, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>
> Some bus clock should always have a minimum (19.2 MHz) vote cast on
> them, otherwise the platform will fall apart, hang and reboot.
>
> Add support for specifying which clocks should be kept alive and
> always keep a vote on XO_A to make sure the clock tree doesn't
> collapse. This removes the need to keep a maximum vote that was
> previously guaranteed by clk_smd_rpm_handoff.
>
> This commit is a combination of existing (not-exactly-upstream) work
> by Taniya Das, Shawn Guo and myself.
>
> Co-developed-by: Shawn Guo <shawn.guo@linaro.org>
> Co-developed-by: Taniya Das <quic_tdas@quicinc.com>
> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> ---
>  drivers/clk/qcom/clk-smd-rpm.c | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
>
> diff --git a/drivers/clk/qcom/clk-smd-rpm.c b/drivers/clk/qcom/clk-smd-rpm.c
> index cce7daa97c1e..8e017c575361 100644
> --- a/drivers/clk/qcom/clk-smd-rpm.c
> +++ b/drivers/clk/qcom/clk-smd-rpm.c
> @@ -4,6 +4,7 @@
>   * Copyright (c) 2014, The Linux Foundation. All rights reserved.
>   */
>
> +#include <linux/clk.h>
>  #include <linux/clk-provider.h>
>  #include <linux/err.h>
>  #include <linux/export.h>
> @@ -178,6 +179,8 @@ struct clk_smd_rpm_req {
>  struct rpm_smd_clk_desc {
>         struct clk_smd_rpm **clks;
>         size_t num_clks;
> +       struct clk_hw **keepalive_clks;
> +       size_t num_keepalive_clks;
>  };
>
>  static DEFINE_MUTEX(rpm_smd_clk_lock);
> @@ -1278,6 +1281,7 @@ static int rpm_smd_clk_probe(struct platform_device *pdev)
>         struct qcom_smd_rpm *rpm;
>         struct clk_smd_rpm **rpm_smd_clks;
>         const struct rpm_smd_clk_desc *desc;
> +       struct clk_hw **keepalive_clks;
>
>         rpm = dev_get_drvdata(pdev->dev.parent);
>         if (!rpm) {
> @@ -1291,6 +1295,7 @@ static int rpm_smd_clk_probe(struct platform_device *pdev)
>
>         rpm_smd_clks = desc->clks;
>         num_clks = desc->num_clks;
> +       keepalive_clks = desc->keepalive_clks;
>
>         for (i = 0; i < num_clks; i++) {
>                 if (!rpm_smd_clks[i])
> @@ -1321,6 +1326,24 @@ static int rpm_smd_clk_probe(struct platform_device *pdev)
>         if (ret)
>                 goto err;
>
> +       /* Leave a permanent active vote on clocks that require it. */
> +       for (i = 0; i < desc->num_keepalive_clks; i++) {
> +               if (WARN_ON(!keepalive_clks[i]))
> +                       continue;
> +
> +               ret = clk_prepare_enable(keepalive_clks[i]->clk);
> +               if (ret)
> +                       return ret;

Would it be better to use CLK_IS_CRITICAL instead? Using the existing
API has a bonus that it is more visible compared to the ad-hoc
solutions.

> +
> +               ret = clk_set_rate(keepalive_clks[i]->clk, 19200000);

Don't we also need to provide a determine_rate() that will not allow
one to set clock frequency below 19.2 MHz?

> +               if (ret)
> +                       return ret;
> +       }
> +
> +       /* Keep an active vote on CXO in case no other driver votes for it. */
> +       if (rpm_smd_clks[RPM_SMD_XO_A_CLK_SRC])
> +               return clk_prepare_enable(rpm_smd_clks[RPM_SMD_XO_A_CLK_SRC]->hw.clk);
> +
>         return 0;
>  err:
>         dev_err(&pdev->dev, "Error registering SMD clock driver (%d)\n", ret);


I have mixed feelings towards this patch (and the rest of the
patchset). It looks to me like we are trying to patch an issue of the
interconnect drivers (or in kernel configuration).


--
With best wishes
Dmitry

Re: [PATCH RFT 03/20] clk: qcom: smd-rpm: Add support for keepalive votes

[Konrad Dybcio]

On 6.03.2023 02:21, Dmitry Baryshkov wrote:
> On Sat, 4 Mar 2023 at 15:27, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>>
>> Some bus clock should always have a minimum (19.2 MHz) vote cast on
>> them, otherwise the platform will fall apart, hang and reboot.
>>
>> Add support for specifying which clocks should be kept alive and
>> always keep a vote on XO_A to make sure the clock tree doesn't
>> collapse. This removes the need to keep a maximum vote that was
>> previously guaranteed by clk_smd_rpm_handoff.
>>
>> This commit is a combination of existing (not-exactly-upstream) work
>> by Taniya Das, Shawn Guo and myself.
>>
>> Co-developed-by: Shawn Guo <shawn.guo@linaro.org>
>> Co-developed-by: Taniya Das <quic_tdas@quicinc.com>
>> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
>> ---
>>  drivers/clk/qcom/clk-smd-rpm.c | 23 +++++++++++++++++++++++
>>  1 file changed, 23 insertions(+)
>>
>> diff --git a/drivers/clk/qcom/clk-smd-rpm.c b/drivers/clk/qcom/clk-smd-rpm.c
>> index cce7daa97c1e..8e017c575361 100644
>> --- a/drivers/clk/qcom/clk-smd-rpm.c
>> +++ b/drivers/clk/qcom/clk-smd-rpm.c
>> @@ -4,6 +4,7 @@
>>   * Copyright (c) 2014, The Linux Foundation. All rights reserved.
>>   */
>>
>> +#include <linux/clk.h>
>>  #include <linux/clk-provider.h>
>>  #include <linux/err.h>
>>  #include <linux/export.h>
>> @@ -178,6 +179,8 @@ struct clk_smd_rpm_req {
>>  struct rpm_smd_clk_desc {
>>         struct clk_smd_rpm **clks;
>>         size_t num_clks;
>> +       struct clk_hw **keepalive_clks;
>> +       size_t num_keepalive_clks;
>>  };
>>
>>  static DEFINE_MUTEX(rpm_smd_clk_lock);
>> @@ -1278,6 +1281,7 @@ static int rpm_smd_clk_probe(struct platform_device *pdev)
>>         struct qcom_smd_rpm *rpm;
>>         struct clk_smd_rpm **rpm_smd_clks;
>>         const struct rpm_smd_clk_desc *desc;
>> +       struct clk_hw **keepalive_clks;
>>
>>         rpm = dev_get_drvdata(pdev->dev.parent);
>>         if (!rpm) {
>> @@ -1291,6 +1295,7 @@ static int rpm_smd_clk_probe(struct platform_device *pdev)
>>
>>         rpm_smd_clks = desc->clks;
>>         num_clks = desc->num_clks;
>> +       keepalive_clks = desc->keepalive_clks;
>>
>>         for (i = 0; i < num_clks; i++) {
>>                 if (!rpm_smd_clks[i])
>> @@ -1321,6 +1326,24 @@ static int rpm_smd_clk_probe(struct platform_device *pdev)
>>         if (ret)
>>                 goto err;
>>
>> +       /* Leave a permanent active vote on clocks that require it. */
>> +       for (i = 0; i < desc->num_keepalive_clks; i++) {
>> +               if (WARN_ON(!keepalive_clks[i]))
>> +                       continue;
>> +
>> +               ret = clk_prepare_enable(keepalive_clks[i]->clk);
>> +               if (ret)
>> +                       return ret;
> 
> Would it be better to use CLK_IS_CRITICAL instead? Using the existing
> API has a bonus that it is more visible compared to the ad-hoc
> solutions.
Yeah, I think that makes sense.

> 
>> +
>> +               ret = clk_set_rate(keepalive_clks[i]->clk, 19200000);
> 
> Don't we also need to provide a determine_rate() that will not allow
> one to set clock frequency below 19.2 MHz?
Hm, sounds like a good idea..

> 
>> +               if (ret)
>> +                       return ret;
>> +       }
>> +
>> +       /* Keep an active vote on CXO in case no other driver votes for it. */
>> +       if (rpm_smd_clks[RPM_SMD_XO_A_CLK_SRC])
>> +               return clk_prepare_enable(rpm_smd_clks[RPM_SMD_XO_A_CLK_SRC]->hw.clk);
>> +
>>         return 0;
>>  err:
>>         dev_err(&pdev->dev, "Error registering SMD clock driver (%d)\n", ret);
> 
> 
> I have mixed feelings towards this patch (and the rest of the
> patchset). It looks to me like we are trying to patch an issue of the
> interconnect drivers (or in kernel configuration).
Well, as you noticed, this patch tries to address a situation where a
critical clock could be disabled. The interconnect driver (as per my
other recent patchset) also has a concept of "keepalive", but:

1. not very many SoCs already have a functional icc driver
2. devices with an existing interconnect driver should also be
   testable without one (through painful ripping out everything-icc
   from the dts) for regression tracking

Konrad

> 
> 
> --
> With best wishes
> Dmitry

Re: [PATCH RFT 03/20] clk: qcom: smd-rpm: Add support for keepalive votes

[Konrad Dybcio]

On 6.03.2023 12:28, Konrad Dybcio wrote:
> 
> 
> On 6.03.2023 02:21, Dmitry Baryshkov wrote:
>> On Sat, 4 Mar 2023 at 15:27, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>>>
>>> Some bus clock should always have a minimum (19.2 MHz) vote cast on
>>> them, otherwise the platform will fall apart, hang and reboot.
>>>
>>> Add support for specifying which clocks should be kept alive and
>>> always keep a vote on XO_A to make sure the clock tree doesn't
>>> collapse. This removes the need to keep a maximum vote that was
>>> previously guaranteed by clk_smd_rpm_handoff.
>>>
>>> This commit is a combination of existing (not-exactly-upstream) work
>>> by Taniya Das, Shawn Guo and myself.
>>>
>>> Co-developed-by: Shawn Guo <shawn.guo@linaro.org>
>>> Co-developed-by: Taniya Das <quic_tdas@quicinc.com>
>>> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
>>> ---
[...]

>>
>>> +
>>> +               ret = clk_set_rate(keepalive_clks[i]->clk, 19200000);
>>
>> Don't we also need to provide a determine_rate() that will not allow
>> one to set clock frequency below 19.2 MHz?
> Hm, sounds like a good idea..
Thinking about it again, I'd have to do it before the clocks are registered
and we'd either end up with 2 loops, one assigning the CLK_IS_CRITICAL flag
and the other one setting the rate.. Will that not be too hacky?

Konrad

> 
>>
>>> +               if (ret)
>>> +                       return ret;
>>> +       }
>>> +
>>> +       /* Keep an active vote on CXO in case no other driver votes for it. */
>>> +       if (rpm_smd_clks[RPM_SMD_XO_A_CLK_SRC])
>>> +               return clk_prepare_enable(rpm_smd_clks[RPM_SMD_XO_A_CLK_SRC]->hw.clk);
>>> +
>>>         return 0;
>>>  err:
>>>         dev_err(&pdev->dev, "Error registering SMD clock driver (%d)\n", ret);
>>
>>
>> I have mixed feelings towards this patch (and the rest of the
>> patchset). It looks to me like we are trying to patch an issue of the
>> interconnect drivers (or in kernel configuration).
> Well, as you noticed, this patch tries to address a situation where a
> critical clock could be disabled. The interconnect driver (as per my
> other recent patchset) also has a concept of "keepalive", but:
> 
> 1. not very many SoCs already have a functional icc driver
> 2. devices with an existing interconnect driver should also be
>    testable without one (through painful ripping out everything-icc
>    from the dts) for regression tracking
> 
> Konrad
> 
>>
>>
>> --
>> With best wishes
>> Dmitry