| Message ID | 6bf927e513554e628fe15d309aac698e@BLUPR07MB002.namprd07.prod.outlook.com |
|---|---|
| State | Superseded |
| Headers | show |
+ Marek Hi Insop, On Mon, Jan 06, 2014 at 05:21:17AM +0000, Insop Song wrote: > In order to use Micron n25q512a, MTD, two changes are required as follows: > - jedec code should be fixed I have a feeling there are more than one "n25q512a" device, with different IDs. > - flag status should be read for writing. > > Check flag status register for Micron n25q512a > - Programing Micron n25q512a requires to check flag status register > - According to datasheet > " > The flag status register must be read any time a PROGRAM, ERASE, > SUSPEND/RESUME command is issued, or after a RESET command while device > is busy. The cycle is not complete until bit 7 of the flag status > register output 1. > " > - Ref: https://www.micron.com/~/media/Documents/Products/Data%20Sheet/NOR%20Flash/Serial%20NOR/N25Q/n25q_512mb_1ce_3v_65nm.pdf Hmm, are you sure that all Micron n25q512a need to check the flag status register? I'll check my datasheets when I'm back in the office, but this seems peculiar. > Signed-off-by: Insop Song <insop.song@gainspeed.com> > --- > drivers/mtd/devices/m25p80.c | 91 +++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 90 insertions(+), 1 deletion(-) > > diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c > index 7eda71d..e53e522 100644 > --- a/drivers/mtd/devices/m25p80.c > +++ b/drivers/mtd/devices/m25p80.c > @@ -38,6 +38,7 @@ > /* Flash opcodes. */ > #define OPCODE_WREN 0x06 /* Write enable */ > #define OPCODE_RDSR 0x05 /* Read status register */ > +#define OPCODE_RDFSR 0x70 /* Read flag status register */ > #define OPCODE_WRSR 0x01 /* Write status register 1 byte */ > #define OPCODE_NORM_READ 0x03 /* Read data bytes (low frequency) */ > #define OPCODE_FAST_READ 0x0b /* Read data bytes (high frequency) */ > @@ -70,6 +71,8 @@ > /* Status Register bits. */ > #define SR_WIP 1 /* Write in progress */ > #define SR_WEL 2 /* Write enable latch */ > +/* Flag Status Register bits. */ > +#define FSR_RDY 0x80 /* FSR ready */ > /* meaning of other SR_* bits may differ between vendors */ > #define SR_BP0 4 /* Block protect 0 */ > #define SR_BP1 8 /* Block protect 1 */ > @@ -95,6 +98,7 @@ struct m25p { > u8 program_opcode; > u8 *command; > bool fast_read; > + bool flag_status; > }; > > static inline struct m25p *mtd_to_m25p(struct mtd_info *mtd) > @@ -131,6 +135,28 @@ static int read_sr(struct m25p *flash) > } > > /* > + * Read the flag status register, returning its value in the location > + * Return the status register value. > + * Returns negative if error occurred. > + */ > +static int read_fsr(struct m25p *flash) > +{ > + ssize_t retval; > + u8 code = OPCODE_RDFSR; > + u8 val; > + > + retval = spi_write_then_read(flash->spi, &code, 1, &val, 1); > + > + if (retval < 0) { > + printk(&flash->spi->dev, "error %d reading FSR\n", > + (int) retval); > + return retval; > + } > + > + return val; > +} > + > +/* > * Write status register 1 byte > * Returns negative if error occurred. > */ > @@ -220,6 +246,30 @@ static int wait_till_ready(struct m25p *flash) > } > > /* > + * Service routine to read flag status register until ready, or timeout occurs. > + * Returns non-zero if error. > + */ > +static int wait_till_fsr_ready(struct m25p *flash) > +{ > + unsigned long deadline; > + int sr; > + > + deadline = jiffies + MAX_READY_WAIT_JIFFIES; > + > + do { > + if ((sr = read_fsr(flash)) < 0) > + break; > + else if ((sr & FSR_RDY)) > + return 0; > + > + cond_resched(); > + > + } while (!time_after_eq(jiffies, deadline)); > + > + return 1; > +} > + > +/* > * Erase the whole flash memory > * > * Returns 0 if successful, non-zero otherwise. > @@ -273,6 +323,14 @@ static int erase_sector(struct m25p *flash, u32 offset) > if (wait_till_ready(flash)) > return 1; > > + /* Flag status, Wait until finished previous write command. */ > + if (flash->flag_status == true) { > + if (wait_till_fsr_ready(flash)) { > + mutex_unlock(&flash->lock); > + return 1; > + } > + } > + > /* Send write enable, then erase commands. */ > write_enable(flash); > > @@ -427,6 +485,14 @@ static int m25p80_write(struct mtd_info *mtd, loff_t to, size_t len, > > mutex_lock(&flash->lock); > > + /* Flag status, Wait until finished previous write command. */ > + if (flash->flag_status == true) { > + if (wait_till_fsr_ready(flash)) { > + mutex_unlock(&flash->lock); > + return 1; > + } > + } > + > /* Wait until finished previous write command. */ > if (wait_till_ready(flash)) { > mutex_unlock(&flash->lock); > @@ -457,6 +523,14 @@ static int m25p80_write(struct mtd_info *mtd, loff_t to, size_t len, > t[1].len = page_size; > spi_sync(flash->spi, &m); > > + /* Flag status, Wait until finished previous write command. */ > + if (flash->flag_status == true) { > + if (wait_till_fsr_ready(flash)) { > + mutex_unlock(&flash->lock); > + return 1; > + } > + } > + > *retlen = m.actual_length - m25p_cmdsz(flash); > > /* write everything in flash->page_size chunks */ > @@ -477,6 +551,14 @@ static int m25p80_write(struct mtd_info *mtd, loff_t to, size_t len, > > spi_sync(flash->spi, &m); > > + /* Flag status, Wait until finished previous write command. */ > + if (flash->flag_status == true) { > + if (wait_till_fsr_ready(flash)) { > + mutex_unlock(&flash->lock); > + return 1; > + } > + } > + > *retlen += m.actual_length - m25p_cmdsz(flash); > } > } > @@ -782,7 +864,7 @@ static const struct spi_device_id m25p_ids[] = { > { "n25q128a11", INFO(0x20bb18, 0, 64 * 1024, 256, 0) }, > { "n25q128a13", INFO(0x20ba18, 0, 64 * 1024, 256, 0) }, > { "n25q256a", INFO(0x20ba19, 0, 64 * 1024, 512, SECT_4K) }, > - { "n25q512a", INFO(0x20bb20, 0, 64 * 1024, 1024, SECT_4K) }, > + { "n25q512a", INFO(0x20ba20, 0, 64 * 1024, 1024, SECT_4K) }, You probably want to figure out the distinction between the old table entry and the new one, and assign your new entry a new string accordingly. > > /* PMC */ > { "pm25lv512", INFO(0, 0, 32 * 1024, 2, SECT_4K_PMC) }, > @@ -996,6 +1078,13 @@ static int m25p_probe(struct spi_device *spi) > spi_set_drvdata(spi, flash); > > /* > + * Micron n25q512a requires to check flag status register > + */ > + flash->flag_status = false; > + if (strcmp(id->name, "n25q512a") == 0) > + flash->flag_status = true;; This doesn't look good at all. If any other flash start to need this, we don't want to code each ID string here. That's fragile and non-scaleable. If we need this option, you need to add another flag to the m25p_ids[] table, I guess... > + > + /* > * Atmel, SST and Intel/Numonyx serial flash tend to power > * up with the software protection bits set > */ Brian
On Thursday, February 27, 2014 at 08:33:14 AM, Brian Norris wrote: > + Marek Thanks, I really need to subscribe to this ML ;-/ > Hi Insop, > > On Mon, Jan 06, 2014 at 05:21:17AM +0000, Insop Song wrote: > > In order to use Micron n25q512a, MTD, two changes are required as > > follows: - jedec code should be fixed > > I have a feeling there are more than one "n25q512a" device, with > different IDs. ACK, I have similar feeling. Jain, can you tell us the precise marking on your N25Q512A chip (that is, every single letter on the top of the chip package exactly as it is engraved there ) please? Or make a photo ... Insop, can you please do the same ? [...] > > @@ -782,7 +864,7 @@ static const struct spi_device_id m25p_ids[] = { > > > > { "n25q128a11", INFO(0x20bb18, 0, 64 * 1024, 256, 0) }, > > { "n25q128a13", INFO(0x20ba18, 0, 64 * 1024, 256, 0) }, > > { "n25q256a", INFO(0x20ba19, 0, 64 * 1024, 512, SECT_4K) }, > > > > - { "n25q512a", INFO(0x20bb20, 0, 64 * 1024, 1024, SECT_4K) }, > > + { "n25q512a", INFO(0x20ba20, 0, 64 * 1024, 1024, SECT_4K) }, > > You probably want to figure out the distinction between the old table > entry and the new one, and assign your new entry a new string > accordingly. ACK. The datasheet actually claims this change is correct, see [1] (page 40, table 21), but as we know, Micron really does shitty job when it comes to using the JEDEC IDs for it's chips. > > /* PMC */ > > { "pm25lv512", INFO(0, 0, 32 * 1024, 2, SECT_4K_PMC) }, > > > > @@ -996,6 +1078,13 @@ static int m25p_probe(struct spi_device *spi) > > > > spi_set_drvdata(spi, flash); > > > > /* > > > > + * Micron n25q512a requires to check flag status register > > + */ > > + flash->flag_status = false; > > + if (strcmp(id->name, "n25q512a") == 0) > > + flash->flag_status = true;; > > This doesn't look good at all. If any other flash start to need this, we > don't want to code each ID string here. That's fragile and > non-scaleable. If we need this option, you need to add another flag to > the m25p_ids[] table, I guess... This waiting for some bit in FSR looks like it can be wrapped into wait_till_ready(), no ? What I cannot find in the datasheet though is any evidence which would clearly mandate the use of FSR bit 0x80 to test if the chip is ready for next operation instead of using the regular STATUS register bit . Insop, can you please elaborate on why using the FSR is needed ? [1] https://www.micron.com/~/media/Documents/Products/Data%20Sheet/NOR%20Flash/Serial%20NOR/N25Q/n25q_512mb_1ce_3v_65nm.pdf
Hi Brian, Thank you for your feedback. > From: Brian Norris [mailto:computersforpeace@gmail.com] > Sent: Wednesday, February 26, 2014 11:33 PM> > > On Mon, Jan 06, 2014 at 05:21:17AM +0000, Insop Song wrote: > > In order to use Micron n25q512a, MTD, two changes are required as > follows: > > - jedec code should be fixed > > I have a feeling there are more than one "n25q512a" device, with different > IDs. > > > - flag status should be read for writing. > > > > Check flag status register for Micron n25q512a > > - Programing Micron n25q512a requires to check flag status register > > - According to datasheet > > " > > The flag status register must be read any time a PROGRAM, ERASE, > > SUSPEND/RESUME command is issued, or after a RESET command > while device > > is busy. The cycle is not complete until bit 7 of the flag status > > register output 1. > > " > > - Ref: > > > https://www.micron.com/~/media/Documents/Products/Data%20Sheet/N > OR%20F > > lash/Serial%20NOR/N25Q/n25q_512mb_1ce_3v_65nm.pdf > > Hmm, are you sure that all Micron n25q512a need to check the flag status > register? I'll check my datasheets when I'm back in the office, but this seems > peculiar. > "Flag status" term can be found in 84 time in the data sheet and I don't think the flag status is specific to particular type of n25q512a. > > Signed-off-by: Insop Song <insop.song@gainspeed.com> > > --- > > drivers/mtd/devices/m25p80.c | 91 > +++++++++++++++++++++++++++++++++++++++++- > > 1 file changed, 90 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/mtd/devices/m25p80.c > > b/drivers/mtd/devices/m25p80.c index 7eda71d..e53e522 100644 > > --- a/drivers/mtd/devices/m25p80.c > > +++ b/drivers/mtd/devices/m25p80.c > > @@ -38,6 +38,7 @@ > > /* Flash opcodes. */ > > #define OPCODE_WREN 0x06 /* Write enable */ > > #define OPCODE_RDSR 0x05 /* Read status register */ > > +#define OPCODE_RDFSR 0x70 /* Read flag status > register */ > > #define OPCODE_WRSR 0x01 /* Write status > register 1 byte */ > > #define OPCODE_NORM_READ 0x03 /* Read data bytes (low > frequency) */ > > #define OPCODE_FAST_READ 0x0b /* Read data bytes (high > frequency) */ > > @@ -70,6 +71,8 @@ > > /* Status Register bits. */ > > #define SR_WIP 1 /* Write in progress > */ > > #define SR_WEL 2 /* Write enable latch > */ > > +/* Flag Status Register bits. */ > > +#define FSR_RDY 0x80 /* FSR ready */ > > /* meaning of other SR_* bits may differ between vendors */ > > #define SR_BP0 4 /* Block protect 0 */ > > #define SR_BP1 8 /* Block protect 1 */ > > @@ -95,6 +98,7 @@ struct m25p { > > u8 program_opcode; > > u8 *command; > > bool fast_read; > > + bool flag_status; > > }; > > > > static inline struct m25p *mtd_to_m25p(struct mtd_info *mtd) @@ > > -131,6 +135,28 @@ static int read_sr(struct m25p *flash) } > > > > /* > > + * Read the flag status register, returning its value in the location > > + * Return the status register value. > > + * Returns negative if error occurred. > > + */ > > +static int read_fsr(struct m25p *flash) { > > + ssize_t retval; > > + u8 code = OPCODE_RDFSR; > > + u8 val; > > + > > + retval = spi_write_then_read(flash->spi, &code, 1, &val, 1); > > + > > + if (retval < 0) { > > + printk(&flash->spi->dev, "error %d reading FSR\n", > > + (int) retval); > > + return retval; > > + } > > + > > + return val; > > +} > > + > > +/* > > * Write status register 1 byte > > * Returns negative if error occurred. > > */ > > @@ -220,6 +246,30 @@ static int wait_till_ready(struct m25p *flash) } > > > > /* > > + * Service routine to read flag status register until ready, or timeout > occurs. > > + * Returns non-zero if error. > > + */ > > +static int wait_till_fsr_ready(struct m25p *flash) { > > + unsigned long deadline; > > + int sr; > > + > > + deadline = jiffies + MAX_READY_WAIT_JIFFIES; > > + > > + do { > > + if ((sr = read_fsr(flash)) < 0) > > + break; > > + else if ((sr & FSR_RDY)) > > + return 0; > > + > > + cond_resched(); > > + > > + } while (!time_after_eq(jiffies, deadline)); > > + > > + return 1; > > +} > > + > > +/* > > * Erase the whole flash memory > > * > > * Returns 0 if successful, non-zero otherwise. > > @@ -273,6 +323,14 @@ static int erase_sector(struct m25p *flash, u32 > offset) > > if (wait_till_ready(flash)) > > return 1; > > > > + /* Flag status, Wait until finished previous write command. */ > > + if (flash->flag_status == true) { > > + if (wait_till_fsr_ready(flash)) { > > + mutex_unlock(&flash->lock); > > + return 1; > > + } > > + } > > + > > /* Send write enable, then erase commands. */ > > write_enable(flash); > > > > @@ -427,6 +485,14 @@ static int m25p80_write(struct mtd_info *mtd, > > loff_t to, size_t len, > > > > mutex_lock(&flash->lock); > > > > + /* Flag status, Wait until finished previous write command. */ > > + if (flash->flag_status == true) { > > + if (wait_till_fsr_ready(flash)) { > > + mutex_unlock(&flash->lock); > > + return 1; > > + } > > + } > > + > > /* Wait until finished previous write command. */ > > if (wait_till_ready(flash)) { > > mutex_unlock(&flash->lock); > > @@ -457,6 +523,14 @@ static int m25p80_write(struct mtd_info *mtd, > loff_t to, size_t len, > > t[1].len = page_size; > > spi_sync(flash->spi, &m); > > > > + /* Flag status, Wait until finished previous write command. */ > > + if (flash->flag_status == true) { > > + if (wait_till_fsr_ready(flash)) { > > + mutex_unlock(&flash->lock); > > + return 1; > > + } > > + } > > + > > *retlen = m.actual_length - m25p_cmdsz(flash); > > > > /* write everything in flash->page_size chunks */ @@ -477,6 > +551,14 > > @@ static int m25p80_write(struct mtd_info *mtd, loff_t to, size_t > > len, > > > > spi_sync(flash->spi, &m); > > > > + /* Flag status, Wait until finished previous write > command. */ > > + if (flash->flag_status == true) { > > + if (wait_till_fsr_ready(flash)) { > > + mutex_unlock(&flash->lock); > > + return 1; > > + } > > + } > > + > > *retlen += m.actual_length - m25p_cmdsz(flash); > > } > > } > > @@ -782,7 +864,7 @@ static const struct spi_device_id m25p_ids[] = { > > { "n25q128a11", INFO(0x20bb18, 0, 64 * 1024, 256, 0) }, > > { "n25q128a13", INFO(0x20ba18, 0, 64 * 1024, 256, 0) }, > > { "n25q256a", INFO(0x20ba19, 0, 64 * 1024, 512, SECT_4K) }, > > - { "n25q512a", INFO(0x20bb20, 0, 64 * 1024, 1024, SECT_4K) }, > > + { "n25q512a", INFO(0x20ba20, 0, 64 * 1024, 1024, SECT_4K) }, > > You probably want to figure out the distinction between the old table entry > and the new one, and assign your new entry a new string accordingly. You mean "0x20bb20" (old value) must be still the valid value? I will wait to you to add new string. > > > > > /* PMC */ > > { "pm25lv512", INFO(0, 0, 32 * 1024, 2, SECT_4K_PMC) }, > > @@ -996,6 +1078,13 @@ static int m25p_probe(struct spi_device *spi) > > spi_set_drvdata(spi, flash); > > > > /* > > + * Micron n25q512a requires to check flag status register > > + */ > > + flash->flag_status = false; > > + if (strcmp(id->name, "n25q512a") == 0) > > + flash->flag_status = true;; > > This doesn't look good at all. If any other flash start to need this, we don't > want to code each ID string here. That's fragile and non-scaleable. If we need > this option, you need to add another flag to the m25p_ids[] table, I guess... > I agree, and will see how I can make it more generic. Also wait to see if you/mail list feedback on this flag status. > > + > > + /* > > * Atmel, SST and Intel/Numonyx serial flash tend to power > > * up with the software protection bits set > > */ > > Brian Insop
Hi Marek, > From: Marek Vasut [mailto:marex@denx.de] > Sent: Thursday, February 27, 2014 12:02 PM > > On Thursday, February 27, 2014 at 08:33:14 AM, Brian Norris wrote: > > + Marek > > Thanks, I really need to subscribe to this ML ;-/ > > > Hi Insop, > > > > On Mon, Jan 06, 2014 at 05:21:17AM +0000, Insop Song wrote: > > > In order to use Micron n25q512a, MTD, two changes are required as > > > follows: - jedec code should be fixed > > > > I have a feeling there are more than one "n25q512a" device, with > > different IDs. > > ACK, I have similar feeling. Jain, can you tell us the precise marking on your > N25Q512A chip (that is, every single letter on the top of the chip package > exactly as it is engraved there ) please? Or make a photo ... > > Insop, can you please do the same ? > > [...] 25Q512A 13640 > > > > @@ -782,7 +864,7 @@ static const struct spi_device_id m25p_ids[] = { > > > > > > { "n25q128a11", INFO(0x20bb18, 0, 64 * 1024, 256, 0) }, > > > { "n25q128a13", INFO(0x20ba18, 0, 64 * 1024, 256, 0) }, > > > { "n25q256a", INFO(0x20ba19, 0, 64 * 1024, 512, SECT_4K) }, > > > > > > - { "n25q512a", INFO(0x20bb20, 0, 64 * 1024, 1024, SECT_4K) }, > > > + { "n25q512a", INFO(0x20ba20, 0, 64 * 1024, 1024, SECT_4K) }, > > > > You probably want to figure out the distinction between the old table > > entry and the new one, and assign your new entry a new string > > accordingly. > > ACK. The datasheet actually claims this change is correct, see [1] (page 40, > table 21), but as we know, Micron really does shitty job when it comes to > using the JEDEC IDs for it's chips. > "n25q512a1" okay? Or any other suggestion? > > > /* PMC */ > > > { "pm25lv512", INFO(0, 0, 32 * 1024, 2, SECT_4K_PMC) }, > > > > > > @@ -996,6 +1078,13 @@ static int m25p_probe(struct spi_device *spi) > > > > > > spi_set_drvdata(spi, flash); > > > > > > /* > > > > > > + * Micron n25q512a requires to check flag status register > > > + */ > > > + flash->flag_status = false; > > > + if (strcmp(id->name, "n25q512a") == 0) > > > + flash->flag_status = true;; > > > > This doesn't look good at all. If any other flash start to need this, > > we don't want to code each ID string here. That's fragile and > > non-scaleable. If we need this option, you need to add another flag to > > the m25p_ids[] table, I guess... > > This waiting for some bit in FSR looks like it can be wrapped into > wait_till_ready(), no ? > > What I cannot find in the datasheet though is any evidence which would > clearly mandate the use of FSR bit 0x80 to test if the chip is ready for next > operation instead of using the regular STATUS register bit . Insop, can you > please elaborate on why using the FSR is needed ? > If I don't check FSR(Flag Status Register), then I was not able to program this flash. I've used C SDK from Aardvark I2C/SPI Host Adapter to program directly, and we had to check FSR. Also in linux driver, I had to put the check otherwise, I am not able to write the flash. I think it is mandatory though datasheet is not so clear about it. > [1] > https://www.micron.com/~/media/Documents/Products/Data%20Sheet/N > OR%20Flash/Serial%20NOR/N25Q/n25q_512mb_1ce_3v_65nm.pdf
We also use this SPI flash and we have a similar mod to make it work. I can't say I like the unique branch in the mtd code, but alternatives are more complicated. Maybe a callback function to check status? Chuck Peplinski Vice President of Engineering Momentum Data Systems On 2/28/2014 8:44 PM, Insop Song wrote: > Hi Marek, > >> From: Marek Vasut [mailto:marex@denx.de] >> Sent: Thursday, February 27, 2014 12:02 PM >> >> On Thursday, February 27, 2014 at 08:33:14 AM, Brian Norris wrote: >>> + Marek >> >> Thanks, I really need to subscribe to this ML ;-/ >> >>> Hi Insop, >>> >>> On Mon, Jan 06, 2014 at 05:21:17AM +0000, Insop Song wrote: >>>> In order to use Micron n25q512a, MTD, two changes are required as >>>> follows: - jedec code should be fixed >>> >>> I have a feeling there are more than one "n25q512a" device, with >>> different IDs. >> >> ACK, I have similar feeling. Jain, can you tell us the precise marking on your >> N25Q512A chip (that is, every single letter on the top of the chip package >> exactly as it is engraved there ) please? Or make a photo ... >> >> Insop, can you please do the same ? >> >> [...] > > 25Q512A > 13640 > >> >>>> @@ -782,7 +864,7 @@ static const struct spi_device_id m25p_ids[] = { >>>> >>>> { "n25q128a11", INFO(0x20bb18, 0, 64 * 1024, 256, 0) }, >>>> { "n25q128a13", INFO(0x20ba18, 0, 64 * 1024, 256, 0) }, >>>> { "n25q256a", INFO(0x20ba19, 0, 64 * 1024, 512, SECT_4K) }, >>>> >>>> - { "n25q512a", INFO(0x20bb20, 0, 64 * 1024, 1024, SECT_4K) }, >>>> + { "n25q512a", INFO(0x20ba20, 0, 64 * 1024, 1024, SECT_4K) }, >>> >>> You probably want to figure out the distinction between the old table >>> entry and the new one, and assign your new entry a new string >>> accordingly. >> >> ACK. The datasheet actually claims this change is correct, see [1] (page 40, >> table 21), but as we know, Micron really does shitty job when it comes to >> using the JEDEC IDs for it's chips. >> > > "n25q512a1" okay? > Or any other suggestion? > >>>> /* PMC */ >>>> { "pm25lv512", INFO(0, 0, 32 * 1024, 2, SECT_4K_PMC) }, >>>> >>>> @@ -996,6 +1078,13 @@ static int m25p_probe(struct spi_device *spi) >>>> >>>> spi_set_drvdata(spi, flash); >>>> >>>> /* >>>> >>>> + * Micron n25q512a requires to check flag status register >>>> + */ >>>> + flash->flag_status = false; >>>> + if (strcmp(id->name, "n25q512a") == 0) >>>> + flash->flag_status = true;; >>> >>> This doesn't look good at all. If any other flash start to need this, >>> we don't want to code each ID string here. That's fragile and >>> non-scaleable. If we need this option, you need to add another flag to >>> the m25p_ids[] table, I guess... >> >> This waiting for some bit in FSR looks like it can be wrapped into >> wait_till_ready(), no ? >> >> What I cannot find in the datasheet though is any evidence which would >> clearly mandate the use of FSR bit 0x80 to test if the chip is ready for next >> operation instead of using the regular STATUS register bit . Insop, can you >> please elaborate on why using the FSR is needed ? >> > > If I don't check FSR(Flag Status Register), then I was not able to program this flash. > I've used C SDK from Aardvark I2C/SPI Host Adapter to program directly, and we had to check FSR. > Also in linux driver, I had to put the check otherwise, I am not able to write the flash. > I think it is mandatory though datasheet is not so clear about it. > > >> [1] >> https://www.micron.com/~/media/Documents/Products/Data%20Sheet/N >> OR%20Flash/Serial%20NOR/N25Q/n25q_512mb_1ce_3v_65nm.pdf > > ______________________________________________________ > Linux MTD discussion mailing list > http://lists.infradead.org/mailman/listinfo/linux-mtd/ >
On Saturday, March 01, 2014 at 03:48:17 PM, Chuck Peplinski wrote: > We also use this SPI flash and we have a similar mod to make it work. I > can't say I like the unique branch in the mtd code, but alternatives are > more complicated. Maybe a callback function to check status? Can you please stop top-posting ? Moreover, I really cannot make heads or tails of your comment, sorry. Best regards, Marek Vasut
On Saturday, March 01, 2014 at 03:00:04 AM, Insop Song wrote: > Hi Brian, > > Thank you for your feedback. > > > From: Brian Norris [mailto:computersforpeace@gmail.com] > > Sent: Wednesday, February 26, 2014 11:33 PM> > > > > On Mon, Jan 06, 2014 at 05:21:17AM +0000, Insop Song wrote: > > > In order to use Micron n25q512a, MTD, two changes are required as > > > > follows: > > > - jedec code should be fixed > > > > I have a feeling there are more than one "n25q512a" device, with > > different IDs. > > > > > - flag status should be read for writing. > > > > > > Check flag status register for Micron n25q512a > > > > > > - Programing Micron n25q512a requires to check flag status register > > > - According to datasheet > > > > > > " > > > The flag status register must be read any time a PROGRAM, ERASE, > > > SUSPEND/RESUME command is issued, or after a RESET command > > > > while device > > > > > is busy. The cycle is not complete until bit 7 of the flag status > > > register output 1. > > > " > > > > > > - Ref: > > https://www.micron.com/~/media/Documents/Products/Data%20Sheet/N > > OR%20F > > > > > lash/Serial%20NOR/N25Q/n25q_512mb_1ce_3v_65nm.pdf > > > > Hmm, are you sure that all Micron n25q512a need to check the flag status > > register? I'll check my datasheets when I'm back in the office, but this > > seems peculiar. > > "Flag status" term can be found in 84 time in the data sheet and I don't > think the flag status is specific to particular type of n25q512a. n25q256a works just fine with regular SR check. [...] > > > @@ -782,7 +864,7 @@ static const struct spi_device_id m25p_ids[] = { > > > > > > { "n25q128a11", INFO(0x20bb18, 0, 64 * 1024, 256, 0) }, > > > { "n25q128a13", INFO(0x20ba18, 0, 64 * 1024, 256, 0) }, > > > { "n25q256a", INFO(0x20ba19, 0, 64 * 1024, 512, SECT_4K) }, > > > > > > - { "n25q512a", INFO(0x20bb20, 0, 64 * 1024, 1024, SECT_4K) }, > > > + { "n25q512a", INFO(0x20ba20, 0, 64 * 1024, 1024, SECT_4K) }, > > > > You probably want to figure out the distinction between the old table > > entry and the new one, and assign your new entry a new string > > accordingly. > > You mean "0x20bb20" (old value) must be still the valid value? Likely, since someone added it before and if you look at the other Micron entries, you see chips with '0x20bb..' as well as '0x20ba' . Micron must have done something funny here (again :-( ) [...]
On Saturday, March 01, 2014 at 03:44:46 AM, Insop Song wrote: > Hi Marek, > > > From: Marek Vasut [mailto:marex@denx.de] > > Sent: Thursday, February 27, 2014 12:02 PM > > > > On Thursday, February 27, 2014 at 08:33:14 AM, Brian Norris wrote: > > > + Marek > > > > Thanks, I really need to subscribe to this ML ;-/ > > > > > Hi Insop, > > > > > > On Mon, Jan 06, 2014 at 05:21:17AM +0000, Insop Song wrote: > > > > In order to use Micron n25q512a, MTD, two changes are required as > > > > follows: - jedec code should be fixed > > > > > > I have a feeling there are more than one "n25q512a" device, with > > > different IDs. > > > > ACK, I have similar feeling. Jain, can you tell us the precise marking on > > your N25Q512A chip (that is, every single letter on the top of the chip > > package exactly as it is engraved there ) please? Or make a photo ... > > > > Insop, can you please do the same ? > > > > [...] > > 25Q512A > 13640 OK, I think I figured it out already. Look at [1] and [2] . The former is 1V8 option, the later is 3V3 option of the same N25Q512A SPI NOR. If you look for 'JEDEC-standard 2-byte signature' in the datasheets, you will notice there's one that is 'BB20h' for the 1V8 part and 'BA20h' for the 3V3 part. So I suppose that's the mystery here and this is the explanation. [1] www.micron.com/~/media/Documents/Products/Data%20Sheet/NOR%20Flash/Serial%20NOR/N25Q/n25q_512mb_1_8v_65nm.pdf [2] www.micron.com/~/media/Documents/Products/Data%20Sheet/NOR%20Flash/Serial%20NOR/N25Q/n25q_512mb_1ce_3v_65nm.pdf > > > > @@ -782,7 +864,7 @@ static const struct spi_device_id m25p_ids[] = { > > > > > > > > { "n25q128a11", INFO(0x20bb18, 0, 64 * 1024, 256, 0) }, > > > > { "n25q128a13", INFO(0x20ba18, 0, 64 * 1024, 256, 0) }, > > > > { "n25q256a", INFO(0x20ba19, 0, 64 * 1024, 512, SECT_4K) }, > > > > > > > > - { "n25q512a", INFO(0x20bb20, 0, 64 * 1024, 1024, SECT_4K) }, > > > > + { "n25q512a", INFO(0x20ba20, 0, 64 * 1024, 1024, SECT_4K) }, > > > > > > You probably want to figure out the distinction between the old table > > > entry and the new one, and assign your new entry a new string > > > accordingly. > > > > ACK. The datasheet actually claims this change is correct, see [1] (page > > 40, table 21), but as we know, Micron really does shitty job when it > > comes to using the JEDEC IDs for it's chips. > > "n25q512a1" okay? > Or any other suggestion? According the chapter 'Part Number Ordering Information', the naming scheme here should be the same as in case of the n25q128a11/n25q128a13 , since the two trailing numbers mean: 1: 1 ... Byte addressability; HOLD pin; Micron XIP 2: 1 ... Vcc = 1.7 to 2.0V 3 ... Vcc = 2.7 to 3.6V > > > > /* PMC */ > > > > { "pm25lv512", INFO(0, 0, 32 * 1024, 2, SECT_4K_PMC) }, > > > > > > > > @@ -996,6 +1078,13 @@ static int m25p_probe(struct spi_device *spi) > > > > > > > > spi_set_drvdata(spi, flash); > > > > > > > > /* > > > > > > > > + * Micron n25q512a requires to check flag status register > > > > + */ > > > > + flash->flag_status = false; > > > > + if (strcmp(id->name, "n25q512a") == 0) > > > > + flash->flag_status = true;; > > > > > > This doesn't look good at all. If any other flash start to need this, > > > we don't want to code each ID string here. That's fragile and > > > non-scaleable. If we need this option, you need to add another flag to > > > the m25p_ids[] table, I guess... > > > > This waiting for some bit in FSR looks like it can be wrapped into > > wait_till_ready(), no ? > > > > What I cannot find in the datasheet though is any evidence which would > > clearly mandate the use of FSR bit 0x80 to test if the chip is ready for > > next operation instead of using the regular STATUS register bit . Insop, > > can you please elaborate on why using the FSR is needed ? > > If I don't check FSR(Flag Status Register), then I was not able to program > this flash. I've used C SDK from Aardvark I2C/SPI Host Adapter to program > directly, and we had to check FSR. Also in linux driver, I had to put the > check otherwise, I am not able to write the flash. I think it is mandatory > though datasheet is not so clear about it. What kind of problems do you get when you try to write the flash ? I am trying to crosscheck this with the N25Q256A I have in here, which also has the FSR, but I have no issues programming the chip either in Linux or in U-Boot. To me, it looks like FSR bit 7 and SR bit 7 should toggle exactly at the same time and exactly for the same events. Can you try for example reading them both and checking that the bit 7 really toggles at different times please? Best regards, Marek Vasut
Sorry about top posting.
I am also using this n25q512. My working code includes a fix similar to
what Song posted. I don't consider it elegant. The problem is that
this 25q512 apparently behaves in a unique fashion. If you read the
status register instead of the flag status register, reads work but
erases and writes fail. I know this from a couple of days of
debugging. You must respond to the flag status register. Yes, this is
different from every other part.
Some possible solutions are:
- hard code support for this device, as Song did.
- add some other abstraction that affects support for every other part.
I leave the decision to you, but neither sounds very pretty.
Chuck
On 3/1/2014 1:22 PM, Marek Vasut wrote:
> To me, it looks like FSR bit 7 and SR bit 7 should toggle exactly at the same
> time and exactly for the same events. Can you try for example reading them both
> and checking that the bit 7 really toggles at different times please?
>
On Sunday, March 02, 2014 at 06:28:26 AM, Chuck Peplinski wrote: > Sorry about top posting. You are sorry, yet you did it again :-( > I am also using this n25q512. My working code includes a fix similar to > what Song posted. I don't consider it elegant. The problem is that > this 25q512 apparently behaves in a unique fashion. If you read the > status register instead of the flag status register, reads work but > erases and writes fail. I know this from a couple of days of > debugging. So does the SR and FSR not toggle the bit 7 at the same time or what ? > You must respond to the flag status register. What do you mean by "respond" ? > Yes, this is > different from every other part. > > Some possible solutions are: > - hard code support for this device, as Song did. > - add some other abstraction that affects support for every other part. > I leave the decision to you, but neither sounds very pretty. > > Chuck > > On 3/1/2014 1:22 PM, Marek Vasut wrote: > > To me, it looks like FSR bit 7 and SR bit 7 should toggle exactly at the > > same time and exactly for the same events. Can you try for example > > reading them both and checking that the bit 7 really toggles at > > different times please? > > ______________________________________________________ > Linux MTD discussion mailing list > http://lists.infradead.org/mailman/listinfo/linux-mtd/ Best regards, Marek Vasut
On 3/2/2014 8:42 AM, Marek Vasut wrote: > On Sunday, March 02, 2014 at 06:28:26 AM, Chuck Peplinski wrote: >> Sorry about top posting. > You are sorry, yet you did it again :-( > >> I am also using this n25q512. My working code includes a fix similar to >> what Song posted. I don't consider it elegant. The problem is that >> this 25q512 apparently behaves in a unique fashion. If you read the >> status register instead of the flag status register, reads work but >> erases and writes fail. I know this from a couple of days of >> debugging. > So does the SR and FSR not toggle the bit 7 at the same time or what ? > >> You must respond to the flag status register. > What do you mean by "respond" ? > >> Yes, this is >> different from every other part. >> >> Some possible solutions are: >> - hard code support for this device, as Song did. >> - add some other abstraction that affects support for every other part. >> I leave the decision to you, but neither sounds very pretty. >> >> Chuck >> >> On 3/1/2014 1:22 PM, Marek Vasut wrote: >>> To me, it looks like FSR bit 7 and SR bit 7 should toggle exactly at the >>> same time and exactly for the same events. Can you try for example >>> reading them both and checking that the bit 7 really toggles at >>> different times please? >> ______________________________________________________ >> Linux MTD discussion mailing list >> http://lists.infradead.org/mailman/listinfo/linux-mtd/ > Best regards, > Marek Vasut > I'm learning about top posting. How about this? FSR can apparently toggle without SR. For the device to work, I must read and then clear the flag status register. My "wait_till_done()" (below) is called at the end of m25p80_erase() and m25p80_write(). If you would like, I could submit a patch, or just a copy of m25p80.c. // used with N25Q512A static int wait_till_done(struct m25p *flash) { unsigned long deadline; u8 code, fsreg; code = OPCODE_RD_FLAG_SR; // read flag status register deadline = jiffies + (1* HZ); // one second do { spi_write_then_read(flash->spi, &code, 1, &fsreg, 1); if (fsreg & BIT_FLAG_STATUS_READY) { // not busy. code = OPCODE_CLEAR_FLAG_SR; // clear flag status register spi_write_then_read(flash->spi, &code, 1, NULL, 0); return 0; } cond_resched(); // a form of kernel sleep } while (!time_after_eq(jiffies, deadline)); return 1; } Chuck
On Monday, March 03, 2014 at 05:52:06 PM, Chuck Peplinski wrote: > On 3/2/2014 8:42 AM, Marek Vasut wrote: > > On Sunday, March 02, 2014 at 06:28:26 AM, Chuck Peplinski wrote: > >> Sorry about top posting. > > > > You are sorry, yet you did it again :-( > > > >> I am also using this n25q512. My working code includes a fix similar to > >> what Song posted. I don't consider it elegant. The problem is that > >> this 25q512 apparently behaves in a unique fashion. If you read the > >> status register instead of the flag status register, reads work but > >> erases and writes fail. I know this from a couple of days of > >> debugging. > > > > So does the SR and FSR not toggle the bit 7 at the same time or what ? > > > >> You must respond to the flag status register. > > > > What do you mean by "respond" ? > > > >> Yes, this is > >> different from every other part. > >> > >> Some possible solutions are: > >> - hard code support for this device, as Song did. > >> - add some other abstraction that affects support for every other part. > >> I leave the decision to you, but neither sounds very pretty. > >> > >> Chuck > >> > >> On 3/1/2014 1:22 PM, Marek Vasut wrote: > >>> To me, it looks like FSR bit 7 and SR bit 7 should toggle exactly at > >>> the same time and exactly for the same events. Can you try for example > >>> reading them both and checking that the bit 7 really toggles at > >>> different times please? > >> > >> ______________________________________________________ > >> Linux MTD discussion mailing list > >> http://lists.infradead.org/mailman/listinfo/linux-mtd/ > > > > Best regards, > > Marek Vasut > > I'm learning about top posting. How about this? Better, good job ;-) Now , how come the CC list was dropped ? > FSR can apparently toggle without SR. Is that documented anywhere ? How can that be ? > For the device to work, I must > read and then clear the flag status register. > > My "wait_till_done()" (below) is called at the end of m25p80_erase() and > m25p80_write(). Hmmmm , I have a feeling that if you actually added wait_till_ready() call at the end of _erase() and _write(), you would get the same effect. This would in turn mean you are instead missing wait_till_ready() somewhere else. Can you try using wait_till_ready() at the end of _erase() and _write() please ? > If you would like, I could submit a patch, or just a copy of m25p80.c. > > // used with N25Q512A > static int wait_till_done(struct m25p *flash) { > unsigned long deadline; > u8 code, fsreg; > > code = OPCODE_RD_FLAG_SR; // read flag status register > deadline = jiffies + (1* HZ); // one second > > do { > spi_write_then_read(flash->spi, &code, 1, &fsreg, 1); > if (fsreg & BIT_FLAG_STATUS_READY) { // not busy. > code = OPCODE_CLEAR_FLAG_SR; // clear flag status register > spi_write_then_read(flash->spi, &code, 1, NULL, 0); > return 0; > } > cond_resched(); // a form of kernel sleep > } while (!time_after_eq(jiffies, deadline)); > return 1; > } > > Chuck
On 3/3/2014 6:29 PM, Marek Vasut wrote: >> FSR can apparently toggle without SR. > Is that documented anywhere ? How can that be ? I'm looking at the data sheet for the part, n25q_512mb_1ce3v_65nm.pdf, from the Micron web site at http://www.micron.com/products/nor-flash/serial-nor-flash#fullPart&236=10. The comment that led us in this direction is on page 62: "ERASE Operations When the operation is in progress, the program or erase controller bit of the flag status register is set to 0. The flag status register must be polled for the operation status. When the operation completes, that bit is cleared to 1. Note that the flag status register must be polled even if operation times out." > Hmmmm , I have a feeling that if you actually added wait_till_ready() > call at the end of _erase() and _write(), you would get the same > effect. This would in turn mean you are instead missing > wait_till_ready() somewhere else. Can you try using wait_till_ready() > at the end of _erase() and _write() please ? That would be the standard code, right? That's where we started and it did not work. At some point I'll try some more tests. I'm not blocked on this now, so it's not totally critical. One thing I notice: The web site notes that this part stacks two 256M dies. Maybe that's why it is non-standard? Sure would be nice to hear from someone at Micron...
Hi All, On Wed, Mar 5, 2014 at 3:15 AM, Chuck Peplinski <chuck@mds.com> wrote: > On 3/3/2014 6:29 PM, Marek Vasut wrote: >>> >>> FSR can apparently toggle without SR. >> >> Is that documented anywhere ? How can that be ? > > I'm looking at the data sheet for the part, n25q_512mb_1ce3v_65nm.pdf, from > the Micron web site at > http://www.micron.com/products/nor-flash/serial-nor-flash#fullPart&236=10. > The comment that led us in this direction is on page 62: > > "ERASE Operations > When the operation is in progress, the program or erase controller bit of > the flag status > register is set to 0. The flag status register must be polled for the > operation status. When > the operation completes, that bit is cleared to 1. > Note that the flag status register must be polled even if operation times > out." > > >> Hmmmm , I have a feeling that if you actually added wait_till_ready() call >> at the end of _erase() and _write(), you would get the same effect. This >> would in turn mean you are instead missing wait_till_ready() somewhere else. >> Can you try using wait_till_ready() at the end of _erase() and _write() >> please ? > > > That would be the standard code, right? That's where we started and it did > not work. > > At some point I'll try some more tests. I'm not blocked on this now, so > it's not totally critical. > One thing I notice: The web site notes that this part stacks two 256M dies. > Maybe that's why it is non-standard? > Sure would be nice to hear from someone at Micron... BTW: I have some information regarding this part. We have tested this part both in u-boot and Linux where we could see an issue while polling read_status, and then we moved to use flag_status then we saw the consistent behavior. Inputs we're got from Micron are: 1. Micron (though not clearly documented), stated that flag_status is clearly required to be polled, even though read_status reports the exact status. They are not willing to change the datasheet but plan on releasing an app note or something because this has led to a lot of confusion. 2. At this moment, no one else has such weird design of needing an flag_status poll. So checking for the device ID and polling flag_status looks like the only option. The same approach we tried as well. CCing - Harini (tested flag_status on Linux) CCing - Todd Legler from Micron. Todd and Harini - Please share your inputs. thanks!
Hi, > -----Original Message----- > From: Jagan Teki [mailto:jagannadh.teki@gmail.com] > Sent: Thursday, March 06, 2014 2:56 PM > To: chuck@mds.com > Cc: Marek Vasut; linux-mtd@lists.infradead.org; Todd Legler (tlegler); Harini > Katakam > Subject: Re: [PATCH] Check flag status register for Micron n25q512a > > Hi All, > > On Wed, Mar 5, 2014 at 3:15 AM, Chuck Peplinski <chuck@mds.com> wrote: > > On 3/3/2014 6:29 PM, Marek Vasut wrote: > >>> > >>> FSR can apparently toggle without SR. > >> > >> Is that documented anywhere ? How can that be ? > > > > I'm looking at the data sheet for the part, n25q_512mb_1ce3v_65nm.pdf, > > from the Micron web site at > > http://www.micron.com/products/nor-flash/serial-nor- > flash#fullPart&236=10. > > The comment that led us in this direction is on page 62: > > > > "ERASE Operations > > When the operation is in progress, the program or erase controller bit > > of the flag status register is set to 0. The flag status register must > > be polled for the operation status. When the operation completes, that > > bit is cleared to 1. > > Note that the flag status register must be polled even if operation > > times out." > > > > > >> Hmmmm , I have a feeling that if you actually added wait_till_ready() > >> call at the end of _erase() and _write(), you would get the same > >> effect. This would in turn mean you are instead missing wait_till_ready() > somewhere else. > >> Can you try using wait_till_ready() at the end of _erase() and > >> _write() please ? > > > > > > That would be the standard code, right? That's where we started and > > it did not work. > > > > At some point I'll try some more tests. I'm not blocked on this now, > > so it's not totally critical. > > One thing I notice: The web site notes that this part stacks two 256M dies. > > Maybe that's why it is non-standard? > > Sure would be nice to hear from someone at Micron... > > BTW: I have some information regarding this part. > > We have tested this part both in u-boot and Linux where we could see an > issue while polling read_status, and then we moved to use flag_status then > we saw the consistent behavior. > > Inputs we're got from Micron are: > 1. Micron (though not clearly documented), stated that flag_status is clearly > required to be polled, even though read_status reports the exact status. > They are not willing to change the datasheet but plan on releasing an > app note or something because this has led to a lot of confusion. > 2. At this moment, no one else has such weird design of needing an > flag_status poll. > So checking for the device ID and polling flag_status looks like the only > option. > > The same approach we tried as well. > > CCing - Harini (tested flag_status on Linux) CCing - Todd Legler from Micron. > > Todd and Harini - Please share your inputs. > Micron devices with more than one die i.e. n25q_512mb and n25q_1gb have this additional check. Notes 14 and 15 under command definition table 18 describe this best. Although, it is stated that the WIP bit in status register is NOT of bit 7 in Flag status register, FSR still needs to be polled. We had a discussion with Micron from which we learnt the following: 1. Program/erase operations in multi-die devices are not complete until Read FSR command receives a ready response form flash at least one time. 2. In addition, if you are doing a Write Non-volatile configuration register/ Write status register operation, the operation is only complete after FSR poll returns ready consecutively 2 or 4 times (as many times as the number of die). In this case, each time the chip select is toggled and Read FSR command is sent, response is sent by alternate die (1,2,1,2... or 1,2,3,4,1,2,3,4.....). Todd, please add to this if you would like to elaborate. Regards, Harini > > thanks! > -- > Jagan. This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.
On Thursday, March 06, 2014 at 11:03:50 AM, Harini Katakam wrote: > Hi, > > > -----Original Message----- > > From: Jagan Teki [mailto:jagannadh.teki@gmail.com] > > Sent: Thursday, March 06, 2014 2:56 PM > > To: chuck@mds.com > > Cc: Marek Vasut; linux-mtd@lists.infradead.org; Todd Legler (tlegler); > > Harini Katakam > > Subject: Re: [PATCH] Check flag status register for Micron n25q512a > > > > Hi All, > > > > On Wed, Mar 5, 2014 at 3:15 AM, Chuck Peplinski <chuck@mds.com> wrote: > > > On 3/3/2014 6:29 PM, Marek Vasut wrote: > > >>> FSR can apparently toggle without SR. > > >> > > >> Is that documented anywhere ? How can that be ? > > > > > > I'm looking at the data sheet for the part, n25q_512mb_1ce3v_65nm.pdf, > > > from the Micron web site at > > > http://www.micron.com/products/nor-flash/serial-nor- > > > > flash#fullPart&236=10. > > > > > The comment that led us in this direction is on page 62: > > > > > > "ERASE Operations > > > When the operation is in progress, the program or erase controller bit > > > of the flag status register is set to 0. The flag status register must > > > be polled for the operation status. When the operation completes, that > > > bit is cleared to 1. > > > Note that the flag status register must be polled even if operation > > > times out." > > > > > >> Hmmmm , I have a feeling that if you actually added wait_till_ready() > > >> call at the end of _erase() and _write(), you would get the same > > >> effect. This would in turn mean you are instead missing > > >> wait_till_ready() > > > > somewhere else. > > > > >> Can you try using wait_till_ready() at the end of _erase() and > > >> _write() please ? > > > > > > That would be the standard code, right? That's where we started and > > > it did not work. > > > > > > At some point I'll try some more tests. I'm not blocked on this now, > > > so it's not totally critical. > > > One thing I notice: The web site notes that this part stacks two 256M > > > dies. Maybe that's why it is non-standard? > > > Sure would be nice to hear from someone at Micron... > > > > BTW: I have some information regarding this part. > > > > We have tested this part both in u-boot and Linux where we could see an > > issue while polling read_status, and then we moved to use flag_status > > then we saw the consistent behavior. > > > > Inputs we're got from Micron are: > > 1. Micron (though not clearly documented), stated that flag_status is > > clearly > > > > required to be polled, even though read_status reports the exact > > status. They are not willing to change the datasheet but plan on > > releasing an app note or something because this has led to a lot of > > confusion. > > > > 2. At this moment, no one else has such weird design of needing an > > flag_status poll. > > > > So checking for the device ID and polling flag_status looks like the > > only > > > > option. > > > > The same approach we tried as well. > > > > CCing - Harini (tested flag_status on Linux) CCing - Todd Legler from > > Micron. > > > > Todd and Harini - Please share your inputs. > > Micron devices with more than one die i.e. n25q_512mb and n25q_1gb > have this additional check. > Notes 14 and 15 under command definition table 18 describe this best. > Although, it is stated that the WIP bit in status register is NOT of > bit 7 in Flag status register, FSR still needs to be polled. > > We had a discussion with Micron from which we learnt the following: > 1. Program/erase operations in multi-die devices are not complete until > Read FSR command receives a ready response form flash at least one time. > 2. In addition, if you are doing a Write Non-volatile configuration > register/ Write status register operation, the operation is only complete > after FSR poll returns ready consecutively 2 or 4 times (as many times as > the number of die). In this case, each time the chip select is toggled and > Read FSR command is sent, response is sent by alternate die (1,2,1,2... or > 1,2,3,4,1,2,3,4.....). > > Todd, please add to this if you would like to elaborate. Thanks for pointing this out.
Insop Song, In the datasheet of the N25Q512A it is mentioning at different places that the Flag Status Register should be polled twice in different spi accesses, once for each die in the chip. However, I don't see this happening in your patch. - Didn't you have any problems using your patch? - Is there some git repo having a patch to support the N25Q512A yet? Kind regards, Yves
diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c index 7eda71d..e53e522 100644 --- a/drivers/mtd/devices/m25p80.c +++ b/drivers/mtd/devices/m25p80.c @@ -38,6 +38,7 @@ /* Flash opcodes. */ #define OPCODE_WREN 0x06 /* Write enable */ #define OPCODE_RDSR 0x05 /* Read status register */ +#define OPCODE_RDFSR 0x70 /* Read flag status register */ #define OPCODE_WRSR 0x01 /* Write status register 1 byte */ #define OPCODE_NORM_READ 0x03 /* Read data bytes (low frequency) */ #define OPCODE_FAST_READ 0x0b /* Read data bytes (high frequency) */ @@ -70,6 +71,8 @@ /* Status Register bits. */ #define SR_WIP 1 /* Write in progress */ #define SR_WEL 2 /* Write enable latch */ +/* Flag Status Register bits. */ +#define FSR_RDY 0x80 /* FSR ready */ /* meaning of other SR_* bits may differ between vendors */ #define SR_BP0 4 /* Block protect 0 */ #define SR_BP1 8 /* Block protect 1 */ @@ -95,6 +98,7 @@ struct m25p { u8 program_opcode; u8 *command; bool fast_read; + bool flag_status; }; static inline struct m25p *mtd_to_m25p(struct mtd_info *mtd) @@ -131,6 +135,28 @@ static int read_sr(struct m25p *flash) } /* + * Read the flag status register, returning its value in the location + * Return the status register value. + * Returns negative if error occurred. + */ +static int read_fsr(struct m25p *flash) +{ + ssize_t retval; + u8 code = OPCODE_RDFSR; + u8 val; + + retval = spi_write_then_read(flash->spi, &code, 1, &val, 1); + + if (retval < 0) { + printk(&flash->spi->dev, "error %d reading FSR\n", + (int) retval); + return retval; + } + + return val; +} + +/* * Write status register 1 byte * Returns negative if error occurred. */ @@ -220,6 +246,30 @@ static int wait_till_ready(struct m25p *flash) } /* + * Service routine to read flag status register until ready, or timeout occurs. + * Returns non-zero if error. + */ +static int wait_till_fsr_ready(struct m25p *flash) +{ + unsigned long deadline; + int sr; + + deadline = jiffies + MAX_READY_WAIT_JIFFIES; + + do { + if ((sr = read_fsr(flash)) < 0) + break; + else if ((sr & FSR_RDY)) + return 0; + + cond_resched(); + + } while (!time_after_eq(jiffies, deadline)); + + return 1; +} + +/* * Erase the whole flash memory * * Returns 0 if successful, non-zero otherwise. @@ -273,6 +323,14 @@ static int erase_sector(struct m25p *flash, u32 offset) if (wait_till_ready(flash)) return 1; + /* Flag status, Wait until finished previous write command. */ + if (flash->flag_status == true) { + if (wait_till_fsr_ready(flash)) { + mutex_unlock(&flash->lock); + return 1; + } + } + /* Send write enable, then erase commands. */ write_enable(flash); @@ -427,6 +485,14 @@ static int m25p80_write(struct mtd_info *mtd, loff_t to, size_t len, mutex_lock(&flash->lock); + /* Flag status, Wait until finished previous write command. */ + if (flash->flag_status == true) { + if (wait_till_fsr_ready(flash)) { + mutex_unlock(&flash->lock); + return 1; + } + } + /* Wait until finished previous write command. */ if (wait_till_ready(flash)) { mutex_unlock(&flash->lock); @@ -457,6 +523,14 @@ static int m25p80_write(struct mtd_info *mtd, loff_t to, size_t len, t[1].len = page_size; spi_sync(flash->spi, &m); + /* Flag status, Wait until finished previous write command. */ + if (flash->flag_status == true) { + if (wait_till_fsr_ready(flash)) { + mutex_unlock(&flash->lock); + return 1; + } + } + *retlen = m.actual_length - m25p_cmdsz(flash); /* write everything in flash->page_size chunks */ @@ -477,6 +551,14 @@ static int m25p80_write(struct mtd_info *mtd, loff_t to, size_t len, spi_sync(flash->spi, &m); + /* Flag status, Wait until finished previous write command. */ + if (flash->flag_status == true) { + if (wait_till_fsr_ready(flash)) { + mutex_unlock(&flash->lock); + return 1; + } + } + *retlen += m.actual_length - m25p_cmdsz(flash); } } @@ -782,7 +864,7 @@ static const struct spi_device_id m25p_ids[] = { { "n25q128a11", INFO(0x20bb18, 0, 64 * 1024, 256, 0) }, { "n25q128a13", INFO(0x20ba18, 0, 64 * 1024, 256, 0) }, { "n25q256a", INFO(0x20ba19, 0, 64 * 1024, 512, SECT_4K) }, - { "n25q512a", INFO(0x20bb20, 0, 64 * 1024, 1024, SECT_4K) }, + { "n25q512a", INFO(0x20ba20, 0, 64 * 1024, 1024, SECT_4K) }, /* PMC */ { "pm25lv512", INFO(0, 0, 32 * 1024, 2, SECT_4K_PMC) }, @@ -996,6 +1078,13 @@ static int m25p_probe(struct spi_device *spi) spi_set_drvdata(spi, flash); /* + * Micron n25q512a requires to check flag status register + */ + flash->flag_status = false; + if (strcmp(id->name, "n25q512a") == 0) + flash->flag_status = true;; + + /* * Atmel, SST and Intel/Numonyx serial flash tend to power * up with the software protection bits set */
All, In order to use Micron n25q512a, MTD, two changes are required as follows: - jedec code should be fixed - flag status should be read for writing. Check flag status register for Micron n25q512a - Programing Micron n25q512a requires to check flag status register - According to datasheet " The flag status register must be read any time a PROGRAM, ERASE, SUSPEND/RESUME command is issued, or after a RESET command while device is busy. The cycle is not complete until bit 7 of the flag status register output 1. " - Ref: https://www.micron.com/~/media/Documents/Products/Data%20Sheet/NOR%20Flash/Serial%20NOR/N25Q/n25q_512mb_1ce_3v_65nm.pdf Signed-off-by: Insop Song <insop.song@gainspeed.com> --- drivers/mtd/devices/m25p80.c | 91 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 90 insertions(+), 1 deletion(-)