[PATCH RFC 0/4] Add set_iofv() callback

[PATCH RFC 0/4] Add set_iofv() callback

[Biju Das]

As per section 8.14 on the AT25QL128A hardware manual[1],
IO0..IO3 must be set to Hi-Z state for this flash for fast read quad IO.
Snippet from HW manual section 8.14:
The upper nibble of the Mode(M7-4) controls the length of the next FAST
Read Quad IO instruction through the inclusion or exclusion of the first
byte instruction code. The lower nibble bits of the Mode(M3-0) are don't
care. However, the IO pins must be high-impedance before the falling edge
of the first data out clock.

Add set_iofv() callback for configuring IO fixed values to control the
pin state.

[1]
https://www.renesas.com/eu/en/document/dst/at25ql128a-datasheet?r=1608586

Ref:
 https://patchwork.kernel.org/project/linux-renesas-soc/patch/20230830145835.296690-1-biju.das.jz@bp.renesas.com/

Biju Das (4):
  spi: spi-mem: Add set_iofv() callback
  mtd: spi-nor: Add post_sfdp() callback
  memory: renesas-rpc-if: Add support for overriding IO fixed values
  spi: rpc-if: Add set_iofv() callback

 drivers/memory/renesas-rpc-if.c | 20 ++++++++++++++++++++
 drivers/mtd/spi-nor/core.c      | 20 ++++++++++++++++++++
 drivers/spi/spi-mem.c           | 20 ++++++++++++++++++++
 drivers/spi/spi-rpc-if.c        |  9 +++++++++
 include/linux/spi/spi-mem.h     |  4 ++++
 include/memory/renesas-rpc-if.h |  1 +
 6 files changed, 74 insertions(+)

-- 
2.25.1

[PATCH RFC 0/4] Add set_iofv() callback

[Biju Das]

As per section 8.14 on the AT25QL128A hardware manual[1],
IO0..IO3 must be set to Hi-Z state for this flash for fast read quad IO.
Snippet from HW manual section 8.14:
The upper nibble of the Mode(M7-4) controls the length of the next FAST
Read Quad IO instruction through the inclusion or exclusion of the first
byte instruction code. The lower nibble bits of the Mode(M3-0) are don't
care. However, the IO pins must be high-impedance before the falling edge
of the first data out clock.

Add set_iofv() callback for configuring IO fixed values to control the
pin state.

[1]
https://www.renesas.com/eu/en/document/dst/at25ql128a-datasheet?r=1608586

Ref:
 https://patchwork.kernel.org/project/linux-renesas-soc/patch/20230830145835.296690-1-biju.das.jz@bp.renesas.com/

Biju Das (4):
  spi: spi-mem: Add set_iofv() callback
  mtd: spi-nor: Add post_sfdp() callback
  memory: renesas-rpc-if: Add support for overriding IO fixed values
  spi: rpc-if: Add set_iofv() callback

 drivers/memory/renesas-rpc-if.c | 20 ++++++++++++++++++++
 drivers/mtd/spi-nor/core.c      | 20 ++++++++++++++++++++
 drivers/spi/spi-mem.c           | 20 ++++++++++++++++++++
 drivers/spi/spi-rpc-if.c        |  9 +++++++++
 include/linux/spi/spi-mem.h     |  4 ++++
 include/memory/renesas-rpc-if.h |  1 +
 6 files changed, 74 insertions(+)

-- 
2.25.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH RFC 1/4] spi: spi-mem: Add set_iofv() callback

[Biju Das]

As per section 8.14 on the AT25QL128A hardware manual,
IO0..IO3 must be set to Hi-Z state for this flash for fast read quad IO.
Snippet from HW manual section 8.14:
The upper nibble of the Mode(M7-4) controls the length of the next FAST
Read Quad IO instruction through the inclusion or exclusion of the first
byte instruction code. The lower nibble bits of the Mode(M3-0) are don't
care. However, the IO pins must be high-impedance before the falling edge
of the first data out clock.

Add set_iofv() callback for configuring IO fixed values to control the
pin state.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 drivers/spi/spi-mem.c       | 20 ++++++++++++++++++++
 include/linux/spi/spi-mem.h |  4 ++++
 2 files changed, 24 insertions(+)

diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c
index edd7430d4c05..0cfca8c438e3 100644
--- a/drivers/spi/spi-mem.c
+++ b/drivers/spi/spi-mem.c
@@ -297,6 +297,26 @@ static void spi_mem_access_end(struct spi_mem *mem)
 		pm_runtime_put(ctlr->dev.parent);
 }
 
