diff mbox series

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

Message ID 20220321154232.56315-3-phil.edworthy@renesas.com (mailing list archive)
State Superseded
Delegated to: Geert Uytterhoeven
Headers show
Series Add new Renesas RZ/V2M SoC and Renesas RZ/V2M EVK support | expand

Commit Message

Phil Edworthy March 21, 2022, 3:42 p.m. UTC 
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>
---
 .../bindings/arm/renesas,rzv2m-sys.yaml       | 39 +++++++++++++++++++
 1 file changed, 39 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/renesas,rzv2m-sys.yaml

Comments

Krzysztof Kozlowski March 23, 2022, 10:41 a.m. UTC | #1 
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
Phil Edworthy March 23, 2022, 2:44 p.m. UTC | #2 
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@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...
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@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?

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
Krzysztof Kozlowski March 23, 2022, 2:54 p.m. UTC | #3 
On 23/03/2022 15:44, Phil Edworthy wrote:
> 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@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...
> 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@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?
> 
> 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.

The existing rzg2l-sysc looks similar and is in power. If arm location
is conscious choice (not just placeholder), fine with me. :)


Best regards,
Krzysztof
Rob Herring (Arm) March 29, 2022, 1:03 a.m. UTC | #4 
On Wed, Mar 23, 2022 at 03:54:42PM +0100, Krzysztof Kozlowski wrote:
> On 23/03/2022 15:44, Phil Edworthy wrote:
> > 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@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...
> > 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@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?
> > 
> > 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.
> 
> The existing rzg2l-sysc looks similar and is in power. If arm location
> is conscious choice (not just placeholder), fine with me. :)

The preference is:

1) subsystem/class
2) soc/ dir

And arm/ for just top-level bindings.

Rob
Geert Uytterhoeven April 26, 2022, 2:10 p.m. UTC | #5 
On Tue, Mar 29, 2022 at 3:03 AM Rob Herring <robh@kernel.org> wrote:
> On Wed, Mar 23, 2022 at 03:54:42PM +0100, Krzysztof Kozlowski wrote:
> > On 23/03/2022 15:44, Phil Edworthy wrote:
> > > 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@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...
> > > 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@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?
> > >
> > > 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.
> >
> > The existing rzg2l-sysc looks similar and is in power. If arm location
> > is conscious choice (not just placeholder), fine with me. :)
>
> The preference is:
>
> 1) subsystem/class
> 2) soc/ dir
>
> And arm/ for just top-level bindings.

Documentation/devicetree/bindings/soc/renesas/ sounds good to me!

I'll send patches to move .../arm/renesas,prr.yaml and
.../power/renesas,rzg2l-sysc.yaml to .../soc/renesas/, too.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
diff mbox series

Patch

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
+
+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>;
+    };