MT-158113: imx8mm-evk-spi-transport devicetree overlay

[MT-MichaelLoh]

MT-158113

Summary

Scope grew 2026-08-19: this PR originally added just the devicetree overlay, with the driver itself living out-of-tree in the firmware repo. The driver's Linux adapter (drivers/spi/spi-mt-transport/) is now back in-tree here instead -- it's genuinely GPL (calls GPL-only-exported kernel symbols core to its function: devm_gpiod_get, gpiod_get_value/gpiod_set_value/gpiod_to_irq, spi_async, spi_slave_abort, dev_err_probe, sysfs_emit -- confirmed against the actual built kernel source, not assumed), and the firmware repo's CODING_STANDARDS.md §7 forbids GPL/LGPL code there. Found by Copilot's review on firmware PR #785; relicensing wasn't an option since MODULE_LICENSE("GPL") is required for these symbols to resolve at all, not a style choice.

What's in this PR now:

• arch/arm64/boot/dts/freescale/imx8mm-evk-spi-transport.dts (+ Makefile entry) -- the devicetree overlay: multitracks,spi-transport binding on ECSPI2 (SCLK/MOSI/MISO/SS0) plus mt-nss-gpios/mt-nrdy-gpios properties for the protocol's NSS/NRDY handshake lines. Unchanged from this PR's original scope.
• drivers/spi/spi-mt-transport/ -- the Linux Host-role driver adapter (spi_mt_transport_drv.c glue, spi_transport_hw_linux.*/spi_transport_os_linux.* HW/OS adapters, kernel-compat/* shims for the portable core's hosted-C11 headers under the kernel's -nostdinc build), moved here verbatim from the firmware repo, content unchanged. Plus the matching drivers/spi/Kconfig (CONFIG_SPI_MT_TRANSPORT) and drivers/spi/Makefile hooks, and arch/arm64/configs/imx_v8_defconfig's =m line.

What's deliberately NOT in this PR: the portable protocol core (spi_transport.c/spi_transport_channel.c/spi_transport_frame.c/spi_transport_crc16.c/spi_transport_hw.c + their headers). That stays the firmware repo's single source of truth -- it's plain C11, calls no kernel APIs, and doesn't need to be GPL. imx8mmini-bb-evk's meta-mt-transport-evk/recipes-kernel/linux-imx/linux-imx_%.bbappend fetches it fresh from the firmware repo at Yocto build time (a second, separately-named SRC_URI entry) and stages it into drivers/spi/spi-mt-transport/core/ via a do_patch[postfuncs] hook -- gitignored here, never committed, so there's no duplication between the two repos. This mirrors the original pre-restructuring layout's directory shape exactly, just populated at build time instead of vendored into git history.

Test plan

• bitbake -c patch linux-imx -f -- confirmed the core-staging postfunc correctly populates drivers/spi/spi-mt-transport/core/ (5 .c files, 6 headers) in the shared kernel source tree.
• bitbake linux-imx -- in-tree driver compiles clean, packages automatically as kernel-module-spi-mt-transport-<kernel-ver> (standard in-tree convention -- no manual IMAGE_INSTALL entry needed, unlike the old out-of-tree recipe).
• bitbake imx-image-core -- fresh manifest confirmed the module package present.
• Flashed real hardware: uname -r confirms the new kernel, modprobe spi_mt_transport probes cleanly, link_state: connected, event_counters clean.
• Write + read smoke test against the real STM32F723-DISCO Client: write near-instant (ring-queue design unaffected by this move), read delivered real traffic-generator bytes, delivery confirmed via the STM32 peer's mismatch counter incrementing exactly once per write.
• Full fault-injection matrix, two multi-hour soaks, and the TX throughput rework were all already hardware-validated before this move (see firmware #785 and #786) -- this move only relocates files, no functional change to the driver itself.

🤖 Generated with Claude Code

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[Copilot]

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.

Suppressed comments (4)

drivers/spi/spi-mt-transport/spi_transport_hw_linux.c:128

• Use gpiod_get_value_cansleep() here since this runs in process context (tick thread) and may be backed by a sleep-capable GPIO provider.

static bool mt_hw_ready_read(void *pContext)
{
	struct mt_transport_hw_ctx *ctx = pContext;

	return gpiod_get_value(ctx->nrdy_gpiod) ? true : false;
}

drivers/spi/spi-mt-transport/spi_transport_hw_linux.c:109

• This path is not in atomic context (it runs from the protocol tick thread / process context), so using the *_cansleep GPIO accessor is safer for GPIO controllers that may sleep. This avoids potential sleeping-in-atomic bugs if the GPIO descriptor ends up being backed by a sleep-capable provider.

This issue also appears on line 123 of the same file.

static void mt_hw_select_assert(void *pContext, bool high)
{
	struct mt_transport_hw_ctx *ctx = pContext;

	gpiod_set_value(ctx->nss_gpiod, high ? 1 : 0);
}

drivers/spi/spi-mt-transport/spi_mt_transport_drv.c:7

• The file header still says the driver “Builds as an out-of-tree Yocto kernel module”, but it is now wired in-tree under drivers/spi/ (and selected via Kconfig/Makefile). Update this description so it matches the current build/integration model (including the fact that only the portable core is staged at build time).

 * SPI transport protocol. Builds as an out-of-tree Yocto kernel module (see
 * imx8mmini-bb-evk's meta-mt-transport-evk recipes-kernel/spi-mt-transport/)
 * against the portable protocol core at ../../{,../inc/spi_transport}/ in
 * this same source tree -- referenced in place, not vendored/copied.

drivers/spi/spi-mt-transport/spi_transport_hw_linux.c:218

• This handler is registered as a threaded IRQ (primary handler is NULL), so it runs in a context where sleeping is allowed. Using gpiod_get_value_cansleep() makes the GPIO read safe even if the GPIO provider can sleep.

irqreturn_t mt_transport_hw_linux_nrdy_irq(int irq, void *dev_id)
{
	struct mt_transport_hw_ctx *ctx = dev_id;
	bool high = gpiod_get_value(ctx->nrdy_gpiod) ? true : false;

[Copilot]

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.

Suppressed comments (5)

drivers/spi/spi-mt-transport/spi_mt_transport_drv.c:335

• This uses '///' (C++-style) comments; prefer kernel-style /* */ comments for in-tree code.

/// @brief True if a new slot can be enqueued. Caller must already hold
///        tx_lock -- occupied total is tx_queued_count (not-yet-submitted
///        slots) plus one more if a slot is currently in flight
///        (tx_in_flight_idx >= 0), since that slot is still reserved even
///        though it doesn't count toward tx_queued_count.

drivers/spi/spi-mt-transport/spi_mt_transport_drv.c:351

• This uses '///' (C++-style) comments; prefer kernel-style /* */ comments for in-tree code.

/// @brief wait_event_interruptible()'s condition check only -- takes and
///        releases tx_lock itself since it must be callable without
///        already holding it. mt_transport_misc_write()'s own room check
///        below calls mt_transport_tx_room_locked() directly instead
///        (already holding the lock at that point) rather than this
///        wrapper, and deliberately so: that check has to stay under the
///        *same* lock acquisition that immediately follows (the enqueue),
///        otherwise a second writer could take the now-free slot in the
///        gap between checking and re-locking.

drivers/spi/spi-mt-transport/spi_mt_transport_drv.c:554

• gpiod_to_irq() can return -EPROBE_DEFER (or other negative errors). Treating all <=0 values as "no IRQ" can prevent proper probe deferral and permanently disables the IRQ optimization when the IRQ provider isn’t ready yet.

	priv->nrdy_irq = gpiod_to_irq(priv->nrdy_gpiod);
	if (priv->nrdy_irq > 0) {
		ret = devm_request_threaded_irq(dev, priv->nrdy_irq, NULL,
						 mt_transport_hw_linux_nrdy_irq,
						 IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING
							 | IRQF_ONESHOT,
						 DRIVER_NAME "-nrdy", &priv->hw_ctx);
		if (ret)
			dev_dbg(dev, "no NRDY IRQ (%d) -- falling back to tick-poll only\n", ret);
	} else {
		dev_dbg(dev, "NRDY line has no IRQ -- tick-poll only\n");
	}

drivers/spi/spi-mt-transport/spi_mt_transport_drv.c:234

• This block uses '///' (C++-style) comments. The kernel generally expects /* / (or kernel-doc /* */ when appropriate) and checkpatch will flag '//' comments outside of the SPDX line.

This issue also appears in the following locations of the same file:

• line 331
• line 343

/// @brief Submit the oldest queued TX slot (if any) via spiTransportSend().
///        A success return proves the *previous* in-flight slot (if any) is
///        now done -- the core only accepts a new send once the last one's
///        final chunk is confirmed -- so that previous slot is freed right
///        here, not after a guessed timeout. Called once per tick thread

arch/arm64/boot/dts/freescale/imx8mm-evk-spi-transport.dts:37

• This DTS introduces a new compatible string ("multitracks,spi-transport") and new vendor properties (mt-nss-gpios/mt-nrdy-gpios) but doesn’t add a devicetree binding document under Documentation/devicetree/bindings/. Without it, dtbs_check can’t validate the schema for these properties.

	mt_transport0: spi@0 {
		compatible = "multitracks,spi-transport";
		reg = <0>;
		spi-max-frequency = <500000>; /* matches mt-connect.dts; revisit once link timing is measured */

		mt-nss-gpios = <&gpio5 13 GPIO_ACTIVE_HIGH>;
		mt-nrdy-gpios = <&gpio4 29 GPIO_ACTIVE_HIGH>;
	};

[Copilot]

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated no new comments.

Suppressed comments (3)

Documentation/devicetree/bindings/spi/multitracks,spi-transport.yaml:55

