linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/8] Tegra124 EMC (external memory controller) support
@ 2014-07-11 14:18 Mikko Perttunen
  2014-07-11 14:18 ` [PATCH 1/8] clk: tegra124: Remove old emc_mux and emc clocks Mikko Perttunen
                   ` (8 more replies)
  0 siblings, 9 replies; 53+ messages in thread
From: Mikko Perttunen @ 2014-07-11 14:18 UTC (permalink / raw)
  To: linux-arm-kernel

Hi everyone,

this series adds support for the EMC (external memory controller) clock
in the Tegra124 system-on-chip. The series has been tested on Jetson TK1.

The first two patches remove the old "emc_mux" and "emc" clocks from the
clock tree and the device tree bindings. This is, of course, not backwards
compatible, but as these clocks have never been useful for anything
(apart from maybe reading the boot rate of the EMC clock). If this is still
not acceptable, the second patch can be dropped.

The second two patches add two clocks, PLL_M_UD and PLL_C_UD, that are
low jitter variants derived from PLL_M and PLL_C respectively. They are used
by some higher EMC clock rates as clock parents.

Patch 5 adds device tree binding documentation for the EMC clock node. It is
a whole separate block on the chip, so it does not go under CAR, as previous
clocks have done.

Patches 6 and 7 enable the EMC clock on Tegra124 and Jetson TK-1. The driver
is enabled always, but can only provide read-only access without EMC tables
that contain hardware characterization data for each operating point of the
clock.

Patch 8 adds the actual driver. The driver needs to write to both CAR and MC
registers in addition to EMC registers. These register accesses are either
shadowed or control a hardware state machine, so delegating them to other
drivers can be dangerous.

Patch 1 must go before patch 2 and patch 4 needs to go after patch 3
but otherwise there shouldn't be any compile-time dependencies. 
Patch 7 has a DTC dependency on patch 3.

Also available from git in
  git://github.com/cyndis/linux.git, branch emc-v1

Mikko Perttunen (8):
  clk: tegra124: Remove old emc_mux and emc clocks
  ARM: tegra: Remove TEGRA124_CLK_EMC from tegra124-car.h
  ARM: tegra: Add PLL_M_UD and PLL_C_UD to tegra124-car binding header
  clk: tegra124: Add PLL_M_UD and PLL_C_UD clocks
  of: Add Tegra124 EMC bindings
  ARM: tegra: Add EMC to Tegra124 device tree
  ARM: tegra: Add EMC timings to Jetson TK1 device tree
  clk: tegra: Add EMC clock driver

 .../bindings/memory-controllers/tegra-emc.txt      |   42 +
 arch/arm/boot/dts/tegra124-jetson-tk1-emc.dtsi     | 2323 ++++++++++++++++++++
 arch/arm/boot/dts/tegra124-jetson-tk1.dts          |    2 +
 arch/arm/boot/dts/tegra124.dtsi                    |    6 +
 drivers/clk/tegra/Makefile                         |    2 +-
 drivers/clk/tegra/clk-emc.c                        | 1508 +++++++++++++
 drivers/clk/tegra/clk-tegra124.c                   |   21 +-
 include/dt-bindings/clock/tegra124-car.h           |    8 +-
 8 files changed, 3896 insertions(+), 16 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/memory-controllers/tegra-emc.txt
 create mode 100644 arch/arm/boot/dts/tegra124-jetson-tk1-emc.dtsi
 create mode 100644 drivers/clk/tegra/clk-emc.c

-- 
1.8.1.5

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH 1/8] clk: tegra124: Remove old emc_mux and emc clocks
  2014-07-11 14:18 [PATCH 0/8] Tegra124 EMC (external memory controller) support Mikko Perttunen
@ 2014-07-11 14:18 ` Mikko Perttunen
  2014-07-11 14:18 ` [PATCH 2/8] ARM: tegra: Remove TEGRA124_CLK_EMC from tegra124-car.h Mikko Perttunen
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 53+ messages in thread
From: Mikko Perttunen @ 2014-07-11 14:18 UTC (permalink / raw)
  To: linux-arm-kernel

These clocks have never been able to do anything.

Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
---
 drivers/clk/tegra/clk-tegra124.c | 13 -------------
 1 file changed, 13 deletions(-)

diff --git a/drivers/clk/tegra/clk-tegra124.c b/drivers/clk/tegra/clk-tegra124.c
index 80efe51..99cc5ea 100644
--- a/drivers/clk/tegra/clk-tegra124.c
+++ b/drivers/clk/tegra/clk-tegra124.c
@@ -29,7 +29,6 @@
 #include "clk-id.h"
 
 #define CLK_SOURCE_CSITE 0x1d4
-#define CLK_SOURCE_EMC 0x19c
 
 #define PLLC_BASE 0x80
 #define PLLC_OUT 0x84
@@ -149,11 +148,6 @@ static const char *mux_plld_out0_plld2_out0[] = {
 };
 #define mux_plld_out0_plld2_out0_idx NULL
 
-static const char *mux_pllmcp_clkm[] = {
-	"pll_m", "pll_c", "pll_p", "clk_m", "pll_m_ud", "pll_c2", "pll_c3",
-};
-#define mux_pllmcp_clkm_idx NULL
-
 static struct div_nmp pllxc_nmp = {
 	.divm_shift = 0,
 	.divm_width = 8,
@@ -789,7 +783,6 @@ static struct tegra_clk tegra124_clks[tegra_clk_max] __initdata = {
 	[tegra_clk_i2c2] = { .dt_id = TEGRA124_CLK_I2C2, .present = true },
 	[tegra_clk_uartc] = { .dt_id = TEGRA124_CLK_UARTC, .present = true },
 	[tegra_clk_mipi_cal] = { .dt_id = TEGRA124_CLK_MIPI_CAL, .present = true },
-	[tegra_clk_emc] = { .dt_id = TEGRA124_CLK_EMC, .present = true },
 	[tegra_clk_usb2] = { .dt_id = TEGRA124_CLK_USB2, .present = true },
 	[tegra_clk_usb3] = { .dt_id = TEGRA124_CLK_USB3, .present = true },
 	[tegra_clk_vde_8] = { .dt_id = TEGRA124_CLK_VDE, .present = true },
@@ -1123,12 +1116,6 @@ static __init void tegra124_periph_clk_init(void __iomem *clk_base,
 			       clk_base + PLLD2_BASE, 25, 1, 0, &pll_d2_lock);
 	clks[TEGRA124_CLK_DSIB_MUX] = clk;
 
-	/* emc mux */
-	clk = clk_register_mux(NULL, "emc_mux", mux_pllmcp_clkm,
-			       ARRAY_SIZE(mux_pllmcp_clkm), 0,
-			       clk_base + CLK_SOURCE_EMC,
-			       29, 3, 0, NULL);
-
 	/* cml0 */
 	clk = clk_register_gate(NULL, "cml0", "pll_e", 0, clk_base + PLLE_AUX,
 				0, 0, &pll_e_lock);
-- 
1.8.1.5

^ permalink raw reply related	[flat|nested] 53+ messages in thread

* [PATCH 2/8] ARM: tegra: Remove TEGRA124_CLK_EMC from tegra124-car.h
  2014-07-11 14:18 [PATCH 0/8] Tegra124 EMC (external memory controller) support Mikko Perttunen
  2014-07-11 14:18 ` [PATCH 1/8] clk: tegra124: Remove old emc_mux and emc clocks Mikko Perttunen
@ 2014-07-11 14:18 ` Mikko Perttunen
  2014-07-21 22:37   ` Stephen Warren
  2014-07-11 14:18 ` [PATCH 3/8] ARM: tegra: Add PLL_M_UD and PLL_C_UD to tegra124-car binding header Mikko Perttunen
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 53+ messages in thread
From: Mikko Perttunen @ 2014-07-11 14:18 UTC (permalink / raw)
  To: linux-arm-kernel

Remove the TEGRA124_CLK_EMC cell value define for tegra124-car
from the binding headers. This clock has never been able to do
anything and is being replaced with a separate EMC driver with
its own device tree node. Removing the define ensures that any
user will not mistakenly refer to <&tegra_car TEGRA124_CLK_EMC>
instead of <&emc> or similar.

Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
---
 include/dt-bindings/clock/tegra124-car.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/dt-bindings/clock/tegra124-car.h b/include/dt-bindings/clock/tegra124-car.h
index 8a4c589..4faabce 100644
--- a/include/dt-bindings/clock/tegra124-car.h
+++ b/include/dt-bindings/clock/tegra124-car.h
@@ -73,7 +73,7 @@
 #define TEGRA124_CLK_I2C2 54
 #define TEGRA124_CLK_UARTC 55
 #define TEGRA124_CLK_MIPI_CAL 56
-#define TEGRA124_CLK_EMC 57
+/* 57 */
 #define TEGRA124_CLK_USB2 58
 #define TEGRA124_CLK_USB3 59
 /* 60 */
-- 
1.8.1.5

^ permalink raw reply related	[flat|nested] 53+ messages in thread

* [PATCH 3/8] ARM: tegra: Add PLL_M_UD and PLL_C_UD to tegra124-car binding header
  2014-07-11 14:18 [PATCH 0/8] Tegra124 EMC (external memory controller) support Mikko Perttunen
  2014-07-11 14:18 ` [PATCH 1/8] clk: tegra124: Remove old emc_mux and emc clocks Mikko Perttunen
  2014-07-11 14:18 ` [PATCH 2/8] ARM: tegra: Remove TEGRA124_CLK_EMC from tegra124-car.h Mikko Perttunen
@ 2014-07-11 14:18 ` Mikko Perttunen
  2014-08-25 17:41   ` Stephen Warren
  2014-07-11 14:18 ` [PATCH 4/8] clk: tegra124: Add PLL_M_UD and PLL_C_UD clocks Mikko Perttunen
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 53+ messages in thread
From: Mikko Perttunen @ 2014-07-11 14:18 UTC (permalink / raw)
  To: linux-arm-kernel

Add these clocks to the binding header so that EMC timings that have
them as parent can refer to the clocks.

Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
---
 include/dt-bindings/clock/tegra124-car.h | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/include/dt-bindings/clock/tegra124-car.h b/include/dt-bindings/clock/tegra124-car.h
index 4faabce..f58b24d 100644
--- a/include/dt-bindings/clock/tegra124-car.h
+++ b/include/dt-bindings/clock/tegra124-car.h
@@ -337,6 +337,10 @@
 #define TEGRA124_CLK_DSIB_MUX 310
 #define TEGRA124_CLK_SOR0_LVDS 311
 #define TEGRA124_CLK_XUSB_SS_DIV2 312
-#define TEGRA124_CLK_CLK_MAX 313
+
+#define TEGRA124_CLK_PLL_M_UD 313
+#define TEGRA124_CLK_PLL_C_UD 314
+
+#define TEGRA124_CLK_CLK_MAX 315
 
 #endif	/* _DT_BINDINGS_CLOCK_TEGRA124_CAR_H */
-- 
1.8.1.5

^ permalink raw reply related	[flat|nested] 53+ messages in thread

* [PATCH 4/8] clk: tegra124: Add PLL_M_UD and PLL_C_UD clocks
  2014-07-11 14:18 [PATCH 0/8] Tegra124 EMC (external memory controller) support Mikko Perttunen
                   ` (2 preceding siblings ...)
  2014-07-11 14:18 ` [PATCH 3/8] ARM: tegra: Add PLL_M_UD and PLL_C_UD to tegra124-car binding header Mikko Perttunen
@ 2014-07-11 14:18 ` Mikko Perttunen
  2014-07-11 14:18 ` [PATCH 5/8] of: Add Tegra124 EMC bindings Mikko Perttunen
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 53+ messages in thread
From: Mikko Perttunen @ 2014-07-11 14:18 UTC (permalink / raw)
  To: linux-arm-kernel

These clocks are used as parents for some EMC timings.

Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
---
 drivers/clk/tegra/clk-tegra124.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/clk/tegra/clk-tegra124.c b/drivers/clk/tegra/clk-tegra124.c
index 99cc5ea..a4f8150 100644
--- a/drivers/clk/tegra/clk-tegra124.c
+++ b/drivers/clk/tegra/clk-tegra124.c
@@ -1153,6 +1153,12 @@ static void __init tegra124_pll_init(void __iomem *clk_base,
 	clk_register_clkdev(clk, "pll_c_out1", NULL);
 	clks[TEGRA124_CLK_PLL_C_OUT1] = clk;
 
+	/* PLLC_UD */
+	clk = clk_register_fixed_factor(NULL, "pll_c_ud", "pll_c",
+					CLK_SET_RATE_PARENT, 1, 1);
+	clk_register_clkdev(clk, "pll_c_ud", NULL);
+	clks[TEGRA124_CLK_PLL_C_UD] = clk;
+
 	/* PLLC2 */
 	clk = tegra_clk_register_pllc("pll_c2", "pll_ref", clk_base, pmc, 0,
 			     &pll_c2_params, NULL);
@@ -1185,6 +1191,8 @@ static void __init tegra124_pll_init(void __iomem *clk_base,
 	/* PLLM_UD */
 	clk = clk_register_fixed_factor(NULL, "pll_m_ud", "pll_m",
 					CLK_SET_RATE_PARENT, 1, 1);
+	clk_register_clkdev(clk, "pll_m_ud", NULL);
+	clks[TEGRA124_CLK_PLL_M_UD] = clk;
 
 	/* PLLU */
 	val = readl(clk_base + pll_u_params.base_reg);
-- 
1.8.1.5

^ permalink raw reply related	[flat|nested] 53+ messages in thread

* [PATCH 5/8] of: Add Tegra124 EMC bindings
  2014-07-11 14:18 [PATCH 0/8] Tegra124 EMC (external memory controller) support Mikko Perttunen
                   ` (3 preceding siblings ...)
  2014-07-11 14:18 ` [PATCH 4/8] clk: tegra124: Add PLL_M_UD and PLL_C_UD clocks Mikko Perttunen
@ 2014-07-11 14:18 ` Mikko Perttunen
  2014-07-11 14:51   ` Thierry Reding
                     ` (2 more replies)
  2014-07-11 14:18 ` [PATCH 6/8] ARM: tegra: Add EMC to Tegra124 device tree Mikko Perttunen
                   ` (3 subsequent siblings)
  8 siblings, 3 replies; 53+ messages in thread
From: Mikko Perttunen @ 2014-07-11 14:18 UTC (permalink / raw)
  To: linux-arm-kernel

Add binding documentation for the nvidia,tegra124-emc device tree
node.

Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
---
 .../bindings/memory-controllers/tegra-emc.txt      | 42 ++++++++++++++++++++++
 1 file changed, 42 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/memory-controllers/tegra-emc.txt

diff --git a/Documentation/devicetree/bindings/memory-controllers/tegra-emc.txt b/Documentation/devicetree/bindings/memory-controllers/tegra-emc.txt
new file mode 100644
index 0000000..2dde17e
--- /dev/null
+++ b/Documentation/devicetree/bindings/memory-controllers/tegra-emc.txt
@@ -0,0 +1,42 @@
+Tegra124 SoC EMC controller
+
+Required properties :
+- compatible : "nvidia,tegra124-emc".
+- reg : Should contain 1 or 2 entries:
+  - EMC register set
+  - MC register set : Required only if no node with
+    'compatible = "nvidia,tegra124-mc"' exists. The MC register set
+    is first read from the MC node. If it doesn't exist, it is read
+    from this property.
+- timings : Should contain 1 entry for each supported clock rate.
+  Entries should be named "timing at n" where n is a 0-based increasing
+  number. The timings must be listed in rate-ascending order.
+
+Required properties for timings :
+- clock-frequency : Should contain the memory clock rate.
+- nvidia,parent-clock-frequency : Should contain the rate of the EMC
+  clock's parent clock.
+- clocks : Must contain an entry for each entry in clock-names.
+  See ../clocks/clock-bindings.txt for details.
+- clock-names : Must include the following entries:
+  - emc-parent : EMC's parent clock.
+- The following properties contain EMC timing characterization values:
+  - nvidia,emc-zcal-cnt-long
+  - nvidia,emc-auto-cal-interval
+  - nvidia,emc-ctt-term-ctrl
+  - nvidia,emc-cfg
+  - nvidia,emc-cfg-2
+  - nvidia,emc-sel-dpd-ctrl
+  - nvidia,emc-cfg-dig-dll
+  - nvidia,emc-bgbias-ctl0
+  - nvidia,emc-auto-cal-config
+  - nvidia,emc-auto-cal-config2
+  - nvidia,emc-auto-cal-config3
+  - nvidia,emc-mode-reset
+  - nvidia,emc-mode-1
+  - nvidia,emc-mode-2
+  - nvidia,emc-mode-4
+- nvidia,emc-burst-data : EMC timing characterization data written to
+                          EMC registers.
+- nvidia,mc-burst-data : EMC timing characterization data written to
+                         MC registers.
-- 
1.8.1.5

^ permalink raw reply related	[flat|nested] 53+ messages in thread

* [PATCH 6/8] ARM: tegra: Add EMC to Tegra124 device tree
  2014-07-11 14:18 [PATCH 0/8] Tegra124 EMC (external memory controller) support Mikko Perttunen
                   ` (4 preceding siblings ...)
  2014-07-11 14:18 ` [PATCH 5/8] of: Add Tegra124 EMC bindings Mikko Perttunen
@ 2014-07-11 14:18 ` Mikko Perttunen
  2014-07-11 14:18 ` [PATCH 7/8] ARM: tegra: Add EMC timings to Jetson TK1 " Mikko Perttunen
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 53+ messages in thread
From: Mikko Perttunen @ 2014-07-11 14:18 UTC (permalink / raw)
  To: linux-arm-kernel

This adds a node for the EMC memory controller. It is always enabled,
but only provides read-only functionality without board-specific
timing tables.

Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
---
 arch/arm/boot/dts/tegra124.dtsi | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/tegra124.dtsi b/arch/arm/boot/dts/tegra124.dtsi
index 7cc535b..78d629a6 100644
--- a/arch/arm/boot/dts/tegra124.dtsi
+++ b/arch/arm/boot/dts/tegra124.dtsi
@@ -136,6 +136,12 @@
 		#reset-cells = <1>;
 	};
 
+	emc at 0,7001b000 {
+		compatible = "nvidia,tegra124-emc";
+		reg = <0x0 0x7001b000 0x0 0x1000>,
+		      <0x0 0x70019000 0x0 0x1000>;
+	};
+
 	gpio: gpio at 0,6000d000 {
 		compatible = "nvidia,tegra124-gpio", "nvidia,tegra30-gpio";
 		reg = <0x0 0x6000d000 0x0 0x1000>;
-- 
1.8.1.5

^ permalink raw reply related	[flat|nested] 53+ messages in thread

* [PATCH 7/8] ARM: tegra: Add EMC timings to Jetson TK1 device tree
  2014-07-11 14:18 [PATCH 0/8] Tegra124 EMC (external memory controller) support Mikko Perttunen
                   ` (5 preceding siblings ...)
  2014-07-11 14:18 ` [PATCH 6/8] ARM: tegra: Add EMC to Tegra124 device tree Mikko Perttunen
@ 2014-07-11 14:18 ` Mikko Perttunen
  2014-07-11 14:18 ` [PATCH 8/8] clk: tegra: Add EMC clock driver Mikko Perttunen
  2014-08-25 17:40 ` [PATCH 0/8] Tegra124 EMC (external memory controller) support Stephen Warren
  8 siblings, 0 replies; 53+ messages in thread
From: Mikko Perttunen @ 2014-07-11 14:18 UTC (permalink / raw)
  To: linux-arm-kernel

This adds a new file, tegra124-jetson-tk1-emc.dtsi that contains
valid timings for the EMC memory clock. The file is included to the
main Jetson TK1 device tree.

Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
---
 arch/arm/boot/dts/tegra124-jetson-tk1-emc.dtsi | 2323 ++++++++++++++++++++++++
 arch/arm/boot/dts/tegra124-jetson-tk1.dts      |    2 +
 2 files changed, 2325 insertions(+)
 create mode 100644 arch/arm/boot/dts/tegra124-jetson-tk1-emc.dtsi

diff --git a/arch/arm/boot/dts/tegra124-jetson-tk1-emc.dtsi b/arch/arm/boot/dts/tegra124-jetson-tk1-emc.dtsi
new file mode 100644
index 0000000..c8f003e
--- /dev/null
+++ b/arch/arm/boot/dts/tegra124-jetson-tk1-emc.dtsi
@@ -0,0 +1,2323 @@
+/ {
+	emc at 0,7001b000 {
+		timings {
+			timing at 0 {
+				clock-frequency = <12750000>;
+				nvidia,parent-clock-frequency = <408000000>;
+				clocks = <&tegra_car TEGRA124_CLK_PLL_P>;
+				clock-names = "emc-parent";
+
+				nvidia,emc-zcal-cnt-long = <0x00000042>;
+				nvidia,emc-auto-cal-interval = <0x001fffff>;
+				nvidia,emc-ctt-term-ctrl = <0x00000802>;
+				nvidia,emc-cfg = <0x73240000>;
+				nvidia,emc-cfg-2 = <0x000008c5>;
+				nvidia,emc-sel-dpd-ctrl = <0x00040128>;
+				nvidia,emc-cfg-dig-dll = <0x002c0068>;
+				nvidia,emc-bgbias-ctl0 = <0x00000008>;
+				nvidia,emc-auto-cal-config = <0xa1430000>;
+				nvidia,emc-auto-cal-config2 = <0x00000000>;
+				nvidia,emc-auto-cal-config3 = <0x00000000>;
+				nvidia,emc-mode-reset = <0x80001221>;
+				nvidia,emc-mode-1 = <0x80100003>;
+				nvidia,emc-mode-2 = <0x80200008>;
+				nvidia,emc-mode-4 = <0x00000000>;
+
+				nvidia,emc-burst-data = <
+					0x00000000 /* EMC_RC */
+					0x00000003 /* EMC_RFC */
+					0x00000000 /* EMC_RFC_SLR */
+					0x00000000 /* EMC_RAS */
+					0x00000000 /* EMC_RP */
+					0x00000004 /* EMC_R2W */
+					0x0000000a /* EMC_W2R */
+					0x00000003 /* EMC_R2P */
+					0x0000000b /* EMC_W2P */
+					0x00000000 /* EMC_RD_RCD */
+					0x00000000 /* EMC_WR_RCD */
+					0x00000003 /* EMC_RRD */
+					0x00000003 /* EMC_REXT */
+					0x00000000 /* EMC_WEXT */
+					0x00000006 /* EMC_WDV */
+					0x00000006 /* EMC_WDV_MASK */
+					0x00000006 /* EMC_QUSE */
+					0x00000002 /* EMC_QUSE_WIDTH */
+					0x00000000 /* EMC_IBDLY */
+					0x00000005 /* EMC_EINPUT */
+					0x00000005 /* EMC_EINPUT_DURATION */
+					0x00010000 /* EMC_PUTERM_EXTRA */
+					0x00000003 /* EMC_PUTERM_WIDTH */
+					0x00000000 /* EMC_PUTERM_ADJ */
+					0x00000000 /* EMC_CDB_CNTL_1 */
+					0x00000000 /* EMC_CDB_CNTL_2 */
+					0x00000000 /* EMC_CDB_CNTL_3 */
+					0x00000004 /* EMC_QRST */
+					0x0000000c /* EMC_QSAFE */
+					0x0000000d /* EMC_RDV */
+					0x0000000f /* EMC_RDV_MASK */
+					0x00000060 /* EMC_REFRESH */
+					0x00000000 /* EMC_BURST_REFRESH_NUM */
+					0x00000018 /* EMC_PRE_REFRESH_REQ_CNT */
+					0x00000002 /* EMC_PDEX2WR */
+					0x00000002 /* EMC_PDEX2RD */
+					0x00000001 /* EMC_PCHG2PDEN */
+					0x00000000 /* EMC_ACT2PDEN */
+					0x00000007 /* EMC_AR2PDEN */
+					0x0000000f /* EMC_RW2PDEN */
+					0x00000005 /* EMC_TXSR */
+					0x00000005 /* EMC_TXSRDLL */
+					0x00000004 /* EMC_TCKE */
+					0x00000005 /* EMC_TCKESR */
+					0x00000004 /* EMC_TPD */
+					0x00000000 /* EMC_TFAW */
+					0x00000000 /* EMC_TRPAB */
+					0x00000005 /* EMC_TCLKSTABLE */
+					0x00000005 /* EMC_TCLKSTOP */
+					0x00000064 /* EMC_TREFBW */
+					0x00000000 /* EMC_FBIO_CFG6 */
+					0x00000000 /* EMC_ODT_WRITE */
+					0x00000000 /* EMC_ODT_READ */
+					0x106aa298 /* EMC_FBIO_CFG5 */
+					0x002c00a0 /* EMC_CFG_DIG_DLL */
+					0x00008000 /* EMC_CFG_DIG_DLL_PERIOD */
+					0x00064000 /* EMC_DLL_XFORM_DQS0 */
+					0x00064000 /* EMC_DLL_XFORM_DQS1 */
+					0x00064000 /* EMC_DLL_XFORM_DQS2 */
+					0x00064000 /* EMC_DLL_XFORM_DQS3 */
+					0x00064000 /* EMC_DLL_XFORM_DQS4 */
+					0x00064000 /* EMC_DLL_XFORM_DQS5 */
+					0x00064000 /* EMC_DLL_XFORM_DQS6 */
+					0x00064000 /* EMC_DLL_XFORM_DQS7 */
+					0x00064000 /* EMC_DLL_XFORM_DQS8 */
+					0x00064000 /* EMC_DLL_XFORM_DQS9 */
+					0x00064000 /* EMC_DLL_XFORM_DQS10 */
+					0x00064000 /* EMC_DLL_XFORM_DQS11 */
+					0x00064000 /* EMC_DLL_XFORM_DQS12 */
+					0x00064000 /* EMC_DLL_XFORM_DQS13 */
+					0x00064000 /* EMC_DLL_XFORM_DQS14 */
+					0x00064000 /* EMC_DLL_XFORM_DQS15 */
+					0x00000000 /* EMC_DLL_XFORM_QUSE0 */
+					0x00000000 /* EMC_DLL_XFORM_QUSE1 */
+					0x00000000 /* EMC_DLL_XFORM_QUSE2 */
+					0x00000000 /* 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_DLL_XFORM_ADDR0 */
+					0x00000000 /* EMC_DLL_XFORM_ADDR1 */
+					0x00000000 /* EMC_DLL_XFORM_ADDR2 */
+					0x00000000 /* EMC_DLL_XFORM_ADDR3 */
+					0x00000000 /* EMC_DLL_XFORM_ADDR4 */
+					0x00000000 /* EMC_DLL_XFORM_ADDR5 */
+					0x00000000 /* EMC_DLL_XFORM_QUSE8 */
+					0x00000000 /* EMC_DLL_XFORM_QUSE9 */
+					0x00000000 /* EMC_DLL_XFORM_QUSE10 */
+					0x00000000 /* EMC_DLL_XFORM_QUSE11 */
+					0x00000000 /* EMC_DLL_XFORM_QUSE12 */
+					0x00000000 /* EMC_DLL_XFORM_QUSE13 */
+					0x00000000 /* EMC_DLL_XFORM_QUSE14 */
+					0x00000000 /* EMC_DLL_XFORM_QUSE15 */
+					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 */
+					0x00000000 /* EMC_DLI_TRIM_TXDQS8 */
+					0x00000000 /* EMC_DLI_TRIM_TXDQS9 */
+					0x00000000 /* EMC_DLI_TRIM_TXDQS10 */
+					0x00000000 /* EMC_DLI_TRIM_TXDQS11 */
+					0x00000000 /* EMC_DLI_TRIM_TXDQS12 */
+					0x00000000 /* EMC_DLI_TRIM_TXDQS13 */
+					0x00000000 /* EMC_DLI_TRIM_TXDQS14 */
+					0x00000000 /* EMC_DLI_TRIM_TXDQS15 */
+					0x000fc000 /* EMC_DLL_XFORM_DQ0 */
+					0x000fc000 /* EMC_DLL_XFORM_DQ1 */
+					0x000fc000 /* EMC_DLL_XFORM_DQ2 */
+					0x000fc000 /* EMC_DLL_XFORM_DQ3 */
+					0x0000fc00 /* EMC_DLL_XFORM_DQ4 */
+					0x0000fc00 /* EMC_DLL_XFORM_DQ5 */
+					0x0000fc00 /* EMC_DLL_XFORM_DQ6 */
+					0x0000fc00 /* EMC_DLL_XFORM_DQ7 */
+					0x10000280 /* EMC_XM2CMDPADCTRL */
+					0x00000000 /* EMC_XM2CMDPADCTRL4 */
+					0x00111111 /* EMC_XM2CMDPADCTRL5 */
+					0x0130b118 /* EMC_XM2DQSPADCTRL2 */
+					0x00000000 /* EMC_XM2DQPADCTRL2 */
+					0x00000000 /* EMC_XM2DQPADCTRL3 */
+					0x77ffc081 /* EMC_XM2CLKPADCTRL */
+					0x00000e0e /* EMC_XM2CLKPADCTRL2 */
+					0x81f1f108 /* EMC_XM2COMPPADCTRL */
+					0x07070004 /* EMC_XM2VTTGENPADCTRL */
+					0x0000003f /* EMC_XM2VTTGENPADCTRL2 */
+					0x016eeeee /* EMC_XM2VTTGENPADCTRL3 */
+					0x51451400 /* EMC_XM2DQSPADCTRL3 */
+					0x00514514 /* EMC_XM2DQSPADCTRL4 */
+					0x00514514 /* EMC_XM2DQSPADCTRL5 */
+					0x51451400 /* EMC_XM2DQSPADCTRL6 */
+					0x0000003f /* EMC_DSR_VTTGEN_DRV */
+					0x00000007 /* EMC_TXDSRVTTGEN */
+					0x00000000 /* EMC_FBIO_SPARE */
+					0x00000000 /* EMC_ZCAL_INTERVAL */
+					0x00000042 /* EMC_ZCAL_WAIT_CNT */
+					0x000e000e /* EMC_MRS_WAIT_CNT */
+					0x000e000e /* EMC_MRS_WAIT_CNT2 */
+					0x00000000 /* EMC_CTT */
+					0x00000003 /* EMC_CTT_DURATION */
+					0x0000f2f3 /* EMC_CFG_PIPE */
+					0x800001c5 /* EMC_DYN_SELF_REF_CONTROL */
+					0x0000000a /* EMC_QPOP */
+				>;
+
+				nvidia,mc-burst-data = <
+					0x40040001 /* MC_EMEM_ARB_CFG */
+					0x8000000a /* MC_EMEM_ARB_OUTSTANDING_REQ */
+					0x00000001 /* MC_EMEM_ARB_TIMING_RCD */
+					0x00000001 /* MC_EMEM_ARB_TIMING_RP */
+					0x00000002 /* MC_EMEM_ARB_TIMING_RC */
+					0x00000000 /* MC_EMEM_ARB_TIMING_RAS */
+					0x00000002 /* MC_EMEM_ARB_TIMING_FAW */
+					0x00000001 /* MC_EMEM_ARB_TIMING_RRD */
+					0x00000002 /* MC_EMEM_ARB_TIMING_RAP2PRE */
+					0x00000008 /* MC_EMEM_ARB_TIMING_WAP2PRE */
+					0x00000003 /* MC_EMEM_ARB_TIMING_R2R */
+					0x00000002 /* MC_EMEM_ARB_TIMING_W2W */
+					0x00000003 /* MC_EMEM_ARB_TIMING_R2W */
+					0x00000006 /* MC_EMEM_ARB_TIMING_W2R */
+					0x06030203 /* MC_EMEM_ARB_DA_TURNS */
+					0x000a0402 /* MC_EMEM_ARB_DA_COVERS */
+					0x77e30303 /* MC_EMEM_ARB_MISC0 */
+					0x70000f03 /* MC_EMEM_ARB_MISC1 */
+					0x001f0000 /* MC_EMEM_ARB_RING1_THROTTLE */
+				>;
+			};
+
+			timing at 1 {
+				clock-frequency = <20400000>;
+				nvidia,parent-clock-frequency = <408000000>;
+				clocks = <&tegra_car TEGRA124_CLK_PLL_P>;
+				clock-names = "emc-parent";
+
+				nvidia,emc-zcal-cnt-long = <0x00000042>;
+				nvidia,emc-auto-cal-interval = <0x001fffff>;
+				nvidia,emc-ctt-term-ctrl = <0x00000802>;
+				nvidia,emc-cfg = <0x73240000>;
+				nvidia,emc-cfg-2 = <0x000008c5>;
+				nvidia,emc-sel-dpd-ctrl = <0x00040128>;
+				nvidia,emc-cfg-dig-dll = <0x002c0068>;
+				nvidia,emc-bgbias-ctl0 = <0x00000008>;
+				nvidia,emc-auto-cal-config = <0x00000000>;
+				nvidia,emc-auto-cal-config2 = <0x00000000>;
+				nvidia,emc-auto-cal-config3 = <0xa1430000>;
+				nvidia,emc-mode-reset = <0x80001221>;
+				nvidia,emc-mode-1 = <0x80100003>;
+				nvidia,emc-mode-2 = <0x80200008>;
+				nvidia,emc-mode-4 = <0x00000000>;
+
+				nvidia,emc-burst-data = <
+					0x00000000
+					0x00000005
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000004
+					0x0000000a
+					0x00000003
+					0x0000000b
+					0x00000000
+					0x00000000
+					0x00000003
+					0x00000003
+					0x00000000
+					0x00000006
+					0x00000006
+					0x00000006
+					0x00000002
+					0x00000000
+					0x00000005
+					0x00000005
+					0x00010000
+					0x00000003
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000004
+					0x0000000c
+					0x0000000d
+					0x0000000f
+					0x0000009a
+					0x00000000
+					0x00000026
+					0x00000002
+					0x00000002
+					0x00000001
+					0x00000000
+					0x00000007
+					0x0000000f
+					0x00000006
+					0x00000006
+					0x00000004
+					0x00000005
+					0x00000004
+					0x00000000
+					0x00000000
+					0x00000005
+					0x00000005
+					0x000000a0
+					0x00000000
+					0x00000000
+					0x00000000
+					0x106aa298
+					0x002c00a0
+					0x00008000
+					0x00064000
+					0x00064000
+					0x00064000
+					0x00064000
+					0x00064000
+					0x00064000
+					0x00064000
+					0x00064000
+					0x00064000
+					0x00064000
+					0x00064000
+					0x00064000
+					0x00064000
+					0x00064000
+					0x00064000
+					0x00064000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x000fc000
+					0x000fc000
+					0x000fc000
+					0x000fc000
+					0x0000fc00
+					0x0000fc00
+					0x0000fc00
+					0x0000fc00
+					0x10000280
+					0x00000000
+					0x00111111
+					0x0130b118
+					0x00000000
+					0x00000000
+					0x77ffc081
+					0x00000e0e
+					0x81f1f108
+					0x07070004
+					0x0000003f
+					0x016eeeee
+					0x51451400
+					0x00514514
+					0x00514514
+					0x51451400
+					0x0000003f
+					0x0000000b
+					0x00000000
+					0x00000000
+					0x00000042
+					0x000e000e
+					0x000e000e
+					0x00000000
+					0x00000003
+					0x0000f2f3
+					0x8000023a
+					0x0000000a
+				>;
+
+				nvidia,mc-burst-data = <
+					0x40020001
+					0x80000012
+					0x00000001
+					0x00000001
+					0x00000002
+					0x00000000
+					0x00000002
+					0x00000001
+					0x00000002
+					0x00000008
+					0x00000003
+					0x00000002
+					0x00000003
+					0x00000006
+					0x06030203
+					0x000a0402
+					0x76230303
+					0x70000f03
+					0x001f0000
+				>;
+			};
+			timing at 2 {
+				clock-frequency = <40800000>;
+				nvidia,parent-clock-frequency = <408000000>;
+				clocks = <&tegra_car TEGRA124_CLK_PLL_P>;
+				clock-names = "emc-parent";
+
+				nvidia,emc-zcal-cnt-long = <0x00000042>;
+				nvidia,emc-auto-cal-interval = <0x001fffff>;
+				nvidia,emc-ctt-term-ctrl = <0x00000802>;
+				nvidia,emc-cfg = <0x73240000>;
+				nvidia,emc-cfg-2 = <0x000008c5>;
+				nvidia,emc-sel-dpd-ctrl = <0x00040128>;
+				nvidia,emc-cfg-dig-dll = <0x002c0068>;
+				nvidia,emc-bgbias-ctl0 = <0x00000008>;
+				nvidia,emc-auto-cal-config = <0x00000000>;
+				nvidia,emc-auto-cal-config2 = <0x00000000>;
+				nvidia,emc-auto-cal-config3 = <0xa1430000>;
+				nvidia,emc-mode-reset = <0x80001221>;
+				nvidia,emc-mode-1 = <0x80100003>;
+				nvidia,emc-mode-2 = <0x80200008>;
+				nvidia,emc-mode-4 = <0x00000000>;
+
+				nvidia,emc-burst-data = <
+					0x00000001
+					0x0000000a
+					0x00000000
+					0x00000001
+					0x00000000
+					0x00000004
+					0x0000000a
+					0x00000003
+					0x0000000b
+					0x00000000
+					0x00000000
+					0x00000003
+					0x00000003
+					0x00000000
+					0x00000006
+					0x00000006
+					0x00000006
+					0x00000002
+					0x00000000
+					0x00000005
+					0x00000005
+					0x00010000
+					0x00000003
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000004
+					0x0000000c
+					0x0000000d
+					0x0000000f
+					0x00000134
+					0x00000000
+					0x0000004d
+					0x00000002
+					0x00000002
+					0x00000001
+					0x00000000
+					0x00000008
+					0x0000000f
+					0x0000000c
+					0x0000000c
+					0x00000004
+					0x00000005
+					0x00000004
+					0x00000000
+					0x00000000
+					0x00000005
+					0x00000005
+					0x0000013f
+					0x00000000
+					0x00000000
+					0x00000000
+					0x106aa298
+					0x002c00a0
+					0x00008000
+					0x00064000
+					0x00064000
+					0x00064000
+					0x00064000
+					0x00064000
+					0x00064000
+					0x00064000
+					0x00064000
+					0x00064000
+					0x00064000
+					0x00064000
+					0x00064000
+					0x00064000
+					0x00064000
+					0x00064000
+					0x00064000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x000fc000
+					0x000fc000
+					0x000fc000
+					0x000fc000
+					0x0000fc00
+					0x0000fc00
+					0x0000fc00
+					0x0000fc00
+					0x10000280
+					0x00000000
+					0x00111111
+					0x0130b118
+					0x00000000
+					0x00000000
+					0x77ffc081
+					0x00000e0e
+					0x81f1f108
+					0x07070004
+					0x0000003f
+					0x016eeeee
+					0x51451400
+					0x00514514
+					0x00514514
+					0x51451400
+					0x0000003f
+					0x00000015
+					0x00000000
+					0x00000000
+					0x00000042
+					0x000e000e
+					0x000e000e
+					0x00000000
+					0x00000003
+					0x0000f2f3
+					0x80000370
+					0x0000000a
+				>;
+
+				nvidia,mc-burst-data = <
+					0xa0000001
+					0x80000017
+					0x00000001
+					0x00000001
+					0x00000002
+					0x00000000
+					0x00000002
+					0x00000001
+					0x00000002
+					0x00000008
+					0x00000003
+					0x00000002
+					0x00000003
+					0x00000006
+					0x06030203
+					0x000a0402
+					0x74a30303
+					0x70000f03
+					0x001f0000
+				>;
+			};
+			timing at 3 {
+				clock-frequency = <68000000>;
+				nvidia,parent-clock-frequency = <408000000>;
+				clocks = <&tegra_car TEGRA124_CLK_PLL_P>;
+				clock-names = "emc-parent";
+
+				nvidia,emc-zcal-cnt-long = <0x00000042>;
+				nvidia,emc-auto-cal-interval = <0x001fffff>;
+				nvidia,emc-ctt-term-ctrl = <0x00000802>;
+				nvidia,emc-cfg = <0x73240000>;
+				nvidia,emc-cfg-2 = <0x000008c5>;
+				nvidia,emc-sel-dpd-ctrl = <0x00040128>;
+				nvidia,emc-cfg-dig-dll = <0x002c0068>;
+				nvidia,emc-bgbias-ctl0 = <0x00000008>;
+				nvidia,emc-auto-cal-config = <0x00000000>;
+				nvidia,emc-auto-cal-config2 = <0x00000000>;
+				nvidia,emc-auto-cal-config3 = <0xa1430000>;
+				nvidia,emc-mode-reset = <0x80001221>;
+				nvidia,emc-mode-1 = <0x80100003>;
+				nvidia,emc-mode-2 = <0x80200008>;
+				nvidia,emc-mode-4 = <0x00000000>;
+
+				nvidia,emc-burst-data = <
+					0x00000003
+					0x00000011
+					0x00000000
+					0x00000002
+					0x00000000
+					0x00000004
+					0x0000000a
+					0x00000003
+					0x0000000b
+					0x00000000
+					0x00000000
+					0x00000003
+					0x00000003
+					0x00000000
+					0x00000006
+					0x00000006
+					0x00000006
+					0x00000002
+					0x00000000
+					0x00000005
+					0x00000005
+					0x00010000
+					0x00000003
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000004
+					0x0000000c
+					0x0000000d
+					0x0000000f
+					0x00000202
+					0x00000000
+					0x00000080
+					0x00000002
+					0x00000002
+					0x00000001
+					0x00000000
+					0x0000000f
+					0x0000000f
+					0x00000013
+					0x00000013
+					0x00000004
+					0x00000005
+					0x00000004
+					0x00000001
+					0x00000000
+					0x00000005
+					0x00000005
+					0x00000213
+					0x00000000
+					0x00000000
+					0x00000000
+					0x106aa298
+					0x002c00a0
+					0x00008000
+					0x00064000
+					0x00064000
+					0x00064000
+					0x00064000
+					0x00064000
+					0x00064000
+					0x00064000
+					0x00064000
+					0x00064000
+					0x00064000
+					0x00064000
+					0x00064000
+					0x00064000
+					0x00064000
+					0x00064000
+					0x00064000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x000fc000
+					0x000fc000
+					0x000fc000
+					0x000fc000
+					0x0000fc00
+					0x0000fc00
+					0x0000fc00
+					0x0000fc00
+					0x10000280
+					0x00000000
+					0x00111111
+					0x0130b118
+					0x00000000
+					0x00000000
+					0x77ffc081
+					0x00000e0e
+					0x81f1f108
+					0x07070004
+					0x0000003f
+					0x016eeeee
+					0x51451400
+					0x00514514
+					0x00514514
+					0x51451400
+					0x0000003f
+					0x00000022
+					0x00000000
+					0x00000000
+					0x00000042
+					0x000e000e
+					0x000e000e
+					0x00000000
+					0x00000003
+					0x0000f2f3
+					0x8000050e
+					0x0000000a
+				>;
+
+				nvidia,mc-burst-data = <
+					0x00000001
+					0x8000001e
+					0x00000001
+					0x00000001
+					0x00000002
+					0x00000000
+					0x00000002
+					0x00000001
+					0x00000002
+					0x00000008
+					0x00000003
+					0x00000002
+					0x00000003
+					0x00000006
+					0x06030203
+					0x000a0402
+					0x74230403
+					0x70000f03
+					0x001f0000
+				>;
+			};
+			timing at 4 {
+				clock-frequency = <102000000>;
+				nvidia,parent-clock-frequency = <408000000>;
+				clocks = <&tegra_car TEGRA124_CLK_PLL_P>;
+				clock-names = "emc-parent";
+
+				nvidia,emc-zcal-cnt-long = <0x00000042>;
+				nvidia,emc-auto-cal-interval = <0x001fffff>;
+				nvidia,emc-ctt-term-ctrl = <0x00000802>;
+				nvidia,emc-cfg = <0x73240000>;
+				nvidia,emc-cfg-2 = <0x000008c5>;
+				nvidia,emc-sel-dpd-ctrl = <0x00040128>;
+				nvidia,emc-cfg-dig-dll = <0x002c0068>;
+				nvidia,emc-bgbias-ctl0 = <0x00000008>;
+				nvidia,emc-auto-cal-config = <0x00000000>;
+				nvidia,emc-auto-cal-config2 = <0x00000000>;
+				nvidia,emc-auto-cal-config3 = <0xa1430000>;
+				nvidia,emc-mode-reset = <0x80001221>;
+				nvidia,emc-mode-1 = <0x80100003>;
+				nvidia,emc-mode-2 = <0x80200008>;
+				nvidia,emc-mode-4 = <0x00000000>;
+
+				nvidia,emc-burst-data = <
+					0x00000004
+					0x0000001a
+					0x00000000
+					0x00000003
+					0x00000001
+					0x00000004
+					0x0000000a
+					0x00000003
+					0x0000000b
+					0x00000001
+					0x00000001
+					0x00000003
+					0x00000003
+					0x00000000
+					0x00000006
+					0x00000006
+					0x00000006
+					0x00000002
+					0x00000000
+					0x00000005
+					0x00000005
+					0x00010000
+					0x00000003
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000004
+					0x0000000c
+					0x0000000d
+					0x0000000f
+					0x00000304
+					0x00000000
+					0x000000c1
+					0x00000002
+					0x00000002
+					0x00000001
+					0x00000000
+					0x00000018
+					0x0000000f
+					0x0000001c
+					0x0000001c
+					0x00000004
+					0x00000005
+					0x00000004
+					0x00000003
+					0x00000000
+					0x00000005
+					0x00000005
+					0x0000031c
+					0x00000000
+					0x00000000
+					0x00000000
+					0x106aa298
+					0x002c00a0
+					0x00008000
+					0x00064000
+					0x00064000
+					0x00064000
+					0x00064000
+					0x00064000
+					0x00064000
+					0x00064000
+					0x00064000
+					0x00064000
+					0x00064000
+					0x00064000
+					0x00064000
+					0x00064000
+					0x00064000
+					0x00064000
+					0x00064000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x000fc000
+					0x000fc000
+					0x000fc000
+					0x000fc000
+					0x0000fc00
+					0x0000fc00
+					0x0000fc00
+					0x0000fc00
+					0x10000280
+					0x00000000
+					0x00111111
+					0x0130b118
+					0x00000000
+					0x00000000
+					0x77ffc081
+					0x00000e0e
+					0x81f1f108
+					0x07070004
+					0x0000003f
+					0x016eeeee
+					0x51451400
+					0x00514514
+					0x00514514
+					0x51451400
+					0x0000003f
+					0x00000033
+					0x00000000
+					0x00000000
+					0x00000042
+					0x000e000e
+					0x000e000e
+					0x00000000
+					0x00000003
+					0x0000f2f3
+					0x80000713
+					0x0000000a
+				>;
+
+				nvidia,mc-burst-data = <
+					0x08000001
+					0x80000026
+					0x00000001
+					0x00000001
+					0x00000003
+					0x00000000
+					0x00000002
+					0x00000001
+					0x00000002
+					0x00000008
+					0x00000003
+					0x00000002
+					0x00000003
+					0x00000006
+					0x06030203
+					0x000a0403
+					0x73c30504
+					0x70000f03
+					0x001f0000
+				>;
+			};
+			timing at 5 {
+				clock-frequency = <204000000>;
+				nvidia,parent-clock-frequency = <408000000>;
+				clocks = <&tegra_car TEGRA124_CLK_PLL_P>;
+				clock-names = "emc-parent";
+
+				nvidia,emc-zcal-cnt-long = <0x00000042>;
+				nvidia,emc-auto-cal-interval = <0x001fffff>;
+				nvidia,emc-ctt-term-ctrl = <0x00000802>;
+				nvidia,emc-cfg = <0x73240000>;
+				nvidia,emc-cfg-2 = <0x0000088d>;
+				nvidia,emc-sel-dpd-ctrl = <0x00040008>;
+				nvidia,emc-cfg-dig-dll = <0x002c0068>;
+				nvidia,emc-bgbias-ctl0 = <0x00000008>;
+				nvidia,emc-auto-cal-config = <0x00000000>;
+				nvidia,emc-auto-cal-config2 = <0x00000000>;
+				nvidia,emc-auto-cal-config3 = <0xa1430000>;
+				nvidia,emc-mode-reset = <0x80001221>;
+				nvidia,emc-mode-1 = <0x80100003>;
+				nvidia,emc-mode-2 = <0x80200008>;
+				nvidia,emc-mode-4 = <0x00000000>;
+
+				nvidia,emc-burst-data = <
+					0x00000009
+					0x00000035
+					0x00000000
+					0x00000006
+					0x00000002
+					0x00000005
+					0x0000000a
+					0x00000003
+					0x0000000b
+					0x00000002
+					0x00000002
+					0x00000003
+					0x00000003
+					0x00000000
+					0x00000005
+					0x00000005
+					0x00000006
+					0x00000002
+					0x00000000
+					0x00000004
+					0x00000006
+					0x00010000
+					0x00000003
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000003
+					0x0000000d
+					0x0000000f
+					0x00000011
+					0x00000607
+					0x00000000
+					0x00000181
+					0x00000002
+					0x00000002
+					0x00000001
+					0x00000000
+					0x00000032
+					0x0000000f
+					0x00000038
+					0x00000038
+					0x00000004
+					0x00000005
+					0x00000004
+					0x00000007
+					0x00000000
+					0x00000005
+					0x00000005
+					0x00000638
+					0x00000000
+					0x00000000
+					0x00000000
+					0x106aa298
+					0x002c00a0
+					0x00008000
+					0x00064000
+					0x00064000
+					0x00064000
+					0x00064000
+					0x00064000
+					0x00064000
+					0x00064000
+					0x00064000
+					0x00064000
+					0x00064000
+					0x00064000
+					0x00064000
+					0x00064000
+					0x00064000
+					0x00064000
+					0x00064000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00008000
+					0x00000000
+					0x00000000
+					0x00008000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00090000
+					0x00090000
+					0x00090000
+					0x00090000
+					0x00009000
+					0x00009000
+					0x00009000
+					0x00009000
+					0x10000280
+					0x00000000
+					0x00111111
+					0x0130b118
+					0x00000000
+					0x00000000
+					0x77ffc081
+					0x00000707
+					0x81f1f108
+					0x07070004
+					0x0000003f
+					0x016eeeee
+					0x51451400
+					0x00514514
+					0x00514514
+					0x51451400
+					0x0000003f
+					0x00000066
+					0x00000000
+					0x00020000
+					0x00000100
+					0x000e000e
+					0x000e000e
+					0x00000000
+					0x00000003
+					0x0000d2b3
+					0x80000d22
+					0x0000000a
+				>;
+
+				nvidia,mc-burst-data = <
+					0x01000003
+					0x80000040
+					0x00000001
+					0x00000001
+					0x00000004
+					0x00000002
+					0x00000004
+					0x00000001
+					0x00000002
+					0x00000008
+					0x00000003
+					0x00000002
+					0x00000004
+					0x00000006
+					0x06040203
+					0x000a0404
+					0x73840a05
+					0x70000f03
+					0x001f0000
+				>;
+			};
+			timing at 6 {
+				clock-frequency = <300000000>;
+				nvidia,parent-clock-frequency = <600000000>;
+				clocks = <&tegra_car TEGRA124_CLK_PLL_C>;
+				clock-names = "emc-parent";
+
+				nvidia,emc-zcal-cnt-long = <0x00000042>;
+				nvidia,emc-auto-cal-interval = <0x001fffff>;
+				nvidia,emc-ctt-term-ctrl = <0x00000802>;
+				nvidia,emc-cfg = <0x73340000>;
+				nvidia,emc-cfg-2 = <0x000008d5>;
+				nvidia,emc-sel-dpd-ctrl = <0x00040128>;
+				nvidia,emc-cfg-dig-dll = <0x002c0068>;
+				nvidia,emc-bgbias-ctl0 = <0x00000000>;
+				nvidia,emc-auto-cal-config = <0x00000000>;
+				nvidia,emc-auto-cal-config2 = <0x00000000>;
+				nvidia,emc-auto-cal-config3 = <0xa1430000>;
+				nvidia,emc-mode-reset = <0x80000321>;
+				nvidia,emc-mode-1 = <0x80100002>;
+				nvidia,emc-mode-2 = <0x80200000>;
+				nvidia,emc-mode-4 = <0x00000000>;
+
+				nvidia,emc-burst-data = <
+					0x0000000d
+					0x0000004d
+					0x00000000
+					0x00000009
+					0x00000003
+					0x00000004
+					0x00000008
+					0x00000002
+					0x00000009
+					0x00000003
+					0x00000003
+					0x00000002
+					0x00000002
+					0x00000000
+					0x00000003
+					0x00000003
+					0x00000005
+					0x00000002
+					0x00000000
+					0x00000002
+					0x00000007
+					0x00020000
+					0x00000003
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000001
+					0x0000000e
+					0x00000010
+					0x00000012
+					0x000008e4
+					0x00000000
+					0x00000239
+					0x00000001
+					0x00000008
+					0x00000001
+					0x00000000
+					0x0000004b
+					0x0000000e
+					0x00000052
+					0x00000200
+					0x00000004
+					0x00000005
+					0x00000004
+					0x00000009
+					0x00000000
+					0x00000005
+					0x00000005
+					0x00000924
+					0x00000000
+					0x00000000
+					0x00000000
+					0x104ab098
+					0x002c00a0
+					0x00008000
+					0x00030000
+					0x00030000
+					0x00030000
+					0x00030000
+					0x00030000
+					0x00030000
+					0x00030000
+					0x00030000
+					0x00030000
+					0x00030000
+					0x00030000
+					0x00030000
+					0x00030000
+					0x00030000
+					0x00030000
+					0x00030000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00098000
+					0x00098000
+					0x00000000
+					0x00098000
+					0x00098000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00060000
+					0x00060000
+					0x00060000
+					0x00060000
+					0x00006000
+					0x00006000
+					0x00006000
+					0x00006000
+					0x10000280
+					0x00000000
+					0x00111111
+					0x01231339
+					0x00000000
+					0x00000000
+					0x77ffc081
+					0x00000505
+					0x81f1f108
+					0x07070004
+					0x00000000
+					0x016eeeee
+					0x51451420
+					0x00514514
+					0x00514514
+					0x51451400
+					0x0000003f
+					0x00000096
+					0x00000000
+					0x00020000
+					0x00000100
+					0x0173000e
+					0x0173000e
+					0x00000000
+					0x00000003
+					0x000052a3
+					0x800012d7
+					0x00000009
+				>;
+
+				nvidia,mc-burst-data = <
+					0x08000004
+					0x80000040
+					0x00000001
+					0x00000002
+					0x00000007
+					0x00000004
+					0x00000005
+					0x00000001
+					0x00000002
+					0x00000007
+					0x00000002
+					0x00000002
+					0x00000004
+					0x00000006
+					0x06040202
+					0x000b0607
+					0x77450e08
+					0x70000f03
+					0x001f0000
+				>;
+			};
+			timing at 7 {
+				clock-frequency = <396000000>;
+				nvidia,parent-clock-frequency = <792000000>;
+				clocks = <&tegra_car TEGRA124_CLK_PLL_M>;
+				clock-names = "emc-parent";
+
+				nvidia,emc-zcal-cnt-long = <0x00000042>;
+				nvidia,emc-auto-cal-interval = <0x001fffff>;
+				nvidia,emc-ctt-term-ctrl = <0x00000802>;
+				nvidia,emc-cfg = <0x73340000>;
+				nvidia,emc-cfg-2 = <0x00000895>;
+				nvidia,emc-sel-dpd-ctrl = <0x00040008>;
+				nvidia,emc-cfg-dig-dll = <0x002c0068>;
+				nvidia,emc-bgbias-ctl0 = <0x00000000>;
+				nvidia,emc-auto-cal-config = <0x00000000>;
+				nvidia,emc-auto-cal-config2 = <0x00000000>;
+				nvidia,emc-auto-cal-config3 = <0xa1430000>;
+				nvidia,emc-mode-reset = <0x80000521>;
+				nvidia,emc-mode-1 = <0x80100002>;
+				nvidia,emc-mode-2 = <0x80200000>;
+				nvidia,emc-mode-4 = <0x00000000>;
+
+				nvidia,emc-burst-data = <
+					0x00000011
+					0x00000066
+					0x00000000
+					0x0000000c
+					0x00000004
+					0x00000004
+					0x00000008
+					0x00000002
+					0x0000000a
+					0x00000004
+					0x00000004
+					0x00000002
+					0x00000002
+					0x00000000
+					0x00000003
+					0x00000003
+					0x00000005
+					0x00000002
+					0x00000000
+					0x00000001
+					0x00000008
+					0x00020000
+					0x00000003
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x0000000f
+					0x00000010
+					0x00000012
+					0x00000bd1
+					0x00000000
+					0x000002f4
+					0x00000001
+					0x00000008
+					0x00000001
+					0x00000000
+					0x00000063
+					0x0000000f
+					0x0000006c
+					0x00000200
+					0x00000004
+					0x00000005
+					0x00000004
+					0x0000000d
+					0x00000000
+					0x00000005
+					0x00000005
+					0x00000c11
+					0x00000000
+					0x00000000
+					0x00000000
+					0x104ab098
+					0x002c00a0
+					0x00008000
+					0x00030000
+					0x00030000
+					0x00030000
+					0x00030000
+					0x00030000
+					0x00030000
+					0x00030000
+					0x00030000
+					0x00030000
+					0x00030000
+					0x00030000
+					0x00030000
+					0x00030000
+					0x00030000
+					0x00030000
+					0x00030000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00070000
+					0x00070000
+					0x00000000
+					0x00070000
+					0x00070000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00038000
+					0x00038000
+					0x00038000
+					0x00038000
+					0x00003800
+					0x00003800
+					0x00003800
+					0x00003800
+					0x10000280
+					0x00000000
+					0x00111111
+					0x01231339
+					0x00000000
+					0x00000000
+					0x77ffc081
+					0x00000505
+					0x81f1f108
+					0x07070004
+					0x00000000
+					0x016eeeee
+					0x51451420
+					0x00514514
+					0x00514514
+					0x51451400
+					0x0000003f
+					0x000000c6
+					0x00000000
+					0x00020000
+					0x00000100
+					0x015b000e
+					0x015b000e
+					0x00000000
+					0x00000003
+					0x000052a3
+					0x8000188b
+					0x00000009
+				>;
+
+				nvidia,mc-burst-data = <
+					0x0f000005
+					0x80000040
+					0x00000001
+					0x00000002
+					0x00000009
+					0x00000005
+					0x00000007
+					0x00000001
+					0x00000002
+					0x00000008
+					0x00000002
+					0x00000002
+					0x00000004
+					0x00000006
+					0x06040202
+					0x000d0709
+					0x7586120a
+					0x70000f03
+					0x001f0000
+				>;
+			};
+			timing at 8 {
+				clock-frequency = <528000000>;
+				nvidia,parent-clock-frequency = <528000000>;
+				clocks = <&tegra_car TEGRA124_CLK_PLL_M_UD>;
+				clock-names = "emc-parent";
+
+				nvidia,emc-zcal-cnt-long = <0x00000042>;
+				nvidia,emc-auto-cal-interval = <0x001fffff>;
+				nvidia,emc-ctt-term-ctrl = <0x00000802>;
+				nvidia,emc-cfg = <0x73300000>;
+				nvidia,emc-cfg-2 = <0x0000089d>;
+				nvidia,emc-sel-dpd-ctrl = <0x00040008>;
+				nvidia,emc-cfg-dig-dll = <0xe0120069>;
+				nvidia,emc-bgbias-ctl0 = <0x00000000>;
+				nvidia,emc-auto-cal-config = <0x00000000>;
+				nvidia,emc-auto-cal-config2 = <0x00000000>;
+				nvidia,emc-auto-cal-config3 = <0xa1430000>;
+				nvidia,emc-mode-reset = <0x80000941>;
+				nvidia,emc-mode-1 = <0x80100002>;
+				nvidia,emc-mode-2 = <0x80200008>;
+				nvidia,emc-mode-4 = <0x00000000>;
+
+				nvidia,emc-burst-data = <
+					0x00000018
+					0x00000088
+					0x00000000
+					0x00000010
+					0x00000006
+					0x00000006
+					0x00000009
+					0x00000002
+					0x0000000d
+					0x00000006
+					0x00000006
+					0x00000002
+					0x00000002
+					0x00000000
+					0x00000003
+					0x00000003
+					0x00000006
+					0x00000002
+					0x00000000
+					0x00000001
+					0x00000009
+					0x00030000
+					0x00000003
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000010
+					0x00000012
+					0x00000014
+					0x00000fd6
+					0x00000000
+					0x000003f5
+					0x00000002
+					0x0000000b
+					0x00000001
+					0x00000000
+					0x00000085
+					0x00000012
+					0x00000090
+					0x00000200
+					0x00000004
+					0x00000005
+					0x00000004
+					0x00000013
+					0x00000000
+					0x00000006
+					0x00000006
+					0x00001017
+					0x00000000
+					0x00000000
+					0x00000000
+					0x104ab098
+					0xe01200b1
+					0x00008000
+					0x0000000a
+					0x0000000a
+					0x0000000a
+					0x0000000a
+					0x0000000a
+					0x0000000a
+					0x0000000a
+					0x0000000a
+					0x0000000a
+					0x0000000a
+					0x0000000a
+					0x0000000a
+					0x0000000a
+					0x0000000a
+					0x0000000a
+					0x0000000a
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00054000
+					0x00054000
+					0x00000000
+					0x00054000
+					0x00054000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x0000000e
+					0x0000000e
+					0x0000000e
+					0x0000000e
+					0x0000000e
+					0x0000000e
+					0x0000000e
+					0x0000000e
+					0x100002a0
+					0x00000000
+					0x00111111
+					0x0123133d
+					0x00000000
+					0x00000000
+					0x77ffc085
+					0x00000505
+					0x81f1f108
+					0x07070004
+					0x00000000
+					0x016eeeee
+					0x51451420
+					0x00514514
+					0x00514514
+					0x51451400
+					0x0606003f
+					0x00000000
+					0x00000000
+					0x00020000
+					0x00000100
+					0x0139000e
+					0x0139000e
+					0x00000000
+					0x00000003
+					0x000042a0
+					0x80002062
+					0x0000000a
+				>;
+
+				nvidia,mc-burst-data = <
+					0x0f000007
+					0x80000040
+					0x00000002
+					0x00000003
+					0x0000000c
+					0x00000007
+					0x0000000a
+					0x00000001
+					0x00000002
+					0x00000009
+					0x00000002
+					0x00000002
+					0x00000005
+					0x00000006
+					0x06050202
+					0x0010090c
+					0x7428180d
+					0x70000f03
+					0x001f0000
+				>;
+			};
+			timing at 9 {
+				clock-frequency = <600000000>;
+				nvidia,parent-clock-frequency = <600000000>;
+				clocks = <&tegra_car TEGRA124_CLK_PLL_C_UD>;
+				clock-names = "emc-parent";
+
+				nvidia,emc-zcal-cnt-long = <0x00000042>;
+				nvidia,emc-auto-cal-interval = <0x001fffff>;
+				nvidia,emc-ctt-term-ctrl = <0x00000802>;
+				nvidia,emc-cfg = <0x73300000>;
+				nvidia,emc-cfg-2 = <0x0000089d>;
+				nvidia,emc-sel-dpd-ctrl = <0x00040008>;
+				nvidia,emc-cfg-dig-dll = <0xe00e0069>;
+				nvidia,emc-bgbias-ctl0 = <0x00000000>;
+				nvidia,emc-auto-cal-config = <0x00000000>;
+				nvidia,emc-auto-cal-config2 = <0x00000000>;
+				nvidia,emc-auto-cal-config3 = <0xa1430000>;
+				nvidia,emc-mode-reset = <0x80000b61>;
+				nvidia,emc-mode-1 = <0x80100002>;
+				nvidia,emc-mode-2 = <0x80200010>;
+				nvidia,emc-mode-4 = <0x00000000>;
+
+				nvidia,emc-burst-data = <
+					0x0000001b
+					0x0000009b
+					0x00000000
+					0x00000013
+					0x00000007
+					0x00000007
+					0x0000000b
+					0x00000003
+					0x00000010
+					0x00000007
+					0x00000007
+					0x00000002
+					0x00000002
+					0x00000000
+					0x00000005
+					0x00000005
+					0x0000000a
+					0x00000002
+					0x00000000
+					0x00000003
+					0x0000000b
+					0x00070000
+					0x00000003
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000002
+					0x00000012
+					0x00000016
+					0x00000018
+					0x00001208
+					0x00000000
+					0x00000482
+					0x00000002
+					0x0000000d
+					0x00000001
+					0x00000000
+					0x00000097
+					0x00000015
+					0x000000a3
+					0x00000200
+					0x00000004
+					0x00000005
+					0x00000004
+					0x00000015
+					0x00000000
+					0x00000006
+					0x00000006
+					0x00001248
+					0x00000000
+					0x00000000
+					0x00000000
+					0x104ab098
+					0xe00e00b1
+					0x00008000
+					0x0000000a
+					0x0000000a
+					0x0000000a
+					0x0000000a
+					0x0000000a
+					0x0000000a
+					0x0000000a
+					0x0000000a
+					0x0000000a
+					0x0000000a
+					0x0000000a
+					0x0000000a
+					0x0000000a
+					0x0000000a
+					0x0000000a
+					0x0000000a
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00048000
+					0x00048000
+					0x00000000
+					0x00048000
+					0x00048000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x0000000d
+					0x0000000d
+					0x0000000d
+					0x0000000d
+					0x0000000d
+					0x0000000d
+					0x0000000d
+					0x0000000d
+					0x100002a0
+					0x00000000
+					0x00111111
+					0x0121113d
+					0x00000000
+					0x00000000
+					0x77ffc085
+					0x00000505
+					0x81f1f108
+					0x07070004
+					0x00000000
+					0x016eeeee
+					0x51451420
+					0x00514514
+					0x00514514
+					0x51451400
+					0x0606003f
+					0x00000000
+					0x00000000
+					0x00020000
+					0x00000100
+					0x0127000e
+					0x0127000e
+					0x00000000
+					0x00000003
+					0x000040a0
+					0x800024a9
+					0x0000000e
+				>;
+
+				nvidia,mc-burst-data = <
+					0x00000009
+					0x80000040
+					0x00000003
+					0x00000004
+					0x0000000e
+					0x00000009
+					0x0000000b
+					0x00000001
+					0x00000003
+					0x0000000b
+					0x00000002
+					0x00000002
+					0x00000005
+					0x00000007
+					0x07050202
+					0x00130b0e
+					0x73a91b0f
+					0x70000f03
+					0x001f0000
+				>;
+			};
+			timing at 10 {
+				clock-frequency = <792000000>;
+				nvidia,parent-clock-frequency = <792000000>;
+				clocks = <&tegra_car TEGRA124_CLK_PLL_M_UD>;
+				clock-names = "emc-parent";
+
+				nvidia,emc-zcal-cnt-long = <0x00000042>;
+				nvidia,emc-auto-cal-interval = <0x001fffff>;
+				nvidia,emc-ctt-term-ctrl = <0x00000802>;
+				nvidia,emc-cfg = <0x73300000>;
+				nvidia,emc-cfg-2 = <0x0000089d>;
+				nvidia,emc-sel-dpd-ctrl = <0x00040000>;
+				nvidia,emc-cfg-dig-dll = <0xe0070069>;
+				nvidia,emc-bgbias-ctl0 = <0x00000000>;
+				nvidia,emc-auto-cal-config = <0x00000000>;
+				nvidia,emc-auto-cal-config2 = <0x00000000>;
+				nvidia,emc-auto-cal-config3 = <0xa1430000>;
+				nvidia,emc-mode-reset = <0x80000d71>;
+				nvidia,emc-mode-1 = <0x80100002>;
+				nvidia,emc-mode-2 = <0x80200018>;
+				nvidia,emc-mode-4 = <0x00000000>;
+
+				nvidia,emc-burst-data = <
+					0x00000024
+					0x000000cd
+					0x00000000
+					0x00000019
+					0x0000000a
+					0x00000008
+					0x0000000d
+					0x00000004
+					0x00000013
+					0x0000000a
+					0x0000000a
+					0x00000003
+					0x00000002
+					0x00000000
+					0x00000006
+					0x00000006
+					0x0000000b
+					0x00000002
+					0x00000000
+					0x00000002
+					0x0000000d
+					0x00080000
+					0x00000004
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000001
+					0x00000014
+					0x00000018
+					0x0000001a
+					0x000017e2
+					0x00000000
+					0x000005f8
+					0x00000003
+					0x00000011
+					0x00000001
+					0x00000000
+					0x000000c7
+					0x00000018
+					0x000000d7
+					0x00000200
+					0x00000005
+					0x00000006
+					0x00000005
+					0x0000001d
+					0x00000000
+					0x00000008
+					0x00000008
+					0x00001822
+					0x00000000
+					0x00000000
+					0x00000000
+					0x104ab098
+					0xe00700b1
+					0x00008000
+					0x00000008
+					0x00000008
+					0x00000008
+					0x00000008
+					0x00000008
+					0x00000008
+					0x00000008
+					0x00000008
+					0x00000008
+					0x00000008
+					0x00000008
+					0x00000008
+					0x00000008
+					0x00000008
+					0x00000008
+					0x00000008
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00034000
+					0x00034000
+					0x00000000
+					0x00034000
+					0x00034000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000005
+					0x00000005
+					0x00000005
+					0x00000005
+					0x00000005
+					0x00000005
+					0x00000005
+					0x00000005
+					0x00000005
+					0x00000005
+					0x00000005
+					0x00000005
+					0x00000005
+					0x00000005
+					0x00000005
+					0x00000005
+					0x0000000a
+					0x0000000a
+					0x0000000a
+					0x0000000a
+					0x0000000a
+					0x0000000a
+					0x0000000a
+					0x0000000a
+					0x100002a0
+					0x00000000
+					0x00111111
+					0x0120113d
+					0x00000000
+					0x00000000
+					0x77ffc085
+					0x00000000
+					0x81f1f108
+					0x07070004
+					0x00000000
+					0x016eeeee
+					0x61861820
+					0x00514514
+					0x00514514
+					0x61861800
+					0x0606003f
+					0x00000000
+					0x00000000
+					0x00020000
+					0x00000100
+					0x00f7000e
+					0x00f7000e
+					0x00000000
+					0x00000004
+					0x00004080
+					0x80003012
+					0x0000000f
+				>;
+
+				nvidia,mc-burst-data = <
+					0x0e00000b
+					0x80000040
+					0x00000004
+					0x00000005
+					0x00000013
+					0x0000000c
+					0x0000000f
+					0x00000002
+					0x00000003
+					0x0000000c
+					0x00000002
+					0x00000002
+					0x00000006
+					0x00000008
+					0x08060202
+					0x00170e13
+					0x736c2414
+					0x70000f02
+					0x001f0000
+				>;
+			};
+			timing at 11 {
+				clock-frequency = <924000000>;
+				nvidia,parent-clock-frequency = <924000000>;
+				clocks = <&tegra_car TEGRA124_CLK_PLL_M_UD>;
+				clock-names = "emc-parent";
+
+				nvidia,emc-zcal-cnt-long = <0x0000004c>;
+				nvidia,emc-auto-cal-interval = <0x001fffff>;
+				nvidia,emc-ctt-term-ctrl = <0x00000802>;
+				nvidia,emc-cfg = <0x73300000>;
+				nvidia,emc-cfg-2 = <0x0000089d>;
+				nvidia,emc-sel-dpd-ctrl = <0x00040000>;
+				nvidia,emc-cfg-dig-dll = <0xe0040069>;
+				nvidia,emc-bgbias-ctl0 = <0x00000000>;
+				nvidia,emc-auto-cal-config = <0x00000000>;
+				nvidia,emc-auto-cal-config2 = <0x00000000>;
+				nvidia,emc-auto-cal-config3 = <0xa1430303>;
+				nvidia,emc-mode-reset = <0x80000f15>;
+				nvidia,emc-mode-1 = <0x80100002>;
+				nvidia,emc-mode-2 = <0x80200020>;
+				nvidia,emc-mode-4 = <0x00000000>;
+
+				nvidia,emc-burst-data = <
+					0x0000002b
+					0x000000f0
+					0x00000000
+					0x0000001e
+					0x0000000b
+					0x00000009
+					0x0000000f
+					0x00000005
+					0x00000016
+					0x0000000b
+					0x0000000b
+					0x00000004
+					0x00000002
+					0x00000000
+					0x00000007
+					0x00000007
+					0x0000000d
+					0x00000002
+					0x00000000
+					0x00000002
+					0x0000000f
+					0x000a0000
+					0x00000004
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000001
+					0x00000016
+					0x0000001a
+					0x0000001c
+					0x00001be7
+					0x00000000
+					0x000006f9
+					0x00000004
+					0x00000015
+					0x00000001
+					0x00000000
+					0x000000e7
+					0x0000001b
+					0x000000fb
+					0x00000200
+					0x00000006
+					0x00000007
+					0x00000006
+					0x00000022
+					0x00000000
+					0x0000000a
+					0x0000000a
+					0x00001c28
+					0x00000000
+					0x00000000
+					0x00000000
+					0x104ab898
+					0xe00400b1
+					0x00008000
+					0x007f800a
+					0x007f800a
+					0x007f800a
+					0x007f800a
+					0x007f800a
+					0x007f800a
+					0x007f800a
+					0x007f800a
+					0x007f800a
+					0x007f800a
+					0x007f800a
+					0x007f800a
+					0x007f800a
+					0x007f800a
+					0x007f800a
+					0x007f800a
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x0002c000
+					0x0002c000
+					0x00000000
+					0x0002c000
+					0x0002c000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000000
+					0x00000005
+					0x00000005
+					0x00000005
+					0x00000005
+					0x00000005
+					0x00000005
+					0x00000005
+					0x00000005
+					0x00000005
+					0x00000005
+					0x00000005
+					0x00000005
+					0x00000005
+					0x00000005
+					0x00000005
+					0x00000005
+					0x00000008
+					0x00000008
+					0x00000008
+					0x00000008
+					0x00000008
+					0x00000008
+					0x00000008
+					0x00000008
+					0x100002a0
+					0x00000000
+					0x00111111
+					0x0120113d
+					0x00000000
+					0x00000000
+					0x77ffc085
+					0x00000000
+					0x81f1f108
+					0x07070004
+					0x00000000
+					0x016eeeee
+					0x5d75d720
+					0x00514514
+					0x00514514
+					0x5d75d700
+					0x0606003f
+					0x00000000
+					0x00000000
+					0x00020000
+					0x00000128
+					0x00cd000e
+					0x00cd000e
+					0x00000000
+					0x00000004
+					0x00004080
+					0x800037ea
+					0x00000011
+				>;
+
+				nvidia,mc-burst-data = <
+					0x0e00000d
+					0x80000040
+					0x00000005
+					0x00000006
+					0x00000016
+					0x0000000e
+					0x00000011
+					0x00000002
+					0x00000004
+					0x0000000e
+					0x00000002
+					0x00000002
+					0x00000006
+					0x00000009
+					0x09060202
+					0x001a1016
+					0x734e2a17
+					0x70000f02
+					0x001f0000
+				>;
+			};
+		};
+	};
+};
diff --git a/arch/arm/boot/dts/tegra124-jetson-tk1.dts b/arch/arm/boot/dts/tegra124-jetson-tk1.dts
index 16082c0..9bbc74e 100644
--- a/arch/arm/boot/dts/tegra124-jetson-tk1.dts
+++ b/arch/arm/boot/dts/tegra124-jetson-tk1.dts
@@ -3,6 +3,8 @@
 #include <dt-bindings/input/input.h>
 #include "tegra124.dtsi"
 
+#include "tegra124-jetson-tk1-emc.dtsi"
+
 / {
 	model = "NVIDIA Tegra124 Jetson TK1";
 	compatible = "nvidia,jetson-tk1", "nvidia,tegra124";
-- 
1.8.1.5

^ permalink raw reply related	[flat|nested] 53+ messages in thread

* [PATCH 8/8] clk: tegra: Add EMC clock driver
  2014-07-11 14:18 [PATCH 0/8] Tegra124 EMC (external memory controller) support Mikko Perttunen
                   ` (6 preceding siblings ...)
  2014-07-11 14:18 ` [PATCH 7/8] ARM: tegra: Add EMC timings to Jetson TK1 " Mikko Perttunen
@ 2014-07-11 14:18 ` Mikko Perttunen
  2014-07-22 16:57   ` Stephen Warren
  2014-08-25 17:40 ` [PATCH 0/8] Tegra124 EMC (external memory controller) support Stephen Warren
  8 siblings, 1 reply; 53+ messages in thread
From: Mikko Perttunen @ 2014-07-11 14:18 UTC (permalink / raw)
  To: linux-arm-kernel

The driver is currently only tested on Tegra124 Jetson TK1, but should
work with other Tegra124 boards, provided that correct EMC tables are
provided through the device tree. Older chip models have differing
timing change sequences, so they are not currently supported.

Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
---
 drivers/clk/tegra/Makefile  |    2 +-
 drivers/clk/tegra/clk-emc.c | 1508 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 1509 insertions(+), 1 deletion(-)
 create mode 100644 drivers/clk/tegra/clk-emc.c

diff --git a/drivers/clk/tegra/Makefile b/drivers/clk/tegra/Makefile
index f7dfb72..240e5b4 100644
--- a/drivers/clk/tegra/Makefile
+++ b/drivers/clk/tegra/Makefile
@@ -14,4 +14,4 @@ obj-y					+= clk-tegra-super-gen4.o
 obj-$(CONFIG_ARCH_TEGRA_2x_SOC)         += clk-tegra20.o
 obj-$(CONFIG_ARCH_TEGRA_3x_SOC)         += clk-tegra30.o
 obj-$(CONFIG_ARCH_TEGRA_114_SOC)	+= clk-tegra114.o
-obj-$(CONFIG_ARCH_TEGRA_124_SOC)	+= clk-tegra124.o
+obj-$(CONFIG_ARCH_TEGRA_124_SOC)	+= clk-tegra124.o clk-emc.o
diff --git a/drivers/clk/tegra/clk-emc.c b/drivers/clk/tegra/clk-emc.c
new file mode 100644
index 0000000..008f9f3
--- /dev/null
+++ b/drivers/clk/tegra/clk-emc.c
@@ -0,0 +1,1508 @@
+/*
+ * drivers/clk/tegra/clk-emc.c
+ *
+ * Copyright (c) 2014, NVIDIA CORPORATION.  All rights reserved.
+ *
+ * Author:
+ *	Mikko Perttunen <mperttunen@nvidia.com>
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/io.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/clk-provider.h>
+#include <linux/clk.h>
+#include <linux/of_address.h>
+#include <linux/string.h>
+#include <linux/debugfs.h>
+#include <linux/clkdev.h>
+
+#define EMC_FBIO_CFG5				0x104
+#define	EMC_FBIO_CFG5_DRAM_TYPE_MASK		0x3
+#define	EMC_FBIO_CFG5_DRAM_TYPE_SHIFT		0
+
+#define MC_EMEM_ADR_CFG				0x54
+#define MC_EMEM_ADR_CFG_EMEM_NUMDEV		BIT(0)
+
+#define EMC_INTSTATUS				0x0
+#define EMC_INTSTATUS_CLKCHANGE_COMPLETE	BIT(4)
+
+#define EMC_CFG					0xc
+#define EMC_CFG_DRAM_CLKSTOP_PD			BIT(31)
+#define EMC_CFG_DRAM_CLKSTOP_SR			BIT(30)
+#define EMC_CFG_DRAM_ACPD			BIT(29)
+#define EMC_CFG_DYN_SREF			BIT(28)
+#define EMC_CFG_PWR_MASK			((0xF << 28) | BIT(18))
+#define EMC_CFG_DSR_VTTGEN_DRV_EN		BIT(18)
+
+#define EMC_REFCTRL				0x20
+#define EMC_REFCTRL_DEV_SEL_SHIFT		0
+#define EMC_REFCTRL_ENABLE			BIT(31)
+
+#define EMC_TIMING_CONTROL			0x28
+#define EMC_RC					0x2c
+#define EMC_RFC					0x30
+#define EMC_RAS					0x34
+#define EMC_RP					0x38
+#define EMC_R2W					0x3c
+#define EMC_W2R					0x40
+#define EMC_R2P					0x44
+#define EMC_W2P					0x48
+#define EMC_RD_RCD				0x4c
+#define EMC_WR_RCD				0x50
+#define EMC_RRD					0x54
+#define EMC_REXT				0x58
+#define EMC_WDV					0x5c
+#define EMC_QUSE				0x60
+#define EMC_QRST				0x64
+#define EMC_QSAFE				0x68
+#define EMC_RDV					0x6c
+#define EMC_REFRESH				0x70
+#define EMC_BURST_REFRESH_NUM			0x74
+#define EMC_PDEX2WR				0x78
+#define EMC_PDEX2RD				0x7c
+#define EMC_PCHG2PDEN				0x80
+#define EMC_ACT2PDEN				0x84
+#define EMC_AR2PDEN				0x88
+#define EMC_RW2PDEN				0x8c
+#define EMC_TXSR				0x90
+#define EMC_TCKE				0x94
+#define EMC_TFAW				0x98
+#define EMC_TRPAB				0x9c
+#define EMC_TCLKSTABLE				0xa0
+#define EMC_TCLKSTOP				0xa4
+#define EMC_TREFBW				0xa8
+#define EMC_ODT_WRITE				0xb0
+#define EMC_ODT_READ				0xb4
+#define EMC_WEXT				0xb8
+#define EMC_CTT					0xbc
+#define EMC_RFC_SLR				0xc0
+#define EMC_MRS_WAIT_CNT2			0xc4
+
+#define EMC_MRS_WAIT_CNT			0xc8
+#define EMC_MRS_WAIT_CNT_SHORT_WAIT_SHIFT	0
+#define EMC_MRS_WAIT_CNT_SHORT_WAIT_MASK	\
+	(0x3FF << EMC_MRS_WAIT_CNT_SHORT_WAIT_SHIFT)
+#define EMC_MRS_WAIT_CNT_LONG_WAIT_SHIFT	16
+#define EMC_MRS_WAIT_CNT_LONG_WAIT_MASK		\
+	(0x3FF << EMC_MRS_WAIT_CNT_LONG_WAIT_SHIFT)
+
+#define EMC_MRS					0xcc
+#define EMC_MODE_SET_DLL_RESET			BIT(8)
+#define EMC_MODE_SET_LONG_CNT			BIT(26)
+#define EMC_EMRS				0xd0
+#define EMC_REF					0xd4
+#define EMC_PRE					0xd8
+
+#define EMC_SELF_REF				0xe0
+#define EMC_SELF_REF_CMD_ENABLED		BIT(0)
+#define EMC_SELF_REF_DEV_SEL_SHIFT		30
+
+#define EMC_MRW					0xe8
+
+#define EMC_MRR					0xec
+#define EMC_MRR_MA_SHIFT			16
+#define LPDDR2_MR4_TEMP_SHIFT			0
+
+#define EMC_XM2DQSPADCTRL3			0xf8
+#define EMC_FBIO_SPARE				0x100
+
+#define EMC_FBIO_CFG6				0x114
+#define EMC_EMRS2				0x12c
+#define EMC_MRW2				0x134
+#define EMC_MRW4				0x13c
+#define EMC_EINPUT				0x14c
+#define EMC_EINPUT_DURATION			0x150
+#define EMC_PUTERM_EXTRA			0x154
+#define EMC_TCKESR				0x158
+#define EMC_TPD					0x15c
+
+#define EMC_AUTO_CAL_CONFIG			0x2a4
+#define EMC_AUTO_CAL_CONFIG_AUTO_CAL_START	BIT(31)
+#define EMC_AUTO_CAL_INTERVAL			0x2a8
+#define EMC_AUTO_CAL_STATUS			0x2ac
+#define EMC_AUTO_CAL_STATUS_ACTIVE		BIT(31)
+#define EMC_STATUS				0x2b4
+#define EMC_STATUS_TIMING_UPDATE_STALLED	BIT(23)
+
+#define EMC_CFG_2				0x2b8
+#define EMC_CFG_2_MODE_SHIFT			0
+#define EMC_CFG_2_DIS_STP_OB_CLK_DURING_NON_WR	BIT(6)
+
+#define EMC_CFG_DIG_DLL				0x2bc
+#define EMC_CFG_DIG_DLL_PERIOD			0x2c0
+#define EMC_RDV_MASK				0x2cc
+#define EMC_WDV_MASK				0x2d0
+#define EMC_CTT_DURATION			0x2d8
+#define EMC_CTT_TERM_CTRL			0x2dc
+#define EMC_ZCAL_INTERVAL			0x2e0
+#define EMC_ZCAL_WAIT_CNT			0x2e4
+
+#define EMC_ZQ_CAL				0x2ec
+#define EMC_ZQ_CAL_CMD				BIT(0)
+#define EMC_ZQ_CAL_LONG				BIT(4)
+#define EMC_ZQ_CAL_LONG_CMD_DEV0		\
+	(DRAM_DEV_SEL_0 | EMC_ZQ_CAL_LONG | EMC_ZQ_CAL_CMD)
+#define EMC_ZQ_CAL_LONG_CMD_DEV1		\
+	(DRAM_DEV_SEL_1 | EMC_ZQ_CAL_LONG | EMC_ZQ_CAL_CMD)
+
+#define EMC_XM2CMDPADCTRL			0x2f0
+#define EMC_XM2DQSPADCTRL			0x2f8
+#define EMC_XM2DQSPADCTRL2			0x2fc
+#define EMC_XM2DQSPADCTRL2_RX_FT_REC_ENABLE	BIT(0)
+#define EMC_XM2DQSPADCTRL2_VREF_ENABLE		BIT(5)
+#define EMC_XM2DQPADCTRL			0x300
+#define EMC_XM2DQPADCTRL2			0x304
+#define EMC_XM2CLKPADCTRL			0x308
+#define EMC_XM2COMPPADCTRL			0x30c
+#define EMC_XM2VTTGENPADCTRL			0x310
+#define EMC_XM2VTTGENPADCTRL2			0x314
+#define EMC_XM2VTTGENPADCTRL3			0x318
+#define EMC_XM2DQSPADCTRL4			0x320
+#define EMC_DLL_XFORM_DQS0			0x328
+#define EMC_DLL_XFORM_DQS1			0x32c
+#define EMC_DLL_XFORM_DQS2			0x330
+#define EMC_DLL_XFORM_DQS3			0x334
+#define EMC_DLL_XFORM_DQS4			0x338
+#define EMC_DLL_XFORM_DQS5			0x33c
+#define EMC_DLL_XFORM_DQS6			0x340
+#define EMC_DLL_XFORM_DQS7			0x344
+#define EMC_DLL_XFORM_QUSE0			0x348
+#define EMC_DLL_XFORM_QUSE1			0x34c
+#define EMC_DLL_XFORM_QUSE2			0x350
+#define EMC_DLL_XFORM_QUSE3			0x354
+#define EMC_DLL_XFORM_QUSE4			0x358
+#define EMC_DLL_XFORM_QUSE5			0x35c
+#define EMC_DLL_XFORM_QUSE6			0x360
+#define EMC_DLL_XFORM_QUSE7			0x364
+#define EMC_DLL_XFORM_DQ0			0x368
+#define EMC_DLL_XFORM_DQ1			0x36c
+#define EMC_DLL_XFORM_DQ2			0x370
+#define EMC_DLL_XFORM_DQ3			0x374
+#define EMC_DLI_TRIM_TXDQS0			0x3a8
+#define EMC_DLI_TRIM_TXDQS1			0x3ac
+#define EMC_DLI_TRIM_TXDQS2			0x3b0
+#define EMC_DLI_TRIM_TXDQS3			0x3b4
+#define EMC_DLI_TRIM_TXDQS4			0x3b8
+#define EMC_DLI_TRIM_TXDQS5			0x3bc
+#define EMC_DLI_TRIM_TXDQS6			0x3c0
+#define EMC_DLI_TRIM_TXDQS7			0x3c4
+#define EMC_STALL_THEN_EXE_AFTER_CLKCHANGE	0x3cc
+#define EMC_SEL_DPD_CTRL			0x3d8
+#define EMC_SEL_DPD_CTRL_DATA_SEL_DPD		BIT(8)
+#define EMC_SEL_DPD_CTRL_ODT_SEL_DPD		BIT(5)
+#define EMC_SEL_DPD_CTRL_RESET_SEL_DPD		BIT(4)
+#define EMC_SEL_DPD_CTRL_CA_SEL_DPD		BIT(3)
+#define EMC_SEL_DPD_CTRL_CLK_SEL_DPD		BIT(2)
+#define EMC_SEL_DPD_CTRL_DDR3_MASK	\
+	((0xf << 2) | BIT(8))
+#define EMC_SEL_DPD_CTRL_MASK \
+	((0x3 << 2) | BIT(5) | BIT(8))
+#define EMC_PRE_REFRESH_REQ_CNT			0x3dc
+#define EMC_DYN_SELF_REF_CONTROL		0x3e0
+#define EMC_TXSRDLL				0x3e4
+#define EMC_CCFIFO_ADDR				0x3e8
+#define EMC_CCFIFO_DATA				0x3ec
+#define EMC_CCFIFO_STATUS			0x3f0
+#define EMC_CDB_CNTL_1				0x3f4
+#define EMC_CDB_CNTL_2				0x3f8
+#define EMC_XM2CLKPADCTRL2			0x3fc
+#define EMC_AUTO_CAL_CONFIG2			0x458
+#define EMC_AUTO_CAL_CONFIG3			0x45c
+#define EMC_IBDLY				0x468
+#define EMC_DLL_XFORM_ADDR0			0x46c
+#define EMC_DLL_XFORM_ADDR1			0x470
+#define EMC_DLL_XFORM_ADDR2			0x474
+#define EMC_DSR_VTTGEN_DRV			0x47c
+#define EMC_TXDSRVTTGEN				0x480
+#define EMC_XM2CMDPADCTRL4			0x484
+#define EMC_XM2CMDPADCTRL5			0x488
+#define EMC_DLL_XFORM_DQS8			0x4a0
+#define EMC_DLL_XFORM_DQS9			0x4a4
+#define EMC_DLL_XFORM_DQS10			0x4a8
+#define EMC_DLL_XFORM_DQS11			0x4ac
+#define EMC_DLL_XFORM_DQS12			0x4b0
+#define EMC_DLL_XFORM_DQS13			0x4b4
+#define EMC_DLL_XFORM_DQS14			0x4b8
+#define EMC_DLL_XFORM_DQS15			0x4bc
+#define EMC_DLL_XFORM_QUSE8			0x4c0
+#define EMC_DLL_XFORM_QUSE9			0x4c4
+#define EMC_DLL_XFORM_QUSE10			0x4c8
+#define EMC_DLL_XFORM_QUSE11			0x4cc
+#define EMC_DLL_XFORM_QUSE12			0x4d0
+#define EMC_DLL_XFORM_QUSE13			0x4d4
+#define EMC_DLL_XFORM_QUSE14			0x4d8
+#define EMC_DLL_XFORM_QUSE15			0x4dc
+#define EMC_DLL_XFORM_DQ4			0x4e0
+#define EMC_DLL_XFORM_DQ5			0x4e4
+#define EMC_DLL_XFORM_DQ6			0x4e8
+#define EMC_DLL_XFORM_DQ7			0x4ec
+#define EMC_DLI_TRIM_TXDQS8			0x520
+#define EMC_DLI_TRIM_TXDQS9			0x524
+#define EMC_DLI_TRIM_TXDQS10			0x528
+#define EMC_DLI_TRIM_TXDQS11			0x52c
+#define EMC_DLI_TRIM_TXDQS12			0x530
+#define EMC_DLI_TRIM_TXDQS13			0x534
+#define EMC_DLI_TRIM_TXDQS14			0x538
+#define EMC_DLI_TRIM_TXDQS15			0x53c
+#define EMC_CDB_CNTL_3				0x540
+#define EMC_XM2DQSPADCTRL5			0x544
+#define EMC_XM2DQSPADCTRL6			0x548
+#define EMC_XM2DQPADCTRL3			0x54c
+#define EMC_DLL_XFORM_ADDR3			0x550
+#define EMC_DLL_XFORM_ADDR4			0x554
+#define EMC_DLL_XFORM_ADDR5			0x558
+#define EMC_CFG_PIPE				0x560
+#define EMC_QPOP				0x564
+#define EMC_QUSE_WIDTH				0x568
+#define EMC_PUTERM_WIDTH			0x56c
+#define EMC_BGBIAS_CTL0				0x570
+#define EMC_BGBIAS_CTL0_BIAS0_DSC_E_PWRD_IBIAS_RX BIT(3)
+#define EMC_BGBIAS_CTL0_BIAS0_DSC_E_PWRD_IBIAS_VTTGEN BIT(2)
+#define EMC_BGBIAS_CTL0_BIAS0_DSC_E_PWRD	BIT(1)
+#define EMC_PUTERM_ADJ				0x574
+
+#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
+
+#define MC_TIMING_CONTROL			0xfc
+
+#define DRAM_DEV_SEL_ALL			0
+#define DRAM_DEV_SEL_0				(2 << 30)
+#define DRAM_DEV_SEL_1				(1 << 30)
+
+#define CLK_SOURCE_EMC				0x19c
+#define CLK_SOURCE_EMC_EMC_2X_CLK_DIVISOR_SHIFT	0
+#define CLK_SOURCE_EMC_EMC_2X_CLK_DIVISOR_MASK	0xff
+#define CLK_SOURCE_EMC_EMC_2X_CLK_SRC_SHIFT	29
+#define CLK_SOURCE_EMC_EMC_2X_CLK_SRC_MASK	0x7
+
+#define EMC_CFG_POWER_FEATURES_MASK		\
+	(EMC_CFG_DYN_SREF | EMC_CFG_DRAM_ACPD | EMC_CFG_DRAM_CLKSTOP_SR | \
+	EMC_CFG_DRAM_CLKSTOP_PD | EMC_CFG_DSR_VTTGEN_DRV_EN)
+#define EMC_REFCTRL_DEV_SEL(n) (((n > 1) ? 0 : 2) << EMC_REFCTRL_DEV_SEL_SHIFT)
+#define EMC_DRAM_DEV_SEL(n) ((n > 1) ? DRAM_DEV_SEL_ALL : DRAM_DEV_SEL_0)
+
+#define EMC_STATUS_UPDATE_TIMEOUT		1000
+
+enum emc_dram_type {
+	DRAM_TYPE_DDR3 = 0,
+	DRAM_TYPE_DDR1 = 1,
+	DRAM_TYPE_LPDDR3 = 2,
+	DRAM_TYPE_DDR2 = 3
+};
+
+enum emc_dll_change {
+	DLL_CHANGE_NONE,
+	DLL_CHANGE_ON,
+	DLL_CHANGE_OFF
+};
+
+static int t124_emc_burst_regs[] = {
+	EMC_RC,
+	EMC_RFC,
+	EMC_RFC_SLR,
+	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_WDV_MASK,
+	EMC_QUSE,
+	EMC_QUSE_WIDTH,
+	EMC_IBDLY,
+	EMC_EINPUT,
+	EMC_EINPUT_DURATION,
+	EMC_PUTERM_EXTRA,
+	EMC_PUTERM_WIDTH,
+	EMC_PUTERM_ADJ,
+	EMC_CDB_CNTL_1,
+	EMC_CDB_CNTL_2,
+	EMC_CDB_CNTL_3,
+	EMC_QRST,
+	EMC_QSAFE,
+	EMC_RDV,
+	EMC_RDV_MASK,
+	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_TCKESR,
+	EMC_TPD,
+	EMC_TFAW,
+	EMC_TRPAB,
+	EMC_TCLKSTABLE,
+	EMC_TCLKSTOP,
+	EMC_TREFBW,
+	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_DQS8,
+	EMC_DLL_XFORM_DQS9,
+	EMC_DLL_XFORM_DQS10,
+	EMC_DLL_XFORM_DQS11,
+	EMC_DLL_XFORM_DQS12,
+	EMC_DLL_XFORM_DQS13,
+	EMC_DLL_XFORM_DQS14,
+	EMC_DLL_XFORM_DQS15,
+	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_DLL_XFORM_ADDR0,
+	EMC_DLL_XFORM_ADDR1,
+	EMC_DLL_XFORM_ADDR2,
+	EMC_DLL_XFORM_ADDR3,
+	EMC_DLL_XFORM_ADDR4,
+	EMC_DLL_XFORM_ADDR5,
+	EMC_DLL_XFORM_QUSE8,
+	EMC_DLL_XFORM_QUSE9,
+	EMC_DLL_XFORM_QUSE10,
+	EMC_DLL_XFORM_QUSE11,
+	EMC_DLL_XFORM_QUSE12,
+	EMC_DLL_XFORM_QUSE13,
+	EMC_DLL_XFORM_QUSE14,
+	EMC_DLL_XFORM_QUSE15,
+	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_DLI_TRIM_TXDQS8,
+	EMC_DLI_TRIM_TXDQS9,
+	EMC_DLI_TRIM_TXDQS10,
+	EMC_DLI_TRIM_TXDQS11,
+	EMC_DLI_TRIM_TXDQS12,
+	EMC_DLI_TRIM_TXDQS13,
+	EMC_DLI_TRIM_TXDQS14,
+	EMC_DLI_TRIM_TXDQS15,
+	EMC_DLL_XFORM_DQ0,
+	EMC_DLL_XFORM_DQ1,
+	EMC_DLL_XFORM_DQ2,
+	EMC_DLL_XFORM_DQ3,
+	EMC_DLL_XFORM_DQ4,
+	EMC_DLL_XFORM_DQ5,
+	EMC_DLL_XFORM_DQ6,
+	EMC_DLL_XFORM_DQ7,
+	EMC_XM2CMDPADCTRL,
+	EMC_XM2CMDPADCTRL4,
+	EMC_XM2CMDPADCTRL5,
+	EMC_XM2DQSPADCTRL2,
+	EMC_XM2DQPADCTRL2,
+	EMC_XM2DQPADCTRL3,
+	EMC_XM2CLKPADCTRL,
+	EMC_XM2CLKPADCTRL2,
+	EMC_XM2COMPPADCTRL,
+	EMC_XM2VTTGENPADCTRL,
+	EMC_XM2VTTGENPADCTRL2,
+	EMC_XM2VTTGENPADCTRL3,
+	EMC_XM2DQSPADCTRL3,
+	EMC_XM2DQSPADCTRL4,
+	EMC_XM2DQSPADCTRL5,
+	EMC_XM2DQSPADCTRL6,
+	EMC_DSR_VTTGEN_DRV,
+	EMC_TXDSRVTTGEN,
+	EMC_FBIO_SPARE,
+	EMC_ZCAL_INTERVAL,
+	EMC_ZCAL_WAIT_CNT,
+	EMC_MRS_WAIT_CNT,
+	EMC_MRS_WAIT_CNT2,
+	EMC_CTT,
+	EMC_CTT_DURATION,
+	EMC_CFG_PIPE,
+	EMC_DYN_SELF_REF_CONTROL,
+	EMC_QPOP
+};
+
+static int t124_mc_burst_regs[] = {
+	MC_EMEM_ARB_CFG,
+	MC_EMEM_ARB_OUTSTANDING_REQ,
+	MC_EMEM_ARB_TIMING_RCD,
+	MC_EMEM_ARB_TIMING_RP,
+	MC_EMEM_ARB_TIMING_RC,
+	MC_EMEM_ARB_TIMING_RAS,
+	MC_EMEM_ARB_TIMING_FAW,
+	MC_EMEM_ARB_TIMING_RRD,
+	MC_EMEM_ARB_TIMING_RAP2PRE,
+	MC_EMEM_ARB_TIMING_WAP2PRE,
+	MC_EMEM_ARB_TIMING_R2R,
+	MC_EMEM_ARB_TIMING_W2W,
+	MC_EMEM_ARB_TIMING_R2W,
+	MC_EMEM_ARB_TIMING_W2R,
+	MC_EMEM_ARB_DA_TURNS,
+	MC_EMEM_ARB_DA_COVERS,
+	MC_EMEM_ARB_MISC0,
+	MC_EMEM_ARB_MISC1,
+	MC_EMEM_ARB_RING1_THROTTLE
+};
+
+const char *emc_parent_clk_names[] = {
+	"pll_m", "pll_c", "pll_p", "clk_m", "pll_m_ud",
+	"pll_c2", "pll_c3", "pll_c_ud"
+};
+
+/* List of clock sources for various parents the EMC clock can have.
+ * When we change the timing to a timing with a parent that has the same
+ * clock source as the current parent, we must first change to a backup
+ * timing that has a different clock source.
+ */
+
+#define EMC_SRC_PLL_M 0
+#define EMC_SRC_PLL_C 1
+#define EMC_SRC_PLL_P 2
+#define EMC_SRC_CLK_M 3
+#define EMC_SRC_PLL_C2 4
+#define EMC_SRC_PLL_C3 5
+const char emc_parent_clk_sources[] = {
+	EMC_SRC_PLL_M, EMC_SRC_PLL_C, EMC_SRC_PLL_P, EMC_SRC_CLK_M,
+	EMC_SRC_PLL_M, EMC_SRC_PLL_C2, EMC_SRC_PLL_C3, EMC_SRC_PLL_C
+};
+
+struct emc_timing {
+	unsigned long rate, parent_rate;
+	u8 parent_index;
+	struct clk *parent;
+
+	/* Store EMC burst data in a union to minimize mistakes. This allows
+	 * us to use the same burst data lists as used by the downstream and
+	 * ChromeOS kernels. */
+	union {
+		u32 emc_burst_data[ARRAY_SIZE(t124_emc_burst_regs)];
+		struct {
+			u32 __pad0[121];
+			u32 emc_xm2dqspadctrl2;
+			u32 __pad1[15];
+			u32 emc_zcal_interval;
+			u32 __pad2[1];
+			u32 emc_mrs_wait_cnt;
+		};
+	};
+	u32 mc_burst_data[ARRAY_SIZE(t124_mc_burst_regs)];
+
+	u32 emc_zcal_cnt_long;
+	u32 emc_auto_cal_interval;
+	u32 emc_ctt_term_ctrl;
+	u32 emc_cfg;
+	u32 emc_cfg_2;
+	u32 emc_sel_dpd_ctrl;
+	u32 __emc_cfg_dig_dll;
+	u32 emc_bgbias_ctl0;
+	u32 emc_auto_cal_config2;
+	u32 emc_auto_cal_config3;
+	u32 emc_auto_cal_config;
+	u32 emc_mode_reset;
+	u32 emc_mode_1;
+	u32 emc_mode_2;
+	u32 emc_mode_4;
+};
+
+struct tegra_emc {
+	struct platform_device *pdev;
+
+	struct clk_hw hw;
+
+	void __iomem *emc_regs;
+	void __iomem *mc_regs;
+	void __iomem *clk_regs;
+
+	enum emc_dram_type dram_type;
+	int dram_num;
+
+	struct emc_timing last_timing;
+	struct emc_timing *timings;
+	unsigned int num_timings;
+
+	struct clk *prev_parent;
+
+	bool changing_timing;
+};
+
+/* * * * * * * * * * * * * * * * * * * * * * * * * *
+ * Timing change sequence functions                *
+ * * * * * * * * * * * * * * * * * * * * * * * * * */
+
+static void emc_ccfifo_writel(struct tegra_emc *tegra, u32 val,
+			      unsigned long offs)
+{
+	writel(val, tegra->emc_regs + EMC_CCFIFO_DATA);
+	writel(offs, tegra->emc_regs + EMC_CCFIFO_ADDR);
+}
+
+static void emc_seq_update_timing(struct tegra_emc *tegra)
+{
+	int i;
+
+	writel(1, tegra->emc_regs + EMC_TIMING_CONTROL);
+
+	for (i = 0; i < EMC_STATUS_UPDATE_TIMEOUT; ++i) {
+		if (!(readl(tegra->emc_regs + EMC_STATUS) &
+		    EMC_STATUS_TIMING_UPDATE_STALLED))
+			return;
+	}
+
+	dev_err(&tegra->pdev->dev, "timing update failed\n");
+	BUG();
+}
+
+static void emc_seq_disable_auto_cal(struct tegra_emc *tegra)
+{
+	int i;
+
+	writel(0, tegra->emc_regs + EMC_AUTO_CAL_INTERVAL);
+
+	for (i = 0; i < EMC_STATUS_UPDATE_TIMEOUT; ++i) {
+		if (!(readl(tegra->emc_regs + EMC_AUTO_CAL_STATUS) &
+		    EMC_AUTO_CAL_STATUS_ACTIVE))
+			return;
+	}
+
+	dev_err(&tegra->pdev->dev, "auto cal disable failed\n");
+	BUG();
+}
+
+static void emc_seq_wait_clkchange(struct tegra_emc *tegra)
+{
+	int i;
+
+	for (i = 0; i < EMC_STATUS_UPDATE_TIMEOUT; ++i) {
+		if (readl(tegra->emc_regs + EMC_INTSTATUS) &
+		    EMC_INTSTATUS_CLKCHANGE_COMPLETE)
+			return;
+	}
+
+	dev_err(&tegra->pdev->dev, "clkchange failed\n");
+	BUG();
+}
+
+static void emc_change_timing(struct tegra_emc *tegra,
+			      const struct emc_timing *timing,
+			      u32 car_value)
+{
+	u32 val, val2;
+	bool update = false;
+	int pre_wait = 0;
+	int i;
+	enum emc_dll_change dll_change;
+
+	BUG_ON(timing->rate == tegra->last_timing.rate);
+	BUG_ON(ARRAY_SIZE(timing->emc_burst_data) !=
+	       ARRAY_SIZE(t124_emc_burst_regs));
+	BUG_ON(ARRAY_SIZE(timing->mc_burst_data) !=
+	       ARRAY_SIZE(t124_mc_burst_regs));
+
+	if ((tegra->last_timing.emc_mode_1 & 0x1) == (timing->emc_mode_1 & 1))
+		dll_change = DLL_CHANGE_NONE;
+	else if (timing->emc_mode_1 & 1)
+		dll_change = DLL_CHANGE_ON;
+	else
+		dll_change = DLL_CHANGE_OFF;
+
+	/* Clear CLKCHANGE_COMPLETE interrupts */
+
+	writel(EMC_INTSTATUS_CLKCHANGE_COMPLETE,
+	       tegra->emc_regs + EMC_INTSTATUS);
+
+
+	/* Disable dynamic self-refresh */
+
+	val = readl(tegra->emc_regs + EMC_CFG);
+	if (val & EMC_CFG_PWR_MASK) {
+		val &= ~EMC_CFG_POWER_FEATURES_MASK;
+		writel(val, tegra->emc_regs + EMC_CFG);
+
+		pre_wait = 5;
+	}
+
+	/* Disable SEL_DPD_CTRL for clock change */
+
+	val = readl(tegra->emc_regs + EMC_SEL_DPD_CTRL);
+	if (val & (tegra->dram_type == DRAM_TYPE_DDR3 ?
+	    EMC_SEL_DPD_CTRL_DDR3_MASK : EMC_SEL_DPD_CTRL_MASK)) {
+		val &= ~(EMC_SEL_DPD_CTRL_DATA_SEL_DPD |
+				EMC_SEL_DPD_CTRL_ODT_SEL_DPD |
+				EMC_SEL_DPD_CTRL_CA_SEL_DPD |
+				EMC_SEL_DPD_CTRL_CLK_SEL_DPD);
+		if (tegra->dram_type == DRAM_TYPE_DDR3)
+			val &= ~EMC_SEL_DPD_CTRL_RESET_SEL_DPD;
+		writel(val, tegra->emc_regs + EMC_SEL_DPD_CTRL);
+	}
+
+	/* Prepare DQ/DQS for clock change */
+
+	val = readl(tegra->emc_regs + EMC_BGBIAS_CTL0);
+	val2 = tegra->last_timing.emc_bgbias_ctl0;
+	if (!(timing->emc_bgbias_ctl0 &
+	      EMC_BGBIAS_CTL0_BIAS0_DSC_E_PWRD_IBIAS_RX) &&
+	    (val & EMC_BGBIAS_CTL0_BIAS0_DSC_E_PWRD_IBIAS_RX)) {
+		val2 &= ~EMC_BGBIAS_CTL0_BIAS0_DSC_E_PWRD_IBIAS_RX;
+		update = true;
+	}
+
+	if ((val & EMC_BGBIAS_CTL0_BIAS0_DSC_E_PWRD) ||
+	    (val & EMC_BGBIAS_CTL0_BIAS0_DSC_E_PWRD_IBIAS_VTTGEN)) {
+		update = true;
+	}
+
+	if (update) {
+		writel(val2, tegra->emc_regs + EMC_BGBIAS_CTL0);
+		if (pre_wait < 5)
+			pre_wait = 5;
+	}
+
+	update = false;
+	val = readl(tegra->emc_regs + EMC_XM2DQSPADCTRL2);
+	if (timing->emc_xm2dqspadctrl2 & EMC_XM2DQSPADCTRL2_VREF_ENABLE &&
+	    !(val & EMC_XM2DQSPADCTRL2_VREF_ENABLE)) {
+		val |= EMC_XM2DQSPADCTRL2_VREF_ENABLE;
+		update = true;
+	}
+
+	if (timing->emc_xm2dqspadctrl2 & EMC_XM2DQSPADCTRL2_RX_FT_REC_ENABLE &&
+	    !(val & EMC_XM2DQSPADCTRL2_RX_FT_REC_ENABLE)) {
+		val |= EMC_XM2DQSPADCTRL2_RX_FT_REC_ENABLE;
+		update = true;
+	}
+
+	if (update) {
+		writel(val, tegra->emc_regs + EMC_XM2DQSPADCTRL2);
+		if (pre_wait < 30)
+			pre_wait = 30;
+	}
+
+	/* Wait to settle */
+
+	if (pre_wait) {
+		emc_seq_update_timing(tegra);
+		udelay(pre_wait);
+	}
+
+	/* Program CTT_TERM control */
+
+	if (tegra->last_timing.emc_ctt_term_ctrl != timing->emc_ctt_term_ctrl) {
+		emc_seq_disable_auto_cal(tegra);
+		writel(timing->emc_ctt_term_ctrl,
+			tegra->emc_regs + EMC_CTT_TERM_CTRL);
+		emc_seq_update_timing(tegra);
+	}
+
+	/* Program burst shadow registers */
+
+	for (i = 0; i < ARRAY_SIZE(timing->emc_burst_data); ++i)
+		__raw_writel(timing->emc_burst_data[i],
+			     tegra->emc_regs + t124_emc_burst_regs[i]);
+
+	/* The MC burst registers are shadowed so they must be written here */
+	for (i = 0; i < ARRAY_SIZE(timing->mc_burst_data); ++i)
+		__raw_writel(timing->mc_burst_data[i],
+			     tegra->mc_regs + t124_mc_burst_regs[i]);
+
+	val = timing->emc_cfg & ~EMC_CFG_POWER_FEATURES_MASK;
+	emc_ccfifo_writel(tegra, val, EMC_CFG);
+
+	/* Program AUTO_CAL_CONFIG */
+
+	if (timing->emc_auto_cal_config2 !=
+	    tegra->last_timing.emc_auto_cal_config2)
+		emc_ccfifo_writel(tegra, timing->emc_auto_cal_config2,
+				  EMC_AUTO_CAL_CONFIG2);
+	if (timing->emc_auto_cal_config3 !=
+	    tegra->last_timing.emc_auto_cal_config3)
+		emc_ccfifo_writel(tegra, timing->emc_auto_cal_config3,
+				  EMC_AUTO_CAL_CONFIG3);
+	if (timing->emc_auto_cal_config !=
+	    tegra->last_timing.emc_auto_cal_config) {
+		val = timing->emc_auto_cal_config;
+		val &= EMC_AUTO_CAL_CONFIG_AUTO_CAL_START;
+		emc_ccfifo_writel(tegra, val, EMC_AUTO_CAL_CONFIG);
+	}
+
+	/* DDR3: predict MRS long wait count */
+
+	if (tegra->dram_type == DRAM_TYPE_DDR3 && dll_change == DLL_CHANGE_ON) {
+		u32 cnt = 32;
+
+		if (timing->emc_zcal_interval != 0 &&
+		    tegra->last_timing.emc_zcal_interval == 0)
+			cnt -= tegra->dram_num * 256;
+
+		val = timing->emc_mrs_wait_cnt
+			& EMC_MRS_WAIT_CNT_SHORT_WAIT_MASK
+			>> EMC_MRS_WAIT_CNT_SHORT_WAIT_SHIFT;
+		if (cnt < val)
+			cnt = val;
+
+		val = timing->emc_mrs_wait_cnt
+			& ~EMC_MRS_WAIT_CNT_LONG_WAIT_MASK;
+		val |= (cnt << EMC_MRS_WAIT_CNT_LONG_WAIT_SHIFT)
+			& EMC_MRS_WAIT_CNT_LONG_WAIT_MASK;
+
+		writel(val, tegra->emc_regs + EMC_MRS_WAIT_CNT);
+	}
+
+	val = timing->emc_cfg_2;
+	val &= ~EMC_CFG_2_DIS_STP_OB_CLK_DURING_NON_WR;
+	emc_ccfifo_writel(tegra, val, EMC_CFG_2);
+
+	/* DDR3: Turn off DLL and enter self-refresh */
+
+	if (tegra->dram_type == DRAM_TYPE_DDR3 && dll_change == DLL_CHANGE_OFF)
+		emc_ccfifo_writel(tegra, timing->emc_mode_1, EMC_EMRS);
+
+	/* Disable refresh controller */
+
+	emc_ccfifo_writel(tegra, EMC_REFCTRL_DEV_SEL(tegra->dram_num),
+			  EMC_REFCTRL);
+	if (tegra->dram_type == DRAM_TYPE_DDR3)
+		emc_ccfifo_writel(tegra,
+				  EMC_DRAM_DEV_SEL(tegra->dram_num)
+					| EMC_SELF_REF_CMD_ENABLED,
+				  EMC_SELF_REF);
+
+	/* Flow control marker */
+
+	emc_ccfifo_writel(tegra, 1, EMC_STALL_THEN_EXE_AFTER_CLKCHANGE);
+
+	/* DDR3: Exit self-refresh */
+
+	if (tegra->dram_type == DRAM_TYPE_DDR3)
+		emc_ccfifo_writel(tegra,
+				  EMC_DRAM_DEV_SEL(tegra->dram_num),
+				  EMC_SELF_REF);
+	emc_ccfifo_writel(tegra,
+			  EMC_REFCTRL_DEV_SEL(tegra->dram_num)
+				| EMC_REFCTRL_ENABLE,
+			  EMC_REFCTRL);
+
+	/* Set DRAM mode registers */
+
+	if (tegra->dram_type == DRAM_TYPE_DDR3) {
+		if (timing->emc_mode_1 != tegra->last_timing.emc_mode_1)
+			emc_ccfifo_writel(tegra, timing->emc_mode_1, EMC_EMRS);
+		if (timing->emc_mode_2 != tegra->last_timing.emc_mode_2)
+			emc_ccfifo_writel(tegra, timing->emc_mode_2, EMC_EMRS2);
+
+		if ((timing->emc_mode_reset !=
+		     tegra->last_timing.emc_mode_reset) ||
+		    dll_change == DLL_CHANGE_ON) {
+			val = timing->emc_mode_reset;
+			if (dll_change == DLL_CHANGE_ON) {
+				val |= EMC_MODE_SET_DLL_RESET;
+				val |= EMC_MODE_SET_LONG_CNT;
+			} else {
+				val &= ~EMC_MODE_SET_DLL_RESET;
+			}
+			emc_ccfifo_writel(tegra, val, EMC_MRS);
+		}
+	} else {
+		if (timing->emc_mode_2 != tegra->last_timing.emc_mode_2)
+			emc_ccfifo_writel(tegra, timing->emc_mode_2, EMC_MRW2);
+		if (timing->emc_mode_1 != tegra->last_timing.emc_mode_1)
+			emc_ccfifo_writel(tegra, timing->emc_mode_1, EMC_MRW);
+		if (timing->emc_mode_4 != tegra->last_timing.emc_mode_4)
+			emc_ccfifo_writel(tegra, timing->emc_mode_4, EMC_MRW4);
+	}
+
+	/*  Issue ZCAL command if turning ZCAL on */
+
+	if (timing->emc_zcal_interval != 0 &&
+	    tegra->last_timing.emc_zcal_interval == 0) {
+		emc_ccfifo_writel(tegra, EMC_ZQ_CAL_LONG_CMD_DEV0, EMC_ZQ_CAL);
+		if (tegra->dram_num > 1)
+			emc_ccfifo_writel(tegra, EMC_ZQ_CAL_LONG_CMD_DEV1,
+					  EMC_ZQ_CAL);
+	}
+
+	/*  Write to RO register to remove stall after change */
+
+	emc_ccfifo_writel(tegra, 0, EMC_CCFIFO_STATUS);
+
+	if (timing->emc_cfg_2 & EMC_CFG_2_DIS_STP_OB_CLK_DURING_NON_WR)
+		emc_ccfifo_writel(tegra, timing->emc_cfg_2, EMC_CFG_2);
+
+	/* Disable AUTO_CAL for clock change */
+
+	emc_seq_disable_auto_cal(tegra);
+
+	/* Read MC register to wait until programming has settled */
+
+	readl(tegra->mc_regs + MC_EMEM_ADR_CFG);
+	readl(tegra->emc_regs + EMC_INTSTATUS);
+
+	/* Program new parent and divisor. This triggers the EMC state machine
+	 * to change timings. */
+
+	writel(car_value, tegra->clk_regs + CLK_SOURCE_EMC);
+	readl(tegra->clk_regs + CLK_SOURCE_EMC);
+
+	/* Wait until the state machine has settled */
+
+	emc_seq_wait_clkchange(tegra);
+
+	/* Restore AUTO_CAL */
+
+	if (timing->emc_ctt_term_ctrl != tegra->last_timing.emc_ctt_term_ctrl)
+		writel(timing->emc_auto_cal_interval,
+		       tegra->emc_regs + EMC_AUTO_CAL_INTERVAL);
+
+	/* Restore dynamic self-refresh */
+
+	if (timing->emc_cfg & EMC_CFG_PWR_MASK)
+		writel(timing->emc_cfg, tegra->emc_regs + EMC_CFG);
+
+	/* Set ZCAL wait count */
+
+	writel(timing->emc_zcal_cnt_long, tegra->emc_regs + EMC_ZCAL_WAIT_CNT);
+
+	/* LPDDR3: Turn off BGBIAS if low frequency */
+
+	if (tegra->dram_type == DRAM_TYPE_LPDDR3 &&
+	    timing->emc_bgbias_ctl0 &
+	      EMC_BGBIAS_CTL0_BIAS0_DSC_E_PWRD_IBIAS_RX) {
+		val = timing->emc_bgbias_ctl0;
+		val |= EMC_BGBIAS_CTL0_BIAS0_DSC_E_PWRD_IBIAS_VTTGEN;
+		val |= EMC_BGBIAS_CTL0_BIAS0_DSC_E_PWRD;
+		writel(val, tegra->emc_regs + EMC_BGBIAS_CTL0);
+	} else {
+		if (tegra->dram_type == DRAM_TYPE_DDR3 &&
+		    readl(tegra->emc_regs + EMC_BGBIAS_CTL0) !=
+		      timing->emc_bgbias_ctl0) {
+			writel(timing->emc_bgbias_ctl0,
+			       tegra->emc_regs + EMC_BGBIAS_CTL0);
+		}
+
+		writel(timing->emc_auto_cal_interval,
+		       tegra->emc_regs + EMC_AUTO_CAL_INTERVAL);
+	}
+
+	/* Wait for timing to settle */
+
+	udelay(2);
+
+	/* Reprogram SEL_DPD_CTRL */
+
+	writel(timing->emc_sel_dpd_ctrl, tegra->emc_regs + EMC_SEL_DPD_CTRL);
+	emc_seq_update_timing(tegra);
+}
+
+/* * * * * * * * * * * * * * * * * * * * * * * * * *
+ * Common clock framework callback implementations *
+ * * * * * * * * * * * * * * * * * * * * * * * * * */
+
+unsigned long emc_recalc_rate(struct clk_hw *hw, unsigned long parent_rate)
+{
+	struct tegra_emc *tegra = container_of(hw, struct tegra_emc, hw);
+	u32 val;
+	u32 div;
+
+	/* CCF wrongly assumes that the parent won't change during set_rate,
+	 * so get the parent rate explicitly. */
+	parent_rate = __clk_get_rate(__clk_get_parent(hw->clk));
+
+	val = readl(tegra->clk_regs + CLK_SOURCE_EMC);
+	div = val & CLK_SOURCE_EMC_EMC_2X_CLK_DIVISOR_MASK;
+
+	return parent_rate / (div + 2) * 2;
+}
+
+/* Rounds up unless no higher rate exists, in which case down. This way is
+ * safer since things have EMC rate floors. Also don't touch parent_rate
+ * since we don't want the CCF to play with our parent clocks. */
+long emc_round_rate(struct clk_hw *hw, unsigned long rate,
+		    unsigned long *parent_rate)
+{
+	struct tegra_emc *tegra = container_of(hw, struct tegra_emc, hw);
+	int i;
+
+	/* Returning the original rate will lead to a more sensible error
+	 * message when emc_set_rate fails. */
+	if (tegra->num_timings == 0)
+		return rate;
+
+	for (i = 0; i < tegra->num_timings; ++i) {
+		struct emc_timing *timing = tegra->timings + i;
+
+		if (timing->rate >= rate)
+			return timing->rate;
+	}
+
+	return tegra->timings[tegra->num_timings - 1].rate;
+}
+
+u8 emc_get_parent(struct clk_hw *hw)
+{
+	struct tegra_emc *tegra = container_of(hw, struct tegra_emc, hw);
+	u32 val;
+
+	val = readl(tegra->clk_regs + CLK_SOURCE_EMC);
+
+	return (val >> CLK_SOURCE_EMC_EMC_2X_CLK_SRC_SHIFT)
+		& CLK_SOURCE_EMC_EMC_2X_CLK_SRC_MASK;
+}
+
+static int emc_set_timing(struct tegra_emc *tegra, struct emc_timing *timing)
+{
+	int err;
+	u8 div;
+	u32 car_value;
+	unsigned long parent_rate;
+
+	dev_dbg(&tegra->pdev->dev, "going to rate %ld prate %ld p %s\n",
+		timing->rate, timing->parent_rate,
+		__clk_get_name(timing->parent));
+
+	if (emc_get_parent(&tegra->hw) == timing->parent_index &&
+	    clk_get_rate(timing->parent) != timing->parent_rate) {
+		BUG();
+		return -EINVAL;
+	}
+
+	tegra->changing_timing = true;
+
+	parent_rate = timing->parent_rate;
+	div = parent_rate / (timing->rate / 2) - 2;
+
+	car_value = 0;
+	car_value |= timing->parent_index <<
+		CLK_SOURCE_EMC_EMC_2X_CLK_SRC_SHIFT;
+	car_value |= div << CLK_SOURCE_EMC_EMC_2X_CLK_DIVISOR_SHIFT;
+
+	err = clk_set_rate(timing->parent, timing->parent_rate);
+	if (err) {
+		dev_err(&tegra->pdev->dev,
+			"cannot change parent %s rate to %ld: %d\n",
+			__clk_get_name(timing->parent),
+			timing->parent_rate, err);
+
+		return err;
+	}
+
+	err = clk_prepare_enable(timing->parent);
+	if (err) {
+		dev_err(&tegra->pdev->dev,
+			"cannot enable parent clock: %d\n", err);
+		return err;
+	}
+
+	emc_change_timing(tegra, timing, car_value);
+
+	__clk_reparent(tegra->hw.clk, timing->parent);
+	clk_disable_unprepare(tegra->prev_parent);
+
+	tegra->last_timing = *timing;
+	tegra->prev_parent = timing->parent;
+	tegra->changing_timing = false;
+
+	return 0;
+}
+
+/* Get backup timing to use as an intermediate step when a change between
+ * two timings with the same clock source has been requested. First try to
+ * find a timing with a higher clock rate to avoid a rate below any set rate
+ * floors. If that is not possible, find a lower rate. */
+static struct emc_timing *get_backup_timing(struct tegra_emc *tegra,
+					    int timing_index)
+{
+	int i;
+
+	for (i = timing_index+1; i < tegra->num_timings; ++i) {
+		struct emc_timing *timing = tegra->timings + i;
+
+		if (emc_parent_clk_sources[timing->parent_index] !=
+		    emc_parent_clk_sources[
+		      tegra->timings[timing_index].parent_index])
+			return timing;
+	}
+
+	for (i = timing_index-1; i >= 0; --i) {
+		struct emc_timing *timing = tegra->timings + i;
+
+		if (emc_parent_clk_sources[timing->parent_index] !=
+		    emc_parent_clk_sources[
+		      tegra->timings[timing_index].parent_index])
+			return timing;
+	}
+
+	return NULL;
+}
+
+int emc_set_rate(struct clk_hw *hw, unsigned long rate,
+		 unsigned long parent_rate)
+{
+	struct tegra_emc *tegra = container_of(hw, struct tegra_emc, hw);
+	struct emc_timing *timing = NULL;
+	int i, err;
+
+	/* When emc_set_timing changes the parent rate, CCF will propagate
+	 * that downward to us, so ignore any set_rate calls while a rate
+	 * change is already going on. */
+	if (tegra->changing_timing)
+		return 0;
+
+	for (i = 0; i < tegra->num_timings; ++i) {
+		if (tegra->timings[i].rate == rate) {
+			timing = tegra->timings + i;
+			break;
+		}
+	}
+
+	if (!timing) {
+		dev_err(&tegra->pdev->dev,
+			"cannot switch to rate %ld without emc table\n", rate);
+		return -EINVAL;
+	}
+
+	if (emc_parent_clk_sources[emc_get_parent(hw)] ==
+	    emc_parent_clk_sources[timing->parent_index] &&
+	    clk_get_rate(timing->parent) != timing->parent_rate) {
+		/* Parent clock source not changed but parent rate has changed,
+		 * need to temporarily switch to another parent */
+
+		struct emc_timing *backup_timing;
+
+		backup_timing = get_backup_timing(tegra, i);
+		if (!backup_timing) {
+			dev_err(&tegra->pdev->dev,
+				"cannot find backup timing\n");
+			return -EINVAL;
+		}
+
+		dev_dbg(&tegra->pdev->dev,
+			"using %ld as backup rate when going to %ld\n",
+			backup_timing->rate, rate);
+
+		err = emc_set_timing(tegra, backup_timing);
+		if (err) {
+			dev_err(&tegra->pdev->dev,
+				"cannot set backup timing: %d\n",
+				err);
+			return err;
+		}
+	}
+
+	return emc_set_timing(tegra, timing);
+}
+
+/* * * * * * * * * * * * * * * * * * * * * * * * * *
+ * Debugfs entry                                   *
+ * * * * * * * * * * * * * * * * * * * * * * * * * */
+
+static int emc_debug_rate_get(void *data, u64 *rate)
+{
+	struct tegra_emc *tegra = data;
+
+	*rate = clk_get_rate(tegra->hw.clk);
+
+	return 0;
+}
+
+static int emc_debug_rate_set(void *data, u64 rate)
+{
+	struct tegra_emc *tegra = data;
+
+	return clk_set_rate(tegra->hw.clk, rate);
+}
+
+DEFINE_SIMPLE_ATTRIBUTE(emc_debug_rate_fops, emc_debug_rate_get,
+			emc_debug_rate_set, "%lld\n");
+
+const struct clk_ops tegra_clk_emc_ops = {
+	.recalc_rate = emc_recalc_rate,
+	.round_rate = emc_round_rate,
+	.set_rate = emc_set_rate,
+	.get_parent = emc_get_parent,
+};
+
+void emc_debugfs_init(struct tegra_emc *tegra)
+{
+	struct dentry *d;
+
+	d = debugfs_create_file("emc_rate", S_IRUGO | S_IWUSR, NULL, tegra,
+				&emc_debug_rate_fops);
+	if (!d)
+		dev_warn(&tegra->pdev->dev,
+			 "failed to create debugfs entries\n");
+}
+
+/* * * * * * * * * * * * * * * * * * * * * * * * * *
+ * Initialization and deinitialization             *
+ * * * * * * * * * * * * * * * * * * * * * * * * * */
+
+static void emc_read_current_timing(struct tegra_emc *tegra,
+				    struct emc_timing *timing)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(t124_emc_burst_regs); ++i)
+		timing->emc_burst_data[i] =
+			readl(tegra->emc_regs + t124_emc_burst_regs[i]);
+
+	for (i = 0; i < ARRAY_SIZE(t124_mc_burst_regs); ++i)
+		timing->mc_burst_data[i] =
+			readl(tegra->mc_regs + t124_mc_burst_regs[i]);
+
+	timing->rate = clk_get_rate(tegra->hw.clk);
+	timing->emc_cfg = readl(tegra->emc_regs + EMC_CFG);
+
+	timing->emc_auto_cal_interval = 0;
+	timing->emc_zcal_cnt_long = 0;
+	timing->emc_mode_1 = 0;
+	timing->emc_mode_2 = 0;
+	timing->emc_mode_4 = 0;
+	timing->emc_mode_reset = 0;
+}
+
+static void emc_init(struct tegra_emc *tegra)
+{
+	tegra->dram_type = readl(tegra->emc_regs + EMC_FBIO_CFG5)
+		& EMC_FBIO_CFG5_DRAM_TYPE_MASK >> EMC_FBIO_CFG5_DRAM_TYPE_SHIFT;
+	tegra->dram_num = (readl(tegra->mc_regs + MC_EMEM_ADR_CFG)
+		& MC_EMEM_ADR_CFG_EMEM_NUMDEV) + 1;
+
+	emc_read_current_timing(tegra, &tegra->last_timing);
+
+	tegra->prev_parent = clk_get_parent_by_index(
+		tegra->hw.clk, emc_get_parent(&tegra->hw));
+	tegra->changing_timing = false;
+}
+
+static int load_one_timing_from_dt(struct tegra_emc *tegra,
+				   struct emc_timing *timing,
+				   struct device_node *node)
+{
+	int err, i;
+	u32 tmp;
+
+	err = of_property_read_u32(node, "clock-frequency", &tmp);
+	if (err) {
+		dev_err(&tegra->pdev->dev,
+			"timing %s: failed to read rate\n", node->name);
+		return err;
+	}
+
+	timing->rate = tmp;
+
+	err = of_property_read_u32(node, "nvidia,parent-clock-frequency", &tmp);
+	if (err) {
+		dev_err(&tegra->pdev->dev,
+			"timing %s: failed to read parent rate\n", node->name);
+		return err;
+	}
+
+	timing->parent_rate = tmp;
+
+	err = of_property_read_u32_array(node, "nvidia,emc-burst-data",
+					 timing->emc_burst_data,
+					 ARRAY_SIZE(timing->emc_burst_data));
+	if (err) {
+		dev_err(&tegra->pdev->dev,
+			"timing %s: failed to read emc burst data\n",
+			node->name);
+		return err;
+	}
+
+	err = of_property_read_u32_array(node, "nvidia,mc-burst-data",
+					 timing->mc_burst_data,
+					 ARRAY_SIZE(timing->mc_burst_data));
+	if (err) {
+		dev_err(&tegra->pdev->dev,
+			"timing %s: failed to read mc burst data\n",
+			node->name);
+		return err;
+	}
+
+#define EMC_READ_PROP(prop, dtprop) { \
+	err = of_property_read_u32(node, dtprop, &timing->prop); \
+	if (err) { \
+		dev_err(&tegra->pdev->dev, \
+			"timing %s: failed to read " #prop "\n", \
+			node->name); \
+		return err; \
+	} \
+}
+
+	EMC_READ_PROP(emc_zcal_cnt_long, "nvidia,emc-zcal-cnt-long")
+	EMC_READ_PROP(emc_auto_cal_interval, "nvidia,emc-auto-cal-interval")
+	EMC_READ_PROP(emc_ctt_term_ctrl, "nvidia,emc-ctt-term-ctrl")
+	EMC_READ_PROP(emc_cfg, "nvidia,emc-cfg")
+	EMC_READ_PROP(emc_cfg_2, "nvidia,emc-cfg-2")
+	EMC_READ_PROP(emc_sel_dpd_ctrl, "nvidia,emc-sel-dpd-ctrl")
+	EMC_READ_PROP(emc_bgbias_ctl0, "nvidia,emc-bgbias-ctl0")
+	EMC_READ_PROP(emc_auto_cal_config2, "nvidia,emc-auto-cal-config2")
+	EMC_READ_PROP(emc_auto_cal_config3, "nvidia,emc-auto-cal-config3")
+	EMC_READ_PROP(emc_auto_cal_config, "nvidia,emc-auto-cal-config")
+	EMC_READ_PROP(emc_mode_reset, "nvidia,emc-mode-reset")
+	EMC_READ_PROP(emc_mode_1, "nvidia,emc-mode-1")
+	EMC_READ_PROP(emc_mode_2, "nvidia,emc-mode-2")
+	EMC_READ_PROP(emc_mode_4, "nvidia,emc-mode-4")
+
+#undef EMC_READ_PROP
+
+	timing->parent = of_clk_get_by_name(node, "emc-parent");
+	if (IS_ERR(timing->parent)) {
+		dev_err(&tegra->pdev->dev,
+			"timing %s: failed to get parent clock\n",
+			node->name);
+		return PTR_ERR(timing->parent);
+	}
+
+	timing->parent_index = 0xff;
+	for (i = 0; i < ARRAY_SIZE(emc_parent_clk_names); ++i) {
+		if (!strcmp(emc_parent_clk_names[i],
+			    __clk_get_name(timing->parent))) {
+			timing->parent_index = i;
+			break;
+		}
+	}
+	if (timing->parent_index == 0xff) {
+		dev_err(&tegra->pdev->dev,
+			"timing %s: %s is not a valid parent\n",
+			node->name, __clk_get_name(timing->parent));
+		clk_put(timing->parent);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int load_timings_from_dt(struct tegra_emc *tegra,
+				struct device_node *node)
+{
+	struct device_node *child;
+	int child_count = of_get_child_count(node);
+	unsigned long prev_rate = 0;
+	int i = 0, err;
+
+	tegra->timings = devm_kzalloc(&tegra->pdev->dev,
+				      sizeof(struct emc_timing) * child_count,
+				      GFP_KERNEL);
+	if (!tegra->timings)
+		return -ENOMEM;
+
+	tegra->num_timings = child_count;
+
+	for_each_child_of_node(node, child) {
+		struct emc_timing *timing = tegra->timings + (i++);
+
+		err = load_one_timing_from_dt(tegra, timing, child);
+		if (err)
+			return err;
+
+		if (timing->rate <= prev_rate) {
+			dev_err(&tegra->pdev->dev,
+				"timing %s: rate not increasing\n",
+				child->name);
+			clk_put(timing->parent);
+			return -EINVAL;
+		}
+
+		prev_rate = timing->rate;
+	}
+
+	return 0;
+}
+
+static void unload_timings(struct tegra_emc *tegra)
+{
+	int i;
+
+	for (i = 0; i < tegra->num_timings; ++i)
+		clk_put(tegra->timings[i].parent);
+}
+
+static int tegra_emc_remove(struct platform_device *pdev)
+{
+	struct tegra_emc *tegra = platform_get_drvdata(pdev);
+
+	unload_timings(tegra);
+
+	return 0;
+}
+
+static const struct of_device_id tegra_emc_of_match[] = {
+	{ .compatible = "nvidia,tegra124-emc" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, tegra_emc_of_match);
+
+static const struct of_device_id tegra_car_of_match[] = {
+	{ .compatible = "nvidia,tegra124-car" },
+	{}
+};
+
+static const struct of_device_id tegra_mc_of_match[] = {
+	{ .compatible = "nvidia,tegra124-mc" },
+	{}
+};
+
+static int tegra_emc_probe(struct platform_device *pdev)
+{
+	struct tegra_emc *tegra;
+	struct clk *clk;
+	struct clk_init_data init;
+	struct device_node *node;
+	struct resource node_res;
+	struct resource *res;
+	int err;
+
+	tegra = devm_kzalloc(&pdev->dev, sizeof(*tegra), GFP_KERNEL);
+	if (!tegra)
+		return -ENOMEM;
+
+	tegra->pdev = pdev;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	tegra->emc_regs = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(tegra->emc_regs)) {
+		dev_err(&pdev->dev, "failed to map EMC regs\n");
+		return PTR_ERR(tegra->emc_regs);
+	}
+
+	node = of_find_matching_node(NULL, tegra_car_of_match);
+	err = of_address_to_resource(node, 0, &node_res);
+	if (err)
+		dev_err(&pdev->dev, "failed to get CAR registers\n");
+
+	tegra->clk_regs = devm_ioremap(&pdev->dev, node_res.start,
+				       resource_size(&node_res));
+	if (!tegra->clk_regs) {
+		dev_err(&pdev->dev, "could not map CAR registers\n");
+		return -ENOMEM;
+	}
+
+	/* First try to get MC registers from the MC device tree node.
+	 * If that doesn't exist, try to get them from the second register
+	 * range in our node. */
+
+	node = of_find_matching_node(NULL, tegra_mc_of_match);
+	err = of_address_to_resource(node, 0, &node_res);
+	if (err) {
+		res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+		tegra->mc_regs = devm_ioremap_resource(&pdev->dev, res);
+		if (IS_ERR(tegra->mc_regs)) {
+			dev_err(&pdev->dev, "failed to get MC registers\n");
+			return PTR_ERR(tegra->mc_regs);
+		}
+	} else {
+		tegra->mc_regs = devm_ioremap(&pdev->dev, node_res.start,
+					resource_size(&node_res));
+		if (!tegra->mc_regs) {
+			dev_err(&pdev->dev, "could not map MC registers\n");
+			return -ENOMEM;
+		}
+	}
+
+	node = of_get_child_by_name(pdev->dev.of_node, "timings");
+	if (node) {
+		err = load_timings_from_dt(tegra, node);
+		if (err)
+			return err;
+	} else {
+		tegra->num_timings = 0;
+	}
+
+	if (tegra->num_timings == 0)
+		dev_warn(&pdev->dev, "no memory timings registered\n");
+
+	init.name = "emc";
+	init.ops = &tegra_clk_emc_ops;
+	init.flags = 0;
+	init.parent_names = emc_parent_clk_names;
+	init.num_parents = ARRAY_SIZE(emc_parent_clk_names);
+
+	tegra->hw.init = &init;
+
+	clk = devm_clk_register(&pdev->dev, &tegra->hw);
+	if (IS_ERR(clk)) {
+		unload_timings(tegra);
+		return PTR_ERR(clk);
+	}
+
+	/* Allow debugging tools to see the EMC clock */
+	clk_register_clkdev(clk, "emc", "tegra-clk-debug");
+
+	clk_prepare_enable(clk);
+
+	platform_set_drvdata(pdev, tegra);
+
+	emc_init(tegra);
+	emc_debugfs_init(tegra);
+
+	return 0;
+};
+
+static struct platform_driver tegra_emc_driver = {
+	.probe = tegra_emc_probe,
+	.remove = tegra_emc_remove,
+	.driver = {
+		.name = "tegra-emc",
+		.of_match_table = tegra_emc_of_match,
+	},
+};
+module_platform_driver(tegra_emc_driver);
+
+MODULE_AUTHOR("Mikko Perttunen <mperttunen@nvidia.com>");
+MODULE_DESCRIPTION("Tegra124 EMC driver");
+MODULE_LICENSE("GPL v2");
-- 
1.8.1.5

^ permalink raw reply related	[flat|nested] 53+ messages in thread

* [PATCH 5/8] of: Add Tegra124 EMC bindings
  2014-07-11 14:18 ` [PATCH 5/8] of: Add Tegra124 EMC bindings Mikko Perttunen
@ 2014-07-11 14:51   ` Thierry Reding
  2014-07-11 16:01     ` Mikko Perttunen
  2014-07-11 16:43   ` Andrew Bresticker
  2014-07-21 22:36   ` Stephen Warren
  2 siblings, 1 reply; 53+ messages in thread
From: Thierry Reding @ 2014-07-11 14:51 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Jul 11, 2014 at 05:18:30PM +0300, Mikko Perttunen wrote:
> Add binding documentation for the nvidia,tegra124-emc device tree
> node.
> 
> Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
> ---
>  .../bindings/memory-controllers/tegra-emc.txt      | 42 ++++++++++++++++++++++
>  1 file changed, 42 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/memory-controllers/tegra-emc.txt
> 
> diff --git a/Documentation/devicetree/bindings/memory-controllers/tegra-emc.txt b/Documentation/devicetree/bindings/memory-controllers/tegra-emc.txt
> new file mode 100644
> index 0000000..2dde17e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/memory-controllers/tegra-emc.txt
> @@ -0,0 +1,42 @@
> +Tegra124 SoC EMC controller
> +
> +Required properties :
> +- compatible : "nvidia,tegra124-emc".
> +- reg : Should contain 1 or 2 entries:
> +  - EMC register set
> +  - MC register set : Required only if no node with
> +    'compatible = "nvidia,tegra124-mc"' exists. The MC register set
> +    is first read from the MC node. If it doesn't exist, it is read
> +    from this property.

No can do. Memory regions shouldn't be shared between drivers like this.
It makes it impossible to ensure that they don't stump on each others'
toes.

One possibility to make this work is to export global functions from the
memory controller driver that this driver can call into. Perhaps if you
want you can be extra explicit by linking them in DT, like this:

	mc: memory-controller at 0,70019000 {
		compatible = "nvidia,tegra124-mc";
		reg = <0x0 0x70019000 0x0 0x00001000>;
		...
	};

	memory-controller at 0,7001b000 {
		compatible = "nvidia,tegra124-emc";
		reg = <0x0 0x7001b000 0x0 0x00001000>;
		memory-controller = <&mc>;
		...
	};

But I think it's safe enough to assume that there will only be a single
memory controller/EMC pair in the device.

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140711/c8076ea6/attachment.sig>

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH 5/8] of: Add Tegra124 EMC bindings
  2014-07-11 14:51   ` Thierry Reding
@ 2014-07-11 16:01     ` Mikko Perttunen
  2014-07-14  7:55       ` Mikko Perttunen
  0 siblings, 1 reply; 53+ messages in thread
From: Mikko Perttunen @ 2014-07-11 16:01 UTC (permalink / raw)
  To: linux-arm-kernel

On 07/11/2014 05:51 PM, Thierry Reding wrote:
> On Fri, Jul 11, 2014 at 05:18:30PM +0300, Mikko Perttunen wrote:
>> Add binding documentation for the nvidia,tegra124-emc device tree
>> node.
>>
>> Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
>> ---
>>   .../bindings/memory-controllers/tegra-emc.txt      | 42 ++++++++++++++++++++++
>>   1 file changed, 42 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/memory-controllers/tegra-emc.txt
>>
>> diff --git a/Documentation/devicetree/bindings/memory-controllers/tegra-emc.txt b/Documentation/devicetree/bindings/memory-controllers/tegra-emc.txt
>> new file mode 100644
>> index 0000000..2dde17e
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/memory-controllers/tegra-emc.txt
>> @@ -0,0 +1,42 @@
>> +Tegra124 SoC EMC controller
>> +
>> +Required properties :
>> +- compatible : "nvidia,tegra124-emc".
>> +- reg : Should contain 1 or 2 entries:
>> +  - EMC register set
>> +  - MC register set : Required only if no node with
>> +    'compatible = "nvidia,tegra124-mc"' exists. The MC register set
>> +    is first read from the MC node. If it doesn't exist, it is read
>> +    from this property.
>
> No can do. Memory regions shouldn't be shared between drivers like this.
> It makes it impossible to ensure that they don't stump on each others'
> toes.

In this case, all the registers that will be written are such that the 
MC driver will never need to write them. They are shadowed registers, 
meaning that all writes are stored and are only effective starting from 
the next time the EMC rate change state machine is activated, so writing 
them from anywhere except than the EMC driver would be pointless.

I can find two users of these registers in downstream:
1) mc.c saves and loads them on suspend/restore (I don't know why, that 
shouldn't do anything. They will be overridden anyway during the next 
EMC rate change).
2) tegra12x_la.c reads MC_EMEM_ARB_MISC0 during a core_initcall to 
calculate a value which it then writes to a register that is also 
shadowed and that is part of downstream burst registers so that doesn't 
do anything either.

