[v3] gpio: mxc: add runtime pm support

Add runtime PM support and allow the GPIO controller to enter
into runtime suspend automatically when not in use to save power.
However, it will automatically resume and enable clocks when a
GPIO or IRQ is requested.

While putting the GPIO module itself into power saving mode may not
have an obvious impact on current dissipation, the function is necessary
because the GPIO module disables its clock when idle. This enables the
system an opportunity to power off the parent subsystem, and this conserves
more power. The typical i.MX8 SoC features up to 8 GPIO controllers, but
most of the controllers often remain unused.

Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com>
---
v3:
  - Improved the error handling logic in the probe() function. Thanks a
    lot to AndyS for reviewing and pointing out the issue.

v2:
  - improved the patch comments
  - using pm_runtime_resume_and_get to avoid reference count disbalance
    when errors
  - removed the __maybe_unused identifier

 drivers/gpio/gpio-mxc.c | 61 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 59 insertions(+), 2 deletions(-)

--
2.34.1

On Thu, Jul 6, 2023 at 5:56 PM Shenwei Wang <shenwei.wang@nxp.com> wrote:
>
> Add runtime PM support and allow the GPIO controller to enter
> into runtime suspend automatically when not in use to save power.
> However, it will automatically resume and enable clocks when a
> GPIO or IRQ is requested.
>
> While putting the GPIO module itself into power saving mode may not
> have an obvious impact on current dissipation, the function is necessary
> because the GPIO module disables its clock when idle. This enables the
> system an opportunity to power off the parent subsystem, and this conserves
> more power. The typical i.MX8 SoC features up to 8 GPIO controllers, but
> most of the controllers often remain unused.

