> -----Original Message----- > From: Grygorii Strashko [mailto:[email protected]] > Sent: Friday, February 12, 2021 9:17 AM > To: Arnd Bergmann <[email protected]> > Cc: luojiaxing <[email protected]>; Linus Walleij > <[email protected]>; Andy Shevchenko <[email protected]>; Andy > Shevchenko <[email protected]>; Santosh Shilimkar > <[email protected]>; Kevin Hilman <[email protected]>; open list:GPIO > SUBSYSTEM <[email protected]>, [email protected] > <[email protected]>; [email protected] > 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 > > <[email protected]> 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
