Minor fix and new Chip IDs for spi-nor.

PATCH In review Michal Suchanek 5 participants 18 messages Apr 30, 2015 – May 21, 2015

Subsystems (2)

Memory technology devices (MTD) SPI NOR

lore

Share & tools

lore

Michal Suchanek

author

2015-04-30 21:55 UTC #0 lore raw

cover Minor fix and new Chip IDs for spi-nor.

Hello,

I tried attaching random flash chips with an ICP cable and found that Linux does not have the chip IDs.

Thanks

Michal

Michal Suchanek (3):
MTD: m25p80: fix write return value.
MTD: spi-nor: Add Pm25LD020 and GD25Q41B chip ID.
MTD: spi-nor: add flag to not use sector erase.

 drivers/mtd/devices/m25p80.c  | 2 +-
 drivers/mtd/spi-nor/spi-nor.c | 4 ++++
 2 files changed, 5 insertions(+), 1 deletion(-)

Signature · 1 line

2.1.4

Michal Suchanek

author

2015-04-30 21:55 UTC #1 lore raw

PATCH 1/3 MTD: m25p80: fix write return value.

The 'retlen' points to a variable representing the number of data bytes written/read (see include/linux/mtd/mtd.h) by the current invocation of the function. This variable must be set, not incremented.

v2: clearer commit message

Signed-off-by Michal Suchanek <[email protected]>

Acked-by Marek Vasut <[email protected]>

---
 drivers/mtd/devices/m25p80.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/mtd/devices/m25p80.c +1 -1

index 7c8b169..0b2bc21 100644
@@ -102,7 +102,7 @@ static void m25p80_write(struct spi_nor *nor, loff_t to, size_t len,
 
 	spi_sync(spi, &m);
 
-	*retlen += m.actual_length - cmd_sz;
+	*retlen = m.actual_length - cmd_sz;
 }
 
 static inline unsigned int m25p80_rx_nbits(struct spi_nor *nor)

Signature · 1 line

2.1.4

Marek Vasut

2015-04-30 23:09 UTC #2 lore raw

Re: [PATCH 1/3] MTD: m25p80: fix write return value.

Quoted 5 lines · Michal Suchanek

The 'retlen' points to a variable representing the number of data bytes written/read (see include/linux/mtd/mtd.h) by the current invocation of the function. This variable must be set, not incremented.
v2: clearer commit message

This V2 goes past the diffstat, so that when the patch is applied, it doesn't end up in the log.

Quoted 5 lines

Signed-off-by Michal Suchanek <[email protected]>
Acked-by Marek Vasut <[email protected]>
---
 drivers/mtd/devices/m25p80.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Aka. V<n> changes go here.

I don't think there's a need to repost tho :)

Quoted 13 lines

drivers/mtd/devices/m25p80.c +1 -1

index 7c8b169..0b2bc21 100644
@@ -102,7 +102,7 @@ static void m25p80_write(struct spi_nor *nor, loff_t
 to, size_t len,
 
 	spi_sync(spi, &m);
 
-	*retlen += m.actual_length - cmd_sz;
+	*retlen = m.actual_length - cmd_sz;
 }

Quoted 1 line

static inline unsigned int m25p80_rx_nbits(struct spi_nor *nor)

Best regards,
Marek Vasut

Brian Norris

2015-05-20 23:45 UTC #3 lore raw

Re: [PATCH 1/3] MTD: m25p80: fix write return value.

In reply to Michal Suchanek

Quoted 17 lines · Michal Suchanek

The 'retlen' points to a variable representing the number of data bytes written/read (see include/linux/mtd/mtd.h) by the current invocation of the function. This variable must be set, not incremented.
v2: clearer commit message
Signed-off-by Michal Suchanek <[email protected]>
Acked-by Marek Vasut <[email protected]>
---
 drivers/mtd/devices/m25p80.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/mtd/devices/m25p80.c +0 -0
index 7c8b169..0b2bc21 100644