+/**
+ * spi_mem_set_iofv() - Set IO fixed values to control the pin state
+ * @mem: the SPI memory
+ * @val: the IO fixed values
+ *
+ * Set IO fixed values to control the pin state.
+ *
+ * Return: 0 in case of success, a negative error code otherwise.
+ */
+int spi_mem_set_iofv(struct spi_mem *mem, u32 val)
+{
+	struct spi_controller *ctlr = mem->spi->controller;
+
+	if (ctlr->mem_ops && ctlr->mem_ops->set_iofv)
+		return ctlr->mem_ops->set_iofv(mem, val);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(spi_mem_set_iofv);
+
 /**
  * spi_mem_exec_op() - Execute a memory operation
  * @mem: the SPI memory
diff --git a/include/linux/spi/spi-mem.h b/include/linux/spi/spi-mem.h
index 6b0a7dc48a4b..e50f89bf5ba9 100644
--- a/include/linux/spi/spi-mem.h
+++ b/include/linux/spi/spi-mem.h
@@ -232,6 +232,7 @@ static inline void *spi_mem_get_drvdata(struct spi_mem *mem)
  *		    limitations (can be alignment or max RX/TX size
  *		    limitations)
  * @supports_op: check if an operation is supported by the controller
+ * @set_iofv: set IO fixed values to control the pin state
  * @exec_op: execute a SPI memory operation
  * @get_name: get a custom name for the SPI mem device from the controller.
  *	      This might be needed if the controller driver has been ported
@@ -274,6 +275,7 @@ struct spi_controller_mem_ops {
 	int (*adjust_op_size)(struct spi_mem *mem, struct spi_mem_op *op);
 	bool (*supports_op)(struct spi_mem *mem,
 			    const struct spi_mem_op *op);
+	int (*set_iofv)(struct spi_mem *mem, u32 val);
 	int (*exec_op)(struct spi_mem *mem,
 		       const struct spi_mem_op *op);
 	const char *(*get_name)(struct spi_mem *mem);
@@ -367,6 +369,8 @@ int spi_mem_adjust_op_size(struct spi_mem *mem, struct spi_mem_op *op);
 bool spi_mem_supports_op(struct spi_mem *mem,
 			 const struct spi_mem_op *op);
 
+int spi_mem_set_iofv(struct spi_mem *mem, u32 val);
+
 int spi_mem_exec_op(struct spi_mem *mem,
 		    const struct spi_mem_op *op);
 
-- 
2.25.1

[PATCH RFC 1/4] spi: spi-mem: Add set_iofv() callback

[Biju Das]

As per section 8.14 on the AT25QL128A hardware manual,
IO0..IO3 must be set to Hi-Z state for this flash for fast read quad IO.
Snippet from HW manual section 8.14:
The upper nibble of the Mode(M7-4) controls the length of the next FAST
Read Quad IO instruction through the inclusion or exclusion of the first
byte instruction code. The lower nibble bits of the Mode(M3-0) are don't
care. However, the IO pins must be high-impedance before the falling edge
of the first data out clock.

Add set_iofv() callback for configuring IO fixed values to control the
pin state.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 drivers/spi/spi-mem.c       | 20 ++++++++++++++++++++
 include/linux/spi/spi-mem.h |  4 ++++
 2 files changed, 24 insertions(+)

diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c
index edd7430d4c05..0cfca8c438e3 100644
--- a/drivers/spi/spi-mem.c
+++ b/drivers/spi/spi-mem.c
@@ -297,6 +297,26 @@ static void spi_mem_access_end(struct spi_mem *mem)
 		pm_runtime_put(ctlr->dev.parent);
 }
 
+/**
+ * spi_mem_set_iofv() - Set IO fixed values to control the pin state
+ * @mem: the SPI memory
+ * @val: the IO fixed values
+ *
+ * Set IO fixed values to control the pin state.
+ *
+ * Return: 0 in case of success, a negative error code otherwise.
+ */
+int spi_mem_set_iofv(struct spi_mem *mem, u32 val)
+{
+	struct spi_controller *ctlr = mem->spi->controller;
+
+	if (ctlr->mem_ops && ctlr->mem_ops->set_iofv)
+		return ctlr->mem_ops->set_iofv(mem, val);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(spi_mem_set_iofv);
+
 /**
  * spi_mem_exec_op() - Execute a memory operation
  * @mem: the SPI memory
diff --git a/include/linux/spi/spi-mem.h b/include/linux/spi/spi-mem.h
index 6b0a7dc48a4b..e50f89bf5ba9 100644
--- a/include/linux/spi/spi-mem.h
+++ b/include/linux/spi/spi-mem.h
@@ -232,6 +232,7 @@ static inline void *spi_mem_get_drvdata(struct spi_mem *mem)
  *		    limitations (can be alignment or max RX/TX size
  *		    limitations)
  * @supports_op: check if an operation is supported by the controller
+ * @set_iofv: set IO fixed values to control the pin state
  * @exec_op: execute a SPI memory operation
  * @get_name: get a custom name for the SPI mem device from the controller.
  *	      This might be needed if the controller driver has been ported
@@ -274,6 +275,7 @@ struct spi_controller_mem_ops {
 	int (*adjust_op_size)(struct spi_mem *mem, struct spi_mem_op *op);
 	bool (*supports_op)(struct spi_mem *mem,
 			    const struct spi_mem_op *op);
+	int (*set_iofv)(struct spi_mem *mem, u32 val);
 	int (*exec_op)(struct spi_mem *mem,
 		       const struct spi_mem_op *op);
 	const char *(*get_name)(struct spi_mem *mem);
@@ -367,6 +369,8 @@ int spi_mem_adjust_op_size(struct spi_mem *mem, struct spi_mem_op *op);
 bool spi_mem_supports_op(struct spi_mem *mem,
 			 const struct spi_mem_op *op);
 
+int spi_mem_set_iofv(struct spi_mem *mem, u32 val);
+
 int spi_mem_exec_op(struct spi_mem *mem,
 		    const struct spi_mem_op *op);
 
-- 
2.25.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH RFC 2/4] mtd: spi-nor: Add post_sfdp() callback

[Biju Das]

Some generic flash devices such as Renesas AT25QL128A serial nor
flash requires the IO pins must be in high-impedance before the
falling edge of the first data out clock for the Fast Read Quad
IO.

Add post_sfdp() callback for generic flash devices and then call
spi_mem_set_iofv() to configure IO fixed value to control the pin
state.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 drivers/mtd/spi-nor/core.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 1c443fe568cf..eef3c286b174 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -1997,6 +1997,21 @@ int spi_nor_sr2_bit7_quad_enable(struct spi_nor *nor)
 	return 0;
 }
 
+static int spi_nor_generic_post_sfdp_fixup(struct spi_nor *nor)
+{
+	struct spi_device *spi = nor->spimem->spi;
+	const u8 *id = nor->id;
+
+	if (spi->mode & SPI_TX_QUAD && nor->spimem) {
+		const struct spi_nor_id *at25ql128a_id = SNOR_ID(0x1f, 0x42, 0x18);
+
+		if (!memcmp(id, at25ql128a_id->bytes, at25ql128a_id->len))
+			return spi_mem_set_iofv(nor->spimem, 0xff);
+	}
+
+	return 0;
+}
+
 static const struct spi_nor_manufacturer *manufacturers[] = {
 	&spi_nor_atmel,
 	&spi_nor_eon,
@@ -2015,8 +2030,13 @@ static const struct spi_nor_manufacturer *manufacturers[] = {
 	&spi_nor_xmc,
 };
 
+static const struct spi_nor_fixups spi_nor_generic_fixups = {
+	.post_sfdp = spi_nor_generic_post_sfdp_fixup,
+};
+
 static const struct flash_info spi_nor_generic_flash = {
 	.name = "spi-nor-generic",
+	.fixups = &spi_nor_generic_fixups,
 };
 
 static const struct flash_info *spi_nor_match_id(struct spi_nor *nor,
-- 
2.25.1

[PATCH RFC 2/4] mtd: spi-nor: Add post_sfdp() callback

[Biju Das]

Some generic flash devices such as Renesas AT25QL128A serial nor
flash requires the IO pins must be in high-impedance before the
falling edge of the first data out clock for the Fast Read Quad
IO.

Add post_sfdp() callback for generic flash devices and then call
spi_mem_set_iofv() to configure IO fixed value to control the pin
state.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 drivers/mtd/spi-nor/core.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 1c443fe568cf..eef3c286b174 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -1997,6 +1997,21 @@ int spi_nor_sr2_bit7_quad_enable(struct spi_nor *nor)
 	return 0;
 }
 
+static int spi_nor_generic_post_sfdp_fixup(struct spi_nor *nor)
+{
+	struct spi_device *spi = nor->spimem->spi;
+	const u8 *id = nor->id;
+
+	if (spi->mode & SPI_TX_QUAD && nor->spimem) {
+		const struct spi_nor_id *at25ql128a_id = SNOR_ID(0x1f, 0x42, 0x18);
+
+		if (!memcmp(id, at25ql128a_id->bytes, at25ql128a_id->len))
+			return spi_mem_set_iofv(nor->spimem, 0xff);
+	}
+
+	return 0;
+}
+
 static const struct spi_nor_manufacturer *manufacturers[] = {
 	&spi_nor_atmel,
 	&spi_nor_eon,
@@ -2015,8 +2030,13 @@ static const struct spi_nor_manufacturer *manufacturers[] = {
 	&spi_nor_xmc,
 };
 
+static const struct spi_nor_fixups spi_nor_generic_fixups = {
+	.post_sfdp = spi_nor_generic_post_sfdp_fixup,
+};
+
 static const struct flash_info spi_nor_generic_flash = {
 	.name = "spi-nor-generic",
+	.fixups = &spi_nor_generic_fixups,
 };
 
 static const struct flash_info *spi_nor_match_id(struct spi_nor *nor,
-- 
2.25.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH RFC 3/4] memory: renesas-rpc-if: Add support for overriding IO fixed values

[Biju Das]

Add support for overriding IO fixed values to control the pin state
based on the flash type.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 drivers/memory/renesas-rpc-if.c | 20 ++++++++++++++++++++
 include/memory/renesas-rpc-if.h |  1 +
 2 files changed, 21 insertions(+)

diff --git a/drivers/memory/renesas-rpc-if.c b/drivers/memory/renesas-rpc-if.c
index 9695b2d3ae59..17bc604cdfff 100644
--- a/drivers/memory/renesas-rpc-if.c
+++ b/drivers/memory/renesas-rpc-if.c
@@ -325,6 +325,26 @@ static void rpcif_rzg2l_timing_adjust_sdr(struct rpcif_priv *rpc)
 	regmap_write(rpc->regmap, RPCIF_PHYADD, 0x80000032);
 }
 
+int rpcif_set_iofv(struct device *dev, u32 val)
+{
+	struct rpcif_priv *rpc = dev_get_drvdata(dev);
+	int ret;
+
+	ret = pm_runtime_resume_and_get(dev);
+	if (ret)
+		return ret;
+
+	regmap_update_bits(rpc->regmap, RPCIF_CMNCR, RPCIF_CMNCR_IOFV(3),
+			   RPCIF_CMNCR_IO0FV(val & 0x3) |
+			   RPCIF_CMNCR_IO2FV((val >> 4) & 0x3) |
+			   RPCIF_CMNCR_IO3FV((val >> 6) & 0x3));
+
+	pm_runtime_put(dev);
+
+	return 0;
+}
+EXPORT_SYMBOL(rpcif_set_iofv);
+
 int rpcif_hw_init(struct device *dev, bool hyperflash)
 {
 	struct rpcif_priv *rpc = dev_get_drvdata(dev);
diff --git a/include/memory/renesas-rpc-if.h b/include/memory/renesas-rpc-if.h
index b8fa30fd6b50..124ca9c16a39 100644
--- a/include/memory/renesas-rpc-if.h
+++ b/include/memory/renesas-rpc-if.h
@@ -71,6 +71,7 @@ struct rpcif {
 
 int rpcif_sw_init(struct rpcif *rpc, struct device *dev);
 int rpcif_hw_init(struct device *dev, bool hyperflash);
+int rpcif_set_iofv(struct device *dev, u32 val);
 void rpcif_prepare(struct device *dev, const struct rpcif_op *op, u64 *offs,
 		   size_t *len);
 int rpcif_manual_xfer(struct device *dev);
-- 
2.25.1

[PATCH RFC 4/4] spi: rpc-if: Add set_iofv() callback

[Biju Das]

Add set_iofv() callback for configuring IO fixed values to control
the pin state.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 drivers/spi/spi-rpc-if.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/spi/spi-rpc-if.c b/drivers/spi/spi-rpc-if.c
index e11146932828..8b5c41de99af 100644
--- a/drivers/spi/spi-rpc-if.c
+++ b/drivers/spi/spi-rpc-if.c
@@ -75,6 +75,14 @@ static bool rpcif_spi_mem_supports_op(struct spi_mem *mem,
 	return true;
 }
 
+static int rpcif_spi_mem_set_iofv(struct spi_mem *mem, const u32 val)
+{
+	struct rpcif *rpc =
+		spi_controller_get_devdata(mem->spi->controller);
+
+	return rpcif_set_iofv(rpc->dev, val);
+}
+
 static ssize_t rpcif_spi_mem_dirmap_read(struct spi_mem_dirmap_desc *desc,
 					 u64 offs, size_t len, void *buf)
 {
@@ -121,6 +129,7 @@ static int rpcif_spi_mem_exec_op(struct spi_mem *mem,
 }
 
 static const struct spi_controller_mem_ops rpcif_spi_mem_ops = {
+	.set_iofv	= rpcif_spi_mem_set_iofv,
 	.supports_op	= rpcif_spi_mem_supports_op,
 	.exec_op	= rpcif_spi_mem_exec_op,
 	.dirmap_create	= rpcif_spi_mem_dirmap_create,
-- 
2.25.1

Re: [PATCH RFC 1/4] spi: spi-mem: Add set_iofv() callback

[Geert Uytterhoeven]

Hi Biju,

On Wed, Nov 8, 2023 at 6:12 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> As per section 8.14 on the AT25QL128A hardware manual,
> IO0..IO3 must be set to Hi-Z state for this flash for fast read quad IO.
> Snippet from HW manual section 8.14:
> The upper nibble of the Mode(M7-4) controls the length of the next FAST
> Read Quad IO instruction through the inclusion or exclusion of the first
> byte instruction code. The lower nibble bits of the Mode(M3-0) are don't
> care. However, the IO pins must be high-impedance before the falling edge
> of the first data out clock.
>
> Add set_iofv() callback for configuring IO fixed values to control the
> pin state.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

Thanks for your patch!

> --- a/drivers/spi/spi-mem.c
> +++ b/drivers/spi/spi-mem.c
> @@ -297,6 +297,26 @@ static void spi_mem_access_end(struct spi_mem *mem)
>                 pm_runtime_put(ctlr->dev.parent);
>  }
>
> +/**
> + * spi_mem_set_iofv() - Set IO fixed values to control the pin state
> + * @mem: the SPI memory
> + * @val: the IO fixed values

Please document the meaning of this value (i.e. what does a
set or cleared bit mean?).

> + *
> + * Set IO fixed values to control the pin state.
> + *
> + * Return: 0 in case of success, a negative error code otherwise.
> + */
> +int spi_mem_set_iofv(struct spi_mem *mem, u32 val)
> +{
> +       struct spi_controller *ctlr = mem->spi->controller;
> +
> +       if (ctlr->mem_ops && ctlr->mem_ops->set_iofv)
> +               return ctlr->mem_ops->set_iofv(mem, val);
> +
> +       return 0;
> +}
> +EXPORT_SYMBOL_GPL(spi_mem_set_iofv);
> +
>  /**
>   * spi_mem_exec_op() - Execute a memory operation
>   * @mem: the SPI memory

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH RFC 1/4] spi: spi-mem: Add set_iofv() callback

[Geert Uytterhoeven]

Hi Biju,

On Wed, Nov 8, 2023 at 6:12 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> As per section 8.14 on the AT25QL128A hardware manual,
> IO0..IO3 must be set to Hi-Z state for this flash for fast read quad IO.
> Snippet from HW manual section 8.14:
> The upper nibble of the Mode(M7-4) controls the length of the next FAST
> Read Quad IO instruction through the inclusion or exclusion of the first
> byte instruction code. The lower nibble bits of the Mode(M3-0) are don't
> care. However, the IO pins must be high-impedance before the falling edge
> of the first data out clock.
>
> Add set_iofv() callback for configuring IO fixed values to control the
> pin state.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

Thanks for your patch!

> --- a/drivers/spi/spi-mem.c
> +++ b/drivers/spi/spi-mem.c
> @@ -297,6 +297,26 @@ static void spi_mem_access_end(struct spi_mem *mem)
>                 pm_runtime_put(ctlr->dev.parent);
>  }
>
> +/**
> + * spi_mem_set_iofv() - Set IO fixed values to control the pin state
> + * @mem: the SPI memory
> + * @val: the IO fixed values

Please document the meaning of this value (i.e. what does a
set or cleared bit mean?).

> + *
> + * Set IO fixed values to control the pin state.
> + *
> + * Return: 0 in case of success, a negative error code otherwise.
> + */
> +int spi_mem_set_iofv(struct spi_mem *mem, u32 val)
> +{
> +       struct spi_controller *ctlr = mem->spi->controller;
> +
> +       if (ctlr->mem_ops && ctlr->mem_ops->set_iofv)
> +               return ctlr->mem_ops->set_iofv(mem, val);
> +
> +       return 0;
> +}
> +EXPORT_SYMBOL_GPL(spi_mem_set_iofv);
> +
>  /**
>   * spi_mem_exec_op() - Execute a memory operation
>   * @mem: the SPI memory

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

RE: [PATCH RFC 1/4] spi: spi-mem: Add set_iofv() callback

[Biju Das]

Hi Geert,

Thanks for the feedback.

> Subject: Re: [PATCH RFC 1/4] spi: spi-mem: Add set_iofv() callback
> 
> Hi Biju,
> 
> On Wed, Nov 8, 2023 at 6:12 PM Biju Das <biju.das.jz@bp.renesas.com>
> wrote:
> > As per section 8.14 on the AT25QL128A hardware manual,
> > IO0..IO3 must be set to Hi-Z state for this flash for fast read quad IO.
> > Snippet from HW manual section 8.14:
> > The upper nibble of the Mode(M7-4) controls the length of the next
> > FAST Read Quad IO instruction through the inclusion or exclusion of
> > the first byte instruction code. The lower nibble bits of the
> > Mode(M3-0) are don't care. However, the IO pins must be high-impedance
> > before the falling edge of the first data out clock.
> >
> > Add set_iofv() callback for configuring IO fixed values to control the
> > pin state.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> 
> Thanks for your patch!
> 
> > --- a/drivers/spi/spi-mem.c
> > +++ b/drivers/spi/spi-mem.c
> > @@ -297,6 +297,26 @@ static void spi_mem_access_end(struct spi_mem *mem)
> >                 pm_runtime_put(ctlr->dev.parent);  }
> >
> > +/**
> > + * spi_mem_set_iofv() - Set IO fixed values to control the pin state
> > + * @mem: the SPI memory
> > + * @val: the IO fixed values
> 
> Please document the meaning of this value (i.e. what does a set or cleared
> bit mean?).

I will document the value as below, 2 bits for setting IO fixed value for a pin and
their values are

00: The output value is 0.
01: The output value is 1.
10: The output value is that of the last bit in the previous transfer (the pin is placed
in the Hi-Z state if that was the case in the previous transfer).
11: The pin is placed in the Hi-Z state.

Cheers,
Biju

> 
> > + *
> > + * Set IO fixed values to control the pin state.
> > + *
> > + * Return: 0 in case of success, a negative error code otherwise.
> > + */
> > +int spi_mem_set_iofv(struct spi_mem *mem, u32 val) {
> > +       struct spi_controller *ctlr = mem->spi->controller;
> > +
> > +       if (ctlr->mem_ops && ctlr->mem_ops->set_iofv)
> > +               return ctlr->mem_ops->set_iofv(mem, val);
> > +
> > +       return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(spi_mem_set_iofv);
> > +
> >  /**
> >   * spi_mem_exec_op() - Execute a memory operation
> >   * @mem: the SPI memory
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-
> m68k.org
> 
> In personal conversations with technical people, I call myself a hacker.
> But when I'm talking to journalists I just say "programmer" or something
> like that.
>                                 -- Linus Torvalds
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH RFC 0/4] Add set_iofv() callback

[Michael Walle]

Hi Biju,

> As per section 8.14 on the AT25QL128A hardware manual[1],
> IO0..IO3 must be set to Hi-Z state for this flash for fast read quad 
> IO.
> Snippet from HW manual section 8.14:
> The upper nibble of the Mode(M7-4) controls the length of the next FAST
> Read Quad IO instruction through the inclusion or exclusion of the 
> first
> byte instruction code. The lower nibble bits of the Mode(M3-0) are 
> don't
> care. However, the IO pins must be high-impedance before the falling 
> edge
> of the first data out clock.

I'm still not sure what you are trying to fix here. For any quad I/O 
mode,
the pins of the controller must be in hiZ during the data phase on a 
read
operation. Otherwise the flash couldn't send any data, there would
be two drivers for one signal. So being in hiZ state should be the 
default
and shouldn't depend on any connected flash.

You've mentioned the micron flash which needs a '1' on its hold/reset
pin. I would have expected a fixup for this flash, not for the flash 
which
behaves normal.

-michael

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH RFC 0/4] Add set_iofv() callback

[Michael Walle]

Hi Biju,

> As per section 8.14 on the AT25QL128A hardware manual[1],
> IO0..IO3 must be set to Hi-Z state for this flash for fast read quad 
> IO.
> Snippet from HW manual section 8.14:
> The upper nibble of the Mode(M7-4) controls the length of the next FAST
> Read Quad IO instruction through the inclusion or exclusion of the 
> first
> byte instruction code. The lower nibble bits of the Mode(M3-0) are 
> don't
> care. However, the IO pins must be high-impedance before the falling 
> edge
> of the first data out clock.

I'm still not sure what you are trying to fix here. For any quad I/O 
mode,
the pins of the controller must be in hiZ during the data phase on a 
read
operation. Otherwise the flash couldn't send any data, there would
be two drivers for one signal. So being in hiZ state should be the 
default
and shouldn't depend on any connected flash.

You've mentioned the micron flash which needs a '1' on its hold/reset
pin. I would have expected a fixup for this flash, not for the flash 
which
behaves normal.

-michael

RE: [PATCH RFC 0/4] Add set_iofv() callback

[Biju Das]

Hi Michael Walle,

Thanks for the feedback.

> Subject: Re: [PATCH RFC 0/4] Add set_iofv() callback
> 
> Hi Biju,
> 
> > As per section 8.14 on the AT25QL128A hardware manual[1],
> > IO0..IO3 must be set to Hi-Z state for this flash for fast read quad
> > IO.
> > Snippet from HW manual section 8.14:
> > The upper nibble of the Mode(M7-4) controls the length of the next
> > FAST Read Quad IO instruction through the inclusion or exclusion of
> > the first byte instruction code. The lower nibble bits of the
> > Mode(M3-0) are don't care. However, the IO pins must be high-impedance
> > before the falling edge of the first data out clock.
> 
> I'm still not sure what you are trying to fix here. For any quad I/O mode,
> the pins of the controller must be in hiZ during the data phase on a read
> operation. Otherwise the flash couldn't send any data, there would be two
> drivers for one signal. So being in hiZ state should be the default and
> shouldn't depend on any connected flash.

OK, I will make hiZ state as the default.

> 
> You've mentioned the micron flash which needs a '1' on its hold/reset pin.
> I would have expected a fixup for this flash, not for the flash which
> behaves normal.

I will drop fixup for Renesas AT25QL128A  and will add fixup for micron flash.


Currently,

With iofv settings {3,3,3,3} (all pins on Hi-Z state) with Micron flash
-----------------------------------------------------------------------

./rpcif_t_001.sh
[   37.950986] spi-nor spi1.0: unrecognized JEDEC id bytes: ff ff ff ff ff ff

EXIT|FAIL|rpcif_t_001.sh|[00:00:01] Failed to detect mt25qu512a flash!||


With iofv settings {3,3,3,1} with Micron falsh
---------------------------------------------
root@smarc-rzg2l:/cip-test-scripts# ./rpcif_t_001.sh
[   26.500035] spi-nor spi1.0: mt25qu512a (65536 Kbytes)
[   26.533995] 2 fixed-partitions partitions found on MTD device spi1.0
[   26.540410] Creating 2 MTD partitions on "spi1.0":
[   26.545239] 0x000000000000-0x000002000000 : "boot"
[   26.554381] 0x000002000000-0x000004000000 : "user"

EXIT|PASS|rpcif_t_001.sh|[00:03:01] ||

Cheers,
Biju

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH RFC 0/4] Add set_iofv() callback

[Michael Walle]

Hi Biju,

>> > As per section 8.14 on the AT25QL128A hardware manual[1],
>> > IO0..IO3 must be set to Hi-Z state for this flash for fast read quad
>> > IO.
>> > Snippet from HW manual section 8.14:
>> > The upper nibble of the Mode(M7-4) controls the length of the next
>> > FAST Read Quad IO instruction through the inclusion or exclusion of
>> > the first byte instruction code. The lower nibble bits of the
>> > Mode(M3-0) are don't care. However, the IO pins must be high-impedance
>> > before the falling edge of the first data out clock.
>> 
>> I'm still not sure what you are trying to fix here. For any quad I/O 
>> mode,
>> the pins of the controller must be in hiZ during the data phase on a 
>> read
>> operation. Otherwise the flash couldn't send any data, there would be 
>> two
>> drivers for one signal. So being in hiZ state should be the default 
>> and
>> shouldn't depend on any connected flash.
> 
> OK, I will make hiZ state as the default.

I still think this iofv setting is the wrong approach, though. Do you
have a link to the spi controller datasheet where I can look up what
the controller is doing.

This seem to be a general problem with what we are sending during the
command phase and I'm curious why there wasn't more reports on non
working micron flashes for now.

>> You've mentioned the micron flash which needs a '1' on its hold/reset 
>> pin.
>> I would have expected a fixup for this flash, not for the flash which
>> behaves normal.
> 
> I will drop fixup for Renesas AT25QL128A  and will add fixup for micron 
> flash.

btw, what will happen if you always use the {3,3,3,1} setting? I guess
the atmel flash will also work? because HiZ should mean "don't care" 
from
the point of view of the flash.

> 
> With iofv settings {3,3,3,3} (all pins on Hi-Z state) with Micron flash
> -----------------------------------------------------------------------
> 
> ./rpcif_t_001.sh
> [   37.950986] spi-nor spi1.0: unrecognized JEDEC id bytes: ff ff ff ff 
> ff ff

As mentioned earlier, I suspect that HiZ on IO3 means low and the flash
will be in reset. Could you perhaps verify that by probing IO3?
I know that other flashes will *either* support RESET#/HOLD# or quad 
mode.
Thus I was saying, that we probably wont support that and the easiest
fix should be to disable this behavior for the atmel flash (there was
nv setting).

I guess, the correct fix would be to somehow add support to control
IO1-IO3 during the (single bit) command phase.

-michael

> 
> EXIT|FAIL|rpcif_t_001.sh|[00:00:01] Failed to detect mt25qu512a 
> flash!||
> 
> 
> With iofv settings {3,3,3,1} with Micron falsh
> ---------------------------------------------
> root@smarc-rzg2l:/cip-test-scripts# ./rpcif_t_001.sh
> [   26.500035] spi-nor spi1.0: mt25qu512a (65536 Kbytes)
> [   26.533995] 2 fixed-partitions partitions found on MTD device spi1.0
> [   26.540410] Creating 2 MTD partitions on "spi1.0":
> [   26.545239] 0x000000000000-0x000002000000 : "boot"
> [   26.554381] 0x000002000000-0x000004000000 : "user"
> 
> EXIT|PASS|rpcif_t_001.sh|[00:03:01] ||
> 
> Cheers,
> Biju

Re: [PATCH RFC 0/4] Add set_iofv() callback

[Michael Walle]

Hi Biju,

>> > As per section 8.14 on the AT25QL128A hardware manual[1],
>> > IO0..IO3 must be set to Hi-Z state for this flash for fast read quad
>> > IO.
>> > Snippet from HW manual section 8.14:
>> > The upper nibble of the Mode(M7-4) controls the length of the next
>> > FAST Read Quad IO instruction through the inclusion or exclusion of
>> > the first byte instruction code. The lower nibble bits of the
>> > Mode(M3-0) are don't care. However, the IO pins must be high-impedance
>> > before the falling edge of the first data out clock.
>> 
>> I'm still not sure what you are trying to fix here. For any quad I/O 
>> mode,
>> the pins of the controller must be in hiZ during the data phase on a 
>> read
>> operation. Otherwise the flash couldn't send any data, there would be 
>> two
>> drivers for one signal. So being in hiZ state should be the default 
>> and
>> shouldn't depend on any connected flash.
> 
> OK, I will make hiZ state as the default.

I still think this iofv setting is the wrong approach, though. Do you
have a link to the spi controller datasheet where I can look up what
the controller is doing.

This seem to be a general problem with what we are sending during the
command phase and I'm curious why there wasn't more reports on non
working micron flashes for now.

>> You've mentioned the micron flash which needs a '1' on its hold/reset 
>> pin.
>> I would have expected a fixup for this flash, not for the flash which
>> behaves normal.
> 
> I will drop fixup for Renesas AT25QL128A  and will add fixup for micron 
> flash.

btw, what will happen if you always use the {3,3,3,1} setting? I guess
the atmel flash will also work? because HiZ should mean "don't care" 
from
the point of view of the flash.

> 
> With iofv settings {3,3,3,3} (all pins on Hi-Z state) with Micron flash
> -----------------------------------------------------------------------
> 
> ./rpcif_t_001.sh
> [   37.950986] spi-nor spi1.0: unrecognized JEDEC id bytes: ff ff ff ff 
> ff ff

As mentioned earlier, I suspect that HiZ on IO3 means low and the flash
will be in reset. Could you perhaps verify that by probing IO3?
I know that other flashes will *either* support RESET#/HOLD# or quad 
mode.
Thus I was saying, that we probably wont support that and the easiest
fix should be to disable this behavior for the atmel flash (there was
nv setting).

I guess, the correct fix would be to somehow add support to control
IO1-IO3 during the (single bit) command phase.

-michael

> 
> EXIT|FAIL|rpcif_t_001.sh|[00:00:01] Failed to detect mt25qu512a 
> flash!||
> 
> 
> With iofv settings {3,3,3,1} with Micron falsh
> ---------------------------------------------
> root@smarc-rzg2l:/cip-test-scripts# ./rpcif_t_001.sh
> [   26.500035] spi-nor spi1.0: mt25qu512a (65536 Kbytes)
> [   26.533995] 2 fixed-partitions partitions found on MTD device spi1.0
> [   26.540410] Creating 2 MTD partitions on "spi1.0":
> [   26.545239] 0x000000000000-0x000002000000 : "boot"
> [   26.554381] 0x000002000000-0x000004000000 : "user"
> 
> EXIT|PASS|rpcif_t_001.sh|[00:03:01] ||
> 
> Cheers,
> Biju

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

RE: [PATCH RFC 0/4] Add set_iofv() callback

[Biju Das]

Hi Michael Walle,

> Subject: Re: [PATCH RFC 0/4] Add set_iofv() callback
> 
> Hi Biju,
> 
> >> > As per section 8.14 on the AT25QL128A hardware manual[1],
> >> > IO0..IO3 must be set to Hi-Z state for this flash for fast read
> >> > quad IO.
> >> > Snippet from HW manual section 8.14:
> >> > The upper nibble of the Mode(M7-4) controls the length of the next
> >> > FAST Read Quad IO instruction through the inclusion or exclusion of
> >> > the first byte instruction code. The lower nibble bits of the
> >> > Mode(M3-0) are don't care. However, the IO pins must be
> >> > high-impedance before the falling edge of the first data out clock.
> >>
> >> I'm still not sure what you are trying to fix here. For any quad I/O
> >> mode, the pins of the controller must be in hiZ during the data phase
> >> on a read operation. Otherwise the flash couldn't send any data,
> >> there would be two drivers for one signal. So being in hiZ state
> >> should be the default and shouldn't depend on any connected flash.
> >
> > OK, I will make hiZ state as the default.
> 
> I still think this iofv setting is the wrong approach, though. Do you have
> a link to the spi controller datasheet where I can look up what the
> controller is doing.

Please find the below link.

https://www.renesas.com/eu/en/products/microcontrollers-microprocessors/rz-mpus/rzg2l-general-purpose-microprocessors-dual-core-arm-cortex-a55-12-ghz-cpus-and-single-core-arm-cortex-m33#overview

> 
> This seem to be a general problem with what we are sending during the
> command phase and I'm curious why there wasn't more reports on non working
> micron flashes for now.

1-bit mode, we don't have any issue. Once we switch to 4-bit mode we have this
issue with micron MT25QU512A flash and we need to set the correct IO fixed values.

Maybe others are testing with 1-bit mode and not testing the full capability of the flash.

> 
> >> You've mentioned the micron flash which needs a '1' on its hold/reset
> >> pin.
> >> I would have expected a fixup for this flash, not for the flash which
> >> behaves normal.
> >
> > I will drop fixup for Renesas AT25QL128A  and will add fixup for
> > micron flash.
> 
> btw, what will happen if you always use the {3,3,3,1} setting? I guess the
> atmel flash will also work? because HiZ should mean "don't care"
> from
> the point of view of the flash.

With atmel flash if use {3,3,3,1} setting, I get below error.

root@smarc-rzg2ul:/cip-test-scripts# ./rpcif_t_001.sh
[  144.078854] spi-nor spi1.0: spi-nor-generic (16384 Kbytes)
[  144.120468] 2 fixed-partitions partitions found on MTD device spi1.0
[  144.126982] Creating 2 MTD partitions on "spi1.0":
[  144.133004] 0x000000000000-0x000000200000 : "boot"
[  144.141879] 0x000000200000-0x000001000000 : "user"
[  358.476963] jffs2: notice: (230) read_dnode: node CRC failed on dnode at 0xdfe084: read 0x336ebbbc, calculated 0x961503c7
[  358.488509] jffs2: notice: (230) read_dnode: node CRC failed on dnode at 0xdfd118: read 0xff6a5df6, calculated 0x786a5df6
[  358.502963] jffs2: notice: (230) read_dnode: node CRC failed on dnode at 0xdfa2d4: read 0x1fc99948, calculated 0xbab22133
[  358.515357] jffs2: notice: (230) read_dnode: node CRC failed on dnode at 0xdf9368: read 0xffd184a7, calculated 0x3d184a7
[  358.528175] jffs2: notice: (230) read_dnode: node CRC failed on dnode at 0xdf6524: read 0x5deb2462, calculated 0xf8909c19


> 
> >
> > With iofv settings {3,3,3,3} (all pins on Hi-Z state) with Micron
> > flash
> > ----------------------------------------------------------------------
> > -
> >
> > ./rpcif_t_001.sh
> > [   37.950986] spi-nor spi1.0: unrecognized JEDEC id bytes: ff ff ff ff
> > ff ff
> 
> As mentioned earlier, I suspect that HiZ on IO3 means low and the flash
> will be in reset. Could you perhaps verify that by probing IO3?
> I know that other flashes will *either* support RESET#/HOLD# or quad mode.
> Thus I was saying, that we probably wont support that and the easiest fix
> should be to disable this behavior for the atmel flash (there was nv
> setting).

The fix up is invoked only for quad mode, I believe it is safe to add fixup for micron flash
As it is the one deviating from normal according to you, rather than adding fixup
for generic flash like ATMEL flash(Now Renesas flash)

Cheers,
Biju


> 
> I guess, the correct fix would be to somehow add support to control
> IO1-IO3 during the (single bit) command phase.



> 
> 
> >
> > EXIT|FAIL|rpcif_t_001.sh|[00:00:01] Failed to detect mt25qu512a
> > flash!||
> >
> >
> > With iofv settings {3,3,3,1} with Micron falsh
> > ---------------------------------------------
> > root@smarc-rzg2l:/cip-test-scripts# ./rpcif_t_001.sh
> > [   26.500035] spi-nor spi1.0: mt25qu512a (65536 Kbytes)
> > [   26.533995] 2 fixed-partitions partitions found on MTD device spi1.0
> > [   26.540410] Creating 2 MTD partitions on "spi1.0":
> > [   26.545239] 0x000000000000-0x000002000000 : "boot"
> > [   26.554381] 0x000002000000-0x000004000000 : "user"
> >
> > EXIT|PASS|rpcif_t_001.sh|[00:03:01] ||
> >
> > Cheers,
> > Biju

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH RFC 0/4] Add set_iofv() callback

[Michael Walle]

Hi Biju,

>> >> > As per section 8.14 on the AT25QL128A hardware manual[1],
>> >> > IO0..IO3 must be set to Hi-Z state for this flash for fast read
>> >> > quad IO.
>> >> > Snippet from HW manual section 8.14:
>> >> > The upper nibble of the Mode(M7-4) controls the length of the next
>> >> > FAST Read Quad IO instruction through the inclusion or exclusion of
>> >> > the first byte instruction code. The lower nibble bits of the
>> >> > Mode(M3-0) are don't care. However, the IO pins must be
>> >> > high-impedance before the falling edge of the first data out clock.
>> >>
>> >> I'm still not sure what you are trying to fix here. For any quad I/O
>> >> mode, the pins of the controller must be in hiZ during the data phase
>> >> on a read operation. Otherwise the flash couldn't send any data,
>> >> there would be two drivers for one signal. So being in hiZ state
>> >> should be the default and shouldn't depend on any connected flash.
>> >
>> > OK, I will make hiZ state as the default.
>> 
>> I still think this iofv setting is the wrong approach, though. Do you 
>> have
>> a link to the spi controller datasheet where I can look up what the
>> controller is doing.
> 
> Please find the below link.
> 
> https://www.renesas.com/eu/en/products/microcontrollers-microprocessors/rz-mpus/rzg2l-general-purpose-microprocessors-dual-core-arm-cortex-a55-12-ghz-cpus-and-single-core-arm-cortex-m33#overview

Thanks.

>> This seem to be a general problem with what we are sending during the
>> command phase and I'm curious why there wasn't more reports on non 
>> working
>> micron flashes for now.
> 
> 1-bit mode, we don't have any issue. Once we switch to 4-bit mode we 
> have this
> issue with micron MT25QU512A flash and we need to set the correct IO 
> fixed values.
> 
> Maybe others are testing with 1-bit mode and not testing the full 
> capability of the flash.
> 
>> 
>> >> You've mentioned the micron flash which needs a '1' on its hold/reset
>> >> pin.
>> >> I would have expected a fixup for this flash, not for the flash which
>> >> behaves normal.
>> >
>> > I will drop fixup for Renesas AT25QL128A  and will add fixup for
>> > micron flash.
>> 
>> btw, what will happen if you always use the {3,3,3,1} setting? I guess 
>> the
>> atmel flash will also work? because HiZ should mean "don't care"
>> from
>> the point of view of the flash.
> 
> With atmel flash if use {3,3,3,1} setting, I get below error.
> 
> root@smarc-rzg2ul:/cip-test-scripts# ./rpcif_t_001.sh
> [  144.078854] spi-nor spi1.0: spi-nor-generic (16384 Kbytes)
> [  144.120468] 2 fixed-partitions partitions found on MTD device spi1.0
> [  144.126982] Creating 2 MTD partitions on "spi1.0":
> [  144.133004] 0x000000000000-0x000000200000 : "boot"
> [  144.141879] 0x000000200000-0x000001000000 : "user"
> [  358.476963] jffs2: notice: (230) read_dnode: node CRC failed on 
> dnode at 0xdfe084: read 0x336ebbbc, calculated 0x961503c7
> [  358.488509] jffs2: notice: (230) read_dnode: node CRC failed on 
> dnode at 0xdfd118: read 0xff6a5df6, calculated 0x786a5df6
> [  358.502963] jffs2: notice: (230) read_dnode: node CRC failed on 
> dnode at 0xdfa2d4: read 0x1fc99948, calculated 0xbab22133
> [  358.515357] jffs2: notice: (230) read_dnode: node CRC failed on 
> dnode at 0xdf9368: read 0xffd184a7, calculated 0x3d184a7
> [  358.528175] jffs2: notice: (230) read_dnode: node CRC failed on 
> dnode at 0xdf6524: read 0x5deb2462, calculated 0xf8909c19

Strange. Can't make any sense of this at the moment.

>> > With iofv settings {3,3,3,3} (all pins on Hi-Z state) with Micron
>> > flash
>> > ----------------------------------------------------------------------
>> > -
>> >
>> > ./rpcif_t_001.sh
>> > [   37.950986] spi-nor spi1.0: unrecognized JEDEC id bytes: ff ff ff ff
>> > ff ff
>> 
>> As mentioned earlier, I suspect that HiZ on IO3 means low and the 
>> flash
>> will be in reset. Could you perhaps verify that by probing IO3?
>> I know that other flashes will *either* support RESET#/HOLD# or quad 
>> mode.
>> Thus I was saying, that we probably wont support that and the easiest 
>> fix
>> should be to disable this behavior for the atmel flash (there was nv
>> setting).
> 
> The fix up is invoked only for quad mode, I believe it is safe to add 
> fixup for micron flash
> As it is the one deviating from normal according to you, rather than 
> adding fixup
> for generic flash like ATMEL flash(Now Renesas flash)

Could you please try setting bit 4 in the Nonvolatile Configuration
Register (Table 7) and see if the problem goes away?

Also could you have a look at the schematics, does the IO3/RESET#
have a pull-up? If not, who is in control of driving the correct
value here? If it has a pull-up, I'm puzzled why you need any
other setting than HiZ.

The correct fix would be to the information about the missing
IO state in the "struct spi_mem_op". That is, what should be the
default values of all the IO lines which are unused. For example
if we have a 1s1s4s transaction, what should be the state of IO0,
IO2 and IO3 during the command and address phase. If we have a
1s2s2s, what should be the state of IO0 during the command phase
etc.

That can then be used within your driver to set the corresponding
IOFV values (for each spi-mem op).

But I'm not sure if other SPI controllers will support that, though.

-michael

Re: [PATCH RFC 0/4] Add set_iofv() callback

[Michael Walle]

Hi Biju,

>> >> > As per section 8.14 on the AT25QL128A hardware manual[1],
>> >> > IO0..IO3 must be set to Hi-Z state for this flash for fast read
>> >> > quad IO.
>> >> > Snippet from HW manual section 8.14:
>> >> > The upper nibble of the Mode(M7-4) controls the length of the next
>> >> > FAST Read Quad IO instruction through the inclusion or exclusion of
>> >> > the first byte instruction code. The lower nibble bits of the
>> >> > Mode(M3-0) are don't care. However, the IO pins must be
>> >> > high-impedance before the falling edge of the first data out clock.
>> >>
>> >> I'm still not sure what you are trying to fix here. For any quad I/O
>> >> mode, the pins of the controller must be in hiZ during the data phase
>> >> on a read operation. Otherwise the flash couldn't send any data,
>> >> there would be two drivers for one signal. So being in hiZ state
>> >> should be the default and shouldn't depend on any connected flash.
>> >
>> > OK, I will make hiZ state as the default.
>> 
>> I still think this iofv setting is the wrong approach, though. Do you 
>> have
>> a link to the spi controller datasheet where I can look up what the
>> controller is doing.
> 
> Please find the below link.
> 
> https://www.renesas.com/eu/en/products/microcontrollers-microprocessors/rz-mpus/rzg2l-general-purpose-microprocessors-dual-core-arm-cortex-a55-12-ghz-cpus-and-single-core-arm-cortex-m33#overview

Thanks.

>> This seem to be a general problem with what we are sending during the
>> command phase and I'm curious why there wasn't more reports on non 
>> working
>> micron flashes for now.
> 
> 1-bit mode, we don't have any issue. Once we switch to 4-bit mode we 
> have this
> issue with micron MT25QU512A flash and we need to set the correct IO 
> fixed values.
> 
> Maybe others are testing with 1-bit mode and not testing the full 
> capability of the flash.
> 
>> 
>> >> You've mentioned the micron flash which needs a '1' on its hold/reset
>> >> pin.
>> >> I would have expected a fixup for this flash, not for the flash which
>> >> behaves normal.
>> >
>> > I will drop fixup for Renesas AT25QL128A  and will add fixup for
>> > micron flash.
>> 
>> btw, what will happen if you always use the {3,3,3,1} setting? I guess 
>> the
>> atmel flash will also work? because HiZ should mean "don't care"
>> from
>> the point of view of the flash.
> 
> With atmel flash if use {3,3,3,1} setting, I get below error.
> 
> root@smarc-rzg2ul:/cip-test-scripts# ./rpcif_t_001.sh
> [  144.078854] spi-nor spi1.0: spi-nor-generic (16384 Kbytes)
> [  144.120468] 2 fixed-partitions partitions found on MTD device spi1.0
> [  144.126982] Creating 2 MTD partitions on "spi1.0":
> [  144.133004] 0x000000000000-0x000000200000 : "boot"
> [  144.141879] 0x000000200000-0x000001000000 : "user"
> [  358.476963] jffs2: notice: (230) read_dnode: node CRC failed on 
> dnode at 0xdfe084: read 0x336ebbbc, calculated 0x961503c7
> [  358.488509] jffs2: notice: (230) read_dnode: node CRC failed on 
> dnode at 0xdfd118: read 0xff6a5df6, calculated 0x786a5df6
> [  358.502963] jffs2: notice: (230) read_dnode: node CRC failed on 
> dnode at 0xdfa2d4: read 0x1fc99948, calculated 0xbab22133
> [  358.515357] jffs2: notice: (230) read_dnode: node CRC failed on 
> dnode at 0xdf9368: read 0xffd184a7, calculated 0x3d184a7
> [  358.528175] jffs2: notice: (230) read_dnode: node CRC failed on 
> dnode at 0xdf6524: read 0x5deb2462, calculated 0xf8909c19

Strange. Can't make any sense of this at the moment.

>> > With iofv settings {3,3,3,3} (all pins on Hi-Z state) with Micron
>> > flash
>> > ----------------------------------------------------------------------
>> > -
>> >
>> > ./rpcif_t_001.sh
>> > [   37.950986] spi-nor spi1.0: unrecognized JEDEC id bytes: ff ff ff ff
>> > ff ff
>> 
>> As mentioned earlier, I suspect that HiZ on IO3 means low and the 
>> flash
>> will be in reset. Could you perhaps verify that by probing IO3?
>> I know that other flashes will *either* support RESET#/HOLD# or quad 
>> mode.
>> Thus I was saying, that we probably wont support that and the easiest 
>> fix
>> should be to disable this behavior for the atmel flash (there was nv
>> setting).
> 
> The fix up is invoked only for quad mode, I believe it is safe to add 
> fixup for micron flash
> As it is the one deviating from normal according to you, rather than 
> adding fixup
> for generic flash like ATMEL flash(Now Renesas flash)

Could you please try setting bit 4 in the Nonvolatile Configuration
Register (Table 7) and see if the problem goes away?

Also could you have a look at the schematics, does the IO3/RESET#
have a pull-up? If not, who is in control of driving the correct
value here? If it has a pull-up, I'm puzzled why you need any
other setting than HiZ.

The correct fix would be to the information about the missing
IO state in the "struct spi_mem_op". That is, what should be the
default values of all the IO lines which are unused. For example
if we have a 1s1s4s transaction, what should be the state of IO0,
IO2 and IO3 during the command and address phase. If we have a
1s2s2s, what should be the state of IO0 during the command phase
etc.

That can then be used within your driver to set the corresponding
IOFV values (for each spi-mem op).

But I'm not sure if other SPI controllers will support that, though.

-michael

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

RE: [PATCH RFC 0/4] Add set_iofv() callback

[Biju Das]

Hi Michael Walle,

> Subject: Re: [PATCH RFC 0/4] Add set_iofv() callback
> 
> Hi Biju,
> 
> >> >> > As per section 8.14 on the AT25QL128A hardware manual[1],
> >> >> > IO0..IO3 must be set to Hi-Z state for this flash for fast read
> >> >> > quad IO.
> >> >> > Snippet from HW manual section 8.14:
> >> >> > The upper nibble of the Mode(M7-4) controls the length of the
> >> >> > next FAST Read Quad IO instruction through the inclusion or
> >> >> > exclusion of the first byte instruction code. The lower nibble
> >> >> > bits of the
> >> >> > Mode(M3-0) are don't care. However, the IO pins must be
> >> >> > high-impedance before the falling edge of the first data out
> clock.
> >> >>
> >> >> I'm still not sure what you are trying to fix here. For any quad
> >> >> I/O mode, the pins of the controller must be in hiZ during the
> >> >> data phase on a read operation. Otherwise the flash couldn't send
> >> >> any data, there would be two drivers for one signal. So being in
> >> >> hiZ state should be the default and shouldn't depend on any
> connected flash.
> >> >
> >> > OK, I will make hiZ state as the default.
> >>
> >> I still think this iofv setting is the wrong approach, though. Do you
> >> have a link to the spi controller datasheet where I can look up what
> >> the controller is doing.
> >
> > Please find the below link.
> >
> > https://www.renesas.com/eu/en/products/microcontrollers-microprocessor
> > s/rz-mpus/rzg2l-general-purpose-microprocessors-dual-core-arm-cortex-a
> > 55-12-ghz-cpus-and-single-core-arm-cortex-m33#overview
> 
> Thanks.
> 
> >> This seem to be a general problem with what we are sending during the
> >> command phase and I'm curious why there wasn't more reports on non
> >> working micron flashes for now.
> >
> > 1-bit mode, we don't have any issue. Once we switch to 4-bit mode we
> > have this issue with micron MT25QU512A flash and we need to set the
> > correct IO fixed values.
> >
> > Maybe others are testing with 1-bit mode and not testing the full
> > capability of the flash.
> >
> >>
> >> >> You've mentioned the micron flash which needs a '1' on its
> >> >> hold/reset pin.
> >> >> I would have expected a fixup for this flash, not for the flash
> >> >> which behaves normal.
> >> >
> >> > I will drop fixup for Renesas AT25QL128A  and will add fixup for
> >> > micron flash.
> >>
> >> btw, what will happen if you always use the {3,3,3,1} setting? I
> >> guess the atmel flash will also work? because HiZ should mean "don't
> >> care"
> >> from
> >> the point of view of the flash.
> >
> > With atmel flash if use {3,3,3,1} setting, I get below error.
> >
> > root@smarc-rzg2ul:/cip-test-scripts# ./rpcif_t_001.sh [  144.078854]
> > spi-nor spi1.0: spi-nor-generic (16384 Kbytes) [  144.120468] 2
> > fixed-partitions partitions found on MTD device spi1.0 [  144.126982]
> > Creating 2 MTD partitions on "spi1.0":
> > [  144.133004] 0x000000000000-0x000000200000 : "boot"
> > [  144.141879] 0x000000200000-0x000001000000 : "user"
> > [  358.476963] jffs2: notice: (230) read_dnode: node CRC failed on
> > dnode at 0xdfe084: read 0x336ebbbc, calculated 0x961503c7 [
> > 358.488509] jffs2: notice: (230) read_dnode: node CRC failed on dnode
> > at 0xdfd118: read 0xff6a5df6, calculated 0x786a5df6 [  358.502963]
> > jffs2: notice: (230) read_dnode: node CRC failed on dnode at 0xdfa2d4:
> > read 0x1fc99948, calculated 0xbab22133 [  358.515357] jffs2: notice:
> > (230) read_dnode: node CRC failed on dnode at 0xdf9368: read
> > 0xffd184a7, calculated 0x3d184a7 [  358.528175] jffs2: notice: (230)
> > read_dnode: node CRC failed on dnode at 0xdf6524: read 0x5deb2462,
> > calculated 0xf8909c19
> 
> Strange. Can't make any sense of this at the moment.
> 
> >> > With iofv settings {3,3,3,3} (all pins on Hi-Z state) with Micron
> >> > flash
> >> > -------------------------------------------------------------------
> >> > ---
> >> > -
> >> >
> >> > ./rpcif_t_001.sh
> >> > [   37.950986] spi-nor spi1.0: unrecognized JEDEC id bytes: ff ff ff
> ff
> >> > ff ff
> >>
> >> As mentioned earlier, I suspect that HiZ on IO3 means low and the
> >> flash will be in reset. Could you perhaps verify that by probing IO3?
> >> I know that other flashes will *either* support RESET#/HOLD# or quad
> >> mode.


> >> Thus I was saying, that we probably wont support that and the easiest
> >> fix should be to disable this behavior for the atmel flash (there was
> >> nv setting).
> >
> > The fix up is invoked only for quad mode, I believe it is safe to add
> > fixup for micron flash As it is the one deviating from normal
> > according to you, rather than adding fixup for generic flash like
> > ATMEL flash(Now Renesas flash)
> 
> Could you please try setting bit 4 in the Nonvolatile Configuration
> Register (Table 7) and see if the problem goes away?

You mean, if it works, we need to disable reset for all the boards, maybe at bootloader level??

OK, I will check that. Currently I have read that register and it is showing a value
Of 0xffbb. I need to do write operation. Before that how do we recover flash, if
something goes wrong during writing for NV register?


> 
> Also could you have a look at the schematics, does the IO3/RESET# have a
> pull-up? If not, who is in control of driving the correct value here? If
> it has a pull-up, I'm puzzled why you need any other setting than HiZ.

Unfortunately, there is no pullup on IO3 line and also there is no SoC pullup.

> 
> The correct fix would be to the information about the missing IO state in
> the "struct spi_mem_op". That is, what should be the default values of all
> the IO lines which are unused. For example if we have a 1s1s4s
> transaction, what should be the state of IO0,
> IO2 and IO3 during the command and address phase. If we have a 1s2s2s,
> what should be the state of IO0 during the command phase etc.
> 
> That can then be used within your driver to set the corresponding IOFV
> values (for each spi-mem op).
> 
> But I'm not sure if other SPI controllers will support that, though.

Currently driving SoC IOFV register, it fixes the issue and it is just one time
operation during post sfdp. I take this as a SoC feature which other controllers
don't have.

Cheers,
Biju

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH RFC 0/4] Add set_iofv() callback

[Michael Walle]

Hi Biju,

>> >> Thus I was saying, that we probably wont support that and the easiest
>> >> fix should be to disable this behavior for the atmel flash (there was
>> >> nv setting).
>> >
>> > The fix up is invoked only for quad mode, I believe it is safe to add
>> > fixup for micron flash As it is the one deviating from normal
>> > according to you, rather than adding fixup for generic flash like
>> > ATMEL flash(Now Renesas flash)
>> 
>> Could you please try setting bit 4 in the Nonvolatile Configuration
>> Register (Table 7) and see if the problem goes away?
> 
> You mean, if it works, we need to disable reset for all the boards, 
> maybe at bootloader level??

Not necessarily. First, just to confirm that it is actually the reset
circuit. You can also compare the part numbers of the flash. There
is a flash with IO3/RESET# and IO3/HOLD# (and a flash with a dedicated
reset pin).

If that's the case, it looks like a hardware bug on your board. You
left the reset pin floating. So you'd also not be able to boot from
the NOR flash, right?

> OK, I will check that. Currently I have read that register and it is 
> showing a value
> Of 0xffbb. I need to do write operation. Before that how do we recover 
> flash, if
> something goes wrong during writing for NV register?

You should always be able to write that register from the bootloader.
Maybe also through raw commands (like sspi in uboot).

>> Also could you have a look at the schematics, does the IO3/RESET# have 
>> a
>> pull-up? If not, who is in control of driving the correct value here? 
>> If
>> it has a pull-up, I'm puzzled why you need any other setting than HiZ.
> 
> Unfortunately, there is no pullup on IO3 line and also there is no SoC 
> pullup.

See above.

-michael

>> The correct fix would be to the information about the missing IO state 
>> in
>> the "struct spi_mem_op". That is, what should be the default values of 
>> all
>> the IO lines which are unused. For example if we have a 1s1s4s
>> transaction, what should be the state of IO0,
>> IO2 and IO3 during the command and address phase. If we have a 1s2s2s,
>> what should be the state of IO0 during the command phase etc.
>> 
>> That can then be used within your driver to set the corresponding IOFV
>> values (for each spi-mem op).
>> 
>> But I'm not sure if other SPI controllers will support that, though.
> 
> Currently driving SoC IOFV register, it fixes the issue and it is just 
> one time
> operation during post sfdp. I take this as a SoC feature which other 
> controllers
> don't have.
> 
> Cheers,
> Biju

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH RFC 0/4] Add set_iofv() callback

[Michael Walle]

Hi Biju,

>> >> Thus I was saying, that we probably wont support that and the easiest
>> >> fix should be to disable this behavior for the atmel flash (there was
>> >> nv setting).
>> >
>> > The fix up is invoked only for quad mode, I believe it is safe to add
>> > fixup for micron flash As it is the one deviating from normal
>> > according to you, rather than adding fixup for generic flash like
>> > ATMEL flash(Now Renesas flash)
>> 
>> Could you please try setting bit 4 in the Nonvolatile Configuration
>> Register (Table 7) and see if the problem goes away?
> 
> You mean, if it works, we need to disable reset for all the boards, 
> maybe at bootloader level??

Not necessarily. First, just to confirm that it is actually the reset
circuit. You can also compare the part numbers of the flash. There
is a flash with IO3/RESET# and IO3/HOLD# (and a flash with a dedicated
reset pin).

If that's the case, it looks like a hardware bug on your board. You
left the reset pin floating. So you'd also not be able to boot from
the NOR flash, right?

> OK, I will check that. Currently I have read that register and it is 
> showing a value
> Of 0xffbb. I need to do write operation. Before that how do we recover 
> flash, if
> something goes wrong during writing for NV register?

You should always be able to write that register from the bootloader.
Maybe also through raw commands (like sspi in uboot).

>> Also could you have a look at the schematics, does the IO3/RESET# have 
>> a
>> pull-up? If not, who is in control of driving the correct value here? 
>> If
>> it has a pull-up, I'm puzzled why you need any other setting than HiZ.
> 
> Unfortunately, there is no pullup on IO3 line and also there is no SoC 
> pullup.

See above.

-michael

>> The correct fix would be to the information about the missing IO state 
>> in
>> the "struct spi_mem_op". That is, what should be the default values of 
>> all
>> the IO lines which are unused. For example if we have a 1s1s4s
>> transaction, what should be the state of IO0,
>> IO2 and IO3 during the command and address phase. If we have a 1s2s2s,
>> what should be the state of IO0 during the command phase etc.
>> 
>> That can then be used within your driver to set the corresponding IOFV
>> values (for each spi-mem op).
>> 
>> But I'm not sure if other SPI controllers will support that, though.
> 
> Currently driving SoC IOFV register, it fixes the issue and it is just 
> one time
> operation during post sfdp. I take this as a SoC feature which other 
> controllers
> don't have.
> 
> Cheers,
> Biju

RE: [PATCH RFC 0/4] Add set_iofv() callback

[Biju Das]

Hi Michael Walle,

Thanks for the feedback.


> Subject: Re: [PATCH RFC 0/4] Add set_iofv() callback
> 
> Hi Biju,
> 
> >> >> Thus I was saying, that we probably wont support that and the
> >> >> easiest fix should be to disable this behavior for the atmel flash
> >> >> (there was nv setting).
> >> >
> >> > The fix up is invoked only for quad mode, I believe it is safe to
> >> > add fixup for micron flash As it is the one deviating from normal
> >> > according to you, rather than adding fixup for generic flash like
> >> > ATMEL flash(Now Renesas flash)
> >>
> >> Could you please try setting bit 4 in the Nonvolatile Configuration
> >> Register (Table 7) and see if the problem goes away?
> >
> > You mean, if it works, we need to disable reset for all the boards,
> > maybe at bootloader level??
> 
> Not necessarily. First, just to confirm that it is actually the reset
> circuit. You can also compare the part numbers of the flash. There is a
> flash with IO3/RESET# and IO3/HOLD# (and a flash with a dedicated reset
> pin).

Part is MT25QU512ABB8E12-0SIT, As per the schematic, flash has a dedicated RESET# with 10K pullup
connected to SoC QSPI_RESET pin.

DQ0, DQ1, W#/DQ2 and DQ3 lines on the flash are connected without any pullups to the SoC QSPI0_{0..3} pins.

> 
> If that's the case, it looks like a hardware bug on your board. You left
> the reset pin floating. So you'd also not be able to boot from the NOR
> flash, right?

I am booting from NOR flash. BootRom code reads SPI flash and executes BL2.
BL2 loads BL33 and U-boot from NOR flash. If this is the case, do you think it is a
Hw bug on the board?

> 
> > OK, I will check that. Currently I have read that register and it is
> > showing a value Of 0xffbb. I need to do write operation. Before that
> > how do we recover flash, if something goes wrong during writing for NV
> > register?
> 
> You should always be able to write that register from the bootloader.
> Maybe also through raw commands (like sspi in uboot).

Thanks for the pointer, I haven't explored the uboot path.

Currently I have added the code for reading and writing NV register in Linux. But looks like
Write is not working as expected. Looks like there is no API for non-volatile write in linux?


+++ b/drivers/mtd/spi-nor/micron-st.c
@@ -12,6 +12,8 @@
 #define USE_FSR                BIT(0)
 
 #define SPINOR_OP_RDFSR                0x70    /* Read flag status register */
+#define SPINOR_OP_RDNV         0xB5    /* Read NV register */
+#define SPINOR_OP_WRNV         0xB1    /* Write NV register */
 #define SPINOR_OP_CLFSR                0x50    /* Clear flag status register */
 #define SPINOR_OP_MT_DTR_RD    0xfd    /* Fast Read opcode in DTR mode */
 #define SPINOR_OP_MT_RD_ANY_REG        0x85    /* Read volatile register */
@@ -41,6 +43,19 @@
                   SPI_MEM_OP_NO_DUMMY,                                 \
                   SPI_MEM_OP_DATA_IN(1, buf, 0))
 
+#define MICRON_ST_RD_NV_OP(buf)                                                \
+       SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_RDNV, 0),                   \
+                  SPI_MEM_OP_NO_ADDR,                                  \
+                  SPI_MEM_OP_NO_DUMMY,                                 \
+                  SPI_MEM_OP_DATA_IN(2, buf, 0))
+
+#define MICRON_ST_WR_NV_OP(naddr, addr, ndata, buf)            \
+       SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WRNV, 0),           \
+                  SPI_MEM_OP_ADDR(naddr, addr, 0),                     \
+                  SPI_MEM_OP_NO_DUMMY,                                 \
+                  SPI_MEM_OP_DATA_OUT(2, buf, 0))
+
+
 #define MICRON_ST_CLFSR_OP                                             \
        SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_CLFSR, 0),                  \
                   SPI_MEM_OP_NO_ADDR,                                  \

static int mt25qu512a_post_sfdp_fixup(struct spi_nor *nor)
{
        if (nor->spimem) {
               struct spi_device *spi = nor->spimem->spi;
               u8 addr_mode_nbytes = nor->params->addr_mode_nbytes;
		   u8 *fsr = nor->bouncebuf;
               int ret;
               struct spi_mem_op op;

               op = (struct spi_mem_op) MICRON_ST_RD_NV_OP(fsr);
               spi_nor_read_any_reg(nor, &op, nor->reg_proto);
               pr_err("######%x/%x",*fsr,*(fsr+1));

		    fsr[0] = 0xef;
		    fsr[1] = 0xff;
                op = (struct spi_mem_op)
                       MICRON_ST_WR_NV_OP(addr_mode_nbytes,
                                                  SPINOR_OP_WRNV, 2, fsr);
              ret = spi_nor_write_any_volatile_reg(nor, &op, nor->reg_proto);

              pr_err("######%x/%x/%d",*fsr,*(fsr+1),ret);
 
               op = (struct spi_mem_op) MICRON_ST_RD_NV_OP(fsr);
               spi_nor_read_any_reg(nor, &op, nor->reg_proto);
               pr_err("######%x/%x",*fsr,*(fsr+1));
        }
 
        return 0;
}

Cheers,
Biju

> 
> >> Also could you have a look at the schematics, does the IO3/RESET#
> >> have a pull-up? If not, who is in control of driving the correct
> >> value here?
> >> If
> >> it has a pull-up, I'm puzzled why you need any other setting than HiZ.
> >
> > Unfortunately, there is no pullup on IO3 line and also there is no SoC
> > pullup.
> 
> See above.
> 
> -michael

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

RE: [PATCH RFC 0/4] Add set_iofv() callback

[Biju Das]

Hi Michael Walle,

> Subject: RE: [PATCH RFC 0/4] Add set_iofv() callback
> 
 
> 
> > Subject: Re: [PATCH RFC 0/4] Add set_iofv() callback
> >
> > Hi Biju,
> >
> > >> >> Thus I was saying, that we probably wont support that and the
> > >> >> easiest fix should be to disable this behavior for the atmel
> > >> >> flash (there was nv setting).
> > >> >
> > >> > The fix up is invoked only for quad mode, I believe it is safe to
> > >> > add fixup for micron flash As it is the one deviating from normal
> > >> > according to you, rather than adding fixup for generic flash like
> > >> > ATMEL flash(Now Renesas flash)
> > >>
> > >> Could you please try setting bit 4 in the Nonvolatile Configuration
> > >> Register (Table 7) and see if the problem goes away?
> > >
> > > You mean, if it works, we need to disable reset for all the boards,
> > > maybe at bootloader level??
> >
> > Not necessarily. First, just to confirm that it is actually the reset
> > circuit. You can also compare the part numbers of the flash. There is
> > a flash with IO3/RESET# and IO3/HOLD# (and a flash with a dedicated
> > reset pin).
> 
> Part is MT25QU512ABB8E12-0SIT, As per the schematic, flash has a dedicated
> RESET# with 10K pullup connected to SoC QSPI_RESET pin.
> 
> DQ0, DQ1, W#/DQ2 and DQ3 lines on the flash are connected without any
> pullups to the SoC QSPI0_{0..3} pins.
> 
> >
> > If that's the case, it looks like a hardware bug on your board. You
> > left the reset pin floating. So you'd also not be able to boot from
> > the NOR flash, right?
> 
> I am booting from NOR flash. BootRom code reads SPI flash and executes
> BL2.
> BL2 loads BL33 and U-boot from NOR flash. If this is the case, do you
> think it is a Hw bug on the board?
> 
> >
> > > OK, I will check that. Currently I have read that register and it is
> > > showing a value Of 0xffbb. I need to do write operation. Before that
> > > how do we recover flash, if something goes wrong during writing for
> > > NV register?
> >
> > You should always be able to write that register from the bootloader.
> > Maybe also through raw commands (like sspi in uboot).
> 
> Thanks for the pointer, I haven't explored the uboot path.

