* [PATCH 0/2] mtd: spi-nor: Add flash device reset support @ 2022-08-29 9:05 Sai Krishna Potthuri 2022-08-29 9:05 ` [PATCH 1/2] dt-bindings: mtd: spi-nor: Add reset-gpios property Sai Krishna Potthuri 2022-08-29 9:05 ` [PATCH 2/2] mtd: spi-nor: Add support for flash reset Sai Krishna Potthuri 0 siblings, 2 replies; 12+ messages in thread From: Sai Krishna Potthuri @ 2022-08-29 9:05 UTC (permalink / raw) To: Tudor Ambarus, Pratyush Yadav, Michael Walle, Miquel Raynal, Richard Weinberger, Vignesh Raghavendra, Rob Herring, Krzysztof Kozlowski Cc: devicetree, linux-mtd, linux-kernel, saikrishna12468, git, Sai Krishna Potthuri This patch series update spi-nor dt-binding and spi-nor core to support flash device reset using reset-gpios property. Sai Krishna Potthuri (2): dt-bindings: mtd: spi-nor: Add reset-gpios property mtd: spi-nor: Add support for flash reset .../bindings/mtd/jedec,spi-nor.yaml | 6 +++ drivers/mtd/spi-nor/core.c | 50 +++++++++++++++++-- 2 files changed, 52 insertions(+), 4 deletions(-) -- 2.17.1 ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/2] dt-bindings: mtd: spi-nor: Add reset-gpios property 2022-08-29 9:05 [PATCH 0/2] mtd: spi-nor: Add flash device reset support Sai Krishna Potthuri @ 2022-08-29 9:05 ` Sai Krishna Potthuri 2022-08-30 9:21 ` Krzysztof Kozlowski 2022-08-29 9:05 ` [PATCH 2/2] mtd: spi-nor: Add support for flash reset Sai Krishna Potthuri 1 sibling, 1 reply; 12+ messages in thread From: Sai Krishna Potthuri @ 2022-08-29 9:05 UTC (permalink / raw) To: Tudor Ambarus, Pratyush Yadav, Michael Walle, Miquel Raynal, Richard Weinberger, Vignesh Raghavendra, Rob Herring, Krzysztof Kozlowski Cc: devicetree, linux-mtd, linux-kernel, saikrishna12468, git, Sai Krishna Potthuri SPI-NOR flashes have RESET pin which can be toggled using GPIO controller, for those platforms reset-gpios property can be used to reset the flash device. Signed-off-by: Sai Krishna Potthuri <sai.krishna.potthuri@amd.com> --- Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml index 7149784a36ac..d2fc8e9c787f 100644 --- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml +++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml @@ -70,6 +70,12 @@ properties: be used on such systems, to denote the absence of a reliable reset mechanism. + reset-gpios: + description: + contains a GPIO specifier. The reset GPIO is asserted and then deasserted + to perform device reset. If "broken-flash-reset" is present then having + this property does not make any difference. + partitions: type: object -- 2.17.1 ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] dt-bindings: mtd: spi-nor: Add reset-gpios property 2022-08-29 9:05 ` [PATCH 1/2] dt-bindings: mtd: spi-nor: Add reset-gpios property Sai Krishna Potthuri @ 2022-08-30 9:21 ` Krzysztof Kozlowski 2022-08-30 9:36 ` Michael Walle 0 siblings, 1 reply; 12+ messages in thread From: Krzysztof Kozlowski @ 2022-08-30 9:21 UTC (permalink / raw) To: Sai Krishna Potthuri, Tudor Ambarus, Pratyush Yadav, Michael Walle, Miquel Raynal, Richard Weinberger, Vignesh Raghavendra, Rob Herring, Krzysztof Kozlowski Cc: devicetree, linux-mtd, linux-kernel, saikrishna12468, git On 29/08/2022 12:05, Sai Krishna Potthuri wrote: > SPI-NOR flashes have RESET pin which can be toggled using GPIO > controller, for those platforms reset-gpios property can be used to > reset the flash device. > > Signed-off-by: Sai Krishna Potthuri <sai.krishna.potthuri@amd.com> > --- > Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml > index 7149784a36ac..d2fc8e9c787f 100644 > --- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml > +++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml > @@ -70,6 +70,12 @@ properties: > be used on such systems, to denote the absence of a reliable reset > mechanism. > > + reset-gpios: > + description: > + contains a GPIO specifier. Skip this part - obvious. > The reset GPIO is asserted and then deasserted > + to perform device reset. If "broken-flash-reset" is present then having > + this property does not make any difference. Isn't then broken-flash-reset conflicting with this one (e.g. disallowing it)? Best regards, Krzysztof ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] dt-bindings: mtd: spi-nor: Add reset-gpios property 2022-08-30 9:21 ` Krzysztof Kozlowski @ 2022-08-30 9:36 ` Michael Walle 2022-08-30 9:48 ` Krzysztof Kozlowski 0 siblings, 1 reply; 12+ messages in thread From: Michael Walle @ 2022-08-30 9:36 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: Sai Krishna Potthuri, Tudor Ambarus, Pratyush Yadav, Miquel Raynal, Richard Weinberger, Vignesh Raghavendra, Rob Herring, Krzysztof Kozlowski, devicetree, linux-mtd, linux-kernel, saikrishna12468, git Am 2022-08-30 11:21, schrieb Krzysztof Kozlowski: > On 29/08/2022 12:05, Sai Krishna Potthuri wrote: >> SPI-NOR flashes have RESET pin which can be toggled using GPIO >> controller, for those platforms reset-gpios property can be used to >> reset the flash device. >> >> Signed-off-by: Sai Krishna Potthuri <sai.krishna.potthuri@amd.com> >> --- >> Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml >> b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml >> index 7149784a36ac..d2fc8e9c787f 100644 >> --- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml >> +++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml >> @@ -70,6 +70,12 @@ properties: >> be used on such systems, to denote the absence of a reliable >> reset >> mechanism. >> >> + reset-gpios: >> + description: >> + contains a GPIO specifier. > > Skip this part - obvious. > >> The reset GPIO is asserted and then deasserted >> + to perform device reset. If "broken-flash-reset" is present >> then having >> + this property does not make any difference. > > Isn't then broken-flash-reset conflicting with this one (e.g. > disallowing it)? Sometimes the spi-nor driver needs to switch modes, which are persistent until you either switch em back or do a hardware reset (or software reset IIRC) of the flash. If broken-flash-reset is set, we try hard to leave the flash in the mode which it is normally in after reset or don't switch modes at all. Of course we cannot make sure, our shutdown gets called in each case, thus there is may be warning during startup. So, even if you have a reset-gpio it might be broken I guess. Think of it being high active, but someone forgot the pull-up. So, if you do an unexpected reset, the flash chip might not be reset automatically. So yes, I think, even if there is a dedicated reset gpio, it could still be messed up. How likely is it? I don't know, probably not very. -michael ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] dt-bindings: mtd: spi-nor: Add reset-gpios property 2022-08-30 9:36 ` Michael Walle @ 2022-08-30 9:48 ` Krzysztof Kozlowski 0 siblings, 0 replies; 12+ messages in thread From: Krzysztof Kozlowski @ 2022-08-30 9:48 UTC (permalink / raw) To: Michael Walle Cc: Sai Krishna Potthuri, Tudor Ambarus, Pratyush Yadav, Miquel Raynal, Richard Weinberger, Vignesh Raghavendra, Rob Herring, Krzysztof Kozlowski, devicetree, linux-mtd, linux-kernel, saikrishna12468, git On 30/08/2022 12:36, Michael Walle wrote: > Am 2022-08-30 11:21, schrieb Krzysztof Kozlowski: >> On 29/08/2022 12:05, Sai Krishna Potthuri wrote: >>> SPI-NOR flashes have RESET pin which can be toggled using GPIO >>> controller, for those platforms reset-gpios property can be used to >>> reset the flash device. >>> >>> Signed-off-by: Sai Krishna Potthuri <sai.krishna.potthuri@amd.com> >>> --- >>> Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml | 6 ++++++ >>> 1 file changed, 6 insertions(+) >>> >>> diff --git a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml >>> b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml >>> index 7149784a36ac..d2fc8e9c787f 100644 >>> --- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml >>> +++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml >>> @@ -70,6 +70,12 @@ properties: >>> be used on such systems, to denote the absence of a reliable >>> reset >>> mechanism. >>> >>> + reset-gpios: >>> + description: >>> + contains a GPIO specifier. >> >> Skip this part - obvious. >> >>> The reset GPIO is asserted and then deasserted >>> + to perform device reset. If "broken-flash-reset" is present >>> then having >>> + this property does not make any difference. >> >> Isn't then broken-flash-reset conflicting with this one (e.g. >> disallowing it)? > > Sometimes the spi-nor driver needs to switch modes, which are persistent > until you either switch em back or do a hardware reset (or software > reset IIRC) of the flash. If broken-flash-reset is set, we try hard > to leave the flash in the mode which it is normally in after reset or > don't switch modes at all. > Of course we cannot make sure, our shutdown gets called in each case, > thus there is may be warning during startup. > > So, even if you have a reset-gpio it might be broken I guess. Think > of it being high active, but someone forgot the pull-up. So, if you > do an unexpected reset, the flash chip might not be reset > automatically. So yes, I think, even if there is a dedicated reset > gpio, it could still be messed up. How likely is it? I don't know, > probably not very. OK, so let's keep it and allow both. Best regards, Krzysztof ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/2] mtd: spi-nor: Add support for flash reset 2022-08-29 9:05 [PATCH 0/2] mtd: spi-nor: Add flash device reset support Sai Krishna Potthuri 2022-08-29 9:05 ` [PATCH 1/2] dt-bindings: mtd: spi-nor: Add reset-gpios property Sai Krishna Potthuri @ 2022-08-29 9:05 ` Sai Krishna Potthuri 2022-08-29 10:04 ` Michael Walle 2022-09-01 1:57 ` Takahiro Kuwano 1 sibling, 2 replies; 12+ messages in thread From: Sai Krishna Potthuri @ 2022-08-29 9:05 UTC (permalink / raw) To: Tudor Ambarus, Pratyush Yadav, Michael Walle, Miquel Raynal, Richard Weinberger, Vignesh Raghavendra, Rob Herring, Krzysztof Kozlowski Cc: devicetree, linux-mtd, linux-kernel, saikrishna12468, git, 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); + 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); + if (ret) + return ret; + + /* + * Experimental Minimum Chip select high to Reset delay value + * based on the flash device spec. + */ + usleep_range(1, 5); + gpiod_set_value(reset, 0); + + /* + * 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; + } + info = spi_nor_get_flash_info(nor, name); if (IS_ERR(info)) return PTR_ERR(info); -- 2.17.1 ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] mtd: spi-nor: Add support for flash reset 2022-08-29 9:05 ` [PATCH 2/2] mtd: spi-nor: Add support for flash reset Sai Krishna Potthuri @ 2022-08-29 10:04 ` Michael Walle 2022-08-30 6:32 ` Potthuri, Sai Krishna 2022-09-01 1:57 ` Takahiro Kuwano 1 sibling, 1 reply; 12+ messages in thread From: Michael Walle @ 2022-08-29 10:04 UTC (permalink / raw) To: Sai Krishna Potthuri Cc: Tudor Ambarus, Pratyush Yadav, Miquel Raynal, Richard Weinberger, Vignesh Raghavendra, Rob Herring, Krzysztof Kozlowski, devicetree, linux-mtd, linux-kernel, saikrishna12468, git 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); ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH 2/2] mtd: spi-nor: Add support for flash reset 2022-08-29 10:04 ` Michael Walle @ 2022-08-30 6:32 ` Potthuri, Sai Krishna 2022-08-30 7:09 ` Michael Walle 0 siblings, 1 reply; 12+ messages in thread From: Potthuri, Sai Krishna @ 2022-08-30 6:32 UTC (permalink / raw) To: Michael Walle Cc: Tudor Ambarus, Pratyush Yadav, Miquel Raynal, Richard Weinberger, Vignesh Raghavendra, Rob Herring, Krzysztof Kozlowski, devicetree, linux-mtd, linux-kernel, saikrishna12468, git (AMD-Xilinx) Hi Michael, > -----Original Message----- > From: Michael Walle <michael@walle.cc> > Sent: Monday, August 29, 2022 3:35 PM > To: Potthuri, Sai Krishna <sai.krishna.potthuri@amd.com> > Cc: Tudor Ambarus <tudor.ambarus@microchip.com>; Pratyush Yadav > <pratyush@kernel.org>; Miquel Raynal <miquel.raynal@bootlin.com>; > Richard Weinberger <richard@nod.at>; Vignesh Raghavendra > <vigneshr@ti.com>; Rob Herring <robh+dt@kernel.org>; Krzysztof Kozlowski > <krzysztof.kozlowski+dt@linaro.org>; devicetree@vger.kernel.org; linux- > mtd@lists.infradead.org; linux-kernel@vger.kernel.org; > saikrishna12468@gmail.com; git (AMD-Xilinx) <git@amd.com> > 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. Agree, I will fix in v2. > > > + if (ret) > > + return ret; > > + > > + /* > > + * Experimental Minimum Chip select high to Reset delay value > > + * based on the flash device spec. > > + */ > > Which flash device spec? I referred some of the qspi, ospi flash vendors datasheets like Micron, Macronix, ISSI, gigadevice, spansion. > > > + 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 Reset Sequence which I implemented here is high(1)->low(0)->high(1). By doing this sequence (active low), flash device is getting reset, this sequence is tested using Octal SPI flash device. > invert the value here. Also please use the cansleep() variant. Ok, I will use gpiod_set_value_cansleep() in v2. > > > + /* > > + * 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. Agree, we can do it unconditionally without checking for reset pin broken. If we have reset specifier in the device tree, then we can do the reset. I will update in v2. > > Also, which tree are you using? That was moved into > spi_nor_init_flags() some time ago. Please rebase to latest > spi-next. Yes, reset pin broken property moved to spi_nor_init_flags(). I moved this from spi_nor_init_flags() to spi_nor_scan() (which is part of this patch) to decide on the flash reset part. With the above agreement (reset not to depend on reset broken pin), I will Revert this change. Regards Sai Krishna > > -michael > > > + > > info = spi_nor_get_flash_info(nor, name); > > if (IS_ERR(info)) > > return PTR_ERR(info); ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] mtd: spi-nor: Add support for flash reset 2022-08-30 6:32 ` Potthuri, Sai Krishna @ 2022-08-30 7:09 ` Michael Walle 2022-08-30 10:22 ` Potthuri, Sai Krishna 0 siblings, 1 reply; 12+ messages in thread From: Michael Walle @ 2022-08-30 7:09 UTC (permalink / raw) To: Potthuri, Sai Krishna Cc: Tudor Ambarus, Pratyush Yadav, Miquel Raynal, Richard Weinberger, Vignesh Raghavendra, Rob Herring, Krzysztof Kozlowski, devicetree, linux-mtd, linux-kernel, saikrishna12468, git (AMD-Xilinx) Hi, Am 2022-08-30 08:32, schrieb Potthuri, Sai Krishna: >> > + if (ret) >> > + return ret; >> > + >> > + /* >> > + * Experimental Minimum Chip select high to Reset delay value >> > + * based on the flash device spec. >> > + */ >> >> Which flash device spec? > I referred some of the qspi, ospi flash vendors datasheets like Micron, > Macronix, ISSI, gigadevice, spansion. Please mention here that you've looked at datasheets of different vendors. And maybe instead of doing three comments, just one and then the reset sequence. >> >> > + 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 > Reset Sequence which I implemented here is high(1)->low(0)->high(1). > By doing this sequence (active low), flash device is getting reset, > this sequence > is tested using Octal SPI flash device. How does the device tree property for your look like? Has it the GPIO_ACTIVE_LOW flag set? -michael ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH 2/2] mtd: spi-nor: Add support for flash reset 2022-08-30 7:09 ` Michael Walle @ 2022-08-30 10:22 ` Potthuri, Sai Krishna 0 siblings, 0 replies; 12+ messages in thread From: Potthuri, Sai Krishna @ 2022-08-30 10:22 UTC (permalink / raw) To: Michael Walle Cc: Tudor Ambarus, Pratyush Yadav, Miquel Raynal, Richard Weinberger, Vignesh Raghavendra, Rob Herring, Krzysztof Kozlowski, devicetree, linux-mtd, linux-kernel, saikrishna12468, git (AMD-Xilinx) Hi Michael, > -----Original Message----- > From: Michael Walle <michael@walle.cc> > Sent: Tuesday, August 30, 2022 12:40 PM > To: Potthuri, Sai Krishna <sai.krishna.potthuri@amd.com> > Cc: Tudor Ambarus <tudor.ambarus@microchip.com>; Pratyush Yadav > <pratyush@kernel.org>; Miquel Raynal <miquel.raynal@bootlin.com>; > Richard Weinberger <richard@nod.at>; Vignesh Raghavendra > <vigneshr@ti.com>; Rob Herring <robh+dt@kernel.org>; Krzysztof Kozlowski > <krzysztof.kozlowski+dt@linaro.org>; devicetree@vger.kernel.org; linux- > mtd@lists.infradead.org; linux-kernel@vger.kernel.org; > saikrishna12468@gmail.com; git (AMD-Xilinx) <git@amd.com> > Subject: Re: [PATCH 2/2] mtd: spi-nor: Add support for flash reset > > Hi, > > Am 2022-08-30 08:32, schrieb Potthuri, Sai Krishna: > > >> > + if (ret) > >> > + return ret; > >> > + > >> > + /* > >> > + * Experimental Minimum Chip select high to Reset delay value > >> > + * based on the flash device spec. > >> > + */ > >> > >> Which flash device spec? > > I referred some of the qspi, ospi flash vendors datasheets like > > Micron, Macronix, ISSI, gigadevice, spansion. > > Please mention here that you've looked at datasheets of different vendors. > And maybe instead of doing three comments, just one and then the reset > sequence. Ok, I will update in v2. > > >> > >> > + 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 > > Reset Sequence which I implemented here is high(1)->low(0)->high(1). > > By doing this sequence (active low), flash device is getting reset, > > this sequence is tested using Octal SPI flash device. > > How does the device tree property for your look like? > Has it the GPIO_ACTIVE_LOW flag set? Sorry, I misunderstand your initial ask, This logic is developed by keeping the GPIO_ACTIVE_HIGH flag set in the device-tree. Agree this will violate the flash reset sequence (active low reset). I will update the sequence in v2 to match with the "active low" flag in the device tree. Regards Sai Krishna > > -michael ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] mtd: spi-nor: Add support for flash reset 2022-08-29 9:05 ` [PATCH 2/2] mtd: spi-nor: Add support for flash reset Sai Krishna Potthuri 2022-08-29 10:04 ` Michael Walle @ 2022-09-01 1:57 ` Takahiro Kuwano 2022-09-01 5:15 ` Potthuri, Sai Krishna 1 sibling, 1 reply; 12+ messages in thread From: Takahiro Kuwano @ 2022-09-01 1:57 UTC (permalink / raw) To: Sai Krishna Potthuri, Tudor Ambarus, Pratyush Yadav, Michael Walle, Miquel Raynal, Richard Weinberger, Vignesh Raghavendra, Rob Herring, Krzysztof Kozlowski Cc: devicetree, linux-mtd, linux-kernel, saikrishna12468, git, takahiro Kuwano Hello, On 8/29/2022 6:05 PM, Sai Krishna Potthuri wrote: > 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); > + 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); > + if (ret) > + return ret; > + > + /* > + * Experimental Minimum Chip select high to Reset delay value > + * based on the flash device spec. > + */ > + usleep_range(1, 5); > + gpiod_set_value(reset, 0); > + > + /* > + * 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); Infineon (Spansion/Cypress) SEMPER flash (S25HL/HS, S28HL/HS) family specifies minimum tRH (Reset Pulse Hold - RESET# Low to CS# Low) as 450~600us. Please take care for this. Please find datasheets at the following links. https://www.infineon.com/dgdl/Infineon-S25HS256T_S25HS512T_S25HS01GT_S25HL256T_S25HL512T_S25HL01GT_256-Mb_(32-MB)_512-Mb_(64-MB)_1-Gb_(128-MB)_HS-T_(1.8-V)_HL-T_(3.0-V)_Semper_Flash_with_Quad_SPI-DataSheet-v02_00-EN.pdf?fileId=8ac78c8c7d0d8da4017d0ee674b86ee3&da=t https://www.infineon.com/dgdl/Infineon-S28HS256T_S28HS512T_S28HS01GT_S28HL256T_S28HL512T_S28HL01GT_256-Mb_(32-MB)_512-Mb_(64-MB)_1-Gb_(128-MB)_HS-T_(1.8-V)_HL-T_(3.0-V)_Semper_Flash_with_Octal_Interface-DataSheet-v03_00-EN.pdf?fileId=8ac78c8c7d0d8da4017d0ee6bca96f97&da=t Thanks, Takahiro Kuwano ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH 2/2] mtd: spi-nor: Add support for flash reset 2022-09-01 1:57 ` Takahiro Kuwano @ 2022-09-01 5:15 ` Potthuri, Sai Krishna 0 siblings, 0 replies; 12+ messages in thread From: Potthuri, Sai Krishna @ 2022-09-01 5:15 UTC (permalink / raw) To: Takahiro Kuwano, Tudor Ambarus, Pratyush Yadav, Michael Walle, Miquel Raynal, Richard Weinberger, Vignesh Raghavendra, Rob Herring, Krzysztof Kozlowski Cc: devicetree, linux-mtd, linux-kernel, saikrishna12468, git (AMD-Xilinx), takahiro Kuwano Hi Takahiro Kuwano, > -----Original Message----- > From: Takahiro Kuwano <tkuw584924@gmail.com> > Sent: Thursday, September 1, 2022 7:28 AM > To: Potthuri, Sai Krishna <sai.krishna.potthuri@amd.com>; Tudor Ambarus > <tudor.ambarus@microchip.com>; Pratyush Yadav <pratyush@kernel.org>; > Michael Walle <michael@walle.cc>; Miquel Raynal > <miquel.raynal@bootlin.com>; Richard Weinberger <richard@nod.at>; > Vignesh Raghavendra <vigneshr@ti.com>; Rob Herring > <robh+dt@kernel.org>; Krzysztof Kozlowski > <krzysztof.kozlowski+dt@linaro.org> > Cc: devicetree@vger.kernel.org; linux-mtd@lists.infradead.org; linux- > kernel@vger.kernel.org; saikrishna12468@gmail.com; git (AMD-Xilinx) > <git@amd.com>; takahiro Kuwano <Takahiro.Kuwano@infineon.com> > Subject: Re: [PATCH 2/2] mtd: spi-nor: Add support for flash reset > > Hello, > > On 8/29/2022 6:05 PM, Sai Krishna Potthuri wrote: > > 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); > > + 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); > > + if (ret) > > + return ret; > > + > > + /* > > + * Experimental Minimum Chip select high to Reset delay value > > + * based on the flash device spec. > > + */ > > + usleep_range(1, 5); > > + gpiod_set_value(reset, 0); > > + > > + /* > > + * 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); > Infineon (Spansion/Cypress) SEMPER flash (S25HL/HS, S28HL/HS) family > specifies minimum tRH (Reset Pulse Hold - RESET# Low to CS# Low) as > 450~600us. Please take care for this. > > Please find datasheets at the following links. > > https://www.infineon.com/dgdl/Infineon- > S25HS256T_S25HS512T_S25HS01GT_S25HL256T_S25HL512T_S25HL01GT_256 > -Mb_(32-MB)_512-Mb_(64-MB)_1-Gb_(128-MB)_HS-T_(1.8-V)_HL-T_(3.0- > V)_Semper_Flash_with_Quad_SPI-DataSheet-v02_00- > EN.pdf?fileId=8ac78c8c7d0d8da4017d0ee674b86ee3&da=t > > https://www.infineon.com/dgdl/Infineon- > S28HS256T_S28HS512T_S28HS01GT_S28HL256T_S28HL512T_S28HL01GT_256 > -Mb_(32-MB)_512-Mb_(64-MB)_1-Gb_(128-MB)_HS-T_(1.8-V)_HL-T_(3.0- > V)_Semper_Flash_with_Octal_Interface-DataSheet-v03_00- > EN.pdf?fileId=8ac78c8c7d0d8da4017d0ee6bca96f97&da=t Thanks for sharing this information. I will take care of this in v2. Regards Sai Krishna ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2022-09-01 5:16 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-08-29 9:05 [PATCH 0/2] mtd: spi-nor: Add flash device reset support Sai Krishna Potthuri 2022-08-29 9:05 ` [PATCH 1/2] dt-bindings: mtd: spi-nor: Add reset-gpios property Sai Krishna Potthuri 2022-08-30 9:21 ` Krzysztof Kozlowski 2022-08-30 9:36 ` Michael Walle 2022-08-30 9:48 ` Krzysztof Kozlowski 2022-08-29 9:05 ` [PATCH 2/2] mtd: spi-nor: Add support for flash reset Sai Krishna Potthuri 2022-08-29 10:04 ` Michael Walle 2022-08-30 6:32 ` Potthuri, Sai Krishna 2022-08-30 7:09 ` Michael Walle 2022-08-30 10:22 ` Potthuri, Sai Krishna 2022-09-01 1:57 ` Takahiro Kuwano 2022-09-01 5:15 ` Potthuri, Sai Krishna
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).