[PATCH 0/2] cpufreq: qcom: Clock provider fixes

[PATCH 0/2] cpufreq: qcom: Clock provider fixes

[Manivannan Sadhasivam via B4 Relay]

Hi,

This series has a couple of fixes for the Qcom CPUFreq clock provider.
Patch 1 fixes an issue where incorrect rate might be reported if LMh IRQ is not
available for a platform (issue identified based on code inspection). Patch 2
fixes a regression reported for v6.13-rc1 [1]. The regression was triggered by
commit 25f1c96a0e84 ("clk: Fix invalid execution of clk_set_rate"), which fixed
the behavior of the clk_set_rate() API. Even though the commit got reverted now,
patch 2 fixes the issue in the clock provider code.

This series is tested on Qcom RB5 development board.

[1] https://lore.kernel.org/all/20241202100621.29209-1-johan+linaro@kernel.org

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
Manivannan Sadhasivam (2):
      cpufreq: qcom: Fix qcom_cpufreq_hw_recalc_rate() to query LUT if LMh IRQ is not available
      cpufreq: qcom: Implement clk_ops::determine_rate() for qcom_cpufreq* clocks

 drivers/cpufreq/qcom-cpufreq-hw.c | 34 ++++++++++++++++++++++++----------
 1 file changed, 24 insertions(+), 10 deletions(-)
---
base-commit: 40384c840ea1944d7c5a392e8975ed088ecf0b37
change-id: 20241205-qcom-cpufreq-clk-fix-a60fbfdd7b84

Best regards,
-- 
Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

[PATCH 1/2] cpufreq: qcom: Fix qcom_cpufreq_hw_recalc_rate() to query LUT if LMh IRQ is not available

[Manivannan Sadhasivam via B4 Relay]

From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

Currently, qcom_cpufreq_hw_recalc_rate() returns the LMh throttled
frequency for the domain even if LMh IRQ is not available. But as per
qcom_cpufreq_hw_get(), the driver has to query LUT entries to get the
actual frequency of the domain. So do the same in
qcom_cpufreq_hw_recalc_rate().

While doing so, refactor the existing qcom_cpufreq_hw_get() function so
that qcom_cpufreq_hw_recalc_rate() can make use of the existing code and
avoid code duplication. This also requires setting the
qcom_cpufreq_data::policy even if LMh IRQ is not available.

Fixes: 4370232c727b ("cpufreq: qcom-hw: Add CPU clock provider support")
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 drivers/cpufreq/qcom-cpufreq-hw.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c
index 98129565acb8..c145ab7b0bb2 100644
--- a/drivers/cpufreq/qcom-cpufreq-hw.c
+++ b/drivers/cpufreq/qcom-cpufreq-hw.c
@@ -143,14 +143,12 @@ static unsigned long qcom_lmh_get_throttle_freq(struct qcom_cpufreq_data *data)
 }
 
 /* Get the frequency requested by the cpufreq core for the CPU */
-static unsigned int qcom_cpufreq_get_freq(unsigned int cpu)
+static unsigned int qcom_cpufreq_get_freq(struct cpufreq_policy *policy)
 {
 	struct qcom_cpufreq_data *data;
 	const struct qcom_cpufreq_soc_data *soc_data;
-	struct cpufreq_policy *policy;
 	unsigned int index;
 
-	policy = cpufreq_cpu_get_raw(cpu);
 	if (!policy)
 		return 0;
 
@@ -163,12 +161,10 @@ static unsigned int qcom_cpufreq_get_freq(unsigned int cpu)
 	return policy->freq_table[index].frequency;
 }
 
