[PATCH v2 0/3] Few fix/improvement for spi-nxp-fspi

[PATCH v2 0/3] Few fix/improvement for spi-nxp-fspi

[haibo.chen]

Patch 1 introduce per-SoC clock rate limits for both SDR and DTR modes
        by adding max_sdr_rate and max_dtr_rate to nxp_fspi_devtype_data.
Patch 2 enter stop mode before reconfiguring MCR0 and DLL to follow
	FlexSPI reference manual initialization sequence
Patch 3 propagate clock reconfig failures in nxp_fspi_select_mem()

Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
---
Changes in v2:
- Patch1: rework the commit log by Frank's comment.
- Add Patch2 and Patch 3 accroding to AI Sashiko's comment.
- Link to v1: https://lore.kernel.org/r/20260727-fspi-clock-v1-1-f4f3be6c366d@nxp.com

---
Haibo Chen (3):
      spi: spi-nxp-fspi: add per-SoC SDR/DTR clock rate limits for all supported SoCs
      spi: spi-nxp-fspi: enter stop mode before reconfiguring MCR0 and DLL
      spi: spi-nxp-fspi: propagate clock reconfig failures in nxp_fspi_select_mem()

 drivers/spi/spi-nxp-fspi.c | 129 +++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 119 insertions(+), 10 deletions(-)
---
base-commit: c5e32e86ca02b003f86e095d379b38148999293d
change-id: 20260727-fspi-clock-adcae0c35283

Best regards,
-- 
Haibo Chen <haibo.chen@nxp.com>

[PATCH v2 1/3] spi: spi-nxp-fspi: add per-SoC SDR/DTR clock rate limits for all supported SoCs

[haibo.chen]

From: Haibo Chen <haibo.chen@nxp.com>

