[PATCH v4 3/8] mtd: spi-nor: Reorder the preparation vs locking steps

[Tudor Ambarus]

On 3/17/23 03:39, Tudor Ambarus wrote:
> 
> 
> On 2/1/23 11:35, Miquel Raynal wrote:
>> The ->prepare()/->unprepare() hooks are now legacy, we no longer accept
>> new drivers supporting them. The only remaining controllers using them
>> acquires a per-chip mutex, which should not interfere with the rest of
>> the operation done in the core. As a result, we should be safe to
>> reorganize these helpers to first perform the preparation, before
>> acquiring the core locks. This is necessary in order to be able to
>> improve the locking mechanism in the core (coming next). No side effects
>> are expected.
>>
>> Signed-off-by: Miquel Raynal <miquel.raynal at bootlin.com>
> 
> Reviewed-by: Tudor Ambarus <tudor.ambarus at linaro.org>

No, I take back my R-b. The reorder is fine, but you missed to update
otp.c and swp.c, see below.
> 
>> ---
>>  drivers/mtd/spi-nor/core.c | 23 ++++++++++-------------
>>  1 file changed, 10 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
>> index 3845de9c874c..01932dbaa5b9 100644
>> --- a/drivers/mtd/spi-nor/core.c
>> +++ b/drivers/mtd/spi-nor/core.c
>> @@ -1071,27 +1071,24 @@ static void spi_nor_set_4byte_opcodes(struct spi_nor *nor)
>>  	}
>>  }
>>  
>> -int spi_nor_lock_and_prep(struct spi_nor *nor)
>> +int spi_nor_prep_and_lock(struct spi_nor *nor)

$ git grep spi_nor_lock_and_prep
drivers/mtd/spi-nor/core.h:int spi_nor_lock_and_prep(struct spi_nor *nor);
drivers/mtd/spi-nor/otp.c:      ret = spi_nor_lock_and_prep(nor);
drivers/mtd/spi-nor/otp.c:      ret = spi_nor_lock_and_prep(nor);
drivers/mtd/spi-nor/otp.c:      ret = spi_nor_lock_and_prep(nor);
drivers/mtd/spi-nor/otp.c:      ret = spi_nor_lock_and_prep(nor);
drivers/mtd/spi-nor/sst.c:      ret = spi_nor_lock_and_prep(nor);
drivers/mtd/spi-nor/swp.c:      ret = spi_nor_lock_and_prep(nor);
drivers/mtd/spi-nor/swp.c:      ret = spi_nor_lock_and_prep(nor);
drivers/mtd/spi-nor/swp.c:      ret = spi_nor_lock_and_prep(nor);

Please make sure you try a build after each patch, you'll affect bisects:

arm-linux-gnueabi-ld: drivers/mtd/spi-nor/swp.o: in function
`spi_nor_unlock':
swp.c:(.text+0x6d0): undefined reference to `spi_nor_lock_and_prep'
arm-linux-gnueabi-ld: drivers/mtd/spi-nor/swp.o: in function
`spi_nor_is_locked':
swp.c:(.text+0x72c): undefined reference to `spi_nor_lock_and_prep'
arm-linux-gnueabi-ld: drivers/mtd/spi-nor/swp.o: in function `spi_nor_lock':
swp.c:(.text+0x788): undefined reference to `spi_nor_lock_and_prep'
arm-linux-gnueabi-ld: drivers/mtd/spi-nor/swp.o: in function
`spi_nor_try_unlock_all':
swp.c:(.text+0x804): undefined reference to `spi_nor_lock_and_prep'
arm-linux-gnueabi-ld: drivers/mtd/spi-nor/otp.o: in function
`spi_nor_mtd_otp_info':
otp.c:(.text+0x30): undefined reference to `spi_nor_lock_and_prep'
arm-linux-gnueabi-ld: drivers/mtd/spi-nor/otp.o:otp.c:(.text+0x138):
more undefined references to `spi_nor_lock_and_prep' follow
make[1]: *** [scripts/Makefile.vmlinux:34: vmlinux] Error 1
make: *** [Makefile:1252: vmlinux] Error 2

>>  {
>>  	int ret = 0;
>>  
>> -	mutex_lock(&nor->lock);
>> -
>> -	if (nor->controller_ops &&  nor->controller_ops->prepare) {
>> +	if (nor->controller_ops && nor->controller_ops->prepare)
>>  		ret = nor->controller_ops->prepare(nor);
>> -		if (ret) {
>> -			mutex_unlock(&nor->lock);
>> -			return ret;
>> -		}
>> -	}
>> +
>> +	mutex_lock(&nor->lock);
>> +
>>  	return ret;
>>  }
>>  
>>  void spi_nor_unlock_and_unprep(struct spi_nor *nor)
>>  {
>> +	mutex_unlock(&nor->lock);
>> +
>>  	if (nor->controller_ops && nor->controller_ops->unprepare)
>>  		nor->controller_ops->unprepare(nor);
>> -	mutex_unlock(&nor->lock);
>>  }
>>  
>>  static u32 spi_nor_convert_addr(struct spi_nor *nor, loff_t addr)
>> @@ -1447,7 +1444,7 @@ static int spi_nor_erase(struct mtd_info *mtd, struct erase_info *instr)
>>  	addr = instr->addr;
>>  	len = instr->len;
>>  
>> -	ret = spi_nor_lock_and_prep(nor);
>> +	ret = spi_nor_prep_and_lock(nor);
>>  	if (ret)
>>  		return ret;
>>  
>> @@ -1707,7 +1704,7 @@ static int spi_nor_read(struct mtd_info *mtd, loff_t from, size_t len,
>>  
>>  	dev_dbg(nor->dev, "from 0x%08x, len %zd\n", (u32)from, len);
>>  
>> -	ret = spi_nor_lock_and_prep(nor);
>> +	ret = spi_nor_prep_and_lock(nor);
>>  	if (ret)
>>  		return ret;
>>  
>> @@ -1753,7 +1750,7 @@ static int spi_nor_write(struct mtd_info *mtd, loff_t to, size_t len,
>>  
>>  	dev_dbg(nor->dev, "to 0x%08x, len %zd\n", (u32)to, len);
>>  
>> -	ret = spi_nor_lock_and_prep(nor);
>> +	ret = spi_nor_prep_and_lock(nor);
>>  	if (ret)
>>  		return ret;
>>