[PATCH 0/7] Update clock definitions

[PATCH 0/7] Update clock definitions

[Biju Das]

This patch series update clock definitions as per the RZG2L_clock_list(Rev.02) manual.

As per this we need to treat each bit as a seperate clock. So
added support for multi clock PM support. Also updated clock
driver enties.

Biju Das (7):
  dt-bindings: clk: r9a07g044-cpg: Update clock definitions
  drivers: clk: renesas: renesas-rzg2l-cpg: Add multi clock PM support
  drivers: clk: renesas: r9a07g044-cpg: Update {GIC,IA55,SCIF} clock
    entries
  arm64: dts: renesas: r9a07g044: Update SCIF0 clock
  drivers: clk: renesas: r9a07g044-cpg: Add I2C Clocks
  drivers: clk: renesas: r9a07g044-cpg: Add DMAC clocks
  arm64: dts: renesas: r9a07g044: Add I2C nodes

 arch/arm64/boot/dts/renesas/r9a07g044.dtsi |  84 ++++++++++-
 drivers/clk/renesas/r9a07g044-cpg.c        |  57 ++++++--
 drivers/clk/renesas/renesas-rzg2l-cpg.c    |  51 ++++---
 drivers/clk/renesas/renesas-rzg2l-cpg.h    |   1 +
 include/dt-bindings/clock/r9a07g044-cpg.h  | 153 ++++++++++++++-------
 5 files changed, 258 insertions(+), 88 deletions(-)

-- 
2.17.1

[PATCH 1/7] dt-bindings: clk: r9a07g044-cpg: Update clock definitions

[Biju Das]

Update clock definitions as per the RZG2L_clock_list(Rev.02) manual.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 include/dt-bindings/clock/r9a07g044-cpg.h | 153 ++++++++++++++--------
 1 file changed, 100 insertions(+), 53 deletions(-)

diff --git a/include/dt-bindings/clock/r9a07g044-cpg.h b/include/dt-bindings/clock/r9a07g044-cpg.h
index 1d8986563fc5..7e0127646973 100644
--- a/include/dt-bindings/clock/r9a07g044-cpg.h
+++ b/include/dt-bindings/clock/r9a07g044-cpg.h
@@ -32,58 +32,105 @@
 #define R9A07G044_OSCCLK		21
 
 /* R9A07G044 Module Clocks */
