Extend the Tegra124 driver to include DFLL configuration settings required
for Tegra114 compatibility.
Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.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,
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, \
+ }
+
+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 */
--
2.43.0On 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@gmail.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
Quoting Svyatoslav Ryhel (2025-03-21 02:55:55) > Extend the Tegra124 driver to include DFLL configuration settings required > for Tegra114 compatibility. > > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com> > --- Drive by nitpick. The subject should drop "drivers: " because it's implicit from "clk:".
вт, 25 бер. 2025 р. о 20:56 Stephen Boyd <sboyd@kernel.org> пише: > > Quoting Svyatoslav Ryhel (2025-03-21 02:55:55) > > Extend the Tegra124 driver to include DFLL configuration settings required > > for Tegra114 compatibility. > > > > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com> > > --- > > Drive by nitpick. The subject should drop "drivers: " because it's > implicit from "clk:". If this is the only remark you have to this commit, I am happy to fix it ;)
On 21/03/2025 10:55, Svyatoslav Ryhel wrote: > Extend the Tegra124 driver to include DFLL configuration settings required > for Tegra114 compatibility. > > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com> <form letter> Please use scripts/get_maintainers.pl to get a list of necessary people and lists to CC (and consider --no-git-fallback argument, so you will not CC people just because they made one commit years ago). It might happen, that command when run on an older kernel, gives you outdated entries. Therefore please be sure you base your patches on recent Linux kernel. Tools like b4 or scripts/get_maintainer.pl provide you proper list of people, so fix your workflow. Tools might also fail if you work on some ancient tree (don't, instead use mainline) or work on fork of kernel (don't, instead use mainline). Just use b4 and everything should be fine, although remember about `b4 prep --auto-to-cc` if you added new patches to the patchset. </form letter> > +++ b/include/dt-bindings/reset/tegra114-car.h Filename based on compatible. > @@ -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)) Does not look like a binding, but some sort of register. Binding IDs start from 0 (or 1) and are incremented by 1. Best regards, Krzysztof
On Fri, Mar 21, 2025 at 09:50:09PM +0100, Krzysztof Kozlowski wrote: > On 21/03/2025 10:55, Svyatoslav Ryhel wrote: > > Extend the Tegra124 driver to include DFLL configuration settings required > > for Tegra114 compatibility. > > > > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com> > > <form letter> > Please use scripts/get_maintainers.pl to get a list of necessary people > and lists to CC (and consider --no-git-fallback argument, so you will > not CC people just because they made one commit years ago). It might > happen, that command when run on an older kernel, gives you outdated > entries. Therefore please be sure you base your patches on recent Linux > kernel. > > Tools like b4 or scripts/get_maintainer.pl provide you proper list of > people, so fix your workflow. Tools might also fail if you work on some > ancient tree (don't, instead use mainline) or work on fork of kernel > (don't, instead use mainline). Just use b4 and everything should be > fine, although remember about `b4 prep --auto-to-cc` if you added new > patches to the patchset. > </form letter> > > > > > +++ b/include/dt-bindings/reset/tegra114-car.h > > Filename based on compatible. > > > @@ -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)) > > > Does not look like a binding, but some sort of register. Binding IDs > start from 0 (or 1) and are incremented by 1. I'll try and clear up some of the confusion around this. The way that resets are handled on these Tegra devices is that there is a set of peripheral clocks & resets which are paired up. This is because they are laid out in banks within the CAR (clock and reset) controller. In most cases we're referring to those resets, so you'll often see a clock ID used in conjection with the same reset ID for a given IP block. In addition to those peripheral resets, there are a number of extra resets that don't have a corresponding clock and which are exposed in registers outside of the peripheral banks, but still part of the CAR. To support those "special" registers, the TEGRA*_RESET() is used to denote resets outside of the regular peripheral resets. Essentially it defines the offset within the CAR at which special resets start. In the above case, Tegra114 has 5 banks with 32 peripheral resets each. The first special reset, TEGRA114_RESET(0), therefore gets ID 5 * 32 + 0. So to summarize: We cannot start enumerating these at 0 because that would fall into the range of peripheral reset IDs. Thierry
On 10/06/2025 13:07, Thierry Reding wrote: >> >>> @@ -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)) >> >> >> Does not look like a binding, but some sort of register. Binding IDs >> start from 0 (or 1) and are incremented by 1. > > I'll try and clear up some of the confusion around this. The way that > resets are handled on these Tegra devices is that there is a set of > peripheral clocks & resets which are paired up. This is because they > are laid out in banks within the CAR (clock and reset) controller. In > most cases we're referring to those resets, so you'll often see a clock > ID used in conjection with the same reset ID for a given IP block. > > In addition to those peripheral resets, there are a number of extra > resets that don't have a corresponding clock and which are exposed in > registers outside of the peripheral banks, but still part of the CAR. > To support those "special" registers, the TEGRA*_RESET() is used to > denote resets outside of the regular peripheral resets. Essentially it > defines the offset within the CAR at which special resets start. In the > above case, Tegra114 has 5 banks with 32 peripheral resets each. The > first special reset, TEGRA114_RESET(0), therefore gets ID 5 * 32 + 0. > > So to summarize: We cannot start enumerating these at 0 because that > would fall into the range of peripheral reset IDs. So these are hardware values, not bindings. Drop the header or move it outside of bindings like other headers for hardware constants. Best regards, Krzysztof
On Wed, Jun 18, 2025 at 11:13:37AM +0200, Krzysztof Kozlowski wrote: > On 10/06/2025 13:07, Thierry Reding wrote: > >> > >>> @@ -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)) > >> > >> > >> Does not look like a binding, but some sort of register. Binding IDs > >> start from 0 (or 1) and are incremented by 1. > > > > I'll try and clear up some of the confusion around this. The way that > > resets are handled on these Tegra devices is that there is a set of > > peripheral clocks & resets which are paired up. This is because they > > are laid out in banks within the CAR (clock and reset) controller. In > > most cases we're referring to those resets, so you'll often see a clock > > ID used in conjection with the same reset ID for a given IP block. > > > > In addition to those peripheral resets, there are a number of extra > > resets that don't have a corresponding clock and which are exposed in > > registers outside of the peripheral banks, but still part of the CAR. > > To support those "special" registers, the TEGRA*_RESET() is used to > > denote resets outside of the regular peripheral resets. Essentially it > > defines the offset within the CAR at which special resets start. In the > > above case, Tegra114 has 5 banks with 32 peripheral resets each. The > > first special reset, TEGRA114_RESET(0), therefore gets ID 5 * 32 + 0. > > > > So to summarize: We cannot start enumerating these at 0 because that > > would fall into the range of peripheral reset IDs. > > So these are hardware values, not bindings. Drop the header or move it > outside of bindings like other headers for hardware constants. 5 banks and 32 peripheral resets per bank are properties of the hardware, yes. However, the notion of starting the enumeration of the extra resets after those 160 resets is a binding. There's no concept in the chip that would tie the DFLL reset to index 160. Dropping the header altogether would mean that we need to hardcode the value, which makes its meaning completely opaque. Besides, there are a bunch of header files in include/dt-bindings that define symbolic names for hardware values, and I'm not sure why you think these here would be different. Thierry
пт, 21 бер. 2025 р. о 22:50 Krzysztof Kozlowski <krzk@kernel.org> пише: > > On 21/03/2025 10:55, Svyatoslav Ryhel wrote: > > Extend the Tegra124 driver to include DFLL configuration settings required > > for Tegra114 compatibility. > > > > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com> > > <form letter> > Please use scripts/get_maintainers.pl to get a list of necessary people > and lists to CC (and consider --no-git-fallback argument, so you will > not CC people just because they made one commit years ago). It might > happen, that command when run on an older kernel, gives you outdated > entries. Therefore please be sure you base your patches on recent Linux > kernel. > > Tools like b4 or scripts/get_maintainer.pl provide you proper list of > people, so fix your workflow. Tools might also fail if you work on some > ancient tree (don't, instead use mainline) or work on fork of kernel > (don't, instead use mainline). Just use b4 and everything should be > fine, although remember about `b4 prep --auto-to-cc` if you added new > patches to the patchset. > </form letter> > > > > > +++ b/include/dt-bindings/reset/tegra114-car.h > > Filename based on compatible. > > > @@ -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)) > > > Does not look like a binding, but some sort of register. Binding IDs > start from 0 (or 1) and are incremented by 1. > Hello there! This file add same logic for Tegra114 as Tegra124 currently implements, check here https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/include/dt-bindings/reset/tegra124-car.h?h=v6.14.5 I did not re-use Tegra124 value, though it is same, to avoid confusion in main Tegra114 device tree. > Best regards, > Krzysztof
On 03/05/2025 10:54, Svyatoslav Ryhel wrote: >>> +/* 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)) >> >> >> Does not look like a binding, but some sort of register. Binding IDs >> start from 0 (or 1) and are incremented by 1. >> > > Hello there! > This file add same logic for Tegra114 as Tegra124 currently > implements, check here > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/include/dt-bindings/reset/tegra124-car.h?h=v6.14.5 > > I did not re-use Tegra124 value, though it is same, to avoid confusion > in main Tegra114 device tree. What confusion? Why would anyone be interested in comparing numbers thus getting confused by different number? These are abstract IDs. Best regards, Krzysztof
нд, 4 трав. 2025 р. о 19:23 Krzysztof Kozlowski <krzk@kernel.org> пише: > > On 03/05/2025 10:54, Svyatoslav Ryhel wrote: > >>> +/* 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)) > >> > >> > >> Does not look like a binding, but some sort of register. Binding IDs > >> start from 0 (or 1) and are incremented by 1. > >> > > > > Hello there! > > This file add same logic for Tegra114 as Tegra124 currently > > implements, check here > > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/include/dt-bindings/reset/tegra124-car.h?h=v6.14.5 > > > > I did not re-use Tegra124 value, though it is same, to avoid confusion > > in main Tegra114 device tree. > > What confusion? Why would anyone be interested in comparing numbers thus > getting confused by different number? These are abstract IDs. > By using TEGRA124_RESET in Tegra114 device tree > Best regards, > Krzysztof
On 04/05/2025 18:25, Svyatoslav Ryhel wrote: > нд, 4 трав. 2025 р. о 19:23 Krzysztof Kozlowski <krzk@kernel.org> пише: >> >> On 03/05/2025 10:54, Svyatoslav Ryhel wrote: >>>>> +/* 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)) >>>> >>>> >>>> Does not look like a binding, but some sort of register. Binding IDs >>>> start from 0 (or 1) and are incremented by 1. >>>> >>> >>> Hello there! >>> This file add same logic for Tegra114 as Tegra124 currently >>> implements, check here >>> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/include/dt-bindings/reset/tegra124-car.h?h=v6.14.5 >>> >>> I did not re-use Tegra124 value, though it is same, to avoid confusion >>> in main Tegra114 device tree. >> >> What confusion? Why would anyone be interested in comparing numbers thus >> getting confused by different number? These are abstract IDs. >> > > By using TEGRA124_RESET in Tegra114 device tree Why would you use define from other SoC... and how is it related to my comment in the first place? Best regards, Krzysztof
нд, 4 трав. 2025 р. о 20:11 Krzysztof Kozlowski <krzk@kernel.org> пише: > > On 04/05/2025 18:25, Svyatoslav Ryhel wrote: > > нд, 4 трав. 2025 р. о 19:23 Krzysztof Kozlowski <krzk@kernel.org> пише: > >> > >> On 03/05/2025 10:54, Svyatoslav Ryhel wrote: > >>>>> +/* 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)) > >>>> > >>>> > >>>> Does not look like a binding, but some sort of register. Binding IDs > >>>> start from 0 (or 1) and are incremented by 1. > >>>> > >>> > >>> Hello there! > >>> This file add same logic for Tegra114 as Tegra124 currently > >>> implements, check here > >>> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/include/dt-bindings/reset/tegra124-car.h?h=v6.14.5 > >>> > >>> I did not re-use Tegra124 value, though it is same, to avoid confusion > >>> in main Tegra114 device tree. > >> > >> What confusion? Why would anyone be interested in comparing numbers thus > >> getting confused by different number? These are abstract IDs. > >> > > > > By using TEGRA124_RESET in Tegra114 device tree > > Why would you use define from other SoC... and how is it related to my > comment in the first place? > You did not even bother to check link that I have provided, did you? You cut the actual device tree compatible definition, TEGRA114_RESET(x) is a macro used further to define device tree compatibles. Like this: #define TEGRA114_RST_DFLL_DVCO TEGRA114_RESET(0) > > Best regards, > Krzysztof
On 04/05/2025 19:30, Svyatoslav Ryhel wrote: > нд, 4 трав. 2025 р. о 20:11 Krzysztof Kozlowski <krzk@kernel.org> пише: >> >> On 04/05/2025 18:25, Svyatoslav Ryhel wrote: >>> нд, 4 трав. 2025 р. о 19:23 Krzysztof Kozlowski <krzk@kernel.org> пише: >>>> >>>> On 03/05/2025 10:54, Svyatoslav Ryhel wrote: >>>>>>> +/* 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)) >>>>>> >>>>>> >>>>>> Does not look like a binding, but some sort of register. Binding IDs >>>>>> start from 0 (or 1) and are incremented by 1. >>>>>> >>>>> >>>>> Hello there! >>>>> This file add same logic for Tegra114 as Tegra124 currently >>>>> implements, check here >>>>> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/include/dt-bindings/reset/tegra124-car.h?h=v6.14.5 >>>>> >>>>> I did not re-use Tegra124 value, though it is same, to avoid confusion >>>>> in main Tegra114 device tree. >>>> >>>> What confusion? Why would anyone be interested in comparing numbers thus >>>> getting confused by different number? These are abstract IDs. >>>> >>> >>> By using TEGRA124_RESET in Tegra114 device tree >> >> Why would you use define from other SoC... and how is it related to my >> comment in the first place? >> > > You did not even bother to check link that I have provided, did you? I did. I clicked it, looked the same as here. Does not help. > > You cut the actual device tree compatible definition, > TEGRA114_RESET(x) is a macro used further to define device tree > compatibles. There are no further users... > > Like this: > > #define TEGRA114_RST_DFLL_DVCO TEGRA114_RESET(0) and still my comment stands. Bindings start from 0 or 1. Best regards, Krzysztof
© 2016 - 2025 Red Hat, Inc.