drivers/mtd/devices/m25p80.c static void m25p80_write(struct spi_nor *nor, loff_t to, size_t len, · line 105

Quoted 5 lines

drivers/mtd/devices/m25p80.c +1 -1

@@ -102,7 +102,7 @@ static void m25p80_write(struct spi_nor *nor, loff_t to, size_t len,
 
 	spi_sync(spi, &m);
 
-	*retlen += m.actual_length - cmd_sz;
+	*retlen = m.actual_length - cmd_sz;

This is very wrong. It gets a little better once you add your next patches (which are also not good) since those patches reinterpret the usage of retlen, but this one definitely does *not* stand a lone.

I'll admit the API is a little odd here, but the callers of this function (see spi_nor_write()) actually depend on calling this multiple times, with the value incrementing each time so we get a summary total. You're now clobbering this value.

I'm willing to accept patches to improve this API, if you think that would help. Or to add comments that document this.

drivers/mtd/devices/m25p80.c static void m25p80_write(struct spi_nor *nor, loff_t to, size_t len, · line 108

Quoted 3 lines

drivers/mtd/devices/m25p80.c +0 -0

 }
 
 static inline unsigned int m25p80_rx_nbits(struct spi_nor *nor)

Also, I'm a little confused because you sent two different patch series very close to each other, and this patch is in both of them. Please don't do that. Either send a single patch series that contains all patches (and is versions v2, v3, etc., as you revise the whole thing) or send completely independent patch series. Don't include the same patch in different series.

Anyway, please consider my comments, and when you have something better, please resend everything. I'm not going to take either series as-is.

Thanks,
Brian

Michal Suchanek

author

2015-05-21 08:34 UTC #4 lore raw

Re: [PATCH 1/3] MTD: m25p80: fix write return value.

Hello,

Quoted 28 lines · Brian Norris

Quoted 17 lines · Michal Suchanek
The 'retlen' points to a variable representing the number of data bytes written/read (see include/linux/mtd/mtd.h) by the current invocation of the function. This variable must be set, not incremented.
v2: clearer commit message
Signed-off-by Michal Suchanek <[email protected]>
Acked-by Marek Vasut <[email protected]>
---
 drivers/mtd/devices/m25p80.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/mtd/devices/m25p80.c +0 -0
index 7c8b169..0b2bc21 100644
drivers/mtd/devices/m25p80.c static void m25p80_write(struct spi_nor *nor, loff_t to, size_t len, · line 105
Quoted 5 lines
drivers/mtd/devices/m25p80.c +1 -1
@@ -102,7 +102,7 @@ static void m25p80_write(struct spi_nor *nor, loff_t to, size_t len,
 
      spi_sync(spi, &m);
 
-     *retlen += m.actual_length - cmd_sz;
+     *retlen = m.actual_length - cmd_sz;
This is very wrong. It gets a little better once you add your next patches (which are also not good) since those patches reinterpret the usage of retlen, but this one definitely does *not* stand a lone.

Quoted 7 lines

I'll admit the API is a little odd here, but the callers of this function (see spi_nor_write()) actually depend on calling this multiple times, with the value incrementing each time so we get a summary total. You're now clobbering this value.
I'm willing to accept patches to improve this API, if you think that would help. Or to add comments that document this.

Yes, the only user of the retlen value ignores it but passes it on so without the following patch this one makes the passed on value different from before.

For m25p80 this would be fixed by truncating the write in the driver and setting actual_length appropriately rather than returning an error when the message is too long. It might possibly break other drivers, though.

Thanks

Michal

Michal Suchanek

author

2015-04-30 21:55 UTC #5 lore raw

PATCH 2/3 MTD: spi-nor: Add Pm25LD020 and GD25Q41B chip ID.

In reply to Michal Suchanek

First chip reads Pm25LD020 or Pm25L0020. Found on some WD HDD PCB. Identified as PMC Pm25LD020.
Flash read does not return consistent data which explains why the disk died.

Second chip reads something like 25Q41BT. Found on Esspif ESP8266 based ESP-01 board. Identified as Elm Techchnology GD25Q41B. Datasheet is not clear about block size so erasing blocks may not work as expected.

Signed-off-by Michal Suchanek <[email protected]>

---
 drivers/mtd/spi-nor/spi-nor.c | 2 ++
 1 file changed, 2 insertions(+)

drivers/mtd/spi-nor/spi-nor.c +2 -0

index 14a5d23..de73cf5 100644
@@ -551,6 +551,7 @@ static const struct spi_device_id spi_nor_ids[] = {
 	{ "mb85rs1mt", INFO(0x047f27, 0, 128 * 1024, 1, SPI_NOR_NO_ERASE) },
 
 	/* GigaDevice */
+	{ "gd25q41b", INFO(0xc84013, 0, 64 * 1024,  8, SECT_4K) },
 	{ "gd25q32", INFO(0xc84016, 0, 64 * 1024,  64, SECT_4K) },
 	{ "gd25q64", INFO(0xc84017, 0, 64 * 1024, 128, SECT_4K) },
 	{ "gd25q128", INFO(0xc84018, 0, 64 * 1024, 256, SECT_4K) },

@@ -589,6 +590,7 @@ static const struct spi_device_id spi_nor_ids[] = {
 	/* PMC */
 	{ "pm25lv512",   INFO(0,        0, 32 * 1024,    2, SECT_4K_PMC) },
 	{ "pm25lv010",   INFO(0,        0, 32 * 1024,    4, SECT_4K_PMC) },
+	{ "pm25ld020",   INFO(0x7f9d22, 0, 64 * 1024,    4, SECT_4K) },
 	{ "pm25lq032",   INFO(0x7f9d46, 0, 64 * 1024,   64, SECT_4K) },
 
 	/* Spansion -- single (large) sector size only, at least

Signature · 1 line

2.1.4

Michal Suchanek

author

2015-04-30 21:55 UTC #6 lore raw

PATCH 3/3 MTD: spi-nor: add flag to not use sector erase.

In reply to Michal Suchanek

The sector size of the flash memory is unclear from datasheet or may possibly vary between chips so add a flag to always use 4k blocks.

Currently 4k blocks are always used when possible but in the future somebody might want to do some optimizations with sector erase.

Signed-off-by Michal Suchanek <[email protected]>

---
 drivers/mtd/spi-nor/spi-nor.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

drivers/mtd/spi-nor/spi-nor.c +3 -1

index de73cf5..371e91c 100644
@@ -55,6 +55,7 @@ struct flash_info {
 #define	SPI_NOR_DUAL_READ	0x20    /* Flash supports Dual Read */
 #define	SPI_NOR_QUAD_READ	0x40    /* Flash supports Quad Read */
 #define	USE_FSR			0x80	/* use flag status register */
+#define	NO_SPINOR_OP_SE	       0x100	/* do not erase sectors */
 };
 
 #define JEDEC_MFR(info)	((info)->id[0])

@@ -333,6 +334,7 @@ static int spi_nor_erase(struct mtd_info *mtd, struct erase_info *instr)
 	/* REVISIT in some cases we could speed up erasing large regions
 	 * by using SPINOR_OP_SE instead of SPINOR_OP_BE_4K.  We may have set up
 	 * to use "small sector erase", but that's not always optimal.
+	 * The block size for SPINOR_OP_SE is unknown/untested for some flashes.
 	 */
 
 	/* "sector"-at-a-time erase */

@@ -551,7 +553,7 @@ static const struct spi_device_id spi_nor_ids[] = {
 	{ "mb85rs1mt", INFO(0x047f27, 0, 128 * 1024, 1, SPI_NOR_NO_ERASE) },
 
 	/* GigaDevice */
-	{ "gd25q41b", INFO(0xc84013, 0, 64 * 1024,  8, SECT_4K) },
+	{ "gd25q41b", INFO(0xc84013, 0, 64 * 1024,  8, SECT_4K | NO_SPINOR_OP_SE) },
 	{ "gd25q32", INFO(0xc84016, 0, 64 * 1024,  64, SECT_4K) },
 	{ "gd25q64", INFO(0xc84017, 0, 64 * 1024, 128, SECT_4K) },
 	{ "gd25q128", INFO(0xc84018, 0, 64 * 1024, 256, SECT_4K) },

Signature · 1 line

2.1.4

Marek Vasut

2015-04-30 23:13 UTC #7 lore raw

Re: [PATCH 3/3] MTD: spi-nor: add flag to not use sector erase.

Quoted 7 lines · Michal Suchanek

The sector size of the flash memory is unclear from datasheet or may possibly vary between chips so add a flag to always use 4k blocks.
Currently 4k blocks are always used when possible but in the future somebody might want to do some optimizations with sector erase.
Signed-off-by Michal Suchanek <[email protected]>

I _think_ you might be able to determine the size, no ?

One way is to ask the vendor, but you can also try something like:
1) erase the whole SPI NOR
2) overwrite it with zeroes (or ones ? I think it should be all ones after erasing).
3) Erase sector 0
4) Read some 128 KiB back
5) Observe what is the difference.

Best regards,
Marek Vasut

Michal Suchanek

author

2015-05-01 07:05 UTC #8 lore raw

Re: [PATCH 3/3] MTD: spi-nor: add flag to not use sector erase.

Quoted 11 lines · Marek Vasut

Quoted 7 lines · Michal Suchanek
The sector size of the flash memory is unclear from datasheet or may possibly vary between chips so add a flag to always use 4k blocks.
Currently 4k blocks are always used when possible but in the future somebody might want to do some optimizations with sector erase.
Signed-off-by Michal Suchanek <[email protected]>
I _think_ you might be able to determine the size, no ?

Quoted 8 lines

One way is to ask the vendor, but you can also try something like:
1) erase the whole SPI NOR
2) overwrite it with zeroes (or ones ? I think it should be all ones after erasing).
3) Erase sector 0
4) Read some 128 KiB back
5) Observe what is the difference.

