On 2/24/21 12:35 AM, Mathieu Poirier wrote:
> Refactor function rproc_del() and rproc_cdev_release() to take
> into account the current state of the remote processor when choosing
> the state to transition to.
>
> Signed-off-by: Mathieu Poirier <mathieu.poirier@xxxxxxxxxx>
> ---
> New for V6:
> - The RPROC_RUNNING -> RPROC_DETACHED transition is no longer permitted.
> to avoid dealing with complex resource table management problems.
> - Transition to the next state is no longer dictated by a DT binding for
> the same reason as above.
> - Removed Peng and Arnaud's RB tags because of the above.
> ---
>
> drivers/remoteproc/remoteproc_cdev.c | 10 ++++++++--
> drivers/remoteproc/remoteproc_core.c | 9 +++++++--
> 2 files changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/remoteproc/remoteproc_cdev.c b/drivers/remoteproc/remoteproc_cdev.c
> index 2db494816d5f..0b8a84c04f76 100644
> --- a/drivers/remoteproc/remoteproc_cdev.c
> +++ b/drivers/remoteproc/remoteproc_cdev.c
> @@ -86,11 +86,17 @@ static long rproc_device_ioctl(struct file *filp, unsigned int ioctl, unsigned l
> static int rproc_cdev_release(struct inode *inode, struct file *filp)
> {
> struct rproc *rproc = container_of(inode->i_cdev, struct rproc, cdev);
> + int ret = 0;
> +
> + if (!rproc->cdev_put_on_release)
> + return 0;
>
> - if (rproc->cdev_put_on_release && rproc->state == RPROC_RUNNING)
> + if (rproc->state == RPROC_RUNNING)
> rproc_shutdown(rproc);
> + else if (rproc->state == RPROC_ATTACHED)
> + ret = rproc_detach(rproc);
>
> - return 0;
> + return ret;
> }
>
> static const struct file_operations rproc_fops = {
> diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
> index 00452da25fba..a05d5fec43b1 100644
> --- a/drivers/remoteproc/remoteproc_core.c
> +++ b/drivers/remoteproc/remoteproc_core.c
> @@ -2542,11 +2542,16 @@ EXPORT_SYMBOL(rproc_put);
> */
> int rproc_del(struct rproc *rproc)
> {
> + int ret = 0;
> +
> if (!rproc)
> return -EINVAL;
>
> /* TODO: make sure this works with rproc->power > 1 */
> - rproc_shutdown(rproc);
> + if (rproc->state == RPROC_RUNNING)
> + rproc_shutdown(rproc);
> + else if (rproc->state == RPROC_ATTACHED)
> + ret = rproc_detach(rproc);
Here i would not update the code to not change the existing behavior of an
attached firmware.
The decision between a detach or a shutdown probably depends on platform.
We could (as a next step) reintroduce the "autonomous-on-core-reboot" DT
property for the decision.
Regards
Arnaud
>
> mutex_lock(&rproc->lock);
> rproc->state = RPROC_DELETED;
> @@ -2565,7 +2570,7 @@ int rproc_del(struct rproc *rproc)
>
> device_del(&rproc->dev);
>
> - return 0;
> + return ret;
> }
> EXPORT_SYMBOL(rproc_del);
>
>
