[PATCH v7 0/4] gpio: mxc: misc bug fixes

[PATCH v7 0/4] gpio: mxc: misc bug fixes

[Peng Fan (OSS)]

This is a subset focusing on bug fixes, that separated from thread [1]

  1. Fix a race where the chained IRQ handler is installed before the IRQ
     domain, generic IRQ chip, and port list entry are ready.  An interrupt
     arriving in that window dereferences port->domain while still NULL
     (mx3 path) or walks an incomplete list (mx2 path).  Move the handler
     install after all dependencies are set up.

  2. Fix non-atomic read-modify-write on both_edges.  gpio_set_irq_type()
     can run concurrently for different pins on the same port (different
     irq descriptors = different locks), so the old |=/&= on a u32 is a
     data race on SMP.  Switch to set_bit()/clear_bit()/test_bit() on
     unsigned long.

  3. Fix non-atomic read-modify-write on wakeup_pads, same class of bug
     as both_edges.  Switch to assign_bit() on unsigned long.  Also fix
     the bit being updated unconditionally even when irq_set_irq_wake()
     fails.

  4. Fix a race where devm_gpiochip_add_data() makes the chip visible
     to consumers before the IRQ domain and generic irq chip are set up.
     A to_irq() call in that window hits a NULL port->domain.  Move
     gpiochip registration after IRQ setup is complete.

[1] https://lore.kernel.org/linux-gpio/20261011-gpio-mxc-cleanup-v6-0-cb47ce2ad758@nxp.com/T/#m49d6a5a14a64774eda299bb711c8d316681e652b

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
Changes in v7:
- Only keep four bug fixes, split cleanup patches into a separate series.
- New patch 4: fix race between gpiochip registration and IRQ domain
  setup by moving devm_gpiochip_add_data() after the IRQ domain and
  generic irq chip are fully initialized.
- Patch 3: rewrite commit message to clarify the writer-vs-writer race
  and explain that reader-side atomicity is not needed (Andy Shevchenko).
- Link to v6: https://patch.msgid.link/20261011-gpio-mxc-cleanup-v6-0-cb47ce2ad758@nxp.com

Changes in v6:
- Patch 9: use retain_and_null_ptr() instead of no_free_ptr() to disarm
  the pm_runtime cleanup guard, as the return value is unused (Frank Li).
- Minor commit message rewording for patches 10-12.
- Collect R-b from Frank Li.
- Link to v5: https://patch.msgid.link/20261010-gpio-mxc-cleanup-v5-0-1e59e6ddfa00@nxp.com

Changes in v5:
- New patch 2: fix both_edges race using atomic bitops (set_bit/
  clear_bit/test_bit), prompted by review feedback on the BIT() patch.
- Patch 3 (wakeup_pads fix): split the irq_set_irq_wake()
  consolidation and for_each_set_bit() conversion into separate cleanup
  patches (4-5) for bisectability.
- Patch 7 (pad wakeup hwdata): renamed MXC_GPIO_IS_IMX8QM to
  MXC_GPIO_FALL_EDGE_WAKEUP_BROKEN to describe the property, not the
  SoC.
- Patch 9 (new): use __free cleanup guard for pm_runtime_get_noresume()
  balance instead of devm_pm_runtime_get_noresume(), which would cause a
  usage counter underflow on unbind due to the double put (devres +
  success-path pm_runtime_put_autosuspend).
- Patch 10 (devres): dropped devm_pm_runtime_get_noresume(); the __free
  guard from patch 9 handles the usage counter balance.  Fixed stale
  &pdev->dev references that should use the local dev variable.
- Patch 11 (dev_err_probe): fixed &pdev->dev references to use dev.
- Reordered series: moved "use local dev variable" before the devres
  and dev_err_probe patches so the local variable is available.
- Link to v4: https://patch.msgid.link/20261007-gpio-mxc-cleanup-v4-0-c9f5a8b50df9@nxp.com

Changes in v4:
- Just a resend of V3 with below changes.
  Add R-b from Linus for patch 10.
  Update commit for patch 2 and drop empty line and separate
  for_each_set_bit changes into a new patch Per Andy.
- Link to v3: https://lore.kernel.org/all/20261006-gpio-mxc-cleanup-v3-0-129f93302a90@nxp.com/

