2016-04-22 10:31:57

by Penny Chiu

[permalink] [raw]
Subject: [PATCH 00/11] arm64: tegra: Add Tegra DFLL for Tegra210 Jetson TX1

This patch series implements the DFLL/CL-DVFS clock source on Tegra210
based on Tegra124 DFLL driver, Tegra210 support on Tegra124 cpufreq
driver, and exposes DFLL HW as a PWM controller and provides DFLL_PWM
driver to generate PWM signals to control an OpenVReg (PWM regulator)
for CPU rail.

This patch series has been tested on the Jetson TX1.

Penny Chiu (11):
clk: tegra: dfll: Fix voltage comparison
clk: tegra: dfll: Move SoC specific data into of_device_id
clk: tegra: Add DFLL DVCO reset control for Tegra210
clk: tegra: Add Tegra210 support in DFLL driver
pwm: tegra-dfll: Add driver for Tegra DFLL PWM controller
clk: tegra: dfll: Add PWM inferface
cpufreq: tegra124: Add Tegra210 support
arm64: tegra: Add PWM regulator for CPU rail on Jetson TX1
arm64: tegra: Add DFLL clock node on Jetson TX1
arm64: tegra: Add clock properties on cpu0 for Tegra210
arm64: config: Enable CPUFreq-DT, Tegra DFLL PWM, and PWM regulator

.../bindings/clock/nvidia,tegra124-dfll.txt | 21 +-
.../bindings/pwm/nvidia,tegra-dfll-pwm.txt | 48 +++
arch/arm64/boot/dts/nvidia/tegra210-p2180.dtsi | 56 +++
arch/arm64/boot/dts/nvidia/tegra210-p2371-2180.dts | 23 ++
arch/arm64/boot/dts/nvidia/tegra210.dtsi | 21 +
arch/arm64/configs/defconfig | 4 +
drivers/clk/tegra/Makefile | 4 +-
drivers/clk/tegra/clk-dfll.c | 422 +++++++++++++++------
drivers/clk/tegra/clk-dfll.h | 1 +
drivers/clk/tegra/clk-tegra124-dfll-fcpu.c | 127 ++++++-
drivers/clk/tegra/clk-tegra210.c | 68 ++++
drivers/clk/tegra/cvb.c | 12 +-
drivers/clk/tegra/cvb.h | 8 +-
drivers/cpufreq/tegra124-cpufreq.c | 3 +-
drivers/pwm/Kconfig | 10 +
drivers/pwm/Makefile | 1 +
drivers/pwm/pwm-tegra-dfll.c | 322 ++++++++++++++++
include/dt-bindings/reset/tegra210-car.h | 12 +
include/soc/tegra/pwm-tegra-dfll.h | 27 ++
19 files changed, 1046 insertions(+), 144 deletions(-)
create mode 100644 Documentation/devicetree/bindings/pwm/nvidia,tegra-dfll-pwm.txt
create mode 100644 drivers/pwm/pwm-tegra-dfll.c
create mode 100644 include/dt-bindings/reset/tegra210-car.h
create mode 100644 include/soc/tegra/pwm-tegra-dfll.h

--
2.8.1


2016-04-22 10:32:11

by Penny Chiu

[permalink] [raw]
Subject: [PATCH 01/11] clk: tegra: dfll: Fix voltage comparison

When generating opp table, the voltage is round down based on the
alignment in cvb table. This alignment should be also applied on all
voltage comparison. By the way, the alignment voltage values should
depend on regulator device specification, so these values should not
be hard code in cvb table. To Move these values into devicetree will
be more independent for different platforms using the same SoC.

Signed-off-by: Penny Chiu <[email protected]>
---
.../bindings/clock/nvidia,tegra124-dfll.txt | 9 ++++++++
drivers/clk/tegra/clk-dfll.c | 27 +++++++++++++---------
drivers/clk/tegra/clk-dfll.h | 1 +
drivers/clk/tegra/clk-tegra124-dfll-fcpu.c | 23 ++++++++++++++----
drivers/clk/tegra/cvb.c | 12 ++++++----
drivers/clk/tegra/cvb.h | 8 ++++---
6 files changed, 58 insertions(+), 22 deletions(-)

diff --git a/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt b/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt
index ee7e5fd..84080a8 100644
--- a/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt
+++ b/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt
@@ -34,6 +34,14 @@ Required properties:
hardware will start controlling. The regulator will be queried for
the I2C register, control values and supported voltages.

+Reguired properties for build voltage table:
+- nvidia,align-step-uv: Step uV of regulator for CPU voltage rail. This
+ value will be applied when building frequency-voltage table to ensure
+ all calculated voltages will be aligned to voltages regulator supplied.
+
+Optional properties for build voltage table:
+- nvidia,align-offset-uv: Offset uV for calculating DFLL output voltage.
+
Required properties for the control loop parameters:
- nvidia,sample-rate: Sample rate of the DFLL control loop.
- nvidia,droop-ctrl: See the register CL_DVFS_DROOP_CTRL in the TRM.
@@ -68,6 +76,7 @@ clock@0,70110000 {
vdd-cpu-supply = <&vdd_cpu>;
status = "okay";

+ nvidia,align-step-uv = <10000>; /* 10mv */
nvidia,sample-rate = <12500>;
nvidia,droop-ctrl = <0x00000f00>;
nvidia,force-mode = <1>;
diff --git a/drivers/clk/tegra/clk-dfll.c b/drivers/clk/tegra/clk-dfll.c
index 19bfa07..a279467 100644
--- a/drivers/clk/tegra/clk-dfll.c
+++ b/drivers/clk/tegra/clk-dfll.c
@@ -630,7 +630,7 @@ static void dfll_init_out_if(struct tegra_dfll *td)
static int find_lut_index_for_rate(struct tegra_dfll *td, unsigned long rate)
{
struct dev_pm_opp *opp;
- int i, uv;
+ int i, align_volt;

rcu_read_lock();

@@ -639,12 +639,13 @@ static int find_lut_index_for_rate(struct tegra_dfll *td, unsigned long rate)
rcu_read_unlock();
return PTR_ERR(opp);
}
- uv = dev_pm_opp_get_voltage(opp);
+ align_volt = dev_pm_opp_get_voltage(opp) / td->soc->alignment;

rcu_read_unlock();

for (i = 0; i < td->i2c_lut_size; i++) {
- if (regulator_list_voltage(td->vdd_reg, td->i2c_lut[i]) == uv)
+ if ((regulator_list_voltage(td->vdd_reg, td->i2c_lut[i]) /
+ td->soc->alignment) == align_volt)
return i;
}

@@ -1380,15 +1381,17 @@ di_err1:
*/
static int find_vdd_map_entry_exact(struct tegra_dfll *td, int uV)
{
- int i, n_voltages, reg_uV;
+ int i, n_voltages, reg_volt, align_volt;

+ align_volt = uV / td->soc->alignment;
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_volt = regulator_list_voltage(td->vdd_reg, i) /
+ td->soc->alignment;
+ if (reg_volt < 0)
break;

- if (uV == reg_uV)
+ if (align_volt == reg_volt)
return i;
}

@@ -1402,15 +1405,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_volt, align_volt;

+ align_volt = uV / td->soc->alignment;
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_volt = regulator_list_voltage(td->vdd_reg, i) /
+ td->soc->alignment;
+ if (reg_volt < 0)
break;

- if (uV <= reg_uV)
+ if (align_volt <= reg_volt)
return i;
}

diff --git a/drivers/clk/tegra/clk-dfll.h b/drivers/clk/tegra/clk-dfll.h
index 2e4c077..5714691 100644
--- a/drivers/clk/tegra/clk-dfll.h
+++ b/drivers/clk/tegra/clk-dfll.h
@@ -37,6 +37,7 @@
struct tegra_dfll_soc_data {
struct device *dev;
unsigned int min_millivolts;
+ unsigned int alignment;
u32 tune0_low;
u32 tune0_high;
u32 tune1;
diff --git a/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c b/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c
index 6125333..5e5958e 100644
--- a/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c
+++ b/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c
@@ -42,9 +42,6 @@ static const struct cvb_table tegra124_cpu_cvb_tables[] = {
.process_id = -1,
.min_millivolts = 900,
.max_millivolts = 1260,
- .alignment = {
- .step_uv = 10000, /* 10mV */
- },
.speedo_scale = 100,
.voltage_scale = 1000,
.cvb_table = {
@@ -84,7 +81,8 @@ static const struct cvb_table tegra124_cpu_cvb_tables[] = {

static int tegra124_dfll_fcpu_probe(struct platform_device *pdev)
{
- int process_id, speedo_id, speedo_value;
+ int process_id, speedo_id, speedo_value, ret;
+ struct rail_alignment align;
struct tegra_dfll_soc_data *soc;
const struct cvb_table *cvb;

@@ -108,8 +106,24 @@ static int tegra124_dfll_fcpu_probe(struct platform_device *pdev)
return -ENODEV;
}

+ ret = of_property_read_u32(pdev->dev.of_node, "nvidia,align-offset-uv",
+ &align.offset_uv);
+ if (ret < 0) {
+ dev_dbg(&pdev->dev,
+ "offset uv not found, the default value will be 0\n");
+ align.offset_uv = 0;
+ }
+
+ ret = of_property_read_u32(pdev->dev.of_node, "nvidia,align-step-uv",
+ &align.step_uv);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "missing step uv\n");
+ return -EINVAL;
+ }
+
cvb = tegra_cvb_build_opp_table(tegra124_cpu_cvb_tables,
ARRAY_SIZE(tegra124_cpu_cvb_tables),
+ &align,
process_id, speedo_id, speedo_value,
cpu_max_freq_table[speedo_id],
soc->dev);
@@ -120,6 +134,7 @@ static int tegra124_dfll_fcpu_probe(struct platform_device *pdev)
}

soc->min_millivolts = cvb->min_millivolts;
+ soc->alignment = align.step_uv;
soc->tune0_low = cvb->cpu_dfll_data.tune0_low;
soc->tune0_high = cvb->cpu_dfll_data.tune0_high;
soc->tune1 = cvb->cpu_dfll_data.tune1;
diff --git a/drivers/clk/tegra/cvb.c b/drivers/clk/tegra/cvb.c
index 69c74ee..a3be166 100644
--- a/drivers/clk/tegra/cvb.c
+++ b/drivers/clk/tegra/cvb.c
@@ -62,13 +62,13 @@ static int round_voltage(int mv, const struct rail_alignment *align, int up)
}

static int build_opp_table(const struct cvb_table *d,
+ const struct rail_alignment *align,
int speedo_value,
unsigned long max_freq,
struct device *opp_dev)
{
int i, ret, dfll_mv, min_mv, max_mv;
const struct cvb_table_freq_entry *table = NULL;
- const struct rail_alignment *align = &d->alignment;

min_mv = round_voltage(d->min_millivolts, align, UP);
max_mv = round_voltage(d->max_millivolts, align, DOWN);
@@ -110,8 +110,11 @@ static int build_opp_table(const struct cvb_table *d,
*/
const struct cvb_table *tegra_cvb_build_opp_table(
const struct cvb_table *cvb_tables,
- size_t sz, int process_id,
- int speedo_id, int speedo_value,
+ size_t sz,
+ const struct rail_alignment *align,
+ int process_id,
+ int speedo_id,
+ int speedo_value,
unsigned long max_rate,
struct device *opp_dev)
{
@@ -125,7 +128,8 @@ const struct cvb_table *tegra_cvb_build_opp_table(
if (d->process_id != -1 && d->process_id != process_id)
continue;

- ret = build_opp_table(d, speedo_value, max_rate, opp_dev);
+ ret = build_opp_table(
+ d, align, speedo_value, max_rate, opp_dev);
return ret ? ERR_PTR(ret) : d;
}

diff --git a/drivers/clk/tegra/cvb.h b/drivers/clk/tegra/cvb.h
index f62cdc4..4964c49 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,11 @@ struct cvb_table {

const struct cvb_table *tegra_cvb_build_opp_table(
const struct cvb_table *cvb_tables,
- size_t sz, int process_id,
- int speedo_id, int speedo_value,
+ size_t sz,
+ const struct rail_alignment *align,
+ int process_id,
+ int speedo_id,
+ int speedo_value,
unsigned long max_rate,
struct device *opp_dev);

--
2.8.1

2016-04-22 10:32:18

by Penny Chiu

[permalink] [raw]
Subject: [PATCH 02/11] clk: tegra: dfll: Move SoC specific data into of_device_id

Move all SoC specific fcpu data into of_device_id structure, and
move SoC fcpu data assignments from init function to probe
function.

Signed-off-by: Penny Chiu <[email protected]>
---
drivers/clk/tegra/clk-tegra124-dfll-fcpu.c | 51 ++++++++++++++++++++++--------
1 file changed, 37 insertions(+), 14 deletions(-)

diff --git a/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c b/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c
index 5e5958e..b577bc6 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/module.h>
+#include <linux/of_device.h>
#include <linux/platform_device.h>
#include <soc/tegra/fuse.h>

@@ -28,8 +29,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,
@@ -79,18 +87,39 @@ static const struct cvb_table tegra124_cpu_cvb_tables[] = {
},
};

+static const struct dfll_fcpu_data tegra124_dfll_fcpu_data = {
+ .cpu_max_freq_table = tegra124_cpu_max_freq_table,
+ .cpu_max_freq_table_size = ARRAY_SIZE(tegra124_cpu_max_freq_table),
+ .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
+ },
+ { },
+};
+MODULE_DEVICE_TABLE(of, tegra124_dfll_fcpu_of_match);
+
static int tegra124_dfll_fcpu_probe(struct platform_device *pdev)
{
int process_id, speedo_id, speedo_value, ret;
struct rail_alignment align;
struct tegra_dfll_soc_data *soc;
const struct cvb_table *cvb;
+ 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;
@@ -121,12 +150,12 @@ static int tegra124_dfll_fcpu_probe(struct platform_device *pdev)
return -EINVAL;
}

- cvb = tegra_cvb_build_opp_table(tegra124_cpu_cvb_tables,
- ARRAY_SIZE(tegra124_cpu_cvb_tables),
- &align,
- process_id, speedo_id, speedo_value,
- cpu_max_freq_table[speedo_id],
- soc->dev);
+ cvb = tegra_cvb_build_opp_table(fcpu_data->cpu_cvb_tables,
+ fcpu_data->cpu_cvb_tables_size,
+ &align,
+ process_id, speedo_id, speedo_value,
+ fcpu_data->cpu_max_freq_table[speedo_id],
+ soc->dev);
if (IS_ERR(cvb)) {
dev_err(&pdev->dev, "couldn't build OPP table: %ld\n",
PTR_ERR(cvb));
@@ -142,12 +171,6 @@ static int tegra124_dfll_fcpu_probe(struct platform_device *pdev)
return tegra_dfll_register(pdev, soc);
}

-static const struct of_device_id tegra124_dfll_fcpu_of_match[] = {
- { .compatible = "nvidia,tegra124-dfll", },
- { },
-};
-MODULE_DEVICE_TABLE(of, tegra124_dfll_fcpu_of_match);
-
static const struct dev_pm_ops tegra124_dfll_pm_ops = {
SET_RUNTIME_PM_OPS(tegra_dfll_runtime_suspend,
tegra_dfll_runtime_resume, NULL)
--
2.8.1

2016-04-22 10:32:22

by Penny Chiu

[permalink] [raw]
Subject: [PATCH 03/11] clk: tegra: Add DFLL DVCO reset control for Tegra210

The DVCO present in the DFLL IP block has a separate reset line,
exposed via the CAR IP block. This reset line is asserted upon SoC
reset. Unless something (such as the DFLL driver) deasserts this
line, the DVCO will not oscillate, although reads and writes to the
DFLL IP block will complete.

Signed-off-by: Penny Chiu <[email protected]>
---
drivers/clk/tegra/clk-tegra210.c | 68 ++++++++++++++++++++++++++++++++
include/dt-bindings/reset/tegra210-car.h | 12 ++++++
2 files changed, 80 insertions(+)
create mode 100644 include/dt-bindings/reset/tegra210-car.h

diff --git a/drivers/clk/tegra/clk-tegra210.c b/drivers/clk/tegra/clk-tegra210.c
index d3709b1..3d70b38 100644
--- a/drivers/clk/tegra/clk-tegra210.c
+++ b/drivers/clk/tegra/clk-tegra210.c
@@ -24,6 +24,7 @@
#include <linux/export.h>
#include <linux/clk/tegra.h>
#include <dt-bindings/clock/tegra210-car.h>
+#include <dt-bindings/reset/tegra210-car.h>

#include "clk.h"
#include "clk-id.h"
@@ -39,6 +40,9 @@
#define CLK_SOURCE_CSITE 0x1d4
#define CLK_SOURCE_EMC 0x19c

+#define RST_DFLL_DVCO 0x2f4
+#define DVFS_DFLL_RESET_SHIFT 0
+
#define PLLC_BASE 0x80
#define PLLC_OUT 0x84
#define PLLC_MISC0 0x88
@@ -2781,6 +2785,68 @@ static void __init tegra210_clock_apply_init_table(void)
}

/**
+ * tegra210_car_barrier - wait for pending writes to the CAR to complete
+ *
+ * Wait for any outstanding writes to the CAR MMIO space from this CPU
+ * to complete before continuing execution. No return value.
+ */
+static void tegra210_car_barrier(void)
+{
+ readl_relaxed(clk_base + RST_DFLL_DVCO);
+}
+
+/**
+ * tegra210_clock_assert_dfll_dvco_reset - assert the DFLL's DVCO reset
+ *
+ * Assert the reset line of the DFLL's DVCO. No return value.
+ */
+static void tegra210_clock_assert_dfll_dvco_reset(void)
+{
+ u32 v;
+
+ v = readl_relaxed(clk_base + RST_DFLL_DVCO);
+ v |= (1 << DVFS_DFLL_RESET_SHIFT);
+ writel_relaxed(v, clk_base + RST_DFLL_DVCO);
+ tegra210_car_barrier();
+}
+
+/**
+ * tegra210_clock_deassert_dfll_dvco_reset - deassert the DFLL's DVCO reset
+ *
+ * Deassert the reset line of the DFLL's DVCO, allowing the DVCO to
+ * operate. No return value.
+ */
+static void tegra210_clock_deassert_dfll_dvco_reset(void)
+{
+ u32 v;
+
+ v = readl_relaxed(clk_base + RST_DFLL_DVCO);
+ v &= ~(1 << DVFS_DFLL_RESET_SHIFT);
+ writel_relaxed(v, clk_base + RST_DFLL_DVCO);
+ tegra210_car_barrier();
+}
+
+static int tegra210_reset_assert(unsigned long id)
+{
+ if (id == TEGRA210_RST_DFLL_DVCO)
+ tegra210_clock_assert_dfll_dvco_reset();
+ else
+ return -EINVAL;
+
+ return 0;
+}
+
+static int tegra210_reset_deassert(unsigned long id)
+{
+ if (id == TEGRA210_RST_DFLL_DVCO)
+ tegra210_clock_deassert_dfll_dvco_reset();
+ else
+ return -EINVAL;
+
+ return 0;
+}
+
+/**
* tegra210_clock_init - Tegra210-specific clock initialization
* @np: struct device_node * of the DT node for the SoC CAR IP block
*
@@ -2844,6 +2910,8 @@ static void __init tegra210_clock_init(struct device_node *np)

tegra_super_clk_gen5_init(clk_base, pmc_base, tegra210_clks,
&pll_x_params);
+ tegra_init_special_resets(1, tegra210_reset_assert,
+ tegra210_reset_deassert);
tegra_add_of_provider(np);
tegra_register_devclks(devclks, ARRAY_SIZE(devclks));

diff --git a/include/dt-bindings/reset/tegra210-car.h b/include/dt-bindings/reset/tegra210-car.h
new file mode 100644
index 0000000..a8632af
--- /dev/null
+++ b/include/dt-bindings/reset/tegra210-car.h
@@ -0,0 +1,12 @@
+/*
+ * This header provides Tegra210-specific constants for binding
+ * nvidia,tegra210-car.
+ */
+
+#ifndef _DT_BINDINGS_RESET_TEGRA210_CAR_H
+#define _DT_BINDINGS_RESET_TEGRA210_CAR_H
+
+#define TEGRA210_RESET(x) (7 * 32 + (x))
+#define TEGRA210_RST_DFLL_DVCO TEGRA210_RESET(0)
+
+#endif /* _DT_BINDINGS_RESET_TEGRA210_CAR_H */
--
2.8.1

2016-04-22 10:32:37

by Penny Chiu

[permalink] [raw]
Subject: [PATCH 07/11] cpufreq: tegra124: Add Tegra210 support

Add tegra210 support in tegra124 cpufreq driver.

Signed-off-by: Penny Chiu <[email protected]>
---
drivers/cpufreq/tegra124-cpufreq.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/cpufreq/tegra124-cpufreq.c b/drivers/cpufreq/tegra124-cpufreq.c
index 20bcceb..c891e6e 100644
--- a/drivers/cpufreq/tegra124-cpufreq.c
+++ b/drivers/cpufreq/tegra124-cpufreq.c
@@ -188,7 +188,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;

/*
--
2.8.1

2016-04-22 10:32:42

by Penny Chiu

[permalink] [raw]
Subject: [PATCH 08/11] arm64: tegra: Add PWM regulator for CPU rail on Jetson TX1

The CPU rail on Jetson TX1 is supplied by a Open Voltage Regulator
(OVR) which is controlled by PWM signals, and the PWM signals are
generated by DFLL PWM controller. So this patch adds DFLL_PWM
device-tree node and a PWM regulator node for the OVR.

Signed-off-by: Penny Chiu <[email protected]>
---
arch/arm64/boot/dts/nvidia/tegra210-p2180.dtsi | 40 ++++++++++++++++++++++
arch/arm64/boot/dts/nvidia/tegra210-p2371-2180.dts | 23 +++++++++++++
2 files changed, 63 insertions(+)

diff --git a/arch/arm64/boot/dts/nvidia/tegra210-p2180.dtsi b/arch/arm64/boot/dts/nvidia/tegra210-p2180.dtsi
index 316c92c..9d02db2 100644
--- a/arch/arm64/boot/dts/nvidia/tegra210-p2180.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra210-p2180.dtsi
@@ -42,4 +42,44 @@
clock-frequency = <32768>;
};
};
+
+ pwm_dfll: pwm@70110000 {
+ compatible = "nvidia,tegra210-dfll-pwm";
+ reg = <0x0 0x70110000 0x0 0x400>;
+ clocks = <&tegra_car TEGRA210_CLK_DFLL_REF>;
+ clock-names = "ref";
+ #pwm-cells = <2>;
+ pwm-regulator = <&cpu_ovr_reg>;
+ status = "disable";
+ };
+
+ pwm_regulators {
+ compatible = "simple-bus";
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ cpu_ovr_reg: pwm-regulator@0 {
+ status = "okay";
+ reg = <0>;
+ compatible = "pwm-regulator";
+ pwms = <&pwm_dfll 0 2500>;
+ regulator-name = "vdd_cpu";
+ regulator-min-microvolt = <708000>;
+ regulator-max-microvolt = <1322400>;
+ regulator-always-on;
+ regulator-boot-on;
+ voltage-table =
+ <708000 0>, <727200 1>, <746400 2>,
+ <765600 3>, <784800 4>, <804000 5>,
+ <823200 6>, <842400 7>, <861600 8>,
+ <880800 9>, <900000 10>, <919200 11>,
+ <938400 12>, <957600 13>, <976800 14>,
+ <996000 15>, <1015200 16>, <1034400 17>,
+ <1053600 18>, <1072800 19>, <1092000 20>,
+ <1111200 21>, <1130400 22>, <1149600 23>,
+ <1168800 24>, <1188000 25>, <1207200 26>,
+ <1226400 27>, <1245600 28>, <1264800 29>,
+ <1284000 30>, <1303200 31>, <1322400 32>;
+ };
+ };
};
diff --git a/arch/arm64/boot/dts/nvidia/tegra210-p2371-2180.dts b/arch/arm64/boot/dts/nvidia/tegra210-p2371-2180.dts
index 683b339..c9fd59a 100644
--- a/arch/arm64/boot/dts/nvidia/tegra210-p2371-2180.dts
+++ b/arch/arm64/boot/dts/nvidia/tegra210-p2371-2180.dts
@@ -6,4 +6,27 @@
/ {
model = "NVIDIA Jetson TX1 Developer Kit";
compatible = "nvidia,p2371-2180", "nvidia,tegra210";
+
+ 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>;
+ };
+ };
+ };
+
+ pwm_dfll: pwm@70110000 {
+ status = "okay";
+ pinctrl-names = "dvfs_pwm_enable", "dvfs_pwm_disable";
+ pinctrl-0 = <&dvfs_pwm_active_state>;
+ pinctrl-1 = <&dvfs_pwm_inactive_state>;
+ };
};
--
2.8.1

2016-04-22 10:32:48

by Penny Chiu

[permalink] [raw]
Subject: [PATCH 10/11] arm64: tegra: Add clock properties on cpu0 for Tegra210

Add clocks, clock-names, and clock-latency into cpu0 node.
These properties will be used by cpufreq driver.

Signed-off-by: Penny Chiu <[email protected]>
---
arch/arm64/boot/dts/nvidia/tegra210.dtsi | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/arch/arm64/boot/dts/nvidia/tegra210.dtsi b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
index 204d9cd..1a85857 100644
--- a/arch/arm64/boot/dts/nvidia/tegra210.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
@@ -782,6 +782,12 @@
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_OUT4>,
+ <&dfll>;
+ clock-names = "cpu_g", "pll_x", "pll_p", "dfll";
+ clock-latency = <300000>;
};

cpu@1 {
--
2.8.1

2016-04-22 10:32:53

by Penny Chiu

[permalink] [raw]
Subject: [PATCH 11/11] arm64: config: Enable CPUFreq-DT, Tegra DFLL PWM, and PWM regulator

Enable CPUFreq-DT, PWM regulator for CPU rail, and related PWM
controller driver.

Signed-off-by: Penny Chiu <[email protected]>
---
arch/arm64/configs/defconfig | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 5091d35..865daca 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -75,6 +75,7 @@ CONFIG_COMPAT=y
CONFIG_CPU_IDLE=y
CONFIG_ARM_CPUIDLE=y
CONFIG_CPU_FREQ=y
+CONFIG_CPUFREQ_DT=y
CONFIG_ARM_BIG_LITTLE_CPUFREQ=y
CONFIG_ARM_SCPI_CPUFREQ=y
CONFIG_NET=y
@@ -177,11 +178,14 @@ CONFIG_THERMAL_EMULATION=y
CONFIG_EXYNOS_THERMAL=y
CONFIG_MFD_SPMI_PMIC=y
CONFIG_MFD_SEC_CORE=y
+CONFIG_PWM=y
+CONFIG_PWM_TEGRA_DFLL=y
CONFIG_REGULATOR=y
CONFIG_REGULATOR_FIXED_VOLTAGE=y
CONFIG_REGULATOR_QCOM_SMD_RPM=y
CONFIG_REGULATOR_QCOM_SPMI=y
CONFIG_REGULATOR_S2MPS11=y
+CONFIG_REGULATOR_PWM=y
CONFIG_FB=y
CONFIG_FB_ARMCLCD=y
CONFIG_FRAMEBUFFER_CONSOLE=y
--
2.8.1

2016-04-22 10:33:41

by Penny Chiu

[permalink] [raw]
Subject: [PATCH 09/11] arm64: tegra: Add DFLL clock node on Jetson TX1

Add DFLL clock device-tree node for Tegra210 DFLL IP block.

Signed-off-by: Penny Chiu <[email protected]>
---
arch/arm64/boot/dts/nvidia/tegra210-p2180.dtsi | 16 ++++++++++++++++
arch/arm64/boot/dts/nvidia/tegra210.dtsi | 15 +++++++++++++++
2 files changed, 31 insertions(+)

diff --git a/arch/arm64/boot/dts/nvidia/tegra210-p2180.dtsi b/arch/arm64/boot/dts/nvidia/tegra210-p2180.dtsi
index 9d02db2..5cf07f2 100644
--- a/arch/arm64/boot/dts/nvidia/tegra210-p2180.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra210-p2180.dtsi
@@ -43,6 +43,22 @@
};
};

+ dfll: clock@70110000 {
+ status = "okay";
+
+ reg = <0x0 0x70110000 0x0 0x400>;
+ vdd-cpu-supply = <&cpu_ovr_reg>;
+ nvidia,pwm-to-pmic;
+ nvidia,init-uv = <1000000>;
+ nvidia,align-step-uv = <19200>; /* 19.2mv */
+ nvidia,sample-rate = <25000>;
+ nvidia,droop-ctrl = <0x00000f00>;
+ nvidia,force-mode = <1>;
+ nvidia,cf = <6>;
+ nvidia,ci = <0>;
+ nvidia,cg = <2>;
+ };
+
pwm_dfll: pwm@70110000 {
compatible = "nvidia,tegra210-dfll-pwm";
reg = <0x0 0x70110000 0x0 0x400>;
diff --git a/arch/arm64/boot/dts/nvidia/tegra210.dtsi b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
index ba0462e..204d9cd 100644
--- a/arch/arm64/boot/dts/nvidia/tegra210.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
@@ -3,6 +3,7 @@
#include <dt-bindings/memory/tegra210-mc.h>
#include <dt-bindings/pinctrl/pinctrl-tegra.h>
#include <dt-bindings/interrupt-controller/arm-gic.h>
+#include <dt-bindings/reset/tegra210-car.h>

/ {
compatible = "nvidia,tegra210";
@@ -684,6 +685,20 @@
status = "disabled";
};

+ dfll: clock@70110000 {
+ compatible = "nvidia,tegra210-dfll";
+ interrupts = <GIC_SPI 62 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&tegra_car TEGRA210_CLK_DFLL_SOC>,
+ <&tegra_car TEGRA210_CLK_DFLL_REF>,
+ <&tegra_car TEGRA210_CLK_I2C5>;
+ clock-names = "soc", "ref", "i2c";
+ resets = <&tegra_car TEGRA210_RST_DFLL_DVCO>;
+ reset-names = "dvco";
+ #clock-cells = <0>;
+ clock-output-names = "dfllCPU_out";
+ status = "disabled";
+ };
+
usb@7d000000 {
compatible = "nvidia,tegra210-ehci", "nvidia,tegra30-ehci", "usb-ehci";
reg = <0x0 0x7d000000 0x0 0x4000>;
--
2.8.1

2016-04-22 10:32:34

by Penny Chiu

[permalink] [raw]
Subject: [PATCH 05/11] pwm: tegra-dfll: Add driver for Tegra DFLL PWM controller

Tegra DFLL IP block controls off-chip PMIC via I2C bus or PWM
signals. This driver exposes DFLL as a PWM controller to generate
PWM signals to PWM regulator.

Tegra DFLL HW changes regulator voltage by adjusting PWM signals
duty cycle automatically based on required DVCO frequency, so PWM
regulator client doesn't need to change voltage by SW.

In this driver, it only configs proper PWM rate in the driver
initialization, and provides two APIs for clients to determine when
to enable and disable generating PWM signals by setting related
pinmux tristate.

Signed-off-by: Penny Chiu <[email protected]>
---
.../bindings/pwm/nvidia,tegra-dfll-pwm.txt | 48 +++
drivers/pwm/Kconfig | 10 +
drivers/pwm/Makefile | 1 +
drivers/pwm/pwm-tegra-dfll.c | 322 +++++++++++++++++++++
include/soc/tegra/pwm-tegra-dfll.h | 27 ++
5 files changed, 408 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pwm/nvidia,tegra-dfll-pwm.txt
create mode 100644 drivers/pwm/pwm-tegra-dfll.c
create mode 100644 include/soc/tegra/pwm-tegra-dfll.h

diff --git a/Documentation/devicetree/bindings/pwm/nvidia,tegra-dfll-pwm.txt b/Documentation/devicetree/bindings/pwm/nvidia,tegra-dfll-pwm.txt
new file mode 100644
index 0000000..bd0d247
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/nvidia,tegra-dfll-pwm.txt
@@ -0,0 +1,48 @@
+Tegra SoC DFLL PWM controller
+
+Required properties:
+- compatible: For Tegra210, must contain "nvidia,tegra210-dfll-pwm".
+- reg: physical base address and length of the controller's registers
+- #pwm-cells: should be 2. See pwm.txt in this directory for a description of
+ the cells format.
+- clock-names: Must include the "ref" entry.
+- clocks: Must contain one entry, for the DFLL closed loop reference clock.
+ See ../clocks/clock-bindings.txt for details.
+- pwm-regulator: phandle to PWM regulator for using this PWM controller.
+- pinctrl-names: Must contain two entries to enable and disable PWM signals
+ output pinmux states.
+- pinctrl-0: pinmux state to enable PWM signals output
+- pinctrl-1: pinmux state to disable PWM signals output
+
+Example:
+
+ 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>;
+ };
+ };
+ };
+
+ pwm_dfll: pwm@70110000 {
+ compatible = "nvidia,tegra210-dfll-pwm";
+ reg = <0x0 0x70110000 0x0 0x400>;
+ clocks = <&tegra_car TEGRA210_CLK_DFLL_REF>;
+ clock-names = "ref";
+ #pwm-cells = <2>;
+ pwm-regulator = <&cpu_ovr_reg>;
+
+ pinctrl-names = "dvfs_pwm_enable", "dvfs_pwm_disable";
+ pinctrl-0 = <&dvfs_pwm_active_state>;
+ pinctrl-1 = <&dvfs_pwm_inactive_state>;
+
+ status = "okay";
+ };
diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index c182efc..dd67cad 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -382,6 +382,16 @@ config PWM_TEGRA
To compile this driver as a module, choose M here: the module
will be called pwm-tegra.

