This series resolves a few minor issues for EMIF driver.
Tested all patches on OMAP4430-sdp.
Patch : "memory: emif: setup LP settings on freq update"
is tested on a local tree, since freq update cannot be
tested on mainline.
Ambresh K (1):
memory: emif: setup LP settings on freq update
Grygorii Strashko (1):
memory: emif: errata i743: Prohibit usage of Power-Down mode
Lokesh Vutla (2):
memory: emif: Correct the lpmode timeout calculation
memory: emif: Load the correct custom config values from dt
Nishanth Menon (3):
memory: emif: handle overflow for timing for LP mode
memory: emif: Handle devices which are not rated for >85C
memory: emif: use restart if power_off not present when out of spec
Oleksandr Dmytryshyn (1):
memory: emif: fix timings initialization issue
drivers/memory/emif.c | 122 +++++++++++++++++++++++++++----
include/linux/platform_data/emif_plat.h | 1 +
2 files changed, 108 insertions(+), 15 deletions(-)
--
1.7.9.5
From: Grygorii Strashko <[email protected]>
ERRATA DESCRIPTION :
The EMIF supports power-down state for low power. The EMIF
automatically puts the SDRAM into power-down after the memory is
not accessed for a defined number of cycles and the
EMIF_PWR_MGMT_CTRL[10:8] REG_LP_MODE bit field is set to 0x4.
As the EMIF supports automatic output impedance calibration, a ZQ
calibration long command is issued every time it exits active
power-down and precharge power-down modes. The EMIF waits and
blocks any other command during this calibration.
The EMIF does not allow selective disabling of ZQ calibration upon
exit of power-down mode. Due to very short periods of power-down
cycles, ZQ calibration overhead creates bandwidth issues and
increases overall system power consumption. On the other hand,
issuing ZQ calibration long commands when exiting self-refresh is
still required.
WORKAROUND :
Because there is no power consumption benefit of the power-down due
to the calibration and there is a performance risk, the guideline
is to not allow power-down state and, therefore, to not have set
the EMIF_PWR_MGMT_CTRL[10:8] REG_LP_MODE bit field to 0x4.
Signed-off-by: Grygorii Strashko <[email protected]>
Signed-off-by: Vitaly Chernooky <[email protected]>
Signed-off-by: Oleksandr Dmytryshyn <[email protected]>
Signed-off-by: Lokesh Vutla <[email protected]>
---
drivers/memory/emif.c | 35 +++++++++++++++++++++++++++++++++++
1 file changed, 35 insertions(+)
diff --git a/drivers/memory/emif.c b/drivers/memory/emif.c
index f75806a..119503a 100644
--- a/drivers/memory/emif.c
+++ b/drivers/memory/emif.c
@@ -257,6 +257,41 @@ static void set_lpmode(struct emif_data *emif, u8 lpmode)
u32 temp;
void __iomem *base = emif->base;
+ /*
+ * Workaround for errata i743 - LPDDR2 Power-Down State is Not
+ * Efficient
+ *
+ * i743 DESCRIPTION:
+ * The EMIF supports power-down state for low power. The EMIF
+ * automatically puts the SDRAM into power-down after the memory is
+ * not accessed for a defined number of cycles and the
+ * EMIF_PWR_MGMT_CTRL[10:8] REG_LP_MODE bit field is set to 0x4.
+ * As the EMIF supports automatic output impedance calibration, a ZQ
+ * calibration long command is issued every time it exits active
+ * power-down and precharge power-down modes. The EMIF waits and
+ * blocks any other command during this calibration.
+ * The EMIF does not allow selective disabling of ZQ calibration upon
+ * exit of power-down mode. Due to very short periods of power-down
+ * cycles, ZQ calibration overhead creates bandwidth issues and
+ * increases overall system power consumption. On the other hand,
+ * issuing ZQ calibration long commands when exiting self-refresh is
+ * still required.
+ *
+ * WORKAROUND
+ * Because there is no power consumption benefit of the power-down due
+ * to the calibration and there is a performance risk, the guideline
+ * is to not allow power-down state and, therefore, to not have set
+ * the EMIF_PWR_MGMT_CTRL[10:8] REG_LP_MODE bit field to 0x4.
+ */
+ if ((emif->plat_data->ip_rev == EMIF_4D) &&
+ (EMIF_LP_MODE_PWR_DN == lpmode)) {
+ WARN_ONCE(1,
+ "REG_LP_MODE = LP_MODE_PWR_DN(4) is prohibited by"
+ "erratum i743 switch to LP_MODE_SELF_REFRESH(2)\n");
+ /* rallback LP_MODE to Self-refresh mode */
+ lpmode = EMIF_LP_MODE_SELF_REFRESH;
+ }
+
temp = readl(base + EMIF_POWER_MANAGEMENT_CONTROL);
temp &= ~LP_MODE_MASK;
temp |= (lpmode << LP_MODE_SHIFT);
--
1.7.9.5
The driver tries to round up the specified timeout cycles to
the next power of 2 value. But this is done wrongly.
Correcting this here.
Reported-by: Nishanth Menon <[email protected]>
Signed-off-by: Lokesh Vutla <[email protected]>
---
drivers/memory/emif.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/memory/emif.c b/drivers/memory/emif.c
index df08736..622638c 100644
--- a/drivers/memory/emif.c
+++ b/drivers/memory/emif.c
@@ -732,9 +732,9 @@ static u32 get_pwr_mgmt_ctrl(u32 freq, struct emif_data *emif, u32 ip_rev)
if (timeout < 16) {
timeout = 0;
} else {
- timeout = __fls(timeout) - 3;
if (timeout & (timeout - 1))
- timeout++;
+ timeout <<= 1;
+ timeout = __fls(timeout) - 3;
}
switch (lpmode) {
--
1.7.9.5
From: Nishanth Menon <[email protected]>
Some machine or kernel variants might have missed implementation
of power off handlers. We DONOT want to let the system be in
"out of spec" state in this condition. So, WARN and attempt
a machine restart in the hopes of clearing the out-of-spec
temperature condition.
NOTE: This is not the safest option, but safer than leaving the
system in unstable conditions.
Signed-off-by: Nishanth Menon <[email protected]>
Signed-off-by: Lokesh Vutla <[email protected]>
---
drivers/memory/emif.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/memory/emif.c b/drivers/memory/emif.c
index 927fb55..02a94fc 100644
--- a/drivers/memory/emif.c
+++ b/drivers/memory/emif.c
@@ -25,6 +25,7 @@
#include <linux/module.h>
#include <linux/list.h>
#include <linux/spinlock.h>
+#include <linux/pm.h>
#include <memory/jedec_ddr.h>
#include "emif.h"
#include "of_memory.h"
@@ -1011,7 +1012,13 @@ static irqreturn_t emif_threaded_isr(int irq, void *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();
+ /* If we have Power OFF ability, use it, else try restarting */
+ if (pm_power_off) {
+ kernel_power_off();
+ } else {
+ WARN(1, "FIXME: NO pm_power_off!!! trying restart\n");
+ kernel_restart("SDRAM Over-temp Emergency restart");
+ }
return IRQ_HANDLED;
}
--
1.7.9.5
From: Oleksandr Dmytryshyn <[email protected]>
The issue was that only the first timings table was added to the
emif platform data at the emif driver registration. All other
timings tables was filled with zeros. Now all emif timings table
are added to the platform data.
Signed-off-by: Oleksandr Dmytryshyn <[email protected]>
Signed-off-by: Lokesh Vutla <[email protected]>
---
drivers/memory/emif.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/memory/emif.c b/drivers/memory/emif.c
index 02a94fc..f75806a 100644
--- a/drivers/memory/emif.c
+++ b/drivers/memory/emif.c
@@ -1463,7 +1463,7 @@ static struct emif_data *__init_or_module get_device_details(
if (pd->timings) {
temp = devm_kzalloc(dev, size, GFP_KERNEL);
if (temp) {
- memcpy(temp, pd->timings, sizeof(*pd->timings));
+ memcpy(temp, pd->timings, size);
pd->timings = temp;
} else {
dev_warn(dev, "%s:%d: allocation error\n", __func__,
--
1.7.9.5
From: Nishanth Menon <[email protected]>
In case the custom timings provide values which overflow
the maximum possible field value, warn and use maximum
permissible value.
Signed-off-by: Nishanth Menon <[email protected]>
Signed-off-by: Lokesh Vutla <[email protected]>
---
drivers/memory/emif.c | 36 ++++++++++++++++++++++++++++--------
1 file changed, 28 insertions(+), 8 deletions(-)
diff --git a/drivers/memory/emif.c b/drivers/memory/emif.c
index 16f5089..37e0c77 100644
--- a/drivers/memory/emif.c
+++ b/drivers/memory/emif.c
@@ -715,6 +715,8 @@ static u32 get_pwr_mgmt_ctrl(u32 freq, struct emif_data *emif, u32 ip_rev)
u32 timeout_perf = EMIF_LP_MODE_TIMEOUT_PERFORMANCE;
u32 timeout_pwr = EMIF_LP_MODE_TIMEOUT_POWER;
u32 freq_threshold = EMIF_LP_MODE_FREQ_THRESHOLD;
+ u32 mask;
+ u8 shift;
struct emif_custom_configs *cust_cfgs = emif->plat_data->custom_configs;
@@ -739,27 +741,45 @@ static u32 get_pwr_mgmt_ctrl(u32 freq, struct emif_data *emif, u32 ip_rev)
switch (lpmode) {
case EMIF_LP_MODE_CLOCK_STOP:
- pwr_mgmt_ctrl = (timeout << CS_TIM_SHIFT) |
- SR_TIM_MASK | PD_TIM_MASK;
+ shift = CS_TIM_SHIFT;
+ mask = CS_TIM_MASK;
break;
case EMIF_LP_MODE_SELF_REFRESH:
/* Workaround for errata i735 */
if (timeout < 6)
timeout = 6;
- pwr_mgmt_ctrl = (timeout << SR_TIM_SHIFT) |
- CS_TIM_MASK | PD_TIM_MASK;
+ shift = SR_TIM_SHIFT;
+ mask = SR_TIM_MASK;
break;
case EMIF_LP_MODE_PWR_DN:
- pwr_mgmt_ctrl = (timeout << PD_TIM_SHIFT) |
- CS_TIM_MASK | SR_TIM_MASK;
+ shift = PD_TIM_SHIFT;
+ mask = PD_TIM_MASK;
break;
case EMIF_LP_MODE_DISABLE:
default:
- pwr_mgmt_ctrl = CS_TIM_MASK |
- PD_TIM_MASK | SR_TIM_MASK;
+ mask = 0;
+ shift = 0;
+ break;
+ }
+ /* Round to maximum in case of overflow, BUT warn! */
+ if (lpmode != EMIF_LP_MODE_DISABLE && timeout > mask >> shift) {
+ pr_err("TIMEOUT Overflow - lpmode=%d perf=%d pwr=%d freq=%d\n",
+ lpmode,
+ timeout_perf,
+ timeout_pwr,
+ freq_threshold);
+ WARN(1, "timeout=0x%02x greater than 0x%02x. Using max\n",
+ timeout, mask >> shift);
+ timeout = mask >> shift;
}
+ /* Setup required timing */
+ pwr_mgmt_ctrl = (timeout << shift) & mask;
+ /* setup a default mask for rest of the modes */
+ pwr_mgmt_ctrl |= (SR_TIM_MASK | CS_TIM_MASK | PD_TIM_MASK) &
+ ~mask;
+
/* No CS_TIM in EMIF_4D5 */
if (ip_rev == EMIF_4D5)
pwr_mgmt_ctrl &= ~CS_TIM_MASK;
--
1.7.9.5
of_get_property returns value in Big Endian format.
Before using this value it should be converted to little endian
using be32_to_cpup().
Custom configs of emif are read from dt using of_get_property,
but these are not converted to litte endian format.
Correcting the same here.
Signed-off-by: Lokesh Vutla <[email protected]>
---
drivers/memory/emif.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/memory/emif.c b/drivers/memory/emif.c
index 119503a..4866f1b 100644
--- a/drivers/memory/emif.c
+++ b/drivers/memory/emif.c
@@ -1258,7 +1258,7 @@ static void __init_or_module of_get_custom_configs(struct device_node *np_emif,
{
struct emif_custom_configs *cust_cfgs = NULL;
int len;
- const int *lpmode, *poll_intvl;
+ const __be32 *lpmode, *poll_intvl;
lpmode = of_get_property(np_emif, "low-power-mode", &len);
poll_intvl = of_get_property(np_emif, "temp-alert-poll-interval", &len);
@@ -1272,7 +1272,7 @@ static void __init_or_module of_get_custom_configs(struct device_node *np_emif,
if (lpmode) {
cust_cfgs->mask |= EMIF_CUSTOM_CONFIG_LPMODE;
- cust_cfgs->lpmode = *lpmode;
+ cust_cfgs->lpmode = be32_to_cpup(lpmode);
of_property_read_u32(np_emif,
"low-power-mode-timeout-performance",
&cust_cfgs->lpmode_timeout_performance);
@@ -1287,7 +1287,8 @@ static void __init_or_module of_get_custom_configs(struct device_node *np_emif,
if (poll_intvl) {
cust_cfgs->mask |=
EMIF_CUSTOM_CONFIG_TEMP_ALERT_POLL_INTERVAL;
- cust_cfgs->temp_alert_poll_interval_ms = *poll_intvl;
+ cust_cfgs->temp_alert_poll_interval_ms =
+ be32_to_cpup(poll_intvl);
}
if (of_find_property(np_emif, "extended-temp-part", &len))
--
1.7.9.5
From: Ambresh K <[email protected]>
Program the power management shadow register on freq update
Else the concept of threshold frequencies dont really matter
as the system always uses the performance mode timing for LP
which is programmed in at init time.
Signed-off-by: Nishanth Menon <[email protected]>
Signed-off-by: Ambresh K <[email protected]>
Signed-off-by: Lokesh Vutla <[email protected]>
---
drivers/memory/emif.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/memory/emif.c b/drivers/memory/emif.c
index 622638c..16f5089 100644
--- a/drivers/memory/emif.c
+++ b/drivers/memory/emif.c
@@ -815,6 +815,8 @@ static void setup_registers(struct emif_data *emif, struct emif_regs *regs)
writel(regs->sdram_tim2_shdw, base + EMIF_SDRAM_TIMING_2_SHDW);
writel(regs->phy_ctrl_1_shdw, base + EMIF_DDR_PHY_CTRL_1_SHDW);
+ writel(regs->pwr_mgmt_ctrl_shdw,
+ base + EMIF_POWER_MANAGEMENT_CTRL_SHDW);
/* Settings specific for EMIF4D5 */
if (emif->plat_data->ip_rev != EMIF_4D5)
--
1.7.9.5
From: Nishanth Menon <[email protected]>
As per JESD209-2E specification for LPDDR2,
http://www.jedec.org/standards-documents/results/jesd209-2E
Table 73, LPDDR2 memories come in two flavors - Standard and
Extended. The Standard types can operate from -25C to +85C
However, beyond that and upto +105C can only be supported by
Extended types.
Unfortunately, it seems there is no info in MR0(device info) or
MR[1,2](device feature) for run time detection of this capability
as far as seen on the spec. Hence, we provide a custom_config
flag to be populated by platforms which have these "extended"
type memories.
For the "Standard" memories, we need to consider MR4 notifications
of temperature triggers >85C as equivalent to thermal shutdown
events (equivalent to Spec specified thermal shutdown events for
"extended" parts).
Reported-by: Richard Woodruff <[email protected]>
Signed-off-by: Nishanth Menon <[email protected]>
Signed-off-by: Lokesh Vutla <[email protected]>
---
drivers/memory/emif.c | 27 +++++++++++++++++++++++++++
include/linux/platform_data/emif_plat.h | 1 +
2 files changed, 28 insertions(+)
diff --git a/drivers/memory/emif.c b/drivers/memory/emif.c
index 37e0c77..927fb55 100644
--- a/drivers/memory/emif.c
+++ b/drivers/memory/emif.c
@@ -914,6 +914,7 @@ static irqreturn_t handle_temp_alert(void __iomem *base, struct emif_data *emif)
{
u32 old_temp_level;
irqreturn_t ret = IRQ_HANDLED;
+ struct emif_custom_configs *custom_configs;
spin_lock_irqsave(&emif_lock, irq_state);
old_temp_level = emif->temperature_level;
@@ -926,6 +927,29 @@ static irqreturn_t handle_temp_alert(void __iomem *base, struct emif_data *emif)
goto out;
}
+ custom_configs = emif->plat_data->custom_configs;
+
+ /*
+ * IF we detect higher than "nominal rating" from DDR sensor
+ * on an unsupported DDR part, shutdown system
+ */
+ if (custom_configs && !(custom_configs->mask &
+ EMIF_CUSTOM_CONFIG_EXTENDED_TEMP_PART)) {
+ if (emif->temperature_level >= SDRAM_TEMP_HIGH_DERATE_REFRESH) {
+ dev_err(emif->dev,
+ "%s:NOT Extended temperature capable memory."
+ "Converting MR4=0x%02x as shutdown event\n",
+ __func__, emif->temperature_level);
+ /*
+ * Temperature far too high - do kernel_power_off()
+ * from thread context
+ */
+ emif->temperature_level = SDRAM_TEMP_VERY_HIGH_SHUTDOWN;
+ ret = IRQ_WAKE_THREAD;
+ goto out;
+ }
+ }
+
if (emif->temperature_level < old_temp_level ||
emif->temperature_level == SDRAM_TEMP_VERY_HIGH_SHUTDOWN) {
/*
@@ -1224,6 +1248,9 @@ static void __init_or_module of_get_custom_configs(struct device_node *np_emif,
cust_cfgs->temp_alert_poll_interval_ms = *poll_intvl;
}
+ if (of_find_property(np_emif, "extended-temp-part", &len))
+ cust_cfgs->mask |= EMIF_CUSTOM_CONFIG_EXTENDED_TEMP_PART;
+
if (!is_custom_config_valid(cust_cfgs, emif->dev)) {
devm_kfree(emif->dev, cust_cfgs);
return;
diff --git a/include/linux/platform_data/emif_plat.h b/include/linux/platform_data/emif_plat.h
index 03378ca..5c19a2a 100644
--- a/include/linux/platform_data/emif_plat.h
+++ b/include/linux/platform_data/emif_plat.h
@@ -40,6 +40,7 @@
/* Custom config requests */
#define EMIF_CUSTOM_CONFIG_LPMODE 0x00000001
#define EMIF_CUSTOM_CONFIG_TEMP_ALERT_POLL_INTERVAL 0x00000002
+#define EMIF_CUSTOM_CONFIG_EXTENDED_TEMP_PART 0x00000004
#ifndef __ASSEMBLY__
/**
--
1.7.9.5
minor nit.
$subject
s/Correct/Fix
On Monday 11 March 2013 10:35 AM, Lokesh Vutla wrote:
> The driver tries to round up the specified timeout cycles to
> the next power of 2 value. But this is done wrongly.
> Correcting this here.
>
Change needs to be improved here. See below.
> Reported-by: Nishanth Menon <[email protected]>
> Signed-off-by: Lokesh Vutla <[email protected]>
> ---
> drivers/memory/emif.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/memory/emif.c b/drivers/memory/emif.c
> index df08736..622638c 100644
> --- a/drivers/memory/emif.c
> +++ b/drivers/memory/emif.c
> @@ -732,9 +732,9 @@ static u32 get_pwr_mgmt_ctrl(u32 freq, struct emif_data *emif, u32 ip_rev)
> if (timeout < 16) {
> timeout = 0;
> } else {
> - timeout = __fls(timeout) - 3;
> if (timeout & (timeout - 1))
So from the change, it appears that, the timeout
check for power of 2 should be done before updating
the variable which seems to be the right fix.
> - timeout++;
> + timeout <<= 1;
> + timeout = __fls(timeout) - 3;
> }
>
> switch (lpmode) {
>
So just make changelog verbose as well as add a
comment about the calculation in the code.
Otherwise, patch looks fine to me.
Acked-by: Santosh Shilimkar <[email protected]>
On Monday 11 March 2013 10:35 AM, Lokesh Vutla wrote:
> From: Ambresh K <[email protected]>
>
> Program the power management shadow register on freq update
> Else the concept of threshold frequencies dont really matter
> as the system always uses the performance mode timing for LP
> which is programmed in at init time.
>
> Signed-off-by: Nishanth Menon <[email protected]>
> Signed-off-by: Ambresh K <[email protected]>
> Signed-off-by: Lokesh Vutla <[email protected]>
> ---
Acked-by: Santosh Shilimkar <[email protected]>
On Monday 11 March 2013 10:36 AM, Lokesh Vutla wrote:
> From: Nishanth Menon <[email protected]>
>
> In case the custom timings provide values which overflow
> the maximum possible field value, warn and use maximum
> permissible value.
>
> Signed-off-by: Nishanth Menon <[email protected]>
> Signed-off-by: Lokesh Vutla <[email protected]>
> ---
Acked-by: Santosh Shilimkar <[email protected]>
On Monday 11 March 2013 10:36 AM, Lokesh Vutla wrote:
> From: Nishanth Menon <[email protected]>
>
> As per JESD209-2E specification for LPDDR2,
> http://www.jedec.org/standards-documents/results/jesd209-2E
> Table 73, LPDDR2 memories come in two flavors - Standard and
> Extended. The Standard types can operate from -25C to +85C
> However, beyond that and upto +105C can only be supported by
> Extended types.
>
right.
> Unfortunately, it seems there is no info in MR0(device info) or
> MR[1,2](device feature) for run time detection of this capability
> as far as seen on the spec. Hence, we provide a custom_config
> flag to be populated by platforms which have these "extended"
> type memories.
>
> For the "Standard" memories, we need to consider MR4 notifications
> of temperature triggers >85C as equivalent to thermal shutdown
> events (equivalent to Spec specified thermal shutdown events for
> "extended" parts).
>
Make sense.
> Reported-by: Richard Woodruff <[email protected]>
> Signed-off-by: Nishanth Menon <[email protected]>
> Signed-off-by: Lokesh Vutla <[email protected]>
> ---
Thanks for the fix.
Acked-by: Santosh Shilimkar <[email protected]>
On Monday 11 March 2013 10:36 AM, Lokesh Vutla wrote:
> From: Nishanth Menon <[email protected]>
>
> Some machine or kernel variants might have missed implementation
> of power off handlers. We DONOT want to let the system be in
> "out of spec" state in this condition. So, WARN and attempt
> a machine restart in the hopes of clearing the out-of-spec
> temperature condition.
>
> NOTE: This is not the safest option, but safer than leaving the
> system in unstable conditions.
>
> Signed-off-by: Nishanth Menon <[email protected]>
> Signed-off-by: Lokesh Vutla <[email protected]>
> ---
> drivers/memory/emif.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/memory/emif.c b/drivers/memory/emif.c
> index 927fb55..02a94fc 100644
> --- a/drivers/memory/emif.c
> +++ b/drivers/memory/emif.c
> @@ -25,6 +25,7 @@
> #include <linux/module.h>
> #include <linux/list.h>
> #include <linux/spinlock.h>
> +#include <linux/pm.h>
> #include <memory/jedec_ddr.h>
> #include "emif.h"
> #include "of_memory.h"
> @@ -1011,7 +1012,13 @@ static irqreturn_t emif_threaded_isr(int irq, void *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();
Extra line here would be good here.
> + /* If we have Power OFF ability, use it, else try restarting */
> + if (pm_power_off) {
> + kernel_power_off();
> + } else {
> + WARN(1, "FIXME: NO pm_power_off!!! trying restart\n");
> + kernel_restart("SDRAM Over-temp Emergency restart");
> + }
> return IRQ_HANDLED;
> }
>
Otherwise,
Acked-by: Santosh Shilimkar <[email protected]>
$subject is too vague. What issue ?
Some thing like "Fix the incorrect 'size' parameter in memcpy' etc
On Monday 11 March 2013 10:36 AM, Lokesh Vutla wrote:
> From: Oleksandr Dmytryshyn <[email protected]>
>
> The issue was that only the first timings table was added to the
> emif platform data at the emif driver registration. All other
> timings tables was filled with zeros. Now all emif timings table
> are added to the platform data.
>
Luckily, most of the cases, first table has been re-used for
symmetric configuration.
> Signed-off-by: Oleksandr Dmytryshyn <[email protected]>
> Signed-off-by: Lokesh Vutla <[email protected]>
> ---
> drivers/memory/emif.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/memory/emif.c b/drivers/memory/emif.c
> index 02a94fc..f75806a 100644
> --- a/drivers/memory/emif.c
> +++ b/drivers/memory/emif.c
> @@ -1463,7 +1463,7 @@ static struct emif_data *__init_or_module get_device_details(
> if (pd->timings) {
> temp = devm_kzalloc(dev, size, GFP_KERNEL);
> if (temp) {
> - memcpy(temp, pd->timings, sizeof(*pd->timings));
> + memcpy(temp, pd->timings, size);
> pd->timings = temp;
> } else {
> dev_warn(dev, "%s:%d: allocation error\n", __func__,
>
Patch as such looks good to me.
Acked-by: Santosh Shilimkar <[email protected]>
On Monday 11 March 2013 10:36 AM, Lokesh Vutla wrote:
> From: Grygorii Strashko <[email protected]>
>
> ERRATA DESCRIPTION :
> The EMIF supports power-down state for low power. The EMIF
> automatically puts the SDRAM into power-down after the memory is
> not accessed for a defined number of cycles and the
> EMIF_PWR_MGMT_CTRL[10:8] REG_LP_MODE bit field is set to 0x4.
> As the EMIF supports automatic output impedance calibration, a ZQ
> calibration long command is issued every time it exits active
> power-down and precharge power-down modes. The EMIF waits and
> blocks any other command during this calibration.
> The EMIF does not allow selective disabling of ZQ calibration upon
> exit of power-down mode. Due to very short periods of power-down
> cycles, ZQ calibration overhead creates bandwidth issues and
> increases overall system power consumption. On the other hand,
> issuing ZQ calibration long commands when exiting self-refresh is
> still required.
>
> WORKAROUND :
> Because there is no power consumption benefit of the power-down due
> to the calibration and there is a performance risk, the guideline
> is to not allow power-down state and, therefore, to not have set
> the EMIF_PWR_MGMT_CTRL[10:8] REG_LP_MODE bit field to 0x4.
>
> Signed-off-by: Grygorii Strashko <[email protected]>
> Signed-off-by: Vitaly Chernooky <[email protected]>
> Signed-off-by: Oleksandr Dmytryshyn <[email protected]>
> Signed-off-by: Lokesh Vutla <[email protected]>
> ---
Nice changelog.
> drivers/memory/emif.c | 35 +++++++++++++++++++++++++++++++++++
> 1 file changed, 35 insertions(+)
>
> diff --git a/drivers/memory/emif.c b/drivers/memory/emif.c
> index f75806a..119503a 100644
> --- a/drivers/memory/emif.c
> +++ b/drivers/memory/emif.c
> @@ -257,6 +257,41 @@ static void set_lpmode(struct emif_data *emif, u8 lpmode)
> u32 temp;
> void __iomem *base = emif->base;
>
> + /*
> + * Workaround for errata i743 - LPDDR2 Power-Down State is Not
> + * Efficient
> + *
> + * i743 DESCRIPTION:
> + * The EMIF supports power-down state for low power. The EMIF
> + * automatically puts the SDRAM into power-down after the memory is
> + * not accessed for a defined number of cycles and the
> + * EMIF_PWR_MGMT_CTRL[10:8] REG_LP_MODE bit field is set to 0x4.
> + * As the EMIF supports automatic output impedance calibration, a ZQ
> + * calibration long command is issued every time it exits active
> + * power-down and precharge power-down modes. The EMIF waits and
> + * blocks any other command during this calibration.
> + * The EMIF does not allow selective disabling of ZQ calibration upon
> + * exit of power-down mode. Due to very short periods of power-down
> + * cycles, ZQ calibration overhead creates bandwidth issues and
> + * increases overall system power consumption. On the other hand,
> + * issuing ZQ calibration long commands when exiting self-refresh is
> + * still required.
> + *
> + * WORKAROUND
> + * Because there is no power consumption benefit of the power-down due
> + * to the calibration and there is a performance risk, the guideline
> + * is to not allow power-down state and, therefore, to not have set
> + * the EMIF_PWR_MGMT_CTRL[10:8] REG_LP_MODE bit field to 0x4.
> + */
> + if ((emif->plat_data->ip_rev == EMIF_4D) &&
> + (EMIF_LP_MODE_PWR_DN == lpmode)) {
Ok. So the errata is limited to only 'EMIF_4D' version and not applicable
for next EMIF version used in OMAP5 devices, right ? If yes, would be good
to just mention that in already good changelog.
> + WARN_ONCE(1,
> + "REG_LP_MODE = LP_MODE_PWR_DN(4) is prohibited by"
> + "erratum i743 switch to LP_MODE_SELF_REFRESH(2)\n");
> + /* rallback LP_MODE to Self-refresh mode */
s/rallback/rollback ?
With above updates,
Acked-by: Santosh Shilimkar <[email protected]>
On Monday 11 March 2013 10:36 AM, Lokesh Vutla wrote:
> of_get_property returns value in Big Endian format.
> Before using this value it should be converted to little endian
> using be32_to_cpup().
> Custom configs of emif are read from dt using of_get_property,
> but these are not converted to litte endian format.
> Correcting the same here.
>
> Signed-off-by: Lokesh Vutla <[email protected]>
> ---
Looks fine.
Acked-by: Santosh Shilimkar <[email protected]>
Hi,
On Monday 11 March 2013 11:20 AM, Santosh Shilimkar wrote:
> On Monday 11 March 2013 10:36 AM, Lokesh Vutla wrote:
>> From: Grygorii Strashko <[email protected]>
>>
>> ERRATA DESCRIPTION :
>> The EMIF supports power-down state for low power. The EMIF
>> automatically puts the SDRAM into power-down after the memory is
>> not accessed for a defined number of cycles and the
>> EMIF_PWR_MGMT_CTRL[10:8] REG_LP_MODE bit field is set to 0x4.
>> As the EMIF supports automatic output impedance calibration, a ZQ
>> calibration long command is issued every time it exits active
>> power-down and precharge power-down modes. The EMIF waits and
>> blocks any other command during this calibration.
>> The EMIF does not allow selective disabling of ZQ calibration upon
>> exit of power-down mode. Due to very short periods of power-down
>> cycles, ZQ calibration overhead creates bandwidth issues and
>> increases overall system power consumption. On the other hand,
>> issuing ZQ calibration long commands when exiting self-refresh is
>> still required.
>>
>> WORKAROUND :
>> Because there is no power consumption benefit of the power-down due
>> to the calibration and there is a performance risk, the guideline
>> is to not allow power-down state and, therefore, to not have set
>> the EMIF_PWR_MGMT_CTRL[10:8] REG_LP_MODE bit field to 0x4.
>>
>> Signed-off-by: Grygorii Strashko <[email protected]>
>> Signed-off-by: Vitaly Chernooky <[email protected]>
>> Signed-off-by: Oleksandr Dmytryshyn <[email protected]>
>> Signed-off-by: Lokesh Vutla <[email protected]>
>> ---
> Nice changelog.
>
>> drivers/memory/emif.c | 35 +++++++++++++++++++++++++++++++++++
>> 1 file changed, 35 insertions(+)
>>
>> diff --git a/drivers/memory/emif.c b/drivers/memory/emif.c
>> index f75806a..119503a 100644
>> --- a/drivers/memory/emif.c
>> +++ b/drivers/memory/emif.c
>> @@ -257,6 +257,41 @@ static void set_lpmode(struct emif_data *emif, u8 lpmode)
>> u32 temp;
>> void __iomem *base = emif->base;
>>
>> + /*
>> + * Workaround for errata i743 - LPDDR2 Power-Down State is Not
>> + * Efficient
>> + *
>> + * i743 DESCRIPTION:
>> + * The EMIF supports power-down state for low power. The EMIF
>> + * automatically puts the SDRAM into power-down after the memory is
>> + * not accessed for a defined number of cycles and the
>> + * EMIF_PWR_MGMT_CTRL[10:8] REG_LP_MODE bit field is set to 0x4.
>> + * As the EMIF supports automatic output impedance calibration, a ZQ
>> + * calibration long command is issued every time it exits active
>> + * power-down and precharge power-down modes. The EMIF waits and
>> + * blocks any other command during this calibration.
>> + * The EMIF does not allow selective disabling of ZQ calibration upon
>> + * exit of power-down mode. Due to very short periods of power-down
>> + * cycles, ZQ calibration overhead creates bandwidth issues and
>> + * increases overall system power consumption. On the other hand,
>> + * issuing ZQ calibration long commands when exiting self-refresh is
>> + * still required.
>> + *
>> + * WORKAROUND
>> + * Because there is no power consumption benefit of the power-down due
>> + * to the calibration and there is a performance risk, the guideline
>> + * is to not allow power-down state and, therefore, to not have set
>> + * the EMIF_PWR_MGMT_CTRL[10:8] REG_LP_MODE bit field to 0x4.
>> + */
>> + if ((emif->plat_data->ip_rev == EMIF_4D) &&
>> + (EMIF_LP_MODE_PWR_DN == lpmode)) {
> Ok. So the errata is limited to only 'EMIF_4D' version and not applicable
> for next EMIF version used in OMAP5 devices, right ? If yes, would be good
> to just mention that in already good changelog.
Yes, it is not applicable for EMIF_4D5 used in OMAP5 ES2.0 Soc's.
Ill update this in change log.
Thanks
Lokesh
>
>> + WARN_ONCE(1,
>> + "REG_LP_MODE = LP_MODE_PWR_DN(4) is prohibited by"
>> + "erratum i743 switch to LP_MODE_SELF_REFRESH(2)\n");
>> + /* rallback LP_MODE to Self-refresh mode */
> s/rallback/rollback ?
>
> With above updates,
> Acked-by: Santosh Shilimkar <[email protected]>
>
On Mon, Mar 11, 2013 at 10:35:57AM +0530, Lokesh Vutla wrote:
> This series resolves a few minor issues for EMIF driver.
>
> Tested all patches on OMAP4430-sdp.
> Patch : "memory: emif: setup LP settings on freq update"
> is tested on a local tree, since freq update cannot be
> tested on mainline.
Can you please resend these with all of the requested changes made, and
acks added?
thanks,
greg k-h
On Friday 15 March 2013 11:38 PM, Greg KH wrote:
> On Mon, Mar 11, 2013 at 10:35:57AM +0530, Lokesh Vutla wrote:
>> This series resolves a few minor issues for EMIF driver.
>>
>> Tested all patches on OMAP4430-sdp.
>> Patch : "memory: emif: setup LP settings on freq update"
>> is tested on a local tree, since freq update cannot be
>> tested on mainline.
>
> Can you please resend these with all of the requested changes made, and
> acks added?
Ok ill send V2 with necessary changes and acks added..
Thanks,
Lokesh
>
> thanks,
>
> greg k-h
>