Changes in v3:
- Patch ("gpio: mxc: convert probe error handling to devres"):
  Replaced pm_runtime_get_noresume() with
  devm_pm_runtime_get_noresume() to fix a PM usage counter leak on
  probe error paths. The non-devm pm_runtime_get_noresume() was not
  balanced by any devres action, so any probe failure after the PM
  block (e.g. -EPROBE_DEFER from devm_gpiochip_add_data()) would
  leave usage_count permanently elevated, preventing runtime suspend
  on re-probe.
  Dropped the explicit pm_runtime_put_noidle() on
  devm_pm_runtime_set_active_enabled() failure - the devres
  registered by devm_pm_runtime_get_noresume() handles the balance
  automatically during probe unwind.
- Patch ("gpio: mxc: use local dev variable"):
  Keep of_device_is_compatible(), only focus on switching &pdev->dev to
  dev.
- Link to v2: https://patch.msgid.link/20261005-gpio-mxc-cleanup-v2-0-bdc3afbb35e2@nxp.com

Changes in V2:
- Reworked compatible-string caching (V1 patch 3) from probe-time bools
  in mxc_gpio_port to static hwdata flags with per-compatible data
  instances.  Split into two patches: one introducing the flags scheme
  with MXC_GPIO_HW_DATA_COMMON macro for power_off (patch 3), one
  extending it to pad_wakeup and imx8qm (patch 4).  (Frank, bot review)
- Replaced devm_pm_runtime_get_noresume() + pm_runtime_set_active() +
  devm_pm_runtime_enable() with devm_pm_runtime_set_active_enabled().
  Keep plain pm_runtime_get_noresume() (non-devm) for the probe-scoped
  reference to avoid usage_count underflow on unbind.  (Frank, bot review)
- Replaced irq_domain_create_legacy() + devm_add_action_or_reset() with
  devm_irq_domain_instantiate().  Squashed with the PM runtime devres
  conversion (V1 patches 4+5) into a single patch (patch 5), since the
  goto labels cannot be removed until both resources are devres-managed.
- Split dev_err_probe() conversion into its own patch (patch 6) for
  bisectability — patch 5 uses bare returns with correct error values.
- Folded gpio_set_wake_irq() assign_bit simplification (V1 patch 9) into
  the wakeup_pads fix (patch 2) where it belongs.
- Used 0x3U (unsigned) in MXC_ICR_MASK() to avoid implementation-defined
  behavior when shifting by 30 bits.
- Replaced linux/of.h with linux/property.h to match the
  of_device_is_compatible() → device_is_compatible() API change (patch 7).
- Added return-value checks for devm_pm_runtime_set_active_enabled().
- Fixed stale error code returns after devm_irq_alloc_descs() and
  devm_irq_domain_instantiate() in the devres conversion patch.
- Link to v1: https://patch.msgid.link/20261003-gpio-mxc-cleanup-v1-0-dad728ce27f2@nxp.com

---
Peng Fan (4):
      gpio: mxc: fix race between chained IRQ handler install and probe completion
      gpio: mxc: fix both_edges bit operations
      gpio: mxc: fix wakeup_pads bit operations
      gpio: mxc: fix race between gpiochip registration and IRQ domain setup

 drivers/gpio/gpio-mxc.c | 36 ++++++++++++++++++++++--------------
 1 file changed, 22 insertions(+), 14 deletions(-)
---
base-commit: ef2a32fc6a059affb32848b8beb27a05d79b9856
change-id: 20261003-gpio-mxc-cleanup-e49cc626c51e

Best regards,
--  
Peng Fan <peng.fan@nxp.com>

[PATCH v7 1/4] gpio: mxc: fix race between chained IRQ handler install and probe completion

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

mxc_update_irq_chained_handler() is called before the IRQ domain, the
generic IRQ chip, and the port list entry are set up. If an interrupt
arrives in that window:

 - mx3_gpio_irq_handler() calls generic_handle_domain_irq() with
   port->domain still NULL.
 - mx2_gpio_irq_handler() walks mxc_gpio_ports, but the port has not
   been added to the list yet.

Additionally, if any of the subsequent probe steps
(gpio_generic_chip_init(), devm_gpiochip_add_data(),
irq_domain_create_legacy(), or mxc_gpio_init_gc()) fail, the error
paths never unregister the chained handler, leaving a dangling handler
that points at freed memory.

Move the handler installation after all its dependencies are ready and
after list_add_tail(), so the handler is never live while the data
structures it touches are incomplete, and is never installed if probe
fails.

