memory: tegra: Introduce Tegra30 EMC driver

PATCH Superseded Dmitry Osipenko 4 participants 23 messages Jun 16, 2019 – Jun 19, 2019

Subsystems (5)

Device tree bindings Common clk Tegra architecture support Memory controller drivers +1 more subsystems

lore

Share & tools

lore

Dmitry Osipenko

author

2019-06-16 23:35 UTC #0 lore raw

v4 cover memory: tegra: Introduce Tegra30 EMC driver

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.

Changelog:

v4: - Addressed review comments that were made by Peter De Schrijver to v3
by adding fence_udelay() after writes in the "Add custom EMC clock implementation" patch.

- Added two new minor patches:

memory: tegra: Ensure timing control debug features are disabled
memory: tegra: Consolidate registers definition into one place

The first one is needed to ensure that EMC driver will work properly regardless of hardware configuration left after boot. The second patch is just a minor code cleanup.

- The "Introduce Tegra30 EMC driver" got also few very minor changes. Now every possible error case is handled, nothing is ignored. The EMC_DBG register is explicitly initialized during probe to be on the safe side.

v3: - Addressed review comments that were made by Stephen Boyd to v2 by
adding explicit typing for the callback variable, by including "clk-provider.h" directly in the code and by dropping __clk_lookup usage where possible.

- Added more patches into this series:

memory: tegra20-emc: Drop setting EMC rate to max on probe
memory: tegra20-emc: Adapt for clock driver changes
memory: tegra20-emc: Include io.h instead of iopoll.h
memory: tegra20-emc: Replace clk_get_sys with devm_clk_get

Initially I was going to include these patches into other patchset, but changed my mind after rearranging things a tad. The "Adapt for clock driver changes" patch is directly related to the clock changes done in the first patch of this series, the rest are minor cleanups that are fine to include here as well.

- Added some more words to the commit message of "Add binding for NVIDIA Tegra30 External Memory Controller" patch, clarifying why common DDR timing device-tree form isn't suitable for Tegra30.

- The Tegra30 EMC driver now explicitly selects the registers access mode (EMC_DBG mux), not relying on the setting left from bootloader.

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 (10):
clk: tegra20/30: Add custom EMC clock implementation
memory: tegra20-emc: Drop setting EMC rate to max on probe
memory: tegra20-emc: Adapt for clock driver changes
memory: tegra20-emc: Include io.h instead of iopoll.h
memory: tegra20-emc: Replace clk_get_sys with devm_clk_get
dt-bindings: memory: Add binding for NVIDIA Tegra30 External Memory
Controller
memory: tegra: Introduce Tegra30 EMC driver
memory: tegra: Ensure timing control debug features are disabled
memory: tegra: Consolidate registers definition into one place
ARM: dts: tegra30: Add External Memory Controller node

 .../memory-controllers/nvidia,tegra30-emc.txt |  249 ++++
 arch/arm/boot/dts/tegra30.dtsi                |   11 +
 drivers/clk/tegra/Makefile                    |    2 +
 drivers/clk/tegra/clk-tegra20-emc.c           |  305 +++++
 drivers/clk/tegra/clk-tegra20.c               |   55 +-
 drivers/clk/tegra/clk-tegra30.c               |   38 +-
 drivers/clk/tegra/clk.h                       |    6 +
 drivers/memory/tegra/Kconfig                  |   10 +
 drivers/memory/tegra/Makefile                 |    1 +
 drivers/memory/tegra/mc.c                     |   42 +-
 drivers/memory/tegra/mc.h                     |   74 +-
 drivers/memory/tegra/tegra124.c               |   20 -
 drivers/memory/tegra/tegra20-emc.c            |   94 +-
 drivers/memory/tegra/tegra30-emc.c            | 1197 +++++++++++++++++
 drivers/memory/tegra/tegra30.c                |   24 +
 include/linux/clk/tegra.h                     |   14 +
 include/soc/tegra/mc.h                        |    2 +-
 17 files changed, 1973 insertions(+), 171 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

Signature · 1 line

2.22.0

Dmitry Osipenko

author

2019-06-16 23:35 UTC #1 lore raw

PATCH v4 1/10 clk: tegra20/30: Add custom EMC clock implementation

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 <[email protected]>

---
 drivers/clk/tegra/Makefile          |   2 +
 drivers/clk/tegra/clk-tegra20-emc.c | 305 ++++++++++++++++++++++++++++
 drivers/clk/tegra/clk-tegra20.c     |  55 ++---
 drivers/clk/tegra/clk-tegra30.c     |  38 +++-
 drivers/clk/tegra/clk.h             |   6 +
 include/linux/clk/tegra.h           |  14 ++
 6 files changed, 368 insertions(+), 52 deletions(-)
 create mode 100644 drivers/clk/tegra/clk-tegra20-emc.c

drivers/clk/tegra/Makefile +2 -0

index 4812e45c2214..df966ca06788 100644
@@ -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

drivers/clk/tegra/clk-tegra20-emc.c new file +305 -0

305 lines not on this page · raw message

drivers/clk/tegra/clk-tegra20.c +14 ~~-41~~

96 lines not on this page · raw message

drivers/clk/tegra/clk-tegra30.c +27 ~~-11~~

index 7b4c6a488527..fab075808c20 100644
@@ -151,7 +151,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)	\

@@ -808,7 +807,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" };

@@ -995,7 +994,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" };

@@ -1044,14 +1042,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 */

@@ -1302,6 +1298,26 @@ 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_hw *hw;
+	struct clk *clk;
+
+	clk = of_clk_src_onecell_get(clkspec, data);
+	if (IS_ERR(clk))
+		return clk;
+
+	hw = __clk_get_hw(clk);
+
+	if (clkspec->args[0] == TEGRA30_CLK_EMC) {
+		if (!tegra30_clk_emc_driver_available(hw))
+			return ERR_PTR(-EPROBE_DEFER);
+	}
+
+	return clk;
+}
+
 static void __init tegra30_clock_init(struct device_node *np)
 {
 	struct device_node *node;

@@ -1345,7 +1361,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;

drivers/clk/tegra/clk.h +6 -0

index 905bf1096558..1eb2ec20e343 100644
@@ -838,4 +838,10 @@ int div_frac_get(unsigned long rate, unsigned parent_rate, u8 width,
 		udelay(delay);		\
 	} while (0)
 
+bool tegra20_clk_emc_driver_available(struct clk_hw *emc_hw);
+struct clk *tegra20_clk_register_emc(void __iomem *ioaddr);
+
+bool tegra30_clk_emc_driver_available(struct clk_hw *emc_hw);
+struct clk *tegra30_clk_register_emc(void __iomem *ioaddr);
+
 #endif /* TEGRA_CLK_H */

include/linux/clk/tegra.h +14 -0

index b8aef62cc3f5..8546e28aa518 100644
@@ -119,4 +119,18 @@ 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;
+
+typedef long (tegra20_clk_emc_round_cb)(unsigned long rate,
+					unsigned long min_rate,
+					unsigned long max_rate,
+					void *arg);
+#define tegra30_clk_emc_round_cb	tegra20_clk_emc_round_cb
+
+void tegra20_clk_set_emc_round_callback(tegra20_clk_emc_round_cb *round_cb,
+					void *cb_arg);
+void tegra30_clk_set_emc_round_callback(tegra30_clk_emc_round_cb *round_cb,
+					void *cb_arg);
+int tegra30_clk_prepare_emc_mc_same_freq(struct clk *emc_clk, bool same);
+
 #endif /* __LINUX_CLK_TEGRA_H_ */

Signature · 1 line

2.22.0

Thierry Reding

2019-06-17 09:35 UTC #2 lore raw

Re: [PATCH v4 01/10] clk: tegra20/30: Add custom EMC clock implementation

Quoted 44 lines · 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 <[email protected]>
---
 drivers/clk/tegra/Makefile          |   2 +
 drivers/clk/tegra/clk-tegra20-emc.c | 305 ++++++++++++++++++++++++++++
 drivers/clk/tegra/clk-tegra20.c     |  55 ++---
 drivers/clk/tegra/clk-tegra30.c     |  38 +++-
 drivers/clk/tegra/clk.h             |   6 +
 include/linux/clk/tegra.h           |  14 ++
 6 files changed, 368 insertions(+), 52 deletions(-)
 create mode 100644 drivers/clk/tegra/clk-tegra20-emc.c
drivers/clk/tegra/Makefile +2 -0
index 4812e45c2214..df966ca06788 100644
@@ -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
drivers/clk/tegra/clk-tegra20-emc.c new file +0 -0
new file mode 100644
index 000000000000..b7f64ad5c04c

drivers/clk/tegra/clk-tegra20-emc.c line 1

Quoted 1 line

drivers/clk/tegra/clk-tegra20-emc.c new file +1 -0
@@ -0,0 +1,305 @@
+// SPDX-License-Identifier: GPL-2.0

Perhaps you want to add copyright information here? Part of this is copied from other drivers, so keep that copyright intact. But there's also quite a bit of new code here, so also make sure to add yourself.

Quoted 261 lines

drivers/clk/tegra/clk-tegra20-emc.c new file +261 -0

+
+#include <linux/bits.h>
+#include <linux/clk-provider.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;
+
+	tegra20_clk_emc_round_cb *round_cb;
+	void *cb_arg;
+};
+
+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);
+
+	fence_udelay(1, 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);
+
+	fence_udelay(1, 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);
+
+	fence_udelay(1, 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->cb_arg);
+	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(tegra20_clk_emc_round_cb *round_cb,
+					void *cb_arg)
+{
+	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->cb_arg = cb_arg;
+	}
+}
+
+bool tegra20_clk_emc_driver_available(struct clk_hw *emc_hw)
+{
+	return to_tegra_clk_emc(emc_hw)->round_cb != NULL;
+}
+
+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;
+}
+

drivers/clk/tegra/clk-tegra20-emc.c line 272

Quoted 10 lines

drivers/clk/tegra/clk-tegra20-emc.c new file +10 -0

+void tegra30_clk_set_emc_round_callback(tegra30_clk_emc_round_cb *round_cb,
+					void *cb_arg)
+{
+	tegra20_clk_set_emc_round_callback(round_cb, cb_arg);
+}
+
+bool tegra30_clk_emc_driver_available(struct clk_hw *emc_hw)
+{
+	return tegra20_clk_emc_driver_available(emc_hw);
+}

Do we really need to make this distinction? Do you have any work in progress patches that would need to override these Tegra30 specific bits by code that's not the same as the Tegra20 variant? I don't see why you would want to duplicate this if there's no use to it. Or perhaps I'm missing something?

Quoted 244 lines

drivers/clk/tegra/clk-tegra20-emc.c new file +33 -0

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

drivers/clk/tegra/clk-tegra20.c +14 ~~-41~~

index bcd871134f45..f937a0f35afb 100644
@@ -130,8 +130,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,	\

@@ -760,7 +758,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),

@@ -787,41 +784,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;

@@ -835,7 +797,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,

@@ -1115,6 +1083,8 @@ static struct clk *tegra20_clk_src_onecell_get(struct of_phandle_args *clkspec,
 	if (IS_ERR(clk))
 		return clk;
 
+	hw = __clk_get_hw(clk);
+
 	/*
 	 * Tegra20 CDEV1 and CDEV2 clocks are a bit special case, their parent
 	 * clock is created by the pinctrl driver. It is possible for clk user

@@ -1124,13 +1094,16 @@ static struct clk *tegra20_clk_src_onecell_get(struct of_phandle_args *clkspec,
 	 */
 	if (clkspec->args[0] == TEGRA20_CLK_CDEV1 ||
 	    clkspec->args[0] == TEGRA20_CLK_CDEV2) {
-		hw = __clk_get_hw(clk);
-
 		parent_hw = clk_hw_get_parent(hw);
 		if (!parent_hw)
 			return ERR_PTR(-EPROBE_DEFER);
 	}
 
+	if (clkspec->args[0] == TEGRA20_CLK_EMC) {
+		if (!tegra20_clk_emc_driver_available(hw))
+			return ERR_PTR(-EPROBE_DEFER);
+	}
+
 	return clk;
 }

drivers/clk/tegra/clk-tegra30.c +27 ~~-11~~

index 7b4c6a488527..fab075808c20 100644
@@ -151,7 +151,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)	\

@@ -808,7 +807,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" };

@@ -995,7 +994,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" };

@@ -1044,14 +1042,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 */

