[PATCH v4 00/10] gpio: mxc: bug fixes and probe cleanup

[PATCH v4 00/10] gpio: mxc: bug fixes and probe cleanup

[Peng Fan (OSS)]

This series cleans up the gpio-mxc driver in several incremental steps:
bug fixes, converting runtime state to static per-compatible data,
modernizing resource management with devres, and cleaning up
register access patterns.

Patches 1-2 are bug fixes:
  - Fix a race where the chained IRQ handler is installed before probe
    completes, allowing interrupts to fire on a half-initialized port.
  - Fix wakeup_pads bit operations that used wrong set_bit/clear_bit
    logic, folding in the assign_bit() simplification from V1.

Patches 3-4 replace runtime of_device_is_compatible() calls with static
per-compatible hwdata flags, eliminating repeated string comparisons in
the suspend/resume path.

Patches 5-6 convert probe error handling to devres and dev_err_probe().

Patches 7-9 are cosmetic cleanups: local dev variable, MXC_ICR macros
with field_prep/field_get, and BIT() macro for single-bit shifts.

Signed-off-by: Peng Fan <peng.fan@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 (10):
      gpio: mxc: fix race between chained IRQ handler install and probe completion
      gpio: mxc: fix wakeup_pads bit operations
      gpio: mxc: use for_each_set_bit() to iterate wakeup pads
      gpio: mxc: replace of_device_is_compatible() with hwdata flags
      gpio: mxc: convert pad wakeup compatible checks to hwdata flags
      gpio: mxc: convert probe error handling to devres
      gpio: mxc: switch probe error paths to dev_err_probe()
      gpio: mxc: use local dev variable
      gpio: mxc: introduce MXC_ICR macros and use field_prep/field_get
      gpio: mxc: use BIT() macro for single-bit operations

 drivers/gpio/gpio-mxc.c | 256 +++++++++++++++++++++++++++---------------------
 1 file changed, 144 insertions(+), 112 deletions(-)
---
base-commit: f0406245cb9855e6318335a8a223551354291a46
change-id: 20261003-gpio-mxc-cleanup-e49cc626c51e

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

[PATCH v4 01/10] 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
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

[PATCH v4 02/10] 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, so we need to 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 member.
Change wakeup_pads to unsigned long and reorder to avoid the overlap.

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.

While at here, simplify the logic by consolidating into a single
irq_set_irq_wake() call based on the enable parameter.

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 | 25 ++++++++++---------------
 1 file changed, 10 insertions(+), 15 deletions(-)

diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
index e05f276a50e8..627fff6f1886 100644
--- a/drivers/gpio/gpio-mxc.c
+++ b/drivers/gpio/gpio-mxc.c
@@ -71,8 +71,8 @@ struct mxc_gpio_port {
 	u32 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;
 };
@@ -325,21 +325,16 @@ 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);
-		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 (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;
 
-	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

[PATCH v4 03/10] gpio: mxc: use for_each_set_bit() to iterate wakeup pads

[Peng Fan (OSS)]

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.

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 627fff6f1886..1ff5c2afb2e8 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

[PATCH v4 04/10] gpio: mxc: replace of_device_is_compatible() with hwdata flags

[Peng Fan (OSS)]

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.

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 1ff5c2afb2e8..c08f0b59d284 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;
 	u32 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

[PATCH v4 05/10] gpio: mxc: convert pad wakeup compatible checks to hwdata flags

[Peng Fan (OSS)]

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_IS_IMX8QM, 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 c08f0b59d284..5da603569d88 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_IS_IMX8QM		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_IS_IMX8QM | 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_is_imx8qm(struct mxc_gpio_port *port)
+{
+	return port->hwdata->flags & MXC_GPIO_IS_IMX8QM;
+}
+
+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_is_imx8qm(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

[PATCH v4 06/10] gpio: mxc: convert probe error handling to devres

[Peng Fan (OSS)]

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 | 49 +++++++++++++++++++++++++------------------------
 1 file changed, 25 insertions(+), 24 deletions(-)

diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
index 5da603569d88..54b09f1a4b50 100644
--- a/drivers/gpio/gpio-mxc.c
+++ b/drivers/gpio/gpio-mxc.c
@@ -449,6 +449,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 irq_domain_info d_info;
 	struct mxc_gpio_port *port;
 	int irq_count;
 	int irq_base;
@@ -484,9 +485,13 @@ static int mxc_gpio_probe(struct platform_device *pdev)
 	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);
+	err = devm_pm_runtime_get_noresume(&pdev->dev);
+	if (err)
+		return dev_err_probe(&pdev->dev, err, "Failed to get PM runtime\n");
+
+	err = devm_pm_runtime_set_active_enabled(&pdev->dev);
+	if (err)
+		return dev_err_probe(&pdev->dev, err, "Failed to enable PM runtime\n");
 
 	/* disable the interrupt and clear the status */
 	writel(0, port->base + GPIO_IMR);
@@ -512,7 +517,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;
@@ -528,27 +533,31 @@ static int mxc_gpio_probe(struct platform_device *pdev)
 
 	err = devm_gpiochip_add_data(&pdev->dev, &port->gen_gc.gc, port);
 	if (err)
-		goto out_bgio;
+		return err;
 
 	irq_base = devm_irq_alloc_descs(&pdev->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(&pdev->dev),
+		.size		= 32,
+		.hwirq_max	= 32,
+		.virq_base	= irq_base,
+		.ops		= &irq_domain_simple_ops,
+		.dev		= &pdev->dev,
+	};
 
-	port->domain = irq_domain_create_legacy(dev_fwnode(&pdev->dev), 32, irq_base, 0,
-						&irq_domain_simple_ops, NULL);
-	if (!port->domain) {
-		err = -ENODEV;
-		goto out_bgio;
-	}
+	port->domain = devm_irq_domain_instantiate(&pdev->dev, &d_info);
+	if (IS_ERR(port->domain))
+		return PTR_ERR(port->domain);
 
 	irq_domain_set_pm_device(port->domain, &pdev->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);
 
@@ -558,14 +567,6 @@ static int mxc_gpio_probe(struct platform_device *pdev)
 	pm_runtime_put_autosuspend(&pdev->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);
-	return err;
 }
 
 static void mxc_gpio_save_regs(struct mxc_gpio_port *port)

-- 
2.51.0

[PATCH v4 07/10] gpio: mxc: switch probe error paths to dev_err_probe()

[Peng Fan (OSS)]

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 54b09f1a4b50..f5f9ef1564cd 100644
--- a/drivers/gpio/gpio-mxc.c
+++ b/drivers/gpio/gpio-mxc.c
@@ -517,7 +517,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(&pdev->dev, err, "Failed to init gpio chip\n");
 
 	port->gen_gc.gc.request = mxc_gpio_request;
 	port->gen_gc.gc.free = mxc_gpio_free;
@@ -533,11 +533,11 @@ static int mxc_gpio_probe(struct platform_device *pdev)
 
 	err = devm_gpiochip_add_data(&pdev->dev, &port->gen_gc.gc, port);
 	if (err)
-		return err;
+		return dev_err_probe(&pdev->dev, err, "Failed to add gpiochip data\n");
 
 	irq_base = devm_irq_alloc_descs(&pdev->dev, -1, 0, 32, numa_node_id());
 	if (irq_base < 0)