Fixes: 5f6d1998adeb ("gpio: mxc: release the parent IRQ in runtime suspend")
Assisted-by: LLM
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/gpio/gpio-mxc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
index 7e2690d92df6..e05f276a50e8 100644
--- a/drivers/gpio/gpio-mxc.c
+++ b/drivers/gpio/gpio-mxc.c
@@ -474,8 +474,6 @@ static int mxc_gpio_probe(struct platform_device *pdev)
 	} else
 		port->mx_irq_handler = mx3_gpio_irq_handler;
 
-	mxc_update_irq_chained_handler(port, true);
-
 	config.dev = &pdev->dev;
 	config.sz = 4;
 	config.dat = port->base + GPIO_PSR;
@@ -525,6 +523,8 @@ static int mxc_gpio_probe(struct platform_device *pdev)
 
 	list_add_tail(&port->node, &mxc_gpio_ports);
 
+	mxc_update_irq_chained_handler(port, true);
+
 	platform_set_drvdata(pdev, port);
 	pm_runtime_put_autosuspend(&pdev->dev);
 

-- 
2.51.0

Re: [PATCH v7 1/4] gpio: mxc: fix race between chained IRQ handler install and probe completion

[Linus Walleij]

On Tue, Oct 13, 2026 at 8:03 AM Peng Fan (OSS) <peng.fan@oss.nxp.com> wrote:
> From: Peng Fan <peng.fan@nxp.com>
>
> mxc_update_irq_chained_handler() is called before the IRQ domain, the
> generic IRQ chip, and the port list entry are set up. If an interrupt
> arrives in that window:
>
>  - mx3_gpio_irq_handler() calls generic_handle_domain_irq() with
>    port->domain still NULL.
>  - mx2_gpio_irq_handler() walks mxc_gpio_ports, but the port has not
>    been added to the list yet.
>
> Additionally, if any of the subsequent probe steps
> (gpio_generic_chip_init(), devm_gpiochip_add_data(),
> irq_domain_create_legacy(), or mxc_gpio_init_gc()) fail, the error
> paths never unregister the chained handler, leaving a dangling handler
> that points at freed memory.
>
> Move the handler installation after all its dependencies are ready and
> after list_add_tail(), so the handler is never live while the data
> structures it touches are incomplete, and is never installed if probe
> fails.
>
> Fixes: 5f6d1998adeb ("gpio: mxc: release the parent IRQ in runtime suspend")
> Assisted-by: LLM
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>

Reviewed-by: Linus Walleij <linusw@kernel.org>

Yours,
Linus Walleij

[PATCH v7 2/4] gpio: mxc: fix both_edges bit operations

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

gpio_set_irq_type() can be called concurrently for different pins on
the same port.  The IRQ core serializes per irq descriptor, but
different pins have different descriptors, so on SMP systems two
concurrent gpio_set_irq_type() calls for pins on the same port run
without mutual exclusion.

The read-modify-write of port->both_edges is performed outside the
port-wide scoped_guard(gpio_generic_lock_irqsave) section, so
concurrent updates for different pins can overwrite each other,
causing missed interrupts or stuck IRQ lines on dual-edge triggered
pins.

The original uniprocessor MXC SoCs were not affected; the race
became reachable on multi-core i.MX7 and i.MX8 SoCs that reuse this
driver.

Convert both_edges from u32 to unsigned long and use set_bit(),
clear_bit(), and test_bit() to make the modifications atomic.

Fixes: 910862ec092c ("mxc: emulate GPIO interrupt on both-edges")
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/gpio/gpio-mxc.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
index e05f276a50e8..7e17d7668980 100644
--- a/drivers/gpio/gpio-mxc.c
+++ b/drivers/gpio/gpio-mxc.c
@@ -7,6 +7,7 @@
 // Authors: Daniel Mack, Juergen Beisert.
 // Copyright (C) 2004-2010 Freescale Semiconductor, Inc. All Rights Reserved.
 
+#include <linux/bitops.h>
 #include <linux/cleanup.h>
 #include <linux/clk.h>
 #include <linux/err.h>
