Re: [PATCH 02/14] dt-bindings: arm: renesas: Document Renesas RZ/V2M System Configuration

[Krzysztof Kozlowski <krzk@kernel.org>]

On 21/03/2022 16:42, Phil Edworthy wrote:
> 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>
> Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>

Could you send reviewed-by tags publicly? Maybe there was internal
review, maybe not and it was just copy-pasted to all submissions...

> ---
>  .../bindings/arm/renesas,rzv2m-sys.yaml       | 39 +++++++++++++++++++
>  1 file changed, 39 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/arm/renesas,rzv2m-sys.yaml
> 
> diff --git a/Documentation/devicetree/bindings/arm/renesas,rzv2m-sys.yaml b/Documentation/devicetree/bindings/arm/renesas,rzv2m-sys.yaml
> new file mode 100644
> index 000000000000..1a58906336b8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/renesas,rzv2m-sys.yaml
> @@ -0,0 +1,39 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/arm/renesas,rzv2m-sys.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +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

Usually all these are separate devices, so what does it mean that SYS is
supporting these functions? Is it related to other Renesas System
Controllers? For example
Documentation/devicetree/bindings/power/renesas,apmu.yaml
?
Why one is in power and one in arm subdirectory? Maybe you should extend
existing one?

> +
> +properties:
> +  compatible:
> +    const: renesas,r9a09g011-sys
> +
> +  reg:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    sysc: system-configuration@a3f03000 {
> +            compatible = "renesas,r9a09g011-sys";
> +            reg = <0 0xa3f03000 0 0x400>;

Did you actually test it (make dt_binding_check)? This looks wrong.

> +    };


Best regards,
Krzysztof