-		return irq_base;
+		return dev_err_probe(&pdev->dev, irq_base, "Failed to alloc irq desc\n");
 
 	d_info = (struct irq_domain_info) {
 		.fwnode		= dev_fwnode(&pdev->dev),
@@ -550,14 +550,15 @@ static int mxc_gpio_probe(struct platform_device *pdev)
 
 	port->domain = devm_irq_domain_instantiate(&pdev->dev, &d_info);
 	if (IS_ERR(port->domain))
-		return PTR_ERR(port->domain);
+		return dev_err_probe(&pdev->dev, PTR_ERR(port->domain),
+				     "Failed to create irq domain\n");
 
 	irq_domain_set_pm_device(port->domain, &pdev->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(&pdev->dev, err, "Failed to init generic irq chip\n");
 
 	list_add_tail(&port->node, &mxc_gpio_ports);
 

-- 
2.51.0

[PATCH v4 08/10] gpio: mxc: use local dev variable

[Peng Fan (OSS)]

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.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/gpio/gpio-mxc.c | 43 ++++++++++++++++++++++---------------------
 1 file changed, 22 insertions(+), 21 deletions(-)

diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
index f5f9ef1564cd..9d69524e06db 100644
--- a/drivers/gpio/gpio-mxc.c
+++ b/drivers/gpio/gpio-mxc.c
@@ -449,18 +449,19 @@ 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;
 	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))
@@ -481,17 +482,17 @@ 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);
 
-	err = devm_pm_runtime_get_noresume(&pdev->dev);
+	err = devm_pm_runtime_get_noresume(dev);
 	if (err)
-		return dev_err_probe(&pdev->dev, err, "Failed to get PM runtime\n");
+		return dev_err_probe(dev, err, "Failed to get PM runtime\n");
 
-	err = devm_pm_runtime_set_active_enabled(&pdev->dev);
+	err = devm_pm_runtime_set_active_enabled(dev);
 	if (err)
-		return dev_err_probe(&pdev->dev, err, "Failed to enable PM runtime\n");
+		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);
@@ -508,7 +509,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;
@@ -517,7 +518,7 @@ static int mxc_gpio_probe(struct platform_device *pdev)
 
 	err = gpio_generic_chip_init(&port->gen_gc, &config);
 	if (err)
-		return dev_err_probe(&pdev->dev, err, "Failed to init gpio chip\n");
+		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;
@@ -531,41 +532,41 @@ 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)
-		return dev_err_probe(&pdev->dev, err, "Failed to add gpiochip data\n");
+		return dev_err_probe(dev, err, "Failed to add gpiochip data\n");
 
-	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)
-		return dev_err_probe(&pdev->dev, irq_base, "Failed to alloc irq desc\n");
+		return dev_err_probe(dev, irq_base, "Failed to alloc irq desc\n");
 
 	d_info = (struct irq_domain_info) {
-		.fwnode		= dev_fwnode(&pdev->dev),
+		.fwnode		= dev_fwnode(dev),
 		.size		= 32,
 		.hwirq_max	= 32,
 		.virq_base	= irq_base,
 		.ops		= &irq_domain_simple_ops,
-		.dev		= &pdev->dev,
+		.dev		= dev,
 	};
 
-	port->domain = devm_irq_domain_instantiate(&pdev->dev, &d_info);
+	port->domain = devm_irq_domain_instantiate(dev, &d_info);
 	if (IS_ERR(port->domain))
-		return dev_err_probe(&pdev->dev, 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, &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);
 	if (err < 0)
-		return dev_err_probe(&pdev->dev, err, "Failed to init generic irq chip\n");
+		return dev_err_probe(dev, err, "Failed to init generic irq chip\n");
 
 	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);
+	pm_runtime_put_autosuspend(dev);
 
 	return 0;
 }

-- 
2.51.0

[PATCH v4 09/10] gpio: mxc: introduce MXC_ICR macros and use field_prep/field_get

[Peng Fan (OSS)]

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).

Use field_prep() and field_get() from linux/bitfield.h for the
shift/extract operations instead of open-coded shifts. The lowercase
variants accept runtime-computed masks.

This eliminates the intermediate 'bit' variable from both functions and
makes the register access pattern self-documenting.

No functional change.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/gpio/gpio-mxc.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
index 9d69524e06db..39de616cd434 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 },
@@ -200,7 +204,7 @@ 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;
@@ -250,10 +254,9 @@ static int gpio_set_irq_type(struct irq_data *d, u32 type)
 		}
 
 		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);
+			reg += MXC_ICR_REG(gpio_idx);
+			val = readl(reg) & ~MXC_ICR_MASK(gpio_idx);
+			writel(val | field_prep(MXC_ICR_MASK(gpio_idx), edge), reg);
 		}
 
 		writel(1 << gpio_idx, port->base + GPIO_ISR);
@@ -266,16 +269,15 @@ 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;
+	u32 val;
 	int edge;
 
 	guard(gpio_generic_lock_irqsave)(&port->gen_gc);
 
-	reg += GPIO_ICR1 + ((gpio & 0x10) >> 2); /* lower or upper register */
-	bit = gpio & 0xf;
+	reg += MXC_ICR_REG(gpio);
 	val = readl(reg);
-	edge = (val >> (bit << 1)) & 3;
-	val &= ~(0x3 << (bit << 1));
+	edge = field_get(MXC_ICR_MASK(gpio), val);
+	val &= ~MXC_ICR_MASK(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 +289,7 @@ static void mxc_flip_edge(struct mxc_gpio_port *port, u32 gpio)
 		       gpio, edge);
 		return;
 	}
-	writel(val | (edge << (bit << 1)), reg);
+	writel(val | field_prep(MXC_ICR_MASK(gpio), edge), reg);
 }
 
 /* handle 32 interrupts in one status register */

-- 
2.51.0

[PATCH v4 10/10] gpio: mxc: use BIT() macro for single-bit operations

[Peng Fan (OSS)]

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>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/gpio/gpio-mxc.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
index 39de616cd434..ad50b602c4a1 100644
--- a/drivers/gpio/gpio-mxc.c
+++ b/drivers/gpio/gpio-mxc.c
@@ -209,7 +209,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);
+	port->both_edges &= ~BIT(gpio_idx);
 	switch (type) {
 	case IRQ_TYPE_EDGE_RISING:
 		edge = GPIO_INT_RISE_EDGE;
@@ -229,7 +229,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;
+			port->both_edges |= BIT(gpio_idx);
 		}
 		break;
 	case IRQ_TYPE_LEVEL_LOW:
@@ -246,10 +246,10 @@ 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);
 		}
 
@@ -259,7 +259,7 @@ static int gpio_set_irq_type(struct irq_data *d, u32 type)
 			writel(val | field_prep(MXC_ICR_MASK(gpio_idx), edge), reg);
 		}
 
-		writel(1 << gpio_idx, port->base + GPIO_ISR);
+		writel(BIT(gpio_idx), port->base + GPIO_ISR);
 		port->pad_type[gpio_idx] = type;
 	}
 
@@ -298,12 +298,12 @@ 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 (port->both_edges & BIT(irqoffset))
 			mxc_flip_edge(port, irqoffset);
 
 		generic_handle_domain_irq(port->domain, irqoffset);
 
-		irq_stat &= ~(1 << irqoffset);
+		irq_stat &= ~BIT(irqoffset);
 	}
 }
 

-- 
2.51.0

Re: [PATCH v4 10/10] gpio: mxc: use BIT() macro for single-bit operations

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] gpio: mxc: race condition in gpio_set_irq_type updating port->both_edges
--

commit 71b368728fc9ada85f870f46f968a6583a740b0c
Author: Peng Fan <peng.fan@nxp.com>

