Hello Jagan, > -----Original Message----- > From: Jagan Teki <ja...@amarulasolutions.com> > Sent: Monday, April 6, 2020 9:30 PM > To: Sagar Kadam <sagar.ka...@sifive.com> > Cc: Bin Meng <bmeng...@gmail.com>; Palmer Dabbelt > <pal...@sifive.com>; U-Boot Mailing List <u-boot@lists.denx.de>; linux- > amarula <linux-amar...@amarulasolutions.com> > Subject: Re: [U-Boot] [PATCH v2 5/5] sifive: fu540: Enable spi-nor flash > support > > [External Email] Do not click links or attachments unless you recognize the > sender and know the content is safe > > Hi Sagar, > > On Sun, Apr 5, 2020 at 12:29 AM Sagar Kadam <sagar.ka...@sifive.com> > wrote: > > > > Hello Jagan, > > > > > -----Original Message----- > > > From: Jagan Teki <ja...@amarulasolutions.com> > > > Sent: Saturday, April 4, 2020 11:45 PM > > > To: Sagar Kadam <sagar.ka...@sifive.com> > > > Cc: Bin Meng <bmeng...@gmail.com>; Palmer Dabbelt > > > <pal...@sifive.com>; U-Boot Mailing List <u-boot@lists.denx.de>; > linux- > > > amarula <linux-amar...@amarulasolutions.com> > > > Subject: Re: [U-Boot] [PATCH v2 5/5] sifive: fu540: Enable spi-nor flash > > > support > > > > > > [External Email] Do not click links or attachments unless you recognize > the > > > sender and know the content is safe > > > > > > Hi Sagar, > > > > > > On Mon, Nov 18, 2019 at 2:29 AM Sagar Kadam > <sagar.ka...@sifive.com> > > > wrote: > > > > > > > > > > > > Hello Jagan/Bin, > > > > > > > > > -----Original Message----- > > > > > From: U-Boot <u-boot-boun...@lists.denx.de> On Behalf Of Bin > Meng > > > > > Sent: Monday, November 11, 2019 8:02 PM > > > > > To: Jagan Teki <ja...@amarulasolutions.com> > > > > > Cc: Palmer Dabbelt ( Sifive) <pal...@sifive.com>; U-Boot Mailing > List > > > <u- > > > > > b...@lists.denx.de>; linux-amarula <linux- > > > amar...@amarulasolutions.com> > > > > > Subject: Re: [U-Boot] [PATCH v2 5/5] sifive: fu540: Enable spi-nor > flash > > > > > support > > > > > > > > > > Hi Jagan, > > > > > > > > > > On Sat, Nov 9, 2019 at 7:57 PM Jagan Teki > > > <ja...@amarulasolutions.com> > > > > > wrote: > > > > > > > > > > > > Hi Bin, > > > > > > > > > > > > On Tue, Oct 29, 2019 at 3:50 PM Bin Meng <bmeng...@gmail.com> > > > wrote: > > > > > > > > > > > > > > Hi Jagan, > > > > > > > > > > > > > > On Tue, Oct 29, 2019 at 5:38 PM Bin Meng > <bmeng...@gmail.com> > > > > > wrote: > > > > > > > > > > > > > > > > Hi Jagan, > > > > > > > > > > > > > > > > On Wed, Oct 16, 2019 at 10:58 PM Jagan Teki > > > > > <ja...@amarulasolutions.com> wrote: > > > > > > > > > > > > > > > > > > HiFive Unleashed A00 support is25wp256 spi-nor flash, > > > > > > > > > So enable the same and add test result log for future > > > > > > > > > reference. > > > > > > > > > > > > > > > > > > Tested on SiFive FU540 board. > > > > > > > > > > > > > > > > > > Signed-off-by: Jagan Teki <ja...@amarulasolutions.com> > > > > > > > > > Reviewed-by: Bin Meng <bmeng...@gmail.com> > > > > > > > > > Tested-by: Bin Meng <bmeng...@gmail.com> > > > > > > > > > --- > > > > > > > > > .../dts/hifive-unleashed-a00-u-boot.dtsi | 1 + > > > > > > > > > board/sifive/fu540/Kconfig | 3 +++ > > > > > > > > > doc/board/sifive/fu540.rst | 19 > > > +++++++++++++++++++ > > > > > > > > > 3 files changed, 23 insertions(+) > > > > > > > > > > > > > > > > > > diff --git a/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi > > > > > b/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi > > > > > > > > > index 25ec8265a5..d7a64134db 100644 > > > > > > > > > --- a/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi > > > > > > > > > +++ b/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi > > > > > > > > > @@ -5,6 +5,7 @@ > > > > > > > > > > > > > > > > > > / { > > > > > > > > > aliases { > > > > > > > > > + spi0 = &qspi0; > > > > > > > > > spi2 = &qspi2; > > > > > > > > > }; > > > > > > > > > }; > > > > > > > > > diff --git a/board/sifive/fu540/Kconfig > > > b/board/sifive/fu540/Kconfig > > > > > > > > > index 5d65080429..c5a1bca03c 100644 > > > > > > > > > --- a/board/sifive/fu540/Kconfig > > > > > > > > > +++ b/board/sifive/fu540/Kconfig > > > > > > > > > @@ -26,6 +26,7 @@ config BOARD_SPECIFIC_OPTIONS # > dummy > > > > > > > > > imply CMD_FS_GENERIC > > > > > > > > > imply CMD_NET > > > > > > > > > imply CMD_PING > > > > > > > > > + imply CMD_SF > > > > > > > > > imply CLK_SIFIVE > > > > > > > > > imply CLK_SIFIVE_FU540_PRCI > > > > > > > > > imply DOS_PARTITION > > > > > > > > > @@ -40,6 +41,8 @@ config BOARD_SPECIFIC_OPTIONS # > dummy > > > > > > > > > imply SIFIVE_SERIAL > > > > > > > > > imply SPI > > > > > > > > > imply SPI_SIFIVE > > > > > > > > > + imply SPI_FLASH > > > > > > > > > + imply SPI_FLASH_ISSI > > > > > > > > > imply MMC > > > > > > > > > imply MMC_SPI > > > > > > > > > imply MMC_BROKEN_CD > > > > > > > > > diff --git a/doc/board/sifive/fu540.rst > > > b/doc/board/sifive/fu540.rst > > > > > > > > > index 91b94ee06f..2e70cad02e 100644 > > > > > > > > > --- a/doc/board/sifive/fu540.rst > > > > > > > > > +++ b/doc/board/sifive/fu540.rst > > > > > > > > > @@ -366,3 +366,22 @@ load uImage. > > > > > > > > > > > > > > > > > > Please press Enter to activate this console. > > > > > > > > > / # > > > > > > > > > + > > > > > > > > > +Sample spi nor flash test > > > > > > > > > +------------------------- > > > > > > > > > + > > > > > > > > > +.. code-block:: none > > > > > > > > > + > > > > > > > > > + => sf probe 0:2 > > > > > > > > > > > > > > > > The cs number can't be 2. It should be zero. > > > > > > > > > > > > > > With this patch series, we got crazy duplicated flash devices > created, > > > > > > > see below: > > > > > > > > > > > > > > => sf probe > > > > > > > unrecognized JEDEC id bytes: ff, ff, ff > > > > > > > Failed to initialize SPI flash at 0:0 (error -2) > > > > > > > => sf probe 0:2 > > > > > > > SF: Detected is25wp256 with page size 256 Bytes, erase size 4 KiB, > > > total 32 > > > > > MiB > > > > > > > => sf probe 0:4 > > > > > > > SF: Detected is25wp256 with page size 256 Bytes, erase size 4 KiB, > > > total 32 > > > > > MiB > > > > > > > => sf probe 0:6 > > > > > > > SF: Detected is25wp256 with page size 256 Bytes, erase size 4 KiB, > > > total 32 > > > > > MiB > > > > > > > => dm tree > > > > > > > Class Index Probed Driver Name > > > > > > > ----------------------------------------------------------- > > > > > > > root 0 [ + ] root_driver root_driver > > > > > > > simple_bus 0 [ + ] generic_simple_bus |-- soc > > > > > > > clk 0 [ + ] sifive-fu540-prci | |-- > > > > > > > clock-controller@10000000 > > > > > > > serial 0 [ + ] serial_sifive | |-- > > > > > > > serial@10010000 > > > > > > > serial 1 [ ] serial_sifive | |-- > > > > > > > serial@10011000 > > > > > > > spi 0 [ + ] sifive_spi | |-- > > > > > > > spi@10040000 > > > > > > > spi_flash 0 [ ] spi_flash_std | | |-- > > > > > > > flash@0 > > > > > > > spi_flash 1 [ + ] spi_flash_std | | |-- > > > > > > > spi_flash@0:2 > > > > > > > spi_flash 2 [ + ] spi_flash_std | | |-- > > > > > > > spi_flash@0:4 > > > > > > > spi_flash 3 [ + ] spi_flash_std | | `-- > > > > > > > spi_flash@0:6 > > > > > > > > > > > > > > With my patch series below > > > > > > > http://patchwork.ozlabs.org/project/uboot/list/?series=129736 > > > > > > > > > > > > > > the CS number check was added to the SPI flash hence we got: > > > > > > > > > > > > > > => sf probe > > > > > > > unrecognized JEDEC id bytes: ff, ff, ff > > > > > > > Failed to initialize SPI flash at 0:0 (error -2) > > > > > > > => sf probe 2 > > > > > > > Invalid cs 2 (err=-22) > > > > > > > Invalid cs 2 (err=-22) > > > > > > > Invalid chip select 0:2 (err=-22) > > > > > > > Failed to initialize SPI flash at 0:2 (error -22) > > > > > > > => sf probe 4 > > > > > > > Invalid cs 4 (err=-22) > > > > > > > Invalid cs 4 (err=-22) > > > > > > > Invalid chip select 0:4 (err=-22) > > > > > > > Failed to initialize SPI flash at 0:4 (error -22) > > > > > > > > > > > > > > So we still need figure out why CS number 0 cannot work on > SiFive. > > > > > > > > > > > > The existing quad wire that driver pushing for flash seems not > > > > > > detecting flash at CS 0 so making cr to single wire detecting flash > > > > > > at > > > > > > > > > > I vaguely remember someone from SiFive posted a patch that forced > the > > > > > SPI controller to work under single wire mode. Maybe someone from > > > > > SiFive could help explain the weird behavior as you mentioned. > > > > > > > > > Sorry, I had not been closely looking into this thread. > > > > Yes, the initial patch posted was to add support for is25wp256 device. > > > > https://patchwork.ozlabs.org/patch/1146523/ > > > > but it seems it was bricking the board, and later couldn't follow-up on > > > this. > > > > IIUC, spi_nor_scan has default reg_proto set to 1_1_1, and further > parses > > > > slave mode and accordingly sets the hwcaps fields. Now since the > device > > > tree > > > > set's the slave mode to 4 bit mode based on the tx-bus-width, this > creates > > > > a conflict due to which reg_read fails and so the sf probe fails too. > > > > Now forcing the SPI-controller in single wire mode helped the sf probe > to > > > > detect the flash. Alternatively I think if we use bitlen passed by > > > spi_mem_exec_op > > > > and prepare spi transfer's in controller based on max of t->rx_nbits, t- > > > >tx_nbits as > > > > done in linux driver this should work. Any thoughts on this? > > > > > > Thanks for the explanation. Infact I found how to handle this via > > > set_mode where I can preserve the mode into driver private structure > > > so-that the same can be used while setting cr. One of your patch doing > > > the same [1] but with every transfer bitlen(which seems reasonable). > > > > > Good to know that it is working in case of cs0 as well. > > > > > With this I can detect the flash at cs 0 but still flash still > > > requires bfpt fixes to make it work. I would like to poke your series > > > and do the changes accordingly (by keeping your respective > > > authorship), will that be fine? > > > > > Yes, please feel free to do so and also let me know if there is > > something which I can be of help here. > > Here are my observations. > Thanks for sharing your input/observations
> 1. flash probe: > Probe happened to cs0 only when we enabled cs format propto value to > single (SIFIVE_SPI_FMT_PROTO_SINGLE). I did check it on the Linux > probe as well, same behaviour. > Yes, this matches with our understanding above, since reg read's during spi_nor_scan is 1-bit mode, and if cs proto value is set to SIFIVE_SPI_FMT_PROTO_SINGLE It works. > 2. flash operations: > Driver always assign single to format proto (say if we enable cr > format proto value based on bitlen) and it can't assign it ot dual and > quad even though the tx/rx-bus-width is 4 or 1 > > flash operations are not working with quad, I did check the quad > enable bit, it is already enabled (RDSR is 0x40 BIT[6] is 1 that means > quad enabled by default). Is Linux flash operations working with quad > or does it revert back to default commands like fast read and page > program? could you please check? > > "flash operations are working only for fast read, that means 2-wire." > I checked in linux-5.6.0-rc7 flash operations for issi devices: - quad enable bit is set via issi_set_default_init. - post spi_nor_init, the read protocol is set to FAST_READ_QUAD I/O mode, I do see corresponding opcode during Read operations (opcode observed here is 0xec) - The page program is set and uses the SPI mode (opcode 12h). > Any idea if flash triggers a opcode of quad (4-wire) does sifive cr > format proto aspected to assign SIFIVE_SPI_FMT_PROTO_QUAD? What > exactly is the relation between flash command operations vs driver cr > format proto bits? or how do we assign cr format proto bits based on > flash operations, is it based on opcode transfer bits or any other > logic? > IIUC, as in linux spi-nor framework in consideration here uses spi_mem operations and set's tx/rx nbits for addr, cmd, dummy and data. For u-boot SPI driver similarly during prepare transfer we need to parse this information And set the proto field within the driver respectively. So if we move to spi_mem_exec_op mechanism instead of xfer's will it be useful, as it does have tx/rx nbits information as per the addr/cmd/dummy and data transfers. Please let me know your views here. BR, Sagar Kadam > Jagan.