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

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 <[email protected]>

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/[email protected]?part=2