gpio: mxc: use BIT() macro for single-bit operations

This commit replaces open-coded shifts with the BIT() macro to avoid potential
signed-shift issues and improve consistency. No functional changes are
intended.

> diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
> index 39de616cd434f..ad50b602c4a16 100644
> --- a/drivers/gpio/gpio-mxc.c
> +++ b/drivers/gpio/gpio-mxc.c
> @@ -209,7 +209,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);
> +	port->both_edges &= ~BIT(gpio_idx);

[Severity: Medium]
Could this unprotected read-modify-write on the shared port bitmask lead to a
race condition?

Concurrent updates to the shared port->both_edges bitmask could cause one CPU
to overwrite and lose the IRQ edge configuration of another pin, potentially
leading to missed interrupts or stuck IRQ lines.

This can happen during parallel device probing or via userspace GPIO
configuration (e.g., libgpiod/sysfs) when two CPU cores concurrently call
gpio_set_irq_type() for different GPIO pins on the same port.

The IRQ subsystem serializes irq_set_type() on a per-pin basis (via
desc->lock), not per-port.

While the driver correctly takes the port-wide lock later in this function via
scoped_guard(gpio_generic_lock_irqsave, &port->gen_gc) for its MMIO register
updates, it performs read-modify-write operations on the shared
port->both_edges software mask completely outside of this critical section.

[ ... ]

> @@ -229,7 +229,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;
> +			port->both_edges |= BIT(gpio_idx);

[Severity: Medium]
This appears to be a second unprotected modification in the IRQ_TYPE_EDGE_BOTH
path.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261007-gpio-mxc-cleanup-v4-0-c9f5a8b50df9@nxp.com?part=10

Re: [PATCH v4 01/10] gpio: mxc: fix race between chained IRQ handler install and probe completion

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] gpio: mxc: data race on mxc_gpio_ports list during concurrent probe and shared IRQ
--

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

gpio: mxc: fix race between chained IRQ handler install and probe completion

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, it leads to a crash or missing port. This commit
moves the handler installation after all dependencies are ready.

> 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);
>  

[Severity: High]
For fsl,imx21-gpio devices, all ports share a single parent IRQ. After the
first port probes, the shared chained handler (mx2_gpio_irq_handler)
becomes active.

When a subsequent port is probed, list_add_tail() mutates the global
mxc_gpio_ports list without any spinlocks or RCU primitives.

If an interrupt fires concurrently on the shared parent IRQ, the handler
running on another CPU will traverse the actively mutating list:

drivers/gpio/gpio-mxc.c:mx2_gpio_irq_handler() {
    ...
    list_for_each_entry(port, &mxc_gpio_ports, node) {
    ...
}

Can this lockless traversal of a list undergoing non-atomic mutation cause
pointer corruption or an immediate crash?

> +	mxc_update_irq_chained_handler(port, true);
> +
>  	platform_set_drvdata(pdev, port);
>  	pm_runtime_put_autosuspend(&pdev->dev);
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261007-gpio-mxc-cleanup-v4-0-c9f5a8b50df9@nxp.com?part=1

Re: [PATCH v4 06/10] gpio: mxc: convert probe error handling to devres

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] pm: usage counter underflow due to mixed devm and manual autosuspend
--

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

    gpio: mxc: convert probe error handling to devres
    
    This patch converts the driver's probe error handling to use devres. It
    replaces irq_domain_create_legacy() with devm_irq_domain_instantiate()
    and updates the PM runtime initialization to use devm_ variants, eliminating
    the need for manual cleanup labels.

> diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
> index 5da603569d88d..54b09f1a4b509 100644
> --- a/drivers/gpio/gpio-mxc.c
> +++ b/drivers/gpio/gpio-mxc.c
> @@ -484,9 +485,13 @@ static int mxc_gpio_probe(struct platform_device *pdev)
>  	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);
> +	err = devm_pm_runtime_get_noresume(&pdev->dev);
> +	if (err)
> +		return dev_err_probe(&pdev->dev, err, "Failed to get PM runtime\n");
> +
> +	err = devm_pm_runtime_set_active_enabled(&pdev->dev);
> +	if (err)
> +		return dev_err_probe(&pdev->dev, err, "Failed to enable PM runtime\n");

[ ... ]

> @@ -558,14 +567,6 @@ static int mxc_gpio_probe(struct platform_device *pdev)
>  	pm_runtime_put_autosuspend(&pdev->dev);

[Severity: High]
Since devm_pm_runtime_get_noresume() increments the usage counter and queues
a devres action to drop it on unbind, will this manual call to
pm_runtime_put_autosuspend() cause an issue?

Because the devres action remains queued, when the device is unbound (e.g.,
via device tree overlay removal or hot-unplug), the devres cleanup will
execute and decrement the usage counter a second time. Could this cause a PM
usage counter underflow, corrupting the device and parent power state?

>  
>  	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);
> -	return err;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261007-gpio-mxc-cleanup-v4-0-c9f5a8b50df9@nxp.com?part=6

Re: [PATCH v4 07/10] gpio: mxc: switch probe error paths to dev_err_probe()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] pm: double put of runtime PM usage count on driver unbind
--

commit 30dda5fb34eeb7a2c552816b7e65724d4491a05c
Author: Peng Fan <peng.fan@nxp.com>

gpio: mxc: switch probe error paths to dev_err_probe()

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>

> diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
> index 54b09f1a4b509..f5f9ef1564cdf 100644
> --- a/drivers/gpio/gpio-mxc.c
> +++ b/drivers/gpio/gpio-mxc.c

[ ... ]

> @@ -550,14 +550,15 @@ static int mxc_gpio_probe(struct platform_device *pdev)
>  
>  	port->domain = devm_irq_domain_instantiate(&pdev->dev, &d_info);
>  	if (IS_ERR(port->domain))
> -		return PTR_ERR(port->domain);
> +		return dev_err_probe(&pdev->dev, PTR_ERR(port->domain),
> +				     "Failed to create irq domain\n");
>  
>  	irq_domain_set_pm_device(port->domain, &pdev->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(&pdev->dev, err, "Failed to init generic irq chip\n");
>  
>  	list_add_tail(&port->node, &mxc_gpio_ports);
>  

[Severity: Medium]
Since this series replaces pm_runtime_get_noresume() with
devm_pm_runtime_get_noresume(), does this leave an unbalanced devres action
on driver unbind?

Earlier in mxc_gpio_probe(), the devres-managed get is called:

	err = devm_pm_runtime_get_noresume(&pdev->dev);

But at the end of the probe function on success, the count is still manually
dropped:

	pm_runtime_put_autosuspend(&pdev->dev);

If the device is ever unbound (e.g., via DT overlay removal or parent device
unregistration), won't devres execute pm_runtime_put_noidle_action and drop
the reference a second time?

While pm_runtime_put_noidle() uses atomic_add_unless() to prevent underflow
if the count is exactly 0, could it erroneously steal a reference if the
count is > 0 (such as from an open sysfs file or an active child device)?

When the legitimate holder later drops their reference, wouldn't the count
then underflow and trigger a warning, breaking the device's runtime PM state?

Even if suppress_bind_attrs is used, does that only prevent manual sysfs
unbinds, rather than programmatic device removal?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261007-gpio-mxc-cleanup-v4-0-c9f5a8b50df9@nxp.com?part=7

Re: [PATCH v4 08/10] gpio: mxc: use local dev variable

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] pm: devres double-put of runtime PM reference on driver unbind
- [Medium] gpio: mxc: double clock disable on driver unbind due to devm_clk_get_optional_enabled and runtime PM
--

