[PATCH] spi: fsl-qspi: Reprogram the clock rate when the operation frequency changes

Frieder Schrempf posted 1 patch 1 week ago
drivers/spi/spi-fsl-qspi.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
[PATCH] spi: fsl-qspi: Reprogram the clock rate when the operation frequency changes
Posted by Frieder Schrempf 1 week ago
From: Frieder Schrempf <frieder.schrempf@kontron.de>

fsl_qspi_select_mem() returns early when the chip select has not changed,
which happens before it reaches clk_set_rate(). Since the rate is now
taken from the spi-mem operation rather than from the SPI device, the
controller honours op->max_freq exactly once per chip select and ignores
it for every operation after that.

q->selected is only reset to -1 in fsl_qspi_default_setup(), i.e. at probe
and on resume, so on the common single chip select board the very first
operation latches a rate that all subsequent operations inherit, whatever
frequency they asked for.

This results in operations being issued with the wrong frequency.

Cache the operation frequency the clock was programmed for next to the
selected chip select, and redo the clock setup when either changes.

Fixes: 2438db5253eb ("spi: fsl-qspi: Support per spi-mem operation frequency switches")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-5
Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
---
The bug is visible on an i.MX6UL board with a Winbond SPI NAND and
spi-max-frequency = <104000000>: the QSPI clock keeps the rate programmed
for the first operation for the lifetime of the system, regardless of what
later operations ask for.

Here is a simple showcase with a SPI NAND where the max freq for the
read ops was artificially limited to 25 MHz.

cd /sys/kernel/debug/tracing
echo 1 > events/clk/clk_set_rate/enable
echo 1 > events/spi-mem/spi_mem_start_op/enable
echo > trace
dd if=/dev/mtd0 of=/dev/null bs=1k count=4

Without fix the clock is set to the high rate initially and never changed:

clk_set_rate: qspi1_podf 396000000
clk_set_rate: qspi1 396000000
[...]
spi_mem_start_op: 21e0000.spi 1S-1S-0S @104000000 Hz op=[13-00-00-00] len=0 tx=[]
spi_mem_start_op: 21e0000.spi 1S-1S-1S @104000000 Hz op=[0f-c0] len=1 tx=[]
spi_mem_start_op: 21e0000.spi 1S-4S-4S @25000000 Hz op=[eb-00-00-ff-ff] len=1024 tx=[]
spi_mem_start_op: 21e0000.spi 1S-4S-4S @25000000 Hz op=[eb-04-00-ff-ff] len=1024 tx=[]
spi_mem_start_op: 21e0000.spi 1S-1S-0S @104000000 Hz op=[13-00-00-00] len=0 tx=[]
spi_mem_start_op: 21e0000.spi 1S-1S-1S @104000000 Hz op=[0f-c0] len=1 tx=[]
spi_mem_start_op: 21e0000.spi 1S-4S-4S @25000000 Hz op=[eb-00-00-ff-ff] len=1024 tx=[]
spi_mem_start_op: 21e0000.spi 1S-4S-4S @25000000 Hz op=[eb-04-00-ff-ff] len=1024 tx=[]

With the fix the clock is actually changed according to what was
requested by the op:

spi_mem_start_op: 21e0000.spi 1S-1S-0S @104000000 Hz op=[13-00-00-00] len=0 tx=[]
clk_set_rate: qspi1_podf 396000000
clk_set_rate: qspi1 396000000
spi_mem_start_op: 21e0000.spi 1S-1S-1S @104000000 Hz op=[0f-c0] len=1 tx=[]
spi_mem_start_op: 21e0000.spi 1S-4S-4S @25000000 Hz op=[eb-00-00-ff-ff] len=1024 tx=[]
clk_set_rate: qspi1_podf 99000000
clk_set_rate: qspi1 99000000
spi_mem_start_op: 21e0000.spi 1S-4S-4S @25000000 Hz op=[eb-04-00-ff-ff] len=1024 tx=[]
spi_mem_start_op: 21e0000.spi 1S-1S-0S @104000000 Hz op=[13-00-00-00] len=0 tx=[]
clk_set_rate: qspi1_podf 396000000
clk_set_rate: qspi1 396000000
spi_mem_start_op: 21e0000.spi 1S-1S-1S @104000000 Hz op=[0f-c0] len=1 tx=[]
spi_mem_start_op: 21e0000.spi 1S-4S-4S @25000000 Hz op=[eb-00-00-ff-ff] len=1024 tx=[]
clk_set_rate: qspi1_podf 99000000
clk_set_rate: qspi1 99000000
spi_mem_start_op: 21e0000.spi 1S-4S-4S @25000000 Hz op=[eb-04-00-ff-ff] len=1024 tx=[]
---
 drivers/spi/spi-fsl-qspi.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi-fsl-qspi.c b/drivers/spi/spi-fsl-qspi.c
