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.schre...@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.kshevets...@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:
