Re: [LINUX PATCH V3 7/9] gpio: gpio-xilinx: Add support for suspend and resume

[Linus Walleij <linus.walleij@xxxxxxxxxx>]

Hi Srinivas,

On Thu, Nov 12, 2020 at 6:13 PM Srinivas Neeli
<srinivas.neeli@xxxxxxxxxx> wrote:

> Add support for suspend and resume, pm runtime suspend and resume.
> Added free and request calls.
>
> Signed-off-by: Srinivas Neeli <srinivas.neeli@xxxxxxxxxx>
> ---
> Changes in V3:
> -Created new patch for suspend and resume.

(...)

I'm following the idea here I think.

> @@ -544,6 +618,8 @@ static int xgpio_probe(struct platform_device *pdev)

> +       pm_runtime_enable(&pdev->dev);
> +       status = pm_runtime_get_sync(&pdev->dev);
> +       if (status < 0)
> +               goto err_unprepare_clk;

Now the clock is enabled a second time. Because
runtime PM kicks in.

Do this instead:
pm_runtime_get_noresume()
pm_runtime_set_active()
pm_runtime_enable()

Now runtime PM knows it is active and will not call
runtime resume and enable the clock a second time.

> +       pm_runtime_put(&pdev->dev);
>         return 0;

This is right, now pm runtime will gate the clock
until the first GPIO is requested.

> +err_pm_put:
> +       pm_runtime_put_sync(&pdev->dev);
>  err_unprepare_clk:
> +       pm_runtime_disable(&pdev->dev);
>         clk_disable_unprepare(chip->clk);
>         return status;

Use this on the errorpath instead:
pm_runtime_put_noidle()
pm_runtime_disable()
clk_disable_unprepare();

Now the code will not call runtime suspend to
gate the clock a second time.

Double-check the references to the clock and check
in debugfs that the clock really gets disabled if you're
not using any GPIOs.

Yours,
Linus Walleij