index 57358851029b..d2c2090442f8 100644
--- a/drivers/spi/spi-fsl-qspi.c
+++ b/drivers/spi/spi-fsl-qspi.c
@@ -289,6 +289,7 @@ struct fsl_qspi {
 	struct pm_qos_request pm_qos_req;
 	struct device *dev;
 	int selected;
+	u32 selected_freq;
 	u32 memmap_phy;
 };
 
@@ -551,7 +552,8 @@ static void fsl_qspi_select_mem(struct fsl_qspi *q, struct spi_device *spi,
 	unsigned long rate = op->max_freq;
 	int ret;
 
-	if (q->selected == spi_get_chipselect(spi, 0))
+	if (q->selected == spi_get_chipselect(spi, 0) &&
+	    q->selected_freq == op->max_freq)
 		return;
 
 	if (needs_4x_clock(q))
@@ -571,6 +573,7 @@ static void fsl_qspi_select_mem(struct fsl_qspi *q, struct spi_device *spi,
 	}
 
 	q->selected = spi_get_chipselect(spi, 0);
+	q->selected_freq = op->max_freq;
 
 	fsl_qspi_invalidate(q);
 }

---
base-commit: 238650ef6c7c7cca08e032527329424c9fbd70e5
change-id: 20260917-fsl-qspi-freq-op-fix-f38fedb13100

Best regards,
--  
Frieder Schrempf <frieder.schrempf@kontron.de>
Re: [PATCH] spi: fsl-qspi: Reprogram the clock rate when the operation frequency changes
Posted by Mark Brown 1 week ago
On Thu, 17 Sep 2026 16:10:15 +0200, Frieder Schrempf wrote:
> spi: fsl-qspi: Reprogram the clock rate when the operation frequency changes

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-7.3

Thanks!

[1/1] spi: fsl-qspi: Reprogram the clock rate when the operation frequency changes
      https://git.kernel.org/broonie/spi/c/3d743adf090c

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark
Re: [PATCH] spi: fsl-qspi: Reprogram the clock rate when the operation frequency changes
Posted by han.xu 1 week ago
On 26/09/17 04:10PM, Frieder Schrempf wrote:
> From: Frieder Schrempf <frieder.schrempf@kontron.de>
> 
> fsl_qspi_select_mem() returns early when the chip select has not changed,
> which happens before it reaches clk_set_rate(). Since the rate is now
> taken from the spi-mem operation rather than from the SPI device, the
> controller honours op->max_freq exactly once per chip select and ignores
> it for every operation after that.
> 
> q->selected is only reset to -1 in fsl_qspi_default_setup(), i.e. at probe
> and on resume, so on the common single chip select board the very first
> operation latches a rate that all subsequent operations inherit, whatever
> frequency they asked for.
> 
> This results in operations being issued with the wrong frequency.
> 
> Cache the operation frequency the clock was programmed for next to the
> selected chip select, and redo the clock setup when either changes.
> 
> Fixes: 2438db5253eb ("spi: fsl-qspi: Support per spi-mem operation frequency switches")
> Cc: stable@vger.kernel.org
> Assisted-by: Claude:claude-opus-5
> Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>

Acked-by: Han Xu <han.xu@nxp.com>

> ---
> The bug is visible on an i.MX6UL board with a Winbond SPI NAND and
> spi-max-frequency = <104000000>: the QSPI clock keeps the rate programmed
> for the first operation for the lifetime of the system, regardless of what
> later operations ask for.
> 
> Here is a simple showcase with a SPI NAND where the max freq for the
> read ops was artificially limited to 25 MHz.
> 
> cd /sys/kernel/debug/tracing
> echo 1 > events/clk/clk_set_rate/enable
> echo 1 > events/spi-mem/spi_mem_start_op/enable
> echo > trace
> dd if=/dev/mtd0 of=/dev/null bs=1k count=4
> 
> Without fix the clock is set to the high rate initially and never changed:
> 
> clk_set_rate: qspi1_podf 396000000
> clk_set_rate: qspi1 396000000
> [...]
> spi_mem_start_op: 21e0000.spi 1S-1S-0S @104000000 Hz op=[13-00-00-00] len=0 tx=[]
> spi_mem_start_op: 21e0000.spi 1S-1S-1S @104000000 Hz op=[0f-c0] len=1 tx=[]
> spi_mem_start_op: 21e0000.spi 1S-4S-4S @25000000 Hz op=[eb-00-00-ff-ff] len=1024 tx=[]
> spi_mem_start_op: 21e0000.spi 1S-4S-4S @25000000 Hz op=[eb-04-00-ff-ff] len=1024 tx=[]
> spi_mem_start_op: 21e0000.spi 1S-1S-0S @104000000 Hz op=[13-00-00-00] len=0 tx=[]
> spi_mem_start_op: 21e0000.spi 1S-1S-1S @104000000 Hz op=[0f-c0] len=1 tx=[]
> spi_mem_start_op: 21e0000.spi 1S-4S-4S @25000000 Hz op=[eb-00-00-ff-ff] len=1024 tx=[]
> spi_mem_start_op: 21e0000.spi 1S-4S-4S @25000000 Hz op=[eb-04-00-ff-ff] len=1024 tx=[]
> 
> With the fix the clock is actually changed according to what was
> requested by the op:
> 
> spi_mem_start_op: 21e0000.spi 1S-1S-0S @104000000 Hz op=[13-00-00-00] len=0 tx=[]
> clk_set_rate: qspi1_podf 396000000
> clk_set_rate: qspi1 396000000
> spi_mem_start_op: 21e0000.spi 1S-1S-1S @104000000 Hz op=[0f-c0] len=1 tx=[]
> spi_mem_start_op: 21e0000.spi 1S-4S-4S @25000000 Hz op=[eb-00-00-ff-ff] len=1024 tx=[]
> clk_set_rate: qspi1_podf 99000000
> clk_set_rate: qspi1 99000000
> spi_mem_start_op: 21e0000.spi 1S-4S-4S @25000000 Hz op=[eb-04-00-ff-ff] len=1024 tx=[]
> spi_mem_start_op: 21e0000.spi 1S-1S-0S @104000000 Hz op=[13-00-00-00] len=0 tx=[]
> clk_set_rate: qspi1_podf 396000000
> clk_set_rate: qspi1 396000000
> spi_mem_start_op: 21e0000.spi 1S-1S-1S @104000000 Hz op=[0f-c0] len=1 tx=[]
> spi_mem_start_op: 21e0000.spi 1S-4S-4S @25000000 Hz op=[eb-00-00-ff-ff] len=1024 tx=[]
> clk_set_rate: qspi1_podf 99000000
> clk_set_rate: qspi1 99000000
> spi_mem_start_op: 21e0000.spi 1S-4S-4S @25000000 Hz op=[eb-04-00-ff-ff] len=1024 tx=[]
> ---
>  drivers/spi/spi-fsl-qspi.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/spi/spi-fsl-qspi.c b/drivers/spi/spi-fsl-qspi.c
> index 57358851029b..d2c2090442f8 100644
> --- a/drivers/spi/spi-fsl-qspi.c
> +++ b/drivers/spi/spi-fsl-qspi.c
> @@ -289,6 +289,7 @@ struct fsl_qspi {
>  	struct pm_qos_request pm_qos_req;
>  	struct device *dev;
>  	int selected;
> +	u32 selected_freq;
>  	u32 memmap_phy;
>  };
>  
> @@ -551,7 +552,8 @@ static void fsl_qspi_select_mem(struct fsl_qspi *q, struct spi_device *spi,
>  	unsigned long rate = op->max_freq;
>  	int ret;
>  
> -	if (q->selected == spi_get_chipselect(spi, 0))
> +	if (q->selected == spi_get_chipselect(spi, 0) &&
> +	    q->selected_freq == op->max_freq)
>  		return;
>  
>  	if (needs_4x_clock(q))
> @@ -571,6 +573,7 @@ static void fsl_qspi_select_mem(struct fsl_qspi *q, struct spi_device *spi,
>  	}
>  
>  	q->selected = spi_get_chipselect(spi, 0);
> +	q->selected_freq = op->max_freq;
>  
>  	fsl_qspi_invalidate(q);
>  }
> 
> ---
> base-commit: 238650ef6c7c7cca08e032527329424c9fbd70e5
> change-id: 20260917-fsl-qspi-freq-op-fix-f38fedb13100
> 
> Best regards,
> --  
> Frieder Schrempf <frieder.schrempf@kontron.de>
>