I can determine it for this particular chip. However, when the vendor datasheet says the block is 64/32K it might mean that chips with this ID can have either block size.

It's a value that we don't use anyway so I just mark it as unknown here for future reference.

Thanks

Michal

Jonas Gorski

2015-05-01 10:50 UTC #9 lore raw

Re: [PATCH 3/3] MTD: spi-nor: add flag to not use sector erase.

Hi,

Quoted 21 lines · Michal Suchanek

Quoted 11 lines · Marek Vasut
Quoted 7 lines · Michal Suchanek
The sector size of the flash memory is unclear from datasheet or may possibly vary between chips so add a flag to always use 4k blocks.
Currently 4k blocks are always used when possible but in the future somebody might want to do some optimizations with sector erase.
Signed-off-by Michal Suchanek <[email protected]>
I _think_ you might be able to determine the size, no ?
Quoted 8 lines
One way is to ask the vendor, but you can also try something like:
1) erase the whole SPI NOR
2) overwrite it with zeroes (or ones ? I think it should be all ones after erasing).
3) Erase sector 0
4) Read some 128 KiB back
5) Observe what is the difference.

Quoted 6 lines

I can determine it for this particular chip. However, when the vendor datasheet says the block is 64/32K it might mean that chips with this ID can have either block size.
It's a value that we don't use anyway so I just mark it as unknown here for future reference.

It will be used if MTD_SPI_NOR_USE_4K_SECTORS is unset, so you should add some code to properly handle that case.

Also I'd suggest switching the order of 2 and 3, so you add the flag handling first and then add support for a flash chip with this issue.

Regards
Jonas

Marek Vasut

2015-05-01 14:21 UTC #10 lore raw

Re: [PATCH 3/3] MTD: spi-nor: add flag to not use sector erase.

In reply to Michal Suchanek

Quoted 20 lines · Michal Suchanek

Quoted 11 lines · Marek Vasut
Quoted 7 lines · Michal Suchanek
The sector size of the flash memory is unclear from datasheet or may possibly vary between chips so add a flag to always use 4k blocks.
Currently 4k blocks are always used when possible but in the future somebody might want to do some optimizations with sector erase.
Signed-off-by Michal Suchanek <[email protected]>
I _think_ you might be able to determine the size, no ?
Quoted 7 lines
One way is to ask the vendor, but you can also try something like:
1) erase the whole SPI NOR
2) overwrite it with zeroes (or ones ? I think it should be all ones after erasing).
3) Erase sector 0
4) Read some 128 KiB back
5) Observe what is the difference.

Quoted 3 lines

I can determine it for this particular chip. However, when the vendor datasheet says the block is 64/32K it might mean that chips with this ID can have either block size.

