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

[Frank Li <Frank.li@oss.nxp.com>]

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