commit 22e651b0a155c89a26d690a7e46fed22ae9cc238
Author: Peng Fan <peng.fan@nxp.com>

gpio: mxc: use local dev variable

Introduce a local 'struct device *dev' variable to replace repeated
'&pdev->dev' dereferences throughout mxc_gpio_probe(), improving
readability.

> diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
> index f5f9ef1564cdf..9d69524e06dbf 100644
> --- a/drivers/gpio/gpio-mxc.c
> +++ b/drivers/gpio/gpio-mxc.c
[ ... ]
> @@ -481,17 +482,17 @@ 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);

[Severity: Medium]
Does this code lead to a double clock disable during driver unbind?

By using devm_clk_get_optional_enabled() here, a devres action is queued to
disable the clock when the device is unbound. However, when the device is
removed, the driver core calls pm_runtime_put_sync() to synchronously suspend
it, which triggers mxc_gpio_runtime_suspend() and disables the clock:

drivers/gpio/gpio-mxc.c:mxc_gpio_runtime_suspend() {
    ...
    clk_disable_unprepare(port->clk);
    ...
}

Afterwards, the devres cleanup sequence runs (leaving the device suspended)
and unconditionally executes the devm clock release action. Won't this call
clk_disable_unprepare() again on an already-disabled clock and trigger a
warning in the clock core?

>  	if (IS_ERR(port->clk))
>  		return PTR_ERR(port->clk);
>  
> -	err = devm_pm_runtime_get_noresume(&pdev->dev);
> +	err = devm_pm_runtime_get_noresume(dev);

[Severity: High]
Does this introduce a devres double-put of the runtime PM reference on driver
unbind?

Using devm_pm_runtime_get_noresume() acquires a runtime PM usage count and
queues a devres action (pm_runtime_put_noidle_action) to release it upon
unbind.

>  	if (err)
> -		return dev_err_probe(&pdev->dev, err, "Failed to get PM runtime\n");
> +		return dev_err_probe(dev, err, "Failed to get PM runtime\n");
[ ... ]
> @@ -531,41 +532,41 @@ 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);
> +	pm_runtime_put_autosuspend(dev);

[Severity: High]
Since gpio-mxc has no .remove callback to re-acquire the reference, won't
unbinding the driver (e.g. via unregistering the platform device) execute the
devres action queued by devm_pm_runtime_get_noresume(), causing a double-put?

Although pm_runtime_put_noidle() uses atomic_add_unless() to prevent the usage
counter from dropping mathematically below 0, this double-put could steal a
reference if another component is holding one. Can this accounting error cause
the device to prematurely runtime-suspend while still in use, leading to
unclocked hardware register access faults?

>  
>  	return 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261007-gpio-mxc-cleanup-v4-0-c9f5a8b50df9@nxp.com?part=8

Re: [PATCH v4 01/10] gpio: mxc: fix race between chained IRQ handler install and probe completion

[Bartosz Golaszewski]

On Wed, 7 Oct 2026 12:57:13 +0200, sashiko-bot@kernel.org said:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] gpio: mxc: data race on mxc_gpio_ports list during concurrent probe and shared IRQ
> --
>
> commit bd719d33a26c1f816073bbdb04f543bc3f5ed730
> Author: Peng Fan <peng.fan@nxp.com>
>
> gpio: mxc: fix race between chained IRQ handler install and probe completion
>
> 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, it leads to a crash or missing port. This commit
> moves the handler installation after all dependencies are ready.
>
>> 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);
>>
>
> [Severity: High]
> For fsl,imx21-gpio devices, all ports share a single parent IRQ. After the
> first port probes, the shared chained handler (mx2_gpio_irq_handler)
> becomes active.
>
> When a subsequent port is probed, list_add_tail() mutates the global
> mxc_gpio_ports list without any spinlocks or RCU primitives.
>

I think we should switch to list_add_tail_rcu() and list_for_each_entry_rcu()
first across the driver.

Bart

Re: [PATCH v4 06/10] gpio: mxc: convert probe error handling to devres

[Bartosz Golaszewski]

On Wed, 7 Oct 2026 12:59:26 +0200, sashiko-bot@kernel.org said:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] pm: usage counter underflow due to mixed devm and manual autosuspend
> --
>
> commit dc287cc46ea891b838f3b39a1fd6ed65d627a965
> Author: Peng Fan <peng.fan@nxp.com>
>
>     gpio: mxc: convert probe error handling to devres
>
>     This patch converts the driver's probe error handling to use devres. It
>     replaces irq_domain_create_legacy() with devm_irq_domain_instantiate()
>     and updates the PM runtime initialization to use devm_ variants, eliminating
>     the need for manual cleanup labels.
>
>> diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
>> index 5da603569d88d..54b09f1a4b509 100644
>> --- a/drivers/gpio/gpio-mxc.c
>> +++ b/drivers/gpio/gpio-mxc.c
>> @@ -484,9 +485,13 @@ static int mxc_gpio_probe(struct platform_device *pdev)
>>  	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);
>> +	err = devm_pm_runtime_get_noresume(&pdev->dev);
>> +	if (err)
>> +		return dev_err_probe(&pdev->dev, err, "Failed to get PM runtime\n");
>> +
>> +	err = devm_pm_runtime_set_active_enabled(&pdev->dev);
>> +	if (err)
>> +		return dev_err_probe(&pdev->dev, err, "Failed to enable PM runtime\n");
>
> [ ... ]
>
>> @@ -558,14 +567,6 @@ static int mxc_gpio_probe(struct platform_device *pdev)
>>  	pm_runtime_put_autosuspend(&pdev->dev);
>
> [Severity: High]
> Since devm_pm_runtime_get_noresume() increments the usage counter and queues
> a devres action to drop it on unbind, will this manual call to
> pm_runtime_put_autosuspend() cause an issue?
>
> Because the devres action remains queued, when the device is unbound (e.g.,
> via device tree overlay removal or hot-unplug), the devres cleanup will
> execute and decrement the usage counter a second time. Could this cause a PM
> usage counter underflow, corrupting the device and parent power state?
>

Sounds right, please remove this call.

Bart

Re: [PATCH v4 02/10] gpio: mxc: fix wakeup_pads bit operations

[Frank Li]

On Wed, Oct 07, 2026 at 06:44:17PM +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 we need to use atomic bitops when modifying
> wakeup_pads.

So Need to use atomic ..

>
> 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 member.

is_pad_wakeup is not member, that's local variable. so should be

"corrupting the adjacent is_pad_wakeup local variable"

> Change wakeup_pads to unsigned long and reorder to avoid the overlap.

reorder can't "avoid the overlay"

Change "corrupting the adjacent is_pad_wakeup" already avoid the overlap.


>
> 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.
>
> While at here, simplify the logic by consolidating into a single
> irq_set_irq_wake() call based on the enable parameter.

And use irq_set_irq_wake() simple code. this part need seperate patch.

Frank

>
> 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 | 25 ++++++++++---------------
>  1 file changed, 10 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
> index e05f276a50e8..627fff6f1886 100644
> --- a/drivers/gpio/gpio-mxc.c
> +++ b/drivers/gpio/gpio-mxc.c
> @@ -71,8 +71,8 @@ struct mxc_gpio_port {
>  	u32 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;
>  };
> @@ -325,21 +325,16 @@ 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);
> -		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 (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;
>
> -	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 v4 03/10] gpio: mxc: use for_each_set_bit() to iterate wakeup pads

