[PATCH v1 0/5] CPUFREQ OPP's and Tegra30 support by tegra20-cpufreq driver

[PATCH v1 0/5] CPUFREQ OPP's and Tegra30 support by tegra20-cpufreq driver

[Dmitry Osipenko]

Hello,

This series adds support for CPU frequency scaling on Tegra30 and device
tree support that allows to implement thermal throttling and customize
available CPU frequencies per-board. The tegra20-cpufreq driver has been
re-worked to support that all.

Note that Tegra30 support not strictly depends on the clock patches that
are under review now, CPUFREQ driver will fail to probe until CCLKG clock
will get exposed by the clock driver. Hence this series can be applied
independently of the clock patches, CPUFREQ will start to work on Tegra30
once both patchsets will be applied.

Dmitry Osipenko (5):
  dt-bindings: cpufreq: Add binding for NVIDIA Tegra20/30
  cpufreq: tegra20: Support OPP, thermal cooling and Tegra30
  ARM: tegra: Create tegra20-cpufreq device on Tegra30
  ARM: dts: tegra20: Add CPU Operating Performance Points
  ARM: dts: tegra30: Add CPU Operating Performance Points

 .../cpufreq/nvidia,tegra20-cpufreq.txt        |  38 ++
 arch/arm/boot/dts/tegra20.dtsi                |  58 +++
 arch/arm/boot/dts/tegra30.dtsi                |  65 ++++
 arch/arm/mach-tegra/tegra.c                   |   4 +
 drivers/cpufreq/Kconfig.arm                   |   2 +
 drivers/cpufreq/cpufreq-dt-platdev.c          |   2 +
 drivers/cpufreq/tegra20-cpufreq.c             | 334 +++++++++++++-----
 7 files changed, 422 insertions(+), 81 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/cpufreq/nvidia,tegra20-cpufreq.txt

-- 
2.18.0

[PATCH v1 1/5] dt-bindings: cpufreq: Add binding for NVIDIA Tegra20/30

[Dmitry Osipenko]

Add device-tree binding that describes CPU frequency-scaling hardware
found on NVIDIA Tegra20/30 SoC's.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 .../cpufreq/nvidia,tegra20-cpufreq.txt        | 38 +++++++++++++++++++
 1 file changed, 38 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/cpufreq/nvidia,tegra20-cpufreq.txt

diff --git a/Documentation/devicetree/bindings/cpufreq/nvidia,tegra20-cpufreq.txt b/Documentation/devicetree/bindings/cpufreq/nvidia,tegra20-cpufreq.txt
new file mode 100644
index 000000000000..2c51f676e958
--- /dev/null
+++ b/Documentation/devicetree/bindings/cpufreq/nvidia,tegra20-cpufreq.txt
@@ -0,0 +1,38 @@
+Binding for NVIDIA Tegra20 CPUFreq
+==================================
+
+Required properties:
+- clocks: Must contain an entry for each entry in clock-names.
+  See ../clocks/clock-bindings.txt for details.
+- clock-names: Must include the following entries:
+  - pll_x: main-parent for CPU clock, must be the first entry
+  - backup: intermediate-parent for CPU clock
+  - cpu: the CPU clock
+- operating-points-v2: See ../bindings/opp/opp.txt for details.
+- #cooling-cells: Should be 2. See ../thermal/thermal.txt for details.
+
+Example:
+	cpu0_opp_table: opp_table0 {
+		compatible = "operating-points-v2";
+
+		opp@216000000 {
+			clock-latency-ns = <300000>;
+			opp-hz = /bits/ 64 <216000000>;
+			opp-microvolt = <7500000>;
+			opp-suspend;
+		};
+
+		...
+	};
+
+	cpus {
+		cpu@0 {
+			compatible = "arm,cortex-a9";
+			clocks = <&tegra_car TEGRA20_CLK_PLL_X>,
+				 <&tegra_car TEGRA20_CLK_PLL_P>,
+				 <&tegra_car TEGRA20_CLK_CCLK>;
+			clock-names = "pll_x", "backup", "cpu";
+			operating-points-v2 = <&cpu0_opp_table>;
+			#cooling-cells = <2>;
+		};
+	};
-- 
2.18.0

[PATCH v1 2/5] cpufreq: tegra20: Support OPP, thermal cooling and Tegra30

[Dmitry Osipenko]

Add support for thermal throttling and Operating Performance Points.
Driver now relies on OPP's supplied via device tree and therefore will
work only on devices that use the updated device tree. The generalization
of the driver allows to transparently support Tegra30.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/cpufreq/Kconfig.arm          |   2 +
 drivers/cpufreq/cpufreq-dt-platdev.c |   2 +
 drivers/cpufreq/tegra20-cpufreq.c    | 334 ++++++++++++++++++++-------
 3 files changed, 257 insertions(+), 81 deletions(-)

diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
index 0cd8eb76ad59..78795d108f5e 100644
--- a/drivers/cpufreq/Kconfig.arm
+++ b/drivers/cpufreq/Kconfig.arm
@@ -262,7 +262,9 @@ config ARM_TANGO_CPUFREQ
 
 config ARM_TEGRA20_CPUFREQ
 	tristate "Tegra20 CPUFreq support"
+	depends on !CPU_THERMAL || THERMAL
 	depends on ARCH_TEGRA
+	select PM_OPP
 	default y
 	help
 	  This adds the CPUFreq driver support for Tegra20 SOCs.
