All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [UBOOT PATCH v3 0/3] spi:xilinx_spi: Modify xilinx spi driver
@ 2018-06-21  9:23 Vipul Kumar
  2018-06-21  9:23 ` [U-Boot] [UBOOT PATCH v3 1/3] spi: xilinx: Read reg base address from DTS file Vipul Kumar
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Vipul Kumar @ 2018-06-21  9:23 UTC (permalink / raw)
  To: u-boot

This series of patches do the following:
- This patch added support to get reg base address from DTS file
- Added rxfifo() and txfifo() functions to add the modularity
- Added support to read JEDEC-id twice at the boot time

Changes in v2:
- Split single patch into the series of patches

Changes in V3:
- Read reg in probe function
- Removed xilinx_spi_ofdata_to_platdata function
- Added fifo_depth read support in 2/3 and removed from 1/3

Michal Simek (1):
  spi: xilinx: Read reg base address from DTS file

Vipul Kumar (2):
  spi: xilinx_spi: Modify transfer logic xilinx_spi_xfer() function
  spi: xilinx_spi: Added support to read JEDEC-id twice at the boot time

 drivers/spi/xilinx_spi.c | 153 ++++++++++++++++++++++++++++++++++-------------
 1 file changed, 111 insertions(+), 42 deletions(-)

--
2.7.4

This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [UBOOT PATCH v3 1/3] spi: xilinx: Read reg base address from DTS file
  2018-06-21  9:23 [U-Boot] [UBOOT PATCH v3 0/3] spi:xilinx_spi: Modify xilinx spi driver Vipul Kumar
@ 2018-06-21  9:23 ` Vipul Kumar
  2018-06-25 10:07   ` Jagan Teki
  2018-06-21  9:23 ` [U-Boot] [UBOOT PATCH v3 2/3] spi: xilinx_spi: Modify transfer logic xilinx_spi_xfer() function Vipul Kumar
  2018-06-21  9:23 ` [U-Boot] [UBOOT PATCH v3 3/3] spi: xilinx_spi: Added support to read JEDEC-id twice at the boot time Vipul Kumar
  2 siblings, 1 reply; 12+ messages in thread
From: Vipul Kumar @ 2018-06-21  9:23 UTC (permalink / raw)
  To: u-boot

From: Michal Simek <michal.simek@xilinx.com>

This patch added support to read register base address
from DTS file.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
Signed-off-by: Vipul Kumar <vipul.kumar@xilinx.com>
---
Changes in v3:
- Read reg in probe function
- Removed xilinx_spi_ofdata_to_platdata function
- Removed reading of fifo_depth
---
 drivers/spi/xilinx_spi.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/spi/xilinx_spi.c b/drivers/spi/xilinx_spi.c
index 8f0f32f..cc5ac51 100644
--- a/drivers/spi/xilinx_spi.c
+++ b/drivers/spi/xilinx_spi.c
@@ -77,10 +77,6 @@
 #define CONFIG_XILINX_SPI_IDLE_VAL     GENMASK(7, 0)
 #endif

-#ifndef CONFIG_SYS_XILINX_SPI_LIST
-#define CONFIG_SYS_XILINX_SPI_LIST     { CONFIG_SYS_SPI_BASE }
-#endif
-
 /* xilinx spi register set */
 struct xilinx_spi_regs {
        u32 __space0__[7];
@@ -107,13 +103,12 @@ struct xilinx_spi_priv {
        unsigned int mode;
 };

-static unsigned long xilinx_spi_base_list[] = CONFIG_SYS_XILINX_SPI_LIST;
 static int xilinx_spi_probe(struct udevice *bus)
 {
        struct xilinx_spi_priv *priv = dev_get_priv(bus);
        struct xilinx_spi_regs *regs = priv->regs;

-       priv->regs = (struct xilinx_spi_regs *)xilinx_spi_base_list[bus->seq];
+       priv->regs = (struct xilinx_spi_regs *)devfdt_get_addr(bus);

        writel(SPISSR_RESET_VALUE, &regs->srr);

--
2.7.4

This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [U-Boot] [UBOOT PATCH v3 2/3] spi: xilinx_spi: Modify transfer logic xilinx_spi_xfer() function
  2018-06-21  9:23 [U-Boot] [UBOOT PATCH v3 0/3] spi:xilinx_spi: Modify xilinx spi driver Vipul Kumar
  2018-06-21  9:23 ` [U-Boot] [UBOOT PATCH v3 1/3] spi: xilinx: Read reg base address from DTS file Vipul Kumar
