[U-Boot] [PATCH v2 5/5] sifive: fu540: Enable spi-nor flash support
Jagan Teki
jagan at amarulasolutions.com
Mon Apr 13 21:22:40 CEST 2020
On Tue, Apr 7, 2020 at 9:48 PM Sagar Kadam <sagar.kadam at sifive.com> wrote:
>
> Hello Jagan,
>
> > -----Original Message-----
> > From: Jagan Teki <jagan at amarulasolutions.com>
> > Sent: Monday, April 6, 2020 9:30 PM
> > To: Sagar Kadam <sagar.kadam at sifive.com>
> > Cc: Bin Meng <bmeng.cn at gmail.com>; Palmer Dabbelt
> > <palmer at sifive.com>; U-Boot Mailing List <u-boot at lists.denx.de>; linux-
> > amarula <linux-amarula at 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.kadam at sifive.com>
> > wrote:
> > >
> > > Hello Jagan,
> > >
> > > > -----Original Message-----
> > > > From: Jagan Teki <jagan at amarulasolutions.com>
> > > > Sent: Saturday, April 4, 2020 11:45 PM
> > > > To: Sagar Kadam <sagar.kadam at sifive.com>
> > > > Cc: Bin Meng <bmeng.cn at gmail.com>; Palmer Dabbelt
> > > > <palmer at sifive.com>; U-Boot Mailing List <u-boot at lists.denx.de>;
> > linux-
> > > > amarula <linux-amarula at 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.kadam at sifive.com>
> > > > wrote:
> > > > >
> > > > >
> > > > > Hello Jagan/Bin,
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: U-Boot <u-boot-bounces at lists.denx.de> On Behalf Of Bin
> > Meng
> > > > > > Sent: Monday, November 11, 2019 8:02 PM
> > > > > > To: Jagan Teki <jagan at amarulasolutions.com>
> > > > > > Cc: Palmer Dabbelt ( Sifive) <palmer at sifive.com>; U-Boot Mailing
> > List
> > > > <u-
> > > > > > boot at lists.denx.de>; linux-amarula <linux-
> > > > amarula at 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
> > > > <jagan at amarulasolutions.com>
> > > > > > wrote:
> > > > > > >
> > > > > > > Hi Bin,
> > > > > > >
> > > > > > > On Tue, Oct 29, 2019 at 3:50 PM Bin Meng <bmeng.cn at gmail.com>
> > > > wrote:
> > > > > > > >
> > > > > > > > Hi Jagan,
> > > > > > > >
> > > > > > > > On Tue, Oct 29, 2019 at 5:38 PM Bin Meng
> > <bmeng.cn at gmail.com>
> > > > > > wrote:
> > > > > > > > >
> > > > > > > > > Hi Jagan,
> > > > > > > > >
> > > > > > > > > On Wed, Oct 16, 2019 at 10:58 PM Jagan Teki
> > > > > > <jagan at 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 <jagan at amarulasolutions.com>
> > > > > > > > > > Reviewed-by: Bin Meng <bmeng.cn at gmail.com>
> > > > > > > > > > Tested-by: Bin Meng <bmeng.cn at 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 at 10000000
> > > > > > > > serial 0 [ + ] serial_sifive | |-- serial at 10010000
> > > > > > > > serial 1 [ ] serial_sifive | |-- serial at 10011000
> > > > > > > > spi 0 [ + ] sifive_spi | |-- spi at 10040000
> > > > > > > > spi_flash 0 [ ] spi_flash_std | | |-- flash at 0
> > > > > > > > spi_flash 1 [ + ] spi_flash_std | | |-- spi_flash at 0:2
> > > > > > > > spi_flash 2 [ + ] spi_flash_std | | |-- spi_flash at 0:4
> > > > > > > > spi_flash 3 [ + ] spi_flash_std | | `-- spi_flash at 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).
So, the transfer itself triggers several read types based on the tx/rx
length type.
>
> > 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.
Vignesh, any insight this. Look like the controller expecting to set
the SPI mode based on the transfer or xfer length of particular
transaction from flash.
Jagan.
More information about the U-Boot
mailing list