linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH] remoteproc: core: Remove state checking before changing state
@ 2022-03-25  3:44 S.J. Wang
  2022-03-25 17:22 ` Mathieu Poirier
  0 siblings, 1 reply; 5+ messages in thread
From: S.J. Wang @ 2022-03-25  3:44 UTC (permalink / raw)
  To: Mathieu Poirier
  Cc: bjorn.andersson, linux-remoteproc, linux-kernel, shengjiu.wang

Hi

> >
> > 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
> 

Thanks for the comments.
I still have one question, if there are two instances independently to 'start'
'stop' remoteproc, for example:

Instance1: echo start > /sys/class/remoteproc/remoteproc0/state
Instance2: echo start > /sys/class/remoteproc/remoteproc0/state

...

Instance2: echo stop > /sys/class/remoteproc/remoteproc0/state
...
Instance1: echo stop > /sys/class/remoteproc/remoteproc0/state

When instance2 'stop' the remoteproc, then instance1 will be impacted for
It still needs the service from remoteproc.

That's why I just removed of the checking state, didn't move them to
rproc_boot()/rproc_shutdown()/rproc_detach(). And in order to utilize
the reference counter (rproc->power) to handle the multi-instance case.


Best regards
Wang Shengjiu

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] remoteproc: core: Remove state checking before changing state
  2022-03-25  3:44 [PATCH] remoteproc: core: Remove state checking before changing state S.J. Wang
@ 2022-03-25 17:22 ` Mathieu Poirier
  0 siblings, 0 replies; 5+ messages in thread
From: Mathieu Poirier @ 2022-03-25 17:22 UTC (permalink / raw)
  To: S.J. Wang; +Cc: bjorn.andersson, linux-remoteproc, linux-kernel, shengjiu.wang

On Fri, Mar 25, 2022 at 03:44:20AM +0000, S.J. Wang wrote:
> Hi
> 
> > >
> > > 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
> > 
> 
> Thanks for the comments.
> I still have one question, if there are two instances independently to 'start'
> 'stop' remoteproc, for example:
> 
> Instance1: echo start > /sys/class/remoteproc/remoteproc0/state
> Instance2: echo start > /sys/class/remoteproc/remoteproc0/state
> 
> ...
> 
> Instance2: echo stop > /sys/class/remoteproc/remoteproc0/state
> ...
> Instance1: echo stop > /sys/class/remoteproc/remoteproc0/state
> 
> When instance2 'stop' the remoteproc, then instance1 will be impacted for
> It still needs the service from remoteproc.
> 
> That's why I just removed of the checking state, didn't move them to
> rproc_boot()/rproc_shutdown()/rproc_detach(). And in order to utilize
> the reference counter (rproc->power) to handle the multi-instance case.

Thanks for the exta information, now I understand the problem.  The above should
be part of the changelog.

There are two problems here:

1) Dealing with race conditions when checking the state of a remote processor.
2) Properly dealing with the remote processor's reference count.

For the first one, state checks control the remote processor state machine and
can't simply be removed.  They have to be brought inside the mutex lock in order
to avoid race conditions when multiple users interact with the remote processor.

For the second one, moving the rproc->state checks inside the mutex lock in
rproc_shutdown() and rproc_detach() will work with the rproc->power check.

The problem is with rproc_boot().  For (at least) two years now we have lost
the capability to increase the rproc->power reference count from sysfs and the
cdev interface due to the rproc-state checks in state_store() and
rproc_cdev_write().  I think that should be corrected but it will introduce a
user space visible change, which needs to be treated carefully.

With the above in mind, please send a patchset that includes one patch that
removes the rproc->state checks from the "start" command in state_store() and
rproc_cdev_write().  The other patch should move the rproc->state checks for the
"stop" and "detach" command inside rproc_shutdown() and rproc_detach().

With that we can start a discussion on the best way to proceed.

Thanks,
Mathieu