diff --git a/drivers/cpufreq/cpufreq-dt-platdev.c b/drivers/cpufreq/cpufreq-dt-platdev.c
index fe14c57de6ca..3c4709159995 100644
--- a/drivers/cpufreq/cpufreq-dt-platdev.c
+++ b/drivers/cpufreq/cpufreq-dt-platdev.c
@@ -114,6 +114,8 @@ static const struct of_device_id blacklist[] __initconst = {
 	{ .compatible = "mediatek,mt8173", },
 	{ .compatible = "mediatek,mt8176", },
 
+	{ .compatible = "nvidia,tegra20", },
+	{ .compatible = "nvidia,tegra30", },
 	{ .compatible = "nvidia,tegra124", },
 
 	{ .compatible = "qcom,apq8096", },
diff --git a/drivers/cpufreq/tegra20-cpufreq.c b/drivers/cpufreq/tegra20-cpufreq.c
index 05f57dcd5215..fd6c40a64175 100644
--- a/drivers/cpufreq/tegra20-cpufreq.c
+++ b/drivers/cpufreq/tegra20-cpufreq.c
@@ -17,163 +17,347 @@
  */
 
 #include <linux/clk.h>
+#include <linux/cpu.h>
+#include <linux/cpu_cooling.h>
 #include <linux/cpufreq.h>
 #include <linux/err.h>
 #include <linux/init.h>
 #include <linux/module.h>
+#include <linux/of.h>
 #include <linux/platform_device.h>
+#include <linux/pm_opp.h>
 #include <linux/types.h>
 
-static struct cpufreq_frequency_table freq_table[] = {
-	{ .frequency = 216000 },
-	{ .frequency = 312000 },
-	{ .frequency = 456000 },
-	{ .frequency = 608000 },
-	{ .frequency = 760000 },
-	{ .frequency = 816000 },
-	{ .frequency = 912000 },
-	{ .frequency = 1000000 },
-	{ .frequency = CPUFREQ_TABLE_END },
-};
+#define PLLX_PREPARE		BIT(0)
+#define PLLX_PREPARED		BIT(1)
 
 struct tegra20_cpufreq {
 	struct device *dev;
+	struct device *cpu_dev;
+	struct cpumask cpu_mask;
 	struct cpufreq_driver driver;
+	struct thermal_cooling_device *cdev;
+	struct cpufreq_frequency_table *freq_table;
 	struct clk *cpu_clk;
 	struct clk *pll_x_clk;
-	struct clk *pll_p_clk;
-	bool pll_x_prepared;
+	struct clk *backup_clk;
+	unsigned long backup_rate;
+	unsigned int state;
 };
 
 static unsigned int tegra_get_intermediate(struct cpufreq_policy *policy,
 					   unsigned int index)
 {
 	struct tegra20_cpufreq *cpufreq = cpufreq_get_driver_data();
-	unsigned int ifreq = clk_get_rate(cpufreq->pll_p_clk) / 1000;
+	struct clk *cpu_parent = clk_get_parent(cpufreq->cpu_clk);
+	unsigned long new_rate = cpufreq->freq_table[index].frequency * 1000;
+	int err;
+
+	/*
+	 * Make sure that backup clock rate stays consistent during
+	 * transition by entering into critical section of the backup clock.
+	 */
+	err = clk_rate_exclusive_get(cpufreq->backup_clk);
+	/* this shouldn't fail */
+	WARN_ON_ONCE(err);
 
 	/*
-	 * Don't switch to intermediate freq if:
-	 * - we are already at it, i.e. policy->cur == ifreq
-	 * - index corresponds to ifreq
+	 * When target rate is equal to backup rate, we don't need to
+	 * switch to backup clock and so the intermediate routine isn't
+	 * called.  Also, we wouldn't be using PLLX anymore and must not
+	 * take extra reference to it, as it can be disabled to save some
+	 * power.
 	 */
-	if (freq_table[index].frequency == ifreq || policy->cur == ifreq)
+	cpufreq->backup_rate = clk_get_rate(cpufreq->backup_clk);
+
+	if (new_rate == cpufreq->backup_rate)
+		cpufreq->state &= ~PLLX_PREPARE;
+	else
+		cpufreq->state |= PLLX_PREPARE;
+
+	/* don't switch to intermediate freq if we are already at it */
+	if (clk_is_match(cpu_parent, cpufreq->backup_clk))
 		return 0;
 
-	return ifreq;
+	return cpufreq->backup_rate / 1000;
 }
 
 static int tegra_target_intermediate(struct cpufreq_policy *policy,
 				     unsigned int index)
 {
 	struct tegra20_cpufreq *cpufreq = cpufreq_get_driver_data();
-	int ret;
+	unsigned int state = cpufreq->state;
+	int err;
 
 	/*
-	 * Take an extra reference to the main pll so it doesn't turn
-	 * off when we move the cpu off of it as enabling it again while we
-	 * switch to it from tegra_target() would take additional time.
-	 *
-	 * When target-freq is equal to intermediate freq we don't need to
-	 * switch to an intermediate freq and so this routine isn't called.
-	 * Also, we wouldn't be using pll_x anymore and must not take extra
-	 * reference to it, as it can be disabled now to save some power.
+	 * Take an extra reference to the main PLLX so it doesn't turn off
+	 * when we move the CPU clock to backup clock as enabling it again
+	 * while we switch to it from tegra_target() would take additional
+	 * time.
 	 */
-	clk_prepare_enable(cpufreq->pll_x_clk);
+	if ((state & (PLLX_PREPARED | PLLX_PREPARE)) == PLLX_PREPARE) {
+		err = clk_prepare_enable(cpufreq->pll_x_clk);
+		if (err)
+			goto err_exclusive_put;
 
-	ret = clk_set_parent(cpufreq->cpu_clk, cpufreq->pll_p_clk);
-	if (ret)
+		cpufreq->state |= PLLX_PREPARED;
+	}
+
+	err = clk_set_parent(cpufreq->cpu_clk, cpufreq->backup_clk);
+	if (err)
+		goto err_exclusive_put;
+
+	return 0;
+
+err_exclusive_put:
+	clk_rate_exclusive_put(cpufreq->backup_clk);
+
+	if (cpufreq->state & PLLX_PREPARED) {
 		clk_disable_unprepare(cpufreq->pll_x_clk);
-	else
-		cpufreq->pll_x_prepared = true;
+		cpufreq->state &= ~PLLX_PREPARED;
+	}
 
-	return ret;
+	/* this shouldn't fail */
+	return WARN_ON_ONCE(err);
 }
 
 static int tegra_target(struct cpufreq_policy *policy, unsigned int index)
 {
 	struct tegra20_cpufreq *cpufreq = cpufreq_get_driver_data();
-	unsigned long rate = freq_table[index].frequency;
-	unsigned int ifreq = clk_get_rate(cpufreq->pll_p_clk) / 1000;
+	unsigned long new_rate = cpufreq->freq_table[index].frequency * 1000;
+	unsigned int state = cpufreq->state;
 	int ret;
 
 	/*
-	 * target freq == pll_p, don't need to take extra reference to pll_x_clk
-	 * as it isn't used anymore.
+	 * Drop refcount to PLLX only if we switched to backup clock earlier
+	 * during transitioning to a target frequency and we are going to
+	 * stay with the backup clock.
 	 */
-	if (rate == ifreq)
-		return clk_set_parent(cpufreq->cpu_clk, cpufreq->pll_p_clk);
+	if ((state & (PLLX_PREPARED | PLLX_PREPARE)) == PLLX_PREPARED) {
+		clk_disable_unprepare(cpufreq->pll_x_clk);
+		state &= ~PLLX_PREPARED;
+	}
 
-	ret = clk_set_rate(cpufreq->pll_x_clk, rate * 1000);
-	/* Restore to earlier frequency on error, i.e. pll_x */
+	/*
+	 * Switch to new OPP, note that this will change PLLX rate and
+	 * not the CCLK.
+	 */
+	ret = dev_pm_opp_set_rate(cpufreq->cpu_dev, new_rate);
 	if (ret)
-		dev_err(cpufreq->dev, "Failed to change pll_x to %lu\n", rate);
+		goto exclusive_put;
+
+	/*
+	 * Target rate == backup rate leaves PLLX turned off, CPU is kept
+	 * running off the backup clock. This should save us some power by
+	 * keeping one more PLL disabled because the backup clock assumed
+	 * to be always-on. In this case PLLX_PREPARE flag will be omitted.
+	 */
+	if (state & PLLX_PREPARE) {
+		/*
+		 * CCF doesn't return error if clock-enabling fails on
+		 * re-parent, hence enable it now.
+		 */
+		ret = clk_prepare_enable(cpufreq->pll_x_clk);
+		if (ret)
+			goto exclusive_put;
+
+		ret = clk_set_parent(cpufreq->cpu_clk, cpufreq->pll_x_clk);
 
-	ret = clk_set_parent(cpufreq->cpu_clk, cpufreq->pll_x_clk);
-	/* This shouldn't fail while changing or restoring */
-	WARN_ON(ret);
+		clk_disable_unprepare(cpufreq->pll_x_clk);
+	}
 
 	/*
-	 * Drop count to pll_x clock only if we switched to intermediate freq
-	 * earlier while transitioning to a target frequency.
+	 * Drop refcount to PLLX only if we switched to backup clock earlier
+	 * during transitioning to a target frequency.
 	 */
-	if (cpufreq->pll_x_prepared) {
+	if (state & PLLX_PREPARED) {
 		clk_disable_unprepare(cpufreq->pll_x_clk);
-		cpufreq->pll_x_prepared = false;
+		state &= ~PLLX_PREPARED;
 	}
 
+exclusive_put:
+	clk_rate_exclusive_put(cpufreq->backup_clk);
+
+	cpufreq->state = state;
+
+	/* this shouldn't fail */
+	return WARN_ON_ONCE(ret);
+}
+
+static int tegra_cpu_setup_opp(struct tegra20_cpufreq *cpufreq)
+{
+	struct device *dev = cpufreq->cpu_dev;
+	int err;
+
+	err = dev_pm_opp_of_cpumask_add_table(cpu_possible_mask);
+	if (err)
+		return err;
+
+	err = dev_pm_opp_init_cpufreq_table(dev, &cpufreq->freq_table);
+	if (err)
+		goto err_remove_table;
+
+	return 0;
+
+err_remove_table:
+	dev_pm_opp_of_cpumask_remove_table(cpu_possible_mask);
+
+	return err;
+}
+
+static void tegra_cpu_release_opp(struct tegra20_cpufreq *cpufreq)
+{
+	dev_pm_opp_free_cpufreq_table(cpufreq->cpu_dev, &cpufreq->freq_table);
+	dev_pm_opp_of_cpumask_remove_table(cpu_possible_mask);
+}
+
+static int tegra_cpu_init_clk(struct tegra20_cpufreq *cpufreq)
+{
+	unsigned long backup_rate;
+	int ret;
+
+	ret = clk_rate_exclusive_get(cpufreq->backup_clk);
+	if (ret)
+		return ret;
+
+	ret = clk_set_parent(cpufreq->cpu_clk, cpufreq->backup_clk);
+	if (ret)
+		goto exclusive_put;
+
+	backup_rate = clk_get_rate(cpufreq->backup_clk);
+
+	/*
+	 * The CCLK has its own clock divider, that divider isn't getting
+	 * disabled on clock reparent. Hence set CCLK parent to backup clock
+	 * in order to disable the divider if it happens to be enabled,
+	 * otherwise clk_set_rate() has no effect.
+	 */
+	ret = clk_set_rate(cpufreq->cpu_clk, backup_rate);
+
+exclusive_put:
+	clk_rate_exclusive_put(cpufreq->backup_clk);
+
 	return ret;
 }
 
 static int tegra_cpu_init(struct cpufreq_policy *policy)
 {
 	struct tegra20_cpufreq *cpufreq = cpufreq_get_driver_data();
-	int ret;
+	struct device *cpu = cpufreq->cpu_dev;
+	int err;
 
-	clk_prepare_enable(cpufreq->cpu_clk);
+	err = tegra_cpu_setup_opp(cpufreq);
+	if (err) {
+		dev_err(cpufreq->dev, "Failed to setup OPP: %d\n", err);
+		return err;
+	}
 
-	/* FIXME: what's the actual transition time? */
-	ret = cpufreq_generic_init(policy, freq_table, 300 * 1000);
-	if (ret) {
-		clk_disable_unprepare(cpufreq->cpu_clk);
-		return ret;
+	err = clk_prepare_enable(cpufreq->cpu_clk);
+	if (err) {
+		dev_err(cpufreq->dev,
+			"Failed to enable CPU clock: %d\n", err);
+		goto er_release_opp;
 	}
 
+	err = clk_prepare_enable(cpufreq->backup_clk);
+	if (err) {
+		dev_err(cpufreq->dev,
+			"Failed to enable backup clock: %d\n", err);
+		goto err_cpu_disable;
+	}
+
+	err = clk_rate_exclusive_get(cpufreq->cpu_clk);
+	if (err) {
+		dev_err(cpufreq->dev,
+			"Failed to make CPU clock exclusive: %d\n", err);
+		goto err_backup_disable;
+	}
+
+	err = tegra_cpu_init_clk(cpufreq);
+	if (err) {
+		dev_err(cpufreq->dev,
+			"Failed to initialize CPU clock: %d\n", err);
+		goto err_exclusive_put;
+	}
+
+	err = cpufreq_generic_init(policy, cpufreq->freq_table,
+				   dev_pm_opp_get_max_transition_latency(cpu));
+	if (err)
+		goto err_exclusive_put;
+
 	policy->clk = cpufreq->cpu_clk;
-	policy->suspend_freq = freq_table[0].frequency;
+	policy->suspend_freq = dev_pm_opp_get_suspend_opp_freq(cpu) / 1000;
+
 	return 0;
+
+err_exclusive_put:
+	clk_rate_exclusive_put(cpufreq->cpu_clk);
+err_backup_disable:
+	clk_disable_unprepare(cpufreq->backup_clk);
+err_cpu_disable:
+	clk_disable_unprepare(cpufreq->cpu_clk);
+er_release_opp:
+	tegra_cpu_release_opp(cpufreq);
+
+	return err;
 }
 
 static int tegra_cpu_exit(struct cpufreq_policy *policy)
 {
 	struct tegra20_cpufreq *cpufreq = cpufreq_get_driver_data();
 
+	cpufreq_cooling_unregister(cpufreq->cdev);
+	clk_rate_exclusive_put(cpufreq->cpu_clk);
+	clk_disable_unprepare(cpufreq->backup_clk);
 	clk_disable_unprepare(cpufreq->cpu_clk);
+	tegra_cpu_release_opp(cpufreq);
+
 	return 0;
 }
 
+static void tegra_cpu_ready(struct cpufreq_policy *policy)
+{
+	struct tegra20_cpufreq *cpufreq = cpufreq_get_driver_data();
+
+	cpufreq->cdev = of_cpufreq_cooling_register(policy);
+}
+
 static int tegra20_cpufreq_probe(struct platform_device *pdev)
 {
 	struct tegra20_cpufreq *cpufreq;
+	struct device_node *np;
 	int err;
 
 	cpufreq = devm_kzalloc(&pdev->dev, sizeof(*cpufreq), GFP_KERNEL);
 	if (!cpufreq)
 		return -ENOMEM;
 
-	cpufreq->cpu_clk = clk_get_sys(NULL, "cclk");
-	if (IS_ERR(cpufreq->cpu_clk))
-		return PTR_ERR(cpufreq->cpu_clk);
+	cpufreq->cpu_dev = get_cpu_device(0);
+	if (!cpufreq->cpu_dev)
+		return -ENODEV;
+
+	np = cpufreq->cpu_dev->of_node;
 
-	cpufreq->pll_x_clk = clk_get_sys(NULL, "pll_x");
+	cpufreq->cpu_clk = devm_get_clk_from_child(&pdev->dev, np, "cpu");
+	if (IS_ERR(cpufreq->cpu_clk)) {
+		err = PTR_ERR(cpufreq->cpu_clk);
+		dev_err(&pdev->dev, "Failed to get cpu clock: %d\n", err);
+		dev_err(&pdev->dev, "Please update your device tree\n");
+		return err;
+	}
+
+	cpufreq->pll_x_clk = devm_get_clk_from_child(&pdev->dev, np, "pll_x");
 	if (IS_ERR(cpufreq->pll_x_clk)) {
 		err = PTR_ERR(cpufreq->pll_x_clk);
-		goto put_cpu;
+		dev_err(&pdev->dev, "Failed to get pll_x clock: %d\n", err);
+		return err;
 	}
 
-	cpufreq->pll_p_clk = clk_get_sys(NULL, "pll_p");
-	if (IS_ERR(cpufreq->pll_p_clk)) {
-		err = PTR_ERR(cpufreq->pll_p_clk);
-		goto put_pll_x;
+	cpufreq->backup_clk = devm_get_clk_from_child(&pdev->dev, np, "backup");
+	if (IS_ERR(cpufreq->backup_clk)) {
+		err = PTR_ERR(cpufreq->backup_clk);
+		dev_err(&pdev->dev, "Failed to get backup clock: %d\n", err);
+		return err;
 	}
 
 	cpufreq->dev = &pdev->dev;
@@ -181,6 +365,7 @@ static int tegra20_cpufreq_probe(struct platform_device *pdev)
 	cpufreq->driver.attr = cpufreq_generic_attr;
 	cpufreq->driver.init = tegra_cpu_init;
 	cpufreq->driver.exit = tegra_cpu_exit;
+	cpufreq->driver.ready = tegra_cpu_ready;
 	cpufreq->driver.flags = CPUFREQ_NEED_INITIAL_FREQ_CHECK;
 	cpufreq->driver.verify = cpufreq_generic_frequency_table_verify;
 	cpufreq->driver.suspend = cpufreq_generic_suspend;
@@ -192,20 +377,11 @@ static int tegra20_cpufreq_probe(struct platform_device *pdev)
 
 	err = cpufreq_register_driver(&cpufreq->driver);
 	if (err)
-		goto put_pll_p;
+		return err;
 
 	platform_set_drvdata(pdev, cpufreq);
 
 	return 0;
-
-put_pll_p:
-	clk_put(cpufreq->pll_p_clk);
-put_pll_x:
-	clk_put(cpufreq->pll_x_clk);
-put_cpu:
-	clk_put(cpufreq->cpu_clk);
-
-	return err;
 }
 
 static int tegra20_cpufreq_remove(struct platform_device *pdev)
@@ -214,10 +390,6 @@ static int tegra20_cpufreq_remove(struct platform_device *pdev)
 
 	cpufreq_unregister_driver(&cpufreq->driver);
 
-	clk_put(cpufreq->pll_p_clk);
-	clk_put(cpufreq->pll_x_clk);
-	clk_put(cpufreq->cpu_clk);
-
 	return 0;
 }
 
-- 
2.18.0

[PATCH v1 3/5] ARM: tegra: Create tegra20-cpufreq device on Tegra30

[Dmitry Osipenko]

Tegra20-cpufreq driver require a platform device in order to be loaded,
instantiate a simple platform device for the driver during of the machines
late initialization. Driver now supports Tegra30 SoC's, hence create the
device on Tegra30 machines.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 arch/arm/mach-tegra/tegra.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/mach-tegra/tegra.c b/arch/arm/mach-tegra/tegra.c
index 67d8ae60ac67..b559e22eab76 100644
--- a/arch/arm/mach-tegra/tegra.c
+++ b/arch/arm/mach-tegra/tegra.c
@@ -111,6 +111,10 @@ static void __init tegra_dt_init_late(void)
 	if (IS_ENABLED(CONFIG_ARCH_TEGRA_2x_SOC) &&
 	    of_machine_is_compatible("nvidia,tegra20"))
 		platform_device_register_simple("tegra20-cpufreq", -1, NULL, 0);
+
+	if (IS_ENABLED(CONFIG_ARCH_TEGRA_3x_SOC) &&
+	    of_machine_is_compatible("nvidia,tegra30"))
+		platform_device_register_simple("tegra20-cpufreq", -1, NULL, 0);
 }
 
 static const char * const tegra_dt_board_compat[] = {
-- 
2.18.0

[PATCH v1 4/5] ARM: dts: tegra20: Add CPU Operating Performance Points

[Dmitry Osipenko]

Add CPU's Operating Performance Points to the device tree, they are used
by the CPUFreq driver and allow to setup thermal throttling for the boards
by linking the cooling device (CPU) with thermal sensors via thermal-zones
description.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 arch/arm/boot/dts/tegra20.dtsi | 58 ++++++++++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)

diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
index 01398db0c9c7..4415de0f7c65 100644
--- a/arch/arm/boot/dts/tegra20.dtsi
+++ b/arch/arm/boot/dts/tegra20.dtsi
@@ -859,6 +859,52 @@
 		status = "disabled";
 	};
 
+	cpu0_opp_table: opp_table0 {
+		compatible = "operating-points-v2";
+		opp-shared;
+
+		opp@216000000 {
+			clock-latency-ns = <2000>;
+			opp-hz = /bits/ 64 <216000000>;
+			opp-suspend;
+		};
+
+		opp@312000000 {
+			clock-latency-ns = <125000>;
+			opp-hz = /bits/ 64 <312000000>;
+		};
+
+		opp@456000000 {
+			clock-latency-ns = <125000>;
+			opp-hz = /bits/ 64 <456000000>;
+		};
+
+		opp@608000000 {
+			clock-latency-ns = <125000>;
+			opp-hz = /bits/ 64 <608000000>;
+		};
+
+		opp@760000000 {
+			clock-latency-ns = <125000>;
+			opp-hz = /bits/ 64 <760000000>;
+		};
+
+		opp@816000000 {
+			clock-latency-ns = <125000>;
+			opp-hz = /bits/ 64 <816000000>;
+		};
+
+		opp@912000000 {
+			clock-latency-ns = <125000>;
+			opp-hz = /bits/ 64 <912000000>;
+		};
+
+		opp@1000000000 {
+			clock-latency-ns = <125000>;
+			opp-hz = /bits/ 64 <1000000000>;
+		};
+	};
+
 	cpus {
 		#address-cells = <1>;
 		#size-cells = <0>;
@@ -867,12 +913,24 @@
 			device_type = "cpu";
 			compatible = "arm,cortex-a9";
 			reg = <0>;
+			clocks = <&tegra_car TEGRA20_CLK_PLL_X>,
+				 <&tegra_car TEGRA20_CLK_PLL_P>,
+				 <&tegra_car TEGRA20_CLK_CCLK>;
+			clock-names = "pll_x", "backup", "cpu";
+			operating-points-v2 = <&cpu0_opp_table>;
+			#cooling-cells = <2>;
 		};
 
 		cpu@1 {
 			device_type = "cpu";
 			compatible = "arm,cortex-a9";
 			reg = <1>;
+			clocks = <&tegra_car TEGRA20_CLK_PLL_X>,
+				 <&tegra_car TEGRA20_CLK_PLL_P>,
+				 <&tegra_car TEGRA20_CLK_CCLK>;
+			clock-names = "pll_x", "backup", "cpu";
+			operating-points-v2 = <&cpu0_opp_table>;
+			#cooling-cells = <2>;
 		};
 	};
 
