Re: [PATCH v9 07/18] spi: cadence-quadspi: add DQS support to read data capture
From: sashiko-bot@kernel.org
Date: 2026-08-25 17:29:18
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] The patch introduces a `dqs` parameter to `cqspi_readdata_capture()` but hardcodes it to `false` in `cqspi_configure()`, unconditionally clearing the hardware DQS bit and breaking platforms that rely on the bootloader to enable it. -- commit e47fa544a8e52f8469fd8ae3d4eeedbed9dc8a9e Author: Santhosh Kumar K [off-list ref] spi: cadence-quadspi: add DQS support to read data capture This commit adds a DQS (Data Strobe) parameter to cqspi_readdata_capture() to control data capture timing. DQS mode uses a dedicated strobe signal for improved timing margins in high-speed SPI modes.
quoted hunk ↗ jump to hunk
diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c index f8e2663723677..046fa5d69b0f7 100644 --- a/drivers/spi/spi-cadence-quadspi.c +++ b/drivers/spi/spi-cadence-quadspi.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -476,6 +477,11 @@ static void cqspi_readdata_capture(struct cqspi_st *cqspi, const bool bypass, reg |= (delay & CQSPI_REG_READCAPTURE_DELAY_MASK) << CQSPI_REG_READCAPTURE_DELAY_LSB; + if (dqs) + reg |= BIT(CQSPI_REG_READCAPTURE_DQS_LSB); + else + reg &= ~BIT(CQSPI_REG_READCAPTURE_DQS_LSB); +
[Severity: High] Does explicitly clearing the DQS bit here destroy hardware state established by the bootloader? Previously, this function used a read-modify-write operation that preserved the existing DQS bit. Platforms that don't utilize the new kernel PHY tuning infrastructure, but rely on the bootloader to enable DQS, might now experience read failures and data corruption in high-speed SPI modes when this state is lost.
quoted hunk ↗ jump to hunk
writel(reg, reg_base + CQSPI_REG_READCAPTURE); }@@ -1317,7 +1323,7 @@ static void cqspi_configure(struct cqspi_flash_pdata *f_pdata, cqspi->sclk = sclk; cqspi_config_baudrate_div(cqspi); cqspi_delay(f_pdata); - cqspi_readdata_capture(cqspi, !cqspi->rclk_en, + cqspi_readdata_capture(cqspi, !cqspi->rclk_en, false, f_pdata->read_delay); }
[Severity: High] Does hardcoding false here unconditionally disable DQS on every configuration cycle? Because cqspi_configure() is executed unconditionally on any chip-select or clock frequency switch, this passes false to cqspi_readdata_capture(). This will unconditionally clear the hardware DQS bit across all Cadence QSPI controllers, leading to regressions for boards that require it to remain enabled. -- Sashiko AI review · https://sashiko.dev/#/patchset/20260825171737.776052-1-s-k6@ti.com?part=7