* [PATCH 1/2] cpufreq: qcom: Fix qcom_cpufreq_hw_recalc_rate() to query LUT if LMh IRQ is not available
2024-12-05 16:50 [PATCH 0/2] cpufreq: qcom: Clock provider fixes Manivannan Sadhasivam via B4 Relay
@ 2024-12-05 16:50 ` Manivannan Sadhasivam via B4 Relay
2024-12-05 16:50 ` [PATCH 2/2] cpufreq: qcom: Implement clk_ops::determine_rate() for qcom_cpufreq* clocks Manivannan Sadhasivam via B4 Relay
2024-12-20 6:40 ` [PATCH 0/2] cpufreq: qcom: Clock provider fixes Viresh Kumar
2 siblings, 0 replies; 5+ messages in thread
From: Manivannan Sadhasivam via B4 Relay @ 2024-12-05 16:50 UTC (permalink / raw)
To: Rafael J. Wysocki, Viresh Kumar, Xiu Jianfeng
Cc: linux-arm-msm, linux-pm, linux-clk, linux-kernel, Johan Hovold,
Stephen Boyd, Manivannan Sadhasivam
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
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 2/2] cpufreq: qcom: Implement clk_ops::determine_rate() for qcom_cpufreq* clocks
2024-12-05 16:50 [PATCH 0/2] cpufreq: qcom: Clock provider fixes Manivannan Sadhasivam via B4 Relay
2024-12-05 16:50 ` [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
@ 2024-12-05 16:50 ` Manivannan Sadhasivam via B4 Relay
2024-12-17 19:46 ` Stephen Boyd
2024-12-20 6:40 ` [PATCH 0/2] cpufreq: qcom: Clock provider fixes Viresh Kumar
2 siblings, 1 reply; 5+ messages in thread
From: Manivannan Sadhasivam via B4 Relay @ 2024-12-05 16:50 UTC (permalink / raw)
To: Rafael J. Wysocki, Viresh Kumar, Xiu Jianfeng
Cc: linux-arm-msm, linux-pm, linux-clk, linux-kernel, Johan Hovold,
Stephen Boyd, Manivannan Sadhasivam
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
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH 0/2] cpufreq: qcom: Clock provider fixes
2024-12-05 16:50 [PATCH 0/2] cpufreq: qcom: Clock provider fixes Manivannan Sadhasivam via B4 Relay
2024-12-05 16:50 ` [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
2024-12-05 16:50 ` [PATCH 2/2] cpufreq: qcom: Implement clk_ops::determine_rate() for qcom_cpufreq* clocks Manivannan Sadhasivam via B4 Relay
@ 2024-12-20 6:40 ` Viresh Kumar
2 siblings, 0 replies; 5+ messages in thread
From: Viresh Kumar @ 2024-12-20 6:40 UTC (permalink / raw)
To: manivannan.sadhasivam
Cc: Rafael J. Wysocki, Xiu Jianfeng, linux-arm-msm, linux-pm,
linux-clk, linux-kernel, Johan Hovold, Stephen Boyd
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
^ permalink raw reply [flat|nested] 5+ messages in thread