Messages in this thread | | | Date | Thu, 2 Mar 2023 11:11:26 +0100 | Subject | Re: [PATCH v4 16/19] thermal/drivers/tegra: Remove unneeded lock when setting a trip point | From | Daniel Lezcano <> |
| |
On 02/03/2023 10:45, Thierry Reding wrote:
[ ... ]
>>>> + /* >>>> + * Disable the interrupt so set_trips() can not be called >>>> + * while we are setting up the register >>>> + * TSENSOR_SENSOR0_CONFIG1. With this we close a potential >>>> + * race window where we are setting up the TH2 and the >>>> + * temperature hits TH1 resulting to an update of the >>>> + * TSENSOR_SENSOR0_CONFIG1 register in the ISR. >>>> + */ >>>> + disable_irq(irq); >>>> + >>>> for (i = 0; i < ARRAY_SIZE(ts->ch); i++) { >>>> err = tegra_tsensor_enable_hw_channel(ts, i); >>>> if (err) >>>> return err; >>>> } >>>> + enable_irq(irq); >>> >>> Instead of disabling and reenabling the interrupt, could we simply move >>> the channel enabling code a couple of lines above, before the IRQ >>> request call? If enabling the channels were to trigger an interrupt, it >>> should get triggered right after requesting the IRQ. >> >> Won't we have a spurious interrupt if that situation happen ? > > It wouldn't be a spurious interrupt, but rather something that we want > to react to. It's ultimately the same result as your patch. In your > variant we basically request the IRQ (which automatically enables it), > then immediately disable it, enable the HW channels and then reenable > the interrupt. If enabling the HW channels were to trigger an interrupt, > that interrupt would be raised immediately after enable_irq() as well, > as far as I can tell.
I see, thanks for the clarification
-- Daniel
-- <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog
| |