[PATCH v2] spi: cadence-qspi: Remove cdns,is-dma property handling
Michal Simek
michal.simek at amd.com
Mon Oct 20 07:51:32 CEST 2025
Hi,
On 10/20/25 00:36, Ronald Wahl wrote:
> On 20.10.25 00:03, E Shattow wrote:
>> On 10/9/25 03:34, Michal Simek wrote:
>>> Remove cdns,is-dma DT property handling. Property is not the part of DT
>>> binding and it is also hardcoded to value 1 in all DTs that's why remove it
>>> because none is also testing value 0.
>>> If there is any use case when this configuration should be supported this
>>> patch can be reverted.
>>>
>>> Signed-off-by: Michal Simek <michal.simek at amd.com>
>>> ---
>>>
>>> Changes in v2:
>>> - Remove other functions which are reported as unused by gitlab CI.
>>>
>>> v1: https://lore.kernel.org/r/
>>> c6a5b4bfe565668eb3299d5c5a5548a2163b129a.1758786568.git.michal.simek at amd.com>>> ---
>>> arch/arm/dts/versal-mini-ospi.dtsi | 1 -
>>> arch/arm/dts/versal-net-mini-ospi.dtsi | 1 -
>>> drivers/spi/cadence_qspi.c | 8 +-
>>> drivers/spi/cadence_qspi.h | 5 --
>>> drivers/spi/cadence_qspi_apb.c | 119 -------------------------
>>> 5 files changed, 1 insertion(+), 133 deletions(-)
>>>
>>> diff --git a/arch/arm/dts/versal-mini-ospi.dtsi b/arch/arm/dts/versal-mini-
>>> ospi.dtsi
>>> index eec2a08e7c70..6991f6a51db0 100644
>>> --- a/arch/arm/dts/versal-mini-ospi.dtsi
>>> +++ b/arch/arm/dts/versal-mini-ospi.dtsi
>>> @@ -38,7 +38,6 @@
>>> num-cs = <1>;
>>> cdns,fifo-depth = <256>;
>>> cdns,fifo-width = <4>;
>>> - cdns,is-dma = <1>;
>>> cdns,trigger-address = <0xc0000000>;
>>> #address-cells = <1>;
>>> #size-cells = <0>;
>>> diff --git a/arch/arm/dts/versal-net-mini-ospi.dtsi b/arch/arm/dts/versal-
>>> net-mini-ospi.dtsi
>>> index 1c94b352dc97..d2d5ec8e5cbf 100644
>>> --- a/arch/arm/dts/versal-net-mini-ospi.dtsi
>>> +++ b/arch/arm/dts/versal-net-mini-ospi.dtsi
>>> @@ -52,7 +52,6 @@
>>> num-cs = <1>;
>>> cdns,fifo-depth = <256>;
>>> cdns,fifo-width = <4>;
>>> - cdns,is-dma = <1>;
>>> cdns,is-stig-pgm = <1>;
>>> cdns,trigger-address = <0xc0000000>;
>>> #address-cells = <1>;
>>> diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c
>>> index 599596f9f087..849bd930edf0 100644
>>> --- a/drivers/spi/cadence_qspi.c
>>> +++ b/drivers/spi/cadence_qspi.c
>>> @@ -210,7 +210,6 @@ static int cadence_spi_probe(struct udevice *bus)
>>>
>>> priv->regbase = plat->regbase;
>>> priv->ahbbase = plat->ahbbase;
>>> - priv->is_dma = plat->is_dma;
>>> priv->is_decoded_cs = plat->is_decoded_cs;
>>> priv->fifo_depth = plat->fifo_depth;
>>> priv->fifo_width = plat->fifo_width;
>>> @@ -348,10 +347,7 @@ static int cadence_spi_mem_exec_op(struct spi_slave *spi,
>>> case CQSPI_READ:
>>> err = cadence_qspi_apb_read_setup(priv, op);
>>> if (!err) {
>>> - if (priv->is_dma)
>>> - err = cadence_qspi_apb_dma_read(priv, op);
>>> - else
>>> - err = cadence_qspi_apb_read_execute(priv, op);
>>> + err = cadence_qspi_apb_dma_read(priv, op);
>>> }
>>> break;
>>> case CQSPI_WRITE:
>>> @@ -412,8 +408,6 @@ static int cadence_spi_of_to_plat(struct udevice *bus)
>>> if (plat->ahbsize >= SZ_8M)
>>> priv->use_dac_mode = true;
>>>
>>> - plat->is_dma = dev_read_bool(bus, "cdns,is-dma");
>>> -
>>> /* All other parameters are embedded in the child node */
>>> subnode = cadence_qspi_get_subnode(bus);
>>> if (!ofnode_valid(subnode)) {
>>> diff --git a/drivers/spi/cadence_qspi.h b/drivers/spi/cadence_qspi.h
>>> index 879e7f8dbfb8..10c4ad14cc05 100644
>>> --- a/drivers/spi/cadence_qspi.h
>>> +++ b/drivers/spi/cadence_qspi.h
>>> @@ -223,8 +223,6 @@ struct cadence_spi_plat {
>>> u32 tchsh_ns;
>>> u32 tslch_ns;
>>> u32 quirks;
>>> -
>>> - bool is_dma;
>>> };
>>>
>>> struct cadence_spi_priv {
>>> @@ -261,7 +259,6 @@ struct cadence_spi_priv {
>>> bool ddr_init;
>>> bool is_decoded_cs;
>>> bool use_dac_mode;
>>> - bool is_dma;
>>>
>>> /* Transaction protocol parameters. */
>>> u8 inst_width;
>>> @@ -292,8 +289,6 @@ int cadence_qspi_apb_command_write(struct
>>> cadence_spi_priv *priv,
>>>
>>> int cadence_qspi_apb_read_setup(struct cadence_spi_priv *priv,
>>> const struct spi_mem_op *op);
>>> -int cadence_qspi_apb_read_execute(struct cadence_spi_priv *priv,
>>> - const struct spi_mem_op *op);
>>> int cadence_qspi_apb_write_setup(struct cadence_spi_priv *priv,
>>> const struct spi_mem_op *op);
>>> int cadence_qspi_apb_write_execute(struct cadence_spi_priv *priv,
>>> diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
>>> index 4696c09f754b..58be017720bc 100644
>>> --- a/drivers/spi/cadence_qspi_apb.c
>>> +++ b/drivers/spi/cadence_qspi_apb.c
>>> @@ -667,125 +667,6 @@ int cadence_qspi_apb_read_setup(struct cadence_spi_priv
>>> *priv,
>>> return 0;
>>> }
>>>
>>> -static u32 cadence_qspi_get_rd_sram_level(struct cadence_spi_priv *priv)
>>> -{
>>> - u32 reg = readl(priv->regbase + CQSPI_REG_SDRAMLEVEL);
>>> - reg >>= CQSPI_REG_SDRAMLEVEL_RD_LSB;
>>> - return reg & CQSPI_REG_SDRAMLEVEL_RD_MASK;
>>> -}
>>> -
>>> -static int cadence_qspi_wait_for_data(struct cadence_spi_priv *priv)
>>> -{
>>> - unsigned int timeout = 10000;
>>> - u32 reg;
>>> -
>>> - while (timeout--) {
>>> - reg = cadence_qspi_get_rd_sram_level(priv);
>>> - if (reg)
>>> - return reg;
>>> - udelay(1);
>>> - }
>>> -
>>> - return -ETIMEDOUT;
>>> -}
>>> -
>>> -static int
>>> -cadence_qspi_apb_indirect_read_execute(struct cadence_spi_priv *priv,
>>> - unsigned int n_rx, u8 *rxbuf)
>>> -{
>>> - unsigned int remaining = n_rx;
>>> - unsigned int bytes_to_read = 0;
>>> - int ret;
>>> -
>>> - writel(n_rx, priv->regbase + CQSPI_REG_INDIRECTRDBYTES);
>>> -
>>> - /* Start the indirect read transfer */
>>> - writel(CQSPI_REG_INDIRECTRD_START,
>>> - priv->regbase + CQSPI_REG_INDIRECTRD);
>>> -
>>> - while (remaining > 0) {
>>> - ret = cadence_qspi_wait_for_data(priv);
>>> - if (ret < 0) {
>>> - printf("Indirect write timed out (%i)\n", ret);
>>> - goto failrd;
>>> - }
>>> -
>>> - bytes_to_read = ret;
>>> -
>>> - while (bytes_to_read != 0) {
>>> - bytes_to_read *= priv->fifo_width;
>>> - bytes_to_read = bytes_to_read > remaining ?
>>> - remaining : bytes_to_read;
>>> - /*
>>> - * Handle non-4-byte aligned access to avoid
>>> - * data abort.
>>> - */
>>> - if (((uintptr_t)rxbuf % 4) || (bytes_to_read % 4))
>>> - readsb(priv->ahbbase, rxbuf, bytes_to_read);
>>> - else
>>> - readsl(priv->ahbbase, rxbuf,
>>> - bytes_to_read >> 2);
>>> - rxbuf += bytes_to_read;
>>> - remaining -= bytes_to_read;
>>> - bytes_to_read = cadence_qspi_get_rd_sram_level(priv);
>>> - }
>>> - }
>>> -
>>> - /* Check indirect done status */
>>> - ret = wait_for_bit_le32(priv->regbase + CQSPI_REG_INDIRECTRD,
>>> - CQSPI_REG_INDIRECTRD_DONE, 1, 10, 0);
>>> - if (ret) {
>>> - printf("Indirect read completion error (%i)\n", ret);
>>> - goto failrd;
>>> - }
>>> -
>>> - /* Clear indirect completion status */
>>> - writel(CQSPI_REG_INDIRECTRD_DONE,
>>> - priv->regbase + CQSPI_REG_INDIRECTRD);
>>> -
>>> - /* Check indirect done status */
>>> - ret = wait_for_bit_le32(priv->regbase + CQSPI_REG_INDIRECTRD,
>>> - CQSPI_REG_INDIRECTRD_DONE, 0, 10, 0);
>>> - if (ret) {
>>> - printf("Indirect read clear completion error (%i)\n", ret);
>>> - goto failrd;
>>> - }
>>> -
>>> - /* Wait til QSPI is idle */
>>> - if (!cadence_qspi_wait_idle(priv->regbase))
>>> - return -EIO;
>>> -
>>> - return 0;
>>> -
>>> -failrd:
>>> - /* Cancel the indirect read */
>>> - writel(CQSPI_REG_INDIRECTRD_CANCEL,
>>> - priv->regbase + CQSPI_REG_INDIRECTRD);
>>> - return ret;
>>> -}
>>> -
>>> -int cadence_qspi_apb_read_execute(struct cadence_spi_priv *priv,
>>> - const struct spi_mem_op *op)
>>> -{
>>> - u64 from = op->addr.val;
>>> - void *buf = op->data.buf.in;
>>> - size_t len = op->data.nbytes;
>>> -
>>> - cadence_qspi_apb_enable_linear_mode(true);
>>> -
>>> - if (priv->use_dac_mode && (from + len < priv->ahbsize)) {
>>> - if (len < 256 ||
>>> - dma_memcpy(buf, priv->ahbbase + from, len) < 0) {
>>> - memcpy_fromio(buf, priv->ahbbase + from, len);
>>> - }
>>> - if (!cadence_qspi_wait_idle(priv->regbase))
>>> - return -EIO;
>>> - return 0;
>>> - }
>>> -
>>> - return cadence_qspi_apb_indirect_read_execute(priv, len, buf);
>>> -}
>>> -
>>> /* Opcode + Address (3/4 bytes) */
>>> int cadence_qspi_apb_write_setup(struct cadence_spi_priv *priv,
>>> const struct spi_mem_op *op)
>>
>> Your patch results in corrupted SPI NOR flash writing from U-Boot on
>> starfive_visionfive2_defconfig target board Pine64 Star64:
>>
>> a040578d8270ed8788d7663808ea63ce5ffd7840 is the first bad commit
>> commit a040578d8270ed8788d7663808ea63ce5ffd7840
>> Author: Michal Simek <michal.simek at amd.com>
>> Date: Thu Oct 9 12:34:48 2025 +0200
>>
>> spi: cadence-qspi: Remove cdns,is-dma property handling
>>
>> Steps to reproduce with Star64:
>>
>> 1). Set JH-7110 SoC RGPIO configured for UART BootROM loader mode
>> 2). Power on and Transfer U-Boot SPL by XMODEM as prompted by BootROM
>> 3). Transfer U-Boot main by YMODEM from U-Boot SPL prompt
>> 4). Having `(cd $HOME/build/u-boot && python3 -m http.server 6789)` or
>> similar for serving U-Boot SPL and U-Boot Main to save some time with
>> UART transfer:
>> 4a). wget $loadaddr http://host:6789/spl/u-boot-spl.bin.normal.out && sf
>> update $loadaddr 0 $filesize
>> 4b). wget $loadaddr http://host:6789/u-boot.itb && sf update $loadaddr
>> 100000 $filesize
>>
>> Repeat steps 4a and 4b, note that each time all bytes are written and no
>> bytes are skipped. Written data is wrong and device unsuccessfully boots
>> in SPI NOR flash BootROM loader configuration showing wrong information
>> to output.
>>
>> Please follow up with your analysis of why these changes have this effect.
>
> I guess this is because the committer didn't take into account that when
> a DT did not mention the property then it is 0 (false).
>
> I assume TI K3 hardware is affected as well. So I think the patch must
> be reverted.
Are you using cadence-qspi in non DMA mode? DT property shouldn't be introduced
in the first place that's why it was removed.
Thanks,
Michal
More information about the U-Boot
mailing list