* [U-Boot] [PATCH v2] mtd: nand: Fix address cycle problem with NAND_CMD_RNDOUT
@ 2014-05-06 9:15 Stefan Roese
2014-05-06 9:25 ` Gupta, Pekon
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Stefan Roese @ 2014-05-06 9:15 UTC (permalink / raw
To: u-boot
From: Marek Belisko <marek.belisko@gmail.com>
From: Marek Belisko <marek.belisko@gmail.com>
On some NAND devices (e.g. Hynix H27U2G8F2CTR-BI on Siemens DXR2 /
Draco boards) the NAND subsystem (SPL & U-Boot drivers) issues the following
bit-flip error messages:
nand: bit-flip corrected @oob=0
...
NAND_CMD_RNDOUT (05h-E0h) needs only two cycles of column address to access data
from different column within the same page. So expected sequence on NAND data-bus is
<05h> <column-addr-byte1> <column-address-byte2> <E0h>
References in datasheets of parts from different NAND vendors
+--------+------------------------+---------------------------------------------
|Vendor | Datasheet/Part# | Reference
+--------+------------------------+---------------------------------------------
|Spansion| S34ML{01|02|04}G2 | Figure 6.12 Random Data Output In a Page
|Micron | MT29F{16|32|64|128}G08A| Figure 47: CHANGE READ COLUMN (05h-E0h) Operation
|Macronix| MX30LF1G08AA | Figure 10. AC Waveforms for Random Data Output
|Toshiba | TC58NVG1S3ETAI0 | Figure Column Address Change in Read Cycle Timing
| | | Diagram (2/2)
+--------+------------------------+---------------------------------------------
Though most NAND Devices mentioned above tend to work even if extra cycles of
page-address is issued between <05h> .... <E0h> command. But Spansion devices
break on this non-compliance.
This patch fixes chip->cmdfunc() for all devices, as datasheet of all mentioned
vendor expect same sequence.
The same issue has been reported by Bacem Daassi:
http://e2e.ti.com/support/arm/sitara_arm/f/791/t/259699.aspx
"
The issue is mainly due to a NAND protocol violation in the omap driver since
the Random Data Output command (05h-E0h) expects to see only the column address
that should be addressed within the already loaded read page into the read
buffer. Only 2 address cycles with ALE active should be provided between the
05h and E0h commands. The Page read command expects the full address footprint
(2bytes for column address + 3bytes for row address), but once the page is
loaded into the read buffer, Random Data Output should be used with only 2bytes
for column address.
"
Signed-off-by: Marek Belisko <marek.belisko@gmail.com>
Signed-off-by: Stefan Roese <sr@denx.de>
Tested-by: Samuel Egli <samuel.egli@siemens.com>
Cc: Pekon Gupta <pekon@ti.com>
Cc: Scott Wood <scottwood@freescale.com>
---
v2:
- Marek as author
- Added additional comment / datasheet reference to the commit
text from Pekon (thanks)
- Added fix to nand_spl_simple.c
- Squashed all three drivers fixes into one patch
drivers/mtd/nand/am335x_spl_bch.c | 12 ++++++++----
drivers/mtd/nand/nand_base.c | 2 +-
drivers/mtd/nand/nand_spl_simple.c | 12 ++++++++----
3 files changed, 17 insertions(+), 9 deletions(-)
diff --git a/drivers/mtd/nand/am335x_spl_bch.c b/drivers/mtd/nand/am335x_spl_bch.c
index c84851b..9b547ce 100644
--- a/drivers/mtd/nand/am335x_spl_bch.c
+++ b/drivers/mtd/nand/am335x_spl_bch.c
@@ -63,15 +63,19 @@ static int nand_command(int block, int page, uint32_t offs,
hwctrl(&mtd, offs & 0xff,
NAND_CTRL_ALE | NAND_CTRL_CHANGE); /* A[7:0] */
hwctrl(&mtd, (offs >> 8) & 0xff, NAND_CTRL_ALE); /* A[11:9] */
+
/* Row address */
- hwctrl(&mtd, (page_addr & 0xff), NAND_CTRL_ALE); /* A[19:12] */
- hwctrl(&mtd, ((page_addr >> 8) & 0xff),
+ if (cmd != NAND_CMD_RNDOUT) {
+ hwctrl(&mtd, (page_addr & 0xff), NAND_CTRL_ALE); /* A[19:12] */
+ hwctrl(&mtd, ((page_addr >> 8) & 0xff),
NAND_CTRL_ALE); /* A[27:20] */
#ifdef CONFIG_SYS_NAND_5_ADDR_CYCLE
- /* One more address cycle for devices > 128MiB */
- hwctrl(&mtd, (page_addr >> 16) & 0x0f,
+ /* One more address cycle for devices > 128MiB */
+ hwctrl(&mtd, (page_addr >> 16) & 0x0f,
NAND_CTRL_ALE); /* A[31:28] */
#endif
+ }
+
hwctrl(&mtd, NAND_CMD_NONE, NAND_NCE | NAND_CTRL_CHANGE);
if (cmd == NAND_CMD_READ0) {
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 1ce55fd..f11fce4 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -674,7 +674,7 @@ static void nand_command_lp(struct mtd_info *mtd, unsigned int command,
ctrl &= ~NAND_CTRL_CHANGE;
chip->cmd_ctrl(mtd, column >> 8, ctrl);
}
- if (page_addr != -1) {
+ if (page_addr != -1 && command != NAND_CMD_RNDOUT) {
chip->cmd_ctrl(mtd, page_addr, ctrl);
chip->cmd_ctrl(mtd, page_addr >> 8,
NAND_NCE | NAND_ALE);
diff --git a/drivers/mtd/nand/nand_spl_simple.c b/drivers/mtd/nand/nand_spl_simple.c
index cead4b5..f45aa21 100644
--- a/drivers/mtd/nand/nand_spl_simple.c
+++ b/drivers/mtd/nand/nand_spl_simple.c
@@ -88,15 +88,19 @@ static int nand_command(int block, int page, uint32_t offs,
hwctrl(&mtd, offs & 0xff,
NAND_CTRL_ALE | NAND_CTRL_CHANGE); /* A[7:0] */
hwctrl(&mtd, (offs >> 8) & 0xff, NAND_CTRL_ALE); /* A[11:9] */
+
/* Row address */
- hwctrl(&mtd, (page_addr & 0xff), NAND_CTRL_ALE); /* A[19:12] */
- hwctrl(&mtd, ((page_addr >> 8) & 0xff),
+ if (cmd != NAND_CMD_RNDOUT) {
+ hwctrl(&mtd, (page_addr & 0xff), NAND_CTRL_ALE); /* A[19:12] */
+ hwctrl(&mtd, ((page_addr >> 8) & 0xff),
NAND_CTRL_ALE); /* A[27:20] */
#ifdef CONFIG_SYS_NAND_5_ADDR_CYCLE
- /* One more address cycle for devices > 128MiB */
- hwctrl(&mtd, (page_addr >> 16) & 0x0f,
+ /* One more address cycle for devices > 128MiB */
+ hwctrl(&mtd, (page_addr >> 16) & 0x0f,
NAND_CTRL_ALE); /* A[31:28] */
#endif
+ }
+
/* Latch in address */
hwctrl(&mtd, NAND_CMD_READSTART,
NAND_CTRL_CLE | NAND_CTRL_CHANGE);
--
1.9.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [U-Boot] [PATCH v2] mtd: nand: Fix address cycle problem with NAND_CMD_RNDOUT
2014-05-06 9:15 [U-Boot] [PATCH v2] mtd: nand: Fix address cycle problem with NAND_CMD_RNDOUT Stefan Roese
@ 2014-05-06 9:25 ` Gupta, Pekon
2014-05-28 9:37 ` Gupta, Pekon
2014-06-20 0:38 ` [U-Boot] [U-Boot, " Scott Wood
2 siblings, 0 replies; 4+ messages in thread
From: Gupta, Pekon @ 2014-05-06 9:25 UTC (permalink / raw
To: u-boot
+ Bacem Daasi <Bacem.Daassi@spansion.com>
who was first to identify and root-cause the issue. So giving some credit to him..
>From: Stefan Roese [mailto:sr at denx.de]
>
>From: Marek Belisko <marek.belisko@gmail.com>
>
>On some NAND devices (e.g. Hynix H27U2G8F2CTR-BI on Siemens DXR2 /
>Draco boards) the NAND subsystem (SPL & U-Boot drivers) issues the following
>bit-flip error messages:
>
>nand: bit-flip corrected @oob=0
>...
>
>NAND_CMD_RNDOUT (05h-E0h) needs only two cycles of column address to access data
>from different column within the same page. So expected sequence on NAND data-bus is
> <05h> <column-addr-byte1> <column-address-byte2> <E0h>
>
>References in datasheets of parts from different NAND vendors
>+--------+------------------------+---------------------------------------------
>|Vendor | Datasheet/Part# | Reference
>+--------+------------------------+---------------------------------------------
>|Spansion| S34ML{01|02|04}G2 | Figure 6.12 Random Data Output In a Page
>|Micron | MT29F{16|32|64|128}G08A| Figure 47: CHANGE READ COLUMN (05h-E0h) Operation
>|Macronix| MX30LF1G08AA | Figure 10. AC Waveforms for Random Data Output
>|Toshiba | TC58NVG1S3ETAI0 | Figure Column Address Change in Read Cycle Timing
>| | | Diagram (2/2)
>+--------+------------------------+---------------------------------------------
>
>Though most NAND Devices mentioned above tend to work even if extra cycles of
>page-address is issued between <05h> .... <E0h> command. But Spansion devices
>break on this non-compliance.
>This patch fixes chip->cmdfunc() for all devices, as datasheet of all mentioned
>vendor expect same sequence.
>
>The same issue has been reported by Bacem Daassi:
>
>http://e2e.ti.com/support/arm/sitara_arm/f/791/t/259699.aspx
>
>"
>The issue is mainly due to a NAND protocol violation in the omap driver since
>the Random Data Output command (05h-E0h) expects to see only the column address
>that should be addressed within the already loaded read page into the read
>buffer. Only 2 address cycles with ALE active should be provided between the
>05h and E0h commands. The Page read command expects the full address footprint
>(2bytes for column address + 3bytes for row address), but once the page is
>loaded into the read buffer, Random Data Output should be used with only 2bytes
>for column address.
>"
>
>Signed-off-by: Marek Belisko <marek.belisko@gmail.com>
>Signed-off-by: Stefan Roese <sr@denx.de>
>Tested-by: Samuel Egli <samuel.egli@siemens.com>
>Cc: Pekon Gupta <pekon@ti.com>
>Cc: Scott Wood <scottwood@freescale.com>
>---
>v2:
>- Marek as author
>- Added additional comment / datasheet reference to the commit
> text from Pekon (thanks)
>- Added fix to nand_spl_simple.c
>- Squashed all three drivers fixes into one patch
>
> drivers/mtd/nand/am335x_spl_bch.c | 12 ++++++++----
> drivers/mtd/nand/nand_base.c | 2 +-
> drivers/mtd/nand/nand_spl_simple.c | 12 ++++++++----
> 3 files changed, 17 insertions(+), 9 deletions(-)
>
>diff --git a/drivers/mtd/nand/am335x_spl_bch.c b/drivers/mtd/nand/am335x_spl_bch.c
>index c84851b..9b547ce 100644
>--- a/drivers/mtd/nand/am335x_spl_bch.c
>+++ b/drivers/mtd/nand/am335x_spl_bch.c
>@@ -63,15 +63,19 @@ static int nand_command(int block, int page, uint32_t offs,
> hwctrl(&mtd, offs & 0xff,
> NAND_CTRL_ALE | NAND_CTRL_CHANGE); /* A[7:0] */
> hwctrl(&mtd, (offs >> 8) & 0xff, NAND_CTRL_ALE); /* A[11:9] */
>+
> /* Row address */
>- hwctrl(&mtd, (page_addr & 0xff), NAND_CTRL_ALE); /* A[19:12] */
>- hwctrl(&mtd, ((page_addr >> 8) & 0xff),
>+ if (cmd != NAND_CMD_RNDOUT) {
>+ hwctrl(&mtd, (page_addr & 0xff), NAND_CTRL_ALE); /* A[19:12] */
>+ hwctrl(&mtd, ((page_addr >> 8) & 0xff),
> NAND_CTRL_ALE); /* A[27:20] */
> #ifdef CONFIG_SYS_NAND_5_ADDR_CYCLE
>- /* One more address cycle for devices > 128MiB */
>- hwctrl(&mtd, (page_addr >> 16) & 0x0f,
>+ /* One more address cycle for devices > 128MiB */
>+ hwctrl(&mtd, (page_addr >> 16) & 0x0f,
> NAND_CTRL_ALE); /* A[31:28] */
> #endif
>+ }
>+
> hwctrl(&mtd, NAND_CMD_NONE, NAND_NCE | NAND_CTRL_CHANGE);
>
> if (cmd == NAND_CMD_READ0) {
>diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
>index 1ce55fd..f11fce4 100644
>--- a/drivers/mtd/nand/nand_base.c
>+++ b/drivers/mtd/nand/nand_base.c
>@@ -674,7 +674,7 @@ static void nand_command_lp(struct mtd_info *mtd, unsigned int command,
> ctrl &= ~NAND_CTRL_CHANGE;
> chip->cmd_ctrl(mtd, column >> 8, ctrl);
> }
>- if (page_addr != -1) {
>+ if (page_addr != -1 && command != NAND_CMD_RNDOUT) {
> chip->cmd_ctrl(mtd, page_addr, ctrl);
> chip->cmd_ctrl(mtd, page_addr >> 8,
> NAND_NCE | NAND_ALE);
>diff --git a/drivers/mtd/nand/nand_spl_simple.c b/drivers/mtd/nand/nand_spl_simple.c
>index cead4b5..f45aa21 100644
>--- a/drivers/mtd/nand/nand_spl_simple.c
>+++ b/drivers/mtd/nand/nand_spl_simple.c
>@@ -88,15 +88,19 @@ static int nand_command(int block, int page, uint32_t offs,
> hwctrl(&mtd, offs & 0xff,
> NAND_CTRL_ALE | NAND_CTRL_CHANGE); /* A[7:0] */
> hwctrl(&mtd, (offs >> 8) & 0xff, NAND_CTRL_ALE); /* A[11:9] */
>+
> /* Row address */
>- hwctrl(&mtd, (page_addr & 0xff), NAND_CTRL_ALE); /* A[19:12] */
>- hwctrl(&mtd, ((page_addr >> 8) & 0xff),
>+ if (cmd != NAND_CMD_RNDOUT) {
>+ hwctrl(&mtd, (page_addr & 0xff), NAND_CTRL_ALE); /* A[19:12] */
>+ hwctrl(&mtd, ((page_addr >> 8) & 0xff),
> NAND_CTRL_ALE); /* A[27:20] */
> #ifdef CONFIG_SYS_NAND_5_ADDR_CYCLE
>- /* One more address cycle for devices > 128MiB */
>- hwctrl(&mtd, (page_addr >> 16) & 0x0f,
>+ /* One more address cycle for devices > 128MiB */
>+ hwctrl(&mtd, (page_addr >> 16) & 0x0f,
> NAND_CTRL_ALE); /* A[31:28] */
> #endif
>+ }
>+
> /* Latch in address */
> hwctrl(&mtd, NAND_CMD_READSTART,
> NAND_CTRL_CLE | NAND_CTRL_CHANGE);
>--
>1.9.2
with regards, pekon
^ permalink raw reply [flat|nested] 4+ messages in thread
* [U-Boot] [PATCH v2] mtd: nand: Fix address cycle problem with NAND_CMD_RNDOUT
2014-05-06 9:15 [U-Boot] [PATCH v2] mtd: nand: Fix address cycle problem with NAND_CMD_RNDOUT Stefan Roese
2014-05-06 9:25 ` Gupta, Pekon
@ 2014-05-28 9:37 ` Gupta, Pekon
2014-06-20 0:38 ` [U-Boot] [U-Boot, " Scott Wood
2 siblings, 0 replies; 4+ messages in thread
From: Gupta, Pekon @ 2014-05-28 9:37 UTC (permalink / raw
To: u-boot
Hi Stefan, Marek,
>>From: Stefan Roese [mailto:sr at denx.de]
>>
>>From: Marek Belisko <marek.belisko@gmail.com>
>>
>>On some NAND devices (e.g. Hynix H27U2G8F2CTR-BI on Siemens DXR2 /
>>Draco boards) the NAND subsystem (SPL & U-Boot drivers) issues the following
>>bit-flip error messages:
>>
>>nand: bit-flip corrected @oob=0
>>...
>>
>>NAND_CMD_RNDOUT (05h-E0h) needs only two cycles of column address to access data
>>from different column within the same page. So expected sequence on NAND data-bus is
>> <05h> <column-addr-byte1> <column-address-byte2> <E0h>
>>
[...]
>>
>>diff --git a/drivers/mtd/nand/am335x_spl_bch.c b/drivers/mtd/nand/am335x_spl_bch.c
>>index c84851b..9b547ce 100644
>>--- a/drivers/mtd/nand/am335x_spl_bch.c
>>+++ b/drivers/mtd/nand/am335x_spl_bch.c
>>@@ -63,15 +63,19 @@ static int nand_command(int block, int page, uint32_t offs,
>> hwctrl(&mtd, offs & 0xff,
>> NAND_CTRL_ALE | NAND_CTRL_CHANGE); /* A[7:0] */
>> hwctrl(&mtd, (offs >> 8) & 0xff, NAND_CTRL_ALE); /* A[11:9] */
>>+
>> /* Row address */
>>- hwctrl(&mtd, (page_addr & 0xff), NAND_CTRL_ALE); /* A[19:12] */
>>- hwctrl(&mtd, ((page_addr >> 8) & 0xff),
I see the conflict here with following commit. Is this patch on latest u-boot tree?
commit 6dd3b566893a99629771e076dca1ce8db7b77dc1
mtd: Add a CONFIG_SPL_MTD_SUPPORT for a more full NAND subsystem in SPL
May be you need to resend rebased on latest tree..
with regards, pekon
^ permalink raw reply [flat|nested] 4+ messages in thread
* [U-Boot] [U-Boot, v2] mtd: nand: Fix address cycle problem with NAND_CMD_RNDOUT
2014-05-06 9:15 [U-Boot] [PATCH v2] mtd: nand: Fix address cycle problem with NAND_CMD_RNDOUT Stefan Roese
2014-05-06 9:25 ` Gupta, Pekon
2014-05-28 9:37 ` Gupta, Pekon
@ 2014-06-20 0:38 ` Scott Wood
2 siblings, 0 replies; 4+ messages in thread
From: Scott Wood @ 2014-06-20 0:38 UTC (permalink / raw
To: u-boot
On Tue, May 06, 2014 at 11:15:35AM +0200, Stefan Roese wrote:
> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> index 1ce55fd..f11fce4 100644
> --- a/drivers/mtd/nand/nand_base.c
> +++ b/drivers/mtd/nand/nand_base.c
> @@ -674,7 +674,7 @@ static void nand_command_lp(struct mtd_info *mtd, unsigned int command,
> ctrl &= ~NAND_CTRL_CHANGE;
> chip->cmd_ctrl(mtd, column >> 8, ctrl);
> }
> - if (page_addr != -1) {
> + if (page_addr != -1 && command != NAND_CMD_RNDOUT) {
> chip->cmd_ctrl(mtd, page_addr, ctrl);
> chip->cmd_ctrl(mtd, page_addr >> 8,
> NAND_NCE | NAND_ALE);
Where is this being called with NAND_CMD_RNDOUT and page_addr != -1, and
shouldn't the fix be to not do that?
Is there a corresponding Linux change?
-Scott
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-06-20 0:38 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-05-06 9:15 [U-Boot] [PATCH v2] mtd: nand: Fix address cycle problem with NAND_CMD_RNDOUT Stefan Roese
2014-05-06 9:25 ` Gupta, Pekon
2014-05-28 9:37 ` Gupta, Pekon
2014-06-20 0:38 ` [U-Boot] [U-Boot, " Scott Wood
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.