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

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

[Venkatesh Yadav Abbarapu]

Add support for spi-nor flash reset via GPIO controller
by reading the reset-gpios property.

[Ported from Linux kernel commit
8f1ee9ef71d0 ("mtd: spi-nor: Add support for flash reset") ]

Signed-off-by: Venkatesh Yadav Abbarapu <venkatesh.abbarapu@amd.com>
---
 drivers/mtd/spi/spi-nor-core.c | 44 ++++++++++++++++++++++++++++++++++
 include/linux/mtd/spi-nor.h    |  1 +
 2 files changed, 45 insertions(+)

diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
index f5c9868bbc..900f66b0e8 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/gpio.h>
 #include <dm.h>
 #include <dm/device_compat.h>
 #include <dm/devres.h>
@@ -4401,7 +4402,13 @@ int spi_nor_remove(struct spi_nor *nor)
 	    nor->flags & SNOR_F_SOFT_RESET)
 		return spi_nor_soft_reset(nor);
 #endif
+	if (CONFIG_IS_ENABLED(DM_GPIO)) {
+		if (nor->flash_gpio_reset) {
+			struct gpio_desc *flash_gpio_reset = nor->flash_gpio_reset;
 
+			dm_gpio_free(flash_gpio_reset->dev, flash_gpio_reset);
+		}
+	}
 	return 0;
 }
 
@@ -4448,6 +4455,37 @@ void spi_nor_set_fixups(struct spi_nor *nor)
 #endif /* SPI_FLASH_MACRONIX */
 }
 
+static int spi_nor_hw_reset(struct spi_nor *nor)
+{
+	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);
+	}
+	return 0;
+}
+
 int spi_nor_scan(struct spi_nor *nor)
 {
 	struct spi_nor_flash_parameter params;
@@ -4473,6 +4511,12 @@ int spi_nor_scan(struct spi_nor *nor)
 
 	nor->setup = spi_nor_default_setup;
 
+	if (CONFIG_IS_ENABLED(DM_GPIO)) {
+		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 655a6d197e..7c5337c636 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;
 
 	int (*setup)(struct spi_nor *nor, const struct flash_info *info,
 		     const struct spi_nor_flash_parameter *params);
-- 
2.17.1

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

[Quentin Schulz]

Hi Venkatesh Yadav Abbarapu,

On 10/28/24 8:03 AM, Venkatesh Yadav Abbarapu wrote:
> Add support for spi-nor flash reset via GPIO controller
> by reading the reset-gpios property.
> 
> [Ported from Linux kernel commit
> 8f1ee9ef71d0 ("mtd: spi-nor: Add support for flash reset") ]
> 
> Signed-off-by: Venkatesh Yadav Abbarapu <venkatesh.abbarapu@amd.com>
> ---
>   drivers/mtd/spi/spi-nor-core.c | 44 ++++++++++++++++++++++++++++++++++
>   include/linux/mtd/spi-nor.h    |  1 +
>   2 files changed, 45 insertions(+)
> 
> diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
> index f5c9868bbc..900f66b0e8 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/gpio.h>
>   #include <dm.h>
>   #include <dm/device_compat.h>
>   #include <dm/devres.h>
> @@ -4401,7 +4402,13 @@ int spi_nor_remove(struct spi_nor *nor)
>   	    nor->flags & SNOR_F_SOFT_RESET)
>   		return spi_nor_soft_reset(nor);
>   #endif
> +	if (CONFIG_IS_ENABLED(DM_GPIO)) {
> +		if (nor->flash_gpio_reset) {
> +			struct gpio_desc *flash_gpio_reset = nor->flash_gpio_reset;
>   
> +			dm_gpio_free(flash_gpio_reset->dev, flash_gpio_reset);

Since we get the gpio descriptor with a devres-managed function, I think 
this is unnecessary as the core should do it by itself when removing

> +		}
> +	}
>   	return 0;
>   }
>   
> @@ -4448,6 +4455,37 @@ void spi_nor_set_fixups(struct spi_nor *nor)
>   #endif /* SPI_FLASH_MACRONIX */
>   }
>   
> +static int spi_nor_hw_reset(struct spi_nor *nor)
> +{
> +	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);

This is a bit odd, I would have assumed a proper reset like 
mmc-pwrseq-emmc or mmc-pwrseq-simple but for nor would have been more 
appropriate than just guessing which timing would cover all NORs?

