[PATCH v3 0/3] riscv: sifive/fu540: Enable SPI-NOR support

[PATCH v3 0/3] riscv: sifive/fu540: Enable SPI-NOR support

[Jagan Teki]

This is series v3 for SPI-NOR support on SiFive FU540 
platform with HiFive Unleashed board.

Here is the previous version changes[1].

Changes for v3:
- fixed QPP support
- dropped sf commands log

Depends on:
https://patchwork.ozlabs.org/project/uboot/list/?series=171332
https://patchwork.ozlabs.org/project/uboot/list/?series=171338
https://patchwork.ozlabs.org/project/uboot/list/?series=171355

Tested on:
- Whole flash area.
- Boot from MMC
- Boot from SPI

[1] https://patchwork.ozlabs.org/project/uboot/list/?series=136556

Any inputs?
Jagan.

Jagan Teki (3):
  spi: sifive: Fix QPP transfer
  riscv: dts: hifive-unleashed-a00: Add -u-boot.dtsi
  sifive: fu540: Enable spi-nor flash support

 .../dts/hifive-unleashed-a00-u-boot.dtsi      | 11 ++++
 board/sifive/fu540/Kconfig                    |  3 +
 drivers/spi/spi-sifive.c                      | 57 ++++++++++++-------
 3 files changed, 51 insertions(+), 20 deletions(-)
 create mode 100644 arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi

-- 
2.17.1

[PATCH v3 1/3] spi: sifive: Fix QPP transfer

[Jagan Teki]

For historical reasons the existing logic of filling tx fifo
with data, rx fifo with NULL for tx transfer and filling rx
fifo with data, tx fifo with NULL for rx transfer is not
clear enough to support the Quad Page Program.
? ?
SiFive SPI controllers have specific sets of watermark
registers and SPI I/O directions bits in order to program
SPI controllers clear enough to support all sets of operating
modes.
? ?
Here is the exact programing sequence that would follow on this
patch and tested via SPI-NOR and MMC_SPI.
? ?
- set the frame format proto, endian
- set the frame format dir, set it for tx and clear it for rx
- TX transfer:
? fill tx fifo with data.
? wait for TX watermark bit to clear.
- TX transfer:
? fill tx fifo with 0xff.
? write nbytes to rx watermark register
? wait for rx watermark bit to clear.
? read the rx fifo data.

So, this patch adopts this program sequence and fixes the existing
I/O direction bit.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
Changes for v3:
- new patch

 drivers/spi/spi-sifive.c | 57 ++++++++++++++++++++++++++--------------
 1 file changed, 37 insertions(+), 20 deletions(-)

diff --git a/drivers/spi/spi-sifive.c b/drivers/spi/spi-sifive.c
index 336b683476..2a0b28dc08 100644
--- a/drivers/spi/spi-sifive.c
+++ b/drivers/spi/spi-sifive.c
@@ -10,6 +10,7 @@
 #include <dm.h>
 #include <malloc.h>
 #include <spi.h>
+#include <wait_bit.h>
 #include <asm/io.h>
 #include <linux/log2.h>
 #include <clk.h>