-- 
2.18.0

[PATCH v1 5/5] ARM: dts: tegra30: Add CPU Operating Performance Points

[Dmitry Osipenko]

Add CPU's Operating Performance Points to the device tree, they are used
by the CPUFreq driver and allow to setup thermal throttling for the boards
by linking the cooling device (CPU) with thermal sensors via thermal-zones
description.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 arch/arm/boot/dts/tegra30.dtsi | 65 ++++++++++++++++++++++++++++++++++
 1 file changed, 65 insertions(+)

diff --git a/arch/arm/boot/dts/tegra30.dtsi b/arch/arm/boot/dts/tegra30.dtsi
index 37c4757516d2..5c8098bdfb2a 100644
--- a/arch/arm/boot/dts/tegra30.dtsi
+++ b/arch/arm/boot/dts/tegra30.dtsi
@@ -980,6 +980,47 @@
 		status = "disabled";
 	};
 
+	cpu0_opp_table: opp_table0 {
+		compatible = "operating-points-v2";
+		opp-shared;
+
+		opp@408000000 {
+			clock-latency-ns = <2000>;
+			opp-hz = /bits/ 64 <408000000>;
+			opp-suspend;
+		};
+
+		opp@456000000 {
+			clock-latency-ns = <50000>;
+			opp-hz = /bits/ 64 <456000000>;
+		};
+
+		opp@608000000 {
+			clock-latency-ns = <50000>;
+			opp-hz = /bits/ 64 <608000000>;
+		};
+
+		opp@760000000 {
+			clock-latency-ns = <50000>;
+			opp-hz = /bits/ 64 <760000000>;
+		};
+
+		opp@816000000 {
+			clock-latency-ns = <50000>;
+			opp-hz = /bits/ 64 <816000000>;
+		};
+
+		opp@912000000 {
+			clock-latency-ns = <50000>;
+			opp-hz = /bits/ 64 <912000000>;
+		};
+
+		opp@1000000000 {
+			clock-latency-ns = <50000>;
+			opp-hz = /bits/ 64 <1000000000>;
+		};
+	};
+
 	cpus {
 		#address-cells = <1>;
 		#size-cells = <0>;
@@ -988,24 +1029,48 @@
 			device_type = "cpu";
 			compatible = "arm,cortex-a9";
 			reg = <0>;
+			clocks = <&tegra_car TEGRA30_CLK_PLL_X>,
+				 <&tegra_car TEGRA30_CLK_PLL_P>,
+				 <&tegra_car TEGRA30_CLK_CCLK_G>;
+			clock-names = "pll_x", "backup", "cpu";
+			operating-points-v2 = <&cpu0_opp_table>;
+			#cooling-cells = <2>;
 		};
 
 		cpu@1 {
 			device_type = "cpu";
 			compatible = "arm,cortex-a9";
 			reg = <1>;
+			clocks = <&tegra_car TEGRA30_CLK_PLL_X>,
+				 <&tegra_car TEGRA30_CLK_PLL_P>,
+				 <&tegra_car TEGRA30_CLK_CCLK_G>;
+			clock-names = "pll_x", "backup", "cpu";
+			operating-points-v2 = <&cpu0_opp_table>;
+			#cooling-cells = <2>;
 		};
 
 		cpu@2 {
 			device_type = "cpu";
 			compatible = "arm,cortex-a9";
 			reg = <2>;
+			clocks = <&tegra_car TEGRA30_CLK_PLL_X>,
+				 <&tegra_car TEGRA30_CLK_PLL_P>,
+				 <&tegra_car TEGRA30_CLK_CCLK_G>;
+			clock-names = "pll_x", "backup", "cpu";
+			operating-points-v2 = <&cpu0_opp_table>;
+			#cooling-cells = <2>;
 		};
 
 		cpu@3 {
 			device_type = "cpu";
 			compatible = "arm,cortex-a9";
 			reg = <3>;
+			clocks = <&tegra_car TEGRA30_CLK_PLL_X>,
+				 <&tegra_car TEGRA30_CLK_PLL_P>,
+				 <&tegra_car TEGRA30_CLK_CCLK_G>;
+			clock-names = "pll_x", "backup", "cpu";
+			operating-points-v2 = <&cpu0_opp_table>;
+			#cooling-cells = <2>;
 		};
 	};
 
-- 
2.18.0

Re: [PATCH v1 0/5] CPUFREQ OPP's and Tegra30 support by tegra20-cpufreq driver

[Marcel Ziswiler]

On Thu, 2018-08-30 at 22:43 +0300, Dmitry Osipenko wrote:
> Hello,
> 
> This series adds support for CPU frequency scaling on Tegra30 and
> device
> tree support that allows to implement thermal throttling and
> customize
> available CPU frequencies per-board. The tegra20-cpufreq driver has
> been
> re-worked to support that all.
> 
> Note that Tegra30 support not strictly depends on the clock patches
> that
> are under review now, CPUFREQ driver will fail to probe until CCLKG
> clock
> will get exposed by the clock driver. Hence this series can be
> applied
> independently of the clock patches, CPUFREQ will start to work on
> Tegra30
> once both patchsets will be applied.

In general this seems to work fine both on Colibri T20 as well as
Apalis/Colibri T30. However I did notice a few things plus have some
additional questions:

- Both T20 as well as T30 currently limit the max frequency to 1 GHz
while there are clearly T30 SKUs which may allow higher frequencies
(e.g. our regular T30 aka Embedded SKU 1.3 GHz max resp. 1.4 GHz single
core only, commercial T30 aka AP33 or T33 or whatever it is called up
to 1.6 GHz max resp. 1.7 GHz single core only). Would I have to allow
this via custom OPPs in my device tree?

- However, certain OPPs may also require adjusting core/cpu voltages
which is not yet taken care of as far as I can tell, correct?

- I believe in downstream certain OPPs also take silicon parameters aka
speedo whatever into account. Any comments/plans concerning this?

- With "cpufreq-info -f" I could only observe like the top 3-4 OPPs
while it does not to go further down even when idling. Why could that
be resp. what could cause this?

- Unfortunately "cpufreq-info --stats" currently does not seem to
output anything. Would that require something special to be
implemented?

Other than that you may add the following to the whole series:

Tested-by: Marcel Ziswiler <marcel.ziswiler@toradex.com>

> Dmitry Osipenko (5):
>   dt-bindings: cpufreq: Add binding for NVIDIA Tegra20/30
>   cpufreq: tegra20: Support OPP, thermal cooling and Tegra30
>   ARM: tegra: Create tegra20-cpufreq device on Tegra30
>   ARM: dts: tegra20: Add CPU Operating Performance Points
>   ARM: dts: tegra30: Add CPU Operating Performance Points
> 
>  .../cpufreq/nvidia,tegra20-cpufreq.txt        |  38 ++
>  arch/arm/boot/dts/tegra20.dtsi                |  58 +++
>  arch/arm/boot/dts/tegra30.dtsi                |  65 ++++
>  arch/arm/mach-tegra/tegra.c                   |   4 +
>  drivers/cpufreq/Kconfig.arm                   |   2 +
>  drivers/cpufreq/cpufreq-dt-platdev.c          |   2 +
>  drivers/cpufreq/tegra20-cpufreq.c             | 334 +++++++++++++---
> --
>  7 files changed, 422 insertions(+), 81 deletions(-)
>  create mode 100644
> Documentation/devicetree/bindings/cpufreq/nvidia,tegra20-cpufreq.txt

Re: [PATCH v1 0/5] CPUFREQ OPP's and Tegra30 support by tegra20-cpufreq driver

[Dmitry Osipenko]

On 9/6/18 3:35 PM, Marcel Ziswiler wrote:
> On Thu, 2018-08-30 at 22:43 +0300, Dmitry Osipenko wrote:
>> Hello,
>>
>> This series adds support for CPU frequency scaling on Tegra30 and
>> device
>> tree support that allows to implement thermal throttling and
>> customize
>> available CPU frequencies per-board. The tegra20-cpufreq driver has
>> been
>> re-worked to support that all.
>>
>> Note that Tegra30 support not strictly depends on the clock patches
>> that
>> are under review now, CPUFREQ driver will fail to probe until CCLKG
>> clock
>> will get exposed by the clock driver. Hence this series can be
>> applied
>> independently of the clock patches, CPUFREQ will start to work on
>> Tegra30
>> once both patchsets will be applied.
> 
> In general this seems to work fine both on Colibri T20 as well as
> Apalis/Colibri T30. However I did notice a few things plus have some
> additional questions:
> 
> - Both T20 as well as T30 currently limit the max frequency to 1 GHz
> while there are clearly T30 SKUs which may allow higher frequencies
> (e.g. our regular T30 aka Embedded SKU 1.3 GHz max resp. 1.4 GHz single
> core only, commercial T30 aka AP33 or T33 or whatever it is called up
> to 1.6 GHz max resp. 1.7 GHz single core only). Would I have to allow
> this via custom OPPs in my device tree?

Yes.

> - However, certain OPPs may also require adjusting core/cpu voltages
> which is not yet taken care of as far as I can tell, correct?

Yes, DVFS isn't implemented yet. That could be supported later.

> - I believe in downstream certain OPPs also take silicon parameters aka
> speedo whatever into account. Any comments/plans concerning this?

Good point. There is 'opp-supported-hw' device-tree property which seems is intended
for that purpose. I'll take a look at making use of the property in the next revision,
alternatively that could be implemented later in a distinct patch.

> - With "cpufreq-info -f" I could only observe like the top 3-4 OPPs
> while it does not to go further down even when idling. Why could that
> be resp. what could cause this?

What cpufreq governor are you using?

Here is my 'cpufreq-info --stats' output from Tegra30 after a several minutes of idling after boot:

408000:245884, 456000:445, 608000:251, 760000:151, 816000:82, 912000:75, 1000000:163  (561)

And full cpufreq-info:

cpufrequtils 008: cpufreq-info (C) Dominik Brodowski 2004-2009
Report errors and bugs to cpufreq@vger.kernel.org, please.
analyzing CPU 0:
   driver: tegra
   CPUs which run at the same hardware frequency: 0 1 2 3
   CPUs which need to have their frequency coordinated by software: 0 1 2 3
   maximum transition latency: 50.0 us.
   hardware limits: 408 MHz - 1000 MHz
   available frequency steps: 408 MHz, 456 MHz, 608 MHz, 760 MHz, 816 MHz, 912 MHz, 1000 MHz
   available cpufreq governors: conservative, userspace, powersave, ondemand, performance, schedutil
   current policy: frequency should be within 408 MHz and 1000 MHz.
                   The governor "ondemand" may decide which speed to use
                   within this range.
   current CPU frequency is 608 MHz (asserted by call to hardware).
   cpufreq stats: 408 MHz:99.53%, 456 MHz:0.18%, 608 MHz:0.10%, 760 MHz:0.06%, 816 MHz:0.03%, 912 MHz:0.03%, 1000 MHz:0.07%  (563)
