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

[Peng Fan <peng.fan@oss.nxp.com>]

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