| Message ID | 1648027970-11488-1-git-send-email-shengjiu.wang@nxp.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | remoteproc: core: Remove state checking before changing state | expand |
On Wed, 23 Mar 2022 at 03:42, Shengjiu Wang <shengjiu.wang@nxp.com> wrote: > > There is no mutex protecting of these state checking, which > can't garantee there is no another instance is trying to do > same operation. > > The reference counter rproc->power is used to manage state > changing and there is mutex protection in each operation > function for multi instance case. > > So remove this state checking in rproc_cdev_write() and > state_store(), just let reference counter rproc->power to > manage the behaviors. > > Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com> > --- > drivers/remoteproc/remoteproc_cdev.c | 11 ----------- > drivers/remoteproc/remoteproc_sysfs.c | 11 ----------- > 2 files changed, 22 deletions(-) > > diff --git a/drivers/remoteproc/remoteproc_cdev.c b/drivers/remoteproc/remoteproc_cdev.c > index 906ff3c4dfdd..687f205fd70a 100644 > --- a/drivers/remoteproc/remoteproc_cdev.c > +++ b/drivers/remoteproc/remoteproc_cdev.c > @@ -32,21 +32,10 @@ static ssize_t rproc_cdev_write(struct file *filp, const char __user *buf, size_ > return -EFAULT; > > if (!strncmp(cmd, "start", len)) { > - if (rproc->state == RPROC_RUNNING || > - rproc->state == RPROC_ATTACHED) > - return -EBUSY; > - > ret = rproc_boot(rproc); > } else if (!strncmp(cmd, "stop", len)) { > - if (rproc->state != RPROC_RUNNING && > - rproc->state != RPROC_ATTACHED) > - return -EINVAL; > - > ret = rproc_shutdown(rproc); > } else if (!strncmp(cmd, "detach", len)) { > - if (rproc->state != RPROC_ATTACHED) > - return -EINVAL; > - > ret = rproc_detach(rproc); > } else { > dev_err(&rproc->dev, "Unrecognized option\n"); > diff --git a/drivers/remoteproc/remoteproc_sysfs.c b/drivers/remoteproc/remoteproc_sysfs.c > index 51a04bc6ba7a..8c7ea8922638 100644 > --- a/drivers/remoteproc/remoteproc_sysfs.c > +++ b/drivers/remoteproc/remoteproc_sysfs.c > @@ -194,23 +194,12 @@ static ssize_t state_store(struct device *dev, > int ret = 0; > > if (sysfs_streq(buf, "start")) { > - if (rproc->state == RPROC_RUNNING || > - rproc->state == RPROC_ATTACHED) > - return -EBUSY; > - As per my previous comment the above conditions need to be moved into rproc_boot(). > ret = rproc_boot(rproc); > if (ret) > dev_err(&rproc->dev, "Boot failed: %d\n", ret); > } else if (sysfs_streq(buf, "stop")) { > - if (rproc->state != RPROC_RUNNING && > - rproc->state != RPROC_ATTACHED) > - return -EINVAL; > - > ret = rproc_shutdown(rproc); > } else if (sysfs_streq(buf, "detach")) { > - if (rproc->state != RPROC_ATTACHED) > - return -EINVAL; > - This patch should have been part of a patch series with your other work sent on March 18th[1]. Thanks, Mathieu [1]. [PATCH] remoteproc: core: check rproc->power value before decreasing it > ret = rproc_detach(rproc); > } else { > dev_err(&rproc->dev, "Unrecognised option: %s\n", buf); > -- > 2.17.1 >
diff --git a/drivers/remoteproc/remoteproc_cdev.c b/drivers/remoteproc/remoteproc_cdev.c index 906ff3c4dfdd..687f205fd70a 100644 --- a/drivers/remoteproc/remoteproc_cdev.c +++ b/drivers/remoteproc/remoteproc_cdev.c @@ -32,21 +32,10 @@ static ssize_t rproc_cdev_write(struct file *filp, const char __user *buf, size_ return -EFAULT; if (!strncmp(cmd, "start", len)) { - if (rproc->state == RPROC_RUNNING || - rproc->state == RPROC_ATTACHED) - return -EBUSY; - ret = rproc_boot(rproc); } else if (!strncmp(cmd, "stop", len)) { - if (rproc->state != RPROC_RUNNING && - rproc->state != RPROC_ATTACHED) - return -EINVAL; - ret = rproc_shutdown(rproc); } else if (!strncmp(cmd, "detach", len)) { - if (rproc->state != RPROC_ATTACHED) - return -EINVAL; - ret = rproc_detach(rproc); } else { dev_err(&rproc->dev, "Unrecognized option\n"); diff --git a/drivers/remoteproc/remoteproc_sysfs.c b/drivers/remoteproc/remoteproc_sysfs.c index 51a04bc6ba7a..8c7ea8922638 100644 --- a/drivers/remoteproc/remoteproc_sysfs.c +++ b/drivers/remoteproc/remoteproc_sysfs.c @@ -194,23 +194,12 @@ static ssize_t state_store(struct device *dev, int ret = 0; if (sysfs_streq(buf, "start")) { - if (rproc->state == RPROC_RUNNING || - rproc->state == RPROC_ATTACHED) - return -EBUSY; - ret = rproc_boot(rproc); if (ret) dev_err(&rproc->dev, "Boot failed: %d\n", ret); } else if (sysfs_streq(buf, "stop")) { - if (rproc->state != RPROC_RUNNING && - rproc->state != RPROC_ATTACHED) - return -EINVAL; - ret = rproc_shutdown(rproc); } else if (sysfs_streq(buf, "detach")) { - if (rproc->state != RPROC_ATTACHED) - return -EINVAL; - ret = rproc_detach(rproc); } else { dev_err(&rproc->dev, "Unrecognised option: %s\n", buf);
There is no mutex protecting of these state checking, which can't garantee there is no another instance is trying to do same operation. The reference counter rproc->power is used to manage state changing and there is mutex protection in each operation function for multi instance case. So remove this state checking in rproc_cdev_write() and state_store(), just let reference counter rproc->power to manage the behaviors. Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com> --- drivers/remoteproc/remoteproc_cdev.c | 11 ----------- drivers/remoteproc/remoteproc_sysfs.c | 11 ----------- 2 files changed, 22 deletions(-)