@ 2018-06-21  9:23 ` Vipul Kumar
  2018-06-25 10:11   ` Jagan Teki
  2018-06-21  9:23 ` [U-Boot] [UBOOT PATCH v3 3/3] spi: xilinx_spi: Added support to read JEDEC-id twice at the boot time Vipul Kumar
  2 siblings, 1 reply; 12+ messages in thread
From: Vipul Kumar @ 2018-06-21  9:23 UTC (permalink / raw)
  To: u-boot

This patch modify xilinx_spi_xfer() function and add rxfifo() and
txfifo() functions to add the modularity so that these functions
can be used by other functions within the same file.

This patch also added support to read fifo_size from dts.

Signed-off-by: Vipul Kumar <vipul.kumar@xilinx.com>
Signed-off-by: Siva Durga Prasad Paladugu <siva.durga.paladugu@xilinx.com>
---
- Changes in v3:
- Added fifo_depth read support and removed from 1/3
---
 drivers/spi/xilinx_spi.c | 102 +++++++++++++++++++++++++++++++----------------
 1 file changed, 67 insertions(+), 35 deletions(-)

diff --git a/drivers/spi/xilinx_spi.c b/drivers/spi/xilinx_spi.c
index cc5ac51..4026540 100644
--- a/drivers/spi/xilinx_spi.c
+++ b/drivers/spi/xilinx_spi.c
@@ -19,6 +19,7 @@
 #include <malloc.h>
 #include <spi.h>
 #include <asm/io.h>
+#include <wait_bit.h>

 /*
  * [0]: http://www.xilinx.com/support/documentation
@@ -77,6 +78,8 @@
 #define CONFIG_XILINX_SPI_IDLE_VAL     GENMASK(7, 0)
 #endif

+#define XILINX_SPISR_TIMEOUT   10000 /* in milliseconds */
+
 /* xilinx spi register set */
 struct xilinx_spi_regs {
        u32 __space0__[7];
@@ -101,6 +104,7 @@ struct xilinx_spi_priv {
        struct xilinx_spi_regs *regs;
        unsigned int freq;
        unsigned int mode;
+       unsigned int fifo_depth;
 };

 static int xilinx_spi_probe(struct udevice *bus)
@@ -110,6 +114,9 @@ static int xilinx_spi_probe(struct udevice *bus)

        priv->regs = (struct xilinx_spi_regs *)devfdt_get_addr(bus);

+       priv->fifo_depth = fdtdec_get_int(gd->fdt_blob, dev_of_offset(bus),
+                                         "fifo-size", 0);
+
        writel(SPISSR_RESET_VALUE, &regs->srr);

        return 0;
@@ -157,6 +164,46 @@ static int xilinx_spi_release_bus(struct udevice *dev)
        return 0;
 }

+static u32 xilinx_spi_fill_txfifo(struct udevice *bus, const u8 *txp,
+                                 u32 txbytes)
+{
+       struct xilinx_spi_priv *priv = dev_get_priv(bus);
+       struct xilinx_spi_regs *regs = priv->regs;
+       unsigned char d;
+       u32 i = 0;
+
+       while (txbytes && !(readl(&regs->spisr) & SPISR_TX_FULL) &&
+              i < priv->fifo_depth) {
+               d = txp ? *txp++ : CONFIG_XILINX_SPI_IDLE_VAL;
+               debug("spi_xfer: tx:%x ", d);
+               /* write out and wait for processing (receive data) */
+               writel(d & SPIDTR_8BIT_MASK, &regs->spidtr);
+               txbytes--;
+               i++;
+       }
+       return i;
+}
+
+static u32 xilinx_spi_read_rxfifo(struct udevice *bus, u8 *rxp, u32 rxbytes)
+{
+       struct xilinx_spi_priv *priv = dev_get_priv(bus);
+       struct xilinx_spi_regs *regs = priv->regs;
+       unsigned char d;
+       unsigned int i = 0;
+
+       while (rxbytes && !(readl(&regs->spisr) & SPISR_RX_EMPTY)) {
+               d = readl(&regs->spidrr) & SPIDRR_8BIT_MASK;
+               if (rxp)
+                       *rxp++ = d;
+               debug("spi_xfer: rx:%x\n", d);
+               rxbytes--;
+               i++;
+       }
+       debug("Rx_done\n");
+
+       return i;
+}
+
 static int xilinx_spi_xfer(struct udevice *dev, unsigned int bitlen,
                            const void *dout, void *din, unsigned long flags)
 {
@@ -168,8 +215,10 @@ static int xilinx_spi_xfer(struct udevice *dev, unsigned int bitlen,
        unsigned int bytes = bitlen / XILSPI_MAX_XFER_BITS;
        const unsigned char *txp = dout;
        unsigned char *rxp = din;
-       unsigned rxecount = 17; /* max. 16 elements in FIFO, leftover 1 */
-       unsigned global_timeout;
+       u32 txbytes = bytes;
+       u32 rxbytes = bytes;
+       u32 reg, count, timeout;
+       int ret;

        debug("spi_xfer: bus:%i cs:%i bitlen:%i bytes:%i flags:%lx\n",
              bus->seq, slave_plat->cs, bitlen, bytes, flags);
@@ -184,48 +233,31 @@ static int xilinx_spi_xfer(struct udevice *dev, unsigned int bitlen,
                goto done;
        }

-       /* empty read buffer */
-       while (rxecount && !(readl(&regs->spisr) & SPISR_RX_EMPTY)) {
-               readl(&regs->spidrr);
-               rxecount--;
-       }
-
-       if (!rxecount) {
-               printf("XILSPI error: Rx buffer not empty\n");
-               return -1;
-       }
-
        if (flags & SPI_XFER_BEGIN)
                spi_cs_activate(dev, slave_plat->cs);

-       /* at least 1usec or greater, leftover 1 */
-       global_timeout = priv->freq > XILSPI_MAX_XFER_BITS * 1000000 ? 2 :
-                       (XILSPI_MAX_XFER_BITS * 1000000 / priv->freq) + 1;

-       while (bytes--) {
-               unsigned timeout = global_timeout;
-               /* get Tx element from data out buffer and count up */
-               unsigned char d = txp ? *txp++ : CONFIG_XILINX_SPI_IDLE_VAL;
-               debug("spi_xfer: tx:%x ", d);
+       while (txbytes && rxbytes) {
+               count = xilinx_spi_fill_txfifo(bus, txp, txbytes);
+               reg = readl(&regs->spicr) & ~SPICR_MASTER_INHIBIT;
+               writel(reg, &regs->spicr);
+               txbytes -= count;
+               if (txp)
+                       txp += count;

-               /* write out and wait for processing (receive data) */
-               writel(d & SPIDTR_8BIT_MASK, &regs->spidtr);
-               while (timeout && readl(&regs->spisr)
-                                               & SPISR_RX_EMPTY) {
-                       timeout--;
-                       udelay(1);
-               }
-
-               if (!timeout) {
+               ret = wait_for_bit_le32(&regs->spisr, SPISR_TX_EMPTY, true,
+                                       XILINX_SPISR_TIMEOUT, false);
+               if (ret < 0) {
                        printf("XILSPI error: Xfer timeout\n");
-                       return -1;
+                       return ret;
                }

-               /* read Rx element and push into data in buffer */
-               d = readl(&regs->spidrr) & SPIDRR_8BIT_MASK;
+               debug("txbytes:0x%x,txp:0x%p\n", txbytes, txp);
+               count = xilinx_spi_read_rxfifo(bus, rxp, rxbytes);
+               rxbytes -= count;
                if (rxp)
-                       *rxp++ = d;
-               debug("spi_xfer: rx:%x\n", d);
+                       rxp += count;
+               debug("rxbytes:0x%x rxp:0x%p\n", rxbytes, rxp);
        }

  done:
--
2.7.4

This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [U-Boot] [UBOOT PATCH v3 3/3] spi: xilinx_spi: Added support to read JEDEC-id twice at the boot time
  2018-06-21  9:23 [U-Boot] [UBOOT PATCH v3 0/3] spi:xilinx_spi: Modify xilinx spi driver Vipul Kumar
  2018-06-21  9:23 ` [U-Boot] [UBOOT PATCH v3 1/3] spi: xilinx: Read reg base address from DTS file Vipul Kumar
  2018-06-21  9:23 ` [U-Boot] [UBOOT PATCH v3 2/3] spi: xilinx_spi: Modify transfer logic xilinx_spi_xfer() function Vipul Kumar
@ 2018-06-21  9:23 ` Vipul Kumar
  2018-06-25 10:17   ` Jagan Teki
  2 siblings, 1 reply; 12+ messages in thread
From: Vipul Kumar @ 2018-06-21  9:23 UTC (permalink / raw)
  To: u-boot

This patch is for the startup block issue in the spi controller.
SPI clock is passing through STARTUP block to FLASH. STARTUP block
don't provide clock as soon as QSPI provides command. So, first
command fails.

This patch added support to read JEDEC id in xilinx_spi_xfer ().

Signed-off-by: Vipul Kumar <vipul.kumar@xilinx.com>
---
Changes in v3:
- No change
---
 drivers/spi/xilinx_spi.c | 41 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/drivers/spi/xilinx_spi.c b/drivers/spi/xilinx_spi.c
index 4026540..61301c2 100644
--- a/drivers/spi/xilinx_spi.c
+++ b/drivers/spi/xilinx_spi.c
@@ -204,6 +204,40 @@ static u32 xilinx_spi_read_rxfifo(struct udevice *bus, u8 *rxp, u32 rxbytes)
        return i;
 }

+static void xilinx_startup_block(struct udevice *dev, unsigned int bytes,
+                                const void *dout, void *din)
+{
+       struct udevice *bus = dev_get_parent(dev);
+       struct xilinx_spi_priv *priv = dev_get_priv(bus);
+       struct xilinx_spi_regs *regs = priv->regs;
+       struct dm_spi_slave_platdata *slave_plat = dev_get_parent_platdata(dev);
+       const unsigned char *txp = dout;
+       unsigned char *rxp = din;
+       static int startup;
+       u32 reg, count;
+       u32 txbytes = bytes;
+       u32 rxbytes = bytes;
+
+       /*
+        * This loop runs two times. First time to send the command.
+        * Second time to transfer data. After transferring data,
+        * it sets txp to the initial value for the normal operation.
+        */
+       for ( ; startup < 2; startup++) {
+               count = xilinx_spi_fill_txfifo(bus, txp, txbytes);
+               reg = readl(&regs->spicr) & ~SPICR_MASTER_INHIBIT;
+               writel(reg, &regs->spicr);
+               count = xilinx_spi_read_rxfifo(bus, rxp, rxbytes);
+               txp = din;
+
+               if (startup) {
+                       spi_cs_deactivate(dev);
+                       spi_cs_activate(dev, slave_plat->cs);
+                       txp = dout;
+               }
+       }
+}
+
 static int xilinx_spi_xfer(struct udevice *dev, unsigned int bitlen,
                            const void *dout, void *din, unsigned long flags)
 {
@@ -236,6 +270,13 @@ static int xilinx_spi_xfer(struct udevice *dev, unsigned int bitlen,
        if (flags & SPI_XFER_BEGIN)
                spi_cs_activate(dev, slave_plat->cs);

+       /*
+        * This is the work around for the startup block issue in
+        * the spi controller. SPI clock is passing through STARTUP
+        * block to FLASH. STARTUP block don't provide clock as soon
+        * as QSPI provides command. So first command fails.
+        */
+       xilinx_startup_block(dev, bytes, dout, din);

        while (txbytes && rxbytes) {
                count = xilinx_spi_fill_txfifo(bus, txp, txbytes);
--
2.7.4

This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [U-Boot] [UBOOT PATCH v3 1/3] spi: xilinx: Read reg base address from DTS file
  2018-06-21  9:23 ` [U-Boot] [UBOOT PATCH v3 1/3] spi: xilinx: Read reg base address from DTS file Vipul Kumar
