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

From: Biju Das <biju.das.jz@bp.renesas.com>
To: Michael Walle <michael@walle.cc>
Cc: Mark Brown <broonie@kernel.org>,
	Miquel Raynal <miquel.raynal@bootlin.com>,
	Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
	"linux-spi@vger.kernel.org" <linux-spi@vger.kernel.org>,
	"linux-mtd@lists.infradead.org" <linux-mtd@lists.infradead.org>,
	Geert Uytterhoeven <geert+renesas@glider.be>,
	Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@bp.renesas.com>,
	biju.das.au <biju.das.au@gmail.com>,
	"linux-renesas-soc@vger.kernel.org"
	<linux-renesas-soc@vger.kernel.org>
Subject: RE: [PATCH RFC 0/4] Add set_iofv() callback
Date: Fri, 10 Nov 2023 11:35:25 +0000	[thread overview]
Message-ID: <TYCPR01MB11269C639CB7AA480E388360B86AEA@TYCPR01MB11269.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <dcfa2cab21fc85bb9b2b0c1ceb754a1a@walle.cc>

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/
