Hi Stefan
-----Original Message----- From: Stefan Roese sr@denx.de Sent: Tuesday, October 23, 2018 10:31 PM To: Rajat Srivastava rajat.srivastava@nxp.com; jagan@openedev.com; simon.k.r.goldschmidt@gmail.com Cc: Ashish Kumar ashish.kumar@nxp.com; u-boot@lists.denx.de Subject: Re: [U-Boot] [PATCH v3] sf: Add auto detection of 4-byte mode (vs standard 3-byte mode)
Hi Rajat,
On 23.10.18 07:17, Rajat Srivastava wrote:
-----Original Message----- From: Stefan Roese [mailto:sr@denx.de] Sent: Monday, October 22, 2018 12:45 PM To: Rajat Srivastava mailto:rajat.srivastava@nxp.com; mailto:jagan@openedev.com; mailto:simon.k.r.goldschmidt@gmail.com Cc: Ashish Kumar mailto:ashish.kumar@nxp.com; mailto:u-boot@lists.denx.de Subject: Re: [U-Boot] [PATCH v3] sf: Add auto detection of 4-byte mode (vs standard 3-byte mode)
Hi Rajat,
On 17.10.18 13:52, Rajat Srivastava wrote:
Hi Stefan
Sorry for top-posting.
Why can't we read SFDP parameters from flash and auto-detect 3-byte/4-byte addressing mode? Using address width information we can support both types of flash i.e. flashes supporting 3-byte addressing mode as well as flashes supporting 4-byte addressing mode.
Our flash supports 3- and 4-byte addressing mode. But this special chip is factory strapped to only support 4-byte mode, even though its device ID tells us that it should support also 3-byte mode. This current pretty simple patch enables the use of this flash with very limited code additions. It also helps others (Simon on SoCFPGA) with their issues regarding 3-byte vs 4-byte mode - especially in regard to the bootrom and its setup.
If you look into my patch, for the flashes that support both 3-byte and 4-byte addressing modes, the default addressing mode is set to 4-byte. In such case if the user wants to send a command in 3-byte mode then he has to set a flag. So SFDP path will be able to handle the special chip that is factory strapped to 4-byte addressing mode.
Code snippet from patch which handles 3-byte/4-byte/both addressing modes:
- switch (bfpt.dwords[BFPT_DWORD(1)] &
BFPT_DWORD1_ADDRESS_BYTES_MASK) {
case BFPT_DWORD1_ADDRESS_BYTES_3_ONLY: // flashes with 3-byteaddressing
flash->addr_width = 3;break;case BFPT_DWORD1_ADDRESS_BYTES_3_OR_4: // flashes with both 3 or4 byte
printf("SF: Flash defaults to 3-Byte mode; enters 4-Byte ");printf("mode on command\n");/** By default, 4-byte addressing mode is set.* To enforce 3-byte addressing mode, set addrwd_3_in_use flag* in struct spi_flash for every command.*/flash->addr_width = 4;break;case BFPT_DWORD1_ADDRESS_BYTES_4_ONLY: // flashes with 4-byteaddressing
flash->addr_width = 4;break;I've floated a similar patch in U-boot that reads and parses SFDP parameters from flash and auto-detects its addressing mode. It send commands according to the address width it detects. Please find the patch set at:
https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatch
work.ozlabs.org%2Fcover%2F985326%2F&data=02%7C01%7Crajat.srivast
ava%40nxp.com%7C56b81d26a6954324bebd08d637ee0c35%7C686ea1d3bc2b4
c6fa92cd99c5c301635%7C0%7C0%7C636757892882342025&sdata=M2aU
WUxSn9wmlBlYj336%2Bay5rwOddG%2Br7Qn5kH%2Bf1uw%3D&reserved=
0
https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatch
work.ozlabs.org%2Fpatch%2F985327%2F&data=02%7C01%7Crajat.srivast
ava%40nxp.com%7C56b81d26a6954324bebd08d637ee0c35%7C686ea1d3bc2b4
c6fa92cd99c5c301635%7C0%7C0%7C636757892882342025&sdata=IIzUJuI
9nL5Wn7K5uAqjig9edpW6YIIcSOExNJNB5qE%3D&reserved=0
https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatch
work.ozlabs.org%2Fpatch%2F985329%2F&data=02%7C01%7Crajat.srivast
ava%40nxp.com%7C56b81d26a6954324bebd08d637ee0c35%7C686ea1d3bc2b4
c6fa92cd99c5c301635%7C0%7C0%7C636757892882342025&sdata=N5qQJ
E1776Wb3siJApPDCkUyY4vn0ZVLjCebn4hi6bk%3D&reserved=0
https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatch
work.ozlabs.org%2Fpatch%2F985328%2F&data=02%7C01%7Crajat.srivast
ava%40nxp.com%7C56b81d26a6954324bebd08d637ee0c35%7C686ea1d3bc2b4
c6fa92cd99c5c301635%7C0%7C0%7C636757892882342025&sdata=tC%2F
%2FsGVwV%2FrHBPX1gJ5TNYmVnJOL13XpAjgP87w3%2Bx0%3D&reserved
=0
I've just applied your 3 patches and have added SFDP support for our equipped SPI chip (with my patch not applied):
{"mx25l25635f", INFO(0xc22019, 0x0, 64 * 1024, 512, RD_FULL |WR_QPP) },
{"mx25l25635f", INFO(0xc22019, 0x0, 64 * 1024, 512, RD_FULL |WR_QPP | SPI_FLASH_USE_SFDP) },
This does not seem to work though:
Simply adding SPI_FLASH_USE_SFDP flag is not enough to make SFDP work.
You'll
need to add the driver code corresponding to the mtd layer code (in
spi_flash.c)
which will send the actual READ SFDP command to flash.
The patch-set I floated adds driver code in fsl_qspi.c (Freescale/NXP QSPI
driver).
Please find the patch at
https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork....
After the mtd layer calls spi_flash_read_common() function to send any read command to flash, it lands on driver which ultimately fires the command (in this case 0x5A command to read SFDP) to flash.
So you say, that each SPI driver needs to get some changes to support the SFDP reading? That does not sound like a generic approach to me. But maybe I misunderstood this.
To read SFDP parameters, a READ_SFDP command (0x5A) needs to be sent to flash which can be sent only with help of a SPI driver. Even if one wants to initiate a simple basic 1-byte read operation, there must exist a SPI driver that actually sends the read command to flash.
I think this is a generic approach because that is how mtd framework is designed.
Since you say the flash is designed to support only 4-byte addressing mode
No. The flash itself (as you have seen in the datasheet) supports both. But the special chip version equipped on the LinkIt MT7688 boards is somehow strapped to only support 4-byte mode. This is not documented anywhere (and did cost me quite some time to figure it out).
Ok I get it. I think this flash does not follow standard SFDP framework. Can you please confirm?
then it is possible that the READ_SFDP command (0x5A) is also required to be sent in 4-byte mode (My patch sends 0x5A in 3-byte addressing mode which is also the SFDP standard that every other flash supports). Although I looked up the datasheet of mx25l25635f and it says that the READ_SFDP command will be sent in 3-byte mode (as supported by my patch).
SF: Detected mx25l25635f with page size 0 Bytes, erase size 64 KiB, total 0
Bytes
*** Warning - bad CRC, using default environment
Please note that I'm not opposed to using your SFDP support. But in our case it does not work - at least not without the SPI_FLASH_USE_SFDP addition. And it needs some fixing as well to fully detect the chip parameters. So I would prefer to go ahead with my patch for now.
My point is that if your case can be handled by adding a generic code, instead of flash specific code then we should prefer the generic approach,
what
do you say?
Please see above, my questions about SPI driver additions for this SFDP support. This does not sound very "generic" to me.
As mentioned above, this is the generic approach and this is how Linux has also implemented SFDP. By design, the SPI driver should adapt to changes in mtd framework.
But I think both solutions, your SFDP support and my "simple" pre-configured 3-byte vs 4-byte addressing mode detection and usage can co-exist.
Ok. Let us wait for the SPI maintainer to comment on this since for both solutions to co-exist we may have to resolve code conflicts.
Thanks, Stefan