| Message ID | 1455041563-12483-1-git-send-email-nnazarenko@radiofid.com |
|---|---|
| State | Changes Requested |
| Headers | show |
+ Gabor, who submitted the other ISSI entry; and some others On Tue, Feb 09, 2016 at 09:12:43PM +0300, Nikita Nazarenko wrote: > Signed-off-by: Nikita Nazarenko <nnazarenko@radiofid.com> > --- > drivers/mtd/spi-nor/spi-nor.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c > index ed0c19c..e0bb151 100644 > --- a/drivers/mtd/spi-nor/spi-nor.c > +++ b/drivers/mtd/spi-nor/spi-nor.c > @@ -739,6 +739,7 @@ static const struct flash_info spi_nor_ids[] = { > > /* ISSI */ Both of these ISSI entries look wrong. > { "is25cd512", INFO(0x7f9d20, 0, 32 * 1024, 2, SECT_4K) }, Looking at the datasheet for this part [1], shouldn't the ID be 0x9d7f20 (i.e., the MFR ID is 0x9d, not 0x7f)? That would match the existing CFI definition for "PMC". And IIUC, PMC got bought by ISSI? Or some kind of merger, I don't follow these things. > + { "is25lp128", INFO(0x9d6018, 0, 32 * 1024, 512, SECT_4K) }, This datasheet [1] says that for SPI mode (not QPI mode), 4K erase is done with opcode 0xD7 (i.e., SPINOR_OP_BE_4K_PMC) not 0x20 (SPINOR_OP_BE_4K). So we would need the SECT_4K_PMC, not the SECT_4K flag. Also, the datasheet for this device says it supports 64K sectors, and the 32K sectors require a different erase command (SPINOR_OP_BE_32K; not currently supported in this driver). Did you test erase to be sure it worked as expected? Or are one or more datasheets wrong? Regards, Brian > /* Macronix */ > { "mx25l512e", INFO(0xc22010, 0, 64 * 1024, 1, SECT_4K) }, [1] I'm looking at these: http://www.issi.com/WW/pdf/25LP128.pdf http://www.issi.com/WW/pdf/25CD512-010-020.pdf
Hi Brian, > + Gabor, who submitted the other ISSI entry; and some others > > On Tue, Feb 09, 2016 at 09:12:43PM +0300, Nikita Nazarenko wrote: >> Signed-off-by: Nikita Nazarenko <nnazarenko@radiofid.com> >> --- >> drivers/mtd/spi-nor/spi-nor.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c >> index ed0c19c..e0bb151 100644 >> --- a/drivers/mtd/spi-nor/spi-nor.c >> +++ b/drivers/mtd/spi-nor/spi-nor.c >> @@ -739,6 +739,7 @@ static const struct flash_info spi_nor_ids[] = { >> >> /* ISSI */ > > Both of these ISSI entries look wrong. > >> { "is25cd512", INFO(0x7f9d20, 0, 32 * 1024, 2, SECT_4K) }, > > Looking at the datasheet for this part [1], shouldn't the ID be > 0x9d7f20 (i.e., the MFR ID is 0x9d, not 0x7f)? That would match the > existing CFI definition for "PMC". And IIUC, PMC got bought by ISSI? Or > some kind of merger, I don't follow these things. The ID is definitely 0x7d9d20, the m28p80 driver detects the device correctly with that: m25p80 spi0.0: found is25cd512, expected m25p80 m25p80 spi0.0: is25cd512 (64 Kbytes) Creating 4 MTD partitions on "spi0.0": 0x000000000000-0x00000000c000 : "routerboot" 0x00000000c000-0x00000000d000 : "hard_config" 0x00000000d000-0x00000000e000 : "bios" 0x00000000e000-0x00000000f000 : "soft_config" The first byte of the manufacturer ID is indeed 0x9d, but the device returns that as the second byte throught the JEDEC ID READ command. Even though it is a weird behaviour, it is described on page 11 of the datasheet. >> + { "is25lp128", INFO(0x9d6018, 0, 32 * 1024, 512, SECT_4K) }, > > This datasheet [1] says that for SPI mode (not QPI mode), 4K erase is > done with opcode 0xD7 (i.e., SPINOR_OP_BE_4K_PMC) not 0x20 > (SPINOR_OP_BE_4K). So we would need the SECT_4K_PMC, not the SECT_4K > flag. I don't have a board with such flash device, but Table 8.1 "Instruction Set" is misleading a bit. In my understanding, erasing 4K sectors should work with both commands. Look at Clause 8.10 "SECTOR ERASE OPERATION (SER, D7h/20h)" on page 33 in the datasheet. Both Figure 8.12 "Sector Erase Sequence" and Figure 8.13 "Sector Erase Sequence (QPI) " says 'D7h/20h'. > Also, the datasheet for this device says it supports 64K sectors, and > the 32K sectors require a different erase command (SPINOR_OP_BE_32K; not > currently supported in this driver). Did you test erase to be sure it > worked as expected? Or are one or more datasheets wrong? It seems that you are correct about this. -Gabor > > Regards, > Brian > >> /* Macronix */ >> { "mx25l512e", INFO(0xc22010, 0, 64 * 1024, 1, SECT_4K) }, > > [1] I'm looking at these: > > http://www.issi.com/WW/pdf/25LP128.pdf > http://www.issi.com/WW/pdf/25CD512-010-020.pdf >
Hi Gabor, On Tue, Mar 08, 2016 at 06:26:41PM +0100, Gabor Juhos wrote: > > On Tue, Feb 09, 2016 at 09:12:43PM +0300, Nikita Nazarenko wrote: > >> Signed-off-by: Nikita Nazarenko <nnazarenko@radiofid.com> > >> --- > >> drivers/mtd/spi-nor/spi-nor.c | 1 + > >> 1 file changed, 1 insertion(+) > >> > >> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c > >> index ed0c19c..e0bb151 100644 > >> --- a/drivers/mtd/spi-nor/spi-nor.c > >> +++ b/drivers/mtd/spi-nor/spi-nor.c > >> @@ -739,6 +739,7 @@ static const struct flash_info spi_nor_ids[] = { > >> > >> /* ISSI */ > > > > Both of these ISSI entries look wrong. > > > >> { "is25cd512", INFO(0x7f9d20, 0, 32 * 1024, 2, SECT_4K) }, > > > > Looking at the datasheet for this part [1], shouldn't the ID be > > 0x9d7f20 (i.e., the MFR ID is 0x9d, not 0x7f)? That would match the > > existing CFI definition for "PMC". And IIUC, PMC got bought by ISSI? Or > > some kind of merger, I don't follow these things. > > The ID is definitely 0x7d9d20, the m28p80 driver detects the device correctly > with that: > > m25p80 spi0.0: found is25cd512, expected m25p80 > m25p80 spi0.0: is25cd512 (64 Kbytes) > Creating 4 MTD partitions on "spi0.0": > 0x000000000000-0x00000000c000 : "routerboot" > 0x00000000c000-0x00000000d000 : "hard_config" > 0x00000000d000-0x00000000e000 : "bios" > 0x00000000e000-0x00000000f000 : "soft_config" > > The first byte of the manufacturer ID is indeed 0x9d, but the device returns > that as the second byte throught the JEDEC ID READ command. Even though it is a > weird behaviour, it is described on page 11 of the datasheet. What a stupid implementation. And they changed it to the sane implementation for is25lp128 it seems? > >> + { "is25lp128", INFO(0x9d6018, 0, 32 * 1024, 512, SECT_4K) }, > > > > This datasheet [1] says that for SPI mode (not QPI mode), 4K erase is > > done with opcode 0xD7 (i.e., SPINOR_OP_BE_4K_PMC) not 0x20 > > (SPINOR_OP_BE_4K). So we would need the SECT_4K_PMC, not the SECT_4K > > flag. > > I don't have a board with such flash device, but Table 8.1 "Instruction Set" is > misleading a bit. In my understanding, erasing 4K sectors should work with both > commands. Look at Clause 8.10 "SECTOR ERASE OPERATION (SER, D7h/20h)" on page 33 > in the datasheet. Both Figure 8.12 "Sector Erase Sequence" and Figure 8.13 > "Sector Erase Sequence (QPI) " says 'D7h/20h'. Ah, I could see how the table could be read either way. I expect that the submitter (Nikita) should be able to confirm this through testing. > > Also, the datasheet for this device says it supports 64K sectors, and > > the 32K sectors require a different erase command (SPINOR_OP_BE_32K; not > > currently supported in this driver). Did you test erase to be sure it > > worked as expected? Or are one or more datasheets wrong? > > It seems that you are correct about this. To be clear, it looks like your submission (is25cd512) does use D8h for 32K erase blocks, since it's such a tiny device. But Nikita's device appears to use 64K erase blocks. I expect Nikita can test and resubmit a revised entry here. Regards, Brian
Hi Brian, > On Tue, Mar 08, 2016 at 06:26:41PM +0100, Gabor Juhos wrote: >>> On Tue, Feb 09, 2016 at 09:12:43PM +0300, Nikita Nazarenko wrote: >>>> Signed-off-by: Nikita Nazarenko <nnazarenko@radiofid.com> >>>> --- >>>> drivers/mtd/spi-nor/spi-nor.c | 1 + >>>> 1 file changed, 1 insertion(+) >>>> >>>> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c >>>> index ed0c19c..e0bb151 100644 >>>> --- a/drivers/mtd/spi-nor/spi-nor.c >>>> +++ b/drivers/mtd/spi-nor/spi-nor.c >>>> @@ -739,6 +739,7 @@ static const struct flash_info spi_nor_ids[] = { >>>> >>>> /* ISSI */ >>> >>> Both of these ISSI entries look wrong. >>> >>>> { "is25cd512", INFO(0x7f9d20, 0, 32 * 1024, 2, SECT_4K) }, >>> >>> Looking at the datasheet for this part [1], shouldn't the ID be >>> 0x9d7f20 (i.e., the MFR ID is 0x9d, not 0x7f)? That would match the >>> existing CFI definition for "PMC". And IIUC, PMC got bought by ISSI? Or >>> some kind of merger, I don't follow these things. >> >> The ID is definitely 0x7d9d20, the m28p80 driver detects the device correctly >> with that: >> >> m25p80 spi0.0: found is25cd512, expected m25p80 >> m25p80 spi0.0: is25cd512 (64 Kbytes) >> Creating 4 MTD partitions on "spi0.0": >> 0x000000000000-0x00000000c000 : "routerboot" >> 0x00000000c000-0x00000000d000 : "hard_config" >> 0x00000000d000-0x00000000e000 : "bios" >> 0x00000000e000-0x00000000f000 : "soft_config" >> >> The first byte of the manufacturer ID is indeed 0x9d, but the device returns >> that as the second byte throught the JEDEC ID READ command. Even though it is a >> weird behaviour, it is described on page 11 of the datasheet. > > What a stupid implementation. Maybe it is not so stupid if they wanted to integrate the continuation id in that. And because 0x7f if not a valid manufacturer ID, it should not collide with any other manufacturer. > And they changed it to the sane implementation for is25lp128 it seems? Yes it seems that they have changed that, but i can't decide that it is better or not. >>>> + { "is25lp128", INFO(0x9d6018, 0, 32 * 1024, 512, SECT_4K) }, >>> >>> This datasheet [1] says that for SPI mode (not QPI mode), 4K erase is >>> done with opcode 0xD7 (i.e., SPINOR_OP_BE_4K_PMC) not 0x20 >>> (SPINOR_OP_BE_4K). So we would need the SECT_4K_PMC, not the SECT_4K >>> flag. >> >> I don't have a board with such flash device, but Table 8.1 "Instruction Set" is >> misleading a bit. In my understanding, erasing 4K sectors should work with both >> commands. Look at Clause 8.10 "SECTOR ERASE OPERATION (SER, D7h/20h)" on page 33 >> in the datasheet. Both Figure 8.12 "Sector Erase Sequence" and Figure 8.13 >> "Sector Erase Sequence (QPI) " says 'D7h/20h'. > > Ah, I could see how the table could be read either way. I expect that > the submitter (Nikita) should be able to confirm this through testing. > >>> Also, the datasheet for this device says it supports 64K sectors, and >>> the 32K sectors require a different erase command (SPINOR_OP_BE_32K; not >>> currently supported in this driver). Did you test erase to be sure it >>> worked as expected? Or are one or more datasheets wrong? >> >> It seems that you are correct about this. > > To be clear, it looks like your submission (is25cd512) does use D8h for > 32K erase blocks, since it's such a tiny device. Yes. > But Nikita's device appears to use 64K erase blocks. Yes, 64K blocks with the D8h opcode. -Gabor > I expect Nikita can test and resubmit a revised entry here. > > Regards, > Brian >
Hi Nikita and Gabor, Tested-by: Angelo Dureghello <angelo@sysam.it> On 09/02/2016 19:12, Nikita Nazarenko wrote: > Hi Gabor, > > On Tue, Mar 08, 2016 at 06:26:41PM +0100, Gabor Juhos wrote: >>> On Tue, Feb 09, 2016 at 09:12:43PM +0300, Nikita Nazarenko wrote: >>>> Signed-off-by: Nikita Nazarenko <nnazarenko at radiofid.com> >>>> --- >>>> drivers/mtd/spi-nor/spi-nor.c | 1 + >>>> 1 file changed, 1 insertion(+) >>>> >>>> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c >>>> index ed0c19c..e0bb151 100644 >>>> --- a/drivers/mtd/spi-nor/spi-nor.c >>>> +++ b/drivers/mtd/spi-nor/spi-nor.c >>>> @@ -739,6 +739,7 @@ static const struct flash_info spi_nor_ids[] = { >>>> >>>> /* ISSI */ >>> Both of these ISSI entries look wrong. >>> >>>> { "is25cd512", INFO(0x7f9d20, 0, 32 * 1024, 2, SECT_4K) }, >>> Looking at the datasheet for this part [1], shouldn't the ID be >>> 0x9d7f20 (i.e., the MFR ID is 0x9d, not 0x7f)? That would match the >>> existing CFI definition for "PMC". And IIUC, PMC got bought by ISSI? Or >>> some kind of merger, I don't follow these things. >> The ID is definitely 0x7d9d20, the m28p80 driver detects the device correctly >> with that: >> >> m25p80 spi0.0: found is25cd512, expected m25p80 >> m25p80 spi0.0: is25cd512 (64 Kbytes) >> Creating 4 MTD partitions on "spi0.0": >> 0x000000000000-0x00000000c000 : "routerboot" >> 0x00000000c000-0x00000000d000 : "hard_config" >> 0x00000000d000-0x00000000e000 : "bios" >> 0x00000000e000-0x00000000f000 : "soft_config" >> >> The first byte of the manufacturer ID is indeed 0x9d, but the device returns >> that as the second byte throught the JEDEC ID READ command. Even though it is a >> weird behaviour, it is described on page 11 of the datasheet. > What a stupid implementation. And they changed it to the sane > implementation for is25lp128 it seems? > >>>> + { "is25lp128", INFO(0x9d6018, 0, 32 * 1024, 512, SECT_4K) }, >>> This datasheet [1] says that for SPI mode (not QPI mode), 4K erase is >>> done with opcode 0xD7 (i.e., SPINOR_OP_BE_4K_PMC) not 0x20 >>> (SPINOR_OP_BE_4K). So we would need the SECT_4K_PMC, not the SECT_4K >>> flag. >> I don't have a board with such flash device, but Table 8.1 "Instruction Set" is >> misleading a bit. In my understanding, erasing 4K sectors should work with both >> commands. Look at Clause 8.10 "SECTOR ERASE OPERATION (SER, D7h/20h)" on page 33 >> in the datasheet. Both Figure 8.12 "Sector Erase Sequence" and Figure 8.13 >> "Sector Erase Sequence (QPI) " says 'D7h/20h'. > Ah, I could see how the table could be read either way. I expect that > the submitter (Nikita) should be able to confirm this through testing. > it happen i mounted the same spi nor (is25lp128) here on a custom Coldfire board running Linux, so i appliedthe line { "is25lp128", INFO(0x9d6018, 0, 32 * 1024, 512, SECT_4K) }, and preformed some testing, the 4k erase works. [ 8.260000] Creating 3 MTD partitions on "is25lp128": [ 8.260000] 0x000000000000-0x000000100000 : "U-Boot (1024K)" [ 8.330000] 0x000000100000-0x000000800000 : "Kernel+initramfs (7168K)" [ 8.400000] 0x000000800000-0x000001000000 : "Flash Free Space (8192K)" /bin # cat /proc/mtd dev: size erasesize name mtd0: 00100000 00001000 "U-Boot (1024K)" mtd1: 00700000 00001000 "Kernel+initramfs (7168K)" mtd2: 00800000 00001000 "Flash Free Space (8192K)" # flash_eraseall /dev/mtd2 / # hexdump -C -n 8192 /dev/mtd2 00000000 ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff |................| * 00002000 / # / # dd if=/dev/zero of=/dev/mtd2 bs=4096 count=1 1+0 records in 1+0 records out 4096 bytes (4.0KB) copied, 0.228499 seconds, 17.5KB/s / # hexdump -C -n 8192 /dev/mtd2 00000000 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................| * 00001000 ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff |................| * 00002000 / # / # flash_erase /dev/mtd2 0 1 Erasing 4 Kibyte @ 0 -- 100 % complete / # / # hexdump -C -n 8192 /dev/mtd2 00000000 ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff |................| * 00002000 / # >>> Also, the datasheet for this device says it supports 64K sectors, and >>> the 32K sectors require a different erase command (SPINOR_OP_BE_32K; not >>> currently supported in this driver). Did you test erase to be sure it >>> worked as expected? Or are one or more datasheets wrong? >> It seems that you are correct about this. > To be clear, it looks like your submission (is25cd512) does use D8h for > 32K erase blocks, since it's such a tiny device. But Nikita's device > appears to use 64K erase blocks. > > I expect Nikita can test and resubmit a revised entry here. > > Regards, > Brian Regards, Angelo Dureghello
Hi Nikita, On 18/10/2017 12:33, Nikita Nazarenko wrote: > Hello Angelo. > > I almost forgot about that patch, and in my tree it little different: > > + { "is25lp128", INFO(0x9d6018, 0, 64 * 1024, 256, 0) }, > > I don't remember on which board that flash were used and why i did that change, so i can't test it again properly. > > Maybe that will help you. > > many thanks for clarifying, ok, looks like 64K granularity should be better in any case for this chip. Also, this driver uses SPINOR_OP_SE <https://elixir.free-electrons.com/linux/latest/ident/SPINOR_OP_SE> (0xd8) so the size should be set to 64k. The SECT_4K can be used since the flash accepts it. Clause 8.10 "SECTOR ERASE OPERATION (SER, D7h/20h)", so both commands are valid. This is my last tuning: { "is25lp128", INFO(0x9d6018, 0, 64 * 1024, 256, SECT_4K) }, Did some more tests on this entry above: TEST 1) With MTD_SPI_NOR_USE_4K_SECTORS set: / # flash_eraseall /dev/mtd2 flash_eraseall has been replaced by `flash_erase <mtddev> 0 0`; please use it Erasing 4 Kibyte @ 7ff000 -- 100 % complete Checked all the flash has been properly erased: TEST 2) Removed MTD_SPI_NOR_USE_4K_SECTORS set: I written 2 blocks full of 0, at begin and at the end of the 8KB partition: / # hexdump -C -n 8192 /dev/mtd2 00000000 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................| * 00001000 ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff |................| * 00002000 / # hexdump -C -n 8192 -s 8380416 /dev/mtd2 007fe000 ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff |................| * 007ff000 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................| * 00800000 / # Doing now flash erase at 64K blocks: / # flash_eraseall /dev/mtd2 flash_eraseall has been replaced by `flash_erase <mtddev> 0 0`; please use it Erasing 64 Kibyte @ 7f0000 -- 100 % complete All erased properly. / # hexdump -C -n 8192 /dev/mtd2 00000000 ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff |................| * 00002000 / # hexdump -C -n 8192 -s 8380416 /dev/mtd2 007fe000 ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff |................| * 00800000 / # So the new line above is well working for me. Let me know how we could proceed, i am very interested to have this chip supported into mainline, mainly for my open hw called stmark2. Do you want to send the patch ? Or, if it's not a problem for you, i can send it, or maybe i can be able to keep your name and adding a tested-by from me. Many thanks, Best regards, Angelo Dureghello > Best regards > > Nikita Nazarenko > > > On 18.10.2017 02:14, angelo wrote: >> Hi Nikita and Gabor, >> >> Tested-by: Angelo Dureghello <angelo@sysam.it> >> >> >> On 09/02/2016 19:12, Nikita Nazarenko wrote: >>> Hi Gabor, >>> >>> On Tue, Mar 08, 2016 at 06:26:41PM +0100, Gabor Juhos wrote: >>>>> On Tue, Feb 09, 2016 at 09:12:43PM +0300, Nikita Nazarenko wrote: >>>>>> Signed-off-by: Nikita Nazarenko <nnazarenko at radiofid.com> >>>>>> --- >>>>>> drivers/mtd/spi-nor/spi-nor.c | 1 + >>>>>> 1 file changed, 1 insertion(+) >>>>>> >>>>>> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c >>>>>> index ed0c19c..e0bb151 100644 >>>>>> --- a/drivers/mtd/spi-nor/spi-nor.c >>>>>> +++ b/drivers/mtd/spi-nor/spi-nor.c >>>>>> @@ -739,6 +739,7 @@ static const struct flash_info spi_nor_ids[] = { >>>>>> >>>>>> /* ISSI */ >>>>> Both of these ISSI entries look wrong. >>>>> >>>>>> { "is25cd512", INFO(0x7f9d20, 0, 32 * 1024, 2, SECT_4K) }, >>>>> Looking at the datasheet for this part [1], shouldn't the ID be >>>>> 0x9d7f20 (i.e., the MFR ID is 0x9d, not 0x7f)? That would match the >>>>> existing CFI definition for "PMC". And IIUC, PMC got bought by ISSI? Or >>>>> some kind of merger, I don't follow these things. >>>> The ID is definitely 0x7d9d20, the m28p80 driver detects the device correctly >>>> with that: >>>> >>>> m25p80 spi0.0: found is25cd512, expected m25p80 >>>> m25p80 spi0.0: is25cd512 (64 Kbytes) >>>> Creating 4 MTD partitions on "spi0.0": >>>> 0x000000000000-0x00000000c000 : "routerboot" >>>> 0x00000000c000-0x00000000d000 : "hard_config" >>>> 0x00000000d000-0x00000000e000 : "bios" >>>> 0x00000000e000-0x00000000f000 : "soft_config" >>>> >>>> The first byte of the manufacturer ID is indeed 0x9d, but the device returns >>>> that as the second byte throught the JEDEC ID READ command. Even though it is a >>>> weird behaviour, it is described on page 11 of the datasheet. >>> What a stupid implementation. And they changed it to the sane >>> implementation for is25lp128 it seems? >>> >>>>>> + { "is25lp128", INFO(0x9d6018, 0, 32 * 1024, 512, SECT_4K) }, >>>>> This datasheet [1] says that for SPI mode (not QPI mode), 4K erase is >>>>> done with opcode 0xD7 (i.e., SPINOR_OP_BE_4K_PMC) not 0x20 >>>>> (SPINOR_OP_BE_4K). So we would need the SECT_4K_PMC, not the SECT_4K >>>>> flag. >>>> I don't have a board with such flash device, but Table 8.1 "Instruction Set" is >>>> misleading a bit. In my understanding, erasing 4K sectors should work with both >>>> commands. Look at Clause 8.10 "SECTOR ERASE OPERATION (SER, D7h/20h)" on page 33 >>>> in the datasheet. Both Figure 8.12 "Sector Erase Sequence" and Figure 8.13 >>>> "Sector Erase Sequence (QPI) " says 'D7h/20h'. >>> Ah, I could see how the table could be read either way. I expect that >>> the submitter (Nikita) should be able to confirm this through testing. >>> >> it happen i mounted the same spi nor (is25lp128) here on a custom Coldfire board running Linux, >> so i appliedthe line >> >> { "is25lp128", INFO(0x9d6018, 0, 32 * 1024, 512, SECT_4K) }, >> >> and preformed some testing, the 4k erase works. >> >> [ 8.260000] Creating 3 MTD partitions on "is25lp128": >> [ 8.260000] 0x000000000000-0x000000100000 : "U-Boot (1024K)" >> [ 8.330000] 0x000000100000-0x000000800000 : "Kernel+initramfs (7168K)" >> [ 8.400000] 0x000000800000-0x000001000000 : "Flash Free Space (8192K)" >> >> >> /bin # cat /proc/mtd >> dev: size erasesize name >> mtd0: 00100000 00001000 "U-Boot (1024K)" >> mtd1: 00700000 00001000 "Kernel+initramfs (7168K)" >> mtd2: 00800000 00001000 "Flash Free Space (8192K)" >> >> # flash_eraseall /dev/mtd2 >> >> / # hexdump -C -n 8192 /dev/mtd2 >> 00000000 ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff |................| >> * >> 00002000 >> / # >> >> / # dd if=/dev/zero of=/dev/mtd2 bs=4096 count=1 >> 1+0 records in >> 1+0 records out >> 4096 bytes (4.0KB) copied, 0.228499 seconds, 17.5KB/s >> >> >> / # hexdump -C -n 8192 /dev/mtd2 >> 00000000 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................| >> * >> 00001000 ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff |................| >> * >> 00002000 >> / # >> >> >> / # flash_erase /dev/mtd2 0 1 >> Erasing 4 Kibyte @ 0 -- 100 % complete >> / # >> >> / # hexdump -C -n 8192 /dev/mtd2 >> 00000000 ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff |................| >> * >> 00002000 >> / # >> >> >> >>>>> Also, the datasheet for this device says it supports 64K sectors, and >>>>> the 32K sectors require a different erase command (SPINOR_OP_BE_32K; not >>>>> currently supported in this driver). Did you test erase to be sure it >>>>> worked as expected? Or are one or more datasheets wrong? >>>> It seems that you are correct about this. >>> To be clear, it looks like your submission (is25cd512) does use D8h for >>> 32K erase blocks, since it's such a tiny device. But Nikita's device >>> appears to use 64K erase blocks. >>> >>> I expect Nikita can test and resubmit a revised entry here. >>> >>> Regards, >>> Brian >> Regards, >> Angelo Dureghello >
Hi Nikita, many thanks, i try. On 19/10/2017 15:12, Nikita Nazarenko wrote: > Hello Angelo > > >> So the new line above is well working for me. Let me know how we >> could proceed, i am very interested to have this chip supported into >> mainline, mainly for my open hw called stmark2. >> Do you want to send the patch ? Or, if it's not a problem for you, i can >> send it, or maybe i can be able to keep your name and adding a tested-by >> from me. >> > You can send that patch from yourself, even without mentioning me. you did most of work. > > Best regards > Nikita Nazarenko Best regards Angelo Durghello
diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c index ed0c19c..e0bb151 100644 --- a/drivers/mtd/spi-nor/spi-nor.c +++ b/drivers/mtd/spi-nor/spi-nor.c @@ -739,6 +739,7 @@ static const struct flash_info spi_nor_ids[] = { /* ISSI */ { "is25cd512", INFO(0x7f9d20, 0, 32 * 1024, 2, SECT_4K) }, + { "is25lp128", INFO(0x9d6018, 0, 32 * 1024, 512, SECT_4K) }, /* Macronix */ { "mx25l512e", INFO(0xc22010, 0, 64 * 1024, 1, SECT_4K) },
Signed-off-by: Nikita Nazarenko <nnazarenko@radiofid.com> --- drivers/mtd/spi-nor/spi-nor.c | 1 + 1 file changed, 1 insertion(+)