• With allOf schemas in place, additionalProperties: false tends to be too strict because it can reject properties coming from the referenced schemas; unevaluatedProperties: false is the more typical pattern for DT schema composition in this tree (e.g. Documentation/devicetree/bindings/spi/spi-mux.yaml:52). Also, spi-max-frequency is generally expected to be specified for SPI peripherals; requiring it improves validation.

required:
  - compatible
  - reg
  - mt-nss-gpios
  - mt-nrdy-gpios

additionalProperties: false

drivers/spi/spi-mt-transport/Makefile:16

• This module’s Makefile hard-requires a non-versioned, gitignored core/ directory that is absent from the tree, so enabling CONFIG_SPI_MT_TRANSPORT outside the Yocto staging flow fails with an opaque "No rule to make target" error. Even if the Yocto path is the intended one, it’s useful to fail fast with a clear, actionable error message when the core hasn’t been staged.

# hook *before* this Makefile ever runs. Enabling CONFIG_SPI_MT_TRANSPORT
# outside that pipeline will fail with a generic "No rule to make target
# core/spi_transport.o" -- that's expected, not a bug; get the core staged
# first (or just build via bitbake, which always does this automatically).
obj-$(CONFIG_SPI_MT_TRANSPORT) += spi-mt-transport.o

Documentation/devicetree/bindings/spi/multitracks,spi-transport.yaml:25

• Building this schema with additionalProperties: false but without referencing the common SPI peripheral properties prevents use of standard SPI DT flags (e.g. spi-cpha, spi-cpol, etc.) and diverges from common SPI binding patterns (see Documentation/devicetree/bindings/spi/spi-mux.yaml:32). Consider adding a $ref to spi-peripheral-props.yaml so the binding can validate/allow standard SPI properties consistently.

This issue also appears on line 49 of the same file.

properties:

[Copilot]

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

drivers/spi/spi-mt-transport/spi_mt_transport_drv.c:556

• gpiod_to_irq() can legally return 0 as a valid IRQ number; checking > 0 will incorrectly treat IRQ0 as “no IRQ” and skip the threaded handler. Kernel drivers generally treat only negative values as errors (e.g. drivers/pps/clients/pps-gpio.c:175-180).

	if (priv->nrdy_irq == -EPROBE_DEFER)
		return dev_err_probe(dev, -EPROBE_DEFER, "NRDY IRQ not ready yet\n");
	if (priv->nrdy_irq > 0) {
		ret = devm_request_threaded_irq(dev, priv->nrdy_irq, NULL,

[Copilot]

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

drivers/spi/spi-mt-transport/spi_mt_transport_drv.c:327

• mt_transport_misc_read() can race with multiple concurrent readers: the initial rx_valid check (or the wait_event) is not tied to the later consume-under-lock section, so two readers can both observe rx_valid=true and both copy/clear the same buffered message. Either enforce single-open semantics for the miscdevice, or re-check rx_valid under rx_lock at consume time and, if it was already consumed, retry the wait (blocking) or return -EAGAIN (non-blocking).

	if (filp->f_flags & O_NONBLOCK) {
		spin_lock_irqsave(&priv->rx_lock, irqflags);
		if (!priv->rx_valid) {
			spin_unlock_irqrestore(&priv->rx_lock, irqflags);
			return -EAGAIN;
		}
		spin_unlock_irqrestore(&priv->rx_lock, irqflags);
	} else {
		ret = wait_event_interruptible(priv->rx_wq, READ_ONCE(priv->rx_valid));
		if (ret)
			return ret;
	}

	/* Snapshot into a local buffer under the lock, then copy_to_user()
	 * outside it -- copy_to_user() can fault/sleep, which is illegal
	 * while holding a spinlock. rx_valid is cleared here too (not after
	 * the copy) since it's rx_lock-protected state, same as rx_buf --
	 * a failing copy_to_user (a broken caller's bad pointer) now
	 * consumes the buffered message rather than leaving it for retry,
	 * a minor, acceptable behavior change for this placeholder interface.
	 */
	spin_lock_irqsave(&priv->rx_lock, irqflags);
	len = priv->rx_len;
	if (len > count)
		len = count;
	memcpy(scratch, priv->rx_buf, len);
	WRITE_ONCE(priv->rx_valid, false);
	spin_unlock_irqrestore(&priv->rx_lock, irqflags);

[Copilot]

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated no new comments.

Suppressed comments (4)

drivers/spi/spi-mt-transport/spi_mt_transport_drv.c:725

• mt_transport_remove() sets priv->removed but does not wake threads blocked in read()/write() (rx_wq / tx_free_wq). This can leave existing fds stuck in wait_event_interruptible() forever after unbind. Wake both waitqueues immediately after marking removed so sleepers re-check the removed flag and exit.

	WRITE_ONCE(priv->removed, true);

drivers/spi/spi-mt-transport/spi_mt_transport_drv.c:378

• Blocking read() can hang indefinitely if the SPI device is unbound while a process is sleeping in wait_event_interruptible(): the wait condition only checks rx_valid and never considers priv->removed, and mt_transport_remove() currently doesn't wake rx_wq. Include priv->removed in the wait condition and return -ENODEV if it becomes true.

This issue also appears on line 724 of the same file.

		ret = wait_event_interruptible(priv->rx_wq, READ_ONCE(priv->rx_valid));
		if (ret)
			return ret;
	}

drivers/spi/spi-mt-transport/spi_mt_transport_drv.c:475

• Blocking write() can hang indefinitely if the device is unbound while a process is sleeping in wait_event_interruptible() for TX ring space: the condition only checks for room, not priv->removed, and mt_transport_remove() doesn't wake tx_free_wq. Include priv->removed in the wait condition and fail with -ENODEV when it becomes true.

		ret = wait_event_interruptible(priv->tx_free_wq, mt_transport_tx_has_room(priv));
		if (ret)
			return ret;
	}

drivers/spi/spi-mt-transport/spi_mt_transport_drv.c:502

• poll() doesn't report device removal: after unbind, existing fds can poll forever because the waitqueue is only rx_wq and the mask never includes HUP/ERR when priv->removed is set. Return EPOLLHUP|EPOLLERR when removed so userspace can detect teardown cleanly.

	poll_wait(filp, &priv->rx_wq, wait);
	if (READ_ONCE(priv->rx_valid))
		mask |= EPOLLIN | EPOLLRDNORM;
	return mask;

[Copilot]

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

drivers/spi/spi-mt-transport/spi_mt_transport_drv.c:56