@@ -68,7 +69,7 @@ struct mxc_gpio_port {
 	struct irq_domain *domain;
 	struct gpio_generic_chip gen_gc;
 	struct device *dev;
-	u32 both_edges;
+	unsigned long both_edges;
 	struct mxc_gpio_reg_saved gpio_saved_reg;
 	bool power_off;
 	u32 wakeup_pads;
@@ -168,7 +169,7 @@ static int gpio_set_irq_type(struct irq_data *d, u32 type)
 	int edge;
 	void __iomem *reg = port->base;
 
-	port->both_edges &= ~(1 << gpio_idx);
+	clear_bit(gpio_idx, &port->both_edges);
 	switch (type) {
 	case IRQ_TYPE_EDGE_RISING:
 		edge = GPIO_INT_RISE_EDGE;
@@ -188,7 +189,7 @@ static int gpio_set_irq_type(struct irq_data *d, u32 type)
 				edge = GPIO_INT_HIGH_LEV;
 				pr_debug("mxc: set GPIO %d to high trigger\n", gpio_idx);
 			}
-			port->both_edges |= 1 << gpio_idx;
+			set_bit(gpio_idx, &port->both_edges);
 		}
 		break;
 	case IRQ_TYPE_LEVEL_LOW:
@@ -259,7 +260,7 @@ static void mxc_gpio_irq_handler(struct mxc_gpio_port *port, u32 irq_stat)
 	while (irq_stat != 0) {
 		int irqoffset = fls(irq_stat) - 1;
 
-		if (port->both_edges & (1 << irqoffset))
+		if (test_bit(irqoffset, &port->both_edges))
 			mxc_flip_edge(port, irqoffset);
 
 		generic_handle_domain_irq(port->domain, irqoffset);

-- 
2.51.0

Re: [PATCH v7 2/4] gpio: mxc: fix both_edges bit operations

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] gpio: state corruption on error path in gpio_set_irq_type()
--

commit 3d5872f9d45fab5b313d49ded5e6a9aacb019069
Author: Peng Fan <peng.fan@nxp.com>

gpio: mxc: fix both_edges bit operations

This commit converts both_edges from u32 to unsigned long and uses atomic
bitops to prevent concurrent updates from overwriting each other.

> diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
> index e05f276a50e8d..7e17d7668980d 100644
> --- a/drivers/gpio/gpio-mxc.c
> +++ b/drivers/gpio/gpio-mxc.c

[ ... ]

> @@ -168,7 +169,7 @@ static int gpio_set_irq_type(struct irq_data *d, u32 type)
>  	int edge;
>  	void __iomem *reg = port->base;
>  
> -	port->both_edges &= ~(1 << gpio_idx);
> +	clear_bit(gpio_idx, &port->both_edges);

[Severity: Medium]
If a caller requests an unsupported IRQ type, the switch statement later in
this function falls through to the default case and returns -EINVAL:

	switch (type) {
	...
	default:
		return -EINVAL;
	}

Could clearing both_edges early here leave the software state desynchronized
from the hardware on this error path?

If the hardware was previously configured for dual-edge interrupts, test_bit()
in the IRQ handler will now return false, failing to flip the edge and
potentially leading to a stuck IRQ line or interrupt storm.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261013-gpio-mxc-cleanup-v7-0-c4d769587d3e@nxp.com?part=2

Re: [PATCH v7 2/4] gpio: mxc: fix both_edges bit operations

[Peng Fan]

On Tue, Oct 13, 2026 at 06:26:06AM +0000, sashiko-bot@kernel.org wrote:
>Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>- [Medium] gpio: state corruption on error path in gpio_set_irq_type()
>--
>
>commit 3d5872f9d45fab5b313d49ded5e6a9aacb019069
>Author: Peng Fan <peng.fan@nxp.com>
>
>gpio: mxc: fix both_edges bit operations
>
>This commit converts both_edges from u32 to unsigned long and uses atomic
>bitops to prevent concurrent updates from overwriting each other.
>
>> diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
>> index e05f276a50e8d..7e17d7668980d 100644
>> --- a/drivers/gpio/gpio-mxc.c
>> +++ b/drivers/gpio/gpio-mxc.c
>
>[ ... ]
>
>> @@ -168,7 +169,7 @@ static int gpio_set_irq_type(struct irq_data *d, u32 type)
>>  	int edge;
>>  	void __iomem *reg = port->base;
>>  
>> -	port->both_edges &= ~(1 << gpio_idx);
>> +	clear_bit(gpio_idx, &port->both_edges);
>
>[Severity: Medium]
>If a caller requests an unsupported IRQ type, the switch statement later in
>this function falls through to the default case and returns -EINVAL:

This is false alarm. __irq_set_trigger() already guard, there is no chance
to 'return -EINVAL' here. The 'return -EINVAL' is just defensive code.

Regards
Peng

