| Message ID | 35add8df33b17e2354d9496eba8597d9d8488f30.1430430153.git.hramrach@gmail.com |
|---|---|
| State | Rejected |
| Headers | show |
On Thursday, April 30, 2015 at 11:13:12 PM, Michal Suchanek wrote: > 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 <hramrach@gmail.com> 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
On 1 May 2015 at 01:13, Marek Vasut <marex@denx.de> wrote: > On Thursday, April 30, 2015 at 11:13:12 PM, Michal Suchanek wrote: >> 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 <hramrach@gmail.com> > > 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. > 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
Hi, On Fri, May 1, 2015 at 9:05 AM, Michal Suchanek <hramrach@gmail.com> wrote: > On 1 May 2015 at 01:13, Marek Vasut <marex@denx.de> wrote: >> On Thursday, April 30, 2015 at 11:13:12 PM, Michal Suchanek wrote: >>> 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 <hramrach@gmail.com> >> >> 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. >> > > 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
On Friday, May 01, 2015 at 09:05:15 AM, Michal Suchanek wrote: > On 1 May 2015 at 01:13, Marek Vasut <marex@denx.de> wrote: > > On Thursday, April 30, 2015 at 11:13:12 PM, Michal Suchanek wrote: > >> 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 <hramrach@gmail.com> > > > > 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. > > 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 > 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
On 30 April 2015 at 23:13, Michal Suchanek <hramrach@gmail.com> wrote: > 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).
Hello, On 1 May 2015 at 16:20, Marek Vasut <marex@denx.de> wrote: > On Friday, May 01, 2015 at 09:05:15 AM, Michal Suchanek wrote: >> On 1 May 2015 at 01:13, Marek Vasut <marex@denx.de> 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: > > 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
On Monday, May 04, 2015 at 01:11:03 PM, Michal Suchanek wrote: > Hello, Hi! > On 1 May 2015 at 16:20, Marek Vasut <marex@denx.de> wrote: > > On Friday, May 01, 2015 at 09:05:15 AM, Michal Suchanek wrote: > >> On 1 May 2015 at 01:13, Marek Vasut <marex@denx.de> 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.p > > df 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 > > 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. > 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
On 4 May 2015 at 14:12, Marek Vasut <marex@denx.de> wrote: > On Monday, May 04, 2015 at 01:11:03 PM, Michal Suchanek wrote: >> Hello, > > Hi! > >> On 1 May 2015 at 16:20, Marek Vasut <marex@denx.de> wrote: >> > On Friday, May 01, 2015 at 09:05:15 AM, Michal Suchanek wrote: >> >> On 1 May 2015 at 01:13, Marek Vasut <marex@denx.de> 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.p >> > df 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 >> >> 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. > >> 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
On Monday, May 04, 2015 at 03:18:56 PM, Michal Suchanek wrote: > On 4 May 2015 at 14:12, Marek Vasut <marex@denx.de> wrote: > > On Monday, May 04, 2015 at 01:11:03 PM, Michal Suchanek wrote: > >> Hello, > > > > Hi! > > > >> On 1 May 2015 at 16:20, Marek Vasut <marex@denx.de> wrote: > >> > On Friday, May 01, 2015 at 09:05:15 AM, Michal Suchanek wrote: > >> >> On 1 May 2015 at 01:13, Marek Vasut <marex@denx.de> 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%20v0 > >> > 4.p df 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 > >> > >> 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. > > > >> 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. 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. > 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
On 4 May 2015 at 15:35, Marek Vasut <marex@denx.de> wrote: > On Monday, May 04, 2015 at 03:18:56 PM, Michal Suchanek wrote: >> On 4 May 2015 at 14:12, Marek Vasut <marex@denx.de> wrote: >> > On Monday, May 04, 2015 at 01:11:03 PM, Michal Suchanek wrote: >> >> >> >> It mentions both >> >> 32KB Block Erase (BE) (52H) >> >> and >> >> 64KB Block Erase (BE) (D8H) >> > >> > The SPI NOR framework will use 0xbe opcode, no problem. >> > >> >> 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. > > 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. > >> 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
On Monday, May 04, 2015 at 03:39:44 PM, Michal Suchanek wrote: > On 4 May 2015 at 15:35, Marek Vasut <marex@denx.de> wrote: > > On Monday, May 04, 2015 at 03:18:56 PM, Michal Suchanek wrote: > >> On 4 May 2015 at 14:12, Marek Vasut <marex@denx.de> wrote: > >> > On Monday, May 04, 2015 at 01:11:03 PM, Michal Suchanek wrote: > >> >> It mentions both > >> >> 32KB Block Erase (BE) (52H) > >> >> and > >> >> 64KB Block Erase (BE) (D8H) > >> > > >> > The SPI NOR framework will use 0xbe opcode, no problem. > >> > > >> >> 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. > > > > 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. > > > >> 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
diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c index de73cf5..371e91c 100644 --- a/drivers/mtd/spi-nor/spi-nor.c +++ b/drivers/mtd/spi-nor/spi-nor.c @@ -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) },
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 <hramrach@gmail.com> --- drivers/mtd/spi-nor/spi-nor.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)