From 8b04d0e3efd8e0b13a42204a7d6dfbdb804a0347 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Tue, 26 Mar 2024 21:52:29 -0400 Subject: [PATCH] Enhance and better-document I2C states. --- Components/I2C/I2C.cpp | 50 +++++++++++++++++++++------ Components/I2C/I2C.hpp | 19 ++++++++-- Machines/Acorn/Archimedes/CMOSRAM.hpp | 3 +- 3 files changed, 58 insertions(+), 14 deletions(-) diff --git a/Components/I2C/I2C.cpp b/Components/I2C/I2C.cpp index a39f09f97..82b49400e 100644 --- a/Components/I2C/I2C.cpp +++ b/Components/I2C/I2C.cpp @@ -37,6 +37,7 @@ bool Bus::clock() { } void Bus::set_clock_data(bool clock_pulled, bool data_pulled) { + // Proceed only if changes are evidenced. if(clock_pulled == clock_ && data_pulled == data_) { return; } @@ -46,6 +47,8 @@ void Bus::set_clock_data(bool clock_pulled, bool data_pulled) { clock_ = clock_pulled; data_ = data_pulled; + // If currently serialising from a peripheral then shift onwards on + // every clock trailing edge. if(peripheral_bits_) { // Trailing edge of clock => bit has been consumed. if(!prior_clock && clock_) { @@ -60,8 +63,9 @@ void Bus::set_clock_data(bool clock_pulled, bool data_pulled) { return; } + // Not currently serialising implies listening. if(!clock_ && prior_data != data_) { - // Data transition outside of a clock cycle => start or stop. + // A data transition outside of a clock cycle implies a start or stop. in_bit_ = false; if(data_) { logger.info().append("S"); @@ -76,7 +80,8 @@ void Bus::set_clock_data(bool clock_pulled, bool data_pulled) { // Rising edge: clock period begins. in_bit_ = true; } else if(in_bit_) { - // Falling edge: clock period ends (assuming it began; otherwise this is prep, post-start bit). + // Falling edge: clock period ends (assuming it began; otherwise this is a preparatory + // clock transition only, immediately after a start bit). in_bit_ = false; if(data_) { @@ -118,6 +123,11 @@ void Bus::signal(Event event) { } }; + const auto stop = [&]() { + set_state(State::AwaitingAddress); + active_peripheral_ = nullptr; + }; + // Allow start and stop conditions at any time. if(event == Event::Start) { set_state(State::CollectingAddress); @@ -126,17 +136,21 @@ void Bus::signal(Event event) { } if(event == Event::Stop) { - set_state(State::AwaitingAddress); if(active_peripheral_) { active_peripheral_->stop(); } - active_peripheral_ = nullptr; + stop(); return; } switch(state_) { + // While waiting for an address, don't respond to anything other than a + // start bit, which is actually dealt with above. case State::AwaitingAddress: break; + // To collect an address: shift in eight bits, and if there's a device + // at that address then acknowledge the address and segue into a read + // or write loop. case State::CollectingAddress: capture_bit(); if(input_count_ == 8) { @@ -147,7 +161,7 @@ void Bus::signal(Event event) { if(input_&1) { acknowledge(); - set_state(State::CompletingWriteAcknowledge); + set_state(State::CompletingReadAcknowledge); } else { acknowledge(); set_state(State::ReceivingByte); @@ -158,32 +172,46 @@ void Bus::signal(Event event) { } break; + // Receiving byte: wait until a scheduled acknowledgment has + // happened, then collect eight bits, then see whether the + // active peripheral will accept them. If so, acknowledge and repeat. + // Otherwise fall silent. case State::ReceivingByte: if(event == Event::FinishedOutput) { return; } capture_bit(); if(input_count_ == 8) { - active_peripheral_->write(static_cast(input_)); - acknowledge(); - set_state(State::ReceivingByte); + if(active_peripheral_->write(static_cast(input_))) { + acknowledge(); + set_state(State::ReceivingByte); + } else { + stop(); + } } break; - case State::CompletingWriteAcknowledge: + // The initial state immediately after a peripheral has been started + // in read mode and the address-select acknowledgement is still + // being serialised. + // + // Once that is completed, enqueues the first byte from the peripheral. + case State::CompletingReadAcknowledge: if(event != Event::FinishedOutput) { break; } enqueue(active_peripheral_->read()); break; + // Repeating state during reading; waits until the previous byte has + // been fully serialised, and if the host acknowledged it then posts + // the next. If the host didn't acknowledge, stops the connection. case State::AwaitingByteAcknowledge: if(event == Event::FinishedOutput) { break; } if(event != Event::Zero) { - logger.info().append("Peripheral byte not acknowledged"); - state_ = State::AwaitingAddress; + stop(); break; } diff --git a/Components/I2C/I2C.hpp b/Components/I2C/I2C.hpp index 870b84416..e8dc0c783 100644 --- a/Components/I2C/I2C.hpp +++ b/Components/I2C/I2C.hpp @@ -17,10 +17,25 @@ namespace I2C { /// Provides the virtual interface for an I2C peripheral; attaching this to a bus /// provides automatic protocol handling. struct Peripheral { + /// Indicates that the host signalled the start condition and addressed this + /// peripheral, along with whether it indicated a read or write. virtual void start([[maybe_unused]] bool is_read) {} + + /// Indicates that the host signalled a stop. virtual void stop() {} + + /// Requests the next byte to serialise onto the I2C bus after this peripheral has + /// been started in read mode. + /// + /// @returns A byte to serialise or std::nullopt if the peripheral declines to + /// continue to communicate. virtual std::optional read() { return std::nullopt; } - virtual void write(uint8_t) {} + + /// Provides a byte received from the bus after this peripheral has been started + /// in write mode. + /// + /// @returns @c true if the write should be acknowledged; @c false otherwise. + virtual bool write(uint8_t) { return false; } }; class Bus { @@ -57,7 +72,7 @@ private: AwaitingAddress, CollectingAddress, - CompletingWriteAcknowledge, + CompletingReadAcknowledge, AwaitingByteAcknowledge, ReceivingByte, diff --git a/Machines/Acorn/Archimedes/CMOSRAM.hpp b/Machines/Acorn/Archimedes/CMOSRAM.hpp index 9a3c5e5e0..61117c4b2 100644 --- a/Machines/Acorn/Archimedes/CMOSRAM.hpp +++ b/Machines/Acorn/Archimedes/CMOSRAM.hpp @@ -46,7 +46,7 @@ struct CMOSRAM: public I2C::Peripheral { return result; } - void write(uint8_t value) override { + bool write(uint8_t value) override { if(expecting_address_) { address_ = value; expecting_address_ = false; @@ -54,6 +54,7 @@ struct CMOSRAM: public I2C::Peripheral { ++address_; // TODO: write. } + return true; } private: