Hi Jagan > -----Original Message----- > From: Rajat Srivastava > Sent: Thursday, October 25, 2018 2:59 PM > To: 'Stefan Roese' <s...@denx.de>; ja...@openedev.com; > simon.k.r.goldschm...@gmail.com > Cc: Ashish Kumar <ashish.ku...@nxp.com>; u-boot@lists.denx.de; York Sun > <york....@nxp.com> > Subject: RE: [U-Boot] [PATCH v3] sf: Add auto detection of 4-byte mode (vs > standard 3-byte mode) > > Hi Stefan > > > -----Original Message----- > > From: Stefan Roese <s...@denx.de> > > Sent: Tuesday, October 23, 2018 10:31 PM > > To: Rajat Srivastava <rajat.srivast...@nxp.com>; ja...@openedev.com; > > simon.k.r.goldschm...@gmail.com > > Cc: Ashish Kumar <ashish.ku...@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:s...@denx.de] > > >> Sent: Monday, October 22, 2018 12:45 PM > > >> To: Rajat Srivastava <mailto:rajat.srivast...@nxp.com>; > > >> mailto:ja...@openedev.com; mailto:simon.k.r.goldschm...@gmail.com > > >> Cc: Ashish Kumar <mailto:ashish.ku...@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-byte > > addressing > > > + flash->addr_width = 3; > > > + break; > > > + case BFPT_DWORD1_ADDRESS_BYTES_3_OR_4: // flashes with both > > > + 3 or > > 4 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-byte > > addressing > > > + 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%2Fpat > > ch > > >> > > > work.ozlabs.org%2Fcover%2F985326%2F&data=02%7C01%7Crajat.srivas > t > > >> > > > ava%40nxp.com%7C56b81d26a6954324bebd08d637ee0c35%7C686ea1d3bc2b > 4 > > >> > > > c6fa92cd99c5c301635%7C0%7C0%7C636757892882342025&sdata=M2aU > > >> > > > WUxSn9wmlBlYj336%2Bay5rwOddG%2Br7Qn5kH%2Bf1uw%3D&reserv > ed= > > >> 0 > > >>> > > >> > > > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpat > > ch > > >> > > > work.ozlabs.org%2Fpatch%2F985327%2F&data=02%7C01%7Crajat.srivas > t > > >> > > > ava%40nxp.com%7C56b81d26a6954324bebd08d637ee0c35%7C686ea1d3bc2b > 4 > > >> > > > c6fa92cd99c5c301635%7C0%7C0%7C636757892882342025&sdata=IIzUJuI > > >> 9nL5Wn7K5uAqjig9edpW6YIIcSOExNJNB5qE%3D&reserved=0 > > >>> > > >> > > > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpat > > ch > > >> > > > work.ozlabs.org%2Fpatch%2F985329%2F&data=02%7C01%7Crajat.srivas > t > > >> > > > ava%40nxp.com%7C56b81d26a6954324bebd08d637ee0c35%7C686ea1d3bc2b > 4 > > >> > > > c6fa92cd99c5c301635%7C0%7C0%7C636757892882342025&sdata=N5qQJ > > >> E1776Wb3siJApPDCkUyY4vn0ZVLjCebn4hi6bk%3D&reserved=0 > > >>> > > >> > > > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpat > > ch > > >> > > > work.ozlabs.org%2Fpatch%2F985328%2F&data=02%7C01%7Crajat.srivas > t > > >> > > > ava%40nxp.com%7C56b81d26a6954324bebd08d637ee0c35%7C686ea1d3bc2b > 4 > > >> > > > c6fa92cd99c5c301635%7C0%7C0%7C636757892882342025&sdata=tC%2F > > >> > > > %2FsGVwV%2FrHBPX1gJ5TNYmVnJOL13XpAjgP87w3%2Bx0%3D&reserv > ed > > >> =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%2Fpat > chwork.ozlabs.org%2Fpatch%2F985329&data=02%7C01%7Crajat.srivast > ava%40nxp.com%7Cc967de2c68464c6561b508d639091e21%7C686ea1d3bc2b > 4c6fa92cd99c5c301635%7C0%7C0%7C636759108670133878&sdata=uZfP5 > lWvGmpA%2Fxi5AS06c15PKqYwwndgU%2BC3zYu2K3w%3D&reserved= > 0. > > > > > > 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.
Are you ok with the decision on co-existence of both methods? Thanks Rajat > > > > > Thanks, > > Stefan _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot