[U-Boot] saveenv corrupts QSPI flash with latest commit U-Boot 2019.04-rc4-00047-gcfb3e102c4

[Ashish Kumar]

> -----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.
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);
}

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