On 08/03/21 06:10PM, Takahiro Kuwano wrote:
> Hi Jagan,
>
> On 2/26/2021 7:42 PM, Jagan Teki wrote:
> > On Fri, Feb 19, 2021 at 7:26 AM <tkuw584...@gmail.com> wrote:
> >>
> >> From: Takahiro Kuwano <takahiro.kuw...@infineon.com>
> >>
> >> Some of Spansion/Cypress chips support volatile version of configuration
> >> registers and it is recommended to update volatile registers in the field
> >> application due to a risk of the non-volatile registers corruption by
> >> power interrupt. This patch adds a function to set Quad Enable bit in CFR1
> >> volatile.
> >>
> >> Signed-off-by: Takahiro Kuwano <takahiro.kuw...@infineon.com>
> >> ---
> >> Changes in v5:
> >> - Fix register address calculation, 'base | offset' -> 'base + offset'
> >>
> >> drivers/mtd/spi/spi-nor-core.c | 53 ++++++++++++++++++++++++++++++++++
> >> include/linux/mtd/spi-nor.h | 1 +
> >> 2 files changed, 54 insertions(+)
> >>
> >> diff --git a/drivers/mtd/spi/spi-nor-core.c
> >> b/drivers/mtd/spi/spi-nor-core.c
> >> index 2803536ed5..87c9fce408 100644
> >> --- a/drivers/mtd/spi/spi-nor-core.c
> >> +++ b/drivers/mtd/spi/spi-nor-core.c
> >> @@ -1576,6 +1576,59 @@ static int spansion_read_cr_quad_enable(struct
> >> spi_nor *nor)
> >> return 0;
> >> }
> >>
> >> +/**
> >> + * spansion_quad_enable_volatile() - enable Quad I/O mode in volatile
> >> register.
> >> + * @nor: pointer to a 'struct spi_nor'
> >> + * @addr_base: base address of register (can be >0 in multi-die parts)
> >> + * @dummy: number of dummy cycles for register read
> >> + *
> >> + * It is recommended to update volatile registers in the field
> >> application due
> >> + * to a risk of the non-volatile registers corruption by power interrupt.
> >> This
> >> + * function sets Quad Enable bit in CFR1 volatile.
> >> + *
> >> + * Return: 0 on success, -errno otherwise.
> >> + */
> >> +static int spansion_quad_enable_volatile(struct spi_nor *nor, u32
> >> addr_base,
> >> + u8 dummy)
> >> +{
> >> + u32 addr = addr_base + SPINOR_REG_ADDR_CFR1V;
> >> +
> >> + u8 cr;
> >> + int ret;
> >> +
> >> + /* Check current Quad Enable bit value. */
> >> + ret = spansion_read_any_reg(nor, addr, dummy, &cr);
> >
> > What if we can use the exiting quad_enable hook by identifying
> > volatile QE at the function beginning instead of having a separate
> > call?
> >
> Do you mean something like this?
>
> static int spansion_read_cr_quad_enable(struct spi_nor *nor)
> {
> u8 sr_cr[2];
> int ret;
>
> if (JEDEC_MFR(nor->info) == SNOR_MFR_CYPRESS) {
> u32 base;
>
> for (base = 0; base < nor->mtd.size; base += SZ_128M) {
> u32 addr = base + SPINOR_REG_ADDR_CFR1V;
>
> /* Check current Quad Enable bit value. */
> ret = spansion_read_any_reg(nor, addr, 0, &sr_cr[1]);
>
> [...]
>
> ret = spansion_write_any_reg(nor, addr, sr_cr[1]);
>
> [...]
>
> /* Read back and check it. */
> ret = spansion_read_any_reg(nor, addr, 0, &sr_cr[1]);
>
> [...]
> }
>
> return 0;
> }
>
> /* Check current Quad Enable bit value. */
> ret = read_cr(nor);
> if (ret < 0) {
> dev_dbg(nor->dev,
> "error while reading configuration register\n");
> return -EINVAL;
> }
>
> [...]
> }
>
> Or defining a new flag like 'SNOR_F_HAS_VOLATILE_QE'?
FWIW I think your current implementation is better than both these
alternatives.
--
Regards,
Pratyush Yadav
Texas Instruments Inc.
