[v2,5/5] sifive: fu540: Enable spi-nor flash support

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@amarulasolutions.com>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Tested-by: Bin Meng <bmeng.cn@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(+)
  

Hi Jagan,

On Wed, Oct 16, 2019 at 10:58 PM Jagan Teki <jagan@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@amarulasolutions.com>
> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> Tested-by: Bin Meng <bmeng.cn@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.

> +   SF: Detected is25wp256 with page size 256 Bytes, erase size 4 KiB, total 32 MiB
> +   => sf erase 0x1000000 0x100000
> +   SF: 1048576 bytes @ 0x1000000 Erased: OK
> +   => mw.b 0xc0000000 0xaa 0x100000
> +   => sf write 0xc0000000 0x1000000 0x100000
> +   device 0 offset 0x1000000, size 0x100000
> +   SF: 1048576 bytes @ 0x1000000 Written: OK
> +   => sf read 0xf0000000 0x1000000 0x100000
> +   device 0 offset 0x1000000, size 0x100000
> +   SF: 1048576 bytes @ 0x1000000 Read: OK
> +   => cmp.b 0xf0000000 0xc0000000 0x100000
> +   Total of 1048576 byte(s) were the same
> --

Regards,
Bin
  

Hi Jagan,

On Tue, Oct 29, 2019 at 5:38 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Jagan,
>
> On Wed, Oct 16, 2019 at 10:58 PM Jagan Teki <jagan@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@amarulasolutions.com>
> > Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> > Tested-by: Bin Meng <bmeng.cn@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.

>
> > +   SF: Detected is25wp256 with page size 256 Bytes, erase size 4 KiB, total 32 MiB
> > +   => sf erase 0x1000000 0x100000
> > +   SF: 1048576 bytes @ 0x1000000 Erased: OK
> > +   => mw.b 0xc0000000 0xaa 0x100000
> > +   => sf write 0xc0000000 0x1000000 0x100000
> > +   device 0 offset 0x1000000, size 0x100000
> > +   SF: 1048576 bytes @ 0x1000000 Written: OK
> > +   => sf read 0xf0000000 0x1000000 0x100000
> > +   device 0 offset 0x1000000, size 0x100000
> > +   SF: 1048576 bytes @ 0x1000000 Read: OK
> > +   => cmp.b 0xf0000000 0xc0000000 0x100000
> > +   Total of 1048576 byte(s) were the same
> > --

Regards,
Bin
  

Hi Bin,

On Tue, Oct 29, 2019 at 3:50 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Jagan,
>
> On Tue, Oct 29, 2019 at 5:38 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > Hi Jagan,
> >
> > On Wed, Oct 16, 2019 at 10:58 PM Jagan Teki <jagan@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@amarulasolutions.com>
> > > Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> > > Tested-by: Bin Meng <bmeng.cn@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
CS0. One more point is that flash seems available in different CS, not
sure whether the hardware wired this logic or it has flash really on
those CS's.

=> sf probe 0:0
SF: Detected is25wp256 with page size 256 Bytes, erase size 4 KiB, total 32 MiB
=> sf probe 0:1
unrecognized JEDEC id bytes: ff, ff, ff
Failed to initialize SPI flash at 0:1 (error -2)
=> sf probe 0:2
SF: Detected is25wp256 with page size 256 Bytes, erase size 4 KiB, total 32 MiB
=> sf probe 0:3
unrecognized JEDEC id bytes: ff, ff, ff
Failed to initialize SPI flash at 0:3 (error -2)
=> sf probe 0:4
SF: Detected is25wp256 with page size 256 Bytes, erase size 4 KiB, total 32 MiB
=> sf probe 0:5
unrecognized JEDEC id bytes: ff, ff, ff
Failed to initialize SPI flash at 0:5 (error -2)
=> sf probe 0:6
SF: Detected is25wp256 with page size 256 Bytes, erase size 4 KiB, total 32 MiB
=> sf probe 0:7
unrecognized JEDEC id bytes: ff, ff, ff
Failed to initialize SPI flash at 0:7 (error -2)
=> sf probe 0:8
SF: Detected is25wp256 with page size 256 Bytes, erase size 4 KiB, total 32 MiB
=> sf probe 0:9
unrecognized JEDEC id bytes: ff, ff, ff
Failed to initialize SPI flash at 0:9 (error -2)
  

Hi Jagan,

On Sat, Nov 9, 2019 at 7:57 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
>
> Hi Bin,
>
> On Tue, Oct 29, 2019 at 3:50 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > Hi Jagan,
> >
> > On Tue, Oct 29, 2019 at 5:38 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> > >
> > > Hi Jagan,
> > >
> > > On Wed, Oct 16, 2019 at 10:58 PM Jagan Teki <jagan@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@amarulasolutions.com>
> > > > Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> > > > Tested-by: Bin Meng <bmeng.cn@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.

> CS0. One more point is that flash seems available in different CS, not
> sure whether the hardware wired this logic or it has flash really on
> those CS's.
>
> => sf probe 0:0
> SF: Detected is25wp256 with page size 256 Bytes, erase size 4 KiB, total 32 MiB
> => sf probe 0:1
> unrecognized JEDEC id bytes: ff, ff, ff
> Failed to initialize SPI flash at 0:1 (error -2)
> => sf probe 0:2
> SF: Detected is25wp256 with page size 256 Bytes, erase size 4 KiB, total 32 MiB
> => sf probe 0:3
> unrecognized JEDEC id bytes: ff, ff, ff
> Failed to initialize SPI flash at 0:3 (error -2)
> => sf probe 0:4
> SF: Detected is25wp256 with page size 256 Bytes, erase size 4 KiB, total 32 MiB
> => sf probe 0:5
> unrecognized JEDEC id bytes: ff, ff, ff
> Failed to initialize SPI flash at 0:5 (error -2)
> => sf probe 0:6
> SF: Detected is25wp256 with page size 256 Bytes, erase size 4 KiB, total 32 MiB
> => sf probe 0:7
> unrecognized JEDEC id bytes: ff, ff, ff
> Failed to initialize SPI flash at 0:7 (error -2)
> => sf probe 0:8
> SF: Detected is25wp256 with page size 256 Bytes, erase size 4 KiB, total 32 MiB
> => sf probe 0:9
> unrecognized JEDEC id bytes: ff, ff, ff
> Failed to initialize SPI flash at 0:9 (error -2)

Regards,
Bin
  

Hello Jagan/Bin,

> -----Original Message-----
> From: U-Boot <u-boot-bounces@lists.denx.de> On Behalf Of Bin Meng
> Sent: Monday, November 11, 2019 8:02 PM
> To: Jagan Teki <jagan@amarulasolutions.com>
> Cc: Palmer Dabbelt ( Sifive) <palmer@sifive.com>; U-Boot Mailing List <u-
> boot@lists.denx.de>; linux-amarula <linux-amarula@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@amarulasolutions.com>
> wrote:
> >
> > Hi Bin,
> >
> > On Tue, Oct 29, 2019 at 3:50 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> > >
> > > Hi Jagan,
> > >
> > > On Tue, Oct 29, 2019 at 5:38 PM Bin Meng <bmeng.cn@gmail.com>
> wrote:
> > > >
> > > > Hi Jagan,
> > > >
> > > > On Wed, Oct 16, 2019 at 10:58 PM Jagan Teki
> <jagan@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@amarulasolutions.com>
> > > > > Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> > > > > Tested-by: Bin Meng <bmeng.cn@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 & BR,
Sagar Kadam

> > CS0. One more point is that flash seems available in different CS, not
> > sure whether the hardware wired this logic or it has flash really on
> > those CS's.
> >
> > => sf probe 0:0
> > SF: Detected is25wp256 with page size 256 Bytes, erase size 4 KiB, total 32
> MiB
> > => sf probe 0:1
> > unrecognized JEDEC id bytes: ff, ff, ff
> > Failed to initialize SPI flash at 0:1 (error -2)
> > => sf probe 0:2
> > SF: Detected is25wp256 with page size 256 Bytes, erase size 4 KiB, total 32
> MiB
> > => sf probe 0:3
> > unrecognized JEDEC id bytes: ff, ff, ff
> > Failed to initialize SPI flash at 0:3 (error -2)
> > => sf probe 0:4
> > SF: Detected is25wp256 with page size 256 Bytes, erase size 4 KiB, total 32
> MiB
> > => sf probe 0:5
> > unrecognized JEDEC id bytes: ff, ff, ff
> > Failed to initialize SPI flash at 0:5 (error -2)
> > => sf probe 0:6
> > SF: Detected is25wp256 with page size 256 Bytes, erase size 4 KiB, total 32
> MiB
> > => sf probe 0:7
> > unrecognized JEDEC id bytes: ff, ff, ff
> > Failed to initialize SPI flash at 0:7 (error -2)
> > => sf probe 0:8
> > SF: Detected is25wp256 with page size 256 Bytes, erase size 4 KiB, total 32
> MiB
> > => sf probe 0:9
> > unrecognized JEDEC id bytes: ff, ff, ff
> > Failed to initialize SPI flash at 0:9 (error -2)
> 
> Regards,
> Bin
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> https://lists.denx.de/listinfo/u-boot
  

Hi Segar Kadam,

On Mon, Nov 18, 2019 at 4:59 AM Sagar Kadam <sagar.kadam@sifive.com> wrote:
>
>
> Hello Jagan/Bin,
>
> > -----Original Message-----
> > From: U-Boot <u-boot-bounces@lists.denx.de> On Behalf Of Bin Meng
> > Sent: Monday, November 11, 2019 8:02 PM
> > To: Jagan Teki <jagan@amarulasolutions.com>
> > Cc: Palmer Dabbelt ( Sifive) <palmer@sifive.com>; U-Boot Mailing List <u-
> > boot@lists.denx.de>; linux-amarula <linux-amarula@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@amarulasolutions.com>
> > wrote:
> > >
> > > Hi Bin,
> > >
> > > On Tue, Oct 29, 2019 at 3:50 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> > > >
> > > > Hi Jagan,
> > > >
> > > > On Tue, Oct 29, 2019 at 5:38 PM Bin Meng <bmeng.cn@gmail.com>
> > wrote:
> > > > >
> > > > > Hi Jagan,
> > > > >
> > > > > On Wed, Oct 16, 2019 at 10:58 PM Jagan Teki
> > <jagan@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@amarulasolutions.com>
> > > > > > Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> > > > > > Tested-by: Bin Meng <bmeng.cn@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.

The strange thing is that the 'sf probe' only fails at CS 0, but
succeeds at CS 2/4/6/8/...

Is this strange behavior caused by the default reg_proto set to 1_1_1
in spi_nor_scan()?

> 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?
>

Maybe Jagan can comment this.

Regards,
Bin
  

Hi Jagan,

On Mon, Nov 18, 2019 at 10:49 AM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Segar Kadam,
>
> On Mon, Nov 18, 2019 at 4:59 AM Sagar Kadam <sagar.kadam@sifive.com> wrote:
> >
> >
> > Hello Jagan/Bin,
> >
> > > -----Original Message-----
> > > From: U-Boot <u-boot-bounces@lists.denx.de> On Behalf Of Bin Meng
> > > Sent: Monday, November 11, 2019 8:02 PM
> > > To: Jagan Teki <jagan@amarulasolutions.com>
> > > Cc: Palmer Dabbelt ( Sifive) <palmer@sifive.com>; U-Boot Mailing List <u-
> > > boot@lists.denx.de>; linux-amarula <linux-amarula@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@amarulasolutions.com>
> > > wrote:
> > > >
> > > > Hi Bin,
> > > >
> > > > On Tue, Oct 29, 2019 at 3:50 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> > > > >
> > > > > Hi Jagan,
> > > > >
> > > > > On Tue, Oct 29, 2019 at 5:38 PM Bin Meng <bmeng.cn@gmail.com>
> > > wrote:
> > > > > >
> > > > > > Hi Jagan,
> > > > > >
> > > > > > On Wed, Oct 16, 2019 at 10:58 PM Jagan Teki
> > > <jagan@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@amarulasolutions.com>
> > > > > > > Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> > > > > > > Tested-by: Bin Meng <bmeng.cn@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.
>
> The strange thing is that the 'sf probe' only fails at CS 0, but
> succeeds at CS 2/4/6/8/...
>
> Is this strange behavior caused by the default reg_proto set to 1_1_1
> in spi_nor_scan()?
>
> > 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?
> >
>
> Maybe Jagan can comment this.

Could you please also take a look at this issue that Sagar reported?

Regards,
Bin
  

Hi Sagar,

On Mon, Nov 18, 2019 at 2:29 AM Sagar Kadam <sagar.kadam@sifive.com> wrote:
>
>
> Hello Jagan/Bin,
>
> > -----Original Message-----
> > From: U-Boot <u-boot-bounces@lists.denx.de> On Behalf Of Bin Meng
> > Sent: Monday, November 11, 2019 8:02 PM
> > To: Jagan Teki <jagan@amarulasolutions.com>
> > Cc: Palmer Dabbelt ( Sifive) <palmer@sifive.com>; U-Boot Mailing List <u-
> > boot@lists.denx.de>; linux-amarula <linux-amarula@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@amarulasolutions.com>
> > wrote:
> > >
> > > Hi Bin,
> > >
> > > On Tue, Oct 29, 2019 at 3:50 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> > > >
> > > > Hi Jagan,
> > > >
> > > > On Tue, Oct 29, 2019 at 5:38 PM Bin Meng <bmeng.cn@gmail.com>
> > wrote:
> > > > >
> > > > > Hi Jagan,
> > > > >
> > > > > On Wed, Oct 16, 2019 at 10:58 PM Jagan Teki
> > <jagan@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@amarulasolutions.com>
> > > > > > Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> > > > > > Tested-by: Bin Meng <bmeng.cn@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).

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?

[1] https://patchwork.ozlabs.org/patch/1228554/

Jagan.
  

Hi Bin,

On Mon, Nov 18, 2019 at 8:19 AM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Segar Kadam,
>
> On Mon, Nov 18, 2019 at 4:59 AM Sagar Kadam <sagar.kadam@sifive.com> wrote:
> >
> >
> > Hello Jagan/Bin,
> >
> > > -----Original Message-----
> > > From: U-Boot <u-boot-bounces@lists.denx.de> On Behalf Of Bin Meng
> > > Sent: Monday, November 11, 2019 8:02 PM
> > > To: Jagan Teki <jagan@amarulasolutions.com>
> > > Cc: Palmer Dabbelt ( Sifive) <palmer@sifive.com>; U-Boot Mailing List <u-
> > > boot@lists.denx.de>; linux-amarula <linux-amarula@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@amarulasolutions.com>
> > > wrote:
> > > >
> > > > Hi Bin,
> > > >
> > > > On Tue, Oct 29, 2019 at 3:50 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> > > > >
> > > > > Hi Jagan,
> > > > >
> > > > > On Tue, Oct 29, 2019 at 5:38 PM Bin Meng <bmeng.cn@gmail.com>
> > > wrote:
> > > > > >
> > > > > > Hi Jagan,
> > > > > >
> > > > > > On Wed, Oct 16, 2019 at 10:58 PM Jagan Teki
> > > <jagan@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@amarulasolutions.com>
> > > > > > > Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> > > > > > > Tested-by: Bin Meng <bmeng.cn@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.
>
> The strange thing is that the 'sf probe' only fails at CS 0, but
> succeeds at CS 2/4/6/8/...
>
> Is this strange behavior caused by the default reg_proto set to 1_1_1
> in spi_nor_scan()?
>
> > 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?
> >
>
> Maybe Jagan can comment this.

Looks like there are several issues to make this flash to work, one
with a probe and others are handling cs between transfer and flash
bfpt fixes.

probe failed with cs 0  (or even with other cs numbers) because of
improper bitlen, spi-nor sending fast read (mean single wire)
initially but the driver checks the bitlen based on slave->mode (which
is from dts) ie is quad.

Default behaviour:

=> sf probe 0:0
sifive_spi_prep_transfer: QUAD
sifive_spi_prep_transfer: QUAD
unrecognized JEDEC id bytes: ff, ff, ff
Failed to initialize SPI flash at 0:0 (error -2)
=> sf probe 0:1
Invalid cs 1 (err=-22)
Invalid cs 1 (err=-22)
Invalid chip select 0:1 (err=-22)
Failed to initialize SPI flash at 0:1 (error -22)
=> sf probe 0: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)

With , update the cr bits based on bitlen instead of slave->mode:

=> sf probe
sifive_spi_prep_transfer: SINGLE
sifive_spi_prep_transfer: SINGLE
sifive_spi_prep_transfer: SINGLE
sifive_spi_prep_transfer: SINGLE
SF: Detected is25wp256 with page size 256 Bytes, erase size 4 KiB, total 32 MiB
=> sf probe 0:1
Invalid cs 1 (err=-22)
Invalid cs 1 (err=-22)
Invalid chip select 0:1 (err=-22)
Failed to initialize SPI flash at 0:1 (error -22)

