* [RFC PATCH v2 01/12] spi: dt-bindings: add spi-has-dqs property
2026-01-13 14:16 [RFC PATCH v2 00/12] spi: cadence-quadspi: add PHY tuning support Santhosh Kumar K
@ 2026-01-13 14:16 ` Santhosh Kumar K
2026-02-04 10:46 ` Miquel Raynal
2026-01-13 14:16 ` [RFC PATCH v2 02/12] spi: spi-mem: add controller tuning support Santhosh Kumar K
` (11 subsequent siblings)
12 siblings, 1 reply; 46+ messages in thread
From: Santhosh Kumar K @ 2026-01-13 14:16 UTC (permalink / raw)
To: broonie, robh, krzk+dt, conor+dt, miquel.raynal, richard,
vigneshr, tudor.ambarus, pratyush, mwalle
Cc: linux-spi, devicetree, linux-kernel, linux-mtd, praneeth,
u-kumar1, p-mantena, a-dutta, s-k6
Add the optional 'spi-has-dqs' boolean property for SPI flash device
subnodes. This property indicates the flash device supports DQS (Data
Strobe) mode, which provides improved timing margins for data capture
in high-speed SPI operations.
Signed-off-by: Santhosh Kumar K <s-k6@ti.com>
---
.../devicetree/bindings/spi/spi-peripheral-props.yaml | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml b/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
index 8b6e8fc009db..c6f330fd32aa 100644
--- a/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
+++ b/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
@@ -89,6 +89,12 @@ properties:
description:
Delay, in microseconds, after a write transfer.
+ spi-has-dqs:
+ description:
+ Indicates the SPI flash device supports DQS (Data Strobe) mode for
+ improved data capture timing.
+ $ref: /schemas/types.yaml#/definitions/flag
+
stacked-memories:
description: Several SPI memories can be wired in stacked mode.
This basically means that either a device features several chip
--
2.34.1
^ permalink raw reply related [flat|nested] 46+ messages in thread* Re: [RFC PATCH v2 01/12] spi: dt-bindings: add spi-has-dqs property
2026-01-13 14:16 ` [RFC PATCH v2 01/12] spi: dt-bindings: add spi-has-dqs property Santhosh Kumar K
@ 2026-02-04 10:46 ` Miquel Raynal
2026-02-05 17:46 ` Santhosh Kumar K
0 siblings, 1 reply; 46+ messages in thread
From: Miquel Raynal @ 2026-02-04 10:46 UTC (permalink / raw)
To: Santhosh Kumar K
Cc: broonie, robh, krzk+dt, conor+dt, richard, vigneshr,
tudor.ambarus, pratyush, mwalle, linux-spi, devicetree,
linux-kernel, linux-mtd, praneeth, u-kumar1, p-mantena, a-dutta
On 13/01/2026 at 19:46:06 +0530, Santhosh Kumar K <s-k6@ti.com> wrote:
> Add the optional 'spi-has-dqs' boolean property for SPI flash device
> subnodes. This property indicates the flash device supports DQS (Data
> Strobe) mode, which provides improved timing margins for data capture
> in high-speed SPI operations.
>
> Signed-off-by: Santhosh Kumar K <s-k6@ti.com>
> ---
> .../devicetree/bindings/spi/spi-peripheral-props.yaml | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml b/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
> index 8b6e8fc009db..c6f330fd32aa 100644
> --- a/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
> +++ b/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
> @@ -89,6 +89,12 @@ properties:
> description:
> Delay, in microseconds, after a write transfer.
>
> + spi-has-dqs:
> + description:
> + Indicates the SPI flash device supports DQS (Data Strobe) mode for
> + improved data capture timing.
> + $ref: /schemas/types.yaml#/definitions/flag
This information is currently lacking indeed, mostly because nobody ever
cared about it. The DT property is IMO not the correct way to give this
information for two reasons:
- this is a capability of the chip, we discover the chip dynamically in
both cases (NAND and NOR) and attach many capabilities to the chips
already, so I believe this information should be provided through a
flag.
- the fact that the DQS signal might be supported does not indicate it
is actually driven. Winbond chips, for instance, can either enable it
or not depending on their configuration (probably through their VCR
register, I need to check again).
The question I have is: shall we enable the DQS pin automatically if it
is available? Not all controllers support it I suppose, and wiring the
line might as well not be done (or incorrectly). For these cases we may
need DT properties in the future. But for the DQS presence indication, I
bet it is not useful, and should be handled at the core level (not
parsed by the driver like you do) because it may have an impact on the
chip internal configuration.
I will try to come up with a proposal soon!
Cheers,
Miquèl
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [RFC PATCH v2 01/12] spi: dt-bindings: add spi-has-dqs property
2026-02-04 10:46 ` Miquel Raynal
@ 2026-02-05 17:46 ` Santhosh Kumar K
2026-02-05 18:06 ` Miquel Raynal
0 siblings, 1 reply; 46+ messages in thread
From: Santhosh Kumar K @ 2026-02-05 17:46 UTC (permalink / raw)
To: Miquel Raynal
Cc: broonie, robh, krzk+dt, conor+dt, richard, vigneshr,
tudor.ambarus, pratyush, mwalle, linux-spi, devicetree,
linux-kernel, linux-mtd, praneeth, u-kumar1, p-mantena, a-dutta,
s-k6
Hello Miquel,
On 04/02/26 16:16, Miquel Raynal wrote:
> On 13/01/2026 at 19:46:06 +0530, Santhosh Kumar K <s-k6@ti.com> wrote:
>
>> Add the optional 'spi-has-dqs' boolean property for SPI flash device
>> subnodes. This property indicates the flash device supports DQS (Data
>> Strobe) mode, which provides improved timing margins for data capture
>> in high-speed SPI operations.
>>
>> Signed-off-by: Santhosh Kumar K <s-k6@ti.com>
>> ---
>> .../devicetree/bindings/spi/spi-peripheral-props.yaml | 6 ++++++
>> 1 file changed, 6 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml b/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
>> index 8b6e8fc009db..c6f330fd32aa 100644
>> --- a/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
>> +++ b/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
>> @@ -89,6 +89,12 @@ properties:
>> description:
>> Delay, in microseconds, after a write transfer.
>>
>> + spi-has-dqs:
>> + description:
>> + Indicates the SPI flash device supports DQS (Data Strobe) mode for
>> + improved data capture timing.
>> + $ref: /schemas/types.yaml#/definitions/flag
>
> This information is currently lacking indeed, mostly because nobody ever
> cared about it. The DT property is IMO not the correct way to give this
> information for two reasons:
> - this is a capability of the chip, we discover the chip dynamically in
> both cases (NAND and NOR) and attach many capabilities to the chips
> already, so I believe this information should be provided through a
> flag.
> - the fact that the DQS signal might be supported does not indicate it
> is actually driven. Winbond chips, for instance, can either enable it
> or not depending on their configuration (probably through their VCR
> register, I need to check again).
I agree. The flash device's capability to provide DQS - whether in SDR,
DDR or both modes - can be represented as a flag in the flash
description. We can list out the possible combinations and use them to
clearly describe the flash's supported capabilities.
However, whether the DQS signal is actually connected to the controller
is a non-discoverable hardware detail and should be described only via
Device Tree. The DT property is not meant to describe the flash's
capabilities, but to indicate whether DQS is physically connected to the
controller.
>
> The question I have is: shall we enable the DQS pin automatically if it
> is available? Not all controllers support it I suppose, and wiring the
> line might as well not be done (or incorrectly). For these cases we may
> need DT properties in the future. But for the DQS presence indication, I
> bet it is not useful, and should be handled at the core level (not
> parsed by the driver like you do) because it may have an impact on the
> chip internal configuration.
We can handle this in either way: keep DQS disabled by default and
enable it using a "has-dqs" property, or enable it by default and
disable it explicitly using a "no-dqs" property.
Thanks,
Santhosh.
>
> I will try to come up with a proposal soon!
>
> Cheers,
> Miquèl
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [RFC PATCH v2 01/12] spi: dt-bindings: add spi-has-dqs property
2026-02-05 17:46 ` Santhosh Kumar K
@ 2026-02-05 18:06 ` Miquel Raynal
0 siblings, 0 replies; 46+ messages in thread
From: Miquel Raynal @ 2026-02-05 18:06 UTC (permalink / raw)
To: Santhosh Kumar K
Cc: broonie, robh, krzk+dt, conor+dt, richard, vigneshr,
tudor.ambarus, pratyush, mwalle, linux-spi, devicetree,
linux-kernel, linux-mtd, praneeth, u-kumar1, p-mantena, a-dutta
Hi Santhosh,
>> This information is currently lacking indeed, mostly because nobody
>> ever
>> cared about it. The DT property is IMO not the correct way to give this
>> information for two reasons:
>> - this is a capability of the chip, we discover the chip dynamically in
>> both cases (NAND and NOR) and attach many capabilities to the chips
>> already, so I believe this information should be provided through a
>> flag.
>> - the fact that the DQS signal might be supported does not indicate it
>> is actually driven. Winbond chips, for instance, can either enable it
>> or not depending on their configuration (probably through their VCR
>> register, I need to check again).
>
> I agree. The flash device's capability to provide DQS - whether in SDR,
> DDR or both modes - can be represented as a flag in the flash
> description. We can list out the possible combinations and use them to
> clearly describe the flash's supported capabilities.
>
> However, whether the DQS signal is actually connected to the controller
> is a non-discoverable hardware detail and should be described only via
> Device Tree. The DT property is not meant to describe the flash's
> capabilities, but to indicate whether DQS is physically connected to the
> controller.
>
>> The question I have is: shall we enable the DQS pin automatically if
>> it
>> is available? Not all controllers support it I suppose, and wiring the
>> line might as well not be done (or incorrectly). For these cases we may
>> need DT properties in the future. But for the DQS presence indication, I
>> bet it is not useful, and should be handled at the core level (not
>> parsed by the driver like you do) because it may have an impact on the
>> chip internal configuration.
>
> We can handle this in either way: keep DQS disabled by default and
> enable it using a "has-dqs" property, or enable it by default and
> disable it explicitly using a "no-dqs" property.
I know we can be surprised by our hardware colleagues :-), but I would
consider "not wiring the DQS signal of a (DQS capable) octal DTR SPI
memory" a bug? Hence I would opt for handling the broken case, when/if
that need arises.
Thanks,
Miquèl
^ permalink raw reply [flat|nested] 46+ messages in thread
* [RFC PATCH v2 02/12] spi: spi-mem: add controller tuning support
2026-01-13 14:16 [RFC PATCH v2 00/12] spi: cadence-quadspi: add PHY tuning support Santhosh Kumar K
2026-01-13 14:16 ` [RFC PATCH v2 01/12] spi: dt-bindings: add spi-has-dqs property Santhosh Kumar K
@ 2026-01-13 14:16 ` Santhosh Kumar K
2026-01-13 14:16 ` [RFC PATCH v2 03/12] mtd: spinand: perform controller tuning during probe Santhosh Kumar K
` (10 subsequent siblings)
12 siblings, 0 replies; 46+ messages in thread
From: Santhosh Kumar K @ 2026-01-13 14:16 UTC (permalink / raw)
To: broonie, robh, krzk+dt, conor+dt, miquel.raynal, richard,
vigneshr, tudor.ambarus, pratyush, mwalle
Cc: linux-spi, devicetree, linux-kernel, linux-mtd, praneeth,
u-kumar1, p-mantena, a-dutta, s-k6
High-speed SPI memory controllers often require timing calibration
to operate reliably at maximum frequencies. Parameters such as
sampling points and delays need to be tuned based on the specific
hardware configuration and operating conditions.
Add spi_mem_execute_tuning() to allow SPI memory drivers to request
controller-specific tuning. The function takes a mandatory read
operation template and an optional write template, as most tuning
procedures are based on read operations and calls the corresponding
execute_tuning callback to spi_controller_mem_ops to allow controller
drivers to implement their tuning procedures.
Signed-off-by: Santhosh Kumar K <s-k6@ti.com>
---
drivers/spi/spi-mem.c | 34 ++++++++++++++++++++++++++++++++++
include/linux/spi/spi-mem.h | 5 +++++
2 files changed, 39 insertions(+)
diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c
index c8b2add2640e..2d34469323ea 100644
--- a/drivers/spi/spi-mem.c
+++ b/drivers/spi/spi-mem.c
@@ -633,6 +633,40 @@ u64 spi_mem_calc_op_duration(struct spi_mem *mem, struct spi_mem_op *op)
}
EXPORT_SYMBOL_GPL(spi_mem_calc_op_duration);
+/**
+ * spi_mem_execute_tuning() - Execute controller tuning procedure
+ * @mem: the SPI memory device
+ * @read_op: read operation template (mandatory)
+ * @write_op: write operation template (optional, may be NULL)
+ *
+ * Requests the controller to perform tuning to optimize timing parameters
+ * for high-speed operation. Controllers use the provided operation templates
+ * to construct their tuning sequences.
+ *
+ * Return: 0 on success, -EINVAL if @mem or @read_op is NULL,
+ * -EOPNOTSUPP if controller doesn't support tuning,
+ * or a controller-specific error code on failure.
+ */
+int spi_mem_execute_tuning(struct spi_mem *mem, struct spi_mem_op *read_op,
+ struct spi_mem_op *write_op)
+{
+ struct spi_controller *ctlr;
+
+ if (!mem || !read_op)
+ return -EINVAL;
+
+ ctlr = mem->spi->controller;
+ if (!ctlr->mem_ops || !ctlr->mem_ops->execute_tuning)
+ return -EOPNOTSUPP;
+
+ spi_mem_adjust_op_freq(mem, read_op);
+ if (write_op)
+ spi_mem_adjust_op_freq(mem, write_op);
+
+ return ctlr->mem_ops->execute_tuning(mem, read_op, write_op);
+}
+EXPORT_SYMBOL_GPL(spi_mem_execute_tuning);
+
static ssize_t spi_mem_no_dirmap_read(struct spi_mem_dirmap_desc *desc,
u64 offs, size_t len, void *buf)
{
diff --git a/include/linux/spi/spi-mem.h b/include/linux/spi/spi-mem.h
index 82390712794c..871e46297517 100644
--- a/include/linux/spi/spi-mem.h
+++ b/include/linux/spi/spi-mem.h
@@ -344,6 +344,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, struct spi_mem_op *read_op,
+ struct spi_mem_op *write_op);
};
/**
@@ -426,6 +428,9 @@ int spi_mem_adjust_op_size(struct spi_mem *mem, 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);
+int spi_mem_execute_tuning(struct spi_mem *mem, struct spi_mem_op *read_op,
+ struct spi_mem_op *write_op);
+
bool spi_mem_supports_op(struct spi_mem *mem,
const struct spi_mem_op *op);
--
2.34.1
^ permalink raw reply related [flat|nested] 46+ messages in thread* [RFC PATCH v2 03/12] mtd: spinand: perform controller tuning during probe
2026-01-13 14:16 [RFC PATCH v2 00/12] spi: cadence-quadspi: add PHY tuning support Santhosh Kumar K
2026-01-13 14:16 ` [RFC PATCH v2 01/12] spi: dt-bindings: add spi-has-dqs property Santhosh Kumar K
2026-01-13 14:16 ` [RFC PATCH v2 02/12] spi: spi-mem: add controller tuning support Santhosh Kumar K
@ 2026-01-13 14:16 ` Santhosh Kumar K
2026-02-05 17:35 ` Miquel Raynal
2026-01-13 14:16 ` [RFC PATCH v2 04/12] mtd: spi-nor: extract read operation setup into helper Santhosh Kumar K
` (9 subsequent siblings)
12 siblings, 1 reply; 46+ messages in thread
From: Santhosh Kumar K @ 2026-01-13 14:16 UTC (permalink / raw)
To: broonie, robh, krzk+dt, conor+dt, miquel.raynal, richard,
vigneshr, tudor.ambarus, pratyush, mwalle
Cc: linux-spi, devicetree, linux-kernel, linux-mtd, praneeth,
u-kumar1, p-mantena, a-dutta, s-k6
SPI controllers may need tuning for reliable high-speed operation.
Without it, controllers use conservative timing that limits performance.
Call spi_mem_execute_tuning() during probe to optimize timing for
the device's read and write operations. Failures are non-fatal as
controllers fall back to default timing.
Signed-off-by: Santhosh Kumar K <s-k6@ti.com>
---
drivers/mtd/nand/spi/core.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
index 0346916b032b..2a45d1047736 100644
--- a/drivers/mtd/nand/spi/core.c
+++ b/drivers/mtd/nand/spi/core.c
@@ -1658,6 +1658,7 @@ static int spinand_probe(struct spi_mem *mem)
{
struct spinand_device *spinand;
struct mtd_info *mtd;
+ struct spi_mem_op read_op, write_op;
int ret;
spinand = devm_kzalloc(&mem->spi->dev, sizeof(*spinand),
@@ -1676,6 +1677,19 @@ static int spinand_probe(struct spi_mem *mem)
if (ret)
return ret;
+ read_op = *spinand->op_templates.read_cache;
+ write_op = *spinand->op_templates.write_cache;
+
+ ret = spi_mem_execute_tuning(mem, &read_op, &write_op);
+ if (ret && ret != -EOPNOTSUPP) {
+ dev_warn(&mem->spi->dev, "Failed to execute PHY tuning: %d\n",
+ ret);
+ /*
+ * Tuning failure is non-fatal; the controller falls back to
+ * default timing, reducing speed but ensuring operation.
+ */
+ }
+
ret = mtd_device_register(mtd, NULL, 0);
if (ret)
goto err_spinand_cleanup;
--
2.34.1
^ permalink raw reply related [flat|nested] 46+ messages in thread* Re: [RFC PATCH v2 03/12] mtd: spinand: perform controller tuning during probe
2026-01-13 14:16 ` [RFC PATCH v2 03/12] mtd: spinand: perform controller tuning during probe Santhosh Kumar K
@ 2026-02-05 17:35 ` Miquel Raynal
2026-02-06 19:23 ` Santhosh Kumar K
0 siblings, 1 reply; 46+ messages in thread
From: Miquel Raynal @ 2026-02-05 17:35 UTC (permalink / raw)
To: Santhosh Kumar K
Cc: broonie, robh, krzk+dt, conor+dt, richard, vigneshr,
tudor.ambarus, pratyush, mwalle, linux-spi, devicetree,
linux-kernel, linux-mtd, praneeth, u-kumar1, p-mantena, a-dutta
Hi Santhosh,
On 13/01/2026 at 19:46:08 +0530, Santhosh Kumar K <s-k6@ti.com> wrote:
> SPI controllers may need tuning for reliable high-speed operation.
> Without it, controllers use conservative timing that limits performance.
>
> Call spi_mem_execute_tuning() during probe to optimize timing for
> the device's read and write operations. Failures are non-fatal as
> controllers fall back to default timing.
>
> Signed-off-by: Santhosh Kumar K <s-k6@ti.com>
> ---
> drivers/mtd/nand/spi/core.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
> index 0346916b032b..2a45d1047736 100644
> --- a/drivers/mtd/nand/spi/core.c
> +++ b/drivers/mtd/nand/spi/core.c
> @@ -1658,6 +1658,7 @@ static int spinand_probe(struct spi_mem *mem)
> {
> struct spinand_device *spinand;
> struct mtd_info *mtd;
> + struct spi_mem_op read_op, write_op;
> int ret;
>
> spinand = devm_kzalloc(&mem->spi->dev, sizeof(*spinand),
> @@ -1676,6 +1677,19 @@ static int spinand_probe(struct spi_mem *mem)
> if (ret)
> return ret;
>
> + read_op = *spinand->op_templates.read_cache;
> + write_op = *spinand->op_templates.write_cache;
These will become:
+ read_op = *spinand->op_templates->read_cache;
+ write_op = *spinand->op_templates->write_cache;
after rebasing on nand/next or next -rc1.
> +
> + ret = spi_mem_execute_tuning(mem, &read_op, &write_op);
> + if (ret && ret != -EOPNOTSUPP) {
> + dev_warn(&mem->spi->dev, "Failed to execute PHY tuning: %d\n",
> + ret);
> + /*
> + * Tuning failure is non-fatal; the controller falls back to
> + * default timing, reducing speed but ensuring operation.
> + */
> + }
Can we move the comment before spi_mem_execute_timing()? Then no more
brackets needed and it will be easier to read (imo).
LGTM otherwise.
Thanks,
Miquèl
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [RFC PATCH v2 03/12] mtd: spinand: perform controller tuning during probe
2026-02-05 17:35 ` Miquel Raynal
@ 2026-02-06 19:23 ` Santhosh Kumar K
0 siblings, 0 replies; 46+ messages in thread
From: Santhosh Kumar K @ 2026-02-06 19:23 UTC (permalink / raw)
To: Miquel Raynal
Cc: broonie, robh, krzk+dt, conor+dt, richard, vigneshr,
tudor.ambarus, pratyush, mwalle, linux-spi, devicetree,
linux-kernel, linux-mtd, praneeth, u-kumar1, p-mantena, a-dutta,
s-k6
Hi Miquel,
On 05/02/26 23:05, Miquel Raynal wrote:
> Hi Santhosh,
>
> On 13/01/2026 at 19:46:08 +0530, Santhosh Kumar K <s-k6@ti.com> wrote:
>
>> SPI controllers may need tuning for reliable high-speed operation.
>> Without it, controllers use conservative timing that limits performance.
>>
>> Call spi_mem_execute_tuning() during probe to optimize timing for
>> the device's read and write operations. Failures are non-fatal as
>> controllers fall back to default timing.
>>
>> Signed-off-by: Santhosh Kumar K <s-k6@ti.com>
>> ---
>> drivers/mtd/nand/spi/core.c | 14 ++++++++++++++
>> 1 file changed, 14 insertions(+)
>>
>> diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
>> index 0346916b032b..2a45d1047736 100644
>> --- a/drivers/mtd/nand/spi/core.c
>> +++ b/drivers/mtd/nand/spi/core.c
>> @@ -1658,6 +1658,7 @@ static int spinand_probe(struct spi_mem *mem)
>> {
>> struct spinand_device *spinand;
>> struct mtd_info *mtd;
>> + struct spi_mem_op read_op, write_op;
>> int ret;
>>
>> spinand = devm_kzalloc(&mem->spi->dev, sizeof(*spinand),
>> @@ -1676,6 +1677,19 @@ static int spinand_probe(struct spi_mem *mem)
>> if (ret)
>> return ret;
>>
>> + read_op = *spinand->op_templates.read_cache;
>> + write_op = *spinand->op_templates.write_cache;
>
> These will become:
>
> + read_op = *spinand->op_templates->read_cache;
> + write_op = *spinand->op_templates->write_cache;
>
> after rebasing on nand/next or next -rc1.
Noted!
>
>> +
>> + ret = spi_mem_execute_tuning(mem, &read_op, &write_op);
>> + if (ret && ret != -EOPNOTSUPP) {
>> + dev_warn(&mem->spi->dev, "Failed to execute PHY tuning: %d\n",
>> + ret);
>> + /*
>> + * Tuning failure is non-fatal; the controller falls back to
>> + * default timing, reducing speed but ensuring operation.
>> + */
>> + }
>
> Can we move the comment before spi_mem_execute_timing()? Then no more
> brackets needed and it will be easier to read (imo).
Yeah, I agree on the readability concern - will move them in v3.
Thanks,
Santhosh.
>
> LGTM otherwise.
>
> Thanks,
> Miquèl
^ permalink raw reply [flat|nested] 46+ messages in thread
* [RFC PATCH v2 04/12] mtd: spi-nor: extract read operation setup into helper
2026-01-13 14:16 [RFC PATCH v2 00/12] spi: cadence-quadspi: add PHY tuning support Santhosh Kumar K
` (2 preceding siblings ...)
2026-01-13 14:16 ` [RFC PATCH v2 03/12] mtd: spinand: perform controller tuning during probe Santhosh Kumar K
@ 2026-01-13 14:16 ` Santhosh Kumar K
2026-01-13 14:16 ` [RFC PATCH v2 05/12] mtd: spi-nor: perform controller tuning during probe Santhosh Kumar K
` (8 subsequent siblings)
12 siblings, 0 replies; 46+ messages in thread
From: Santhosh Kumar K @ 2026-01-13 14:16 UTC (permalink / raw)
To: broonie, robh, krzk+dt, conor+dt, miquel.raynal, richard,
vigneshr, tudor.ambarus, pratyush, mwalle
Cc: linux-spi, devicetree, linux-kernel, linux-mtd, praneeth,
u-kumar1, p-mantena, a-dutta, s-k6
From: Pratyush Yadav <pratyush@kernel.org>
spi_nor_spimem_read_data() and spi_nor_create_read_dirmap() duplicate
the logic for constructing read operations: both create a spi_mem_op,
call spi_nor_spimem_setup_op(), and convert dummy cycles to bytes.
Extract this into spi_nor_spimem_get_read_op() to eliminate duplication.
The helper returns a configured template that callers populate with
address, data length, and buffer.
Signed-off-by: Pratyush Yadav <pratyush@kernel.org>
Signed-off-by: Santhosh Kumar K <s-k6@ti.com>
---
drivers/mtd/spi-nor/core.c | 60 +++++++++++++++++++-------------------
1 file changed, 30 insertions(+), 30 deletions(-)
diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index d3f8a78efd3b..63f3051d7a6b 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -188,6 +188,31 @@ static int spi_nor_controller_ops_erase(struct spi_nor *nor, loff_t offs)
return nor->controller_ops->erase(nor, offs);
}
+/**
+ * spi_nor_spimem_get_read_op() - get spi_mem read operation template
+ * @nor: the spi-nor device
+ *
+ * Returns a read operation template with buswidths and dummy cycles
+ * configured. Caller must set address, data length, and data buffer.
+ */
+static struct spi_mem_op spi_nor_spimem_get_read_op(struct spi_nor *nor)
+{
+ struct spi_mem_op op =
+ SPI_MEM_OP(SPI_MEM_OP_CMD(nor->read_opcode, 0),
+ SPI_MEM_OP_ADDR(nor->addr_nbytes, 0, 0),
+ SPI_MEM_OP_DUMMY(nor->read_dummy, 0),
+ SPI_MEM_OP_DATA_IN(1, NULL, 0));
+
+ spi_nor_spimem_setup_op(nor, &op, nor->read_proto);
+
+ /* convert the dummy cycles to the number of bytes */
+ op.dummy.nbytes = (nor->read_dummy * op.dummy.buswidth) / 8;
+ if (spi_nor_protocol_is_dtr(nor->read_proto))
+ op.dummy.nbytes *= 2;
+
+ return op;
+}
+
/**
* spi_nor_spimem_read_data() - read data from flash's memory region via
* spi-mem
@@ -201,21 +226,14 @@ static int spi_nor_controller_ops_erase(struct spi_nor *nor, loff_t offs)
static ssize_t spi_nor_spimem_read_data(struct spi_nor *nor, loff_t from,
size_t len, u8 *buf)
{
- struct spi_mem_op op =
- SPI_MEM_OP(SPI_MEM_OP_CMD(nor->read_opcode, 0),
- SPI_MEM_OP_ADDR(nor->addr_nbytes, from, 0),
- SPI_MEM_OP_DUMMY(nor->read_dummy, 0),
- SPI_MEM_OP_DATA_IN(len, buf, 0));
+ struct spi_mem_op op = spi_nor_spimem_get_read_op(nor);
bool usebouncebuf;
ssize_t nbytes;
int error;
- spi_nor_spimem_setup_op(nor, &op, nor->read_proto);
-
- /* convert the dummy cycles to the number of bytes */
- op.dummy.nbytes = (nor->read_dummy * op.dummy.buswidth) / 8;
- if (spi_nor_protocol_is_dtr(nor->read_proto))
- op.dummy.nbytes *= 2;
+ op.addr.val = from;
+ op.data.nbytes = len;
+ op.data.buf.in = buf;
usebouncebuf = spi_nor_spimem_bounce(nor, &op);
@@ -3641,28 +3659,10 @@ EXPORT_SYMBOL_GPL(spi_nor_scan);
static int spi_nor_create_read_dirmap(struct spi_nor *nor)
{
struct spi_mem_dirmap_info info = {
- .op_tmpl = SPI_MEM_OP(SPI_MEM_OP_CMD(nor->read_opcode, 0),
- SPI_MEM_OP_ADDR(nor->addr_nbytes, 0, 0),
- SPI_MEM_OP_DUMMY(nor->read_dummy, 0),
- SPI_MEM_OP_DATA_IN(0, NULL, 0)),
+ .op_tmpl = spi_nor_spimem_get_read_op(nor),
.offset = 0,
.length = nor->params->size,
};
- struct spi_mem_op *op = &info.op_tmpl;
-
- spi_nor_spimem_setup_op(nor, op, nor->read_proto);
-
- /* convert the dummy cycles to the number of bytes */
- op->dummy.nbytes = (nor->read_dummy * op->dummy.buswidth) / 8;
- if (spi_nor_protocol_is_dtr(nor->read_proto))
- op->dummy.nbytes *= 2;
-
- /*
- * Since spi_nor_spimem_setup_op() only sets buswidth when the number
- * of data bytes is non-zero, the data buswidth won't be set here. So,
- * do it explicitly.
- */
- op->data.buswidth = spi_nor_get_protocol_data_nbits(nor->read_proto);
nor->dirmap.rdesc = devm_spi_mem_dirmap_create(nor->dev, nor->spimem,
&info);
--
2.34.1
^ permalink raw reply related [flat|nested] 46+ messages in thread* [RFC PATCH v2 05/12] mtd: spi-nor: perform controller tuning during probe
2026-01-13 14:16 [RFC PATCH v2 00/12] spi: cadence-quadspi: add PHY tuning support Santhosh Kumar K
` (3 preceding siblings ...)
2026-01-13 14:16 ` [RFC PATCH v2 04/12] mtd: spi-nor: extract read operation setup into helper Santhosh Kumar K
@ 2026-01-13 14:16 ` Santhosh Kumar K
2026-01-13 14:16 ` [RFC PATCH v2 06/12] spi: cadence-quadspi: move cqspi_readdata_capture earlier Santhosh Kumar K
` (7 subsequent siblings)
12 siblings, 0 replies; 46+ messages in thread
From: Santhosh Kumar K @ 2026-01-13 14:16 UTC (permalink / raw)
To: broonie, robh, krzk+dt, conor+dt, miquel.raynal, richard,
vigneshr, tudor.ambarus, pratyush, mwalle
Cc: linux-spi, devicetree, linux-kernel, linux-mtd, praneeth,
u-kumar1, p-mantena, a-dutta, s-k6
SPI controllers may need tuning for reliable high-speed operation.
Without it, controllers use conservative timing that limits performance.
Call spi_mem_execute_tuning() during probe to optimize timing for
the device's read operation. Failures are non-fatal as controllers
fall back to default timing.
Signed-off-by: Santhosh Kumar K <s-k6@ti.com>
---
drivers/mtd/spi-nor/core.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 63f3051d7a6b..d69cb8eaad83 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -3704,6 +3704,7 @@ static int spi_nor_probe(struct spi_mem *spimem)
struct device *dev = &spi->dev;
struct flash_platform_data *data = dev_get_platdata(dev);
struct spi_nor *nor;
+ struct spi_mem_op read_op;
/*
* Enable all caps by default. The core will mask them after
* checking what's really supported using spi_mem_supports_op().
@@ -3773,6 +3774,16 @@ static int spi_nor_probe(struct spi_mem *spimem)
if (ret)
return ret;
+ read_op = spi_nor_spimem_get_read_op(nor);
+ ret = spi_mem_execute_tuning(spimem, &read_op, NULL);
+ if (ret && ret != -EOPNOTSUPP) {
+ dev_warn(dev, "Failed to execute PHY tuning: %d\n", ret);
+ /*
+ * Tuning failure is non-fatal; the controller falls back to
+ * default timing, reducing speed but ensuring operation.
+ */
+ }
+
return mtd_device_register(&nor->mtd, data ? data->parts : NULL,
data ? data->nr_parts : 0);
}
--
2.34.1
^ permalink raw reply related [flat|nested] 46+ messages in thread* [RFC PATCH v2 06/12] spi: cadence-quadspi: move cqspi_readdata_capture earlier
2026-01-13 14:16 [RFC PATCH v2 00/12] spi: cadence-quadspi: add PHY tuning support Santhosh Kumar K
` (4 preceding siblings ...)
2026-01-13 14:16 ` [RFC PATCH v2 05/12] mtd: spi-nor: perform controller tuning during probe Santhosh Kumar K
@ 2026-01-13 14:16 ` Santhosh Kumar K
2026-02-05 17:35 ` Miquel Raynal
2026-01-13 14:16 ` [RFC PATCH v2 07/12] spi: cadence-quadspi: add DQS support to read data capture Santhosh Kumar K
` (6 subsequent siblings)
12 siblings, 1 reply; 46+ messages in thread
From: Santhosh Kumar K @ 2026-01-13 14:16 UTC (permalink / raw)
To: broonie, robh, krzk+dt, conor+dt, miquel.raynal, richard,
vigneshr, tudor.ambarus, pratyush, mwalle
Cc: linux-spi, devicetree, linux-kernel, linux-mtd, praneeth,
u-kumar1, p-mantena, a-dutta, s-k6
Move cqspi_readdata_capture() function earlier in the file. This is
preparatory refactoring for upcoming PHY tuning support for read and
write operations.
No functional changes.
Signed-off-by: Santhosh Kumar K <s-k6@ti.com>
---
drivers/spi/spi-cadence-quadspi.c | 45 +++++++++++++++----------------
1 file changed, 22 insertions(+), 23 deletions(-)
diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
index b1cf182d6566..303064fdfe2c 100644
--- a/drivers/spi/spi-cadence-quadspi.c
+++ b/drivers/spi/spi-cadence-quadspi.c
@@ -450,6 +450,28 @@ static int cqspi_wait_idle(struct cqspi_st *cqspi)
}
}
+static void cqspi_readdata_capture(struct cqspi_st *cqspi, const bool bypass,
+ const unsigned int delay)
+{
+ void __iomem *reg_base = cqspi->iobase;
+ unsigned int reg;
+
+ reg = readl(reg_base + CQSPI_REG_READCAPTURE);
+
+ if (bypass)
+ reg |= BIT(CQSPI_REG_READCAPTURE_BYPASS_LSB);
+ else
+ reg &= ~BIT(CQSPI_REG_READCAPTURE_BYPASS_LSB);
+
+ reg &= ~(CQSPI_REG_READCAPTURE_DELAY_MASK
+ << CQSPI_REG_READCAPTURE_DELAY_LSB);
+
+ reg |= (delay & CQSPI_REG_READCAPTURE_DELAY_MASK)
+ << CQSPI_REG_READCAPTURE_DELAY_LSB;
+
+ writel(reg, reg_base + CQSPI_REG_READCAPTURE);
+}
+
static int cqspi_exec_flash_cmd(struct cqspi_st *cqspi, unsigned int reg)
{
void __iomem *reg_base = cqspi->iobase;
@@ -1267,29 +1289,6 @@ static void cqspi_config_baudrate_div(struct cqspi_st *cqspi)
writel(reg, reg_base + CQSPI_REG_CONFIG);
}
-static void cqspi_readdata_capture(struct cqspi_st *cqspi,
- const bool bypass,
- const unsigned int delay)
-{
- void __iomem *reg_base = cqspi->iobase;
- unsigned int reg;
-
- reg = readl(reg_base + CQSPI_REG_READCAPTURE);
-
- if (bypass)
- reg |= BIT(CQSPI_REG_READCAPTURE_BYPASS_LSB);
- else
- reg &= ~BIT(CQSPI_REG_READCAPTURE_BYPASS_LSB);
-
- reg &= ~(CQSPI_REG_READCAPTURE_DELAY_MASK
- << CQSPI_REG_READCAPTURE_DELAY_LSB);
-
- reg |= (delay & CQSPI_REG_READCAPTURE_DELAY_MASK)
- << CQSPI_REG_READCAPTURE_DELAY_LSB;
-
- writel(reg, reg_base + CQSPI_REG_READCAPTURE);
-}
-
static void cqspi_configure(struct cqspi_flash_pdata *f_pdata,
unsigned long sclk)
{
--
2.34.1
^ permalink raw reply related [flat|nested] 46+ messages in thread* Re: [RFC PATCH v2 06/12] spi: cadence-quadspi: move cqspi_readdata_capture earlier
2026-01-13 14:16 ` [RFC PATCH v2 06/12] spi: cadence-quadspi: move cqspi_readdata_capture earlier Santhosh Kumar K
@ 2026-02-05 17:35 ` Miquel Raynal
0 siblings, 0 replies; 46+ messages in thread
From: Miquel Raynal @ 2026-02-05 17:35 UTC (permalink / raw)
To: Santhosh Kumar K
Cc: broonie, robh, krzk+dt, conor+dt, richard, vigneshr,
tudor.ambarus, pratyush, mwalle, linux-spi, devicetree,
linux-kernel, linux-mtd, praneeth, u-kumar1, p-mantena, a-dutta
On 13/01/2026 at 19:46:11 +0530, Santhosh Kumar K <s-k6@ti.com> wrote:
> Move cqspi_readdata_capture() function earlier in the file. This is
> preparatory refactoring for upcoming PHY tuning support for read and
> write operations.
>
> No functional changes.
>
> Signed-off-by: Santhosh Kumar K <s-k6@ti.com>
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
^ permalink raw reply [flat|nested] 46+ messages in thread
* [RFC PATCH v2 07/12] spi: cadence-quadspi: add DQS support to read data capture
2026-01-13 14:16 [RFC PATCH v2 00/12] spi: cadence-quadspi: add PHY tuning support Santhosh Kumar K
` (5 preceding siblings ...)
2026-01-13 14:16 ` [RFC PATCH v2 06/12] spi: cadence-quadspi: move cqspi_readdata_capture earlier Santhosh Kumar K
@ 2026-01-13 14:16 ` Santhosh Kumar K
2026-02-05 17:35 ` Miquel Raynal
2026-01-13 14:16 ` [RFC PATCH v2 08/12] spi: cadence-quadspi: read 'has-dqs' DT property Santhosh Kumar K
` (5 subsequent siblings)
12 siblings, 1 reply; 46+ messages in thread
From: Santhosh Kumar K @ 2026-01-13 14:16 UTC (permalink / raw)
To: broonie, robh, krzk+dt, conor+dt, miquel.raynal, richard,
vigneshr, tudor.ambarus, pratyush, mwalle
Cc: linux-spi, devicetree, linux-kernel, linux-mtd, praneeth,
u-kumar1, p-mantena, a-dutta, s-k6
Add 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.
Signed-off-by: Santhosh Kumar K <s-k6@ti.com>
---
drivers/spi/spi-cadence-quadspi.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
index 303064fdfe2c..1d708dde4463 100644
--- a/drivers/spi/spi-cadence-quadspi.c
+++ b/drivers/spi/spi-cadence-quadspi.c
@@ -186,6 +186,7 @@ struct cqspi_driver_platdata {
#define CQSPI_REG_READCAPTURE_BYPASS_LSB 0
#define CQSPI_REG_READCAPTURE_DELAY_LSB 1
#define CQSPI_REG_READCAPTURE_DELAY_MASK 0xF
+#define CQSPI_REG_READCAPTURE_DQS_LSB 8
#define CQSPI_REG_SIZE 0x14
#define CQSPI_REG_SIZE_ADDRESS_LSB 0
@@ -451,7 +452,7 @@ static int cqspi_wait_idle(struct cqspi_st *cqspi)
}
static void cqspi_readdata_capture(struct cqspi_st *cqspi, const bool bypass,
- const unsigned int delay)
+ const bool dqs, const unsigned int delay)
{
void __iomem *reg_base = cqspi->iobase;
unsigned int reg;
@@ -469,6 +470,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);
+
writel(reg, reg_base + CQSPI_REG_READCAPTURE);
}
@@ -1310,7 +1316,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);
}
--
2.34.1
^ permalink raw reply related [flat|nested] 46+ messages in thread* Re: [RFC PATCH v2 07/12] spi: cadence-quadspi: add DQS support to read data capture
2026-01-13 14:16 ` [RFC PATCH v2 07/12] spi: cadence-quadspi: add DQS support to read data capture Santhosh Kumar K
@ 2026-02-05 17:35 ` Miquel Raynal
0 siblings, 0 replies; 46+ messages in thread
From: Miquel Raynal @ 2026-02-05 17:35 UTC (permalink / raw)
To: Santhosh Kumar K
Cc: broonie, robh, krzk+dt, conor+dt, richard, vigneshr,
tudor.ambarus, pratyush, mwalle, linux-spi, devicetree,
linux-kernel, linux-mtd, praneeth, u-kumar1, p-mantena, a-dutta
On 13/01/2026 at 19:46:12 +0530, Santhosh Kumar K <s-k6@ti.com> wrote:
> Add 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.
>
> Signed-off-by: Santhosh Kumar K <s-k6@ti.com>
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
^ permalink raw reply [flat|nested] 46+ messages in thread
* [RFC PATCH v2 08/12] spi: cadence-quadspi: read 'has-dqs' DT property
2026-01-13 14:16 [RFC PATCH v2 00/12] spi: cadence-quadspi: add PHY tuning support Santhosh Kumar K
` (6 preceding siblings ...)
2026-01-13 14:16 ` [RFC PATCH v2 07/12] spi: cadence-quadspi: add DQS support to read data capture Santhosh Kumar K
@ 2026-01-13 14:16 ` Santhosh Kumar K
2026-02-05 17:35 ` Miquel Raynal
2026-01-13 14:16 ` [RFC PATCH v2 09/12] spi: cadence-quadspi: add PHY tuning infrastructure Santhosh Kumar K
` (4 subsequent siblings)
12 siblings, 1 reply; 46+ messages in thread
From: Santhosh Kumar K @ 2026-01-13 14:16 UTC (permalink / raw)
To: broonie, robh, krzk+dt, conor+dt, miquel.raynal, richard,
vigneshr, tudor.ambarus, pratyush, mwalle
Cc: linux-spi, devicetree, linux-kernel, linux-mtd, praneeth,
u-kumar1, p-mantena, a-dutta, s-k6
Add a boolean field to struct cqspi_flash_pdata to store whether the
attached flash device supports DQS (Data Strobe) mode. Read this from
the 'spi-has-dqs' device tree property during flash node parsing.
This is preparatory infrastructure for PHY tuning support. The field
will be used by subsequent patches to configure read data capture timing
with DQS enabled for improved margins in high-speed operations.
Signed-off-by: Santhosh Kumar K <s-k6@ti.com>
---
drivers/spi/spi-cadence-quadspi.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
index 1d708dde4463..0df286d24256 100644
--- a/drivers/spi/spi-cadence-quadspi.c
+++ b/drivers/spi/spi-cadence-quadspi.c
@@ -70,6 +70,7 @@ struct cqspi_flash_pdata {
u32 tsd2d_ns;
u32 tchsh_ns;
u32 tslch_ns;
+ bool has_dqs;
u8 cs;
};
@@ -1583,6 +1584,8 @@ static int cqspi_of_get_flash_pdata(struct platform_device *pdev,
return -ENXIO;
}
+ f_pdata->has_dqs = of_property_read_bool(np, "spi-has-dqs");
+
return 0;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 46+ messages in thread* Re: [RFC PATCH v2 08/12] spi: cadence-quadspi: read 'has-dqs' DT property
2026-01-13 14:16 ` [RFC PATCH v2 08/12] spi: cadence-quadspi: read 'has-dqs' DT property Santhosh Kumar K
@ 2026-02-05 17:35 ` Miquel Raynal
2026-02-19 12:14 ` Michael Walle
0 siblings, 1 reply; 46+ messages in thread
From: Miquel Raynal @ 2026-02-05 17:35 UTC (permalink / raw)
To: Santhosh Kumar K
Cc: broonie, robh, krzk+dt, conor+dt, richard, vigneshr,
tudor.ambarus, pratyush, mwalle, linux-spi, devicetree,
linux-kernel, linux-mtd, praneeth, u-kumar1, p-mantena, a-dutta
On 13/01/2026 at 19:46:13 +0530, Santhosh Kumar K <s-k6@ti.com> wrote:
> Add a boolean field to struct cqspi_flash_pdata to store whether the
> attached flash device supports DQS (Data Strobe) mode. Read this from
> the 'spi-has-dqs' device tree property during flash node parsing.
>
> This is preparatory infrastructure for PHY tuning support. The field
> will be used by subsequent patches to configure read data capture timing
> with DQS enabled for improved margins in high-speed operations.
>
> Signed-off-by: Santhosh Kumar K <s-k6@ti.com>
As mentioned in my answer to the cover letter, I am not too much in
favour of this property because this is something that is somewhat
related to the chip ID, thus discoverable. I drafted something to get
rid of this property already, I will share it for opening the
discussion.
However, for now I am closing my eyes on the fact that the DQS pin might
not be wired to the controller. In this case we will need some kind of
"dqs-not-wired" DT property, that's true, but also easily manageable at
the core level later, when/if the need arises.
Thanks,
Miquèl
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [RFC PATCH v2 08/12] spi: cadence-quadspi: read 'has-dqs' DT property
2026-02-05 17:35 ` Miquel Raynal
@ 2026-02-19 12:14 ` Michael Walle
2026-02-20 8:21 ` Miquel Raynal
0 siblings, 1 reply; 46+ messages in thread
From: Michael Walle @ 2026-02-19 12:14 UTC (permalink / raw)
To: Miquel Raynal, Santhosh Kumar K
Cc: broonie, robh, krzk+dt, conor+dt, richard, vigneshr,
tudor.ambarus, pratyush, linux-spi, devicetree, linux-kernel,
linux-mtd, praneeth, u-kumar1, p-mantena, a-dutta
[-- Attachment #1: Type: text/plain, Size: 1835 bytes --]
On Thu Feb 5, 2026 at 6:35 PM CET, Miquel Raynal wrote:
> On 13/01/2026 at 19:46:13 +0530, Santhosh Kumar K <s-k6@ti.com> wrote:
>
>> Add a boolean field to struct cqspi_flash_pdata to store whether the
>> attached flash device supports DQS (Data Strobe) mode. Read this from
>> the 'spi-has-dqs' device tree property during flash node parsing.
>>
>> This is preparatory infrastructure for PHY tuning support. The field
>> will be used by subsequent patches to configure read data capture timing
>> with DQS enabled for improved margins in high-speed operations.
>>
>> Signed-off-by: Santhosh Kumar K <s-k6@ti.com>
>
> As mentioned in my answer to the cover letter, I am not too much in
> favour of this property because this is something that is somewhat
> related to the chip ID, thus discoverable. I drafted something to get
> rid of this property already, I will share it for opening the
> discussion.
>
> However, for now I am closing my eyes on the fact that the DQS pin might
> not be wired to the controller. In this case we will need some kind of
> "dqs-not-wired" DT property, that's true, but also easily manageable at
> the core level later, when/if the need arises.
Usually, a DQS pin is optional. AFAIK, there is no requirement for
it. I.e. it will probably work fine with slower frequencies and
using an internal loopback. So if you run your flash with slower
frequency you can probably save one pin and use it for something
different.
What I wanted to say is, that not having a DQS pin wired is not
really a mistake. But "dqs-not-wired" sounds exactly like it.
So IMHO it should be the other way around and the device tree should
tell you that *is* wired, if we cannot find detect it otherwise,
like looking at pinmuxing for example (not sure that is feasible
though).
-michael
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 297 bytes --]
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [RFC PATCH v2 08/12] spi: cadence-quadspi: read 'has-dqs' DT property
2026-02-19 12:14 ` Michael Walle
@ 2026-02-20 8:21 ` Miquel Raynal
0 siblings, 0 replies; 46+ messages in thread
From: Miquel Raynal @ 2026-02-20 8:21 UTC (permalink / raw)
To: Michael Walle
Cc: Santhosh Kumar K, broonie, robh, krzk+dt, conor+dt, richard,
vigneshr, tudor.ambarus, pratyush, linux-spi, devicetree,
linux-kernel, linux-mtd, praneeth, u-kumar1, p-mantena, a-dutta
On 19/02/2026 at 13:14:39 +01, "Michael Walle" <mwalle@kernel.org> wrote:
> On Thu Feb 5, 2026 at 6:35 PM CET, Miquel Raynal wrote:
>> On 13/01/2026 at 19:46:13 +0530, Santhosh Kumar K <s-k6@ti.com> wrote:
>>
>>> Add a boolean field to struct cqspi_flash_pdata to store whether the
>>> attached flash device supports DQS (Data Strobe) mode. Read this from
>>> the 'spi-has-dqs' device tree property during flash node parsing.
>>>
>>> This is preparatory infrastructure for PHY tuning support. The field
>>> will be used by subsequent patches to configure read data capture timing
>>> with DQS enabled for improved margins in high-speed operations.
>>>
>>> Signed-off-by: Santhosh Kumar K <s-k6@ti.com>
>>
>> As mentioned in my answer to the cover letter, I am not too much in
>> favour of this property because this is something that is somewhat
>> related to the chip ID, thus discoverable. I drafted something to get
>> rid of this property already, I will share it for opening the
>> discussion.
>>
>> However, for now I am closing my eyes on the fact that the DQS pin might
>> not be wired to the controller. In this case we will need some kind of
>> "dqs-not-wired" DT property, that's true, but also easily manageable at
>> the core level later, when/if the need arises.
>
> Usually, a DQS pin is optional. AFAIK, there is no requirement for
> it. I.e. it will probably work fine with slower frequencies and
> using an internal loopback. So if you run your flash with slower
> frequency you can probably save one pin and use it for something
> different.
>
> What I wanted to say is, that not having a DQS pin wired is not
> really a mistake. But "dqs-not-wired" sounds exactly like it.
> So IMHO it should be the other way around and the device tree should
> tell you that *is* wired, if we cannot find detect it otherwise,
> like looking at pinmuxing for example (not sure that is feasible
> though).
I was in favour of the opt-out property because I feel like it doesn't
make sense to fill in the high speed spi property which involves tuning,
without wiring the DQS pin. It is possible, but if you're looking for
speed, it doesn't make much sense IMO. Hence I was seeing this as a
specific case which legitimately needs a property. But I don't want to
bikeshed on that for too long, I'm fine with the opposite approach,
let's make it a "dqs-is-wired" (or something alike) property.
Thanks,
Miquèl
^ permalink raw reply [flat|nested] 46+ messages in thread
* [RFC PATCH v2 09/12] spi: cadence-quadspi: add PHY tuning infrastructure
2026-01-13 14:16 [RFC PATCH v2 00/12] spi: cadence-quadspi: add PHY tuning support Santhosh Kumar K
` (7 preceding siblings ...)
2026-01-13 14:16 ` [RFC PATCH v2 08/12] spi: cadence-quadspi: read 'has-dqs' DT property Santhosh Kumar K
@ 2026-01-13 14:16 ` Santhosh Kumar K
2026-02-05 17:39 ` Miquel Raynal
2026-02-09 9:48 ` Michael Walle
2026-01-13 14:16 ` [RFC PATCH v2 10/12] spi: cadence-quadspi: implement PHY tuning algorithm Santhosh Kumar K
` (3 subsequent siblings)
12 siblings, 2 replies; 46+ messages in thread
From: Santhosh Kumar K @ 2026-01-13 14:16 UTC (permalink / raw)
To: broonie, robh, krzk+dt, conor+dt, miquel.raynal, richard,
vigneshr, tudor.ambarus, pratyush, mwalle
Cc: linux-spi, devicetree, linux-kernel, linux-mtd, praneeth,
u-kumar1, p-mantena, a-dutta, s-k6
Implement the spi_controller_mem_ops execute_tuning callback to enable
PHY tuning support for the Cadence controller. PHY tuning optimizes data
capture timing at high frequencies by calibrating the read data capture
delay through the controller's PHY interface.
Tuning algorithm functions (cqspi_phy_tuning_ddr/sdr and
cqspi_phy_pre/post_config) are placeholders to be implemented
in subsequent commits.
Signed-off-by: Santhosh Kumar K <s-k6@ti.com>
---
drivers/spi/spi-cadence-quadspi.c | 241 ++++++++++++++++++++++++++++++
1 file changed, 241 insertions(+)
diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
index 0df286d24256..b8b0e85f4f68 100644
--- a/drivers/spi/spi-cadence-quadspi.c
+++ b/drivers/spi/spi-cadence-quadspi.c
@@ -32,6 +32,7 @@
#define CQSPI_NAME "cadence-qspi"
#define CQSPI_MAX_CHIPSELECT 4
+#define CQSPI_AM654_NON_PHY_CLK_RATE 25000000
static_assert(CQSPI_MAX_CHIPSELECT <= SPI_DEVICE_CS_CNT_MAX);
@@ -65,6 +66,7 @@ struct cqspi_st;
struct cqspi_flash_pdata {
struct cqspi_st *cqspi;
u32 clk_rate;
+ u32 non_phy_clk_rate;
u32 read_delay;
u32 tshsl_ns;
u32 tsd2d_ns;
@@ -72,6 +74,8 @@ struct cqspi_flash_pdata {
u32 tslch_ns;
bool has_dqs;
u8 cs;
+ struct spi_mem_op phy_read_op;
+ struct spi_mem_op phy_write_op;
};
struct cqspi_st {
@@ -124,6 +128,9 @@ struct cqspi_driver_platdata {
u32 (*get_dma_status)(struct cqspi_st *cqspi);
int (*jh7110_clk_init)(struct platform_device *pdev,
struct cqspi_st *cqspi);
+ int (*execute_tuning)(struct spi_mem *mem, struct spi_mem_op *read_op,
+ struct spi_mem_op *write_op);
+ u32 (*get_non_phy_clk_rate)(struct cqspi_st *cqspi);
};
/* Operation timeout value */
@@ -314,6 +321,25 @@ struct cqspi_driver_platdata {
#define CQSPI_REG_VERSAL_DMA_VAL 0x602
+/*
+ * PHY tuning pattern for calibrating read data capture delay. This 128-byte
+ * pattern provides sufficient bit transitions across all byte lanes to
+ * reliably detect timing windows at high frequencies.
+ */
+static const u8 phy_tuning_pattern[] __aligned(64) = {
+ 0xFE, 0xFF, 0x01, 0x01, 0x01, 0x01, 0x01, 0x00, 0x00, 0xFE, 0xFE, 0x01,
+ 0x01, 0x01, 0x01, 0x00, 0x00, 0xFE, 0xFE, 0x01, 0xFF, 0xFF, 0xFF, 0xFF,
+ 0xFF, 0x00, 0x00, 0xFE, 0xFE, 0xFF, 0xFF, 0xFF, 0xFF, 0x00, 0x00, 0xFE,
+ 0xFE, 0xFF, 0x01, 0x01, 0x01, 0x01, 0x01, 0xFE, 0x00, 0xFE, 0xFE, 0x01,
+ 0x01, 0x01, 0x01, 0xFE, 0x00, 0xFE, 0xFE, 0x01, 0xFF, 0xFF, 0xFF, 0xFF,
+ 0xFF, 0xFE, 0x00, 0xFE, 0xFE, 0xFF, 0xFF, 0xFF, 0xFF, 0xFE, 0x00, 0xFE,
+ 0xFE, 0xFF, 0x01, 0x01, 0x01, 0x01, 0x01, 0x00, 0xFE, 0xFE, 0xFE, 0x01,
+ 0x01, 0x01, 0x01, 0x00, 0xFE, 0xFE, 0xFE, 0x01, 0xFF, 0xFF, 0xFF, 0xFF,
+ 0xFF, 0x00, 0xFE, 0xFE, 0xFE, 0xFF, 0xFF, 0xFF, 0xFF, 0x00, 0xFE, 0xFE,
+ 0xFE, 0xFF, 0x01, 0x01, 0x01, 0x01, 0x01, 0xFE, 0xFE, 0xFE, 0xFE, 0x01,
+ 0x01, 0x01, 0x01, 0xFE, 0xFE, 0xFE, 0xFE, 0x01,
+};
+
static int cqspi_wait_for_bit(const struct cqspi_driver_platdata *ddata,
void __iomem *reg, const u32 mask, bool clr,
bool busywait)
@@ -1550,6 +1576,214 @@ static bool cqspi_supports_mem_op(struct spi_mem *mem,
return spi_mem_default_supports_op(mem, op);
}
+static int cqspi_write_pattern_to_cache(struct cqspi_flash_pdata *f_pdata,
+ struct spi_mem *mem,
+ struct spi_mem_op *write_op)
+{
+ struct device *dev = &f_pdata->cqspi->pdev->dev;
+ int ret;
+
+ write_op->data.nbytes = sizeof(phy_tuning_pattern);
+ write_op->data.buf.out = phy_tuning_pattern;
+
+ ret = spi_mem_exec_op(mem, write_op);
+ if (ret) {
+ dev_err(dev, "Failed to write PHY pattern to cache: %d\n", ret);
+ return ret;
+ }
+ dev_dbg(dev, "PHY pattern (%zu bytes) written to cache\n",
+ sizeof(phy_tuning_pattern));
+
+ return 0;
+}
+
+static int cqspi_get_phy_pattern_offset(struct device *dev, u32 *offset)
+{
+ struct device_node *np, *flash_np = NULL;
+ struct device_node *partition_np, *part_np;
+ const char *label;
+ const __be32 *reg;
+ int len;
+
+ if (!dev || !dev->of_node)
+ return -EINVAL;
+
+ for_each_child_of_node(dev->of_node, np) {
+ if (of_node_name_prefix(np, "flash")) {
+ flash_np = np;
+ break;
+ }
+ }
+
+ if (!flash_np)
+ return -ENODEV;
+
+ partition_np = of_get_child_by_name(flash_np, "partitions");
+ if (!partition_np) {
+ of_node_put(flash_np);
+ return -ENODEV;
+ }
+
+ for_each_child_of_node(partition_np, part_np) {
+ if (of_property_read_string(part_np, "label", &label) ||
+ !strstr(label, "phypattern"))
+ continue;
+
+ reg = of_get_property(part_np, "reg", &len);
+ if (reg && len >= sizeof(__be32)) {
+ *offset = be32_to_cpu(reg[0]);
+ of_node_put(part_np);
+ of_node_put(partition_np);
+ of_node_put(flash_np);
+ return 0;
+ }
+ }
+
+ of_node_put(partition_np);
+ of_node_put(flash_np);
+ return -ENOENT;
+}
+
+static int cqspi_phy_check_pattern(struct cqspi_flash_pdata *f_pdata,
+ struct spi_mem *mem)
+{
+ struct spi_mem_op op;
+ u8 *read_data;
+ int ret;
+
+ read_data = kmalloc_array(1, sizeof(phy_tuning_pattern), GFP_KERNEL);
+ if (!read_data)
+ return -ENOMEM;
+
+ op = f_pdata->phy_read_op;
+ op.data.buf.in = read_data;
+ op.data.nbytes = sizeof(phy_tuning_pattern);
+
+ ret = spi_mem_exec_op(mem, &op);
+ if (ret)
+ goto out;
+
+ if (memcmp(read_data, phy_tuning_pattern, sizeof(phy_tuning_pattern)))
+ ret = -EAGAIN;
+
+out:
+ kfree(read_data);
+ return ret;
+}
+
+static void cqspi_phy_pre_config(struct cqspi_st *cqspi,
+ struct cqspi_flash_pdata *f_pdata,
+ const bool bypass)
+{
+ /* Placeholder for PHY pre-configuration */
+}
+
+static void cqspi_phy_post_config(struct cqspi_st *cqspi,
+ const unsigned int delay)
+{
+ /* Placeholder for PHY post-configuration */
+}
+
+static int cqspi_phy_tuning_ddr(struct cqspi_flash_pdata *f_pdata,
+ struct spi_mem *mem)
+{
+ /* Placeholder for DDR mode PHY tuning algorithm */
+ return 0;
+}
+
+static int cqspi_phy_tuning_sdr(struct cqspi_flash_pdata *f_pdata,
+ struct spi_mem *mem)
+{
+ /* Placeholder for SDR mode PHY tuning algorithm */
+ return 0;
+}
+
+static int cqspi_am654_ospi_execute_tuning(struct spi_mem *mem,
+ struct spi_mem_op *read_op,
+ struct spi_mem_op *write_op)
+{
+ struct cqspi_st *cqspi =
+ spi_controller_get_devdata(mem->spi->controller);
+ struct cqspi_flash_pdata *f_pdata;
+ struct device *dev = &cqspi->pdev->dev;
+ int ret;
+ u32 phy_offset;
+
+ f_pdata = &cqspi->f_pdata[spi_get_chipselect(mem->spi, 0)];
+
+ if (read_op->max_freq <= f_pdata->non_phy_clk_rate) {
+ dev_dbg(dev,
+ "Frequency %u Hz below PHY threshold %u Hz, skipping tuning\n",
+ read_op->max_freq, f_pdata->non_phy_clk_rate);
+ return 0;
+ }
+
+ if (write_op) {
+ ret = cqspi_write_pattern_to_cache(f_pdata, mem, write_op);
+ if (ret) {
+ dev_warn(dev,
+ "failed to write pattern to cache: %d, skipping PHY tuning\n",
+ ret);
+ return ret;
+ }
+
+ f_pdata->phy_write_op = *write_op;
+ } else {
+ ret = cqspi_get_phy_pattern_offset(dev, &phy_offset);
+ if (ret) {
+ dev_warn(dev,
+ "PHY pattern partition not found: %d, skipping PHY tuning\n",
+ ret);
+ return ret;
+ }
+
+ read_op->addr.val = phy_offset;
+ }
+
+ f_pdata->phy_read_op = *read_op;
+
+ ret = cqspi_phy_check_pattern(f_pdata, mem);
+ if (ret) {
+ dev_err(dev, "PHY pattern not found: %d, skipping PHY tuning\n",
+ ret);
+ return ret;
+ }
+
+ if (read_op->cmd.dtr || read_op->addr.dtr || read_op->dummy.dtr ||
+ read_op->data.dtr) {
+ cqspi_phy_pre_config(cqspi, f_pdata, false);
+ ret = cqspi_phy_tuning_ddr(f_pdata, mem);
+ } else {
+ cqspi_phy_pre_config(cqspi, f_pdata, true);
+ ret = cqspi_phy_tuning_sdr(f_pdata, mem);
+ }
+
+ if (ret)
+ dev_warn(dev, "PHY tuning failed: %d\n", ret);
+
+ cqspi_phy_post_config(cqspi, f_pdata->read_delay);
+
+ return ret;
+}
+
+static u32 cqspi_am654_ospi_get_non_phy_clk_rate(struct cqspi_st *cqspi)
+{
+ return CQSPI_AM654_NON_PHY_CLK_RATE;
+}
+
+static int cqspi_mem_op_execute_tuning(struct spi_mem *mem,
+ struct spi_mem_op *read_op,
+ struct spi_mem_op *write_op)
+{
+ struct cqspi_st *cqspi =
+ spi_controller_get_devdata(mem->spi->controller);
+
+ if (!cqspi->ddata->execute_tuning)
+ return -EOPNOTSUPP;
+
+ return cqspi->ddata->execute_tuning(mem, read_op, write_op);
+}
+
static int cqspi_of_get_flash_pdata(struct platform_device *pdev,
struct cqspi_flash_pdata *f_pdata,
struct device_node *np)
@@ -1584,6 +1818,10 @@ static int cqspi_of_get_flash_pdata(struct platform_device *pdev,
return -ENXIO;
}
+ if (f_pdata->cqspi->ddata->get_non_phy_clk_rate)
+ f_pdata->non_phy_clk_rate =
+ f_pdata->cqspi->ddata->get_non_phy_clk_rate(f_pdata->cqspi);
+
f_pdata->has_dqs = of_property_read_bool(np, "spi-has-dqs");
return 0;
@@ -1725,6 +1963,7 @@ static const struct spi_controller_mem_ops cqspi_mem_ops = {
.exec_op = cqspi_exec_mem_op,
.get_name = cqspi_get_name,
.supports_op = cqspi_supports_mem_op,
+ .execute_tuning = cqspi_mem_op_execute_tuning,
};
static const struct spi_controller_mem_caps cqspi_mem_caps = {
@@ -2136,6 +2375,8 @@ static const struct cqspi_driver_platdata k2g_qspi = {
static const struct cqspi_driver_platdata am654_ospi = {
.hwcaps_mask = CQSPI_SUPPORTS_OCTAL | CQSPI_SUPPORTS_QUAD,
.quirks = CQSPI_NEEDS_WR_DELAY,
+ .execute_tuning = cqspi_am654_ospi_execute_tuning,
+ .get_non_phy_clk_rate = cqspi_am654_ospi_get_non_phy_clk_rate,
};
static const struct cqspi_driver_platdata intel_lgm_qspi = {
--
2.34.1
^ permalink raw reply related [flat|nested] 46+ messages in thread* Re: [RFC PATCH v2 09/12] spi: cadence-quadspi: add PHY tuning infrastructure
2026-01-13 14:16 ` [RFC PATCH v2 09/12] spi: cadence-quadspi: add PHY tuning infrastructure Santhosh Kumar K
@ 2026-02-05 17:39 ` Miquel Raynal
2026-02-06 19:25 ` Santhosh Kumar K
2026-02-09 9:48 ` Michael Walle
1 sibling, 1 reply; 46+ messages in thread
From: Miquel Raynal @ 2026-02-05 17:39 UTC (permalink / raw)
To: Santhosh Kumar K
Cc: broonie, robh, krzk+dt, conor+dt, richard, vigneshr,
tudor.ambarus, pratyush, mwalle, linux-spi, devicetree,
linux-kernel, linux-mtd, praneeth, u-kumar1, p-mantena, a-dutta
On 13/01/2026 at 19:46:14 +0530, Santhosh Kumar K <s-k6@ti.com> wrote:
> Implement the spi_controller_mem_ops execute_tuning callback to enable
> PHY tuning support for the Cadence controller. PHY tuning optimizes data
> capture timing at high frequencies by calibrating the read data capture
> delay through the controller's PHY interface.
>
> Tuning algorithm functions (cqspi_phy_tuning_ddr/sdr and
> cqspi_phy_pre/post_config) are placeholders to be implemented
> in subsequent commits.
>
> Signed-off-by: Santhosh Kumar K <s-k6@ti.com>
> ---
> drivers/spi/spi-cadence-quadspi.c | 241 ++++++++++++++++++++++++++++++
> 1 file changed, 241 insertions(+)
>
> diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
> index 0df286d24256..b8b0e85f4f68 100644
> --- a/drivers/spi/spi-cadence-quadspi.c
> +++ b/drivers/spi/spi-cadence-quadspi.c
> @@ -32,6 +32,7 @@
>
> #define CQSPI_NAME "cadence-qspi"
> #define CQSPI_MAX_CHIPSELECT 4
> +#define CQSPI_AM654_NON_PHY_CLK_RATE 25000000
>
> static_assert(CQSPI_MAX_CHIPSELECT <= SPI_DEVICE_CS_CNT_MAX);
>
> @@ -65,6 +66,7 @@ struct cqspi_st;
> struct cqspi_flash_pdata {
> struct cqspi_st *cqspi;
> u32 clk_rate;
> + u32 non_phy_clk_rate;
This is the second (and last) main issue I have with the series as it is
right now. We cannot set this type of frequency in the driver IMO, it is
too board specific.
We currently have a DT property for the SPI maximum supported
frequency. I believe this is no longer enough. Why not making this
frequency property an array? First frequency would be the default,
non tuned maximum frequency. The second would be the maximum frequency
reachable when tuning the PHY.
The rest of the patch LGTM otherwise, but there is this frequency
information which I think should be handled with more care.
Thanks,
Miquèl
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [RFC PATCH v2 09/12] spi: cadence-quadspi: add PHY tuning infrastructure
2026-02-05 17:39 ` Miquel Raynal
@ 2026-02-06 19:25 ` Santhosh Kumar K
2026-02-13 8:18 ` Miquel Raynal
0 siblings, 1 reply; 46+ messages in thread
From: Santhosh Kumar K @ 2026-02-06 19:25 UTC (permalink / raw)
To: Miquel Raynal
Cc: broonie, robh, krzk+dt, conor+dt, richard, vigneshr,
tudor.ambarus, pratyush, mwalle, linux-spi, devicetree,
linux-kernel, linux-mtd, praneeth, u-kumar1, p-mantena, a-dutta,
s-k6
On 05/02/26 23:09, Miquel Raynal wrote:
> On 13/01/2026 at 19:46:14 +0530, Santhosh Kumar K <s-k6@ti.com> wrote:
>
>> Implement the spi_controller_mem_ops execute_tuning callback to enable
>> PHY tuning support for the Cadence controller. PHY tuning optimizes data
>> capture timing at high frequencies by calibrating the read data capture
>> delay through the controller's PHY interface.
>>
>> Tuning algorithm functions (cqspi_phy_tuning_ddr/sdr and
>> cqspi_phy_pre/post_config) are placeholders to be implemented
>> in subsequent commits.
>>
>> Signed-off-by: Santhosh Kumar K <s-k6@ti.com>
>> ---
>> drivers/spi/spi-cadence-quadspi.c | 241 ++++++++++++++++++++++++++++++
>> 1 file changed, 241 insertions(+)
>>
>> diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
>> index 0df286d24256..b8b0e85f4f68 100644
>> --- a/drivers/spi/spi-cadence-quadspi.c
>> +++ b/drivers/spi/spi-cadence-quadspi.c
>> @@ -32,6 +32,7 @@
>>
>> #define CQSPI_NAME "cadence-qspi"
>> #define CQSPI_MAX_CHIPSELECT 4
>> +#define CQSPI_AM654_NON_PHY_CLK_RATE 25000000
>>
>> static_assert(CQSPI_MAX_CHIPSELECT <= SPI_DEVICE_CS_CNT_MAX);
>>
>> @@ -65,6 +66,7 @@ struct cqspi_st;
>> struct cqspi_flash_pdata {
>> struct cqspi_st *cqspi;
>> u32 clk_rate;
>> + u32 non_phy_clk_rate;
>
> This is the second (and last) main issue I have with the series as it is
> right now. We cannot set this type of frequency in the driver IMO, it is
> too board specific.
>
> We currently have a DT property for the SPI maximum supported
> frequency. I believe this is no longer enough. Why not making this
> frequency property an array? First frequency would be the default,
> non tuned maximum frequency. The second would be the maximum frequency
> reachable when tuning the PHY.
If the concern is only about where this is set, we could introduce a DT
property such as "non-phy-max-freq" to carry this information. This
would allow us to avoid any changes to the existing "spi-max-frequency"
handling. Let me know your thoughts on this.
I'll also test the approach you suggested and share my inputs based on
the results. By the way, where are you insisting to adjust/switch to
the maximum frequency - within the controller driver or in the
spi-core?
Regards,
Santhosh.
>
> The rest of the patch LGTM otherwise, but there is this frequency
> information which I think should be handled with more care.
>
> Thanks,
> Miquèl
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [RFC PATCH v2 09/12] spi: cadence-quadspi: add PHY tuning infrastructure
2026-02-06 19:25 ` Santhosh Kumar K
@ 2026-02-13 8:18 ` Miquel Raynal
2026-02-18 18:07 ` Santhosh Kumar K
0 siblings, 1 reply; 46+ messages in thread
From: Miquel Raynal @ 2026-02-13 8:18 UTC (permalink / raw)
To: Santhosh Kumar K
Cc: broonie, robh, krzk+dt, conor+dt, richard, vigneshr,
tudor.ambarus, pratyush, mwalle, linux-spi, devicetree,
linux-kernel, linux-mtd, praneeth, u-kumar1, p-mantena, a-dutta
On 07/02/2026 at 00:55:49 +0530, Santhosh Kumar K <s-k6@ti.com> wrote:
> On 05/02/26 23:09, Miquel Raynal wrote:
>> On 13/01/2026 at 19:46:14 +0530, Santhosh Kumar K <s-k6@ti.com> wrote:
>>
>>> Implement the spi_controller_mem_ops execute_tuning callback to enable
>>> PHY tuning support for the Cadence controller. PHY tuning optimizes data
>>> capture timing at high frequencies by calibrating the read data capture
>>> delay through the controller's PHY interface.
>>>
>>> Tuning algorithm functions (cqspi_phy_tuning_ddr/sdr and
>>> cqspi_phy_pre/post_config) are placeholders to be implemented
>>> in subsequent commits.
>>>
>>> Signed-off-by: Santhosh Kumar K <s-k6@ti.com>
>>> ---
>>> drivers/spi/spi-cadence-quadspi.c | 241 ++++++++++++++++++++++++++++++
>>> 1 file changed, 241 insertions(+)
>>>
>>> diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
>>> index 0df286d24256..b8b0e85f4f68 100644
>>> --- a/drivers/spi/spi-cadence-quadspi.c
>>> +++ b/drivers/spi/spi-cadence-quadspi.c
>>> @@ -32,6 +32,7 @@
>>> #define CQSPI_NAME "cadence-qspi"
>>> #define CQSPI_MAX_CHIPSELECT 4
>>> +#define CQSPI_AM654_NON_PHY_CLK_RATE 25000000
>>> static_assert(CQSPI_MAX_CHIPSELECT <= SPI_DEVICE_CS_CNT_MAX);
>>> @@ -65,6 +66,7 @@ struct cqspi_st;
>>> struct cqspi_flash_pdata {
>>> struct cqspi_st *cqspi;
>>> u32 clk_rate;
>>> + u32 non_phy_clk_rate;
>> This is the second (and last) main issue I have with the series as it
>> is
>> right now. We cannot set this type of frequency in the driver IMO, it is
>> too board specific.
>> We currently have a DT property for the SPI maximum supported
>> frequency. I believe this is no longer enough. Why not making this
>> frequency property an array? First frequency would be the default,
>> non tuned maximum frequency. The second would be the maximum frequency
>> reachable when tuning the PHY.
>
> If the concern is only about where this is set, we could introduce a DT
> property such as "non-phy-max-freq" to carry this information. This
> would allow us to avoid any changes to the existing "spi-max-frequency"
> handling. Let me know your thoughts on this.
Naming is difficult, non-phy-max-freq is too TI specific. I was
proposing the evolution of spi-max-frequency because it is backward
compatible. The naming can be discussed after you send a proposal, but
do not include "non-phy" in it. It shall reflect the fact that with fine
tuning we can reach higher frequencies on certain operations.
Mark, any take on this?
> I'll also test the approach you suggested and share my inputs based on
> the results. By the way, where are you insisting to adjust/switch to
> the maximum frequency - within the controller driver or in the
> spi-core?
It is preferable to make the decisions in the core and avoid being smart
in controller drivers, if possible.
Thanks,
Miquèl
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [RFC PATCH v2 09/12] spi: cadence-quadspi: add PHY tuning infrastructure
2026-02-13 8:18 ` Miquel Raynal
@ 2026-02-18 18:07 ` Santhosh Kumar K
2026-02-19 10:30 ` Miquel Raynal
0 siblings, 1 reply; 46+ messages in thread
From: Santhosh Kumar K @ 2026-02-18 18:07 UTC (permalink / raw)
To: Miquel Raynal
Cc: broonie, robh, krzk+dt, conor+dt, richard, vigneshr,
tudor.ambarus, pratyush, mwalle, linux-spi, devicetree,
linux-kernel, linux-mtd, praneeth, u-kumar1, p-mantena, a-dutta,
s-k6
Hello Miquel,
On 13/02/26 13:48, Miquel Raynal wrote:
> On 07/02/2026 at 00:55:49 +0530, Santhosh Kumar K <s-k6@ti.com> wrote:
>
>> On 05/02/26 23:09, Miquel Raynal wrote:
>>> On 13/01/2026 at 19:46:14 +0530, Santhosh Kumar K <s-k6@ti.com> wrote:
>>>
>>>> Implement the spi_controller_mem_ops execute_tuning callback to enable
>>>> PHY tuning support for the Cadence controller. PHY tuning optimizes data
>>>> capture timing at high frequencies by calibrating the read data capture
>>>> delay through the controller's PHY interface.
>>>>
>>>> Tuning algorithm functions (cqspi_phy_tuning_ddr/sdr and
>>>> cqspi_phy_pre/post_config) are placeholders to be implemented
>>>> in subsequent commits.
>>>>
>>>> Signed-off-by: Santhosh Kumar K <s-k6@ti.com>
>>>> ---
>>>> drivers/spi/spi-cadence-quadspi.c | 241 ++++++++++++++++++++++++++++++
>>>> 1 file changed, 241 insertions(+)
>>>>
>>>> diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
>>>> index 0df286d24256..b8b0e85f4f68 100644
>>>> --- a/drivers/spi/spi-cadence-quadspi.c
>>>> +++ b/drivers/spi/spi-cadence-quadspi.c
>>>> @@ -32,6 +32,7 @@
>>>> #define CQSPI_NAME "cadence-qspi"
>>>> #define CQSPI_MAX_CHIPSELECT 4
>>>> +#define CQSPI_AM654_NON_PHY_CLK_RATE 25000000
>>>> static_assert(CQSPI_MAX_CHIPSELECT <= SPI_DEVICE_CS_CNT_MAX);
>>>> @@ -65,6 +66,7 @@ struct cqspi_st;
>>>> struct cqspi_flash_pdata {
>>>> struct cqspi_st *cqspi;
>>>> u32 clk_rate;
>>>> + u32 non_phy_clk_rate;
>>> This is the second (and last) main issue I have with the series as it
>>> is
>>> right now. We cannot set this type of frequency in the driver IMO, it is
>>> too board specific.
>>> We currently have a DT property for the SPI maximum supported
>>> frequency. I believe this is no longer enough. Why not making this
>>> frequency property an array? First frequency would be the default,
>>> non tuned maximum frequency. The second would be the maximum frequency
>>> reachable when tuning the PHY.
>>
>> If the concern is only about where this is set, we could introduce a DT
>> property such as "non-phy-max-freq" to carry this information. This
>> would allow us to avoid any changes to the existing "spi-max-frequency"
>> handling. Let me know your thoughts on this.
>
> Naming is difficult, non-phy-max-freq is too TI specific. I was
> proposing the evolution of spi-max-frequency because it is backward
> compatible. The naming can be discussed after you send a proposal, but
> do not include "non-phy" in it. It shall reflect the fact that with fine
> tuning we can reach higher frequencies on certain operations.
I tried your suggestion of keeping an array of frequencies in
spi-max-frequency:
spi-max-frequency = <25000000 166000000>;
(non_phy_freq phy_freq)
and updating max_speed_hz with phy_freq once tuning succeeds.
Bad news! this doesn't seem to work as we expected. The
read_op->max_freq for both NOR and NAND is initially set to
non_phy_freq, and it does not appear to be updated again by
adjust_op_freq() after tuning completes as the if case fails.
Regards,
Santhosh.
>
> Mark, any take on this?
>
>> I'll also test the approach you suggested and share my inputs based on
>> the results. By the way, where are you insisting to adjust/switch to
>> the maximum frequency - within the controller driver or in the
>> spi-core?
>
> It is preferable to make the decisions in the core and avoid being smart
> in controller drivers, if possible.
>
> Thanks,
> Miquèl
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [RFC PATCH v2 09/12] spi: cadence-quadspi: add PHY tuning infrastructure
2026-02-18 18:07 ` Santhosh Kumar K
@ 2026-02-19 10:30 ` Miquel Raynal
0 siblings, 0 replies; 46+ messages in thread
From: Miquel Raynal @ 2026-02-19 10:30 UTC (permalink / raw)
To: Santhosh Kumar K
Cc: broonie, robh, krzk+dt, conor+dt, richard, vigneshr,
tudor.ambarus, pratyush, mwalle, linux-spi, devicetree,
linux-kernel, linux-mtd, praneeth, u-kumar1, p-mantena, a-dutta
Hello,
>>>> This is the second (and last) main issue I have with the series as it
>>>> is
>>>> right now. We cannot set this type of frequency in the driver IMO, it is
>>>> too board specific.
>>>> We currently have a DT property for the SPI maximum supported
>>>> frequency. I believe this is no longer enough. Why not making this
>>>> frequency property an array? First frequency would be the default,
>>>> non tuned maximum frequency. The second would be the maximum frequency
>>>> reachable when tuning the PHY.
>>>
>>> If the concern is only about where this is set, we could introduce a DT
>>> property such as "non-phy-max-freq" to carry this information. This
>>> would allow us to avoid any changes to the existing "spi-max-frequency"
>>> handling. Let me know your thoughts on this.
>> Naming is difficult, non-phy-max-freq is too TI specific. I was
>> proposing the evolution of spi-max-frequency because it is backward
>> compatible. The naming can be discussed after you send a proposal, but
>> do not include "non-phy" in it. It shall reflect the fact that with fine
>> tuning we can reach higher frequencies on certain operations.
>
> I tried your suggestion of keeping an array of frequencies in
> spi-max-frequency:
>
> spi-max-frequency = <25000000 166000000>;
> (non_phy_freq phy_freq)
>
> and updating max_speed_hz with phy_freq once tuning succeeds.
>
> Bad news! this doesn't seem to work as we expected. The
> read_op->max_freq for both NOR and NAND is initially set to
> non_phy_freq, and it does not appear to be updated again by
> adjust_op_freq() after tuning completes as the if case fails.
Yes, none of the core parts are ready for this, we may need extra logic
to handle this gracefully. But with such an option, once tuning has
happened, the core could use the correct frequency for each operation?
Thanks,
Miquèl
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [RFC PATCH v2 09/12] spi: cadence-quadspi: add PHY tuning infrastructure
2026-01-13 14:16 ` [RFC PATCH v2 09/12] spi: cadence-quadspi: add PHY tuning infrastructure Santhosh Kumar K
2026-02-05 17:39 ` Miquel Raynal
@ 2026-02-09 9:48 ` Michael Walle
2026-02-12 10:50 ` Miquel Raynal
1 sibling, 1 reply; 46+ messages in thread
From: Michael Walle @ 2026-02-09 9:48 UTC (permalink / raw)
To: Santhosh Kumar K, broonie, robh, krzk+dt, conor+dt, miquel.raynal,
richard, vigneshr, tudor.ambarus, pratyush
Cc: linux-spi, devicetree, linux-kernel, linux-mtd, praneeth,
u-kumar1, p-mantena, a-dutta
[-- Attachment #1: Type: text/plain, Size: 667 bytes --]
On Tue Jan 13, 2026 at 3:16 PM CET, Santhosh Kumar K wrote:
> +static int cqspi_get_phy_pattern_offset(struct device *dev, u32 *offset)
..
> + partition_np = of_get_child_by_name(flash_np, "partitions");
> + if (!partition_np) {
> + of_node_put(flash_np);
> + return -ENODEV;
> + }
> +
> + for_each_child_of_node(partition_np, part_np) {
> + if (of_property_read_string(part_np, "label", &label) ||
> + !strstr(label, "phypattern"))
> + continue;
There was already a review comment on the last version. Moving this
into the driver doesn't make it any better. In fact this might
create a (bad) precedent for future drivers.
-michael
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 297 bytes --]
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [RFC PATCH v2 09/12] spi: cadence-quadspi: add PHY tuning infrastructure
2026-02-09 9:48 ` Michael Walle
@ 2026-02-12 10:50 ` Miquel Raynal
2026-02-12 11:14 ` Michael Walle
0 siblings, 1 reply; 46+ messages in thread
From: Miquel Raynal @ 2026-02-12 10:50 UTC (permalink / raw)
To: Michael Walle
Cc: Santhosh Kumar K, broonie, robh, krzk+dt, conor+dt, richard,
vigneshr, tudor.ambarus, pratyush, linux-spi, devicetree,
linux-kernel, linux-mtd, praneeth, u-kumar1, p-mantena, a-dutta
Hi Michael,
On 09/02/2026 at 10:48:21 +01, "Michael Walle" <mwalle@kernel.org> wrote:
> On Tue Jan 13, 2026 at 3:16 PM CET, Santhosh Kumar K wrote:
>> +static int cqspi_get_phy_pattern_offset(struct device *dev, u32 *offset)
>
> ..
>
>> + partition_np = of_get_child_by_name(flash_np, "partitions");
>> + if (!partition_np) {
>> + of_node_put(flash_np);
>> + return -ENODEV;
>> + }
>> +
>> + for_each_child_of_node(partition_np, part_np) {
>> + if (of_property_read_string(part_np, "label", &label) ||
>> + !strstr(label, "phypattern"))
>> + continue;
>
> There was already a review comment on the last version. Moving this
> into the driver doesn't make it any better. In fact this might
> create a (bad) precedent for future drivers.
I remember complaining about it but not if there was a solution
foreseen. In SPI NAND the solution has been found: the pattern is in the
driver and we load it into cache before PHY tuning. But for SPI NOR I
understood this wasn't possible. What would be an alternative?
Thanks,
Miquèl
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [RFC PATCH v2 09/12] spi: cadence-quadspi: add PHY tuning infrastructure
2026-02-12 10:50 ` Miquel Raynal
@ 2026-02-12 11:14 ` Michael Walle
2026-02-12 12:55 ` Miquel Raynal
0 siblings, 1 reply; 46+ messages in thread
From: Michael Walle @ 2026-02-12 11:14 UTC (permalink / raw)
To: Miquel Raynal
Cc: Santhosh Kumar K, broonie, robh, krzk+dt, conor+dt, richard,
vigneshr, tudor.ambarus, pratyush, linux-spi, devicetree,
linux-kernel, linux-mtd, praneeth, u-kumar1, p-mantena, a-dutta
Hi,
On Thu Feb 12, 2026 at 11:50 AM CET, Miquel Raynal wrote:
> On 09/02/2026 at 10:48:21 +01, "Michael Walle" <mwalle@kernel.org> wrote:
>> On Tue Jan 13, 2026 at 3:16 PM CET, Santhosh Kumar K wrote:
>>> +static int cqspi_get_phy_pattern_offset(struct device *dev, u32 *offset)
>>
>> ..
>>
>>> + partition_np = of_get_child_by_name(flash_np, "partitions");
>>> + if (!partition_np) {
>>> + of_node_put(flash_np);
>>> + return -ENODEV;
>>> + }
>>> +
>>> + for_each_child_of_node(partition_np, part_np) {
>>> + if (of_property_read_string(part_np, "label", &label) ||
>>> + !strstr(label, "phypattern"))
>>> + continue;
>>
>> There was already a review comment on the last version. Moving this
>> into the driver doesn't make it any better. In fact this might
>> create a (bad) precedent for future drivers.
>
> I remember complaining about it but not if there was a solution
> foreseen. In SPI NAND the solution has been found: the pattern is in the
> driver and we load it into cache before PHY tuning. But for SPI NOR I
> understood this wasn't possible. What would be an alternative?
I'm not complaining about using a partition for the pattern but
about the hardcoded name of it.
It was proposed to use at least a device tree phandle to point to a
partition (or so).
-michael
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [RFC PATCH v2 09/12] spi: cadence-quadspi: add PHY tuning infrastructure
2026-02-12 11:14 ` Michael Walle
@ 2026-02-12 12:55 ` Miquel Raynal
2026-02-18 18:07 ` Santhosh Kumar K
0 siblings, 1 reply; 46+ messages in thread
From: Miquel Raynal @ 2026-02-12 12:55 UTC (permalink / raw)
To: Michael Walle
Cc: Santhosh Kumar K, broonie, robh, krzk+dt, conor+dt, richard,
vigneshr, tudor.ambarus, pratyush, linux-spi, devicetree,
linux-kernel, linux-mtd, praneeth, u-kumar1, p-mantena, a-dutta
Hello,
>>>> + for_each_child_of_node(partition_np, part_np) {
>>>> + if (of_property_read_string(part_np, "label", &label) ||
>>>> + !strstr(label, "phypattern"))
>>>> + continue;
>>>
>>> There was already a review comment on the last version. Moving this
>>> into the driver doesn't make it any better. In fact this might
>>> create a (bad) precedent for future drivers.
>>
>> I remember complaining about it but not if there was a solution
>> foreseen. In SPI NAND the solution has been found: the pattern is in the
>> driver and we load it into cache before PHY tuning. But for SPI NOR I
>> understood this wasn't possible. What would be an alternative?
>
> I'm not complaining about using a partition for the pattern but
> about the hardcoded name of it.
>
> It was proposed to use at least a device tree phandle to point to a
> partition (or so).
Ah, yes indeed, thanks for clarifying this up (again) for me. I also
agree the hardcoded name is not ideal.
Thanks,
Miquèl
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [RFC PATCH v2 09/12] spi: cadence-quadspi: add PHY tuning infrastructure
2026-02-12 12:55 ` Miquel Raynal
@ 2026-02-18 18:07 ` Santhosh Kumar K
2026-02-19 8:33 ` Michael Walle
2026-02-19 10:34 ` Miquel Raynal
0 siblings, 2 replies; 46+ messages in thread
From: Santhosh Kumar K @ 2026-02-18 18:07 UTC (permalink / raw)
To: Miquel Raynal, Michael Walle
Cc: broonie, robh, krzk+dt, conor+dt, richard, vigneshr,
tudor.ambarus, pratyush, linux-spi, devicetree, linux-kernel,
linux-mtd, praneeth, u-kumar1, p-mantena, a-dutta, s-k6
Hello Michael and Miquel,
On 12/02/26 18:25, Miquel Raynal wrote:
> Hello,
>
>>>>> + for_each_child_of_node(partition_np, part_np) {
>>>>> + if (of_property_read_string(part_np, "label", &label) ||
>>>>> + !strstr(label, "phypattern"))
>>>>> + continue;
>>>>
>>>> There was already a review comment on the last version. Moving this
>>>> into the driver doesn't make it any better. In fact this might
>>>> create a (bad) precedent for future drivers.
>>>
>>> I remember complaining about it but not if there was a solution
>>> foreseen. In SPI NAND the solution has been found: the pattern is in the
>>> driver and we load it into cache before PHY tuning. But for SPI NOR I
>>> understood this wasn't possible. What would be an alternative?
>>
>> I'm not complaining about using a partition for the pattern but
>> about the hardcoded name of it.
>>
>> It was proposed to use at least a device tree phandle to point to a
>> partition (or so).
>
> Ah, yes indeed, thanks for clarifying this up (again) for me. I also
> agree the hardcoded name is not ideal.
I remember this was discussed in the previous version. As mentioned
in v1, using a phandle may not be ideal since a single controller can
be associated with multiple flashes. Regarding the suggestion to
maintain an array of phandles - consider a configuration with three
flashes (NAND, NOR, and another NAND). In such a case, we would not
need a phandle for the NAND devices, right?
Also, I'm trying to understand the practical difference between using
the partition name versus a phandle. Since the phandle would still be
named something like "phy_partition", it seems functionally similar.
Please let me know if I'm missing something here.
Regards,
Santhosh.
>
> Thanks,
> Miquèl
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [RFC PATCH v2 09/12] spi: cadence-quadspi: add PHY tuning infrastructure
2026-02-18 18:07 ` Santhosh Kumar K
@ 2026-02-19 8:33 ` Michael Walle
2026-02-19 10:34 ` Miquel Raynal
1 sibling, 0 replies; 46+ messages in thread
From: Michael Walle @ 2026-02-19 8:33 UTC (permalink / raw)
To: Santhosh Kumar K, Miquel Raynal
Cc: broonie, robh, krzk+dt, conor+dt, richard, vigneshr,
tudor.ambarus, pratyush, linux-spi, devicetree, linux-kernel,
linux-mtd, praneeth, u-kumar1, p-mantena, a-dutta
[-- Attachment #1: Type: text/plain, Size: 2783 bytes --]
On Wed Feb 18, 2026 at 7:07 PM CET, Santhosh Kumar K wrote:
> Hello Michael and Miquel,
>
> On 12/02/26 18:25, Miquel Raynal wrote:
>> Hello,
>>
>>>>>> + for_each_child_of_node(partition_np, part_np) {
>>>>>> + if (of_property_read_string(part_np, "label", &label) ||
>>>>>> + !strstr(label, "phypattern"))
>>>>>> + continue;
>>>>>
>>>>> There was already a review comment on the last version. Moving this
>>>>> into the driver doesn't make it any better. In fact this might
>>>>> create a (bad) precedent for future drivers.
>>>>
>>>> I remember complaining about it but not if there was a solution
>>>> foreseen. In SPI NAND the solution has been found: the pattern is in the
>>>> driver and we load it into cache before PHY tuning. But for SPI NOR I
>>>> understood this wasn't possible. What would be an alternative?
>>>
>>> I'm not complaining about using a partition for the pattern but
>>> about the hardcoded name of it.
>>>
>>> It was proposed to use at least a device tree phandle to point to a
>>> partition (or so).
>>
>> Ah, yes indeed, thanks for clarifying this up (again) for me. I also
>> agree the hardcoded name is not ideal.
>
> I remember this was discussed in the previous version. As mentioned
> in v1, using a phandle may not be ideal since a single controller can
> be associated with multiple flashes. Regarding the suggestion to
> maintain an array of phandles - consider a configuration with three
> flashes (NAND, NOR, and another NAND). In such a case, we would not
> need a phandle for the NAND devices, right?
Miquel said:
| I find pretty strange to have this property in the flash node,
| even though I understand the reason. Perhaps an array of phandles
| may work in the controller node instead? So maybe the phandle
| should be inside the flash node?
But why is this strange? Can't we treat it as some kind of hint for
the controller, esp. because as we now learned, that it's not needed
for NAND flashes?
I.e. for phy tuning to work you'll have either:
- nand flash, there it just works out of the box
- nor flash, you'll need a phy-calibration-pattern-hint property in
the flash node pointing to a flash partition.
If you think about it, a flash partition with a fixed name is also
part of the flash node. You just add one more indirection to get rid
of the hardcoded name and have a proper DT ABI.
> Also, I'm trying to understand the practical difference between using
> the partition name versus a phandle. Since the phandle would still be
> named something like "phy_partition", it seems functionally similar.
> Please let me know if I'm missing something here.
A phandle will be part of the DT ABI. And it will be configurable by
the user.
-michael
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 297 bytes --]
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [RFC PATCH v2 09/12] spi: cadence-quadspi: add PHY tuning infrastructure
2026-02-18 18:07 ` Santhosh Kumar K
2026-02-19 8:33 ` Michael Walle
@ 2026-02-19 10:34 ` Miquel Raynal
1 sibling, 0 replies; 46+ messages in thread
From: Miquel Raynal @ 2026-02-19 10:34 UTC (permalink / raw)
To: Santhosh Kumar K
Cc: Michael Walle, broonie, robh, krzk+dt, conor+dt, richard,
vigneshr, tudor.ambarus, pratyush, linux-spi, devicetree,
linux-kernel, linux-mtd, praneeth, u-kumar1, p-mantena, a-dutta
On 18/02/2026 at 23:37:00 +0530, Santhosh Kumar K <s-k6@ti.com> wrote:
> Hello Michael and Miquel,
>
> On 12/02/26 18:25, Miquel Raynal wrote:
>> Hello,
>>
>>>>>> + for_each_child_of_node(partition_np, part_np) {
>>>>>> + if (of_property_read_string(part_np, "label", &label) ||
>>>>>> + !strstr(label, "phypattern"))
>>>>>> + continue;
>>>>>
>>>>> There was already a review comment on the last version. Moving this
>>>>> into the driver doesn't make it any better. In fact this might
>>>>> create a (bad) precedent for future drivers.
>>>>
>>>> I remember complaining about it but not if there was a solution
>>>> foreseen. In SPI NAND the solution has been found: the pattern is in the
>>>> driver and we load it into cache before PHY tuning. But for SPI NOR I
>>>> understood this wasn't possible. What would be an alternative?
>>>
>>> I'm not complaining about using a partition for the pattern but
>>> about the hardcoded name of it.
>>>
>>> It was proposed to use at least a device tree phandle to point to a
>>> partition (or so).
>> Ah, yes indeed, thanks for clarifying this up (again) for me. I also
>> agree the hardcoded name is not ideal.
>
> I remember this was discussed in the previous version. As mentioned
> in v1, using a phandle may not be ideal since a single controller can
> be associated with multiple flashes. Regarding the suggestion to
> maintain an array of phandles - consider a configuration with three
> flashes (NAND, NOR, and another NAND). In such a case, we would not
> need a phandle for the NAND devices, right?
We could have a per-chip phy-tuning-data phandle?
> Also, I'm trying to understand the practical difference between using
> the partition name versus a phandle. Since the phandle would still be
> named something like "phy_partition", it seems functionally similar.
> Please let me know if I'm missing something here.
Functionally, yes, but conceptually I'd say it looks cleaner (and
somewhat a bit faster when there are many partitions).
Thanks,
Miquèl
^ permalink raw reply [flat|nested] 46+ messages in thread
* [RFC PATCH v2 10/12] spi: cadence-quadspi: implement PHY tuning algorithm
2026-01-13 14:16 [RFC PATCH v2 00/12] spi: cadence-quadspi: add PHY tuning support Santhosh Kumar K
` (8 preceding siblings ...)
2026-01-13 14:16 ` [RFC PATCH v2 09/12] spi: cadence-quadspi: add PHY tuning infrastructure Santhosh Kumar K
@ 2026-01-13 14:16 ` Santhosh Kumar K
2026-02-05 17:42 ` Miquel Raynal
2026-01-13 14:16 ` [RFC PATCH v2 11/12] spi: cadence-quadspi: restrict PHY frequency to tuned operations Santhosh Kumar K
` (2 subsequent siblings)
12 siblings, 1 reply; 46+ messages in thread
From: Santhosh Kumar K @ 2026-01-13 14:16 UTC (permalink / raw)
To: broonie, robh, krzk+dt, conor+dt, miquel.raynal, richard,
vigneshr, tudor.ambarus, pratyush, mwalle
Cc: linux-spi, devicetree, linux-kernel, linux-mtd, praneeth,
u-kumar1, p-mantena, a-dutta, s-k6
Implement PHY tuning for SDR and DDR modes. PHY tuning calibrates RX
and TX delay lines to find optimal timing for high-speed operation.
Add DLL management functions:
- cqspi_resync_dll(): Reset DLL and wait for lock
- cqspi_set_dll(): Configure RX/TX delays (0-127)
Add pre/post config functions that enable PHY mode during tuning and
restore normal operation afterward. PHY mode consumes one dummy cycle,
so adjust dummy count to maintain correct flash timing.
SDR tuning uses 1D search across RX delays at fixed TX. Search for
two valid windows at consecutive read_delay values, select the larger
window, and use the midpoint.
DDR tuning uses 2D search across RX and TX delays:
- Primary and secondary RX boundary searches at different TX values
- Binary search for gap boundaries within valid region
- Temperature compensation with midpoint calculation
- Systematic boundary searches using 4-step increments
The DDR algorithm finds the four corners of the valid region, identifies
gaps, calculates temperature-aware midpoints, and validates final settings.
Signed-off-by: Santhosh Kumar K <s-k6@ti.com>
---
drivers/spi/spi-cadence-quadspi.c | 1519 ++++++++++++++++++++++++++++-
1 file changed, 1513 insertions(+), 6 deletions(-)
diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
index b8b0e85f4f68..930ea094f6d8 100644
--- a/drivers/spi/spi-cadence-quadspi.c
+++ b/drivers/spi/spi-cadence-quadspi.c
@@ -63,6 +63,12 @@ enum {
struct cqspi_st;
+struct phy_setting {
+ u8 rx;
+ u8 tx;
+ u8 read_delay;
+};
+
struct cqspi_flash_pdata {
struct cqspi_st *cqspi;
u32 clk_rate;
@@ -73,7 +79,9 @@ struct cqspi_flash_pdata {
u32 tchsh_ns;
u32 tslch_ns;
bool has_dqs;
+ bool use_phy;
u8 cs;
+ struct phy_setting phy_setting;
struct spi_mem_op phy_read_op;
struct spi_mem_op phy_write_op;
};
@@ -137,6 +145,7 @@ struct cqspi_driver_platdata {
#define CQSPI_TIMEOUT_MS 500
#define CQSPI_READ_TIMEOUT_MS 10
#define CQSPI_BUSYWAIT_TIMEOUT_US 500
+#define CQSPI_DLL_TIMEOUT_US 300
/* Runtime_pm autosuspend delay */
#define CQSPI_AUTOSUSPEND_TIMEOUT 2000
@@ -150,12 +159,14 @@ struct cqspi_driver_platdata {
/* Register map */
#define CQSPI_REG_CONFIG 0x00
#define CQSPI_REG_CONFIG_ENABLE_MASK BIT(0)
+#define CQSPI_REG_CONFIG_PHY_EN BIT(3)
#define CQSPI_REG_CONFIG_ENB_DIR_ACC_CTRL BIT(7)
#define CQSPI_REG_CONFIG_DECODE_MASK BIT(9)
#define CQSPI_REG_CONFIG_CHIPSELECT_LSB 10
#define CQSPI_REG_CONFIG_DMA_MASK BIT(15)
#define CQSPI_REG_CONFIG_BAUD_LSB 19
#define CQSPI_REG_CONFIG_DTR_PROTO BIT(24)
+#define CQSPI_REG_CONFIG_PHY_PIPELINE BIT(25)
#define CQSPI_REG_CONFIG_DUAL_OPCODE BIT(30)
#define CQSPI_REG_CONFIG_IDLE_LSB 31
#define CQSPI_REG_CONFIG_CHIPSELECT_MASK 0xF
@@ -194,6 +205,7 @@ struct cqspi_driver_platdata {
#define CQSPI_REG_READCAPTURE_BYPASS_LSB 0
#define CQSPI_REG_READCAPTURE_DELAY_LSB 1
#define CQSPI_REG_READCAPTURE_DELAY_MASK 0xF
+#define CQSPI_REG_READCAPTURE_EDGE_LSB 5
#define CQSPI_REG_READCAPTURE_DQS_LSB 8
#define CQSPI_REG_SIZE 0x14
@@ -273,6 +285,27 @@ struct cqspi_driver_platdata {
#define CQSPI_REG_POLLING_STATUS 0xB0
#define CQSPI_REG_POLLING_STATUS_DUMMY_LSB 16
+#define CQSPI_REG_PHY_CONFIG 0xB4
+#define CQSPI_REG_PHY_CONFIG_RX_DEL_LSB 0
+#define CQSPI_REG_PHY_CONFIG_RX_DEL_MASK 0x7F
+#define CQSPI_REG_PHY_CONFIG_TX_DEL_LSB 16
+#define CQSPI_REG_PHY_CONFIG_TX_DEL_MASK 0x7F
+#define CQSPI_REG_PHY_CONFIG_DLL_RESET BIT(30)
+#define CQSPI_REG_PHY_CONFIG_RESYNC BIT(31)
+
+#define CQSPI_REG_PHY_DLL_MASTER 0xB8
+#define CQSPI_REG_PHY_DLL_MASTER_INIT_DELAY_LSB 0
+#define CQSPI_REG_PHY_DLL_MASTER_INIT_DELAY_VAL 16
+#define CQSPI_REG_PHY_DLL_MASTER_DLY_ELMTS_LEN 0x7
+#define CQSPI_REG_PHY_DLL_MASTER_DLY_ELMTS_LSB 20
+#define CQSPI_REG_PHY_DLL_MASTER_DLY_ELMTS_3 0x2
+#define CQSPI_REG_PHY_DLL_MASTER_BYPASS BIT(23)
+#define CQSPI_REG_PHY_DLL_MASTER_CYCLE BIT(24)
+
+#define CQSPI_REG_DLL_OBS_LOW 0xBC
+#define CQSPI_REG_DLL_OBS_LOW_DLL_LOCK BIT(0)
+#define CQSPI_REG_DLL_OBS_LOW_LOOPBACK_LOCK BIT(15)
+
#define CQSPI_REG_OP_EXT_LOWER 0xE0
#define CQSPI_REG_OP_EXT_READ_LSB 24
#define CQSPI_REG_OP_EXT_WRITE_LSB 16
@@ -321,6 +354,33 @@ struct cqspi_driver_platdata {
#define CQSPI_REG_VERSAL_DMA_VAL 0x602
+#define CQSPI_PHY_INIT_RD 1
+#define CQSPI_PHY_MAX_RD 4
+#define CQSPI_PHY_MAX_DELAY 127
+#define CQSPI_PHY_DDR_SEARCH_STEP 4
+#define CQSPI_PHY_MAX_RX 63
+#define CQSPI_PHY_MAX_TX 63
+#define CQSPI_PHY_TX_LOOKUP_LOW_START 28
+#define CQSPI_PHY_TX_LOOKUP_LOW_END 48
+#define CQSPI_PHY_TX_LOOKUP_HIGH_START 60
+#define CQSPI_PHY_TX_LOOKUP_HIGH_END 96
+#define CQSPI_PHY_RX_LOW_SEARCH_START 0
+#define CQSPI_PHY_RX_LOW_SEARCH_END 40
+#define CQSPI_PHY_RX_HIGH_SEARCH_START 24
+#define CQSPI_PHY_RX_HIGH_SEARCH_END 127
+#define CQSPI_PHY_TX_LOW_SEARCH_START 0
+#define CQSPI_PHY_TX_LOW_SEARCH_END 64
+#define CQSPI_PHY_TX_HIGH_SEARCH_START 78
+#define CQSPI_PHY_TX_HIGH_SEARCH_END 127
+#define CQSPI_PHY_SEARCH_OFFSET 8
+
+#define CQSPI_PHY_DEFAULT_TEMP 45
+#define CQSPI_PHY_MIN_TEMP -45
+#define CQSPI_PHY_MAX_TEMP 130
+#define CQSPI_PHY_MID_TEMP (CQSPI_PHY_MIN_TEMP + \
+ ((CQSPI_PHY_MAX_TEMP - \
+ CQSPI_PHY_MIN_TEMP) / 2))
+
/*
* PHY tuning pattern for calibrating read data capture delay. This 128-byte
* pattern provides sufficient bit transitions across all byte lanes to
@@ -1671,31 +1731,1478 @@ static int cqspi_phy_check_pattern(struct cqspi_flash_pdata *f_pdata,
return ret;
}
+static void cqspi_phy_set_dll_master(struct cqspi_st *cqspi)
+{
+ void __iomem *reg_base = cqspi->iobase;
+ unsigned int reg;
+
+ reg = readl(reg_base + CQSPI_REG_PHY_DLL_MASTER);
+ reg &= ~((CQSPI_REG_PHY_DLL_MASTER_DLY_ELMTS_LEN
+ << CQSPI_REG_PHY_DLL_MASTER_DLY_ELMTS_LSB) |
+ CQSPI_REG_PHY_DLL_MASTER_BYPASS |
+ CQSPI_REG_PHY_DLL_MASTER_CYCLE);
+ reg |= ((CQSPI_REG_PHY_DLL_MASTER_DLY_ELMTS_3
+ << CQSPI_REG_PHY_DLL_MASTER_DLY_ELMTS_LSB) |
+ CQSPI_REG_PHY_DLL_MASTER_CYCLE);
+
+ writel(reg, reg_base + CQSPI_REG_PHY_DLL_MASTER);
+}
+
static void cqspi_phy_pre_config(struct cqspi_st *cqspi,
struct cqspi_flash_pdata *f_pdata,
const bool bypass)
{
- /* Placeholder for PHY pre-configuration */
+ void __iomem *reg_base = cqspi->iobase;
+ unsigned int reg;
+ u8 dummy;
+
+ cqspi_readdata_capture(cqspi, bypass, f_pdata->has_dqs,
+ f_pdata->phy_setting.read_delay);
+
+ reg = readl(reg_base + CQSPI_REG_CONFIG);
+ reg &= ~(CQSPI_REG_CONFIG_PHY_EN | CQSPI_REG_CONFIG_PHY_PIPELINE);
+ reg |= CQSPI_REG_CONFIG_PHY_EN;
+ writel(reg, reg_base + CQSPI_REG_CONFIG);
+
+ reg = readl(reg_base + CQSPI_REG_RD_INSTR);
+ dummy = FIELD_GET(CQSPI_REG_RD_INSTR_DUMMY_MASK
+ << CQSPI_REG_RD_INSTR_DUMMY_LSB,
+ reg);
+ dummy--;
+ reg &= ~(CQSPI_REG_RD_INSTR_DUMMY_MASK << CQSPI_REG_RD_INSTR_DUMMY_LSB);
+ reg |= FIELD_PREP(CQSPI_REG_RD_INSTR_DUMMY_MASK
+ << CQSPI_REG_RD_INSTR_DUMMY_LSB,
+ dummy);
+ writel(reg, reg_base + CQSPI_REG_RD_INSTR);
+
+ cqspi_phy_set_dll_master(cqspi);
}
static void cqspi_phy_post_config(struct cqspi_st *cqspi,
const unsigned int delay)
{
- /* Placeholder for PHY post-configuration */
+ void __iomem *reg_base = cqspi->iobase;
+ unsigned int reg;
+ u8 dummy;
+
+ reg = readl(reg_base + CQSPI_REG_READCAPTURE);
+ reg &= ~(CQSPI_REG_READCAPTURE_DELAY_MASK
+ << CQSPI_REG_READCAPTURE_DELAY_LSB);
+
+ reg |= (delay & CQSPI_REG_READCAPTURE_DELAY_MASK)
+ << CQSPI_REG_READCAPTURE_DELAY_LSB;
+ writel(reg, reg_base + CQSPI_REG_READCAPTURE);
+
+ reg = readl(reg_base + CQSPI_REG_CONFIG);
+ reg &= ~(CQSPI_REG_CONFIG_PHY_EN | CQSPI_REG_CONFIG_PHY_PIPELINE);
+ writel(reg, reg_base + CQSPI_REG_CONFIG);
+
+ reg = readl(reg_base + CQSPI_REG_RD_INSTR);
+ dummy = FIELD_GET(CQSPI_REG_RD_INSTR_DUMMY_MASK
+ << CQSPI_REG_RD_INSTR_DUMMY_LSB,
+ reg);
+ dummy++;
+ reg &= ~(CQSPI_REG_RD_INSTR_DUMMY_MASK << CQSPI_REG_RD_INSTR_DUMMY_LSB);
+ reg |= FIELD_PREP(CQSPI_REG_RD_INSTR_DUMMY_MASK
+ << CQSPI_REG_RD_INSTR_DUMMY_LSB,
+ dummy);
+ writel(reg, reg_base + CQSPI_REG_RD_INSTR);
+}
+
+static void cqspi_set_dll(void __iomem *reg_base, u8 rx_dll, u8 tx_dll)
+{
+ unsigned int reg;
+
+ reg = readl(reg_base + CQSPI_REG_PHY_CONFIG);
+ reg &= ~((CQSPI_REG_PHY_CONFIG_RX_DEL_MASK
+ << CQSPI_REG_PHY_CONFIG_RX_DEL_LSB) |
+ (CQSPI_REG_PHY_CONFIG_TX_DEL_MASK
+ << CQSPI_REG_PHY_CONFIG_TX_DEL_LSB));
+ reg |= ((rx_dll & CQSPI_REG_PHY_CONFIG_RX_DEL_MASK)
+ << CQSPI_REG_PHY_CONFIG_RX_DEL_LSB) |
+ ((tx_dll & CQSPI_REG_PHY_CONFIG_TX_DEL_MASK)
+ << CQSPI_REG_PHY_CONFIG_TX_DEL_LSB) |
+ CQSPI_REG_PHY_CONFIG_RESYNC;
+ writel(reg, reg_base + CQSPI_REG_PHY_CONFIG);
+}
+
+static int cqspi_resync_dll(struct cqspi_st *cqspi)
+{
+ void __iomem *reg_base = cqspi->iobase;
+ unsigned int reg;
+ int ret;
+
+ ret = cqspi_wait_idle(cqspi);
+ if (ret)
+ return ret;
+
+ reg = readl(reg_base + CQSPI_REG_CONFIG);
+ reg &= ~(CQSPI_REG_CONFIG_ENABLE_MASK);
+ writel(reg, reg_base + CQSPI_REG_CONFIG);
+
+ reg = readl(reg_base + CQSPI_REG_PHY_CONFIG);
+ reg &= ~(CQSPI_REG_PHY_CONFIG_DLL_RESET |
+ CQSPI_REG_PHY_CONFIG_RESYNC);
+ writel(reg, reg_base + CQSPI_REG_PHY_CONFIG);
+
+ reg = readl(reg_base + CQSPI_REG_PHY_DLL_MASTER);
+ reg |= (CQSPI_REG_PHY_DLL_MASTER_INIT_DELAY_VAL
+ << CQSPI_REG_PHY_DLL_MASTER_INIT_DELAY_LSB);
+ writel(reg, reg_base + CQSPI_REG_PHY_DLL_MASTER);
+
+ reg = readl(reg_base + CQSPI_REG_PHY_CONFIG);
+ reg |= CQSPI_REG_PHY_CONFIG_DLL_RESET;
+ writel(reg, reg_base + CQSPI_REG_PHY_CONFIG);
+
+ ret = readl_poll_timeout(reg_base + CQSPI_REG_DLL_OBS_LOW, reg,
+ (reg & CQSPI_REG_DLL_OBS_LOW_DLL_LOCK), 0,
+ CQSPI_DLL_TIMEOUT_US);
+ if (ret)
+ return ret;
+
+ ret = readl_poll_timeout(reg_base + CQSPI_REG_DLL_OBS_LOW, reg,
+ (reg & CQSPI_REG_DLL_OBS_LOW_LOOPBACK_LOCK), 0,
+ CQSPI_DLL_TIMEOUT_US);
+ if (ret)
+ return ret;
+
+ reg = readl(reg_base + CQSPI_REG_PHY_CONFIG);
+ reg |= CQSPI_REG_PHY_CONFIG_RESYNC;
+ writel(reg, reg_base + CQSPI_REG_PHY_CONFIG);
+
+ reg = readl(reg_base + CQSPI_REG_CONFIG);
+ reg |= CQSPI_REG_CONFIG_ENABLE_MASK;
+ writel(reg, reg_base + CQSPI_REG_CONFIG);
+
+ return 0;
+}
+
+static int cqspi_phy_apply_setting(struct cqspi_flash_pdata *f_pdata,
+ struct phy_setting *phy)
+{
+ struct cqspi_st *cqspi = f_pdata->cqspi;
+ unsigned int reg;
+
+ reg = readl(cqspi->iobase + CQSPI_REG_READCAPTURE);
+ reg |= BIT(CQSPI_REG_READCAPTURE_EDGE_LSB);
+ writel(reg, cqspi->iobase + CQSPI_REG_READCAPTURE);
+
+ cqspi_set_dll(cqspi->iobase, phy->rx, phy->tx);
+ f_pdata->phy_setting.read_delay = phy->read_delay;
+
+ return cqspi_resync_dll(cqspi);
+}
+
+static int cqspi_find_rx_low_ddr(struct cqspi_flash_pdata *f_pdata,
+ struct spi_mem *mem, struct phy_setting *phy)
+{
+ struct device *dev = &f_pdata->cqspi->pdev->dev;
+ int ret;
+
+ do {
+ phy->rx = CQSPI_PHY_RX_LOW_SEARCH_START;
+ do {
+ ret = cqspi_phy_apply_setting(f_pdata, phy);
+ if (!ret) {
+ ret = cqspi_phy_check_pattern(f_pdata, mem);
+ if (!ret)
+ return 0;
+ }
+
+ phy->rx += CQSPI_PHY_DDR_SEARCH_STEP;
+ } while (phy->rx <= CQSPI_PHY_RX_LOW_SEARCH_END);
+
+ phy->read_delay++;
+ } while (phy->read_delay <= CQSPI_PHY_MAX_RD);
+
+ dev_dbg(dev, "Unable to find RX low\n");
+ return -ENOENT;
+}
+
+static int cqspi_find_rx_low_sdr(struct cqspi_flash_pdata *f_pdata,
+ struct spi_mem *mem, struct phy_setting *phy)
+{
+ struct device *dev = &f_pdata->cqspi->pdev->dev;
+ int ret;
+
+ phy->rx = 0;
+ do {
+ ret = cqspi_phy_apply_setting(f_pdata, phy);
+ if (!ret) {
+ ret = cqspi_phy_check_pattern(f_pdata, mem);
+ if (!ret)
+ return 0;
+ }
+ phy->rx++;
+ } while (phy->rx < CQSPI_PHY_MAX_DELAY - 1);
+
+ dev_dbg(dev, "Unable to find RX low\n");
+ return -ENOENT;
+}
+
+static int cqspi_find_rx_high_ddr(struct cqspi_flash_pdata *f_pdata,
+ struct spi_mem *mem, struct phy_setting *phy)
+{
+ struct device *dev = &f_pdata->cqspi->pdev->dev;
+ int ret;
+
+ do {
+ phy->rx = CQSPI_PHY_RX_HIGH_SEARCH_END;
+ do {
+ ret = cqspi_phy_apply_setting(f_pdata, phy);
+ if (!ret) {
+ ret = cqspi_phy_check_pattern(f_pdata, mem);
+ if (!ret)
+ return 0;
+ }
+
+ phy->rx -= CQSPI_PHY_DDR_SEARCH_STEP;
+ } while (phy->rx >= CQSPI_PHY_RX_HIGH_SEARCH_START);
+
+ phy->read_delay--;
+ } while (phy->read_delay >= CQSPI_PHY_INIT_RD);
+
+ dev_dbg(dev, "Unable to find RX high\n");
+ return -ENOENT;
+}
+
+static int cqspi_find_rx_high_sdr(struct cqspi_flash_pdata *f_pdata,
+ struct spi_mem *mem, struct phy_setting *phy,
+ u8 lowerbound)
+{
+ struct device *dev = &f_pdata->cqspi->pdev->dev;
+ int ret;
+
+ phy->rx = CQSPI_PHY_MAX_DELAY;
+ do {
+ ret = cqspi_phy_apply_setting(f_pdata, phy);
+ if (!ret) {
+ ret = cqspi_phy_check_pattern(f_pdata, mem);
+ if (!ret)
+ return 0;
+ }
+ phy->rx--;
+ } while (phy->rx > lowerbound);
+
+ dev_dbg(dev, "Unable to find RX high\n");
+ return -ENOENT;
+}
+
+static int cqspi_find_tx_low_ddr(struct cqspi_flash_pdata *f_pdata,
+ struct spi_mem *mem, struct phy_setting *phy)
+{
+ struct device *dev = &f_pdata->cqspi->pdev->dev;
+ int ret;
+
+ do {
+ phy->tx = CQSPI_PHY_TX_LOW_SEARCH_START;
+ do {
+ ret = cqspi_phy_apply_setting(f_pdata, phy);
+ if (!ret) {
+ ret = cqspi_phy_check_pattern(f_pdata, mem);
+ if (!ret)
+ return 0;
+ }
+
+ phy->tx += CQSPI_PHY_DDR_SEARCH_STEP;
+ } while (phy->tx <= CQSPI_PHY_TX_LOW_SEARCH_END);
+
+ phy->read_delay++;
+ } while (phy->read_delay <= CQSPI_PHY_MAX_RD);
+
+ dev_dbg(dev, "Unable to find TX low\n");
+ return -ENOENT;
+}
+
+static int cqspi_find_tx_high_ddr(struct cqspi_flash_pdata *f_pdata,
+ struct spi_mem *mem, struct phy_setting *phy)
+{
+ struct device *dev = &f_pdata->cqspi->pdev->dev;
+ int ret;
+
+ do {
+ phy->tx = CQSPI_PHY_TX_HIGH_SEARCH_END;
+ do {
+ ret = cqspi_phy_apply_setting(f_pdata, phy);
+ if (!ret) {
+ ret = cqspi_phy_check_pattern(f_pdata, mem);
+ if (!ret)
+ return 0;
+ }
+
+ phy->tx -= CQSPI_PHY_DDR_SEARCH_STEP;
+ } while (phy->tx >= CQSPI_PHY_TX_HIGH_SEARCH_START);
+
+ phy->read_delay--;
+ } while (phy->read_delay >= CQSPI_PHY_INIT_RD);
+
+ dev_dbg(dev, "Unable to find TX high\n");
+ return -ENOENT;
+}
+
+static int cqspi_phy_find_gaplow_ddr(struct cqspi_flash_pdata *f_pdata,
+ struct spi_mem *mem,
+ struct phy_setting *bottomleft,
+ struct phy_setting *topright,
+ struct phy_setting *gaplow)
+{
+ struct phy_setting left, right, mid;
+ int ret;
+
+ left = *bottomleft;
+ right = *topright;
+
+ mid.tx = left.tx + ((right.tx - left.tx) / 2);
+ mid.rx = left.rx + ((right.rx - left.rx) / 2);
+ mid.read_delay = left.read_delay;
+
+ do {
+ ret = cqspi_phy_apply_setting(f_pdata, &mid);
+ if (!ret)
+ ret = cqspi_phy_check_pattern(f_pdata, mem);
+
+ if (ret) {
+ /* The pattern was not found. Go to the lower half. */
+ right.tx = mid.tx;
+ right.rx = mid.rx;
+
+ mid.tx = left.tx + ((mid.tx - left.tx) / 2);
+ mid.rx = left.rx + ((mid.rx - left.rx) / 2);
+ } else {
+ /* The pattern was found. Go to the upper half. */
+ left.tx = mid.tx;
+ left.rx = mid.rx;
+
+ mid.tx = mid.tx + ((right.tx - mid.tx) / 2);
+ mid.rx = mid.rx + ((right.rx - mid.rx) / 2);
+ }
+
+ /* Break the loop if the window has closed. */
+ } while ((right.tx - left.tx >= 2) && (right.rx - left.rx >= 2));
+
+ *gaplow = mid;
+ return 0;
+}
+
+static int cqspi_phy_find_gaphigh_ddr(struct cqspi_flash_pdata *f_pdata,
+ struct spi_mem *mem,
+ struct phy_setting *bottomleft,
+ struct phy_setting *topright,
+ struct phy_setting *gaphigh)
+{
+ struct phy_setting left, right, mid;
+ int ret;
+
+ left = *bottomleft;
+ right = *topright;
+
+ mid.tx = left.tx + ((right.tx - left.tx) / 2);
+ mid.rx = left.rx + ((right.rx - left.rx) / 2);
+ mid.read_delay = right.read_delay;
+
+ do {
+ ret = cqspi_phy_apply_setting(f_pdata, &mid);
+ if (!ret)
+ ret = cqspi_phy_check_pattern(f_pdata, mem);
+
+ if (ret) {
+ /* The pattern was not found. Go to the upper half. */
+ left.tx = mid.tx;
+ left.rx = mid.rx;
+
+ mid.tx = mid.tx + ((right.tx - mid.tx) / 2);
+ mid.rx = mid.rx + ((right.rx - mid.rx) / 2);
+ } else {
+ /* The pattern was found. Go to the lower half. */
+ right.tx = mid.tx;
+ right.rx = mid.rx;
+
+ mid.tx = left.tx + ((mid.tx - left.tx) / 2);
+ mid.rx = left.rx + ((mid.rx - left.rx) / 2);
+ }
+
+ /* Break the loop if the window has closed. */
+ } while ((right.tx - left.tx >= 2) && (right.rx - left.rx >= 2));
+
+ *gaphigh = mid;
+ return 0;
+}
+
+static int cqspi_get_temp(int *temp)
+{
+ /*
+ * TODO: Implement temperature sensor reading for DDR PHY tuning.
+ *
+ * The DDR tuning algorithm uses temperature compensation to adjust
+ * the final tuning point based on current die temperature. The valid
+ * timing region shifts with temperature variations, and this offset
+ * helps maintain optimal settings across the operating range
+ * (-45°C to 130°C).
+ *
+ * This function should:
+ * - Read the SoC die temperature from the thermal sensor
+ * - Populate the temp parameter with temperature in degrees Celsius
+ * - Return 0 on success, negative error code on failure
+ *
+ * Until implemented, the tuning algorithm falls back to assuming
+ * room temperature (45°C) for temperature compensation calculations.
+ */
+
+ return -EOPNOTSUPP;
+}
+
+static inline void cqspi_phy_reset_setting(struct phy_setting *phy)
+{
+ *phy = (struct phy_setting){ .rx = 0, .tx = 127, .read_delay = 0 };
}
static int cqspi_phy_tuning_ddr(struct cqspi_flash_pdata *f_pdata,
struct spi_mem *mem)
{
- /* Placeholder for DDR mode PHY tuning algorithm */
- return 0;
+ struct cqspi_st *cqspi = f_pdata->cqspi;
+ struct device *dev = &cqspi->pdev->dev;
+ struct phy_setting rxlow, rxhigh, txlow, txhigh;
+ struct phy_setting srxlow, srxhigh;
+ struct phy_setting bottomleft, topright, searchpoint;
+ struct phy_setting gaplow, gaphigh;
+ struct phy_setting backuppoint, backupcornerpoint;
+ int ret, tmp;
+ bool primary = 1, secondary = 1;
+
+ /*
+ * DDR tuning: 2D search across RX and TX delays for optimal timing.
+ *
+ * Algorithm: Find RX boundaries (rxlow/rxhigh) using TX window search,
+ * find TX boundaries (txlow/txhigh) at fixed RX, define valid region,
+ * locate gaps via binary search, select final point with temperature
+ * compensation.
+ *
+ * rx
+ * 127 ^
+ * | topright
+ * | *
+ * | xxxxx ++++++++++++++++++++
+ * | xxxxxx +++++++++++++++++++
+ * | xxxxxxx ++++++++++++++++++
+ * | xxxxxxxx +++++++++++++++++
+ * | xxxxxxxxx ++++++++++++++++
+ * | xxxxxxxxxx +++++++++++++++
+ * | xxxxxxxxxxx ++++++++++++++
+ * | xxxxxxxxxxxx +++++++++++++
+ * | xxxxxxxxxxxxx ++++++++++++
+ * | xxxxxxxxxxxxxx +++++++++++
+ * | xxxxxxxxxxxxxxx ++++++++++
+ * | xxxxxxxxxxxxxxxx +++++++++
+ * | xxxxxxxxxxxxxxxxx ++++++++
+ * | xxxxxxxxxxxxxxxxxx +++++++
+ * | *
+ * | bottomleft
+ * -----------------------------------------> tx
+ * 0 127
+ */
+
+ f_pdata->use_phy = true;
+
+ /* Golden rxlow search: Find lower RX boundary using TX window sweep */
+
+ /*
+ * rx
+ * 127 ^
+ * | xxxxx ++++++++++++++++++++
+ * | xxxxxx +++++++++++++++++++
+ * | xxxxxxx ++++++++++++++++++
+ * | xxxxxxxx +++++++++++++++++
+ * | xxxxxxxxx ++++++++++++++++
+ * | xxxxxxxxxx +++++++++++++++
+ * | xxxxxxxxxxx ++++++++++++++
+ * | |xxxxx|xxxxx +++++++++++++
+ * | |xxxxx|xxxxxx ++++++++++++
+ * search | |xxxxx|xxxxxxx +++++++++++
+ * rxlow --------->|xxxxx|xxxxxxxx ++++++++++
+ * | |xxxxx|xxxxxxxxx +++++++++
+ * | |xxxxx|xxxxxxxxxx ++++++++
+ * | |xxxxx|xxxxxxxxxxx +++++++
+ * | | |
+ * --------|-----|----------------------------> tx
+ * 0 | | 127
+ * txlow txlow
+ * start end
+ *
+ * |----------------------------------------------------------|
+ * | Primary | Secondary | Final |
+ * | Search | Search | Point |
+ * |---------|-----------|------------------------------------|
+ * | Fail | Fail | Return Fail |
+ * |---------|-----------|------------------------------------|
+ * | Fail | Pass | Return Fail |
+ * |---------|-----------|------------------------------------|
+ * | Pass | Fail | Return Fail |
+ * |---------|-----------|------------------------------------|
+ * | Pass | Pass | rx = min(primary.rx, secondary.rx) |
+ * | | | tx = primary.tx |
+ * | | | read_delay = |
+ * | | | min(primary.read_delay, |
+ * | | | secondary.read_delay) |
+ * |----------------------------------------------------------|
+ */
+
+ /* Primary rxlow: Sweep TX window to find valid RX lower bound */
+
+ rxlow.tx = CQSPI_PHY_TX_LOOKUP_LOW_START;
+ do {
+ dev_dbg(dev, "Searching for Golden Primary rxlow on TX = %d\n",
+ rxlow.tx);
+ rxlow.read_delay = CQSPI_PHY_INIT_RD;
+ ret = cqspi_find_rx_low_ddr(f_pdata, mem, &rxlow);
+ rxlow.tx += CQSPI_PHY_DDR_SEARCH_STEP;
+ } while (ret && rxlow.tx <= CQSPI_PHY_TX_LOOKUP_LOW_END);
+ if (ret)
+ goto out;
+ dev_dbg(dev, "Golden Primary rxlow: RX: %d TX: %d RD: %d\n", rxlow.rx,
+ rxlow.tx, rxlow.read_delay);
+
+ /* Secondary rxlow: Verify at offset TX for robustness */
+
+ if (rxlow.tx <= (CQSPI_PHY_TX_LOOKUP_LOW_END - CQSPI_PHY_SEARCH_OFFSET))
+ srxlow.tx = rxlow.tx + CQSPI_PHY_SEARCH_OFFSET;
+ else
+ srxlow.tx = CQSPI_PHY_TX_LOOKUP_LOW_END;
+ dev_dbg(dev, "Searching for Golden Secondary rxlow on TX = %d\n",
+ srxlow.tx);
+ srxlow.read_delay = CQSPI_PHY_INIT_RD;
+ ret = cqspi_find_rx_low_ddr(f_pdata, mem, &srxlow);
+ if (ret)
+ goto out;
+ dev_dbg(dev, "Golden Secondary rxlow: RX: %d TX: %d RD: %d\n",
+ srxlow.rx, srxlow.tx, srxlow.read_delay);
+
+ rxlow.rx = min(rxlow.rx, srxlow.rx);
+ rxlow.read_delay = min(rxlow.read_delay, srxlow.read_delay);
+ dev_dbg(dev, "Golden Final rxlow: RX: %d TX: %d RD: %d\n", rxlow.rx,
+ rxlow.tx, rxlow.read_delay);
+
+ /* Golden rxhigh search: Find upper RX boundary at fixed TX */
+
+ /*
+ * rx
+ * 127 ^
+ * | |xxxx ++++++++++++++++++++
+ * | |xxxxx +++++++++++++++++++
+ * search | |xxxxxx ++++++++++++++++++
+ * rxhigh --------->|xxxxxxx +++++++++++++++++
+ * on fixed | |xxxxxxxx ++++++++++++++++
+ * tx | |xxxxxxxxx +++++++++++++++
+ * | |xxxxxxxxxx ++++++++++++++
+ * | xxxxxxxxxxxx +++++++++++++
+ * | xxxxxxxxxxxxx ++++++++++++
+ * | xxxxxxxxxxxxxx +++++++++++
+ * | xxxxxxxxxxxxxxx ++++++++++
+ * | xxxxxxxxxxxxxxxx +++++++++
+ * | xxxxxxxxxxxxxxxxx ++++++++
+ * | xxxxxxxxxxxxxxxxxx +++++++
+ * |
+ * -------------------------------------------> tx
+ * 0 127
+ *
+ * |----------------------------------------------------------|
+ * | Primary | Secondary | Final |
+ * | Search | Search | Point |
+ * |---------|-----------|------------------------------------|
+ * | Fail | Fail | Return Fail |
+ * |---------|-----------|------------------------------------|
+ * | Fail | Pass | Choose Secondary |
+ * |---------|-----------|------------------------------------|
+ * | Pass | Fail | Choose Primary |
+ * |---------|-----------|------------------------------------|
+ * | Pass | Pass | if (secondary.rx > primary.rx) |
+ * | | | Choose Secondary |
+ * | | | else |
+ * | | | Choose Primary |
+ * |----------------------------------------------------------|
+ */
+
+ /* Primary rxhigh: Search at rxlow's TX, decrement from max read_delay */
+
+ rxhigh.tx = rxlow.tx;
+ dev_dbg(dev, "Searching for Golden Primary rxhigh on TX = %d\n",
+ rxhigh.tx);
+ rxhigh.read_delay = CQSPI_PHY_MAX_RD;
+ ret = cqspi_find_rx_high_ddr(f_pdata, mem, &rxhigh);
+ if (ret)
+ primary = 0;
+ dev_dbg(dev, "Golden Primary rxhigh: RX: %d TX: %d RD: %d\n", rxhigh.rx,
+ rxhigh.tx, rxhigh.read_delay);
+
+ /* Secondary rxhigh: Verify at offset TX */
+
+ if (rxhigh.tx <=
+ (CQSPI_PHY_TX_LOOKUP_LOW_END - CQSPI_PHY_SEARCH_OFFSET))
+ srxhigh.tx = rxhigh.tx + CQSPI_PHY_SEARCH_OFFSET;
+ else
+ srxhigh.tx = CQSPI_PHY_TX_LOOKUP_LOW_END;
+ dev_dbg(dev, "Searching for Golden Secondary rxhigh on TX = %d\n",
+ srxhigh.tx);
+ srxhigh.read_delay = CQSPI_PHY_MAX_RD;
+ ret = cqspi_find_rx_high_ddr(f_pdata, mem, &srxhigh);
+ if (ret)
+ secondary = 0;
+ dev_dbg(dev, "Golden Secondary rxhigh: RX: %d TX: %d RD: %d\n",
+ srxhigh.rx, srxhigh.tx, srxhigh.read_delay);
+
+ if (primary || secondary) {
+ if (srxhigh.rx > rxhigh.rx)
+ rxhigh = srxhigh;
+ } else {
+ goto out;
+ }
+ dev_dbg(dev, "Golden Final rxhigh: RX: %d TX: %d RD: %d\n", rxhigh.rx,
+ rxhigh.tx, rxhigh.read_delay);
+
+ primary = 1;
+ secondary = 1;
+
+ /* If rxlow/rxhigh at same read_delay, search backup at upper TX range */
+
+ if (rxlow.read_delay == rxhigh.read_delay) {
+ dev_dbg(dev, "rxlow and rxhigh at the same read delay.\n");
+
+ /* Backup rxlow: Search at high TX window */
+
+ /*
+ * rx
+ * 127 ^
+ * | xxxxx ++++++++++++++++++++
+ * | xxxxxx +++++++++++++++++++
+ * | xxxxxxx ++++++++++++++++++
+ * | xxxxxxxx +++++++++++++++++
+ * | xxxxxxxxx ++++++++++++++++
+ * | xxxxxxxxxx +++++++++++++++
+ * | xxxxxxxxxxx ++++++++++++++
+ * | xxxxxxxxxxxx +++++++|++++|
+ * | xxxxxxxxxxxxx ++++++|++++|
+ * search | xxxxxxxxxxxxxx +++++|++++|
+ * rxlow --------------------------------->|++++|
+ * | xxxxxxxxxxxxxxxx +++|++++|
+ * | xxxxxxxxxxxxxxxxx ++|++++|
+ * | xxxxxxxxxxxxxxxxxx +|++++|
+ * | | |
+ * --------------------------------|----|-----> tx
+ * 0 | | 127
+ * txhigh txhigh
+ * start end
+ *
+ * |-----------------------------------------------------|
+ * | Primary | Secondary | Final |
+ * | Search | Search | Point |
+ * |---------|-----------|-------------------------------|
+ * | Fail | Fail | Return Fail |
+ * |---------|-----------|-------------------------------|
+ * | Fail | Pass | Return Fail |
+ * |---------|-----------|-------------------------------|
+ * | Pass | Fail | Return Fail |
+ * |---------|-----------|-------------------------------|
+ * | Pass | Pass | rx = |
+ * | | | min(primary.rx, secondary.rx)|
+ * | | | tx = primary.tx |
+ * | | | read_delay = |
+ * | | | min(primary.read_delay, |
+ * | | | secondary.read_delay) |
+ * |-----------------------------------------------------|
+ */
+
+ /* Primary backup: Decrement TX from high window end */
+
+ backuppoint.tx = CQSPI_PHY_TX_LOOKUP_HIGH_END;
+ do {
+ dev_dbg(dev,
+ "Searching for Backup Primary rxlow on TX = %d\n",
+ backuppoint.tx);
+ backuppoint.read_delay = CQSPI_PHY_INIT_RD;
+ ret = cqspi_find_rx_low_ddr(f_pdata, mem, &backuppoint);
+ backuppoint.tx -= CQSPI_PHY_DDR_SEARCH_STEP;
+ } while (ret &&
+ backuppoint.tx >= CQSPI_PHY_TX_LOOKUP_HIGH_START);
+ if (ret)
+ goto out;
+ dev_dbg(dev, "Backup Primary rxlow: RX: %d TX: %d RD: %d\n",
+ backuppoint.rx, backuppoint.tx, backuppoint.read_delay);
+
+ /* Secondary backup: Verify at offset TX */
+
+ if (backuppoint.tx >=
+ (CQSPI_PHY_TX_LOOKUP_HIGH_START + CQSPI_PHY_SEARCH_OFFSET))
+ srxlow.tx = backuppoint.tx - CQSPI_PHY_SEARCH_OFFSET;
+ else
+ srxlow.tx = CQSPI_PHY_TX_LOOKUP_HIGH_START;
+ dev_dbg(dev,
+ "Searching for Backup Secondary rxlow on TX = %d\n",
+ srxlow.tx);
+ srxlow.read_delay = CQSPI_PHY_INIT_RD;
+ ret = cqspi_find_rx_low_ddr(f_pdata, mem, &srxlow);
+ if (ret)
+ goto out;
+ dev_dbg(dev, "Backup Secondary rxlow: RX: %d TX: %d RD: %d\n",
+ srxlow.rx, srxlow.tx, srxlow.read_delay);
+
+ backuppoint.rx = min(backuppoint.rx, srxlow.rx);
+ backuppoint.read_delay =
+ min(backuppoint.read_delay, srxlow.read_delay);
+ dev_dbg(dev, "Backup Final rxlow: RX: %d TX: %d RD: %d\n",
+ backuppoint.rx, backuppoint.tx, backuppoint.read_delay);
+
+ if (backuppoint.rx < rxlow.rx) {
+ rxlow = backuppoint;
+ dev_dbg(dev, "Updating rxlow to the one at TX = %d\n",
+ backuppoint.tx);
+ }
+ dev_dbg(dev, "Final rxlow: RX: %d TX: %d RD: %d\n", rxlow.rx,
+ rxlow.tx, rxlow.read_delay);
+
+ /* Backup rxhigh: Search at fixed backup TX */
+
+ /*
+ * rx
+ * 127 ^
+ * | xxxxx +++++++++++++++++++|
+ * | xxxxxx ++++++++++++++++++|
+ * search | xxxxxxx +++++++++++++++++|
+ * rxhigh -------------------------------------->|
+ * on fixed | xxxxxxxxx +++++++++++++++|
+ * tx | xxxxxxxxxx ++++++++++++++|
+ * | xxxxxxxxxxx +++++++++++++|
+ * | xxxxxxxxxxxx +++++++++++++
+ * | xxxxxxxxxxxxx ++++++++++++
+ * | xxxxxxxxxxxxxx +++++++++++
+ * | xxxxxxxxxxxxxxx ++++++++++
+ * | xxxxxxxxxxxxxxxx +++++++++
+ * | xxxxxxxxxxxxxxxxx ++++++++
+ * | xxxxxxxxxxxxxxxxxx +++++++
+ * |
+ * -------------------------------------------> tx
+ * 0 127
+ *
+ * |-----------------------------------------------------|
+ * | Primary | Secondary | Final |
+ * | Search | Search | Point |
+ * |---------|-----------|-------------------------------|
+ * | Fail | Fail | Return Fail |
+ * |---------|-----------|-------------------------------|
+ * | Fail | Pass | Choose Secondary |
+ * |---------|-----------|-------------------------------|
+ * | Pass | Fail | Choose Primary |
+ * |---------|-----------|-------------------------------|
+ * | Pass | Pass | if (secondary.rx > primary.rx)|
+ * | | | Choose Secondary |
+ * | | | else |
+ * | | | Choose Primary |
+ * |-----------------------------------------------------|
+ */
+
+ /* Primary backup rxhigh: Use backup TX, decrement from max read_delay */
+
+ dev_dbg(dev, "Searching for Backup Primary rxhigh on TX = %d\n",
+ backuppoint.tx);
+ backuppoint.read_delay = CQSPI_PHY_MAX_RD;
+ ret = cqspi_find_rx_high_ddr(f_pdata, mem, &backuppoint);
+ if (ret)
+ primary = 0;
+ dev_dbg(dev, "Backup Primary rxhigh: RX: %d TX: %d RD: %d\n",
+ backuppoint.rx, backuppoint.tx, backuppoint.read_delay);
+
+ /* Secondary backup rxhigh: Verify at offset TX */
+
+ if (backuppoint.tx >=
+ (CQSPI_PHY_TX_LOOKUP_HIGH_START + CQSPI_PHY_SEARCH_OFFSET))
+ srxhigh.tx = backuppoint.tx - CQSPI_PHY_SEARCH_OFFSET;
+ else
+ srxhigh.tx = CQSPI_PHY_TX_LOOKUP_HIGH_START;
+ dev_dbg(dev,
+ "Searching for Backup Secondary rxhigh on TX = %d\n",
+ srxhigh.tx);
+ srxhigh.read_delay = CQSPI_PHY_MAX_RD;
+ ret = cqspi_find_rx_high_ddr(f_pdata, mem, &srxhigh);
+ if (ret)
+ secondary = 0;
+ dev_dbg(dev, "Backup Secondary rxhigh: RX: %d TX: %d RD: %d\n",
+ srxhigh.rx, srxhigh.tx, srxhigh.read_delay);
+
+ if (primary || secondary) {
+ if (srxhigh.rx > backuppoint.rx)
+ backuppoint = srxhigh;
+ } else {
+ goto out;
+ }
+ dev_dbg(dev, "Backup Final rxhigh: RX: %d TX: %d RD: %d\n",
+ backuppoint.rx, backuppoint.tx, backuppoint.read_delay);
+
+ if (backuppoint.rx > rxhigh.rx) {
+ rxhigh = backuppoint;
+ dev_dbg(dev, "Updating rxhigh to the one at TX = %d\n",
+ backuppoint.tx);
+ }
+ dev_dbg(dev, "Final rxhigh: RX: %d TX: %d RD: %d\n", rxhigh.rx,
+ rxhigh.tx, rxhigh.read_delay);
+ }
+
+ /* Golden txlow: Fix RX at 1/4 of RX window, search TX lower bound */
+
+ /*
+ * rx
+ * 127 ^
+ * |
+ * rxhigh --------->xxxxx ++++++++++++++++++++
+ * | xxxxxx +++++++++++++++++++
+ * | xxxxxxx ++++++++++++++++++
+ * | xxxxxxxx +++++++++++++++++
+ * | xxxxxxxxx ++++++++++++++++
+ * | xxxxxxxxxx +++++++++++++++
+ * | xxxxxxxxxxx ++++++++++++++
+ * | xxxxxxxxxxxx +++++++++++++
+ * fix rx | xxxxxxxxxxxxx ++++++++++++
+ * 1/4 b/w ---------><------->xxxxx +++++++++++
+ * rxlow and | xxxx|xxxxxxxxxx ++++++++++
+ * rxhigh | xxxx|xxxxxxxxxxx +++++++++
+ * | xxxx|xxxxxxxxxxxx ++++++++
+ * rxlow --------->xxxx|xxxxxxxxxxxxx +++++++
+ * | |
+ * ------------|------------------------------> tx
+ * 0 | 127
+ * search
+ * txlow
+ */
+
+ tmp = rxhigh.rx - rxlow.rx;
+ txlow.rx = rxlow.rx + (tmp / 4);
+ dev_dbg(dev, "Searching for Golden txlow on RX = %d\n", txlow.rx);
+ txlow.read_delay = CQSPI_PHY_INIT_RD;
+ ret = cqspi_find_tx_low_ddr(f_pdata, mem, &txlow);
+ if (ret)
+ goto out;
+ dev_dbg(dev, "Golden txlow: RX: %d TX: %d RD: %d\n", txlow.rx, txlow.tx,
+ txlow.read_delay);
+
+ /* Golden txhigh: Same RX as txlow, decrement from max read_delay */
+
+ /*
+ * rx
+ * 127 ^
+ * |
+ * rxhigh --------->xxxxx ++++++++++++++++++++
+ * | xxxxxx +++++++++++++++++++
+ * | xxxxxxx ++++++++++++++++++
+ * | xxxxxxxx +++++++++++++++++
+ * | xxxxxxxxx ++++++++++++++++
+ * | xxxxxxxxxx +++++++++++++++
+ * | xxxxxxxxxxx ++++++++++++++
+ * | xxxxxxxxxxxx +++++++++++++
+ * fix rx | xxxxxxxxxxxxx ++++++++++++
+ * 1/4 b/w --------------------------------><----->
+ * rxlow and | xxxxxxxxxxxxxxx ++++++|+++
+ * rxhigh | xxxxxxxxxxxxxxxx +++++|+++
+ * | xxxxxxxxxxxxxxxxx ++++|+++
+ * rxlow --------->xxxxxxxxxxxxxxxxxx +++|+++
+ * | |
+ * ----------------------------------|--------> tx
+ * 0 | 127
+ * search
+ * txhigh
+ */
+
+ txhigh.rx = txlow.rx;
+ dev_dbg(dev, "Searching for Golden txhigh on RX = %d\n", txhigh.rx);
+ txhigh.read_delay = CQSPI_PHY_MAX_RD;
+ ret = cqspi_find_tx_high_ddr(f_pdata, mem, &txhigh);
+ if (ret)
+ goto out;
+ dev_dbg(dev, "Golden txhigh: RX: %d TX: %d RD: %d\n", txhigh.rx,
+ txhigh.tx, txhigh.read_delay);
+
+ /* If txlow/txhigh at same read_delay, search backup at 3/4 RX window */
+
+ if (txlow.read_delay == txhigh.read_delay) {
+ /* Backup txlow: Fix RX at 3/4 of RX window */
+
+ /*
+ * rx
+ * 127 ^
+ * |
+ * rxhigh --------->xxxxx ++++++++++++++++++++
+ * | xxxxxx +++++++++++++++++++
+ * fix rx | xxxxxxx ++++++++++++++++++
+ * 3/4 b/w ---------><----->x +++++++++++++++++
+ * rxlow and | xxxx|xxxx ++++++++++++++++
+ * rxhigh | xxxx|xxxxx +++++++++++++++
+ * | xxxx|xxxxxx ++++++++++++++
+ * | xxxx|xxxxxxx +++++++++++++
+ * | xxxx|xxxxxxxx ++++++++++++
+ * | xxxx|xxxxxxxxx +++++++++++
+ * | xxxx|xxxxxxxxxx ++++++++++
+ * | xxxx|xxxxxxxxxxx +++++++++
+ * | xxxx|xxxxxxxxxxxx ++++++++
+ * rxlow --------->xxxx|xxxxxxxxxxxxx +++++++
+ * | |
+ * ------------|------------------------------> tx
+ * 0 | 127
+ * search
+ * txlow
+ */
+
+ dev_dbg(dev, "txlow and txhigh at the same read delay.\n");
+ backuppoint.rx = rxlow.rx + ((tmp * 3) / 4);
+ dev_dbg(dev, "Searching for Backup txlow on RX = %d\n",
+ backuppoint.rx);
+ backuppoint.read_delay = CQSPI_PHY_INIT_RD;
+ ret = cqspi_find_tx_low_ddr(f_pdata, mem, &backuppoint);
+ if (ret)
+ goto out;
+ dev_dbg(dev, "Backup txlow: RX: %d TX: %d RD: %d\n",
+ backuppoint.rx, backuppoint.tx, backuppoint.read_delay);
+
+ if (backuppoint.tx < txlow.tx) {
+ txlow = backuppoint;
+ dev_dbg(dev, "Updating txlow with the one at RX = %d\n",
+ backuppoint.rx);
+ }
+ dev_dbg(dev, "Final txlow: RX: %d TX: %d RD: %d\n", txlow.rx,
+ txlow.tx, txlow.read_delay);
+
+ /* Backup txhigh: Same RX as backup txlow, decrement from max */
+
+ /*
+ * rx
+ * 127 ^
+ * |
+ * rxhigh --------->xxxxx ++++++++++++++++++++
+ * | xxxxxx +++++++++++++++++++
+ * fix rx | xxxxxxx ++++++++++++++++++
+ * 3/4 b/w ------------------------------><------->
+ * rxlow and | xxxxxxxxx +++++++++++|++++
+ * rxhigh | xxxxxxxxxx ++++++++++|++++
+ * | xxxxxxxxxxx +++++++++|++++
+ * | xxxxxxxxxxxx ++++++++|++++
+ * | xxxxxxxxxxxxx +++++++|++++
+ * | xxxxxxxxxxxxxx ++++++|++++
+ * | xxxxxxxxxxxxxxx +++++|++++
+ * | xxxxxxxxxxxxxxxx ++++|++++
+ * | xxxxxxxxxxxxxxxxx +++|++++
+ * rxlow --------->xxxxxxxxxxxxxxxxxx ++|++++
+ * | |
+ * ---------------------------------|---------> tx
+ * 0 | 127
+ * search
+ * txhigh
+ */
+
+ dev_dbg(dev, "Searching for Backup txhigh on RX = %d\n",
+ backuppoint.rx);
+ backuppoint.read_delay = CQSPI_PHY_MAX_RD;
+ ret = cqspi_find_tx_high_ddr(f_pdata, mem, &backuppoint);
+ if (ret)
+ goto out;
+ dev_dbg(dev, "Backup txhigh: RX: %d TX: %d RD: %d\n",
+ backuppoint.rx, backuppoint.tx, backuppoint.read_delay);
+
+ if (backuppoint.tx > txhigh.tx) {
+ txhigh = backuppoint;
+ dev_dbg(dev,
+ "Updating txhigh with the one at RX = %d\n",
+ backuppoint.rx);
+ }
+ dev_dbg(dev, "Final txhigh: RX: %d TX: %d RD: %d\n", txhigh.rx,
+ txhigh.tx, txhigh.read_delay);
+ }
+
+ /* Corner points: Define and verify bottomleft and topright boundaries */
+
+ /*
+ * rx
+ * 127 ^
+ * | topright
+ * | *
+ * rxhigh -----------xxxxx ++++++++++++++++++++
+ * | xxxxxx +++++++++++++++++++
+ * | xxxxxxx ++++++++++++++++++
+ * | xxxxxxxx +++++++++++++++++
+ * | xxxxxxxxx ++++++++++++++++
+ * | xxxxxxxxxx +++++++++++++++
+ * | xxxxxxxxxxx ++++++++++++++
+ * | xxxxxxxxxxxx +++++++++++++
+ * | xxxxxxxxxxxxx ++++++++++++
+ * | xxxxxxxxxxxxxx +++++++++++
+ * | xxxxxxxxxxxxxxx ++++++++++
+ * | xxxxxxxxxxxxxxxx +++++++++
+ * | xxxxxxxxxxxxxxxxx ++++++++
+ * rxlow -----------xxxxxxxxxxxxxxxxxx +++++++
+ * | * |
+ * | bottom|left |
+ * --------|----------------------------|---> tx
+ * 0 | | 127
+ * | |
+ * txlow txhigh
+ *
+ * Verification: Test point 4 taps inside each corner, adjust
+ * read_delay ±1 if needed to ensure valid corners for gap search.
+ */
+
+ bottomleft.tx = txlow.tx;
+ bottomleft.rx = rxlow.rx;
+ if (txlow.read_delay <= rxlow.read_delay)
+ bottomleft.read_delay = txlow.read_delay;
+ else
+ bottomleft.read_delay = rxlow.read_delay;
+
+ /* Verify bottomleft: Test 4 taps inside, adjust read_delay if needed */
+ backupcornerpoint = bottomleft;
+ backupcornerpoint.tx += 4;
+ backupcornerpoint.rx += 4;
+ ret = cqspi_phy_apply_setting(f_pdata, &backupcornerpoint);
+ if (!ret)
+ ret = cqspi_phy_check_pattern(f_pdata, mem);
+
+ if (ret) {
+ backupcornerpoint.read_delay--;
+ ret = cqspi_phy_apply_setting(f_pdata, &backupcornerpoint);
+ if (!ret)
+ ret = cqspi_phy_check_pattern(f_pdata, mem);
+ }
+
+ /* TODO: if (ret) - handle case where corner cannot be verified */
+
+ if (!ret)
+ bottomleft.read_delay = backupcornerpoint.read_delay;
+
+ topright.tx = txhigh.tx;
+ topright.rx = rxhigh.rx;
+ if (txhigh.read_delay >= rxhigh.read_delay)
+ topright.read_delay = txhigh.read_delay;
+ else
+ topright.read_delay = rxhigh.read_delay;
+
+ /* Verify topright: Test 4 taps inside, adjust read_delay if needed */
+ backupcornerpoint = topright;
+ backupcornerpoint.tx -= 4;
+ backupcornerpoint.rx -= 4;
+ ret = cqspi_phy_apply_setting(f_pdata, &backupcornerpoint);
+ if (!ret)
+ ret = cqspi_phy_check_pattern(f_pdata, mem);
+
+ if (ret) {
+ backupcornerpoint.read_delay++;
+ ret = cqspi_phy_apply_setting(f_pdata, &backupcornerpoint);
+ if (!ret)
+ ret = cqspi_phy_check_pattern(f_pdata, mem);
+ }
+
+ /* TODO: if (ret) - handle case where corner cannot be verified */
+
+ if (!ret)
+ topright.read_delay = backupcornerpoint.read_delay;
+
+ dev_dbg(dev, "topright: RX: %d TX: %d RD: %d\n", topright.rx,
+ topright.tx, topright.read_delay);
+ dev_dbg(dev, "bottomleft: RX: %d TX: %d RD: %d\n", bottomleft.rx,
+ bottomleft.tx, bottomleft.read_delay);
+ ret = cqspi_phy_find_gaplow_ddr(f_pdata, mem, &bottomleft, &topright,
+ &gaplow);
+ if (ret)
+ goto out;
+ dev_dbg(dev, "gaplow: RX: %d TX: %d RD: %d\n", gaplow.rx, gaplow.tx,
+ gaplow.read_delay);
+
+ /* Final point selection: Handle single vs dual passing regions */
+
+ if (bottomleft.read_delay == topright.read_delay) {
+ /*
+ * Single region: Use midpoint with temperature compensation.
+ * Gaplow approximates upper boundary of valid region.
+ *
+ * rx
+ * 127 ^
+ * | gaplow (approx. topright)
+ * | |
+ * rxhigh -----------xxxxxxx| failing
+ * | xxxxxxx| region
+ * | xxxxxxx| <--------------->
+ * | xxxxxxx| +++++++++++++++++
+ * | xxxxxxxxx ++++++++++++++++
+ * | xxxxxxxxxx +++++++++++++++
+ * | xxxxxxxxxxx ++++++++++++++
+ * | xxxxxxxxxxxx +++++++++++++
+ * | xxxxxxxxxxxxx ++++++++++++
+ * | xxxxxxxxxxxxxx +++++++++++
+ * | xxxxxxxxxxxxxxx ++++++++++
+ * | xxxxxxxxxxxxxxxx +++++++++
+ * | xxxxxxxxxxxxxxxxx ++++++++
+ * rxlow -----------xxxxxxxxxxxxxxxxxx +++++++
+ * | * |
+ * | bottom|left |
+ * --------|----------------------------|---> tx
+ * 0 | | 127
+ * | |
+ * txlow txhigh
+ * (same read_delay)
+ *
+ * Temperature compensation: Valid region shifts with temp.
+ * Offset = region_size / (330 / (temp - 87.5°C))
+ * Factor 330 is empirically determined for this hardware.
+ */
+
+ dev_dbg(dev,
+ "bottomleft and topright at the same read delay.\n");
+
+ topright = gaplow;
+ searchpoint.read_delay = bottomleft.read_delay;
+ searchpoint.tx =
+ bottomleft.tx + ((topright.tx - bottomleft.tx) / 2);
+ searchpoint.rx =
+ bottomleft.rx + ((topright.rx - bottomleft.rx) / 2);
+
+ ret = cqspi_get_temp(&tmp);
+ if (ret) {
+ /* Assume room temperature if sensor unavailable */
+ dev_dbg(dev,
+ "Unable to get temperature. Assuming room temperature\n");
+ tmp = CQSPI_PHY_DEFAULT_TEMP;
+ }
+
+ if (tmp < CQSPI_PHY_MIN_TEMP || tmp > CQSPI_PHY_MAX_TEMP) {
+ dev_err(dev,
+ "Temperature outside operating range: %dC\n",
+ tmp);
+ ret = -EINVAL;
+ goto out;
+ }
+
+ if (tmp == CQSPI_PHY_MID_TEMP)
+ tmp++; /* Avoid divide-by-zero */
+ dev_dbg(dev, "Temperature: %dC\n", tmp);
+
+ /* Apply temperature offset: positive at high temp, negative at low */
+ searchpoint.tx += (topright.tx - bottomleft.tx) /
+ (330 / (tmp - CQSPI_PHY_MID_TEMP));
+ searchpoint.rx += (topright.rx - bottomleft.rx) /
+ (330 / (tmp - CQSPI_PHY_MID_TEMP));
+ } else {
+ /*
+ * Dual regions: Gap separates two valid regions, choose larger.
+ *
+ * rx
+ * 127 ^
+ * | topright
+ * | *
+ * rxhigh -----------xxxxx +++++++++++++++++++|
+ * | xxxxxx <region 2> ++++++++|
+ * | xxxxxxx +++++++++++++++++|
+ * | xxxxxxxx ++++++++++++++++|
+ * | xxxxxxxxx +++++++++++++++|
+ * | xxxxxxxxxx ++++++++++++++|
+ * | failing |
+ * | region |
+ * | xxxxxxxxxxxxx +++++++++++|
+ * | xxxxxxxxxxxxxx ++++++++++|
+ * | xxxxxxxxxxxxxxx +++++++++|
+ * | xxxxxxxxxxxxxxxx ++++++++|
+ * | xxxxxxxxx <region 1> +++++++|
+ * rxlow -----------xxxxxxxxxxxxxxxxxx ++++++|
+ * | * |
+ * | bottom|left |
+ * --------|----------------------------|---> tx
+ * 0 | | 127
+ * | |
+ * txlow txhigh
+ *
+ * Strategy: Compare Manhattan distances from gap boundaries to
+ * corners. Choose corner furthest from gap (larger region).
+ * Apply 16-tap margin inward, scale RX proportionally.
+ */
+
+ ret = cqspi_phy_find_gaphigh_ddr(f_pdata, mem, &bottomleft,
+ &topright, &gaphigh);
+ if (ret)
+ goto out;
+ dev_dbg(dev, "gaphigh: RX: %d TX: %d RD: %d\n", gaphigh.rx,
+ gaphigh.tx, gaphigh.read_delay);
+
+ /* Compare Manhattan distances: choose corner furthest from gap */
+ if ((abs(gaplow.tx - bottomleft.tx) +
+ abs(gaplow.rx - bottomleft.rx)) <
+ (abs(gaphigh.tx - topright.tx) +
+ abs(gaphigh.rx - topright.rx))) {
+ /* Topright further: Use Region 2, 16 taps inward */
+ searchpoint = topright;
+ searchpoint.tx -= 16;
+ searchpoint.rx -= (16 * (topright.rx - bottomleft.rx)) /
+ (topright.tx - bottomleft.tx);
+ } else {
+ /* Bottomleft further: Use Region 1, 16 taps inward */
+ searchpoint = bottomleft;
+ searchpoint.tx += 16;
+ searchpoint.rx += (16 * (topright.rx - bottomleft.rx)) /
+ (topright.tx - bottomleft.tx);
+ }
+ }
+
+ /* Apply and verify final tuning point */
+ dev_dbg(dev, "Final tuning point: RX: %d TX: %d RD: %d\n",
+ searchpoint.rx, searchpoint.tx, searchpoint.read_delay);
+ ret = cqspi_phy_apply_setting(f_pdata, &searchpoint);
+ if (!ret)
+ ret = cqspi_phy_check_pattern(f_pdata, mem);
+
+ if (ret) {
+ dev_err(dev,
+ "Failed to find pattern at final calibration point\n");
+ ret = -EINVAL;
+ goto out;
+ }
+
+ ret = 0;
+ f_pdata->phy_setting.read_delay = searchpoint.read_delay;
+ f_pdata->phy_setting.rx = searchpoint.rx;
+ f_pdata->phy_setting.tx = searchpoint.tx;
+out:
+ if (ret)
+ f_pdata->use_phy = false;
+
+ return ret;
}
static int cqspi_phy_tuning_sdr(struct cqspi_flash_pdata *f_pdata,
struct spi_mem *mem)
{
- /* Placeholder for SDR mode PHY tuning algorithm */
- return 0;
+ struct cqspi_st *cqspi = f_pdata->cqspi;
+ struct device *dev = &cqspi->pdev->dev;
+ struct phy_setting rxlow, rxhigh, first, second, final;
+ u8 window1 = 0;
+ u8 window2 = 0;
+ int ret;
+
+ /*
+ * SDR tuning: 1D search for optimal RX delay (TX less critical).
+ * Find two consecutive windows, choose larger, use midpoint.
+ *
+ * rx
+ * 127 ^
+ * | |-----window at----------|
+ * | |-----read_dealy = n+1---|
+ * | |xxxxxxxxxxxxxxxxxxxxxxxx|
+ * | |xxxxxxxxxxxxxxxxxxxxxxxx|
+ * | |xxxxxxxxxxxxxxxxxxxxxxxx|
+ * | |xxxxxxxxxxxxxxxxxxxxxxxx|
+ * | |xxxxxxxxxxxxxxxxxxxxxxxx|
+ * | rxlow(n+1) midpoint rxhigh(n+1)
+ * |
+ * | |---window at--------|
+ * | |---read_dealy = n---|
+ * | |xxxxxxxxxxxxxxxxxxxx|
+ * | |xxxxxxxxxxxxxxxxxxxx|
+ * | |xxxxxxxxxxxxxxxxxxxx|
+ * | |xxxxxxxxxxxxxxxxxxxx|
+ * | |xxxxxxxxxxxxxxxxxxxx|
+ * | rxlow(n) midpoint rxhigh(n)
+ * |
+ * -----------------------------------------> tx
+ * 0 127
+ * read_delay=n read_dealy=n+1
+ */
+
+ f_pdata->use_phy = true;
+ cqspi_phy_reset_setting(&rxlow);
+ cqspi_phy_reset_setting(&rxhigh);
+ cqspi_phy_reset_setting(&first);
+
+ /* First window: Find rxlow by incrementing read_delay from 0 */
+
+ /*
+ * rx
+ * 127 ^
+ * | |xxxxxxxxxxxxxxxxxxxx|
+ * search | |xxxxxxxxxxxxxxxxxxxx|
+ * rxlow | |xxxxxxxxxxxxxxxxxxxx|
+ * increasing | |xxxxxxxxxxxxxxxxxxxx|
+ * --------->|xxxxxxxxxxxxxxxxxxxx|
+ * read_delay | |xxxxxxxxxxxxxxxxxxx|
+ * until found | |xxxxxxxxxxxxxxxxxxx|
+ * | rxlow
+ * -----------------------------------------> tx
+ * 0 tx fixed at 127
+ */
+
+ do {
+ ret = cqspi_find_rx_low_sdr(f_pdata, mem, &rxlow);
+
+ if (ret)
+ rxlow.read_delay++;
+ } while (ret && rxlow.read_delay <= CQSPI_PHY_MAX_RD);
+
+ /* Find rxhigh: Decrement from RX=127 at same read_delay */
+
+ /*
+ * rx
+ * 127 ^ search rxhigh
+ * | (decrement from
+ * | 127 until found)
+ * | |
+ * | |
+ * | v
+ * | |------------------------|
+ * | |xxxxxxxxxxxxxxxxxxxxxxxx|
+ * | |xxxxxxxxxxxxxxxxxxxxxxxx|
+ * | |xxxxxxxxxxxxxxxxxxxxxxxx|
+ * | |xxxxxxxxxxxxxxxxxxxxxxxx|
+ * | |xxxxxxxxxxxxxxxxxxxxxxxx|
+ * | rxlow rxhigh
+ * -----------------------------------------> tx
+ * 0 tx fixed at 127
+ */
+
+ rxhigh.read_delay = rxlow.read_delay;
+ ret = cqspi_find_rx_high_sdr(f_pdata, mem, &rxhigh, rxlow.rx);
+ if (ret)
+ goto out;
+
+ /* Calculate first window midpoint for max margin */
+
+ /*
+ * rx
+ * 127 ^
+ * | |--------window1---------|
+ * | |xxxxxxxxxxxxxxxxxxxxxxxx|
+ * | |xxxxxxxxxxxxxxxxxxxxxxxx|
+ * | |xxxxxxxxxxx * xxxxxxxxxx|
+ * | |xxxxxxxxxxxxxxxxxxxxxxxx|
+ * | |xxxxxxxxxxxxxxxxxxxxxxxx|
+ * | rxlow ^ rxhigh
+ * ----------------------|------------------> tx
+ * 0 | tx fixed at 127
+ * window1/2
+ */
+
+ first.read_delay = rxlow.read_delay;
+ window1 = rxhigh.rx - rxlow.rx;
+ first.rx = rxlow.rx + (window1 / 2);
+
+ dev_dbg(dev, "First tuning point: RX: %d TX: %d RD: %d\n", first.rx,
+ first.tx, first.read_delay);
+ ret = cqspi_phy_apply_setting(f_pdata, &first);
+ if (!ret)
+ ret = cqspi_phy_check_pattern(f_pdata, mem);
+
+ if (ret || first.read_delay > CQSPI_PHY_MAX_RD)
+ goto out;
+
+ /* Second window: Search at read_delay+1, may differ in size */
+
+ /*
+ * rx
+ * 127 ^
+ * | |-------|
+ * | |xxxxxxx|
+ * | |xxxxxxx|
+ * | |xxxxxxx|
+ * | |xxxxxxx|
+ * | |xxxxxxx|
+ * | rxlow rxhigh
+ * -----------------------------------------> tx
+ * 0
+ * read_delay = n (smaller window)
+ *
+ * rx
+ * 127 ^
+ * | |-----------------|
+ * | |xxxxxxxxxxxxxxxxx|
+ * | |xxxxxxxxxxxxxxxxx|
+ * | |xxxxxxxxxxxxxxxxx|
+ * | |xxxxxxxxxxxxxxxxx|
+ * | |xxxxxxxxxxxxxxxxx|
+ * | rxlow rxhigh
+ * -----------------------------------------> tx
+ * 0
+ * read_delay = n+! (larger window - better)
+ */
+
+ cqspi_phy_reset_setting(&rxlow);
+ cqspi_phy_reset_setting(&rxhigh);
+ cqspi_phy_reset_setting(&second);
+
+ rxlow.read_delay = first.read_delay + 1;
+ if (rxlow.read_delay > CQSPI_PHY_MAX_RD)
+ goto compare;
+
+ ret = cqspi_find_rx_low_sdr(f_pdata, mem, &rxlow);
+ if (ret)
+ goto compare;
+
+ rxhigh.read_delay = rxlow.read_delay;
+ ret = cqspi_find_rx_high_sdr(f_pdata, mem, &rxhigh, rxlow.rx);
+ if (ret)
+ goto compare;
+
+ /* Calculate second window midpoint */
+
+ /*
+ * rx
+ * 127 ^
+ * | |--------window2---------|
+ * | |xxxxxxxxxxxxxxxxxxxxxxxx|
+ * | |xxxxxxxxxxxxxxxxxxxxxxxx|
+ * | |xxxxxxxxxxx * xxxxxxxxxx|
+ * | |xxxxxxxxxxxxxxxxxxxxxxxx|
+ * | |xxxxxxxxxxxxxxxxxxxxxxxx|
+ * | rxlow ^ rxhigh
+ * ----------------------|------------------> tx
+ * 0 | tx fixed at 127
+ * window1/2
+ * read_delay = n+1
+ */
+
+ window2 = rxhigh.rx - rxlow.rx;
+ second.rx = rxlow.rx + (window2 / 2);
+ second.read_delay = rxlow.read_delay;
+
+ dev_dbg(dev, "Second tuning point: RX: %d TX: %d RD: %d\n", second.rx,
+ second.tx, second.read_delay);
+ ret = cqspi_phy_apply_setting(f_pdata, &second);
+ if (!ret)
+ ret = cqspi_phy_check_pattern(f_pdata, mem);
+
+ if (ret || second.read_delay > CQSPI_PHY_MAX_RD)
+ window2 = 0;
+
+ /* Window comparison: Choose larger window for better margin */
+
+compare:
+ cqspi_phy_reset_setting(&final);
+ if (window2 > window1) {
+ final.rx = second.rx;
+ final.read_delay = second.read_delay;
+ } else {
+ final.rx = first.rx;
+ final.read_delay = first.read_delay;
+ }
+
+ /* Apply and verify final tuning point */
+
+ dev_dbg(dev, "Final tuning point: RX: %d TX: %d RD: %d\n", final.rx,
+ final.tx, final.read_delay);
+ ret = cqspi_phy_apply_setting(f_pdata, &final);
+ if (!ret)
+ ret = cqspi_phy_check_pattern(f_pdata, mem);
+
+ if (ret) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ f_pdata->phy_setting.read_delay = final.read_delay;
+ f_pdata->phy_setting.rx = final.rx;
+ f_pdata->phy_setting.tx = final.tx;
+
+out:
+ if (ret)
+ f_pdata->use_phy = false;
+
+ return ret;
}
static int cqspi_am654_ospi_execute_tuning(struct spi_mem *mem,
--
2.34.1
^ permalink raw reply related [flat|nested] 46+ messages in thread* Re: [RFC PATCH v2 10/12] spi: cadence-quadspi: implement PHY tuning algorithm
2026-01-13 14:16 ` [RFC PATCH v2 10/12] spi: cadence-quadspi: implement PHY tuning algorithm Santhosh Kumar K
@ 2026-02-05 17:42 ` Miquel Raynal
0 siblings, 0 replies; 46+ messages in thread
From: Miquel Raynal @ 2026-02-05 17:42 UTC (permalink / raw)
To: Santhosh Kumar K
Cc: broonie, robh, krzk+dt, conor+dt, richard, vigneshr,
tudor.ambarus, pratyush, mwalle, linux-spi, devicetree,
linux-kernel, linux-mtd, praneeth, u-kumar1, p-mantena, a-dutta
On 13/01/2026 at 19:46:15 +0530, Santhosh Kumar K <s-k6@ti.com> wrote:
> Implement PHY tuning for SDR and DDR modes. PHY tuning calibrates RX
> and TX delay lines to find optimal timing for high-speed operation.
>
> Add DLL management functions:
> - cqspi_resync_dll(): Reset DLL and wait for lock
> - cqspi_set_dll(): Configure RX/TX delays (0-127)
>
> Add pre/post config functions that enable PHY mode during tuning and
> restore normal operation afterward. PHY mode consumes one dummy cycle,
> so adjust dummy count to maintain correct flash timing.
>
> SDR tuning uses 1D search across RX delays at fixed TX. Search for
> two valid windows at consecutive read_delay values, select the larger
> window, and use the midpoint.
>
> DDR tuning uses 2D search across RX and TX delays:
> - Primary and secondary RX boundary searches at different TX values
> - Binary search for gap boundaries within valid region
> - Temperature compensation with midpoint calculation
> - Systematic boundary searches using 4-step increments
>
> The DDR algorithm finds the four corners of the valid region, identifies
> gaps, calculates temperature-aware midpoints, and validates final settings.
>
> Signed-off-by: Santhosh Kumar K <s-k6@ti.com>
This commit is gold, thanks a lot for the details in the comments.
I have no authority on this part to acknowledge it formally, but I
believe this is great work. Looking forward to see it work in octal DTR
mode now ;-)
Thanks,
Miquèl
^ permalink raw reply [flat|nested] 46+ messages in thread
* [RFC PATCH v2 11/12] spi: cadence-quadspi: restrict PHY frequency to tuned operations
2026-01-13 14:16 [RFC PATCH v2 00/12] spi: cadence-quadspi: add PHY tuning support Santhosh Kumar K
` (9 preceding siblings ...)
2026-01-13 14:16 ` [RFC PATCH v2 10/12] spi: cadence-quadspi: implement PHY tuning algorithm Santhosh Kumar K
@ 2026-01-13 14:16 ` Santhosh Kumar K
2026-02-05 17:47 ` Miquel Raynal
2026-01-13 14:16 ` [RFC PATCH v2 12/12] spi: cadence-quadspi: enable PHY for direct reads and writes Santhosh Kumar K
2026-02-04 10:29 ` [RFC PATCH v2 00/12] spi: cadence-quadspi: add PHY tuning support Miquel Raynal
12 siblings, 1 reply; 46+ messages in thread
From: Santhosh Kumar K @ 2026-01-13 14:16 UTC (permalink / raw)
To: broonie, robh, krzk+dt, conor+dt, miquel.raynal, richard,
vigneshr, tudor.ambarus, pratyush, mwalle
Cc: linux-spi, devicetree, linux-kernel, linux-mtd, praneeth,
u-kumar1, p-mantena, a-dutta, s-k6
PHY tuning calibrates timing delays for specific read and write commands
at high frequency. Using this frequency for uncalibrated operations
causes timing violations.
Apply PHY frequency only to operations that were tuned. All other
operations use the lower non-PHY frequency.
Signed-off-by: Santhosh Kumar K <s-k6@ti.com>
---
drivers/spi/spi-cadence-quadspi.c | 33 ++++++++++++++++++++++++++++++-
1 file changed, 32 insertions(+), 1 deletion(-)
diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
index 930ea094f6d8..3669936ae4e1 100644
--- a/drivers/spi/spi-cadence-quadspi.c
+++ b/drivers/spi/spi-cadence-quadspi.c
@@ -1532,13 +1532,44 @@ static ssize_t cqspi_read(struct cqspi_flash_pdata *f_pdata,
return cqspi_indirect_read_execute(f_pdata, buf, from, len);
}
+/*
+ * Check if operation exactly matches the tuned operations.
+ */
+static bool cqspi_op_matches_tuned(const struct spi_mem_op *op,
+ const struct spi_mem_op *tuned_op)
+{
+ return op->cmd.opcode == tuned_op->cmd.opcode &&
+ op->cmd.buswidth == tuned_op->cmd.buswidth &&
+ op->cmd.dtr == tuned_op->cmd.dtr &&
+ op->addr.buswidth == tuned_op->addr.buswidth &&
+ op->addr.dtr == tuned_op->addr.dtr &&
+ op->data.buswidth == tuned_op->data.buswidth &&
+ op->data.dtr == tuned_op->data.dtr;
+}
+
static int cqspi_mem_process(struct spi_mem *mem, const struct spi_mem_op *op)
{
struct cqspi_st *cqspi = spi_controller_get_devdata(mem->spi->controller);
struct cqspi_flash_pdata *f_pdata;
f_pdata = &cqspi->f_pdata[spi_get_chipselect(mem->spi, 0)];
- cqspi_configure(f_pdata, op->max_freq);
+
+ /*
+ * PHY tuning allows high-frequency operation only for calibrated
+ * commands. Uncalibrated operations use safe non-PHY frequency to
+ * avoid timing violations.
+ */
+ if (cqspi->ddata->execute_tuning && f_pdata->use_phy &&
+ (cqspi_op_matches_tuned(op, &f_pdata->phy_read_op) ||
+ cqspi_op_matches_tuned(op, &f_pdata->phy_write_op))) {
+ cqspi_configure(f_pdata, op->max_freq);
+ } else if (cqspi->ddata->execute_tuning) {
+ /* Use safe frequency for untuned operations */
+ cqspi_configure(f_pdata, f_pdata->non_phy_clk_rate);
+ } else {
+ /* No tuning support, always use requested frequency */
+ cqspi_configure(f_pdata, op->max_freq);
+ }
if (op->data.dir == SPI_MEM_DATA_IN && op->data.buf.in) {
/*
--
2.34.1
^ permalink raw reply related [flat|nested] 46+ messages in thread* Re: [RFC PATCH v2 11/12] spi: cadence-quadspi: restrict PHY frequency to tuned operations
2026-01-13 14:16 ` [RFC PATCH v2 11/12] spi: cadence-quadspi: restrict PHY frequency to tuned operations Santhosh Kumar K
@ 2026-02-05 17:47 ` Miquel Raynal
2026-02-06 19:27 ` Santhosh Kumar K
0 siblings, 1 reply; 46+ messages in thread
From: Miquel Raynal @ 2026-02-05 17:47 UTC (permalink / raw)
To: Santhosh Kumar K
Cc: broonie, robh, krzk+dt, conor+dt, richard, vigneshr,
tudor.ambarus, pratyush, mwalle, linux-spi, devicetree,
linux-kernel, linux-mtd, praneeth, u-kumar1, p-mantena, a-dutta
Hi Santhosh,
> + /*
> + * PHY tuning allows high-frequency operation only for calibrated
> + * commands. Uncalibrated operations use safe non-PHY frequency to
> + * avoid timing violations.
> + */
> + if (cqspi->ddata->execute_tuning && f_pdata->use_phy &&
> + (cqspi_op_matches_tuned(op, &f_pdata->phy_read_op) ||
> + cqspi_op_matches_tuned(op, &f_pdata->phy_write_op))) {
> + cqspi_configure(f_pdata, op->max_freq);
> + } else if (cqspi->ddata->execute_tuning) {
> + /* Use safe frequency for untuned operations */
> + cqspi_configure(f_pdata, f_pdata->non_phy_clk_rate);
> + } else {
> + /* No tuning support, always use requested frequency */
> + cqspi_configure(f_pdata, op->max_freq);
> + }
Shouldn't we handle this at the core level? We know what kind of
operation pattern we provided, so it is easy to set the correct
frequency in the operation structure.
Can you please make this happen? Perhaps you can return the operation
frequency once the calibration is successful (in the read and write op
templates maybe?) so this can be picked up by the core and used for the
following operations. This way the controller driver no longer needs to
check if the operation has been tuned or not, it can just look at the
frequency. When using the highest frequency, PHY tuning must be
used/enabled, otherwise not.
Thanks,
Miquèl
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [RFC PATCH v2 11/12] spi: cadence-quadspi: restrict PHY frequency to tuned operations
2026-02-05 17:47 ` Miquel Raynal
@ 2026-02-06 19:27 ` Santhosh Kumar K
2026-02-13 8:21 ` Miquel Raynal
0 siblings, 1 reply; 46+ messages in thread
From: Santhosh Kumar K @ 2026-02-06 19:27 UTC (permalink / raw)
To: Miquel Raynal
Cc: broonie, robh, krzk+dt, conor+dt, richard, vigneshr,
tudor.ambarus, pratyush, mwalle, linux-spi, devicetree,
linux-kernel, linux-mtd, praneeth, u-kumar1, p-mantena, a-dutta,
s-k6
On 05/02/26 23:17, Miquel Raynal wrote:
> Hi Santhosh,
>
>> + /*
>> + * PHY tuning allows high-frequency operation only for calibrated
>> + * commands. Uncalibrated operations use safe non-PHY frequency to
>> + * avoid timing violations.
>> + */
>> + if (cqspi->ddata->execute_tuning && f_pdata->use_phy &&
>> + (cqspi_op_matches_tuned(op, &f_pdata->phy_read_op) ||
>> + cqspi_op_matches_tuned(op, &f_pdata->phy_write_op))) {
>> + cqspi_configure(f_pdata, op->max_freq);
>> + } else if (cqspi->ddata->execute_tuning) {
>> + /* Use safe frequency for untuned operations */
>> + cqspi_configure(f_pdata, f_pdata->non_phy_clk_rate);
>> + } else {
>> + /* No tuning support, always use requested frequency */
>> + cqspi_configure(f_pdata, op->max_freq);
>> + }
>
> Shouldn't we handle this at the core level? We know what kind of
> operation pattern we provided, so it is easy to set the correct
> frequency in the operation structure.
>
> Can you please make this happen? Perhaps you can return the operation
> frequency once the calibration is successful (in the read and write op
> templates maybe?) so this can be picked up by the core and used for the
> following operations. This way the controller driver no longer needs to
> check if the operation has been tuned or not, it can just look at the
> frequency. When using the highest frequency, PHY tuning must be
> used/enabled, otherwise not.
No, Miquel, this may not be correct. There can be cases where an
operation does not require tuning but still can run at maximum
frequency (166 MHz, for instance). In such scenarios, simply setting
op->max_freq to the maximum frequency value and deciding whether to
enable tuning based on an op->max_freq comparison would not be
sufficient.
Regards,
Santhosh.
>
> Thanks,
> Miquèl
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [RFC PATCH v2 11/12] spi: cadence-quadspi: restrict PHY frequency to tuned operations
2026-02-06 19:27 ` Santhosh Kumar K
@ 2026-02-13 8:21 ` Miquel Raynal
2026-03-17 15:17 ` Miquel Raynal
0 siblings, 1 reply; 46+ messages in thread
From: Miquel Raynal @ 2026-02-13 8:21 UTC (permalink / raw)
To: Santhosh Kumar K
Cc: broonie, robh, krzk+dt, conor+dt, richard, vigneshr,
tudor.ambarus, pratyush, mwalle, linux-spi, devicetree,
linux-kernel, linux-mtd, praneeth, u-kumar1, p-mantena, a-dutta
On 07/02/2026 at 00:57:04 +0530, Santhosh Kumar K <s-k6@ti.com> wrote:
> On 05/02/26 23:17, Miquel Raynal wrote:
>> Hi Santhosh,
>>
>>> + /*
>>> + * PHY tuning allows high-frequency operation only for calibrated
>>> + * commands. Uncalibrated operations use safe non-PHY frequency to
>>> + * avoid timing violations.
>>> + */
>>> + if (cqspi->ddata->execute_tuning && f_pdata->use_phy &&
>>> + (cqspi_op_matches_tuned(op, &f_pdata->phy_read_op) ||
>>> + cqspi_op_matches_tuned(op, &f_pdata->phy_write_op))) {
>>> + cqspi_configure(f_pdata, op->max_freq);
>>> + } else if (cqspi->ddata->execute_tuning) {
>>> + /* Use safe frequency for untuned operations */
>>> + cqspi_configure(f_pdata, f_pdata->non_phy_clk_rate);
>>> + } else {
>>> + /* No tuning support, always use requested frequency */
>>> + cqspi_configure(f_pdata, op->max_freq);
>>> + }
>> Shouldn't we handle this at the core level? We know what kind of
>> operation pattern we provided, so it is easy to set the correct
>> frequency in the operation structure.
>> Can you please make this happen? Perhaps you can return the operation
>> frequency once the calibration is successful (in the read and write op
>> templates maybe?) so this can be picked up by the core and used for the
>> following operations. This way the controller driver no longer needs to
>> check if the operation has been tuned or not, it can just look at the
>> frequency. When using the highest frequency, PHY tuning must be
>> used/enabled, otherwise not.
>
> No, Miquel, this may not be correct. There can be cases where an
> operation does not require tuning but still can run at maximum
> frequency (166 MHz, for instance).
This is currently not the case. Currently you tune for one or two ops
(read/write) and you enable PHY tuning only on these. Do you plan on
adding such a feature? If not, I would not bother with this now.
> In such scenarios, simply setting
> op->max_freq to the maximum frequency value and deciding whether to
> enable tuning based on an op->max_freq comparison would not be
> sufficient.
If there are such cases, can they be listed? I am sorry but I fail to
see where this would not work. Any examples to share?
Thanks,
Miquèl
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [RFC PATCH v2 11/12] spi: cadence-quadspi: restrict PHY frequency to tuned operations
2026-02-13 8:21 ` Miquel Raynal
@ 2026-03-17 15:17 ` Miquel Raynal
0 siblings, 0 replies; 46+ messages in thread
From: Miquel Raynal @ 2026-03-17 15:17 UTC (permalink / raw)
To: Santhosh Kumar K
Cc: broonie, robh, krzk+dt, conor+dt, richard, vigneshr,
tudor.ambarus, pratyush, mwalle, linux-spi, devicetree,
linux-kernel, linux-mtd, praneeth, u-kumar1, p-mantena, a-dutta,
Frieder Schrempf, Eberhard Stoll
Hi Santhosh,
+ Frieder and Eberhard
On 13/02/2026 at 09:21:11 +01, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> On 07/02/2026 at 00:57:04 +0530, Santhosh Kumar K <s-k6@ti.com> wrote:
>
>> On 05/02/26 23:17, Miquel Raynal wrote:
>>> Hi Santhosh,
>>>
>>>> + /*
>>>> + * PHY tuning allows high-frequency operation only for calibrated
>>>> + * commands. Uncalibrated operations use safe non-PHY frequency to
>>>> + * avoid timing violations.
>>>> + */
>>>> + if (cqspi->ddata->execute_tuning && f_pdata->use_phy &&
>>>> + (cqspi_op_matches_tuned(op, &f_pdata->phy_read_op) ||
>>>> + cqspi_op_matches_tuned(op, &f_pdata->phy_write_op))) {
>>>> + cqspi_configure(f_pdata, op->max_freq);
>>>> + } else if (cqspi->ddata->execute_tuning) {
>>>> + /* Use safe frequency for untuned operations */
>>>> + cqspi_configure(f_pdata, f_pdata->non_phy_clk_rate);
>>>> + } else {
>>>> + /* No tuning support, always use requested frequency */
>>>> + cqspi_configure(f_pdata, op->max_freq);
>>>> + }
>>> Shouldn't we handle this at the core level? We know what kind of
>>> operation pattern we provided, so it is easy to set the correct
>>> frequency in the operation structure.
>>> Can you please make this happen? Perhaps you can return the operation
>>> frequency once the calibration is successful (in the read and write op
>>> templates maybe?) so this can be picked up by the core and used for the
>>> following operations. This way the controller driver no longer needs to
>>> check if the operation has been tuned or not, it can just look at the
>>> frequency. When using the highest frequency, PHY tuning must be
>>> used/enabled, otherwise not.
>>
>> No, Miquel, this may not be correct. There can be cases where an
>> operation does not require tuning but still can run at maximum
>> frequency (166 MHz, for instance).
>
> This is currently not the case. Currently you tune for one or two ops
> (read/write) and you enable PHY tuning only on these. Do you plan on
> adding such a feature? If not, I would not bother with this now.
>
>> In such scenarios, simply setting
>> op->max_freq to the maximum frequency value and deciding whether to
>> enable tuning based on an op->max_freq comparison would not be
>> sufficient.
>
> If there are such cases, can they be listed? I am sorry but I fail to
> see where this would not work. Any examples to share?
I don't know if you got my feedback, but I would like to have all cases
in mind to decide in which direction we must go. Especially, I would
like to make the bridge with Frieder's work who is also "playing" with
the maximum frequency.
We need to clarify our mental picture of the max_freq handling. How it
should be derived, how autonomous shall the SPI controllers be wrt this
value, shall we flag operations that can go faster and if yes, can we
attach a meaningful value to these operations, etc etc.
I feel like this is the part that needs extra thinking. The rest of the
series is promising. I would like us to clarify the needs, maybe propose
some kind of drawing/slides or even take half an hour to discuss in a
call once we have all cases in mind.
Thanks,
Miquèl
^ permalink raw reply [flat|nested] 46+ messages in thread
* [RFC PATCH v2 12/12] spi: cadence-quadspi: enable PHY for direct reads and writes
2026-01-13 14:16 [RFC PATCH v2 00/12] spi: cadence-quadspi: add PHY tuning support Santhosh Kumar K
` (10 preceding siblings ...)
2026-01-13 14:16 ` [RFC PATCH v2 11/12] spi: cadence-quadspi: restrict PHY frequency to tuned operations Santhosh Kumar K
@ 2026-01-13 14:16 ` Santhosh Kumar K
2026-02-05 17:51 ` Miquel Raynal
2026-02-04 10:29 ` [RFC PATCH v2 00/12] spi: cadence-quadspi: add PHY tuning support Miquel Raynal
12 siblings, 1 reply; 46+ messages in thread
From: Santhosh Kumar K @ 2026-01-13 14:16 UTC (permalink / raw)
To: broonie, robh, krzk+dt, conor+dt, miquel.raynal, richard,
vigneshr, tudor.ambarus, pratyush, mwalle
Cc: linux-spi, devicetree, linux-kernel, linux-mtd, praneeth,
u-kumar1, p-mantena, a-dutta, s-k6
Enable PHY mode for direct memory-mapped reads and large indirect writes
(>= 1KB) to leverage calibrated RX/TX timing delays for high-frequency
operations.
PHY mode requires 16-byte alignment. For reads, split unaligned transfers
into non-PHY head, PHY-enabled middle, and non-PHY tail. Small transfers
use CPU copy to avoid DMA setup overhead.
PHY mode requires one less dummy cycle due to improved timing margins.
Adjust dummy cycles when toggling PHY mode.
Add optimized I/O copy for 8D-8D-8D operations using 4-byte bulk reads
for 32-bit platform compatibility.
Signed-off-by: Santhosh Kumar K <s-k6@ti.com>
---
drivers/spi/spi-cadence-quadspi.c | 177 ++++++++++++++++++++++++++++--
1 file changed, 167 insertions(+), 10 deletions(-)
diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
index 3669936ae4e1..8ed6b2f5b573 100644
--- a/drivers/spi/spi-cadence-quadspi.c
+++ b/drivers/spi/spi-cadence-quadspi.c
@@ -565,6 +565,59 @@ static void cqspi_readdata_capture(struct cqspi_st *cqspi, const bool bypass,
writel(reg, reg_base + CQSPI_REG_READCAPTURE);
}
+static void cqspi_phy_enable(struct cqspi_flash_pdata *f_pdata, bool enable)
+{
+ struct cqspi_st *cqspi = f_pdata->cqspi;
+ void __iomem *reg_base = cqspi->iobase;
+ u32 reg;
+ u8 dummy;
+
+ if (enable) {
+ cqspi_readdata_capture(cqspi, true, f_pdata->has_dqs,
+ f_pdata->phy_setting.read_delay);
+
+ reg = readl(reg_base + CQSPI_REG_CONFIG);
+ reg |= CQSPI_REG_CONFIG_PHY_EN | CQSPI_REG_CONFIG_PHY_PIPELINE;
+ writel(reg, reg_base + CQSPI_REG_CONFIG);
+
+ /* PHY mode requires one less dummy cycle */
+ reg = readl(reg_base + CQSPI_REG_RD_INSTR);
+ dummy = FIELD_GET(CQSPI_REG_RD_INSTR_DUMMY_MASK
+ << CQSPI_REG_RD_INSTR_DUMMY_LSB,
+ reg);
+ dummy--;
+ reg &= ~(CQSPI_REG_RD_INSTR_DUMMY_MASK
+ << CQSPI_REG_RD_INSTR_DUMMY_LSB);
+ reg |= FIELD_PREP(CQSPI_REG_RD_INSTR_DUMMY_MASK
+ << CQSPI_REG_RD_INSTR_DUMMY_LSB,
+ dummy);
+ writel(reg, reg_base + CQSPI_REG_RD_INSTR);
+ } else {
+ cqspi_readdata_capture(cqspi, !cqspi->rclk_en, false,
+ f_pdata->read_delay);
+
+ reg = readl(reg_base + CQSPI_REG_CONFIG);
+ reg &= ~(CQSPI_REG_CONFIG_PHY_EN |
+ CQSPI_REG_CONFIG_PHY_PIPELINE);
+ writel(reg, reg_base + CQSPI_REG_CONFIG);
+
+ /* Restore original dummy cycle count */
+ reg = readl(reg_base + CQSPI_REG_RD_INSTR);
+ dummy = FIELD_GET(CQSPI_REG_RD_INSTR_DUMMY_MASK
+ << CQSPI_REG_RD_INSTR_DUMMY_LSB,
+ reg);
+ dummy++;
+ reg &= ~(CQSPI_REG_RD_INSTR_DUMMY_MASK
+ << CQSPI_REG_RD_INSTR_DUMMY_LSB);
+ reg |= FIELD_PREP(CQSPI_REG_RD_INSTR_DUMMY_MASK
+ << CQSPI_REG_RD_INSTR_DUMMY_LSB,
+ dummy);
+ writel(reg, reg_base + CQSPI_REG_RD_INSTR);
+ }
+
+ cqspi_wait_idle(cqspi);
+}
+
static int cqspi_exec_flash_cmd(struct cqspi_st *cqspi, unsigned int reg)
{
void __iomem *reg_base = cqspi->iobase;
@@ -1192,6 +1245,7 @@ static int cqspi_indirect_write_execute(struct cqspi_flash_pdata *f_pdata,
void __iomem *reg_base = cqspi->iobase;
unsigned int remaining = n_tx;
unsigned int write_bytes;
+ bool use_phy_write;
int ret;
if (!refcount_read(&cqspi->refcount))
@@ -1227,6 +1281,12 @@ static int cqspi_indirect_write_execute(struct cqspi_flash_pdata *f_pdata,
if (cqspi->apb_ahb_hazard)
readl(reg_base + CQSPI_REG_INDIRECTWR);
+ /* Use PHY only for large writes where setup overhead is amortized */
+ use_phy_write = n_tx >= SZ_1K && f_pdata->use_phy;
+
+ if (use_phy_write)
+ cqspi_phy_enable(f_pdata, true);
+
while (remaining > 0) {
size_t write_words, mod_bytes;
@@ -1267,6 +1327,9 @@ static int cqspi_indirect_write_execute(struct cqspi_flash_pdata *f_pdata,
goto failwr;
}
+ if (use_phy_write)
+ cqspi_phy_enable(f_pdata, false);
+
/* Disable interrupt. */
writel(0, reg_base + CQSPI_REG_IRQMASK);
@@ -1278,6 +1341,9 @@ static int cqspi_indirect_write_execute(struct cqspi_flash_pdata *f_pdata,
return 0;
failwr:
+ if (use_phy_write)
+ cqspi_phy_enable(f_pdata, false);
+
/* Disable interrupt. */
writel(0, reg_base + CQSPI_REG_IRQMASK);
@@ -1448,8 +1514,17 @@ static void cqspi_rx_dma_callback(void *param)
complete(&cqspi->rx_dma_complete);
}
-static int cqspi_direct_read_execute(struct cqspi_flash_pdata *f_pdata,
- u_char *buf, loff_t from, size_t len)
+static bool cqspi_use_phy(struct cqspi_flash_pdata *f_pdata,
+ const struct spi_mem_op *op)
+{
+ if (!f_pdata->use_phy || op->data.nbytes < 16)
+ return false;
+
+ return op->max_freq > f_pdata->non_phy_clk_rate;
+}
+
+static int cqspi_direct_read_dma(struct cqspi_flash_pdata *f_pdata, u_char *buf,
+ loff_t from, size_t len)
{
struct cqspi_st *cqspi = f_pdata->cqspi;
struct device *dev = &cqspi->pdev->dev;
@@ -1461,19 +1536,14 @@ static int cqspi_direct_read_execute(struct cqspi_flash_pdata *f_pdata,
dma_addr_t dma_dst;
struct device *ddev;
- if (!cqspi->rx_chan || !virt_addr_valid(buf)) {
- memcpy_fromio(buf, cqspi->ahb_base + from, len);
- return 0;
- }
-
ddev = cqspi->rx_chan->device->dev;
dma_dst = dma_map_single(ddev, buf, len, DMA_FROM_DEVICE);
if (dma_mapping_error(ddev, dma_dst)) {
dev_err(dev, "dma mapping failed\n");
return -ENOMEM;
}
- tx = dmaengine_prep_dma_memcpy(cqspi->rx_chan, dma_dst, dma_src,
- len, flags);
+ tx = dmaengine_prep_dma_memcpy(cqspi->rx_chan, dma_dst, dma_src, len,
+ flags);
if (!tx) {
dev_err(dev, "device_prep_dma_memcpy error\n");
ret = -EIO;
@@ -1507,6 +1577,93 @@ static int cqspi_direct_read_execute(struct cqspi_flash_pdata *f_pdata,
return ret;
}
+static void cqspi_memcpy_fromio(const struct spi_mem_op *op, void *to,
+ const void __iomem *from, size_t count)
+{
+ if (op->data.buswidth == 8 && op->data.dtr) {
+ unsigned long from_addr = (unsigned long)from;
+
+ /* Handle unaligned start with 2-byte read */
+ if (count && !IS_ALIGNED(from_addr, 4)) {
+ *(u16 *)to = __raw_readw(from);
+ from += 2;
+ to += 2;
+ count -= 2;
+ }
+
+ /* Use 4-byte reads for aligned bulk (no readq for 32-bit) */
+ if (count >= 4) {
+ size_t len = round_down(count, 4);
+
+ memcpy_fromio(to, from, len);
+ from += len;
+ to += len;
+ count -= len;
+ }
+
+ /* Handle remaining 2 bytes */
+ if (count)
+ *(u16 *)to = __raw_readw(from);
+
+ return;
+ }
+
+ memcpy_fromio(to, from, count);
+}
+
+static int cqspi_direct_read_execute(struct cqspi_flash_pdata *f_pdata,
+ const struct spi_mem_op *op)
+{
+ struct cqspi_st *cqspi = f_pdata->cqspi;
+ loff_t from = op->addr.val;
+ loff_t from_aligned, to_aligned;
+ size_t len = op->data.nbytes;
+ size_t len_aligned;
+ u_char *buf = op->data.buf.in;
+ int ret;
+
+ if (!cqspi->rx_chan || !virt_addr_valid(buf) || len <= 16) {
+ cqspi_memcpy_fromio(op, buf, cqspi->ahb_base + from, len);
+ return 0;
+ }
+
+ if (!cqspi_use_phy(f_pdata, op))
+ return cqspi_direct_read_dma(f_pdata, buf, from, len);
+
+ /* Split into unaligned head, aligned middle, unaligned tail */
+ from_aligned = ALIGN(from, 16);
+ to_aligned = ALIGN_DOWN(from + len, 16);
+ len_aligned = to_aligned - from_aligned;
+
+ if (from != from_aligned) {
+ ret = cqspi_direct_read_dma(f_pdata, buf, from,
+ from_aligned - from);
+ if (ret)
+ return ret;
+ buf += from_aligned - from;
+ }
+
+ if (len_aligned) {
+ cqspi_phy_enable(f_pdata, true);
+ ret = cqspi_direct_read_dma(f_pdata, buf, from_aligned,
+ len_aligned);
+ cqspi_phy_enable(f_pdata, false);
+ if (ret)
+ return ret;
+ buf += len_aligned;
+ }
+
+ if (to_aligned != (from + len)) {
+ ret = cqspi_direct_read_dma(f_pdata, buf, to_aligned,
+ (from + len) - to_aligned);
+ if (ret)
+ return ret;
+ buf += (from + len) - to_aligned;
+ }
+
+ return 0;
+}
+
static ssize_t cqspi_read(struct cqspi_flash_pdata *f_pdata,
const struct spi_mem_op *op)
{
@@ -1523,7 +1680,7 @@ static ssize_t cqspi_read(struct cqspi_flash_pdata *f_pdata,
return ret;
if (cqspi->use_direct_mode && ((from + len) <= cqspi->ahb_size))
- return cqspi_direct_read_execute(f_pdata, buf, from, len);
+ return cqspi_direct_read_execute(f_pdata, op);
if (cqspi->use_dma_read && ddata && ddata->indirect_read_dma &&
virt_addr_valid(buf) && ((dma_align & CQSPI_DMA_UNALIGN) == 0))
--
2.34.1
^ permalink raw reply related [flat|nested] 46+ messages in thread* Re: [RFC PATCH v2 12/12] spi: cadence-quadspi: enable PHY for direct reads and writes
2026-01-13 14:16 ` [RFC PATCH v2 12/12] spi: cadence-quadspi: enable PHY for direct reads and writes Santhosh Kumar K
@ 2026-02-05 17:51 ` Miquel Raynal
0 siblings, 0 replies; 46+ messages in thread
From: Miquel Raynal @ 2026-02-05 17:51 UTC (permalink / raw)
To: Santhosh Kumar K
Cc: broonie, robh, krzk+dt, conor+dt, richard, vigneshr,
tudor.ambarus, pratyush, mwalle, linux-spi, devicetree,
linux-kernel, linux-mtd, praneeth, u-kumar1, p-mantena, a-dutta
On 13/01/2026 at 19:46:17 +0530, Santhosh Kumar K <s-k6@ti.com> wrote:
> Enable PHY mode for direct memory-mapped reads and large indirect writes
> (>= 1KB) to leverage calibrated RX/TX timing delays for high-frequency
> operations.
Is enabling the calibration so impacting? Did you measure it? Isn't
skipping any non read or non write operations enough? Status reads for
example should have a smaller op->max_freq based on my previous comment,
so in theory I do not see when we will meet these cases.
Fine for the other conditions.
Miquèl
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [RFC PATCH v2 00/12] spi: cadence-quadspi: add PHY tuning support
2026-01-13 14:16 [RFC PATCH v2 00/12] spi: cadence-quadspi: add PHY tuning support Santhosh Kumar K
` (11 preceding siblings ...)
2026-01-13 14:16 ` [RFC PATCH v2 12/12] spi: cadence-quadspi: enable PHY for direct reads and writes Santhosh Kumar K
@ 2026-02-04 10:29 ` Miquel Raynal
2026-02-05 15:48 ` Miquel Raynal
12 siblings, 1 reply; 46+ messages in thread
From: Miquel Raynal @ 2026-02-04 10:29 UTC (permalink / raw)
To: Santhosh Kumar K
Cc: broonie, robh, krzk+dt, conor+dt, richard, vigneshr,
tudor.ambarus, pratyush, mwalle, linux-spi, devicetree,
linux-kernel, linux-mtd, praneeth, u-kumar1, p-mantena, a-dutta
Hi Santhosh,
On 13/01/2026 at 19:46:05 +0530, Santhosh Kumar K <s-k6@ti.com> wrote:
> This series implements PHY tuning support for the Cadence QSPI controller to
> enable reliable high-speed operations. Without PHY tuning, controllers use
> conservative timing that limits the performance. PHY tuning calibrates RX/TX
> delay lines to find optimal data capture timing windows, enabling operation up
> to the controller's maximum frequency.
>
> Background:
> High-speed SPI memory controllers require precise timing calibration for
> reliable operation. At higher frequencies, board-to-board variations make
> fixed timing parameters inadequate. The Cadence QSPI controller includes
> a PHY interface with programmable delay lines (0-127 taps) for RX and TX
> paths, but these require runtime calibration to find the valid timing window.
>
> Approach:
> Add SDR/DDR PHY tuning algorithms for the Cadence controller:
>
> SDR Mode Tuning (1D search):
> - Searches for two consecutive valid RX delay windows
> - Selects the larger window and uses its midpoint for maximum margin
> - TX delay fixed at maximum (127) as it's less critical in SDR
>
> DDR Mode Tuning (2D search):
> - Finds RX boundaries (rxlow/rxhigh) using TX window sweeps
> - Finds TX boundaries (txlow/txhigh) at fixed RX positions
> - Defines valid region corners and detects gaps via binary search
> - Applies temperature compensation for optimal point selection
> - Handles single or dual passing regions with different strategies
>
> DQS Support:
> - Adds optional DQS (Data Strobe) mode for improved timing margins
> - Configures read data capture to use dedicated strobe signal
I am glad to know this signal is useful. I do not consider the DT
property as being the correct way to carry this information ATM, so I
will investigate a bit a propose a solution that is more uniform with
the rest of the chips description we have today.
> Patch description:
> Infrastructure (1-5):
> - Patch 1: Add DT binding for spi-has-dqs property
> - Patch 2: Implement spi_mem_execute_tuning() API in SPI core
> - Patch 3-5: Refactor and integrate tuning in MTD SPI-NAND/NOR layers and call
> tuning during probe
>
> Cadence QSPI Implementation (6-12):
> - Patch 6-8: Preparatory refactoring and DQS support
> - Patch 9: Add PHY tuning infrastructure with placeholders
> - Patch 10: Implement complete SDR/DDR tuning algorithms
> - Patch 11: Restrict PHY frequency to calibrated operations only
> - Patch 12: Enable PHY for direct memory-mapped reads and large writes
>
> Testing:
> This series was tested on TI's
> AM62A SK with OSPI NAND flash and
> AM62P SK with OSPI NOR flash:
>
> Read throughput:
> |-------------------------------------|
> | | without PHY | with PHY |
> |-------------------------------------|
> |OSPI NOR | 37.5 MB/s | 216 MB/s |
> |-------------------------------------|
> |OSPI NAND | 9.2 MB/s | 35.1 MB/s |
> |-------------------------------------|
I am surprised by these numbers, I would expect these to get higher for
SPI NANDs. I will test the series and report my observations, especially
since there is also ODDR SPI NAND support now (in nand/next, should be
part of my upcoming merge request to Linus for 6.19+1);
> Write throughput:
> |-------------------------------------|
> | | without PHY | with PHY |
> |-------------------------------------|
> |OSPI NAND | 6 MB/s | 9.2 MB/s |
> |-------------------------------------|
Overall I want to say that this series has greatly improved already, I
am really looking forward seeing this merged. I have several comments to
make, but they are mostly minor improvements which won't be very
impacting.
The tuning procedure is very well described in the code as well, which
is appreciated.
Please remove the RFC prefix for v3, it is clearly no longer needed.
Thanks,
Miquèl
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [RFC PATCH v2 00/12] spi: cadence-quadspi: add PHY tuning support
2026-02-04 10:29 ` [RFC PATCH v2 00/12] spi: cadence-quadspi: add PHY tuning support Miquel Raynal
@ 2026-02-05 15:48 ` Miquel Raynal
2026-02-06 19:28 ` Santhosh Kumar K
0 siblings, 1 reply; 46+ messages in thread
From: Miquel Raynal @ 2026-02-05 15:48 UTC (permalink / raw)
To: Santhosh Kumar K
Cc: broonie, robh, krzk+dt, conor+dt, richard, vigneshr,
tudor.ambarus, pratyush, mwalle, linux-spi, devicetree,
linux-kernel, linux-mtd, praneeth, u-kumar1, p-mantena, a-dutta
Hi Santhosh,
> I am surprised by these numbers, I would expect these to get higher for
> SPI NANDs. I will test the series and report my observations, especially
> since there is also ODDR SPI NAND support now (in nand/next, should be
> part of my upcoming merge request to Linus for 6.19+1);
I just tested the series, here are some numbers I grabbed on TI AM62A7
LP SK with a Winbond W35N02 SPI NAND chip (so in the end very close to
your report):
+-----------------+-----------+------------+
| SPI NAND | no tuning | PHY tuning |
| Unit: MiB/s | 25MHz | 166MHz |
|-----------------+-----------+------------|
| Octal SDR read | 13.8 | 34.2 |
| write | 7.2 | 10.2 |
|-----------------+-----------+------------|
| Octal DTR read | 21.2 | N/A |
| write | 9.0 | N/A |
+-----------------+-----------+------------+
Please mind I used MiB/s and not MB/s (so kiB / 1024), I don't know
which one you used for measuring, as you marked MB, whereas the most
common unit seems to be MiB.
However PHY tuning failed in Octal DTR mode (your series applied on top
of nand/next) with the following logs, can you have a look?
[ 2.261647] spi-nand spi0.0: Winbond SPI NAND was found.
[ 2.266956] spi-nand spi0.0: 128 MiB, block size: 256 KiB, page size: 4096, OOB size: 128
[ 2.285257] cadence-qspi fc40000.spi: PHY tuning failed: -2
[ 2.290835] spi-nand spi0.0: Failed to execute PHY tuning: -2
The fallback worked well though, the memory was still usable like
before, which is a very good point.
Thanks,
Miquèl
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [RFC PATCH v2 00/12] spi: cadence-quadspi: add PHY tuning support
2026-02-05 15:48 ` Miquel Raynal
@ 2026-02-06 19:28 ` Santhosh Kumar K
2026-02-13 9:01 ` Miquel Raynal
0 siblings, 1 reply; 46+ messages in thread
From: Santhosh Kumar K @ 2026-02-06 19:28 UTC (permalink / raw)
To: Miquel Raynal
Cc: broonie, robh, krzk+dt, conor+dt, richard, vigneshr,
tudor.ambarus, pratyush, mwalle, linux-spi, devicetree,
linux-kernel, linux-mtd, praneeth, u-kumar1, p-mantena, a-dutta,
s-k6
On 05/02/26 21:18, Miquel Raynal wrote:
> Hi Santhosh,
>
>> I am surprised by these numbers, I would expect these to get higher for
>> SPI NANDs. I will test the series and report my observations, especially
>> since there is also ODDR SPI NAND support now (in nand/next, should be
>> part of my upcoming merge request to Linus for 6.19+1);
>
> I just tested the series, here are some numbers I grabbed on TI AM62A7
> LP SK with a Winbond W35N02 SPI NAND chip (so in the end very close to
> your report):
>
> +-----------------+-----------+------------+
> | SPI NAND | no tuning | PHY tuning |
> | Unit: MiB/s | 25MHz | 166MHz |
> |-----------------+-----------+------------|
> | Octal SDR read | 13.8 | 34.2 |
> | write | 7.2 | 10.2 |
> |-----------------+-----------+------------|
> | Octal DTR read | 21.2 | N/A |
> | write | 9.0 | N/A |
> +-----------------+-----------+------------+
>
> Please mind I used MiB/s and not MB/s (so kiB / 1024), I don't know
> which one you used for measuring, as you marked MB, whereas the most
> common unit seems to be MiB.
>
> However PHY tuning failed in Octal DTR mode (your series applied on top
> of nand/next) with the following logs, can you have a look?
>
> [ 2.261647] spi-nand spi0.0: Winbond SPI NAND was found.
> [ 2.266956] spi-nand spi0.0: 128 MiB, block size: 256 KiB, page size: 4096, OOB size: 128
> [ 2.285257] cadence-qspi fc40000.spi: PHY tuning failed: -2
> [ 2.290835] spi-nand spi0.0: Failed to execute PHY tuning: -2
Unfortunately, due to a known erratum in the Cadence controller, PHY DDR
mode cannot be used with 2-byte addressing.
Refer:
Errata i2383: OSPI: 2-byte address is not supported in PHY DDR mode [1]
As a result, the Cadence controller supports only the following
operating modes:
- PHY DDR mode with 4-byte addressing
- PHY SDR mode
- TAP (non-PHY) DDR mode
- TAP (non-PHY) SDR mode
[1]
https://www.ti.com/lit/er/sprz544c/sprz544c.pdf?ts=1770404630843&ref_url=https%253A%252F%252Fwww.google.com%252F
Regards,
Santhosh.
>
> The fallback worked well though, the memory was still usable like
> before, which is a very good point.
>
> Thanks,
> Miquèl
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [RFC PATCH v2 00/12] spi: cadence-quadspi: add PHY tuning support
2026-02-06 19:28 ` Santhosh Kumar K
@ 2026-02-13 9:01 ` Miquel Raynal
2026-02-18 18:08 ` Santhosh Kumar K
0 siblings, 1 reply; 46+ messages in thread
From: Miquel Raynal @ 2026-02-13 9:01 UTC (permalink / raw)
To: Santhosh Kumar K
Cc: broonie, robh, krzk+dt, conor+dt, richard, vigneshr,
tudor.ambarus, pratyush, mwalle, linux-spi, devicetree,
linux-kernel, linux-mtd, praneeth, u-kumar1, p-mantena, a-dutta
On 07/02/2026 at 00:58:03 +0530, Santhosh Kumar K <s-k6@ti.com> wrote:
> On 05/02/26 21:18, Miquel Raynal wrote:
>> Hi Santhosh,
>>
>>> I am surprised by these numbers, I would expect these to get higher for
>>> SPI NANDs. I will test the series and report my observations, especially
>>> since there is also ODDR SPI NAND support now (in nand/next, should be
>>> part of my upcoming merge request to Linus for 6.19+1);
>> I just tested the series, here are some numbers I grabbed on TI AM62A7
>> LP SK with a Winbond W35N02 SPI NAND chip (so in the end very close to
>> your report):
>> +-----------------+-----------+------------+
>> | SPI NAND | no tuning | PHY tuning |
>> | Unit: MiB/s | 25MHz | 166MHz |
>> |-----------------+-----------+------------|
>> | Octal SDR read | 13.8 | 34.2 |
>> | write | 7.2 | 10.2 |
>> |-----------------+-----------+------------|
>> | Octal DTR read | 21.2 | N/A |
>> | write | 9.0 | N/A |
>> +-----------------+-----------+------------+
>> Please mind I used MiB/s and not MB/s (so kiB / 1024), I don't know
>> which one you used for measuring, as you marked MB, whereas the most
>> common unit seems to be MiB.
>> However PHY tuning failed in Octal DTR mode (your series applied on
>> top
>> of nand/next) with the following logs, can you have a look?
>> [ 2.261647] spi-nand spi0.0: Winbond SPI NAND was found.
>> [ 2.266956] spi-nand spi0.0: 128 MiB, block size: 256 KiB, page size: 4096, OOB size: 128
>> [ 2.285257] cadence-qspi fc40000.spi: PHY tuning failed: -2
>> [ 2.290835] spi-nand spi0.0: Failed to execute PHY tuning: -2
>
> Unfortunately, due to a known erratum in the Cadence controller, PHY DDR
> mode cannot be used with 2-byte addressing.
>
> Refer:
> Errata i2383: OSPI: 2-byte address is not supported in PHY DDR mode [1]
>
> As a result, the Cadence controller supports only the following
> operating modes:
> - PHY DDR mode with 4-byte addressing
> - PHY SDR mode
> - TAP (non-PHY) DDR mode
> - TAP (non-PHY) SDR mode
I do not think we have 4-byte addressing capabilities on SPI NAND chips,
esp. Winbond's chips. So there is a down side: the core will pick-up
Octal DTR modes rather than Octal SDR (with PHY) mode, which is not the
fastest mode. Maybe we can guess that once we have access to the max
(tuned PHY) spi frequency, with an extra flag in the driver indicating
that the PHY speed is not accessible in DTR mode. But this again
requires different handling between SPI NAND and SPI NOR, as SPI NOR
IIRC may have 4-byte addressing capabilities.
Thanks,
Miquèl
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [RFC PATCH v2 00/12] spi: cadence-quadspi: add PHY tuning support
2026-02-13 9:01 ` Miquel Raynal
@ 2026-02-18 18:08 ` Santhosh Kumar K
0 siblings, 0 replies; 46+ messages in thread
From: Santhosh Kumar K @ 2026-02-18 18:08 UTC (permalink / raw)
To: Miquel Raynal
Cc: broonie, robh, krzk+dt, conor+dt, richard, vigneshr,
tudor.ambarus, pratyush, mwalle, linux-spi, devicetree,
linux-kernel, linux-mtd, praneeth, u-kumar1, p-mantena, a-dutta,
s-k6
On 13/02/26 14:31, Miquel Raynal wrote:
> On 07/02/2026 at 00:58:03 +0530, Santhosh Kumar K <s-k6@ti.com> wrote:
>
>> On 05/02/26 21:18, Miquel Raynal wrote:
>>> Hi Santhosh,
>>>
>>>> I am surprised by these numbers, I would expect these to get higher for
>>>> SPI NANDs. I will test the series and report my observations, especially
>>>> since there is also ODDR SPI NAND support now (in nand/next, should be
>>>> part of my upcoming merge request to Linus for 6.19+1);
>>> I just tested the series, here are some numbers I grabbed on TI AM62A7
>>> LP SK with a Winbond W35N02 SPI NAND chip (so in the end very close to
>>> your report):
>>> +-----------------+-----------+------------+
>>> | SPI NAND | no tuning | PHY tuning |
>>> | Unit: MiB/s | 25MHz | 166MHz |
>>> |-----------------+-----------+------------|
>>> | Octal SDR read | 13.8 | 34.2 |
>>> | write | 7.2 | 10.2 |
>>> |-----------------+-----------+------------|
>>> | Octal DTR read | 21.2 | N/A |
>>> | write | 9.0 | N/A |
>>> +-----------------+-----------+------------+
>>> Please mind I used MiB/s and not MB/s (so kiB / 1024), I don't know
>>> which one you used for measuring, as you marked MB, whereas the most
>>> common unit seems to be MiB.
>>> However PHY tuning failed in Octal DTR mode (your series applied on
>>> top
>>> of nand/next) with the following logs, can you have a look?
>>> [ 2.261647] spi-nand spi0.0: Winbond SPI NAND was found.
>>> [ 2.266956] spi-nand spi0.0: 128 MiB, block size: 256 KiB, page size: 4096, OOB size: 128
>>> [ 2.285257] cadence-qspi fc40000.spi: PHY tuning failed: -2
>>> [ 2.290835] spi-nand spi0.0: Failed to execute PHY tuning: -2
>>
>> Unfortunately, due to a known erratum in the Cadence controller, PHY DDR
>> mode cannot be used with 2-byte addressing.
>>
>> Refer:
>> Errata i2383: OSPI: 2-byte address is not supported in PHY DDR mode [1]
>>
>> As a result, the Cadence controller supports only the following
>> operating modes:
>> - PHY DDR mode with 4-byte addressing
>> - PHY SDR mode
>> - TAP (non-PHY) DDR mode
>> - TAP (non-PHY) SDR mode
>
> I do not think we have 4-byte addressing capabilities on SPI NAND chips,
> esp. Winbond's chips. So there is a down side: the core will pick-up
> Octal DTR modes rather than Octal SDR (with PHY) mode, which is not the
> fastest mode. Maybe we can guess that once we have access to the max
> (tuned PHY) spi frequency, with an extra flag in the driver indicating
> that the PHY speed is not accessible in DTR mode. But this again
> requires different handling between SPI NAND and SPI NOR, as SPI NOR
> IIRC may have 4-byte addressing capabilities.
I'm currently handling this in the Cadence controller's supports_op()
callback, and it seems to be working as expected.
If a DDR operation is passed with addr.nbytes <= 2 and PHY tuning
support is available, return false. I'll include this change in v3.
Regards,
Santhosh.
>
> Thanks,
> Miquèl
^ permalink raw reply [flat|nested] 46+ messages in thread