http://www.chingistek.com/img/Product_Files/Pm25LD010020datasheet%20v04.pdf
page 21:

SECTOR_ER (20h) erases 4kByte sector.
BLOCK_ER (d8h) erases 64kByte sector.

http://www.gigadevice.com/product/download/366.html?locale=en_US
page 27-28:

Sector Erase (SE) (20h) erases 4kByte sector
64KB Block Erase (BE) (d8h) erases 64kByte sector

Quoted 2 lines

It's a value that we don't use anyway so I just mark it as unknown here for future reference.

Looks like standard SPI NOR opcodes [1], nothing unknown there ;-)

[1] include/linux/mtd/spi-nor.h

Best regards,
Marek Vasut

Michal Suchanek

author

2015-05-04 11:11 UTC #11 lore raw

Re: [PATCH 3/3] MTD: spi-nor: add flag to not use sector erase.

Hello,

Quoted 9 lines · Marek Vasut

Quoted 4 lines · Michal Suchanek
On 1 May 2015 at 01:13, Marek Vasut <[email protected]> wrote:
I can determine it for this particular chip. However, when the vendor datasheet says the block is 64/32K it might mean that chips with this ID can have either block size.
http://www.chingistek.com/img/Product_Files/Pm25LD010020datasheet%20v04.pdf
page 21:

Quoted 9 lines

SECTOR_ER (20h) erases 4kByte sector.
BLOCK_ER (d8h) erases 64kByte sector.
http://www.gigadevice.com/product/download/366.html?locale=en_US
page 27-28:
Sector Erase (SE) (20h) erases 4kByte sector
64KB Block Erase (BE) (d8h) erases 64kByte sector

It's pretty much the same as the datasheet I used
http://www.elm-tech.com/en/products/spi-flash-memory/gd25q41/gd25q41.pdf

It mentions both
32KB Block Erase (BE) (52H)
and
64KB Block Erase (BE) (D8H)

So the chip probably tries its best to be compatible with any command set and this last patch is not needed. The memory organization table on page 7 is not all that reassuring, though.

Thanks

Michal

Marek Vasut

2015-05-04 12:58 UTC #12 lore raw

Re: [PATCH 3/3] MTD: spi-nor: add flag to not use sector erase.

Quoted 1 line · Michal Suchanek

Hello,

Hi!

Quoted 19 lines · Marek Vasut

Quoted 9 lines · Michal Suchanek
Quoted 4 lines · Marek Vasut
I can determine it for this particular chip. However, when the vendor datasheet says the block is 64/32K it might mean that chips with this ID can have either block size.
http://www.chingistek.com/img/Product_Files/Pm25LD010020datasheet%20v04.p
df page 21:
Quoted 8 lines
SECTOR_ER (20h) erases 4kByte sector.
BLOCK_ER (d8h) erases 64kByte sector.
http://www.gigadevice.com/product/download/366.html?locale=en_US
page 27-28:
Sector Erase (SE) (20h) erases 4kByte sector
64KB Block Erase (BE) (d8h) erases 64kByte sector

Quoted 7 lines

It's pretty much the same as the datasheet I used
http://www.elm-tech.com/en/products/spi-flash-memory/gd25q41/gd25q41.pdf
It mentions both
32KB Block Erase (BE) (52H)
and
64KB Block Erase (BE) (D8H)

The SPI NOR framework will use 0xbe opcode, no problem.

Quoted 3 lines

