[PATCH for next v1 0/2] gpio: few clean up patches to replace spin_lock_irqsave with spin_lock

[PATCH for next v1 0/2] gpio: few clean up patches to replace spin_lock_irqsave with spin_lock

[Luo Jiaxing]

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

[PATCH for next v1 1/2] gpio: omap: Replace raw_spin_lock_irqsave with raw_spin_lock in omap_gpio_irq_handler()

[Luo Jiaxing]

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

[PATCH for next v1 2/2] gpio: grgpio: Replace spin_lock_irqsave with spin_lock in grgpio_irq_handler()

[Luo Jiaxing]

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

Re: [PATCH for next v1 2/2] gpio: grgpio: Replace spin_lock_irqsave with spin_lock in grgpio_irq_handler()

[Andy Shevchenko]

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

[PATCH for next v1 0/2] gpio: few clean up patches to replace spin_lock_irqsave with spin_lock

[Luo Jiaxing]

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