-static unsigned int qcom_cpufreq_hw_get(unsigned int cpu)
+static unsigned int __qcom_cpufreq_hw_get(struct cpufreq_policy *policy)
 {
 	struct qcom_cpufreq_data *data;
-	struct cpufreq_policy *policy;
 
-	policy = cpufreq_cpu_get_raw(cpu);
 	if (!policy)
 		return 0;
 
@@ -177,7 +173,12 @@ static unsigned int qcom_cpufreq_hw_get(unsigned int cpu)
 	if (data->throttle_irq >= 0)
 		return qcom_lmh_get_throttle_freq(data) / HZ_PER_KHZ;
 
-	return qcom_cpufreq_get_freq(cpu);
+	return qcom_cpufreq_get_freq(policy);
+}
+
+static unsigned int qcom_cpufreq_hw_get(unsigned int cpu)
+{
+	return __qcom_cpufreq_hw_get(cpufreq_cpu_get_raw(cpu));
 }
 
 static unsigned int qcom_cpufreq_hw_fast_switch(struct cpufreq_policy *policy,
@@ -363,7 +364,7 @@ static void qcom_lmh_dcvs_notify(struct qcom_cpufreq_data *data)
 	 * If h/w throttled frequency is higher than what cpufreq has requested
 	 * for, then stop polling and switch back to interrupt mechanism.
 	 */
-	if (throttled_freq >= qcom_cpufreq_get_freq(cpu))
+	if (throttled_freq >= qcom_cpufreq_get_freq(cpufreq_cpu_get_raw(cpu)))
 		enable_irq(data->throttle_irq);
 	else
 		mod_delayed_work(system_highpri_wq, &data->throttle_work,
@@ -441,7 +442,6 @@ static int qcom_cpufreq_hw_lmh_init(struct cpufreq_policy *policy, int index)
 		return data->throttle_irq;
 
 	data->cancel_throttle = false;
-	data->policy = policy;
 
 	mutex_init(&data->throttle_lock);
 	INIT_DEFERRABLE_WORK(&data->throttle_work, qcom_lmh_dcvs_poll);
@@ -552,6 +552,7 @@ static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy)
 
 	policy->driver_data = data;
 	policy->dvfs_possible_from_any_cpu = true;
+	data->policy = policy;
 
 	ret = qcom_cpufreq_hw_read_lut(cpu_dev, policy);
 	if (ret) {
@@ -622,7 +623,7 @@ static unsigned long qcom_cpufreq_hw_recalc_rate(struct clk_hw *hw, unsigned lon
 {
 	struct qcom_cpufreq_data *data = container_of(hw, struct qcom_cpufreq_data, cpu_clk);
 
-	return qcom_lmh_get_throttle_freq(data);
+	return __qcom_cpufreq_hw_get(data->policy) * HZ_PER_KHZ;
 }
 
 static const struct clk_ops qcom_cpufreq_hw_clk_ops = {

-- 
2.25.1

[PATCH 2/2] cpufreq: qcom: Implement clk_ops::determine_rate() for qcom_cpufreq* clocks

[Manivannan Sadhasivam via B4 Relay]

From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

determine_rate() callback is used by the clk_set_rate() API to get the
closest rate of the target rate supported by the clock. If this callback
is not implemented (nor round_rate() callback), then the API will assume
that the clock cannot set the requested rate. And since there is no parent,
it will return -EINVAL.

This is not an issue right now as clk_set_rate() mistakenly compares the
target rate with cached rate and bails out early. But once that is fixed
to compare the target rate with the actual rate of the clock (returned by
recalc_rate()), then clk_set_rate() for this clock will start to fail as
below:

cpu cpu0: _opp_config_clk_single: failed to set clock rate: -22

So implement the determine_rate() callback that just returns the actual
rate at which the clock is passed to the CPUs in a domain.

Fixes: 4370232c727b ("cpufreq: qcom-hw: Add CPU clock provider support")
Reported-by: Johan Hovold <johan+linaro@kernel.org>
Closes: https://lore.kernel.org/all/20241202100621.29209-1-johan+linaro@kernel.org
Suggested-by: Stephen Boyd <sboyd@kernel.org>
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 drivers/cpufreq/qcom-cpufreq-hw.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c
index c145ab7b0bb2..b2e7e89feaac 100644
--- a/drivers/cpufreq/qcom-cpufreq-hw.c
+++ b/drivers/cpufreq/qcom-cpufreq-hw.c
@@ -626,8 +626,21 @@ static unsigned long qcom_cpufreq_hw_recalc_rate(struct clk_hw *hw, unsigned lon
 	return __qcom_cpufreq_hw_get(data->policy) * HZ_PER_KHZ;
 }
 
+/*
+ * Since we cannot determine the closest rate of the target rate, let's just
+ * return the actual rate at which the clock is running at. This is needed to
+ * make clk_set_rate() API work properly.
+ */
+static int qcom_cpufreq_hw_determine_rate(struct clk_hw *hw, struct clk_rate_request *req)
+{
+	req->rate = qcom_cpufreq_hw_recalc_rate(hw, 0);
+
+	return 0;
+}
+
 static const struct clk_ops qcom_cpufreq_hw_clk_ops = {
 	.recalc_rate = qcom_cpufreq_hw_recalc_rate,
+	.determine_rate = qcom_cpufreq_hw_determine_rate,
 };
 
 static int qcom_cpufreq_hw_driver_probe(struct platform_device *pdev)

-- 
2.25.1

Re: [PATCH 2/2] cpufreq: qcom: Implement clk_ops::determine_rate() for qcom_cpufreq* clocks

[Stephen Boyd]

Quoting Manivannan Sadhasivam via B4 Relay (2024-12-05 08:50:29)
> From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> 
> determine_rate() callback is used by the clk_set_rate() API to get the
> closest rate of the target rate supported by the clock. If this callback
> is not implemented (nor round_rate() callback), then the API will assume
> that the clock cannot set the requested rate. And since there is no parent,
> it will return -EINVAL.
> 
> This is not an issue right now as clk_set_rate() mistakenly compares the
> target rate with cached rate and bails out early. But once that is fixed
> to compare the target rate with the actual rate of the clock (returned by
> recalc_rate()), then clk_set_rate() for this clock will start to fail as
> below:
> 
> cpu cpu0: _opp_config_clk_single: failed to set clock rate: -22
> 
> So implement the determine_rate() callback that just returns the actual
> rate at which the clock is passed to the CPUs in a domain.
> 
> Fixes: 4370232c727b ("cpufreq: qcom-hw: Add CPU clock provider support")
> Reported-by: Johan Hovold <johan+linaro@kernel.org>
> Closes: https://lore.kernel.org/all/20241202100621.29209-1-johan+linaro@kernel.org
> Suggested-by: Stephen Boyd <sboyd@kernel.org>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---

Reviewed-by: Stephen Boyd <sboyd@kernel.org>

Re: [PATCH 0/2] cpufreq: qcom: Clock provider fixes

[Viresh Kumar]

On 05-12-24, 22:20, Manivannan Sadhasivam via B4 Relay wrote:
> Hi,
> 
> This series has a couple of fixes for the Qcom CPUFreq clock provider.
> Patch 1 fixes an issue where incorrect rate might be reported if LMh IRQ is not
> available for a platform (issue identified based on code inspection). Patch 2
> fixes a regression reported for v6.13-rc1 [1]. The regression was triggered by
> commit 25f1c96a0e84 ("clk: Fix invalid execution of clk_set_rate"), which fixed
> the behavior of the clk_set_rate() API. Even though the commit got reverted now,
> patch 2 fixes the issue in the clock provider code.
> 
> This series is tested on Qcom RB5 development board.
> 
> [1] https://lore.kernel.org/all/20241202100621.29209-1-johan+linaro@kernel.org
> 
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---
> Manivannan Sadhasivam (2):
>       cpufreq: qcom: Fix qcom_cpufreq_hw_recalc_rate() to query LUT if LMh IRQ is not available
>       cpufreq: qcom: Implement clk_ops::determine_rate() for qcom_cpufreq* clocks
> 
>  drivers/cpufreq/qcom-cpufreq-hw.c | 34 ++++++++++++++++++++++++----------
>  1 file changed, 24 insertions(+), 10 deletions(-)

Applied. Thanks.

-- 
viresh