Received-SPF: pass (google.com: domain of linux-kernel+bounces-204178-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Precedence: bulk
MIME-Version: 1.0
References: <20240524082800.333991-1-prabhakar.mahadev-lad.rj@bp.renesas.com>
 <20240524082800.333991-2-prabhakar.mahadev-lad.rj@bp.renesas.com> <CAMuHMdWzrEKFHauJ=6UnsufJjDO3LfJ45eJXx1V72AmVzvsjyw@mail.gmail.com>
In-Reply-To: <CAMuHMdWzrEKFHauJ=6UnsufJjDO3LfJ45eJXx1V72AmVzvsjyw@mail.gmail.com>
From: "Lad, Prabhakar" <prabhakar.csengg@gmail.com>
Date: Thu, 6 Jun 2024 12:34:11 +0100
Message-ID: <CA+V-a8tVRJB0U-c4BNv-YxvW0ydcw2EqsRkBQvu_HscJvYuiRA@mail.gmail.com>
Subject: Re: [PATCH 1/4] dt-bindings: clock: renesas: Document RZ/V2H(P) SoC
 CPG driver
To: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Michael Turquette <mturquette@baylibre.com>, Stephen Boyd <sboyd@kernel.org>, 
	Rob Herring <robh@kernel.org>, Krzysztof Kozlowski <krzk+dt@kernel.org>, Conor Dooley <conor+dt@kernel.org>, 
	Philipp Zabel <p.zabel@pengutronix.de>, Magnus Damm <magnus.damm@gmail.com>, 
	linux-renesas-soc@vger.kernel.org, linux-clk@vger.kernel.org, 
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, 
	Fabrizio Castro <fabrizio.castro.jz@renesas.com>, 
	Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi Geert,

Thank you for the review.

On Tue, Jun 4, 2024 at 4:50=E2=80=AFPM Geert Uytterhoeven <geert@linux-m68k=
.org> wrote:
>
> Hi Prabhakar,
>
> Thanks for your patch!
>
> Please drop "driver" from the one-line summary.
>
OK, I will drop it.

> On Fri, May 24, 2024 at 10:29=E2=80=AFAM Prabhakar <prabhakar.csengg@gmai=
l.com> wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > Document the device tree bindings of the Renesas RZ/V2H(P) SoC
>
> s/of/for/
>
OK.

> > Clock Pulse Generator (CPG).
> >
> > CPG block handles the below operations:
> > - Handles the generation and control of clock signals for the IP module=
s
>
> Please drop "Handles the"
>
OK.

> > - The generation and control of resets
>
> Please drop "The".
>
OK.

> > - Control over booting
> > - Low power consumption and the power supply domains
>
> Please drop "the".
>
OK.

> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/clock/renesas,rzv2h-cpg.yaml
> > @@ -0,0 +1,78 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/clock/renesas,rzv2h-cpg.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Renesas RZ/V2H(P) Clock Pulse Generator (CPG)
> > +
> > +maintainers:
> > +  - Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > +
> > +description: |
> > +  On Renesas RZ/V2H(P) SoC's, the CPG (Clock Pulse Generator) handles =
the generation
>
> SoCs
>
OK.

> > +  and control of clock signals for the IP modules, the generation and =
control of resets,
> > +  and control over booting, low power consumption and the power supply=
 domains.
>
> Please drop "the".
>
OK.

> > +
> > +properties:
> > +  compatible:
> > +    const: renesas,r9a09g057-cpg
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    maxItems: 1
> > +
> > +  clock-names:
> > +    description:
> > +      Clock source to CPG can be either from external clock input (EXC=
LK) or
> > +      crystal oscillator (XIN/XOUT).
> > +    const: extal
>
> According to Figure 4.4-1 ("CPG Functional Block Diagram"), there are 3
> (RTC, audio, main).
>
Agreed, I will add the below:
- QEXTAL
- RTXIN
- AUDIO_EXTAL
- AUDIO_CLKB
- AUDIO_CLKC

> > +
> > +  '#clock-cells':
> > +    description: |
> > +      - For CPG core clocks, the two clock specifier cells must be "CP=
G_CORE"
> > +        and a core clock reference, as defined in
> > +        <dt-bindings/clock/r9a09g057-cpg.h>,
> > +      - For module clocks, the two clock specifier cells must be "CPG_=
MOD" and
> > +        a module number, as defined in <dt-bindings/clock/r9a09g057-cp=
g.h>.
> > +    const: 2
>
> I understand this will be changed to 1, the clock number?
>
I'll keep this '2'. I will introduce core clocks (clocks which cannot
be controlled by  CLKON_m register) for example,
- SYS_0_PCLK
- CA55_0_CORE_CLK[x]
- IOTOP_0_SHCLK.

> > +  '#power-domain-cells':
> > +    description:
> > +      SoC devices that are part of the CPG/Module Standby Mode Clock D=
omain and
> > +      can be power-managed through Module Standby should refer to the =
CPG device
> > +      node in their "power-domains" property, as documented by the gen=
eric PM
> > +      Domain bindings in Documentation/devicetree/bindings/power/power=
-domain.yaml.
> > +      The power domain specifiers defined in <dt-bindings/clock/r9a09g=
057-cpg.h> could
> > +      be used to reference individual CPG power domains.
>
> The latter suggests "const: 1".
> But the example below uses zero, as does the code?
>
This should be '0' indeed.

> > +
> > +  '#reset-cells':
> > +    description:
> > +      The single reset specifier cell must be the module number, as de=
fined in
>
> reset number (or index).
>
OK.

Cheers,
Prabhakar