+config PWM_TEGRA_DFLL
+ tristate "NVIDIA Tegra DFLL PWM support"
+ depends on ARCH_TEGRA
+ help
+ PWM driver support for the Tegra DFLL module found on NVIDIA
+ Tegra SoCs.
+
+ To compile this driver as a module, choose M here: the module
+ will be called pwm-tegra-dfll.
+
config PWM_TIECAP
tristate "ECAP PWM support"
depends on ARCH_OMAP2PLUS || ARCH_DAVINCI_DA8XX
diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
index dd35bc1..22414cb 100644
--- a/drivers/pwm/Makefile
+++ b/drivers/pwm/Makefile
@@ -36,6 +36,7 @@ obj-$(CONFIG_PWM_SPEAR) += pwm-spear.o
obj-$(CONFIG_PWM_STI) += pwm-sti.o
obj-$(CONFIG_PWM_SUN4I) += pwm-sun4i.o
obj-$(CONFIG_PWM_TEGRA) += pwm-tegra.o
+obj-$(CONFIG_PWM_TEGRA_DFLL) += pwm-tegra-dfll.o
obj-$(CONFIG_PWM_TIECAP) += pwm-tiecap.o
obj-$(CONFIG_PWM_TIEHRPWM) += pwm-tiehrpwm.o
obj-$(CONFIG_PWM_TIPWMSS) += pwm-tipwmss.o
diff --git a/drivers/pwm/pwm-tegra-dfll.c b/drivers/pwm/pwm-tegra-dfll.c
new file mode 100644
index 0000000..74d6b97
--- /dev/null
+++ b/drivers/pwm/pwm-tegra-dfll.c
@@ -0,0 +1,322 @@
+/*
+ * drivers/pwm/pwm-tegra-dfll.c
+ *
+ * Tegra DFLL PWM controller driver
+ *
+ * Copyright (c) 2016, NVIDIA Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ */
+
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/pwm.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/pinctrl/consumer.h>
+#include <soc/tegra/pwm-tegra-dfll.h>
+
+/* DFLL_CTRL: DFLL control register */
+#define DFLL_CTRL 0x00
+
+/* DFLL_OUTPUT_CFG: closed loop mode control registers */
+#define DFLL_OUTPUT_CFG 0x20
+#define OUT_MASK 0x3f
+#define DFLL_OUTPUT_CFG_PWM_DELTA (0x1 << 7)
+#define DFLL_OUTPUT_CFG_PWM_ENABLE (0x1 << 6)
+#define DFLL_OUTPUT_CFG_PWM_DIV_SHIFT 0
+#define DFLL_OUTPUT_CFG_PWM_DIV_MASK \
+ (OUT_MASK << DFLL_OUTPUT_CFG_PWM_DIV_SHIFT)
+
+/* MAX_DFLL_VOLTAGES: number of LUT entries in the DFLL IP block */
+#define DFLL_MAX_VOLTAGES 33
+
+#define DFLL_OF_PWM_PERIOD_CELL 1
+
+/**
+ * struct tegra_dfll_pwm_chip - DFLL PWM controller data
+ * @pwm_pin: pinmux for PWM signals output
+ * @pwm_enable_state: enabled states of pinmux for PWM signals output
+ * @pwm_disable_state: disabled states of pinmux for PWM signals output
+ * @mmio_base: mmio base for access DFLL registers
+ * @ref_clk: referenced source clock
+ * @pwm_rate: PWM rate for DFLL PWM output config register
+ */
+struct tegra_dfll_pwm_chip {
+ struct pwm_chip chip;
+ struct device *dev;
+
+ struct pinctrl *pwm_pin;
+ struct pinctrl_state *pwm_enable_state;
+ struct pinctrl_state *pwm_disable_state;
+
+ void __iomem *mmio_base;
+ struct clk *ref_clk;
+
+ unsigned long ref_rate;
+ unsigned long pwm_rate;
+};
+
+static struct tegra_dfll_pwm_chip *tdpc;
+
+/*
+ * Register accessors
+ */
+static inline u32 pwm_readl(u32 offs)
+{
+ return __raw_readl(tdpc->mmio_base + offs);
+}
+
+static inline void pwm_writel(u32 val, u32 offs)
+{
+ __raw_writel(val, tdpc->mmio_base + offs);
+ pwm_readl(DFLL_CTRL);
+}
+
+static int tegra_dfll_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
+ int duty_ns, int period_ns)
+{
+ return 0;
+}
+
+static int tegra_dfll_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
+{
+ u32 val;
+
+ val = pwm_readl(DFLL_OUTPUT_CFG);
+ val |= DFLL_OUTPUT_CFG_PWM_ENABLE;
+ pwm_writel(val, DFLL_OUTPUT_CFG);
+
+ dev_info(tdpc->dev, "DFLL_PWM is enabled\n");
+
+ return 0;
+}
+
+static void tegra_dfll_pwm_disable(struct pwm_chip *chip,
+ struct pwm_device *pwm)
+{
+ u32 val;
+
+ val = pwm_readl(DFLL_OUTPUT_CFG);
+ val &= ~DFLL_OUTPUT_CFG_PWM_ENABLE;
+ pwm_writel(val, DFLL_OUTPUT_CFG);
+
+ dev_info(tdpc->dev, "DFLL_PWM is disabled\n");
+
+ return 0;
+}
+
+/**
+ * tegra_dfll_pwm_output_enable - enable DFLL PWM signals output
+ *
+ * Enable DFLL PWM signals output by changing related pinmux state
+ */
+int tegra_dfll_pwm_output_enable(void)
+{
+ int ret;
+
+ ret = pinctrl_select_state(tdpc->pwm_pin, tdpc->pwm_enable_state);
+ if (ret < 0) {
+ dev_err(tdpc->dev, "setting enable state failed\n");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(tegra_dfll_pwm_output_enable);
+
+/**
+ * tegra_dfll_pwm_output_disable - disable DFLL PWM signals output
+ *
+ * Disable DFLL PWM signals output by changing related pinmux state
+ */
+int tegra_dfll_pwm_output_disable(void)
+{
+ int ret;
+
+ ret = pinctrl_select_state(tdpc->pwm_pin, tdpc->pwm_disable_state);
+ if (ret < 0) {
+ dev_err(tdpc->dev, "setting enable state failed\n");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(tegra_dfll_pwm_output_disable);
+
+static const struct pwm_ops tegra_dfll_pwm_ops = {
+ .config = tegra_dfll_pwm_config,
+ .enable = tegra_dfll_pwm_enable,
+ .disable = tegra_dfll_pwm_disable,
+ .owner = THIS_MODULE,
+};
+
+/**
+ * dt_parse_pwm_regulator - parse PWM regulator from device-tree
+ *
+ * Parse DFLL PWM controller client to get and calcluate initialized
+ * DFLL PWM rate.
+ */
+static int dt_parse_pwm_regulator(void)
+{
+ struct device_node *r_dn =
+ of_parse_phandle(tdpc->dev->of_node, "pwm-regulator", 0);
+ struct of_phandle_args args;
+ unsigned long val;
+ int ret;
+
+ /* pwm regulator device */
+ if (!r_dn) {
+ dev_err(tdpc->dev, "DT: missing pwm-regulator property\n");
+ return -EINVAL;
+ }
+
+ ret = of_parse_phandle_with_args(r_dn, "pwms", "#pwm-cells", 0, &args);
+ if (ret) {
+ dev_err(tdpc->dev, "DT: failed to parse pwms property\n");
+ return -EINVAL;
+ }
+ of_node_put(args.np);
+
+ if (args.args_count <= DFLL_OF_PWM_PERIOD_CELL) {
+ dev_err(tdpc->dev, "DT: low #pwm-cells %d\n", args.args_count);
+ return -EINVAL;
+ }
+
+ /* convert pwm period in ns to DFLL pwm rate in Hz */
+ val = args.args[DFLL_OF_PWM_PERIOD_CELL];
+ val = (NSEC_PER_SEC / val) * (DFLL_MAX_VOLTAGES - 1);
+ tdpc->pwm_rate = val;
+ dev_info(tdpc->dev, "DFLL pwm-rate: %lu\n", val);
+
+ return 0;
+}
+
+/**
+ * tegra_dfll_pwm_init - init Tegra DFLL PWM controller
+ *
+ * Calculate the DIV value and write into DFLL register
+ */
+static int tegra_dfll_pwm_init(void)
+{
+ u32 div, val;
+
+ pwm_writel(0, DFLL_OUTPUT_CFG);
+
+ div = DIV_ROUND_UP(tdpc->ref_rate, tdpc->pwm_rate);
+ val = (div << DFLL_OUTPUT_CFG_PWM_DIV_SHIFT) &
+ DFLL_OUTPUT_CFG_PWM_DIV_MASK;
+ pwm_writel(val, DFLL_OUTPUT_CFG);
+
+ return 0;
+}
+
+static int tegra_dfll_pwm_probe(struct platform_device *pdev)
+{
+ struct resource *res;
+ int ret;
+
+ tdpc = devm_kzalloc(&pdev->dev, sizeof(*tdpc), GFP_KERNEL);
+ if (!tdpc)
+ return -ENOMEM;
+
+ tdpc->dev = &pdev->dev;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ tdpc->mmio_base = devm_ioremap_resource(tdpc->dev, res);
+ if (IS_ERR(tdpc->mmio_base))
+ return PTR_ERR(tdpc->mmio_base);
+
+ platform_set_drvdata(pdev, tdpc);
+
+ tdpc->chip.dev = tdpc->dev;
+ tdpc->chip.ops = &tegra_dfll_pwm_ops;
+ tdpc->chip.base = -1;
+ tdpc->chip.npwm = 1;
+
+ ret = pwmchip_add(&tdpc->chip);
+ if (ret < 0) {
+ dev_err(tdpc->dev, "pwmchip_add() failed: %d\n", ret);
+ return ret;
+ }
+
+ tdpc->ref_clk = devm_clk_get(tdpc->dev, "ref");
+ if (IS_ERR(tdpc->ref_clk)) {
+ dev_err(tdpc->dev, "DT: missing ref clock\n");
+ return PTR_ERR(tdpc->ref_clk);
+ }
+
+ tdpc->ref_rate = clk_get_rate(tdpc->ref_clk);
+
+ tdpc->pwm_pin = devm_pinctrl_get(tdpc->dev);
+ if (IS_ERR(tdpc->pwm_pin)) {
+ dev_err(tdpc->dev, "DT: missing pinctrl device\n");
+ return PTR_ERR(tdpc->pwm_pin);
+ }
+
+ tdpc->pwm_enable_state = pinctrl_lookup_state(tdpc->pwm_pin,
+ "dvfs_pwm_enable");
+ if (IS_ERR(tdpc->pwm_enable_state)) {
+ dev_err(tdpc->dev, "DT: missing pwm enabled state\n");
+ return PTR_ERR(tdpc->pwm_enable_state);
+ }
+
+ tdpc->pwm_disable_state = pinctrl_lookup_state(tdpc->pwm_pin,
+ "dvfs_pwm_disable");
+ if (IS_ERR(tdpc->pwm_disable_state)) {
+ dev_err(tdpc->dev, "DT: missing pwm disabled state\n");
+ return PTR_ERR(tdpc->pwm_disable_state);
+ }
+
+ ret = dt_parse_pwm_regulator();
+ if (ret < 0) {
+ dev_err(tdpc->dev, "failed to parse pwm regulator\n");
+ return ret;
+ }
+
+ ret = tegra_dfll_pwm_init();
+ if (ret < 0) {
+ dev_err(tdpc->dev, "failed to init DFLL pwm\n");
+ return ret;
+ }
+
+ return 0;
+}
+
+static int tegra_dfll_pwm_remove(struct platform_device *pdev)
+{
+ return pwmchip_remove(&tdpc->chip);
+}
+
+static const struct of_device_id tegra_dfll_pwm_of_match[] = {
+ { .compatible = "nvidia,tegra210-dfll-pwm" },
+ { }
+};
+
+MODULE_DEVICE_TABLE(of, tegra_dfll_pwm_of_match);
+
+static struct platform_driver tegra_dfll_pwm_driver = {
+ .driver = {
+ .name = "tegra-dfll-pwm",
+ .of_match_table = tegra_dfll_pwm_of_match,
+ },
+ .probe = tegra_dfll_pwm_probe,
+ .remove = tegra_dfll_pwm_remove,
+};
+
+module_platform_driver(tegra_dfll_pwm_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("NVIDIA Corporation");
+MODULE_ALIAS("platform:tegra-dfll-pwm");
diff --git a/include/soc/tegra/pwm-tegra-dfll.h b/include/soc/tegra/pwm-tegra-dfll.h
new file mode 100644
index 0000000..d34c6e8
--- /dev/null
+++ b/include/soc/tegra/pwm-tegra-dfll.h
@@ -0,0 +1,27 @@
+/*
+ * Copyright (C) 2016 NVIDIA Corporation
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef __SOC_TEGRA_PWM_TEGRA_DFLL_H__
+#define __SOC_TEGRA_PWM_TEGRA_DFLL_H__
+
+#ifdef CONFIG_PWM_TEGRA_DFLL
+int tegra_dfll_pwm_output_enable(void);
+int tegra_dfll_pwm_output_disable(void);
+#else
+static inline int tegra_dfll_pwm_output_enable(void)
+{
+ return -ENOTSUPP;
+}
+
+static inline int tegra_dfll_pwm_output_disable(void)
+{
+ return -ENOTSUPP;
+}
+#endif
+
+#endif /* __SOC_TEGRA_PWM_TEGRA_DFLL_H__ */
--
2.8.1

2016-04-22 10:32:32

by Penny Chiu

[permalink] [raw]
Subject: [PATCH 04/11] clk: tegra: Add Tegra210 support in DFLL driver

Add Tegra210 support and related CVB table in tegra124 DFLL driver,
and also update the binding document.

Signed-off-by: Penny Chiu <[email protected]>
---
.../bindings/clock/nvidia,tegra124-dfll.txt | 4 +-
drivers/clk/tegra/Makefile | 4 +-
drivers/clk/tegra/clk-tegra124-dfll-fcpu.c | 55 ++++++++++++++++++++++
3 files changed, 61 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt b/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt
index 84080a8..42a1fe6 100644
--- a/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt
+++ b/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt
@@ -11,7 +11,9 @@ communicating with an off-chip PMIC either via an I2C bus or via PWM signals.
Currently only the I2C mode is supported by these bindings.

Required properties:
-- compatible : should be "nvidia,tegra124-dfll"
+- compatible : should be one of following:
+ - "nvidia,tegra124-dfll" for the Tegra124 SoC
+ - "nvidia,tegra210-dfll" for the Tegra210 SoC
- reg : Defines the following set of registers, in the order listed:
- registers for the DFLL control logic.
- registers for the I2C output logic.
diff --git a/drivers/clk/tegra/Makefile b/drivers/clk/tegra/Makefile
index 97984c5..9b8e9de 100644
--- a/drivers/clk/tegra/Makefile
+++ b/drivers/clk/tegra/Makefile
@@ -17,7 +17,9 @@ 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
+ifneq ($(filter y, $(CONFIG_ARCH_TEGRA_124_SOC) $(CONFIG_ARCH_TEGRA_210_SOC)),)
+obj-y += clk-tegra124-dfll-fcpu.o
+endif
obj-$(CONFIG_ARCH_TEGRA_132_SOC) += clk-tegra124.o
obj-y += cvb.o
obj-$(CONFIG_ARCH_TEGRA_210_SOC) += clk-tegra210.o
diff --git a/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c b/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c
index b577bc6..6b3316c 100644
--- a/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c
+++ b/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c
@@ -44,6 +44,11 @@ static const unsigned long tegra124_cpu_max_freq_table[] = {
[3] = 2524500000UL,
};

+static const unsigned long tegra210_cpu_max_freq_table[] = {
+ [0] = 1912500000UL,
+ [1] = 1912500000UL,
+};
+
static const struct cvb_table tegra124_cpu_cvb_tables[] = {
{
.speedo_id = -1,
@@ -87,6 +92,45 @@ static const struct cvb_table tegra124_cpu_cvb_tables[] = {
},
};

+static const struct cvb_table tegra210_cpu_cvb_tables[] = {
+ {
+ .speedo_id = -1,
+ .process_id = -1,
+ .min_millivolts = 850,
+ .max_millivolts = 1170,
+ .speedo_scale = 100,
+ .voltage_scale = 1000,
+ .cvb_table = {
+ {51000000UL, {1007452, -23865, 370} },
+ {102000000UL, {1007452, -23865, 370} },
+ {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} },
+ {0, { 0, 0, 0} },
+ },
+ .cpu_dfll_data = {
+ .tune0_low = 0xffead0ff,
+ .tune0_high = 0xffead0ff,
+ .tune1 = 0x20091d9,
+ }
+ },
+};
+
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),
@@ -94,11 +138,22 @@ static const struct dfll_fcpu_data tegra124_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
+ },
{ },
};
MODULE_DEVICE_TABLE(of, tegra124_dfll_fcpu_of_match);
--
2.8.1

2016-04-22 10:34:37

by Penny Chiu

[permalink] [raw]
Subject: [PATCH 06/11] clk: tegra: dfll: Add PWM inferface

The DFLL module can autonomously change the supply voltage by
communicating with an off-chip PMIC via either I2C or PWM signals.

Original driver only supports I2C interface, this patch adds PWM
interface support.

Signed-off-by: Penny Chiu <[email protected]>
---
.../bindings/clock/nvidia,tegra124-dfll.txt | 8 +-
drivers/clk/tegra/clk-dfll.c | 399 +++++++++++++++------
2 files changed, 299 insertions(+), 108 deletions(-)

diff --git a/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt b/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt
index 42a1fe6..a3e350e 100644
--- a/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt
+++ b/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt
@@ -8,7 +8,6 @@ the fast CPU cluster. It consists of a free-running voltage controlled
oscillator connected to the CPU voltage rail (VDD_CPU), and a closed loop
control module that will automatically adjust the VDD_CPU voltage by
communicating with an off-chip PMIC either via an I2C bus or via PWM signals.
-Currently only the I2C mode is supported by these bindings.

Required properties:
- compatible : should be one of following:
@@ -36,6 +35,10 @@ Required properties:
hardware will start controlling. The regulator will be queried for
the I2C register, control values and supported voltages.

+Optional properties:
+- nvidia,pwm-to-pmic: Specify DFLL PMU interface as PWM. If the property
+ is not defined, default PMU interface is I2C.
+
Reguired properties for build voltage table:
- nvidia,align-step-uv: Step uV of regulator for CPU voltage rail. This
value will be applied when building frequency-voltage table to ensure
@@ -58,6 +61,9 @@ Optional properties for the control loop parameters:
Required properties for I2C mode:
- nvidia,i2c-fs-rate: I2C transfer rate, if using full speed mode.

+Required properties for PWM mode:
+- nvidia,init-uv: Initialized CPU voltage before DFLL is not ready yet.
+
Example:

clock@0,70110000 {
diff --git a/drivers/clk/tegra/clk-dfll.c b/drivers/clk/tegra/clk-dfll.c
index a279467..56829c6 100644
--- a/drivers/clk/tegra/clk-dfll.c
+++ b/drivers/clk/tegra/clk-dfll.c
@@ -53,6 +53,7 @@
#include <linux/regulator/consumer.h>
#include <linux/reset.h>
#include <linux/seq_file.h>
+#include <soc/tegra/pwm-tegra-dfll.h>

#include "clk-dfll.h"

@@ -242,6 +243,11 @@ 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
@@ -293,6 +299,7 @@ struct tegra_dfll {
u32 ci;
u32 cg;
bool cg_scale;
+ u32 reg_init_uV;

/* I2C interface parameters */
u32 i2c_fs_rate;
@@ -300,9 +307,12 @@ 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;
+ unsigned lut[MAX_DFLL_VOLTAGES];
+ int lut_size;
u8 lut_min, lut_max, lut_safe;
+
+ /* PWM interface */
+ enum tegra_dfll_pmu_if pmu_if;
};

#define clk_hw_to_dfll(_hw) container_of(_hw, struct tegra_dfll, dfll_clk_hw)
@@ -326,7 +336,6 @@ static inline u32 dfll_readl(struct tegra_dfll *td, u32 offs)

static inline void dfll_writel(struct tegra_dfll *td, u32 val, u32 offs)
{
- WARN_ON(offs >= DFLL_I2C_CFG);
__raw_writel(val, td->base + offs);
}

@@ -538,7 +547,7 @@ static void dfll_load_i2c_lut(struct tegra_dfll *td)
lut_index = i;

val = regulator_list_hardware_vsel(td->vdd_reg,
- td->i2c_lut[lut_index]);
+ td->lut[lut_index]);
__raw_writel(val, td->lut_base + i * 4);
}

@@ -581,36 +590,135 @@ static void dfll_init_i2c_if(struct tegra_dfll *td)
dfll_i2c_wmb(td);
}