Jagan.
  

Hello Jagan,

> -----Original Message-----
> From: Jagan Teki <jagan@amarulasolutions.com>
> Sent: Saturday, April 4, 2020 11:45 PM
> To: Sagar Kadam <sagar.kadam@sifive.com>
> Cc: Bin Meng <bmeng.cn@gmail.com>; Palmer Dabbelt
> <palmer@sifive.com>; U-Boot Mailing List <u-boot@lists.denx.de>; linux-
> amarula <linux-amarula@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@sifive.com>
> wrote:
> >
> >
> > Hello Jagan/Bin,
> >
> > > -----Original Message-----
> > > From: U-Boot <u-boot-bounces@lists.denx.de> On Behalf Of Bin Meng
> > > Sent: Monday, November 11, 2019 8:02 PM
> > > To: Jagan Teki <jagan@amarulasolutions.com>
> > > Cc: Palmer Dabbelt ( Sifive) <palmer@sifive.com>; U-Boot Mailing List
> <u-
> > > boot@lists.denx.de>; linux-amarula <linux-
> amarula@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@amarulasolutions.com>
> > > wrote:
> > > >
> > > > Hi Bin,
> > > >
> > > > On Tue, Oct 29, 2019 at 3:50 PM Bin Meng <bmeng.cn@gmail.com>
> wrote:
> > > > >
> > > > > Hi Jagan,
> > > > >
> > > > > On Tue, Oct 29, 2019 at 5:38 PM Bin Meng <bmeng.cn@gmail.com>
> > > wrote:
> > > > > >
> > > > > > Hi Jagan,
> > > > > >
> > > > > > On Wed, Oct 16, 2019 at 10:58 PM Jagan Teki
> > > <jagan@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@amarulasolutions.com>
> > > > > > > Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> > > > > > > Tested-by: Bin Meng <bmeng.cn@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.

BR,
Sagar Kadam

> [1] https://patchwork.ozlabs.org/patch/1228554/
> 
> Jagan.
  

Hi Jagan,

On Sun, Apr 5, 2020 at 2:44 AM Jagan Teki <jagan@amarulasolutions.com> wrote:
>
> Hi Bin,
>
> On Mon, Nov 18, 2019 at 8:19 AM Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > Hi Segar Kadam,
> >
> > On Mon, Nov 18, 2019 at 4:59 AM Sagar Kadam <sagar.kadam@sifive.com> wrote:
> > >
> > >
> > > Hello Jagan/Bin,
> > >
> > > > -----Original Message-----
> > > > From: U-Boot <u-boot-bounces@lists.denx.de> On Behalf Of Bin Meng
> > > > Sent: Monday, November 11, 2019 8:02 PM
> > > > To: Jagan Teki <jagan@amarulasolutions.com>
> > > > Cc: Palmer Dabbelt ( Sifive) <palmer@sifive.com>; U-Boot Mailing List <u-
> > > > boot@lists.denx.de>; linux-amarula <linux-amarula@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@amarulasolutions.com>
> > > > wrote:
> > > > >
> > > > > Hi Bin,
> > > > >
> > > > > On Tue, Oct 29, 2019 at 3:50 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> > > > > >
> > > > > > Hi Jagan,
> > > > > >
> > > > > > On Tue, Oct 29, 2019 at 5:38 PM Bin Meng <bmeng.cn@gmail.com>
> > > > wrote:
> > > > > > >
> > > > > > > Hi Jagan,
> > > > > > >
> > > > > > > On Wed, Oct 16, 2019 at 10:58 PM Jagan Teki
> > > > <jagan@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@amarulasolutions.com>
> > > > > > > > Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> > > > > > > > Tested-by: Bin Meng <bmeng.cn@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.
> >
> > The strange thing is that the 'sf probe' only fails at CS 0, but
> > succeeds at CS 2/4/6/8/...
> >
> > Is this strange behavior caused by the default reg_proto set to 1_1_1
> > in spi_nor_scan()?
> >
> > > 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?
> > >
> >
> > Maybe Jagan can comment this.
>
> Looks like there are several issues to make this flash to work, one
> with a probe and others are handling cs between transfer and flash
> bfpt fixes.
>
> probe failed with cs 0  (or even with other cs numbers) because of
> improper bitlen, spi-nor sending fast read (mean single wire)
> initially but the driver checks the bitlen based on slave->mode (which
> is from dts) ie is quad.
>

Thank you for looking into this. Could you please work with Sagar to
come up with a new version patch that fix all these issues?

> Default behaviour:
>
> => sf probe 0:0
> sifive_spi_prep_transfer: QUAD
> sifive_spi_prep_transfer: QUAD
> unrecognized JEDEC id bytes: ff, ff, ff
> Failed to initialize SPI flash at 0:0 (error -2)
> => sf probe 0:1
> Invalid cs 1 (err=-22)
> Invalid cs 1 (err=-22)
> Invalid chip select 0:1 (err=-22)
> Failed to initialize SPI flash at 0:1 (error -22)
> => sf probe 0: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)
>
> With , update the cr bits based on bitlen instead of slave->mode:
>
> => sf probe
> sifive_spi_prep_transfer: SINGLE
> sifive_spi_prep_transfer: SINGLE
> sifive_spi_prep_transfer: SINGLE
> sifive_spi_prep_transfer: SINGLE
> SF: Detected is25wp256 with page size 256 Bytes, erase size 4 KiB, total 32 MiB
> => sf probe 0:1
> Invalid cs 1 (err=-22)
> Invalid cs 1 (err=-22)
> Invalid chip select 0:1 (err=-22)
> Failed to initialize SPI flash at 0:1 (error -22)
>

Regards,
Bin
  

Hi Sagar,

On Sun, Apr 5, 2020 at 12:29 AM Sagar Kadam <sagar.kadam@sifive.com> wrote:
>
> Hello Jagan,
>
> > -----Original Message-----
> > From: Jagan Teki <jagan@amarulasolutions.com>
> > Sent: Saturday, April 4, 2020 11:45 PM
> > To: Sagar Kadam <sagar.kadam@sifive.com>
> > Cc: Bin Meng <bmeng.cn@gmail.com>; Palmer Dabbelt
> > <palmer@sifive.com>; U-Boot Mailing List <u-boot@lists.denx.de>; linux-
> > amarula <linux-amarula@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@sifive.com>
> > wrote:
> > >
> > >
> > > Hello Jagan/Bin,
> > >
> > > > -----Original Message-----
> > > > From: U-Boot <u-boot-bounces@lists.denx.de> On Behalf Of Bin Meng
> > > > Sent: Monday, November 11, 2019 8:02 PM
> > > > To: Jagan Teki <jagan@amarulasolutions.com>
> > > > Cc: Palmer Dabbelt ( Sifive) <palmer@sifive.com>; U-Boot Mailing List
> > <u-
> > > > boot@lists.denx.de>; linux-amarula <linux-
> > amarula@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@amarulasolutions.com>
> > > > wrote:
> > > > >
> > > > > Hi Bin,
> > > > >
> > > > > On Tue, Oct 29, 2019 at 3:50 PM Bin Meng <bmeng.cn@gmail.com>
> > wrote:
> > > > > >
> > > > > > Hi Jagan,
> > > > > >
> > > > > > On Tue, Oct 29, 2019 at 5:38 PM Bin Meng <bmeng.cn@gmail.com>
> > > > wrote:
> > > > > > >
> > > > > > > Hi Jagan,
> > > > > > >
> > > > > > > On Wed, Oct 16, 2019 at 10:58 PM Jagan Teki
> > > > <jagan@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@amarulasolutions.com>
> > > > > > > > Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> > > > > > > > Tested-by: Bin Meng <bmeng.cn@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.

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.

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."

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?

Jagan.
  

Hello Jagan,

