On Fri, Jan 8, 2021 at 12:41 PM Srinivas Neeli [off-list ref] wrote:

quoted

On Wed, Jan 6, 2021 at 1:27 PM Srinivas Neeli [off-list ref]
wrote:

quoted

quoted

 /**
  * xgpio_remove - Remove method for the GPIO device.
  * @pdev: pointer to the platform device @@ -289,7 +323,10 @@ static
int xgpio_remove(struct platform_device *pdev)  {
        struct xgpio_instance *gpio = platform_get_drvdata(pdev);

-       clk_disable_unprepare(gpio->clk);
+       if (!pm_runtime_suspended(&pdev->dev))
+               clk_disable_unprepare(gpio->clk);
+
+       pm_runtime_disable(&pdev->dev);

This looks complex and racy. What if the device is resumed after you
executed the first part of the statement.

Could you please explain more on this.
What is the need to call pm_runtime_get_sync(); in remove API ?

I explain that on the lines right below your comment ;D

quoted

The normal sequence is:

pm_runtime_get_sync(dev);
pm_runtime_put_noidle(dev);
pm_runtime_disable(dev);

This will make sure the clock is enabled and pm runtime is disabled.
After this you can unconditionally call clk_disable_unprepare(gpio->clk);

^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Yours,
Linus Walleij