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

[PATCH] 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>
---
 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;
 
 	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;
 }
 
 /*
-- 
2.6.2

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

[Brian Norris]

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

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

[Ezequiel Garcia]

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.
-- 
Ezequiel García, VanguardiaSur
www.vanguardiasur.com.ar