[PATCH v4 0/3] Add CPG support for RZ/V2H(P) SoC

[PATCH v4 0/3] Add CPG support for RZ/V2H(P) SoC

[Prabhakar]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Hi All,

This patch series aims to add the CPG support for the Renesas
RZ/V2H(P) SoC.

v3 -> v4
- Included RB tag form Krzysztof for patch 1/3
- Changed reset and clock values to hex
- Renamed __DT_BINDINGS_CLOCK_R9A09G057_CPG_H__ to
  __DT_BINDINGS_CLOCK_RENESAS_R9A09G057_CPG_H__
- Dropped masking of parent clks with 0xffff
- Dropped storing mod clk id and now calculating it
  based on index and bit.
- Made parent to u16 in struct rzv2h_mod_clk
- Made a copy of resets array in struct rzv2h_cpg_priv
- Added a comment for core clocks in patch 3/3
- Made r9a09g057_mod_clks as __initconst
- Made r9a09g057_resets as __initconst
- Made r9a09g057_cpg_info as __initconst

v2 -> v3
- Dropped '|' for CPG description
- Dropped description for '#power-domain-cells' property
- Added 3 clock inputs for CPG
- Dropped label in example node
- Used 4 spaces for example node
- Renamed r9a09g057-cpg.h -> renesas,r9a09g057-cpg.h
- Merged adding renesas,r9a09g057-cpg.h in DT binding patch
- Updated commit message for patch 1/3
- Dropped num_hw_resets from struct rzv2h_cpg_priv
- Dropped range_check for module clocks
- Made mon_index to s8 instead of u8 in struct rzv2h_mod_clk
- Added support for critical module clocks with DEF_MOD_CRITICAL
- Added check for mon_index in rzv2h_mod_clock_endisable and
  rzv2h_mod_clock_is_enabled()
- Added CLK_PLLDTY
- Added core clocks sys_0_pclk and iotop_0_shclk
- Dropped r9a09g057_crit_mod_clks

v1 -> v2
- Updated commit message
- Updated description for binding as suggested by Geert
- Updated descriptions for clocks and resets property
- Renamed extal->qextal
- Updated '#power-domain-cells' value
- Dropped the module clocks and just added the core clocks
- Introduced family specific config option
- Now using register indexes for CLKON/CLKMON/RST/RSTMON
- Introduced PLL_CONF macro
- Dropped function pointer to get PLL_CLK1/2 offsets
- Added range check for core clks
- Dropped NULLified clocks check
- Dropped pll_clk1/clk2_offset
- Made r9a09g057_mod_clks/r9a09g057_resets as static const

v1: https://patchwork.kernel.org/project/linux-renesas-soc/cover/20240524082800.333991-1-prabhakar.mahadev-lad.rj@bp.renesas.com/

Cheers,
Prabhakar

Lad Prabhakar (3):
  dt-bindings: clock: renesas: Document RZ/V2H(P) SoC CPG
  clk: renesas: Add family-specific clock driver for RZ/V2H(P)
  clk: renesas: Add RZ/V2H(P) CPG driver

 .../bindings/clock/renesas,rzv2h-cpg.yaml     |  80 ++
 drivers/clk/renesas/Kconfig                   |   9 +
 drivers/clk/renesas/Makefile                  |   2 +
 drivers/clk/renesas/r9a09g057-cpg.c           |  80 ++
 drivers/clk/renesas/rzv2h-cpg.c               | 685 ++++++++++++++++++
 drivers/clk/renesas/rzv2h-cpg.h               | 153 ++++
 .../dt-bindings/clock/renesas,r9a09g057-cpg.h |  21 +
 7 files changed, 1030 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/renesas,rzv2h-cpg.yaml
 create mode 100644 drivers/clk/renesas/r9a09g057-cpg.c
 create mode 100644 drivers/clk/renesas/rzv2h-cpg.c
 create mode 100644 drivers/clk/renesas/rzv2h-cpg.h
 create mode 100644 include/dt-bindings/clock/renesas,r9a09g057-cpg.h

-- 
2.34.1

[PATCH v4 1/3] dt-bindings: clock: renesas: Document RZ/V2H(P) SoC CPG

[Prabhakar]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Document the device tree bindings for the Renesas RZ/V2H(P) SoC
Clock Pulse Generator (CPG).

CPG block handles the below operations:
- Generation and control of clock signals for the IP modules
- Generation and control of resets
- Control over booting
- Low power consumption and power supply domains

Also define constants for the core clocks of the RZ/V2H(P) SoC. Note the
core clocks are a subset of the ones which are listed as part of section
4.4.2 of HW manual Rev.1.01 which cannot be controlled by CLKON register.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
v3->v4
- Included RB tag form Krzysztof
- Changed reset and clock values to hex
- Renamed __DT_BINDINGS_CLOCK_R9A09G057_CPG_H__ to
  __DT_BINDINGS_CLOCK_RENESAS_R9A09G057_CPG_H__

v2->v3
- Dropped '|' for CPG description
- Dropped description for '#power-domain-cells' property
- Added 3 clock inputs for CPG
- Dropped label in example node
- Used 4 spaces for example node
- Renamed r9a09g057-cpg.h -> renesas,r9a09g057-cpg.h
- Merged adding renesas,r9a09g057-cpg.h in DT binding patch
- Updated commit message

v1->v2
- Updated commit message
- Updated description for binding as suggested by Geert
- Updated descriptions for clocks and resets property
- Renamed extal->qextal
- Updated '#power-domain-cells' value
---
 .../bindings/clock/renesas,rzv2h-cpg.yaml     | 80 +++++++++++++++++++
 .../dt-bindings/clock/renesas,r9a09g057-cpg.h | 21 +++++
 2 files changed, 101 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/renesas,rzv2h-cpg.yaml
 create mode 100644 include/dt-bindings/clock/renesas,r9a09g057-cpg.h

diff --git a/Documentation/devicetree/bindings/clock/renesas,rzv2h-cpg.yaml b/Documentation/devicetree/bindings/clock/renesas,rzv2h-cpg.yaml
new file mode 100644
index 000000000000..926c503bed1f
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/renesas,rzv2h-cpg.yaml
@@ -0,0 +1,80 @@
+# 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) SoCs, the CPG (Clock Pulse Generator) handles generation
+  and control of clock signals for the IP modules, generation and control of resets,
+  and control over booting, low power consumption and power supply domains.
+
+properties:
+  compatible:
+    const: renesas,r9a09g057-cpg
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: AUDIO_EXTAL clock input
+      - description: RTXIN clock input
+      - description: QEXTAL clock input
+
+  clock-names:
+    items:
+      - const: audio_extal
+      - const: rtxin
+      - const: qextal
+
+  '#clock-cells':
+    description: |
+      - For CPG core clocks, the two clock specifier cells must be "CPG_CORE"
+        and a core clock reference, as defined in
+        <dt-bindings/clock/renesas,r9a09g057-cpg.h>,
+      - For module clocks, the two clock specifier cells must be "CPG_MOD" and
+        a module number.  The module number is calculated as the CLKON register
+        offset index multiplied by 16, plus the actual bit in the register
+        used to turn the CLK ON. For example, for CGC_GIC_0_GICCLK, the
+        calculation is (1 * 16 + 3) = 0x13.
+    const: 2
+
+  '#power-domain-cells':
+    const: 0
+
+  '#reset-cells':
+    description:
+      The single reset specifier cell must be the reset number. The reset number
+      is calculated as the reset register offset index multiplied by 16, plus the
+      actual bit in the register used to reset the specific IP block. For example,
+      for SYS_0_PRESETN, the calculation is (3 * 16 + 0) = 0x30.
+    const: 1
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - '#clock-cells'
+  - '#power-domain-cells'
+  - '#reset-cells'
+
+additionalProperties: false
+
+examples:
+  - |
+    clock-controller@10420000 {
+        compatible = "renesas,r9a09g057-cpg";
+        reg = <0x10420000 0x10000>;
+        clocks = <&audio_extal_clk>, <&rtxin_clk>, <&qextal_clk>;
+        clock-names = "audio_extal", "rtxin", "qextal";
+        #clock-cells = <2>;
+        #power-domain-cells = <0>;
+        #reset-cells = <1>;
+    };
diff --git a/include/dt-bindings/clock/renesas,r9a09g057-cpg.h b/include/dt-bindings/clock/renesas,r9a09g057-cpg.h
new file mode 100644
index 000000000000..541e6d719bd6
--- /dev/null
+++ b/include/dt-bindings/clock/renesas,r9a09g057-cpg.h
@@ -0,0 +1,21 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+ *
+ * Copyright (C) 2024 Renesas Electronics Corp.
+ */
+#ifndef __DT_BINDINGS_CLOCK_RENESAS_R9A09G057_CPG_H__
+#define __DT_BINDINGS_CLOCK_RENESAS_R9A09G057_CPG_H__
+
+#include <dt-bindings/clock/renesas-cpg-mssr.h>
+
+/* Core Clock list */
+#define R9A09G057_SYS_0_PCLK			0
+#define R9A09G057_CA55_0_CORE_CLK0		1
+#define R9A09G057_CA55_0_CORE_CLK1		2
+#define R9A09G057_CA55_0_CORE_CLK2		3
+#define R9A09G057_CA55_0_CORE_CLK3		4
+#define R9A09G057_CA55_0_PERIPHCLK		5
+#define R9A09G057_CM33_CLK0			6
+#define R9A09G057_CST_0_SWCLKTCK		7
+#define R9A09G057_IOTOP_0_SHCLK			8
+
+#endif /* __DT_BINDINGS_CLOCK_RENESAS_R9A09G057_CPG_H__ */
-- 
2.34.1

Re: [PATCH v4 1/3] dt-bindings: clock: renesas: Document RZ/V2H(P) SoC CPG

[Geert Uytterhoeven]

On Mon, Jul 15, 2024 at 2:56 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Document the device tree bindings for the Renesas RZ/V2H(P) SoC
> Clock Pulse Generator (CPG).
>
> CPG block handles the below operations:
> - Generation and control of clock signals for the IP modules
> - Generation and control of resets
> - Control over booting
> - Low power consumption and power supply domains
>
> Also define constants for the core clocks of the RZ/V2H(P) SoC. Note the
> core clocks are a subset of the ones which are listed as part of section
> 4.4.2 of HW manual Rev.1.01 which cannot be controlled by CLKON register.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
> v3->v4
> - Included RB tag form Krzysztof
> - Changed reset and clock values to hex
> - Renamed __DT_BINDINGS_CLOCK_R9A09G057_CPG_H__ to
>   __DT_BINDINGS_CLOCK_RENESAS_R9A09G057_CPG_H__

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

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

[PATCH v4 2/3] clk: renesas: Add family-specific clock driver for RZ/V2H(P)

[Prabhakar]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Add family-specific clock driver for RZ/V2H(P) SoCs.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
v3->v4
- Dropped masking of parent clks with 0xffff
- Dropped storing mod clk id and now calculating it
  based on index and bit.
- Made parent to u16 in struct rzv2h_mod_clk
- Made a copy of resets array in struct rzv2h_cpg_priv

v2->v3
- Dropped num_hw_resets from struct rzv2h_cpg_priv
- Dropped range_check for module clocks
- Made mon_index to s8 instead of u8 in struct rzv2h_mod_clk
- Added support for critical module clocks with DEF_MOD_CRITICAL
- Added check for mon_index in rzv2h_mod_clock_endisable and
  rzv2h_mod_clock_is_enabled()

v1->v2
- Introduced family specific config option
- Now using register indexes for CLKON/CLKMON/RST/RSTMON
- Introduced PLL_CONF macro
- Dropped function pointer to get PLL_CLK1/2 offsets
- Added range check for core clks
- Dropped NULLified clocks check
- Updated commit description
---
 drivers/clk/renesas/Kconfig     |   4 +
 drivers/clk/renesas/Makefile    |   1 +
 drivers/clk/renesas/rzv2h-cpg.c | 681 ++++++++++++++++++++++++++++++++
 drivers/clk/renesas/rzv2h-cpg.h | 151 +++++++
 4 files changed, 837 insertions(+)
 create mode 100644 drivers/clk/renesas/rzv2h-cpg.c
 create mode 100644 drivers/clk/renesas/rzv2h-cpg.h

diff --git a/drivers/clk/renesas/Kconfig b/drivers/clk/renesas/Kconfig
index 4410d16de4e2..f078ccb635bb 100644
--- a/drivers/clk/renesas/Kconfig
+++ b/drivers/clk/renesas/Kconfig
@@ -228,6 +228,10 @@ config CLK_RZG2L
 	bool "RZ/{G2L,G2UL,G3S,V2L} family clock support" if COMPILE_TEST
 	select RESET_CONTROLLER
 
+config CLK_RZV2H
+	bool "RZ/V2H(P) family clock support" if COMPILE_TEST
+	select RESET_CONTROLLER
+
 # Generic
 config CLK_RENESAS_CPG_MSSR
 	bool "CPG/MSSR clock support" if COMPILE_TEST
diff --git a/drivers/clk/renesas/Makefile b/drivers/clk/renesas/Makefile
index f7e18679c3b8..d81a62e78345 100644
--- a/drivers/clk/renesas/Makefile
+++ b/drivers/clk/renesas/Makefile
@@ -46,6 +46,7 @@ obj-$(CONFIG_CLK_RCAR_GEN3_CPG)		+= rcar-gen3-cpg.o
 obj-$(CONFIG_CLK_RCAR_GEN4_CPG)		+= rcar-gen4-cpg.o
 obj-$(CONFIG_CLK_RCAR_USB2_CLOCK_SEL)	+= rcar-usb2-clock-sel.o
 obj-$(CONFIG_CLK_RZG2L)			+= rzg2l-cpg.o
+obj-$(CONFIG_CLK_RZV2H)			+= rzv2h-cpg.o
 
 # Generic
 obj-$(CONFIG_CLK_RENESAS_CPG_MSSR)	+= renesas-cpg-mssr.o
diff --git a/drivers/clk/renesas/rzv2h-cpg.c b/drivers/clk/renesas/rzv2h-cpg.c
new file mode 100644
index 000000000000..fc3ff7a65aac
--- /dev/null
+++ b/drivers/clk/renesas/rzv2h-cpg.c
@@ -0,0 +1,681 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Renesas RZ/V2H(P) Clock Pulse Generator
+ *
+ * Copyright (C) 2024 Renesas Electronics Corp.
+ *
+ * Based on rzg2l-cpg.c
+ *
+ * Copyright (C) 2015 Glider bvba
+ * Copyright (C) 2013 Ideas On Board SPRL
+ * Copyright (C) 2015 Renesas Electronics Corp.
+ */
+
+#include <linux/bitfield.h>
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/delay.h>
+#include <linux/init.h>
+#include <linux/iopoll.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/pm_clock.h>
+#include <linux/pm_domain.h>
+#include <linux/reset-controller.h>
+
+#include <dt-bindings/clock/renesas-cpg-mssr.h>
+
+#include "rzv2h-cpg.h"
+
+#ifdef DEBUG
+#define WARN_DEBUG(x)		WARN_ON(x)
+#else
+#define WARN_DEBUG(x)		do { } while (0)
+#endif
+
+#define GET_CLK_ON_OFFSET(x)	(0x600 + ((x) * 4))
+#define GET_CLK_MON_OFFSET(x)	(0x800 + ((x) * 4))
+#define GET_RST_OFFSET(x)	(0x900 + ((x) * 4))
+#define GET_RST_MON_OFFSET(x)	(0xA00 + ((x) * 4))
+
+#define KDIV(val)		((s16)FIELD_GET(GENMASK(31, 16), (val)))
+#define MDIV(val)		FIELD_GET(GENMASK(15, 6), (val))
+#define PDIV(val)		FIELD_GET(GENMASK(5, 0), (val))
+#define SDIV(val)		FIELD_GET(GENMASK(2, 0), (val))
+
+#define GET_MOD_CLK_ID(base, index, bit)		\
+			((base) + ((((index) * (16))) + (bit)))
+
+/**
+ * struct rzv2h_cpg_priv - Clock Pulse Generator Private Data
+ *
+ * @info: Pointer to platform data
+ * @dev: CPG device
+ * @base: CPG register block base address
+ * @clks: Array containing all Core and Module Clocks
+ * @num_core_clks: Number of Core Clocks in clks[]
+ * @num_mod_clks: Number of Module Clocks in clks[]
+ * @resets: Array of resets
+ * @num_resets: Number of Module Resets in info->resets[]
+ * @last_dt_core_clk: ID of the last Core Clock exported to DT
+ * @rcdev: Reset controller entity
+ */
+struct rzv2h_cpg_priv {
+	struct device *dev;
+	void __iomem *base;
+
+	struct clk **clks;
+	unsigned int num_core_clks;
+	unsigned int num_mod_clks;
+	struct rzv2h_reset *resets;
+	unsigned int num_resets;
+	unsigned int last_dt_core_clk;
+
+	struct reset_controller_dev rcdev;
+};
+
+#define rcdev_to_priv(x)	container_of(x, struct rzv2h_cpg_priv, rcdev)
+
+struct pll_clk {
+	struct rzv2h_cpg_priv *priv;
+	void __iomem *base;
+	struct clk_hw hw;
+	unsigned int conf;
+	unsigned int type;
+};
+
+#define to_pll(_hw)	container_of(_hw, struct pll_clk, hw)
+
+/**
+ * struct mod_clock - Module clock
+ *
+ * @priv: CPG private data
+ * @hw: handle between common and hardware-specific interfaces
+ * @on_index: register offset
+ * @on_bit: ON/MON bit
+ * @mon_index: monitor register offset
+ * @mon_bit: montor bit
+ */
+struct mod_clock {
+	struct rzv2h_cpg_priv *priv;
+	struct clk_hw hw;
+	u8 on_index;
+	u8 on_bit;
+	s8 mon_index;
+	u8 mon_bit;
+};
+
+#define to_mod_clock(_hw) container_of(_hw, struct mod_clock, hw)
+
+static unsigned long rzv2h_cpg_pll_clk_recalc_rate(struct clk_hw *hw,
+						   unsigned long parent_rate)
+{
+	struct pll_clk *pll_clk = to_pll(hw);
+	struct rzv2h_cpg_priv *priv = pll_clk->priv;
+	unsigned int clk1, clk2;
+	u64 rate;
+
+	if (!PLL_CLK_ACCESS(pll_clk->conf))
+		return 0;
+
+	clk1 = readl(priv->base + PLL_CLK1_OFFSET(pll_clk->conf));
+	clk2 = readl(priv->base + PLL_CLK2_OFFSET(pll_clk->conf));
+
+	rate = mul_u64_u32_shr(parent_rate, (MDIV(clk1) << 16) + KDIV(clk1),
+			       16 + SDIV(clk2));
+
+	return DIV_ROUND_CLOSEST_ULL(rate, PDIV(clk1));
+}
+
+static const struct clk_ops rzv2h_cpg_pll_ops = {
+	.recalc_rate = rzv2h_cpg_pll_clk_recalc_rate,
+};
+
+static struct clk * __init
+rzv2h_cpg_pll_clk_register(const struct cpg_core_clk *core,
+			   struct rzv2h_cpg_priv *priv,
+			   const struct clk_ops *ops)
+{
+	void __iomem *base = priv->base;
+	struct device *dev = priv->dev;
+	struct clk_init_data init;
+	const struct clk *parent;
+	const char *parent_name;
+	struct pll_clk *pll_clk;
+	int ret;
+
+	parent = priv->clks[core->parent];
+	if (IS_ERR(parent))
+		return ERR_CAST(parent);
+
+	pll_clk = devm_kzalloc(dev, sizeof(*pll_clk), GFP_KERNEL);
+	if (!pll_clk)
+		return ERR_PTR(-ENOMEM);
+
+	parent_name = __clk_get_name(parent);
+	init.name = core->name;
+	init.ops = ops;
+	init.flags = 0;
+	init.parent_names = &parent_name;
+	init.num_parents = 1;
+
+	pll_clk->hw.init = &init;
+	pll_clk->conf = core->conf;
+	pll_clk->base = base;
+	pll_clk->priv = priv;
+	pll_clk->type = core->type;
+
+	ret = devm_clk_hw_register(dev, &pll_clk->hw);
+	if (ret)
+		return ERR_PTR(ret);
+
+	return pll_clk->hw.clk;
+}
+
+static struct clk
+*rzv2h_cpg_clk_src_twocell_get(struct of_phandle_args *clkspec,
+			       void *data)
+{
+	unsigned int clkidx = clkspec->args[1];
+	struct rzv2h_cpg_priv *priv = data;
+	struct device *dev = priv->dev;
+	const char *type;
+	struct clk *clk;
+
+	switch (clkspec->args[0]) {
+	case CPG_CORE:
+		type = "core";
+		if (clkidx > priv->last_dt_core_clk) {
+			dev_err(dev, "Invalid %s clock index %u\n", type, clkidx);
+			return ERR_PTR(-EINVAL);
+		}
+		clk = priv->clks[clkidx];
+		break;
+
+	case CPG_MOD:
+		type = "module";
+		if (clkidx >= priv->num_mod_clks) {
+			dev_err(dev, "Invalid %s clock index %u\n", type, clkidx);
+			return ERR_PTR(-EINVAL);
+		}
+		clk = priv->clks[priv->num_core_clks + clkidx];
+		break;
+
+	default:
+		dev_err(dev, "Invalid CPG clock type %u\n", clkspec->args[0]);
+		return ERR_PTR(-EINVAL);
+	}
+
+	if (IS_ERR(clk))
+		dev_err(dev, "Cannot get %s clock %u: %ld", type, clkidx,
+			PTR_ERR(clk));
+	else
+		dev_dbg(dev, "clock (%u, %u) is %pC at %lu Hz\n",
+			clkspec->args[0], clkspec->args[1], clk,
+			clk_get_rate(clk));
+	return clk;
+}
+
+static void __init
+rzv2h_cpg_register_core_clk(const struct cpg_core_clk *core,
+			    struct rzv2h_cpg_priv *priv)
+{
+	struct clk *clk = ERR_PTR(-EOPNOTSUPP), *parent;
+	unsigned int id = core->id, div = core->div;
+	struct device *dev = priv->dev;
+	const char *parent_name;
+	struct clk_hw *clk_hw;
+
+	WARN_DEBUG(id >= priv->num_core_clks);
+	WARN_DEBUG(PTR_ERR(priv->clks[id]) != -ENOENT);
+
+	switch (core->type) {
+	case CLK_TYPE_IN:
+		clk = of_clk_get_by_name(priv->dev->of_node, core->name);
+		break;
+	case CLK_TYPE_FF:
+		WARN_DEBUG(core->parent >= priv->num_core_clks);
+		parent = priv->clks[core->parent];
+		if (IS_ERR(parent)) {
+			clk = parent;
+			goto fail;
+		}
+
+		parent_name = __clk_get_name(parent);
+		clk_hw = devm_clk_hw_register_fixed_factor(dev, core->name,
+							   parent_name, CLK_SET_RATE_PARENT,
+							   core->mult, div);
+		if (IS_ERR(clk_hw)) {
+			clk = ERR_CAST(clk_hw);
+			break;
+		}
+		clk = clk_hw->clk;
+		break;
+	case CLK_TYPE_PLL:
+		clk = rzv2h_cpg_pll_clk_register(core, priv, &rzv2h_cpg_pll_ops);
+		break;
+	default:
+		goto fail;
+	}
+
+	if (IS_ERR_OR_NULL(clk))
+		goto fail;
+
+	dev_dbg(dev, "Core clock %pC at %lu Hz\n", clk, clk_get_rate(clk));
+	priv->clks[id] = clk;
+	return;
+
+fail:
+	dev_err(dev, "Failed to register core clock %s: %ld\n",
+		core->name, PTR_ERR(clk));
+}
+
+static int rzv2h_mod_clock_endisable(struct clk_hw *hw, bool enable)
+{
+	struct mod_clock *clock = to_mod_clock(hw);
+	unsigned int reg = GET_CLK_ON_OFFSET(clock->on_index);
+	struct rzv2h_cpg_priv *priv = clock->priv;
+	u32 bitmask = BIT(clock->on_bit);
+	struct device *dev = priv->dev;
+	u32 value;
+	int error;
+
+	dev_dbg(dev, "CLK_ON 0x%x/%pC %s\n", reg, hw->clk,
+		enable ? "ON" : "OFF");
+
+	value = bitmask << 16;
+	if (enable)
+		value |= bitmask;
+
+	writel(value, priv->base + reg);
+
+	if (!enable || clock->mon_index < 0)
+		return 0;
+
+	reg = GET_CLK_MON_OFFSET(clock->mon_index);
+	bitmask = BIT(clock->mon_bit);
+	error = readl_poll_timeout_atomic(priv->base + reg, value,
+					  value & bitmask, 0, 10);
+	if (error)
+		dev_err(dev, "Failed to enable CLK_ON %p\n",
+			priv->base + reg);
+
+	return error;
+}
+
+static int rzv2h_mod_clock_enable(struct clk_hw *hw)
+{
+	return rzv2h_mod_clock_endisable(hw, true);
+}
+
+static void rzv2h_mod_clock_disable(struct clk_hw *hw)
+{
+	rzv2h_mod_clock_endisable(hw, false);
+}
+
+static int rzv2h_mod_clock_is_enabled(struct clk_hw *hw)
+{
+	struct mod_clock *clock = to_mod_clock(hw);
+	struct rzv2h_cpg_priv *priv = clock->priv;
+	u32 bitmask;
+	u32 offset;
+
+	if (clock->mon_index >= 0) {
+		offset = GET_CLK_MON_OFFSET(clock->mon_index);
+		bitmask = BIT(clock->mon_bit);
+	} else {
+		offset = GET_CLK_ON_OFFSET(clock->on_index);
+		bitmask = BIT(clock->on_bit);
+	}
+
+	return readl(priv->base + offset) & bitmask;
+}
+
+static const struct clk_ops rzv2h_mod_clock_ops = {
+	.enable = rzv2h_mod_clock_enable,
+	.disable = rzv2h_mod_clock_disable,
+	.is_enabled = rzv2h_mod_clock_is_enabled,
+};
+
+static void __init
+rzv2h_cpg_register_mod_clk(const struct rzv2h_mod_clk *mod,
+			   struct rzv2h_cpg_priv *priv)
+{
+	struct mod_clock *clock = NULL;
+	struct device *dev = priv->dev;
+	struct clk_init_data init;
+	struct clk *parent, *clk;
+	const char *parent_name;
+	unsigned int id;
+	int ret;
+
+	id = GET_MOD_CLK_ID(priv->num_core_clks, mod->on_index, mod->on_bit);
+	WARN_DEBUG(id >= priv->num_core_clks + priv->num_mod_clks);
+	WARN_DEBUG(mod->parent >= priv->num_core_clks + priv->num_mod_clks);
+	WARN_DEBUG(PTR_ERR(priv->clks[id]) != -ENOENT);
+
+	parent = priv->clks[mod->parent];
+	if (IS_ERR(parent)) {
+		clk = parent;
+		goto fail;
+	}
+
+	clock = devm_kzalloc(dev, sizeof(*clock), GFP_KERNEL);
+	if (!clock) {
+		clk = ERR_PTR(-ENOMEM);
+		goto fail;
+	}
+
+	init.name = mod->name;
+	init.ops = &rzv2h_mod_clock_ops;
+	init.flags = CLK_SET_RATE_PARENT;
+	if (mod->critical)
+		init.flags |= CLK_IS_CRITICAL;
+
+	parent_name = __clk_get_name(parent);
+	init.parent_names = &parent_name;
+	init.num_parents = 1;
+
+	clock->on_index = mod->on_index;
+	clock->on_bit = mod->on_bit;
+	clock->mon_index = mod->mon_index;
+	clock->mon_bit = mod->mon_bit;
+	clock->priv = priv;
+	clock->hw.init = &init;
+
+	ret = devm_clk_hw_register(dev, &clock->hw);
+	if (ret) {
+		clk = ERR_PTR(ret);
+		goto fail;
+	}
+
+	priv->clks[id] = clock->hw.clk;
+
+	return;
+
+fail:
+	dev_err(dev, "Failed to register module clock %s: %ld\n",
+		mod->name, PTR_ERR(clk));
+}
+
+static int rzv2h_cpg_assert(struct reset_controller_dev *rcdev,
+			    unsigned long id)
+{
+	struct rzv2h_cpg_priv *priv = rcdev_to_priv(rcdev);
+	unsigned int reg = GET_RST_OFFSET(priv->resets[id].reset_index);
+	u32 mask = BIT(priv->resets[id].reset_bit);
+	u8 monbit = priv->resets[id].mon_bit;
+	u32 value = mask << 16;
+
+	dev_dbg(rcdev->dev, "assert id:%ld offset:0x%x\n", id, reg);
+
+	writel(value, priv->base + reg);
+
+	reg = GET_RST_MON_OFFSET(priv->resets[id].mon_index);
+	mask = BIT(monbit);
+
+	return readl_poll_timeout_atomic(priv->base + reg, value,
+					 value & mask, 10, 200);
+}
+
+static int rzv2h_cpg_deassert(struct reset_controller_dev *rcdev,
+			      unsigned long id)
+{
+	struct rzv2h_cpg_priv *priv = rcdev_to_priv(rcdev);
+	unsigned int reg = GET_RST_OFFSET(priv->resets[id].reset_index);
+	u32 mask = BIT(priv->resets[id].reset_bit);
+	u8 monbit = priv->resets[id].mon_bit;
+	u32 value = (mask << 16) | mask;
+
+	dev_dbg(rcdev->dev, "deassert id:%ld offset:0x%x\n", id, reg);
+
+	writel(value, priv->base + reg);
+
+	reg = GET_RST_MON_OFFSET(priv->resets[id].mon_index);
+	mask = BIT(monbit);
+
+	return readl_poll_timeout_atomic(priv->base + reg, value,
+					 !(value & mask), 10, 200);
+}
+
+static int rzv2h_cpg_reset(struct reset_controller_dev *rcdev,
+			   unsigned long id)
+{
+	int ret;
+
+	ret = rzv2h_cpg_assert(rcdev, id);
+	if (ret)
+		return ret;
+
+	return rzv2h_cpg_deassert(rcdev, id);
+}
+
+static int rzv2h_cpg_status(struct reset_controller_dev *rcdev,
+			    unsigned long id)
+{
+	struct rzv2h_cpg_priv *priv = rcdev_to_priv(rcdev);
+	unsigned int reg = GET_RST_MON_OFFSET(priv->resets[id].mon_index);
+	u8 monbit = priv->resets[id].mon_bit;
+
+	return !!(readl(priv->base + reg) & BIT(monbit));
+}
+
+static const struct reset_control_ops rzv2h_cpg_reset_ops = {
+	.reset = rzv2h_cpg_reset,
+	.assert = rzv2h_cpg_assert,
+	.deassert = rzv2h_cpg_deassert,
+	.status = rzv2h_cpg_status,
+};
+
+static int rzv2h_cpg_reset_xlate(struct reset_controller_dev *rcdev,
+				 const struct of_phandle_args *reset_spec)
+{
+	unsigned int id = reset_spec->args[0];
+
+	if (id >= rcdev->nr_resets) {
+		dev_err(rcdev->dev, "Invalid reset index %u\n", id);
+		return -EINVAL;
+	}
+
+	return id;
+}
+
+static int rzv2h_cpg_reset_controller_register(struct rzv2h_cpg_priv *priv)
+{
+	priv->rcdev.ops = &rzv2h_cpg_reset_ops;
+	priv->rcdev.of_node = priv->dev->of_node;
+	priv->rcdev.dev = priv->dev;
+	priv->rcdev.of_reset_n_cells = 1;
+	priv->rcdev.of_xlate = rzv2h_cpg_reset_xlate;
+	priv->rcdev.nr_resets = priv->num_resets;
+
+	return devm_reset_controller_register(priv->dev, &priv->rcdev);
+}
+
+/**
+ * struct rzv2h_cpg_pd - RZ/V2H power domain data structure
+ * @priv: pointer to CPG private data structure
+ * @genpd: generic PM domain
+ */
+struct rzv2h_cpg_pd {
+	struct rzv2h_cpg_priv *priv;
+	struct generic_pm_domain genpd;
+};
+
+static int rzv2h_cpg_attach_dev(struct generic_pm_domain *domain, struct device *dev)
+{
+	struct device_node *np = dev->of_node;
+	struct of_phandle_args clkspec;
+	bool once = true;
+	struct clk *clk;
+	int error;
+	int i = 0;
+
+	while (!of_parse_phandle_with_args(np, "clocks", "#clock-cells", i,
+					   &clkspec)) {
+		if (once) {
+			once = false;
+			error = pm_clk_create(dev);
+			if (error) {
+				of_node_put(clkspec.np);
+				goto err;
+			}
+		}
+		clk = of_clk_get_from_provider(&clkspec);
+		of_node_put(clkspec.np);
+		if (IS_ERR(clk)) {
+			error = PTR_ERR(clk);
+			goto fail_destroy;
+		}
+
+		error = pm_clk_add_clk(dev, clk);
+		if (error) {
+			dev_err(dev, "pm_clk_add_clk failed %d\n",
+				error);
+			goto fail_put;
+		}
+		i++;
+	}
+
+	return 0;
+
+fail_put:
+	clk_put(clk);
+
+fail_destroy:
+	pm_clk_destroy(dev);
+err:
+	return error;
+}
+
+static void rzv2h_cpg_detach_dev(struct generic_pm_domain *unused, struct device *dev)
+{
+	if (!pm_clk_no_clocks(dev))
+		pm_clk_destroy(dev);
+}
+
+static void rzv2h_cpg_genpd_remove_simple(void *data)
+{
+	pm_genpd_remove(data);
+}
+
+static int __init rzv2h_cpg_add_pm_domains(struct rzv2h_cpg_priv *priv)
+{
+	struct device *dev = priv->dev;
+	struct device_node *np = dev->of_node;
+	struct rzv2h_cpg_pd *pd;
+	int ret;
+
+	pd = devm_kzalloc(dev, sizeof(*pd), GFP_KERNEL);
+	if (!pd)
+		return -ENOMEM;
+
+	pd->genpd.name = np->name;
+	pd->priv = priv;
+	pd->genpd.flags |= GENPD_FLAG_ALWAYS_ON | GENPD_FLAG_PM_CLK | GENPD_FLAG_ACTIVE_WAKEUP;
+	pd->genpd.attach_dev = rzv2h_cpg_attach_dev;
+	pd->genpd.detach_dev = rzv2h_cpg_detach_dev;
+	ret = pm_genpd_init(&pd->genpd, &pm_domain_always_on_gov, false);
+	if (ret)
+		return ret;
+
+	ret = devm_add_action_or_reset(dev, rzv2h_cpg_genpd_remove_simple, &pd->genpd);
+	if (ret)
+		return ret;
+
+	return of_genpd_add_provider_simple(np, &pd->genpd);
+}
+
+static void rzv2h_cpg_del_clk_provider(void *data)
+{
+	of_clk_del_provider(data);
+}
+
+static int __init rzv2h_cpg_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	const struct rzv2h_cpg_info *info;
+	struct rzv2h_cpg_priv *priv;
+	unsigned int nclks, i;
+	struct clk **clks;
+	int error;
+
+	info = of_device_get_match_data(dev);
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->dev = dev;
+
+	priv->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(priv->base))
+		return PTR_ERR(priv->base);
+
+	nclks = info->num_total_core_clks + info->num_hw_mod_clks;
+	clks = devm_kmalloc_array(dev, nclks, sizeof(*clks), GFP_KERNEL);
+	if (!clks)
+		return -ENOMEM;
+
+	priv->resets = devm_kmemdup(dev, info->resets, sizeof(*info->resets) *
+				    info->num_resets, GFP_KERNEL);
+	if (!priv->resets)
+		return -ENOMEM;
+
+	dev_set_drvdata(dev, priv);
+	priv->clks = clks;
+	priv->num_core_clks = info->num_total_core_clks;
+	priv->num_mod_clks = info->num_hw_mod_clks;
+	priv->last_dt_core_clk = info->last_dt_core_clk;
+	priv->num_resets = info->num_resets;
+
+	for (i = 0; i < nclks; i++)
+		clks[i] = ERR_PTR(-ENOENT);
+
+	for (i = 0; i < info->num_core_clks; i++)
+		rzv2h_cpg_register_core_clk(&info->core_clks[i], priv);
+
+	for (i = 0; i < info->num_mod_clks; i++)
+		rzv2h_cpg_register_mod_clk(&info->mod_clks[i], priv);
+
+	error = of_clk_add_provider(np, rzv2h_cpg_clk_src_twocell_get, priv);
+	if (error)
+		return error;
+
+	error = devm_add_action_or_reset(dev, rzv2h_cpg_del_clk_provider, np);
+	if (error)
+		return error;
+
+	error = rzv2h_cpg_add_pm_domains(priv);
+	if (error)
+		return error;
+
+	error = rzv2h_cpg_reset_controller_register(priv);
+	if (error)
+		return error;
+
+	return 0;
+}
+
+static const struct of_device_id rzv2h_cpg_match[] = {
+	{ /* sentinel */ }
+};
+
+static struct platform_driver rzv2h_cpg_driver = {
+	.driver		= {
+		.name	= "rzv2h-cpg",
+		.of_match_table = rzv2h_cpg_match,
+	},
+};
+
+static int __init rzv2h_cpg_init(void)
+{
+	return platform_driver_probe(&rzv2h_cpg_driver, rzv2h_cpg_probe);
+}
+
+subsys_initcall(rzv2h_cpg_init);
+
+MODULE_DESCRIPTION("Renesas RZ/V2H CPG Driver");
diff --git a/drivers/clk/renesas/rzv2h-cpg.h b/drivers/clk/renesas/rzv2h-cpg.h
new file mode 100644
index 000000000000..33631c101541
--- /dev/null
+++ b/drivers/clk/renesas/rzv2h-cpg.h
@@ -0,0 +1,151 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Renesas RZ/V2H(P) Clock Pulse Generator
+ *
+ * Copyright (C) 2024 Renesas Electronics Corp.
+ */
+
+#ifndef __RENESAS_RZV2H_CPG_H__
+#define __RENESAS_RZV2H_CPG_H__
+
+/**
+ * Definitions of CPG Core Clocks
+ *
+ * These include:
+ *   - Clock outputs exported to DT
+ *   - External input clocks
+ *   - Internal CPG clocks
+ */
+struct cpg_core_clk {
+	const char *name;
+	unsigned int id;
+	unsigned int parent;
+	unsigned int div;
+	unsigned int mult;
+	unsigned int type;
+	unsigned int conf;
+};
+
+enum clk_types {
+	/* Generic */
+	CLK_TYPE_IN,		/* External Clock Input */
+	CLK_TYPE_FF,		/* Fixed Factor Clock */
+	CLK_TYPE_PLL,
+};
+
+/* BIT(31) indicates if CLK1/2 are accessible or not */
+#define PLL_CONF(n)		(BIT(31) | ((n) & ~GENMASK(31, 16)))
+#define PLL_CLK_ACCESS(n)	((n) & BIT(31) ? 1 : 0)
+#define PLL_CLK1_OFFSET(n)	((n) & ~GENMASK(31, 16))
+#define PLL_CLK2_OFFSET(n)	(((n) & ~GENMASK(31, 16)) + (0x4))
+
+#define DEF_TYPE(_name, _id, _type...) \
+	{ .name = _name, .id = _id, .type = _type }
+#define DEF_BASE(_name, _id, _type, _parent...) \
+	DEF_TYPE(_name, _id, _type, .parent = _parent)
+#define DEF_PLL(_name, _id, _parent, _conf) \
+	DEF_TYPE(_name, _id, CLK_TYPE_PLL, .parent = _parent, .conf = _conf)
+#define DEF_INPUT(_name, _id) \
+	DEF_TYPE(_name, _id, CLK_TYPE_IN)
+#define DEF_FIXED(_name, _id, _parent, _mult, _div) \
+	DEF_BASE(_name, _id, CLK_TYPE_FF, _parent, .div = _div, .mult = _mult)
+
+/**
+ * struct rzv2h_mod_clk - Module Clocks definitions
+ *
+ * @name: handle between common and hardware-specific interfaces
+ * @parent: id of parent clock
+ * @critical: flag to indicate the clock is critical
+ * @on_index: control register index
+ * @on_bit: ON bit
+ * @mon_index: monitor register index
+ * @mon_bit: monitor bit
+ */
+struct rzv2h_mod_clk {
+	const char *name;
+	u16 parent;
+	bool critical;
+	u8 on_index;
+	u8 on_bit;
+	s8 mon_index;
+	u8 mon_bit;
+};
+
+#define DEF_MOD_BASE(_name, _parent, _critical, _onindex, _onbit, _monindex, _monbit) \
+	{ \
+		.name = (_name), \
+		.parent = (_parent), \
+		.critical = (_critical), \
+		.on_index = (_onindex), \
+		.on_bit = (_onbit), \
+		.mon_index = (_monindex), \
+		.mon_bit = (_monbit), \
+	}
+
+#define DEF_MOD(_name, _parent, _onindex, _onbit, _monindex, _monbit)		\
+	DEF_MOD_BASE(_name, _parent, false, _onindex, _onbit, _monindex, _monbit)
+
+#define DEF_MOD_CRITICAL(_name, _parent, _onindex, _onbit, _monindex, _monbit)	\
+	DEF_MOD_BASE(_name, _parent, true, _onindex, _onbit, _monindex, _monbit)
+
+/**
+ * struct rzv2h_reset - Reset definitions
+ *
+ * @reset_index: reset register index
+ * @reset_bit: reset bit
+ * @mon_index: monitor register index
+ * @mon_bit: monitor bit
+ */
+struct rzv2h_reset {
+	u8 reset_index;
+	u8 reset_bit;
+	u8 mon_index;
+	u8 mon_bit;
+};
+
+#define RST_ID(x, y)	((((x) * 16)) + (y))
+
+#define DEF_RST_BASE(_id, _resindex, _resbit, _monindex, _monbit)	\
+	[_id] = { \
+		.reset_index = (_resindex), \
+		.reset_bit = (_resbit), \
+		.mon_index = (_monindex), \
+		.mon_bit = (_monbit), \
+	}
+
+#define DEF_RST(_resindex, _resbit, _monindex, _monbit)	\
+	DEF_RST_BASE(RST_ID((_resindex), (_resbit)), _resindex, _resbit, _monindex, _monbit)
+
+/**
+ * struct rzv2h_cpg_info - SoC-specific CPG Description
+ *
+ * @core_clks: Array of Core Clock definitions
+ * @num_core_clks: Number of entries in core_clks[]
+ * @last_dt_core_clk: ID of the last Core Clock exported to DT
+ * @num_total_core_clks: Total number of Core Clocks (exported + internal)
+ *
+ * @mod_clks: Array of Module Clock definitions
+ * @num_mod_clks: Number of entries in mod_clks[]
+ * @num_hw_mod_clks: Number of Module Clocks supported by the hardware
+ *
+ * @resets: Array of Module Reset definitions
+ * @num_resets: Number of entries in resets[]
+ */
+struct rzv2h_cpg_info {
+	/* Core Clocks */
+	const struct cpg_core_clk *core_clks;
+	unsigned int num_core_clks;
+	unsigned int last_dt_core_clk;
+	unsigned int num_total_core_clks;
+
+	/* Module Clocks */
+	const struct rzv2h_mod_clk *mod_clks;
+	unsigned int num_mod_clks;
+	unsigned int num_hw_mod_clks;
+
+	/* Resets */
+	const struct rzv2h_reset *resets;
+	unsigned int num_resets;
+};
+
+#endif	/* __RENESAS_RZV2H_CPG_H__ */
-- 
2.34.1

