mtd: nand: samsung: Disable subpage writes on E-die NAND

PATCH Superseded Ladislav Michl 2 participants 8 messages Jan 8, 2018 – Jan 9, 2018

Subsystems (2)

Memory technology devices (MTD) NAND flash

lore

Share & tools

lore

Ladislav Michl

author

2018-01-08 23:48 UTC #0 lore raw

PATCH 1/1 mtd: nand: samsung: Disable subpage writes on E-die NAND

Samsung E-die SLC NAND manufactured using 21nm process supports only 1 partial program cycle, so disable subpage writes for it. Manufacturing process is stored in lowest two bits of 5th ID byte.

Signed-off-by Ladislav Michl <[email protected]>

---
Note: Patch generated and tested against next-20180108 on at91sam9g20

       board with K9F1G08U0E.

 drivers/mtd/nand/nand_samsung.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

drivers/mtd/nand/nand_samsung.c +11 -4

index f6b0a63a068c..9400b4a84243 100644
@@ -92,10 +92,17 @@ static void samsung_nand_decode_id(struct nand_chip *chip)
 	} else {
 		nand_decode_ext_id(chip);
 
-		/* Datasheet values for SLC Samsung K9F4G08U0D-S[I|C]B0(T00) */
-		if (nand_is_slc(chip) && chip->id.data[1] == 0xDC) {
-			chip->ecc_step_ds = 512;
-			chip->ecc_strength_ds = 1;
+		if (nand_is_slc(chip)) {
+			/* K9F4G08U0D-S[I|C]B0(T00) */
+			if (chip->id.data[1] == 0xDC) {
+				chip->ecc_step_ds = 512;
+				chip->ecc_strength_ds = 1;
+			}
+
+			/* 21nm chips do not support partial page write */
+			if (chip->id.len > 4 &&
+			    (chip->id.data[4] & GENMASK(1,0)) == 0x1)
+				chip->options |= NAND_NO_SUBPAGE_WRITE;
 		}
 	}
 }

Signature · 1 line

2.15.1

Boris Brezillon

2018-01-09 08:46 UTC #1 lore raw

Quoted 1 line · On Tue, 9 Jan 2018 00:48:37 +0100 Ladislav Michl <[email protected]> wrote:

Samsung E-die SLC NAND manufactured using 21nm process supports only

I would add the chip name here (K9F1G08U0E).

Quoted 1 line

1 partial program cycle, so disable subpage writes for it.

Which means it does not support partial page programming, so how about rewording it like that:

Samsung E-die SLC NAND manufactured using 21nm process (K9F1G08U0E) does not support partial page programming, so disable subpage writes for it.

Quoted 6 lines

Manufacturing process is stored in lowest two bits of 5th ID byte.
Signed-off-by Ladislav Michl <[email protected]>
---
Note: Patch generated and tested against next-20180108 on at91sam9g20
       board with K9F1G08U0E.

Just out of curiosity, what are the symptoms when you don't have this flag set?

Quoted 16 lines

 drivers/mtd/nand/nand_samsung.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

drivers/mtd/nand/nand_samsung.c +0 -4

index f6b0a63a068c..9400b4a84243 100644
@@ -92,10 +92,17 @@ static void samsung_nand_decode_id(struct nand_chip *chip)
 	} else {
 		nand_decode_ext_id(chip);
 
-		/* Datasheet values for SLC Samsung K9F4G08U0D-S[I|C]B0(T00) */
-		if (nand_is_slc(chip) && chip->id.data[1] == 0xDC) {
-			chip->ecc_step_ds = 512;
-			chip->ecc_strength_ds = 1;

drivers/mtd/nand/nand_samsung.c static void samsung_nand_decode_id(struct nand_chip *chip) · line 104

Quoted 10 lines

drivers/mtd/nand/nand_samsung.c +10 -0

+		if (nand_is_slc(chip)) {
+			/* K9F4G08U0D-S[I|C]B0(T00) */
+			if (chip->id.data[1] == 0xDC) {
+				chip->ecc_step_ds = 512;
+				chip->ecc_strength_ds = 1;
+			}
+
+			/* 21nm chips do not support partial page write */
+			if (chip->id.len > 4 &&
+			    (chip->id.data[4] & GENMASK(1,0)) == 0x1)

NAND vendors tend to change their ID decoding scheme a lot, so maybe we should be more restrictive here: replace "chip->id.len > 4" by "chip->id.len == 5" and restrict it to chip->id.data[1] == 0xF1.

Quoted 4 lines

drivers/mtd/nand/nand_samsung.c +1 -0

+				chip->options |= NAND_NO_SUBPAGE_WRITE;
 		}
 	}
 }

Ladislav Michl

author

2018-01-09 09:09 UTC #2 lore raw

Quoted 6 lines · Boris Brezillon

Quoted 1 line · On Tue, 9 Jan 2018 00:48:37 +0100 Ladislav Michl <[email protected]> wrote:
Samsung E-die SLC NAND manufactured using 21nm process supports only
I would add the chip name here (K9F1G08U0E).

Ok.

Quoted 8 lines

Quoted 1 line
1 partial program cycle, so disable subpage writes for it.
Which means it does not support partial page programming, so how about rewording it like that:
Samsung E-die SLC NAND manufactured using 21nm process (K9F1G08U0E) does not support partial page programming, so disable subpage writes for it.

Yes, will post v2 eventually, but see bellow.

Quoted 9 lines

Quoted 6 lines
Manufacturing process is stored in lowest two bits of 5th ID byte.
Signed-off-by Ladislav Michl <[email protected]>
---
Note: Patch generated and tested against next-20180108 on at91sam9g20
       board with K9F1G08U0E.
Just out of curiosity, what are the symptoms when you don't have this flag set?

Device is identified as:
nand: device found, Manufacturer ID: 0xec, Chip ID: 0xf1
nand: Samsung NAND 128MiB 3,3V 8-bit
nand: 128 MiB, SLC, erase size: 128 KiB, page size: 2048, OOB size: 64

Short test:

# flash_erase /dev/mtd4 0 0
Erasing 128 Kibyte @ 7f60000 -- 99 % complete flash_erase: Skipping bad block at 07f80000
flash_erase: Skipping bad block at 07fa0000
flash_erase: Skipping bad block at 07fc0000
flash_erase: Skipping bad block at 07fe0000
Erasing 128 Kibyte @ 7fe0000 -- 100 % complete 
# ubiformat /dev/mtd4
ubiformat: mtd4 (nand), size 134217728 bytes (128.0 MiB), 1024 eraseblocks of 131072 bytes (128.0 KiB), min. I/O size 2048 bytes
libscan: scanning eraseblock 1023 -- 100 % complete  
ubiformat: 1020 eraseblocks are supposedly empty
ubiformat: 4 bad eraseblocks found, numbers: 1020, 1021, 1022, 1023
ubiformat: formatting eraseblock 1023 -- 100 % complete  
# ubiattach -m 4
ubi0: default fastmap pool size: 50
ubi0: default fastmap WL pool size: 25
ubi0: attaching mtd4
ubi0: scanning is finished
ubi0: attached mtd4 (name "atmel_nand", size 128 MiB)
ubi0: PEB size: 131072 bytes (128 KiB), LEB size: 126976 bytes
ubi0: min./max. I/O unit sizes: 2048/2048, sub-page size 2048
ubi0: VID header offset: 2048 (aligned 2048), data offset: 4096
ubi0: good PEBs: 1020, bad PEBs: 4, corrupted PEBs: 0
ubi0: user volume: 0, internal volumes: 1, max. volumes count: 128
ubi0: max/mean erase counter: 0/0, WL threshold: 4096, image sequence number: 1387212117
ubi0: available PEBs: 998, total reserved PEBs: 22, PEBs reserved for bad PEB handling: 16
ubi0: background thread "ubi_bgt0d" started, PID 716
UBI device number 0, total 1020 LEBs (129515520 bytes, 123.5 MiB), available 998 LEBs (126722048 bytes, 120.9 MiB), LEB size 126976 bytes (124.0 KiB)
# ubimkvol /dev/ubi0 -s 4MiB -N kernel -t static
Volume ID 0, size 34 LEBs (4317184 bytes, 4.1 MiB), LEB size 126976 bytes (124.0 KiB), static, name "kernel", alignment 1
# ubiupdatevol /dev/ubi0_0 linuximage 
__nand_correct_data: uncorrectable ECC error
ubi0 warning: ubi_io_read: error -74 (ECC error) while reading 64 bytes from PEB 309:512, read only 64 bytes, retry
__nand_correct_data: uncorrectable ECC error
ubi0 warning: ubi_io_read: error -74 (ECC error) while reading 64 bytes from PEB 309:512, read only 64 bytes, retry
__nand_correct_data: uncorrectable ECC error
ubi0 warning: ubi_io_read: error -74 (ECC error) while reading 64 bytes from PEB 309:512, read only 64 bytes, retry
__nand_correct_data: uncorrectable ECC error
ubi0 error: ubi_io_read: error -74 (ECC error) while reading 64 bytes from PEB 309:512, read 64 bytes
CPU: 0 PID: 720 Comm: ubiupdatevol Not tainted 4.14.4 #7
Hardware name: Atmel AT91SAM9
[<c010796c>] (unwind_backtrace) from [<c01053e0>] (show_stack+0x10/0x14)
[<c01053e0>] (show_stack) from [<c0337fac>] (ubi_io_read+0x1d8/0x2ac)
[<c0337fac>] (ubi_io_read) from [<c03384c4>] (ubi_io_read_vid_hdr+0x70/0x214)
[<c03384c4>] (ubi_io_read_vid_hdr) from [<c0335b44>] (ubi_eba_read_leb+0x154/0x414)
[<c0335b44>] (ubi_eba_read_leb) from [<c033e5b8>] (ubi_check_volume+0x7c/0xb8)
[<c033e5b8>] (ubi_check_volume) from [<c0334180>] (vol_cdev_write+0x254/0x364)
[<c0334180>] (vol_cdev_write) from [<c01a617c>] (__vfs_write+0x20/0x128)
[<c01a617c>] (__vfs_write) from [<c01a6420>] (vfs_write+0xb4/0x13c)
[<c01a6420>] (vfs_write) from [<c01a65b8>] (SyS_write+0x40/0x74)
[<c01a65b8>] (SyS_write) from [<c0102480>] (ret_fast_syscall+0x0/0x44)
ubi0 warning: ubi_eba_read_leb: corrupted VID header at PEB 309, LEB 0:0
ubi0 warning: vol_cdev_write: volume 0 on UBI device 0 is corrupted

There's another patchset to deal with this issue:
http://thread.gmane.org/gmane.linux.drivers.mtd/54167

And it brings a problem to us as those patches are mutually exclusive. Once no subpage write patch is applied UBI's VID header offset changes to 2048 from 512, so on flash filesystem is no longer readable if we wish to give optimize subpage writes a chance later.

I do want to introduce any hard to overcome incompatibility, so isn't Pekon's patch worth considering again?

Quoted 21 lines

 drivers/mtd/nand/nand_samsung.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

drivers/mtd/nand/nand_samsung.c +6 -4

index f6b0a63a068c..9400b4a84243 100644
@@ -92,10 +92,17 @@ static void samsung_nand_decode_id(struct nand_chip *chip)
 	} else {
 		nand_decode_ext_id(chip);
 
-		/* Datasheet values for SLC Samsung K9F4G08U0D-S[I|C]B0(T00) */
-		if (nand_is_slc(chip) && chip->id.data[1] == 0xDC) {
-			chip->ecc_step_ds = 512;
-			chip->ecc_strength_ds = 1;
+		if (nand_is_slc(chip)) {
+			/* K9F4G08U0D-S[I|C]B0(T00) */
+			if (chip->id.data[1] == 0xDC) {
+				chip->ecc_step_ds = 512;
+				chip->ecc_strength_ds = 1;
+			}

Quoted 8 lines

drivers/mtd/nand/nand_samsung.c static void samsung_nand_decode_id(struct nand_chip *chip) · line 104
Quoted 4 lines
drivers/mtd/nand/nand_samsung.c +4 -0
+
+			/* 21nm chips do not support partial page write */
+			if (chip->id.len > 4 &&
+			    (chip->id.data[4] & GENMASK(1,0)) == 0x1)
NAND vendors tend to change their ID decoding scheme a lot, so maybe we should be more restrictive here: replace "chip->id.len > 4" by "chip->id.len == 5" and restrict it to chip->id.data[1] == 0xF1.

Well, K9F1G08U0E returns chip->id.len = 6, but adding chip->id.data[1] seems to be a good idea.

Quoted 4 lines

drivers/mtd/nand/nand_samsung.c static void samsung_nand_decode_id(struct nand_chip *chip) · line 108
Quoted 4 lines
drivers/mtd/nand/nand_samsung.c +1 -0
+				chip->options |= NAND_NO_SUBPAGE_WRITE;
 		}
 	}
 }

Best regards,
ladis

Boris Brezillon

2018-01-09 09:27 UTC #3 lore raw

Quoted 90 lines · On Tue, 9 Jan 2018 10:08:45 +0100 Ladislav Michl <[email protected]> wrote:

Quoted 6 lines · Boris Brezillon

Quoted 1 line · On Tue, 9 Jan 2018 00:48:37 +0100 Ladislav Michl <[email protected]> wrote:
Samsung E-die SLC NAND manufactured using 21nm process supports only
I would add the chip name here (K9F1G08U0E).

Ok.

Quoted 8 lines

Quoted 1 line
1 partial program cycle, so disable subpage writes for it.
Which means it does not support partial page programming, so how about rewording it like that:
Samsung E-die SLC NAND manufactured using 21nm process (K9F1G08U0E) does not support partial page programming, so disable subpage writes for it.

Yes, will post v2 eventually, but see bellow.

Quoted 9 lines

Quoted 6 lines
Manufacturing process is stored in lowest two bits of 5th ID byte.
Signed-off-by Ladislav Michl <[email protected]>
---
Note: Patch generated and tested against next-20180108 on at91sam9g20
       board with K9F1G08U0E.  
Just out of curiosity, what are the symptoms when you don't have this flag set?

Device is identified as:
nand: device found, Manufacturer ID: 0xec, Chip ID: 0xf1
nand: Samsung NAND 128MiB 3,3V 8-bit
nand: 128 MiB, SLC, erase size: 128 KiB, page size: 2048, OOB size: 64

Short test:

# flash_erase /dev/mtd4 0 0
Erasing 128 Kibyte @ 7f60000 -- 99 % complete flash_erase: Skipping bad block at 07f80000
flash_erase: Skipping bad block at 07fa0000
flash_erase: Skipping bad block at 07fc0000
flash_erase: Skipping bad block at 07fe0000
Erasing 128 Kibyte @ 7fe0000 -- 100 % complete 
# ubiformat /dev/mtd4
ubiformat: mtd4 (nand), size 134217728 bytes (128.0 MiB), 1024 eraseblocks of 131072 bytes (128.0 KiB), min. I/O size 2048 bytes
libscan: scanning eraseblock 1023 -- 100 % complete  
ubiformat: 1020 eraseblocks are supposedly empty
ubiformat: 4 bad eraseblocks found, numbers: 1020, 1021, 1022, 1023
ubiformat: formatting eraseblock 1023 -- 100 % complete  
# ubiattach -m 4
ubi0: default fastmap pool size: 50
ubi0: default fastmap WL pool size: 25
ubi0: attaching mtd4
ubi0: scanning is finished
ubi0: attached mtd4 (name "atmel_nand", size 128 MiB)
ubi0: PEB size: 131072 bytes (128 KiB), LEB size: 126976 bytes
ubi0: min./max. I/O unit sizes: 2048/2048, sub-page size 2048
ubi0: VID header offset: 2048 (aligned 2048), data offset: 4096
ubi0: good PEBs: 1020, bad PEBs: 4, corrupted PEBs: 0
ubi0: user volume: 0, internal volumes: 1, max. volumes count: 128
ubi0: max/mean erase counter: 0/0, WL threshold: 4096, image sequence number: 1387212117
ubi0: available PEBs: 998, total reserved PEBs: 22, PEBs reserved for bad PEB handling: 16
ubi0: background thread "ubi_bgt0d" started, PID 716
UBI device number 0, total 1020 LEBs (129515520 bytes, 123.5 MiB), available 998 LEBs (126722048 bytes, 120.9 MiB), LEB size 126976 bytes (124.0 KiB)
# ubimkvol /dev/ubi0 -s 4MiB -N kernel -t static
Volume ID 0, size 34 LEBs (4317184 bytes, 4.1 MiB), LEB size 126976 bytes (124.0 KiB), static, name "kernel", alignment 1
# ubiupdatevol /dev/ubi0_0 linuximage 
__nand_correct_data: uncorrectable ECC error
ubi0 warning: ubi_io_read: error -74 (ECC error) while reading 64 bytes from PEB 309:512, read only 64 bytes, retry
__nand_correct_data: uncorrectable ECC error
ubi0 warning: ubi_io_read: error -74 (ECC error) while reading 64 bytes from PEB 309:512, read only 64 bytes, retry
__nand_correct_data: uncorrectable ECC error
ubi0 warning: ubi_io_read: error -74 (ECC error) while reading 64 bytes from PEB 309:512, read only 64 bytes, retry
__nand_correct_data: uncorrectable ECC error
ubi0 error: ubi_io_read: error -74 (ECC error) while reading 64 bytes from PEB 309:512, read 64 bytes
CPU: 0 PID: 720 Comm: ubiupdatevol Not tainted 4.14.4 #7
Hardware name: Atmel AT91SAM9
[<c010796c>] (unwind_backtrace) from [<c01053e0>] (show_stack+0x10/0x14)
[<c01053e0>] (show_stack) from [<c0337fac>] (ubi_io_read+0x1d8/0x2ac)
[<c0337fac>] (ubi_io_read) from [<c03384c4>] (ubi_io_read_vid_hdr+0x70/0x214)
[<c03384c4>] (ubi_io_read_vid_hdr) from [<c0335b44>] (ubi_eba_read_leb+0x154/0x414)
[<c0335b44>] (ubi_eba_read_leb) from [<c033e5b8>] (ubi_check_volume+0x7c/0xb8)
[<c033e5b8>] (ubi_check_volume) from [<c0334180>] (vol_cdev_write+0x254/0x364)
[<c0334180>] (vol_cdev_write) from [<c01a617c>] (__vfs_write+0x20/0x128)
[<c01a617c>] (__vfs_write) from [<c01a6420>] (vfs_write+0xb4/0x13c)
[<c01a6420>] (vfs_write) from [<c01a65b8>] (SyS_write+0x40/0x74)
[<c01a65b8>] (SyS_write) from [<c0102480>] (ret_fast_syscall+0x0/0x44)
ubi0 warning: ubi_eba_read_leb: corrupted VID header at PEB 309, LEB 0:0
ubi0 warning: vol_cdev_write: volume 0 on UBI device 0 is corrupted

Quoted 2 lines

There's another patchset to deal with this issue:
http://thread.gmane.org/gmane.linux.drivers.mtd/54167

Are you sure it fixes your problem??? Did you try it?

Quoted 5 lines

And it brings a problem to us as those patches are mutually exclusive. Once no subpage write patch is applied UBI's VID header offset changes to 2048 from 512, so on flash filesystem is no longer readable if we wish to give optimize subpage writes a chance later.

If your NAND does not support subpage writes, you have to expose a min_io_size of a page not a subpage. AFAICT, the patchset you're referring to won't fix your problem.

Quoted 3 lines

I do want to introduce any hard to overcome incompatibility, so isn't Pekon's patch worth considering again?

Incompatibility with what? The datasheet clearly says that the chip does not support subpage writes.

Quoted 31 lines

Quoted 22 lines
Quoted 22 lines
 drivers/mtd/nand/nand_samsung.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)
drivers/mtd/nand/nand_samsung.c +6 -4
index f6b0a63a068c..9400b4a84243 100644
@@ -92,10 +92,17 @@ static void samsung_nand_decode_id(struct nand_chip *chip)
 	} else {
 		nand_decode_ext_id(chip);
 
-		/* Datasheet values for SLC Samsung K9F4G08U0D-S[I|C]B0(T00) */
-		if (nand_is_slc(chip) && chip->id.data[1] == 0xDC) {
-			chip->ecc_step_ds = 512;
-			chip->ecc_strength_ds = 1;
+		if (nand_is_slc(chip)) {
+			/* K9F4G08U0D-S[I|C]B0(T00) */
+			if (chip->id.data[1] == 0xDC) {
+				chip->ecc_step_ds = 512;
+				chip->ecc_strength_ds = 1;
+			}
Quoted 8 lines
drivers/mtd/nand/nand_samsung.c static void samsung_nand_decode_id(struct nand_chip *chip) · line 104
Quoted 4 lines
drivers/mtd/nand/nand_samsung.c +4 -0
+
+			/* 21nm chips do not support partial page write */
+			if (chip->id.len > 4 &&
+			    (chip->id.data[4] & GENMASK(1,0)) == 0x1)  
NAND vendors tend to change their ID decoding scheme a lot, so maybe we should be more restrictive here: replace "chip->id.len > 4" by "chip->id.len == 5" and restrict it to chip->id.data[1] == 0xF1.

Quoted 2 lines

Well, K9F1G08U0E returns chip->id.len = 6, but adding chip->id.data[1] seems to be a good idea.

That's not what the datasheet says :-/. What's the value of chip->id.data[5]?

Quoted 8 lines

Quoted 5 lines
drivers/mtd/nand/nand_samsung.c static void samsung_nand_decode_id(struct nand_chip *chip) · line 108
Quoted 5 lines
drivers/mtd/nand/nand_samsung.c +1 -0
+				chip->options |= NAND_NO_SUBPAGE_WRITE;
 		}
 	}
 }  
Best regards,
ladis

Ladislav Michl

author

2018-01-09 09:58 UTC #4 lore raw

Quoted 93 lines · Boris Brezillon

Quoted 90 lines · On Tue, 9 Jan 2018 10:08:45 +0100 Ladislav Michl <[email protected]> wrote:

Quoted 6 lines · Boris Brezillon

Quoted 1 line · On Tue, 9 Jan 2018 00:48:37 +0100 Ladislav Michl <[email protected]> wrote:
Samsung E-die SLC NAND manufactured using 21nm process supports only
I would add the chip name here (K9F1G08U0E).

Ok.

Quoted 8 lines

Quoted 1 line
1 partial program cycle, so disable subpage writes for it.
Which means it does not support partial page programming, so how about rewording it like that:
Samsung E-die SLC NAND manufactured using 21nm process (K9F1G08U0E) does not support partial page programming, so disable subpage writes for it.

Yes, will post v2 eventually, but see bellow.

Quoted 9 lines

Quoted 6 lines
Manufacturing process is stored in lowest two bits of 5th ID byte.
Signed-off-by Ladislav Michl <[email protected]>
---
Note: Patch generated and tested against next-20180108 on at91sam9g20
       board with K9F1G08U0E.  
Just out of curiosity, what are the symptoms when you don't have this flag set?

Device is identified as:
nand: device found, Manufacturer ID: 0xec, Chip ID: 0xf1
nand: Samsung NAND 128MiB 3,3V 8-bit
nand: 128 MiB, SLC, erase size: 128 KiB, page size: 2048, OOB size: 64

Short test:

# flash_erase /dev/mtd4 0 0
Erasing 128 Kibyte @ 7f60000 -- 99 % complete flash_erase: Skipping bad block at 07f80000
flash_erase: Skipping bad block at 07fa0000
flash_erase: Skipping bad block at 07fc0000
flash_erase: Skipping bad block at 07fe0000
Erasing 128 Kibyte @ 7fe0000 -- 100 % complete 
# ubiformat /dev/mtd4
ubiformat: mtd4 (nand), size 134217728 bytes (128.0 MiB), 1024 eraseblocks of 131072 bytes (128.0 KiB), min. I/O size 2048 bytes
libscan: scanning eraseblock 1023 -- 100 % complete  
ubiformat: 1020 eraseblocks are supposedly empty
ubiformat: 4 bad eraseblocks found, numbers: 1020, 1021, 1022, 1023
ubiformat: formatting eraseblock 1023 -- 100 % complete  
# ubiattach -m 4
ubi0: default fastmap pool size: 50
ubi0: default fastmap WL pool size: 25
ubi0: attaching mtd4
ubi0: scanning is finished
ubi0: attached mtd4 (name "atmel_nand", size 128 MiB)
ubi0: PEB size: 131072 bytes (128 KiB), LEB size: 126976 bytes
ubi0: min./max. I/O unit sizes: 2048/2048, sub-page size 2048
ubi0: VID header offset: 2048 (aligned 2048), data offset: 4096
ubi0: good PEBs: 1020, bad PEBs: 4, corrupted PEBs: 0
ubi0: user volume: 0, internal volumes: 1, max. volumes count: 128
ubi0: max/mean erase counter: 0/0, WL threshold: 4096, image sequence number: 1387212117
ubi0: available PEBs: 998, total reserved PEBs: 22, PEBs reserved for bad PEB handling: 16
ubi0: background thread "ubi_bgt0d" started, PID 716
UBI device number 0, total 1020 LEBs (129515520 bytes, 123.5 MiB), available 998 LEBs (126722048 bytes, 120.9 MiB), LEB size 126976 bytes (124.0 KiB)
# ubimkvol /dev/ubi0 -s 4MiB -N kernel -t static
Volume ID 0, size 34 LEBs (4317184 bytes, 4.1 MiB), LEB size 126976 bytes (124.0 KiB), static, name "kernel", alignment 1
# ubiupdatevol /dev/ubi0_0 linuximage 
__nand_correct_data: uncorrectable ECC error
ubi0 warning: ubi_io_read: error -74 (ECC error) while reading 64 bytes from PEB 309:512, read only 64 bytes, retry
__nand_correct_data: uncorrectable ECC error
ubi0 warning: ubi_io_read: error -74 (ECC error) while reading 64 bytes from PEB 309:512, read only 64 bytes, retry
__nand_correct_data: uncorrectable ECC error
ubi0 warning: ubi_io_read: error -74 (ECC error) while reading 64 bytes from PEB 309:512, read only 64 bytes, retry
__nand_correct_data: uncorrectable ECC error
ubi0 error: ubi_io_read: error -74 (ECC error) while reading 64 bytes from PEB 309:512, read 64 bytes
CPU: 0 PID: 720 Comm: ubiupdatevol Not tainted 4.14.4 #7
Hardware name: Atmel AT91SAM9
[<c010796c>] (unwind_backtrace) from [<c01053e0>] (show_stack+0x10/0x14)
[<c01053e0>] (show_stack) from [<c0337fac>] (ubi_io_read+0x1d8/0x2ac)
[<c0337fac>] (ubi_io_read) from [<c03384c4>] (ubi_io_read_vid_hdr+0x70/0x214)
[<c03384c4>] (ubi_io_read_vid_hdr) from [<c0335b44>] (ubi_eba_read_leb+0x154/0x414)
[<c0335b44>] (ubi_eba_read_leb) from [<c033e5b8>] (ubi_check_volume+0x7c/0xb8)
[<c033e5b8>] (ubi_check_volume) from [<c0334180>] (vol_cdev_write+0x254/0x364)
[<c0334180>] (vol_cdev_write) from [<c01a617c>] (__vfs_write+0x20/0x128)
[<c01a617c>] (__vfs_write) from [<c01a6420>] (vfs_write+0xb4/0x13c)
[<c01a6420>] (vfs_write) from [<c01a65b8>] (SyS_write+0x40/0x74)
[<c01a65b8>] (SyS_write) from [<c0102480>] (ret_fast_syscall+0x0/0x44)
ubi0 warning: ubi_eba_read_leb: corrupted VID header at PEB 309, LEB 0:0
ubi0 warning: vol_cdev_write: volume 0 on UBI device 0 is corrupted

Quoted 4 lines

Quoted 2 lines
There's another patchset to deal with this issue:
http://thread.gmane.org/gmane.linux.drivers.mtd/54167
Are you sure it fixes your problem??? Did you try it?

Just forward ported them to -next yesterday. Will give it a try for sure.

Quoted 47 lines

Quoted 4 lines
And it brings a problem to us as those patches are mutually exclusive. Once no subpage write patch is applied UBI's VID header offset changes to 2048 from 512, so on flash filesystem is no longer readable if we wish to give optimize subpage writes a chance later.
If your NAND does not support subpage writes, you have to expose a min_io_size of a page not a subpage. AFAICT, the patchset you're referring to won't fix your problem.
Quoted 3 lines
I do want to introduce any hard to overcome incompatibility, so isn't Pekon's patch worth considering again?
Incompatibility with what? The datasheet clearly says that the chip does not support subpage writes.
Quoted 31 lines
Quoted 30 lines
Quoted 16 lines
 drivers/mtd/nand/nand_samsung.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)
drivers/mtd/nand/nand_samsung.c +0 -4
index f6b0a63a068c..9400b4a84243 100644
@@ -92,10 +92,17 @@ static void samsung_nand_decode_id(struct nand_chip *chip)
 	} else {
 		nand_decode_ext_id(chip);
 
-		/* Datasheet values for SLC Samsung K9F4G08U0D-S[I|C]B0(T00) */
-		if (nand_is_slc(chip) && chip->id.data[1] == 0xDC) {
-			chip->ecc_step_ds = 512;
-			chip->ecc_strength_ds = 1;
drivers/mtd/nand/nand_samsung.c static void samsung_nand_decode_id(struct nand_chip *chip) · line 104
Quoted 10 lines
drivers/mtd/nand/nand_samsung.c +10 -0
+		if (nand_is_slc(chip)) {
+			/* K9F4G08U0D-S[I|C]B0(T00) */
+			if (chip->id.data[1] == 0xDC) {
+				chip->ecc_step_ds = 512;
+				chip->ecc_strength_ds = 1;
+			}
+
+			/* 21nm chips do not support partial page write */
+			if (chip->id.len > 4 &&
+			    (chip->id.data[4] & GENMASK(1,0)) == 0x1)  
NAND vendors tend to change their ID decoding scheme a lot, so maybe we should be more restrictive here: replace "chip->id.len > 4" by "chip->id.len == 5" and restrict it to chip->id.data[1] == 0xF1.

Quoted 5 lines

Quoted 2 lines
Well, K9F1G08U0E returns chip->id.len = 6, but adding chip->id.data[1] seems to be a good idea.
That's not what the datasheet says :-/. What's the value of chip->id.data[5]?

The result of
printk("id_len %d, id_data4 %x, id_data5 %x\n",

	chip->id.len, chip->id.data[4], chip->id.data[5]);

is
id_len 6, id_data4 41, id_data5 ec

Quoted 7 lines

Quoted 7 lines
Quoted 4 lines
drivers/mtd/nand/nand_samsung.c static void samsung_nand_decode_id(struct nand_chip *chip) · line 108
Quoted 4 lines
drivers/mtd/nand/nand_samsung.c +1 -0
+				chip->options |= NAND_NO_SUBPAGE_WRITE;
 		}
 	}
 }  
Best regards,
ladis

Boris Brezillon

2018-01-09 10:06 UTC #5 lore raw

Quoted 151 lines · On Tue, 9 Jan 2018 10:58:03 +0100 Ladislav Michl <[email protected]> wrote:

