[PATCH v4 08/24] mtd: spinand: minor refactoring

[Frieder Schrempf]

Suggestion for subject:

mtd: spinand: Refactor spinand_init* functions

Am 09.08.25 um 03:04 schrieb Mikhail Kshevetskiy:
> No functional changes, just some refactoring to match better linux
> kernel driver.

Nitpick: "... better match Linux kernel driver"

With the subject line improved:

Reviewed-by: Frieder Schrempf <frieder.schrempf at kontron.de>

> 
> changes:
>  * move spinand configuration reading out from spinand_init_cfg_cache()
>    to separate function spinand_read_cfg()
>  * move spinand flash initialization to separate function
>    spinand_init_flash()
>  * move direct mapping initialization to the end of spinand_init()>
> Signed-off-by: Mikhail Kshevetskiy <mikhail.kshevetskiy at iopsys.eu>
> ---
>  drivers/mtd/nand/spi/core.c | 112 ++++++++++++++++++++++--------------
>  1 file changed, 70 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
> index 098ed0c4cd2..df59d3d23ce 100644
> --- a/drivers/mtd/nand/spi/core.c
> +++ b/drivers/mtd/nand/spi/core.c
> @@ -160,20 +160,12 @@ int spinand_select_target(struct spinand_device *spinand, unsigned int target)
>  	return 0;
>  }
>  
> -static int spinand_init_cfg_cache(struct spinand_device *spinand)
> +static int spinand_read_cfg(struct spinand_device *spinand)
>  {
>  	struct nand_device *nand = spinand_to_nand(spinand);
> -	struct udevice *dev = spinand->slave->dev;
>  	unsigned int target;
>  	int ret;
>  
> -	spinand->cfg_cache = devm_kzalloc(dev,
> -					  sizeof(*spinand->cfg_cache) *
> -					  nand->memorg.ntargets,
> -					  GFP_KERNEL);
> -	if (!spinand->cfg_cache)
> -		return -ENOMEM;
> -
>  	for (target = 0; target < nand->memorg.ntargets; target++) {
>  		ret = spinand_select_target(spinand, target);
>  		if (ret)
> @@ -192,6 +184,21 @@ static int spinand_init_cfg_cache(struct spinand_device *spinand)
>  	return 0;
>  }
>  
> +static int spinand_init_cfg_cache(struct spinand_device *spinand)
> +{
> +	struct nand_device *nand = spinand_to_nand(spinand);
> +	struct udevice *dev = spinand->slave->dev;
> +
> +	spinand->cfg_cache = devm_kcalloc(dev,
> +					  nand->memorg.ntargets,
> +					  sizeof(*spinand->cfg_cache),
> +					  GFP_KERNEL);
> +	if (!spinand->cfg_cache)
> +		return -ENOMEM;
> +
> +	return 0;
> +}
> +
>  static int spinand_init_quad_enable(struct spinand_device *spinand)
>  {
>  	bool enable = false;
> @@ -1091,11 +1098,55 @@ static const struct mtd_ooblayout_ops spinand_noecc_ooblayout = {
>  	.rfree = spinand_noecc_ooblayout_free,
>  };
>  
> +static int spinand_init_flash(struct spinand_device *spinand)
> +{
> +	struct udevice *dev = spinand->slave->dev;
> +	struct nand_device *nand = spinand_to_nand(spinand);
> +	int ret, i;
> +
> +	ret = spinand_read_cfg(spinand);
> +	if (ret)
> +		return ret;
> +
> +	ret = spinand_init_quad_enable(spinand);
> +	if (ret)
> +		return ret;
> +
> +	ret = spinand_upd_cfg(spinand, CFG_OTP_ENABLE, 0);
> +	if (ret)
> +		return ret;
> +
> +	ret = spinand_manufacturer_init(spinand);
> +	if (ret) {
> +		dev_err(dev,
> +			"Failed to initialize the SPI NAND chip (err = %d)\n",
> +			ret);
> +		return ret;
> +	}
> +
> +	/* After power up, all blocks are locked, so unlock them here. */
> +	for (i = 0; i < nand->memorg.ntargets; i++) {
> +		ret = spinand_select_target(spinand, i);
> +		if (ret)
> +			break;
> +
> +		ret = spinand_lock_block(spinand, BL_ALL_UNLOCKED);
> +		if (ret)
> +			break;
> +	}
> +
> +	if (ret)
> +		spinand_manufacturer_cleanup(spinand);
> +
> +	return ret;
> +}
> +
>  static int spinand_init(struct spinand_device *spinand)
>  {
> +	struct udevice *dev = spinand->slave->dev;
>  	struct mtd_info *mtd = spinand_to_mtd(spinand);
>  	struct nand_device *nand = mtd_to_nanddev(mtd);
> -	int ret, i;
> +	int ret;
>  
>  	/*
>  	 * We need a scratch buffer because the spi_mem interface requires that
> @@ -1128,41 +1179,10 @@ static int spinand_init(struct spinand_device *spinand)
>  	if (ret)
>  		goto err_free_bufs;
>  
> -	ret = spinand_init_quad_enable(spinand);
> +	ret = spinand_init_flash(spinand);
>  	if (ret)
>  		goto err_free_bufs;
>  
> -	ret = spinand_upd_cfg(spinand, CFG_OTP_ENABLE, 0);
> -	if (ret)
> -		goto err_free_bufs;
> -
> -	ret = spinand_manufacturer_init(spinand);
> -	if (ret) {
> -		dev_err(spinand->slave->dev,
> -			"Failed to initialize the SPI NAND chip (err = %d)\n",
> -			ret);
> -		goto err_free_bufs;
> -	}
> -
> -	ret = spinand_create_dirmaps(spinand);
> -	if (ret) {
> -		dev_err(spinand->slave->dev,
> -			"Failed to create direct mappings for read/write operations (err = %d)\n",
> -			ret);
> -		goto err_manuf_cleanup;
> -	}
> -
> -	/* After power up, all blocks are locked, so unlock them here. */
> -	for (i = 0; i < nand->memorg.ntargets; i++) {
> -		ret = spinand_select_target(spinand, i);
> -		if (ret)
> -			goto err_manuf_cleanup;
> -
> -		ret = spinand_lock_block(spinand, BL_ALL_UNLOCKED);
> -		if (ret)
> -			goto err_manuf_cleanup;
> -	}
> -
>  	ret = nanddev_init(nand, &spinand_ops, THIS_MODULE);
>  	if (ret)
>  		goto err_manuf_cleanup;
> @@ -1189,6 +1209,14 @@ static int spinand_init(struct spinand_device *spinand)
>  
>  	mtd->oobavail = ret;
>  
> +	ret = spinand_create_dirmaps(spinand);
> +	if (ret) {
> +		dev_err(dev,
> +			"Failed to create direct mappings for read/write operations (err = %d)\n",
> +			ret);
> +		goto err_cleanup_nanddev;
> +	}
> +
>  	return 0;
>  
>  err_cleanup_nanddev: