Re: [PATCH v1 12/14] mtd: spi-nor: move all spansion specifics into spansion.c

[Tudor.Ambarus]

On 2/2/22 16:58, Michael Walle wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> The clear status register flags is only available on spansion flashes.
> Move all the functions around that into the spanion module.
> 
> Signed-off-by: Michael Walle <michael@xxxxxxxx>
> ---
>  drivers/mtd/spi-nor/core.c     | 52 +------------------------
>  drivers/mtd/spi-nor/spansion.c | 70 ++++++++++++++++++++++++++++++++++
>  include/linux/mtd/spi-nor.h    |  1 -
>  3 files changed, 72 insertions(+), 51 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index be65aaa954ca..5b00dfab77a6 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -554,33 +554,6 @@ int spi_nor_write_ear(struct spi_nor *nor, u8 ear)
>         return ret;
>  }
> 
> -/**
> - * spi_nor_clear_sr() - Clear the Status Register.
> - * @nor:       pointer to 'struct spi_nor'.
> - */
> -static void spi_nor_clear_sr(struct spi_nor *nor)
> -{
> -       int ret;
> -
> -       if (nor->spimem) {
> -               struct spi_mem_op op =
> -                       SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_CLSR, 0),
> -                                  SPI_MEM_OP_NO_ADDR,
> -                                  SPI_MEM_OP_NO_DUMMY,
> -                                  SPI_MEM_OP_NO_DATA);
> -
> -               spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
> -
> -               ret = spi_mem_exec_op(nor->spimem, &op);
> -       } else {
> -               ret = spi_nor_controller_ops_write_reg(nor, SPINOR_OP_CLSR,
> -                                                      NULL, 0);
> -       }
> -
> -       if (ret)
> -               dev_dbg(nor->dev, "error %d clearing SR\n", ret);
> -}
> -
>  /**
>   * spi_nor_sr_ready() - Query the Status Register to see if the flash is ready
>   * for new commands.
> @@ -590,33 +563,12 @@ static void spi_nor_clear_sr(struct spi_nor *nor)
>   */
>  int spi_nor_sr_ready(struct spi_nor *nor)
>  {
> -       int ret = spi_nor_read_sr(nor, nor->bouncebuf);
> +       int ret;
> 
> +       ret = spi_nor_read_sr(nor, nor->bouncebuf);
>         if (ret)
>                 return ret;

:) don't change style for no reason. What's wrong with the previous version?

Anyway, with the reports fixed and no hidden style changes, it looks good to me.

> 
> -       if (nor->flags & SNOR_F_USE_CLSR &&
> -           nor->bouncebuf[0] & (SR_E_ERR | SR_P_ERR)) {
> -               if (nor->bouncebuf[0] & SR_E_ERR)
> -                       dev_err(nor->dev, "Erase Error occurred\n");
> -               else
> -                       dev_err(nor->dev, "Programming Error occurred\n");
> -
> -               spi_nor_clear_sr(nor);
> -
> -               /*
> -                * WEL bit remains set to one when an erase or page program
> -                * error occurs. Issue a Write Disable command to protect
> -                * against inadvertent writes that can possibly corrupt the
> -                * contents of the memory.
> -                */
> -               ret = spi_nor_write_disable(nor);
> -               if (ret)
> -                       return ret;
> -
> -               return -EIO;
> -       }
> -
>         return !(nor->bouncebuf[0] & SR_WIP);
>  }
> 
> diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
> index dedc2de90cb8..4756fb88eab2 100644
> --- a/drivers/mtd/spi-nor/spansion.c
> +++ b/drivers/mtd/spi-nor/spansion.c
> @@ -8,6 +8,7 @@
> 
>  #include "core.h"
> 
> +#define SPINOR_OP_CLSR         0x30    /* Clear status register 1 */
>  #define SPINOR_OP_RD_ANY_REG                   0x65    /* Read any register */
>  #define SPINOR_OP_WR_ANY_REG                   0x71    /* Write any register */
>  #define SPINOR_REG_CYPRESS_CFR2V               0x00800003
> @@ -294,6 +295,72 @@ static const struct flash_info spansion_parts[] = {
>         },
>  };
> 
> +/**
> + * spi_nor_clear_sr() - Clear the Status Register.
> + * @nor:       pointer to 'struct spi_nor'.
> + */
> +static void spi_nor_clear_sr(struct spi_nor *nor)
> +{
> +       int ret;
> +
> +       if (nor->spimem) {
> +               struct spi_mem_op op =
> +                       SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_CLSR, 0),
> +                                  SPI_MEM_OP_NO_ADDR,
> +                                  SPI_MEM_OP_NO_DUMMY,
> +                                  SPI_MEM_OP_NO_DATA);
> +
> +               spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
> +
> +               ret = spi_mem_exec_op(nor->spimem, &op);
> +       } else {
> +               ret = spi_nor_controller_ops_write_reg(nor, SPINOR_OP_CLSR,
> +                                                      NULL, 0);
> +       }
> +
> +       if (ret)
> +               dev_dbg(nor->dev, "error %d clearing SR\n", ret);
> +}
> +
> +/**
> + * spi_nor_sr_ready_and_clear() - Query the Status Register to see if the flash
> + * is ready for new commands and clear it.
> + * @nor:       pointer to 'struct spi_nor'.
> + *
> + * Return: 1 if ready, 0 if not ready, -errno on errors.
> + */
> +int spi_nor_sr_ready_and_clear(struct spi_nor *nor)
> +{
> +       int ret;
> +
> +       ret = spi_nor_read_sr(nor, nor->bouncebuf);
> +       if (ret)
> +               return ret;
> +
> +       if (nor->bouncebuf[0] & (SR_E_ERR | SR_P_ERR)) {
> +               if (nor->bouncebuf[0] & SR_E_ERR)
> +                       dev_err(nor->dev, "Erase Error occurred\n");
> +               else
> +                       dev_err(nor->dev, "Programming Error occurred\n");
> +
> +               spi_nor_clear_sr(nor);
> +
> +               /*
> +                * WEL bit remains set to one when an erase or page program
> +                * error occurs. Issue a Write Disable command to protect
> +                * against inadvertent writes that can possibly corrupt the
> +                * contents of the memory.
> +                */
> +               ret = spi_nor_write_disable(nor);
> +               if (ret)
> +                       return ret;
> +
> +               return -EIO;
> +       }
> +
> +       return !(nor->bouncebuf[0] & SR_WIP);
> +}
> +
>  static void spansion_late_init(struct spi_nor *nor)
>  {
>         if (nor->params->size > SZ_16M) {
> @@ -302,6 +369,9 @@ static void spansion_late_init(struct spi_nor *nor)
>                 nor->erase_opcode = SPINOR_OP_SE;
>                 nor->mtd.erasesize = nor->info->sector_size;
>         }
> +
> +       if (nor->flags & SNOR_F_USE_CLSR)
> +               nor->params->ready = spi_nor_sr_ready_and_clear;
>  }
> 
>  static const struct spi_nor_fixups spansion_fixups = {
> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
> index 4622251a79ff..5e25a7b75ae2 100644
> --- a/include/linux/mtd/spi-nor.h
> +++ b/include/linux/mtd/spi-nor.h
> @@ -90,7 +90,6 @@
> 
>  /* Used for Spansion flashes only. */
>  #define SPINOR_OP_BRWR         0x17    /* Bank register write */
> -#define SPINOR_OP_CLSR         0x30    /* Clear status register 1 */
> 
>  /* Used for Micron flashes only. */
>  #define SPINOR_OP_RD_EVCR      0x65    /* Read EVCR register */
> --
> 2.30.2
>