| Message ID | 20240130073710.10110-1-arturas.moskvinas@gmail.com |
|---|---|
| State | Accepted |
| Commit | 2c0aafdf4a7c9e6381bcaa7d8aa2e90e42b028d7 |
| Headers | show |
| Series | pinctrl: mcp23s08: Check only GPIOs which have interrupts enabled | expand |
On Tue, Jan 30, 2024 at 8:37 AM Arturas Moskvinas <arturas.moskvinas@gmail.com> wrote: > GPINTEN register contains information about GPIOs with enabled > interrupts no need to check other GPIOs for changes. > > Signed-off-by: Arturas Moskvinas <arturas.moskvinas@gmail.com> This driver has a lot of users, so adding some reviewers. https://lore.kernel.org/linux-gpio/20240130073710.10110-1-arturas.moskvinas@gmail.com/ 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. */
On Thu, Feb 01, 2024 at 02:19:38PM +0200, Arturas Moskvinas wrote: > On Thu, Feb 1, 2024 at 2:03 PM Andy Shevchenko <andriy.shevchenko@intel.com> > 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.
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
GPINTEN register contains information about GPIOs with enabled interrupts no need to check other GPIOs for changes. Signed-off-by: Arturas Moskvinas <arturas.moskvinas@gmail.com> --- drivers/pinctrl/pinctrl-mcp23s08.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) base-commit: 41bccc98fb7931d63d03f326a746ac4d429c1dd3