The reason I implemented two ways to specify the MC register area was 
that this could be merged before an MC driver and retain 
backwards-compatibility after the MC driver arrives.

If this is not acceptable, we can certainly wait for the MC driver to be 
merged first. (Although with the general rate of things, I hope I won't 
be back at school at that point..) I assume that this is blocked on the 
IOMMU bindings discussion? In that case, there are several options: the 
MC driver could have its own tables for each EMC rate or we could just 
make the EMC tables global (i.e. not under the EMC node). In any case, 
the MC driver would need to implement a function that would just write 
these values but be guaranteed to not do anything else, since that could 
cause nasty things during the EMC rate change sequence.

Yet another option is to just not write to these registers at all. In my 
tests, that would entail a 20-25% penalty to memory throughput for most 
timings.

>
> One possibility to make this work is to export global functions from the
> memory controller driver that this driver can call into. Perhaps if you
> want you can be extra explicit by linking them in DT, like this:
>
> 	mc: memory-controller at 0,70019000 {
> 		compatible = "nvidia,tegra124-mc";
> 		reg = <0x0 0x70019000 0x0 0x00001000>;
> 		...
> 	};
>
> 	memory-controller at 0,7001b000 {
> 		compatible = "nvidia,tegra124-emc";
> 		reg = <0x0 0x7001b000 0x0 0x00001000>;
> 		memory-controller = <&mc>;
> 		...
> 	};
>
> But I think it's safe enough to assume that there will only be a single
> memory controller/EMC pair in the device.
>
> Thierry
>

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH 5/8] of: Add Tegra124 EMC bindings
  2014-07-11 14:18 ` [PATCH 5/8] of: Add Tegra124 EMC bindings Mikko Perttunen
  2014-07-11 14:51   ` Thierry Reding
@ 2014-07-11 16:43   ` Andrew Bresticker
  2014-07-11 16:48     ` Mikko Perttunen
  2014-07-21 21:28     ` Stephen Warren
  2014-07-21 22:36   ` Stephen Warren
  2 siblings, 2 replies; 53+ messages in thread
From: Andrew Bresticker @ 2014-07-11 16:43 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Jul 11, 2014 at 7:18 AM, Mikko Perttunen <mperttunen@nvidia.com> wrote:
> Add binding documentation for the nvidia,tegra124-emc device tree
> node.

> diff --git a/Documentation/devicetree/bindings/memory-controllers/tegra-emc.txt b/Documentation/devicetree/bindings/memory-controllers/tegra-emc.txt

> +Required properties :
> +- compatible : "nvidia,tegra124-emc".
> +- reg : Should contain 1 or 2 entries:
> +  - EMC register set
> +  - MC register set : Required only if no node with
> +    'compatible = "nvidia,tegra124-mc"' exists. The MC register set
> +    is first read from the MC node. If it doesn't exist, it is read
> +    from this property.
> +- timings : Should contain 1 entry for each supported clock rate.
> +  Entries should be named "timing at n" where n is a 0-based increasing
> +  number. The timings must be listed in rate-ascending order.

There are upcoming boards which support multiple DRAM configurations
and require a separate set of timings for each configuration.  Could
we instead have multiple sets of timings with the proper one selected
at runtime by RAM code, as reported by PMC_STRAPPING_OPT_A_0?
Something like:

emc {
        emc-table at 0 {
                nvidia,ram-code = <0>;
                timing at 0 {
                        ...
                };
                ...
        };
        emc-table at 1 {
                nvidia,ram-code = <4>;
                timing at 0 {
                        ...
                };
                ...
        };
        ...
};

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH 5/8] of: Add Tegra124 EMC bindings
  2014-07-11 16:43   ` Andrew Bresticker
@ 2014-07-11 16:48     ` Mikko Perttunen
  2014-07-21 21:28     ` Stephen Warren
  1 sibling, 0 replies; 53+ messages in thread
From: Mikko Perttunen @ 2014-07-11 16:48 UTC (permalink / raw)
  To: linux-arm-kernel

Sure, I'll do that for v2.

On 07/11/2014 07:43 PM, Andrew Bresticker wrote:
> On Fri, Jul 11, 2014 at 7:18 AM, Mikko Perttunen <mperttunen@nvidia.com> wrote:
>> Add binding documentation for the nvidia,tegra124-emc device tree
>> node.
>
>> diff --git a/Documentation/devicetree/bindings/memory-controllers/tegra-emc.txt b/Documentation/devicetree/bindings/memory-controllers/tegra-emc.txt
>
>> +Required properties :
>> +- compatible : "nvidia,tegra124-emc".
>> +- reg : Should contain 1 or 2 entries:
>> +  - EMC register set
>> +  - MC register set : Required only if no node with
>> +    'compatible = "nvidia,tegra124-mc"' exists. The MC register set
>> +    is first read from the MC node. If it doesn't exist, it is read
>> +    from this property.
>> +- timings : Should contain 1 entry for each supported clock rate.
>> +  Entries should be named "timing at n" where n is a 0-based increasing
>> +  number. The timings must be listed in rate-ascending order.
>
> There are upcoming boards which support multiple DRAM configurations
> and require a separate set of timings for each configuration.  Could
> we instead have multiple sets of timings with the proper one selected
> at runtime by RAM code, as reported by PMC_STRAPPING_OPT_A_0?
> Something like:
>
> emc {
>          emc-table at 0 {
>                  nvidia,ram-code = <0>;
>                  timing at 0 {
>                          ...
>                  };
>                  ...
>          };
>          emc-table at 1 {
>                  nvidia,ram-code = <4>;
>                  timing at 0 {
>                          ...
>                  };
>                  ...
>          };
>          ...
> };
> --
> To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH 5/8] of: Add Tegra124 EMC bindings
  2014-07-11 16:01     ` Mikko Perttunen
@ 2014-07-14  7:55       ` Mikko Perttunen
  2014-07-14  8:15         ` Thierry Reding
  0 siblings, 1 reply; 53+ messages in thread
From: Mikko Perttunen @ 2014-07-14  7:55 UTC (permalink / raw)
  To: linux-arm-kernel

On 11/07/14 19:01, Mikko Perttunen wrote:
> On 07/11/2014 05:51 PM, Thierry Reding wrote:
>> On Fri, Jul 11, 2014 at 05:18:30PM +0300, Mikko Perttunen wrote:
>>> ...
>> ...
>
> In this case, all the registers that will be written are such that the
> MC driver will never need to write them. They are shadowed registers,
> meaning that all writes are stored and are only effective starting from
> the next time the EMC rate change state machine is activated, so writing
> them from anywhere except than the EMC driver would be pointless.
>
> I can find two users of these registers in downstream:
> 1) mc.c saves and loads them on suspend/restore (I don't know why, that
> shouldn't do anything. They will be overridden anyway during the next
> EMC rate change).
> 2) tegra12x_la.c reads MC_EMEM_ARB_MISC0 during a core_initcall to
> calculate a value which it then writes to a register that is also
> shadowed and that is part of downstream burst registers so that doesn't
> do anything either.
>
> The reason I implemented two ways to specify the MC register area was
> that this could be merged before an MC driver and retain
> backwards-compatibility after the MC driver arrives.
>
> If this is not acceptable, we can certainly wait for the MC driver to be
> merged first. (Although with the general rate of things, I hope I won't
> be back at school at that point..) I assume that this is blocked on the
> IOMMU bindings discussion? In that case, there are several options: the
> MC driver could have its own tables for each EMC rate or we could just
> make the EMC tables global (i.e. not under the EMC node). In any case,
> the MC driver would need to implement a function that would just write
> these values but be guaranteed to not do anything else, since that could
> cause nasty things during the EMC rate change sequence.

Having taken another look at the code, I don't think the MC driver could 
do anything that bad. There are also two other places where the EMC 
driver needs to read MC registers: Inside the sequence, it reads a 
register but discards its contents. According to comments, this acts as 
a memory barrier, probably for the preceding step that writes into MC 
memory. If the register writes are moved to the MC driver, it could also 
handle that. In another place it reads the number of RAM modules from a 
MC register. The MC driver could export this as another function.

That said, I still suspect it might not be worth it to divide this 
between two drivers.

- Mikko

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH 5/8] of: Add Tegra124 EMC bindings
  2014-07-14  7:55       ` Mikko Perttunen
@ 2014-07-14  8:15         ` Thierry Reding
  2014-07-14  9:06           ` Mikko Perttunen
  0 siblings, 1 reply; 53+ messages in thread
From: Thierry Reding @ 2014-07-14  8:15 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Jul 14, 2014 at 10:55:51AM +0300, Mikko Perttunen wrote:
> On 11/07/14 19:01, Mikko Perttunen wrote:
> >On 07/11/2014 05:51 PM, Thierry Reding wrote:
> >>On Fri, Jul 11, 2014 at 05:18:30PM +0300, Mikko Perttunen wrote:
> >>>...
> >>...
> >
> >In this case, all the registers that will be written are such that the
> >MC driver will never need to write them. They are shadowed registers,
> >meaning that all writes are stored and are only effective starting from
> >the next time the EMC rate change state machine is activated, so writing
> >them from anywhere except than the EMC driver would be pointless.
> >
> >I can find two users of these registers in downstream:
> >1) mc.c saves and loads them on suspend/restore (I don't know why, that
> >shouldn't do anything. They will be overridden anyway during the next
> >EMC rate change).
> >2) tegra12x_la.c reads MC_EMEM_ARB_MISC0 during a core_initcall to
> >calculate a value which it then writes to a register that is also
> >shadowed and that is part of downstream burst registers so that doesn't
> >do anything either.
> >
> >The reason I implemented two ways to specify the MC register area was
> >that this could be merged before an MC driver and retain
> >backwards-compatibility after the MC driver arrives.
> >
> >If this is not acceptable, we can certainly wait for the MC driver to be
> >merged first. (Although with the general rate of things, I hope I won't
> >be back at school at that point..) I assume that this is blocked on the
> >IOMMU bindings discussion? In that case, there are several options: the
> >MC driver could have its own tables for each EMC rate or we could just
> >make the EMC tables global (i.e. not under the EMC node). In any case,
> >the MC driver would need to implement a function that would just write
> >these values but be guaranteed to not do anything else, since that could
> >cause nasty things during the EMC rate change sequence.
> 
> Having taken another look at the code, I don't think the MC driver could do
> anything that bad. There are also two other places where the EMC driver
> needs to read MC registers: Inside the sequence, it reads a register but
> discards its contents. According to comments, this acts as a memory barrier,
> probably for the preceding step that writes into MC memory. If the register
> writes are moved to the MC driver, it could also handle that. In another
> place it reads the number of RAM modules from a MC register. The MC driver
> could export this as another function.

Exporting this functionality from the MC driver is the right thing to do
in my opinion.

> That said, I still suspect it might not be worth it to divide this between
> two drivers.

If we don't separate, then we make it easy for people to break things in
the future. Both drivers may not be accessing the same registers now,
but if there's no barrier in place to actively enforce this split, then
it's only a matter of time before it gets broken. A typical way to
ensure this is done properly is via request_resource() (called by the
devm_ioremap_resource() function). That will fail if two drivers try to
use the same memory region, and with good reason.

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140714/0dc610fd/attachment-0001.sig>

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH 5/8] of: Add Tegra124 EMC bindings
  2014-07-14  8:15         ` Thierry Reding
@ 2014-07-14  9:06           ` Mikko Perttunen
  2014-07-14  9:31             ` Thierry Reding
  0 siblings, 1 reply; 53+ messages in thread
From: Mikko Perttunen @ 2014-07-14  9:06 UTC (permalink / raw)
  To: linux-arm-kernel

On 14/07/14 11:15, Thierry Reding wrote:
> * PGP Signed by an unknown key
>
> On Mon, Jul 14, 2014 at 10:55:51AM +0300, Mikko Perttunen wrote:
>> On 11/07/14 19:01, Mikko Perttunen wrote:
>>> On 07/11/2014 05:51 PM, Thierry Reding wrote:
>>>> On Fri, Jul 11, 2014 at 05:18:30PM +0300, Mikko Perttunen wrote:
>>>>> ...
>>>> ...
>>>
>>> In this case, all the registers that will be written are such that the
>>> MC driver will never need to write them. They are shadowed registers,
>>> meaning that all writes are stored and are only effective starting from
>>> the next time the EMC rate change state machine is activated, so writing
>>> them from anywhere except than the EMC driver would be pointless.
>>>
>>> I can find two users of these registers in downstream:
>>> 1) mc.c saves and loads them on suspend/restore (I don't know why, that
>>> shouldn't do anything. They will be overridden anyway during the next
>>> EMC rate change).
>>> 2) tegra12x_la.c reads MC_EMEM_ARB_MISC0 during a core_initcall to
>>> calculate a value which it then writes to a register that is also
>>> shadowed and that is part of downstream burst registers so that doesn't
>>> do anything either.
>>>
>>> The reason I implemented two ways to specify the MC register area was
>>> that this could be merged before an MC driver and retain
>>> backwards-compatibility after the MC driver arrives.
>>>
>>> If this is not acceptable, we can certainly wait for the MC driver to be
>>> merged first. (Although with the general rate of things, I hope I won't
>>> be back at school at that point..) I assume that this is blocked on the
>>> IOMMU bindings discussion? In that case, there are several options: the
>>> MC driver could have its own tables for each EMC rate or we could just
>>> make the EMC tables global (i.e. not under the EMC node). In any case,
>>> the MC driver would need to implement a function that would just write
>>> these values but be guaranteed to not do anything else, since that could
>>> cause nasty things during the EMC rate change sequence.
>>
>> Having taken another look at the code, I don't think the MC driver could do
>> anything that bad. There are also two other places where the EMC driver
>> needs to read MC registers: Inside the sequence, it reads a register but
>> discards its contents. According to comments, this acts as a memory barrier,
>> probably for the preceding step that writes into MC memory. If the register
>> writes are moved to the MC driver, it could also handle that. In another
>> place it reads the number of RAM modules from a MC register. The MC driver
>> could export this as another function.
>
> Exporting this functionality from the MC driver is the right thing to do
> in my opinion.

Ok, let's do that then. Do you think I could make a bare-bones MC driver 
to support the EMC driver before your MC driver with IOMMU/LA is ready? 
Can the MC device tree node be stabilized yet? Of course, if things go 
well, that concern might turn out to be unnecessary.

Thanks, Mikko.

>
>> That said, I still suspect it might not be worth it to divide this between
>> two drivers.
>
> If we don't separate, then we make it easy for people to break things in
> the future. Both drivers may not be accessing the same registers now,
> but if there's no barrier in place to actively enforce this split, then
> it's only a matter of time before it gets broken. A typical way to
> ensure this is done properly is via request_resource() (called by the
> devm_ioremap_resource() function). That will fail if two drivers try to
> use the same memory region, and with good reason.
>
> Thierry
>
> * Unknown Key
> * 0x7F3EB3A1
>

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH 5/8] of: Add Tegra124 EMC bindings
  2014-07-14  9:06           ` Mikko Perttunen
@ 2014-07-14  9:31             ` Thierry Reding
  2014-07-14  9:57               ` Mikko Perttunen
  0 siblings, 1 reply; 53+ messages in thread
From: Thierry Reding @ 2014-07-14  9:31 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Jul 14, 2014 at 12:06:32PM +0300, Mikko Perttunen wrote:
> On 14/07/14 11:15, Thierry Reding wrote:
> >* PGP Signed by an unknown key
> >
> >On Mon, Jul 14, 2014 at 10:55:51AM +0300, Mikko Perttunen wrote:
> >>On 11/07/14 19:01, Mikko Perttunen wrote:
> >>>On 07/11/2014 05:51 PM, Thierry Reding wrote:
> >>>>On Fri, Jul 11, 2014 at 05:18:30PM +0300, Mikko Perttunen wrote:
> >>>>>...
> >>>>...
> >>>
> >>>In this case, all the registers that will be written are such that the
> >>>MC driver will never need to write them. They are shadowed registers,
> >>>meaning that all writes are stored and are only effective starting from
> >>>the next time the EMC rate change state machine is activated, so writing
> >>>them from anywhere except than the EMC driver would be pointless.
> >>>
> >>>I can find two users of these registers in downstream:
> >>>1) mc.c saves and loads them on suspend/restore (I don't know why, that
> >>>shouldn't do anything. They will be overridden anyway during the next
> >>>EMC rate change).
> >>>2) tegra12x_la.c reads MC_EMEM_ARB_MISC0 during a core_initcall to
> >>>calculate a value which it then writes to a register that is also
> >>>shadowed and that is part of downstream burst registers so that doesn't
> >>>do anything either.
> >>>
> >>>The reason I implemented two ways to specify the MC register area was
> >>>that this could be merged before an MC driver and retain
> >>>backwards-compatibility after the MC driver arrives.
> >>>
> >>>If this is not acceptable, we can certainly wait for the MC driver to be
> >>>merged first. (Although with the general rate of things, I hope I won't
> >>>be back at school at that point..) I assume that this is blocked on the
> >>>IOMMU bindings discussion? In that case, there are several options: the
> >>>MC driver could have its own tables for each EMC rate or we could just
> >>>make the EMC tables global (i.e. not under the EMC node). In any case,
> >>>the MC driver would need to implement a function that would just write
> >>>these values but be guaranteed to not do anything else, since that could
> >>>cause nasty things during the EMC rate change sequence.
> >>
> >>Having taken another look at the code, I don't think the MC driver could do
> >>anything that bad. There are also two other places where the EMC driver
> >>needs to read MC registers: Inside the sequence, it reads a register but
> >>discards its contents. According to comments, this acts as a memory barrier,
> >>probably for the preceding step that writes into MC memory. If the register
> >>writes are moved to the MC driver, it could also handle that. In another
> >>place it reads the number of RAM modules from a MC register. The MC driver
> >>could export this as another function.
> >
> >Exporting this functionality from the MC driver is the right thing to do
> >in my opinion.
> 
> Ok, let's do that then. Do you think I could make a bare-bones MC driver to
> support the EMC driver before your MC driver with IOMMU/LA is ready? Can the
> MC device tree node be stabilized yet? Of course, if things go well, that
> concern might turn out to be unnecessary.

Well, at this point this isn't 3.17 material anyway, so there's no need
to rush things. I'd prefer to take a patch on top of my proposed MC
driver patch in anticipation of merging that for 3.18. But if it turns
out that for whatever reason we can't do that, having a separate patch
makes it easy to extract the changes into a bare-bones driver.

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140714/ff6d249a/attachment-0001.sig>

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH 5/8] of: Add Tegra124 EMC bindings
  2014-07-14  9:31             ` Thierry Reding
@ 2014-07-14  9:57               ` Mikko Perttunen
  2014-07-14 10:29                 ` Thierry Reding
  0 siblings, 1 reply; 53+ messages in thread
From: Mikko Perttunen @ 2014-07-14  9:57 UTC (permalink / raw)
  To: linux-arm-kernel

On 14/07/14 12:31, Thierry Reding wrote:
> * PGP Signed by an unknown key
>
> On Mon, Jul 14, 2014 at 12:06:32PM +0300, Mikko Perttunen wrote:
>> On 14/07/14 11:15, Thierry Reding wrote:
>>>> Old Signed by an unknown key
>>>
>>> On Mon, Jul 14, 2014 at 10:55:51AM +0300, Mikko Perttunen wrote:
>>>> On 11/07/14 19:01, Mikko Perttunen wrote:
>>>>> On 07/11/2014 05:51 PM, Thierry Reding wrote:
>>>>>> On Fri, Jul 11, 2014 at 05:18:30PM +0300, Mikko Perttunen wrote:
>>>>>>> ...
>>>>>> ...
>>>>>
>>>>> In this case, all the registers that will be written are such that the
>>>>> MC driver will never need to write them. They are shadowed registers,
>>>>> meaning that all writes are stored and are only effective starting from
>>>>> the next time the EMC rate change state machine is activated, so writing
>>>>> them from anywhere except than the EMC driver would be pointless.
>>>>>
>>>>> I can find two users of these registers in downstream:
>>>>> 1) mc.c saves and loads them on suspend/restore (I don't know why, that
>>>>> shouldn't do anything. They will be overridden anyway during the next
>>>>> EMC rate change).
>>>>> 2) tegra12x_la.c reads MC_EMEM_ARB_MISC0 during a core_initcall to
>>>>> calculate a value which it then writes to a register that is also
>>>>> shadowed and that is part of downstream burst registers so that doesn't
>>>>> do anything either.
>>>>>
>>>>> The reason I implemented two ways to specify the MC register area was
>>>>> that this could be merged before an MC driver and retain
>>>>> backwards-compatibility after the MC driver arrives.
>>>>>
>>>>> If this is not acceptable, we can certainly wait for the MC driver to be
>>>>> merged first. (Although with the general rate of things, I hope I won't
>>>>> be back at school at that point..) I assume that this is blocked on the
>>>>> IOMMU bindings discussion? In that case, there are several options: the
>>>>> MC driver could have its own tables for each EMC rate or we could just
>>>>> make the EMC tables global (i.e. not under the EMC node). In any case,
>>>>> the MC driver would need to implement a function that would just write
>>>>> these values but be guaranteed to not do anything else, since that could
>>>>> cause nasty things during the EMC rate change sequence.
>>>>
>>>> Having taken another look at the code, I don't think the MC driver could do
>>>> anything that bad. There are also two other places where the EMC driver
>>>> needs to read MC registers: Inside the sequence, it reads a register but
>>>> discards its contents. According to comments, this acts as a memory barrier,
>>>> probably for the preceding step that writes into MC memory. If the register
>>>> writes are moved to the MC driver, it could also handle that. In another
>>>> place it reads the number of RAM modules from a MC register. The MC driver
>>>> could export this as another function.
>>>
>>> Exporting this functionality from the MC driver is the right thing to do
>>> in my opinion.
>>
>> Ok, let's do that then. Do you think I could make a bare-bones MC driver to
>> support the EMC driver before your MC driver with IOMMU/LA is ready? Can the
>> MC device tree node be stabilized yet? Of course, if things go well, that
>> concern might turn out to be unnecessary.
>
> Well, at this point this isn't 3.17 material anyway, so there's no need
> to rush things.

Very true.

> I'd prefer to take a patch on top of my proposed MC
> driver patch in anticipation of merging that for 3.18. But if it turns
> out that for whatever reason we can't do that, having a separate patch
> makes it easy to extract the changes into a bare-bones driver.

Yes, this sounds sensible. I'll make such a patch. I suppose having 
another timings table in the MC node with just the rate and 
mc-burst-data would separate the concerns best. It occurs to me that we 
could also write the regs in the pre-rate change notifier, but this 
would turn the dependency around and would mean that the regs are not 
written when entering backup rates. The latter shouldn't be a problem 
but the reversed dependency would, so I guess a custom function is the 
way to go, and we need to add at least one anyway.

The downstream kernel also overwrites most LA registers during EMC rate 
change without regard for the driver-set values, and we might have to 
read those values from the device tree too. Upstream can do this in rate 
change notifiers if needed. I'll look into this a bit more.

>
> Thierry
>
> * Unknown Key
> * 0x7F3EB3A1
>

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH 5/8] of: Add Tegra124 EMC bindings
  2014-07-14  9:57               ` Mikko Perttunen
@ 2014-07-14 10:29                 ` Thierry Reding
  2014-07-14 10:54                   ` Mikko Perttunen
  0 siblings, 1 reply; 53+ messages in thread
From: Thierry Reding @ 2014-07-14 10:29 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Jul 14, 2014 at 12:57:26PM +0300, Mikko Perttunen wrote:
> On 14/07/14 12:31, Thierry Reding wrote:
> >* PGP Signed by an unknown key
> >
> >On Mon, Jul 14, 2014 at 12:06:32PM +0300, Mikko Perttunen wrote:
> >>On 14/07/14 11:15, Thierry Reding wrote:
> >>>>Old Signed by an unknown key
> >>>
> >>>On Mon, Jul 14, 2014 at 10:55:51AM +0300, Mikko Perttunen wrote:
> >>>>On 11/07/14 19:01, Mikko Perttunen wrote:
> >>>>>On 07/11/2014 05:51 PM, Thierry Reding wrote:
> >>>>>>On Fri, Jul 11, 2014 at 05:18:30PM +0300, Mikko Perttunen wrote:
> >>>>>>>...
> >>>>>>...
> >>>>>
> >>>>>In this case, all the registers that will be written are such that the
> >>>>>MC driver will never need to write them. They are shadowed registers,
> >>>>>meaning that all writes are stored and are only effective starting from
> >>>>>the next time the EMC rate change state machine is activated, so writing
> >>>>>them from anywhere except than the EMC driver would be pointless.
> >>>>>
> >>>>>I can find two users of these registers in downstream:
> >>>>>1) mc.c saves and loads them on suspend/restore (I don't know why, that
> >>>>>shouldn't do anything. They will be overridden anyway during the next
> >>>>>EMC rate change).
> >>>>>2) tegra12x_la.c reads MC_EMEM_ARB_MISC0 during a core_initcall to
> >>>>>calculate a value which it then writes to a register that is also
> >>>>>shadowed and that is part of downstream burst registers so that doesn't
> >>>>>do anything either.
> >>>>>
> >>>>>The reason I implemented two ways to specify the MC register area was
> >>>>>that this could be merged before an MC driver and retain
> >>>>>backwards-compatibility after the MC driver arrives.
> >>>>>
> >>>>>If this is not acceptable, we can certainly wait for the MC driver to be
> >>>>>merged first. (Although with the general rate of things, I hope I won't
> >>>>>be back at school at that point..) I assume that this is blocked on the
> >>>>>IOMMU bindings discussion? In that case, there are several options: the
> >>>>>MC driver could have its own tables for each EMC rate or we could just
> >>>>>make the EMC tables global (i.e. not under the EMC node). In any case,
> >>>>>the MC driver would need to implement a function that would just write
> >>>>>these values but be guaranteed to not do anything else, since that could
> >>>>>cause nasty things during the EMC rate change sequence.
> >>>>
> >>>>Having taken another look at the code, I don't think the MC driver could do
> >>>>anything that bad. There are also two other places where the EMC driver
> >>>>needs to read MC registers: Inside the sequence, it reads a register but
> >>>>discards its contents. According to comments, this acts as a memory barrier,
> >>>>probably for the preceding step that writes into MC memory. If the register
> >>>>writes are moved to the MC driver, it could also handle that. In another
> >>>>place it reads the number of RAM modules from a MC register. The MC driver
> >>>>could export this as another function.
> >>>
> >>>Exporting this functionality from the MC driver is the right thing to do
> >>>in my opinion.
> >>
> >>Ok, let's do that then. Do you think I could make a bare-bones MC driver to
> >>support the EMC driver before your MC driver with IOMMU/LA is ready? Can the
> >>MC device tree node be stabilized yet? Of course, if things go well, that
> >>concern might turn out to be unnecessary.
> >
> >Well, at this point this isn't 3.17 material anyway, so there's no need
> >to rush things.
> 
> Very true.
> 
> >I'd prefer to take a patch on top of my proposed MC
> >driver patch in anticipation of merging that for 3.18. But if it turns
> >out that for whatever reason we can't do that, having a separate patch
> >makes it easy to extract the changes into a bare-bones driver.
> 
> Yes, this sounds sensible. I'll make such a patch. I suppose having another
> timings table in the MC node with just the rate and mc-burst-data would
> separate the concerns best. It occurs to me that we could also write the
> regs in the pre-rate change notifier, but this would turn the dependency
> around and would mean that the regs are not written when entering backup
> rates. The latter shouldn't be a problem but the reversed dependency would,
> so I guess a custom function is the way to go, and we need to add at least
> one anyway.

It sounds like maybe moving enough code and data into the MC driver to
handle frequency changes would be a good move. From the above it sounds
like all the MC driver needs to know is that a frequency change is about
to happen and what the new frequency is.

In addition to exposing things like number of DRAM banks, etc.

> The downstream kernel also overwrites most LA registers during EMC rate
> change without regard for the driver-set values, and we might have to read
> those values from the device tree too. Upstream can do this in rate change
> notifiers if needed. I'll look into this a bit more.

As I understand it, the latency allowance should be specified in terms
of the maximum amount of time that requests are delayed, so that the
proper values for the LA registers can be recomputed on an EMC rate
change.

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140714/9009e80a/attachment.sig>

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH 5/8] of: Add Tegra124 EMC bindings
  2014-07-14 10:29                 ` Thierry Reding
@ 2014-07-14 10:54                   ` Mikko Perttunen
  2014-07-14 11:10                     ` Thierry Reding
  0 siblings, 1 reply; 53+ messages in thread
From: Mikko Perttunen @ 2014-07-14 10:54 UTC (permalink / raw)
  To: linux-arm-kernel

On 14/07/14 13:29, Thierry Reding wrote:
> * PGP Signed by an unknown key
 > ...
>> Yes, this sounds sensible. I'll make such a patch. I suppose having another
>> timings table in the MC node with just the rate and mc-burst-data would
>> separate the concerns best. It occurs to me that we could also write the
>> regs in the pre-rate change notifier, but this would turn the dependency
>> around and would mean that the regs are not written when entering backup
>> rates. The latter shouldn't be a problem but the reversed dependency would,
>> so I guess a custom function is the way to go, and we need to add at least
>> one anyway.
>
> It sounds like maybe moving enough code and data into the MC driver to
> handle frequency changes would be a good move. From the above it sounds
> like all the MC driver needs to know is that a frequency change is about
> to happen and what the new frequency is.
>
> In addition to exposing things like number of DRAM banks, etc.
>

Yes, so there are two ways to do this:
1) EMC calls tegra_mc_emem_update(freq) at the correct time
2) MC has an optional clock phandle to the EMC clock and registers a 
pre-change notifier.

Both work, but the dependency is reversed. In both cases, the other 
driver is also optional. In the first case, the EMC driver can give a 
warning if the call fails. (As mentioned, if the MC_EMEM updates don't 
happen, things still work but potentially with a hefty perf loss.)
TBH, I haven't yet decided which one is better. If you have an opinion,
I'll go with it.

>> The downstream kernel also overwrites most LA registers during EMC rate
>> change without regard for the driver-set values, and we might have to read
>> those values from the device tree too. Upstream can do this in rate change
>> notifiers if needed. I'll look into this a bit more.
>
> As I understand it, the latency allowance should be specified in terms
> of the maximum amount of time that requests are delayed, so that the
> proper values for the LA registers can be recomputed on an EMC rate
> change.

That's how I understand it too, but in downstream, the LA register 
values are hardcoded into EMC tables in platform data/DTS that are just 
written into the LA registers as-is during rate change.

>
> Thierry
>
> * Unknown Key
> * 0x7F3EB3A1
>

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH 5/8] of: Add Tegra124 EMC bindings
  2014-07-14 10:54                   ` Mikko Perttunen
@ 2014-07-14 11:10                     ` Thierry Reding
  2014-07-14 12:28                       ` Mikko Perttunen
  0 siblings, 1 reply; 53+ messages in thread
From: Thierry Reding @ 2014-07-14 11:10 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Jul 14, 2014 at 01:54:36PM +0300, Mikko Perttunen wrote:
> On 14/07/14 13:29, Thierry Reding wrote:
> >* PGP Signed by an unknown key
> > ...
> >>Yes, this sounds sensible. I'll make such a patch. I suppose having another
> >>timings table in the MC node with just the rate and mc-burst-data would
> >>separate the concerns best. It occurs to me that we could also write the
> >>regs in the pre-rate change notifier, but this would turn the dependency
> >>around and would mean that the regs are not written when entering backup
> >>rates. The latter shouldn't be a problem but the reversed dependency would,
> >>so I guess a custom function is the way to go, and we need to add at least
> >>one anyway.
> >
> >It sounds like maybe moving enough code and data into the MC driver to
> >handle frequency changes would be a good move. From the above it sounds
> >like all the MC driver needs to know is that a frequency change is about
> >to happen and what the new frequency is.
> >
> >In addition to exposing things like number of DRAM banks, etc.
> >
> 
> Yes, so there are two ways to do this:
> 1) EMC calls tegra_mc_emem_update(freq) at the correct time
> 2) MC has an optional clock phandle to the EMC clock and registers a
> pre-change notifier.
> 
> Both work, but the dependency is reversed. In both cases, the other driver
> is also optional. In the first case, the EMC driver can give a warning if
> the call fails. (As mentioned, if the MC_EMEM updates don't happen, things
> still work but potentially with a hefty perf loss.)
> TBH, I haven't yet decided which one is better. If you have an opinion,
> I'll go with it.

I think I prefer 1. Using an explicit call into the MC driver allow us
to precisely determine the moment in time when the registers should be
updated. The pre-change notifier, as I understand it, doesn't give us
that. Also, the notifier doesn't give us a way to determine success or
failure of the MC call.

> >>The downstream kernel also overwrites most LA registers during EMC rate
> >>change without regard for the driver-set values, and we might have to read
> >>those values from the device tree too. Upstream can do this in rate change
> >>notifiers if needed. I'll look into this a bit more.
> >
> >As I understand it, the latency allowance should be specified in terms
> >of the maximum amount of time that requests are delayed, so that the
> >proper values for the LA registers can be recomputed on an EMC rate
> >change.
> 
> That's how I understand it too, but in downstream, the LA register values
> are hardcoded into EMC tables in platform data/DTS that are just written
> into the LA registers as-is during rate change.

Hehe, well, we don't want any of that upstream. =) If it can be computed
at runtime, then let's compute it. Also, if it's encoded in platform
data or DTS, then there's no way it can be adjusted based on use-case.
For example if you have a device with two display outputs (an internal
panel and HDMI for example) but you never have HDMI plugged in, then
there's no reason why you would want to program the latency allowance
for the second display controller. If you provide the values in a static
frequency/register value table, then you need to account for any
possible scenario up front, irrespective of what (if any) HDMI monitor
is attached.

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140714/72999219/attachment.sig>

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH 5/8] of: Add Tegra124 EMC bindings
  2014-07-14 11:10                     ` Thierry Reding
@ 2014-07-14 12:28                       ` Mikko Perttunen
  0 siblings, 0 replies; 53+ messages in thread
From: Mikko Perttunen @ 2014-07-14 12:28 UTC (permalink / raw)
  To: linux-arm-kernel

On 14/07/14 14:10, Thierry Reding wrote:
> * PGP Signed by an unknown key
>
> On Mon, Jul 14, 2014 at 01:54:36PM +0300, Mikko Perttunen wrote:
>> On 14/07/14 13:29, Thierry Reding wrote:
>>>> Old Signed by an unknown key
>>> ...
>>>> Yes, this sounds sensible. I'll make such a patch. I suppose having another
>>>> timings table in the MC node with just the rate and mc-burst-data would
>>>> separate the concerns best. It occurs to me that we could also write the
>>>> regs in the pre-rate change notifier, but this would turn the dependency
>>>> around and would mean that the regs are not written when entering backup
>>>> rates. The latter shouldn't be a problem but the reversed dependency would,
>>>> so I guess a custom function is the way to go, and we need to add at least
>>>> one anyway.
>>>
>>> It sounds like maybe moving enough code and data into the MC driver to
>>> handle frequency changes would be a good move. From the above it sounds
>>> like all the MC driver needs to know is that a frequency change is about
>>> to happen and what the new frequency is.
>>>
>>> In addition to exposing things like number of DRAM banks, etc.
>>>
>>
>> Yes, so there are two ways to do this:
>> 1) EMC calls tegra_mc_emem_update(freq) at the correct time
>> 2) MC has an optional clock phandle to the EMC clock and registers a
>> pre-change notifier.
>>
>> Both work, but the dependency is reversed. In both cases, the other driver
>> is also optional. In the first case, the EMC driver can give a warning if
>> the call fails. (As mentioned, if the MC_EMEM updates don't happen, things
>> still work but potentially with a hefty perf loss.)
>> TBH, I haven't yet decided which one is better. If you have an opinion,
>> I'll go with it.
>
> I think I prefer 1. Using an explicit call into the MC driver allow us
> to precisely determine the moment in time when the registers should be
> updated. The pre-change notifier, as I understand it, doesn't give us
> that. Also, the notifier doesn't give us a way to determine success or
> failure of the MC call.

Indeed. I'll go with this.

>
>>>> The downstream kernel also overwrites most LA registers during EMC rate
>>>> change without regard for the driver-set values, and we might have to read
>>>> those values from the device tree too. Upstream can do this in rate change
>>>> notifiers if needed. I'll look into this a bit more.
>>>
>>> As I understand it, the latency allowance should be specified in terms
>>> of the maximum amount of time that requests are delayed, so that the
>>> proper values for the LA registers can be recomputed on an EMC rate
>>> change.
>>
>> That's how I understand it too, but in downstream, the LA register values
>> are hardcoded into EMC tables in platform data/DTS that are just written
>> into the LA registers as-is during rate change.
>
> Hehe, well, we don't want any of that upstream. =) If it can be computed
> at runtime, then let's compute it. Also, if it's encoded in platform
> data or DTS, then there's no way it can be adjusted based on use-case.
> For example if you have a device with two display outputs (an internal
> panel and HDMI for example) but you never have HDMI plugged in, then
> there's no reason why you would want to program the latency allowance
> for the second display controller. If you provide the values in a static
> frequency/register value table, then you need to account for any
> possible scenario up front, irrespective of what (if any) HDMI monitor
> is attached.

Yeah, I guess the values in downstream must be designed for the worst 
case :P the strange thing is that downstream also has an API for drivers 
to specify their requirements. I guess the clients that have hardcoded 
values and that use the API don't overlap. But I definitely agree that 
on upstream we can have something nicer.

>
> Thierry
>
> * Unknown Key
> * 0x7F3EB3A1
>

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH 5/8] of: Add Tegra124 EMC bindings
  2014-07-11 16:43   ` Andrew Bresticker
  2014-07-11 16:48     ` Mikko Perttunen
@ 2014-07-21 21:28     ` Stephen Warren
  2014-07-21 22:52       ` Andrew Bresticker
  1 sibling, 1 reply; 53+ messages in thread
From: Stephen Warren @ 2014-07-21 21:28 UTC (permalink / raw)
  To: linux-arm-kernel

On 07/11/2014 10:43 AM, Andrew Bresticker wrote:
> On Fri, Jul 11, 2014 at 7:18 AM, Mikko Perttunen <mperttunen@nvidia.com> wrote:
>> Add binding documentation for the nvidia,tegra124-emc device tree
>> node.
> 
>> diff --git a/Documentation/devicetree/bindings/memory-controllers/tegra-emc.txt b/Documentation/devicetree/bindings/memory-controllers/tegra-emc.txt
> 
>> +Required properties :
>> +- compatible : "nvidia,tegra124-emc".
>> +- reg : Should contain 1 or 2 entries:
>> +  - EMC register set
>> +  - MC register set : Required only if no node with
>> +    'compatible = "nvidia,tegra124-mc"' exists. The MC register set
>> +    is first read from the MC node. If it doesn't exist, it is read
>> +    from this property.
>> +- timings : Should contain 1 entry for each supported clock rate.
>> +  Entries should be named "timing at n" where n is a 0-based increasing
>> +  number. The timings must be listed in rate-ascending order.
> 
> There are upcoming boards which support multiple DRAM configurations
> and require a separate set of timings for each configuration.  Could
> we instead have multiple sets of timings with the proper one selected
> at runtime by RAM code, as reported by PMC_STRAPPING_OPT_A_0?
> Something like:
> 
> emc {
>         emc-table at 0 {
>                 nvidia,ram-code = <0>;
>                 timing at 0 {
>                         ...
>                 };
>                 ...
>         };

Until recently, there was a binding for Tegra20 EMC in mainline. We
should make sure the Tegra124 driver (or rather binding...) is at least
as feature-ful as that was.

Furthermore, I thought that with some boards, there were more RAM
options that there were available RAM code strap bits. I assume that in
mainline, we'll simply have different base DT files for the different
sets of configurations? Or, will we want to add another level to the DT
to represent different sets of RAM code values? I'm not sure what data
source the bootloader uses to determine which set of RAM configuration
to use when there aren't enough entries in the BCT for the boot ROM to
do this automatically, and whether we have any way to get that value
into the kernel, so it could use it for the extra DT level lookup?

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH 5/8] of: Add Tegra124 EMC bindings
  2014-07-11 14:18 ` [PATCH 5/8] of: Add Tegra124 EMC bindings Mikko Perttunen
  2014-07-11 14:51   ` Thierry Reding
  2014-07-11 16:43   ` Andrew Bresticker
@ 2014-07-21 22:36   ` Stephen Warren
  2 siblings, 0 replies; 53+ messages in thread
From: Stephen Warren @ 2014-07-21 22:36 UTC (permalink / raw)
  To: linux-arm-kernel

On 07/11/2014 08:18 AM, Mikko Perttunen wrote:
> Add binding documentation for the nvidia,tegra124-emc device tree
> node.

> diff --git a/Documentation/devicetree/bindings/memory-controllers/tegra-emc.txt b/Documentation/devicetree/bindings/memory-controllers/tegra-emc.txt

> +Required properties :
> +- compatible : "nvidia,tegra124-emc".
> +- reg : Should contain 1 or 2 entries:
> +  - EMC register set
> +  - MC register set : Required only if no node with
> +    'compatible = "nvidia,tegra124-mc"' exists. The MC register set
> +    is first read from the MC node. If it doesn't exist, it is read
> +    from this property.

I echo what Thierry said about the MC register sharing.

> +- timings : Should contain 1 entry for each supported clock rate.

s/entry/node/ I think.

> +  Entries should be named "timing at n" where n is a 0-based increasing
> +  number. The timings must be listed in rate-ascending order.

That implies that #address-cells and #size-cells are required too.

> +Required properties for timings :

... and a reg property is needed here.

> +- clock-frequency : Should contain the memory clock rate.
> +- nvidia,parent-clock-frequency : Should contain the rate of the EMC
> +  clock's parent clock.

> +- clocks : Must contain an entry for each entry in clock-names.
> +  See ../clocks/clock-bindings.txt for details.
> +- clock-names : Must include the following entries:
> +  - emc-parent : EMC's parent clock.

Shouldn't clocks/clock-names be part of the main node, not the
per-timing node?

> +- The following properties contain EMC timing characterization values:
> +  - nvidia,emc-zcal-cnt-long
> +  - nvidia,emc-auto-cal-interval
> +  - nvidia,emc-ctt-term-ctrl
> +  - nvidia,emc-cfg
> +  - nvidia,emc-cfg-2
> +  - nvidia,emc-sel-dpd-ctrl
> +  - nvidia,emc-cfg-dig-dll
> +  - nvidia,emc-bgbias-ctl0
> +  - nvidia,emc-auto-cal-config
> +  - nvidia,emc-auto-cal-config2
> +  - nvidia,emc-auto-cal-config3
> +  - nvidia,emc-mode-reset
> +  - nvidia,emc-mode-1
> +  - nvidia,emc-mode-2
> +  - nvidia,emc-mode-4
> +- nvidia,emc-burst-data : EMC timing characterization data written to
> +                          EMC registers.
> +- nvidia,mc-burst-data : EMC timing characterization data written to
> +                         MC registers.
> 

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH 2/8] ARM: tegra: Remove TEGRA124_CLK_EMC from tegra124-car.h
  2014-07-11 14:18 ` [PATCH 2/8] ARM: tegra: Remove TEGRA124_CLK_EMC from tegra124-car.h Mikko Perttunen
@ 2014-07-21 22:37   ` Stephen Warren
  2014-07-29  8:28     ` Mikko Perttunen
  0 siblings, 1 reply; 53+ messages in thread
From: Stephen Warren @ 2014-07-21 22:37 UTC (permalink / raw)
  To: linux-arm-kernel

On 07/11/2014 08:18 AM, Mikko Perttunen wrote:
> Remove the TEGRA124_CLK_EMC cell value define for tegra124-car
> from the binding headers. This clock has never been able to do
> anything and is being replaced with a separate EMC driver with
> its own device tree node. Removing the define ensures that any
> user will not mistakenly refer to <&tegra_car TEGRA124_CLK_EMC>
> instead of <&emc> or similar.

If the clock physically exists in HW, I see no reason to remove it from
the binding definition, even if we don't implement it (or remove the
implementation of it) in the Linux clock driver.

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH 5/8] of: Add Tegra124 EMC bindings
  2014-07-21 21:28     ` Stephen Warren
@ 2014-07-21 22:52       ` Andrew Bresticker
  2014-07-22 16:45         ` Stephen Warren
  0 siblings, 1 reply; 53+ messages in thread
From: Andrew Bresticker @ 2014-07-21 22:52 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Jul 21, 2014 at 2:28 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 07/11/2014 10:43 AM, Andrew Bresticker wrote:
>> On Fri, Jul 11, 2014 at 7:18 AM, Mikko Perttunen <mperttunen@nvidia.com> wrote:
>>> Add binding documentation for the nvidia,tegra124-emc device tree
>>> node.
>>
>>> diff --git a/Documentation/devicetree/bindings/memory-controllers/tegra-emc.txt b/Documentation/devicetree/bindings/memory-controllers/tegra-emc.txt
>>
>>> +Required properties :
>>> +- compatible : "nvidia,tegra124-emc".
>>> +- reg : Should contain 1 or 2 entries:
>>> +  - EMC register set
>>> +  - MC register set : Required only if no node with
>>> +    'compatible = "nvidia,tegra124-mc"' exists. The MC register set
>>> +    is first read from the MC node. If it doesn't exist, it is read
>>> +    from this property.
>>> +- timings : Should contain 1 entry for each supported clock rate.
>>> +  Entries should be named "timing at n" where n is a 0-based increasing
>>> +  number. The timings must be listed in rate-ascending order.
>>
>> There are upcoming boards which support multiple DRAM configurations
>> and require a separate set of timings for each configuration.  Could
>> we instead have multiple sets of timings with the proper one selected
>> at runtime by RAM code, as reported by PMC_STRAPPING_OPT_A_0?
>> Something like:
>>
>> emc {
>>         emc-table at 0 {
>>                 nvidia,ram-code = <0>;
>>                 timing at 0 {
>>                         ...
>>                 };
>>                 ...
>>         };
>
> Until recently, there was a binding for Tegra20 EMC in mainline. We
> should make sure the Tegra124 driver (or rather binding...) is at least
> as feature-ful as that was.
>
> Furthermore, I thought that with some boards, there were more RAM
> options that there were available RAM code strap bits. I assume that in
> mainline, we'll simply have different base DT files for the different
> sets of configurations? Or, will we want to add another level to the DT
> to represent different sets of RAM code values? I'm not sure what data
> source the bootloader uses to determine which set of RAM configuration
> to use when there aren't enough entries in the BCT for the boot ROM to
> do this automatically, and whether we have any way to get that value
> into the kernel, so it could use it for the extra DT level lookup?

For the ChromeOS boards at least we are neither limited by the number
of strapping bits (4) nor the number of BCT entries supported by the
boot ROM (since coreboot does not rely on the boot ROM for SDRAM
initialization), so having a single set of SDRAM configurations for
each board, indexed by APBDEV_PMC_STRAPPING_OPT_A_0[7:4] works just
fine.  Not sure how boards which do use the boot ROM for SDRAM
initialization handle the BCT limitation.  I believe we considered
using more board ID strapping bits in order to map the RAM codes to
sets of configurations before switching to having coreboot do SDRAM
initialization.

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH 5/8] of: Add Tegra124 EMC bindings
  2014-07-21 22:52       ` Andrew Bresticker
@ 2014-07-22 16:45         ` Stephen Warren
  2014-07-22 17:22           ` Andrew Bresticker
  0 siblings, 1 reply; 53+ messages in thread
From: Stephen Warren @ 2014-07-22 16:45 UTC (permalink / raw)
  To: linux-arm-kernel

On 07/21/2014 04:52 PM, Andrew Bresticker wrote:
> On Mon, Jul 21, 2014 at 2:28 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> On 07/11/2014 10:43 AM, Andrew Bresticker wrote:
>>> On Fri, Jul 11, 2014 at 7:18 AM, Mikko Perttunen <mperttunen@nvidia.com> wrote:
>>>> Add binding documentation for the nvidia,tegra124-emc device tree
>>>> node.
>>>
>>>> diff --git a/Documentation/devicetree/bindings/memory-controllers/tegra-emc.txt b/Documentation/devicetree/bindings/memory-controllers/tegra-emc.txt
>>>
>>>> +Required properties :
>>>> +- compatible : "nvidia,tegra124-emc".
>>>> +- reg : Should contain 1 or 2 entries:
>>>> +  - EMC register set
>>>> +  - MC register set : Required only if no node with
>>>> +    'compatible = "nvidia,tegra124-mc"' exists. The MC register set
>>>> +    is first read from the MC node. If it doesn't exist, it is read
>>>> +    from this property.
>>>> +- timings : Should contain 1 entry for each supported clock rate.
>>>> +  Entries should be named "timing at n" where n is a 0-based increasing
>>>> +  number. The timings must be listed in rate-ascending order.
>>>
>>> There are upcoming boards which support multiple DRAM configurations
>>> and require a separate set of timings for each configuration.  Could
>>> we instead have multiple sets of timings with the proper one selected
>>> at runtime by RAM code, as reported by PMC_STRAPPING_OPT_A_0?
>>> Something like:
>>>
>>> emc {
>>>         emc-table at 0 {
>>>                 nvidia,ram-code = <0>;
>>>                 timing at 0 {
>>>                         ...
>>>                 };
>>>                 ...
>>>         };
>>
>> Until recently, there was a binding for Tegra20 EMC in mainline. We
>> should make sure the Tegra124 driver (or rather binding...) is at least
>> as feature-ful as that was.
>>
>> Furthermore, I thought that with some boards, there were more RAM
>> options that there were available RAM code strap bits. I assume that in
>> mainline, we'll simply have different base DT files for the different
>> sets of configurations? Or, will we want to add another level to the DT
>> to represent different sets of RAM code values? I'm not sure what data
>> source the bootloader uses to determine which set of RAM configuration
>> to use when there aren't enough entries in the BCT for the boot ROM to
>> do this automatically, and whether we have any way to get that value
>> into the kernel, so it could use it for the extra DT level lookup?
> 
> For the ChromeOS boards at least we are neither limited by the number
> of strapping bits (4) nor the number of BCT entries supported by the
> boot ROM (since coreboot does not rely on the boot ROM for SDRAM
> initialization), so having a single set of SDRAM configurations for
> each board, indexed by APBDEV_PMC_STRAPPING_OPT_A_0[7:4] works just
> fine.

Does the bootloader adjust the DT that's passed to the kernel so that
only the relevant single set of EMC timings is contained in the DT?

On a system where the boot ROM initializes RAM, and where the HW design
might have multiple SDRAM configuration, here's what usually happens:

a) The BCT contains N sets of SDRAM configurations.

b) The boot ROM reads the SDRAM strapping bits, and uses them to pick
the correct SDRAM configuration from the N sets in the BCT.

c) The kernel DT has N sets of SDRAM configurations.

