2020-11-27 08:46:06

by Mathieu Poirier

[permalink] [raw]
Subject: [PATCH v3 00/15] remoteproc: Add support for detaching from rproc

Following the work done here [1], this set provides support for the
remoteproc core to release resources associated with a remote processor
without having to switch it off. That way a platform driver can be removed
or the application processor power cycled while the remote processor is
still operating.

Applies cleanly on rproc-next (c3c21b356505).

Thanks,
Mathieu

[1]. https://lkml.org/lkml/2020/7/14/1600

----
New for V3:
- Added RB from Arnaud where applicable.
- Reformatted comments about "detach" operation in struct rproc_ops.
- Fixed error path in rproc_shutdown().
- Fixed processing of "start" command in state_store() and rproc_cdev_write().
- Changed binding from "autonomous-on-core-reboot" to
"autonomous-on-core-shutdown".
- Wrote a proper YAML file for the binding.

Mathieu Poirier (15):
dt-bindings: remoteproc: Add bindind to support autonomous processors
remoteproc: Re-check state in rproc_shutdown()
remoteproc: Remove useless check in rproc_del()
remoteproc: Add new RPROC_ATTACHED state
remoteproc: Properly represent the attached state
remoteproc: Properly deal with a kernel panic when attached
remoteproc: Add new detach() remoteproc operation
remoteproc: Introduce function __rproc_detach()
remoteproc: Introduce function rproc_detach()
remoteproc: Rename function rproc_actuate()
remoteproc: Add return value to function rproc_shutdown()
remoteproc: Properly deal with a stop request when attached
remoteproc: Properly deal with a start request when attached
remoteproc: Properly deal with detach request
remoteproc: Refactor rproc delete and cdev release path

.../bindings/remoteproc/remoteproc-core.yaml | 25 +++
drivers/remoteproc/remoteproc_cdev.c | 27 ++-
drivers/remoteproc/remoteproc_core.c | 182 +++++++++++++++---
drivers/remoteproc/remoteproc_sysfs.c | 20 +-
include/linux/remoteproc.h | 18 +-
5 files changed, 225 insertions(+), 47 deletions(-)
create mode 100644 Documentation/devicetree/bindings/remoteproc/remoteproc-core.yaml

--
2.25.1


2020-11-27 08:46:16

by Mathieu Poirier

[permalink] [raw]
Subject: [PATCH v3 01/15] dt-bindings: remoteproc: Add bindind to support autonomous processors

This patch adds a binding to guide the remoteproc core on how to deal with
remote processors in two cases:

1) When an application holding a reference to a remote processor character
device interface crashes.

2) when the platform driver for a remote processor is removed.

In both cases if "autonomous-on-core-reboot" is specified in the remote
processor DT node, the remoteproc core will detach the remote processor
rather than switching it off.

Signed-off-by: Mathieu Poirier <[email protected]>
---
.../bindings/remoteproc/remoteproc-core.yaml | 25 +++++++++++++++++++
1 file changed, 25 insertions(+)
create mode 100644 Documentation/devicetree/bindings/remoteproc/remoteproc-core.yaml

diff --git a/Documentation/devicetree/bindings/remoteproc/remoteproc-core.yaml b/Documentation/devicetree/bindings/remoteproc/remoteproc-core.yaml
new file mode 100644
index 000000000000..3032734f42a3
--- /dev/null
+++ b/Documentation/devicetree/bindings/remoteproc/remoteproc-core.yaml
@@ -0,0 +1,25 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/remoteproc/remoteproc-core.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Binding for the remoteproc core applicable to all remote processors
+
+maintainers:
+ - Bjorn Andersson <[email protected]>
+ - Mathieu Poirier <[email protected]>
+
+description:
+ This document defines the binding recognised by the remoteproc core that can
+ be used by any remote processor in the subsystem.
+
+properties:
+ autonomous-on-core-reboot:
+ $ref: /schemas/types.yaml#/definitions/flag
+ description:
+ Used in two situations, i.e when a user space application releases the
+ handle it has on the remote processor's character driver interface and
+ when a remote processor's platform driver is being removed. If defined,
+ this flag instructs the remoteproc core to detach the remote processor
+ rather than turning it off.
--
2.25.1

