>
> On 25/10/21 12:30PM, Jagan Teki wrote:
> > On Mon, Oct 18, 2021 at 11:54 AM JaimeLiao <jaimeliao...@gmail.com> 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.
> > >
> > > Command extension type is not standardized across flash vendors in DTR
> > > mode.
> > >
> > > For suiting different vendor flash devices, adding a flag to seperate
> > > types for
> > > soft reset on boot.
> > >
> > > Signed-off-by: JaimeLiao <jaimeliao...@gmail.com>
> > > ---
> > > drivers/mtd/spi/Kconfig | 7 +++++++
> > > drivers/mtd/spi/spi-nor-core.c | 7 ++++++-
> > > 2 files changed, 13 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/mtd/spi/Kconfig b/drivers/mtd/spi/Kconfig
> > > index 67599b32c9..8304d6c973 100644
> > > --- a/drivers/mtd/spi/Kconfig
> > > +++ b/drivers/mtd/spi/Kconfig
> > > @@ -97,6 +97,13 @@ 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_BOOT_SOFT_RESET_EXT_INVERT
> > > + bool "Command extension type is INVERT for Software Reset on boot"
> > > + default n
> > > + help
> > > + Because of SFDP information can not be get before boot.
> > > + So define command extension type is INVERT when Software Reset
> > > on boot only.
> > > +
> > > 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 fdaca0a0d3..87a7de7d60 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 == SPI_NOR_EXT_NONE) {
> > > + nor->cmd_ext_type = SPI_NOR_EXT_REPEAT;
> > > +#if CONFIG_IS_ENABLED(SPI_NOR_BOOT_SOFT_RESET_EXT_INVERT)
> > > + nor->cmd_ext_type = SPI_NOR_EXT_INVERT;
> > > +#endif /* SPI_NOR_BOOT_SOFT_RESET_EXT_INVERT */
> >
> > I think we can get the SPI_NOR_EXT_INVERT by parsing bfpt, can you try
> > it that way instead of new macro?
>
> The problem is that for OSPI boot mode the ROM can often leave the flash
> in 8D-8D-8D mode. So when U-Boot first starts executing the flash is
> already in 8D-8D-8D mode and there is no easy and reliable way to detect
> this mode. So we must blindly perform a soft reset before probing the
> flash and reading SFDP so that it is reset back to a usable state.
>
> This is why we can't detect the extension via BFPT since the flash is in
> an unknown state. This is not the case when resetting before booting the
> OS since at that point we have fully probed the flash. So I think this
> config must only be used for the reset at boot time. For later resets we
> should indeed use the extension provided by BFPT.
>
> This is a kinda hacky but I can't come up with a better alternative.
>
> Jamie,
>
> Below diff is what I have been suggesting you in my earlier replies.
> Note that this is just a quick and dirty implementation, I have not
> tested this at all. That is up to you to do. Please also test soft reset
> _after_ the probe is finished so we know that it works correctly when
> using data from BFPT as well.
>
> -- 8< --
> diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
> index 4388a08a90..b0f22e0ce5 100644
> --- a/drivers/mtd/spi/spi-nor-core.c
> +++ b/drivers/mtd/spi/spi-nor-core.c
> @@ -3616,10 +3616,6 @@ static int spi_nor_soft_reset(struct spi_nor *nor)
> {
> struct spi_mem_op op;
> int ret;
> - enum spi_nor_cmd_ext ext;
> -
> - ext = nor->cmd_ext_type;
> - 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,
> @@ -3629,7 +3625,7 @@ static int spi_nor_soft_reset(struct spi_nor *nor)
> ret = spi_mem_exec_op(nor->spi, &op);
> if (ret) {
> dev_warn(nor->dev, "Software reset enable failed: %d\n", ret);
> - goto out;
> + return ret;
> }
>
> op = (struct spi_mem_op)SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_SRST, 0),
> @@ -3640,7 +3636,7 @@ static int spi_nor_soft_reset(struct spi_nor *nor)
> ret = spi_mem_exec_op(nor->spi, &op);
> if (ret) {
> dev_warn(nor->dev, "Software reset failed: %d\n", ret);
> - goto out;
> + return ret;
> }
>
> /*
> @@ -3650,9 +3646,7 @@ static int spi_nor_soft_reset(struct spi_nor *nor)
> */
> udelay(SPI_NOR_SRST_SLEEP_LEN);
>
> -out:
> - nor->cmd_ext_type = ext;
> - return ret;
> + return 0;
> }
> #endif /* CONFIG_SPI_FLASH_SOFT_RESET */
>
> @@ -3698,6 +3692,44 @@ void spi_nor_set_fixups(struct spi_nor *nor)
> #endif
> }
>
> +/*
> + * When the flash is handed to us in a stateful mode like 8D-8D-8D, it is
> + * difficult to detect the mode the flash is in. One option is to read SFDP
> in
> + * all modes and see which one gives the correct "SFDP" signature, but not
> all
> + * flashes support SFDP in 8D-8D-8D mode.
> + *
> + * Further, even if you detect the mode of the flash via SFDP, you still have
> + * the problem of actually reading the ID. The Read ID command is not
> + * standardized across flash vendors. Flashes can have different dummy cycles
> + * needed for reading the ID. Some flashes even expect a 4-byte dummy address
> + * with the Read ID command. All this information cannot be obtained from the
> + * SFDP table.
> + *
> + * So, perform a Software Reset sequence before reading the ID and
> initializing
> + * the flash. A Soft Reset will bring back the flash in its default protocol
> + * mode assuming no non-volatile configuration was set. This will let us
> detect
> + * the flash even if ROM hands it to us in Octal DTR mode.
> + *
> + * To accommodate cases where there is more than one flash on a board, and
> only
> + * one of them needs a soft reset, failure to reset is not made fatal, and we
> + * still try to read ID if possible.
> + */
> +static void spi_nor_soft_reset_on_boot(struct spi_nor *nor)
> +{
> + enum spi_nor_cmd_ext ext;
> +
> + ext = nor->cmd_ext_type;
> +
> + if (CONFIG_IS_ENABLED(SPI_NOR_BOOT_SOFT_RESET_EXT_INVERT))
> + nor->cmd_ext_type = SPI_NOR_EXT_INVERT;
> + else
> + nor->cmd_ext_type = SPI_NOR_EXT_REPEAT;
> +
> + spi_nor_soft_reset(nor);
> +
> + nor->cmd_ext_type = ext;
> +}
> +
> int spi_nor_scan(struct spi_nor *nor)
> {
> struct spi_nor_flash_parameter params;
> @@ -3722,32 +3754,8 @@ int spi_nor_scan(struct spi_nor *nor)
>
> nor->setup = spi_nor_default_setup;
>
> -#ifdef CONFIG_SPI_FLASH_SOFT_RESET_ON_BOOT
> - /*
> - * When the flash is handed to us in a stateful mode like 8D-8D-8D, it
> - * is difficult to detect the mode the flash is in. One option is to
> - * read SFDP in all modes and see which one gives the correct "SFDP"
> - * signature, but not all flashes support SFDP in 8D-8D-8D mode.
> - *
> - * Further, even if you detect the mode of the flash via SFDP, you
> - * still have the problem of actually reading the ID. The Read ID
> - * command is not standardized across flash vendors. Flashes can have
> - * different dummy cycles needed for reading the ID. Some flashes even
> - * expect a 4-byte dummy address with the Read ID command. All this
> - * information cannot be obtained from the SFDP table.
> - *
> - * So, perform a Software Reset sequence before reading the ID and
> - * initializing the flash. A Soft Reset will bring back the flash in
> - * its default protocol mode assuming no non-volatile configuration
> was
> - * set. This will let us detect the flash even if ROM hands it to us
> in
> - * Octal DTR mode.
> - *
> - * To accommodate cases where there is more than one flash on a board,
> - * and only one of them needs a soft reset, failure to reset is not
> - * made fatal, and we still try to read ID if possible.
> - */
> - spi_nor_soft_reset(nor);
> -#endif /* CONFIG_SPI_FLASH_SOFT_RESET_ON_BOOT */
> + if (CONFIG_IS_ENABLED(SPI_FLASH_SOFT_RESET_ON_BOOT))
> + spi_nor_soft_reset_on_boot(nor);
>
> info = spi_nor_read_id(nor);
> if (IS_ERR_OR_NULL(info))
>
> --
For validating soft_reset after probe, I have test it on my side.
It is working with command extension type which got from BFPT.
> Regards,
> Pratyush Yadav
> Texas Instruments Inc.
