| 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
| ||
|
Message-ID: <2253671.Mh6RI2rZIc@senjougahara>
Date: Fri, 22 Aug 2025 11:53:05 +0900
From: Mikko Perttunen <mperttunen@...dia.com>
To: Thierry Reding <thierry.reding@...il.com>,
Jonathan Hunter <jonathanh@...dia.com>,
Peter De Schrijver <pdeschrijver@...dia.com>,
Prashant Gaikwad <pgaikwad@...dia.com>,
Michael Turquette <mturquette@...libre.com>, Stephen Boyd <sboyd@...nel.org>,
"Rafael J. Wysocki" <rafael@...nel.org>,
Viresh Kumar <viresh.kumar@...aro.org>,
Philipp Zabel <p.zabel@...gutronix.de>,
Svyatoslav Ryhel <clamor95@...il.com>, Svyatoslav Ryhel <clamor95@...il.com>
Cc: linux-tegra@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-clk@...r.kernel.org, linux-pm@...r.kernel.org
Subject: Re: [PATCH v1 2/3] drivers: clk: tegra: add DFLL support for Tegra 4
On Friday, March 21, 2025 6:55 PM Svyatoslav Ryhel wrote:
> Extend the Tegra124 driver to include DFLL configuration settings required
> for Tegra114 compatibility.
>
> Signed-off-by: Svyatoslav Ryhel <clamor95@...il.com>
> ---
> drivers/clk/tegra/Kconfig | 2 +-
> drivers/clk/tegra/clk-tegra114.c | 30 +++++-
> drivers/clk/tegra/clk-tegra124-dfll-fcpu.c | 104 +++++++++++++++++++++
> drivers/clk/tegra/clk.h | 2 -
> include/dt-bindings/reset/tegra114-car.h | 13 +++
> 5 files changed, 144 insertions(+), 7 deletions(-)
> create mode 100644 include/dt-bindings/reset/tegra114-car.h
>
> diff --git a/drivers/clk/tegra/Kconfig b/drivers/clk/tegra/Kconfig
> index 90df619dc087..62147a069606 100644
> --- a/drivers/clk/tegra/Kconfig
> +++ b/drivers/clk/tegra/Kconfig
> @@ -4,7 +4,7 @@ config CLK_TEGRA_BPMP
> depends on TEGRA_BPMP
>
> config TEGRA_CLK_DFLL
> - depends on ARCH_TEGRA_124_SOC || ARCH_TEGRA_210_SOC
> + depends on ARCH_TEGRA_114_SOC || ARCH_TEGRA_124_SOC ||
ARCH_TEGRA_210_SOC
> select PM_OPP
> def_bool y
>
> diff --git a/drivers/clk/tegra/clk-tegra114.c
> b/drivers/clk/tegra/clk-tegra114.c index b19dd4e6e17c..9b6794b951a2 100644
> --- a/drivers/clk/tegra/clk-tegra114.c
> +++ b/drivers/clk/tegra/clk-tegra114.c
> @@ -11,6 +11,7 @@
> #include <linux/export.h>
> #include <linux/clk/tegra.h>
> #include <dt-bindings/clock/tegra114-car.h>
> +#include <dt-bindings/reset/tegra114-car.h>
>
> #include "clk.h"
> #include "clk-id.h"
> @@ -1260,7 +1261,7 @@ EXPORT_SYMBOL(tegra114_clock_tune_cpu_trimmers_init);
> *
> * Assert the reset line of the DFLL's DVCO. No return value.
> */
> -void tegra114_clock_assert_dfll_dvco_reset(void)
> +static void tegra114_clock_assert_dfll_dvco_reset(void)
> {
> u32 v;
>
> @@ -1269,7 +1270,6 @@ void tegra114_clock_assert_dfll_dvco_reset(void)
> writel_relaxed(v, clk_base + RST_DFLL_DVCO);
> tegra114_car_barrier();
> }
> -EXPORT_SYMBOL(tegra114_clock_assert_dfll_dvco_reset);
>
> /**
> * tegra114_clock_deassert_dfll_dvco_reset - deassert the DFLL's DVCO reset
> @@ -1277,7 +1277,7 @@ EXPORT_SYMBOL(tegra114_clock_assert_dfll_dvco_reset);
> * Deassert the reset line of the DFLL's DVCO, allowing the DVCO to *
> operate. No return value.
> */
> -void tegra114_clock_deassert_dfll_dvco_reset(void)
> +static void tegra114_clock_deassert_dfll_dvco_reset(void)
> {
> u32 v;
>
> @@ -1286,7 +1286,26 @@ void tegra114_clock_deassert_dfll_dvco_reset(void)
> writel_relaxed(v, clk_base + RST_DFLL_DVCO);
> tegra114_car_barrier();
> }
> -EXPORT_SYMBOL(tegra114_clock_deassert_dfll_dvco_reset);
> +
> +static int tegra114_reset_assert(unsigned long id)
> +{
> + if (id == TEGRA114_RST_DFLL_DVCO)
> + tegra114_clock_assert_dfll_dvco_reset();
> + else
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> +static int tegra114_reset_deassert(unsigned long id)
> +{
> + if (id == TEGRA114_RST_DFLL_DVCO)
> + tegra114_clock_deassert_dfll_dvco_reset();
> + else
> + return -EINVAL;
> +
> + return 0;
> +}
>
> #ifdef CONFIG_TEGRA124_CLK_EMC
> static struct clk *tegra114_clk_src_onecell_get(struct of_phandle_args
> *clkspec, @@ -1357,6 +1376,9 @@ static void __init
> tegra114_clock_init(struct device_node *np)
> tegra_super_clk_gen4_init(clk_base, pmc_base, tegra114_clks,
> &pll_x_params);
>
> + tegra_init_special_resets(1, tegra114_reset_assert,
> + tegra114_reset_deassert);
> +
> #ifdef CONFIG_TEGRA124_CLK_EMC
> tegra_add_of_provider(np, tegra114_clk_src_onecell_get);
> clks[TEGRA114_CLK_EMC] = tegra124_clk_register_emc(clk_base, np,
Could you split this up into separate patches for the reset portion and the
DFLL portion.
> diff --git a/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c
> b/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c index
> 0251618b82c8..7a43380ce519 100644
> --- a/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c
> +++ b/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c
> @@ -28,6 +28,99 @@ struct dfll_fcpu_data {
> unsigned int cpu_cvb_tables_size;
> };
>
> +/* Maximum CPU frequency, indexed by CPU speedo id */
> +static const unsigned long tegra114_cpu_max_freq_table[] = {
> + [0] = 2040000000UL,
> + [1] = 1810500000UL,
> + [2] = 1912500000UL,
> + [3] = 1810500000UL,
> +};
> +
> +#define T114_CPU_CVB_TABLE \
> + .min_millivolts = 1000, \
> + .max_millivolts = 1320, \
> + .speedo_scale = 100, \
> + .voltage_scale = 1000, \
> + .entries = { \
> + { 306000000UL, { 2190643, -141851, 3576 } }, \
> + { 408000000UL, { 2250968, -144331, 3576 } }, \
> + { 510000000UL, { 2313333, -146811, 3576 } }, \
> + { 612000000UL, { 2377738, -149291, 3576 } }, \
> + { 714000000UL, { 2444183, -151771, 3576 } }, \
> + { 816000000UL, { 2512669, -154251, 3576 } }, \
> + { 918000000UL, { 2583194, -156731, 3576 } }, \
> + { 1020000000UL, { 2655759, -159211, 3576 } }, \
> + { 1122000000UL, { 2730365, -161691, 3576 } }, \
> + { 1224000000UL, { 2807010, -164171, 3576 } }, \
> + { 1326000000UL, { 2885696, -166651, 3576 } }, \
> + { 1428000000UL, { 2966422, -169131, 3576 } }, \
> + { 1530000000UL, { 3049183, -171601, 3576 } }, \
> + { 1606500000UL, { 3112179, -173451, 3576 } }, \
> + { 1708500000UL, { 3198504, -175931, 3576 } }, \
> + { 1810500000UL, { 3304747, -179126, 3576 } }, \
> + { 1912500000UL, { 3395401, -181606, 3576 } }, \
> + { 0UL, { 0, 0, 0 } }, \
> + }, \
> + .cpu_dfll_data = { \
> + .tune0_low = 0x00b0039d, \
> + .tune0_high = 0x00b0009d, \
> + .tune1 = 0x0000001f, \
> + .tune_high_min_millivolts = 1050, \
> + }
> +
Looks good -- could you add a T210_ prefix into the existing CVB table macro
names to avoid any confusion later.
> +static const struct cvb_table tegra114_cpu_cvb_tables[] = {
> + {
> + .speedo_id = 0,
> + .process_id = -1,
> + .min_millivolts = 1000,
> + .max_millivolts = 1250,
> + .speedo_scale = 100,
> + .voltage_scale = 100,
> + .entries = {
> + { 306000000UL, { 107330, -1569, 0 } },
> + { 408000000UL, { 111250, -1666, 0 } },
> + { 510000000UL, { 110000, -1460, 0 } },
> + { 612000000UL, { 117290, -1745, 0 } },
> + { 714000000UL, { 122700, -1910, 0 } },
> + { 816000000UL, { 125620, -1945, 0 } },
> + { 918000000UL, { 130560, -2076, 0 } },
> + { 1020000000UL, { 137280, -2303, 0 } },
> + { 1122000000UL, { 146440, -2660, 0 } },
> + { 1224000000UL, { 152190, -2825, 0 } },
> + { 1326000000UL, { 157520, -2953, 0 } },
> + { 1428000000UL, { 166100, -3261, 0 } },
> + { 1530000000UL, { 176410, -3647, 0 } },
> + { 1632000000UL, { 189620, -4186, 0 } },
> + { 1734000000UL, { 203190, -4725, 0 } },
> + { 1836000000UL, { 222670, -5573, 0 } },
> + { 1938000000UL, { 256210, -7165, 0 } },
> + { 2040000000UL, { 250050, -6544, 0 } },
> + { 0UL, { 0, 0, 0 } },
> + },
> + .cpu_dfll_data = {
> + .tune0_low = 0x00b0019d,
> + .tune0_high = 0x00b0019d,
> + .tune1 = 0x0000001f,
> + .tune_high_min_millivolts = 1000,
> + }
> + },
> + {
> + .speedo_id = 1,
> + .process_id = -1,
> + T114_CPU_CVB_TABLE
> + },
> + {
> + .speedo_id = 2,
> + .process_id = -1,
> + T114_CPU_CVB_TABLE
> + },
> + {
> + .speedo_id = 3,
> + .process_id = -1,
> + T114_CPU_CVB_TABLE
> + },
> +};
> +
> /* Maximum CPU frequency, indexed by CPU speedo id */
> static const unsigned long tegra124_cpu_max_freq_table[] = {
> [0] = 2014500000UL,
> @@ -494,6 +587,13 @@ static struct cvb_table tegra210_cpu_cvb_tables[] = {
> },
> };
>
> +static const struct dfll_fcpu_data tegra114_dfll_fcpu_data = {
> + .cpu_max_freq_table = tegra114_cpu_max_freq_table,
> + .cpu_max_freq_table_size = ARRAY_SIZE(tegra114_cpu_max_freq_table),
> + .cpu_cvb_tables = tegra114_cpu_cvb_tables,
> + .cpu_cvb_tables_size = ARRAY_SIZE(tegra114_cpu_cvb_tables)
> +};
> +
> static const struct dfll_fcpu_data tegra124_dfll_fcpu_data = {
> .cpu_max_freq_table = tegra124_cpu_max_freq_table,
> .cpu_max_freq_table_size = ARRAY_SIZE(tegra124_cpu_max_freq_table),
> @@ -509,6 +609,10 @@ static const struct dfll_fcpu_data
> tegra210_dfll_fcpu_data = { };
>
> static const struct of_device_id tegra124_dfll_fcpu_of_match[] = {
> + {
> + .compatible = "nvidia,tegra114-dfll",
> + .data = &tegra114_dfll_fcpu_data,
> + },
> {
> .compatible = "nvidia,tegra124-dfll",
> .data = &tegra124_dfll_fcpu_data,
> diff --git a/drivers/clk/tegra/clk.h b/drivers/clk/tegra/clk.h
> index 5d80d8b79b8e..58e860b18e5e 100644
> --- a/drivers/clk/tegra/clk.h
> +++ b/drivers/clk/tegra/clk.h
> @@ -898,8 +898,6 @@ static inline bool
> tegra124_clk_emc_driver_available(struct clk_hw *emc_hw) void
> tegra114_clock_tune_cpu_trimmers_high(void);
> void tegra114_clock_tune_cpu_trimmers_low(void);
> void tegra114_clock_tune_cpu_trimmers_init(void);
> -void tegra114_clock_assert_dfll_dvco_reset(void);
> -void tegra114_clock_deassert_dfll_dvco_reset(void);
>
> typedef void (*tegra_clk_apply_init_table_func)(void);
> extern tegra_clk_apply_init_table_func tegra_clk_apply_init_table;
> diff --git a/include/dt-bindings/reset/tegra114-car.h
> b/include/dt-bindings/reset/tegra114-car.h new file mode 100644
> index 000000000000..d7908d810ddf
> --- /dev/null
> +++ b/include/dt-bindings/reset/tegra114-car.h
> @@ -0,0 +1,13 @@
> +/* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause */
> +/*
> + * This header provides Tegra114-specific constants for binding
> + * nvidia,tegra114-car.
> + */
> +
> +#ifndef _DT_BINDINGS_RESET_TEGRA114_CAR_H
> +#define _DT_BINDINGS_RESET_TEGRA114_CAR_H
> +
> +#define TEGRA114_RESET(x) (5 * 32 + (x))
> +#define TEGRA114_RST_DFLL_DVCO TEGRA114_RESET(0)
> +
> +#endif /* _DT_BINDINGS_RESET_TEGRA114_CAR_H */
Bindings look fine to me, they follow existing pattern used on other chips for
DFLL. Perhaps add a note to the commit message along the lines of 'Binding
values for special resets are placed starting from software-defined index 160
in line with other chips.', for extra clarity.
Thanks,
Mikko
Powered by blists - more mailing lists