Hi Wolfram,
On Wed, Jun 15, 2022 at 12:12 PM Wolfram Sang
<wsa+renesas@xxxxxxxxxxxxxxxxxxxx> wrote:
> Depending on the divider setting, SDSRC can have a different parent.
> Implement this when reading the divider, to get a correct clock tree.
> Setting the divider is left to the bootloader for now.
>
> Signed-off-by: Wolfram Sang <wsa+renesas@xxxxxxxxxxxxxxxxxxxx>
Thanks for your patch!
> Tested on my Spider board (r8a779f0). Only build tested for r8a779g0 but
> the docs for the registers are the same.
While the SDSRCSEL bits are the same, the register at offset 0x8a4 is
called SD0CKCR1 on R-Car S4-8, and CKSRCSELCR on R-Car V4H.
I guess that is why you removed the definition of SD0CKCR1, and stored
the register offset in DEF_GEN4_SDSRC(), despite both being the same?
> --- a/drivers/clk/renesas/r8a779f0-cpg-mssr.c
> +++ b/drivers/clk/renesas/r8a779f0-cpg-mssr.c
> @@ -71,7 +71,7 @@ static const struct cpg_core_clk r8a779f0_core_clks[] __initconst = {
> DEF_FIXED(".pll6_div2", CLK_PLL6_DIV2, CLK_PLL6, 2, 1),
> DEF_FIXED(".s0", CLK_S0, CLK_PLL1_DIV2, 2, 1),
>
> - DEF_BASE(".sdsrc", CLK_SDSRC, CLK_TYPE_GEN4_SDSRC, CLK_PLL5),
> + DEF_GEN4_SDSRC(".sdsrc", CLK_SDSRC, CLK_PLL5_DIV2, CLK_PLL5, 0x08a4),
> DEF_RATE(".oco", CLK_OCO, 32768),
>
> DEF_BASE(".rpcsrc", CLK_RPCSRC, CLK_TYPE_GEN4_RPCSRC, CLK_PLL5),
> diff --git a/drivers/clk/renesas/r8a779g0-cpg-mssr.c b/drivers/clk/renesas/r8a779g0-cpg-mssr.c
> index 3fc4233b1ead..c8cd32cf4606 100644
> --- a/drivers/clk/renesas/r8a779g0-cpg-mssr.c
> +++ b/drivers/clk/renesas/r8a779g0-cpg-mssr.c
> @@ -86,7 +86,7 @@ static const struct cpg_core_clk r8a779g0_core_clks[] __initconst = {
> DEF_FIXED(".s0_hsc", CLK_S0_HSC, CLK_PLL1_DIV2, 2, 1),
> DEF_FIXED(".sv_vip", CLK_SV_VIP, CLK_PLL1, 5, 1),
> DEF_FIXED(".sv_ir", CLK_SV_IR, CLK_PLL1, 5, 1),
> - DEF_BASE(".sdsrc", CLK_SDSRC, CLK_TYPE_GEN4_SDSRC, CLK_PLL5),
> + DEF_GEN4_SDSRC(".sdsrc", CLK_SDSRC, CLK_PLL5_DIV2, CLK_PLL5, 0x08a4),
> DEF_RATE(".oco", CLK_OCO, 32768),
>
> DEF_BASE(".rpcsrc", CLK_RPCSRC, CLK_TYPE_GEN4_RPCSRC, CLK_PLL5),
> diff --git a/drivers/clk/renesas/rcar-gen4-cpg.c b/drivers/clk/renesas/rcar-gen4-cpg.c
> index c7ed43d6aa67..c6662ec10292 100644
> --- a/drivers/clk/renesas/rcar-gen4-cpg.c
> +++ b/drivers/clk/renesas/rcar-gen4-cpg.c
> @@ -240,7 +240,15 @@ struct clk * __init rcar_gen4_cpg_clk_register(struct device *dev,
> base, core->div, core->offset);
>
> case CLK_TYPE_GEN4_SDSRC:
> - div = ((readl(base + SD0CKCR1) >> 29) & 0x03) + 4;
> + value = (readl(base + core->offset) >> 29) & 3;
> + if (value) {
> + div = value + 4;
> + } else {
> + parent = clks[core->parent >> 16];
> + if (IS_ERR(parent))
> + return ERR_CAST(parent);
> + div = 2;
> + }
So this gives the exact same divider of PLL5 before.
The clock diagram indeed shows different paths for value 0
(PLL5 -> 1/2 -> 1/2) and values 1 and 2 (PLL5 -> {1/5 or 1/6}).
But the textual description for SDSRC says "The SDSRC divider divides
PLL5 output clock", matching the original code.
Do we have to complicate the code? ;-)
I guess the clock diagram was based on the diagram for R-Car H3
(which has two daisy-chained fixed 1/2 dividers), with the new 1/5
and 1/6 dividers added.
> --- a/drivers/clk/renesas/rcar-gen4-cpg.h
> +++ b/drivers/clk/renesas/rcar-gen4-cpg.h
> @@ -67,7 +71,6 @@ struct rcar_gen4_cpg_pll_config {
> };
>
> #define CPG_RPCCKCR 0x874
> -#define SD0CKCR1 0x8a4
>
> struct clk *rcar_gen4_cpg_clk_register(struct device *dev,
> const struct cpg_core_clk *core, const struct cpg_mssr_info *info,
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