analyzing CPU 1:
   driver: tegra
   CPUs which run at the same hardware frequency: 0 1 2 3
   CPUs which need to have their frequency coordinated by software: 0 1 2 3
   maximum transition latency: 50.0 us.
   hardware limits: 408 MHz - 1000 MHz
   available frequency steps: 408 MHz, 456 MHz, 608 MHz, 760 MHz, 816 MHz, 912 MHz, 1000 MHz
   available cpufreq governors: conservative, userspace, powersave, ondemand, performance, schedutil
   current policy: frequency should be within 408 MHz and 1000 MHz.
                   The governor "ondemand" may decide which speed to use
                   within this range.
   current CPU frequency is 608 MHz (asserted by call to hardware).
   cpufreq stats: 408 MHz:99.53%, 456 MHz:0.18%, 608 MHz:0.10%, 760 MHz:0.06%, 816 MHz:0.03%, 912 MHz:0.03%, 1000 MHz:0.07%  (563)
analyzing CPU 2:
   driver: tegra
   CPUs which run at the same hardware frequency: 0 1 2 3
   CPUs which need to have their frequency coordinated by software: 0 1 2 3
   maximum transition latency: 50.0 us.
   hardware limits: 408 MHz - 1000 MHz
   available frequency steps: 408 MHz, 456 MHz, 608 MHz, 760 MHz, 816 MHz, 912 MHz, 1000 MHz
   available cpufreq governors: conservative, userspace, powersave, ondemand, performance, schedutil
   current policy: frequency should be within 408 MHz and 1000 MHz.
                   The governor "ondemand" may decide which speed to use
                   within this range.
   current CPU frequency is 608 MHz (asserted by call to hardware).
   cpufreq stats: 408 MHz:99.53%, 456 MHz:0.18%, 608 MHz:0.10%, 760 MHz:0.06%, 816 MHz:0.03%, 912 MHz:0.03%, 1000 MHz:0.07%  (563)
analyzing CPU 3:
   driver: tegra
   CPUs which run at the same hardware frequency: 0 1 2 3
   CPUs which need to have their frequency coordinated by software: 0 1 2 3
   maximum transition latency: 50.0 us.
   hardware limits: 408 MHz - 1000 MHz
   available frequency steps: 408 MHz, 456 MHz, 608 MHz, 760 MHz, 816 MHz, 912 MHz, 1000 MHz
   available cpufreq governors: conservative, userspace, powersave, ondemand, performance, schedutil
   current policy: frequency should be within 408 MHz and 1000 MHz.
                   The governor "ondemand" may decide which speed to use
                   within this range.
   current CPU frequency is 608 MHz (asserted by call to hardware).
   cpufreq stats: 408 MHz:99.53%, 456 MHz:0.18%, 608 MHz:0.10%, 760 MHz:0.06%, 816 MHz:0.03%, 912 MHz:0.03%, 1000 MHz:0.07%  (563)


> - Unfortunately "cpufreq-info --stats" currently does not seem to
> output anything. Would that require something special to be
> implemented?

Make sure that CONFIG_CPU_FREQ_STAT is enabled in the kernels config.

> Other than that you may add the following to the whole series:
> 
> Tested-by: Marcel Ziswiler <marcel.ziswiler@toradex.com>

Thank you very much!

Re: [PATCH v1 0/5] CPUFREQ OPP's and Tegra30 support by tegra20-cpufreq driver

[Marcel Ziswiler]

On Fri, 2018-09-07 at 19:59 +0300, Dmitry Osipenko wrote:

- snip -

> > - With "cpufreq-info -f" I could only observe like the top 3-4 OPPs
> > while it does not to go further down even when idling. Why could
> > that
> > be resp. what could cause this?
> 
> What cpufreq governor are you using?

ondemand

> Here is my 'cpufreq-info --stats' output from Tegra30 after a several
> minutes of idling after boot:
> 
> 408000:245884, 456000:445, 608000:251, 760000:151, 816000:82,
> 912000:75, 1000000:163  (561)
> 
> And full cpufreq-info:
> 
> cpufrequtils 008: cpufreq-info (C) Dominik Brodowski 2004-2009
> Report errors and bugs to cpufreq@vger.kernel.org, please.
> analyzing CPU 0:
>    driver: tegra
>    CPUs which run at the same hardware frequency: 0 1 2 3
>    CPUs which need to have their frequency coordinated by software: 0
> 1 2 3
>    maximum transition latency: 50.0 us.
>    hardware limits: 408 MHz - 1000 MHz
>    available frequency steps: 408 MHz, 456 MHz, 608 MHz, 760 MHz, 816
> MHz, 912 MHz, 1000 MHz
>    available cpufreq governors: conservative, userspace, powersave,
> ondemand, performance, schedutil
>    current policy: frequency should be within 408 MHz and 1000 MHz.
>                    The governor "ondemand" may decide which speed to
> use
>                    within this range.
>    current CPU frequency is 608 MHz (asserted by call to hardware).
>    cpufreq stats: 408 MHz:99.53%, 456 MHz:0.18%, 608 MHz:0.10%, 760
> MHz:0.06%, 816 MHz:0.03%, 912 MHz:0.03%, 1000 MHz:0.07%  (563)
> analyzing CPU 1:
>    driver: tegra
>    CPUs which run at the same hardware frequency: 0 1 2 3
>    CPUs which need to have their frequency coordinated by software: 0
> 1 2 3
>    maximum transition latency: 50.0 us.
>    hardware limits: 408 MHz - 1000 MHz
>    available frequency steps: 408 MHz, 456 MHz, 608 MHz, 760 MHz, 816
> MHz, 912 MHz, 1000 MHz
>    available cpufreq governors: conservative, userspace, powersave,
> ondemand, performance, schedutil
>    current policy: frequency should be within 408 MHz and 1000 MHz.
>                    The governor "ondemand" may decide which speed to
> use
>                    within this range.
>    current CPU frequency is 608 MHz (asserted by call to hardware).
>    cpufreq stats: 408 MHz:99.53%, 456 MHz:0.18%, 608 MHz:0.10%, 760
> MHz:0.06%, 816 MHz:0.03%, 912 MHz:0.03%, 1000 MHz:0.07%  (563)
> analyzing CPU 2:
>    driver: tegra
>    CPUs which run at the same hardware frequency: 0 1 2 3
>    CPUs which need to have their frequency coordinated by software: 0
> 1 2 3
>    maximum transition latency: 50.0 us.
>    hardware limits: 408 MHz - 1000 MHz
>    available frequency steps: 408 MHz, 456 MHz, 608 MHz, 760 MHz, 816
> MHz, 912 MHz, 1000 MHz
>    available cpufreq governors: conservative, userspace, powersave,
> ondemand, performance, schedutil
>    current policy: frequency should be within 408 MHz and 1000 MHz.
>                    The governor "ondemand" may decide which speed to
> use
>                    within this range.
>    current CPU frequency is 608 MHz (asserted by call to hardware).
>    cpufreq stats: 408 MHz:99.53%, 456 MHz:0.18%, 608 MHz:0.10%, 760
> MHz:0.06%, 816 MHz:0.03%, 912 MHz:0.03%, 1000 MHz:0.07%  (563)
> analyzing CPU 3:
>    driver: tegra
>    CPUs which run at the same hardware frequency: 0 1 2 3
>    CPUs which need to have their frequency coordinated by software: 0
> 1 2 3
>    maximum transition latency: 50.0 us.
>    hardware limits: 408 MHz - 1000 MHz
>    available frequency steps: 408 MHz, 456 MHz, 608 MHz, 760 MHz, 816
> MHz, 912 MHz, 1000 MHz
>    available cpufreq governors: conservative, userspace, powersave,
> ondemand, performance, schedutil
>    current policy: frequency should be within 408 MHz and 1000 MHz.
>                    The governor "ondemand" may decide which speed to
> use
>                    within this range.
>    current CPU frequency is 608 MHz (asserted by call to hardware).
>    cpufreq stats: 408 MHz:99.53%, 456 MHz:0.18%, 608 MHz:0.10%, 760
> MHz:0.06%, 816 MHz:0.03%, 912 MHz:0.03%, 1000 MHz:0.07%  (563)
> 
> 
> > - Unfortunately "cpufreq-info --stats" currently does not seem to
> > output anything. Would that require something special to be
> > implemented?
> 
> Make sure that CONFIG_CPU_FREQ_STAT is enabled in the kernels config.

Yes, sorry. That was it, of course. Just wondering why that one isn't
enabled in tegra_defconfig...

> > Other than that you may add the following to the whole series:
> > 
> > Tested-by: Marcel Ziswiler <marcel.ziswiler@toradex.com>
> 
> Thank you very much!

You are very welcome. Thank you!

Re: [PATCH v1 0/5] CPUFREQ OPP's and Tegra30 support by tegra20-cpufreq driver

[Dmitry Osipenko]

On 9/11/18 11:27 AM, Marcel Ziswiler wrote:
> On Fri, 2018-09-07 at 19:59 +0300, Dmitry Osipenko wrote:
> 
> - snip -
> 
>>> - With "cpufreq-info -f" I could only observe like the top 3-4 OPPs
>>> while it does not to go further down even when idling. Why could
>>> that
>>> be resp. what could cause this?
>>
>> What cpufreq governor are you using?
> 
> ondemand
> 
>> Here is my 'cpufreq-info --stats' output from Tegra30 after a several
>> minutes of idling after boot:
>>
>> 408000:245884, 456000:445, 608000:251, 760000:151, 816000:82,
>> 912000:75, 1000000:163  (561)
>>
>> And full cpufreq-info:
>>
>> cpufrequtils 008: cpufreq-info (C) Dominik Brodowski 2004-2009
>> Report errors and bugs to cpufreq@vger.kernel.org, please.
>> analyzing CPU 0:
>>     driver: tegra
>>     CPUs which run at the same hardware frequency: 0 1 2 3
>>     CPUs which need to have their frequency coordinated by software: 0
>> 1 2 3
>>     maximum transition latency: 50.0 us.
>>     hardware limits: 408 MHz - 1000 MHz
>>     available frequency steps: 408 MHz, 456 MHz, 608 MHz, 760 MHz, 816
>> MHz, 912 MHz, 1000 MHz
>>     available cpufreq governors: conservative, userspace, powersave,
>> ondemand, performance, schedutil
>>     current policy: frequency should be within 408 MHz and 1000 MHz.
>>                     The governor "ondemand" may decide which speed to
>> use
>>                     within this range.
>>     current CPU frequency is 608 MHz (asserted by call to hardware).
>>     cpufreq stats: 408 MHz:99.53%, 456 MHz:0.18%, 608 MHz:0.10%, 760
>> MHz:0.06%, 816 MHz:0.03%, 912 MHz:0.03%, 1000 MHz:0.07%  (563)
>> analyzing CPU 1:
>>     driver: tegra
>>     CPUs which run at the same hardware frequency: 0 1 2 3
>>     CPUs which need to have their frequency coordinated by software: 0
>> 1 2 3
>>     maximum transition latency: 50.0 us.
>>     hardware limits: 408 MHz - 1000 MHz
>>     available frequency steps: 408 MHz, 456 MHz, 608 MHz, 760 MHz, 816
>> MHz, 912 MHz, 1000 MHz
>>     available cpufreq governors: conservative, userspace, powersave,
>> ondemand, performance, schedutil
>>     current policy: frequency should be within 408 MHz and 1000 MHz.
>>                     The governor "ondemand" may decide which speed to
>> use
>>                     within this range.
>>     current CPU frequency is 608 MHz (asserted by call to hardware).
>>     cpufreq stats: 408 MHz:99.53%, 456 MHz:0.18%, 608 MHz:0.10%, 760
>> MHz:0.06%, 816 MHz:0.03%, 912 MHz:0.03%, 1000 MHz:0.07%  (563)
>> analyzing CPU 2:
>>     driver: tegra
>>     CPUs which run at the same hardware frequency: 0 1 2 3
>>     CPUs which need to have their frequency coordinated by software: 0
>> 1 2 3
>>     maximum transition latency: 50.0 us.
>>     hardware limits: 408 MHz - 1000 MHz
>>     available frequency steps: 408 MHz, 456 MHz, 608 MHz, 760 MHz, 816
>> MHz, 912 MHz, 1000 MHz
>>     available cpufreq governors: conservative, userspace, powersave,
>> ondemand, performance, schedutil
>>     current policy: frequency should be within 408 MHz and 1000 MHz.
>>                     The governor "ondemand" may decide which speed to
>> use
>>                     within this range.
>>     current CPU frequency is 608 MHz (asserted by call to hardware).
>>     cpufreq stats: 408 MHz:99.53%, 456 MHz:0.18%, 608 MHz:0.10%, 760
>> MHz:0.06%, 816 MHz:0.03%, 912 MHz:0.03%, 1000 MHz:0.07%  (563)
>> analyzing CPU 3:
>>     driver: tegra
>>     CPUs which run at the same hardware frequency: 0 1 2 3
>>     CPUs which need to have their frequency coordinated by software: 0
>> 1 2 3
>>     maximum transition latency: 50.0 us.
>>     hardware limits: 408 MHz - 1000 MHz
>>     available frequency steps: 408 MHz, 456 MHz, 608 MHz, 760 MHz, 816
>> MHz, 912 MHz, 1000 MHz
>>     available cpufreq governors: conservative, userspace, powersave,
>> ondemand, performance, schedutil
>>     current policy: frequency should be within 408 MHz and 1000 MHz.
>>                     The governor "ondemand" may decide which speed to
>> use
>>                     within this range.
>>     current CPU frequency is 608 MHz (asserted by call to hardware).
>>     cpufreq stats: 408 MHz:99.53%, 456 MHz:0.18%, 608 MHz:0.10%, 760
>> MHz:0.06%, 816 MHz:0.03%, 912 MHz:0.03%, 1000 MHz:0.07%  (563)
>>
>>
>>> - Unfortunately "cpufreq-info --stats" currently does not seem to
>>> output anything. Would that require something special to be
>>> implemented?
>>
>> Make sure that CONFIG_CPU_FREQ_STAT is enabled in the kernels config.
> 
> Yes, sorry. That was it, of course. Just wondering why that one isn't
> enabled in tegra_defconfig...

