[PATCH v2 4/5] spi: spi-mux: pass child CS and word delays to the parent

[Joseph CT Chan <jctchan@gmail.com>]

From: Joseph CT Chan <josephctchan@google.com>

spi_mux_select() copies only max_speed_hz, mode and bits_per_word from
the child to the spi-mux device on the parent controller. The child's
cs_setup, cs_hold, cs_inactive and word_delay are never passed on, so
the parent controller uses the spi-mux device's own delays (usually
zero) for every child. A child that needs a CS setup time or a delay
between words, for example from spi-cs-setup-delay-ns in its device
tree node, does not get it when it sits behind a mux.

Save the spi-mux device's own delays at probe time, and when switching
to a child, merge them with the child's in a new helper,
spi_mux_merge_cs_delay(): use the longer CS delay of the two, so that
the needs of the mux and of the child are both met; on a tie the
spi-mux delay is kept. A delay given in SCK cycles cannot be converted
to time without a transfer, so it cannot be compared: a child delay in
SCK cycles is used when the spi-mux delay is zero, and otherwise the
spi-mux delay is kept and a warning is printed once. If only the
spi-mux delay is in SCK cycles, it is kept without a warning; device
tree only gives delays in ns or us, so this case does not come from
DT. The child's word_delay is used when it is set, else the spi-mux
device's own. The delays are written before spi_setup() on the parent,
so they reach both set_cs_timing() and the software CS delays in
spi_set_cs().

spi_mux_select() overwrites the delays of the spi-mux device, so put
its own delays back when the driver unbinds. Otherwise a later probe
would save the last child's merged delays as the spi-mux device's own.

Unlike a device directly on a controller, whose CS delays are read on
every message, the delays are now copied when the mux switches to the
child or after the child calls spi_setup(). A child that changes its
delays must call spi_setup() for the change to take effect.