I have disabled RESET# bit in the Nonvolatile Configuration
Register (Table 7) and borad doesn't boot any more.

By default that bit is set.

[    2.530291] ###### Before write Read cmd=b5 val=ff
[    2.530431] ###### write cmd=b1 val=ef
[    2.535518] ###### Read cmd=b5 val=ef


NOTICE:  BL2: Built : 14:59:28, Nov 10 2023
ERROR:   BL2: Failed to load image id 3 (-2)
NOTICE:  BL2: v2.9(release):v2.5/rzg2l-1.00-3883-gc314a391c
NOTICE:  BL2: Built : 14:59:28, Nov 10 2023
ERROR:   BL2: Failed to load image id 3 (-2)
NOTICE:  BL2: v2.9(release):v2.5/rzg2l-1.00-3883-gc314a391c

What is your thoughts on this? How do we proceed now?


Cheers,
Biju


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

RE: [PATCH RFC 0/4] Add set_iofv() callback

[Biju Das]

Hi Michael Walle,

> Subject: RE: [PATCH RFC 0/4] Add set_iofv() callback
> 
> Hi Michael Walle,
> 
> > Subject: RE: [PATCH RFC 0/4] Add set_iofv() callback
> >
> 
> >
> > > Subject: Re: [PATCH RFC 0/4] Add set_iofv() callback
> > >
> > > Hi Biju,
> > >
> > > >> >> Thus I was saying, that we probably wont support that and the
> > > >> >> easiest fix should be to disable this behavior for the atmel
> > > >> >> flash (there was nv setting).
> > > >> >
> > > >> > The fix up is invoked only for quad mode, I believe it is safe
> > > >> > to add fixup for micron flash As it is the one deviating from
> > > >> > normal according to you, rather than adding fixup for generic
> > > >> > flash like ATMEL flash(Now Renesas flash)
> > > >>
> > > >> Could you please try setting bit 4 in the Nonvolatile
> > > >> Configuration Register (Table 7) and see if the problem goes away?
> > > >
> > > > You mean, if it works, we need to disable reset for all the
> > > > boards, maybe at bootloader level??
> > >
> > > Not necessarily. First, just to confirm that it is actually the
> > > reset circuit. You can also compare the part numbers of the flash.
> > > There is a flash with IO3/RESET# and IO3/HOLD# (and a flash with a
> > > dedicated reset pin).
> >
> > Part is MT25QU512ABB8E12-0SIT, As per the schematic, flash has a
> > dedicated RESET# with 10K pullup connected to SoC QSPI_RESET pin.
> >
> > DQ0, DQ1, W#/DQ2 and DQ3 lines on the flash are connected without any
> > pullups to the SoC QSPI0_{0..3} pins.
> >
> > >
> > > If that's the case, it looks like a hardware bug on your board. You
> > > left the reset pin floating. So you'd also not be able to boot from
> > > the NOR flash, right?
> >
> > I am booting from NOR flash. BootRom code reads SPI flash and executes
> > BL2.
> > BL2 loads BL33 and U-boot from NOR flash. If this is the case, do you
> > think it is a Hw bug on the board?
> >
> > >
> > > > OK, I will check that. Currently I have read that register and it
> > > > is showing a value Of 0xffbb. I need to do write operation. Before
> > > > that how do we recover flash, if something goes wrong during
> > > > writing for NV register?
> > >
> > > You should always be able to write that register from the bootloader.
> > > Maybe also through raw commands (like sspi in uboot).
> >
> > Thanks for the pointer, I haven't explored the uboot path.
> 
> I have disabled RESET# bit in the Nonvolatile Configuration Register
> (Table 7) and borad doesn't boot any more.
> 
> By default that bit is set.
> 
> [    2.530291] ###### Before write Read cmd=b5 val=ff
> [    2.530431] ###### write cmd=b1 val=ef
> [    2.535518] ###### Read cmd=b5 val=ef
> 
> 
> NOTICE:  BL2: Built : 14:59:28, Nov 10 2023
> ERROR:   BL2: Failed to load image id 3 (-2)
> NOTICE:  BL2: v2.9(release):v2.5/rzg2l-1.00-3883-gc314a391c
> NOTICE:  BL2: Built : 14:59:28, Nov 10 2023
> ERROR:   BL2: Failed to load image id 3 (-2)
> NOTICE:  BL2: v2.9(release):v2.5/rzg2l-1.00-3883-gc314a391c
> 
> What is your thoughts on this? How do we proceed now?
> 

