[PATCH 0/3] spi: Better specification for rx-sample-delay-ns and core parsing

[PATCH 0/3] spi: Better specification for rx-sample-delay-ns and core parsing

[Frieder Schrempf]

"rx-sample-delay-ns" has been a generic SPI peripheral property since it
was moved to spi-peripheral-props.yaml, but the core has never looked at
it, and what it is meant to describe has become blurred along the way.

It was introduced in 2015 for Rockchip, to compensate "boards with
high-capacitance SPI lines". The wording now in the generic schema came
later, from a description of a DesignWare register, and lost its
controller-specific qualifier on the way. It says what the value does to
the controller, but not what a board should measure to arrive at it.

Patch 1 says what the property describes. Patch 2 parses it in the core,
so that a controller driver can act on it without open-coding the
property name, and patch 3 converts spi-dw, its only user that reads it
from the peripheral node as the binding intends.

Deliberately not converted: spi-rockchip and spi-mtk-snfi read
"rx-sample-delay-ns" from the *controller* node rather than the
peripheral node, which contradicts the binding but is what their device
trees rely on. Converting them would break those boards, so they keep
their private parsing. The controller-wide default that spi-dw reads
from its own node is left in place for the same reason - it lives in a
different node, which is not what the core parses when it looks at a
peripheral.

The one behavioural corner is in patch 3 and is called out there: an
explicit "rx-sample-delay-ns = <0>" on a peripheral is now
indistinguishable from an absent property, so it no longer overrides a
non-zero controller-level default. There are two users of the property
in the tree and neither does this.

This is groundwork for letting SPI devices declare their datasheet
clock-to-output-valid time so that controllers can move their sampling
point instead of forcing a lower spi-max-frequency, posted as an RFC at

  https://lore.kernel.org/r/20260303-fsl-qspi-rx-sampling-delay-v1-0-9326bbc492d6@kontron.de

Nothing in that work is needed to read this series, and nothing here
depends on it: the chip side is a separate quantity that composes with
this one, which is why patch 1 spends a paragraph on keeping them apart.

Assisted-by: Claude:claude-opus-5
Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
---
Frieder Schrempf (3):
      dt-bindings: spi: Clarify what rx-sample-delay-ns describes
      spi: Parse the rx-sample-delay-ns peripheral property in the core
      spi: dw: Use the rx-sample-delay-ns value parsed by the core

 .../devicetree/bindings/spi/spi-peripheral-props.yaml       |  8 ++++++++
 drivers/spi/spi-dw-core.c                                   | 13 +++++++------
 drivers/spi/spi.c                                           |  9 +++++++++
 include/linux/spi/spi.h                                     |  9 +++++++++
 4 files changed, 33 insertions(+), 6 deletions(-)
---
base-commit: 238650ef6c7c7cca08e032527329424c9fbd70e5
change-id: 20260917-spi-sample-delay-cleanup-ab1d03ae02fa

Best regards,
--  
Frieder Schrempf <frieder.schrempf@kontron.de>

[PATCH 1/3] dt-bindings: spi: Clarify what rx-sample-delay-ns describes

[Frieder Schrempf]

From: Frieder Schrempf <frieder.schrempf@kontron.de>

The property is documented as an offset from the controller's default
sampling time. That says what the value does, but not what it is meant to
describe, and it leaves room for two readings: a delay that belongs to
the board, or a stand-in for the peripheral's own clock-to-output-valid
time, which has no other expression in a device tree today.

