Skip to content

soc: apple: mailbox: keep enable_irq() balanced when a send times out - #599

Open
brentkearney wants to merge 39 commits into
AsahiLinux:asahi-wipfrom
brentkearney:mailbox-tx-irq-balance
Open

soc: apple: mailbox: keep enable_irq() balanced when a send times out#599
brentkearney wants to merge 39 commits into
AsahiLinux:asahi-wipfrom
brentkearney:mailbox-tx-irq-balance

Conversation

@brentkearney

@brentkearney brentkearney commented Sep 4, 2026

Copy link
Copy Markdown

Balance the send-empty interrupt enable in apple_mbox_send(), so a send whose wait times out or takes a signal no longer leaves the interrupt enabled. Fixes #598.

Problem

  • apple_mbox_send() enables irq_send_empty before waiting for the A2I FIFO to drain; only apple_mbox_send_empty_irq() disables it again.
  • The timeout and signal returns skip that disable, so the interrupt's disable depth is off by one.
  • The next send that finds the FIFO full calls enable_irq() on an already-enabled interrupt, so __enable_irq() warns and underflows the depth counter. Every further stalled send warns again with a backtrace.
  • Both paths are reachable in normal operation, and every RTKit coprocessor on these machines uses this function: sep, dcp, dcpext, aop, isp, and nvme.
  • The same code is in mainline, so the patch applies there unchanged apart from the header include path.

Changes

  • Add tx_irq_enabled to struct apple_mbox, guarded by tx_lock.
  • Enable the interrupt only when it is not already enabled.
  • Clear the flag in the interrupt handler, where it already calls disable_irq_nosync().
  • Disable the interrupt on the timeout and signal paths when the handler never ran.

Notes for reviewers

  • The flag settles the race where the handler runs just after the wait expires: whichever side takes tx_lock first performs the single disable.
  • A completion left signaled by that handler is cleared by the existing reinit_completion() before the next wait, so no send returns early on a stale completion.
  • The failure path holds tx_lock, so it calls disable_irq_nosync(); disable_irq() can sleep.

Testing

Runtime-tested on a MacBook Pro 16-inch 2021 (apple,j316s / apple,t6000, M1 Pro). The stall must be provoked from a healthy SEP: the message that makes SEPOS stop draining only has that effect early in a boot. Later SEPOS reaches a state where it stops answering but keeps draining, so the FIFO never fills and the timeout path is never taken.

  • Before, on 7.1.6-1-4-ARCH: eight sends timed out and the last seven each printed Unbalanced enable for IRQ 57 with a backtrace through apple_mbox_send+0xb4/0x1b8. Only the first timed-out send is silent, matching the analysis.
  • After, on 7.1.6-1-5-ARCH: eight sends filled the A2I FIFO and the next 56 timed out, with zero Unbalanced enable WARNs.
  • After: no WARNING line anywhere in the boot, and taint stayed at 4 (arm64 S baseline, W clear).
  • Display, audio, NVMe, and the SEP boot handshake behave normally across the boot.

marcan and others added 30 commits August 30, 2026 10:36
Apple M2 devices expose the multi-touch device over the HID over
DockChannel transport, which we represent as the HOST bus type. The
report format is the same, except the legacy mouse header is gone and
there is no enable request needed.

Signed-off-by: Hector Martin <marcan@marcan.st>
The trackpad has to request multi touch reports during resume.

Signed-off-by: Janne Grunau <j@jannau.net>
On at least some SPI devices (e.g. recent Apple Silicon machines), the
Broadcom touch controller is prone to crashing. When this happens, the
STM eventually notices and resets it. It then notifies the driver via
HID report 0x60, and the driver needs to re-enable MT mode to make
things work again.

This poses an additional issue: the hidinput core will close the
low-level transport while the device is closed, which can cause us to
miss a reset notification. To fix this, override the input open/close
callbacks and send the MT enable every time the HID device is opened,
instead of only once on probe. This should increase general robustness,
even if the reset mechanism doesn't work for some reason, so it's worth
doing it for USB devices too. MTP devices are exempt since they do not
require the MT enable at all.

Signed-off-by: Hector Martin <marcan@marcan.st>
For SPI/MTP trackpads, query the dimensions via HID report instead of
hardcoding values.

TODO: Does this work for the USB/BT devices? Maybe we can get rid of the
hardcoded sizes everywhere?

Signed-off-by: Hector Martin <marcan@marcan.st>
Keyboard and trackpad of Apple Sillicon SoCs (M1, M1 Pro/Max) laptops
are are HID devices connected via SPI.

This is the same protocol as implemented by applespi.c. It was not
noticed that protocol is a transport for HID. Adding support for ACPI
based Intel MacBooks will be done in a separate commit.

How HID is mapped in this protocol is not yet fully understood.