That option isn't essential for the kernel, though usually such useful and 
low-overhead features are welcome. IIRC, tegra_defconfig misses more important 
options (like namespaces) that are required by Linux distro's to work properly.

>>> Other than that you may add the following to the whole series:
>>>
>>> Tested-by: Marcel Ziswiler <marcel.ziswiler@toradex.com>
>>
>> Thank you very much!
> 
> You are very welcome. Thank you!
> 

Re: [PATCH v1 1/5] dt-bindings: cpufreq: Add binding for NVIDIA Tegra20/30

[Rob Herring]

On Thu, Aug 30, 2018 at 10:43:52PM +0300, Dmitry Osipenko wrote:
> Add device-tree binding that describes CPU frequency-scaling hardware
> found on NVIDIA Tegra20/30 SoC's.
> 
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
>  .../cpufreq/nvidia,tegra20-cpufreq.txt        | 38 +++++++++++++++++++
>  1 file changed, 38 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/cpufreq/nvidia,tegra20-cpufreq.txt
> 
> diff --git a/Documentation/devicetree/bindings/cpufreq/nvidia,tegra20-cpufreq.txt b/Documentation/devicetree/bindings/cpufreq/nvidia,tegra20-cpufreq.txt
> new file mode 100644
> index 000000000000..2c51f676e958
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/cpufreq/nvidia,tegra20-cpufreq.txt
> @@ -0,0 +1,38 @@
> +Binding for NVIDIA Tegra20 CPUFreq
> +==================================
> +
> +Required properties:
> +- clocks: Must contain an entry for each entry in clock-names.
> +  See ../clocks/clock-bindings.txt for details.
> +- clock-names: Must include the following entries:
> +  - pll_x: main-parent for CPU clock, must be the first entry
> +  - backup: intermediate-parent for CPU clock
> +  - cpu: the CPU clock

The Cortex A9 has CLK, PERIPHCLK, and PERIPHCLKEN clocks and only CLK 
is used for the cpu core. You can't just define your own clocks that 
you happen to want access to.

Otherwise, you're not defining anything new here, so a binding document 
isn't required.

Rob

Re: [PATCH v1 1/5] dt-bindings: cpufreq: Add binding for NVIDIA Tegra20/30

[Dmitry Osipenko]

On 9/25/18 7:58 PM, Rob Herring wrote:
> On Thu, Aug 30, 2018 at 10:43:52PM +0300, Dmitry Osipenko wrote:
>> Add device-tree binding that describes CPU frequency-scaling hardware
>> found on NVIDIA Tegra20/30 SoC's.
>>
>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>> ---
>>  .../cpufreq/nvidia,tegra20-cpufreq.txt        | 38 +++++++++++++++++++
>>  1 file changed, 38 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/cpufreq/nvidia,tegra20-cpufreq.txt
>>
>> diff --git a/Documentation/devicetree/bindings/cpufreq/nvidia,tegra20-cpufreq.txt b/Documentation/devicetree/bindings/cpufreq/nvidia,tegra20-cpufreq.txt
>> new file mode 100644
>> index 000000000000..2c51f676e958
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/cpufreq/nvidia,tegra20-cpufreq.txt
>> @@ -0,0 +1,38 @@
>> +Binding for NVIDIA Tegra20 CPUFreq
>> +==================================
>> +
>> +Required properties:
>> +- clocks: Must contain an entry for each entry in clock-names.
>> +  See ../clocks/clock-bindings.txt for details.
>> +- clock-names: Must include the following entries:
>> +  - pll_x: main-parent for CPU clock, must be the first entry
>> +  - backup: intermediate-parent for CPU clock
>> +  - cpu: the CPU clock
> 
> The Cortex A9 has CLK, PERIPHCLK, and PERIPHCLKEN clocks and only CLK 
> is used for the cpu core. You can't just define your own clocks that 
> you happen to want access to.
> 
> Otherwise, you're not defining anything new here, so a binding document 
> isn't required.

PERIPHCLK is a different thing. Here we are defining the CPU clock and
its *parent* sources, the PLLX (main) and backup (intermediate clock
that is used while PLLX changes its rate). These are not some random
clocks "that you happen to want access to", they are essential for the
Tegra CPUFreq driver, CPU is running off them.

I assume that PERIPHCLK and other clocks are derived from the "CPU"
clock and their configuration is hardwired. Probably Peter knows how
it's implemented in HW.

I'm now working on v2 that will include more Tegra-specific stuff in the
binding, like custom "opp-supported-hw" property and probably some more.

Re: [PATCH v1 1/5] dt-bindings: cpufreq: Add binding for NVIDIA Tegra20/30

[Rob Herring]

On Tue, Sep 25, 2018 at 12:29 PM Dmitry Osipenko <digetx@gmail.com> wrote:
>
> On 9/25/18 7:58 PM, Rob Herring wrote:
> > On Thu, Aug 30, 2018 at 10:43:52PM +0300, Dmitry Osipenko wrote:
> >> Add device-tree binding that describes CPU frequency-scaling hardware
> >> found on NVIDIA Tegra20/30 SoC's.
> >>
> >> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> >> ---
> >>  .../cpufreq/nvidia,tegra20-cpufreq.txt        | 38 +++++++++++++++++++
> >>  1 file changed, 38 insertions(+)
> >>  create mode 100644 Documentation/devicetree/bindings/cpufreq/nvidia,tegra20-cpufreq.txt
> >>
> >> diff --git a/Documentation/devicetree/bindings/cpufreq/nvidia,tegra20-cpufreq.txt b/Documentation/devicetree/bindings/cpufreq/nvidia,tegra20-cpufreq.txt
> >> new file mode 100644
> >> index 000000000000..2c51f676e958
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/cpufreq/nvidia,tegra20-cpufreq.txt
> >> @@ -0,0 +1,38 @@
> >> +Binding for NVIDIA Tegra20 CPUFreq
> >> +==================================
> >> +
> >> +Required properties:
> >> +- clocks: Must contain an entry for each entry in clock-names.
> >> +  See ../clocks/clock-bindings.txt for details.
> >> +- clock-names: Must include the following entries:
> >> +  - pll_x: main-parent for CPU clock, must be the first entry
> >> +  - backup: intermediate-parent for CPU clock
> >> +  - cpu: the CPU clock
> >
> > The Cortex A9 has CLK, PERIPHCLK, and PERIPHCLKEN clocks and only CLK
> > is used for the cpu core. You can't just define your own clocks that
> > you happen to want access to.
> >
> > Otherwise, you're not defining anything new here, so a binding document
> > isn't required.
>
> PERIPHCLK is a different thing.

Right, that's what I meant. Only CLK is used.

> Here we are defining the CPU clock and
> its *parent* sources, the PLLX (main) and backup (intermediate clock
> that is used while PLLX changes its rate). These are not some random
> clocks "that you happen to want access to", they are essential for the
> Tegra CPUFreq driver, CPU is running off them.

assigned-clocks is generally how we get parent clocks in this
situation. "clocks" is for what physical clocks are attached to a
given block. ARM very clearly documents the clocks for their IP
blocks.

> I assume that PERIPHCLK and other clocks are derived from the "CPU"
> clock and their configuration is hardwired. Probably Peter knows how
> it's implemented in HW.

Yes, because PERIPHCLK must be synchronous. In any case, it is
irrelevant to cpu nodes and applies to timer, SCU, and GIC nodes.

> I'm now working on v2 that will include more Tegra-specific stuff in the
> binding, like custom "opp-supported-hw" property and probably some more.

Re: [PATCH v1 1/5] dt-bindings: cpufreq: Add binding for NVIDIA Tegra20/30

[Dmitry Osipenko]

On 9/25/18 10:36 PM, Rob Herring wrote:
> On Tue, Sep 25, 2018 at 12:29 PM Dmitry Osipenko <digetx@gmail.com> wrote:
>>
>> On 9/25/18 7:58 PM, Rob Herring wrote:
>>> On Thu, Aug 30, 2018 at 10:43:52PM +0300, Dmitry Osipenko wrote:
>>>> Add device-tree binding that describes CPU frequency-scaling hardware
>>>> found on NVIDIA Tegra20/30 SoC's.
>>>>
>>>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>>>> ---
>>>>  .../cpufreq/nvidia,tegra20-cpufreq.txt        | 38 +++++++++++++++++++
>>>>  1 file changed, 38 insertions(+)
>>>>  create mode 100644 Documentation/devicetree/bindings/cpufreq/nvidia,tegra20-cpufreq.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/cpufreq/nvidia,tegra20-cpufreq.txt b/Documentation/devicetree/bindings/cpufreq/nvidia,tegra20-cpufreq.txt
>>>> new file mode 100644
>>>> index 000000000000..2c51f676e958
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/cpufreq/nvidia,tegra20-cpufreq.txt
>>>> @@ -0,0 +1,38 @@
>>>> +Binding for NVIDIA Tegra20 CPUFreq
>>>> +==================================
>>>> +
>>>> +Required properties:
>>>> +- clocks: Must contain an entry for each entry in clock-names.
>>>> +  See ../clocks/clock-bindings.txt for details.
>>>> +- clock-names: Must include the following entries:
>>>> +  - pll_x: main-parent for CPU clock, must be the first entry
>>>> +  - backup: intermediate-parent for CPU clock
>>>> +  - cpu: the CPU clock
>>>
>>> The Cortex A9 has CLK, PERIPHCLK, and PERIPHCLKEN clocks and only CLK
>>> is used for the cpu core. You can't just define your own clocks that
>>> you happen to want access to.
>>>
>>> Otherwise, you're not defining anything new here, so a binding document
>>> isn't required.
>>
>> PERIPHCLK is a different thing.
> 
> Right, that's what I meant. Only CLK is used.
> 
>> Here we are defining the CPU clock and
>> its *parent* sources, the PLLX (main) and backup (intermediate clock
>> that is used while PLLX changes its rate). These are not some random
>> clocks "that you happen to want access to", they are essential for the
>> Tegra CPUFreq driver, CPU is running off them.
> 
> assigned-clocks is generally how we get parent clocks in this
> situation. "clocks" is for what physical clocks are attached to a
> given block. ARM very clearly documents the clocks for their IP
> blocks.

Okay, seems I see now what you're meaning. The PLLX is specifically
dedicated to CPU HW on Tegra, so it shouldn't be questioning to put it
within the CPU node, unlike the backup.

The assigned-clocks are about static clocks configuration, that is
exactly opposite to what is needed. I'm not sure what you are trying to
convey.. we don't need to get the parent clock, but set it.

How the backup/intermediate clock should be represented then? It is a
part of HW description which potentially may vary depending on the board
configuration.

Re: [PATCH v1 1/5] dt-bindings: cpufreq: Add binding for NVIDIA Tegra20/30

[Jon Hunter]

On 30/08/2018 20:43, Dmitry Osipenko wrote:
> Add device-tree binding that describes CPU frequency-scaling hardware
> found on NVIDIA Tegra20/30 SoC's.
> 
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
>  .../cpufreq/nvidia,tegra20-cpufreq.txt        | 38 +++++++++++++++++++
>  1 file changed, 38 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/cpufreq/nvidia,tegra20-cpufreq.txt
> 
> diff --git a/Documentation/devicetree/bindings/cpufreq/nvidia,tegra20-cpufreq.txt b/Documentation/devicetree/bindings/cpufreq/nvidia,tegra20-cpufreq.txt
> new file mode 100644
> index 000000000000..2c51f676e958
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/cpufreq/nvidia,tegra20-cpufreq.txt
> @@ -0,0 +1,38 @@
> +Binding for NVIDIA Tegra20 CPUFreq
> +==================================
> +
> +Required properties:
> +- clocks: Must contain an entry for each entry in clock-names.
> +  See ../clocks/clock-bindings.txt for details.
> +- clock-names: Must include the following entries:
> +  - pll_x: main-parent for CPU clock, must be the first entry
> +  - backup: intermediate-parent for CPU clock
> +  - cpu: the CPU clock

