| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20220215192345.3pv4soo3grjsr7op@ti.com>
Date: Wed, 16 Feb 2022 00:53:45 +0530
From: Pratyush Yadav <p.yadav@...com>
To: <Tudor.Ambarus@...rochip.com>
CC: <michael@...le.cc>, <linux-mtd@...ts.infradead.org>,
<linux-kernel@...r.kernel.org>, <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 10/02/22 03:32AM, Tudor.Ambarus@...rochip.com wrote:
> 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?
FWIW, I like the newer style better. But that should come in a separate
patch in either case.
>
> 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)
Make it static.
[...]
--
Regards,
Pratyush Yadav
Texas Instruments Inc.
Powered by blists - more mailing lists