From fbf581149619617c74d6070a72fdd22dca737988 Mon Sep 17 00:00:00 2001 From: Allen Hill Date: Sat, 19 Sep 2026 13:35:46 -0700 Subject: [PATCH] Refine (further) the Phy API - Refactor send/read data to read entire array instead of byte-oriented reads - Simplify/improve consistency of asynchrony (all send functions now optionally async) - Fix start-bit TX (now actually validate joinable start-bits) Assisted-by: Claude Code (Opus 5) --- src/avclan/bus.cc | 35 +-- src/avclan/hal/phy.h | 39 ++-- src/avclan/target/avr-attiny3216/phy_avr.c | 28 ++- src/avclan/target/pico2/iebus.pio | 93 ++++---- src/avclan/target/pico2/phy.cc | 248 ++++++++++----------- 5 files changed, 236 insertions(+), 207 deletions(-) diff --git a/src/avclan/bus.cc b/src/avclan/bus.cc index 50b9576..0d256a5 100644 --- a/src/avclan/bus.cc +++ b/src/avclan/bus.cc @@ -81,7 +81,9 @@ public: }; Read read_control(uint8_t *control) { return phy_read_control(control); }; Read read_length(uint8_t *length) { return phy_read_length(length); }; - Read read_data(uint8_t *data) { return phy_read_data(data); }; + Read read_data(uint8_t *data, uint8_t length, uint8_t *data_index) { + return phy_read_data(data, length, data_index); + }; Send send_header(bool is_unicast) { return phy_send_header(is_unicast); }; Send send_controller_addr(uint16_t addr) { @@ -96,8 +98,9 @@ public: Send send_length(uint8_t length, bool expect_ack) { return phy_send_length(length, expect_ack); }; - Send send_data(uint8_t data, bool expect_ack) { - return phy_send_data(data, expect_ack); + Send send_data(const uint8_t *data, uint8_t length, bool expect_ack, + uint8_t *data_index) { + return phy_send_data(data, length, expect_ack, data_index); }; // NOLINTEND(readability-convert-member-functions-to-static) }; @@ -183,14 +186,13 @@ auto Bus::read(Frame::Print print) goto handle_err; } - for (uint8_t i = 0; i < in->length; i++) { - err.type = handle.read_data(&in->data[i]); - if (err.type != Read{0}) { - if (print.verbose) - err.val = in->data[i]; + uint8_t data_i = 0; + err.type = handle.read_data(in->data, in->length, &data_i); + if (err.type != Read{0}) { + if (print.verbose) + err.val = in->data[data_i]; - goto handle_err; - } + goto handle_err; } } // destroy handle @@ -279,17 +281,16 @@ auto Bus::send(const Frame &out, Frame::Print print) -> Send { if (err.type != Send{0}) goto handle_err; - for (uint8_t i = 0; i < out.length; i++) { - err.type = handle.send_data(out.data[i], out.is_unicast); - if (err.type != Send{0}) { - err.val = i; - goto handle_err; - } - } + err.type = + handle.send_data(out.data, out.length, out.is_unicast, &err.val); + if (err.type != Send{0}) + goto handle_err; // A phy that only queued the fields above settles them here uint8_t data_i = 0; err.type = phy_send_done(&data_i); + if (err.type == BUSY || err.type == LOST_ARBITRATION) // Queued header + continue; if (err.type != Send{0}) { err.val = data_i; goto handle_err; diff --git a/src/avclan/hal/phy.h b/src/avclan/hal/phy.h index c67f134..bdc7240 100644 --- a/src/avclan/hal/phy.h +++ b/src/avclan/hal/phy.h @@ -50,14 +50,13 @@ void phy_guard_leave(void); /* Per-field frame I/O. * - * Excluding the header and controller_addr send functions, all other send - * functions may have asynchronous implementations (e.g. return before the send - * has completed on the bus). Success is indicated by a zero value `Read` or - * `Send` enum. Non-zero error codes indicate a synchronously completed send - * failure. Otherwise, `phy_send_done` must be called to block until all queued - * send's have completed, and may return the error code for a previous (queued) - * send failure; a success return value indicates that all queued send's have - * finished sending over the bus. + * All send functions may have asynchronous implementations (e.g. return before + * the send has completed on the bus). Success is indicated by a zero value + * `Read` or `Send` enum. Non-zero error codes indicate a synchronously + * completed send failure. Otherwise, `phy_send_done` must be called to block + * until all queued send's have completed, and may return the error code for a + * previous (queued) send failure; a success return value indicates that all + * queued send's have finished sending over the bus. * * The read functions are similarly optionally asynchronous, and may return the * results of buffered reads. When this is the case, phy_read_header returns the @@ -75,25 +74,31 @@ Read phy_read_controller_addr(uint16_t *addr); Read phy_read_peripheral_addr(uint16_t *addr); Read phy_read_control(uint8_t *control); Read phy_read_length(uint8_t *length); -Read phy_read_data(uint8_t *data); +// Read `length` data bytes. `data_index` is only written to on error, with the +// index of the failed byte. +Read phy_read_data(uint8_t *data, uint8_t length, uint8_t *data_index); -// Send start and broadcast bits. Always synchronous. Returns success or one of -// these error values: MUTED, BUSY, or LOST_ARBITRATION (if another device -// overrides our frame with a broadcast). +// Send start and broadcast bits. Returns success or one of these error values: +// MUTED, BUSY, or LOST_ARBITRATION (if another device overrides our frame with +// a broadcast). Send phy_send_header(bool is_unicast); -// Send the controller address. Always synchronous. Returns success or -// LOST_ARBITRATION (a device with a lower device is sending a frame). +// Send the controller address. Returns success or LOST_ARBITRATION (a device +// with a lower device is sending a frame). Send phy_send_controller_addr(uint16_t addr); Send phy_send_peripheral_addr(uint16_t addr, bool expect_ack); Send phy_send_control(uint8_t control, bool expect_ack); Send phy_send_length(uint8_t length, bool expect_ack); -Send phy_send_data(uint8_t data, bool expect_ack); +// Send `length` data bytes. `data_index` is only written to for NAK_DATA. +Send phy_send_data(const uint8_t *data, uint8_t length, bool expect_ack, + uint8_t *data_index); // Allows asynchronous ports to block until the phy has finished sending the // frame. Returns success or the relevant field-specific NAK (e.g. NAK_ADDRESS, -// etc) or CONTENDED_BUS. `data_index` is only written to for NAK_DATA. A fully -// synchronous port should always report success. +// etc) or CONTENDED_BUS; a port that queues the header and controller address +// also reports their BUSY or LOST_ARBITRATION here. `data_index` is only +// written to for NAK_DATA. A fully synchronous port should always report +// success. Send phy_send_done(uint8_t *data_index); #ifndef NDEBUG diff --git a/src/avclan/target/avr-attiny3216/phy_avr.c b/src/avclan/target/avr-attiny3216/phy_avr.c index e6db06b..1b4dc87 100644 --- a/src/avclan/target/avr-attiny3216/phy_avr.c +++ b/src/avclan/target/avr-attiny3216/phy_avr.c @@ -543,11 +543,15 @@ Read phy_read_length(uint8_t *length) { return (Read)0; } -Read phy_read_data(uint8_t *data) { - const Read err = read_parity(phy_read_byte(data), BAD_DATA_PARITY); - if (err != (Read)0) - return err; - ack_slot(); +Read phy_read_data(uint8_t *data, uint8_t length, uint8_t *data_index) { + for (uint8_t i = 0; i < length; i++) { + const Read err = read_parity(phy_read_byte(&data[i]), BAD_DATA_PARITY); + if (err != (Read)0) { + *data_index = i; + return err; + } + ack_slot(); + } return (Read)0; } @@ -595,9 +599,17 @@ Send phy_send_length(uint8_t length, bool expect_ack) { return send_ack_slot(expect_ack, NAK_MESSAGE_LENGTH); } -Send phy_send_data(uint8_t data, bool expect_ack) { - phy_send_bit(phy_send_byte(&data)); - return send_ack_slot(expect_ack, NAK_DATA); +Send phy_send_data(const uint8_t *data, uint8_t length, bool expect_ack, + uint8_t *data_index) { + for (uint8_t i = 0; i < length; i++) { + phy_send_bit(phy_send_byte(&data[i])); + const Send err = send_ack_slot(expect_ack, NAK_DATA); + if (err != (Send)0) { + *data_index = i; + return err; + } + } + return (Send)0; } // Every field above is on the wire, and has reported its own outcome, by the diff --git a/src/avclan/target/pico2/iebus.pio b/src/avclan/target/pico2/iebus.pio index 6f6deb6..2c54083 100644 --- a/src/avclan/target/pico2/iebus.pio +++ b/src/avclan/target/pico2/iebus.pio @@ -2,8 +2,8 @@ .define public ack_irq 5 ; (PIO1) rx -> iebus_ack: hold this slot low ; This is used to set the clkdiv. -; The delay cycle counts in read_bit (AVCLAN_READBIT_THRESHOLD) and iebus_ack -; (AVCLAN_BIT0_LOGIC_0) must be updated in sync with this variable +; The delay cycle counts in read_bit (AVCLAN_READBIT_THRESHOLD) and iebus_tx's +; ack_drive (AVCLAN_BIT0_LOGIC_0) must be updated in sync with this variable .define public CYCLES_PER_READBIT_PERIOD 64 .pio_version 1 @@ -105,18 +105,18 @@ push_slot: out x, 1 ; Load "has ACK" jmp !x wait_read wait 1 pin 0 - wait 0 pin 0 [31] ; Sender begins the ACK bit, and will hold it dominant - ; for ~48 cycles (~AVCLAN_BIT1_LOGIC_0) before releasing + wait 0 pin 0 [30] ; Sender begins the ACK bit, and will hold it dominant + ; for ~48 cycles (~AVCLAN_BIT1_LOGIC_0) before releasing ; for peripheral to take over ; Delay reloading ack_latch to give the driver as much ; time as possible to veto an ACK (i.e. clear it for ; e.g. a bad length) mov x, status ; Load ack_latch state into x jmp !x wait_read - irq next set ack_irq ; iebus_ack released 33 cycles after sender began bit - ; (~65% of AVCLAN_BIT1_LOGIC_0), well - ; iebus_ack, on the other (TX) PIO, holds it - ; dominant. + irq next set ack_irq ; The ack SM (iebus_tx's ack_entry, on the other (TX) + ; PIO) is released 32 cycles after sender began bit, + ; and its jmp to ack_drive takes the slot over at 34 + ; (~70% of AVCLAN_BIT1_LOGIC_0), holding it dominant. .wrap ; Can execute one streamed instruction every 3 cycles @@ -128,21 +128,6 @@ push_slot: ; jmp x-- do_exec ; Cycle 3: Restart loop (if x != 0) ; jmp wait_read -; Send an ack for iebus_rx. Must be loaded on the same PIO as iebus_tx to properly -; share the IEBUS_TX pin -; Drives bus dominant for *less than* a complete AVCLAN_BIT0_LOGIC_0 duration. -; Delays are coordinated with the iebus_rx release to ensure this SM releases the -; bus to recessive at the correct time -.program iebus_ack -.side_set 1 - - ; Released by iebus_rx after bus goes dominant - wait 1 irq ack_irq side 1 ; Idle recessive; the wait clears the flag - set x, 1 side 0 [15] ; Dominant, taking the slot over from the sender -hold: - jmp x-- hold side 0 [15] ; 16 + 2*16 = 48 cycles, releasing at - ; ~AVCLAN_BIT0_LOGIC_0 after the slot's leading edge - .program iebus_tx .fifo tx @@ -161,26 +146,45 @@ hold: .define public lost_arb_irq 6 ;;; Emit or synchronize to a start bit -; Reached by a CPU initiated force exec jmp -public tx_startbit: - jmp pin originate ; Recessive == bus idle => we originate - jmp joined +; Reached by a CPU initiated force exec jmp to tx_startbit, with x = JOIN_SAMPLES +; and y = IDLE_SAMPLES. We originate once the bus has stayed recessive for the +; idle window. A dominant pulse is joined only if it outlasts every bit's +; dominant period (i.e. it's a start bit); otherwise the bus is busy, reported as +; a lost bid before any word is pulled. +.define public IDLE_SAMPLES 11 ; 12 * 16 = 192 cycles (~2 * AVCLAN_BIT_LENGTH_MAX) +; TODO: Explore tightening the idle window to 3 * AVCLAN_STARTBIT_LOGIC_1 (the +; longest recessive period within a frame) +.define public JOIN_SAMPLES 8 ; 9 * 12 = 108 cycles (> 1.2 * AVCLAN_BIT0_LOGIC_0) -originate: - set x 24 - set pins, 0 [15] ; 16 cycles, then +idle_ok: + jmp y-- tx_startbit [14] ; Sampled every 16 cycles + set x 31 ; Idle: originate + set pins, 0 [30] +public tx_startbit: + jmp pin idle_ok hold: - jmp x-- hold [15] ; 25 * 16 = 416 total (~AVCLAN_STARTBIT_LOGIC_0) + jmp pin lost ; Released too soon for a start bit (never while we drive) + jmp x-- hold [10] ; Sampled every 12 cycles, within a zero bit's recessive + ; period (~AVCLAN_BIT0_LOGIC_1). Ours is 31 + 1 + 32 * 12 + ; = 416 total (~AVCLAN_STARTBIT_LOGIC_0) set pins, 1 ; Release -joined: wait 1 pin 0 [31] ; The start bit's trailing edge, whoever drove it jmp reset [16] ; 51 cycles from that edge to reset's first ; `set pins, 0` (~AVCLAN_STARTBIT_LOGIC_1) handle_nak: - jmp x-- reset ; Reset if NAK was expected + jmp x-- ack_done ; Slot is over either way if a NAK was expected irq wait nak_irq +;;; Close the acknowledge slot +; The peripheral owns this slot's trailing edge, so the recessive period that +; ends the slot is timed from its release rather than from our leading edge -- +; the one bit in a frame we don't drive to completion ourselves. A slot nobody +; answered is already recessive and falls straight through. +ack_done: + wait 1 pin 0 [12] ; 15 cycles from the release to `set pins, 0` below + ; (~AVCLAN_BIT0_LOGIC_1) + reset: .wrap_target pull @@ -202,6 +206,7 @@ bit_zero: ; device sending a zero will "override" the value of that bit) jmp !y bit_end [2] ; Delay 3 more cycles before releasing bus at ~AVCLAN_BIT0_LOGIC_0 ; Fall through means we lost arbitration (we read a zero, when we expected a one) +lost: irq wait lost_arb_irq .wrap @@ -212,19 +217,29 @@ bit_end: jmp !x reset ; Fall through to read ACK if "has ACK" is set ;;; read ack +public ack_drive: set pins, 0 [31] ; Start ACK bit with a AVCLAN_BIT1_LOGIC_0 length pulse out x, 1 [15] ; Preload NAK ok +public ack_release: set pins, 1 [15] ; Release bus - jmp pin handle_nak [28] ; Sampled at 64 cycles (~AVCLAN_READBIT_THRESHOLD), - ; then delayed for the full 96-cycle logic-0 period - ; (Both flow paths add one more instr) - jmp reset + jmp pin handle_nak [13] ; Sampled at 64 cycles (~AVCLAN_READBIT_THRESHOLD); + ; both flow paths reach ack_done one instr later + jmp ack_done slow_jmp: -;;; The recessive ("prep") period in the send_bit loop is 2 instr's shorter than -; the non-"has ACK" branch and +;;; The recessive ("prep") period in the send_bit loop is 2 instr's shorter than +; the non-"has ACK" branch and jmp send_bit [3] +;;; Send an ack for iebus_rx +; Run by its own SM, wrapping from ack_release back to ack_entry: it borrows +; ack_drive's 48-cycle pulse to hold the slot dominant for *less than* a complete +; AVCLAN_BIT0_LOGIC_0, releasing at ~AVCLAN_BIT0_LOGIC_0 after the slot's +; leading edge. +public ack_entry: + wait 1 irq ack_irq ; Released by iebus_rx after bus goes dominant + jmp ack_drive + ; `mov pins, pins` reads the IN mapping (bit 0 == in base) and writes the OUT ; mapping (out base, 1 pin), so one program instance serves any src->dst pair; ; give each pair its own SM with its own pin mapping. diff --git a/src/avclan/target/pico2/phy.cc b/src/avclan/target/pico2/phy.cc index 2411c31..81b3e8f 100644 --- a/src/avclan/target/pico2/phy.cc +++ b/src/avclan/target/pico2/phy.cc @@ -1,15 +1,15 @@ #include -#include #include #include +#include #include +#include #include "avclan.h" #include "hal/phy.h" #include "hardware/gpio.h" #include "hardware/pio.h" #include "iebus.pio.h" -#include "phy_debug.hpp" #define TICK_US 1000000 #include "timing.h" @@ -160,13 +160,13 @@ public: } // Disarm RX ACK'ing behavior; called prior to frame TX to prevent - // self-ACK'ing. Safe to rearm any time after sending controller addr. + // self-ACK'ing. void disarm_ack() { transmitting_ = true; sync_ack_arming(); } - // Rearm RX ACK'ing behavior; called after sending controller addr. + // Rearm RX ACK'ing behavior; called once our frame is over or lost. void rearm_ack() { transmitting_ = false; sync_ack_arming(); @@ -189,6 +189,13 @@ private: static void __time_critical_func(irq_handler)() { instance_->isr(); } void __time_critical_func(isr)() { + // Every field after the controller address is followed by an ack slot, + // which the SM consumes whether or not it drives it. + const auto next_after_ack = [this](RxField next, uint8_t bits) { + state_ = next; + enqueue_rx(bits, true); + }; + while (!pio_sm_is_rx_fifo_empty(pio_, sm_)) { const auto slot = (uint16_t)pio_sm_get(pio_, sm_); uint16_t value = 0; @@ -227,9 +234,7 @@ private: // still ahead of the SM. Re-dispatching the start-bit block hands // the rest of the frame back to it: those bits are far too short to // read as a start bit, so it re-syncs on the next real one by - // itself. This also settles a race on our own frames -- rearm_ack() - // restores y the moment we win the controller address, in time for - // that same read to have matched it. + // itself. pio_interrupt_clear(pio_, ack_latch); begin_frame(); break; @@ -359,13 +364,6 @@ private: begin_frame(); } - // Every field after the controller address is followed by an ack slot, which - // the SM consumes whether or not it drives it. - void __time_critical_func(next_after_ack)(RxField next, uint8_t bits) { - state_ = next; - enqueue_rx(bits, true); - } - // The initialized instance, for irq_handler: SDK IRQ handlers take no // context. static inline IEBusRx *instance_ = nullptr; @@ -401,16 +399,17 @@ private: bool controller_ok_ = false; // parity verdict, held until the peripheral slot }; -// The transmit engine: the iebus_tx SM, plus the iebus_ack SM that drives the -// ack slot on the rx engine's behalf. Both live on one PIO because they share -// the TX pin. +// The transmit engine: the iebus_tx SM, plus the ack SM that drives the ack +// slot on the rx engine's behalf. Both run iebus_tx, sharing its ack-bit drive +// and the TX pin. // -// Everything past the arbitration window is queued, not sent, so the fields -// report nothing and the frame's verdict comes from send_done. +// Every field, arbitration included, is queued, not sent, so the fields report +// nothing and the frame's verdict comes from send_done. class IEBusTx { - // The flag the SM parked on. Sticky until reported; nothing more is queued - // meanwhile. - enum class Fault : uint8_t { None, Nak, Mismatch }; + // Word positions in a frame + static constexpr uint8_t WORD_PERIPHERAL = 2; // After broadcast, controller + static constexpr uint8_t WORD_CONTROL = 3; + static constexpr uint8_t WORD_LENGTH = 4; public: // The ack match lives in the rx SM but exists for our sake: it has to be @@ -430,8 +429,7 @@ public: pio_set_sm_mask_enabled(pio_, sm_mask(), false); pio_remove_program_and_unclaim_sm(&iebus_tx_program, pio_, tx_sm_, tx_offset_); - pio_remove_program_and_unclaim_sm(&iebus_ack_program, pio_, ack_sm_, - ack_offset_); + pio_sm_unclaim(pio_, ack_sm_); } // Claims and configures both SMs, but doesn't start them. @@ -439,7 +437,6 @@ public: pio_ = pio; pin_tx_ = pin_tx; tx_offset_ = (uint)pio_add_program(pio_, &iebus_tx_program); - ack_offset_ = (uint)pio_add_program(pio_, &iebus_ack_program); tx_sm_ = (uint)pio_claim_unused_sm(pio_, true); ack_sm_ = (uint)pio_claim_unused_sm(pio_, true); @@ -455,17 +452,21 @@ public: sm_config_set_set_pins(&tx_cfg, pin_tx, 1); // Both the bit value and the arbitration check come from the readback. sm_config_set_jmp_pin(&tx_cfg, pin_rx); + // The start bit's trailing-edge `wait` indexes the IN mapping, not JMP_PIN. + sm_config_set_in_pins(&tx_cfg, pin_rx); sm_config_set_clkdiv(&tx_cfg, bus_clkdiv()); - // Entry is `reset`, not the program start -- offset 0 is handle_nak, which - // would read the first word's top bit as the NAK flag. + // Entry is `reset`; offset 0 is the start-bit block. pio_sm_init(pio_, tx_sm_, tx_offset_ + iebus_tx_wrap_target, &tx_cfg); - pio_sm_config ack_cfg = iebus_ack_program_get_default_config(ack_offset_); - sm_config_set_sideset_pins(&ack_cfg, pin_tx); + pio_sm_config ack_cfg = iebus_tx_program_get_default_config(tx_offset_); + sm_config_set_wrap(&ack_cfg, tx_offset_ + iebus_tx_offset_ack_entry, + tx_offset_ + iebus_tx_offset_ack_release); + sm_config_set_set_pins(&ack_cfg, pin_tx, 1); sm_config_set_clkdiv(&ack_cfg, bus_clkdiv()); - pio_sm_init(pio_, ack_sm_, ack_offset_, &ack_cfg); + pio_sm_init(pio_, ack_sm_, tx_offset_ + iebus_tx_offset_ack_entry, + &ack_cfg); claimed_ = true; } @@ -504,53 +505,67 @@ public: return MUTED; rx_.disarm_ack(); + words_ = 0; + data_start_ = std::numeric_limits::max(); - // Jump to the tx_startbit section from the default "reset" stall on pull - pio_sm_exec(pio_, tx_sm_, - pio_encode_jmp(tx_offset_ + iebus_tx_offset_tx_startbit)); + // Jump to the tx_startbit section from the default "reset" stall on pull, + // with the start-bit block's sample counts loaded + pio_sm_exec_wait_blocking(pio_, tx_sm_, + pio_encode_set(pio_x, iebus_tx_JOIN_SAMPLES)); + pio_sm_exec_wait_blocking(pio_, tx_sm_, + pio_encode_set(pio_y, iebus_tx_IDLE_SAMPLES)); + pio_sm_exec_wait_blocking( + pio_, tx_sm_, pio_encode_jmp(tx_offset_ + iebus_tx_offset_tx_startbit)); // The broadcast bit carries no parity, so it is sent as a bare bit: length // 0 (one bit) with the parity slot standing in for the bit itself. - return arbitrate(is_unicast ? 1U : 0U, 0); + return send_field(0, is_unicast ? 1U : 0U, false, false); } - Send send_controller_addr(uint16_t addr) { - // Last field of the arbitration window; no acknowledge slot follows it. - const Send err = arbitrate(addr, 12); - if (err == Send{0}) { - // We won: nobody else is transmitting, so the match can come back. - rx_.rearm_ack(); - words_ = 0; - } - return err; - } - - // Past arbitration a field is only queued, and its outcome left to - // send_done. Once one has failed, the rest of the frame is dropped. - Send send_field(size_t len, uint32_t bits, bool expect_ack) { + // A field is only queued, and its outcome left to send_done. Once one has + // failed, the rest of the frame is dropped. + Send send_field(size_t len, uint32_t bits, bool has_ack, bool expect_ack) { if (muted_) return MUTED; if (!check()) - put(encode_tx(len, (uint16_t)bits, true, expect_ack)); + put(encode_tx(len, (uint16_t)bits, has_ack, expect_ack)); + return Send{0}; + } + + Send send_data(const uint8_t *data, uint8_t length, bool expect_ack) { + if (muted_) + return MUTED; + data_start_ = words_; + for (uint8_t i = 0; i < length && !check(); i++) + put(encode_tx(8, data[i], true, expect_ack)); return Send{0}; } Send send_done(uint8_t *data_index) { - wait_done(); - check(); - const Fault fault = fault_; - fault_ = Fault::None; + const auto flagged = [this] { + return pio_interrupt_get(pio_, iebus_tx_lost_arb_irq) || + pio_interrupt_get(pio_, iebus_tx_nak_irq); + }; - if (fault == Fault::None) - return Send{0}; - if (fault == Fault::Mismatch) - return CONTENDED_BUS; - switch (failed_word_) { - case 0: return NAK_ADDRESS; - case 1: return NAK_CONTROL; - case 2: return NAK_MESSAGE_LENGTH; - default: *data_index = (uint8_t)(failed_word_ - 3); return NAK_DATA; - } + // The second PC test confirms that SM has finished sending last value and + // not in-progress (i.e. pulled the last FIFO value and still mid-send) + const auto idle = [this] { + return pio_sm_is_tx_fifo_empty(pio_, tx_sm_) && // Read before the pc + pio_sm_get_pc(pio_, tx_sm_) == tx_offset_ + iebus_tx_wrap_target; + }; + + while (!(flagged() || idle())) + tight_loop_contents(); + + check(); + // Our frame is over, won or not. + rx_.rearm_ack(); + words_ = 0; + const Send err = fault_; + fault_ = Send{0}; + if (err == NAK_DATA) + *data_index = (uint8_t)(failed_word_ - data_start_); + return err; } private: @@ -573,48 +588,46 @@ private: return word; } - bool flagged() const { - return pio_interrupt_get(pio_, iebus_tx_lost_arb_irq) || - pio_interrupt_get(pio_, iebus_tx_nak_irq); - } - - // The SM has nothing left to do: every path through a word ends back on - // `reset`'s pull. The pc test only counts once the FIFO is empty -- before - // the SM takes a word it is still sitting on that same pull. A parked SM is - // not idle; see flagged. - bool idle() const { - return pio_sm_is_tx_fifo_empty(pio_, tx_sm_) && // Read before the pc - pio_sm_get_pc(pio_, tx_sm_) == tx_offset_ + iebus_tx_wrap_target; - } - - void wait_done() { - while (!(flagged() || idle())) - tight_loop_contents(); - } - // Collect a flag if the SM has raised one, without waiting. Returns whether // the frame has failed, now or earlier. bool check() { const bool lost_arb = pio_interrupt_get(pio_, iebus_tx_lost_arb_irq); if (!lost_arb && !pio_interrupt_get(pio_, iebus_tx_nak_irq)) - return fault_ != Fault::None; + return fault_ != Send{0}; // Read the level before the clear throws it away, and clear before the // flag: releasing the SM with words still queued would send the rest of the // frame. - failed_word_ = - (uint8_t)(words_ - pio_sm_get_tx_fifo_level(pio_, tx_sm_) - 1U); + const auto pulled = + (uint8_t)(words_ - pio_sm_get_tx_fifo_level(pio_, tx_sm_)); + failed_word_ = (uint8_t)(pulled - 1U); pio_sm_clear_fifos(pio_, tx_sm_); if (lost_arb) { pio_interrupt_clear(pio_, iebus_tx_lost_arb_irq); - // The winner's frame is still arriving; put the ack match back so we can + // The other frame is still arriving; put the ack match back so we can // answer it if it turns out to be addressed to us. rx_.rearm_ack(); - fault_ = Fault::Mismatch; + // A bid lost before any word was pulled: the start-bit block saw the bus + // dominant with something other than a start bit. + if (pulled == 0) + fault_ = BUSY; + else if (failed_word_ < WORD_PERIPHERAL && failed_word_ < data_start_) + fault_ = LOST_ARBITRATION; + else + fault_ = CONTENDED_BUS; } else { pio_interrupt_clear(pio_, iebus_tx_nak_irq); - fault_ = Fault::Nak; + if (failed_word_ >= data_start_) + fault_ = NAK_DATA; + else if (failed_word_ == WORD_PERIPHERAL) + fault_ = NAK_ADDRESS; + else if (failed_word_ == WORD_CONTROL) + fault_ = NAK_CONTROL; + else if (failed_word_ == WORD_LENGTH) + fault_ = NAK_MESSAGE_LENGTH; + else + fault_ = NAK; } return true; } @@ -631,20 +644,6 @@ private: words_++; } - // The arbitration window goes out a field at a time: a lost bid has to be - // known before anything more is queued. - Send arbitrate(uint32_t bits, uint8_t len) { - if (muted_) - return MUTED; - put(encode_tx(len, (uint16_t)bits, false, false)); - wait_done(); - // Reported here and now, so nothing is left for send_done. With no ack slot - // in these fields, the only fault is a lost bid. - const bool lost = check(); - fault_ = Fault::None; - return lost ? LOST_ARBITRATION : Send{0}; - } - IEBusRx &rx_; PIO pio_; @@ -652,17 +651,18 @@ private: uint tx_sm_; uint tx_offset_; uint ack_sm_; - uint ack_offset_; bool claimed_ = false; // init() ran, so the destructor has something to undo bool muted_ = false; - // Words put since arbitration was won, taken by the SM or not. Its flags park - // it, which freezes the FIFO, so the word it failed on is the last one it - // pulled: words_ - level - 1. Wrapping is harmless; the FIFO holds at most 8. + // Words put since the header, taken by the SM or not. Its flags park it, + // which freezes the FIFO, so the word it failed on is the last one it pulled: + // words_ - level - 1. Wrapping is harmless; the FIFO holds at most 8. uint8_t words_; + uint8_t data_start_; // Position of the first data word uint8_t failed_word_; - Fault fault_; + // Sticky until send_done reports it; nothing more is queued meanwhile. + Send fault_; }; // The bus as a whole. Only it can hold the invariants that span the two @@ -718,7 +718,7 @@ private: // Reading needs nothing but the pad's input buffer, which is on for both bus // pins already -- asserted here so the mirror cannot go dark if that changes. void pin_mirror_sm_init(uint sm, uint offset, uint src, uint dst) { - PIO pio = tx_.pio(); + PIO pio = pio2; gpio_set_input_enabled(src, true); pio_gpio_init(pio, dst); pio_sm_set_consecutive_pindirs(pio, sm, dst, 1, true); @@ -738,7 +738,7 @@ private: // IEBUS TX/RX pins to the LED pins so that dominant bus activity (i.e. LOW // state for IEBUS TX/RX pins) lights the respective LED void activity_leds_init() { - PIO pio = tx_.pio(); // iebus_rx fills its PIO + PIO pio = pio2; // iebus_rx and iebus_tx each fill a block // Indicators are cosmetic; never fail the bus bring-up for them. if (!pio_can_add_program(pio, &pin_mirror_program)) return; @@ -765,7 +765,7 @@ private: void activity_leds_deinit() { if (!leds_claimed_) return; - PIO pio = tx_.pio(); + PIO pio = pio2; pio_sm_set_enabled(pio, led_sm_rx_, false); pio_sm_set_enabled(pio, led_sm_tx_, false); pio_sm_unclaim(pio, led_sm_rx_); @@ -836,16 +836,10 @@ extern "C" Read phy_read_length(uint8_t *length) { return Read{0}; } -extern "C" Read phy_read_data(uint8_t *data) { - static uint8_t idx; - const IEBusRx::RxFrame &frame = phy.rx().frame(); - if (idx >= frame.length) - idx = 0; - *data = frame.data[idx++]; - if (idx >= frame.length) { // Frame consumed - idx = 0; - phy.rx().release(); - } +extern "C" Read phy_read_data(uint8_t *data, uint8_t length, + [[maybe_unused]] uint8_t *data_index) { + memcpy(data, phy.rx().frame().data, length); + phy.rx().release(); return Read{0}; } @@ -854,23 +848,25 @@ extern "C" Send phy_send_header(bool is_unicast) { } extern "C" Send phy_send_controller_addr(uint16_t addr) { - return phy.tx().send_controller_addr(addr); + return phy.tx().send_field(12, addr, false, false); } extern "C" Send phy_send_peripheral_addr(uint16_t addr, bool expect_ack) { - return phy.tx().send_field(12, addr, expect_ack); + return phy.tx().send_field(12, addr, true, expect_ack); } extern "C" Send phy_send_control(uint8_t control, bool expect_ack) { - return phy.tx().send_field(4, control, expect_ack); + return phy.tx().send_field(4, control, true, expect_ack); } extern "C" Send phy_send_length(uint8_t length, bool expect_ack) { - return phy.tx().send_field(8, length, expect_ack); + return phy.tx().send_field(8, length, true, expect_ack); } -extern "C" Send phy_send_data(uint8_t data, bool expect_ack) { - return phy.tx().send_field(8, data, expect_ack); +extern "C" Send phy_send_data(const uint8_t *data, uint8_t length, + bool expect_ack, + [[maybe_unused]] uint8_t *data_index) { + return phy.tx().send_data(data, length, expect_ack); } extern "C" Send phy_send_done(uint8_t *data_index) {