* flash_lock issue for n25q 128mb spi nor part @ 2019-12-02 17:28 John Garry 2019-12-03 9:45 ` Tudor.Ambarus 0 siblings, 1 reply; 27+ messages in thread From: John Garry @ 2019-12-02 17:28 UTC (permalink / raw) To: tudor.ambarus, linux-mtd; +Cc: Mark Brown, chenxiang Hi guys, I am testing the following driver on top of Linus' master branch with a n25q128a11 part: https://lore.kernel.org/linux-mtd/1572886297-45400-3-git-send-email-john.garry@huawei.com/ I am finding flash lock is not working. Even after a “successful” lock, flash_lock is reporting flash is unlocked. And I can still write to the flash. I find that since 39d1e3340c73 ("mtd: spi-nor: Fix clearing of QE bit on lock()/unlock()"), we're using a 16b SR for this part in the driver, but this part only has a 8b SR. This hack fixes the problem for me: @ -4691,7 +4691,7 @@ static void spi_nor_info_init_params(struct spi_nor *nor) params->set_4byte = spansion_set_4byte; params->setup = spi_nor_default_setup; /* Default to 16-bit Write Status (01h) Command */ - nor->flags |= SNOR_F_HAS_16BIT_SR; +// nor->flags |= SNOR_F_HAS_16BIT_SR; Any idea what's going wrong? Thanks, John ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: flash_lock issue for n25q 128mb spi nor part 2019-12-02 17:28 flash_lock issue for n25q 128mb spi nor part John Garry @ 2019-12-03 9:45 ` Tudor.Ambarus 2019-12-03 10:31 ` John Garry 0 siblings, 1 reply; 27+ messages in thread From: Tudor.Ambarus @ 2019-12-03 9:45 UTC (permalink / raw) To: john.garry, linux-mtd; +Cc: broonie, chenxiang66 Hi, John, On 12/2/19 7:28 PM, John Garry wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > Hi guys, > > I am testing the following driver on top of Linus' master branch with a > n25q128a11 part: I understand the you enabled locking support for this flash on your side, because in mainline it is not yet there. Be aware that this flash has BP3 support, and in mainline we support for now just BP0-BP2, so just a partial lock can be achieved, and it depends on what you tried to lock. It will be helpful to tell how you enabled the locking and what commands did you use. > > https://lore.kernel.org/linux-mtd/1572886297-45400-3-git-send-email-john.garry@huawei.com/ > > I am finding flash lock is not working. Even after a “successful” lock, > flash_lock is reporting flash is unlocked. And I can still write to the > flash. > > I find that since 39d1e3340c73 ("mtd: spi-nor: Fix clearing of QE bit on > lock()/unlock()"), we're using a 16b SR for this part in the driver, but > this part only has a 8b SR. > > This hack fixes the problem for me: > > @ -4691,7 +4691,7 @@ static void spi_nor_info_init_params(struct spi_nor > *nor) > params->set_4byte = spansion_set_4byte; > params->setup = spi_nor_default_setup; > /* Default to 16-bit Write Status (01h) Command */ > - nor->flags |= SNOR_F_HAS_16BIT_SR; > +// nor->flags |= SNOR_F_HAS_16BIT_SR; This means that bfpt is not parsed. You can confirm this by running with the patch from below. diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c index f4afe123e9dc..735cd5202598 100644 --- a/drivers/mtd/spi-nor/spi-nor.c +++ b/drivers/mtd/spi-nor/spi-nor.c @@ -3545,8 +3545,11 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor, return err; /* Fix endianness of the BFPT DWORDs. */ - for (i = 0; i < BFPT_DWORD_MAX; i++) + for (i = 0; i < BFPT_DWORD_MAX; i++) { bfpt.dwords[i] = le32_to_cpu(bfpt.dwords[i]); + dev_err(nor->dev, "bfpt.dwords[%d] = %08x\n", i, + bfpt.dwords[i]); + } /* Number of address bytes. */ switch (bfpt.dwords[BFPT_DWORD(1)] & BFPT_DWORD1_ADDRESS_BYTES_MASK) { @@ -3649,6 +3652,9 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor, params->page_size >>= BFPT_DWORD11_PAGE_SIZE_SHIFT; params->page_size = 1U << params->page_size; + dev_err(nor->dev, "bfpt.dwords[BFPT_DWORD(15)] & BFPT_DWORD15_QER_MASK = %08x\n", + bfpt.dwords[BFPT_DWORD(15)] & BFPT_DWORD15_QER_MASK); + /* Quad Enable Requirements. */ switch (bfpt.dwords[BFPT_DWORD(15)] & BFPT_DWORD15_QER_MASK) { case BFPT_DWORD15_QER_NONE: ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: flash_lock issue for n25q 128mb spi nor part 2019-12-03 9:45 ` Tudor.Ambarus @ 2019-12-03 10:31 ` John Garry 2019-12-03 11:07 ` Tudor.Ambarus 0 siblings, 1 reply; 27+ messages in thread From: John Garry @ 2019-12-03 10:31 UTC (permalink / raw) To: Tudor.Ambarus, linux-mtd; +Cc: broonie, chenxiang66 On 03/12/2019 09:45, Tudor.Ambarus@microchip.com wrote: > Hi, John, Hi Tudor, Thanks for getting back to me. > > On 12/2/19 7:28 PM, John Garry wrote: >> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe >> >> Hi guys, >> >> I am testing the following driver on top of Linus' master branch with a >> n25q128a11 part: > > I understand the you enabled locking support for this flash on your side, > because in mainline it is not yet there. > > Be aware that this flash has BP3 support, and in mainline we support for now > just BP0-BP2, so just a partial lock can be achieved, and it depends on what you > tried to lock. It will be helpful to tell how you enabled the locking and what > commands did you use. So I am simply using flash_lock -l /dev/mtd0, which will try to lock the whole device. Now I see what you're saying about BP3 and how complete locking is not available. > >> >> https://lore.kernel.org/linux-mtd/1572886297-45400-3-git-send-email-john.garry@huawei.com/ >> >> I am finding flash lock is not working. Even after a “successful” lock, >> flash_lock is reporting flash is unlocked. And I can still write to the >> flash. >> >> I find that since 39d1e3340c73 ("mtd: spi-nor: Fix clearing of QE bit on >> lock()/unlock()"), we're using a 16b SR for this part in the driver, but >> this part only has a 8b SR. >> >> This hack fixes the problem for me: >> >> @ -4691,7 +4691,7 @@ static void spi_nor_info_init_params(struct spi_nor >> *nor) >> params->set_4byte = spansion_set_4byte; >> params->setup = spi_nor_default_setup; >> /* Default to 16-bit Write Status (01h) Command */ >> - nor->flags |= SNOR_F_HAS_16BIT_SR; >> +// nor->flags |= SNOR_F_HAS_16BIT_SR; > > This means that bfpt is not parsed. You can confirm this by running with the > patch from below. > > diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c > index f4afe123e9dc..735cd5202598 100644 > --- a/drivers/mtd/spi-nor/spi-nor.c > +++ b/drivers/mtd/spi-nor/spi-nor.c > @@ -3545,8 +3545,11 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor, > return err; > > /* Fix endianness of the BFPT DWORDs. */ > - for (i = 0; i < BFPT_DWORD_MAX; i++) > + for (i = 0; i < BFPT_DWORD_MAX; i++) { > bfpt.dwords[i] = le32_to_cpu(bfpt.dwords[i]); > + dev_err(nor->dev, "bfpt.dwords[%d] = %08x\n", i, > + bfpt.dwords[i]); > + } > > /* Number of address bytes. */ > switch (bfpt.dwords[BFPT_DWORD(1)] & BFPT_DWORD1_ADDRESS_BYTES_MASK) { > @@ -3649,6 +3652,9 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor, > params->page_size >>= BFPT_DWORD11_PAGE_SIZE_SHIFT; > params->page_size = 1U << params->page_size; > > + dev_err(nor->dev, "bfpt.dwords[BFPT_DWORD(15)] & BFPT_DWORD15_QER_MASK = > %08x\n", > + bfpt.dwords[BFPT_DWORD(15)] & BFPT_DWORD15_QER_MASK); > + > /* Quad Enable Requirements. */ > switch (bfpt.dwords[BFPT_DWORD(15)] & BFPT_DWORD15_QER_MASK) { > case BFPT_DWORD15_QER_NONE: > john@ubuntu:~$ dmesg | grep spi [ 14.935740] spi-nor spi-PRP0001:00: bfpt.dwords[0] = fff920e5 [ 14.941480] spi-nor spi-PRP0001:00: bfpt.dwords[1] = 07ffffff [ 14.947215] spi-nor spi-PRP0001:00: bfpt.dwords[2] = 6b27eb29 [ 14.952949] spi-nor spi-PRP0001:00: bfpt.dwords[3] = bb273b27 [ 14.958683] spi-nor spi-PRP0001:00: bfpt.dwords[4] = ffffffff [ 14.964417] spi-nor spi-PRP0001:00: bfpt.dwords[5] = bb27ffff [ 14.970150] spi-nor spi-PRP0001:00: bfpt.dwords[6] = eb29ffff [ 14.975884] spi-nor spi-PRP0001:00: bfpt.dwords[7] = d810200c [ 14.981618] spi-nor spi-PRP0001:00: bfpt.dwords[8] = 0000520f [ 14.987351] spi-nor spi-PRP0001:00: bfpt.dwords[9] = 00994a24 [ 14.993085] spi-nor spi-PRP0001:00: bfpt.dwords[10] = c9038e8b [ 14.998906] spi-nor spi-PRP0001:00: bfpt.dwords[11] = 382701ac [ 15.004726] spi-nor spi-PRP0001:00: bfpt.dwords[12] = 757a757a [ 15.010547] spi-nor spi-PRP0001:00: bfpt.dwords[13] = 5cd5bdfb [ 15.016367] spi-nor spi-PRP0001:00: bfpt.dwords[14] = ff820f4a [ 15.022187] spi-nor spi-PRP0001:00: bfpt.dwords[15] = 00003d81 Thanks, John ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: flash_lock issue for n25q 128mb spi nor part 2019-12-03 10:31 ` John Garry @ 2019-12-03 11:07 ` Tudor.Ambarus 2019-12-03 11:44 ` John Garry 0 siblings, 1 reply; 27+ messages in thread From: Tudor.Ambarus @ 2019-12-03 11:07 UTC (permalink / raw) To: john.garry, linux-mtd; +Cc: broonie, chenxiang66 On 12/3/19 12:31 PM, John Garry wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > On 03/12/2019 09:45, Tudor.Ambarus@microchip.com wrote: >> Hi, John, > > Hi Tudor, > > Thanks for getting back to me. > >> >> On 12/2/19 7:28 PM, John Garry wrote: >>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe >>> >>> Hi guys, >>> >>> I am testing the following driver on top of Linus' master branch with a >>> n25q128a11 part: >> >> I understand the you enabled locking support for this flash on your side, >> because in mainline it is not yet there. >> >> Be aware that this flash has BP3 support, and in mainline we support for now >> just BP0-BP2, so just a partial lock can be achieved, and it depends on what you >> tried to lock. It will be helpful to tell how you enabled the locking and what >> commands did you use. > > So I am simply using flash_lock -l /dev/mtd0, which will try to lock the > whole device. Now I see what you're saying about BP3 and how complete > locking is not available. > >> >>> >>> https://lore.kernel.org/linux-mtd/1572886297-45400-3-git-send-email-john.garry@huawei.com/ >>> >>> I am finding flash lock is not working. Even after a “successful” lock, >>> flash_lock is reporting flash is unlocked. And I can still write to the >>> flash. >>> >>> I find that since 39d1e3340c73 ("mtd: spi-nor: Fix clearing of QE bit on >>> lock()/unlock()"), we're using a 16b SR for this part in the driver, but >>> this part only has a 8b SR. >>> >>> This hack fixes the problem for me: >>> >>> @ -4691,7 +4691,7 @@ static void spi_nor_info_init_params(struct spi_nor >>> *nor) >>> params->set_4byte = spansion_set_4byte; >>> params->setup = spi_nor_default_setup; >>> /* Default to 16-bit Write Status (01h) Command */ >>> - nor->flags |= SNOR_F_HAS_16BIT_SR; >>> +// nor->flags |= SNOR_F_HAS_16BIT_SR; >> >> This means that bfpt is not parsed. You can confirm this by running with the >> patch from below. >> >> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c >> index f4afe123e9dc..735cd5202598 100644 >> --- a/drivers/mtd/spi-nor/spi-nor.c >> +++ b/drivers/mtd/spi-nor/spi-nor.c >> @@ -3545,8 +3545,11 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor, >> return err; >> >> /* Fix endianness of the BFPT DWORDs. */ >> - for (i = 0; i < BFPT_DWORD_MAX; i++) >> + for (i = 0; i < BFPT_DWORD_MAX; i++) { >> bfpt.dwords[i] = le32_to_cpu(bfpt.dwords[i]); >> + dev_err(nor->dev, "bfpt.dwords[%d] = %08x\n", i, >> + bfpt.dwords[i]); >> + } >> >> /* Number of address bytes. */ >> switch (bfpt.dwords[BFPT_DWORD(1)] & BFPT_DWORD1_ADDRESS_BYTES_MASK) { >> @@ -3649,6 +3652,9 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor, >> params->page_size >>= BFPT_DWORD11_PAGE_SIZE_SHIFT; >> params->page_size = 1U << params->page_size; >> >> + dev_err(nor->dev, "bfpt.dwords[BFPT_DWORD(15)] & BFPT_DWORD15_QER_MASK = >> %08x\n", >> + bfpt.dwords[BFPT_DWORD(15)] & BFPT_DWORD15_QER_MASK); >> + >> /* Quad Enable Requirements. */ >> switch (bfpt.dwords[BFPT_DWORD(15)] & BFPT_DWORD15_QER_MASK) { >> case BFPT_DWORD15_QER_NONE: >> > > john@ubuntu:~$ dmesg | grep spi > [ 14.935740] spi-nor spi-PRP0001:00: bfpt.dwords[0] = fff920e5 > [ 14.941480] spi-nor spi-PRP0001:00: bfpt.dwords[1] = 07ffffff > [ 14.947215] spi-nor spi-PRP0001:00: bfpt.dwords[2] = 6b27eb29 > [ 14.952949] spi-nor spi-PRP0001:00: bfpt.dwords[3] = bb273b27 > [ 14.958683] spi-nor spi-PRP0001:00: bfpt.dwords[4] = ffffffff > [ 14.964417] spi-nor spi-PRP0001:00: bfpt.dwords[5] = bb27ffff > [ 14.970150] spi-nor spi-PRP0001:00: bfpt.dwords[6] = eb29ffff > [ 14.975884] spi-nor spi-PRP0001:00: bfpt.dwords[7] = d810200c > [ 14.981618] spi-nor spi-PRP0001:00: bfpt.dwords[8] = 0000520f > [ 14.987351] spi-nor spi-PRP0001:00: bfpt.dwords[9] = 00994a24 > [ 14.993085] spi-nor spi-PRP0001:00: bfpt.dwords[10] = c9038e8b > [ 14.998906] spi-nor spi-PRP0001:00: bfpt.dwords[11] = 382701ac > [ 15.004726] spi-nor spi-PRP0001:00: bfpt.dwords[12] = 757a757a > [ 15.010547] spi-nor spi-PRP0001:00: bfpt.dwords[13] = 5cd5bdfb > [ 15.016367] spi-nor spi-PRP0001:00: bfpt.dwords[14] = ff820f4a > [ 15.022187] spi-nor spi-PRP0001:00: bfpt.dwords[15] = 00003d81 > This falls into the BFPT_DWORD15_QER_NONE case. If there's a correspondence between the BFPT_DWORD15_QER_NONE and no 16-bit Write SR support, we can clear the flag there, but JESD216D does not specify this :(. Please enable CONFIG_DEBUG on top of the file to see dev_dbg messages. I'll try to find a micron flash in the meantime. Cheers, ta ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: flash_lock issue for n25q 128mb spi nor part 2019-12-03 11:07 ` Tudor.Ambarus @ 2019-12-03 11:44 ` John Garry 2019-12-03 12:05 ` Tudor.Ambarus 0 siblings, 1 reply; 27+ messages in thread From: John Garry @ 2019-12-03 11:44 UTC (permalink / raw) To: Tudor.Ambarus, linux-mtd; +Cc: broonie, chenxiang66 On 03/12/2019 11:07, Tudor.Ambarus@microchip.com wrote: >>> + bfpt.dwords[BFPT_DWORD(15)] & BFPT_DWORD15_QER_MASK); >>> + >>> /* Quad Enable Requirements. */ >>> switch (bfpt.dwords[BFPT_DWORD(15)] & BFPT_DWORD15_QER_MASK) { >>> c Hi Tudor, ase BFPT_DWORD15_QER_NONE: >>> >> john@ubuntu:~$ dmesg | grep spi >> [ 14.935740] spi-nor spi-PRP0001:00: bfpt.dwords[0] = fff920e5 >> [ 14.941480] spi-nor spi-PRP0001:00: bfpt.dwords[1] = 07ffffff >> [ 14.947215] spi-nor spi-PRP0001:00: bfpt.dwords[2] = 6b27eb29 >> [ 14.952949] spi-nor spi-PRP0001:00: bfpt.dwords[3] = bb273b27 >> [ 14.958683] spi-nor spi-PRP0001:00: bfpt.dwords[4] = ffffffff >> [ 14.964417] spi-nor spi-PRP0001:00: bfpt.dwords[5] = bb27ffff >> [ 14.970150] spi-nor spi-PRP0001:00: bfpt.dwords[6] = eb29ffff >> [ 14.975884] spi-nor spi-PRP0001:00: bfpt.dwords[7] = d810200c >> [ 14.981618] spi-nor spi-PRP0001:00: bfpt.dwords[8] = 0000520f >> [ 14.987351] spi-nor spi-PRP0001:00: bfpt.dwords[9] = 00994a24 >> [ 14.993085] spi-nor spi-PRP0001:00: bfpt.dwords[10] = c9038e8b >> [ 14.998906] spi-nor spi-PRP0001:00: bfpt.dwords[11] = 382701ac >> [ 15.004726] spi-nor spi-PRP0001:00: bfpt.dwords[12] = 757a757a >> [ 15.010547] spi-nor spi-PRP0001:00: bfpt.dwords[13] = 5cd5bdfb >> [ 15.016367] spi-nor spi-PRP0001:00: bfpt.dwords[14] = ff820f4a >> [ 15.022187] spi-nor spi-PRP0001:00: bfpt.dwords[15] = 00003d81 >> > This falls into the BFPT_DWORD15_QER_NONE case. If there's a correspondence > between the BFPT_DWORD15_QER_NONE and no 16-bit Write SR support, we can clear > the flag there, but JESD216D does not specify this:(. > > Please enable CONFIG_DEBUG on top of the file to see dev_dbg messages. I'll try > to find a micron flash in the meantime. > From what you say, the spec seems to be in conflict/unspecified with this micron part in terms of using a 16b SR, so I am not sure what a log can reveal. Anyway here's what I got: john@ubuntu:~$ dmesg | grep spi [ 14.934354] spi-nor spi-PRP0001:00: bfpt.dwords[0] = fff920e5 [ 14.940094] spi-nor spi-PRP0001:00: bfpt.dwords[1] = 07ffffff [ 14.945828] spi-nor spi-PRP0001:00: bfpt.dwords[2] = 6b27eb29 [ 14.951563] spi-nor spi-PRP0001:00: bfpt.dwords[3] = bb273b27 [ 14.957297] spi-nor spi-PRP0001:00: bfpt.dwords[4] = ffffffff [ 14.963030] spi-nor spi-PRP0001:00: bfpt.dwords[5] = bb27ffff [ 14.968764] spi-nor spi-PRP0001:00: bfpt.dwords[6] = eb29ffff [ 14.974498] spi-nor spi-PRP0001:00: bfpt.dwords[7] = d810200c [ 14.980231] spi-nor spi-PRP0001:00: bfpt.dwords[8] = 0000520f [ 14.985965] spi-nor spi-PRP0001:00: bfpt.dwords[9] = 00994a24 [ 14.991699] spi-nor spi-PRP0001:00: bfpt.dwords[10] = c9038e8b [ 14.997520] spi-nor spi-PRP0001:00: bfpt.dwords[11] = 382701ac [ 15.003340] spi-nor spi-PRP0001:00: bfpt.dwords[12] = 757a757a [ 15.009161] spi-nor spi-PRP0001:00: bfpt.dwords[13] = 5cd5bdfb [ 15.014981] spi-nor spi-PRP0001:00: bfpt.dwords[14] = ff820f4a [ 15.020801] spi-nor spi-PRP0001:00: bfpt.dwords[15] = 00003d81 [ 15.026624] spi-nor spi-PRP0001:00: bfpt.dwords[BFPT_DWORD(15)] & BFPT_DWORD15_QER_MASK =00000000 [ 15.035515] spi-nor spi-PRP0001:00: n25q128a11 (16384 Kbytes) [ 15.041250] spi-nor spi-PRP0001:00: mtd .name = spi-PRP0001:00, .size = 0x1000000 (16MiB), .erasesize = 0x00001000 (4KiB) .numeraseregions = 0 [ 23.338259] spi-nor spi-PRP0001:00: from 0x00ff0000, len 512 [ 23.338380] spi-nor spi-PRP0001:00: from 0x00ff0200, len 512 [ 23.338497] spi-nor spi-PRP0001:00: from 0x00ff0400, len 512 [ 23.338612] spi-nor spi-PRP0001:00: from 0x00ff0600, len 512 [ 23.338728] spi-nor spi-PRP0001:00: from 0x00ff0800, len 512 [ 23.338844] spi-nor spi-PRP0001:00: from 0x00ff0a00, len 512 [ 23.338959] spi-nor spi-PRP0001:00: from 0x00ff0c00, len 512 [ 23.339075] spi-nor spi-PRP0001:00: from 0x00ff0e00, len 512 [ 23.339237] spi-nor spi-PRP0001:00: from 0x00ffe000, len 512 [ 23.339359] spi-nor spi-PRP0001:00: from 0x00ffe200, len 512 [ 23.339474] spi-nor spi-PRP0001:00: from 0x00ffe400, len 512 [ 23.339589] spi-nor spi-PRP0001:00: from 0x00ffe600, len 512 [ 23.339704] spi-nor spi-PRP0001:00: from 0x00ffe800, len 512 [ 23.339818] spi-nor spi-PRP0001:00: from 0x00ffea00, len 512 [ 23.339933] spi-nor spi-PRP0001:00: from 0x00ffec00, len 512 [ 23.340047] spi-nor spi-PRP0001:00: from 0x00ffee00, len 512 [ 23.340188] spi-nor spi-PRP0001:00: from 0x00000000, len 512 [ 23.340306] spi-nor spi-PRP0001:00: from 0x00000200, len 512 [ 23.340422] spi-nor spi-PRP0001:00: from 0x00000400, len 512 [ 23.340539] spi-nor spi-PRP0001:00: from 0x00000600, len 512 [ 23.3401:00: from 0x00000c00, len 512 [ 23.340999] spi-nor spi-PRP0001:00: from 0x00000e00, len 512 [ 23.341139] spi-nor spi-PRP0001:00: from 0x00001000, len 512 [ 23.341257] spi-nor spi-PRP0001:00: from 0x00001200, len 512 [snip] [ 23.410270] spi-nor spi-PRP0001:00: from 0x0003ce00, len 512 [ 23.410386] spi-nor spi-PRP0001:00: from 0x0003d000, len 512 [ 23.410502] spi-nor spi-PRP0001:00: from 0x0003d200, len 512 [ 23.410617] spi-nor spi-PRP0001:00: from 0x0003d400, len 512 [ 23.410733] spi-nor spi-PRP0001:00: from 0x0003d600, len 512 [ 23.410849] spi-nor spi-PRP0001:00: from 0x0003d800, len 512 [ 23.410964] spi-nor spi-PRP0001:00: from 0x0003da00, len 512 [ 23.411080] spi-nor spi-PRP0001:00: from 0x0003dc00, len 512 [ 23.411196] spi-nor spi-PRP0001:00: from 0x0003de00, len 512 [ 23.411312] spi-nor spi-PRP0001:00: from 0x0003e000, len 512 [ 23.411428] spi-nor spi-PRP0001:00: from 0x0003e200, len 512 [ 23.411543] spi-nor spi-PRP0001:00: from 0x0003e400, len 512 [ 23.411659] spi-nor spi-PRP0001:00: from 0x0003e600, len 512 [ 23.411775] spi-nor spi-PRP0001:00: from 0x0003e800, len 512 [ 23.411891] spi-nor spi-PRP0001:00: from 0x0003ea00, len 512 [ 23.412007] spi-nor spi-PRP0001:00: from 0x0003ec00, len 512 [ 23.412122] spi-nor spi-PRP0001:00: from 0x0003ee00, len 512 [ 23.412239] spi-nor spi-PRP0001:00: from 0x0003f000, len 512 [ 23.412354] spi-nor spi-PRP0001:00: from 0x0003f200, len 512 [ 23.412470] spi-nor spi-PRP0001:00: from 0x0003f400, len 512 [ 23.412587] spi-nor spi-PRP0001:00: from 0x0003f600, len 512 [ 23.412704] spi-nor spi-PRP0001:00: from 0x0003f800, len 512 [ 23.412819] spi-nor spi-PRP0001:00: from 0x0003fa00, len 512 [ 23.412935] spi-nor spi-PRP00 512 [ 23.413257] spi-nor spi-PRP0001:00: from 0x00040000, len 512 [ 23.413375] spi-nor spi-PRP0001:00: from 0x00040200, len 512 [ 23.413490] spi-nor spi-PRP0001:00: from 0x00040400, len 512 [ 23.413605] spi-nor spi-PRP0001:00: from 0x00040600, len 512 [ 23.413720] spi-nor spi-PRP0001:00: from 0x00040800, len 512 [ 23.413835] spi-nor spi-PRP0001:00: from 0x00040a00, len 512 [ 23.413950] spi-nor spi-PRP0001:00: from 0x00040c00, len 512 [ 23.414065] spi-nor spi-PRP0001:00: from 0x00040e00, len 512 [ 23.414210] spi-nor spi-PRP0001:00: from 0x00200000, len 512 [ 23.414328] spi-nor spi-PRP0001:00: from 0x00200200, len 512 [ 23.414444] spi-nor spi-PRP0001:00: from 0x00200400, len 512 [ 23.414560] spi-nor spi-PRP0001:00: from 0x00200600, len 512 [ 23.414676] spi-nor spi-PRP0001:00: from 0x00200800, len 512 [ 23.414792] spi-nor spi-PRP0001:00: from 0x00200a00, len 512 [ 23.414908] spi-nor spi-PRP0001:00: from 0x00200c00, len 512 [ 23.415024] spi-nor spi-PRP0001:00: from 0x00200e00, len 512 john@ubuntu:~$ john@ubuntu:~$ john@ubuntu:~$ john@ubuntu:~$ sudo su [sudo] password for john: root@ubuntu:/home/john# flash_lock -i /dev/mtd0 Device: /dev/mtd0 Start: 0 Len: 0x1000000 Lock status: unlocked Return code: 0 root@ubuntu:/home/john# flash_lock -l /dev/mtd0 0x800000 0x800000 root@ubuntu:/home/john# flash_lock -i /dev/mtd0 Device: /dev/mtd0 Start: 0 Len: 0x1000000 Lock status: unlocked Return code: 0 root@ubuntu:/home/john# Thanks, John ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: flash_lock issue for n25q 128mb spi nor part 2019-12-03 11:44 ` John Garry @ 2019-12-03 12:05 ` Tudor.Ambarus 2019-12-03 12:27 ` Tudor.Ambarus 0 siblings, 1 reply; 27+ messages in thread From: Tudor.Ambarus @ 2019-12-03 12:05 UTC (permalink / raw) To: john.garry, linux-mtd; +Cc: broonie, chenxiang66 On 12/3/19 1:44 PM, John Garry wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > On 03/12/2019 11:07, Tudor.Ambarus@microchip.com wrote: >>>> + bfpt.dwords[BFPT_DWORD(15)] & BFPT_DWORD15_QER_MASK); >>>> + >>>> /* Quad Enable Requirements. */ >>>> switch (bfpt.dwords[BFPT_DWORD(15)] & BFPT_DWORD15_QER_MASK) { >>>> c > > Hi Tudor, > > ase BFPT_DWORD15_QER_NONE: >>>> >>> john@ubuntu:~$ dmesg | grep spi >>> [ 14.935740] spi-nor spi-PRP0001:00: bfpt.dwords[0] = fff920e5 >>> [ 14.941480] spi-nor spi-PRP0001:00: bfpt.dwords[1] = 07ffffff >>> [ 14.947215] spi-nor spi-PRP0001:00: bfpt.dwords[2] = 6b27eb29 >>> [ 14.952949] spi-nor spi-PRP0001:00: bfpt.dwords[3] = bb273b27 >>> [ 14.958683] spi-nor spi-PRP0001:00: bfpt.dwords[4] = ffffffff >>> [ 14.964417] spi-nor spi-PRP0001:00: bfpt.dwords[5] = bb27ffff >>> [ 14.970150] spi-nor spi-PRP0001:00: bfpt.dwords[6] = eb29ffff >>> [ 14.975884] spi-nor spi-PRP0001:00: bfpt.dwords[7] = d810200c >>> [ 14.981618] spi-nor spi-PRP0001:00: bfpt.dwords[8] = 0000520f >>> [ 14.987351] spi-nor spi-PRP0001:00: bfpt.dwords[9] = 00994a24 >>> [ 14.993085] spi-nor spi-PRP0001:00: bfpt.dwords[10] = c9038e8b >>> [ 14.998906] spi-nor spi-PRP0001:00: bfpt.dwords[11] = 382701ac >>> [ 15.004726] spi-nor spi-PRP0001:00: bfpt.dwords[12] = 757a757a >>> [ 15.010547] spi-nor spi-PRP0001:00: bfpt.dwords[13] = 5cd5bdfb >>> [ 15.016367] spi-nor spi-PRP0001:00: bfpt.dwords[14] = ff820f4a >>> [ 15.022187] spi-nor spi-PRP0001:00: bfpt.dwords[15] = 00003d81 >>> >> This falls into the BFPT_DWORD15_QER_NONE case. If there's a correspondence >> between the BFPT_DWORD15_QER_NONE and no 16-bit Write SR support, we can clear >> the flag there, but JESD216D does not specify this:(. >> >> Please enable CONFIG_DEBUG on top of the file to see dev_dbg messages. I'll try >> to find a micron flash in the meantime. >> > > From what you say, the spec seems to be in conflict/unspecified with > this micron part in terms of using a 16b SR, so I am not sure what a log > can reveal. > > Anyway here's what I got: > > john@ubuntu:~$ dmesg | grep spi > [ 14.934354] spi-nor spi-PRP0001:00: bfpt.dwords[0] = fff920e5 > [ 14.940094] spi-nor spi-PRP0001:00: bfpt.dwords[1] = 07ffffff > [ 14.945828] spi-nor spi-PRP0001:00: bfpt.dwords[2] = 6b27eb29 > [ 14.951563] spi-nor spi-PRP0001:00: bfpt.dwords[3] = bb273b27 > [ 14.957297] spi-nor spi-PRP0001:00: bfpt.dwords[4] = ffffffff > [ 14.963030] spi-nor spi-PRP0001:00: bfpt.dwords[5] = bb27ffff > [ 14.968764] spi-nor spi-PRP0001:00: bfpt.dwords[6] = eb29ffff > [ 14.974498] spi-nor spi-PRP0001:00: bfpt.dwords[7] = d810200c > [ 14.980231] spi-nor spi-PRP0001:00: bfpt.dwords[8] = 0000520f > [ 14.985965] spi-nor spi-PRP0001:00: bfpt.dwords[9] = 00994a24 > [ 14.991699] spi-nor spi-PRP0001:00: bfpt.dwords[10] = c9038e8b > [ 14.997520] spi-nor spi-PRP0001:00: bfpt.dwords[11] = 382701ac > [ 15.003340] spi-nor spi-PRP0001:00: bfpt.dwords[12] = 757a757a > [ 15.009161] spi-nor spi-PRP0001:00: bfpt.dwords[13] = 5cd5bdfb > [ 15.014981] spi-nor spi-PRP0001:00: bfpt.dwords[14] = ff820f4a > [ 15.020801] spi-nor spi-PRP0001:00: bfpt.dwords[15] = 00003d81 > [ 15.026624] spi-nor spi-PRP0001:00: bfpt.dwords[BFPT_DWORD(15)] & > BFPT_DWORD15_QER_MASK =00000000 > [ 15.035515] spi-nor spi-PRP0001:00: n25q128a11 (16384 Kbytes) > [ 15.041250] spi-nor spi-PRP0001:00: mtd .name = spi-PRP0001:00, .size > = 0x1000000 (16MiB), .erasesize = 0x00001000 (4KiB) .numeraseregions = 0 > [ 23.338259] spi-nor spi-PRP0001:00: from 0x00ff0000, len 512 > [ 23.338380] spi-nor spi-PRP0001:00: from 0x00ff0200, len 512 > [ 23.338497] spi-nor spi-PRP0001:00: from 0x00ff0400, len 512 > [ 23.338612] spi-nor spi-PRP0001:00: from 0x00ff0600, len 512 > [ 23.338728] spi-nor spi-PRP0001:00: from 0x00ff0800, len 512 > [ 23.338844] spi-nor spi-PRP0001:00: from 0x00ff0a00, len 512 > [ 23.338959] spi-nor spi-PRP0001:00: from 0x00ff0c00, len 512 > [ 23.339075] spi-nor spi-PRP0001:00: from 0x00ff0e00, len 512 > [ 23.339237] spi-nor spi-PRP0001:00: from 0x00ffe000, len 512 > [ 23.339359] spi-nor spi-PRP0001:00: from 0x00ffe200, len 512 > [ 23.339474] spi-nor spi-PRP0001:00: from 0x00ffe400, len 512 > [ 23.339589] spi-nor spi-PRP0001:00: from 0x00ffe600, len 512 > [ 23.339704] spi-nor spi-PRP0001:00: from 0x00ffe800, len 512 > [ 23.339818] spi-nor spi-PRP0001:00: from 0x00ffea00, len 512 > [ 23.339933] spi-nor spi-PRP0001:00: from 0x00ffec00, len 512 > [ 23.340047] spi-nor spi-PRP0001:00: from 0x00ffee00, len 512 > [ 23.340188] spi-nor spi-PRP0001:00: from 0x00000000, len 512 > [ 23.340306] spi-nor spi-PRP0001:00: from 0x00000200, len 512 > [ 23.340422] spi-nor spi-PRP0001:00: from 0x00000400, len 512 > [ 23.340539] spi-nor spi-PRP0001:00: from 0x00000600, len 512 > [ 23.3401:00: from 0x00000c00, len 512 > [ 23.340999] spi-nor spi-PRP0001:00: from 0x00000e00, len 512 > [ 23.341139] spi-nor spi-PRP0001:00: from 0x00001000, len 512 > [ 23.341257] spi-nor spi-PRP0001:00: from 0x00001200, len 512 > > [snip] > > [ 23.410270] spi-nor spi-PRP0001:00: from 0x0003ce00, len 512 > [ 23.410386] spi-nor spi-PRP0001:00: from 0x0003d000, len 512 > [ 23.410502] spi-nor spi-PRP0001:00: from 0x0003d200, len 512 > [ 23.410617] spi-nor spi-PRP0001:00: from 0x0003d400, len 512 > [ 23.410733] spi-nor spi-PRP0001:00: from 0x0003d600, len 512 > [ 23.410849] spi-nor spi-PRP0001:00: from 0x0003d800, len 512 > [ 23.410964] spi-nor spi-PRP0001:00: from 0x0003da00, len 512 > [ 23.411080] spi-nor spi-PRP0001:00: from 0x0003dc00, len 512 > [ 23.411196] spi-nor spi-PRP0001:00: from 0x0003de00, len 512 > [ 23.411312] spi-nor spi-PRP0001:00: from 0x0003e000, len 512 > [ 23.411428] spi-nor spi-PRP0001:00: from 0x0003e200, len 512 > [ 23.411543] spi-nor spi-PRP0001:00: from 0x0003e400, len 512 > [ 23.411659] spi-nor spi-PRP0001:00: from 0x0003e600, len 512 > [ 23.411775] spi-nor spi-PRP0001:00: from 0x0003e800, len 512 > [ 23.411891] spi-nor spi-PRP0001:00: from 0x0003ea00, len 512 > [ 23.412007] spi-nor spi-PRP0001:00: from 0x0003ec00, len 512 > [ 23.412122] spi-nor spi-PRP0001:00: from 0x0003ee00, len 512 > [ 23.412239] spi-nor spi-PRP0001:00: from 0x0003f000, len 512 > [ 23.412354] spi-nor spi-PRP0001:00: from 0x0003f200, len 512 > [ 23.412470] spi-nor spi-PRP0001:00: from 0x0003f400, len 512 > [ 23.412587] spi-nor spi-PRP0001:00: from 0x0003f600, len 512 > [ 23.412704] spi-nor spi-PRP0001:00: from 0x0003f800, len 512 > [ 23.412819] spi-nor spi-PRP0001:00: from 0x0003fa00, len 512 > [ 23.412935] spi-nor spi-PRP00 512 > [ 23.413257] spi-nor spi-PRP0001:00: from 0x00040000, len 512 > [ 23.413375] spi-nor spi-PRP0001:00: from 0x00040200, len 512 > [ 23.413490] spi-nor spi-PRP0001:00: from 0x00040400, len 512 > [ 23.413605] spi-nor spi-PRP0001:00: from 0x00040600, len 512 > [ 23.413720] spi-nor spi-PRP0001:00: from 0x00040800, len 512 > [ 23.413835] spi-nor spi-PRP0001:00: from 0x00040a00, len 512 > [ 23.413950] spi-nor spi-PRP0001:00: from 0x00040c00, len 512 > [ 23.414065] spi-nor spi-PRP0001:00: from 0x00040e00, len 512 > [ 23.414210] spi-nor spi-PRP0001:00: from 0x00200000, len 512 > [ 23.414328] spi-nor spi-PRP0001:00: from 0x00200200, len 512 > [ 23.414444] spi-nor spi-PRP0001:00: from 0x00200400, len 512 > [ 23.414560] spi-nor spi-PRP0001:00: from 0x00200600, len 512 > [ 23.414676] spi-nor spi-PRP0001:00: from 0x00200800, len 512 > [ 23.414792] spi-nor spi-PRP0001:00: from 0x00200a00, len 512 > [ 23.414908] spi-nor spi-PRP0001:00: from 0x00200c00, len 512 > [ 23.415024] spi-nor spi-PRP0001:00: from 0x00200e00, len 512 > john@ubuntu:~$ > john@ubuntu:~$ > john@ubuntu:~$ > john@ubuntu:~$ sudo su > [sudo] password for john: > root@ubuntu:/home/john# flash_lock -i /dev/mtd0 > Device: /dev/mtd0 > Start: 0 > Len: 0x1000000 > Lock status: unlocked > Return code: 0 > root@ubuntu:/home/john# flash_lock -l /dev/mtd0 0x800000 0x800000 > root@ubuntu:/home/john# flash_lock -i /dev/mtd0 > Device: /dev/mtd0 > Start: 0 > Len: 0x1000000 > Lock status: unlocked > Return code: 0 > root@ubuntu:/home/john# > this should reveal what happens after a lock, with the 16 bit status reg write: diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c index f4afe123e9dc..f1490c7b5cb9 100644 --- a/drivers/mtd/spi-nor/spi-nor.c +++ b/drivers/mtd/spi-nor/spi-nor.c @@ -1033,10 +1033,19 @@ static int spi_nor_write_16bit_sr_and_check(struct spi_nor *nor, u8 sr1) sr_cr[0] = sr1; + dev_err(nor->dev, "before write: sr_cr[0] = %02x, sr_cr[1] = %02x\n", + sr_cr[0], sr_cr[1]); + ret = spi_nor_write_sr(nor, sr_cr, 2); if (ret) return ret; + ret = spi_nor_read_sr(nor, &sr_cr[0]); + if (ret) + return ret; + + dev_err(nor->dev, "read back sr1: sr_cr[0] = %02x\n", sr_cr[0]); + if (nor->flags & SNOR_F_NO_READ_CR) return 0; @@ -1046,6 +1055,8 @@ static int spi_nor_write_16bit_sr_and_check(struct spi_nor *nor, u8 sr1) if (ret) return ret; + dev_err(nor->dev, "read back sr2: sr_cr[1] = %02x\n", sr_cr[1]); + if (cr_written != sr_cr[1]) { dev_dbg(nor->dev, "CR: read back test failed\n"); return -EIO; ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: flash_lock issue for n25q 128mb spi nor part 2019-12-03 12:05 ` Tudor.Ambarus @ 2019-12-03 12:27 ` Tudor.Ambarus 2019-12-03 12:35 ` John Garry 0 siblings, 1 reply; 27+ messages in thread From: Tudor.Ambarus @ 2019-12-03 12:27 UTC (permalink / raw) To: john.garry, linux-mtd; +Cc: broonie, chenxiang66 On 12/3/19 2:05 PM, Tudor.Ambarus@microchip.com wrote: > diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c > index f4afe123e9dc..f1490c7b5cb9 100644 > --- a/drivers/mtd/spi-nor/spi-nor.c > +++ b/drivers/mtd/spi-nor/spi-nor.c > @@ -1033,10 +1033,19 @@ static int spi_nor_write_16bit_sr_and_check(struct spi_nor *nor, u8 sr1) > > sr_cr[0] = sr1; > > + dev_err(nor->dev, "before write: sr_cr[0] = %02x, sr_cr[1] = %02x\n", > + sr_cr[0], sr_cr[1]); > + > ret = spi_nor_write_sr(nor, sr_cr, 2); > if (ret) > return ret; > > + ret = spi_nor_read_sr(nor, &sr_cr[0]); > + if (ret) > + return ret; > + > + dev_err(nor->dev, "read back sr1: sr_cr[0] = %02x\n", sr_cr[0]); > + > if (nor->flags & SNOR_F_NO_READ_CR) > return 0; > > @@ -1046,6 +1055,8 @@ static int spi_nor_write_16bit_sr_and_check(struct spi_nor *nor, u8 sr1) > if (ret) > return ret; > > + dev_err(nor->dev, "read back sr2: sr_cr[1] = %02x\n", sr_cr[1]); > + > if (cr_written != sr_cr[1]) { > dev_dbg(nor->dev, "CR: read back test failed\ On n25q256a I obtain: root@sama5d2-xplained-sd:~# flash_lock -l /dev/mtd1 spi-nor spi1.0: before write: sr_cr[0] = 9e, sr_cr[1] = ff spi-nor spi1.0: read back sr1: sr_cr[0] = 02 spi-nor spi1.0: read back sr2: sr_cr[1] = ff the 16 bit write SR does not execute correctly and the WE remains set. If neither of the micron flashes do not support the 16 bit write SR, we can add a condition based on MFR. Let me check few datasheets. ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: flash_lock issue for n25q 128mb spi nor part 2019-12-03 12:27 ` Tudor.Ambarus @ 2019-12-03 12:35 ` John Garry 2019-12-03 13:57 ` John Garry 2019-12-03 14:16 ` [PATCH] mtd: spi-nor: Fix the write Status Register on micron flashes Tudor.Ambarus 0 siblings, 2 replies; 27+ messages in thread From: John Garry @ 2019-12-03 12:35 UTC (permalink / raw) To: Tudor.Ambarus, linux-mtd; +Cc: broonie, chenxiang66 On 03/12/2019 12:27, Tudor.Ambarus@microchip.com wrote: > > > On 12/3/19 2:05 PM, Tudor.Ambarus@microchip.com wrote: >> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c >> index f4afe123e9dc..f1490c7b5cb9 100644 >> --- a/drivers/mtd/spi-nor/spi-nor.c >> +++ b/drivers/mtd/spi-nor/spi-nor.c >> @@ -1033,10 +1033,19 @@ static int spi_nor_write_16bit_sr_and_check(struct spi_nor *nor, u8 sr1) >> >> sr_cr[0] = sr1; >> >> + dev_err(nor->dev, "before write: sr_cr[0] = %02x, sr_cr[1] = %02x\n", >> + sr_cr[0], sr_cr[1]); >> + >> ret = spi_nor_write_sr(nor, sr_cr, 2); >> if (ret) >> return ret; >> >> + ret = spi_nor_read_sr(nor, &sr_cr[0]); >> + if (ret) >> + return ret; >> + >> + dev_err(nor->dev, "read back sr1: sr_cr[0] = %02x\n", sr_cr[0]); >> + >> if (nor->flags & SNOR_F_NO_READ_CR) >> return 0; >> >> @@ -1046,6 +1055,8 @@ static int spi_nor_write_16bit_sr_and_check(struct spi_nor *nor, u8 sr1) >> if (ret) >> return ret; >> >> + dev_err(nor->dev, "read back sr2: sr_cr[1] = %02x\n", sr_cr[1]); >> + >> if (cr_written != sr_cr[1]) { >> dev_dbg(nor->dev, "CR: read back test failed\ > > On n25q256a I obtain: > > root@sama5d2-xplained-sd:~# flash_lock -l /dev/mtd1 > spi-nor spi1.0: before write: sr_cr[0] = 9e, sr_cr[1] = ff > spi-nor spi1.0: read back sr1: sr_cr[0] = 02 > spi-nor spi1.0: read back sr2: sr_cr[1] = ff > So here is what I get: root@ubuntu:/home/john# flash_lock -l /dev/mtd0 [ 109.037492] spi-nor spi-PRP0001:00: before write: sr_cr[0] = 9e, sr_cr[1] = 00 [ 109.044769] spi-nor spi-PRP0001:00: read back sr1: sr_cr[0] = 02 [ 109.050790] spi-nor spi-PRP0001:00: read back sr2: sr_cr[1] = 00 > the 16 bit write SR does not execute correctly and the WE remains set. If > neither of the micron flashes do not support the 16 bit write SR, we can add a > condition based on MFR. Let me check few datasheets. > OK, thanks. John ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: flash_lock issue for n25q 128mb spi nor part 2019-12-03 12:35 ` John Garry @ 2019-12-03 13:57 ` John Garry 2019-12-03 14:44 ` Tudor.Ambarus 2019-12-03 14:16 ` [PATCH] mtd: spi-nor: Fix the write Status Register on micron flashes Tudor.Ambarus 1 sibling, 1 reply; 27+ messages in thread From: John Garry @ 2019-12-03 13:57 UTC (permalink / raw) To: Tudor.Ambarus, linux-mtd; +Cc: broonie, fengsheng5 On 03/12/2019 12:35, John Garry wrote: > On 03/12/2019 12:27, Tudor.Ambarus@microchip.com wrote: >> >> >> On 12/3/19 2:05 PM, Tudor.Ambarus@microchip.com wrote: >>> diff --git a/drivers/mtd/spi-nor/spi-nor.c >>> b/drivers/mtd/spi-nor/spi-nor.c >>> index f4afe123e9dc..f1490c7b5cb9 100644 >>> --- a/drivers/mtd/spi-nor/spi-nor.c >>> +++ b/drivers/mtd/spi-nor/spi-nor.c >>> @@ -1033,10 +1033,19 @@ static int >>> spi_nor_write_16bit_sr_and_check(struct spi_nor *nor, u8 sr1) >>> >>> sr_cr[0] = sr1; >>> >>> + dev_err(nor->dev, "before write: sr_cr[0] = %02x, sr_cr[1] = >>> %02x\n", >>> + sr_cr[0], sr_cr[1]); >>> + >>> ret = spi_nor_write_sr(nor, sr_cr, 2); >>> if (ret) >>> return ret; >>> >>> + ret = spi_nor_read_sr(nor, &sr_cr[0]); >>> + if (ret) >>> + return ret; >>> + >>> + dev_err(nor->dev, "read back sr1: sr_cr[0] = %02x\n", sr_cr[0]); >>> + >>> if (nor->flags & SNOR_F_NO_READ_CR) >>> return 0; >>> >>> @@ -1046,6 +1055,8 @@ static int >>> spi_nor_write_16bit_sr_and_check(struct spi_nor *nor, u8 sr1) >>> if (ret) >>> return ret; >>> >>> + dev_err(nor->dev, "read back sr2: sr_cr[1] = %02x\n", sr_cr[1]); >>> + >>> if (cr_written != sr_cr[1]) { >>> dev_dbg(nor->dev, "CR: read back test failed\ >> >> On n25q256a I obtain: >> >> root@sama5d2-xplained-sd:~# flash_lock -l /dev/mtd1 >> spi-nor spi1.0: before write: sr_cr[0] = 9e, sr_cr[1] = ff >> spi-nor spi1.0: read back sr1: sr_cr[0] = 02 >> spi-nor spi1.0: read back sr2: sr_cr[1] = ff >> > > So here is what I get: > > root@ubuntu:/home/john# flash_lock -l /dev/mtd0 > [ 109.037492] spi-nor spi-PRP0001:00: before write: sr_cr[0] = 9e, > sr_cr[1] = 00 > [ 109.044769] spi-nor spi-PRP0001:00: read back sr1: sr_cr[0] = 02 > [ 109.050790] spi-nor spi-PRP0001:00: read back sr2: sr_cr[1] = 00 > > >> the 16 bit write SR does not execute correctly and the WE remains set. If >> neither of the micron flashes do not support the 16 bit write SR, we >> can add a >> condition based on MFR. Let me check few datasheets. >> > > OK, thanks. Hi Tudor, Something else which I wanted to ask is why we don't check the FSR after a write command for that part? Without it, if the flash is locked, a flash write reports no error, which could be maybe misleading. A more real problem is that I find that SR.WEL is not cleared when we attempt to write when the flash is locked, and this causes issues when we attempt to reset the SR. Here's an example flow (with my hack to stop using 16b SR method): root@ubuntu:/home/john# flash_lock -l /dev/mtd0 root@ubuntu:/home/john# mtd_debug erase /dev/mtd0 0xe00000 4096 [ 69.650642] spi-nor spi-PRP0001:00: at 0xe00000, len 4096 Erased 4096 bytes from address 0x00e00000 in flash root@ubuntu:/home/john# mtd_debug write /dev/mtd0 0xe00000 4096 dump4096 [ 77.093755] spi-nor spi-PRP0001:00: to 0x00e00000, len 4096 Copied 4096 bytes from dump4096 to address 0x00e00000 in flash root@ubuntu:/home/john# mtd_debug read /dev/mtd0 0xe00000 4096 temp [ 82.162445] spi-nor spi-PRP0001:00: from 0x00e00000, len 4096 Copied 4096 bytes from address 0x00e00000 in flash to temp root@ubuntu:/home/john# flash_lock -u /dev/mtd0 [ 87.558435] spi-nor spi-PRP0001:00: SR1: read back test failed flash_lock: error!: could not unlock device: /dev/mtd0 error 5 (Input/output error) root@ubuntu:/home/john# Unlock reports an error as the the read back test in spi_nor_write_sr1_and_check() fails as the SR.WEL has never been cleared. I'm just saying this while it's fresh in my head - I don't want to seem pushy :) Thanks, John ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: flash_lock issue for n25q 128mb spi nor part 2019-12-03 13:57 ` John Garry @ 2019-12-03 14:44 ` Tudor.Ambarus 2019-12-03 15:29 ` John Garry 0 siblings, 1 reply; 27+ messages in thread From: Tudor.Ambarus @ 2019-12-03 14:44 UTC (permalink / raw) To: john.garry, linux-mtd; +Cc: broonie, fengsheng5 > Hi Tudor, Hi, John, > > Something else which I wanted to ask is why we don't check the FSR after > a write command for that part? no particular reason, no one asked for it :). > > Without it, if the flash is locked, a flash write reports no error, > which could be maybe misleading. true > > A more real problem is that I find that SR.WEL is not cleared when we > attempt to write when the flash is locked, and this causes issues when > we attempt to reset the SR. > > Here's an example flow (with my hack to stop using 16b SR method): > > root@ubuntu:/home/john# flash_lock -l /dev/mtd0 > root@ubuntu:/home/john# mtd_debug erase /dev/mtd0 0xe00000 4096 > [ 69.650642] spi-nor spi-PRP0001:00: at 0xe00000, len 4096 > Erased 4096 bytes from address 0x00e00000 in flash > root@ubuntu:/home/john# mtd_debug write /dev/mtd0 0xe00000 4096 dump4096 > [ 77.093755] spi-nor spi-PRP0001:00: to 0x00e00000, len 4096 > Copied 4096 bytes from dump4096 to address 0x00e00000 in flash > root@ubuntu:/home/john# mtd_debug read /dev/mtd0 0xe00000 4096 temp > [ 82.162445] spi-nor spi-PRP0001:00: from 0x00e00000, len 4096 > Copied 4096 bytes from address 0x00e00000 in flash to temp > root@ubuntu:/home/john# flash_lock -u /dev/mtd0 > [ 87.558435] spi-nor spi-PRP0001:00: SR1: read back test failed > flash_lock: error!: could not unlock device: /dev/mtd0 > > error 5 (Input/output error) > root@ubuntu:/home/john# > > Unlock reports an error as the the read back test in > spi_nor_write_sr1_and_check() fails as the SR.WEL has never been cleared. > Interesting. Does the following do the trick? - { "n25q128a11", INFO(0x20bb18, 0, 64 * 1024, 256, SECT_4K | SPI_NOR_QUAD_READ) }, + { "n25q128a11", INFO(0x20bb18, 0, 64 * 1024, 256, SECT_4K | USE_FSR | + SPI_NOR_QUAD_READ) }, This can of course be extended to all micron flashes, if all support FSR, but some documentation work has to be made. > I'm just saying this while it's fresh in my head - I don't want to seem > pushy :) No worries, this is a good initiative, thanks. Cheers, ta ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: flash_lock issue for n25q 128mb spi nor part 2019-12-03 14:44 ` Tudor.Ambarus @ 2019-12-03 15:29 ` John Garry 2019-12-04 11:10 ` John Garry 0 siblings, 1 reply; 27+ messages in thread From: John Garry @ 2019-12-03 15:29 UTC (permalink / raw) To: Tudor.Ambarus, linux-mtd; +Cc: broonie, fengsheng5 >> >> Here's an example flow (with my hack to stop using 16b SR method): >> >> root@ubuntu:/home/john# flash_lock -l /dev/mtd0 >> root@ubuntu:/home/john# mtd_debug erase /dev/mtd0 0xe00000 4096 >> [ 69.650642] spi-nor spi-PRP0001:00: at 0xe00000, len 4096 >> Erased 4096 bytes from address 0x00e00000 in flash >> root@ubuntu:/home/john# mtd_debug write /dev/mtd0 0xe00000 4096 dump4096 >> [ 77.093755] spi-nor spi-PRP0001:00: to 0x00e00000, len 4096 >> Copied 4096 bytes from dump4096 to address 0x00e00000 in flash >> root@ubuntu:/home/john# mtd_debug read /dev/mtd0 0xe00000 4096 temp >> [ 82.162445] spi-nor spi-PRP0001:00: from 0x00e00000, len 4096 >> Copied 4096 bytes from address 0x00e00000 in flash to temp >> root@ubuntu:/home/john# flash_lock -u /dev/mtd0 >> [ 87.558435] spi-nor spi-PRP0001:00: SR1: read back test failed >> flash_lock: error!: could not unlock device: /dev/mtd0 >> >> error 5 (Input/output error) >> root@ubuntu:/home/john# >> >> Unlock reports an error as the the read back test in >> spi_nor_write_sr1_and_check() fails as the SR.WEL has never been cleared. >> > > Interesting. I note that spi_nor_erase() exits with a call to spi_nor_write_disable(), yet spi_nor_write() doesn't - maybe we should add a similar call there. > > > Does the following do the trick? > > - { "n25q128a11", INFO(0x20bb18, 0, 64 * 1024, 256, SECT_4K | > SPI_NOR_QUAD_READ) }, > + { "n25q128a11", INFO(0x20bb18, 0, 64 * 1024, 256, SECT_4K | USE_FSR | > + SPI_NOR_QUAD_READ) }, > I'll give it a spin - along with your SR lock fix - when I get back on the HW again. > This can of course be extended to all micron flashes, if all support FSR, but > some documentation work has to be made. > >> I'm just saying this while it's fresh in my head - I don't want to seem >> pushy :) > > No worries, this is a good initiative, thanks. > Much appreciated, John ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: flash_lock issue for n25q 128mb spi nor part 2019-12-03 15:29 ` John Garry @ 2019-12-04 11:10 ` John Garry 2019-12-16 18:09 ` Tudor.Ambarus 0 siblings, 1 reply; 27+ messages in thread From: John Garry @ 2019-12-04 11:10 UTC (permalink / raw) To: Tudor.Ambarus, linux-mtd; +Cc: broonie, fengsheng5 On 03/12/2019 15:29, John Garry wrote: >>> >>> Here's an example flow (with my hack to stop using 16b SR method): >>> >>> root@ubuntu:/home/john# flash_lock -l /dev/mtd0 >>> root@ubuntu:/home/john# mtd_debug erase /dev/mtd0 0xe00000 4096 >>> [ 69.650642] spi-nor spi-PRP0001:00: at 0xe00000, len 4096 >>> Erased 4096 bytes from address 0x00e00000 in flash >>> root@ubuntu:/home/john# mtd_debug write /dev/mtd0 0xe00000 4096 dump4096 >>> [ 77.093755] spi-nor spi-PRP0001:00: to 0x00e00000, len 4096 >>> Copied 4096 bytes from dump4096 to address 0x00e00000 in flash >>> root@ubuntu:/home/john# mtd_debug read /dev/mtd0 0xe00000 4096 temp >>> [ 82.162445] spi-nor spi-PRP0001:00: from 0x00e00000, len 4096 >>> Copied 4096 bytes from address 0x00e00000 in flash to temp >>> root@ubuntu:/home/john# flash_lock -u /dev/mtd0 >>> [ 87.558435] spi-nor spi-PRP0001:00: SR1: read back test failed >>> flash_lock: error!: could not unlock device: /dev/mtd0 >>> >>> error 5 (Input/output error) >>> root@ubuntu:/home/john# >>> >>> Unlock reports an error as the the read back test in >>> spi_nor_write_sr1_and_check() fails as the SR.WEL has never been >>> cleared. >>> >> >> Interesting. > > I note that spi_nor_erase() exits with a call to > spi_nor_write_disable(), yet spi_nor_write() doesn't - maybe we should > add a similar call there. So this remedies that issue for me: commit c90de583d81f7c5c6cb1c8c021108a0e7e244b91 (HEAD) Author: John Garry <john.garry@huawei.com> Date: Wed Dec 4 10:26:49 2019 +0000 Ensure we clear SR.WEL for any write failure due to locking diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c index aeb3ad2dbfb8..3f12335eb20c 100644 --- a/drivers/mtd/spi-nor/spi-nor.c +++ b/drivers/mtd/spi-nor/spi-nor.c @@ -2875,6 +2875,8 @@ static int spi_nor_write(struct mtd_info *mtd, loff_t to, size_t len, i += written; } + ret = spi_nor_write_disable(nor); + write_err: spi_nor_unlock_and_unprep(nor, SPI_NOR_OPS_WRITE); return ret; > >> >> >> Does the following do the trick? >> >> - { "n25q128a11", INFO(0x20bb18, 0, 64 * 1024, 256, SECT_4K | >> SPI_NOR_QUAD_READ) }, >> + { "n25q128a11", INFO(0x20bb18, 0, 64 * 1024, 256, SECT_4K | >> USE_FSR | >> + SPI_NOR_QUAD_READ) }, >> Yes, that works and it's nice to not have the silent failures: root@ubuntu:/home/john# flash_lock -i /dev/mtd0 Device: /dev/mtd0 Start: 0 Len: 0x1000000 Lock status: locked Return code: 1 root@ubuntu:/home/john# root@ubuntu:/home/john# mtd_debug erase /dev/mtd0 0xe00000 4096 [ 45.023353] spi-nor spi0.0: Erase operation failed. [ 45.028257] spi-nor spi0.0: Attempted to modify a protected sector. MEMERASE: Input/output error mtd_debug write /dev/mtd0 0xe00000 4096 dump4096 [ 50.212013] spi-nor spi0.0: Program operation failed. [ 50.217084] spi-nor spi0.0: Attempted to modify a protected sector. file_to_flash: write, size 0x1000, n 0x1000 write(): Input/output error root@ubuntu:/home/john# mtd_debug read /dev/mtd0 0xe00000 4096 temp Copied 4096 bytes from address 0x00e00000 in flash to temp root@ubuntu:/home/john# flash_lock -u /dev/mtd0 flash_lock: error!: could not unlock device: /dev/mtd0 error 5 (Input/output error) root@ubuntu:/home/john# flash_lock -i /dev/mtd0 Device: /dev/mtd0 Start: 0 Len: 0x1000000 Lock status: unlocked Return code: 0 root@ubuntu:/home/john# flash_lock -u /dev/mtd0 root@ubuntu:/home/john# But, as you may see, it seems that my change to spi_nor_write() is still required to stop the first unlock failure message, but it needs to be relocated to after write_err label, as we now jump there for spi_nor_wait_till_ready() failure. I guess the equivalent relocation is also required for spi_nor_erase(). Or maybe spi_nor_wait_till_ready() should clear this flag always. > > I'll give it a spin - along with your SR lock fix - when I get back on > the HW again. > >> This can of course be extended to all micron flashes, if all support >> FSR, but >> some documentation work has to be made. >> Thanks, John ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: flash_lock issue for n25q 128mb spi nor part 2019-12-04 11:10 ` John Garry @ 2019-12-16 18:09 ` Tudor.Ambarus 2019-12-17 8:57 ` Vignesh Raghavendra 0 siblings, 1 reply; 27+ messages in thread From: Tudor.Ambarus @ 2019-12-16 18:09 UTC (permalink / raw) To: john.garry, linux-mtd; +Cc: broonie, fengsheng5 Hi, John, On 12/4/19 1:10 PM, John Garry wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > On 03/12/2019 15:29, John Garry wrote: >>>> >>>> Here's an example flow (with my hack to stop using 16b SR method): >>>> >>>> root@ubuntu:/home/john# flash_lock -l /dev/mtd0 >>>> root@ubuntu:/home/john# mtd_debug erase /dev/mtd0 0xe00000 4096 >>>> [ 69.650642] spi-nor spi-PRP0001:00: at 0xe00000, len 4096 >>>> Erased 4096 bytes from address 0x00e00000 in flash >>>> root@ubuntu:/home/john# mtd_debug write /dev/mtd0 0xe00000 4096 dump4096 >>>> [ 77.093755] spi-nor spi-PRP0001:00: to 0x00e00000, len 4096 >>>> Copied 4096 bytes from dump4096 to address 0x00e00000 in flash >>>> root@ubuntu:/home/john# mtd_debug read /dev/mtd0 0xe00000 4096 temp >>>> [ 82.162445] spi-nor spi-PRP0001:00: from 0x00e00000, len 4096 >>>> Copied 4096 bytes from address 0x00e00000 in flash to temp >>>> root@ubuntu:/home/john# flash_lock -u /dev/mtd0 >>>> [ 87.558435] spi-nor spi-PRP0001:00: SR1: read back test failed >>>> flash_lock: error!: could not unlock device: /dev/mtd0 >>>> >>>> error 5 (Input/output error) >>>> root@ubuntu:/home/john# >>>> >>>> Unlock reports an error as the the read back test in >>>> spi_nor_write_sr1_and_check() fails as the SR.WEL has never been >>>> cleared. >>>> >>> >>> Interesting. >> >> I note that spi_nor_erase() exits with a call to >> spi_nor_write_disable(), yet spi_nor_write() doesn't - maybe we should >> add a similar call there. > > So this remedies that issue for me: > > commit c90de583d81f7c5c6cb1c8c021108a0e7e244b91 (HEAD) > Author: John Garry <john.garry@huawei.com> > Date: Wed Dec 4 10:26:49 2019 +0000 > > Ensure we clear SR.WEL for any write failure due to locking > > diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c > index aeb3ad2dbfb8..3f12335eb20c 100644 > --- a/drivers/mtd/spi-nor/spi-nor.c > +++ b/drivers/mtd/spi-nor/spi-nor.c > @@ -2875,6 +2875,8 @@ static int spi_nor_write(struct mtd_info *mtd, > loff_t to, size_t len, > i += written; > } > > + ret = spi_nor_write_disable(nor); > + > write_err: > spi_nor_unlock_and_unprep(nor, SPI_NOR_OPS_WRITE); > return ret; > > >> >>> >>> >>> Does the following do the trick? >>> >>> - { "n25q128a11", INFO(0x20bb18, 0, 64 * 1024, 256, SECT_4K | >>> SPI_NOR_QUAD_READ) }, >>> + { "n25q128a11", INFO(0x20bb18, 0, 64 * 1024, 256, SECT_4K | >>> USE_FSR | >>> + SPI_NOR_QUAD_READ) }, >>> > > Yes, that works and it's nice to not have the silent failures: > > root@ubuntu:/home/john# flash_lock -i /dev/mtd0 > Device: /dev/mtd0 > Start: 0 > Len: 0x1000000 > Lock status: locked > Return code: 1 > root@ubuntu:/home/john# > root@ubuntu:/home/john# mtd_debug erase /dev/mtd0 0xe00000 4096 > [ 45.023353] spi-nor spi0.0: Erase operation failed. > [ 45.028257] spi-nor spi0.0: Attempted to modify a protected sector. > MEMERASE: Input/output error > mtd_debug write /dev/mtd0 0xe00000 4096 dump4096 > [ 50.212013] spi-nor spi0.0: Program operation failed. > [ 50.217084] spi-nor spi0.0: Attempted to modify a protected sector. > file_to_flash: write, size 0x1000, n 0x1000 > write(): Input/output error > root@ubuntu:/home/john# mtd_debug read /dev/mtd0 0xe00000 4096 temp > Copied 4096 bytes from address 0x00e00000 in flash to temp > root@ubuntu:/home/john# flash_lock -u /dev/mtd0 > flash_lock: error!: could not unlock device: /dev/mtd0 > > error 5 (Input/output error) > root@ubuntu:/home/john# flash_lock -i /dev/mtd0 > Device: /dev/mtd0 > Start: 0 > Len: 0x1000000 > Lock status: unlocked > Return code: 0 > root@ubuntu:/home/john# flash_lock -u /dev/mtd0 > root@ubuntu:/home/john# > > But, as you may see, it seems that my change to spi_nor_write() is still > required to stop the first unlock failure message, but it needs to be > relocated to after write_err label, as we now jump there for > spi_nor_wait_till_ready() failure. I guess the equivalent relocation is > also required for spi_nor_erase(). > > Or maybe spi_nor_wait_till_ready() should clear this flag always. I reproduced this on a n25q256a, with both erase and write. Did a lock, an erase or write, and then the unlock raises an error on the read back test: it receives 0x02 to write (the prev operation let the SR.WE set to 1), and after write, it reads back 0x00 (which is correct, WE is de-asserted). What is pretty strange is that Micron says about erase or program operations that: "When the operation is in progress, the write in progress bit is set to 1. The write enable latch bit is cleared to 0, whether the operation is successful or not". So what I guess it happens, is that when an erase/write command tries to modify a software protected area, the flash completely ignores the command, so no Write In Progress, and no clearing of the WE. An idea would be to clear the WE in spi_nor_sr/fsr_ready() when errors. I'll continue to debug/study this. Cheers, ta ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: flash_lock issue for n25q 128mb spi nor part 2019-12-16 18:09 ` Tudor.Ambarus @ 2019-12-17 8:57 ` Vignesh Raghavendra 2019-12-17 10:09 ` John Garry 2020-01-09 10:36 ` John Garry 0 siblings, 2 replies; 27+ messages in thread From: Vignesh Raghavendra @ 2019-12-17 8:57 UTC (permalink / raw) To: Tudor.Ambarus, john.garry, linux-mtd; +Cc: broonie, fengsheng5 Hi Tudor, On 12/16/2019 11:39 PM, Tudor.Ambarus@microchip.com wrote: [...] >> >> But, as you may see, it seems that my change to spi_nor_write() is still >> required to stop the first unlock failure message, but it needs to be >> relocated to after write_err label, as we now jump there for >> spi_nor_wait_till_ready() failure. I guess the equivalent relocation is >> also required for spi_nor_erase(). >> >> Or maybe spi_nor_wait_till_ready() should clear this flag always. > > I reproduced this on a n25q256a, with both erase and write. Did a lock, > an erase or write, and then the unlock raises an error on the read back test: > it receives 0x02 to write (the prev operation let the SR.WE set to 1), > and after write, it reads back 0x00 (which is correct, WE is de-asserted). > > What is pretty strange is that Micron says about erase or program operations > that: "When the operation is in progress, the write in progress bit is set > to 1. The write enable latch bit is cleared to 0, whether the operation is > successful or not". > > So what I guess it happens, is that when an erase/write command tries to > modify a software protected area, the flash completely ignores the command, > so no Write In Progress, and no clearing of the WE. > From PROGRAM Operations section of mt25q datasheet: " When a command is applied to a protected sector, the command is not executed, the write enable latch bit remains set to 1, and flag status register bits 1 and 4 are set." So, Data sheet is quite clear about this and SW would need to clear WEL (if required) after write failure. Regards Vignesh ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: flash_lock issue for n25q 128mb spi nor part 2019-12-17 8:57 ` Vignesh Raghavendra @ 2019-12-17 10:09 ` John Garry 2020-01-09 10:36 ` John Garry 1 sibling, 0 replies; 27+ messages in thread From: John Garry @ 2019-12-17 10:09 UTC (permalink / raw) To: Vignesh Raghavendra, Tudor.Ambarus, linux-mtd; +Cc: broonie, fengsheng5 On 17/12/2019 08:57, Vignesh Raghavendra wrote: > Hi Tudor, > > On 12/16/2019 11:39 PM, Tudor.Ambarus@microchip.com wrote: > [...] > >>> >>> But, as you may see, it seems that my change to spi_nor_write() is still >>> required to stop the first unlock failure message, but it needs to be >>> relocated to after write_err label, as we now jump there for >>> spi_nor_wait_till_ready() failure. I guess the equivalent relocation is >>> also required for spi_nor_erase(). >>> >>> Or maybe spi_nor_wait_till_ready() should clear this flag always. >> >> I reproduced this on a n25q256a, with both erase and write. Did a lock, >> an erase or write, and then the unlock raises an error on the read back test: >> it receives 0x02 to write (the prev operation let the SR.WE set to 1), >> and after write, it reads back 0x00 (which is correct, WE is de-asserted). >> >> What is pretty strange is that Micron says about erase or program operations >> that: "When the operation is in progress, the write in progress bit is set >> to 1. The write enable latch bit is cleared to 0, whether the operation is >> successful or not". >> >> So what I guess it happens, is that when an erase/write command tries to >> modify a software protected area, the flash completely ignores the command, >> so no Write In Progress, and no clearing of the WE. >> > > >>From PROGRAM Operations section of mt25q datasheet: > > " When a command is applied to a protected sector, the command is not > executed, the write enable latch bit remains set to 1, and flag status > register bits 1 and 4 are set." > > So, Data sheet is quite clear about this and SW would need to clear WEL > (if required) after write failure. Yeah, I think that the datasheet could have been better written in that regard. So about "whether the operation is successful or not" - I wonder what they mean by "successful". Does it mean simply that the command completes, even with error? Thanks, John ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: flash_lock issue for n25q 128mb spi nor part 2019-12-17 8:57 ` Vignesh Raghavendra 2019-12-17 10:09 ` John Garry @ 2020-01-09 10:36 ` John Garry 2020-01-10 11:51 ` Tudor.Ambarus 1 sibling, 1 reply; 27+ messages in thread From: John Garry @ 2020-01-09 10:36 UTC (permalink / raw) To: Vignesh Raghavendra, Tudor.Ambarus, linux-mtd; +Cc: broonie, fengsheng5 On 17/12/2019 08:57, Vignesh Raghavendra wrote: > Hi Tudor, > > On 12/16/2019 11:39 PM, Tudor.Ambarus@microchip.com wrote: > [...] > >>> >>> But, as you may see, it seems that my change to spi_nor_write() is still >>> required to stop the first unlock failure message, but it needs to be >>> relocated to after write_err label, as we now jump there for >>> spi_nor_wait_till_ready() failure. I guess the equivalent relocation is >>> also required for spi_nor_erase(). >>> >>> Or maybe spi_nor_wait_till_ready() should clear this flag always. >> >> I reproduced this on a n25q256a, with both erase and write. Did a lock, >> an erase or write, and then the unlock raises an error on the read back test: >> it receives 0x02 to write (the prev operation let the SR.WE set to 1), >> and after write, it reads back 0x00 (which is correct, WE is de-asserted). >> >> What is pretty strange is that Micron says about erase or program operations >> that: "When the operation is in progress, the write in progress bit is set >> to 1. The write enable latch bit is cleared to 0, whether the operation is >> successful or not". >> >> So what I guess it happens, is that when an erase/write command tries to >> modify a software protected area, the flash completely ignores the command, >> so no Write In Progress, and no clearing of the WE. >> > > >>From PROGRAM Operations section of mt25q datasheet: > > " When a command is applied to a protected sector, the command is not > executed, the write enable latch bit remains set to 1, and flag status > register bits 1 and 4 are set." > > So, Data sheet is quite clear about this and SW would need to clear WEL > (if required) after write failure. > Hi guys, Apart from this issue, on another topic, is there any special reason which we don't support status register.BP3? Is it that it is not adjacent to BP2 in the register, so makes handling trickier? I ran these commands, which show how this is broken: root@ubuntu:/home/john# sudo flash_lock -i /dev/mtd0 Device: /dev/mtd0 Start: 0 Len: 0x1000000 Lock status: locked Return code: 1 root@ubuntu:/home/john# sudo mtd_debug write /dev/mtd0 0xf80000 4096 dump4096 [ 134.573819] spi-nor spi-PRP0001:00: Program operation failed. [ 134.579567] spi-nor spi-PRP0001:00: Attempted to modify a protected sector. file_to_flash: write, size 0x1000, n 0x1000 write(): Input/output error root@ubuntu:/home/john# sudo mtd_debug read /dev/mtd0 0x100000 4096 temp [ 105.657411] spi-nor spi-PRP0001:00: from 0x00100000, len 4096 Copied 4096 bytes from address 0x00100000 in flash to temp root@ubuntu:/home/john# hexdump temp 0000000 ffff ffff ffff ffff ffff ffff ffff ffff * 0001000 root@ubuntu:/home/john# sudo mtd_debug write /dev/mtd0 0x100000 4096 dump4096 [ 140.161265] spi-nor spi-PRP0001:00: to 0x00100000, len 4096 Copied 4096 bytes from dump4096 to address 0x00100000 in flash root@ubuntu:/home/john# sudo mtd_debug read /dev/mtd0 0x100000 4096 temp [ 150.697181] spi-nor spi-PRP0001:00: from 0x00100000, len 4096 Copied 4096 bytes from address 0x00100000 in flash to temp root@ubuntu:/home/john# diff temp dump4096 root@ubuntu:/home/john# Here is are told that all sectors for the chip are locked, which is untrue, and we could execute the write command successfully on the bottom half. Thanks, John ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: flash_lock issue for n25q 128mb spi nor part 2020-01-09 10:36 ` John Garry @ 2020-01-10 11:51 ` Tudor.Ambarus 2020-01-10 11:56 ` John Garry 2020-03-09 10:15 ` [RESEND PATCH 1/2] mtd: spi-nor: Clear WEL bit when erase or program errors occur Tudor.Ambarus 0 siblings, 2 replies; 27+ messages in thread From: Tudor.Ambarus @ 2020-01-10 11:51 UTC (permalink / raw) To: john.garry; +Cc: js07.lee, broonie, linux-mtd, vigneshr, fengsheng5 On Thursday, January 9, 2020 12:36:11 PM EET John Garry wrote: > Apart from this issue, on another topic, is there any special reason Ups, I forgot about this. I had a patch for it, let me find it. > which we don't support status register.BP3? Is it that it is not > adjacent to BP2 in the register, so makes handling trickier? Jungseung worked in the past for adding BP3 support. He have just sent a new patch set for addressing this, feedback is welcomed: https://patchwork.ozlabs.org/project/linux-mtd/list/?series=152401 Cheers, ta ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: flash_lock issue for n25q 128mb spi nor part 2020-01-10 11:51 ` Tudor.Ambarus @ 2020-01-10 11:56 ` John Garry 2020-01-15 9:28 ` John Garry 2020-03-09 10:15 ` [RESEND PATCH 1/2] mtd: spi-nor: Clear WEL bit when erase or program errors occur Tudor.Ambarus 1 sibling, 1 reply; 27+ messages in thread From: John Garry @ 2020-01-10 11:56 UTC (permalink / raw) To: Tudor.Ambarus; +Cc: js07.lee, broonie, linux-mtd, vigneshr, fengsheng5 On 10/01/2020 11:51, Tudor.Ambarus@microchip.com wrote: > On Thursday, January 9, 2020 12:36:11 PM EET John Garry wrote: >> Apart from this issue, on another topic, is there any special reason > > Ups, I forgot about this. I had a patch for it, let me find it. ok, cool > >> which we don't support status register.BP3? Is it that it is not >> adjacent to BP2 in the register, so makes handling trickier? > > Jungseung worked in the past for adding BP3 support. He have just sent a new > patch set for addressing this, feedback is welcomed: > https://patchwork.ozlabs.org/project/linux-mtd/list/?series=152401 oh, great. I'll have a look. Thanks, John ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: flash_lock issue for n25q 128mb spi nor part 2020-01-10 11:56 ` John Garry @ 2020-01-15 9:28 ` John Garry 0 siblings, 0 replies; 27+ messages in thread From: John Garry @ 2020-01-15 9:28 UTC (permalink / raw) To: Tudor.Ambarus Cc: vigneshr, chenxiang, fengsheng5, broonie, linux-mtd, js07.lee On 10/01/2020 11:56, John Garry wrote: > On 10/01/2020 11:51, Tudor.Ambarus@microchip.com wrote: >> On Thursday, January 9, 2020 12:36:11 PM EET John Garry wrote: >>> Apart from this issue, on another topic, is there any special reason >> >> Ups, I forgot about this. I had a patch for it, let me find it. > > ok, cool > >> >>> which we don't support status register.BP3? Is it that it is not >>> adjacent to BP2 in the register, so makes handling trickier? >> >> Jungseung worked in the past for adding BP3 support. He have just sent >> a new >> patch set for addressing this, feedback is welcomed: >> https://patchwork.ozlabs.org/project/linux-mtd/list/?series=152401 > > oh, great. I'll have a look. > > Thanks, > John Hi guys, It turns out now that we may need support a macronix part also - mx25u12835f. Do you know why flash locking is not supported yet in the kernel for macronix parts? Did no one get around to it? Or just a technical issue? Cheers, John ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 27+ messages in thread
* [RESEND PATCH 1/2] mtd: spi-nor: Clear WEL bit when erase or program errors occur 2020-01-10 11:51 ` Tudor.Ambarus 2020-01-10 11:56 ` John Garry @ 2020-03-09 10:15 ` Tudor.Ambarus 2020-03-09 10:15 ` [RESEND PATCH 2/2] mtd: spi-nor: Fix description of the sr_ready() return value Tudor.Ambarus 2020-03-09 15:04 ` [RESEND PATCH 1/2] mtd: spi-nor: Clear WEL bit when erase or program errors occur John Garry 1 sibling, 2 replies; 27+ messages in thread From: Tudor.Ambarus @ 2020-03-09 10:15 UTC (permalink / raw) To: john.garry, vigneshr, chenxiang66; +Cc: Tudor.Ambarus, linux-mtd, js07.lee From: Tudor Ambarus <tudor.ambarus@microchip.com> When an Erase or Program error occurs on a spansion/cypress or a micron flash, the WEL bit remains set to one and should be cleared with a WRDI command in order to protect against inadvertent writes that can possible corrupt the contents of the memory. Winbond, macronix, gd, etc., do not support the E_ERR and P_ERR bits in the Status Register and always clear the WEL bit regardless of the outcome of the erase or page program operation (ex w25q40bw, MX25L25635E). Issue a WRDI command when erase or page program errors occur. Reported-by: John Garry <john.garry@huawei.com> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com> --- drivers/mtd/spi-nor/spi-nor.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c index caf0c109cca0..4376d8ad5834 100644 --- a/drivers/mtd/spi-nor/spi-nor.c +++ b/drivers/mtd/spi-nor/spi-nor.c @@ -819,6 +819,17 @@ static int spi_nor_sr_ready(struct spi_nor *nor) dev_err(nor->dev, "Programming Error occurred\n"); spi_nor_clear_sr(nor); + + /* + * WEL bit remains set to one when an erase or page program + * error occurs. Issue a Write Disable command to protect + * against inadvertent writes that can possibly corrupt the + * contents of the memory. + */ + ret = spi_nor_write_disable(nor); + if (ret) + return ret; + return -EIO; } @@ -875,6 +886,17 @@ static int spi_nor_fsr_ready(struct spi_nor *nor) "Attempted to modify a protected sector.\n"); spi_nor_clear_fsr(nor); + + /* + * WEL bit remains set to one when an erase or page program + * error occurs. Issue a Write Disable command to protect + * against inadvertent writes that can possibly corrupt the + * contents of the memory. + */ + ret = spi_nor_write_disable(nor); + if (ret) + return ret; + return -EIO; } -- 2.23.0 ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [RESEND PATCH 2/2] mtd: spi-nor: Fix description of the sr_ready() return value 2020-03-09 10:15 ` [RESEND PATCH 1/2] mtd: spi-nor: Clear WEL bit when erase or program errors occur Tudor.Ambarus @ 2020-03-09 10:15 ` Tudor.Ambarus 2020-03-09 15:04 ` [RESEND PATCH 1/2] mtd: spi-nor: Clear WEL bit when erase or program errors occur John Garry 1 sibling, 0 replies; 27+ messages in thread From: Tudor.Ambarus @ 2020-03-09 10:15 UTC (permalink / raw) To: john.garry, vigneshr, chenxiang66; +Cc: Tudor.Ambarus, linux-mtd, js07.lee From: Tudor Ambarus <tudor.ambarus@microchip.com> The functions return 1 if ready, 0 if not ready, -errno on errors. Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com> --- drivers/mtd/spi-nor/spi-nor.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c index 4376d8ad5834..604043ec95cc 100644 --- a/drivers/mtd/spi-nor/spi-nor.c +++ b/drivers/mtd/spi-nor/spi-nor.c @@ -759,7 +759,7 @@ static int spi_nor_xread_sr(struct spi_nor *nor, u8 *sr) * flash is ready for new commands. * @nor: pointer to 'struct spi_nor'. * - * Return: 0 on success, -errno otherwise. + * Return: 1 if ready, 0 if not ready, -errno on errors. */ static int s3an_sr_ready(struct spi_nor *nor) { @@ -802,7 +802,7 @@ static void spi_nor_clear_sr(struct spi_nor *nor) * for new commands. * @nor: pointer to 'struct spi_nor'. * - * Return: 0 on success, -errno otherwise. + * Return: 1 if ready, 0 if not ready, -errno on errors. */ static int spi_nor_sr_ready(struct spi_nor *nor) { @@ -866,7 +866,7 @@ static void spi_nor_clear_fsr(struct spi_nor *nor) * ready for new commands. * @nor: pointer to 'struct spi_nor'. * - * Return: 0 on success, -errno otherwise. + * Return: 1 if ready, 0 if not ready, -errno on errors. */ static int spi_nor_fsr_ready(struct spi_nor *nor) { @@ -907,7 +907,7 @@ static int spi_nor_fsr_ready(struct spi_nor *nor) * spi_nor_ready() - Query the flash to see if it is ready for new commands. * @nor: pointer to 'struct spi_nor'. * - * Return: 0 on success, -errno otherwise. + * Return: 1 if ready, 0 if not ready, -errno on errors. */ static int spi_nor_ready(struct spi_nor *nor) { -- 2.23.0 ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [RESEND PATCH 1/2] mtd: spi-nor: Clear WEL bit when erase or program errors occur 2020-03-09 10:15 ` [RESEND PATCH 1/2] mtd: spi-nor: Clear WEL bit when erase or program errors occur Tudor.Ambarus 2020-03-09 10:15 ` [RESEND PATCH 2/2] mtd: spi-nor: Fix description of the sr_ready() return value Tudor.Ambarus @ 2020-03-09 15:04 ` John Garry 2020-03-23 17:58 ` Tudor.Ambarus 1 sibling, 1 reply; 27+ messages in thread From: John Garry @ 2020-03-09 15:04 UTC (permalink / raw) To: Tudor.Ambarus, vigneshr, chenxiang66; +Cc: linux-mtd, js07.lee On 09/03/2020 10:15, Tudor.Ambarus@microchip.com wrote: > From: Tudor Ambarus <tudor.ambarus@microchip.com> > > When an Erase or Program error occurs on a spansion/cypress or a > micron flash, the WEL bit remains set to one and should be cleared > with a WRDI command in order to protect against inadvertent writes > that can possible corrupt the contents of the memory. > > Winbond, macronix, gd, etc., do not support the E_ERR and P_ERR bits in the > Status Register and always clear the WEL bit regardless of the outcome > of the erase or page program operation (ex w25q40bw, MX25L25635E). > > Issue a WRDI command when erase or page program errors occur. > > Reported-by: John Garry <john.garry@huawei.com> Previously I would get this unlock complaint: root@ubuntu:/home/john# flash_lock -l /dev/mtd0 root@ubuntu:/home/john# sudo mtd_debug erase /dev/mtd0 0xf80000 4096 [ 167.458647] spi-nor spi-PRP0001:00: Erase operation failed. [ 167.464248] spi-nor spi-PRP0001:00: Attempted to modify a protected sector. MEMERASE: Input/output error root@ubuntu:/home/john# flash_lock -u /dev/mtd0 flash_lock: error!: could not unlock device: /dev/mtd0 error 5 (Input/output error) Now, no such issue. Thanks Tested-by: John Garry <john.garry@huawei.com> > Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com> > --- > drivers/mtd/spi-nor/spi-nor.c | 22 ++++++++++++++++++++++ > 1 file changed, 22 insertions(+) > > diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c > index caf0c109cca0..4376d8ad5834 100644 > --- a/drivers/mtd/spi-nor/spi-nor.c > +++ b/drivers/mtd/spi-nor/spi-nor.c > @@ -819,6 +819,17 @@ static int spi_nor_sr_ready(struct spi_nor *nor) > dev_err(nor->dev, "Programming Error occurred\n"); > > spi_nor_clear_sr(nor); > + > + /* > + * WEL bit remains set to one when an erase or page program > + * error occurs. Issue a Write Disable command to protect > + * against inadvertent writes that can possibly corrupt the > + * contents of the memory. > + */ > + ret = spi_nor_write_disable(nor); > + if (ret) > + return ret; > + > return -EIO; > } > > @@ -875,6 +886,17 @@ static int spi_nor_fsr_ready(struct spi_nor *nor) > "Attempted to modify a protected sector.\n"); > > spi_nor_clear_fsr(nor); > + > + /* > + * WEL bit remains set to one when an erase or page program > + * error occurs. Issue a Write Disable command to protect > + * against inadvertent writes that can possibly corrupt the > + * contents of the memory. > + */ > + ret = spi_nor_write_disable(nor); > + if (ret) > + return ret; > + > return -EIO; > } > > ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RESEND PATCH 1/2] mtd: spi-nor: Clear WEL bit when erase or program errors occur 2020-03-09 15:04 ` [RESEND PATCH 1/2] mtd: spi-nor: Clear WEL bit when erase or program errors occur John Garry @ 2020-03-23 17:58 ` Tudor.Ambarus 0 siblings, 0 replies; 27+ messages in thread From: Tudor.Ambarus @ 2020-03-23 17:58 UTC (permalink / raw) To: linux-mtd; +Cc: js07.lee, john.garry, vigneshr, chenxiang66 On Monday, March 9, 2020 5:04:53 PM EET John Garry wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the > content is safe > On 09/03/2020 10:15, Tudor.Ambarus@microchip.com wrote: > > From: Tudor Ambarus <tudor.ambarus@microchip.com> > > > > When an Erase or Program error occurs on a spansion/cypress or a > > micron flash, the WEL bit remains set to one and should be cleared > > with a WRDI command in order to protect against inadvertent writes > > that can possible corrupt the contents of the memory. > > > > Winbond, macronix, gd, etc., do not support the E_ERR and P_ERR bits in > > the > > Status Register and always clear the WEL bit regardless of the outcome > > of the erase or page program operation (ex w25q40bw, MX25L25635E). > > > > Issue a WRDI command when erase or page program errors occur. > > > > Reported-by: John Garry <john.garry@huawei.com> > > Previously I would get this unlock complaint: > > root@ubuntu:/home/john# flash_lock -l /dev/mtd0 > root@ubuntu:/home/john# sudo mtd_debug erase /dev/mtd0 0xf80000 4096 > [ 167.458647] spi-nor spi-PRP0001:00: Erase operation failed. > [ 167.464248] spi-nor spi-PRP0001:00: Attempted to modify a protected > sector. > MEMERASE: Input/output error > root@ubuntu:/home/john# flash_lock -u /dev/mtd0 > flash_lock: error!: could not unlock device: /dev/mtd0 > > error 5 (Input/output error) > > Now, no such issue. Thanks > > Tested-by: John Garry <john.garry@huawei.com> > > > Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com> > > --- > > > > drivers/mtd/spi-nor/spi-nor.c | 22 ++++++++++++++++++++++ > > 1 file changed, 22 insertions(+) Rebased and applied on top of spi-nor/next. Thanks. ta ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH] mtd: spi-nor: Fix the write Status Register on micron flashes 2019-12-03 12:35 ` John Garry 2019-12-03 13:57 ` John Garry @ 2019-12-03 14:16 ` Tudor.Ambarus 2019-12-03 14:50 ` [PATCH v2] mtd: spi-nor: Fix the writing of the " Tudor.Ambarus 1 sibling, 1 reply; 27+ messages in thread From: Tudor.Ambarus @ 2019-12-03 14:16 UTC (permalink / raw) To: john.garry, vigneshr, richard, miquel.raynal Cc: linux-mtd, linux-kernel, Tudor.Ambarus From: Tudor Ambarus <tudor.ambarus@microchip.com> Micron flashes do not support 16 bit writes on the Status Register. According to micron datasheets, when using the Write Status Register (01h) command, the chip select should be driven LOW and held LOW until the eighth bit of the last data byte has been latched in, after which it must be driven HIGH. If CS is not driven HIGH, the command is not executed, flag status register error bits are not set, and the write enable latch remains set to 1. This fixes the lock operations on micron flashes. Reported-by: John Garry <john.garry@huawei.com> Fixes: 39d1e3340c73 ("mtd: spi-nor: Fix clearing of QE bit on lock()/unlock()") Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.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 f1490c7b5cb9..7e41493f69d8 100644 --- a/drivers/mtd/spi-nor/spi-nor.c +++ b/drivers/mtd/spi-nor/spi-nor.c @@ -4607,6 +4607,7 @@ static void sst_set_default_init(struct spi_nor *nor) static void st_micron_set_default_init(struct spi_nor *nor) { nor->flags |= SNOR_F_HAS_LOCK; + nor->flags &= ~SNOR_F_HAS_16BIT_SR; nor->params.quad_enable = NULL; nor->params.set_4byte = st_micron_set_4byte; } -- 2.14.5 ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v2] mtd: spi-nor: Fix the writing of the Status Register on micron flashes 2019-12-03 14:16 ` [PATCH] mtd: spi-nor: Fix the write Status Register on micron flashes Tudor.Ambarus @ 2019-12-03 14:50 ` Tudor.Ambarus 2019-12-04 10:18 ` John Garry 2020-01-09 19:14 ` Miquel Raynal 0 siblings, 2 replies; 27+ messages in thread From: Tudor.Ambarus @ 2019-12-03 14:50 UTC (permalink / raw) To: john.garry, vigneshr, richard, miquel.raynal Cc: linux-mtd, linux-kernel, Tudor.Ambarus From: Tudor Ambarus <tudor.ambarus@microchip.com> Micron flashes do not support 16 bit writes on the Status Register. According to micron datasheets, when using the Write Status Register (01h) command, the chip select should be driven LOW and held LOW until the eighth bit of the last data byte has been latched in, after which it must be driven HIGH. If CS is not driven HIGH, the command is not executed, flag status register error bits are not set, and the write enable latch remains set to 1. This fixes the lock operations on micron flashes. Reported-by: John Garry <john.garry@huawei.com> Fixes: 39d1e3340c73 ("mtd: spi-nor: Fix clearing of QE bit on lock()/unlock()") Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com> --- v2: reword commit subject 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 f1490c7b5cb9..7e41493f69d8 100644 --- a/drivers/mtd/spi-nor/spi-nor.c +++ b/drivers/mtd/spi-nor/spi-nor.c @@ -4607,6 +4607,7 @@ static void sst_set_default_init(struct spi_nor *nor) static void st_micron_set_default_init(struct spi_nor *nor) { nor->flags |= SNOR_F_HAS_LOCK; + nor->flags &= ~SNOR_F_HAS_16BIT_SR; nor->params.quad_enable = NULL; nor->params.set_4byte = st_micron_set_4byte; } -- 2.14.5 ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH v2] mtd: spi-nor: Fix the writing of the Status Register on micron flashes 2019-12-03 14:50 ` [PATCH v2] mtd: spi-nor: Fix the writing of the " Tudor.Ambarus @ 2019-12-04 10:18 ` John Garry 2020-01-09 19:14 ` Miquel Raynal 1 sibling, 0 replies; 27+ messages in thread From: John Garry @ 2019-12-04 10:18 UTC (permalink / raw) To: Tudor.Ambarus, vigneshr, richard, miquel.raynal Cc: fengsheng5, linux-mtd, linux-kernel, chenxiang On 03/12/2019 14:50, Tudor.Ambarus@microchip.com wrote: > From: Tudor Ambarus <tudor.ambarus@microchip.com> > > Micron flashes do not support 16 bit writes on the Status Register. > According to micron datasheets, when using the Write Status Register > (01h) command, the chip select should be driven LOW and held LOW until > the eighth bit of the last data byte has been latched in, after which > it must be driven HIGH. If CS is not driven HIGH, the command is not > executed, flag status register error bits are not set, and the write enable > latch remains set to 1. This fixes the lock operations on micron flashes. > > Reported-by: John Garry <john.garry@huawei.com> > Fixes: 39d1e3340c73 ("mtd: spi-nor: Fix clearing of QE bit on lock()/unlock()") > Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com> Thanks, Tested-by: John Garry <john.garry@huawei.com> > --- > v2: reword commit subject > > 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 f1490c7b5cb9..7e41493f69d8 100644 > --- a/drivers/mtd/spi-nor/spi-nor.c > +++ b/drivers/mtd/spi-nor/spi-nor.c > @@ -4607,6 +4607,7 @@ static void sst_set_default_init(struct spi_nor *nor) > static void st_micron_set_default_init(struct spi_nor *nor) > { > nor->flags |= SNOR_F_HAS_LOCK; > + nor->flags &= ~SNOR_F_HAS_16BIT_SR; > nor->params.quad_enable = NULL; > nor->params.set_4byte = st_micron_set_4byte; > } > ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v2] mtd: spi-nor: Fix the writing of the Status Register on micron flashes 2019-12-03 14:50 ` [PATCH v2] mtd: spi-nor: Fix the writing of the " Tudor.Ambarus 2019-12-04 10:18 ` John Garry @ 2020-01-09 19:14 ` Miquel Raynal 1 sibling, 0 replies; 27+ messages in thread From: Miquel Raynal @ 2020-01-09 19:14 UTC (permalink / raw) To: Tudor.Ambarus, john.garry, vigneshr, richard, miquel.raynal Cc: linux-mtd, linux-kernel On Tue, 2019-12-03 at 14:50:01 UTC, wrote: > From: Tudor Ambarus <tudor.ambarus@microchip.com> > > Micron flashes do not support 16 bit writes on the Status Register. > According to micron datasheets, when using the Write Status Register > (01h) command, the chip select should be driven LOW and held LOW until > the eighth bit of the last data byte has been latched in, after which > it must be driven HIGH. If CS is not driven HIGH, the command is not > executed, flag status register error bits are not set, and the write enable > latch remains set to 1. This fixes the lock operations on micron flashes. > > Reported-by: John Garry <john.garry@huawei.com> > Fixes: 39d1e3340c73 ("mtd: spi-nor: Fix clearing of QE bit on lock()/unlock()") > Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com> > Tested-by: John Garry <john.garry@huawei.com> Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git mtd/fixes, thanks. Miquel ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2020-03-23 17:58 UTC | newest] Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-12-02 17:28 flash_lock issue for n25q 128mb spi nor part John Garry 2019-12-03 9:45 ` Tudor.Ambarus 2019-12-03 10:31 ` John Garry 2019-12-03 11:07 ` Tudor.Ambarus 2019-12-03 11:44 ` John Garry 2019-12-03 12:05 ` Tudor.Ambarus 2019-12-03 12:27 ` Tudor.Ambarus 2019-12-03 12:35 ` John Garry 2019-12-03 13:57 ` John Garry 2019-12-03 14:44 ` Tudor.Ambarus 2019-12-03 15:29 ` John Garry 2019-12-04 11:10 ` John Garry 2019-12-16 18:09 ` Tudor.Ambarus 2019-12-17 8:57 ` Vignesh Raghavendra 2019-12-17 10:09 ` John Garry 2020-01-09 10:36 ` John Garry 2020-01-10 11:51 ` Tudor.Ambarus 2020-01-10 11:56 ` John Garry 2020-01-15 9:28 ` John Garry 2020-03-09 10:15 ` [RESEND PATCH 1/2] mtd: spi-nor: Clear WEL bit when erase or program errors occur Tudor.Ambarus 2020-03-09 10:15 ` [RESEND PATCH 2/2] mtd: spi-nor: Fix description of the sr_ready() return value Tudor.Ambarus 2020-03-09 15:04 ` [RESEND PATCH 1/2] mtd: spi-nor: Clear WEL bit when erase or program errors occur John Garry 2020-03-23 17:58 ` Tudor.Ambarus 2019-12-03 14:16 ` [PATCH] mtd: spi-nor: Fix the write Status Register on micron flashes Tudor.Ambarus 2019-12-03 14:50 ` [PATCH v2] mtd: spi-nor: Fix the writing of the " Tudor.Ambarus 2019-12-04 10:18 ` John Garry 2020-01-09 19:14 ` Miquel Raynal
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).