[PATCH v3 4/9] mtd: spi-nor: Reorder the preparation vs locking steps

[Tudor Ambarus]

Hi, Miquel,

On 12/15/22 08:12, Miquel Raynal wrote:
> The ->prepare()/->unprepare() hooks are now legacy, and there are only
> two controllers left supporting them. In both cases, the implementation

now there's only one, hisi-sfc.c.

> acquires a mutex, which is somehow redundant with the spi-nor main lock

I see a HIFMC_MAX_CHIP_NUM with value 2, the controller seems to be able
to operate 2 flashes in parallel and that's why the internal mutex.

> that we acquire as well in the spi_nor_[un]lock_and_[un]prep() helpers.
> 
> While the mutex taken in the core is necessary, the helper can be
> reorganized to first do the preparation, then acquire the core
> lock. This is necessary in order to be able to improve the locking
> mechanism in the core and should have no side effect.
> 

the change seems fine for hisi, and since we're no longer adding drivers
under spi-nor/controllers we should be good.

> Signed-off-by: Miquel Raynal <miquel.raynal at bootlin.com>
> ---
>   drivers/mtd/spi-nor/core.c | 15 ++++++---------
>   1 file changed, 6 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index 38a57aac6754..de77ca55f74d 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -1074,23 +1074,20 @@ int spi_nor_lock_and_prep(struct spi_nor *nor)

you'll have to rename it to spi_nor_prep_and_lock to reflect the order
of ops.

>   {
>   	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;
>   }
> 

Cheers,
ta