docs(kernel): document DATA line locking design to prevent RMW collisions

Add row to "Decisions baked into the code" table explaining the DATA line
control design: all DATA assert/release operations in the worker and ATN ISR
go through iec_data_*_sync() under iec_data_lock to serialize the GPFSEL0
read-modify-write across CPUs. The release wrapper refuses to release while
ATN is asserted, preventing the ready-for-data release from erasing the ISR's
presence acknowledge — the bug that caused intermittently missed
LISTEN/UNLISTEN commands. Init/exit/self-test keep raw unconditional helpers.

Generated by Clanker 🤖

docs/kernel-notes.md

@@ -16,6 +16,7 @@ the code in `kernel/` is built on.
 | Kernel↔userspace | Character device `/dev/iec0` + `kfifo` + wait queue (IEC ≤ 1000 B/s; relayfs not justified) | `iec_read`, `emit_record` |
 | `udelay` vs. poll | Poll-with-timeout for CLK transitions; `udelay` only for fixed delays (EOI ack 80 µs, EOI detect 250 µs) | `iec_timing.h`, `wait_clk` |
 | Sense-line reads | Debounced (glitch-filtered): a level change is believed only after it holds `IEC_DEBOUNCE_US` (5 µs); shorter pulses are rejected as noise. Mirrors a confirmed-working reference listener. Fast path is a single register read, so tight CLK polls stay cheap. ATN ISR + self-test stay **raw** | `iec_read_stable`, `db_*_asserted` |
+| DATA line control | All DATA assert/release in the worker + ATN ISR go through `iec_data_*_sync()` under `iec_data_lock`. The pin direction-flip is a read-modify-write of the shared `GPFSEL0`; without the lock the worker (one CPU) and the ATN ISR (another CPU on the quad-core Pi) could clobber each other's RMW. The release wrapper also **refuses to release while ATN is asserted**, so a ready-for-data release can never erase the ISR's presence acknowledge — the bug that caused intermittently missed LISTEN/UNLISTEN commands. init/exit/self-test keep the **raw** unconditional helpers | `iec_data_assert_sync`, `iec_data_release_sync`, `atn_isr` |
 | isolcpus / nohz_full | **Not** in the Phase-1 baseline. Add `isolcpus=3 nohz_full=3 rcu_nocbs=3 irqaffinity=0-2` only if Phase-2 bit-error rate > 1% | (boot cmdline) |
 | PREEMPT_RT | Stock kernel sufficient; RT is a last resort | — |
 | Module signing | Not required on stock RPi OS Bookworm | — |

kernel/iec_listener.c