+/*
+ * DFLL-to-PWM controller interface
+ */
+
/**
- * dfll_init_out_if - prepare DFLL-to-PMIC interface
+ * dfll_set_force_output_value - set fixed value for force output
* @td: DFLL instance
+ * @out_val: value to force output
*
- * During DFLL driver initialization or resume from context loss,
- * disable the I2C command output to the PMIC, set safe voltage and
- * output limits, and disable and clear limit interrupts.
+ * Set the fixec value for force output, DFLL will output this value when
+ * force output is enabled.
*/
-static void dfll_init_out_if(struct tegra_dfll *td)
+static u32 dfll_set_force_output_value(struct tegra_dfll *td, u8 out_val)
+{
+ u32 val = dfll_readl(td, DFLL_OUTPUT_FORCE);
+
+ 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;

- td->lut_min = 0;
- td->lut_max = td->i2c_lut_size - 1;
- td->lut_safe = td->lut_min + 1;
+ if (out_sel > OUT_MASK)
+ return -EINVAL;

- 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);
+ 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);
+ }

- 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);
+ return 0;
+}
+
+/**
+ * dfll_init_out_if - prepare DFLL-to-PMIC interface
+ * @td: DFLL instance
+ *
+ * During DFLL driver initialization or resume from context loss,
+ * it needs to set safe voltage and output limits, and disable and
+ * clear limit interrupts. For PWM interface, it needs to set
+ * initial voltage for force output to avoid CPU voltage glitch
+ * during DFLL is from open to closed loop transition.
+ */
+static void dfll_init_out_if(struct tegra_dfll *td)
+{
+ u32 val = 0;

- 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;
+ int vmin = regulator_list_voltage(td->vdd_reg, 0);
+
+ td->lut_min = td->lut[0];
+ td->lut_max = td->lut[td->lut_size - 1];
+ td->lut_safe = td->lut_min + 1;
+
+ 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_writel(td, 0, DFLL_INTR_EN);
+ dfll_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 {
+ td->lut_min = 0;
+ td->lut_max = td->lut_size - 1;
+ td->lut_safe = td->lut_min + 1;
+
+ 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_i2c_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);
+ }
}

