diff --git a/.gitea/workflows/build-kernel.yml b/.gitea/workflows/build-kernel.yml index de478e9..03459cc 100644 --- a/.gitea/workflows/build-kernel.yml +++ b/.gitea/workflows/build-kernel.yml @@ -142,6 +142,7 @@ jobs: cp -r dist/modules "$root"/ # modules/iec_listener_.ko cp kernel/selftest.sh "$root"/ # one-shot hardware self-test cp launch.sh "$root"/ # load module -> run frontend -> unload + chmod +x "$root/launch.sh" "$root/selftest.sh" # iecpoc Python frontend + its packaging metadata. launch.sh pip-installs # this on the Pi; the build reads pyproject.toml's readme = "README.md", # so README.md must travel with it. __pycache__ is stripped to stay clean. @@ -177,6 +178,7 @@ jobs: - name: Zip release asset run: | repo="${{ github.event.repository.name }}" + chmod +x "dist/$repo/launch.sh" "dist/$repo/selftest.sh" (cd dist && zip -r "../${repo}-${{ github.ref_name }}.zip" "$repo") - name: Publish to Gitea release diff --git a/_plans/poc-listener-printer-PLAN.md b/_plans/poc-listener-printer-PLAN.md index 926b51e..f087003 100644 --- a/_plans/poc-listener-printer-PLAN.md +++ b/_plans/poc-listener-printer-PLAN.md @@ -94,10 +94,10 @@ the 1 ms ATN ack and the per-byte ack — both forgiving. The tight constraint i | IEC signal | Pi GPIO (BCM) | Header pin | Direction | Logic note | |------------|---------------|------------|-----------|------------| -| ATN | GPIO 2 | 3 | input (divider) | bus low = Pi low = asserted | -| CLK | GPIO 17 | 11 | input (divider) | bus low = Pi low = asserted | -| DATA (in) | GPIO 18 | 12 | output → 7406 → bus | Pi HIGH = bus asserted (inverted) | -| RESET | GPIO 3 | 5 | input (divider) | bus low = Pi low = asserted | +| ATN | GPIO 4 | 7 | input (divider) | bus low = Pi low = asserted | +| CLK | GPIO 3 | 5 | input (divider) | bus low = Pi low = asserted | +| DATA (in) | GPIO 2 | 3 | output → 7406 → bus | Pi HIGH = bus asserted (inverted) | +| RESET | GPIO 17 | 11 | input (divider) | bus low = Pi low = asserted | | GND | — | 6 (or any) | — | tie to IEC pin 2 | > Convention used in code: a helper layer converts electrical reads/writes into diff --git a/docs/kernel-notes.md b/docs/kernel-notes.md index fae5df7..f9444ae 100644 --- a/docs/kernel-notes.md +++ b/docs/kernel-notes.md @@ -15,6 +15,8 @@ the code in `kernel/` is built on. | GPIO access (init/exit) | gpiod descriptor API; descriptors resolved by `(chip, hwnum)` via `gpio_device_find_by_label("pinctrl-bcm2835")` + `gpio_device_get_desc()`, **not** `gpio_to_desc()` (see "GPIO descriptor lookup" below) | `iec_init`/`iec_exit` | | 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 | — | @@ -58,11 +60,11 @@ at global number 0 — it's `gpiochip512`, **base 512**: $ cat /sys/class/gpio/gpiochip*/base # -> 512 (pinctrl-bcm2835), 566 (exp-gpio) ``` -The old code used `gpio_to_desc(2/3/17/18)`, i.e. the *global* numberspace +The old code used `gpio_to_desc(2/3/4/17)`, i.e. the *global* numberspace assuming base 0. Those small numbers now fall outside the chip's range (512–565), so every `gpio_to_desc()` returned `NULL`, the descriptor check tripped, and init bailed with `-ENODEV`. The `(chip, hwnum)` lookup passes the -**BCM number as the chip-relative offset** (ATN=2, RESET=3, CLK=17, DATA=18), +**BCM number as the chip-relative offset** (ATN=4, RESET=17, CLK=3, DATA=2), which is base-independent and survives kernel bumps / gpiochip renumbering. Notes: @@ -222,8 +224,10 @@ The host needs qemu binfmt for arm64; the script registers it once via ## Starting timing constants See `kernel/iec_timing.h`. Tune in Phase 2 against a real C64 / logic analyser. -The likely first knobs: `IEC_EOI_DETECT_US` (EOI false positives/negatives) and -`IEC_CLK_TIMEOUT_US` (frame errors under load). +The likely first knobs: `IEC_EOI_DETECT_US` (EOI false positives/negatives), +`IEC_CLK_TIMEOUT_US` (frame errors under load), and `IEC_DEBOUNCE_US` (the +sense-line glitch-filter window — raise it if a noisy bus still yields bit +errors, lower it if it smears fast edges). ## Open items to verify on hardware diff --git a/docs/wiring.md b/docs/wiring.md index b1ceb7b..b6e9893 100644 --- a/docs/wiring.md +++ b/docs/wiring.md @@ -25,20 +25,26 @@ The shifter is **non-inverting**: bus low (asserted) ⇒ Pi reads LOW. | IEC signal | IEC DIN pin | Pi GPIO (BCM) | Header pin | Direction | |------------|-------------|---------------|------------|-----------| -| ATN | 3 | GPIO 2 | 3 | input (via shifter) | -| CLK | 4 | GPIO 17 | 11 | input (via shifter) | -| DATA | 5 | GPIO 18 | 12 | **bidirectional** (via shifter) | -| RESET | 6 | GPIO 3 | 5 | input (via shifter) | +| ATN | 3 | GPIO 4 | 7 | input (via shifter) | +| CLK | 4 | GPIO 3 | 5 | input (via shifter) | +| DATA | 5 | GPIO 2 | 3 | **bidirectional** (via shifter) | +| RESET | 6 | GPIO 17 | 11 | input (via shifter) | | GND | 2 | GND | 6 (or any) | — | | SRQ | 1 | — | — | not connected | +> ℹ️ **DATA (GPIO2) and CLK (GPIO3) are the Pi's ARM I²C pins** and carry the +> SoC's fixed ~1.8 kΩ pull-ups (benign — DATA is open-drain). Keep +> `dtparam=i2c_arm` **off** in `/boot/firmware/config.txt` (the default) so I²C +> does not claim these pins. This layout matches a confirmed-working reference +> listener. + ## DATA open-drain emulation -GPIO 18 is never a push-pull driver of the bus. The module flips its direction: +GPIO 2 is never a push-pull driver of the bus. The module flips its direction: -- **assert** (pull bus low): GPIO 18 = output **LOW** → shifter pulls bus to 0 V. -- **release / read**: GPIO 18 = **input (Hi-Z)** → bus pull-ups float it to 5 V, - and GPIO 18 reads the level the *talker* (C64) is driving — this is how bits +- **assert** (pull bus low): GPIO 2 = output **LOW** → shifter pulls bus to 0 V. +- **release / read**: GPIO 2 = **input (Hi-Z)** → bus pull-ups float it to 5 V, + and GPIO 2 reads the level the *talker* (C64) is driving — this is how bits are sampled during receive. Logical mapping (handled in `kernel/iec_lines.h`): asserted ⇔ Pi reads LOW. diff --git a/iecpoc/log.py b/iecpoc/log.py index 881d046..da7f26e 100644 --- a/iecpoc/log.py +++ b/iecpoc/log.py @@ -10,7 +10,7 @@ from __future__ import annotations from typing import Iterable, Iterator -from . import decode, device +from . import decode, device, petscii # bracketed state tag, padded so the content columns line up _TAG_WIDTH = 8 @@ -32,12 +32,19 @@ class TraceFormatter: the symbolic annotations and state events. """ - def __init__(self, my_address: int = 4, raw: bool = False): + def __init__(self, my_address: int = 4, raw: bool = False, debug: bool = False): self.my_address = my_address self.raw = raw + self.debug = debug + + @property + def plain(self) -> bool: + return not self.debug and not self.raw def format(self, rec: device.IecRecord) -> str | None: """Format one record. Returns ``None`` for records to be skipped.""" + if self.plain: + return self._format_plain(rec) if self.raw: # raw mode: just the byte value of command/data records if rec.is_command or rec.is_data: @@ -60,6 +67,23 @@ class TraceFormatter: yield line # -- per-kind formatters ------------------------------------------------- + def _format_plain(self, rec: device.IecRecord) -> str | None: + if rec.is_event: + return None + if rec.is_command: + cmd = decode.decode_command(rec.value) + if cmd.mnemonic == decode.LISTEN: + return f"\n--- LISTEN {cmd.primary} ---\n" + if cmd.mnemonic == decode.UNLISTEN: + return "\n--- UNLISTEN ---\n" + return None + if rec.is_data: + if rec.value in (0x0D, 0x0A, 0x8D): # CR, LF, SHIFT-CR + return "\n" + g = petscii.to_glyph(rec.value) + return g.text if g.printable else None + return None + def _format_event(self, rec: device.IecRecord) -> str: text = decode.describe_event(rec) if rec.value == device.EV_IDLE: diff --git a/iecpoc/main.py b/iecpoc/main.py index c1ae198..cb51c5c 100644 --- a/iecpoc/main.py +++ b/iecpoc/main.py @@ -33,6 +33,10 @@ def build_parser() -> argparse.ArgumentParser: "--replay", metavar="FILE", help="replay a captured record file instead of opening the device", ) + p.add_argument( + "--debug", action="store_true", + help="print the full annotated trace instead of plain received text", + ) p.add_argument( "--raw", action="store_true", help="dump a bare hex stream instead of the annotated trace", @@ -47,11 +51,12 @@ def build_parser() -> argparse.ArgumentParser: def run(records: Iterable[device.IecRecord], fmt: TraceFormatter, out: TextIO, logfile: TextIO | None = None) -> None: """Format ``records`` through ``fmt`` and write lines to ``out`` (+ logfile).""" + line_end = "" if fmt.plain else "\n" try: for line in fmt.format_stream(records): - print(line, file=out, flush=True) + print(line, end=line_end, file=out, flush=True) if logfile is not None: - print(line, file=logfile, flush=True) + print(line, end=line_end, file=logfile, flush=True) except BrokenPipeError: # downstream (e.g. `head`) closed the pipe; redirect stdout to devnull so # the interpreter's shutdown flush doesn't re-raise, then exit quietly. @@ -61,7 +66,7 @@ def run(records: Iterable[device.IecRecord], fmt: TraceFormatter, def main(argv: list[str] | None = None) -> int: args = build_parser().parse_args(argv) - fmt = TraceFormatter(my_address=args.address, raw=args.raw) + fmt = TraceFormatter(my_address=args.address, raw=args.raw, debug=args.debug) logfile = open(args.logfile, "a") if args.logfile else None try: diff --git a/kernel/README.md b/kernel/README.md index 97c7912..ebd48a7 100644 --- a/kernel/README.md +++ b/kernel/README.md @@ -43,6 +43,24 @@ actually capture and decode C64 traffic, use **`launch.sh`**, which installs the frontend, loads the matching module, runs `iecpoc`, and unloads on exit — see the [top-level README](../README.md#running-the-frontend-on-the-pi). +### Frontend output modes + +`iecpoc` has three output modes, selected by flags passed after `--` to `launch.sh` +(or directly on the command line when running `iecpoc` standalone): + +| Mode | Flag | Output | +|---|---|---| +| Plain text (default) | *(none)* | Received characters only; LISTEN/UNLISTEN print a divider; CR/LF become real newlines | +| Debug / annotated trace | `--debug` | Full `[CMD]` / `[DATA]` / `[ATN]` / `[IDLE]` tag-per-record trace | +| Raw hex | `--raw` | Bare uppercase hex bytes of command and data records, no annotations | + +```bash +sudo ./launch.sh # plain text — what the C64 is printing +sudo ./launch.sh -- --debug # full annotated trace +sudo ./launch.sh -- --raw # bare hex stream +sudo ./launch.sh -- --logfile trace.txt # also write output to a file +``` + You don't pick the file yourself: the script reads `uname -r`, then scans `modules/` (and a few fallback locations) and selects the `.ko` whose **vermagic** matches the running kernel. Matching is by vermagic rather than filename because @@ -92,18 +110,21 @@ Constants are defined in [`iec_listener.h`](iec_listener.h); the ioctl number is ### ⚠️ Bare-board caveat -With **nothing connected**, the expected result is **`0x15`**, *not* a fault: +With **nothing connected**, the expected result is **`0x0B`**, *not* a fault: - `DATA_ASSERT_OK` (`0x01`) passes — it's internal to the Pi. -- `ATN_RELEASED` (`0x04`) and `RESET_RELEASED` (`0x10`) pass **only because - GPIO2/GPIO3 have fixed ~1.8 kΩ pull-ups built into the BCM2710 SoC** (they are - the I²C0 pins; the pull-ups can't be disabled). They read high even with - nothing wired, so on a bare board these two bits prove **nothing** about your - wiring. -- `DATA_FLOAT_OK` (`0x02`) and `CLK_RELEASED` (`0x08`) read low because GPIO17/18 - have no such built-in pull-up and nothing is attached. +- `DATA_FLOAT_OK` (`0x02`) and `CLK_RELEASED` (`0x08`) pass **only because + DATA=GPIO2 and CLK=GPIO3 have fixed ~1.8 kΩ pull-ups built into the BCM2710 + SoC** (they are the I²C pins; the pull-ups can't be disabled). They read high + even with nothing wired, so on a bare board these two bits prove **nothing** + about your wiring. In particular `DATA_FLOAT_OK` now floats high via the SoC + pull-up even without the shifter, so it no longer confirms the shifter's DATA + pull-up — it's only meaningful once the shifter is wired. +- `ATN_RELEASED` (`0x04`) and `RESET_RELEASED` (`0x10`) read low because + ATN=GPIO4 and RESET=GPIO17 have no such built-in pull-up and nothing is + attached. -So on a bare board the only meaningful signal is `DATA_ASSERT_OK`. A full `0x1F` -is only reachable once the level shifter (with its CLK/DATA pull-ups) is wired -and powered. To make ATN/RESET meaningful, briefly ground each at the connector -and confirm the corresponding bit *drops*. +So on a bare board the only fully internal signal is `DATA_ASSERT_OK`. A full +`0x1F` is only reachable once the level shifter (with its ATN/RESET pull-ups) is +wired and powered. To make ATN/RESET meaningful, briefly ground each at the +connector and confirm the corresponding bit *drops*. diff --git a/kernel/dts/iec-overlay.dts b/kernel/dts/iec-overlay.dts index b003222..c2fe67d 100644 --- a/kernel/dts/iec-overlay.dts +++ b/kernel/dts/iec-overlay.dts @@ -1,10 +1,15 @@ /* * iec-overlay.dts - pin reservation + pull configuration for the IEC listener. * - * GPIO 2 = ATN (input, pull-up: bus idle = 5 V = high) - * GPIO 17 = CLK (input, pull-up) - * GPIO 3 = RESET (input, pull-up) - * GPIO 18 = DATA (bidirectional; starts as input/Hi-Z, the module flips it) + * GPIO 4 = ATN (input, pull-up: bus idle = 5 V = high) + * GPIO 3 = CLK (input, pull-up) + * GPIO 17 = RESET (input, pull-up) + * GPIO 2 = DATA (bidirectional; starts as input/Hi-Z, the module flips it) + * + * NOTE: DATA (GPIO2) and CLK (GPIO3) are the ARM I2C pins and carry the SoC's + * fixed ~1.8 kOhm pull-ups, which the overlay's pull settings cannot fully + * override. Benign here (DATA is open-drain). Keep dtparam=i2c_arm OFF so I2C + * does not claim these pins. * * The module drives GPIO via direct registers, so this overlay's main job is to * reserve the pins and set sane pull defaults. Build & install: @@ -25,12 +30,12 @@ target = <&gpio>; __overlay__ { iec_input_pins: iec_input_pins { - brcm,pins = <2 17 3>; /* ATN, CLK, RESET */ + brcm,pins = <4 3 17>; /* ATN, CLK, RESET */ brcm,function = <0>; /* 0 = input */ brcm,pull = <2>; /* 2 = pull-up */ }; iec_data_pin: iec_data_pin { - brcm,pins = <18>; /* DATA */ + brcm,pins = <2>; /* DATA */ brcm,function = <0>; /* start as input (Hi-Z) */ brcm,pull = <0>; /* 0 = none (shifter drives) */ }; diff --git a/kernel/iec_lines.h b/kernel/iec_lines.h index 0d98e89..3dff4e1 100644 --- a/kernel/iec_lines.h +++ b/kernel/iec_lines.h @@ -10,8 +10,8 @@ * bus released (5 V) <=> Pi GPIO reads HIGH * * DATA is open-drain emulated on a single bidirectional pin: - * assert : drive GPIO18 OUTPUT LOW -> shifter pulls bus to 0 V - * release : set GPIO18 INPUT (Hi-Z) -> bus pull-ups float it to 5 V; + * assert : drive GPIO2 OUTPUT LOW -> shifter pulls bus to 0 V + * release : set GPIO2 INPUT (Hi-Z) -> bus pull-ups float it to 5 V; * the talker's bits can now be read back through the shifter. * * The hot path uses direct BCM register access (ioremap'd), which the kernel @@ -30,10 +30,10 @@ #include /* --- GPIO assignment (BCM numbering, PLAN.md §3.2) ----------------------- */ -#define IEC_GPIO_ATN 2 /* header pin 3 - input */ -#define IEC_GPIO_RESET 3 /* header pin 5 - input */ -#define IEC_GPIO_CLK 17 /* header pin 11 - input */ -#define IEC_GPIO_DATA 18 /* header pin 12 - bidirectional (open-drain) */ +#define IEC_GPIO_ATN 4 /* header pin 7 - input */ +#define IEC_GPIO_RESET 17 /* header pin 11 - input */ +#define IEC_GPIO_CLK 3 /* header pin 5 - input */ +#define IEC_GPIO_DATA 2 /* header pin 3 - bidirectional (open-drain) */ /* --- BCM2710A1 / BCM2837 (Pi Zero 2 W) peripheral map ------------------- */ /* Same peripheral base as RPi 3. RPi 1 = 0x20000000, RPi 4 = 0xFE000000. */ diff --git a/kernel/iec_listener.c b/kernel/iec_listener.c index 2314d33..e5bd10d 100644 --- a/kernel/iec_listener.c +++ b/kernel/iec_listener.c @@ -81,6 +81,97 @@ static DECLARE_WAIT_QUEUE_HEAD(iec_work_wq); /* listener addressing state (decoded in-kernel, just enough to participate) */ static bool addressed_listener; +/* + * --- debounced sense lines (glitch filter) ------------------------------- + * + * Mirrors the 5 us glitch filter from a confirmed-working reference listener. + * The raw iec_*_asserted() helpers in iec_lines.h read + * the GPIO register once; on a level-shifted 5 V<->3.3 V bus a single sample + * can land on noise and yield a wrong bit, with no chance to retry since the + * bit loop runs with IRQs off. These wrappers only believe a level change once + * it has held for IEC_DEBOUNCE_US; a shorter pulse is rejected as a glitch and + * the previous stable level is kept. + * + * Fast path (no change since last read) is a single register read, so the tight + * CLK polls in wait_clk() stay cheap; the debounce cost is paid only when an + * edge actually appears. udelay() is safe with IRQs disabled. + * + * Concurrency: the entire IEC receive path runs single-threaded in the worker + * kthread, so the per-line stable state needs no locking. The ATN ISR must NOT + * use these (it needs an instantaneous read and must not touch this state) -- + * it keeps using the raw iec_atn_asserted() from iec_lines.h. + */ +static int db_clk = 1; /* last stable level, 1 = released (high) = idle bus */ +static int db_data = 1; +static int db_atn = 1; +static int db_reset = 1; + +static int iec_read_stable(unsigned int pin, int *last_stable) +{ + int raw = iec_gpio_read(pin); + unsigned int held; + + if (raw == *last_stable) + return *last_stable; /* fast path: no change */ + + /* a transition appeared: require it to hold IEC_DEBOUNCE_US */ + for (held = 0; held < IEC_DEBOUNCE_US; held++) { + udelay(1); + if (iec_gpio_read(pin) != raw) + return *last_stable; /* bounced back -> glitch, ignore */ + } + *last_stable = raw; /* held steady -> accept */ + return raw; +} + +static inline bool db_clk_asserted(void) { return iec_read_stable(IEC_GPIO_CLK, &db_clk) == 0; } +static inline bool db_atn_asserted(void) { return iec_read_stable(IEC_GPIO_ATN, &db_atn) == 0; } +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 */ @@ -126,10 +217,10 @@ static enum iec_rx wait_clk(bool want_asserted, unsigned int timeout_us, { unsigned int waited = 0; - while (iec_clk_asserted() != want_asserted) { - if (iec_atn_asserted() == data_phase) + while (db_clk_asserted() != want_asserted) { + if (db_atn_asserted() == data_phase) return IEC_RX_ATN; - if (iec_reset_asserted()) + if (db_reset_asserted()) return IEC_RX_RESET; if (waited++ >= timeout_us) return IEC_RX_TIMEOUT; @@ -156,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 */ @@ -164,14 +255,14 @@ static enum iec_rx receive_byte(u8 *out, bool *eoi, bool data_phase) if (data_phase) { unsigned int waited = 0; - while (!iec_clk_asserted()) { - if (iec_atn_asserted()) { rc = IEC_RX_ATN; goto out; } - if (iec_reset_asserted()) { rc = IEC_RX_RESET; goto out; } + while (!db_clk_asserted()) { + if (db_atn_asserted()) { rc = IEC_RX_ATN; goto out; } + 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; } @@ -190,12 +281,21 @@ static enum iec_rx receive_byte(u8 *out, bool *eoi, bool data_phase) if (rc != IEC_RX_OK) goto out; /* released(high) = bit 1, asserted(low) = bit 0 */ - if (!iec_data_asserted()) + if (!db_data_asserted()) value |= (1u << i); } - /* 4. BYTE ACKNOWLEDGE: pull DATA low (Tf), hold for the next RFD */ - iec_data_assert(); + /* 4. BYTE ACKNOWLEDGE: wait for the talker to pull CLK low at end of + * byte (mirrors the reference listener), THEN pull DATA low (Tf). + * Without this wait we would assert the ack while CLK is still high + * from bit 7, racing the talker's end-of-byte edge and desyncing the + * handshake. The next receive_byte() releases DATA after CLK goes high + * again (RFD), completing the per-byte handshake. */ + rc = wait_clk(true /* asserted: talker finished the byte */, + IEC_CLK_TIMEOUT_US, data_phase); + if (rc != IEC_RX_OK) + goto out; + iec_data_assert_sync(); *out = value; rc = IEC_RX_OK; @@ -229,10 +329,14 @@ static void run_state_machine(void) bool eoi; enum iec_rx rc; - if (iec_reset_asserted()) + if (kthread_should_stop()) { + iec_data_release_sync(); + return; + } + if (db_reset_asserted()) goto reset; - if (iec_atn_asserted()) { + if (db_atn_asserted()) { /* RECEIVE_COMMAND: ATN held low */ rc = receive_byte(&b, &eoi, false); if (rc == IEC_RX_RESET) @@ -249,16 +353,20 @@ 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 */ } /* LISTENER: data phase */ emit_record(IEC_KIND_EVENT, IEC_EV_ATN_RELEASED, IEC_FLAG_ADDRESSED); for (;;) { - if (iec_reset_asserted()) + if (kthread_should_stop()) { + iec_data_release_sync(); + return; + } + if (db_reset_asserted()) goto reset; - if (iec_atn_asserted()) { + if (db_atn_asserted()) { /* C64 interrupts with a new command */ emit_record(IEC_KIND_EVENT, IEC_EV_ATN_COMMAND, 0); break; @@ -279,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); } @@ -304,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); } diff --git a/kernel/iec_timing.h b/kernel/iec_timing.h index 10ffb39..10632ab 100644 --- a/kernel/iec_timing.h +++ b/kernel/iec_timing.h @@ -25,8 +25,12 @@ * 80 us gives margin. */ #define IEC_EOI_ACK_HOLD_US 80 -/* Debounce between consecutive bus reads (mirrors sd2iec / 1571 ROM). */ -#define IEC_DEBOUNCE_US 2 +/* Glitch filter window for sense-line reads. A level change is only believed + * once it has held this long; a shorter pulse is treated as noise. Matches a + * confirmed-working reference listener (5 us). sd2iec / the 1571 ROM + * use a comparable few-microsecond debounce. Far shorter than a real IEC bit + * (tens of us), so it never smears valid data. */ +#define IEC_DEBOUNCE_US 5 /* Between-bytes minimum hold before the talker releases CLK for the next byte * (Tbb). We simply wait for the next CLK transition; this is documentary. */ diff --git a/kernel/selftest.sh b/kernel/selftest.sh index 1b39b7a..48d6fef 100755 --- a/kernel/selftest.sh +++ b/kernel/selftest.sh @@ -189,8 +189,9 @@ if [ "$ST" -eq 0 ]; then ok "self-test PASSED (0x1F) - all four lines wired correctly" else info "self-test did not fully pass." - info "On a BARE board 0x15 is expected and fine: only DATA_ASSERT_OK is" - info "meaningful; ATN/RESET pass via the SoC's fixed GPIO2/3 pull-ups." + info "On a BARE board 0x0b is expected and fine: DATA_FLOAT_OK + CLK_RELEASED" + info "pass via the SoC's fixed GPIO2/3 (DATA/CLK) pull-ups, and DATA_ASSERT_OK" + info "is internal; ATN/RESET read low (GPIO4/17 have no built-in pull-up)." info "Re-run once the level shifter is wired + powered; you want 0x1F." fi exit "$ST" diff --git a/launch.sh b/launch.sh index 2780313..f63e935 100755 --- a/launch.sh +++ b/launch.sh @@ -19,11 +19,13 @@ # sudo ./launch.sh [--address N] [--device PATH] [--ko FILE] [-- ] # # Examples: -# sudo ./launch.sh # address 4, /dev/iec0, annotated trace +# sudo ./launch.sh # address 4, /dev/iec0, plain text (default) # sudo ./launch.sh --address 8 # listen as device 8 +# sudo ./launch.sh -- --debug # full annotated trace (verbose) # sudo ./launch.sh -- --raw # forward --raw to iecpoc # sudo ./launch.sh --ko modules/iec_listener_1-6.12.93-1+rpt1.ko # +# Any flag after -- is forwarded verbatim to iecpoc (--debug, --raw, --logfile FILE, …). # Defaults: address=4 (printer), device=/dev/iec0. set -euo pipefail diff --git a/tests/test_device.py b/tests/test_device.py index 2a47ed4..545da27 100644 --- a/tests/test_device.py +++ b/tests/test_device.py @@ -16,7 +16,11 @@ from . import fixtures def _trace(records, my_address=4, raw=False): - return list(TraceFormatter(my_address=my_address, raw=raw).format_stream(records)) + return list(TraceFormatter(my_address=my_address, raw=raw, debug=True).format_stream(records)) + + +def _plain(records, my_address=4): + return list(TraceFormatter(my_address=my_address).format_stream(records)) # --- wire-format round trip ------------------------------------------------- @@ -101,3 +105,90 @@ def test_raw_mode_is_hex_only(): assert trace[0] == "24" # data bytes present as hex assert "48" in trace # 'H' + + +# --- plain mode ------------------------------------------------------------- +def _make_record(kind, value, flags=0): + from iecpoc.device import IecRecord + return IecRecord(kind, value, flags, 0) + + +def test_plain_property(): + assert TraceFormatter(debug=False).plain is True + assert TraceFormatter(debug=True).plain is False + assert TraceFormatter(raw=True).plain is False + + +def test_plain_listen_divider(): + rec = _make_record(device.KIND_COMMAND, 0x24, device.FLAG_ADDRESSED) # LISTEN 4 + out = _plain([rec]) + assert len(out) == 1 + assert "LISTEN 4" in out[0] + + +def test_plain_unlisten_divider(): + rec = _make_record(device.KIND_COMMAND, 0x3F) # UNLISTEN + out = _plain([rec]) + assert len(out) == 1 + assert "UNLISTEN" in out[0] + + +def test_plain_data_printable(): + rec = _make_record(device.KIND_DATA, 0x48, device.FLAG_ADDRESSED) # 'H' + assert _plain([rec]) == ["H"] + + +def test_plain_data_cr_yields_newline(): + rec = _make_record(device.KIND_DATA, 0x0D, device.FLAG_ADDRESSED) + assert _plain([rec]) == ["\n"] + + +def test_plain_data_lf_yields_newline(): + rec = _make_record(device.KIND_DATA, 0x0A, device.FLAG_ADDRESSED) + assert _plain([rec]) == ["\n"] + + +def test_plain_data_shift_cr_yields_newline(): + rec = _make_record(device.KIND_DATA, 0x8D, device.FLAG_ADDRESSED) + assert _plain([rec]) == ["\n"] + + +def test_plain_events_are_silent(): + recs = [ + _make_record(device.KIND_EVENT, device.EV_IDLE), + _make_record(device.KIND_EVENT, device.EV_ATN_ASSERTED), + _make_record(device.KIND_EVENT, device.EV_ATN_RELEASED), + ] + assert _plain(recs) == [] + + +def test_plain_non_listen_commands_silent(): + recs = [ + _make_record(device.KIND_COMMAND, 0xF0), # OPEN SA=0 + _make_record(device.KIND_COMMAND, 0xE0), # CLOSE SA=0 + _make_record(device.KIND_COMMAND, 0x60), # SECOND/DATA SA=0 + _make_record(device.KIND_COMMAND, 0x48), # TALK 8 + _make_record(device.KIND_COMMAND, 0x5F), # UNTALK + ] + assert _plain(recs) == [] + + +def test_plain_graphic_bytes_silent(): + rec = _make_record(device.KIND_DATA, 0x70, device.FLAG_ADDRESSED) # graphic range + assert _plain([rec]) == [] + + +def test_plain_control_bytes_silent(): + rec = _make_record(device.KIND_DATA, 0x12, device.FLAG_ADDRESSED) # RVS-ON + assert _plain([rec]) == [] + + +def test_plain_full_session(): + out = _plain(fixtures.build_session()) + joined = "".join(out) + assert "HELLO WORLD" in joined + assert "LINE TWO" in joined + assert "LISTEN 4" in joined + assert "UNLISTEN" in joined + # each PRINT# record ends with CR → two newlines from data, plus divider newlines + assert joined.count("\n") >= 2