Looks better, thank you.
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com>
> ---
> v3:
>   - Improved the error handling logic in the probe() function. Thanks a
>     lot to AndyS for reviewing and pointing out the issue.
>
> v2:
>   - improved the patch comments
>   - using pm_runtime_resume_and_get to avoid reference count disbalance
>     when errors
>   - removed the __maybe_unused identifier
>
>  drivers/gpio/gpio-mxc.c | 61 +++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 59 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
> index 9d0cec4b82a3..a9fb6bd9aa6f 100644
> --- a/drivers/gpio/gpio-mxc.c
> +++ b/drivers/gpio/gpio-mxc.c
> @@ -17,6 +17,7 @@
>  #include <linux/irqchip/chained_irq.h>
>  #include <linux/module.h>
>  #include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
>  #include <linux/slab.h>
>  #include <linux/spinlock.h>
>  #include <linux/syscore_ops.h>
> @@ -382,6 +383,23 @@ static int mxc_gpio_to_irq(struct gpio_chip *gc, unsigned offset)
>         return irq_find_mapping(port->domain, offset);
>  }
>
> +static int mxc_gpio_request(struct gpio_chip *chip, unsigned int offset)
> +{
> +       int ret;
> +
> +       ret = gpiochip_generic_request(chip, offset);
> +       if (ret)
> +               return ret;
> +
> +       return pm_runtime_resume_and_get(chip->parent);
> +}
> +
> +static void mxc_gpio_free(struct gpio_chip *chip, unsigned int offset)
> +{
> +       gpiochip_generic_free(chip, offset);
> +       pm_runtime_put(chip->parent);
> +}
> +
>  static int mxc_gpio_probe(struct platform_device *pdev)
>  {
>         struct device_node *np = pdev->dev.of_node;
> @@ -429,6 +447,10 @@ static int mxc_gpio_probe(struct platform_device *pdev)
>         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);
> +
>         /* disable the interrupt and clear the status */
>         writel(0, port->base + GPIO_IMR);
>         writel(~0, port->base + GPIO_ISR);
> @@ -459,8 +481,8 @@ static int mxc_gpio_probe(struct platform_device *pdev)
>         if (err)
>                 goto out_bgio;
>
> -       port->gc.request = gpiochip_generic_request;
> -       port->gc.free = gpiochip_generic_free;
> +       port->gc.request = mxc_gpio_request;
> +       port->gc.free = mxc_gpio_free;
>         port->gc.to_irq = mxc_gpio_to_irq;
>         port->gc.base = (pdev->id < 0) ? of_alias_get_id(np, "gpio") * 32 :
>                                              pdev->id * 32;
> @@ -482,6 +504,8 @@ static int mxc_gpio_probe(struct platform_device *pdev)
>                 goto out_bgio;
>         }
>
> +       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)
> @@ -490,12 +514,15 @@ static int mxc_gpio_probe(struct platform_device *pdev)
>         list_add_tail(&port->node, &mxc_gpio_ports);
>
>         platform_set_drvdata(pdev, port);
> +       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);
>         clk_disable_unprepare(port->clk);
>         dev_info(&pdev->dev, "%s failed with errno %d\n", __func__, err);
>         return err;
> @@ -572,6 +599,30 @@ static bool mxc_gpio_set_pad_wakeup(struct mxc_gpio_port *port, bool enable)
>         return ret;
>  }
>
> +static int mxc_gpio_runtime_suspend(struct device *dev)
> +{
> +       struct mxc_gpio_port *port = dev_get_drvdata(dev);
> +
> +       mxc_gpio_save_regs(port);
> +       clk_disable_unprepare(port->clk);
> +
> +       return 0;
> +}
> +
> +static int mxc_gpio_runtime_resume(struct device *dev)
> +{
> +       struct mxc_gpio_port *port = dev_get_drvdata(dev);
> +       int ret;
> +
> +       ret = clk_prepare_enable(port->clk);
> +       if (ret)
> +               return ret;
> +
> +       mxc_gpio_restore_regs(port);
> +
> +       return 0;
> +}
> +
>  static int __maybe_unused mxc_gpio_noirq_suspend(struct device *dev)
>  {
>         struct platform_device *pdev = to_platform_device(dev);
> @@ -597,14 +648,19 @@ static int __maybe_unused mxc_gpio_noirq_resume(struct device *dev)
>
>  static const struct dev_pm_ops mxc_gpio_dev_pm_ops = {
>         SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(mxc_gpio_noirq_suspend, mxc_gpio_noirq_resume)
> +       SET_RUNTIME_PM_OPS(mxc_gpio_runtime_suspend, mxc_gpio_runtime_resume, NULL)
>  };
>
>  static int mxc_gpio_syscore_suspend(void)
>  {
>         struct mxc_gpio_port *port;
> +       int ret;
>
>         /* walk through all ports */
>         list_for_each_entry(port, &mxc_gpio_ports, node) {
> +               ret = clk_prepare_enable(port->clk);
> +               if (ret)
> +                       return ret;
>                 mxc_gpio_save_regs(port);
>                 clk_disable_unprepare(port->clk);
>         }
> @@ -625,6 +681,7 @@ static void mxc_gpio_syscore_resume(void)
>                         return;
>                 }
>                 mxc_gpio_restore_regs(port);
> +               clk_disable_unprepare(port->clk);
>         }
>  }
>
> --
> 2.34.1
>

On Thu, Jul 6, 2023 at 4:56 PM Shenwei Wang <shenwei.wang@nxp.com> wrote:
>
> Add runtime PM support and allow the GPIO controller to enter
> into runtime suspend automatically when not in use to save power.
> However, it will automatically resume and enable clocks when a
> GPIO or IRQ is requested.
>
> While putting the GPIO module itself into power saving mode may not
> have an obvious impact on current dissipation, the function is necessary
> because the GPIO module disables its clock when idle. This enables the
> system an opportunity to power off the parent subsystem, and this conserves
> more power. The typical i.MX8 SoC features up to 8 GPIO controllers, but
> most of the controllers often remain unused.
>
> Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com>
> ---

Applied, thanks!

Bart