Re: [PATCH 07/15] dt-bindings: clock: Add r8a779g0 CPG Core Clock Definitions

From: Geert Uytterhoeven <geert@linux-m68k.org>
To: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Cc: Magnus Damm <magnus.damm@gmail.com>,
	Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Greg KH <gregkh@linuxfoundation.org>,
	Linux-Renesas <linux-renesas-soc@vger.kernel.org>,
	"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
	<devicetree@vger.kernel.org>,
	"open list:SERIAL DRIVERS" <linux-serial@vger.kernel.org>
Subject: Re: [PATCH 07/15] dt-bindings: clock: Add r8a779g0 CPG Core Clock Definitions
Date: Thu, 21 Apr 2022 12:18:51 +0200	[thread overview]
Message-ID: <CAMuHMdVkcG-edq=v_onOc66y3UpJgr74R1c9t7kfYhnOnqZdZQ@mail.gmail.com> (raw)
In-Reply-To: <20220420084255.375700-8-yoshihiro.shimoda.uh@renesas.com>

Hi Shimoda-san,

On Wed, Apr 20, 2022 at 10:43 AM Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
> Add all Clock Pulse Generator Core Clock Outputs for the Renesas
> R-Car V4H (R8A779G0) SoC.
>
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>

Thanks for your patch!

> --- /dev/null
> +++ b/include/dt-bindings/clock/r8a779g0-cpg-mssr.h
> @@ -0,0 +1,87 @@
> +/* SPDX-License-Identifier: (GPL-2.0 or MIT) */
> +/*
> + * Copyright (C) 2022 Renesas Electronics Corp.
> + */
> +#ifndef __DT_BINDINGS_CLOCK_R8A779G0_CPG_MSSR_H__
> +#define __DT_BINDINGS_CLOCK_R8A779G0_CPG_MSSR_H__
> +
> +#include <dt-bindings/clock/renesas-cpg-mssr.h>
> +
> +/* r8a779g0 CPG Core Clocks */
> +
> +#define R8A779G0_CLK_ZX                        0
> +#define R8A779G0_CLK_ZS                        1
> +#define R8A779G0_CLK_ZT                        2
> +#define R8A779G0_CLK_ZTR               3
> +#define R8A779G0_CLK_S0D2              4
> +#define R8A779G0_CLK_S0D3              5
> +#define R8A779G0_CLK_S0D4              6
> +#define R8A779G0_CLK_S0D1_VIO          7
> +#define R8A779G0_CLK_S0D2_VIO          8
> +#define R8A779G0_CLK_S0D4_VIO          9
> +#define R8A779G0_CLK_S0D8_VIO          10
> +#define R8A779G0_CLK_S0D1_VC           11
> +#define R8A779G0_CLK_S0D2_VC           12
> +#define R8A779G0_CLK_S0D4_VC           13
> +#define R8A779G0_CLK_S0D2_MM           14
> +#define R8A779G0_CLK_S0D4_MM           15
> +#define R8A779G0_CLK_S0D2_U3DG         16
> +#define R8A779G0_CLK_S0D4_U3DG         17
> +#define R8A779G0_CLK_S0D2_RT           18
> +#define R8A779G0_CLK_S0D3_RT           19
> +#define R8A779G0_CLK_S0D4_RT           20
> +#define R8A779G0_CLK_S0D6_RT           21
> +#define R8A779G0_CLK_S0D24_RT          22
> +#define R8A779G0_CLK_S0D2_PER          23
> +#define R8A779G0_CLK_S0D3_PER          24

Missing S0D4_PER?

> +#define R8A779G0_CLK_S0D6_PER          25
> +#define R8A779G0_CLK_S0D12_PER         26
> +#define R8A779G0_CLK_S0D24_PER         27
> +#define R8A779G0_CLK_S0D1_HSC          28
> +#define R8A779G0_CLK_S0D2_HSC          29
> +#define R8A779G0_CLK_S0D4_HSC          30
> +#define R8A779G0_CLK_S0D2_CC           31
> +#define R8A779G0_CLK_SVD1_IR           32
> +#define R8A779G0_CLK_SVD2_IR           33

Missing IMPA0?
Or is it internal-only? Perhaps the same for IMPA1 below?

> +#define R8A779G0_CLK_SVD1_VIP          34
> +#define R8A779G0_CLK_SVD2_VIP          35
> +#define R8A779G0_CLK_CL                        36
> +#define R8A779G0_CLK_CL16M             37
> +#define R8A779G0_CLK_CL16M_MM          38
> +#define R8A779G0_CLK_CL16M_RT          39
> +#define R8A779G0_CLK_CL16M_PER         40
> +#define R8A779G0_CLK_CL16M_HSC         41
> +#define R8A779G0_CLK_Z0                        42
> +#define R8A779G0_CLK_ZB3               43
> +#define R8A779G0_CLK_ZB3D2             44
> +#define R8A779G0_CLK_ZB3D4             45
> +#define R8A779G0_CLK_ZG                        46
> +#define R8A779G0_CLK_SD0H              47
> +#define R8A779G0_CLK_SD0               48
> +#define R8A779G0_CLK_RPC               49
> +#define R8A779G0_CLK_RPCD2             50
> +#define R8A779G0_CLK_MSO               51
> +#define R8A779G0_CLK_CANFD             52
> +#define R8A779G0_CLK_CSI               53
> +#define R8A779G0_CLK_FRAY              54
> +#define R8A779G0_CLK_IPC               55
> +#define R8A779G0_CLK_SASYNCRT          56
> +#define R8A779G0_CLK_SASYNCPERD1       57
> +#define R8A779G0_CLK_SASYNCPERD2       58
> +#define R8A779G0_CLK_SASYNCPERD4       59

Missing VIOBUS? You do have it as an internal core clock.

> +#define R8A779G0_CLK_VIOBUSD2          60

Missing VCBUS? You do have it as an internal core clock.

> +#define R8A779G0_CLK_VCBUSD2           61
> +#define R8A779G0_CLK_IMPA1             62
> +#define R8A779G0_CLK_DSIEXT            63
> +#define R8A779G0_CLK_DSIREF            64
> +#define R8A779G0_CLK_ADGH              65
> +#define R8A779G0_CLK_OSC               66
> +#define R8A779G0_CLK_ZR0               67
> +#define R8A779G0_CLK_ZR1               68
> +#define R8A779G0_CLK_ZR2               69

Missing IMPA?
Figure 8.1.1 (Block Diagram of CPG) indicates it's a direct
input to the IMP block, hence not an internal core clock.

> +#define R8A779G0_CLK_IMPAD4            70
> +#define R8A779G0_CLK_CPEX              71
> +#define R8A779G0_CLK_CBFUSA            72
> +#define R8A779G0_CLK_R                 73
> +
> +#endif /* __DT_BINDINGS_CLOCK_R8A779G0_CPG_MSSR_H__ */

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