From: Pratyush Yadav <pratyush@kernel.org>
Some controllers like the Cadence OSPI controller need to perform a
tuning sequence to operate at high data rates. Tuning is needs to happen
once the device is switched to appropriate mode (say 8S-8S-8S or
8D-8D-8D). Add a hook that spi-mem client devices can call in order to tune
the controller to operate in a given mode and data rate.
This is somewhat similar to eMMC/SD tuning for higher speed modes like
HS200, but there isn't a standard specification around the same though.
Signed-off-by: Pratyush Yadav <pratyush@kernel.org>
Signed-off-by: Santhosh Kumar K <s-k6@ti.com>
---
drivers/spi/spi-mem.c | 11 +++++++++++
include/linux/spi/spi-mem.h | 9 +++++++++
2 files changed, 20 insertions(+)
diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c
index 064b99204d9a..6c254291ee23 100644
--- a/drivers/spi/spi-mem.c
+++ b/drivers/spi/spi-mem.c
@@ -559,6 +559,17 @@ int spi_mem_adjust_op_size(struct spi_mem *mem, struct spi_mem_op *op)
}
EXPORT_SYMBOL_GPL(spi_mem_adjust_op_size);
+int spi_mem_execute_tuning(struct spi_mem *mem, const struct spi_mem_op *op)
+{
+ struct spi_controller *ctlr = mem->spi->controller;
+
+ if (!ctlr->mem_ops || !ctlr->mem_ops->execute_tuning)
+ return -EOPNOTSUPP;
+
+ return ctlr->mem_ops->execute_tuning(mem, op);
+}
+EXPORT_SYMBOL_GPL(spi_mem_execute_tuning);
+
/**
* spi_mem_adjust_op_freq() - Adjust the frequency of a SPI mem operation to
* match controller, PCB and chip limitations
diff --git a/include/linux/spi/spi-mem.h b/include/linux/spi/spi-mem.h
index 82390712794c..639fee61251c 100644
--- a/include/linux/spi/spi-mem.h
+++ b/include/linux/spi/spi-mem.h
@@ -314,6 +314,12 @@ static inline void *spi_mem_get_drvdata(struct spi_mem *mem)
* @poll_status: poll memory device status until (status & mask) == match or
* when the timeout has expired. It fills the data buffer with
* the last status value.
+ * @execute_tuning: perform PHY tuning to achieve high speed SPI operations.
+ * Should be called after the SPI memory device has been
+ * completely initialized. The op passed should contain a
+ * template for the read operation used for the device so the
+ * controller can decide what type of tuning is required
+ * for the type of read op passed.
*
* This interface should be implemented by SPI controllers providing an
* high-level interface to execute SPI memory operation, which is usually the
@@ -344,6 +350,8 @@ struct spi_controller_mem_ops {
unsigned long initial_delay_us,
unsigned long polling_rate_us,
unsigned long timeout_ms);
+ int (*execute_tuning)(struct spi_mem *mem,
+ const struct spi_mem_op *op);
};
/**
@@ -423,6 +431,7 @@ bool spi_mem_default_supports_op(struct spi_mem *mem,
#endif /* CONFIG_SPI_MEM */
int spi_mem_adjust_op_size(struct spi_mem *mem, struct spi_mem_op *op);
+int spi_mem_execute_tuning(struct spi_mem *mem, const struct spi_mem_op *op);
void spi_mem_adjust_op_freq(struct spi_mem *mem, struct spi_mem_op *op);
u64 spi_mem_calc_op_duration(struct spi_mem *mem, struct spi_mem_op *op);
--
2.34.1On 12/08/2025 at 01:02:10 +0530, Santhosh Kumar K <s-k6@ti.com> wrote: > From: Pratyush Yadav <pratyush@kernel.org> > > Some controllers like the Cadence OSPI controller need to perform a > tuning sequence to operate at high data rates. Tuning is needs to happen > once the device is switched to appropriate mode (say 8S-8S-8S or > 8D-8D-8D). This is actually wrong. Tuning is way more generic than that :) If someone wants to use a chip at a high frequency (50MHz in your case, but whatever, there is a threshold above which additional care must be taken), it must go through the calibration step. It does not matter in which mode you are. Calibration would still be relevant in single SDR mode. This 50MHz bothered Mark because it is too Cadence specific. Maybe this should be a controller parameter? If the spi-mem core (or even the spi core, by extensino) sees that the design allows running at XMHz (due to the SPI peripheral properties or simply the absence of any limitation), and if the controller states that it requires an extra tuning step above YMHz (and X > Y), then it launches the calibration. From a core perspective, I would like the calibration hook to be as simple as possible, because what "calibration" means is highly controller and chip specific. The Cadence SPI controller driver could request the pattern through the nvmem interface or maybe we can even include it in the kernel through some type of firmware interface (it could be stored anywhere) and if it gets it, it writes it to the device cache. Once done, it takes the fastest available read operation available for the chip and performs its calibration. The calibration hook no longer needs anything SPI driver specific. I don't know if still requires anything chip specific though (like the optimal read operation), but can you please try implementing that and then we'll discuss this further. Thanks, Miquèl
On Tue, Aug 12, 2025 at 01:02:10AM +0530, Santhosh Kumar K wrote: > From: Pratyush Yadav <pratyush@kernel.org> > > Some controllers like the Cadence OSPI controller need to perform a > tuning sequence to operate at high data rates. Tuning is needs to happen > once the device is switched to appropriate mode (say 8S-8S-8S or > 8D-8D-8D). Add a hook that spi-mem client devices can call in order to tune > the controller to operate in a given mode and data rate. > > This is somewhat similar to eMMC/SD tuning for higher speed modes like > HS200, but there isn't a standard specification around the same though. Should we have something that blocks these tuning required modes without the appropriate tuning, and/or allows discovery of which modes require this tuning? This all feels very landmineish - client drivers just have to know when tuning is required.
Hello, On 13/08/2025 at 21:26:06 +01, Mark Brown <broonie@kernel.org> wrote: > On Tue, Aug 12, 2025 at 01:02:10AM +0530, Santhosh Kumar K wrote: >> From: Pratyush Yadav <pratyush@kernel.org> >> >> Some controllers like the Cadence OSPI controller need to perform a >> tuning sequence to operate at high data rates. Tuning is needs to happen >> once the device is switched to appropriate mode (say 8S-8S-8S or >> 8D-8D-8D). Add a hook that spi-mem client devices can call in order to tune >> the controller to operate in a given mode and data rate. >> >> This is somewhat similar to eMMC/SD tuning for higher speed modes like >> HS200, but there isn't a standard specification around the same though. > > Should we have something that blocks these tuning required modes without > the appropriate tuning, and/or allows discovery of which modes require > this tuning? This all feels very landmineish - client drivers just have > to know when tuning is required. The maximum bus frequency will tell whether tuning is relevant or not I guess. In the case of the Cadence controller, the bus speed is key to determine whether calibration should happen or not because when PHY calibration is enabled, the SPI bus frequency is equal to the controller clock rate (pre-scalers are bypassed). So the criteria for enabling calibration is: max SPI bus freq >= min controller clock rate Thanks, Miquèl
Hello Mark, On 14/08/25 01:56, Mark Brown wrote: > On Tue, Aug 12, 2025 at 01:02:10AM +0530, Santhosh Kumar K wrote: >> From: Pratyush Yadav <pratyush@kernel.org> >> >> Some controllers like the Cadence OSPI controller need to perform a >> tuning sequence to operate at high data rates. Tuning is needs to happen >> once the device is switched to appropriate mode (say 8S-8S-8S or >> 8D-8D-8D). Add a hook that spi-mem client devices can call in order to tune >> the controller to operate in a given mode and data rate. >> >> This is somewhat similar to eMMC/SD tuning for higher speed modes like >> HS200, but there isn't a standard specification around the same though. > > Should we have something that blocks these tuning required modes without > the appropriate tuning, and/or allows discovery of which modes require > this tuning? This all feels very landmineish - client drivers just have > to know when tuning is required. The flash's maximum operating frequency determines whether PHY tuning is required, as we need tuning in case of Cadence controller for frequencies over 50 MHz. And we do check for this condition - see Patch 07/10, cqspi_phy_op_eligible_sdr(), which currently verifies the flash frequency against 166 MHz. This logic can be improved by implementing both min and max frequency checks, will update in the following version. Thanks, Santhosh.
On Thu, Aug 14, 2025 at 05:04:33PM +0530, Santhosh Kumar K wrote: > On 14/08/25 01:56, Mark Brown wrote: > > Should we have something that blocks these tuning required modes without > > the appropriate tuning, and/or allows discovery of which modes require > > this tuning? This all feels very landmineish - client drivers just have > > to know when tuning is required. > The flash's maximum operating frequency determines whether PHY tuning is > required, as we need tuning in case of Cadence controller for frequencies > over 50 MHz. That's entirely specific to the Candence controller from the sounds of it, that makes it hard to write a client driver if you need to know exactly what the controller you're dealing with is and what it's requirements are. > And we do check for this condition - see Patch 07/10, > cqspi_phy_op_eligible_sdr(), which currently verifies the flash frequency > against 166 MHz. This logic can be improved by implementing both min and max > frequency checks, will update in the following version. I can't actually tell how that verifies if the tuning has been done appropriately TBH, at least not without more effort than I'd care to (and the tuning only gets added in patch 10?).
On 14/08/2025 at 13:34:38 +01, Mark Brown <broonie@kernel.org> wrote: > On Thu, Aug 14, 2025 at 05:04:33PM +0530, Santhosh Kumar K wrote: >> On 14/08/25 01:56, Mark Brown wrote: > >> > Should we have something that blocks these tuning required modes without >> > the appropriate tuning, and/or allows discovery of which modes require >> > this tuning? This all feels very landmineish - client drivers just have >> > to know when tuning is required. > >> The flash's maximum operating frequency determines whether PHY tuning is >> required, as we need tuning in case of Cadence controller for frequencies >> over 50 MHz. > > That's entirely specific to the Candence controller from the sounds of > it, that makes it hard to write a client driver if you need to know > exactly what the controller you're dealing with is and what it's > requirements are. > >> And we do check for this condition - see Patch 07/10, >> cqspi_phy_op_eligible_sdr(), which currently verifies the flash frequency >> against 166 MHz. This logic can be improved by implementing both min and max >> frequency checks, will update in the following version. > > I can't actually tell how that verifies if the tuning has been done > appropriately TBH, at least not without more effort than I'd care to > (and the tuning only gets added in patch 10?). Santhosh, do you need more inputs? Or can you send an updated version? I am still thinking about the interface on the spi-mem/spi-nand side, but please iterate so we can move forward. Thanks, Miquèl
On 14/08/25 18:04, Mark Brown wrote: > On Thu, Aug 14, 2025 at 05:04:33PM +0530, Santhosh Kumar K wrote: >> On 14/08/25 01:56, Mark Brown wrote: > >>> Should we have something that blocks these tuning required modes without >>> the appropriate tuning, and/or allows discovery of which modes require >>> this tuning? This all feels very landmineish - client drivers just have >>> to know when tuning is required. > >> The flash's maximum operating frequency determines whether PHY tuning is >> required, as we need tuning in case of Cadence controller for frequencies >> over 50 MHz. > > That's entirely specific to the Candence controller from the sounds of > it, that makes it hard to write a client driver if you need to know > exactly what the controller you're dealing with is and what it's > requirements are. PHY tuning is not very specific to the Cadence controller; this has been added for other controllers as well. [1] - [3] spi_mem simply verifies the execute_tuning hook within the controller's mem_ops and invokes it if it exists, and the tuning implementation is entirely controller-dependent - ranging from straightforward parameter configuration of PHY registers to advanced tuning algorithms such as the one implemented in this tuning series. Currently, spi_mem_execute_tuning() is called by default from flash. In the future, this could be improved by asking the controller if tuning is actually needed (considering different factors such as frequency), similar to *_get_tuning_params implementation. Let me know your opinion in this. The get_tuning_params and execute_tuning hooks in spi_mem can also be utilized by any non-MTD spi-mem users. > >> And we do check for this condition - see Patch 07/10, >> cqspi_phy_op_eligible_sdr(), which currently verifies the flash frequency >> against 166 MHz. This logic can be improved by implementing both min and max >> frequency checks, will update in the following version. > > I can't actually tell how that verifies if the tuning has been done > appropriately TBH, at least not without more effort than I'd care to The *_execute_tuning function takes the read_op as an argument from flash, and considering flash continues to utilize the same read_op and frequency, it should make sure the tuning is appropriately completed. In the Cadence controller, the tuning process is validated by performing a read-back of a pre-defined tuning pattern using the read_op provided by flash. > (and the tuning only gets added in patch 10?). Patches 7 and 8 add PHY read/write support, and patch 9 adds tuning. These three patches could be squashed into one, but kept them separate to make it more granular for the reviewers. [1] https://lore.kernel.org/linux-spi/20220509175616.1089346-1-clg@kaod.org/ [2] https://lore.kernel.org/all/20230322090451.3179431-2-haibo.chen@nxp.com/ [3] https://lore.kernel.org/all/20241128174316.3209354-1-csokas.bence@prolan.hu/
© 2016 - 2025 Red Hat, Inc.