@@ -1302,6 +1298,26 @@ 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_hw *hw;
+	struct clk *clk;
+
+	clk = of_clk_src_onecell_get(clkspec, data);
+	if (IS_ERR(clk))
+		return clk;
+
+	hw = __clk_get_hw(clk);
+
+	if (clkspec->args[0] == TEGRA30_CLK_EMC) {
+		if (!tegra30_clk_emc_driver_available(hw))
+			return ERR_PTR(-EPROBE_DEFER);
+	}
+
+	return clk;
+}
+
 static void __init tegra30_clock_init(struct device_node *np)
 {
 	struct device_node *node;

@@ -1345,7 +1361,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;

drivers/clk/tegra/clk.h +6 -0

index 905bf1096558..1eb2ec20e343 100644
@@ -838,4 +838,10 @@ int div_frac_get(unsigned long rate, unsigned parent_rate, u8 width,
 		udelay(delay);		\
 	} while (0)
 
+bool tegra20_clk_emc_driver_available(struct clk_hw *emc_hw);
+struct clk *tegra20_clk_register_emc(void __iomem *ioaddr);
+
+bool tegra30_clk_emc_driver_available(struct clk_hw *emc_hw);
+struct clk *tegra30_clk_register_emc(void __iomem *ioaddr);
+
 #endif /* TEGRA_CLK_H */

include/linux/clk/tegra.h +0 -0

index b8aef62cc3f5..8546e28aa518 100644

include/linux/clk/tegra.h extern void tegra210_put_utmipll_in_iddq(void); · line 128

Quoted 10 lines

include/linux/clk/tegra.h +7 -0

@@ -119,4 +119,18 @@ 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;
+
+typedef long (tegra20_clk_emc_round_cb)(unsigned long rate,
+					unsigned long min_rate,
+					unsigned long max_rate,
+					void *arg);
+#define tegra30_clk_emc_round_cb	tegra20_clk_emc_round_cb

Again, I don't see any advantage in quirky things like this. It seems to me like the only reason why this exists is so that Tegra30 code doesn't have to call functions that start with a tegra20_ prefix. However, we already have code that does similar things elsewhere, so I think this can be considered "common" practice. No need for this duplication.

Again, if I'm missing something please let me know. Might be worth noting why this is done in a code comment or the commit message.

Thierry

Quoted 11 lines

include/linux/clk/tegra.h +7 -0

+
+void tegra20_clk_set_emc_round_callback(tegra20_clk_emc_round_cb *round_cb,
+					void *cb_arg);
+void tegra30_clk_set_emc_round_callback(tegra30_clk_emc_round_cb *round_cb,
+					void *cb_arg);
+int tegra30_clk_prepare_emc_mc_same_freq(struct clk *emc_clk, bool same);
+
 #endif /* __LINUX_CLK_TEGRA_H_ */

Signature · 1 line

2.22.0

Dmitry Osipenko

author

2019-06-17 15:00 UTC #3 lore raw

Re: [PATCH v4 01/10] clk: tegra20/30: Add custom EMC clock implementation

17.06.2019 12:35, Thierry Reding пишет:

Quoted 42 lines · Dmitry Osipenko

Quoted 41 lines
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 <[email protected]>
---
 drivers/clk/tegra/Makefile          |   2 +
 drivers/clk/tegra/clk-tegra20-emc.c | 305 ++++++++++++++++++++++++++++
 drivers/clk/tegra/clk-tegra20.c     |  55 ++---
 drivers/clk/tegra/clk-tegra30.c     |  38 +++-
 drivers/clk/tegra/clk.h             |   6 +
 include/linux/clk/tegra.h           |  14 ++
 6 files changed, 368 insertions(+), 52 deletions(-)
 create mode 100644 drivers/clk/tegra/clk-tegra20-emc.c
drivers/clk/tegra/Makefile +2 -0
index 4812e45c2214..df966ca06788 100644
@@ -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
drivers/clk/tegra/clk-tegra20-emc.c new file +0 -0
new file mode 100644
index 000000000000..b7f64ad5c04c

Quoted 8 lines

drivers/clk/tegra/clk-tegra20-emc.c line 1
Quoted 4 lines
drivers/clk/tegra/clk-tegra20-emc.c new file +1 -0
@@ -0,0 +1,305 @@
+// SPDX-License-Identifier: GPL-2.0
Perhaps you want to add copyright information here? Part of this is copied from other drivers, so keep that copyright intact. But there's also quite a bit of new code here, so also make sure to add yourself.

Okay! And it's true that I initially used clk-emc as a template.

[snip]

Quoted 8 lines

drivers/clk/tegra/clk-tegra20-emc.c new file +8 -0

+void tegra30_clk_set_emc_round_callback(tegra30_clk_emc_round_cb *round_cb,
+					void *cb_arg)
+{
+	tegra20_clk_set_emc_round_callback(round_cb, cb_arg);
+}
+
+bool tegra30_clk_emc_driver_available(struct clk_hw *emc_hw)
+{

Quoted 8 lines

drivers/clk/tegra/clk-tegra20-emc.c line 11
Quoted 2 lines
drivers/clk/tegra/clk-tegra20-emc.c new file +2 -0
+	return tegra20_clk_emc_driver_available(emc_hw);
+}
Do we really need to make this distinction? Do you have any work in progress patches that would need to override these Tegra30 specific bits by code that's not the same as the Tegra20 variant? I don't see why you would want to duplicate this if there's no use to it. Or perhaps I'm missing something?

There are no other patches planned for this code. The primary reason for the distinction is that I don't like to have T20 functions mixed with T30 because this leads to inconsistency and confusion.

[snip]

Quoted 5 lines

Again, I don't see any advantage in quirky things like this. It seems to me like the only reason why this exists is so that Tegra30 code doesn't have to call functions that start with a tegra20_ prefix. However, we already have code that does similar things elsewhere, so I think this can be considered "common" practice. No need for this duplication.

Oh, well. But this is not a very good practice in my opinion. I'll adhere to yours comment in v5.

Quoted 2 lines

Again, if I'm missing something please let me know. Might be worth noting why this is done in a code comment or the commit message.

You got everything right.

Thierry Reding

2019-06-18 12:21 UTC #4 lore raw

Re: [PATCH v4 01/10] clk: tegra20/30: Add custom EMC clock implementation

In reply to Dmitry Osipenko

Quoted 14 lines · 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 <[email protected]>

Quoted 9 lines

---
 drivers/clk/tegra/Makefile          |   2 +
 drivers/clk/tegra/clk-tegra20-emc.c | 305 ++++++++++++++++++++++++++++
 drivers/clk/tegra/clk-tegra20.c     |  55 ++---
 drivers/clk/tegra/clk-tegra30.c     |  38 +++-
 drivers/clk/tegra/clk.h             |   6 +
 include/linux/clk/tegra.h           |  14 ++
 6 files changed, 368 insertions(+), 52 deletions(-)
 create mode 100644 drivers/clk/tegra/clk-tegra20-emc.c

Hi Mike, Stephen,

The remaining patches of this series have a build-time dependency on this clock driver patch. Would you mind if I pick this up into the Tegra tree, so that I can resolve the dependency there? I can send a pull request of the stable branch with this one patch if we need to resolve a conflict between the clk and Tegra trees.

Thierry

Stephen Boyd

maintainer

2019-06-19 01:14 UTC #5 lore raw

Re: [PATCH v4 01/10] clk: tegra20/30: Add custom EMC clock implementation

Quoting Thierry Reding (2019-06-18 05:21:08)

Quoted 25 lines · Dmitry Osipenko

Quoted 14 lines
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 <[email protected]>
Quoted 9 lines
---
 drivers/clk/tegra/Makefile          |   2 +
 drivers/clk/tegra/clk-tegra20-emc.c | 305 ++++++++++++++++++++++++++++
 drivers/clk/tegra/clk-tegra20.c     |  55 ++---
 drivers/clk/tegra/clk-tegra30.c     |  38 +++-
 drivers/clk/tegra/clk.h             |   6 +
 include/linux/clk/tegra.h           |  14 ++
 6 files changed, 368 insertions(+), 52 deletions(-)
 create mode 100644 drivers/clk/tegra/clk-tegra20-emc.c

Quoted 8 lines

Hi Mike, Stephen,
The remaining patches of this series have a build-time dependency on this clock driver patch. Would you mind if I pick this up into the Tegra tree, so that I can resolve the dependency there? I can send a pull request of the stable branch with this one patch if we need to resolve a conflict between the clk and Tegra trees.

Sure. I have review comments though so hopefully they can be addressed first.

Stephen Boyd

maintainer

2019-06-19 01:14 UTC #6 lore raw

Re: [PATCH v4 01/10] clk: tegra20/30: Add custom EMC clock implementation

In reply to Dmitry Osipenko

Quoting Dmitry Osipenko (2019-06-16 16:35:42)

Quoted 3 lines

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.

Why isn't it available? Please add this information to the commit text.

Quoted 12 lines

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 <[email protected]>

[...]

Quoted 30 lines

drivers/clk/tegra/clk-tegra20-emc.c new file +24 -0

new file mode 100644
index 000000000000..b7f64ad5c04c
@@ -0,0 +1,305 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/bits.h>
+#include <linux/clk-provider.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
+

[...]

Quoted 12 lines

drivers/clk/tegra/clk-tegra20-emc.c new file +12 -0

+void tegra20_clk_set_emc_round_callback(tegra20_clk_emc_round_cb *round_cb,
+                                       void *cb_arg)
+{
+       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;

drivers/clk/tegra/clk-tegra20-emc.c line 235

Quoted 10 lines

drivers/clk/tegra/clk-tegra20-emc.c new file +10 -0

+               emc->cb_arg = cb_arg;
+       }
+}
+
+bool tegra20_clk_emc_driver_available(struct clk_hw *emc_hw)
+{
+       return to_tegra_clk_emc(emc_hw)->round_cb != NULL;
+}
+
+struct clk *tegra20_clk_register_emc(void __iomem *ioaddr)

Is this used outside this file?

drivers/clk/tegra/clk-tegra20-emc.c line 247

Quoted 12 lines

drivers/clk/tegra/clk-tegra20-emc.c new file +12 -0

+{
+       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;

Can you please add a comment in the code why this clk is critical?

Quoted 27 lines

drivers/clk/tegra/clk-tegra20-emc.c new file +27 -0

+       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(tegra30_clk_emc_round_cb *round_cb,
+                                       void *cb_arg)
+{
+       tegra20_clk_set_emc_round_callback(round_cb, cb_arg);
+}
+
+bool tegra30_clk_emc_driver_available(struct clk_hw *emc_hw)
+{
+       return tegra20_clk_emc_driver_available(emc_hw);
+}
+
+struct clk *tegra30_clk_register_emc(void __iomem *ioaddr)

drivers/clk/tegra/clk-tegra20-emc.c line 284

Quoted 10 lines

drivers/clk/tegra/clk-tegra20-emc.c new file +10 -0

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

It would be nicer to not use __clk_get_hw() and have the above function return the clk_hw pointer instead. Then some driver can return the clk pointer from there, if it's even needed for anything?

Quoted 70 lines

drivers/clk/tegra/clk-tegra20-emc.c new file +21 -0

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

drivers/clk/tegra/clk-tegra20.c +7 -2

index bcd871134f45..f937a0f35afb 100644
@@ -1115,6 +1083,8 @@ static struct clk *tegra20_clk_src_onecell_get(struct of_phandle_args *clkspec,
        if (IS_ERR(clk))
                return clk;
 
+       hw = __clk_get_hw(clk);
+
        /*
         * Tegra20 CDEV1 and CDEV2 clocks are a bit special case, their parent
         * clock is created by the pinctrl driver. It is possible for clk user

@@ -1124,13 +1094,16 @@ static struct clk *tegra20_clk_src_onecell_get(struct of_phandle_args *clkspec,
         */
        if (clkspec->args[0] == TEGRA20_CLK_CDEV1 ||
            clkspec->args[0] == TEGRA20_CLK_CDEV2) {
-               hw = __clk_get_hw(clk);
-
                parent_hw = clk_hw_get_parent(hw);
                if (!parent_hw)
                        return ERR_PTR(-EPROBE_DEFER);
        }
 
+       if (clkspec->args[0] == TEGRA20_CLK_EMC) {
+               if (!tegra20_clk_emc_driver_available(hw))
+                       return ERR_PTR(-EPROBE_DEFER);
+       }
+
        return clk;
 }

drivers/clk/tegra/clk-tegra30.c +9 -0

index 7b4c6a488527..fab075808c20 100644
@@ -1302,6 +1298,26 @@ 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_hw *hw;
+       struct clk *clk;
+
+       clk = of_clk_src_onecell_get(clkspec, data);
+       if (IS_ERR(clk))
+               return clk;

drivers/clk/tegra/clk-tegra30.c static struct tegra_audio_clk_info tegra30_audio_plls[] = { · line 1319

Quoted 10 lines

drivers/clk/tegra/clk-tegra30.c +10 -0

+
+       hw = __clk_get_hw(clk);
+
+       if (clkspec->args[0] == TEGRA30_CLK_EMC) {
+               if (!tegra30_clk_emc_driver_available(hw))
+                       return ERR_PTR(-EPROBE_DEFER);
+       }
+
+       return clk;
+}

This above function makes me uneasy because it looks like a clk_get() on top of a clk_get()?

Quoted 4 lines

drivers/clk/tegra/clk-tegra30.c +1 -0

+
 static void __init tegra30_clock_init(struct device_node *np)
 {
        struct device_node *node;

Dmitry Osipenko

author

2019-06-19 15:37 UTC #7 lore raw

Re: [PATCH v4 01/10] clk: tegra20/30: Add custom EMC clock implementation

19.06.2019 4:14, Stephen Boyd пишет:

Quoted 6 lines

Quoting Dmitry Osipenko (2019-06-16 16:35:42)
Quoted 3 lines
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.
Why isn't it available? Please add this information to the commit text.

Ok! It's not available because only the EMC driver has information about available memory timings and thus about the available rates (and parents consequently).

Quoted 60 lines

Quoted 12 lines
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 <[email protected]>
[...]
Quoted 30 lines
drivers/clk/tegra/clk-tegra20-emc.c new file +24 -0
new file mode 100644
index 000000000000..b7f64ad5c04c
@@ -0,0 +1,305 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/bits.h>
+#include <linux/clk-provider.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
+
[...]
Quoted 16 lines
drivers/clk/tegra/clk-tegra20-emc.c new file +16 -0
+void tegra20_clk_set_emc_round_callback(tegra20_clk_emc_round_cb *round_cb,
+                                       void *cb_arg)
+{
+       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->cb_arg = cb_arg;
+       }
+}
+

Quoted 8 lines

drivers/clk/tegra/clk-tegra20-emc.c line 235
Quoted 6 lines
drivers/clk/tegra/clk-tegra20-emc.c new file +6 -0
+bool tegra20_clk_emc_driver_available(struct clk_hw *emc_hw)
+{
+       return to_tegra_clk_emc(emc_hw)->round_cb != NULL;
+}
+
+struct clk *tegra20_clk_register_emc(void __iomem *ioaddr)
Is this used outside this file?

Yes, it is. It is getting used even in this patch, you just snipped it off in the reply.

Quoted 6 lines

drivers/clk/tegra/clk-tegra20-emc.c new file +6 -0

+{
+       struct tegra_clk_emc *emc;
+       struct clk_init_data init;
+       struct clk *clk;
+
+       emc = kzalloc(sizeof(*emc), GFP_KERNEL);

Quoted 8 lines

drivers/clk/tegra/clk-tegra20-emc.c line 247
Quoted 6 lines
drivers/clk/tegra/clk-tegra20-emc.c new file +6 -0
+       if (!emc)
+               return NULL;
+
+       init.name = "emc";
+       init.ops = &tegra_clk_emc_ops;
+       init.flags = CLK_IS_CRITICAL;
Can you please add a comment in the code why this clk is critical?

Okay!

Quoted 33 lines

drivers/clk/tegra/clk-tegra20-emc.c new file +33 -0

+       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(tegra30_clk_emc_round_cb *round_cb,
+                                       void *cb_arg)
+{
+       tegra20_clk_set_emc_round_callback(round_cb, cb_arg);
+}
+
+bool tegra30_clk_emc_driver_available(struct clk_hw *emc_hw)
+{
+       return tegra20_clk_emc_driver_available(emc_hw);
+}
+
+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);

Quoted 8 lines

drivers/clk/tegra/clk-tegra20-emc.c line 284
Quoted 4 lines
drivers/clk/tegra/clk-tegra20-emc.c new file +4 -0
+       if (!clk)
+               return NULL;
+
+       hw = __clk_get_hw(clk);
It would be nicer to not use __clk_get_hw() and have the above function return the clk_hw pointer instead. Then some driver can return the clk pointer from there, if it's even needed for anything?

This is solely for internal use by the tegra-clk driver itself, this function isn't publicly exposed. Again, you snipped off a lot in the reply, please take a look at the original patch.

Technically I could return clk_hw from here, but it doesn't make much sense in the context of the tegra-clk driver. Please see how these functions are getting used in the original patch.

In short, tegra-clk driver operates with clk struct and not clk_hw. You already was asking the same question in v3 and I replied that converting the whole driver for clk_hw will take a lot of effort. I suppose it will be somewhat similar to what was done for the IMX driver recently [1].

[1] https://lkml.org/lkml/2019/5/2/170

Quoted 75 lines

drivers/clk/tegra/clk-tegra20-emc.c new file +21 -0

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

drivers/clk/tegra/clk-tegra20.c +7 -2

index bcd871134f45..f937a0f35afb 100644
@@ -1115,6 +1083,8 @@ static struct clk *tegra20_clk_src_onecell_get(struct of_phandle_args *clkspec,
        if (IS_ERR(clk))
                return clk;
 
+       hw = __clk_get_hw(clk);
+
        /*
         * Tegra20 CDEV1 and CDEV2 clocks are a bit special case, their parent
         * clock is created by the pinctrl driver. It is possible for clk user

@@ -1124,13 +1094,16 @@ static struct clk *tegra20_clk_src_onecell_get(struct of_phandle_args *clkspec,
         */
        if (clkspec->args[0] == TEGRA20_CLK_CDEV1 ||
            clkspec->args[0] == TEGRA20_CLK_CDEV2) {
-               hw = __clk_get_hw(clk);
-
                parent_hw = clk_hw_get_parent(hw);
                if (!parent_hw)
                        return ERR_PTR(-EPROBE_DEFER);
        }
 
+       if (clkspec->args[0] == TEGRA20_CLK_EMC) {
+               if (!tegra20_clk_emc_driver_available(hw))
+                       return ERR_PTR(-EPROBE_DEFER);
+       }
+
        return clk;
 }

drivers/clk/tegra/clk-tegra30.c +14 -0

index 7b4c6a488527..fab075808c20 100644
@@ -1302,6 +1298,26 @@ 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_hw *hw;
+       struct clk *clk;
+
+       clk = of_clk_src_onecell_get(clkspec, data);
+       if (IS_ERR(clk))
+               return clk;
+
+       hw = __clk_get_hw(clk);
+
+       if (clkspec->args[0] == TEGRA30_CLK_EMC) {
+               if (!tegra30_clk_emc_driver_available(hw))

Quoted 8 lines

drivers/clk/tegra/clk-tegra30.c static struct tegra_audio_clk_info tegra30_audio_plls[] = { · line 1319
Quoted 5 lines
drivers/clk/tegra/clk-tegra30.c +5 -0
+                       return ERR_PTR(-EPROBE_DEFER);
+       }
+
+       return clk;
+}
This above function makes me uneasy because it looks like a clk_get() on top of a clk_get()?

Yes, this way we're intercepting clk_get() within the driver, which is a very nice feature. We're already doing that in the clk-tegra20.c, can't see any problems with that.

Dmitry Osipenko

author

2019-06-16 23:35 UTC #8 lore raw

PATCH v4 2/10 memory: tegra20-emc: Drop setting EMC rate to max on probe

In reply to Dmitry Osipenko

The memory frequency scaling will be managed by tegra20-devfreq driver and PM QoS once all the prerequisite patches will get upstreamed. The parent clock is now managed by the clock driver and we also should assume that PLLM rate can't be changed on some devices (Galaxy Tab 10.1 for example). Altogether there is no point in touching of clock's rate from the EMC driver.

Signed-off-by Dmitry Osipenko <[email protected]>

---
 drivers/memory/tegra/tegra20-emc.c | 45 ------------------------------
 1 file changed, 45 deletions(-)

drivers/memory/tegra/tegra20-emc.c +0 ~~-45~~

index 9ee5bef49e47..55ac3863a354 100644
@@ -424,41 +424,6 @@ static int emc_setup_hw(struct tegra_emc *emc)
 	return 0;
 }
 
-static int emc_init(struct tegra_emc *emc, unsigned long rate)
-{
-	int err;
-
-	err = clk_set_parent(emc->emc_mux, emc->backup_clk);
-	if (err) {
-		dev_err(emc->dev,
-			"failed to reparent to backup source: %d\n", err);
-		return err;
-	}
-
-	err = clk_set_rate(emc->pll_m, rate);
-	if (err) {
-		dev_err(emc->dev,
-			"failed to change pll_m rate: %d\n", err);
-		return err;
-	}
-
-	err = clk_set_parent(emc->emc_mux, emc->pll_m);
-	if (err) {
-		dev_err(emc->dev,
-			"failed to reparent to pll_m: %d\n", err);
-		return err;
-	}
-
-	err = clk_set_rate(emc->clk, rate);
-	if (err) {
-		dev_err(emc->dev,
-			"failed to change emc rate: %d\n", err);
-		return err;
-	}
-
-	return 0;
-}
-
 static int tegra_emc_probe(struct platform_device *pdev)
 {
 	struct device_node *np;

@@ -550,18 +515,8 @@ static int tegra_emc_probe(struct platform_device *pdev)
 		goto put_backup;
 	}
 
-	/* set DRAM clock rate to maximum */
-	err = emc_init(emc, emc->timings[emc->num_timings - 1].rate);
-	if (err) {
-		dev_err(&pdev->dev, "failed to initialize EMC clock rate: %d\n",
-			err);
-		goto unreg_notifier;
-	}
-
 	return 0;
 
-unreg_notifier:
-	clk_notifier_unregister(emc->clk, &emc->clk_nb);
 put_backup:
 	clk_put(emc->backup_clk);
 put_pll_m:

Signature · 1 line

2.22.0

Dmitry Osipenko

author

2019-06-16 23:35 UTC #9 lore raw

PATCH v4 3/10 memory: tegra20-emc: Adapt for clock driver changes

In reply to Dmitry Osipenko

The emc_mux clock is gone now and EMC driver should provide the clock rounding functionality.

Signed-off-by Dmitry Osipenko <[email protected]>

---
 drivers/memory/tegra/tegra20-emc.c | 55 ++++++++++++++++++++++++------
 1 file changed, 45 insertions(+), 10 deletions(-)

drivers/memory/tegra/tegra20-emc.c +45 ~~-10~~

index 55ac3863a354..d3e1f898d745 100644
@@ -6,6 +6,7 @@
  */
 
 #include <linux/clk.h>
+#include <linux/clk/tegra.h>
 #include <linux/completion.h>
 #include <linux/err.h>
 #include <linux/interrupt.h>

@@ -138,7 +139,6 @@ struct tegra_emc {
 	struct completion clk_handshake_complete;
 	struct notifier_block clk_nb;
 	struct clk *backup_clk;
-	struct clk *emc_mux;
 	struct clk *pll_m;
 	struct clk *clk;
 	void __iomem *regs;

@@ -424,6 +424,44 @@ static int emc_setup_hw(struct tegra_emc *emc)
 	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;
+
+	min_rate = min(min_rate, emc->timings[emc->num_timings - 1].rate);
+
+	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 device_node *np;

@@ -480,18 +518,20 @@ static int tegra_emc_probe(struct platform_device *pdev)
 		return err;
 	}
 
+	tegra20_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);
-		return err;
+		goto unset_cb;
 	}
 
 	emc->pll_m = clk_get_sys(NULL, "pll_m");
 	if (IS_ERR(emc->pll_m)) {
 		err = PTR_ERR(emc->pll_m);
 		dev_err(&pdev->dev, "failed to get pll_m clock: %d\n", err);
-		return err;
+		goto unset_cb;
 	}
 
 	emc->backup_clk = clk_get_sys(NULL, "pll_p");

@@ -501,13 +541,6 @@ static int tegra_emc_probe(struct platform_device *pdev)
 		goto put_pll_m;
 	}
 
