[PATCH v3 00/11] Tegra210 DFLL implementation

[PATCH v3 00/11] Tegra210 DFLL implementation

[Peter De Schrijver]

This series introduces support for the DFLL as a CPU clock source
on Tegra210. As Jetson TX2 uses a PWM controlled regulator IC which
is driven directly by the DFLLs PWM output, we also introduce support
for PWM regulators next to I2C controlled regulators. The DFLL output
frequency is directly controlled by the regulator voltage. The registers                                                                                       for controlling the PWM are part of the DFLL IP block, so there's no
separate linux regulator object involved because the regulator IC only                                                                                         supplies the rail powering the DFLL and the CPUs. It doesn't have any
other controls.

Changes since v2:
* added DT updates for Tegra210
* updated dfll DT binding documentation
* split changes to i2c support into its own patch
* retrieve regulator parameters from framework rather than from CVB table
* bug fixes

Changes since v1:
* improved commit messages
* some style cleanups                                                                                                                                           
Laxman Dewangan (1):
  regulator: core: add API to get voltage constraints

Peter De Schrijver (10):
  clk: tegra: retrieve regulator info from framework
  clk: tegra: dfll registration for multiple SoCs
  clk: tegra: add CVB tables for Tegra210 CPU DFLL
  clk: tegra: prepare dfll driver for PWM regulator
  clk: tegra: dfll: support PWM regulator control
  dt-bindings: tegra: Update DFLL binding for PWM regulator
  clk: tegra: build clk-dfll.c for Tegra124 and Tegra210
  cpufreq: tegra124-cpufreq: extend to support Tegra210
  arm64: dts: tegra: Add Tegra210 DFLL definition
  arm64: dts: nvidia: Tegra210 CPU clock definition

 .../bindings/clock/nvidia,tegra124-dfll.txt        |  76 ++-
 arch/arm64/boot/dts/nvidia/tegra210-p2180.dtsi     |  18 +
 arch/arm64/boot/dts/nvidia/tegra210-p2597.dtsi     |  12 +
 arch/arm64/boot/dts/nvidia/tegra210.dtsi           |  26 +
 drivers/clk/tegra/Kconfig                          |   5 +
 drivers/clk/tegra/Makefile                         |   2 +-
 drivers/clk/tegra/clk-dfll.c                       | 462 +++++++++++++++---
 drivers/clk/tegra/clk-dfll.h                       |   2 +
 drivers/clk/tegra/clk-tegra124-dfll-fcpu.c         | 526 ++++++++++++++++++++-
 drivers/clk/tegra/cvb.c                            |  16 +-
 drivers/clk/tegra/cvb.h                            |   7 +-
 drivers/cpufreq/tegra124-cpufreq.c                 |  15 +-
 drivers/regulator/core.c                           |  31 ++
 include/linux/regulator/consumer.h                 |   2 +
 14 files changed, 1106 insertions(+), 94 deletions(-)

-- 
1.9.1

[PATCH v3 01/11] regulator: core: add API to get voltage constraints

[Peter De Schrijver]

From: Laxman Dewangan <ldewangan@nvidia.com>

Add API to get min/max rail voltage configured from platform for
given rails.

Changes to the commit message by
Peter De Schrijver <pdeschrijver@nvidia.com>

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
 drivers/regulator/core.c           | 31 +++++++++++++++++++++++++++++++
 include/linux/regulator/consumer.h |  2 ++
 2 files changed, 33 insertions(+)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index be767dd..c498774 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -3246,6 +3246,37 @@ int regulator_get_voltage(struct regulator *regulator)
 EXPORT_SYMBOL_GPL(regulator_get_voltage);
 
 /**
+ * regulator_get_constraint_voltages - get platform specific constraint voltage,
+ * @regulator: regulator source
+ * @min_uV: Minimum microvolts.
+ * @max_uV: Maximum microvolts.
+ *
+ * This returns the current regulator voltage in uV.
+ *
+ * NOTE: If the regulator is disabled it will return the voltage value. This
+ * function should not be used to determine regulator state.
+ */
+
+int regulator_get_constraint_voltages(struct regulator *regulator,
+	int *min_uV, int *max_uV)
+{
+	struct regulator_dev *rdev = regulator->rdev;
+
+	if (rdev->desc && rdev->desc->fixed_uV && rdev->desc->n_voltages == 1) {
+		*min_uV = rdev->desc->fixed_uV;
+		*max_uV = rdev->desc->fixed_uV;
+		return 0;
+	}
+	if (rdev->constraints) {
+		*min_uV = rdev->constraints->min_uV;
+		*max_uV = rdev->constraints->max_uV;
+		return 0;
+	}
+	return -EINVAL;
+}
+EXPORT_SYMBOL_GPL(regulator_get_constraint_voltages);
+
+/**
  * regulator_set_current_limit - set regulator output current limit
  * @regulator: regulator source
  * @min_uA: Minimum supported current in uA
diff --git a/include/linux/regulator/consumer.h b/include/linux/regulator/consumer.h
index df176d7..d3f495a 100644
--- a/include/linux/regulator/consumer.h
+++ b/include/linux/regulator/consumer.h
@@ -250,6 +250,8 @@ int regulator_is_supported_voltage(struct regulator *regulator,
 int regulator_set_voltage_time(struct regulator *regulator,
 			       int old_uV, int new_uV);
 int regulator_get_voltage(struct regulator *regulator);
+int regulator_get_constraint_voltages(struct regulator *regulator,
+	int *min_uV, int *max_uV);
 int regulator_sync_voltage(struct regulator *regulator);
 int regulator_set_current_limit(struct regulator *regulator,
 			       int min_uA, int max_uA);
-- 
1.9.1

[PATCH v3 02/11] clk: tegra: retrieve regulator info from framework

[Peter De Schrijver]

The CVB table contains calibration data for the CPU DFLL based on
process charaterization. The regulator step and offset parameters depend
on the regulator supplying vdd-cpu , not on the specific Tegra SKU.
Hence than hardcoding those regulator parameters in the CVB table,
retrieve them from the regulator framework and store them as part of the
tegra_dfll_soc_data struct.

Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
---
 drivers/clk/tegra/clk-dfll.h               |  2 ++
 drivers/clk/tegra/clk-tegra124-dfll-fcpu.c | 42 +++++++++++++++++++++++++-----
 drivers/clk/tegra/cvb.c                    | 16 +++++++++---
 drivers/clk/tegra/cvb.h                    |  6 ++---
 4 files changed, 53 insertions(+), 13 deletions(-)

diff --git a/drivers/clk/tegra/clk-dfll.h b/drivers/clk/tegra/clk-dfll.h
index 83352c8..e7cbc5b 100644
--- a/drivers/clk/tegra/clk-dfll.h
+++ b/drivers/clk/tegra/clk-dfll.h
@@ -21,6 +21,7 @@
 #include <linux/platform_device.h>
 #include <linux/reset.h>
 #include <linux/types.h>
+#include "cvb.h"
 
 /**
  * struct tegra_dfll_soc_data - SoC-specific hooks/integration for the DFLL driver
@@ -35,6 +36,7 @@ struct tegra_dfll_soc_data {
 	struct device *dev;
 	unsigned long max_freq;
 	const struct cvb_table *cvb;
+	struct rail_alignment alignment;
 
 	void (*init_clock_trimmers)(void);
 	void (*set_clock_trimmers_high)(void);
diff --git a/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c b/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c
index 269d359..e2dbb79 100644
--- a/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c
+++ b/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c
@@ -22,6 +22,7 @@
 #include <linux/kernel.h>
 #include <linux/init.h>
 #include <linux/platform_device.h>
+#include <linux/regulator/consumer.h>
 #include <soc/tegra/fuse.h>
 
 #include "clk.h"
@@ -42,9 +43,6 @@
 		.process_id = -1,
 		.min_millivolts = 900,
 		.max_millivolts = 1260,
-		.alignment = {
-			.step_uv = 10000, /* 10mV */
-		},
 		.speedo_scale = 100,
 		.voltage_scale = 1000,
 		.entries = {
@@ -82,6 +80,34 @@
 	},
 };
 
+static int get_alignment_from_regulator(struct device *dev,
+					struct rail_alignment *align)
+{
+	int min_uV, max_uV, n_voltages, ret;
+	struct regulator *reg;
+
+	reg = devm_regulator_get(dev, "vdd-cpu");
+	if (IS_ERR(reg))
+		return PTR_ERR(reg);
+
+	ret = regulator_get_constraint_voltages(reg, &min_uV, &max_uV);
+	if (!ret)
+		align->offset_uv = min_uV;
+	else
+		return ret;
+
+	align->step_uv = regulator_get_linear_step(reg);
+	if (!align->step_uv && !ret) {
+		n_voltages = regulator_count_voltages(reg);
+		if (n_voltages > 1)
+			align->step_uv = (max_uV - min_uV) / (n_voltages - 1);
+	}
+
+	devm_regulator_put(reg);
+
+	return 0;
+}
+
 static int tegra124_dfll_fcpu_probe(struct platform_device *pdev)
 {
 	int process_id, speedo_id, speedo_value, err;
@@ -108,10 +134,14 @@ static int tegra124_dfll_fcpu_probe(struct platform_device *pdev)
 	}
 
 	soc->max_freq = cpu_max_freq_table[speedo_id];
+	err = get_alignment_from_regulator(&pdev->dev, &soc->alignment);
+	if (err < 0)
+		return err;
 
