diff mbox series

[3/9] gpio: mxc: cache compatible checks at probe time

Message ID 20261003-gpio-mxc-cleanup-v1-3-dad728ce27f2@nxp.com
State New
Headers show
Series gpio: mxc: bug fixes and probe cleanup | expand

Commit Message

Peng Fan Oct. 3, 2026, 12:06 p.m. UTC 
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.

Cache them as bool fields (has_pad_wakeup, is_imx8qm) in mxc_gpio_port
during probe, eliminating repeated device tree string comparisons in
the suspend/resume hot path.

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

Comments

Andy Shevchenko Oct. 3, 2026, 5:45 p.m. UTC | #1 
On Sat, Oct 3, 2026 at 3:09 PM Peng Fan (OSS) <peng.fan@oss.nxp.com> wrote:

> 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.
>
> Cache them as bool fields (has_pad_wakeup, is_imx8qm) in mxc_gpio_port
> during probe, eliminating repeated device tree string comparisons in
> the suspend/resume hot path.

...

>  {
> -       struct device_node *np = port->dev->of_node;
> -
> -       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);
> +       if (!port->has_pad_wakeup)
> +               return false;
>
> -       return false;
> +       return (gpiochip_generic_config(&port->gen_gc.gc, offset, conf) == 0);

Too many parentheses, also the semantic of 0 is not obvious. Better,
for example, this one

  int ret;
  ...
  ret = gpiochip_generic_config(...);
  if (ret)
    return false;

  return true;

>  }
Frank Li Oct. 4, 2026, 3:06 a.m. UTC | #2 
On Sat, Oct 03, 2026 at 08:06:45PM +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.
>
> Cache them as bool fields (has_pad_wakeup, is_imx8qm) in mxc_gpio_port
> during probe, eliminating repeated device tree string comparisons in
> the suspend/resume hot path.
>
> Assisted-by: LLM
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  drivers/gpio/gpio-mxc.c | 24 ++++++++++++++----------
>  1 file changed, 14 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
> index 8a755ac1af83..1c27232f6a80 100644
> --- a/drivers/gpio/gpio-mxc.c
> +++ b/drivers/gpio/gpio-mxc.c
> @@ -73,6 +73,8 @@ struct mxc_gpio_port {
>  	bool power_off;
>  	unsigned long wakeup_pads;
>  	bool is_pad_wakeup;
> +	bool has_pad_wakeup;
> +	bool is_imx8qm;
>  	u32 pad_type[32];
>  	const struct mxc_gpio_hwdata *hwdata;
>  };
> @@ -457,6 +459,14 @@ static int mxc_gpio_probe(struct platform_device *pdev)
>  	if (of_device_is_compatible(np, "fsl,imx7d-gpio"))
>  		port->power_off = true;
>
> +	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"))
> +		port->has_pad_wakeup = true;

can you move has_pad_wakeup/is_imx8qm in mxc_gpio_hw_data?

Frank

> +
> +	if (of_device_is_compatible(np, "fsl,imx8qm-gpio"))
> +		port->is_imx8qm = true;
> +
>  	pm_runtime_get_noresume(&pdev->dev);
>  	pm_runtime_set_active(&pdev->dev);
>  	pm_runtime_enable(&pdev->dev);
> @@ -570,15 +580,10 @@ 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;
> -
> -	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);
> +	if (!port->has_pad_wakeup)
> +		return false;
>
> -	return false;
> +	return (gpiochip_generic_config(&port->gen_gc.gc, offset, conf) == 0);
>  }
>
>  static bool mxc_gpio_set_pad_wakeup(struct mxc_gpio_port *port, bool enable)
> @@ -586,7 +591,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 */
> @@ -608,7 +612,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 (port->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;
>
> --
> 2.51.0
>
>
diff mbox series

Patch

diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
index 8a755ac1af83..1c27232f6a80 100644
--- a/drivers/gpio/gpio-mxc.c
+++ b/drivers/gpio/gpio-mxc.c
@@ -73,6 +73,8 @@  struct mxc_gpio_port {
 	bool power_off;
 	unsigned long wakeup_pads;
 	bool is_pad_wakeup;
+	bool has_pad_wakeup;
+	bool is_imx8qm;
 	u32 pad_type[32];
 	const struct mxc_gpio_hwdata *hwdata;
 };
@@ -457,6 +459,14 @@  static int mxc_gpio_probe(struct platform_device *pdev)
 	if (of_device_is_compatible(np, "fsl,imx7d-gpio"))
 		port->power_off = true;
 
+	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"))
+		port->has_pad_wakeup = true;
+
+	if (of_device_is_compatible(np, "fsl,imx8qm-gpio"))
+		port->is_imx8qm = true;
+
 	pm_runtime_get_noresume(&pdev->dev);
 	pm_runtime_set_active(&pdev->dev);
 	pm_runtime_enable(&pdev->dev);
@@ -570,15 +580,10 @@  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;
-
-	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);
+	if (!port->has_pad_wakeup)
+		return false;
 
-	return false;
+	return (gpiochip_generic_config(&port->gen_gc.gc, offset, conf) == 0);
 }
 
 static bool mxc_gpio_set_pad_wakeup(struct mxc_gpio_port *port, bool enable)
@@ -586,7 +591,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 */
@@ -608,7 +612,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 (port->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;