* [PATCH v5 01/13] gpio: mxc: fix race between chained IRQ handler install and probe completion
2026-10-09 18:05 [PATCH v5 00/13] gpio: mxc: bug fixes and probe cleanup Peng Fan (OSS)
@ 2026-10-09 18:05 ` Peng Fan (OSS)
2026-10-09 18:22 ` sashiko-bot
2026-10-09 18:29 ` Frank Li
2026-10-09 18:05 ` [PATCH v5 02/13] gpio: mxc: fix both_edges bit operations Peng Fan (OSS)
` (11 subsequent siblings)
12 siblings, 2 replies; 27+ messages in thread
From: Peng Fan (OSS) @ 2026-10-09 18:05 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski, Frank Li, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Shenwei Wang,
Andy Shevchenko
Cc: linux-gpio, imx, linux-arm-kernel, linux-kernel, Peng Fan
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
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
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v5 01/13] gpio: mxc: fix race between chained IRQ handler install and probe completion
2026-10-09 18:05 ` [PATCH v5 01/13] gpio: mxc: fix race between chained IRQ handler install and probe completion Peng Fan (OSS)
@ 2026-10-09 18:22 ` sashiko-bot
2026-10-09 18:29 ` Frank Li
1 sibling, 0 replies; 27+ messages in thread
From: sashiko-bot @ 2026-10-09 18:22 UTC (permalink / raw)
To: Peng Fan (OSS); +Cc: imx, linux-gpio, Frank.Li
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] gpio: mxc: lockless mxc_gpio_ports list race with shared IRQ handler
--
commit cfebba0884b34920e4ae09750fb2bbf30c42ba32
Author: Peng Fan <peng.fan@nxp.com>
gpio: mxc: fix race between chained IRQ handler install and probe completion
This commit delays installing the chained IRQ handler in mxc_gpio_probe()
until after the generic IRQ chip, port list entry, and IRQ domain are fully
setup. This ensures the handler does not execute while data structures are
incomplete and prevents leaving a dangling handler if probe fails.
> diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
> index 7e2690d92df6f..e05f276a50e8d 100644
> --- a/drivers/gpio/gpio-mxc.c
> +++ b/drivers/gpio/gpio-mxc.c
[ ... ]
> @@ -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);
> +
[Severity: High]
Does this still leave a race condition for shared IRQs?
On i.MX21/i.MX27, all GPIO ports share the same parent IRQ. When the first
port probes, it registers mx2_gpio_irq_handler for the shared IRQ, enabling
it.
When the second port probes, it calls list_add_tail() to add itself to
mxc_gpio_ports. If an interrupt arrives concurrently on the already-active
first port, mx2_gpio_irq_handler() iterates over mxc_gpio_ports locklessly in
hardirq context.
Since list_add_tail() uses non-atomic pointer updates without compiler or
memory barriers (unlike list_add_rcu), could the active IRQ handler traverse a
corrupted list if it preempts the update, dereferencing invalid memory?
While moving the handler installation prevents races for the first port, for
shared IRQs the handler is already live. This leaves the lockless list
mutation race unresolved.
> platform_set_drvdata(pdev, port);
> pm_runtime_put_autosuspend(&pdev->dev);
>
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261010-gpio-mxc-cleanup-v5-0-1e59e6ddfa00@nxp.com?part=1
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v5 01/13] gpio: mxc: fix race between chained IRQ handler install and probe completion
2026-10-09 18:05 ` [PATCH v5 01/13] gpio: mxc: fix race between chained IRQ handler install and probe completion Peng Fan (OSS)
2026-10-09 18:22 ` sashiko-bot
@ 2026-10-09 18:29 ` Frank Li
1 sibling, 0 replies; 27+ messages in thread
From: Frank Li @ 2026-10-09 18:29 UTC (permalink / raw)
To: Peng Fan (OSS)
Cc: Linus Walleij, Bartosz Golaszewski, Frank Li, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Shenwei Wang,
Andy Shevchenko, linux-gpio, imx, linux-arm-kernel, linux-kernel,
Peng Fan
On Sat, Oct 10, 2026 at 02:05:22AM +0800, Peng Fan (OSS) 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
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
Reviewed-by: Frank Li <Frank.Li@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
>
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v5 02/13] gpio: mxc: fix both_edges bit operations
2026-10-09 18:05 [PATCH v5 00/13] gpio: mxc: bug fixes and probe cleanup Peng Fan (OSS)
2026-10-09 18:05 ` [PATCH v5 01/13] gpio: mxc: fix race between chained IRQ handler install and probe completion Peng Fan (OSS)
@ 2026-10-09 18:05 ` Peng Fan (OSS)
2026-10-09 18:33 ` Frank Li
2026-10-09 18:05 ` [PATCH v5 03/13] gpio: mxc: fix wakeup_pads " Peng Fan (OSS)
` (10 subsequent siblings)
12 siblings, 1 reply; 27+ messages in thread
From: Peng Fan (OSS) @ 2026-10-09 18:05 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski, Frank Li, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Shenwei Wang,
Andy Shevchenko
Cc: linux-gpio, imx, linux-arm-kernel, linux-kernel, Peng Fan
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")
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
drivers/gpio/gpio-mxc.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
index e05f276a50e8..2f83a0f076bf 100644
--- a/drivers/gpio/gpio-mxc.c
+++ b/drivers/gpio/gpio-mxc.c
@@ -68,7 +68,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 +168,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 +188,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 +259,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
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v5 02/13] gpio: mxc: fix both_edges bit operations
2026-10-09 18:05 ` [PATCH v5 02/13] gpio: mxc: fix both_edges bit operations Peng Fan (OSS)
@ 2026-10-09 18:33 ` Frank Li
0 siblings, 0 replies; 27+ messages in thread
From: Frank Li @ 2026-10-09 18:33 UTC (permalink / raw)
To: Peng Fan (OSS)
Cc: Linus Walleij, Bartosz Golaszewski, Frank Li, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Shenwei Wang,
Andy Shevchenko, linux-gpio, imx, linux-arm-kernel, linux-kernel,
Peng Fan
On Sat, Oct 10, 2026 at 02:05:23AM +0800, Peng Fan (OSS) 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")
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
Reviewed-by: Frank Li <Frank.Li@nxp.com>
> drivers/gpio/gpio-mxc.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
> index e05f276a50e8..2f83a0f076bf 100644
> --- a/drivers/gpio/gpio-mxc.c
> +++ b/drivers/gpio/gpio-mxc.c
> @@ -68,7 +68,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 +168,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 +188,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 +259,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
>
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v5 03/13] gpio: mxc: fix wakeup_pads bit operations
2026-10-09 18:05 [PATCH v5 00/13] gpio: mxc: bug fixes and probe cleanup Peng Fan (OSS)
2026-10-09 18:05 ` [PATCH v5 01/13] gpio: mxc: fix race between chained IRQ handler install and probe completion Peng Fan (OSS)
2026-10-09 18:05 ` [PATCH v5 02/13] gpio: mxc: fix both_edges bit operations Peng Fan (OSS)
@ 2026-10-09 18:05 ` Peng Fan (OSS)
2026-10-09 18:35 ` Frank Li
2026-10-09 18:05 ` [PATCH v5 04/13] gpio: mxc: simplify gpio_set_wake_irq Peng Fan (OSS)
` (9 subsequent siblings)
12 siblings, 1 reply; 27+ messages in thread
From: Peng Fan (OSS) @ 2026-10-09 18:05 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski, Frank Li, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Shenwei Wang,
Andy Shevchenko
Cc: linux-gpio, imx, linux-arm-kernel, linux-kernel, Peng Fan
From: Peng Fan <peng.fan@nxp.com>
gpio_set_wake_irq() can be called concurrently for different pins on
the same port, so use atomic bitops when modifying wakeup_pads.
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 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
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 2f83a0f076bf..29396135717a 100644
--- a/drivers/gpio/gpio-mxc.c
+++ b/drivers/gpio/gpio-mxc.c
@@ -71,8 +71,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;
};
@@ -330,16 +330,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
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v5 03/13] gpio: mxc: fix wakeup_pads bit operations
2026-10-09 18:05 ` [PATCH v5 03/13] gpio: mxc: fix wakeup_pads " Peng Fan (OSS)
@ 2026-10-09 18:35 ` Frank Li
0 siblings, 0 replies; 27+ messages in thread
From: Frank Li @ 2026-10-09 18:35 UTC (permalink / raw)
To: Peng Fan (OSS)
Cc: Linus Walleij, Bartosz Golaszewski, Frank Li, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Shenwei Wang,
Andy Shevchenko, linux-gpio, imx, linux-arm-kernel, linux-kernel,
Peng Fan
On Sat, Oct 10, 2026 at 02:05:24AM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
>
> gpio_set_wake_irq() can be called concurrently for different pins on
> the same port, so use atomic bitops when modifying wakeup_pads.
>
> 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 the enable/disable path unconditionally sets/clears the wakeup_pads
Nit: And fix the enable/disable ....
Reviewed-by: Frank Li <Frank.Li@nxp.com>
> 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
> 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 2f83a0f076bf..29396135717a 100644
> --- a/drivers/gpio/gpio-mxc.c
> +++ b/drivers/gpio/gpio-mxc.c
> @@ -71,8 +71,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;
> };
> @@ -330,16 +330,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
>
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v5 04/13] gpio: mxc: simplify gpio_set_wake_irq
2026-10-09 18:05 [PATCH v5 00/13] gpio: mxc: bug fixes and probe cleanup Peng Fan (OSS)
` (2 preceding siblings ...)
2026-10-09 18:05 ` [PATCH v5 03/13] gpio: mxc: fix wakeup_pads " Peng Fan (OSS)
@ 2026-10-09 18:05 ` Peng Fan (OSS)
2026-10-09 18:39 ` Frank Li
2026-10-09 18:05 ` [PATCH v5 05/13] gpio: mxc: use for_each_set_bit() to iterate wakeup pads Peng Fan (OSS)
` (8 subsequent siblings)
12 siblings, 1 reply; 27+ messages in thread
From: Peng Fan (OSS) @ 2026-10-09 18:05 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski, Frank Li, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Shenwei Wang,
Andy Shevchenko
Cc: linux-gpio, imx, linux-arm-kernel, linux-kernel, Peng Fan
From: Peng Fan <peng.fan@nxp.com>
Simplify the logic by consolidating into a single irq_set_irq_wake()
call based on the enable parameter.
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
drivers/gpio/gpio-mxc.c | 15 ++++-----------
1 file changed, 4 insertions(+), 11 deletions(-)
diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
index 29396135717a..c921534aafc2 100644
--- a/drivers/gpio/gpio-mxc.c
+++ b/drivers/gpio/gpio-mxc.c
@@ -325,17 +325,10 @@ static int gpio_set_wake_irq(struct irq_data *d, u32 enable)
u32 gpio_idx = d->hwirq;
int ret;
- if (enable) {
- if (port->irq_high && (gpio_idx >= 16))
- ret = enable_irq_wake(port->irq_high);
- else
- ret = enable_irq_wake(port->irq);
- } else {
- if (port->irq_high && (gpio_idx >= 16))
- ret = disable_irq_wake(port->irq_high);
- else
- ret = disable_irq_wake(port->irq);
- }
+ if (port->irq_high && (gpio_idx >= 16))
+ ret = irq_set_irq_wake(port->irq_high, enable);
+ else
+ ret = irq_set_irq_wake(port->irq, enable);
if (ret)
return ret;
--
2.51.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v5 04/13] gpio: mxc: simplify gpio_set_wake_irq
2026-10-09 18:05 ` [PATCH v5 04/13] gpio: mxc: simplify gpio_set_wake_irq Peng Fan (OSS)
@ 2026-10-09 18:39 ` Frank Li
0 siblings, 0 replies; 27+ messages in thread
From: Frank Li @ 2026-10-09 18:39 UTC (permalink / raw)
To: Peng Fan (OSS)
Cc: Linus Walleij, Bartosz Golaszewski, Frank Li, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Shenwei Wang,
Andy Shevchenko, linux-gpio, imx, linux-arm-kernel, linux-kernel,
Peng Fan
On Sat, Oct 10, 2026 at 02:05:25AM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
subject: need () after gpio_set_wake_irq
gpio: mxc: Use irq_set_irq_wake() to simplify code
>
> Simplify the logic by consolidating into a single irq_set_irq_wake()
> call based on the enable parameter.
>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
> drivers/gpio/gpio-mxc.c | 15 ++++-----------
> 1 file changed, 4 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
> index 29396135717a..c921534aafc2 100644
> --- a/drivers/gpio/gpio-mxc.c
> +++ b/drivers/gpio/gpio-mxc.c
> @@ -325,17 +325,10 @@ static int gpio_set_wake_irq(struct irq_data *d, u32 enable)
> u32 gpio_idx = d->hwirq;
> int ret;
>
> - if (enable) {
> - if (port->irq_high && (gpio_idx >= 16))
> - ret = enable_irq_wake(port->irq_high);
> - else
> - ret = enable_irq_wake(port->irq);
> - } else {
> - if (port->irq_high && (gpio_idx >= 16))
> - ret = disable_irq_wake(port->irq_high);
> - else
> - ret = disable_irq_wake(port->irq);
> - }
> + if (port->irq_high && (gpio_idx >= 16))
> + ret = irq_set_irq_wake(port->irq_high, enable);
> + else
> + ret = irq_set_irq_wake(port->irq, enable);
> if (ret)
> return ret;
>
>
> --
> 2.51.0
>
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v5 05/13] gpio: mxc: use for_each_set_bit() to iterate wakeup pads
2026-10-09 18:05 [PATCH v5 00/13] gpio: mxc: bug fixes and probe cleanup Peng Fan (OSS)
` (3 preceding siblings ...)
2026-10-09 18:05 ` [PATCH v5 04/13] gpio: mxc: simplify gpio_set_wake_irq Peng Fan (OSS)
@ 2026-10-09 18:05 ` Peng Fan (OSS)
2026-10-09 18:05 ` [PATCH v5 06/13] gpio: mxc: replace of_device_is_compatible() with hwdata flags Peng Fan (OSS)
` (7 subsequent siblings)
12 siblings, 0 replies; 27+ messages in thread
From: Peng Fan (OSS) @ 2026-10-09 18:05 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski, Frank Li, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Shenwei Wang,
Andy Shevchenko
Cc: linux-gpio, imx, linux-arm-kernel, linux-kernel, Peng Fan
From: Peng Fan <peng.fan@nxp.com>
Use for_each_set_bit() to iterate over the enabled wakeup pads instead
of checking every bit individually.
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
drivers/gpio/gpio-mxc.c | 26 ++++++++++++--------------
1 file changed, 12 insertions(+), 14 deletions(-)
diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
index c921534aafc2..28578028b9ff 100644
--- a/drivers/gpio/gpio-mxc.c
+++ b/drivers/gpio/gpio-mxc.c
@@ -593,22 +593,20 @@ static bool mxc_gpio_set_pad_wakeup(struct mxc_gpio_port *port, bool enable)
IMX_SCU_WAKEUP_LOW_LVL, /* IRQ_TYPE_LEVEL_LOW */
};
- for (i = 0; i < 32; i++) {
- if ((port->wakeup_pads & (1 << i))) {
- type = port->pad_type[i];
- if (enable)
- config = pad_type_map[type];
- else
- config = IMX_SCU_WAKEUP_OFF;
-
- if (is_imx8qm && config == IMX_SCU_WAKEUP_FALL_EDGE) {
- dev_warn_once(port->dev,
- "No falling-edge support for wakeup on i.MX8QM\n");
- config = IMX_SCU_WAKEUP_OFF;
- }
+ for_each_set_bit(i, &port->wakeup_pads, 32) {
+ type = port->pad_type[i];
+ if (enable)
+ config = pad_type_map[type];
+ else
+ config = IMX_SCU_WAKEUP_OFF;
- ret |= mxc_gpio_generic_config(port, i, config);
+ if (is_imx8qm && config == IMX_SCU_WAKEUP_FALL_EDGE) {
+ dev_warn_once(port->dev,
+ "No falling-edge support for wakeup on i.MX8QM\n");
+ config = IMX_SCU_WAKEUP_OFF;
}
+
+ ret |= mxc_gpio_generic_config(port, i, config);
}
return ret;
--
2.51.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v5 06/13] gpio: mxc: replace of_device_is_compatible() with hwdata flags
2026-10-09 18:05 [PATCH v5 00/13] gpio: mxc: bug fixes and probe cleanup Peng Fan (OSS)
` (4 preceding siblings ...)
2026-10-09 18:05 ` [PATCH v5 05/13] gpio: mxc: use for_each_set_bit() to iterate wakeup pads Peng Fan (OSS)
@ 2026-10-09 18:05 ` Peng Fan (OSS)
2026-10-09 18:05 ` [PATCH v5 07/13] gpio: mxc: convert pad wakeup compatible checks to " Peng Fan (OSS)
` (6 subsequent siblings)
12 siblings, 0 replies; 27+ messages in thread
From: Peng Fan (OSS) @ 2026-10-09 18:05 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski, Frank Li, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Shenwei Wang,
Andy Shevchenko
Cc: linux-gpio, imx, linux-arm-kernel, linux-kernel, Peng Fan
From: Peng Fan <peng.fan@nxp.com>
Replace the runtime of_device_is_compatible() check for "fsl,imx7d-gpio"
with a flags field in mxc_gpio_hwdata to move the power-off capability
from a per-instance bool populated at probe time to static per-compatible
data.
Introduce MXC_GPIO_HAS_POWER_OFF and a dedicated imx7d_gpio_hwdata
instance that carries it, along with a mxc_gpio_has_power_off() helper
that replaces every former port->power_off test.
While at it, factor the register offsets shared by imx35 and imx7d into
a MXC_GPIO_HW_DATA_COMMON macro to avoid duplicating twelve identical
initializers.
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
drivers/gpio/gpio-mxc.c | 50 ++++++++++++++++++++++++++++++-------------------
1 file changed, 31 insertions(+), 19 deletions(-)
diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
index 28578028b9ff..1a35b23747b4 100644
--- a/drivers/gpio/gpio-mxc.c
+++ b/drivers/gpio/gpio-mxc.c
@@ -33,6 +33,8 @@
#define IMX_SCU_WAKEUP_RISE_EDGE 6
#define IMX_SCU_WAKEUP_HIGH_LVL 7
+#define MXC_GPIO_HAS_POWER_OFF BIT(0)
+
/* device type dependent stuff */
struct mxc_gpio_hwdata {
unsigned dr_reg;
@@ -47,6 +49,7 @@ struct mxc_gpio_hwdata {
unsigned high_level;
unsigned rise_edge;
unsigned fall_edge;
+ unsigned int flags;
};
struct mxc_gpio_reg_saved {
@@ -70,13 +73,26 @@ struct mxc_gpio_port {
struct device *dev;
unsigned long both_edges;
struct mxc_gpio_reg_saved gpio_saved_reg;
- bool power_off;
bool is_pad_wakeup;
unsigned long wakeup_pads;
u32 pad_type[32];
const struct mxc_gpio_hwdata *hwdata;
};
+#define MXC_GPIO_HW_DATA_COMMON \
+ .dr_reg = 0x00, \
+ .gdir_reg = 0x04, \
+ .psr_reg = 0x08, \
+ .icr1_reg = 0x0c, \
+ .icr2_reg = 0x10, \
+ .imr_reg = 0x14, \
+ .isr_reg = 0x18, \
+ .edge_sel_reg = 0x1c, \
+ .low_level = 0x00, \
+ .high_level = 0x01, \
+ .rise_edge = 0x02, \
+ .fall_edge = 0x03
+
static struct mxc_gpio_hwdata imx1_imx21_gpio_hwdata = {
.dr_reg = 0x1c,
.gdir_reg = 0x00,
@@ -108,20 +124,19 @@ static struct mxc_gpio_hwdata imx31_gpio_hwdata = {
};
static struct mxc_gpio_hwdata imx35_gpio_hwdata = {
- .dr_reg = 0x00,
- .gdir_reg = 0x04,
- .psr_reg = 0x08,
- .icr1_reg = 0x0c,
- .icr2_reg = 0x10,
- .imr_reg = 0x14,
- .isr_reg = 0x18,
- .edge_sel_reg = 0x1c,
- .low_level = 0x00,
- .high_level = 0x01,
- .rise_edge = 0x02,
- .fall_edge = 0x03,
+ MXC_GPIO_HW_DATA_COMMON,
+};
+
+static struct mxc_gpio_hwdata imx7d_gpio_hwdata = {
+ MXC_GPIO_HW_DATA_COMMON,
+ .flags = MXC_GPIO_HAS_POWER_OFF,
};
+static inline bool mxc_gpio_has_power_off(struct mxc_gpio_port *port)
+{
+ return port->hwdata->flags & MXC_GPIO_HAS_POWER_OFF;
+}
+
#define GPIO_DR (port->hwdata->dr_reg)
#define GPIO_GDIR (port->hwdata->gdir_reg)
#define GPIO_PSR (port->hwdata->psr_reg)
@@ -142,7 +157,7 @@ static const struct of_device_id mxc_gpio_dt_ids[] = {
{ .compatible = "fsl,imx21-gpio", .data = &imx1_imx21_gpio_hwdata },
{ .compatible = "fsl,imx31-gpio", .data = &imx31_gpio_hwdata },
{ .compatible = "fsl,imx35-gpio", .data = &imx35_gpio_hwdata },
- { .compatible = "fsl,imx7d-gpio", .data = &imx35_gpio_hwdata },
+ { .compatible = "fsl,imx7d-gpio", .data = &imx7d_gpio_hwdata },
{ .compatible = "fsl,imx8dxl-gpio", .data = &imx35_gpio_hwdata },
{ .compatible = "fsl,imx8qm-gpio", .data = &imx35_gpio_hwdata },
{ .compatible = "fsl,imx8qxp-gpio", .data = &imx35_gpio_hwdata },
@@ -447,9 +462,6 @@ static int mxc_gpio_probe(struct platform_device *pdev)
if (IS_ERR(port->clk))
return PTR_ERR(port->clk);
- if (of_device_is_compatible(np, "fsl,imx7d-gpio"))
- port->power_off = true;
-
pm_runtime_get_noresume(&pdev->dev);
pm_runtime_set_active(&pdev->dev);
pm_runtime_enable(&pdev->dev);
@@ -536,7 +548,7 @@ static int mxc_gpio_probe(struct platform_device *pdev)
static void mxc_gpio_save_regs(struct mxc_gpio_port *port)
{
- if (!port->power_off)
+ if (!mxc_gpio_has_power_off(port))
return;
port->gpio_saved_reg.icr1 = readl(port->base + GPIO_ICR1);
@@ -549,7 +561,7 @@ static void mxc_gpio_save_regs(struct mxc_gpio_port *port)
static void mxc_gpio_restore_regs(struct mxc_gpio_port *port)
{
- if (!port->power_off)
+ if (!mxc_gpio_has_power_off(port))
return;
writel(port->gpio_saved_reg.icr1, port->base + GPIO_ICR1);
--
2.51.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v5 07/13] gpio: mxc: convert pad wakeup compatible checks to hwdata flags
2026-10-09 18:05 [PATCH v5 00/13] gpio: mxc: bug fixes and probe cleanup Peng Fan (OSS)
` (5 preceding siblings ...)
2026-10-09 18:05 ` [PATCH v5 06/13] gpio: mxc: replace of_device_is_compatible() with hwdata flags Peng Fan (OSS)
@ 2026-10-09 18:05 ` Peng Fan (OSS)
2026-10-09 18:45 ` Frank Li
2026-10-09 18:05 ` [PATCH v5 08/13] gpio: mxc: use local dev variable Peng Fan (OSS)
` (5 subsequent siblings)
12 siblings, 1 reply; 27+ messages in thread
From: Peng Fan (OSS) @ 2026-10-09 18:05 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski, Frank Li, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Shenwei Wang,
Andy Shevchenko
Cc: linux-gpio, imx, linux-arm-kernel, linux-kernel, Peng Fan
From: Peng Fan <peng.fan@nxp.com>
mxc_gpio_generic_config() and mxc_gpio_set_pad_wakeup() call
of_device_is_compatible() on every invocation to determine pad wakeup
capability and i.MX8QM-specific behavior. These properties are
invariant for the lifetime of the device.
Extend the hwdata flags scheme introduced in the previous commit with
MXC_GPIO_HAS_PAD_WAKEUP and MXC_GPIO_FALL_EDGE_WAKEUP_BROKEN, adding
dedicated hwdata instances for imx8qm and imx8qxp (also used by imx8dxl).
This replaces the repeated device tree string comparisons in the
suspend/resume path with simple flag tests on static per-compatible
data.
While at it, clean up mxc_gpio_generic_config() to use a local ret
variable for clarity instead of the == 0 comparison.
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
drivers/gpio/gpio-mxc.c | 46 ++++++++++++++++++++++++++++++++++------------
1 file changed, 34 insertions(+), 12 deletions(-)
diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
index 1a35b23747b4..70fa139338c9 100644
--- a/drivers/gpio/gpio-mxc.c
+++ b/drivers/gpio/gpio-mxc.c
@@ -34,6 +34,8 @@
#define IMX_SCU_WAKEUP_HIGH_LVL 7
#define MXC_GPIO_HAS_POWER_OFF BIT(0)
+#define MXC_GPIO_HAS_PAD_WAKEUP BIT(1)
+#define MXC_GPIO_FALL_EDGE_WAKEUP_BROKEN BIT(2)
/* device type dependent stuff */
struct mxc_gpio_hwdata {
@@ -132,6 +134,26 @@ static struct mxc_gpio_hwdata imx7d_gpio_hwdata = {
.flags = MXC_GPIO_HAS_POWER_OFF,
};
+static struct mxc_gpio_hwdata imx8qm_gpio_hwdata = {
+ MXC_GPIO_HW_DATA_COMMON,
+ .flags = MXC_GPIO_FALL_EDGE_WAKEUP_BROKEN | MXC_GPIO_HAS_PAD_WAKEUP,
+};
+
+static struct mxc_gpio_hwdata imx8qxp_gpio_hwdata = {
+ MXC_GPIO_HW_DATA_COMMON,
+ .flags = MXC_GPIO_HAS_PAD_WAKEUP,
+};
+
+static inline bool mxc_gpio_fall_edge_wakeup_broken(struct mxc_gpio_port *port)
+{
+ return port->hwdata->flags & MXC_GPIO_FALL_EDGE_WAKEUP_BROKEN;
+}
+
+static inline bool mxc_gpio_has_pad_wakeup(struct mxc_gpio_port *port)
+{
+ return port->hwdata->flags & MXC_GPIO_HAS_PAD_WAKEUP;
+}
+
static inline bool mxc_gpio_has_power_off(struct mxc_gpio_port *port)
{
return port->hwdata->flags & MXC_GPIO_HAS_POWER_OFF;
@@ -158,9 +180,9 @@ static const struct of_device_id mxc_gpio_dt_ids[] = {
{ .compatible = "fsl,imx31-gpio", .data = &imx31_gpio_hwdata },
{ .compatible = "fsl,imx35-gpio", .data = &imx35_gpio_hwdata },
{ .compatible = "fsl,imx7d-gpio", .data = &imx7d_gpio_hwdata },
- { .compatible = "fsl,imx8dxl-gpio", .data = &imx35_gpio_hwdata },
- { .compatible = "fsl,imx8qm-gpio", .data = &imx35_gpio_hwdata },
- { .compatible = "fsl,imx8qxp-gpio", .data = &imx35_gpio_hwdata },
+ { .compatible = "fsl,imx8dxl-gpio", .data = &imx8qxp_gpio_hwdata },
+ { .compatible = "fsl,imx8qm-gpio", .data = &imx8qm_gpio_hwdata },
+ { .compatible = "fsl,imx8qxp-gpio", .data = &imx8qxp_gpio_hwdata },
{ /* sentinel */ }
};
MODULE_DEVICE_TABLE(of, mxc_gpio_dt_ids);
@@ -575,15 +597,16 @@ static void mxc_gpio_restore_regs(struct mxc_gpio_port *port)
static bool mxc_gpio_generic_config(struct mxc_gpio_port *port,
unsigned int offset, unsigned long conf)
{
- struct device_node *np = port->dev->of_node;
+ int ret;
+
+ if (!mxc_gpio_has_pad_wakeup(port))
+ return false;
- if (of_device_is_compatible(np, "fsl,imx8dxl-gpio") ||
- of_device_is_compatible(np, "fsl,imx8qxp-gpio") ||
- of_device_is_compatible(np, "fsl,imx8qm-gpio"))
- return (gpiochip_generic_config(&port->gen_gc.gc,
- offset, conf) == 0);
+ ret = gpiochip_generic_config(&port->gen_gc.gc, offset, conf);
+ if (ret)
+ return false;
- return false;
+ return true;
}
static bool mxc_gpio_set_pad_wakeup(struct mxc_gpio_port *port, bool enable)
@@ -591,7 +614,6 @@ static bool mxc_gpio_set_pad_wakeup(struct mxc_gpio_port *port, bool enable)
unsigned long config;
bool ret = false;
int i, type;
- bool is_imx8qm = of_device_is_compatible(port->dev->of_node, "fsl,imx8qm-gpio");
static const u32 pad_type_map[] = {
IMX_SCU_WAKEUP_OFF, /* 0 */
@@ -612,7 +634,7 @@ static bool mxc_gpio_set_pad_wakeup(struct mxc_gpio_port *port, bool enable)
else
config = IMX_SCU_WAKEUP_OFF;
- if (is_imx8qm && config == IMX_SCU_WAKEUP_FALL_EDGE) {
+ if (mxc_gpio_fall_edge_wakeup_broken(port) && config == IMX_SCU_WAKEUP_FALL_EDGE) {
dev_warn_once(port->dev,
"No falling-edge support for wakeup on i.MX8QM\n");
config = IMX_SCU_WAKEUP_OFF;
--
2.51.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v5 07/13] gpio: mxc: convert pad wakeup compatible checks to hwdata flags
2026-10-09 18:05 ` [PATCH v5 07/13] gpio: mxc: convert pad wakeup compatible checks to " Peng Fan (OSS)
@ 2026-10-09 18:45 ` Frank Li
0 siblings, 0 replies; 27+ messages in thread
From: Frank Li @ 2026-10-09 18:45 UTC (permalink / raw)
To: Peng Fan (OSS)
Cc: Linus Walleij, Bartosz Golaszewski, Frank Li, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Shenwei Wang,
Andy Shevchenko, linux-gpio, imx, linux-arm-kernel, linux-kernel,
Peng Fan
On Sat, Oct 10, 2026 at 02:05:28AM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
>
> mxc_gpio_generic_config() and mxc_gpio_set_pad_wakeup() call
> of_device_is_compatible() on every invocation to determine pad wakeup
> capability and i.MX8QM-specific behavior. These properties are
> invariant for the lifetime of the device.
>
> Extend the hwdata flags scheme introduced in the previous commit with
> MXC_GPIO_HAS_PAD_WAKEUP and MXC_GPIO_FALL_EDGE_WAKEUP_BROKEN, adding
> dedicated hwdata instances for imx8qm and imx8qxp (also used by imx8dxl).
> This replaces the repeated device tree string comparisons in the
> suspend/resume path with simple flag tests on static per-compatible
> data.
>
> While at it, clean up mxc_gpio_generic_config() to use a local ret
> variable for clarity instead of the == 0 comparison.
>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
Reviewed-by: Frank Li <Frank.Li@nxp.com>
> drivers/gpio/gpio-mxc.c | 46 ++++++++++++++++++++++++++++++++++------------
> 1 file changed, 34 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
> index 1a35b23747b4..70fa139338c9 100644
> --- a/drivers/gpio/gpio-mxc.c
> +++ b/drivers/gpio/gpio-mxc.c
> @@ -34,6 +34,8 @@
> #define IMX_SCU_WAKEUP_HIGH_LVL 7
>
> #define MXC_GPIO_HAS_POWER_OFF BIT(0)
> +#define MXC_GPIO_HAS_PAD_WAKEUP BIT(1)
> +#define MXC_GPIO_FALL_EDGE_WAKEUP_BROKEN BIT(2)
>
> /* device type dependent stuff */
> struct mxc_gpio_hwdata {
> @@ -132,6 +134,26 @@ static struct mxc_gpio_hwdata imx7d_gpio_hwdata = {
> .flags = MXC_GPIO_HAS_POWER_OFF,
> };
>
> +static struct mxc_gpio_hwdata imx8qm_gpio_hwdata = {
> + MXC_GPIO_HW_DATA_COMMON,
> + .flags = MXC_GPIO_FALL_EDGE_WAKEUP_BROKEN | MXC_GPIO_HAS_PAD_WAKEUP,
> +};
> +
> +static struct mxc_gpio_hwdata imx8qxp_gpio_hwdata = {
> + MXC_GPIO_HW_DATA_COMMON,
> + .flags = MXC_GPIO_HAS_PAD_WAKEUP,
> +};
> +
> +static inline bool mxc_gpio_fall_edge_wakeup_broken(struct mxc_gpio_port *port)
> +{
> + return port->hwdata->flags & MXC_GPIO_FALL_EDGE_WAKEUP_BROKEN;
> +}
> +
> +static inline bool mxc_gpio_has_pad_wakeup(struct mxc_gpio_port *port)
> +{
> + return port->hwdata->flags & MXC_GPIO_HAS_PAD_WAKEUP;
> +}
> +
> static inline bool mxc_gpio_has_power_off(struct mxc_gpio_port *port)
> {
> return port->hwdata->flags & MXC_GPIO_HAS_POWER_OFF;
> @@ -158,9 +180,9 @@ static const struct of_device_id mxc_gpio_dt_ids[] = {
> { .compatible = "fsl,imx31-gpio", .data = &imx31_gpio_hwdata },
> { .compatible = "fsl,imx35-gpio", .data = &imx35_gpio_hwdata },
> { .compatible = "fsl,imx7d-gpio", .data = &imx7d_gpio_hwdata },
> - { .compatible = "fsl,imx8dxl-gpio", .data = &imx35_gpio_hwdata },
> - { .compatible = "fsl,imx8qm-gpio", .data = &imx35_gpio_hwdata },
> - { .compatible = "fsl,imx8qxp-gpio", .data = &imx35_gpio_hwdata },
> + { .compatible = "fsl,imx8dxl-gpio", .data = &imx8qxp_gpio_hwdata },
> + { .compatible = "fsl,imx8qm-gpio", .data = &imx8qm_gpio_hwdata },
> + { .compatible = "fsl,imx8qxp-gpio", .data = &imx8qxp_gpio_hwdata },
> { /* sentinel */ }
> };
> MODULE_DEVICE_TABLE(of, mxc_gpio_dt_ids);
> @@ -575,15 +597,16 @@ static void mxc_gpio_restore_regs(struct mxc_gpio_port *port)
> static bool mxc_gpio_generic_config(struct mxc_gpio_port *port,
> unsigned int offset, unsigned long conf)
> {
> - struct device_node *np = port->dev->of_node;
> + int ret;
> +
> + if (!mxc_gpio_has_pad_wakeup(port))
> + return false;
>
> - if (of_device_is_compatible(np, "fsl,imx8dxl-gpio") ||
> - of_device_is_compatible(np, "fsl,imx8qxp-gpio") ||
> - of_device_is_compatible(np, "fsl,imx8qm-gpio"))
> - return (gpiochip_generic_config(&port->gen_gc.gc,
> - offset, conf) == 0);
> + ret = gpiochip_generic_config(&port->gen_gc.gc, offset, conf);
> + if (ret)
> + return false;
>
> - return false;
> + return true;
> }
>
> static bool mxc_gpio_set_pad_wakeup(struct mxc_gpio_port *port, bool enable)
> @@ -591,7 +614,6 @@ static bool mxc_gpio_set_pad_wakeup(struct mxc_gpio_port *port, bool enable)
> unsigned long config;
> bool ret = false;
> int i, type;
> - bool is_imx8qm = of_device_is_compatible(port->dev->of_node, "fsl,imx8qm-gpio");
>
> static const u32 pad_type_map[] = {
> IMX_SCU_WAKEUP_OFF, /* 0 */
> @@ -612,7 +634,7 @@ static bool mxc_gpio_set_pad_wakeup(struct mxc_gpio_port *port, bool enable)
> else
> config = IMX_SCU_WAKEUP_OFF;
>
> - if (is_imx8qm && config == IMX_SCU_WAKEUP_FALL_EDGE) {
> + if (mxc_gpio_fall_edge_wakeup_broken(port) && config == IMX_SCU_WAKEUP_FALL_EDGE) {
> dev_warn_once(port->dev,
> "No falling-edge support for wakeup on i.MX8QM\n");
> config = IMX_SCU_WAKEUP_OFF;
>
> --
> 2.51.0
>
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v5 08/13] gpio: mxc: use local dev variable
2026-10-09 18:05 [PATCH v5 00/13] gpio: mxc: bug fixes and probe cleanup Peng Fan (OSS)
` (6 preceding siblings ...)
2026-10-09 18:05 ` [PATCH v5 07/13] gpio: mxc: convert pad wakeup compatible checks to " Peng Fan (OSS)
@ 2026-10-09 18:05 ` Peng Fan (OSS)
2026-10-09 18:05 ` [PATCH v5 09/13] gpio: mxc: use cleanup guard for pm_runtime_get_noresume() balance Peng Fan (OSS)
` (4 subsequent siblings)
12 siblings, 0 replies; 27+ messages in thread
From: Peng Fan (OSS) @ 2026-10-09 18:05 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski, Frank Li, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Shenwei Wang,
Andy Shevchenko
Cc: linux-gpio, imx, linux-arm-kernel, linux-kernel, Peng Fan
From: Peng Fan <peng.fan@nxp.com>
Introduce a local 'struct device *dev' variable to replace repeated
'&pdev->dev' dereferences throughout mxc_gpio_probe(), improving
readability.
No functional change.
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
drivers/gpio/gpio-mxc.c | 33 +++++++++++++++++----------------
1 file changed, 17 insertions(+), 16 deletions(-)
diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
index 70fa139338c9..5ae9cdee6424 100644
--- a/drivers/gpio/gpio-mxc.c
+++ b/drivers/gpio/gpio-mxc.c
@@ -449,17 +449,18 @@ static int mxc_gpio_probe(struct platform_device *pdev)
{
struct gpio_generic_chip_config config = { };
struct device_node *np = pdev->dev.of_node;
+ struct device *dev = &pdev->dev;
struct mxc_gpio_port *port;
int irq_count;
int irq_base;
int err;
- port = devm_kzalloc(&pdev->dev, sizeof(*port), GFP_KERNEL);
+ port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
if (!port)
return -ENOMEM;
- port->dev = &pdev->dev;
- port->hwdata = device_get_match_data(&pdev->dev);
+ port->dev = dev;
+ port->hwdata = device_get_match_data(dev);
port->base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(port->base))
@@ -480,13 +481,13 @@ static int mxc_gpio_probe(struct platform_device *pdev)
return port->irq;
/* the controller clock is optional */
- port->clk = devm_clk_get_optional_enabled(&pdev->dev, NULL);
+ port->clk = devm_clk_get_optional_enabled(dev, NULL);
if (IS_ERR(port->clk))
return PTR_ERR(port->clk);
- pm_runtime_get_noresume(&pdev->dev);
- pm_runtime_set_active(&pdev->dev);
- pm_runtime_enable(&pdev->dev);
+ pm_runtime_get_noresume(dev);
+ pm_runtime_set_active(dev);
+ pm_runtime_enable(dev);
/* disable the interrupt and clear the status */
writel(0, port->base + GPIO_IMR);
@@ -503,7 +504,7 @@ static int mxc_gpio_probe(struct platform_device *pdev)
} else
port->mx_irq_handler = mx3_gpio_irq_handler;
- config.dev = &pdev->dev;
+ config.dev = dev;
config.sz = 4;
config.dat = port->base + GPIO_PSR;
config.set = port->base + GPIO_DR;
@@ -526,24 +527,24 @@ 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);
+ err = devm_gpiochip_add_data(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());
+ irq_base = devm_irq_alloc_descs(dev, -1, 0, 32, numa_node_id());
if (irq_base < 0) {
err = irq_base;
goto out_bgio;
}
- port->domain = irq_domain_create_legacy(dev_fwnode(&pdev->dev), 32, irq_base, 0,
+ port->domain = irq_domain_create_legacy(dev_fwnode(dev), 32, irq_base, 0,
&irq_domain_simple_ops, NULL);
if (!port->domain) {
err = -ENODEV;
goto out_bgio;
}
- irq_domain_set_pm_device(port->domain, &pdev->dev);
+ irq_domain_set_pm_device(port->domain, dev);
/* gpio-mxc can be a generic irq chip */
err = mxc_gpio_init_gc(port, irq_base);
@@ -555,16 +556,16 @@ static int mxc_gpio_probe(struct platform_device *pdev)
mxc_update_irq_chained_handler(port, true);
platform_set_drvdata(pdev, port);
- pm_runtime_put_autosuspend(&pdev->dev);
+ pm_runtime_put_autosuspend(dev);
return 0;
out_irqdomain_remove:
irq_domain_remove(port->domain);
out_bgio:
- pm_runtime_disable(&pdev->dev);
- pm_runtime_put_noidle(&pdev->dev);
- dev_info(&pdev->dev, "%s failed with errno %d\n", __func__, err);
+ pm_runtime_disable(dev);
+ pm_runtime_put_noidle(dev);
+ dev_info(dev, "%s failed with errno %d\n", __func__, err);
return err;
}
--
2.51.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v5 09/13] gpio: mxc: use cleanup guard for pm_runtime_get_noresume() balance
2026-10-09 18:05 [PATCH v5 00/13] gpio: mxc: bug fixes and probe cleanup Peng Fan (OSS)
` (7 preceding siblings ...)
2026-10-09 18:05 ` [PATCH v5 08/13] gpio: mxc: use local dev variable Peng Fan (OSS)
@ 2026-10-09 18:05 ` Peng Fan (OSS)
2026-10-09 18:19 ` sashiko-bot
2026-10-09 18:51 ` Frank Li
2026-10-09 18:05 ` [PATCH v5 10/13] gpio: mxc: convert probe error handling to devres Peng Fan (OSS)
` (3 subsequent siblings)
12 siblings, 2 replies; 27+ messages in thread
From: Peng Fan (OSS) @ 2026-10-09 18:05 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski, Frank Li, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Shenwei Wang,
Andy Shevchenko
Cc: linux-gpio, imx, linux-arm-kernel, linux-kernel, Peng Fan
From: Peng Fan <peng.fan@nxp.com>
pm_runtime_get_noresume() in probe must be balanced by
pm_runtime_put_noidle() on error paths, or pm_runtime_put_autosuspend()
on success. devm_pm_runtime_get_noresume() cannot be used here because
its devres action (pm_runtime_put_noidle()) would fire on unbind in
addition to the success-path pm_runtime_put_autosuspend(), causing a
usage counter underflow.
Use a __free guard to automatically call pm_runtime_put_noidle() on
error paths, and disarm it with no_free_ptr() before the success-path
pm_runtime_put_autosuspend().
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
drivers/gpio/gpio-mxc.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
index 5ae9cdee6424..3d5a6566d3f4 100644
--- a/drivers/gpio/gpio-mxc.c
+++ b/drivers/gpio/gpio-mxc.c
@@ -445,6 +445,8 @@ static void mxc_update_irq_chained_handler(struct mxc_gpio_port *port, bool enab
}
}
+DEFINE_FREE(pm_runtime_put_noidle, struct device *, if (_T) pm_runtime_put_noidle(_T))
+
static int mxc_gpio_probe(struct platform_device *pdev)
{
struct gpio_generic_chip_config config = { };
@@ -486,6 +488,9 @@ static int mxc_gpio_probe(struct platform_device *pdev)
return PTR_ERR(port->clk);
pm_runtime_get_noresume(dev);
+
+ struct device *pm_guard __free(pm_runtime_put_noidle) = dev;
+
pm_runtime_set_active(dev);
pm_runtime_enable(dev);
@@ -556,6 +561,8 @@ static int mxc_gpio_probe(struct platform_device *pdev)
mxc_update_irq_chained_handler(port, true);
platform_set_drvdata(pdev, port);
+
+ pm_guard = no_free_ptr(pm_guard);
pm_runtime_put_autosuspend(dev);
return 0;
@@ -564,7 +571,6 @@ static int mxc_gpio_probe(struct platform_device *pdev)
irq_domain_remove(port->domain);
out_bgio:
pm_runtime_disable(dev);
- pm_runtime_put_noidle(dev);
dev_info(dev, "%s failed with errno %d\n", __func__, err);
return err;
}
--
2.51.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v5 09/13] gpio: mxc: use cleanup guard for pm_runtime_get_noresume() balance
2026-10-09 18:05 ` [PATCH v5 09/13] gpio: mxc: use cleanup guard for pm_runtime_get_noresume() balance Peng Fan (OSS)
@ 2026-10-09 18:19 ` sashiko-bot
2026-10-09 18:51 ` Frank Li
1 sibling, 0 replies; 27+ messages in thread
From: sashiko-bot @ 2026-10-09 18:19 UTC (permalink / raw)
To: Peng Fan (OSS); +Cc: Frank.Li, imx, linux-gpio
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] pm: double put of usage counter due to no_free_ptr() self-assignment
--
commit f4335cfd7512992cdd6d3fcb1a49da9aabbb176c
Author: Peng Fan <peng.fan@nxp.com>
gpio: mxc: use cleanup guard for pm_runtime_get_noresume() balance
This commit replaces the manual pm_runtime_put_noidle() error path cleanup
with a __free scope-based guard in mxc_gpio_probe(). It also attempts to
disarm the guard on the success path before calling
pm_runtime_put_autosuspend().
> diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
> index 5ae9cdee6424d..3d5a6566d3f43 100644
> --- a/drivers/gpio/gpio-mxc.c
> +++ b/drivers/gpio/gpio-mxc.c
[ ... ]
> @@ -556,6 +561,8 @@ static int mxc_gpio_probe(struct platform_device *pdev)
> mxc_update_irq_chained_handler(port, true);
>
> platform_set_drvdata(pdev, port);
> +
> + pm_guard = no_free_ptr(pm_guard);
> pm_runtime_put_autosuspend(dev);
[Severity: High]
Does this self-assignment defeat the no_free_ptr() cleanup inhibition,
causing a double-put of the usage counter?
Because no_free_ptr() is marked __must_check, assigning it back to the
same variable is a common attempt to silence compiler warnings. However,
no_free_ptr(pm_guard) internally sets pm_guard to NULL but evaluates to
the original pointer value.
By assigning the evaluated result back to pm_guard, the NULL value is
immediately overwritten with the original pointer, leaving the guard
armed.
When mxc_gpio_probe() returns successfully, the cleanup handler will execute
pm_runtime_put_noidle(dev) because pm_guard is still armed. Since this path
also explicitly calls pm_runtime_put_autosuspend(dev) right after the
assignment, will this result in the usage counter being decremented twice?
>
> return 0;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261010-gpio-mxc-cleanup-v5-0-1e59e6ddfa00@nxp.com?part=9
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v5 09/13] gpio: mxc: use cleanup guard for pm_runtime_get_noresume() balance
2026-10-09 18:05 ` [PATCH v5 09/13] gpio: mxc: use cleanup guard for pm_runtime_get_noresume() balance Peng Fan (OSS)
2026-10-09 18:19 ` sashiko-bot
@ 2026-10-09 18:51 ` Frank Li
1 sibling, 0 replies; 27+ messages in thread
From: Frank Li @ 2026-10-09 18:51 UTC (permalink / raw)
To: Peng Fan (OSS)
Cc: Linus Walleij, Bartosz Golaszewski, Frank Li, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Shenwei Wang,
Andy Shevchenko, linux-gpio, imx, linux-arm-kernel, linux-kernel,
Peng Fan
On Sat, Oct 10, 2026 at 02:05:30AM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
>
> pm_runtime_get_noresume() in probe must be balanced by
> pm_runtime_put_noidle() on error paths, or pm_runtime_put_autosuspend()
> on success. devm_pm_runtime_get_noresume() cannot be used here because
> its devres action (pm_runtime_put_noidle()) would fire on unbind in
> addition to the success-path pm_runtime_put_autosuspend(), causing a
> usage counter underflow.
>
> Use a __free guard to automatically call pm_runtime_put_noidle() on
> error paths, and disarm it with no_free_ptr() before the success-path
> pm_runtime_put_autosuspend().
>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
> drivers/gpio/gpio-mxc.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
> index 5ae9cdee6424..3d5a6566d3f4 100644
> --- a/drivers/gpio/gpio-mxc.c
> +++ b/drivers/gpio/gpio-mxc.c
> @@ -445,6 +445,8 @@ static void mxc_update_irq_chained_handler(struct mxc_gpio_port *port, bool enab
> }
> }
>
> +DEFINE_FREE(pm_runtime_put_noidle, struct device *, if (_T) pm_runtime_put_noidle(_T))
> +
This change need put into common runtime_pm.h and like pm maintainer to
review it. Other system may use it, like DMA engine, CSI, ...
And if someone add to common file later, it will cause build error.
> static int mxc_gpio_probe(struct platform_device *pdev)
> {
> struct gpio_generic_chip_config config = { };
> @@ -486,6 +488,9 @@ static int mxc_gpio_probe(struct platform_device *pdev)
> return PTR_ERR(port->clk);
>
> pm_runtime_get_noresume(dev);
> +
> + struct device *pm_guard __free(pm_runtime_put_noidle) = dev;
> +
> pm_runtime_set_active(dev);
> pm_runtime_enable(dev);
>
> @@ -556,6 +561,8 @@ static int mxc_gpio_probe(struct platform_device *pdev)
> mxc_update_irq_chained_handler(port, true);
>
> platform_set_drvdata(pdev, port);
> +
> + pm_guard = no_free_ptr(pm_guard);
use retain_and_null_ptr();
Frank
> pm_runtime_put_autosuspend(dev);
>
> return 0;
> @@ -564,7 +571,6 @@ static int mxc_gpio_probe(struct platform_device *pdev)
> irq_domain_remove(port->domain);
> out_bgio:
> pm_runtime_disable(dev);
> - pm_runtime_put_noidle(dev);
> dev_info(dev, "%s failed with errno %d\n", __func__, err);
> return err;
> }
>
> --
> 2.51.0
>
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v5 10/13] gpio: mxc: convert probe error handling to devres
2026-10-09 18:05 [PATCH v5 00/13] gpio: mxc: bug fixes and probe cleanup Peng Fan (OSS)
` (8 preceding siblings ...)
2026-10-09 18:05 ` [PATCH v5 09/13] gpio: mxc: use cleanup guard for pm_runtime_get_noresume() balance Peng Fan (OSS)
@ 2026-10-09 18:05 ` Peng Fan (OSS)
2026-10-09 18:23 ` sashiko-bot
2026-10-09 18:53 ` Frank Li
2026-10-09 18:05 ` [PATCH v5 11/13] gpio: mxc: switch probe error paths to dev_err_probe() Peng Fan (OSS)
` (2 subsequent siblings)
12 siblings, 2 replies; 27+ messages in thread
From: Peng Fan (OSS) @ 2026-10-09 18:05 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski, Frank Li, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Shenwei Wang,
Andy Shevchenko
Cc: linux-gpio, imx, linux-arm-kernel, linux-kernel, Peng Fan
From: Peng Fan <peng.fan@nxp.com>
Replace irq_domain_create_legacy() with devm_irq_domain_instantiate()
and the open-coded pm_runtime_set_active() + pm_runtime_enable() pair
with devm_pm_runtime_set_active_enabled(), converting the remaining
manually-unwound resources in probe to devres management.
With every allocation after the PM block now devm-managed, the
out_irqdomain_remove and out_bgio error-path labels are eliminated
entirely - probe errors simply return directly.
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
drivers/gpio/gpio-mxc.c | 43 +++++++++++++++++++++----------------------
1 file changed, 21 insertions(+), 22 deletions(-)
diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
index 3d5a6566d3f4..6074b1e3d683 100644
--- a/drivers/gpio/gpio-mxc.c
+++ b/drivers/gpio/gpio-mxc.c
@@ -452,6 +452,7 @@ static int mxc_gpio_probe(struct platform_device *pdev)
struct gpio_generic_chip_config config = { };
struct device_node *np = pdev->dev.of_node;
struct device *dev = &pdev->dev;
+ struct irq_domain_info d_info;
struct mxc_gpio_port *port;
int irq_count;
int irq_base;
@@ -491,8 +492,9 @@ static int mxc_gpio_probe(struct platform_device *pdev)
struct device *pm_guard __free(pm_runtime_put_noidle) = dev;
- pm_runtime_set_active(dev);
- pm_runtime_enable(dev);
+ err = devm_pm_runtime_set_active_enabled(dev);
+ if (err)
+ return dev_err_probe(dev, err, "Failed to enable PM runtime\n");
/* disable the interrupt and clear the status */
writel(0, port->base + GPIO_IMR);
@@ -518,7 +520,7 @@ static int mxc_gpio_probe(struct platform_device *pdev)
err = gpio_generic_chip_init(&port->gen_gc, &config);
if (err)
- goto out_bgio;
+ return err;
port->gen_gc.gc.request = mxc_gpio_request;
port->gen_gc.gc.free = mxc_gpio_free;
@@ -534,27 +536,31 @@ static int mxc_gpio_probe(struct platform_device *pdev)
err = devm_gpiochip_add_data(dev, &port->gen_gc.gc, port);
if (err)
- goto out_bgio;
+ return err;
irq_base = devm_irq_alloc_descs(dev, -1, 0, 32, numa_node_id());
- if (irq_base < 0) {
- err = irq_base;
- goto out_bgio;
- }
+ if (irq_base < 0)
+ return irq_base;
+
+ d_info = (struct irq_domain_info) {
+ .fwnode = dev_fwnode(dev),
+ .size = 32,
+ .hwirq_max = 32,
+ .virq_base = irq_base,
+ .ops = &irq_domain_simple_ops,
+ .dev = dev,
+ };
- port->domain = irq_domain_create_legacy(dev_fwnode(dev), 32, irq_base, 0,
- &irq_domain_simple_ops, NULL);
- if (!port->domain) {
- err = -ENODEV;
- goto out_bgio;
- }
+ port->domain = devm_irq_domain_instantiate(dev, &d_info);
+ if (IS_ERR(port->domain))
+ return PTR_ERR(port->domain);
irq_domain_set_pm_device(port->domain, dev);
/* gpio-mxc can be a generic irq chip */
err = mxc_gpio_init_gc(port, irq_base);
if (err < 0)
- goto out_irqdomain_remove;
+ return err;
list_add_tail(&port->node, &mxc_gpio_ports);
@@ -566,13 +572,6 @@ static int mxc_gpio_probe(struct platform_device *pdev)
pm_runtime_put_autosuspend(dev);
return 0;
-
-out_irqdomain_remove:
- irq_domain_remove(port->domain);
-out_bgio:
- pm_runtime_disable(dev);
- dev_info(dev, "%s failed with errno %d\n", __func__, err);
- return err;
}
static void mxc_gpio_save_regs(struct mxc_gpio_port *port)
--
2.51.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v5 10/13] gpio: mxc: convert probe error handling to devres
2026-10-09 18:05 ` [PATCH v5 10/13] gpio: mxc: convert probe error handling to devres Peng Fan (OSS)
@ 2026-10-09 18:23 ` sashiko-bot
2026-10-09 18:53 ` Frank Li
1 sibling, 0 replies; 27+ messages in thread
From: sashiko-bot @ 2026-10-09 18:23 UTC (permalink / raw)
To: Peng Fan (OSS); +Cc: linux-gpio, imx, Frank.Li
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] pm: double-put of PM runtime usage count on probe success
- [High] gpio: mxc: NULL ptr dereference on probe error due to inverted PM cleanup order
- [High] gpio: mxc: early gpiochip exposure returns -ENXIO from to_irq
--
commit 8d3ca67baef31cc2d1c0b0ff840208db9ae18818
Author: Peng Fan <peng.fan@nxp.com>
gpio: mxc: convert probe error handling to devres
This commit converts the manual probe error handling in mxc_gpio_probe() to
devres management by utilizing devm_pm_runtime_set_active_enabled() and
devm_irq_domain_instantiate(). Error path labels are eliminated, replacing
them with direct returns.
> diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
> index 3d5a6566d3f43..6074b1e3d6834 100644
> --- a/drivers/gpio/gpio-mxc.c
> +++ b/drivers/gpio/gpio-mxc.c
[ ... ]
> @@ -491,8 +492,9 @@ static int mxc_gpio_probe(struct platform_device *pdev)
>
> struct device *pm_guard __free(pm_runtime_put_noidle) = dev;
>
> - pm_runtime_set_active(dev);
> - pm_runtime_enable(dev);
> + err = devm_pm_runtime_set_active_enabled(dev);
> + if (err)
> + return dev_err_probe(dev, err, "Failed to enable PM runtime\n");
[ ... ]
> @@ -534,27 +536,31 @@ static int mxc_gpio_probe(struct platform_device *pdev)
>
> err = devm_gpiochip_add_data(dev, &port->gen_gc.gc, port);
> if (err)
> - goto out_bgio;
> + return err;
[Severity: High]
Could exposing the GPIO chip before the IRQ domain is ready cause issues for
concurrent consumers?
Because devm_gpiochip_add_data() is called before the IRQ domain is
instantiated, a concurrent consumer might call gpiod_to_irq() while
port->domain is still NULL.
In this scenario, irq_find_mapping() inside mxc_gpio_to_irq() will safely
handle the NULL domain and return 0. However, gpiod_to_irq() translates a
0 return value from a custom to_irq callback as -ENXIO. This bypasses the
gpiolib protection that normally returns -EPROBE_DEFER for uninitialized
irqchips, causing the consumer probe to fail permanently.
>
> irq_base = devm_irq_alloc_descs(dev, -1, 0, 32, numa_node_id());
> - if (irq_base < 0) {
> - err = irq_base;
> - goto out_bgio;
> - }
> + if (irq_base < 0)
> + return irq_base;
[Severity: High]
Does this sequence create a window for a NULL pointer dereference on probe
error?
When returning an error after devm_gpiochip_add_data(), the
__free(pm_runtime_put_noidle) cleanup will execute immediately, dropping the
usage count to 0. Since devm_pm_runtime_set_active_enabled() uses devres, PM
runtime won't be disabled until later during devres release.
If a concurrent consumer requests and frees a GPIO during this window, it can
drop the final usage count and trigger mxc_gpio_runtime_suspend(). Because
platform_set_drvdata() hasn't been called yet, dev_get_drvdata() will return
NULL, which is then dereferenced in mxc_gpio_save_regs().
[ ... ]
> @@ -566,13 +572,6 @@ static int mxc_gpio_probe(struct platform_device *pdev)
> pm_runtime_put_autosuspend(dev);
>
> return 0;
[Severity: High]
Does the self-assignment of no_free_ptr() earlier in this block fail to
inhibit the cleanup?
The mxc_gpio_probe() function ends with:
pm_guard = no_free_ptr(pm_guard);
pm_runtime_put_autosuspend(dev);
return 0;
The no_free_ptr() macro is implemented as a statement expression that
nullifies the internal pointer but evaluates to the original pointer value.
Assigning this output back to the same variable overwrites the nullified
value with the original pointer.
Because pm_guard is still non-NULL when the function returns, the
pm_runtime_put_noidle() cleanup function will also execute, resulting in a
double-put of the PM runtime usage count on success.
Can this be resolved by using no_free_ptr(pm_guard) without the assignment?
> -
> -out_irqdomain_remove:
> - irq_domain_remove(port->domain);
> -out_bgio:
> - pm_runtime_disable(dev);
> - dev_info(dev, "%s failed with errno %d\n", __func__, err);
> - return err;
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261010-gpio-mxc-cleanup-v5-0-1e59e6ddfa00@nxp.com?part=10
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v5 10/13] gpio: mxc: convert probe error handling to devres
2026-10-09 18:05 ` [PATCH v5 10/13] gpio: mxc: convert probe error handling to devres Peng Fan (OSS)
2026-10-09 18:23 ` sashiko-bot
@ 2026-10-09 18:53 ` Frank Li
1 sibling, 0 replies; 27+ messages in thread
From: Frank Li @ 2026-10-09 18:53 UTC (permalink / raw)
To: Peng Fan (OSS)
Cc: Linus Walleij, Bartosz Golaszewski, Frank Li, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Shenwei Wang,
Andy Shevchenko, linux-gpio, imx, linux-arm-kernel, linux-kernel,
Peng Fan
On Sat, Oct 10, 2026 at 02:05:31AM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
subject:
gpio: mxc: use devres functions simplify probe code
Reviewed-by: Frank Li <Frank.Li@nxp.com>
>
> Replace irq_domain_create_legacy() with devm_irq_domain_instantiate()
> and the open-coded pm_runtime_set_active() + pm_runtime_enable() pair
> with devm_pm_runtime_set_active_enabled(), converting the remaining
> manually-unwound resources in probe to devres management.
>
> With every allocation after the PM block now devm-managed, the
> out_irqdomain_remove and out_bgio error-path labels are eliminated
> entirely - probe errors simply return directly.
>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
> drivers/gpio/gpio-mxc.c | 43 +++++++++++++++++++++----------------------
> 1 file changed, 21 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
> index 3d5a6566d3f4..6074b1e3d683 100644
> --- a/drivers/gpio/gpio-mxc.c
> +++ b/drivers/gpio/gpio-mxc.c
> @@ -452,6 +452,7 @@ static int mxc_gpio_probe(struct platform_device *pdev)
> struct gpio_generic_chip_config config = { };
> struct device_node *np = pdev->dev.of_node;
> struct device *dev = &pdev->dev;
> + struct irq_domain_info d_info;
> struct mxc_gpio_port *port;
> int irq_count;
> int irq_base;
> @@ -491,8 +492,9 @@ static int mxc_gpio_probe(struct platform_device *pdev)
>
> struct device *pm_guard __free(pm_runtime_put_noidle) = dev;
>
> - pm_runtime_set_active(dev);
> - pm_runtime_enable(dev);
> + err = devm_pm_runtime_set_active_enabled(dev);
> + if (err)
> + return dev_err_probe(dev, err, "Failed to enable PM runtime\n");
>
> /* disable the interrupt and clear the status */
> writel(0, port->base + GPIO_IMR);
> @@ -518,7 +520,7 @@ static int mxc_gpio_probe(struct platform_device *pdev)
>
> err = gpio_generic_chip_init(&port->gen_gc, &config);
> if (err)
> - goto out_bgio;
> + return err;
>
> port->gen_gc.gc.request = mxc_gpio_request;
> port->gen_gc.gc.free = mxc_gpio_free;
> @@ -534,27 +536,31 @@ static int mxc_gpio_probe(struct platform_device *pdev)
>
> err = devm_gpiochip_add_data(dev, &port->gen_gc.gc, port);
> if (err)
> - goto out_bgio;
> + return err;
>
> irq_base = devm_irq_alloc_descs(dev, -1, 0, 32, numa_node_id());
> - if (irq_base < 0) {
> - err = irq_base;
> - goto out_bgio;
> - }
> + if (irq_base < 0)
> + return irq_base;
> +
> + d_info = (struct irq_domain_info) {
> + .fwnode = dev_fwnode(dev),
> + .size = 32,
> + .hwirq_max = 32,
> + .virq_base = irq_base,
> + .ops = &irq_domain_simple_ops,
> + .dev = dev,
> + };
>
> - port->domain = irq_domain_create_legacy(dev_fwnode(dev), 32, irq_base, 0,
> - &irq_domain_simple_ops, NULL);
> - if (!port->domain) {
> - err = -ENODEV;
> - goto out_bgio;
> - }
> + port->domain = devm_irq_domain_instantiate(dev, &d_info);
> + if (IS_ERR(port->domain))
> + return PTR_ERR(port->domain);
>
> irq_domain_set_pm_device(port->domain, dev);
>
> /* gpio-mxc can be a generic irq chip */
> err = mxc_gpio_init_gc(port, irq_base);
> if (err < 0)
> - goto out_irqdomain_remove;
> + return err;
>
> list_add_tail(&port->node, &mxc_gpio_ports);
>
> @@ -566,13 +572,6 @@ static int mxc_gpio_probe(struct platform_device *pdev)
> pm_runtime_put_autosuspend(dev);
>
> return 0;
> -
> -out_irqdomain_remove:
> - irq_domain_remove(port->domain);
> -out_bgio:
> - pm_runtime_disable(dev);
> - dev_info(dev, "%s failed with errno %d\n", __func__, err);
> - return err;
> }
>
> static void mxc_gpio_save_regs(struct mxc_gpio_port *port)
>
> --
> 2.51.0
>
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v5 11/13] gpio: mxc: switch probe error paths to dev_err_probe()
2026-10-09 18:05 [PATCH v5 00/13] gpio: mxc: bug fixes and probe cleanup Peng Fan (OSS)
` (9 preceding siblings ...)
2026-10-09 18:05 ` [PATCH v5 10/13] gpio: mxc: convert probe error handling to devres Peng Fan (OSS)
@ 2026-10-09 18:05 ` Peng Fan (OSS)
2026-10-09 18:21 ` sashiko-bot
2026-10-09 18:55 ` Frank Li
2026-10-09 18:05 ` [PATCH v5 12/13] gpio: mxc: introduce MXC_ICR macros and use field_prep/field_get Peng Fan (OSS)
2026-10-09 18:05 ` [PATCH v5 13/13] gpio: mxc: use BIT() macro for single-bit operations Peng Fan (OSS)
12 siblings, 2 replies; 27+ messages in thread
From: Peng Fan (OSS) @ 2026-10-09 18:05 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski, Frank Li, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Shenwei Wang,
Andy Shevchenko
Cc: linux-gpio, imx, linux-arm-kernel, linux-kernel, Peng Fan
From: Peng Fan <peng.fan@nxp.com>
Replace bare return statements with dev_err_probe() across all probe
error paths for consistent diagnostics and deferred-probe support.
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
drivers/gpio/gpio-mxc.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
index 6074b1e3d683..1eb5cd148c28 100644
--- a/drivers/gpio/gpio-mxc.c
+++ b/drivers/gpio/gpio-mxc.c
@@ -520,7 +520,7 @@ static int mxc_gpio_probe(struct platform_device *pdev)
err = gpio_generic_chip_init(&port->gen_gc, &config);
if (err)
- return err;
+ return dev_err_probe(dev, err, "Failed to init gpio chip\n");
port->gen_gc.gc.request = mxc_gpio_request;
port->gen_gc.gc.free = mxc_gpio_free;
@@ -536,11 +536,11 @@ static int mxc_gpio_probe(struct platform_device *pdev)
err = devm_gpiochip_add_data(dev, &port->gen_gc.gc, port);
if (err)
- return err;
+ return dev_err_probe(dev, err, "Failed to add gpiochip data\n");
irq_base = devm_irq_alloc_descs(dev, -1, 0, 32, numa_node_id());
if (irq_base < 0)
- return irq_base;
+ return dev_err_probe(dev, irq_base, "Failed to alloc irq desc\n");
d_info = (struct irq_domain_info) {
.fwnode = dev_fwnode(dev),
@@ -553,14 +553,15 @@ static int mxc_gpio_probe(struct platform_device *pdev)
port->domain = devm_irq_domain_instantiate(dev, &d_info);
if (IS_ERR(port->domain))
- return PTR_ERR(port->domain);
+ return dev_err_probe(dev, PTR_ERR(port->domain),
+ "Failed to create irq domain\n");
irq_domain_set_pm_device(port->domain, dev);
/* gpio-mxc can be a generic irq chip */
err = mxc_gpio_init_gc(port, irq_base);
if (err < 0)
- return err;
+ return dev_err_probe(dev, err, "Failed to init generic irq chip\n");
list_add_tail(&port->node, &mxc_gpio_ports);
--
2.51.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v5 11/13] gpio: mxc: switch probe error paths to dev_err_probe()
2026-10-09 18:05 ` [PATCH v5 11/13] gpio: mxc: switch probe error paths to dev_err_probe() Peng Fan (OSS)
@ 2026-10-09 18:21 ` sashiko-bot
2026-10-09 18:55 ` Frank Li
1 sibling, 0 replies; 27+ messages in thread
From: sashiko-bot @ 2026-10-09 18:21 UTC (permalink / raw)
To: Peng Fan (OSS); +Cc: imx, Frank.Li, linux-gpio
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] gpio: mxc: missed dev_err_probe() conversion for irq_count and clk
--
commit 9d304bc479f48f3358f5e78d0a6c7c31a38cd860
Author: Peng Fan <peng.fan@nxp.com>
gpio: mxc: switch probe error paths to dev_err_probe()
This commit converts the error handling paths in the probe function of the
mxc gpio driver to use dev_err_probe(). It aims to provide consistent
diagnostics and support for deferred probe tracking.
> diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
> index 6074b1e3d6834..1eb5cd148c28a 100644
> --- a/drivers/gpio/gpio-mxc.c
> +++ b/drivers/gpio/gpio-mxc.c
> @@ -520,7 +520,7 @@ static int mxc_gpio_probe(struct platform_device *pdev)
>
> err = gpio_generic_chip_init(&port->gen_gc, &config);
> if (err)
> - return err;
> + return dev_err_probe(dev, err, "Failed to init gpio chip\n");
[Severity: Low]
The commit message states that bare return statements are replaced across
all probe error paths. Are there some that were missed in this conversion?
Looking earlier in mxc_gpio_probe(), there are still bare returns for the
IRQ count and clock setup:
irq_count = platform_irq_count(pdev);
if (irq_count < 0)
return irq_count;
and:
/* the controller clock is optional */
port->clk = devm_clk_get_optional_enabled(dev, NULL);
if (IS_ERR(port->clk))
return PTR_ERR(port->clk);
Since platform_irq_count() and devm_clk_get_optional_enabled() can return
-EPROBE_DEFER when their dependencies are not yet ready, should these paths
also be converted to use dev_err_probe() to ensure those deferrals are
properly logged to the deferred-probe debugfs tracking mechanism?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261010-gpio-mxc-cleanup-v5-0-1e59e6ddfa00@nxp.com?part=11
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v5 11/13] gpio: mxc: switch probe error paths to dev_err_probe()
2026-10-09 18:05 ` [PATCH v5 11/13] gpio: mxc: switch probe error paths to dev_err_probe() Peng Fan (OSS)
2026-10-09 18:21 ` sashiko-bot
@ 2026-10-09 18:55 ` Frank Li
1 sibling, 0 replies; 27+ messages in thread
From: Frank Li @ 2026-10-09 18:55 UTC (permalink / raw)
To: Peng Fan (OSS)
Cc: Linus Walleij, Bartosz Golaszewski, Frank Li, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Shenwei Wang,
Andy Shevchenko, linux-gpio, imx, linux-arm-kernel, linux-kernel,
Peng Fan
On Sat, Oct 10, 2026 at 02:05:32AM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
subject need update
gpio: mxc: add dev_err_probe() for error paths to help diagnostics
Frank
>
> Replace bare return statements with dev_err_probe() across all probe
> error paths for consistent diagnostics and deferred-probe support.
>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
> drivers/gpio/gpio-mxc.c | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
> index 6074b1e3d683..1eb5cd148c28 100644
> --- a/drivers/gpio/gpio-mxc.c
> +++ b/drivers/gpio/gpio-mxc.c
> @@ -520,7 +520,7 @@ static int mxc_gpio_probe(struct platform_device *pdev)
>
> err = gpio_generic_chip_init(&port->gen_gc, &config);
> if (err)
> - return err;
> + return dev_err_probe(dev, err, "Failed to init gpio chip\n");
>
> port->gen_gc.gc.request = mxc_gpio_request;
> port->gen_gc.gc.free = mxc_gpio_free;
> @@ -536,11 +536,11 @@ static int mxc_gpio_probe(struct platform_device *pdev)
>
> err = devm_gpiochip_add_data(dev, &port->gen_gc.gc, port);
> if (err)
> - return err;
> + return dev_err_probe(dev, err, "Failed to add gpiochip data\n");
>
> irq_base = devm_irq_alloc_descs(dev, -1, 0, 32, numa_node_id());
> if (irq_base < 0)
> - return irq_base;
> + return dev_err_probe(dev, irq_base, "Failed to alloc irq desc\n");
>
> d_info = (struct irq_domain_info) {
> .fwnode = dev_fwnode(dev),
> @@ -553,14 +553,15 @@ static int mxc_gpio_probe(struct platform_device *pdev)
>
> port->domain = devm_irq_domain_instantiate(dev, &d_info);
> if (IS_ERR(port->domain))
> - return PTR_ERR(port->domain);
> + return dev_err_probe(dev, PTR_ERR(port->domain),
> + "Failed to create irq domain\n");
>
> irq_domain_set_pm_device(port->domain, dev);
>
> /* gpio-mxc can be a generic irq chip */
> err = mxc_gpio_init_gc(port, irq_base);
> if (err < 0)
> - return err;
> + return dev_err_probe(dev, err, "Failed to init generic irq chip\n");
>
> list_add_tail(&port->node, &mxc_gpio_ports);
>
>
> --
> 2.51.0
>
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v5 12/13] gpio: mxc: introduce MXC_ICR macros and use field_prep/field_get
2026-10-09 18:05 [PATCH v5 00/13] gpio: mxc: bug fixes and probe cleanup Peng Fan (OSS)
` (10 preceding siblings ...)
2026-10-09 18:05 ` [PATCH v5 11/13] gpio: mxc: switch probe error paths to dev_err_probe() Peng Fan (OSS)
@ 2026-10-09 18:05 ` Peng Fan (OSS)
2026-10-09 18:58 ` Frank Li
2026-10-09 18:05 ` [PATCH v5 13/13] gpio: mxc: use BIT() macro for single-bit operations Peng Fan (OSS)
12 siblings, 1 reply; 27+ messages in thread
From: Peng Fan (OSS) @ 2026-10-09 18:05 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski, Frank Li, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Shenwei Wang,
Andy Shevchenko
Cc: linux-gpio, imx, linux-arm-kernel, linux-kernel, Peng Fan
From: Peng Fan <peng.fan@nxp.com>
Both gpio_set_irq_type() and mxc_flip_edge() open-code the same ICR
register selection and 2-bit field shift/mask arithmetic with magic
numbers (0x10, 0xf, 0x3).
Introduce two macros:
- MXC_ICR_REG(gpio): selects ICR1 (pins 0-15) or ICR2 (pins 16-31)
- MXC_ICR_MASK(gpio): 2-bit mask at the correct position
Use 0x3U in MXC_ICR_MASK() to avoid implementation-defined behavior
when shifting by 30 bits (pin 15 or 31).
Extract the read-modify-write pattern into icr_update_edge()
and the edge readback into icr_get_edge(), using field_prep() and
field_get() from linux/bitfield.h for the shift/extract operations.
No functional change.
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
drivers/gpio/gpio-mxc.c | 42 +++++++++++++++++++++++++-----------------
1 file changed, 25 insertions(+), 17 deletions(-)
diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
index 1eb5cd148c28..97147a5d747b 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/bitfield.h>
#include <linux/cleanup.h>
#include <linux/clk.h>
#include <linux/err.h>
@@ -174,6 +175,9 @@ static inline bool mxc_gpio_has_power_off(struct mxc_gpio_port *port)
#define GPIO_INT_FALL_EDGE (port->hwdata->fall_edge)
#define GPIO_INT_BOTH_EDGES 0x4
+#define MXC_ICR_REG(gpio) (GPIO_ICR1 + (((gpio) & 0x10) >> 2))
+#define MXC_ICR_MASK(gpio) (0x3U << (((gpio) & 0xf) << 1))
+
static const struct of_device_id mxc_gpio_dt_ids[] = {
{ .compatible = "fsl,imx1-gpio", .data = &imx1_imx21_gpio_hwdata },
{ .compatible = "fsl,imx21-gpio", .data = &imx1_imx21_gpio_hwdata },
@@ -196,14 +200,28 @@ static LIST_HEAD(mxc_gpio_ports);
/* Note: This driver assumes 32 GPIOs are handled in one register */
+static void icr_update_edge(struct mxc_gpio_port *port, u32 gpio, u32 edge)
+{
+ void __iomem *reg = port->base;
+ u32 val;
+
+ reg += MXC_ICR_REG(gpio);
+ val = readl(reg) & ~MXC_ICR_MASK(gpio);
+ writel(val | field_prep(MXC_ICR_MASK(gpio), edge), reg);
+}
+
+static u32 icr_get_edge(struct mxc_gpio_port *port, u32 gpio)
+{
+ return field_get(MXC_ICR_MASK(gpio), readl(port->base + MXC_ICR_REG(gpio)));
+}
+
static int gpio_set_irq_type(struct irq_data *d, u32 type)
{
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
struct mxc_gpio_port *port = gc->private;
- u32 bit, val;
+ u32 val;
u32 gpio_idx = d->hwirq;
int edge;
- void __iomem *reg = port->base;
clear_bit(gpio_idx, &port->both_edges);
switch (type) {
@@ -249,12 +267,8 @@ static int gpio_set_irq_type(struct irq_data *d, u32 type)
port->base + GPIO_EDGE_SEL);
}
- if (edge != GPIO_INT_BOTH_EDGES) {
- reg += GPIO_ICR1 + ((gpio_idx & 0x10) >> 2); /* lower or upper register */
- bit = gpio_idx & 0xf;
- val = readl(reg) & ~(0x3 << (bit << 1));
- writel(val | (edge << (bit << 1)), reg);
- }
+ if (edge != GPIO_INT_BOTH_EDGES)
+ icr_update_edge(port, gpio_idx, edge);
writel(1 << gpio_idx, port->base + GPIO_ISR);
port->pad_type[gpio_idx] = type;
@@ -265,17 +279,11 @@ static int gpio_set_irq_type(struct irq_data *d, u32 type)
static void mxc_flip_edge(struct mxc_gpio_port *port, u32 gpio)
{
- void __iomem *reg = port->base;
- u32 bit, val;
- int edge;
+ u32 edge;
guard(gpio_generic_lock_irqsave)(&port->gen_gc);
- reg += GPIO_ICR1 + ((gpio & 0x10) >> 2); /* lower or upper register */
- bit = gpio & 0xf;
- val = readl(reg);
- edge = (val >> (bit << 1)) & 3;
- val &= ~(0x3 << (bit << 1));
+ edge = icr_get_edge(port, gpio);
if (edge == GPIO_INT_HIGH_LEV) {
edge = GPIO_INT_LOW_LEV;
pr_debug("mxc: switch GPIO %d to low trigger\n", gpio);
@@ -287,7 +295,7 @@ static void mxc_flip_edge(struct mxc_gpio_port *port, u32 gpio)
gpio, edge);
return;
}
- writel(val | (edge << (bit << 1)), reg);
+ icr_update_edge(port, gpio, edge);
}
/* handle 32 interrupts in one status register */
--
2.51.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v5 12/13] gpio: mxc: introduce MXC_ICR macros and use field_prep/field_get
2026-10-09 18:05 ` [PATCH v5 12/13] gpio: mxc: introduce MXC_ICR macros and use field_prep/field_get Peng Fan (OSS)
@ 2026-10-09 18:58 ` Frank Li
0 siblings, 0 replies; 27+ messages in thread
From: Frank Li @ 2026-10-09 18:58 UTC (permalink / raw)
To: Peng Fan (OSS)
Cc: Linus Walleij, Bartosz Golaszewski, Frank Li, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Shenwei Wang,
Andy Shevchenko, linux-gpio, imx, linux-arm-kernel, linux-kernel,
Peng Fan
On Sat, Oct 10, 2026 at 02:05:33AM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
>
> Both gpio_set_irq_type() and mxc_flip_edge() open-code the same ICR
> register selection and 2-bit field shift/mask arithmetic with magic
> numbers (0x10, 0xf, 0x3).
>
> Introduce two macros:
> - MXC_ICR_REG(gpio): selects ICR1 (pins 0-15) or ICR2 (pins 16-31)
> - MXC_ICR_MASK(gpio): 2-bit mask at the correct position
>
> Use 0x3U in MXC_ICR_MASK() to avoid implementation-defined behavior
> when shifting by 30 bits (pin 15 or 31).
>
> Extract the read-modify-write pattern into icr_update_edge()
> and the edge readback into icr_get_edge(), using field_prep() and
> field_get() from linux/bitfield.h for the shift/extract operations.
Nit: remove "from linux/bitfield.h", which is reduntant information.
Reviewed-by: Frank Li <Frank.Li@nxp.com>
>
> No functional change.
>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
> drivers/gpio/gpio-mxc.c | 42 +++++++++++++++++++++++++-----------------
> 1 file changed, 25 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
> index 1eb5cd148c28..97147a5d747b 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/bitfield.h>
> #include <linux/cleanup.h>
> #include <linux/clk.h>
> #include <linux/err.h>
> @@ -174,6 +175,9 @@ static inline bool mxc_gpio_has_power_off(struct mxc_gpio_port *port)
> #define GPIO_INT_FALL_EDGE (port->hwdata->fall_edge)
> #define GPIO_INT_BOTH_EDGES 0x4
>
> +#define MXC_ICR_REG(gpio) (GPIO_ICR1 + (((gpio) & 0x10) >> 2))
> +#define MXC_ICR_MASK(gpio) (0x3U << (((gpio) & 0xf) << 1))
> +
> static const struct of_device_id mxc_gpio_dt_ids[] = {
> { .compatible = "fsl,imx1-gpio", .data = &imx1_imx21_gpio_hwdata },
> { .compatible = "fsl,imx21-gpio", .data = &imx1_imx21_gpio_hwdata },
> @@ -196,14 +200,28 @@ static LIST_HEAD(mxc_gpio_ports);
>
> /* Note: This driver assumes 32 GPIOs are handled in one register */
>
> +static void icr_update_edge(struct mxc_gpio_port *port, u32 gpio, u32 edge)
> +{
> + void __iomem *reg = port->base;
> + u32 val;
> +
> + reg += MXC_ICR_REG(gpio);
> + val = readl(reg) & ~MXC_ICR_MASK(gpio);
> + writel(val | field_prep(MXC_ICR_MASK(gpio), edge), reg);
> +}
> +
> +static u32 icr_get_edge(struct mxc_gpio_port *port, u32 gpio)
> +{
> + return field_get(MXC_ICR_MASK(gpio), readl(port->base + MXC_ICR_REG(gpio)));
> +}
> +
> static int gpio_set_irq_type(struct irq_data *d, u32 type)
> {
> struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
> struct mxc_gpio_port *port = gc->private;
> - u32 bit, val;
> + u32 val;
> u32 gpio_idx = d->hwirq;
> int edge;
> - void __iomem *reg = port->base;
>
> clear_bit(gpio_idx, &port->both_edges);
> switch (type) {
> @@ -249,12 +267,8 @@ static int gpio_set_irq_type(struct irq_data *d, u32 type)
> port->base + GPIO_EDGE_SEL);
> }
>
> - if (edge != GPIO_INT_BOTH_EDGES) {
> - reg += GPIO_ICR1 + ((gpio_idx & 0x10) >> 2); /* lower or upper register */
> - bit = gpio_idx & 0xf;
> - val = readl(reg) & ~(0x3 << (bit << 1));
> - writel(val | (edge << (bit << 1)), reg);
> - }
> + if (edge != GPIO_INT_BOTH_EDGES)
> + icr_update_edge(port, gpio_idx, edge);
>
> writel(1 << gpio_idx, port->base + GPIO_ISR);
> port->pad_type[gpio_idx] = type;
> @@ -265,17 +279,11 @@ static int gpio_set_irq_type(struct irq_data *d, u32 type)
>
> static void mxc_flip_edge(struct mxc_gpio_port *port, u32 gpio)
> {
> - void __iomem *reg = port->base;
> - u32 bit, val;
> - int edge;
> + u32 edge;
>
> guard(gpio_generic_lock_irqsave)(&port->gen_gc);
>
> - reg += GPIO_ICR1 + ((gpio & 0x10) >> 2); /* lower or upper register */
> - bit = gpio & 0xf;
> - val = readl(reg);
> - edge = (val >> (bit << 1)) & 3;
> - val &= ~(0x3 << (bit << 1));
> + edge = icr_get_edge(port, gpio);
> if (edge == GPIO_INT_HIGH_LEV) {
> edge = GPIO_INT_LOW_LEV;
> pr_debug("mxc: switch GPIO %d to low trigger\n", gpio);
> @@ -287,7 +295,7 @@ static void mxc_flip_edge(struct mxc_gpio_port *port, u32 gpio)
> gpio, edge);
> return;
> }
> - writel(val | (edge << (bit << 1)), reg);
> + icr_update_edge(port, gpio, edge);
> }
>
> /* handle 32 interrupts in one status register */
>
> --
> 2.51.0
>
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v5 13/13] gpio: mxc: use BIT() macro for single-bit operations
2026-10-09 18:05 [PATCH v5 00/13] gpio: mxc: bug fixes and probe cleanup Peng Fan (OSS)
` (11 preceding siblings ...)
2026-10-09 18:05 ` [PATCH v5 12/13] gpio: mxc: introduce MXC_ICR macros and use field_prep/field_get Peng Fan (OSS)
@ 2026-10-09 18:05 ` Peng Fan (OSS)
12 siblings, 0 replies; 27+ messages in thread
From: Peng Fan (OSS) @ 2026-10-09 18:05 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski, Frank Li, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Shenwei Wang,
Andy Shevchenko
Cc: linux-gpio, imx, linux-arm-kernel, linux-kernel, Peng Fan
From: Peng Fan <peng.fan@nxp.com>
Replace open-coded '1 << n' shifts with the BIT() macro throughout
the driver for consistency and to avoid potential signed-shift issues
when the bit index is 31 (1 << 31 is implementation-defined for
signed int).
No functional change.
Reviewed-by: Linus Walleij <linusw@kernel.org>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
drivers/gpio/gpio-mxc.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
index 97147a5d747b..4140fb2359c8 100644
--- a/drivers/gpio/gpio-mxc.c
+++ b/drivers/gpio/gpio-mxc.c
@@ -260,17 +260,17 @@ static int gpio_set_irq_type(struct irq_data *d, u32 type)
if (GPIO_EDGE_SEL >= 0) {
val = readl(port->base + GPIO_EDGE_SEL);
if (edge == GPIO_INT_BOTH_EDGES)
- writel(val | (1 << gpio_idx),
+ writel(val | BIT(gpio_idx),
port->base + GPIO_EDGE_SEL);
else
- writel(val & ~(1 << gpio_idx),
+ writel(val & ~BIT(gpio_idx),
port->base + GPIO_EDGE_SEL);
}
if (edge != GPIO_INT_BOTH_EDGES)
icr_update_edge(port, gpio_idx, edge);
- writel(1 << gpio_idx, port->base + GPIO_ISR);
+ writel(BIT(gpio_idx), port->base + GPIO_ISR);
port->pad_type[gpio_idx] = type;
}
@@ -309,7 +309,7 @@ static void mxc_gpio_irq_handler(struct mxc_gpio_port *port, u32 irq_stat)
generic_handle_domain_irq(port->domain, irqoffset);
- irq_stat &= ~(1 << irqoffset);
+ irq_stat &= ~BIT(irqoffset);
}
}
--
2.51.0
^ permalink raw reply related [flat|nested] 27+ messages in thread