Message ID | a322caf3813cc4e8fc028537bca883f3016b1028.1611729896.git.Takahiro.Kuwano@infineon.com |
---|---|
State | Superseded |
Delegated to: | Jagannadha Sutradharudu Teki |
Headers | show |
Series | mtd: spi-nor: Add support for Cypress s25hl-t/s25hs-t | expand |
Hi, On 28/01/21 01:36PM, tkuw584924@gmail.com wrote: > From: Takahiro Kuwano <Takahiro.Kuwano@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.Kuwano@infineon.com> > --- > drivers/mtd/spi/spi-nor-core.c | 53 ++++++++++++++++++++++++++++++++++ > 1 file changed, 53 insertions(+) > > diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c > index 2803536ed5..624e730524 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; Why do you OR the register offset with the base? Shouldn't you be adding to it? > + > + u8 cr; > + int ret; > + > + /* Check current Quad Enable bit value. */ > + ret = spansion_read_any_reg(nor, addr, dummy, &cr); > + if (ret < 0) { > + dev_dbg(nor->dev, > + "error while reading configuration register\n"); > + return -EINVAL; > + } > + > + if (cr & CR_QUAD_EN_SPAN) > + return 0; > + > + cr |= CR_QUAD_EN_SPAN; > + > + write_enable(nor); > + > + ret = spansion_write_any_reg(nor, addr, cr); > + > + if (ret < 0) { > + dev_dbg(nor->dev, > + "error while writing configuration register\n"); > + return -EINVAL; > + } > + > + /* Read back and check it. */ > + ret = spansion_read_any_reg(nor, addr, dummy, &cr); > + if (ret || !(cr & CR_QUAD_EN_SPAN)) { > + dev_dbg(nor->dev, "Spansion Quad bit not set\n"); > + return -EINVAL; > + } > + > + return 0; > +} > + Rest of the patch LGTM. > #if CONFIG_IS_ENABLED(SPI_FLASH_SFDP_SUPPORT) > /** > * spansion_no_read_cr_quad_enable() - set QE bit in Configuration Register. > -- > 2.25.1 >
Hi Pratyush, On 1/30/2021 3:40 AM, Pratyush Yadav wrote: > Hi, > > On 28/01/21 01:36PM, tkuw584924@gmail.com wrote: >> From: Takahiro Kuwano <Takahiro.Kuwano@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.Kuwano@infineon.com> >> --- >> drivers/mtd/spi/spi-nor-core.c | 53 ++++++++++++++++++++++++++++++++++ >> 1 file changed, 53 insertions(+) >> >> diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c >> index 2803536ed5..624e730524 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; > > Why do you OR the register offset with the base? Shouldn't you be adding > to it? > I missed it during review... I will fix. >> + >> + u8 cr; >> + int ret; >> + >> + /* Check current Quad Enable bit value. */ >> + ret = spansion_read_any_reg(nor, addr, dummy, &cr); >> + if (ret < 0) { >> + dev_dbg(nor->dev, >> + "error while reading configuration register\n"); >> + return -EINVAL; >> + } >> + >> + if (cr & CR_QUAD_EN_SPAN) >> + return 0; >> + >> + cr |= CR_QUAD_EN_SPAN; >> + >> + write_enable(nor); >> + >> + ret = spansion_write_any_reg(nor, addr, cr); >> + >> + if (ret < 0) { >> + dev_dbg(nor->dev, >> + "error while writing configuration register\n"); >> + return -EINVAL; >> + } >> + >> + /* Read back and check it. */ >> + ret = spansion_read_any_reg(nor, addr, dummy, &cr); >> + if (ret || !(cr & CR_QUAD_EN_SPAN)) { >> + dev_dbg(nor->dev, "Spansion Quad bit not set\n"); >> + return -EINVAL; >> + } >> + >> + return 0; >> +} >> + > > Rest of the patch LGTM. > >> #if CONFIG_IS_ENABLED(SPI_FLASH_SFDP_SUPPORT) >> /** >> * spansion_no_read_cr_quad_enable() - set QE bit in Configuration Register. >> -- >> 2.25.1 >> > Best Regards, Takahiro
diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index 2803536ed5..624e730524 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); + if (ret < 0) { + dev_dbg(nor->dev, + "error while reading configuration register\n"); + return -EINVAL; + } + + if (cr & CR_QUAD_EN_SPAN) + return 0; + + cr |= CR_QUAD_EN_SPAN; + + write_enable(nor); + + ret = spansion_write_any_reg(nor, addr, cr); + + if (ret < 0) { + dev_dbg(nor->dev, + "error while writing configuration register\n"); + return -EINVAL; + } + + /* Read back and check it. */ + ret = spansion_read_any_reg(nor, addr, dummy, &cr); + if (ret || !(cr & CR_QUAD_EN_SPAN)) { + dev_dbg(nor->dev, "Spansion Quad bit not set\n"); + return -EINVAL; + } + + return 0; +} + #if CONFIG_IS_ENABLED(SPI_FLASH_SFDP_SUPPORT) /** * spansion_no_read_cr_quad_enable() - set QE bit in Configuration Register.