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

From: Rob Herring <robh@kernel.org>
To: Krzysztof Kozlowski <krzk@kernel.org>
Cc: Phil Edworthy <phil.edworthy@renesas.com>,
	Geert Uytterhoeven <geert+renesas@glider.be>,
	"linux-renesas-soc@vger.kernel.org" 
	<linux-renesas-soc@vger.kernel.org>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	Biju Das <biju.das.jz@bp.renesas.com>
Subject: Re: [PATCH 02/14] dt-bindings: arm: renesas: Document Renesas RZ/V2M System Configuration
Date: Mon, 28 Mar 2022 20:03:18 -0500	[thread overview]
Message-ID: <YkJa1oLSEP8R4U6y@robh.at.kernel.org> (raw)
In-Reply-To: <4297b8c2-1958-9abb-7d93-0e6d283b6194@kernel.org>

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