| Message ID | 20220314220012.218731-1-marek.vasut@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | [1/3] clk: renesas: r8a7795: Add ZG clocks | expand |
Hi Marek, On Mon, Mar 14, 2022 at 11:00 PM <marek.vasut@gmail.com> wrote: > This patch adds ZG clocks used by the PowerVR GPU. > > Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com> Thanks for your patch! > --- a/drivers/clk/renesas/r8a7795-cpg-mssr.c > +++ b/drivers/clk/renesas/r8a7795-cpg-mssr.c > @@ -83,6 +83,7 @@ static struct cpg_core_clk r8a7795_core_clks[] __initdata = { > /* Core Clock Outputs */ > DEF_GEN3_Z("z", R8A7795_CLK_Z, CLK_TYPE_GEN3_Z, CLK_PLL0, 2, 8), > DEF_GEN3_Z("z2", R8A7795_CLK_Z2, CLK_TYPE_GEN3_Z, CLK_PLL2, 2, 0), > + DEF_GEN3_Z("zg", R8A7795_CLK_ZG, CLK_TYPE_GEN3_Z, CLK_PLL4, 2, 0), The "2" is not correct: /sys/kernel/debug/clk/clk_summary should tell you it is running at 1200 MHz instead of 600 MHz. The correct value is "4", as PLL4 is divided by 4 and by the ZG/3DGE divider, cfr. the documentation for the PLL4CR.STC6 field. The "0" is also not correct, as it refers to the bit field starting in the FRQCRC register starting at bit 0, which is not the ZG but the Z2 divider, cfr. the line above. The actual ZG divider field was never documented, and the documentation states the divider is fixed[1]. However, the BSP assumes the field does exist, and is located in the FRQCRB register[2], starting at bit 24[3]. Older BSP versions assumed a fixed clock[4]. What do we do? Follow the current BSP, or the documentation? Do you have a use case for changing the divider to lower\ the ZG clock rate? Note that we do not need to change the divider to increase the ZG clock rate to 700 MHz for High Performance mode on R-Car M3-W/W+/N, as that would be done by making PLL4 configurable, cfr. PLL0 and PLL2. > DEF_FIXED("ztr", R8A7795_CLK_ZTR, CLK_PLL1_DIV2, 6, 1), > DEF_FIXED("ztrd2", R8A7795_CLK_ZTRD2, CLK_PLL1_DIV2, 12, 1), > DEF_FIXED("zt", R8A7795_CLK_ZT, CLK_PLL1_DIV2, 4, 1), > @@ -129,6 +130,7 @@ static struct cpg_core_clk r8a7795_core_clks[] __initdata = { > }; > > static struct mssr_mod_clk r8a7795_mod_clks[] __initdata = { > + DEF_MOD("3dge", 112, R8A7795_CLK_ZG), > DEF_MOD("fdp1-2", 117, R8A7795_CLK_S2D1), /* ES1.x */ > DEF_MOD("fdp1-1", 118, R8A7795_CLK_S0D1), > DEF_MOD("fdp1-0", 119, R8A7795_CLK_S0D1), This part looks good to me. The same comments apply to patches 2/3 and 3/3 of this series. [1] Section 8.3.1 ("Changing Frequency"): "R-Car H3, R-Car H3-N, R-Car M3-W, R-Car M3-W+, and R-Car M3-N do not support to change division ratio of ZGϕ." [2] https://github.com/renesas-rcar/linux-bsp/blob/rcar-5.1.4/drivers/clk/renesas/rcar-gen3-cpg.c#L365 [3] https://github.com/renesas-rcar/linux-bsp/blob/rcar-5.1.4/drivers/clk/renesas/r8a7795-cpg-mssr.c#L86 [4] https://github.com/renesas-rcar/linux-bsp/blob/rcar-3.5.3/drivers/clk/renesas/r8a7795-cpg-mssr.c#L80 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
diff --git a/drivers/clk/renesas/r8a7795-cpg-mssr.c b/drivers/clk/renesas/r8a7795-cpg-mssr.c index 991a44315d71..35fb0922d924 100644 --- a/drivers/clk/renesas/r8a7795-cpg-mssr.c +++ b/drivers/clk/renesas/r8a7795-cpg-mssr.c @@ -83,6 +83,7 @@ static struct cpg_core_clk r8a7795_core_clks[] __initdata = { /* Core Clock Outputs */ DEF_GEN3_Z("z", R8A7795_CLK_Z, CLK_TYPE_GEN3_Z, CLK_PLL0, 2, 8), DEF_GEN3_Z("z2", R8A7795_CLK_Z2, CLK_TYPE_GEN3_Z, CLK_PLL2, 2, 0), + DEF_GEN3_Z("zg", R8A7795_CLK_ZG, CLK_TYPE_GEN3_Z, CLK_PLL4, 2, 0), DEF_FIXED("ztr", R8A7795_CLK_ZTR, CLK_PLL1_DIV2, 6, 1), DEF_FIXED("ztrd2", R8A7795_CLK_ZTRD2, CLK_PLL1_DIV2, 12, 1), DEF_FIXED("zt", R8A7795_CLK_ZT, CLK_PLL1_DIV2, 4, 1), @@ -129,6 +130,7 @@ static struct cpg_core_clk r8a7795_core_clks[] __initdata = { }; static struct mssr_mod_clk r8a7795_mod_clks[] __initdata = { + DEF_MOD("3dge", 112, R8A7795_CLK_ZG), DEF_MOD("fdp1-2", 117, R8A7795_CLK_S2D1), /* ES1.x */ DEF_MOD("fdp1-1", 118, R8A7795_CLK_S0D1), DEF_MOD("fdp1-0", 119, R8A7795_CLK_S0D1),