Thread (13 messages) 13 messages, 2 authors, 2026-01-26

Re: [PATCH 2/3] gpio: brcmstb: implement irq_mask_ack

From: Florian Fainelli <florian.fainelli@broadcom.com>
Date: 2026-01-22 19:17:33
Also in: linux-gpio, lkml


On 1/21/2026 11:36 PM, Andy Shevchenko wrote:
On Thu, Jan 22, 2026 at 3:06 AM Florian Fainelli
[off-list ref] wrote:
quoted
From: Doug Berger <opendmb@gmail.com>

The irq_mask_ack operation is slightly more efficient than doing
irq_mask and irq_ack separately.
I would refer to the callbacks as

.irq_mask()
.irq_ack()

et cetera.
Ack.
quoted
More importantly for this driver it bypasses the check of
irqd_irq_masked ensuring a previously masked but still active
interrupt gets remasked if unmasked at the hardware level. This
allows the driver to more efficiently unmask the wake capable
interrupts when quiescing without needing to enable the irqs
individually to clear the irqd_irq_masked state.
...
quoted
-// Copyright (C) 2015-2017 Broadcom
+// Copyright (C) 2015-2026 Broadcom
Shouldn't it be rather 2015-2017,2026 ? (In one case when I updated a
driver for Intel, I went via Git history to gather the info.)
Ack.
...
quoted
  static void brcmstb_gpio_set_imask(struct brcmstb_gpio_bank *bank,
-               unsigned int hwirq, bool enable)
+               unsigned int hwirq, bool enable, bool ack)
This type of interface is usually discouraged as it makes code harder
to read and follow. Since there are a lot of duplication, I recommend
to move the ack op to a separate helper.
Good point, knowing the order and what set in those parameters can be 
confusing.
...
quoted
-       gpio_generic_write_reg(&bank->chip,
-                              priv->reg_base + GIO_MASK(bank->id), imask);
+       if (ack)
+               gpio_generic_write_reg(&bank->chip,
+                                      priv->reg_base + GIO_MASK(bank->id),
+                                      imask);
Id est this piece...


quoted
+static void brcmstb_gpio_irq_mask_ack(struct irq_data *d)
+{
+       struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+       struct brcmstb_gpio_bank *bank = gpiochip_get_data(gc);
+
+       brcmstb_gpio_set_imask(bank, d->hwirq, false, true);
...and call it here explicitly (seems the only place for it, so it can
even be just moved here without an intermediate helper).
Will do, thanks!
-- 
Florian

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help