Re: [PATCH 2/2] clk: renesas: r9a09g077: Add xSPI core and module clocks

[Geert Uytterhoeven <geert@linux-m68k.org>]

Hi Prabhakar,

On Tue, 14 Oct 2025 at 17:13, Prabhakar <prabhakar.csengg@gmail.com> wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Add module and core clocks used by xSPI (Expanded SPI) IP on the
> R9A09G077 SoC.
>
> The xSPI block uses PCLKH as its bus clock, while the operation clock
> (XSPI_CLKn) is derived from PLL4. To support this, define new selectors
> and dividers (FSELXSPI0/1 and DIVSEL_XSPI0/1) in SCKCR.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Thanks for your patch!

> --- a/drivers/clk/renesas/r9a09g077-cpg.c
> +++ b/drivers/clk/renesas/r9a09g077-cpg.c

> @@ -105,6 +113,15 @@ static const struct clk_div_table dtable_1_2[] = {
>         {0, 0},
>  };
>
> +static const struct clk_div_table dtable_6_8_16_32_64[] = {
> +       {6, 64},
> +       {5, 32},
> +       {4, 16},
> +       {3, 8},
> +       {2, 6},
> +       {0, 0},
> +};
> +
>  static const struct clk_div_table dtable_24_25_30_32[] = {
>         {0, 32},
>         {1, 30},
> @@ -119,6 +136,7 @@ static const char * const sel_clk_pll0[] = { ".loco", ".pll0" };
>  static const char * const sel_clk_pll1[] = { ".loco", ".pll1" };
>  static const char * const sel_clk_pll2[] = { ".loco", ".pll2" };
>  static const char * const sel_clk_pll4[] = { ".loco", ".pll4" };
> +static const char * const sel_clk_pll4d1_div3_div4[] = { ".pll4d1_div3", ".pll4d1_div4" };
>
>  static const struct cpg_core_clk r9a09g077_core_clks[] __initconst = {
>         /* External Clock Inputs */
> @@ -154,6 +172,15 @@ static const struct cpg_core_clk r9a09g077_core_clks[] __initconst = {
>         DEF_DIV(".sci5async", CLK_SCI5ASYNC, CLK_PLL4D1, DIVSCI5ASYNC,
>                 dtable_24_25_30_32),
>
> +       DEF_FIXED(".pll4d1_div3", CLK_PLL4D1_DIV3, CLK_PLL4D1, 3, 1),
> +       DEF_FIXED(".pll4d1_div4", CLK_PLL4D1_DIV4, CLK_PLL4D1, 4, 1),
> +       DEF_MUX(".divselxspi0", CLK_DIVSELXSPI0_SCKCR, DIVSEL_XSPI0,
> +               sel_clk_pll4d1_div3_div4,
> +               ARRAY_SIZE(sel_clk_pll4d1_div3_div4), CLK_MUX_HIWORD_MASK),
> +       DEF_MUX(".divselxspi1", CLK_DIVSELXSPI1_SCKCR, DIVSEL_XSPI1,
> +               sel_clk_pll4d1_div3_div4,
> +               ARRAY_SIZE(sel_clk_pll4d1_div3_div4), CLK_MUX_HIWORD_MASK),
> +
>         /* Core output clk */
>         DEF_DIV("CA55C0", R9A09G077_CLK_CA55C0, CLK_SEL_CLK_PLL0, DIVCA55C0,
>                 dtable_1_2),
> @@ -178,9 +205,15 @@ static const struct cpg_core_clk r9a09g077_core_clks[] __initconst = {
>         DEF_FIXED("ETCLKC", R9A09G077_ETCLKC, CLK_SEL_CLK_PLL1, 10, 1),
>         DEF_FIXED("ETCLKD", R9A09G077_ETCLKD, CLK_SEL_CLK_PLL1, 20, 1),
>         DEF_FIXED("ETCLKE", R9A09G077_ETCLKE, CLK_SEL_CLK_PLL1, 40, 1),
> +       DEF_DIV("XSPI_CLK0", R9A09G077_XSPI_CLK0, CLK_DIVSELXSPI0_SCKCR,
> +               FSELXSPI0, dtable_6_8_16_32_64),
> +       DEF_DIV("XSPI_CLK1", R9A09G077_XSPI_CLK1, CLK_DIVSELXSPI1_SCKCR,
> +               FSELXSPI1, dtable_6_8_16_32_64),
>  };

Perhaps we need a custom clock for this?
According to Section 7.3.1 "SCKCR : System Clock Control Register",
some divider combinations are prohibited:
  - 4 x 6,
  - 4 x 32,
  - 4 x 64.
The last two are probably not an issue iff the xSPI driver never tries
to set the corresponding clock rates.
However, the first one may be an issue, as both 3 x 8 (valid) and 4 x 6
(prohibited) yield the same resulting divider, and I believe we cannot
be sure the clock core will never pick the prohibited combination.

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