Just to add I have recovered the spi flash by booting with eMMC boot and
Setting bit4 in Table 7. Now I can boot with SPI flash.

[    2.521879] ###### Before write Read cmd=b5 val=ef
[    2.522163] ###### write cmd=b1 val=ff
[    2.527350] ###### Read cmd=b5 val=ff

How do we proceed now? Krysztof/Geert, what is your thoughts?

Shall we set default values as Hi-Z for IO FV in bus driver and

As Micron flash is behaving differently shall we use SoC register(IOFV) to fix the
4-bit tx mode like patch [1] or you prefer [2]? 

[1]
https://patchwork.kernel.org/project/linux-renesas-soc/patch/20230830145835.296690-1-biju.das.jz@bp.renesas.com/


[2]
https://lore.kernel.org/linux-renesas-soc/dcfa2cab21fc85bb9b2b0c1ceb754a1a@walle.cc/T/#t


Please find the sample code for setting NV Config register for micron flash

+#define MICRON_ST_RD_NV_OP(buf)                                                \
+       SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_RDNV, 0),                   \
+                  SPI_MEM_OP_NO_ADDR,                                  \
+                  SPI_MEM_OP_NO_DUMMY,                                 \
+                  SPI_MEM_OP_DATA_IN(2, buf, 0))
+
+#define MICRON_ST_WR_NV_OP(naddr, addr, ndata, buf)            \
+       SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WRNV, 0),           \
+                  SPI_MEM_OP_NO_ADDR,                  \
+                  SPI_MEM_OP_NO_DUMMY,                                 \
+                  SPI_MEM_OP_DATA_OUT(2, buf, 0))
+
+#define SPINOR_OP_RDNV         0xb5    /* Read flag status register */
+#define SPINOR_OP_WRNV         0xb1    /* Write flag status register */


int spi_nor_scan(struct spi_nor *nor, const char *name,
                 const struct spi_nor_hwcaps *hwcaps)
 {
@@ -3455,6 +3470,8 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
        struct mtd_info *mtd = &nor->mtd;
        int ret;
        int i;
+       struct spi_mem_op op;
+
 
        ret = spi_nor_check(nor);
        if (ret)
@@ -3479,6 +3496,23 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
        if (!nor->bouncebuf)
                return -ENOMEM;
 
+       nor->flags &= ~SNOR_F_HAS_16BIT_SR;
+       op = (struct spi_mem_op) MICRON_ST_RD_NV_OP(nor->bouncebuf);
+       spi_nor_read_any_reg(nor, &op, nor->reg_proto);
+       pr_err("###### Before write Read cmd=%x val=%x",SPINOR_OP_RDNV, nor->bouncebuf[0]);
+
+       nor->bouncebuf[0] = 0xff;
+       nor->bouncebuf[1] = 0xff;
+       op = (struct spi_mem_op)
+               MICRON_ST_WR_NV_OP(0, SPINOR_OP_WRNV, 2, nor->bouncebuf);
+       ret = spi_nor_write_any_volatile_reg(nor, &op, nor->reg_proto);
+
+       pr_err("###### write cmd=%x val=%x", SPINOR_OP_WRNV, nor->bouncebuf[0]);
+
+       op = (struct spi_mem_op) MICRON_ST_RD_NV_OP(nor->bouncebuf);
+       spi_nor_read_any_reg(nor, &op, nor->reg_proto);
+       pr_err("###### Read cmd=%x val=%x",SPINOR_OP_RDNV, nor->bouncebuf[0]);
+
        ret = spi_nor_hw_reset(nor);
        if (ret)
                return ret;

Cheers,
Biju


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

RE: [PATCH RFC 0/4] Add set_iofv() callback

[Biju Das]

Hi Michael Walle,

> Subject: Re: [PATCH RFC 0/4] Add set_iofv() callback
> 
> Hi Biju,
> 
> >> >> Thus I was saying, that we probably wont support that and the
> >> >> easiest fix should be to disable this behavior for the atmel flash
> >> >> (there was nv setting).
> >> >
> >> > The fix up is invoked only for quad mode, I believe it is safe to
> >> > add fixup for micron flash As it is the one deviating from normal
> >> > according to you, rather than adding fixup for generic flash like
> >> > ATMEL flash(Now Renesas flash)
> >>
> >> Could you please try setting bit 4 in the Nonvolatile Configuration
> >> Register (Table 7) and see if the problem goes away?
> >
> > You mean, if it works, we need to disable reset for all the boards,
> > maybe at bootloader level??
> 
> Not necessarily. First, just to confirm that it is actually the reset
> circuit. You can also compare the part numbers of the flash. There is a
> flash with IO3/RESET# and IO3/HOLD# (and a flash with a dedicated reset
> pin).
> 
> If that's the case, it looks like a hardware bug on your board. You left
> the reset pin floating. So you'd also not be able to boot from the NOR
> flash, right?
> 
> > OK, I will check that. Currently I have read that register and it is
> > showing a value Of 0xffbb. I need to do write operation. Before that
> > how do we recover flash, if something goes wrong during writing for NV
> > register?
> 
> You should always be able to write that register from the bootloader.
> Maybe also through raw commands (like sspi in uboot).

Just an update, now clearing bit4 on Micron flash, I am able to test erase/read/write
Micron flash with IOFV state {3,3,3,3}. Not sure what went wrong previously.
only thing I changed is related to enabling the QUAD spi mode by disabling bit 2 and 3 on NV register.
This again result in boot failure as boot ROM expects extended SPI mode.
Then restored the extended SPI mode by sending the command FFh (Power Loss and Interface Rescue)
by booting from eMMC.

At least one board with micron flash is now working with IOFV state {3,3,3,3}.
I need to test more boards to confirm the behaviour.

Cheers,
Biju


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH RFC 0/4] Add set_iofv() callback

