[PATCH v2 0/4] memory: tegra: Introduce Tegra30 EMC driver

[PATCH v2 0/4] memory: tegra: Introduce Tegra30 EMC driver

[Dmitry Osipenko]

Hello,

This series introduces driver for the External Memory Controller (EMC)
found on Tegra30 chips, it controls the external DRAM on the board. The
purpose of this driver is to program memory timing for external memory on
the EMC clock rate change. The driver was tested using the ACTMON devfreq
driver that performs memory frequency scaling based on memory-usage load.

Please also note that this patchset is based on this series:

https://www.spinics.net/lists/linux-tegra/msg39687.html

Changelog:

v2: - Added support for changing MC clock diver configuration based on
      Memory Controller (MC) configuration which is part of the memory
      timing.

    - Merged the "Add custom EMC clock implementation" patch into this
      series because the "Introduce Tegra30 EMC driver" patch directly
      depends on it. Please note that Tegra20 EMC driver will need to be
      adapted for the clock changes as well, I'll send out the Tegra20
      patches after this series will be applied because of some other
      dependencies (devfreq) and because the temporary breakage won't
      be critical (driver will just error out on probe).

    - EMC driver now performs MC configuration validation by checking
      that the number of MC / EMC timings matches and that the timings
      rate is the same.

    - EMC driver now supports timings that want to change the MC clock
      configuration.

    - Other minor prettifying changes of the code.

Dmitry Osipenko (4):
  clk: tegra20/30: Add custom EMC clock implementation
  dt-bindings: memory: Add binding for NVIDIA Tegra30 External Memory
    Controller
  memory: tegra: Introduce Tegra30 EMC driver
  ARM: dts: tegra30: Add External Memory Controller node

 .../memory-controllers/nvidia,tegra30-emc.txt |  257 ++++
 arch/arm/boot/dts/tegra30.dtsi                |   11 +
 drivers/clk/tegra/Makefile                    |    2 +
 drivers/clk/tegra/clk-tegra20-emc.c           |  307 +++++
 drivers/clk/tegra/clk-tegra20.c               |   51 +-
 drivers/clk/tegra/clk-tegra30.c               |   35 +-
 drivers/clk/tegra/clk.h                       |    6 +
 drivers/memory/tegra/Kconfig                  |   10 +
 drivers/memory/tegra/Makefile                 |    1 +
 drivers/memory/tegra/mc.c                     |    3 -
 drivers/memory/tegra/mc.h                     |   30 +-
 drivers/memory/tegra/tegra30-emc.c            | 1159 +++++++++++++++++
 drivers/memory/tegra/tegra30.c                |   44 +
 include/linux/clk/tegra.h                     |    6 +
 14 files changed, 1860 insertions(+), 62 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-emc.txt
 create mode 100644 drivers/clk/tegra/clk-tegra20-emc.c
 create mode 100644 drivers/memory/tegra/tegra30-emc.c

-- 
2.21.0

[PATCH v2 1/4] clk: tegra20/30: Add custom EMC clock implementation

[Dmitry Osipenko]

A proper External Memory Controller clock rounding and parent selection
functionality is required by the EMC drivers. It is not available using
the generic clock implementation, hence add a custom one. The clock rate
rounding shall be done by the EMC drivers because they have information
about available memory timings, so the drivers will have to register a
callback that will round the requested rate. EMC clock users won't be able
to request EMC clock by getting -EPROBE_DEFER until EMC driver is probed
and the callback is set up. The functionality is somewhat similar to the
clk-emc.c which serves Tegra124+ SoC's, the later HW generations support
more parent clock sources and the HW configuration and integration with
the EMC drivers differs a tad from the older gens, hence it's not really
worth to try to squash everything into a single source file.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/clk/tegra/Makefile          |   2 +
 drivers/clk/tegra/clk-tegra20-emc.c | 307 ++++++++++++++++++++++++++++
 drivers/clk/tegra/clk-tegra20.c     |  51 ++---
 drivers/clk/tegra/clk-tegra30.c     |  35 +++-
 drivers/clk/tegra/clk.h             |   6 +
 include/linux/clk/tegra.h           |   6 +
 6 files changed, 357 insertions(+), 50 deletions(-)
 create mode 100644 drivers/clk/tegra/clk-tegra20-emc.c

diff --git a/drivers/clk/tegra/Makefile b/drivers/clk/tegra/Makefile
index 4812e45c2214..df966ca06788 100644
--- a/drivers/clk/tegra/Makefile
+++ b/drivers/clk/tegra/Makefile
@@ -17,7 +17,9 @@ obj-y					+= clk-tegra-fixed.o
 obj-y					+= clk-tegra-super-gen4.o
 obj-$(CONFIG_TEGRA_CLK_EMC)		+= clk-emc.o
 obj-$(CONFIG_ARCH_TEGRA_2x_SOC)         += clk-tegra20.o
+obj-$(CONFIG_ARCH_TEGRA_2x_SOC)		+= clk-tegra20-emc.o
 obj-$(CONFIG_ARCH_TEGRA_3x_SOC)         += clk-tegra30.o
+obj-$(CONFIG_ARCH_TEGRA_3x_SOC)		+= clk-tegra20-emc.o
 obj-$(CONFIG_ARCH_TEGRA_114_SOC)	+= clk-tegra114.o
 obj-$(CONFIG_ARCH_TEGRA_124_SOC)	+= clk-tegra124.o
 obj-$(CONFIG_TEGRA_CLK_DFLL)		+= clk-tegra124-dfll-fcpu.o