>
>	switch (type) {
>	...
>	default:
>		return -EINVAL;
>	}
>
>Could clearing both_edges early here leave the software state desynchronized
>from the hardware on this error path?
>
>If the hardware was previously configured for dual-edge interrupts, test_bit()
>in the IRQ handler will now return false, failing to flip the edge and
>potentially leading to a stuck IRQ line or interrupt storm.
>
>-- 
>Sashiko AI review · https://sashiko.dev/#/patchset/20261013-gpio-mxc-cleanup-v7-0-c4d769587d3e@nxp.com?part=2
>

Re: [PATCH v7 2/4] gpio: mxc: fix both_edges bit operations

[Linus Walleij]

On Tue, Oct 13, 2026 at 8:04 AM Peng Fan (OSS) <peng.fan@oss.nxp.com> wrote:

> From: Peng Fan <peng.fan@nxp.com>
>
> gpio_set_irq_type() can be called concurrently for different pins on
> the same port.  The IRQ core serializes per irq descriptor, but
> different pins have different descriptors, so on SMP systems two
> concurrent gpio_set_irq_type() calls for pins on the same port run
> without mutual exclusion.
>
> The read-modify-write of port->both_edges is performed outside the
> port-wide scoped_guard(gpio_generic_lock_irqsave) section, so
> concurrent updates for different pins can overwrite each other,
> causing missed interrupts or stuck IRQ lines on dual-edge triggered
> pins.
>
> The original uniprocessor MXC SoCs were not affected; the race
> became reachable on multi-core i.MX7 and i.MX8 SoCs that reuse this
> driver.
>
> Convert both_edges from u32 to unsigned long and use set_bit(),
> clear_bit(), and test_bit() to make the modifications atomic.
>
> Fixes: 910862ec092c ("mxc: emulate GPIO interrupt on both-edges")
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>

Reviewed-by: Linus Walleij <linusw@kernel.org>

Yours,
Linus Walleij

[PATCH v7 3/4] gpio: mxc: fix wakeup_pads bit operations

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

gpio_set_wake_irq() can be called concurrently for different pins on
the same port. The writer-vs-writer race is the concern: two concurrent
gpio_set_wake_irq() calls for different pins on the same port both do
a non-atomic read-modify-write on the same wakeup_pads word. The old
|= and &= compile to separate load/modify/store instructions that can
interleave, losing one CPU's update.

assign_bit() uses an atomic RMW instruction, so concurrent writers to
different bits in the same word are safe.

The reader side (for_each_set_bit in noirq_suspend, wakeup_pads > 0)
runs in noirq context where no concurrent writer can execute, so
reader atomicity is not needed.

wakeup_pads is a u32, while assign_bit() operates on unsigned long
pointers. On 64-bit platforms, this causes an 8-byte read-modify-write
on a 4-byte field, corrupting the adjacent is_pad_wakeup. So change
wakeup_pads to unsigned long and reorder to avoid wasting words.

And fix the enable/disable path unconditionally sets/clears the
wakeup_pads bit even when enable_irq_wake()/disable_irq_wake() fails.
Only update the bit on success.

Fixes: f60c9eac54af ("gpio: mxc: enable pad wakeup on i.MX8x platforms")
Assisted-by: LLM
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/gpio/gpio-mxc.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
index 7e17d7668980..e3a80f17d041 100644
--- a/drivers/gpio/gpio-mxc.c
+++ b/drivers/gpio/gpio-mxc.c
@@ -72,8 +72,8 @@ struct mxc_gpio_port {
 	unsigned long both_edges;
 	struct mxc_gpio_reg_saved gpio_saved_reg;
 	bool power_off;
-	u32 wakeup_pads;
 	bool is_pad_wakeup;
+	unsigned long wakeup_pads;
 	u32 pad_type[32];
 	const struct mxc_gpio_hwdata *hwdata;
 };
@@ -331,16 +331,18 @@ static int gpio_set_wake_irq(struct irq_data *d, u32 enable)
 			ret = enable_irq_wake(port->irq_high);
 		else
 			ret = enable_irq_wake(port->irq);
-		port->wakeup_pads |= BIT(gpio_idx);
 	} else {
 		if (port->irq_high && (gpio_idx >= 16))
 			ret = disable_irq_wake(port->irq_high);
 		else
 			ret = disable_irq_wake(port->irq);
-		port->wakeup_pads &= ~BIT(gpio_idx);
 	}