[Frank Li]

On Wed, Oct 07, 2026 at 06:44:18PM +0800, Peng Fan (OSS) wrote:
> 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.
>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---

Reviewed-by: Frank Li <Frank.Li@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 627fff6f1886..1ff5c2afb2e8 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
>
>

Re: [PATCH v4 04/10] gpio: mxc: replace of_device_is_compatible() with hwdata flags

[Frank Li]

On Wed, Oct 07, 2026 at 06:44:19PM +0800, Peng Fan (OSS) wrote:
> 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.
>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---

Reviewed-by: Frank Li <Frank.Li@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 1ff5c2afb2e8..c08f0b59d284 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;
>  	u32 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
>
>

Re: [PATCH v4 05/10] gpio: mxc: convert pad wakeup compatible checks to hwdata flags

[Frank Li]

On Wed, Oct 07, 2026 at 06:44:20PM +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_IS_IMX8QM, 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 c08f0b59d284..5da603569d88 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_IS_IMX8QM		BIT(2)

Look like QM don't support fall edge

Can you use MXC_GPIO_FALL_EDGE_WAKEUP_BROKEN?

So it will be easy to know what feature missed for QM by hwdata.

Frank

>
>  /* 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_IS_IMX8QM | 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_is_imx8qm(struct mxc_gpio_port *port)
> +{
> +	return port->hwdata->flags & MXC_GPIO_IS_IMX8QM;
> +}
> +
> +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_is_imx8qm(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
>
>

Re: [PATCH v4 06/10] gpio: mxc: convert probe error handling to devres

[Frank Li]

On Wed, Oct 07, 2026 at 06:44:21PM +0800, Peng Fan (OSS) wrote:
> 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 | 49 +++++++++++++++++++++++++------------------------
>  1 file changed, 25 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
> index 5da603569d88..54b09f1a4b50 100644
> --- a/drivers/gpio/gpio-mxc.c
> +++ b/drivers/gpio/gpio-mxc.c
> @@ -449,6 +449,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 irq_domain_info d_info;
>  	struct mxc_gpio_port *port;
>  	int irq_count;
>  	int irq_base;
> @@ -484,9 +485,13 @@ static int mxc_gpio_probe(struct platform_device *pdev)
>  	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);
> +	err = devm_pm_runtime_get_noresume(&pdev->dev);
> +	if (err)
> +		return dev_err_probe(&pdev->dev, err, "Failed to get PM runtime\n");

look like needn't call devm_pm_runtime_Get_noresume() to pump ref count.

Frank

> +
> +	err = devm_pm_runtime_set_active_enabled(&pdev->dev);
> +	if (err)
> +		return dev_err_probe(&pdev->dev, err, "Failed to enable PM runtime\n");
>
>  	/* disable the interrupt and clear the status */
>  	writel(0, port->base + GPIO_IMR);
> @@ -512,7 +517,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;
> @@ -528,27 +533,31 @@ static int mxc_gpio_probe(struct platform_device *pdev)
>
>  	err = devm_gpiochip_add_data(&pdev->dev, &port->gen_gc.gc, port);
>  	if (err)
> -		goto out_bgio;
> +		return err;
>
>  	irq_base = devm_irq_alloc_descs(&pdev->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(&pdev->dev),
> +		.size		= 32,
> +		.hwirq_max	= 32,
> +		.virq_base	= irq_base,
> +		.ops		= &irq_domain_simple_ops,
> +		.dev		= &pdev->dev,
> +	};
>
> -	port->domain = irq_domain_create_legacy(dev_fwnode(&pdev->dev), 32, irq_base, 0,
> -						&irq_domain_simple_ops, NULL);
> -	if (!port->domain) {
> -		err = -ENODEV;
> -		goto out_bgio;
> -	}
> +	port->domain = devm_irq_domain_instantiate(&pdev->dev, &d_info);
> +	if (IS_ERR(port->domain))
> +		return PTR_ERR(port->domain);
>
>  	irq_domain_set_pm_device(port->domain, &pdev->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);
>
> @@ -558,14 +567,6 @@ static int mxc_gpio_probe(struct platform_device *pdev)
>  	pm_runtime_put_autosuspend(&pdev->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);
> -	return err;
>  }
>
>  static void mxc_gpio_save_regs(struct mxc_gpio_port *port)
>
> --
> 2.51.0
>
>

Re: [PATCH v4 07/10] gpio: mxc: switch probe error paths to dev_err_probe()

[Frank Li]

On Wed, Oct 07, 2026 at 06:44:22PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>

subject: add dev_err_probe() for error paths.

>
> Replace bare return statements with dev_err_probe() across all probe
> error paths for consistent diagnostics and deferred-probe support.

return err, suppose already support deferred-probe.

Add error just help diagnostics.

Frank

>
> 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 54b09f1a4b50..f5f9ef1564cd 100644
> --- a/drivers/gpio/gpio-mxc.c
> +++ b/drivers/gpio/gpio-mxc.c
> @@ -517,7 +517,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(&pdev->dev, err, "Failed to init gpio chip\n");
>
>  	port->gen_gc.gc.request = mxc_gpio_request;
>  	port->gen_gc.gc.free = mxc_gpio_free;
> @@ -533,11 +533,11 @@ static int mxc_gpio_probe(struct platform_device *pdev)
>
>  	err = devm_gpiochip_add_data(&pdev->dev, &port->gen_gc.gc, port);
>  	if (err)
> -		return err;
> +		return dev_err_probe(&pdev->dev, err, "Failed to add gpiochip data\n");
>
>  	irq_base = devm_irq_alloc_descs(&pdev->dev, -1, 0, 32, numa_node_id());
>  	if (irq_base < 0)
> -		return irq_base;
> +		return dev_err_probe(&pdev->dev, irq_base, "Failed to alloc irq desc\n");
>
>  	d_info = (struct irq_domain_info) {
>  		.fwnode		= dev_fwnode(&pdev->dev),
> @@ -550,14 +550,15 @@ static int mxc_gpio_probe(struct platform_device *pdev)
>
>  	port->domain = devm_irq_domain_instantiate(&pdev->dev, &d_info);
>  	if (IS_ERR(port->domain))
> -		return PTR_ERR(port->domain);
> +		return dev_err_probe(&pdev->dev, PTR_ERR(port->domain),
> +				     "Failed to create irq domain\n");
>
>  	irq_domain_set_pm_device(port->domain, &pdev->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(&pdev->dev, err, "Failed to init generic irq chip\n");
>
>  	list_add_tail(&port->node, &mxc_gpio_ports);
>
>
> --
> 2.51.0
>
>

Re: [PATCH v4 08/10] gpio: mxc: use local dev variable

[Frank Li]

On Wed, Oct 07, 2026 at 06:44:23PM +0800, Peng Fan (OSS) wrote:
> 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.
>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---

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

>  drivers/gpio/gpio-mxc.c | 43 ++++++++++++++++++++++---------------------
>  1 file changed, 22 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
> index f5f9ef1564cd..9d69524e06db 100644
> --- a/drivers/gpio/gpio-mxc.c
> +++ b/drivers/gpio/gpio-mxc.c
> @@ -449,18 +449,19 @@ 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;
>  	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))
> @@ -481,17 +482,17 @@ 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);
>
> -	err = devm_pm_runtime_get_noresume(&pdev->dev);
> +	err = devm_pm_runtime_get_noresume(dev);
>  	if (err)
> -		return dev_err_probe(&pdev->dev, err, "Failed to get PM runtime\n");
> +		return dev_err_probe(dev, err, "Failed to get PM runtime\n");
>
> -	err = devm_pm_runtime_set_active_enabled(&pdev->dev);
> +	err = devm_pm_runtime_set_active_enabled(dev);
>  	if (err)
> -		return dev_err_probe(&pdev->dev, err, "Failed to enable PM runtime\n");
> +		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);
> @@ -508,7 +509,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;
> @@ -517,7 +518,7 @@ static int mxc_gpio_probe(struct platform_device *pdev)
>
>  	err = gpio_generic_chip_init(&port->gen_gc, &config);
>  	if (err)
> -		return dev_err_probe(&pdev->dev, err, "Failed to init gpio chip\n");
> +		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;
> @@ -531,41 +532,41 @@ 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)
> -		return dev_err_probe(&pdev->dev, err, "Failed to add gpiochip data\n");
> +		return dev_err_probe(dev, err, "Failed to add gpiochip data\n");
>
> -	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)
> -		return dev_err_probe(&pdev->dev, irq_base, "Failed to alloc irq desc\n");
> +		return dev_err_probe(dev, irq_base, "Failed to alloc irq desc\n");
>
>  	d_info = (struct irq_domain_info) {
> -		.fwnode		= dev_fwnode(&pdev->dev),
> +		.fwnode		= dev_fwnode(dev),
>  		.size		= 32,
>  		.hwirq_max	= 32,
>  		.virq_base	= irq_base,
>  		.ops		= &irq_domain_simple_ops,
> -		.dev		= &pdev->dev,
> +		.dev		= dev,
>  	};
>
> -	port->domain = devm_irq_domain_instantiate(&pdev->dev, &d_info);
> +	port->domain = devm_irq_domain_instantiate(dev, &d_info);
>  	if (IS_ERR(port->domain))
> -		return dev_err_probe(&pdev->dev, 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, &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);
>  	if (err < 0)
> -		return dev_err_probe(&pdev->dev, err, "Failed to init generic irq chip\n");
> +		return dev_err_probe(dev, err, "Failed to init generic irq chip\n");
>
>  	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);
> +	pm_runtime_put_autosuspend(dev);
>
>  	return 0;
>  }
>
> --
> 2.51.0
>
>

