RE: [PATCH v2 08/14] clk: renesas: rcar-gen4-cpg: Introduce R-Car Gen4 CPG driver

Hi Geert-san,

Thank you for your comments!

From: Geert Uytterhoeven, Sent: Monday, December 6, 2021 9:43 PM

Hi Shimoda-san,

On Wed, Dec 1, 2021 at 8:33 AM Yoshihiro Shimoda
[off-list ref] wrote:

quoted

According to the official website [1], the R-Car V3U SoC is based
on the R-Car Gen4 architecture. So, introduce R-Car Gen4 CPG
driver.

[1]

https://www.renesas.com/us/en/products/automotive-products/automotive-system-chips-socs/r-car-v3u-best-class-r-car-v
3u-asil-d-system-chip-automated-driving

quoted

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>

Thanks for your patch!

quoted

--- /dev/null
+++ b/drivers/clk/renesas/rcar-gen4-cpg.c

quoted

+/*
+ * RPC Clocks
+ */
+#define CPG_RPCCKCR 0x874

This is also defined in rcar-gen4-cpg.h, so I will drop it while applying.

Thanks!

quoted

+

quoted

--- /dev/null
+++ b/drivers/clk/renesas/rcar-gen4-cpg.h

@@ -0,0 +1,76 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * R-Car Gen4 Clock Pulse Generator
+ *
+ * Copyright (C) 2021 Renesas Electronics Corp.
+ *
+ */
+
+#ifndef __CLK_RENESAS_RCAR_GEN4_CPG_H__
+#define __CLK_RENESAS_RCAR_GEN4_CPG_H__
+
+enum rcar_gen4_clk_types {
+       CLK_TYPE_GEN4_MAIN = CLK_TYPE_CUSTOM,
+       CLK_TYPE_GEN4_PLL1,
+       CLK_TYPE_GEN4_PLL2,
+       CLK_TYPE_GEN4_PLL2X_3X, /* r8a779a0 only */
+       CLK_TYPE_GEN4_PLL3,
+       CLK_TYPE_GEN4_PLL5,
+       CLK_TYPE_GEN4_PLL6,
+       CLK_TYPE_GEN4_SDSRC,
+       CLK_TYPE_GEN4_SDH,
+       CLK_TYPE_GEN4_SD,
+       CLK_TYPE_GEN4_MDSEL,    /* Select parent/divider using mode pin */
+       CLK_TYPE_GEN4_Z,
+       CLK_TYPE_GEN4_OSC,      /* OSC EXTAL predivider and fixed divider */
+       CLK_TYPE_GEN4_RPCSRC,
+       CLK_TYPE_GEN4_RPC,
+       CLK_TYPE_GEN4_RPCD2,
+
+       /* SoC specific definitions start here */
+       CLK_TYPE_GEN4_SOC_BASE,
+};
+
+#define DEF_GEN4_SDH(_name, _id, _parent, _offset)     \
+       DEF_BASE(_name, _id, CLK_TYPE_GEN4_SDH, _parent, .offset = _offset)
+
+#define DEF_GEN4_SD(_name, _id, _parent, _offset)      \
+       DEF_BASE(_name, _id, CLK_TYPE_GEN4_SD, _parent, .offset = _offset)
+
+#define DEF_GEN4_MDSEL(_name, _id, _md, _parent0, _div0, _parent1, _div1) \
+       DEF_BASE(_name, _id, CLK_TYPE_GEN4_MDSEL,       \
+                (_parent0) << 16 | (_parent1),         \
+                .div = (_div0) << 16 | (_div1), .offset = _md)
+
+#define DEF_GEN4_OSC(_name, _id, _parent, _div)                \
+       DEF_BASE(_name, _id, CLK_TYPE_GEN4_OSC, _parent, .div = _div)
+
+#define DEF_GEN4_Z(_name, _id, _type, _parent, _div, _offset)  \
+       DEF_BASE(_name, _id, _type, _parent, .div = _div, .offset = _offset)

Is there any specific reason _type is not fixed to CLK_TYPE_GEN4_Z,
like before? Perhaps you have a future use-case in mind?

This is a similar definition with DEF_GEN3_Z. And, there is not
upstream use-case though, if we support ZG clock, we have to use
_type for ZG like Gen3 BSP. In Gen4, we will support ZG clock
on other SoCs in the future, the _type is not fixed.

quoted

+
+struct rcar_gen4_cpg_pll_config {
+       u8 extal_div;
+       u8 pll1_mult;
+       u8 pll1_div;
+       u8 pll2_mult;
+       u8 pll2_div;
+       u8 pll3_mult;
+       u8 pll3_div;
+       u8 pll5_mult;
+       u8 pll5_div;
+       u8 pll6_mult;
+       u8 pll6_div;
+       u8 osc_prediv;
+};
+
+#define CPG_RPCCKCR    0x874
+#define SD0CKCR1       0x8a4

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
i.e. will queue in renesas-clk-for-v5.17 when the above has been sorted
out.

Thank you very much for your review!

Best regards,
Yoshihiro Shimoda