| Message ID | 20241022210633.271534-1-marek.vasut+renesas@mailbox.org |
|---|---|
| State | Superseded |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | [1/6] Revert "spi: zynq_qspi: Add parallel memories support in QSPI driver" | expand |
Hi Marek, There was some issue and fix is sent https://lore.kernel.org/u-boot/20241018082644.22495-1-venkatesh.abbarapu@amd.com/T/#u Not sure we need to revert whole parallel/stacked support? Thanks Venkatesh > -----Original Message----- > From: Marek Vasut <marek.vasut+renesas@mailbox.org> > Sent: Wednesday, October 23, 2024 2:36 AM > To: u-boot@lists.denx.de > Cc: Marek Vasut <marek.vasut+renesas@mailbox.org>; Andre Przywara > <andre.przywara@arm.com>; Ashok Reddy Soma <ashok.reddy.soma@amd.com>; > Jagan Teki <jagan@amarulasolutions.com>; Michael Walle <mwalle@kernel.org>; > Simek, Michal <michal.simek@amd.com>; Patrice Chotard > <patrice.chotard@foss.st.com>; Patrick Delaunay <patrick.delaunay@foss.st.com>; > Pratyush Yadav <p.yadav@ti.com>; Quentin Schulz <quentin.schulz@cherry.de>; > Sean Anderson <seanga2@gmail.com>; Simon Glass <sjg@chromium.org>; > Takahiro Kuwano <Takahiro.Kuwano@infineon.com>; Tom Rini > <trini@konsulko.com>; Tudor Ambarus <tudor.ambarus@linaro.org>; Abbarapu, > Venkatesh <venkatesh.abbarapu@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+renesas@mailbox.org> > --- > Cc: Andre Przywara <andre.przywara@arm.com> > Cc: Ashok Reddy Soma <ashok.reddy.soma@amd.com> > Cc: Jagan Teki <jagan@amarulasolutions.com> > Cc: Michael Walle <mwalle@kernel.org> > Cc: Michal Simek <michal.simek@amd.com> > Cc: Patrice Chotard <patrice.chotard@foss.st.com> > Cc: Patrick Delaunay <patrick.delaunay@foss.st.com> > Cc: Pratyush Yadav <p.yadav@ti.com> > Cc: Quentin Schulz <quentin.schulz@cherry.de> > Cc: Sean Anderson <seanga2@gmail.com> > Cc: Simon Glass <sjg@chromium.org> > Cc: Takahiro Kuwano <Takahiro.Kuwano@infineon.com> > Cc: Tom Rini <trini@konsulko.com> > Cc: Tudor Ambarus <tudor.ambarus@linaro.org> > Cc: Venkatesh Yadav Abbarapu <venkatesh.abbarapu@amd.com> > Cc: u-boot@lists.denx.de > Cc: uboot-stm32@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 <jteki@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
On 10/22/24 23:06, Marek Vasut wrote: > 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 Reverting everything seems to me too much. Tom has tested it on his HW and didn't see any issue. That's why better to look at code which is causing this. You are reverting everything but likely there is specific patch which is causing this. Which one is it? Which board was used for your testing? Likely we don't have access to it. Is there any QEMU available which can be used for debugging? > > Since none of this seems to be in Linux either, revert it all. This has been discussed with Tom before. It wasn't in sync even before and we can't really stop development on subsystems. Thanks, Michal
On 10/23/24 10:17 AM, Michal Simek wrote: > > > On 10/22/24 23:06, Marek Vasut wrote: >> 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 > > Reverting everything seems to me too much. Tom has tested it on his HW > and didn't see any issue. That's why better to look at code which is > causing this. > You are reverting everything but likely there is specific patch which is > causing this. Which one is it? > Which board was used for your testing? Likely we don't have access to it. > Is there any QEMU available which can be used for debugging? The testcase including the exact SPI NOR model is above. iMX6 with w25q16dw seems to be broken too. Basically every board I have access no longer has a working "sf probe ; sf update" combination ... so yeah, this means this patchset is fundamentally broken. >> Since none of this seems to be in Linux either, revert it all. > > This has been discussed with Tom before. Tom is not the U-Boot MTD maintainer ? > It wasn't in sync even before > and we can't really stop development on subsystems. These patches have been rejected from mainline Linux for many years, why are they added to U-Boot which includes an import of MTD subsystem from Linux ? Such diverging code base with random patches will only make it much harder to synchronize Linux MTD subsystem back to U-Boot.
On 10/23/24 5:18 AM, Abbarapu, Venkatesh wrote: > Hi Marek, > There was some issue and fix is sent > https://lore.kernel.org/u-boot/20241018082644.22495-1-venkatesh.abbarapu@amd.com/T/#u Is this one fix or three fixes for three different issues ? This seems to fix READ errors, which is apparently another error introduced by this stuff. In my case, plain and simply 'sf probe ; sf update' combination with single non-stacked SPI NOR does not work. Was such a simple configuration ever tested ? > Not sure we need to revert whole parallel/stacked support? Please stop top-posting.
Hi, Tested with the non-stacked default single configuration on ZynqMP zcu102 board and didn’t see any issue. ZynqMP> sf probe 0 0 0 SF: Detected mt25qu512a with page size 256 Bytes, erase size 64 KiB, total 64 MiB ZynqMP> sf erase 0x0 0x4000000;mw.b 0x8000 aabbccdd 0x4000000;sf write 0x8000 0x0 0x4000000;mw.b 0x8008000 0x0 0x4000000;sf read 0x8008000 0x0 0x4000000;cmp.b 0x8000 0x8008000 0x4000000 SF: 67108864 bytes @ 0x0 Erased: OK device 0 whole chip SF: 67108864 bytes @ 0x0 Written: OK device 0 whole chip SF: 67108864 bytes @ 0x0 Read: OK Total of 67108864 byte(s) were the same Thanks Venkatesh > -----Original Message----- > From: Marek Vasut <marek.vasut@mailbox.org> > Sent: Wednesday, October 23, 2024 2:12 PM > To: Abbarapu, Venkatesh <venkatesh.abbarapu@amd.com>; Marek Vasut > <marek.vasut+renesas@mailbox.org>; u-boot@lists.denx.de > Cc: Andre Przywara <andre.przywara@arm.com>; Ashok Reddy Soma > <ashok.reddy.soma@amd.com>; Jagan Teki <jagan@amarulasolutions.com>; > Michael Walle <mwalle@kernel.org>; Simek, Michal <michal.simek@amd.com>; > Patrice Chotard <patrice.chotard@foss.st.com>; Patrick Delaunay > <patrick.delaunay@foss.st.com>; Pratyush Yadav <p.yadav@ti.com>; Quentin > Schulz <quentin.schulz@cherry.de>; Sean Anderson <seanga2@gmail.com>; > Simon Glass <sjg@chromium.org>; Takahiro Kuwano > <Takahiro.Kuwano@infineon.com>; Tom Rini <trini@konsulko.com>; Tudor > Ambarus <tudor.ambarus@linaro.org>; uboot-stm32@st-md- > mailman.stormreply.com > Subject: Re: [PATCH 1/6] Revert "spi: zynq_qspi: Add parallel memories support in > QSPI driver" > > On 10/23/24 5:18 AM, Abbarapu, Venkatesh wrote: > > Hi Marek, > > There was some issue and fix is sent > > https://lore.kernel.org/u-boot/20241018082644.22495-1-venkatesh.abbara > > pu@amd.com/T/#u > > Is this one fix or three fixes for three different issues ? > > This seems to fix READ errors, which is apparently another error introduced by this > stuff. In my case, plain and simply 'sf probe ; sf update' combination with single non- > stacked SPI NOR does not work. Was such a simple configuration ever tested ? > > > Not sure we need to revert whole parallel/stacked support? > Please stop top-posting.
Hi Marek On Wed, Oct 23, 2024 at 10:41 AM Marek Vasut <marek.vasut@mailbox.org> wrote: > > On 10/23/24 5:18 AM, Abbarapu, Venkatesh wrote: > > Hi Marek, > > There was some issue and fix is sent > > https://lore.kernel.org/u-boot/20241018082644.22495-1-venkatesh.abbarapu@amd.com/T/#u > > Is this one fix or three fixes for three different issues ? > > This seems to fix READ errors, which is apparently another error > introduced by this stuff. In my case, plain and simply 'sf probe ; sf > update' combination with single non-stacked SPI NOR does not work. Was > such a simple configuration ever tested ? > > > Not sure we need to revert whole parallel/stacked support? > Please stop top-posting. spi-nor was taken care buy Jagan but I dont' know if he is having time. This series was not merged from our side Michael
On 10/23/24 11:07 AM, Abbarapu, Venkatesh wrote: > Hi, > Tested with the non-stacked default single configuration on ZynqMP zcu102 board and didn’t see any issue. > > ZynqMP> sf probe 0 0 0 > SF: Detected mt25qu512a with page size 256 Bytes, erase size 64 KiB, total 64 MiB > ZynqMP> sf erase 0x0 0x4000000;mw.b 0x8000 aabbccdd 0x4000000;sf write 0x8000 0x0 0x4000000;mw.b 0x8008000 0x0 0x4000000;sf read 0x8008000 0x0 0x4000000;cmp.b 0x8000 0x8008000 0x4000000 > SF: 67108864 bytes @ 0x0 Erased: OK > device 0 whole chip > SF: 67108864 bytes @ 0x0 Written: OK > device 0 whole chip > SF: 67108864 bytes @ 0x0 Read: OK > Total of 67108864 byte(s) were the same > > Thanks > Venkatesh > >> -----Original Message----- >> From: Marek Vasut <marek.vasut@mailbox.org> >> Sent: Wednesday, October 23, 2024 2:12 PM >> To: Abbarapu, Venkatesh <venkatesh.abbarapu@amd.com>; Marek Vasut >> <marek.vasut+renesas@mailbox.org>; u-boot@lists.denx.de >> Cc: Andre Przywara <andre.przywara@arm.com>; Ashok Reddy Soma >> <ashok.reddy.soma@amd.com>; Jagan Teki <jagan@amarulasolutions.com>; >> Michael Walle <mwalle@kernel.org>; Simek, Michal <michal.simek@amd.com>; >> Patrice Chotard <patrice.chotard@foss.st.com>; Patrick Delaunay >> <patrick.delaunay@foss.st.com>; Pratyush Yadav <p.yadav@ti.com>; Quentin >> Schulz <quentin.schulz@cherry.de>; Sean Anderson <seanga2@gmail.com>; >> Simon Glass <sjg@chromium.org>; Takahiro Kuwano >> <Takahiro.Kuwano@infineon.com>; Tom Rini <trini@konsulko.com>; Tudor >> Ambarus <tudor.ambarus@linaro.org>; uboot-stm32@st-md- >> mailman.stormreply.com >> Subject: Re: [PATCH 1/6] Revert "spi: zynq_qspi: Add parallel memories support in >> QSPI driver" >> >> On 10/23/24 5:18 AM, Abbarapu, Venkatesh wrote: >>> Hi Marek, >>> There was some issue and fix is sent >>> https://lore.kernel.org/u-boot/20241018082644.22495-1-venkatesh.abbara >>> pu@amd.com/T/#u >> >> Is this one fix or three fixes for three different issues ? >> >> This seems to fix READ errors, which is apparently another error introduced by this >> stuff. In my case, plain and simply 'sf probe ; sf update' combination with single non- >> stacked SPI NOR does not work. Was such a simple configuration ever tested ? >> >>> Not sure we need to revert whole parallel/stacked support? >> Please stop top-posting. You ran completely different test on completely different chip. Stop top posting.
Hi, > -----Original Message----- > From: Marek Vasut <marek.vasut@mailbox.org> > Sent: Wednesday, October 23, 2024 6:15 PM > To: Abbarapu, Venkatesh <venkatesh.abbarapu@amd.com>; u-boot@lists.denx.de > Cc: Andre Przywara <andre.przywara@arm.com>; Ashok Reddy Soma > <ashok.reddy.soma@amd.com>; Jagan Teki <jagan@amarulasolutions.com>; > Michael Walle <mwalle@kernel.org>; Simek, Michal <michal.simek@amd.com>; > Patrice Chotard <patrice.chotard@foss.st.com>; Patrick Delaunay > <patrick.delaunay@foss.st.com>; Pratyush Yadav <p.yadav@ti.com>; Quentin > Schulz <quentin.schulz@cherry.de>; Sean Anderson <seanga2@gmail.com>; > Simon Glass <sjg@chromium.org>; Takahiro Kuwano > <Takahiro.Kuwano@infineon.com>; Tom Rini <trini@konsulko.com>; Tudor > Ambarus <tudor.ambarus@linaro.org>; uboot-stm32@st-md- > mailman.stormreply.com > Subject: Re: [PATCH 1/6] Revert "spi: zynq_qspi: Add parallel memories support in > QSPI driver" > > On 10/23/24 11:07 AM, Abbarapu, Venkatesh wrote: > > Hi, > > Tested with the non-stacked default single configuration on ZynqMP zcu102 board > and didn’t see any issue. > > > > ZynqMP> sf probe 0 0 0 > > SF: Detected mt25qu512a with page size 256 Bytes, erase size 64 KiB, > > total 64 MiB > > ZynqMP> sf erase 0x0 0x4000000;mw.b 0x8000 aabbccdd 0x4000000;sf write > > ZynqMP> 0x8000 0x0 0x4000000;mw.b 0x8008000 0x0 0x4000000;sf read > > ZynqMP> 0x8008000 0x0 0x4000000;cmp.b 0x8000 0x8008000 0x4000000 > > SF: 67108864 bytes @ 0x0 Erased: OK > > device 0 whole chip > > SF: 67108864 bytes @ 0x0 Written: OK > > device 0 whole chip > > SF: 67108864 bytes @ 0x0 Read: OK > > Total of 67108864 byte(s) were the same > > > > Thanks > > Venkatesh > > > >> -----Original Message----- > >> From: Marek Vasut <marek.vasut@mailbox.org> > >> Sent: Wednesday, October 23, 2024 2:12 PM > >> To: Abbarapu, Venkatesh <venkatesh.abbarapu@amd.com>; Marek Vasut > >> <marek.vasut+renesas@mailbox.org>; u-boot@lists.denx.de > >> Cc: Andre Przywara <andre.przywara@arm.com>; Ashok Reddy Soma > >> <ashok.reddy.soma@amd.com>; Jagan Teki <jagan@amarulasolutions.com>; > >> Michael Walle <mwalle@kernel.org>; Simek, Michal > >> <michal.simek@amd.com>; Patrice Chotard > >> <patrice.chotard@foss.st.com>; Patrick Delaunay > >> <patrick.delaunay@foss.st.com>; Pratyush Yadav <p.yadav@ti.com>; > >> Quentin Schulz <quentin.schulz@cherry.de>; Sean Anderson > >> <seanga2@gmail.com>; Simon Glass <sjg@chromium.org>; Takahiro Kuwano > >> <Takahiro.Kuwano@infineon.com>; Tom Rini <trini@konsulko.com>; Tudor > >> Ambarus <tudor.ambarus@linaro.org>; uboot-stm32@st-md- > >> mailman.stormreply.com > >> Subject: Re: [PATCH 1/6] Revert "spi: zynq_qspi: Add parallel > >> memories support in QSPI driver" > >> > >> On 10/23/24 5:18 AM, Abbarapu, Venkatesh wrote: > >>> Hi Marek, > >>> There was some issue and fix is sent > >>> https://lore.kernel.org/u-boot/20241018082644.22495-1-venkatesh.abba > >>> ra > >>> pu@amd.com/T/#u > >> > >> Is this one fix or three fixes for three different issues ? > >> > >> This seems to fix READ errors, which is apparently another error > >> introduced by this stuff. In my case, plain and simply 'sf probe ; sf > >> update' combination with single non- stacked SPI NOR does not work. Was such > a simple configuration ever tested ? > >> > >>> Not sure we need to revert whole parallel/stacked support? > >> Please stop top-posting. > > You ran completely different test on completely different chip. > > Stop top posting. Sorry for top posting Will try to get the spansion flash part and try the below tests. At this point tried testing on different board with different flash part. Zynq> sf probe 0 0 0 SF: Detected mx66l1g45g with page size 256 Bytes, erase size 64 KiB, total 128 MiB Zynq> sf update 0x4000000 0 0x160000 device 0 offset 0x0, size 0x160000 1441792 bytes written, 0 bytes skipped in 5.735s, speed 257435 B/s Zynq> sf erase 0x0 0x4000000;mw.b 0x8000 aabbccdd 0x4000000;sf write 0x8000 0x0 0x4000000;mw.b 0x8008000 0x0 0x4000000;sf read 0x8008000 0x0 0x4000000;cmp.b 0x8000 0x8008000 0x4000000 SF: 67108864 bytes @ 0x0 Erased: OK device 0 offset 0x0, size 0x4000000 SF: 67108864 bytes @ 0x0 Written: OK device 0 offset 0x0, size 0x4000000 SF: 67108864 bytes @ 0x0 Read: OK Total of 67108864 byte(s) were the same Thanks Venkatesh
On 10/23/24 4:14 PM, Abbarapu, Venkatesh wrote: > Hi, > >> -----Original Message----- >> From: Marek Vasut <marek.vasut@mailbox.org> >> Sent: Wednesday, October 23, 2024 6:15 PM >> To: Abbarapu, Venkatesh <venkatesh.abbarapu@amd.com>; u-boot@lists.denx.de >> Cc: Andre Przywara <andre.przywara@arm.com>; Ashok Reddy Soma >> <ashok.reddy.soma@amd.com>; Jagan Teki <jagan@amarulasolutions.com>; >> Michael Walle <mwalle@kernel.org>; Simek, Michal <michal.simek@amd.com>; >> Patrice Chotard <patrice.chotard@foss.st.com>; Patrick Delaunay >> <patrick.delaunay@foss.st.com>; Pratyush Yadav <p.yadav@ti.com>; Quentin >> Schulz <quentin.schulz@cherry.de>; Sean Anderson <seanga2@gmail.com>; >> Simon Glass <sjg@chromium.org>; Takahiro Kuwano >> <Takahiro.Kuwano@infineon.com>; Tom Rini <trini@konsulko.com>; Tudor >> Ambarus <tudor.ambarus@linaro.org>; uboot-stm32@st-md- >> mailman.stormreply.com >> Subject: Re: [PATCH 1/6] Revert "spi: zynq_qspi: Add parallel memories support in >> QSPI driver" >> >> On 10/23/24 11:07 AM, Abbarapu, Venkatesh wrote: >>> Hi, >>> Tested with the non-stacked default single configuration on ZynqMP zcu102 board >> and didn’t see any issue. >>> >>> ZynqMP> sf probe 0 0 0 >>> SF: Detected mt25qu512a with page size 256 Bytes, erase size 64 KiB, >>> total 64 MiB >>> ZynqMP> sf erase 0x0 0x4000000;mw.b 0x8000 aabbccdd 0x4000000;sf write >>> ZynqMP> 0x8000 0x0 0x4000000;mw.b 0x8008000 0x0 0x4000000;sf read >>> ZynqMP> 0x8008000 0x0 0x4000000;cmp.b 0x8000 0x8008000 0x4000000 >>> SF: 67108864 bytes @ 0x0 Erased: OK >>> device 0 whole chip >>> SF: 67108864 bytes @ 0x0 Written: OK >>> device 0 whole chip >>> SF: 67108864 bytes @ 0x0 Read: OK >>> Total of 67108864 byte(s) were the same >>> >>> Thanks >>> Venkatesh >>> >>>> -----Original Message----- >>>> From: Marek Vasut <marek.vasut@mailbox.org> >>>> Sent: Wednesday, October 23, 2024 2:12 PM >>>> To: Abbarapu, Venkatesh <venkatesh.abbarapu@amd.com>; Marek Vasut >>>> <marek.vasut+renesas@mailbox.org>; u-boot@lists.denx.de >>>> Cc: Andre Przywara <andre.przywara@arm.com>; Ashok Reddy Soma >>>> <ashok.reddy.soma@amd.com>; Jagan Teki <jagan@amarulasolutions.com>; >>>> Michael Walle <mwalle@kernel.org>; Simek, Michal >>>> <michal.simek@amd.com>; Patrice Chotard >>>> <patrice.chotard@foss.st.com>; Patrick Delaunay >>>> <patrick.delaunay@foss.st.com>; Pratyush Yadav <p.yadav@ti.com>; >>>> Quentin Schulz <quentin.schulz@cherry.de>; Sean Anderson >>>> <seanga2@gmail.com>; Simon Glass <sjg@chromium.org>; Takahiro Kuwano >>>> <Takahiro.Kuwano@infineon.com>; Tom Rini <trini@konsulko.com>; Tudor >>>> Ambarus <tudor.ambarus@linaro.org>; uboot-stm32@st-md- >>>> mailman.stormreply.com >>>> Subject: Re: [PATCH 1/6] Revert "spi: zynq_qspi: Add parallel >>>> memories support in QSPI driver" >>>> >>>> On 10/23/24 5:18 AM, Abbarapu, Venkatesh wrote: >>>>> Hi Marek, >>>>> There was some issue and fix is sent >>>>> https://lore.kernel.org/u-boot/20241018082644.22495-1-venkatesh.abba >>>>> ra >>>>> pu@amd.com/T/#u >>>> >>>> Is this one fix or three fixes for three different issues ? >>>> >>>> This seems to fix READ errors, which is apparently another error >>>> introduced by this stuff. In my case, plain and simply 'sf probe ; sf >>>> update' combination with single non- stacked SPI NOR does not work. Was such >> a simple configuration ever tested ? >>>> >>>>> Not sure we need to revert whole parallel/stacked support? >>>> Please stop top-posting. >> >> You ran completely different test on completely different chip. >> >> Stop top posting. > > Sorry for top posting > > Will try to get the spansion flash part and try the below tests. > At this point tried testing on different board with different flash part. > > Zynq> sf probe 0 0 0 > SF: Detected mx66l1g45g with page size 256 Bytes, erase size 64 KiB, total 128 MiB > Zynq> sf update 0x4000000 0 0x160000 > device 0 offset 0x0, size 0x160000 > 1441792 bytes written, 0 bytes skipped in 5.735s, speed 257435 B/s > > Zynq> sf erase 0x0 0x4000000;mw.b 0x8000 aabbccdd 0x4000000;sf write 0x8000 0x0 0x4000000;mw.b 0x8008000 0x0 0x4000000;sf read 0x8008000 0x0 0x4000000;cmp.b 0x8000 0x8008000 0x4000000 > SF: 67108864 bytes @ 0x0 Erased: OK > device 0 offset 0x0, size 0x4000000 > SF: 67108864 bytes @ 0x0 Written: OK > device 0 offset 0x0, size 0x4000000 > SF: 67108864 bytes @ 0x0 Read: OK > Total of 67108864 byte(s) were the same Commit message reads: " 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 " You ran completely different test on completely different chip. The test is "sf probe && sf update 0x50000000 0 0x160000" , did you ever test "sf update" ?
Hi, > -----Original Message----- > From: Marek Vasut <marek.vasut@mailbox.org> > Sent: Wednesday, October 23, 2024 7:49 PM > To: Abbarapu, Venkatesh <venkatesh.abbarapu@amd.com>; u-boot@lists.denx.de > Cc: Andre Przywara <andre.przywara@arm.com>; Ashok Reddy Soma > <ashok.reddy.soma@amd.com>; Jagan Teki <jagan@amarulasolutions.com>; > Michael Walle <mwalle@kernel.org>; Simek, Michal <michal.simek@amd.com>; > Patrice Chotard <patrice.chotard@foss.st.com>; Patrick Delaunay > <patrick.delaunay@foss.st.com>; Pratyush Yadav <p.yadav@ti.com>; Quentin > Schulz <quentin.schulz@cherry.de>; Sean Anderson <seanga2@gmail.com>; > Simon Glass <sjg@chromium.org>; Takahiro Kuwano > <Takahiro.Kuwano@infineon.com>; Tom Rini <trini@konsulko.com>; Tudor > Ambarus <tudor.ambarus@linaro.org>; uboot-stm32@st-md- > mailman.stormreply.com > Subject: Re: [PATCH 1/6] Revert "spi: zynq_qspi: Add parallel memories support in > QSPI driver" > > On 10/23/24 4:14 PM, Abbarapu, Venkatesh wrote: > > Hi, > > > >> -----Original Message----- > >> From: Marek Vasut <marek.vasut@mailbox.org> > >> Sent: Wednesday, October 23, 2024 6:15 PM > >> To: Abbarapu, Venkatesh <venkatesh.abbarapu@amd.com>; > >> u-boot@lists.denx.de > >> Cc: Andre Przywara <andre.przywara@arm.com>; Ashok Reddy Soma > >> <ashok.reddy.soma@amd.com>; Jagan Teki <jagan@amarulasolutions.com>; > >> Michael Walle <mwalle@kernel.org>; Simek, Michal > >> <michal.simek@amd.com>; Patrice Chotard > >> <patrice.chotard@foss.st.com>; Patrick Delaunay > >> <patrick.delaunay@foss.st.com>; Pratyush Yadav <p.yadav@ti.com>; > >> Quentin Schulz <quentin.schulz@cherry.de>; Sean Anderson > >> <seanga2@gmail.com>; Simon Glass <sjg@chromium.org>; Takahiro Kuwano > >> <Takahiro.Kuwano@infineon.com>; Tom Rini <trini@konsulko.com>; Tudor > >> Ambarus <tudor.ambarus@linaro.org>; uboot-stm32@st-md- > >> mailman.stormreply.com > >> Subject: Re: [PATCH 1/6] Revert "spi: zynq_qspi: Add parallel > >> memories support in QSPI driver" > >> > >> On 10/23/24 11:07 AM, Abbarapu, Venkatesh wrote: > >>> Hi, > >>> Tested with the non-stacked default single configuration on ZynqMP > >>> zcu102 board > >> and didn’t see any issue. > >>> > >>> ZynqMP> sf probe 0 0 0 > >>> SF: Detected mt25qu512a with page size 256 Bytes, erase size 64 KiB, > >>> total 64 MiB > >>> ZynqMP> sf erase 0x0 0x4000000;mw.b 0x8000 aabbccdd 0x4000000;sf > >>> ZynqMP> write > >>> ZynqMP> 0x8000 0x0 0x4000000;mw.b 0x8008000 0x0 0x4000000;sf read > >>> ZynqMP> 0x8008000 0x0 0x4000000;cmp.b 0x8000 0x8008000 0x4000000 > >>> SF: 67108864 bytes @ 0x0 Erased: OK > >>> device 0 whole chip > >>> SF: 67108864 bytes @ 0x0 Written: OK device 0 whole chip > >>> SF: 67108864 bytes @ 0x0 Read: OK > >>> Total of 67108864 byte(s) were the same > >>> > >>> Thanks > >>> Venkatesh > >>> > >>>> -----Original Message----- > >>>> From: Marek Vasut <marek.vasut@mailbox.org> > >>>> Sent: Wednesday, October 23, 2024 2:12 PM > >>>> To: Abbarapu, Venkatesh <venkatesh.abbarapu@amd.com>; Marek Vasut > >>>> <marek.vasut+renesas@mailbox.org>; u-boot@lists.denx.de > >>>> Cc: Andre Przywara <andre.przywara@arm.com>; Ashok Reddy Soma > >>>> <ashok.reddy.soma@amd.com>; Jagan Teki > >>>> <jagan@amarulasolutions.com>; Michael Walle <mwalle@kernel.org>; > >>>> Simek, Michal <michal.simek@amd.com>; Patrice Chotard > >>>> <patrice.chotard@foss.st.com>; Patrick Delaunay > >>>> <patrick.delaunay@foss.st.com>; Pratyush Yadav <p.yadav@ti.com>; > >>>> Quentin Schulz <quentin.schulz@cherry.de>; Sean Anderson > >>>> <seanga2@gmail.com>; Simon Glass <sjg@chromium.org>; Takahiro > >>>> Kuwano <Takahiro.Kuwano@infineon.com>; Tom Rini > >>>> <trini@konsulko.com>; Tudor Ambarus <tudor.ambarus@linaro.org>; > >>>> uboot-stm32@st-md- mailman.stormreply.com > >>>> Subject: Re: [PATCH 1/6] Revert "spi: zynq_qspi: Add parallel > >>>> memories support in QSPI driver" > >>>> > >>>> On 10/23/24 5:18 AM, Abbarapu, Venkatesh wrote: > >>>>> Hi Marek, > >>>>> There was some issue and fix is sent > >>>>> https://lore.kernel.org/u-boot/20241018082644.22495-1-venkatesh.ab > >>>>> ba > >>>>> ra > >>>>> pu@amd.com/T/#u > >>>> > >>>> Is this one fix or three fixes for three different issues ? > >>>> > >>>> This seems to fix READ errors, which is apparently another error > >>>> introduced by this stuff. In my case, plain and simply 'sf probe ; > >>>> sf update' combination with single non- stacked SPI NOR does not > >>>> work. Was such > >> a simple configuration ever tested ? > >>>> > >>>>> Not sure we need to revert whole parallel/stacked support? > >>>> Please stop top-posting. > >> > >> You ran completely different test on completely different chip. > >> > >> Stop top posting. > > > > Sorry for top posting > > > > Will try to get the spansion flash part and try the below tests. > > At this point tried testing on different board with different flash part. > > > > Zynq> sf probe 0 0 0 > > SF: Detected mx66l1g45g with page size 256 Bytes, erase size 64 KiB, > > total 128 MiB > > Zynq> sf update 0x4000000 0 0x160000 > > device 0 offset 0x0, size 0x160000 > > 1441792 bytes written, 0 bytes skipped in 5.735s, speed 257435 B/s > > > > Zynq> sf erase 0x0 0x4000000;mw.b 0x8000 aabbccdd 0x4000000;sf write > > Zynq> 0x8000 0x0 0x4000000;mw.b 0x8008000 0x0 0x4000000;sf read > > Zynq> 0x8008000 0x0 0x4000000;cmp.b 0x8000 0x8008000 0x4000000 > > SF: 67108864 bytes @ 0x0 Erased: OK > > device 0 offset 0x0, size 0x4000000 > > SF: 67108864 bytes @ 0x0 Written: OK > > device 0 offset 0x0, size 0x4000000 > > SF: 67108864 bytes @ 0x0 Read: OK > > Total of 67108864 byte(s) were the same > Commit message reads: > > " > 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 " > > You ran completely different test on completely different chip. > > The test is "sf probe && sf update 0x50000000 0 0x160000" , did you ever test "sf > update" ? Tried the test " sf probe && sf update 0x4000000 0 0x160000" Zynq> sf probe && sf update 0x4000000 0 0x160000 SF: Detected mx66l1g45g with page size 256 Bytes, erase size 64 KiB, total 128 MiB device 0 offset 0x0, size 0x160000 0 bytes written, 1441792 bytes skipped in 0.154s, speed 9586980 B/s Zynq> Thanks Venkatesh
Hi, > -----Original Message----- > From: Abbarapu, Venkatesh > Sent: Wednesday, October 23, 2024 7:55 PM > To: Marek Vasut <marek.vasut@mailbox.org>; u-boot@lists.denx.de > Cc: Andre Przywara <andre.przywara@arm.com>; Ashok Reddy Soma > <ashok.reddy.soma@amd.com>; Jagan Teki <jagan@amarulasolutions.com>; > Michael Walle <mwalle@kernel.org>; Simek, Michal <michal.simek@amd.com>; > Patrice Chotard <patrice.chotard@foss.st.com>; Patrick Delaunay > <patrick.delaunay@foss.st.com>; Pratyush Yadav <p.yadav@ti.com>; Quentin > Schulz <quentin.schulz@cherry.de>; Sean Anderson <seanga2@gmail.com>; > Simon Glass <sjg@chromium.org>; Takahiro Kuwano > <Takahiro.Kuwano@infineon.com>; Tom Rini <trini@konsulko.com>; Tudor > Ambarus <tudor.ambarus@linaro.org>; uboot-stm32@st-md- > mailman.stormreply.com > Subject: RE: [PATCH 1/6] Revert "spi: zynq_qspi: Add parallel memories support in > QSPI driver" > > Hi, > > > -----Original Message----- > > From: Marek Vasut <marek.vasut@mailbox.org> > > Sent: Wednesday, October 23, 2024 7:49 PM > > To: Abbarapu, Venkatesh <venkatesh.abbarapu@amd.com>; > > u-boot@lists.denx.de > > Cc: Andre Przywara <andre.przywara@arm.com>; Ashok Reddy Soma > > <ashok.reddy.soma@amd.com>; Jagan Teki <jagan@amarulasolutions.com>; > > Michael Walle <mwalle@kernel.org>; Simek, Michal > > <michal.simek@amd.com>; Patrice Chotard <patrice.chotard@foss.st.com>; > > Patrick Delaunay <patrick.delaunay@foss.st.com>; Pratyush Yadav > > <p.yadav@ti.com>; Quentin Schulz <quentin.schulz@cherry.de>; Sean > > Anderson <seanga2@gmail.com>; Simon Glass <sjg@chromium.org>; Takahiro > > Kuwano <Takahiro.Kuwano@infineon.com>; Tom Rini <trini@konsulko.com>; > > Tudor Ambarus <tudor.ambarus@linaro.org>; uboot-stm32@st-md- > > mailman.stormreply.com > > Subject: Re: [PATCH 1/6] Revert "spi: zynq_qspi: Add parallel memories > > support in QSPI driver" > > > > On 10/23/24 4:14 PM, Abbarapu, Venkatesh wrote: > > > Hi, > > > > > >> -----Original Message----- > > >> From: Marek Vasut <marek.vasut@mailbox.org> > > >> Sent: Wednesday, October 23, 2024 6:15 PM > > >> To: Abbarapu, Venkatesh <venkatesh.abbarapu@amd.com>; > > >> u-boot@lists.denx.de > > >> Cc: Andre Przywara <andre.przywara@arm.com>; Ashok Reddy Soma > > >> <ashok.reddy.soma@amd.com>; Jagan Teki > > >> <jagan@amarulasolutions.com>; Michael Walle <mwalle@kernel.org>; > > >> Simek, Michal <michal.simek@amd.com>; Patrice Chotard > > >> <patrice.chotard@foss.st.com>; Patrick Delaunay > > >> <patrick.delaunay@foss.st.com>; Pratyush Yadav <p.yadav@ti.com>; > > >> Quentin Schulz <quentin.schulz@cherry.de>; Sean Anderson > > >> <seanga2@gmail.com>; Simon Glass <sjg@chromium.org>; Takahiro > > >> Kuwano <Takahiro.Kuwano@infineon.com>; Tom Rini > > >> <trini@konsulko.com>; Tudor Ambarus <tudor.ambarus@linaro.org>; > > >> uboot-stm32@st-md- mailman.stormreply.com > > >> Subject: Re: [PATCH 1/6] Revert "spi: zynq_qspi: Add parallel > > >> memories support in QSPI driver" > > >> > > >> On 10/23/24 11:07 AM, Abbarapu, Venkatesh wrote: > > >>> Hi, > > >>> Tested with the non-stacked default single configuration on ZynqMP > > >>> zcu102 board > > >> and didn’t see any issue. > > >>> > > >>> ZynqMP> sf probe 0 0 0 > > >>> SF: Detected mt25qu512a with page size 256 Bytes, erase size 64 > > >>> KiB, total 64 MiB > > >>> ZynqMP> sf erase 0x0 0x4000000;mw.b 0x8000 aabbccdd 0x4000000;sf > > >>> ZynqMP> write > > >>> ZynqMP> 0x8000 0x0 0x4000000;mw.b 0x8008000 0x0 0x4000000;sf read > > >>> ZynqMP> 0x8008000 0x0 0x4000000;cmp.b 0x8000 0x8008000 0x4000000 > > >>> SF: 67108864 bytes @ 0x0 Erased: OK device 0 whole chip > > >>> SF: 67108864 bytes @ 0x0 Written: OK device 0 whole chip > > >>> SF: 67108864 bytes @ 0x0 Read: OK > > >>> Total of 67108864 byte(s) were the same > > >>> > > >>> Thanks > > >>> Venkatesh > > >>> > > >>>> -----Original Message----- > > >>>> From: Marek Vasut <marek.vasut@mailbox.org> > > >>>> Sent: Wednesday, October 23, 2024 2:12 PM > > >>>> To: Abbarapu, Venkatesh <venkatesh.abbarapu@amd.com>; Marek Vasut > > >>>> <marek.vasut+renesas@mailbox.org>; u-boot@lists.denx.de > > >>>> Cc: Andre Przywara <andre.przywara@arm.com>; Ashok Reddy Soma > > >>>> <ashok.reddy.soma@amd.com>; Jagan Teki > > >>>> <jagan@amarulasolutions.com>; Michael Walle <mwalle@kernel.org>; > > >>>> Simek, Michal <michal.simek@amd.com>; Patrice Chotard > > >>>> <patrice.chotard@foss.st.com>; Patrick Delaunay > > >>>> <patrick.delaunay@foss.st.com>; Pratyush Yadav <p.yadav@ti.com>; > > >>>> Quentin Schulz <quentin.schulz@cherry.de>; Sean Anderson > > >>>> <seanga2@gmail.com>; Simon Glass <sjg@chromium.org>; Takahiro > > >>>> Kuwano <Takahiro.Kuwano@infineon.com>; Tom Rini > > >>>> <trini@konsulko.com>; Tudor Ambarus <tudor.ambarus@linaro.org>; > > >>>> uboot-stm32@st-md- mailman.stormreply.com > > >>>> Subject: Re: [PATCH 1/6] Revert "spi: zynq_qspi: Add parallel > > >>>> memories support in QSPI driver" > > >>>> > > >>>> On 10/23/24 5:18 AM, Abbarapu, Venkatesh wrote: > > >>>>> Hi Marek, > > >>>>> There was some issue and fix is sent > > >>>>> https://lore.kernel.org/u-boot/20241018082644.22495-1-venkatesh. > > >>>>> ab > > >>>>> ba > > >>>>> ra > > >>>>> pu@amd.com/T/#u > > >>>> > > >>>> Is this one fix or three fixes for three different issues ? > > >>>> > > >>>> This seems to fix READ errors, which is apparently another error > > >>>> introduced by this stuff. In my case, plain and simply 'sf probe > > >>>> ; sf update' combination with single non- stacked SPI NOR does > > >>>> not work. Was such > > >> a simple configuration ever tested ? > > >>>> > > >>>>> Not sure we need to revert whole parallel/stacked support? > > >>>> Please stop top-posting. > > >> > > >> You ran completely different test on completely different chip. > > >> > > >> Stop top posting. > > > > > > Sorry for top posting > > > > > > Will try to get the spansion flash part and try the below tests. > > > At this point tried testing on different board with different flash part. > > > > > > Zynq> sf probe 0 0 0 > > > SF: Detected mx66l1g45g with page size 256 Bytes, erase size 64 KiB, > > > total 128 MiB > > > Zynq> sf update 0x4000000 0 0x160000 > > > device 0 offset 0x0, size 0x160000 > > > 1441792 bytes written, 0 bytes skipped in 5.735s, speed 257435 B/s > > > > > > Zynq> sf erase 0x0 0x4000000;mw.b 0x8000 aabbccdd 0x4000000;sf write > > > Zynq> 0x8000 0x0 0x4000000;mw.b 0x8008000 0x0 0x4000000;sf read > > > Zynq> 0x8008000 0x0 0x4000000;cmp.b 0x8000 0x8008000 0x4000000 > > > SF: 67108864 bytes @ 0x0 Erased: OK > > > device 0 offset 0x0, size 0x4000000 > > > SF: 67108864 bytes @ 0x0 Written: OK device 0 offset 0x0, size > > > 0x4000000 > > > SF: 67108864 bytes @ 0x0 Read: OK > > > Total of 67108864 byte(s) were the same > > Commit message reads: > > > > " > > 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 " > > > > You ran completely different test on completely different chip. > > > > The test is "sf probe && sf update 0x50000000 0 0x160000" , did you > > ever test "sf update" ? > > Tried the test " sf probe && sf update 0x4000000 0 0x160000" > > Zynq> sf probe && sf update 0x4000000 0 0x160000 > SF: Detected mx66l1g45g with page size 256 Bytes, erase size 64 KiB, total 128 > MiB device 0 offset 0x0, size 0x160000 > 0 bytes written, 1441792 bytes skipped in 0.154s, speed 9586980 B/s > Zynq> > > Got the board with spansion flash part, tried below commands Zynq> sf probe && sf update 0x4000000 0 0x160000 SF: Detected s25fl512s with page size 256 Bytes, erase size 256 KiB, total 64 MiB device 0 offset 0x0, size 0x160000 1441792 bytes written, 0 bytes skipped in 5.186s, speed 284688 B/s Zynq> Thanks Venkatesh > Thanks > Venkatesh
On 10/23/24 4:14 PM, Abbarapu, Venkatesh wrote: > Hi, > >> -----Original Message----- >> From: Marek Vasut <marek.vasut@mailbox.org> >> Sent: Wednesday, October 23, 2024 6:15 PM >> To: Abbarapu, Venkatesh <venkatesh.abbarapu@amd.com>; u-boot@lists.denx.de >> Cc: Andre Przywara <andre.przywara@arm.com>; Ashok Reddy Soma >> <ashok.reddy.soma@amd.com>; Jagan Teki <jagan@amarulasolutions.com>; >> Michael Walle <mwalle@kernel.org>; Simek, Michal <michal.simek@amd.com>; >> Patrice Chotard <patrice.chotard@foss.st.com>; Patrick Delaunay >> <patrick.delaunay@foss.st.com>; Pratyush Yadav <p.yadav@ti.com>; Quentin >> Schulz <quentin.schulz@cherry.de>; Sean Anderson <seanga2@gmail.com>; >> Simon Glass <sjg@chromium.org>; Takahiro Kuwano >> <Takahiro.Kuwano@infineon.com>; Tom Rini <trini@konsulko.com>; Tudor >> Ambarus <tudor.ambarus@linaro.org>; uboot-stm32@st-md- >> mailman.stormreply.com >> Subject: Re: [PATCH 1/6] Revert "spi: zynq_qspi: Add parallel memories support in >> QSPI driver" >> >> On 10/23/24 11:07 AM, Abbarapu, Venkatesh wrote: >>> Hi, >>> Tested with the non-stacked default single configuration on ZynqMP zcu102 board >> and didn’t see any issue. >>> >>> ZynqMP> sf probe 0 0 0 >>> SF: Detected mt25qu512a with page size 256 Bytes, erase size 64 KiB, >>> total 64 MiB >>> ZynqMP> sf erase 0x0 0x4000000;mw.b 0x8000 aabbccdd 0x4000000;sf write >>> ZynqMP> 0x8000 0x0 0x4000000;mw.b 0x8008000 0x0 0x4000000;sf read >>> ZynqMP> 0x8008000 0x0 0x4000000;cmp.b 0x8000 0x8008000 0x4000000 >>> SF: 67108864 bytes @ 0x0 Erased: OK >>> device 0 whole chip >>> SF: 67108864 bytes @ 0x0 Written: OK >>> device 0 whole chip >>> SF: 67108864 bytes @ 0x0 Read: OK >>> Total of 67108864 byte(s) were the same >>> >>> Thanks >>> Venkatesh >>> >>>> -----Original Message----- >>>> From: Marek Vasut <marek.vasut@mailbox.org> >>>> Sent: Wednesday, October 23, 2024 2:12 PM >>>> To: Abbarapu, Venkatesh <venkatesh.abbarapu@amd.com>; Marek Vasut >>>> <marek.vasut+renesas@mailbox.org>; u-boot@lists.denx.de >>>> Cc: Andre Przywara <andre.przywara@arm.com>; Ashok Reddy Soma >>>> <ashok.reddy.soma@amd.com>; Jagan Teki <jagan@amarulasolutions.com>; >>>> Michael Walle <mwalle@kernel.org>; Simek, Michal >>>> <michal.simek@amd.com>; Patrice Chotard >>>> <patrice.chotard@foss.st.com>; Patrick Delaunay >>>> <patrick.delaunay@foss.st.com>; Pratyush Yadav <p.yadav@ti.com>; >>>> Quentin Schulz <quentin.schulz@cherry.de>; Sean Anderson >>>> <seanga2@gmail.com>; Simon Glass <sjg@chromium.org>; Takahiro Kuwano >>>> <Takahiro.Kuwano@infineon.com>; Tom Rini <trini@konsulko.com>; Tudor >>>> Ambarus <tudor.ambarus@linaro.org>; uboot-stm32@st-md- >>>> mailman.stormreply.com >>>> Subject: Re: [PATCH 1/6] Revert "spi: zynq_qspi: Add parallel >>>> memories support in QSPI driver" >>>> >>>> On 10/23/24 5:18 AM, Abbarapu, Venkatesh wrote: >>>>> Hi Marek, >>>>> There was some issue and fix is sent >>>>> https://lore.kernel.org/u-boot/20241018082644.22495-1-venkatesh.abba >>>>> ra >>>>> pu@amd.com/T/#u >>>> >>>> Is this one fix or three fixes for three different issues ? >>>> >>>> This seems to fix READ errors, which is apparently another error >>>> introduced by this stuff. In my case, plain and simply 'sf probe ; sf >>>> update' combination with single non- stacked SPI NOR does not work. Was such >> a simple configuration ever tested ? >>>> >>>>> Not sure we need to revert whole parallel/stacked support? >>>> Please stop top-posting. >> >> You ran completely different test on completely different chip. >> >> Stop top posting. > > Sorry for top posting > > Will try to get the spansion flash part and try the below tests. > At this point tried testing on different board with different flash part. > > Zynq> sf probe 0 0 0 > SF: Detected mx66l1g45g with page size 256 Bytes, erase size 64 KiB, total 128 MiB > Zynq> sf update 0x4000000 0 0x160000 > device 0 offset 0x0, size 0x160000 > 1441792 bytes written, 0 bytes skipped in 5.735s, speed 257435 B/s I'm sorry, I missed this ^ part.
On 10/23/24 4:37 PM, Abbarapu, Venkatesh wrote: [...] >> Tried the test " sf probe && sf update 0x4000000 0 0x160000" >> >> Zynq> sf probe && sf update 0x4000000 0 0x160000 >> SF: Detected mx66l1g45g with page size 256 Bytes, erase size 64 KiB, total 128 >> MiB device 0 offset 0x0, size 0x160000 >> 0 bytes written, 1441792 bytes skipped in 0.154s, speed 9586980 B/s >> Zynq> >> >> > > Got the board with spansion flash part, tried below commands > > Zynq> sf probe && sf update 0x4000000 0 0x160000 > SF: Detected s25fl512s with page size 256 Bytes, erase size 256 KiB, total 64 MiB > device 0 offset 0x0, size 0x160000 > 1441792 bytes written, 0 bytes skipped in 5.186s, speed 284688 B/s > Zynq> Which board (or DT) is this and which U-Boot tree commit ? And I am sorry, I missed the sf update test in your previous replies.
Hi, > -----Original Message----- > From: Marek Vasut <marek.vasut@mailbox.org> > Sent: Wednesday, October 23, 2024 8:48 PM > To: Abbarapu, Venkatesh <venkatesh.abbarapu@amd.com>; u-boot@lists.denx.de > Cc: Andre Przywara <andre.przywara@arm.com>; Ashok Reddy Soma > <ashok.reddy.soma@amd.com>; Jagan Teki <jagan@amarulasolutions.com>; > Michael Walle <mwalle@kernel.org>; Simek, Michal <michal.simek@amd.com>; > Patrice Chotard <patrice.chotard@foss.st.com>; Patrick Delaunay > <patrick.delaunay@foss.st.com>; Pratyush Yadav <p.yadav@ti.com>; Quentin > Schulz <quentin.schulz@cherry.de>; Sean Anderson <seanga2@gmail.com>; > Simon Glass <sjg@chromium.org>; Takahiro Kuwano > <Takahiro.Kuwano@infineon.com>; Tom Rini <trini@konsulko.com>; Tudor > Ambarus <tudor.ambarus@linaro.org>; uboot-stm32@st-md- > mailman.stormreply.com > Subject: Re: [PATCH 1/6] Revert "spi: zynq_qspi: Add parallel memories support in > QSPI driver" > > On 10/23/24 4:37 PM, Abbarapu, Venkatesh wrote: > > [...] > > >> Tried the test " sf probe && sf update 0x4000000 0 0x160000" > >> > >> Zynq> sf probe && sf update 0x4000000 0 0x160000 > >> SF: Detected mx66l1g45g with page size 256 Bytes, erase size 64 KiB, > >> total 128 MiB device 0 offset 0x0, size 0x160000 > >> 0 bytes written, 1441792 bytes skipped in 0.154s, speed 9586980 B/s > >> Zynq> > >> > >> > > > > Got the board with spansion flash part, tried below commands > > > > Zynq> sf probe && sf update 0x4000000 0 0x160000 > > SF: Detected s25fl512s with page size 256 Bytes, erase size 256 KiB, > > total 64 MiB device 0 offset 0x0, size 0x160000 > > 1441792 bytes written, 0 bytes skipped in 5.186s, speed 284688 B/s > > Zynq> > > Which board (or DT) is this and which U-Boot tree commit ? > > And I am sorry, I missed the sf update test in your previous replies. The U-Boot tree commit is f1de0b97d1cbfd982b7a507962bb21b12a024b2f and applied this change on top of this https://lore.kernel.org/u-boot/20241018082644.22495-1-venkatesh.abbarapu@amd.com/T/#u The board used is Zynq and DT is zynq-zc706.dts tested with the non-stacked single configuration. Zynq> sf probe && sf update 0x4000000 0 0x160000 SF: Detected s25fl512s with page size 256 Bytes, erase size 256 KiB, total 64 MiB device 0 offset 0x0, size 0x160000 1441792 bytes written, 0 bytes skipped in 5.186s, speed 284688 B/s Thanks Venkatesh
Marek Vasut <marek.vasut@mailbox.org> writes: > On 10/23/24 10:17 AM, Michal Simek wrote: >> >> >> On 10/22/24 23:06, Marek Vasut wrote: >>> 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 >> >> Reverting everything seems to me too much. Tom has tested it on his HW >> and didn't see any issue. That's why better to look at code which is >> causing this. >> You are reverting everything but likely there is specific patch which is >> causing this. Which one is it? >> Which board was used for your testing? Likely we don't have access to it. >> Is there any QEMU available which can be used for debugging? > > The testcase including the exact SPI NOR model is above. > > iMX6 with w25q16dw seems to be broken too. > > Basically every board I have access no longer has a working "sf probe ; > sf update" combination ... so yeah, this means this patchset is > fundamentally broken. > I can also confirm that the patch series: f8efc68b30e Merge patch series "spi-nor: Add parallel and stacked memories support" breaks SPI NOR on TI platforms, particularly SK-AM62 and SK-AM62P: U-Boot 2024.10-00752-gf8efc68b30e2 (Nov 06 2024 - 12:25:13 -0600) SoC: AM62X SR1.0 HS-FS Model: Texas Instruments AM625 SK ... Hit any key to stop autoboot: 0 => sf probe && sf update ${loadaddr} 0x400000 0x10 SF: Detected s28hs512t with page size 256 Bytes, erase size 256 KiB, total 64 MiB device 0 offset 0x400000, size 0x10 SPI flash failed in read step => Jon >>> Since none of this seems to be in Linux either, revert it all. >> >> This has been discussed with Tom before. > > Tom is not the U-Boot MTD maintainer ? > >> It wasn't in sync even before >> and we can't really stop development on subsystems. > These patches have been rejected from mainline Linux for many years, why > are they added to U-Boot which includes an import of MTD subsystem from > Linux ? Such diverging code base with random patches will only make it > much harder to synchronize Linux MTD subsystem back to U-Boot.
On 11/6/24 8:18 PM, Jon Humphreys wrote: > Marek Vasut <marek.vasut@mailbox.org> writes: > >> On 10/23/24 10:17 AM, Michal Simek wrote: >>> >>> >>> On 10/22/24 23:06, Marek Vasut wrote: >>>> 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 >>> >>> Reverting everything seems to me too much. Tom has tested it on his HW >>> and didn't see any issue. That's why better to look at code which is >>> causing this. >>> You are reverting everything but likely there is specific patch which is >>> causing this. Which one is it? >>> Which board was used for your testing? Likely we don't have access to it. >>> Is there any QEMU available which can be used for debugging? >> >> The testcase including the exact SPI NOR model is above. >> >> iMX6 with w25q16dw seems to be broken too. >> >> Basically every board I have access no longer has a working "sf probe ; >> sf update" combination ... so yeah, this means this patchset is >> fundamentally broken. >> > > I can also confirm that the patch series: > > f8efc68b30e Merge patch series "spi-nor: Add parallel and stacked memories > support" > > breaks SPI NOR on TI platforms, particularly SK-AM62 and SK-AM62P: > > U-Boot 2024.10-00752-gf8efc68b30e2 (Nov 06 2024 - 12:25:13 -0600) > > SoC: AM62X SR1.0 HS-FS > Model: Texas Instruments AM625 SK > ... > Hit any key to stop autoboot: 0 > => sf probe && sf update ${loadaddr} 0x400000 0x10 > SF: Detected s28hs512t with page size 256 Bytes, erase size 256 KiB, total 64 MiB > device 0 offset 0x400000, size 0x10 > SPI flash failed in read step > => Sigh ... can you please test current u-boot/master and see if the error is fixed there ? We really should've gone with a full revert I think ...
Marek Vasut <marek.vasut@mailbox.org> writes: > On 11/6/24 8:18 PM, Jon Humphreys wrote: >> Marek Vasut <marek.vasut@mailbox.org> writes: >> >>> On 10/23/24 10:17 AM, Michal Simek wrote: >>>> >>>> >>>> On 10/22/24 23:06, Marek Vasut wrote: >>>>> 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 >>>> >>>> Reverting everything seems to me too much. Tom has tested it on his HW >>>> and didn't see any issue. That's why better to look at code which is >>>> causing this. >>>> You are reverting everything but likely there is specific patch which is >>>> causing this. Which one is it? >>>> Which board was used for your testing? Likely we don't have access to it. >>>> Is there any QEMU available which can be used for debugging? >>> >>> The testcase including the exact SPI NOR model is above. >>> >>> iMX6 with w25q16dw seems to be broken too. >>> >>> Basically every board I have access no longer has a working "sf probe ; >>> sf update" combination ... so yeah, this means this patchset is >>> fundamentally broken. >>> >> >> I can also confirm that the patch series: >> >> f8efc68b30e Merge patch series "spi-nor: Add parallel and stacked memories >> support" >> >> breaks SPI NOR on TI platforms, particularly SK-AM62 and SK-AM62P: >> >> U-Boot 2024.10-00752-gf8efc68b30e2 (Nov 06 2024 - 12:25:13 -0600) >> >> SoC: AM62X SR1.0 HS-FS >> Model: Texas Instruments AM625 SK >> ... >> Hit any key to stop autoboot: 0 >> => sf probe && sf update ${loadaddr} 0x400000 0x10 >> SF: Detected s28hs512t with page size 256 Bytes, erase size 256 KiB, total 64 MiB >> device 0 offset 0x400000, size 0x10 >> SPI flash failed in read step >> => > Sigh ... can you please test current u-boot/master and see if the error > is fixed there ? > Yes I had verified it also fails against master, although the behavior was a bit different. The .'s below are our DMA engine waiting indefinitely. => sf probe && sf update ${loadaddr} 0x400000 0x10 SF: Detected s28hs512t with page size 256 Bytes, erase size 256 KiB, total 64 MiB device 0 offset 0x400000, size 0x10 ..................................................... I have not investigated further. Jon > We really should've gone with a full revert I think ...
On 11/6/24 10:58 PM, Jon Humphreys wrote: > Marek Vasut <marek.vasut@mailbox.org> writes: > >> On 11/6/24 8:18 PM, Jon Humphreys wrote: >>> Marek Vasut <marek.vasut@mailbox.org> writes: >>> >>>> On 10/23/24 10:17 AM, Michal Simek wrote: >>>>> >>>>> >>>>> On 10/22/24 23:06, Marek Vasut wrote: >>>>>> 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 >>>>> >>>>> Reverting everything seems to me too much. Tom has tested it on his HW >>>>> and didn't see any issue. That's why better to look at code which is >>>>> causing this. >>>>> You are reverting everything but likely there is specific patch which is >>>>> causing this. Which one is it? >>>>> Which board was used for your testing? Likely we don't have access to it. >>>>> Is there any QEMU available which can be used for debugging? >>>> >>>> The testcase including the exact SPI NOR model is above. >>>> >>>> iMX6 with w25q16dw seems to be broken too. >>>> >>>> Basically every board I have access no longer has a working "sf probe ; >>>> sf update" combination ... so yeah, this means this patchset is >>>> fundamentally broken. >>>> >>> >>> I can also confirm that the patch series: >>> >>> f8efc68b30e Merge patch series "spi-nor: Add parallel and stacked memories >>> support" >>> >>> breaks SPI NOR on TI platforms, particularly SK-AM62 and SK-AM62P: >>> >>> U-Boot 2024.10-00752-gf8efc68b30e2 (Nov 06 2024 - 12:25:13 -0600) >>> >>> SoC: AM62X SR1.0 HS-FS >>> Model: Texas Instruments AM625 SK >>> ... >>> Hit any key to stop autoboot: 0 >>> => sf probe && sf update ${loadaddr} 0x400000 0x10 >>> SF: Detected s28hs512t with page size 256 Bytes, erase size 256 KiB, total 64 MiB >>> device 0 offset 0x400000, size 0x10 >>> SPI flash failed in read step >>> => >> Sigh ... can you please test current u-boot/master and see if the error >> is fixed there ? >> > > Yes I had verified it also fails against master, although the behavior was > a bit different. The .'s below are our DMA engine waiting indefinitely. > > => sf probe && sf update ${loadaddr} 0x400000 0x10 > SF: Detected s28hs512t with page size 256 Bytes, erase size 256 KiB, total 64 MiB > device 0 offset 0x400000, size 0x10 > ..................................................... > > I have not investigated further. Can you try and run some 'git bisect' to find out exactly which commit broke your use case ? There is a bunch of fixes for the worst breakage that landed recently, but clearly there is more. Full revert seems increasingly appealing ...
Marek Vasut <marek.vasut@mailbox.org> writes: > On 11/6/24 10:58 PM, Jon Humphreys wrote: >> Marek Vasut <marek.vasut@mailbox.org> writes: >> >>> On 11/6/24 8:18 PM, Jon Humphreys wrote: >>>> Marek Vasut <marek.vasut@mailbox.org> writes: >>>> >>>>> On 10/23/24 10:17 AM, Michal Simek wrote: >>>>>> >>>>>> >>>>>> On 10/22/24 23:06, Marek Vasut wrote: >>>>>>> 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 >>>>>> >>>>>> Reverting everything seems to me too much. Tom has tested it on his HW >>>>>> and didn't see any issue. That's why better to look at code which is >>>>>> causing this. >>>>>> You are reverting everything but likely there is specific patch which is >>>>>> causing this. Which one is it? >>>>>> Which board was used for your testing? Likely we don't have access to it. >>>>>> Is there any QEMU available which can be used for debugging? >>>>> >>>>> The testcase including the exact SPI NOR model is above. >>>>> >>>>> iMX6 with w25q16dw seems to be broken too. >>>>> >>>>> Basically every board I have access no longer has a working "sf probe ; >>>>> sf update" combination ... so yeah, this means this patchset is >>>>> fundamentally broken. >>>>> >>>> >>>> I can also confirm that the patch series: >>>> >>>> f8efc68b30e Merge patch series "spi-nor: Add parallel and stacked memories >>>> support" >>>> >>>> breaks SPI NOR on TI platforms, particularly SK-AM62 and SK-AM62P: >>>> >>>> U-Boot 2024.10-00752-gf8efc68b30e2 (Nov 06 2024 - 12:25:13 -0600) >>>> >>>> SoC: AM62X SR1.0 HS-FS >>>> Model: Texas Instruments AM625 SK >>>> ... >>>> Hit any key to stop autoboot: 0 >>>> => sf probe && sf update ${loadaddr} 0x400000 0x10 >>>> SF: Detected s28hs512t with page size 256 Bytes, erase size 256 KiB, total 64 MiB >>>> device 0 offset 0x400000, size 0x10 >>>> SPI flash failed in read step >>>> => >>> Sigh ... can you please test current u-boot/master and see if the error >>> is fixed there ? >>> >> >> Yes I had verified it also fails against master, although the behavior was >> a bit different. The .'s below are our DMA engine waiting indefinitely. >> >> => sf probe && sf update ${loadaddr} 0x400000 0x10 >> SF: Detected s28hs512t with page size 256 Bytes, erase size 256 KiB, total 64 MiB >> device 0 offset 0x400000, size 0x10 >> ..................................................... >> >> I have not investigated further. > > Can you try and run some 'git bisect' to find out exactly which commit > broke your use case ? There is a bunch of fixes for the worst breakage > that landed recently, but clearly there is more. > > Full revert seems increasingly appealing ... commit 5d40b3d384d Jon
On 11/7/24 4:49 PM, Jon Humphreys wrote: > Marek Vasut <marek.vasut@mailbox.org> writes: > >> On 11/6/24 10:58 PM, Jon Humphreys wrote: >>> Marek Vasut <marek.vasut@mailbox.org> writes: >>> >>>> On 11/6/24 8:18 PM, Jon Humphreys wrote: >>>>> Marek Vasut <marek.vasut@mailbox.org> writes: >>>>> >>>>>> On 10/23/24 10:17 AM, Michal Simek wrote: >>>>>>> >>>>>>> >>>>>>> On 10/22/24 23:06, Marek Vasut wrote: >>>>>>>> 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 >>>>>>> >>>>>>> Reverting everything seems to me too much. Tom has tested it on his HW >>>>>>> and didn't see any issue. That's why better to look at code which is >>>>>>> causing this. >>>>>>> You are reverting everything but likely there is specific patch which is >>>>>>> causing this. Which one is it? >>>>>>> Which board was used for your testing? Likely we don't have access to it. >>>>>>> Is there any QEMU available which can be used for debugging? >>>>>> >>>>>> The testcase including the exact SPI NOR model is above. >>>>>> >>>>>> iMX6 with w25q16dw seems to be broken too. >>>>>> >>>>>> Basically every board I have access no longer has a working "sf probe ; >>>>>> sf update" combination ... so yeah, this means this patchset is >>>>>> fundamentally broken. >>>>>> >>>>> >>>>> I can also confirm that the patch series: >>>>> >>>>> f8efc68b30e Merge patch series "spi-nor: Add parallel and stacked memories >>>>> support" >>>>> >>>>> breaks SPI NOR on TI platforms, particularly SK-AM62 and SK-AM62P: >>>>> >>>>> U-Boot 2024.10-00752-gf8efc68b30e2 (Nov 06 2024 - 12:25:13 -0600) >>>>> >>>>> SoC: AM62X SR1.0 HS-FS >>>>> Model: Texas Instruments AM625 SK >>>>> ... >>>>> Hit any key to stop autoboot: 0 >>>>> => sf probe && sf update ${loadaddr} 0x400000 0x10 >>>>> SF: Detected s28hs512t with page size 256 Bytes, erase size 256 KiB, total 64 MiB >>>>> device 0 offset 0x400000, size 0x10 >>>>> SPI flash failed in read step >>>>> => >>>> Sigh ... can you please test current u-boot/master and see if the error >>>> is fixed there ? >>>> >>> >>> Yes I had verified it also fails against master, although the behavior was >>> a bit different. The .'s below are our DMA engine waiting indefinitely. >>> >>> => sf probe && sf update ${loadaddr} 0x400000 0x10 >>> SF: Detected s28hs512t with page size 256 Bytes, erase size 256 KiB, total 64 MiB >>> device 0 offset 0x400000, size 0x10 >>> ..................................................... >>> >>> I have not investigated further. >> >> Can you try and run some 'git bisect' to find out exactly which commit >> broke your use case ? There is a bunch of fixes for the worst breakage >> that landed recently, but clearly there is more. >> >> Full revert seems increasingly appealing ... > > commit 5d40b3d384d So there is still something broken in that specific commit that I missed when removing the defects ? Sigh ... can you try to narrow it down ?
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 <jteki@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;
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+renesas@mailbox.org> --- Cc: Andre Przywara <andre.przywara@arm.com> Cc: Ashok Reddy Soma <ashok.reddy.soma@amd.com> Cc: Jagan Teki <jagan@amarulasolutions.com> Cc: Michael Walle <mwalle@kernel.org> Cc: Michal Simek <michal.simek@amd.com> Cc: Patrice Chotard <patrice.chotard@foss.st.com> Cc: Patrick Delaunay <patrick.delaunay@foss.st.com> Cc: Pratyush Yadav <p.yadav@ti.com> Cc: Quentin Schulz <quentin.schulz@cherry.de> Cc: Sean Anderson <seanga2@gmail.com> Cc: Simon Glass <sjg@chromium.org> Cc: Takahiro Kuwano <Takahiro.Kuwano@infineon.com> Cc: Tom Rini <trini@konsulko.com> Cc: Tudor Ambarus <tudor.ambarus@linaro.org> Cc: Venkatesh Yadav Abbarapu <venkatesh.abbarapu@amd.com> Cc: u-boot@lists.denx.de Cc: uboot-stm32@st-md-mailman.stormreply.com --- drivers/spi/zynq_qspi.c | 115 ++++------------------------------------ 1 file changed, 11 insertions(+), 104 deletions(-)