[PATCH v3] cpufreq: qcom-cpufreq-hw: Clear dcvs interrupts

[PATCH v3] cpufreq: qcom-cpufreq-hw: Clear dcvs interrupts

[Vladimir Zapolskiy]

It's noted that dcvs interrupts are not self-clearing, thus an interrupt
handler runs constantly, which leads to a severe regression in runtime.
To fix the problem an explicit write to clear interrupt register is
required, note that on OSM platforms the register may not be present.

Fixes: 275157b367f4 ("cpufreq: qcom-cpufreq-hw: Add dcvs interrupt support")
Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
---
Changes from v2 to v3:
* split the change from the series and returned to v1 state of the fix
  by removing a minor optimization
* added a check for non-zero reg_intr_clr value before writel

Changes from v1 to v2:
* added a check for pending interrupt status before its handling

 drivers/cpufreq/qcom-cpufreq-hw.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c
index 0ec18e1589dc..0253731d6d25 100644
--- a/drivers/cpufreq/qcom-cpufreq-hw.c
+++ b/drivers/cpufreq/qcom-cpufreq-hw.c
@@ -24,6 +24,8 @@
 #define CLK_HW_DIV			2
 #define LUT_TURBO_IND			1
 
+#define GT_IRQ_STATUS			BIT(2)
+
 #define HZ_PER_KHZ			1000
 
 struct qcom_cpufreq_soc_data {
@@ -32,6 +34,7 @@ struct qcom_cpufreq_soc_data {
 	u32 reg_dcvs_ctrl;
 	u32 reg_freq_lut;
 	u32 reg_volt_lut;
+	u32 reg_intr_clr;
 	u32 reg_current_vote;
 	u32 reg_perf_state;
 	u8 lut_row_size;
@@ -360,6 +363,10 @@ static irqreturn_t qcom_lmh_dcvs_handle_irq(int irq, void *data)
 	disable_irq_nosync(c_data->throttle_irq);
 	schedule_delayed_work(&c_data->throttle_work, 0);
 
+	if (c_data->soc_data->reg_intr_clr)
+		writel_relaxed(GT_IRQ_STATUS,
+			       c_data->base + c_data->soc_data->reg_intr_clr);
+
 	return IRQ_HANDLED;
 }
 
@@ -379,6 +386,7 @@ static const struct qcom_cpufreq_soc_data epss_soc_data = {
 	.reg_dcvs_ctrl = 0xb0,
 	.reg_freq_lut = 0x100,
 	.reg_volt_lut = 0x200,
+	.reg_intr_clr = 0x308,
 	.reg_perf_state = 0x320,
 	.lut_row_size = 4,
 };
-- 
2.33.0

Re: [PATCH v3] cpufreq: qcom-cpufreq-hw: Clear dcvs interrupts

[Vladimir Zapolskiy]

Hi Bjorn, Viresh,

On 4/7/22 23:09, Vladimir Zapolskiy wrote:
> It's noted that dcvs interrupts are not self-clearing, thus an interrupt
> handler runs constantly, which leads to a severe regression in runtime.
> To fix the problem an explicit write to clear interrupt register is
> required, note that on OSM platforms the register may not be present.
> 
> Fixes: 275157b367f4 ("cpufreq: qcom-cpufreq-hw: Add dcvs interrupt support")
> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
> ---
> Changes from v2 to v3:
> * split the change from the series and returned to v1 state of the fix
>    by removing a minor optimization
> * added a check for non-zero reg_intr_clr value before writel
> 
> Changes from v1 to v2:
> * added a check for pending interrupt status before its handling
> 
>   drivers/cpufreq/qcom-cpufreq-hw.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c
> index 0ec18e1589dc..0253731d6d25 100644
> --- a/drivers/cpufreq/qcom-cpufreq-hw.c
> +++ b/drivers/cpufreq/qcom-cpufreq-hw.c
> @@ -24,6 +24,8 @@
>   #define CLK_HW_DIV			2
>   #define LUT_TURBO_IND			1
>   
> +#define GT_IRQ_STATUS			BIT(2)
> +
>   #define HZ_PER_KHZ			1000
>   
>   struct qcom_cpufreq_soc_data {
> @@ -32,6 +34,7 @@ struct qcom_cpufreq_soc_data {
>   	u32 reg_dcvs_ctrl;
>   	u32 reg_freq_lut;
>   	u32 reg_volt_lut;
> +	u32 reg_intr_clr;
>   	u32 reg_current_vote;
>   	u32 reg_perf_state;
>   	u8 lut_row_size;
> @@ -360,6 +363,10 @@ static irqreturn_t qcom_lmh_dcvs_handle_irq(int irq, void *data)
>   	disable_irq_nosync(c_data->throttle_irq);
>   	schedule_delayed_work(&c_data->throttle_work, 0);
>   
> +	if (c_data->soc_data->reg_intr_clr)
> +		writel_relaxed(GT_IRQ_STATUS,
> +			       c_data->base + c_data->soc_data->reg_intr_clr);
> +
>   	return IRQ_HANDLED;
>   }
>   
> @@ -379,6 +386,7 @@ static const struct qcom_cpufreq_soc_data epss_soc_data = {
>   	.reg_dcvs_ctrl = 0xb0,
>   	.reg_freq_lut = 0x100,
>   	.reg_volt_lut = 0x200,
> +	.reg_intr_clr = 0x308,
>   	.reg_perf_state = 0x320,
>   	.lut_row_size = 4,
>   };

I believe this fix of a critical bug is intended to be included into the next
release, could you please share your review? Thank you in advance.

--
Best wishes,
Vladimir

Re: [PATCH v3] cpufreq: qcom-cpufreq-hw: Clear dcvs interrupts

[Viresh Kumar]

On 20-04-22, 22:42, Vladimir Zapolskiy wrote:
> I believe this fix of a critical bug is intended to be included into the next
> release, could you please share your review? Thank you in advance.

I was waiting for Bjorn to Ack it.

-- 
viresh

Re: [PATCH v3] cpufreq: qcom-cpufreq-hw: Clear dcvs interrupts

[Viresh Kumar]

On 07-04-22, 23:09, Vladimir Zapolskiy wrote:
> It's noted that dcvs interrupts are not self-clearing, thus an interrupt
> handler runs constantly, which leads to a severe regression in runtime.
> To fix the problem an explicit write to clear interrupt register is
> required, note that on OSM platforms the register may not be present.
> 
> Fixes: 275157b367f4 ("cpufreq: qcom-cpufreq-hw: Add dcvs interrupt support")
> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>

Applied. Thanks.

-- 
viresh