@@ -129,6 +129,49 @@ static inline bool db_atn_asserted(void)   { return iec_read_stable(IEC_GPIO_ATN
 static inline bool db_reset_asserted(void) { return iec_read_stable(IEC_GPIO_RESET, &db_reset) == 0; }
 static inline bool db_data_asserted(void)  { return iec_read_stable(IEC_GPIO_DATA,  &db_data)  == 0; }
 
+/*
+ * --- synchronized DATA line control --------------------------------------
+ *
+ * DATA is open-drain-emulated by flipping GPIO2's direction (iec_lines.h), and
+ * that flip is a read-modify-write of the shared GPFSEL0 register. Two contexts
+ * change DATA: the worker kthread (handshake) and the ATN falling-edge ISR
+ * (presence acknowledge). On the quad-core Pi these can run on different CPUs at
+ * the same time, so their RMWs can clobber each other -- e.g. the worker's RFD
+ * release erasing the ISR's ATN ack, which makes the C64 see "device not
+ * present" and drop the LISTEN/UNLISTEN command (the missed-command bug).
+ *
+ * iec_data_lock serializes every DATA change so the RMW is never split. In
+ * addition, the release wrapper refuses to release while ATN is asserted: when
+ * ATN is low the device must keep DATA low as the acknowledge, so a release the
+ * worker was about to do for ready-for-data must not undo a just-arrived ack.
+ * iec_atn_asserted() is the raw instantaneous read (matching the ISR's view);
+ * the debounced db_atn_asserted() must NOT be used here.
+ *
+ * Only the worker/ISR hot path uses these. init/exit/selftest keep the raw
+ * helpers: exit() must release the bus unconditionally, and selftest needs the
+ * unconditional behaviour to probe the line.
+ */
+static DEFINE_SPINLOCK(iec_data_lock);
+
+static void iec_data_assert_sync(void)
+{
+	unsigned long f;
+
+	spin_lock_irqsave(&iec_data_lock, f);
+	iec_data_assert();
+	spin_unlock_irqrestore(&iec_data_lock, f);
+}
+
+static void iec_data_release_sync(void)
+{
+	unsigned long f;
+
+	spin_lock_irqsave(&iec_data_lock, f);
+	if (!iec_atn_asserted())
+		iec_data_release();
+	spin_unlock_irqrestore(&iec_data_lock, f);
+}
+
 /* receive_byte() outcomes */
 enum iec_rx {
 	IEC_RX_OK = 0,    /* byte received                          */
@@ -204,7 +247,7 @@ static enum iec_rx receive_byte(u8 *out, bool *eoi, bool data_phase)
 	rc = wait_clk(false /* released */, IEC_CLK_TIMEOUT_US, data_phase);
 	if (rc != IEC_RX_OK)
 		return rc;
-	iec_data_release();
+	iec_data_release_sync();
 
 	local_irq_save(flags); /* IRQs off for the duration of the byte */
 
@@ -217,9 +260,9 @@ static enum iec_rx receive_byte(u8 *out, bool *eoi, bool data_phase)
 			if (db_reset_asserted()) { rc = IEC_RX_RESET; goto out; }
 			if (waited++ >= IEC_EOI_DETECT_US) {
 				/* ack EOI: pull DATA low for Tei, then release */
-				iec_data_assert();
+				iec_data_assert_sync();
 				udelay(IEC_EOI_ACK_HOLD_US);
-				iec_data_release();
+				iec_data_release_sync();
 				*eoi = true;
 				break;
 			}
@@ -252,7 +295,7 @@ static enum iec_rx receive_byte(u8 *out, bool *eoi, bool data_phase)
 		      IEC_CLK_TIMEOUT_US, data_phase);
 	if (rc != IEC_RX_OK)
 		goto out;
-	iec_data_assert();
+	iec_data_assert_sync();
 	*out = value;
 	rc = IEC_RX_OK;
 
@@ -287,7 +330,7 @@ static void run_state_machine(void)
 		enum iec_rx rc;
 
 		if (kthread_should_stop()) {
-			iec_data_release();
+			iec_data_release_sync();
 			return;
 		}
 		if (db_reset_asserted())
@@ -310,7 +353,7 @@ static void run_state_machine(void)
 		/* ATN released */
 		if (!addressed_listener) {
 			emit_record(IEC_KIND_EVENT, IEC_EV_ATN_RELEASED, 0);
-			iec_data_release();
+			iec_data_release_sync();
 			return; /* not our business -> IDLE */
 		}
 
@@ -318,7 +361,7 @@ static void run_state_machine(void)
 		emit_record(IEC_KIND_EVENT, IEC_EV_ATN_RELEASED, IEC_FLAG_ADDRESSED);
 		for (;;) {
 			if (kthread_should_stop()) {
-				iec_data_release();
+				iec_data_release_sync();
 				return;
 			}
 			if (db_reset_asserted())
@@ -344,7 +387,7 @@ static void run_state_machine(void)
 
 reset:
 	addressed_listener = false;
-	iec_data_release();
+	iec_data_release_sync();
 	emit_record(IEC_KIND_EVENT, IEC_EV_RESET, 0);
 }
 
@@ -369,7 +412,7 @@ static int iec_worker(void *unused)
 static irqreturn_t atn_isr(int irq, void *dev)
 {
 	if (iec_atn_asserted()) {
-		iec_data_assert();              /* pull DATA low immediately (ack) */
+		iec_data_assert_sync();         /* pull DATA low immediately (ack) */
 		atomic_set(&iec_atn_pending, 1);
 		wake_up_interruptible(&iec_work_wq);
 	}