@@ -127,8 +128,8 @@ static void sifive_spi_clear_cs(struct sifive_spi *spi)
 }
 
 static void sifive_spi_prep_transfer(struct sifive_spi *spi,
-				     bool is_rx_xfer,
-				     struct dm_spi_slave_platdata *slave_plat)
+				     struct dm_spi_slave_platdata *slave_plat,
+				     u8 *rx_ptr)
 {
 	u32 cr;
 
@@ -160,7 +161,7 @@ static void sifive_spi_prep_transfer(struct sifive_spi *spi,
 
 	/* SPI direction in/out ? */
 	cr &= ~SIFIVE_SPI_FMT_DIR;
-	if (!is_rx_xfer)
+	if (!rx_ptr)
 		cr |= SIFIVE_SPI_FMT_DIR;
 
 	writel(cr, spi->regs + SIFIVE_SPI_REG_FMT);
@@ -191,13 +192,19 @@ static void sifive_spi_tx(struct sifive_spi *spi, const u8 *tx_ptr)
 	writel(tx_data, spi->regs + SIFIVE_SPI_REG_TXDATA);
 }
 
+static int sifive_spi_wait(struct sifive_spi *spi, u32 bit)
+{
+	return wait_for_bit_le32(spi->regs + SIFIVE_SPI_REG_IP,
+				 bit, true, 100, false);
+}
+
 static int sifive_spi_xfer(struct udevice *dev, unsigned int bitlen,
 			   const void *dout, void *din, unsigned long flags)
 {
 	struct udevice *bus = dev->parent;
 	struct sifive_spi *spi = dev_get_priv(bus);
 	struct dm_spi_slave_platdata *slave_plat = dev_get_parent_platdata(dev);
-	const unsigned char *tx_ptr = dout;
+	const u8 *tx_ptr = dout;
 	u8 *rx_ptr = din;
 	u32 remaining_len;
 	int ret;
@@ -210,31 +217,37 @@ static int sifive_spi_xfer(struct udevice *dev, unsigned int bitlen,
 			return ret;
 	}
 
-	sifive_spi_prep_transfer(spi, true, slave_plat);
+	sifive_spi_prep_transfer(spi, slave_plat, rx_ptr);
 
 	remaining_len = bitlen / 8;
 
 	while (remaining_len) {
-		int n_words, tx_words, rx_words;
-
-		n_words = min(remaining_len, spi->fifo_depth);
+		unsigned int n_words = min(remaining_len, spi->fifo_depth);
+		unsigned int tx_words, rx_words;
 
 		/* Enqueue n_words for transmission */
-		if (tx_ptr) {
-			for (tx_words = 0; tx_words < n_words; ++tx_words) {
-				sifive_spi_tx(spi, tx_ptr);
-				sifive_spi_rx(spi, NULL);
-				tx_ptr++;
-			}
+		for (tx_words = 0; tx_words < n_words; tx_words++) {
+			if (!tx_ptr)
+				sifive_spi_tx(spi, NULL);
+			else
+				sifive_spi_tx(spi, tx_ptr++);
 		}
 
-		/* Read out all the data from the RX FIFO */
 		if (rx_ptr) {
-			for (rx_words = 0; rx_words < n_words; ++rx_words) {
-				sifive_spi_tx(spi, NULL);
-				sifive_spi_rx(spi, rx_ptr);
-				rx_ptr++;
-			}
+			/* Wait for transmission + reception to complete */
+			writel(n_words - 1, spi->regs + SIFIVE_SPI_REG_RXMARK);
+			ret = sifive_spi_wait(spi, SIFIVE_SPI_IP_RXWM);
+			if (ret)
+				return ret;
+
+			/* Read out all the data from the RX FIFO */
+			for (rx_words = 0; rx_words < n_words; rx_words++)
+				sifive_spi_rx(spi, rx_ptr++);
+		} else {
+			/* Wait for transmission to complete */
+			ret = sifive_spi_wait(spi, SIFIVE_SPI_IP_TXWM);
+			if (ret)
+				return ret;
 		}
 
 		remaining_len -= n_words;
@@ -314,6 +327,10 @@ static void sifive_spi_init_hw(struct sifive_spi *spi)
 	/* Watermark interrupts are disabled by default */
 	writel(0, spi->regs + SIFIVE_SPI_REG_IE);
 
+	/* Default watermark FIFO threshold values */
+	writel(1, spi->regs + SIFIVE_SPI_REG_TXMARK);
+	writel(0, spi->regs + SIFIVE_SPI_REG_RXMARK);
+
 	/* Set CS/SCK Delays and Inactive Time to defaults */
 	writel(SIFIVE_SPI_DELAY0_CSSCK(1) | SIFIVE_SPI_DELAY0_SCKCS(1),
 	       spi->regs + SIFIVE_SPI_REG_DELAY0);
-- 
2.17.1

[PATCH v3 2/3] riscv: dts: hifive-unleashed-a00: Add -u-boot.dtsi

[Jagan Teki]

Add U-Boot specific dts file for hifive-unleashed-a00, this
would help to add u-boot specific properties and other node
changes without touching the base dts(i) files which are easy
to sync from Linux.

Added spi2 alias for qspi2 as an initial u-boot specific
property change.

spi probing in current dm model is very much rely on aliases
numbering. Even though the qspi2 can't come under any associated
spi nor flash it would require to specify the same to make proper
binding happen for other spi slaves.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
---
Changes for v3:
- none 

 arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi | 10 ++++++++++
 1 file changed, 10 insertions(+)
 create mode 100644 arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi

diff --git a/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi b/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi
new file mode 100644
index 0000000000..25ec8265a5
--- /dev/null
+++ b/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi
@@ -0,0 +1,10 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2019 Jagan Teki <jagan@amarulasolutions.com>
+ */
+
+/ {
+	aliases {
+		spi2 = &qspi2;
+	};
+};
-- 
2.17.1

[PATCH v3 3/3] sifive: fu540: Enable spi-nor flash support

[Jagan Teki]

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>
---
Changes for v3:
- drop sf commands log

 arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi | 1 +
 board/sifive/fu540/Kconfig                      | 3 +++
 2 files changed, 4 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 5ca21474de..75661f35f8 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
-- 
2.17.1

[PATCH v3 1/3] spi: sifive: Fix QPP transfer

[Bin Meng]

Hi Jagan,

On Mon, Apr 20, 2020 at 8:52 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
>
> For historical reasons the existing logic of filling tx fifo

What historical reasons?

> with data, rx fifo with NULL for tx transfer and filling rx
> fifo with data, tx fifo with NULL for rx transfer is not
> clear enough to support the Quad Page Program.
>
> SiFive SPI controllers have specific sets of watermark
> registers and SPI I/O directions bits in order to program
> SPI controllers clear enough to support all sets of operating
> modes.
>
> Here is the exact programing sequence that would follow on this
> patch and tested via SPI-NOR and MMC_SPI.
>
> - set the frame format proto, endian
> - set the frame format dir, set it for tx and clear it for rx
> - TX transfer:
>   fill tx fifo with data.
>   wait for TX watermark bit to clear.
> - TX transfer:

RX transfer ?

>   fill tx fifo with 0xff.

rx fifo ?

>   write nbytes to rx watermark register
>   wait for rx watermark bit to clear.
>   read the rx fifo data.
>
> So, this patch adopts this program sequence and fixes the existing
> I/O direction bit.
>
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> ---
> Changes for v3:
> - new patch
>
>  drivers/spi/spi-sifive.c | 57 ++++++++++++++++++++++++++--------------
>  1 file changed, 37 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/spi/spi-sifive.c b/drivers/spi/spi-sifive.c
> index 336b683476..2a0b28dc08 100644
> --- a/drivers/spi/spi-sifive.c
> +++ b/drivers/spi/spi-sifive.c
> @@ -10,6 +10,7 @@
>  #include <dm.h>
>  #include <malloc.h>
>  #include <spi.h>
> +#include <wait_bit.h>
>  #include <asm/io.h>
>  #include <linux/log2.h>
>  #include <clk.h>
> @@ -127,8 +128,8 @@ static void sifive_spi_clear_cs(struct sifive_spi *spi)
>  }
>
>  static void sifive_spi_prep_transfer(struct sifive_spi *spi,
> -                                    bool is_rx_xfer,
> -                                    struct dm_spi_slave_platdata *slave_plat)
> +                                    struct dm_spi_slave_platdata *slave_plat,
> +                                    u8 *rx_ptr)
>  {
>         u32 cr;
>
> @@ -160,7 +161,7 @@ static void sifive_spi_prep_transfer(struct sifive_spi *spi,
>
>         /* SPI direction in/out ? */
>         cr &= ~SIFIVE_SPI_FMT_DIR;
> -       if (!is_rx_xfer)
> +       if (!rx_ptr)
>                 cr |= SIFIVE_SPI_FMT_DIR;
>
>         writel(cr, spi->regs + SIFIVE_SPI_REG_FMT);
> @@ -191,13 +192,19 @@ static void sifive_spi_tx(struct sifive_spi *spi, const u8 *tx_ptr)
>         writel(tx_data, spi->regs + SIFIVE_SPI_REG_TXDATA);
>  }
>
> +static int sifive_spi_wait(struct sifive_spi *spi, u32 bit)
> +{
> +       return wait_for_bit_le32(spi->regs + SIFIVE_SPI_REG_IP,
> +                                bit, true, 100, false);
> +}
> +
>  static int sifive_spi_xfer(struct udevice *dev, unsigned int bitlen,
>                            const void *dout, void *din, unsigned long flags)
>  {
>         struct udevice *bus = dev->parent;
>         struct sifive_spi *spi = dev_get_priv(bus);
>         struct dm_spi_slave_platdata *slave_plat = dev_get_parent_platdata(dev);
> -       const unsigned char *tx_ptr = dout;
> +       const u8 *tx_ptr = dout;
>         u8 *rx_ptr = din;
>         u32 remaining_len;
>         int ret;
> @@ -210,31 +217,37 @@ static int sifive_spi_xfer(struct udevice *dev, unsigned int bitlen,
>                         return ret;
>         }
>
> -       sifive_spi_prep_transfer(spi, true, slave_plat);
> +       sifive_spi_prep_transfer(spi, slave_plat, rx_ptr);
>
>         remaining_len = bitlen / 8;
>
>         while (remaining_len) {
> -               int n_words, tx_words, rx_words;
> -
> -               n_words = min(remaining_len, spi->fifo_depth);
> +               unsigned int n_words = min(remaining_len, spi->fifo_depth);
> +               unsigned int tx_words, rx_words;
>
>                 /* Enqueue n_words for transmission */
> -               if (tx_ptr) {
> -                       for (tx_words = 0; tx_words < n_words; ++tx_words) {
> -                               sifive_spi_tx(spi, tx_ptr);
> -                               sifive_spi_rx(spi, NULL);
> -                               tx_ptr++;
> -                       }
> +               for (tx_words = 0; tx_words < n_words; tx_words++) {
> +                       if (!tx_ptr)
> +                               sifive_spi_tx(spi, NULL);
> +                       else
> +                               sifive_spi_tx(spi, tx_ptr++);
>                 }
>
> -               /* Read out all the data from the RX FIFO */
>                 if (rx_ptr) {
> -                       for (rx_words = 0; rx_words < n_words; ++rx_words) {
> -                               sifive_spi_tx(spi, NULL);
> -                               sifive_spi_rx(spi, rx_ptr);
> -                               rx_ptr++;
> -                       }
> +                       /* Wait for transmission + reception to complete */
> +                       writel(n_words - 1, spi->regs + SIFIVE_SPI_REG_RXMARK);
> +                       ret = sifive_spi_wait(spi, SIFIVE_SPI_IP_RXWM);
> +                       if (ret)
> +                               return ret;
> +
> +                       /* Read out all the data from the RX FIFO */
> +                       for (rx_words = 0; rx_words < n_words; rx_words++)
> +                               sifive_spi_rx(spi, rx_ptr++);
> +               } else {
> +                       /* Wait for transmission to complete */
> +                       ret = sifive_spi_wait(spi, SIFIVE_SPI_IP_TXWM);
> +                       if (ret)
> +                               return ret;
>                 }
>
>                 remaining_len -= n_words;
> @@ -314,6 +327,10 @@ static void sifive_spi_init_hw(struct sifive_spi *spi)
>         /* Watermark interrupts are disabled by default */
>         writel(0, spi->regs + SIFIVE_SPI_REG_IE);
>
> +       /* Default watermark FIFO threshold values */
> +       writel(1, spi->regs + SIFIVE_SPI_REG_TXMARK);
> +       writel(0, spi->regs + SIFIVE_SPI_REG_RXMARK);
> +
>         /* Set CS/SCK Delays and Inactive Time to defaults */
>         writel(SIFIVE_SPI_DELAY0_CSSCK(1) | SIFIVE_SPI_DELAY0_SCKCS(1),
>                spi->regs + SIFIVE_SPI_REG_DELAY0);

Regards,
Bin

[PATCH v3 2/3] riscv: dts: hifive-unleashed-a00: Add -u-boot.dtsi

[Bin Meng]

On Mon, Apr 20, 2020 at 8:52 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
>
> Add U-Boot specific dts file for hifive-unleashed-a00, this
> would help to add u-boot specific properties and other node
> changes without touching the base dts(i) files which are easy
> to sync from Linux.
>
> Added spi2 alias for qspi2 as an initial u-boot specific
> property change.
>
> spi probing in current dm model is very much rely on aliases
> numbering. Even though the qspi2 can't come under any associated
> spi nor flash it would require to specify the same to make proper
> binding happen for other spi slaves.
>
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> ---
> Changes for v3:
> - none
>
>  arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>  create mode 100644 arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi
>
> diff --git a/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi b/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi
> new file mode 100644
> index 0000000000..25ec8265a5
> --- /dev/null
> +++ b/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi
> @@ -0,0 +1,10 @@
> +// SPDX-License-Identifier: GPL-2.0+

Please dual-license this file, as other SiFive fu540 dts files do.

> +/*
> + * Copyright (C) 2019 Jagan Teki <jagan@amarulasolutions.com>
> + */
> +
> +/ {
> +       aliases {
> +               spi2 = &qspi2;
> +       };
> +};

Regards,
Bin

[PATCH v3 3/3] sifive: fu540: Enable spi-nor flash support

[Bin Meng]

On Mon, Apr 20, 2020 at 8:52 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>
> ---
> Changes for v3:
> - drop sf commands log
>
>  arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi | 1 +
>  board/sifive/fu540/Kconfig                      | 3 +++
>  2 files changed, 4 insertions(+)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

[PATCH v3 1/3] spi: sifive: Fix QPP transfer

[Jagan Teki]

On Mon, Apr 20, 2020 at 7:31 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Jagan,
>
> On Mon, Apr 20, 2020 at 8:52 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
> >
> > For historical reasons the existing logic of filling tx fifo
>
> What historical reasons?

No real idea, seems like exciting logic is developed to make mmc_spi workable.

>
> > with data, rx fifo with NULL for tx transfer and filling rx
> > fifo with data, tx fifo with NULL for rx transfer is not
> > clear enough to support the Quad Page Program.
> >
> > SiFive SPI controllers have specific sets of watermark
> > registers and SPI I/O directions bits in order to program
> > SPI controllers clear enough to support all sets of operating
> > modes.
> >
> > Here is the exact programing sequence that would follow on this
> > patch and tested via SPI-NOR and MMC_SPI.
> >
> > - set the frame format proto, endian
> > - set the frame format dir, set it for tx and clear it for rx
> > - TX transfer:
> >   fill tx fifo with data.
> >   wait for TX watermark bit to clear.
> > - TX transfer:
>
> RX transfer ?

Thanks, my bad.

>
> >   fill tx fifo with 0xff.
>
> rx fifo ?

No it is rx fifo in order to read the rx fifo we need to pass 0xff to
tx fifo, typical SPI protocol.

Jagan.

[PATCH v3 1/3] spi: sifive: Fix QPP transfer

[Bin Meng]

Hi Jagan,

On Mon, Apr 20, 2020 at 10:09 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
>
> On Mon, Apr 20, 2020 at 7:31 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > Hi Jagan,
> >
> > On Mon, Apr 20, 2020 at 8:52 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
> > >
> > > For historical reasons the existing logic of filling tx fifo
> >
> > What historical reasons?
>
> No real idea, seems like exciting logic is developed to make mmc_spi workable.

Could you put such "guessed reason" other than "historical" in the
commit message, to help better understanding?

>
> >
> > > with data, rx fifo with NULL for tx transfer and filling rx
> > > fifo with data, tx fifo with NULL for rx transfer is not
> > > clear enough to support the Quad Page Program.
> > >
> > > SiFive SPI controllers have specific sets of watermark
> > > registers and SPI I/O directions bits in order to program
> > > SPI controllers clear enough to support all sets of operating
> > > modes.
> > >
> > > Here is the exact programing sequence that would follow on this
> > > patch and tested via SPI-NOR and MMC_SPI.
> > >
> > > - set the frame format proto, endian
> > > - set the frame format dir, set it for tx and clear it for rx
> > > - TX transfer:
> > >   fill tx fifo with data.
> > >   wait for TX watermark bit to clear.
> > > - TX transfer:
> >
> > RX transfer ?
>
> Thanks, my bad.
>
> >
> > >   fill tx fifo with 0xff.
> >
> > rx fifo ?
>
> No it is rx fifo in order to read the rx fifo we need to pass 0xff to

I assume here it is tx fifo

> tx fifo, typical SPI protocol.

Regards
Bin

[PATCH v3 1/3] spi: sifive: Fix QPP transfer

[Jagan Teki]

Hi Bin,

On Mon, Apr 20, 2020 at 7:45 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Jagan,
>
> On Mon, Apr 20, 2020 at 10:09 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
> >
> > On Mon, Apr 20, 2020 at 7:31 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> > >
> > > Hi Jagan,
> > >
> > > On Mon, Apr 20, 2020 at 8:52 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
> > > >
> > > > For historical reasons the existing logic of filling tx fifo
> > >
> > > What historical reasons?
> >
> > No real idea, seems like exciting logic is developed to make mmc_spi workable.
>
> Could you put such "guessed reason" other than "historical" in the
> commit message, to help better understanding?

Okay.

>
> >
> > >
> > > > with data, rx fifo with NULL for tx transfer and filling rx
> > > > fifo with data, tx fifo with NULL for rx transfer is not
> > > > clear enough to support the Quad Page Program.
> > > >
> > > > SiFive SPI controllers have specific sets of watermark
> > > > registers and SPI I/O directions bits in order to program
> > > > SPI controllers clear enough to support all sets of operating
> > > > modes.
> > > >
> > > > Here is the exact programing sequence that would follow on this
> > > > patch and tested via SPI-NOR and MMC_SPI.
> > > >
> > > > - set the frame format proto, endian
> > > > - set the frame format dir, set it for tx and clear it for rx
> > > > - TX transfer:
> > > >   fill tx fifo with data.
> > > >   wait for TX watermark bit to clear.
> > > > - TX transfer:
> > >
> > > RX transfer ?
> >
> > Thanks, my bad.
> >
> > >
> > > >   fill tx fifo with 0xff.
> > >
> > > rx fifo ?
> >
> > No it is rx fifo in order to read the rx fifo we need to pass 0xff to
>
> I assume here it is tx fifo

Yes it is tx fifo.