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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux