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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CACRpkdasQEgsOEim5oFMBk-K24SzxYAR17jzuPR3_CTjYZ1hvA@mail.gmail.com>
Date:   Wed, 18 Nov 2020 01:38:50 +0100
From:   Linus Walleij <linus.walleij@...aro.org>
To:     Srinivas Neeli <srinivas.neeli@...inx.com>
Cc:     Bartosz Golaszewski <bgolaszewski@...libre.com>,
        Michal Simek <michal.simek@...inx.com>,
        Shubhrajyoti Datta <shubhrajyoti.datta@...inx.com>,
        sgoud@...inx.com, Robert Hancock <hancock@...systems.ca>,
        "open list:GPIO SUBSYSTEM" <linux-gpio@...r.kernel.org>,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        git@...inx.com
Subject: Re: [LINUX PATCH V3 7/9] gpio: gpio-xilinx: Add support for suspend
 and resume

Hi Srinivas,

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

> Add support for suspend and resume, pm runtime suspend and resume.
> Added free and request calls.
>
> Signed-off-by: Srinivas Neeli <srinivas.neeli@...inx.com>
> ---
> 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