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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <603a0227-7d25-b9da-6dc3-fa9fe1b951e7@collabora.com>
Date:   Mon, 31 Oct 2022 03:34:07 +0300
From:   Dmitry Osipenko <dmitry.osipenko@...labora.com>
To:     luca.ceresoli@...tlin.com, linux-tegra@...r.kernel.org
Cc:     Peter De Schrijver <pdeschrijver@...dia.com>,
        Prashant Gaikwad <pgaikwad@...dia.com>,
        Michael Turquette <mturquette@...libre.com>,
        Stephen Boyd <sboyd@...nel.org>,
        Thierry Reding <thierry.reding@...il.com>,
        Jonathan Hunter <jonathanh@...dia.com>,
        linux-clk@...r.kernel.org, linux-kernel@...r.kernel.org,
        Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
        stable@...r.kernel.org
Subject: Re: [PATCH] clk: tegra: fix HOST1X clock divider on Tegra20 and
 Tegra30

On 10/28/22 10:48, luca.ceresoli@...tlin.com wrote:
> From: Luca Ceresoli <luca.ceresoli@...tlin.com>
> 
> 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@...r.kernel.org
> Cc: Peter De Schrijver <pdeschrijver@...dia.com>
> Signed-off-by: Luca Ceresoli <luca.ceresoli@...tlin.com>
> ---
>  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@tbergstrom-lnx.Nvidia.com/

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