RE: [PATCH 1/2] clk: renesas: Add r8a77990 CPG Core Clock Definitions

[Yoshihiro Shimoda]

Hi Geert-san,

Thank you for the review!

> From: Geert Uytterhoeven, Sent: Thursday, April 12, 2018 9:23 PM
> 
> Hi Shimoda-san,
> 
> On Wed, Apr 11, 2018 at 11:37 AM, Yoshihiro Shimoda
>  wrote:

> > --- /dev/null
> > +++ b/include/dt-bindings/clock/r8a77990-cpg-mssr.h
> > @@ -0,0 +1,63 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * Copyright (C) 2018 Renesas Electronics Corp.
> > + */
> > +#ifndef __DT_BINDINGS_CLOCK_R8A77990_CPG_MSSR_H__
> > +#define __DT_BINDINGS_CLOCK_R8A77990_CPG_MSSR_H__
> > +
> > +#include 
> > +
> > +/* r8a77990 CPG Core Clocks */
> 
> [...]
> 
> > +#define R8A77990_CLK_CSI0  47
> 
> Note that CSI0 is not listed in Table 8.2g ("R-Car E3").
> Probably it does exist, given:
>   - Table 8.11 ("Register Configuration") says CSI0CKCR exists on R-Car E3,
>   - Figure 25.6 ("CSI2 Block Diagram (R-Car E3)") shows CSI0.

I think so. I'm asking HW team about missing CSI0 in Table 8.2g now.

> > +#define R8A77990_CLK_POST3 48
> 
> I noticed these POSTx clocks have been added to all R-Car Gen3 clock tables.
> It doesn't look like we will ever need to refer them from DT, so I think we
> can treat them as internal clocks, and omit them from the DT bindings.
> 
> What do you think?

I agree with you. So, I will omit POSTx clocks from the DT bindings in v2 patch.

Best regards,
Yoshihiro Shimoda

Re: [PATCH 1/2] clk: renesas: Add r8a77990 CPG Core Clock Definitions

[Geert Uytterhoeven]

Hi Shimoda-san,

On Wed, Apr 11, 2018 at 11:37 AM, Yoshihiro Shimoda
 wrote:
> From: Takeshi Kihara 
>
> This patch adds all R-Car E3 Clock Pulse Generator Core Clock Outputs.
>
> Note that internal CPG clocks (S0, S1, S2, S3, SDSRC) are not included,
> as they are used as internal clock sources only, and never referenced
> from DT.
>
> Signed-off-by: Takeshi Kihara 
> [shimoda: add SPDX-License-Identifier]
> Signed-off-by: Yoshihiro Shimoda 

Thanks for your patch!

> --- /dev/null
> +++ b/include/dt-bindings/clock/r8a77990-cpg-mssr.h
> @@ -0,0 +1,63 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) 2018 Renesas Electronics Corp.
> + */
> +#ifndef __DT_BINDINGS_CLOCK_R8A77990_CPG_MSSR_H__
> +#define __DT_BINDINGS_CLOCK_R8A77990_CPG_MSSR_H__
> +
> +#include 
> +
> +/* r8a77990 CPG Core Clocks */

[...]

> +#define R8A77990_CLK_CSI0  47

Note that CSI0 is not listed in Table 8.2g ("R-Car E3").
Probably it does exist, given:
  - Table 8.11 ("Register Configuration") says CSI0CKCR exists on R-Car E3,
  - Figure 25.6 ("CSI2 Block Diagram (R-Car E3)") shows CSI0.

> +#define R8A77990_CLK_POST3 48

I noticed these POSTx clocks have been added to all R-Car Gen3 clock tables.
It doesn't look like we will ever need to refer them from DT, so I think we
can treat them as internal clocks, and omit them from the DT bindings.

What do you think?
Thanks!

Gr{oetje,eeting}s,

Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@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

[PATCH 1/2] clk: renesas: Add r8a77990 CPG Core Clock Definitions

[Yoshihiro Shimoda]