diff --git a/drivers/clk/tegra/clk-tegra20-emc.c b/drivers/clk/tegra/clk-tegra20-emc.c
new file mode 100644
index 000000000000..35b67a9989c8
--- /dev/null
+++ b/drivers/clk/tegra/clk-tegra20-emc.c
@@ -0,0 +1,307 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/bits.h>
+#include <linux/clk/tegra.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/slab.h>
+
+#include "clk.h"
+
+#define CLK_SOURCE_EMC_2X_CLK_DIVISOR_MASK	GENMASK(7, 0)
+#define CLK_SOURCE_EMC_2X_CLK_SRC_MASK		GENMASK(31, 30)
+#define CLK_SOURCE_EMC_2X_CLK_SRC_SHIFT		30
+
+#define MC_EMC_SAME_FREQ	BIT(16)
+#define USE_PLLM_UD		BIT(29)
+
+#define EMC_SRC_PLL_M		0
+#define EMC_SRC_PLL_C		1
+#define EMC_SRC_PLL_P		2
+#define EMC_SRC_CLK_M		3
+
+static const char * const emc_parent_clk_names[] = {
+	"pll_m", "pll_c", "pll_p", "clk_m",
+};
+
+struct tegra_clk_emc {
+	struct clk_hw hw;
+	void __iomem *reg;
+	bool mc_same_freq;
+	bool want_low_jitter;
+
+	long (*round_cb)(ulong rate, ulong rate_min, ulong rate_max, void *arg);
+	void *arg_cb;
+};
+
+static inline struct tegra_clk_emc *to_tegra_clk_emc(struct clk_hw *hw)
+{
+	return container_of(hw, struct tegra_clk_emc, hw);
+}
+
+static unsigned long emc_recalc_rate(struct clk_hw *hw,
+				     unsigned long parent_rate)
+{
+	struct tegra_clk_emc *emc = to_tegra_clk_emc(hw);
+	u32 val, div;
+
+	val = readl_relaxed(emc->reg);
+	div = val & CLK_SOURCE_EMC_2X_CLK_DIVISOR_MASK;
+
+	return DIV_ROUND_UP(parent_rate * 2, div + 2);
+}
+
+static u8 emc_get_parent(struct clk_hw *hw)
+{
+	struct tegra_clk_emc *emc = to_tegra_clk_emc(hw);
+
+	return readl_relaxed(emc->reg) >> CLK_SOURCE_EMC_2X_CLK_SRC_SHIFT;
+}
+
+static int emc_set_parent(struct clk_hw *hw, u8 index)
+{
+	struct tegra_clk_emc *emc = to_tegra_clk_emc(hw);
+	u32 val, div;
+
+	val = readl_relaxed(emc->reg);
+	val &= ~CLK_SOURCE_EMC_2X_CLK_SRC_MASK;
+	val |= index << CLK_SOURCE_EMC_2X_CLK_SRC_SHIFT;
+
+	div = val & CLK_SOURCE_EMC_2X_CLK_DIVISOR_MASK;
+
+	if (index == EMC_SRC_PLL_M && div == 0 && emc->want_low_jitter)
+		val |= USE_PLLM_UD;
+	else
+		val &= ~USE_PLLM_UD;
+
+	if (emc->mc_same_freq)
+		val |= MC_EMC_SAME_FREQ;
+	else
+		val &= ~MC_EMC_SAME_FREQ;
+
+	writel_relaxed(val, emc->reg);
+
+	return 0;
+}
+
+static int emc_set_rate(struct clk_hw *hw, unsigned long rate,
+			unsigned long parent_rate)
+{
+	struct tegra_clk_emc *emc = to_tegra_clk_emc(hw);
+	unsigned int index;
+	u32 val, div;
+
+	div = div_frac_get(rate, parent_rate, 8, 1, 0);
+
+	val = readl_relaxed(emc->reg);
+	val &= ~CLK_SOURCE_EMC_2X_CLK_DIVISOR_MASK;
+	val |= div;
+
+	index = val >> CLK_SOURCE_EMC_2X_CLK_SRC_SHIFT;
+
+	if (index == EMC_SRC_PLL_M && div == 0 && emc->want_low_jitter)
+		val |= USE_PLLM_UD;
+	else
+		val &= ~USE_PLLM_UD;
+
+	if (emc->mc_same_freq)
+		val |= MC_EMC_SAME_FREQ;
+	else
+		val &= ~MC_EMC_SAME_FREQ;
+
+	writel_relaxed(val, emc->reg);
+
+	return 0;
+}
+
+static int emc_set_rate_and_parent(struct clk_hw *hw,
+				   unsigned long rate,
+				   unsigned long parent_rate,
+				   u8 index)
+{
+	struct tegra_clk_emc *emc = to_tegra_clk_emc(hw);
+	u32 val, div;
+
+	div = div_frac_get(rate, parent_rate, 8, 1, 0);
+
+	val = readl_relaxed(emc->reg);
+
+	val &= ~CLK_SOURCE_EMC_2X_CLK_SRC_MASK;
+	val |= index << CLK_SOURCE_EMC_2X_CLK_SRC_SHIFT;
+
+	val &= ~CLK_SOURCE_EMC_2X_CLK_DIVISOR_MASK;
+	val |= div;
+
+	if (index == EMC_SRC_PLL_M && div == 0 && emc->want_low_jitter)
+		val |= USE_PLLM_UD;
+	else
+		val &= ~USE_PLLM_UD;
+
+	if (emc->mc_same_freq)
+		val |= MC_EMC_SAME_FREQ;
+	else
+		val &= ~MC_EMC_SAME_FREQ;
+
+	writel_relaxed(val, emc->reg);
+
+	return 0;
+}
+
+static int emc_determine_rate(struct clk_hw *hw, struct clk_rate_request *req)
+{
+	struct tegra_clk_emc *emc = to_tegra_clk_emc(hw);
+	struct clk_hw *parent_hw;
+	unsigned long divided_rate;
+	unsigned long parent_rate;
+	unsigned int i;
+	long emc_rate;
+	int div;
+
+	emc_rate = emc->round_cb(req->rate, req->min_rate, req->max_rate,
+				 emc->arg_cb);
+	if (emc_rate < 0)
+		return emc_rate;
+
+	for (i = 0; i < ARRAY_SIZE(emc_parent_clk_names); i++) {
+		parent_hw = clk_hw_get_parent_by_index(hw, i);
+
+		if (req->best_parent_hw == parent_hw)
+			parent_rate = req->best_parent_rate;
+		else
+			parent_rate = clk_hw_get_rate(parent_hw);
+
+		if (emc_rate > parent_rate)
+			continue;
+
+		div = div_frac_get(emc_rate, parent_rate, 8, 1, 0);
+		divided_rate = DIV_ROUND_UP(parent_rate * 2, div + 2);
+
+		if (divided_rate != emc_rate)
+			continue;
+
+		req->best_parent_rate = parent_rate;
+		req->best_parent_hw = parent_hw;
+		req->rate = emc_rate;
+		break;
+	}
+
+	if (i == ARRAY_SIZE(emc_parent_clk_names)) {
+		pr_err_once("%s: can't find parent for rate %lu emc_rate %lu\n",
+			    __func__, req->rate, emc_rate);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static const struct clk_ops tegra_clk_emc_ops = {
+	.recalc_rate = emc_recalc_rate,
+	.get_parent = emc_get_parent,
+	.set_parent = emc_set_parent,
+	.set_rate = emc_set_rate,
+	.set_rate_and_parent = emc_set_rate_and_parent,
+	.determine_rate = emc_determine_rate,
+};
+
+void tegra20_clk_set_emc_round_callback(void *round_cb, void *arg_cb)
+{
+	struct clk *clk = __clk_lookup("emc");
+	struct tegra_clk_emc *emc;
+	struct clk_hw *hw;
+
+	if (clk) {
+		hw = __clk_get_hw(clk);
+		emc = to_tegra_clk_emc(hw);
+
+		emc->round_cb = round_cb;
+		emc->arg_cb = arg_cb;
+	}
+}
+
+bool tegra20_clk_emc_driver_available(void)
+{
+	struct clk *clk = __clk_lookup("emc");
+	struct tegra_clk_emc *emc;
+	struct clk_hw *hw;
+
+	if (clk) {
+		hw = __clk_get_hw(clk);
+		emc = to_tegra_clk_emc(hw);
+
+		return !!emc->round_cb;
+	}
+
+	return false;
+}
+
+struct clk *tegra20_clk_register_emc(void __iomem *ioaddr)
+{
+	struct tegra_clk_emc *emc;
+	struct clk_init_data init;
+	struct clk *clk;
+
+	emc = kzalloc(sizeof(*emc), GFP_KERNEL);
+	if (!emc)
+		return NULL;
+
+	init.name = "emc";
+	init.ops = &tegra_clk_emc_ops;
+	init.flags = CLK_IS_CRITICAL;
+	init.parent_names = emc_parent_clk_names;
+	init.num_parents = ARRAY_SIZE(emc_parent_clk_names);
+
+	emc->reg = ioaddr;
+	emc->hw.init = &init;
+
+	clk = clk_register(NULL, &emc->hw);
+	if (IS_ERR(clk)) {
+		kfree(emc);
+		return NULL;
+	}
+
+	return clk;
+}
+
+void tegra30_clk_set_emc_round_callback(void *round_cb, void *arg_cb)
+{
+	tegra20_clk_set_emc_round_callback(round_cb, arg_cb);
+}
+
+bool tegra30_clk_emc_driver_available(void)
+{
+	return tegra20_clk_emc_driver_available();
+}
+
+struct clk *tegra30_clk_register_emc(void __iomem *ioaddr)
+{
+	struct tegra_clk_emc *emc;
+	struct clk_hw *hw;
+	struct clk *clk;
+
+	clk = tegra20_clk_register_emc(ioaddr);
+	if (!clk)
+		return NULL;
+
+	hw = __clk_get_hw(clk);
+	emc = to_tegra_clk_emc(hw);
+	emc->want_low_jitter = true;
+
+	return clk;
+}
+
+int tegra30_clk_prepare_emc_mc_same_freq(struct clk *emc_clk, bool same)
+{
+	struct tegra_clk_emc *emc;
+	struct clk_hw *hw;
+
+	if (emc_clk) {
+		hw = __clk_get_hw(emc_clk);
+		emc = to_tegra_clk_emc(hw);
+		emc->mc_same_freq = same;
+
+		return 0;
+	}
+
+	return -EINVAL;
+}
diff --git a/drivers/clk/tegra/clk-tegra20.c b/drivers/clk/tegra/clk-tegra20.c
index c71b61162a32..45472e43ab50 100644
--- a/drivers/clk/tegra/clk-tegra20.c
+++ b/drivers/clk/tegra/clk-tegra20.c
@@ -141,8 +141,6 @@ static struct cpu_clk_suspend_context {
 static void __iomem *clk_base;
 static void __iomem *pmc_base;
 
-static DEFINE_SPINLOCK(emc_lock);
-
 #define TEGRA_INIT_DATA_MUX(_name, _parents, _offset,	\
 			    _clk_num, _gate_flags, _clk_id)	\
 	TEGRA_INIT_DATA(_name, NULL, NULL, _parents, _offset,	\
@@ -771,7 +769,6 @@ static const char *pwm_parents[] = { "pll_p", "pll_c", "audio", "clk_m",
 static const char *mux_pllpcm_clkm[] = { "pll_p", "pll_c", "pll_m", "clk_m" };
 static const char *mux_pllpdc_clkm[] = { "pll_p", "pll_d_out0", "pll_c",
 					 "clk_m" };
-static const char *mux_pllmcp_clkm[] = { "pll_m", "pll_c", "pll_p", "clk_m" };
 
 static struct tegra_periph_init_data tegra_periph_clk_list[] = {
 	TEGRA_INIT_DATA_MUX("i2s1", i2s1_parents,     CLK_SOURCE_I2S1,   11, TEGRA_PERIPH_ON_APB, TEGRA20_CLK_I2S1),
@@ -798,41 +795,6 @@ static struct tegra_periph_init_data tegra_periph_nodiv_clk_list[] = {
 	TEGRA_INIT_DATA_NODIV("disp2",	mux_pllpdc_clkm, CLK_SOURCE_DISP2, 30, 2, 26,  0, TEGRA20_CLK_DISP2),
 };
 
-static void __init tegra20_emc_clk_init(void)
-{
-	const u32 use_pllm_ud = BIT(29);
-	struct clk *clk;
-	u32 emc_reg;
-
-	clk = clk_register_mux(NULL, "emc_mux", mux_pllmcp_clkm,
-			       ARRAY_SIZE(mux_pllmcp_clkm),
-			       CLK_SET_RATE_NO_REPARENT,
-			       clk_base + CLK_SOURCE_EMC,
-			       30, 2, 0, &emc_lock);
-
-	clk = tegra_clk_register_mc("mc", "emc_mux", clk_base + CLK_SOURCE_EMC,
-				    &emc_lock);
-	clks[TEGRA20_CLK_MC] = clk;
-
-	/* un-divided pll_m_out0 is currently unsupported */
-	emc_reg = readl_relaxed(clk_base + CLK_SOURCE_EMC);
-	if (emc_reg & use_pllm_ud) {
-		pr_err("%s: un-divided PllM_out0 used as clock source\n",
-		       __func__);
-		return;
-	}
-
-	/*
-	 * Note that 'emc_mux' source and 'emc' rate shouldn't be changed at
-	 * the same time due to a HW bug, this won't happen because we're
-	 * defining 'emc_mux' and 'emc' as distinct clocks.
-	 */
-	clk = tegra_clk_register_divider("emc", "emc_mux",
-				clk_base + CLK_SOURCE_EMC, CLK_IS_CRITICAL,
-				TEGRA_DIVIDER_INT, 0, 8, 1, &emc_lock);
-	clks[TEGRA20_CLK_EMC] = clk;
-}
-
 static void __init tegra20_periph_clk_init(void)
 {
 	struct tegra_periph_init_data *data;
@@ -846,7 +808,13 @@ static void __init tegra20_periph_clk_init(void)
 	clks[TEGRA20_CLK_AC97] = clk;
 
 	/* emc */
-	tegra20_emc_clk_init();
+	clk = tegra20_clk_register_emc(clk_base + CLK_SOURCE_EMC);
+
+	clks[TEGRA20_CLK_EMC] = clk;
+
+	clk = tegra_clk_register_mc("mc", "emc", clk_base + CLK_SOURCE_EMC,
+				    NULL);
+	clks[TEGRA20_CLK_MC] = clk;
 
 	/* dsi */
 	clk = tegra_clk_register_periph_gate("dsi", "pll_d", 0, clk_base, 0,
@@ -1142,6 +1110,11 @@ static struct clk *tegra20_clk_src_onecell_get(struct of_phandle_args *clkspec,
 			return ERR_PTR(-EPROBE_DEFER);
 	}
 
+	if (clkspec->args[0] == TEGRA20_CLK_EMC) {
+		if (!tegra20_clk_emc_driver_available())
+			return ERR_PTR(-EPROBE_DEFER);
+	}
+
 	return clk;
 }
 
diff --git a/drivers/clk/tegra/clk-tegra30.c b/drivers/clk/tegra/clk-tegra30.c
index fa8d573ac626..9b5ffa0a6ed8 100644
--- a/drivers/clk/tegra/clk-tegra30.c
+++ b/drivers/clk/tegra/clk-tegra30.c
@@ -162,7 +162,6 @@ static unsigned long input_freq;
 
 static DEFINE_SPINLOCK(cml_lock);
 static DEFINE_SPINLOCK(pll_d_lock);
-static DEFINE_SPINLOCK(emc_lock);
 
 #define TEGRA_INIT_DATA_MUX(_name, _parents, _offset,	\
 			    _clk_num, _gate_flags, _clk_id)	\
@@ -819,7 +818,7 @@ static struct tegra_clk tegra30_clks[tegra_clk_max] __initdata = {
 	[tegra_clk_pll_a] = { .dt_id = TEGRA30_CLK_PLL_A, .present = true },
 	[tegra_clk_pll_a_out0] = { .dt_id = TEGRA30_CLK_PLL_A_OUT0, .present = true },
 	[tegra_clk_cec] = { .dt_id = TEGRA30_CLK_CEC, .present = true },
-	[tegra_clk_emc] = { .dt_id = TEGRA30_CLK_EMC, .present = true },
+	[tegra_clk_emc] = { .dt_id = TEGRA30_CLK_EMC, .present = false },
 };
 
 static const char *pll_e_parents[] = { "pll_ref", "pll_p" };
@@ -1006,7 +1005,6 @@ static void __init tegra30_super_clk_init(void)
 static const char *mux_pllacp_clkm[] = { "pll_a_out0", "unused", "pll_p",
 					 "clk_m" };
 static const char *mux_pllpcm_clkm[] = { "pll_p", "pll_c", "pll_m", "clk_m" };
-static const char *mux_pllmcp_clkm[] = { "pll_m", "pll_c", "pll_p", "clk_m" };
 static const char *spdif_out_parents[] = { "pll_a_out0", "spdif_2x", "pll_p",
 					   "clk_m" };
 static const char *mux_pllmcpa[] = { "pll_m", "pll_c", "pll_p", "pll_a_out0" };
@@ -1055,14 +1053,12 @@ static void __init tegra30_periph_clk_init(void)
 	clks[TEGRA30_CLK_AFI] = clk;
 
 	/* emc */
-	clk = clk_register_mux(NULL, "emc_mux", mux_pllmcp_clkm,
-			       ARRAY_SIZE(mux_pllmcp_clkm),
-			       CLK_SET_RATE_NO_REPARENT,
-			       clk_base + CLK_SOURCE_EMC,
-			       30, 2, 0, &emc_lock);
+	clk = tegra30_clk_register_emc(clk_base + CLK_SOURCE_EMC);
+
+	clks[TEGRA30_CLK_EMC] = clk;
 
-	clk = tegra_clk_register_mc("mc", "emc_mux", clk_base + CLK_SOURCE_EMC,
-				    &emc_lock);
+	clk = tegra_clk_register_mc("mc", "emc", clk_base + CLK_SOURCE_EMC,
+				    NULL);
 	clks[TEGRA30_CLK_MC] = clk;
 
 	/* cml0 */
@@ -1313,6 +1309,23 @@ static struct tegra_audio_clk_info tegra30_audio_plls[] = {
 	{ "pll_a", &pll_a_params, tegra_clk_pll_a, "pll_p_out1" },
 };
 
+static struct clk *tegra30_clk_src_onecell_get(struct of_phandle_args *clkspec,
+					       void *data)
+{
+	struct clk *clk;
+
+	clk = of_clk_src_onecell_get(clkspec, data);
+	if (IS_ERR(clk))
+		return clk;
+
+	if (clkspec->args[0] == TEGRA30_CLK_EMC) {
+		if (!tegra30_clk_emc_driver_available())
+			return ERR_PTR(-EPROBE_DEFER);
+	}
+
+	return clk;
+}
+
 static void __init tegra30_clock_init(struct device_node *np)
 {
 	struct device_node *node;
@@ -1356,7 +1369,7 @@ static void __init tegra30_clock_init(struct device_node *np)
 
 	tegra_init_dup_clks(tegra_clk_duplicates, clks, TEGRA30_CLK_CLK_MAX);
 
-	tegra_add_of_provider(np, of_clk_src_onecell_get);
+	tegra_add_of_provider(np, tegra30_clk_src_onecell_get);
 	tegra_register_devclks(devclks, ARRAY_SIZE(devclks));
 
 	tegra_clk_apply_init_table = tegra30_clock_apply_init_table;
diff --git a/drivers/clk/tegra/clk.h b/drivers/clk/tegra/clk.h
index 09bccbb9640c..197cc1122e20 100644
--- a/drivers/clk/tegra/clk.h
+++ b/drivers/clk/tegra/clk.h
@@ -849,4 +849,10 @@ int div_frac_get(unsigned long rate, unsigned parent_rate, u8 width,
 		udelay(delay);		\
 	} while (0)
 
+bool tegra20_clk_emc_driver_available(void);
+struct clk *tegra20_clk_register_emc(void __iomem *ioaddr);
+
+bool tegra30_clk_emc_driver_available(void);
+struct clk *tegra30_clk_register_emc(void __iomem *ioaddr);
+
 #endif /* TEGRA_CLK_H */
diff --git a/include/linux/clk/tegra.h b/include/linux/clk/tegra.h
index afb9edfa5d58..8e86951b2759 100644
--- a/include/linux/clk/tegra.h
+++ b/include/linux/clk/tegra.h
@@ -130,4 +130,10 @@ extern void tegra210_put_utmipll_in_iddq(void);
 extern void tegra210_put_utmipll_out_iddq(void);
 extern int tegra210_clk_handle_mbist_war(unsigned int id);
 
+struct clk;
+
+void tegra20_clk_set_emc_round_callback(void *round_cb, void *arg_cb);
+void tegra30_clk_set_emc_round_callback(void *round_cb, void *arg_cb);
+int tegra30_clk_prepare_emc_mc_same_freq(struct clk *emc_clk, bool same);
+
 #endif /* __LINUX_CLK_TEGRA_H_ */
-- 
2.21.0

Re: [PATCH v2 1/4] clk: tegra20/30: Add custom EMC clock implementation

[Stephen Boyd]

Quoting Dmitry Osipenko (2019-04-14 13:20:06)
> diff --git a/drivers/clk/tegra/clk-tegra20-emc.c b/drivers/clk/tegra/clk-tegra20-emc.c
> new file mode 100644
> index 000000000000..35b67a9989c8
> --- /dev/null
> +++ b/drivers/clk/tegra/clk-tegra20-emc.c
> @@ -0,0 +1,307 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <linux/bits.h>
> +#include <linux/clk/tegra.h>

Include clk-provider.h as this is a clk provider driver.

> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/slab.h>
> +
> +#include "clk.h"
> +
> +#define CLK_SOURCE_EMC_2X_CLK_DIVISOR_MASK     GENMASK(7, 0)
> +#define CLK_SOURCE_EMC_2X_CLK_SRC_MASK         GENMASK(31, 30)
[...]
> +
> +static const struct clk_ops tegra_clk_emc_ops = {
> +       .recalc_rate = emc_recalc_rate,
> +       .get_parent = emc_get_parent,
> +       .set_parent = emc_set_parent,
> +       .set_rate = emc_set_rate,
> +       .set_rate_and_parent = emc_set_rate_and_parent,
> +       .determine_rate = emc_determine_rate,
> +};
> +
> +void tegra20_clk_set_emc_round_callback(void *round_cb, void *arg_cb)

Why can't we have type safety on these function pointers?

> +{
> +       struct clk *clk = __clk_lookup("emc");
> +       struct tegra_clk_emc *emc;
> +       struct clk_hw *hw;
> +
> +       if (clk) {
> +               hw = __clk_get_hw(clk);
> +               emc = to_tegra_clk_emc(hw);
> +
> +               emc->round_cb = round_cb;
> +               emc->arg_cb = arg_cb;
> +       }
> +}
> +
> +bool tegra20_clk_emc_driver_available(void)
> +{
> +       struct clk *clk = __clk_lookup("emc");

Can we avoid using __clk_lookup()? Maybe by having the clk_hw pointer in
this driver somehow?

> +       struct tegra_clk_emc *emc;
> +       struct clk_hw *hw;
> +
> +       if (clk) {
> +               hw = __clk_get_hw(clk);

This gets further to the point, we don't prefer to see drivers use
__clk_get_hw() unless they absolutely need to. Maybe I don't understand
the driver structure, but it seems like maybe the driver that's
providing the callbacks could be the same driver that's registering
these clks, and thus everything could be inside one file so that we
don't have to pass around callbacks and clk_hw pointers? Commit text
says "this is how it's been" but that's not a reason to keep doing it.

> +               emc = to_tegra_clk_emc(hw);
> +

Re: [PATCH v2 1/4] clk: tegra20/30: Add custom EMC clock implementation

[Dmitry Osipenko]

25.04.2019 22:07, Stephen Boyd пишет:
> Quoting Dmitry Osipenko (2019-04-14 13:20:06)
>> diff --git a/drivers/clk/tegra/clk-tegra20-emc.c b/drivers/clk/tegra/clk-tegra20-emc.c
>> new file mode 100644
>> index 000000000000..35b67a9989c8
>> --- /dev/null
>> +++ b/drivers/clk/tegra/clk-tegra20-emc.c
>> @@ -0,0 +1,307 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +#include <linux/bits.h>
>> +#include <linux/clk/tegra.h>
> 
> Include clk-provider.h as this is a clk provider driver.

Okay, although clk-provider.h is also included by clk.h below.

>> +#include <linux/err.h>
>> +#include <linux/io.h>
>> +#include <linux/kernel.h>
>> +#include <linux/slab.h>
>> +
>> +#include "clk.h"
>> +
>> +#define CLK_SOURCE_EMC_2X_CLK_DIVISOR_MASK     GENMASK(7, 0)
>> +#define CLK_SOURCE_EMC_2X_CLK_SRC_MASK         GENMASK(31, 30)
> [...]
>> +
>> +static const struct clk_ops tegra_clk_emc_ops = {
>> +       .recalc_rate = emc_recalc_rate,
>> +       .get_parent = emc_get_parent,
>> +       .set_parent = emc_set_parent,
>> +       .set_rate = emc_set_rate,
>> +       .set_rate_and_parent = emc_set_rate_and_parent,
>> +       .determine_rate = emc_determine_rate,
>> +};
>> +
>> +void tegra20_clk_set_emc_round_callback(void *round_cb, void *arg_cb)
> 
> Why can't we have type safety on these function pointers?

It is probably not really necessary since it's a platform-specific API.
But I'll add an explicit type in v3 for consistency, thanks.

>> +{
>> +       struct clk *clk = __clk_lookup("emc");
>> +       struct tegra_clk_emc *emc;
>> +       struct clk_hw *hw;
>> +
>> +       if (clk) {
>> +               hw = __clk_get_hw(clk);
>> +               emc = to_tegra_clk_emc(hw);
>> +
>> +               emc->round_cb = round_cb;
>> +               emc->arg_cb = arg_cb;
>> +       }
>> +}
>> +
>> +bool tegra20_clk_emc_driver_available(void)
>> +{
>> +       struct clk *clk = __clk_lookup("emc");
> 
> Can we avoid using __clk_lookup()? Maybe by having the clk_hw pointer in
> this driver somehow?

tegra20_clk_emc_driver_available() is a private function of the Tegra's
clk-core. Please note that prototype of this function is added to the
local "drivers/clk/tegra/clk.h" file.

It is indeed possible to use clk pointer instead of __clk_lookup() and
I'll change that in v3 since it's causing some confusion.

>> +       struct tegra_clk_emc *emc;
>> +       struct clk_hw *hw;
>> +
>> +       if (clk) {
>> +               hw = __clk_get_hw(clk);
> 
> This gets further to the point, we don't prefer to see drivers use
> __clk_get_hw() unless they absolutely need to. Maybe I don't understand
> the driver structure, but it seems like maybe the driver that's
> providing the callbacks could be the same driver that's registering
> these clks, and thus everything could be inside one file so that we
> don't have to pass around callbacks and clk_hw pointers? Commit text
> says "this is how it's been" but that's not a reason to keep doing it.

Again, tegra20_clk_emc_driver_available() is for the Terga's clk-core
and not for the EMC (External Memory Controller) driver. This function
is used to determine whether EMC driver is ready to handle clock-rate
changes (PRE/POST rate-change notifications), clk users can't get EMC
clock until driver is ready (i.e. the "round" callback is registered by
the driver).

Please see tegra20_clk_src_onecell_get() changes in this patch and
drivers/memory/tegra/tegra20-emc.c for clarity.

It is not possible to register clk from the EMC driver without having
some custom integration with the clk-core because CLK and EMC are
different hardware units and hence EMC driver doesn't have direct access
to the CLK registers. I think it is better to keep CLK and memory-timing
programming logically separated simply for consistency, although I'm
open to suggestions.

Re: [PATCH v2 1/4] clk: tegra20/30: Add custom EMC clock implementation

[Stephen Boyd]

Quoting Dmitry Osipenko (2019-04-25 14:42:19)
> 25.04.2019 22:07, Stephen Boyd пишет:
> > Quoting Dmitry Osipenko (2019-04-14 13:20:06)
> >> diff --git a/drivers/clk/tegra/clk-tegra20-emc.c b/drivers/clk/tegra/clk-tegra20-emc.c
> >> new file mode 100644
> >> index 000000000000..35b67a9989c8
> >> --- /dev/null
> >> +++ b/drivers/clk/tegra/clk-tegra20-emc.c
> >> @@ -0,0 +1,307 @@
> >> +// SPDX-License-Identifier: GPL-2.0
> >> +
> >> +#include <linux/bits.h>
> >> +#include <linux/clk/tegra.h>
> > 
> > Include clk-provider.h as this is a clk provider driver.
> 
> Okay, although clk-provider.h is also included by clk.h below.

We prefer explicit includes instead of implicit ones. Failing to do that
leads to changes like the one I'm making now to push out linux/io.h to
all the users who are relying on clk-provider.h to provide it for them.

> 
> >> +{
> >> +       struct clk *clk = __clk_lookup("emc");
> >> +       struct tegra_clk_emc *emc;
> >> +       struct clk_hw *hw;
> >> +
> >> +       if (clk) {
> >> +               hw = __clk_get_hw(clk);
> >> +               emc = to_tegra_clk_emc(hw);
> >> +
> >> +               emc->round_cb = round_cb;
> >> +               emc->arg_cb = arg_cb;
> >> +       }
> >> +}
> >> +
> >> +bool tegra20_clk_emc_driver_available(void)
> >> +{
> >> +       struct clk *clk = __clk_lookup("emc");
> > 
> > Can we avoid using __clk_lookup()? Maybe by having the clk_hw pointer in
> > this driver somehow?
> 
> tegra20_clk_emc_driver_available() is a private function of the Tegra's
> clk-core. Please note that prototype of this function is added to the
> local "drivers/clk/tegra/clk.h" file.
> 
> It is indeed possible to use clk pointer instead of __clk_lookup() and
> I'll change that in v3 since it's causing some confusion.

Can you use a clk_hw pointer directly? I'd like to see clk provider
drivers only use struct clk_hw, and clk consumers only use struct clk.

> 
> >> +       struct tegra_clk_emc *emc;
> >> +       struct clk_hw *hw;
> >> +
> >> +       if (clk) {
> >> +               hw = __clk_get_hw(clk);
> > 
> > This gets further to the point, we don't prefer to see drivers use
> > __clk_get_hw() unless they absolutely need to. Maybe I don't understand
> > the driver structure, but it seems like maybe the driver that's
> > providing the callbacks could be the same driver that's registering
> > these clks, and thus everything could be inside one file so that we
> > don't have to pass around callbacks and clk_hw pointers? Commit text
> > says "this is how it's been" but that's not a reason to keep doing it.
> 
> Again, tegra20_clk_emc_driver_available() is for the Terga's clk-core
> and not for the EMC (External Memory Controller) driver. This function
> is used to determine whether EMC driver is ready to handle clock-rate
> changes (PRE/POST rate-change notifications), clk users can't get EMC
> clock until driver is ready (i.e. the "round" callback is registered by
> the driver).

Who are the other users of the EMC clk? Why does the EMC driver and the
clk driver need to be in sync in such a way that requires us to check if
some code in the EMC driver is ready before we can hand out clks from
this clk provider? It looks like some tight coupling between these two
pieces of code on the clk_get() path which is setting off alarms for me.

> 
> Please see tegra20_clk_src_onecell_get() changes in this patch and
> drivers/memory/tegra/tegra20-emc.c for clarity.
> 
> It is not possible to register clk from the EMC driver without having
> some custom integration with the clk-core because CLK and EMC are
> different hardware units and hence EMC driver doesn't have direct access
> to the CLK registers. I think it is better to keep CLK and memory-timing
> programming logically separated simply for consistency, although I'm
> open to suggestions.

Sorry, I'm really confused and it's probably because I'm not taking the
time to read all the Tegra code right now. I'm trying to understand the
overall memory controller frequency logic.

I understand that there's a memory controller, and a clk controller, and
they're different hardware blocks. Presumably the clk controller can't
change frequencies without informing the memory controller that it's
going to change something so the memory controller can fix timings in
the EMC register space.

Is there some other entity that's needing to be deferred while the EMC
and clk drivers tell each other that they're probed and ready so that
these notifiers all work? Is that entity using clks directly instead of
going through the EMC to change frequencies? If so, why can't we funnel
that logic through EMC which then goes to the clk driver? That way it's
clearly "thing that changes frequency" -> EMC -> clk controller call
chain. It might even allow us to get rid of the rate notifiers?

Re: [PATCH v2 1/4] clk: tegra20/30: Add custom EMC clock implementation

[Dmitry Osipenko]

26.04.2019 1:25, Stephen Boyd пишет:
> Quoting Dmitry Osipenko (2019-04-25 14:42:19)
>> 25.04.2019 22:07, Stephen Boyd пишет:
>>> Quoting Dmitry Osipenko (2019-04-14 13:20:06)
>>>> diff --git a/drivers/clk/tegra/clk-tegra20-emc.c b/drivers/clk/tegra/clk-tegra20-emc.c
>>>> new file mode 100644
>>>> index 000000000000..35b67a9989c8
>>>> --- /dev/null
>>>> +++ b/drivers/clk/tegra/clk-tegra20-emc.c
>>>> @@ -0,0 +1,307 @@
>>>> +// SPDX-License-Identifier: GPL-2.0
>>>> +
>>>> +#include <linux/bits.h>
>>>> +#include <linux/clk/tegra.h>
>>>
>>> Include clk-provider.h as this is a clk provider driver.
>>
>> Okay, although clk-provider.h is also included by clk.h below.
> 
> We prefer explicit includes instead of implicit ones. Failing to do that
> leads to changes like the one I'm making now to push out linux/io.h to
> all the users who are relying on clk-provider.h to provide it for them.

Ok.

>>
>>>> +{
>>>> +       struct clk *clk = __clk_lookup("emc");
>>>> +       struct tegra_clk_emc *emc;
>>>> +       struct clk_hw *hw;
>>>> +
>>>> +       if (clk) {
>>>> +               hw = __clk_get_hw(clk);
>>>> +               emc = to_tegra_clk_emc(hw);
>>>> +
>>>> +               emc->round_cb = round_cb;
>>>> +               emc->arg_cb = arg_cb;
>>>> +       }
>>>> +}
>>>> +
>>>> +bool tegra20_clk_emc_driver_available(void)
>>>> +{
>>>> +       struct clk *clk = __clk_lookup("emc");
>>>
>>> Can we avoid using __clk_lookup()? Maybe by having the clk_hw pointer in
>>> this driver somehow?
>>
>> tegra20_clk_emc_driver_available() is a private function of the Tegra's
>> clk-core. Please note that prototype of this function is added to the
>> local "drivers/clk/tegra/clk.h" file.
>>
>> It is indeed possible to use clk pointer instead of __clk_lookup() and
>> I'll change that in v3 since it's causing some confusion.
> 
> Can you use a clk_hw pointer directly? I'd like to see clk provider
> drivers only use struct clk_hw, and clk consumers only use struct clk.

The clk_hw pointers are not stored directly anywhere in the Tegra's
clk-driver, it will be a quite massive change to make the clk driver's
code to use clk_hw pointers instead of struct clk.

I can add a global variable for the tegra_clk_emc pointer, but it is not
really necessary since there is __clk_get_hw.

Could you please explain what's the problem with __clk_get_hw?

>>
>>>> +       struct tegra_clk_emc *emc;
>>>> +       struct clk_hw *hw;
>>>> +
>>>> +       if (clk) {
>>>> +               hw = __clk_get_hw(clk);
>>>
>>> This gets further to the point, we don't prefer to see drivers use
>>> __clk_get_hw() unless they absolutely need to. Maybe I don't understand
>>> the driver structure, but it seems like maybe the driver that's
>>> providing the callbacks could be the same driver that's registering
>>> these clks, and thus everything could be inside one file so that we
>>> don't have to pass around callbacks and clk_hw pointers? Commit text
>>> says "this is how it's been" but that's not a reason to keep doing it.
>>
>> Again, tegra20_clk_emc_driver_available() is for the Terga's clk-core
>> and not for the EMC (External Memory Controller) driver. This function
>> is used to determine whether EMC driver is ready to handle clock-rate
>> changes (PRE/POST rate-change notifications), clk users can't get EMC
>> clock until driver is ready (i.e. the "round" callback is registered by
>> the driver).
> 
> Who are the other users of the EMC clk? Why does the EMC driver and the
> clk driver need to be in sync in such a way that requires us to check if
> some code in the EMC driver is ready before we can hand out clks from
> this clk provider? It looks like some tight coupling between these two
> pieces of code on the clk_get() path which is setting off alarms for me.

The other EMC clk user is the devfreq driver. We don't want the devfreq
driver to get and use the EMC clock if memory timing can't be programmed.

>>
>> Please see tegra20_clk_src_onecell_get() changes in this patch and
>> drivers/memory/tegra/tegra20-emc.c for clarity.
>>
>> It is not possible to register clk from the EMC driver without having
>> some custom integration with the clk-core because CLK and EMC are
>> different hardware units and hence EMC driver doesn't have direct access
>> to the CLK registers. I think it is better to keep CLK and memory-timing
>> programming logically separated simply for consistency, although I'm
>> open to suggestions.
> 
> Sorry, I'm really confused and it's probably because I'm not taking the
> time to read all the Tegra code right now. I'm trying to understand the
> overall memory controller frequency logic.

No problems, sorry for the confusion.

> I understand that there's a memory controller, and a clk controller, and
> they're different hardware blocks. Presumably the clk controller can't
> change frequencies without informing the memory controller that it's
> going to change something so the memory controller can fix timings in
> the EMC register space.

That is correct.

> Is there some other entity that's needing to be deferred while the EMC
> and clk drivers tell each other that they're probed and ready so that
> these notifiers all work? Is that entity using clks directly instead of
> going through the EMC to change frequencies? If so, why can't we funnel
> that logic through EMC which then goes to the clk driver? That way it's
> clearly "thing that changes frequency" -> EMC -> clk controller call
> chain. It might even allow us to get rid of the rate notifiers?
> 

That entity is the devfreq driver which uses EMC clk directly right now.

	https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/devfreq/tegra-devfreq.c#n484

I think we can funnel all the rate-changing logic through the EMC
drivers using the PM memory bandwidth QoS API, which is currently
unsupported by the drivers. And then indeed the rate notifiers won't be
needed.

	https://github.com/grate-driver/linux/commit/2aa2390dd0cb8bca98e4cea6a29d7bda64a51bb3

Actually it will be the next step to wire up the PM QoS support to all
relevant drivers that I'm going to implement after the current
CLK/EMC/DEVFREQ patch sets will land, it's just not realistic to do
everything in a single hop.

Re: [PATCH v2 1/4] clk: tegra20/30: Add custom EMC clock implementation

[Stephen Boyd]

Quoting Dmitry Osipenko (2019-04-25 17:03:11)
> 26.04.2019 1:25, Stephen Boyd пишет:
> > Quoting Dmitry Osipenko (2019-04-25 14:42:19)
> >> 25.04.2019 22:07, Stephen Boyd пишет:
> >>> Quoting Dmitry Osipenko (2019-04-14 13:20:06)
> >>>> +{
> >>>> +       struct clk *clk = __clk_lookup("emc");
> >>>> +       struct tegra_clk_emc *emc;
> >>>> +       struct clk_hw *hw;
> >>>> +
> >>>> +       if (clk) {
> >>>> +               hw = __clk_get_hw(clk);
> >>>> +               emc = to_tegra_clk_emc(hw);
> >>>> +
> >>>> +               emc->round_cb = round_cb;
> >>>> +               emc->arg_cb = arg_cb;
> >>>> +       }
> >>>> +}
> >>>> +
> >>>> +bool tegra20_clk_emc_driver_available(void)
> >>>> +{
> >>>> +       struct clk *clk = __clk_lookup("emc");
> >>>
> >>> Can we avoid using __clk_lookup()? Maybe by having the clk_hw pointer in
> >>> this driver somehow?
> >>
> >> tegra20_clk_emc_driver_available() is a private function of the Tegra's
> >> clk-core. Please note that prototype of this function is added to the
> >> local "drivers/clk/tegra/clk.h" file.
> >>
> >> It is indeed possible to use clk pointer instead of __clk_lookup() and
> >> I'll change that in v3 since it's causing some confusion.
> > 
> > Can you use a clk_hw pointer directly? I'd like to see clk provider
> > drivers only use struct clk_hw, and clk consumers only use struct clk.
> 
> The clk_hw pointers are not stored directly anywhere in the Tegra's
> clk-driver, it will be a quite massive change to make the clk driver's
> code to use clk_hw pointers instead of struct clk.

Well they must be stored in the clk driver somewhere because that's how
the whole clk framework works.

> 
> I can add a global variable for the tegra_clk_emc pointer, but it is
> not really necessary since there is __clk_get_hw.
> 
> Could you please explain what's the problem with __clk_get_hw?

If it's a large invasive change then no worries. The thin that's wrong
with __clk_get_hw() is that it's using a struct clk, meaing that most
likely this is a clk consumer driver doing something weird.

> 
> >>
> >>>> +       struct tegra_clk_emc *emc; +       struct clk_hw *hw; + +
> >>>> if (clk) { +               hw = __clk_get_hw(clk);
> >>>
> >>> This gets further to the point, we don't prefer to see drivers use
> >>> __clk_get_hw() unless they absolutely need to. Maybe I don't
> >>> understand the driver structure, but it seems like maybe the
> >>> driver that's providing the callbacks could be the same driver
> >>> that's registering these clks, and thus everything could be inside
> >>> one file so that we don't have to pass around callbacks and clk_hw
> >>> pointers? Commit text says "this is how it's been" but that's not
> >>> a reason to keep doing it.
> >>
> >> Again, tegra20_clk_emc_driver_available() is for the Terga's
> >> clk-core and not for the EMC (External Memory Controller) driver.
> >> This function is used to determine whether EMC driver is ready to
> >> handle clock-rate changes (PRE/POST rate-change notifications), clk
> >> users can't get EMC clock until driver is ready (i.e. the "round"
> >> callback is registered by the driver).
> > 
> > Who are the other users of the EMC clk? Why does the EMC driver and
> > the clk driver need to be in sync in such a way that requires us to
> > check if some code in the EMC driver is ready before we can hand out
> > clks from this clk provider? It looks like some tight coupling
> > between these two pieces of code on the clk_get() path which is
> > setting off alarms for me.
> 
> The other EMC clk user is the devfreq driver. We don't want the
> devfreq driver to get and use the EMC clock if memory timing can't be
> programmed.

Alright, got it!

> That is correct.
> 
> > Is there some other entity that's needing to be deferred while the
> > EMC and clk drivers tell each other that they're probed and ready so
> > that these notifiers all work? Is that entity using clks directly
> > instead of going through the EMC to change frequencies? If so, why
> > can't we funnel that logic through EMC which then goes to the clk
> > driver? That way it's clearly "thing that changes frequency" -> EMC
> > -> clk controller call chain. It might even allow us to get rid of
> > the rate notifiers?
> > 
> 
> That entity is the devfreq driver which uses EMC clk directly right
> now.
> 
>         https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/devfreq/tegra-devfreq.c#n484
> 
> I think we can funnel all the rate-changing logic through the EMC
> drivers using the PM memory bandwidth QoS API, which is currently
> unsupported by the drivers. And then indeed the rate notifiers won't
> be needed.

Why does PM QoS matter here? I'm mostly asking for the chain to be
stacked and the devfreq driver to be the same as the EMC driver and then
have the EMC/devfreq combo driver use clk APIs to set rates and do
things before and after.

> 
>         https://github.com/grate-driver/linux/commit/2aa2390dd0cb8bca98e4cea6a29d7bda64a51bb3
> 
> Actually it will be the next step to wire up the PM QoS support to all
> relevant drivers that I'm going to implement after the current
> CLK/EMC/DEVFREQ patch sets will land, it's just not realistic to do
> everything in a single hop.

Ok. It doesn't make me feel great but I suppose if you don't disappear
after this code is merged things will work out in the end.

Re: [PATCH v2 1/4] clk: tegra20/30: Add custom EMC clock implementation

[Dmitry Osipenko]

26.04.2019 3:41, Stephen Boyd пишет:
> Quoting Dmitry Osipenko (2019-04-25 17:03:11)
>> 26.04.2019 1:25, Stephen Boyd пишет:
>>> Quoting Dmitry Osipenko (2019-04-25 14:42:19)
>>>> 25.04.2019 22:07, Stephen Boyd пишет:
>>>>> Quoting Dmitry Osipenko (2019-04-14 13:20:06)
>>>>>> +{
>>>>>> +       struct clk *clk = __clk_lookup("emc");
>>>>>> +       struct tegra_clk_emc *emc;
>>>>>> +       struct clk_hw *hw;
>>>>>> +
>>>>>> +       if (clk) {
>>>>>> +               hw = __clk_get_hw(clk);
>>>>>> +               emc = to_tegra_clk_emc(hw);
>>>>>> +
>>>>>> +               emc->round_cb = round_cb;
>>>>>> +               emc->arg_cb = arg_cb;
>>>>>> +       }
>>>>>> +}
>>>>>> +
>>>>>> +bool tegra20_clk_emc_driver_available(void)
>>>>>> +{
>>>>>> +       struct clk *clk = __clk_lookup("emc");
>>>>>
>>>>> Can we avoid using __clk_lookup()? Maybe by having the clk_hw pointer in
>>>>> this driver somehow?
>>>>
>>>> tegra20_clk_emc_driver_available() is a private function of the Tegra's
>>>> clk-core. Please note that prototype of this function is added to the
>>>> local "drivers/clk/tegra/clk.h" file.
>>>>
>>>> It is indeed possible to use clk pointer instead of __clk_lookup() and
>>>> I'll change that in v3 since it's causing some confusion.
>>>
>>> Can you use a clk_hw pointer directly? I'd like to see clk provider
>>> drivers only use struct clk_hw, and clk consumers only use struct clk.
>>
>> The clk_hw pointers are not stored directly anywhere in the Tegra's
>> clk-driver, it will be a quite massive change to make the clk driver's
>> code to use clk_hw pointers instead of struct clk.
> 
> Well they must be stored in the clk driver somewhere because that's how
> the whole clk framework works.

AFAIK, Tegra stores them only in a form of clk->core->hw.

>>
>> I can add a global variable for the tegra_clk_emc pointer, but it is
>> not really necessary since there is __clk_get_hw.
>>
>> Could you please explain what's the problem with __clk_get_hw?
> 
> If it's a large invasive change then no worries. The thin that's wrong
> with __clk_get_hw() is that it's using a struct clk, meaing that most
> likely this is a clk consumer driver doing something weird.
> 
>>
>>>>
>>>>>> +       struct tegra_clk_emc *emc; +       struct clk_hw *hw; + +
>>>>>> if (clk) { +               hw = __clk_get_hw(clk);
>>>>>
>>>>> This gets further to the point, we don't prefer to see drivers use
>>>>> __clk_get_hw() unless they absolutely need to. Maybe I don't
>>>>> understand the driver structure, but it seems like maybe the
>>>>> driver that's providing the callbacks could be the same driver
>>>>> that's registering these clks, and thus everything could be inside
>>>>> one file so that we don't have to pass around callbacks and clk_hw
>>>>> pointers? Commit text says "this is how it's been" but that's not
>>>>> a reason to keep doing it.
>>>>
>>>> Again, tegra20_clk_emc_driver_available() is for the Terga's
>>>> clk-core and not for the EMC (External Memory Controller) driver.
>>>> This function is used to determine whether EMC driver is ready to
>>>> handle clock-rate changes (PRE/POST rate-change notifications), clk
>>>> users can't get EMC clock until driver is ready (i.e. the "round"
>>>> callback is registered by the driver).
>>>
>>> Who are the other users of the EMC clk? Why does the EMC driver and
>>> the clk driver need to be in sync in such a way that requires us to
>>> check if some code in the EMC driver is ready before we can hand out
>>> clks from this clk provider? It looks like some tight coupling
>>> between these two pieces of code on the clk_get() path which is
>>> setting off alarms for me.
>>
>> The other EMC clk user is the devfreq driver. We don't want the
>> devfreq driver to get and use the EMC clock if memory timing can't be
>> programmed.
> 
> Alright, got it!
> 
>> That is correct.
>>
>>> Is there some other entity that's needing to be deferred while the
>>> EMC and clk drivers tell each other that they're probed and ready so
>>> that these notifiers all work? Is that entity using clks directly
>>> instead of going through the EMC to change frequencies? If so, why
>>> can't we funnel that logic through EMC which then goes to the clk
>>> driver? That way it's clearly "thing that changes frequency" -> EMC
>>> -> clk controller call chain. It might even allow us to get rid of
>>> the rate notifiers?
>>>
>>
>> That entity is the devfreq driver which uses EMC clk directly right
>> now.
>>
>>         https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/devfreq/tegra-devfreq.c#n484
>>
>> I think we can funnel all the rate-changing logic through the EMC
>> drivers using the PM memory bandwidth QoS API, which is currently
>> unsupported by the drivers. And then indeed the rate notifiers won't
>> be needed.
> 
> Why does PM QoS matter here? I'm mostly asking for the chain to be
> stacked and the devfreq driver to be the same as the EMC driver and then
> have the EMC/devfreq combo driver use clk APIs to set rates and do
> things before and after.

It could be implemented either way, the result will be the same. The
final decision probably will be made after wiring up the PM QoS support
and doing some more testing. Also note that EMC and devfreq are actually
separate hardware blocks that are integrated.

So it all could look like this (current variant that in works):

 +-------+   PM QoS   +-+
 |DEVICE1+----------->+E|    CLK API
 +-------+   PM QoS   |M+<----------+   +-+
             +------->+C|           +--->C|
             |        +-+               |L|
 +-------+   |                       +-->K|
 |DEVICE2+---+    +-------+  CLK API |  +-+
 +-------+        |DEVFREQ+<---------+
                  +-------+

And like this:

+-------+ PM QoS  +-+
|DEVICE1+---------+ |
+-------+         | |       +-+
+-------+ PM QoS  |E|  CLK  |C|
|DEVICE2+-------->+M+<----->+L|
+-------+         |C|  API  |K|
+-------+ PM QoS  | |       +++
|DEVFREQ+---------+ |        |
+---+---+         +-+        |
    ^                        |
    +------------------------+
             CLK API

Or like this:

+-------+ PM QoS  +-+
|DEVICE1+---------+ |
+-------+         | |       +-+
+-------+ PM QoS  |E|  CLK  |C|
|DEVICE2+-------->+M+<----->+L|
+-------+         |C|  API  |K|
+-------+ PM QoS  | |       +-+
|DEVFREQ+---------+ |
+---+---+         +++
    ^              |
    +--------------+
         PM QoS

As you can see there will be other drivers that will demand memory
bandwidth too. For example the display controller performs isochronous
memory transfers and hence may require higher bandwidth than devfreq
suggests because devfreq judges the bandwidth demand by taking average
of bandwidth consumption over a period of time.

The other thing is that drivers should be abstracted from the knowledge
about the EMC because:

  a) memory/device hardware units do not know about each other

  b) it is common to express memory bandwidth in Mbs and not in a clock rate

It's only the devfreq driver that is a bit special because devfreq
hardware is actually integrated with the EMC.

>>
>>         https://github.com/grate-driver/linux/commit/2aa2390dd0cb8bca98e4cea6a29d7bda64a51bb3
>>
>> Actually it will be the next step to wire up the PM QoS support to all
>> relevant drivers that I'm going to implement after the current
>> CLK/EMC/DEVFREQ patch sets will land, it's just not realistic to do
>> everything in a single hop.
> 
> Ok. It doesn't make me feel great but I suppose if you don't disappear
> after this code is merged things will work out in the end.
> 

Correct.

[PATCH v2 2/4] dt-bindings: memory: Add binding for NVIDIA Tegra30 External Memory Controller

[Dmitry Osipenko]

Add device-tree binding for NVIDIA Tegra30 External Memory Controller.
The binding is based on the Tegra124 EMC binding since hardware is
similar, although there are couple significant differences.

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

diff --git a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-emc.txt b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-emc.txt
new file mode 100644
index 000000000000..dffe396c5d79
--- /dev/null
+++ b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-emc.txt
@@ -0,0 +1,257 @@
+NVIDIA Tegra30 SoC EMC (external memory controller)
+====================================================
+
+Required properties :
+- compatible : Should be "nvidia,tegra30-emc".
+- reg : physical base address and length of the controller's registers.
+- #address-cells : Should be 1
+- #size-cells : Should be 0
+- interrupts : Should contain EMC General interrupt.
+- clocks : Should contain EMC clock.
+- nvidia,memory-controller : phandle of the MC driver.
+
+The node should contain a "emc-timings" subnode for each supported RAM type
+(see field RAM_CODE in register PMC_STRAPPING_OPT_A), with its unit address
+being its RAM_CODE.
+
+Required properties for "emc-timings" nodes :
+- nvidia,ram-code : Should contain the value of RAM_CODE this timing set is
+used for.
+
+Each "emc-timings" node should contain a "timing" subnode for every supported
+EMC clock rate. The "timing" subnodes should have the clock rate in Hz as
+their unit address.
+
+Required properties for "timing" nodes :
+- clock-frequency : Should contain the memory clock rate in Hz.
+- The following properties contain EMC timing characterization values
+(specified in the board documentation) :
+  - nvidia,emc-auto-cal-interval : EMC_AUTO_CAL_INTERVAL
+  - nvidia,emc-mode-1 : Mode Register 1
+  - nvidia,emc-mode-2 : Mode Register 2
+  - nvidia,emc-mode-reset : Mode Register 0
+  - nvidia,emc-zcal-cnt-long : EMC_ZCAL_WAIT_CNT after clock change
+  - nvidia,emc-cfg-dyn-self-ref : dynamic self-refresh enabled
+  - nvidia,emc-cfg-periodic-qrst : FBIO "read" FIFO periodic resetting enabled
+- nvidia,emc-configuration : EMC timing characterization data. These are the
+registers (see section "18.13.2 EMC Registers" in the TRM) whose values need to
+be specified, according to the board documentation:
+
+	EMC_RC
+	EMC_RFC
+	EMC_RAS
+	EMC_RP
+	EMC_R2W
+	EMC_W2R
+	EMC_R2P
+	EMC_W2P
+	EMC_RD_RCD
+	EMC_WR_RCD
+	EMC_RRD
+	EMC_REXT
+	EMC_WEXT
+	EMC_WDV
+	EMC_QUSE
+	EMC_QRST
+	EMC_QSAFE
+	EMC_RDV
+	EMC_REFRESH
+	EMC_BURST_REFRESH_NUM
+	EMC_PRE_REFRESH_REQ_CNT
+	EMC_PDEX2WR
+	EMC_PDEX2RD
+	EMC_PCHG2PDEN
+	EMC_ACT2PDEN
+	EMC_AR2PDEN
+	EMC_RW2PDEN
+	EMC_TXSR
+	EMC_TXSRDLL
+	EMC_TCKE
+	EMC_TFAW
+	EMC_TRPAB
+	EMC_TCLKSTABLE
+	EMC_TCLKSTOP
+	EMC_TREFBW
+	EMC_QUSE_EXTRA
+	EMC_FBIO_CFG6
+	EMC_ODT_WRITE
+	EMC_ODT_READ
+	EMC_FBIO_CFG5
+	EMC_CFG_DIG_DLL
+	EMC_CFG_DIG_DLL_PERIOD
+	EMC_DLL_XFORM_DQS0
+	EMC_DLL_XFORM_DQS1
+	EMC_DLL_XFORM_DQS2
+	EMC_DLL_XFORM_DQS3
+	EMC_DLL_XFORM_DQS4
+	EMC_DLL_XFORM_DQS5
+	EMC_DLL_XFORM_DQS6
+	EMC_DLL_XFORM_DQS7
+	EMC_DLL_XFORM_QUSE0
+	EMC_DLL_XFORM_QUSE1
+	EMC_DLL_XFORM_QUSE2
+	EMC_DLL_XFORM_QUSE3
+	EMC_DLL_XFORM_QUSE4
+	EMC_DLL_XFORM_QUSE5
+	EMC_DLL_XFORM_QUSE6
+	EMC_DLL_XFORM_QUSE7
+	EMC_DLI_TRIM_TXDQS0
+	EMC_DLI_TRIM_TXDQS1
+	EMC_DLI_TRIM_TXDQS2
+	EMC_DLI_TRIM_TXDQS3
+	EMC_DLI_TRIM_TXDQS4
+	EMC_DLI_TRIM_TXDQS5
+	EMC_DLI_TRIM_TXDQS6
+	EMC_DLI_TRIM_TXDQS7
+	EMC_DLL_XFORM_DQ0
+	EMC_DLL_XFORM_DQ1
+	EMC_DLL_XFORM_DQ2
+	EMC_DLL_XFORM_DQ3
+	EMC_XM2CMDPADCTRL
+	EMC_XM2DQSPADCTRL2
+	EMC_XM2DQPADCTRL2
+	EMC_XM2CLKPADCTRL
+	EMC_XM2COMPPADCTRL
+	EMC_XM2VTTGENPADCTRL
+	EMC_XM2VTTGENPADCTRL2
+	EMC_XM2QUSEPADCTRL
+	EMC_XM2DQSPADCTRL3
+	EMC_CTT_TERM_CTRL
+	EMC_ZCAL_INTERVAL
+	EMC_ZCAL_WAIT_CNT
+	EMC_MRS_WAIT_CNT
+	EMC_AUTO_CAL_CONFIG
+	EMC_CTT
+	EMC_CTT_DURATION
+	EMC_DYN_SELF_REF_CONTROL
+	EMC_FBIO_SPARE
+	EMC_CFG_RSV
+
+Example SoC include file:
+
+/ {
+	emc@7000f400 {
+		compatible = "nvidia,tegra30-emc";
+		reg = <0x7000f400 0x400>;
+		interrupts = <GIC_SPI 78 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&tegra_car TEGRA30_CLK_EMC>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		nvidia,memory-controller = <&mc>;
+	};
+};
+
+Example board file:
+
+/ {
+	emc@7000f400 {
+		emc-timings-1 {
+			nvidia,ram-code = <1>;
+
+			timing-667000000 {
+				clock-frequency = <667000000>;
+
+				nvidia,emc-auto-cal-interval = <0x001fffff>;
+				nvidia,emc-mode-1 = <0x80100002>;
+				nvidia,emc-mode-2 = <0x80200018>;
+				nvidia,emc-mode-reset = <0x80000b71>;
+				nvidia,emc-zcal-cnt-long = <0x00000040>;
+				nvidia,emc-cfg-dyn-self-ref = <0x00000000>;
+				nvidia,emc-cfg-periodic-qrst = <0x00000001>;
+
+				nvidia,emc-configuration = <
+					0x00000020 /* EMC_RC */
+					0x0000006a /* EMC_RFC */
+					0x00000017 /* EMC_RAS */
+					0x00000007 /* EMC_RP */
+					0x00000005 /* EMC_R2W */
+					0x0000000c /* EMC_W2R */
+					0x00000003 /* EMC_R2P */
+					0x00000011 /* EMC_W2P */
+					0x00000007 /* EMC_RD_RCD */
+					0x00000007 /* EMC_WR_RCD */
+					0x00000002 /* EMC_RRD */
+					0x00000001 /* EMC_REXT */
+					0x00000000 /* EMC_WEXT */
+					0x00000007 /* EMC_WDV */
+					0x0000000a /* EMC_QUSE */
+					0x00000009 /* EMC_QRST */
+					0x0000000b /* EMC_QSAFE */
+					0x00000011 /* EMC_RDV */
+					0x00001412 /* EMC_REFRESH */
+					0x00000000 /* EMC_BURST_REFRESH_NUM */
+					0x00000504 /* EMC_PRE_REFRESH_REQ_CNT */
+					0x00000002 /* EMC_PDEX2WR */
+					0x0000000e /* EMC_PDEX2RD */
+					0x00000001 /* EMC_PCHG2PDEN */
+					0x00000000 /* EMC_ACT2PDEN */
+					0x0000000c /* EMC_AR2PDEN */
+					0x00000016 /* EMC_RW2PDEN */
+					0x00000072 /* EMC_TXSR */
+					0x00000200 /* EMC_TXSRDLL */
+					0x00000005 /* EMC_TCKE */
+					0x00000015 /* EMC_TFAW */
+					0x00000000 /* EMC_TRPAB */
+					0x00000006 /* EMC_TCLKSTABLE */
+					0x00000007 /* EMC_TCLKSTOP */
+					0x00001453 /* EMC_TREFBW */
+					0x0000000b /* EMC_QUSE_EXTRA */
+					0x00000006 /* EMC_FBIO_CFG6 */
+					0x00000000 /* EMC_ODT_WRITE */
+					0x00000000 /* EMC_ODT_READ */
+					0x00005088 /* EMC_FBIO_CFG5 */
+					0xf00b0191 /* EMC_CFG_DIG_DLL */
+					0x00008000 /* EMC_CFG_DIG_DLL_PERIOD */
+					0x00000008 /* EMC_DLL_XFORM_DQS0 */
+					0x00000008 /* EMC_DLL_XFORM_DQS1 */
+					0x00000008 /* EMC_DLL_XFORM_DQS2 */
+					0x00000008 /* EMC_DLL_XFORM_DQS3 */
+					0x0000000a /* EMC_DLL_XFORM_DQS4 */
+					0x0000000a /* EMC_DLL_XFORM_DQS5 */
+					0x0000000a /* EMC_DLL_XFORM_DQS6 */
+					0x0000000a /* EMC_DLL_XFORM_DQS7 */
+					0x00018000 /* EMC_DLL_XFORM_QUSE0 */
+					0x00018000 /* EMC_DLL_XFORM_QUSE1 */
+					0x00018000 /* EMC_DLL_XFORM_QUSE2 */
+					0x00018000 /* EMC_DLL_XFORM_QUSE3 */
+					0x00000000 /* EMC_DLL_XFORM_QUSE4 */
+					0x00000000 /* EMC_DLL_XFORM_QUSE5 */
+					0x00000000 /* EMC_DLL_XFORM_QUSE6 */
+					0x00000000 /* EMC_DLL_XFORM_QUSE7 */
+					0x00000000 /* EMC_DLI_TRIM_TXDQS0 */
+					0x00000000 /* EMC_DLI_TRIM_TXDQS1 */
+					0x00000000 /* EMC_DLI_TRIM_TXDQS2 */
+					0x00000000 /* EMC_DLI_TRIM_TXDQS3 */
+					0x00000000 /* EMC_DLI_TRIM_TXDQS4 */
+					0x00000000 /* EMC_DLI_TRIM_TXDQS5 */
+					0x00000000 /* EMC_DLI_TRIM_TXDQS6 */
+					0x00000000 /* EMC_DLI_TRIM_TXDQS7 */
+					0x0000000a /* EMC_DLL_XFORM_DQ0 */
+					0x0000000a /* EMC_DLL_XFORM_DQ1 */
+					0x0000000a /* EMC_DLL_XFORM_DQ2 */
+					0x0000000a /* EMC_DLL_XFORM_DQ3 */
+					0x000002a0 /* EMC_XM2CMDPADCTRL */
+					0x0800013d /* EMC_XM2DQSPADCTRL2 */
+					0x22220000 /* EMC_XM2DQPADCTRL2 */
+					0x77fff884 /* EMC_XM2CLKPADCTRL */
+					0x01f1f501 /* EMC_XM2COMPPADCTRL */
+					0x07077404 /* EMC_XM2VTTGENPADCTRL */
+					0x54000000 /* EMC_XM2VTTGENPADCTRL2 */
+					0x080001e8 /* EMC_XM2QUSEPADCTRL */
+					0x0c000021 /* EMC_XM2DQSPADCTRL3 */
+					0x00000802 /* EMC_CTT_TERM_CTRL */
+					0x00020000 /* EMC_ZCAL_INTERVAL */
+					0x00000100 /* EMC_ZCAL_WAIT_CNT */
+					0x0155000c /* EMC_MRS_WAIT_CNT */
+					0xa0f10000 /* EMC_AUTO_CAL_CONFIG */
+					0x00000000 /* EMC_CTT */
+					0x00000000 /* EMC_CTT_DURATION */
+					0x800028a5 /* EMC_DYN_SELF_REF_CONTROL */
+					0xe8000000 /* EMC_FBIO_SPARE */
+					0xff00ff49 /* EMC_CFG_RSV */
+				>;
+			};
+		};
+	};
+};
-- 
2.21.0

Re: [PATCH v2 2/4] dt-bindings: memory: Add binding for NVIDIA Tegra30 External Memory Controller

[Rob Herring]

On Sun, Apr 14, 2019 at 11:20:07PM +0300, Dmitry Osipenko wrote:
> Add device-tree binding for NVIDIA Tegra30 External Memory Controller.
> The binding is based on the Tegra124 EMC binding since hardware is
> similar, although there are couple significant differences.

My comments on Tegra124 binding apply here.

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

Re: [PATCH v2 2/4] dt-bindings: memory: Add binding for NVIDIA Tegra30 External Memory Controller

[Dmitry Osipenko]

30.04.2019 1:05, Rob Herring пишет:
> On Sun, Apr 14, 2019 at 11:20:07PM +0300, Dmitry Osipenko wrote:
>> Add device-tree binding for NVIDIA Tegra30 External Memory Controller.
>> The binding is based on the Tegra124 EMC binding since hardware is
>> similar, although there are couple significant differences.
> 
> My comments on Tegra124 binding apply here.

The common timing definition doesn't fully match the definition that is
used by Tegra's Memory Controller, thus the DQS (data strobe) timing
parameter is comprised of multiple sub-parameters that describe how to
generate the strobe in hardware. There are also more additional
parameters that are specific to Tegra and they are individually
characterized for each memory model and clock rate. Hence the common
timing definition isn't usable.

Re: [PATCH v2 2/4] dt-bindings: memory: Add binding for NVIDIA Tegra30 External Memory Controller

[Rob Herring]

On Wed, May 1, 2019 at 7:06 PM Dmitry Osipenko <digetx@gmail.com> wrote:
>
> 30.04.2019 1:05, Rob Herring пишет:
> > On Sun, Apr 14, 2019 at 11:20:07PM +0300, Dmitry Osipenko wrote:
> >> Add device-tree binding for NVIDIA Tegra30 External Memory Controller.
> >> The binding is based on the Tegra124 EMC binding since hardware is
> >> similar, although there are couple significant differences.
> >
> > My comments on Tegra124 binding apply here.
>
> The common timing definition doesn't fully match the definition that is
> used by Tegra's Memory Controller, thus the DQS (data strobe) timing
> parameter is comprised of multiple sub-parameters that describe how to
> generate the strobe in hardware. There are also more additional
> parameters that are specific to Tegra and they are individually
> characterized for each memory model and clock rate. Hence the common
> timing definition isn't usable.

I don't understand. Every PC in the world can work with any DIMM
(within a given generation) just with SPD data. Why is that not
sufficient here?

In any case, it seems for Tegra124 a different approach is going to be
taken. Seems like an "avoid DT" solution to me, but if it's contained
within the firmware it's not my problem.

Rob

Re: [PATCH v2 2/4] dt-bindings: memory: Add binding for NVIDIA Tegra30 External Memory Controller

[Dmitry Osipenko]

02.05.2019 3:17, Rob Herring пишет:
> On Wed, May 1, 2019 at 7:06 PM Dmitry Osipenko <digetx@gmail.com> wrote:
>>
>> 30.04.2019 1:05, Rob Herring пишет:
>>> On Sun, Apr 14, 2019 at 11:20:07PM +0300, Dmitry Osipenko wrote:
>>>> Add device-tree binding for NVIDIA Tegra30 External Memory Controller.
>>>> The binding is based on the Tegra124 EMC binding since hardware is
>>>> similar, although there are couple significant differences.
>>>
>>> My comments on Tegra124 binding apply here.
>>
>> The common timing definition doesn't fully match the definition that is
>> used by Tegra's Memory Controller, thus the DQS (data strobe) timing
>> parameter is comprised of multiple sub-parameters that describe how to
>> generate the strobe in hardware. There are also more additional
>> parameters that are specific to Tegra and they are individually
>> characterized for each memory model and clock rate. Hence the common
>> timing definition isn't usable.
> 
> I don't understand. Every PC in the world can work with any DIMM
> (within a given generation) just with SPD data. Why is that not
> sufficient here?

Because this is not a standard PC, but a custom embedded hardware that
is simpler and also doesn't fully follow the standards in some cases.

> In any case, it seems for Tegra124 a different approach is going to be
> taken. Seems like an "avoid DT" solution to me, but if it's contained
> within the firmware it's not my problem.

My above comment really applies to all Terga's.

The Tegra210 is also a bit more complicated case because of the
proprietary signed firmware that can't be easily replaced with
opensource alternative without special hacks, but AFAIK the unofficial
opensource firmware will be available in some form for at least one
consumer device (Nintendo Switch).

Please write a detailed comment to the Tegra210's patch, saying what you
would want to see changed. I'm sure Joseph will try to do his best.

Note that it is always possible to define a proper device tree binding
and then also "unofficially" support the downstream binding, IIRC that's
what some drivers are already doing in upstream kernel. So I think you
could just demand for the proper binding regardless of the firmware
situation.

Re: [PATCH v2 2/4] dt-bindings: memory: Add binding for NVIDIA Tegra30 External Memory Controller

[Dmitry Osipenko]

02.05.2019 3:52, Dmitry Osipenko пишет:
> 02.05.2019 3:17, Rob Herring пишет:
>> On Wed, May 1, 2019 at 7:06 PM Dmitry Osipenko <digetx@gmail.com> wrote:
>>>
>>> 30.04.2019 1:05, Rob Herring пишет:
>>>> On Sun, Apr 14, 2019 at 11:20:07PM +0300, Dmitry Osipenko wrote:
>>>>> Add device-tree binding for NVIDIA Tegra30 External Memory Controller.
>>>>> The binding is based on the Tegra124 EMC binding since hardware is
>>>>> similar, although there are couple significant differences.
>>>>
>>>> My comments on Tegra124 binding apply here.
>>>
>>> The common timing definition doesn't fully match the definition that is
>>> used by Tegra's Memory Controller, thus the DQS (data strobe) timing
>>> parameter is comprised of multiple sub-parameters that describe how to
>>> generate the strobe in hardware. There are also more additional
>>> parameters that are specific to Tegra and they are individually
>>> characterized for each memory model and clock rate. Hence the common
>>> timing definition isn't usable.
>>
>> I don't understand. Every PC in the world can work with any DIMM
>> (within a given generation) just with SPD data. Why is that not
>> sufficient here?
> 
> Because this is not a standard PC, but a custom embedded hardware that
> is simpler and also doesn't fully follow the standards in some cases.

Even if there is a way to derive at least some of required parameters
from the common timing, I don't have information about how to do it and
there is pretty much no chance to get it into public.

Rob, please let me know if you're okay with this binding.

Re: [PATCH v2 2/4] dt-bindings: memory: Add binding for NVIDIA Tegra30 External Memory Controller

[Dmitry Osipenko]

02.05.2019 4:39, Dmitry Osipenko пишет:
> 02.05.2019 3:52, Dmitry Osipenko пишет:
>> 02.05.2019 3:17, Rob Herring пишет:
>>> On Wed, May 1, 2019 at 7:06 PM Dmitry Osipenko <digetx@gmail.com> wrote:
>>>>
>>>> 30.04.2019 1:05, Rob Herring пишет:
>>>>> On Sun, Apr 14, 2019 at 11:20:07PM +0300, Dmitry Osipenko wrote:
>>>>>> Add device-tree binding for NVIDIA Tegra30 External Memory Controller.
>>>>>> The binding is based on the Tegra124 EMC binding since hardware is
>>>>>> similar, although there are couple significant differences.
>>>>>
>>>>> My comments on Tegra124 binding apply here.
>>>>
>>>> The common timing definition doesn't fully match the definition that is
>>>> used by Tegra's Memory Controller, thus the DQS (data strobe) timing
>>>> parameter is comprised of multiple sub-parameters that describe how to
>>>> generate the strobe in hardware. There are also more additional
>>>> parameters that are specific to Tegra and they are individually
>>>> characterized for each memory model and clock rate. Hence the common
>>>> timing definition isn't usable.
>>>
>>> I don't understand. Every PC in the world can work with any DIMM
>>> (within a given generation) just with SPD data. Why is that not
>>> sufficient here?
>>
>> Because this is not a standard PC, but a custom embedded hardware that
>> is simpler and also doesn't fully follow the standards in some cases.
> 
> Even if there is a way to derive at least some of required parameters
> from the common timing, I don't have information about how to do it and
> there is pretty much no chance to get it into public.
> 
> Rob, please let me know if you're okay with this binding.
> 

I looked through the Technical Reference Manual a bit more closely and
it actually has comments about how to convert some of the common timing
parameters into register values. Some of those parameters are not
trivial to convert and the doc could be incomplete, and some of the
parameters are not documented at all.

The other much more minor reason against the common timing description
is that all boards come with the memory timing definition in a
platform-specific form in downstream kernel. We're re-using that
definition in the device-tree, hence it is easy to port the timings from
downstream into upstream.

[PATCH v2 3/4] memory: tegra: Introduce Tegra30 EMC driver

[Dmitry Osipenko]

Introduce driver for the External Memory Controller (EMC) found on Tegra30
chips, it controls the external DRAM on the board. The purpose of this
driver is to program memory timing for external memory on the EMC clock
rate change.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/memory/tegra/Kconfig       |   10 +
 drivers/memory/tegra/Makefile      |    1 +
 drivers/memory/tegra/mc.c          |    3 -
 drivers/memory/tegra/mc.h          |   30 +-
 drivers/memory/tegra/tegra30-emc.c | 1159 ++++++++++++++++++++++++++++
 drivers/memory/tegra/tegra30.c     |   44 ++
 6 files changed, 1235 insertions(+), 12 deletions(-)
 create mode 100644 drivers/memory/tegra/tegra30-emc.c

diff --git a/drivers/memory/tegra/Kconfig b/drivers/memory/tegra/Kconfig
index 34e0b70f5c5f..89c02a368c65 100644
--- a/drivers/memory/tegra/Kconfig
+++ b/drivers/memory/tegra/Kconfig
@@ -16,6 +16,16 @@ config TEGRA20_EMC
 	  This driver is required to change memory timings / clock rate for
 	  external memory.
 
+config TEGRA30_EMC
+	bool "NVIDIA Tegra30 External Memory Controller driver"
+	default y
+	depends on TEGRA_MC && ARCH_TEGRA_3x_SOC
+	help
+	  This driver is for the External Memory Controller (EMC) found on
+	  Tegra30 chips. The EMC controls the external DRAM on the board.
+	  This driver is required to change memory timings / clock rate for
+	  external memory.
+
 config TEGRA124_EMC
 	bool "NVIDIA Tegra124 External Memory Controller driver"
 	default y
diff --git a/drivers/memory/tegra/Makefile b/drivers/memory/tegra/Makefile
index 3971a6b7c487..3d23c4261104 100644
--- a/drivers/memory/tegra/Makefile
+++ b/drivers/memory/tegra/Makefile
@@ -11,5 +11,6 @@ tegra-mc-$(CONFIG_ARCH_TEGRA_210_SOC) += tegra210.o
 obj-$(CONFIG_TEGRA_MC) += tegra-mc.o
 
 obj-$(CONFIG_TEGRA20_EMC)  += tegra20-emc.o
+obj-$(CONFIG_TEGRA30_EMC)  += tegra30-emc.o
 obj-$(CONFIG_TEGRA124_EMC) += tegra124-emc.o
 obj-$(CONFIG_ARCH_TEGRA_186_SOC) += tegra186.o
diff --git a/drivers/memory/tegra/mc.c b/drivers/memory/tegra/mc.c
index 31b47459c84d..52ede43f1e07 100644
--- a/drivers/memory/tegra/mc.c
+++ b/drivers/memory/tegra/mc.c
@@ -51,9 +51,6 @@
 #define MC_EMEM_ADR_CFG 0x54
 #define MC_EMEM_ADR_CFG_EMEM_NUMDEV BIT(0)
 
-#define MC_TIMING_CONTROL		0xfc
-#define MC_TIMING_UPDATE		BIT(0)
-
 static const struct of_device_id tegra_mc_of_match[] = {
 #ifdef CONFIG_ARCH_TEGRA_2x_SOC
 	{ .compatible = "nvidia,tegra20-mc-gart", .data = &tegra20_mc_soc },
diff --git a/drivers/memory/tegra/mc.h b/drivers/memory/tegra/mc.h
index 887a3b07334f..90da9f62ce2b 100644
--- a/drivers/memory/tegra/mc.h
+++ b/drivers/memory/tegra/mc.h
@@ -9,20 +9,32 @@
 #ifndef MEMORY_TEGRA_MC_H
 #define MEMORY_TEGRA_MC_H
 
+#include <linux/bits.h>
 #include <linux/io.h>
 #include <linux/types.h>
 
 #include <soc/tegra/mc.h>
 
-#define MC_INT_DECERR_MTS (1 << 16)
-#define MC_INT_SECERR_SEC (1 << 13)
-#define MC_INT_DECERR_VPR (1 << 12)
-#define MC_INT_INVALID_APB_ASID_UPDATE (1 << 11)
-#define MC_INT_INVALID_SMMU_PAGE (1 << 10)
-#define MC_INT_ARBITRATION_EMEM (1 << 9)
-#define MC_INT_SECURITY_VIOLATION (1 << 8)
-#define MC_INT_INVALID_GART_PAGE (1 << 7)
-#define MC_INT_DECERR_EMEM (1 << 6)
+#define MC_INT_DECERR_MTS				BIT(16)
+#define MC_INT_SECERR_SEC				BIT(13)
+#define MC_INT_DECERR_VPR				BIT(12)
+#define MC_INT_INVALID_APB_ASID_UPDATE			BIT(11)
+#define MC_INT_INVALID_SMMU_PAGE			BIT(10)
+#define MC_INT_ARBITRATION_EMEM				BIT(9)
+#define MC_INT_SECURITY_VIOLATION			BIT(8)
+#define MC_INT_INVALID_GART_PAGE			BIT(7)
+#define MC_INT_DECERR_EMEM				BIT(6)
+
+#define MC_EMEM_ARB_OUTSTANDING_REQ			0x94
+#define MC_EMEM_ARB_OUTSTANDING_REQ_MAX_MASK		0x1ff
+#define MC_EMEM_ARB_OUTSTANDING_REQ_HOLDOFF_OVERRIDE	BIT(30)
+#define MC_EMEM_ARB_OUTSTANDING_REQ_LIMIT_ENABLE	BIT(31)
+
+#define MC_EMEM_ARB_OVERRIDE				0xe8
+#define MC_EMEM_ARB_OVERRIDE_EACK_MASK			0x3
+
+#define MC_TIMING_CONTROL				0xfc
+#define MC_TIMING_UPDATE				BIT(0)
 
 static inline u32 mc_readl(struct tegra_mc *mc, unsigned long offset)
 {
diff --git a/drivers/memory/tegra/tegra30-emc.c b/drivers/memory/tegra/tegra30-emc.c
new file mode 100644
index 000000000000..09817a3d6a44
--- /dev/null
+++ b/drivers/memory/tegra/tegra30-emc.c
@@ -0,0 +1,1159 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Tegra30 External Memory Controller driver
+ *
+ * Author: Dmitry Osipenko <digetx@gmail.com>
+ */
+
+#include <linux/clk.h>
+#include <linux/clk/tegra.h>
+#include <linux/completion.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/sort.h>
+#include <linux/types.h>
+
+#include <soc/tegra/fuse.h>
+
+#include "mc.h"
+
+#define EMC_INTSTATUS				0x000
+#define EMC_INTMASK				0x004
+#define EMC_DBG					0x008
+#define EMC_CFG					0x00c
+#define EMC_REFCTRL				0x020
+#define EMC_TIMING_CONTROL			0x028
+#define EMC_RC					0x02c
+#define EMC_RFC					0x030
+#define EMC_RAS					0x034
+#define EMC_RP					0x038
+#define EMC_R2W					0x03c
+#define EMC_W2R					0x040
+#define EMC_R2P					0x044
+#define EMC_W2P					0x048
+#define EMC_RD_RCD				0x04c
+#define EMC_WR_RCD				0x050
+#define EMC_RRD					0x054
+#define EMC_REXT				0x058
+#define EMC_WDV					0x05c
+#define EMC_QUSE				0x060
+#define EMC_QRST				0x064
+#define EMC_QSAFE				0x068
+#define EMC_RDV					0x06c
+#define EMC_REFRESH				0x070
+#define EMC_BURST_REFRESH_NUM			0x074
+#define EMC_PDEX2WR				0x078
+#define EMC_PDEX2RD				0x07c
+#define EMC_PCHG2PDEN				0x080
+#define EMC_ACT2PDEN				0x084
+#define EMC_AR2PDEN				0x088
+#define EMC_RW2PDEN				0x08c
+#define EMC_TXSR				0x090
+#define EMC_TCKE				0x094
+#define EMC_TFAW				0x098
+#define EMC_TRPAB				0x09c
+#define EMC_TCLKSTABLE				0x0a0
+#define EMC_TCLKSTOP				0x0a4
+#define EMC_TREFBW				0x0a8
+#define EMC_QUSE_EXTRA				0x0ac
+#define EMC_ODT_WRITE				0x0b0
+#define EMC_ODT_READ				0x0b4
+#define EMC_WEXT				0x0b8
+#define EMC_CTT					0x0bc
+#define EMC_MRS_WAIT_CNT			0x0c8
+#define EMC_MRS					0x0cc
+#define EMC_EMRS				0x0d0
+#define EMC_SELF_REF				0x0e0
+#define EMC_MRW					0x0e8
+#define EMC_XM2DQSPADCTRL3			0x0f8
+#define EMC_FBIO_SPARE				0x100
+#define EMC_FBIO_CFG5				0x104
+#define EMC_FBIO_CFG6				0x114
+#define EMC_CFG_RSV				0x120
+#define EMC_AUTO_CAL_CONFIG			0x2a4
+#define EMC_AUTO_CAL_INTERVAL			0x2a8
+#define EMC_AUTO_CAL_STATUS			0x2ac
+#define EMC_STATUS				0x2b4
+#define EMC_CFG_2				0x2b8
+#define EMC_CFG_DIG_DLL				0x2bc
+#define EMC_CFG_DIG_DLL_PERIOD			0x2c0
+#define EMC_CTT_DURATION			0x2d8
+#define EMC_CTT_TERM_CTRL			0x2dc
+#define EMC_ZCAL_INTERVAL			0x2e0
+#define EMC_ZCAL_WAIT_CNT			0x2e4
+#define EMC_ZQ_CAL				0x2ec
+#define EMC_XM2CMDPADCTRL			0x2f0
+#define EMC_XM2DQSPADCTRL2			0x2fc
+#define EMC_XM2DQPADCTRL2			0x304
+#define EMC_XM2CLKPADCTRL			0x308
+#define EMC_XM2COMPPADCTRL			0x30c
+#define EMC_XM2VTTGENPADCTRL			0x310
+#define EMC_XM2VTTGENPADCTRL2			0x314
+#define EMC_XM2QUSEPADCTRL			0x318
+#define EMC_DLL_XFORM_DQS0			0x328
+#define EMC_DLL_XFORM_DQS1			0x32c
+#define EMC_DLL_XFORM_DQS2			0x330
+#define EMC_DLL_XFORM_DQS3			0x334
+#define EMC_DLL_XFORM_DQS4			0x338
+#define EMC_DLL_XFORM_DQS5			0x33c
+#define EMC_DLL_XFORM_DQS6			0x340
+#define EMC_DLL_XFORM_DQS7			0x344
+#define EMC_DLL_XFORM_QUSE0			0x348
+#define EMC_DLL_XFORM_QUSE1			0x34c
+#define EMC_DLL_XFORM_QUSE2			0x350
+#define EMC_DLL_XFORM_QUSE3			0x354
+#define EMC_DLL_XFORM_QUSE4			0x358
+#define EMC_DLL_XFORM_QUSE5			0x35c
+#define EMC_DLL_XFORM_QUSE6			0x360
+#define EMC_DLL_XFORM_QUSE7			0x364
+#define EMC_DLL_XFORM_DQ0			0x368
+#define EMC_DLL_XFORM_DQ1			0x36c
+#define EMC_DLL_XFORM_DQ2			0x370
+#define EMC_DLL_XFORM_DQ3			0x374
+#define EMC_DLI_TRIM_TXDQS0			0x3a8
+#define EMC_DLI_TRIM_TXDQS1			0x3ac
+#define EMC_DLI_TRIM_TXDQS2			0x3b0
+#define EMC_DLI_TRIM_TXDQS3			0x3b4
+#define EMC_DLI_TRIM_TXDQS4			0x3b8
+#define EMC_DLI_TRIM_TXDQS5			0x3bc
+#define EMC_DLI_TRIM_TXDQS6			0x3c0
+#define EMC_DLI_TRIM_TXDQS7			0x3c4
+#define EMC_STALL_THEN_EXE_BEFORE_CLKCHANGE	0x3c8
+#define EMC_STALL_THEN_EXE_AFTER_CLKCHANGE	0x3cc
+#define EMC_UNSTALL_RW_AFTER_CLKCHANGE		0x3d0
+#define EMC_SEL_DPD_CTRL			0x3d8
+#define EMC_PRE_REFRESH_REQ_CNT			0x3dc
+#define EMC_DYN_SELF_REF_CONTROL		0x3e0
+#define EMC_TXSRDLL				0x3e4
+
+#define EMC_STATUS_TIMING_UPDATE_STALLED	BIT(23)
+
+#define EMC_MODE_SET_DLL_RESET			BIT(8)
+#define EMC_MODE_SET_LONG_CNT			BIT(26)
+
+#define EMC_SELF_REF_CMD_ENABLED		BIT(0)
+
+#define DRAM_DEV_SEL_ALL			(0 << 30)
+#define DRAM_DEV_SEL_0				(2 << 30)
+#define DRAM_DEV_SEL_1				(1 << 30)
+#define DRAM_BROADCAST(num) \
+	((num) > 1 ? DRAM_DEV_SEL_ALL : DRAM_DEV_SEL_0)
+
+#define EMC_ZQ_CAL_CMD				BIT(0)
+#define EMC_ZQ_CAL_LONG				BIT(4)
+#define EMC_ZQ_CAL_LONG_CMD_DEV0 \
+	(DRAM_DEV_SEL_0 | EMC_ZQ_CAL_LONG | EMC_ZQ_CAL_CMD)
+#define EMC_ZQ_CAL_LONG_CMD_DEV1 \
+	(DRAM_DEV_SEL_1 | EMC_ZQ_CAL_LONG | EMC_ZQ_CAL_CMD)
+
+#define EMC_DBG_WRITE_MUX_ACTIVE		BIT(1)
+
+#define EMC_CFG5_QUSE_MODE_SHIFT		13
+#define EMC_CFG5_QUSE_MODE_MASK			(7 << EMC_CFG5_QUSE_MODE_SHIFT)
+
+#define EMC_CFG5_QUSE_MODE_INTERNAL_LPBK	2
+#define EMC_CFG5_QUSE_MODE_PULSE_INTERN		3
+
+#define EMC_SEL_DPD_CTRL_QUSE_DPD_ENABLE	BIT(9)
+
+#define EMC_XM2COMPPADCTRL_VREF_CAL_ENABLE	BIT(10)
+
+#define EMC_XM2QUSEPADCTRL_IVREF_ENABLE		BIT(4)
+
+#define EMC_XM2DQSPADCTRL2_VREF_ENABLE		BIT(5)
+#define EMC_XM2DQSPADCTRL3_VREF_ENABLE		BIT(5)
+
+#define EMC_AUTO_CAL_STATUS_ACTIVE		BIT(31)
+
+#define	EMC_FBIO_CFG5_DRAM_TYPE_MASK		0x3
+
+#define EMC_MRS_WAIT_CNT_SHORT_WAIT_MASK	0x3ff
+#define EMC_MRS_WAIT_CNT_LONG_WAIT_SHIFT	16
+#define EMC_MRS_WAIT_CNT_LONG_WAIT_MASK \
+	(0x3ff << EMC_MRS_WAIT_CNT_LONG_WAIT_SHIFT)
+
+#define EMC_REFCTRL_DEV_SEL_MASK		0x3
+#define EMC_REFCTRL_ENABLE			BIT(31)
+#define EMC_REFCTRL_ENABLE_ALL(num) \
+	(((num) > 1 ? 0 : 2) | EMC_REFCTRL_ENABLE)
+#define EMC_REFCTRL_DISABLE_ALL(num)		((num) > 1 ? 0 : 2)
+
+#define EMC_CFG_PERIODIC_QRST			BIT(21)
+#define EMC_CFG_DYN_SREF_ENABLE			BIT(28)
+
+#define EMC_CLKCHANGE_REQ_ENABLE		BIT(0)
+#define EMC_CLKCHANGE_PD_ENABLE			BIT(1)
+#define EMC_CLKCHANGE_SR_ENABLE			BIT(2)
+
+#define EMC_TIMING_UPDATE			BIT(0)
+
+#define EMC_REFRESH_OVERFLOW_INT		BIT(3)
+#define EMC_CLKCHANGE_COMPLETE_INT		BIT(4)
+
+enum emc_dram_type {
+	DRAM_TYPE_DDR3,
+	DRAM_TYPE_DDR1,
+	DRAM_TYPE_LPDDR2,
+	DRAM_TYPE_DDR2,
+};
+
+enum emc_dll_change {
+	DLL_CHANGE_NONE,
+	DLL_CHANGE_ON,
+	DLL_CHANGE_OFF
+};
+
+static const u16 emc_timing_registers[] = {
+	[0] = EMC_RC,
+	[1] = EMC_RFC,
+	[2] = EMC_RAS,
+	[3] = EMC_RP,
+	[4] = EMC_R2W,
+	[5] = EMC_W2R,
+	[6] = EMC_R2P,
+	[7] = EMC_W2P,
+	[8] = EMC_RD_RCD,
+	[9] = EMC_WR_RCD,
+	[10] = EMC_RRD,
+	[11] = EMC_REXT,
+	[12] = EMC_WEXT,
+	[13] = EMC_WDV,
+	[14] = EMC_QUSE,
+	[15] = EMC_QRST,
+	[16] = EMC_QSAFE,
+	[17] = EMC_RDV,
+	[18] = EMC_REFRESH,
+	[19] = EMC_BURST_REFRESH_NUM,
+	[20] = EMC_PRE_REFRESH_REQ_CNT,
+	[21] = EMC_PDEX2WR,
+	[22] = EMC_PDEX2RD,
+	[23] = EMC_PCHG2PDEN,
+	[24] = EMC_ACT2PDEN,
+	[25] = EMC_AR2PDEN,
+	[26] = EMC_RW2PDEN,
+	[27] = EMC_TXSR,
+	[28] = EMC_TXSRDLL,
+	[29] = EMC_TCKE,
+	[30] = EMC_TFAW,
+	[31] = EMC_TRPAB,
+	[32] = EMC_TCLKSTABLE,
+	[33] = EMC_TCLKSTOP,
+	[34] = EMC_TREFBW,
+	[35] = EMC_QUSE_EXTRA,
+	[36] = EMC_FBIO_CFG6,
+	[37] = EMC_ODT_WRITE,
+	[38] = EMC_ODT_READ,
+	[39] = EMC_FBIO_CFG5,
+	[40] = EMC_CFG_DIG_DLL,
+	[41] = EMC_CFG_DIG_DLL_PERIOD,
+	[42] = EMC_DLL_XFORM_DQS0,
+	[43] = EMC_DLL_XFORM_DQS1,
+	[44] = EMC_DLL_XFORM_DQS2,
+	[45] = EMC_DLL_XFORM_DQS3,
+	[46] = EMC_DLL_XFORM_DQS4,
+	[47] = EMC_DLL_XFORM_DQS5,
+	[48] = EMC_DLL_XFORM_DQS6,
+	[49] = EMC_DLL_XFORM_DQS7,
+	[50] = EMC_DLL_XFORM_QUSE0,
+	[51] = EMC_DLL_XFORM_QUSE1,
+	[52] = EMC_DLL_XFORM_QUSE2,
+	[53] = EMC_DLL_XFORM_QUSE3,
+	[54] = EMC_DLL_XFORM_QUSE4,
+	[55] = EMC_DLL_XFORM_QUSE5,
+	[56] = EMC_DLL_XFORM_QUSE6,
+	[57] = EMC_DLL_XFORM_QUSE7,
+	[58] = EMC_DLI_TRIM_TXDQS0,
+	[59] = EMC_DLI_TRIM_TXDQS1,
+	[60] = EMC_DLI_TRIM_TXDQS2,
+	[61] = EMC_DLI_TRIM_TXDQS3,
+	[62] = EMC_DLI_TRIM_TXDQS4,
+	[63] = EMC_DLI_TRIM_TXDQS5,
+	[64] = EMC_DLI_TRIM_TXDQS6,
+	[65] = EMC_DLI_TRIM_TXDQS7,
+	[66] = EMC_DLL_XFORM_DQ0,
+	[67] = EMC_DLL_XFORM_DQ1,
+	[68] = EMC_DLL_XFORM_DQ2,
+	[69] = EMC_DLL_XFORM_DQ3,
+	[70] = EMC_XM2CMDPADCTRL,
+	[71] = EMC_XM2DQSPADCTRL2,
+	[72] = EMC_XM2DQPADCTRL2,
+	[73] = EMC_XM2CLKPADCTRL,
+	[74] = EMC_XM2COMPPADCTRL,
+	[75] = EMC_XM2VTTGENPADCTRL,
+	[76] = EMC_XM2VTTGENPADCTRL2,
+	[77] = EMC_XM2QUSEPADCTRL,
+	[78] = EMC_XM2DQSPADCTRL3,
+	[79] = EMC_CTT_TERM_CTRL,
+	[80] = EMC_ZCAL_INTERVAL,
+	[81] = EMC_ZCAL_WAIT_CNT,
+	[82] = EMC_MRS_WAIT_CNT,
+	[83] = EMC_AUTO_CAL_CONFIG,
+	[84] = EMC_CTT,
+	[85] = EMC_CTT_DURATION,
+	[86] = EMC_DYN_SELF_REF_CONTROL,
+	[87] = EMC_FBIO_SPARE,
+	[88] = EMC_CFG_RSV,
+};
+
+struct emc_timing {
+	unsigned long rate;
+
+	u32 data[ARRAY_SIZE(emc_timing_registers)];
+
+	u32 emc_auto_cal_interval;
+	u32 emc_mode_1;
+	u32 emc_mode_2;
+	u32 emc_mode_reset;
+	u32 emc_zcal_cnt_long;
+	u32 emc_cfg_periodic_qrst;
+	u32 emc_cfg_dyn_self_ref;
+};
+
+struct tegra_emc {
+	struct device *dev;
+	struct tegra_mc *mc;
+	struct completion clk_handshake_complete;
+	struct notifier_block clk_nb;
+	struct clk *clk;
+	void __iomem *regs;
+	int irq;
+
+	struct emc_timing *timings;
+	unsigned int num_timings;
+
+	u32 mc_override;
+	u32 emc_cfg;
+
+	u32 emc_mode_1;
+	u32 emc_mode_2;
+	u32 emc_mode_reset;
+
+	bool vref_cal_toggle : 1;
+	bool zcal_long : 1;
+	bool dll_on : 1;
+	bool prepared : 1;
+	bool bad_state : 1;
+};
+
+static irqreturn_t tegra_emc_isr(int irq, void *data)
+{
+	struct tegra_emc *emc = data;
+	u32 intmask = EMC_REFRESH_OVERFLOW_INT | EMC_CLKCHANGE_COMPLETE_INT;
+	u32 status;
+
+	status = readl_relaxed(emc->regs + EMC_INTSTATUS) & intmask;
+	if (!status)
+		return IRQ_NONE;
+
+	/* notify about EMC-CAR handshake completion */
+	if (status & EMC_CLKCHANGE_COMPLETE_INT)
+		complete(&emc->clk_handshake_complete);
+
+	/* notify about HW problem */
+	if (status & EMC_REFRESH_OVERFLOW_INT)
+		dev_err_ratelimited(emc->dev,
+				    "refresh request overflow timeout\n");
+
+	/* clear interrupts */
+	writel_relaxed(status, emc->regs + EMC_INTSTATUS);
+
+	return IRQ_HANDLED;
+}
+
+static struct emc_timing *emc_find_timing(struct tegra_emc *emc,
+					  unsigned long rate)
+{
+	struct emc_timing *timing = NULL;
+	unsigned int i;
+
+	for (i = 0; i < emc->num_timings; i++) {
+		if (emc->timings[i].rate >= rate) {
+			timing = &emc->timings[i];
+			break;
+		}
+	}
+
+	if (!timing) {
+		dev_err(emc->dev, "no timing for rate %lu\n", rate);
+		return NULL;
+	}
+
+	return timing;
+}
+
+static bool emc_dqs_preset(struct tegra_emc *emc, struct emc_timing *timing,
+			   bool *schmitt_to_vref)
+{
+	bool preset = false;
+	u32 val;
+
+	if (timing->data[71] & EMC_XM2DQSPADCTRL2_VREF_ENABLE) {
+		val = readl_relaxed(emc->regs + EMC_XM2DQSPADCTRL2);
+
+		if (!(val & EMC_XM2DQSPADCTRL2_VREF_ENABLE)) {
+			val |= EMC_XM2DQSPADCTRL2_VREF_ENABLE;
+			writel_relaxed(val, emc->regs + EMC_XM2DQSPADCTRL2);
+
+			preset = true;
+		}
+	}
+
+	if (timing->data[78] & EMC_XM2DQSPADCTRL3_VREF_ENABLE) {
+		val = readl_relaxed(emc->regs + EMC_XM2DQSPADCTRL3);
+
+		if (!(val & EMC_XM2DQSPADCTRL3_VREF_ENABLE)) {
+			val |= EMC_XM2DQSPADCTRL3_VREF_ENABLE;
+			writel_relaxed(val, emc->regs + EMC_XM2DQSPADCTRL3);
+
+			preset = true;
+		}
+	}
+
+	if (timing->data[77] & EMC_XM2QUSEPADCTRL_IVREF_ENABLE) {
+		val = readl_relaxed(emc->regs + EMC_XM2QUSEPADCTRL);
+
+		if (!(val & EMC_XM2QUSEPADCTRL_IVREF_ENABLE)) {
+			val |= EMC_XM2QUSEPADCTRL_IVREF_ENABLE;
+			writel_relaxed(val, emc->regs + EMC_XM2QUSEPADCTRL);
+
+			*schmitt_to_vref = true;
+			preset = true;
+		}
+	}
+
+	return preset;
+}
+
+static void emc_seq_update_timing(struct tegra_emc *emc)
+{
+	u32 val;
+	int err;
+
+	writel_relaxed(EMC_TIMING_UPDATE, emc->regs + EMC_TIMING_CONTROL);
+
+	err = readl_relaxed_poll_timeout_atomic(emc->regs + EMC_STATUS, val,
+				!(val & EMC_STATUS_TIMING_UPDATE_STALLED),
+				1, 100);
+	if (err)
+		dev_err(emc->dev, "failed to update timing: %d\n", err);
+}
+
+static int emc_prepare_mc_clk_cfg(struct tegra_emc *emc, unsigned long rate)
+{
+	struct tegra_mc *mc = emc->mc;
+	unsigned int misc0_index = 16;
+	unsigned int i;
+	bool same;
+
+	for (i = 0; i < mc->num_timings; i++) {
+		if (mc->timings[i].rate != rate)
+			continue;
+
+		if (mc->timings[i].emem_data[misc0_index] & BIT(16))
+			same = true;
+		else
+			same = false;
+
+		return tegra30_clk_prepare_emc_mc_same_freq(emc->clk, same);
+	}
+
+	return -EINVAL;
+}
+
+static int emc_prepare_timing_change(struct tegra_emc *emc, unsigned long rate)
+{
+	struct emc_timing *timing = emc_find_timing(emc, rate);
+	enum emc_dll_change dll_change;
+	enum emc_dram_type dram_type;
+	bool schmitt_to_vref = false;
+	unsigned int pre_wait = 0;
+	bool qrst_used = false;
+	unsigned int dram_num;
+	unsigned int i;
+	u32 fbio_cfg5;
+	u32 emc_dbg;
+	u32 val;
+	int err;
+
+	if (!timing || emc->bad_state)
+		return -EINVAL;
+
+	dev_dbg(emc->dev, "%s: using timing rate %lu for requested rate %lu\n",
+		__func__, timing->rate, rate);
+
+	err = emc_prepare_mc_clk_cfg(emc, rate);
+	if (err) {
+		dev_err(emc->dev, "mc clock preparation failed: %d\n", err);
+		return err;
+	}
+
+	emc->vref_cal_toggle = false;
+	emc->mc_override = mc_readl(emc->mc, MC_EMEM_ARB_OVERRIDE);
+	emc->emc_cfg = readl_relaxed(emc->regs + EMC_CFG);
+	emc_dbg = readl_relaxed(emc->regs + EMC_DBG);
+
+	if (emc->dll_on == !!(timing->emc_mode_1 & 0x1))
+		dll_change = DLL_CHANGE_NONE;
+	else if (timing->emc_mode_1 & 0x1)
+		dll_change = DLL_CHANGE_ON;
+	else
+		dll_change = DLL_CHANGE_OFF;
+
+	emc->dll_on = !!(timing->emc_mode_1 & 0x1);
+
+	if (timing->data[80] && !readl_relaxed(emc->regs + EMC_ZCAL_INTERVAL))
+		emc->zcal_long = true;
+	else
+		emc->zcal_long = false;
+
+	fbio_cfg5 = readl_relaxed(emc->regs + EMC_FBIO_CFG5);
+	dram_type = fbio_cfg5 & EMC_FBIO_CFG5_DRAM_TYPE_MASK;
+
+	dram_num = tegra_mc_get_emem_device_count(emc->mc);
+
+	/* disable dynamic self-refresh */
+	if (emc->emc_cfg & EMC_CFG_DYN_SREF_ENABLE) {
+		emc->emc_cfg &= ~EMC_CFG_DYN_SREF_ENABLE;
+		writel_relaxed(emc->emc_cfg, emc->regs + EMC_CFG);
+
+		pre_wait = 5;
+	}
+
+	/* update MC arbiter settings */
+	val = mc_readl(emc->mc, MC_EMEM_ARB_OUTSTANDING_REQ);
+	if (!(val & MC_EMEM_ARB_OUTSTANDING_REQ_HOLDOFF_OVERRIDE) ||
+	    ((val & MC_EMEM_ARB_OUTSTANDING_REQ_MAX_MASK) > 0x50)) {
+
+		val = MC_EMEM_ARB_OUTSTANDING_REQ_LIMIT_ENABLE |
+		      MC_EMEM_ARB_OUTSTANDING_REQ_HOLDOFF_OVERRIDE | 0x50;
+		mc_writel(emc->mc, val, MC_EMEM_ARB_OUTSTANDING_REQ);
+		mc_writel(emc->mc, MC_TIMING_UPDATE, MC_TIMING_CONTROL);
+	}
+
+	if (emc->mc_override & MC_EMEM_ARB_OVERRIDE_EACK_MASK)
+		mc_writel(emc->mc,
+			  emc->mc_override & ~MC_EMEM_ARB_OVERRIDE_EACK_MASK,
+			  MC_EMEM_ARB_OVERRIDE);
+
+	/* check DQ/DQS VREF delay */
+	if (emc_dqs_preset(emc, timing, &schmitt_to_vref)) {
+		if (pre_wait < 3)
+			pre_wait = 3;
+	}
+
+	if (pre_wait) {
+		emc_seq_update_timing(emc);
+		udelay(pre_wait);
+	}
+
+	/* disable auto-calibration if VREF mode is switching */
+	if (timing->emc_auto_cal_interval) {
+		val = readl_relaxed(emc->regs + EMC_XM2COMPPADCTRL);
+		val ^= timing->data[74];
+
+		if (val & EMC_XM2COMPPADCTRL_VREF_CAL_ENABLE) {
+			writel_relaxed(0, emc->regs + EMC_AUTO_CAL_INTERVAL);
+
+			err = readl_relaxed_poll_timeout_atomic(
+				emc->regs + EMC_AUTO_CAL_STATUS, val,
+				!(val & EMC_AUTO_CAL_STATUS_ACTIVE), 1, 300);
+			if (err)
+				dev_err(emc->dev,
+					"failed to disable auto-cal: %d\n",
+					err);
+
+			emc->vref_cal_toggle = true;
+		}
+	}
+
+	/* program shadow registers */
+	for (i = 0; i < ARRAY_SIZE(timing->data); i++) {
+		/* EMC_XM2CLKPADCTRL should be programmed separately */
+		if (i != 73)
+			writel_relaxed(timing->data[i],
+				       emc->regs + emc_timing_registers[i]);
+	}
+
+	tegra_mc_write_emem_configuration(emc->mc, timing->rate);
+
+	/* DDR3: predict MRS long wait count */
+	if (dram_type == DRAM_TYPE_DDR3 &&
+	    dll_change == DLL_CHANGE_ON) {
+		u32 cnt = 512;
+
+		if (emc->zcal_long)
+			cnt -= dram_num * 256;
+
+		val = timing->data[82] & EMC_MRS_WAIT_CNT_SHORT_WAIT_MASK;
+		if (cnt < val)
+			cnt = val;
+
+		val = timing->data[82] & ~EMC_MRS_WAIT_CNT_LONG_WAIT_MASK;
+		val |= (cnt << EMC_MRS_WAIT_CNT_LONG_WAIT_SHIFT)
+			& EMC_MRS_WAIT_CNT_LONG_WAIT_MASK;
+
+		writel_relaxed(val, emc->regs + EMC_MRS_WAIT_CNT);
+	}
+
+	/* disable interrupt since read access is prohibited after stalling */
+	disable_irq(emc->irq);
+
+	/* this read also completes the writes */
+	val = readl_relaxed(emc->regs + EMC_SEL_DPD_CTRL);
+
+	if (!(val & EMC_SEL_DPD_CTRL_QUSE_DPD_ENABLE) && schmitt_to_vref) {
+		u32 cur_mode, new_mode;
+
+		cur_mode = fbio_cfg5 & EMC_CFG5_QUSE_MODE_MASK;
+		cur_mode >>= EMC_CFG5_QUSE_MODE_SHIFT;
+
+		new_mode = timing->data[39] & EMC_CFG5_QUSE_MODE_MASK;
+		new_mode >>= EMC_CFG5_QUSE_MODE_SHIFT;
+
+		if ((cur_mode != EMC_CFG5_QUSE_MODE_PULSE_INTERN &&
+		     cur_mode != EMC_CFG5_QUSE_MODE_INTERNAL_LPBK) ||
+		    (new_mode != EMC_CFG5_QUSE_MODE_PULSE_INTERN &&
+		     new_mode != EMC_CFG5_QUSE_MODE_INTERNAL_LPBK))
+			qrst_used = true;
+	}
+
+	/* flow control marker 1 */
+	writel_relaxed(0x1, emc->regs + EMC_STALL_THEN_EXE_BEFORE_CLKCHANGE);
+
+	/* enable periodic reset */
+	if (qrst_used) {
+		writel_relaxed(emc_dbg | EMC_DBG_WRITE_MUX_ACTIVE,
+			       emc->regs + EMC_DBG);
+		writel_relaxed(emc->emc_cfg | EMC_CFG_PERIODIC_QRST,
+			       emc->regs + EMC_CFG);
+		writel_relaxed(emc_dbg, emc->regs + EMC_DBG);
+	}
+
+	/* disable auto-refresh to save time after clock change */
+	writel_relaxed(EMC_REFCTRL_DISABLE_ALL(dram_num),
+		       emc->regs + EMC_REFCTRL);
+
+	/* turn off DLL and enter self-refresh on DDR3 */
+	if (dram_type == DRAM_TYPE_DDR3) {
+		if (dll_change == DLL_CHANGE_OFF)
+			writel_relaxed(timing->emc_mode_1,
+				       emc->regs + EMC_EMRS);
+
+		writel_relaxed(DRAM_BROADCAST(dram_num) |
+			       EMC_SELF_REF_CMD_ENABLED,
+			       emc->regs + EMC_SELF_REF);
+	}
+
+	/* flow control marker 2 */
+	writel_relaxed(0x1, emc->regs + EMC_STALL_THEN_EXE_AFTER_CLKCHANGE);
+
+	/* enable write MUX, update unshadowed pad control */
+	writel_relaxed(emc_dbg | EMC_DBG_WRITE_MUX_ACTIVE, emc->regs + EMC_DBG);
+	writel_relaxed(timing->data[73], emc->regs + EMC_XM2CLKPADCTRL);
+
+	/* restore periodic QRST and disable write MUX */
+	val = emc->emc_cfg & EMC_CFG_PERIODIC_QRST;
+	if (qrst_used || !!timing->emc_cfg_periodic_qrst != !!val) {
+		if (timing->emc_cfg_periodic_qrst)
+			emc->emc_cfg |= EMC_CFG_PERIODIC_QRST;
+		else
+			emc->emc_cfg &= ~EMC_CFG_PERIODIC_QRST;
+
+		writel_relaxed(emc->emc_cfg, emc->regs + EMC_CFG);
+	}
+	writel_relaxed(emc_dbg, emc->regs + EMC_DBG);
+
+	/* exit self-refresh on DDR3 */
+	if (dram_type == DRAM_TYPE_DDR3)
+		writel_relaxed(DRAM_BROADCAST(dram_num),
+			       emc->regs + EMC_SELF_REF);
+
+	/* set DRAM mode registers */
+	if (dram_type == DRAM_TYPE_DDR3) {
+		if (timing->emc_mode_1 != emc->emc_mode_1)
+			writel_relaxed(timing->emc_mode_1,
+				       emc->regs + EMC_EMRS);
+		if (timing->emc_mode_2 != emc->emc_mode_2)
+			writel_relaxed(timing->emc_mode_2,
+				       emc->regs + EMC_EMRS);
+
+		if (timing->emc_mode_reset != emc->emc_mode_reset ||
+		    dll_change == DLL_CHANGE_ON) {
+			val = timing->emc_mode_reset;
+			if (dll_change == DLL_CHANGE_ON) {
+				val |= EMC_MODE_SET_DLL_RESET;
+				val |= EMC_MODE_SET_LONG_CNT;
+			} else {
+				val &= ~EMC_MODE_SET_DLL_RESET;
+			}
+			writel_relaxed(val, emc->regs + EMC_MRS);
+		}
+	} else {
+		if (timing->emc_mode_2 != emc->emc_mode_2)
+			writel_relaxed(timing->emc_mode_2,
+				       emc->regs + EMC_MRW);
+		if (timing->emc_mode_1 != emc->emc_mode_1)
+			writel_relaxed(timing->emc_mode_1,
+				       emc->regs + EMC_MRW);
+	}
+
+	emc->emc_mode_1 = timing->emc_mode_1;
+	emc->emc_mode_2 = timing->emc_mode_2;
+	emc->emc_mode_reset = timing->emc_mode_reset;
+
+	/* issue ZCAL command if turning ZCAL on */
+	if (emc->zcal_long) {
+		writel_relaxed(EMC_ZQ_CAL_LONG_CMD_DEV0,
+			       emc->regs + EMC_ZQ_CAL);
+
+		if (dram_num > 1)
+			writel_relaxed(EMC_ZQ_CAL_LONG_CMD_DEV1,
+				       emc->regs + EMC_ZQ_CAL);
+	}
+
+	/* flow control marker 3 */
+	writel_relaxed(0x1, emc->regs + EMC_UNSTALL_RW_AFTER_CLKCHANGE);
+
+	reinit_completion(&emc->clk_handshake_complete);
+
+	/* interrupt can be re-enabled now */
+	enable_irq(emc->irq);
+
+	emc->prepared = true;
+
+	return 0;
+}
+
+static int emc_complete_timing_change(struct tegra_emc *emc,
+				      unsigned long rate)
+{
+	struct emc_timing *timing = emc_find_timing(emc, rate);
+	unsigned int dram_num;
+	long timeout;
+
+	timeout = wait_for_completion_timeout(&emc->clk_handshake_complete,
+					      usecs_to_jiffies(100));
+	if (timeout == 0) {
+		dev_err(emc->dev, "emc-car handshake failed\n");
+		emc->bad_state = true;
+		return -EIO;
+	} else if (timeout < 0) {
+		dev_err(emc->dev, "failed to wait for emc-car handshake: %ld\n",
+			timeout);
+		udelay(100);
+	}
+
+	dram_num = tegra_mc_get_emem_device_count(emc->mc);
+
+	/* re-enable auto-refresh */
+	writel_relaxed(EMC_REFCTRL_ENABLE_ALL(dram_num),
+		       emc->regs + EMC_REFCTRL);
+
+	/* restore auto-calibration */
+	if (emc->vref_cal_toggle)
+		writel_relaxed(timing->emc_auto_cal_interval,
+			       emc->regs + EMC_AUTO_CAL_INTERVAL);
+
+	/* restore dynamic self-refresh */
+	if (timing->emc_cfg_dyn_self_ref) {
+		emc->emc_cfg |= EMC_CFG_DYN_SREF_ENABLE;
+		writel_relaxed(emc->emc_cfg, emc->regs + EMC_CFG);
+	}
+
+	/* set ZCAL wait count */
+	if (emc->zcal_long)
+		writel_relaxed(timing->emc_zcal_cnt_long,
+			       emc->regs + EMC_ZCAL_WAIT_CNT);
+
+	/* update restored timing */
+	udelay(2);
+	emc_seq_update_timing(emc);
+
+	/* restore early ACK */
+	mc_writel(emc->mc, emc->mc_override, MC_EMEM_ARB_OVERRIDE);
+
+	emc->prepared = false;
+
+	return 0;
+}
+
+static int emc_unprepare_timing_change(struct tegra_emc *emc,
+				       unsigned long rate)
+{
+	if (emc->prepared && !emc->bad_state) {
+		/* shouldn't ever happen in practice */
+		dev_err(emc->dev, "timing configuration can't be reverted\n");
+		emc->bad_state = true;
+	}
+
+	return 0;
+}
+
+static int emc_clk_change_notify(struct notifier_block *nb,
+				 unsigned long msg, void *data)
+{
+	struct tegra_emc *emc = container_of(nb, struct tegra_emc, clk_nb);
+	struct clk_notifier_data *cnd = data;
+	int err;
+
+	switch (msg) {
+	case PRE_RATE_CHANGE:
+		err = emc_prepare_timing_change(emc, cnd->new_rate);
+		break;
+
+	case ABORT_RATE_CHANGE:
+		err = emc_unprepare_timing_change(emc, cnd->old_rate);
+		break;
+
+	case POST_RATE_CHANGE:
+		err = emc_complete_timing_change(emc, cnd->new_rate);
+		break;
+
+	default:
+		return NOTIFY_DONE;
+	}
+
+	return notifier_from_errno(err);
+}
+
+static int load_one_timing_from_dt(struct tegra_emc *emc,
+				   struct emc_timing *timing,
+				   struct device_node *node)
+{
+	u32 value;
+	int err;
+
+	err = of_property_read_u32(node, "clock-frequency", &value);
+	if (err) {
+		dev_err(emc->dev, "timing %pOF: failed to read rate: %d\n",
+			node, err);
+		return err;
+	}
+
+	timing->rate = value;
+
+	err = of_property_read_u32_array(node, "nvidia,emc-configuration",
+					 timing->data,
+					 ARRAY_SIZE(emc_timing_registers));
+	if (err) {
+		dev_err(emc->dev,
+			"timing %pOF: failed to read emc timing data: %d\n",
+			node, err);
+		return err;
+	}
+
+#define EMC_READ_PROP(prop, dtprop) { \
+	err = of_property_read_u32(node, dtprop, &timing->prop); \
+	if (err) { \
+		dev_err(emc->dev, \
+			"timing %pOFn: failed to read " #prop ": %d\n", \
+			node, err); \
+		return err; \
+	} \
+}
+
+	EMC_READ_PROP(emc_auto_cal_interval, "nvidia,emc-auto-cal-interval")
+	EMC_READ_PROP(emc_mode_1, "nvidia,emc-mode-1")
+	EMC_READ_PROP(emc_mode_2, "nvidia,emc-mode-2")
+	EMC_READ_PROP(emc_mode_reset, "nvidia,emc-mode-reset")
+	EMC_READ_PROP(emc_zcal_cnt_long, "nvidia,emc-zcal-cnt-long")
+	EMC_READ_PROP(emc_cfg_dyn_self_ref, "nvidia,emc-cfg-dyn-self-ref")
+	EMC_READ_PROP(emc_cfg_periodic_qrst, "nvidia,emc-cfg-periodic-qrst")
+
+#undef EMC_READ_PROP
+
+	dev_dbg(emc->dev, "%s: %pOF: rate %lu\n", __func__, node, timing->rate);
+
+	return 0;
+}
+
+static int cmp_timings(const void *_a, const void *_b)
+{
+	const struct emc_timing *a = _a;
+	const struct emc_timing *b = _b;
+
+	if (a->rate < b->rate)
+		return -1;
+
+	if (a->rate > b->rate)
+		return 1;
+
+	return 0;
+}
+
+static int emc_check_mc_timings(struct tegra_emc *emc)
+{
+	struct tegra_mc *mc = emc->mc;
+	unsigned int i;
+
+	if (emc->num_timings != mc->num_timings) {
+		dev_err(emc->dev, "emc-mc timings number mismatch: %u %u\n",
+			emc->num_timings, mc->num_timings);
+		return -EINVAL;
+	}
+
+	for (i = 0; i < mc->num_timings; i++) {
+		if (emc->timings[i].rate != mc->timings[i].rate) {
+			dev_err(emc->dev,
+				"emc-mc timing rate mismatch: %lu %lu\n",
+				emc->timings[i].rate, mc->timings[i].rate);
+			return -EINVAL;
+		}
+	}
+
+	return 0;
+}
+
+static int emc_load_timings_from_dt(struct tegra_emc *emc,
+				    struct device_node *node)
+{
+	struct device_node *child;
+	struct emc_timing *timing;
+	int child_count;
+	int err;
+
+	child_count = of_get_child_count(node);
+	if (!child_count) {
+		dev_err(emc->dev, "no memory timings in: %pOF\n", node);
+		return -EINVAL;
+	}
+
+	emc->timings = devm_kcalloc(emc->dev, child_count, sizeof(*timing),
+				    GFP_KERNEL);
+	if (!emc->timings)
+		return -ENOMEM;
+
+	emc->num_timings = child_count;
+	timing = emc->timings;
+
+	for_each_child_of_node(node, child) {
+		err = load_one_timing_from_dt(emc, timing++, child);
+		if (err) {
+			of_node_put(child);
+			return err;
+		}
+	}
+
+	sort(emc->timings, emc->num_timings, sizeof(*timing), cmp_timings,
+	     NULL);
+
+	err = emc_check_mc_timings(emc);
+	if (err)
+		return err;
+
+	return 0;
+}
+
+static struct device_node *emc_find_node_by_ram_code(struct device *dev)
+{
+	struct device_node *np;
+	u32 value, ram_code;
+	int err;
+
+	ram_code = tegra_read_ram_code();
+
+	for_each_child_of_node(dev->of_node, np) {
+		err = of_property_read_u32(np, "nvidia,ram-code", &value);
+		if (err || value != ram_code)
+			continue;
+
+		return np;
+	}
+
+	dev_err(dev, "no memory timings for RAM code %u found in device-tree\n",
+		ram_code);
+
+	return NULL;
+}
+
+static int emc_setup_hw(struct tegra_emc *emc)
+{
+	u32 intmask = EMC_REFRESH_OVERFLOW_INT | EMC_CLKCHANGE_COMPLETE_INT;
+	enum emc_dram_type dram_type;
+	u32 fbio_cfg5;
+	u32 emc_cfg;
+
+	fbio_cfg5 = readl_relaxed(emc->regs + EMC_FBIO_CFG5);
+	dram_type = fbio_cfg5 & EMC_FBIO_CFG5_DRAM_TYPE_MASK;
+
+	emc_cfg = readl_relaxed(emc->regs + EMC_CFG_2);
+
+	/* enable EMC and CAR to handshake on PLL divider/source changes */
+	emc_cfg |= EMC_CLKCHANGE_REQ_ENABLE;
+
+	/* configure clock change mode according to DRAM type */
+	switch (dram_type) {
+	case DRAM_TYPE_LPDDR2:
+		emc_cfg |= EMC_CLKCHANGE_PD_ENABLE;
+		emc_cfg &= ~EMC_CLKCHANGE_SR_ENABLE;
+		break;
+
+	default:
+		emc_cfg &= ~EMC_CLKCHANGE_SR_ENABLE;
+		emc_cfg &= ~EMC_CLKCHANGE_PD_ENABLE;
+		break;
+	}
+
+	writel_relaxed(emc_cfg, emc->regs + EMC_CFG_2);
+
+	/* initialize interrupt */
+	writel_relaxed(intmask, emc->regs + EMC_INTMASK);
+	writel_relaxed(intmask, emc->regs + EMC_INTSTATUS);
+
+	return 0;
+}
+
+static long emc_round_rate(unsigned long rate,
+			   unsigned long min_rate,
+			   unsigned long max_rate,
+			   void *arg)
+{
+	struct emc_timing *timing = NULL;
+	struct tegra_emc *emc = arg;
+	unsigned int i;
+
+	for (i = 0; i < emc->num_timings; i++) {
+		if (emc->timings[i].rate < rate && i != emc->num_timings - 1)
+			continue;
+
+		if (emc->timings[i].rate > max_rate) {
+			i = max(i, 1u) - 1;
+
+			if (emc->timings[i].rate < min_rate)
+				break;
+		}
+
+		if (emc->timings[i].rate < min_rate)
+			continue;
+
+		timing = &emc->timings[i];
+		break;
+	}
+
+	if (!timing) {
+		dev_err(emc->dev, "no timing for rate %lu min %lu max %lu\n",
+			rate, min_rate, max_rate);
+		return -EINVAL;
+	}
+
+	return timing->rate;
+}
+
+static int tegra_emc_probe(struct platform_device *pdev)
+{
+	struct platform_device *mc;
+	struct device_node *np;
+	struct tegra_emc *emc;
+	int err;
+
+	if (of_get_child_count(pdev->dev.of_node) == 0) {
+		dev_info(&pdev->dev,
+			 "device-tree node doesn't have memory timings\n");
+		return 0;
+	}
+
+	np = of_parse_phandle(pdev->dev.of_node, "nvidia,memory-controller", 0);
+	if (!np) {
+		dev_err(&pdev->dev, "could not get memory controller node\n");
+		return -ENOENT;
+	}
+
+	mc = of_find_device_by_node(np);
+	of_node_put(np);
+	if (!mc)
+		return -ENOENT;
+
+	np = emc_find_node_by_ram_code(&pdev->dev);
+	if (!np)
+		return -EINVAL;
+
+	emc = devm_kzalloc(&pdev->dev, sizeof(*emc), GFP_KERNEL);
+	if (!emc) {
+		of_node_put(np);
+		return -ENOMEM;
+	}
+
+	emc->mc = platform_get_drvdata(mc);
+	if (!emc->mc)
+		return -EPROBE_DEFER;
+
+	init_completion(&emc->clk_handshake_complete);
+	emc->clk_nb.notifier_call = emc_clk_change_notify;
+	emc->dev = &pdev->dev;
+
+	err = emc_load_timings_from_dt(emc, np);
+	of_node_put(np);
+	if (err)
+		return err;
+
+	emc->regs = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(emc->regs))
+		return PTR_ERR(emc->regs);
+
+	err = emc_setup_hw(emc);
+	if (err)
+		return err;
+
+	emc->irq = platform_get_irq(pdev, 0);
+	if (emc->irq < 0) {
+		dev_err(&pdev->dev, "interrupt not specified\n");
+		return emc->irq;
+	}
+
+	err = devm_request_irq(&pdev->dev, emc->irq, tegra_emc_isr, 0,
+			       dev_name(&pdev->dev), emc);
+	if (err) {
+		dev_err(&pdev->dev, "failed to request irq: %d\n", err);
+		return err;
+	}
+
+	tegra30_clk_set_emc_round_callback(emc_round_rate, emc);
+
+	emc->clk = devm_clk_get(&pdev->dev, "emc");
+	if (IS_ERR(emc->clk)) {
+		err = PTR_ERR(emc->clk);
+		dev_err(&pdev->dev, "failed to get emc clock: %d\n", err);
+		goto unset_cb;
+	}
+
+	err = clk_notifier_register(emc->clk, &emc->clk_nb);
+	if (err) {
+		dev_err(&pdev->dev, "failed to register clk notifier: %d\n",
+			err);
+		goto unset_cb;
+	}
+
+	return 0;
+
+unset_cb:
+	tegra30_clk_set_emc_round_callback(NULL, NULL);
+
+	return err;
+}
+
+static const struct of_device_id tegra_emc_of_match[] = {
+	{ .compatible = "nvidia,tegra30-emc", },
+	{},
+};
+
+static struct platform_driver tegra_emc_driver = {
+	.probe = tegra_emc_probe,
+	.driver = {
+		.name = "tegra30-emc",
+		.of_match_table = tegra_emc_of_match,
+		.suppress_bind_attrs = true,
+	},
+};
+
+static int __init tegra_emc_init(void)
+{
+	return platform_driver_register(&tegra_emc_driver);
+}
+subsys_initcall(tegra_emc_init);
diff --git a/drivers/memory/tegra/tegra30.c b/drivers/memory/tegra/tegra30.c
index bee5314ed404..3905ba95bdbc 100644
--- a/drivers/memory/tegra/tegra30.c
+++ b/drivers/memory/tegra/tegra30.c
@@ -13,6 +13,48 @@
 
 #include "mc.h"
 
+#define MC_EMEM_ARB_CFG				0x90
+#define MC_EMEM_ARB_OUTSTANDING_REQ		0x94
+#define MC_EMEM_ARB_TIMING_RCD			0x98
+#define MC_EMEM_ARB_TIMING_RP			0x9c
+#define MC_EMEM_ARB_TIMING_RC			0xa0
+#define MC_EMEM_ARB_TIMING_RAS			0xa4
+#define MC_EMEM_ARB_TIMING_FAW			0xa8
+#define MC_EMEM_ARB_TIMING_RRD			0xac
+#define MC_EMEM_ARB_TIMING_RAP2PRE		0xb0
+#define MC_EMEM_ARB_TIMING_WAP2PRE		0xb4
+#define MC_EMEM_ARB_TIMING_R2R			0xb8
+#define MC_EMEM_ARB_TIMING_W2W			0xbc
+#define MC_EMEM_ARB_TIMING_R2W			0xc0
+#define MC_EMEM_ARB_TIMING_W2R			0xc4
+#define MC_EMEM_ARB_DA_TURNS			0xd0
+#define MC_EMEM_ARB_DA_COVERS			0xd4
+#define MC_EMEM_ARB_MISC0			0xd8
+#define MC_EMEM_ARB_MISC1			0xdc
+#define MC_EMEM_ARB_RING1_THROTTLE		0xe0
+
+static const unsigned long tegra30_mc_emem_regs[] = {
+	MC_EMEM_ARB_CFG,
+	MC_EMEM_ARB_OUTSTANDING_REQ,
+	MC_EMEM_ARB_TIMING_RCD,
+	MC_EMEM_ARB_TIMING_RP,
+	MC_EMEM_ARB_TIMING_RC,
+	MC_EMEM_ARB_TIMING_RAS,
+	MC_EMEM_ARB_TIMING_FAW,
+	MC_EMEM_ARB_TIMING_RRD,
+	MC_EMEM_ARB_TIMING_RAP2PRE,
+	MC_EMEM_ARB_TIMING_WAP2PRE,
+	MC_EMEM_ARB_TIMING_R2R,
+	MC_EMEM_ARB_TIMING_W2W,
+	MC_EMEM_ARB_TIMING_R2W,
+	MC_EMEM_ARB_TIMING_W2R,
+	MC_EMEM_ARB_DA_TURNS,
+	MC_EMEM_ARB_DA_COVERS,
+	MC_EMEM_ARB_MISC0,
+	MC_EMEM_ARB_MISC1,
+	MC_EMEM_ARB_RING1_THROTTLE,
+};
+
 static const struct tegra_mc_client tegra30_mc_clients[] = {
 	{
 		.id = 0x00,
@@ -997,6 +1039,8 @@ const struct tegra_mc_soc tegra30_mc_soc = {
 	.atom_size = 16,
 	.client_id_mask = 0x7f,
 	.smmu = &tegra30_smmu_soc,
+	.emem_regs = tegra30_mc_emem_regs,
+	.num_emem_regs = ARRAY_SIZE(tegra30_mc_emem_regs),
 	.intmask = MC_INT_INVALID_SMMU_PAGE | MC_INT_SECURITY_VIOLATION |
 		   MC_INT_DECERR_EMEM,
 	.reset_ops = &terga_mc_reset_ops_common,
-- 
2.21.0

[PATCH v2 4/4] ARM: dts: tegra30: Add External Memory Controller node

[Dmitry Osipenko]

Add External Memory Controller node to the device-tree.

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

diff --git a/arch/arm/boot/dts/tegra30.dtsi b/arch/arm/boot/dts/tegra30.dtsi
index e074258d4518..92c4aeafab29 100644
--- a/arch/arm/boot/dts/tegra30.dtsi
+++ b/arch/arm/boot/dts/tegra30.dtsi
@@ -732,6 +732,17 @@
 		#reset-cells = <1>;
 	};
 
+	memory-controller@7000f400 {
+		compatible = "nvidia,tegra30-emc";
+		reg = <0x7000f400 0x400>;
+		interrupts = <GIC_SPI 78 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&tegra_car TEGRA30_CLK_EMC>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		nvidia,memory-controller = <&mc>;
+	};
+
 	fuse@7000f800 {
 		compatible = "nvidia,tegra30-efuse";
 		reg = <0x7000f800 0x400>;
-- 
2.21.0