-#define R9A07G044_CLK_GIC600		0
-#define R9A07G044_CLK_IA55		1
-#define R9A07G044_CLK_SYC		2
-#define R9A07G044_CLK_DMAC		3
-#define R9A07G044_CLK_SYSC		4
-#define R9A07G044_CLK_MTU		5
-#define R9A07G044_CLK_GPT		6
-#define R9A07G044_CLK_ETH0		7
-#define R9A07G044_CLK_ETH1		8
-#define R9A07G044_CLK_I2C0		9
-#define R9A07G044_CLK_I2C1		10
-#define R9A07G044_CLK_I2C2		11
-#define R9A07G044_CLK_I2C3		12
-#define R9A07G044_CLK_SCIF0		13
-#define R9A07G044_CLK_SCIF1		14
-#define R9A07G044_CLK_SCIF2		15
-#define R9A07G044_CLK_SCIF3		16
-#define R9A07G044_CLK_SCIF4		17
-#define R9A07G044_CLK_SCI0		18
-#define R9A07G044_CLK_SCI1		19
-#define R9A07G044_CLK_GPIO		20
-#define R9A07G044_CLK_SDHI0		21
-#define R9A07G044_CLK_SDHI1		22
-#define R9A07G044_CLK_USB0		23
-#define R9A07G044_CLK_USB1		24
-#define R9A07G044_CLK_CANFD		25
-#define R9A07G044_CLK_SSI0		26
-#define R9A07G044_CLK_SSI1		27
-#define R9A07G044_CLK_SSI2		28
-#define R9A07G044_CLK_SSI3		29
-#define R9A07G044_CLK_MHU		30
-#define R9A07G044_CLK_OSTM0		31
-#define R9A07G044_CLK_OSTM1		32
-#define R9A07G044_CLK_OSTM2		33
-#define R9A07G044_CLK_WDT0		34
-#define R9A07G044_CLK_WDT1		35
-#define R9A07G044_CLK_WDT2		36
-#define R9A07G044_CLK_WDT_PON		37
-#define R9A07G044_CLK_GPU		38
-#define R9A07G044_CLK_ISU		39
-#define R9A07G044_CLK_H264		40
-#define R9A07G044_CLK_CRU		41
-#define R9A07G044_CLK_MIPI_DSI		42
-#define R9A07G044_CLK_LCDC		43
-#define R9A07G044_CLK_SRC		44
-#define R9A07G044_CLK_RSPI0		45
-#define R9A07G044_CLK_RSPI1		46
-#define R9A07G044_CLK_RSPI2		47
-#define R9A07G044_CLK_ADC		48
-#define R9A07G044_CLK_TSU_PCLK		49
-#define R9A07G044_CLK_SPI		50
-#define R9A07G044_CLK_MIPI_DSI_V	51
-#define R9A07G044_CLK_MIPI_DSI_PIN	52
+#define R9A07G044_CA55_SCLK		0
+#define R9A07G044_CA55_PCLK		1
+#define R9A07G044_CA55_ATCLK		2
+#define R9A07G044_CA55_GICCLK		3
+#define R9A07G044_CA55_PERICLK		4
+#define R9A07G044_CA55_ACLK		5
+#define R9A07G044_CA55_TSCLK		6
+#define R9A07G044_GIC600_GICCLK		7
+#define R9A07G044_IA55_CLK		8
+#define R9A07G044_IA55_PCLK		9
+#define R9A07G044_MHU_PCLK		10
+#define R9A07G044_SYC_CNT_CLK		11
+#define R9A07G044_DMAC_ACLK		12
+#define R9A07G044_DMAC_PCLK		13
+#define R9A07G044_OSTM0_PCLK		14
+#define R9A07G044_OSTM1_PCLK		15
+#define R9A07G044_OSTM2_PCLK		16
+#define R9A07G044_MTU_X_MCK_MTU3	17
+#define R9A07G044_POE3_CLKM_POE		18
+#define R9A07G044_GPT_PCLK		19
+#define R9A07G044_POEG_A_CLKP		20
+#define R9A07G044_POEG_B_CLKP		21
+#define R9A07G044_POEG_C_CLKP		22
+#define R9A07G044_POEG_D_CLKP		23
+#define R9A07G044_WDT0_PCLK		24
+#define R9A07G044_WDT0_CLK		25
+#define R9A07G044_WDT1_PCLK		26
+#define R9A07G044_WDT1_CLK		27
+#define R9A07G044_WDT2_PCLK		28
+#define R9A07G044_WDT2_CLK		29
+#define R9A07G044_SPI_CLK2		30
+#define R9A07G044_SPI_CLK		31
+#define R9A07G044_SDHI0_IMCLK		32
+#define R9A07G044_SDHI0_IMCLK2		33
+#define R9A07G044_SDHI0_CLK_HS		34
+#define R9A07G044_SDHI0_ACLK		35
+#define R9A07G044_SDHI1_IMCLK		36
+#define R9A07G044_SDHI1_IMCLK2		37
+#define R9A07G044_SDHI1_CLK_HS		38
+#define R9A07G044_SDHI1_ACLK		39
+#define R9A07G044_GPU_CLK		40
+#define R9A07G044_GPU_AXI_CLK		41
+#define R9A07G044_GPU_ACE_CLK		42
+#define R9A07G044_ISU_ACLK		43
+#define R9A07G044_ISU_PCLK		44
+#define R9A07G044_H264_CLK_A		45
+#define R9A07G044_H264_CLK_P		46
+#define R9A07G044_CRU_SYSCLK		47
+#define R9A07G044_CRU_VCLK		48
+#define R9A07G044_CRU_PCLK		49
+#define R9A07G044_CRU_ACLK		50
+#define R9A07G044_MIPI_DSI_PLLCLK	51
+#define R9A07G044_MIPI_DSI_SYSCLK	52
+#define R9A07G044_MIPI_DSI_ACLK		53
+#define R9A07G044_MIPI_DSI_PCLK		54
+#define R9A07G044_MIPI_DSI_VCLK		55
+#define R9A07G044_MIPI_DSI_LPCLK	56
+#define R9A07G044_LCDC_CLK_A		57
+#define R9A07G044_LCDC_CLK_P		58
+#define R9A07G044_LCDC_CLK_D		59
+#define R9A07G044_SSI0_PCLK2		60
+#define R9A07G044_SSI0_PCLK_SFR		61
+#define R9A07G044_SSI1_PCLK2		62
+#define R9A07G044_SSI1_PCLK_SFR		63
+#define R9A07G044_SSI2_PCLK2		64
+#define R9A07G044_SSI2_PCLK_SFR		65
+#define R9A07G044_SSI3_PCLK2		66
+#define R9A07G044_SSI3_PCLK_SFR		67
+#define R9A07G044_SRC_CLKP		68
+#define R9A07G044_USB_U2H0_HCLK		69
+#define R9A07G044_USB_U2H1_HCLK		70
+#define R9A07G044_USB_U2P_EXR_CPUCLK	71
+#define R9A07G044_USB_PCLK		72
+#define R9A07G044_USB_SCLK		73
+#define R9A07G044_ETH0_CLK_AXI		74
+#define R9A07G044_ETH0_CLK_CHI		75
+#define R9A07G044_ETH0_REFCLK		76
+#define R9A07G044_ETH1_CLK_AXI		77
+#define R9A07G044_ETH1_CLK_CHI		78
+#define R9A07G044_ETH1_REFCLK		79
+#define R9A07G044_I2C0_PCLK		80
+#define R9A07G044_I2C1_PCLK		81
+#define R9A07G044_I2C2_PCLK		82
+#define R9A07G044_I2C3_PCLK		83
+#define R9A07G044_SCIF0_CLK_PCK		84
+#define R9A07G044_SCIF1_CLK_PCK		85
+#define R9A07G044_SCIF2_CLK_PCK		86
+#define R9A07G044_SCIF3_CLK_PCK		87
+#define R9A07G044_SCIF4_CLK_PCK		88
+#define R9A07G044_SCI0_CLKP		89
+#define R9A07G044_SCI1_CLKP		90
+#define R9A07G044_IRDA_CLKP		91
+#define R9A07G044_RSPI0_CLKB		92
+#define R9A07G044_RSPI1_CLKB		93
+#define R9A07G044_RSPI2_CLKB		94
+#define R9A07G044_CANFD_PCLK		95
+#define R9A07G044_GPIO_HCLK		96
+#define R9A07G044_ADC_ADCLK		97
+#define R9A07G044_ADC_PCLK		98
+#define R9A07G044_TSU_PCLK		99
 
 #endif /* __DT_BINDINGS_CLOCK_R9A07G044_CPG_H__ */
-- 
2.17.1

[PATCH 2/7] drivers: clk: renesas: renesas-rzg2l-cpg: Add multi clock PM support

[Biju Das]

Add multi clock PM support for cpg driver.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 drivers/clk/renesas/renesas-rzg2l-cpg.c | 51 ++++++++++++++-----------
 1 file changed, 29 insertions(+), 22 deletions(-)

diff --git a/drivers/clk/renesas/renesas-rzg2l-cpg.c b/drivers/clk/renesas/renesas-rzg2l-cpg.c
index ef308cb2368f..f32a270319e5 100644
--- a/drivers/clk/renesas/renesas-rzg2l-cpg.c
+++ b/drivers/clk/renesas/renesas-rzg2l-cpg.c
@@ -594,42 +594,49 @@ static int rzg2l_cpg_attach_dev(struct generic_pm_domain *unused, struct device
 {
 	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 (rzg2l_cpg_is_pm_clk(&clkspec))
-			goto found;
-
-		of_node_put(clkspec.np);
+		if (rzg2l_cpg_is_pm_clk(&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;
+			}
+		} else {
+			of_node_put(clkspec.np);
+		}
 		i++;
 	}
 
 	return 0;
 
-found:
-	clk = of_clk_get_from_provider(&clkspec);
-	of_node_put(clkspec.np);
-
-	if (IS_ERR(clk))
-		return PTR_ERR(clk);
-
-	error = pm_clk_create(dev);
-	if (error)
-		goto fail_put;
-
-	error = pm_clk_add_clk(dev, clk);
-	if (error)
-		goto fail_destroy;
-
-	return 0;
+fail_put:
+	clk_put(clk);
 
 fail_destroy:
 	pm_clk_destroy(dev);
-fail_put:
-	clk_put(clk);
+err:
 	return error;
 }
 
-- 
2.17.1

[PATCH 3/7] drivers: clk: renesas: r9a07g044-cpg: Update {GIC,IA55,SCIF} clock entries

[Biju Das]