-	soc->cvb = tegra_cvb_add_opp_table(soc->dev, tegra124_cpu_cvb_tables,
-					   ARRAY_SIZE(tegra124_cpu_cvb_tables),
-					   process_id, speedo_id, speedo_value,
+	soc->cvb = tegra_cvb_add_opp_table(soc->dev, fcpu_data->cpu_cvb_tables,
+					   fcpu_data->cpu_cvb_tables_size,
+					   &soc->alignment, process_id,
+					   speedo_id, speedo_value,
 					   soc->max_freq);
 	if (IS_ERR(soc->cvb)) {
 		dev_err(&pdev->dev, "couldn't add OPP table: %ld\n",
diff --git a/drivers/clk/tegra/cvb.c b/drivers/clk/tegra/cvb.c
index da9e8e7..2aa9639 100644
--- a/drivers/clk/tegra/cvb.c
+++ b/drivers/clk/tegra/cvb.c
@@ -62,11 +62,17 @@ static int round_voltage(int mv, const struct rail_alignment *align, int up)
 }
 
 static int build_opp_table(struct device *dev, const struct cvb_table *table,
+			   struct rail_alignment *align,
 			   int speedo_value, unsigned long max_freq)
 {
-	const struct rail_alignment *align = &table->alignment;
 	int i, ret, dfll_mv, min_mv, max_mv;
 
+	if (!align->step_uv)
+		return -EINVAL;
+
+	if (!align->offset_uv)
+		return -EINVAL;
+
 	min_mv = round_voltage(table->min_millivolts, align, UP);
 	max_mv = round_voltage(table->max_millivolts, align, DOWN);
 
@@ -109,8 +115,9 @@ static int build_opp_table(struct device *dev, const struct cvb_table *table,
  */
 const struct cvb_table *
 tegra_cvb_add_opp_table(struct device *dev, const struct cvb_table *tables,
-			size_t count, int process_id, int speedo_id,
-			int speedo_value, unsigned long max_freq)
+			size_t count, struct rail_alignment *align,
+			int process_id, int speedo_id, int speedo_value,
+			unsigned long max_freq)
 {
 	size_t i;
 	int ret;
@@ -124,7 +131,8 @@ static int build_opp_table(struct device *dev, const struct cvb_table *table,
 		if (table->process_id != -1 && table->process_id != process_id)
 			continue;
 
-		ret = build_opp_table(dev, table, speedo_value, max_freq);
+		ret = build_opp_table(dev, table, align, speedo_value,
+				      max_freq);
 		return ret ? ERR_PTR(ret) : table;
 	}
 
diff --git a/drivers/clk/tegra/cvb.h b/drivers/clk/tegra/cvb.h
index c1f0779..bcf15a0 100644
--- a/drivers/clk/tegra/cvb.h
+++ b/drivers/clk/tegra/cvb.h
@@ -49,7 +49,6 @@ struct cvb_table {
 
 	int min_millivolts;
 	int max_millivolts;
-	struct rail_alignment alignment;
 
 	int speedo_scale;
 	int voltage_scale;
@@ -59,8 +58,9 @@ struct cvb_table {
 
 const struct cvb_table *
 tegra_cvb_add_opp_table(struct device *dev, const struct cvb_table *cvb_tables,
-			size_t count, int process_id, int speedo_id,
-			int speedo_value, unsigned long max_freq);
+			size_t count, struct rail_alignment *align,
+			int process_id, int speedo_id, int speedo_value,
+			unsigned long max_freq);
 void tegra_cvb_remove_opp_table(struct device *dev,
 				const struct cvb_table *table,
 				unsigned long max_freq);
-- 
1.9.1

[PATCH v3 03/11] clk: tegra: dfll registration for multiple SoCs

[Peter De Schrijver]

In a future patch, support for the DFLL in Tegra210 will be introduced.
This requires support for more than 1 set of CVB and CPU max frequency
tables.

Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
---
 drivers/clk/tegra/clk-tegra124-dfll-fcpu.c | 37 ++++++++++++++++++++++++------
 1 file changed, 30 insertions(+), 7 deletions(-)

diff --git a/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c b/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c
index e2dbb79..6486ad9 100644
--- a/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c
+++ b/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c
@@ -21,6 +21,7 @@
 #include <linux/err.h>
 #include <linux/kernel.h>
 #include <linux/init.h>
+#include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <linux/regulator/consumer.h>
 #include <soc/tegra/fuse.h>
@@ -29,8 +30,15 @@
 #include "clk-dfll.h"
 #include "cvb.h"
 
+struct dfll_fcpu_data {
+	const unsigned long *cpu_max_freq_table;
+	unsigned int cpu_max_freq_table_size;
+	const struct cvb_table *cpu_cvb_tables;
+	unsigned int cpu_cvb_tables_size;
+};
+
 /* Maximum CPU frequency, indexed by CPU speedo id */
-static const unsigned long cpu_max_freq_table[] = {
+static const unsigned long tegra124_cpu_max_freq_table[] = {
 	[0] = 2014500000UL,
 	[1] = 2320500000UL,
 	[2] = 2116500000UL,
@@ -80,6 +88,21 @@
 	},
 };
 
+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),
+	.cpu_cvb_tables = tegra124_cpu_cvb_tables,
+	.cpu_cvb_tables_size = ARRAY_SIZE(tegra124_cpu_cvb_tables)
+};
+
+static const struct of_device_id tegra124_dfll_fcpu_of_match[] = {
+	{
+		.compatible = "nvidia,tegra124-dfll",
+		.data = &tegra124_dfll_fcpu_data,
+	},
+	{ },
+};
+
 static int get_alignment_from_regulator(struct device *dev,
 					struct rail_alignment *align)
 {
@@ -112,12 +135,17 @@ static int tegra124_dfll_fcpu_probe(struct platform_device *pdev)
 {
 	int process_id, speedo_id, speedo_value, err;
 	struct tegra_dfll_soc_data *soc;
+	const struct of_device_id *of_id;
+	const struct dfll_fcpu_data *fcpu_data;
+
+	of_id = of_match_device(tegra124_dfll_fcpu_of_match, &pdev->dev);
+	fcpu_data = of_id->data;
 
 	process_id = tegra_sku_info.cpu_process_id;
 	speedo_id = tegra_sku_info.cpu_speedo_id;
 	speedo_value = tegra_sku_info.cpu_speedo_value;
 
-	if (speedo_id >= ARRAY_SIZE(cpu_max_freq_table)) {
+	if (speedo_id >= fcpu_data->cpu_max_freq_table_size) {
 		dev_err(&pdev->dev, "unknown max CPU freq for speedo_id=%d\n",
 			speedo_id);
 		return -ENODEV;
@@ -172,11 +200,6 @@ static int tegra124_dfll_fcpu_remove(struct platform_device *pdev)
 	return 0;
 }
 
-static const struct of_device_id tegra124_dfll_fcpu_of_match[] = {
-	{ .compatible = "nvidia,tegra124-dfll", },
-	{ },
-};
-
 static const struct dev_pm_ops tegra124_dfll_pm_ops = {
 	SET_RUNTIME_PM_OPS(tegra_dfll_runtime_suspend,
 			   tegra_dfll_runtime_resume, NULL)
-- 
1.9.1

[PATCH v3 04/11] clk: tegra: add CVB tables for Tegra210 CPU DFLL

[Peter De Schrijver]

Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
---
 drivers/clk/tegra/clk-tegra124-dfll-fcpu.c | 426 +++++++++++++++++++++++++++++
 drivers/clk/tegra/cvb.h                    |   1 +
 2 files changed, 427 insertions(+)

diff --git a/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c b/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c
index 6486ad9..78dddab 100644
--- a/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c
+++ b/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c
@@ -88,6 +88,421 @@ struct dfll_fcpu_data {
 	},
 };
 
+static const unsigned long tegra210_cpu_max_freq_table[] = {
+	[0] = 1912500000UL,
+	[1] = 1912500000UL,
+	[2] = 2218500000UL,
+	[3] = 1785000000UL,
+	[4] = 1632000000UL,
+	[5] = 1912500000UL,
+	[6] = 2014500000UL,
+	[7] = 1734000000UL,
+	[8] = 1683000000UL,
+	[9] = 1555500000UL,
+	[10] = 1504500000UL,
+};
+
+#define CPU_CVB_TABLE \
+	.speedo_scale = 100,	\
+	.voltage_scale = 1000,	\
+	.entries = {		\
+		{204000000UL,	{1007452, -23865, 370} }, \
+		{306000000UL,	{1052709, -24875, 370} }, \
+		{408000000UL,	{1099069, -25895, 370} }, \
+		{510000000UL,	{1146534, -26905, 370} }, \
+		{612000000UL,	{1195102, -27915, 370} }, \
+		{714000000UL,	{1244773, -28925, 370} }, \
+		{816000000UL,	{1295549, -29935, 370} }, \
+		{918000000UL,	{1347428, -30955, 370} }, \
+		{1020000000UL,	{1400411, -31965, 370} }, \
+		{1122000000UL,	{1454497, -32975, 370} }, \
+		{1224000000UL,	{1509687, -33985, 370} }, \
+		{1326000000UL,	{1565981, -35005, 370} }, \
+		{1428000000UL,	{1623379, -36015, 370} }, \
+		{1530000000UL,	{1681880, -37025, 370} }, \
+		{1632000000UL,	{1741485, -38035, 370} }, \
+		{1734000000UL,	{1802194, -39055, 370} }, \
+		{1836000000UL,	{1864006, -40065, 370} }, \
+		{1912500000UL,	{1910780, -40815, 370} }, \
+		{2014500000UL,	{1227000,      0,   0} }, \
+		{2218500000UL,	{1227000,      0,   0} }, \
+		{0,		{      0,      0,   0} }, \
+	}
+
+#define CPU_CVB_TABLE_XA \
+	.speedo_scale = 100,	\
+	.voltage_scale = 1000,	\
+	.entries = {		\
+		{204000000UL,	{1250024, -39785, 565} }, \
+		{306000000UL,	{1297556, -41145, 565} }, \
+		{408000000UL,	{1346718, -42505, 565} }, \
+		{510000000UL,	{1397511, -43855, 565} }, \
+		{612000000UL,	{1449933, -45215, 565} }, \
+		{714000000UL,	{1503986, -46575, 565} }, \
+		{816000000UL,	{1559669, -47935, 565} }, \
+		{918000000UL,	{1616982, -49295, 565} }, \
+		{1020000000UL,	{1675926, -50645, 565} }, \
+		{1122000000UL,	{1736500, -52005, 565} }, \
+		{1224000000UL,	{1798704, -53365, 565} }, \
+		{1326000000UL,	{1862538, -54725, 565} }, \
+		{1428000000UL,	{1928003, -56085, 565} }, \
+		{1530000000UL,	{1995097, -57435, 565} }, \
+		{1606500000UL,	{2046149, -58445, 565} }, \
+		{1632000000UL,	{2063822, -58795, 565} }, \
+		{0,		{      0,      0,   0} }, \
+	}
+
+#define CPU_CVB_TABLE_EUCM1 \
+	.speedo_scale = 100,	\
+	.voltage_scale = 1000,	\
+	.entries = {		\
+		{204000000UL,	{734429, 0, 0} }, \
+		{306000000UL,	{768191, 0, 0} }, \
+		{408000000UL,	{801953, 0, 0} }, \
+		{510000000UL,	{835715, 0, 0} }, \
+		{612000000UL,	{869477, 0, 0} }, \
+		{714000000UL,	{903239, 0, 0} }, \
+		{816000000UL,	{937001, 0, 0} }, \
+		{918000000UL,	{970763, 0, 0} }, \
+		{1020000000UL,	{1004525, 0, 0} }, \
+		{1122000000UL,	{1038287, 0, 0} }, \
+		{1224000000UL,	{1072049, 0, 0} }, \
+		{1326000000UL,	{1105811, 0, 0} }, \
+		{1428000000UL,	{1130000, 0, 0} }, \
+		{1555500000UL,	{1130000, 0, 0} }, \
+		{1632000000UL,	{1170000, 0, 0} }, \
+		{1734000000UL,	{1227500, 0, 0} }, \
+		{0,		{      0, 0, 0} }, \
+	}
+
+#define CPU_CVB_TABLE_EUCM2 \
+	.speedo_scale = 100,	\
+	.voltage_scale = 1000,	\
+	.entries = {		\
+		{204000000UL,	{742283, 0, 0} }, \
+		{306000000UL,	{776249, 0, 0} }, \
+		{408000000UL,	{810215, 0, 0} }, \
+		{510000000UL,	{844181, 0, 0} }, \
+		{612000000UL,	{878147, 0, 0} }, \
+		{714000000UL,	{912113, 0, 0} }, \
+		{816000000UL,	{946079, 0, 0} }, \
+		{918000000UL,	{980045, 0, 0} }, \
+		{1020000000UL,	{1014011, 0, 0} }, \
+		{1122000000UL,	{1047977, 0, 0} }, \
+		{1224000000UL,	{1081943, 0, 0} }, \
+		{1326000000UL,	{1090000, 0, 0} }, \
+		{1479000000UL,	{1090000, 0, 0} }, \
+		{1555500000UL,	{1162000, 0, 0} }, \
+		{1683000000UL,	{1195000, 0, 0} }, \
+		{0,		{      0, 0, 0} }, \
+	}
+
+#define CPU_CVB_TABLE_EUCM2_JOINT_RAIL \
+	.speedo_scale = 100,	\
+	.voltage_scale = 1000,	\
+	.entries = {		\
+		{204000000UL,	{742283, 0, 0} }, \
+		{306000000UL,	{776249, 0, 0} }, \
+		{408000000UL,	{810215, 0, 0} }, \
+		{510000000UL,	{844181, 0, 0} }, \
+		{612000000UL,	{878147, 0, 0} }, \
+		{714000000UL,	{912113, 0, 0} }, \
+		{816000000UL,	{946079, 0, 0} }, \
+		{918000000UL,	{980045, 0, 0} }, \
+		{1020000000UL,	{1014011, 0, 0} }, \
+		{1122000000UL,	{1047977, 0, 0} }, \
+		{1224000000UL,	{1081943, 0, 0} }, \
+		{1326000000UL,	{1090000, 0, 0} }, \
+		{1479000000UL,	{1090000, 0, 0} }, \
+		{1504500000UL,	{1120000, 0, 0} }, \
+		{0,		{      0, 0, 0} }, \
+	}
+
+#define CPU_CVB_TABLE_ODN \
+	.speedo_scale = 100,	\
+	.voltage_scale = 1000,	\
+	.entries = {		\
+		{204000000UL,	{721094, 0, 0} }, \
+		{306000000UL,	{754040, 0, 0} }, \
+		{408000000UL,	{786986, 0, 0} }, \
+		{510000000UL,	{819932, 0, 0} }, \
+		{612000000UL,	{852878, 0, 0} }, \
+		{714000000UL,	{885824, 0, 0} }, \
+		{816000000UL,	{918770, 0, 0} }, \
+		{918000000UL,	{915716, 0, 0} }, \
+		{1020000000UL,	{984662, 0, 0} }, \
+		{1122000000UL,	{1017608, 0, 0} }, \
+		{1224000000UL,	{1050554, 0, 0} }, \
+		{1326000000UL,	{1083500, 0, 0} }, \
+		{1428000000UL,	{1116446, 0, 0} }, \
+		{1581000000UL,	{1130000, 0, 0} }, \
+		{1683000000UL,	{1168000, 0, 0} }, \
+		{1785000000UL,	{1227500, 0, 0} }, \
+		{0,		{      0, 0, 0} }, \
+	}
+
+struct cvb_table tegra210_cpu_cvb_tables[] = {
+	{
+		.speedo_id = 10,
+		.process_id = 0,
+		.min_millivolts = 840,
+		.max_millivolts = 1120,
+		CPU_CVB_TABLE_EUCM2_JOINT_RAIL,
+		.cpu_dfll_data = {
+			.tune0_low = 0xffead0ff,
+			.tune0_high = 0xffead0ff,
+			.tune1 = 0x20091d9,
+			.tune_high_min_millivolts = 864,
+		}
+	},
+	{
+		.speedo_id = 10,
+		.process_id = 1,
+		.min_millivolts = 840,
+		.max_millivolts = 1120,
+		CPU_CVB_TABLE_EUCM2_JOINT_RAIL,
+		.cpu_dfll_data = {
+			.tune0_low = 0xffead0ff,
+			.tune0_high = 0xffead0ff,
+			.tune1 = 0x20091d9,
+			.tune_high_min_millivolts = 864,
+		}
+	},
+	{
+		.speedo_id = 9,
+		.process_id = 0,
+		.min_millivolts = 900,
+		.max_millivolts = 1162,
+		CPU_CVB_TABLE_EUCM2,
+		.cpu_dfll_data = {
+			.tune0_low = 0xffead0ff,
+			.tune0_high = 0xffead0ff,
+			.tune1 = 0x20091d9,
+		}
+	},
+	{
+		.speedo_id = 9,
+		.process_id = 1,
+		.min_millivolts = 900,
+		.max_millivolts = 1162,
+		CPU_CVB_TABLE_EUCM2,
+		.cpu_dfll_data = {
+			.tune0_low = 0xffead0ff,
+			.tune0_high = 0xffead0ff,
+			.tune1 = 0x20091d9,
+		}
+	},
+	{
+		.speedo_id = 8,
+		.process_id = 0,
+		.min_millivolts = 900,
+		.max_millivolts = 1195,
+		CPU_CVB_TABLE_EUCM2,
+		.cpu_dfll_data = {
+			.tune0_low = 0xffead0ff,
+			.tune0_high = 0xffead0ff,
+			.tune1 = 0x20091d9,
+		}
+	},
+	{
+		.speedo_id = 8,
+		.process_id = 1,
+		.min_millivolts = 900,
+		.max_millivolts = 1195,
+		CPU_CVB_TABLE_EUCM2,
+		.cpu_dfll_data = {
+			.tune0_low = 0xffead0ff,
+			.tune0_high = 0xffead0ff,
+			.tune1 = 0x20091d9,
+		}
+	},
+	{
+		.speedo_id = 7,
+		.process_id = 0,
+		.min_millivolts = 841,
+		.max_millivolts = 1227,
+		CPU_CVB_TABLE_EUCM1,
+		.cpu_dfll_data = {
+			.tune0_low = 0xffead0ff,
+			.tune0_high = 0xffead0ff,
+			.tune1 = 0x20091d9,
+			.tune_high_min_millivolts = 864,
+		}
+	},
+	{
+		.speedo_id = 7,
+		.process_id = 1,
+		.min_millivolts = 841,
+		.max_millivolts = 1227,
+		CPU_CVB_TABLE_EUCM1,
+		.cpu_dfll_data = {
+			.tune0_low = 0xffead0ff,
+			.tune0_high = 0xffead0ff,
+			.tune1 = 0x20091d9,
+			.tune_high_min_millivolts = 864,
+		}
+	},
+	{
+		.speedo_id = 6,
+		.process_id = 0,
+		.min_millivolts = 870,
+		.max_millivolts = 1150,
+		CPU_CVB_TABLE,
+		.cpu_dfll_data = {
+			.tune0_low = 0xffead0ff,
+			.tune1 = 0x20091d9,
+		}
+	},
+	{
+		.speedo_id = 6,
+		.process_id = 1,
+		.min_millivolts = 870,
+		.max_millivolts = 1150,
+		CPU_CVB_TABLE,
+		.cpu_dfll_data = {
+			.tune0_low = 0xffead0ff,
+			.tune1 = 0x25501d0,
+		}
+	},
+	{
+		.speedo_id = 5,
+		.process_id = 0,
+		.min_millivolts = 818,
+		.max_millivolts = 1227,
+		CPU_CVB_TABLE,
+		.cpu_dfll_data = {
+			.tune0_low = 0xffead0ff,
+			.tune0_high = 0xffead0ff,
+			.tune1 = 0x20091d9,
+			.tune_high_min_millivolts = 864,
+		}
+	},
+	{
+		.speedo_id = 5,
+		.process_id = 1,
+		.min_millivolts = 818,
+		.max_millivolts = 1227,
+		CPU_CVB_TABLE,
+		.cpu_dfll_data = {
+			.tune0_low = 0xffead0ff,
+			.tune0_high = 0xffead0ff,
+			.tune1 = 0x25501d0,
+			.tune_high_min_millivolts = 864,
+		}
+	},
+	{
+		.speedo_id = 4,
+		.process_id = -1,
+		.min_millivolts = 918,
+		.max_millivolts = 1113,
+		CPU_CVB_TABLE_XA,
+		.cpu_dfll_data = {
+			.tune0_low = 0xffead0ff,
+			.tune1 = 0x17711BD,
+		}
+	},
+	{
+		.speedo_id = 3,
+		.process_id = 0,
+		.min_millivolts = 825,
+		.max_millivolts = 1227,
+		CPU_CVB_TABLE_ODN,
+		.cpu_dfll_data = {
+			.tune0_low = 0xffead0ff,
+			.tune0_high = 0xffead0ff,
+			.tune1 = 0x20091d9,
+			.tune_high_min_millivolts = 864,
+		}
+	},
+	{
+		.speedo_id = 3,
+		.process_id = 1,
+		.min_millivolts = 825,
+		.max_millivolts = 1227,
+		CPU_CVB_TABLE_ODN,
+		.cpu_dfll_data = {
+			.tune0_low = 0xffead0ff,
+			.tune0_high = 0xffead0ff,
+			.tune1 = 0x25501d0,
+			.tune_high_min_millivolts = 864,
+		}
+	},
+	{
+		.speedo_id = 2,
+		.process_id = 0,
+		.min_millivolts = 870,
+		.max_millivolts = 1227,
+		CPU_CVB_TABLE,
+		.cpu_dfll_data = {
+			.tune0_low = 0xffead0ff,
+			.tune1 = 0x20091d9,
+		}
+	},
+	{
+		.speedo_id = 2,
+		.process_id = 1,
+		.min_millivolts = 870,
+		.max_millivolts = 1227,
+		CPU_CVB_TABLE,
+		.cpu_dfll_data = {
+			.tune0_low = 0xffead0ff,
+			.tune1 = 0x25501d0,
+		}
+	},
+	{
+		.speedo_id = 1,
+		.process_id = 0,
+		.min_millivolts = 837,
+		.max_millivolts = 1227,
+		CPU_CVB_TABLE,
+		.cpu_dfll_data = {
+			.tune0_low = 0xffead0ff,
+			.tune0_high = 0xffead0ff,
+			.tune1 = 0x20091d9,
+			.tune_high_min_millivolts = 864,
+		}
+	},
+	{
+		.speedo_id = 1,
+		.process_id = 1,
+		.min_millivolts = 837,
+		.max_millivolts = 1227,
+		CPU_CVB_TABLE,
+		.cpu_dfll_data = {
+			.tune0_low = 0xffead0ff,
+			.tune0_high = 0xffead0ff,
+			.tune1 = 0x25501d0,
+			.tune_high_min_millivolts = 864,
+		}
+	},
+	{
+		.speedo_id = 0,
+		.process_id = 0,
+		.min_millivolts = 850,
+		.max_millivolts = 1170,
+		CPU_CVB_TABLE,
+		.cpu_dfll_data = {
+			.tune0_low = 0xffead0ff,
+			.tune0_high = 0xffead0ff,
+			.tune1 = 0x20091d9,
+			.tune_high_min_millivolts = 864,
+		}
+	},
+	{
+		.speedo_id = 0,
+		.process_id = 1,
+		.min_millivolts = 850,
+		.max_millivolts = 1170,
+		CPU_CVB_TABLE,
+		.cpu_dfll_data = {
+			.tune0_low = 0xffead0ff,
+			.tune0_high = 0xffead0ff,
+			.tune1 = 0x25501d0,
+			.tune_high_min_millivolts = 864,
+		}
+	},
+};
+
 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),
@@ -95,11 +510,22 @@ struct dfll_fcpu_data {
 	.cpu_cvb_tables_size = ARRAY_SIZE(tegra124_cpu_cvb_tables)
 };
 
+static const struct dfll_fcpu_data tegra210_dfll_fcpu_data = {
+	.cpu_max_freq_table = tegra210_cpu_max_freq_table,
+	.cpu_max_freq_table_size = ARRAY_SIZE(tegra210_cpu_max_freq_table),
+	.cpu_cvb_tables = tegra210_cpu_cvb_tables,
+	.cpu_cvb_tables_size = ARRAY_SIZE(tegra210_cpu_cvb_tables),
+};
+
 static const struct of_device_id tegra124_dfll_fcpu_of_match[] = {
 	{
 		.compatible = "nvidia,tegra124-dfll",
 		.data = &tegra124_dfll_fcpu_data,
 	},
+	{
+		.compatible = "nvidia,tegra210-dfll",
+		.data = &tegra210_dfll_fcpu_data
+	},
 	{ },
 };
 
diff --git a/drivers/clk/tegra/cvb.h b/drivers/clk/tegra/cvb.h
index bcf15a0..91a1941 100644
--- a/drivers/clk/tegra/cvb.h
+++ b/drivers/clk/tegra/cvb.h
@@ -41,6 +41,7 @@ struct cvb_cpu_dfll_data {
 	u32 tune0_low;
 	u32 tune0_high;
 	u32 tune1;
+	unsigned int tune_high_min_millivolts;
 };
 
 struct cvb_table {
-- 
1.9.1

[PATCH v3 05/11] clk: tegra: prepare dfll driver for PWM regulator

[Peter De Schrijver]

This patch prepares the dfll driver to work with PWM regulators.
To do this we introduce a new array lut_uv which gives the voltage for
a given index generated by the dfll logic. This index will then be
translated to a PMIC voltage ID in case of I2C using the i2c_lut. In case
of a PWM regulator, it will be used to determine the PWM duty cycle.
We also introduce lut_bottom which holds the lowest voltage we will ever
need. In case of I2C this can be set to zero because the i2c_lut will be
initialized such that entry 0 will be the lowest voltage we will ever
need. In case of PWM, the lowest voltage is determined by the regulator
hardware so we need this software limit. Note that this is different
from lut_min which gives the lowest voltage we allow taking temperature
into account. In a future patchset we will update lut_vmin dynamically.
Similarly lut_max will be the highest voltage allowed taking temperature
into accouint. Also this will be updated dynamically once temperature
dependence will be introduced.

Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
---
 drivers/clk/tegra/clk-dfll.c | 62 ++++++++++++++++++++++++++------------------
 1 file changed, 37 insertions(+), 25 deletions(-)

diff --git a/drivers/clk/tegra/clk-dfll.c b/drivers/clk/tegra/clk-dfll.c
index 0a7deee..fa97763 100644
--- a/drivers/clk/tegra/clk-dfll.c
+++ b/drivers/clk/tegra/clk-dfll.c
@@ -301,9 +301,10 @@ struct tegra_dfll {
 	u32				i2c_slave_addr;
 
 	/* i2c_lut array entries are regulator framework selectors */
-	unsigned			i2c_lut[MAX_DFLL_VOLTAGES];
-	int				i2c_lut_size;
-	u8				lut_min, lut_max, lut_safe;
+	unsigned int			i2c_lut[MAX_DFLL_VOLTAGES];
+	unsigned int			lut_uv[MAX_DFLL_VOLTAGES];
+	int				lut_size;
+	u8				lut_bottom, lut_min, lut_max, lut_safe;
 };
 
 #define clk_hw_to_dfll(_hw) container_of(_hw, struct tegra_dfll, dfll_clk_hw)
@@ -531,10 +532,10 @@ static void dfll_load_i2c_lut(struct tegra_dfll *td)
 	u32 val;
 
 	for (i = 0; i < MAX_DFLL_VOLTAGES; i++) {
-		if (i < td->lut_min)
-			lut_index = td->lut_min;
-		else if (i > td->lut_max)
-			lut_index = td->lut_max;
+		if (i < td->lut_bottom)
+			lut_index = td->lut_bottom;
+		else if (i > td->lut_size - 1)
+			lut_index = td->lut_size - 1;
 		else
 			lut_index = i;
 
@@ -594,9 +595,9 @@ static void dfll_init_out_if(struct tegra_dfll *td)
 {
 	u32 val;
 
-	td->lut_min = 0;
-	td->lut_max = td->i2c_lut_size - 1;
-	td->lut_safe = td->lut_min + 1;
+	td->lut_min = td->lut_bottom;
+	td->lut_max = td->lut_size - 1;
+	td->lut_safe = td->lut_min + (td->lut_min < td->lut_max ? 1 : 0);
 
 	dfll_i2c_writel(td, 0, DFLL_OUTPUT_CFG);
 	val = (td->lut_safe << DFLL_OUTPUT_CFG_SAFE_SHIFT) |
@@ -619,11 +620,11 @@ static void dfll_init_out_if(struct tegra_dfll *td)
  */
 
 /**
- * find_lut_index_for_rate - determine I2C LUT index for given DFLL rate
+ * find_lut_index_for_rate - determine LUT index for given DFLL rate
  * @td: DFLL instance
  * @rate: clock rate
  *
- * Determines the index of a I2C LUT entry for a voltage that approximately
+ * Determines the index of a LUT entry for a voltage that approximately
  * produces the given DFLL clock rate. This is used when forcing a value
  * to the integrator during rate changes. Returns -ENOENT if a suitable
  * LUT index is not found.
@@ -637,11 +638,11 @@ static int find_lut_index_for_rate(struct tegra_dfll *td, unsigned long rate)
 	if (IS_ERR(opp))
 		return PTR_ERR(opp);
 
-	uv = dev_pm_opp_get_voltage(opp);
+	uv = dev_pm_opp_get_voltage(opp) / td->soc->alignment.step_uv;
 	dev_pm_opp_put(opp);
 
-	for (i = 0; i < td->i2c_lut_size; i++) {
-		if (regulator_list_voltage(td->vdd_reg, td->i2c_lut[i]) == uv)
+	for (i = td->lut_bottom; i < td->lut_size; i++) {
+		if ((td->lut_uv[i] / td->soc->alignment.step_uv) >= uv)
 			return i;
 	}
 
@@ -1377,15 +1378,17 @@ static int dfll_init(struct tegra_dfll *td)
  */
 static int find_vdd_map_entry_exact(struct tegra_dfll *td, int uV)
 {
-	int i, n_voltages, reg_uV;
+	int i, n_voltages, reg_mult, align_mult;
 
+	align_mult = uV / td->soc->alignment.step_uv;
 	n_voltages = regulator_count_voltages(td->vdd_reg);
 	for (i = 0; i < n_voltages; i++) {
-		reg_uV = regulator_list_voltage(td->vdd_reg, i);
-		if (reg_uV < 0)
+		reg_mult = regulator_list_voltage(td->vdd_reg, i) /
+				td->soc->alignment.step_uv;
+		if (reg_mult < 0)
 			break;
 
-		if (uV == reg_uV)
+		if (align_mult == reg_mult)
 			return i;
 	}
 
@@ -1399,15 +1402,17 @@ static int find_vdd_map_entry_exact(struct tegra_dfll *td, int uV)
  * */
 static int find_vdd_map_entry_min(struct tegra_dfll *td, int uV)
 {
-	int i, n_voltages, reg_uV;
+	int i, n_voltages, reg_mult, align_mult;
 
+	align_mult = uV / td->soc->alignment.step_uv;
 	n_voltages = regulator_count_voltages(td->vdd_reg);
 	for (i = 0; i < n_voltages; i++) {
-		reg_uV = regulator_list_voltage(td->vdd_reg, i);
-		if (reg_uV < 0)
+		reg_mult = regulator_list_voltage(td->vdd_reg, i) /
+				td->soc->alignment.step_uv;
+		if (reg_mult < 0)
 			break;
 
-		if (uV <= reg_uV)
+		if (align_mult <= reg_mult)
 			return i;
 	}
 
@@ -1450,8 +1455,10 @@ static int dfll_build_i2c_lut(struct tegra_dfll *td)
 	if (lut < 0)
 		goto out;
 	td->i2c_lut[0] = lut;
+	td->lut_bottom = 0;
 
 	for (j = 1, rate = 0; ; rate++) {
+
 		opp = dev_pm_opp_find_freq_ceil(td->soc->dev, &rate);
 		if (IS_ERR(opp))
 			break;
@@ -1484,13 +1491,18 @@ static int dfll_build_i2c_lut(struct tegra_dfll *td)
 		if (v >= v_max)
 			break;
 	}
-	td->i2c_lut_size = j;
+	td->lut_size = j;
 
 	if (!td->dvco_rate_min)
 		dev_err(td->dev, "no opp above DFLL minimum voltage %d mV\n",
 			td->soc->cvb->min_millivolts);
-	else
+	else {
 		ret = 0;
+		for (j = 0; j < td->lut_size; j++)
+			td->lut_uv[j] =
+				regulator_list_voltage(td->vdd_reg,
+						       td->i2c_lut[j]);
+	}
 
 out:
 	return ret;
-- 
1.9.1

[PATCH v3 06/11] clk: tegra: dfll: support PWM regulator control

[Peter De Schrijver]

The DFLL can directly generate a PWM signal to control the regulator IC
rather then sending i2c messages. This patch adds support for this mode.
In this mode the hardware LUT is not used and also there is no regulator
object involved because there is no way to control the regulator voltage
without also changing the DFLL output frequency. Also the register debugfs
file is slightly reworked to only show the i2c registers when i2c mode is
in use. As there is no regulator object for the PWM regulator, its step and
offset values are retrieved from DT instead.

Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
---
 drivers/clk/tegra/clk-dfll.c               | 398 ++++++++++++++++++++++++++---
 drivers/clk/tegra/clk-tegra124-dfll-fcpu.c |  23 +-
 2 files changed, 382 insertions(+), 39 deletions(-)

diff --git a/drivers/clk/tegra/clk-dfll.c b/drivers/clk/tegra/clk-dfll.c
index fa97763..228edb4 100644
--- a/drivers/clk/tegra/clk-dfll.c
+++ b/drivers/clk/tegra/clk-dfll.c
@@ -243,6 +243,12 @@ enum dfll_tune_range {
 	DFLL_TUNE_LOW = 1,
 };
 
+
+enum tegra_dfll_pmu_if {
+	TEGRA_DFLL_PMU_I2C = 0,
+	TEGRA_DFLL_PMU_PWM = 1,
+};
+
 /**
  * struct dfll_rate_req - target DFLL rate request data
  * @rate: target frequency, after the postscaling
@@ -294,17 +300,25 @@ struct tegra_dfll {
 	u32				ci;
 	u32				cg;
 	bool				cg_scale;
+	u32				reg_init_uV;
 
 	/* I2C interface parameters */
 	u32				i2c_fs_rate;
 	u32				i2c_reg;
 	u32				i2c_slave_addr;
 
-	/* i2c_lut array entries are regulator framework selectors */
+	/* lut array entries are regulator framework selectors or PWM values*/
 	unsigned int			i2c_lut[MAX_DFLL_VOLTAGES];
 	unsigned int			lut_uv[MAX_DFLL_VOLTAGES];
 	int				lut_size;
 	u8				lut_bottom, lut_min, lut_max, lut_safe;
+
+	/* PWM interface */
+	enum tegra_dfll_pmu_if		pmu_if;
+	unsigned long			pwm_rate;
+	struct pinctrl			*pwm_pin;
+	struct pinctrl_state		*pwm_enable_state;
+	struct pinctrl_state		*pwm_disable_state;
 };
 
 #define clk_hw_to_dfll(_hw) container_of(_hw, struct tegra_dfll, dfll_clk_hw)
@@ -491,6 +505,36 @@ static void dfll_set_mode(struct tegra_dfll *td,
 }
 
 /*
+ * DVCO rate control
+ */
+
+static unsigned long get_dvco_rate_below(struct tegra_dfll *td, u8 out_min)
+{
+	struct dev_pm_opp *opp;
+	unsigned long rate, prev_rate;
+	int uv, min_uv;
+
+	min_uv = td->lut_uv[out_min];
+	for (rate = 0, prev_rate = 0; ; rate++) {
+		rcu_read_lock();
+		opp = dev_pm_opp_find_freq_ceil(td->soc->dev, &rate);
+		if (IS_ERR(opp)) {
+			rcu_read_unlock();
+			break;
+		}
+		uv = dev_pm_opp_get_voltage(opp);
+		rcu_read_unlock();
+
+		if (uv && uv > min_uv)
+			return prev_rate;
+
+		prev_rate = rate;
+	}
+
+	return prev_rate;
+}
+
+/*
  * DFLL-to-I2C controller interface
  */
 
@@ -519,6 +563,119 @@ static int dfll_i2c_set_output_enabled(struct tegra_dfll *td, bool enable)
 	return 0;
 }
 
+
+/*
+ * DFLL-to-PWM controller interface
+ */
+
+/**
+ * dfll_pwm_set_output_enabled - enable/disable PWM voltage requests
+ * @td: DFLL instance
+ * @enable: whether to enable or disable the PWM voltage requests
+ *
+ * Set the master enable control for PWM control value updates. If disabled,
+ * then the PWM signal is not driven. Also configure the PWM output pad
+ * to the appropriate state.
+ */
+static int dfll_pwm_set_output_enabled(struct tegra_dfll *td, bool enable)
+{
+	int ret;
+	u32 val, div;
+
+	if (enable) {
+		ret = pinctrl_select_state(td->pwm_pin, td->pwm_enable_state);
+		if (ret < 0) {
+			dev_err(td->dev, "setting enable state failed\n");
+			return ret;
+		}
+		val = dfll_readl(td, DFLL_OUTPUT_CFG);
+		val &= ~DFLL_OUTPUT_CFG_PWM_DIV_MASK;
+		div = DIV_ROUND_UP(td->ref_rate, td->pwm_rate);
+		val |= (div << DFLL_OUTPUT_CFG_PWM_DIV_SHIFT)
+				& DFLL_OUTPUT_CFG_PWM_DIV_MASK;
+		dfll_writel(td, val, DFLL_OUTPUT_CFG);
+		dfll_wmb(td);
+
+		val |= DFLL_OUTPUT_CFG_PWM_ENABLE;
+		dfll_writel(td, val, DFLL_OUTPUT_CFG);
+		dfll_wmb(td);
+	} else {
+		ret = pinctrl_select_state(td->pwm_pin, td->pwm_disable_state);
+		if (ret < 0)
+			dev_warn(td->dev, "setting disable state failed\n");
+
+		val = dfll_readl(td, DFLL_OUTPUT_CFG);
+		val &= ~DFLL_OUTPUT_CFG_PWM_ENABLE;
+		dfll_writel(td, val, DFLL_OUTPUT_CFG);
+		dfll_wmb(td);
+	}
+
+	return 0;
+}
+/**
+ * dfll_set_force_output_value - set fixed value for force output
+ * @td: DFLL instance
+ * @out_val: value to force output
+ *
+ * Set the fixed value for force output, DFLL will output this value when
+ * force output is enabled.
+ */
+static u32 dfll_set_force_output_value(struct tegra_dfll *td, u8 out_val)
+{
+	u32 val = dfll_readl(td, DFLL_OUTPUT_FORCE);
+
+	val &= ~OUT_MASK;
+	val = (val & DFLL_OUTPUT_FORCE_ENABLE) | (out_val & OUT_MASK);
+	dfll_writel(td, val, DFLL_OUTPUT_FORCE);
+	dfll_wmb(td);
+
+	return dfll_readl(td, DFLL_OUTPUT_FORCE);
+}
+
+/**
+ * dfll_set_force_output_enabled - enable/disable force output
+ * @td: DFLL instance
+ * @enable: whether to enable or disable the force output
+ *
+ * Set the enable control for fouce output with fixed value.
+ */
+static void dfll_set_force_output_enabled(struct tegra_dfll *td, bool enable)
+{
+	u32 val = dfll_readl(td, DFLL_OUTPUT_FORCE);
+
+	if (enable)
+		val |= DFLL_OUTPUT_FORCE_ENABLE;
+	else
+		val &= ~DFLL_OUTPUT_FORCE_ENABLE;
+
+	dfll_writel(td, val, DFLL_OUTPUT_FORCE);
+	dfll_wmb(td);
+}
+
+/**
+ * dfll_i2c_set_output_enabled - enable/disable I2C PMIC voltage requests
+ * @td: DFLL instance
+ * @enable: whether to enable or disable the I2C voltage requests
+ *
+ * Set the master enable control for I2C control value updates. If disabled,
+ * then I2C control messages are inhibited, regardless of the DFLL mode.
+ */
+static int dfll_force_output(struct tegra_dfll *td, unsigned int out_sel)
+{
+	u32 val;
+
+	if (out_sel > OUT_MASK)
+		return -EINVAL;
+
+	val = dfll_set_force_output_value(td, out_sel);
+	if ((td->mode < DFLL_CLOSED_LOOP) &&
+	    !(val & DFLL_OUTPUT_FORCE_ENABLE)) {
+		dfll_set_force_output_enabled(td, true);
+	}
+
+	return 0;
+}
+
 /**
  * dfll_load_lut - load the voltage lookup table
  * @td: struct tegra_dfll *
@@ -599,20 +756,50 @@ static void dfll_init_out_if(struct tegra_dfll *td)
 	td->lut_max = td->lut_size - 1;
 	td->lut_safe = td->lut_min + (td->lut_min < td->lut_max ? 1 : 0);
 
-	dfll_i2c_writel(td, 0, DFLL_OUTPUT_CFG);
-	val = (td->lut_safe << DFLL_OUTPUT_CFG_SAFE_SHIFT) |
-		(td->lut_max << DFLL_OUTPUT_CFG_MAX_SHIFT) |
-		(td->lut_min << DFLL_OUTPUT_CFG_MIN_SHIFT);
-	dfll_i2c_writel(td, val, DFLL_OUTPUT_CFG);
-	dfll_i2c_wmb(td);
-
-	dfll_writel(td, 0, DFLL_OUTPUT_FORCE);
-	dfll_i2c_writel(td, 0, DFLL_INTR_EN);
-	dfll_i2c_writel(td, DFLL_INTR_MAX_MASK | DFLL_INTR_MIN_MASK,
-			DFLL_INTR_STS);
-
-	dfll_load_i2c_lut(td);
-	dfll_init_i2c_if(td);
+	if (td->pmu_if == TEGRA_DFLL_PMU_PWM) {
+		int vinit = td->reg_init_uV;
+		int vstep = td->soc->alignment.step_uv;
+		int vmin = td->lut_uv[0];
+
+		/* clear DFLL_OUTPUT_CFG before setting new value */
+		dfll_writel(td, 0, DFLL_OUTPUT_CFG);
+		dfll_wmb(td);
+
+		val = dfll_readl(td, DFLL_OUTPUT_CFG);
+		val |= (td->lut_safe << DFLL_OUTPUT_CFG_SAFE_SHIFT) |
+		       (td->lut_max << DFLL_OUTPUT_CFG_MAX_SHIFT) |
+		       (td->lut_min << DFLL_OUTPUT_CFG_MIN_SHIFT);
+		dfll_writel(td, val, DFLL_OUTPUT_CFG);
+		dfll_wmb(td);
+
+		dfll_writel(td, 0, DFLL_OUTPUT_FORCE);
+		dfll_i2c_writel(td, 0, DFLL_INTR_EN);
+		dfll_i2c_writel(td, DFLL_INTR_MAX_MASK | DFLL_INTR_MIN_MASK,
+				DFLL_INTR_STS);
+
+		/* set initial voltage */
+		if ((vinit >= vmin) && vstep) {
+			unsigned int vsel;
+
+			vsel = DIV_ROUND_UP((vinit - vmin), vstep);
+			dfll_force_output(td, vsel);
+		}
+	} else {
+		dfll_i2c_writel(td, 0, DFLL_OUTPUT_CFG);
+		val = (td->lut_safe << DFLL_OUTPUT_CFG_SAFE_SHIFT) |
+			(td->lut_max << DFLL_OUTPUT_CFG_MAX_SHIFT) |
+			(td->lut_min << DFLL_OUTPUT_CFG_MIN_SHIFT);
+		dfll_i2c_writel(td, val, DFLL_OUTPUT_CFG);
+		dfll_i2c_wmb(td);
+
+		dfll_writel(td, 0, DFLL_OUTPUT_FORCE);
+		dfll_i2c_writel(td, 0, DFLL_INTR_EN);
+		dfll_i2c_writel(td, DFLL_INTR_MAX_MASK | DFLL_INTR_MIN_MASK,
+				DFLL_INTR_STS);
+
+		dfll_load_i2c_lut(td);
+		dfll_init_i2c_if(td);
+	}
 }
 
 /*
@@ -864,9 +1051,14 @@ static int dfll_lock(struct tegra_dfll *td)
 			return -EINVAL;
 		}
 
-		dfll_i2c_set_output_enabled(td, true);
+		if (td->pmu_if == TEGRA_DFLL_PMU_PWM)
+			dfll_pwm_set_output_enabled(td, true);
+		else
+			dfll_i2c_set_output_enabled(td, true);
+
 		dfll_set_mode(td, DFLL_CLOSED_LOOP);
 		dfll_set_frequency_request(td, req);
+		dfll_set_force_output_enabled(td, false);
 		return 0;
 
 	default:
@@ -890,7 +1082,10 @@ static int dfll_unlock(struct tegra_dfll *td)
 	case DFLL_CLOSED_LOOP:
 		dfll_set_open_loop_config(td);
 		dfll_set_mode(td, DFLL_OPEN_LOOP);
-		dfll_i2c_set_output_enabled(td, false);
+		if (td->pmu_if == TEGRA_DFLL_PMU_PWM)
+			dfll_pwm_set_output_enabled(td, false);
+		else
+			dfll_i2c_set_output_enabled(td, false);
 		return 0;
 
 	case DFLL_OPEN_LOOP:
@@ -1172,15 +1367,17 @@ static int attr_registers_show(struct seq_file *s, void *data)
 		seq_printf(s, "[0x%02x] = 0x%08x\n", offs,
 			   dfll_i2c_readl(td, offs));
 
-	seq_puts(s, "\nINTEGRATED I2C CONTROLLER REGISTERS:\n");
-	offs = DFLL_I2C_CLK_DIVISOR;
-	seq_printf(s, "[0x%02x] = 0x%08x\n", offs,
-		   __raw_readl(td->i2c_controller_base + offs));
-
-	seq_puts(s, "\nLUT:\n");
-	for (offs = 0; offs <  4 * MAX_DFLL_VOLTAGES; offs += 4)
+	if (td->pmu_if == TEGRA_DFLL_PMU_I2C) {
+		seq_puts(s, "\nINTEGRATED I2C CONTROLLER REGISTERS:\n");
+		offs = DFLL_I2C_CLK_DIVISOR;
 		seq_printf(s, "[0x%02x] = 0x%08x\n", offs,
-			   __raw_readl(td->lut_base + offs));
+			   __raw_readl(td->i2c_controller_base + offs));
+
+		seq_puts(s, "\nLUT:\n");
+		for (offs = 0; offs <  4 * MAX_DFLL_VOLTAGES; offs += 4)
+			seq_printf(s, "[0x%02x] = 0x%08x\n", offs,
+				   __raw_readl(td->lut_base + offs));
+	}
 
 	return 0;
 }
@@ -1289,6 +1486,9 @@ static int dfll_init_clks(struct tegra_dfll *td)
 		return PTR_ERR(td->soc_clk);
 	}
 
+	if (td->pmu_if != TEGRA_DFLL_PMU_I2C)
+		return 0;
+
 	td->i2c_clk = devm_clk_get(td->dev, "i2c");
 	if (IS_ERR(td->i2c_clk)) {
 		dev_err(td->dev, "missing i2c clock\n");
@@ -1420,6 +1620,52 @@ static int find_vdd_map_entry_min(struct tegra_dfll *td, int uV)
 	return -EINVAL;
 }
 
+/*
+ * Look-up table in h/w is ignored when PWM is used as DFLL interface to PMIC.
+ * In this case closed loop output is controlling duty cycle directly. The s/w
+ * look-up that maps PWM duty cycle to voltage is still built by this function.
+ */
+static int dfll_build_lut_pwm(struct tegra_dfll *td, int v_max)
+{
+	int i, reg_volt;
+	unsigned long rate;
+	u8 lut_bottom = MAX_DFLL_VOLTAGES;
+	int v_min = td->soc->cvb->min_millivolts * 1000;
+
+	for (i = 0; i < MAX_DFLL_VOLTAGES; i++) {
+		reg_volt = td->lut_uv[i];
+
+		/* since opp voltage is exact mv */
+		reg_volt = (reg_volt / 1000) * 1000;
+		if (reg_volt > v_max)
+			break;
+
+		if ((lut_bottom == MAX_DFLL_VOLTAGES) && (reg_volt >= v_min))
+			lut_bottom = i;
+	}
+
+	/* determine voltage boundaries */
+	td->lut_size = i;
+	if ((lut_bottom == MAX_DFLL_VOLTAGES) ||
+	    (lut_bottom + 1 >= td->lut_size)) {
+		dev_err(td->dev, "no voltage above DFLL minimum %d mV\n",
+			td->soc->cvb->min_millivolts);
+		return -EINVAL;
+	}
+	td->lut_bottom = lut_bottom;
+
+	/* determine rate boundaries */
+	rate = get_dvco_rate_below(td, td->lut_bottom);
+	if (!rate) {
+		dev_err(td->dev, "no opp below DFLL minimum voltage %d mV\n",
+			td->soc->cvb->min_millivolts);
+		return -EINVAL;
+	}
+	td->dvco_rate_min = rate;
+
+	return 0;
+}
+
 /**
  * dfll_build_i2c_lut - build the I2C voltage register lookup table
  * @td: DFLL instance
@@ -1432,10 +1678,10 @@ static int find_vdd_map_entry_min(struct tegra_dfll *td, int uV)
  *
  * On success, fills in td->i2c_lut and returns 0, or -err on failure.
  */
-static int dfll_build_i2c_lut(struct tegra_dfll *td)
+static int dfll_build_i2c_lut(struct tegra_dfll *td, int v_max)
 {
 	int ret = -EINVAL;
-	int j, v, v_max, v_opp;
+	int j, v, v_opp;
 	int selector;
 	unsigned long rate;
 	struct dev_pm_opp *opp;
@@ -1508,6 +1754,30 @@ static int dfll_build_i2c_lut(struct tegra_dfll *td)
 	return ret;
 }
 
+static int dfll_build_lut(struct tegra_dfll *td)
+{
+	unsigned long rate;
+	struct dev_pm_opp *opp;
+	int v_max;
+
+	rcu_read_lock();
+
+	rate = ULONG_MAX;
+	opp = dev_pm_opp_find_freq_floor(td->soc->dev, &rate);
+	if (IS_ERR(opp)) {
+		dev_err(td->dev, "couldn't get vmax opp, empty opp table?\n");
+		return -EINVAL;
+	}
+	v_max = dev_pm_opp_get_voltage(opp);
+
+	rcu_read_unlock();
+
+	if (td->pmu_if == TEGRA_DFLL_PMU_PWM)
+		return dfll_build_lut_pwm(td, v_max);
+	else
+		return dfll_build_i2c_lut(td, v_max);
+}
+
 /**
  * read_dt_param - helper function for reading required parameters from the DT
  * @td: DFLL instance
@@ -1566,11 +1836,54 @@ static int dfll_fetch_i2c_params(struct tegra_dfll *td)
 	}
 	td->i2c_reg = vsel_reg;
 
-	ret = dfll_build_i2c_lut(td);
-	if (ret) {
-		dev_err(td->dev, "couldn't build I2C LUT\n");
+	return 0;
+}
+
+static int dfll_fetch_pwm_params(struct tegra_dfll *td)
+{
+	int ret, i;
+	u32 pwm_period;
+
+	if (!td->soc->alignment.step_uv || !td->soc->alignment.offset_uv) {
+		dev_err(td->dev, "Missing step or alignment info for PWM regulator");
+		return -EINVAL;
+	}
+	for (i = 0; i < MAX_DFLL_VOLTAGES; i++)
+		td->lut_uv[i] = td->soc->alignment.offset_uv +
+				i * td->soc->alignment.step_uv;
+
+	ret = read_dt_param(td, "nvidia,init-uv", &td->reg_init_uV);
+	if (!ret) {
+		dev_err(td->dev, "couldn't get initialized voltage\n");
+		return ret;
+	}
+
+	ret = read_dt_param(td, "nvidia,pwm-period", &pwm_period);
+	if (!ret) {
+		dev_err(td->dev, "couldn't get PWM period\n");
 		return ret;
 	}
+	td->pwm_rate = (NSEC_PER_SEC / pwm_period) * (MAX_DFLL_VOLTAGES - 1);
+
+	td->pwm_pin = devm_pinctrl_get(td->dev);
+	if (IS_ERR(td->pwm_pin)) {
+		dev_err(td->dev, "DT: missing pinctrl device\n");
+		return PTR_ERR(td->pwm_pin);
+	}
+
+	td->pwm_enable_state = pinctrl_lookup_state(td->pwm_pin,
+						    "dvfs_pwm_enable");
+	if (IS_ERR(td->pwm_enable_state)) {
+		dev_err(td->dev, "DT: missing pwm enabled state\n");
+		return PTR_ERR(td->pwm_enable_state);
+	}
+
+	td->pwm_disable_state = pinctrl_lookup_state(td->pwm_pin,
+						     "dvfs_pwm_disable");
+	if (IS_ERR(td->pwm_disable_state)) {
+		dev_err(td->dev, "DT: missing pwm disabled state\n");
+		return PTR_ERR(td->pwm_disable_state);
+	}
 
 	return 0;
 }
@@ -1637,12 +1950,6 @@ int tegra_dfll_register(struct platform_device *pdev,
 
 	td->soc = soc;
 
-	td->vdd_reg = devm_regulator_get(td->dev, "vdd-cpu");
-	if (IS_ERR(td->vdd_reg)) {
-		dev_err(td->dev, "couldn't get vdd_cpu regulator\n");
-		return PTR_ERR(td->vdd_reg);
-	}
-
 	td->dvco_rst = devm_reset_control_get(td->dev, "dvco");
 	if (IS_ERR(td->dvco_rst)) {
 		dev_err(td->dev, "couldn't get dvco reset\n");
@@ -1655,10 +1962,27 @@ int tegra_dfll_register(struct platform_device *pdev,
 		return ret;
 	}
 
-	ret = dfll_fetch_i2c_params(td);
+	if (of_property_read_bool(td->dev->of_node, "nvidia,pwm-to-pmic")) {
+		td->pmu_if = TEGRA_DFLL_PMU_PWM;
+		ret = dfll_fetch_pwm_params(td);
+	} else  {
+		td->vdd_reg = devm_regulator_get(td->dev, "vdd-cpu");
+		if (IS_ERR(td->vdd_reg)) {
+			dev_err(td->dev, "couldn't get vdd_cpu regulator\n");
+			return PTR_ERR(td->vdd_reg);
+		}
+		td->pmu_if = TEGRA_DFLL_PMU_I2C;
+		ret = dfll_fetch_i2c_params(td);
+	}
 	if (ret)
 		return ret;
 
+	ret = dfll_build_lut(td);
+	if (ret) {
+		dev_err(td->dev, "couldn't build LUT\n");
+		return ret;
+	}
+
 	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!mem) {
 		dev_err(td->dev, "no control register resource\n");
diff --git a/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c b/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c
index 78dddab..9d413e4 100644
--- a/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c
+++ b/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c
@@ -557,6 +557,21 @@ static int get_alignment_from_regulator(struct device *dev,
 	return 0;
 }
 
+static int get_alignment_from_dt(struct device *dev,
+				 struct rail_alignment *align)
+{
+	int err;
+
+	err = of_property_read_u32(dev->of_node, "nvidia,align-step-uv",
+				   &align->step_uv);
+	if (err < 0)
+		return err;
+
+	err = of_property_read_u32(dev->of_node,
+				   "nvidia,align-offset-uv", &align->offset_uv);
+	return err;
+}
+
 static int tegra124_dfll_fcpu_probe(struct platform_device *pdev)
 {
 	int process_id, speedo_id, speedo_value, err;
@@ -587,8 +602,12 @@ static int tegra124_dfll_fcpu_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
-	soc->max_freq = cpu_max_freq_table[speedo_id];
-	err = get_alignment_from_regulator(&pdev->dev, &soc->alignment);
+	soc->max_freq = fcpu_data->cpu_max_freq_table[speedo_id];
+	if (of_property_read_bool(pdev->dev.of_node, "nvidia,pwm-to-pmic"))
+		err = get_alignment_from_dt(&pdev->dev, &soc->alignment);
+	else
+		err = get_alignment_from_regulator(&pdev->dev,
+						   &soc->alignment);
 	if (err < 0)
 		return err;
 
-- 
1.9.1

[PATCH v3 07/11] dt-bindings: tegra: Update DFLL binding for PWM regulator

[Peter De Schrijver]

Add new properties to configure the DFLL PWM regulator support. Also
add an example and make the I2C clock only required when I2C support is
used.

Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
---
 .../bindings/clock/nvidia,tegra124-dfll.txt        | 76 +++++++++++++++++++++-
 1 file changed, 74 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt b/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt
index dff236f..a4903f7 100644
--- a/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt
+++ b/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt
@@ -23,7 +23,8 @@ Required properties:
 - clock-names: Must include the following entries:
   - soc: Clock source for the DFLL control logic.
   - ref: The closed loop reference clock
-  - i2c: Clock source for the integrated I2C master.
+  - i2c: Clock source for the integrated I2C master (only required when
+	 using I2C mode).
 - resets: Must contain an entry for each entry in reset-names.
   See ../reset/reset.txt for details.
 - reset-names: Must include the following entries:
@@ -45,10 +46,28 @@ Required properties for the control loop parameters:
 Optional properties for the control loop parameters:
 - nvidia,cg-scale: Boolean value, see the field DFLL_PARAMS_CG_SCALE in the TRM.
 
+Optional properties for mode selection:
+- nvidia,pwm-to-pmic: Use PWM to control regulator rather then I2C.
+
 Required properties for I2C mode:
 - nvidia,i2c-fs-rate: I2C transfer rate, if using full speed mode.
 
-Example:
+Required properties for PWM mode:
+- nvidia,pwm-period: period of PWM square wave in us.
+- nvidia,init-uv: Regulator voltage in uV when PWM control is disabled.
+- nvidia,align-offset-uv: Regulator voltage in uV when PWM control is enabled
+			  and PWM output is low.
+- nvidia,align-step-uv: Voltage increase in uV corresponding to a 1/33th
+			increase in duty cycle. Eg the voltage for 2/33th duty
+			cycle would be:
+			nvidia,align-offset-uv + nvidia,align-step-uv * 2.
+- pinctrl-0: I/O pad configuration when PWM control is enabled.
+- pinctrl-1: I/O pad configuration when PWM control is disabled.
+- pinctrl-names: must include the following entries:
+  - dvfs_pwm_enable: I/O pad configuration when PWM control is enabled.
+  - dvfs_pwm_disable: I/O pad configuration when PWM control is disabled.
+
+Example for I2C:
 
 clock@70110000 {
         compatible = "nvidia,tegra124-dfll";
@@ -76,3 +95,56 @@ clock@70110000 {
 
         nvidia,i2c-fs-rate = <400000>;
 };
+
+Example for PWM:
+
+clock@70110000 {
+        compatible = "nvidia,tegra124-dfll";
+        reg = <0 0x70110000 0 0x100>, /* DFLL control */
+              <0 0x70110000 0 0x100>, /* I2C output control */
+              <0 0x70110100 0 0x100>, /* Integrated I2C controller */
+              <0 0x70110200 0 0x100>; /* Look-up table RAM */
+        interrupts = <GIC_SPI 62 IRQ_TYPE_LEVEL_HIGH>;
+        clocks = <&tegra_car TEGRA210_CLK_DFLL_SOC>,
+                 <&tegra_car TEGRA210_CLK_DFLL_REF>;
+        clock-names = "soc", "ref";
+        resets = <&tegra_car TEGRA124_RST_DFLL_DVCO>;
+        reset-names = "dvco";
+        #clock-cells = <0>;
+        clock-output-names = "dfllCPU_out";
+	nvidia,pwm-to-pmic;
+	nvidia,init-uv = <1000000>;
+	nvidia,align-step-uv = <19200>; /* 19.2mV */
+	nvidia,align-offset-uv = <708000>; /* 708mV */
+	nvidia,sample-rate = <25000>;
+	nvidia,droop-ctrl = <0x00000f00>;
+	nvidia,force-mode = <1>;
+	nvidia,cf = <6>;
+	nvidia,ci = <0>;
+	nvidia,cg = <2>;
+	nvidia,idle-override;
+	nvidia,one-shot-calibrate;
+	nvidia,pwm-period = <2500>; /* 2.5us */
+	pinctrl-names = "dvfs_pwm_enable", "dvfs_pwm_disable";
+	pinctrl-0 = <&dvfs_pwm_active_state>;
+	pinctrl-1 = <&dvfs_pwm_inactive_state>;
+};
+
+/* pinmux nodes added for completeness. Binding doc can be found in:
+ * Documentation/devicetree/bindings/pinctrl/nvidia,tegra210-pinmux.txt
+ */
+
+pinmux: pinmux@700008d4 {
+	dvfs_pwm_active_state: dvfs_pwm_active {
+		dvfs_pwm_pbb1 {
+			nvidia,pins = "dvfs_pwm_pbb1";
+			nvidia,tristate = <TEGRA_PIN_DISABLE>;
+		};
+	};
+	dvfs_pwm_inactive_state: dvfs_pwm_inactive {
+		dvfs_pwm_pbb1 {
+			nvidia,pins = "dvfs_pwm_pbb1";
+			nvidia,tristate = <TEGRA_PIN_ENABLE>;
+		};
+	};
+};
-- 
1.9.1

[PATCH v3 08/11] clk: tegra: build clk-dfll.c for Tegra124 and Tegra210

[Peter De Schrijver]

Tegra210 has a DFLL as well and can share the majority of the code with
the Tegra124 implementation. So build the same code for both platforms.

Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
---
 drivers/clk/tegra/Kconfig  | 5 +++++
 drivers/clk/tegra/Makefile | 2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/tegra/Kconfig b/drivers/clk/tegra/Kconfig
index 7ddacae..57902ab 100644
--- a/drivers/clk/tegra/Kconfig
+++ b/drivers/clk/tegra/Kconfig
@@ -5,3 +5,8 @@ config TEGRA_CLK_EMC
 config CLK_TEGRA_BPMP
 	def_bool y
 	depends on TEGRA_BPMP
+
+config TEGRA_CLK_DFLL
+	depends on (ARCH_TEGRA_124_SOC || ARCH_TEGRA_210_SOC)
+	select PM_OPP
+	def_bool y
diff --git a/drivers/clk/tegra/Makefile b/drivers/clk/tegra/Makefile
index b716923..fc7f4b4 100644
--- a/drivers/clk/tegra/Makefile
+++ b/drivers/clk/tegra/Makefile
@@ -19,7 +19,7 @@ obj-$(CONFIG_ARCH_TEGRA_2x_SOC)         += clk-tegra20.o
 obj-$(CONFIG_ARCH_TEGRA_3x_SOC)         += clk-tegra30.o
 obj-$(CONFIG_ARCH_TEGRA_114_SOC)	+= clk-tegra114.o
 obj-$(CONFIG_ARCH_TEGRA_124_SOC)	+= clk-tegra124.o
-obj-$(CONFIG_ARCH_TEGRA_124_SOC)	+= clk-tegra124-dfll-fcpu.o
+obj-$(CONFIG_TEGRA_CLK_DFLL)		+= clk-tegra124-dfll-fcpu.o
 obj-$(CONFIG_ARCH_TEGRA_132_SOC)	+= clk-tegra124.o
 obj-y					+= cvb.o
 obj-$(CONFIG_ARCH_TEGRA_210_SOC)	+= clk-tegra210.o
-- 
1.9.1

[PATCH v3 09/11] cpufreq: tegra124-cpufreq: extend to support Tegra210

[Peter De Schrijver]

Tegra210 has a very similar CPU clocking scheme than Tegra124. So add
support in this driver. Also allow for the case where the CPU voltage is
controlled directly by the DFLL rather than by a separate regulator object.

Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
---
 drivers/cpufreq/tegra124-cpufreq.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/cpufreq/tegra124-cpufreq.c b/drivers/cpufreq/tegra124-cpufreq.c
index 4353025..f8e01a8 100644
--- a/drivers/cpufreq/tegra124-cpufreq.c
+++ b/drivers/cpufreq/tegra124-cpufreq.c
@@ -64,7 +64,8 @@ static void tegra124_cpu_switch_to_pllx(struct tegra124_cpufreq_priv *priv)
 {
 	clk_set_parent(priv->cpu_clk, priv->pllp_clk);
 	clk_disable_unprepare(priv->dfll_clk);
-	regulator_sync_voltage(priv->vdd_cpu_reg);
+	if (priv->vdd_cpu_reg)
+		regulator_sync_voltage(priv->vdd_cpu_reg);
 	clk_set_parent(priv->cpu_clk, priv->pllx_clk);
 }
 
@@ -89,10 +90,10 @@ static int tegra124_cpufreq_probe(struct platform_device *pdev)
 		return -ENODEV;
 
 	priv->vdd_cpu_reg = regulator_get(cpu_dev, "vdd-cpu");
-	if (IS_ERR(priv->vdd_cpu_reg)) {
-		ret = PTR_ERR(priv->vdd_cpu_reg);
-		goto out_put_np;
-	}
+	if (IS_ERR(priv->vdd_cpu_reg) != -EPROBE_DEFER)
+		priv->vdd_cpu_reg = NULL;
+	else
+		return -EPROBE_DEFER;
 
 	priv->cpu_clk = of_clk_get_by_name(np, "cpu_g");
 	if (IS_ERR(priv->cpu_clk)) {
@@ -148,7 +149,6 @@ static int tegra124_cpufreq_probe(struct platform_device *pdev)
 	clk_put(priv->cpu_clk);
 out_put_vdd_cpu_reg:
 	regulator_put(priv->vdd_cpu_reg);
-out_put_np:
 	of_node_put(np);
 
 	return ret;
@@ -181,7 +181,8 @@ static int __init tegra_cpufreq_init(void)
 	int ret;
 	struct platform_device *pdev;
 
-	if (!of_machine_is_compatible("nvidia,tegra124"))
+	if (!(of_machine_is_compatible("nvidia,tegra124")
+		|| of_machine_is_compatible("nvidia,tegra210")))
 		return -ENODEV;
 
 	/*
-- 
1.9.1

[PATCH v3 10/11] arm64: dts: tegra: Add Tegra210 DFLL definition

[Peter De Schrijver]

Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
---
 arch/arm64/boot/dts/nvidia/tegra210-p2180.dtsi | 18 ++++++++++++++++++
 arch/arm64/boot/dts/nvidia/tegra210-p2597.dtsi | 12 ++++++++++++
 arch/arm64/boot/dts/nvidia/tegra210.dtsi       | 19 +++++++++++++++++++
 3 files changed, 49 insertions(+)

diff --git a/arch/arm64/boot/dts/nvidia/tegra210-p2180.dtsi b/arch/arm64/boot/dts/nvidia/tegra210-p2180.dtsi
index 212e663..19720b5 100644
--- a/arch/arm64/boot/dts/nvidia/tegra210-p2180.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra210-p2180.dtsi
@@ -284,6 +284,24 @@
 		non-removable;
 	};
 
+	clock@70110000 {
+		status = "okay";
+		nvidia,pwm-to-pmic;
+		nvidia,init-uv = <1000000>;
+		nvidia,align-step-uv = <19200>; /* 19.2mV */
+		nvidia,align-offset-uv = <708000>; /* 708mV */
+		nvidia,sample-rate = <25000>;
+		nvidia,droop-ctrl = <0x00000f00>;
+		nvidia,force-mode = <1>;
+		nvidia,cf = <6>;
+		nvidia,ci = <0>;
+		nvidia,cg = <2>;
+		nvidia,pwm-period = <2500>; /* 2.5us */
+		pinctrl-names = "dvfs_pwm_enable", "dvfs_pwm_disable";
+		pinctrl-0 = <&dvfs_pwm_active_state>;
+		pinctrl-1 = <&dvfs_pwm_inactive_state>;
+	};
+
 	clocks {
 		compatible = "simple-bus";
 		#address-cells = <1>;
diff --git a/arch/arm64/boot/dts/nvidia/tegra210-p2597.dtsi b/arch/arm64/boot/dts/nvidia/tegra210-p2597.dtsi
index d67ef43..8145aef 100644
--- a/arch/arm64/boot/dts/nvidia/tegra210-p2597.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra210-p2597.dtsi
@@ -1278,6 +1278,18 @@
 				nvidia,open-drain = <TEGRA_PIN_DISABLE>;
 			};
 		};
+		dvfs_pwm_active_state: dvfs_pwm_active {
+			dvfs_pwm_pbb1 {
+				nvidia,pins = "dvfs_pwm_pbb1";
+				nvidia,tristate = <TEGRA_PIN_DISABLE>;
+			};
+		};
+		dvfs_pwm_inactive_state: dvfs_pwm_inactive {
+			dvfs_pwm_pbb1 {
+				nvidia,pins = "dvfs_pwm_pbb1";
+				nvidia,tristate = <TEGRA_PIN_ENABLE>;
+			};
+		};
 	};
 
 	pwm@7000a000 {
diff --git a/arch/arm64/boot/dts/nvidia/tegra210.dtsi b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
index 9c24021..bc9851a 100644
--- a/arch/arm64/boot/dts/nvidia/tegra210.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
@@ -3,6 +3,7 @@
 #include <dt-bindings/gpio/tegra-gpio.h>
 #include <dt-bindings/memory/tegra210-mc.h>
 #include <dt-bindings/pinctrl/pinctrl-tegra.h>
+#include <dt-bindings/reset/tegra210-car.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/thermal/tegra124-soctherm.h>
 
@@ -1056,6 +1057,24 @@
 		#nvidia,mipi-calibrate-cells = <1>;
 	};
 
+	dfll: clock@70110000 {
+		compatible = "nvidia,tegra210-dfll";
+		reg = <0 0x70110000 0 0x100>, /* DFLL control */
+		      <0 0x70110000 0 0x100>, /* I2C output control */
+		      <0 0x70110100 0 0x100>, /* Integrated I2C controller */
+		      <0 0x70110200 0 0x100>; /* Look-up table RAM */
+		interrupts = <GIC_SPI 62 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&tegra_car TEGRA210_CLK_DFLL_SOC>,
+			 <&tegra_car TEGRA210_CLK_DFLL_REF>;
+		clock-names = "soc", "ref";
+		resets = <&tegra_car TEGRA210_RST_DFLL_DVCO>;
+		reset-names = "dvco";
+		#clock-cells = <0>;
+		clock-output-names = "dfllCPU_out";
+		out-clock-name="dfll_cpu";
+		status = "disabled";
+	};
+
 	aconnect@702c0000 {
 		compatible = "nvidia,tegra210-aconnect";
 		clocks = <&tegra_car TEGRA210_CLK_APE>,
-- 
1.9.1

[PATCH v3 11/11] arm64: dts: nvidia: Tegra210 CPU clock definition

[Peter De Schrijver]

Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
---
 arch/arm64/boot/dts/nvidia/tegra210.dtsi | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm64/boot/dts/nvidia/tegra210.dtsi b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
index bc9851a..a7fddae 100644
--- a/arch/arm64/boot/dts/nvidia/tegra210.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
@@ -1229,6 +1229,13 @@
 			device_type = "cpu";
 			compatible = "arm,cortex-a57";
 			reg = <0>;
+			clocks = <&tegra_car TEGRA210_CLK_CCLK_G>,
+				 <&tegra_car TEGRA210_CLK_PLL_X>,
+				 <&tegra_car TEGRA210_CLK_PLL_P_OUT_CPU>,
+				 <&dfll>;
+			clock-names = "cpu_g", "pll_x", "pll_p", "dfll";
+			/* FIXME: what's the actual transition time? */
+			clock-latency = <300000>;
 		};
 
 		cpu@1 {
-- 
1.9.1

Re: [PATCH v3 01/11] regulator: core: add API to get voltage constraints

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 219 bytes --]

On Tue, Feb 06, 2018 at 06:34:02PM +0200, Peter De Schrijver wrote:
> From: Laxman Dewangan <ldewangan@nvidia.com>
> 
> Add API to get min/max rail voltage configured from platform for
> given rails.

because...

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v3 01/11] regulator: core: add API to get voltage constraints

[Peter De Schrijver]

On Tue, Feb 06, 2018 at 04:35:44PM +0000, Mark Brown wrote:
> * PGP Signed by an unknown key
> 
> On Tue, Feb 06, 2018 at 06:34:02PM +0200, Peter De Schrijver wrote:
> > From: Laxman Dewangan <ldewangan@nvidia.com>
> > 
> > Add API to get min/max rail voltage configured from platform for
> > given rails.
> 
> because...

of the next patch where this is used to retrieve the minimum rail voltage.

Peter.

Re: [PATCH v3 01/11] regulator: core: add API to get voltage constraints

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 423 bytes --]

On Wed, Feb 07, 2018 at 10:47:44AM +0200, Peter De Schrijver wrote:
> On Tue, Feb 06, 2018 at 04:35:44PM +0000, Mark Brown wrote:
> > On Tue, Feb 06, 2018 at 06:34:02PM +0200, Peter De Schrijver wrote:

> > > Add API to get min/max rail voltage configured from platform for
> > > given rails.

> > because...

> of the next patch where this is used to retrieve the minimum rail voltage.

And that in turn is needed for...?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v3 01/11] regulator: core: add API to get voltage constraints