@ 2018-06-25 10:07   ` Jagan Teki
  0 siblings, 0 replies; 12+ messages in thread
From: Jagan Teki @ 2018-06-25 10:07 UTC (permalink / raw)
  To: u-boot

On Thu, Jun 21, 2018 at 2:53 PM, Vipul Kumar <vipul.kumar@xilinx.com> wrote:
> From: Michal Simek <michal.simek@xilinx.com>
>
> This patch added support to read register base address
> from DTS file.
>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> Signed-off-by: Vipul Kumar <vipul.kumar@xilinx.com>
> ---
> Changes in v3:
> - Read reg in probe function
> - Removed xilinx_spi_ofdata_to_platdata function
> - Removed reading of fifo_depth
> ---

Reviewed-by: Jagan Teki <jagan@openedev.com>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [UBOOT PATCH v3 2/3] spi: xilinx_spi: Modify transfer logic xilinx_spi_xfer() function
  2018-06-21  9:23 ` [U-Boot] [UBOOT PATCH v3 2/3] spi: xilinx_spi: Modify transfer logic xilinx_spi_xfer() function Vipul Kumar
@ 2018-06-25 10:11   ` Jagan Teki
  0 siblings, 0 replies; 12+ messages in thread
From: Jagan Teki @ 2018-06-25 10:11 UTC (permalink / raw)
  To: u-boot

On Thu, Jun 21, 2018 at 2:53 PM, Vipul Kumar <vipul.kumar@xilinx.com> wrote:
> This patch modify xilinx_spi_xfer() function and add rxfifo() and
> txfifo() functions to add the modularity so that these functions
> can be used by other functions within the same file.
>
> This patch also added support to read fifo_size from dts.
>
> Signed-off-by: Vipul Kumar <vipul.kumar@xilinx.com>
> Signed-off-by: Siva Durga Prasad Paladugu <siva.durga.paladugu@xilinx.com>
> ---
> - Changes in v3:
> - Added fifo_depth read support and removed from 1/3
> ---
>  drivers/spi/xilinx_spi.c | 102 +++++++++++++++++++++++++++++++----------------
>  1 file changed, 67 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/spi/xilinx_spi.c b/drivers/spi/xilinx_spi.c
> index cc5ac51..4026540 100644
> --- a/drivers/spi/xilinx_spi.c
> +++ b/drivers/spi/xilinx_spi.c
> @@ -19,6 +19,7 @@
>  #include <malloc.h>
>  #include <spi.h>
>  #include <asm/io.h>
> +#include <wait_bit.h>
>
>  /*
>   * [0]: http://www.xilinx.com/support/documentation
> @@ -77,6 +78,8 @@
>  #define CONFIG_XILINX_SPI_IDLE_VAL     GENMASK(7, 0)
>  #endif
>
> +#define XILINX_SPISR_TIMEOUT   10000 /* in milliseconds */
> +
>  /* xilinx spi register set */
>  struct xilinx_spi_regs {
>         u32 __space0__[7];
> @@ -101,6 +104,7 @@ struct xilinx_spi_priv {
>         struct xilinx_spi_regs *regs;
>         unsigned int freq;
>         unsigned int mode;
> +       unsigned int fifo_depth;
>  };
>
>  static int xilinx_spi_probe(struct udevice *bus)
> @@ -110,6 +114,9 @@ static int xilinx_spi_probe(struct udevice *bus)
>
>         priv->regs = (struct xilinx_spi_regs *)devfdt_get_addr(bus);
>
> +       priv->fifo_depth = fdtdec_get_int(gd->fdt_blob, dev_of_offset(bus),
> +                                         "fifo-size", 0);
> +
>         writel(SPISSR_RESET_VALUE, &regs->srr);
>
>         return 0;
> @@ -157,6 +164,46 @@ static int xilinx_spi_release_bus(struct udevice *dev)
>         return 0;
>  }
>
> +static u32 xilinx_spi_fill_txfifo(struct udevice *bus, const u8 *txp,
> +                                 u32 txbytes)
> +{
> +       struct xilinx_spi_priv *priv = dev_get_priv(bus);
> +       struct xilinx_spi_regs *regs = priv->regs;
> +       unsigned char d;
> +       u32 i = 0;
> +
> +       while (txbytes && !(readl(&regs->spisr) & SPISR_TX_FULL) &&
> +              i < priv->fifo_depth) {
> +               d = txp ? *txp++ : CONFIG_XILINX_SPI_IDLE_VAL;
> +               debug("spi_xfer: tx:%x ", d);
> +               /* write out and wait for processing (receive data) */
> +               writel(d & SPIDTR_8BIT_MASK, &regs->spidtr);
> +               txbytes--;
> +               i++;
> +       }

need space

> +       return i;
> +}
> +
> +static u32 xilinx_spi_read_rxfifo(struct udevice *bus, u8 *rxp, u32 rxbytes)
> +{
> +       struct xilinx_spi_priv *priv = dev_get_priv(bus);
> +       struct xilinx_spi_regs *regs = priv->regs;
> +       unsigned char d;
> +       unsigned int i = 0;
> +
> +       while (rxbytes && !(readl(&regs->spisr) & SPISR_RX_EMPTY)) {
> +               d = readl(&regs->spidrr) & SPIDRR_8BIT_MASK;
> +               if (rxp)
> +                       *rxp++ = d;
> +               debug("spi_xfer: rx:%x\n", d);
> +               rxbytes--;
> +               i++;
> +       }
> +       debug("Rx_done\n");
> +
> +       return i;
> +}

Reviewed-by: Jagan Teki <jagan@openedev.com>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [UBOOT PATCH v3 3/3] spi: xilinx_spi: Added support to read JEDEC-id twice at the boot time
  2018-06-21  9:23 ` [U-Boot] [UBOOT PATCH v3 3/3] spi: xilinx_spi: Added support to read JEDEC-id twice at the boot time Vipul Kumar