/*
@@ -643,9 +751,9 @@ static int find_lut_index_for_rate(struct tegra_dfll *td, unsigned long rate)

rcu_read_unlock();

- for (i = 0; i < td->i2c_lut_size; i++) {
- if ((regulator_list_voltage(td->vdd_reg, td->i2c_lut[i]) /
- td->soc->alignment) == align_volt)
+ for (i = 0; i < td->lut_size; i++) {
+ if ((regulator_list_voltage(td->vdd_reg, td->lut[i]) /
+ td->soc->alignment) == align_volt)
return i;
}

@@ -723,7 +831,12 @@ static void dfll_set_frequency_request(struct tegra_dfll *td,
int force_val;
int coef = 128; /* FIXME: td->cg_scale? */;

- force_val = (req->lut_index - td->lut_safe) * coef / td->cg;
+ if (td->pmu_if == TEGRA_DFLL_PMU_PWM)
+ force_val = td->lut[req->lut_index] - td->lut[td->lut_safe];
+ else
+ force_val = req->lut_index - td->lut_safe;
+
+ force_val = force_val * coef / td->cg;
force_val = clamp(force_val, FORCE_MIN, FORCE_MAX);

val |= req->mult_bits << DFLL_FREQ_REQ_MULT_SHIFT;
@@ -867,9 +980,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)
+ tegra_dfll_pwm_output_enable();
+ 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:
@@ -889,11 +1007,17 @@ static int dfll_lock(struct tegra_dfll *td)
*/
static int dfll_unlock(struct tegra_dfll *td)
{
+
switch (td->mode) {
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)
+ tegra_dfll_pwm_output_disable();
+ else
+ dfll_i2c_set_output_enabled(td, false);
+
return 0;

case DFLL_OPEN_LOOP:
@@ -1160,7 +1284,8 @@ static int attr_registers_show(struct seq_file *s, void *data)

seq_puts(s, "CONTROL REGISTERS:\n");
for (offs = 0; offs <= DFLL_MONITOR_DATA; offs += 4) {
- if (offs == DFLL_OUTPUT_CFG)
+ if (td->pmu_if == TEGRA_DFLL_PMU_I2C &&
+ offs == DFLL_OUTPUT_CFG)
val = dfll_i2c_readl(td, offs);
else
val = dfll_readl(td, offs);
@@ -1168,22 +1293,33 @@ static int attr_registers_show(struct seq_file *s, void *data)
}

seq_puts(s, "\nI2C and INTR REGISTERS:\n");
- for (offs = DFLL_I2C_CFG; offs <= DFLL_I2C_STS; offs += 4)
- seq_printf(s, "[0x%02x] = 0x%08x\n", offs,
- dfll_i2c_readl(td, offs));
- for (offs = DFLL_INTR_STS; offs <= DFLL_INTR_EN; offs += 4)
- seq_printf(s, "[0x%02x] = 0x%08x\n", offs,
- dfll_i2c_readl(td, offs));
+ for (offs = DFLL_I2C_CFG; offs <= DFLL_I2C_STS; offs += 4) {
+ if (td->pmu_if == TEGRA_DFLL_PMU_I2C)
+ val = dfll_i2c_readl(td, offs);
+ else
+ val = dfll_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_printf(s, "[0x%02x] = 0x%08x\n", offs, val);
+ }
+ for (offs = DFLL_INTR_STS; offs <= DFLL_INTR_EN; offs += 4) {
+ if (td->pmu_if == TEGRA_DFLL_PMU_I2C)
+ val = dfll_i2c_readl(td, offs);
+ else
+ val = dfll_readl(td, offs);
+ seq_printf(s, "[0x%02x] = 0x%08x\n", offs, val);
+ }

- 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;
}
@@ -1385,9 +1521,11 @@ static int find_vdd_map_entry_exact(struct tegra_dfll *td, int uV)

align_volt = uV / td->soc->alignment;
n_voltages = regulator_count_voltages(td->vdd_reg);
+
for (i = 0; i < n_voltages; i++) {
reg_volt = regulator_list_voltage(td->vdd_reg, i) /
td->soc->alignment;
+
if (reg_volt < 0)
break;

@@ -1409,9 +1547,11 @@ static int find_vdd_map_entry_min(struct tegra_dfll *td, int uV)

align_volt = uV / td->soc->alignment;
n_voltages = regulator_count_voltages(td->vdd_reg);
+
for (i = 0; i < n_voltages; i++) {
reg_volt = regulator_list_voltage(td->vdd_reg, i) /
td->soc->alignment;
+
if (reg_volt < 0)
break;

@@ -1424,7 +1564,7 @@ static int find_vdd_map_entry_min(struct tegra_dfll *td, int uV)
}

/**
- * dfll_build_i2c_lut - build the I2C voltage register lookup table
+ * dfll_build_lut - build the I2C and PWM voltage register lookup table
* @td: DFLL instance
*
* The DFLL hardware has 33 bytes of look-up table RAM that must be filled with
@@ -1433,12 +1573,12 @@ static int find_vdd_map_entry_min(struct tegra_dfll *td, int uV)
* the soc-specific platform driver (td->soc->opp_dev) and the PMIC
* register-to-voltage mapping queried from the regulator framework.
*
- * On success, fills in td->i2c_lut and returns 0, or -err on failure.
+ * On success, fills in td->lut and returns 0, or -err on failure.
*/
-static int dfll_build_i2c_lut(struct tegra_dfll *td)
+static int dfll_build_lut(struct tegra_dfll *td)
{
int ret = -EINVAL;
- int j, v, v_max, v_opp;
+ int j, v, v_max, v_opp, v_min_align;
int selector;
unsigned long rate;
struct dev_pm_opp *opp;
@@ -1454,19 +1594,21 @@ static int dfll_build_i2c_lut(struct tegra_dfll *td)
}
v_max = dev_pm_opp_get_voltage(opp);

+ v_min_align = DIV_ROUND_UP(td->soc->min_millivolts * 1000,
+ td->soc->alignment) * td->soc->alignment;
+
v = td->soc->min_millivolts * 1000;
lut = find_vdd_map_entry_exact(td, v);
if (lut < 0)
goto out;
- td->i2c_lut[0] = lut;
+ td->lut[0] = lut;

for (j = 1, rate = 0; ; rate++) {
opp = dev_pm_opp_find_freq_ceil(td->soc->dev, &rate);
if (IS_ERR(opp))
break;
v_opp = dev_pm_opp_get_voltage(opp);
-
- if (v_opp <= td->soc->min_millivolts * 1000)
+ if (v_opp <= v_min_align)
td->dvco_rate_min = dev_pm_opp_get_freq(opp);

for (;;) {
@@ -1477,21 +1619,21 @@ static int dfll_build_i2c_lut(struct tegra_dfll *td)
selector = find_vdd_map_entry_min(td, v);
if (selector < 0)
goto out;
- if (selector != td->i2c_lut[j - 1])
- td->i2c_lut[j++] = selector;
+ if (selector != td->lut[j - 1])
+ td->lut[j++] = selector;
}

v = (j == MAX_DFLL_VOLTAGES - 1) ? v_max : v_opp;
selector = find_vdd_map_entry_exact(td, v);
if (selector < 0)
goto out;
- if (selector != td->i2c_lut[j - 1])
- td->i2c_lut[j++] = selector;
+ if (selector != td->lut[j - 1])
+ td->lut[j++] = selector;

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",
@@ -1563,12 +1705,6 @@ 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 ret;
- }
-
return 0;
}

@@ -1601,6 +1737,61 @@ static int dfll_fetch_common_params(struct tegra_dfll *td)
return ok ? 0 : -EINVAL;
}

+/**
+ * dfll_dt_parse - read necessary parameters from the device tree
+ * @td: DFLL instance
+ *
+ * Read necessary DT parameters based on I2C or PWM operation.
+ * Returns 0 on success or -EINVAL on any failure.
+ */
+static int dfll_dt_parse(struct tegra_dfll *td)
+{
+ int ret;
+ struct device_node *dn = td->dev->of_node;
+
+ ret = dfll_fetch_common_params(td);
+ if (ret) {
+ dev_err(td->dev, "couldn't parse device tree parameters\n");
+ return ret;
+ }
+
+ 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);
+ }
+
+ /* I2C or PWM interface */
+ if (of_property_read_bool(dn, "nvidia,pwm-to-pmic")) {
+ td->pmu_if = TEGRA_DFLL_PMU_PWM;
+ dev_info(td->dev, "config PMU interface as PWM\n");
+
+ ret = of_property_read_u32(dn, "nvidia,init-uv",
+ &td->reg_init_uV);
+ if (ret) {
+ dev_err(td->dev, "couldn't get initialized voltage\n");
+ return ret;
+ }
+ } else {
+ td->pmu_if = TEGRA_DFLL_PMU_I2C;
+ dev_info(td->dev, "config PMU interface as I2C\n");
+
+ ret = dfll_fetch_i2c_params(td);
+ if (ret) {
+ dev_err(td->dev, "couldn't get I2C parameters\n");
+ return ret;
+ }
+ }
+
+ ret = dfll_build_lut(td);
+ if (ret) {
+ dev_err(td->dev, "couldn't build LUT\n");
+ return ret;
+ }
+
+ return 0;
+}
+
/*
* API exported to per-SoC platform drivers
*/
@@ -1634,10 +1825,10 @@ 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);
+ ret = dfll_dt_parse(td);
+ if (ret) {
+ dev_err(td->dev, "parse device-tree failed\n");
+ return ret;
}

td->dvco_rst = devm_reset_control_get(td->dev, "dvco");
@@ -1646,16 +1837,6 @@ int tegra_dfll_register(struct platform_device *pdev,
return PTR_ERR(td->dvco_rst);
}