[Peter De Schrijver]

On Wed, Feb 07, 2018 at 10:43:51AM +0000, Mark Brown wrote:
> On Wed, Feb 07, 2018 at 10:47:44AM +0200, Peter De Schrijver wrote:
> > On Tue, Feb 06, 2018 at 04:35:44PM +0000, Mark Brown wrote:
> > > On Tue, Feb 06, 2018 at 06:34:02PM +0200, Peter De Schrijver wrote:
> 
> > > > Add API to get min/max rail voltage configured from platform for
> > > > given rails.
> 
> > > because...
> 
> > of the next patch where this is used to retrieve the minimum rail voltage.
> 
> And that in turn is needed for...?

To calculate the required voltage for each frequency.

Peter.

Re: [PATCH v3 01/11] regulator: core: add API to get voltage constraints

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1065 bytes --]

On Wed, Feb 07, 2018 at 02:37:51PM +0200, Peter De Schrijver wrote:
> On Wed, Feb 07, 2018 at 10:43:51AM +0000, Mark Brown wrote:
> > On Wed, Feb 07, 2018 at 10:47:44AM +0200, Peter De Schrijver wrote:
> > > On Tue, Feb 06, 2018 at 04:35:44PM +0000, Mark Brown wrote:
> > > > On Tue, Feb 06, 2018 at 06:34:02PM +0200, Peter De Schrijver wrote:

> > > > > Add API to get min/max rail voltage configured from platform for
> > > > > given rails.

> > > > because...

> > > of the next patch where this is used to retrieve the minimum rail voltage.

> > And that in turn is needed for...?

> To calculate the required voltage for each frequency.

You're going to have to provide a much better explanation of what this
is doing - right now it seems like an abuse of constraints.  Client
drivers can already determine if a particular voltage they want to set
is available via regulator_list_voltage() and so on, that's what
constraints are there to set.  It sounds like you're trying to use them
for something else but you're really not explaining your use case
clearly.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v3 01/11] regulator: core: add API to get voltage constraints

[Peter De Schrijver]

On Wed, Feb 07, 2018 at 02:18:46PM +0000, Mark Brown wrote:
> On Wed, Feb 07, 2018 at 02:37:51PM +0200, Peter De Schrijver wrote:
> > On Wed, Feb 07, 2018 at 10:43:51AM +0000, Mark Brown wrote:
> > > On Wed, Feb 07, 2018 at 10:47:44AM +0200, Peter De Schrijver wrote:
> > > > On Tue, Feb 06, 2018 at 04:35:44PM +0000, Mark Brown wrote:
> > > > > On Tue, Feb 06, 2018 at 06:34:02PM +0200, Peter De Schrijver wrote:
> 
> > > > > > Add API to get min/max rail voltage configured from platform for
> > > > > > given rails.
> 
> > > > > because...
> 
> > > > of the next patch where this is used to retrieve the minimum rail voltage.
> 
> > > And that in turn is needed for...?
> 
> > To calculate the required voltage for each frequency.
> 
> You're going to have to provide a much better explanation of what this
> is doing - right now it seems like an abuse of constraints.  Client
> drivers can already determine if a particular voltage they want to set
> is available via regulator_list_voltage() and so on, that's what
> constraints are there to set.  It sounds like you're trying to use them
> for something else but you're really not explaining your use case
> clearly.

There is no way to query what voltage I will actually get for a given input
voltage. If you read drivers/clk/tegra/cvb. (you did do that right?), you
will see that there is a minimum and maximum voltage defined by
charaterization which needs to be capped to the regulator generated voltages
for those points.

Peter.

Re: [PATCH v3 01/11] regulator: core: add API to get voltage constraints

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1474 bytes --]

On Wed, Feb 07, 2018 at 04:32:13PM +0200, Peter De Schrijver wrote:
> On Wed, Feb 07, 2018 at 02:18:46PM +0000, Mark Brown wrote:

> > You're going to have to provide a much better explanation of what this
> > is doing - right now it seems like an abuse of constraints.  Client
> > drivers can already determine if a particular voltage they want to set
> > is available via regulator_list_voltage() and so on, that's what
> > constraints are there to set.  It sounds like you're trying to use them
> > for something else but you're really not explaining your use case
> > clearly.

> There is no way to query what voltage I will actually get for a given input

I looked at patch 2.  It looked like an abuse of what constraints do,
and had zero explanation of why it was doing what it was doing.  In any
case we need the regulator code and changelog to be clear about what the
interface is for and why it should be used, that's not happening here.

> voltage. If you read drivers/clk/tegra/cvb. (you did do that right?), you
> will see that there is a minimum and maximum voltage defined by
> charaterization which needs to be capped to the regulator generated voltages
> for those points.

I can't really tell what you're saying here.  If the driver needs to
know if it can set the a given voltage there's already an API for doing
that as I said.  If you're trying to convey this minimum and maximum
voltage via the constraints that sounds like an abuse of the constraints.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v3 01/11] regulator: core: add API to get voltage constraints

[Peter De Schrijver]

On Wed, Feb 07, 2018 at 03:01:55PM +0000, Mark Brown wrote:
> On Wed, Feb 07, 2018 at 04:32:13PM +0200, Peter De Schrijver wrote:
> > On Wed, Feb 07, 2018 at 02:18:46PM +0000, Mark Brown wrote:
> 
> > > You're going to have to provide a much better explanation of what this
> > > is doing - right now it seems like an abuse of constraints.  Client
> > > drivers can already determine if a particular voltage they want to set
> > > is available via regulator_list_voltage() and so on, that's what
> > > constraints are there to set.  It sounds like you're trying to use them
> > > for something else but you're really not explaining your use case
> > > clearly.
> 
> > There is no way to query what voltage I will actually get for a given input
> 
> I looked at patch 2.  It looked like an abuse of what constraints do,
> and had zero explanation of why it was doing what it was doing.  In any
> case we need the regulator code and changelog to be clear about what the
> interface is for and why it should be used, that's not happening here.
> 
> > voltage. If you read drivers/clk/tegra/cvb. (you did do that right?), you
> > will see that there is a minimum and maximum voltage defined by
> > charaterization which needs to be capped to the regulator generated voltages
> > for those points.
> 
> I can't really tell what you're saying here.  If the driver needs to
> know if it can set the a given voltage there's already an API for doing
> that as I said.  If you're trying to convey this minimum and maximum
> voltage via the constraints that sounds like an abuse of the constraints.


No, what I want is the voltage which the regulator will output for a given
regulator_set_voltage request taking constraints, regulator step etc into
account.

Peter.

Re: [PATCH v3 01/11] regulator: core: add API to get voltage constraints

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1028 bytes --]

On Wed, Feb 07, 2018 at 05:20:45PM +0200, Peter De Schrijver wrote:
> On Wed, Feb 07, 2018 at 03:01:55PM +0000, Mark Brown wrote:

> > I can't really tell what you're saying here.  If the driver needs to
> > know if it can set the a given voltage there's already an API for doing
> > that as I said.  If you're trying to convey this minimum and maximum
> > voltage via the constraints that sounds like an abuse of the constraints.

> No, what I want is the voltage which the regulator will output for a given
> regulator_set_voltage request taking constraints, regulator step etc into
> account.

Knowing the range of the constaints is going to tell you nothing useful
about that, it has zero information on steps or anything.  The way to
find out what voltages can be set is to enumerate the voltages that can
be set through the existing API and then if you want to set a specific
voltage that you've confirmed is available you can set exactly that
voltage via the normal voltage setting interface, no need to provide a
range.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v3 01/11] regulator: core: add API to get voltage constraints

[Laxman Dewangan]

On Wednesday 07 February 2018 09:07 PM, Mark Brown wrote:
> On Wed, Feb 07, 2018 at 05:20:45PM +0200, Peter De Schrijver wrote:
>> On Wed, Feb 07, 2018 at 03:01:55PM +0000, Mark Brown wrote:
>>> I can't really tell what you're saying here.  If the driver needs to
>>> know if it can set the a given voltage there's already an API for doing
>>> that as I said.  If you're trying to convey this minimum and maximum
>>> voltage via the constraints that sounds like an abuse of the constraints.
>> No, what I want is the voltage which the regulator will output for a given
>> regulator_set_voltage request taking constraints, regulator step etc into
>> account.
> Knowing the range of the constaints is going to tell you nothing useful
> about that, it has zero information on steps or anything.  The way to
> find out what voltages can be set is to enumerate the voltages that can
> be set through the existing API and then if you want to set a specific
> voltage that you've confirmed is available you can set exactly that
> voltage via the normal voltage setting interface, no need to provide a
> range.