d) The kernel reads the SDRAM strapping bits, and uses them to pick the
correct SDRAM configuration from the N sets in the DT.

On the ChromeOS boards (so (a) and (b) above are irrelevant) where N is
too large to fit into APBDEV_PMC_STRAPPING_OPT_A_0[7:4], (c) and (d)
won't work. I assume the kernel DT therefore must be adjusted to only
contain the single SDRAM configuration that is relevant for the current HW?

(isn't STRAPPING_OPT_A split into 2 2-bit fields; 2 bits for SDRAM index
and 2 bits for boot flash index, so max N is quite small?)

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH 8/8] clk: tegra: Add EMC clock driver
  2014-07-11 14:18 ` [PATCH 8/8] clk: tegra: Add EMC clock driver Mikko Perttunen
@ 2014-07-22 16:57   ` Stephen Warren
  2014-07-29  8:47     ` Mikko Perttunen
  0 siblings, 1 reply; 53+ messages in thread
From: Stephen Warren @ 2014-07-22 16:57 UTC (permalink / raw)
  To: linux-arm-kernel

On 07/11/2014 08:18 AM, Mikko Perttunen wrote:
> The driver is currently only tested on Tegra124 Jetson TK1, but should
> work with other Tegra124 boards, provided that correct EMC tables are
> provided through the device tree. Older chip models have differing
> timing change sequences, so they are not currently supported.

> diff --git a/drivers/clk/tegra/clk-emc.c b/drivers/clk/tegra/clk-emc.c

> +struct emc_timing {
> +	unsigned long rate, parent_rate;
> +	u8 parent_index;
> +	struct clk *parent;
> +
> +	/* Store EMC burst data in a union to minimize mistakes. This allows
> +	 * us to use the same burst data lists as used by the downstream and
> +	 * ChromeOS kernels. */

Nit: */ should be on its own line. This applies to many comments in the
file.

> +/* * * * * * * * * * * * * * * * * * * * * * * * * *
> + * Timing change sequence functions                *
> + * * * * * * * * * * * * * * * * * * * * * * * * * */

Nit: This kind of banner comment is unusual, but I guess it's fine.

> +static void emc_seq_update_timing(struct tegra_emc *tegra)
...
> +	dev_err(&tegra->pdev->dev, "timing update failed\n");
> +	BUG();
> +}

Is there any way to avoid all these BUGs? Can we just continue on and
retry the next time, or disallow any further clock rate changes or
something?

> +/* * * * * * * * * * * * * * * * * * * * * * * * * *
> + * Debugfs entry                                   *
> + * * * * * * * * * * * * * * * * * * * * * * * * * */
> +
> +static int emc_debug_rate_get(void *data, u64 *rate)
> +{
> +	struct tegra_emc *tegra = data;
> +
> +	*rate = clk_get_rate(tegra->hw.clk);
> +
> +	return 0;
> +}
> +
> +static int emc_debug_rate_set(void *data, u64 rate)
> +{
> +	struct tegra_emc *tegra = data;
> +
> +	return clk_set_rate(tegra->hw.clk, rate);
> +}
> +
> +DEFINE_SIMPLE_ATTRIBUTE(emc_debug_rate_fops, emc_debug_rate_get,
> +			emc_debug_rate_set, "%lld\n");

I think the rate can already be obtained through
...debug/clock/clock_summary. I'm not sure about changing the rate, but
shouldn't that be a feature of the common clock core, not individual
drivers?

> +static int load_timings_from_dt(struct tegra_emc *tegra,
> +				struct device_node *node)
> +{
...
> +	for_each_child_of_node(node, child) {
...
> +		if (timing->rate <= prev_rate) {
> +			dev_err(&tegra->pdev->dev,
> +				"timing %s: rate not increasing\n",
> +				child->name);

I don't believe there's any guaranteed node enumeration order. If the
driver needs the child nodes sorted, it should sort them itself.

> +static const struct of_device_id tegra_car_of_match[] = {
> +	{ .compatible = "nvidia,tegra124-car" },
> +	{}
> +};
> +
> +static const struct of_device_id tegra_mc_of_match[] = {
> +	{ .compatible = "nvidia,tegra124-mc" },
> +	{}
> +};

It would be better if this driver explicitly called into the driver for
other modules, rather than directly touching their registers.

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH 5/8] of: Add Tegra124 EMC bindings
  2014-07-22 16:45         ` Stephen Warren
@ 2014-07-22 17:22           ` Andrew Bresticker
  2014-07-22 17:34             ` Stephen Warren
  0 siblings, 1 reply; 53+ messages in thread
From: Andrew Bresticker @ 2014-07-22 17:22 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Jul 22, 2014 at 9:45 AM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>
> Does the bootloader adjust the DT that's passed to the kernel so that
> only the relevant single set of EMC timings is contained in the DT?

No, the DT contains all possible EMC timings for that board.

> On a system where the boot ROM initializes RAM, and where the HW design
> might have multiple SDRAM configuration, here's what usually happens:
>
> a) The BCT contains N sets of SDRAM configurations.
>
> b) The boot ROM reads the SDRAM strapping bits, and uses them to pick
> the correct SDRAM configuration from the N sets in the BCT.
>
> c) The kernel DT has N sets of SDRAM configurations.
>
> d) The kernel reads the SDRAM strapping bits, and uses them to pick the
> correct SDRAM configuration from the N sets in the DT.
>
> On the ChromeOS boards (so (a) and (b) above are irrelevant) where N is
> too large to fit into APBDEV_PMC_STRAPPING_OPT_A_0[7:4], (c) and (d)
> won't work. I assume the kernel DT therefore must be adjusted to only
> contain the single SDRAM configuration that is relevant for the current HW?
>
> (isn't STRAPPING_OPT_A split into 2 2-bit fields; 2 bits for SDRAM index
> and 2 bits for boot flash index, so max N is quite small?)

Right, there are normally only 2 SDRAM strapping bits available.
ChromeOS gets around this by having 4 identical boot device entries in
the BCT, so all possible values of STRAPPING_OPT_A[7:6] map to the
same boot device.  This allows us to use all 4 strapping bits in
coreboot to pick the SDRAM configuration.

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH 5/8] of: Add Tegra124 EMC bindings
  2014-07-22 17:22           ` Andrew Bresticker
@ 2014-07-22 17:34             ` Stephen Warren
  2014-07-29  8:30               ` Mikko Perttunen
  0 siblings, 1 reply; 53+ messages in thread
From: Stephen Warren @ 2014-07-22 17:34 UTC (permalink / raw)
  To: linux-arm-kernel

On 07/22/2014 11:22 AM, Andrew Bresticker wrote:
> On Tue, Jul 22, 2014 at 9:45 AM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>
>> Does the bootloader adjust the DT that's passed to the kernel so that
>> only the relevant single set of EMC timings is contained in the DT?
> 
> No, the DT contains all possible EMC timings for that board.
> 
>> On a system where the boot ROM initializes RAM, and where the HW design
>> might have multiple SDRAM configuration, here's what usually happens:
>>
>> a) The BCT contains N sets of SDRAM configurations.
>>
>> b) The boot ROM reads the SDRAM strapping bits, and uses them to pick
>> the correct SDRAM configuration from the N sets in the BCT.
>>
>> c) The kernel DT has N sets of SDRAM configurations.
>>
>> d) The kernel reads the SDRAM strapping bits, and uses them to pick the
>> correct SDRAM configuration from the N sets in the DT.
>>
>> On the ChromeOS boards (so (a) and (b) above are irrelevant) where N is
>> too large to fit into APBDEV_PMC_STRAPPING_OPT_A_0[7:4], (c) and (d)
>> won't work. I assume the kernel DT therefore must be adjusted to only
>> contain the single SDRAM configuration that is relevant for the current HW?
>>
>> (isn't STRAPPING_OPT_A split into 2 2-bit fields; 2 bits for SDRAM index
>> and 2 bits for boot flash index, so max N is quite small?)
> 
> Right, there are normally only 2 SDRAM strapping bits available.
> ChromeOS gets around this by having 4 identical boot device entries in
> the BCT, so all possible values of STRAPPING_OPT_A[7:6] map to the
> same boot device.  This allows us to use all 4 strapping bits in
> coreboot to pick the SDRAM configuration.

OK, that explains how it works.

But that means that when the kernel reads the strapping options, it will
have to know if it uses just 2 bits (standard) or all 4 bits
(non-standard) to index into the DT's array of SDRAM configurations.
We'll need a DT property to represent that.

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH 2/8] ARM: tegra: Remove TEGRA124_CLK_EMC from tegra124-car.h
  2014-07-21 22:37   ` Stephen Warren
@ 2014-07-29  8:28     ` Mikko Perttunen
  0 siblings, 0 replies; 53+ messages in thread
From: Mikko Perttunen @ 2014-07-29  8:28 UTC (permalink / raw)
  To: linux-arm-kernel

Yeah, I'll drop this one.

- Mikko

On 22/07/14 01:37, Stephen Warren wrote:
> On 07/11/2014 08:18 AM, Mikko Perttunen wrote:
>> Remove the TEGRA124_CLK_EMC cell value define for tegra124-car
>> from the binding headers. This clock has never been able to do
>> anything and is being replaced with a separate EMC driver with
>> its own device tree node. Removing the define ensures that any
>> user will not mistakenly refer to <&tegra_car TEGRA124_CLK_EMC>
>> instead of <&emc> or similar.
>
> If the clock physically exists in HW, I see no reason to remove it from
> the binding definition, even if we don't implement it (or remove the
> implementation of it) in the Linux clock driver.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH 5/8] of: Add Tegra124 EMC bindings
  2014-07-22 17:34             ` Stephen Warren
@ 2014-07-29  8:30               ` Mikko Perttunen
  2014-07-29 15:49                 ` Stephen Warren
  0 siblings, 1 reply; 53+ messages in thread
From: Mikko Perttunen @ 2014-07-29  8:30 UTC (permalink / raw)
  To: linux-arm-kernel

Looks like the TRM doesn't document this either. I'll add an option 
("nvidia,short-ram-code" ?) for the next version.

On 22/07/14 20:34, Stephen Warren wrote:
> On 07/22/2014 11:22 AM, Andrew Bresticker wrote:
>> On Tue, Jul 22, 2014 at 9:45 AM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>>
>>> Does the bootloader adjust the DT that's passed to the kernel so that
>>> only the relevant single set of EMC timings is contained in the DT?
>>
>> No, the DT contains all possible EMC timings for that board.
>>
>>> On a system where the boot ROM initializes RAM, and where the HW design
>>> might have multiple SDRAM configuration, here's what usually happens:
>>>
>>> a) The BCT contains N sets of SDRAM configurations.
>>>
>>> b) The boot ROM reads the SDRAM strapping bits, and uses them to pick
>>> the correct SDRAM configuration from the N sets in the BCT.
>>>
>>> c) The kernel DT has N sets of SDRAM configurations.
>>>
>>> d) The kernel reads the SDRAM strapping bits, and uses them to pick the
>>> correct SDRAM configuration from the N sets in the DT.
>>>
>>> On the ChromeOS boards (so (a) and (b) above are irrelevant) where N is
>>> too large to fit into APBDEV_PMC_STRAPPING_OPT_A_0[7:4], (c) and (d)
>>> won't work. I assume the kernel DT therefore must be adjusted to only
>>> contain the single SDRAM configuration that is relevant for the current HW?
>>>
>>> (isn't STRAPPING_OPT_A split into 2 2-bit fields; 2 bits for SDRAM index
>>> and 2 bits for boot flash index, so max N is quite small?)
>>
>> Right, there are normally only 2 SDRAM strapping bits available.
>> ChromeOS gets around this by having 4 identical boot device entries in
>> the BCT, so all possible values of STRAPPING_OPT_A[7:6] map to the
>> same boot device.  This allows us to use all 4 strapping bits in
>> coreboot to pick the SDRAM configuration.
>
> OK, that explains how it works.
>
> But that means that when the kernel reads the strapping options, it will
> have to know if it uses just 2 bits (standard) or all 4 bits
> (non-standard) to index into the DT's array of SDRAM configurations.
> We'll need a DT property to represent that.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH 8/8] clk: tegra: Add EMC clock driver
  2014-07-22 16:57   ` Stephen Warren
@ 2014-07-29  8:47     ` Mikko Perttunen
  2014-07-29 20:19       ` Mike Turquette
  0 siblings, 1 reply; 53+ messages in thread
From: Mikko Perttunen @ 2014-07-29  8:47 UTC (permalink / raw)
  To: linux-arm-kernel

On 22/07/14 19:57, Stephen Warren wrote:
> On 07/11/2014 08:18 AM, Mikko Perttunen wrote:
>> The driver is currently only tested on Tegra124 Jetson TK1, but should
>> work with other Tegra124 boards, provided that correct EMC tables are
>> provided through the device tree. Older chip models have differing
>> timing change sequences, so they are not currently supported.
>
>> diff --git a/drivers/clk/tegra/clk-emc.c b/drivers/clk/tegra/clk-emc.c
>
>> +struct emc_timing {
>> +	unsigned long rate, parent_rate;
>> +	u8 parent_index;
>> +	struct clk *parent;
>> +
>> +	/* Store EMC burst data in a union to minimize mistakes. This allows
>> +	 * us to use the same burst data lists as used by the downstream and
>> +	 * ChromeOS kernels. */
>
> Nit: */ should be on its own line. This applies to many comments in the
> file.

Will fix.

>
>> +/* * * * * * * * * * * * * * * * * * * * * * * * * *
>> + * Timing change sequence functions                *
>> + * * * * * * * * * * * * * * * * * * * * * * * * * */
>
> Nit: This kind of banner comment is unusual, but I guess it's fine.
>
>> +static void emc_seq_update_timing(struct tegra_emc *tegra)
> ...
>> +	dev_err(&tegra->pdev->dev, "timing update failed\n");
>> +	BUG();
>> +}
>
> Is there any way to avoid all these BUGs? Can we just continue on and
> retry the next time, or disallow any further clock rate changes or
> something?

I guess I can just remove the BUG()s and keep going. The clock might 
temporarily end up in a strange state but that should be fine since 
these shouldn't be happening anyway.

>
>> +/* * * * * * * * * * * * * * * * * * * * * * * * * *
>> + * Debugfs entry                                   *
>> + * * * * * * * * * * * * * * * * * * * * * * * * * */
>> +
>> +static int emc_debug_rate_get(void *data, u64 *rate)
>> +{
>> +	struct tegra_emc *tegra = data;
>> +
>> +	*rate = clk_get_rate(tegra->hw.clk);
>> +
>> +	return 0;
>> +}
>> +
>> +static int emc_debug_rate_set(void *data, u64 rate)
>> +{
>> +	struct tegra_emc *tegra = data;
>> +
>> +	return clk_set_rate(tegra->hw.clk, rate);
>> +}
>> +
>> +DEFINE_SIMPLE_ATTRIBUTE(emc_debug_rate_fops, emc_debug_rate_get,
>> +			emc_debug_rate_set, "%lld\n");
>
> I think the rate can already be obtained through
> ...debug/clock/clock_summary. I'm not sure about changing the rate, but
> shouldn't that be a feature of the common clock core, not individual
> drivers?

The core doesn't allow writing to the rate debugfs files, so this is the 
only way to trigger an EMC clock change for now. I agree that the core 
might be a better place. I don't know if there are any philosophical 
objections to that. I'd like to keep this in until a possible core 
feature addition. Mike, any comments?

>
>> +static int load_timings_from_dt(struct tegra_emc *tegra,
>> +				struct device_node *node)
>> +{
> ...
>> +	for_each_child_of_node(node, child) {
> ...
>> +		if (timing->rate <= prev_rate) {
>> +			dev_err(&tegra->pdev->dev,
>> +				"timing %s: rate not increasing\n",
>> +				child->name);
>
> I don't believe there's any guaranteed node enumeration order. If the
> driver needs the child nodes sorted, it should sort them itself.

True. I'll fix this.

>
>> +static const struct of_device_id tegra_car_of_match[] = {
>> +	{ .compatible = "nvidia,tegra124-car" },
>> +	{}
>> +};
>> +
>> +static const struct of_device_id tegra_mc_of_match[] = {
>> +	{ .compatible = "nvidia,tegra124-mc" },
>> +	{}
>> +};
>
> It would be better if this driver explicitly called into the driver for
> other modules, rather than directly touching their registers.
>

My local v2 already has the MC-related code split into Thierry's MC 
driver. The CAR register writing is still done from the EMC driver. I 
could add helpers for it to the CAR driver.

Thanks,
Mikko

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH 5/8] of: Add Tegra124 EMC bindings
  2014-07-29  8:30               ` Mikko Perttunen
@ 2014-07-29 15:49                 ` Stephen Warren
  2014-07-31 10:48                   ` Mikko Perttunen
  0 siblings, 1 reply; 53+ messages in thread
From: Stephen Warren @ 2014-07-29 15:49 UTC (permalink / raw)
  To: linux-arm-kernel

On 07/29/2014 02:30 AM, Mikko Perttunen wrote:
> Looks like the TRM doesn't document this either. I'll add an option
> ("nvidia,short-ram-code" ?) for the next version.

Using the 2-bit RAM code field as the RAM code is normal operation, so I 
wouldn't call this "short".

Using the 2-bit boot device code field as extra RAM code bits is 
non-standard.

I would suggest nvidia,use-4-bit-ram-code or 
nvidia,use-boot-device-code-as-ram-code-msbs(!) as the property.

I see that the TRM implies the whole 4-bit field is RAM code, rather 
than there being 2 separate 2-bit fields for RAM code and boot device 
code. Can you please file a bug against the TRM to document this 
correctly? (The details of which bits are which are visible on the 
Jetson TK1 schematics for example).

> On 22/07/14 20:34, Stephen Warren wrote:
>> On 07/22/2014 11:22 AM, Andrew Bresticker wrote:
>>> On Tue, Jul 22, 2014 at 9:45 AM, Stephen Warren
>>> <swarren@wwwdotorg.org> wrote:
>>>>
>>>> Does the bootloader adjust the DT that's passed to the kernel so that
>>>> only the relevant single set of EMC timings is contained in the DT?
>>>
>>> No, the DT contains all possible EMC timings for that board.
>>>
>>>> On a system where the boot ROM initializes RAM, and where the HW design
>>>> might have multiple SDRAM configuration, here's what usually happens:
>>>>
>>>> a) The BCT contains N sets of SDRAM configurations.
>>>>
>>>> b) The boot ROM reads the SDRAM strapping bits, and uses them to pick
>>>> the correct SDRAM configuration from the N sets in the BCT.
>>>>
>>>> c) The kernel DT has N sets of SDRAM configurations.
>>>>
>>>> d) The kernel reads the SDRAM strapping bits, and uses them to pick the
>>>> correct SDRAM configuration from the N sets in the DT.
>>>>
>>>> On the ChromeOS boards (so (a) and (b) above are irrelevant) where N is
>>>> too large to fit into APBDEV_PMC_STRAPPING_OPT_A_0[7:4], (c) and (d)
>>>> won't work. I assume the kernel DT therefore must be adjusted to only
>>>> contain the single SDRAM configuration that is relevant for the
>>>> current HW?
>>>>
>>>> (isn't STRAPPING_OPT_A split into 2 2-bit fields; 2 bits for SDRAM
>>>> index
>>>> and 2 bits for boot flash index, so max N is quite small?)
>>>
>>> Right, there are normally only 2 SDRAM strapping bits available.
>>> ChromeOS gets around this by having 4 identical boot device entries in
>>> the BCT, so all possible values of STRAPPING_OPT_A[7:6] map to the
>>> same boot device.  This allows us to use all 4 strapping bits in
>>> coreboot to pick the SDRAM configuration.
>>
>> OK, that explains how it works.
>>
>> But that means that when the kernel reads the strapping options, it will
>> have to know if it uses just 2 bits (standard) or all 4 bits
>> (non-standard) to index into the DT's array of SDRAM configurations.
>> We'll need a DT property to represent that.
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
>> the body of a message to majordomo at vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH 8/8] clk: tegra: Add EMC clock driver
  2014-07-29  8:47     ` Mikko Perttunen
@ 2014-07-29 20:19       ` Mike Turquette
  2014-07-29 22:14         ` Stephen Warren
  0 siblings, 1 reply; 53+ messages in thread
From: Mike Turquette @ 2014-07-29 20:19 UTC (permalink / raw)
  To: linux-arm-kernel

Quoting Mikko Perttunen (2014-07-29 01:47:35)
> On 22/07/14 19:57, Stephen Warren wrote:
> > On 07/11/2014 08:18 AM, Mikko Perttunen wrote:
> >> +static int emc_debug_rate_set(void *data, u64 rate)
> >> +{
> >> +    struct tegra_emc *tegra = data;
> >> +
> >> +    return clk_set_rate(tegra->hw.clk, rate);
> >> +}
> >> +
> >> +DEFINE_SIMPLE_ATTRIBUTE(emc_debug_rate_fops, emc_debug_rate_get,
> >> +                    emc_debug_rate_set, "%lld\n");
> >
> > I think the rate can already be obtained through
> > ...debug/clock/clock_summary. I'm not sure about changing the rate, but
> > shouldn't that be a feature of the common clock core, not individual
> > drivers?
> 
> The core doesn't allow writing to the rate debugfs files, so this is the 
> only way to trigger an EMC clock change for now. I agree that the core 
> might be a better place. I don't know if there are any philosophical 
> objections to that. I'd like to keep this in until a possible core 
> feature addition. Mike, any comments?

Yes, there is a philosophical rejection to exposing rate-change knobs to
userspace through debugfs. These can and will ship in real products
(typically Android) with lots of nasty userspace hacks, and also
represent pretty dangerous things to expose to userspace. I have always
maintained that such knobs should remain out of tree or, with the advent
of the custom debugfs entries, should be burden of the clock drivers.

Regards,
Mike

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH 8/8] clk: tegra: Add EMC clock driver
  2014-07-29 20:19       ` Mike Turquette
@ 2014-07-29 22:14         ` Stephen Warren
  2014-07-30  9:34           ` Thierry Reding
  0 siblings, 1 reply; 53+ messages in thread
From: Stephen Warren @ 2014-07-29 22:14 UTC (permalink / raw)
  To: linux-arm-kernel

On 07/29/2014 02:19 PM, Mike Turquette wrote:
> Quoting Mikko Perttunen (2014-07-29 01:47:35)
>> On 22/07/14 19:57, Stephen Warren wrote:
>>> On 07/11/2014 08:18 AM, Mikko Perttunen wrote:
>>>> +static int emc_debug_rate_set(void *data, u64 rate)
>>>> +{
>>>> +    struct tegra_emc *tegra = data;
>>>> +
>>>> +    return clk_set_rate(tegra->hw.clk, rate);
>>>> +}
>>>> +
>>>> +DEFINE_SIMPLE_ATTRIBUTE(emc_debug_rate_fops, emc_debug_rate_get,
>>>> +                    emc_debug_rate_set, "%lld\n");
>>>
>>> I think the rate can already be obtained through
>>> ...debug/clock/clock_summary. I'm not sure about changing the rate, but
>>> shouldn't that be a feature of the common clock core, not individual
>>> drivers?
>>
>> The core doesn't allow writing to the rate debugfs files, so this is the
>> only way to trigger an EMC clock change for now. I agree that the core
>> might be a better place. I don't know if there are any philosophical
>> objections to that. I'd like to keep this in until a possible core
>> feature addition. Mike, any comments?
>
> Yes, there is a philosophical rejection to exposing rate-change knobs to
> userspace through debugfs. These can and will ship in real products
> (typically Android) with lots of nasty userspace hacks, and also
> represent pretty dangerous things to expose to userspace. I have always
> maintained that such knobs should remain out of tree or, with the advent
> of the custom debugfs entries, should be burden of the clock drivers.

That argument seems a bit inconsistent.

I can see the argument to disallow code that lets user-space fiddle with 
clocks. However, if that argument holds, then surely it must apply to 
either the clock core *or* a clock driver; the end effect of allowing 
the code in either place is that people will be able to implement the 
user-space hacks you want to avoid. Yet, if we allow the code because 
it's a useful debug tool, then surely it should be in the clock core so 
we don't implement it redundantly in each clock driver.

We could always taint the kernel if the feature is used. Admittedly that 
wouldn't stop people using the feature as a hack in Android/product 
kernels, but at least nobody would have to unknowingly debug problems 
due to such manipulation, in the context of an upstream kernel.

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH 8/8] clk: tegra: Add EMC clock driver
  2014-07-29 22:14         ` Stephen Warren
@ 2014-07-30  9:34           ` Thierry Reding
  2014-07-31 19:06             ` Mike Turquette
  0 siblings, 1 reply; 53+ messages in thread
From: Thierry Reding @ 2014-07-30  9:34 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Jul 29, 2014 at 04:14:44PM -0600, Stephen Warren wrote:
> On 07/29/2014 02:19 PM, Mike Turquette wrote:
> >Quoting Mikko Perttunen (2014-07-29 01:47:35)
> >>On 22/07/14 19:57, Stephen Warren wrote:
> >>>On 07/11/2014 08:18 AM, Mikko Perttunen wrote:
> >>>>+static int emc_debug_rate_set(void *data, u64 rate)
> >>>>+{
> >>>>+    struct tegra_emc *tegra = data;
> >>>>+
> >>>>+    return clk_set_rate(tegra->hw.clk, rate);
> >>>>+}
> >>>>+
> >>>>+DEFINE_SIMPLE_ATTRIBUTE(emc_debug_rate_fops, emc_debug_rate_get,
> >>>>+                    emc_debug_rate_set, "%lld\n");
> >>>
> >>>I think the rate can already be obtained through
> >>>...debug/clock/clock_summary. I'm not sure about changing the rate, but
> >>>shouldn't that be a feature of the common clock core, not individual
> >>>drivers?
> >>
> >>The core doesn't allow writing to the rate debugfs files, so this is the
> >>only way to trigger an EMC clock change for now. I agree that the core
> >>might be a better place. I don't know if there are any philosophical
> >>objections to that. I'd like to keep this in until a possible core
> >>feature addition. Mike, any comments?
> >
> >Yes, there is a philosophical rejection to exposing rate-change knobs to
> >userspace through debugfs. These can and will ship in real products
> >(typically Android) with lots of nasty userspace hacks, and also
> >represent pretty dangerous things to expose to userspace. I have always
> >maintained that such knobs should remain out of tree or, with the advent
> >of the custom debugfs entries, should be burden of the clock drivers.
> 
> That argument seems a bit inconsistent.
> 
> I can see the argument to disallow code that lets user-space fiddle with
> clocks. However, if that argument holds, then surely it must apply to either
> the clock core *or* a clock driver; the end effect of allowing the code in
> either place is that people will be able to implement the user-space hacks
> you want to avoid. Yet, if we allow the code because it's a useful debug
> tool, then surely it should be in the clock core so we don't implement it
> redundantly in each clock driver.
> 
> We could always taint the kernel if the feature is used. Admittedly that
> wouldn't stop people using the feature as a hack in Android/product kernels,
> but at least nobody would have to unknowingly debug problems due to such
> manipulation, in the context of an upstream kernel.

Not merging this feature upstream won't stop anybody from implementing
it as a hack in Android/product kernels either. If it's useful then
somebody will implement it in whatever downstream kernel they use. And
if it's useful to more than one vendor then we'll end up with a copy of
the implementation (and derivations on top) in each vendor's tree.

debugfs requires superuser privileges anyway, in which case you could
equally well write userspace software that directly bangs on the clock
controller registers.

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140730/656bf6d5/attachment.sig>

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH 5/8] of: Add Tegra124 EMC bindings
  2014-07-29 15:49                 ` Stephen Warren
@ 2014-07-31 10:48                   ` Mikko Perttunen
  2014-07-31 11:05                     ` Mikko Perttunen
  0 siblings, 1 reply; 53+ messages in thread
From: Mikko Perttunen @ 2014-07-31 10:48 UTC (permalink / raw)
  To: linux-arm-kernel

On 29/07/14 18:49, Stephen Warren wrote:
> On 07/29/2014 02:30 AM, Mikko Perttunen wrote:
>> Looks like the TRM doesn't document this either. I'll add an option
>> ("nvidia,short-ram-code" ?) for the next version.
>
> Using the 2-bit RAM code field as the RAM code is normal operation, so I
> wouldn't call this "short".
>
> Using the 2-bit boot device code field as extra RAM code bits is
> non-standard.
>
> I would suggest nvidia,use-4-bit-ram-code or
> nvidia,use-boot-device-code-as-ram-code-msbs(!) as the property.

Sure.

>
> I see that the TRM implies the whole 4-bit field is RAM code, rather
> than there being 2 separate 2-bit fields for RAM code and boot device
> code. Can you please file a bug against the TRM to document this
> correctly? (The details of which bits are which are visible on the
> Jetson TK1 schematics for example).

Yes, I'll file a bug.

- Mikko

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH 5/8] of: Add Tegra124 EMC bindings
  2014-07-31 10:48                   ` Mikko Perttunen
@ 2014-07-31 11:05                     ` Mikko Perttunen
  2014-07-31 15:32                       ` Stephen Warren
  0 siblings, 1 reply; 53+ messages in thread
From: Mikko Perttunen @ 2014-07-31 11:05 UTC (permalink / raw)
  To: linux-arm-kernel

On 31/07/14 13:48, Mikko Perttunen wrote:
>>
>> I see that the TRM implies the whole 4-bit field is RAM code, rather
>> than there being 2 separate 2-bit fields for RAM code and boot device
>> code. Can you please file a bug against the TRM to document this
>> correctly? (The details of which bits are which are visible on the
>> Jetson TK1 schematics for example).
>
> Yes, I'll file a bug.

On a closer look, the downstream kernel has been recently updated to 
consider the whole 4 bits the ram code. The relevant bug also has a 
comment mentioning that starting from T124, the whole 4 bits is 
considered the RAM code.

>
> - Mikko
> --
> To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH 5/8] of: Add Tegra124 EMC bindings
  2014-07-31 11:05                     ` Mikko Perttunen
@ 2014-07-31 15:32                       ` Stephen Warren
  0 siblings, 0 replies; 53+ messages in thread
From: Stephen Warren @ 2014-07-31 15:32 UTC (permalink / raw)
  To: linux-arm-kernel

On 07/31/2014 05:05 AM, Mikko Perttunen wrote:
> On 31/07/14 13:48, Mikko Perttunen wrote:
>>>
>>> I see that the TRM implies the whole 4-bit field is RAM code, rather
>>> than there being 2 separate 2-bit fields for RAM code and boot device
>>> code. Can you please file a bug against the TRM to document this
>>> correctly? (The details of which bits are which are visible on the
>>> Jetson TK1 schematics for example).
>>
>> Yes, I'll file a bug.
>
> On a closer look, the downstream kernel has been recently updated to
> consider the whole 4 bits the ram code. The relevant bug also has a
> comment mentioning that starting from T124, the whole 4 bits is
> considered the RAM code.

That's odd. Given the structure of the BCT hasn't change, I suspect 
that's a documentation bug that's propagated elsewhere. Can you send me 
the bug number internally, and I'll take a look? Thanks.

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH 8/8] clk: tegra: Add EMC clock driver
  2014-07-30  9:34           ` Thierry Reding
