Hi Prabhakar,
On Thu, May 5, 2022 at 9:32 PM Lad Prabhakar
<prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx> wrote:
> In preparation for adding support for Renesas RZ/Five SoC as part of
> r9a07g043-cpg.c file, split up the core clock, module clock and resets
> array into common and SoC specific.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
Thanks for your patch!
> --- a/drivers/clk/renesas/r9a07g043-cpg.c
> +++ b/drivers/clk/renesas/r9a07g043-cpg.c
> @@ -36,9 +36,11 @@ enum clk_ids {
> CLK_PLL3_DIV2_4_2,
> CLK_SEL_PLL3_3,
> CLK_DIV_PLL3_C,
> +#ifndef CONFIG_RISCV
Perhaps make this a positive check, i.e. check for CONFIG_ARM64?
Just in case Renesas spins out another variant with a
non-arm64/non-riscv core ;-)
> CLK_PLL5,
> CLK_PLL5_500,
> CLK_PLL5_250,
> +#endif
Technically, this #ifdef protection is not needed, but it helps to
catch errors below.
> CLK_PLL6,
> CLK_PLL6_250,
> CLK_P1_DIV2,
> @@ -76,227 +78,271 @@ static const char * const sel_pll3_3[] = { ".pll3_533", ".pll3_400" };
> static const char * const sel_pll6_2[] = { ".pll6_250", ".pll5_250" };
> static const char * const sel_shdi[] = { ".clk_533", ".clk_400", ".clk_266" };
>
> -static const struct cpg_core_clk r9a07g043_core_clks[] __initconst = {
> - /* External Clock Inputs */
> - DEF_INPUT("extal", CLK_EXTAL),
> +static const struct {
> + struct cpg_core_clk common[38];
> +#ifndef CONFIG_RISCV
> + struct cpg_core_clk rzg2ul[3];
> +#endif
Unlike in the r9a07g044 vs. r9a07g054 case, there is no need to have
access to the individual arrays at runtime. So you can just keep the
single r9a07g043_core_clks[] array, and add #ifdef/#endif around the
clock definitions for clocks that are present on one variant only.
> +} core_clks __initconst = {
> + .common = {
> + /* External Clock Inputs */
> + DEF_INPUT("extal", CLK_EXTAL),
>
> - /* Internal Core Clocks */
> - DEF_FIXED(".osc", R9A07G043_OSCCLK, CLK_EXTAL, 1, 1),
> - DEF_FIXED(".osc_div1000", CLK_OSC_DIV1000, CLK_EXTAL, 1, 1000),
> - DEF_SAMPLL(".pll1", CLK_PLL1, CLK_EXTAL, PLL146_CONF(0)),
> - DEF_FIXED(".pll2", CLK_PLL2, CLK_EXTAL, 200, 3),
> - DEF_FIXED(".pll2_div2", CLK_PLL2_DIV2, CLK_PLL2, 1, 2),
> - DEF_FIXED(".clk_800", CLK_PLL2_800, CLK_PLL2, 1, 2),
> - DEF_FIXED(".clk_533", CLK_PLL2_SDHI_533, CLK_PLL2, 1, 3),
> - DEF_FIXED(".clk_400", CLK_PLL2_SDHI_400, CLK_PLL2_800, 1, 2),
> - DEF_FIXED(".clk_266", CLK_PLL2_SDHI_266, CLK_PLL2_SDHI_533, 1, 2),
> - DEF_FIXED(".pll2_div2_8", CLK_PLL2_DIV2_8, CLK_PLL2_DIV2, 1, 8),
> - DEF_FIXED(".pll2_div2_10", CLK_PLL2_DIV2_10, CLK_PLL2_DIV2, 1, 10),
> - DEF_FIXED(".pll3", CLK_PLL3, CLK_EXTAL, 200, 3),
> - DEF_FIXED(".pll3_div2", CLK_PLL3_DIV2, CLK_PLL3, 1, 2),
> - DEF_FIXED(".pll3_div2_4", CLK_PLL3_DIV2_4, CLK_PLL3_DIV2, 1, 4),
> - DEF_FIXED(".pll3_div2_4_2", CLK_PLL3_DIV2_4_2, CLK_PLL3_DIV2_4, 1, 2),
> - DEF_FIXED(".pll3_400", CLK_PLL3_400, CLK_PLL3, 1, 4),
> - DEF_FIXED(".pll3_533", CLK_PLL3_533, CLK_PLL3, 1, 3),
> - DEF_MUX(".sel_pll3_3", CLK_SEL_PLL3_3, SEL_PLL3_3,
> - sel_pll3_3, ARRAY_SIZE(sel_pll3_3), 0, CLK_MUX_READ_ONLY),
> - DEF_DIV("divpl3c", CLK_DIV_PLL3_C, CLK_SEL_PLL3_3,
> - DIVPL3C, dtable_1_32, CLK_DIVIDER_HIWORD_MASK),
I.e. add
#ifdef CONFIG_ARM64
> - DEF_FIXED(".pll5", CLK_PLL5, CLK_EXTAL, 125, 1),
> - DEF_FIXED(".pll5_500", CLK_PLL5_500, CLK_PLL5, 1, 6),
> - DEF_FIXED(".pll5_250", CLK_PLL5_250, CLK_PLL5_500, 1, 2),
#endif
> - DEF_FIXED(".pll6", CLK_PLL6, CLK_EXTAL, 125, 6),
> - DEF_FIXED(".pll6_250", CLK_PLL6_250, CLK_PLL6, 1, 2),
> + /* Internal Core Clocks */
> + DEF_FIXED(".osc", R9A07G043_OSCCLK, CLK_EXTAL, 1, 1),
[...]
> + DEF_FIXED("SD1_DIV4", CLK_SD1_DIV4, R9A07G043_CLK_SD1, 1, 4),
> + },
> +#ifndef CONFIG_RISCV
> + .rzg2ul = {
> + DEF_FIXED(".pll5", CLK_PLL5, CLK_EXTAL, 125, 1),
> + DEF_FIXED(".pll5_500", CLK_PLL5_500, CLK_PLL5, 1, 6),
> + DEF_FIXED(".pll5_250", CLK_PLL5_250, CLK_PLL5_500, 1, 2),
> + },
> +#endif
>
> - /* Core output clk */
> - DEF_DIV("I", R9A07G043_CLK_I, CLK_PLL1, DIVPL1A, dtable_1_8,
> - CLK_DIVIDER_HIWORD_MASK),
> - DEF_DIV("P0", R9A07G043_CLK_P0, CLK_PLL2_DIV2_8, DIVPL2A,
> - dtable_1_32, CLK_DIVIDER_HIWORD_MASK),
> - DEF_FIXED("P0_DIV2", R9A07G043_CLK_P0_DIV2, R9A07G043_CLK_P0, 1, 2),
> - DEF_FIXED("TSU", R9A07G043_CLK_TSU, CLK_PLL2_DIV2_10, 1, 1),
> - DEF_DIV("P1", R9A07G043_CLK_P1, CLK_PLL3_DIV2_4,
> - DIVPL3B, dtable_1_32, CLK_DIVIDER_HIWORD_MASK),
> - DEF_FIXED("P1_DIV2", CLK_P1_DIV2, R9A07G043_CLK_P1, 1, 2),
> - DEF_DIV("P2", R9A07G043_CLK_P2, CLK_PLL3_DIV2_4_2,
> - DIVPL3A, dtable_1_32, CLK_DIVIDER_HIWORD_MASK),
> - DEF_FIXED("M0", R9A07G043_CLK_M0, CLK_PLL3_DIV2_4, 1, 1),
> - DEF_FIXED("ZT", R9A07G043_CLK_ZT, CLK_PLL3_DIV2_4_2, 1, 1),
> - DEF_MUX("HP", R9A07G043_CLK_HP, SEL_PLL6_2,
> - sel_pll6_2, ARRAY_SIZE(sel_pll6_2), 0, CLK_MUX_HIWORD_MASK),
> - DEF_FIXED("SPI0", R9A07G043_CLK_SPI0, CLK_DIV_PLL3_C, 1, 2),
> - DEF_FIXED("SPI1", R9A07G043_CLK_SPI1, CLK_DIV_PLL3_C, 1, 4),
> - DEF_SD_MUX("SD0", R9A07G043_CLK_SD0, SEL_SDHI0,
> - sel_shdi, ARRAY_SIZE(sel_shdi)),
> - DEF_SD_MUX("SD1", R9A07G043_CLK_SD1, SEL_SDHI1,
> - sel_shdi, ARRAY_SIZE(sel_shdi)),
> - DEF_FIXED("SD0_DIV4", CLK_SD0_DIV4, R9A07G043_CLK_SD0, 1, 4),
> - DEF_FIXED("SD1_DIV4", CLK_SD1_DIV4, R9A07G043_CLK_SD1, 1, 4),
> };
>
> -static struct rzg2l_mod_clk r9a07g043_mod_clks[] = {
Likewise, you can keep r9a07g043_mod_clks[].
#ifdef CONFIG_ARM64
> - DEF_MOD("gic", R9A07G043_GIC600_GICCLK, R9A07G043_CLK_P1,
> - 0x514, 0),
> - DEF_MOD("ia55_pclk", R9A07G043_IA55_PCLK, R9A07G043_CLK_P2,
> - 0x518, 0),
> - DEF_MOD("ia55_clk", R9A07G043_IA55_CLK, R9A07G043_CLK_P1,
> - 0x518, 1),
#endif
and add the RZ/Five-only module clocks later protected by
#ifdef CONFIG_RISCV.
> - DEF_MOD("dmac_aclk", R9A07G043_DMAC_ACLK, R9A07G043_CLK_P1,
> - 0x52c, 0),
[...]
> +static const struct {
> + struct rzg2l_mod_clk common[54];
> +#ifdef CONFIG_RISCV
> + struct rzg2l_mod_clk rzfive[0];
> +#else
> + struct rzg2l_mod_clk rzg2ul[3];
> +#endif
> +} mod_clks = {
> + .common = {
> + DEF_MOD("dmac_aclk", R9A07G043_DMAC_ACLK, R9A07G043_CLK_P1,
> + 0x52c, 0),
[...]
> + DEF_MOD("tsu_pclk", R9A07G043_TSU_PCLK, R9A07G043_CLK_TSU,
> + 0x5ac, 0),
> + },
> +#ifdef CONFIG_RISCV
> + .rzfive = {
> + },
> +#else
> + .rzg2ul = {
> + DEF_MOD("gic", R9A07G043_GIC600_GICCLK, R9A07G043_CLK_P1,
> + 0x514, 0),
> + DEF_MOD("ia55_pclk", R9A07G043_IA55_PCLK, R9A07G043_CLK_P2,
> + 0x518, 0),
> + DEF_MOD("ia55_clk", R9A07G043_IA55_CLK, R9A07G043_CLK_P1,
> + 0x518, 1),
> + },
> +#endif
> };
>
> -static struct rzg2l_reset r9a07g043_resets[] = {
If you do the same for the resets, you can drop "[RFC PATCH 2/4]
clk: renesas: rzg2l-cpg: Add support to stack the resets instead
of indexing".
> @@ -307,8 +353,8 @@ static const unsigned int r9a07g043_crit_mod_clks[] __initconst = {
>
> const struct rzg2l_cpg_info r9a07g043_cpg_info = {
> /* Core Clocks */
> - .core_clks = r9a07g043_core_clks,
> - .num_core_clks = ARRAY_SIZE(r9a07g043_core_clks),
> + .core_clks = core_clks.common,
> + .num_core_clks = ARRAY_SIZE(core_clks.common) + ARRAY_SIZE(core_clks.rzg2ul),
Then this change is not needed...
> .last_dt_core_clk = LAST_DT_CORE_CLK,
> .num_total_core_clks = MOD_CLK_BASE,
>
> @@ -317,11 +363,11 @@ const struct rzg2l_cpg_info r9a07g043_cpg_info = {
> .num_crit_mod_clks = ARRAY_SIZE(r9a07g043_crit_mod_clks),
>
> /* Module Clocks */
> - .mod_clks = r9a07g043_mod_clks,
> - .num_mod_clks = ARRAY_SIZE(r9a07g043_mod_clks),
> + .mod_clks = mod_clks.common,
> + .num_mod_clks = ARRAY_SIZE(mod_clks.common) + ARRAY_SIZE(mod_clks.rzg2ul),
Same here.
#ifdef CONFIG_ARM64
> .num_hw_mod_clks = R9A07G043_TSU_PCLK + 1,
#endif
Note that compile-testing on non-arm64/non-riscv still works fine,
as .num_hw_mod_clks = 0 is not an issue at build-time.
>
> /* Resets */
> - .resets = r9a07g043_resets,
#ifdef CONFIG_ARM64
> - .num_resets = R9A07G043_TSU_PRESETN + 1, /* Last reset ID + 1 */
#endif
> + .resets = resets.common,
> + ARRAY_SIZE(resets.common) + ARRAY_SIZE(resets.rzg2ul),
(BTW, ".num_resets = " was lost here)
> };
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