- ret = dfll_fetch_common_params(td);
- if (ret) {
- dev_err(td->dev, "couldn't parse device tree parameters\n");
- return ret;
- }
-
- ret = dfll_fetch_i2c_params(td);
- if (ret)
- return ret;
-
mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!mem) {
dev_err(td->dev, "no control register resource\n");
@@ -1668,43 +1849,47 @@ int tegra_dfll_register(struct platform_device *pdev,
return -ENODEV;
}

- mem = platform_get_resource(pdev, IORESOURCE_MEM, 1);
- if (!mem) {
- dev_err(td->dev, "no i2c_base resource\n");
- return -ENODEV;
- }
+ if (td->pmu_if == TEGRA_DFLL_PMU_I2C) {
+ mem = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+ if (!mem) {
+ dev_err(td->dev, "no i2c_base resource\n");
+ return -ENODEV;
+ }

- td->i2c_base = devm_ioremap(td->dev, mem->start, resource_size(mem));
- if (!td->i2c_base) {
- dev_err(td->dev, "couldn't ioremap i2c_base resource\n");
- return -ENODEV;
- }
+ td->i2c_base = devm_ioremap(td->dev, mem->start,
+ resource_size(mem));
+ if (!td->i2c_base) {
+ dev_err(td->dev, "couldn't ioremap i2c_base resource\n");
+ return -ENODEV;
+ }

- mem = platform_get_resource(pdev, IORESOURCE_MEM, 2);
- if (!mem) {
- dev_err(td->dev, "no i2c_controller_base resource\n");
- return -ENODEV;
- }
+ mem = platform_get_resource(pdev, IORESOURCE_MEM, 2);
+ if (!mem) {
+ dev_err(td->dev, "no i2c_controller_base resource\n");
+ return -ENODEV;
+ }

- td->i2c_controller_base = devm_ioremap(td->dev, mem->start,
+ td->i2c_controller_base = devm_ioremap(td->dev, mem->start,
resource_size(mem));
- if (!td->i2c_controller_base) {
- dev_err(td->dev,
- "couldn't ioremap i2c_controller_base resource\n");
- return -ENODEV;
- }
+ if (!td->i2c_controller_base) {
+ dev_err(td->dev,
+ "couldn't ioremap i2c_controller_base resource\n");
+ return -ENODEV;
+ }

- mem = platform_get_resource(pdev, IORESOURCE_MEM, 3);
- if (!mem) {
- dev_err(td->dev, "no lut_base resource\n");
- return -ENODEV;
- }
+ mem = platform_get_resource(pdev, IORESOURCE_MEM, 3);
+ if (!mem) {
+ dev_err(td->dev, "no lut_base resource\n");
+ return -ENODEV;
+ }

- td->lut_base = devm_ioremap(td->dev, mem->start, resource_size(mem));
- if (!td->lut_base) {
- dev_err(td->dev,
- "couldn't ioremap lut_base resource\n");
- return -ENODEV;
+ td->lut_base = devm_ioremap(td->dev, mem->start,
+ resource_size(mem));
+ if (!td->lut_base) {
+ dev_err(td->dev,
+ "couldn't ioremap lut_base resource\n");
+ return -ENODEV;
+ }
}

ret = dfll_init_clks(td);
--
2.8.1

2016-04-22 11:00:21

by Viresh Kumar

[permalink] [raw]
Subject: Re: [PATCH 07/11] cpufreq: tegra124: Add Tegra210 support

On 22-04-16, 18:31, Penny Chiu wrote:
> Add tegra210 support in tegra124 cpufreq driver.
>
> Signed-off-by: Penny Chiu <[email protected]>
> ---
> drivers/cpufreq/tegra124-cpufreq.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)

Acked-by: Viresh Kumar <[email protected]>

--
viresh

2016-04-22 11:44:30

by Jon Hunter

[permalink] [raw]
Subject: Re: [PATCH 10/11] arm64: tegra: Add clock properties on cpu0 for Tegra210

Hi Penny,

On 22/04/16 11:31, Penny Chiu wrote:
> Add clocks, clock-names, and clock-latency into cpu0 node.
> These properties will be used by cpufreq driver.
>
> Signed-off-by: Penny Chiu <[email protected]>
> ---
> arch/arm64/boot/dts/nvidia/tegra210.dtsi | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/nvidia/tegra210.dtsi b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
> index 204d9cd..1a85857 100644
> --- a/arch/arm64/boot/dts/nvidia/tegra210.dtsi
> +++ b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
> @@ -782,6 +782,12 @@
> 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_OUT4>,
> + <&dfll>;
> + clock-names = "cpu_g", "pll_x", "pll_p", "dfll";
> + clock-latency = <300000>;
> };
>
> cpu@1 {
>

Can you include a patch with this series to update the binding
documentation for the nvidia,tegra124-cpufreq.txt? I think that although
there is no specific nvidia,tegra210-cpufreq compatible string the
documentation should state that both tegra124 and tegra210 are supported
so it is clear.

Also I see the above binding no longer includes the "cpu_lp" for
tegra210 which I understand we don't use here. However, the binding
documentation should reflect this. Having said that, looking at the
driver it appears the "cpu_lp" clock is not even used for tegra124. I
wonder if we should drop this from the binding documentation and the
tegra124.dtsi altogether?

What about the "vdd-cpu-supply" property? Don't we need this?

Cheers
Jon

2016-04-22 12:55:22

by Thierry Reding

[permalink] [raw]
Subject: Re: [PATCH 05/11] pwm: tegra-dfll: Add driver for Tegra DFLL PWM controller

On Fri, Apr 22, 2016 at 06:31:05PM +0800, Penny Chiu wrote:
> Tegra DFLL IP block controls off-chip PMIC via I2C bus or PWM
> signals. This driver exposes DFLL as a PWM controller to generate
> PWM signals to PWM regulator.
>
> Tegra DFLL HW changes regulator voltage by adjusting PWM signals
> duty cycle automatically based on required DVCO frequency, so PWM
> regulator client doesn't need to change voltage by SW.
>
> In this driver, it only configs proper PWM rate in the driver
> initialization, and provides two APIs for clients to determine when
> to enable and disable generating PWM signals by setting related
> pinmux tristate.
>
> Signed-off-by: Penny Chiu <[email protected]>
> ---
> .../bindings/pwm/nvidia,tegra-dfll-pwm.txt | 48 +++
> drivers/pwm/Kconfig | 10 +
> drivers/pwm/Makefile | 1 +
> drivers/pwm/pwm-tegra-dfll.c | 322 +++++++++++++++++++++
> include/soc/tegra/pwm-tegra-dfll.h | 27 ++
> 5 files changed, 408 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/pwm/nvidia,tegra-dfll-pwm.txt
> create mode 100644 drivers/pwm/pwm-tegra-dfll.c
> create mode 100644 include/soc/tegra/pwm-tegra-dfll.h
>
> diff --git a/Documentation/devicetree/bindings/pwm/nvidia,tegra-dfll-pwm.txt b/Documentation/devicetree/bindings/pwm/nvidia,tegra-dfll-pwm.txt
> new file mode 100644
> index 0000000..bd0d247
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pwm/nvidia,tegra-dfll-pwm.txt
> @@ -0,0 +1,48 @@
> +Tegra SoC DFLL PWM controller

Does this exist on Tegra124 as well? The reason why I ask is that the
file name should usually contain the name of the first SoC generation
that contains an IP block described by the binding. I'm not sure we've
ever clarified whether it is supposed to be the first chip where a
compatible IP block was introduced or where we've made use of it in the
Linux kernel. I think it's usually the latter, but technically the
former would be more correct (since DT describes hardware, not on OS
implementation for said hardware).

Stephen, we have in the past used tegra124 in names, even if the IP was
already included in tegra114, but we never supported it on Tegra114 and
hence couldn't even verify that the binding was valid. Any preference as
to the name in this particular case?

> +Required properties:
> +- compatible: For Tegra210, must contain "nvidia,tegra210-dfll-pwm".
> +- reg: physical base address and length of the controller's registers
> +- #pwm-cells: should be 2. See pwm.txt in this directory for a description of
> + the cells format.
> +- clock-names: Must include the "ref" entry.
> +- clocks: Must contain one entry, for the DFLL closed loop reference clock.
> + See ../clocks/clock-bindings.txt for details.

I think it would be more idiomatic to describe this the other way
around. That is, describe in the clock-names property what each entry
means, then the clocks property can simply say must contain one entry
for each entry in clock-names.

> +- pwm-regulator: phandle to PWM regulator for using this PWM controller.

Why do we need the back link? I'd expect the regulator to be simply a
user of the PWM. Why do we need to access the regulator from the PWM?
I'm not sure (yet) how you've solved this in the driver, but it seems
like you've just created a circular dependency, which is usually not
good at all.

> +- pinctrl-names: Must contain two entries to enable and disable PWM signals
> + output pinmux states.
> +- pinctrl-0: pinmux state to enable PWM signals output
> +- pinctrl-1: pinmux state to disable PWM signals output

Here again I think you need to specify what names are expected in the
pinctrl-names property and then pinctrl-N should be described as
representing the phandle to the pinmux state for the Nth entry in
pinctrl-names.

> +
> +Example:
> +
> + 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>;
> + };
> + };
> + };
> +
> + pwm_dfll: pwm@70110000 {
> + compatible = "nvidia,tegra210-dfll-pwm";

The TRM denotes this block as DVFS and in fact on Tegra124 we've added
an entry that exposes this as clock@70110000 and with a different
compatible: nvidia,tegra124-dfll.

I'd like to understand how this works. The DFLL binding suggests that
this PWM mode is supported on Tegra124 as well, so I'm presuming that
Tegra210 has pretty much the same IP and supports both I2C and PWM
modes.

Does this binding then effectively replace the nvidia,tegra124-dfll one
and uses the IP block in some different mode?

> + reg = <0x0 0x70110000 0x0 0x400>;
> + clocks = <&tegra_car TEGRA210_CLK_DFLL_REF>;
> + clock-names = "ref";
> + #pwm-cells = <2>;
> + pwm-regulator = <&cpu_ovr_reg>;
> +
> + pinctrl-names = "dvfs_pwm_enable", "dvfs_pwm_disable";

"enable" and "disable" should be enough, since these are scoped within
the pwm_dfll node anyway.

> diff --git a/drivers/pwm/pwm-tegra-dfll.c b/drivers/pwm/pwm-tegra-dfll.c
> new file mode 100644
> index 0000000..74d6b97
> --- /dev/null
> +++ b/drivers/pwm/pwm-tegra-dfll.c
> @@ -0,0 +1,322 @@
> +/*
> + * drivers/pwm/pwm-tegra-dfll.c
> + *
> + * Tegra DFLL PWM controller driver
> + *
> + * Copyright (c) 2016, NVIDIA Corporation.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
> + * more details.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/pwm.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/pinctrl/consumer.h>
> +#include <soc/tegra/pwm-tegra-dfll.h>
> +
> +/* DFLL_CTRL: DFLL control register */
> +#define DFLL_CTRL 0x00
> +
> +/* DFLL_OUTPUT_CFG: closed loop mode control registers */
> +#define DFLL_OUTPUT_CFG 0x20
> +#define OUT_MASK 0x3f
> +#define DFLL_OUTPUT_CFG_PWM_DELTA (0x1 << 7)
> +#define DFLL_OUTPUT_CFG_PWM_ENABLE (0x1 << 6)

If these are simple bits you should leave away the 0x prefix. 0x prefix
suggests that it is one particular value of a multi-bit field.

> +#define DFLL_OUTPUT_CFG_PWM_DIV_SHIFT 0
> +#define DFLL_OUTPUT_CFG_PWM_DIV_MASK \
> + (OUT_MASK << DFLL_OUTPUT_CFG_PWM_DIV_SHIFT)
> +
> +/* MAX_DFLL_VOLTAGES: number of LUT entries in the DFLL IP block */
> +#define DFLL_MAX_VOLTAGES 33
> +
> +#define DFLL_OF_PWM_PERIOD_CELL 1
> +
> +/**
> + * struct tegra_dfll_pwm_chip - DFLL PWM controller data
> + * @pwm_pin: pinmux for PWM signals output
> + * @pwm_enable_state: enabled states of pinmux for PWM signals output
> + * @pwm_disable_state: disabled states of pinmux for PWM signals output
> + * @mmio_base: mmio base for access DFLL registers
> + * @ref_clk: referenced source clock
> + * @pwm_rate: PWM rate for DFLL PWM output config register
> + */
> +struct tegra_dfll_pwm_chip {
> + struct pwm_chip chip;
> + struct device *dev;

struct pwm_chip already has a struct device *dev member, perhaps reuse
that instead of keeping another copy?

> +
> + struct pinctrl *pwm_pin;
> + struct pinctrl_state *pwm_enable_state;
> + struct pinctrl_state *pwm_disable_state;
> +
> + void __iomem *mmio_base;
> + struct clk *ref_clk;
> +
> + unsigned long ref_rate;
> + unsigned long pwm_rate;
> +};
> +
> +static struct tegra_dfll_pwm_chip *tdpc;

Try to avoid this global variable, please.

> +
> +/*
> + * Register accessors
> + */
> +static inline u32 pwm_readl(u32 offs)

I prefer the offset to be unsigned int, otherwise it might be mistaken
for a register value (because of the explicit width). Also I find it
easier to read if offs here...

> +{
> + return __raw_readl(tdpc->mmio_base + offs);
> +}
> +
> +static inline void pwm_writel(u32 val, u32 offs)

and val here are spelled out as offset and value.

> +{
> + __raw_writel(val, tdpc->mmio_base + offs);

Any particular reason why you use __raw_*() accessors here?

> + pwm_readl(DFLL_CTRL);

Why is this required?

> +}
> +
> +static int tegra_dfll_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
> + int duty_ns, int period_ns)
> +{
> + return 0;
> +}