-	emc->emc_mux = clk_get_parent(emc->clk);
-	if (IS_ERR(emc->emc_mux)) {
-		err = PTR_ERR(emc->emc_mux);
-		dev_err(&pdev->dev, "failed to get emc_mux clock: %d\n", err);
-		goto put_backup;
-	}
-
 	err = clk_notifier_register(emc->clk, &emc->clk_nb);
 	if (err) {
 		dev_err(&pdev->dev, "failed to register clk notifier: %d\n",

@@ -521,6 +554,8 @@ static int tegra_emc_probe(struct platform_device *pdev)
 	clk_put(emc->backup_clk);
 put_pll_m:
 	clk_put(emc->pll_m);
+unset_cb:
+	tegra20_clk_set_emc_round_callback(NULL, NULL);
 
 	return err;
 }

Signature · 1 line

2.22.0

Dmitry Osipenko

author

2019-06-16 23:35 UTC #10 lore raw

PATCH v4 4/10 memory: tegra20-emc: Include io.h instead of iopoll.h

In reply to Dmitry Osipenko

The register polling code was gone, but the included header change was missed. Fix it up for consistency.

Signed-off-by Dmitry Osipenko <[email protected]>

---
 drivers/memory/tegra/tegra20-emc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/memory/tegra/tegra20-emc.c +1 -1

index d3e1f898d745..43aef3614b65 100644
@@ -10,7 +10,7 @@
 #include <linux/completion.h>
 #include <linux/err.h>
 #include <linux/interrupt.h>
-#include <linux/iopoll.h>
+#include <linux/io.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/of.h>

Signature · 1 line

2.22.0

Dmitry Osipenko

author

2019-06-16 23:35 UTC #11 lore raw

PATCH v4 5/10 memory: tegra20-emc: Replace clk_get_sys with devm_clk_get

In reply to Dmitry Osipenko

There is no problem for drivers to request pll_m and pll_p clocks for the device, hence there is no need to use clk_get_sys() and it could be replaced with devm_clk_get() for consistency.

Signed-off-by Dmitry Osipenko <[email protected]>

---
 drivers/memory/tegra/tegra20-emc.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

drivers/memory/tegra/tegra20-emc.c +4 -8

index 43aef3614b65..527aa4b90e95 100644
@@ -527,33 +527,29 @@ static int tegra_emc_probe(struct platform_device *pdev)
 		goto unset_cb;
 	}
 
-	emc->pll_m = clk_get_sys(NULL, "pll_m");
+	emc->pll_m = devm_clk_get(&pdev->dev, "pll_m");
 	if (IS_ERR(emc->pll_m)) {
 		err = PTR_ERR(emc->pll_m);
 		dev_err(&pdev->dev, "failed to get pll_m clock: %d\n", err);
 		goto unset_cb;
 	}
 
-	emc->backup_clk = clk_get_sys(NULL, "pll_p");
+	emc->backup_clk = devm_clk_get(&pdev->dev, "pll_p");
 	if (IS_ERR(emc->backup_clk)) {
 		err = PTR_ERR(emc->backup_clk);
 		dev_err(&pdev->dev, "failed to get pll_p clock: %d\n", err);
-		goto put_pll_m;
+		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 put_backup;
+		goto unset_cb;
 	}
 
 	return 0;
 
-put_backup:
-	clk_put(emc->backup_clk);
-put_pll_m:
-	clk_put(emc->pll_m);
 unset_cb:
 	tegra20_clk_set_emc_round_callback(NULL, NULL);

Signature · 1 line

2.22.0

Thierry Reding

2019-06-17 09:46 UTC #12 lore raw

Re: [PATCH v4 05/10] memory: tegra20-emc: Replace clk_get_sys with devm_clk_get

Quoted 14 lines · Dmitry Osipenko

There is no problem for drivers to request pll_m and pll_p clocks for the device, hence there is no need to use clk_get_sys() and it could be replaced with devm_clk_get() for consistency.
Signed-off-by Dmitry Osipenko <[email protected]>
---
 drivers/memory/tegra/tegra20-emc.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)
drivers/memory/tegra/tegra20-emc.c +0 -0
index 43aef3614b65..527aa4b90e95 100644

drivers/memory/tegra/tegra20-emc.c static int tegra_emc_probe(struct platform_device *pdev) · line 530

Quoted 5 lines

drivers/memory/tegra/tegra20-emc.c +1 -1

@@ -527,33 +527,29 @@ static int tegra_emc_probe(struct platform_device *pdev)
 		goto unset_cb;
 	}
 
-	emc->pll_m = clk_get_sys(NULL, "pll_m");
+	emc->pll_m = devm_clk_get(&pdev->dev, "pll_m");

Interesting... I didn't know that clk_get() had a fallback path to return clk_get_sys() if the named clock wasn't found in DT. That's nice.

Looks good to me.

Thierry

Quoted 35 lines

drivers/memory/tegra/tegra20-emc.c +3 -7

 	if (IS_ERR(emc->pll_m)) {
 		err = PTR_ERR(emc->pll_m);
 		dev_err(&pdev->dev, "failed to get pll_m clock: %d\n", err);
 		goto unset_cb;
 	}
 
-	emc->backup_clk = clk_get_sys(NULL, "pll_p");
+	emc->backup_clk = devm_clk_get(&pdev->dev, "pll_p");
 	if (IS_ERR(emc->backup_clk)) {
 		err = PTR_ERR(emc->backup_clk);
 		dev_err(&pdev->dev, "failed to get pll_p clock: %d\n", err);
-		goto put_pll_m;
+		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 put_backup;
+		goto unset_cb;
 	}
 
 	return 0;
 
-put_backup:
-	clk_put(emc->backup_clk);
-put_pll_m:
-	clk_put(emc->pll_m);
 unset_cb:
 	tegra20_clk_set_emc_round_callback(NULL, NULL);

Signature · 1 line

2.22.0

Dmitry Osipenko

author

2019-06-17 15:01 UTC #13 lore raw

Re: [PATCH v4 05/10] memory: tegra20-emc: Replace clk_get_sys with devm_clk_get

17.06.2019 12:46, Thierry Reding пишет:

Quoted 18 lines · Dmitry Osipenko

Quoted 17 lines
There is no problem for drivers to request pll_m and pll_p clocks for the device, hence there is no need to use clk_get_sys() and it could be replaced with devm_clk_get() for consistency.
Signed-off-by Dmitry Osipenko <[email protected]>
---
 drivers/memory/tegra/tegra20-emc.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)
drivers/memory/tegra/tegra20-emc.c +0 -0
index 43aef3614b65..527aa4b90e95 100644
@@ -527,33 +527,29 @@ static int tegra_emc_probe(struct platform_device *pdev)
 		goto unset_cb;
 	}
 

Quoted 8 lines

drivers/memory/tegra/tegra20-emc.c static int tegra_emc_probe(struct platform_device *pdev) · line 530
Quoted 2 lines
drivers/memory/tegra/tegra20-emc.c +1 -1
-	emc->pll_m = clk_get_sys(NULL, "pll_m");
+	emc->pll_m = devm_clk_get(&pdev->dev, "pll_m");
Interesting... I didn't know that clk_get() had a fallback path to return clk_get_sys() if the named clock wasn't found in DT. That's nice.
Looks good to me.

Yes, I didn't know either until recently. Thanks!

Dmitry Osipenko

author

2019-06-16 23:35 UTC #14 lore raw

PATCH v4 6/10 dt-bindings: memory: Add binding for NVIDIA Tegra30 External Memory Controller

In reply to 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.

Note that the memory timing description is given in a platform-specific form because there is no detailed information on how to convert a typical-common DDR timing into the register values. The timing format is borrowed from downstream kernel, hence there is no hurdle in regards to upstreaming of memory timings for the boards.

Signed-off-by Dmitry Osipenko <[email protected]>

---
 .../memory-controllers/nvidia,tegra30-emc.txt | 249 ++++++++++++++++++
 1 file changed, 249 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-emc.txt

Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-emc.txt new file +249 -0

new file mode 100644
index 000000000000..c2bc1dbcaa07
@@ -0,0 +1,249 @@
+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:
+
+	external-memory-controller {
+		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>;
+
+		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 */
+				>;
+			};
+		};
+	};

Signature · 1 line

2.22.0

Dmitry Osipenko

author

2019-06-16 23:35 UTC #15 lore raw

PATCH v4 7/10 memory: tegra: Introduce Tegra30 EMC driver

In reply to 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 <[email protected]>

