Mini2440 boot failure on kernel 3.7-rc1
Subsystems (3)
A message that is not in the archive (sent off-list or not yet received): [email protected]
A message that is not in the archive (sent off-list or not yet received): [email protected]
Hi Juergen, Brian,
Quoted 12 lines · Juergen Beisert
Hi Sylwester,
Quoted 8 lines · Sylwester Nawrocki
[...]
[ 2.455000] NAND device: Manufacturer ID: 0xec, Chip ID: 0xf1 (Samsung NAND 128MiB 3,3V 8-bit), 8 [ 2.460000] 4 cmdlinepart partitions found on MTD device nand [ 2.465000] Creating 4 MTD partitions on "nand": [ 2.470000] 0x000000000000-0x000000080000 : "barebox" [ 2.475000] mtd: partition "barebox" doesn't end on an erase block -- force read-only^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ This "force read-only" doesn't happen with v3.6.
Quoted 6 lines
This looks to me the NAND database uses a wrong block size for this NAND device. This device has 128 kiB block sizes. So, a 512 kiB partition *is* block aligned.
Maybe you should take a look into the generic code, what block size the 3.7-rc1 uses for this NAND.
Thanks a lot for these pointers. I've found that the commit causing this regression is this one:
commit e2d3a35ee427aaba99b6c68a56609ce276c51270
Author: Brian Norris <[email protected]> Date: Mon Sep 24 20:40:55 2012 -0700
mtd: nand: detect Samsung K9GBG08U0A, K9GAG08U0F ID
Datasheets for the following Samsung NAND parts (both MLC and SLC)
describe
extensions to the Samsung 6-byte extended ID decoding table:
K9GBG08U0A (MLC, 6-byte ID)
K9GAG08U0F (MLC, 6-byte ID)
K9FAG08U0M (SLC, 6-byte ID)
The table found in K9GAG08U0F, p.44, contains a superset of the information
found in other previous datasheets.
This patch adds support for all of these chips, with 512B and 640B
OOB sizes.
It also changes the detection pattern such that this table applies
to all
Samsung 6-byte ID NAND, not just MLC. This is safe, according to
the NAND
parameter data I have collected:
Note that nand_base.c does not yet support the bad block marker
scheme defined
for these chips (i.e., scan 1st and last page for BB markers).
When I revert it the crash is gone and everything works well. The Flash chip is K9F1G08U0C and according to the datasheet it has 5 byte ID. However nand_id_len() returns 6 and id_data[] is: ec, f1, 00, 95, 40, ec.
I've found a mail thread where similar issue has been discussed [1]. It seems the mentioned chunk in commit
"mtd: nand: add generic READ ID length calculation functions"
is missing. Still it doesn't fix the problem for me.
drivers/mtd/nand/nand_base.c +0 -0
@@ -2989,7 +2989,8 @@ static void nand_decode_ext_id(struct mtd_info *mtd, struct nand_chip *chip,* Check for ID length, cell type, and Hynix/Samsung ID to
decide what
* to do.
*/
- if (id_len == 6 && id_data[0] == NAND_MFR_SAMSUNG) {
+ if (id_len == 6 && id_data[0] == NAND_MFR_SAMSUNG &&
+ id_data[5] != 0x00) {
/* Calc pagesize */
mtd->writesize = 2048 << (extid & 0x03);
extid >>= 2;With this change erasesize looks still incorrectly calculated (4 MB):
nand_decode_ext_id:3089 mtd writesize: 4096, oobsize: 128, erasesize: 4194304
writesize also seems wrong, according to the datasheet it is 2 KB.
It looks like this Flash really needs to have id_len = 5 assigned, when I forced it:
nand_decode_ext_id:3090 mtd writesize: 2048, oobsize: 64, erasesize: 131072 everything looks as specified in the datasheet.
The following change eliminates the problem for this particular chip, however it will likely break others.
From efab2f7d0a9049588c8b155fab21f8f8c2433b19 Mon Sep 17 00:00:00 2001
From: Sylwester Nawrocki <[email protected]>
Date: Tue, 13 Nov 2012 00:10:06 +0100
Subject: [PATCH] mtd: Change calculation of length of nand id with repeated pattern
Corrects ID length calculation for Samsung K9F1G08U0C NAND Flash,
ID: [ec, f1, 00, 95, 40], ec.
--- drivers/mtd/nand/nand_base.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/mtd/nand/nand_base.c +1 -1
@@ -2954,7 +2954,7 @@ static int nand_id_len(u8 *id_data, int arrlen)/* There's a repeated pattern */ if (period < arrlen) -return period; +return period -1 ;/* There are trailing zeros */ if (last_nonzero < arrlen -1 )
Signature · 8 lines
1.7.4.1 [1] http://lists.infradead.org/pipermail/linux-mtd/2012-October/044358.html -- Thanks, Sylwester
Hi Sylwester,
Quoted 21 lines · Sylwester Nawrocki
Quoted 13 lines · Juergen Beisert
Quoted 11 lines · Sylwester Nawrocki
[...]
[ 2.455000] NAND device: Manufacturer ID: 0xec, Chip ID: 0xf1 (Samsung NAND 128MiB 3,3V 8-bit), 8 [ 2.460000] 4 cmdlinepart partitions found on MTD device nand [ 2.465000] Creating 4 MTD partitions on "nand": [ 2.470000] 0x000000000000-0x000000080000 : "barebox" [ 2.475000] mtd: partition "barebox" doesn't end on an erase block --force read-only ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ This "force read-only" doesn't happen with v3.6.
Quoted 6 lines
This looks to me the NAND database uses a wrong block size for this NAND device. This device has 128 kiB block sizes. So, a 512 kiB partition *is* block aligned.
Maybe you should take a look into the generic code, what block size the 3.7-rc1 uses for this NAND.
Quoted 8 lines
Thanks a lot for these pointers. I've found that the commit causing this regression is this one:
commit
e2d3a35ee427aaba99b6c68a56609ce276c51270Author: Brian Norris <[email protected]> Date: Mon Sep 24 20:40:55 2012 -0700mtd: nand: detect Samsung K9GBG08U0A, K9GAG08U0F ID
Yes, this patch is already known to cause at least one type of regression. There's been a fix for a while but David still has neglected to send it to Linus. It seems that you have quoted it below, though, and it may not fix your particular regression.
Quoted 3 lines
When I revert it the crash is gone and everything works well. The Flash chip is K9F1G08U0C and according to the datasheet it has 5 byte ID. However
nand_id_len()returns 6 and id_data[] is: ec, f1, 00, 95, 40, ec.
Samsung seems to have created some problems by under-specifying the ID on their NAND. Your NAND is not detected as 5-byte ID for good reason:
the ID wraps around after 6 bytes, not 5. But that's partly speculation, as you have not provided the full ID.
Can you please list a full 8-byte string of id_data[]? This will be very helpful. I know of at least one solution that will fix your problem, but I need to know your full ID to help lay this issue to rest for good.
Quoted 24 lines
I've found a mail thread where similar issue has been discussed [1]. It seems
the mentioned chunk in commit
"mtd: nand: add generic READ ID length calculation functions"is missing. Still it doesn't fix the problem for me.
drivers/mtd/nand/nand_base.c +0
-0@@ -2989,7 +2989,8 @@ static void nand_decode_ext_id(struct mtd_info *mtd,struct nand_chip *chip, * Check for ID length, cell type, and Hynix/Samsung ID to decidewhat
* to do.
*/drivers/mtd/nand/nand_base.c +2
-1-if (id_len ==6 && id_data[0 ] ==NAND_MFR_SAMSUNG ) { +if (id_len ==6 && id_data[0 ] ==NAND_MFR_SAMSUNG && + id_data[5 ] !=0x00 ) {/* Calc pagesize */ mtd->writesize =2048 << (extid &0x03 ); extid >>=2 ;
...
Quoted 22 lines
The following change eliminates the problem for this particular chip, however it will likely break others.
From
efab2f7d0a9049588c8b155fab21f8f8c2433b19Mon Sep 17 00:00:00 2001
From: Sylwester Nawrocki <[email protected]>
Date: Tue, 13 Nov 2012 00:10:06 +0100
Subject: [PATCH] mtd: Change calculation of length of nand id with repeated patternCorrects ID length calculation for Samsung K9F1G08U0C NAND Flash,
ID: [ec, f1, 00, 95, 40], ec.Signed-off-by Sylwester Nawrocki <[email protected]>--- drivers/mtd/nand/nand_base.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)drivers/mtd/nand/nand_base.c +0
-0
drivers/mtd/nand/nand_base.c static int nand_id_len(u8 *id_data, int arrlen) · line 2960
Quoted 8 lines
drivers/mtd/nand/nand_base.c +1
-1@@ -2954,7 +2954,7 @@ static int nand_id_len(u8 *id_data, int arrlen)/* There's a repeated pattern */ if (period < arrlen) -return period; +return period -1 ;/* There are trailing zeros */ if (last_nonzero < arrlen -1 )
You're right, this patch is not correct. The period of repetition *should* be equal to the ID length, but it seems in your case that your NAND repeats at 'period + 1'. So we have to figure out something else.
Thanks,
Brian
Hi Brian,
Quoted 30 lines · Brian Norris
Quoted 28 lines · Sylwester Nawrocki
Quoted 13 lines · Juergen Beisert
Quoted 11 lines · Sylwester Nawrocki
[...]
[ 2.455000] NAND device: Manufacturer ID: 0xec, Chip ID: 0xf1 (Samsung NAND 128MiB 3,3V 8-bit), 8 [ 2.460000] 4 cmdlinepart partitions found on MTD device nand [ 2.465000] Creating 4 MTD partitions on "nand": [ 2.470000] 0x000000000000-0x000000080000 : "barebox" [ 2.475000] mtd: partition "barebox" doesn't end on an erase block --force read-only ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ This "force read-only" doesn't happen with v3.6.
Quoted 6 lines
This looks to me the NAND database uses a wrong block size for this NAND device. This device has 128 kiB block sizes. So, a 512 kiB partition *is* block aligned.
Maybe you should take a look into the generic code, what block size the 3.7-rc1 uses for this NAND.
Thanks a lot for these pointers. I've found that the commit causing this regression is this one:
commit
e2d3a35ee427aaba99b6c68a56609ce276c51270Author: Brian Norris<[email protected]> Date: Mon Sep 24 20:40:55 2012 -0700
Quoted 6 lines
Quoted 1 line
mtd: nand: detect Samsung K9GBG08U0A, K9GAG08U0F ID
Yes, this patch is already known to cause at least one type of regression. There's been a fix for a while but David still has neglected to send it to Linus. It seems that you have quoted it below, though, and it may not fix your particular regression.
No, it wasn't helpful for me.
Quoted 9 lines
Quoted 3 lines
When I revert it the crash is gone and everything works well. The Flash chip is K9F1G08U0C and according to the datasheet it has 5 byte ID. However
nand_id_len()returns 6 and id_data[] is: ec, f1, 00, 95, 40, ec.Samsung seems to have created some problems by under-specifying the ID on their NAND. Your NAND is not detected as 5-byte ID for good reason:
the ID wraps around after 6 bytes, not 5. But that's partly speculation, as you have not provided the full ID.
Quoted 4 lines
Can you please list a full 8-byte string of id_data[]? This will be very helpful. I know of at least one solution that will fix your problem, but I need to know your full ID to help lay this issue to rest for good.
Indeed, I wish there was good hardware documentation, still it seems an exception rather than a rule for some hardware vendors...
Here is my id_data[] dump:
ec, f1, 00, 95, 40, ec, ec, f1
Looks a bit strange, I hope you know how to handle this. :)
Quoted 53 lines
Quoted 24 lines
I've found a mail thread where similar issue has been discussed [1]. It seems
the mentioned chunk in commit
"mtd: nand: add generic READ ID length calculation functions"is missing. Still it doesn't fix the problem for me.
drivers/mtd/nand/nand_base.c +0
-0@@ -2989,7 +2989,8 @@ static void nand_decode_ext_id(struct mtd_info *mtd,struct nand_chip *chip, * Check for ID length, cell type, and Hynix/Samsung ID to decidewhat
* to do.
*/drivers/mtd/nand/nand_base.c +2
-1-if (id_len ==6 && id_data[0 ] ==NAND_MFR_SAMSUNG ) { +if (id_len ==6 && id_data[0 ] ==NAND_MFR_SAMSUNG && + id_data[5 ] !=0x00 ) {/* Calc pagesize */ mtd->writesize =2048 << (extid&0x03 ); extid>>=2 ;...
Quoted 28 lines
The following change eliminates the problem for this particular chip, however it will likely break others.
From
efab2f7d0a9049588c8b155fab21f8f8c2433b19Mon Sep 17 00:00:00 2001
From: Sylwester Nawrocki<[email protected]>
Date: Tue, 13 Nov 2012 00:10:06 +0100
Subject: [PATCH] mtd: Change calculation of length of nand id with repeated patternCorrects ID length calculation for Samsung K9F1G08U0C NAND Flash,
ID: [ec, f1, 00, 95, 40], ec.Signed-off-by Sylwester Nawrocki<[email protected]>--- drivers/mtd/nand/nand_base.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)drivers/mtd/nand/nand_base.c +1
-1@@ -2954,7 +2954,7 @@ static int nand_id_len(u8 *id_data, int arrlen)/* There's a repeated pattern */ if (period< arrlen) -return period; +return period -1 ;
Quoted 7 lines
drivers/mtd/nand/nand_base.c static int nand_id_len(u8 *id_data, int arrlen) · line 2960
Quoted 2 lines
drivers/mtd/nand/nand_base.c +0
-0/* There are trailing zeros */ if (last_nonzero< arrlen -1 )You're right, this patch is not correct. The period of repetition *should* be equal to the ID length, but it seems in your case that your NAND repeats at 'period + 1'. So we have to figure out something else.
OK. BTW, is this comment in drivers/mtd/nand_base.c
/*
* nand_id_has_period - Check if an ID string has a given wraparound period
...
* specific repetition interval period (e.g., {0x20,0x01,0x7F,0x20} has a
* period of 2). This is a helper function for nand_id_len(). Returns
non-zero
...
correct ? Shouldn't it be "has a period of 3" ? Since there are 3 different values and then it repeats starting with 0x20 ?
--
Thanks,
Sylwester
Quoted 7 lines · Sylwester Nawrocki
Quoted 5 lines · Brian Norris
On Mon, Nov 12, 2012 at 3:18 PM, Sylwester Nawrocki
Can you please list a full 8-byte string of id_data[]? This will be very helpful. I know of at least one solution that will fix your problem, but I need to know your full ID to help lay this issue to rest for good.
Quoted 8 lines
Indeed, I wish there was good hardware documentation, still it seems an exception rather than a rule for some hardware vendors...
Here is my id_data[] dump:
ec, f1, 00, 95, 40, ec, ec, f1
Looks a bit strange, I hope you know how to handle this. :)
Well, sort of. I could add yet another quirk: that some "5-byte" IDs have the manufacturer ID repeated twice (0xEC 0xEC). In fact, I just got another report of a similar (stupid) ID.
But I have a more reasonable change. It's disappointing, but I'll have to do a partial revert of my previous commits, dropping support for some of the newest SLC in favor of not breaking the old. I'll have to try to get in contact with Samsung if I'm ever going to support the newest stuff.
Please try my diff (below), on top of the patch from:
http://lists.infradead.org/pipermail/linux-mtd/2012-October/044356.html
Quoted 11 lines
OK. BTW, is this comment in
drivers/mtd/nand_base.c/*
*nand_id_has_period- Check if an ID string has a given wraparound
period
...
* specific repetition interval period (e.g., {0x20,0x01,0x7F,0x20} has a
* period of 2). This is a helper function fornand_id_len(). Returns
non-zero
...
Quoted 2 lines
correct ? Shouldn't it be "has a period of 3" ? Since there are 3 different values and then it repeats starting with
0x20?
Yes, good eye. I also noticed my typo when re-reviewing this code just now. I'll fix this for the 3.8 cycle.
Brian
--- drivers/mtd/nand/nand_base.c | 1 + 1 file changed, 1 insertion(+)
drivers/mtd/nand/nand_base.c +1 -0
@@ -2990,6 +2990,7 @@ static void nand_decode_ext_id(struct mtd_info *mtd, struct nand_chip *chip,* ID to decide what to do. */ if (id_len ==6 && id_data[0 ] ==NAND_MFR_SAMSUNG && + (chip->cellinfo &NAND_CI_CELLTYPE_MSK ) && id_data[5 ] !=0x00 ) {/* Calc pagesize */ mtd->writesize =2048 << (extid &0x03 );