[U-Boot] saveenv corrupts QSPI flash with latest commit U-Boot 2019.04-rc4-00047-gcfb3e102c4
Vignesh Raghavendra
vigneshr at ti.com
Fri Mar 29 06:46:58 UTC 2019
On 28/03/19 3:37 PM, Ashish Kumar wrote:
>> -----Original Message-----
>> From: Vignesh Raghavendra <vigneshr at ti.com>
>> Sent: Wednesday, March 27, 2019 9:44 AM
>> To: Ashish Kumar <ashish.kumar at nxp.com>; Jagan Teki
>> <jagan at openedev.com>; u-boot at lists.denx.de; Tom Rini
>> <trini at konsulko.com>
>> Cc: Kuldeep Singh <kuldeep.singh at 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?
> 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".
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.
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 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);
+ }
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_MASK
Honestly, 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.
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
More information about the U-Boot
mailing list