@ 2014-07-31 19:06             ` Mike Turquette
  2014-07-31 19:53               ` Stephen Warren
  2014-08-01  8:40               ` Thierry Reding
  0 siblings, 2 replies; 53+ messages in thread
From: Mike Turquette @ 2014-07-31 19:06 UTC (permalink / raw)
  To: linux-arm-kernel

Quoting Thierry Reding (2014-07-30 02:34:57)
> On Tue, Jul 29, 2014 at 04:14:44PM -0600, Stephen Warren wrote:
> > On 07/29/2014 02:19 PM, Mike Turquette wrote:
> > >Quoting Mikko Perttunen (2014-07-29 01:47:35)
> > >>On 22/07/14 19:57, Stephen Warren wrote:
> > >>>On 07/11/2014 08:18 AM, Mikko Perttunen wrote:
> > >>>>+static int emc_debug_rate_set(void *data, u64 rate)
> > >>>>+{
> > >>>>+    struct tegra_emc *tegra = data;
> > >>>>+
> > >>>>+    return clk_set_rate(tegra->hw.clk, rate);
> > >>>>+}
> > >>>>+
> > >>>>+DEFINE_SIMPLE_ATTRIBUTE(emc_debug_rate_fops, emc_debug_rate_get,
> > >>>>+                    emc_debug_rate_set, "%lld\n");
> > >>>
> > >>>I think the rate can already be obtained through
> > >>>...debug/clock/clock_summary. I'm not sure about changing the rate, but
> > >>>shouldn't that be a feature of the common clock core, not individual
> > >>>drivers?
> > >>
> > >>The core doesn't allow writing to the rate debugfs files, so this is the
> > >>only way to trigger an EMC clock change for now. I agree that the core
> > >>might be a better place. I don't know if there are any philosophical
> > >>objections to that. I'd like to keep this in until a possible core
> > >>feature addition. Mike, any comments?
> > >
> > >Yes, there is a philosophical rejection to exposing rate-change knobs to
> > >userspace through debugfs. These can and will ship in real products
> > >(typically Android) with lots of nasty userspace hacks, and also
> > >represent pretty dangerous things to expose to userspace. I have always
> > >maintained that such knobs should remain out of tree or, with the advent
> > >of the custom debugfs entries, should be burden of the clock drivers.
> > 
> > That argument seems a bit inconsistent.
> > 
> > I can see the argument to disallow code that lets user-space fiddle with
> > clocks. However, if that argument holds, then surely it must apply to either
> > the clock core *or* a clock driver; the end effect of allowing the code in

Stephen,

You meant to say, "it must apply to both the clock core and a clock
driver"? I agree.

> > either place is that people will be able to implement the user-space hacks
> > you want to avoid. Yet, if we allow the code because it's a useful debug
> > tool, then surely it should be in the clock core so we don't implement it
> > redundantly in each clock driver.

I don't want it anywhere to be honest. Read-only debugfs stuff is great
and I'm happy to merge it. I have repeatedly NAK'd any attempt to get
the userspace rate-change stuff merged into the core.

Recently we have the ability to assign custom debugfs entries that are
specific to the clock driver. I'm trying to find the right balance
between giving the clock driver authors the right amount of autonomy to
implement what they need while trying to keep the crazy out of the
kernel. Maybe in this case I should stick to my guns and NAK this patch.

> > 
> > We could always taint the kernel if the feature is used. Admittedly that
> > wouldn't stop people using the feature as a hack in Android/product kernels,
> > but at least nobody would have to unknowingly debug problems due to such
> > manipulation, in the context of an upstream kernel.
> 
> Not merging this feature upstream won't stop anybody from implementing
> it as a hack in Android/product kernels either. If it's useful then
> somebody will implement it in whatever downstream kernel they use. And
> if it's useful to more than one vendor then we'll end up with a copy of
> the implementation (and derivations on top) in each vendor's tree.

Thierry,

That argument is not sufficient to merit merging code. There is all
kinds of wacky downstream code that gets duplicated by various hardware
vendors, Linux distributions, the Cyanogenmod community, etc. Should we
merge any piece of downstream code just because more than one person
wants to use it?

> 
> debugfs requires superuser privileges anyway, in which case you could
> equally well write userspace software that directly bangs on the clock
> controller registers.

Sure. There are lots of technical reasons why this isn't such a bad idea
(e.g. you need admin privileges, we shouldn't ship devices with debugfs
turned on, etc). But by merging it we tell people, "hey, this is an OK
thing to do", which it is not.

Regards,
Mike

> 
> Thierry

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH 8/8] clk: tegra: Add EMC clock driver
  2014-07-31 19:06             ` Mike Turquette
@ 2014-07-31 19:53               ` Stephen Warren
  2014-07-31 23:08                 ` Mike Turquette
  2014-08-01  8:40               ` Thierry Reding
  1 sibling, 1 reply; 53+ messages in thread
From: Stephen Warren @ 2014-07-31 19:53 UTC (permalink / raw)
  To: linux-arm-kernel

On 07/31/2014 01:06 PM, Mike Turquette wrote:
> Quoting Thierry Reding (2014-07-30 02:34:57)
>> On Tue, Jul 29, 2014 at 04:14:44PM -0600, Stephen Warren wrote:
>>> On 07/29/2014 02:19 PM, Mike Turquette wrote:
>>>> Quoting Mikko Perttunen (2014-07-29 01:47:35)
>>>>> On 22/07/14 19:57, Stephen Warren wrote:
>>>>>> On 07/11/2014 08:18 AM, Mikko Perttunen wrote:
>>>>>>> +static int emc_debug_rate_set(void *data, u64 rate)
>>>>>>> +{
>>>>>>> +    struct tegra_emc *tegra = data;
>>>>>>> +
>>>>>>> +    return clk_set_rate(tegra->hw.clk, rate);
>>>>>>> +}
>>>>>>> +
>>>>>>> +DEFINE_SIMPLE_ATTRIBUTE(emc_debug_rate_fops, emc_debug_rate_get,
>>>>>>> +                    emc_debug_rate_set, "%lld\n");
>>>>>>
>>>>>> I think the rate can already be obtained through
>>>>>> ...debug/clock/clock_summary. I'm not sure about changing the rate, but
>>>>>> shouldn't that be a feature of the common clock core, not individual
>>>>>> drivers?
>>>>>
>>>>> The core doesn't allow writing to the rate debugfs files, so this is the
>>>>> only way to trigger an EMC clock change for now. I agree that the core
>>>>> might be a better place. I don't know if there are any philosophical
>>>>> objections to that. I'd like to keep this in until a possible core
>>>>> feature addition. Mike, any comments?
>>>>
>>>> Yes, there is a philosophical rejection to exposing rate-change knobs to
>>>> userspace through debugfs. These can and will ship in real products
>>>> (typically Android) with lots of nasty userspace hacks, and also
>>>> represent pretty dangerous things to expose to userspace. I have always
>>>> maintained that such knobs should remain out of tree or, with the advent
>>>> of the custom debugfs entries, should be burden of the clock drivers.
>>>
>>> That argument seems a bit inconsistent.
>>>
>>> I can see the argument to disallow code that lets user-space fiddle with
>>> clocks. However, if that argument holds, then surely it must apply to either
>>> the clock core *or* a clock driver; the end effect of allowing the code in
>
> Stephen,
>
> You meant to say, "it must apply to both the clock core and a clock
> driver"? I agree.

Sure; s/either/both/ in what I said.

>>> either place is that people will be able to implement the user-space hacks
>>> you want to avoid. Yet, if we allow the code because it's a useful debug
>>> tool, then surely it should be in the clock core so we don't implement it
>>> redundantly in each clock driver.
>
> I don't want it anywhere to be honest. Read-only debugfs stuff is great
> and I'm happy to merge it. I have repeatedly NAK'd any attempt to get
> the userspace rate-change stuff merged into the core.
>
> Recently we have the ability to assign custom debugfs entries that are
> specific to the clock driver. I'm trying to find the right balance
> between giving the clock driver authors the right amount of autonomy to
> implement what they need while trying to keep the crazy out of the
> kernel. Maybe in this case I should stick to my guns and NAK this patch.

If someone implements the same thing in some downstream/product kernel, 
and it can't be upstreamed, I'd argue they should still do that in the 
clock core rather than individual clock drivers, since they'll probably 
want the feature for multiple clocks. I don't think  the per-clock 
debugfs hook is useful in this case, although I can certainly imagine 
other read-only per-clock debug files could be useful.

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH 8/8] clk: tegra: Add EMC clock driver
  2014-07-31 19:53               ` Stephen Warren
@ 2014-07-31 23:08                 ` Mike Turquette
  2014-08-01  6:31                   ` Mikko Perttunen
  0 siblings, 1 reply; 53+ messages in thread
From: Mike Turquette @ 2014-07-31 23:08 UTC (permalink / raw)
  To: linux-arm-kernel

Quoting Stephen Warren (2014-07-31 12:53:54)
> On 07/31/2014 01:06 PM, Mike Turquette wrote:
> > Quoting Thierry Reding (2014-07-30 02:34:57)
> >> On Tue, Jul 29, 2014 at 04:14:44PM -0600, Stephen Warren wrote:
> >>> On 07/29/2014 02:19 PM, Mike Turquette wrote:
> >>>> Quoting Mikko Perttunen (2014-07-29 01:47:35)
> >>>>> On 22/07/14 19:57, Stephen Warren wrote:
> >>>>>> On 07/11/2014 08:18 AM, Mikko Perttunen wrote:
> >>>>>>> +static int emc_debug_rate_set(void *data, u64 rate)
> >>>>>>> +{
> >>>>>>> +    struct tegra_emc *tegra = data;
> >>>>>>> +
> >>>>>>> +    return clk_set_rate(tegra->hw.clk, rate);
> >>>>>>> +}
> >>>>>>> +
> >>>>>>> +DEFINE_SIMPLE_ATTRIBUTE(emc_debug_rate_fops, emc_debug_rate_get,
> >>>>>>> +                    emc_debug_rate_set, "%lld\n");
> >>>>>>
> >>>>>> I think the rate can already be obtained through
> >>>>>> ...debug/clock/clock_summary. I'm not sure about changing the rate, but
> >>>>>> shouldn't that be a feature of the common clock core, not individual
> >>>>>> drivers?
> >>>>>
> >>>>> The core doesn't allow writing to the rate debugfs files, so this is the
> >>>>> only way to trigger an EMC clock change for now. I agree that the core
> >>>>> might be a better place. I don't know if there are any philosophical
> >>>>> objections to that. I'd like to keep this in until a possible core
> >>>>> feature addition. Mike, any comments?
> >>>>
> >>>> Yes, there is a philosophical rejection to exposing rate-change knobs to
> >>>> userspace through debugfs. These can and will ship in real products
> >>>> (typically Android) with lots of nasty userspace hacks, and also
> >>>> represent pretty dangerous things to expose to userspace. I have always
> >>>> maintained that such knobs should remain out of tree or, with the advent
> >>>> of the custom debugfs entries, should be burden of the clock drivers.
> >>>
> >>> That argument seems a bit inconsistent.
> >>>
> >>> I can see the argument to disallow code that lets user-space fiddle with
> >>> clocks. However, if that argument holds, then surely it must apply to either
> >>> the clock core *or* a clock driver; the end effect of allowing the code in
> >
> > Stephen,
> >
> > You meant to say, "it must apply to both the clock core and a clock
> > driver"? I agree.
> 
> Sure; s/either/both/ in what I said.
> 
> >>> either place is that people will be able to implement the user-space hacks
> >>> you want to avoid. Yet, if we allow the code because it's a useful debug
> >>> tool, then surely it should be in the clock core so we don't implement it
> >>> redundantly in each clock driver.
> >
> > I don't want it anywhere to be honest. Read-only debugfs stuff is great
> > and I'm happy to merge it. I have repeatedly NAK'd any attempt to get
> > the userspace rate-change stuff merged into the core.
> >
> > Recently we have the ability to assign custom debugfs entries that are
> > specific to the clock driver. I'm trying to find the right balance
> > between giving the clock driver authors the right amount of autonomy to
> > implement what they need while trying to keep the crazy out of the
> > kernel. Maybe in this case I should stick to my guns and NAK this patch.
> 
> If someone implements the same thing in some downstream/product kernel, 
> and it can't be upstreamed, I'd argue they should still do that in the 
> clock core rather than individual clock drivers, since they'll probably 
> want the feature for multiple clocks. I don't think  the per-clock 
> debugfs hook is useful in this case, although I can certainly imagine 
> other read-only per-clock debug files could be useful.

That is sensible, and all the more reason that this patch shouldn't
implement the rate-change feature within the clock driver. So consider
it NAK'd.

Also I agree that the per-clock debugfs entries are very useful for RO
operations, especially exposing how registers are programmed or other
relevant data.

Regards,
Mike

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH 8/8] clk: tegra: Add EMC clock driver
  2014-07-31 23:08                 ` Mike Turquette
@ 2014-08-01  6:31                   ` Mikko Perttunen
  0 siblings, 0 replies; 53+ messages in thread
From: Mikko Perttunen @ 2014-08-01  6:31 UTC (permalink / raw)
  To: linux-arm-kernel

On 01/08/14 02:08, Mike Turquette wrote:
> ...
> That is sensible, and all the more reason that this patch shouldn't
> implement the rate-change feature within the clock driver. So consider
> it NAK'd.
>
> Also I agree that the per-clock debugfs entries are very useful for RO
> operations, especially exposing how registers are programmed or other
> relevant data.
>
> Regards,
> Mike
>

Sure, I'll remove the debugfs code from the next version.

Thanks, Mikko

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH 8/8] clk: tegra: Add EMC clock driver
  2014-07-31 19:06             ` Mike Turquette
  2014-07-31 19:53               ` Stephen Warren
@ 2014-08-01  8:40               ` Thierry Reding
  1 sibling, 0 replies; 53+ messages in thread
From: Thierry Reding @ 2014-08-01  8:40 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Jul 31, 2014 at 12:06:59PM -0700, Mike Turquette wrote:
> Quoting Thierry Reding (2014-07-30 02:34:57)
[...]
> > Not merging this feature upstream won't stop anybody from implementing
> > it as a hack in Android/product kernels either. If it's useful then
> > somebody will implement it in whatever downstream kernel they use. And
> > if it's useful to more than one vendor then we'll end up with a copy of
> > the implementation (and derivations on top) in each vendor's tree.
> 
> Thierry,
> 
> That argument is not sufficient to merit merging code. There is all
> kinds of wacky downstream code that gets duplicated by various hardware
> vendors, Linux distributions, the Cyanogenmod community, etc. Should we
> merge any piece of downstream code just because more than one person
> wants to use it?

Of course not. And that wasn't my point. My point was that if people
want to implement hacks as shortcuts and avoid the work involved in
doing things properly, then they will find ways to do so irrespective
of whether code can be merged upstream or not.

But we're talking about a debugfs interface here. It was designed to be
used to make life easier for *developers* and provide a way to make it
easier to debug problems. I've certainly been in a situation a couple of
times where I wished this kind of knob had been available to avoid
rebuilding the kernel and rebooting just to tune some clock frequency to
see if it would make things work.

> > debugfs requires superuser privileges anyway, in which case you could
> > equally well write userspace software that directly bangs on the clock
> > controller registers.
> 
> Sure. There are lots of technical reasons why this isn't such a bad idea
> (e.g. you need admin privileges, we shouldn't ship devices with debugfs
> turned on, etc). But by merging it we tell people, "hey, this is an OK
> thing to do", which it is not.

I disagree. What we'd be telling people is: "Here's a bunch of knobs
that you can use to play around with clock frequencies for *debugging*
purposes." Using debugfs is never an OK thing to do for production
software. But I don't think you'll prevent people from doing stupid
things by limiting what a debug interface can do. The only thing you'll
achieve is to make it more difficult for people who want to use it the
right way.

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140801/e2d68350/attachment.sig>

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH 0/8] Tegra124 EMC (external memory controller) support
  2014-07-11 14:18 [PATCH 0/8] Tegra124 EMC (external memory controller) support Mikko Perttunen
                   ` (7 preceding siblings ...)
  2014-07-11 14:18 ` [PATCH 8/8] clk: tegra: Add EMC clock driver Mikko Perttunen
@ 2014-08-25 17:40 ` Stephen Warren
  2014-08-26  7:42   ` Mikko Perttunen
  8 siblings, 1 reply; 53+ messages in thread
From: Stephen Warren @ 2014-08-25 17:40 UTC (permalink / raw)
  To: linux-arm-kernel

On 07/11/2014 08:18 AM, Mikko Perttunen wrote:
> Hi everyone,
>
> this series adds support for the EMC (external memory controller) clock
> in the Tegra124 system-on-chip. The series has been tested on Jetson TK1.
>
> The first two patches remove the old "emc_mux" and "emc" clocks from the
> clock tree and the device tree bindings. This is, of course, not backwards
> compatible, but as these clocks have never been useful for anything
> (apart from maybe reading the boot rate of the EMC clock). If this is still
> not acceptable, the second patch can be dropped.
...

Mikko, this series had some comments, especially on the DT binding 
(patch 5/8) and how the MC/EMC drivers interact. Is there an updated 
version of the series? Or, is the series replaced by Tomeu Vizoso's work?

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH 3/8] ARM: tegra: Add PLL_M_UD and PLL_C_UD to tegra124-car binding header
  2014-07-11 14:18 ` [PATCH 3/8] ARM: tegra: Add PLL_M_UD and PLL_C_UD to tegra124-car binding header Mikko Perttunen
@ 2014-08-25 17:41   ` Stephen Warren
  2014-09-17 13:41     ` Peter De Schrijver
  0 siblings, 1 reply; 53+ messages in thread
From: Stephen Warren @ 2014-08-25 17:41 UTC (permalink / raw)
  To: linux-arm-kernel

On 07/11/2014 08:18 AM, Mikko Perttunen wrote:
> Add these clocks to the binding header so that EMC timings that have
> them as parent can refer to the clocks.

Peter, I assume this patch (and patch 4/8 too) will be needed by any EMC 
driver, whether it's part of this series or Tomeu Vizoso's work. Can you 
please review/ack those two patches for later reference. Thanks.

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH 0/8] Tegra124 EMC (external memory controller) support
  2014-08-25 17:40 ` [PATCH 0/8] Tegra124 EMC (external memory controller) support Stephen Warren
@ 2014-08-26  7:42   ` Mikko Perttunen
  2014-08-26  7:47     ` Thierry Reding
  2014-09-05 10:22     ` Tomeu Vizoso
  0 siblings, 2 replies; 53+ messages in thread
From: Mikko Perttunen @ 2014-08-26  7:42 UTC (permalink / raw)
  To: linux-arm-kernel

On 25/08/14 20:40, Stephen Warren wrote:
> On 07/11/2014 08:18 AM, Mikko Perttunen wrote:
>> Hi everyone,
>>
>> this series adds support for the EMC (external memory controller) clock
>> in the Tegra124 system-on-chip. The series has been tested on Jetson TK1.
>>
>> The first two patches remove the old "emc_mux" and "emc" clocks from the
>> clock tree and the device tree bindings. This is, of course, not
>> backwards
>> compatible, but as these clocks have never been useful for anything
>> (apart from maybe reading the boot rate of the EMC clock). If this is
>> still
>> not acceptable, the second patch can be dropped.
> ...
>
> Mikko, this series had some comments, especially on the DT binding
> (patch 5/8) and how the MC/EMC drivers interact. Is there an updated
> version of the series? Or, is the series replaced by Tomeu Vizoso's work?

Yes, I have a v2 with these comments addressed. One concern, though, is 
the part writing to CLK_SOURCE_EMC. If some other driver also wants to 
read this register (MC, likely), we might need to have an API for it in 
the CAR driver. On the other hand, maybe not, since it's only one 
register. Thierry?

Another point is that v2 adds a new API to the MC driver, which also 
doesn't exist yet. The EMC driver can technically work without the MC 
driver (but with a header for MC added), but I'm not sure the result 
would be very useful.

I believe the plan is that Tomeu's EMC code will be integrated into this 
EMC driver once both are ready.

Mikko

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH 0/8] Tegra124 EMC (external memory controller) support
  2014-08-26  7:42   ` Mikko Perttunen
@ 2014-08-26  7:47     ` Thierry Reding
  2014-08-26  8:02       ` Mikko Perttunen
  2014-09-05 10:22     ` Tomeu Vizoso
  1 sibling, 1 reply; 53+ messages in thread
From: Thierry Reding @ 2014-08-26  7:47 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Aug 26, 2014 at 10:42:21AM +0300, Mikko Perttunen wrote:
> On 25/08/14 20:40, Stephen Warren wrote:
> >On 07/11/2014 08:18 AM, Mikko Perttunen wrote:
> >>Hi everyone,
> >>
> >>this series adds support for the EMC (external memory controller) clock
> >>in the Tegra124 system-on-chip. The series has been tested on Jetson TK1.
> >>
> >>The first two patches remove the old "emc_mux" and "emc" clocks from the
> >>clock tree and the device tree bindings. This is, of course, not
> >>backwards
> >>compatible, but as these clocks have never been useful for anything
> >>(apart from maybe reading the boot rate of the EMC clock). If this is
> >>still
> >>not acceptable, the second patch can be dropped.
> >...
> >
> >Mikko, this series had some comments, especially on the DT binding
> >(patch 5/8) and how the MC/EMC drivers interact. Is there an updated
> >version of the series? Or, is the series replaced by Tomeu Vizoso's work?
> 
> Yes, I have a v2 with these comments addressed. One concern, though, is the
> part writing to CLK_SOURCE_EMC. If some other driver also wants to read this
> register (MC, likely), we might need to have an API for it in the CAR
> driver. On the other hand, maybe not, since it's only one register. Thierry?

I don't think any of these drivers should directly access registers that
aren't in the memory region that they've claimed. If they need to access
functionality provided by some other driver then they should do so via a
custom API.

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140826/07704403/attachment.sig>

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH 0/8] Tegra124 EMC (external memory controller) support
  2014-08-26  7:47     ` Thierry Reding
@ 2014-08-26  8:02       ` Mikko Perttunen
  0 siblings, 0 replies; 53+ messages in thread
From: Mikko Perttunen @ 2014-08-26  8:02 UTC (permalink / raw)
  To: linux-arm-kernel

On 26/08/14 10:47, Thierry Reding wrote:
> * PGP Signed by an unknown key
>
> On Tue, Aug 26, 2014 at 10:42:21AM +0300, Mikko Perttunen wrote:
>> On 25/08/14 20:40, Stephen Warren wrote:
>>> On 07/11/2014 08:18 AM, Mikko Perttunen wrote:
>>>> Hi everyone,
>>>>
>>>> this series adds support for the EMC (external memory controller) clock
>>>> in the Tegra124 system-on-chip. The series has been tested on Jetson TK1.
>>>>
>>>> The first two patches remove the old "emc_mux" and "emc" clocks from the
>>>> clock tree and the device tree bindings. This is, of course, not
>>>> backwards
>>>> compatible, but as these clocks have never been useful for anything
>>>> (apart from maybe reading the boot rate of the EMC clock). If this is
>>>> still
>>>> not acceptable, the second patch can be dropped.
>>> ...
>>>
>>> Mikko, this series had some comments, especially on the DT binding
>>> (patch 5/8) and how the MC/EMC drivers interact. Is there an updated
>>> version of the series? Or, is the series replaced by Tomeu Vizoso's work?
>>
>> Yes, I have a v2 with these comments addressed. One concern, though, is the
>> part writing to CLK_SOURCE_EMC. If some other driver also wants to read this
>> register (MC, likely), we might need to have an API for it in the CAR
>> driver. On the other hand, maybe not, since it's only one register. Thierry?
>
> I don't think any of these drivers should directly access registers that
> aren't in the memory region that they've claimed. If they need to access
> functionality provided by some other driver then they should do so via a
> custom API.

I'll make a simple API and post patches.

>
> Thierry
>
> * Unknown Key
> * 0x7F3EB3A1
>

Mikko

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH 0/8] Tegra124 EMC (external memory controller) support
  2014-08-26  7:42   ` Mikko Perttunen
  2014-08-26  7:47     ` Thierry Reding
@ 2014-09-05 10:22     ` Tomeu Vizoso
  2014-09-05 10:55       ` Mikko Perttunen
  1 sibling, 1 reply; 53+ messages in thread
From: Tomeu Vizoso @ 2014-09-05 10:22 UTC (permalink / raw)
  To: linux-arm-kernel

On 26 August 2014 09:42, Mikko Perttunen <mperttunen@nvidia.com> wrote:
> On 25/08/14 20:40, Stephen Warren wrote:
>>
>> On 07/11/2014 08:18 AM, Mikko Perttunen wrote:
>>>
>>> Hi everyone,
>>>
>>> this series adds support for the EMC (external memory controller) clock
>>> in the Tegra124 system-on-chip. The series has been tested on Jetson TK1.
>>>
>>> The first two patches remove the old "emc_mux" and "emc" clocks from the
>>> clock tree and the device tree bindings. This is, of course, not
>>> backwards
>>> compatible, but as these clocks have never been useful for anything
>>> (apart from maybe reading the boot rate of the EMC clock). If this is
>>> still
>>> not acceptable, the second patch can be dropped.
>>
>> ...
>>
>> Mikko, this series had some comments, especially on the DT binding
>> (patch 5/8) and how the MC/EMC drivers interact. Is there an updated
>> version of the series? Or, is the series replaced by Tomeu Vizoso's work?
>
>
> Yes, I have a v2 with these comments addressed. One concern, though, is the
> part writing to CLK_SOURCE_EMC. If some other driver also wants to read this
> register (MC, likely), we might need to have an API for it in the CAR
> driver. On the other hand, maybe not, since it's only one register. Thierry?
>
> Another point is that v2 adds a new API to the MC driver, which also doesn't
> exist yet. The EMC driver can technically work without the MC driver (but
> with a header for MC added), but I'm not sure the result would be very
> useful.
>
> I believe the plan is that Tomeu's EMC code will be integrated into this EMC
> driver once both are ready.

That's mostly right. My clk constraints series just allow consumers to
influence the effective rate of a clock, such as the EMC. And the
pm_qos series will allow to track the total memory bandwidth needs in
the system. I'm not sure yet where in the code this tracking will be
done. It could be done in an EMC driver in drivers/memory, but it's
probably too unrelated to be placed in the EMC clock driver.

Regards,

Tomeu

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH 0/8] Tegra124 EMC (external memory controller) support
  2014-09-05 10:22     ` Tomeu Vizoso
@ 2014-09-05 10:55       ` Mikko Perttunen
  0 siblings, 0 replies; 53+ messages in thread
From: Mikko Perttunen @ 2014-09-05 10:55 UTC (permalink / raw)
  To: linux-arm-kernel

FYI, the structure the EMC series currently takes (in my local branch) 
is as follows:
- clock implementation in CAR code. This just knows about allowed rates 
and which parents should have them and handles reparenting when required.
- memory/tegra124-emc: this contains the EMC rate change sequence and 
exports an API which the clock driver uses.
- memory/tegra124-mc: contains API to write registers in MC memory 
space, used by memory/tegra124-emc.

(I decided that this is cleaner than having a huge EMC driver that uses 
an API exported by CAR)

I'm not sure if adding the bandwidth management code to the clock or 
memory/emc driver would be best. But we can decide that later.

I need to clean up the series after the big refactorings; I'll submit a 
new version once that's done.

Mikko

(P.S. my internship ended so I'm using this address now)

On 09/05/2014 01:22 PM, Tomeu Vizoso wrote:
> On 26 August 2014 09:42, Mikko Perttunen <mperttunen@nvidia.com> wrote:
>> On 25/08/14 20:40, Stephen Warren wrote:
>>>
>>> On 07/11/2014 08:18 AM, Mikko Perttunen wrote:
>>>>
>>>> Hi everyone,
>>>>
>>>> this series adds support for the EMC (external memory controller) clock
>>>> in the Tegra124 system-on-chip. The series has been tested on Jetson TK1.
>>>>
>>>> The first two patches remove the old "emc_mux" and "emc" clocks from the
>>>> clock tree and the device tree bindings. This is, of course, not
>>>> backwards
>>>> compatible, but as these clocks have never been useful for anything
>>>> (apart from maybe reading the boot rate of the EMC clock). If this is
>>>> still
>>>> not acceptable, the second patch can be dropped.
>>>
>>> ...
>>>
>>> Mikko, this series had some comments, especially on the DT binding
>>> (patch 5/8) and how the MC/EMC drivers interact. Is there an updated
>>> version of the series? Or, is the series replaced by Tomeu Vizoso's work?
>>
>>
>> Yes, I have a v2 with these comments addressed. One concern, though, is the
>> part writing to CLK_SOURCE_EMC. If some other driver also wants to read this
>> register (MC, likely), we might need to have an API for it in the CAR
>> driver. On the other hand, maybe not, since it's only one register. Thierry?
>>
>> Another point is that v2 adds a new API to the MC driver, which also doesn't
>> exist yet. The EMC driver can technically work without the MC driver (but
>> with a header for MC added), but I'm not sure the result would be very
>> useful.
>>
>> I believe the plan is that Tomeu's EMC code will be integrated into this EMC
>> driver once both are ready.
>
> That's mostly right. My clk constraints series just allow consumers to
> influence the effective rate of a clock, such as the EMC. And the
> pm_qos series will allow to track the total memory bandwidth needs in
> the system. I'm not sure yet where in the code this tracking will be
> done. It could be done in an EMC driver in drivers/memory, but it's
> probably too unrelated to be placed in the EMC clock driver.
>
> Regards,
>
> Tomeu
> --
> To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH 3/8] ARM: tegra: Add PLL_M_UD and PLL_C_UD to tegra124-car binding header
  2014-08-25 17:41   ` Stephen Warren
@ 2014-09-17 13:41     ` Peter De Schrijver
  0 siblings, 0 replies; 53+ messages in thread
From: Peter De Schrijver @ 2014-09-17 13:41 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Aug 25, 2014 at 07:41:56PM +0200, Stephen Warren wrote:
> On 07/11/2014 08:18 AM, Mikko Perttunen wrote:
> > Add these clocks to the binding header so that EMC timings that have
> > them as parent can refer to the clocks.
> 
> Peter, I assume this patch (and patch 4/8 too) will be needed by any EMC 
> driver, whether it's part of this series or Tomeu Vizoso's work. Can you 
> please review/ack those two patches for later reference. Thanks.

Looks ok to me. I will take them in tegra-clk-next

^ permalink raw reply	[flat|nested] 53+ messages in thread

end of thread, other threads:[~2014-09-17 13:41 UTC | newest]

Thread overview: 53+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-11 14:18 [PATCH 0/8] Tegra124 EMC (external memory controller) support Mikko Perttunen
2014-07-11 14:18 ` [PATCH 1/8] clk: tegra124: Remove old emc_mux and emc clocks Mikko Perttunen
2014-07-11 14:18 ` [PATCH 2/8] ARM: tegra: Remove TEGRA124_CLK_EMC from tegra124-car.h Mikko Perttunen
2014-07-21 22:37   ` Stephen Warren
2014-07-29  8:28     ` Mikko Perttunen
2014-07-11 14:18 ` [PATCH 3/8] ARM: tegra: Add PLL_M_UD and PLL_C_UD to tegra124-car binding header Mikko Perttunen
2014-08-25 17:41   ` Stephen Warren
2014-09-17 13:41     ` Peter De Schrijver
2014-07-11 14:18 ` [PATCH 4/8] clk: tegra124: Add PLL_M_UD and PLL_C_UD clocks Mikko Perttunen
2014-07-11 14:18 ` [PATCH 5/8] of: Add Tegra124 EMC bindings Mikko Perttunen
2014-07-11 14:51   ` Thierry Reding
2014-07-11 16:01     ` Mikko Perttunen
2014-07-14  7:55       ` Mikko Perttunen
2014-07-14  8:15         ` Thierry Reding
2014-07-14  9:06           ` Mikko Perttunen
2014-07-14  9:31             ` Thierry Reding
2014-07-14  9:57               ` Mikko Perttunen
2014-07-14 10:29                 ` Thierry Reding
2014-07-14 10:54                   ` Mikko Perttunen
2014-07-14 11:10                     ` Thierry Reding
2014-07-14 12:28                       ` Mikko Perttunen
2014-07-11 16:43   ` Andrew Bresticker
2014-07-11 16:48     ` Mikko Perttunen
2014-07-21 21:28     ` Stephen Warren
2014-07-21 22:52       ` Andrew Bresticker
2014-07-22 16:45         ` Stephen Warren
2014-07-22 17:22           ` Andrew Bresticker
2014-07-22 17:34             ` Stephen Warren
2014-07-29  8:30               ` Mikko Perttunen
2014-07-29 15:49                 ` Stephen Warren
2014-07-31 10:48                   ` Mikko Perttunen
2014-07-31 11:05                     ` Mikko Perttunen
2014-07-31 15:32                       ` Stephen Warren
2014-07-21 22:36   ` Stephen Warren
2014-07-11 14:18 ` [PATCH 6/8] ARM: tegra: Add EMC to Tegra124 device tree Mikko Perttunen
2014-07-11 14:18 ` [PATCH 7/8] ARM: tegra: Add EMC timings to Jetson TK1 " Mikko Perttunen
2014-07-11 14:18 ` [PATCH 8/8] clk: tegra: Add EMC clock driver Mikko Perttunen
2014-07-22 16:57   ` Stephen Warren
2014-07-29  8:47     ` Mikko Perttunen
2014-07-29 20:19       ` Mike Turquette
2014-07-29 22:14         ` Stephen Warren
2014-07-30  9:34           ` Thierry Reding
2014-07-31 19:06             ` Mike Turquette
2014-07-31 19:53               ` Stephen Warren
2014-07-31 23:08                 ` Mike Turquette
2014-08-01  6:31                   ` Mikko Perttunen
2014-08-01  8:40               ` Thierry Reding
2014-08-25 17:40 ` [PATCH 0/8] Tegra124 EMC (external memory controller) support Stephen Warren
2014-08-26  7:42   ` Mikko Perttunen
2014-08-26  7:47     ` Thierry Reding
2014-08-26  8:02       ` Mikko Perttunen
2014-09-05 10:22     ` Tomeu Vizoso
2014-09-05 10:55       ` Mikko Perttunen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).