The first reading is the original one. The property was introduced for
Rockchip in commit 76b17e6e4923 ("spi/rockchip: Add device tree property
to configure Rx Sample Delay") to deal with "boards with high-capacitance
SPI lines", where "the controller samples the Rx data line too early".
The current wording arrived later, in commit 5ce78f4456a9 ("dt-bindings:
snps, dw-apb-ssi: Add sparx5 support, plus rx-sample-delay-ns property"),
where it described a DesignWare register and was qualified as such. That
qualification was dropped when the property was moved to the generic
schema in commit b658be56e867 ("spi: dt-bindings: Move
'rx-sample-delay-ns' to spi-peripheral-props.yaml"), leaving a register
description standing in for a definition.

The distinction matters because the two compose. A board delay is
specific to one design, while a datasheet timing parameter is the same
wherever the chip is soldered, so a mechanism that derives the chip side
from the chip would add to a value that already accounts for it.

Spell out that the property describes the board. Nothing changes in what
the value means or in how existing device trees are interpreted.

Assisted-by: Claude:claude-opus-5
Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
---
 Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml b/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
index 880a9f624566..afcf9c41c058 100644
--- a/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
+++ b/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
@@ -91,6 +91,14 @@ properties:
       The delay from the default sample time before the actual
       sample of the rxd input signal occurs.
 
+      This describes the board rather than the peripheral, namely the extra
+      delay this particular design needs, for example because of the flight time
+      of the clock and data signals between controller and peripheral.
+
+      Timing parameters of the peripheral itself, such as its
+      clock-to-output-valid time, are the same on every board using that chip
+      and should be described with the chip.
+
   spi-tx-bus-width:
     description:
       Bus width to the SPI bus used for write transfers.

-- 
2.55.0

[PATCH 2/3] spi: Parse the rx-sample-delay-ns peripheral property in the core

[Frieder Schrempf]

From: Frieder Schrempf <frieder.schrempf@kontron.de>

"rx-sample-delay-ns" is documented in spi-peripheral-props.yaml as a
generic SPI peripheral property, but the core never looks at it. Only
spi-dw reads it from the peripheral node as the binding describes, and
any other driver wanting to act on it would have to duplicate that.

Add spi_device.rx_sample_delay_ns and parse the property once, in
__spi_add_device(), which covers every way a device can be instantiated
and runs before ->setup().

Use device_property_read_u32() rather than adding this to
of_spi_parse_dt(). spi-dw already uses the fwnode accessor, so the
property works today on ACPI and software node platforms such as the
ones served by spi-dw-pci, and parsing it only in the device tree path
would quietly drop it there.

No functional change, since nothing reads the new field yet.

Assisted-by: Claude:claude-opus-5
Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
---
 drivers/spi/spi.c       | 9 +++++++++
 include/linux/spi/spi.h | 9 +++++++++
 2 files changed, 18 insertions(+)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 5b5b3bc5f0d8..9e3c24ed8f9f 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -929,6 +929,15 @@ static int __spi_add_device(struct spi_device *spi, struct spi_device *parent)
 		}
 	}
 
+	/*
+	 * Peripheral properties the core handles on behalf of controller
+	 * drivers are parsed here, rather than in the firmware specific
+	 * instantiation paths, so that device tree, ACPI and software nodes
+	 * are covered alike, and early enough for ->setup() to act on them.
+	 */
+	device_property_read_u32(&spi->dev, "rx-sample-delay-ns",
+				 &spi->rx_sample_delay_ns);
+
 	/*
 	 * Drivers may modify this initial i/o setup, but will
 	 * normally rely on the device being setup.  Devices
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index 88d17fce02dc..fb4baa0d3398 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -169,6 +169,12 @@ extern void spi_transfer_cs_change_delay_exec(struct spi_message *msg,
  * @cs_inactive: delay to be introduced by the controller after CS is
  *	deasserted. If @cs_change_delay is used from @spi_transfer, then the
  *	two delays will be added up.
+ * @rx_sample_delay_ns: Delay in nanoseconds by which the controller should
+ *	postpone sampling the incoming data, relative to the sampling point it
+ *	uses by default. Describes the board rather than the device, namely the
+ *	flight time of the clock and data signals between controller and
+ *	device, and comes from the "rx-sample-delay-ns" property. Zero when the
+ *	property is absent.
  * @chip_select: Array of physical chipselect, spi->chipselect[i] gives
  *	the corresponding physical CS for logical CS i.
  * @num_chipselect: Number of physical chipselects used.
@@ -235,6 +241,9 @@ struct spi_device {
 	struct spi_delay	cs_hold;
 	struct spi_delay	cs_inactive;
 
+	/* Additional delay before the incoming data is sampled, in ns */
+	u32			rx_sample_delay_ns;
+
 	u8			chip_select[SPI_DEVICE_CS_CNT_MAX];
 	u8			num_chipselect;
 

-- 
2.55.0

[PATCH 3/3] spi: dw: Use the rx-sample-delay-ns value parsed by the core

[Frieder Schrempf]

From: Frieder Schrempf <frieder.schrempf@kontron.de>

The core now parses the "rx-sample-delay-ns" peripheral property into
spi_device.rx_sample_delay_ns, so drop the private copy of that parsing.

The controller-wide default is deliberately left alone. It is read from
the *controller* node into dws->def_rx_sample_dly_ns, which is a
different node and none of the core's business when it parses properties
of a peripheral.

One corner case changes: the peripheral value is now treated as unset
when it is zero, whereas before an absent property could be told apart
from an explicit "rx-sample-delay-ns = <0>", the latter overriding a
non-zero controller default with no delay at all. There are two users of
the property in the whole tree and neither does this, and zero means the
same as unset for every other user of the new field.

Assisted-by: Claude:claude-opus-5
Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
---
 drivers/spi/spi-dw-core.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
index 206d3f9dd83d..b86b607cc817 100644
--- a/drivers/spi/spi-dw-core.c
+++ b/drivers/spi/spi-dw-core.c
@@ -1104,12 +1104,13 @@ static int dw_spi_setup(struct spi_device *spi)
 		if (!chip)
 			return -ENOMEM;
 		spi_set_ctldata(spi, chip);
-		/* Get specific / default rx-sample-delay */
-		if (device_property_read_u32(&spi->dev,
-					     "rx-sample-delay-ns",
-					     &rx_sample_dly_ns) != 0)
-			/* Use default controller value */
-			rx_sample_dly_ns = dws->def_rx_sample_dly_ns;
+		/*
+		 * Use the per-device value the core parsed from the peripheral
+		 * node, and fall back to the controller-wide default when the
+		 * device does not ask for a delay of its own.
+		 */
+		rx_sample_dly_ns = spi->rx_sample_delay_ns ?:
+				   dws->def_rx_sample_dly_ns;
 		chip->rx_sample_dly = DIV_ROUND_CLOSEST(rx_sample_dly_ns,
 							NSEC_PER_SEC /
 							dws->max_freq);

-- 
2.55.0

Re: [PATCH 1/3] dt-bindings: spi: Clarify what rx-sample-delay-ns describes

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 614 bytes --]