So the chip probably tries its best to be compatible with any command set and this last patch is not needed. The memory organization table on page 7 is not all that reassuring, though.

Which exact part do you refer to please ?

Best regards,
Marek Vasut

Michal Suchanek

author

2015-05-04 13:19 UTC #13 lore raw

Re: [PATCH 3/3] MTD: spi-nor: add flag to not use sector erase.

Quoted 32 lines · Marek Vasut

Quoted 1 line · Michal Suchanek
Hello,
Hi!
Quoted 19 lines · Marek Vasut
Quoted 9 lines · Michal Suchanek
Quoted 4 lines · Marek Vasut
I can determine it for this particular chip. However, when the vendor datasheet says the block is 64/32K it might mean that chips with this ID can have either block size.
http://www.chingistek.com/img/Product_Files/Pm25LD010020datasheet%20v04.p
df page 21:
Quoted 8 lines
SECTOR_ER (20h) erases 4kByte sector.
BLOCK_ER (d8h) erases 64kByte sector.
http://www.gigadevice.com/product/download/366.html?locale=en_US
page 27-28:
Sector Erase (SE) (20h) erases 4kByte sector
64KB Block Erase (BE) (d8h) erases 64kByte sector
Quoted 7 lines
It's pretty much the same as the datasheet I used
http://www.elm-tech.com/en/products/spi-flash-memory/gd25q41/gd25q41.pdf
It mentions both
32KB Block Erase (BE) (52H)
and
64KB Block Erase (BE) (D8H)

Quoted 7 lines

The SPI NOR framework will use 0xbe opcode, no problem.
Quoted 3 lines
So the chip probably tries its best to be compatible with any command set and this last patch is not needed. The memory organization table on page 7 is not all that reassuring, though.
Which exact part do you refer to please ?

Start of page 7 where it says sector size 32/64K in either datasheet.

It can refer to both BE opcode variants being supported but it's quite unclear.

Write protection seems to be calculated in 4k sectors and not blocks so the block size does not seem very relevant.

Thanks

Michal

Marek Vasut

2015-05-04 13:35 UTC #14 lore raw

Re: [PATCH 3/3] MTD: spi-nor: add flag to not use sector erase.

Quoted 39 lines · Michal Suchanek

Quoted 38 lines · Marek Vasut
Quoted 1 line · Michal Suchanek
Hello,
Hi!
Quoted 19 lines · Marek Vasut
Quoted 9 lines · Michal Suchanek
Quoted 4 lines · Marek Vasut
I can determine it for this particular chip. However, when the vendor datasheet says the block is 64/32K it might mean that chips with this ID can have either block size.
http://www.chingistek.com/img/Product_Files/Pm25LD010020datasheet%20v0
4.p df page 21:
Quoted 8 lines
SECTOR_ER (20h) erases 4kByte sector.
BLOCK_ER (d8h) erases 64kByte sector.
http://www.gigadevice.com/product/download/366.html?locale=en_US
page 27-28:
Sector Erase (SE) (20h) erases 4kByte sector
64KB Block Erase (BE) (d8h) erases 64kByte sector
Quoted 7 lines
It's pretty much the same as the datasheet I used
http://www.elm-tech.com/en/products/spi-flash-memory/gd25q41/gd25q41.pdf
It mentions both
32KB Block Erase (BE) (52H)
and
64KB Block Erase (BE) (D8H)
The SPI NOR framework will use 0xbe opcode, no problem.
Quoted 3 lines
So the chip probably tries its best to be compatible with any command set and this last patch is not needed. The memory organization table on page 7 is not all that reassuring, though.

Quoted 6 lines

Quoted 1 line
Which exact part do you refer to please ?
Start of page 7 where it says sector size 32/64K in either datasheet.
It can refer to both BE opcode variants being supported but it's quite unclear.

My guess here would be that the internal organisation of the SPI NOR is in 4k blocks, which is no surprise really. My understanding is that opcode 0x52 erases 8x4k sector (ie. 32k of data) while 0xd8 erases 16x4k sector (ie. 64k of data). I don't see any problem here -- there are two different opcodes which do two different things and their behavior matches the one on various other SPI NORs.