Re: [PATCH v4 2/3] clk: renesas: Add family-specific clock driver for RZ/V2H(P)

[Geert Uytterhoeven]

Hi Prabhakar,

On Mon, Jul 15, 2024 at 2:56 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Add family-specific clock driver for RZ/V2H(P) SoCs.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
> v3->v4
> - Dropped masking of parent clks with 0xffff
> - Dropped storing mod clk id and now calculating it
>   based on index and bit.
> - Made parent to u16 in struct rzv2h_mod_clk
> - Made a copy of resets array in struct rzv2h_cpg_priv

Thanks for the update!

> --- /dev/null
> +++ b/drivers/clk/renesas/rzv2h-cpg.c

> +/**
> + * struct rzv2h_cpg_priv - Clock Pulse Generator Private Data
> + *
> + * @info: Pointer to platform data

There is no longer an info member.

Hint: W=1 would have told you.

> + * @dev: CPG device
> + * @base: CPG register block base address
> + * @clks: Array containing all Core and Module Clocks
> + * @num_core_clks: Number of Core Clocks in clks[]
> + * @num_mod_clks: Number of Module Clocks in clks[]
> + * @resets: Array of resets
> + * @num_resets: Number of Module Resets in info->resets[]
> + * @last_dt_core_clk: ID of the last Core Clock exported to DT
> + * @rcdev: Reset controller entity
> + */
> +struct rzv2h_cpg_priv {
> +       struct device *dev;
> +       void __iomem *base;
> +
> +       struct clk **clks;
> +       unsigned int num_core_clks;
> +       unsigned int num_mod_clks;
> +       struct rzv2h_reset *resets;
> +       unsigned int num_resets;
> +       unsigned int last_dt_core_clk;
> +
> +       struct reset_controller_dev rcdev;
> +};