This depends on patch 2 of this series ("spi: spi-mux: re-apply
settings after child calls spi_setup()"), which makes the copy happen
again after the child calls spi_setup().

Tested with KUnit (spi_mux: spi_mux_cs_delay_test and
spi_mux_settings_lifecycle_test, added later in this series) on ARCH=um;
they fail without this patch. Builds with W=1 on x86_64. Not tested on
hardware.

Developed with help from the Claude and Gemini AI assistants.

Fixes: e9e40543ad5b ("spi: Add generic SPI multiplexer")
Fixes: 8c33ebfeeb59 ("spi: move cs spi_delay to spi_device")
Signed-off-by: Joseph CT Chan <josephctchan@google.com>
---
v2: Updated the patch reference in the commit message after the split.

 drivers/spi/spi-mux.c | 83 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 83 insertions(+)

diff --git a/drivers/spi/spi-mux.c b/drivers/spi/spi-mux.c
index 0e770137b5bb..45d8920727e0 100644
--- a/drivers/spi/spi-mux.c
+++ b/drivers/spi/spi-mux.c
@@ -34,6 +34,10 @@
  * @child_msg_dev:	Used to store the spi_device pointer to the child
  * @mux:		mux_control structure used to provide chip selects for
  *			downstream spi devices
+ * @mux_cs_setup:	CS setup delay of the spi-mux device itself
+ * @mux_cs_hold:	CS hold delay of the spi-mux device itself
+ * @mux_cs_inactive:	CS inactive delay of the spi-mux device itself
+ * @mux_word_delay:	word delay of the spi-mux device itself
  */
 struct spi_mux_priv {
 	struct spi_device	*spi;
@@ -43,8 +47,55 @@ struct spi_mux_priv {
 	void			*child_msg_context;
 	struct spi_device	*child_msg_dev;
 	struct mux_control	*mux;
+
+	struct spi_delay	mux_cs_setup;
+	struct spi_delay	mux_cs_hold;
+	struct spi_delay	mux_cs_inactive;
+	struct spi_delay	mux_word_delay;
 };
 
+/*
+ * Pick the CS delay for the parent from the spi-mux device's own delay and
+ * the child's: the longer of the two, so that both are honored; on a tie the
+ * spi-mux delay is kept. A delay in SCK cycles cannot be converted to time
+ * without a transfer, so it cannot be compared:
+ * - a child delay in SCK cycles is used when the spi-mux delay is zero;
+ *   otherwise the spi-mux delay is kept and a warning is printed once;
+ * - if only the spi-mux delay is in SCK cycles, it is kept. Device tree
+ *   only gives delays in ns or us, so this case does not come from DT.
+ */
+static void spi_mux_merge_cs_delay(struct spi_mux_priv *priv,
+				   struct spi_delay *dst,
+				   struct spi_delay *mux,
+				   struct spi_delay *child)
+{
+	int mux_ns, child_ns;
+
+	if (!child->value) {
+		*dst = *mux;
+		return;
+	}
+
+	mux_ns = spi_delay_to_ns(mux, NULL);
+	child_ns = spi_delay_to_ns(child, NULL);
+
+	if (mux_ns >= 0 && child_ns >= 0) {
+		*dst = child_ns > mux_ns ? *child : *mux;
+		return;
+	}
+
+	if (child_ns < 0) {
+		if (!mux->value) {
+			*dst = *child;
+			return;
+		}
+		dev_warn_once(&priv->spi->dev,
+			      "child CS delay in SCK cycles ignored, using the mux delay\n");
+	}
+
+	*dst = *mux;
+}
+
 /* should not get called when the parent controller is doing a transfer */
 static int spi_mux_select(struct spi_device *spi)
 {
@@ -65,6 +116,14 @@ static int spi_mux_select(struct spi_device *spi)
 	priv->spi->max_speed_hz = spi->max_speed_hz;
 	priv->spi->mode = spi->mode;
 	priv->spi->bits_per_word = spi->bits_per_word;
+	spi_mux_merge_cs_delay(priv, &priv->spi->cs_setup, &priv->mux_cs_setup,
+			       &spi->cs_setup);
+	spi_mux_merge_cs_delay(priv, &priv->spi->cs_hold, &priv->mux_cs_hold,
+			       &spi->cs_hold);
+	spi_mux_merge_cs_delay(priv, &priv->spi->cs_inactive,
+			       &priv->mux_cs_inactive, &spi->cs_inactive);
+	priv->spi->word_delay = spi->word_delay.value ? spi->word_delay :
+							 priv->mux_word_delay;
 
 	ret = spi_setup(priv->spi);
 	if (ret) {
@@ -156,10 +215,26 @@ static int spi_mux_transfer_one_message(struct spi_controller *ctlr,
 	return 0;
 }
 
+/*
+ * spi_mux_select() overwrites the delays of the spi-mux device with the
+ * merged ones. Put its own delays back when the driver unbinds, so that a
+ * later probe does not take the last child's delays as its own.
+ */
+static void spi_mux_restore_delays(void *data)
+{
+	struct spi_mux_priv *priv = data;
+
+	priv->spi->cs_setup = priv->mux_cs_setup;
+	priv->spi->cs_hold = priv->mux_cs_hold;
+	priv->spi->cs_inactive = priv->mux_cs_inactive;
+	priv->spi->word_delay = priv->mux_word_delay;
+}
+
 static int spi_mux_probe(struct spi_device *spi)
 {
 	struct spi_controller *ctlr;
 	struct spi_mux_priv *priv;
+	int ret;
 
 	ctlr = devm_spi_alloc_host(&spi->dev, sizeof(*priv));
 	if (!ctlr)
@@ -168,6 +243,14 @@ static int spi_mux_probe(struct spi_device *spi)
 	spi_set_drvdata(spi, ctlr);
 	priv = spi_controller_get_devdata(ctlr);
 	priv->spi = spi;
+	priv->mux_cs_setup = spi->cs_setup;
+	priv->mux_cs_hold = spi->cs_hold;
+	priv->mux_cs_inactive = spi->cs_inactive;
+	priv->mux_word_delay = spi->word_delay;
+
+	ret = devm_add_action_or_reset(&spi->dev, spi_mux_restore_delays, priv);
+	if (ret)
+		return ret;
 
 	/*
 	 * Increase lockdep class as these lock are taken while the parent bus
-- 
2.56.0.385.gac671c819a-goog