| Message ID | 1448240997-26969-1-git-send-email-ezequiel@vanguardiasur.com.ar |
|---|---|
| State | Changes Requested |
| Headers | show |
On Sun, Nov 22, 2015 at 10:09:57PM -0300, Ezequiel Garcia wrote: > On Micron and Numonyx devices, the status register write command > (WRSR), raises a work-in-progress bit (WIP) on the status register. > The datasheets for these devices specify that while the status > register write is in progress, the status register WIP bit can still > be read to check the end of the operation. > > This commit adds a wait_till_ready call on lock/unlock operations, > only for these manufacturers. This is needed to prevent applications > from issuing erase or program operations before the unlock operation > is completed. > > Reported-by: Stas Sergeev <stsp@list.ru> > Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> > --- > drivers/mtd/spi-nor/spi-nor.c | 18 ++++++++++++++++-- > 1 file changed, 16 insertions(+), 2 deletions(-) > > diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c > index e9c26c0e2258..acc9b05e02be 100644 > --- a/drivers/mtd/spi-nor/spi-nor.c > +++ b/drivers/mtd/spi-nor/spi-nor.c > @@ -485,6 +485,7 @@ static int stm_lock(struct spi_nor *nor, loff_t ofs, uint64_t len) > u8 status_old, status_new; > u8 mask = SR_BP2 | SR_BP1 | SR_BP0; > u8 shift = ffs(mask) - 1, pow, val; > + int ret; I've merged other patches in the meantime (sorry), so this might need a rebasing on l2-mtd.git. I'd do it, but should get a v2 anyway. > > status_old = read_sr(nor); > > @@ -521,7 +522,13 @@ static int stm_lock(struct spi_nor *nor, loff_t ofs, uint64_t len) > return -EINVAL; > > write_enable(nor); > - return write_sr(nor, status_new); > + ret = write_sr(nor, status_new); > + if (ret) > + return ret; > + ret = spi_nor_wait_till_ready(nor); > + if (ret) > + return ret; > + return 0; Why not just return spi_nor_wait_till_ready(nor)? > } > > /* > @@ -535,6 +542,7 @@ static int stm_unlock(struct spi_nor *nor, loff_t ofs, uint64_t len) > uint8_t status_old, status_new; > u8 mask = SR_BP2 | SR_BP1 | SR_BP0; > u8 shift = ffs(mask) - 1, pow, val; > + int ret; > > status_old = read_sr(nor); > > @@ -569,7 +577,13 @@ static int stm_unlock(struct spi_nor *nor, loff_t ofs, uint64_t len) > return -EINVAL; > > write_enable(nor); > - return write_sr(nor, status_new); > + ret = write_sr(nor, status_new); > + if (ret) > + return ret; > + ret = spi_nor_wait_till_ready(nor); > + if (ret) > + return ret; > + return 0; Same? > } > > /* Brian
Brian, On 15 December 2015 at 15:12, Brian Norris <computersforpeace@gmail.com> wrote: > On Sun, Nov 22, 2015 at 10:09:57PM -0300, Ezequiel Garcia wrote: >> On Micron and Numonyx devices, the status register write command >> (WRSR), raises a work-in-progress bit (WIP) on the status register. >> The datasheets for these devices specify that while the status >> register write is in progress, the status register WIP bit can still >> be read to check the end of the operation. >> >> This commit adds a wait_till_ready call on lock/unlock operations, >> only for these manufacturers. This is needed to prevent applications >> from issuing erase or program operations before the unlock operation >> is completed. >> >> Reported-by: Stas Sergeev <stsp@list.ru> >> Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> >> --- >> drivers/mtd/spi-nor/spi-nor.c | 18 ++++++++++++++++-- >> 1 file changed, 16 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c >> index e9c26c0e2258..acc9b05e02be 100644 >> --- a/drivers/mtd/spi-nor/spi-nor.c >> +++ b/drivers/mtd/spi-nor/spi-nor.c >> @@ -485,6 +485,7 @@ static int stm_lock(struct spi_nor *nor, loff_t ofs, uint64_t len) >> u8 status_old, status_new; >> u8 mask = SR_BP2 | SR_BP1 | SR_BP0; >> u8 shift = ffs(mask) - 1, pow, val; >> + int ret; > > I've merged other patches in the meantime (sorry), so this might need a > rebasing on l2-mtd.git. I'd do it, but should get a v2 anyway. > No problem. >> >> status_old = read_sr(nor); >> >> @@ -521,7 +522,13 @@ static int stm_lock(struct spi_nor *nor, loff_t ofs, uint64_t len) >> return -EINVAL; >> >> write_enable(nor); >> - return write_sr(nor, status_new); >> + ret = write_sr(nor, status_new); >> + if (ret) >> + return ret; >> + ret = spi_nor_wait_till_ready(nor); >> + if (ret) >> + return ret; >> + return 0; > > Why not just return spi_nor_wait_till_ready(nor)? > >> } >> >> /* >> @@ -535,6 +542,7 @@ static int stm_unlock(struct spi_nor *nor, loff_t ofs, uint64_t len) >> uint8_t status_old, status_new; >> u8 mask = SR_BP2 | SR_BP1 | SR_BP0; >> u8 shift = ffs(mask) - 1, pow, val; >> + int ret; >> >> status_old = read_sr(nor); >> >> @@ -569,7 +577,13 @@ static int stm_unlock(struct spi_nor *nor, loff_t ofs, uint64_t len) >> return -EINVAL; >> >> write_enable(nor); >> - return write_sr(nor, status_new); >> + ret = write_sr(nor, status_new); >> + if (ret) >> + return ret; >> + ret = spi_nor_wait_till_ready(nor); >> + if (ret) >> + return ret; >> + return 0; > > Same? > Sure, somehow I missed that. I'll send a fixed v2.
diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c index e9c26c0e2258..acc9b05e02be 100644 --- a/drivers/mtd/spi-nor/spi-nor.c +++ b/drivers/mtd/spi-nor/spi-nor.c @@ -485,6 +485,7 @@ static int stm_lock(struct spi_nor *nor, loff_t ofs, uint64_t len) u8 status_old, status_new; u8 mask = SR_BP2 | SR_BP1 | SR_BP0; u8 shift = ffs(mask) - 1, pow, val; + int ret; status_old = read_sr(nor); @@ -521,7 +522,13 @@ static int stm_lock(struct spi_nor *nor, loff_t ofs, uint64_t len) return -EINVAL; write_enable(nor); - return write_sr(nor, status_new); + ret = write_sr(nor, status_new); + if (ret) + return ret; + ret = spi_nor_wait_till_ready(nor); + if (ret) + return ret; + return 0; } /* @@ -535,6 +542,7 @@ static int stm_unlock(struct spi_nor *nor, loff_t ofs, uint64_t len) uint8_t status_old, status_new; u8 mask = SR_BP2 | SR_BP1 | SR_BP0; u8 shift = ffs(mask) - 1, pow, val; + int ret; status_old = read_sr(nor); @@ -569,7 +577,13 @@ static int stm_unlock(struct spi_nor *nor, loff_t ofs, uint64_t len) return -EINVAL; write_enable(nor); - return write_sr(nor, status_new); + ret = write_sr(nor, status_new); + if (ret) + return ret; + ret = spi_nor_wait_till_ready(nor); + if (ret) + return ret; + return 0; } /*
On Micron and Numonyx devices, the status register write command (WRSR), raises a work-in-progress bit (WIP) on the status register. The datasheets for these devices specify that while the status register write is in progress, the status register WIP bit can still be read to check the end of the operation. This commit adds a wait_till_ready call on lock/unlock operations, only for these manufacturers. This is needed to prevent applications from issuing erase or program operations before the unlock operation is completed. Reported-by: Stas Sergeev <stsp@list.ru> Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> --- drivers/mtd/spi-nor/spi-nor.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-)