[PATCH v2 3/4] Revert "mtd: spi-nor-core: Perform a Soft Reset on boot"

[Pratyush Yadav]

Hi Tudor,

On 04/11/21 01:49AM, Tudor Ambarus wrote:
> This reverts commit 0be8ab1f166844d53477387dc9a1184161ef44ef.
> 
> There are multiple reset commands and it was used one at guess,

Correct.

> see BFPT[dword(16)]. It is preferable to avoid issuing unsupported
> commands to a flash. Since there's no config in mainline that actually
> uses SPI_FLASH_SOFT_RESET_ON_BOOT, remove it entirely until proper

I have been lagging behind on that front. I have some defconfig patches 
for TI platforms but I did not get around to cleaning them up and 
posting them upstream. Still, this feature is very much needed on TI 
platforms.

> support is added. One should instead determine the mode in which the
> flash device is configured, then to parse SFDP to determine the
> cmd_ext_type and then to issue a READID command if flash identification
> is really a must. JESD216D-01 proposes an algorithm to try to read the

Firstly, Read ID command is not standardized in 8D-8D-8D mode. For 
example, Cypress S28 flash family expects 4 address bytes for Read ID in 
8D-8D-8D mode whereas Micron MT35 flash family does not. Number of dummy 
cycles also tends to vary. There is no way to determine these parameters 
from SFDP.

Also, you would have to run the detection algorithm for _every_ flash 
that SPI NOR supports since we don't really know what we are dealing 
with at this point. This would include flashes that do not support the 
Read SFDP command at all. If your goal is to not send unsupported 
commands to a flash then you won't get very far. On top of that Read 
SFDP is not mandatory in 8D-8D-8D mode per the xSPI standard so there is 
no guarantee that the detection algorithm would even work. It _should_ 
work for most flashes but we will eventually have to deal with the 
corner cases.

In other words, if you drop this then you would have to run the 
detection algorithm for every flash and see what mode it is in. If an 
8D-8D-8D mode flash does support Read SFDP in 8D-8D-8D mode, then you 
would have to read SFDP, determine the extension and reset type, and 
then perform the reset. If it does not support the Read SFDP command 
then you are left with a non-working flash. You would still probably end 
up issuing unsupported commands.

I can implement such an algorithm but is it really worth the hassle?

> SFDP signature to determine the mode in which the flash is configured:
> '''
> try to read the SFDP signature (see 6.1) in 4-4-4 mode, if that fails
> try 2-2-2 mode, and if that fails try 1-1-1 mode. For Octal devices,
> these typically support SFDP read operation in both 1S-1S-1S mode and
> 8D-8D-8D mode. If the host controller does not know exactly which
> protocol mode is used for SFDP in 8D-8D-8D mode, this information can be
> found by reading SFDP in 1S-1S-1S mode first. (To read an unknown device
> directly in 8D-8D-8D mode, the host controller may read from address 0,
> and count the number of dummy clocks required before the SFDP signature
> is received.)
> '''
> If the flash does not support SFDP at all, one should introduce dedicated
> configs for each reset type and issue just the needed reset command.
> 
> Signed-off-by: Tudor Ambarus <tudor.ambarus at microchip.com>
> ---
>  drivers/mtd/spi/Kconfig        |  9 ---------
>  drivers/mtd/spi/spi-nor-core.c | 27 ---------------------------
>  2 files changed, 36 deletions(-)
> 
> diff --git a/drivers/mtd/spi/Kconfig b/drivers/mtd/spi/Kconfig
> index 2d2b71c52d..14800b5e93 100644
> --- a/drivers/mtd/spi/Kconfig
> +++ b/drivers/mtd/spi/Kconfig
> @@ -103,15 +103,6 @@ config SPI_FLASH_SOFT_RESET
>  	 Enable support for xSPI Software Reset. It will be used to switch from
>  	 Octal DTR mode to legacy mode on shutdown and boot (if enabled).
>  
> -config SPI_FLASH_SOFT_RESET_ON_BOOT
> -	bool "Perform a Software Reset on boot on flashes that boot in stateful mode"
> -	depends on SPI_FLASH_SOFT_RESET
> -	help
> -	 Perform a Software Reset on boot to allow detecting flashes that are
> -	 handed to us in Octal DTR mode. Do not enable this config on flashes
> -	 that are not supposed to be handed to U-Boot in Octal DTR mode, even
> -	 if they _do_ support the Soft Reset sequence.
> -
>  config SPI_FLASH_BAR
>  	bool "SPI flash Bank/Extended address register support"
>  	help
> diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
> index df66a73495..caf764720c 100644
> --- a/drivers/mtd/spi/spi-nor-core.c
> +++ b/drivers/mtd/spi/spi-nor-core.c
> @@ -3796,33 +3796,6 @@ 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 */
> -
>  	info = spi_nor_read_id(nor);
>  	if (IS_ERR_OR_NULL(info))
>  		return -ENOENT;
> -- 
> 2.25.1
> 

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.