[PATCH] mtd: spi-nor: Check that BP bits are set properly
Aaron Sierra
asierra at xes-inc.com
Mon Dec 4 08:56:56 PST 2017
----- Original Message -----
> From: "Cyrille Pitchen" <cyrille.pitchen at wedev4u.fr>
> Sent: Thursday, November 30, 2017 7:57:27 AM
Hi Cyrille,
> Hi Aaron,
>
> Le 29/10/2017 à 17:11, Aaron Sierra a écrit :
>> Previously, the lock and unlock functions returned success even if the
>> BP bits were not actually updated in the status register due to
>> hardware write protection. Introduce write_sr_and_check() to write and
>> read back the status register to ensure the desired BP bits are
>> actually set as requested.
>>
>> Signed-off-by: Joe Schultz <jschultz at xes-inc.com>
>> Signed-off-by: Aaron Sierra <asierra at xes-inc.com>
>> ---
>> drivers/mtd/spi-nor/spi-nor.c | 39 +++++++++++++++++++++++++++------------
>> 1 file changed, 27 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
>> index 19c00072..8a87bd1 100644
>> --- a/drivers/mtd/spi-nor/spi-nor.c
>> +++ b/drivers/mtd/spi-nor/spi-nor.c
>> @@ -550,6 +550,31 @@ static int spi_nor_erase(struct mtd_info *mtd, struct
>> erase_info *instr)
>> return ret;
>> }
>>
>> +/* Write status register and ensure bits in mask match written values */
>> +static int write_sr_and_check(struct spi_nor *nor, u8 new, u8 mask)
>
> Maybe keep the previous name 'status_new' to be consistent with stm_lock()
> and stm_unlock() and also because 'new' is a C++ reserved keyword so it might
> be better to avoid using it to name some variable (editor syntax coloring issue
> for instance).
Sure, both of those reasons seem reasonable.
>> +{
>> + int ret;
>> +
>> + ret = spi_nor_wait_till_ready(nor);
>> + if (ret)
>> + return ret;
>> +
>
> Why do you poll the status register here ?
> Based on the source code before your patch, it should not be needed.
> Do you think it is ?
This polling was due to an excess of caution. I've reviewed the preceding
code and agree that this can be removed from the next version of this patch.
> Otherwise, your patch looks good to me :)
Thanks for your review!
-Aaron S.
> Best regards,
>
> Cyrille
>
More information about the linux-mtd
mailing list