-----Original Message----- From: Vignesh Raghavendra vigneshr@ti.com Sent: Friday, March 29, 2019 12:17 PM To: Ashish Kumar ashish.kumar@nxp.com; Jagan Teki jagan@openedev.com; u-boot@lists.denx.de Cc: Kuldeep Singh kuldeep.singh@nxp.com Subject: Re: saveenv corrupts QSPI flash with latest commit U-Boot 2019.04-rc4- 00047-gcfb3e102c4
On 28/03/19 3:37 PM, Ashish Kumar wrote:
-----Original Message----- From: Vignesh Raghavendra vigneshr@ti.com Sent: Wednesday, March 27, 2019 9:44 AM To: Ashish Kumar ashish.kumar@nxp.com; Jagan Teki jagan@openedev.com; u-boot@lists.denx.de; Tom Rini trini@konsulko.com Cc: Kuldeep Singh kuldeep.singh@nxp.com Subject: Re: saveenv corrupts QSPI flash with latest commit U-Boot 2019.04-rc4- 00047-gcfb3e102c4
On 26/03/19 7:11 PM, Ashish Kumar wrote:
On 26/03/19 10:36 AM, Ashish Kumar wrote:
Hello Maintainers,
With latest commit I see that saveenv command corrupts QSPI-flash, meaning if I read QSPI-flash at 0x0 offset RCW(reset configuration word) is erased after saveenv command was executed.
This is tested on LS1088ARDB, but it should not be limited to LS1088ARDB rather it will valid for all LS Freescale ARM boards.( like LS1088, LS2080/LS2085, LS1012, LS1043, LS1046).
Which is the SPI controller driver? Does the controller driver support reading/writing to flash using 4 byte addressing opcode?
fsl_qspi.c. As per the migration it was not migrated to 4 byte. Although it
actually supports 4 byte for some SoC and 3 byte for older SoC that is the reason you see CONFIG_SPI_FLASH_BAR in code. My concerned SoC are all supporting 4 byte. I have not added any code modification in the current u-boot.
OK, does normal read/write to offset 0 work fine? That is:
sf probe sf erase 0x0 40000 sf write <add1r> 0x0 0x40000 (write some random data) sf read <addr2> 0x0 0x40000 (read back) md.b <addr2>
If this fails. Could you post full debug log (all debug prints enabled in spi_mem_exec_op()) to pastebin.ubuntu.com and provide a link here?
Hi Vignesh, This is working now(READ, WRITE), after some change in fsl_qspi driver, where
I check for 4byte op codes now.
Hmm, I don't expect any 4 byte opcode to be used when SPI_FLASH_BAR is enabled. Could you what 4byte opcodes are being used and share the changes here?
Since SPI_FLASH_BAR and set_4byte are inversely related, I had disable this SPI_FLASH_BAR via defconfig, and Set this 4B_OP_CODE in SPANSION flash ids. I will post my patch in upstream. Once I test the new flow of erase as suggested below.
But now I see that erase is getting address as ZERO. Which in my opinion is because spi_nor_erase_sector() call write_reg which has SPI_MEM_OP_NO_ADDR?
/*
- Initiate the erasure of a single sector */ static int
spi_nor_erase_sector(struct spi_nor *nor, u32 addr) { u8 buf[SPI_NOR_MAX_ADDR_WIDTH]; int i;
if (nor->erase) return nor->erase(nor, addr); /* * Default implementation, if driver doesn't have a specialized HW * control */ for (i = nor->addr_width - 1; i >= 0; i--) { buf[i] = addr & 0xff; addr >>= 8; } return nor->write_reg(nor, nor->erase_opcode, buf,nor->addr_width); }
static int spi_nor_write_reg(struct spi_nor *nor, u8 opcode, u8 *buf, int len) { struct spi_mem_op op = SPI_MEM_OP(SPI_MEM_OP_CMD(opcode, 1), SPI_MEM_OP_NO_ADDR, SPI_MEM_OP_NO_DUMMY, SPI_MEM_OP_DATA_OUT(len, NULL, 1));
return spi_nor_read_write_reg(nor, &op, buf); }The address of the sector to erased is in "buf".
Ok. So the address is in second xfer call, i.e. " SPI_XFER_END "
And rationale in using nor->write_reg() for erase is that, in case of controllers that support memory mapped access to flash, read/write is done via MMIO interface whereas erase is using done via register/FIFO interface.
Could you please explain meaning of register/FIFO interface ? how erase is different from write?
But, problem is at driver that tries to interpret spi_xfer() to decode messages into cmd+addr+dummy+data format. With moving to spi-mem layer, erase sector cmd+addr+dummy+address is now passed as part of message that has SPI_XFER_END (instead of SPI_XFER_BEGIN) flag.
fsl_qspi.c driver should be able to handle this similar to register write commands such as Bank Register write etc.
Something along the line of below diff(untested and just an indicative fix) this:
diff --git a/drivers/spi/fsl_qspi.c b/drivers/spi/fsl_qspi.c index 1598c4f69890..1fd12b8728d3 100644 --- a/drivers/spi/fsl_qspi.c +++ b/drivers/spi/fsl_qspi.c @@ -783,18 +783,21 @@ int qspi_xfer(struct fsl_qspi_priv *priv, unsigned int bitlen, }
if (flags == SPI_XFER_END) {
priv->sf_addr = wr_sfaddr;qspi_op_write(priv, (u8 *)dout, bytes);
if ((priv->cur_seqid == QSPI_CMD_SE) ||(priv->cur_seqid == QSPI_CMD_BE_4K)) {/* DO: populate priv->sf_addr with addr from* dout */qspi_op_erase(priv);} else {priv->sf_addr = wr_sfaddr;qspi_op_write(priv, (u8 *)dout, bytes);}
Yes something similar should work.
return 0; } if (priv->cur_seqid == QSPI_CMD_FAST_READ || priv->cur_seqid == QSPI_CMD_RDAR) { priv->sf_addr = swab32(txbuf) & OFFSET_BITS_MASK;
} else if ((priv->cur_seqid == QSPI_CMD_SE) ||(priv->cur_seqid == QSPI_CMD_BE_4K)) {priv->sf_addr = swab32(txbuf) & OFFSET_BITS_MASK;qspi_op_erase(priv); } else if (priv->cur_seqid == QSPI_CMD_PP || priv->cur_seqid == QSPI_CMD_WRAR) { wr_sfaddr = swab32(txbuf) & OFFSET_BITS_MASKHonestly, fsl_qspi.c is almost a parallel spi-nor-core framework and should be moved to spi-mem as soon as possible. Otherwise any simple change to spi-nor-core will break the driver.
First I will post this these changes that will unblock fsl_qspi hopefully. Next step would be to migrated to mem_ops.
Regards Ashish
Regards Vignesh
Regards Ashish
Could you enable all the debug prints in spi_mem_exec_op() (especially those at the end)?
Logs are very verbose: I have commented two debug logs which are in for loop Logs here are from the end, 0b 33 f3 43 | [59B in] [ret 0]
I dont know the address from which saveenv is trying to read from. But does the address bytes match (33 f3 43)?
[...]
0b 33 ff f0 | [16B in] [ret 0] Erasing SPI flash...06 | [0B -] [ret 0] d8 | [3B out] [ret 0]
This is the sector erase command but you haven't enabled log for the part where address of the sector is dumped. Could you provide that info?
Also, the commit ID in the subject is not present in upstream tree. So I am not sure what exactly maybe broken. Does 2019.01 work fine? U-Boot had major sf layer refactoring in 2019.04-rc1. Could you see if 2019.04-rc1 works fine?
-- Regards Vignesh
-- Regards Vignesh