[Michael Walle]

Am 2023-11-11 13:26, schrieb Biju Das:
> Hi Michael Walle,
> 
>> Subject: RE: [PATCH RFC 0/4] Add set_iofv() callback
>> 
> 
>> 
>> > Subject: Re: [PATCH RFC 0/4] Add set_iofv() callback
>> >
>> > Hi Biju,
>> >
>> > >> >> Thus I was saying, that we probably wont support that and the
>> > >> >> easiest fix should be to disable this behavior for the atmel
>> > >> >> flash (there was nv setting).
>> > >> >
>> > >> > The fix up is invoked only for quad mode, I believe it is safe to
>> > >> > add fixup for micron flash As it is the one deviating from normal
>> > >> > according to you, rather than adding fixup for generic flash like
>> > >> > ATMEL flash(Now Renesas flash)
>> > >>
>> > >> Could you please try setting bit 4 in the Nonvolatile Configuration
>> > >> Register (Table 7) and see if the problem goes away?
>> > >
>> > > You mean, if it works, we need to disable reset for all the boards,
>> > > maybe at bootloader level??
>> >
>> > Not necessarily. First, just to confirm that it is actually the reset
>> > circuit. You can also compare the part numbers of the flash. There is
>> > a flash with IO3/RESET# and IO3/HOLD# (and a flash with a dedicated
>> > reset pin).
>> 
>> Part is MT25QU512ABB8E12-0SIT, As per the schematic, flash has a 
>> dedicated
>> RESET# with 10K pullup connected to SoC QSPI_RESET pin.
>> 
>> DQ0, DQ1, W#/DQ2 and DQ3 lines on the flash are connected without any
>> pullups to the SoC QSPI0_{0..3} pins.
>> 
>> >
>> > If that's the case, it looks like a hardware bug on your board. You
>> > left the reset pin floating. So you'd also not be able to boot from
>> > the NOR flash, right?
>> 
>> I am booting from NOR flash. BootRom code reads SPI flash and executes
>> BL2.
>> BL2 loads BL33 and U-boot from NOR flash. If this is the case, do you
>> think it is a Hw bug on the board?
>> 
>> >
>> > > OK, I will check that. Currently I have read that register and it is
>> > > showing a value Of 0xffbb. I need to do write operation. Before that
>> > > how do we recover flash, if something goes wrong during writing for
>> > > NV register?
>> >
>> > You should always be able to write that register from the bootloader.
>> > Maybe also through raw commands (like sspi in uboot).
>> 
>> Thanks for the pointer, I haven't explored the uboot path.
> 
> I have disabled RESET# bit in the Nonvolatile Configuration
> Register (Table 7) and borad doesn't boot any more.
> 
> By default that bit is set.
> 
> [    2.530291] ###### Before write Read cmd=b5 val=ff
> [    2.530431] ###### write cmd=b1 val=ef
> [    2.535518] ###### Read cmd=b5 val=ef
> 
> 
> NOTICE:  BL2: Built : 14:59:28, Nov 10 2023
> ERROR:   BL2: Failed to load image id 3 (-2)
> NOTICE:  BL2: v2.9(release):v2.5/rzg2l-1.00-3883-gc314a391c
> NOTICE:  BL2: Built : 14:59:28, Nov 10 2023
> ERROR:   BL2: Failed to load image id 3 (-2)
> NOTICE:  BL2: v2.9(release):v2.5/rzg2l-1.00-3883-gc314a391c
> 
> What is your thoughts on this? How do we proceed now?

I guessed you fixed this? Because.. if you boot from NOR the BL2
should come from the NOR flash too, correct? And that is actually
working.

-michael

Re: [PATCH RFC 0/4] Add set_iofv() callback

[Michael Walle]

Am 2023-11-11 13:26, schrieb Biju Das:
> Hi Michael Walle,
> 
>> Subject: RE: [PATCH RFC 0/4] Add set_iofv() callback
>> 
> 
>> 
>> > Subject: Re: [PATCH RFC 0/4] Add set_iofv() callback
>> >
>> > Hi Biju,
>> >
>> > >> >> Thus I was saying, that we probably wont support that and the
>> > >> >> easiest fix should be to disable this behavior for the atmel
>> > >> >> flash (there was nv setting).
>> > >> >
>> > >> > The fix up is invoked only for quad mode, I believe it is safe to
>> > >> > add fixup for micron flash As it is the one deviating from normal
>> > >> > according to you, rather than adding fixup for generic flash like
>> > >> > ATMEL flash(Now Renesas flash)
>> > >>
>> > >> Could you please try setting bit 4 in the Nonvolatile Configuration
>> > >> Register (Table 7) and see if the problem goes away?
>> > >
>> > > You mean, if it works, we need to disable reset for all the boards,
>> > > maybe at bootloader level??
>> >
>> > Not necessarily. First, just to confirm that it is actually the reset
>> > circuit. You can also compare the part numbers of the flash. There is
>> > a flash with IO3/RESET# and IO3/HOLD# (and a flash with a dedicated
>> > reset pin).
>> 
>> Part is MT25QU512ABB8E12-0SIT, As per the schematic, flash has a 
>> dedicated
>> RESET# with 10K pullup connected to SoC QSPI_RESET pin.
>> 
>> DQ0, DQ1, W#/DQ2 and DQ3 lines on the flash are connected without any
>> pullups to the SoC QSPI0_{0..3} pins.
>> 
>> >
>> > If that's the case, it looks like a hardware bug on your board. You
>> > left the reset pin floating. So you'd also not be able to boot from
>> > the NOR flash, right?
>> 
>> I am booting from NOR flash. BootRom code reads SPI flash and executes
>> BL2.
>> BL2 loads BL33 and U-boot from NOR flash. If this is the case, do you
>> think it is a Hw bug on the board?
>> 
>> >
>> > > OK, I will check that. Currently I have read that register and it is
>> > > showing a value Of 0xffbb. I need to do write operation. Before that
>> > > how do we recover flash, if something goes wrong during writing for
>> > > NV register?
>> >
>> > You should always be able to write that register from the bootloader.
>> > Maybe also through raw commands (like sspi in uboot).
>> 
>> Thanks for the pointer, I haven't explored the uboot path.
> 
> I have disabled RESET# bit in the Nonvolatile Configuration
> Register (Table 7) and borad doesn't boot any more.
> 
> By default that bit is set.
> 
> [    2.530291] ###### Before write Read cmd=b5 val=ff
> [    2.530431] ###### write cmd=b1 val=ef
> [    2.535518] ###### Read cmd=b5 val=ef
> 
> 
> NOTICE:  BL2: Built : 14:59:28, Nov 10 2023
> ERROR:   BL2: Failed to load image id 3 (-2)
> NOTICE:  BL2: v2.9(release):v2.5/rzg2l-1.00-3883-gc314a391c
> NOTICE:  BL2: Built : 14:59:28, Nov 10 2023
> ERROR:   BL2: Failed to load image id 3 (-2)
> NOTICE:  BL2: v2.9(release):v2.5/rzg2l-1.00-3883-gc314a391c
> 
> What is your thoughts on this? How do we proceed now?

I guessed you fixed this? Because.. if you boot from NOR the BL2
should come from the NOR flash too, correct? And that is actually
working.

-michael

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

RE: [PATCH RFC 0/4] Add set_iofv() callback

[Biju Das]

Hi Michael Walle,

> Subject: Re: [PATCH RFC 0/4] Add set_iofv() callback
> 
> Am 2023-11-11 13:26, schrieb Biju Das:
> > Hi Michael Walle,
> >
> >> Subject: RE: [PATCH RFC 0/4] Add set_iofv() callback
> >>
> >
> >>
> >> > Subject: Re: [PATCH RFC 0/4] Add set_iofv() callback
> >> >
> >> > Hi Biju,
> >> >
> >> > >> >> Thus I was saying, that we probably wont support that and the
> >> > >> >> easiest fix should be to disable this behavior for the atmel
> >> > >> >> flash (there was nv setting).
> >> > >> >
> >> > >> > The fix up is invoked only for quad mode, I believe it is safe
> >> > >> > to add fixup for micron flash As it is the one deviating from
> >> > >> > normal according to you, rather than adding fixup for generic
> >> > >> > flash like ATMEL flash(Now Renesas flash)
> >> > >>
> >> > >> Could you please try setting bit 4 in the Nonvolatile
> >> > >> Configuration Register (Table 7) and see if the problem goes away?
> >> > >
> >> > > You mean, if it works, we need to disable reset for all the
> >> > > boards, maybe at bootloader level??
> >> >
> >> > Not necessarily. First, just to confirm that it is actually the
> >> > reset circuit. You can also compare the part numbers of the flash.
> >> > There is a flash with IO3/RESET# and IO3/HOLD# (and a flash with a
> >> > dedicated reset pin).
> >>
> >> Part is MT25QU512ABB8E12-0SIT, As per the schematic, flash has a
> >> dedicated RESET# with 10K pullup connected to SoC QSPI_RESET pin.
> >>
> >> DQ0, DQ1, W#/DQ2 and DQ3 lines on the flash are connected without any
> >> pullups to the SoC QSPI0_{0..3} pins.
> >>
> >> >
> >> > If that's the case, it looks like a hardware bug on your board. You
> >> > left the reset pin floating. So you'd also not be able to boot from
> >> > the NOR flash, right?
> >>
> >> I am booting from NOR flash. BootRom code reads SPI flash and
> >> executes BL2.
> >> BL2 loads BL33 and U-boot from NOR flash. If this is the case, do you
> >> think it is a Hw bug on the board?
> >>
> >> >
> >> > > OK, I will check that. Currently I have read that register and it
> >> > > is showing a value Of 0xffbb. I need to do write operation.
> >> > > Before that how do we recover flash, if something goes wrong
> >> > > during writing for NV register?
> >> >
> >> > You should always be able to write that register from the bootloader.
> >> > Maybe also through raw commands (like sspi in uboot).
> >>
> >> Thanks for the pointer, I haven't explored the uboot path.
> >
> > I have disabled RESET# bit in the Nonvolatile Configuration Register
> > (Table 7) and borad doesn't boot any more.
> >
> > By default that bit is set.
> >
> > [    2.530291] ###### Before write Read cmd=b5 val=ff
> > [    2.530431] ###### write cmd=b1 val=ef
> > [    2.535518] ###### Read cmd=b5 val=ef
> >
> >
> > NOTICE:  BL2: Built : 14:59:28, Nov 10 2023
> > ERROR:   BL2: Failed to load image id 3 (-2)
> > NOTICE:  BL2: v2.9(release):v2.5/rzg2l-1.00-3883-gc314a391c
> > NOTICE:  BL2: Built : 14:59:28, Nov 10 2023
> > ERROR:   BL2: Failed to load image id 3 (-2)
> > NOTICE:  BL2: v2.9(release):v2.5/rzg2l-1.00-3883-gc314a391c
> >
> > What is your thoughts on this? How do we proceed now?
> 
> I guessed you fixed this? Because.. if you boot from NOR the BL2 should
> come from the NOR flash too, correct? And that is actually working.

Yes, it is working.

I updated the NV settings on micron flash on the second RZ/G2L board and 
both boards seems to be working(ie, by disabling RESET# on DQ3) and using IOFV state as hiZ.

I am planning to update NV stings on micron flash for 2 more boards (RZ/G2LC and RZ/V2L).

After that I will send a patch using IOFV {3,3,3,3} for both micron and Adesto flash.

and will request the bootloader team to disable RESET# on DQ3 using NV register.

Cheers,
Biju



______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH RFC 0/4] Add set_iofv() callback

[Michael Walle]

Hi Biju,

>> >> >> Thus I was saying, that we probably wont support that and the
>> >> >> easiest fix should be to disable this behavior for the atmel flash
>> >> >> (there was nv setting).
>> >> >
>> >> > The fix up is invoked only for quad mode, I believe it is safe to
>> >> > add fixup for micron flash As it is the one deviating from normal
>> >> > according to you, rather than adding fixup for generic flash like
>> >> > ATMEL flash(Now Renesas flash)
>> >>
>> >> Could you please try setting bit 4 in the Nonvolatile Configuration
>> >> Register (Table 7) and see if the problem goes away?
>> >
>> > You mean, if it works, we need to disable reset for all the boards,
>> > maybe at bootloader level??
>> 
>> Not necessarily. First, just to confirm that it is actually the reset
>> circuit. You can also compare the part numbers of the flash. There is 
>> a
>> flash with IO3/RESET# and IO3/HOLD# (and a flash with a dedicated 
>> reset
>> pin).
>> 
>> If that's the case, it looks like a hardware bug on your board. You 
>> left
>> the reset pin floating. So you'd also not be able to boot from the NOR
>> flash, right?
>> 
>> > OK, I will check that. Currently I have read that register and it is
>> > showing a value Of 0xffbb. I need to do write operation. Before that
>> > how do we recover flash, if something goes wrong during writing for NV
>> > register?
>> 
>> You should always be able to write that register from the bootloader.
>> Maybe also through raw commands (like sspi in uboot).
> 
> Just an update, now clearing bit4 on Micron flash, I am able to test 
> erase/read/write
> Micron flash with IOFV state {3,3,3,3}. Not sure what went wrong 
> previously.
> only thing I changed is related to enabling the QUAD spi mode by 
> disabling bit 2 and 3 on NV register.

This enables QPI mode, that is the command is also expected to be
transferred over the four IO lines. That's not something linux supports.

We'll always send the command in single bit mode (the only exception is
octal DTR mode).

> This again result in boot failure as boot ROM expects extended SPI 
> mode.
> Then restored the extended SPI mode by sending the command FFh (Power 
> Loss and Interface Rescue)
> by booting from eMMC.
> 
> At least one board with micron flash is now working with IOFV state 
> {3,3,3,3}.
> I need to test more boards to confirm the behaviour.