+	if (ret)
+		return ret;
 
-	return ret;
+	assign_bit(gpio_idx, &port->wakeup_pads, enable);
+
+	return 0;
 }
 
 static int mxc_gpio_init_gc(struct mxc_gpio_port *port, int irq_base)

-- 
2.51.0

Re: [PATCH v7 3/4] gpio: mxc: fix wakeup_pads bit operations

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] gpio: mxc: missing for_each_set_bit hunk and sign-extension bug
--

commit 163a0b84d8efa779538111b96ea2ce2543fbd9da
Author: Peng Fan <peng.fan@nxp.com>

    gpio: mxc: fix wakeup_pads bit operations
    
    gpio_set_wake_irq() can be called concurrently for different pins on
    the same port. The writer-vs-writer race is the concern: two concurrent
    gpio_set_wake_irq() calls for different pins on the same port both do
    a non-atomic read-modify-write on the same wakeup_pads word.

> diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
> index 7e17d7668980d..e3a80f17d0412 100644
> --- a/drivers/gpio/gpio-mxc.c
> +++ b/drivers/gpio/gpio-mxc.c
> @@ -72,8 +72,8 @@ struct mxc_gpio_port {
>  	unsigned long both_edges;
>  	struct mxc_gpio_reg_saved gpio_saved_reg;
>  	bool power_off;
> -	u32 wakeup_pads;
>  	bool is_pad_wakeup;
> +	unsigned long wakeup_pads;

[Severity: Medium]
The commit message states "The reader side (for_each_set_bit in
noirq_suspend, wakeup_pads > 0) runs in noirq context", but it appears
the hunk that changes this loop is missing from the patch.

