[PATCH v2] mtd: spi-nor: wait until lock/unlock operations are ready

[PATCH v2] mtd: spi-nor: wait until lock/unlock operations are ready

[Ezequiel Garcia]

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>
---
Changes from v1:
 * Simplify style by tail calling spi_nor_wait_till_ready
 
 drivers/mtd/spi-nor/spi-nor.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 7e5051e604b0..835a010d9339 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -481,6 +481,7 @@ static int stm_lock(struct spi_nor *nor, loff_t ofs, uint64_t len)
 	int 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);
 	if (status_old < 0)
@@ -519,7 +520,10 @@ 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;
+	return spi_nor_wait_till_ready(nor);
 }
 
 /*
@@ -533,6 +537,7 @@ static int stm_unlock(struct spi_nor *nor, loff_t ofs, uint64_t len)
 	int 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);
 	if (status_old < 0)
@@ -569,7 +574,10 @@ 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;
+	return spi_nor_wait_till_ready(nor);
 }
 
 /*
-- 
2.6.4

Re: [PATCH v2] mtd: spi-nor: wait until lock/unlock operations are ready

[Brian Norris]

On Mon, Dec 28, 2015 at 05:54:51PM -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

Edited the above sentence, since you're not doing this "only for these
manufacturers"; you're doing it for all that support lock/unlock.
Instead, it'll read:

  "This commit adds a wait_till_ready call on lock/unlock operations,
  which is required for Micron and Numonyx but should be harmless for
  others."

With that, applied to l2-mtd.git

Brian

> 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>
> ---
> Changes from v1:
>  * Simplify style by tail calling spi_nor_wait_till_ready
>  
>  drivers/mtd/spi-nor/spi-nor.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 7e5051e604b0..835a010d9339 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -481,6 +481,7 @@ static int stm_lock(struct spi_nor *nor, loff_t ofs, uint64_t len)
>  	int 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);
>  	if (status_old < 0)
> @@ -519,7 +520,10 @@ 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;
> +	return spi_nor_wait_till_ready(nor);
>  }
>  
>  /*
> @@ -533,6 +537,7 @@ static int stm_unlock(struct spi_nor *nor, loff_t ofs, uint64_t len)
>  	int 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);
>  	if (status_old < 0)
> @@ -569,7 +574,10 @@ 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;
> +	return spi_nor_wait_till_ready(nor);
>  }
>  
>  /*
> -- 
> 2.6.4
> 

Re: [PATCH v2] mtd: spi-nor: wait until lock/unlock operations are ready

[Ezequiel Garcia]

On 6 January 2016 at 22:20, Brian Norris <computersforpeace@gmail.com> wrote:
> On Mon, Dec 28, 2015 at 05:54:51PM -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
>
> Edited the above sentence, since you're not doing this "only for these
> manufacturers"; you're doing it for all that support lock/unlock.

Just to make sure we are on the same page here, this commit only
changes stm_lock, stm_unlock. So the change currently affects devices that
match JEDEC_MFR(info) == SNOR_MFR_MICRON.

-- 
Ezequiel García, VanguardiaSur
www.vanguardiasur.com.ar

Re: [PATCH v2] mtd: spi-nor: wait until lock/unlock operations are ready

[Brian Norris]

On Thu, Jan 07, 2016 at 09:42:54AM -0300, Ezequiel Garcia wrote:
> On 6 January 2016 at 22:20, Brian Norris <computersforpeace@gmail.com> wrote:
> > Edited the above sentence, since you're not doing this "only for these
> > manufacturers"; you're doing it for all that support lock/unlock.
> 
> Just to make sure we are on the same page here, this commit only
> changes stm_lock, stm_unlock. So the change currently affects devices that
> match JEDEC_MFR(info) == SNOR_MFR_MICRON.

Yes, that's currently correct. There were attempts to make this work for
Winbond too, but those were mostly reverted recently due to regressions.
I plan to re-extend that support again soon. So depending on when we're
talking about, that statement changes.

Brian