> index 000000000000..33631c101541
> --- /dev/null
> +++ b/drivers/clk/renesas/rzv2h-cpg.h

> +#define DEF_RST_BASE(_id, _resindex, _resbit, _monindex, _monbit)      \
> +       [_id] = { \

Indexing by _id means the reset array will be very sparse.  E.g. the
innocent-looking r9a09g057_resets[] with only a single entry takes
600 bytes:

    $ nm -S drivers/clk/renesas/r9a09g057-cpg.o | grep r9a09g057_resets
    0000000000000038 0000000000000258 r r9a09g057_resets

So please pack the array here, and either unpack it while making the
priv->resets copy, or implement translation ("look-up") from ID to
packed index in rzv2h_cpg_reset_xlate().

> +               .reset_index = (_resindex), \
> +               .reset_bit = (_resbit), \
> +               .mon_index = (_monindex), \
> +               .mon_bit = (_monbit), \
> +       }

The rest LGTM.

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

Re: [PATCH v4 2/3] clk: renesas: Add family-specific clock driver for RZ/V2H(P)

[Lad, Prabhakar]

Hi Geert,

Thank you for the review.

On Fri, Jul 26, 2024 at 3:53 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> On Mon, Jul 15, 2024 at 2:56 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > Add family-specific clock driver for RZ/V2H(P) SoCs.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > ---
> > v3->v4
> > - Dropped masking of parent clks with 0xffff
> > - Dropped storing mod clk id and now calculating it
> >   based on index and bit.
> > - Made parent to u16 in struct rzv2h_mod_clk
> > - Made a copy of resets array in struct rzv2h_cpg_priv
>
> Thanks for the update!
>
> > --- /dev/null
> > +++ b/drivers/clk/renesas/rzv2h-cpg.c
>
> > +/**
> > + * struct rzv2h_cpg_priv - Clock Pulse Generator Private Data
> > + *
> > + * @info: Pointer to platform data
>
> There is no longer an info member.
>
Agreed, I will drop it.

> Hint: W=1 would have told you.
>
Thanks for the pointer.

> > + * @dev: CPG device
> > + * @base: CPG register block base address
> > + * @clks: Array containing all Core and Module Clocks
> > + * @num_core_clks: Number of Core Clocks in clks[]
> > + * @num_mod_clks: Number of Module Clocks in clks[]
> > + * @resets: Array of resets
> > + * @num_resets: Number of Module Resets in info->resets[]
> > + * @last_dt_core_clk: ID of the last Core Clock exported to DT
> > + * @rcdev: Reset controller entity
> > + */
> > +struct rzv2h_cpg_priv {
> > +       struct device *dev;
> > +       void __iomem *base;
> > +
> > +       struct clk **clks;
> > +       unsigned int num_core_clks;
> > +       unsigned int num_mod_clks;
> > +       struct rzv2h_reset *resets;
> > +       unsigned int num_resets;
> > +       unsigned int last_dt_core_clk;
> > +
> > +       struct reset_controller_dev rcdev;
> > +};
>
> > index 000000000000..33631c101541
> > --- /dev/null
> > +++ b/drivers/clk/renesas/rzv2h-cpg.h
>
> > +#define DEF_RST_BASE(_id, _resindex, _resbit, _monindex, _monbit)      \
> > +       [_id] = { \
>
> Indexing by _id means the reset array will be very sparse.  E.g. the
> innocent-looking r9a09g057_resets[] with only a single entry takes
> 600 bytes:
>
>     $ nm -S drivers/clk/renesas/r9a09g057-cpg.o | grep r9a09g057_resets
>     0000000000000038 0000000000000258 r r9a09g057_resets
>
Agreed.

> So please pack the array here, and either unpack it while making the
> priv->resets copy, or implement translation ("look-up") from ID to
> packed index in rzv2h_cpg_reset_xlate().
>
OK, I will implement the below:

#define PACK_RESET(_resindex, _resbit, _monindex, _monbit)    \
    (((_resindex) << 24) | ((_resbit) << 16) | ((_monindex) << 8) | (_monbit))

#define DEF_RST(_resindex, _resbit, _monindex, _monbit)    \
    PACK_RESET(_resindex, _resbit, _monindex, _monbit)

#define GET_RESET_INDEX(x)    (((x) >> 24) & 0xFF)
#define GET_RESET_BIT(x)    (((x) >> 16) & 0xFF)
#define GET_MON_INDEX(x)    (((x) >> 8) & 0xFF)
#define GET_MON_BIT(x)        ((x) & 0xFF)

static int rzv2h_cpg_reset_xlate(struct reset_controller_dev *rcdev,
                 const struct of_phandle_args *reset_spec)
{
    struct rzv2h_cpg_priv *priv = rcdev_to_priv(rcdev);
    unsigned int id = reset_spec->args[0];
    u8 rst_index = id / 16;
    u8 rst_bit = id % 16;
    unsigned int i;

    for (i = 0; i < rcdev->nr_resets; i++) {
        u8 cur_index = GET_RESET_INDEX(priv->resets[i]);
        u8 cur_bit = GET_RESET_BIT(priv->resets[i]);

        if (rst_index == cur_index && rst_bit == cur_bit)
            return i;
    }

    return -EINVAL;
}

Let me know if this is OK, or to avoid looping in xlate maybe we can
have a packed entry in the resets property of DT by this way we can
avoid having the resets array all together?

Cheers,
Prabhakar

Re: [PATCH v4 2/3] clk: renesas: Add family-specific clock driver for RZ/V2H(P)

[Geert Uytterhoeven]

Hi Prabhakar,

On Sat, Jul 27, 2024 at 12:51 PM Lad, Prabhakar
<prabhakar.csengg@gmail.com> wrote:
> On Fri, Jul 26, 2024 at 3:53 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Mon, Jul 15, 2024 at 2:56 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
> > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > >
> > > Add family-specific clock driver for RZ/V2H(P) SoCs.
> > >
> > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > --- /dev/null
> > > +++ b/drivers/clk/renesas/rzv2h-cpg.h
> >
> > > +#define DEF_RST_BASE(_id, _resindex, _resbit, _monindex, _monbit)      \
> > > +       [_id] = { \
> >
> > Indexing by _id means the reset array will be very sparse.  E.g. the
> > innocent-looking r9a09g057_resets[] with only a single entry takes
> > 600 bytes:
> >
> >     $ nm -S drivers/clk/renesas/r9a09g057-cpg.o | grep r9a09g057_resets
> >     0000000000000038 0000000000000258 r r9a09g057_resets
> >
> Agreed.
>
> > So please pack the array here, and either unpack it while making the
> > priv->resets copy, or implement translation ("look-up") from ID to
> > packed index in rzv2h_cpg_reset_xlate().
> >
> OK, I will implement the below:
>
> #define PACK_RESET(_resindex, _resbit, _monindex, _monbit)    \
>     (((_resindex) << 24) | ((_resbit) << 16) | ((_monindex) << 8) | (_monbit))
>
> #define DEF_RST(_resindex, _resbit, _monindex, _monbit)    \
>     PACK_RESET(_resindex, _resbit, _monindex, _monbit)
>
> #define GET_RESET_INDEX(x)    (((x) >> 24) & 0xFF)
> #define GET_RESET_BIT(x)    (((x) >> 16) & 0xFF)
> #define GET_MON_INDEX(x)    (((x) >> 8) & 0xFF)
> #define GET_MON_BIT(x)        ((x) & 0xFF)
>
> static int rzv2h_cpg_reset_xlate(struct reset_controller_dev *rcdev,
>                  const struct of_phandle_args *reset_spec)
> {
>     struct rzv2h_cpg_priv *priv = rcdev_to_priv(rcdev);
>     unsigned int id = reset_spec->args[0];
>     u8 rst_index = id / 16;
>     u8 rst_bit = id % 16;
>     unsigned int i;
>
>     for (i = 0; i < rcdev->nr_resets; i++) {
>         u8 cur_index = GET_RESET_INDEX(priv->resets[i]);
>         u8 cur_bit = GET_RESET_BIT(priv->resets[i]);
>
>         if (rst_index == cur_index && rst_bit == cur_bit)
>             return i;
>     }
>
>     return -EINVAL;
> }
>
> Let me know if this is OK, or to avoid looping in xlate maybe we can
> have a packed entry in the resets property of DT by this way we can
> avoid having the resets array all together?

Sorry for being unclear. I did not mean packing the fields in the struct
into a single word, but packing the entries in the r9a09g057_resets[]
array.  Using the rzv2h_reset structure is fine.

With:

    #define DEF_RST_BASE(_id, _resindex, _resbit, _monindex, _monbit)       \
            [_id] = { \
                    .reset_index = (_resindex), \
                    .reset_bit = (_resbit), \
                    .mon_index = (_monindex), \
                    .mon_bit = (_monbit), \
            }

    #define DEF_RST(_resindex, _resbit, _monindex, _monbit) \
            DEF_RST_BASE(RST_ID((_resindex), (_resbit)), _resindex,
_resbit, _monindex, _monbit)

    static const struct rzv2h_reset r9a09g057_resets[] __initconst = {
        DEF_RST(9, 5, 4, 6),            /* SCIF_0_RST_SYSTEM_N */
    };

is expanded into an array of 150 entries (9 * 16 + 5 = 149 empty entries
followed by the SCIF_0_RST_SYSTEM_N entry), which is wasteful.
Over time the array will be filled more, but I expect there will still
be lots of unused entries.

Hence I suggest to drop the "[id]":

   - define DEF_RST_BASE(_id, _resindex, _resbit, _monindex, _monbit)       \
   -       [_id] = { \
   +#define DEF_RST(_resindex, _resbit, _monindex, _monbit)       \
   +       { \
                   .reset_index = (_resindex), \
                    .reset_bit = (_resbit), \
                    .mon_index = (_monindex), \
                    .mon_bit = (_monbit), \
            }
   -
   -#define DEF_RST(_resindex, _resbit, _monindex, _monbit) \
   -        DEF_RST_BASE(RST_ID((_resindex), (_resbit)), _resindex,
_resbit, _monindex, _monbit)

Then r9a09g057_resets[] will contain only non-empty entries, at the
expense of no longer being able to index it directly by reset ID.
To solve the indexing, there are two options.

Option A: Translate from reset ID to real index during lookup, like
          you do in the rzv2h_cpg_reset_xlate() above:

    static int rzv2h_cpg_reset_xlate(struct reset_controller_dev *rcdev,
                     const struct of_phandle_args *reset_spec)
    {
        struct rzv2h_cpg_priv *priv = rcdev_to_priv(rcdev);
        unsigned int id = reset_spec->args[0];
        u8 rst_index = id / 16;
        u8 rst_bit = id % 16;
        unsigned int i;

        for (i = 0; i < rcdev->nr_resets; i++) {
            if (rst_index == priv->resets[i].reset_index &&
                rst_bit == ->resets[i].reset_bit)
                return i;
        }

        return -EINVAL;
    }

Option B: "Unpack" rzv2h_cpg_info.resets[] during copying in
          rzv2h_cpg_probe():

    priv->resets = devm_kcalloc(dev, max_num_reset_ids,
                                 sizeof(*priv->resets), GFP_KERNEL);
    for (i = 0; i < ARRAY_SIZE(info->resets); i++) {
            id = RST_ID(info->resets[i].reset_index, info->resets[i].reset_bit);
            priv->resets[id] = info->resets[i];
    }

BTW, for option B (and for the current code in v4),
rzv2h_cpg_reset_xlate() should validate that the entry is non-empty.

I hope this is more clear?

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

Re: [PATCH v4 2/3] clk: renesas: Add family-specific clock driver for RZ/V2H(P)

[Lad, Prabhakar]

Hi Geert,

On Mon, Jul 29, 2024 at 9:14 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> On Sat, Jul 27, 2024 at 12:51 PM Lad, Prabhakar
> <prabhakar.csengg@gmail.com> wrote:
> > On Fri, Jul 26, 2024 at 3:53 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > On Mon, Jul 15, 2024 at 2:56 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
> > > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > >
> > > > Add family-specific clock driver for RZ/V2H(P) SoCs.
> > > >
> > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > > --- /dev/null
> > > > +++ b/drivers/clk/renesas/rzv2h-cpg.h
> > >
> > > > +#define DEF_RST_BASE(_id, _resindex, _resbit, _monindex, _monbit)      \
> > > > +       [_id] = { \
> > >
> > > Indexing by _id means the reset array will be very sparse.  E.g. the
> > > innocent-looking r9a09g057_resets[] with only a single entry takes
> > > 600 bytes:
> > >
> > >     $ nm -S drivers/clk/renesas/r9a09g057-cpg.o | grep r9a09g057_resets
> > >     0000000000000038 0000000000000258 r r9a09g057_resets
> > >
> > Agreed.
> >
> > > So please pack the array here, and either unpack it while making the
> > > priv->resets copy, or implement translation ("look-up") from ID to
> > > packed index in rzv2h_cpg_reset_xlate().
> > >
> > OK, I will implement the below:
> >
> > #define PACK_RESET(_resindex, _resbit, _monindex, _monbit)    \
> >     (((_resindex) << 24) | ((_resbit) << 16) | ((_monindex) << 8) | (_monbit))
> >
> > #define DEF_RST(_resindex, _resbit, _monindex, _monbit)    \
> >     PACK_RESET(_resindex, _resbit, _monindex, _monbit)
> >
> > #define GET_RESET_INDEX(x)    (((x) >> 24) & 0xFF)
> > #define GET_RESET_BIT(x)    (((x) >> 16) & 0xFF)
> > #define GET_MON_INDEX(x)    (((x) >> 8) & 0xFF)
> > #define GET_MON_BIT(x)        ((x) & 0xFF)
> >
> > static int rzv2h_cpg_reset_xlate(struct reset_controller_dev *rcdev,
> >                  const struct of_phandle_args *reset_spec)
> > {
> >     struct rzv2h_cpg_priv *priv = rcdev_to_priv(rcdev);
> >     unsigned int id = reset_spec->args[0];
> >     u8 rst_index = id / 16;
> >     u8 rst_bit = id % 16;
> >     unsigned int i;
> >
> >     for (i = 0; i < rcdev->nr_resets; i++) {
> >         u8 cur_index = GET_RESET_INDEX(priv->resets[i]);
> >         u8 cur_bit = GET_RESET_BIT(priv->resets[i]);
> >
> >         if (rst_index == cur_index && rst_bit == cur_bit)
> >             return i;
> >     }
> >
> >     return -EINVAL;
> > }
> >
> > Let me know if this is OK, or to avoid looping in xlate maybe we can
> > have a packed entry in the resets property of DT by this way we can
> > avoid having the resets array all together?
>
> Sorry for being unclear. I did not mean packing the fields in the struct
> into a single word, but packing the entries in the r9a09g057_resets[]
> array.  Using the rzv2h_reset structure is fine.
>
> With:
>
>     #define DEF_RST_BASE(_id, _resindex, _resbit, _monindex, _monbit)       \
>             [_id] = { \
>                     .reset_index = (_resindex), \
>                     .reset_bit = (_resbit), \
>                     .mon_index = (_monindex), \
>                     .mon_bit = (_monbit), \
>             }
>
>     #define DEF_RST(_resindex, _resbit, _monindex, _monbit) \
>             DEF_RST_BASE(RST_ID((_resindex), (_resbit)), _resindex,
> _resbit, _monindex, _monbit)
>
>     static const struct rzv2h_reset r9a09g057_resets[] __initconst = {
>         DEF_RST(9, 5, 4, 6),            /* SCIF_0_RST_SYSTEM_N */
>     };
>
> is expanded into an array of 150 entries (9 * 16 + 5 = 149 empty entries
> followed by the SCIF_0_RST_SYSTEM_N entry), which is wasteful.
> Over time the array will be filled more, but I expect there will still
> be lots of unused entries.
>
> Hence I suggest to drop the "[id]":
>
>    - define DEF_RST_BASE(_id, _resindex, _resbit, _monindex, _monbit)       \
>    -       [_id] = { \
>    +#define DEF_RST(_resindex, _resbit, _monindex, _monbit)       \
>    +       { \
>                    .reset_index = (_resindex), \
>                     .reset_bit = (_resbit), \
>                     .mon_index = (_monindex), \
>                     .mon_bit = (_monbit), \
>             }
>    -
>    -#define DEF_RST(_resindex, _resbit, _monindex, _monbit) \
>    -        DEF_RST_BASE(RST_ID((_resindex), (_resbit)), _resindex,
> _resbit, _monindex, _monbit)
>
> Then r9a09g057_resets[] will contain only non-empty entries, at the
> expense of no longer being able to index it directly by reset ID.
> To solve the indexing, there are two options.
>
> Option A: Translate from reset ID to real index during lookup, like
>           you do in the rzv2h_cpg_reset_xlate() above:
>
>     static int rzv2h_cpg_reset_xlate(struct reset_controller_dev *rcdev,
>                      const struct of_phandle_args *reset_spec)
>     {
>         struct rzv2h_cpg_priv *priv = rcdev_to_priv(rcdev);
>         unsigned int id = reset_spec->args[0];
>         u8 rst_index = id / 16;
>         u8 rst_bit = id % 16;
>         unsigned int i;
>
>         for (i = 0; i < rcdev->nr_resets; i++) {
>             if (rst_index == priv->resets[i].reset_index &&
>                 rst_bit == ->resets[i].reset_bit)
>                 return i;
>         }
>
>         return -EINVAL;
>     }
>
> Option B: "Unpack" rzv2h_cpg_info.resets[] during copying in
>           rzv2h_cpg_probe():
>
>     priv->resets = devm_kcalloc(dev, max_num_reset_ids,
>                                  sizeof(*priv->resets), GFP_KERNEL);
>     for (i = 0; i < ARRAY_SIZE(info->resets); i++) {
>             id = RST_ID(info->resets[i].reset_index, info->resets[i].reset_bit);
>             priv->resets[id] = info->resets[i];
>     }
>
> BTW, for option B (and for the current code in v4),
> rzv2h_cpg_reset_xlate() should validate that the entry is non-empty.
>
> I hope this is more clear?
>
Yes, thanks for the clarification. I will go with option A, so we
don't waste memory.

Cheers,
Prabhakar

[PATCH v4 3/3] clk: renesas: Add RZ/V2H(P) CPG driver

[Prabhakar]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Add RZ/V2H(P) CPG driver.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
v3->v4
- Added a comment for core clocks
- Made r9a09g057_mod_clks as __initconst
- Made r9a09g057_resets as __initconst
- Made r9a09g057_cpg_info as __initconst

v2->v3
- Added CLK_PLLDTY
- Added core clocks sys_0_pclk and iotop_0_shclk
- Dropped r9a09g057_crit_mod_clks

v1->v2
- Updated commit description
- Dropped pll_clk1/clk2_offset
- Made r9a09g057_mod_clks/r9a09g057_resets as static const
- Now using register indexes
---
 drivers/clk/renesas/Kconfig         |  5 ++
 drivers/clk/renesas/Makefile        |  1 +
 drivers/clk/renesas/r9a09g057-cpg.c | 80 +++++++++++++++++++++++++++++
 drivers/clk/renesas/rzv2h-cpg.c     |  4 ++
 drivers/clk/renesas/rzv2h-cpg.h     |  2 +
 5 files changed, 92 insertions(+)
 create mode 100644 drivers/clk/renesas/r9a09g057-cpg.c

diff --git a/drivers/clk/renesas/Kconfig b/drivers/clk/renesas/Kconfig
index f078ccb635bb..76791a1c50ac 100644
--- a/drivers/clk/renesas/Kconfig
+++ b/drivers/clk/renesas/Kconfig
@@ -40,6 +40,7 @@ config CLK_RENESAS
 	select CLK_R9A07G054 if ARCH_R9A07G054
 	select CLK_R9A08G045 if ARCH_R9A08G045
 	select CLK_R9A09G011 if ARCH_R9A09G011
+	select CLK_R9A09G057 if ARCH_R9A09G057
 	select CLK_SH73A0 if ARCH_SH73A0
 
 if CLK_RENESAS
@@ -193,6 +194,10 @@ config CLK_R9A09G011
 	bool "RZ/V2M clock support" if COMPILE_TEST
 	select CLK_RZG2L
 
+config CLK_R9A09G057
+       bool "RZ/V2H(P) clock support" if COMPILE_TEST
+       select CLK_RZV2H
+
 config CLK_SH73A0
 	bool "SH-Mobile AG5 clock support" if COMPILE_TEST
 	select CLK_RENESAS_CPG_MSTP
diff --git a/drivers/clk/renesas/Makefile b/drivers/clk/renesas/Makefile
index d81a62e78345..23d2e26051c8 100644
--- a/drivers/clk/renesas/Makefile
+++ b/drivers/clk/renesas/Makefile
@@ -37,6 +37,7 @@ obj-$(CONFIG_CLK_R9A07G044)		+= r9a07g044-cpg.o
 obj-$(CONFIG_CLK_R9A07G054)		+= r9a07g044-cpg.o
 obj-$(CONFIG_CLK_R9A08G045)		+= r9a08g045-cpg.o
 obj-$(CONFIG_CLK_R9A09G011)		+= r9a09g011-cpg.o
+obj-$(CONFIG_CLK_R9A09G057)		+= r9a09g057-cpg.o
 obj-$(CONFIG_CLK_SH73A0)		+= clk-sh73a0.o
 
 # Family
diff --git a/drivers/clk/renesas/r9a09g057-cpg.c b/drivers/clk/renesas/r9a09g057-cpg.c
new file mode 100644
index 000000000000..9722b810e027
--- /dev/null
+++ b/drivers/clk/renesas/r9a09g057-cpg.c
@@ -0,0 +1,80 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Renesas RZ/V2H(P) CPG driver
+ *
+ * Copyright (C) 2024 Renesas Electronics Corp.
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/device.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+
+#include <dt-bindings/clock/renesas,r9a09g057-cpg.h>
+
+#include "rzv2h-cpg.h"
+
+enum clk_ids {
+	/* Core Clock Outputs exported to DT */
+	LAST_DT_CORE_CLK = R9A09G057_IOTOP_0_SHCLK,
+
+	/* External Input Clocks */
+	CLK_AUDIO_EXTAL,
+	CLK_RTXIN,
+	CLK_QEXTAL,
+
+	/* PLL Clocks */
+	CLK_PLLCM33,
+	CLK_PLLDTY,
+	CLK_PLLCA55,
+
+	/* Internal Core Clocks */
+	CLK_PLLCM33_DIV16,
+
+	/* Module Clocks */
+	MOD_CLK_BASE,
+};
+
+static const struct cpg_core_clk r9a09g057_core_clks[] __initconst = {
+	/* External Clock Inputs */
+	DEF_INPUT("audio_extal", CLK_AUDIO_EXTAL),
+	DEF_INPUT("rtxin", CLK_RTXIN),
+	DEF_INPUT("qextal", CLK_QEXTAL),
+
+	/* PLL Clocks */
+	DEF_FIXED(".pllcm33", CLK_PLLCM33, CLK_QEXTAL, 200, 3),
+	DEF_FIXED(".plldty", CLK_PLLDTY, CLK_QEXTAL, 200, 3),
+	DEF_PLL(".pllca55", CLK_PLLCA55, CLK_QEXTAL, PLL_CONF(0x64)),
+
+	/* Internal Core Clocks */
+	DEF_FIXED(".pllcm33_div16", CLK_PLLCM33_DIV16, CLK_PLLCM33, 1, 16),
+
+	/* Core Clocks */
+	DEF_FIXED("sys_0_pclk", R9A09G057_SYS_0_PCLK, CLK_QEXTAL, 1, 1),
+	DEF_FIXED("iotop_0_shclk", R9A09G057_IOTOP_0_SHCLK, CLK_PLLCM33_DIV16, 1, 1),
+};
+
+static const struct rzv2h_mod_clk r9a09g057_mod_clks[] __initconst = {
+	DEF_MOD("scif_0_clk_pck",		CLK_PLLCM33_DIV16, 8, 15, 4, 15),
+};
+
+static const struct rzv2h_reset r9a09g057_resets[] __initconst = {
+	DEF_RST(9, 5, 4, 6),		/* SCIF_0_RST_SYSTEM_N */
+};
+
+const struct rzv2h_cpg_info r9a09g057_cpg_info __initconst = {
+	/* Core Clocks */
+	.core_clks = r9a09g057_core_clks,
+	.num_core_clks = ARRAY_SIZE(r9a09g057_core_clks),
+	.last_dt_core_clk = LAST_DT_CORE_CLK,
+	.num_total_core_clks = MOD_CLK_BASE,
+
+	/* Module Clocks */
+	.mod_clks = r9a09g057_mod_clks,
+	.num_mod_clks = ARRAY_SIZE(r9a09g057_mod_clks),
+	.num_hw_mod_clks = 25 * 16,
+
+	/* Resets */
+	.resets = r9a09g057_resets,
+	.num_resets = ARRAY_SIZE(r9a09g057_resets),
+};
diff --git a/drivers/clk/renesas/rzv2h-cpg.c b/drivers/clk/renesas/rzv2h-cpg.c
index fc3ff7a65aac..286a6ece8f80 100644
--- a/drivers/clk/renesas/rzv2h-cpg.c
+++ b/drivers/clk/renesas/rzv2h-cpg.c
@@ -661,6 +661,10 @@ static int __init rzv2h_cpg_probe(struct platform_device *pdev)
 }
 
 static const struct of_device_id rzv2h_cpg_match[] = {
+	{
+		.compatible = "renesas,r9a09g057-cpg",
+		.data = &r9a09g057_cpg_info,
+	},
 	{ /* sentinel */ }
 };
 
diff --git a/drivers/clk/renesas/rzv2h-cpg.h b/drivers/clk/renesas/rzv2h-cpg.h
index 33631c101541..d3ad98123b29 100644
--- a/drivers/clk/renesas/rzv2h-cpg.h
+++ b/drivers/clk/renesas/rzv2h-cpg.h
@@ -148,4 +148,6 @@ struct rzv2h_cpg_info {
 	unsigned int num_resets;
 };
 
+extern const struct rzv2h_cpg_info r9a09g057_cpg_info;
+
 #endif	/* __RENESAS_RZV2H_CPG_H__ */
-- 
2.34.1

Re: [PATCH v4 3/3] clk: renesas: Add RZ/V2H(P) CPG driver

[Geert Uytterhoeven]

On Mon, Jul 15, 2024 at 2:56 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Add RZ/V2H(P) CPG driver.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
> v3->v4
> - Added a comment for core clocks
> - Made r9a09g057_mod_clks as __initconst
> - Made r9a09g057_resets as __initconst
> - Made r9a09g057_cpg_info as __initconst

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

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