linux-kernel - Re: [PATCH v2 1/5] dt-bindings: remoteproc: sse710: Add the External Systems remote processors

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <gzlncpyzwm7x4jcxtdrthrlv2dofk7u3oxn4taadwog5tt37wo@ot6s6kwukd4k>
Date: Fri, 23 Aug 2024 08:23:17 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Abdellatif El Khlifi <abdellatif.elkhlifi@....com>
Cc: mathieu.poirier@...aro.org, Adam.Johnston@....com, 
	Hugues.KambaMpiana@....com, Drew.Reed@....com, andersson@...nel.org, conor+dt@...nel.org, 
	devicetree@...r.kernel.org, krzysztof.kozlowski+dt@...aro.org, 
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org, linux-remoteproc@...r.kernel.org, 
	liviu.dudau@....com, lpieralisi@...nel.org, robh@...nel.org, sudeep.holla@....com, 
	robin.murphy@....com
Subject: Re: [PATCH v2 1/5] dt-bindings: remoteproc: sse710: Add the External
 Systems remote processors

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@....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@....com>
> +  - Hugues Kamba Mpiana <hugues.kambampiana@....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@...01000 {
> +            reg = <0 0x82001000 0 0x8000>;
> +            no-map;
> +        };
> +
> +        extsys0_vring1: vdev0vring1@...09000 {
> +            reg = <0 0x82009000 0 0x8000>;
> +            no-map;
> +        };
> +    };

Drop, it is fairly common.

> +
> +    syscon@...10000 {
> +        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