> -----Original Message-----
> From: Jagan Teki <jagan@amarulasolutions.com>
> Sent: Monday, April 6, 2020 9:30 PM
> To: Sagar Kadam <sagar.kadam@sifive.com>
> Cc: Bin Meng <bmeng.cn@gmail.com>; Palmer Dabbelt
> <palmer@sifive.com>; U-Boot Mailing List <u-boot@lists.denx.de>; linux-
> amarula <linux-amarula@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@sifive.com>
> wrote:
> >
> > Hello Jagan,
> >
> > > -----Original Message-----
> > > From: Jagan Teki <jagan@amarulasolutions.com>
> > > Sent: Saturday, April 4, 2020 11:45 PM
> > > To: Sagar Kadam <sagar.kadam@sifive.com>
> > > Cc: Bin Meng <bmeng.cn@gmail.com>; Palmer Dabbelt
> > > <palmer@sifive.com>; U-Boot Mailing List <u-boot@lists.denx.de>;
> linux-
> > > amarula <linux-amarula@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@sifive.com>
> > > wrote:
> > > >
> > > >
> > > > Hello Jagan/Bin,
> > > >
> > > > > -----Original Message-----
> > > > > From: U-Boot <u-boot-bounces@lists.denx.de> On Behalf Of Bin
> Meng
> > > > > Sent: Monday, November 11, 2019 8:02 PM
> > > > > To: Jagan Teki <jagan@amarulasolutions.com>
> > > > > Cc: Palmer Dabbelt ( Sifive) <palmer@sifive.com>; U-Boot Mailing
> List
> > > <u-
> > > > > boot@lists.denx.de>; linux-amarula <linux-
> > > amarula@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@amarulasolutions.com>
> > > > > wrote:
> > > > > >
> > > > > > Hi Bin,
> > > > > >
> > > > > > On Tue, Oct 29, 2019 at 3:50 PM Bin Meng <bmeng.cn@gmail.com>
> > > wrote:
> > > > > > >
> > > > > > > Hi Jagan,
> > > > > > >
> > > > > > > On Tue, Oct 29, 2019 at 5:38 PM Bin Meng
> <bmeng.cn@gmail.com>
> > > > > wrote:
> > > > > > > >
> > > > > > > > Hi Jagan,
> > > > > > > >
> > > > > > > > On Wed, Oct 16, 2019 at 10:58 PM Jagan Teki
> > > > > <jagan@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@amarulasolutions.com>
> > > > > > > > > Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> > > > > > > > > Tested-by: Bin Meng <bmeng.cn@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.
  

On Tue, Apr 7, 2020 at 9:48 PM Sagar Kadam <sagar.kadam@sifive.com> wrote:
>
> Hello Jagan,
>
> > -----Original Message-----
> > From: Jagan Teki <jagan@amarulasolutions.com>
> > Sent: Monday, April 6, 2020 9:30 PM
> > To: Sagar Kadam <sagar.kadam@sifive.com>
> > Cc: Bin Meng <bmeng.cn@gmail.com>; Palmer Dabbelt
> > <palmer@sifive.com>; U-Boot Mailing List <u-boot@lists.denx.de>; linux-
> > amarula <linux-amarula@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@sifive.com>
> > wrote:
> > >
> > > Hello Jagan,
> > >
> > > > -----Original Message-----
> > > > From: Jagan Teki <jagan@amarulasolutions.com>
> > > > Sent: Saturday, April 4, 2020 11:45 PM
> > > > To: Sagar Kadam <sagar.kadam@sifive.com>
> > > > Cc: Bin Meng <bmeng.cn@gmail.com>; Palmer Dabbelt
> > > > <palmer@sifive.com>; U-Boot Mailing List <u-boot@lists.denx.de>;
> > linux-
> > > > amarula <linux-amarula@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@sifive.com>
> > > > wrote:
> > > > >
> > > > >
> > > > > Hello Jagan/Bin,
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: U-Boot <u-boot-bounces@lists.denx.de> On Behalf Of Bin
> > Meng
> > > > > > Sent: Monday, November 11, 2019 8:02 PM
> > > > > > To: Jagan Teki <jagan@amarulasolutions.com>
> > > > > > Cc: Palmer Dabbelt ( Sifive) <palmer@sifive.com>; U-Boot Mailing
> > List
> > > > <u-
> > > > > > boot@lists.denx.de>; linux-amarula <linux-
> > > > amarula@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@amarulasolutions.com>
> > > > > > wrote:
> > > > > > >
> > > > > > > Hi Bin,
> > > > > > >
> > > > > > > On Tue, Oct 29, 2019 at 3:50 PM Bin Meng <bmeng.cn@gmail.com>
> > > > wrote:
> > > > > > > >
> > > > > > > > Hi Jagan,
> > > > > > > >
> > > > > > > > On Tue, Oct 29, 2019 at 5:38 PM Bin Meng
> > <bmeng.cn@gmail.com>
> > > > > > wrote:
> > > > > > > > >
> > > > > > > > > Hi Jagan,
> > > > > > > > >
> > > > > > > > > On Wed, Oct 16, 2019 at 10:58 PM Jagan Teki
> > > > > > <jagan@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@amarulasolutions.com>
> > > > > > > > > > Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> > > > > > > > > > Tested-by: Bin Meng <bmeng.cn@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).

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.
  

Hi Sagar,

On Tue, Apr 7, 2020 at 9:48 PM Sagar Kadam <sagar.kadam@sifive.com> wrote:
>
> Hello Jagan,
>
> > -----Original Message-----
> > From: Jagan Teki <jagan@amarulasolutions.com>
> > Sent: Monday, April 6, 2020 9:30 PM
> > To: Sagar Kadam <sagar.kadam@sifive.com>
> > Cc: Bin Meng <bmeng.cn@gmail.com>; Palmer Dabbelt
> > <palmer@sifive.com>; U-Boot Mailing List <u-boot@lists.denx.de>; linux-
> > amarula <linux-amarula@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@sifive.com>
> > wrote:
> > >
> > > Hello Jagan,
> > >
> > > > -----Original Message-----
> > > > From: Jagan Teki <jagan@amarulasolutions.com>
> > > > Sent: Saturday, April 4, 2020 11:45 PM
> > > > To: Sagar Kadam <sagar.kadam@sifive.com>
> > > > Cc: Bin Meng <bmeng.cn@gmail.com>; Palmer Dabbelt
> > > > <palmer@sifive.com>; U-Boot Mailing List <u-boot@lists.denx.de>;
> > linux-
> > > > amarula <linux-amarula@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@sifive.com>
> > > > wrote:
> > > > >
> > > > >
> > > > > Hello Jagan/Bin,
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: U-Boot <u-boot-bounces@lists.denx.de> On Behalf Of Bin
> > Meng
> > > > > > Sent: Monday, November 11, 2019 8:02 PM
> > > > > > To: Jagan Teki <jagan@amarulasolutions.com>
> > > > > > Cc: Palmer Dabbelt ( Sifive) <palmer@sifive.com>; U-Boot Mailing
> > List
> > > > <u-
> > > > > > boot@lists.denx.de>; linux-amarula <linux-
> > > > amarula@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@amarulasolutions.com>
> > > > > > wrote:
> > > > > > >
> > > > > > > Hi Bin,
> > > > > > >
> > > > > > > On Tue, Oct 29, 2019 at 3:50 PM Bin Meng <bmeng.cn@gmail.com>
> > > > wrote:
> > > > > > > >
> > > > > > > > Hi Jagan,
> > > > > > > >
> > > > > > > > On Tue, Oct 29, 2019 at 5:38 PM Bin Meng
> > <bmeng.cn@gmail.com>
> > > > > > wrote:
> > > > > > > > >
> > > > > > > > > Hi Jagan,
> > > > > > > > >
> > > > > > > > > On Wed, Oct 16, 2019 at 10:58 PM Jagan Teki
> > > > > > <jagan@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@amarulasolutions.com>
> > > > > > > > > > Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> > > > > > > > > > Tested-by: Bin Meng <bmeng.cn@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).

I did manage to get the tx or rx bits logic related to u-boot via
buswidth. I can see it is working with quad read and quad write is not
working. Seems like Quad write is also not working even for Linux any
idea, why [2]?

[2] https://paste.ubuntu.com/p/nhZzQkkG33/

Jagan.
  

Hello Jagan,