Hi,
I added this API in downstream for the purpose of finding whether rail 
output can be changed or not(fixed) based on constraints.
If min and max is same then it can not change. I used this information 
to set the pad voltage of SOC automatically.

I dont know other usage when I added this API. May be Peter is needed here.

Re: [PATCH v3 01/11] regulator: core: add API to get voltage constraints

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1053 bytes --]

On Thu, Feb 08, 2018 at 03:34:56PM +0530, Laxman Dewangan wrote:

> I added this API in downstream for the purpose of finding whether rail
> output can be changed or not(fixed) based on constraints.
> If min and max is same then it can not change. I used this information to
> set the pad voltage of SOC automatically.

That's a reasonable thing to want to know.  The way the API wants you to
ask that question at the minute is to check if you can set the voltages
you might want to set and then if there's only one possible voltage you
can conclude that change is not possible.  I appreciate that that's a
bit of a long way round to do it, the thinking was to try to keep the
client drivers more robust in cases you can change the voltage but not
to all the values the driver wants.  I'd consider relaxing that and
having a direct API to ask about variability (just as a boolean) since
it does occur to me that this can be a useful speedup during init but I
need to think.

> I dont know other usage when I added this API. May be Peter is needed here.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v3 07/11] dt-bindings: tegra: Update DFLL binding for PWM regulator

[Rob Herring]

On Tue, Feb 06, 2018 at 06:34:08PM +0200, Peter De Schrijver wrote:
> Add new properties to configure the DFLL PWM regulator support. Also
> add an example and make the I2C clock only required when I2C support is
> used.
> 
> Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
> ---
>  .../bindings/clock/nvidia,tegra124-dfll.txt        | 76 +++++++++++++++++++++-
>  1 file changed, 74 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt b/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt
> index dff236f..a4903f7 100644
> --- a/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt
> +++ b/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt
> @@ -23,7 +23,8 @@ Required properties:
>  - clock-names: Must include the following entries:
>    - soc: Clock source for the DFLL control logic.
>    - ref: The closed loop reference clock
> -  - i2c: Clock source for the integrated I2C master.
> +  - i2c: Clock source for the integrated I2C master (only required when
> +	 using I2C mode).
>  - resets: Must contain an entry for each entry in reset-names.
>    See ../reset/reset.txt for details.
>  - reset-names: Must include the following entries:
> @@ -45,10 +46,28 @@ Required properties for the control loop parameters:
>  Optional properties for the control loop parameters:
>  - nvidia,cg-scale: Boolean value, see the field DFLL_PARAMS_CG_SCALE in the TRM.
>  
> +Optional properties for mode selection:
> +- nvidia,pwm-to-pmic: Use PWM to control regulator rather then I2C.
> +
>  Required properties for I2C mode:
>  - nvidia,i2c-fs-rate: I2C transfer rate, if using full speed mode.
>  
> -Example:
> +Required properties for PWM mode:
> +- nvidia,pwm-period: period of PWM square wave in us.

Add standard unit suffix.

> +- nvidia,init-uv: Regulator voltage in uV when PWM control is disabled.
> +- nvidia,align-offset-uv: Regulator voltage in uV when PWM control is enabled
> +			  and PWM output is low.
> +- nvidia,align-step-uv: Voltage increase in uV corresponding to a 1/33th

Use the standard unit suffix, not a custom one. See property-units.txt.

