[U-Boot] [PATCH v3] sf: Add auto detection of 4-byte mode (vs standard 3-byte mode)
Rajat Srivastava
rajat.srivastava at nxp.com
Fri Oct 26 10:03:03 UTC 2018
Hi Jagan
> -----Original Message-----
> From: Rajat Srivastava
> Sent: Thursday, October 25, 2018 2:59 PM
> To: 'Stefan Roese' <sr at denx.de>; jagan at openedev.com;
> simon.k.r.goldschmidt at gmail.com
> Cc: Ashish Kumar <ashish.kumar at nxp.com>; u-boot at lists.denx.de; York Sun
> <york.sun at 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 <sr at denx.de>
> > Sent: Tuesday, October 23, 2018 10:31 PM
> > To: Rajat Srivastava <rajat.srivastava at nxp.com>; jagan at openedev.com;
> > simon.k.r.goldschmidt at gmail.com
> > Cc: Ashish Kumar <ashish.kumar at nxp.com>; u-boot at 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 at denx.de]
> > >> Sent: Monday, October 22, 2018 12:45 PM
> > >> To: Rajat Srivastava <mailto:rajat.srivastava at nxp.com>;
> > >> mailto:jagan at openedev.com; mailto:simon.k.r.goldschmidt at gmail.com
> > >> Cc: Ashish Kumar <mailto:ashish.kumar at nxp.com>;
> > >> mailto:u-boot at 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
More information about the U-Boot
mailing list