[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ceef013d-a337-0059-6207-b30d2274d5cb@microchip.com>
Date: Thu, 10 Feb 2022 03:32:22 +0000
From: <Tudor.Ambarus@...rochip.com>
To: <michael@...le.cc>, <linux-mtd@...ts.infradead.org>,
<linux-kernel@...r.kernel.org>
CC: <p.yadav@...com>, <miquel.raynal@...tlin.com>, <richard@....at>,
<vigneshr@...com>
Subject: Re: [PATCH v1 12/14] mtd: spi-nor: move all spansion specifics into
spansion.c
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@...le.cc>
> ---
> 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
>
Powered by blists - more mailing lists