> +			increase in duty cycle. Eg the voltage for 2/33th duty
> +			cycle would be:
> +			nvidia,align-offset-uv + nvidia,align-step-uv * 2.
> +- pinctrl-0: I/O pad configuration when PWM control is enabled.
> +- pinctrl-1: I/O pad configuration when PWM control is disabled.
> +- pinctrl-names: must include the following entries:
> +  - dvfs_pwm_enable: I/O pad configuration when PWM control is enabled.
> +  - dvfs_pwm_disable: I/O pad configuration when PWM control is disabled.
> +
> +Example for I2C:
>  
>  clock@70110000 {
>          compatible = "nvidia,tegra124-dfll";
> @@ -76,3 +95,56 @@ clock@70110000 {
>  
>          nvidia,i2c-fs-rate = <400000>;
>  };
> +
> +Example for PWM:
> +
> +clock@70110000 {
> +        compatible = "nvidia,tegra124-dfll";
> +        reg = <0 0x70110000 0 0x100>, /* DFLL control */
> +              <0 0x70110000 0 0x100>, /* I2C output control */
> +              <0 0x70110100 0 0x100>, /* Integrated I2C controller */
> +              <0 0x70110200 0 0x100>; /* Look-up table RAM */
> +        interrupts = <GIC_SPI 62 IRQ_TYPE_LEVEL_HIGH>;
> +        clocks = <&tegra_car TEGRA210_CLK_DFLL_SOC>,
> +                 <&tegra_car TEGRA210_CLK_DFLL_REF>;
> +        clock-names = "soc", "ref";
> +        resets = <&tegra_car TEGRA124_RST_DFLL_DVCO>;
> +        reset-names = "dvco";
> +        #clock-cells = <0>;
> +        clock-output-names = "dfllCPU_out";
> +	nvidia,pwm-to-pmic;
> +	nvidia,init-uv = <1000000>;
> +	nvidia,align-step-uv = <19200>; /* 19.2mV */
> +	nvidia,align-offset-uv = <708000>; /* 708mV */
> +	nvidia,sample-rate = <25000>;
> +	nvidia,droop-ctrl = <0x00000f00>;
> +	nvidia,force-mode = <1>;
> +	nvidia,cf = <6>;
> +	nvidia,ci = <0>;
> +	nvidia,cg = <2>;
> +	nvidia,idle-override;
> +	nvidia,one-shot-calibrate;
> +	nvidia,pwm-period = <2500>; /* 2.5us */
> +	pinctrl-names = "dvfs_pwm_enable", "dvfs_pwm_disable";
> +	pinctrl-0 = <&dvfs_pwm_active_state>;
> +	pinctrl-1 = <&dvfs_pwm_inactive_state>;
> +};
> +
> +/* pinmux nodes added for completeness. Binding doc can be found in:
> + * Documentation/devicetree/bindings/pinctrl/nvidia,tegra210-pinmux.txt
> + */
> +
> +pinmux: pinmux@700008d4 {
> +	dvfs_pwm_active_state: dvfs_pwm_active {
> +		dvfs_pwm_pbb1 {
> +			nvidia,pins = "dvfs_pwm_pbb1";
> +			nvidia,tristate = <TEGRA_PIN_DISABLE>;
> +		};
> +	};
> +	dvfs_pwm_inactive_state: dvfs_pwm_inactive {
> +		dvfs_pwm_pbb1 {
> +			nvidia,pins = "dvfs_pwm_pbb1";
> +			nvidia,tristate = <TEGRA_PIN_ENABLE>;
> +		};
> +	};
> +};
> -- 
> 1.9.1
> 

Re: [PATCH v3 03/11] clk: tegra: dfll registration for multiple SoCs

[Jon Hunter]

On 06/02/18 16:34, Peter De Schrijver wrote:
> In a future patch, support for the DFLL in Tegra210 will be introduced.
> This requires support for more than 1 set of CVB and CPU max frequency
> tables.
> 
> Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
> ---
>  drivers/clk/tegra/clk-tegra124-dfll-fcpu.c | 37 ++++++++++++++++++++++++------
>  1 file changed, 30 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c b/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c
> index e2dbb79..6486ad9 100644
> --- a/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c
> +++ b/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c
> @@ -21,6 +21,7 @@
>  #include <linux/err.h>
>  #include <linux/kernel.h>
>  #include <linux/init.h>
> +#include <linux/of_device.h>
>  #include <linux/platform_device.h>
>  #include <linux/regulator/consumer.h>
>  #include <soc/tegra/fuse.h>
> @@ -29,8 +30,15 @@
>  #include "clk-dfll.h"
>  #include "cvb.h"
>  
> +struct dfll_fcpu_data {
> +	const unsigned long *cpu_max_freq_table;
> +	unsigned int cpu_max_freq_table_size;
> +	const struct cvb_table *cpu_cvb_tables;
> +	unsigned int cpu_cvb_tables_size;
> +};
> +
>  /* Maximum CPU frequency, indexed by CPU speedo id */
> -static const unsigned long cpu_max_freq_table[] = {
> +static const unsigned long tegra124_cpu_max_freq_table[] = {
>  	[0] = 2014500000UL,
>  	[1] = 2320500000UL,
>  	[2] = 2116500000UL,
> @@ -80,6 +88,21 @@
>  	},
>  };
>  
> +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),
> +	.cpu_cvb_tables = tegra124_cpu_cvb_tables,
> +	.cpu_cvb_tables_size = ARRAY_SIZE(tegra124_cpu_cvb_tables)
> +};
> +
> +static const struct of_device_id tegra124_dfll_fcpu_of_match[] = {
> +	{
> +		.compatible = "nvidia,tegra124-dfll",
> +		.data = &tegra124_dfll_fcpu_data,
> +	},
> +	{ },
> +};
> +
>  static int get_alignment_from_regulator(struct device *dev,
>  					struct rail_alignment *align)
>  {
> @@ -112,12 +135,17 @@ static int tegra124_dfll_fcpu_probe(struct platform_device *pdev)
>  {
>  	int process_id, speedo_id, speedo_value, err;
>  	struct tegra_dfll_soc_data *soc;
> +	const struct of_device_id *of_id;
> +	const struct dfll_fcpu_data *fcpu_data;
> +
> +	of_id = of_match_device(tegra124_dfll_fcpu_of_match, &pdev->dev);
> +	fcpu_data = of_id->data;

Nit-pick, we can use of_device_get_match_data() here.


>  	process_id = tegra_sku_info.cpu_process_id;
>  	speedo_id = tegra_sku_info.cpu_speedo_id;
>  	speedo_value = tegra_sku_info.cpu_speedo_value;
>  
> -	if (speedo_id >= ARRAY_SIZE(cpu_max_freq_table)) {
> +	if (speedo_id >= fcpu_data->cpu_max_freq_table_size) {
>  		dev_err(&pdev->dev, "unknown max CPU freq for speedo_id=%d\n",
>  			speedo_id);
>  		return -ENODEV;
> @@ -172,11 +200,6 @@ static int tegra124_dfll_fcpu_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> -static const struct of_device_id tegra124_dfll_fcpu_of_match[] = {
> -	{ .compatible = "nvidia,tegra124-dfll", },
> -	{ },
> -};
> -
>  static const struct dev_pm_ops tegra124_dfll_pm_ops = {
>  	SET_RUNTIME_PM_OPS(tegra_dfll_runtime_suspend,
>  			   tegra_dfll_runtime_resume, NULL)
> 

Otherwise ...

Reviewed-by: Jon Hunter <jonathanh@nvidia.com>

Cheers
Jon

-- 
nvpublic

Re: [PATCH v3 02/11] clk: tegra: retrieve regulator info from framework

[Jon Hunter]

On 06/02/18 16:34, Peter De Schrijver wrote:
> The CVB table contains calibration data for the CPU DFLL based on
> process charaterization. The regulator step and offset parameters depend
> on the regulator supplying vdd-cpu , not on the specific Tegra SKU.
> Hence than hardcoding those regulator parameters in the CVB table,
> retrieve them from the regulator framework and store them as part of the
> tegra_dfll_soc_data struct.
> 
> Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
> ---
>  drivers/clk/tegra/clk-dfll.h               |  2 ++
>  drivers/clk/tegra/clk-tegra124-dfll-fcpu.c | 42 +++++++++++++++++++++++++-----
>  drivers/clk/tegra/cvb.c                    | 16 +++++++++---
>  drivers/clk/tegra/cvb.h                    |  6 ++---
>  4 files changed, 53 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/clk/tegra/clk-dfll.h b/drivers/clk/tegra/clk-dfll.h
> index 83352c8..e7cbc5b 100644
> --- a/drivers/clk/tegra/clk-dfll.h
> +++ b/drivers/clk/tegra/clk-dfll.h
> @@ -21,6 +21,7 @@
>  #include <linux/platform_device.h>
>  #include <linux/reset.h>
>  #include <linux/types.h>
> +#include "cvb.h"
>  
>  /**
>   * struct tegra_dfll_soc_data - SoC-specific hooks/integration for the DFLL driver
> @@ -35,6 +36,7 @@ struct tegra_dfll_soc_data {
>  	struct device *dev;
>  	unsigned long max_freq;
>  	const struct cvb_table *cvb;
> +	struct rail_alignment alignment;
>  
>  	void (*init_clock_trimmers)(void);
>  	void (*set_clock_trimmers_high)(void);
> diff --git a/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c b/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c
> index 269d359..e2dbb79 100644
> --- a/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c
> +++ b/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c
> @@ -22,6 +22,7 @@
>  #include <linux/kernel.h>
>  #include <linux/init.h>
>  #include <linux/platform_device.h>
> +#include <linux/regulator/consumer.h>
>  #include <soc/tegra/fuse.h>
>  
>  #include "clk.h"
> @@ -42,9 +43,6 @@
>  		.process_id = -1,
>  		.min_millivolts = 900,
>  		.max_millivolts = 1260,
> -		.alignment = {
> -			.step_uv = 10000, /* 10mV */
> -		},
>  		.speedo_scale = 100,
>  		.voltage_scale = 1000,
>  		.entries = {
> @@ -82,6 +80,34 @@
>  	},
>  };
>  
> +static int get_alignment_from_regulator(struct device *dev,
> +					struct rail_alignment *align)
> +{
> +	int min_uV, max_uV, n_voltages, ret;
> +	struct regulator *reg;
> +
> +	reg = devm_regulator_get(dev, "vdd-cpu");
> +	if (IS_ERR(reg))
> +		return PTR_ERR(reg);
> +
> +	ret = regulator_get_constraint_voltages(reg, &min_uV, &max_uV);
> +	if (!ret)
> +		align->offset_uv = min_uV;
> +	else
> +		return ret;

Nit-pick ... looks a bit odd, why not ...

	if (ret)
		return ret;

	align->offset_uv = min_uV;

> +
> +	align->step_uv = regulator_get_linear_step(reg);
> +	if (!align->step_uv && !ret) {

Do you need to test 'ret' here?

> +		n_voltages = regulator_count_voltages(reg);
> +		if (n_voltages > 1)
> +			align->step_uv = (max_uV - min_uV) / (n_voltages - 1);

Later in the patch !align->step_uv is treated as an error, so if
n_voltages should always be greater 1 then why not return an error here?
Seems that this should not happen?

Cheers
Jon

-- 
nvpublic

Re: [PATCH v3 04/11] clk: tegra: add CVB tables for Tegra210 CPU DFLL

[Jon Hunter]

On 06/02/18 16:34, Peter De Schrijver wrote:

I think that you should have some description here.

> Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
> ---
>  drivers/clk/tegra/clk-tegra124-dfll-fcpu.c | 426 +++++++++++++++++++++++++++++
>  drivers/clk/tegra/cvb.h                    |   1 +
>  2 files changed, 427 insertions(+)
> 
> diff --git a/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c b/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c
> index 6486ad9..78dddab 100644
> --- a/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c
> +++ b/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c
> @@ -88,6 +88,421 @@ struct dfll_fcpu_data {
>  	},
>  };
>  
> +static const unsigned long tegra210_cpu_max_freq_table[] = {
> +	[0] = 1912500000UL,
> +	[1] = 1912500000UL,
> +	[2] = 2218500000UL,
> +	[3] = 1785000000UL,
> +	[4] = 1632000000UL,
> +	[5] = 1912500000UL,
> +	[6] = 2014500000UL,
> +	[7] = 1734000000UL,
> +	[8] = 1683000000UL,
> +	[9] = 1555500000UL,
> +	[10] = 1504500000UL,
> +};
> +
> +#define CPU_CVB_TABLE \
> +	.speedo_scale = 100,	\
> +	.voltage_scale = 1000,	\
> +	.entries = {		\
> +		{204000000UL,	{1007452, -23865, 370} }, \
> +		{306000000UL,	{1052709, -24875, 370} }, \
> +		{408000000UL,	{1099069, -25895, 370} }, \
> +		{510000000UL,	{1146534, -26905, 370} }, \
> +		{612000000UL,	{1195102, -27915, 370} }, \
> +		{714000000UL,	{1244773, -28925, 370} }, \
> +		{816000000UL,	{1295549, -29935, 370} }, \
> +		{918000000UL,	{1347428, -30955, 370} }, \
> +		{1020000000UL,	{1400411, -31965, 370} }, \
> +		{1122000000UL,	{1454497, -32975, 370} }, \
> +		{1224000000UL,	{1509687, -33985, 370} }, \
> +		{1326000000UL,	{1565981, -35005, 370} }, \
> +		{1428000000UL,	{1623379, -36015, 370} }, \
> +		{1530000000UL,	{1681880, -37025, 370} }, \
> +		{1632000000UL,	{1741485, -38035, 370} }, \
> +		{1734000000UL,	{1802194, -39055, 370} }, \
> +		{1836000000UL,	{1864006, -40065, 370} }, \
> +		{1912500000UL,	{1910780, -40815, 370} }, \
> +		{2014500000UL,	{1227000,      0,   0} }, \
> +		{2218500000UL,	{1227000,      0,   0} }, \
> +		{0,		{      0,      0,   0} }, \
> +	}
> +
> +#define CPU_CVB_TABLE_XA \
> +	.speedo_scale = 100,	\
> +	.voltage_scale = 1000,	\
> +	.entries = {		\
> +		{204000000UL,	{1250024, -39785, 565} }, \
> +		{306000000UL,	{1297556, -41145, 565} }, \
> +		{408000000UL,	{1346718, -42505, 565} }, \
> +		{510000000UL,	{1397511, -43855, 565} }, \
> +		{612000000UL,	{1449933, -45215, 565} }, \
> +		{714000000UL,	{1503986, -46575, 565} }, \
> +		{816000000UL,	{1559669, -47935, 565} }, \
> +		{918000000UL,	{1616982, -49295, 565} }, \
> +		{1020000000UL,	{1675926, -50645, 565} }, \
> +		{1122000000UL,	{1736500, -52005, 565} }, \
> +		{1224000000UL,	{1798704, -53365, 565} }, \
> +		{1326000000UL,	{1862538, -54725, 565} }, \
> +		{1428000000UL,	{1928003, -56085, 565} }, \
> +		{1530000000UL,	{1995097, -57435, 565} }, \
> +		{1606500000UL,	{2046149, -58445, 565} }, \
> +		{1632000000UL,	{2063822, -58795, 565} }, \
> +		{0,		{      0,      0,   0} }, \
> +	}
> +
> +#define CPU_CVB_TABLE_EUCM1 \
> +	.speedo_scale = 100,	\
> +	.voltage_scale = 1000,	\
> +	.entries = {		\
> +		{204000000UL,	{734429, 0, 0} }, \
> +		{306000000UL,	{768191, 0, 0} }, \
> +		{408000000UL,	{801953, 0, 0} }, \
> +		{510000000UL,	{835715, 0, 0} }, \
> +		{612000000UL,	{869477, 0, 0} }, \
> +		{714000000UL,	{903239, 0, 0} }, \
> +		{816000000UL,	{937001, 0, 0} }, \
> +		{918000000UL,	{970763, 0, 0} }, \
> +		{1020000000UL,	{1004525, 0, 0} }, \
> +		{1122000000UL,	{1038287, 0, 0} }, \
> +		{1224000000UL,	{1072049, 0, 0} }, \
> +		{1326000000UL,	{1105811, 0, 0} }, \
> +		{1428000000UL,	{1130000, 0, 0} }, \
> +		{1555500000UL,	{1130000, 0, 0} }, \
> +		{1632000000UL,	{1170000, 0, 0} }, \
> +		{1734000000UL,	{1227500, 0, 0} }, \
> +		{0,		{      0, 0, 0} }, \
> +	}
> +
> +#define CPU_CVB_TABLE_EUCM2 \
> +	.speedo_scale = 100,	\
> +	.voltage_scale = 1000,	\
> +	.entries = {		\
> +		{204000000UL,	{742283, 0, 0} }, \
> +		{306000000UL,	{776249, 0, 0} }, \
> +		{408000000UL,	{810215, 0, 0} }, \
> +		{510000000UL,	{844181, 0, 0} }, \
> +		{612000000UL,	{878147, 0, 0} }, \
> +		{714000000UL,	{912113, 0, 0} }, \
> +		{816000000UL,	{946079, 0, 0} }, \
> +		{918000000UL,	{980045, 0, 0} }, \
> +		{1020000000UL,	{1014011, 0, 0} }, \
> +		{1122000000UL,	{1047977, 0, 0} }, \
> +		{1224000000UL,	{1081943, 0, 0} }, \
> +		{1326000000UL,	{1090000, 0, 0} }, \
> +		{1479000000UL,	{1090000, 0, 0} }, \
> +		{1555500000UL,	{1162000, 0, 0} }, \
> +		{1683000000UL,	{1195000, 0, 0} }, \
> +		{0,		{      0, 0, 0} }, \
> +	}
> +
> +#define CPU_CVB_TABLE_EUCM2_JOINT_RAIL \
> +	.speedo_scale = 100,	\
> +	.voltage_scale = 1000,	\
> +	.entries = {		\
> +		{204000000UL,	{742283, 0, 0} }, \
> +		{306000000UL,	{776249, 0, 0} }, \
> +		{408000000UL,	{810215, 0, 0} }, \
> +		{510000000UL,	{844181, 0, 0} }, \
> +		{612000000UL,	{878147, 0, 0} }, \
> +		{714000000UL,	{912113, 0, 0} }, \
> +		{816000000UL,	{946079, 0, 0} }, \
> +		{918000000UL,	{980045, 0, 0} }, \
> +		{1020000000UL,	{1014011, 0, 0} }, \
> +		{1122000000UL,	{1047977, 0, 0} }, \
> +		{1224000000UL,	{1081943, 0, 0} }, \
> +		{1326000000UL,	{1090000, 0, 0} }, \
> +		{1479000000UL,	{1090000, 0, 0} }, \
> +		{1504500000UL,	{1120000, 0, 0} }, \
> +		{0,		{      0, 0, 0} }, \
> +	}
> +
> +#define CPU_CVB_TABLE_ODN \
> +	.speedo_scale = 100,	\
> +	.voltage_scale = 1000,	\
> +	.entries = {		\
> +		{204000000UL,	{721094, 0, 0} }, \
> +		{306000000UL,	{754040, 0, 0} }, \
> +		{408000000UL,	{786986, 0, 0} }, \
> +		{510000000UL,	{819932, 0, 0} }, \
> +		{612000000UL,	{852878, 0, 0} }, \
> +		{714000000UL,	{885824, 0, 0} }, \
> +		{816000000UL,	{918770, 0, 0} }, \
> +		{918000000UL,	{915716, 0, 0} }, \
> +		{1020000000UL,	{984662, 0, 0} }, \
> +		{1122000000UL,	{1017608, 0, 0} }, \
> +		{1224000000UL,	{1050554, 0, 0} }, \
> +		{1326000000UL,	{1083500, 0, 0} }, \
> +		{1428000000UL,	{1116446, 0, 0} }, \
> +		{1581000000UL,	{1130000, 0, 0} }, \
> +		{1683000000UL,	{1168000, 0, 0} }, \
> +		{1785000000UL,	{1227500, 0, 0} }, \
> +		{0,		{      0, 0, 0} }, \
> +	}
> +
> +struct cvb_table tegra210_cpu_cvb_tables[] = {
> +	{
> +		.speedo_id = 10,
> +		.process_id = 0,
> +		.min_millivolts = 840,
> +		.max_millivolts = 1120,
> +		CPU_CVB_TABLE_EUCM2_JOINT_RAIL,
> +		.cpu_dfll_data = {
> +			.tune0_low = 0xffead0ff,
> +			.tune0_high = 0xffead0ff,
> +			.tune1 = 0x20091d9,
> +			.tune_high_min_millivolts = 864,
> +		}
> +	},
> +	{
> +		.speedo_id = 10,
> +		.process_id = 1,
> +		.min_millivolts = 840,
> +		.max_millivolts = 1120,
> +		CPU_CVB_TABLE_EUCM2_JOINT_RAIL,
> +		.cpu_dfll_data = {
> +			.tune0_low = 0xffead0ff,
> +			.tune0_high = 0xffead0ff,
> +			.tune1 = 0x20091d9,
> +			.tune_high_min_millivolts = 864,
> +		}
> +	},
> +	{
> +		.speedo_id = 9,
> +		.process_id = 0,
> +		.min_millivolts = 900,
> +		.max_millivolts = 1162,
> +		CPU_CVB_TABLE_EUCM2,
> +		.cpu_dfll_data = {
> +			.tune0_low = 0xffead0ff,
> +			.tune0_high = 0xffead0ff,
> +			.tune1 = 0x20091d9,
> +		}
> +	},
> +	{
> +		.speedo_id = 9,
> +		.process_id = 1,
> +		.min_millivolts = 900,
> +		.max_millivolts = 1162,
> +		CPU_CVB_TABLE_EUCM2,
> +		.cpu_dfll_data = {
> +			.tune0_low = 0xffead0ff,
> +			.tune0_high = 0xffead0ff,
> +			.tune1 = 0x20091d9,
> +		}
> +	},
> +	{
> +		.speedo_id = 8,
> +		.process_id = 0,
> +		.min_millivolts = 900,
> +		.max_millivolts = 1195,
> +		CPU_CVB_TABLE_EUCM2,
> +		.cpu_dfll_data = {
> +			.tune0_low = 0xffead0ff,
> +			.tune0_high = 0xffead0ff,
> +			.tune1 = 0x20091d9,
> +		}
> +	},
> +	{
> +		.speedo_id = 8,
> +		.process_id = 1,
> +		.min_millivolts = 900,
> +		.max_millivolts = 1195,
> +		CPU_CVB_TABLE_EUCM2,
> +		.cpu_dfll_data = {
> +			.tune0_low = 0xffead0ff,
> +			.tune0_high = 0xffead0ff,
> +			.tune1 = 0x20091d9,
> +		}
> +	},
> +	{
> +		.speedo_id = 7,
> +		.process_id = 0,
> +		.min_millivolts = 841,
> +		.max_millivolts = 1227,
> +		CPU_CVB_TABLE_EUCM1,
> +		.cpu_dfll_data = {
> +			.tune0_low = 0xffead0ff,
> +			.tune0_high = 0xffead0ff,
> +			.tune1 = 0x20091d9,
> +			.tune_high_min_millivolts = 864,
> +		}
> +	},
> +	{
> +		.speedo_id = 7,
> +		.process_id = 1,
> +		.min_millivolts = 841,
> +		.max_millivolts = 1227,
> +		CPU_CVB_TABLE_EUCM1,
> +		.cpu_dfll_data = {
> +			.tune0_low = 0xffead0ff,
> +			.tune0_high = 0xffead0ff,
> +			.tune1 = 0x20091d9,
> +			.tune_high_min_millivolts = 864,
> +		}
> +	},
> +	{
> +		.speedo_id = 6,
> +		.process_id = 0,
> +		.min_millivolts = 870,
> +		.max_millivolts = 1150,
> +		CPU_CVB_TABLE,
> +		.cpu_dfll_data = {
> +			.tune0_low = 0xffead0ff,
> +			.tune1 = 0x20091d9,
> +		}
> +	},
> +	{
> +		.speedo_id = 6,
> +		.process_id = 1,
> +		.min_millivolts = 870,
> +		.max_millivolts = 1150,
> +		CPU_CVB_TABLE,
> +		.cpu_dfll_data = {
> +			.tune0_low = 0xffead0ff,
> +			.tune1 = 0x25501d0,
> +		}
> +	},
> +	{
> +		.speedo_id = 5,
> +		.process_id = 0,
> +		.min_millivolts = 818,
> +		.max_millivolts = 1227,
> +		CPU_CVB_TABLE,
> +		.cpu_dfll_data = {
> +			.tune0_low = 0xffead0ff,
> +			.tune0_high = 0xffead0ff,
> +			.tune1 = 0x20091d9,
> +			.tune_high_min_millivolts = 864,
> +		}
> +	},
> +	{
> +		.speedo_id = 5,
> +		.process_id = 1,
> +		.min_millivolts = 818,
> +		.max_millivolts = 1227,
> +		CPU_CVB_TABLE,
> +		.cpu_dfll_data = {
> +			.tune0_low = 0xffead0ff,
> +			.tune0_high = 0xffead0ff,
> +			.tune1 = 0x25501d0,
> +			.tune_high_min_millivolts = 864,
> +		}
> +	},
> +	{
> +		.speedo_id = 4,
> +		.process_id = -1,
> +		.min_millivolts = 918,
> +		.max_millivolts = 1113,
> +		CPU_CVB_TABLE_XA,
> +		.cpu_dfll_data = {
> +			.tune0_low = 0xffead0ff,
> +			.tune1 = 0x17711BD,
> +		}
> +	},
> +	{
> +		.speedo_id = 3,
> +		.process_id = 0,
> +		.min_millivolts = 825,
> +		.max_millivolts = 1227,
> +		CPU_CVB_TABLE_ODN,
> +		.cpu_dfll_data = {
> +			.tune0_low = 0xffead0ff,
> +			.tune0_high = 0xffead0ff,
> +			.tune1 = 0x20091d9,
> +			.tune_high_min_millivolts = 864,
> +		}
> +	},
> +	{
> +		.speedo_id = 3,
> +		.process_id = 1,
> +		.min_millivolts = 825,
> +		.max_millivolts = 1227,
> +		CPU_CVB_TABLE_ODN,
> +		.cpu_dfll_data = {
> +			.tune0_low = 0xffead0ff,
> +			.tune0_high = 0xffead0ff,
> +			.tune1 = 0x25501d0,
> +			.tune_high_min_millivolts = 864,
> +		}
> +	},
> +	{
> +		.speedo_id = 2,
> +		.process_id = 0,
> +		.min_millivolts = 870,
> +		.max_millivolts = 1227,
> +		CPU_CVB_TABLE,
> +		.cpu_dfll_data = {
> +			.tune0_low = 0xffead0ff,
> +			.tune1 = 0x20091d9,
> +		}
> +	},
> +	{
> +		.speedo_id = 2,
> +		.process_id = 1,
> +		.min_millivolts = 870,
> +		.max_millivolts = 1227,
> +		CPU_CVB_TABLE,
> +		.cpu_dfll_data = {
> +			.tune0_low = 0xffead0ff,
> +			.tune1 = 0x25501d0,
> +		}
> +	},
> +	{
> +		.speedo_id = 1,
> +		.process_id = 0,
> +		.min_millivolts = 837,
> +		.max_millivolts = 1227,
> +		CPU_CVB_TABLE,
> +		.cpu_dfll_data = {
> +			.tune0_low = 0xffead0ff,
> +			.tune0_high = 0xffead0ff,
> +			.tune1 = 0x20091d9,
> +			.tune_high_min_millivolts = 864,
> +		}
> +	},
> +	{
> +		.speedo_id = 1,
> +		.process_id = 1,
> +		.min_millivolts = 837,
> +		.max_millivolts = 1227,
> +		CPU_CVB_TABLE,
> +		.cpu_dfll_data = {
> +			.tune0_low = 0xffead0ff,
> +			.tune0_high = 0xffead0ff,
> +			.tune1 = 0x25501d0,
> +			.tune_high_min_millivolts = 864,
> +		}
> +	},
> +	{
> +		.speedo_id = 0,
> +		.process_id = 0,
> +		.min_millivolts = 850,
> +		.max_millivolts = 1170,
> +		CPU_CVB_TABLE,
> +		.cpu_dfll_data = {
> +			.tune0_low = 0xffead0ff,
> +			.tune0_high = 0xffead0ff,
> +			.tune1 = 0x20091d9,
> +			.tune_high_min_millivolts = 864,
> +		}
> +	},
> +	{
> +		.speedo_id = 0,
> +		.process_id = 1,
> +		.min_millivolts = 850,
> +		.max_millivolts = 1170,
> +		CPU_CVB_TABLE,
> +		.cpu_dfll_data = {
> +			.tune0_low = 0xffead0ff,
> +			.tune0_high = 0xffead0ff,
> +			.tune1 = 0x25501d0,
> +			.tune_high_min_millivolts = 864,
> +		}
> +	},
> +};
> +
>  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),
> @@ -95,11 +510,22 @@ struct dfll_fcpu_data {
>  	.cpu_cvb_tables_size = ARRAY_SIZE(tegra124_cpu_cvb_tables)
>  };
>  
> +static const struct dfll_fcpu_data tegra210_dfll_fcpu_data = {
> +	.cpu_max_freq_table = tegra210_cpu_max_freq_table,
> +	.cpu_max_freq_table_size = ARRAY_SIZE(tegra210_cpu_max_freq_table),
> +	.cpu_cvb_tables = tegra210_cpu_cvb_tables,
> +	.cpu_cvb_tables_size = ARRAY_SIZE(tegra210_cpu_cvb_tables),
> +};
> +
>  static const struct of_device_id tegra124_dfll_fcpu_of_match[] = {
>  	{
>  		.compatible = "nvidia,tegra124-dfll",
>  		.data = &tegra124_dfll_fcpu_data,
>  	},
> +	{
> +		.compatible = "nvidia,tegra210-dfll",
> +		.data = &tegra210_dfll_fcpu_data
> +	},
>  	{ },
>  };
>  
> diff --git a/drivers/clk/tegra/cvb.h b/drivers/clk/tegra/cvb.h
> index bcf15a0..91a1941 100644
> --- a/drivers/clk/tegra/cvb.h
> +++ b/drivers/clk/tegra/cvb.h
> @@ -41,6 +41,7 @@ struct cvb_cpu_dfll_data {
>  	u32 tune0_low;
>  	u32 tune0_high;
>  	u32 tune1;
> +	unsigned int tune_high_min_millivolts;
>  };
>  
>  struct cvb_table {
> 

Otherwise ...

Acked-by: Jon Hunter <jonathanh@nvidia.com>

Cheers
Jon

-- 
nvpublic

Re: [PATCH v3 05/11] clk: tegra: prepare dfll driver for PWM regulator

[Jon Hunter]

On 06/02/18 16:34, Peter De Schrijver wrote:
> This patch prepares the dfll driver to work with PWM regulators.
> To do this we introduce a new array lut_uv which gives the voltage for
> a given index generated by the dfll logic. This index will then be
> translated to a PMIC voltage ID in case of I2C using the i2c_lut. In case
> of a PWM regulator, it will be used to determine the PWM duty cycle.
> We also introduce lut_bottom which holds the lowest voltage we will ever
> need. In case of I2C this can be set to zero because the i2c_lut will be
> initialized such that entry 0 will be the lowest voltage we will ever
> need. In case of PWM, the lowest voltage is determined by the regulator
> hardware so we need this software limit. Note that this is different
> from lut_min which gives the lowest voltage we allow taking temperature
> into account. In a future patchset we will update lut_vmin dynamically.
> Similarly lut_max will be the highest voltage allowed taking temperature
> into accouint. Also this will be updated dynamically once temperature
> dependence will be introduced.
> 
> Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
> ---
>  drivers/clk/tegra/clk-dfll.c | 62 ++++++++++++++++++++++++++------------------
>  1 file changed, 37 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/clk/tegra/clk-dfll.c b/drivers/clk/tegra/clk-dfll.c
> index 0a7deee..fa97763 100644
> --- a/drivers/clk/tegra/clk-dfll.c
> +++ b/drivers/clk/tegra/clk-dfll.c
> @@ -301,9 +301,10 @@ struct tegra_dfll {
>  	u32				i2c_slave_addr;
>  
>  	/* i2c_lut array entries are regulator framework selectors */
> -	unsigned			i2c_lut[MAX_DFLL_VOLTAGES];
> -	int				i2c_lut_size;
> -	u8				lut_min, lut_max, lut_safe;
> +	unsigned int			i2c_lut[MAX_DFLL_VOLTAGES];
> +	unsigned int			lut_uv[MAX_DFLL_VOLTAGES];
> +	int				lut_size;
> +	u8				lut_bottom, lut_min, lut_max, lut_safe;
>  };
>  
>  #define clk_hw_to_dfll(_hw) container_of(_hw, struct tegra_dfll, dfll_clk_hw)
> @@ -531,10 +532,10 @@ static void dfll_load_i2c_lut(struct tegra_dfll *td)
>  	u32 val;
>  
>  	for (i = 0; i < MAX_DFLL_VOLTAGES; i++) {
> -		if (i < td->lut_min)
> -			lut_index = td->lut_min;
> -		else if (i > td->lut_max)
> -			lut_index = td->lut_max;
> +		if (i < td->lut_bottom)
> +			lut_index = td->lut_bottom;
> +		else if (i > td->lut_size - 1)
> +			lut_index = td->lut_size - 1;
>  		else
>  			lut_index = i;
>  
> @@ -594,9 +595,9 @@ static void dfll_init_out_if(struct tegra_dfll *td)
>  {
>  	u32 val;
>  
> -	td->lut_min = 0;
> -	td->lut_max = td->i2c_lut_size - 1;
> -	td->lut_safe = td->lut_min + 1;
> +	td->lut_min = td->lut_bottom;
> +	td->lut_max = td->lut_size - 1;
> +	td->lut_safe = td->lut_min + (td->lut_min < td->lut_max ? 1 : 0);
>  
>  	dfll_i2c_writel(td, 0, DFLL_OUTPUT_CFG);
>  	val = (td->lut_safe << DFLL_OUTPUT_CFG_SAFE_SHIFT) |
> @@ -619,11 +620,11 @@ static void dfll_init_out_if(struct tegra_dfll *td)
>   */
>  
>  /**
> - * find_lut_index_for_rate - determine I2C LUT index for given DFLL rate
> + * find_lut_index_for_rate - determine LUT index for given DFLL rate
>   * @td: DFLL instance
>   * @rate: clock rate
>   *
> - * Determines the index of a I2C LUT entry for a voltage that approximately
> + * Determines the index of a LUT entry for a voltage that approximately
>   * produces the given DFLL clock rate. This is used when forcing a value
>   * to the integrator during rate changes. Returns -ENOENT if a suitable
>   * LUT index is not found.
> @@ -637,11 +638,11 @@ static int find_lut_index_for_rate(struct tegra_dfll *td, unsigned long rate)
>  	if (IS_ERR(opp))
>  		return PTR_ERR(opp);
>  
> -	uv = dev_pm_opp_get_voltage(opp);
> +	uv = dev_pm_opp_get_voltage(opp) / td->soc->alignment.step_uv;
>  	dev_pm_opp_put(opp);
>  
> -	for (i = 0; i < td->i2c_lut_size; i++) {
> -		if (regulator_list_voltage(td->vdd_reg, td->i2c_lut[i]) == uv)
> +	for (i = td->lut_bottom; i < td->lut_size; i++) {
> +		if ((td->lut_uv[i] / td->soc->alignment.step_uv) >= uv)
>  			return i;
>  	}
>  
> @@ -1377,15 +1378,17 @@ static int dfll_init(struct tegra_dfll *td)
>   */
>  static int find_vdd_map_entry_exact(struct tegra_dfll *td, int uV)
>  {
> -	int i, n_voltages, reg_uV;
> +	int i, n_voltages, reg_mult, align_mult;
>  
> +	align_mult = uV / td->soc->alignment.step_uv;
>  	n_voltages = regulator_count_voltages(td->vdd_reg);
>  	for (i = 0; i < n_voltages; i++) {
> -		reg_uV = regulator_list_voltage(td->vdd_reg, i);
> -		if (reg_uV < 0)
> +		reg_mult = regulator_list_voltage(td->vdd_reg, i) /
> +				td->soc->alignment.step_uv;
> +		if (reg_mult < 0)
>  			break;
>  
> -		if (uV == reg_uV)
> +		if (align_mult == reg_mult)
>  			return i;
>  	}
>  
> @@ -1399,15 +1402,17 @@ static int find_vdd_map_entry_exact(struct tegra_dfll *td, int uV)
>   * */
>  static int find_vdd_map_entry_min(struct tegra_dfll *td, int uV)
>  {
> -	int i, n_voltages, reg_uV;
> +	int i, n_voltages, reg_mult, align_mult;
>  
> +	align_mult = uV / td->soc->alignment.step_uv;
>  	n_voltages = regulator_count_voltages(td->vdd_reg);
>  	for (i = 0; i < n_voltages; i++) {
> -		reg_uV = regulator_list_voltage(td->vdd_reg, i);
> -		if (reg_uV < 0)
> +		reg_mult = regulator_list_voltage(td->vdd_reg, i) /
> +				td->soc->alignment.step_uv;
> +		if (reg_mult < 0)
>  			break;
>  
> -		if (uV <= reg_uV)
> +		if (align_mult <= reg_mult)
>  			return i;
>  	}
>  
> @@ -1450,8 +1455,10 @@ static int dfll_build_i2c_lut(struct tegra_dfll *td)
>  	if (lut < 0)
>  		goto out;
>  	td->i2c_lut[0] = lut;
> +	td->lut_bottom = 0;
>  
>  	for (j = 1, rate = 0; ; rate++) {
> +
>  		opp = dev_pm_opp_find_freq_ceil(td->soc->dev, &rate);
>  		if (IS_ERR(opp))
>  			break;
> @@ -1484,13 +1491,18 @@ static int dfll_build_i2c_lut(struct tegra_dfll *td)
>  		if (v >= v_max)
>  			break;
>  	}
> -	td->i2c_lut_size = j;
> +	td->lut_size = j;
>  
>  	if (!td->dvco_rate_min)
>  		dev_err(td->dev, "no opp above DFLL minimum voltage %d mV\n",
>  			td->soc->cvb->min_millivolts);
> -	else
> +	else {
>  		ret = 0;
> +		for (j = 0; j < td->lut_size; j++)
> +			td->lut_uv[j] =
> +				regulator_list_voltage(td->vdd_reg,
> +						       td->i2c_lut[j]);
> +	}
>  
>  out:
>  	return ret;
> 

I am a bit confused by this patch as I don't fully understand from the
description what is being changed. For example, there are a few places
where you are dividing by td->soc->alignment.step_uv, which seems to be
changing the calculations/behaviour for I2C unless I am missing something?

I would also consider renaming lut_bottom and lut_min as something like
lut_abs_min and lut_cur_min, to indicate that one is the absolute min
and the other is the current min depending on operating conditions.

Cheers
Jon

-- 
nvpublic

Re: [PATCH v3 06/11] clk: tegra: dfll: support PWM regulator control

[Jon Hunter]

On 06/02/18 16:34, Peter De Schrijver wrote:
> The DFLL can directly generate a PWM signal to control the regulator IC
> rather then sending i2c messages. This patch adds support for this mode.
> In this mode the hardware LUT is not used and also there is no regulator
> object involved because there is no way to control the regulator voltage
> without also changing the DFLL output frequency. Also the register debugfs
> file is slightly reworked to only show the i2c registers when i2c mode is
> in use. As there is no regulator object for the PWM regulator, its step and
> offset values are retrieved from DT instead.

It is unclear to me why we bother creating the LUT for PWM if it is not
used? Is this for debug to get an approximation? Why do we do this?

> Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
> ---
>  drivers/clk/tegra/clk-dfll.c               | 398 ++++++++++++++++++++++++++---
>  drivers/clk/tegra/clk-tegra124-dfll-fcpu.c |  23 +-
>  2 files changed, 382 insertions(+), 39 deletions(-)
> 
> diff --git a/drivers/clk/tegra/clk-dfll.c b/drivers/clk/tegra/clk-dfll.c
> index fa97763..228edb4 100644
> --- a/drivers/clk/tegra/clk-dfll.c
> +++ b/drivers/clk/tegra/clk-dfll.c
> @@ -243,6 +243,12 @@ enum dfll_tune_range {
>  	DFLL_TUNE_LOW = 1,
>  };
>  
> +
> +enum tegra_dfll_pmu_if {
> +	TEGRA_DFLL_PMU_I2C = 0,
> +	TEGRA_DFLL_PMU_PWM = 1,
> +};
> +
>  /**
>   * struct dfll_rate_req - target DFLL rate request data
>   * @rate: target frequency, after the postscaling
> @@ -294,17 +300,25 @@ struct tegra_dfll {
>  	u32				ci;
>  	u32				cg;
>  	bool				cg_scale;
> +	u32				reg_init_uV;
>  
>  	/* I2C interface parameters */
>  	u32				i2c_fs_rate;
>  	u32				i2c_reg;
>  	u32				i2c_slave_addr;
>  
> -	/* i2c_lut array entries are regulator framework selectors */
> +	/* lut array entries are regulator framework selectors or PWM values*/
>  	unsigned int			i2c_lut[MAX_DFLL_VOLTAGES];
>  	unsigned int			lut_uv[MAX_DFLL_VOLTAGES];
>  	int				lut_size;
>  	u8				lut_bottom, lut_min, lut_max, lut_safe;
> +
> +	/* PWM interface */
> +	enum tegra_dfll_pmu_if		pmu_if;
> +	unsigned long			pwm_rate;
> +	struct pinctrl			*pwm_pin;
> +	struct pinctrl_state		*pwm_enable_state;
> +	struct pinctrl_state		*pwm_disable_state;
>  };
>  
>  #define clk_hw_to_dfll(_hw) container_of(_hw, struct tegra_dfll, dfll_clk_hw)
> @@ -491,6 +505,36 @@ static void dfll_set_mode(struct tegra_dfll *td,
>  }
>  
>  /*
> + * DVCO rate control
> + */
> +
> +static unsigned long get_dvco_rate_below(struct tegra_dfll *td, u8 out_min)
> +{
> +	struct dev_pm_opp *opp;
> +	unsigned long rate, prev_rate;
> +	int uv, min_uv;
> +
> +	min_uv = td->lut_uv[out_min];
> +	for (rate = 0, prev_rate = 0; ; rate++) {
> +		rcu_read_lock();
> +		opp = dev_pm_opp_find_freq_ceil(td->soc->dev, &rate);
> +		if (IS_ERR(opp)) {
> +			rcu_read_unlock();
> +			break;
> +		}
> +		uv = dev_pm_opp_get_voltage(opp);
> +		rcu_read_unlock();
> +
> +		if (uv && uv > min_uv)
> +			return prev_rate;
> +
> +		prev_rate = rate;
> +	}
> +
> +	return prev_rate;
> +}
> +
> +/*
>   * DFLL-to-I2C controller interface
>   */
>  
> @@ -519,6 +563,119 @@ static int dfll_i2c_set_output_enabled(struct tegra_dfll *td, bool enable)
>  	return 0;
>  }
>  
> +
> +/*
> + * DFLL-to-PWM controller interface
> + */
> +
> +/**
> + * dfll_pwm_set_output_enabled - enable/disable PWM voltage requests
> + * @td: DFLL instance
> + * @enable: whether to enable or disable the PWM voltage requests
> + *
> + * Set the master enable control for PWM control value updates. If disabled,
> + * then the PWM signal is not driven. Also configure the PWM output pad
> + * to the appropriate state.
> + */
> +static int dfll_pwm_set_output_enabled(struct tegra_dfll *td, bool enable)
> +{
> +	int ret;
> +	u32 val, div;
> +
> +	if (enable) {
> +		ret = pinctrl_select_state(td->pwm_pin, td->pwm_enable_state);
> +		if (ret < 0) {
> +			dev_err(td->dev, "setting enable state failed\n");
> +			return ret;
> +		}
> +		val = dfll_readl(td, DFLL_OUTPUT_CFG);
> +		val &= ~DFLL_OUTPUT_CFG_PWM_DIV_MASK;
> +		div = DIV_ROUND_UP(td->ref_rate, td->pwm_rate);
> +		val |= (div << DFLL_OUTPUT_CFG_PWM_DIV_SHIFT)
> +				& DFLL_OUTPUT_CFG_PWM_DIV_MASK;
> +		dfll_writel(td, val, DFLL_OUTPUT_CFG);
> +		dfll_wmb(td);
> +
> +		val |= DFLL_OUTPUT_CFG_PWM_ENABLE;
> +		dfll_writel(td, val, DFLL_OUTPUT_CFG);
> +		dfll_wmb(td);
> +	} else {
> +		ret = pinctrl_select_state(td->pwm_pin, td->pwm_disable_state);
> +		if (ret < 0)
> +			dev_warn(td->dev, "setting disable state failed\n");
> +
> +		val = dfll_readl(td, DFLL_OUTPUT_CFG);
> +		val &= ~DFLL_OUTPUT_CFG_PWM_ENABLE;
> +		dfll_writel(td, val, DFLL_OUTPUT_CFG);
> +		dfll_wmb(td);
> +	}
> +
> +	return 0;
> +}
> +/**
> + * dfll_set_force_output_value - set fixed value for force output
> + * @td: DFLL instance
> + * @out_val: value to force output
> + *
> + * Set the fixed value for force output, DFLL will output this value when
> + * force output is enabled.
> + */
> +static u32 dfll_set_force_output_value(struct tegra_dfll *td, u8 out_val)
> +{
> +	u32 val = dfll_readl(td, DFLL_OUTPUT_FORCE);
> +
> +	val &= ~OUT_MASK;
> +	val = (val & DFLL_OUTPUT_FORCE_ENABLE) | (out_val & OUT_MASK);

This masking of out_val is not needed as you check in dfll_force_output().

> +	dfll_writel(td, val, DFLL_OUTPUT_FORCE);
> +	dfll_wmb(td);
> +
> +	return dfll_readl(td, DFLL_OUTPUT_FORCE);
> +}
> +
> +/**
> + * dfll_set_force_output_enabled - enable/disable force output
> + * @td: DFLL instance
> + * @enable: whether to enable or disable the force output
> + *
> + * Set the enable control for fouce output with fixed value.
> + */
> +static void dfll_set_force_output_enabled(struct tegra_dfll *td, bool enable)
> +{
> +	u32 val = dfll_readl(td, DFLL_OUTPUT_FORCE);
> +
> +	if (enable)
> +		val |= DFLL_OUTPUT_FORCE_ENABLE;
> +	else
> +		val &= ~DFLL_OUTPUT_FORCE_ENABLE;
> +
> +	dfll_writel(td, val, DFLL_OUTPUT_FORCE);
> +	dfll_wmb(td);
> +}
> +
> +/**
> + * dfll_i2c_set_output_enabled - enable/disable I2C PMIC voltage requests
> + * @td: DFLL instance
> + * @enable: whether to enable or disable the I2C voltage requests
> + *
> + * Set the master enable control for I2C control value updates. If disabled,
> + * then I2C control messages are inhibited, regardless of the DFLL mode.
> + */

The above description needs correcting.

> +static int dfll_force_output(struct tegra_dfll *td, unsigned int out_sel)
> +{
> +	u32 val;
> +
> +	if (out_sel > OUT_MASK)
> +		return -EINVAL;
> +
> +	val = dfll_set_force_output_value(td, out_sel);
> +	if ((td->mode < DFLL_CLOSED_LOOP) &&
> +	    !(val & DFLL_OUTPUT_FORCE_ENABLE)) {
> +		dfll_set_force_output_enabled(td, true);
> +	}
> +
> +	return 0;
> +}
> +
>  /**
>   * dfll_load_lut - load the voltage lookup table
>   * @td: struct tegra_dfll *
> @@ -599,20 +756,50 @@ static void dfll_init_out_if(struct tegra_dfll *td)
>  	td->lut_max = td->lut_size - 1;
>  	td->lut_safe = td->lut_min + (td->lut_min < td->lut_max ? 1 : 0);
>  
> -	dfll_i2c_writel(td, 0, DFLL_OUTPUT_CFG);
> -	val = (td->lut_safe << DFLL_OUTPUT_CFG_SAFE_SHIFT) |
> -		(td->lut_max << DFLL_OUTPUT_CFG_MAX_SHIFT) |
> -		(td->lut_min << DFLL_OUTPUT_CFG_MIN_SHIFT);
> -	dfll_i2c_writel(td, val, DFLL_OUTPUT_CFG);
> -	dfll_i2c_wmb(td);
> -
> -	dfll_writel(td, 0, DFLL_OUTPUT_FORCE);
> -	dfll_i2c_writel(td, 0, DFLL_INTR_EN);
> -	dfll_i2c_writel(td, DFLL_INTR_MAX_MASK | DFLL_INTR_MIN_MASK,
> -			DFLL_INTR_STS);
> -
> -	dfll_load_i2c_lut(td);
> -	dfll_init_i2c_if(td);
> +	if (td->pmu_if == TEGRA_DFLL_PMU_PWM) {
> +		int vinit = td->reg_init_uV;
> +		int vstep = td->soc->alignment.step_uv;
> +		int vmin = td->lut_uv[0];
> +
> +		/* clear DFLL_OUTPUT_CFG before setting new value */
> +		dfll_writel(td, 0, DFLL_OUTPUT_CFG);
> +		dfll_wmb(td);
> +
> +		val = dfll_readl(td, DFLL_OUTPUT_CFG);
> +		val |= (td->lut_safe << DFLL_OUTPUT_CFG_SAFE_SHIFT) |
> +		       (td->lut_max << DFLL_OUTPUT_CFG_MAX_SHIFT) |
> +		       (td->lut_min << DFLL_OUTPUT_CFG_MIN_SHIFT);
> +		dfll_writel(td, val, DFLL_OUTPUT_CFG);
> +		dfll_wmb(td);
> +
> +		dfll_writel(td, 0, DFLL_OUTPUT_FORCE);
> +		dfll_i2c_writel(td, 0, DFLL_INTR_EN);
> +		dfll_i2c_writel(td, DFLL_INTR_MAX_MASK | DFLL_INTR_MIN_MASK,
> +				DFLL_INTR_STS);
> +
> +		/* set initial voltage */
> +		if ((vinit >= vmin) && vstep) {
> +			unsigned int vsel;
> +
> +			vsel = DIV_ROUND_UP((vinit - vmin), vstep);
> +			dfll_force_output(td, vsel);
> +		}
> +	} else {
> +		dfll_i2c_writel(td, 0, DFLL_OUTPUT_CFG);
> +		val = (td->lut_safe << DFLL_OUTPUT_CFG_SAFE_SHIFT) |
> +			(td->lut_max << DFLL_OUTPUT_CFG_MAX_SHIFT) |
> +			(td->lut_min << DFLL_OUTPUT_CFG_MIN_SHIFT);
> +		dfll_i2c_writel(td, val, DFLL_OUTPUT_CFG);
> +		dfll_i2c_wmb(td);
> +
> +		dfll_writel(td, 0, DFLL_OUTPUT_FORCE);
> +		dfll_i2c_writel(td, 0, DFLL_INTR_EN);
> +		dfll_i2c_writel(td, DFLL_INTR_MAX_MASK | DFLL_INTR_MIN_MASK,
> +				DFLL_INTR_STS);
> +
> +		dfll_load_i2c_lut(td);
> +		dfll_init_i2c_if(td);
> +	}
>  }
>  
>  /*
> @@ -864,9 +1051,14 @@ static int dfll_lock(struct tegra_dfll *td)
>  			return -EINVAL;
>  		}
>  
> -		dfll_i2c_set_output_enabled(td, true);
> +		if (td->pmu_if == TEGRA_DFLL_PMU_PWM)
> +			dfll_pwm_set_output_enabled(td, true);
> +		else
> +			dfll_i2c_set_output_enabled(td, true);
> +
>  		dfll_set_mode(td, DFLL_CLOSED_LOOP);
>  		dfll_set_frequency_request(td, req);
> +		dfll_set_force_output_enabled(td, false);
>  		return 0;
>  
>  	default:
> @@ -890,7 +1082,10 @@ static int dfll_unlock(struct tegra_dfll *td)
>  	case DFLL_CLOSED_LOOP:
>  		dfll_set_open_loop_config(td);
>  		dfll_set_mode(td, DFLL_OPEN_LOOP);
> -		dfll_i2c_set_output_enabled(td, false);
> +		if (td->pmu_if == TEGRA_DFLL_PMU_PWM)
> +			dfll_pwm_set_output_enabled(td, false);
> +		else
> +			dfll_i2c_set_output_enabled(td, false);
>  		return 0;
>  
>  	case DFLL_OPEN_LOOP:
> @@ -1172,15 +1367,17 @@ static int attr_registers_show(struct seq_file *s, void *data)
>  		seq_printf(s, "[0x%02x] = 0x%08x\n", offs,
>  			   dfll_i2c_readl(td, offs));
>  
> -	seq_puts(s, "\nINTEGRATED I2C CONTROLLER REGISTERS:\n");
> -	offs = DFLL_I2C_CLK_DIVISOR;
> -	seq_printf(s, "[0x%02x] = 0x%08x\n", offs,
> -		   __raw_readl(td->i2c_controller_base + offs));
> -
> -	seq_puts(s, "\nLUT:\n");
> -	for (offs = 0; offs <  4 * MAX_DFLL_VOLTAGES; offs += 4)
> +	if (td->pmu_if == TEGRA_DFLL_PMU_I2C) {
> +		seq_puts(s, "\nINTEGRATED I2C CONTROLLER REGISTERS:\n");
> +		offs = DFLL_I2C_CLK_DIVISOR;
>  		seq_printf(s, "[0x%02x] = 0x%08x\n", offs,
> -			   __raw_readl(td->lut_base + offs));
> +			   __raw_readl(td->i2c_controller_base + offs));
> +
> +		seq_puts(s, "\nLUT:\n");
> +		for (offs = 0; offs <  4 * MAX_DFLL_VOLTAGES; offs += 4)
> +			seq_printf(s, "[0x%02x] = 0x%08x\n", offs,
> +				   __raw_readl(td->lut_base + offs));
> +	}
>  
>  	return 0;
>  }
> @@ -1289,6 +1486,9 @@ static int dfll_init_clks(struct tegra_dfll *td)
>  		return PTR_ERR(td->soc_clk);
>  	}
>  
> +	if (td->pmu_if != TEGRA_DFLL_PMU_I2C)
> +		return 0;
> +
>  	td->i2c_clk = devm_clk_get(td->dev, "i2c");
>  	if (IS_ERR(td->i2c_clk)) {
>  		dev_err(td->dev, "missing i2c clock\n");
> @@ -1420,6 +1620,52 @@ static int find_vdd_map_entry_min(struct tegra_dfll *td, int uV)
>  	return -EINVAL;
>  }
>  
> +/*
> + * Look-up table in h/w is ignored when PWM is used as DFLL interface to PMIC.
> + * In this case closed loop output is controlling duty cycle directly. The s/w
> + * look-up that maps PWM duty cycle to voltage is still built by this function.

Why?

> + */
> +static int dfll_build_lut_pwm(struct tegra_dfll *td, int v_max)
> +{
> +	int i, reg_volt;
> +	unsigned long rate;
> +	u8 lut_bottom = MAX_DFLL_VOLTAGES;
> +	int v_min = td->soc->cvb->min_millivolts * 1000;
> +
> +	for (i = 0; i < MAX_DFLL_VOLTAGES; i++) {
> +		reg_volt = td->lut_uv[i];
> +
> +		/* since opp voltage is exact mv */
> +		reg_volt = (reg_volt / 1000) * 1000;
> +		if (reg_volt > v_max)
> +			break;
> +
> +		if ((lut_bottom == MAX_DFLL_VOLTAGES) && (reg_volt >= v_min))
> +			lut_bottom = i;
> +	}
> +
> +	/* determine voltage boundaries */
> +	td->lut_size = i;
> +	if ((lut_bottom == MAX_DFLL_VOLTAGES) ||
> +	    (lut_bottom + 1 >= td->lut_size)) {
> +		dev_err(td->dev, "no voltage above DFLL minimum %d mV\n",
> +			td->soc->cvb->min_millivolts);
> +		return -EINVAL;
> +	}
> +	td->lut_bottom = lut_bottom;
> +
> +	/* determine rate boundaries */
> +	rate = get_dvco_rate_below(td, td->lut_bottom);
> +	if (!rate) {
> +		dev_err(td->dev, "no opp below DFLL minimum voltage %d mV\n",
> +			td->soc->cvb->min_millivolts);
> +		return -EINVAL;
> +	}
> +	td->dvco_rate_min = rate;
> +
> +	return 0;
> +}
> +
>  /**
>   * dfll_build_i2c_lut - build the I2C voltage register lookup table
>   * @td: DFLL instance
> @@ -1432,10 +1678,10 @@ static int find_vdd_map_entry_min(struct tegra_dfll *td, int uV)
>   *
>   * On success, fills in td->i2c_lut and returns 0, or -err on failure.
>   */
> -static int dfll_build_i2c_lut(struct tegra_dfll *td)
> +static int dfll_build_i2c_lut(struct tegra_dfll *td, int v_max)
>  {
>  	int ret = -EINVAL;
> -	int j, v, v_max, v_opp;
> +	int j, v, v_opp;
>  	int selector;
>  	unsigned long rate;
>  	struct dev_pm_opp *opp;
> @@ -1508,6 +1754,30 @@ static int dfll_build_i2c_lut(struct tegra_dfll *td)
>  	return ret;
>  }
>  
> +static int dfll_build_lut(struct tegra_dfll *td)
> +{
> +	unsigned long rate;
> +	struct dev_pm_opp *opp;
> +	int v_max;
> +
> +	rcu_read_lock();
> +
> +	rate = ULONG_MAX;
> +	opp = dev_pm_opp_find_freq_floor(td->soc->dev, &rate);
> +	if (IS_ERR(opp)) {
> +		dev_err(td->dev, "couldn't get vmax opp, empty opp table?\n");
> +		return -EINVAL;
> +	}
> +	v_max = dev_pm_opp_get_voltage(opp);
> +
> +	rcu_read_unlock();
> +
> +	if (td->pmu_if == TEGRA_DFLL_PMU_PWM)
> +		return dfll_build_lut_pwm(td, v_max);
> +	else
> +		return dfll_build_i2c_lut(td, v_max);
> +}
> +
>  /**
>   * read_dt_param - helper function for reading required parameters from the DT
>   * @td: DFLL instance
> @@ -1566,11 +1836,54 @@ static int dfll_fetch_i2c_params(struct tegra_dfll *td)
>  	}
>  	td->i2c_reg = vsel_reg;
>  
> -	ret = dfll_build_i2c_lut(td);
> -	if (ret) {
> -		dev_err(td->dev, "couldn't build I2C LUT\n");
> +	return 0;
> +}
> +
> +static int dfll_fetch_pwm_params(struct tegra_dfll *td)
> +{
> +	int ret, i;
> +	u32 pwm_period;
> +
> +	if (!td->soc->alignment.step_uv || !td->soc->alignment.offset_uv) {
> +		dev_err(td->dev, "Missing step or alignment info for PWM regulator");
> +		return -EINVAL;
> +	}
> +	for (i = 0; i < MAX_DFLL_VOLTAGES; i++)
> +		td->lut_uv[i] = td->soc->alignment.offset_uv +
> +				i * td->soc->alignment.step_uv;
> +
> +	ret = read_dt_param(td, "nvidia,init-uv", &td->reg_init_uV);
> +	if (!ret) {
> +		dev_err(td->dev, "couldn't get initialized voltage\n");
> +		return ret;
> +	}
> +
> +	ret = read_dt_param(td, "nvidia,pwm-period", &pwm_period);
> +	if (!ret) {
> +		dev_err(td->dev, "couldn't get PWM period\n");
>  		return ret;
>  	}
> +	td->pwm_rate = (NSEC_PER_SEC / pwm_period) * (MAX_DFLL_VOLTAGES - 1);

Do we need to check that this pwm_rate is not too big so we don't end up
with a bad value in dfll_pwm_set_output_enabled()? If this pwm_rate is
not updated and neither is ref_rate, can we not just store the divisor
so we can use in dfll_pwm_set_output_enabled()?

Cheers
Jon

-- 
nvpublic

Re: [PATCH v3 07/11] dt-bindings: tegra: Update DFLL binding for PWM regulator

[Jon Hunter]

On 06/02/18 16:34, Peter De Schrijver wrote:
> Add new properties to configure the DFLL PWM regulator support. Also
> add an example and make the I2C clock only required when I2C support is
> used.
> 
> Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
> ---
>  .../bindings/clock/nvidia,tegra124-dfll.txt        | 76 +++++++++++++++++++++-
>  1 file changed, 74 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt b/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt
> index dff236f..a4903f7 100644
> --- a/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt
> +++ b/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt
> @@ -23,7 +23,8 @@ Required properties:
>  - clock-names: Must include the following entries:
>    - soc: Clock source for the DFLL control logic.
>    - ref: The closed loop reference clock
> -  - i2c: Clock source for the integrated I2C master.
> +  - i2c: Clock source for the integrated I2C master (only required when
> +	 using I2C mode).
>  - resets: Must contain an entry for each entry in reset-names.
>    See ../reset/reset.txt for details.
>  - reset-names: Must include the following entries:
> @@ -45,10 +46,28 @@ Required properties for the control loop parameters:
>  Optional properties for the control loop parameters:
>  - nvidia,cg-scale: Boolean value, see the field DFLL_PARAMS_CG_SCALE in the TRM.
>  
> +Optional properties for mode selection:
> +- nvidia,pwm-to-pmic: Use PWM to control regulator rather then I2C.
> +

Do we need this property? Seems that we should be able to detect if it
is I2C or PWM based upon the other properties.

Cheers
Jon

-- 
nvpublic

Re: [PATCH v3 08/11] clk: tegra: build clk-dfll.c for Tegra124 and Tegra210

[Jon Hunter]

On 06/02/18 16:34, Peter De Schrijver wrote:
> Tegra210 has a DFLL as well and can share the majority of the code with
> the Tegra124 implementation. So build the same code for both platforms.
> 
> Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
> ---
>  drivers/clk/tegra/Kconfig  | 5 +++++
>  drivers/clk/tegra/Makefile | 2 +-
>  2 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/tegra/Kconfig b/drivers/clk/tegra/Kconfig
> index 7ddacae..57902ab 100644
> --- a/drivers/clk/tegra/Kconfig
> +++ b/drivers/clk/tegra/Kconfig
> @@ -5,3 +5,8 @@ config TEGRA_CLK_EMC
>  config CLK_TEGRA_BPMP
>  	def_bool y
>  	depends on TEGRA_BPMP
> +
> +config TEGRA_CLK_DFLL
> +	depends on (ARCH_TEGRA_124_SOC || ARCH_TEGRA_210_SOC)
> +	select PM_OPP
> +	def_bool y
> diff --git a/drivers/clk/tegra/Makefile b/drivers/clk/tegra/Makefile
> index b716923..fc7f4b4 100644
> --- a/drivers/clk/tegra/Makefile
> +++ b/drivers/clk/tegra/Makefile
> @@ -19,7 +19,7 @@ obj-$(CONFIG_ARCH_TEGRA_2x_SOC)         += clk-tegra20.o
>  obj-$(CONFIG_ARCH_TEGRA_3x_SOC)         += clk-tegra30.o
>  obj-$(CONFIG_ARCH_TEGRA_114_SOC)	+= clk-tegra114.o
>  obj-$(CONFIG_ARCH_TEGRA_124_SOC)	+= clk-tegra124.o
> -obj-$(CONFIG_ARCH_TEGRA_124_SOC)	+= clk-tegra124-dfll-fcpu.o
> +obj-$(CONFIG_TEGRA_CLK_DFLL)		+= clk-tegra124-dfll-fcpu.o
>  obj-$(CONFIG_ARCH_TEGRA_132_SOC)	+= clk-tegra124.o
>  obj-y					+= cvb.o
>  obj-$(CONFIG_ARCH_TEGRA_210_SOC)	+= clk-tegra210.o
> 

Reviewed-by: Jon Hunter <jonathanh@nvidia.com>

Cheers
Jon

-- 
nvpublic

Re: [PATCH v3 09/11] cpufreq: tegra124-cpufreq: extend to support Tegra210

[Jon Hunter]

On 06/02/18 16:34, Peter De Schrijver wrote:
> Tegra210 has a very similar CPU clocking scheme than Tegra124. So add
> support in this driver. Also allow for the case where the CPU voltage is
> controlled directly by the DFLL rather than by a separate regulator object.
> 
> Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
> ---
>  drivers/cpufreq/tegra124-cpufreq.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/cpufreq/tegra124-cpufreq.c b/drivers/cpufreq/tegra124-cpufreq.c
> index 4353025..f8e01a8 100644
> --- a/drivers/cpufreq/tegra124-cpufreq.c
> +++ b/drivers/cpufreq/tegra124-cpufreq.c
> @@ -64,7 +64,8 @@ static void tegra124_cpu_switch_to_pllx(struct tegra124_cpufreq_priv *priv)
>  {
>  	clk_set_parent(priv->cpu_clk, priv->pllp_clk);
>  	clk_disable_unprepare(priv->dfll_clk);
> -	regulator_sync_voltage(priv->vdd_cpu_reg);
> +	if (priv->vdd_cpu_reg)
> +		regulator_sync_voltage(priv->vdd_cpu_reg);
>  	clk_set_parent(priv->cpu_clk, priv->pllx_clk);
>  }
>  
> @@ -89,10 +90,10 @@ static int tegra124_cpufreq_probe(struct platform_device *pdev)
>  		return -ENODEV;
>  
>  	priv->vdd_cpu_reg = regulator_get(cpu_dev, "vdd-cpu");
> -	if (IS_ERR(priv->vdd_cpu_reg)) {
> -		ret = PTR_ERR(priv->vdd_cpu_reg);
> -		goto out_put_np;
> -	}
> +	if (IS_ERR(priv->vdd_cpu_reg) != -EPROBE_DEFER)
> +		priv->vdd_cpu_reg = NULL;
> +	else
> +		return -EPROBE_DEFER;

I am still not sure that we should rely on the fact that the regulator
is not present in DT to imply that we do not need it. I think that we
should be checking if we are using I2C mode here.

Cheers
Jon

-- 
nvpublic

Re: [PATCH v3 06/11] clk: tegra: dfll: support PWM regulator control

[Peter De Schrijver]

On Thu, Mar 08, 2018 at 11:15:17PM +0000, Jon Hunter wrote:
> 
> On 06/02/18 16:34, Peter De Schrijver wrote:
> > The DFLL can directly generate a PWM signal to control the regulator IC
> > rather then sending i2c messages. This patch adds support for this mode.
> > In this mode the hardware LUT is not used and also there is no regulator
> > object involved because there is no way to control the regulator voltage
> > without also changing the DFLL output frequency. Also the register debugfs
> > file is slightly reworked to only show the i2c registers when i2c mode is
> > in use. As there is no regulator object for the PWM regulator, its step and
> > offset values are retrieved from DT instead.
> 
> It is unclear to me why we bother creating the LUT for PWM if it is not
> used? Is this for debug to get an approximation? Why do we do this?
> 

lut_uv certainly is used. This makes it easier to abstract PWM vs i2c. It
would also be possible to replace every reference to lut_uv with a function
which calculates the corresponding voltage by either querying the regulator
framework in case of i2c or doing alignment.offset_uv + x * alignment.step_uv
in case of PWM. Doing this once seems a bit easier to me.

> > Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
> > ---
> >  drivers/clk/tegra/clk-dfll.c               | 398 ++++++++++++++++++++++++++---
> >  drivers/clk/tegra/clk-tegra124-dfll-fcpu.c |  23 +-
> >  2 files changed, 382 insertions(+), 39 deletions(-)
> > 
> > diff --git a/drivers/clk/tegra/clk-dfll.c b/drivers/clk/tegra/clk-dfll.c
> > index fa97763..228edb4 100644
> > --- a/drivers/clk/tegra/clk-dfll.c
> > +++ b/drivers/clk/tegra/clk-dfll.c
> > @@ -243,6 +243,12 @@ enum dfll_tune_range {
> >  	DFLL_TUNE_LOW = 1,
> >  };
> >  
> > +
> > +enum tegra_dfll_pmu_if {
> > +	TEGRA_DFLL_PMU_I2C = 0,
> > +	TEGRA_DFLL_PMU_PWM = 1,
> > +};
> > +
> >  /**
> >   * struct dfll_rate_req - target DFLL rate request data
> >   * @rate: target frequency, after the postscaling
> > @@ -294,17 +300,25 @@ struct tegra_dfll {
> >  	u32				ci;
> >  	u32				cg;
> >  	bool				cg_scale;
> > +	u32				reg_init_uV;
> >  
> >  	/* I2C interface parameters */
> >  	u32				i2c_fs_rate;
> >  	u32				i2c_reg;
> >  	u32				i2c_slave_addr;
> >  
> > -	/* i2c_lut array entries are regulator framework selectors */
> > +	/* lut array entries are regulator framework selectors or PWM values*/
> >  	unsigned int			i2c_lut[MAX_DFLL_VOLTAGES];
> >  	unsigned int			lut_uv[MAX_DFLL_VOLTAGES];
> >  	int				lut_size;
> >  	u8				lut_bottom, lut_min, lut_max, lut_safe;
> > +
> > +	/* PWM interface */
> > +	enum tegra_dfll_pmu_if		pmu_if;
> > +	unsigned long			pwm_rate;
> > +	struct pinctrl			*pwm_pin;
> > +	struct pinctrl_state		*pwm_enable_state;
> > +	struct pinctrl_state		*pwm_disable_state;
> >  };
> >  
> >  #define clk_hw_to_dfll(_hw) container_of(_hw, struct tegra_dfll, dfll_clk_hw)
> > @@ -491,6 +505,36 @@ static void dfll_set_mode(struct tegra_dfll *td,
> >  }
> >  
> >  /*
> > + * DVCO rate control
> > + */
> > +
> > +static unsigned long get_dvco_rate_below(struct tegra_dfll *td, u8 out_min)
> > +{
> > +	struct dev_pm_opp *opp;
> > +	unsigned long rate, prev_rate;
> > +	int uv, min_uv;
> > +
> > +	min_uv = td->lut_uv[out_min];
> > +	for (rate = 0, prev_rate = 0; ; rate++) {
> > +		rcu_read_lock();
> > +		opp = dev_pm_opp_find_freq_ceil(td->soc->dev, &rate);
> > +		if (IS_ERR(opp)) {
> > +			rcu_read_unlock();
> > +			break;
> > +		}
> > +		uv = dev_pm_opp_get_voltage(opp);
> > +		rcu_read_unlock();
> > +
> > +		if (uv && uv > min_uv)
> > +			return prev_rate;
> > +
> > +		prev_rate = rate;
> > +	}
> > +
> > +	return prev_rate;
> > +}
> > +
> > +/*
> >   * DFLL-to-I2C controller interface
> >   */
> >  
> > @@ -519,6 +563,119 @@ static int dfll_i2c_set_output_enabled(struct tegra_dfll *td, bool enable)
> >  	return 0;
> >  }
> >  
> > +
> > +/*
> > + * DFLL-to-PWM controller interface
> > + */
> > +
> > +/**
> > + * dfll_pwm_set_output_enabled - enable/disable PWM voltage requests
> > + * @td: DFLL instance
> > + * @enable: whether to enable or disable the PWM voltage requests
> > + *
> > + * Set the master enable control for PWM control value updates. If disabled,
> > + * then the PWM signal is not driven. Also configure the PWM output pad
> > + * to the appropriate state.
> > + */
> > +static int dfll_pwm_set_output_enabled(struct tegra_dfll *td, bool enable)
> > +{
> > +	int ret;
> > +	u32 val, div;
> > +
> > +	if (enable) {
> > +		ret = pinctrl_select_state(td->pwm_pin, td->pwm_enable_state);
> > +		if (ret < 0) {
> > +			dev_err(td->dev, "setting enable state failed\n");
> > +			return ret;
> > +		}
> > +		val = dfll_readl(td, DFLL_OUTPUT_CFG);
> > +		val &= ~DFLL_OUTPUT_CFG_PWM_DIV_MASK;
> > +		div = DIV_ROUND_UP(td->ref_rate, td->pwm_rate);
> > +		val |= (div << DFLL_OUTPUT_CFG_PWM_DIV_SHIFT)
> > +				& DFLL_OUTPUT_CFG_PWM_DIV_MASK;
> > +		dfll_writel(td, val, DFLL_OUTPUT_CFG);
> > +		dfll_wmb(td);
> > +
> > +		val |= DFLL_OUTPUT_CFG_PWM_ENABLE;
> > +		dfll_writel(td, val, DFLL_OUTPUT_CFG);
> > +		dfll_wmb(td);
> > +	} else {
> > +		ret = pinctrl_select_state(td->pwm_pin, td->pwm_disable_state);
> > +		if (ret < 0)
> > +			dev_warn(td->dev, "setting disable state failed\n");
> > +
> > +		val = dfll_readl(td, DFLL_OUTPUT_CFG);
> > +		val &= ~DFLL_OUTPUT_CFG_PWM_ENABLE;
> > +		dfll_writel(td, val, DFLL_OUTPUT_CFG);
> > +		dfll_wmb(td);
> > +	}
> > +
> > +	return 0;
> > +}
> > +/**
> > + * dfll_set_force_output_value - set fixed value for force output
> > + * @td: DFLL instance
> > + * @out_val: value to force output
> > + *
> > + * Set the fixed value for force output, DFLL will output this value when
> > + * force output is enabled.
> > + */
> > +static u32 dfll_set_force_output_value(struct tegra_dfll *td, u8 out_val)
> > +{
> > +	u32 val = dfll_readl(td, DFLL_OUTPUT_FORCE);
> > +
> > +	val &= ~OUT_MASK;
> > +	val = (val & DFLL_OUTPUT_FORCE_ENABLE) | (out_val & OUT_MASK);
> 
> This masking of out_val is not needed as you check in dfll_force_output().
> 
> > +	dfll_writel(td, val, DFLL_OUTPUT_FORCE);
> > +	dfll_wmb(td);
> > +
> > +	return dfll_readl(td, DFLL_OUTPUT_FORCE);
> > +}
> > +
> > +/**
> > + * dfll_set_force_output_enabled - enable/disable force output
> > + * @td: DFLL instance
> > + * @enable: whether to enable or disable the force output
> > + *
> > + * Set the enable control for fouce output with fixed value.
> > + */
> > +static void dfll_set_force_output_enabled(struct tegra_dfll *td, bool enable)
> > +{
> > +	u32 val = dfll_readl(td, DFLL_OUTPUT_FORCE);
> > +
> > +	if (enable)
> > +		val |= DFLL_OUTPUT_FORCE_ENABLE;
> > +	else
> > +		val &= ~DFLL_OUTPUT_FORCE_ENABLE;
> > +
> > +	dfll_writel(td, val, DFLL_OUTPUT_FORCE);
> > +	dfll_wmb(td);
> > +}
> > +
> > +/**
> > + * dfll_i2c_set_output_enabled - enable/disable I2C PMIC voltage requests
> > + * @td: DFLL instance
> > + * @enable: whether to enable or disable the I2C voltage requests
> > + *
> > + * Set the master enable control for I2C control value updates. If disabled,
> > + * then I2C control messages are inhibited, regardless of the DFLL mode.
> > + */
> 
> The above description needs correcting.
> 
> > +static int dfll_force_output(struct tegra_dfll *td, unsigned int out_sel)
> > +{
> > +	u32 val;
> > +
> > +	if (out_sel > OUT_MASK)
> > +		return -EINVAL;
> > +
> > +	val = dfll_set_force_output_value(td, out_sel);
> > +	if ((td->mode < DFLL_CLOSED_LOOP) &&
> > +	    !(val & DFLL_OUTPUT_FORCE_ENABLE)) {
> > +		dfll_set_force_output_enabled(td, true);
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> >  /**
> >   * dfll_load_lut - load the voltage lookup table
> >   * @td: struct tegra_dfll *
> > @@ -599,20 +756,50 @@ static void dfll_init_out_if(struct tegra_dfll *td)
> >  	td->lut_max = td->lut_size - 1;
> >  	td->lut_safe = td->lut_min + (td->lut_min < td->lut_max ? 1 : 0);
> >  
> > -	dfll_i2c_writel(td, 0, DFLL_OUTPUT_CFG);
> > -	val = (td->lut_safe << DFLL_OUTPUT_CFG_SAFE_SHIFT) |
> > -		(td->lut_max << DFLL_OUTPUT_CFG_MAX_SHIFT) |
> > -		(td->lut_min << DFLL_OUTPUT_CFG_MIN_SHIFT);
> > -	dfll_i2c_writel(td, val, DFLL_OUTPUT_CFG);
> > -	dfll_i2c_wmb(td);
> > -
> > -	dfll_writel(td, 0, DFLL_OUTPUT_FORCE);
> > -	dfll_i2c_writel(td, 0, DFLL_INTR_EN);
> > -	dfll_i2c_writel(td, DFLL_INTR_MAX_MASK | DFLL_INTR_MIN_MASK,
> > -			DFLL_INTR_STS);
> > -
> > -	dfll_load_i2c_lut(td);
> > -	dfll_init_i2c_if(td);
> > +	if (td->pmu_if == TEGRA_DFLL_PMU_PWM) {
> > +		int vinit = td->reg_init_uV;
> > +		int vstep = td->soc->alignment.step_uv;
> > +		int vmin = td->lut_uv[0];
> > +
> > +		/* clear DFLL_OUTPUT_CFG before setting new value */
> > +		dfll_writel(td, 0, DFLL_OUTPUT_CFG);
> > +		dfll_wmb(td);
> > +
> > +		val = dfll_readl(td, DFLL_OUTPUT_CFG);
> > +		val |= (td->lut_safe << DFLL_OUTPUT_CFG_SAFE_SHIFT) |
> > +		       (td->lut_max << DFLL_OUTPUT_CFG_MAX_SHIFT) |
> > +		       (td->lut_min << DFLL_OUTPUT_CFG_MIN_SHIFT);
> > +		dfll_writel(td, val, DFLL_OUTPUT_CFG);
> > +		dfll_wmb(td);
> > +
> > +		dfll_writel(td, 0, DFLL_OUTPUT_FORCE);
> > +		dfll_i2c_writel(td, 0, DFLL_INTR_EN);
> > +		dfll_i2c_writel(td, DFLL_INTR_MAX_MASK | DFLL_INTR_MIN_MASK,
> > +				DFLL_INTR_STS);
> > +
> > +		/* set initial voltage */
> > +		if ((vinit >= vmin) && vstep) {
> > +			unsigned int vsel;
> > +
> > +			vsel = DIV_ROUND_UP((vinit - vmin), vstep);
> > +			dfll_force_output(td, vsel);
> > +		}
> > +	} else {
> > +		dfll_i2c_writel(td, 0, DFLL_OUTPUT_CFG);
> > +		val = (td->lut_safe << DFLL_OUTPUT_CFG_SAFE_SHIFT) |
> > +			(td->lut_max << DFLL_OUTPUT_CFG_MAX_SHIFT) |
> > +			(td->lut_min << DFLL_OUTPUT_CFG_MIN_SHIFT);
> > +		dfll_i2c_writel(td, val, DFLL_OUTPUT_CFG);
> > +		dfll_i2c_wmb(td);
> > +
> > +		dfll_writel(td, 0, DFLL_OUTPUT_FORCE);
> > +		dfll_i2c_writel(td, 0, DFLL_INTR_EN);
> > +		dfll_i2c_writel(td, DFLL_INTR_MAX_MASK | DFLL_INTR_MIN_MASK,
> > +				DFLL_INTR_STS);
> > +
> > +		dfll_load_i2c_lut(td);
> > +		dfll_init_i2c_if(td);
> > +	}
> >  }
> >  
> >  /*
> > @@ -864,9 +1051,14 @@ static int dfll_lock(struct tegra_dfll *td)
> >  			return -EINVAL;
> >  		}
> >  
> > -		dfll_i2c_set_output_enabled(td, true);
> > +		if (td->pmu_if == TEGRA_DFLL_PMU_PWM)
> > +			dfll_pwm_set_output_enabled(td, true);
> > +		else
> > +			dfll_i2c_set_output_enabled(td, true);
> > +
> >  		dfll_set_mode(td, DFLL_CLOSED_LOOP);
> >  		dfll_set_frequency_request(td, req);
> > +		dfll_set_force_output_enabled(td, false);
> >  		return 0;
> >  
> >  	default:
> > @@ -890,7 +1082,10 @@ static int dfll_unlock(struct tegra_dfll *td)
> >  	case DFLL_CLOSED_LOOP:
> >  		dfll_set_open_loop_config(td);
> >  		dfll_set_mode(td, DFLL_OPEN_LOOP);
> > -		dfll_i2c_set_output_enabled(td, false);
> > +		if (td->pmu_if == TEGRA_DFLL_PMU_PWM)
> > +			dfll_pwm_set_output_enabled(td, false);
> > +		else
> > +			dfll_i2c_set_output_enabled(td, false);
> >  		return 0;
> >  
> >  	case DFLL_OPEN_LOOP:
> > @@ -1172,15 +1367,17 @@ static int attr_registers_show(struct seq_file *s, void *data)
> >  		seq_printf(s, "[0x%02x] = 0x%08x\n", offs,
> >  			   dfll_i2c_readl(td, offs));
> >  
> > -	seq_puts(s, "\nINTEGRATED I2C CONTROLLER REGISTERS:\n");
> > -	offs = DFLL_I2C_CLK_DIVISOR;
> > -	seq_printf(s, "[0x%02x] = 0x%08x\n", offs,
> > -		   __raw_readl(td->i2c_controller_base + offs));
> > -
> > -	seq_puts(s, "\nLUT:\n");
> > -	for (offs = 0; offs <  4 * MAX_DFLL_VOLTAGES; offs += 4)
> > +	if (td->pmu_if == TEGRA_DFLL_PMU_I2C) {
> > +		seq_puts(s, "\nINTEGRATED I2C CONTROLLER REGISTERS:\n");
> > +		offs = DFLL_I2C_CLK_DIVISOR;
> >  		seq_printf(s, "[0x%02x] = 0x%08x\n", offs,
> > -			   __raw_readl(td->lut_base + offs));
> > +			   __raw_readl(td->i2c_controller_base + offs));
> > +
> > +		seq_puts(s, "\nLUT:\n");
> > +		for (offs = 0; offs <  4 * MAX_DFLL_VOLTAGES; offs += 4)
> > +			seq_printf(s, "[0x%02x] = 0x%08x\n", offs,
> > +				   __raw_readl(td->lut_base + offs));
> > +	}
> >  
> >  	return 0;
> >  }
> > @@ -1289,6 +1486,9 @@ static int dfll_init_clks(struct tegra_dfll *td)
> >  		return PTR_ERR(td->soc_clk);
> >  	}
> >  
> > +	if (td->pmu_if != TEGRA_DFLL_PMU_I2C)
> > +		return 0;
> > +
> >  	td->i2c_clk = devm_clk_get(td->dev, "i2c");
> >  	if (IS_ERR(td->i2c_clk)) {
> >  		dev_err(td->dev, "missing i2c clock\n");
> > @@ -1420,6 +1620,52 @@ static int find_vdd_map_entry_min(struct tegra_dfll *td, int uV)
> >  	return -EINVAL;
> >  }
> >  
> > +/*
> > + * Look-up table in h/w is ignored when PWM is used as DFLL interface to PMIC.
> > + * In this case closed loop output is controlling duty cycle directly. The s/w
> > + * look-up that maps PWM duty cycle to voltage is still built by this function.
> 
> Why?

This actually incorrect, the table is built by dfll_fetch_pwm_params() in case
of PWM.

> 
> > + */
> > +static int dfll_build_lut_pwm(struct tegra_dfll *td, int v_max)
> > +{
> > +	int i, reg_volt;
> > +	unsigned long rate;
> > +	u8 lut_bottom = MAX_DFLL_VOLTAGES;
> > +	int v_min = td->soc->cvb->min_millivolts * 1000;
> > +
> > +	for (i = 0; i < MAX_DFLL_VOLTAGES; i++) {
> > +		reg_volt = td->lut_uv[i];
> > +
> > +		/* since opp voltage is exact mv */
> > +		reg_volt = (reg_volt / 1000) * 1000;
> > +		if (reg_volt > v_max)
> > +			break;
> > +
> > +		if ((lut_bottom == MAX_DFLL_VOLTAGES) && (reg_volt >= v_min))
> > +			lut_bottom = i;
> > +	}
> > +
> > +	/* determine voltage boundaries */
> > +	td->lut_size = i;
> > +	if ((lut_bottom == MAX_DFLL_VOLTAGES) ||
> > +	    (lut_bottom + 1 >= td->lut_size)) {
> > +		dev_err(td->dev, "no voltage above DFLL minimum %d mV\n",
> > +			td->soc->cvb->min_millivolts);
> > +		return -EINVAL;
> > +	}
> > +	td->lut_bottom = lut_bottom;
> > +
> > +	/* determine rate boundaries */
> > +	rate = get_dvco_rate_below(td, td->lut_bottom);
> > +	if (!rate) {
> > +		dev_err(td->dev, "no opp below DFLL minimum voltage %d mV\n",
> > +			td->soc->cvb->min_millivolts);
> > +		return -EINVAL;
> > +	}
> > +	td->dvco_rate_min = rate;
> > +
> > +	return 0;
> > +}
> > +
> >  /**
> >   * dfll_build_i2c_lut - build the I2C voltage register lookup table
> >   * @td: DFLL instance
> > @@ -1432,10 +1678,10 @@ static int find_vdd_map_entry_min(struct tegra_dfll *td, int uV)
> >   *
> >   * On success, fills in td->i2c_lut and returns 0, or -err on failure.
> >   */
> > -static int dfll_build_i2c_lut(struct tegra_dfll *td)
> > +static int dfll_build_i2c_lut(struct tegra_dfll *td, int v_max)
> >  {
> >  	int ret = -EINVAL;
> > -	int j, v, v_max, v_opp;
> > +	int j, v, v_opp;
> >  	int selector;
> >  	unsigned long rate;
> >  	struct dev_pm_opp *opp;
> > @@ -1508,6 +1754,30 @@ static int dfll_build_i2c_lut(struct tegra_dfll *td)
> >  	return ret;
> >  }
> >  
> > +static int dfll_build_lut(struct tegra_dfll *td)
> > +{
> > +	unsigned long rate;
> > +	struct dev_pm_opp *opp;
> > +	int v_max;
> > +
> > +	rcu_read_lock();
> > +
> > +	rate = ULONG_MAX;
> > +	opp = dev_pm_opp_find_freq_floor(td->soc->dev, &rate);
> > +	if (IS_ERR(opp)) {
> > +		dev_err(td->dev, "couldn't get vmax opp, empty opp table?\n");
> > +		return -EINVAL;
> > +	}
> > +	v_max = dev_pm_opp_get_voltage(opp);
> > +
> > +	rcu_read_unlock();
> > +
> > +	if (td->pmu_if == TEGRA_DFLL_PMU_PWM)
> > +		return dfll_build_lut_pwm(td, v_max);
> > +	else
> > +		return dfll_build_i2c_lut(td, v_max);
> > +}
> > +
> >  /**
> >   * read_dt_param - helper function for reading required parameters from the DT
> >   * @td: DFLL instance
> > @@ -1566,11 +1836,54 @@ static int dfll_fetch_i2c_params(struct tegra_dfll *td)
> >  	}
> >  	td->i2c_reg = vsel_reg;
> >  
> > -	ret = dfll_build_i2c_lut(td);
> > -	if (ret) {
> > -		dev_err(td->dev, "couldn't build I2C LUT\n");
> > +	return 0;
> > +}
> > +
> > +static int dfll_fetch_pwm_params(struct tegra_dfll *td)
> > +{
> > +	int ret, i;
> > +	u32 pwm_period;
> > +
> > +	if (!td->soc->alignment.step_uv || !td->soc->alignment.offset_uv) {
> > +		dev_err(td->dev, "Missing step or alignment info for PWM regulator");
> > +		return -EINVAL;
> > +	}
> > +	for (i = 0; i < MAX_DFLL_VOLTAGES; i++)
> > +		td->lut_uv[i] = td->soc->alignment.offset_uv +
> > +				i * td->soc->alignment.step_uv;
> > +
> > +	ret = read_dt_param(td, "nvidia,init-uv", &td->reg_init_uV);
> > +	if (!ret) {
> > +		dev_err(td->dev, "couldn't get initialized voltage\n");
> > +		return ret;
> > +	}
> > +
> > +	ret = read_dt_param(td, "nvidia,pwm-period", &pwm_period);
> > +	if (!ret) {
> > +		dev_err(td->dev, "couldn't get PWM period\n");
> >  		return ret;
> >  	}
> > +	td->pwm_rate = (NSEC_PER_SEC / pwm_period) * (MAX_DFLL_VOLTAGES - 1);
> 
> Do we need to check that this pwm_rate is not too big so we don't end up
> with a bad value in dfll_pwm_set_output_enabled()? If this pwm_rate is
> not updated and neither is ref_rate, can we not just store the divisor
> so we can use in dfll_pwm_set_output_enabled()?
> 

ref_rate is fixed. So we could indeed store the divider rather than the rate.

Peter.

> Cheers
> Jon
> 
> -- 
> nvpublic

Re: [PATCH v3 09/11] cpufreq: tegra124-cpufreq: extend to support Tegra210

[Peter De Schrijver]

On Thu, Mar 08, 2018 at 11:25:04PM +0000, Jon Hunter wrote:
> 
> On 06/02/18 16:34, Peter De Schrijver wrote:
> > Tegra210 has a very similar CPU clocking scheme than Tegra124. So add
> > support in this driver. Also allow for the case where the CPU voltage is
> > controlled directly by the DFLL rather than by a separate regulator object.
> > 
> > Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
> > ---
> >  drivers/cpufreq/tegra124-cpufreq.c | 15 ++++++++-------
> >  1 file changed, 8 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/cpufreq/tegra124-cpufreq.c b/drivers/cpufreq/tegra124-cpufreq.c
> > index 4353025..f8e01a8 100644
> > --- a/drivers/cpufreq/tegra124-cpufreq.c
> > +++ b/drivers/cpufreq/tegra124-cpufreq.c
> > @@ -64,7 +64,8 @@ static void tegra124_cpu_switch_to_pllx(struct tegra124_cpufreq_priv *priv)
> >  {
> >  	clk_set_parent(priv->cpu_clk, priv->pllp_clk);
> >  	clk_disable_unprepare(priv->dfll_clk);
> > -	regulator_sync_voltage(priv->vdd_cpu_reg);
> > +	if (priv->vdd_cpu_reg)
> > +		regulator_sync_voltage(priv->vdd_cpu_reg);
> >  	clk_set_parent(priv->cpu_clk, priv->pllx_clk);
> >  }
> >  
> > @@ -89,10 +90,10 @@ static int tegra124_cpufreq_probe(struct platform_device *pdev)
> >  		return -ENODEV;
> >  
> >  	priv->vdd_cpu_reg = regulator_get(cpu_dev, "vdd-cpu");
> > -	if (IS_ERR(priv->vdd_cpu_reg)) {
> > -		ret = PTR_ERR(priv->vdd_cpu_reg);
> > -		goto out_put_np;
> > -	}
> > +	if (IS_ERR(priv->vdd_cpu_reg) != -EPROBE_DEFER)
> > +		priv->vdd_cpu_reg = NULL;
> > +	else
> > +		return -EPROBE_DEFER;
> 
> I am still not sure that we should rely on the fact that the regulator
> is not present in DT to imply that we do not need it. I think that we
> should be checking if we are using I2C mode here.
> 

The cpufreq driver doesn't know this however. Also the current approach of
setting the same voltage when switching to pll_x is incorrect. The CVB
tables when using pll_x include more margin than when using the DFLL.

Peter.

Re: [PATCH v3 09/11] cpufreq: tegra124-cpufreq: extend to support Tegra210

[Viresh Kumar]

On Tue, Feb 6, 2018 at 10:04 PM, Peter De Schrijver
<pdeschrijver@nvidia.com> wrote:
> Tegra210 has a very similar CPU clocking scheme than Tegra124. So add
> support in this driver. Also allow for the case where the CPU voltage is
> controlled directly by the DFLL rather than by a separate regulator object.
>
> Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
> ---
>  drivers/cpufreq/tegra124-cpufreq.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)

You forgot to Cc maintainers and pm-list from the cpufreq world :(

Re: [PATCH v3 07/11] dt-bindings: tegra: Update DFLL binding for PWM regulator

[Peter De Schrijver]

On Thu, Mar 08, 2018 at 11:21:40PM +0000, Jon Hunter wrote:
> 
> On 06/02/18 16:34, Peter De Schrijver wrote:
> > Add new properties to configure the DFLL PWM regulator support. Also
> > add an example and make the I2C clock only required when I2C support is
> > used.
> > 
> > Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
> > ---
> >  .../bindings/clock/nvidia,tegra124-dfll.txt        | 76 +++++++++++++++++++++-
> >  1 file changed, 74 insertions(+), 2 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt b/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt
> > index dff236f..a4903f7 100644
> > --- a/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt
> > +++ b/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt
> > @@ -23,7 +23,8 @@ Required properties:
> >  - clock-names: Must include the following entries:
> >    - soc: Clock source for the DFLL control logic.
> >    - ref: The closed loop reference clock
> > -  - i2c: Clock source for the integrated I2C master.
> > +  - i2c: Clock source for the integrated I2C master (only required when
> > +	 using I2C mode).
> >  - resets: Must contain an entry for each entry in reset-names.
> >    See ../reset/reset.txt for details.
> >  - reset-names: Must include the following entries:
> > @@ -45,10 +46,28 @@ Required properties for the control loop parameters:
> >  Optional properties for the control loop parameters:
> >  - nvidia,cg-scale: Boolean value, see the field DFLL_PARAMS_CG_SCALE in the TRM.
> >  
> > +Optional properties for mode selection:
> > +- nvidia,pwm-to-pmic: Use PWM to control regulator rather then I2C.
> > +
> 
> Do we need this property? Seems that we should be able to detect if it
> is I2C or PWM based upon the other properties.
> 

I guess we could look for nvidia,pwm-period and switch to PWM mode if this is
found, but I think it's more clear to have explicit property.

Peter.

Re: [PATCH v3 05/11] clk: tegra: prepare dfll driver for PWM regulator

[Peter De Schrijver]

On Thu, Mar 08, 2018 at 10:50:06PM +0000, Jon Hunter wrote:
> 
> On 06/02/18 16:34, Peter De Schrijver wrote:
> > This patch prepares the dfll driver to work with PWM regulators.
> > To do this we introduce a new array lut_uv which gives the voltage for
> > a given index generated by the dfll logic. This index will then be
> > translated to a PMIC voltage ID in case of I2C using the i2c_lut. In case
> > of a PWM regulator, it will be used to determine the PWM duty cycle.
> > We also introduce lut_bottom which holds the lowest voltage we will ever
> > need. In case of I2C this can be set to zero because the i2c_lut will be
> > initialized such that entry 0 will be the lowest voltage we will ever
> > need. In case of PWM, the lowest voltage is determined by the regulator
> > hardware so we need this software limit. Note that this is different
> > from lut_min which gives the lowest voltage we allow taking temperature
> > into account. In a future patchset we will update lut_vmin dynamically.
> > Similarly lut_max will be the highest voltage allowed taking temperature
> > into accouint. Also this will be updated dynamically once temperature
> > dependence will be introduced.
> > 
> > Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
> > ---
> >  drivers/clk/tegra/clk-dfll.c | 62 ++++++++++++++++++++++++++------------------
> >  1 file changed, 37 insertions(+), 25 deletions(-)
> > 
> > diff --git a/drivers/clk/tegra/clk-dfll.c b/drivers/clk/tegra/clk-dfll.c
> > index 0a7deee..fa97763 100644
> > --- a/drivers/clk/tegra/clk-dfll.c
> > +++ b/drivers/clk/tegra/clk-dfll.c
> > @@ -301,9 +301,10 @@ struct tegra_dfll {
> >  	u32				i2c_slave_addr;
> >  
> >  	/* i2c_lut array entries are regulator framework selectors */
> > -	unsigned			i2c_lut[MAX_DFLL_VOLTAGES];
> > -	int				i2c_lut_size;
> > -	u8				lut_min, lut_max, lut_safe;
> > +	unsigned int			i2c_lut[MAX_DFLL_VOLTAGES];
> > +	unsigned int			lut_uv[MAX_DFLL_VOLTAGES];
> > +	int				lut_size;
> > +	u8				lut_bottom, lut_min, lut_max, lut_safe;
> >  };
> >  
> >  #define clk_hw_to_dfll(_hw) container_of(_hw, struct tegra_dfll, dfll_clk_hw)
> > @@ -531,10 +532,10 @@ static void dfll_load_i2c_lut(struct tegra_dfll *td)
> >  	u32 val;
> >  
> >  	for (i = 0; i < MAX_DFLL_VOLTAGES; i++) {
> > -		if (i < td->lut_min)
> > -			lut_index = td->lut_min;
> > -		else if (i > td->lut_max)
> > -			lut_index = td->lut_max;
> > +		if (i < td->lut_bottom)
> > +			lut_index = td->lut_bottom;
> > +		else if (i > td->lut_size - 1)
> > +			lut_index = td->lut_size - 1;
> >  		else
> >  			lut_index = i;
> >  
> > @@ -594,9 +595,9 @@ static void dfll_init_out_if(struct tegra_dfll *td)
> >  {
> >  	u32 val;
> >  
> > -	td->lut_min = 0;
> > -	td->lut_max = td->i2c_lut_size - 1;
> > -	td->lut_safe = td->lut_min + 1;
> > +	td->lut_min = td->lut_bottom;
> > +	td->lut_max = td->lut_size - 1;
> > +	td->lut_safe = td->lut_min + (td->lut_min < td->lut_max ? 1 : 0);
> >  
> >  	dfll_i2c_writel(td, 0, DFLL_OUTPUT_CFG);
> >  	val = (td->lut_safe << DFLL_OUTPUT_CFG_SAFE_SHIFT) |
> > @@ -619,11 +620,11 @@ static void dfll_init_out_if(struct tegra_dfll *td)
> >   */
> >  
> >  /**
> > - * find_lut_index_for_rate - determine I2C LUT index for given DFLL rate
> > + * find_lut_index_for_rate - determine LUT index for given DFLL rate
> >   * @td: DFLL instance
> >   * @rate: clock rate
> >   *
> > - * Determines the index of a I2C LUT entry for a voltage that approximately
> > + * Determines the index of a LUT entry for a voltage that approximately
> >   * produces the given DFLL clock rate. This is used when forcing a value
> >   * to the integrator during rate changes. Returns -ENOENT if a suitable
> >   * LUT index is not found.
> > @@ -637,11 +638,11 @@ static int find_lut_index_for_rate(struct tegra_dfll *td, unsigned long rate)
> >  	if (IS_ERR(opp))
> >  		return PTR_ERR(opp);
> >  
> > -	uv = dev_pm_opp_get_voltage(opp);
> > +	uv = dev_pm_opp_get_voltage(opp) / td->soc->alignment.step_uv;
> >  	dev_pm_opp_put(opp);
> >  
> > -	for (i = 0; i < td->i2c_lut_size; i++) {
> > -		if (regulator_list_voltage(td->vdd_reg, td->i2c_lut[i]) == uv)
> > +	for (i = td->lut_bottom; i < td->lut_size; i++) {
> > +		if ((td->lut_uv[i] / td->soc->alignment.step_uv) >= uv)
> >  			return i;
> >  	}
> >  
> > @@ -1377,15 +1378,17 @@ static int dfll_init(struct tegra_dfll *td)
> >   */
> >  static int find_vdd_map_entry_exact(struct tegra_dfll *td, int uV)
> >  {
> > -	int i, n_voltages, reg_uV;
> > +	int i, n_voltages, reg_mult, align_mult;
> >  
> > +	align_mult = uV / td->soc->alignment.step_uv;
> >  	n_voltages = regulator_count_voltages(td->vdd_reg);
> >  	for (i = 0; i < n_voltages; i++) {
> > -		reg_uV = regulator_list_voltage(td->vdd_reg, i);
> > -		if (reg_uV < 0)
> > +		reg_mult = regulator_list_voltage(td->vdd_reg, i) /
> > +				td->soc->alignment.step_uv;
> > +		if (reg_mult < 0)
> >  			break;
> >  
> > -		if (uV == reg_uV)
> > +		if (align_mult == reg_mult)
> >  			return i;
> >  	}
> >  
> > @@ -1399,15 +1402,17 @@ static int find_vdd_map_entry_exact(struct tegra_dfll *td, int uV)
> >   * */
> >  static int find_vdd_map_entry_min(struct tegra_dfll *td, int uV)
> >  {
> > -	int i, n_voltages, reg_uV;
> > +	int i, n_voltages, reg_mult, align_mult;
> >  
> > +	align_mult = uV / td->soc->alignment.step_uv;
> >  	n_voltages = regulator_count_voltages(td->vdd_reg);
> >  	for (i = 0; i < n_voltages; i++) {
> > -		reg_uV = regulator_list_voltage(td->vdd_reg, i);
> > -		if (reg_uV < 0)
> > +		reg_mult = regulator_list_voltage(td->vdd_reg, i) /
> > +				td->soc->alignment.step_uv;
> > +		if (reg_mult < 0)
> >  			break;
> >  
> > -		if (uV <= reg_uV)
> > +		if (align_mult <= reg_mult)
> >  			return i;
> >  	}
> >  
> > @@ -1450,8 +1455,10 @@ static int dfll_build_i2c_lut(struct tegra_dfll *td)
> >  	if (lut < 0)
> >  		goto out;
> >  	td->i2c_lut[0] = lut;
> > +	td->lut_bottom = 0;
> >  
> >  	for (j = 1, rate = 0; ; rate++) {
> > +
> >  		opp = dev_pm_opp_find_freq_ceil(td->soc->dev, &rate);
> >  		if (IS_ERR(opp))
> >  			break;
> > @@ -1484,13 +1491,18 @@ static int dfll_build_i2c_lut(struct tegra_dfll *td)
> >  		if (v >= v_max)
> >  			break;
> >  	}
> > -	td->i2c_lut_size = j;
> > +	td->lut_size = j;
> >  
> >  	if (!td->dvco_rate_min)
> >  		dev_err(td->dev, "no opp above DFLL minimum voltage %d mV\n",
> >  			td->soc->cvb->min_millivolts);
> > -	else
> > +	else {
> >  		ret = 0;
> > +		for (j = 0; j < td->lut_size; j++)
> > +			td->lut_uv[j] =
> > +				regulator_list_voltage(td->vdd_reg,
> > +						       td->i2c_lut[j]);
> > +	}
> >  
> >  out:
> >  	return ret;
> > 
> 
> I am a bit confused by this patch as I don't fully understand from the
> description what is being changed. For example, there are a few places
> where you are dividing by td->soc->alignment.step_uv, which seems to be
> changing the calculations/behaviour for I2C unless I am missing something?
> 

The goal is to use td->lut_uv for all voltage related lookups so we can unify
the code for i2c and PWM regulators.

Peter.

> I would also consider renaming lut_bottom and lut_min as something like
> lut_abs_min and lut_cur_min, to indicate that one is the absolute min
> and the other is the current min depending on operating conditions.
> 
> Cheers
> Jon
> 
> -- 
> nvpublic

Re: [PATCH v3 09/11] cpufreq: tegra124-cpufreq: extend to support Tegra210

[Jon Hunter]

On 09/03/18 08:14, Peter De Schrijver wrote:
> On Thu, Mar 08, 2018 at 11:25:04PM +0000, Jon Hunter wrote:
>>
>> On 06/02/18 16:34, Peter De Schrijver wrote:
>>> Tegra210 has a very similar CPU clocking scheme than Tegra124. So add
>>> support in this driver. Also allow for the case where the CPU voltage is
>>> controlled directly by the DFLL rather than by a separate regulator object.
>>>
>>> Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
>>> ---
>>>  drivers/cpufreq/tegra124-cpufreq.c | 15 ++++++++-------
>>>  1 file changed, 8 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/cpufreq/tegra124-cpufreq.c b/drivers/cpufreq/tegra124-cpufreq.c
>>> index 4353025..f8e01a8 100644
>>> --- a/drivers/cpufreq/tegra124-cpufreq.c
>>> +++ b/drivers/cpufreq/tegra124-cpufreq.c
>>> @@ -64,7 +64,8 @@ static void tegra124_cpu_switch_to_pllx(struct tegra124_cpufreq_priv *priv)
>>>  {
>>>  	clk_set_parent(priv->cpu_clk, priv->pllp_clk);
>>>  	clk_disable_unprepare(priv->dfll_clk);
>>> -	regulator_sync_voltage(priv->vdd_cpu_reg);
>>> +	if (priv->vdd_cpu_reg)
>>> +		regulator_sync_voltage(priv->vdd_cpu_reg);
>>>  	clk_set_parent(priv->cpu_clk, priv->pllx_clk);
>>>  }
>>>  
>>> @@ -89,10 +90,10 @@ static int tegra124_cpufreq_probe(struct platform_device *pdev)
>>>  		return -ENODEV;
>>>  
>>>  	priv->vdd_cpu_reg = regulator_get(cpu_dev, "vdd-cpu");
>>> -	if (IS_ERR(priv->vdd_cpu_reg)) {
>>> -		ret = PTR_ERR(priv->vdd_cpu_reg);
>>> -		goto out_put_np;
>>> -	}
>>> +	if (IS_ERR(priv->vdd_cpu_reg) != -EPROBE_DEFER)
>>> +		priv->vdd_cpu_reg = NULL;
>>> +	else
>>> +		return -EPROBE_DEFER;
>>
>> I am still not sure that we should rely on the fact that the regulator
>> is not present in DT to imply that we do not need it. I think that we
>> should be checking if we are using I2C mode here.
>>
> 
> The cpufreq driver doesn't know this however. Also the current approach of
> setting the same voltage when switching to pll_x is incorrect. The CVB
> tables when using pll_x include more margin than when using the DFLL.

Ah yes I see now. However, we are going to need to update the DT doc,
because 'vdd-cpu-supply' is listed as required.

Cheers
Jon

-- 
nvpublic

Re: [PATCH v3 05/11] clk: tegra: prepare dfll driver for PWM regulator

[Jon Hunter]

On 12/03/18 09:14, Peter De Schrijver wrote:
> On Thu, Mar 08, 2018 at 10:50:06PM +0000, Jon Hunter wrote:
>>
>> On 06/02/18 16:34, Peter De Schrijver wrote:
>>> This patch prepares the dfll driver to work with PWM regulators.
>>> To do this we introduce a new array lut_uv which gives the voltage for
>>> a given index generated by the dfll logic. This index will then be
>>> translated to a PMIC voltage ID in case of I2C using the i2c_lut. In case
>>> of a PWM regulator, it will be used to determine the PWM duty cycle.
>>> We also introduce lut_bottom which holds the lowest voltage we will ever
>>> need. In case of I2C this can be set to zero because the i2c_lut will be
>>> initialized such that entry 0 will be the lowest voltage we will ever
>>> need. In case of PWM, the lowest voltage is determined by the regulator
>>> hardware so we need this software limit. Note that this is different
>>> from lut_min which gives the lowest voltage we allow taking temperature
>>> into account. In a future patchset we will update lut_vmin dynamically.
>>> Similarly lut_max will be the highest voltage allowed taking temperature
>>> into accouint. Also this will be updated dynamically once temperature
>>> dependence will be introduced.
>>>
>>> Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
>>> ---
>>>  drivers/clk/tegra/clk-dfll.c | 62 ++++++++++++++++++++++++++------------------
>>>  1 file changed, 37 insertions(+), 25 deletions(-)
>>>
>>> diff --git a/drivers/clk/tegra/clk-dfll.c b/drivers/clk/tegra/clk-dfll.c
>>> index 0a7deee..fa97763 100644
>>> --- a/drivers/clk/tegra/clk-dfll.c
>>> +++ b/drivers/clk/tegra/clk-dfll.c
>>> @@ -301,9 +301,10 @@ struct tegra_dfll {
>>>  	u32				i2c_slave_addr;
>>>  
>>>  	/* i2c_lut array entries are regulator framework selectors */
>>> -	unsigned			i2c_lut[MAX_DFLL_VOLTAGES];
>>> -	int				i2c_lut_size;
>>> -	u8				lut_min, lut_max, lut_safe;
>>> +	unsigned int			i2c_lut[MAX_DFLL_VOLTAGES];
>>> +	unsigned int			lut_uv[MAX_DFLL_VOLTAGES];
>>> +	int				lut_size;
>>> +	u8				lut_bottom, lut_min, lut_max, lut_safe;
>>>  };
>>>  
>>>  #define clk_hw_to_dfll(_hw) container_of(_hw, struct tegra_dfll, dfll_clk_hw)
>>> @@ -531,10 +532,10 @@ static void dfll_load_i2c_lut(struct tegra_dfll *td)
>>>  	u32 val;
>>>  
>>>  	for (i = 0; i < MAX_DFLL_VOLTAGES; i++) {
>>> -		if (i < td->lut_min)
>>> -			lut_index = td->lut_min;
>>> -		else if (i > td->lut_max)
>>> -			lut_index = td->lut_max;
>>> +		if (i < td->lut_bottom)
>>> +			lut_index = td->lut_bottom;
>>> +		else if (i > td->lut_size - 1)
>>> +			lut_index = td->lut_size - 1;
>>>  		else
>>>  			lut_index = i;
>>>  
>>> @@ -594,9 +595,9 @@ static void dfll_init_out_if(struct tegra_dfll *td)
>>>  {
>>>  	u32 val;
>>>  
>>> -	td->lut_min = 0;
>>> -	td->lut_max = td->i2c_lut_size - 1;
>>> -	td->lut_safe = td->lut_min + 1;
>>> +	td->lut_min = td->lut_bottom;
>>> +	td->lut_max = td->lut_size - 1;
>>> +	td->lut_safe = td->lut_min + (td->lut_min < td->lut_max ? 1 : 0);
>>>  
>>>  	dfll_i2c_writel(td, 0, DFLL_OUTPUT_CFG);
>>>  	val = (td->lut_safe << DFLL_OUTPUT_CFG_SAFE_SHIFT) |
>>> @@ -619,11 +620,11 @@ static void dfll_init_out_if(struct tegra_dfll *td)
>>>   */
>>>  
>>>  /**
>>> - * find_lut_index_for_rate - determine I2C LUT index for given DFLL rate
>>> + * find_lut_index_for_rate - determine LUT index for given DFLL rate
>>>   * @td: DFLL instance
>>>   * @rate: clock rate
>>>   *
>>> - * Determines the index of a I2C LUT entry for a voltage that approximately
>>> + * Determines the index of a LUT entry for a voltage that approximately
>>>   * produces the given DFLL clock rate. This is used when forcing a value
>>>   * to the integrator during rate changes. Returns -ENOENT if a suitable
>>>   * LUT index is not found.
>>> @@ -637,11 +638,11 @@ static int find_lut_index_for_rate(struct tegra_dfll *td, unsigned long rate)
>>>  	if (IS_ERR(opp))
>>>  		return PTR_ERR(opp);
>>>  
>>> -	uv = dev_pm_opp_get_voltage(opp);
>>> +	uv = dev_pm_opp_get_voltage(opp) / td->soc->alignment.step_uv;
>>>  	dev_pm_opp_put(opp);
>>>  
>>> -	for (i = 0; i < td->i2c_lut_size; i++) {
>>> -		if (regulator_list_voltage(td->vdd_reg, td->i2c_lut[i]) == uv)
>>> +	for (i = td->lut_bottom; i < td->lut_size; i++) {
>>> +		if ((td->lut_uv[i] / td->soc->alignment.step_uv) >= uv)
>>>  			return i;
>>>  	}
>>>  
>>> @@ -1377,15 +1378,17 @@ static int dfll_init(struct tegra_dfll *td)
>>>   */
>>>  static int find_vdd_map_entry_exact(struct tegra_dfll *td, int uV)
>>>  {
>>> -	int i, n_voltages, reg_uV;
>>> +	int i, n_voltages, reg_mult, align_mult;
>>>  
>>> +	align_mult = uV / td->soc->alignment.step_uv;
>>>  	n_voltages = regulator_count_voltages(td->vdd_reg);
>>>  	for (i = 0; i < n_voltages; i++) {
>>> -		reg_uV = regulator_list_voltage(td->vdd_reg, i);
>>> -		if (reg_uV < 0)
>>> +		reg_mult = regulator_list_voltage(td->vdd_reg, i) /
>>> +				td->soc->alignment.step_uv;
>>> +		if (reg_mult < 0)
>>>  			break;
>>>  
>>> -		if (uV == reg_uV)
>>> +		if (align_mult == reg_mult)
>>>  			return i;
>>>  	}
>>>  
>>> @@ -1399,15 +1402,17 @@ static int find_vdd_map_entry_exact(struct tegra_dfll *td, int uV)
>>>   * */
>>>  static int find_vdd_map_entry_min(struct tegra_dfll *td, int uV)
>>>  {
>>> -	int i, n_voltages, reg_uV;
>>> +	int i, n_voltages, reg_mult, align_mult;
>>>  
>>> +	align_mult = uV / td->soc->alignment.step_uv;
>>>  	n_voltages = regulator_count_voltages(td->vdd_reg);
>>>  	for (i = 0; i < n_voltages; i++) {
>>> -		reg_uV = regulator_list_voltage(td->vdd_reg, i);
>>> -		if (reg_uV < 0)
>>> +		reg_mult = regulator_list_voltage(td->vdd_reg, i) /
>>> +				td->soc->alignment.step_uv;
>>> +		if (reg_mult < 0)
>>>  			break;
>>>  
>>> -		if (uV <= reg_uV)
>>> +		if (align_mult <= reg_mult)
>>>  			return i;
>>>  	}
>>>  
>>> @@ -1450,8 +1455,10 @@ static int dfll_build_i2c_lut(struct tegra_dfll *td)
>>>  	if (lut < 0)
>>>  		goto out;
>>>  	td->i2c_lut[0] = lut;
>>> +	td->lut_bottom = 0;
>>>  
>>>  	for (j = 1, rate = 0; ; rate++) {
>>> +
>>>  		opp = dev_pm_opp_find_freq_ceil(td->soc->dev, &rate);
>>>  		if (IS_ERR(opp))
>>>  			break;
>>> @@ -1484,13 +1491,18 @@ static int dfll_build_i2c_lut(struct tegra_dfll *td)
>>>  		if (v >= v_max)
>>>  			break;
>>>  	}
>>> -	td->i2c_lut_size = j;
>>> +	td->lut_size = j;
>>>  
>>>  	if (!td->dvco_rate_min)
>>>  		dev_err(td->dev, "no opp above DFLL minimum voltage %d mV\n",
>>>  			td->soc->cvb->min_millivolts);
>>> -	else
>>> +	else {
>>>  		ret = 0;
>>> +		for (j = 0; j < td->lut_size; j++)
>>> +			td->lut_uv[j] =
>>> +				regulator_list_voltage(td->vdd_reg,
>>> +						       td->i2c_lut[j]);
>>> +	}
>>>  
>>>  out:
>>>  	return ret;
>>>
>>
>> I am a bit confused by this patch as I don't fully understand from the
>> description what is being changed. For example, there are a few places
>> where you are dividing by td->soc->alignment.step_uv, which seems to be
>> changing the calculations/behaviour for I2C unless I am missing something?
>>
> 
> The goal is to use td->lut_uv for all voltage related lookups so we can unify
> the code for i2c and PWM regulators.

Yes but looking at the patch there is more going on here than just that.
Any changes to the exisiting i2c code should be a separate change unless
I am completely misunderstanding you.

Cheers
Jon

-- 
nvpublic

Re: [PATCH v3 09/11] cpufreq: tegra124-cpufreq: extend to support Tegra210

[Jon Hunter]

On 06/02/18 16:34, Peter De Schrijver wrote:
> Tegra210 has a very similar CPU clocking scheme than Tegra124. So add
> support in this driver. Also allow for the case where the CPU voltage is
> controlled directly by the DFLL rather than by a separate regulator object.
> 
> Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
> ---
>  drivers/cpufreq/tegra124-cpufreq.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/cpufreq/tegra124-cpufreq.c b/drivers/cpufreq/tegra124-cpufreq.c
> index 4353025..f8e01a8 100644
> --- a/drivers/cpufreq/tegra124-cpufreq.c
> +++ b/drivers/cpufreq/tegra124-cpufreq.c
> @@ -64,7 +64,8 @@ static void tegra124_cpu_switch_to_pllx(struct tegra124_cpufreq_priv *priv)
>  {
>  	clk_set_parent(priv->cpu_clk, priv->pllp_clk);
>  	clk_disable_unprepare(priv->dfll_clk);
> -	regulator_sync_voltage(priv->vdd_cpu_reg);
> +	if (priv->vdd_cpu_reg)
> +		regulator_sync_voltage(priv->vdd_cpu_reg);
>  	clk_set_parent(priv->cpu_clk, priv->pllx_clk);
>  }

OK, so this bit does not make sense to me. In the above we are switching
from the DFLL to the PLL (ie. disabling the DFLL) and so to ensure we
are operating at the correct voltage after disabling the DFLL we need to
sync the voltage. Seems we would need to do this for all devices, no?
How is the different between Tegra124 and Tegra210?

Cheers
Jon

-- 
nvpublic

Re: [PATCH v3 05/11] clk: tegra: prepare dfll driver for PWM regulator

[Peter De Schrijver]

On Mon, Mar 12, 2018 at 11:08:51AM +0000, Jon Hunter wrote:
> 
> On 12/03/18 09:14, Peter De Schrijver wrote:
> > On Thu, Mar 08, 2018 at 10:50:06PM +0000, Jon Hunter wrote:
> >>
> >> On 06/02/18 16:34, Peter De Schrijver wrote:
> >>> This patch prepares the dfll driver to work with PWM regulators.
> >>> To do this we introduce a new array lut_uv which gives the voltage for
> >>> a given index generated by the dfll logic. This index will then be
> >>> translated to a PMIC voltage ID in case of I2C using the i2c_lut. In case
> >>> of a PWM regulator, it will be used to determine the PWM duty cycle.
> >>> We also introduce lut_bottom which holds the lowest voltage we will ever
> >>> need. In case of I2C this can be set to zero because the i2c_lut will be
> >>> initialized such that entry 0 will be the lowest voltage we will ever
> >>> need. In case of PWM, the lowest voltage is determined by the regulator
> >>> hardware so we need this software limit. Note that this is different
> >>> from lut_min which gives the lowest voltage we allow taking temperature
> >>> into account. In a future patchset we will update lut_vmin dynamically.
> >>> Similarly lut_max will be the highest voltage allowed taking temperature
> >>> into accouint. Also this will be updated dynamically once temperature
> >>> dependence will be introduced.
> >>>
> >>> Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
> >>> ---
> >>>  drivers/clk/tegra/clk-dfll.c | 62 ++++++++++++++++++++++++++------------------
> >>>  1 file changed, 37 insertions(+), 25 deletions(-)
> >>>
> >>> diff --git a/drivers/clk/tegra/clk-dfll.c b/drivers/clk/tegra/clk-dfll.c
> >>> index 0a7deee..fa97763 100644
> >>> --- a/drivers/clk/tegra/clk-dfll.c
> >>> +++ b/drivers/clk/tegra/clk-dfll.c
> >>> @@ -301,9 +301,10 @@ struct tegra_dfll {
> >>>  	u32				i2c_slave_addr;
> >>>  
> >>>  	/* i2c_lut array entries are regulator framework selectors */
> >>> -	unsigned			i2c_lut[MAX_DFLL_VOLTAGES];
> >>> -	int				i2c_lut_size;
> >>> -	u8				lut_min, lut_max, lut_safe;
> >>> +	unsigned int			i2c_lut[MAX_DFLL_VOLTAGES];
> >>> +	unsigned int			lut_uv[MAX_DFLL_VOLTAGES];
> >>> +	int				lut_size;
> >>> +	u8				lut_bottom, lut_min, lut_max, lut_safe;
> >>>  };
> >>>  
> >>>  #define clk_hw_to_dfll(_hw) container_of(_hw, struct tegra_dfll, dfll_clk_hw)
> >>> @@ -531,10 +532,10 @@ static void dfll_load_i2c_lut(struct tegra_dfll *td)
> >>>  	u32 val;
> >>>  
> >>>  	for (i = 0; i < MAX_DFLL_VOLTAGES; i++) {
> >>> -		if (i < td->lut_min)
> >>> -			lut_index = td->lut_min;
> >>> -		else if (i > td->lut_max)
> >>> -			lut_index = td->lut_max;
> >>> +		if (i < td->lut_bottom)
> >>> +			lut_index = td->lut_bottom;
> >>> +		else if (i > td->lut_size - 1)
> >>> +			lut_index = td->lut_size - 1;
> >>>  		else
> >>>  			lut_index = i;
> >>>  
> >>> @@ -594,9 +595,9 @@ static void dfll_init_out_if(struct tegra_dfll *td)
> >>>  {
> >>>  	u32 val;
> >>>  
> >>> -	td->lut_min = 0;
> >>> -	td->lut_max = td->i2c_lut_size - 1;
> >>> -	td->lut_safe = td->lut_min + 1;
> >>> +	td->lut_min = td->lut_bottom;
> >>> +	td->lut_max = td->lut_size - 1;
> >>> +	td->lut_safe = td->lut_min + (td->lut_min < td->lut_max ? 1 : 0);
> >>>  
> >>>  	dfll_i2c_writel(td, 0, DFLL_OUTPUT_CFG);
> >>>  	val = (td->lut_safe << DFLL_OUTPUT_CFG_SAFE_SHIFT) |
> >>> @@ -619,11 +620,11 @@ static void dfll_init_out_if(struct tegra_dfll *td)
> >>>   */
> >>>  
> >>>  /**
> >>> - * find_lut_index_for_rate - determine I2C LUT index for given DFLL rate
> >>> + * find_lut_index_for_rate - determine LUT index for given DFLL rate
> >>>   * @td: DFLL instance
> >>>   * @rate: clock rate
> >>>   *
> >>> - * Determines the index of a I2C LUT entry for a voltage that approximately
> >>> + * Determines the index of a LUT entry for a voltage that approximately
> >>>   * produces the given DFLL clock rate. This is used when forcing a value
> >>>   * to the integrator during rate changes. Returns -ENOENT if a suitable
> >>>   * LUT index is not found.
> >>> @@ -637,11 +638,11 @@ static int find_lut_index_for_rate(struct tegra_dfll *td, unsigned long rate)
> >>>  	if (IS_ERR(opp))
> >>>  		return PTR_ERR(opp);
> >>>  
> >>> -	uv = dev_pm_opp_get_voltage(opp);
> >>> +	uv = dev_pm_opp_get_voltage(opp) / td->soc->alignment.step_uv;
> >>>  	dev_pm_opp_put(opp);
> >>>  
> >>> -	for (i = 0; i < td->i2c_lut_size; i++) {
> >>> -		if (regulator_list_voltage(td->vdd_reg, td->i2c_lut[i]) == uv)
> >>> +	for (i = td->lut_bottom; i < td->lut_size; i++) {
> >>> +		if ((td->lut_uv[i] / td->soc->alignment.step_uv) >= uv)
> >>>  			return i;
> >>>  	}
> >>>  
> >>> @@ -1377,15 +1378,17 @@ static int dfll_init(struct tegra_dfll *td)
> >>>   */
> >>>  static int find_vdd_map_entry_exact(struct tegra_dfll *td, int uV)
> >>>  {
> >>> -	int i, n_voltages, reg_uV;
> >>> +	int i, n_voltages, reg_mult, align_mult;
> >>>  
> >>> +	align_mult = uV / td->soc->alignment.step_uv;
> >>>  	n_voltages = regulator_count_voltages(td->vdd_reg);
> >>>  	for (i = 0; i < n_voltages; i++) {
> >>> -		reg_uV = regulator_list_voltage(td->vdd_reg, i);
> >>> -		if (reg_uV < 0)
> >>> +		reg_mult = regulator_list_voltage(td->vdd_reg, i) /
> >>> +				td->soc->alignment.step_uv;
> >>> +		if (reg_mult < 0)
> >>>  			break;
> >>>  
> >>> -		if (uV == reg_uV)
> >>> +		if (align_mult == reg_mult)
> >>>  			return i;
> >>>  	}
> >>>  
> >>> @@ -1399,15 +1402,17 @@ static int find_vdd_map_entry_exact(struct tegra_dfll *td, int uV)
> >>>   * */
> >>>  static int find_vdd_map_entry_min(struct tegra_dfll *td, int uV)
> >>>  {
> >>> -	int i, n_voltages, reg_uV;
> >>> +	int i, n_voltages, reg_mult, align_mult;
> >>>  
> >>> +	align_mult = uV / td->soc->alignment.step_uv;
> >>>  	n_voltages = regulator_count_voltages(td->vdd_reg);
> >>>  	for (i = 0; i < n_voltages; i++) {
> >>> -		reg_uV = regulator_list_voltage(td->vdd_reg, i);
> >>> -		if (reg_uV < 0)
> >>> +		reg_mult = regulator_list_voltage(td->vdd_reg, i) /
> >>> +				td->soc->alignment.step_uv;
> >>> +		if (reg_mult < 0)
> >>>  			break;
> >>>  
> >>> -		if (uV <= reg_uV)
> >>> +		if (align_mult <= reg_mult)
> >>>  			return i;
> >>>  	}
> >>>  
> >>> @@ -1450,8 +1455,10 @@ static int dfll_build_i2c_lut(struct tegra_dfll *td)
> >>>  	if (lut < 0)
> >>>  		goto out;
> >>>  	td->i2c_lut[0] = lut;
> >>> +	td->lut_bottom = 0;
> >>>  
> >>>  	for (j = 1, rate = 0; ; rate++) {
> >>> +
> >>>  		opp = dev_pm_opp_find_freq_ceil(td->soc->dev, &rate);
> >>>  		if (IS_ERR(opp))
> >>>  			break;
> >>> @@ -1484,13 +1491,18 @@ static int dfll_build_i2c_lut(struct tegra_dfll *td)
> >>>  		if (v >= v_max)
> >>>  			break;
> >>>  	}
> >>> -	td->i2c_lut_size = j;
> >>> +	td->lut_size = j;
> >>>  
> >>>  	if (!td->dvco_rate_min)
> >>>  		dev_err(td->dev, "no opp above DFLL minimum voltage %d mV\n",
> >>>  			td->soc->cvb->min_millivolts);
> >>> -	else
> >>> +	else {
> >>>  		ret = 0;
> >>> +		for (j = 0; j < td->lut_size; j++)
> >>> +			td->lut_uv[j] =
> >>> +				regulator_list_voltage(td->vdd_reg,
> >>> +						       td->i2c_lut[j]);
> >>> +	}
> >>>  
> >>>  out:
> >>>  	return ret;
> >>>
> >>
> >> I am a bit confused by this patch as I don't fully understand from the
> >> description what is being changed. For example, there are a few places
> >> where you are dividing by td->soc->alignment.step_uv, which seems to be
> >> changing the calculations/behaviour for I2C unless I am missing something?
> >>
> > 
> > The goal is to use td->lut_uv for all voltage related lookups so we can unify
> > the code for i2c and PWM regulators.
> 
> Yes but looking at the patch there is more going on here than just that.
> Any changes to the exisiting i2c code should be a separate change unless
> I am completely misunderstanding you.
> 

This requires changes to the existing i2c code, which is what this patch does.

Peter.

Re: [PATCH v3 09/11] cpufreq: tegra124-cpufreq: extend to support Tegra210

[Peter De Schrijver]

On Mon, Mar 12, 2018 at 10:14:17AM +0000, Jon Hunter wrote:
> 
> On 09/03/18 08:14, Peter De Schrijver wrote:
> > On Thu, Mar 08, 2018 at 11:25:04PM +0000, Jon Hunter wrote:
> >>
> >> On 06/02/18 16:34, Peter De Schrijver wrote:
> >>> Tegra210 has a very similar CPU clocking scheme than Tegra124. So add
> >>> support in this driver. Also allow for the case where the CPU voltage is
> >>> controlled directly by the DFLL rather than by a separate regulator object.
> >>>
> >>> Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
> >>> ---
> >>>  drivers/cpufreq/tegra124-cpufreq.c | 15 ++++++++-------
> >>>  1 file changed, 8 insertions(+), 7 deletions(-)
> >>>
> >>> diff --git a/drivers/cpufreq/tegra124-cpufreq.c b/drivers/cpufreq/tegra124-cpufreq.c
> >>> index 4353025..f8e01a8 100644
> >>> --- a/drivers/cpufreq/tegra124-cpufreq.c
> >>> +++ b/drivers/cpufreq/tegra124-cpufreq.c
> >>> @@ -64,7 +64,8 @@ static void tegra124_cpu_switch_to_pllx(struct tegra124_cpufreq_priv *priv)
> >>>  {
> >>>  	clk_set_parent(priv->cpu_clk, priv->pllp_clk);
> >>>  	clk_disable_unprepare(priv->dfll_clk);
> >>> -	regulator_sync_voltage(priv->vdd_cpu_reg);
> >>> +	if (priv->vdd_cpu_reg)
> >>> +		regulator_sync_voltage(priv->vdd_cpu_reg);
> >>>  	clk_set_parent(priv->cpu_clk, priv->pllx_clk);
> >>>  }
> >>>  
> >>> @@ -89,10 +90,10 @@ static int tegra124_cpufreq_probe(struct platform_device *pdev)
> >>>  		return -ENODEV;
> >>>  
> >>>  	priv->vdd_cpu_reg = regulator_get(cpu_dev, "vdd-cpu");
> >>> -	if (IS_ERR(priv->vdd_cpu_reg)) {
> >>> -		ret = PTR_ERR(priv->vdd_cpu_reg);
> >>> -		goto out_put_np;
> >>> -	}
> >>> +	if (IS_ERR(priv->vdd_cpu_reg) != -EPROBE_DEFER)
> >>> +		priv->vdd_cpu_reg = NULL;
> >>> +	else
> >>> +		return -EPROBE_DEFER;
> >>
> >> I am still not sure that we should rely on the fact that the regulator
> >> is not present in DT to imply that we do not need it. I think that we
> >> should be checking if we are using I2C mode here.
> >>
> > 
> > The cpufreq driver doesn't know this however. Also the current approach of
> > setting the same voltage when switching to pll_x is incorrect. The CVB
> > tables when using pll_x include more margin than when using the DFLL.
> 
> Ah yes I see now. However, we are going to need to update the DT doc,
> because 'vdd-cpu-supply' is listed as required.
> 

Ok.

Peter.

Re: [PATCH v3 09/11] cpufreq: tegra124-cpufreq: extend to support Tegra210

[Peter De Schrijver]

On Mon, Mar 12, 2018 at 12:15:22PM +0000, Jon Hunter wrote:
> 
> On 06/02/18 16:34, Peter De Schrijver wrote:
> > Tegra210 has a very similar CPU clocking scheme than Tegra124. So add
> > support in this driver. Also allow for the case where the CPU voltage is
> > controlled directly by the DFLL rather than by a separate regulator object.
> > 
> > Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
> > ---
> >  drivers/cpufreq/tegra124-cpufreq.c | 15 ++++++++-------
> >  1 file changed, 8 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/cpufreq/tegra124-cpufreq.c b/drivers/cpufreq/tegra124-cpufreq.c
> > index 4353025..f8e01a8 100644
> > --- a/drivers/cpufreq/tegra124-cpufreq.c
> > +++ b/drivers/cpufreq/tegra124-cpufreq.c
> > @@ -64,7 +64,8 @@ static void tegra124_cpu_switch_to_pllx(struct tegra124_cpufreq_priv *priv)
> >  {
> >  	clk_set_parent(priv->cpu_clk, priv->pllp_clk);
> >  	clk_disable_unprepare(priv->dfll_clk);
> > -	regulator_sync_voltage(priv->vdd_cpu_reg);
> > +	if (priv->vdd_cpu_reg)
> > +		regulator_sync_voltage(priv->vdd_cpu_reg);
> >  	clk_set_parent(priv->cpu_clk, priv->pllx_clk);
> >  }
> 
> OK, so this bit does not make sense to me. In the above we are switching
> from the DFLL to the PLL (ie. disabling the DFLL) and so to ensure we
> are operating at the correct voltage after disabling the DFLL we need to
> sync the voltage. Seems we would need to do this for all devices, no?
> How is the different between Tegra124 and Tegra210?

Yes. So in case of i2c the regulator framework will reapply the voltage it
knows which in our case is the boot voltage for VDD_CPU because noone else
from a regulator framework pov has ever changed the voltage. In case of PWM
putting the PWM output pad in tri state will cause the OVR regulator to output
a hardware defined voltage. This is done as part of the dfll_clk_disable()
function. To summarize:

switch from pll_x to DFLL for i2c regulator:

entry: voltage is boot voltage set bootloader
1) set dfll rate to pll_x rate
2) set parent to pll_p so we run at 408MHz which is below Fmax@Vmin when
   running from PLL
3) enable DFLL this will switch to closed loop mode and start controlling
   the voltage via i2c, however because we are below Fmax@Vmin there's no
   V/f curve violation.
4) change parent from pll_x to DFLL

switch from DFLL to pll_x for i2c regulator:

entry: voltage is whatever the DFLL has programmed but not lower than Vmin
1) set parent to pll_p so we run at 408MHz (below Fmax@Vmin)
2) disable DFLL, this will cause the voltage to be the last value programmed
   by the DFLL.
3) go back to the voltage as programmed by the boot loader using
   regulator_sync_voltage().
4) change parent from DFLL to pll_x. Because pll_x is still programmed at
   the bootloader frequency, we're within the V/f curve.

switch from pll_x to DFLL for PWM regulator:

entry: voltage is boot voltage set by hardware because PWM pin is in tristate
1) set dfll rate to pll_x rate
2) set parent to pll_p so we run at 408MHz which is below Fmax@Vmin when
   running from PLL
3) enable DFLL this will set the PWM pad to output, switch to closed loop mode
   and start controlling the voltage via PWM, however because we are below
   Fmax@Vmin there's no V/f curve violation.
4) change parent from pll_x to DFLL

switch from DFLL to pll_x for PWM regulator:

entry: voltage is whatever the DFLL has programmed but not lower than Vmin
1) set parent to pll_p so we run at 408MHz (below Fmax@Vmin)
2) disable DFLL, this will cause the PWM pad to be programmed to tristate
   and the voltage to change back to the hardware defined voltage.
3) change parent from DFLL to pll_x. Because pll_x is still programmed at
   the bootloader frequency, we're within the V/f curve.

Peter.

Re: [PATCH v3 05/11] clk: tegra: prepare dfll driver for PWM regulator

[Jon Hunter]

On 13/03/18 09:03, Peter De Schrijver wrote:
> On Mon, Mar 12, 2018 at 11:08:51AM +0000, Jon Hunter wrote:
>>
>> On 12/03/18 09:14, Peter De Schrijver wrote:
>>> On Thu, Mar 08, 2018 at 10:50:06PM +0000, Jon Hunter wrote:
>>>>
>>>> On 06/02/18 16:34, Peter De Schrijver wrote:
>>>>> This patch prepares the dfll driver to work with PWM regulators.
>>>>> To do this we introduce a new array lut_uv which gives the voltage for
>>>>> a given index generated by the dfll logic. This index will then be
>>>>> translated to a PMIC voltage ID in case of I2C using the i2c_lut. In case
>>>>> of a PWM regulator, it will be used to determine the PWM duty cycle.
>>>>> We also introduce lut_bottom which holds the lowest voltage we will ever
>>>>> need. In case of I2C this can be set to zero because the i2c_lut will be
>>>>> initialized such that entry 0 will be the lowest voltage we will ever
>>>>> need. In case of PWM, the lowest voltage is determined by the regulator
>>>>> hardware so we need this software limit. Note that this is different
>>>>> from lut_min which gives the lowest voltage we allow taking temperature
>>>>> into account. In a future patchset we will update lut_vmin dynamically.
>>>>> Similarly lut_max will be the highest voltage allowed taking temperature
>>>>> into accouint. Also this will be updated dynamically once temperature
>>>>> dependence will be introduced.
>>>>>
>>>>> Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
>>>>> ---
>>>>>  drivers/clk/tegra/clk-dfll.c | 62 ++++++++++++++++++++++++++------------------
>>>>>  1 file changed, 37 insertions(+), 25 deletions(-)
>>>>>
>>>>> diff --git a/drivers/clk/tegra/clk-dfll.c b/drivers/clk/tegra/clk-dfll.c
>>>>> index 0a7deee..fa97763 100644
>>>>> --- a/drivers/clk/tegra/clk-dfll.c
>>>>> +++ b/drivers/clk/tegra/clk-dfll.c
>>>>> @@ -301,9 +301,10 @@ struct tegra_dfll {
>>>>>  	u32				i2c_slave_addr;
>>>>>  
>>>>>  	/* i2c_lut array entries are regulator framework selectors */
>>>>> -	unsigned			i2c_lut[MAX_DFLL_VOLTAGES];
>>>>> -	int				i2c_lut_size;
>>>>> -	u8				lut_min, lut_max, lut_safe;
>>>>> +	unsigned int			i2c_lut[MAX_DFLL_VOLTAGES];
>>>>> +	unsigned int			lut_uv[MAX_DFLL_VOLTAGES];
>>>>> +	int				lut_size;
>>>>> +	u8				lut_bottom, lut_min, lut_max, lut_safe;
>>>>>  };
>>>>>  
>>>>>  #define clk_hw_to_dfll(_hw) container_of(_hw, struct tegra_dfll, dfll_clk_hw)
>>>>> @@ -531,10 +532,10 @@ static void dfll_load_i2c_lut(struct tegra_dfll *td)
>>>>>  	u32 val;
>>>>>  
>>>>>  	for (i = 0; i < MAX_DFLL_VOLTAGES; i++) {
>>>>> -		if (i < td->lut_min)
>>>>> -			lut_index = td->lut_min;
>>>>> -		else if (i > td->lut_max)
>>>>> -			lut_index = td->lut_max;
>>>>> +		if (i < td->lut_bottom)
>>>>> +			lut_index = td->lut_bottom;
>>>>> +		else if (i > td->lut_size - 1)
>>>>> +			lut_index = td->lut_size - 1;
>>>>>  		else
>>>>>  			lut_index = i;
>>>>>  
>>>>> @@ -594,9 +595,9 @@ static void dfll_init_out_if(struct tegra_dfll *td)
>>>>>  {
>>>>>  	u32 val;
>>>>>  
>>>>> -	td->lut_min = 0;
>>>>> -	td->lut_max = td->i2c_lut_size - 1;
>>>>> -	td->lut_safe = td->lut_min + 1;
>>>>> +	td->lut_min = td->lut_bottom;
>>>>> +	td->lut_max = td->lut_size - 1;
>>>>> +	td->lut_safe = td->lut_min + (td->lut_min < td->lut_max ? 1 : 0);
>>>>>  
>>>>>  	dfll_i2c_writel(td, 0, DFLL_OUTPUT_CFG);
>>>>>  	val = (td->lut_safe << DFLL_OUTPUT_CFG_SAFE_SHIFT) |
>>>>> @@ -619,11 +620,11 @@ static void dfll_init_out_if(struct tegra_dfll *td)
>>>>>   */
>>>>>  
>>>>>  /**
>>>>> - * find_lut_index_for_rate - determine I2C LUT index for given DFLL rate
>>>>> + * find_lut_index_for_rate - determine LUT index for given DFLL rate
>>>>>   * @td: DFLL instance
>>>>>   * @rate: clock rate
>>>>>   *
>>>>> - * Determines the index of a I2C LUT entry for a voltage that approximately
>>>>> + * Determines the index of a LUT entry for a voltage that approximately
>>>>>   * produces the given DFLL clock rate. This is used when forcing a value
>>>>>   * to the integrator during rate changes. Returns -ENOENT if a suitable
>>>>>   * LUT index is not found.
>>>>> @@ -637,11 +638,11 @@ static int find_lut_index_for_rate(struct tegra_dfll *td, unsigned long rate)
>>>>>  	if (IS_ERR(opp))
>>>>>  		return PTR_ERR(opp);
>>>>>  
>>>>> -	uv = dev_pm_opp_get_voltage(opp);
>>>>> +	uv = dev_pm_opp_get_voltage(opp) / td->soc->alignment.step_uv;
>>>>>  	dev_pm_opp_put(opp);
>>>>>  
>>>>> -	for (i = 0; i < td->i2c_lut_size; i++) {
>>>>> -		if (regulator_list_voltage(td->vdd_reg, td->i2c_lut[i]) == uv)
>>>>> +	for (i = td->lut_bottom; i < td->lut_size; i++) {
>>>>> +		if ((td->lut_uv[i] / td->soc->alignment.step_uv) >= uv)
>>>>>  			return i;
>>>>>  	}
>>>>>  
>>>>> @@ -1377,15 +1378,17 @@ static int dfll_init(struct tegra_dfll *td)
>>>>>   */
>>>>>  static int find_vdd_map_entry_exact(struct tegra_dfll *td, int uV)
>>>>>  {
>>>>> -	int i, n_voltages, reg_uV;
>>>>> +	int i, n_voltages, reg_mult, align_mult;
>>>>>  
>>>>> +	align_mult = uV / td->soc->alignment.step_uv;
>>>>>  	n_voltages = regulator_count_voltages(td->vdd_reg);
>>>>>  	for (i = 0; i < n_voltages; i++) {
>>>>> -		reg_uV = regulator_list_voltage(td->vdd_reg, i);
>>>>> -		if (reg_uV < 0)
>>>>> +		reg_mult = regulator_list_voltage(td->vdd_reg, i) /
>>>>> +				td->soc->alignment.step_uv;
>>>>> +		if (reg_mult < 0)
>>>>>  			break;
>>>>>  
>>>>> -		if (uV == reg_uV)
>>>>> +		if (align_mult == reg_mult)
>>>>>  			return i;
>>>>>  	}
>>>>>  
>>>>> @@ -1399,15 +1402,17 @@ static int find_vdd_map_entry_exact(struct tegra_dfll *td, int uV)
>>>>>   * */
>>>>>  static int find_vdd_map_entry_min(struct tegra_dfll *td, int uV)
>>>>>  {
>>>>> -	int i, n_voltages, reg_uV;
>>>>> +	int i, n_voltages, reg_mult, align_mult;
>>>>>  
>>>>> +	align_mult = uV / td->soc->alignment.step_uv;
>>>>>  	n_voltages = regulator_count_voltages(td->vdd_reg);
>>>>>  	for (i = 0; i < n_voltages; i++) {
>>>>> -		reg_uV = regulator_list_voltage(td->vdd_reg, i);
>>>>> -		if (reg_uV < 0)
>>>>> +		reg_mult = regulator_list_voltage(td->vdd_reg, i) /
>>>>> +				td->soc->alignment.step_uv;
>>>>> +		if (reg_mult < 0)
>>>>>  			break;
>>>>>  
>>>>> -		if (uV <= reg_uV)
>>>>> +		if (align_mult <= reg_mult)
>>>>>  			return i;
>>>>>  	}
>>>>>  
>>>>> @@ -1450,8 +1455,10 @@ static int dfll_build_i2c_lut(struct tegra_dfll *td)
>>>>>  	if (lut < 0)
>>>>>  		goto out;
>>>>>  	td->i2c_lut[0] = lut;
>>>>> +	td->lut_bottom = 0;
>>>>>  
>>>>>  	for (j = 1, rate = 0; ; rate++) {
>>>>> +
>>>>>  		opp = dev_pm_opp_find_freq_ceil(td->soc->dev, &rate);
>>>>>  		if (IS_ERR(opp))
>>>>>  			break;
>>>>> @@ -1484,13 +1491,18 @@ static int dfll_build_i2c_lut(struct tegra_dfll *td)
>>>>>  		if (v >= v_max)
>>>>>  			break;
>>>>>  	}
>>>>> -	td->i2c_lut_size = j;
>>>>> +	td->lut_size = j;
>>>>>  
>>>>>  	if (!td->dvco_rate_min)
>>>>>  		dev_err(td->dev, "no opp above DFLL minimum voltage %d mV\n",
>>>>>  			td->soc->cvb->min_millivolts);
>>>>> -	else
>>>>> +	else {
>>>>>  		ret = 0;
>>>>> +		for (j = 0; j < td->lut_size; j++)
>>>>> +			td->lut_uv[j] =
>>>>> +				regulator_list_voltage(td->vdd_reg,
>>>>> +						       td->i2c_lut[j]);
>>>>> +	}
>>>>>  
>>>>>  out:
>>>>>  	return ret;
>>>>>
>>>>
>>>> I am a bit confused by this patch as I don't fully understand from the
>>>> description what is being changed. For example, there are a few places
>>>> where you are dividing by td->soc->alignment.step_uv, which seems to be
>>>> changing the calculations/behaviour for I2C unless I am missing something?
>>>>
>>>
>>> The goal is to use td->lut_uv for all voltage related lookups so we can unify
>>> the code for i2c and PWM regulators.
>>
>> Yes but looking at the patch there is more going on here than just that.
>> Any changes to the exisiting i2c code should be a separate change unless
>> I am completely misunderstanding you.
>>
> 
> This requires changes to the existing i2c code, which is what this patch does.

Yes and that maybe necessary, but it is unclear why. Can the i2c related
changes be made in a separate patch to isolate them?

Cheers
Jon

-- 
nvpublic

Re: [PATCH v3 09/11] cpufreq: tegra124-cpufreq: extend to support Tegra210

[Jon Hunter]

On 13/03/18 09:51, Peter De Schrijver wrote:
> On Mon, Mar 12, 2018 at 12:15:22PM +0000, Jon Hunter wrote:
>>
>> On 06/02/18 16:34, Peter De Schrijver wrote:
>>> Tegra210 has a very similar CPU clocking scheme than Tegra124. So add
>>> support in this driver. Also allow for the case where the CPU voltage is
>>> controlled directly by the DFLL rather than by a separate regulator object.
>>>
>>> Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
>>> ---
>>>  drivers/cpufreq/tegra124-cpufreq.c | 15 ++++++++-------
>>>  1 file changed, 8 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/cpufreq/tegra124-cpufreq.c b/drivers/cpufreq/tegra124-cpufreq.c
>>> index 4353025..f8e01a8 100644
>>> --- a/drivers/cpufreq/tegra124-cpufreq.c
>>> +++ b/drivers/cpufreq/tegra124-cpufreq.c
>>> @@ -64,7 +64,8 @@ static void tegra124_cpu_switch_to_pllx(struct tegra124_cpufreq_priv *priv)
>>>  {
>>>  	clk_set_parent(priv->cpu_clk, priv->pllp_clk);
>>>  	clk_disable_unprepare(priv->dfll_clk);
>>> -	regulator_sync_voltage(priv->vdd_cpu_reg);
>>> +	if (priv->vdd_cpu_reg)
>>> +		regulator_sync_voltage(priv->vdd_cpu_reg);
>>>  	clk_set_parent(priv->cpu_clk, priv->pllx_clk);
>>>  }
>>
>> OK, so this bit does not make sense to me. In the above we are switching
>> from the DFLL to the PLL (ie. disabling the DFLL) and so to ensure we
>> are operating at the correct voltage after disabling the DFLL we need to
>> sync the voltage. Seems we would need to do this for all devices, no?
>> How is the different between Tegra124 and Tegra210?
> 
> Yes. So in case of i2c the regulator framework will reapply the voltage it
> knows which in our case is the boot voltage for VDD_CPU because noone else
> from a regulator framework pov has ever changed the voltage. In case of PWM
> putting the PWM output pad in tri state will cause the OVR regulator to output
> a hardware defined voltage. This is done as part of the dfll_clk_disable()
> function. To summarize:

So this is the piece of information I was missing. Maybe add this to the
changelog so it is clear why we do not need to handle the cpu rail in
the case of PWM.

Cheers
Jon

-- 
nvpublic