Huh? How come this doesn't need to be implemented?

> +static int tegra_dfll_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
> +{
> + u32 val;
> +
> + val = pwm_readl(DFLL_OUTPUT_CFG);
> + val |= DFLL_OUTPUT_CFG_PWM_ENABLE;
> + pwm_writel(val, DFLL_OUTPUT_CFG);
> +
> + dev_info(tdpc->dev, "DFLL_PWM is enabled\n");

dev_info() is a little strong here, don't you think?

> +
> + return 0;
> +}
> +
> +static void tegra_dfll_pwm_disable(struct pwm_chip *chip,
> + struct pwm_device *pwm)
> +{
> + u32 val;
> +
> + val = pwm_readl(DFLL_OUTPUT_CFG);
> + val &= ~DFLL_OUTPUT_CFG_PWM_ENABLE;
> + pwm_writel(val, DFLL_OUTPUT_CFG);
> +
> + dev_info(tdpc->dev, "DFLL_PWM is disabled\n");

Same here. This is debugging information and should at most be dev_dbg()
if anything at all.

> +
> + return 0;
> +}
> +
> +/**
> + * tegra_dfll_pwm_output_enable - enable DFLL PWM signals output
> + *
> + * Enable DFLL PWM signals output by changing related pinmux state
> + */
> +int tegra_dfll_pwm_output_enable(void)
> +{
> + int ret;
> +
> + ret = pinctrl_select_state(tdpc->pwm_pin, tdpc->pwm_enable_state);
> + if (ret < 0) {
> + dev_err(tdpc->dev, "setting enable state failed\n");
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(tegra_dfll_pwm_output_enable);
> +
> +/**
> + * tegra_dfll_pwm_output_disable - disable DFLL PWM signals output
> + *
> + * Disable DFLL PWM signals output by changing related pinmux state
> + */
> +int tegra_dfll_pwm_output_disable(void)
> +{
> + int ret;
> +
> + ret = pinctrl_select_state(tdpc->pwm_pin, tdpc->pwm_disable_state);
> + if (ret < 0) {
> + dev_err(tdpc->dev, "setting enable state failed\n");
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(tegra_dfll_pwm_output_disable);

I don't get why you need to export these symbols as custom API. Why not
simply make this part of the ->enable() and ->disable() implementations?

> +
> +static const struct pwm_ops tegra_dfll_pwm_ops = {
> + .config = tegra_dfll_pwm_config,
> + .enable = tegra_dfll_pwm_enable,
> + .disable = tegra_dfll_pwm_disable,
> + .owner = THIS_MODULE,
> +};
> +
> +/**
> + * dt_parse_pwm_regulator - parse PWM regulator from device-tree
> + *
> + * Parse DFLL PWM controller client to get and calcluate initialized
> + * DFLL PWM rate.
> + */
> +static int dt_parse_pwm_regulator(void)
> +{
> + struct device_node *r_dn =
> + of_parse_phandle(tdpc->dev->of_node, "pwm-regulator", 0);
> + struct of_phandle_args args;
> + unsigned long val;
> + int ret;
> +
> + /* pwm regulator device */
> + if (!r_dn) {
> + dev_err(tdpc->dev, "DT: missing pwm-regulator property\n");
> + return -EINVAL;
> + }
> +
> + ret = of_parse_phandle_with_args(r_dn, "pwms", "#pwm-cells", 0, &args);
> + if (ret) {
> + dev_err(tdpc->dev, "DT: failed to parse pwms property\n");
> + return -EINVAL;
> + }
> + of_node_put(args.np);

That's completely backwards, isn't it? Isn't the regulator using this
very PWM to control the voltage? Would args.np not always be the same as
tdpc->dev->of_node?

> +
> + if (args.args_count <= DFLL_OF_PWM_PERIOD_CELL) {
> + dev_err(tdpc->dev, "DT: low #pwm-cells %d\n", args.args_count);

You may want to provide at least some information about how many cells
you expect.

> + return -EINVAL;
> + }
> +
> + /* convert pwm period in ns to DFLL pwm rate in Hz */
> + val = args.args[DFLL_OF_PWM_PERIOD_CELL];
> + val = (NSEC_PER_SEC / val) * (DFLL_MAX_VOLTAGES - 1);
> + tdpc->pwm_rate = val;
> + dev_info(tdpc->dev, "DFLL pwm-rate: %lu\n", val);

Again, this is much too verbose.

> +
> + return 0;
> +}
> +
> +/**
> + * tegra_dfll_pwm_init - init Tegra DFLL PWM controller
> + *
> + * Calculate the DIV value and write into DFLL register
> + */
> +static int tegra_dfll_pwm_init(void)
> +{
> + u32 div, val;
> +
> + pwm_writel(0, DFLL_OUTPUT_CFG);

Why write it with 0 if you're going to overwrite it below again?

> +
> + div = DIV_ROUND_UP(tdpc->ref_rate, tdpc->pwm_rate);
> + val = (div << DFLL_OUTPUT_CFG_PWM_DIV_SHIFT) &
> + DFLL_OUTPUT_CFG_PWM_DIV_MASK;
> + pwm_writel(val, DFLL_OUTPUT_CFG);
> +
> + return 0;
> +}
> +
> +static int tegra_dfll_pwm_probe(struct platform_device *pdev)
> +{
> + struct resource *res;
> + int ret;
> +
> + tdpc = devm_kzalloc(&pdev->dev, sizeof(*tdpc), GFP_KERNEL);
> + if (!tdpc)
> + return -ENOMEM;
> +
> + tdpc->dev = &pdev->dev;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + tdpc->mmio_base = devm_ioremap_resource(tdpc->dev, res);
> + if (IS_ERR(tdpc->mmio_base))
> + return PTR_ERR(tdpc->mmio_base);
> +
> + platform_set_drvdata(pdev, tdpc);
> +
> + tdpc->chip.dev = tdpc->dev;
> + tdpc->chip.ops = &tegra_dfll_pwm_ops;
> + tdpc->chip.base = -1;
> + tdpc->chip.npwm = 1;
> +
> + ret = pwmchip_add(&tdpc->chip);
> + if (ret < 0) {
> + dev_err(tdpc->dev, "pwmchip_add() failed: %d\n", ret);
> + return ret;
> + }

This should be the very last thing you do. pwmchip_add() will otherwise
make the PWM chip available to all users in the system and keep it
around even if any of the below fails. You could always remove it again
on failure, but then why go through the trouble of registering something
that you may fail to set up?

> +
> + tdpc->ref_clk = devm_clk_get(tdpc->dev, "ref");
> + if (IS_ERR(tdpc->ref_clk)) {
> + dev_err(tdpc->dev, "DT: missing ref clock\n");
> + return PTR_ERR(tdpc->ref_clk);
> + }
> +
> + tdpc->ref_rate = clk_get_rate(tdpc->ref_clk);
> +
> + tdpc->pwm_pin = devm_pinctrl_get(tdpc->dev);
> + if (IS_ERR(tdpc->pwm_pin)) {
> + dev_err(tdpc->dev, "DT: missing pinctrl device\n");
> + return PTR_ERR(tdpc->pwm_pin);
> + }
> +
> + tdpc->pwm_enable_state = pinctrl_lookup_state(tdpc->pwm_pin,
> + "dvfs_pwm_enable");
> + if (IS_ERR(tdpc->pwm_enable_state)) {
> + dev_err(tdpc->dev, "DT: missing pwm enabled state\n");
> + return PTR_ERR(tdpc->pwm_enable_state);
> + }
> +
> + tdpc->pwm_disable_state = pinctrl_lookup_state(tdpc->pwm_pin,
> + "dvfs_pwm_disable");
> + if (IS_ERR(tdpc->pwm_disable_state)) {
> + dev_err(tdpc->dev, "DT: missing pwm disabled state\n");
> + return PTR_ERR(tdpc->pwm_disable_state);
> + }
> +
> + ret = dt_parse_pwm_regulator();
> + if (ret < 0) {
> + dev_err(tdpc->dev, "failed to parse pwm regulator\n");
> + return ret;
> + }
> +
> + ret = tegra_dfll_pwm_init();
> + if (ret < 0) {
> + dev_err(tdpc->dev, "failed to init DFLL pwm\n");
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int tegra_dfll_pwm_remove(struct platform_device *pdev)
> +{
> + return pwmchip_remove(&tdpc->chip);
> +}
> +
> +static const struct of_device_id tegra_dfll_pwm_of_match[] = {
> + { .compatible = "nvidia,tegra210-dfll-pwm" },
> + { }
> +};
> +
> +MODULE_DEVICE_TABLE(of, tegra_dfll_pwm_of_match);
> +
> +static struct platform_driver tegra_dfll_pwm_driver = {
> + .driver = {
> + .name = "tegra-dfll-pwm",
> + .of_match_table = tegra_dfll_pwm_of_match,
> + },
> + .probe = tegra_dfll_pwm_probe,
> + .remove = tegra_dfll_pwm_remove,
> +};
> +
> +module_platform_driver(tegra_dfll_pwm_driver);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("NVIDIA Corporation");

You should put the name of a real person here. It's meant as contact
information if anyone wants to contact the author about issues with the
driver, so ideally it'd include an email address as well.

> +MODULE_ALIAS("platform:tegra-dfll-pwm");
> diff --git a/include/soc/tegra/pwm-tegra-dfll.h b/include/soc/tegra/pwm-tegra-dfll.h
> new file mode 100644
> index 0000000..d34c6e8
> --- /dev/null
> +++ b/include/soc/tegra/pwm-tegra-dfll.h
> @@ -0,0 +1,27 @@
> +/*
> + * Copyright (C) 2016 NVIDIA Corporation
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#ifndef __SOC_TEGRA_PWM_TEGRA_DFLL_H__
> +#define __SOC_TEGRA_PWM_TEGRA_DFLL_H__
> +
> +#ifdef CONFIG_PWM_TEGRA_DFLL
> +int tegra_dfll_pwm_output_enable(void);
> +int tegra_dfll_pwm_output_disable(void);
> +#else
> +static inline int tegra_dfll_pwm_output_enable(void)
> +{
> + return -ENOTSUPP;
> +}
> +
> +static inline int tegra_dfll_pwm_output_disable(void)
> +{
> + return -ENOTSUPP;
> +}
> +#endif
> +
> +#endif /* __SOC_TEGRA_PWM_TEGRA_DFLL_H__ */

I'd prefer to avoid this header if at all possible.

Thierry


Attachments:
(No filename) (16.72 kB)
signature.asc (819.00 B)
Download all attachments

2016-04-22 13:04:20

by Thierry Reding

[permalink] [raw]
Subject: Re: [PATCH 02/11] clk: tegra: dfll: Move SoC specific data into of_device_id

On Fri, Apr 22, 2016 at 06:31:02PM +0800, Penny Chiu wrote:
> Move all SoC specific fcpu data into of_device_id structure, and
> move SoC fcpu data assignments from init function to probe
> function.
>
> Signed-off-by: Penny Chiu <[email protected]>
> ---
> drivers/clk/tegra/clk-tegra124-dfll-fcpu.c | 51 ++++++++++++++++++++++--------
> 1 file changed, 37 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c b/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c
> index 5e5958e..b577bc6 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/module.h>
> +#include <linux/of_device.h>
> #include <linux/platform_device.h>
> #include <soc/tegra/fuse.h>
>
> @@ -28,8 +29,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,
> @@ -79,18 +87,39 @@ static const struct cvb_table tegra124_cpu_cvb_tables[] = {
> },
> };
>
> +static const struct dfll_fcpu_data tegra124_dfll_fcpu_data = {
> + .cpu_max_freq_table = tegra124_cpu_max_freq_table,
> + .cpu_max_freq_table_size = ARRAY_SIZE(tegra124_cpu_max_freq_table),
> + .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[] = {

There's no need to prefix this with tegra124_ since obviously the goal
is to make this a table that works at least on Tegra210 as well.

> + {
> + .compatible = "nvidia,tegra124-dfll",
> + .data = &tegra124_dfll_fcpu_data
> + },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, tegra124_dfll_fcpu_of_match);
> +
> static int tegra124_dfll_fcpu_probe(struct platform_device *pdev)
> {
> int process_id, speedo_id, speedo_value, ret;
> struct rail_alignment align;
> struct tegra_dfll_soc_data *soc;
> const struct cvb_table *cvb;
> + 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;

You should be using of_device_get_match_data() nowadays. That allows you
to do this in one step while at the same time allowing to keep the match
tables where they are.

>
> 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;
> @@ -121,12 +150,12 @@ static int tegra124_dfll_fcpu_probe(struct platform_device *pdev)
> return -EINVAL;
> }
>
> - cvb = tegra_cvb_build_opp_table(tegra124_cpu_cvb_tables,
> - ARRAY_SIZE(tegra124_cpu_cvb_tables),
> - &align,
> - process_id, speedo_id, speedo_value,
> - cpu_max_freq_table[speedo_id],
> - soc->dev);
> + cvb = tegra_cvb_build_opp_table(fcpu_data->cpu_cvb_tables,
> + fcpu_data->cpu_cvb_tables_size,
> + &align,
> + process_id, speedo_id, speedo_value,
> + fcpu_data->cpu_max_freq_table[speedo_id],
> + soc->dev);

This, and potentially other parts will conflict with some cleanup I
recently did to this code. You may want to rebase on some linux-next
earlier next week which should have those cleanup patches.

Thierry


Attachments:
(No filename) (3.80 kB)
signature.asc (819.00 B)
Download all attachments

2016-04-22 13:11:50

by Thierry Reding

[permalink] [raw]
Subject: Re: [PATCH 03/11] clk: tegra: Add DFLL DVCO reset control for Tegra210

On Fri, Apr 22, 2016 at 06:31:03PM +0800, Penny Chiu wrote:
> The DVCO present in the DFLL IP block has a separate reset line,
> exposed via the CAR IP block. This reset line is asserted upon SoC
> reset. Unless something (such as the DFLL driver) deasserts this
> line, the DVCO will not oscillate, although reads and writes to the
> DFLL IP block will complete.
>
> Signed-off-by: Penny Chiu <[email protected]>
> ---
> drivers/clk/tegra/clk-tegra210.c | 68 ++++++++++++++++++++++++++++++++
> include/dt-bindings/reset/tegra210-car.h | 12 ++++++
> 2 files changed, 80 insertions(+)
> create mode 100644 include/dt-bindings/reset/tegra210-car.h
>
> diff --git a/drivers/clk/tegra/clk-tegra210.c b/drivers/clk/tegra/clk-tegra210.c
> index d3709b1..3d70b38 100644
> --- a/drivers/clk/tegra/clk-tegra210.c
> +++ b/drivers/clk/tegra/clk-tegra210.c
> @@ -24,6 +24,7 @@
> #include <linux/export.h>
> #include <linux/clk/tegra.h>
> #include <dt-bindings/clock/tegra210-car.h>
> +#include <dt-bindings/reset/tegra210-car.h>
>
> #include "clk.h"
> #include "clk-id.h"
> @@ -39,6 +40,9 @@
> #define CLK_SOURCE_CSITE 0x1d4
> #define CLK_SOURCE_EMC 0x19c
>
> +#define RST_DFLL_DVCO 0x2f4
> +#define DVFS_DFLL_RESET_SHIFT 0

It'd be more idiomatic to make this:

#define DVFS_DFLL_RESET (1 << 0)

and use that below instead of hard-coding the 1 << and shifting by the
define.

> +
> #define PLLC_BASE 0x80
> #define PLLC_OUT 0x84
> #define PLLC_MISC0 0x88
> @@ -2781,6 +2785,68 @@ static void __init tegra210_clock_apply_init_table(void)
> }
>
> /**
> + * tegra210_car_barrier - wait for pending writes to the CAR to complete
> + *
> + * Wait for any outstanding writes to the CAR MMIO space from this CPU
> + * to complete before continuing execution. No return value.
> + */
> +static void tegra210_car_barrier(void)
> +{
> + readl_relaxed(clk_base + RST_DFLL_DVCO);
> +}

If you use the plain readl() and writel() functions, do you still need
the barrier? Or is there actually a requirement from the hardware to
flush writes by reading from any of the registers?

Thierry


Attachments:
(No filename) (2.05 kB)
signature.asc (819.00 B)
Download all attachments

2016-04-22 13:16:44

by Thierry Reding

[permalink] [raw]
Subject: Re: [PATCH 04/11] clk: tegra: Add Tegra210 support in DFLL driver

On Fri, Apr 22, 2016 at 06:31:04PM +0800, Penny Chiu wrote:
> Add Tegra210 support and related CVB table in tegra124 DFLL driver,
> and also update the binding document.
>
> Signed-off-by: Penny Chiu <[email protected]>
> ---
> .../bindings/clock/nvidia,tegra124-dfll.txt | 4 +-
> drivers/clk/tegra/Makefile | 4 +-
> drivers/clk/tegra/clk-tegra124-dfll-fcpu.c | 55 ++++++++++++++++++++++
> 3 files changed, 61 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt b/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt
> index 84080a8..42a1fe6 100644
> --- a/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt
> +++ b/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt
> @@ -11,7 +11,9 @@ communicating with an off-chip PMIC either via an I2C bus or via PWM signals.
> Currently only the I2C mode is supported by these bindings.
>
> Required properties:
> -- compatible : should be "nvidia,tegra124-dfll"
> +- compatible : should be one of following:
> + - "nvidia,tegra124-dfll" for the Tegra124 SoC
> + - "nvidia,tegra210-dfll" for the Tegra210 SoC
> - reg : Defines the following set of registers, in the order listed:
> - registers for the DFLL control logic.
> - registers for the I2C output logic.
> diff --git a/drivers/clk/tegra/Makefile b/drivers/clk/tegra/Makefile
> index 97984c5..9b8e9de 100644
> --- a/drivers/clk/tegra/Makefile
> +++ b/drivers/clk/tegra/Makefile
> @@ -17,7 +17,9 @@ 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
> +ifneq ($(filter y, $(CONFIG_ARCH_TEGRA_124_SOC) $(CONFIG_ARCH_TEGRA_210_SOC)),)
> +obj-y += clk-tegra124-dfll-fcpu.o
> +endif

This is better written as:

obj-$(CONFIG_ARCH_TEGRA_124_SOC) += clk-tegra124-dfll-fcpu.o
obj-$(CONFIG_ARCH_TEGRA_210_SOC) += clk-tegra124-dfll-fcpu.o

This works because obj-* variables are sorted using make's $(sort)
function, which lexically sorts and removes duplicates.

> +static const unsigned long tegra210_cpu_max_freq_table[] = {
> + [0] = 1912500000UL,
> + [1] = 1912500000UL,
> +};
> +
> static const struct cvb_table tegra124_cpu_cvb_tables[] = {
> {
> .speedo_id = -1,
> @@ -87,6 +92,45 @@ static const struct cvb_table tegra124_cpu_cvb_tables[] = {
> },
> };
>
> +static const struct cvb_table tegra210_cpu_cvb_tables[] = {
> + {
> + .speedo_id = -1,
> + .process_id = -1,
> + .min_millivolts = 850,
> + .max_millivolts = 1170,
> + .speedo_scale = 100,
> + .voltage_scale = 1000,
> + .cvb_table = {
> + {51000000UL, {1007452, -23865, 370} },
> + {102000000UL, {1007452, -23865, 370} },
> + {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} },
> + {0, { 0, 0, 0} },
> + },
> + .cpu_dfll_data = {
> + .tune0_low = 0xffead0ff,
> + .tune0_high = 0xffead0ff,
> + .tune1 = 0x20091d9,
> + }
> + },
> +};
> +
> 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),
> @@ -94,11 +138,22 @@ static const struct dfll_fcpu_data tegra124_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)
> +};

I think it would be better to keep all Tegra124 and Tegra210 data
together, so you should first list the Tegra124 maximum frequencies,
followed by the Tegra124 CVB table and the Tegra124 DFLL FCPU data,
then the same for Tegra210 data. That way we can make sure all new
appears in one hunk in a patch, which makes review and maintenance a
lot easier.

Thierry


Attachments:
(No filename) (4.88 kB)
signature.asc (819.00 B)
Download all attachments

2016-04-22 13:23:32

by Thierry Reding

[permalink] [raw]
Subject: Re: [PATCH 10/11] arm64: tegra: Add clock properties on cpu0 for Tegra210

On Fri, Apr 22, 2016 at 12:44:21PM +0100, Jon Hunter wrote:
> Hi Penny,
>
> On 22/04/16 11:31, Penny Chiu wrote:
> > Add clocks, clock-names, and clock-latency into cpu0 node.
> > These properties will be used by cpufreq driver.
> >
> > Signed-off-by: Penny Chiu <[email protected]>
> > ---
> > arch/arm64/boot/dts/nvidia/tegra210.dtsi | 6 ++++++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/nvidia/tegra210.dtsi b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
> > index 204d9cd..1a85857 100644
> > --- a/arch/arm64/boot/dts/nvidia/tegra210.dtsi
> > +++ b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
> > @@ -782,6 +782,12 @@
> > 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_OUT4>,
> > + <&dfll>;
> > + clock-names = "cpu_g", "pll_x", "pll_p", "dfll";
> > + clock-latency = <300000>;
> > };
> >
> > cpu@1 {
> >
>
> Can you include a patch with this series to update the binding
> documentation for the nvidia,tegra124-cpufreq.txt? I think that although
> there is no specific nvidia,tegra210-cpufreq compatible string the
> documentation should state that both tegra124 and tegra210 are supported
> so it is clear.
>
> Also I see the above binding no longer includes the "cpu_lp" for
> tegra210 which I understand we don't use here. However, the binding
> documentation should reflect this. Having said that, looking at the
> driver it appears the "cpu_lp" clock is not even used for tegra124. I
> wonder if we should drop this from the binding documentation and the
> tegra124.dtsi altogether?
>
> What about the "vdd-cpu-supply" property? Don't we need this?

For some reason that seems to have moved into the clock@70110000 node.

Thierry


Attachments:
(No filename) (1.82 kB)
signature.asc (819.00 B)
Download all attachments

2016-04-22 13:28:46

by Thierry Reding

[permalink] [raw]
Subject: Re: [PATCH 09/11] arm64: tegra: Add DFLL clock node on Jetson TX1

On Fri, Apr 22, 2016 at 06:31:09PM +0800, Penny Chiu wrote:
> Add DFLL clock device-tree node for Tegra210 DFLL IP block.
>
> Signed-off-by: Penny Chiu <[email protected]>
> ---
> arch/arm64/boot/dts/nvidia/tegra210-p2180.dtsi | 16 ++++++++++++++++
> arch/arm64/boot/dts/nvidia/tegra210.dtsi | 15 +++++++++++++++
> 2 files changed, 31 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/nvidia/tegra210-p2180.dtsi b/arch/arm64/boot/dts/nvidia/tegra210-p2180.dtsi
> index 9d02db2..5cf07f2 100644
> --- a/arch/arm64/boot/dts/nvidia/tegra210-p2180.dtsi
> +++ b/arch/arm64/boot/dts/nvidia/tegra210-p2180.dtsi
> @@ -43,6 +43,22 @@
> };
> };
>
> + dfll: clock@70110000 {
> + status = "okay";
> +
> + reg = <0x0 0x70110000 0x0 0x400>;
> + vdd-cpu-supply = <&cpu_ovr_reg>;
> + nvidia,pwm-to-pmic;
> + nvidia,init-uv = <1000000>;
> + nvidia,align-step-uv = <19200>; /* 19.2mv */
> + nvidia,sample-rate = <25000>;
> + nvidia,droop-ctrl = <0x00000f00>;
> + nvidia,force-mode = <1>;
> + nvidia,cf = <6>;
> + nvidia,ci = <0>;
> + nvidia,cg = <2>;
> + };
> +
> pwm_dfll: pwm@70110000 {
> compatible = "nvidia,tegra210-dfll-pwm";
> reg = <0x0 0x70110000 0x0 0x400>;

This isn't good. We're effectively sharing the same I/O memory between
two devices. Let's avoid that if possible.

It would seem to me that this DFLL PWM device isn't really a proper PWM
controller in the sense supported by the PWM framework. As such it might
be easier to have the nvidia,tegra210-dfll compatible device expose a
regulator directly rather than go via a "fake" PWM device and a
PWM-based regulator on top of that.

Thierry


Attachments:
(No filename) (1.59 kB)
signature.asc (819.00 B)
Download all attachments

2016-04-22 13:36:59

by Jon Hunter

[permalink] [raw]
Subject: Re: [PATCH 10/11] arm64: tegra: Add clock properties on cpu0 for Tegra210


On 22/04/16 14:23, Thierry Reding wrote:
> * PGP Signed by an unknown key
>
> On Fri, Apr 22, 2016 at 12:44:21PM +0100, Jon Hunter wrote:
>> Hi Penny,
>>
>> On 22/04/16 11:31, Penny Chiu wrote:
>>> Add clocks, clock-names, and clock-latency into cpu0 node.
>>> These properties will be used by cpufreq driver.
>>>
>>> Signed-off-by: Penny Chiu <[email protected]>
>>> ---
>>> arch/arm64/boot/dts/nvidia/tegra210.dtsi | 6 ++++++
>>> 1 file changed, 6 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/nvidia/tegra210.dtsi b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
>>> index 204d9cd..1a85857 100644
>>> --- a/arch/arm64/boot/dts/nvidia/tegra210.dtsi
>>> +++ b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
>>> @@ -782,6 +782,12 @@
>>> 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_OUT4>,
>>> + <&dfll>;
>>> + clock-names = "cpu_g", "pll_x", "pll_p", "dfll";
>>> + clock-latency = <300000>;
>>> };
>>>
>>> cpu@1 {
>>>
>>
>> Can you include a patch with this series to update the binding
>> documentation for the nvidia,tegra124-cpufreq.txt? I think that although
>> there is no specific nvidia,tegra210-cpufreq compatible string the
>> documentation should state that both tegra124 and tegra210 are supported
>> so it is clear.
>>
>> Also I see the above binding no longer includes the "cpu_lp" for
>> tegra210 which I understand we don't use here. However, the binding
>> documentation should reflect this. Having said that, looking at the
>> driver it appears the "cpu_lp" clock is not even used for tegra124. I
>> wonder if we should drop this from the binding documentation and the
>> tegra124.dtsi altogether?
>>
>> What about the "vdd-cpu-supply" property? Don't we need this?
>
> For some reason that seems to have moved into the clock@70110000 node.

Yes I saw that. I surprised that the cpufreq driver would probe without
this, unless I am missing something ...

Jon