Update {GIC,IA55,SCIF} clock entries to CPG driver to match with
RZ/G2L clock list hardware manual(Rev0.2).

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 drivers/clk/renesas/r9a07g044-cpg.c     | 37 +++++++++++++++++--------
 drivers/clk/renesas/renesas-rzg2l-cpg.h |  1 +
 2 files changed, 27 insertions(+), 11 deletions(-)

diff --git a/drivers/clk/renesas/r9a07g044-cpg.c b/drivers/clk/renesas/r9a07g044-cpg.c
index 50b5269586a4..5c215b6c06e0 100644
--- a/drivers/clk/renesas/r9a07g044-cpg.c
+++ b/drivers/clk/renesas/r9a07g044-cpg.c
@@ -32,6 +32,7 @@ enum clk_ids {
 	CLK_PLL3_DIV2,
 	CLK_PLL3_DIV4,
 	CLK_PLL3_DIV8,
+	CLK_PLL3_DIV16,
 	CLK_PLL4,
 	CLK_PLL5,
 	CLK_PLL5_DIV2,
@@ -42,6 +43,14 @@ enum clk_ids {
 };
 
 /* Divider tables */
+static const struct clk_div_table dtable_3a[] = {
+	{0, 1},
+	{1, 2},
+	{2, 4},
+	{3, 8},
+	{4, 32},
+};
+
 static const struct clk_div_table dtable_3b[] = {
 	{0, 1},
 	{1, 2},
@@ -68,6 +77,7 @@ static const struct cpg_core_clk r9a07g044_core_clks[] __initconst = {
 	DEF_FIXED(".pll3_div2", CLK_PLL3_DIV2, CLK_PLL3, 1, 2),
 	DEF_FIXED(".pll3_div4", CLK_PLL3_DIV4, CLK_PLL3, 1, 4),
 	DEF_FIXED(".pll3_div8", CLK_PLL3_DIV8, CLK_PLL3, 1, 8),
+	DEF_FIXED(".pll3_div16", CLK_PLL3_DIV16, CLK_PLL3, 1, 16),
 
 	/* Core output clk */
 	DEF_FIXED("I", R9A07G044_CLK_I, CLK_PLL1, 1, 1),
@@ -76,37 +86,42 @@ static const struct cpg_core_clk r9a07g044_core_clks[] __initconst = {
 	DEF_FIXED("TSU", R9A07G044_CLK_TSU, CLK_PLL2_DIV20, 1, 1),
 	DEF_DIV("P1", R9A07G044_CLK_P1, CLK_PLL3_DIV8,
 		DIVPL3B, dtable_3b, CLK_DIVIDER_HIWORD_MASK),
+	DEF_DIV("P2", R9A07G044_CLK_P2, CLK_PLL3_DIV16,
+		DIVPL3A, dtable_3a, CLK_DIVIDER_HIWORD_MASK),
 };
 
 static struct rzg2l_mod_clk r9a07g044_mod_clks[] = {
-	DEF_MOD("gic",		R9A07G044_CLK_GIC600,
+	DEF_MOD("gic",		R9A07G044_GIC600_GICCLK,
 				R9A07G044_CLK_P1,
 				0x514, BIT(0), (BIT(0) | BIT(1))),
-	DEF_MOD("ia55",		R9A07G044_CLK_IA55,
+	DEF_MOD("ia55_pclk",	R9A07G044_IA55_PCLK,
+				R9A07G044_CLK_P2,
+				0x518, BIT(0), BIT(0)),
+	DEF_MOD("ia55_clk",	R9A07G044_IA55_CLK,
 				R9A07G044_CLK_P1,
-				0x518, (BIT(0) | BIT(1)), BIT(0)),
-	DEF_MOD("scif0",	R9A07G044_CLK_SCIF0,
+				0x518, BIT(1), BIT(0)),
+	DEF_MOD("scif0",	R9A07G044_SCIF0_CLK_PCK,
 				R9A07G044_CLK_P0,
 				0x584, BIT(0), BIT(0)),
-	DEF_MOD("scif1",	R9A07G044_CLK_SCIF1,
+	DEF_MOD("scif1",	R9A07G044_SCIF1_CLK_PCK,
 				R9A07G044_CLK_P0,
 				0x584, BIT(1), BIT(1)),
-	DEF_MOD("scif2",	R9A07G044_CLK_SCIF2,
+	DEF_MOD("scif2",	R9A07G044_SCIF2_CLK_PCK,
 				R9A07G044_CLK_P0,
 				0x584, BIT(2), BIT(2)),
-	DEF_MOD("scif3",	R9A07G044_CLK_SCIF3,
+	DEF_MOD("scif3",	R9A07G044_SCIF3_CLK_PCK,
 				R9A07G044_CLK_P0,
 				0x584, BIT(3), BIT(3)),
-	DEF_MOD("scif4",	R9A07G044_CLK_SCIF4,
+	DEF_MOD("scif4",	R9A07G044_SCIF4_CLK_PCK,
 				R9A07G044_CLK_P0,
 				0x584, BIT(4), BIT(4)),
-	DEF_MOD("sci0",		R9A07G044_CLK_SCI0,
+	DEF_MOD("sci0",		R9A07G044_SCI0_CLKP,
 				R9A07G044_CLK_P0,
 				0x588, BIT(0), BIT(0)),
 };
 
 static const unsigned int r9a07g044_crit_mod_clks[] __initconst = {
-	MOD_CLK_BASE + R9A07G044_CLK_GIC600,
+	MOD_CLK_BASE + R9A07G044_GIC600_GICCLK,
 };
 
 const struct rzg2l_cpg_info r9a07g044_cpg_info = {
@@ -123,5 +138,5 @@ const struct rzg2l_cpg_info r9a07g044_cpg_info = {
 	/* Module Clocks */
 	.mod_clks = r9a07g044_mod_clks,
 	.num_mod_clks = ARRAY_SIZE(r9a07g044_mod_clks),
-	.num_hw_mod_clks = R9A07G044_CLK_MIPI_DSI_PIN + 1,
+	.num_hw_mod_clks = R9A07G044_TSU_PCLK + 1,
 };
diff --git a/drivers/clk/renesas/renesas-rzg2l-cpg.h b/drivers/clk/renesas/renesas-rzg2l-cpg.h
index 3948bdd8afc9..a6a3bade1985 100644
--- a/drivers/clk/renesas/renesas-rzg2l-cpg.h
+++ b/drivers/clk/renesas/renesas-rzg2l-cpg.h
@@ -21,6 +21,7 @@
 #define DDIV_PACK(offset, bitpos, size) \
 		(((offset) << 20) | ((bitpos) << 12) | ((size) << 8))
 #define DIVPL2A		DDIV_PACK(CPG_PL2_DDIV, 0, 3)
+#define DIVPL3A		DDIV_PACK(CPG_PL3A_DDIV, 0, 3)
 #define DIVPL3B		DDIV_PACK(CPG_PL3A_DDIV, 4, 3)
 
 /**
-- 
2.17.1

[PATCH 4/7] arm64: dts: renesas: r9a07g044: Update SCIF0 clock

[Biju Das]

Update SCIF0 clock index in SoC DTSI.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 arch/arm64/boot/dts/renesas/r9a07g044.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/renesas/r9a07g044.dtsi b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
index 476ee9a69065..544f040a4e1d 100644
--- a/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
+++ b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
@@ -82,10 +82,10 @@
 				     <GIC_SPI 384 IRQ_TYPE_LEVEL_HIGH>;
 			interrupt-names = "eri", "rxi", "txi",
 					  "bri", "dri", "tei";
-			clocks = <&cpg CPG_MOD R9A07G044_CLK_SCIF0>;
+			clocks = <&cpg CPG_MOD R9A07G044_SCIF0_CLK_PCK>;
 			clock-names = "fck";
 			power-domains = <&cpg>;
-			resets = <&cpg R9A07G044_CLK_SCIF0>;
+			resets = <&cpg R9A07G044_SCIF0_CLK_PCK>;
 			status = "disabled";
 		};
 
-- 
2.17.1

[PATCH 5/7] drivers: clk: renesas: r9a07g044-cpg: Add I2C Clocks

[Biju Das]

Add I2C{0,1,2.3} clock entries.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 drivers/clk/renesas/r9a07g044-cpg.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/clk/renesas/r9a07g044-cpg.c b/drivers/clk/renesas/r9a07g044-cpg.c
index 5c215b6c06e0..c7be8ede494d 100644
--- a/drivers/clk/renesas/r9a07g044-cpg.c
+++ b/drivers/clk/renesas/r9a07g044-cpg.c
@@ -100,6 +100,18 @@ static struct rzg2l_mod_clk r9a07g044_mod_clks[] = {
 	DEF_MOD("ia55_clk",	R9A07G044_IA55_CLK,
 				R9A07G044_CLK_P1,
 				0x518, BIT(1), BIT(0)),
+	DEF_MOD("i2c0",		R9A07G044_I2C0_PCLK,
+				R9A07G044_CLK_P0,
+				0x580, BIT(0), BIT(0)),
+	DEF_MOD("i2c1",		R9A07G044_I2C1_PCLK,
+				R9A07G044_CLK_P0,
+				0x580, BIT(1), BIT(1)),
+	DEF_MOD("i2c2",		R9A07G044_I2C2_PCLK,
+				R9A07G044_CLK_P0,
+				0x580, BIT(2), BIT(2)),
+	DEF_MOD("i2c3",		R9A07G044_I2C3_PCLK,
+				R9A07G044_CLK_P0,
+				0x580, BIT(3), BIT(3)),
 	DEF_MOD("scif0",	R9A07G044_SCIF0_CLK_PCK,
 				R9A07G044_CLK_P0,
 				0x584, BIT(0), BIT(0)),
-- 
2.17.1

[PATCH 6/7] drivers: clk: renesas: r9a07g044-cpg: Add DMAC clocks

[Biju Das]

Add DMAC clock entry in CPG driver.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 drivers/clk/renesas/r9a07g044-cpg.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/clk/renesas/r9a07g044-cpg.c b/drivers/clk/renesas/r9a07g044-cpg.c
index c7be8ede494d..bdede1d28086 100644
--- a/drivers/clk/renesas/r9a07g044-cpg.c
+++ b/drivers/clk/renesas/r9a07g044-cpg.c
@@ -37,6 +37,7 @@ enum clk_ids {
 	CLK_PLL5,
 	CLK_PLL5_DIV2,
 	CLK_PLL6,
+	CLK_P1_DIV2,
 
 	/* Module Clocks */
 	MOD_CLK_BASE,
@@ -86,6 +87,7 @@ static const struct cpg_core_clk r9a07g044_core_clks[] __initconst = {
 	DEF_FIXED("TSU", R9A07G044_CLK_TSU, CLK_PLL2_DIV20, 1, 1),
 	DEF_DIV("P1", R9A07G044_CLK_P1, CLK_PLL3_DIV8,
 		DIVPL3B, dtable_3b, CLK_DIVIDER_HIWORD_MASK),
+	DEF_FIXED("P1_DIV2", CLK_P1_DIV2, R9A07G044_CLK_P1, 1, 2),
 	DEF_DIV("P2", R9A07G044_CLK_P2, CLK_PLL3_DIV16,
 		DIVPL3A, dtable_3a, CLK_DIVIDER_HIWORD_MASK),
 };
@@ -100,6 +102,12 @@ static struct rzg2l_mod_clk r9a07g044_mod_clks[] = {
 	DEF_MOD("ia55_clk",	R9A07G044_IA55_CLK,
 				R9A07G044_CLK_P1,
 				0x518, BIT(1), BIT(0)),
+	DEF_MOD("dmac_aclk",	R9A07G044_DMAC_ACLK,
+				R9A07G044_CLK_P1,
+				0x52c, BIT(0), BIT(0)),
+	DEF_MOD("dmac_pclk",	R9A07G044_DMAC_PCLK,
+				CLK_P1_DIV2,
+				0x52c, BIT(1), BIT(1)),
 	DEF_MOD("i2c0",		R9A07G044_I2C0_PCLK,
 				R9A07G044_CLK_P0,
 				0x580, BIT(0), BIT(0)),
-- 
2.17.1

[PATCH 7/7] arm64: dts: renesas: r9a07g044: Add I2C nodes

[Biju Das]

Add I2C{0,1,2,3} nodes to RZ/G2 (R9A07G044) SoC DTSI.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 arch/arm64/boot/dts/renesas/r9a07g044.dtsi | 80 ++++++++++++++++++++++
 1 file changed, 80 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r9a07g044.dtsi b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
index 544f040a4e1d..b573ce88f4da 100644
--- a/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
+++ b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
@@ -89,6 +89,86 @@
 			status = "disabled";
 		};
 
+		i2c0: i2c@10058000 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "renesas,riic-r9a07g044", "renesas,riic-rz";
+			reg = <0 0x10058000 0 0x400>;
+			interrupts = <GIC_SPI 350  IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 348 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 349 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 352 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 353 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 351 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 354 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 355 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD R9A07G044_I2C0_PCLK>;
+			clock-frequency = <100000>;
+			resets = <&cpg R9A07G044_I2C0_PCLK>;
+			power-domains = <&cpg>;
+			status = "disabled";
+		};
+
+		i2c1: i2c@10058400 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "renesas,riic-r9a07g044", "renesas,riic-rz";
+			reg = <0 0x10058400 0 0x400>;
+			interrupts = <GIC_SPI 358  IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 356 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 357 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 360 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 361 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 359 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 362 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 363 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD R9A07G044_I2C1_PCLK>;
+			clock-frequency = <100000>;
+			resets = <&cpg R9A07G044_I2C1_PCLK>;
+			power-domains = <&cpg>;
+			status = "disabled";
+		};
+
+		i2c2: i2c@10058800 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "renesas,riic-r9a07g044", "renesas,riic-rz";
+			reg = <0 0x10058800 0 0x400>;
+			interrupts = <GIC_SPI 366  IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 364 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 365 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 368 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 369 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 367 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 370 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 371 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD R9A07G044_I2C2_PCLK>;
+			clock-frequency = <100000>;
+			resets = <&cpg R9A07G044_I2C2_PCLK>;
+			power-domains = <&cpg>;
+			status = "disabled";
+		};
+
+		i2c3: i2c@10058c00 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "renesas,riic-r9a07g044", "renesas,riic-rz";
+			reg = <0 0x10058c00 0 0x400>;
+			interrupts = <GIC_SPI 374  IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 372 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 373 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 376 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 377 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 375 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 378 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 379 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD R9A07G044_I2C3_PCLK>;
+			clock-frequency = <100000>;
+			resets = <&cpg R9A07G044_I2C3_PCLK>;
+			power-domains = <&cpg>;
+			status = "disabled";
+		};
+
 		cpg: clock-controller@11010000 {
 			compatible = "renesas,r9a07g044-cpg";
 			reg = <0 0x11010000 0 0x10000>;
-- 
2.17.1

Re: [PATCH 1/7] dt-bindings: clk: r9a07g044-cpg: Update clock definitions

[Geert Uytterhoeven]

Hi Biju,

On Fri, Jun 18, 2021 at 11:58 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> Update clock definitions as per the RZG2L_clock_list(Rev.02) manual.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Thanks for your patch!

> --- a/include/dt-bindings/clock/r9a07g044-cpg.h
> +++ b/include/dt-bindings/clock/r9a07g044-cpg.h

> +#define R9A07G044_USB_SCLK             73

It looks like USB_SCLK cannot be gated, but is driven directly from
OSCCLK, so I think it should be left out, and the DTS should just
reference R9A07G044_OSCCLK

> +#define R9A07G044_ETH0_CLK_AXI         74
> +#define R9A07G044_ETH0_CLK_CHI         75
> +#define R9A07G044_ETH0_REFCLK          76
> +#define R9A07G044_ETH1_CLK_AXI         77
> +#define R9A07G044_ETH1_CLK_CHI         78
> +#define R9A07G044_ETH1_REFCLK          79

According to the Hardware User's Manual, ETH0_REFCLK and  ETH1_REFCLK
cannot be gated (see the note for CPG_CLKMON_ETH), so I think it
should be left out, and the DTS should just reference R9A07G044_CLK_HP.

The rest of the clocks look good to me.  Some are still missing, but they
can be added later.

I do think we need a separate list of definitions for resets.  While
simple modules like SCIF and I2C have a one-to-one mapping from
clock bits to reset bits for, this is not the case for all modules.
E.g. SDHI has 4 clocks per instance, but only a single reset signal
per instance, while CANFD has a single clock, but two reset signals.

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 1/7] dt-bindings: clk: r9a07g044-cpg: Update clock definitions

[Biju Das]

Hi Geert,

Thanks for the feedback.

> Subject: Re: [PATCH 1/7] dt-bindings: clk: r9a07g044-cpg: Update clock
> definitions
> 
> Hi Biju,
> 
> On Fri, Jun 18, 2021 at 11:58 AM Biju Das <biju.das.jz@bp.renesas.com>
> wrote:
> > Update clock definitions as per the RZG2L_clock_list(Rev.02) manual.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> 
> Thanks for your patch!
> 
> > --- a/include/dt-bindings/clock/r9a07g044-cpg.h
> > +++ b/include/dt-bindings/clock/r9a07g044-cpg.h
> 
> > +#define R9A07G044_USB_SCLK             73
> 
> It looks like USB_SCLK cannot be gated, but is driven directly from
> OSCCLK, so I think it should be left out, and the DTS should just
> reference R9A07G044_OSCCLK

OK. Agreed, will remove this.

> 
> > +#define R9A07G044_ETH0_CLK_AXI         74
> > +#define R9A07G044_ETH0_CLK_CHI         75
> > +#define R9A07G044_ETH0_REFCLK          76
> > +#define R9A07G044_ETH1_CLK_AXI         77
> > +#define R9A07G044_ETH1_CLK_CHI         78
> > +#define R9A07G044_ETH1_REFCLK          79
> 
> According to the Hardware User's Manual, ETH0_REFCLK and  ETH1_REFCLK
> cannot be gated (see the note for CPG_CLKMON_ETH), so I think it should be
> left out, and the DTS should just reference R9A07G044_CLK_HP.

OK. Agreed. Will remove this.
> 
> The rest of the clocks look good to me.  Some are still missing, but they
> can be added later.
> 
> I do think we need a separate list of definitions for resets.  While
> simple modules like SCIF and I2C have a one-to-one mapping from clock bits
> to reset bits for, this is not the case for all modules.
> E.g. SDHI has 4 clocks per instance, but only a single reset signal per
> instance, while CANFD has a single clock, but two reset signals.


OK, Agreed. We will list separate definitions for resets like,

#define R9A07G044_RST_SDHI0		X1
#define R9A07G044_RST_SDHI1		X2
#define R9A07G044_RST_CAN		X3

Clk definitions

	DEF_MOD("sdhi0_imclk",	R9A07G044_SDHI0_IMCLK,
				CLK_SD0_DIV4,
				0x554, BIT(0)),
	DEF_MOD("sdhi0_imclk2",	R9A07G044_SDHI0_IMCLK2,
				CLK_SD0_DIV4,
				0x554, BIT(1)),
	DEF_MOD("sdhi0_clk_hs",	R9A07G044_SDHI0_CLK_HS,
				R9A07G044_CLK_SD0,
				0x554, BIT(2),
	DEF_MOD("sdhi0_aclk",	R9A07G044_SDHI0_ACLK,
				R9A07G044_CLK_P1,
				0x554, BIT(3)),


Reset definitions
--------------------
	DEF_RST("sdhi0_RST",	R9A07G044_RST_SDHI0,
				0x854, BIT(0)),


And DTS instantiate both reset and clock entries.

Is it ok to you? What is your thoughts on this?

Regards,
Biju



> 
> 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 1/7] dt-bindings: clk: r9a07g044-cpg: Update clock definitions

[Geert Uytterhoeven]

Hi Biju,

On Tue, Jun 22, 2021 at 11:26 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > Subject: Re: [PATCH 1/7] dt-bindings: clk: r9a07g044-cpg: Update clock
> > definitions
> > On Fri, Jun 18, 2021 at 11:58 AM Biju Das <biju.das.jz@bp.renesas.com>
> > wrote:
> > > Update clock definitions as per the RZG2L_clock_list(Rev.02) manual.
> > >
> > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > Thanks for your patch!
> >
> > > --- a/include/dt-bindings/clock/r9a07g044-cpg.h
> > > +++ b/include/dt-bindings/clock/r9a07g044-cpg.h

> > I do think we need a separate list of definitions for resets.  While
> > simple modules like SCIF and I2C have a one-to-one mapping from clock bits
> > to reset bits for, this is not the case for all modules.
> > E.g. SDHI has 4 clocks per instance, but only a single reset signal per
> > instance, while CANFD has a single clock, but two reset signals.
>
> OK, Agreed. We will list separate definitions for resets like,
>
> #define R9A07G044_RST_SDHI0             X1
> #define R9A07G044_RST_SDHI1             X2
> #define R9A07G044_RST_CAN               X3

Please use names that match the documentation, like
R9A07G044_SDHI0_IXRST and R9A07G044_SDHI0_CANFD_RSTP_N.

> Clk definitions
>
>         DEF_MOD("sdhi0_imclk",  R9A07G044_SDHI0_IMCLK,
>                                 CLK_SD0_DIV4,
>                                 0x554, BIT(0)),
>         DEF_MOD("sdhi0_imclk2", R9A07G044_SDHI0_IMCLK2,
>                                 CLK_SD0_DIV4,
>                                 0x554, BIT(1)),
>         DEF_MOD("sdhi0_clk_hs", R9A07G044_SDHI0_CLK_HS,
>                                 R9A07G044_CLK_SD0,
>                                 0x554, BIT(2),
>         DEF_MOD("sdhi0_aclk",   R9A07G044_SDHI0_ACLK,
>                                 R9A07G044_CLK_P1,
>                                 0x554, BIT(3)),

As each clock now corresponds to a single bit, you can store the bit
number (e.g. "0") instead of the bitmask ("BIT(0)").  This also works
for bits > 8, without needing to enlarge rzg2l_mod_clk.onoff  ;-)

> Reset definitions
> --------------------
>         DEF_RST("sdhi0_RST",    R9A07G044_RST_SDHI0,
>                                 0x854, BIT(0)),

Same here.
Note that you do not need names for resets, unlike clocks.

> And DTS instantiate both reset and clock entries.

What do you mean by "instantiate"?
The "clocks" and "resets" properties?

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 2/7] drivers: clk: renesas: renesas-rzg2l-cpg: Add multi clock PM support

[Geert Uytterhoeven]

Hi Biju,

On Fri, Jun 18, 2021 at 11:58 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> Add multi clock PM support for cpg driver.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

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

Re: [PATCH 3/7] drivers: clk: renesas: r9a07g044-cpg: Update {GIC,IA55,SCIF} clock entries

[Geert Uytterhoeven]

Hi Biju,

On Fri, Jun 18, 2021 at 11:58 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> Update {GIC,IA55,SCIF} clock entries to CPG driver to match with
> RZ/G2L clock list hardware manual(Rev0.2).
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Thanks for your patch!

> --- a/drivers/clk/renesas/r9a07g044-cpg.c
> +++ b/drivers/clk/renesas/r9a07g044-cpg.c
> @@ -32,6 +32,7 @@ enum clk_ids {
>         CLK_PLL3_DIV2,
>         CLK_PLL3_DIV4,
>         CLK_PLL3_DIV8,
> +       CLK_PLL3_DIV16,
>         CLK_PLL4,
>         CLK_PLL5,
>         CLK_PLL5_DIV2,
> @@ -42,6 +43,14 @@ enum clk_ids {
>  };
>
>  /* Divider tables */
> +static const struct clk_div_table dtable_3a[] = {
> +       {0, 1},
> +       {1, 2},
> +       {2, 4},
> +       {3, 8},
> +       {4, 32},
> +};

Divider tables have to end with a sentinel entry that has .div = 0.
Actually the same bug is present for dtable_3b[], oops.
Both tables are identical, perhaps they can be shared?

> +
>  static const struct clk_div_table dtable_3b[] = {
>         {0, 1},
>         {1, 2},

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 3/7] drivers: clk: renesas: r9a07g044-cpg: Update {GIC,IA55,SCIF} clock entries

[Biju Das]

Hi Geert,

Thanks for the feedback.

> Subject: Re: [PATCH 3/7] drivers: clk: renesas: r9a07g044-cpg: Update
> {GIC,IA55,SCIF} clock entries
> 
> Hi Biju,
> 
> On Fri, Jun 18, 2021 at 11:58 AM Biju Das <biju.das.jz@bp.renesas.com>
> wrote:
> > Update {GIC,IA55,SCIF} clock entries to CPG driver to match with
> > RZ/G2L clock list hardware manual(Rev0.2).
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> 
> Thanks for your patch!
> 
> > --- a/drivers/clk/renesas/r9a07g044-cpg.c
> > +++ b/drivers/clk/renesas/r9a07g044-cpg.c
> > @@ -32,6 +32,7 @@ enum clk_ids {
> >         CLK_PLL3_DIV2,
> >         CLK_PLL3_DIV4,
> >         CLK_PLL3_DIV8,
> > +       CLK_PLL3_DIV16,
> >         CLK_PLL4,
> >         CLK_PLL5,
> >         CLK_PLL5_DIV2,
> > @@ -42,6 +43,14 @@ enum clk_ids {
> >  };
> >
> >  /* Divider tables */
> > +static const struct clk_div_table dtable_3a[] = {
> > +       {0, 1},
> > +       {1, 2},
> > +       {2, 4},
> > +       {3, 8},
> > +       {4, 32},
> > +};
> 
> Divider tables have to end with a sentinel entry that has .div = 0.
> Actually the same bug is present for dtable_3b[], oops.
> Both tables are identical, perhaps they can be shared?

OK. Will fix this in next version.

Cheers,
Biju

> 
> > +
> >  static const struct clk_div_table dtable_3b[] = {
> >         {0, 1},
> >         {1, 2},
> 
> 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 1/7] dt-bindings: clk: r9a07g044-cpg: Update clock definitions

[Biju Das]

Hi Geert,

Thanks for the feedback.

> Subject: Re: [PATCH 1/7] dt-bindings: clk: r9a07g044-cpg: Update clock
> definitions
> 
> Hi Biju,
> 
> On Tue, Jun 22, 2021 at 11:26 AM Biju Das <biju.das.jz@bp.renesas.com>
> wrote:
> > > Subject: Re: [PATCH 1/7] dt-bindings: clk: r9a07g044-cpg: Update
> > > clock definitions On Fri, Jun 18, 2021 at 11:58 AM Biju Das
> > > <biju.das.jz@bp.renesas.com>
> > > wrote:
> > > > Update clock definitions as per the RZG2L_clock_list(Rev.02) manual.
> > > >
> > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > > > Reviewed-by: Lad Prabhakar
> > > > <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > >
> > > Thanks for your patch!
> > >
> > > > --- a/include/dt-bindings/clock/r9a07g044-cpg.h
> > > > +++ b/include/dt-bindings/clock/r9a07g044-cpg.h
> 
> > > I do think we need a separate list of definitions for resets.  While
> > > simple modules like SCIF and I2C have a one-to-one mapping from
> > > clock bits to reset bits for, this is not the case for all modules.
> > > E.g. SDHI has 4 clocks per instance, but only a single reset signal
> > > per instance, while CANFD has a single clock, but two reset signals.
> >
> > OK, Agreed. We will list separate definitions for resets like,
> >
> > #define R9A07G044_RST_SDHI0             X1
> > #define R9A07G044_RST_SDHI1             X2
> > #define R9A07G044_RST_CAN               X3
> 
> Please use names that match the documentation, like R9A07G044_SDHI0_IXRST
> and R9A07G044_SDHI0_CANFD_RSTP_N.

Just rethinking by looking at R-Car approach, We may not need defining resets in dt-binding file.

We can create a 16 bit unique index with register offset in the last 12bits and control bits in last 4 bits.
Device tree passes this index and driver extracts this info for reset handling.

This will avoid dt-binding dependency. Are you ok this approach for resets?? What about clock, existing method or similar 16bit index method??

Please share your thoughts.

> > Clk definitions
> >
> >         DEF_MOD("sdhi0_imclk",  R9A07G044_SDHI0_IMCLK,
> >                                 CLK_SD0_DIV4,
> >                                 0x554, BIT(0)),
> >         DEF_MOD("sdhi0_imclk2", R9A07G044_SDHI0_IMCLK2,
> >                                 CLK_SD0_DIV4,
> >                                 0x554, BIT(1)),
> >         DEF_MOD("sdhi0_clk_hs", R9A07G044_SDHI0_CLK_HS,
> >                                 R9A07G044_CLK_SD0,
> >                                 0x554, BIT(2),
> >         DEF_MOD("sdhi0_aclk",   R9A07G044_SDHI0_ACLK,
> >                                 R9A07G044_CLK_P1,
> >                                 0x554, BIT(3)),
> 
> As each clock now corresponds to a single bit, you can store the bit
> number (e.g. "0") instead of the bitmask ("BIT(0)").  This also works for
> bits > 8, without needing to enlarge rzg2l_mod_clk.onoff  ;-)

I agree, please see my above comment for unique index(offset + this val).

> > Reset definitions
> > --------------------
> >         DEF_RST("sdhi0_RST",    R9A07G044_RST_SDHI0,
> >                                 0x854, BIT(0)),
> 
> Same here.
> Note that you do not need names for resets, unlike clocks.

OK.

> > And DTS instantiate both reset and clock entries.
> 
> What do you mean by "instantiate"?
> The "clocks" and "resets" properties?

I mean clocks and reset properties.

Regards,
Biju

RE: [PATCH 3/7] drivers: clk: renesas: r9a07g044-cpg: Update {GIC,IA55,SCIF} clock entries

[Biju Das]

Hi Geert,

Thanks for the feedback

> Subject: Re: [PATCH 3/7] drivers: clk: renesas: r9a07g044-cpg: Update
> {GIC,IA55,SCIF} clock entries
> 
> Hi Biju,
> 
> On Fri, Jun 18, 2021 at 11:58 AM Biju Das <biju.das.jz@bp.renesas.com>
> wrote:
> > Update {GIC,IA55,SCIF} clock entries to CPG driver to match with
> > RZ/G2L clock list hardware manual(Rev0.2).
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> 
> Thanks for your patch!
> 
> > --- a/drivers/clk/renesas/r9a07g044-cpg.c
> > +++ b/drivers/clk/renesas/r9a07g044-cpg.c
> > @@ -32,6 +32,7 @@ enum clk_ids {
> >         CLK_PLL3_DIV2,
> >         CLK_PLL3_DIV4,
> >         CLK_PLL3_DIV8,
> > +       CLK_PLL3_DIV16,
> >         CLK_PLL4,
> >         CLK_PLL5,
> >         CLK_PLL5_DIV2,
> > @@ -42,6 +43,14 @@ enum clk_ids {
> >  };
> >
> >  /* Divider tables */
> > +static const struct clk_div_table dtable_3a[] = {
> > +       {0, 1},
> > +       {1, 2},
> > +       {2, 4},
> > +       {3, 8},
> > +       {4, 32},
> > +};
> 
> Divider tables have to end with a sentinel entry that has .div = 0.
> Actually the same bug is present for dtable_3b[], oops.
> Both tables are identical, perhaps they can be shared?

OK. The same table entries used by 4 dividers. So will change it to
dtable_common and also will add a sentinel entry that has .div = 0.

Regards,
Biju

> 
> > +
> >  static const struct clk_div_table dtable_3b[] = {
> >         {0, 1},
> >         {1, 2},
> 
> 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 1/7] dt-bindings: clk: r9a07g044-cpg: Update clock definitions

[Geert Uytterhoeven]

Hi Biju,

On Wed, Jun 23, 2021 at 1:11 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > Subject: Re: [PATCH 1/7] dt-bindings: clk: r9a07g044-cpg: Update clock
> > definitions
> > On Tue, Jun 22, 2021 at 11:26 AM Biju Das <biju.das.jz@bp.renesas.com>
> > wrote:
> > > > Subject: Re: [PATCH 1/7] dt-bindings: clk: r9a07g044-cpg: Update
> > > > clock definitions On Fri, Jun 18, 2021 at 11:58 AM Biju Das
> > > > <biju.das.jz@bp.renesas.com>
> > > > wrote:
> > > > > Update clock definitions as per the RZG2L_clock_list(Rev.02) manual.
> > > > >
> > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > > > > Reviewed-by: Lad Prabhakar
> > > > > <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > >
> > > > Thanks for your patch!
> > > >
> > > > > --- a/include/dt-bindings/clock/r9a07g044-cpg.h
> > > > > +++ b/include/dt-bindings/clock/r9a07g044-cpg.h
> >
> > > > I do think we need a separate list of definitions for resets.  While
> > > > simple modules like SCIF and I2C have a one-to-one mapping from
> > > > clock bits to reset bits for, this is not the case for all modules.
> > > > E.g. SDHI has 4 clocks per instance, but only a single reset signal
> > > > per instance, while CANFD has a single clock, but two reset signals.
> > >
> > > OK, Agreed. We will list separate definitions for resets like,
> > >
> > > #define R9A07G044_RST_SDHI0             X1
> > > #define R9A07G044_RST_SDHI1             X2
> > > #define R9A07G044_RST_CAN               X3
> >
> > Please use names that match the documentation, like R9A07G044_SDHI0_IXRST
> > and R9A07G044_SDHI0_CANFD_RSTP_N.
>
> Just rethinking by looking at R-Car approach, We may not need defining resets in dt-binding file.
>
> We can create a 16 bit unique index with register offset in the last 12bits and control bits in last 4 bits.
> Device tree passes this index and driver extracts this info for reset handling.
>
> This will avoid dt-binding dependency. Are you ok this approach for resets?? What about clock, existing method or similar 16bit index method??
>
> Please share your thoughts.

I did consider that option, too.  However, you would still need a bit
of thought/processing to convert from register offsets and bit indices
to clock/reset numbers and vice versa.
Compare this to MSTP clock numbers on R-Car (and GIC SPI IDs, and
DMA slave MID/RIDs), where you can just read the number from a table in
the Hardware User's Manual.
So I think it's easier to have a list of clock definitions in a
dt-bindings file.

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 1/7] dt-bindings: clk: r9a07g044-cpg: Update clock definitions

[Biju Das]

Hi Geert,

Thanks for the feedback.

> -----Original Message-----
> From: Geert Uytterhoeven <geert@linux-m68k.org>
> Sent: 23 June 2021 13:00
> To: Biju Das <biju.das.jz@bp.renesas.com>
> Cc: Rob Herring <robh+dt@kernel.org>; Prabhakar Mahadev Lad
> <prabhakar.mahadev-lad.rj@bp.renesas.com>; open list:OPEN FIRMWARE AND
> FLATTENED DEVICE TREE BINDINGS <devicetree@vger.kernel.org>; Chris
> Paterson <Chris.Paterson2@renesas.com>; Linux-Renesas <linux-renesas-
> soc@vger.kernel.org>
> Subject: Re: [PATCH 1/7] dt-bindings: clk: r9a07g044-cpg: Update clock
> definitions
> 
> Hi Biju,
> 
> On Wed, Jun 23, 2021 at 1:11 PM Biju Das <biju.das.jz@bp.renesas.com>
> wrote:
> > > Subject: Re: [PATCH 1/7] dt-bindings: clk: r9a07g044-cpg: Update
> > > clock definitions On Tue, Jun 22, 2021 at 11:26 AM Biju Das
> > > <biju.das.jz@bp.renesas.com>
> > > wrote:
> > > > > Subject: Re: [PATCH 1/7] dt-bindings: clk: r9a07g044-cpg: Update
> > > > > clock definitions On Fri, Jun 18, 2021 at 11:58 AM Biju Das
> > > > > <biju.das.jz@bp.renesas.com>
> > > > > wrote:
> > > > > > Update clock definitions as per the RZG2L_clock_list(Rev.02)
> manual.
> > > > > >
> > > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > > > > > Reviewed-by: Lad Prabhakar
> > > > > > <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > > >
> > > > > Thanks for your patch!
> > > > >
> > > > > > --- a/include/dt-bindings/clock/r9a07g044-cpg.h
> > > > > > +++ b/include/dt-bindings/clock/r9a07g044-cpg.h
> > >
> > > > > I do think we need a separate list of definitions for resets.
> > > > > While simple modules like SCIF and I2C have a one-to-one mapping
> > > > > from clock bits to reset bits for, this is not the case for all
> modules.
> > > > > E.g. SDHI has 4 clocks per instance, but only a single reset
> > > > > signal per instance, while CANFD has a single clock, but two reset
> signals.
> > > >
> > > > OK, Agreed. We will list separate definitions for resets like,
> > > >
> > > > #define R9A07G044_RST_SDHI0             X1
> > > > #define R9A07G044_RST_SDHI1             X2
> > > > #define R9A07G044_RST_CAN               X3
> > >
> > > Please use names that match the documentation, like
> > > R9A07G044_SDHI0_IXRST and R9A07G044_SDHI0_CANFD_RSTP_N.
> >
> > Just rethinking by looking at R-Car approach, We may not need defining
> resets in dt-binding file.
> >
> > We can create a 16 bit unique index with register offset in the last
> 12bits and control bits in last 4 bits.
> > Device tree passes this index and driver extracts this info for reset
> handling.
> >
> > This will avoid dt-binding dependency. Are you ok this approach for
> resets?? What about clock, existing method or similar 16bit index method??
> >
> > Please share your thoughts.
> 
> I did consider that option, too.  However, you would still need a bit of
> thought/processing to convert from register offsets and bit indices to
> clock/reset numbers and vice versa.

For resets, I have made some prototype(I2C/USB) with both the options and it works OK.

I2C0-->0x8800 (Offset:-0x880, bit index:0)
I2C1-->0x8801 (Offset:-0x880, bit index:1)
I2C2-->0x8802 (Offset:-0x880, bit index:2)
I2C3-->0x8803 (Offset:-0x880, bit index:3)

For USBHost0 reset:- 0x8783 and 0x8780
For USBHost1 reset:- 0x8783 and 0x8781
For USBdevice reset:- 0x8783 and 0x8782

On the code, reg = (index & 0xffff) >> 4; 
             Bitmask = BIT(index & 0xf);


> Compare this to MSTP clock numbers on R-Car (and GIC SPI IDs, and DMA
> slave MID/RIDs), where you can just read the number from a table in the
> Hardware User's Manual.
> So I think it's easier to have a list of clock definitions in a dt-
> bindings file.

OK. I will send V2 with this options.

Regards,
Biju

> 
> --
> 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