[PATCH v2] mtd: spi-nor: Add support for flash reset

[Michal Simek]

On 2/8/26 19:35, Padmarao Begari wrote:
> Add support for spi-nor flash reset via GPIO controller
> by reading the reset-gpios property.
> 
> Signed-off-by: Venkatesh Yadav Abbarapu <venkatesh.abbarapu at amd.com>
> Signed-off-by: Padmarao Begari <padmarao.begari at amd.com>
> ---
> Change in v2:
> -Add missing gpio header in the patch
> ---
>   drivers/mtd/spi/spi-nor-core.c | 41 ++++++++++++++++++++++++++++++++++
>   include/linux/mtd/spi-nor.h    |  1 +
>   2 files changed, 42 insertions(+)
> 
> diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
> index 76c33b24368..e3de667214b 100644
> --- a/drivers/mtd/spi/spi-nor-core.c
> +++ b/drivers/mtd/spi/spi-nor-core.c
> @@ -12,6 +12,7 @@
>   #include <display_options.h>
>   #include <log.h>
>   #include <watchdog.h>
> +#include <asm-generic/gpio.h>
>   #include <dm.h>
>   #include <dm/device_compat.h>
>   #include <dm/devres.h>
> @@ -4466,6 +4467,40 @@ void spi_nor_set_fixups(struct spi_nor *nor)
>   #endif /* SPI_FLASH_MACRONIX */
>   }
>   
> +static int spi_nor_hw_reset(struct spi_nor *nor)
> +{
> +#if CONFIG_IS_ENABLED(DM_GPIO)
> +	struct udevice *dev = nor->spi->dev;
> +	int rc;
> +
> +	nor->flash_gpio_reset = devm_gpiod_get_optional(dev, "reset",
> +							GPIOD_IS_OUT |
> +							GPIOD_ACTIVE_LOW);
> +
> +	if (nor->flash_gpio_reset) {
> +		/*
> +		 * Experimental delay values by looking at different flash device
> +		 * vendors datasheets.
> +		 */
> +		udelay(5);
> +
> +		/* Toggle gpio to reset the flash device. */
> +		rc = dm_gpio_set_value(nor->flash_gpio_reset, 1);
> +		if (rc)
> +			return rc;
> +
> +		udelay(150);
> +
> +		rc = dm_gpio_set_value(nor->flash_gpio_reset, 0);
> +		if (rc)
> +			return rc;
> +
> +		udelay(1200);

I would remove all of these delays and if there is necessary to setup some times 
it should be done via gpio delay driver I sent recently.

https://lore.kernel.org/all/cover.1770105146.git.michal.simek@amd.com/


> +	}
> +#endif
> +	return 0;
> +}
> +
>   int spi_nor_scan(struct spi_nor *nor)
>   {
>   	struct spi_nor_flash_parameter params;
> @@ -4491,6 +4526,12 @@ int spi_nor_scan(struct spi_nor *nor)
>   
>   	nor->setup = spi_nor_default_setup;
>   
> +	if (CONFIG_IS_ENABLED(DM_GPIO)) {

I can't see any reason why DM_GPIO should be check here when you have it above too.

> +		ret = spi_nor_hw_reset(nor);
> +		if (ret)
> +			return ret;
> +	}
> +
>   #ifdef CONFIG_SPI_FLASH_SOFT_RESET_ON_BOOT
>   	/*
>   	 * When the flash is handed to us in a stateful mode like 8D-8D-8D, it
> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
> index 4eef4ab0488..8017d0dd9db 100644
> --- a/include/linux/mtd/spi-nor.h
> +++ b/include/linux/mtd/spi-nor.h
> @@ -583,6 +583,7 @@ struct spi_nor {
>   	u8			cmd_buf[SPI_NOR_MAX_CMD_SIZE];
>   	enum spi_nor_cmd_ext	cmd_ext_type;
>   	struct spi_nor_fixups	*fixups;
> +	struct gpio_desc	*flash_gpio_reset;

Do we need it here? Because it is used only once and never again. I don't think 
you should have it here. You can just have it as variable in spi_nor_hw_reset().

>   
>   	int (*setup)(struct spi_nor *nor, const struct flash_info *info,
>   		     const struct spi_nor_flash_parameter *params);

M