@ 2018-06-25 10:17   ` Jagan Teki
  2018-06-25 10:36     ` Vipul Kumar
  0 siblings, 1 reply; 12+ messages in thread
From: Jagan Teki @ 2018-06-25 10:17 UTC (permalink / raw)
  To: u-boot

On Thu, Jun 21, 2018 at 2:53 PM, Vipul Kumar <vipul.kumar@xilinx.com> wrote:
> This patch is for the startup block issue in the spi controller.
> SPI clock is passing through STARTUP block to FLASH. STARTUP block
> don't provide clock as soon as QSPI provides command. So, first
> command fails.

Does this a controller issue?

>
> This patch added support to read JEDEC id in xilinx_spi_xfer ().
>
> Signed-off-by: Vipul Kumar <vipul.kumar@xilinx.com>
> ---
> Changes in v3:
> - No change
> ---
>  drivers/spi/xilinx_spi.c | 41 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
>
> diff --git a/drivers/spi/xilinx_spi.c b/drivers/spi/xilinx_spi.c
> index 4026540..61301c2 100644
> --- a/drivers/spi/xilinx_spi.c
> +++ b/drivers/spi/xilinx_spi.c
> @@ -204,6 +204,40 @@ static u32 xilinx_spi_read_rxfifo(struct udevice *bus, u8 *rxp, u32 rxbytes)
>         return i;
>  }
>
> +static void xilinx_startup_block(struct udevice *dev, unsigned int bytes,
> +                                const void *dout, void *din)
> +{
> +       struct udevice *bus = dev_get_parent(dev);
> +       struct xilinx_spi_priv *priv = dev_get_priv(bus);
> +       struct xilinx_spi_regs *regs = priv->regs;
> +       struct dm_spi_slave_platdata *slave_plat = dev_get_parent_platdata(dev);
> +       const unsigned char *txp = dout;
> +       unsigned char *rxp = din;
> +       static int startup;
> +       u32 reg, count;
> +       u32 txbytes = bytes;
> +       u32 rxbytes = bytes;
> +
> +       /*
> +        * This loop runs two times. First time to send the command.
> +        * Second time to transfer data. After transferring data,
> +        * it sets txp to the initial value for the normal operation.
> +        */
> +       for ( ; startup < 2; startup++) {
> +               count = xilinx_spi_fill_txfifo(bus, txp, txbytes);
> +               reg = readl(&regs->spicr) & ~SPICR_MASTER_INHIBIT;
> +               writel(reg, &regs->spicr);
> +               count = xilinx_spi_read_rxfifo(bus, rxp, rxbytes);
> +               txp = din;
> +
> +               if (startup) {
> +                       spi_cs_deactivate(dev);
> +                       spi_cs_activate(dev, slave_plat->cs);
> +                       txp = dout;
> +               }
> +       }
> +}
> +
>  static int xilinx_spi_xfer(struct udevice *dev, unsigned int bitlen,
>                             const void *dout, void *din, unsigned long flags)
>  {
> @@ -236,6 +270,13 @@ static int xilinx_spi_xfer(struct udevice *dev, unsigned int bitlen,
>         if (flags & SPI_XFER_BEGIN)
>                 spi_cs_activate(dev, slave_plat->cs);
>
> +       /*
> +        * This is the work around for the startup block issue in
> +        * the spi controller. SPI clock is passing through STARTUP
> +        * block to FLASH. STARTUP block don't provide clock as soon
> +        * as QSPI provides command. So first command fails.
> +        */
> +       xilinx_startup_block(dev, bytes, dout, din);

But not every spi_xfer from flash side is to read JEDEC ID? during
probe of sf, the relevant spi_xfer is to read JEDEC ID

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [UBOOT PATCH v3 3/3] spi: xilinx_spi: Added support to read JEDEC-id twice at the boot time
  2018-06-25 10:17   ` Jagan Teki
@ 2018-06-25 10:36     ` Vipul Kumar
  2018-06-27  6:43       ` Jagan Teki
  0 siblings, 1 reply; 12+ messages in thread
From: Vipul Kumar @ 2018-06-25 10:36 UTC (permalink / raw)
  To: u-boot

Hi Jagan,

> -----Original Message-----
> From: Jagan Teki [mailto:jagannadh.teki at gmail.com]
> Sent: Monday, June 25, 2018 3:47 PM
> To: Vipul Kumar <vipulk@xilinx.com>
> Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Michal Simek
> <michals@xilinx.com>
> Subject: Re: [U-Boot] [UBOOT PATCH v3 3/3] spi: xilinx_spi: Added support to
> read JEDEC-id twice at the boot time
> 
> On Thu, Jun 21, 2018 at 2:53 PM, Vipul Kumar <vipul.kumar@xilinx.com>
> wrote:
> > This patch is for the startup block issue in the spi controller.
> > SPI clock is passing through STARTUP block to FLASH. STARTUP block
> > don't provide clock as soon as QSPI provides command. So, first
> > command fails.
> 
> Does this a controller issue?

Yes, this is a controller issue.

> 
> >
> > This patch added support to read JEDEC id in xilinx_spi_xfer ().
> >
> > Signed-off-by: Vipul Kumar <vipul.kumar@xilinx.com>
> > ---
> > Changes in v3:
> > - No change
> > ---
> >  drivers/spi/xilinx_spi.c | 41
> > +++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 41 insertions(+)
> >
> > diff --git a/drivers/spi/xilinx_spi.c b/drivers/spi/xilinx_spi.c index
> > 4026540..61301c2 100644
> > --- a/drivers/spi/xilinx_spi.c
> > +++ b/drivers/spi/xilinx_spi.c
> > @@ -204,6 +204,40 @@ static u32 xilinx_spi_read_rxfifo(struct udevice
> *bus, u8 *rxp, u32 rxbytes)
> >         return i;
> >  }
> >
> > +static void xilinx_startup_block(struct udevice *dev, unsigned int bytes,
> > +                                const void *dout, void *din) {
> > +       struct udevice *bus = dev_get_parent(dev);
> > +       struct xilinx_spi_priv *priv = dev_get_priv(bus);
> > +       struct xilinx_spi_regs *regs = priv->regs;
> > +       struct dm_spi_slave_platdata *slave_plat =
> dev_get_parent_platdata(dev);
> > +       const unsigned char *txp = dout;
> > +       unsigned char *rxp = din;
> > +       static int startup;
> > +       u32 reg, count;
> > +       u32 txbytes = bytes;
> > +       u32 rxbytes = bytes;
> > +
> > +       /*
> > +        * This loop runs two times. First time to send the command.
> > +        * Second time to transfer data. After transferring data,
> > +        * it sets txp to the initial value for the normal operation.
> > +        */
> > +       for ( ; startup < 2; startup++) {
> > +               count = xilinx_spi_fill_txfifo(bus, txp, txbytes);
> > +               reg = readl(&regs->spicr) & ~SPICR_MASTER_INHIBIT;
> > +               writel(reg, &regs->spicr);
> > +               count = xilinx_spi_read_rxfifo(bus, rxp, rxbytes);
> > +               txp = din;
> > +
> > +               if (startup) {
> > +                       spi_cs_deactivate(dev);
> > +                       spi_cs_activate(dev, slave_plat->cs);
> > +                       txp = dout;
> > +               }
> > +       }
> > +}
> > +
> >  static int xilinx_spi_xfer(struct udevice *dev, unsigned int bitlen,
> >                             const void *dout, void *din, unsigned long
> > flags)  { @@ -236,6 +270,13 @@ static int xilinx_spi_xfer(struct
> > udevice *dev, unsigned int bitlen,
> >         if (flags & SPI_XFER_BEGIN)
> >                 spi_cs_activate(dev, slave_plat->cs);
> >
> > +       /*
> > +        * This is the work around for the startup block issue in
> > +        * the spi controller. SPI clock is passing through STARTUP
> > +        * block to FLASH. STARTUP block don't provide clock as soon
> > +        * as QSPI provides command. So first command fails.
> > +        */
> > +       xilinx_startup_block(dev, bytes, dout, din);
> 
> But not every spi_xfer from flash side is to read JEDEC ID? during probe of sf,
> the relevant spi_xfer is to read JEDEC ID

Inside xilinx_startup_block() function, we are using static variable, so that code will execute only first time.

Regards,
Vipul

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [UBOOT PATCH v3 3/3] spi: xilinx_spi: Added support to read JEDEC-id twice at the boot time
  2018-06-25 10:36     ` Vipul Kumar
@ 2018-06-27  6:43       ` Jagan Teki
  2018-06-27  7:29         ` Michal Simek
  0 siblings, 1 reply; 12+ messages in thread
From: Jagan Teki @ 2018-06-27  6:43 UTC (permalink / raw)
  To: u-boot

On Mon, Jun 25, 2018 at 4:06 PM, Vipul Kumar <vipulk@xilinx.com> wrote:
> Hi Jagan,
>
>> -----Original Message-----
>> From: Jagan Teki [mailto:jagannadh.teki at gmail.com]
>> Sent: Monday, June 25, 2018 3:47 PM
>> To: Vipul Kumar <vipulk@xilinx.com>
>> Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Michal Simek
>> <michals@xilinx.com>
>> Subject: Re: [U-Boot] [UBOOT PATCH v3 3/3] spi: xilinx_spi: Added support to
>> read JEDEC-id twice at the boot time
>>
>> On Thu, Jun 21, 2018 at 2:53 PM, Vipul Kumar <vipul.kumar@xilinx.com>
>> wrote:
>> > This patch is for the startup block issue in the spi controller.
>> > SPI clock is passing through STARTUP block to FLASH. STARTUP block
>> > don't provide clock as soon as QSPI provides command. So, first
>> > command fails.
>>
>> Does this a controller issue?
>
> Yes, this is a controller issue.
>
>>
>> >
>> > This patch added support to read JEDEC id in xilinx_spi_xfer ().
>> >
>> > Signed-off-by: Vipul Kumar <vipul.kumar@xilinx.com>
>> > ---
>> > Changes in v3:
>> > - No change
>> > ---
>> >  drivers/spi/xilinx_spi.c | 41
>> > +++++++++++++++++++++++++++++++++++++++++
>> >  1 file changed, 41 insertions(+)
>> >
>> > diff --git a/drivers/spi/xilinx_spi.c b/drivers/spi/xilinx_spi.c index
>> > 4026540..61301c2 100644
>> > --- a/drivers/spi/xilinx_spi.c
>> > +++ b/drivers/spi/xilinx_spi.c
>> > @@ -204,6 +204,40 @@ static u32 xilinx_spi_read_rxfifo(struct udevice
>> *bus, u8 *rxp, u32 rxbytes)
>> >         return i;
>> >  }
>> >
>> > +static void xilinx_startup_block(struct udevice *dev, unsigned int bytes,
>> > +                                const void *dout, void *din) {
>> > +       struct udevice *bus = dev_get_parent(dev);
>> > +       struct xilinx_spi_priv *priv = dev_get_priv(bus);
>> > +       struct xilinx_spi_regs *regs = priv->regs;
>> > +       struct dm_spi_slave_platdata *slave_plat =
>> dev_get_parent_platdata(dev);
>> > +       const unsigned char *txp = dout;
>> > +       unsigned char *rxp = din;
>> > +       static int startup;
>> > +       u32 reg, count;
>> > +       u32 txbytes = bytes;
>> > +       u32 rxbytes = bytes;
>> > +
>> > +       /*
>> > +        * This loop runs two times. First time to send the command.
>> > +        * Second time to transfer data. After transferring data,
>> > +        * it sets txp to the initial value for the normal operation.
>> > +        */
>> > +       for ( ; startup < 2; startup++) {
>> > +               count = xilinx_spi_fill_txfifo(bus, txp, txbytes);
>> > +               reg = readl(&regs->spicr) & ~SPICR_MASTER_INHIBIT;
>> > +               writel(reg, &regs->spicr);
>> > +               count = xilinx_spi_read_rxfifo(bus, rxp, rxbytes);
>> > +               txp = din;
>> > +
>> > +               if (startup) {
>> > +                       spi_cs_deactivate(dev);
>> > +                       spi_cs_activate(dev, slave_plat->cs);
>> > +                       txp = dout;
>> > +               }
>> > +       }
>> > +}
>> > +
>> >  static int xilinx_spi_xfer(struct udevice *dev, unsigned int bitlen,
>> >                             const void *dout, void *din, unsigned long
>> > flags)  { @@ -236,6 +270,13 @@ static int xilinx_spi_xfer(struct
>> > udevice *dev, unsigned int bitlen,
>> >         if (flags & SPI_XFER_BEGIN)
>> >                 spi_cs_activate(dev, slave_plat->cs);
>> >
>> > +       /*
>> > +        * This is the work around for the startup block issue in
>> > +        * the spi controller. SPI clock is passing through STARTUP
>> > +        * block to FLASH. STARTUP block don't provide clock as soon
>> > +        * as QSPI provides command. So first command fails.
>> > +        */
>> > +       xilinx_startup_block(dev, bytes, dout, din);
>>
>> But not every spi_xfer from flash side is to read JEDEC ID? during probe of sf,
>> the relevant spi_xfer is to read JEDEC ID
>
> Inside xilinx_startup_block() function, we are using static variable, so that code will execute only first time.

Sorry, IMHO mainline won't accept this kind of hack (particularly
avoiding function execution through static variables) by for the sake
of bug fix. Having a proper condition to check and fix the errata look
fine to me, any idea how Linux handling this?

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [UBOOT PATCH v3 3/3] spi: xilinx_spi: Added support to read JEDEC-id twice at the boot time
  2018-06-27  6:43       ` Jagan Teki
@ 2018-06-27  7:29         ` Michal Simek
  2018-06-27  7:35           ` Jagan Teki
  0 siblings, 1 reply; 12+ messages in thread
From: Michal Simek @ 2018-06-27  7:29 UTC (permalink / raw)
  To: u-boot

Hi Jagan,

On 27.6.2018 08:43, Jagan Teki wrote:
> On Mon, Jun 25, 2018 at 4:06 PM, Vipul Kumar <vipulk@xilinx.com> wrote:
>> Hi Jagan,
>>
>>> -----Original Message-----
>>> From: Jagan Teki [mailto:jagannadh.teki at gmail.com]
>>> Sent: Monday, June 25, 2018 3:47 PM
>>> To: Vipul Kumar <vipulk@xilinx.com>
>>> Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Michal Simek
>>> <michals@xilinx.com>
>>> Subject: Re: [U-Boot] [UBOOT PATCH v3 3/3] spi: xilinx_spi: Added support to
>>> read JEDEC-id twice at the boot time
>>>
>>> On Thu, Jun 21, 2018 at 2:53 PM, Vipul Kumar <vipul.kumar@xilinx.com>
>>> wrote:
>>>> This patch is for the startup block issue in the spi controller.
>>>> SPI clock is passing through STARTUP block to FLASH. STARTUP block
>>>> don't provide clock as soon as QSPI provides command. So, first
>>>> command fails.
>>>
>>> Does this a controller issue?
>>
>> Yes, this is a controller issue.
>>
>>>
>>>>
>>>> This patch added support to read JEDEC id in xilinx_spi_xfer ().
>>>>
>>>> Signed-off-by: Vipul Kumar <vipul.kumar@xilinx.com>
>>>> ---
>>>> Changes in v3:
>>>> - No change
>>>> ---
>>>>  drivers/spi/xilinx_spi.c | 41
>>>> +++++++++++++++++++++++++++++++++++++++++
>>>>  1 file changed, 41 insertions(+)
>>>>
>>>> diff --git a/drivers/spi/xilinx_spi.c b/drivers/spi/xilinx_spi.c index
>>>> 4026540..61301c2 100644
>>>> --- a/drivers/spi/xilinx_spi.c
>>>> +++ b/drivers/spi/xilinx_spi.c
>>>> @@ -204,6 +204,40 @@ static u32 xilinx_spi_read_rxfifo(struct udevice
>>> *bus, u8 *rxp, u32 rxbytes)
>>>>         return i;
>>>>  }
>>>>
>>>> +static void xilinx_startup_block(struct udevice *dev, unsigned int bytes,
>>>> +                                const void *dout, void *din) {
>>>> +       struct udevice *bus = dev_get_parent(dev);
>>>> +       struct xilinx_spi_priv *priv = dev_get_priv(bus);
>>>> +       struct xilinx_spi_regs *regs = priv->regs;
>>>> +       struct dm_spi_slave_platdata *slave_plat =
>>> dev_get_parent_platdata(dev);
>>>> +       const unsigned char *txp = dout;
>>>> +       unsigned char *rxp = din;
>>>> +       static int startup;
>>>> +       u32 reg, count;
>>>> +       u32 txbytes = bytes;
>>>> +       u32 rxbytes = bytes;
>>>> +
>>>> +       /*
>>>> +        * This loop runs two times. First time to send the command.
>>>> +        * Second time to transfer data. After transferring data,
>>>> +        * it sets txp to the initial value for the normal operation.
>>>> +        */
>>>> +       for ( ; startup < 2; startup++) {
>>>> +               count = xilinx_spi_fill_txfifo(bus, txp, txbytes);
>>>> +               reg = readl(&regs->spicr) & ~SPICR_MASTER_INHIBIT;
>>>> +               writel(reg, &regs->spicr);
>>>> +               count = xilinx_spi_read_rxfifo(bus, rxp, rxbytes);
>>>> +               txp = din;
>>>> +
>>>> +               if (startup) {
>>>> +                       spi_cs_deactivate(dev);
>>>> +                       spi_cs_activate(dev, slave_plat->cs);
>>>> +                       txp = dout;
>>>> +               }
>>>> +       }
>>>> +}
>>>> +
>>>>  static int xilinx_spi_xfer(struct udevice *dev, unsigned int bitlen,
>>>>                             const void *dout, void *din, unsigned long
>>>> flags)  { @@ -236,6 +270,13 @@ static int xilinx_spi_xfer(struct
>>>> udevice *dev, unsigned int bitlen,
>>>>         if (flags & SPI_XFER_BEGIN)
>>>>                 spi_cs_activate(dev, slave_plat->cs);
>>>>
>>>> +       /*
>>>> +        * This is the work around for the startup block issue in
>>>> +        * the spi controller. SPI clock is passing through STARTUP
>>>> +        * block to FLASH. STARTUP block don't provide clock as soon
>>>> +        * as QSPI provides command. So first command fails.
>>>> +        */
>>>> +       xilinx_startup_block(dev, bytes, dout, din);
>>>
>>> But not every spi_xfer from flash side is to read JEDEC ID? during probe of sf,
>>> the relevant spi_xfer is to read JEDEC ID
>>
>> Inside xilinx_startup_block() function, we are using static variable, so that code will execute only first time.
> 
> Sorry, IMHO mainline won't accept this kind of hack (particularly
> avoiding function execution through static variables) by for the sake
> of bug fix. Having a proper condition to check and fix the errata look
> fine to me, any idea how Linux handling this?

ok - static startup variable should be move to private data structure to
be private for every instance.

Also please put a link to errata for a record.

Jagan: It will be much easier if you can simply write snippet which you
are suggesting.

Thanks,
Michal

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [UBOOT PATCH v3 3/3] spi: xilinx_spi: Added support to read JEDEC-id twice at the boot time
  2018-06-27  7:29         ` Michal Simek
@ 2018-06-27  7:35           ` Jagan Teki
  2018-06-27  9:47             ` Michal Simek
  0 siblings, 1 reply; 12+ messages in thread
From: Jagan Teki @ 2018-06-27  7:35 UTC (permalink / raw)
  To: u-boot

On Wed, Jun 27, 2018 at 12:59 PM, Michal Simek <michal.simek@xilinx.com> wrote:
> Hi Jagan,
>
> On 27.6.2018 08:43, Jagan Teki wrote:
>> On Mon, Jun 25, 2018 at 4:06 PM, Vipul Kumar <vipulk@xilinx.com> wrote:
>>> Hi Jagan,
>>>
>>>> -----Original Message-----
>>>> From: Jagan Teki [mailto:jagannadh.teki at gmail.com]
>>>> Sent: Monday, June 25, 2018 3:47 PM
>>>> To: Vipul Kumar <vipulk@xilinx.com>
>>>> Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Michal Simek
>>>> <michals@xilinx.com>
>>>> Subject: Re: [U-Boot] [UBOOT PATCH v3 3/3] spi: xilinx_spi: Added support to
>>>> read JEDEC-id twice at the boot time
>>>>
>>>> On Thu, Jun 21, 2018 at 2:53 PM, Vipul Kumar <vipul.kumar@xilinx.com>
>>>> wrote:
>>>>> This patch is for the startup block issue in the spi controller.
>>>>> SPI clock is passing through STARTUP block to FLASH. STARTUP block
>>>>> don't provide clock as soon as QSPI provides command. So, first
>>>>> command fails.
>>>>
>>>> Does this a controller issue?
>>>
>>> Yes, this is a controller issue.
>>>
>>>>
>>>>>
>>>>> This patch added support to read JEDEC id in xilinx_spi_xfer ().
>>>>>
>>>>> Signed-off-by: Vipul Kumar <vipul.kumar@xilinx.com>
>>>>> ---
>>>>> Changes in v3:
>>>>> - No change
>>>>> ---
>>>>>  drivers/spi/xilinx_spi.c | 41
>>>>> +++++++++++++++++++++++++++++++++++++++++
>>>>>  1 file changed, 41 insertions(+)
>>>>>
>>>>> diff --git a/drivers/spi/xilinx_spi.c b/drivers/spi/xilinx_spi.c index
>>>>> 4026540..61301c2 100644
>>>>> --- a/drivers/spi/xilinx_spi.c
>>>>> +++ b/drivers/spi/xilinx_spi.c
>>>>> @@ -204,6 +204,40 @@ static u32 xilinx_spi_read_rxfifo(struct udevice
>>>> *bus, u8 *rxp, u32 rxbytes)
>>>>>         return i;
>>>>>  }
>>>>>
>>>>> +static void xilinx_startup_block(struct udevice *dev, unsigned int bytes,
>>>>> +                                const void *dout, void *din) {
>>>>> +       struct udevice *bus = dev_get_parent(dev);
>>>>> +       struct xilinx_spi_priv *priv = dev_get_priv(bus);
>>>>> +       struct xilinx_spi_regs *regs = priv->regs;
>>>>> +       struct dm_spi_slave_platdata *slave_plat =
>>>> dev_get_parent_platdata(dev);
>>>>> +       const unsigned char *txp = dout;
>>>>> +       unsigned char *rxp = din;
>>>>> +       static int startup;
>>>>> +       u32 reg, count;
>>>>> +       u32 txbytes = bytes;
>>>>> +       u32 rxbytes = bytes;
>>>>> +
>>>>> +       /*
>>>>> +        * This loop runs two times. First time to send the command.
>>>>> +        * Second time to transfer data. After transferring data,
>>>>> +        * it sets txp to the initial value for the normal operation.
>>>>> +        */
>>>>> +       for ( ; startup < 2; startup++) {
>>>>> +               count = xilinx_spi_fill_txfifo(bus, txp, txbytes);
>>>>> +               reg = readl(&regs->spicr) & ~SPICR_MASTER_INHIBIT;
>>>>> +               writel(reg, &regs->spicr);
>>>>> +               count = xilinx_spi_read_rxfifo(bus, rxp, rxbytes);
>>>>> +               txp = din;
>>>>> +
>>>>> +               if (startup) {
>>>>> +                       spi_cs_deactivate(dev);
>>>>> +                       spi_cs_activate(dev, slave_plat->cs);
>>>>> +                       txp = dout;
>>>>> +               }
>>>>> +       }
>>>>> +}
>>>>> +
>>>>>  static int xilinx_spi_xfer(struct udevice *dev, unsigned int bitlen,
>>>>>                             const void *dout, void *din, unsigned long
>>>>> flags)  { @@ -236,6 +270,13 @@ static int xilinx_spi_xfer(struct
>>>>> udevice *dev, unsigned int bitlen,
>>>>>         if (flags & SPI_XFER_BEGIN)
>>>>>                 spi_cs_activate(dev, slave_plat->cs);
>>>>>
>>>>> +       /*
>>>>> +        * This is the work around for the startup block issue in
>>>>> +        * the spi controller. SPI clock is passing through STARTUP
>>>>> +        * block to FLASH. STARTUP block don't provide clock as soon
>>>>> +        * as QSPI provides command. So first command fails.
>>>>> +        */
>>>>> +       xilinx_startup_block(dev, bytes, dout, din);
>>>>
>>>> But not every spi_xfer from flash side is to read JEDEC ID? during probe of sf,
>>>> the relevant spi_xfer is to read JEDEC ID
>>>
>>> Inside xilinx_startup_block() function, we are using static variable, so that code will execute only first time.
>>
>> Sorry, IMHO mainline won't accept this kind of hack (particularly
>> avoiding function execution through static variables) by for the sake
>> of bug fix. Having a proper condition to check and fix the errata look
>> fine to me, any idea how Linux handling this?
>
> ok - static startup variable should be move to private data structure to
> be private for every instance.
>
> Also please put a link to errata for a record.
>
> Jagan: It will be much easier if you can simply write snippet which you
> are suggesting.

thought of, I'm still thinking how we can do this. I didn't see the
Linux code yet, any idea how Linux does?

Jagan.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [UBOOT PATCH v3 3/3] spi: xilinx_spi: Added support to read JEDEC-id twice at the boot time
  2018-06-27  7:35           ` Jagan Teki
@ 2018-06-27  9:47             ` Michal Simek
  0 siblings, 0 replies; 12+ messages in thread
From: Michal Simek @ 2018-06-27  9:47 UTC (permalink / raw)
  To: u-boot

On 27.6.2018 09:35, Jagan Teki wrote:
> On Wed, Jun 27, 2018 at 12:59 PM, Michal Simek <michal.simek@xilinx.com> wrote:
>> Hi Jagan,
>>
>> On 27.6.2018 08:43, Jagan Teki wrote:
>>> On Mon, Jun 25, 2018 at 4:06 PM, Vipul Kumar <vipulk@xilinx.com> wrote:
>>>> Hi Jagan,
>>>>
>>>>> -----Original Message-----
>>>>> From: Jagan Teki [mailto:jagannadh.teki at gmail.com]
>>>>> Sent: Monday, June 25, 2018 3:47 PM
>>>>> To: Vipul Kumar <vipulk@xilinx.com>
>>>>> Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Michal Simek
>>>>> <michals@xilinx.com>
>>>>> Subject: Re: [U-Boot] [UBOOT PATCH v3 3/3] spi: xilinx_spi: Added support to
>>>>> read JEDEC-id twice at the boot time
>>>>>
>>>>> On Thu, Jun 21, 2018 at 2:53 PM, Vipul Kumar <vipul.kumar@xilinx.com>
>>>>> wrote:
>>>>>> This patch is for the startup block issue in the spi controller.
>>>>>> SPI clock is passing through STARTUP block to FLASH. STARTUP block
>>>>>> don't provide clock as soon as QSPI provides command. So, first
>>>>>> command fails.
>>>>>
>>>>> Does this a controller issue?
>>>>
>>>> Yes, this is a controller issue.
>>>>
>>>>>
>>>>>>
>>>>>> This patch added support to read JEDEC id in xilinx_spi_xfer ().
>>>>>>
>>>>>> Signed-off-by: Vipul Kumar <vipul.kumar@xilinx.com>
>>>>>> ---
>>>>>> Changes in v3:
>>>>>> - No change
>>>>>> ---
>>>>>>  drivers/spi/xilinx_spi.c | 41
>>>>>> +++++++++++++++++++++++++++++++++++++++++
>>>>>>  1 file changed, 41 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/spi/xilinx_spi.c b/drivers/spi/xilinx_spi.c index
>>>>>> 4026540..61301c2 100644
>>>>>> --- a/drivers/spi/xilinx_spi.c
>>>>>> +++ b/drivers/spi/xilinx_spi.c
>>>>>> @@ -204,6 +204,40 @@ static u32 xilinx_spi_read_rxfifo(struct udevice
>>>>> *bus, u8 *rxp, u32 rxbytes)
>>>>>>         return i;
>>>>>>  }
>>>>>>
>>>>>> +static void xilinx_startup_block(struct udevice *dev, unsigned int bytes,
>>>>>> +                                const void *dout, void *din) {
>>>>>> +       struct udevice *bus = dev_get_parent(dev);
>>>>>> +       struct xilinx_spi_priv *priv = dev_get_priv(bus);
>>>>>> +       struct xilinx_spi_regs *regs = priv->regs;
>>>>>> +       struct dm_spi_slave_platdata *slave_plat =
>>>>> dev_get_parent_platdata(dev);
>>>>>> +       const unsigned char *txp = dout;
>>>>>> +       unsigned char *rxp = din;
>>>>>> +       static int startup;
>>>>>> +       u32 reg, count;
>>>>>> +       u32 txbytes = bytes;
>>>>>> +       u32 rxbytes = bytes;
>>>>>> +
>>>>>> +       /*
>>>>>> +        * This loop runs two times. First time to send the command.
>>>>>> +        * Second time to transfer data. After transferring data,
>>>>>> +        * it sets txp to the initial value for the normal operation.
>>>>>> +        */
>>>>>> +       for ( ; startup < 2; startup++) {
>>>>>> +               count = xilinx_spi_fill_txfifo(bus, txp, txbytes);
>>>>>> +               reg = readl(&regs->spicr) & ~SPICR_MASTER_INHIBIT;
>>>>>> +               writel(reg, &regs->spicr);
>>>>>> +               count = xilinx_spi_read_rxfifo(bus, rxp, rxbytes);
>>>>>> +               txp = din;
>>>>>> +
>>>>>> +               if (startup) {
>>>>>> +                       spi_cs_deactivate(dev);
>>>>>> +                       spi_cs_activate(dev, slave_plat->cs);
>>>>>> +                       txp = dout;
>>>>>> +               }
>>>>>> +       }
>>>>>> +}
>>>>>> +
>>>>>>  static int xilinx_spi_xfer(struct udevice *dev, unsigned int bitlen,
>>>>>>                             const void *dout, void *din, unsigned long
>>>>>> flags)  { @@ -236,6 +270,13 @@ static int xilinx_spi_xfer(struct
>>>>>> udevice *dev, unsigned int bitlen,
>>>>>>         if (flags & SPI_XFER_BEGIN)
>>>>>>                 spi_cs_activate(dev, slave_plat->cs);
>>>>>>
>>>>>> +       /*
>>>>>> +        * This is the work around for the startup block issue in
>>>>>> +        * the spi controller. SPI clock is passing through STARTUP
>>>>>> +        * block to FLASH. STARTUP block don't provide clock as soon
>>>>>> +        * as QSPI provides command. So first command fails.
>>>>>> +        */
>>>>>> +       xilinx_startup_block(dev, bytes, dout, din);
>>>>>
>>>>> But not every spi_xfer from flash side is to read JEDEC ID? during probe of sf,
>>>>> the relevant spi_xfer is to read JEDEC ID
>>>>
>>>> Inside xilinx_startup_block() function, we are using static variable, so that code will execute only first time.
>>>
>>> Sorry, IMHO mainline won't accept this kind of hack (particularly
>>> avoiding function execution through static variables) by for the sake
>>> of bug fix. Having a proper condition to check and fix the errata look
>>> fine to me, any idea how Linux handling this?
>>
>> ok - static startup variable should be move to private data structure to
>> be private for every instance.
>>
>> Also please put a link to errata for a record.
>>
>> Jagan: It will be much easier if you can simply write snippet which you
>> are suggesting.
> 
> thought of, I'm still thinking how we can do this. I didn't see the
> Linux code yet, any idea how Linux does?

The issue is present in Linux but it is not fixed there yet.
It means Linux can't be taken as pattern.
It is quite clear that this should be handled in the driver code and it
should be enabled by default for all IPs till this is fixed in hw IP.

Thanks,
Michal

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2018-06-27  9:47 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-21  9:23 [U-Boot] [UBOOT PATCH v3 0/3] spi:xilinx_spi: Modify xilinx spi driver Vipul Kumar
2018-06-21  9:23 ` [U-Boot] [UBOOT PATCH v3 1/3] spi: xilinx: Read reg base address from DTS file Vipul Kumar
2018-06-25 10:07   ` Jagan Teki
2018-06-21  9:23 ` [U-Boot] [UBOOT PATCH v3 2/3] spi: xilinx_spi: Modify transfer logic xilinx_spi_xfer() function Vipul Kumar
2018-06-25 10:11   ` Jagan Teki
2018-06-21  9:23 ` [U-Boot] [UBOOT PATCH v3 3/3] spi: xilinx_spi: Added support to read JEDEC-id twice at the boot time Vipul Kumar
2018-06-25 10:17   ` Jagan Teki
2018-06-25 10:36     ` Vipul Kumar
2018-06-27  6:43       ` Jagan Teki
2018-06-27  7:29         ` Michal Simek
2018-06-27  7:35           ` Jagan Teki
2018-06-27  9:47             ` Michal Simek

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.