* [RFC PATCH 1/8] OMAP4: hwmod: add EMIF hw mod data
2012-02-04 12:16 [RFC PATCH 0/8] Add TI EMIF SDRAM controller driver Aneesh V
@ 2012-02-04 12:16 ` Aneesh V
2012-02-16 10:02 ` Santosh Shilimkar
2012-02-04 12:16 ` [RFC PATCH 2/8] misc: ddr: add LPDDR2 data from JESD209-2 Aneesh V
` (7 subsequent siblings)
8 siblings, 1 reply; 39+ messages in thread
From: Aneesh V @ 2012-02-04 12:16 UTC (permalink / raw)
To: linux-omap; +Cc: linux-kernel, akpm, Benoit Cousson
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
^ permalink raw reply related [flat|nested] 39+ messages in thread* [RFC PATCH 2/8] misc: ddr: add LPDDR2 data from JESD209-2
2012-02-04 12:16 [RFC PATCH 0/8] Add TI EMIF SDRAM controller driver Aneesh V
2012-02-04 12:16 ` [RFC PATCH 1/8] OMAP4: hwmod: add EMIF hw mod data Aneesh V
@ 2012-02-04 12:16 ` Aneesh V
2012-02-16 10:06 ` Santosh Shilimkar
2012-02-16 10:07 ` Santosh Shilimkar
2012-02-04 12:16 ` [RFC PATCH 3/8] misc: emif: add register definitions for EMIF Aneesh V
` (6 subsequent siblings)
8 siblings, 2 replies; 39+ messages in thread
From: Aneesh V @ 2012-02-04 12:16 UTC (permalink / raw)
To: linux-omap; +Cc: linux-kernel, akpm, 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
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [RFC PATCH 2/8] misc: ddr: add LPDDR2 data from JESD209-2
2012-02-04 12:16 ` [RFC PATCH 2/8] misc: ddr: add LPDDR2 data from JESD209-2 Aneesh V
@ 2012-02-16 10:06 ` Santosh Shilimkar
2012-02-16 10:07 ` Santosh Shilimkar
1 sibling, 0 replies; 39+ messages in thread
From: Santosh Shilimkar @ 2012-02-16 10:06 UTC (permalink / raw)
To: Aneesh V; +Cc: linux-omap, linux-kernel, akpm
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
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RFC PATCH 2/8] misc: ddr: add LPDDR2 data from JESD209-2
2012-02-04 12:16 ` [RFC PATCH 2/8] misc: ddr: add LPDDR2 data from JESD209-2 Aneesh V
2012-02-16 10:06 ` Santosh Shilimkar
@ 2012-02-16 10:07 ` Santosh Shilimkar
2012-02-16 10:27 ` Aneesh V
1 sibling, 1 reply; 39+ messages in thread
From: Santosh Shilimkar @ 2012-02-16 10:07 UTC (permalink / raw)
To: Aneesh V; +Cc: linux-omap, linux-kernel, akpm
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
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RFC PATCH 2/8] misc: ddr: add LPDDR2 data from JESD209-2
2012-02-16 10:07 ` Santosh Shilimkar
@ 2012-02-16 10:27 ` Aneesh V
2012-02-16 11:10 ` Alan Cox
0 siblings, 1 reply; 39+ messages in thread
From: Aneesh V @ 2012-02-16 10:27 UTC (permalink / raw)
To: Santosh Shilimkar; +Cc: linux-omap, linux-kernel, akpm
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.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RFC PATCH 2/8] misc: ddr: add LPDDR2 data from JESD209-2
2012-02-16 10:27 ` Aneesh V
@ 2012-02-16 11:10 ` Alan Cox
2012-02-16 11:25 ` Shilimkar, Santosh
2012-02-16 11:55 ` Aneesh V
0 siblings, 2 replies; 39+ messages in thread
From: Alan Cox @ 2012-02-16 11:10 UTC (permalink / raw)
To: Aneesh V; +Cc: Santosh Shilimkar, linux-omap, linux-kernel, akpm
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
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RFC PATCH 2/8] misc: ddr: add LPDDR2 data from JESD209-2
2012-02-16 11:10 ` Alan Cox
@ 2012-02-16 11:25 ` Shilimkar, Santosh
2012-02-16 11:55 ` Aneesh V
1 sibling, 0 replies; 39+ messages in thread
From: Shilimkar, Santosh @ 2012-02-16 11:25 UTC (permalink / raw)
To: Alan Cox; +Cc: Aneesh V, linux-omap, linux-kernel, akpm
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
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RFC PATCH 2/8] misc: ddr: add LPDDR2 data from JESD209-2
2012-02-16 11:10 ` Alan Cox
2012-02-16 11:25 ` Shilimkar, Santosh
@ 2012-02-16 11:55 ` Aneesh V
1 sibling, 0 replies; 39+ messages in thread
From: Aneesh V @ 2012-02-16 11:55 UTC (permalink / raw)
To: Alan Cox; +Cc: Santosh Shilimkar, linux-omap, linux-kernel, akpm
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
^ permalink raw reply [flat|nested] 39+ messages in thread
* [RFC PATCH 3/8] misc: emif: add register definitions for EMIF
2012-02-04 12:16 [RFC PATCH 0/8] Add TI EMIF SDRAM controller driver Aneesh V
2012-02-04 12:16 ` [RFC PATCH 1/8] OMAP4: hwmod: add EMIF hw mod data Aneesh V
2012-02-04 12:16 ` [RFC PATCH 2/8] misc: ddr: add LPDDR2 data from JESD209-2 Aneesh V
@ 2012-02-04 12:16 ` Aneesh V
2012-02-16 10:10 ` Santosh Shilimkar
2012-02-04 12:16 ` [RFC PATCH 4/8] misc: emif: add basic infrastructure for EMIF driver Aneesh V
` (5 subsequent siblings)
8 siblings, 1 reply; 39+ messages in thread
From: Aneesh V @ 2012-02-04 12:16 UTC (permalink / raw)
To: linux-omap; +Cc: linux-kernel, akpm, 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
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [RFC PATCH 3/8] misc: emif: add register definitions for EMIF
2012-02-04 12:16 ` [RFC PATCH 3/8] misc: emif: add register definitions for EMIF Aneesh V
@ 2012-02-16 10:10 ` Santosh Shilimkar
2012-02-16 10:30 ` Aneesh V
0 siblings, 1 reply; 39+ messages in thread
From: Santosh Shilimkar @ 2012-02-16 10:10 UTC (permalink / raw)
To: Aneesh V; +Cc: linux-omap, linux-kernel, akpm
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
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RFC PATCH 3/8] misc: emif: add register definitions for EMIF
2012-02-16 10:10 ` Santosh Shilimkar
@ 2012-02-16 10:30 ` Aneesh V
0 siblings, 0 replies; 39+ messages in thread
From: Aneesh V @ 2012-02-16 10:30 UTC (permalink / raw)
To: Santosh Shilimkar; +Cc: linux-omap, linux-kernel, akpm
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
^ permalink raw reply [flat|nested] 39+ messages in thread
* [RFC PATCH 4/8] misc: emif: add basic infrastructure for EMIF driver
2012-02-04 12:16 [RFC PATCH 0/8] Add TI EMIF SDRAM controller driver Aneesh V
` (2 preceding siblings ...)
2012-02-04 12:16 ` [RFC PATCH 3/8] misc: emif: add register definitions for EMIF Aneesh V
@ 2012-02-04 12:16 ` Aneesh V
2012-02-16 10:33 ` Santosh Shilimkar
2012-02-16 16:30 ` Cousson, Benoit
2012-02-04 12:16 ` [RFC PATCH 5/8] misc: emif: handle frequency and voltage change events Aneesh V
` (4 subsequent siblings)
8 siblings, 2 replies; 39+ messages in thread
From: Aneesh V @ 2012-02-04 12:16 UTC (permalink / raw)
To: linux-omap; +Cc: linux-kernel, akpm, 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
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [RFC PATCH 4/8] misc: emif: add basic infrastructure for EMIF driver
2012-02-04 12:16 ` [RFC PATCH 4/8] misc: emif: add basic infrastructure for EMIF driver Aneesh V
@ 2012-02-16 10:33 ` Santosh Shilimkar
2012-02-16 11:15 ` Aneesh V
2012-02-16 16:30 ` Cousson, Benoit
1 sibling, 1 reply; 39+ messages in thread
From: Santosh Shilimkar @ 2012-02-16 10:33 UTC (permalink / raw)
To: Aneesh V; +Cc: linux-omap, linux-kernel, akpm
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
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [RFC PATCH 4/8] misc: emif: add basic infrastructure for EMIF driver
2012-02-16 10:33 ` Santosh Shilimkar
@ 2012-02-16 11:15 ` Aneesh V
0 siblings, 0 replies; 39+ messages in thread
From: Aneesh V @ 2012-02-16 11:15 UTC (permalink / raw)
To: Santosh Shilimkar; +Cc: linux-omap, linux-kernel, akpm
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
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RFC PATCH 4/8] misc: emif: add basic infrastructure for EMIF driver
2012-02-04 12:16 ` [RFC PATCH 4/8] misc: emif: add basic infrastructure for EMIF driver Aneesh V
2012-02-16 10:33 ` Santosh Shilimkar
@ 2012-02-16 16:30 ` Cousson, Benoit
2012-02-17 13:26 ` Aneesh V
2012-02-24 11:10 ` Aneesh V
1 sibling, 2 replies; 39+ messages in thread
From: Cousson, Benoit @ 2012-02-16 16:30 UTC (permalink / raw)
To: Aneesh V; +Cc: linux-omap, linux-kernel, akpm
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 */
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [RFC PATCH 4/8] misc: emif: add basic infrastructure for EMIF driver
2012-02-16 16:30 ` Cousson, Benoit
@ 2012-02-17 13:26 ` Aneesh V
2012-02-17 13:44 ` Cousson, Benoit
2012-02-24 11:10 ` Aneesh V
1 sibling, 1 reply; 39+ messages in thread
From: Aneesh V @ 2012-02-17 13:26 UTC (permalink / raw)
To: Cousson, Benoit; +Cc: linux-omap, linux-kernel, akpm
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
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [RFC PATCH 4/8] misc: emif: add basic infrastructure for EMIF driver
2012-02-17 13:26 ` Aneesh V
@ 2012-02-17 13:44 ` Cousson, Benoit
2012-02-17 15:27 ` Aneesh V
0 siblings, 1 reply; 39+ messages in thread
From: Cousson, Benoit @ 2012-02-17 13:44 UTC (permalink / raw)
To: Aneesh V; +Cc: linux-omap, linux-kernel, akpm
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
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RFC PATCH 4/8] misc: emif: add basic infrastructure for EMIF driver
2012-02-17 13:44 ` Cousson, Benoit
@ 2012-02-17 15:27 ` Aneesh V
0 siblings, 0 replies; 39+ messages in thread
From: Aneesh V @ 2012-02-17 15:27 UTC (permalink / raw)
To: Cousson, Benoit; +Cc: linux-omap, linux-kernel, akpm
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
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RFC PATCH 4/8] misc: emif: add basic infrastructure for EMIF driver
2012-02-16 16:30 ` Cousson, Benoit
2012-02-17 13:26 ` Aneesh V
@ 2012-02-24 11:10 ` Aneesh V
2012-02-24 11:16 ` Cousson, Benoit
1 sibling, 1 reply; 39+ messages in thread
From: Aneesh V @ 2012-02-24 11:10 UTC (permalink / raw)
To: Cousson, Benoit; +Cc: linux-omap, linux-kernel, akpm
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
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [RFC PATCH 4/8] misc: emif: add basic infrastructure for EMIF driver
2012-02-24 11:10 ` Aneesh V
@ 2012-02-24 11:16 ` Cousson, Benoit
0 siblings, 0 replies; 39+ messages in thread
From: Cousson, Benoit @ 2012-02-24 11:16 UTC (permalink / raw)
To: Aneesh V; +Cc: linux-omap, linux-kernel, akpm
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
^ permalink raw reply [flat|nested] 39+ messages in thread
* [RFC PATCH 5/8] misc: emif: handle frequency and voltage change events
2012-02-04 12:16 [RFC PATCH 0/8] Add TI EMIF SDRAM controller driver Aneesh V
` (3 preceding siblings ...)
2012-02-04 12:16 ` [RFC PATCH 4/8] misc: emif: add basic infrastructure for EMIF driver Aneesh V
@ 2012-02-04 12:16 ` Aneesh V
2012-02-16 10:38 ` Santosh Shilimkar
2012-02-04 12:16 ` [RFC PATCH 6/8] misc: emif: add interrupt and temperature handling Aneesh V
` (3 subsequent siblings)
8 siblings, 1 reply; 39+ messages in thread
From: Aneesh V @ 2012-02-04 12:16 UTC (permalink / raw)
To: linux-omap; +Cc: linux-kernel, akpm, 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
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [RFC PATCH 5/8] misc: emif: handle frequency and voltage change events
2012-02-04 12:16 ` [RFC PATCH 5/8] misc: emif: handle frequency and voltage change events Aneesh V
@ 2012-02-16 10:38 ` Santosh Shilimkar
2012-02-16 11:22 ` Aneesh V
0 siblings, 1 reply; 39+ messages in thread
From: Santosh Shilimkar @ 2012-02-16 10:38 UTC (permalink / raw)
To: Aneesh V; +Cc: linux-omap, linux-kernel, akpm
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.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RFC PATCH 5/8] misc: emif: handle frequency and voltage change events
2012-02-16 10:38 ` Santosh Shilimkar
@ 2012-02-16 11:22 ` Aneesh V
0 siblings, 0 replies; 39+ messages in thread
From: Aneesh V @ 2012-02-16 11:22 UTC (permalink / raw)
To: Santosh Shilimkar; +Cc: linux-omap, linux-kernel, akpm
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.
^ permalink raw reply [flat|nested] 39+ messages in thread
* [RFC PATCH 6/8] misc: emif: add interrupt and temperature handling
2012-02-04 12:16 [RFC PATCH 0/8] Add TI EMIF SDRAM controller driver Aneesh V
` (4 preceding siblings ...)
2012-02-04 12:16 ` [RFC PATCH 5/8] misc: emif: handle frequency and voltage change events Aneesh V
@ 2012-02-04 12:16 ` Aneesh V
2012-02-16 10:41 ` Santosh Shilimkar
2012-02-04 12:16 ` [RFC PATCH 7/8] misc: emif: add one-time settings Aneesh V
` (2 subsequent siblings)
8 siblings, 1 reply; 39+ messages in thread
From: Aneesh V @ 2012-02-04 12:16 UTC (permalink / raw)
To: linux-omap; +Cc: linux-kernel, akpm, 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
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [RFC PATCH 6/8] misc: emif: add interrupt and temperature handling
2012-02-04 12:16 ` [RFC PATCH 6/8] misc: emif: add interrupt and temperature handling Aneesh V
@ 2012-02-16 10:41 ` Santosh Shilimkar
2012-02-16 11:50 ` Aneesh V
0 siblings, 1 reply; 39+ messages in thread
From: Santosh Shilimkar @ 2012-02-16 10:41 UTC (permalink / raw)
To: Aneesh V; +Cc: linux-omap, linux-kernel, akpm
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
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [RFC PATCH 6/8] misc: emif: add interrupt and temperature handling
2012-02-16 10:41 ` Santosh Shilimkar
@ 2012-02-16 11:50 ` Aneesh V
0 siblings, 0 replies; 39+ messages in thread
From: Aneesh V @ 2012-02-16 11:50 UTC (permalink / raw)
To: Santosh Shilimkar; +Cc: linux-omap, linux-kernel, akpm
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
^ permalink raw reply [flat|nested] 39+ messages in thread
* [RFC PATCH 7/8] misc: emif: add one-time settings
2012-02-04 12:16 [RFC PATCH 0/8] Add TI EMIF SDRAM controller driver Aneesh V
` (5 preceding siblings ...)
2012-02-04 12:16 ` [RFC PATCH 6/8] misc: emif: add interrupt and temperature handling Aneesh V
@ 2012-02-04 12:16 ` Aneesh V
2012-02-16 10:44 ` Santosh Shilimkar
2012-02-04 12:16 ` [RFC PATCH 8/8] misc: emif: add debugfs entries for emif Aneesh V
2012-02-16 10:51 ` [RFC PATCH 0/8] Add TI EMIF SDRAM controller driver Santosh Shilimkar
8 siblings, 1 reply; 39+ messages in thread
From: Aneesh V @ 2012-02-04 12:16 UTC (permalink / raw)
To: linux-omap; +Cc: linux-kernel, akpm, 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
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [RFC PATCH 7/8] misc: emif: add one-time settings
2012-02-04 12:16 ` [RFC PATCH 7/8] misc: emif: add one-time settings Aneesh V
@ 2012-02-16 10:44 ` Santosh Shilimkar
2012-02-16 11:56 ` Aneesh V
0 siblings, 1 reply; 39+ messages in thread
From: Santosh Shilimkar @ 2012-02-16 10:44 UTC (permalink / raw)
To: Aneesh V; +Cc: linux-omap, linux-kernel, akpm
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
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RFC PATCH 7/8] misc: emif: add one-time settings
2012-02-16 10:44 ` Santosh Shilimkar
@ 2012-02-16 11:56 ` Aneesh V
0 siblings, 0 replies; 39+ messages in thread
From: Aneesh V @ 2012-02-16 11:56 UTC (permalink / raw)
To: Santosh Shilimkar; +Cc: linux-omap, linux-kernel, akpm
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
^ permalink raw reply [flat|nested] 39+ messages in thread
* [RFC PATCH 8/8] misc: emif: add debugfs entries for emif
2012-02-04 12:16 [RFC PATCH 0/8] Add TI EMIF SDRAM controller driver Aneesh V
` (6 preceding siblings ...)
2012-02-04 12:16 ` [RFC PATCH 7/8] misc: emif: add one-time settings Aneesh V
@ 2012-02-04 12:16 ` Aneesh V
2012-02-16 10:46 ` Santosh Shilimkar
2012-02-16 10:51 ` [RFC PATCH 0/8] Add TI EMIF SDRAM controller driver Santosh Shilimkar
8 siblings, 1 reply; 39+ messages in thread
From: Aneesh V @ 2012-02-04 12:16 UTC (permalink / raw)
To: linux-omap; +Cc: linux-kernel, akpm, 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
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [RFC PATCH 0/8] Add TI EMIF SDRAM controller driver
2012-02-04 12:16 [RFC PATCH 0/8] Add TI EMIF SDRAM controller driver Aneesh V
` (7 preceding siblings ...)
2012-02-04 12:16 ` [RFC PATCH 8/8] misc: emif: add debugfs entries for emif Aneesh V
@ 2012-02-16 10:51 ` Santosh Shilimkar
2012-02-16 16:23 ` Greg KH
8 siblings, 1 reply; 39+ messages in thread
From: Santosh Shilimkar @ 2012-02-16 10:51 UTC (permalink / raw)
To: akpm, greg; +Cc: Aneesh V, linux-omap, linux-kernel
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
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [RFC PATCH 0/8] Add TI EMIF SDRAM controller driver
2012-02-16 10:51 ` [RFC PATCH 0/8] Add TI EMIF SDRAM controller driver Santosh Shilimkar
@ 2012-02-16 16:23 ` Greg KH
2012-02-17 13:56 ` Aneesh V
0 siblings, 1 reply; 39+ messages in thread
From: Greg KH @ 2012-02-16 16:23 UTC (permalink / raw)
To: Santosh Shilimkar; +Cc: akpm, Aneesh V, linux-omap, linux-kernel
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
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RFC PATCH 0/8] Add TI EMIF SDRAM controller driver
2012-02-16 16:23 ` Greg KH
@ 2012-02-17 13:56 ` Aneesh V
2012-02-17 17:50 ` Greg KH
0 siblings, 1 reply; 39+ messages in thread
From: Aneesh V @ 2012-02-17 13:56 UTC (permalink / raw)
To: Greg KH; +Cc: Santosh Shilimkar, akpm, linux-omap, linux-kernel
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
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RFC PATCH 0/8] Add TI EMIF SDRAM controller driver
2012-02-17 13:56 ` Aneesh V
@ 2012-02-17 17:50 ` Greg KH
2012-02-20 14:07 ` Aneesh V
0 siblings, 1 reply; 39+ messages in thread
From: Greg KH @ 2012-02-17 17:50 UTC (permalink / raw)
To: Aneesh V; +Cc: Santosh Shilimkar, akpm, linux-omap, linux-kernel
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
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RFC PATCH 0/8] Add TI EMIF SDRAM controller driver
2012-02-17 17:50 ` Greg KH
@ 2012-02-20 14:07 ` Aneesh V
0 siblings, 0 replies; 39+ messages in thread
From: Aneesh V @ 2012-02-20 14:07 UTC (permalink / raw)
To: Greg KH; +Cc: Santosh Shilimkar, akpm, linux-omap, linux-kernel
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
^ permalink raw reply [flat|nested] 39+ messages in thread