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#L34...
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