On 13/09/21 01:42PM, JaimeLiao wrote: > Power-on-Reset is a method to restore flash back to 1S-1S-1S mode from > 8D-8D-8D > in the begging of probe.
Typo: begging -> beginning > > Command extension type is not standardized across flash vendors in DTR mode. > > For suiting different vendor flash devices, adding a flag to seperate types if > nor->cmd_ext_type didn't configure from SFDP. No, the code does not do what the commit message says. The code only sets the extension for soft reset, which is then reset back to the default at the end of the function. Then we read from SFDP and do the usual initialization. > > Signed-off-by: JaimeLiao <jaimeliao...@gmail.com> > --- > drivers/mtd/spi/Kconfig | 6 ++++++ > drivers/mtd/spi/spi-nor-core.c | 7 ++++++- > 2 files changed, 12 insertions(+), 1 deletion(-) > > diff --git a/drivers/mtd/spi/Kconfig b/drivers/mtd/spi/Kconfig > index 67599b32c9..d850480401 100644 > --- a/drivers/mtd/spi/Kconfig > +++ b/drivers/mtd/spi/Kconfig > @@ -97,6 +97,12 @@ config SPI_FLASH_SMART_HWCAPS > can support a type of operation in a much more refined way compared > to using flags like SPI_RX_DUAL, SPI_TX_QUAD, etc. > > +config SPI_NOR_CMD_EXT_INVERT Let's only have this extension for the soft reset on boot, and then let the usual initialization process discover it via SFDP. This will make the code a little less complicated IMO. So I suggest calling it SPI_NOR_BOOT_SOFT_RESET_EXT. > + bool "Command extension type is INVERT for SPI NOR flashed" > + default n > + help > + Define command extension type is INVERT. And explain here that this only sets the extension for the soft reset on boot since we can't discover it at that point. Then we drop the information and rediscover it as normal via SFDP or flash fixup hooks. > + > config SPI_FLASH_SOFT_RESET > bool "Software Reset support for SPI NOR flashes" > default n > diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c > index b8dda02aa7..4bcd58d839 100644 > --- a/drivers/mtd/spi/spi-nor-core.c > +++ b/drivers/mtd/spi/spi-nor-core.c > @@ -3661,7 +3661,12 @@ static int spi_nor_soft_reset(struct spi_nor *nor) > enum spi_nor_cmd_ext ext; > > ext = nor->cmd_ext_type; > - nor->cmd_ext_type = SPI_NOR_EXT_REPEAT; > + if (!nor->cmd_ext_type) { Use (nor->cmd_ext_type == SPI_NOR_EXT_NONE). Also add a comment explaining here why you do this check. > + nor->cmd_ext_type = SPI_NOR_EXT_REPEAT; > +#ifdef CONFIG_SPI_NOR_CMD_EXT_INVERT > + nor->cmd_ext_type = SPI_NOR_EXT_INVERT; > +#endif Avoid using #ifdef. You can replace it with if (CONFIG_IS_ENABLED(SPI_NOR_CMD_EXT_INVERT)) nor->cmd_ext_type = SPI_NOR_EXT_INVERT; else nor->cmd_ext_type = SPI_NOR_EXT_REPEAT; > + } > > op = (struct spi_mem_op)SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_SRSTEN, 0), > SPI_MEM_OP_NO_DUMMY, > -- > 2.17.1 > -- Regards, Pratyush Yadav Texas Instruments Inc.