GPINTEN register contains information about GPIOs with enabled
interrupts no need to check other GPIOs for changes.
Signed-off-by: Arturas Moskvinas <[email protected]>
---
drivers/pinctrl/pinctrl-mcp23s08.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/pinctrl/pinctrl-mcp23s08.c b/drivers/pinctrl/pinctrl-mcp23s08.c
index 4551575e4e7d..bfa933284e84 100644
--- a/drivers/pinctrl/pinctrl-mcp23s08.c
+++ b/drivers/pinctrl/pinctrl-mcp23s08.c
@@ -375,7 +375,8 @@ mcp23s08_direction_output(struct gpio_chip *chip, unsigned offset, int value)
static irqreturn_t mcp23s08_irq(int irq, void *data)
{
struct mcp23s08 *mcp = data;
- int intcap, intcon, intf, i, gpio, gpio_orig, intcap_mask, defval;
+ int intcap, intcon, intf, i, gpio, gpio_orig, intcap_mask, defval, gpinten;
+ unsigned long int enabled_interrupts;
unsigned int child_irq;
bool intf_set, intcap_changed, gpio_bit_changed,
defval_changed, gpio_set;
@@ -395,6 +396,10 @@ static irqreturn_t mcp23s08_irq(int irq, void *data)
if (mcp_read(mcp, MCP_INTCON, &intcon))
goto unlock;
+ if (mcp_read(mcp, MCP_GPINTEN, &gpinten))
+ goto unlock;
+ enabled_interrupts = gpinten;
+
if (mcp_read(mcp, MCP_DEFVAL, &defval))
goto unlock;
@@ -410,9 +415,10 @@ static irqreturn_t mcp23s08_irq(int irq, void *data)
"intcap 0x%04X intf 0x%04X gpio_orig 0x%04X gpio 0x%04X\n",
intcap, intf, gpio_orig, gpio);
- for (i = 0; i < mcp->chip.ngpio; i++) {
- /* We must check all of the inputs on the chip,
- * otherwise we may not notice a change on >=2 pins.
+ for_each_set_bit(i, &enabled_interrupts, mcp->chip.ngpio) {
+ /* We must check all of the inputs with enabled interrupts
+ * on the chip, otherwise we may not notice a change
+ * on >=2 pins.
*
* On at least the mcp23s17, INTCAP is only updated
* one byte at a time(INTCAPA and INTCAPB are
base-commit: 41bccc98fb7931d63d03f326a746ac4d429c1dd3
--
2.43.0
On Tue, Jan 30, 2024 at 8:37 AM Arturas Moskvinas
<[email protected]> wrote:
> GPINTEN register contains information about GPIOs with enabled
> interrupts no need to check other GPIOs for changes.
>
> Signed-off-by: Arturas Moskvinas <[email protected]>
This driver has a lot of users, so adding some reviewers.
https://lore.kernel.org/linux-gpio/[email protected]/
Yours,
Linus Walleij
On Tue, Jan 30, 2024 at 09:37:10AM +0200, Arturas Moskvinas wrote:
> GPINTEN register contains information about GPIOs with enabled
> interrupts no need to check other GPIOs for changes.
..
> + if (mcp_read(mcp, MCP_GPINTEN, &gpinten))
> + goto unlock;
> + enabled_interrupts = gpinten;
Move this line to be...
..
> - for (i = 0; i < mcp->chip.ngpio; i++) {
> - /* We must check all of the inputs on the chip,
> - * otherwise we may not notice a change on >=2 pins.
..just here (w/o any blank line in between).
> + for_each_set_bit(i, &enabled_interrupts, mcp->chip.ngpio) {
..
> + /* We must check all of the inputs with enabled interrupts
> + * on the chip, otherwise we may not notice a change
> + * on >=2 pins.
Missing space after =. But better to spell in proper English, i.e.
"...great than or equal to 2 pins."
> *
> * On at least the mcp23s17, INTCAP is only updated
/*
* Use proper multi-line
* comment style as depicted
* in this example.
*/
--
With Best Regards,
Andy Shevchenko
On Thu, Feb 01, 2024 at 02:19:38PM +0200, Arturas Moskvinas wrote:
> On Thu, Feb 1, 2024 at 2:03 PM Andy Shevchenko <[email protected]>
> wrote:
..
> > > + /* We must check all of the inputs with enabled interrupts
> > > + * on the chip, otherwise we may not notice a change
> > > + * on >=2 pins.
> >
> > Missing space after =. But better to spell in proper English, i.e.
> > "...great than or equal to 2 pins."
> >
> + /*
> + * We must check all of the inputs with enabled interrupts
> + * on the chip, otherwise we may not notice a change
> + * on more than one pin.
>
> Does this sound better?
LGTM, thanks.
--
With Best Regards,
Andy Shevchenko