> -----Original Message-----
> From: Jagan Teki <jagan@amarulasolutions.com>
> Sent: Friday, April 17, 2020 1:30 PM
> To: Sagar Kadam <sagar.kadam@sifive.com>
> Cc: Bin Meng <bmeng.cn@gmail.com>; U-Boot Mailing List <u-
> boot@lists.denx.de>; linux-amarula <linux-
> amarula@amarulasolutions.com>; palmer@dabbelt.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 Tue, Apr 7, 2020 at 9:48 PM Sagar Kadam <sagar.kadam@sifive.com>
> wrote:
> >
> > Hello Jagan,
> >
> > > -----Original Message-----
> > > From: Jagan Teki <jagan@amarulasolutions.com>
> > > Sent: Monday, April 6, 2020 9:30 PM
> > > To: Sagar Kadam <sagar.kadam@sifive.com>
> > > Cc: Bin Meng <bmeng.cn@gmail.com>; Palmer Dabbelt
> > > <palmer@sifive.com>; U-Boot Mailing List <u-boot@lists.denx.de>;
> > > linux- amarula <linux-amarula@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@sifive.com>
> > > wrote:
> > > >
> > > > Hello Jagan,
> > > >
> > > > > -----Original Message-----
> > > > > From: Jagan Teki <jagan@amarulasolutions.com>
> > > > > Sent: Saturday, April 4, 2020 11:45 PM
> > > > > To: Sagar Kadam <sagar.kadam@sifive.com>
> > > > > Cc: Bin Meng <bmeng.cn@gmail.com>; Palmer Dabbelt
> > > > > <palmer@sifive.com>; U-Boot Mailing List <u-boot@lists.denx.de>;
> > > linux-
> > > > > amarula <linux-amarula@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@sifive.com>
> > > > > wrote:
> > > > > >
> > > > > >
> > > > > > Hello Jagan/Bin,
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: U-Boot <u-boot-bounces@lists.denx.de> On Behalf Of Bin
> > > Meng
> > > > > > > Sent: Monday, November 11, 2019 8:02 PM
> > > > > > > To: Jagan Teki <jagan@amarulasolutions.com>
> > > > > > > Cc: Palmer Dabbelt ( Sifive) <palmer@sifive.com>; U-Boot
> > > > > > > Mailing
> > > List
> > > > > <u-
> > > > > > > boot@lists.denx.de>; linux-amarula <linux-
> > > > > amarula@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@amarulasolutions.com>
> > > > > > > wrote:
> > > > > > > >
> > > > > > > > Hi Bin,
> > > > > > > >
> > > > > > > > On Tue, Oct 29, 2019 at 3:50 PM Bin Meng
> > > > > > > > <bmeng.cn@gmail.com>
> > > > > wrote:
> > > > > > > > >
> > > > > > > > > Hi Jagan,
> > > > > > > > >
> > > > > > > > > On Tue, Oct 29, 2019 at 5:38 PM Bin Meng
> > > <bmeng.cn@gmail.com>
> > > > > > > wrote:
> > > > > > > > > >
> > > > > > > > > > Hi Jagan,
> > > > > > > > > >
> > > > > > > > > > On Wed, Oct 16, 2019 at 10:58 PM Jagan Teki
> > > > > > > <jagan@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@amarulasolutions.com>
> > > > > > > > > > > Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> > > > > > > > > > > Tested-by: Bin Meng <bmeng.cn@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.dts
> > > > > > > > > > > +++ i
> > > > > > > > > > > @@ -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=1
> > > > > > > > > 29736
> > > > > > > > >
> > > > > > > > > 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).
> 
> I did manage to get the tx or rx bits logic related to u-boot via buswidth. I
> can see it is working with quad read and quad write is not working. Seems
> like Quad write is also not working even for Linux any idea, why [2]?
> 
> [2] https://paste.ubuntu.com/p/nhZzQkkG33/
> 

Yes, It looks that in linux the write protocol is set to SPI mode and it uses  0x12 for
PAGE PROGRAM operation. 
For is25wp256 flash device the SFDP table has NPH Field as 0x01 (number of parameter headers) i.e
just  has two parameter table's and the optional 4-byte address function specific table is not populated. 
Page Program capabilities of hardware are present in this table along with other information. Now since 
the spi_nor_parse_sfdp function doesn't get the 4BAIT table it setup's spi_nor with single Page 
program setting (i.e the page program opcode used for writing flash is 0x12)

So in case of linux adding the following code into xxxx_post_bfpt_fixup hooks updates the flash hwcaps and 
enable's flash write in QUAD mode

