linux-kernel - [PATCH v2] clk: qcom: smd: Disable unused clocks

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20231004-clk-qcom-smd-rpm-unused-v2-1-9a5281f324dc@kernkonzept.com>
Date:   Wed, 04 Oct 2023 14:10:34 +0200
From:   Stephan Gerhold <stephan.gerhold@...nkonzept.com>
To:     Bjorn Andersson <andersson@...nel.org>
Cc:     Andy Gross <agross@...nel.org>,
        Konrad Dybcio <konrad.dybcio@...aro.org>,
        Michael Turquette <mturquette@...libre.com>,
        Stephen Boyd <sboyd@...nel.org>, linux-arm-msm@...r.kernel.org,
        linux-clk@...r.kernel.org, linux-kernel@...r.kernel.org,
        Stephan Gerhold <stephan@...hold.net>,
        Stephan Gerhold <stephan.gerhold@...nkonzept.com>
Subject: [PATCH v2] clk: qcom: smd: Disable unused clocks

At the moment, clk-smd-rpm forces all clocks on at probe time (for
"handoff"). However, it does not make the clk core aware of that.

This means that the clocks stay enabled forever if they are not used
by anything. We can easily disable them again after bootup has been
completed, by making the clk core aware of the state. This is
implemented by returning the current state of the clock in
is_prepared().

Checking the SPMI clock registers reveals that this allows the RPM to
disable unused BB/RF clocks. This reduces the power consumption quite
significantly and is also needed to allow entering low-power states.

As of commit d6edc31f3a68 ("clk: qcom: smd-rpm: Separate out
interconnect bus clocks") the interconnect-related clocks are no longer
managed/exposed by clk-smd-rpm. Also the BI_TCXO_AO clock is now
critical (and never disabled).

There is still a slight chance that this change will break boot on some
devices. However, this will be most likely caused by actual mistakes in
the device tree (where required clocks were not actually specified).

Signed-off-by: Stephan Gerhold <stephan.gerhold@...nkonzept.com>
---
Changes in v2:
- Rebase on latest qcom/for-next, update commit message with other recent
  related changes
- Link to v1: https://lore.kernel.org/linux-arm-msm/20200817140908.185976-1-stephan@gerhold.net/
---
Keeping all unused clocks on makes it very easy to forget to enable
actually required clocks. [1] is one example of that, where the crypto
engine worked fine without any clocks. IMHO we should try to get this
change in sooner than later to avoid introducing more new mistakes.

[1]: https://lore.kernel.org/linux-arm-msm/ZGdLCdSof027mk5u@gerhold.net/
---
 drivers/clk/qcom/clk-smd-rpm.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/clk/qcom/clk-smd-rpm.c b/drivers/clk/qcom/clk-smd-rpm.c
index 0191fc0dd7da..eba650ad7291 100644
--- a/drivers/clk/qcom/clk-smd-rpm.c
+++ b/drivers/clk/qcom/clk-smd-rpm.c
@@ -335,6 +335,13 @@ static void clk_smd_rpm_unprepare(struct clk_hw *hw)
 	mutex_unlock(&rpm_smd_clk_lock);
 }
 
+static int clk_smd_rpm_is_prepared(struct clk_hw *hw)
+{
+	struct clk_smd_rpm *r = to_clk_smd_rpm(hw);
+
+	return r->enabled;
+}
+
 static int clk_smd_rpm_set_rate(struct clk_hw *hw, unsigned long rate,
 				unsigned long parent_rate)
 {
@@ -431,6 +438,7 @@ static int clk_smd_rpm_enable_scaling(void)
 static const struct clk_ops clk_smd_rpm_ops = {
 	.prepare	= clk_smd_rpm_prepare,
 	.unprepare	= clk_smd_rpm_unprepare,
+	.is_prepared	= clk_smd_rpm_is_prepared,
 	.set_rate	= clk_smd_rpm_set_rate,
 	.round_rate	= clk_smd_rpm_round_rate,
 	.recalc_rate	= clk_smd_rpm_recalc_rate,
@@ -439,6 +447,7 @@ static const struct clk_ops clk_smd_rpm_ops = {
 static const struct clk_ops clk_smd_rpm_branch_ops = {
 	.prepare	= clk_smd_rpm_prepare,
 	.unprepare	= clk_smd_rpm_unprepare,
+	.is_prepared	= clk_smd_rpm_is_prepared,
 	.recalc_rate	= clk_smd_rpm_recalc_rate,
 };
 
@@ -1279,6 +1288,9 @@ static int rpm_smd_clk_probe(struct platform_device *pdev)
 		ret = clk_smd_rpm_handoff(rpm_smd_clks[i]);
 		if (ret)
 			goto err;
+
+		/* During handoff we force all clocks on */
+		rpm_smd_clks[i]->enabled = true;
 	}
 
 	for (i = 0; i < desc->num_icc_clks; i++) {

---
base-commit: 870b5222204243ee6dbeada9ad1b90cda9ecb4da
change-id: 20231004-clk-qcom-smd-rpm-unused-b79d00122811

Best regards,
-- 
Stephan Gerhold <stephan.gerhold@...nkonzept.com>
Kernkonzept GmbH at Dresden, Germany, HRB 31129, CEO Dr.-Ing. Michael Hohmuth