Re: [PATCH v4 09/10] gpio: mxc: introduce MXC_ICR macros and use field_prep/field_get

[Frank Li]

On Wed, Oct 07, 2026 at 06:44:24PM +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).
>
> Use field_prep() and field_get() from linux/bitfield.h for the
> shift/extract operations instead of open-coded shifts. The lowercase
> variants accept runtime-computed masks.
>
> This eliminates the intermediate 'bit' variable from both functions and
> makes the register access pattern self-documenting.
>
> No functional change.
>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  drivers/gpio/gpio-mxc.c | 24 +++++++++++++-----------
>  1 file changed, 13 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
> index 9d69524e06db..39de616cd434 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 },
> @@ -200,7 +204,7 @@ 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;
> @@ -250,10 +254,9 @@ static int gpio_set_irq_type(struct irq_data *d, u32 type)
>  		}
>
>  		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);
> +			reg += MXC_ICR_REG(gpio_idx);
> +			val = readl(reg) & ~MXC_ICR_MASK(gpio_idx);
> +			writel(val | field_prep(MXC_ICR_MASK(gpio_idx), edge), reg);

look like have not simpify much. can you add help function, such as

update_edge(val, gpio_idex, edge).

move & ~MXC_ICR_MASK(gpio_idx) and val | field_prep() into it.

Frank

>  		}
>
>  		writel(1 << gpio_idx, port->base + GPIO_ISR);
> @@ -266,16 +269,15 @@ 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;
> +	u32 val;
>  	int edge;
>
>  	guard(gpio_generic_lock_irqsave)(&port->gen_gc);
>
> -	reg += GPIO_ICR1 + ((gpio & 0x10) >> 2); /* lower or upper register */
> -	bit = gpio & 0xf;
> +	reg += MXC_ICR_REG(gpio);
>  	val = readl(reg);
> -	edge = (val >> (bit << 1)) & 3;
> -	val &= ~(0x3 << (bit << 1));
> +	edge = field_get(MXC_ICR_MASK(gpio), val);
> +	val &= ~MXC_ICR_MASK(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 +289,7 @@ static void mxc_flip_edge(struct mxc_gpio_port *port, u32 gpio)
>  		       gpio, edge);
>  		return;
>  	}
> -	writel(val | (edge << (bit << 1)), reg);
> +	writel(val | field_prep(MXC_ICR_MASK(gpio), edge), reg);
>  }
>
>  /* handle 32 interrupts in one status register */
>
> --
> 2.51.0
>
>

Re: [PATCH v4 10/10] gpio: mxc: use BIT() macro for single-bit operations

[Frank Li]

On Wed, Oct 07, 2026 at 06:44:25PM +0800, Peng Fan (OSS) wrote:
> 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>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---

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

>  drivers/gpio/gpio-mxc.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
> index 39de616cd434..ad50b602c4a1 100644
> --- a/drivers/gpio/gpio-mxc.c
> +++ b/drivers/gpio/gpio-mxc.c
> @@ -209,7 +209,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);
> +	port->both_edges &= ~BIT(gpio_idx);
>  	switch (type) {
>  	case IRQ_TYPE_EDGE_RISING:
>  		edge = GPIO_INT_RISE_EDGE;
> @@ -229,7 +229,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;
> +			port->both_edges |= BIT(gpio_idx);
>  		}
>  		break;
>  	case IRQ_TYPE_LEVEL_LOW:
> @@ -246,10 +246,10 @@ 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);
>  		}
>
> @@ -259,7 +259,7 @@ static int gpio_set_irq_type(struct irq_data *d, u32 type)
>  			writel(val | field_prep(MXC_ICR_MASK(gpio_idx), edge), reg);
>  		}
>
> -		writel(1 << gpio_idx, port->base + GPIO_ISR);
> +		writel(BIT(gpio_idx), port->base + GPIO_ISR);
>  		port->pad_type[gpio_idx] = type;
>  	}
>
> @@ -298,12 +298,12 @@ 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 (port->both_edges & BIT(irqoffset))
>  			mxc_flip_edge(port, irqoffset);
>
>  		generic_handle_domain_irq(port->domain, irqoffset);
>
> -		irq_stat &= ~(1 << irqoffset);
> +		irq_stat &= ~BIT(irqoffset);
>  	}
>  }
>
>
> --
> 2.51.0
>
>

Re: [PATCH v4 01/10] gpio: mxc: fix race between chained IRQ handler install and probe completion

[Peng Fan]

Hi Bartosz,
On Thu, Oct 08, 2026 at 09:44:40AM +0000, Bartosz Golaszewski wrote:
>On Wed, 7 Oct 2026 12:57:13 +0200, sashiko-bot@kernel.org said:
>> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>> - [High] gpio: mxc: data race on mxc_gpio_ports list during concurrent probe and shared IRQ
>> --
>>
>> commit bd719d33a26c1f816073bbdb04f543bc3f5ed730
>> Author: Peng Fan <peng.fan@nxp.com>
>>
>> gpio: mxc: fix race between chained IRQ handler install and probe completion
>>
>> 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, it leads to a crash or missing port. This commit
>> moves the handler installation after all dependencies are ready.
>>
>>> 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);
>>>
>>
>> [Severity: High]
>> For fsl,imx21-gpio devices, all ports share a single parent IRQ. After the
>> first port probes, the shared chained handler (mx2_gpio_irq_handler)
>> becomes active.
>>
>> When a subsequent port is probed, list_add_tail() mutates the global
>> mxc_gpio_ports list without any spinlocks or RCU primitives.
>>
>
>I think we should switch to list_add_tail_rcu() and list_for_each_entry_rcu()
>first across the driver.

