Hi Krzysztof, Thanks for the review. On 23 March 2022 10:42, Krzysztof Kozlowski wrote: > 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@xxxxxxxxxxx> > > Reviewed-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx> > > Could you send reviewed-by tags publicly? Maybe there was internal > review, maybe not and it was just copy-pasted to all submissions... Yes, it was reviewed internally. We've done it like this for a while, I'll see what we can do to change the way we do it. Would just copying the person who reviewed it be enough? > > --- > > .../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@xxxxxxxxx> > > + > > +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? SYS looks like somewhere to put registers that don't have a logical home. There are lots of little bits, I just listed the main functions. On other Renesas SoCs, it's similar but they include power related registers. Actually, I originally put it in the power directory, then moved it. > > + > > +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. Sorry, I missed that. I just tried, but it's failing on some unrelated bindings for a different SoC. It may be because I'm basing it off next-20220321 at the moment. > > + }; > > > Best regards, > Krzysztof Thanks Phil