From: Takeshi Kihara 

This patch adds all R-Car E3 Clock Pulse Generator Core Clock Outputs.

Note that internal CPG clocks (S0, S1, S2, S3, SDSRC) are not included,
as they are used as internal clock sources only, and never referenced
from DT.

Signed-off-by: Takeshi Kihara 
[shimoda: add SPDX-License-Identifier]
Signed-off-by: Yoshihiro Shimoda 
---
 include/dt-bindings/clock/r8a77990-cpg-mssr.h | 63 +++
 1 file changed, 63 insertions(+)
 create mode 100644 include/dt-bindings/clock/r8a77990-cpg-mssr.h

diff --git a/include/dt-bindings/clock/r8a77990-cpg-mssr.h 
b/include/dt-bindings/clock/r8a77990-cpg-mssr.h
new file mode 100644
index 000..c806fce
--- /dev/null
+++ b/include/dt-bindings/clock/r8a77990-cpg-mssr.h
@@ -0,0 +1,63 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2018 Renesas Electronics Corp.
+ */
+#ifndef __DT_BINDINGS_CLOCK_R8A77990_CPG_MSSR_H__
+#define __DT_BINDINGS_CLOCK_R8A77990_CPG_MSSR_H__
+
+#include 
+
+/* r8a77990 CPG Core Clocks */
+#define R8A77990_CLK_Z20
+#define R8A77990_CLK_ZR1
+#define R8A77990_CLK_ZG2
+#define R8A77990_CLK_ZTR   3
+#define R8A77990_CLK_ZT4
+#define R8A77990_CLK_ZX5
+#define R8A77990_CLK_S0D1  6
+#define R8A77990_CLK_S0D3  7
+#define R8A77990_CLK_S0D6  8
+#define R8A77990_CLK_S0D12 9
+#define R8A77990_CLK_S0D24 10
+#define R8A77990_CLK_S1D1  11
+#define R8A77990_CLK_S1D2  12
+#define R8A77990_CLK_S1D4  13
+#define R8A77990_CLK_S2D1  14
+#define R8A77990_CLK_S2D2  15
+#define R8A77990_CLK_S2D4  16
+#define R8A77990_CLK_S3D1  17
+#define R8A77990_CLK_S3D2  18
+#define R8A77990_CLK_S3D4  19
+#define R8A77990_CLK_S0D6C 20
+#define R8A77990_CLK_S3D1C 21
+#define R8A77990_CLK_S3D2C 22
+#define R8A77990_CLK_S3D4C 23
+#define R8A77990_CLK_LB24
+#define R8A77990_CLK_CL25
+#define R8A77990_CLK_ZB3   26
+#define R8A77990_CLK_ZB3D2 27
+#define R8A77990_CLK_CR28
+#define R8A77990_CLK_CRD2  29
+#define R8A77990_CLK_SD0H  30
+#define R8A77990_CLK_SD0   31
+#define R8A77990_CLK_SD1H  32
+#define R8A77990_CLK_SD1   33
+#define R8A77990_CLK_SD3H  34
+#define R8A77990_CLK_SD3   35
+#define R8A77990_CLK_RPC   36
+#define R8A77990_CLK_RPCD2 37
+#define R8A77990_CLK_ZA2   38
+#define R8A77990_CLK_ZA8   39
+#define R8A77990_CLK_Z2D   40
+#define R8A77990_CLK_CANFD 41
+#define R8A77990_CLK_MSO   42
+#define R8A77990_CLK_R 43
+#define R8A77990_CLK_OSC   44
+#define R8A77990_CLK_LV0   45
+#define R8A77990_CLK_LV1   46
+#define R8A77990_CLK_CSI0  47
+#define R8A77990_CLK_POST3 48
+#define R8A77990_CLK_CP49
+#define R8A77990_CLK_CPEX  50
+
+#endif /* __DT_BINDINGS_CLOCK_R8A77990_CPG_MSSR_H__ */
-- 
1.9.1