The commit f43579ef3500 ("spi: spi-nxp-fspi: limit the clock rate for
different sample clock source selection") introduced a global 166MHz
cap for DTR mode (RXCLKSRC=3), based on the i.MX8MN datasheet timing
specification (Section 3.9.9, page 65).

After reviewing the FlexSPI timing parameters in the datasheets for all
supported SoCs, the following corrections and additions are needed:

1. SDR mode (RXCLKSRC=0) limits vary per SoC:
   - i.MX8MN/MM/MP/95: 66MHz  (IMX8MNCEC §3.9.9, IMX8MMCEC §3.9.10,
                                IMX8MPCEC, IMX95CEC Rev.8 §4.11.7)
   - i.MX8QXP/QM/DXL/ULP: 60MHz (IMX8QXPCEC, IMX8QMCEC, IMX8DXLCEC,
                                   IMX8ULPCEC §7.3.1 ND mode)
   - LX2160A: 100MHz            (LX2160ACEC FlexSPI timing parameters)

2. DTR mode (RXCLKSRC=3) limits vary per SoC:
   - i.MX8MN/MM/MP/ULP: 166MHz
   - i.MX8QXP/QM/DXL: 200MHz   (same FlexSPI IP across this family)
   - i.MX95: 200MHz             (IMX95CEC §4.11.7.3.2.3 Table 106)
   - LX2160A: DTR disabled      (FSPI_QUIRK_DISABLE_DTR)

Update related platform data with correct speed limation according
to datasheet.

Fixes: f43579ef3500 ("spi: spi-nxp-fspi: limit the clock rate for different sample clock source selection")
Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
---
 drivers/spi/spi-nxp-fspi.c | 83 ++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 80 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/spi-nxp-fspi.c b/drivers/spi/spi-nxp-fspi.c
index d94a2a7b98d44db217ba96a8ce7df23e37a08ba4..6de6d432cdcb2ee383a02d72f20000045903e176 100644
--- a/drivers/spi/spi-nxp-fspi.c
+++ b/drivers/spi/spi-nxp-fspi.c
@@ -340,6 +340,18 @@ struct nxp_fspi_devtype_data {
 	unsigned int quirks;
 	unsigned int lut_num;
 	bool little_endian;
+	/*
+	 * The max clock rate (Hz) that FlexSPI can output to the device
+	 * in SDR mode (RXCLKSRC=0). Defaults to 66MHz if zero.
+	 * Some SoCs (e.g. LX2160A) support up to 100MHz in SDR mode.
+	 */
+	unsigned long max_sdr_rate;
+	/*
+	 * The max clock rate (Hz) that FlexSPI can output to the device
+	 * in DTR mode (RXCLKSRC=3). Defaults to 166MHz if zero.
+	 * Some SoCs (e.g. i.MX95, i.MX8QM, i.MX8DXL) support up to 200MHz.
+	 */
+	unsigned long max_dtr_rate;
 };
 
 static struct nxp_fspi_devtype_data lx2160a_data = {
@@ -349,6 +361,10 @@ static struct nxp_fspi_devtype_data lx2160a_data = {
 	.quirks = FSPI_QUIRK_DISABLE_DTR,
 	.lut_num = 32,
 	.little_endian = true,  /* little-endian    */
+	/*
+	 * LX2160ACEC: SDR RXCLKSRC=0 max 100MHz, DTR disabled via quirk.
+	 */
+	.max_sdr_rate = 100000000,
 };
 
 static struct nxp_fspi_devtype_data imx8mm_data = {
@@ -358,6 +374,21 @@ static struct nxp_fspi_devtype_data imx8mm_data = {
 	.quirks = 0,
 	.lut_num = 32,
 	.little_endian = true,  /* little-endian    */
+	/* IMX8MMCEC §3.9.10: SDR RXCLKSRC=0 max 66MHz, DDR RXCLKSRC=3 max 166MHz */
+	.max_sdr_rate = 66000000,
+	.max_dtr_rate = 166000000,
+};
+
+static struct nxp_fspi_devtype_data imx8mp_data = {
+	.rxfifo = SZ_512,       /* (64  * 64 bits)  */
+	.txfifo = SZ_1K,        /* (128 * 64 bits)  */
+	.ahb_buf_size = SZ_2K,  /* (256 * 64 bits)  */
+	.quirks = 0,
+	.lut_num = 32,
+	.little_endian = true,  /* little-endian    */
+	/* IMX8MPCEC: SDR RXCLKSRC=0 max 66MHz, DDR RXCLKSRC=3 max 166MHz */
+	.max_sdr_rate = 66000000,
+	.max_dtr_rate = 166000000,
 };
 
 static struct nxp_fspi_devtype_data imx8qxp_data = {
@@ -367,6 +398,12 @@ static struct nxp_fspi_devtype_data imx8qxp_data = {
 	.quirks = 0,
 	.lut_num = 32,
 	.little_endian = true,  /* little-endian    */
+	/*
+	 * IMX8QXPCEC: SDR RXCLKSRC=0 max 60MHz, DDR RXCLKSRC=3 max 200MHz.
+	 * i.MX8QM and i.MX8DXL share the same FlexSPI IP and limits.
+	 */
+	.max_sdr_rate = 60000000,
+	.max_dtr_rate = 200000000,
 };
 
 static struct nxp_fspi_devtype_data imx8dxl_data = {
@@ -376,6 +413,12 @@ static struct nxp_fspi_devtype_data imx8dxl_data = {
 	.quirks = FSPI_QUIRK_USE_IP_ONLY,
 	.lut_num = 32,
 	.little_endian = true,  /* little-endian    */
+	/*
+	 * IMX8DXLCEC (i.MX 8XLite): SDR RXCLKSRC=0 max 60MHz,
+	 * DDR RXCLKSRC=3 max 200MHz.
+	 */
+	.max_sdr_rate = 60000000,
+	.max_dtr_rate = 200000000,
 };
 
 static struct nxp_fspi_devtype_data imx8ulp_data = {
@@ -385,6 +428,29 @@ static struct nxp_fspi_devtype_data imx8ulp_data = {
 	.quirks = 0,
 	.lut_num = 16,
 	.little_endian = true,  /* little-endian    */
+	/*
+	 * IMX8ULPCEC §7.3.1, Normal Drive (ND, 1.0V) mode:
+	 * SDR RXCLKSRC=0 max 60MHz, DDR RXCLKSRC=3 max 166MHz.
+	 * Note: Overdrive (OD, 1.05V) allows up to 180MHz DTR
+	 * but is not the default use case.
+	 */
+	.max_sdr_rate = 60000000,
+	.max_dtr_rate = 166000000,
+};
+
+static struct nxp_fspi_devtype_data imx95_data = {
+	.rxfifo = SZ_512,       /* (64  * 64 bits)  */
+	.txfifo = SZ_1K,        /* (128 * 64 bits)  */
+	.ahb_buf_size = SZ_2K,  /* (256 * 64 bits)  */
+	.quirks = 0,
+	.lut_num = 32,
+	.little_endian = true,  /* little-endian    */
+	/*
+	 * IMX95CEC Rev.8 §4.11.7: SDR RXCLKSRC=0 max 66MHz,
+	 * DDR RXCLKSRC=3 max 200MHz (Nominal/Overdrive mode).
+	 */
+	.max_sdr_rate = 66000000,
+	.max_dtr_rate = 200000000,
 };
 
 struct nxp_fspi {
@@ -691,10 +757,20 @@ static void nxp_fspi_select_rx_sample_clk_source(struct nxp_fspi *f,
 	reg = fspi_readl(f, f->iobase + FSPI_MCR0);
 	if (op_is_dtr) {
 		reg |= FSPI_MCR0_RXCLKSRC(3);
-		f->max_rate = 166000000;
+		/*
+		 * Use the SoC-specific DTR max rate if provided, otherwise
+		 * fall back to 166MHz (limit from IMX8MN datasheet §3.9.9).
+		 */
+		f->max_rate = f->devtype_data->max_dtr_rate ?
+			      f->devtype_data->max_dtr_rate : 166000000;
 	} else {	/*select mode 0 */
 		reg &= ~FSPI_MCR0_RXCLKSRC(3);
-		f->max_rate = 66000000;
+		/*
+		 * Use the SoC-specific SDR max rate if provided, otherwise
+		 * fall back to 66MHz (limit from IMX8MN datasheet §3.9.9).
+		 */
+		f->max_rate = f->devtype_data->max_sdr_rate ?
+			      f->devtype_data->max_sdr_rate : 66000000;
 	}
 	fspi_writel(f, reg, f->iobase + FSPI_MCR0);
 }
@@ -1457,10 +1533,11 @@ static const struct dev_pm_ops nxp_fspi_pm_ops = {
 static const struct of_device_id nxp_fspi_dt_ids[] = {
 	{ .compatible = "nxp,lx2160a-fspi", .data = (void *)&lx2160a_data, },
 	{ .compatible = "nxp,imx8mm-fspi", .data = (void *)&imx8mm_data, },
-	{ .compatible = "nxp,imx8mp-fspi", .data = (void *)&imx8mm_data, },
+	{ .compatible = "nxp,imx8mp-fspi", .data = (void *)&imx8mp_data, },
 	{ .compatible = "nxp,imx8qxp-fspi", .data = (void *)&imx8qxp_data, },
 	{ .compatible = "nxp,imx8dxl-fspi", .data = (void *)&imx8dxl_data, },
 	{ .compatible = "nxp,imx8ulp-fspi", .data = (void *)&imx8ulp_data, },
+	{ .compatible = "nxp,imx95-fspi",   .data = (void *)&imx95_data, },
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, nxp_fspi_dt_ids);

-- 
2.34.1

[PATCH v2 2/3] spi: spi-nxp-fspi: enter stop mode before reconfiguring MCR0 and DLL

[haibo.chen]

From: Haibo Chen <haibo.chen@nxp.com>

In nxp_fspi_select_mem() the RX sample clock source (MCR0[RXCLKSRC])
and the DLL control registers (DLLxCR) are reconfigured while the
FlexSPI module is still enabled. According to the FlexSPI reference
manual initialization sequence, MCR0 and the DLL control registers
should be programmed while the module is in stop mode, i.e. with
MCR0[MDIS] set to 1, and the module re-enabled (MCR0[MDIS] = 0)
afterwards.

Wrap the RX sample clock source selection and the DLL calibration/
override reconfiguration in a stop-mode window to align with the RM
and avoid reconfiguring timing-critical registers while the module is
active.

Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
---
 drivers/spi/spi-nxp-fspi.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/spi/spi-nxp-fspi.c b/drivers/spi/spi-nxp-fspi.c
index 6de6d432cdcb2ee383a02d72f20000045903e176..a426b8d92193a4e303f411cd17c16cb31c3f3873 100644
--- a/drivers/spi/spi-nxp-fspi.c
+++ b/drivers/spi/spi-nxp-fspi.c
@@ -867,6 +867,7 @@ static void nxp_fspi_select_mem(struct nxp_fspi *f, struct spi_device *spi,
 	unsigned long rate = op->max_freq;
 	int ret;
 	uint64_t size_kb;
+	u32 reg;
 
 	/*
 	 * Return when following condition all meet,
@@ -896,6 +897,15 @@ static void nxp_fspi_select_mem(struct nxp_fspi *f, struct spi_device *spi,
 
 	dev_dbg(f->dev, "Target device [CS:%x] selected\n", spi_get_chipselect(spi, 0));
 
+	/*
+	 * Per the FlexSPI reference manual (initialization sequence), MCR0 and
+	 * the DLL control registers should be configured while the module is in
+	 * stop mode (MCR0[MDIS] = 1). Enter stop mode before reconfiguring the
+	 * RX sample clock source and the DLL, then exit stop mode afterwards.
+	 */
+	reg = fspi_readl(f, f->iobase + FSPI_MCR0);
+	fspi_writel(f, reg | FSPI_MCR0_MDIS, f->iobase + FSPI_MCR0);
+
 	nxp_fspi_select_rx_sample_clk_source(f, op_is_dtr);
 	rate = min(f->max_rate, op->max_freq);
 
@@ -928,6 +938,10 @@ static void nxp_fspi_select_mem(struct nxp_fspi *f, struct spi_device *spi,
 	else
 		nxp_fspi_dll_override(f);
 
+	/* Exit stop mode now that MCR0 and the DLL have been reconfigured. */
+	reg = fspi_readl(f, f->iobase + FSPI_MCR0);
+	fspi_writel(f, reg & ~FSPI_MCR0_MDIS, f->iobase + FSPI_MCR0);
+
 	f->pre_op_rate = op->max_freq;
 
 	f->selected = spi_get_chipselect(spi, 0);

-- 
2.34.1

[PATCH v2 3/3] spi: spi-nxp-fspi: propagate clock reconfig failures in nxp_fspi_select_mem()

[haibo.chen]

From: Haibo Chen <haibo.chen@nxp.com>

nxp_fspi_select_mem() disables the FlexSPI clocks before calling
clk_set_rate() and re-enabling them. If clk_set_rate() or the clock
re-enable fails, the function returned early (as void) leaving both the
serial root clock and the register interface clock disabled.

As the function returned void, nxp_fspi_exec_op() had no way to know
about the failure and continued to access FlexSPI registers (LUT setup,
data transfer, AHB buffer invalidation). Accessing the controller
registers while its clock is gated off results in a synchronous external
abort.

Make nxp_fspi_select_mem() return an error code and have
nxp_fspi_exec_op() bail out on failure before any further register
access, including nxp_fspi_invalid().

Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
---
 drivers/spi/spi-nxp-fspi.c | 32 +++++++++++++++++++++++++-------
 1 file changed, 25 insertions(+), 7 deletions(-)

diff --git a/drivers/spi/spi-nxp-fspi.c b/drivers/spi/spi-nxp-fspi.c
index a426b8d92193a4e303f411cd17c16cb31c3f3873..67d991dd249d283e5a7b777c3ef4a70a9af3f065 100644
--- a/drivers/spi/spi-nxp-fspi.c
+++ b/drivers/spi/spi-nxp-fspi.c
@@ -859,8 +859,8 @@ static void nxp_fspi_dll_override(struct nxp_fspi *f)
  * Value for rest of the CS FLSHxxCR0 register would be zero.
  *
  */
-static void nxp_fspi_select_mem(struct nxp_fspi *f, struct spi_device *spi,
-				const struct spi_mem_op *op)
+static int nxp_fspi_select_mem(struct nxp_fspi *f, struct spi_device *spi,
+			       const struct spi_mem_op *op)
 {
 	/* flexspi only support one DTR mode: 8D-8D-8D */
 	bool op_is_dtr = op->cmd.dtr && op->addr.dtr && op->dummy.dtr && op->data.dtr;
@@ -881,7 +881,7 @@ static void nxp_fspi_select_mem(struct nxp_fspi *f, struct spi_device *spi,
 	if ((f->selected == spi_get_chipselect(spi, 0)) &&
 	    (!!(f->flags & FSPI_DTR_MODE) == op_is_dtr) &&
 	    (f->pre_op_rate == op->max_freq))
-		return;
+		return 0;
 
 	/* Reset FLSHxxCR0 registers */
 	fspi_writel(f, 0, f->iobase + FSPI_FLSHA1CR0);
@@ -922,12 +922,19 @@ static void nxp_fspi_select_mem(struct nxp_fspi *f, struct spi_device *spi,
 	nxp_fspi_clk_disable_unprep(f);
 
 	ret = clk_set_rate(f->clk, rate);
-	if (ret)
-		return;
+	if (ret) {
+		/*
+		 * clk_set_rate() failed with the clocks already disabled.
+		 * Re-enable them so the enable count matches what the caller's
+		 * pm_runtime_put() (runtime_suspend) will drop.
+		 */
+		nxp_fspi_clk_prep_enable(f);
+		return ret;
+	}
 
 	ret = nxp_fspi_clk_prep_enable(f);
 	if (ret)
-		return;
+		return ret;
 
 	/*
 	 * If clock rate > 100MHz, then switch from DLL override mode to
@@ -945,6 +952,8 @@ static void nxp_fspi_select_mem(struct nxp_fspi *f, struct spi_device *spi,
 	f->pre_op_rate = op->max_freq;
 
 	f->selected = spi_get_chipselect(spi, 0);
+
+	return 0;
 }
 
 static int nxp_fspi_read_ahb(struct nxp_fspi *f, const struct spi_mem_op *op)
@@ -1132,7 +1141,16 @@ static int nxp_fspi_exec_op(struct spi_mem *mem, const struct spi_mem_op *op)
 				   FSPI_STS0_ARB_IDLE, 1, POLL_TOUT, true);
 	WARN_ON(err);
 
-	nxp_fspi_select_mem(f, mem->spi, op);
+	err = nxp_fspi_select_mem(f, mem->spi, op);
+	if (err) {
+		/*
+		 * On failure the FlexSPI clock may be left disabled, so avoid
+		 * any further register access (which would trigger a synchronous
+		 * external abort) and bail out.
+		 */
+		pm_runtime_put_autosuspend(f->dev);
+		return err;
+	}
 
 	nxp_fspi_prepare_lut(f, op);
 	/*

-- 
2.34.1

Re: [PATCH v2 2/3] spi: spi-nxp-fspi: enter stop mode before reconfiguring MCR0 and DLL

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Error paths in nxp_fspi_select_mem() leave the FlexSPI module permanently in stop mode.
--

--- Patch [2]: [PATCH v2 2/3] spi: spi-nxp-fspi: enter stop mode before reconfiguring MCR0 and DLL ---
commit 4391ccb7c73c0489ac0b6f714a7073bb2b08b3c1
Author: Haibo Chen <haibo.chen@nxp.com>

spi: spi-nxp-fspi: enter stop mode before reconfiguring MCR0 and DLL

This patch modifies nxp_fspi_select_mem() to enter stop mode by setting
MCR0[MDIS] before reconfiguring the RX sample clock source and DLL control
registers. It then exits stop mode once the reconfiguration is complete
according to the reference manual.

> diff --git a/drivers/spi/spi-nxp-fspi.c b/drivers/spi/spi-nxp-fspi.c
> index 6de6d432cdcb2..a426b8d92193a 100644
> --- a/drivers/spi/spi-nxp-fspi.c
> +++ b/drivers/spi/spi-nxp-fspi.c
[ ... ]
> @@ -896,6 +897,15 @@ static void nxp_fspi_select_mem(struct nxp_fspi *f, struct spi_device *spi,
>  
>  	dev_dbg(f->dev, "Target device [CS:%x] selected\n", spi_get_chipselect(spi, 0));
>  
> +	/*
> +	 * Per the FlexSPI reference manual (initialization sequence), MCR0 and
> +	 * the DLL control registers should be configured while the module is in
> +	 * stop mode (MCR0[MDIS] = 1). Enter stop mode before reconfiguring the
> +	 * RX sample clock source and the DLL, then exit stop mode afterwards.
> +	 */
> +	reg = fspi_readl(f, f->iobase + FSPI_MCR0);
> +	fspi_writel(f, reg | FSPI_MCR0_MDIS, f->iobase + FSPI_MCR0);
> +
>  	nxp_fspi_select_rx_sample_clk_source(f, op_is_dtr);
>  	rate = min(f->max_rate, op->max_freq);
>  
[ ... ]
> @@ -928,6 +938,10 @@ static void nxp_fspi_select_mem(struct nxp_fspi *f, struct spi_device *spi,
>  	else
>  		nxp_fspi_dll_override(f);
>  
> +	/* Exit stop mode now that MCR0 and the DLL have been reconfigured. */
> +	reg = fspi_readl(f, f->iobase + FSPI_MCR0);
> +	fspi_writel(f, reg & ~FSPI_MCR0_MDIS, f->iobase + FSPI_MCR0);
> +

[Severity: High]
Does this leave the FlexSPI module permanently in stop mode if the clock
configuration fails earlier in this function?

If clk_set_rate() or nxp_fspi_clk_prep_enable() fails, the code returns early
without executing this exit block:

        ret = clk_set_rate(f->clk, rate);
        if (ret) {
                /* ... */
                nxp_fspi_clk_prep_enable(f);
                return ret;
        }

Because the early returns happen before the stop mode is cleared, the
hardware module is left disabled since FSPI_MCR0_MDIS remains set.

If this error occurs, a subsequent SPI transfer matching the previous
successful state could hit the early-return fast path at the top of
nxp_fspi_select_mem(), skip the hardware reconfiguration, and attempt
to execute a transfer while the hardware is still disabled.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260728-fspi-clock-v2-0-dbe786a4a6eb@nxp.com?part=2

Re: [PATCH v2 1/3] spi: spi-nxp-fspi: add per-SoC SDR/DTR clock rate limits for all supported SoCs

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but clock configuration failures in `nxp_fspi_select_mem()` and `nxp_fspi_default_setup()` are silently ignored, leaving the controller unclocked and leading to system crashes upon subsequent register accesses.
--

--- Patch [1]: [PATCH v2 1/3] spi: spi-nxp-fspi: add per-SoC SDR/DTR clock rate limits for all supported SoCs ---
commit f96188480d6d41456b1f17269268e30b0c1612e5
Author: Haibo Chen <haibo.chen@nxp.com>

spi: spi-nxp-fspi: add per-SoC SDR/DTR clock rate limits for all supported SoCs

This commit corrects and updates the maximum clock rates for SDR and DTR
modes across various supported NXP SoCs. It adds max_sdr_rate and
max_dtr_rate fields to the devtype data and updates the runtime clock
rate selection to respect these SoC-specific limits.

> diff --git a/drivers/spi/spi-nxp-fspi.c b/drivers/spi/spi-nxp-fspi.c
> --- a/drivers/spi/spi-nxp-fspi.c
> +++ b/drivers/spi/spi-nxp-fspi.c

[ ... ]

> @@ -691,10 +757,20 @@ static void nxp_fspi_select_rx_sample_clk_source(struct nxp_fspi *f,
>  	reg = fspi_readl(f, f->iobase + FSPI_MCR0);
>  	if (op_is_dtr) {
>  		reg |= FSPI_MCR0_RXCLKSRC(3);
> -		f->max_rate = 166000000;
> +		/*
> +		 * Use the SoC-specific DTR max rate if provided, otherwise
> +		 * fall back to 166MHz (limit from IMX8MN datasheet §3.9.9).
> +		 */
> +		f->max_rate = f->devtype_data->max_dtr_rate ?
> +			      f->devtype_data->max_dtr_rate : 166000000;
>  	} else {	/*select mode 0 */
>  		reg &= ~FSPI_MCR0_RXCLKSRC(3);
> -		f->max_rate = 66000000;
> +		/*
> +		 * Use the SoC-specific SDR max rate if provided, otherwise
> +		 * fall back to 66MHz (limit from IMX8MN datasheet §3.9.9).
> +		 */
> +		f->max_rate = f->devtype_data->max_sdr_rate ?
> +			      f->devtype_data->max_sdr_rate : 66000000;

[Severity: High]
This is a pre-existing issue, but I noticed that changing the maximum rate
limit could potentially expose a silent failure path if the clock framework
returns an error for these newly configured max limits.

In nxp_fspi_select_mem(), the clock is disabled before configuring the
rate. If clk_set_rate() or nxp_fspi_clk_prep_enable() fails, the function
returns early without propagating the error:

drivers/spi/spi-nxp-fspi.c:nxp_fspi_select_mem() {
    ...
    nxp_fspi_clk_disable_unprep(f);

    ret = clk_set_rate(f->clk, rate);
    if (ret)
        return;

    ret = nxp_fspi_clk_prep_enable(f);
    if (ret)
        return;
    ...
}

Since nxp_fspi_select_mem() returns void, its caller nxp_fspi_exec_op()
continues executing and accesses registers in nxp_fspi_prepare_lut(), even
though the clock is left disabled:

drivers/spi/spi-nxp-fspi.c:nxp_fspi_exec_op() {
    ...
    nxp_fspi_select_mem(f, mem->spi, op);

    nxp_fspi_prepare_lut(f, op);
    ...
}

Could this unclocked IP block access lead to a system crash (such as an
external abort or kernel panic)?

Additionally, nxp_fspi_probe() seems to completely ignore the return value
of nxp_fspi_default_setup():

drivers/spi/spi-nxp-fspi.c:nxp_fspi_probe() {
    ...
    nxp_fspi_default_setup(f);

    ret = pm_runtime_put_sync(dev);
    ...
}

Should these errors be propagated up the call chain to ensure we avoid
accessing memory-mapped registers when the clock fails to initialize or
enable?

>  	}
>  	fspi_writel(f, reg, f->iobase + FSPI_MCR0);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260728-fspi-clock-v2-0-dbe786a4a6eb@nxp.com?part=1

Re: [PATCH v2 3/3] spi: spi-nxp-fspi: propagate clock reconfig failures in nxp_fspi_select_mem()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:

New issues:
- [Critical] Incomplete state rollback on `nxp_fspi_select_mem()` failure leaves cached flags out of sync and the controller permanently stuck in Stop mode.
- [High] Error handling in `nxp_fspi_select_mem()` leaves hardware clocks disabled while PM runtime remains active, leading to deferred synchronous external aborts and unbalanced clock disables.

Pre-existing issues:
- [High] Missing `pm_runtime_put_noidle()` when `pm_runtime_get_sync()` fails, leaking the PM runtime usage counter.
- [High] The return value of `nxp_fspi_default_setup()` is completely ignored, masking initialization and clock setup failures.
- [High] The timeout error from polling the controller status is completely discarded in `nxp_fspi_exec_op()`.
--

--- Patch [3]: [PATCH v2 3/3] spi: spi-nxp-fspi: propagate clock reconfig failures in nxp_fspi_select_mem() ---
commit dbeb737d74251745ba44e92f371ba23d08069151
Author: Haibo Chen <haibo.chen@nxp.com>

spi: spi-nxp-fspi: propagate clock reconfig failures in nxp_fspi_select_mem()

This commit modifies nxp_fspi_select_mem() to return an error code if
clk_set_rate() or clock enabling fails. It also updates nxp_fspi_exec_op()
to check this return value and bail out to prevent accessing registers while
the clock is gated off.

> diff --git a/drivers/spi/spi-nxp-fspi.c b/drivers/spi/spi-nxp-fspi.c
> --- a/drivers/spi/spi-nxp-fspi.c
> +++ b/drivers/spi/spi-nxp-fspi.c
> @@ -922,12 +922,19 @@ static void nxp_fspi_select_mem(struct nxp_fspi *f, struct spi_device *spi,
>  	nxp_fspi_clk_disable_unprep(f);
>  
>  	ret = clk_set_rate(f->clk, rate);
> -	if (ret)
> -		return;
> +	if (ret) {
> +		/*
> +		 * clk_set_rate() failed with the clocks already disabled.
> +		 * Re-enable them so the enable count matches what the caller's
> +		 * pm_runtime_put() (runtime_suspend) will drop.
> +		 */
> +		nxp_fspi_clk_prep_enable(f);
> +		return ret;

[Severity: Critical]
If clk_set_rate() fails and we return early here, does this leave the
controller permanently in stop mode?

Earlier in nxp_fspi_select_mem(), the controller is placed into stop mode:

    reg = fspi_readl(f, f->iobase + FSPI_MCR0);
    fspi_writel(f, reg | FSPI_MCR0_MDIS, f->iobase + FSPI_MCR0);

Since the early return bypasses the exit from stop mode at the end of the
function, the controller remains stopped. Additionally, f->flags might have
been updated with FSPI_DTR_MODE. If a subsequent SPI operation matches the
fast-path check at the beginning of nxp_fspi_select_mem(), it would bypass
reconfiguration and attempt to execute commands on a stopped controller,
resulting in a hang.

[ ... ]

> @@ -1132,7 +1141,16 @@ static int nxp_fspi_exec_op(struct spi_mem *mem, const struct spi_mem_op *op)
>  				   FSPI_STS0_ARB_IDLE, 1, POLL_TOUT, true);
>  	WARN_ON(err);
>  
> -	nxp_fspi_select_mem(f, mem->spi, op);
> +	err = nxp_fspi_select_mem(f, mem->spi, op);

[Severity: High]
This is a pre-existing issue, but does assigning to err here discard the
timeout error from fspi_readl_poll_tout() immediately above?

If fspi_readl_poll_tout() times out, it returns -ETIMEDOUT to err, which is
flagged by WARN_ON(), but the function does not abort. Instead, err is
immediately overwritten by nxp_fspi_select_mem(), causing subsequent
operations to execute on a busy controller.

> +	if (err) {
> +		/*
> +		 * On failure the FlexSPI clock may be left disabled, so avoid
> +		 * any further register access (which would trigger a synchronous
> +		 * external abort) and bail out.
> +		 */
> +		pm_runtime_put_autosuspend(f->dev);

[Severity: High]
If nxp_fspi_select_mem() fails and leaves the clocks physically disabled, does
calling pm_runtime_put_autosuspend() leave the PM runtime state out of sync?

pm_runtime_put_autosuspend() maintains the RPM_ACTIVE state until the
autosuspend timer expires. If a subsequent SPI operation begins before the
timer fires, pm_runtime_get_sync() will succeed without re-enabling the
clocks. nxp_fspi_exec_op() will then attempt register access while the clock
is gated off, triggering a synchronous external abort.

If no operation occurs, the timer fires and runtime_suspend will disable
the already-gated clocks, potentially causing an enable count underflow.

> +		return err;
> +	}
>  
>  	nxp_fspi_prepare_lut(f, op);

[ ... ]

For pre-existing issues not directly modified by this patch, there are a few
other observations in this file:

[Severity: High]
This is a pre-existing issue, but does nxp_fspi_exec_op() leak the PM
runtime usage counter if pm_runtime_get_sync() fails?

drivers/spi/spi-nxp-fspi.c:nxp_fspi_exec_op() {
    ...
    err = pm_runtime_get_sync(f->dev);
    if (err < 0) {
        dev_err(f->dev, "Failed to enable clock %d\n", __LINE__);
        return err;
    }
    ...
}

If pm_runtime_get_sync() returns a negative error code, the usage counter
remains incremented. Returning immediately without calling
pm_runtime_put_noidle() permanently prevents the device from runtime
suspending again.

[Severity: High]
This is a pre-existing issue, but are initialization and clock setup failures
being masked during probe and resume?

drivers/spi/spi-nxp-fspi.c:nxp_fspi_probe() {
    ...
    nxp_fspi_default_setup(f);
    ...
}

The return value of nxp_fspi_default_setup() is completely ignored in both
nxp_fspi_probe() and nxp_fspi_runtime_resume(). If clk_set_rate() or clock
enabling fails inside setup, the driver proceeds as if the hardware is
fully initialized, which could lead to silent failures.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260728-fspi-clock-v2-0-dbe786a4a6eb@nxp.com?part=3

Re: [PATCH v2 1/3] spi: spi-nxp-fspi: add per-SoC SDR/DTR clock rate limits for all supported SoCs

[Frank Li]

On Tue, Jul 28, 2026 at 06:18:08PM +0800, haibo.chen@oss.nxp.com wrote:
> From: Haibo Chen <haibo.chen@nxp.com>
>
> The commit f43579ef3500 ("spi: spi-nxp-fspi: limit the clock rate for
> different sample clock source selection") introduced a global 166MHz
> cap for DTR mode (RXCLKSRC=3), based on the i.MX8MN datasheet timing
> specification (Section 3.9.9, page 65).
>
> After reviewing the FlexSPI timing parameters in the datasheets for all
> supported SoCs, the following corrections and additions are needed:
>
> 1. SDR mode (RXCLKSRC=0) limits vary per SoC:
>    - i.MX8MN/MM/MP/95: 66MHz  (IMX8MNCEC §3.9.9, IMX8MMCEC §3.9.10,
>                                 IMX8MPCEC, IMX95CEC Rev.8 §4.11.7)
>    - i.MX8QXP/QM/DXL/ULP: 60MHz (IMX8QXPCEC, IMX8QMCEC, IMX8DXLCEC,
>                                    IMX8ULPCEC §7.3.1 ND mode)
>    - LX2160A: 100MHz            (LX2160ACEC FlexSPI timing parameters)
>
> 2. DTR mode (RXCLKSRC=3) limits vary per SoC:
>    - i.MX8MN/MM/MP/ULP: 166MHz
>    - i.MX8QXP/QM/DXL: 200MHz   (same FlexSPI IP across this family)
>    - i.MX95: 200MHz             (IMX95CEC §4.11.7.3.2.3 Table 106)
>    - LX2160A: DTR disabled      (FSPI_QUIRK_DISABLE_DTR)
>
> Update related platform data with correct speed limation according
> to datasheet.
>
> Fixes: f43579ef3500 ("spi: spi-nxp-fspi: limit the clock rate for different sample clock source selection")
> Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>  drivers/spi/spi-nxp-fspi.c | 83 ++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 80 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/spi/spi-nxp-fspi.c b/drivers/spi/spi-nxp-fspi.c
> index d94a2a7b98d44db217ba96a8ce7df23e37a08ba4..6de6d432cdcb2ee383a02d72f20000045903e176 100644
> --- a/drivers/spi/spi-nxp-fspi.c
> +++ b/drivers/spi/spi-nxp-fspi.c
> @@ -340,6 +340,18 @@ struct nxp_fspi_devtype_data {
>  	unsigned int quirks;
>  	unsigned int lut_num;
>  	bool little_endian;
> +	/*
> +	 * The max clock rate (Hz) that FlexSPI can output to the device
> +	 * in SDR mode (RXCLKSRC=0). Defaults to 66MHz if zero.
> +	 * Some SoCs (e.g. LX2160A) support up to 100MHz in SDR mode.
> +	 */
> +	unsigned long max_sdr_rate;
> +	/*
> +	 * The max clock rate (Hz) that FlexSPI can output to the device
> +	 * in DTR mode (RXCLKSRC=3). Defaults to 166MHz if zero.
> +	 * Some SoCs (e.g. i.MX95, i.MX8QM, i.MX8DXL) support up to 200MHz.
> +	 */
> +	unsigned long max_dtr_rate;
>  };
>
>  static struct nxp_fspi_devtype_data lx2160a_data = {
> @@ -349,6 +361,10 @@ static struct nxp_fspi_devtype_data lx2160a_data = {
>  	.quirks = FSPI_QUIRK_DISABLE_DTR,
>  	.lut_num = 32,
>  	.little_endian = true,  /* little-endian    */
> +	/*
> +	 * LX2160ACEC: SDR RXCLKSRC=0 max 100MHz, DTR disabled via quirk.
> +	 */
> +	.max_sdr_rate = 100000000,
>  };
>
>  static struct nxp_fspi_devtype_data imx8mm_data = {
> @@ -358,6 +374,21 @@ static struct nxp_fspi_devtype_data imx8mm_data = {
>  	.quirks = 0,
>  	.lut_num = 32,
>  	.little_endian = true,  /* little-endian    */
> +	/* IMX8MMCEC §3.9.10: SDR RXCLKSRC=0 max 66MHz, DDR RXCLKSRC=3 max 166MHz */
> +	.max_sdr_rate = 66000000,
> +	.max_dtr_rate = 166000000,
> +};
> +
> +static struct nxp_fspi_devtype_data imx8mp_data = {
> +	.rxfifo = SZ_512,       /* (64  * 64 bits)  */
> +	.txfifo = SZ_1K,        /* (128 * 64 bits)  */
> +	.ahb_buf_size = SZ_2K,  /* (256 * 64 bits)  */
> +	.quirks = 0,
> +	.lut_num = 32,
> +	.little_endian = true,  /* little-endian    */
> +	/* IMX8MPCEC: SDR RXCLKSRC=0 max 66MHz, DDR RXCLKSRC=3 max 166MHz */
> +	.max_sdr_rate = 66000000,
> +	.max_dtr_rate = 166000000,
>  };
>
>  static struct nxp_fspi_devtype_data imx8qxp_data = {
> @@ -367,6 +398,12 @@ static struct nxp_fspi_devtype_data imx8qxp_data = {
>  	.quirks = 0,
>  	.lut_num = 32,
>  	.little_endian = true,  /* little-endian    */
> +	/*
> +	 * IMX8QXPCEC: SDR RXCLKSRC=0 max 60MHz, DDR RXCLKSRC=3 max 200MHz.
> +	 * i.MX8QM and i.MX8DXL share the same FlexSPI IP and limits.
> +	 */
> +	.max_sdr_rate = 60000000,
> +	.max_dtr_rate = 200000000,
>  };
>
>  static struct nxp_fspi_devtype_data imx8dxl_data = {
> @@ -376,6 +413,12 @@ static struct nxp_fspi_devtype_data imx8dxl_data = {
>  	.quirks = FSPI_QUIRK_USE_IP_ONLY,
>  	.lut_num = 32,
>  	.little_endian = true,  /* little-endian    */
> +	/*
> +	 * IMX8DXLCEC (i.MX 8XLite): SDR RXCLKSRC=0 max 60MHz,
> +	 * DDR RXCLKSRC=3 max 200MHz.
> +	 */
> +	.max_sdr_rate = 60000000,
> +	.max_dtr_rate = 200000000,
>  };
>
>  static struct nxp_fspi_devtype_data imx8ulp_data = {
> @@ -385,6 +428,29 @@ static struct nxp_fspi_devtype_data imx8ulp_data = {
>  	.quirks = 0,
>  	.lut_num = 16,
>  	.little_endian = true,  /* little-endian    */
> +	/*
> +	 * IMX8ULPCEC §7.3.1, Normal Drive (ND, 1.0V) mode:
> +	 * SDR RXCLKSRC=0 max 60MHz, DDR RXCLKSRC=3 max 166MHz.
> +	 * Note: Overdrive (OD, 1.05V) allows up to 180MHz DTR
> +	 * but is not the default use case.
> +	 */
> +	.max_sdr_rate = 60000000,
> +	.max_dtr_rate = 166000000,
> +};
> +
> +static struct nxp_fspi_devtype_data imx95_data = {
> +	.rxfifo = SZ_512,       /* (64  * 64 bits)  */
> +	.txfifo = SZ_1K,        /* (128 * 64 bits)  */
> +	.ahb_buf_size = SZ_2K,  /* (256 * 64 bits)  */
> +	.quirks = 0,
> +	.lut_num = 32,
> +	.little_endian = true,  /* little-endian    */
> +	/*
> +	 * IMX95CEC Rev.8 §4.11.7: SDR RXCLKSRC=0 max 66MHz,
> +	 * DDR RXCLKSRC=3 max 200MHz (Nominal/Overdrive mode).
> +	 */
> +	.max_sdr_rate = 66000000,
> +	.max_dtr_rate = 200000000,
>  };
>
>  struct nxp_fspi {
> @@ -691,10 +757,20 @@ static void nxp_fspi_select_rx_sample_clk_source(struct nxp_fspi *f,
>  	reg = fspi_readl(f, f->iobase + FSPI_MCR0);
>  	if (op_is_dtr) {
>  		reg |= FSPI_MCR0_RXCLKSRC(3);
> -		f->max_rate = 166000000;
> +		/*
> +		 * Use the SoC-specific DTR max rate if provided, otherwise
> +		 * fall back to 166MHz (limit from IMX8MN datasheet §3.9.9).
> +		 */
> +		f->max_rate = f->devtype_data->max_dtr_rate ?
> +			      f->devtype_data->max_dtr_rate : 166000000;
>  	} else {	/*select mode 0 */
>  		reg &= ~FSPI_MCR0_RXCLKSRC(3);
> -		f->max_rate = 66000000;
> +		/*
> +		 * Use the SoC-specific SDR max rate if provided, otherwise
> +		 * fall back to 66MHz (limit from IMX8MN datasheet §3.9.9).
> +		 */
> +		f->max_rate = f->devtype_data->max_sdr_rate ?
> +			      f->devtype_data->max_sdr_rate : 66000000;
>  	}
>  	fspi_writel(f, reg, f->iobase + FSPI_MCR0);
>  }
> @@ -1457,10 +1533,11 @@ static const struct dev_pm_ops nxp_fspi_pm_ops = {
>  static const struct of_device_id nxp_fspi_dt_ids[] = {
>  	{ .compatible = "nxp,lx2160a-fspi", .data = (void *)&lx2160a_data, },
>  	{ .compatible = "nxp,imx8mm-fspi", .data = (void *)&imx8mm_data, },
> -	{ .compatible = "nxp,imx8mp-fspi", .data = (void *)&imx8mm_data, },
> +	{ .compatible = "nxp,imx8mp-fspi", .data = (void *)&imx8mp_data, },
>  	{ .compatible = "nxp,imx8qxp-fspi", .data = (void *)&imx8qxp_data, },
>  	{ .compatible = "nxp,imx8dxl-fspi", .data = (void *)&imx8dxl_data, },
>  	{ .compatible = "nxp,imx8ulp-fspi", .data = (void *)&imx8ulp_data, },
> +	{ .compatible = "nxp,imx95-fspi",   .data = (void *)&imx95_data, },
>  	{ /* sentinel */ }
>  };
>  MODULE_DEVICE_TABLE(of, nxp_fspi_dt_ids);
>
> --
> 2.34.1
>
>

Re: [PATCH v2 2/3] spi: spi-nxp-fspi: enter stop mode before reconfiguring MCR0 and DLL

[Frank Li]

On Tue, Jul 28, 2026 at 06:18:09PM +0800, haibo.chen@oss.nxp.com wrote:
> From: Haibo Chen <haibo.chen@nxp.com>
>
> In nxp_fspi_select_mem() the RX sample clock source (MCR0[RXCLKSRC])
> and the DLL control registers (DLLxCR) are reconfigured while the
> FlexSPI module is still enabled. According to the FlexSPI reference
> manual initialization sequence, MCR0 and the DLL control registers
> should be programmed while the module is in stop mode, i.e. with
> MCR0[MDIS] set to 1, and the module re-enabled (MCR0[MDIS] = 0)
> afterwards.
>
> Wrap the RX sample clock source selection and the DLL calibration/
> override reconfiguration in a stop-mode window to align with the RM
> and avoid reconfiguring timing-critical registers while the module is
> active.
>
> Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>  drivers/spi/spi-nxp-fspi.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/drivers/spi/spi-nxp-fspi.c b/drivers/spi/spi-nxp-fspi.c
> index 6de6d432cdcb2ee383a02d72f20000045903e176..a426b8d92193a4e303f411cd17c16cb31c3f3873 100644
> --- a/drivers/spi/spi-nxp-fspi.c
> +++ b/drivers/spi/spi-nxp-fspi.c
> @@ -867,6 +867,7 @@ static void nxp_fspi_select_mem(struct nxp_fspi *f, struct spi_device *spi,
>  	unsigned long rate = op->max_freq;
>  	int ret;
>  	uint64_t size_kb;
> +	u32 reg;
>
>  	/*
>  	 * Return when following condition all meet,
> @@ -896,6 +897,15 @@ static void nxp_fspi_select_mem(struct nxp_fspi *f, struct spi_device *spi,
>
>  	dev_dbg(f->dev, "Target device [CS:%x] selected\n", spi_get_chipselect(spi, 0));
>
> +	/*
> +	 * Per the FlexSPI reference manual (initialization sequence), MCR0 and
> +	 * the DLL control registers should be configured while the module is in
> +	 * stop mode (MCR0[MDIS] = 1). Enter stop mode before reconfiguring the
> +	 * RX sample clock source and the DLL, then exit stop mode afterwards.
> +	 */
> +	reg = fspi_readl(f, f->iobase + FSPI_MCR0);
> +	fspi_writel(f, reg | FSPI_MCR0_MDIS, f->iobase + FSPI_MCR0);
> +
>  	nxp_fspi_select_rx_sample_clk_source(f, op_is_dtr);
>  	rate = min(f->max_rate, op->max_freq);
>
> @@ -928,6 +938,10 @@ static void nxp_fspi_select_mem(struct nxp_fspi *f, struct spi_device *spi,
>  	else
>  		nxp_fspi_dll_override(f);
>
> +	/* Exit stop mode now that MCR0 and the DLL have been reconfigured. */
> +	reg = fspi_readl(f, f->iobase + FSPI_MCR0);
> +	fspi_writel(f, reg & ~FSPI_MCR0_MDIS, f->iobase + FSPI_MCR0);
> +
>  	f->pre_op_rate = op->max_freq;
>
>  	f->selected = spi_get_chipselect(spi, 0);
>
> --
> 2.34.1
>
>

Re: [PATCH v2 3/3] spi: spi-nxp-fspi: propagate clock reconfig failures in nxp_fspi_select_mem()

[Frank Li]

On Tue, Jul 28, 2026 at 10:28:31AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
>
> New issues:
> - [Critical] Incomplete state rollback on `nxp_fspi_select_mem()` failure leaves cached flags out of sync and the controller permanently stuck in Stop mode.
> - [High] Error handling in `nxp_fspi_select_mem()` leaves hardware clocks disabled while PM runtime remains active, leading to deferred synchronous external aborts and unbalanced clock disables.
>
> Pre-existing issues:
> - [High] Missing `pm_runtime_put_noidle()` when `pm_runtime_get_sync()` fails, leaking the PM runtime usage counter.
> - [High] The return value of `nxp_fspi_default_setup()` is completely ignored, masking initialization and clock setup failures.
> - [High] The timeout error from polling the controller status is completely discarded in `nxp_fspi_exec_op()`.
> --
>
> --- Patch [3]: [PATCH v2 3/3] spi: spi-nxp-fspi: propagate clock reconfig failures in nxp_fspi_select_mem() ---
> commit dbeb737d74251745ba44e92f371ba23d08069151
> Author: Haibo Chen <haibo.chen@nxp.com>
>
> spi: spi-nxp-fspi: propagate clock reconfig failures in nxp_fspi_select_mem()
>
> This commit modifies nxp_fspi_select_mem() to return an error code if
> clk_set_rate() or clock enabling fails. It also updates nxp_fspi_exec_op()
> to check this return value and bail out to prevent accessing registers while
> the clock is gated off.
>
> > diff --git a/drivers/spi/spi-nxp-fspi.c b/drivers/spi/spi-nxp-fspi.c
> > --- a/drivers/spi/spi-nxp-fspi.c
> > +++ b/drivers/spi/spi-nxp-fspi.c
> > @@ -922,12 +922,19 @@ static void nxp_fspi_select_mem(struct nxp_fspi *f, struct spi_device *spi,
> >  	nxp_fspi_clk_disable_unprep(f);
> >
> >  	ret = clk_set_rate(f->clk, rate);
> > -	if (ret)
> > -		return;
> > +	if (ret) {
> > +		/*
> > +		 * clk_set_rate() failed with the clocks already disabled.
> > +		 * Re-enable them so the enable count matches what the caller's
> > +		 * pm_runtime_put() (runtime_suspend) will drop.
> > +		 */
> > +		nxp_fspi_clk_prep_enable(f);
> > +		return ret;
>
> [Severity: Critical]
> If clk_set_rate() fails and we return early here, does this leave the
> controller permanently in stop mode?
>
> Earlier in nxp_fspi_select_mem(), the controller is placed into stop mode:
>
>     reg = fspi_readl(f, f->iobase + FSPI_MCR0);
>     fspi_writel(f, reg | FSPI_MCR0_MDIS, f->iobase + FSPI_MCR0);
>
> Since the early return bypasses the exit from stop mode at the end of the
> function, the controller remains stopped. Additionally, f->flags might have
> been updated with FSPI_DTR_MODE. If a subsequent SPI operation matches the
> fast-path check at the beginning of nxp_fspi_select_mem(), it would bypass
> reconfiguration and attempt to execute commands on a stopped controller,
> resulting in a hang.

Is it true?

Frank
>
> [ ... ]
>
> > @@ -1132,7 +1141,16 @@ static int nxp_fspi_exec_op(struct spi_mem *mem, const struct spi_mem_op *op)
> >  				   FSPI_STS0_ARB_IDLE, 1, POLL_TOUT, true);
> >  	WARN_ON(err);
> >
> > -	nxp_fspi_select_mem(f, mem->spi, op);
> > +	err = nxp_fspi_select_mem(f, mem->spi, op);
>
> [Severity: High]
> This is a pre-existing issue, but does assigning to err here discard the
> timeout error from fspi_readl_poll_tout() immediately above?
>
> If fspi_readl_poll_tout() times out, it returns -ETIMEDOUT to err, which is
> flagged by WARN_ON(), but the function does not abort. Instead, err is
> immediately overwritten by nxp_fspi_select_mem(), causing subsequent
> operations to execute on a busy controller.
>
> > +	if (err) {
> > +		/*
> > +		 * On failure the FlexSPI clock may be left disabled, so avoid
> > +		 * any further register access (which would trigger a synchronous
> > +		 * external abort) and bail out.
> > +		 */
> > +		pm_runtime_put_autosuspend(f->dev);
>
> [Severity: High]
> If nxp_fspi_select_mem() fails and leaves the clocks physically disabled, does
> calling pm_runtime_put_autosuspend() leave the PM runtime state out of sync?
>
> pm_runtime_put_autosuspend() maintains the RPM_ACTIVE state until the
> autosuspend timer expires. If a subsequent SPI operation begins before the
> timer fires, pm_runtime_get_sync() will succeed without re-enabling the
> clocks. nxp_fspi_exec_op() will then attempt register access while the clock
> is gated off, triggering a synchronous external abort.
>
> If no operation occurs, the timer fires and runtime_suspend will disable
> the already-gated clocks, potentially causing an enable count underflow.
>
> > +		return err;
> > +	}
> >
> >  	nxp_fspi_prepare_lut(f, op);
>
> [ ... ]
>
> For pre-existing issues not directly modified by this patch, there are a few
> other observations in this file:
>
> [Severity: High]
> This is a pre-existing issue, but does nxp_fspi_exec_op() leak the PM
> runtime usage counter if pm_runtime_get_sync() fails?
>
> drivers/spi/spi-nxp-fspi.c:nxp_fspi_exec_op() {
>     ...
>     err = pm_runtime_get_sync(f->dev);
>     if (err < 0) {
>         dev_err(f->dev, "Failed to enable clock %d\n", __LINE__);
>         return err;
>     }
>     ...
> }
>
> If pm_runtime_get_sync() returns a negative error code, the usage counter
> remains incremented. Returning immediately without calling
> pm_runtime_put_noidle() permanently prevents the device from runtime
> suspending again.
>
> [Severity: High]
> This is a pre-existing issue, but are initialization and clock setup failures
> being masked during probe and resume?
>
> drivers/spi/spi-nxp-fspi.c:nxp_fspi_probe() {
>     ...
>     nxp_fspi_default_setup(f);
>     ...
> }
>
> The return value of nxp_fspi_default_setup() is completely ignored in both
> nxp_fspi_probe() and nxp_fspi_runtime_resume(). If clk_set_rate() or clock
> enabling fails inside setup, the driver proceeds as if the hardware is
> fully initialized, which could lead to silent failures.
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260728-fspi-clock-v2-0-dbe786a4a6eb@nxp.com?part=3