Because wakeup_pads is now an unsigned long, the existing loop in
drivers/gpio/gpio-mxc.c:mxc_gpio_set_pad_wakeup() has a sign-extension issue:

	for (i = 0; i < 32; i++) {
		if ((port->wakeup_pads & (1 << i))) {
			...

On 64-bit platforms, when i == 31, the 32-bit signed expression (1 << 31)
evaluates to -2147483648. When bitwise ANDed with the newly widened
unsigned long, it sign-extends to 0xFFFFFFFF80000000UL, which inadvertently
masks against bits 32-63 as well.

Could the missing for_each_set_bit hunk be added to resolve this?

>  	u32 pad_type[32];
>  	const struct mxc_gpio_hwdata *hwdata;
>  };

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261013-gpio-mxc-cleanup-v7-0-c4d769587d3e@nxp.com?part=3

Re: [PATCH v7 3/4] gpio: mxc: fix wakeup_pads bit operations

[Peng Fan]

On Tue, Oct 13, 2026 at 06:35:07AM +0000, sashiko-bot@kernel.org wrote:
>Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>- [Medium] gpio: mxc: missing for_each_set_bit hunk and sign-extension bug
>--
>
>commit 163a0b84d8efa779538111b96ea2ce2543fbd9da
>Author: Peng Fan <peng.fan@nxp.com>
>
>    gpio: mxc: fix wakeup_pads bit operations
>    
>    gpio_set_wake_irq() can be called concurrently for different pins on
>    the same port. The writer-vs-writer race is the concern: two concurrent
>    gpio_set_wake_irq() calls for different pins on the same port both do
>    a non-atomic read-modify-write on the same wakeup_pads word.
>
>> diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
>> index 7e17d7668980d..e3a80f17d0412 100644
>> --- a/drivers/gpio/gpio-mxc.c
>> +++ b/drivers/gpio/gpio-mxc.c
>> @@ -72,8 +72,8 @@ struct mxc_gpio_port {
>>  	unsigned long both_edges;
>>  	struct mxc_gpio_reg_saved gpio_saved_reg;
>>  	bool power_off;
>> -	u32 wakeup_pads;
>>  	bool is_pad_wakeup;
>> +	unsigned long wakeup_pads;
>
>[Severity: Medium]
>The commit message states "The reader side (for_each_set_bit in
>noirq_suspend, wakeup_pads > 0) runs in noirq context", but it appears
>the hunk that changes this loop is missing from the patch.
>
>Because wakeup_pads is now an unsigned long, the existing loop in
>drivers/gpio/gpio-mxc.c:mxc_gpio_set_pad_wakeup() has a sign-extension issue:
>
>	for (i = 0; i < 32; i++) {
>		if ((port->wakeup_pads & (1 << i))) {

The code will not incur error. for_each_set_bit() patch will be in
the following up patchset after the fixes patchset.

Regards
Peng

>			...
>
>On 64-bit platforms, when i == 31, the 32-bit signed expression (1 << 31)
>evaluates to -2147483648. When bitwise ANDed with the newly widened
>unsigned long, it sign-extends to 0xFFFFFFFF80000000UL, which inadvertently
>masks against bits 32-63 as well.
>
>Could the missing for_each_set_bit hunk be added to resolve this?
>
>>  	u32 pad_type[32];
>>  	const struct mxc_gpio_hwdata *hwdata;
>>  };
>
>-- 
>Sashiko AI review · https://sashiko.dev/#/patchset/20261013-gpio-mxc-cleanup-v7-0-c4d769587d3e@nxp.com?part=3
>

Re: [PATCH v7 3/4] gpio: mxc: fix wakeup_pads bit operations

[Andy Shevchenko]

On Tue, Oct 13, 2026 at 9:04 AM Peng Fan (OSS) <peng.fan@oss.nxp.com> wrote:
>
> gpio_set_wake_irq() can be called concurrently for different pins on
> the same port. The writer-vs-writer race is the concern: two concurrent
> gpio_set_wake_irq() calls for different pins on the same port both do
> a non-atomic read-modify-write on the same wakeup_pads word. The old
> |= and &= compile to separate load/modify/store instructions that can
> interleave, losing one CPU's update.
>
> assign_bit() uses an atomic RMW instruction, so concurrent writers to
> different bits in the same word are safe.
>
> The reader side (for_each_set_bit in noirq_suspend, wakeup_pads > 0)

for_each_set_bit()

> runs in noirq context where no concurrent writer can execute, so
> reader atomicity is not needed.
>
> wakeup_pads is a u32, while assign_bit() operates on unsigned long
> pointers. On 64-bit platforms, this causes an 8-byte read-modify-write
> on a 4-byte field, corrupting the adjacent is_pad_wakeup. So change
> wakeup_pads to unsigned long and reorder to avoid wasting words.
>
> And fix the enable/disable path unconditionally sets/clears the
> wakeup_pads bit even when enable_irq_wake()/disable_irq_wake() fails.
> Only update the bit on success.

-- 
With Best Regards,
Andy Shevchenko

[PATCH v7 4/4] gpio: mxc: fix race between gpiochip registration and IRQ domain setup

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

devm_gpiochip_add_data() makes the GPIO chip visible to consumers
immediately. Since the chip sets .to_irq = mxc_gpio_to_irq, which calls
irq_find_mapping(port->domain, offset), any to_irq() call between
gpiochip registration and IRQ domain creation will operate on a NULL
domain.

Fix by moving devm_gpiochip_add_data() after the IRQ domain and generic
irq chip are fully initialized, ensuring port->domain and the irq chip
handlers are ready before the chip becomes visible to consumers.

Fixes: d37a65bb4663 ("gpio/mxc: Move Freescale MXC gpio driver to drivers/gpio")
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/gpio/gpio-mxc.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
index e3a80f17d041..32ded7b548fb 100644
--- a/drivers/gpio/gpio-mxc.c
+++ b/drivers/gpio/gpio-mxc.c
@@ -500,10 +500,6 @@ static int mxc_gpio_probe(struct platform_device *pdev)
 	else /* silence boot time warning */
 		port->gen_gc.gc.base = -1;
 
-	err = devm_gpiochip_add_data(&pdev->dev, &port->gen_gc.gc, port);
-	if (err)
-		goto out_bgio;
-
 	irq_base = devm_irq_alloc_descs(&pdev->dev, -1, 0, 32, numa_node_id());
 	if (irq_base < 0) {
 		err = irq_base;
@@ -524,6 +520,15 @@ static int mxc_gpio_probe(struct platform_device *pdev)
 	if (err < 0)
 		goto out_irqdomain_remove;
 
+	/*
+	 * Register the GPIO chip last: devm_gpiochip_add_data() makes the
+	 * chip visible to consumers immediately, so the IRQ domain and
+	 * generic irq chip must be ready before any to_irq() call.
+	 */
+	err = devm_gpiochip_add_data(&pdev->dev, &port->gen_gc.gc, port);
+	if (err)
+		goto out_irqdomain_remove;
+
 	list_add_tail(&port->node, &mxc_gpio_ports);
 
 	mxc_update_irq_chained_handler(port, true);

-- 
2.51.0

Re: [PATCH v7 4/4] gpio: mxc: fix race between gpiochip registration and IRQ domain setup

[Frank Li]

On Tue, Oct 13, 2026 at 02:01:12PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
>
> devm_gpiochip_add_data() makes the GPIO chip visible to consumers
> immediately. Since the chip sets .to_irq = mxc_gpio_to_irq, which calls
> irq_find_mapping(port->domain, offset), any to_irq() call between
> gpiochip registration and IRQ domain creation will operate on a NULL
> domain.
>
> Fix by moving devm_gpiochip_add_data() after the IRQ domain and generic
> irq chip are fully initialized, ensuring port->domain and the irq chip
> handlers are ready before the chip becomes visible to consumers.
>
> Fixes: d37a65bb4663 ("gpio/mxc: Move Freescale MXC gpio driver to drivers/gpio")
> Signed-off-by: Peng Fan <peng.fan@nxp.com>

Reviewed-by: Frank Li <Frank.Li@nxp.com>

> ---
>  drivers/gpio/gpio-mxc.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
> index e3a80f17d041..32ded7b548fb 100644
> --- a/drivers/gpio/gpio-mxc.c
> +++ b/drivers/gpio/gpio-mxc.c
> @@ -500,10 +500,6 @@ static int mxc_gpio_probe(struct platform_device *pdev)
>  	else /* silence boot time warning */
>  		port->gen_gc.gc.base = -1;
>
> -	err = devm_gpiochip_add_data(&pdev->dev, &port->gen_gc.gc, port);
> -	if (err)
> -		goto out_bgio;
> -
>  	irq_base = devm_irq_alloc_descs(&pdev->dev, -1, 0, 32, numa_node_id());
>  	if (irq_base < 0) {
>  		err = irq_base;
> @@ -524,6 +520,15 @@ static int mxc_gpio_probe(struct platform_device *pdev)
>  	if (err < 0)
>  		goto out_irqdomain_remove;
>
> +	/*
> +	 * Register the GPIO chip last: devm_gpiochip_add_data() makes the
> +	 * chip visible to consumers immediately, so the IRQ domain and
> +	 * generic irq chip must be ready before any to_irq() call.
> +	 */
> +	err = devm_gpiochip_add_data(&pdev->dev, &port->gen_gc.gc, port);
> +	if (err)
> +		goto out_irqdomain_remove;
> +
>  	list_add_tail(&port->node, &mxc_gpio_ports);
>
>  	mxc_update_irq_chained_handler(port, true);
>
> --
> 2.51.0
>
>

Re: [PATCH v7 0/4] gpio: mxc: misc bug fixes

[Andy Shevchenko]

On Tue, Oct 13, 2026 at 9:03 AM Peng Fan (OSS) <peng.fan@oss.nxp.com> wrote:
>
> This is a subset focusing on bug fixes, that separated from thread [1]
>
>   1. Fix a race where the chained IRQ handler is installed before the IRQ
>      domain, generic IRQ chip, and port list entry are ready.  An interrupt
>      arriving in that window dereferences port->domain while still NULL
>      (mx3 path) or walks an incomplete list (mx2 path).  Move the handler
>      install after all dependencies are set up.
>
>   2. Fix non-atomic read-modify-write on both_edges.  gpio_set_irq_type()
>      can run concurrently for different pins on the same port (different
>      irq descriptors = different locks), so the old |=/&= on a u32 is a
>      data race on SMP.  Switch to set_bit()/clear_bit()/test_bit() on
>      unsigned long.
>
>   3. Fix non-atomic read-modify-write on wakeup_pads, same class of bug
>      as both_edges.  Switch to assign_bit() on unsigned long.  Also fix
>      the bit being updated unconditionally even when irq_set_irq_wake()
>      fails.
>
>   4. Fix a race where devm_gpiochip_add_data() makes the chip visible
>      to consumers before the IRQ domain and generic irq chip are set up.
>      A to_irq() call in that window hits a NULL port->domain.  Move
>      gpiochip registration after IRQ setup is complete.

Reviewed-by: Andy Shevchenko <andy@kernel.org>
(no need to resend for that small issue in the commit message in one
of the patches)

> [1] https://lore.kernel.org/linux-gpio/20261011-gpio-mxc-cleanup-v6-0-cb47ce2ad758@nxp.com/T/#m49d6a5a14a64774eda299bb711c8d316681e652b



-- 
With Best Regards,
Andy Shevchenko