I'm still trying to make sense of this (and trying to figure out
if we need something or not).

So you have a MT25QU512ABB8E12-0SIT, which according to to Figure 4 and
Figure 5 has a dedicated RESET# line and the IO3 is multiplexed with 
HOLD#.
Thus, it seems the flash will go into hold mode if IO3 is not high 
during
command phase. Wether this is a hardware bug? I'd tend to say yes. As 
with
the RESET# you depend on the software/bootROM whatever to set the state 
of
IO3 correctly. But it might depend on the SPI controller used etc. Maybe
it will already drive IO3 high by default?! (and esp. what the bootROM 
is doing
if that SoC is able to load the first stage bootloader from NOR).

I still see two possible fixes here:
(1) Disable HOLD#, either during board production, in your 
bootloader/TFA
     or by introducing a fixup for this flash in linux.
     (And configure your SPI controller to use IOFV state 3,3,3,3 as that
     should be the sane default).
(2) Introduce a mechanism to spi_mem_op to control the unused bits (as
     explained in an earlier reply). Then somehow integrate that into
     the spi-nor micron driver to set IO3 to this pariticular value for
     this operation.

Alternatively, fix your board to have a weak pull-up on IO3 so the IOFV
state 3,3,3,3 will work.

HTH,
-michael

Re: [PATCH RFC 0/4] Add set_iofv() callback

[Michael Walle]

Hi Biju,

>> >> >> Thus I was saying, that we probably wont support that and the
>> >> >> easiest fix should be to disable this behavior for the atmel flash
>> >> >> (there was nv setting).
>> >> >
>> >> > The fix up is invoked only for quad mode, I believe it is safe to
>> >> > add fixup for micron flash As it is the one deviating from normal
>> >> > according to you, rather than adding fixup for generic flash like
>> >> > ATMEL flash(Now Renesas flash)
>> >>
>> >> Could you please try setting bit 4 in the Nonvolatile Configuration
>> >> Register (Table 7) and see if the problem goes away?
>> >
>> > You mean, if it works, we need to disable reset for all the boards,
>> > maybe at bootloader level??
>> 
>> Not necessarily. First, just to confirm that it is actually the reset
>> circuit. You can also compare the part numbers of the flash. There is 
>> a
>> flash with IO3/RESET# and IO3/HOLD# (and a flash with a dedicated 
>> reset
>> pin).
>> 
>> If that's the case, it looks like a hardware bug on your board. You 
>> left
>> the reset pin floating. So you'd also not be able to boot from the NOR
>> flash, right?
>> 
>> > OK, I will check that. Currently I have read that register and it is
>> > showing a value Of 0xffbb. I need to do write operation. Before that
>> > how do we recover flash, if something goes wrong during writing for NV
>> > register?
>> 
>> You should always be able to write that register from the bootloader.
>> Maybe also through raw commands (like sspi in uboot).
> 
> Just an update, now clearing bit4 on Micron flash, I am able to test 
> erase/read/write
> Micron flash with IOFV state {3,3,3,3}. Not sure what went wrong 
> previously.
> only thing I changed is related to enabling the QUAD spi mode by 
> disabling bit 2 and 3 on NV register.

This enables QPI mode, that is the command is also expected to be
transferred over the four IO lines. That's not something linux supports.

We'll always send the command in single bit mode (the only exception is
octal DTR mode).

> This again result in boot failure as boot ROM expects extended SPI 
> mode.
> Then restored the extended SPI mode by sending the command FFh (Power 
> Loss and Interface Rescue)
> by booting from eMMC.
> 
> At least one board with micron flash is now working with IOFV state 
> {3,3,3,3}.
> I need to test more boards to confirm the behaviour.

I'm still trying to make sense of this (and trying to figure out
if we need something or not).

So you have a MT25QU512ABB8E12-0SIT, which according to to Figure 4 and
Figure 5 has a dedicated RESET# line and the IO3 is multiplexed with 
HOLD#.
Thus, it seems the flash will go into hold mode if IO3 is not high 
during
command phase. Wether this is a hardware bug? I'd tend to say yes. As 
with
the RESET# you depend on the software/bootROM whatever to set the state 
of
IO3 correctly. But it might depend on the SPI controller used etc. Maybe
it will already drive IO3 high by default?! (and esp. what the bootROM 
is doing
if that SoC is able to load the first stage bootloader from NOR).

I still see two possible fixes here:
(1) Disable HOLD#, either during board production, in your 
bootloader/TFA
     or by introducing a fixup for this flash in linux.
     (And configure your SPI controller to use IOFV state 3,3,3,3 as that
     should be the sane default).
(2) Introduce a mechanism to spi_mem_op to control the unused bits (as
     explained in an earlier reply). Then somehow integrate that into
     the spi-nor micron driver to set IO3 to this pariticular value for
     this operation.

Alternatively, fix your board to have a weak pull-up on IO3 so the IOFV
state 3,3,3,3 will work.

HTH,
-michael

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH RFC 0/4] Add set_iofv() callback

[Michael Walle]

> Maybe
> it will already drive IO3 high by default?! (and esp. what the bootROM 
> is doing
> if that SoC is able to load the first stage bootloader from NOR).

I just had another look at your Renesas SoC and indeed, the register 
default is
to set IO3 high if not used. Mh. I still think 3,3,3,3 is the saner 
default.
But I might be wrong. Hard to tell, as the sample size is just Micron 
and Atmel
for now. And it's still unclear to me why the Atmel isn't working with 
3,3,3,1.

-michael

Re: [PATCH RFC 0/4] Add set_iofv() callback

[Michael Walle]

> Maybe
> it will already drive IO3 high by default?! (and esp. what the bootROM 
> is doing
> if that SoC is able to load the first stage bootloader from NOR).

I just had another look at your Renesas SoC and indeed, the register 
default is
to set IO3 high if not used. Mh. I still think 3,3,3,3 is the saner 
default.
But I might be wrong. Hard to tell, as the sample size is just Micron 
and Atmel
for now. And it's still unclear to me why the Atmel isn't working with 
3,3,3,1.

-michael

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH RFC 0/4] Add set_iofv() callback

[Michael Walle]

Hi Biju,

> After that I will send a patch using IOFV {3,3,3,3} for both micron and 
> Adesto flash.

Just to be clear, that will just touch the spi controller as a global 
default, right?
That shouldn't go through spi-nor. Otherwise I'd prefer to use fix (2) 
from my previous
mail.

-michael

Re: [PATCH RFC 0/4] Add set_iofv() callback

[Michael Walle]

Hi Biju,

> After that I will send a patch using IOFV {3,3,3,3} for both micron and 
> Adesto flash.

Just to be clear, that will just touch the spi controller as a global 
default, right?
That shouldn't go through spi-nor. Otherwise I'd prefer to use fix (2) 
from my previous
mail.

-michael

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

RE: [PATCH RFC 0/4] Add set_iofv() callback

[Biju Das]

Hi Michael Walle,

> Subject: Re: [PATCH RFC 0/4] Add set_iofv() callback
> 
> Hi Biju,
> 
> > After that I will send a patch using IOFV {3,3,3,3} for both micron
> > and Adesto flash.
> 
> Just to be clear, that will just touch the spi controller as a global
> default, right?

Yes, it is in SoC specific bus controller driver(driver/memory/renesas-rpc-if.c)

> That shouldn't go through spi-nor. Otherwise I'd prefer to use fix (2)
> from my previous mail.

Agreed. Fix(2) won't work as renesas-rpc-if probe which sets {3,3,3,3} is called before flash detection.
and that will make flash detection to fail. So we cannot use fixup. The only way (2) to work
is to like patch[1].

[1] https://patchwork.kernel.org/project/linux-renesas-soc/patch/20230830145835.296690-1-biju.das.jz@bp.renesas.com/

Cheers,
Biju

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH RFC 0/4] Add set_iofv() callback

[Michael Walle]

Hi Biju,

>> > After that I will send a patch using IOFV {3,3,3,3} for both micron
>> > and Adesto flash.
>> 
>> Just to be clear, that will just touch the spi controller as a global
>> default, right?
> 
> Yes, it is in SoC specific bus controller 
> driver(driver/memory/renesas-rpc-if.c)
> 
>> That shouldn't go through spi-nor. Otherwise I'd prefer to use fix (2)
>> from my previous mail.
> 
> Agreed. Fix(2) won't work as renesas-rpc-if probe which sets {3,3,3,3} 
> is called before flash detection.
> and that will make flash detection to fail. So we cannot use fixup. The 
> only way (2) to work
> is to like patch[1].

Ohh I see. Makes sense. Can you ask your SoC engineers, why they
choose the IO3 default to high? I'd guess because it's usually
shared with HOLD# or RESET#. But that really begs the question
why the Atmel flash isn't working with that setting. I suspect
some problems during the turn around of the direction of IO3.
You'd really have to probe with an oscilloscope though.

-michael

Re: [PATCH RFC 0/4] Add set_iofv() callback

[Michael Walle]

Hi Biju,

>> > After that I will send a patch using IOFV {3,3,3,3} for both micron
>> > and Adesto flash.
>> 
>> Just to be clear, that will just touch the spi controller as a global
>> default, right?
> 
> Yes, it is in SoC specific bus controller 
> driver(driver/memory/renesas-rpc-if.c)
> 
>> That shouldn't go through spi-nor. Otherwise I'd prefer to use fix (2)
>> from my previous mail.
> 
> Agreed. Fix(2) won't work as renesas-rpc-if probe which sets {3,3,3,3} 
> is called before flash detection.
> and that will make flash detection to fail. So we cannot use fixup. The 
> only way (2) to work
> is to like patch[1].

Ohh I see. Makes sense. Can you ask your SoC engineers, why they
choose the IO3 default to high? I'd guess because it's usually
shared with HOLD# or RESET#. But that really begs the question
why the Atmel flash isn't working with that setting. I suspect
some problems during the turn around of the direction of IO3.
You'd really have to probe with an oscilloscope though.

-michael

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

RE: [PATCH RFC 0/4] Add set_iofv() callback

[Biju Das]

Hi Michael Walle,

> Subject: Re: [PATCH RFC 0/4] Add set_iofv() callback
> 
> Hi Biju,
> 
> >> > After that I will send a patch using IOFV {3,3,3,3} for both micron
> >> > and Adesto flash.
> >>
> >> Just to be clear, that will just touch the spi controller as a global
> >> default, right?
> >
> > Yes, it is in SoC specific bus controller
> > driver(driver/memory/renesas-rpc-if.c)
> >
> >> That shouldn't go through spi-nor. Otherwise I'd prefer to use fix
> >> (2) from my previous mail.
> >
> > Agreed. Fix(2) won't work as renesas-rpc-if probe which sets {3,3,3,3}
> > is called before flash detection.
> > and that will make flash detection to fail. So we cannot use fixup.
> > The only way (2) to work is to like patch[1].
> 
> Ohh I see. Makes sense. Can you ask your SoC engineers, why they choose
> the IO3 default to high? I'd guess because it's usually shared with HOLD#
> or RESET#. 

OK will ask and will update you once I get feedback.

>But that really begs the question why the Atmel flash isn't
> working with that setting. I suspect some problems during the turn around
> of the direction of IO3.
> You'd really have to probe with an oscilloscope though.

OK, but as per [1], 8.14, IO3 must be HiZ for Atmel flash.

[1] https://www.renesas.com/eu/en/document/dst/at25ql128a-datasheet?r=1608586

Cheers,
Biju


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH RFC 0/4] Add set_iofv() callback

[Michael Walle]

Hi Biju,

>> But that really begs the question why the Atmel flash isn't
>> working with that setting. I suspect some problems during the turn 
>> around
>> of the direction of IO3.
>> You'd really have to probe with an oscilloscope though.
> 
> OK, but as per [1], 8.14, IO3 must be HiZ for Atmel flash.

Sure, but the question is *why*? The flash shouldn't drive IO3
during the command phase. Also, because it might be in HiZ it
cannot read the state (as it is undefined at this point). So
what is going wrong here?

As mentioned, I suspect something is going wrong during the
change of direction of the IO3 line. Either the SoC is driving
it for too long, or the flash is driving it too early?

-michael

Re: [PATCH RFC 0/4] Add set_iofv() callback

[Michael Walle]

Hi Biju,

>> But that really begs the question why the Atmel flash isn't
>> working with that setting. I suspect some problems during the turn 
>> around
>> of the direction of IO3.
>> You'd really have to probe with an oscilloscope though.
> 
> OK, but as per [1], 8.14, IO3 must be HiZ for Atmel flash.

Sure, but the question is *why*? The flash shouldn't drive IO3
during the command phase. Also, because it might be in HiZ it
cannot read the state (as it is undefined at this point). So
what is going wrong here?

As mentioned, I suspect something is going wrong during the
change of direction of the IO3 line. Either the SoC is driving
it for too long, or the flash is driving it too early?

-michael

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH RFC 3/4] memory: renesas-rpc-if: Add support for overriding IO fixed values

[Krzysztof Kozlowski]

On 08/11/2023 18:11, Biju Das wrote:
> Add support for overriding IO fixed values to control the pin state
> based on the flash type.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.c


> +
> +	regmap_update_bits(rpc->regmap, RPCIF_CMNCR, RPCIF_CMNCR_IOFV(3),
> +			   RPCIF_CMNCR_IO0FV(val & 0x3) |
> +			   RPCIF_CMNCR_IO2FV((val >> 4) & 0x3) |
> +			   RPCIF_CMNCR_IO3FV((val >> 6) & 0x3));
> +
> +	pm_runtime_put(dev);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(rpcif_set_iofv);

EXPORT_SYMBOL+GPL

Best regards,
Krzysztof