fix(iec_listener): serialize DATA line control to fix missed command bytes

The worker kthread and ATN ISR both modify the DATA line by changing GPIO2's
direction (open-drain emulation). On quad-core Pi they run simultaneously on
different CPUs, causing their read-modify-write operations on the shared GPFSEL0
register to clobber each other. The ISR's ATN presence-acknowledge could be
erased by the worker's ready-for-data release, causing the C64 to see "device
not present" and drop LISTEN/UNLISTEN command bytes.

Add iec_data_lock spinlock and two synchronized wrappers:
- iec_data_assert_sync(): serializes DATA assert
- iec_data_release_sync(): serializes DATA release and refuses to release while
  ATN is asserted, preventing the acknowledge from being undone

Convert all worker and ISR DATA accesses to use the _sync() wrappers. Keep raw
unconditional helpers for init/exit/self-test (exit must free the bus
unconditionally; self-test probes directly).

Generated by Clanker 🤖
This commit is contained in:
Christian Werner 2026-06-22 00:08:45 +02:00
parent 6c549b164c
commit ec615dbb80

View File

@ -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);
}