Quoted 2 lines

Write protection seems to be calculated in 4k sectors and not blocks so the block size does not seem very relevant.

See above. Does it make sense now please ?

Best regards,
Marek Vasut

Michal Suchanek

author

2015-05-04 13:40 UTC #15 lore raw

Re: [PATCH 3/3] MTD: spi-nor: add flag to not use sector erase.

Quoted 29 lines · Marek Vasut

Quoted 14 lines · Michal Suchanek
Quoted 13 lines · Marek Vasut
Quoted 5 lines · Michal Suchanek
It mentions both
32KB Block Erase (BE) (52H)
and
64KB Block Erase (BE) (D8H)
The SPI NOR framework will use 0xbe opcode, no problem.
Quoted 3 lines
So the chip probably tries its best to be compatible with any command set and this last patch is not needed. The memory organization table on page 7 is not all that reassuring, though.
Quoted 6 lines
Quoted 1 line
Which exact part do you refer to please ?
Start of page 7 where it says sector size 32/64K in either datasheet.
It can refer to both BE opcode variants being supported but it's quite unclear.
My guess here would be that the internal organisation of the SPI NOR is in 4k blocks, which is no surprise really. My understanding is that opcode 0x52 erases 8x4k sector (ie. 32k of data) while 0xd8 erases 16x4k sector (ie. 64k of data). I don't see any problem here -- there are two different opcodes which do two different things and their behavior matches the one on various other SPI NORs.

Quoted 5 lines

Quoted 2 lines
Write protection seems to be calculated in 4k sectors and not blocks so the block size does not seem very relevant.
See above. Does it make sense now please ?

Yes,

makes sense.

Thanks

Michal

Marek Vasut

2015-05-04 14:11 UTC #16 lore raw

Re: [PATCH 3/3] MTD: spi-nor: add flag to not use sector erase.

Quoted 29 lines · Michal Suchanek

Quoted 28 lines · Marek Vasut
Quoted 13 lines · Michal Suchanek
Quoted 12 lines · Marek Vasut
Quoted 4 lines · Michal Suchanek
It mentions both
32KB Block Erase (BE) (52H)
and
64KB Block Erase (BE) (D8H)
The SPI NOR framework will use 0xbe opcode, no problem.
Quoted 3 lines
So the chip probably tries its best to be compatible with any command set and this last patch is not needed. The memory organization table on page 7 is not all that reassuring, though.
Quoted 6 lines
Quoted 1 line
Which exact part do you refer to please ?
Start of page 7 where it says sector size 32/64K in either datasheet.
It can refer to both BE opcode variants being supported but it's quite unclear.
My guess here would be that the internal organisation of the SPI NOR is in 4k blocks, which is no surprise really. My understanding is that opcode 0x52 erases 8x4k sector (ie. 32k of data) while 0xd8 erases 16x4k sector (ie. 64k of data). I don't see any problem here -- there are two different opcodes which do two different things and their behavior matches the one on various other SPI NORs.

Quoted 8 lines

Quoted 4 lines
Quoted 2 lines
Write protection seems to be calculated in 4k sectors and not blocks so the block size does not seem very relevant.
See above. Does it make sense now please ?
Yes,
makes sense.

I'm glad to hear this got cleared up, thanks ! :)

Best regards,
Marek Vasut

Rafał Miłecki

2015-05-01 21:56 UTC #17 lore raw

Re: [PATCH 3/3] MTD: spi-nor: add flag to not use sector erase.

In reply to Michal Suchanek

Quoted 5 lines · Michal Suchanek

The sector size of the flash memory is unclear from datasheet or may possibly vary between chips so add a flag to always use 4k blocks.
Currently 4k blocks are always used when possible but in the future somebody might want to do some optimizations with sector erase.

AFAIK most sources call 4K a sector and 64K a block. I think we should try to stick to that (even if some current code doesn't). So it would be nice to call 4K a sector (instead of block).