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

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

 



Hi Geert-san,

Thank you for your review!

> From: Geert Uytterhoeven, Sent: Thursday, April 21, 2022 7:19 PM
> 
> Hi Shimoda-san,
> 
> On Wed, Apr 20, 2022 at 10:43 AM Yoshihiro Shimoda
> <yoshihiro.shimoda.uh@xxxxxxxxxxx> 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@xxxxxxxxxxx>
> 
> 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?

Oops. I'll add it.

> > +#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?

IIUC, IMPA[01] are internal-only.

> > +#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.

Thank you.
I misunderstood that VIOBUS is an internal core clock.
The VIO is internal core clock, but VIOBUS is not an internal core clock.
I'll add VIOBUS here.

> > +#define R8A779G0_CLK_VIOBUSD2          60
> 
> Missing VCBUS? You do have it as an internal core clock.

Correct. I'll add VCBUS here.

> > +#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.

You're correct. I'll add IMPA here.

Best regards,
Yoshihiro Shimoda

> > +#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@xxxxxxxxxxxxxx
> 
> 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




[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