---
 drivers/memory/tegra/Kconfig       |   10 +
 drivers/memory/tegra/Makefile      |    1 +
 drivers/memory/tegra/mc.c          |    9 +-
 drivers/memory/tegra/mc.h          |   30 +-
 drivers/memory/tegra/tegra30-emc.c | 1197 ++++++++++++++++++++++++++++
 drivers/memory/tegra/tegra30.c     |   44 +
 include/soc/tegra/mc.h             |    2 +-
 7 files changed, 1278 insertions(+), 15 deletions(-)
 create mode 100644 drivers/memory/tegra/tegra30-emc.c

drivers/memory/tegra/Kconfig +10 -0

index 4680124ddcab..fbfbaada61a2 100644
@@ -17,6 +17,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

drivers/memory/tegra/Makefile +1 -0

index 3971a6b7c487..3d23c4261104 100644
@@ -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

drivers/memory/tegra/mc.c +4 -5

index 163b6c69e651..eaebe371625c 100644
@@ -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 },

@@ -310,7 +307,7 @@ static int tegra_mc_setup_latency_allowance(struct tegra_mc *mc)
 	return 0;
 }
 
-void tegra_mc_write_emem_configuration(struct tegra_mc *mc, unsigned long rate)
+int tegra_mc_write_emem_configuration(struct tegra_mc *mc, unsigned long rate)
 {
 	unsigned int i;
 	struct tegra_mc_timing *timing = NULL;

@@ -325,11 +322,13 @@ void tegra_mc_write_emem_configuration(struct tegra_mc *mc, unsigned long rate)
 	if (!timing) {
 		dev_err(mc->dev, "no memory timing registered for rate %lu\n",
 			rate);
-		return;
+		return -EINVAL;
 	}
 
 	for (i = 0; i < mc->soc->num_emem_regs; ++i)
 		mc_writel(mc, timing->emem_data[i], mc->soc->emem_regs[i]);
+
+	return 0;
 }
 
 unsigned int tegra_mc_get_emem_device_count(struct tegra_mc *mc)

drivers/memory/tegra/mc.h +21 -9

index 392993955c93..0720a1d2023e 100644
@@ -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)
 {

drivers/memory/tegra/tegra30-emc.c new file +1197 -0

1,197 lines not on this page · raw message

drivers/memory/tegra/tegra30.c +44 -0

56 lines not on this page · raw message

include/soc/tegra/mc.h +1 -1

index e489a028ec9f..fbb91f44913b 100644
@@ -184,7 +184,7 @@ struct tegra_mc {
 	spinlock_t lock;
 };
 
-void tegra_mc_write_emem_configuration(struct tegra_mc *mc, unsigned long rate);
+int tegra_mc_write_emem_configuration(struct tegra_mc *mc, unsigned long rate);
 unsigned int tegra_mc_get_emem_device_count(struct tegra_mc *mc);
 
 #endif /* __SOC_TEGRA_MC_H__ */

Signature · 1 line

2.22.0

Thierry Reding

2019-06-17 09:50 UTC #16 lore raw

Quoted 109 lines · Dmitry Osipenko

Signed-off-by Dmitry Osipenko <[email protected]>

---
 drivers/memory/tegra/Kconfig       |   10 +
 drivers/memory/tegra/Makefile      |    1 +
 drivers/memory/tegra/mc.c          |    9 +-
 drivers/memory/tegra/mc.h          |   30 +-
 drivers/memory/tegra/tegra30-emc.c | 1197 ++++++++++++++++++++++++++++
 drivers/memory/tegra/tegra30.c     |   44 +
 include/soc/tegra/mc.h             |    2 +-
 7 files changed, 1278 insertions(+), 15 deletions(-)
 create mode 100644 drivers/memory/tegra/tegra30-emc.c

drivers/memory/tegra/Kconfig +10 -0

index 4680124ddcab..fbfbaada61a2 100644
@@ -17,6 +17,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

drivers/memory/tegra/Makefile +1 -0

index 3971a6b7c487..3d23c4261104 100644
@@ -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

drivers/memory/tegra/mc.c +4 -5

index 163b6c69e651..eaebe371625c 100644
@@ -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 },

@@ -310,7 +307,7 @@ static int tegra_mc_setup_latency_allowance(struct tegra_mc *mc)
 	return 0;
 }
 
-void tegra_mc_write_emem_configuration(struct tegra_mc *mc, unsigned long rate)
+int tegra_mc_write_emem_configuration(struct tegra_mc *mc, unsigned long rate)
 {
 	unsigned int i;
 	struct tegra_mc_timing *timing = NULL;

@@ -325,11 +322,13 @@ void tegra_mc_write_emem_configuration(struct tegra_mc *mc, unsigned long rate)
 	if (!timing) {
 		dev_err(mc->dev, "no memory timing registered for rate %lu\n",
 			rate);
-		return;
+		return -EINVAL;
 	}
 
 	for (i = 0; i < mc->soc->num_emem_regs; ++i)
 		mc_writel(mc, timing->emem_data[i], mc->soc->emem_regs[i]);
+
+	return 0;
 }
 
 unsigned int tegra_mc_get_emem_device_count(struct tegra_mc *mc)

drivers/memory/tegra/mc.h +1 -8

index 392993955c93..0720a1d2023e 100644
@@ -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)

drivers/memory/tegra/mc.h line 26

Quoted 10 lines

drivers/memory/tegra/mc.h +9 -1

-#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)

This /could/ be a separate patch, with it being unrelated to the EMC support, but probably not worth it.

Quoted 19 lines

drivers/memory/tegra/mc.h +10 -0

+#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)
 {

drivers/memory/tegra/tegra30-emc.c new file +0 -0

new file mode 100644
index 000000000000..4700f7c8022e

drivers/memory/tegra/tegra30-emc.c line 6

Quoted 6 lines

drivers/memory/tegra/tegra30-emc.c new file +6 -0

@@ -0,0 +1,1197 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Tegra30 External Memory Controller driver
+ *
+ * Author: Dmitry Osipenko <[email protected]>
+ */

Otherwise looks good to me.

Thierry

Dmitry Osipenko

author

2019-06-17 15:03 UTC #17 lore raw

17.06.2019 12:50, Thierry Reding пишет:

Quoted 115 lines · Dmitry Osipenko

Quoted 114 lines

Signed-off-by Dmitry Osipenko <[email protected]>

---
 drivers/memory/tegra/Kconfig       |   10 +
 drivers/memory/tegra/Makefile      |    1 +
 drivers/memory/tegra/mc.c          |    9 +-
 drivers/memory/tegra/mc.h          |   30 +-
 drivers/memory/tegra/tegra30-emc.c | 1197 ++++++++++++++++++++++++++++
 drivers/memory/tegra/tegra30.c     |   44 +
 include/soc/tegra/mc.h             |    2 +-
 7 files changed, 1278 insertions(+), 15 deletions(-)
 create mode 100644 drivers/memory/tegra/tegra30-emc.c

drivers/memory/tegra/Kconfig +10 -0

index 4680124ddcab..fbfbaada61a2 100644
@@ -17,6 +17,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

drivers/memory/tegra/Makefile +1 -0

index 3971a6b7c487..3d23c4261104 100644
@@ -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

drivers/memory/tegra/mc.c +4 -5

index 163b6c69e651..eaebe371625c 100644
@@ -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 },

@@ -310,7 +307,7 @@ static int tegra_mc_setup_latency_allowance(struct tegra_mc *mc)
 	return 0;
 }
 
-void tegra_mc_write_emem_configuration(struct tegra_mc *mc, unsigned long rate)
+int tegra_mc_write_emem_configuration(struct tegra_mc *mc, unsigned long rate)
 {
 	unsigned int i;
 	struct tegra_mc_timing *timing = NULL;

@@ -325,11 +322,13 @@ void tegra_mc_write_emem_configuration(struct tegra_mc *mc, unsigned long rate)
 	if (!timing) {
 		dev_err(mc->dev, "no memory timing registered for rate %lu\n",
 			rate);
-		return;
+		return -EINVAL;
 	}
 
 	for (i = 0; i < mc->soc->num_emem_regs; ++i)
 		mc_writel(mc, timing->emem_data[i], mc->soc->emem_regs[i]);
+
+	return 0;
 }
 
 unsigned int tegra_mc_get_emem_device_count(struct tegra_mc *mc)

