-----Original Message----- From: Grygorii Strashko [mailto:grygorii.strashko@ti.com] Sent: Friday, February 12, 2021 9:17 AM To: Arnd Bergmann arnd@kernel.org Cc: luojiaxing luojiaxing@huawei.com; Linus Walleij linus.walleij@linaro.org; Andy Shevchenko andy.shevchenko@gmail.com; Andy Shevchenko andriy.shevchenko@linux.intel.com; Santosh Shilimkar ssantosh@kernel.org; Kevin Hilman khilman@kernel.org; open list:GPIO SUBSYSTEM linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org linux-kernel@vger.kernel.org; linuxarm@openeuler.org Subject: [Linuxarm] Re: [PATCH for next v1 1/2] gpio: omap: Replace raw_spin_lock_irqsave with raw_spin_lock in omap_gpio_irq_handler()
On 11/02/2021 21:39, Arnd Bergmann wrote:
On Thu, Feb 11, 2021 at 7:25 PM Grygorii Strashko grygorii.strashko@ti.com wrote:
On 08/02/2021 10:56, Luo Jiaxing wrote:
There is no need to use API with _irqsave in omap_gpio_irq_handler(), because it already be in a irq-disabled context.
NACK. Who said that this is always hard IRQ handler? What about RT-kernel or boot with "threadirqs"?
In those cases, the interrupt handler is just a normal thread, so the preempt_disable() that is implied by raw_spin_lock() is sufficient.
Disabling interrupts inside of an interrupt handler is always incorrect, the patch looks like a useful cleanup to me, if only for readability.
Note. there is also generic_handle_irq() call inside.
So generic_handle_irq() is not safe to run in thread thus requires an interrupt-disabled environment to run? If so, I'd rather this irqsave moved into generic_handle_irq() rather than asking everyone calling it to do irqsave.
On the other hand, the author changed a couple of spin_lock_irqsave to spin_lock, if only this one is incorrect, it seems it is worth a new version to fix this.
-- Best regards, grygorii
Thanks Barry