* [PATCH 1/3] dt-bindings: spi: Clarify what rx-sample-delay-ns describes
2026-09-17 15:37 [PATCH 0/3] spi: Better specification for rx-sample-delay-ns and core parsing Frieder Schrempf
@ 2026-09-17 15:37 ` Frieder Schrempf
2026-09-17 22:23 ` Mark Brown
2026-09-17 15:37 ` [PATCH 2/3] spi: Parse the rx-sample-delay-ns peripheral property in the core Frieder Schrempf
2026-09-17 15:37 ` [PATCH 3/3] spi: dw: Use the rx-sample-delay-ns value parsed by " Frieder Schrempf
2 siblings, 1 reply; 6+ messages in thread
From: Frieder Schrempf @ 2026-09-17 15:37 UTC (permalink / raw)
To: Mark Brown, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Heiko Stuebner
Cc: linux-spi, devicetree, linux-kernel, linux-arm-kernel,
linux-rockchip, 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
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 1/3] dt-bindings: spi: Clarify what rx-sample-delay-ns describes
2026-09-17 15:37 ` [PATCH 1/3] dt-bindings: spi: Clarify what rx-sample-delay-ns describes Frieder Schrempf
@ 2026-09-17 22:23 ` Mark Brown
2026-09-21 10:50 ` Frieder Schrempf
0 siblings, 1 reply; 6+ messages in thread
From: Mark Brown @ 2026-09-17 22:23 UTC (permalink / raw)
To: Frieder Schrempf
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner,
linux-spi, devicetree, linux-kernel, linux-arm-kernel,
linux-rockchip, Frieder Schrempf
[-- 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 --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/3] dt-bindings: spi: Clarify what rx-sample-delay-ns describes
2026-09-17 22:23 ` Mark Brown
@ 2026-09-21 10:50 ` Frieder Schrempf
0 siblings, 0 replies; 6+ messages in thread
From: Frieder Schrempf @ 2026-09-21 10:50 UTC (permalink / raw)
To: Mark Brown, Frieder Schrempf
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner,
linux-spi, devicetree, linux-kernel, linux-arm-kernel,
linux-rockchip
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?
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/3] spi: Parse the rx-sample-delay-ns peripheral property in the core
2026-09-17 15:37 [PATCH 0/3] spi: Better specification for rx-sample-delay-ns and core parsing Frieder Schrempf
2026-09-17 15:37 ` [PATCH 1/3] dt-bindings: spi: Clarify what rx-sample-delay-ns describes Frieder Schrempf
@ 2026-09-17 15:37 ` Frieder Schrempf
2026-09-17 15:37 ` [PATCH 3/3] spi: dw: Use the rx-sample-delay-ns value parsed by " Frieder Schrempf
2 siblings, 0 replies; 6+ messages in thread
From: Frieder Schrempf @ 2026-09-17 15:37 UTC (permalink / raw)
To: Mark Brown, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Heiko Stuebner
Cc: linux-spi, devicetree, linux-kernel, linux-arm-kernel,
linux-rockchip, 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
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 3/3] spi: dw: Use the rx-sample-delay-ns value parsed by the core
2026-09-17 15:37 [PATCH 0/3] spi: Better specification for rx-sample-delay-ns and core parsing Frieder Schrempf
2026-09-17 15:37 ` [PATCH 1/3] dt-bindings: spi: Clarify what rx-sample-delay-ns describes Frieder Schrempf
2026-09-17 15:37 ` [PATCH 2/3] spi: Parse the rx-sample-delay-ns peripheral property in the core Frieder Schrempf
@ 2026-09-17 15:37 ` Frieder Schrempf
2 siblings, 0 replies; 6+ messages in thread
From: Frieder Schrempf @ 2026-09-17 15:37 UTC (permalink / raw)
To: Mark Brown, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Heiko Stuebner
Cc: linux-spi, devicetree, linux-kernel, linux-arm-kernel,
linux-rockchip, 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
^ permalink raw reply related [flat|nested] 6+ messages in thread