drivers/memory/tegra/mc.h +5 -9

index 392993955c93..0720a1d2023e 100644
@@ -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)

Quoted 8 lines

drivers/memory/tegra/mc.h line 26
Quoted 5 lines
drivers/memory/tegra/mc.h +5 -0
+#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)
This /could/ be a separate patch, with it being unrelated to the EMC support, but probably not worth it.

I had the same feeling about this change and decided that it's not really worth it.

Quoted 28 lines

Quoted 19 lines

drivers/memory/tegra/mc.h +10 -0

+#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)
 {

drivers/memory/tegra/tegra30-emc.c new file +0 -0

new file mode 100644
index 000000000000..4700f7c8022e

drivers/memory/tegra/tegra30-emc.c line 6

Quoted 6 lines

drivers/memory/tegra/tegra30-emc.c new file +6 -0

@@ -0,0 +1,1197 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Tegra30 External Memory Controller driver
+ *
+ * Author: Dmitry Osipenko <[email protected]>
+ */

Quoted 1 line

Otherwise looks good to me.

Okay!

Dmitry Osipenko

author

2019-06-16 23:35 UTC #18 lore raw

PATCH v4 8/10 memory: tegra: Ensure timing control debug features are disabled

In reply to Dmitry Osipenko

Timing control debug features should be disabled at a boot time, but you never now and hence it's better to disable them explicitly because some of those features are crucial for the driver to do a proper thing.

Signed-off-by Dmitry Osipenko <[email protected]>

---
 drivers/memory/tegra/mc.c | 3 +++
 drivers/memory/tegra/mc.h | 2 ++
 2 files changed, 5 insertions(+)

drivers/memory/tegra/mc.c +3 -0

index eaebe371625c..4c1492c653e1 100644
@@ -660,6 +660,9 @@ static int tegra_mc_probe(struct platform_device *pdev)
 	} else
 #endif
 	{
+		/* ensure that debug features are disabled */
+		mc_writel(mc, 0x00000000, MC_TIMING_CONTROL_DBG);
+
 		err = tegra_mc_setup_latency_allowance(mc);
 		if (err < 0) {
 			dev_err(&pdev->dev,

drivers/memory/tegra/mc.h +2 -0

index 0720a1d2023e..abc565b42225 100644
@@ -33,6 +33,8 @@
 #define MC_EMEM_ARB_OVERRIDE				0xe8
 #define MC_EMEM_ARB_OVERRIDE_EACK_MASK			0x3
 
+#define MC_TIMING_CONTROL_DBG				0xf8
+
 #define MC_TIMING_CONTROL				0xfc
 #define MC_TIMING_UPDATE				BIT(0)

Signature · 1 line

2.22.0

Dmitry Osipenko

author

2019-06-16 23:35 UTC #19 lore raw

PATCH v4 9/10 memory: tegra: Consolidate registers definition into one place

In reply to Dmitry Osipenko

The Memory Controller registers definition is sparse and duplicated, let's consolidate everything into a common place for consistency.

Signed-off-by Dmitry Osipenko <[email protected]>

---
 drivers/memory/tegra/mc.c       | 30 -------------------
 drivers/memory/tegra/mc.h       | 52 +++++++++++++++++++++++++++++----
 drivers/memory/tegra/tegra124.c | 20 -------------
 drivers/memory/tegra/tegra30.c  | 20 -------------
 4 files changed, 47 insertions(+), 75 deletions(-)

drivers/memory/tegra/mc.c +0 ~~-30~~

index 4c1492c653e1..c926aa81a6ac 100644
@@ -21,36 +21,6 @@
 
 #include "mc.h"
 
-#define MC_INTSTATUS 0x000
-
-#define MC_INTMASK 0x004
-
-#define MC_ERR_STATUS 0x08
-#define  MC_ERR_STATUS_TYPE_SHIFT 28
-#define  MC_ERR_STATUS_TYPE_INVALID_SMMU_PAGE (6 << MC_ERR_STATUS_TYPE_SHIFT)
-#define  MC_ERR_STATUS_TYPE_MASK (0x7 << MC_ERR_STATUS_TYPE_SHIFT)
-#define  MC_ERR_STATUS_READABLE (1 << 27)
-#define  MC_ERR_STATUS_WRITABLE (1 << 26)
-#define  MC_ERR_STATUS_NONSECURE (1 << 25)
-#define  MC_ERR_STATUS_ADR_HI_SHIFT 20
-#define  MC_ERR_STATUS_ADR_HI_MASK 0x3
-#define  MC_ERR_STATUS_SECURITY (1 << 17)
-#define  MC_ERR_STATUS_RW (1 << 16)
-
-#define MC_ERR_ADR 0x0c
-
-#define MC_GART_ERROR_REQ		0x30
-#define MC_DECERR_EMEM_OTHERS_STATUS	0x58
-#define MC_SECURITY_VIOLATION_STATUS	0x74
-
-#define MC_EMEM_ARB_CFG 0x90
-#define  MC_EMEM_ARB_CFG_CYCLES_PER_UPDATE(x)	(((x) & 0x1ff) << 0)
-#define  MC_EMEM_ARB_CFG_CYCLES_PER_UPDATE_MASK	0x1ff
-#define MC_EMEM_ARB_MISC0 0xd8
-
-#define MC_EMEM_ADR_CFG 0x54
-#define MC_EMEM_ADR_CFG_EMEM_NUMDEV 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 },

drivers/memory/tegra/mc.h +47 -5

70 lines not on this page · raw message

drivers/memory/tegra/tegra124.c +0 ~~-20~~

index 8f8487bda642..c0acf988d7a5 100644
@@ -13,26 +13,6 @@
 
 #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 tegra124_mc_emem_regs[] = {
 	MC_EMEM_ARB_CFG,
 	MC_EMEM_ARB_OUTSTANDING_REQ,

drivers/memory/tegra/tegra30.c +0 ~~-20~~

index 67676677fd6a..f17488a0c5c7 100644
@@ -13,26 +13,6 @@
 
 #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,

Signature · 1 line

2.22.0

Dmitry Osipenko

author

2019-06-16 23:35 UTC #20 lore raw

PATCH v4 10/10 ARM: dts: tegra30: Add External Memory Controller node

In reply to Dmitry Osipenko

Add External Memory Controller node to the device-tree.

Signed-off-by Dmitry Osipenko <[email protected]>

---
 arch/arm/boot/dts/tegra30.dtsi | 11 +++++++++++
 1 file changed, 11 insertions(+)

arch/arm/boot/dts/tegra30.dtsi +11 -0

index e074258d4518..92c4aeafab29 100644
@@ -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>;

Signature · 1 line

2.22.0

Peter De Schrijver

Acked-by on the v4 series

2019-06-17 08:21 UTC #21 lore raw

In reply to Dmitry Osipenko

Quoted 7 lines · 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.

Acked-by Peter De Schrijver <[email protected]>

Dmitry Osipenko

author

2019-06-17 15:08 UTC #22 lore raw

17.06.2019 11:21, Peter De Schrijver пишет:

Quoted 11 lines · Dmitry Osipenko

Quoted 7 lines
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.
Acked-by Peter De Schrijver <[email protected]>

Thank you very much! I'll address comments from Thierry in v5 and probably add one more very minor change. I'll add yours ACK to v5 if there won't be any radical changes. Thanks again for helping with the review!