> 
> 
> Best regards
> Wang Shengjiu

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] remoteproc: core: Remove state checking before changing state
@ 2022-03-28  2:00 S.J. Wang
  0 siblings, 0 replies; 5+ messages in thread
From: S.J. Wang @ 2022-03-28  2:00 UTC (permalink / raw)
  To: Mathieu Poirier
  Cc: bjorn.andersson, linux-remoteproc, linux-kernel, shengjiu.wang

Hi

> > > >
> > > > 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
> > >
> >
> > Thanks for the comments.
> > I still have one question, if there are two instances independently to 'start'
> > 'stop' remoteproc, for example:
> >
> > Instance1: echo start > /sys/class/remoteproc/remoteproc0/state
> > Instance2: echo start > /sys/class/remoteproc/remoteproc0/state
> >
> > ...
> >
> > Instance2: echo stop > /sys/class/remoteproc/remoteproc0/state
> > ...
> > Instance1: echo stop > /sys/class/remoteproc/remoteproc0/state
> >
> > When instance2 'stop' the remoteproc, then instance1 will be impacted
> > for It still needs the service from remoteproc.
> >
> > That's why I just removed of the checking state, didn't move them to
> > rproc_boot()/rproc_shutdown()/rproc_detach(). And in order to utilize
> > the reference counter (rproc->power) to handle the multi-instance case.
> 
> Thanks for the exta information, now I understand the problem.  The above
> should be part of the changelog.
> 
> There are two problems here:
> 
> 1) Dealing with race conditions when checking the state of a remote
> processor.
> 2) Properly dealing with the remote processor's reference count.
> 
> For the first one, state checks control the remote processor state machine
> and can't simply be removed.  They have to be brought inside the mutex lock
> in order to avoid race conditions when multiple users interact with the
> remote processor.
> 
> For the second one, moving the rproc->state checks inside the mutex lock in
> rproc_shutdown() and rproc_detach() will work with the rproc->power check.
> 
> The problem is with rproc_boot().  For (at least) two years now we have lost
> the capability to increase the rproc->power reference count from sysfs and
> the cdev interface due to the rproc-state checks in state_store() and
> rproc_cdev_write().  I think that should be corrected but it will introduce a
> user space visible change, which needs to be treated carefully.
> 
> With the above in mind, please send a patchset that includes one patch that
> removes the rproc->state checks from the "start" command in state_store()
> and rproc_cdev_write().  The other patch should move the rproc->state
> checks for the "stop" and "detach" command inside rproc_shutdown() and
> rproc_detach().
> 
> With that we can start a discussion on the best way to proceed.
> 

Ok, thanks,  will send the patches.

Best regards
Wang Shengjiu

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] remoteproc: core: Remove state checking before changing state
  2022-03-23  9:32 Shengjiu Wang
@ 2022-03-24 16:53 ` Mathieu Poirier
  0 siblings, 0 replies; 5+ messages in thread
From: Mathieu Poirier @ 2022-03-24 16:53 UTC (permalink / raw)
  To: Shengjiu Wang
  Cc: bjorn.andersson, linux-remoteproc, linux-kernel, shengjiu.wang

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
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH] remoteproc: core: Remove state checking before changing state
@ 2022-03-23  9:32 Shengjiu Wang
  2022-03-24 16:53 ` Mathieu Poirier
  0 siblings, 1 reply; 5+ messages in thread
From: Shengjiu Wang @ 2022-03-23  9:32 UTC (permalink / raw)
  To: bjorn.andersson, mathieu.poirier
  Cc: linux-remoteproc, linux-kernel, shengjiu.wang

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;
-
 		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);
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-03-28  2:00 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-25  3:44 [PATCH] remoteproc: core: Remove state checking before changing state S.J. Wang
2022-03-25 17:22 ` Mathieu Poirier
  -- strict thread matches above, loose matches on Subject: below --
2022-03-28  2:00 S.J. Wang
2022-03-23  9:32 Shengjiu Wang
2022-03-24 16:53 ` Mathieu Poirier

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).