> +	}
> +	return 0;
> +}
> +
>   int spi_nor_scan(struct spi_nor *nor)
>   {
>   	struct spi_nor_flash_parameter params;
> @@ -4473,6 +4511,12 @@ int spi_nor_scan(struct spi_nor *nor)

This function also exists in spi-nor-tiny, should we add support for the 
hw-reset there as well?

Cheers,
Quentin

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

[Abbarapu, Venkatesh]

Hi,

> -----Original Message-----
> From: Quentin Schulz <quentin.schulz@cherry.de>
> Sent: Monday, October 28, 2024 3:48 PM
> To: Abbarapu, Venkatesh <venkatesh.abbarapu@amd.com>; u-boot@lists.denx.de
> Cc: Simek, Michal <michal.simek@amd.com>; jagan@amarulasolutions.com;
> vigneshr@ti.com; p.yadav@ti.com; trini@konsulko.com; tudor.ambarus@linaro.org;
> marex@denx.de; sjg@chromium.org; git (AMD-Xilinx) <git@amd.com>
> Subject: Re: [PATCH] mtd: spi-nor: Add support for flash reset
> 
> Hi Venkatesh Yadav Abbarapu,
> 
> On 10/28/24 8:03 AM, Venkatesh Yadav Abbarapu wrote:
> > Add support for spi-nor flash reset via GPIO controller by reading the
> > reset-gpios property.
> >
> > [Ported from Linux kernel commit
> > 8f1ee9ef71d0 ("mtd: spi-nor: Add support for flash reset") ]
> >
> > Signed-off-by: Venkatesh Yadav Abbarapu <venkatesh.abbarapu@amd.com>
> > ---
> >   drivers/mtd/spi/spi-nor-core.c | 44 ++++++++++++++++++++++++++++++++++
> >   include/linux/mtd/spi-nor.h    |  1 +
> >   2 files changed, 45 insertions(+)
> >
> > diff --git a/drivers/mtd/spi/spi-nor-core.c
> > b/drivers/mtd/spi/spi-nor-core.c index f5c9868bbc..900f66b0e8 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/gpio.h>
> >   #include <dm.h>
> >   #include <dm/device_compat.h>
> >   #include <dm/devres.h>
> > @@ -4401,7 +4402,13 @@ int spi_nor_remove(struct spi_nor *nor)
> >   	    nor->flags & SNOR_F_SOFT_RESET)
> >   		return spi_nor_soft_reset(nor);
> >   #endif
> > +	if (CONFIG_IS_ENABLED(DM_GPIO)) {
> > +		if (nor->flash_gpio_reset) {
> > +			struct gpio_desc *flash_gpio_reset = nor->flash_gpio_reset;
> >
> > +			dm_gpio_free(flash_gpio_reset->dev, flash_gpio_reset);
> 
> Since we get the gpio descriptor with a devres-managed function, I think this is
> unnecessary as the core should do it by itself when removing
> 
> > +		}
> > +	}
> >   	return 0;
> >   }
> >
> > @@ -4448,6 +4455,37 @@ void spi_nor_set_fixups(struct spi_nor *nor)
> >   #endif /* SPI_FLASH_MACRONIX */
> >   }
> >
> > +static int spi_nor_hw_reset(struct spi_nor *nor) {
> > +	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);
> 
> This is a bit odd, I would have assumed a proper reset like mmc-pwrseq-emmc or
> mmc-pwrseq-simple but for nor would have been more appropriate than just
> guessing which timing would cover all NORs?
These delays have been updated by looking at different flash vendor datasheets as per below reference
https://github.com/torvalds/linux/blob/master/drivers/mtd/spi-nor/core.c#L3428

Do you have anything with respect to these delay values?

Thanks
Venkatesh
> 
> > +	}
> > +	return 0;
> > +}
> > +
> >   int spi_nor_scan(struct spi_nor *nor)
> >   {
> >   	struct spi_nor_flash_parameter params; @@ -4473,6 +4511,12 @@ int
> > spi_nor_scan(struct spi_nor *nor)
> 
> This function also exists in spi-nor-tiny, should we add support for the hw-reset there
> as well?
> 
> Cheers,
> Quentin

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

[Padmarao Begari]

Add support for spi-nor flash reset via GPIO controller
by reading the reset-gpios property.

Signed-off-by: Venkatesh Yadav Abbarapu <venkatesh.abbarapu@amd.com>
Signed-off-by: Padmarao Begari <padmarao.begari@amd.com>
---
 drivers/mtd/spi/spi-nor-core.c | 40 ++++++++++++++++++++++++++++++++++
 include/linux/mtd/spi-nor.h    |  1 +
 2 files changed, 41 insertions(+)

diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
index 76c33b24368..0c132a39190 100644
--- a/drivers/mtd/spi/spi-nor-core.c
+++ b/drivers/mtd/spi/spi-nor-core.c
@@ -4466,6 +4466,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);
+	}
+#endif
+	return 0;
+}
+
 int spi_nor_scan(struct spi_nor *nor)
 {
 	struct spi_nor_flash_parameter params;
@@ -4491,6 +4525,12 @@ int spi_nor_scan(struct spi_nor *nor)
 
 	nor->setup = spi_nor_default_setup;
 
+	if (CONFIG_IS_ENABLED(DM_GPIO)) {
+		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;
 
 	int (*setup)(struct spi_nor *nor, const struct flash_info *info,
 		     const struct spi_nor_flash_parameter *params);
-- 
2.34.1