MT-149180: add basic ipc/extcon mechanism to send usb events from the stm through the kernel#41
Draft
Overdr0ne wants to merge 32 commits into
Draft
MT-149180: add basic ipc/extcon mechanism to send usb events from the stm through the kernel#41Overdr0ne wants to merge 32 commits into
Overdr0ne wants to merge 32 commits into
Conversation
Overdr0ne
marked this pull request as ready for review
June 3, 2026 19:31
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds an STM32 SPI-based IPC driver that publishes USB role/connection events as virtual extcon connectors, and wires it up for the mt-connect platform.
Changes:
- Introduce
extcon-stm32-ipcSPI core + child “virtual connector” platform driver with debugfs simulation support - Add Kconfig/Makefile integration for
CONFIG_EXTCON_STM32_SPI_IPC - Update mt-connect DTS/defconfig to instantiate the SPI IPC device and connect USB controllers via
extcon
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| drivers/extcon/extcon-stm32-ipc.c | New SPI IPC + virtual extcon implementation, IRQ handling, and debugfs simulation hooks |
| drivers/extcon/Makefile | Build integration for new extcon driver object |
| drivers/extcon/Kconfig | New EXTCON_STM32_SPI_IPC configuration option |
| arch/arm64/configs/mt_connect_defconfig | Enables the new driver in the platform defconfig |
| arch/arm64/boot/dts/freescale/mt-connect.dts | Replaces spidev node with STM32 IPC node, adds connector children, and links USB extcon phandles |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ardentra
approved these changes
Jun 17, 2026
Collaborator
|
@Overdr0ne unresolved comments here please |
Add a custom SPI client driver that functions as an extcon event provider. This driver communicates with an external STM32 co-processor over SPI using an attention line (interrupt) to propagate USB cable events. It registers separate virtual extcon sub-devices for each connector, allowing ChipIdea OTG controllers to switch between Host/Peripheral roles. A debugfs simulation interface is also exposed for runtime testing.
Enable CONFIG_EXTCON_STM32_SPI_IPC in mt_connect_defconfig to compile the custom stm-spi-ipc extcon driver directly into the kernel.
Configure the ECSPI2 controller on mt-connect to support the STM32 SPI IPC slave node and its virtual USB connector child nodes. Bind &usbotg1 and &usbotg2 to their respective virtual extcon connectors and enable role switching. Set up pin control and active-low interrupt for GPIO2_IO11.
Add a dependency on CONFIG_OF for CONFIG_EXTCON_STM32_SPI_IPC to prevent compile/configuration failures on architectures where Device Tree is not supported.
Move variable declarations in stm_usb_connector_probe() and stm_ipc_probe() to the top of functions. This resolves compilation failures under -Wdeclaration-after-statement.
Validate the 'port-id' Device Tree property in stm_usb_connector_probe() to ensure it fits within [0, 255] before casting it to u8. Avoids silently ignoring out-of-bounds inputs from the device tree.
Replace pr_warn_ratelimited() with dev_warn_ratelimited(&edev->dev, ...) in stm_ipc_update_state(). This logs the warning in the context of the specific extcon device instance, making debugging easier.
Move crc8_populate_msb() from stm_ipc_probe() to stm_ipc_init(). Populating the table during module initialization instead of probe prevents a concurrent write race condition when multiple devices are probed.
Each connector's sim file was torn down by a devres action registered in stm_usb_connector_probe(), while the parent SPI driver *also* removed the whole subtree via debugfs_remove_recursive(priv->debugfs_root) in its .remove() and probe error path. On unbind the recursive removal frees the connectors' sim-file dentries first, then each connector's devres action runs debugfs_remove() on the already-freed dentry -> use-after-free. Keep the per-connector cleanup -- it is what prevents a use-after-free of the debugfs data pointer when a connector is unbound on its own -- and tear the parent directory down through devres as well. Registering that action before devm_of_platform_populate() makes it run *after* the connectors are depopulated (LIFO devres order), so each connector removes its own sim file first and the parent action only reaps the now-empty directory. With teardown fully handled by devres, the manual recursive removal in the probe error path and .remove() is gone, leaving .remove() empty, so drop it.
Document the STM32 SPI IPC device and its child USB connector nodes. The
SPI peripheral talks to an external STM32 co-processor that reports USB
cable/role events over SPI (signalled by an attention interrupt); each
child connector node is an extcon provider for one i.MX8M USB OTG
controller and carries a required port-id.
These compatibles ("multitracks,stm32-spi-ipc" and
"multitracks,stm32-usb-connector") are used by the mt-connect device tree
but were previously undocumented, which triggers dtbs_check warnings.
The ChipIdea controller parses the "extcon" property as two phandles: index 0 is the VBUS (EXTCON_USB) source and index 1 is the ID (EXTCON_USB_HOST) source. ci_extcon_register() only registers the ID notifier when phandle 1 resolves, independent of usb-role-switch. With a single phandle the EXTCON_USB_HOST events emitted by the connector for host-role detection were never consumed. Point both indices at the same connector -- it provides both cable types -- so peripheral and host notifications are delivered.
match_and_update_state() returned 1 to mean "found" so the caller could test device_for_each_child()'s return value. That overloads the iterator return -- which exists to propagate a callback's error code -- as a found/not-found sentinel. Pass a small stm_ipc_port_event payload instead: the callback records the match in event->handled and always returns 0, leaving the return value free to carry a genuine error. The caller warns on a negative return and, separately, when no connector matched the reported port.
Requesting the attention IRQ with a hard-coded IRQF_TRIGGER_FALLING overrides the trigger type described by the device tree "interrupts" property and can provoke a trigger-type mismatch warning if the two ever disagree. Drop the flag and pass only IRQF_ONESHOT; the falling-edge type is already specified by the DT node (IRQ_TYPE_EDGE_FALLING) and configured by the irqchip, so behaviour is unchanged while the binding stays authoritative.
The STM32 SPI IPC attention line was wired to GPIO2_IO11 (SD1_STROBE), which is actually the leftover Type-C controller interrupt pad. The real attention/handshake line for the ECSPI2 co-processor is GPIO4_IO29 (SAI3_RXC): the existing user-space implementation reads it as gpiochip3 line 29 on /dev/spidev1.0 (imx8-a53/connect: spi.c SPI_BUSY_GPIO_PIN=29, gpio.h GPIO_CHIP_NAME="/dev/gpiochip3"), and pinctrl_ecspi2 already muxed GPIO4_IO29 with an "ECSPI2 INT" note that nothing consumed. Point the stm_ipc interrupt at gpio4/29 and move the pad into pinctrl_stm_ipc, dropping the now-duplicate entry from pinctrl_ecspi2 so the muxed pin and the interrupt agree. GPIO2_IO11 is freed. The pad keeps the 0x82 setting proven on this line by the user-space driver.
The attention line is how the STM32 signals pending events; without it the threaded handler never runs and no USB-role events are ever processed. The probe previously skipped devm_request_threaded_irq() when spi->irq was unset and still returned success, so a device tree missing the (binding- required) "interrupts" property would boot looking healthy while the IPC path was silently dead. Fail probe when no interrupt is provided instead.
No functional change. Address checkpatch --strict nits: drop the file name from the header comment, describe what priv->lock protects, move the misplaced "parent SPI controller" comment onto the struct it documents, wrap the lines that exceeded 100 columns, and add the missing blank line between stm_ipc_init() and stm_ipc_exit().
pinctrl_ecspi2 muxed the ECSPI2_SS0 pad twice -- once as native ECSPI2_SS0 and once as GPIO5_IO13 -- which are the same pad (IOMUXC mux-ctl 0x210) configured two ways. The controller uses cs-gpios = <&gpio5 13 ...>, so the pad must be a GPIO; the native SS0 entry is redundant and only muddies which mux actually takes effect. Keep the GPIO5_IO13 mux and drop the native one.
Release the SPI IPC serialization mutex immediately after completing the spi_sync() transfer in the threaded IRQ handler. Holding the lock during packet validation and extcon state updates (which can sleep and trigger userspace uevents) extends the critical section unnecessarily and could block other SPI operations.
Overdr0ne
marked this pull request as draft
July 13, 2026 21:14
Author
|
@dnappier-mt I didn't realize I had pending reviews on this since I sent it out way back when. Took me all day to get to the end of it after every push would trigger another review. Note, in our current scheme, this is not ready to be merged, because it will break the connect app, because they are both using the interrupt pin. That's why I switched it back to draft mode. But it is ready for dev QA, you should be able to perform the tests described above. I think it is time to switch to fixed hashes because this particular set will require a lot of changes to be merged atomically. So I am making a task for that, then I will create the udev rules that will replace what a lot of the connect app is currently doing, and extract that component. AFAIK, we will still need the connect app for the iap2 negotiation. We'll see exactly how that shakes out. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
MT-149180
This change implements a SPI IPC driver that triggers extcon events. The idea is, those extcon events look just like they would if they came from an onboard usb controller. I created some debug nodes to simulate disconnect and vbus detected events, which get sent to the extcon nodes where you can see the uevents in userspace here. So I’m only simulating the extcon part of the signal path, not yet anything with SPI. That part is in place, but I just need to get a clear spec on the SPI packet- we might already have that, I just need to look. Once the stm side is done, it should be a pretty simple modification to finish wiring things up.
The debug sim knobs are created per SPI device, under /sys/kernel/debug/stm_ipc//usbN_sim (here is spi1.0), to avoid collisions when more than one IPC device is present.
I'm leaving this as a draft for now because the stm side isn't done, and I made up my own packet spec, but it would be safe to integrate so people can try it out
dev QA: @dnappier-mt