Messages in this thread | | | Subject | Re: [PATCH 2/5] thermal: qcom: Add support for LMh driver | From | Thara Gopinath <> | Date | Fri, 18 Jun 2021 17:53:11 -0400 |
| |
On 6/18/21 1:54 PM, Bjorn Andersson wrote: > On Mon 14 Jun 20:38 CDT 2021, Thara Gopinath wrote: >> On 6/14/21 4:53 PM, Bjorn Andersson wrote: >>> On Tue 08 Jun 17:29 CDT 2021, Thara Gopinath wrote: >>>> diff --git a/drivers/thermal/qcom/Makefile b/drivers/thermal/qcom/Makefile > [..] >>>> +static irqreturn_t lmh_handle_irq(int hw_irq, void *data) >>>> +{ >>>> + struct lmh_hw_data *lmh_data = data; >>>> + int irq = irq_find_mapping(lmh_data->domain, 0); >>>> + >>>> + /* >>>> + * Disable interrupt and call the cpufreq driver to handle the interrupt >>>> + * cpufreq will enable the interrupt once finished processing. >>>> + */ >>>> + disable_irq_nosync(lmh_data->irq); >>> >>> The contract between this driver's disabling of the IRQ and the >>> cpufreq-hw driver's enabling it when we're done polling does worry me. >>> >>> In the case of EPSS, don't we disable the interrupt during the polling >>> there as well? If that's the case wouldn't it be better to implement >>> irq_chip->irq_disable and have the cpufreq-hw driver do the disable in >>> both cases? >> >> Yes. You are right. In case of EPSS, the cpufreq-hw will have to disable the >> interrupt. I did think of the approach you suggested here. My only issue is >> that we will dispatch the interrupt to cpufreq-hw without it disabling it >> and hence the interrupt could fire again, right ? >> > > Does it fire again before you INTR_CLK it?
You mean clear it ? I couldn't reproduce it either way. I did not try the irq_chip->irq_disable either. So I will give it a try and if my tests pass , I will post it.
> > Regards, > Bjorn >
-- Warm Regards Thara (She/Her/Hers)
| |