The sashiko report is false alarm per my understanding. imx21 only has
one arm32 uniprocessor. Another CPU does not exist.

Even the single-core IRQ preemption case is safe.
The only real concern on uniprocessor is a hardirq preempting list_add_tail()
mid-update. But list_add_tail() in the kernel uses
WRITE_ONCE(prev->next, new) as the final store, with a compiler barrier
ensuring new->next and new->prev are written first:
// __list_add():
next->prev = new;           // (1) backward link
new->next = next;           // (2) new's forward link → head
new->prev = prev;           // (3) new's backward link → old tail
WRITE_ONCE(prev->next, new); // (4) linearization point — makes new visible
mx2_gpio_irq_handler does forward-only traversal
(list_for_each_entry follows ->next). If the IRQ fires before step 4, the new
entry isn't visible - traversal sees the old list. If it fires after step 4,
new->next already points to head, so traversal terminates correctly.
Both cases are safe.

Thanks,
Peng

>
>Bart
>

Re: [PATCH v4 02/10] gpio: mxc: fix wakeup_pads bit operations

[Peng Fan]

On Thu, Oct 08, 2026 at 02:59:22PM -0500, Frank Li wrote:
>On Wed, Oct 07, 2026 at 06:44:17PM +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 we need to use atomic bitops when modifying
>> wakeup_pads.
>
>So Need to use atomic ..

Andy suggested: so we need
https://lore.kernel.org/all/CAHp75Ve+p6TMr7MwQw0orGdjxLgNBN8gfTw2SY+EKBBgCyYXgA@mail.gmail.com/

>
>>
>> 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 member.
>
>is_pad_wakeup is not member, that's local variable. so should be
>
>"corrupting the adjacent is_pad_wakeup local variable"
>
>> Change wakeup_pads to unsigned long and reorder to avoid the overlap.
>
>reorder can't "avoid the overlay"
>
>Change "corrupting the adjacent is_pad_wakeup" already avoid the overlap.
>
>
>>
>> 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.
>>
>> While at here, simplify the logic by consolidating into a single
>> irq_set_irq_wake() call based on the enable parameter.
>
>And use irq_set_irq_wake() simple code. this part need seperate patch.

Thanks,
Peng

>
>Frank
>
>>
>> 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 | 25 ++++++++++---------------
>>  1 file changed, 10 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
>> index e05f276a50e8..627fff6f1886 100644
>> --- a/drivers/gpio/gpio-mxc.c
>> +++ b/drivers/gpio/gpio-mxc.c
>> @@ -71,8 +71,8 @@ struct mxc_gpio_port {
>>  	u32 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;
>>  };
>> @@ -325,21 +325,16 @@ 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);
>> -		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 (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;
>>
>> -	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 v4 05/10] gpio: mxc: convert pad wakeup compatible checks to hwdata flags

[Peng Fan]

On Thu, Oct 08, 2026 at 03:10:03PM -0500, Frank Li wrote:
>On Wed, Oct 07, 2026 at 06:44:20PM +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_IS_IMX8QM, 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 c08f0b59d284..5da603569d88 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_IS_IMX8QM		BIT(2)
>
>Look like QM don't support fall edge
>
>Can you use MXC_GPIO_FALL_EDGE_WAKEUP_BROKEN?
>
>So it will be easy to know what feature missed for QM by hwdata.

ok. Fix in next version.

Thanks
Peng

>
>Frank
>
>>
>>  /* 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_IS_IMX8QM | 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_is_imx8qm(struct mxc_gpio_port *port)
>> +{
>> +	return port->hwdata->flags & MXC_GPIO_IS_IMX8QM;
>> +}
>> +
>> +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_is_imx8qm(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
>>
>>
>

Re: [PATCH v4 06/10] gpio: mxc: convert probe error handling to devres

[Peng Fan]

On Thu, Oct 08, 2026 at 03:12:43PM -0500, Frank Li wrote:
>On Wed, Oct 07, 2026 at 06:44:21PM +0800, Peng Fan (OSS) wrote:
>> 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 | 49 +++++++++++++++++++++++++------------------------
>>  1 file changed, 25 insertions(+), 24 deletions(-)
>>
>> diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
>> index 5da603569d88..54b09f1a4b50 100644
>> --- a/drivers/gpio/gpio-mxc.c
>> +++ b/drivers/gpio/gpio-mxc.c
>> @@ -449,6 +449,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 irq_domain_info d_info;
>>  	struct mxc_gpio_port *port;
>>  	int irq_count;
>>  	int irq_base;
>> @@ -484,9 +485,13 @@ static int mxc_gpio_probe(struct platform_device *pdev)
>>  	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);
>> +	err = devm_pm_runtime_get_noresume(&pdev->dev);
>> +	if (err)
>> +		return dev_err_probe(&pdev->dev, err, "Failed to get PM runtime\n");
>
>look like needn't call devm_pm_runtime_Get_noresume() to pump ref count.

We need pm_runtime_get_noresume to keep ref count.

Without it, the device is at usage_count = 0 immediately after
pm_runtime_enable() (inside devm_pm_runtime_set_active_enabled).
There's hardware access after that point - writel() to GPIO_IMR/GPIO_ISR,
and devm_irq_setup_generic_chip() inside mxc_gpio_init_gc().
More critically, devm_gpiochip_add_data() makes the chip visible to consumers
mid-probe. With async probing, a consumer on another bus could do:
mxc_gpio_request()  -> pm_runtime_resume_and_get()  -> usage_count 0->1
mxc_gpio_free()     -> pm_runtime_put()             -> usage_count 1->0 -> idle -> suspend
That triggers mxc_gpio_runtime_suspend() which calls clk_disable_unprepare()
while probe is still accessing registers. The get_noresume keeps
usage_count >= 1 through the entire probe, preventing that.

So the correct order:
pm_runtime_get_noresume(dev);            // hold active through probe
devm_pm_runtime_set_active_enabled(dev); // set active + enable (devres-managed)
... probe with HW access ...
pm_runtime_put_autosuspend(dev);         // release, allow idle/suspend

Regards
Peng
>
>Frank
>
>> +
>> +	err = devm_pm_runtime_set_active_enabled(&pdev->dev);
>> +	if (err)
>> +		return dev_err_probe(&pdev->dev, err, "Failed to enable PM runtime\n");
>>
>>  	/* disable the interrupt and clear the status */
>>  	writel(0, port->base + GPIO_IMR);
>> @@ -512,7 +517,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;
>> @@ -528,27 +533,31 @@ static int mxc_gpio_probe(struct platform_device *pdev)
>>
>>  	err = devm_gpiochip_add_data(&pdev->dev, &port->gen_gc.gc, port);
>>  	if (err)
>> -		goto out_bgio;
>> +		return err;
>>
>>  	irq_base = devm_irq_alloc_descs(&pdev->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(&pdev->dev),
>> +		.size		= 32,
>> +		.hwirq_max	= 32,
>> +		.virq_base	= irq_base,
>> +		.ops		= &irq_domain_simple_ops,
>> +		.dev		= &pdev->dev,
>> +	};
>>
>> -	port->domain = irq_domain_create_legacy(dev_fwnode(&pdev->dev), 32, irq_base, 0,
>> -						&irq_domain_simple_ops, NULL);
>> -	if (!port->domain) {
>> -		err = -ENODEV;
>> -		goto out_bgio;
>> -	}
>> +	port->domain = devm_irq_domain_instantiate(&pdev->dev, &d_info);
>> +	if (IS_ERR(port->domain))
>> +		return PTR_ERR(port->domain);
>>
>>  	irq_domain_set_pm_device(port->domain, &pdev->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);
>>
>> @@ -558,14 +567,6 @@ static int mxc_gpio_probe(struct platform_device *pdev)
>>  	pm_runtime_put_autosuspend(&pdev->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);
>> -	return err;
>>  }
>>
>>  static void mxc_gpio_save_regs(struct mxc_gpio_port *port)
>>
>> --
>> 2.51.0
>>
>>
>

