Hi Marek,
There was some issue and fix is sent
https://lore.kernel.org/u-boot/20241018082644.22495-1-venkatesh.abbar...@amd.com/T/#u
Not sure we need to revert whole parallel/stacked support?
Thanks
Venkatesh
> -----Original Message-----
> From: Marek Vasut <marek.vasut+rene...@mailbox.org>
> Sent: Wednesday, October 23, 2024 2:36 AM
> To: u-boot@lists.denx.de
> Cc: Marek Vasut <marek.vasut+rene...@mailbox.org>; Andre Przywara
> <andre.przyw...@arm.com>; Ashok Reddy Soma <ashok.reddy.s...@amd.com>;
> Jagan Teki <ja...@amarulasolutions.com>; Michael Walle <mwa...@kernel.org>;
> Simek, Michal <michal.si...@amd.com>; Patrice Chotard
> <patrice.chot...@foss.st.com>; Patrick Delaunay
> <patrick.delau...@foss.st.com>;
> Pratyush Yadav <p.ya...@ti.com>; Quentin Schulz <quentin.sch...@cherry.de>;
> Sean Anderson <sean...@gmail.com>; Simon Glass <s...@chromium.org>;
> Takahiro Kuwano <takahiro.kuw...@infineon.com>; Tom Rini
> <tr...@konsulko.com>; Tudor Ambarus <tudor.amba...@linaro.org>; Abbarapu,
> Venkatesh <venkatesh.abbar...@amd.com>; uboot-stm32@st-md-
> mailman.stormreply.com
> Subject: [PATCH 1/6] Revert "spi: zynq_qspi: Add parallel memories support in
> QSPI
> driver"
>
> This reverts commit 1e36d34b52e7a1ebe5a2a5339d6905540f4253aa.
>
> This parallel/stacked support breaks basic SPI NOR support, e.g. this no
> longer
> works:
>
> => sf probe && sf update 0x50000000 0 0x160000
> SF: Detected s25fs512s with page size 256 Bytes, erase size 256 KiB, total 64
> MiB
> device 0 offset 0x0, size 0x160000 SPI flash failed in read step
>
> Since none of this seems to be in Linux either, revert it all.
>
> Signed-off-by: Marek Vasut <marek.vasut+rene...@mailbox.org>
> ---
> Cc: Andre Przywara <andre.przyw...@arm.com>
> Cc: Ashok Reddy Soma <ashok.reddy.s...@amd.com>
> Cc: Jagan Teki <ja...@amarulasolutions.com>
> Cc: Michael Walle <mwa...@kernel.org>
> Cc: Michal Simek <michal.si...@amd.com>
> Cc: Patrice Chotard <patrice.chot...@foss.st.com>
> Cc: Patrick Delaunay <patrick.delau...@foss.st.com>
> Cc: Pratyush Yadav <p.ya...@ti.com>
> Cc: Quentin Schulz <quentin.sch...@cherry.de>
> Cc: Sean Anderson <sean...@gmail.com>
> Cc: Simon Glass <s...@chromium.org>
> Cc: Takahiro Kuwano <takahiro.kuw...@infineon.com>
> Cc: Tom Rini <tr...@konsulko.com>
> Cc: Tudor Ambarus <tudor.amba...@linaro.org>
> Cc: Venkatesh Yadav Abbarapu <venkatesh.abbar...@amd.com>
> Cc: u-boot@lists.denx.de
> Cc: uboot-st...@st-md-mailman.stormreply.com
> ---
> drivers/spi/zynq_qspi.c | 115 ++++------------------------------------
> 1 file changed, 11 insertions(+), 104 deletions(-)
>
> diff --git a/drivers/spi/zynq_qspi.c b/drivers/spi/zynq_qspi.c index
> f5b3fb5c125..e8bc196ce9e 100644
> --- a/drivers/spi/zynq_qspi.c
> +++ b/drivers/spi/zynq_qspi.c
> @@ -1,8 +1,7 @@
> // SPDX-License-Identifier: GPL-2.0+
> /*
> - * (C) Copyright 2013 - 2022, Xilinx, Inc.
> + * (C) Copyright 2013 Xilinx, Inc.
> * (C) Copyright 2015 Jagan Teki <jt...@openedev.com>
> - * (C) Copyright 2023, Advanced Micro Devices, Inc.
> *
> * Xilinx Zynq Quad-SPI(QSPI) controller driver (master mode only)
> */
> @@ -13,12 +12,10 @@
> #include <log.h>
> #include <malloc.h>
> #include <spi.h>
> -#include <spi_flash.h>
> #include <asm/global_data.h>
> #include <asm/io.h>
> #include <linux/bitops.h>
> #include <spi-mem.h>
> -#include "../mtd/spi/sf_internal.h"
>
> DECLARE_GLOBAL_DATA_PTR;
>
> @@ -44,21 +41,6 @@ DECLARE_GLOBAL_DATA_PTR;
> #define ZYNQ_QSPI_TXD_00_01_OFFSET 0x80 /* Transmit 1-byte inst */
> #define ZYNQ_QSPI_TXD_00_10_OFFSET 0x84 /* Transmit 2-byte inst */
> #define ZYNQ_QSPI_TXD_00_11_OFFSET 0x88 /* Transmit 3-byte inst */
> -#define ZYNQ_QSPI_FR_QOUT_CODE 0x6B /* read instruction code
> */
> -
> -#define QSPI_SELECT_LOWER_CS BIT(0)
> -#define QSPI_SELECT_UPPER_CS BIT(1)
> -
> -/*
> - * QSPI Linear Configuration Register
> - *
> - * It is named Linear Configuration but it controls other modes when not in
> - * linear mode also.
> - */
> -#define ZYNQ_QSPI_LCFG_TWO_MEM_MASK 0x40000000 /* QSPI Enable
> Bit Mask */
> -#define ZYNQ_QSPI_LCFG_SEP_BUS_MASK 0x20000000 /* QSPI Enable Bit
> Mask */
> -#define ZYNQ_QSPI_LCFG_U_PAGE 0x10000000 /* QSPI Upper memory
> set */
> -#define ZYNQ_QSPI_LCFG_DUMMY_SHIFT 8
>
> #define ZYNQ_QSPI_TXFIFO_THRESHOLD 1 /* Tx FIFO threshold
> level*/
> #define ZYNQ_QSPI_RXFIFO_THRESHOLD 32 /* Rx FIFO threshold
> level */
> @@ -118,11 +100,7 @@ struct zynq_qspi_priv {
> int bytes_to_transfer;
> int bytes_to_receive;
> unsigned int is_inst;
> - unsigned int is_parallel;
> - unsigned int is_stacked;
> - unsigned int u_page;
> unsigned cs_change:1;
> - unsigned is_strip:1;
> };
>
> static int zynq_qspi_of_to_plat(struct udevice *bus) @@ -133,6 +111,7 @@
> static
> int zynq_qspi_of_to_plat(struct udevice *bus)
>
> plat->regs = (struct zynq_qspi_regs *)fdtdec_get_addr(blob,
> node, "reg");
> +
> return 0;
> }
>
> @@ -167,9 +146,6 @@ static void zynq_qspi_init_hw(struct zynq_qspi_priv *priv)
> /* Disable Interrupts */
> writel(ZYNQ_QSPI_IXR_ALL_MASK, ®s->idr);
>
> - /* Disable linear mode as the boot loader may have used it */
> - writel(0x0, ®s->lqspicfg);
> -
> /* Clear the TX and RX threshold reg */
> writel(ZYNQ_QSPI_TXFIFO_THRESHOLD, ®s->txftr);
> writel(ZYNQ_QSPI_RXFIFO_THRESHOLD, ®s->rxftr); @@ -187,12
> +163,13 @@ static void zynq_qspi_init_hw(struct zynq_qspi_priv *priv)
> confr |= ZYNQ_QSPI_CR_IFMODE_MASK |
> ZYNQ_QSPI_CR_MCS_MASK |
> ZYNQ_QSPI_CR_PCS_MASK | ZYNQ_QSPI_CR_FW_MASK |
> ZYNQ_QSPI_CR_MSTREN_MASK;
> -
> - if (priv->is_stacked)
> - confr |= 0x10;
> -
> writel(confr, ®s->cr);
>
> + /* Disable the LQSPI feature */
> + confr = readl(®s->lqspicfg);
> + confr &= ~ZYNQ_QSPI_LQSPICFG_LQMODE_MASK;
> + writel(confr, ®s->lqspicfg);
> +
> /* Enable SPI */
> writel(ZYNQ_QSPI_ENR_SPI_EN_MASK, ®s->enr); } @@ -203,7
> +180,6 @@ static int zynq_qspi_child_pre_probe(struct udevice *bus)
> struct zynq_qspi_priv *priv = dev_get_priv(bus->parent);
>
> priv->max_hz = slave->max_hz;
> - slave->multi_cs_cap = true;
>
> return 0;
> }
> @@ -386,8 +362,8 @@ static void zynq_qspi_fill_tx_fifo(struct zynq_qspi_priv
> *priv,
> u32 size)
> unsigned len, offset;
> struct zynq_qspi_regs *regs = priv->regs;
> static const unsigned offsets[4] = {
> - ZYNQ_QSPI_TXD_00_01_OFFSET,
> ZYNQ_QSPI_TXD_00_10_OFFSET,
> - ZYNQ_QSPI_TXD_00_11_OFFSET,
> ZYNQ_QSPI_TXD_00_00_OFFSET };
> + ZYNQ_QSPI_TXD_00_00_OFFSET,
> ZYNQ_QSPI_TXD_00_01_OFFSET,
> + ZYNQ_QSPI_TXD_00_10_OFFSET,
> ZYNQ_QSPI_TXD_00_11_OFFSET };
>
> while ((fifocount < size) &&
> (priv->bytes_to_transfer > 0)) {
> @@ -409,11 +385,7 @@ static void zynq_qspi_fill_tx_fifo(struct zynq_qspi_priv
> *priv, u32 size)
> return;
> len = priv->bytes_to_transfer;
> zynq_qspi_write_data(priv, &data, len);
> - if ((priv->is_parallel || priv->is_stacked) &&
> - !priv->is_inst && (len % 2))
> - len++;
> - offset = (priv->rx_buf) ?
> - offsets[3] : offsets[len - 1];
> + offset = (priv->rx_buf) ? offsets[0] : offsets[len];
> writel(data, ®s->cr + (offset / 4));
> }
> }
> @@ -518,7 +490,6 @@ static int zynq_qspi_irq_poll(struct zynq_qspi_priv *priv)
> */
> static int zynq_qspi_start_transfer(struct zynq_qspi_priv *priv) {
> - static u8 current_u_page;
> u32 data = 0;
> struct zynq_qspi_regs *regs = priv->regs;
>
> @@ -528,34 +499,6 @@ static int zynq_qspi_start_transfer(struct zynq_qspi_priv
> *priv)
> priv->bytes_to_transfer = priv->len;
> priv->bytes_to_receive = priv->len;
>
> - if (priv->is_parallel)
> - writel((ZYNQ_QSPI_LCFG_TWO_MEM_MASK |
> - ZYNQ_QSPI_LCFG_SEP_BUS_MASK |
> - (1 << ZYNQ_QSPI_LCFG_DUMMY_SHIFT) |
> - ZYNQ_QSPI_FR_QOUT_CODE), ®s->lqspicfg);
> -
> - if (priv->is_inst && priv->is_stacked && current_u_page !=
> priv->u_page) {
> - if (priv->u_page) {
> - /* Configure two memories on shared bus
> - * by enabling upper mem
> - */
> - writel((ZYNQ_QSPI_LCFG_TWO_MEM_MASK |
> - ZYNQ_QSPI_LCFG_U_PAGE |
> - (1 << ZYNQ_QSPI_LCFG_DUMMY_SHIFT) |
> - ZYNQ_QSPI_FR_QOUT_CODE),
> - ®s->lqspicfg);
> - } else {
> - /* Configure two memories on shared bus
> - * by enabling lower mem
> - */
> - writel((ZYNQ_QSPI_LCFG_TWO_MEM_MASK |
> - (1 << ZYNQ_QSPI_LCFG_DUMMY_SHIFT) |
> - ZYNQ_QSPI_FR_QOUT_CODE),
> - ®s->lqspicfg);
> - }
> - current_u_page = priv->u_page;
> - }
> -
> if (priv->len < 4)
> zynq_qspi_fill_tx_fifo(priv, priv->len);
> else
> @@ -655,8 +598,7 @@ static int zynq_qspi_xfer(struct udevice *dev, unsigned
> int
> bitlen,
> * Assume that the beginning of a transfer with bits to
> * transmit must contain a device command.
> */
> - if ((dout && flags & SPI_XFER_BEGIN) ||
> - (flags & SPI_XFER_END && !priv->is_strip))
> + if (dout && flags & SPI_XFER_BEGIN)
> priv->is_inst = 1;
> else
> priv->is_inst = 0;
> @@ -666,11 +608,6 @@ static int zynq_qspi_xfer(struct udevice *dev, unsigned
> int
> bitlen,
> else
> priv->cs_change = 0;
>
> - if (flags & SPI_XFER_U_PAGE)
> - priv->u_page = 1;
> - else
> - priv->u_page = 0;
> -
> zynq_qspi_transfer(priv);
>
> return 0;
> @@ -734,35 +671,14 @@ static int zynq_qspi_set_mode(struct udevice *bus, uint
> mode)
> return 0;
> }
>
> -bool update_stripe(const struct spi_mem_op *op) -{
> - if (op->cmd.opcode == SPINOR_OP_BE_4K ||
> - op->cmd.opcode == SPINOR_OP_CHIP_ERASE ||
> - op->cmd.opcode == SPINOR_OP_SE ||
> - op->cmd.opcode == SPINOR_OP_WREAR ||
> - op->cmd.opcode == SPINOR_OP_WRSR
> - )
> - return false;
> -
> - return true;
> -}
> -
> static int zynq_qspi_exec_op(struct spi_slave *slave,
> const struct spi_mem_op *op)
> {
> - struct udevice *bus = slave->dev->parent;
> - struct zynq_qspi_priv *priv = dev_get_priv(bus);
> int op_len, pos = 0, ret, i;
> unsigned int flag = 0;
> const u8 *tx_buf = NULL;
> u8 *rx_buf = NULL;
>
> - if ((slave->flags & QSPI_SELECT_LOWER_CS) &&
> - (slave->flags & QSPI_SELECT_UPPER_CS))
> - priv->is_parallel = true;
> - if (slave->flags & SPI_XFER_STACKED)
> - priv->is_stacked = true;
> -
> if (op->data.nbytes) {
> if (op->data.dir == SPI_MEM_DATA_IN)
> rx_buf = op->data.buf.in;
> @@ -787,9 +703,6 @@ static int zynq_qspi_exec_op(struct spi_slave *slave,
> if (op->dummy.nbytes)
> memset(op_buf + pos, 0xff, op->dummy.nbytes);
>
> - if (slave->flags & SPI_XFER_U_PAGE)
> - flag |= SPI_XFER_U_PAGE;
> -
> /* 1st transfer: opcode + address + dummy cycles */
> /* Make sure to set END bit if no tx or rx data messages follow */
> if (!tx_buf && !rx_buf)
> @@ -800,9 +713,6 @@ static int zynq_qspi_exec_op(struct spi_slave *slave,
> if (ret)
> return ret;
>
> - if (priv->is_parallel)
> - priv->is_strip = update_stripe(op);
> -
> /* 2nd transfer: rx or tx data path */
> if (tx_buf || rx_buf) {
> ret = zynq_qspi_xfer(slave->dev, op->data.nbytes * 8, tx_buf,
> @@ -
> 811,9 +721,6 @@ static int zynq_qspi_exec_op(struct spi_slave *slave,
> return ret;
> }
>
> - priv->is_parallel = false;
> - priv->is_stacked = false;
> - slave->flags &= ~SPI_XFER_MASK;
> spi_release_bus(slave);
>
> return 0;
> --
> 2.45.2