Quoted 93 lines · Boris Brezillon
Quoted 90 lines · On Tue, 9 Jan 2018 10:08:45 +0100 Ladislav Michl <[email protected]> wrote:
Quoted 6 lines · Boris Brezillon
Quoted 1 line · On Tue, 9 Jan 2018 00:48:37 +0100 Ladislav Michl <[email protected]> wrote:
Samsung E-die SLC NAND manufactured using 21nm process supports only
I would add the chip name here (K9F1G08U0E).
Ok.
Quoted 8 lines
Quoted 1 line
1 partial program cycle, so disable subpage writes for it.
Which means it does not support partial page programming, so how about rewording it like that:
Samsung E-die SLC NAND manufactured using 21nm process (K9F1G08U0E) does not support partial page programming, so disable subpage writes for it.
Yes, will post v2 eventually, but see bellow.
Quoted 9 lines
Quoted 6 lines
Manufacturing process is stored in lowest two bits of 5th ID byte.
Signed-off-by Ladislav Michl <[email protected]>
---
Note: Patch generated and tested against next-20180108 on at91sam9g20
       board with K9F1G08U0E.    
Just out of curiosity, what are the symptoms when you don't have this flag set?
Device is identified as:
nand: device found, Manufacturer ID: 0xec, Chip ID: 0xf1
nand: Samsung NAND 128MiB 3,3V 8-bit
nand: 128 MiB, SLC, erase size: 128 KiB, page size: 2048, OOB size: 64
Short test:
# flash_erase /dev/mtd4 0 0
Erasing 128 Kibyte @ 7f60000 -- 99 % complete flash_erase: Skipping bad block at 07f80000
flash_erase: Skipping bad block at 07fa0000
flash_erase: Skipping bad block at 07fc0000
flash_erase: Skipping bad block at 07fe0000
Erasing 128 Kibyte @ 7fe0000 -- 100 % complete 
# ubiformat /dev/mtd4
ubiformat: mtd4 (nand), size 134217728 bytes (128.0 MiB), 1024 eraseblocks of 131072 bytes (128.0 KiB), min. I/O size 2048 bytes
libscan: scanning eraseblock 1023 -- 100 % complete  
ubiformat: 1020 eraseblocks are supposedly empty
ubiformat: 4 bad eraseblocks found, numbers: 1020, 1021, 1022, 1023
ubiformat: formatting eraseblock 1023 -- 100 % complete  
# ubiattach -m 4
ubi0: default fastmap pool size: 50
ubi0: default fastmap WL pool size: 25
ubi0: attaching mtd4
ubi0: scanning is finished
ubi0: attached mtd4 (name "atmel_nand", size 128 MiB)
ubi0: PEB size: 131072 bytes (128 KiB), LEB size: 126976 bytes
ubi0: min./max. I/O unit sizes: 2048/2048, sub-page size 2048
ubi0: VID header offset: 2048 (aligned 2048), data offset: 4096
ubi0: good PEBs: 1020, bad PEBs: 4, corrupted PEBs: 0
ubi0: user volume: 0, internal volumes: 1, max. volumes count: 128
ubi0: max/mean erase counter: 0/0, WL threshold: 4096, image sequence number: 1387212117
ubi0: available PEBs: 998, total reserved PEBs: 22, PEBs reserved for bad PEB handling: 16
ubi0: background thread "ubi_bgt0d" started, PID 716
UBI device number 0, total 1020 LEBs (129515520 bytes, 123.5 MiB), available 998 LEBs (126722048 bytes, 120.9 MiB), LEB size 126976 bytes (124.0 KiB)
# ubimkvol /dev/ubi0 -s 4MiB -N kernel -t static
Volume ID 0, size 34 LEBs (4317184 bytes, 4.1 MiB), LEB size 126976 bytes (124.0 KiB), static, name "kernel", alignment 1
# ubiupdatevol /dev/ubi0_0 linuximage 
__nand_correct_data: uncorrectable ECC error
ubi0 warning: ubi_io_read: error -74 (ECC error) while reading 64 bytes from PEB 309:512, read only 64 bytes, retry
__nand_correct_data: uncorrectable ECC error
ubi0 warning: ubi_io_read: error -74 (ECC error) while reading 64 bytes from PEB 309:512, read only 64 bytes, retry
__nand_correct_data: uncorrectable ECC error
ubi0 warning: ubi_io_read: error -74 (ECC error) while reading 64 bytes from PEB 309:512, read only 64 bytes, retry
__nand_correct_data: uncorrectable ECC error
ubi0 error: ubi_io_read: error -74 (ECC error) while reading 64 bytes from PEB 309:512, read 64 bytes
CPU: 0 PID: 720 Comm: ubiupdatevol Not tainted 4.14.4 #7
Hardware name: Atmel AT91SAM9
[<c010796c>] (unwind_backtrace) from [<c01053e0>] (show_stack+0x10/0x14)
[<c01053e0>] (show_stack) from [<c0337fac>] (ubi_io_read+0x1d8/0x2ac)
[<c0337fac>] (ubi_io_read) from [<c03384c4>] (ubi_io_read_vid_hdr+0x70/0x214)
[<c03384c4>] (ubi_io_read_vid_hdr) from [<c0335b44>] (ubi_eba_read_leb+0x154/0x414)
[<c0335b44>] (ubi_eba_read_leb) from [<c033e5b8>] (ubi_check_volume+0x7c/0xb8)
[<c033e5b8>] (ubi_check_volume) from [<c0334180>] (vol_cdev_write+0x254/0x364)
[<c0334180>] (vol_cdev_write) from [<c01a617c>] (__vfs_write+0x20/0x128)
[<c01a617c>] (__vfs_write) from [<c01a6420>] (vfs_write+0xb4/0x13c)
[<c01a6420>] (vfs_write) from [<c01a65b8>] (SyS_write+0x40/0x74)
[<c01a65b8>] (SyS_write) from [<c0102480>] (ret_fast_syscall+0x0/0x44)
ubi0 warning: ubi_eba_read_leb: corrupted VID header at PEB 309, LEB 0:0
ubi0 warning: vol_cdev_write: volume 0 on UBI device 0 is corrupted
Quoted 4 lines
Quoted 2 lines
There's another patchset to deal with this issue:
http://thread.gmane.org/gmane.linux.drivers.mtd/54167
Are you sure it fixes your problem??? Did you try it?
Just forward ported them to -next yesterday. Will give it a try for sure.
Quoted 50 lines
Quoted 4 lines
And it brings a problem to us as those patches are mutually exclusive. Once no subpage write patch is applied UBI's VID header offset changes to 2048 from 512, so on flash filesystem is no longer readable if we wish to give optimize subpage writes a chance later.
If your NAND does not support subpage writes, you have to expose a min_io_size of a page not a subpage. AFAICT, the patchset you're referring to won't fix your problem.
Quoted 3 lines
I do want to introduce any hard to overcome incompatibility, so isn't Pekon's patch worth considering again?
Incompatibility with what? The datasheet clearly says that the chip does not support subpage writes.
Quoted 33 lines
Quoted 22 lines
Quoted 22 lines
  
 drivers/mtd/nand/nand_samsung.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)
