Re: [PATCH 2/2] Revert "remoteproc: Add support for runtime PM"

[Suman Anna <s-anna@xxxxxx>]

Hi Mathieu,

On 6/30/20 11:31 AM, Mathieu Poirier wrote:
> This reverts commit a99a37f6cd5a74d5b22c08544aa6c5890813c8ba.
>
> Removing PM runtime operations from the remoteproc core in order to:
>
> 1) Keep all power management operations in platform drivers.  That way we
> do not loose flexibility in an area that is very HW specific.
>
> 2) Avoid making the support for remote processor managed by external
> entities more complex that it already is.
>
> 3) Fix regression introduced for the Omap remoteproc driver.

Thanks for following up on the discussion, I have verified that the 
autosuspend regression is fixed.

> Signed-off-by: Mathieu Poirier <mathieu.poirier@xxxxxxxxxx>

Tested-by: Suman Anna <s-anna@xxxxxx>
Acked-by: Suman Anna <s-anna@xxxxxx>

regards
Suman

> ---
>   drivers/remoteproc/remoteproc_core.c | 17 +----------------
>   1 file changed, 1 insertion(+), 16 deletions(-)
>
> diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
> index 9f04c30c4aaf..0f95e025ba03 100644
> --- a/drivers/remoteproc/remoteproc_core.c
> +++ b/drivers/remoteproc/remoteproc_core.c
> @@ -29,7 +29,6 @@
>   #include <linux/devcoredump.h>
>   #include <linux/rculist.h>
>   #include <linux/remoteproc.h>
> -#include <linux/pm_runtime.h>
>   #include <linux/iommu.h>
>   #include <linux/idr.h>
>   #include <linux/elf.h>
> @@ -1383,12 +1382,6 @@ static int rproc_fw_boot(struct rproc *rproc, const struct firmware *fw)
>   	if (ret)
>   		return ret;
>   
> -	ret = pm_runtime_get_sync(dev);
> -	if (ret < 0) {
> -		dev_err(dev, "pm_runtime_get_sync failed: %d\n", ret);
> -		return ret;
> -	}
> -
>   	dev_info(dev, "Booting fw image %s, size %zd\n", name, fw->size);
>   
>   	/*
> @@ -1398,7 +1391,7 @@ static int rproc_fw_boot(struct rproc *rproc, const struct firmware *fw)
>   	ret = rproc_enable_iommu(rproc);
>   	if (ret) {
>   		dev_err(dev, "can't enable iommu: %d\n", ret);
> -		goto put_pm_runtime;
> +		return ret;
>   	}
>   
>   	/* Prepare rproc for firmware loading if needed */
> @@ -1452,8 +1445,6 @@ static int rproc_fw_boot(struct rproc *rproc, const struct firmware *fw)
>   	rproc_unprepare_device(rproc);
>   disable_iommu:
>   	rproc_disable_iommu(rproc);
> -put_pm_runtime:
> -	pm_runtime_put(dev);
>   	return ret;
>   }
>   
> @@ -1891,8 +1882,6 @@ void rproc_shutdown(struct rproc *rproc)
>   
>   	rproc_disable_iommu(rproc);
>   
> -	pm_runtime_put(dev);
> -
>   	/* Free the copy of the resource table */
>   	kfree(rproc->cached_table);
>   	rproc->cached_table = NULL;
> @@ -2183,9 +2172,6 @@ struct rproc *rproc_alloc(struct device *dev, const char *name,
>   
>   	rproc->state = RPROC_OFFLINE;
>   
> -	pm_runtime_no_callbacks(&rproc->dev);
> -	pm_runtime_enable(&rproc->dev);
> -
>   	return rproc;
>   
>   put_device:
> @@ -2205,7 +2191,6 @@ EXPORT_SYMBOL(rproc_alloc);
>    */
>   void rproc_free(struct rproc *rproc)
>   {
> -	pm_runtime_disable(&rproc->dev);
>   	put_device(&rproc->dev);
>   }
>   EXPORT_SYMBOL(rproc_free);