Microsoft has a specification for HID over SPI [1] incompatible with the
transport protocol used by Apple.

[1] https://docs.microsoft.com/en-us/windows-hardware/drivers/hid/hid-over-spi

Contains "HID: transport: spi: apple: Increase receive buffer size"

The SPI receive buffer is passed directly to hid_input_report() if it
contains a complete report. It is then passed to hid_report_raw_event()
which computes the expected report size and memsets the "missing
trailing data up to HID_MAX_BUFFER_SIZE (16K) or
hid_ll_driver.max_buffer_size (if set) to zero.

Co-developed-by: Hector Martin <marcan@marcan.st>
Signed-off-by: Hector Martin <marcan@marcan.st>
Signed-off-by: Janne Grunau <j@jannau.net>
DockChannel is a simple FIFO interface used to communicate between SoC
blocks. Add a driver that represents the shared interrupt controller for
the DockChannel block, and then exposes probe and data transfer
functions that child device drivers can use to instantiate individual
FIFOs.

Signed-off-by: Hector Martin <marcan@marcan.st>
Apple M2 devices have an MTP coprocessor embedded in the SoC that
handles HID for the integrated touchpad/keyboard, and communicates
over the DockChannel interface. This driver implements this new
interface.

Signed-off-by: Hector Martin <marcan@marcan.st>
This driver can be used for coprocessors that do some background task or
communicate out-of-band, and do not do any mailbox I/O beyond the
standard RTKit initialization.

Signed-off-by: Hector Martin <marcan@marcan.st>
Certain Broadcom bluetooth chips (bcm4377/bcm4378/bcm438) need ACL
streams carrying audio to be set as "high priority" using a vendor
specific command to prevent 10-ish second-long dropouts whenever
something does a device scan. This patch sends the command when the
socket priority is set to TC_PRIO_INTERACTIVE, as BlueZ does for audio.

Signed-off-by: Sasha Finkelstein <fnkl.kernel@gmail.com>
The current approach of silently disabling all rust drivers if the
toolchain is missing results in users that try to compile their own
kernels getting a "successful" build and then being confused about where
did their drivers go. In comparison, missing openssl results in a build
failure, not a disappearance of everything that depends on it.

This also means that allyesconfig will depend on rust, but since the
rust experiment concluded with "rust is here to stay", i believe that
allyesconfig should be building rust drivers too.

Signed-off-by: Sasha Finkelstein <k@chaosmail.tech>
Apple M3 Pro and Max devices are using 'gp00' keys for GPIO in addition
to 'gP00' keys. Add a second compatible to handle this keys with an
additional macsmc-gpio instance.

Signed-off-by: Janne Grunau <j@jannau.net>
Add support for SMC GPIO keys with a lower letter 'p' via the
"apple,smc-low-gpio" compatible. This adds support for a second
macsmc-gpio controller using 'gp00' keys.
These keys are used on Apple M3 Pro and Max MacBooks in the controller
for keyboard and trackpad and for the built-in DisplayPort to HDMI
converter.

Signed-off-by: Janne Grunau <j@jannau.net>
Apple M3 Pro and Max devices are using 'gp00' keys for GPIO in addition
to 'gP00' keys. These keys are handled by an additional macsmc-gpio
instance using the "apple,smc-low-gpio" compatible.

Signed-off-by: Janne Grunau <j@jannau.net>
Signed-off-by: Janne Grunau <j@jannau.net>
…fails

apple_mbox_send() enables irq_send_empty and waits for the coprocessor to
drain the A2I FIFO. Only the interrupt handler disables that interrupt
again, so when the wait does not complete -- the FIFO stays full for
APPLE_MBOX_TX_TIMEOUT, or the wait is interrupted by a signal -- the
interrupt is left enabled and its disable depth is off by one.

The next send that finds the FIFO full then calls enable_irq() on an
already-enabled interrupt, which warns and underflows the depth counter:

  Unbalanced enable for IRQ 57
  WARNING: kernel/irq/manage.c:774 at __enable_irq+0x4c/0x80, CPU#0
  Call trace:
   __enable_irq+0x4c/0x80 (P)
   enable_irq+0x74/0xe4
   apple_mbox_send+0xb4/0x1b8

Observed on an M1 Pro (apple,t6000) after SEPOS stopped draining its
mailbox: of eight consecutive sends that timed out, the last seven each
warned with a full backtrace.

Track whether the interrupt is enabled in struct apple_mbox under tx_lock
and disable it on the failure paths. The flag also settles the race with
the handler running just after the wait expired: whichever side takes
tx_lock first performs the single disable, and the stale completion is
cleared by the reinit_completion() before the next wait.

Signed-off-by: Brent <1550934+brentkearney@users.noreply.github.com>
@WhatAmISupposedToPutHere

Copy link
Copy Markdown
Member

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants