Hi Marek,
On Mon, Mar 14, 2022 at 11:00 PM <marek.vasut@xxxxxxxxx> wrote:
> This patch adds ZG clocks used by the PowerVR GPU.
>
> Signed-off-by: Marek Vasut <marek.vasut+renesas@xxxxxxxxx>
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@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