• Kernel coding style uses struct foo { on a single line; the current formatting for struct mt_transport_tx_slot is likely to trigger checkpatch/style warnings and is inconsistent with the rest of this file (e.g. struct mt_transport_priv {).

struct mt_transport_tx_slot
{
	uint8_t buf[SPI_TRANSPORT_CHANNEL_MESSAGE_MAX];
	uint16_t len;
};

[Copilot]

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated no new comments.

Suppressed comments (1)

drivers/spi/spi-mt-transport/spi_mt_transport_drv.c:289

• tx_in_flight_idx is set on every successful spiTransportSend(), but it is never cleared back to -1 anywhere. As a result, mt_transport_tx_room_locked() will permanently treat one slot as occupied even after the final in-flight TX finishes, reducing the usable TX queue depth by 1 for the lifetime of the device and potentially causing unexpected -EAGAIN/-blocking earlier than MT_TRANSPORT_TX_QUEUE_DEPTH implies. Consider clearing tx_in_flight_idx when the core indicates the in-flight send has fully completed (and no queued slots remain), and waking tx_free_wq at that point.

	spin_lock_irqsave(&priv->tx_lock, flags);
	priv->tx_queued_count--;
	priv->tx_in_flight_idx = idx;
	priv->tx_head = (priv->tx_head + 1) % MT_TRANSPORT_TX_QUEUE_DEPTH;
	spin_unlock_irqrestore(&priv->tx_lock, flags);

[Copilot]

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.

Suppressed comments (3)

drivers/spi/spi-mt-transport/spi_mt_transport_drv.c:723

• If spiTransportStart() fails after the NRDY IRQ was requested, priv is freed via kref_put() while the devm-managed IRQ can still fire until devres unwind after probe returns. Free the IRQ on this error path too to avoid a potential UAF in the threaded handler.

	if (spiTransportStart(priv->htransport) != eSpiTransportErrorNone) {
		kthread_stop(priv->tick_thread);
		kref_put(&priv->refcount, mt_transport_priv_release);
		return dev_err_probe(dev, -EINVAL, "spiTransportStart failed\n");
	}

drivers/spi/spi-mt-transport/spi_mt_transport_drv.c:744

• If misc_register() fails after the NRDY IRQ was requested, priv is freed via kref_put() while the devm-managed IRQ can still fire until devres unwind after probe returns. Free the IRQ in this failure path (ideally before stopping the core) to avoid the threaded handler calling into torn-down state or freed hw_ctx.

	ret = misc_register(&priv->misc);
	if (ret) {
		/* kthread_stop() before spiTransportStop(): the tick thread
		 * must not be able to call into the core after it's been
		 * stopped (same ordering as mt_transport_remove() below).
		 */
		kthread_stop(priv->tick_thread);
		spiTransportStop(priv->htransport);
		kref_put(&priv->refcount, mt_transport_priv_release);
		return dev_err_probe(dev, ret, "misc_register failed\n");

drivers/spi/Kconfig:1217

• SPI_MT_TRANSPORT depends on COMPILE_TEST, but this module intentionally does not build from a plain tree checkout unless an external core/ directory is staged (and the Makefile hard-errors when it is not). With COMPILE_TEST=y, common config targets like allmodconfig/allyesconfig can select this symbol and break builds. Consider dropping COMPILE_TEST here (or making the driver self-contained in-tree).

config SPI_MT_TRANSPORT
	tristate "MultiTracks SPI transport protocol driver (Host role)"
	depends on ARCH_MXC || COMPILE_TEST
	depends on GPIOLIB

[Copilot]

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated no new comments.

Suppressed comments (1)

drivers/spi/spi-mt-transport/spi_transport_os_linux.c:105

• mt_os_log() takes a va_list parameter, but initializes struct va_format with .va = &args. On architectures where va_list is an array type, function parameters of array type are adjusted to pointers, so &args becomes a pointer-to-pointer rather than a va_list *, which can break %pV formatting. Make a local va_list copy and point va_format.va at that local.

	struct mt_transport_os_ctx *ctx = pContext;
	struct va_format vaf = { .fmt = pFormat, .va = &args };

	dev_dbg(ctx->dev, "%pV", &vaf);
}

[Copilot]

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.