2020-11-27 08:46:38

by Mathieu Poirier

[permalink] [raw]
Subject: [PATCH v3 04/15] remoteproc: Add new RPROC_ATTACHED state

Add a new RPROC_ATTACHED state to take into account scenarios
where the remoteproc core needs to attach to a remote processor
that is booted by another entity.

Signed-off-by: Mathieu Poirier <[email protected]>
Reviewed-by: Peng Fan <[email protected]>
Reviewed-by: Arnaud Pouliquen <[email protected]>
---
drivers/remoteproc/remoteproc_sysfs.c | 1 +
include/linux/remoteproc.h | 7 +++++--
2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/remoteproc/remoteproc_sysfs.c b/drivers/remoteproc/remoteproc_sysfs.c
index 1dbef895e65e..4b4aab0d4c4b 100644
--- a/drivers/remoteproc/remoteproc_sysfs.c
+++ b/drivers/remoteproc/remoteproc_sysfs.c
@@ -172,6 +172,7 @@ static const char * const rproc_state_string[] = {
[RPROC_RUNNING] = "running",
[RPROC_CRASHED] = "crashed",
[RPROC_DELETED] = "deleted",
+ [RPROC_ATTACHED] = "attached",
[RPROC_DETACHED] = "detached",
[RPROC_LAST] = "invalid",
};
diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index e8ac041c64d9..71d531c64dfd 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -403,6 +403,8 @@ struct rproc_ops {
* @RPROC_RUNNING: device is up and running
* @RPROC_CRASHED: device has crashed; need to start recovery
* @RPROC_DELETED: device is deleted
+ * @RPROC_ATTACHED: device has been booted by another entity and the core
+ * has attached to it
* @RPROC_DETACHED: device has been booted by another entity and waiting
* for the core to attach to it
* @RPROC_LAST: just keep this one at the end
@@ -419,8 +421,9 @@ enum rproc_state {
RPROC_RUNNING = 2,
RPROC_CRASHED = 3,
RPROC_DELETED = 4,
- RPROC_DETACHED = 5,
- RPROC_LAST = 6,
+ RPROC_ATTACHED = 5,
+ RPROC_DETACHED = 6,
+ RPROC_LAST = 7,
};

/**
--
2.25.1

2020-11-30 17:27:23

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v3 01/15] dt-bindings: remoteproc: Add bindind to support autonomous processors

On Thu, 26 Nov 2020 14:06:28 -0700, Mathieu Poirier wrote:
> This patch adds a binding to guide the remoteproc core on how to deal with
> remote processors in two cases:
>
> 1) When an application holding a reference to a remote processor character
> device interface crashes.
>
> 2) when the platform driver for a remote processor is removed.
>
> In both cases if "autonomous-on-core-reboot" is specified in the remote
> processor DT node, the remoteproc core will detach the remote processor
> rather than switching it off.
>
> Signed-off-by: Mathieu Poirier <[email protected]>
> ---
> .../bindings/remoteproc/remoteproc-core.yaml | 25 +++++++++++++++++++
> 1 file changed, 25 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/remoteproc/remoteproc-core.yaml
>


My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/remoteproc/remoteproc-core.yaml: 'additionalProperties' is a required property
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/remoteproc/remoteproc-core.yaml: ignoring, error in schema:
warning: no schema found in file: ./Documentation/devicetree/bindings/remoteproc/remoteproc-core.yaml


See https://patchwork.ozlabs.org/patch/1406889

The base for the patch is generally the last rc1. Any dependencies
should be noted.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

2020-11-30 17:37:52

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v3 01/15] dt-bindings: remoteproc: Add bindind to support autonomous processors

On Thu, Nov 26, 2020 at 02:06:28PM -0700, Mathieu Poirier wrote:
> This patch adds a binding to guide the remoteproc core on how to deal with
> remote processors in two cases:
>
> 1) When an application holding a reference to a remote processor character
> device interface crashes.
>
> 2) when the platform driver for a remote processor is removed.
>
> In both cases if "autonomous-on-core-reboot" is specified in the remote
> processor DT node, the remoteproc core will detach the remote processor
> rather than switching it off.
>
> Signed-off-by: Mathieu Poirier <[email protected]>
> ---
> .../bindings/remoteproc/remoteproc-core.yaml | 25 +++++++++++++++++++
> 1 file changed, 25 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/remoteproc/remoteproc-core.yaml
>
> diff --git a/Documentation/devicetree/bindings/remoteproc/remoteproc-core.yaml b/Documentation/devicetree/bindings/remoteproc/remoteproc-core.yaml
> new file mode 100644
> index 000000000000..3032734f42a3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/remoteproc/remoteproc-core.yaml
> @@ -0,0 +1,25 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/remoteproc/remoteproc-core.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: Binding for the remoteproc core applicable to all remote processors
> +
> +maintainers:
> + - Bjorn Andersson <[email protected]>
> + - Mathieu Poirier <[email protected]>
> +
> +description:
> + This document defines the binding recognised by the remoteproc core that can
> + be used by any remote processor in the subsystem.
> +
> +properties:
> + autonomous-on-core-reboot:
> + $ref: /schemas/types.yaml#/definitions/flag
> + description:
> + Used in two situations, i.e when a user space application releases the
> + handle it has on the remote processor's character driver interface and
> + when a remote processor's platform driver is being removed. If defined,
> + this flag instructs the remoteproc core to detach the remote processor
> + rather than turning it off.

Userspace? character driver? platform driver? remoteproc core? Please
explain this without OS specific terms.

Seems to me this would be implied by functionality the remote proc
provides.

Rob

2020-12-01 23:49:29

by Mathieu Poirier

[permalink] [raw]
Subject: Re: [PATCH v3 01/15] dt-bindings: remoteproc: Add bindind to support autonomous processors

Hi Rob,

On Mon, Nov 30, 2020 at 10:33:21AM -0700, Rob Herring wrote:
> On Thu, Nov 26, 2020 at 02:06:28PM -0700, Mathieu Poirier wrote:
> > This patch adds a binding to guide the remoteproc core on how to deal with
> > remote processors in two cases:
> >
> > 1) When an application holding a reference to a remote processor character
> > device interface crashes.
> >
> > 2) when the platform driver for a remote processor is removed.
> >
> > In both cases if "autonomous-on-core-reboot" is specified in the remote
> > processor DT node, the remoteproc core will detach the remote processor
> > rather than switching it off.
> >
> > Signed-off-by: Mathieu Poirier <[email protected]>
> > ---
> > .../bindings/remoteproc/remoteproc-core.yaml | 25 +++++++++++++++++++
> > 1 file changed, 25 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/remoteproc/remoteproc-core.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/remoteproc/remoteproc-core.yaml b/Documentation/devicetree/bindings/remoteproc/remoteproc-core.yaml
> > new file mode 100644
> > index 000000000000..3032734f42a3
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/remoteproc/remoteproc-core.yaml
> > @@ -0,0 +1,25 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: "http://devicetree.org/schemas/remoteproc/remoteproc-core.yaml#"
> > +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> > +
> > +title: Binding for the remoteproc core applicable to all remote processors
> > +
> > +maintainers:
> > + - Bjorn Andersson <[email protected]>
> > + - Mathieu Poirier <[email protected]>
> > +
> > +description:
> > + This document defines the binding recognised by the remoteproc core that can
> > + be used by any remote processor in the subsystem.
> > +
> > +properties:
> > + autonomous-on-core-reboot:
> > + $ref: /schemas/types.yaml#/definitions/flag
> > + description:
> > + Used in two situations, i.e when a user space application releases the
> > + handle it has on the remote processor's character driver interface and
> > + when a remote processor's platform driver is being removed. If defined,
> > + this flag instructs the remoteproc core to detach the remote processor
> > + rather than turning it off.
>
> Userspace? character driver? platform driver? remoteproc core? Please
> explain this without OS specific terms.

The remoteproc state machine is gaining in complexity and having technical terms
in the binding's description helps understand when and how it should be used. I
could make it more generic but that will lead to confusion and abuse. Should I
make it "rproc,autonomous-on-core-reboot" ?

>
> Seems to me this would be implied by functionality the remote proc
> provides.

Exactly - this binding is used by the remoteproc core itself. It is specified
in the remote processor DT nodes but the individual platform drivers don't do
anything with it - the core takes care of enacting the desired behavior on their
behalf. Otherwise each platform driver would end up adding the same code, which
nobody wants to see happening.

How do you want me to proceed?

Thanks,
Mathieu

>
> Rob

2020-12-11 10:17:50

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v3 01/15] dt-bindings: remoteproc: Add bindind to support autonomous processors

On Tue, Dec 1, 2020 at 5:43 PM Mathieu Poirier
<[email protected]> wrote:
>
> Hi Rob,
>
> On Mon, Nov 30, 2020 at 10:33:21AM -0700, Rob Herring wrote:
> > On Thu, Nov 26, 2020 at 02:06:28PM -0700, Mathieu Poirier wrote:
> > > This patch adds a binding to guide the remoteproc core on how to deal with
> > > remote processors in two cases:
> > >
> > > 1) When an application holding a reference to a remote processor character
> > > device interface crashes.
> > >
> > > 2) when the platform driver for a remote processor is removed.
> > >
> > > In both cases if "autonomous-on-core-reboot" is specified in the remote
> > > processor DT node, the remoteproc core will detach the remote processor
> > > rather than switching it off.
> > >
> > > Signed-off-by: Mathieu Poirier <[email protected]>
> > > ---
> > > .../bindings/remoteproc/remoteproc-core.yaml | 25 +++++++++++++++++++
> > > 1 file changed, 25 insertions(+)
> > > create mode 100644 Documentation/devicetree/bindings/remoteproc/remoteproc-core.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/remoteproc/remoteproc-core.yaml b/Documentation/devicetree/bindings/remoteproc/remoteproc-core.yaml
> > > new file mode 100644
> > > index 000000000000..3032734f42a3
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/remoteproc/remoteproc-core.yaml
> > > @@ -0,0 +1,25 @@
> > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: "http://devicetree.org/schemas/remoteproc/remoteproc-core.yaml#"
> > > +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> > > +
> > > +title: Binding for the remoteproc core applicable to all remote processors
> > > +
> > > +maintainers:
> > > + - Bjorn Andersson <[email protected]>
> > > + - Mathieu Poirier <[email protected]>
> > > +
> > > +description:
> > > + This document defines the binding recognised by the remoteproc core that can
> > > + be used by any remote processor in the subsystem.
> > > +
> > > +properties:
> > > + autonomous-on-core-reboot:
> > > + $ref: /schemas/types.yaml#/definitions/flag
> > > + description:
> > > + Used in two situations, i.e when a user space application releases the
> > > + handle it has on the remote processor's character driver interface and
> > > + when a remote processor's platform driver is being removed. If defined,
> > > + this flag instructs the remoteproc core to detach the remote processor
> > > + rather than turning it off.
> >
> > Userspace? character driver? platform driver? remoteproc core? Please
> > explain this without OS specific terms.
>
> The remoteproc state machine is gaining in complexity and having technical terms
> in the binding's description helps understand when and how it should be used. I
> could make it more generic but that will lead to confusion and abuse.

Explaining in Linux specific terms will confuse any other OS user.

> Should I
> make it "rproc,autonomous-on-core-reboot" ?

No, 'rproc' is not a vendor.

> >
> > Seems to me this would be implied by functionality the remote proc
> > provides.
>
> Exactly - this binding is used by the remoteproc core itself. It is specified
> in the remote processor DT nodes but the individual platform drivers don't do
> anything with it - the core takes care of enacting the desired behavior on their
> behalf. Otherwise each platform driver would end up adding the same code, which
> nobody wants to see happening.

The platform drivers just need to set a flag to enable some behavior
that the core code checks and handles. That should be 1 to a few lines
in the drivers. It's not really any different, just a question of
where the flag lives.

Rob