[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 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 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
>>
>>
>