drivers/mtd/nand/nand_samsung.c +6 -4
index f6b0a63a068c..9400b4a84243 100644
@@ -92,10 +92,17 @@ static void samsung_nand_decode_id(struct nand_chip *chip)
 	} else {
 		nand_decode_ext_id(chip);
 
-		/* Datasheet values for SLC Samsung K9F4G08U0D-S[I|C]B0(T00) */
-		if (nand_is_slc(chip) && chip->id.data[1] == 0xDC) {
-			chip->ecc_step_ds = 512;
-			chip->ecc_strength_ds = 1;
+		if (nand_is_slc(chip)) {
+			/* K9F4G08U0D-S[I|C]B0(T00) */
+			if (chip->id.data[1] == 0xDC) {
+				chip->ecc_step_ds = 512;
+				chip->ecc_strength_ds = 1;
+			}
Quoted 8 lines
drivers/mtd/nand/nand_samsung.c static void samsung_nand_decode_id(struct nand_chip *chip) · line 104
Quoted 4 lines
drivers/mtd/nand/nand_samsung.c +4 -0
+
+			/* 21nm chips do not support partial page write */
+			if (chip->id.len > 4 &&
+			    (chip->id.data[4] & GENMASK(1,0)) == 0x1)    
NAND vendors tend to change their ID decoding scheme a lot, so maybe we should be more restrictive here: replace "chip->id.len > 4" by "chip->id.len == 5" and restrict it to chip->id.data[1] == 0xF1.
Well, K9F1G08U0E returns chip->id.len = 6, but adding chip->id.data[1] seems to be a good idea.

Quoted 8 lines

Quoted 2 lines
That's not what the datasheet says :-/. What's the value of chip->id.data[5]?
The result of
printk("id_len %d, id_data4 %x, id_data5 %x\n",
	chip->id.len, chip->id.data[4], chip->id.data[5]);
is
id_len 6, id_data4 41, id_data5 ec

So id.data[5] == id.data[0], can you print id.data[6] and id.data[7]?

Ladislav Michl

author

2018-01-09 11:19 UTC #6 lore raw

Quoted 97 lines · Boris Brezillon

Quoted 94 lines · On Tue, 9 Jan 2018 10:58:03 +0100 Ladislav Michl <[email protected]> wrote:

Quoted 93 lines · Boris Brezillon

Quoted 90 lines · On Tue, 9 Jan 2018 10:08:45 +0100 Ladislav Michl <[email protected]> wrote:

Quoted 6 lines · Boris Brezillon

Quoted 1 line · On Tue, 9 Jan 2018 00:48:37 +0100 Ladislav Michl <[email protected]> wrote:
Samsung E-die SLC NAND manufactured using 21nm process supports only
I would add the chip name here (K9F1G08U0E).

Ok.

Quoted 8 lines

Quoted 1 line
1 partial program cycle, so disable subpage writes for it.
Which means it does not support partial page programming, so how about rewording it like that:
Samsung E-die SLC NAND manufactured using 21nm process (K9F1G08U0E) does not support partial page programming, so disable subpage writes for it.

Yes, will post v2 eventually, but see bellow.

Quoted 9 lines

Quoted 6 lines
Manufacturing process is stored in lowest two bits of 5th ID byte.
Signed-off-by Ladislav Michl <[email protected]>
---
Note: Patch generated and tested against next-20180108 on at91sam9g20
       board with K9F1G08U0E.    
Just out of curiosity, what are the symptoms when you don't have this flag set?

Device is identified as:
nand: device found, Manufacturer ID: 0xec, Chip ID: 0xf1
nand: Samsung NAND 128MiB 3,3V 8-bit
nand: 128 MiB, SLC, erase size: 128 KiB, page size: 2048, OOB size: 64

Short test:

# flash_erase /dev/mtd4 0 0
Erasing 128 Kibyte @ 7f60000 -- 99 % complete flash_erase: Skipping bad block at 07f80000
flash_erase: Skipping bad block at 07fa0000
flash_erase: Skipping bad block at 07fc0000
flash_erase: Skipping bad block at 07fe0000
Erasing 128 Kibyte @ 7fe0000 -- 100 % complete 
# ubiformat /dev/mtd4
ubiformat: mtd4 (nand), size 134217728 bytes (128.0 MiB), 1024 eraseblocks of 131072 bytes (128.0 KiB), min. I/O size 2048 bytes
libscan: scanning eraseblock 1023 -- 100 % complete  
ubiformat: 1020 eraseblocks are supposedly empty
ubiformat: 4 bad eraseblocks found, numbers: 1020, 1021, 1022, 1023
ubiformat: formatting eraseblock 1023 -- 100 % complete  
# ubiattach -m 4
ubi0: default fastmap pool size: 50
ubi0: default fastmap WL pool size: 25
ubi0: attaching mtd4
ubi0: scanning is finished
ubi0: attached mtd4 (name "atmel_nand", size 128 MiB)
ubi0: PEB size: 131072 bytes (128 KiB), LEB size: 126976 bytes
ubi0: min./max. I/O unit sizes: 2048/2048, sub-page size 2048
ubi0: VID header offset: 2048 (aligned 2048), data offset: 4096
ubi0: good PEBs: 1020, bad PEBs: 4, corrupted PEBs: 0
ubi0: user volume: 0, internal volumes: 1, max. volumes count: 128
ubi0: max/mean erase counter: 0/0, WL threshold: 4096, image sequence number: 1387212117
ubi0: available PEBs: 998, total reserved PEBs: 22, PEBs reserved for bad PEB handling: 16
ubi0: background thread "ubi_bgt0d" started, PID 716
UBI device number 0, total 1020 LEBs (129515520 bytes, 123.5 MiB), available 998 LEBs (126722048 bytes, 120.9 MiB), LEB size 126976 bytes (124.0 KiB)
# ubimkvol /dev/ubi0 -s 4MiB -N kernel -t static
Volume ID 0, size 34 LEBs (4317184 bytes, 4.1 MiB), LEB size 126976 bytes (124.0 KiB), static, name "kernel", alignment 1
# ubiupdatevol /dev/ubi0_0 linuximage 
__nand_correct_data: uncorrectable ECC error
ubi0 warning: ubi_io_read: error -74 (ECC error) while reading 64 bytes from PEB 309:512, read only 64 bytes, retry
__nand_correct_data: uncorrectable ECC error
ubi0 warning: ubi_io_read: error -74 (ECC error) while reading 64 bytes from PEB 309:512, read only 64 bytes, retry
__nand_correct_data: uncorrectable ECC error
ubi0 warning: ubi_io_read: error -74 (ECC error) while reading 64 bytes from PEB 309:512, read only 64 bytes, retry
__nand_correct_data: uncorrectable ECC error
ubi0 error: ubi_io_read: error -74 (ECC error) while reading 64 bytes from PEB 309:512, read 64 bytes
CPU: 0 PID: 720 Comm: ubiupdatevol Not tainted 4.14.4 #7
Hardware name: Atmel AT91SAM9
[<c010796c>] (unwind_backtrace) from [<c01053e0>] (show_stack+0x10/0x14)
[<c01053e0>] (show_stack) from [<c0337fac>] (ubi_io_read+0x1d8/0x2ac)
[<c0337fac>] (ubi_io_read) from [<c03384c4>] (ubi_io_read_vid_hdr+0x70/0x214)
[<c03384c4>] (ubi_io_read_vid_hdr) from [<c0335b44>] (ubi_eba_read_leb+0x154/0x414)
[<c0335b44>] (ubi_eba_read_leb) from [<c033e5b8>] (ubi_check_volume+0x7c/0xb8)
[<c033e5b8>] (ubi_check_volume) from [<c0334180>] (vol_cdev_write+0x254/0x364)
[<c0334180>] (vol_cdev_write) from [<c01a617c>] (__vfs_write+0x20/0x128)
[<c01a617c>] (__vfs_write) from [<c01a6420>] (vfs_write+0xb4/0x13c)
[<c01a6420>] (vfs_write) from [<c01a65b8>] (SyS_write+0x40/0x74)
[<c01a65b8>] (SyS_write) from [<c0102480>] (ret_fast_syscall+0x0/0x44)
ubi0 warning: ubi_eba_read_leb: corrupted VID header at PEB 309, LEB 0:0
ubi0 warning: vol_cdev_write: volume 0 on UBI device 0 is corrupted

Quoted 6 lines

Quoted 6 lines
Quoted 4 lines
Quoted 2 lines
There's another patchset to deal with this issue:
http://thread.gmane.org/gmane.linux.drivers.mtd/54167
Are you sure it fixes your problem??? Did you try it?
Just forward ported them to -next yesterday. Will give it a try for sure.

Assuming I forward ported patches correctly they does _not_ solve my problem, as you expected :)

Quoted 53 lines

Quoted 53 lines
Quoted 52 lines
Quoted 4 lines
And it brings a problem to us as those patches are mutually exclusive. Once no subpage write patch is applied UBI's VID header offset changes to 2048 from 512, so on flash filesystem is no longer readable if we wish to give optimize subpage writes a chance later.
If your NAND does not support subpage writes, you have to expose a min_io_size of a page not a subpage. AFAICT, the patchset you're referring to won't fix your problem.
Quoted 3 lines
I do want to introduce any hard to overcome incompatibility, so isn't Pekon's patch worth considering again?
Incompatibility with what? The datasheet clearly says that the chip does not support subpage writes.
Quoted 31 lines
Quoted 22 lines
Quoted 22 lines
  
 drivers/mtd/nand/nand_samsung.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)
drivers/mtd/nand/nand_samsung.c +6 -4
index f6b0a63a068c..9400b4a84243 100644
@@ -92,10 +92,17 @@ static void samsung_nand_decode_id(struct nand_chip *chip)
 	} else {
 		nand_decode_ext_id(chip);
 
-		/* Datasheet values for SLC Samsung K9F4G08U0D-S[I|C]B0(T00) */
-		if (nand_is_slc(chip) && chip->id.data[1] == 0xDC) {
-			chip->ecc_step_ds = 512;
-			chip->ecc_strength_ds = 1;
+		if (nand_is_slc(chip)) {
+			/* K9F4G08U0D-S[I|C]B0(T00) */
+			if (chip->id.data[1] == 0xDC) {
+				chip->ecc_step_ds = 512;
+				chip->ecc_strength_ds = 1;
+			}
Quoted 8 lines
drivers/mtd/nand/nand_samsung.c static void samsung_nand_decode_id(struct nand_chip *chip) · line 104
Quoted 4 lines
drivers/mtd/nand/nand_samsung.c +4 -0
+
+			/* 21nm chips do not support partial page write */
+			if (chip->id.len > 4 &&
+			    (chip->id.data[4] & GENMASK(1,0)) == 0x1)    
NAND vendors tend to change their ID decoding scheme a lot, so maybe we should be more restrictive here: replace "chip->id.len > 4" by "chip->id.len == 5" and restrict it to chip->id.data[1] == 0xF1.
Quoted 2 lines
Well, K9F1G08U0E returns chip->id.len = 6, but adding chip->id.data[1] seems to be a good idea.
That's not what the datasheet says :-/. What's the value of chip->id.data[5]?

Quoted 7 lines

Quoted 5 lines
The result of
printk("id_len %d, id_data4 %x, id_data5 %x\n",
	chip->id.len, chip->id.data[4], chip->id.data[5]);
is
id_len 6, id_data4 41, id_data5 ec
So id.data[5] == id.data[0], can you print id.data[6] and id.data[7]?

The result of
printk("id_len %d, id_data4 %x, id_data5 %x, id_data6 %x, id_data7 %x\n",

        chip->id.len, chip->id.data[4], chip->id.data[5], chip->id.data[6], chip->id.data[7]);

is
id_len 6, id_data4 41, id_data5 ec, id_data6 ec, id_data7 f1

Based on previous comments patch so far looks like (and unless there are objections, I'm temped to replace if (chip->id.data[1] == XX) with switch statement):

drivers/mtd/nand/nand_samsung.c +12 -4

index f6b0a63a068c..67ca9978cf27 100644
@@ -92,10 +92,18 @@ static void samsung_nand_decode_id(struct nand_chip *chip)
 	} else {
 		nand_decode_ext_id(chip);
 
-		/* Datasheet values for SLC Samsung K9F4G08U0D-S[I|C]B0(T00) */
-		if (nand_is_slc(chip) && chip->id.data[1] == 0xDC) {
-			chip->ecc_step_ds = 512;
-			chip->ecc_strength_ds = 1;
+		if (nand_is_slc(chip)) {
+			/* K9F4G08U0D-S[I|C]B0(T00) */
+			if (chip->id.data[1] == 0xDC) {
+				chip->ecc_step_ds = 512;
+				chip->ecc_strength_ds = 1;
+			}
+
+			/* 21nm chips do not support partial page write */
+			if (chip->id.data[1] == 0xF1 &&
+			    chip->id.len > 4 &&
+			    (chip->id.data[4] & GENMASK(1,0)) == 0x1)
+				chip->options |= NAND_NO_SUBPAGE_WRITE;
 		}
 	}
 }

Boris Brezillon

2018-01-09 13:07 UTC #7 lore raw

Quoted 167 lines · On Tue, 9 Jan 2018 12:19:13 +0100 Ladislav Michl <[email protected]> wrote:

Quoted 97 lines · Boris Brezillon
Quoted 94 lines · On Tue, 9 Jan 2018 10:58:03 +0100 Ladislav Michl <[email protected]> wrote:
Quoted 93 lines · Boris Brezillon
Quoted 90 lines · On Tue, 9 Jan 2018 10:08:45 +0100 Ladislav Michl <[email protected]> wrote:
Quoted 6 lines · Boris Brezillon
Quoted 1 line · On Tue, 9 Jan 2018 00:48:37 +0100 Ladislav Michl <[email protected]> wrote:
Samsung E-die SLC NAND manufactured using 21nm process supports only
I would add the chip name here (K9F1G08U0E).
Ok.
Quoted 8 lines
Quoted 1 line
1 partial program cycle, so disable subpage writes for it.
Which means it does not support partial page programming, so how about rewording it like that:
Samsung E-die SLC NAND manufactured using 21nm process (K9F1G08U0E) does not support partial page programming, so disable subpage writes for it.
Yes, will post v2 eventually, but see bellow.
Quoted 9 lines
Quoted 6 lines
Manufacturing process is stored in lowest two bits of 5th ID byte.
Signed-off-by Ladislav Michl <[email protected]>
---
Note: Patch generated and tested against next-20180108 on at91sam9g20
       board with K9F1G08U0E.      
Just out of curiosity, what are the symptoms when you don't have this flag set?
Device is identified as:
nand: device found, Manufacturer ID: 0xec, Chip ID: 0xf1
nand: Samsung NAND 128MiB 3,3V 8-bit
nand: 128 MiB, SLC, erase size: 128 KiB, page size: 2048, OOB size: 64
Short test:
# flash_erase /dev/mtd4 0 0
Erasing 128 Kibyte @ 7f60000 -- 99 % complete flash_erase: Skipping bad block at 07f80000
flash_erase: Skipping bad block at 07fa0000
flash_erase: Skipping bad block at 07fc0000
flash_erase: Skipping bad block at 07fe0000
Erasing 128 Kibyte @ 7fe0000 -- 100 % complete 
# ubiformat /dev/mtd4
ubiformat: mtd4 (nand), size 134217728 bytes (128.0 MiB), 1024 eraseblocks of 131072 bytes (128.0 KiB), min. I/O size 2048 bytes
libscan: scanning eraseblock 1023 -- 100 % complete  
ubiformat: 1020 eraseblocks are supposedly empty
ubiformat: 4 bad eraseblocks found, numbers: 1020, 1021, 1022, 1023
ubiformat: formatting eraseblock 1023 -- 100 % complete  
# ubiattach -m 4
ubi0: default fastmap pool size: 50
ubi0: default fastmap WL pool size: 25
ubi0: attaching mtd4
ubi0: scanning is finished
ubi0: attached mtd4 (name "atmel_nand", size 128 MiB)
ubi0: PEB size: 131072 bytes (128 KiB), LEB size: 126976 bytes
ubi0: min./max. I/O unit sizes: 2048/2048, sub-page size 2048
ubi0: VID header offset: 2048 (aligned 2048), data offset: 4096
ubi0: good PEBs: 1020, bad PEBs: 4, corrupted PEBs: 0
ubi0: user volume: 0, internal volumes: 1, max. volumes count: 128
ubi0: max/mean erase counter: 0/0, WL threshold: 4096, image sequence number: 1387212117
ubi0: available PEBs: 998, total reserved PEBs: 22, PEBs reserved for bad PEB handling: 16
ubi0: background thread "ubi_bgt0d" started, PID 716
UBI device number 0, total 1020 LEBs (129515520 bytes, 123.5 MiB), available 998 LEBs (126722048 bytes, 120.9 MiB), LEB size 126976 bytes (124.0 KiB)
# ubimkvol /dev/ubi0 -s 4MiB -N kernel -t static
Volume ID 0, size 34 LEBs (4317184 bytes, 4.1 MiB), LEB size 126976 bytes (124.0 KiB), static, name "kernel", alignment 1
# ubiupdatevol /dev/ubi0_0 linuximage 
__nand_correct_data: uncorrectable ECC error
ubi0 warning: ubi_io_read: error -74 (ECC error) while reading 64 bytes from PEB 309:512, read only 64 bytes, retry
__nand_correct_data: uncorrectable ECC error
ubi0 warning: ubi_io_read: error -74 (ECC error) while reading 64 bytes from PEB 309:512, read only 64 bytes, retry
__nand_correct_data: uncorrectable ECC error
ubi0 warning: ubi_io_read: error -74 (ECC error) while reading 64 bytes from PEB 309:512, read only 64 bytes, retry
__nand_correct_data: uncorrectable ECC error
ubi0 error: ubi_io_read: error -74 (ECC error) while reading 64 bytes from PEB 309:512, read 64 bytes
CPU: 0 PID: 720 Comm: ubiupdatevol Not tainted 4.14.4 #7
Hardware name: Atmel AT91SAM9
[<c010796c>] (unwind_backtrace) from [<c01053e0>] (show_stack+0x10/0x14)
[<c01053e0>] (show_stack) from [<c0337fac>] (ubi_io_read+0x1d8/0x2ac)
[<c0337fac>] (ubi_io_read) from [<c03384c4>] (ubi_io_read_vid_hdr+0x70/0x214)
[<c03384c4>] (ubi_io_read_vid_hdr) from [<c0335b44>] (ubi_eba_read_leb+0x154/0x414)
[<c0335b44>] (ubi_eba_read_leb) from [<c033e5b8>] (ubi_check_volume+0x7c/0xb8)
[<c033e5b8>] (ubi_check_volume) from [<c0334180>] (vol_cdev_write+0x254/0x364)
[<c0334180>] (vol_cdev_write) from [<c01a617c>] (__vfs_write+0x20/0x128)
[<c01a617c>] (__vfs_write) from [<c01a6420>] (vfs_write+0xb4/0x13c)
[<c01a6420>] (vfs_write) from [<c01a65b8>] (SyS_write+0x40/0x74)
[<c01a65b8>] (SyS_write) from [<c0102480>] (ret_fast_syscall+0x0/0x44)
ubi0 warning: ubi_eba_read_leb: corrupted VID header at PEB 309, LEB 0:0
ubi0 warning: vol_cdev_write: volume 0 on UBI device 0 is corrupted
Quoted 6 lines
Quoted 6 lines
Quoted 4 lines
Quoted 2 lines
There's another patchset to deal with this issue:
http://thread.gmane.org/gmane.linux.drivers.mtd/54167
Are you sure it fixes your problem??? Did you try it?
Just forward ported them to -next yesterday. Will give it a try for sure.
Assuming I forward ported patches correctly they does _not_ solve my problem, as you expected :)
Quoted 59 lines
Quoted 58 lines
Quoted 47 lines
Quoted 4 lines
And it brings a problem to us as those patches are mutually exclusive. Once no subpage write patch is applied UBI's VID header offset changes to 2048 from 512, so on flash filesystem is no longer readable if we wish to give optimize subpage writes a chance later.
If your NAND does not support subpage writes, you have to expose a min_io_size of a page not a subpage. AFAICT, the patchset you're referring to won't fix your problem.
Quoted 3 lines
I do want to introduce any hard to overcome incompatibility, so isn't Pekon's patch worth considering again?
Incompatibility with what? The datasheet clearly says that the chip does not support subpage writes.
Quoted 31 lines
Quoted 30 lines
Quoted 16 lines
    
 drivers/mtd/nand/nand_samsung.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)
drivers/mtd/nand/nand_samsung.c +0 -4
index f6b0a63a068c..9400b4a84243 100644
@@ -92,10 +92,17 @@ static void samsung_nand_decode_id(struct nand_chip *chip)
 	} else {
 		nand_decode_ext_id(chip);
 
-		/* Datasheet values for SLC Samsung K9F4G08U0D-S[I|C]B0(T00) */
-		if (nand_is_slc(chip) && chip->id.data[1] == 0xDC) {
-			chip->ecc_step_ds = 512;
-			chip->ecc_strength_ds = 1;
drivers/mtd/nand/nand_samsung.c static void samsung_nand_decode_id(struct nand_chip *chip) · line 104
Quoted 10 lines
drivers/mtd/nand/nand_samsung.c +10 -0
+		if (nand_is_slc(chip)) {
+			/* K9F4G08U0D-S[I|C]B0(T00) */
+			if (chip->id.data[1] == 0xDC) {
+				chip->ecc_step_ds = 512;
+				chip->ecc_strength_ds = 1;
+			}
+
+			/* 21nm chips do not support partial page write */
+			if (chip->id.len > 4 &&
+			    (chip->id.data[4] & GENMASK(1,0)) == 0x1)      
NAND vendors tend to change their ID decoding scheme a lot, so maybe we should be more restrictive here: replace "chip->id.len > 4" by "chip->id.len == 5" and restrict it to chip->id.data[1] == 0xF1.
Quoted 5 lines
Quoted 2 lines
Well, K9F1G08U0E returns chip->id.len = 6, but adding chip->id.data[1] seems to be a good idea.
That's not what the datasheet says :-/. What's the value of chip->id.data[5]?
The result of
printk("id_len %d, id_data4 %x, id_data5 %x\n",
	chip->id.len, chip->id.data[4], chip->id.data[5]);
is
id_len 6, id_data4 41, id_data5 ec

Quoted 7 lines

Quoted 1 line
So id.data[5] == id.data[0], can you print id.data[6] and id.data[7]?
The result of
printk("id_len %d, id_data4 %x, id_data5 %x, id_data6 %x, id_data7 %x\n",
        chip->id.len, chip->id.data[4], chip->id.data[5], chip->id.data[6], chip->id.data[7]);
is
id_len 6, id_data4 41, id_data5 ec, id_data6 ec, id_data7 f1

Okay, this explains why we get a len of 6 instead of 5: the manufacturer code is repeated twice (data[5] and data[6]).

Quoted 4 lines

Based on previous comments patch so far looks like (and unless there are objections, I'm temped to replace if (chip->id.data[1] == XX) with switch statement):

Yes, sounds good (one extra comment below)

Quoted 11 lines

drivers/mtd/nand/nand_samsung.c +0 -2

index f6b0a63a068c..67ca9978cf27 100644
@@ -92,10 +92,18 @@ static void samsung_nand_decode_id(struct nand_chip *chip)
 	} else {
 		nand_decode_ext_id(chip);
 
-		/* Datasheet values for SLC Samsung K9F4G08U0D-S[I|C]B0(T00) */
-		if (nand_is_slc(chip) && chip->id.data[1] == 0xDC) {

drivers/mtd/nand/nand_samsung.c static void samsung_nand_decode_id(struct nand_chip *chip) · line 102

Quoted 10 lines

drivers/mtd/nand/nand_samsung.c +8 -2

-			chip->ecc_step_ds = 512;
-			chip->ecc_strength_ds = 1;
+		if (nand_is_slc(chip)) {
+			/* K9F4G08U0D-S[I|C]B0(T00) */
+			if (chip->id.data[1] == 0xDC) {
+				chip->ecc_step_ds = 512;
+				chip->ecc_strength_ds = 1;
+			}
+
+			/* 21nm chips do not support partial page write */

Please give the chip name in your comment (K9F1G08U0E).

Quoted 7 lines

drivers/mtd/nand/nand_samsung.c +4 -0

+			if (chip->id.data[1] == 0xF1 &&
+			    chip->id.len > 4 &&
+			    (chip->id.data[4] & GENMASK(1,0)) == 0x1)
+				chip->options |= NAND_NO_SUBPAGE_WRITE;
 		}
 	}
 }