Re: [PATCH] clk: tegra: fix HOST1X clock divider on Tegra20 and Tegra30

[Dmitry Osipenko]

On 10/28/22 10:48, luca.ceresoli@xxxxxxxxxxx wrote:
> From: Luca Ceresoli <luca.ceresoli@xxxxxxxxxxx>
> 
> On Tegra20 and Tegra30 the HOST1X clock is a fractional clock divider with
> 7 integer bits + 1 decimal bit. This has been verified on both
> documentation and real hardware for Tegra20 an on the documentation I was
> able to find for Tegra30.
> 
> However in the kernel code this clock is declared as an integer divider. A
> consequence of this is that requesting 144 MHz for HOST1X which is fed by
> pll_p running at 216 MHz would result in 108 MHz (216 / 2) instead of 144
> MHz (216 / 1.5).
> 
> Fix by replacing the INT() macro with the MUX() macro which, despite the
> name, defines a fractional divider. The only difference between the two
> macros is the former does not have the TEGRA_DIVIDER_INT flag.
> 
> Also move the line together with the other MUX*() ones to keep the existing
> file organization.
> 
> Fixes: 76ebc134d45d ("clk: tegra: move periph clocks to common file")
> Cc: stable@xxxxxxxxxxxxxxx
> Cc: Peter De Schrijver <pdeschrijver@xxxxxxxxxx>
> Signed-off-by: Luca Ceresoli <luca.ceresoli@xxxxxxxxxxx>
> ---
>  drivers/clk/tegra/clk-tegra-periph.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/tegra/clk-tegra-periph.c b/drivers/clk/tegra/clk-tegra-periph.c
> index 4dcf7f7cb8a0..806d835ca0d2 100644
> --- a/drivers/clk/tegra/clk-tegra-periph.c
> +++ b/drivers/clk/tegra/clk-tegra-periph.c
> @@ -615,7 +615,6 @@ static struct tegra_periph_init_data periph_clks[] = {
>  	INT("vde", mux_pllp_pllc_pllm_clkm, CLK_SOURCE_VDE, 61, 0, tegra_clk_vde),
>  	INT("vi", mux_pllm_pllc_pllp_plla, CLK_SOURCE_VI, 20, 0, tegra_clk_vi),
>  	INT("epp", mux_pllm_pllc_pllp_plla, CLK_SOURCE_EPP, 19, 0, tegra_clk_epp),
> -	INT("host1x", mux_pllm_pllc_pllp_plla, CLK_SOURCE_HOST1X, 28, 0, tegra_clk_host1x),
>  	INT("mpe", mux_pllm_pllc_pllp_plla, CLK_SOURCE_MPE, 60, 0, tegra_clk_mpe),
>  	INT("2d", mux_pllm_pllc_pllp_plla, CLK_SOURCE_2D, 21, 0, tegra_clk_gr2d),
>  	INT("3d", mux_pllm_pllc_pllp_plla, CLK_SOURCE_3D, 24, 0, tegra_clk_gr3d),
> @@ -664,6 +663,7 @@ static struct tegra_periph_init_data periph_clks[] = {
>  	MUX("owr", mux_pllp_pllc_clkm, CLK_SOURCE_OWR, 71, TEGRA_PERIPH_ON_APB, tegra_clk_owr_8),
>  	MUX("nor", mux_pllp_pllc_pllm_clkm, CLK_SOURCE_NOR, 42, 0, tegra_clk_nor),
>  	MUX("mipi", mux_pllp_pllc_pllm_clkm, CLK_SOURCE_MIPI, 50, TEGRA_PERIPH_ON_APB, tegra_clk_mipi),
> +	MUX("host1x", mux_pllm_pllc_pllp_plla, CLK_SOURCE_HOST1X, 28, 0, tegra_clk_host1x),
>  	MUX("vi_sensor", mux_pllm_pllc_pllp_plla, CLK_SOURCE_VI_SENSOR, 20, TEGRA_PERIPH_NO_RESET, tegra_clk_vi_sensor),
>  	MUX("vi_sensor", mux_pllc_pllp_plla, CLK_SOURCE_VI_SENSOR, 20, TEGRA_PERIPH_NO_RESET, tegra_clk_vi_sensor_9),
>  	MUX("cilab", mux_pllp_pllc_clkm, CLK_SOURCE_CILAB, 144, 0, tegra_clk_cilab),

This was attempted in the past
https://lore.kernel.org/all/20180723085010.GK1636@xxxxxxxxxxxxxxxxxxxxxxxxx/

I assume here you're also porting the downstream patches to upstream.
This one is too questionable. The host1x clock shouldn't affect overall
performance to begin with. It doesn't make sense to use fractional clock
just for getting extra KHz.

-- 
Best regards,
Dmitry