Re: [PATCH v4 06/10] gpio: mxc: convert probe error handling to devres

[Peng Fan]

On Thu, Oct 08, 2026 at 04:47:48AM -0500, Bartosz Golaszewski wrote:
>On Wed, 7 Oct 2026 12:59:26 +0200, sashiko-bot@kernel.org said:
>> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>> - [High] pm: usage counter underflow due to mixed devm and manual autosuspend
>> --
>>
>> commit dc287cc46ea891b838f3b39a1fd6ed65d627a965
>> Author: Peng Fan <peng.fan@nxp.com>
>>
>>     gpio: mxc: convert probe error handling to devres
>>
>>     This patch converts the driver's probe error handling to use devres. It
>>     replaces irq_domain_create_legacy() with devm_irq_domain_instantiate()
>>     and updates the PM runtime initialization to use devm_ variants, eliminating
>>     the need for manual cleanup labels.
>>
>>> diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
>>> index 5da603569d88d..54b09f1a4b509 100644
>>> --- a/drivers/gpio/gpio-mxc.c
>>> +++ b/drivers/gpio/gpio-mxc.c
>>> @@ -484,9 +485,13 @@ static int mxc_gpio_probe(struct platform_device *pdev)
>>>  	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);
>>> +	err = devm_pm_runtime_get_noresume(&pdev->dev);
>>> +	if (err)
>>> +		return dev_err_probe(&pdev->dev, err, "Failed to get PM runtime\n");
>>> +
>>> +	err = devm_pm_runtime_set_active_enabled(&pdev->dev);
>>> +	if (err)
>>> +		return dev_err_probe(&pdev->dev, err, "Failed to enable PM runtime\n");
>>
>> [ ... ]
>>
>>> @@ -558,14 +567,6 @@ static int mxc_gpio_probe(struct platform_device *pdev)
>>>  	pm_runtime_put_autosuspend(&pdev->dev);
>>
>> [Severity: High]
>> Since devm_pm_runtime_get_noresume() increments the usage counter and queues
>> a devres action to drop it on unbind, will this manual call to
>> pm_runtime_put_autosuspend() cause an issue?
>>
>> Because the devres action remains queued, when the device is unbound (e.g.,
>> via device tree overlay removal or hot-unplug), the devres cleanup will
>> execute and decrement the usage counter a second time. Could this cause a PM
>> usage counter underflow, corrupting the device and parent power state?
>>
>
>Sounds right, please remove this call.

we need to keep it. Without it, the gpio port will never runs into suspended state.

I think it should be not be marked as High severity (:

The driver sets suppress_bind_attrs = true and has no .remove callback. So never
unbinds. Even if we do have unbind:
Probe success:
  devm_pm_runtime_get_noresume()     -> usage_count = 1, devres queues put_noidle
  devm_pm_runtime_set_active_enabled -> devres queues disable + set_suspended
  ...probe succeeds...
  pm_runtime_put_autosuspend()       -> usage_count = 0, device may suspend

Unbind (devres cleanup, reverse order):
  [1] pm_runtime_disable_action       -> PM runtime disabled
  [2] pm_runtime_set_suspended_action -> RPM_SUSPENDED
  [3] pm_runtime_put_noidle_action    -> usage_count = -1  <- underflow

With PM runtime disabled[1], pm_runtime_put_noidle() just does atomic_dec() on
a counter nobody reads - no idle/suspend work is queued, no parent power
state is touched.


So to make things a bit cleaner, we need use pm_runtime_get_resume(no devres),
and define __free for it.

DEFINE_FREE(pm_runtime_put_noidle, struct device *, if (_T) pm_runtime_put_noidle(_T))
Then in probe:
pm_runtime_get_noresume(dev);

struct device *pm_guard __free(pm_runtime_put_noidle) = dev;

err = devm_pm_runtime_set_active_enabled(dev);
if (err)
    return dev_err_probe(dev, err, "...");  // guard fires -> put_noidle

/* ... all error returns automatically balanced ... */

/* success — disarm guard, do the real put */
no_free_ptr(pm_guard);
pm_runtime_put_autosuspend(dev);
return 0;

Thanks,
Peng

>
>Bart
>

Re: [PATCH v4 10/10] gpio: mxc: use BIT() macro for single-bit operations

[Peng Fan]

On Wed, Oct 07, 2026 at 10:56:01AM +0000, sashiko-bot@kernel.org wrote:
>Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>- [Medium] gpio: mxc: race condition in gpio_set_irq_type updating port->both_edges
>--
>
>commit 71b368728fc9ada85f870f46f968a6583a740b0c
>Author: Peng Fan <peng.fan@nxp.com>
>
>gpio: mxc: use BIT() macro for single-bit operations
>
>This commit replaces open-coded shifts with the BIT() macro to avoid potential
>signed-shift issues and improve consistency. No functional changes are
>intended.
>
>> diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
>> index 39de616cd434f..ad50b602c4a16 100644
>> --- a/drivers/gpio/gpio-mxc.c
>> +++ b/drivers/gpio/gpio-mxc.c
>> @@ -209,7 +209,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);
>> +	port->both_edges &= ~BIT(gpio_idx);
>
>[Severity: Medium]
>Could this unprotected read-modify-write on the shared port bitmask lead to a
>race condition?

This is a pre-existing issue. I need to use a separate patch to address it.

Regards
Peng

>
>Concurrent updates to the shared port->both_edges bitmask could cause one CPU
>to overwrite and lose the IRQ edge configuration of another pin, potentially
>leading to missed interrupts or stuck IRQ lines.
>
>This can happen during parallel device probing or via userspace GPIO
>configuration (e.g., libgpiod/sysfs) when two CPU cores concurrently call
>gpio_set_irq_type() for different GPIO pins on the same port.
>
>The IRQ subsystem serializes irq_set_type() on a per-pin basis (via
>desc->lock), not per-port.
>
>While the driver correctly takes the port-wide lock later in this function via
>scoped_guard(gpio_generic_lock_irqsave, &port->gen_gc) for its MMIO register
>updates, it performs read-modify-write operations on the shared
>port->both_edges software mask completely outside of this critical section.
>
>[ ... ]
>
>> @@ -229,7 +229,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;
>> +			port->both_edges |= BIT(gpio_idx);
>
>[Severity: Medium]
>This appears to be a second unprotected modification in the IRQ_TYPE_EDGE_BOTH
>path.
>
>-- 
>Sashiko AI review · https://sashiko.dev/#/patchset/20261007-gpio-mxc-cleanup-v4-0-c9f5a8b50df9@nxp.com?part=10
>