| Message ID | 20240822170951.339492-2-abdellatif.elkhlifi@arm.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,1/5] dt-bindings: remoteproc: sse710: Add the External Systems remote processors | expand |
On Thu, 22 Aug 2024 18:09:47 +0100, Abdellatif El Khlifi wrote: > Add devicetree binding schema for the External Systems remote processors > > The External Systems remote processors are provided on the Corstone-1000 > IoT Reference Design Platform via the SSE-710 subsystem. > > For more details about the External Systems, please see Corstone SSE-710 > subsystem features [1]. > > [1]: https://developer.arm.com/documentation/102360/0000/Overview-of-Corstone-1000/Corstone-SSE-710-subsystem-features > > Signed-off-by: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com> > --- > .../remoteproc/arm,sse710-extsys.yaml | 90 +++++++++++++++++++ > 1 file changed, 90 insertions(+) > create mode 100644 Documentation/devicetree/bindings/remoteproc/arm,sse710-extsys.yaml > My bot found errors running 'make dt_binding_check' on your patch: yamllint warnings/errors: dtschema/dtc warnings/errors: Documentation/devicetree/bindings/remoteproc/arm,sse710-extsys.example.dtb: /example-0/syscon@1a010000: failed to match any schema with compatible: ['arm,sse710-host-base-sysctrl', 'simple-mfd', 'syscon'] doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240822170951.339492-2-abdellatif.elkhlifi@arm.com The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch. 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 after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.
On Thu, Aug 22, 2024 at 06:09:47PM +0100, Abdellatif El Khlifi wrote: > Add devicetree binding schema for the External Systems remote processors > > The External Systems remote processors are provided on the Corstone-1000 > IoT Reference Design Platform via the SSE-710 subsystem. > > For more details about the External Systems, please see Corstone SSE-710 > subsystem features [1]. > Do not attach (thread) your patchsets to some other threads (unrelated or older versions). This buries them deep in the mailbox and might interfere with applying entire sets. > [1]: https://developer.arm.com/documentation/102360/0000/Overview-of-Corstone-1000/Corstone-SSE-710-subsystem-features > > Signed-off-by: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com> > --- > .../remoteproc/arm,sse710-extsys.yaml | 90 +++++++++++++++++++ > 1 file changed, 90 insertions(+) > create mode 100644 Documentation/devicetree/bindings/remoteproc/arm,sse710-extsys.yaml > > diff --git a/Documentation/devicetree/bindings/remoteproc/arm,sse710-extsys.yaml b/Documentation/devicetree/bindings/remoteproc/arm,sse710-extsys.yaml > new file mode 100644 > index 000000000000..827ba8d962f1 > --- /dev/null > +++ b/Documentation/devicetree/bindings/remoteproc/arm,sse710-extsys.yaml > @@ -0,0 +1,90 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/remoteproc/arm,sse710-extsys.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: SSE-710 External System Remote Processor > + > +maintainers: > + - Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com> > + - Hugues Kamba Mpiana <hugues.kambampiana@arm.com> > + > +description: | dt-preserve-formatting > + SSE-710 is an heterogeneous subsystem supporting up to two remote > + processors aka the External Systems. > + > +properties: > + compatible: > + enum: > + - arm,sse710-extsys > + > + firmware-name: > + description: > + The default name of the firmware to load to the remote processor. > + > + '#extsys-id': '#' is not correct for sure, that's not a cell specifier. But anyway, we do not accept in general instance IDs. > + description: > + The External System ID. This tells me nothing. You basically copied property name. > + enum: [0, 1] > + > + mbox-names: > + items: > + - const: txes0 > + - const: rxes0 > + > + mboxes: > + description: > + The list of Message Handling Unit (MHU) channels used for bidirectional > + communication. This property is only required if the virtio-based Rpmsg > + messaging bus is used. For more details see the Arm MHUv2 Mailbox > + Controller at devicetree/bindings/mailbox/arm,mhuv2.yaml > + Drop blank line > + minItems: 2 This is redundant if equals to maxItemns, drop. > + maxItems: 2 > + > + memory-region: > + description: > + If present, a phandle for a reserved memory area that used for vdev > + buffer, resource table, vring region and others used by the remote > + processor. > + minItems: 2 > + maxItems: 32 > + > +required: > + - compatible > + - firmware-name > + - '#extsys-id' > + > +additionalProperties: false > + > +examples: > + - | > + reserved-memory { > + #address-cells = <2>; > + #size-cells = <2>; > + > + extsys0_vring0: vdev0vring0@82001000 { > + reg = <0 0x82001000 0 0x8000>; > + no-map; > + }; > + > + extsys0_vring1: vdev0vring1@82009000 { > + reg = <0 0x82009000 0 0x8000>; > + no-map; > + }; > + }; Drop, it is fairly common. > + > + syscon@1a010000 { > + compatible = "arm,sse710-host-base-sysctrl", "simple-mfd", "syscon"; > + reg = <0x1a010000 0x1000>; So this is a part of other block? Then make one complete example in the parent device bindings. > + > + extsys0 { Node names should be generic. See also an explanation and list of examples (not exhaustive) in DT specification: https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation e.g. remoteproc > + compatible = "arm,sse710-extsys"; > + #extsys-id = <0>; > + firmware-name = "es_flashfw.elf"; > + mbox-names = "txes0", "rxes0"; > + mboxes = <&mhu0_hes0 0 1>, <&mhu0_es0h 0 1>; First go mboxes, then mbox-names. The same in the binding, BTW. Best regards, Krzysztof
Hi Krzysztof, > > Add devicetree binding schema for the External Systems remote processors > > > > The External Systems remote processors are provided on the Corstone-1000 > > IoT Reference Design Platform via the SSE-710 subsystem. > > > > For more details about the External Systems, please see Corstone SSE-710 > > subsystem features [1]. > > > > Do not attach (thread) your patchsets to some other threads (unrelated > or older versions). This buries them deep in the mailbox and might > interfere with applying entire sets. > > > [1]: https://developer.arm.com/documentation/102360/0000/Overview-of-Corstone-1000/Corstone-SSE-710-subsystem-features > > > > Signed-off-by: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com> > > --- > > .../remoteproc/arm,sse710-extsys.yaml | 90 +++++++++++++++++++ > > 1 file changed, 90 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/remoteproc/arm,sse710-extsys.yaml > > > > diff --git a/Documentation/devicetree/bindings/remoteproc/arm,sse710-extsys.yaml b/Documentation/devicetree/bindings/remoteproc/arm,sse710-extsys.yaml > > new file mode 100644 > > index 000000000000..827ba8d962f1 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/remoteproc/arm,sse710-extsys.yaml > > @@ -0,0 +1,90 @@ > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/remoteproc/arm,sse710-extsys.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: SSE-710 External System Remote Processor > > + > > +maintainers: > > + - Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com> > > + - Hugues Kamba Mpiana <hugues.kambampiana@arm.com> > > + > > +description: | > > dt-preserve-formatting Do you mean I should remove the '|' please ? (I didn't find examples of use of dt-preserve-formatting in Documentation/devicetree/bindings/) > > > + SSE-710 is an heterogeneous subsystem supporting up to two remote > > + processors aka the External Systems. > > + > > +properties: > > + compatible: > > + enum: > > + - arm,sse710-extsys > > + > > + firmware-name: > > + description: > > + The default name of the firmware to load to the remote processor. > > + > > + '#extsys-id': > > '#' is not correct for sure, that's not a cell specifier. > > But anyway, we do not accept in general instance IDs. I'm happy to replace the instance ID with another solution. In our case the remoteproc instance does not have a base address to use. So, we can't put remoteproc@address What do you recommend in this case please ? Kind regards, Abdellatif
On 19/09/2024 11:35, Abdellatif El Khlifi wrote: > Hi Krzysztof, > >>> Add devicetree binding schema for the External Systems remote processors >>> >>> The External Systems remote processors are provided on the Corstone-1000 >>> IoT Reference Design Platform via the SSE-710 subsystem. >>> >>> For more details about the External Systems, please see Corstone SSE-710 >>> subsystem features [1]. >>> >> >> Do not attach (thread) your patchsets to some other threads (unrelated >> or older versions). This buries them deep in the mailbox and might >> interfere with applying entire sets. >> >>> [1]: https://developer.arm.com/documentation/102360/0000/Overview-of-Corstone-1000/Corstone-SSE-710-subsystem-features >>> >>> Signed-off-by: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com> >>> --- >>> .../remoteproc/arm,sse710-extsys.yaml | 90 +++++++++++++++++++ >>> 1 file changed, 90 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/remoteproc/arm,sse710-extsys.yaml >>> >>> diff --git a/Documentation/devicetree/bindings/remoteproc/arm,sse710-extsys.yaml b/Documentation/devicetree/bindings/remoteproc/arm,sse710-extsys.yaml >>> new file mode 100644 >>> index 000000000000..827ba8d962f1 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/remoteproc/arm,sse710-extsys.yaml >>> @@ -0,0 +1,90 @@ >>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) >>> +%YAML 1.2 >>> +--- >>> +$id: http://devicetree.org/schemas/remoteproc/arm,sse710-extsys.yaml# >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>> + >>> +title: SSE-710 External System Remote Processor >>> + >>> +maintainers: >>> + - Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com> >>> + - Hugues Kamba Mpiana <hugues.kambampiana@arm.com> >>> + >>> +description: | >> >> dt-preserve-formatting > > Do you mean I should remove the '|' please ? (I didn't find examples of use of > dt-preserve-formatting in Documentation/devicetree/bindings/) > >> >>> + SSE-710 is an heterogeneous subsystem supporting up to two remote >>> + processors aka the External Systems. >>> + >>> +properties: >>> + compatible: >>> + enum: >>> + - arm,sse710-extsys >>> + >>> + firmware-name: >>> + description: >>> + The default name of the firmware to load to the remote processor. >>> + >>> + '#extsys-id': >> >> '#' is not correct for sure, that's not a cell specifier. >> >> But anyway, we do not accept in general instance IDs. > > I'm happy to replace the instance ID with another solution. > In our case the remoteproc instance does not have a base address > to use. So, we can't put remoteproc@address > > What do you recommend in this case please ? Waiting one month to respond is a great way to drop all context from my memory. The emails are not even available for me - gone from inbox. Bus addressing could note it. Or you have different devices, so different compatibles. Tricky to say, because you did not describe the hardware really and it's one month later... Best regards, Krzysztof
Hi Krzysztof, > >>> + '#extsys-id': > >> > >> '#' is not correct for sure, that's not a cell specifier. > >> > >> But anyway, we do not accept in general instance IDs. > > > > I'm happy to replace the instance ID with another solution. > > In our case the remoteproc instance does not have a base address > > to use. So, we can't put remoteproc@address > > > > What do you recommend in this case please ? > > Waiting one month to respond is a great way to drop all context from my > memory. The emails are not even available for me - gone from inbox. > > Bus addressing could note it. Or you have different devices, so > different compatibles. Tricky to say, because you did not describe the > hardware really and it's one month later... > Sorry for waiting. I was in holidays. I'll add more documentation about the external system for more clarity [1]. Basically, Linux runs on the Cortex-A35. The External system is a Cortex-M core. The Cortex-A35 can not access the memory of the Cortex-M. It can only control Cortex-M core using the reset control and status registers mapped in the memory space of the Cortex-A35. I'll make sure this explanation is added to the binding and commit log for more clarity. Thanks for the suggestion regarding supporting multiple instances of the External system. I will send a new version shortly addressing all comments. [1]: paragraph 2.3, https://developer.arm.com/documentation/dai0550/D/?lang=en Kind regards Abdellatif
On 19/09/2024 16:57, Abdellatif El Khlifi wrote: > Hi Krzysztof, > >>>>> + '#extsys-id': >>>> >>>> '#' is not correct for sure, that's not a cell specifier. >>>> >>>> But anyway, we do not accept in general instance IDs. >>> >>> I'm happy to replace the instance ID with another solution. >>> In our case the remoteproc instance does not have a base address >>> to use. So, we can't put remoteproc@address >>> >>> What do you recommend in this case please ? >> >> Waiting one month to respond is a great way to drop all context from my >> memory. The emails are not even available for me - gone from inbox. >> >> Bus addressing could note it. Or you have different devices, so >> different compatibles. Tricky to say, because you did not describe the >> hardware really and it's one month later... >> > > Sorry for waiting. I was in holidays. > > I'll add more documentation about the external system for more clarity [1]. > > Basically, Linux runs on the Cortex-A35. The External system is a > Cortex-M core. The Cortex-A35 can not access the memory of the Cortex-M. > It can only control Cortex-M core using the reset control and status registers mapped > in the memory space of the Cortex-A35. That's pretty standard. It does not explain me why bus addressing or different compatible are not sufficient here. Best regards, Krzysztof
Hi Krzysztof, > >>>>> + '#extsys-id': > >>>> > >>>> '#' is not correct for sure, that's not a cell specifier. > >>>> > >>>> But anyway, we do not accept in general instance IDs. > >>> > >>> I'm happy to replace the instance ID with another solution. > >>> In our case the remoteproc instance does not have a base address > >>> to use. So, we can't put remoteproc@address > >>> > >>> What do you recommend in this case please ? > >> > >> Waiting one month to respond is a great way to drop all context from my > >> memory. The emails are not even available for me - gone from inbox. > >> > >> Bus addressing could note it. Or you have different devices, so > >> different compatibles. Tricky to say, because you did not describe the > >> hardware really and it's one month later... > >> > > > > Sorry for waiting. I was in holidays. > > > > I'll add more documentation about the external system for more clarity [1]. > > > > Basically, Linux runs on the Cortex-A35. The External system is a > > Cortex-M core. The Cortex-A35 can not access the memory of the Cortex-M. > > It can only control Cortex-M core using the reset control and status registers mapped > > in the memory space of the Cortex-A35. > > That's pretty standard. > > It does not explain me why bus addressing or different compatible are > not sufficient here. Using an instance ID was a design choice. I'm happy to replace it with the use of compatible and match data (WIP). The match data will be pointing to a data structure containing the right offsets to be used with regmap APIs. syscon node is used to represent the Host Base System Control register area [1] where the external system reset registers are mapped (EXT_SYS*). The nodes will look like this: syscon@1a010000 { compatible = "arm,sse710-host-base-sysctrl", "simple-mfd", "syscon"; reg = <0x1a010000 0x1000>; #address-cells = <1>; #size-cells = <1>; remoteproc@310 { compatible = "arm,sse710-extsys0"; reg = <0x310 4>; ... } remoteproc@318 { compatible = "arm,sse710-extsys1"; reg = <0x318 4>; ... } [1]: https://developer.arm.com/documentation/102342/0000/Programmers-model/Register-descriptions/Host-Base-System-Control-register-summary Cheers Abdellatif
On 20/09/2024 16:19, Abdellatif El Khlifi wrote: > Hi Krzysztof, > >>>>>>> + '#extsys-id': >>>>>> >>>>>> '#' is not correct for sure, that's not a cell specifier. >>>>>> >>>>>> But anyway, we do not accept in general instance IDs. >>>>> >>>>> I'm happy to replace the instance ID with another solution. >>>>> In our case the remoteproc instance does not have a base address >>>>> to use. So, we can't put remoteproc@address >>>>> >>>>> What do you recommend in this case please ? >>>> >>>> Waiting one month to respond is a great way to drop all context from my >>>> memory. The emails are not even available for me - gone from inbox. >>>> >>>> Bus addressing could note it. Or you have different devices, so >>>> different compatibles. Tricky to say, because you did not describe the >>>> hardware really and it's one month later... >>>> >>> >>> Sorry for waiting. I was in holidays. >>> >>> I'll add more documentation about the external system for more clarity [1]. >>> >>> Basically, Linux runs on the Cortex-A35. The External system is a >>> Cortex-M core. The Cortex-A35 can not access the memory of the Cortex-M. >>> It can only control Cortex-M core using the reset control and status registers mapped >>> in the memory space of the Cortex-A35. >> >> That's pretty standard. >> >> It does not explain me why bus addressing or different compatible are >> not sufficient here. > > Using an instance ID was a design choice. > I'm happy to replace it with the use of compatible and match data (WIP). > > The match data will be pointing to a data structure containing the right offsets > to be used with regmap APIs. > > syscon node is used to represent the Host Base System Control register area [1] > where the external system reset registers are mapped (EXT_SYS*). > > The nodes will look like this: > > syscon@1a010000 { > compatible = "arm,sse710-host-base-sysctrl", "simple-mfd", "syscon"; > reg = <0x1a010000 0x1000>; > > #address-cells = <1>; > #size-cells = <1>; > > remoteproc@310 { > compatible = "arm,sse710-extsys0"; > reg = <0x310 4>; Uh, why do you create device nodes for one word? This really suggests it is part of parent device and your split is artificial. Best regards, Krzysztof
Hi Krzysztof, > >>>>>>> + '#extsys-id': > >>>>>> > >>>>>> '#' is not correct for sure, that's not a cell specifier. > >>>>>> > >>>>>> But anyway, we do not accept in general instance IDs. > >>>>> > >>>>> I'm happy to replace the instance ID with another solution. > >>>>> In our case the remoteproc instance does not have a base address > >>>>> to use. So, we can't put remoteproc@address > >>>>> > >>>>> What do you recommend in this case please ? > >>>> > >>>> Waiting one month to respond is a great way to drop all context from my > >>>> memory. The emails are not even available for me - gone from inbox. > >>>> > >>>> Bus addressing could note it. Or you have different devices, so > >>>> different compatibles. Tricky to say, because you did not describe the > >>>> hardware really and it's one month later... > >>>> > >>> > >>> Sorry for waiting. I was in holidays. > >>> > >>> I'll add more documentation about the external system for more clarity [1]. > >>> > >>> Basically, Linux runs on the Cortex-A35. The External system is a > >>> Cortex-M core. The Cortex-A35 can not access the memory of the Cortex-M. > >>> It can only control Cortex-M core using the reset control and status registers mapped > >>> in the memory space of the Cortex-A35. > >> > >> That's pretty standard. > >> > >> It does not explain me why bus addressing or different compatible are > >> not sufficient here. > > > > Using an instance ID was a design choice. > > I'm happy to replace it with the use of compatible and match data (WIP). > > > > The match data will be pointing to a data structure containing the right offsets > > to be used with regmap APIs. > > > > syscon node is used to represent the Host Base System Control register area [1] > > where the external system reset registers are mapped (EXT_SYS*). > > > > The nodes will look like this: > > > > syscon@1a010000 { > > compatible = "arm,sse710-host-base-sysctrl", "simple-mfd", "syscon"; > > reg = <0x1a010000 0x1000>; > > > > #address-cells = <1>; > > #size-cells = <1>; > > > > remoteproc@310 { > > compatible = "arm,sse710-extsys0"; > > reg = <0x310 4>; > > Uh, why do you create device nodes for one word? This really suggests it > is part of parent device and your split is artificial. The external system registers (described by the remoteproc node) are part of the parent device (the Host Base System Control register area) described by syscon. In case of the external system 0 , its registers are located at offset 0x310 (physical address: 0x1a010310) When instantiating the devices without @address, the DTC compiler detects 2 nodes with the same name (remoteproc). syscon@1a010000 { ... remoteproc { compatible = "arm,sse710-extsys0"; ... } remoteproc { compatible = "arm,sse710-extsys1"; ... } Cheers Abdellatif
diff --git a/Documentation/devicetree/bindings/remoteproc/arm,sse710-extsys.yaml b/Documentation/devicetree/bindings/remoteproc/arm,sse710-extsys.yaml new file mode 100644 index 000000000000..827ba8d962f1 --- /dev/null +++ b/Documentation/devicetree/bindings/remoteproc/arm,sse710-extsys.yaml @@ -0,0 +1,90 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/remoteproc/arm,sse710-extsys.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: SSE-710 External System Remote Processor + +maintainers: + - Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com> + - Hugues Kamba Mpiana <hugues.kambampiana@arm.com> + +description: | + SSE-710 is an heterogeneous subsystem supporting up to two remote + processors aka the External Systems. + +properties: + compatible: + enum: + - arm,sse710-extsys + + firmware-name: + description: + The default name of the firmware to load to the remote processor. + + '#extsys-id': + description: + The External System ID. + enum: [0, 1] + + mbox-names: + items: + - const: txes0 + - const: rxes0 + + mboxes: + description: + The list of Message Handling Unit (MHU) channels used for bidirectional + communication. This property is only required if the virtio-based Rpmsg + messaging bus is used. For more details see the Arm MHUv2 Mailbox + Controller at devicetree/bindings/mailbox/arm,mhuv2.yaml + + minItems: 2 + maxItems: 2 + + memory-region: + description: + If present, a phandle for a reserved memory area that used for vdev + buffer, resource table, vring region and others used by the remote + processor. + minItems: 2 + maxItems: 32 + +required: + - compatible + - firmware-name + - '#extsys-id' + +additionalProperties: false + +examples: + - | + reserved-memory { + #address-cells = <2>; + #size-cells = <2>; + + extsys0_vring0: vdev0vring0@82001000 { + reg = <0 0x82001000 0 0x8000>; + no-map; + }; + + extsys0_vring1: vdev0vring1@82009000 { + reg = <0 0x82009000 0 0x8000>; + no-map; + }; + }; + + syscon@1a010000 { + compatible = "arm,sse710-host-base-sysctrl", "simple-mfd", "syscon"; + reg = <0x1a010000 0x1000>; + + extsys0 { + compatible = "arm,sse710-extsys"; + #extsys-id = <0>; + firmware-name = "es_flashfw.elf"; + mbox-names = "txes0", "rxes0"; + mboxes = <&mhu0_hes0 0 1>, <&mhu0_es0h 0 1>; + memory-region = <&extsys0_vring0>, <&extsys0_vring1>; + }; + };
Add devicetree binding schema for the External Systems remote processors The External Systems remote processors are provided on the Corstone-1000 IoT Reference Design Platform via the SSE-710 subsystem. For more details about the External Systems, please see Corstone SSE-710 subsystem features [1]. [1]: https://developer.arm.com/documentation/102360/0000/Overview-of-Corstone-1000/Corstone-SSE-710-subsystem-features Signed-off-by: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com> --- .../remoteproc/arm,sse710-extsys.yaml | 90 +++++++++++++++++++ 1 file changed, 90 insertions(+) create mode 100644 Documentation/devicetree/bindings/remoteproc/arm,sse710-extsys.yaml