Re: [PATCH v2 1/3] dt-bindings: arm: renesas: Document Renesas RZ/V2M System Configuration

[Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>]

On 10/11/2022 17:21, Biju Das wrote:
> From: Phil Edworthy <phil.edworthy@renesas.com>
> 
> Add DT binding documentation for System Configuration (SYS) found on
> RZ/V2M SoC's.
> 
> SYS block contains the SYS_VERSION register which can be used to retrieve
> SoC version information.
> 
> Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> [biju: Updated the example ]
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
> v1->v2:
>  * Moved the file from arm->soc/renesas
>  * Updated the path for binding file
>  * Updated the example
> ---
>  .../soc/renesas/renesas,rzv2m-sys.yaml        | 39 +++++++++++++++++++
>  1 file changed, 39 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/soc/renesas/renesas,rzv2m-sys.yaml
> 
> diff --git a/Documentation/devicetree/bindings/soc/renesas/renesas,rzv2m-sys.yaml b/Documentation/devicetree/bindings/soc/renesas/renesas,rzv2m-sys.yaml
> new file mode 100644
> index 000000000000..cc41747798e2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/renesas/renesas,rzv2m-sys.yaml

Filename should be based on the compatible. Pretty often some common
parts of both are fine (e.g. when file contains multiple compatibles),
but this very different then what I see below.

> @@ -0,0 +1,39 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/soc/renesas/renesas,rzv2m-sys.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"

Drop quotes from both.

> +
> +title: Renesas RZ/V2M System Configuration (SYS)
> +
> +maintainers:
> +  - Geert Uytterhoeven <geert+renesas@glider.be>
> +
> +description:
> +  The RZ/V2M System Configuration (SYS) performs system control of the LSI
> +  and supports the following functions,
> +  - LSI version
> +  - 34-bit address space access function
> +  - PCIe related settings
> +  - WDT stop control
> +  - Temperature sensor (TSU) monitor
> +
> +properties:
> +  compatible:
> +    const: renesas,r9a09g011-sys
> +
> +  reg:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    sysc: system-configuration@a3f03000 {

If I get properly the purpose of the device, usually this is called
"system-controller". Use that as device node.

> +            compatible = "renesas,r9a09g011-sys";
> +            reg = <0xa3f03000 0x400>;

Use 4 spaces for example indentation.

> +    };

Best regards,
Krzysztof