* [PATCH for next v1 0/2] gpio: few clean up patches to replace spin_lock_irqsave with spin_lock @ 2021-02-08 8:57 Luo Jiaxing 2021-02-08 8:57 ` [PATCH for next v1 1/2] gpio: omap: Replace raw_spin_lock_irqsave with raw_spin_lock in omap_gpio_irq_handler() Luo Jiaxing 2021-02-08 8:57 ` [PATCH for next v1 2/2] gpio: grgpio: Replace spin_lock_irqsave with spin_lock in grgpio_irq_handler() Luo Jiaxing 0 siblings, 2 replies; 8+ messages in thread From: Luo Jiaxing @ 2021-02-08 8:57 UTC (permalink / raw) To: linus.walleij, andy.shevchenko, andriy.shevchenko, grygorii.strashko, ssantosh, khilman Cc: linux-gpio, linux-kernel, linuxarm There is no need to use API with _irqsave in hard IRQ handler, So replace those with spin_lock. Luo Jiaxing (2): gpio: omap: Replace raw_spin_lock_irqsave with raw_spin_lock in omap_gpio_irq_handler() gpio: grgpio: Replace spin_lock_irqsave with spin_lock in grgpio_irq_handler() drivers/gpio/gpio-grgpio.c | 5 ++--- drivers/gpio/gpio-omap.c | 15 ++++++--------- 2 files changed, 8 insertions(+), 12 deletions(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH for next v1 1/2] gpio: omap: Replace raw_spin_lock_irqsave with raw_spin_lock in omap_gpio_irq_handler() 2021-02-08 8:57 [PATCH for next v1 0/2] gpio: few clean up patches to replace spin_lock_irqsave with spin_lock Luo Jiaxing @ 2021-02-08 8:57 ` Luo Jiaxing 2021-02-08 8:57 ` [PATCH for next v1 2/2] gpio: grgpio: Replace spin_lock_irqsave with spin_lock in grgpio_irq_handler() Luo Jiaxing 1 sibling, 0 replies; 8+ messages in thread From: Luo Jiaxing @ 2021-02-08 8:57 UTC (permalink / raw) To: linus.walleij, andy.shevchenko, andriy.shevchenko, grygorii.strashko, ssantosh, khilman Cc: linux-gpio, linux-kernel, linuxarm There is no need to use API with _irqsave in omap_gpio_irq_handler(), because it already be in a irq-disabled context. Signed-off-by: Luo Jiaxing <luojiaxing@huawei.com> --- drivers/gpio/gpio-omap.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c index 41952bb..dc8bbf4 100644 --- a/drivers/gpio/gpio-omap.c +++ b/drivers/gpio/gpio-omap.c @@ -560,8 +560,6 @@ static irqreturn_t omap_gpio_irq_handler(int irq, void *gpiobank) u32 enabled, isr, edge; unsigned int bit; struct gpio_bank *bank = gpiobank; - unsigned long wa_lock_flags; - unsigned long lock_flags; isr_reg = bank->base + bank->regs->irqstatus; if (WARN_ON(!isr_reg)) @@ -572,7 +570,7 @@ static irqreturn_t omap_gpio_irq_handler(int irq, void *gpiobank) return IRQ_NONE; while (1) { - raw_spin_lock_irqsave(&bank->lock, lock_flags); + raw_spin_lock(&bank->lock); enabled = omap_get_gpio_irqbank_mask(bank); isr = readl_relaxed(isr_reg) & enabled; @@ -586,7 +584,7 @@ static irqreturn_t omap_gpio_irq_handler(int irq, void *gpiobank) if (edge) omap_clear_gpio_irqbank(bank, edge); - raw_spin_unlock_irqrestore(&bank->lock, lock_flags); + raw_spin_unlock(&bank->lock); if (!isr) break; @@ -595,7 +593,7 @@ static irqreturn_t omap_gpio_irq_handler(int irq, void *gpiobank) bit = __ffs(isr); isr &= ~(BIT(bit)); - raw_spin_lock_irqsave(&bank->lock, lock_flags); + raw_spin_lock(&bank->lock); /* * Some chips can't respond to both rising and falling * at the same time. If this irq was requested with @@ -606,15 +604,14 @@ static irqreturn_t omap_gpio_irq_handler(int irq, void *gpiobank) if (bank->toggle_mask & (BIT(bit))) omap_toggle_gpio_edge_triggering(bank, bit); - raw_spin_unlock_irqrestore(&bank->lock, lock_flags); + raw_spin_unlock(&bank->lock); - raw_spin_lock_irqsave(&bank->wa_lock, wa_lock_flags); + raw_spin_lock(&bank->wa_lock); generic_handle_irq(irq_find_mapping(bank->chip.irq.domain, bit)); - raw_spin_unlock_irqrestore(&bank->wa_lock, - wa_lock_flags); + raw_spin_unlock(&bank->wa_lock); } } exit: -- 2.7.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH for next v1 2/2] gpio: grgpio: Replace spin_lock_irqsave with spin_lock in grgpio_irq_handler() 2021-02-08 8:57 [PATCH for next v1 0/2] gpio: few clean up patches to replace spin_lock_irqsave with spin_lock Luo Jiaxing 2021-02-08 8:57 ` [PATCH for next v1 1/2] gpio: omap: Replace raw_spin_lock_irqsave with raw_spin_lock in omap_gpio_irq_handler() Luo Jiaxing @ 2021-02-08 8:57 ` Luo Jiaxing 2021-02-10 10:52 ` Andy Shevchenko 1 sibling, 1 reply; 8+ messages in thread From: Luo Jiaxing @ 2021-02-08 8:57 UTC (permalink / raw) To: linus.walleij, andy.shevchenko, andriy.shevchenko, grygorii.strashko, ssantosh, khilman Cc: linux-gpio, linux-kernel, linuxarm There is no need to use API with _irqsave in grgpio_irq_handler(), because it already be in a irq-disabled context. Signed-off-by: Luo Jiaxing <luojiaxing@huawei.com> --- drivers/gpio/gpio-grgpio.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/gpio/gpio-grgpio.c b/drivers/gpio/gpio-grgpio.c index f954359..fa5aa31 100644 --- a/drivers/gpio/gpio-grgpio.c +++ b/drivers/gpio/gpio-grgpio.c @@ -195,11 +195,10 @@ static irqreturn_t grgpio_irq_handler(int irq, void *dev) { struct grgpio_priv *priv = dev; int ngpio = priv->gc.ngpio; - unsigned long flags; int i; int match = 0; - spin_lock_irqsave(&priv->gc.bgpio_lock, flags); + spin_lock(&priv->gc.bgpio_lock); /* * For each gpio line, call its interrupt handler if it its underlying @@ -215,7 +214,7 @@ static irqreturn_t grgpio_irq_handler(int irq, void *dev) } } - spin_unlock_irqrestore(&priv->gc.bgpio_lock, flags); + spin_unlock(&priv->gc.bgpio_lock); if (!match) dev_warn(priv->dev, "No gpio line matched irq %d\n", irq); -- 2.7.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH for next v1 2/2] gpio: grgpio: Replace spin_lock_irqsave with spin_lock in grgpio_irq_handler() 2021-02-08 8:57 ` [PATCH for next v1 2/2] gpio: grgpio: Replace spin_lock_irqsave with spin_lock in grgpio_irq_handler() Luo Jiaxing @ 2021-02-10 10:52 ` Andy Shevchenko 0 siblings, 0 replies; 8+ messages in thread From: Andy Shevchenko @ 2021-02-10 10:52 UTC (permalink / raw) To: Luo Jiaxing Cc: Linus Walleij, Andy Shevchenko, Grygorii Strashko, Santosh Shilimkar, Kevin Hilman, open list:GPIO SUBSYSTEM, Linux Kernel Mailing List, linuxarm On Mon, Feb 8, 2021 at 10:58 AM Luo Jiaxing <luojiaxing@huawei.com> wrote: > > There is no need to use API with _irqsave in grgpio_irq_handler(), > because it already be in a irq-disabled context. It seems you haven't read the code. The handler here is shared. And lock there is about something else that we discussed in the cover letter. Moreover, the driver is quite outdated and code inside is horrible according to the modern APIs / standards. I would rather remove the driver completely. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH for next v1 0/2] gpio: few clean up patches to replace spin_lock_irqsave with spin_lock @ 2021-02-08 8:56 Luo Jiaxing 2021-02-08 8:56 ` [PATCH for next v1 1/2] gpio: omap: Replace raw_spin_lock_irqsave with raw_spin_lock in omap_gpio_irq_handler() Luo Jiaxing 0 siblings, 1 reply; 8+ messages in thread From: Luo Jiaxing @ 2021-02-08 8:56 UTC (permalink / raw) To: linus.walleij, andy.shevchenko, andriy.shevchenko, grygorii.strashko, ssantosh, khilman Cc: linux-gpio, linux-kernel, linuxarm There is no need to use API with _irqsave in hard IRQ handler, So replace those with spin_lock. Luo Jiaxing (2): gpio: omap: Replace raw_spin_lock_irqsave with raw_spin_lock in omap_gpio_irq_handler() gpio: grgpio: Replace spin_lock_irqsave with spin_lock in grgpio_irq_handler() drivers/gpio/gpio-grgpio.c | 5 ++--- drivers/gpio/gpio-omap.c | 15 ++++++--------- 2 files changed, 8 insertions(+), 12 deletions(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH for next v1 1/2] gpio: omap: Replace raw_spin_lock_irqsave with raw_spin_lock in omap_gpio_irq_handler() 2021-02-08 8:56 [PATCH for next v1 0/2] gpio: few clean up patches to replace spin_lock_irqsave with spin_lock Luo Jiaxing @ 2021-02-08 8:56 ` Luo Jiaxing 2021-02-11 18:14 ` Grygorii Strashko 0 siblings, 1 reply; 8+ messages in thread From: Luo Jiaxing @ 2021-02-08 8:56 UTC (permalink / raw) To: linus.walleij, andy.shevchenko, andriy.shevchenko, grygorii.strashko, ssantosh, khilman Cc: linux-gpio, linux-kernel, linuxarm There is no need to use API with _irqsave in omap_gpio_irq_handler(), because it already be in a irq-disabled context. Signed-off-by: Luo Jiaxing <luojiaxing@huawei.com> --- drivers/gpio/gpio-omap.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c index 41952bb..dc8bbf4 100644 --- a/drivers/gpio/gpio-omap.c +++ b/drivers/gpio/gpio-omap.c @@ -560,8 +560,6 @@ static irqreturn_t omap_gpio_irq_handler(int irq, void *gpiobank) u32 enabled, isr, edge; unsigned int bit; struct gpio_bank *bank = gpiobank; - unsigned long wa_lock_flags; - unsigned long lock_flags; isr_reg = bank->base + bank->regs->irqstatus; if (WARN_ON(!isr_reg)) @@ -572,7 +570,7 @@ static irqreturn_t omap_gpio_irq_handler(int irq, void *gpiobank) return IRQ_NONE; while (1) { - raw_spin_lock_irqsave(&bank->lock, lock_flags); + raw_spin_lock(&bank->lock); enabled = omap_get_gpio_irqbank_mask(bank); isr = readl_relaxed(isr_reg) & enabled; @@ -586,7 +584,7 @@ static irqreturn_t omap_gpio_irq_handler(int irq, void *gpiobank) if (edge) omap_clear_gpio_irqbank(bank, edge); - raw_spin_unlock_irqrestore(&bank->lock, lock_flags); + raw_spin_unlock(&bank->lock); if (!isr) break; @@ -595,7 +593,7 @@ static irqreturn_t omap_gpio_irq_handler(int irq, void *gpiobank) bit = __ffs(isr); isr &= ~(BIT(bit)); - raw_spin_lock_irqsave(&bank->lock, lock_flags); + raw_spin_lock(&bank->lock); /* * Some chips can't respond to both rising and falling * at the same time. If this irq was requested with @@ -606,15 +604,14 @@ static irqreturn_t omap_gpio_irq_handler(int irq, void *gpiobank) if (bank->toggle_mask & (BIT(bit))) omap_toggle_gpio_edge_triggering(bank, bit); - raw_spin_unlock_irqrestore(&bank->lock, lock_flags); + raw_spin_unlock(&bank->lock); - raw_spin_lock_irqsave(&bank->wa_lock, wa_lock_flags); + raw_spin_lock(&bank->wa_lock); generic_handle_irq(irq_find_mapping(bank->chip.irq.domain, bit)); - raw_spin_unlock_irqrestore(&bank->wa_lock, - wa_lock_flags); + raw_spin_unlock(&bank->wa_lock); } } exit: -- 2.7.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH for next v1 1/2] gpio: omap: Replace raw_spin_lock_irqsave with raw_spin_lock in omap_gpio_irq_handler() 2021-02-08 8:56 ` [PATCH for next v1 1/2] gpio: omap: Replace raw_spin_lock_irqsave with raw_spin_lock in omap_gpio_irq_handler() Luo Jiaxing @ 2021-02-11 18:14 ` Grygorii Strashko 2021-02-11 19:39 ` Arnd Bergmann 0 siblings, 1 reply; 8+ messages in thread From: Grygorii Strashko @ 2021-02-11 18:14 UTC (permalink / raw) To: Luo Jiaxing, linus.walleij, andy.shevchenko, andriy.shevchenko, ssantosh, khilman Cc: linux-gpio, linux-kernel, linuxarm 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. > > Signed-off-by: Luo Jiaxing <luojiaxing@huawei.com> > --- > drivers/gpio/gpio-omap.c | 15 ++++++--------- > 1 file changed, 6 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c > index 41952bb..dc8bbf4 100644 > --- a/drivers/gpio/gpio-omap.c > +++ b/drivers/gpio/gpio-omap.c > @@ -560,8 +560,6 @@ static irqreturn_t omap_gpio_irq_handler(int irq, void *gpiobank) > u32 enabled, isr, edge; > unsigned int bit; > struct gpio_bank *bank = gpiobank; > - unsigned long wa_lock_flags; > - unsigned long lock_flags; > > isr_reg = bank->base + bank->regs->irqstatus; > if (WARN_ON(!isr_reg)) > @@ -572,7 +570,7 @@ static irqreturn_t omap_gpio_irq_handler(int irq, void *gpiobank) > return IRQ_NONE; > > while (1) { > - raw_spin_lock_irqsave(&bank->lock, lock_flags); > + raw_spin_lock(&bank->lock); > > enabled = omap_get_gpio_irqbank_mask(bank); > isr = readl_relaxed(isr_reg) & enabled; > @@ -586,7 +584,7 @@ static irqreturn_t omap_gpio_irq_handler(int irq, void *gpiobank) > if (edge) > omap_clear_gpio_irqbank(bank, edge); > > - raw_spin_unlock_irqrestore(&bank->lock, lock_flags); > + raw_spin_unlock(&bank->lock); > > if (!isr) > break; > @@ -595,7 +593,7 @@ static irqreturn_t omap_gpio_irq_handler(int irq, void *gpiobank) > bit = __ffs(isr); > isr &= ~(BIT(bit)); > > - raw_spin_lock_irqsave(&bank->lock, lock_flags); > + raw_spin_lock(&bank->lock); > /* > * Some chips can't respond to both rising and falling > * at the same time. If this irq was requested with > @@ -606,15 +604,14 @@ static irqreturn_t omap_gpio_irq_handler(int irq, void *gpiobank) > if (bank->toggle_mask & (BIT(bit))) > omap_toggle_gpio_edge_triggering(bank, bit); > > - raw_spin_unlock_irqrestore(&bank->lock, lock_flags); > + raw_spin_unlock(&bank->lock); > > - raw_spin_lock_irqsave(&bank->wa_lock, wa_lock_flags); > + raw_spin_lock(&bank->wa_lock); > > generic_handle_irq(irq_find_mapping(bank->chip.irq.domain, > bit)); > > - raw_spin_unlock_irqrestore(&bank->wa_lock, > - wa_lock_flags); > + raw_spin_unlock(&bank->wa_lock); > } > } > exit: > NACK. Who said that this is always hard IRQ handler? What about RT-kernel or boot with "threadirqs"? -- Best regards, grygorii ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH for next v1 1/2] gpio: omap: Replace raw_spin_lock_irqsave with raw_spin_lock in omap_gpio_irq_handler() 2021-02-11 18:14 ` Grygorii Strashko @ 2021-02-11 19:39 ` Arnd Bergmann 2021-02-11 20:16 ` Grygorii Strashko 0 siblings, 1 reply; 8+ messages in thread From: Arnd Bergmann @ 2021-02-11 19:39 UTC (permalink / raw) To: Grygorii Strashko Cc: Luo Jiaxing, Linus Walleij, Andy Shevchenko, Andy Shevchenko, Santosh Shilimkar, Kevin Hilman, open list:GPIO SUBSYSTEM, linux-kernel, linuxarm 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. Arnd ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH for next v1 1/2] gpio: omap: Replace raw_spin_lock_irqsave with raw_spin_lock in omap_gpio_irq_handler() 2021-02-11 19:39 ` Arnd Bergmann @ 2021-02-11 20:16 ` Grygorii Strashko 0 siblings, 0 replies; 8+ messages in thread From: Grygorii Strashko @ 2021-02-11 20:16 UTC (permalink / raw) To: Arnd Bergmann Cc: Luo Jiaxing, Linus Walleij, Andy Shevchenko, Andy Shevchenko, Santosh Shilimkar, Kevin Hilman, open list:GPIO SUBSYSTEM, linux-kernel, linuxarm 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. -- Best regards, grygorii ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-02-11 20:18 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-02-08 8:57 [PATCH for next v1 0/2] gpio: few clean up patches to replace spin_lock_irqsave with spin_lock Luo Jiaxing 2021-02-08 8:57 ` [PATCH for next v1 1/2] gpio: omap: Replace raw_spin_lock_irqsave with raw_spin_lock in omap_gpio_irq_handler() Luo Jiaxing 2021-02-08 8:57 ` [PATCH for next v1 2/2] gpio: grgpio: Replace spin_lock_irqsave with spin_lock in grgpio_irq_handler() Luo Jiaxing 2021-02-10 10:52 ` Andy Shevchenko -- strict thread matches above, loose matches on Subject: below -- 2021-02-08 8:56 [PATCH for next v1 0/2] gpio: few clean up patches to replace spin_lock_irqsave with spin_lock Luo Jiaxing 2021-02-08 8:56 ` [PATCH for next v1 1/2] gpio: omap: Replace raw_spin_lock_irqsave with raw_spin_lock in omap_gpio_irq_handler() Luo Jiaxing 2021-02-11 18:14 ` Grygorii Strashko 2021-02-11 19:39 ` Arnd Bergmann 2021-02-11 20:16 ` Grygorii Strashko
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).