diff mbox series

[v3,01/15] dt-bindings: remoteproc: Add bindind to support autonomous processors

Message ID 20201126210642.897302-2-mathieu.poirier@linaro.org
State Changes Requested
Headers show
Series remoteproc: Add support for detaching from rproc | expand

Checks

Context Check Description
robh/dt-meta-schema fail build log
robh/checkpatch success

Commit Message

Mathieu Poirier Nov. 26, 2020, 9:06 p.m. UTC
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 <mathieu.poirier@linaro.org>
---
 .../bindings/remoteproc/remoteproc-core.yaml  | 25 +++++++++++++++++++
 1 file changed, 25 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/remoteproc/remoteproc-core.yaml

Comments

Rob Herring Nov. 30, 2020, 5:23 p.m. UTC | #1
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 <mathieu.poirier@linaro.org>
> ---
>  .../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.
Rob Herring Nov. 30, 2020, 5:33 p.m. UTC | #2
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 <mathieu.poirier@linaro.org>
> ---
>  .../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 <bjorn.andersson@linaro.org>
> +  - Mathieu Poirier <mathieu.poirier@linaro.org>
> +
> +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
Mathieu Poirier Dec. 1, 2020, 11:43 p.m. UTC | #3
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 <mathieu.poirier@linaro.org>
> > ---
> >  .../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 <bjorn.andersson@linaro.org>
> > +  - Mathieu Poirier <mathieu.poirier@linaro.org>
> > +
> > +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
Rob Herring Dec. 10, 2020, 11:10 p.m. UTC | #4
On Tue, Dec 1, 2020 at 5:43 PM Mathieu Poirier
<mathieu.poirier@linaro.org> 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 <mathieu.poirier@linaro.org>
> > > ---
> > >  .../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 <bjorn.andersson@linaro.org>
> > > +  - Mathieu Poirier <mathieu.poirier@linaro.org>
> > > +
> > > +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
diff mbox series

Patch

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 <bjorn.andersson@linaro.org>
+  - Mathieu Poirier <mathieu.poirier@linaro.org>
+
+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.