static int
 is25lp256_post_bfpt_fixups(struct spi_nor *nor,
                           const struct sfdp_parameter_header *bfpt_header,
@@ -2256,6 +2267,15 @@ is25lp256_post_bfpt_fixups(struct spi_nor *nor,
        if ((bfpt->dwords[BFPT_DWORD(1)] & BFPT_DWORD1_ADDRESS_BYTES_MASK) ==
                BFPT_DWORD1_ADDRESS_BYTES_3_ONLY)
                nor->addr_width = 4;
+       
+       if (params->hwcaps.mask != SNOR_HWCAPS_PP_1_1_4) {
+               struct spi_nor_pp_command *params_pp = params->page_programs;
+
+               params->hwcaps.mask |= SNOR_HWCAPS_PP_1_1_4;
+               spi_nor_set_pp_settings(&params->page_programs[SNOR_CMD_PP_1_1_4],
+                                         SPINOR_OP_PP_1_1_4,
+                                         SNOR_PROTO_1_1_4);
+       }

I tried these changes in linux, and did observe that flash write's happened in quad mode.

Thanks & BR,
Sagar Kadam

> Jagan.
  

Hi Sagar,

On Fri, Apr 17, 2020 at 11:50 PM Sagar Kadam <sagar.kadam@sifive.com> wrote:
>
> Hello Jagan,
>
> > -----Original Message-----
> > From: Jagan Teki <jagan@amarulasolutions.com>
> > Sent: Friday, April 17, 2020 1:30 PM
> > To: Sagar Kadam <sagar.kadam@sifive.com>
> > Cc: Bin Meng <bmeng.cn@gmail.com>; U-Boot Mailing List <u-
> > boot@lists.denx.de>; linux-amarula <linux-
> > amarula@amarulasolutions.com>; palmer@dabbelt.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 Tue, Apr 7, 2020 at 9:48 PM Sagar Kadam <sagar.kadam@sifive.com>
> > wrote:
> > >
> > > Hello Jagan,
> > >
> > > > -----Original Message-----
> > > > From: Jagan Teki <jagan@amarulasolutions.com>
> > > > Sent: Monday, April 6, 2020 9:30 PM
> > > > To: Sagar Kadam <sagar.kadam@sifive.com>
> > > > Cc: Bin Meng <bmeng.cn@gmail.com>; Palmer Dabbelt
> > > > <palmer@sifive.com>; U-Boot Mailing List <u-boot@lists.denx.de>;
> > > > linux- amarula <linux-amarula@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@sifive.com>
> > > > wrote:
> > > > >
> > > > > Hello Jagan,
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Jagan Teki <jagan@amarulasolutions.com>
> > > > > > Sent: Saturday, April 4, 2020 11:45 PM
> > > > > > To: Sagar Kadam <sagar.kadam@sifive.com>
> > > > > > Cc: Bin Meng <bmeng.cn@gmail.com>; Palmer Dabbelt
> > > > > > <palmer@sifive.com>; U-Boot Mailing List <u-boot@lists.denx.de>;
> > > > linux-
> > > > > > amarula <linux-amarula@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@sifive.com>
> > > > > > wrote:
> > > > > > >
> > > > > > >
> > > > > > > Hello Jagan/Bin,
> > > > > > >
> > > > > > > > -----Original Message-----
> > > > > > > > From: U-Boot <u-boot-bounces@lists.denx.de> On Behalf Of Bin
> > > > Meng
> > > > > > > > Sent: Monday, November 11, 2019 8:02 PM
> > > > > > > > To: Jagan Teki <jagan@amarulasolutions.com>
> > > > > > > > Cc: Palmer Dabbelt ( Sifive) <palmer@sifive.com>; U-Boot
> > > > > > > > Mailing
> > > > List
> > > > > > <u-
> > > > > > > > boot@lists.denx.de>; linux-amarula <linux-
> > > > > > amarula@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@amarulasolutions.com>
> > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > Hi Bin,
> > > > > > > > >
> > > > > > > > > On Tue, Oct 29, 2019 at 3:50 PM Bin Meng
> > > > > > > > > <bmeng.cn@gmail.com>
> > > > > > wrote:
> > > > > > > > > >
> > > > > > > > > > Hi Jagan,
> > > > > > > > > >
> > > > > > > > > > On Tue, Oct 29, 2019 at 5:38 PM Bin Meng
> > > > <bmeng.cn@gmail.com>
> > > > > > > > wrote:
> > > > > > > > > > >
> > > > > > > > > > > Hi Jagan,
> > > > > > > > > > >
> > > > > > > > > > > On Wed, Oct 16, 2019 at 10:58 PM Jagan Teki
> > > > > > > > <jagan@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@amarulasolutions.com>
> > > > > > > > > > > > Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> > > > > > > > > > > > Tested-by: Bin Meng <bmeng.cn@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.dts
> > > > > > > > > > > > +++ i
> > > > > > > > > > > > @@ -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=1
> > > > > > > > > > 29736
> > > > > > > > > >
> > > > > > > > > > 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).
> >
> > I did manage to get the tx or rx bits logic related to u-boot via buswidth. I
> > can see it is working with quad read and quad write is not working. Seems
> > like Quad write is also not working even for Linux any idea, why [2]?
> >
> > [2] https://paste.ubuntu.com/p/nhZzQkkG33/
> >
>
> Yes, It looks that in linux the write protocol is set to SPI mode and it uses  0x12 for
> PAGE PROGRAM operation.
> For is25wp256 flash device the SFDP table has NPH Field as 0x01 (number of parameter headers) i.e
> just  has two parameter table's and the optional 4-byte address function specific table is not populated.
> Page Program capabilities of hardware are present in this table along with other information. Now since
> the spi_nor_parse_sfdp function doesn't get the 4BAIT table it setup's spi_nor with single Page
> program setting (i.e the page program opcode used for writing flash is 0x12)
>
> So in case of linux adding the following code into xxxx_post_bfpt_fixup hooks updates the flash hwcaps and
> enable's flash write in QUAD mode
>
> static int
>  is25lp256_post_bfpt_fixups(struct spi_nor *nor,
>                            const struct sfdp_parameter_header *bfpt_header,
> @@ -2256,6 +2267,15 @@ is25lp256_post_bfpt_fixups(struct spi_nor *nor,
>         if ((bfpt->dwords[BFPT_DWORD(1)] & BFPT_DWORD1_ADDRESS_BYTES_MASK) ==
>                 BFPT_DWORD1_ADDRESS_BYTES_3_ONLY)
>                 nor->addr_width = 4;
> +
> +       if (params->hwcaps.mask != SNOR_HWCAPS_PP_1_1_4) {
> +               struct spi_nor_pp_command *params_pp = params->page_programs;
> +
> +               params->hwcaps.mask |= SNOR_HWCAPS_PP_1_1_4;
> +               spi_nor_set_pp_settings(&params->page_programs[SNOR_CMD_PP_1_1_4],
> +                                         SPINOR_OP_PP_1_1_4,
> +                                         SNOR_PROTO_1_1_4);
> +       }
>
> I tried these changes in linux, and did observe that flash write's happened in quad mode.

Thanks for the update, what about the SPI direction field incase of TX
QUAD, is the direction needed to set for all TX transactions or only
for QUAD?

Jagan.
  

Hello Jagan,

> -----Original Message-----
> From: Jagan Teki <jagan@amarulasolutions.com>
> Sent: Saturday, April 18, 2020 1:11 AM
> To: Sagar Kadam <sagar.kadam@sifive.com>
> Cc: Bin Meng <bmeng.cn@gmail.com>; U-Boot Mailing List <u-
> boot@lists.denx.de>; linux-amarula <linux-
> amarula@amarulasolutions.com>; palmer@dabbelt.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 Fri, Apr 17, 2020 at 11:50 PM Sagar Kadam <sagar.kadam@sifive.com>
> wrote:
> >
> > Hello Jagan,
> >
> > > -----Original Message-----
> > > From: Jagan Teki <jagan@amarulasolutions.com>
> > > Sent: Friday, April 17, 2020 1:30 PM
> > > To: Sagar Kadam <sagar.kadam@sifive.com>
> > > Cc: Bin Meng <bmeng.cn@gmail.com>; U-Boot Mailing List <u-
> > > boot@lists.denx.de>; linux-amarula <linux-
> > > amarula@amarulasolutions.com>; palmer@dabbelt.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 Tue, Apr 7, 2020 at 9:48 PM Sagar Kadam <sagar.kadam@sifive.com>
> > > wrote:
> > > >
> > > > Hello Jagan,
> > > >
> > > > > -----Original Message-----
> > > > > From: Jagan Teki <jagan@amarulasolutions.com>
> > > > > Sent: Monday, April 6, 2020 9:30 PM
> > > > > To: Sagar Kadam <sagar.kadam@sifive.com>
> > > > > Cc: Bin Meng <bmeng.cn@gmail.com>; Palmer Dabbelt
> > > > > <palmer@sifive.com>; U-Boot Mailing List <u-boot@lists.denx.de>;
> > > > > linux- amarula <linux-amarula@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@sifive.com>
> > > > > wrote:
> > > > > >
> > > > > > Hello Jagan,
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: Jagan Teki <jagan@amarulasolutions.com>
> > > > > > > Sent: Saturday, April 4, 2020 11:45 PM
> > > > > > > To: Sagar Kadam <sagar.kadam@sifive.com>
> > > > > > > Cc: Bin Meng <bmeng.cn@gmail.com>; Palmer Dabbelt
> > > > > > > <palmer@sifive.com>; U-Boot Mailing List <u-
> boot@lists.denx.de>;
> > > > > linux-
> > > > > > > amarula <linux-amarula@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@sifive.com>
> > > > > > > wrote:
> > > > > > > >
> > > > > > > >
> > > > > > > > Hello Jagan/Bin,
> > > > > > > >
> > > > > > > > > -----Original Message-----
> > > > > > > > > From: U-Boot <u-boot-bounces@lists.denx.de> On Behalf Of
> Bin
> > > > > Meng
> > > > > > > > > Sent: Monday, November 11, 2019 8:02 PM
> > > > > > > > > To: Jagan Teki <jagan@amarulasolutions.com>
> > > > > > > > > Cc: Palmer Dabbelt ( Sifive) <palmer@sifive.com>; U-Boot
> > > > > > > > > Mailing
> > > > > List
> > > > > > > <u-
> > > > > > > > > boot@lists.denx.de>; linux-amarula <linux-
> > > > > > > amarula@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@amarulasolutions.com>
> > > > > > > > > wrote:
> > > > > > > > > >
> > > > > > > > > > Hi Bin,
> > > > > > > > > >
> > > > > > > > > > On Tue, Oct 29, 2019 at 3:50 PM Bin Meng
> > > > > > > > > > <bmeng.cn@gmail.com>
> > > > > > > wrote:
> > > > > > > > > > >
> > > > > > > > > > > Hi Jagan,
> > > > > > > > > > >
> > > > > > > > > > > On Tue, Oct 29, 2019 at 5:38 PM Bin Meng
> > > > > <bmeng.cn@gmail.com>
> > > > > > > > > wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > Hi Jagan,
> > > > > > > > > > > >
> > > > > > > > > > > > On Wed, Oct 16, 2019 at 10:58 PM Jagan Teki
> > > > > > > > > <jagan@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@amarulasolutions.com>
> > > > > > > > > > > > > Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> > > > > > > > > > > > > Tested-by: Bin Meng <bmeng.cn@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.dts
> > > > > > > > > > > > > +++ i
> > > > > > > > > > > > > @@ -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=1
> > > > > > > > > > > 29736
> > > > > > > > > > >
> > > > > > > > > > > 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).
> > >
> > > I did manage to get the tx or rx bits logic related to u-boot via buswidth.
> I
> > > can see it is working with quad read and quad write is not working.
> Seems
> > > like Quad write is also not working even for Linux any idea, why [2]?
> > >
> > > [2] https://paste.ubuntu.com/p/nhZzQkkG33/
> > >
> >
> > Yes, It looks that in linux the write protocol is set to SPI mode and it uses
> 0x12 for
> > PAGE PROGRAM operation.
> > For is25wp256 flash device the SFDP table has NPH Field as 0x01 (number
> of parameter headers) i.e
> > just  has two parameter table's and the optional 4-byte address function
> specific table is not populated.
> > Page Program capabilities of hardware are present in this table along with
> other information. Now since
> > the spi_nor_parse_sfdp function doesn't get the 4BAIT table it setup's
> spi_nor with single Page
> > program setting (i.e the page program opcode used for writing flash is
> 0x12)
> >
> > So in case of linux adding the following code into xxxx_post_bfpt_fixup
> hooks updates the flash hwcaps and
> > enable's flash write in QUAD mode
> >
> > static int
> >  is25lp256_post_bfpt_fixups(struct spi_nor *nor,
> >                            const struct sfdp_parameter_header *bfpt_header,
> > @@ -2256,6 +2267,15 @@ is25lp256_post_bfpt_fixups(struct spi_nor
> *nor,
> >         if ((bfpt->dwords[BFPT_DWORD(1)] &
> BFPT_DWORD1_ADDRESS_BYTES_MASK) ==
> >                 BFPT_DWORD1_ADDRESS_BYTES_3_ONLY)
> >                 nor->addr_width = 4;
> > +
> > +       if (params->hwcaps.mask != SNOR_HWCAPS_PP_1_1_4) {
> > +               struct spi_nor_pp_command *params_pp = params-
> >page_programs;
> > +
> > +               params->hwcaps.mask |= SNOR_HWCAPS_PP_1_1_4;
> > +               spi_nor_set_pp_settings(&params-
> >page_programs[SNOR_CMD_PP_1_1_4],
> > +                                         SPINOR_OP_PP_1_1_4,
> > +                                         SNOR_PROTO_1_1_4);
> > +       }
> >
> > I tried these changes in linux, and did observe that flash write's happened
> in quad mode.
> 
> Thanks for the update, what about the SPI direction field incase of TX
> QUAD, is the direction needed to set for all TX transactions or only
> for QUAD?
> 
For our case i.e QUAD TX we need to set the TX direction bit and clear it during 
read.

BR,
Sagar Kadam

> Jagan.
  

Hi Sagar,

On Sat, Apr 18, 2020 at 4:05 PM Sagar Kadam <sagar.kadam@sifive.com> wrote:
>
> Hello Jagan,
>
> > -----Original Message-----
> > From: Jagan Teki <jagan@amarulasolutions.com>
> > Sent: Saturday, April 18, 2020 1:11 AM
> > To: Sagar Kadam <sagar.kadam@sifive.com>
> > Cc: Bin Meng <bmeng.cn@gmail.com>; U-Boot Mailing List <u-
> > boot@lists.denx.de>; linux-amarula <linux-
> > amarula@amarulasolutions.com>; palmer@dabbelt.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 Fri, Apr 17, 2020 at 11:50 PM Sagar Kadam <sagar.kadam@sifive.com>
> > wrote:
> > >
> > > Hello Jagan,
> > >
> > > > -----Original Message-----
> > > > From: Jagan Teki <jagan@amarulasolutions.com>
> > > > Sent: Friday, April 17, 2020 1:30 PM
> > > > To: Sagar Kadam <sagar.kadam@sifive.com>
> > > > Cc: Bin Meng <bmeng.cn@gmail.com>; U-Boot Mailing List <u-
> > > > boot@lists.denx.de>; linux-amarula <linux-
> > > > amarula@amarulasolutions.com>; palmer@dabbelt.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 Tue, Apr 7, 2020 at 9:48 PM Sagar Kadam <sagar.kadam@sifive.com>
> > > > wrote:
> > > > >
> > > > > Hello Jagan,
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Jagan Teki <jagan@amarulasolutions.com>
> > > > > > Sent: Monday, April 6, 2020 9:30 PM
> > > > > > To: Sagar Kadam <sagar.kadam@sifive.com>
> > > > > > Cc: Bin Meng <bmeng.cn@gmail.com>; Palmer Dabbelt
> > > > > > <palmer@sifive.com>; U-Boot Mailing List <u-boot@lists.denx.de>;
> > > > > > linux- amarula <linux-amarula@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@sifive.com>
> > > > > > wrote:
> > > > > > >
> > > > > > > Hello Jagan,
> > > > > > >
> > > > > > > > -----Original Message-----
> > > > > > > > From: Jagan Teki <jagan@amarulasolutions.com>
> > > > > > > > Sent: Saturday, April 4, 2020 11:45 PM
> > > > > > > > To: Sagar Kadam <sagar.kadam@sifive.com>
> > > > > > > > Cc: Bin Meng <bmeng.cn@gmail.com>; Palmer Dabbelt
> > > > > > > > <palmer@sifive.com>; U-Boot Mailing List <u-
> > boot@lists.denx.de>;
> > > > > > linux-
> > > > > > > > amarula <linux-amarula@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@sifive.com>
> > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > Hello Jagan/Bin,
> > > > > > > > >
> > > > > > > > > > -----Original Message-----
> > > > > > > > > > From: U-Boot <u-boot-bounces@lists.denx.de> On Behalf Of
> > Bin
> > > > > > Meng
> > > > > > > > > > Sent: Monday, November 11, 2019 8:02 PM
> > > > > > > > > > To: Jagan Teki <jagan@amarulasolutions.com>
> > > > > > > > > > Cc: Palmer Dabbelt ( Sifive) <palmer@sifive.com>; U-Boot
> > > > > > > > > > Mailing
> > > > > > List
> > > > > > > > <u-
> > > > > > > > > > boot@lists.denx.de>; linux-amarula <linux-
> > > > > > > > amarula@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@amarulasolutions.com>
> > > > > > > > > > wrote:
> > > > > > > > > > >
> > > > > > > > > > > Hi Bin,
> > > > > > > > > > >
> > > > > > > > > > > On Tue, Oct 29, 2019 at 3:50 PM Bin Meng
> > > > > > > > > > > <bmeng.cn@gmail.com>
> > > > > > > > wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > Hi Jagan,
> > > > > > > > > > > >
> > > > > > > > > > > > On Tue, Oct 29, 2019 at 5:38 PM Bin Meng
> > > > > > <bmeng.cn@gmail.com>
> > > > > > > > > > wrote:
> > > > > > > > > > > > >
> > > > > > > > > > > > > Hi Jagan,
> > > > > > > > > > > > >
> > > > > > > > > > > > > On Wed, Oct 16, 2019 at 10:58 PM Jagan Teki
> > > > > > > > > > <jagan@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@amarulasolutions.com>
> > > > > > > > > > > > > > Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> > > > > > > > > > > > > > Tested-by: Bin Meng <bmeng.cn@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.dts
> > > > > > > > > > > > > > +++ i
> > > > > > > > > > > > > > @@ -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=1
> > > > > > > > > > > > 29736
> > > > > > > > > > > >
> > > > > > > > > > > > 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).
> > > >
> > > > I did manage to get the tx or rx bits logic related to u-boot via buswidth.
> > I
> > > > can see it is working with quad read and quad write is not working.
> > Seems
> > > > like Quad write is also not working even for Linux any idea, why [2]?
> > > >
> > > > [2] https://paste.ubuntu.com/p/nhZzQkkG33/
> > > >
> > >
> > > Yes, It looks that in linux the write protocol is set to SPI mode and it uses
> > 0x12 for
> > > PAGE PROGRAM operation.
> > > For is25wp256 flash device the SFDP table has NPH Field as 0x01 (number
> > of parameter headers) i.e
> > > just  has two parameter table's and the optional 4-byte address function
> > specific table is not populated.
> > > Page Program capabilities of hardware are present in this table along with
> > other information. Now since
> > > the spi_nor_parse_sfdp function doesn't get the 4BAIT table it setup's
> > spi_nor with single Page
> > > program setting (i.e the page program opcode used for writing flash is
> > 0x12)
> > >
> > > So in case of linux adding the following code into xxxx_post_bfpt_fixup
> > hooks updates the flash hwcaps and
> > > enable's flash write in QUAD mode
> > >
> > > static int
> > >  is25lp256_post_bfpt_fixups(struct spi_nor *nor,
> > >                            const struct sfdp_parameter_header *bfpt_header,
> > > @@ -2256,6 +2267,15 @@ is25lp256_post_bfpt_fixups(struct spi_nor
> > *nor,
> > >         if ((bfpt->dwords[BFPT_DWORD(1)] &
> > BFPT_DWORD1_ADDRESS_BYTES_MASK) ==
> > >                 BFPT_DWORD1_ADDRESS_BYTES_3_ONLY)
> > >                 nor->addr_width = 4;
> > > +
> > > +       if (params->hwcaps.mask != SNOR_HWCAPS_PP_1_1_4) {
> > > +               struct spi_nor_pp_command *params_pp = params-
> > >page_programs;
> > > +
> > > +               params->hwcaps.mask |= SNOR_HWCAPS_PP_1_1_4;
> > > +               spi_nor_set_pp_settings(&params-
> > >page_programs[SNOR_CMD_PP_1_1_4],
> > > +                                         SPINOR_OP_PP_1_1_4,
> > > +                                         SNOR_PROTO_1_1_4);
> > > +       }
> > >
> > > I tried these changes in linux, and did observe that flash write's happened
> > in quad mode.
> >
> > Thanks for the update, what about the SPI direction field incase of TX
> > QUAD, is the direction needed to set for all TX transactions or only
> > for QUAD?
> >
> For our case i.e QUAD TX we need to set the TX direction bit and clear it during
> read.

Yes, I'm trying to do exactly the same. Can you try to check the
attached file (sorry it has prints, hope you can comment). It worked
with QPP and I can see quad read exception after reading 8K.
Unhandled exception: Load access fault
EPC: 00000000fffac14a TVAL: 0000000000002000

Let me know if you find any? by the way are you online in #u-boot?

Jagan.
  

Hi Jagan,

> -----Original Message-----
> From: Jagan Teki <jagan@amarulasolutions.com>
> Sent: Saturday, April 18, 2020 4:12 PM
> To: Sagar Kadam <sagar.kadam@sifive.com>
> Cc: Bin Meng <bmeng.cn@gmail.com>; U-Boot Mailing List <u-
> boot@lists.denx.de>; linux-amarula <linux-
> amarula@amarulasolutions.com>; palmer@dabbelt.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 Sat, Apr 18, 2020 at 4:05 PM Sagar Kadam <sagar.kadam@sifive.com>
> wrote:
> >
> > Hello Jagan,
> >
> > > -----Original Message-----
> > > From: Jagan Teki <jagan@amarulasolutions.com>
> > > Sent: Saturday, April 18, 2020 1:11 AM
> > > To: Sagar Kadam <sagar.kadam@sifive.com>
> > > Cc: Bin Meng <bmeng.cn@gmail.com>; U-Boot Mailing List <u-
> > > boot@lists.denx.de>; linux-amarula <linux-
> > > amarula@amarulasolutions.com>; palmer@dabbelt.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 Fri, Apr 17, 2020 at 11:50 PM Sagar Kadam
> <sagar.kadam@sifive.com>
> > > wrote:
> > > >
> > > > Hello Jagan,
> > > >
> > > > > -----Original Message-----
> > > > > From: Jagan Teki <jagan@amarulasolutions.com>
> > > > > Sent: Friday, April 17, 2020 1:30 PM
> > > > > To: Sagar Kadam <sagar.kadam@sifive.com>
> > > > > Cc: Bin Meng <bmeng.cn@gmail.com>; U-Boot Mailing List <u-
> > > > > boot@lists.denx.de>; linux-amarula <linux-
> > > > > amarula@amarulasolutions.com>; palmer@dabbelt.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 Tue, Apr 7, 2020 at 9:48 PM Sagar Kadam
> <sagar.kadam@sifive.com>
> > > > > wrote:
> > > > > >
> > > > > > Hello Jagan,
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: Jagan Teki <jagan@amarulasolutions.com>
> > > > > > > Sent: Monday, April 6, 2020 9:30 PM
> > > > > > > To: Sagar Kadam <sagar.kadam@sifive.com>
> > > > > > > Cc: Bin Meng <bmeng.cn@gmail.com>; Palmer Dabbelt
> > > > > > > <palmer@sifive.com>; U-Boot Mailing List <u-
> boot@lists.denx.de>;
> > > > > > > linux- amarula <linux-amarula@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@sifive.com>
> > > > > > > wrote:
> > > > > > > >
> > > > > > > > Hello Jagan,
> > > > > > > >
> > > > > > > > > -----Original Message-----
> > > > > > > > > From: Jagan Teki <jagan@amarulasolutions.com>
> > > > > > > > > Sent: Saturday, April 4, 2020 11:45 PM
> > > > > > > > > To: Sagar Kadam <sagar.kadam@sifive.com>
> > > > > > > > > Cc: Bin Meng <bmeng.cn@gmail.com>; Palmer Dabbelt
> > > > > > > > > <palmer@sifive.com>; U-Boot Mailing List <u-
> > > boot@lists.denx.de>;
> > > > > > > linux-
> > > > > > > > > amarula <linux-amarula@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@sifive.com>
> > > > > > > > > wrote:
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > Hello Jagan/Bin,
> > > > > > > > > >
> > > > > > > > > > > -----Original Message-----
> > > > > > > > > > > From: U-Boot <u-boot-bounces@lists.denx.de> On Behalf
> Of
> > > Bin
> > > > > > > Meng
> > > > > > > > > > > Sent: Monday, November 11, 2019 8:02 PM
> > > > > > > > > > > To: Jagan Teki <jagan@amarulasolutions.com>
> > > > > > > > > > > Cc: Palmer Dabbelt ( Sifive) <palmer@sifive.com>; U-Boot
> > > > > > > > > > > Mailing
> > > > > > > List
> > > > > > > > > <u-
> > > > > > > > > > > boot@lists.denx.de>; linux-amarula <linux-
> > > > > > > > > amarula@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@amarulasolutions.com>
> > > > > > > > > > > wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > Hi Bin,
> > > > > > > > > > > >
> > > > > > > > > > > > On Tue, Oct 29, 2019 at 3:50 PM Bin Meng
> > > > > > > > > > > > <bmeng.cn@gmail.com>
> > > > > > > > > wrote:
> > > > > > > > > > > > >
> > > > > > > > > > > > > Hi Jagan,
> > > > > > > > > > > > >
> > > > > > > > > > > > > On Tue, Oct 29, 2019 at 5:38 PM Bin Meng
> > > > > > > <bmeng.cn@gmail.com>
> > > > > > > > > > > wrote:
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > Hi Jagan,
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > On Wed, Oct 16, 2019 at 10:58 PM Jagan Teki
> > > > > > > > > > > <jagan@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@amarulasolutions.com>
> > > > > > > > > > > > > > > Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> > > > > > > > > > > > > > > Tested-by: Bin Meng <bmeng.cn@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.dts
> > > > > > > > > > > > > > > +++ i
> > > > > > > > > > > > > > > @@ -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=1
> > > > > > > > > > > > > 29736
> > > > > > > > > > > > >
> > > > > > > > > > > > > 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).
> > > > >
> > > > > I did manage to get the tx or rx bits logic related to u-boot via
> buswidth.
> > > I
> > > > > can see it is working with quad read and quad write is not working.
> > > Seems
> > > > > like Quad write is also not working even for Linux any idea, why [2]?
> > > > >
> > > > > [2] https://paste.ubuntu.com/p/nhZzQkkG33/
> > > > >
> > > >
> > > > Yes, It looks that in linux the write protocol is set to SPI mode and it
> uses
> > > 0x12 for
> > > > PAGE PROGRAM operation.
> > > > For is25wp256 flash device the SFDP table has NPH Field as 0x01
> (number
> > > of parameter headers) i.e
> > > > just  has two parameter table's and the optional 4-byte address
> function
> > > specific table is not populated.
> > > > Page Program capabilities of hardware are present in this table along
> with
> > > other information. Now since
> > > > the spi_nor_parse_sfdp function doesn't get the 4BAIT table it setup's
> > > spi_nor with single Page
> > > > program setting (i.e the page program opcode used for writing flash is
> > > 0x12)
> > > >
> > > > So in case of linux adding the following code into
> xxxx_post_bfpt_fixup
> > > hooks updates the flash hwcaps and
> > > > enable's flash write in QUAD mode
> > > >
> > > > static int
> > > >  is25lp256_post_bfpt_fixups(struct spi_nor *nor,
> > > >                            const struct sfdp_parameter_header *bfpt_header,
> > > > @@ -2256,6 +2267,15 @@ is25lp256_post_bfpt_fixups(struct spi_nor
> > > *nor,
> > > >         if ((bfpt->dwords[BFPT_DWORD(1)] &
> > > BFPT_DWORD1_ADDRESS_BYTES_MASK) ==
> > > >                 BFPT_DWORD1_ADDRESS_BYTES_3_ONLY)
> > > >                 nor->addr_width = 4;
> > > > +
> > > > +       if (params->hwcaps.mask != SNOR_HWCAPS_PP_1_1_4) {
> > > > +               struct spi_nor_pp_command *params_pp = params-
> > > >page_programs;
> > > > +
> > > > +               params->hwcaps.mask |= SNOR_HWCAPS_PP_1_1_4;
> > > > +               spi_nor_set_pp_settings(&params-
> > > >page_programs[SNOR_CMD_PP_1_1_4],
> > > > +                                         SPINOR_OP_PP_1_1_4,
> > > > +                                         SNOR_PROTO_1_1_4);
> > > > +       }
> > > >
> > > > I tried these changes in linux, and did observe that flash write's
> happened
> > > in quad mode.
> > >
> > > Thanks for the update, what about the SPI direction field incase of TX
> > > QUAD, is the direction needed to set for all TX transactions or only
> > > for QUAD?
> > >
> > For our case i.e QUAD TX we need to set the TX direction bit and clear it
> during
> > read.
> 
> Yes, I'm trying to do exactly the same. Can you try to check the
> attached file (sorry it has prints, hope you can comment). It worked
> with QPP and I can see quad read exception after reading 8K.
> Unhandled exception: Load access fault
> EPC: 00000000fffac14a TVAL: 0000000000002000
> 
> Let me know if you find any? by the way are you online in #u-boot?
> 

Thanks, I will check your file. And yes, I will ping you on #u-boot channel.

> Jagan.