On Thu, Sep 17, 2026 at 05:37:35PM +0200, Frieder Schrempf wrote:
> From: Frieder Schrempf <frieder.schrempf@kontron.de>
> 
> The property is documented as an offset from the controller's default
> sampling time. That says what the value does, but not what it is meant to

Please submit patches using subject lines reflecting the style for the
subsystem, this makes it easier for people to identify relevant patches.
Look at what existing commits in the area you're changing are doing and
make sure your subject lines visually resemble what they're doing.
There's no need to resubmit to fix this alone.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 1/3] dt-bindings: spi: Clarify what rx-sample-delay-ns describes

[Frieder Schrempf]

On 18.09.26 00:23, Mark Brown wrote:
> On Thu, Sep 17, 2026 at 05:37:35PM +0200, Frieder Schrempf wrote:
>> From: Frieder Schrempf <frieder.schrempf@kontron.de>
>>
>> The property is documented as an offset from the controller's default
>> sampling time. That says what the value does, but not what it is meant to
> 
> Please submit patches using subject lines reflecting the style for the
> subsystem, this makes it easier for people to identify relevant patches.
> Look at what existing commits in the area you're changing are doing and
> make sure your subject lines visually resemble what they're doing.
> There's no need to resubmit to fix this alone.

Sorry, fixed in v2. But I have to mention that this mix of "dt-bindings:
subsystem:" vs. "subsystem: dt-bindings:" across subsystems is somewhat
annoying for contributors. Has anyone ever thought about aligning this
across subsystems?