Is it likely that 'backup' will be anything other that pll_p? If not why
not just call it pll_p? Personally, I don't 'backup' to descriptive even
though I can see what you mean.

I can see that you want to make this flexible, but if the likelihood is
that we will just use pll_p then I am not sure it is warranted at this
point.

Cheers
Jon

-- 
nvpublic

Re: [PATCH v1 2/5] cpufreq: tegra20: Support OPP, thermal cooling and Tegra30

[Jon Hunter]

On 30/08/2018 20:43, Dmitry Osipenko wrote:
> Add support for thermal throttling and Operating Performance Points.
> Driver now relies on OPP's supplied via device tree and therefore will
> work only on devices that use the updated device tree. The generalization
> of the driver allows to transparently support Tegra30.

Thierry, can we or should we do this?

Cheers
Jon

-- 
nvpublic

Re: [PATCH v1 3/5] ARM: tegra: Create tegra20-cpufreq device on Tegra30

[Jon Hunter]

On 30/08/2018 20:43, Dmitry Osipenko wrote:
> Tegra20-cpufreq driver require a platform device in order to be loaded,
> instantiate a simple platform device for the driver during of the machines
> late initialization. Driver now supports Tegra30 SoC's, hence create the
> device on Tegra30 machines.
> 
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
>  arch/arm/mach-tegra/tegra.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/arch/arm/mach-tegra/tegra.c b/arch/arm/mach-tegra/tegra.c
> index 67d8ae60ac67..b559e22eab76 100644
> --- a/arch/arm/mach-tegra/tegra.c
> +++ b/arch/arm/mach-tegra/tegra.c
> @@ -111,6 +111,10 @@ static void __init tegra_dt_init_late(void)
>  	if (IS_ENABLED(CONFIG_ARCH_TEGRA_2x_SOC) &&
>  	    of_machine_is_compatible("nvidia,tegra20"))
>  		platform_device_register_simple("tegra20-cpufreq", -1, NULL, 0);
> +
> +	if (IS_ENABLED(CONFIG_ARCH_TEGRA_3x_SOC) &&
> +	    of_machine_is_compatible("nvidia,tegra30"))
> +		platform_device_register_simple("tegra20-cpufreq", -1, NULL, 0);
>  }
>  
>  static const char * const tegra_dt_board_compat[] = {

Not sure why you would do this if now the driver only works with DT. Am
I missing something?

Cheers
Jon

-- 
nvpublic

Re: [PATCH v1 3/5] ARM: tegra: Create tegra20-cpufreq device on Tegra30

[Jon Hunter]

On 17/10/2018 09:49, Jon Hunter wrote:
> 
> On 30/08/2018 20:43, Dmitry Osipenko wrote:
>> Tegra20-cpufreq driver require a platform device in order to be loaded,
>> instantiate a simple platform device for the driver during of the machines
>> late initialization. Driver now supports Tegra30 SoC's, hence create the
>> device on Tegra30 machines.
>>
>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>> ---
>>  arch/arm/mach-tegra/tegra.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/arch/arm/mach-tegra/tegra.c b/arch/arm/mach-tegra/tegra.c
>> index 67d8ae60ac67..b559e22eab76 100644
>> --- a/arch/arm/mach-tegra/tegra.c
>> +++ b/arch/arm/mach-tegra/tegra.c
>> @@ -111,6 +111,10 @@ static void __init tegra_dt_init_late(void)
>>  	if (IS_ENABLED(CONFIG_ARCH_TEGRA_2x_SOC) &&
>>  	    of_machine_is_compatible("nvidia,tegra20"))
>>  		platform_device_register_simple("tegra20-cpufreq", -1, NULL, 0);
>> +
>> +	if (IS_ENABLED(CONFIG_ARCH_TEGRA_3x_SOC) &&
>> +	    of_machine_is_compatible("nvidia,tegra30"))
>> +		platform_device_register_simple("tegra20-cpufreq", -1, NULL, 0);
>>  }
>>  
>>  static const char * const tegra_dt_board_compat[] = {
> 
> Not sure why you would do this if now the driver only works with DT. Am
> I missing something?

Actually, not sure why we just don't move this into the actual driver
itself like we have for tegra124.

Cheers
Jon

-- 
nvpublic

Re: [PATCH v1 3/5] ARM: tegra: Create tegra20-cpufreq device on Tegra30

[Dmitry Osipenko]

On 10/17/18 12:54 PM, Jon Hunter wrote:
> 
> On 17/10/2018 09:49, Jon Hunter wrote:
>>
>> On 30/08/2018 20:43, Dmitry Osipenko wrote:
>>> Tegra20-cpufreq driver require a platform device in order to be loaded,
>>> instantiate a simple platform device for the driver during of the machines
>>> late initialization. Driver now supports Tegra30 SoC's, hence create the
>>> device on Tegra30 machines.
>>>
>>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>>> ---
>>>  arch/arm/mach-tegra/tegra.c | 4 ++++
>>>  1 file changed, 4 insertions(+)
>>>
>>> diff --git a/arch/arm/mach-tegra/tegra.c b/arch/arm/mach-tegra/tegra.c
>>> index 67d8ae60ac67..b559e22eab76 100644
>>> --- a/arch/arm/mach-tegra/tegra.c
>>> +++ b/arch/arm/mach-tegra/tegra.c
>>> @@ -111,6 +111,10 @@ static void __init tegra_dt_init_late(void)
>>>  	if (IS_ENABLED(CONFIG_ARCH_TEGRA_2x_SOC) &&
>>>  	    of_machine_is_compatible("nvidia,tegra20"))
>>>  		platform_device_register_simple("tegra20-cpufreq", -1, NULL, 0);
>>> +
>>> +	if (IS_ENABLED(CONFIG_ARCH_TEGRA_3x_SOC) &&
>>> +	    of_machine_is_compatible("nvidia,tegra30"))
>>> +		platform_device_register_simple("tegra20-cpufreq", -1, NULL, 0);
>>>  }
>>>  
>>>  static const char * const tegra_dt_board_compat[] = {
>>
>> Not sure why you would do this if now the driver only works with DT. Am
>> I missing something?
> 
> Actually, not sure why we just don't move this into the actual driver
> itself like we have for tegra124.

Tegra124 has specific HW for the CPUFreq, T20/30 do not. Hence on T20/30 CPUFreq control is implemented purely in software and there is no real HW device for the driver.

Re: [PATCH v1 1/5] dt-bindings: cpufreq: Add binding for NVIDIA Tegra20/30

[Dmitry Osipenko]

On 10/17/18 11:40 AM, Jon Hunter wrote:
> 
> On 30/08/2018 20:43, Dmitry Osipenko wrote:
>> Add device-tree binding that describes CPU frequency-scaling hardware
>> found on NVIDIA Tegra20/30 SoC's.
>>
>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>> ---
>>  .../cpufreq/nvidia,tegra20-cpufreq.txt        | 38 +++++++++++++++++++
>>  1 file changed, 38 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/cpufreq/nvidia,tegra20-cpufreq.txt
>>
>> diff --git a/Documentation/devicetree/bindings/cpufreq/nvidia,tegra20-cpufreq.txt b/Documentation/devicetree/bindings/cpufreq/nvidia,tegra20-cpufreq.txt
>> new file mode 100644
>> index 000000000000..2c51f676e958
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/cpufreq/nvidia,tegra20-cpufreq.txt
>> @@ -0,0 +1,38 @@
>> +Binding for NVIDIA Tegra20 CPUFreq
>> +==================================
>> +
>> +Required properties:
>> +- clocks: Must contain an entry for each entry in clock-names.
>> +  See ../clocks/clock-bindings.txt for details.
>> +- clock-names: Must include the following entries:
>> +  - pll_x: main-parent for CPU clock, must be the first entry
>> +  - backup: intermediate-parent for CPU clock
>> +  - cpu: the CPU clock
> 
> Is it likely that 'backup' will be anything other that pll_p? If not why
> not just call it pll_p? Personally, I don't 'backup' to descriptive even
> though I can see what you mean.
> 
> I can see that you want to make this flexible, but if the likelihood is
> that we will just use pll_p then I am not sure it is warranted at this
> point.

That won't describe HW, but software. And device tree should describe HW.

Re: [PATCH v1 1/5] dt-bindings: cpufreq: Add binding for NVIDIA Tegra20/30

[Dmitry Osipenko]

On 10/17/18 3:37 PM, Dmitry Osipenko wrote:
> On 10/17/18 11:40 AM, Jon Hunter wrote:
>>
>> On 30/08/2018 20:43, Dmitry Osipenko wrote:
>>> Add device-tree binding that describes CPU frequency-scaling hardware
>>> found on NVIDIA Tegra20/30 SoC's.
>>>
>>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>>> ---
>>>  .../cpufreq/nvidia,tegra20-cpufreq.txt        | 38 +++++++++++++++++++
>>>  1 file changed, 38 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/cpufreq/nvidia,tegra20-cpufreq.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/cpufreq/nvidia,tegra20-cpufreq.txt b/Documentation/devicetree/bindings/cpufreq/nvidia,tegra20-cpufreq.txt
>>> new file mode 100644
>>> index 000000000000..2c51f676e958
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/cpufreq/nvidia,tegra20-cpufreq.txt
>>> @@ -0,0 +1,38 @@
>>> +Binding for NVIDIA Tegra20 CPUFreq
>>> +==================================
>>> +
>>> +Required properties:
>>> +- clocks: Must contain an entry for each entry in clock-names.
>>> +  See ../clocks/clock-bindings.txt for details.
>>> +- clock-names: Must include the following entries:
>>> +  - pll_x: main-parent for CPU clock, must be the first entry
>>> +  - backup: intermediate-parent for CPU clock
>>> +  - cpu: the CPU clock
>>
>> Is it likely that 'backup' will be anything other that pll_p? If not why
>> not just call it pll_p? Personally, I don't 'backup' to descriptive even
>> though I can see what you mean.
>>
>> I can see that you want to make this flexible, but if the likelihood is
>> that we will just use pll_p then I am not sure it is warranted at this
>> point.
> 
> That won't describe HW, but software. And device tree should describe HW.
> 

Though indeed it is unlikely that anything else other than pll_p will be used, so it is a software/firmware description anyway.

Re: [PATCH v1 1/5] dt-bindings: cpufreq: Add binding for NVIDIA Tegra20/30

[Jon Hunter]

On 17/10/2018 13:37, Dmitry Osipenko wrote:
> On 10/17/18 11:40 AM, Jon Hunter wrote:
>>
>> On 30/08/2018 20:43, Dmitry Osipenko wrote:
>>> Add device-tree binding that describes CPU frequency-scaling hardware
>>> found on NVIDIA Tegra20/30 SoC's.
>>>
>>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>>> ---
>>>  .../cpufreq/nvidia,tegra20-cpufreq.txt        | 38 +++++++++++++++++++
>>>  1 file changed, 38 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/cpufreq/nvidia,tegra20-cpufreq.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/cpufreq/nvidia,tegra20-cpufreq.txt b/Documentation/devicetree/bindings/cpufreq/nvidia,tegra20-cpufreq.txt
>>> new file mode 100644
>>> index 000000000000..2c51f676e958
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/cpufreq/nvidia,tegra20-cpufreq.txt
>>> @@ -0,0 +1,38 @@
>>> +Binding for NVIDIA Tegra20 CPUFreq
>>> +==================================
>>> +
>>> +Required properties:
>>> +- clocks: Must contain an entry for each entry in clock-names.
>>> +  See ../clocks/clock-bindings.txt for details.
>>> +- clock-names: Must include the following entries:
>>> +  - pll_x: main-parent for CPU clock, must be the first entry
>>> +  - backup: intermediate-parent for CPU clock
>>> +  - cpu: the CPU clock
>>
>> Is it likely that 'backup' will be anything other that pll_p? If not why
>> not just call it pll_p? Personally, I don't 'backup' to descriptive even
>> though I can see what you mean.
>>
>> I can see that you want to make this flexible, but if the likelihood is
>> that we will just use pll_p then I am not sure it is warranted at this
>> point.
> 
> That won't describe HW, but software. And device tree should describe HW.

Hmm ... well that's my point exactly. So why call it 'backup'? Sounds
like a software description to me.

Jon

-- 
nvpublic

Re: [PATCH v1 1/5] dt-bindings: cpufreq: Add binding for NVIDIA Tegra20/30

[Dmitry Osipenko]

On 10/17/18 3:59 PM, Jon Hunter wrote:
> 
> On 17/10/2018 13:37, Dmitry Osipenko wrote:
>> On 10/17/18 11:40 AM, Jon Hunter wrote:
>>>
>>> On 30/08/2018 20:43, Dmitry Osipenko wrote:
>>>> Add device-tree binding that describes CPU frequency-scaling hardware
>>>> found on NVIDIA Tegra20/30 SoC's.
>>>>
>>>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>>>> ---
>>>>  .../cpufreq/nvidia,tegra20-cpufreq.txt        | 38 +++++++++++++++++++
>>>>  1 file changed, 38 insertions(+)
>>>>  create mode 100644 Documentation/devicetree/bindings/cpufreq/nvidia,tegra20-cpufreq.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/cpufreq/nvidia,tegra20-cpufreq.txt b/Documentation/devicetree/bindings/cpufreq/nvidia,tegra20-cpufreq.txt
>>>> new file mode 100644
>>>> index 000000000000..2c51f676e958
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/cpufreq/nvidia,tegra20-cpufreq.txt
>>>> @@ -0,0 +1,38 @@
>>>> +Binding for NVIDIA Tegra20 CPUFreq
>>>> +==================================
>>>> +
>>>> +Required properties:
>>>> +- clocks: Must contain an entry for each entry in clock-names.
>>>> +  See ../clocks/clock-bindings.txt for details.
>>>> +- clock-names: Must include the following entries:
>>>> +  - pll_x: main-parent for CPU clock, must be the first entry
>>>> +  - backup: intermediate-parent for CPU clock
>>>> +  - cpu: the CPU clock
>>>
>>> Is it likely that 'backup' will be anything other that pll_p? If not why
>>> not just call it pll_p? Personally, I don't 'backup' to descriptive even
>>> though I can see what you mean.
>>>
>>> I can see that you want to make this flexible, but if the likelihood is
>>> that we will just use pll_p then I am not sure it is warranted at this
>>> point.
>>
>> That won't describe HW, but software. And device tree should describe HW.
> 
> Hmm ... well that's my point exactly. So why call it 'backup'? Sounds
> like a software description to me.

Because HW is designed the way that CPU parent need to be switched to the backup clock source while main clock changes its rate. HW also allow to select among different parents, pll_p is one of those parents.

I think cpufreq-mediatek has a similar description, see Documentation/devicetree/bindings/cpufreq/cpufreq-mediatek.txt

Re: [PATCH v1 1/5] dt-bindings: cpufreq: Add binding for NVIDIA Tegra20/30

[Jon Hunter]

On 17/10/2018 14:07, Dmitry Osipenko wrote:
> On 10/17/18 3:59 PM, Jon Hunter wrote:
>>
>> On 17/10/2018 13:37, Dmitry Osipenko wrote:
>>> On 10/17/18 11:40 AM, Jon Hunter wrote:
>>>>
>>>> On 30/08/2018 20:43, Dmitry Osipenko wrote:
>>>>> Add device-tree binding that describes CPU frequency-scaling hardware
>>>>> found on NVIDIA Tegra20/30 SoC's.
>>>>>
>>>>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>>>>> ---
>>>>>  .../cpufreq/nvidia,tegra20-cpufreq.txt        | 38 +++++++++++++++++++
>>>>>  1 file changed, 38 insertions(+)
>>>>>  create mode 100644 Documentation/devicetree/bindings/cpufreq/nvidia,tegra20-cpufreq.txt
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/cpufreq/nvidia,tegra20-cpufreq.txt b/Documentation/devicetree/bindings/cpufreq/nvidia,tegra20-cpufreq.txt
>>>>> new file mode 100644
>>>>> index 000000000000..2c51f676e958
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/cpufreq/nvidia,tegra20-cpufreq.txt
>>>>> @@ -0,0 +1,38 @@
>>>>> +Binding for NVIDIA Tegra20 CPUFreq
>>>>> +==================================
>>>>> +
>>>>> +Required properties:
>>>>> +- clocks: Must contain an entry for each entry in clock-names.
>>>>> +  See ../clocks/clock-bindings.txt for details.
>>>>> +- clock-names: Must include the following entries:
>>>>> +  - pll_x: main-parent for CPU clock, must be the first entry
>>>>> +  - backup: intermediate-parent for CPU clock
>>>>> +  - cpu: the CPU clock
>>>>
>>>> Is it likely that 'backup' will be anything other that pll_p? If not why
>>>> not just call it pll_p? Personally, I don't 'backup' to descriptive even
>>>> though I can see what you mean.
>>>>
>>>> I can see that you want to make this flexible, but if the likelihood is
>>>> that we will just use pll_p then I am not sure it is warranted at this
>>>> point.
>>>
>>> That won't describe HW, but software. And device tree should describe HW.
>>
>> Hmm ... well that's my point exactly. So why call it 'backup'? Sounds
>> like a software description to me.
> 
> Because HW is designed the way that CPU parent need to be switched to the backup clock source while main clock changes its rate. HW also allow to select among different parents, pll_p is one of those parents.

Yes that part is understood. I am just splitting hairs over the actual
name. We do the same for tegra124 but we just call it 'pll_p'. See ...

Documentation/devicetree/bindings/cpufreq/nvidia,tegra124-cpufreq.txt

Cheers
Jon

-- 
nvpublic

Re: [PATCH v1 1/5] dt-bindings: cpufreq: Add binding for NVIDIA Tegra20/30

[Dmitry Osipenko]

On 10/17/18 4:34 PM, Jon Hunter wrote:
> 
> On 17/10/2018 14:07, Dmitry Osipenko wrote:
>> On 10/17/18 3:59 PM, Jon Hunter wrote:
>>>
>>> On 17/10/2018 13:37, Dmitry Osipenko wrote:
>>>> On 10/17/18 11:40 AM, Jon Hunter wrote:
>>>>>
>>>>> On 30/08/2018 20:43, Dmitry Osipenko wrote:
>>>>>> Add device-tree binding that describes CPU frequency-scaling hardware
>>>>>> found on NVIDIA Tegra20/30 SoC's.
>>>>>>
>>>>>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>>>>>> ---
>>>>>>  .../cpufreq/nvidia,tegra20-cpufreq.txt        | 38 +++++++++++++++++++
>>>>>>  1 file changed, 38 insertions(+)
>>>>>>  create mode 100644 Documentation/devicetree/bindings/cpufreq/nvidia,tegra20-cpufreq.txt
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/cpufreq/nvidia,tegra20-cpufreq.txt b/Documentation/devicetree/bindings/cpufreq/nvidia,tegra20-cpufreq.txt
>>>>>> new file mode 100644
>>>>>> index 000000000000..2c51f676e958
>>>>>> --- /dev/null
>>>>>> +++ b/Documentation/devicetree/bindings/cpufreq/nvidia,tegra20-cpufreq.txt
>>>>>> @@ -0,0 +1,38 @@
>>>>>> +Binding for NVIDIA Tegra20 CPUFreq
>>>>>> +==================================
>>>>>> +
>>>>>> +Required properties:
>>>>>> +- clocks: Must contain an entry for each entry in clock-names.
>>>>>> +  See ../clocks/clock-bindings.txt for details.
>>>>>> +- clock-names: Must include the following entries:
>>>>>> +  - pll_x: main-parent for CPU clock, must be the first entry
>>>>>> +  - backup: intermediate-parent for CPU clock
>>>>>> +  - cpu: the CPU clock
>>>>>
>>>>> Is it likely that 'backup' will be anything other that pll_p? If not why
>>>>> not just call it pll_p? Personally, I don't 'backup' to descriptive even
>>>>> though I can see what you mean.
>>>>>
>>>>> I can see that you want to make this flexible, but if the likelihood is
>>>>> that we will just use pll_p then I am not sure it is warranted at this
>>>>> point.
>>>>
>>>> That won't describe HW, but software. And device tree should describe HW.
>>>
>>> Hmm ... well that's my point exactly. So why call it 'backup'? Sounds
>>> like a software description to me.
>>
>> Because HW is designed the way that CPU parent need to be switched to the backup clock source while main clock changes its rate. HW also allow to select among different parents, pll_p is one of those parents.
> 
> Yes that part is understood. I am just splitting hairs over the actual
> name. We do the same for tegra124 but we just call it 'pll_p'. See ...
> 
> Documentation/devicetree/bindings/cpufreq/nvidia,tegra124-cpufreq.txt

tegra124-cpufreq choose to hardwire to the pll_p, but it could be other clocks. Technically abstracting backup clock should be more correct, but result is the same in the end.

Re: [PATCH v1 1/5] dt-bindings: cpufreq: Add binding for NVIDIA Tegra20/30

[Jon Hunter]

On 17/10/2018 14:46, Dmitry Osipenko wrote:
> On 10/17/18 4:34 PM, Jon Hunter wrote:
>>
>> On 17/10/2018 14:07, Dmitry Osipenko wrote:
>>> On 10/17/18 3:59 PM, Jon Hunter wrote:
>>>>
>>>> On 17/10/2018 13:37, Dmitry Osipenko wrote:
>>>>> On 10/17/18 11:40 AM, Jon Hunter wrote:
>>>>>>
>>>>>> On 30/08/2018 20:43, Dmitry Osipenko wrote:
>>>>>>> Add device-tree binding that describes CPU frequency-scaling hardware
>>>>>>> found on NVIDIA Tegra20/30 SoC's.
>>>>>>>
>>>>>>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>>>>>>> ---
>>>>>>>  .../cpufreq/nvidia,tegra20-cpufreq.txt        | 38 +++++++++++++++++++
>>>>>>>  1 file changed, 38 insertions(+)
>>>>>>>  create mode 100644 Documentation/devicetree/bindings/cpufreq/nvidia,tegra20-cpufreq.txt
>>>>>>>
>>>>>>> diff --git a/Documentation/devicetree/bindings/cpufreq/nvidia,tegra20-cpufreq.txt b/Documentation/devicetree/bindings/cpufreq/nvidia,tegra20-cpufreq.txt
>>>>>>> new file mode 100644
>>>>>>> index 000000000000..2c51f676e958
>>>>>>> --- /dev/null
>>>>>>> +++ b/Documentation/devicetree/bindings/cpufreq/nvidia,tegra20-cpufreq.txt
>>>>>>> @@ -0,0 +1,38 @@
>>>>>>> +Binding for NVIDIA Tegra20 CPUFreq
>>>>>>> +==================================
>>>>>>> +
>>>>>>> +Required properties:
>>>>>>> +- clocks: Must contain an entry for each entry in clock-names.
>>>>>>> +  See ../clocks/clock-bindings.txt for details.
>>>>>>> +- clock-names: Must include the following entries:
>>>>>>> +  - pll_x: main-parent for CPU clock, must be the first entry
>>>>>>> +  - backup: intermediate-parent for CPU clock
>>>>>>> +  - cpu: the CPU clock
>>>>>>
>>>>>> Is it likely that 'backup' will be anything other that pll_p? If not why
>>>>>> not just call it pll_p? Personally, I don't 'backup' to descriptive even
>>>>>> though I can see what you mean.
>>>>>>
>>>>>> I can see that you want to make this flexible, but if the likelihood is
>>>>>> that we will just use pll_p then I am not sure it is warranted at this
>>>>>> point.
>>>>>
>>>>> That won't describe HW, but software. And device tree should describe HW.
>>>>
>>>> Hmm ... well that's my point exactly. So why call it 'backup'? Sounds
>>>> like a software description to me.
>>>
>>> Because HW is designed the way that CPU parent need to be switched to the backup clock source while main clock changes its rate. HW also allow to select among different parents, pll_p is one of those parents.
>>
>> Yes that part is understood. I am just splitting hairs over the actual
>> name. We do the same for tegra124 but we just call it 'pll_p'. See ...
>>
>> Documentation/devicetree/bindings/cpufreq/nvidia,tegra124-cpufreq.txt
> 
> tegra124-cpufreq choose to hardwire to the pll_p, but it could be other clocks. Technically abstracting backup clock should be more correct, but result is the same in the end.

Yes and that is probably intentional as that is the configuration that
has been validated. So unless we are planning to test and verify every
possibility, my preference is to keep it simple. But yes the result is
the same.

I was simply curious of your intention here because of the other series
you posted with regard to the cpu clocks.

Cheers
Jon

-- 
nvpublic

Re: [PATCH v1 1/5] dt-bindings: cpufreq: Add binding for NVIDIA Tegra20/30

[Dmitry Osipenko]

On 10/17/18 5:14 PM, Jon Hunter wrote:
> 
> On 17/10/2018 14:46, Dmitry Osipenko wrote:
>> On 10/17/18 4:34 PM, Jon Hunter wrote:
>>>
>>> On 17/10/2018 14:07, Dmitry Osipenko wrote:
>>>> On 10/17/18 3:59 PM, Jon Hunter wrote:
>>>>>
>>>>> On 17/10/2018 13:37, Dmitry Osipenko wrote:
>>>>>> On 10/17/18 11:40 AM, Jon Hunter wrote:
>>>>>>>
>>>>>>> On 30/08/2018 20:43, Dmitry Osipenko wrote:
>>>>>>>> Add device-tree binding that describes CPU frequency-scaling hardware
>>>>>>>> found on NVIDIA Tegra20/30 SoC's.
>>>>>>>>
>>>>>>>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>>>>>>>> ---
>>>>>>>>  .../cpufreq/nvidia,tegra20-cpufreq.txt        | 38 +++++++++++++++++++
>>>>>>>>  1 file changed, 38 insertions(+)
>>>>>>>>  create mode 100644 Documentation/devicetree/bindings/cpufreq/nvidia,tegra20-cpufreq.txt
>>>>>>>>
>>>>>>>> diff --git a/Documentation/devicetree/bindings/cpufreq/nvidia,tegra20-cpufreq.txt b/Documentation/devicetree/bindings/cpufreq/nvidia,tegra20-cpufreq.txt
>>>>>>>> new file mode 100644
>>>>>>>> index 000000000000..2c51f676e958
>>>>>>>> --- /dev/null
>>>>>>>> +++ b/Documentation/devicetree/bindings/cpufreq/nvidia,tegra20-cpufreq.txt
>>>>>>>> @@ -0,0 +1,38 @@
>>>>>>>> +Binding for NVIDIA Tegra20 CPUFreq
>>>>>>>> +==================================
>>>>>>>> +
>>>>>>>> +Required properties:
>>>>>>>> +- clocks: Must contain an entry for each entry in clock-names.
>>>>>>>> +  See ../clocks/clock-bindings.txt for details.
>>>>>>>> +- clock-names: Must include the following entries:
>>>>>>>> +  - pll_x: main-parent for CPU clock, must be the first entry
>>>>>>>> +  - backup: intermediate-parent for CPU clock
>>>>>>>> +  - cpu: the CPU clock
>>>>>>>
>>>>>>> Is it likely that 'backup' will be anything other that pll_p? If not why
>>>>>>> not just call it pll_p? Personally, I don't 'backup' to descriptive even
>>>>>>> though I can see what you mean.
>>>>>>>
>>>>>>> I can see that you want to make this flexible, but if the likelihood is
>>>>>>> that we will just use pll_p then I am not sure it is warranted at this
>>>>>>> point.
>>>>>>
>>>>>> That won't describe HW, but software. And device tree should describe HW.
>>>>>
>>>>> Hmm ... well that's my point exactly. So why call it 'backup'? Sounds
>>>>> like a software description to me.
>>>>
>>>> Because HW is designed the way that CPU parent need to be switched to the backup clock source while main clock changes its rate. HW also allow to select among different parents, pll_p is one of those parents.
>>>
>>> Yes that part is understood. I am just splitting hairs over the actual
>>> name. We do the same for tegra124 but we just call it 'pll_p'. See ...
>>>
>>> Documentation/devicetree/bindings/cpufreq/nvidia,tegra124-cpufreq.txt
>>
>> tegra124-cpufreq choose to hardwire to the pll_p, but it could be other clocks. Technically abstracting backup clock should be more correct, but result is the same in the end.
> 
> Yes and that is probably intentional as that is the configuration that
> has been validated. So unless we are planning to test and verify every
> possibility, my preference is to keep it simple. But yes the result is
> the same.
> 
> I was simply curious of your intention here because of the other series
> you posted with regard to the cpu clocks.

Actually I tried other parents on T30 and they worked with no problems. The intention of having 'backup' is to describe HW properly in the device tree, that's it. We'll have backup set to pll_p by default. ACK?

Re: [PATCH v1 1/5] dt-bindings: cpufreq: Add binding for NVIDIA Tegra20/30

[Jon Hunter]

On 17/10/2018 15:43, Dmitry Osipenko wrote:
> On 10/17/18 5:14 PM, Jon Hunter wrote:
>>
>> On 17/10/2018 14:46, Dmitry Osipenko wrote:
>>> On 10/17/18 4:34 PM, Jon Hunter wrote:
>>>>
>>>> On 17/10/2018 14:07, Dmitry Osipenko wrote:
>>>>> On 10/17/18 3:59 PM, Jon Hunter wrote:
>>>>>>
>>>>>> On 17/10/2018 13:37, Dmitry Osipenko wrote:
>>>>>>> On 10/17/18 11:40 AM, Jon Hunter wrote:
>>>>>>>>
>>>>>>>> On 30/08/2018 20:43, Dmitry Osipenko wrote:
>>>>>>>>> Add device-tree binding that describes CPU frequency-scaling hardware
>>>>>>>>> found on NVIDIA Tegra20/30 SoC's.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>>>>>>>>> ---
>>>>>>>>>  .../cpufreq/nvidia,tegra20-cpufreq.txt        | 38 +++++++++++++++++++
>>>>>>>>>  1 file changed, 38 insertions(+)
>>>>>>>>>  create mode 100644 Documentation/devicetree/bindings/cpufreq/nvidia,tegra20-cpufreq.txt
>>>>>>>>>
>>>>>>>>> diff --git a/Documentation/devicetree/bindings/cpufreq/nvidia,tegra20-cpufreq.txt b/Documentation/devicetree/bindings/cpufreq/nvidia,tegra20-cpufreq.txt
>>>>>>>>> new file mode 100644
>>>>>>>>> index 000000000000..2c51f676e958
>>>>>>>>> --- /dev/null
>>>>>>>>> +++ b/Documentation/devicetree/bindings/cpufreq/nvidia,tegra20-cpufreq.txt
>>>>>>>>> @@ -0,0 +1,38 @@
>>>>>>>>> +Binding for NVIDIA Tegra20 CPUFreq
>>>>>>>>> +==================================
>>>>>>>>> +
>>>>>>>>> +Required properties:
>>>>>>>>> +- clocks: Must contain an entry for each entry in clock-names.
>>>>>>>>> +  See ../clocks/clock-bindings.txt for details.
>>>>>>>>> +- clock-names: Must include the following entries:
>>>>>>>>> +  - pll_x: main-parent for CPU clock, must be the first entry
>>>>>>>>> +  - backup: intermediate-parent for CPU clock
>>>>>>>>> +  - cpu: the CPU clock
>>>>>>>>
>>>>>>>> Is it likely that 'backup' will be anything other that pll_p? If not why
>>>>>>>> not just call it pll_p? Personally, I don't 'backup' to descriptive even
>>>>>>>> though I can see what you mean.
>>>>>>>>
>>>>>>>> I can see that you want to make this flexible, but if the likelihood is
>>>>>>>> that we will just use pll_p then I am not sure it is warranted at this
>>>>>>>> point.
>>>>>>>
>>>>>>> That won't describe HW, but software. And device tree should describe HW.
>>>>>>
>>>>>> Hmm ... well that's my point exactly. So why call it 'backup'? Sounds
>>>>>> like a software description to me.
>>>>>
>>>>> Because HW is designed the way that CPU parent need to be switched to the backup clock source while main clock changes its rate. HW also allow to select among different parents, pll_p is one of those parents.
>>>>
>>>> Yes that part is understood. I am just splitting hairs over the actual
>>>> name. We do the same for tegra124 but we just call it 'pll_p'. See ...
>>>>
>>>> Documentation/devicetree/bindings/cpufreq/nvidia,tegra124-cpufreq.txt
>>>
>>> tegra124-cpufreq choose to hardwire to the pll_p, but it could be other clocks. Technically abstracting backup clock should be more correct, but result is the same in the end.
>>
>> Yes and that is probably intentional as that is the configuration that
>> has been validated. So unless we are planning to test and verify every
>> possibility, my preference is to keep it simple. But yes the result is
>> the same.
>>
>> I was simply curious of your intention here because of the other series
>> you posted with regard to the cpu clocks.
> 
> Actually I tried other parents on T30 and they worked with no problems. The intention of having 'backup' is to describe HW properly in the device tree, that's it. We'll have backup set to pll_p by default. ACK?

Yes but I don't find the name 'backup' very descriptive because like you
say it is an intermediate clock for switching frequency. But at the same
time I don't have a good suggestion. Anyway it is more of a
bike-shedding comment.

Cheers
Jon

-- 
nvpublic

Re: [PATCH v1 1/5] dt-bindings: cpufreq: Add binding for NVIDIA Tegra20/30

[Dmitry Osipenko]

On 10/17/18 10:29 PM, Jon Hunter wrote:
> 
> On 17/10/2018 15:43, Dmitry Osipenko wrote:
>> On 10/17/18 5:14 PM, Jon Hunter wrote:
>>>
>>> On 17/10/2018 14:46, Dmitry Osipenko wrote:
>>>> On 10/17/18 4:34 PM, Jon Hunter wrote:
>>>>>
>>>>> On 17/10/2018 14:07, Dmitry Osipenko wrote:
>>>>>> On 10/17/18 3:59 PM, Jon Hunter wrote:
>>>>>>>
>>>>>>> On 17/10/2018 13:37, Dmitry Osipenko wrote:
>>>>>>>> On 10/17/18 11:40 AM, Jon Hunter wrote:
>>>>>>>>>
>>>>>>>>> On 30/08/2018 20:43, Dmitry Osipenko wrote:
>>>>>>>>>> Add device-tree binding that describes CPU frequency-scaling hardware
>>>>>>>>>> found on NVIDIA Tegra20/30 SoC's.
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>>>>>>>>>> ---
>>>>>>>>>>  .../cpufreq/nvidia,tegra20-cpufreq.txt        | 38 +++++++++++++++++++
>>>>>>>>>>  1 file changed, 38 insertions(+)
>>>>>>>>>>  create mode 100644 Documentation/devicetree/bindings/cpufreq/nvidia,tegra20-cpufreq.txt
>>>>>>>>>>
>>>>>>>>>> diff --git a/Documentation/devicetree/bindings/cpufreq/nvidia,tegra20-cpufreq.txt b/Documentation/devicetree/bindings/cpufreq/nvidia,tegra20-cpufreq.txt
>>>>>>>>>> new file mode 100644
>>>>>>>>>> index 000000000000..2c51f676e958
>>>>>>>>>> --- /dev/null
>>>>>>>>>> +++ b/Documentation/devicetree/bindings/cpufreq/nvidia,tegra20-cpufreq.txt
>>>>>>>>>> @@ -0,0 +1,38 @@
>>>>>>>>>> +Binding for NVIDIA Tegra20 CPUFreq
>>>>>>>>>> +==================================
>>>>>>>>>> +
>>>>>>>>>> +Required properties:
>>>>>>>>>> +- clocks: Must contain an entry for each entry in clock-names.
>>>>>>>>>> +  See ../clocks/clock-bindings.txt for details.
>>>>>>>>>> +- clock-names: Must include the following entries:
>>>>>>>>>> +  - pll_x: main-parent for CPU clock, must be the first entry
>>>>>>>>>> +  - backup: intermediate-parent for CPU clock
>>>>>>>>>> +  - cpu: the CPU clock
>>>>>>>>>
>>>>>>>>> Is it likely that 'backup' will be anything other that pll_p? If not why
>>>>>>>>> not just call it pll_p? Personally, I don't 'backup' to descriptive even
>>>>>>>>> though I can see what you mean.
>>>>>>>>>
>>>>>>>>> I can see that you want to make this flexible, but if the likelihood is
>>>>>>>>> that we will just use pll_p then I am not sure it is warranted at this
>>>>>>>>> point.
>>>>>>>>
>>>>>>>> That won't describe HW, but software. And device tree should describe HW.
>>>>>>>
>>>>>>> Hmm ... well that's my point exactly. So why call it 'backup'? Sounds
>>>>>>> like a software description to me.
>>>>>>
>>>>>> Because HW is designed the way that CPU parent need to be switched to the backup clock source while main clock changes its rate. HW also allow to select among different parents, pll_p is one of those parents.
>>>>>
>>>>> Yes that part is understood. I am just splitting hairs over the actual
>>>>> name. We do the same for tegra124 but we just call it 'pll_p'. See ...
>>>>>
>>>>> Documentation/devicetree/bindings/cpufreq/nvidia,tegra124-cpufreq.txt
>>>>
>>>> tegra124-cpufreq choose to hardwire to the pll_p, but it could be other clocks. Technically abstracting backup clock should be more correct, but result is the same in the end.
>>>
>>> Yes and that is probably intentional as that is the configuration that
>>> has been validated. So unless we are planning to test and verify every
>>> possibility, my preference is to keep it simple. But yes the result is
>>> the same.
>>>
>>> I was simply curious of your intention here because of the other series
>>> you posted with regard to the cpu clocks.
>>
>> Actually I tried other parents on T30 and they worked with no problems. The intention of having 'backup' is to describe HW properly in the device tree, that's it. We'll have backup set to pll_p by default. ACK?
> 
> Yes but I don't find the name 'backup' very descriptive because like you
> say it is an intermediate clock for switching frequency. But at the same
> time I don't have a good suggestion. Anyway it is more of a
> bike-shedding comment.

I'll consider renaming 'backup' to 'intermediate' in the v2 which I'll probably manage to send out tomorrow as RFC (CPUFreq now depends on yet-not-reviewed coupled-regulators patches).