| Message ID | 20220407200919.3054189-1-vladimir.zapolskiy@linaro.org (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | [v3] cpufreq: qcom-cpufreq-hw: Clear dcvs interrupts | expand |
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
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.
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.
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, };
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(+)