Messages in this thread | | | Date | Mon, 29 Aug 2022 12:04:38 +0200 | From | Michael Walle <> | Subject | Re: [PATCH 2/2] mtd: spi-nor: Add support for flash reset |
| |
Hi,
Am 2022-08-29 11:05, schrieb Sai Krishna Potthuri: > Add support for spi-nor flash reset via GPIO controller by reading the > reset-gpio property. If there is a valid GPIO specifier then reset will > be performed by asserting and deasserting the GPIO using gpiod APIs > otherwise it will not perform any operation. > > Signed-off-by: Sai Krishna Potthuri <sai.krishna.potthuri@amd.com> > --- > drivers/mtd/spi-nor/core.c | 50 +++++++++++++++++++++++++++++++++++--- > 1 file changed, 46 insertions(+), 4 deletions(-) > > diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c > index f2c64006f8d7..d4703ff69ad0 100644 > --- a/drivers/mtd/spi-nor/core.c > +++ b/drivers/mtd/spi-nor/core.c > @@ -2401,12 +2401,8 @@ static void spi_nor_no_sfdp_init_params(struct > spi_nor *nor) > */ > static void spi_nor_init_flags(struct spi_nor *nor) > { > - struct device_node *np = spi_nor_get_flash_node(nor); > const u16 flags = nor->info->flags; > > - if (of_property_read_bool(np, "broken-flash-reset")) > - nor->flags |= SNOR_F_BROKEN_RESET; > - > if (flags & SPI_NOR_SWP_IS_VOLATILE) > nor->flags |= SNOR_F_SWP_IS_VOLATILE; > > @@ -2933,9 +2929,47 @@ static void spi_nor_set_mtd_info(struct spi_nor > *nor) > mtd->_put_device = spi_nor_put_device; > } > > +static int spi_nor_hw_reset(struct spi_nor *nor) > +{ > + struct gpio_desc *reset; > + int ret; > + > + reset = devm_gpiod_get_optional(nor->dev, "reset", GPIOD_ASIS);
devm_gpiod_get_optional(nor->dev, "reset", GPIOD_OUT_HIGH);
> + if (IS_ERR_OR_NULL(reset)) > + return PTR_ERR_OR_ZERO(reset); > + > + /* Set the direction as output and enable the output */ > + ret = gpiod_direction_output(reset, 1);
Not necessary then.
> + if (ret) > + return ret; > + > + /* > + * Experimental Minimum Chip select high to Reset delay value > + * based on the flash device spec. > + */
Which flash device spec?
> + usleep_range(1, 5); > + gpiod_set_value(reset, 0);
Mh, is your logic inverted here? If I read the code correctly, you should use a value of 1 to take the device into reset. The device tree should then have a flag "active low", which will invert the value here. Also please use the cansleep() variant.
> + /* > + * Experimental Minimum Reset pulse width value based on the > + * flash device spec. > + */ > + usleep_range(10, 15); > + gpiod_set_value(reset, 1); > + > + /* > + * Experimental Minimum Reset recovery delay value based on the > + * flash device spec. > + */ > + usleep_range(35, 40); > + > + return 0; > +} > + > int spi_nor_scan(struct spi_nor *nor, const char *name, > const struct spi_nor_hwcaps *hwcaps) > { > + struct device_node *np = spi_nor_get_flash_node(nor); > const struct flash_info *info; > struct device *dev = nor->dev; > struct mtd_info *mtd = &nor->mtd; > @@ -2965,6 +2999,14 @@ int spi_nor_scan(struct spi_nor *nor, const char > *name, > if (!nor->bouncebuf) > return -ENOMEM; > > + if (of_property_read_bool(np, "broken-flash-reset")) { > + nor->flags |= SNOR_F_BROKEN_RESET; > + } else { > + ret = spi_nor_hw_reset(nor); > + if (ret) > + return ret; > + }
This should be done unconditionally, no? Even if the reset pin is broken, we know we have one (otherwise the device tree would be broken) and we can do a reset in any case.
Also, which tree are you using? That was moved into spi_nor_init_flags() some time ago. Please rebase to latest spi-next.
-michael
> + > info = spi_nor_get_flash_info(nor, name); > if (IS_ERR(info)) > return PTR_ERR(info);
| |