[RFC PATCH 0/8] Add TI EMIF SDRAM controller driver

[RFC PATCH 0/8] Add TI EMIF SDRAM controller driver

[Aneesh V]

Add a driver for the EMIF SDRAM controller used in TI SoCs

EMIF is an SDRAM controller that supports, based on its revision,
one or more of LPDDR2/DDR2/DDR3 protocols.This driver adds support
for LPDDR2.

The driver supports the following features:
- Calculates the DDR AC timing parameters to be set in EMIF
  registers using data from the device data-sheets and based
  on the DDR frequency. If data from data-sheets is not available
  default timing values from the JEDEC spec are used. These
  will be safe, but not necessarily optimal
- API for changing timings during DVFS or at boot-up
- Temperature alert configuration and handling of temperature
  alerts, if any for LPDDR2 devices
  * temperature alert is based on periodic polling of MR4 mode
    register in DDR devices automatically performed by hardware
  * timings are de-rated and brought back to nominal when
    temperature raises and falls respectively
- Cache of calculated register values to avoid re-calculating
  them

The driver will need some minor updates when it is eventually
integrated with DVFS. This can not be done now as DVFS support
is not available yet in mainline.

Discussions with Santosh Shilimkar <santosh.shilimkar@ti.com>
were immensely helpful in shaping up the interfaces. Vibhore Vardhan
<vvardhan@gmail.com> did the initial code snippet for thermal
handling.

Testing: 
- The driver is tested on OMAP4430 SDP.
- The driver in a slightly adapted form is also tested on OMAP5.
- Since mainline kernel doesn't have DVFS support yet,
  testing was done using a test module.
- Temperature alert handling was tested with simulated interrupts
  and faked temperature values as testing all cases in real-life
  scenarios is difficult.

Aneesh V (7):
  misc: ddr: add LPDDR2 data from JESD209-2
  misc: emif: add register definitions for EMIF
  misc: emif: add basic infrastructure for EMIF driver
  misc: emif: handle frequency and voltage change events
  misc: emif: add interrupt and temperature handling
  misc: emif: add one-time settings
  misc: emif: add debugfs entries for emif

Benoit Cousson (1):
  OMAP4: hwmod: add EMIF hw mod data

 arch/arm/mach-omap2/omap_hwmod_44xx_data.c |  110 ++
 drivers/misc/Kconfig                       |   20 +
 drivers/misc/Makefile                      |    2 +
 drivers/misc/emif.c                        | 1522 ++++++++++++++++++++++++++++
 drivers/misc/emif_regs.h                   |  461 +++++++++
 drivers/misc/jedec_ddr_data.c              |  141 +++
 include/linux/emif.h                       |  257 +++++
 include/linux/jedec_ddr.h                  |  174 ++++
 8 files changed, 2687 insertions(+), 0 deletions(-)
 create mode 100644 drivers/misc/emif.c
 create mode 100644 drivers/misc/emif_regs.h
 create mode 100644 drivers/misc/jedec_ddr_data.c
 create mode 100644 include/linux/emif.h
 create mode 100644 include/linux/jedec_ddr.h

[RFC PATCH 1/8] OMAP4: hwmod: add EMIF hw mod data

[Aneesh V]

From: Benoit Cousson <b-cousson@ti.com>

Signed-off-by: Benoit Cousson <b-cousson@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod_44xx_data.c |  110 ++++++++++++++++++++++++++++
 1 files changed, 110 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
index f9f1510..2b107c7 100644
--- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
@@ -5480,6 +5480,111 @@ static struct omap_hwmod omap44xx_usb_tll_hs_hwmod = {
 	.slaves_cnt	= ARRAY_SIZE(omap44xx_usb_tll_hs_slaves),
 };
 
+/*
+ * 'emif' class
+ * external memory interface no1
+ */
+
+static struct omap_hwmod_class omap44xx_emif_hwmod_class = {
+	.name	= "emif",
+};
+
+/* emif1 */
+static struct omap_hwmod omap44xx_emif1_hwmod;
+static struct omap_hwmod_irq_info omap44xx_emif1_irqs[] = {
+	{ .irq = 110 + OMAP44XX_IRQ_GIC_START },
+	{ .irq = -1 }
+};
+
+static struct omap_hwmod_addr_space omap44xx_emif1_addrs[] = {
+	{
+		.pa_start	= 0x4c000000,
+		.pa_end		= 0x4c0000ff,
+		.flags		= ADDR_TYPE_RT
+	},
+	{ }
+};
+
+/* emif_fw -> emif1 */
+static struct omap_hwmod_ocp_if omap44xx_emif_fw__emif1 = {
+	.master		= &omap44xx_emif_fw_hwmod,
+	.slave		= &omap44xx_emif1_hwmod,
+	.clk		= "l3_div_ck",
+	.addr		= omap44xx_emif1_addrs,
+	.user		= OCP_USER_MPU | OCP_USER_SDMA,
+};
+
+/* emif1 slave ports */
+static struct omap_hwmod_ocp_if *omap44xx_emif1_slaves[] = {
+	&omap44xx_emif_fw__emif1,
+};
+
+static struct omap_hwmod omap44xx_emif1_hwmod = {
+	.name		= "emif1",
+	.class		= &omap44xx_emif_hwmod_class,
+	.flags		= HWMOD_INIT_NO_IDLE | HWMOD_INIT_NO_RESET,
+	.mpu_irqs	= omap44xx_emif1_irqs,
+	.main_clk	= "emif1_fck",
+	.clkdm_name	= "l3_emif_clkdm",
+	.prcm		= {
+		.omap4 = {
+			.clkctrl_offs = OMAP4_CM_MEMIF_EMIF_1_CLKCTRL_OFFSET,
+			.context_offs = OMAP4_RM_MEMIF_EMIF_1_CONTEXT_OFFSET,
+			.modulemode   = MODULEMODE_HWCTRL,
+		},
+	},
+	.slaves		= omap44xx_emif1_slaves,
+	.slaves_cnt	= ARRAY_SIZE(omap44xx_emif1_slaves),
+};
+
+/* emif2 */
+static struct omap_hwmod omap44xx_emif2_hwmod;
+static struct omap_hwmod_irq_info omap44xx_emif2_irqs[] = {
+	{ .irq = 111 + OMAP44XX_IRQ_GIC_START },
+	{ .irq = -1 }
+};
+
+static struct omap_hwmod_addr_space omap44xx_emif2_addrs[] = {
+	{
+		.pa_start	= 0x4d000000,
+		.pa_end		= 0x4d0000ff,
+		.flags		= ADDR_TYPE_RT
+	},
+	{ }
+};
+
+/* emif_fw -> emif2 */
+static struct omap_hwmod_ocp_if omap44xx_emif_fw__emif2 = {
+	.master		= &omap44xx_emif_fw_hwmod,
+	.slave		= &omap44xx_emif2_hwmod,
+	.clk		= "l3_div_ck",
+	.addr		= omap44xx_emif2_addrs,
+	.user		= OCP_USER_MPU | OCP_USER_SDMA,
+};
+
+/* emif2 slave ports */
+static struct omap_hwmod_ocp_if *omap44xx_emif2_slaves[] = {
+	&omap44xx_emif_fw__emif2,
+};
+
+static struct omap_hwmod omap44xx_emif2_hwmod = {
+	.name		= "emif2",
+	.class		= &omap44xx_emif_hwmod_class,
+	.flags		= HWMOD_INIT_NO_IDLE | HWMOD_INIT_NO_RESET,
+	.mpu_irqs	= omap44xx_emif2_irqs,
+	.main_clk	= "emif2_fck",
+	.clkdm_name	= "l3_emif_clkdm",
+	.prcm		= {
+		.omap4 = {
+			.clkctrl_offs = OMAP4_CM_MEMIF_EMIF_1_CLKCTRL_OFFSET,
+			.context_offs = OMAP4_RM_MEMIF_EMIF_1_CONTEXT_OFFSET,
+			.modulemode   = MODULEMODE_HWCTRL,
+		}
+	},
+	.slaves		= omap44xx_emif2_slaves,
+	.slaves_cnt	= ARRAY_SIZE(omap44xx_emif2_slaves),
+};
+
 static __initdata struct omap_hwmod *omap44xx_hwmods[] = {
 
 	/* dmm class */
@@ -5629,6 +5734,11 @@ static __initdata struct omap_hwmod *omap44xx_hwmods[] = {
 	/* wd_timer class */
 	&omap44xx_wd_timer2_hwmod,
 	&omap44xx_wd_timer3_hwmod,
+
+	/* emif class */
+	&omap44xx_emif1_hwmod,
+	&omap44xx_emif2_hwmod,
+
 	NULL,
 };
 
-- 
1.7.1

[RFC PATCH 2/8] misc: ddr: add LPDDR2 data from JESD209-2

[Aneesh V]

add LPDDR2 data from the JEDEC spec JESD209-2. The data
includes:

1. Addressing information for LPDDR2 memories of different
   densities and types(S2/S4)
2. AC timing data.

This data will useful for memory controller device drivers

Signed-off-by: Aneesh V <aneesh@ti.com>
---
 drivers/misc/Kconfig          |    8 ++
 drivers/misc/Makefile         |    1 +
 drivers/misc/jedec_ddr_data.c |  141 +++++++++++++++++++++++++++++++++
 include/linux/jedec_ddr.h     |  174 +++++++++++++++++++++++++++++++++++++++++
 4 files changed, 324 insertions(+), 0 deletions(-)
 create mode 100644 drivers/misc/jedec_ddr_data.c
 create mode 100644 include/linux/jedec_ddr.h

diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 6a1a092..8337bf6 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -451,6 +451,14 @@ config VMWARE_BALLOON
 	  To compile this driver as a module, choose M here: the
 	  module will be called vmw_balloon.
 
+config DDR
+	bool "JEDEC DDR data"
+	help
+	  Data from JEDEC specs for DDR SDRAM memories,
+	  particularly the AC timing parameters and addressing
+	  information. This data is useful for drivers handling
+	  DDR SDRAM controllers.
+
 config ARM_CHARLCD
 	bool "ARM Ltd. Character LCD Driver"
 	depends on PLAT_VERSATILE
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index 3e1d801..4759166 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -36,6 +36,7 @@ obj-$(CONFIG_TI_DAC7512)	+= ti_dac7512.o
 obj-$(CONFIG_C2PORT)		+= c2port/
 obj-$(CONFIG_IWMC3200TOP)      += iwmc3200top/
 obj-$(CONFIG_HMC6352)		+= hmc6352.o
+obj-$(CONFIG_DDR)		+= jedec_ddr_data.o
 obj-y				+= eeprom/
 obj-y				+= cb710/
 obj-$(CONFIG_SPEAR13XX_PCIE_GADGET)	+= spear13xx_pcie_gadget.o
diff --git a/drivers/misc/jedec_ddr_data.c b/drivers/misc/jedec_ddr_data.c
new file mode 100644
index 0000000..299c056
--- /dev/null
+++ b/drivers/misc/jedec_ddr_data.c
@@ -0,0 +1,141 @@
+/*
+ * DDR addressing details and AC timing parameters from JEDEC specs
+ *
+ * Copyright (C) 2010 Texas Instruments, Inc.
+ *
+ * Aneesh V <aneesh@ti.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/jedec_ddr.h>
+#include <linux/module.h>
+
+/* LPDDR2 addressing details from JESD209-2 section 2.4 */
+const struct lpddr2_addressing lpddr2_jedec_addressing_table[] = {
+	{B4, T_REFI_15_6, T_RFC_90}, /* 64M */
+	{B4, T_REFI_15_6, T_RFC_90}, /* 128M */
+	{B4, T_REFI_7_8,  T_RFC_90}, /* 256M */
+	{B4, T_REFI_7_8,  T_RFC_90}, /* 512M */
+	{B8, T_REFI_7_8, T_RFC_130}, /* 1GS4 */
+	{B8, T_REFI_3_9, T_RFC_130}, /* 2GS4 */
+	{B8, T_REFI_3_9, T_RFC_130}, /* 4G */
+	{B8, T_REFI_3_9, T_RFC_210}, /* 8G */
+	{B4, T_REFI_7_8, T_RFC_130}, /* 1GS2 */
+	{B4, T_REFI_3_9, T_RFC_130}, /* 2GS2 */
+};
+EXPORT_SYMBOL(lpddr2_jedec_addressing_table);
+
+/* LPDDR2 AC timing parameters from JESD209-2 section 12 */
+const struct lpddr2_timings lpddr2_jedec_timings[] = {
+	/* Speed bin 400(200 MHz) */
+	[0] = {
+		.max_freq	= 200000000,
+		.min_freq	= 10000000,
+		.tRPab		= 21000,
+		.tRCD		= 18000,
+		.tWR		= 15000,
+		.tRAS_min	= 42000,
+		.tRRD		= 10000,
+		.tWTR		= 10000,
+		.tXP		= 7500,
+		.tRTP		= 7500,
+		.tCKESR		= 15000,
+		.tDQSCK_max	= 5500,
+		.tFAW		= 50000,
+		.tZQCS		= 90000,
+		.tZQCL		= 360000,
+		.tZQinit	= 1000000,
+		.tRAS_max_ns	= 70000,
+		.tRTW		= 7500,
+		.tAONPD		= 1000,
+		.tDQSCK_max_derated = 6000,
+	},
+	/* Speed bin 533(266 MHz) */
+	[1] = {
+		.max_freq	= 266666666,
+		.min_freq	= 10000000,
+		.tRPab		= 21000,
+		.tRCD		= 18000,
+		.tWR		= 15000,
+		.tRAS_min	= 42000,
+		.tRRD		= 10000,
+		.tWTR		= 7500,
+		.tXP		= 7500,
+		.tRTP		= 7500,
+		.tCKESR		= 15000,
+		.tDQSCK_max	= 5500,
+		.tFAW		= 50000,
+		.tZQCS		= 90000,
+		.tZQCL		= 360000,
+		.tZQinit	= 1000000,
+		.tRAS_max_ns	= 70000,
+		.tRTW		= 7500,
+		.tAONPD		= 1000,
+		.tDQSCK_max_derated = 6000,
+	},
+	/* Speed bin 800(400 MHz) */
+	[2] = {
+		.max_freq	= 400000000,
+		.min_freq	= 10000000,
+		.tRPab		= 21000,
+		.tRCD		= 18000,
+		.tWR		= 15000,
+		.tRAS_min	= 42000,
+		.tRRD		= 10000,
+		.tWTR		= 7500,
+		.tXP		= 7500,
+		.tRTP		= 7500,
+		.tCKESR		= 15000,
+		.tDQSCK_max	= 5500,
+		.tFAW		= 50000,
+		.tZQCS		= 90000,
+		.tZQCL		= 360000,
+		.tZQinit	= 1000000,
+		.tRAS_max_ns	= 70000,
+		.tRTW		= 7500,
+		.tAONPD		= 1000,
+		.tDQSCK_max_derated = 6000,
+	},
+	/* Speed bin 1066(533 MHz) */
+	[3] = {
+		.max_freq	= 533333333,
+		.min_freq	= 10000000,
+		.tRPab		= 21000,
+		.tRCD		= 18000,
+		.tWR		= 15000,
+		.tRAS_min	= 42000,
+		.tRRD		= 10000,
+		.tWTR		= 7500,
+		.tXP		= 7500,
+		.tRTP		= 7500,
+		.tCKESR		= 15000,
+		.tDQSCK_max	= 5500,
+		.tFAW		= 50000,
+		.tZQCS		= 90000,
+		.tZQCL		= 360000,
+		.tZQinit	= 1000000,
+		.tRAS_max_ns	= 70000,
+		.tRTW		= 7500,
+		.tAONPD		= 1000,
+		.tDQSCK_max_derated = 5620,
+	},
+};
+EXPORT_SYMBOL(lpddr2_jedec_timings);
+
+const struct lpddr2_min_tck lpddr2_min_tck = {
+	.tRPab		= 3,
+	.tRCD		= 3,
+	.tWR		= 3,
+	.tRASmin	= 3,
+	.tRRD		= 2,
+	.tWTR		= 2,
+	.tXP		= 2,
+	.tRTP		= 2,
+	.tCKE		= 3,
+	.tCKESR		= 3,
+	.tFAW		= 8
+};
+EXPORT_SYMBOL(lpddr2_min_tck);
diff --git a/include/linux/jedec_ddr.h b/include/linux/jedec_ddr.h
new file mode 100644
index 0000000..e3b329a
--- /dev/null
+++ b/include/linux/jedec_ddr.h
@@ -0,0 +1,174 @@
+/*
+ * Definitions for DDR memories based on JEDEC specs
+ *
+ * Copyright (C) 2010 Texas Instruments, Inc.
+ *
+ * Aneesh V <aneesh@ti.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#ifndef __LINUX_JEDEC_DDR_H
+#define __LINUX_JEDEC_DDR_H
+
+#ifndef __ASSEMBLY__
+#include <linux/types.h>
+
+/* DDR Densities */
+#define DDR_DENSITY_64Mb	1
+#define DDR_DENSITY_128Mb	2
+#define DDR_DENSITY_256Mb	3
+#define DDR_DENSITY_512Mb	4
+#define DDR_DENSITY_1Gb		5
+#define DDR_DENSITY_2Gb		6
+#define DDR_DENSITY_4Gb		7
+#define DDR_DENSITY_8Gb		8
+#define DDR_DENSITY_16Gb	9
+#define DDR_DENSITY_32Gb	10
+
+/* DDR type */
+#define DDR_TYPE_DDR2		1
+#define DDR_TYPE_DDR3		2
+#define DDR_TYPE_LPDDR2_S4	3
+#define DDR_TYPE_LPDDR2_S2	4
+#define DDR_TYPE_LPDDR2_NVM	5
+
+/* DDR IO width */
+#define DDR_IO_WIDTH_4		1
+#define DDR_IO_WIDTH_8		2
+#define DDR_IO_WIDTH_16		3
+#define DDR_IO_WIDTH_32		4
+
+/* Number of Row bits */
+#define R9	9
+#define R10	10
+#define R11	11
+#define R12	12
+#define R13	13
+#define R14	14
+#define R15	15
+#define R16	16
+
+/* Number of Column bits */
+#define C7	7
+#define C8	8
+#define C9	9
+#define C10	10
+#define C11	11
+#define C12	12
+
+/* Number of Banks */
+#define B1	0
+#define B2	1
+#define B4	2
+#define B8	3
+
+/* Refresh rate in nano-seconds */
+#define T_REFI_15_6	15600
+#define T_REFI_7_8	7800
+#define T_REFI_3_9	3900
+
+/* tRFC values */
+#define T_RFC_90	90000
+#define T_RFC_110	110000
+#define T_RFC_130	130000
+#define T_RFC_160	160000
+#define T_RFC_210	210000
+#define T_RFC_300	300000
+#define T_RFC_350	350000
+
+/* Mode register numbers */
+#define DDR_MR0		0
+#define DDR_MR1		1
+#define DDR_MR2		2
+#define DDR_MR3		3
+#define DDR_MR4		4
+#define DDR_MR5		5
+#define DDR_MR6		6
+#define DDR_MR7		7
+#define DDR_MR8		8
+#define DDR_MR9		9
+#define DDR_MR10	10
+#define DDR_MR11	11
+#define DDR_MR16	16
+#define DDR_MR17	17
+#define DDR_MR18	18
+
+/*
+ * LPDDR2 related defines
+ */
+
+/* MR4 register fields */
+#define MR4_SDRAM_REF_RATE_SHIFT	0
+#define MR4_SDRAM_REF_RATE_MASK		7
+#define MR4_TUF_SHIFT			7
+#define MR4_TUF_MASK			(1 << 7)
+
+/* MR4 SDRAM Refresh Rate field values */
+#define SDRAM_TEMP_NOMINAL				0x3
+#define SDRAM_TEMP_RESERVED_4				0x4
+#define SDRAM_TEMP_HIGH_DERATE_REFRESH			0x5
+#define SDRAM_TEMP_HIGH_DERATE_REFRESH_AND_TIMINGS	0x6
+#define SDRAM_TEMP_VERY_HIGH_SHUTDOWN			0x7
+
+/* Structure for DDR addressing info from the JEDEC spec */
+struct lpddr2_addressing {
+	u32 num_banks;
+	u32 tREFI_ns;
+	u32 tRFCab_ps;	/* t_RFCab for LPDDR2 */
+};
+
+/*
+ * Structure for timings from the LPDDR2 datasheet
+ * All parameters are in pico seconds(ps) unless explicitly indicated
+ * with a suffix like tRAS_max_ns below
+ */
+struct lpddr2_timings {
+	u32 max_freq;
+	u32 min_freq;
+	u32 tRPab;
+	u32 tRCD;
+	u32 tWR;
+	u32 tRAS_min;
+	u32 tRRD;
+	u32 tWTR;
+	u32 tXP;
+	u32 tRTP;
+	u32 tCKESR;
+	u32 tDQSCK_max;
+	u32 tDQSCK_max_derated;
+	u32 tFAW;
+	u32 tZQCS;
+	u32 tZQCL;
+	u32 tZQinit;
+	u32 tRAS_max_ns;
+	u32 tRTW;
+	u32 tAONPD;
+};
+
+/*
+ * Min value for some parameters in terms of number of tCK cycles(nCK)
+ * Please set to zero parameters that are not valid for a given memory
+ * type
+ */
+struct lpddr2_min_tck {
+	u32 tRPab;
+	u32 tRCD;
+	u32 tWR;
+	u32 tRASmin;
+	u32 tRRD;
+	u32 tWTR;
+	u32 tXP;
+	u32 tRTP;
+	u32 tCKE;
+	u32 tCKESR;
+	u32 tFAW;
+};
+
+extern const struct lpddr2_addressing lpddr2_jedec_addressing_table[11];
+extern const struct lpddr2_timings lpddr2_jedec_timings[4];
+extern const struct lpddr2_min_tck lpddr2_min_tck;
+#endif /* __ASSEMBLY__ */
+
+#endif /* __LINUX_JEDEC_DDR_H */
-- 
1.7.1

[RFC PATCH 3/8] misc: emif: add register definitions for EMIF

[Aneesh V]

Signed-off-by: Aneesh V <aneesh@ti.com>
---
 drivers/misc/emif_regs.h |  461 ++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 461 insertions(+), 0 deletions(-)
 create mode 100644 drivers/misc/emif_regs.h

diff --git a/drivers/misc/emif_regs.h b/drivers/misc/emif_regs.h
new file mode 100644
index 0000000..6ec818a
--- /dev/null
+++ b/drivers/misc/emif_regs.h
@@ -0,0 +1,461 @@
+/*
+ * EMIF registers and bitfields
+ *
+ * Copyright (C) 2009-2011 Texas Instruments, Inc.
+ *
+ * Benoit Cousson (b-cousson@ti.com)
+ *
+ * This file is automatically generated from the OMAP hardware databases.
+ * We respectfully ask that any modifications to this file be coordinated
+ * with the public linux-omap@vger.kernel.org mailing list and the
+ * authors above to ensure that the autogeneration scripts are kept
+ * up-to-date with the file contents.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef __EMIF_REGS_H
+#define __EMIF_REGS_H
+
+/* Registers offset */
+#define EMIF_MODULE_ID_AND_REVISION				0x0000
+#define EMIF_STATUS						0x0004
+#define EMIF_SDRAM_CONFIG					0x0008
+#define EMIF_SDRAM_CONFIG_2					0x000c
+#define EMIF_SDRAM_REFRESH_CONTROL				0x0010
+#define EMIF_SDRAM_REFRESH_CTRL_SHDW				0x0014
+#define EMIF_SDRAM_TIMING_1					0x0018
+#define EMIF_SDRAM_TIMING_1_SHDW				0x001c
+#define EMIF_SDRAM_TIMING_2					0x0020
+#define EMIF_SDRAM_TIMING_2_SHDW				0x0024
+#define EMIF_SDRAM_TIMING_3					0x0028
+#define EMIF_SDRAM_TIMING_3_SHDW				0x002c
+#define EMIF_LPDDR2_NVM_TIMING					0x0030
+#define EMIF_LPDDR2_NVM_TIMING_SHDW				0x0034
+#define EMIF_POWER_MANAGEMENT_CONTROL				0x0038
+#define EMIF_POWER_MANAGEMENT_CTRL_SHDW				0x003c
+#define EMIF_LPDDR2_MODE_REG_DATA				0x0040
+#define EMIF_LPDDR2_MODE_REG_CONFIG				0x0050
+#define EMIF_OCP_CONFIG						0x0054
+#define EMIF_OCP_CONFIG_VALUE_1					0x0058
+#define EMIF_OCP_CONFIG_VALUE_2					0x005c
+#define EMIF_IODFT_TEST_LOGIC_GLOBAL_CONTROL			0x0060
+#define EMIF_IODFT_TEST_LOGIC_CTRL_MISR_RESULT			0x0064
+#define EMIF_IODFT_TEST_LOGIC_ADDRESS_MISR_RESULT		0x0068
+#define EMIF_IODFT_TEST_LOGIC_DATA_MISR_RESULT_1		0x006c
+#define EMIF_IODFT_TEST_LOGIC_DATA_MISR_RESULT_2		0x0070
+#define EMIF_IODFT_TEST_LOGIC_DATA_MISR_RESULT_3		0x0074
+#define EMIF_PERFORMANCE_COUNTER_1				0x0080
+#define EMIF_PERFORMANCE_COUNTER_2				0x0084
+#define EMIF_PERFORMANCE_COUNTER_CONFIG				0x0088
+#define EMIF_PERFORMANCE_COUNTER_MASTER_REGION_SELECT		0x008c
+#define EMIF_PERFORMANCE_COUNTER_TIME				0x0090
+#define EMIF_MISC_REG						0x0094
+#define EMIF_DLL_CALIB_CTRL					0x0098
+#define EMIF_DLL_CALIB_CTRL_SHDW				0x009c
+#define EMIF_END_OF_INTERRUPT					0x00a0
+#define EMIF_SYSTEM_OCP_INTERRUPT_RAW_STATUS			0x00a4
+#define EMIF_LL_OCP_INTERRUPT_RAW_STATUS			0x00a8
+#define EMIF_SYSTEM_OCP_INTERRUPT_STATUS			0x00ac
+#define EMIF_LL_OCP_INTERRUPT_STATUS				0x00b0
+#define EMIF_SYSTEM_OCP_INTERRUPT_ENABLE_SET			0x00b4
+#define EMIF_LL_OCP_INTERRUPT_ENABLE_SET			0x00b8
+#define EMIF_SYSTEM_OCP_INTERRUPT_ENABLE_CLEAR			0x00bc
+#define EMIF_LL_OCP_INTERRUPT_ENABLE_CLEAR			0x00c0
+#define EMIF_SDRAM_OUTPUT_IMPEDANCE_CALIBRATION_CONFIG		0x00c8
+#define EMIF_TEMPERATURE_ALERT_CONFIG				0x00cc
+#define EMIF_OCP_ERROR_LOG					0x00d0
+#define EMIF_READ_WRITE_LEVELING_RAMP_WINDOW			0x00d4
+#define EMIF_READ_WRITE_LEVELING_RAMP_CONTROL			0x00d8
+#define EMIF_READ_WRITE_LEVELING_CONTROL			0x00dc
+#define EMIF_DDR_PHY_CTRL_1					0x00e4
+#define EMIF_DDR_PHY_CTRL_1_SHDW				0x00e8
+#define EMIF_DDR_PHY_CTRL_2					0x00ec
+#define EMIF_PRIORITY_TO_CLASS_OF_SERVICE_MAPPING		0x0100
+#define EMIF_CONNECTION_ID_TO_CLASS_OF_SERVICE_1_MAPPING	0x0104
+#define EMIF_CONNECTION_ID_TO_CLASS_OF_SERVICE_2_MAPPING	0x0108
+#define EMIF_READ_WRITE_EXECUTION_THRESHOLD			0x0120
+#define EMIF_COS_CONFIG						0x0124
+#define EMIF_PHY_STATUS_1					0x0140
+#define EMIF_PHY_STATUS_2					0x0144
+#define EMIF_PHY_STATUS_3					0x0148
+#define EMIF_PHY_STATUS_4					0x014c
+#define EMIF_PHY_STATUS_5					0x0150
+#define EMIF_PHY_STATUS_6					0x0154
+#define EMIF_PHY_STATUS_7					0x0158
+#define EMIF_PHY_STATUS_8					0x015c
+#define EMIF_PHY_STATUS_9					0x0160
+#define EMIF_PHY_STATUS_10					0x0164
+#define EMIF_PHY_STATUS_11					0x0168
+#define EMIF_PHY_STATUS_12					0x016c
+#define EMIF_PHY_STATUS_13					0x0170
+#define EMIF_PHY_STATUS_14					0x0174
+#define EMIF_PHY_STATUS_15					0x0178
+#define EMIF_PHY_STATUS_16					0x017c
+#define EMIF_PHY_STATUS_17					0x0180
+#define EMIF_PHY_STATUS_18					0x0184
+#define EMIF_PHY_STATUS_19					0x0188
+#define EMIF_PHY_STATUS_20					0x018c
+#define EMIF_PHY_STATUS_21					0x0190
+#define EMIF_EXT_PHY_CTRL_1					0x0200
+#define EMIF_EXT_PHY_CTRL_1_SHDW				0x0204
+#define EMIF_EXT_PHY_CTRL_2					0x0208
+#define EMIF_EXT_PHY_CTRL_2_SHDW				0x020c
+#define EMIF_EXT_PHY_CTRL_3					0x0210
+#define EMIF_EXT_PHY_CTRL_3_SHDW				0x0214
+#define EMIF_EXT_PHY_CTRL_4					0x0218
+#define EMIF_EXT_PHY_CTRL_4_SHDW				0x021c
+#define EMIF_EXT_PHY_CTRL_5					0x0220
+#define EMIF_EXT_PHY_CTRL_5_SHDW				0x0224
+#define EMIF_EXT_PHY_CTRL_6					0x0228
+#define EMIF_EXT_PHY_CTRL_6_SHDW				0x022c
+#define EMIF_EXT_PHY_CTRL_7					0x0230
+#define EMIF_EXT_PHY_CTRL_7_SHDW				0x0234
+#define EMIF_EXT_PHY_CTRL_8					0x0238
+#define EMIF_EXT_PHY_CTRL_8_SHDW				0x023c
+#define EMIF_EXT_PHY_CTRL_9					0x0240
+#define EMIF_EXT_PHY_CTRL_9_SHDW				0x0244
+#define EMIF_EXT_PHY_CTRL_10					0x0248
+#define EMIF_EXT_PHY_CTRL_10_SHDW				0x024c
+#define EMIF_EXT_PHY_CTRL_11					0x0250
+#define EMIF_EXT_PHY_CTRL_11_SHDW				0x0254
+#define EMIF_EXT_PHY_CTRL_12					0x0258
+#define EMIF_EXT_PHY_CTRL_12_SHDW				0x025c
+#define EMIF_EXT_PHY_CTRL_13					0x0260
+#define EMIF_EXT_PHY_CTRL_13_SHDW				0x0264
+#define EMIF_EXT_PHY_CTRL_14					0x0268
+#define EMIF_EXT_PHY_CTRL_14_SHDW				0x026c
+#define EMIF_EXT_PHY_CTRL_15					0x0270
+#define EMIF_EXT_PHY_CTRL_15_SHDW				0x0274
+#define EMIF_EXT_PHY_CTRL_16					0x0278
+#define EMIF_EXT_PHY_CTRL_16_SHDW				0x027c
+#define EMIF_EXT_PHY_CTRL_17					0x0280
+#define EMIF_EXT_PHY_CTRL_17_SHDW				0x0284
+#define EMIF_EXT_PHY_CTRL_18					0x0288
+#define EMIF_EXT_PHY_CTRL_18_SHDW				0x028c
+#define EMIF_EXT_PHY_CTRL_19					0x0290
+#define EMIF_EXT_PHY_CTRL_19_SHDW				0x0294
+#define EMIF_EXT_PHY_CTRL_20					0x0298
+#define EMIF_EXT_PHY_CTRL_20_SHDW				0x029c
+#define EMIF_EXT_PHY_CTRL_21					0x02a0
+#define EMIF_EXT_PHY_CTRL_21_SHDW				0x02a4
+#define EMIF_EXT_PHY_CTRL_22					0x02a8
+#define EMIF_EXT_PHY_CTRL_22_SHDW				0x02ac
+#define EMIF_EXT_PHY_CTRL_23					0x02b0
+#define EMIF_EXT_PHY_CTRL_23_SHDW				0x02b4
+#define EMIF_EXT_PHY_CTRL_24					0x02b8
+#define EMIF_EXT_PHY_CTRL_24_SHDW				0x02bc
+#define EMIF_EXT_PHY_CTRL_25					0x02c0
+#define EMIF_EXT_PHY_CTRL_25_SHDW				0x02c4
+#define EMIF_EXT_PHY_CTRL_26					0x02c8
+#define EMIF_EXT_PHY_CTRL_26_SHDW				0x02cc
+#define EMIF_EXT_PHY_CTRL_27					0x02d0
+#define EMIF_EXT_PHY_CTRL_27_SHDW				0x02d4
+#define EMIF_EXT_PHY_CTRL_28					0x02d8
+#define EMIF_EXT_PHY_CTRL_28_SHDW				0x02dc
+#define EMIF_EXT_PHY_CTRL_29					0x02e0
+#define EMIF_EXT_PHY_CTRL_29_SHDW				0x02e4
+#define EMIF_EXT_PHY_CTRL_30					0x02e8
+#define EMIF_EXT_PHY_CTRL_30_SHDW				0x02ec
+
+/* Registers shifts and masks */
+
+/* EMIF_MODULE_ID_AND_REVISION */
+#define SCHEME_SHIFT			30
+#define SCHEME_MASK			(0x3 << 30)
+#define MODULE_ID_SHIFT			16
+#define MODULE_ID_MASK			(0xfff << 16)
+#define RTL_VERSION_SHIFT		11
+#define RTL_VERSION_MASK		(0x1f << 11)
+#define MAJOR_REVISION_SHIFT		8
+#define MAJOR_REVISION_MASK		(0x7 << 8)
+#define MINOR_REVISION_SHIFT		0
+#define MINOR_REVISION_MASK		(0x3f << 0)
+
+/* STATUS */
+#define BE_SHIFT			31
+#define BE_MASK				(1 << 31)
+#define DUAL_CLK_MODE_SHIFT		30
+#define DUAL_CLK_MODE_MASK		(1 << 30)
+#define FAST_INIT_SHIFT			29
+#define FAST_INIT_MASK			(1 << 29)
+#define RDLVLGATETO_SHIFT		6
+#define RDLVLGATETO_MASK		(1 << 6)
+#define RDLVLTO_SHIFT			5
+#define RDLVLTO_MASK			(1 << 5)
+#define WRLVLTO_SHIFT			4
+#define WRLVLTO_MASK			(1 << 4)
+#define PHY_DLL_READY_SHIFT		2
+#define PHY_DLL_READY_MASK		(1 << 2)
+
+/* SDRAM_CONFIG */
+#define SDRAM_TYPE_SHIFT		29
+#define SDRAM_TYPE_MASK			(0x7 << 29)
+#define IBANK_POS_SHIFT			27
+#define IBANK_POS_MASK			(0x3 << 27)
+#define DDR_TERM_SHIFT			24
+#define DDR_TERM_MASK			(0x7 << 24)
+#define DDR2_DDQS_SHIFT			23
+#define DDR2_DDQS_MASK			(1 << 23)
+#define DYN_ODT_SHIFT			21
+#define DYN_ODT_MASK			(0x3 << 21)
+#define DDR_DISABLE_DLL_SHIFT		20
+#define DDR_DISABLE_DLL_MASK		(1 << 20)
+#define SDRAM_DRIVE_SHIFT		18
+#define SDRAM_DRIVE_MASK		(0x3 << 18)
+#define CWL_SHIFT			16
+#define CWL_MASK			(0x3 << 16)
+#define NARROW_MODE_SHIFT		14
+#define NARROW_MODE_MASK		(0x3 << 14)
+#define CL_SHIFT			10
+#define CL_MASK				(0xf << 10)
+#define ROWSIZE_SHIFT			7
+#define ROWSIZE_MASK			(0x7 << 7)
+#define IBANK_SHIFT			4
+#define IBANK_MASK			(0x7 << 4)
+#define EBANK_SHIFT			3
+#define EBANK_MASK			(1 << 3)
+#define PAGESIZE_SHIFT			0
+#define PAGESIZE_MASK			(0x7 << 0)
+
+/* SDRAM_CONFIG_2 */
+#define CS1NVMEN_SHIFT			30
+#define CS1NVMEN_MASK			(1 << 30)
+#define EBANK_POS_SHIFT			27
+#define EBANK_POS_MASK			(1 << 27)
+#define RDBNUM_SHIFT			4
+#define RDBNUM_MASK			(0x3 << 4)
+#define RDBSIZE_SHIFT			0
+#define RDBSIZE_MASK			(0x7 << 0)
+
+/* SDRAM_REFRESH_CONTROL */
+#define INITREF_DIS_SHIFT		31
+#define INITREF_DIS_MASK		(1 << 31)
+#define SRT_SHIFT			29
+#define SRT_MASK			(1 << 29)
+#define ASR_SHIFT			28
+#define ASR_MASK			(1 << 28)
+#define PASR_SHIFT			24
+#define PASR_MASK			(0x7 << 24)
+#define REFRESH_RATE_SHIFT		0
+#define REFRESH_RATE_MASK		(0xffff << 0)
+
+/* SDRAM_TIMING_1 */
+#define T_RTW_SHIFT			29
+#define T_RTW_MASK			(0x7 << 29)
+#define T_RP_SHIFT			25
+#define T_RP_MASK			(0xf << 25)
+#define T_RCD_SHIFT			21
+#define T_RCD_MASK			(0xf << 21)
+#define T_WR_SHIFT			17
+#define T_WR_MASK			(0xf << 17)
+#define T_RAS_SHIFT			12
+#define T_RAS_MASK			(0x1f << 12)
+#define T_RC_SHIFT			6
+#define T_RC_MASK			(0x3f << 6)
+#define T_RRD_SHIFT			3
+#define T_RRD_MASK			(0x7 << 3)
+#define T_WTR_SHIFT			0
+#define T_WTR_MASK			(0x7 << 0)
+
+/* SDRAM_TIMING_2 */
+#define T_XP_SHIFT			28
+#define T_XP_MASK			(0x7 << 28)
+#define T_ODT_SHIFT			25
+#define T_ODT_MASK			(0x7 << 25)
+#define T_XSNR_SHIFT			16
+#define T_XSNR_MASK			(0x1ff << 16)
+#define T_XSRD_SHIFT			6
+#define T_XSRD_MASK			(0x3ff << 6)
+#define T_RTP_SHIFT			3
+#define T_RTP_MASK			(0x7 << 3)
+#define T_CKE_SHIFT			0
+#define T_CKE_MASK			(0x7 << 0)
+
+/* SDRAM_TIMING_3 */
+#define T_PDLL_UL_SHIFT			28
+#define T_PDLL_UL_MASK			(0xf << 28)
+#define T_CSTA_SHIFT			24
+#define T_CSTA_MASK			(0xf << 24)
+#define T_CKESR_SHIFT			21
+#define T_CKESR_MASK			(0x7 << 21)
+#define ZQ_ZQCS_SHIFT			15
+#define ZQ_ZQCS_MASK			(0x3f << 15)
+#define T_TDQSCKMAX_SHIFT		13
+#define T_TDQSCKMAX_MASK		(0x3 << 13)
+#define T_RFC_SHIFT			4
+#define T_RFC_MASK			(0x1ff << 4)
+#define T_RAS_MAX_SHIFT			0
+#define T_RAS_MAX_MASK			(0xf << 0)
+
+/* POWER_MANAGEMENT_CONTROL */
+#define PD_TIM_SHIFT			12
+#define PD_TIM_MASK			(0xf << 12)
+#define DPD_EN_SHIFT			11
+#define DPD_EN_MASK			(1 << 11)
+#define LP_MODE_SHIFT			8
+#define LP_MODE_MASK			(0x7 << 8)
+#define SR_TIM_SHIFT			4
+#define SR_TIM_MASK			(0xf << 4)
+#define CS_TIM_SHIFT			0
+#define CS_TIM_MASK			(0xf << 0)
+
+/* LPDDR2_MODE_REG_DATA */
+#define VALUE_0_SHIFT			0
+#define VALUE_0_MASK			(0x7f << 0)
+
+/* LPDDR2_MODE_REG_CONFIG */
+#define CS_SHIFT			31
+#define CS_MASK				(1 << 31)
+#define REFRESH_EN_SHIFT		30
+#define REFRESH_EN_MASK			(1 << 30)
+#define ADDRESS_SHIFT			0
+#define ADDRESS_MASK			(0xff << 0)
+
+/* OCP_CONFIG */
+#define SYS_THRESH_MAX_SHIFT		24
+#define SYS_THRESH_MAX_MASK		(0xf << 24)
+#define MPU_THRESH_MAX_SHIFT		20
+#define MPU_THRESH_MAX_MASK		(0xf << 20)
+#define LL_THRESH_MAX_SHIFT		16
+#define LL_THRESH_MAX_MASK		(0xf << 16)
+
+/* PERFORMANCE_COUNTER_1 */
+#define COUNTER1_SHIFT			0
+#define COUNTER1_MASK			(0xffffffff << 0)
+
+/* PERFORMANCE_COUNTER_2 */
+#define COUNTER2_SHIFT			0
+#define COUNTER2_MASK			(0xffffffff << 0)
+
+/* PERFORMANCE_COUNTER_CONFIG */
+#define CNTR2_MCONNID_EN_SHIFT		31
+#define CNTR2_MCONNID_EN_MASK		(1 << 31)
+#define CNTR2_REGION_EN_SHIFT		30
+#define CNTR2_REGION_EN_MASK		(1 << 30)
+#define CNTR2_CFG_SHIFT			16
+#define CNTR2_CFG_MASK			(0xf << 16)
+#define CNTR1_MCONNID_EN_SHIFT		15
+#define CNTR1_MCONNID_EN_MASK		(1 << 15)
+#define CNTR1_REGION_EN_SHIFT		14
+#define CNTR1_REGION_EN_MASK		(1 << 14)
+#define CNTR1_CFG_SHIFT			0
+#define CNTR1_CFG_MASK			(0xf << 0)
+
+/* PERFORMANCE_COUNTER_MASTER_REGION_SELECT */
+#define MCONNID2_SHIFT			24
+#define MCONNID2_MASK			(0xff << 24)
+#define REGION_SEL2_SHIFT		16
+#define REGION_SEL2_MASK		(0x3 << 16)
+#define MCONNID1_SHIFT			8
+#define MCONNID1_MASK			(0xff << 8)
+#define REGION_SEL1_SHIFT		0
+#define REGION_SEL1_MASK		(0x3 << 0)
+
+/* PERFORMANCE_COUNTER_TIME */
+#define TOTAL_TIME_SHIFT		0
+#define TOTAL_TIME_MASK			(0xffffffff << 0)
+
+/* DLL_CALIB_CTRL */
+#define ACK_WAIT_SHIFT			16
+#define ACK_WAIT_MASK			(0xf << 16)
+#define DLL_CALIB_INTERVAL_SHIFT	0
+#define DLL_CALIB_INTERVAL_MASK		(0x1ff << 0)
+
+/* END_OF_INTERRUPT */
+#define EOI_SHIFT			0
+#define EOI_MASK			(1 << 0)
+
+/* SYSTEM_OCP_INTERRUPT_RAW_STATUS */
+#define DNV_SYS_SHIFT			2
+#define DNV_SYS_MASK			(1 << 2)
+#define TA_SYS_SHIFT			1
+#define TA_SYS_MASK			(1 << 1)
+#define ERR_SYS_SHIFT			0
+#define ERR_SYS_MASK			(1 << 0)
+
+/* LOW_LATENCY_OCP_INTERRUPT_RAW_STATUS */
+#define DNV_LL_SHIFT			2
+#define DNV_LL_MASK			(1 << 2)
+#define TA_LL_SHIFT			1
+#define TA_LL_MASK			(1 << 1)
+#define ERR_LL_SHIFT			0
+#define ERR_LL_MASK			(1 << 0)
+
+/* SYSTEM_OCP_INTERRUPT_ENABLE_SET */
+#define EN_DNV_SYS_SHIFT		2
+#define EN_DNV_SYS_MASK			(1 << 2)
+#define EN_TA_SYS_SHIFT			1
+#define EN_TA_SYS_MASK			(1 << 1)
+#define EN_ERR_SYS_SHIFT			0
+#define EN_ERR_SYS_MASK			(1 << 0)
+
+/* LOW_LATENCY_OCP_INTERRUPT_ENABLE_SET */
+#define EN_DNV_LL_SHIFT			2
+#define EN_DNV_LL_MASK			(1 << 2)
+#define EN_TA_LL_SHIFT			1
+#define EN_TA_LL_MASK			(1 << 1)
+#define EN_ERR_LL_SHIFT			0
+#define EN_ERR_LL_MASK			(1 << 0)
+
+/* SDRAM_OUTPUT_IMPEDANCE_CALIBRATION_CONFIG */
+#define ZQ_CS1EN_SHIFT			31
+#define ZQ_CS1EN_MASK			(1 << 31)
+#define ZQ_CS0EN_SHIFT			30
+#define ZQ_CS0EN_MASK			(1 << 30)
+#define ZQ_DUALCALEN_SHIFT		29
+#define ZQ_DUALCALEN_MASK		(1 << 29)
+#define ZQ_SFEXITEN_SHIFT		28
+#define ZQ_SFEXITEN_MASK		(1 << 28)
+#define ZQ_ZQINIT_MULT_SHIFT		18
+#define ZQ_ZQINIT_MULT_MASK		(0x3 << 18)
+#define ZQ_ZQCL_MULT_SHIFT		16
+#define ZQ_ZQCL_MULT_MASK		(0x3 << 16)
+#define ZQ_REFINTERVAL_SHIFT		0
+#define ZQ_REFINTERVAL_MASK		(0xffff << 0)
+
+/* TEMPERATURE_ALERT_CONFIG */
+#define TA_CS1EN_SHIFT			31
+#define TA_CS1EN_MASK			(1 << 31)
+#define TA_CS0EN_SHIFT			30
+#define TA_CS0EN_MASK			(1 << 30)
+#define TA_SFEXITEN_SHIFT		28
+#define TA_SFEXITEN_MASK		(1 << 28)
+#define TA_DEVWDT_SHIFT			26
+#define TA_DEVWDT_MASK			(0x3 << 26)
+#define TA_DEVCNT_SHIFT			24
+#define TA_DEVCNT_MASK			(0x3 << 24)
+#define TA_REFINTERVAL_SHIFT		0
+#define TA_REFINTERVAL_MASK		(0x3fffff << 0)
+
+/* OCP_ERROR_LOG */
+#define MADDRSPACE_SHIFT		14
+#define MADDRSPACE_MASK			(0x3 << 14)
+#define MBURSTSEQ_SHIFT			11
+#define MBURSTSEQ_MASK			(0x7 << 11)
+#define MCMD_SHIFT			8
+#define MCMD_MASK			(0x7 << 8)
+#define MCONNID_SHIFT			0
+#define MCONNID_MASK			(0xff << 0)
+
+/* DDR_PHY_CTRL_1 - EMIF4D */
+#define DLL_SLAVE_DLY_CTRL_SHIFT_4D	4
+#define DLL_SLAVE_DLY_CTRL_MASK_4D	(0xFF << 4)
+#define READ_LATENCY_SHIFT_4D		0
+#define READ_LATENCY_MASK_4D		(0xf << 0)
+
+/* DDR_PHY_CTRL_1 - EMIF4D5 */
+#define DLL_HALF_DELAY_SHIFT_4D5	21
+#define DLL_HALF_DELAY_MASK_4D5		(1 << 21)
+#define READ_LATENCY_SHIFT_4D5		0
+#define READ_LATENCY_MASK_4D5		(0x1f << 0)
+
+/* DDR_PHY_CTRL_1_SHDW */
+#define DDR_PHY_CTRL_1_SHDW_SHIFT	5
+#define DDR_PHY_CTRL_1_SHDW_MASK	(0x7ffffff << 5)
+#define READ_LATENCY_SHDW_SHIFT		0
+#define READ_LATENCY_SHDW_MASK		(0x1f << 0)
+
+#endif
-- 
1.7.1

[RFC PATCH 4/8] misc: emif: add basic infrastructure for EMIF driver

[Aneesh V]

EMIF is an SDRAM controller used in various Texas Instruments
SoCs. EMIF supports, based on its revision, one or more of
LPDDR2/DDR2/DDR3 protocols.

Add the basic infrastructure for EMIF driver that includes
driver registration, probe, parsing of platform data etc.

Signed-off-by: Aneesh V <aneesh@ti.com>
---
 drivers/misc/Kconfig  |   12 ++
 drivers/misc/Makefile |    1 +
 drivers/misc/emif.c   |  300 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/emif.h  |  160 ++++++++++++++++++++++++++
 4 files changed, 473 insertions(+), 0 deletions(-)
 create mode 100644 drivers/misc/emif.c
 create mode 100644 include/linux/emif.h

diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 8337bf6..d68184a 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -459,6 +459,18 @@ config DDR
 	  information. This data is useful for drivers handling
 	  DDR SDRAM controllers.
 
+config EMIF
+	tristate "Texas Instruments EMIF driver"
+	select DDR
+	help
+	  This driver is for the EMIF module available in Texas Instruments
+	  SoCs. EMIF is an SDRAM controller that, based on its revision,
+	  supports one or more of DDR2, DDR3, and LPDDR2 SDRAM protocols.
+	  This driver takes care of only LPDDR2 memories presently. The
+	  functions of the driver includes re-configuring AC timing
+	  parameters and other settings during frequency, voltage and
+	  temperature changes
+
 config ARM_CHARLCD
 	bool "ARM Ltd. Character LCD Driver"
 	depends on PLAT_VERSATILE
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index 4759166..076db0f 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -37,6 +37,7 @@ obj-$(CONFIG_C2PORT)		+= c2port/
 obj-$(CONFIG_IWMC3200TOP)      += iwmc3200top/
 obj-$(CONFIG_HMC6352)		+= hmc6352.o
 obj-$(CONFIG_DDR)		+= jedec_ddr_data.o
+obj-$(CONFIG_EMIF)		+= emif.o
 obj-y				+= eeprom/
 obj-y				+= cb710/
 obj-$(CONFIG_SPEAR13XX_PCIE_GADGET)	+= spear13xx_pcie_gadget.o
diff --git a/drivers/misc/emif.c b/drivers/misc/emif.c
new file mode 100644
index 0000000..ba1e3f9
--- /dev/null
+++ b/drivers/misc/emif.c
@@ -0,0 +1,300 @@
+/*
+ * EMIF driver
+ *
+ * Copyright (C) 2010 Texas Instruments, Inc.
+ *
+ * Aneesh V <aneesh@ti.com>
+ * Santosh Shilimkar <santosh.shilimkar@ti.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#include <linux/kernel.h>
+#include <linux/reboot.h>
+#include <linux/emif.h>
+#include <linux/io.h>
+#include <linux/device.h>
+#include <linux/platform_device.h>
+#include <linux/interrupt.h>
+#include <linux/slab.h>
+#include <linux/seq_file.h>
+#include <linux/module.h>
+#include <linux/spinlock.h>
+#include "emif_regs.h"
+
+/**
+ * struct emif_data - Per device static data for driver's use
+ * @duplicate:			Whether the DDR devices attached to this EMIF
+ *				instance are exactly same as that on EMIF1. In
+ *				this case we can save some memory and processing
+ * @temperature_level:		Maximum temperature of LPDDR2 devices attached
+ *				to this EMIF - read from MR4 register. If there
+ *				are two devices attached to this EMIF, this
+ *				value is the maximum of the two temperature
+ *				levels.
+ * @irq:			IRQ number
+ * @lock:			lock for protecting temperature_level and
+ *				associated actions from race conditions
+ * @base:			base address of memory-mapped IO registers.
+ * @dev:			device pointer.
+ * @addressing			table with addressing information from the spec
+ * @regs_cache:			An array of 'struct emif_regs' that stores
+ *				calculated register values for different
+ *				frequencies, to avoid re-calculating them on
+ *				each DVFS transition.
+ * @curr_regs:			The set of register values used in the last
+ *				frequency change (i.e. corresponding to the
+ *				frequency in effect at the moment)
+ * @plat_data:			Pointer to saved platform data.
+ */
+struct emif_data {
+	u8				duplicate;
+	u8				temperature_level;
+	u32				irq;
+	spinlock_t			lock; /* lock to prevent races */
+	void __iomem			*base;
+	struct device			*dev;
+	const struct lpddr2_addressing	*addressing;
+	struct emif_regs		*regs_cache[EMIF_MAX_NUM_FREQUENCIES];
+	struct emif_regs		*curr_regs;
+	struct emif_platform_data	*plat_data;
+};
+
+static struct emif_data *emif1;
+
+static void __exit cleanup_emif(struct emif_data *emif)
+{
+	if (emif) {
+		if (emif->plat_data) {
+			kfree(emif->plat_data->custom_configs);
+			kfree(emif->plat_data->timings);
+			kfree(emif->plat_data->min_tck);
+			kfree(emif->plat_data->device_info);
+			kfree(emif->plat_data);
+		}
+		kfree(emif);
+	}
+}
+
+static void get_default_timings(struct emif_data *emif)
+{
+	struct emif_platform_data *pd = emif->plat_data;
+
+	pd->timings = lpddr2_jedec_timings;
+	pd->timings_arr_size = ARRAY_SIZE(lpddr2_jedec_timings);
+
+	dev_warn(emif->dev, "Using default timings\n");
+}
+
+static int is_dev_data_valid(u32 type, u32 density, u32 io_width, u32 phy_type,
+		u32 ip_rev, struct device *dev)
+{
+	int valid;
+
+	valid = (type == DDR_TYPE_LPDDR2_S4 ||
+			type == DDR_TYPE_LPDDR2_S2)
+		&& (density >= DDR_DENSITY_64Mb
+			&& density <= DDR_DENSITY_8Gb)
+		&& (io_width >= DDR_IO_WIDTH_8
+			&& io_width <= DDR_IO_WIDTH_32);
+
+	/* Combinations of EMIF and PHY revisions that we support today */
+	switch (ip_rev) {
+	case EMIF_4D:
+		valid = valid && (phy_type == EMIF_PHY_TYPE_ATTILAPHY);
+		break;
+	case EMIF_4D5:
+		valid = valid && (phy_type == EMIF_PHY_TYPE_INTELLIPHY);
+		break;
+	default:
+		valid = 0;
+	}
+
+	if (!valid)
+		dev_err(dev, "Invalid DDR details\n");
+	return valid;
+}
+
+static int is_custom_config_valid(struct emif_custom_configs *cust_cfgs,
+		struct device *dev)
+{
+	int valid = 1;
+
+	if ((cust_cfgs->mask & EMIF_CUSTOM_CONFIG_LPMODE) &&
+		(cust_cfgs->lpmode != EMIF_LP_MODE_DISABLE))
+		valid = cust_cfgs->lpmode_freq_threshold &&
+			cust_cfgs->lpmode_timeout_performance &&
+			cust_cfgs->lpmode_timeout_power;
+
+	if (cust_cfgs->mask & EMIF_CUSTOM_CONFIG_TEMP_ALERT_POLL_INTERVAL)
+		valid = valid && cust_cfgs->temp_alert_poll_interval_ms;
+
+	if (!valid)
+		dev_warn(dev, "Invalid custom configs\n");
+
+	return valid;
+}
+
+static struct emif_data * __init get_device_details(
+		struct platform_device *pdev)
+{
+	u32			size;
+	struct emif_data	*emif = NULL;
+	struct ddr_device_info	*dev_info;
+	struct emif_platform_data *pd;
+
+	pd = pdev->dev.platform_data;
+
+	if (!(pd && pd->device_info && is_dev_data_valid(pd->device_info->type,
+			pd->device_info->density, pd->device_info->io_width,
+			pd->phy_type, pd->ip_rev, &pdev->dev)))
+		goto error;
+
+	emif	= kzalloc(sizeof(struct emif_data), GFP_KERNEL);
+	pd	= kmemdup(pd, sizeof(*pd), GFP_KERNEL);
+	dev_info = kmemdup(pd->device_info,
+			sizeof(*pd->device_info), GFP_KERNEL);
+
+	if (!emif || !pd || !dev_info)
+		goto error;
+
+	emif->plat_data		= pd;
+	emif->dev		= &pdev->dev;
+	emif->temperature_level	= SDRAM_TEMP_NOMINAL;
+
+	pd->device_info	= dev_info;
+
+	/*
+	 * For EMIF instances other than EMIF1 see if the devices connected
+	 * are exactly same as on EMIF1(which is typically the case). If so,
+	 * mark it as a duplicate of EMIF1 and skip copying timings data.
+	 * This will save some memory and some computation later.
+	 */
+	emif->duplicate = emif1 && (memcmp(dev_info,
+		emif1->plat_data->device_info,
+		sizeof(struct ddr_device_info)) == 0);
+
+	if (emif->duplicate) {
+		pd->timings = NULL;
+		pd->min_tck = NULL;
+		goto out;
+	}
+
+	/*
+	 * Copy custom configs - ignore allocation error, if any, as
+	 * custom_configs is not very critical
+	 */
+	if (pd->custom_configs)
+		pd->custom_configs = kmemdup(pd->custom_configs,
+			sizeof(*pd->custom_configs), GFP_KERNEL);
+
+	if (pd->custom_configs &&
+	    !is_custom_config_valid(pd->custom_configs, emif->dev)) {
+		kfree(pd->custom_configs);
+		pd->custom_configs = NULL;
+	}
+
+	/*
+	 * Copy timings and min-tck values from platform data. If it is not
+	 * available or if memory allocation fails, use JEDEC defaults
+	 */
+	size = sizeof(struct lpddr2_timings) * pd->timings_arr_size;
+	if (pd->timings)
+		pd->timings = kmemdup(pd->timings, size, GFP_KERNEL);
+
+	if (!pd->timings)
+		get_default_timings(emif);
+
+	if (pd->min_tck)
+		pd->min_tck = kmemdup(pd->min_tck, sizeof(*pd->min_tck),
+				GFP_KERNEL);
+
+	if (!pd->min_tck) {
+		pd->min_tck = &lpddr2_min_tck;
+		dev_warn(emif->dev, "Using default min-tck values\n");
+	}
+
+	goto out;
+
+error:
+	dev_err(&pdev->dev, "get_device_details() failure!!\n");
+	cleanup_emif(emif);
+	return NULL;
+
+out:
+	return emif;
+}
+
+static int __init emif_probe(struct platform_device *pdev)
+{
+	struct emif_data	*emif;
+	struct resource		*res;
+
+	emif = get_device_details(pdev);
+
+	if (!emif)
+		goto error;
+
+	if (!emif1)
+		emif1 = emif;
+
+	/* Save pointers to each other in emif and device structures */
+	emif->dev = &pdev->dev;
+	platform_set_drvdata(pdev, emif);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res)
+		goto error;
+
+	emif->base = ioremap(res->start, SZ_1M);
+	if (!emif->base)
+		goto error;
+
+	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+	if (!res)
+		goto error;
+	emif->irq = res->start;
+
+	dev_info(&pdev->dev, "Device configured with addr = %p and IRQ%d\n",
+			emif->base, emif->irq);
+	return 0;
+
+error:
+	dev_err(&pdev->dev, "probe failure!!\n");
+	cleanup_emif(emif);
+	return -ENODEV;
+}
+
+static int __exit emif_remove(struct platform_device *pdev)
+{
+	struct emif_data *emif = platform_get_drvdata(pdev);
+
+	cleanup_emif(emif);
+
+	return 0;
+}
+
+static struct platform_driver emif_driver = {
+	.remove		= __exit_p(emif_remove),
+	.driver = {
+		.name = "emif",
+	},
+};
+
+static int __init emif_register(void)
+{
+	return platform_driver_probe(&emif_driver, emif_probe);
+}
+
+static void __exit emif_unregister(void)
+{
+	platform_driver_unregister(&emif_driver);
+}
+
+module_init(emif_register);
+module_exit(emif_unregister);
+MODULE_DESCRIPTION("TI EMIF SDRAM Controller Driver");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:emif");
+MODULE_AUTHOR("Texas Instruments Inc");
diff --git a/include/linux/emif.h b/include/linux/emif.h
new file mode 100644
index 0000000..4ddf20b
--- /dev/null
+++ b/include/linux/emif.h
@@ -0,0 +1,160 @@
+/*
+ * Definitions for EMIF SDRAM controller in TI SoCs
+ *
+ * Copyright (C) 2010 Texas Instruments, Inc.
+ *
+ * Aneesh V <aneesh@ti.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#ifndef __LINUX_EMIF_H
+#define __LINUX_EMIF_H
+
+#ifndef __ASSEMBLY__
+#include <linux/jedec_ddr.h>
+
+/*
+ * Maximum number of different frequencies supported by EMIF driver
+ * Determines the number of entries in the pointer array for register
+ * cache
+ */
+#define EMIF_MAX_NUM_FREQUENCIES	6
+
+/* EMIF_PWR_MGMT_CTRL register */
+/* Low power modes */
+#define EMIF_LP_MODE_DISABLE		0
+#define EMIF_LP_MODE_CLOCK_STOP		1
+#define EMIF_LP_MODE_SELF_REFRESH	2
+#define EMIF_LP_MODE_PWR_DN		4
+
+/* Hardware capabilities */
+#define	EMIF_HW_CAPS_LL_INTERFACE	0x00000001
+
+/* EMIF IP Revisions */
+#define	EMIF_4D				1
+#define	EMIF_4D5			2
+
+/* PHY types */
+#define	EMIF_PHY_TYPE_ATTILAPHY		1
+#define	EMIF_PHY_TYPE_INTELLIPHY	2
+
+/* Custom config requests */
+#define EMIF_CUSTOM_CONFIG_LPMODE			0x00000001
+#define EMIF_CUSTOM_CONFIG_TEMP_ALERT_POLL_INTERVAL	0x00000002
+
+/*
+ * Structure containing shadow of important registers in EMIF
+ * The calculation function fills in this structure to be later used for
+ * initialisation and DVFS
+ */
+struct emif_regs {
+	u32 freq;
+	u32 ref_ctrl_shdw;
+	u32 ref_ctrl_shdw_derated;
+	u32 sdram_tim1_shdw;
+	u32 sdram_tim1_shdw_derated;
+	u32 sdram_tim2_shdw;
+	u32 sdram_tim3_shdw;
+	u32 sdram_tim3_shdw_derated;
+	u32 pwr_mgmt_ctrl_shdw;
+	union {
+		u32 read_idle_ctrl_shdw_normal;
+		u32 dll_calib_ctrl_shdw_normal;
+	};
+	union {
+		u32 read_idle_ctrl_shdw_volt_ramp;
+		u32 dll_calib_ctrl_shdw_volt_ramp;
+	};
+
+	u32 phy_ctrl_1_shdw;
+	u32 ext_phy_ctrl_2_shdw;
+	u32 ext_phy_ctrl_3_shdw;
+	u32 ext_phy_ctrl_4_shdw;
+};
+
+/**
+ * struct ddr_device_info - All information about the DDR device except AC
+ *		timing parameters
+ * @type:	Device type (LPDDR2-S4, LPDDR2-S2 etc)
+ * @density:	Device density
+ * @io_width:	Bus width
+ * @cs1_used:	Whether there is a DDR device attached to the second
+ *		chip-select(CS1) of this EMIF instance
+ * @cal_resistors_per_cs: Whether there is one calibration resistor per
+ *		chip-select or whether it's a single one for both
+ * @manufacturer: Manufacturer name string
+ */
+struct ddr_device_info {
+	u32	type;
+	u32	density;
+	u32	io_width;
+	u32	cs1_used;
+	u32	cal_resistors_per_cs;
+	char	manufacturer[10];
+};
+
+/**
+ * struct emif_custom_configs - Custom configuration parameters/policies
+ *		passed from the platform layer
+ * @mask:	Mask to indicate which configs are requested
+ * @lpmode:	LPMODE to be used in PWR_MGMT_CTRL register
+ * @lpmode_timeout_performance: Timeout before LPMODE entry when higher
+ *		performance is desired at the cost of power (typically
+ *		at higher OPPs)
+ * @lpmode_timeout_power: Timeout before LPMODE entry when better power
+ *		savings is desired and performance is not important
+ *		(typically at lower loads indicated by lower OPPs)
+ * @lpmode_freq_threshold: The DDR frequency threshold to identify between
+ *		the above two cases:
+ *		timeout = (freq >= lpmode_freq_threshold) ?
+ *			lpmode_timeout_performance :
+ *			lpmode_timeout_power;
+ * @temp_alert_poll_interval_ms: LPDDR2 MR4 polling interval at nominal
+ *		temperature(in milliseconds). When temperature is high
+ *		polling is done 4 times as frequently.
+ */
+struct emif_custom_configs {
+	u32 mask;
+	u32 lpmode;
+	u32 lpmode_timeout_performance;
+	u32 lpmode_timeout_power;
+	u32 lpmode_freq_threshold;
+	u32 temp_alert_poll_interval_ms;
+};
+
+/**
+ * struct emif_platform_data - Platform data passed on EMIF platform
+ *				device creation. Used by the driver.
+ * @hw_caps:		Hw capabilities of the EMIF IP in the respective SoC
+ * @device_info:	Device info structure containing information such
+ *			as type, bus width, density etc
+ * @timings:		Timings information from device datasheet passed
+ *			as an array of 'struct lpddr2_timings'. Can be NULL
+ *			if if default timings are ok
+ * @timings_arr_size:	Size of the timings array. Depends on the number
+ *			of different frequencies for which timings data
+ *			is provided
+ * @min_tck:		Minimum value of some timing parameters in terms
+ *			of number of cycles. Can be NULL if default values
+ *			are ok
+ * @custom_configs:	Custom configurations requested by SoC or board
+ *			code and the data for them. Can be NULL if default
+ *			configurations done by the driver are ok. See
+ *			documentation for 'struct emif_custom_configs' for
+ *			more details
+ */
+struct emif_platform_data {
+	u32 hw_caps;
+	struct ddr_device_info *device_info;
+	const struct lpddr2_timings *timings;
+	u32 timings_arr_size;
+	const struct lpddr2_min_tck *min_tck;
+	struct emif_custom_configs *custom_configs;
+	u32 ip_rev;
+	u32 phy_type;
+};
+#endif /* __ASSEMBLY__ */
+
+#endif /* __LINUX_EMIF_H */
-- 
1.7.1

[RFC PATCH 5/8] misc: emif: handle frequency and voltage change events

[Aneesh V]

Change SDRAM timings and other settings as necessary
on voltage and frequency changes. We calculate these
register settings based on data from the device data
sheet and inputs such a frequency, voltage state(stable
or ramping), temperature level etc.

Signed-off-by: Aneesh V <aneesh@ti.com>
---
 drivers/misc/emif.c  |  741 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/emif.h |   97 +++++++
 2 files changed, 838 insertions(+), 0 deletions(-)

diff --git a/drivers/misc/emif.c b/drivers/misc/emif.c
index ba1e3f9..36ba6f4 100644
--- a/drivers/misc/emif.c
+++ b/drivers/misc/emif.c
@@ -62,6 +62,527 @@ struct emif_data {
 };
 
 static struct emif_data *emif1;
+static u32 t_ck; /* DDR clock period in ps */
+
+/*
+ * Calculate the period of DDR clock from frequency value
+ */
+static void set_ddr_clk_period(u32 freq)
+{
+	/* Divide 10^12 by frequency to get period in ps */
+	t_ck = (u32)DIV_ROUND_UP_ULL(1000000000000ull, freq);
+}
+
+/*
+ * Get the CL from SDRAM_CONFIG register
+ */
+static u32 get_cl(struct emif_data *emif)
+{
+	u32		cl;
+	void __iomem	*base = emif->base;
+
+	cl = (readl(base + EMIF_SDRAM_CONFIG) & CL_MASK) >> CL_SHIFT;
+
+	return cl;
+}
+
+static void do_freq_update(void)
+{
+	/* TODO: Add an implementation when DVFS framework is available */
+}
+
+/* Find addressing table entry based on the device's type and density */
+static const struct lpddr2_addressing *get_addressing_table(
+	const struct ddr_device_info *device_info)
+{
+	u32		index, type, density;
+
+	type = device_info->type;
+	density = device_info->density;
+
+	switch (type) {
+	case DDR_TYPE_LPDDR2_S4:
+		index = density - 1;
+		break;
+	case DDR_TYPE_LPDDR2_S2:
+		switch (density) {
+		case DDR_DENSITY_1Gb:
+		case DDR_DENSITY_2Gb:
+			index = density + 3;
+			break;
+		default:
+			index = density - 1;
+		}
+		break;
+	default:
+		return NULL;
+	}
+
+	return &lpddr2_jedec_addressing_table[index];
+}
+
+/*
+ * Find the the right timing table from the array of timing
+ * tables of the device using DDR clock frequency
+ */
+static const struct lpddr2_timings *get_timings_table(struct emif_data *emif,
+		u32 freq)
+{
+	u32				i, min, max, freq_nearest;
+	const struct lpddr2_timings	*timings = NULL;
+	const struct lpddr2_timings	*timings_arr = emif->plat_data->timings;
+	struct				device *dev = emif->dev;
+
+	/* Start with a very high frequency - 1GHz */
+	freq_nearest = 1000000000;
+
+	/*
+	 * Find the timings table such that:
+	 *  1. the frequency range covers the required frequency(safe) AND
+	 *  2. the max_freq is closest to the required frequency(optimal)
+	 */
+	for (i = 0; i < emif->plat_data->timings_arr_size; i++) {
+		max = timings_arr[i].max_freq;
+		min = timings_arr[i].min_freq;
+		if ((freq >= min) && (freq <= max) && (max < freq_nearest)) {
+			freq_nearest = max;
+			timings = &timings_arr[i];
+		}
+	}
+
+	if (!timings)
+		dev_err(dev, "Couldn't find timings for - %dHz\n", freq);
+
+	dev_dbg(dev, "timings table: freq %d, speed bin freq %d\n",
+		freq, freq_nearest);
+
+	return timings;
+}
+
+static u32 get_sdram_ref_ctrl_shdw(u32 freq,
+		const struct lpddr2_addressing *addressing)
+{
+	u32 ref_ctrl_shdw = 0, val = 0, freq_khz, t_refi;
+
+	/* Scale down frequency and t_refi to avoid overflow */
+	freq_khz = freq / 1000;
+	t_refi = addressing->tREFI_ns / 100;
+
+	/*
+	 * refresh rate to be set is 'tREFI(in us) * freq in MHz
+	 * division by 10000 to account for change in units
+	 */
+	val = t_refi * freq_khz / 10000;
+	ref_ctrl_shdw |= val << REFRESH_RATE_SHIFT;
+
+	return ref_ctrl_shdw;
+}
+
+static u32 get_sdram_tim_1_shdw(const struct lpddr2_timings *timings,
+		const struct lpddr2_min_tck *min_tck,
+		const struct lpddr2_addressing *addressing,
+		u32 ip_rev)
+{
+	u32 tim1 = 0, val = 0;
+
+	val = max(min_tck->tWTR, DIV_ROUND_UP(timings->tWTR, t_ck)) - 1;
+	tim1 |= val << T_WTR_SHIFT;
+
+	if (addressing->num_banks == B8)
+		val = DIV_ROUND_UP(timings->tFAW, t_ck*4);
+	else
+		val = max(min_tck->tRRD, DIV_ROUND_UP(timings->tRRD, t_ck));
+	tim1 |= (val - 1) << T_RRD_SHIFT;
+
+	val = DIV_ROUND_UP(timings->tRAS_min + timings->tRPab, t_ck) - 1;
+	tim1 |= val << T_RC_SHIFT;
+
+	val = max(min_tck->tRASmin, DIV_ROUND_UP(timings->tRAS_min, t_ck));
+	tim1 |= (val - 1) << T_RAS_SHIFT;
+
+	val = max(min_tck->tWR, DIV_ROUND_UP(timings->tWR, t_ck)) - 1;
+	tim1 |= val << T_WR_SHIFT;
+
+	val = max(min_tck->tRCD, DIV_ROUND_UP(timings->tRCD, t_ck)) - 1;
+	tim1 |= val << T_RCD_SHIFT;
+
+	val = max(min_tck->tRPab, DIV_ROUND_UP(timings->tRPab, t_ck)) - 1;
+	tim1 |= val << T_RP_SHIFT;
+
+	if (ip_rev == EMIF_4D5) {
+		val = DIV_ROUND_UP(timings->tRTW, t_ck) - 1;
+		tim1 |= val << T_RTW_SHIFT;
+	}
+
+	return tim1;
+}
+
+static u32 get_sdram_tim_1_shdw_derated(const struct lpddr2_timings *timings,
+		const struct lpddr2_min_tck *min_tck,
+		const struct lpddr2_addressing *addressing, u32 ip_rev)
+{
+	u32 tim1 = 0, val = 0;
+
+	val = max(min_tck->tWTR, DIV_ROUND_UP(timings->tWTR, t_ck)) - 1;
+	tim1 = val << T_WTR_SHIFT;
+
+	/*
+	 * tFAW is approximately 4 times tRRD. So add 1875*4 = 7500ps
+	 * to tFAW for de-rating
+	 */
+	if (addressing->num_banks == B8) {
+		val = DIV_ROUND_UP(timings->tFAW + 7500, 4 * t_ck) - 1;
+	} else {
+		val = DIV_ROUND_UP(timings->tRRD + 1875, t_ck);
+		val = max(min_tck->tRRD, val) - 1;
+	}
+	tim1 |= val << T_RRD_SHIFT;
+
+	val = DIV_ROUND_UP(timings->tRAS_min + timings->tRPab + 1875, t_ck);
+	tim1 |= (val - 1) << T_RC_SHIFT;
+
+	val = DIV_ROUND_UP(timings->tRAS_min + 1875, t_ck);
+	val = max(min_tck->tRASmin, val) - 1;
+	tim1 |= val << T_RAS_SHIFT;
+
+	val = max(min_tck->tWR, DIV_ROUND_UP(timings->tWR, t_ck)) - 1;
+	tim1 |= val << T_WR_SHIFT;
+
+	val = max(min_tck->tRCD, DIV_ROUND_UP(timings->tRCD + 1875, t_ck));
+	tim1 |= (val - 1) << T_RCD_SHIFT;
+
+	val = max(min_tck->tRPab, DIV_ROUND_UP(timings->tRPab + 1875, t_ck));
+	tim1 |= (val - 1) << T_RP_SHIFT;
+
+	if (ip_rev == EMIF_4D5) {
+		val = DIV_ROUND_UP(timings->tRTW, t_ck) - 1;
+		tim1 |= val << T_RTW_SHIFT;
+	}
+
+	return tim1;
+}
+
+static u32 get_sdram_tim_2_shdw(const struct lpddr2_timings *timings,
+		const struct lpddr2_min_tck *min_tck,
+		const struct lpddr2_addressing *addressing,
+		u32 type, u32 ip_rev)
+{
+	u32 tim2 = 0, val = 0;
+
+	val = min_tck->tCKE - 1;
+	tim2 |= val << T_CKE_SHIFT;
+
+	val = max(min_tck->tRTP, DIV_ROUND_UP(timings->tRTP, t_ck)) - 1;
+	tim2 |= val << T_RTP_SHIFT;
+
+	/* tXSNR = tRFCab_ps + 10 ns(tRFCab_ps for LPDDR2). */
+	val = DIV_ROUND_UP(addressing->tRFCab_ps + 10000, t_ck) - 1;
+	tim2 |= val << T_XSNR_SHIFT;
+
+	/* XSRD same as XSNR for LPDDR2 */
+	tim2 |= val << T_XSRD_SHIFT;
+
+	if (ip_rev == EMIF_4D5) {
+		val = DIV_ROUND_UP(timings->tAONPD, t_ck) - 1;
+		tim2 |= val << T_ODT_SHIFT;
+	}
+
+	val = max(min_tck->tXP, DIV_ROUND_UP(timings->tXP, t_ck)) - 1;
+	tim2 |= val << T_XP_SHIFT;
+
+	return tim2;
+}
+
+static u32 get_sdram_tim_3_shdw(const struct lpddr2_timings *timings,
+		const struct lpddr2_min_tck *min_tck,
+		const struct lpddr2_addressing *addressing,
+		u32 type, u32 ip_rev, u32 derated)
+{
+	u32 tim3 = 0, val = 0;
+
+	val = timings->tRAS_max_ns / addressing->tREFI_ns - 1;
+	val = val > 0xF ? 0xF : val;
+	tim3 |= val << T_RAS_MAX_SHIFT;
+
+	val = DIV_ROUND_UP(addressing->tRFCab_ps, t_ck) - 1;
+	tim3 |= val << T_RFC_SHIFT;
+
+	if (ip_rev == EMIF_4D5)
+		val = DIV_ROUND_UP(timings->tDQSCK_max + 1000, t_ck) - 1;
+	else
+		val = DIV_ROUND_UP(timings->tDQSCK_max, t_ck) - 1;
+
+	tim3 |= val << T_TDQSCKMAX_SHIFT;
+
+	val = DIV_ROUND_UP(timings->tZQCS, t_ck) - 1;
+	tim3 |= val << ZQ_ZQCS_SHIFT;
+
+	val = DIV_ROUND_UP(timings->tCKESR, t_ck);
+	val = max(min_tck->tCKESR, val) - 1;
+	tim3 |= val << T_CKESR_SHIFT;
+
+	if (ip_rev == EMIF_4D5) {
+		tim3 |= (EMIF_T_CSTA - 1) << T_CSTA_SHIFT;
+
+		val = DIV_ROUND_UP(EMIF_T_PDLL_UL, 128) - 1;
+		tim3 |= val << T_PDLL_UL_SHIFT;
+	}
+
+	return tim3;
+}
+
+static u32 get_read_idle_ctrl_shdw(u8 volt_ramp)
+{
+	u32 idle = 0, val = 0;
+
+	/*
+	 * Maximum value in normal conditions and increased frequency
+	 * when voltage is ramping
+	 */
+	if (volt_ramp)
+		val = READ_IDLE_INTERVAL_DVFS / t_ck / 64 - 1;
+	else
+		val = 0x1FF;
+
+	/*
+	 * READ_IDLE_CTRL register in EMIF4D has same offset and fields
+	 * as DLL_CALIB_CTRL in EMIF4D5, so use the same shifts
+	 */
+	idle |= val << DLL_CALIB_INTERVAL_SHIFT;
+	idle |= EMIF_READ_IDLE_LEN_VAL << ACK_WAIT_SHIFT;
+
+	return idle;
+}
+
+static u32 get_dll_calib_ctrl_shdw(u8 volt_ramp)
+{
+	u32 calib = 0, val = 0;
+
+	if (volt_ramp == DDR_VOLTAGE_RAMPING)
+		val = DLL_CALIB_INTERVAL_DVFS / t_ck / 16 - 1;
+	else
+		val = 0; /* Disabled when voltage is stable */
+
+	calib |= val << DLL_CALIB_INTERVAL_SHIFT;
+	calib |= DLL_CALIB_ACK_WAIT_VAL << ACK_WAIT_SHIFT;
+
+	return calib;
+}
+
+static u32 get_ddr_phy_ctrl_1_attilaphy_4d(const struct lpddr2_timings *timings,
+	u32 freq, u8 RL)
+{
+	u32 phy = EMIF_DDR_PHY_CTRL_1_BASE_VAL_ATTILAPHY, val = 0;
+
+	val = RL + DIV_ROUND_UP(timings->tDQSCK_max, t_ck) - 1;
+	phy |= val << READ_LATENCY_SHIFT_4D;
+
+	if (freq <= 100000000)
+		val = EMIF_DLL_SLAVE_DLY_CTRL_100_MHZ_AND_LESS_ATTILAPHY;
+	else if (freq <= 200000000)
+		val = EMIF_DLL_SLAVE_DLY_CTRL_200_MHZ_ATTILAPHY;
+	else
+		val = EMIF_DLL_SLAVE_DLY_CTRL_400_MHZ_ATTILAPHY;
+
+	phy |= val << DLL_SLAVE_DLY_CTRL_SHIFT_4D;
+
+	return phy;
+}
+
+static u32 get_phy_ctrl_1_intelliphy_4d5(u32 freq, u8 cl)
+{
+	u32 phy = EMIF_DDR_PHY_CTRL_1_BASE_VAL_INTELLIPHY, half_delay;
+
+	/*
+	 * DLL operates at 266 MHz. If DDR frequency is near 266 MHz,
+	 * half-delay is not needed else set half-delay
+	 */
+	if (freq >= 265000000 && freq < 267000000)
+		half_delay = 0;
+	else
+		half_delay = 1;
+
+	phy |= half_delay << DLL_HALF_DELAY_SHIFT_4D5;
+	phy |= ((cl + DIV_ROUND_UP(EMIF_PHY_TOTAL_READ_LATENCY_INTELLIPHY_PS,
+			t_ck) - 1) << READ_LATENCY_SHIFT_4D5);
+
+	return phy;
+}
+
+static u32 get_ext_phy_ctrl_2_intelliphy_4d5(void)
+{
+	u32 fifo_we_slave_ratio;
+
+	fifo_we_slave_ratio =  DIV_ROUND_CLOSEST(
+		EMIF_INTELLI_PHY_DQS_GATE_OPENING_DELAY_PS * 256 , t_ck);
+
+	return fifo_we_slave_ratio | fifo_we_slave_ratio << 11 |
+		fifo_we_slave_ratio << 22;
+}
+
+static u32 get_ext_phy_ctrl_3_intelliphy_4d5(void)
+{
+	u32 fifo_we_slave_ratio;
+
+	fifo_we_slave_ratio =  DIV_ROUND_CLOSEST(
+		EMIF_INTELLI_PHY_DQS_GATE_OPENING_DELAY_PS * 256 , t_ck);
+
+	return fifo_we_slave_ratio >> 10 | fifo_we_slave_ratio << 1 |
+		fifo_we_slave_ratio << 12 | fifo_we_slave_ratio << 23;
+}
+
+static u32 get_ext_phy_ctrl_4_intelliphy_4d5(void)
+{
+	u32 fifo_we_slave_ratio;
+
+	fifo_we_slave_ratio =  DIV_ROUND_CLOSEST(
+		EMIF_INTELLI_PHY_DQS_GATE_OPENING_DELAY_PS * 256 , t_ck);
+
+	return fifo_we_slave_ratio >> 9 | fifo_we_slave_ratio << 2 |
+		fifo_we_slave_ratio << 13;
+}
+
+static u32 get_pwr_mgmt_ctrl(u32 freq, struct emif_data *emif, u32 ip_rev)
+{
+	u32 pwr_mgmt_ctrl	= 0, timeout;
+	u32 lpmode		= EMIF_LP_MODE_SELF_REFRESH;
+	u32 timeout_perf	= EMIF_LP_MODE_TIMEOUT_PERFORMANCE;
+	u32 timeout_pwr		= EMIF_LP_MODE_TIMEOUT_POWER;
+	u32 freq_threshold	= EMIF_LP_MODE_FREQ_THRESHOLD;
+
+	struct emif_custom_configs *cust_cfgs = emif->plat_data->custom_configs;
+
+	if (cust_cfgs && (cust_cfgs->mask & EMIF_CUSTOM_CONFIG_LPMODE)) {
+		lpmode		= cust_cfgs->lpmode;
+		timeout_perf	= cust_cfgs->lpmode_timeout_performance;
+		timeout_pwr	= cust_cfgs->lpmode_timeout_power;
+		freq_threshold  = cust_cfgs->lpmode_freq_threshold;
+	}
+
+	/* Timeout based on DDR frequency */
+	timeout = freq >= freq_threshold ? timeout_perf : timeout_pwr;
+
+	/* The value to be set in register is "log2(timeout) - 3" */
+	if (timeout < 16) {
+		timeout = 0;
+	} else {
+		timeout = __fls(timeout) - 3;
+		if (timeout & (timeout - 1))
+			timeout++;
+	}
+
+	switch (lpmode) {
+	case EMIF_LP_MODE_CLOCK_STOP:
+		pwr_mgmt_ctrl = (timeout << CS_TIM_SHIFT) |
+					SR_TIM_MASK | PD_TIM_MASK;
+		break;
+	case EMIF_LP_MODE_SELF_REFRESH:
+		pwr_mgmt_ctrl = (timeout << SR_TIM_SHIFT) |
+					CS_TIM_MASK | PD_TIM_MASK;
+		break;
+	case EMIF_LP_MODE_PWR_DN:
+		pwr_mgmt_ctrl = (timeout << PD_TIM_SHIFT) |
+					CS_TIM_MASK | SR_TIM_MASK;
+		break;
+	case EMIF_LP_MODE_DISABLE:
+	default:
+		pwr_mgmt_ctrl = CS_TIM_MASK |
+					PD_TIM_MASK | SR_TIM_MASK;
+	}
+
+	/* No CS_TIM in EMIF_4D5 */
+	if (ip_rev == EMIF_4D5)
+		pwr_mgmt_ctrl &= ~CS_TIM_MASK;
+
+	pwr_mgmt_ctrl |= lpmode << LP_MODE_SHIFT;
+
+	return pwr_mgmt_ctrl;
+}
+
+/*
+ * Program EMIF shadow registers that are not dependent on temperature
+ * or voltage
+ */
+static void setup_registers(struct emif_data *emif, struct emif_regs *regs)
+{
+	void __iomem	*base = emif->base;
+
+	writel(regs->sdram_tim2_shdw, base + EMIF_SDRAM_TIMING_2_SHDW);
+	writel(regs->phy_ctrl_1_shdw, base + EMIF_DDR_PHY_CTRL_1_SHDW);
+
+	/* Settings specific for EMIF4D5 */
+	if (emif->plat_data->ip_rev != EMIF_4D5)
+		return;
+	writel(regs->ext_phy_ctrl_2_shdw, base + EMIF_EXT_PHY_CTRL_2_SHDW);
+	writel(regs->ext_phy_ctrl_3_shdw, base + EMIF_EXT_PHY_CTRL_3_SHDW);
+	writel(regs->ext_phy_ctrl_4_shdw, base + EMIF_EXT_PHY_CTRL_4_SHDW);
+}
+
+/*
+ * When voltage ramps dll calibration and forced read idle should
+ * happen more often
+ */
+static void setup_volt_sensitive_regs(struct emif_data *emif,
+		struct emif_regs *regs, u32 volt_state)
+{
+	u32		calib_ctrl;
+	void __iomem	*base = emif->base;
+
+	/*
+	 * EMIF_READ_IDLE_CTRL in EMIF4D refers to the same register as
+	 * EMIF_DLL_CALIB_CTRL in EMIF4D5 and dll_calib_ctrl_shadow_*
+	 * is an alias of the respective read_idle_ctrl_shdw_* (members of
+	 * a union). So, the below code takes care of both cases
+	 */
+	if (volt_state == DDR_VOLTAGE_RAMPING)
+		calib_ctrl = regs->dll_calib_ctrl_shdw_volt_ramp;
+	else
+		calib_ctrl = regs->dll_calib_ctrl_shdw_normal;
+
+	writel(calib_ctrl, base + EMIF_DLL_CALIB_CTRL_SHDW);
+}
+
+/*
+ * setup_temperature_sensitive_regs() - set the timings for temperature
+ * sensitive registers. This happens once at initialisation time based
+ * on the temperature at boot time and subsequently based on the temperature
+ * alert interrupt. Temperature alert can happen when the temperature
+ * increases or drops. So this function can have the effect of either
+ * derating the timings or going back to nominal values.
+ */
+static void setup_temperature_sensitive_regs(struct emif_data *emif,
+		struct emif_regs *regs)
+{
+	u32		tim1, tim3, ref_ctrl, type, irqs;
+	void __iomem	*base = emif->base;
+	u32		temperature;
+
+	type = emif->plat_data->device_info->type;
+
+	tim1 = regs->sdram_tim1_shdw;
+	tim3 = regs->sdram_tim3_shdw;
+	ref_ctrl = regs->ref_ctrl_shdw;
+
+	spin_lock_irqsave(&emif->lock, irqs);
+	/* No de-rating for non-lpddr2 devices */
+	if (type != DDR_TYPE_LPDDR2_S2 && type != DDR_TYPE_LPDDR2_S4)
+		goto out;
+
+	temperature_level = emif->temperature_level;
+	if (temperature == SDRAM_TEMP_HIGH_DERATE_REFRESH) {
+		ref_ctrl = regs->ref_ctrl_shdw_derated;
+	} else if (temperature == SDRAM_TEMP_HIGH_DERATE_REFRESH_AND_TIMINGS) {
+		tim1 = regs->sdram_tim1_shdw_derated;
+		tim3 = regs->sdram_tim3_shdw_derated;
+		ref_ctrl = regs->ref_ctrl_shdw_derated;
+	}
+
+out:
+	writel(tim1, base + EMIF_SDRAM_TIMING_1_SHDW);
+	writel(tim3, base + EMIF_SDRAM_TIMING_3_SHDW);
+	writel(ref_ctrl, base + EMIF_SDRAM_REFRESH_CTRL_SHDW);
+	spin_unlock_irqrestore(&emif->lock, irqs);
+}
 
 static void __exit cleanup_emif(struct emif_data *emif)
 {
@@ -239,6 +760,8 @@ static int __init emif_probe(struct platform_device *pdev)
 	if (!emif1)
 		emif1 = emif;
 
+	emif->addressing = get_addressing_table(emif->plat_data->device_info);
+
 	/* Save pointers to each other in emif and device structures */
 	emif->dev = &pdev->dev;
 	platform_set_drvdata(pdev, emif);
@@ -275,6 +798,224 @@ static int __exit emif_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static int get_emif_reg_values(struct emif_data *emif, u32 freq,
+		struct emif_regs *regs)
+{
+	u32				cs1_used, ip_rev, phy_type;
+	u32				cl, type;
+	const struct lpddr2_timings	*timings;
+	const struct lpddr2_min_tck	*min_tck;
+	const struct ddr_device_info	*device_info;
+	const struct lpddr2_addressing	*addressing;
+	struct emif_data		*emif_for_calc;
+	struct device			*dev;
+	const struct emif_custom_configs *custom_configs;
+
+	dev = emif->dev;
+	/*
+	 * If the devices on this EMIF instance is duplicate of EMIF1,
+	 * use EMIF1 details for the calculation
+	 */
+	emif_for_calc	= emif->duplicate ? emif1 : emif;
+	timings		= get_timings_table(emif_for_calc, freq);
+	addressing	= emif_for_calc->addressing;
+	if (!timings || !addressing) {
+		dev_err(dev, "not enough data available for %dHz", freq);
+		return -1;
+	}
+
+	device_info	= emif_for_calc->plat_data->device_info;
+	type		= device_info->type;
+	cs1_used	= device_info->cs1_used;
+	ip_rev		= emif_for_calc->plat_data->ip_rev;
+	phy_type	= emif_for_calc->plat_data->phy_type;
+
+	min_tck		= emif_for_calc->plat_data->min_tck;
+	custom_configs	= emif_for_calc->plat_data->custom_configs;
+
+	set_ddr_clk_period(freq);
+
+	regs->ref_ctrl_shdw = get_sdram_ref_ctrl_shdw(freq, addressing);
+	regs->sdram_tim1_shdw = get_sdram_tim_1_shdw(timings, min_tck,
+			addressing, ip_rev);
+	regs->sdram_tim2_shdw = get_sdram_tim_2_shdw(timings, min_tck,
+			addressing, type, ip_rev);
+	regs->sdram_tim3_shdw = get_sdram_tim_3_shdw(timings, min_tck,
+		addressing, type, ip_rev, EMIF_NORMAL_TIMINGS);
+
+	cl = get_cl(emif);
+
+	if (phy_type == EMIF_PHY_TYPE_ATTILAPHY && ip_rev == EMIF_4D) {
+		regs->phy_ctrl_1_shdw = get_ddr_phy_ctrl_1_attilaphy_4d(
+			timings, freq, cl);
+	} else if (phy_type == EMIF_PHY_TYPE_INTELLIPHY && ip_rev == EMIF_4D5) {
+		regs->phy_ctrl_1_shdw = get_phy_ctrl_1_intelliphy_4d5(freq, cl);
+		regs->ext_phy_ctrl_2_shdw = get_ext_phy_ctrl_2_intelliphy_4d5();
+		regs->ext_phy_ctrl_3_shdw = get_ext_phy_ctrl_3_intelliphy_4d5();
+		regs->ext_phy_ctrl_4_shdw = get_ext_phy_ctrl_4_intelliphy_4d5();
+	} else {
+		return -1;
+	}
+
+	/* Only timeout values in pwr_mgmt_ctrl_shdw register */
+	regs->pwr_mgmt_ctrl_shdw =
+		get_pwr_mgmt_ctrl(freq, emif_for_calc, ip_rev) &
+		(CS_TIM_MASK | SR_TIM_MASK | PD_TIM_MASK);
+
+	if (ip_rev & EMIF_4D) {
+		regs->read_idle_ctrl_shdw_normal =
+			get_read_idle_ctrl_shdw(DDR_VOLTAGE_STABLE);
+
+		regs->read_idle_ctrl_shdw_volt_ramp =
+			get_read_idle_ctrl_shdw(DDR_VOLTAGE_RAMPING);
+	} else if (ip_rev & EMIF_4D5) {
+		regs->dll_calib_ctrl_shdw_normal =
+			get_dll_calib_ctrl_shdw(DDR_VOLTAGE_STABLE);
+
+		regs->dll_calib_ctrl_shdw_volt_ramp =
+			get_dll_calib_ctrl_shdw(DDR_VOLTAGE_RAMPING);
+	}
+
+	if (type == DDR_TYPE_LPDDR2_S2 || type == DDR_TYPE_LPDDR2_S4) {
+		regs->ref_ctrl_shdw_derated = get_sdram_ref_ctrl_shdw(freq / 4,
+			addressing);
+
+		regs->sdram_tim3_shdw_derated = get_sdram_tim_3_shdw(timings,
+			min_tck, addressing, type, ip_rev,
+			EMIF_DERATED_TIMINGS);
+
+		regs->sdram_tim1_shdw_derated =
+			get_sdram_tim_1_shdw_derated(timings, min_tck,
+				addressing, ip_rev);
+	}
+
+	regs->freq = freq;
+
+	return 0;
+}
+
+/*
+ * get_regs() - gets the cached emif_regs structure for a given EMIF instance
+ * given frequency(freq):
+ *
+ * As an optimisation, every EMIF instance other than EMIF1 shares the
+ * register cache with EMIF1 if the devices connected on this instance
+ * are same as that on EMIF1(indicated by the duplicate flag)
+ *
+ * If we do not have an entry corresponding to the frequency given, we
+ * allocate a new entry and calculate the values
+ *
+ * Upon finding the right reg dump, save it in curr_regs. It can be
+ * directly used for thermal de-rating and voltage ramping changes.
+ */
+static struct emif_regs *get_regs(struct emif_data *emif, u32 freq)
+{
+	int			i;
+	struct emif_regs	**regs_cache;
+	struct emif_regs	*regs = NULL;
+	struct device		*dev;
+
+	dev = emif->dev;
+	if (emif->curr_regs && emif->curr_regs->freq == freq) {
+		dev_dbg(dev, "Using curr_regs - %u Hz", freq);
+		return emif->curr_regs;
+	}
+
+	if (emif->duplicate)
+		regs_cache = emif1->regs_cache;
+	else
+		regs_cache = emif->regs_cache;
+
+	for (i = 0; i < EMIF_MAX_NUM_FREQUENCIES && regs_cache[i]; i++) {
+		if (regs_cache[i]->freq == freq) {
+			regs = regs_cache[i];
+			dev_dbg(dev, "Reg dump found in reg cache for %u Hz\n",
+				freq);
+			break;
+		}
+	}
+
+	/*
+	 * If we don't have an entry for this frequency in the cache create one
+	 * and calculate the values
+	 */
+	if (!regs) {
+		regs = kmalloc(sizeof(struct emif_regs), GFP_ATOMIC);
+		if (!regs)
+			return NULL;
+
+		if (get_emif_reg_values(emif, freq, regs)) {
+			kfree(regs);
+			return NULL;
+		}
+
+		/*
+		 * Now look for an un-used entry in the cache and save the
+		 * newly created struct. If there are no free entries
+		 * over-write the last entry
+		 */
+		for (i = 0; i < EMIF_MAX_NUM_FREQUENCIES && regs_cache[i]; i++)
+			;
+
+		if (i >= EMIF_MAX_NUM_FREQUENCIES) {
+			dev_warn(dev, "regs_cache full - reusing a slot!!\n");
+			i = EMIF_MAX_NUM_FREQUENCIES - 1;
+			kfree(regs_cache[i]);
+		}
+		regs_cache[i] = regs;
+	}
+
+	return regs;
+}
+
+/*
+ * Function un-used right now. Will be used later when DVFS framework
+ * is available
+ */
+static void __attribute__((unused)) do_volt_notify_handling(
+		struct emif_data *emif, u32 volt_state)
+{
+	dev_dbg(emif->dev, "voltage notification : %d", volt_state);
+
+	if (!emif->curr_regs) {
+		dev_err(emif->dev,
+			"Volt-notify before registers are ready: %d\n",
+			volt_state);
+		return;
+	}
+
+	setup_volt_sensitive_regs(emif, emif->curr_regs, volt_state);
+	do_freq_update();
+}
+
+/*
+ * Function un-used right now. Will be used later when DVFS framework
+ * is available
+ */
+static void __attribute__((unused)) do_freq_pre_notify_handling(void *emif_data,
+		u32 new_freq)
+{
+	struct emif_regs *regs;
+	struct emif_data *emif = emif_data;
+
+	regs = get_regs(emif, new_freq);
+	if (!regs)
+		return;
+
+	emif->curr_regs = regs;
+
+	/*
+	 * Update the shadow registers:
+	 * Temperature and voltage-ramp sensitive settings are also configured
+	 * in terms of DDR cycles. So, we need to update them too when there
+	 * is a freq change
+	 */
+	dev_dbg(emif->dev, "setting up shadow registers for %uHz", new_freq);
+	setup_registers(emif, regs);
+	setup_temperature_sensitive_regs(emif, regs);
+	setup_volt_sensitive_regs(emif, regs, DDR_VOLTAGE_STABLE);
+}
+
 static struct platform_driver emif_driver = {
 	.remove		= __exit_p(emif_remove),
 	.driver = {
diff --git a/include/linux/emif.h b/include/linux/emif.h
index 4ddf20b..347125f 100644
--- a/include/linux/emif.h
+++ b/include/linux/emif.h
@@ -22,6 +22,49 @@
  */
 #define EMIF_MAX_NUM_FREQUENCIES	6
 
+/* State of the core voltage */
+#define DDR_VOLTAGE_STABLE		0
+#define DDR_VOLTAGE_RAMPING		1
+
+/* Defines for timing De-rating */
+#define EMIF_NORMAL_TIMINGS		0
+#define EMIF_DERATED_TIMINGS		1
+
+/* Length of the forced read idle period in terms of cycles */
+#define EMIF_READ_IDLE_LEN_VAL		5
+
+/*
+ * forced read idle interval to be used when voltage
+ * is changed as part of DVFS/DPS - 1ms
+ */
+#define READ_IDLE_INTERVAL_DVFS		(1*1000000)
+
+/*
+ * Forced read idle interval to be used when voltage is stable
+ * 50us - or maximum value will do
+ */
+#define READ_IDLE_INTERVAL_NORMAL	(50*1000000)
+
+/* DLL calibration interval when voltage is NOT stable - 1us */
+#define DLL_CALIB_INTERVAL_DVFS		(1*1000000)
+
+#define DLL_CALIB_ACK_WAIT_VAL		5
+
+/* Interval between ZQCS commands - hw team recommended value */
+#define EMIF_ZQCS_INTERVAL_US		(50*1000)
+/* Enable ZQ Calibration on exiting Self-refresh */
+#define ZQ_SFEXITEN_ENABLE		1
+/*
+ * ZQ Calibration simultaneously on both chip-selects:
+ * Needs one calibration resistor per CS
+ */
+#define	ZQ_DUALCALEN_DISABLE		0
+#define	ZQ_DUALCALEN_ENABLE		1
+
+#define T_ZQCS_DEFAULT_NS		90
+#define T_ZQCL_DEFAULT_NS		360
+#define T_ZQINIT_DEFAULT_NS		1000
+
 /* EMIF_PWR_MGMT_CTRL register */
 /* Low power modes */
 #define EMIF_LP_MODE_DISABLE		0
@@ -29,6 +72,35 @@
 #define EMIF_LP_MODE_SELF_REFRESH	2
 #define EMIF_LP_MODE_PWR_DN		4
 
+/* DPD_EN */
+#define DPD_DISABLE			0
+#define DPD_ENABLE			1
+
+/*
+ * Default values for the low-power entry to be used if not provided by user.
+ * OMAP4/5 has a hw bug(i735) due to which this value can not be less than 512
+ * Timeout values are in DDR clock 'cycles' and frequency threshold in Hz
+ */
+#define EMIF_LP_MODE_TIMEOUT_PERFORMANCE			2048
+#define EMIF_LP_MODE_TIMEOUT_POWER				512
+#define EMIF_LP_MODE_FREQ_THRESHOLD				400000000
+
+/* DDR_PHY_CTRL_1 values for EMIF4D - ATTILA PHY combination */
+#define EMIF_DDR_PHY_CTRL_1_BASE_VAL_ATTILAPHY			0x049FF000
+#define EMIF_DLL_SLAVE_DLY_CTRL_400_MHZ_ATTILAPHY		0x41
+#define EMIF_DLL_SLAVE_DLY_CTRL_200_MHZ_ATTILAPHY		0x80
+#define EMIF_DLL_SLAVE_DLY_CTRL_100_MHZ_AND_LESS_ATTILAPHY	0xFF
+
+/* DDR_PHY_CTRL_1 values for EMIF4D5 INTELLIPHY combination */
+#define EMIF_DDR_PHY_CTRL_1_BASE_VAL_INTELLIPHY			0x0E084200
+#define EMIF_PHY_TOTAL_READ_LATENCY_INTELLIPHY_PS		10000
+
+/* TEMP_ALERT_CONFIG - corresponding to temp gradient 5 C/s */
+#define TEMP_ALERT_POLL_INTERVAL_DEFAULT_MS			360
+
+#define EMIF_T_CSTA			3
+#define EMIF_T_PDLL_UL			128
+
 /* Hardware capabilities */
 #define	EMIF_HW_CAPS_LL_INTERFACE	0x00000001
 
@@ -44,6 +116,31 @@
 #define EMIF_CUSTOM_CONFIG_LPMODE			0x00000001
 #define EMIF_CUSTOM_CONFIG_TEMP_ALERT_POLL_INTERVAL	0x00000002
 
+/* External PHY control registers magic values */
+#define EMIF_EXT_PHY_CTRL_1_VAL				0x04020080
+#define EMIF_EXT_PHY_CTRL_5_VAL				0x04010040
+#define EMIF_EXT_PHY_CTRL_6_VAL				0x01004010
+#define EMIF_EXT_PHY_CTRL_7_VAL				0x00001004
+#define EMIF_EXT_PHY_CTRL_8_VAL				0x04010040
+#define EMIF_EXT_PHY_CTRL_9_VAL				0x01004010
+#define EMIF_EXT_PHY_CTRL_10_VAL			0x00001004
+#define EMIF_EXT_PHY_CTRL_11_VAL			0x00000000
+#define EMIF_EXT_PHY_CTRL_12_VAL			0x00000000
+#define EMIF_EXT_PHY_CTRL_13_VAL			0x00000000
+#define EMIF_EXT_PHY_CTRL_14_VAL			0x80080080
+#define EMIF_EXT_PHY_CTRL_15_VAL			0x00800800
+#define EMIF_EXT_PHY_CTRL_16_VAL			0x08102040
+#define EMIF_EXT_PHY_CTRL_17_VAL			0x00000001
+#define EMIF_EXT_PHY_CTRL_18_VAL			0x540A8150
+#define EMIF_EXT_PHY_CTRL_19_VAL			0xA81502A0
+#define EMIF_EXT_PHY_CTRL_20_VAL			0x002A0540
+#define EMIF_EXT_PHY_CTRL_21_VAL			0x00000000
+#define EMIF_EXT_PHY_CTRL_22_VAL			0x00000000
+#define EMIF_EXT_PHY_CTRL_23_VAL			0x00000000
+#define EMIF_EXT_PHY_CTRL_24_VAL			0x00000077
+
+#define EMIF_INTELLI_PHY_DQS_GATE_OPENING_DELAY_PS	1200
+
 /*
  * Structure containing shadow of important registers in EMIF
  * The calculation function fills in this structure to be later used for
-- 
1.7.1

[RFC PATCH 6/8] misc: emif: add interrupt and temperature handling

[Aneesh V]

Add an ISR for EMIF that:
	1. reports details of access errors
	2. takes action on thermal events

On thermal events SDRAM timing parameters are
adjusted to ensure safe operation

Also clear all interrupts on shut-down. Pending IRQs
may casue problems during warm-reset.

Signed-off-by: Aneesh V <aneesh@ti.com>
---
 drivers/misc/emif.c |  209 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 207 insertions(+), 2 deletions(-)

diff --git a/drivers/misc/emif.c b/drivers/misc/emif.c
index 36ba6f4..5c2b0ae 100644
--- a/drivers/misc/emif.c
+++ b/drivers/misc/emif.c
@@ -500,6 +500,45 @@ static u32 get_pwr_mgmt_ctrl(u32 freq, struct emif_data *emif, u32 ip_rev)
 }
 
 /*
+ * Get the temperature level of the EMIF instance:
+ * Reads the MR4 register of attached SDRAM parts to find out the temperature
+ * level. If there are two parts attached(one on each CS), then the temperature
+ * level for the EMIF instance is the higher of the two temperatures.
+ */
+static void get_temperature_level(struct emif_data *emif)
+{
+	u32		temp, temperature_level;
+	unsigned long	irqs;
+	void __iomem	*base;
+
+	base = emif->base;
+
+	/* Read mode register 4 */
+	writel(DDR_MR4, base + EMIF_LPDDR2_MODE_REG_CONFIG);
+	temperature_level = readl(base + EMIF_LPDDR2_MODE_REG_DATA);
+	temperature_level = (temperature_level & MR4_SDRAM_REF_RATE_MASK) >>
+				MR4_SDRAM_REF_RATE_SHIFT;
+
+	if (emif->plat_data->device_info->cs1_used) {
+		writel(DDR_MR4 | CS_MASK, base + EMIF_LPDDR2_MODE_REG_CONFIG);
+		temp = readl(base + EMIF_LPDDR2_MODE_REG_DATA);
+		temp = (temp & MR4_SDRAM_REF_RATE_MASK)
+				>> MR4_SDRAM_REF_RATE_SHIFT;
+		temperature_level = max(temp, temperature_level);
+	}
+
+	spin_lock_irqsave(&emif->lock, irqs);
+	/* treat everything less than nominal(3) in MR4 as nominal */
+	if (unlikely(temperature_level < SDRAM_TEMP_NOMINAL))
+		temperature_level = SDRAM_TEMP_NOMINAL;
+
+	/* if we get reserved value in MR4 persist with the existing value */
+	if (likely(temperature_level != SDRAM_TEMP_RESERVED_4))
+		emif->temperature_level = temperature_level;
+	spin_unlock_irqrestore(&emif->lock, irqs);
+}
+
+/*
  * Program EMIF shadow registers that are not dependent on temperature
  * or voltage
  */
@@ -553,7 +592,8 @@ static void setup_volt_sensitive_regs(struct emif_data *emif,
 static void setup_temperature_sensitive_regs(struct emif_data *emif,
 		struct emif_regs *regs)
 {
-	u32		tim1, tim3, ref_ctrl, type, irqs;
+	u32		tim1, tim3, ref_ctrl, type;
+	unsigned long	irqs;
 	void __iomem	*base = emif->base;
 	u32		temperature;
 
@@ -568,7 +608,7 @@ static void setup_temperature_sensitive_regs(struct emif_data *emif,
 	if (type != DDR_TYPE_LPDDR2_S2 && type != DDR_TYPE_LPDDR2_S4)
 		goto out;
 
-	temperature_level = emif->temperature_level;
+	temperature = emif->temperature_level;
 	if (temperature == SDRAM_TEMP_HIGH_DERATE_REFRESH) {
 		ref_ctrl = regs->ref_ctrl_shdw_derated;
 	} else if (temperature == SDRAM_TEMP_HIGH_DERATE_REFRESH_AND_TIMINGS) {
@@ -584,6 +624,153 @@ out:
 	spin_unlock_irqrestore(&emif->lock, irqs);
 }
 
+static irqreturn_t handle_temp_alert(void __iomem *base, struct emif_data *emif)
+{
+	u32		old_temp_level;
+
+	old_temp_level = emif->temperature_level;
+	get_temperature_level(emif);
+
+	if (unlikely(emif->temperature_level == old_temp_level))
+		goto handled;
+
+	if (emif->temperature_level < old_temp_level ||
+		emif->temperature_level == SDRAM_TEMP_VERY_HIGH_SHUTDOWN) {
+		/*
+		 * Temperature coming down - defer handling to thread OR
+		 * Temperature far too high - do kernel_power_off() from
+		 * thread context
+		 */
+		goto thread;
+	} else {
+		/* Temperature is going up - handle immediately */
+		if (!emif->curr_regs) {
+			dev_err(emif->dev, "temperature alert before registers are calculated, not de-rating timings\n");
+			goto handled;
+		}
+		setup_temperature_sensitive_regs(emif, emif->curr_regs);
+		do_freq_update();
+	}
+
+handled:
+	return IRQ_HANDLED;
+thread:
+	return IRQ_WAKE_THREAD;
+}
+
+static irqreturn_t emif_interrupt_handler(int irq, void *dev_id)
+{
+	u32			interrupts;
+	struct emif_data	*emif = dev_id;
+	void __iomem		*base = emif->base;
+	struct device		*dev = emif->dev;
+	irqreturn_t		ret = IRQ_HANDLED;
+
+	/* Save the status and clear it */
+	interrupts = readl(base + EMIF_SYSTEM_OCP_INTERRUPT_STATUS);
+	writel(interrupts, base + EMIF_SYSTEM_OCP_INTERRUPT_STATUS);
+
+	/*
+	 * Handle temperature alert
+	 * Temperature alert should be same for all ports
+	 * So, it's enough to process it only for one of the ports
+	 */
+	if (interrupts & TA_SYS_MASK)
+		ret = handle_temp_alert(base, emif);
+
+	if (interrupts & ERR_SYS_MASK)
+		dev_err(dev, "Access error from SYS port - %x\n", interrupts);
+
+	if (emif->plat_data->hw_caps & EMIF_HW_CAPS_LL_INTERFACE) {
+		/* Save the status and clear it */
+		interrupts = readl(base + EMIF_LL_OCP_INTERRUPT_STATUS);
+		writel(interrupts, base + EMIF_LL_OCP_INTERRUPT_STATUS);
+
+		if (interrupts & ERR_LL_MASK)
+			dev_err(dev, "Access error from LL port - %x\n",
+				interrupts);
+	}
+
+	return ret;
+}
+
+static irqreturn_t emif_threaded_isr(int irq, void *dev_id)
+{
+	struct emif_data	*emif = dev_id;
+
+	if (emif->temperature_level == SDRAM_TEMP_VERY_HIGH_SHUTDOWN) {
+		dev_emerg(emif->dev, "SDRAM temperature exceeds operating limit.. Needs shut down!!!\n");
+		kernel_power_off();
+		return IRQ_HANDLED;
+	}
+
+	if (emif->curr_regs) {
+		setup_temperature_sensitive_regs(emif, emif->curr_regs);
+		do_freq_update();
+	} else {
+		dev_err(emif->dev, "temperature alert before registers are calculated, not de-rating timings\n");
+	}
+
+	return IRQ_HANDLED;
+}
+
+static void clear_all_interrupts(struct emif_data *emif)
+{
+	void __iomem	*base = emif->base;
+
+	writel(readl(base + EMIF_SYSTEM_OCP_INTERRUPT_STATUS),
+		base + EMIF_SYSTEM_OCP_INTERRUPT_STATUS);
+	if (emif->plat_data->hw_caps & EMIF_HW_CAPS_LL_INTERFACE)
+		writel(readl(base + EMIF_LL_OCP_INTERRUPT_STATUS),
+			base + EMIF_LL_OCP_INTERRUPT_STATUS);
+}
+
+static void disable_and_clear_all_interrupts(struct emif_data *emif)
+{
+	void __iomem		*base = emif->base;
+
+	/* Disable all interrupts */
+	writel(readl(base + EMIF_SYSTEM_OCP_INTERRUPT_ENABLE_SET),
+		base + EMIF_SYSTEM_OCP_INTERRUPT_ENABLE_CLEAR);
+	if (emif->plat_data->hw_caps & EMIF_HW_CAPS_LL_INTERFACE)
+		writel(readl(base + EMIF_LL_OCP_INTERRUPT_ENABLE_SET),
+			base + EMIF_LL_OCP_INTERRUPT_ENABLE_CLEAR);
+
+	/* Clear all interrupts */
+	clear_all_interrupts(emif);
+}
+
+static int __init setup_interrupts(struct emif_data *emif)
+{
+	u32		interrupts, type;
+	void __iomem	*base = emif->base;
+
+	type = emif->plat_data->device_info->type;
+
+	clear_all_interrupts(emif);
+
+	/* Enable interrupts for SYS interface */
+	interrupts = EN_ERR_SYS_MASK;
+	if (type == DDR_TYPE_LPDDR2_S2 || type == DDR_TYPE_LPDDR2_S4)
+		interrupts |= EN_TA_SYS_MASK;
+	writel(interrupts, base + EMIF_SYSTEM_OCP_INTERRUPT_ENABLE_SET);
+
+	/* Enable interrupts for LL interface */
+	if (emif->plat_data->hw_caps & EMIF_HW_CAPS_LL_INTERFACE) {
+		/* TA need not be enabled for LL */
+		interrupts = EN_ERR_LL_MASK;
+		writel(interrupts, base + EMIF_LL_OCP_INTERRUPT_ENABLE_SET);
+	}
+
+	/* setup IRQ handlers */
+	return request_threaded_irq(emif->irq,
+				    emif_interrupt_handler,
+				    emif_threaded_isr,
+				    0, dev_name(emif->dev),
+				    emif);
+
+}
+
 static void __exit cleanup_emif(struct emif_data *emif)
 {
 	if (emif) {
@@ -779,6 +966,8 @@ static int __init emif_probe(struct platform_device *pdev)
 		goto error;
 	emif->irq = res->start;
 
+	disable_and_clear_all_interrupts(emif);
+
 	dev_info(&pdev->dev, "Device configured with addr = %p and IRQ%d\n",
 			emif->base, emif->irq);
 	return 0;
@@ -798,6 +987,13 @@ static int __exit emif_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static void emif_shutdown(struct platform_device *pdev)
+{
+	struct emif_data	*emif = platform_get_drvdata(pdev);
+
+	disable_and_clear_all_interrupts(emif);
+}
+
 static int get_emif_reg_values(struct emif_data *emif, u32 freq,
 		struct emif_regs *regs)
 {
@@ -1002,6 +1198,14 @@ static void __attribute__((unused)) do_freq_pre_notify_handling(void *emif_data,
 	if (!regs)
 		return;
 
+	/*
+	 * As soon as registers are calculated for the first time
+	 * setup interrupts. We are ready to handle the thermal events
+	 * now.
+	 */
+	if (!emif->curr_regs)
+		setup_interrupts(emif);
+
 	emif->curr_regs = regs;
 
 	/*
@@ -1018,6 +1222,7 @@ static void __attribute__((unused)) do_freq_pre_notify_handling(void *emif_data,
 
 static struct platform_driver emif_driver = {
 	.remove		= __exit_p(emif_remove),
+	.shutdown	= emif_shutdown,
 	.driver = {
 		.name = "emif",
 	},
-- 
1.7.1

[RFC PATCH 7/8] misc: emif: add one-time settings

[Aneesh V]

Add settings that are not dependent on frequency
or any other transient parameters

Signed-off-by: Aneesh V <aneesh@ti.com>
---
 drivers/misc/emif.c |  147 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 147 insertions(+), 0 deletions(-)

diff --git a/drivers/misc/emif.c b/drivers/misc/emif.c
index 5c2b0ae..9d24cc7 100644
--- a/drivers/misc/emif.c
+++ b/drivers/misc/emif.c
@@ -74,6 +74,24 @@ static void set_ddr_clk_period(u32 freq)
 }
 
 /*
+ * Get bus width used by EMIF. Note that this may be different from the
+ * bus width of the DDR devices used. For instance two 16-bit DDR devices
+ * may be connected to a given CS of EMIF. In this case bus width as far
+ * as EMIF is concerned is 32, where as the DDR bus width is 16 bits.
+ */
+static u32 get_emif_bus_width(struct emif_data *emif)
+{
+	u32		width;
+	void __iomem	*base = emif->base;
+
+	width = (readl(base + EMIF_SDRAM_CONFIG) & NARROW_MODE_MASK)
+			>> NARROW_MODE_SHIFT;
+	width = width == 0 ? 32 : 16;
+
+	return width;
+}
+
+/*
  * Get the CL from SDRAM_CONFIG register
  */
 static u32 get_cl(struct emif_data *emif)
@@ -331,6 +349,70 @@ static u32 get_sdram_tim_3_shdw(const struct lpddr2_timings *timings,
 	return tim3;
 }
 
+static u32 get_zq_config_reg(const struct lpddr2_addressing *addressing,
+		bool cs1_used, bool cal_resistors_per_cs)
+{
+	u32 zq = 0, val = 0;
+
+	val = EMIF_ZQCS_INTERVAL_US * 1000 / addressing->tREFI_ns;
+	zq |= val << ZQ_REFINTERVAL_SHIFT;
+
+	val = DIV_ROUND_UP(T_ZQCL_DEFAULT_NS, T_ZQCS_DEFAULT_NS) - 1;
+	zq |= val << ZQ_ZQCL_MULT_SHIFT;
+
+	val = DIV_ROUND_UP(T_ZQINIT_DEFAULT_NS, T_ZQCL_DEFAULT_NS) - 1;
+	zq |= val << ZQ_ZQINIT_MULT_SHIFT;
+
+	zq |= ZQ_SFEXITEN_ENABLE << ZQ_SFEXITEN_SHIFT;
+
+	if (cal_resistors_per_cs)
+		zq |= ZQ_DUALCALEN_ENABLE << ZQ_DUALCALEN_SHIFT;
+	else
+		zq |= ZQ_DUALCALEN_DISABLE << ZQ_DUALCALEN_SHIFT;
+
+	zq |= ZQ_CS0EN_MASK; /* CS0 is used for sure */
+
+	val = cs1_used ? 1 : 0;
+	zq |= val << ZQ_CS1EN_SHIFT;
+
+	return zq;
+}
+
+static u32 get_temp_alert_config(const struct lpddr2_addressing *addressing,
+		const struct emif_custom_configs *custom_configs, bool cs1_used,
+		u32 sdram_io_width, u32 emif_bus_width)
+{
+	u32 alert = 0, interval, devcnt;
+
+	if (custom_configs && (custom_configs->mask &
+				EMIF_CUSTOM_CONFIG_TEMP_ALERT_POLL_INTERVAL))
+		interval = custom_configs->temp_alert_poll_interval_ms;
+	else
+		interval = TEMP_ALERT_POLL_INTERVAL_DEFAULT_MS;
+
+	interval *= 1000000;			/* Convert to ns */
+	interval /= addressing->tREFI_ns;	/* Convert to refresh cycles */
+	alert |= (interval << TA_REFINTERVAL_SHIFT);
+
+	/*
+	 * sdram_io_width is in 'log2(x) - 1' form. Convert emif_bus_width
+	 * also to this form and subtract to get TA_DEVCNT, which is
+	 * in log2(x) form.
+	 */
+	emif_bus_width = __fls(emif_bus_width) - 1;
+	devcnt = emif_bus_width - sdram_io_width;
+	alert |= devcnt << TA_DEVCNT_SHIFT;
+
+	/* DEVWDT is in 'log2(x) - 3' form */
+	alert |= (sdram_io_width - 2) << TA_DEVWDT_SHIFT;
+
+	alert |= 1 << TA_SFEXITEN_SHIFT;
+	alert |= 1 << TA_CS0EN_SHIFT;
+	alert |= (cs1_used ? 1 : 0) << TA_CS1EN_SHIFT;
+
+	return alert;
+}
+
 static u32 get_read_idle_ctrl_shdw(u8 volt_ramp)
 {
 	u32 idle = 0, val = 0;
@@ -771,6 +853,70 @@ static int __init setup_interrupts(struct emif_data *emif)
 
 }
 
+static void __init emif_onetime_settings(struct emif_data *emif)
+{
+	u32				pwr_mgmt_ctrl, zq, temp_alert_cfg;
+	void __iomem			*base = emif->base;
+	const struct lpddr2_addressing	*addressing;
+	const struct ddr_device_info	*device_info;
+
+	device_info = emif->plat_data->device_info;
+	addressing = get_addressing_table(device_info);
+
+	/*
+	 * Init power management settings
+	 * We don't know the frequency yet. Use a high frequency
+	 * value for a conservative timeout setting
+	 */
+	pwr_mgmt_ctrl = get_pwr_mgmt_ctrl(1000000000, emif,
+			emif->plat_data->ip_rev);
+	writel(pwr_mgmt_ctrl, base + EMIF_POWER_MANAGEMENT_CONTROL);
+
+	/* Init ZQ calibration settings */
+	zq = get_zq_config_reg(addressing, device_info->cs1_used,
+		device_info->cal_resistors_per_cs);
+	writel(zq, base + EMIF_SDRAM_OUTPUT_IMPEDANCE_CALIBRATION_CONFIG);
+
+	/* Check temperature level temperature level*/
+	get_temperature_level(emif);
+	if (emif->temperature_level == SDRAM_TEMP_VERY_HIGH_SHUTDOWN)
+		dev_emerg(emif->dev, "SDRAM temperature exceeds operating limit.. Needs shut down!!!\n");
+
+	/* Init temperature polling */
+	temp_alert_cfg = get_temp_alert_config(addressing,
+		emif->plat_data->custom_configs, device_info->cs1_used,
+		device_info->io_width, get_emif_bus_width(emif));
+	writel(temp_alert_cfg, base + EMIF_TEMPERATURE_ALERT_CONFIG);
+
+	/*
+	 * Program external PHY control registers that are not frequency
+	 * dependent
+	 */
+	if (emif->plat_data->phy_type != EMIF_PHY_TYPE_INTELLIPHY)
+		return;
+	writel(EMIF_EXT_PHY_CTRL_1_VAL, base + EMIF_EXT_PHY_CTRL_1_SHDW);
+	writel(EMIF_EXT_PHY_CTRL_5_VAL, base + EMIF_EXT_PHY_CTRL_5_SHDW);
+	writel(EMIF_EXT_PHY_CTRL_6_VAL, base + EMIF_EXT_PHY_CTRL_6_SHDW);
+	writel(EMIF_EXT_PHY_CTRL_7_VAL, base + EMIF_EXT_PHY_CTRL_7_SHDW);
+	writel(EMIF_EXT_PHY_CTRL_8_VAL, base + EMIF_EXT_PHY_CTRL_8_SHDW);
+	writel(EMIF_EXT_PHY_CTRL_9_VAL, base + EMIF_EXT_PHY_CTRL_9_SHDW);
+	writel(EMIF_EXT_PHY_CTRL_10_VAL, base + EMIF_EXT_PHY_CTRL_10_SHDW);
+	writel(EMIF_EXT_PHY_CTRL_11_VAL, base + EMIF_EXT_PHY_CTRL_11_SHDW);
+	writel(EMIF_EXT_PHY_CTRL_12_VAL, base + EMIF_EXT_PHY_CTRL_12_SHDW);
+	writel(EMIF_EXT_PHY_CTRL_13_VAL, base + EMIF_EXT_PHY_CTRL_13_SHDW);
+	writel(EMIF_EXT_PHY_CTRL_14_VAL, base + EMIF_EXT_PHY_CTRL_14_SHDW);
+	writel(EMIF_EXT_PHY_CTRL_15_VAL, base + EMIF_EXT_PHY_CTRL_15_SHDW);
+	writel(EMIF_EXT_PHY_CTRL_16_VAL, base + EMIF_EXT_PHY_CTRL_16_SHDW);
+	writel(EMIF_EXT_PHY_CTRL_17_VAL, base + EMIF_EXT_PHY_CTRL_17_SHDW);
+	writel(EMIF_EXT_PHY_CTRL_18_VAL, base + EMIF_EXT_PHY_CTRL_18_SHDW);
+	writel(EMIF_EXT_PHY_CTRL_19_VAL, base + EMIF_EXT_PHY_CTRL_19_SHDW);
+	writel(EMIF_EXT_PHY_CTRL_20_VAL, base + EMIF_EXT_PHY_CTRL_20_SHDW);
+	writel(EMIF_EXT_PHY_CTRL_21_VAL, base + EMIF_EXT_PHY_CTRL_21_SHDW);
+	writel(EMIF_EXT_PHY_CTRL_22_VAL, base + EMIF_EXT_PHY_CTRL_22_SHDW);
+	writel(EMIF_EXT_PHY_CTRL_23_VAL, base + EMIF_EXT_PHY_CTRL_23_SHDW);
+	writel(EMIF_EXT_PHY_CTRL_24_VAL, base + EMIF_EXT_PHY_CTRL_24_SHDW);
+}
+
 static void __exit cleanup_emif(struct emif_data *emif)
 {
 	if (emif) {
@@ -967,6 +1113,7 @@ static int __init emif_probe(struct platform_device *pdev)
 	emif->irq = res->start;
 
 	disable_and_clear_all_interrupts(emif);
+	emif_onetime_settings(emif);
 
 	dev_info(&pdev->dev, "Device configured with addr = %p and IRQ%d\n",
 			emif->base, emif->irq);
-- 
1.7.1

[RFC PATCH 8/8] misc: emif: add debugfs entries for emif

[Aneesh V]

Add debug entries for:
	1. calculated registers per frequency
	2. last polled value of MR4(temperature level
	   of LPDDR2 memory)

Signed-off-by: Aneesh V <aneesh@ti.com>
---
 drivers/misc/emif.c |  129 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 129 insertions(+), 0 deletions(-)

diff --git a/drivers/misc/emif.c b/drivers/misc/emif.c
index 9d24cc7..f67a9e7 100644
--- a/drivers/misc/emif.c
+++ b/drivers/misc/emif.c
@@ -18,6 +18,7 @@
 #include <linux/platform_device.h>
 #include <linux/interrupt.h>
 #include <linux/slab.h>
+#include <linux/debugfs.h>
 #include <linux/seq_file.h>
 #include <linux/module.h>
 #include <linux/spinlock.h>
@@ -47,6 +48,7 @@
  *				frequency change (i.e. corresponding to the
  *				frequency in effect at the moment)
  * @plat_data:			Pointer to saved platform data.
+ * @debugfs_root:		dentry to the root folder for EMIF in debugfs
  */
 struct emif_data {
 	u8				duplicate;
@@ -59,11 +61,136 @@ struct emif_data {
 	struct emif_regs		*regs_cache[EMIF_MAX_NUM_FREQUENCIES];
 	struct emif_regs		*curr_regs;
 	struct emif_platform_data	*plat_data;
+	struct dentry			*debugfs_root;
 };
 
 static struct emif_data *emif1;
 static u32 t_ck; /* DDR clock period in ps */
 
+static void do_emif_regdump_show(struct seq_file *s, struct emif_data *emif,
+	struct emif_regs *regs)
+{
+	u32 type = emif->plat_data->device_info->type;
+	u32 ip_rev = emif->plat_data->ip_rev;
+
+	seq_printf(s, "EMIF register cache dump for %dMHz\n",
+		regs->freq/100000);
+
+	seq_printf(s, "ref_ctrl_shdw\t: 0x%08x\n", regs->ref_ctrl_shdw);
+	seq_printf(s, "sdram_tim1_shdw\t: 0x%08x\n", regs->sdram_tim1_shdw);
+	seq_printf(s, "sdram_tim2_shdw\t: 0x%08x\n", regs->sdram_tim2_shdw);
+	seq_printf(s, "sdram_tim3_shdw\t: 0x%08x\n", regs->sdram_tim3_shdw);
+
+	if (ip_rev == EMIF_4D) {
+		seq_printf(s, "read_idle_ctrl_shdw_normal\t: 0x%08x\n",
+			regs->read_idle_ctrl_shdw_normal);
+		seq_printf(s, "read_idle_ctrl_shdw_volt_ramp\t: 0x%08x\n",
+			regs->read_idle_ctrl_shdw_volt_ramp);
+	} else if (ip_rev == EMIF_4D5) {
+		seq_printf(s, "dll_calib_ctrl_shdw_normal\t: 0x%08x\n",
+			regs->dll_calib_ctrl_shdw_normal);
+		seq_printf(s, "dll_calib_ctrl_shdw_volt_ramp\t: 0x%08x\n",
+			regs->dll_calib_ctrl_shdw_volt_ramp);
+	}
+
+	if (type == DDR_TYPE_LPDDR2_S2 || type == DDR_TYPE_LPDDR2_S4) {
+		seq_printf(s, "ref_ctrl_shdw_derated\t: 0x%08x\n",
+			regs->ref_ctrl_shdw_derated);
+		seq_printf(s, "sdram_tim1_shdw_derated\t: 0x%08x\n",
+			regs->sdram_tim1_shdw_derated);
+		seq_printf(s, "sdram_tim3_shdw_derated\t: 0x%08x\n",
+			regs->sdram_tim3_shdw_derated);
+	}
+}
+
+static int emif_regdump_show(struct seq_file *s, void *unused)
+{
+	struct emif_data	*emif	= s->private;
+	struct emif_regs	**regs_cache;
+	int			i;
+
+	if (emif->duplicate)
+		regs_cache = emif1->regs_cache;
+	else
+		regs_cache = emif->regs_cache;
+
+	for (i = 0; i < EMIF_MAX_NUM_FREQUENCIES && regs_cache[i]; i++) {
+		do_emif_regdump_show(s, emif, regs_cache[i]);
+		seq_printf(s, "\n");
+	}
+
+	return 0;
+}
+
+static int emif_regdump_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, emif_regdump_show, inode->i_private);
+}
+
+static const struct file_operations emif_regdump_fops = {
+	.open			= emif_regdump_open,
+	.read			= seq_read,
+	.release		= single_release,
+};
+
+static int emif_mr4_show(struct seq_file *s, void *unused)
+{
+	struct emif_data *emif = s->private;
+
+	seq_printf(s, "MR4=%d\n", emif->temperature_level);
+	return 0;
+}
+
+static int emif_mr4_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, emif_mr4_show, inode->i_private);
+}
+
+static const struct file_operations emif_mr4_fops = {
+	.open			= emif_mr4_open,
+	.read			= seq_read,
+	.release		= single_release,
+};
+
+static int __init emif_debugfs_init(struct emif_data *emif)
+{
+	struct dentry	*dentry;
+	int		ret;
+
+	dentry = debugfs_create_dir(dev_name(emif->dev), NULL);
+	if (IS_ERR(dentry)) {
+		ret = PTR_ERR(dentry);
+		goto err0;
+	}
+	emif->debugfs_root = dentry;
+
+	dentry = debugfs_create_file("regcache_dump", S_IRUGO,
+			emif->debugfs_root, emif, &emif_regdump_fops);
+	if (IS_ERR(dentry)) {
+		ret = PTR_ERR(dentry);
+		goto err1;
+	}
+
+	dentry = debugfs_create_file("mr4", S_IRUGO,
+			emif->debugfs_root, emif, &emif_mr4_fops);
+	if (IS_ERR(dentry)) {
+		ret = PTR_ERR(dentry);
+		goto err1;
+	}
+
+	return 0;
+err1:
+	debugfs_remove_recursive(emif->debugfs_root);
+err0:
+	return ret;
+}
+
+static void __exit emif_debugfs_exit(struct emif_data *emif)
+{
+	debugfs_remove_recursive(emif->debugfs_root);
+	emif->debugfs_root = NULL;
+}
+
 /*
  * Calculate the period of DDR clock from frequency value
  */
@@ -1114,6 +1241,7 @@ static int __init emif_probe(struct platform_device *pdev)
 
 	disable_and_clear_all_interrupts(emif);
 	emif_onetime_settings(emif);
+	emif_debugfs_init(emif);
 
 	dev_info(&pdev->dev, "Device configured with addr = %p and IRQ%d\n",
 			emif->base, emif->irq);
@@ -1129,6 +1257,7 @@ static int __exit emif_remove(struct platform_device *pdev)
 {
 	struct emif_data *emif = platform_get_drvdata(pdev);
 
+	emif_debugfs_exit(emif);
 	cleanup_emif(emif);
 
 	return 0;
-- 
1.7.1

Re: [RFC PATCH 1/8] OMAP4: hwmod: add EMIF hw mod data

[Santosh Shilimkar]

On Saturday 04 February 2012 05:46 PM, Aneesh V wrote:
> From: Benoit Cousson <b-cousson@ti.com>
>
One line of change log will do here.

Regards
santosh

Re: [RFC PATCH 2/8] misc: ddr: add LPDDR2 data from JESD209-2

[Santosh Shilimkar]

On Saturday 04 February 2012 05:46 PM, Aneesh V wrote:
> add LPDDR2 data from the JEDEC spec JESD209-2. The data
> includes:
> 
> 1. Addressing information for LPDDR2 memories of different
>    densities and types(S2/S4)
> 2. AC timing data.
> 
> This data will useful for memory controller device drivers
> 
> Signed-off-by: Aneesh V <aneesh@ti.com>
Looks good to me.

Regards
santosh

Re: [RFC PATCH 2/8] misc: ddr: add LPDDR2 data from JESD209-2

[Santosh Shilimkar]

On Saturday 04 February 2012 05:46 PM, Aneesh V wrote:
> add LPDDR2 data from the JEDEC spec JESD209-2. The data
> includes:
> 
> 1. Addressing information for LPDDR2 memories of different
>    densities and types(S2/S4)
> 2. AC timing data.
> 
> This data will useful for memory controller device drivers
> 
> Signed-off-by: Aneesh V <aneesh@ti.com>
Sorry.. Missed one minor comment.

> ---
>  drivers/misc/Kconfig          |    8 ++
>  drivers/misc/Makefile         |    1 +
>  drivers/misc/jedec_ddr_data.c |  141 +++++++++++++++++++++++++++++++++
>  include/linux/jedec_ddr.h     |  174 +++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 324 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/misc/jedec_ddr_data.c
>  create mode 100644 include/linux/jedec_ddr.h
> 

[...]

> diff --git a/drivers/misc/jedec_ddr_data.c b/drivers/misc/jedec_ddr_data.c
> new file mode 100644
> index 0000000..299c056
> --- /dev/null
> +++ b/drivers/misc/jedec_ddr_data.c
> @@ -0,0 +1,141 @@
> +/*
> + * DDR addressing details and AC timing parameters from JEDEC specs
> + *
> + * Copyright (C) 2010 Texas Instruments, Inc.
Fix the year please. Should be 2012

Re: [RFC PATCH 3/8] misc: emif: add register definitions for EMIF

[Santosh Shilimkar]

On Saturday 04 February 2012 05:46 PM, Aneesh V wrote:
> Signed-off-by: Aneesh V <aneesh@ti.com>
> ---
>  drivers/misc/emif_regs.h |  461 ++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 461 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/misc/emif_regs.h
> 
Changelog please. O.w looks fine to me.

Regards
santosh

Re: [RFC PATCH 1/8] OMAP4: hwmod: add EMIF hw mod data

[Aneesh V]

Santosh,

Thanks for the review.

On Thursday 16 February 2012 03:32 PM, Santosh Shilimkar wrote:
> On Saturday 04 February 2012 05:46 PM, Aneesh V wrote:
>> From: Benoit Cousson<b-cousson@ti.com>
>>
> One line of change log will do here.

Ok. Will add.

br,
Aneesh

Re: [RFC PATCH 2/8] misc: ddr: add LPDDR2 data from JESD209-2

[Aneesh V]

On Thursday 16 February 2012 03:37 PM, Santosh Shilimkar wrote:
> On Saturday 04 February 2012 05:46 PM, Aneesh V wrote:
>> add LPDDR2 data from the JEDEC spec JESD209-2. The data
>> includes:
>>
>> 1. Addressing information for LPDDR2 memories of different
>>     densities and types(S2/S4)
>> 2. AC timing data.
>>
>> This data will useful for memory controller device drivers
>>
>> Signed-off-by: Aneesh V<aneesh@ti.com>
> Sorry.. Missed one minor comment.
>
>> ---
>>   drivers/misc/Kconfig          |    8 ++
>>   drivers/misc/Makefile         |    1 +
>>   drivers/misc/jedec_ddr_data.c |  141 +++++++++++++++++++++++++++++++++
>>   include/linux/jedec_ddr.h     |  174 +++++++++++++++++++++++++++++++++++++++++
>>   4 files changed, 324 insertions(+), 0 deletions(-)
>>   create mode 100644 drivers/misc/jedec_ddr_data.c
>>   create mode 100644 include/linux/jedec_ddr.h
>>
>
> [...]
>
>> diff --git a/drivers/misc/jedec_ddr_data.c b/drivers/misc/jedec_ddr_data.c
>> new file mode 100644
>> index 0000000..299c056
>> --- /dev/null
>> +++ b/drivers/misc/jedec_ddr_data.c
>> @@ -0,0 +1,141 @@
>> +/*
>> + * DDR addressing details and AC timing parameters from JEDEC specs
>> + *
>> + * Copyright (C) 2010 Texas Instruments, Inc.
> Fix the year please. Should be 2012

Ok. Will do.

Re: [RFC PATCH 3/8] misc: emif: add register definitions for EMIF

[Aneesh V]

On Thursday 16 February 2012 03:40 PM, Santosh Shilimkar wrote:
> On Saturday 04 February 2012 05:46 PM, Aneesh V wrote:
>> Signed-off-by: Aneesh V<aneesh@ti.com>
>> ---
>>   drivers/misc/emif_regs.h |  461 ++++++++++++++++++++++++++++++++++++++++++++++
>>   1 files changed, 461 insertions(+), 0 deletions(-)
>>   create mode 100644 drivers/misc/emif_regs.h
>>
> Changelog please. O.w looks fine to me.

Ok. Will add.

thanks,
Aneesh

Re: [RFC PATCH 4/8] misc: emif: add basic infrastructure for EMIF driver

[Santosh Shilimkar]

On Saturday 04 February 2012 05:46 PM, Aneesh V wrote:
> EMIF is an SDRAM controller used in various Texas Instruments
> SoCs. EMIF supports, based on its revision, one or more of
> LPDDR2/DDR2/DDR3 protocols.
> 
> Add the basic infrastructure for EMIF driver that includes
> driver registration, probe, parsing of platform data etc.
>
> Signed-off-by: Aneesh V <aneesh@ti.com>
> ---
>  drivers/misc/Kconfig  |   12 ++
>  drivers/misc/Makefile |    1 +
>  drivers/misc/emif.c   |  300 +++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/emif.h  |  160 ++++++++++++++++++++++++++
>  4 files changed, 473 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/misc/emif.c
>  create mode 100644 include/linux/emif.h
> 
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index 8337bf6..d68184a 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -459,6 +459,18 @@ config DDR
>  	  information. This data is useful for drivers handling
>  	  DDR SDRAM controllers.
>  
> +config EMIF

Add TI prefix here since it's TI IP and not a generic one.

> +	tristate "Texas Instruments EMIF driver"
> +	select DDR
> +	help
> +	  This driver is for the EMIF module available in Texas Instruments
> +	  SoCs. EMIF is an SDRAM controller that, based on its revision,
> +	  supports one or more of DDR2, DDR3, and LPDDR2 SDRAM protocols.
> +	  This driver takes care of only LPDDR2 memories presently. The
> +	  functions of the driver includes re-configuring AC timing
> +	  parameters and other settings during frequency, voltage and
> +	  temperature changes
> +
>  config ARM_CHARLCD
>  	bool "ARM Ltd. Character LCD Driver"
>  	depends on PLAT_VERSATILE
> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> index 4759166..076db0f 100644
> --- a/drivers/misc/Makefile
> +++ b/drivers/misc/Makefile
> @@ -37,6 +37,7 @@ obj-$(CONFIG_C2PORT)		+= c2port/
>  obj-$(CONFIG_IWMC3200TOP)      += iwmc3200top/
>  obj-$(CONFIG_HMC6352)		+= hmc6352.o
>  obj-$(CONFIG_DDR)		+= jedec_ddr_data.o
> +obj-$(CONFIG_EMIF)		+= emif.o
>  obj-y				+= eeprom/
>  obj-y				+= cb710/
>  obj-$(CONFIG_SPEAR13XX_PCIE_GADGET)	+= spear13xx_pcie_gadget.o
> diff --git a/drivers/misc/emif.c b/drivers/misc/emif.c
> new file mode 100644
> index 0000000..ba1e3f9
> --- /dev/null
> +++ b/drivers/misc/emif.c
> @@ -0,0 +1,300 @@
> +/*
> + * EMIF driver
> + *
> + * Copyright (C) 2010 Texas Instruments, Inc.
Fix year. 2012
> + *
> + * Aneesh V <aneesh@ti.com>
> + * Santosh Shilimkar <santosh.shilimkar@ti.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +#include <linux/kernel.h>
> +#include <linux/reboot.h>
> +#include <linux/emif.h>
> +#include <linux/io.h>
> +#include <linux/device.h>
> +#include <linux/platform_device.h>
> +#include <linux/interrupt.h>
> +#include <linux/slab.h>
> +#include <linux/seq_file.h>
> +#include <linux/module.h>
> +#include <linux/spinlock.h>
> +#include "emif_regs.h"
> +
> +/**
> + * struct emif_data - Per device static data for driver's use
> + * @duplicate:			Whether the DDR devices attached to this EMIF
> + *				instance are exactly same as that on EMIF1. In
> + *				this case we can save some memory and processing
> + * @temperature_level:		Maximum temperature of LPDDR2 devices attached
> + *				to this EMIF - read from MR4 register. If there
> + *				are two devices attached to this EMIF, this
> + *				value is the maximum of the two temperature
> + *				levels.
> + * @irq:			IRQ number
> + * @lock:			lock for protecting temperature_level and
> + *				associated actions from race conditions
> + * @base:			base address of memory-mapped IO registers.
> + * @dev:			device pointer.
> + * @addressing			table with addressing information from the spec
> + * @regs_cache:			An array of 'struct emif_regs' that stores
> + *				calculated register values for different
> + *				frequencies, to avoid re-calculating them on
> + *				each DVFS transition.
> + * @curr_regs:			The set of register values used in the last
> + *				frequency change (i.e. corresponding to the
> + *				frequency in effect at the moment)
> + * @plat_data:			Pointer to saved platform data.
> + */
> +struct emif_data {
> +	u8				duplicate;
> +	u8				temperature_level;
> +	u32				irq;
> +	spinlock_t			lock; /* lock to prevent races */
> +	void __iomem			*base;
> +	struct device			*dev;
> +	const struct lpddr2_addressing	*addressing;
> +	struct emif_regs		*regs_cache[EMIF_MAX_NUM_FREQUENCIES];
> +	struct emif_regs		*curr_regs;
> +	struct emif_platform_data	*plat_data;
> +};
> +
> +static struct emif_data *emif1;
> +
> +static void __exit cleanup_emif(struct emif_data *emif)
> +{
> +	if (emif) {
> +		if (emif->plat_data) {
> +			kfree(emif->plat_data->custom_configs);
> +			kfree(emif->plat_data->timings);
> +			kfree(emif->plat_data->min_tck);
> +			kfree(emif->plat_data->device_info);
> +			kfree(emif->plat_data);
> +		}
> +		kfree(emif);
> +	}
> +}
> +
You might want to consider use of devm_kzalloc() kind of
API so that you can get rid of above free stuff.

> +static void get_default_timings(struct emif_data *emif)
> +{
> +	struct emif_platform_data *pd = emif->plat_data;
> +
> +	pd->timings = lpddr2_jedec_timings;
> +	pd->timings_arr_size = ARRAY_SIZE(lpddr2_jedec_timings);
> +
> +	dev_warn(emif->dev, "Using default timings\n");
> +}
Would be good if you add __line__ in all the errors and
warnings. Helps to trace messages faster.

> +
> +static int is_dev_data_valid(u32 type, u32 density, u32 io_width, u32 phy_type,
> +		u32 ip_rev, struct device *dev)
> +{
> +	int valid;
> +
> +	valid = (type == DDR_TYPE_LPDDR2_S4 ||
> +			type == DDR_TYPE_LPDDR2_S2)
> +		&& (density >= DDR_DENSITY_64Mb
> +			&& density <= DDR_DENSITY_8Gb)
> +		&& (io_width >= DDR_IO_WIDTH_8
> +			&& io_width <= DDR_IO_WIDTH_32);
> +
> +	/* Combinations of EMIF and PHY revisions that we support today */
> +	switch (ip_rev) {
> +	case EMIF_4D:
> +		valid = valid && (phy_type == EMIF_PHY_TYPE_ATTILAPHY);
> +		break;
> +	case EMIF_4D5:
> +		valid = valid && (phy_type == EMIF_PHY_TYPE_INTELLIPHY);
> +		break;
> +	default:
> +		valid = 0;
> +	}
> +
> +	if (!valid)
> +		dev_err(dev, "Invalid DDR details\n");
> +	return valid;
Generally return 0 means success. you might want to consider
returning -EINVAL or similar.

> +}
> +
> +static int is_custom_config_valid(struct emif_custom_configs *cust_cfgs,
> +		struct device *dev)
> +{
> +	int valid = 1;
> +
> +	if ((cust_cfgs->mask & EMIF_CUSTOM_CONFIG_LPMODE) &&
> +		(cust_cfgs->lpmode != EMIF_LP_MODE_DISABLE))
> +		valid = cust_cfgs->lpmode_freq_threshold &&
> +			cust_cfgs->lpmode_timeout_performance &&
> +			cust_cfgs->lpmode_timeout_power;
> +
> +	if (cust_cfgs->mask & EMIF_CUSTOM_CONFIG_TEMP_ALERT_POLL_INTERVAL)
> +		valid = valid && cust_cfgs->temp_alert_poll_interval_ms;
> +
> +	if (!valid)
> +		dev_warn(dev, "Invalid custom configs\n");
> +
> +	return valid;
> +}
> +
Ditto

> +static struct emif_data * __init get_device_details(
> +		struct platform_device *pdev)
> +{
> +	u32			size;
> +	struct emif_data	*emif = NULL;
> +	struct ddr_device_info	*dev_info;
> +	struct emif_platform_data *pd;
> +
> +	pd = pdev->dev.platform_data;
> +
extra line
> +	if (!(pd && pd->device_info && is_dev_data_valid(pd->device_info->type,
> +			pd->device_info->density, pd->device_info->io_width,
> +			pd->phy_type, pd->ip_rev, &pdev->dev)))
> +		goto error;
> +
> +	emif	= kzalloc(sizeof(struct emif_data), GFP_KERNEL);
> +	pd	= kmemdup(pd, sizeof(*pd), GFP_KERNEL);
> +	dev_info = kmemdup(pd->device_info,
> +			sizeof(*pd->device_info), GFP_KERNEL);
> +
here too
> +	if (!emif || !pd || !dev_info)
> +		goto error;
> +
> +	emif->plat_data		= pd;
> +	emif->dev		= &pdev->dev;
> +	emif->temperature_level	= SDRAM_TEMP_NOMINAL;
> +
and here
> +	pd->device_info	= dev_info;
> +
> +	/*
> +	 * For EMIF instances other than EMIF1 see if the devices connected
> +	 * are exactly same as on EMIF1(which is typically the case). If so,
> +	 * mark it as a duplicate of EMIF1 and skip copying timings data.
> +	 * This will save some memory and some computation later.
> +	 */
> +	emif->duplicate = emif1 && (memcmp(dev_info,
> +		emif1->plat_data->device_info,
> +		sizeof(struct ddr_device_info)) == 0);
> +
and here
> +	if (emif->duplicate) {
> +		pd->timings = NULL;
> +		pd->min_tck = NULL;
> +		goto out;
> +	}
> +
> +	/*
> +	 * Copy custom configs - ignore allocation error, if any, as
> +	 * custom_configs is not very critical
> +	 */
> +	if (pd->custom_configs)
> +		pd->custom_configs = kmemdup(pd->custom_configs,
> +			sizeof(*pd->custom_configs), GFP_KERNEL);
> +
> +	if (pd->custom_configs &&
> +	    !is_custom_config_valid(pd->custom_configs, emif->dev)) {
> +		kfree(pd->custom_configs);
> +		pd->custom_configs = NULL;
> +	}
> +
> +	/*
> +	 * Copy timings and min-tck values from platform data. If it is not
> +	 * available or if memory allocation fails, use JEDEC defaults
> +	 */
> +	size = sizeof(struct lpddr2_timings) * pd->timings_arr_size;
> +	if (pd->timings)
> +		pd->timings = kmemdup(pd->timings, size, GFP_KERNEL);
> +
> +	if (!pd->timings)
> +		get_default_timings(emif);
> +
> +	if (pd->min_tck)
> +		pd->min_tck = kmemdup(pd->min_tck, sizeof(*pd->min_tck),
> +				GFP_KERNEL);
> +
> +	if (!pd->min_tck) {
else on above if should work, right ?

> +		pd->min_tck = &lpddr2_min_tck;
> +		dev_warn(emif->dev, "Using default min-tck values\n");
> +	}
> +
> +	goto out;
> +
> +error:
> +	dev_err(&pdev->dev, "get_device_details() failure!!\n");
> +	cleanup_emif(emif);
> +	return NULL;
> +
> +out:
> +	return emif;
> +}
> +
> +static int __init emif_probe(struct platform_device *pdev)
> +{
> +	struct emif_data	*emif;
> +	struct resource		*res;
> +
> +	emif = get_device_details(pdev);
> +
extra line

> +	if (!emif)
> +		goto error;
> +
> +	if (!emif1)
> +		emif1 = emif;
> +
> +	/* Save pointers to each other in emif and device structures */
> +	emif->dev = &pdev->dev;
> +	platform_set_drvdata(pdev, emif);
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!res)
> +		goto error;
> +
> +	emif->base = ioremap(res->start, SZ_1M);
> +	if (!emif->base)
> +		goto error;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> +	if (!res)
> +		goto error;
you might want add a line here
> +	emif->irq = res->start;
> +
And remove this one
> +	dev_info(&pdev->dev, "Device configured with addr = %p and IRQ%d\n",
> +			emif->base, emif->irq);
> +	return 0;
> +
> +error:
> +	dev_err(&pdev->dev, "probe failure!!\n");
> +	cleanup_emif(emif);
> +	return -ENODEV;
> +}
> +
> +static int __exit emif_remove(struct platform_device *pdev)
> +{
> +	struct emif_data *emif = platform_get_drvdata(pdev);
> +
> +	cleanup_emif(emif);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver emif_driver = {
> +	.remove		= __exit_p(emif_remove),
> +	.driver = {
> +		.name = "emif",
> +	},
> +};
> +
> +static int __init emif_register(void)
> +{
> +	return platform_driver_probe(&emif_driver, emif_probe);
> +}
> +
> +static void __exit emif_unregister(void)
> +{
> +	platform_driver_unregister(&emif_driver);
> +}
> +
> +module_init(emif_register);
> +module_exit(emif_unregister);
> +MODULE_DESCRIPTION("TI EMIF SDRAM Controller Driver");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("platform:emif");
> +MODULE_AUTHOR("Texas Instruments Inc");
> diff --git a/include/linux/emif.h b/include/linux/emif.h
> new file mode 100644
> index 0000000..4ddf20b
> --- /dev/null
> +++ b/include/linux/emif.h
> @@ -0,0 +1,160 @@
> +/*
> + * Definitions for EMIF SDRAM controller in TI SoCs
> + *
> + * Copyright (C) 2010 Texas Instruments, Inc.
> + *
2012

> + * Aneesh V <aneesh@ti.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +#ifndef __LINUX_EMIF_H
> +#define __LINUX_EMIF_H
> +
> +#ifndef __ASSEMBLY__
> +#include <linux/jedec_ddr.h>
> +
> +/*
> + * Maximum number of different frequencies supported by EMIF driver
> + * Determines the number of entries in the pointer array for register
> + * cache
> + */
> +#define EMIF_MAX_NUM_FREQUENCIES	6
> +
> +/* EMIF_PWR_MGMT_CTRL register */
> +/* Low power modes */
Combine above two line comments in one
comment.

> +#define EMIF_LP_MODE_DISABLE		0
> +#define EMIF_LP_MODE_CLOCK_STOP		1
> +#define EMIF_LP_MODE_SELF_REFRESH	2
> +#define EMIF_LP_MODE_PWR_DN		4
> +
> +/* Hardware capabilities */
> +#define	EMIF_HW_CAPS_LL_INTERFACE	0x00000001
> +
> +/* EMIF IP Revisions */
> +#define	EMIF_4D				1
> +#define	EMIF_4D5			2
> +
> +/* PHY types */
> +#define	EMIF_PHY_TYPE_ATTILAPHY		1
> +#define	EMIF_PHY_TYPE_INTELLIPHY	2
> +
> +/* Custom config requests */
> +#define EMIF_CUSTOM_CONFIG_LPMODE			0x00000001
> +#define EMIF_CUSTOM_CONFIG_TEMP_ALERT_POLL_INTERVAL	0x00000002
> +

try to aling numbers for all the above defines.

With above minor comments, the patch looks fine to me.

Regards
Santosh

Re: [RFC PATCH 5/8] misc: emif: handle frequency and voltage change events

[Santosh Shilimkar]

On Saturday 04 February 2012 05:46 PM, Aneesh V wrote:
> Change SDRAM timings and other settings as necessary
> on voltage and frequency changes. We calculate these
> register settings based on data from the device data
> sheet and inputs such a frequency, voltage state(stable
> or ramping), temperature level etc.
>
May be you want add TBD or FIXME for notifiers when they
are available. Do that in commit log as well as
in the code so that we don't forget about it.

> Signed-off-by: Aneesh V <aneesh@ti.com>
> ---
Rest of the patch looks fine.

Re: [RFC PATCH 6/8] misc: emif: add interrupt and temperature handling

[Santosh Shilimkar]

On Saturday 04 February 2012 05:46 PM, Aneesh V wrote:
> Add an ISR for EMIF that:
> 	1. reports details of access errors
> 	2. takes action on thermal events
> 
> On thermal events SDRAM timing parameters are
> adjusted to ensure safe operation
> 
> Also clear all interrupts on shut-down. Pending IRQs
> may casue problems during warm-reset.
> 
Add some more details like MR4, EMIF polling frequency etc
for better understanding.

> Signed-off-by: Aneesh V <aneesh@ti.com>
> ---
>  drivers/misc/emif.c |  209 ++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 files changed, 207 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/misc/emif.c b/drivers/misc/emif.c
> index 36ba6f4..5c2b0ae 100644
> --- a/drivers/misc/emif.c
> +++ b/drivers/misc/emif.c
> @@ -500,6 +500,45 @@ static u32 get_pwr_mgmt_ctrl(u32 freq, struct emif_data *emif, u32 ip_rev)
>  }
>  
>  /*
> + * Get the temperature level of the EMIF instance:
> + * Reads the MR4 register of attached SDRAM parts to find out the temperature
> + * level. If there are two parts attached(one on each CS), then the temperature
> + * level for the EMIF instance is the higher of the two temperatures.
> + */
> +static void get_temperature_level(struct emif_data *emif)
> +{
> +	u32		temp, temperature_level;
> +	unsigned long	irqs;
> +	void __iomem	*base;
> +
> +	base = emif->base;
> +
> +	/* Read mode register 4 */
> +	writel(DDR_MR4, base + EMIF_LPDDR2_MODE_REG_CONFIG);
> +	temperature_level = readl(base + EMIF_LPDDR2_MODE_REG_DATA);
> +	temperature_level = (temperature_level & MR4_SDRAM_REF_RATE_MASK) >>
> +				MR4_SDRAM_REF_RATE_SHIFT;
> +
> +	if (emif->plat_data->device_info->cs1_used) {
> +		writel(DDR_MR4 | CS_MASK, base + EMIF_LPDDR2_MODE_REG_CONFIG);
> +		temp = readl(base + EMIF_LPDDR2_MODE_REG_DATA);
> +		temp = (temp & MR4_SDRAM_REF_RATE_MASK)
> +				>> MR4_SDRAM_REF_RATE_SHIFT;
> +		temperature_level = max(temp, temperature_level);
> +	}
> +
> +	spin_lock_irqsave(&emif->lock, irqs);
Add a line here.
> +	/* treat everything less than nominal(3) in MR4 as nominal */
> +	if (unlikely(temperature_level < SDRAM_TEMP_NOMINAL))
> +		temperature_level = SDRAM_TEMP_NOMINAL;
> +
> +	/* if we get reserved value in MR4 persist with the existing value */
> +	if (likely(temperature_level != SDRAM_TEMP_RESERVED_4))
> +		emif->temperature_level = temperature_level;
> +	spin_unlock_irqrestore(&emif->lock, irqs);
> +}
> +

rest of the patch looks fine to me

Re: [RFC PATCH 7/8] misc: emif: add one-time settings

[Santosh Shilimkar]

On Saturday 04 February 2012 05:46 PM, Aneesh V wrote:
> Add settings that are not dependent on frequency
> or any other transient parameters
> 
Expand the changelog a bit. One time settings like
SDRAM_CONFIG, PHY_CONTROL, TEMP alert etc.

> Signed-off-by: Aneesh V <aneesh@ti.com>
> ---
Patch looks fine.

Regards
santosh

Re: [RFC PATCH 8/8] misc: emif: add debugfs entries for emif

[Santosh Shilimkar]

On Saturday 04 February 2012 05:46 PM, Aneesh V wrote:
> Add debug entries for:
> 	1. calculated registers per frequency
> 	2. last polled value of MR4(temperature level
> 	   of LPDDR2 memory)
> 
> Signed-off-by: Aneesh V <aneesh@ti.com>

looks good.

Regards
santosh

Re: [RFC PATCH 0/8] Add TI EMIF SDRAM controller driver

[Santosh Shilimkar]

Andrew, Greg,

On Saturday 04 February 2012 05:46 PM, Aneesh V wrote:
> Add a driver for the EMIF SDRAM controller used in TI SoCs
> 
> EMIF is an SDRAM controller that supports, based on its revision,
> one or more of LPDDR2/DDR2/DDR3 protocols.This driver adds support
> for LPDDR2.
> 
> The driver supports the following features:
> - Calculates the DDR AC timing parameters to be set in EMIF
>   registers using data from the device data-sheets and based
>   on the DDR frequency. If data from data-sheets is not available
>   default timing values from the JEDEC spec are used. These
>   will be safe, but not necessarily optimal
> - API for changing timings during DVFS or at boot-up
> - Temperature alert configuration and handling of temperature
>   alerts, if any for LPDDR2 devices
>   * temperature alert is based on periodic polling of MR4 mode
>     register in DDR devices automatically performed by hardware
>   * timings are de-rated and brought back to nominal when
>     temperature raises and falls respectively
> - Cache of calculated register values to avoid re-calculating
>   them
> 
> The driver will need some minor updates when it is eventually
> integrated with DVFS. This can not be done now as DVFS support
> is not available yet in mainline.
> 
> Discussions with Santosh Shilimkar <santosh.shilimkar@ti.com>
> were immensely helpful in shaping up the interfaces. Vibhore Vardhan
> <vvardhan@gmail.com> did the initial code snippet for thermal
> handling.
> 
> Testing: 
> - The driver is tested on OMAP4430 SDP.
> - The driver in a slightly adapted form is also tested on OMAP5.
> - Since mainline kernel doesn't have DVFS support yet,
>   testing was done using a test module.
> - Temperature alert handling was tested with simulated interrupts
>   and faked temperature values as testing all cases in real-life
>   scenarios is difficult.
> 
[...]

>  arch/arm/mach-omap2/omap_hwmod_44xx_data.c |  110 ++
>  drivers/misc/Kconfig                       |   20 +
>  drivers/misc/Makefile                      |    2 +
>  drivers/misc/emif.c                        | 1522 ++++++++++++++++++++++++++++
>  drivers/misc/emif_regs.h                   |  461 +++++++++
>  drivers/misc/jedec_ddr_data.c              |  141 +++
>  include/linux/emif.h                       |  257 +++++
>  include/linux/jedec_ddr.h                  |  174 ++++

Any suggestion on where this driver can reside. It's a memory
controller driver which supports standard DDR functionality
as per JDEC specs including thermal alert. On top of
that it does support DVFS using the TI PRCM IP block.

Regards,
Santosh

Re: [RFC PATCH 2/8] misc: ddr: add LPDDR2 data from JESD209-2

[Alan Cox]

On Thu, 16 Feb 2012 15:57:57 +0530
Aneesh V <aneesh@ti.com> wrote:

> On Thursday 16 February 2012 03:37 PM, Santosh Shilimkar wrote:
> > On Saturday 04 February 2012 05:46 PM, Aneesh V wrote:
> >> add LPDDR2 data from the JEDEC spec JESD209-2. The data
> >> includes:
> >>
> >> 1. Addressing information for LPDDR2 memories of different
> >>     densities and types(S2/S4)
> >> 2. AC timing data.
> >>
> >> This data will useful for memory controller device drivers

I don't think it belongs in drivers/misc. It's not a driver but a
library. lib/ might be a better place for it perhaps ?

Alan

Re: [RFC PATCH 4/8] misc: emif: add basic infrastructure for EMIF driver

[Aneesh V]

On Thursday 16 February 2012 04:03 PM, Santosh Shilimkar wrote:
> On Saturday 04 February 2012 05:46 PM, Aneesh V wrote:
>> EMIF is an SDRAM controller used in various Texas Instruments
>> SoCs. EMIF supports, based on its revision, one or more of
>> LPDDR2/DDR2/DDR3 protocols.
>>
>> Add the basic infrastructure for EMIF driver that includes
>> driver registration, probe, parsing of platform data etc.
>>
>> Signed-off-by: Aneesh V<aneesh@ti.com>
>> ---
>>   drivers/misc/Kconfig  |   12 ++
>>   drivers/misc/Makefile |    1 +
>>   drivers/misc/emif.c   |  300 +++++++++++++++++++++++++++++++++++++++++++++++++
>>   include/linux/emif.h  |  160 ++++++++++++++++++++++++++
>>   4 files changed, 473 insertions(+), 0 deletions(-)
>>   create mode 100644 drivers/misc/emif.c
>>   create mode 100644 include/linux/emif.h
>>
>> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
>> index 8337bf6..d68184a 100644
>> --- a/drivers/misc/Kconfig
>> +++ b/drivers/misc/Kconfig
>> @@ -459,6 +459,18 @@ config DDR
>>   	  information. This data is useful for drivers handling
>>   	  DDR SDRAM controllers.
>>
>> +config EMIF
>
> Add TI prefix here since it's TI IP and not a generic one.

Ok.

>
>> +	tristate "Texas Instruments EMIF driver"
>> +	select DDR
>> +	help
>> +	  This driver is for the EMIF module available in Texas Instruments
>> +	  SoCs. EMIF is an SDRAM controller that, based on its revision,
>> +	  supports one or more of DDR2, DDR3, and LPDDR2 SDRAM protocols.
>> +	  This driver takes care of only LPDDR2 memories presently. The
>> +	  functions of the driver includes re-configuring AC timing
>> +	  parameters and other settings during frequency, voltage and
>> +	  temperature changes
>> +
>>   config ARM_CHARLCD
>>   	bool "ARM Ltd. Character LCD Driver"
>>   	depends on PLAT_VERSATILE
>> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
>> index 4759166..076db0f 100644
>> --- a/drivers/misc/Makefile
>> +++ b/drivers/misc/Makefile
>> @@ -37,6 +37,7 @@ obj-$(CONFIG_C2PORT)		+= c2port/
>>   obj-$(CONFIG_IWMC3200TOP)      += iwmc3200top/
>>   obj-$(CONFIG_HMC6352)		+= hmc6352.o
>>   obj-$(CONFIG_DDR)		+= jedec_ddr_data.o
>> +obj-$(CONFIG_EMIF)		+= emif.o
>>   obj-y				+= eeprom/
>>   obj-y				+= cb710/
>>   obj-$(CONFIG_SPEAR13XX_PCIE_GADGET)	+= spear13xx_pcie_gadget.o
>> diff --git a/drivers/misc/emif.c b/drivers/misc/emif.c
>> new file mode 100644
>> index 0000000..ba1e3f9
>> --- /dev/null
>> +++ b/drivers/misc/emif.c
>> @@ -0,0 +1,300 @@
>> +/*
>> + * EMIF driver
>> + *
>> + * Copyright (C) 2010 Texas Instruments, Inc.
> Fix year. 2012
>> + *
>> + * Aneesh V<aneesh@ti.com>
>> + * Santosh Shilimkar<santosh.shilimkar@ti.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +#include<linux/kernel.h>
>> +#include<linux/reboot.h>
>> +#include<linux/emif.h>
>> +#include<linux/io.h>
>> +#include<linux/device.h>
>> +#include<linux/platform_device.h>
>> +#include<linux/interrupt.h>
>> +#include<linux/slab.h>
>> +#include<linux/seq_file.h>
>> +#include<linux/module.h>
>> +#include<linux/spinlock.h>
>> +#include "emif_regs.h"
>> +
>> +/**
>> + * struct emif_data - Per device static data for driver's use
>> + * @duplicate:			Whether the DDR devices attached to this EMIF
>> + *				instance are exactly same as that on EMIF1. In
>> + *				this case we can save some memory and processing
>> + * @temperature_level:		Maximum temperature of LPDDR2 devices attached
>> + *				to this EMIF - read from MR4 register. If there
>> + *				are two devices attached to this EMIF, this
>> + *				value is the maximum of the two temperature
>> + *				levels.
>> + * @irq:			IRQ number
>> + * @lock:			lock for protecting temperature_level and
>> + *				associated actions from race conditions
>> + * @base:			base address of memory-mapped IO registers.
>> + * @dev:			device pointer.
>> + * @addressing			table with addressing information from the spec
>> + * @regs_cache:			An array of 'struct emif_regs' that stores
>> + *				calculated register values for different
>> + *				frequencies, to avoid re-calculating them on
>> + *				each DVFS transition.
>> + * @curr_regs:			The set of register values used in the last
>> + *				frequency change (i.e. corresponding to the
>> + *				frequency in effect at the moment)
>> + * @plat_data:			Pointer to saved platform data.
>> + */
>> +struct emif_data {
>> +	u8				duplicate;
>> +	u8				temperature_level;
>> +	u32				irq;
>> +	spinlock_t			lock; /* lock to prevent races */
>> +	void __iomem			*base;
>> +	struct device			*dev;
>> +	const struct lpddr2_addressing	*addressing;
>> +	struct emif_regs		*regs_cache[EMIF_MAX_NUM_FREQUENCIES];
>> +	struct emif_regs		*curr_regs;
>> +	struct emif_platform_data	*plat_data;
>> +};
>> +
>> +static struct emif_data *emif1;
>> +
>> +static void __exit cleanup_emif(struct emif_data *emif)
>> +{
>> +	if (emif) {
>> +		if (emif->plat_data) {
>> +			kfree(emif->plat_data->custom_configs);
>> +			kfree(emif->plat_data->timings);
>> +			kfree(emif->plat_data->min_tck);
>> +			kfree(emif->plat_data->device_info);
>> +			kfree(emif->plat_data);
>> +		}
>> +		kfree(emif);
>> +	}
>> +}
>> +
> You might want to consider use of devm_kzalloc() kind of
> API so that you can get rid of above free stuff.

I had considered that. But please note that most of these pointers are
initialized using kmemdup() and kmemdup() doesn't have a 'devm_'
equivalent. So, I didn't use it even for the regular kmalloc() cases in
order to have a uniform cleanup approach.

>
>> +static void get_default_timings(struct emif_data *emif)
>> +{
>> +	struct emif_platform_data *pd = emif->plat_data;
>> +
>> +	pd->timings = lpddr2_jedec_timings;
>> +	pd->timings_arr_size = ARRAY_SIZE(lpddr2_jedec_timings);
>> +
>> +	dev_warn(emif->dev, "Using default timings\n");
>> +}
> Would be good if you add __line__ in all the errors and
> warnings. Helps to trace messages faster.

Ok. I shall add __FILE__ and __func__

>
>> +
>> +static int is_dev_data_valid(u32 type, u32 density, u32 io_width, u32 phy_type,
>> +		u32 ip_rev, struct device *dev)
>> +{
>> +	int valid;
>> +
>> +	valid = (type == DDR_TYPE_LPDDR2_S4 ||
>> +			type == DDR_TYPE_LPDDR2_S2)
>> +		&&  (density>= DDR_DENSITY_64Mb
>> +			&&  density<= DDR_DENSITY_8Gb)
>> +		&&  (io_width>= DDR_IO_WIDTH_8
>> +			&&  io_width<= DDR_IO_WIDTH_32);
>> +
>> +	/* Combinations of EMIF and PHY revisions that we support today */
>> +	switch (ip_rev) {
>> +	case EMIF_4D:
>> +		valid = valid&&  (phy_type == EMIF_PHY_TYPE_ATTILAPHY);
>> +		break;
>> +	case EMIF_4D5:
>> +		valid = valid&&  (phy_type == EMIF_PHY_TYPE_INTELLIPHY);
>> +		break;
>> +	default:
>> +		valid = 0;
>> +	}
>> +
>> +	if (!valid)
>> +		dev_err(dev, "Invalid DDR details\n");
>> +	return valid;
> Generally return 0 means success. you might want to consider
> returning -EINVAL or similar.

Please note that the function name here is a predicate. In this case 1
for success 0 for failure is correct, I believe.

>
>> +}
>> +
>> +static int is_custom_config_valid(struct emif_custom_configs *cust_cfgs,
>> +		struct device *dev)
>> +{
>> +	int valid = 1;
>> +
>> +	if ((cust_cfgs->mask&  EMIF_CUSTOM_CONFIG_LPMODE)&&
>> +		(cust_cfgs->lpmode != EMIF_LP_MODE_DISABLE))
>> +		valid = cust_cfgs->lpmode_freq_threshold&&
>> +			cust_cfgs->lpmode_timeout_performance&&
>> +			cust_cfgs->lpmode_timeout_power;
>> +
>> +	if (cust_cfgs->mask&  EMIF_CUSTOM_CONFIG_TEMP_ALERT_POLL_INTERVAL)
>> +		valid = valid&&  cust_cfgs->temp_alert_poll_interval_ms;
>> +
>> +	if (!valid)
>> +		dev_warn(dev, "Invalid custom configs\n");
>> +
>> +	return valid;
>> +}
>> +
> Ditto
>
>> +static struct emif_data * __init get_device_details(
>> +		struct platform_device *pdev)
>> +{
>> +	u32			size;
>> +	struct emif_data	*emif = NULL;
>> +	struct ddr_device_info	*dev_info;
>> +	struct emif_platform_data *pd;
>> +
>> +	pd = pdev->dev.platform_data;
>> +
> extra line

Will fix them all.

>> +	if (!(pd&&  pd->device_info&&  is_dev_data_valid(pd->device_info->type,
>> +			pd->device_info->density, pd->device_info->io_width,
>> +			pd->phy_type, pd->ip_rev,&pdev->dev)))
>> +		goto error;
>> +
>> +	emif	= kzalloc(sizeof(struct emif_data), GFP_KERNEL);
>> +	pd	= kmemdup(pd, sizeof(*pd), GFP_KERNEL);
>> +	dev_info = kmemdup(pd->device_info,
>> +			sizeof(*pd->device_info), GFP_KERNEL);
>> +
> here too
>> +	if (!emif || !pd || !dev_info)
>> +		goto error;
>> +
>> +	emif->plat_data		= pd;
>> +	emif->dev		=&pdev->dev;
>> +	emif->temperature_level	= SDRAM_TEMP_NOMINAL;
>> +
> and here
>> +	pd->device_info	= dev_info;
>> +
>> +	/*
>> +	 * For EMIF instances other than EMIF1 see if the devices connected
>> +	 * are exactly same as on EMIF1(which is typically the case). If so,
>> +	 * mark it as a duplicate of EMIF1 and skip copying timings data.
>> +	 * This will save some memory and some computation later.
>> +	 */
>> +	emif->duplicate = emif1&&  (memcmp(dev_info,
>> +		emif1->plat_data->device_info,
>> +		sizeof(struct ddr_device_info)) == 0);
>> +
> and here
>> +	if (emif->duplicate) {
>> +		pd->timings = NULL;
>> +		pd->min_tck = NULL;
>> +		goto out;
>> +	}
>> +
>> +	/*
>> +	 * Copy custom configs - ignore allocation error, if any, as
>> +	 * custom_configs is not very critical
>> +	 */
>> +	if (pd->custom_configs)
>> +		pd->custom_configs = kmemdup(pd->custom_configs,
>> +			sizeof(*pd->custom_configs), GFP_KERNEL);
>> +
>> +	if (pd->custom_configs&&
>> +	    !is_custom_config_valid(pd->custom_configs, emif->dev)) {
>> +		kfree(pd->custom_configs);
>> +		pd->custom_configs = NULL;
>> +	}
>> +
>> +	/*
>> +	 * Copy timings and min-tck values from platform data. If it is not
>> +	 * available or if memory allocation fails, use JEDEC defaults
>> +	 */
>> +	size = sizeof(struct lpddr2_timings) * pd->timings_arr_size;
>> +	if (pd->timings)
>> +		pd->timings = kmemdup(pd->timings, size, GFP_KERNEL);
>> +
>> +	if (!pd->timings)
>> +		get_default_timings(emif);
>> +
>> +	if (pd->min_tck)
>> +		pd->min_tck = kmemdup(pd->min_tck, sizeof(*pd->min_tck),
>> +				GFP_KERNEL);
>> +
>> +	if (!pd->min_tck) {
> else on above if should work, right ?

No. Please note that the pd->min_tck is allocated as part of the above
if.

>
>> +		pd->min_tck =&lpddr2_min_tck;
>> +		dev_warn(emif->dev, "Using default min-tck values\n");
>> +	}
>> +
>> +	goto out;
>> +
>> +error:
>> +	dev_err(&pdev->dev, "get_device_details() failure!!\n");
>> +	cleanup_emif(emif);
>> +	return NULL;
>> +
>> +out:
>> +	return emif;
>> +}
>> +
>> +static int __init emif_probe(struct platform_device *pdev)
>> +{
>> +	struct emif_data	*emif;
>> +	struct resource		*res;
>> +
>> +	emif = get_device_details(pdev);
>> +
> extra line
>
>> +	if (!emif)
>> +		goto error;
>> +
>> +	if (!emif1)
>> +		emif1 = emif;
>> +
>> +	/* Save pointers to each other in emif and device structures */
>> +	emif->dev =&pdev->dev;
>> +	platform_set_drvdata(pdev, emif);
>> +
>> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +	if (!res)
>> +		goto error;
>> +
>> +	emif->base = ioremap(res->start, SZ_1M);
>> +	if (!emif->base)
>> +		goto error;
>> +
>> +	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
>> +	if (!res)
>> +		goto error;
> you might want add a line here

will do.

>> +	emif->irq = res->start;
>> +
> And remove this one
>> +	dev_info(&pdev->dev, "Device configured with addr = %p and IRQ%d\n",
>> +			emif->base, emif->irq);
>> +	return 0;
>> +
>> +error:
>> +	dev_err(&pdev->dev, "probe failure!!\n");
>> +	cleanup_emif(emif);
>> +	return -ENODEV;
>> +}
>> +
>> +static int __exit emif_remove(struct platform_device *pdev)
>> +{
>> +	struct emif_data *emif = platform_get_drvdata(pdev);
>> +
>> +	cleanup_emif(emif);
>> +
>> +	return 0;
>> +}
>> +
>> +static struct platform_driver emif_driver = {
>> +	.remove		= __exit_p(emif_remove),
>> +	.driver = {
>> +		.name = "emif",
>> +	},
>> +};
>> +
>> +static int __init emif_register(void)
>> +{
>> +	return platform_driver_probe(&emif_driver, emif_probe);
>> +}
>> +
>> +static void __exit emif_unregister(void)
>> +{
>> +	platform_driver_unregister(&emif_driver);
>> +}
>> +
>> +module_init(emif_register);
>> +module_exit(emif_unregister);
>> +MODULE_DESCRIPTION("TI EMIF SDRAM Controller Driver");
>> +MODULE_LICENSE("GPL");
>> +MODULE_ALIAS("platform:emif");
>> +MODULE_AUTHOR("Texas Instruments Inc");
>> diff --git a/include/linux/emif.h b/include/linux/emif.h
>> new file mode 100644
>> index 0000000..4ddf20b
>> --- /dev/null
>> +++ b/include/linux/emif.h
>> @@ -0,0 +1,160 @@
>> +/*
>> + * Definitions for EMIF SDRAM controller in TI SoCs
>> + *
>> + * Copyright (C) 2010 Texas Instruments, Inc.
>> + *
> 2012

Will fix.

>
>> + * Aneesh V<aneesh@ti.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +#ifndef __LINUX_EMIF_H
>> +#define __LINUX_EMIF_H
>> +
>> +#ifndef __ASSEMBLY__
>> +#include<linux/jedec_ddr.h>
>> +
>> +/*
>> + * Maximum number of different frequencies supported by EMIF driver
>> + * Determines the number of entries in the pointer array for register
>> + * cache
>> + */
>> +#define EMIF_MAX_NUM_FREQUENCIES	6
>> +
>> +/* EMIF_PWR_MGMT_CTRL register */
>> +/* Low power modes */
> Combine above two line comments in one
> comment.

Will do.

>
>> +#define EMIF_LP_MODE_DISABLE		0
>> +#define EMIF_LP_MODE_CLOCK_STOP		1
>> +#define EMIF_LP_MODE_SELF_REFRESH	2
>> +#define EMIF_LP_MODE_PWR_DN		4
>> +
>> +/* Hardware capabilities */
>> +#define	EMIF_HW_CAPS_LL_INTERFACE	0x00000001
>> +
>> +/* EMIF IP Revisions */
>> +#define	EMIF_4D				1
>> +#define	EMIF_4D5			2
>> +
>> +/* PHY types */
>> +#define	EMIF_PHY_TYPE_ATTILAPHY		1
>> +#define	EMIF_PHY_TYPE_INTELLIPHY	2
>> +
>> +/* Custom config requests */
>> +#define EMIF_CUSTOM_CONFIG_LPMODE			0x00000001
>> +#define EMIF_CUSTOM_CONFIG_TEMP_ALERT_POLL_INTERVAL	0x00000002
>> +
>
> try to aling numbers for all the above defines.

Will do.

>
> With above minor comments, the patch looks fine to me.

Thanks,
Aneesh

Re: [RFC PATCH 5/8] misc: emif: handle frequency and voltage change events

[Aneesh V]

On Thursday 16 February 2012 04:08 PM, Santosh Shilimkar wrote:
> On Saturday 04 February 2012 05:46 PM, Aneesh V wrote:
>> Change SDRAM timings and other settings as necessary
>> on voltage and frequency changes. We calculate these
>> register settings based on data from the device data
>> sheet and inputs such a frequency, voltage state(stable
>> or ramping), temperature level etc.
>>
> May be you want add TBD or FIXME for notifiers when they
> are available. Do that in commit log as well as
> in the code so that we don't forget about it.

Will do.

>
>> Signed-off-by: Aneesh V<aneesh@ti.com>
>> ---
> Rest of the patch looks fine.

Re: [RFC PATCH 2/8] misc: ddr: add LPDDR2 data from JESD209-2

[Shilimkar, Santosh]

On Thu, Feb 16, 2012 at 4:40 PM, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
> On Thu, 16 Feb 2012 15:57:57 +0530
> Aneesh V <aneesh@ti.com> wrote:
>
>> On Thursday 16 February 2012 03:37 PM, Santosh Shilimkar wrote:
>> > On Saturday 04 February 2012 05:46 PM, Aneesh V wrote:
>> >> add LPDDR2 data from the JEDEC spec JESD209-2. The data
>> >> includes:
>> >>
>> >> 1. Addressing information for LPDDR2 memories of different
>> >>     densities and types(S2/S4)
>> >> 2. AC timing data.
>> >>
>> >> This data will useful for memory controller device drivers
>
> I don't think it belongs in drivers/misc. It's not a driver but a
> library. lib/ might be a better place for it perhaps ?
>
Agree for the JDEC data part. We can move this JDEC data to
lib/
Thanks for the input.

Any suggestion on the controller driver ?

Regards
Santosh

Re: [RFC PATCH 6/8] misc: emif: add interrupt and temperature handling

[Aneesh V]

On Thursday 16 February 2012 04:11 PM, Santosh Shilimkar wrote:
> On Saturday 04 February 2012 05:46 PM, Aneesh V wrote:
>> Add an ISR for EMIF that:
>> 	1. reports details of access errors
>> 	2. takes action on thermal events
>>
>> On thermal events SDRAM timing parameters are
>> adjusted to ensure safe operation
>>
>> Also clear all interrupts on shut-down. Pending IRQs
>> may casue problems during warm-reset.
>>
> Add some more details like MR4, EMIF polling frequency etc
> for better understanding.

Will do.



>
>> Signed-off-by: Aneesh V<aneesh@ti.com>
>> ---
>>   drivers/misc/emif.c |  209 ++++++++++++++++++++++++++++++++++++++++++++++++++-
>>   1 files changed, 207 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/misc/emif.c b/drivers/misc/emif.c
>> index 36ba6f4..5c2b0ae 100644
>> --- a/drivers/misc/emif.c
>> +++ b/drivers/misc/emif.c
>> @@ -500,6 +500,45 @@ static u32 get_pwr_mgmt_ctrl(u32 freq, struct emif_data *emif, u32 ip_rev)
>>   }
>>
>>   /*
>> + * Get the temperature level of the EMIF instance:
>> + * Reads the MR4 register of attached SDRAM parts to find out the temperature
>> + * level. If there are two parts attached(one on each CS), then the temperature
>> + * level for the EMIF instance is the higher of the two temperatures.
>> + */
>> +static void get_temperature_level(struct emif_data *emif)
>> +{
>> +	u32		temp, temperature_level;
>> +	unsigned long	irqs;
>> +	void __iomem	*base;
>> +
>> +	base = emif->base;
>> +
>> +	/* Read mode register 4 */
>> +	writel(DDR_MR4, base + EMIF_LPDDR2_MODE_REG_CONFIG);
>> +	temperature_level = readl(base + EMIF_LPDDR2_MODE_REG_DATA);
>> +	temperature_level = (temperature_level&  MR4_SDRAM_REF_RATE_MASK)>>
>> +				MR4_SDRAM_REF_RATE_SHIFT;
>> +
>> +	if (emif->plat_data->device_info->cs1_used) {
>> +		writel(DDR_MR4 | CS_MASK, base + EMIF_LPDDR2_MODE_REG_CONFIG);
>> +		temp = readl(base + EMIF_LPDDR2_MODE_REG_DATA);
>> +		temp = (temp&  MR4_SDRAM_REF_RATE_MASK)
>> +				>>  MR4_SDRAM_REF_RATE_SHIFT;
>> +		temperature_level = max(temp, temperature_level);
>> +	}
>> +
>> +	spin_lock_irqsave(&emif->lock, irqs);
> Add a line here.

Will do.

>> +	/* treat everything less than nominal(3) in MR4 as nominal */
>> +	if (unlikely(temperature_level<  SDRAM_TEMP_NOMINAL))
>> +		temperature_level = SDRAM_TEMP_NOMINAL;
>> +
>> +	/* if we get reserved value in MR4 persist with the existing value */
>> +	if (likely(temperature_level != SDRAM_TEMP_RESERVED_4))
>> +		emif->temperature_level = temperature_level;
>> +	spin_unlock_irqrestore(&emif->lock, irqs);
>> +}
>> +
>
> rest of the patch looks fine to me

Thanks,
Aneesh

Re: [RFC PATCH 2/8] misc: ddr: add LPDDR2 data from JESD209-2

[Aneesh V]

On Thursday 16 February 2012 04:40 PM, Alan Cox wrote:
> On Thu, 16 Feb 2012 15:57:57 +0530
> Aneesh V<aneesh@ti.com>  wrote:
>
>> On Thursday 16 February 2012 03:37 PM, Santosh Shilimkar wrote:
>>> On Saturday 04 February 2012 05:46 PM, Aneesh V wrote:
>>>> add LPDDR2 data from the JEDEC spec JESD209-2. The data
>>>> includes:
>>>>
>>>> 1. Addressing information for LPDDR2 memories of different
>>>>      densities and types(S2/S4)
>>>> 2. AC timing data.
>>>>
>>>> This data will useful for memory controller device drivers
>
> I don't think it belongs in drivers/misc. It's not a driver but a
> library. lib/ might be a better place for it perhaps ?

Agree. I shall move it to lib/

br,
Aneesh

Re: [RFC PATCH 7/8] misc: emif: add one-time settings

[Aneesh V]

On Thursday 16 February 2012 04:14 PM, Santosh Shilimkar wrote:
> On Saturday 04 February 2012 05:46 PM, Aneesh V wrote:
>> Add settings that are not dependent on frequency
>> or any other transient parameters
>>
> Expand the changelog a bit. One time settings like
> SDRAM_CONFIG, PHY_CONTROL, TEMP alert etc.

Will do.

>
>> Signed-off-by: Aneesh V<aneesh@ti.com>
>> ---
> Patch looks fine.

Thanks,
Aneesh

Re: [RFC PATCH 0/8] Add TI EMIF SDRAM controller driver

[Greg KH]

On Thu, Feb 16, 2012 at 04:21:11PM +0530, Santosh Shilimkar wrote:
> Andrew, Greg,
> 
> On Saturday 04 February 2012 05:46 PM, Aneesh V wrote:
> > Add a driver for the EMIF SDRAM controller used in TI SoCs
> > 
> > EMIF is an SDRAM controller that supports, based on its revision,
> > one or more of LPDDR2/DDR2/DDR3 protocols.This driver adds support
> > for LPDDR2.
> > 
> > The driver supports the following features:
> > - Calculates the DDR AC timing parameters to be set in EMIF
> >   registers using data from the device data-sheets and based
> >   on the DDR frequency. If data from data-sheets is not available
> >   default timing values from the JEDEC spec are used. These
> >   will be safe, but not necessarily optimal
> > - API for changing timings during DVFS or at boot-up
> > - Temperature alert configuration and handling of temperature
> >   alerts, if any for LPDDR2 devices
> >   * temperature alert is based on periodic polling of MR4 mode
> >     register in DDR devices automatically performed by hardware
> >   * timings are de-rated and brought back to nominal when
> >     temperature raises and falls respectively
> > - Cache of calculated register values to avoid re-calculating
> >   them
> > 
> > The driver will need some minor updates when it is eventually
> > integrated with DVFS. This can not be done now as DVFS support
> > is not available yet in mainline.
> > 
> > Discussions with Santosh Shilimkar <santosh.shilimkar@ti.com>
> > were immensely helpful in shaping up the interfaces. Vibhore Vardhan
> > <vvardhan@gmail.com> did the initial code snippet for thermal
> > handling.
> > 
> > Testing: 
> > - The driver is tested on OMAP4430 SDP.
> > - The driver in a slightly adapted form is also tested on OMAP5.
> > - Since mainline kernel doesn't have DVFS support yet,
> >   testing was done using a test module.
> > - Temperature alert handling was tested with simulated interrupts
> >   and faked temperature values as testing all cases in real-life
> >   scenarios is difficult.
> > 
> [...]
> 
> >  arch/arm/mach-omap2/omap_hwmod_44xx_data.c |  110 ++
> >  drivers/misc/Kconfig                       |   20 +
> >  drivers/misc/Makefile                      |    2 +
> >  drivers/misc/emif.c                        | 1522 ++++++++++++++++++++++++++++
> >  drivers/misc/emif_regs.h                   |  461 +++++++++
> >  drivers/misc/jedec_ddr_data.c              |  141 +++
> >  include/linux/emif.h                       |  257 +++++
> >  include/linux/jedec_ddr.h                  |  174 ++++
> 
> Any suggestion on where this driver can reside. It's a memory
> controller driver which supports standard DDR functionality
> as per JDEC specs including thermal alert. On top of
> that it does support DVFS using the TI PRCM IP block.

I don't know what any of those TLA words mean, so I really can't suggest
where this code should go.  But just from this diffstat, it looks like
you are creating a new user/kernel interface, without documenting it
anywhere, which isn't ok.

greg k-h

Re: [RFC PATCH 4/8] misc: emif: add basic infrastructure for EMIF driver

[Cousson, Benoit]

Hi Aneesh,

On 2/4/2012 1:16 PM, Aneesh V wrote:
> EMIF is an SDRAM controller used in various Texas Instruments
> SoCs. EMIF supports, based on its revision, one or more of
> LPDDR2/DDR2/DDR3 protocols.
> 
> Add the basic infrastructure for EMIF driver that includes
> driver registration, probe, parsing of platform data etc.
> 
> Signed-off-by: Aneesh V<aneesh@ti.com>
> ---
>   drivers/misc/Kconfig  |   12 ++
>   drivers/misc/Makefile |    1 +
>   drivers/misc/emif.c   |  300 +++++++++++++++++++++++++++++++++++++++++++++++++
>   include/linux/emif.h  |  160 ++++++++++++++++++++++++++
>   4 files changed, 473 insertions(+), 0 deletions(-)
>   create mode 100644 drivers/misc/emif.c
>   create mode 100644 include/linux/emif.h
> 
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index 8337bf6..d68184a 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -459,6 +459,18 @@ config DDR
>   	  information. This data is useful for drivers handling
>   	  DDR SDRAM controllers.
> 
> +config EMIF
> +	tristate "Texas Instruments EMIF driver"
> +	select DDR
> +	help
> +	  This driver is for the EMIF module available in Texas Instruments
> +	  SoCs. EMIF is an SDRAM controller that, based on its revision,
> +	  supports one or more of DDR2, DDR3, and LPDDR2 SDRAM protocols.
> +	  This driver takes care of only LPDDR2 memories presently. The
> +	  functions of the driver includes re-configuring AC timing
> +	  parameters and other settings during frequency, voltage and
> +	  temperature changes
> +
>   config ARM_CHARLCD
>   	bool "ARM Ltd. Character LCD Driver"
>   	depends on PLAT_VERSATILE
> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> index 4759166..076db0f 100644
> --- a/drivers/misc/Makefile
> +++ b/drivers/misc/Makefile
> @@ -37,6 +37,7 @@ obj-$(CONFIG_C2PORT)		+= c2port/
>   obj-$(CONFIG_IWMC3200TOP)      += iwmc3200top/
>   obj-$(CONFIG_HMC6352)		+= hmc6352.o
>   obj-$(CONFIG_DDR)		+= jedec_ddr_data.o
> +obj-$(CONFIG_EMIF)		+= emif.o
>   obj-y				+= eeprom/
>   obj-y				+= cb710/
>   obj-$(CONFIG_SPEAR13XX_PCIE_GADGET)	+= spear13xx_pcie_gadget.o
> diff --git a/drivers/misc/emif.c b/drivers/misc/emif.c
> new file mode 100644
> index 0000000..ba1e3f9
> --- /dev/null
> +++ b/drivers/misc/emif.c
> @@ -0,0 +1,300 @@
> +/*
> + * EMIF driver
> + *
> + * Copyright (C) 2010 Texas Instruments, Inc.

Nit: 2012?

> + *
> + * Aneesh V<aneesh@ti.com>
> + * Santosh Shilimkar<santosh.shilimkar@ti.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +#include<linux/kernel.h>
> +#include<linux/reboot.h>
> +#include<linux/emif.h>
> +#include<linux/io.h>
> +#include<linux/device.h>
> +#include<linux/platform_device.h>
> +#include<linux/interrupt.h>
> +#include<linux/slab.h>
> +#include<linux/seq_file.h>
> +#include<linux/module.h>
> +#include<linux/spinlock.h>
> +#include "emif_regs.h"
> +
> +/**
> + * struct emif_data - Per device static data for driver's use
> + * @duplicate:			Whether the DDR devices attached to this EMIF
> + *				instance are exactly same as that on EMIF1. In
> + *				this case we can save some memory and processing
> + * @temperature_level:		Maximum temperature of LPDDR2 devices attached
> + *				to this EMIF - read from MR4 register. If there
> + *				are two devices attached to this EMIF, this
> + *				value is the maximum of the two temperature
> + *				levels.
> + * @irq:			IRQ number

Do you really need to store the IRQ number?

> + * @lock:			lock for protecting temperature_level and
> + *				associated actions from race conditions
> + * @base:			base address of memory-mapped IO registers.
> + * @dev:			device pointer.
> + * @addressing			table with addressing information from the spec
> + * @regs_cache:			An array of 'struct emif_regs' that stores
> + *				calculated register values for different
> + *				frequencies, to avoid re-calculating them on
> + *				each DVFS transition.
> + * @curr_regs:			The set of register values used in the last
> + *				frequency change (i.e. corresponding to the
> + *				frequency in effect at the moment)
> + * @plat_data:			Pointer to saved platform data.
> + */
> +struct emif_data {
> +	u8				duplicate;
> +	u8				temperature_level;
> +	u32				irq;
> +	spinlock_t			lock; /* lock to prevent races */

Nit: That comment is useless, since you already have the kerneldoc comment before.

> +	void __iomem			*base;
> +	struct device			*dev;
> +	const struct lpddr2_addressing	*addressing;
> +	struct emif_regs		*regs_cache[EMIF_MAX_NUM_FREQUENCIES];
> +	struct emif_regs		*curr_regs;
> +	struct emif_platform_data	*plat_data;
> +};
> +
> +static struct emif_data *emif1;
> +
> +static void __exit cleanup_emif(struct emif_data *emif)
> +{
> +	if (emif) {
> +		if (emif->plat_data) {
> +			kfree(emif->plat_data->custom_configs);
> +			kfree(emif->plat_data->timings);
> +			kfree(emif->plat_data->min_tck);
> +			kfree(emif->plat_data->device_info);
> +			kfree(emif->plat_data);
> +		}
> +		kfree(emif);
> +	}
> +}
> +
> +static void get_default_timings(struct emif_data *emif)
> +{
> +	struct emif_platform_data *pd = emif->plat_data;
> +
> +	pd->timings = lpddr2_jedec_timings;
> +	pd->timings_arr_size = ARRAY_SIZE(lpddr2_jedec_timings);
> +
> +	dev_warn(emif->dev, "Using default timings\n");
> +}
> +
> +static int is_dev_data_valid(u32 type, u32 density, u32 io_width, u32 phy_type,
> +		u32 ip_rev, struct device *dev)
> +{
> +	int valid;
> +
> +	valid = (type == DDR_TYPE_LPDDR2_S4 ||
> +			type == DDR_TYPE_LPDDR2_S2)
> +		&&  (density>= DDR_DENSITY_64Mb
> +			&&  density<= DDR_DENSITY_8Gb)
> +		&&  (io_width>= DDR_IO_WIDTH_8
> +			&&  io_width<= DDR_IO_WIDTH_32);
> +
> +	/* Combinations of EMIF and PHY revisions that we support today */
> +	switch (ip_rev) {
> +	case EMIF_4D:
> +		valid = valid&&  (phy_type == EMIF_PHY_TYPE_ATTILAPHY);
> +		break;
> +	case EMIF_4D5:
> +		valid = valid&&  (phy_type == EMIF_PHY_TYPE_INTELLIPHY);
> +		break;
> +	default:
> +		valid = 0;
> +	}
> +
> +	if (!valid)
> +		dev_err(dev, "Invalid DDR details\n");
> +	return valid;
> +}
> +
> +static int is_custom_config_valid(struct emif_custom_configs *cust_cfgs,
> +		struct device *dev)
> +{
> +	int valid = 1;
> +
> +	if ((cust_cfgs->mask&  EMIF_CUSTOM_CONFIG_LPMODE)&&
> +		(cust_cfgs->lpmode != EMIF_LP_MODE_DISABLE))
> +		valid = cust_cfgs->lpmode_freq_threshold&&
> +			cust_cfgs->lpmode_timeout_performance&&
> +			cust_cfgs->lpmode_timeout_power;
> +
> +	if (cust_cfgs->mask&  EMIF_CUSTOM_CONFIG_TEMP_ALERT_POLL_INTERVAL)
> +		valid = valid&&  cust_cfgs->temp_alert_poll_interval_ms;
> +
> +	if (!valid)
> +		dev_warn(dev, "Invalid custom configs\n");
> +
> +	return valid;
> +}
> +
> +static struct emif_data * __init get_device_details(
> +		struct platform_device *pdev)
> +{
> +	u32			size;
> +	struct emif_data	*emif = NULL;
> +	struct ddr_device_info	*dev_info;
> +	struct emif_platform_data *pd;
> +
> +	pd = pdev->dev.platform_data;
> +
> +	if (!(pd&&  pd->device_info&&  is_dev_data_valid(pd->device_info->type,
> +			pd->device_info->density, pd->device_info->io_width,
> +			pd->phy_type, pd->ip_rev,&pdev->dev)))
> +		goto error;
> +
> +	emif	= kzalloc(sizeof(struct emif_data), GFP_KERNEL);

You should use the devm_* version of this API to get the simplify the error handling / removal.

> +	pd	= kmemdup(pd, sizeof(*pd), GFP_KERNEL);
> +	dev_info = kmemdup(pd->device_info,
> +			sizeof(*pd->device_info), GFP_KERNEL);
> +
> +	if (!emif || !pd || !dev_info)
> +		goto error;

The error report will not be really useful since you will not return the exact source of failure.

> +
> +	emif->plat_data		= pd;
> +	emif->dev		=&pdev->dev;
> +	emif->temperature_level	= SDRAM_TEMP_NOMINAL;
> +
> +	pd->device_info	= dev_info;
> +
> +	/*
> +	 * For EMIF instances other than EMIF1 see if the devices connected
> +	 * are exactly same as on EMIF1(which is typically the case). If so,
> +	 * mark it as a duplicate of EMIF1 and skip copying timings data.
> +	 * This will save some memory and some computation later.
> +	 */
> +	emif->duplicate = emif1&&  (memcmp(dev_info,
> +		emif1->plat_data->device_info,
> +		sizeof(struct ddr_device_info)) == 0);
> +
> +	if (emif->duplicate) {
> +		pd->timings = NULL;
> +		pd->min_tck = NULL;
> +		goto out;
> +	}
> +
> +	/*
> +	 * Copy custom configs - ignore allocation error, if any, as
> +	 * custom_configs is not very critical
> +	 */
> +	if (pd->custom_configs)
> +		pd->custom_configs = kmemdup(pd->custom_configs,
> +			sizeof(*pd->custom_configs), GFP_KERNEL);
> +
> +	if (pd->custom_configs&&
> +	    !is_custom_config_valid(pd->custom_configs, emif->dev)) {
> +		kfree(pd->custom_configs);
> +		pd->custom_configs = NULL;
> +	}
> +
> +	/*
> +	 * Copy timings and min-tck values from platform data. If it is not
> +	 * available or if memory allocation fails, use JEDEC defaults
> +	 */
> +	size = sizeof(struct lpddr2_timings) * pd->timings_arr_size;
> +	if (pd->timings)
> +		pd->timings = kmemdup(pd->timings, size, GFP_KERNEL);
> +
> +	if (!pd->timings)
> +		get_default_timings(emif);
> +
> +	if (pd->min_tck)
> +		pd->min_tck = kmemdup(pd->min_tck, sizeof(*pd->min_tck),
> +				GFP_KERNEL);
> +
> +	if (!pd->min_tck) {
> +		pd->min_tck =&lpddr2_min_tck;
> +		dev_warn(emif->dev, "Using default min-tck values\n");
> +	}
> +
> +	goto out;
> +
> +error:
> +	dev_err(&pdev->dev, "get_device_details() failure!!\n");

That's not a very good error message, isn't?

> +	cleanup_emif(emif);
> +	return NULL;
> +
> +out:
> +	return emif;
> +}
> +
> +static int __init emif_probe(struct platform_device *pdev)
> +{
> +	struct emif_data	*emif;
> +	struct resource		*res;
> +
> +	emif = get_device_details(pdev);
> +
> +	if (!emif)
> +		goto error;
> +
> +	if (!emif1)
> +		emif1 = emif;
> +
> +	/* Save pointers to each other in emif and device structures */
> +	emif->dev =&pdev->dev;
> +	platform_set_drvdata(pdev, emif);
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!res)
> +		goto error;

You should reserve the region before ioremapping it. Something like that:

if (!devm_request_mem_region(dev, res->start, resource_size(res), pdev->name)) {
       dev_err(dev, "Region already claimed\n");
       return -EBUSY;
}

> +
> +	emif->base = ioremap(res->start, SZ_1M);

Use devm_ioremap.

> +	if (!emif->base)
> +		goto error;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);

You should use platform_get_irq instead.

> +	if (!res)
> +		goto error;
> +	emif->irq = res->start;
> +
> +	dev_info(&pdev->dev, "Device configured with addr = %p and IRQ%d\n",
> +			emif->base, emif->irq);
> +	return 0;
> +
> +error:
> +	dev_err(&pdev->dev, "probe failure!!\n");

Because??? You should be a little bit more verbose in case of failure.

Regards,
Benoit

> +	cleanup_emif(emif);
> +	return -ENODEV;
> +}
> +
> +static int __exit emif_remove(struct platform_device *pdev)
> +{
> +	struct emif_data *emif = platform_get_drvdata(pdev);
> +
> +	cleanup_emif(emif);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver emif_driver = {
> +	.remove		= __exit_p(emif_remove),
> +	.driver = {
> +		.name = "emif",
> +	},
> +};
> +
> +static int __init emif_register(void)
> +{
> +	return platform_driver_probe(&emif_driver, emif_probe);
> +}
> +
> +static void __exit emif_unregister(void)
> +{
> +	platform_driver_unregister(&emif_driver);
> +}
> +
> +module_init(emif_register);
> +module_exit(emif_unregister);
> +MODULE_DESCRIPTION("TI EMIF SDRAM Controller Driver");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("platform:emif");
> +MODULE_AUTHOR("Texas Instruments Inc");
> diff --git a/include/linux/emif.h b/include/linux/emif.h
> new file mode 100644
> index 0000000..4ddf20b
> --- /dev/null
> +++ b/include/linux/emif.h
> @@ -0,0 +1,160 @@
> +/*
> + * Definitions for EMIF SDRAM controller in TI SoCs
> + *
> + * Copyright (C) 2010 Texas Instruments, Inc.
> + *
> + * Aneesh V<aneesh@ti.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +#ifndef __LINUX_EMIF_H
> +#define __LINUX_EMIF_H
> +
> +#ifndef __ASSEMBLY__
> +#include<linux/jedec_ddr.h>
> +
> +/*
> + * Maximum number of different frequencies supported by EMIF driver
> + * Determines the number of entries in the pointer array for register
> + * cache
> + */
> +#define EMIF_MAX_NUM_FREQUENCIES	6
> +
> +/* EMIF_PWR_MGMT_CTRL register */
> +/* Low power modes */
> +#define EMIF_LP_MODE_DISABLE		0
> +#define EMIF_LP_MODE_CLOCK_STOP		1
> +#define EMIF_LP_MODE_SELF_REFRESH	2
> +#define EMIF_LP_MODE_PWR_DN		4
> +
> +/* Hardware capabilities */
> +#define	EMIF_HW_CAPS_LL_INTERFACE	0x00000001
> +
> +/* EMIF IP Revisions */
> +#define	EMIF_4D				1
> +#define	EMIF_4D5			2
> +
> +/* PHY types */
> +#define	EMIF_PHY_TYPE_ATTILAPHY		1
> +#define	EMIF_PHY_TYPE_INTELLIPHY	2
> +
> +/* Custom config requests */
> +#define EMIF_CUSTOM_CONFIG_LPMODE			0x00000001
> +#define EMIF_CUSTOM_CONFIG_TEMP_ALERT_POLL_INTERVAL	0x00000002
> +
> +/*
> + * Structure containing shadow of important registers in EMIF
> + * The calculation function fills in this structure to be later used for
> + * initialisation and DVFS
> + */
> +struct emif_regs {
> +	u32 freq;
> +	u32 ref_ctrl_shdw;
> +	u32 ref_ctrl_shdw_derated;
> +	u32 sdram_tim1_shdw;
> +	u32 sdram_tim1_shdw_derated;
> +	u32 sdram_tim2_shdw;
> +	u32 sdram_tim3_shdw;
> +	u32 sdram_tim3_shdw_derated;
> +	u32 pwr_mgmt_ctrl_shdw;
> +	union {
> +		u32 read_idle_ctrl_shdw_normal;
> +		u32 dll_calib_ctrl_shdw_normal;
> +	};
> +	union {
> +		u32 read_idle_ctrl_shdw_volt_ramp;
> +		u32 dll_calib_ctrl_shdw_volt_ramp;
> +	};
> +
> +	u32 phy_ctrl_1_shdw;
> +	u32 ext_phy_ctrl_2_shdw;
> +	u32 ext_phy_ctrl_3_shdw;
> +	u32 ext_phy_ctrl_4_shdw;
> +};
> +
> +/**
> + * struct ddr_device_info - All information about the DDR device except AC
> + *		timing parameters
> + * @type:	Device type (LPDDR2-S4, LPDDR2-S2 etc)
> + * @density:	Device density
> + * @io_width:	Bus width
> + * @cs1_used:	Whether there is a DDR device attached to the second
> + *		chip-select(CS1) of this EMIF instance
> + * @cal_resistors_per_cs: Whether there is one calibration resistor per
> + *		chip-select or whether it's a single one for both
> + * @manufacturer: Manufacturer name string
> + */
> +struct ddr_device_info {
> +	u32	type;
> +	u32	density;
> +	u32	io_width;
> +	u32	cs1_used;
> +	u32	cal_resistors_per_cs;
> +	char	manufacturer[10];
> +};
> +
> +/**
> + * struct emif_custom_configs - Custom configuration parameters/policies
> + *		passed from the platform layer
> + * @mask:	Mask to indicate which configs are requested
> + * @lpmode:	LPMODE to be used in PWR_MGMT_CTRL register
> + * @lpmode_timeout_performance: Timeout before LPMODE entry when higher
> + *		performance is desired at the cost of power (typically
> + *		at higher OPPs)
> + * @lpmode_timeout_power: Timeout before LPMODE entry when better power
> + *		savings is desired and performance is not important
> + *		(typically at lower loads indicated by lower OPPs)
> + * @lpmode_freq_threshold: The DDR frequency threshold to identify between
> + *		the above two cases:
> + *		timeout = (freq>= lpmode_freq_threshold) ?
> + *			lpmode_timeout_performance :
> + *			lpmode_timeout_power;
> + * @temp_alert_poll_interval_ms: LPDDR2 MR4 polling interval at nominal
> + *		temperature(in milliseconds). When temperature is high
> + *		polling is done 4 times as frequently.
> + */
> +struct emif_custom_configs {
> +	u32 mask;
> +	u32 lpmode;
> +	u32 lpmode_timeout_performance;
> +	u32 lpmode_timeout_power;
> +	u32 lpmode_freq_threshold;
> +	u32 temp_alert_poll_interval_ms;
> +};
> +
> +/**
> + * struct emif_platform_data - Platform data passed on EMIF platform
> + *				device creation. Used by the driver.
> + * @hw_caps:		Hw capabilities of the EMIF IP in the respective SoC
> + * @device_info:	Device info structure containing information such
> + *			as type, bus width, density etc
> + * @timings:		Timings information from device datasheet passed
> + *			as an array of 'struct lpddr2_timings'. Can be NULL
> + *			if if default timings are ok
> + * @timings_arr_size:	Size of the timings array. Depends on the number
> + *			of different frequencies for which timings data
> + *			is provided
> + * @min_tck:		Minimum value of some timing parameters in terms
> + *			of number of cycles. Can be NULL if default values
> + *			are ok
> + * @custom_configs:	Custom configurations requested by SoC or board
> + *			code and the data for them. Can be NULL if default
> + *			configurations done by the driver are ok. See
> + *			documentation for 'struct emif_custom_configs' for
> + *			more details
> + */
> +struct emif_platform_data {
> +	u32 hw_caps;
> +	struct ddr_device_info *device_info;
> +	const struct lpddr2_timings *timings;
> +	u32 timings_arr_size;
> +	const struct lpddr2_min_tck *min_tck;
> +	struct emif_custom_configs *custom_configs;
> +	u32 ip_rev;
> +	u32 phy_type;
> +};
> +#endif /* __ASSEMBLY__ */
> +
> +#endif /* __LINUX_EMIF_H */

Re: [RFC PATCH 4/8] misc: emif: add basic infrastructure for EMIF driver

[Cousson, Benoit]

Hi Aneesh,

On 2/4/2012 1:16 PM, Aneesh V wrote:
> EMIF is an SDRAM controller used in various Texas Instruments
> SoCs. EMIF supports, based on its revision, one or more of
> LPDDR2/DDR2/DDR3 protocols.
> 
> Add the basic infrastructure for EMIF driver that includes
> driver registration, probe, parsing of platform data etc.
> 
> Signed-off-by: Aneesh V<aneesh@ti.com>
> ---
>   drivers/misc/Kconfig  |   12 ++
>   drivers/misc/Makefile |    1 +
>   drivers/misc/emif.c   |  300 +++++++++++++++++++++++++++++++++++++++++++++++++
>   include/linux/emif.h  |  160 ++++++++++++++++++++++++++
>   4 files changed, 473 insertions(+), 0 deletions(-)
>   create mode 100644 drivers/misc/emif.c
>   create mode 100644 include/linux/emif.h
> 
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index 8337bf6..d68184a 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -459,6 +459,18 @@ config DDR
>   	  information. This data is useful for drivers handling
>   	  DDR SDRAM controllers.
> 
> +config EMIF
> +	tristate "Texas Instruments EMIF driver"
> +	select DDR
> +	help
> +	  This driver is for the EMIF module available in Texas Instruments
> +	  SoCs. EMIF is an SDRAM controller that, based on its revision,
> +	  supports one or more of DDR2, DDR3, and LPDDR2 SDRAM protocols.
> +	  This driver takes care of only LPDDR2 memories presently. The
> +	  functions of the driver includes re-configuring AC timing
> +	  parameters and other settings during frequency, voltage and
> +	  temperature changes
> +
>   config ARM_CHARLCD
>   	bool "ARM Ltd. Character LCD Driver"
>   	depends on PLAT_VERSATILE
> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> index 4759166..076db0f 100644
> --- a/drivers/misc/Makefile
> +++ b/drivers/misc/Makefile
> @@ -37,6 +37,7 @@ obj-$(CONFIG_C2PORT)		+= c2port/
>   obj-$(CONFIG_IWMC3200TOP)      += iwmc3200top/
>   obj-$(CONFIG_HMC6352)		+= hmc6352.o
>   obj-$(CONFIG_DDR)		+= jedec_ddr_data.o
> +obj-$(CONFIG_EMIF)		+= emif.o
>   obj-y				+= eeprom/
>   obj-y				+= cb710/
>   obj-$(CONFIG_SPEAR13XX_PCIE_GADGET)	+= spear13xx_pcie_gadget.o
> diff --git a/drivers/misc/emif.c b/drivers/misc/emif.c
> new file mode 100644
> index 0000000..ba1e3f9
> --- /dev/null
> +++ b/drivers/misc/emif.c
> @@ -0,0 +1,300 @@
> +/*
> + * EMIF driver
> + *
> + * Copyright (C) 2010 Texas Instruments, Inc.

Nit: 2012?

> + *
> + * Aneesh V<aneesh@ti.com>
> + * Santosh Shilimkar<santosh.shilimkar@ti.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +#include<linux/kernel.h>
> +#include<linux/reboot.h>
> +#include<linux/emif.h>
> +#include<linux/io.h>
> +#include<linux/device.h>
> +#include<linux/platform_device.h>
> +#include<linux/interrupt.h>
> +#include<linux/slab.h>
> +#include<linux/seq_file.h>
> +#include<linux/module.h>
> +#include<linux/spinlock.h>
> +#include "emif_regs.h"
> +
> +/**
> + * struct emif_data - Per device static data for driver's use
> + * @duplicate:			Whether the DDR devices attached to this EMIF
> + *				instance are exactly same as that on EMIF1. In
> + *				this case we can save some memory and processing
> + * @temperature_level:		Maximum temperature of LPDDR2 devices attached
> + *				to this EMIF - read from MR4 register. If there
> + *				are two devices attached to this EMIF, this
> + *				value is the maximum of the two temperature
> + *				levels.
> + * @irq:			IRQ number

Do you really need to store the IRQ number?

> + * @lock:			lock for protecting temperature_level and
> + *				associated actions from race conditions
> + * @base:			base address of memory-mapped IO registers.
> + * @dev:			device pointer.
> + * @addressing			table with addressing information from the spec
> + * @regs_cache:			An array of 'struct emif_regs' that stores
> + *				calculated register values for different
> + *				frequencies, to avoid re-calculating them on
> + *				each DVFS transition.
> + * @curr_regs:			The set of register values used in the last
> + *				frequency change (i.e. corresponding to the
> + *				frequency in effect at the moment)
> + * @plat_data:			Pointer to saved platform data.
> + */
> +struct emif_data {
> +	u8				duplicate;
> +	u8				temperature_level;
> +	u32				irq;
> +	spinlock_t			lock; /* lock to prevent races */

Nit: That comment is useless, since you already have the kerneldoc comment before.

> +	void __iomem			*base;
> +	struct device			*dev;
> +	const struct lpddr2_addressing	*addressing;
> +	struct emif_regs		*regs_cache[EMIF_MAX_NUM_FREQUENCIES];
> +	struct emif_regs		*curr_regs;
> +	struct emif_platform_data	*plat_data;
> +};
> +
> +static struct emif_data *emif1;
> +
> +static void __exit cleanup_emif(struct emif_data *emif)
> +{
> +	if (emif) {
> +		if (emif->plat_data) {
> +			kfree(emif->plat_data->custom_configs);
> +			kfree(emif->plat_data->timings);
> +			kfree(emif->plat_data->min_tck);
> +			kfree(emif->plat_data->device_info);
> +			kfree(emif->plat_data);
> +		}
> +		kfree(emif);
> +	}
> +}
> +
> +static void get_default_timings(struct emif_data *emif)
> +{
> +	struct emif_platform_data *pd = emif->plat_data;
> +
> +	pd->timings = lpddr2_jedec_timings;
> +	pd->timings_arr_size = ARRAY_SIZE(lpddr2_jedec_timings);
> +
> +	dev_warn(emif->dev, "Using default timings\n");
> +}
> +
> +static int is_dev_data_valid(u32 type, u32 density, u32 io_width, u32 phy_type,
> +		u32 ip_rev, struct device *dev)
> +{
> +	int valid;
> +
> +	valid = (type == DDR_TYPE_LPDDR2_S4 ||
> +			type == DDR_TYPE_LPDDR2_S2)
> +		&&  (density>= DDR_DENSITY_64Mb
> +			&&  density<= DDR_DENSITY_8Gb)
> +		&&  (io_width>= DDR_IO_WIDTH_8
> +			&&  io_width<= DDR_IO_WIDTH_32);
> +
> +	/* Combinations of EMIF and PHY revisions that we support today */
> +	switch (ip_rev) {
> +	case EMIF_4D:
> +		valid = valid&&  (phy_type == EMIF_PHY_TYPE_ATTILAPHY);
> +		break;
> +	case EMIF_4D5:
> +		valid = valid&&  (phy_type == EMIF_PHY_TYPE_INTELLIPHY);
> +		break;
> +	default:
> +		valid = 0;
> +	}
> +
> +	if (!valid)
> +		dev_err(dev, "Invalid DDR details\n");
> +	return valid;
> +}
> +
> +static int is_custom_config_valid(struct emif_custom_configs *cust_cfgs,
> +		struct device *dev)
> +{
> +	int valid = 1;
> +
> +	if ((cust_cfgs->mask&  EMIF_CUSTOM_CONFIG_LPMODE)&&
> +		(cust_cfgs->lpmode != EMIF_LP_MODE_DISABLE))
> +		valid = cust_cfgs->lpmode_freq_threshold&&
> +			cust_cfgs->lpmode_timeout_performance&&
> +			cust_cfgs->lpmode_timeout_power;
> +
> +	if (cust_cfgs->mask&  EMIF_CUSTOM_CONFIG_TEMP_ALERT_POLL_INTERVAL)
> +		valid = valid&&  cust_cfgs->temp_alert_poll_interval_ms;
> +
> +	if (!valid)
> +		dev_warn(dev, "Invalid custom configs\n");
> +
> +	return valid;
> +}
> +
> +static struct emif_data * __init get_device_details(
> +		struct platform_device *pdev)
> +{
> +	u32			size;
> +	struct emif_data	*emif = NULL;
> +	struct ddr_device_info	*dev_info;
> +	struct emif_platform_data *pd;
> +
> +	pd = pdev->dev.platform_data;
> +
> +	if (!(pd&&  pd->device_info&&  is_dev_data_valid(pd->device_info->type,
> +			pd->device_info->density, pd->device_info->io_width,
> +			pd->phy_type, pd->ip_rev,&pdev->dev)))
> +		goto error;
> +
> +	emif	= kzalloc(sizeof(struct emif_data), GFP_KERNEL);

You should use the devm_* version of this API to get the simplify the error handling / removal.

> +	pd	= kmemdup(pd, sizeof(*pd), GFP_KERNEL);
> +	dev_info = kmemdup(pd->device_info,
> +			sizeof(*pd->device_info), GFP_KERNEL);
> +
> +	if (!emif || !pd || !dev_info)
> +		goto error;

The error report will not be really useful since you will not return the exact source of failure.

> +
> +	emif->plat_data		= pd;
> +	emif->dev		=&pdev->dev;
> +	emif->temperature_level	= SDRAM_TEMP_NOMINAL;
> +
> +	pd->device_info	= dev_info;
> +
> +	/*
> +	 * For EMIF instances other than EMIF1 see if the devices connected
> +	 * are exactly same as on EMIF1(which is typically the case). If so,
> +	 * mark it as a duplicate of EMIF1 and skip copying timings data.
> +	 * This will save some memory and some computation later.
> +	 */
> +	emif->duplicate = emif1&&  (memcmp(dev_info,
> +		emif1->plat_data->device_info,
> +		sizeof(struct ddr_device_info)) == 0);
> +
> +	if (emif->duplicate) {
> +		pd->timings = NULL;
> +		pd->min_tck = NULL;
> +		goto out;
> +	}
> +
> +	/*
> +	 * Copy custom configs - ignore allocation error, if any, as
> +	 * custom_configs is not very critical
> +	 */
> +	if (pd->custom_configs)
> +		pd->custom_configs = kmemdup(pd->custom_configs,
> +			sizeof(*pd->custom_configs), GFP_KERNEL);
> +
> +	if (pd->custom_configs&&
> +	    !is_custom_config_valid(pd->custom_configs, emif->dev)) {
> +		kfree(pd->custom_configs);
> +		pd->custom_configs = NULL;
> +	}
> +
> +	/*
> +	 * Copy timings and min-tck values from platform data. If it is not
> +	 * available or if memory allocation fails, use JEDEC defaults
> +	 */
> +	size = sizeof(struct lpddr2_timings) * pd->timings_arr_size;
> +	if (pd->timings)
> +		pd->timings = kmemdup(pd->timings, size, GFP_KERNEL);
> +
> +	if (!pd->timings)
> +		get_default_timings(emif);
> +
> +	if (pd->min_tck)
> +		pd->min_tck = kmemdup(pd->min_tck, sizeof(*pd->min_tck),
> +				GFP_KERNEL);
> +
> +	if (!pd->min_tck) {
> +		pd->min_tck =&lpddr2_min_tck;
> +		dev_warn(emif->dev, "Using default min-tck values\n");
> +	}
> +
> +	goto out;
> +
> +error:
> +	dev_err(&pdev->dev, "get_device_details() failure!!\n");

That's not a very good error message, isn't?

> +	cleanup_emif(emif);
> +	return NULL;
> +
> +out:
> +	return emif;
> +}
> +
> +static int __init emif_probe(struct platform_device *pdev)
> +{
> +	struct emif_data	*emif;
> +	struct resource		*res;
> +
> +	emif = get_device_details(pdev);
> +
> +	if (!emif)
> +		goto error;
> +
> +	if (!emif1)
> +		emif1 = emif;
> +
> +	/* Save pointers to each other in emif and device structures */
> +	emif->dev =&pdev->dev;
> +	platform_set_drvdata(pdev, emif);
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!res)
> +		goto error;

You should reserve the region before ioremapping it. Something like that:

if (!devm_request_mem_region(dev, res->start, resource_size(res), pdev->name)) {
       dev_err(dev, "Region already claimed\n");
       return -EBUSY;
}

> +
> +	emif->base = ioremap(res->start, SZ_1M);

Use devm_ioremap.

> +	if (!emif->base)
> +		goto error;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);

You should use platform_get_irq instead.

> +	if (!res)
> +		goto error;
> +	emif->irq = res->start;
> +
> +	dev_info(&pdev->dev, "Device configured with addr = %p and IRQ%d\n",
> +			emif->base, emif->irq);
> +	return 0;
> +
> +error:
> +	dev_err(&pdev->dev, "probe failure!!\n");

Because??? You should be a little bit more verbose in case of failure.

Regards,
Benoit

> +	cleanup_emif(emif);
> +	return -ENODEV;
> +}
> +
> +static int __exit emif_remove(struct platform_device *pdev)
> +{
> +	struct emif_data *emif = platform_get_drvdata(pdev);
> +
> +	cleanup_emif(emif);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver emif_driver = {
> +	.remove		= __exit_p(emif_remove),
> +	.driver = {
> +		.name = "emif",
> +	},
> +};
> +
> +static int __init emif_register(void)
> +{
> +	return platform_driver_probe(&emif_driver, emif_probe);
> +}
> +
> +static void __exit emif_unregister(void)
> +{
> +	platform_driver_unregister(&emif_driver);
> +}
> +
> +module_init(emif_register);
> +module_exit(emif_unregister);
> +MODULE_DESCRIPTION("TI EMIF SDRAM Controller Driver");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("platform:emif");
> +MODULE_AUTHOR("Texas Instruments Inc");
> diff --git a/include/linux/emif.h b/include/linux/emif.h
> new file mode 100644
> index 0000000..4ddf20b
> --- /dev/null
> +++ b/include/linux/emif.h
> @@ -0,0 +1,160 @@
> +/*
> + * Definitions for EMIF SDRAM controller in TI SoCs
> + *
> + * Copyright (C) 2010 Texas Instruments, Inc.
> + *
> + * Aneesh V<aneesh@ti.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +#ifndef __LINUX_EMIF_H
> +#define __LINUX_EMIF_H
> +
> +#ifndef __ASSEMBLY__
> +#include<linux/jedec_ddr.h>
> +
> +/*
> + * Maximum number of different frequencies supported by EMIF driver
> + * Determines the number of entries in the pointer array for register
> + * cache
> + */
> +#define EMIF_MAX_NUM_FREQUENCIES	6
> +
> +/* EMIF_PWR_MGMT_CTRL register */
> +/* Low power modes */
> +#define EMIF_LP_MODE_DISABLE		0
> +#define EMIF_LP_MODE_CLOCK_STOP		1
> +#define EMIF_LP_MODE_SELF_REFRESH	2
> +#define EMIF_LP_MODE_PWR_DN		4
> +
> +/* Hardware capabilities */
> +#define	EMIF_HW_CAPS_LL_INTERFACE	0x00000001
> +
> +/* EMIF IP Revisions */
> +#define	EMIF_4D				1
> +#define	EMIF_4D5			2
> +
> +/* PHY types */
> +#define	EMIF_PHY_TYPE_ATTILAPHY		1
> +#define	EMIF_PHY_TYPE_INTELLIPHY	2
> +
> +/* Custom config requests */
> +#define EMIF_CUSTOM_CONFIG_LPMODE			0x00000001
> +#define EMIF_CUSTOM_CONFIG_TEMP_ALERT_POLL_INTERVAL	0x00000002
> +
> +/*
> + * Structure containing shadow of important registers in EMIF
> + * The calculation function fills in this structure to be later used for
> + * initialisation and DVFS
> + */
> +struct emif_regs {
> +	u32 freq;
> +	u32 ref_ctrl_shdw;
> +	u32 ref_ctrl_shdw_derated;
> +	u32 sdram_tim1_shdw;
> +	u32 sdram_tim1_shdw_derated;
> +	u32 sdram_tim2_shdw;
> +	u32 sdram_tim3_shdw;
> +	u32 sdram_tim3_shdw_derated;
> +	u32 pwr_mgmt_ctrl_shdw;
> +	union {
> +		u32 read_idle_ctrl_shdw_normal;
> +		u32 dll_calib_ctrl_shdw_normal;
> +	};
> +	union {
> +		u32 read_idle_ctrl_shdw_volt_ramp;
> +		u32 dll_calib_ctrl_shdw_volt_ramp;
> +	};
> +
> +	u32 phy_ctrl_1_shdw;
> +	u32 ext_phy_ctrl_2_shdw;
> +	u32 ext_phy_ctrl_3_shdw;
> +	u32 ext_phy_ctrl_4_shdw;
> +};
> +
> +/**
> + * struct ddr_device_info - All information about the DDR device except AC
> + *		timing parameters
> + * @type:	Device type (LPDDR2-S4, LPDDR2-S2 etc)
> + * @density:	Device density
> + * @io_width:	Bus width
> + * @cs1_used:	Whether there is a DDR device attached to the second
> + *		chip-select(CS1) of this EMIF instance
> + * @cal_resistors_per_cs: Whether there is one calibration resistor per
> + *		chip-select or whether it's a single one for both
> + * @manufacturer: Manufacturer name string
> + */
> +struct ddr_device_info {
> +	u32	type;
> +	u32	density;
> +	u32	io_width;
> +	u32	cs1_used;
> +	u32	cal_resistors_per_cs;
> +	char	manufacturer[10];
> +};
> +
> +/**
> + * struct emif_custom_configs - Custom configuration parameters/policies
> + *		passed from the platform layer
> + * @mask:	Mask to indicate which configs are requested
> + * @lpmode:	LPMODE to be used in PWR_MGMT_CTRL register
> + * @lpmode_timeout_performance: Timeout before LPMODE entry when higher
> + *		performance is desired at the cost of power (typically
> + *		at higher OPPs)
> + * @lpmode_timeout_power: Timeout before LPMODE entry when better power
> + *		savings is desired and performance is not important
> + *		(typically at lower loads indicated by lower OPPs)
> + * @lpmode_freq_threshold: The DDR frequency threshold to identify between
> + *		the above two cases:
> + *		timeout = (freq>= lpmode_freq_threshold) ?
> + *			lpmode_timeout_performance :
> + *			lpmode_timeout_power;
> + * @temp_alert_poll_interval_ms: LPDDR2 MR4 polling interval at nominal
> + *		temperature(in milliseconds). When temperature is high
> + *		polling is done 4 times as frequently.
> + */
> +struct emif_custom_configs {
> +	u32 mask;
> +	u32 lpmode;
> +	u32 lpmode_timeout_performance;
> +	u32 lpmode_timeout_power;
> +	u32 lpmode_freq_threshold;
> +	u32 temp_alert_poll_interval_ms;
> +};
> +
> +/**
> + * struct emif_platform_data - Platform data passed on EMIF platform
> + *				device creation. Used by the driver.
> + * @hw_caps:		Hw capabilities of the EMIF IP in the respective SoC
> + * @device_info:	Device info structure containing information such
> + *			as type, bus width, density etc
> + * @timings:		Timings information from device datasheet passed
> + *			as an array of 'struct lpddr2_timings'. Can be NULL
> + *			if if default timings are ok
> + * @timings_arr_size:	Size of the timings array. Depends on the number
> + *			of different frequencies for which timings data
> + *			is provided
> + * @min_tck:		Minimum value of some timing parameters in terms
> + *			of number of cycles. Can be NULL if default values
> + *			are ok
> + * @custom_configs:	Custom configurations requested by SoC or board
> + *			code and the data for them. Can be NULL if default
> + *			configurations done by the driver are ok. See
> + *			documentation for 'struct emif_custom_configs' for
> + *			more details
> + */
> +struct emif_platform_data {
> +	u32 hw_caps;
> +	struct ddr_device_info *device_info;
> +	const struct lpddr2_timings *timings;
> +	u32 timings_arr_size;
> +	const struct lpddr2_min_tck *min_tck;
> +	struct emif_custom_configs *custom_configs;
> +	u32 ip_rev;
> +	u32 phy_type;
> +};
> +#endif /* __ASSEMBLY__ */
> +
> +#endif /* __LINUX_EMIF_H */

Re: [RFC PATCH 4/8] misc: emif: add basic infrastructure for EMIF driver

[Aneesh V]

Hi Benoit,

On Thursday 16 February 2012 10:00 PM, Cousson, Benoit wrote:
> Hi Aneesh,
>
> On 2/4/2012 1:16 PM, Aneesh V wrote:
>> EMIF is an SDRAM controller used in various Texas Instruments
>> SoCs. EMIF supports, based on its revision, one or more of
>> LPDDR2/DDR2/DDR3 protocols.
>>
>> Add the basic infrastructure for EMIF driver that includes
>> driver registration, probe, parsing of platform data etc.
>>
>> Signed-off-by: Aneesh V<aneesh@ti.com>
>> ---
>>    drivers/misc/Kconfig  |   12 ++
>>    drivers/misc/Makefile |    1 +
>>    drivers/misc/emif.c   |  300 +++++++++++++++++++++++++++++++++++++++++++++++++
>>    include/linux/emif.h  |  160 ++++++++++++++++++++++++++
>>    4 files changed, 473 insertions(+), 0 deletions(-)
>>    create mode 100644 drivers/misc/emif.c
>>    create mode 100644 include/linux/emif.h
>>
>> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
>> index 8337bf6..d68184a 100644
>> --- a/drivers/misc/Kconfig
>> +++ b/drivers/misc/Kconfig
>> @@ -459,6 +459,18 @@ config DDR
>>    	  information. This data is useful for drivers handling
>>    	  DDR SDRAM controllers.
>>
>> +config EMIF
>> +	tristate "Texas Instruments EMIF driver"
>> +	select DDR
>> +	help
>> +	  This driver is for the EMIF module available in Texas Instruments
>> +	  SoCs. EMIF is an SDRAM controller that, based on its revision,
>> +	  supports one or more of DDR2, DDR3, and LPDDR2 SDRAM protocols.
>> +	  This driver takes care of only LPDDR2 memories presently. The
>> +	  functions of the driver includes re-configuring AC timing
>> +	  parameters and other settings during frequency, voltage and
>> +	  temperature changes
>> +
>>    config ARM_CHARLCD
>>    	bool "ARM Ltd. Character LCD Driver"
>>    	depends on PLAT_VERSATILE
>> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
>> index 4759166..076db0f 100644
>> --- a/drivers/misc/Makefile
>> +++ b/drivers/misc/Makefile
>> @@ -37,6 +37,7 @@ obj-$(CONFIG_C2PORT)		+= c2port/
>>    obj-$(CONFIG_IWMC3200TOP)      += iwmc3200top/
>>    obj-$(CONFIG_HMC6352)		+= hmc6352.o
>>    obj-$(CONFIG_DDR)		+= jedec_ddr_data.o
>> +obj-$(CONFIG_EMIF)		+= emif.o
>>    obj-y				+= eeprom/
>>    obj-y				+= cb710/
>>    obj-$(CONFIG_SPEAR13XX_PCIE_GADGET)	+= spear13xx_pcie_gadget.o
>> diff --git a/drivers/misc/emif.c b/drivers/misc/emif.c
>> new file mode 100644
>> index 0000000..ba1e3f9
>> --- /dev/null
>> +++ b/drivers/misc/emif.c
>> @@ -0,0 +1,300 @@
>> +/*
>> + * EMIF driver
>> + *
>> + * Copyright (C) 2010 Texas Instruments, Inc.
>
> Nit: 2012?

Will fix it.

>
>> + *
>> + * Aneesh V<aneesh@ti.com>
>> + * Santosh Shilimkar<santosh.shilimkar@ti.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +#include<linux/kernel.h>
>> +#include<linux/reboot.h>
>> +#include<linux/emif.h>
>> +#include<linux/io.h>
>> +#include<linux/device.h>
>> +#include<linux/platform_device.h>
>> +#include<linux/interrupt.h>
>> +#include<linux/slab.h>
>> +#include<linux/seq_file.h>
>> +#include<linux/module.h>
>> +#include<linux/spinlock.h>
>> +#include "emif_regs.h"
>> +
>> +/**
>> + * struct emif_data - Per device static data for driver's use
>> + * @duplicate:			Whether the DDR devices attached to this EMIF
>> + *				instance are exactly same as that on EMIF1. In
>> + *				this case we can save some memory and processing
>> + * @temperature_level:		Maximum temperature of LPDDR2 devices attached
>> + *				to this EMIF - read from MR4 register. If there
>> + *				are two devices attached to this EMIF, this
>> + *				value is the maximum of the two temperature
>> + *				levels.
>> + * @irq:			IRQ number
>
> Do you really need to store the IRQ number?

Yes, I need it right now because setup_interrupts() is called later,
after the first frequency notification, because that's when I have the
registers to be programmed on a temperature event. But I am re-thinking
on this strategy. I will move it back to probe() because other
interrupts can/should be enabled at probe() time. When I do that I
won't have to store it anymore and I will remove it.

>
>> + * @lock:			lock for protecting temperature_level and
>> + *				associated actions from race conditions
>> + * @base:			base address of memory-mapped IO registers.
>> + * @dev:			device pointer.
>> + * @addressing			table with addressing information from the spec
>> + * @regs_cache:			An array of 'struct emif_regs' that stores
>> + *				calculated register values for different
>> + *				frequencies, to avoid re-calculating them on
>> + *				each DVFS transition.
>> + * @curr_regs:			The set of register values used in the last
>> + *				frequency change (i.e. corresponding to the
>> + *				frequency in effect at the moment)
>> + * @plat_data:			Pointer to saved platform data.
>> + */
>> +struct emif_data {
>> +	u8				duplicate;
>> +	u8				temperature_level;
>> +	u32				irq;
>> +	spinlock_t			lock; /* lock to prevent races */
>
> Nit: That comment is useless, since you already have the kerneldoc comment before.

Ok. Will remove.

>
>> +	void __iomem			*base;
>> +	struct device			*dev;
>> +	const struct lpddr2_addressing	*addressing;
>> +	struct emif_regs		*regs_cache[EMIF_MAX_NUM_FREQUENCIES];
>> +	struct emif_regs		*curr_regs;
>> +	struct emif_platform_data	*plat_data;
>> +};
>> +
>> +static struct emif_data *emif1;
>> +
>> +static void __exit cleanup_emif(struct emif_data *emif)
>> +{
>> +	if (emif) {
>> +		if (emif->plat_data) {
>> +			kfree(emif->plat_data->custom_configs);
>> +			kfree(emif->plat_data->timings);
>> +			kfree(emif->plat_data->min_tck);
>> +			kfree(emif->plat_data->device_info);
>> +			kfree(emif->plat_data);
>> +		}
>> +		kfree(emif);
>> +	}
>> +}
>> +
>> +static void get_default_timings(struct emif_data *emif)
>> +{
>> +	struct emif_platform_data *pd = emif->plat_data;
>> +
>> +	pd->timings = lpddr2_jedec_timings;
>> +	pd->timings_arr_size = ARRAY_SIZE(lpddr2_jedec_timings);
>> +
>> +	dev_warn(emif->dev, "Using default timings\n");
>> +}
>> +
>> +static int is_dev_data_valid(u32 type, u32 density, u32 io_width, u32 phy_type,
>> +		u32 ip_rev, struct device *dev)
>> +{
>> +	int valid;
>> +
>> +	valid = (type == DDR_TYPE_LPDDR2_S4 ||
>> +			type == DDR_TYPE_LPDDR2_S2)
>> +		&&   (density>= DDR_DENSITY_64Mb
>> +			&&   density<= DDR_DENSITY_8Gb)
>> +		&&   (io_width>= DDR_IO_WIDTH_8
>> +			&&   io_width<= DDR_IO_WIDTH_32);
>> +
>> +	/* Combinations of EMIF and PHY revisions that we support today */
>> +	switch (ip_rev) {
>> +	case EMIF_4D:
>> +		valid = valid&&   (phy_type == EMIF_PHY_TYPE_ATTILAPHY);
>> +		break;
>> +	case EMIF_4D5:
>> +		valid = valid&&   (phy_type == EMIF_PHY_TYPE_INTELLIPHY);
>> +		break;
>> +	default:
>> +		valid = 0;
>> +	}
>> +
>> +	if (!valid)
>> +		dev_err(dev, "Invalid DDR details\n");
>> +	return valid;
>> +}
>> +
>> +static int is_custom_config_valid(struct emif_custom_configs *cust_cfgs,
>> +		struct device *dev)
>> +{
>> +	int valid = 1;
>> +
>> +	if ((cust_cfgs->mask&   EMIF_CUSTOM_CONFIG_LPMODE)&&
>> +		(cust_cfgs->lpmode != EMIF_LP_MODE_DISABLE))
>> +		valid = cust_cfgs->lpmode_freq_threshold&&
>> +			cust_cfgs->lpmode_timeout_performance&&
>> +			cust_cfgs->lpmode_timeout_power;
>> +
>> +	if (cust_cfgs->mask&   EMIF_CUSTOM_CONFIG_TEMP_ALERT_POLL_INTERVAL)
>> +		valid = valid&&   cust_cfgs->temp_alert_poll_interval_ms;
>> +
>> +	if (!valid)
>> +		dev_warn(dev, "Invalid custom configs\n");
>> +
>> +	return valid;
>> +}
>> +
>> +static struct emif_data * __init get_device_details(
>> +		struct platform_device *pdev)
>> +{
>> +	u32			size;
>> +	struct emif_data	*emif = NULL;
>> +	struct ddr_device_info	*dev_info;
>> +	struct emif_platform_data *pd;
>> +
>> +	pd = pdev->dev.platform_data;
>> +
>> +	if (!(pd&&   pd->device_info&&   is_dev_data_valid(pd->device_info->type,
>> +			pd->device_info->density, pd->device_info->io_width,
>> +			pd->phy_type, pd->ip_rev,&pdev->dev)))
>> +		goto error;
>> +
>> +	emif	= kzalloc(sizeof(struct emif_data), GFP_KERNEL);
>
> You should use the devm_* version of this API to get the simplify the error handling / removal.

Please note that most of my allocations are happening through
kmemdup(). kmemdup() doesn't have a devm_* equivalent. So, I have a
cleanup() function and in the interest of uniformity decided to avoid
devm_* variants altogether.

>
>> +	pd	= kmemdup(pd, sizeof(*pd), GFP_KERNEL);
>> +	dev_info = kmemdup(pd->device_info,
>> +			sizeof(*pd->device_info), GFP_KERNEL);
>> +
>> +	if (!emif || !pd || !dev_info)
>> +		goto error;
>
> The error report will not be really useful since you will not return the exact source of failure.
>

Ok. Will add a message right here.

>> +
>> +	emif->plat_data		= pd;
>> +	emif->dev		=&pdev->dev;
>> +	emif->temperature_level	= SDRAM_TEMP_NOMINAL;
>> +
>> +	pd->device_info	= dev_info;
>> +
>> +	/*
>> +	 * For EMIF instances other than EMIF1 see if the devices connected
>> +	 * are exactly same as on EMIF1(which is typically the case). If so,
>> +	 * mark it as a duplicate of EMIF1 and skip copying timings data.
>> +	 * This will save some memory and some computation later.
>> +	 */
>> +	emif->duplicate = emif1&&   (memcmp(dev_info,
>> +		emif1->plat_data->device_info,
>> +		sizeof(struct ddr_device_info)) == 0);
>> +
>> +	if (emif->duplicate) {
>> +		pd->timings = NULL;
>> +		pd->min_tck = NULL;
>> +		goto out;
>> +	}
>> +
>> +	/*
>> +	 * Copy custom configs - ignore allocation error, if any, as
>> +	 * custom_configs is not very critical
>> +	 */
>> +	if (pd->custom_configs)
>> +		pd->custom_configs = kmemdup(pd->custom_configs,
>> +			sizeof(*pd->custom_configs), GFP_KERNEL);
>> +
>> +	if (pd->custom_configs&&
>> +	    !is_custom_config_valid(pd->custom_configs, emif->dev)) {
>> +		kfree(pd->custom_configs);
>> +		pd->custom_configs = NULL;
>> +	}
>> +
>> +	/*
>> +	 * Copy timings and min-tck values from platform data. If it is not
>> +	 * available or if memory allocation fails, use JEDEC defaults
>> +	 */
>> +	size = sizeof(struct lpddr2_timings) * pd->timings_arr_size;
>> +	if (pd->timings)
>> +		pd->timings = kmemdup(pd->timings, size, GFP_KERNEL);
>> +
>> +	if (!pd->timings)
>> +		get_default_timings(emif);
>> +
>> +	if (pd->min_tck)
>> +		pd->min_tck = kmemdup(pd->min_tck, sizeof(*pd->min_tck),
>> +				GFP_KERNEL);
>> +
>> +	if (!pd->min_tck) {
>> +		pd->min_tck =&lpddr2_min_tck;
>> +		dev_warn(emif->dev, "Using default min-tck values\n");
>> +	}
>> +
>> +	goto out;
>> +
>> +error:
>> +	dev_err(&pdev->dev, "get_device_details() failure!!\n");
>
> That's not a very good error message, isn't?

Agree. Will fix it.

>
>> +	cleanup_emif(emif);
>> +	return NULL;
>> +
>> +out:
>> +	return emif;
>> +}
>> +
>> +static int __init emif_probe(struct platform_device *pdev)
>> +{
>> +	struct emif_data	*emif;
>> +	struct resource		*res;
>> +
>> +	emif = get_device_details(pdev);
>> +
>> +	if (!emif)
>> +		goto error;
>> +
>> +	if (!emif1)
>> +		emif1 = emif;
>> +
>> +	/* Save pointers to each other in emif and device structures */
>> +	emif->dev =&pdev->dev;
>> +	platform_set_drvdata(pdev, emif);
>> +
>> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +	if (!res)
>> +		goto error;
>
> You should reserve the region before ioremapping it. Something like that:
>
> if (!devm_request_mem_region(dev, res->start, resource_size(res), pdev->name)) {
>         dev_err(dev, "Region already claimed\n");
>         return -EBUSY;
> }
>
>> +
>> +	emif->base = ioremap(res->start, SZ_1M);
>
> Use devm_ioremap.

Will do. Guess devm_request_and_ioremap() will do for both.

>
>> +	if (!emif->base)
>> +		goto error;
>> +
>> +	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
>
> You should use platform_get_irq instead.

Ok Will do.

>
>> +	if (!res)
>> +		goto error;
>> +	emif->irq = res->start;
>> +
>> +	dev_info(&pdev->dev, "Device configured with addr = %p and IRQ%d\n",
>> +			emif->base, emif->irq);
>> +	return 0;
>> +
>> +error:
>> +	dev_err(&pdev->dev, "probe failure!!\n");
>
> Because??? You should be a little bit more verbose in case of failure.

Ok. Will add the error messages at the respective error locations.

Thanks for the review.

br,
Aneesh

Re: [RFC PATCH 4/8] misc: emif: add basic infrastructure for EMIF driver

[Cousson, Benoit]

Hi Aneesh,

On 2/17/2012 2:26 PM, Aneesh V wrote:
> On Thursday 16 February 2012 10:00 PM, Cousson, Benoit wrote:
>> On 2/4/2012 1:16 PM, Aneesh V wrote:

[...]

>>> +/**
>>> + * struct emif_data - Per device static data for driver's use
>>> + * @duplicate: Whether the DDR devices attached to this EMIF
>>> + * instance are exactly same as that on EMIF1. In
>>> + * this case we can save some memory and processing
>>> + * @temperature_level: Maximum temperature of LPDDR2 devices attached
>>> + * to this EMIF - read from MR4 register. If there
>>> + * are two devices attached to this EMIF, this
>>> + * value is the maximum of the two temperature
>>> + * levels.
>>> + * @irq: IRQ number
>>
>> Do you really need to store the IRQ number?
>
> Yes, I need it right now because setup_interrupts() is called later,
> after the first frequency notification, because that's when I have the
> registers to be programmed on a temperature event. But I am re-thinking
> on this strategy. I will move it back to probe() because other
> interrupts can/should be enabled at probe() time. When I do that I
> won't have to store it anymore and I will remove it.

Yes, I saw the code in a later patch. But in that case you should have 
introduced that attribute in the patch that will use it and not before.

But I do agree, that requesting the interrupt in the probe is probably 
better.

[...]

>>> + emif = kzalloc(sizeof(struct emif_data), GFP_KERNEL);
>>
>> You should use the devm_* version of this API to get the simplify the
>> error handling / removal.
>
> Please note that most of my allocations are happening through
> kmemdup(). kmemdup() doesn't have a devm_* equivalent. So, I have a
> cleanup() function and in the interest of uniformity decided to avoid
> devm_* variants altogether.

I think it still worth using devm_kzalloc + memcopy here instead of 
kmemdup to avoid the cleanup() and simplify as well the error handling.

You might even propose a new devm_kmemdup API if you want.

Regards,
Benoit

Re: [RFC PATCH 4/8] misc: emif: add basic infrastructure for EMIF driver

[Cousson, Benoit]

Hi Aneesh,

On 2/17/2012 2:26 PM, Aneesh V wrote:
> On Thursday 16 February 2012 10:00 PM, Cousson, Benoit wrote:
>> On 2/4/2012 1:16 PM, Aneesh V wrote:

[...]

>>> +/**
>>> + * struct emif_data - Per device static data for driver's use
>>> + * @duplicate: Whether the DDR devices attached to this EMIF
>>> + * instance are exactly same as that on EMIF1. In
>>> + * this case we can save some memory and processing
>>> + * @temperature_level: Maximum temperature of LPDDR2 devices attached
>>> + * to this EMIF - read from MR4 register. If there
>>> + * are two devices attached to this EMIF, this
>>> + * value is the maximum of the two temperature
>>> + * levels.
>>> + * @irq: IRQ number
>>
>> Do you really need to store the IRQ number?
>
> Yes, I need it right now because setup_interrupts() is called later,
> after the first frequency notification, because that's when I have the
> registers to be programmed on a temperature event. But I am re-thinking
> on this strategy. I will move it back to probe() because other
> interrupts can/should be enabled at probe() time. When I do that I
> won't have to store it anymore and I will remove it.

Yes, I saw the code in a later patch. But in that case you should have 
introduced that attribute in the patch that will use it and not before.

But I do agree, that requesting the interrupt in the probe is probably 
better.

[...]

>>> + emif = kzalloc(sizeof(struct emif_data), GFP_KERNEL);
>>
>> You should use the devm_* version of this API to get the simplify the
>> error handling / removal.
>
> Please note that most of my allocations are happening through
> kmemdup(). kmemdup() doesn't have a devm_* equivalent. So, I have a
> cleanup() function and in the interest of uniformity decided to avoid
> devm_* variants altogether.

I think it still worth using devm_kzalloc + memcopy here instead of 
kmemdup to avoid the cleanup() and simplify as well the error handling.

You might even propose a new devm_kmemdup API if you want.

Regards,
Benoit

Re: [RFC PATCH 0/8] Add TI EMIF SDRAM controller driver

[Aneesh V]

Greg,

On Thursday 16 February 2012 09:53 PM, Greg KH wrote:
> On Thu, Feb 16, 2012 at 04:21:11PM +0530, Santosh Shilimkar wrote:
>> Andrew, Greg,
>>
>> On Saturday 04 February 2012 05:46 PM, Aneesh V wrote:
>>> Add a driver for the EMIF SDRAM controller used in TI SoCs
>>>
>>> EMIF is an SDRAM controller that supports, based on its revision,
>>> one or more of LPDDR2/DDR2/DDR3 protocols.This driver adds support
>>> for LPDDR2.
>>>
>>> The driver supports the following features:
>>> - Calculates the DDR AC timing parameters to be set in EMIF
>>>    registers using data from the device data-sheets and based
>>>    on the DDR frequency. If data from data-sheets is not available
>>>    default timing values from the JEDEC spec are used. These
>>>    will be safe, but not necessarily optimal
>>> - API for changing timings during DVFS or at boot-up
>>> - Temperature alert configuration and handling of temperature
>>>    alerts, if any for LPDDR2 devices
>>>    * temperature alert is based on periodic polling of MR4 mode
>>>      register in DDR devices automatically performed by hardware
>>>    * timings are de-rated and brought back to nominal when
>>>      temperature raises and falls respectively
>>> - Cache of calculated register values to avoid re-calculating
>>>    them
>>>
>>> The driver will need some minor updates when it is eventually
>>> integrated with DVFS. This can not be done now as DVFS support
>>> is not available yet in mainline.
>>>
>>> Discussions with Santosh Shilimkar<santosh.shilimkar@ti.com>
>>> were immensely helpful in shaping up the interfaces. Vibhore Vardhan
>>> <vvardhan@gmail.com>  did the initial code snippet for thermal
>>> handling.
>>>
>>> Testing:
>>> - The driver is tested on OMAP4430 SDP.
>>> - The driver in a slightly adapted form is also tested on OMAP5.
>>> - Since mainline kernel doesn't have DVFS support yet,
>>>    testing was done using a test module.
>>> - Temperature alert handling was tested with simulated interrupts
>>>    and faked temperature values as testing all cases in real-life
>>>    scenarios is difficult.
>>>
>> [...]
>>
>>>   arch/arm/mach-omap2/omap_hwmod_44xx_data.c |  110 ++
>>>   drivers/misc/Kconfig                       |   20 +
>>>   drivers/misc/Makefile                      |    2 +
>>>   drivers/misc/emif.c                        | 1522 ++++++++++++++++++++++++++++
>>>   drivers/misc/emif_regs.h                   |  461 +++++++++
>>>   drivers/misc/jedec_ddr_data.c              |  141 +++
>>>   include/linux/emif.h                       |  257 +++++
>>>   include/linux/jedec_ddr.h                  |  174 ++++
>>
>> Any suggestion on where this driver can reside. It's a memory
>> controller driver which supports standard DDR functionality
>> as per JDEC specs including thermal alert. On top of
>> that it does support DVFS using the TI PRCM IP block.
>
> I don't know what any of those TLA words mean, so I really can't suggest

This is a driver for TI's memory controller(called EMIF). The
driver is needed for adjusting the controller settings on frequency,
voltage, and temperature changes. Any suggestion as to where this
should go?

> where this code should go.  But just from this diffstat, it looks like
> you are creating a new user/kernel interface, without documenting it
> anywhere, which isn't ok.

I think you are referring to the header files added in include/linux/
They are not creating new user/kernel interface per se.

"include/linux/jedec_ddr.h" is the interface to a library that contains
data from the DDR specs. "include/linux/emif.h" has definitions for
platform data needed by the driver. Maybe these should go to some other
sub-directory within include/ or include/linux/ ?

I shall add documentation for the driver in the next revision.

Thanks,
Aneesh

Re: [RFC PATCH 4/8] misc: emif: add basic infrastructure for EMIF driver

[Aneesh V]

On Friday 17 February 2012 07:14 PM, Cousson, Benoit wrote:
> Hi Aneesh,

[...]

>>>> + emif = kzalloc(sizeof(struct emif_data), GFP_KERNEL);
>>>
>>> You should use the devm_* version of this API to get the simplify the
>>> error handling / removal.
>>
>> Please note that most of my allocations are happening through
>> kmemdup(). kmemdup() doesn't have a devm_* equivalent. So, I have a
>> cleanup() function and in the interest of uniformity decided to avoid
>> devm_* variants altogether.
>
> I think it still worth using devm_kzalloc + memcopy here instead of
> kmemdup to avoid the cleanup() and simplify as well the error handling.

I will do that.

>
> You might even propose a new devm_kmemdup API if you want.

Ok. I will attempt that, maybe both devm_kmalloc() and devm_kmemdup().
But I would like to de-couple that from this series. That is, I will do
the patch separately and if that gets up-streamed I will update EMIF
driver to use them. Until then I will go with what you suggested above.

br,
Aneesh

Re: [RFC PATCH 0/8] Add TI EMIF SDRAM controller driver

[Greg KH]

On Fri, Feb 17, 2012 at 07:26:29PM +0530, Aneesh V wrote:
> Greg,
> 
> On Thursday 16 February 2012 09:53 PM, Greg KH wrote:
> >On Thu, Feb 16, 2012 at 04:21:11PM +0530, Santosh Shilimkar wrote:
> >>Andrew, Greg,
> >>
> >>On Saturday 04 February 2012 05:46 PM, Aneesh V wrote:
> >>>Add a driver for the EMIF SDRAM controller used in TI SoCs
> >>>
> >>>EMIF is an SDRAM controller that supports, based on its revision,
> >>>one or more of LPDDR2/DDR2/DDR3 protocols.This driver adds support
> >>>for LPDDR2.
> >>>
> >>>The driver supports the following features:
> >>>- Calculates the DDR AC timing parameters to be set in EMIF
> >>>   registers using data from the device data-sheets and based
> >>>   on the DDR frequency. If data from data-sheets is not available
> >>>   default timing values from the JEDEC spec are used. These
> >>>   will be safe, but not necessarily optimal
> >>>- API for changing timings during DVFS or at boot-up
> >>>- Temperature alert configuration and handling of temperature
> >>>   alerts, if any for LPDDR2 devices
> >>>   * temperature alert is based on periodic polling of MR4 mode
> >>>     register in DDR devices automatically performed by hardware
> >>>   * timings are de-rated and brought back to nominal when
> >>>     temperature raises and falls respectively
> >>>- Cache of calculated register values to avoid re-calculating
> >>>   them
> >>>
> >>>The driver will need some minor updates when it is eventually
> >>>integrated with DVFS. This can not be done now as DVFS support
> >>>is not available yet in mainline.
> >>>
> >>>Discussions with Santosh Shilimkar<santosh.shilimkar@ti.com>
> >>>were immensely helpful in shaping up the interfaces. Vibhore Vardhan
> >>><vvardhan@gmail.com>  did the initial code snippet for thermal
> >>>handling.
> >>>
> >>>Testing:
> >>>- The driver is tested on OMAP4430 SDP.
> >>>- The driver in a slightly adapted form is also tested on OMAP5.
> >>>- Since mainline kernel doesn't have DVFS support yet,
> >>>   testing was done using a test module.
> >>>- Temperature alert handling was tested with simulated interrupts
> >>>   and faked temperature values as testing all cases in real-life
> >>>   scenarios is difficult.
> >>>
> >>[...]
> >>
> >>>  arch/arm/mach-omap2/omap_hwmod_44xx_data.c |  110 ++
> >>>  drivers/misc/Kconfig                       |   20 +
> >>>  drivers/misc/Makefile                      |    2 +
> >>>  drivers/misc/emif.c                        | 1522 ++++++++++++++++++++++++++++
> >>>  drivers/misc/emif_regs.h                   |  461 +++++++++
> >>>  drivers/misc/jedec_ddr_data.c              |  141 +++
> >>>  include/linux/emif.h                       |  257 +++++
> >>>  include/linux/jedec_ddr.h                  |  174 ++++
> >>
> >>Any suggestion on where this driver can reside. It's a memory
> >>controller driver which supports standard DDR functionality
> >>as per JDEC specs including thermal alert. On top of
> >>that it does support DVFS using the TI PRCM IP block.
> >
> >I don't know what any of those TLA words mean, so I really can't suggest
> 
> This is a driver for TI's memory controller(called EMIF). The
> driver is needed for adjusting the controller settings on frequency,
> voltage, and temperature changes. Any suggestion as to where this
> should go?

For those type of things, don't the iio framework provide what you need
and what?  Or perhaps the cpufreq layer?

> >where this code should go.  But just from this diffstat, it looks like
> >you are creating a new user/kernel interface, without documenting it
> >anywhere, which isn't ok.
> 
> I think you are referring to the header files added in include/linux/
> They are not creating new user/kernel interface per se.

Then why are they in include/linux/ ?

> "include/linux/jedec_ddr.h" is the interface to a library that contains
> data from the DDR specs. "include/linux/emif.h" has definitions for
> platform data needed by the driver. Maybe these should go to some other
> sub-directory within include/ or include/linux/ ?

Who needs these files?  If it's only the drivers themselves, then put it
in the same directory as the driver.  If it's platform data, then put
it, and only it, in the include/linux/platform_data/ directory.

> I shall add documentation for the driver in the next revision.

That would be good.  Please also cc: me on the next revision if you wish
me to take the patches (hint, get_maintainer.pl should have told you
this...)

thanks,

greg k-h

Re: [RFC PATCH 0/8] Add TI EMIF SDRAM controller driver

[Aneesh V]

On Friday 17 February 2012 11:20 PM, Greg KH wrote:
> On Fri, Feb 17, 2012 at 07:26:29PM +0530, Aneesh V wrote:

[...]

>>> I don't know what any of those TLA words mean, so I really can't suggest
>>
>> This is a driver for TI's memory controller(called EMIF). The
>> driver is needed for adjusting the controller settings on frequency,
>> voltage, and temperature changes. Any suggestion as to where this
>> should go?
>
> For those type of things, don't the iio framework provide what you need
> and what?  Or perhaps the cpufreq layer?
>

I don't think this driver fits into the iio framework. This is not a
sensor or IO kind of device. Neither does it generate events. The
primary responsibility of this driver is to re-configure the SDRAM
controller settings on all transient events affecting it after the
bootloader has set it up at boot-time. These are typically events
generated by other sub-systems in the kernel such as the clock
framework (DDR frequency change) regulator framework (voltage
transitions) etc. Temperature events are generated (by polling the
SDRAM device) and handled within the driver.

Neither do I think it will fit into cpufreq layer because DDR
frequency  can be triggered by clock framework independent of cpufreq.
Moreover handling DDR frequency change is only one of the functions of
the driver.

  >>> where this code should go.  But just from this diffstat, it looks like
>>> you are creating a new user/kernel interface, without documenting it
>>> anywhere, which isn't ok.
>>
>> I think you are referring to the header files added in include/linux/
>> They are not creating new user/kernel interface per se.
>
> Then why are they in include/linux/ ?

My mistake. I will move them to more appropriate places.

>
>> "include/linux/jedec_ddr.h" is the interface to a library that contains
>> data from the DDR specs. "include/linux/emif.h" has definitions for
>> platform data needed by the driver. Maybe these should go to some other
>> sub-directory within include/ or include/linux/ ?
>
> Who needs these files?  If it's only the drivers themselves, then put it
> in the same directory as the driver.  If it's platform data, then put
> it, and only it, in the include/linux/platform_data/ directory.

The above work has two components:
1. The driver
2. A small library with data from the SDRAM specs that the driver uses.

Alan Cox has suggested to move the library part to lib/ so the
corresponding header file jedec_ddr.h has to be in some common place.
Can this be in include/misc or do you have a better place to suggest?

The other one emif.h can be split into two parts, one for platform data
that can go under include/linux/platform_data/ and the rest in the same
directory as the driver, as you suggested.

>
>> I shall add documentation for the driver in the next revision.
>
> That would be good.  Please also cc: me on the next revision if you wish
> me to take the patches (hint, get_maintainer.pl should have told you
> this...)

Sure. Thanks.

- Aneesh

Re: [RFC PATCH 4/8] misc: emif: add basic infrastructure for EMIF driver

[Aneesh V]

On Thursday 16 February 2012 10:00 PM, Cousson, Benoit wrote:
> Hi Aneesh,
>

[...]

>> +struct emif_data {
>> +	u8				duplicate;
>> +	u8				temperature_level;
>> +	u32				irq;
>> +	spinlock_t			lock; /* lock to prevent races */
>
> Nit: That comment is useless, since you already have the kerneldoc comment before.

Now I remember why I did that. Without that comment checkpatch gives
this "check".

CHECK: spinlock_t definition without comment
#124: FILE: drivers/misc/emif.c:54:
+	spinlock_t			lock;

br,
Aneesh

Re: [RFC PATCH 4/8] misc: emif: add basic infrastructure for EMIF driver

[Cousson, Benoit]

On 2/24/2012 12:10 PM, Aneesh V wrote:
> On Thursday 16 February 2012 10:00 PM, Cousson, Benoit wrote:
>> Hi Aneesh,
>>
>
> [...]
>
>>> +struct emif_data {
>>> + u8 duplicate;
>>> + u8 temperature_level;
>>> + u32 irq;
>>> + spinlock_t lock; /* lock to prevent races */
>>
>> Nit: That comment is useless, since you already have the kerneldoc
>> comment before.
>
> Now I remember why I did that. Without that comment checkpatch gives
> this "check".
>
> CHECK: spinlock_t definition without comment
> #124: FILE: drivers/misc/emif.c:54:
> + spinlock_t lock;

That's a pretty interesting comment :-)
I guess checkpatch should be able to check for a potential kerneldoc as 
well. You might want to report that to the checkpatch maintainer.

Thanks,
Benoit

Re: [RFC PATCH 4/8] misc: emif: add basic infrastructure for EMIF driver

[Cousson, Benoit]

On 2/24/2012 12:10 PM, Aneesh V wrote:
> On Thursday 16 February 2012 10:00 PM, Cousson, Benoit wrote:
>> Hi Aneesh,
>>
>
> [...]
>
>>> +struct emif_data {
>>> + u8 duplicate;
>>> + u8 temperature_level;
>>> + u32 irq;
>>> + spinlock_t lock; /* lock to prevent races */
>>
>> Nit: That comment is useless, since you already have the kerneldoc
>> comment before.
>
> Now I remember why I did that. Without that comment checkpatch gives
> this "check".
>
> CHECK: spinlock_t definition without comment
> #124: FILE: drivers/misc/emif.c:54:
> + spinlock_t lock;

That's a pretty interesting comment :-)
I guess checkpatch should be able to check for a potential kerneldoc as 
well. You might want to report that to the checkpatch maintainer.

Thanks,
Benoit