Unsound (?) Attiny84 I2C implementation through USI

avr-hal.diff

diff --git a/avr-hal-generic/src/port.rs b/avr-hal-generic/src/port.rs
index 1ecc275..6b6e070 100644
--- a/avr-hal-generic/src/port.rs
+++ b/avr-hal-generic/src/port.rs
@@ -121,6 +121,15 @@ impl<PIN: PinOps> Pin<mode::Input<mode::Floating>, PIN> {
     }
 }

+impl<PIN: PinOps, MODE: mode::Io> Pin<MODE, PIN> {
+    /// Returns the underlying pin for direct manipulation
+    /// This loses any type safety provided by this struct, but allows more precise operations
+    /// through its `unsafe` methods.
+    pub fn into_raw_pin(self) -> PIN {
+        self.pin
+    }
+}
+
 /// # Configuration
 /// To change the mode of a pin, use one of the following conversion functions.  They consume the
 /// original [`Pin`] and return one with the desired mode.  Only when a pin is in the correct mode,

i2c.rs

use core::marker::PhantomData;

use attiny_hal::{
    clock::MHz1,
    pac::USI,
    port::{
        mode::{Floating, Input},
        Pin, PinOps,
    },
};

// Attiny84
type SDA = attiny_hal::port::PA6;
type SCL = attiny_hal::port::PA4;

#[derive(Debug)]
pub enum Error {
    /// Empty message buffer
    EmptyMessage,
    /// The address has its MSB set. Addresses should only be 7 bits.
    InvalidAddress,
    /// Received NACK on addressing
    NackAddress,
    /// Received NACK on data byte
    NackData,
}

pub type Result<T> = ::core::result::Result<T, Error>;

#[inline(always)]
fn nop() {
    #[cfg(all(target_arch = "avr", avr_hal_asm_macro))]
    use core::arch::asm;

    #[cfg(all(target_arch = "avr", avr_hal_asm_macro))]
    unsafe {
        asm!("nop");
    }

    #[cfg(all(target_arch = "avr", not(avr_hal_asm_macro)))]
    unsafe {
        llvm_asm!("nop" :::: "volatile");
    }
}

pub struct Delay4us;
pub struct Delay47us;

pub trait Delay<CLOCK> {
    fn delay();
}

impl Delay<MHz1> for Delay4us {
    #[inline(always)]
    fn delay() {
        nop();
        nop();
        nop();
        nop();
    }
}

impl Delay<MHz1> for Delay47us {
    #[inline(always)]
    fn delay() {
        nop();
        nop();
        nop();
        nop();
        nop();
    }
}

pub struct StandardMode;

pub trait Timings<CLOCK> {
    type SetupRepeatedStart: Delay<CLOCK>;
    type HoldRepeatedStart: Delay<CLOCK>;
    type SclLowPeriod: Delay<CLOCK>;
    type SclHighPeriod: Delay<CLOCK>;
    type SetupStop: Delay<CLOCK>;
    type SdaHighStopStartBuf: Delay<CLOCK>;
}

impl<CLOCK> Timings<CLOCK> for StandardMode
where
    Delay4us: Delay<CLOCK>,
    Delay47us: Delay<CLOCK>,
{
    type SetupRepeatedStart = Delay47us;
    type HoldRepeatedStart = Delay4us;
    type SclLowPeriod = Delay47us;
    type SclHighPeriod = Delay4us;
    type SetupStop = Delay4us;
    type SdaHighStopStartBuf = Delay47us;
}

// TODO Implement delays for other speeds and clocks

pub struct UsiI2c<CLOCK, MODE = StandardMode> {
    usi: USI,
    // We manage the SDA/SCL states directly
    // Outside of the control flow of this type's methods, these should be set to output high.
    sda: SDA,
    scl: SCL,
    _clock: PhantomData<CLOCK>,
    _speed: PhantomData<MODE>,
}

impl<CLOCK, MODE> UsiI2c<CLOCK, MODE>
where
    MODE: Timings<CLOCK>,
{
    pub fn with_external_pullup(
        usi: USI,
        sda: Pin<Input<Floating>, SDA>,
        scl: Pin<Input<Floating>, SCL>,
    ) -> Self {
        // Set pins as output pulling high as the "released" state
        let mut sda = sda.into_raw_pin();
        let mut scl = scl.into_raw_pin();

        unsafe {
            sda.out_set();
            scl.out_set();

            sda.make_output();
            scl.make_output();
        }

        // Preload data register with "released level" data
        usi.usidr.write(|w| w.bits(0xFF));

        // Set up USI control register
        usi.usicr.write(|w| {
            w
                // Disable interrupts
                .usisie()
                .clear_bit()
                .usioie()
                .clear_bit()
                // Set USI to two-wire mode without SCL hold on counter overflow
                // NOTE: this is mislabelled in the rust code as "slave".
                .usiwm()
                .two_wire_slave()
                // Set the clock source to be external on a positive edge
                .usics()
                .ext_pos()
                // Set the counter clock source to software strobe
                .usiclk()
                .set_bit()
                .usitc()
                .clear_bit()
        });

        // Set up USI status register
        usi.usisr.write(|w| {
            w
                // Clear flags
                .usisif()
                .set_bit()
                .usioif()
                .set_bit()
                .usipf()
                .set_bit()
                // Clear counter
                .usicnt()
                .bits(0x00)
        });

        Self {
            usi,
            sda,
            scl,
            _clock: PhantomData,
            _speed: PhantomData,
        }
    }

    pub fn send_message(&mut self, address: u8, message: &[u8]) -> Result<()> {
        if message.is_empty() {
            return Err(Error::EmptyMessage);
        }

        if (address & 0x80) != 0 {
            return Err(Error::InvalidAddress);
        }

        self.start();

        let address_rw = address << 1;
        self.send_byte(address_rw);

        if !self.receive_ack() {
            self.stop();
            return Err(Error::NackAddress);
        }

        for byte in message {
            self.send_byte(*byte);

            if !self.receive_ack() {
                self.stop();
                return Err(Error::NackData);
            }
        }

        self.stop();

        Ok(())
    }

    pub fn receive_message(&mut self, address: u8, buf: &mut [u8]) -> Result<()> {
        if (address & 0x80) != 0 {
            return Err(Error::InvalidAddress);
        }

        self.start();

        let address_rw = (address << 1) | 0x1;

        self.send_byte(address_rw);
        if !self.receive_ack() {
            self.stop();
            return Err(Error::NackAddress);
        }

        let (first, rest) = buf.split_first_mut().ok_or(Error::EmptyMessage)?;
        *first = self.receive_byte();
        for byte in rest {
            self.send_ack();
            *byte = self.receive_byte();
        }

        self.send_nack();

        self.stop();

        Ok(())
    }

    fn start(&mut self) {
        // Release scl to let it float high for repeated start
        unsafe {
            self.scl.out_set();
        }
        wait_until_high(&mut self.scl);

        // Wait to allow repeated starts
        MODE::SetupRepeatedStart::delay();

        // Pull SDA low
        unsafe {
            self.sda.out_clear();
        }

        MODE::HoldRepeatedStart::delay();

        // Pull SCL low
        unsafe {
            self.scl.out_clear();
        }

        // Release SDA
        unsafe {
            self.sda.out_set();
        }
    }

    fn stop(&mut self) {
        // Release SCL
        unsafe {
            self.scl.out_set();
        }
        wait_until_high(&mut self.scl);

        MODE::SetupStop::delay();

        // Release SDA
        unsafe {
            self.scl.out_set();
        }
        wait_until_high(&mut self.sda);

        MODE::SdaHighStopStartBuf::delay();
    }

    fn send_byte(&mut self, byte: u8) {
        // Load byte into data register
        self.usi.usidr.write(|w| w.bits(byte));

        self.usi.usisr.write(|w| {
            w
                // Clear flags
                .usisif()
                .set_bit()
                .usioif()
                .set_bit()
                .usipf()
                .set_bit()
                // Set USI to shift 8 bits (count 16 edges)
                .usicnt()
                .bits(0x0)
        });

        self.shift_bits();
    }

    fn receive_byte(&mut self) -> u8 {
        // Make SDA an input
        unsafe {
            self.sda.make_input(true);
        }

        self.usi.usisr.write(|w| {
            w
                // Clear flags
                .usisif()
                .set_bit()
                .usioif()
                .set_bit()
                .usipf()
                .set_bit()
                // Set USI to shift 8 bits (count 16 edges)
                .usicnt()
                .bits(0x0)
        });

        let byte = self.shift_bits();

        // Make SDA an output
        unsafe {
            self.sda.make_output();
        }

        byte
    }

    fn receive_ack(&mut self) -> bool {
        // Set SDA to input
        unsafe {
            self.sda.make_input(true);
        }

        self.usi.usisr.write(|w| {
            w
                // Clear flags
                .usisif()
                .set_bit()
                .usioif()
                .set_bit()
                .usipf()
                .set_bit()
                // Set USI to shift 1 bit (count 2 edges)
                .usicnt()
                .bits(0xE)
        });

        let bit = self.shift_bits() & 0x1;

        // Set SDA to output
        unsafe {
            self.sda.make_output();
        }

        bit == 0
    }

    #[inline(always)]
    fn send_nack(&mut self) {
        self.send_bit(true);
    }

    #[inline(always)]
    fn send_ack(&mut self) {
        self.send_bit(false);
    }

    fn send_bit(&mut self, bit: bool) {
        let byte = if bit { 0xFF } else { 0x00 };

        self.usi.usidr.write(|w| w.bits(byte));

        self.usi.usisr.write(|w| {
            w
                // Clear flags
                .usisif()
                .set_bit()
                .usioif()
                .set_bit()
                .usipf()
                .set_bit()
                // Set USI to shift 1 bit (count 2 edges)
                .usicnt()
                .bits(0xE)
        });

        self.shift_bits();
    }

    fn shift_bits(&mut self) -> u8 {
        for _ in 0..16 {
            MODE::SclLowPeriod::delay();

            // Generate positive SCL edge
            self.usi.usicr.write(|w| {
                w
                    // Disable interrupts
                    .usisie()
                    .clear_bit()
                    .usioie()
                    .clear_bit()
                    // Set USI to two-wire mode without SCL hold on counter overflow
                    // NOTE: this is mislabelled in the rust code as "slave".
                    .usiwm()
                    .two_wire_slave()
                    // Set the clock source to be external on a positive edge
                    .usics()
                    .ext_pos()
                    // Set the counter clock source to software strobe
                    .usiclk()
                    .set_bit()
                    // Strobe the clock line
                    .usitc()
                    .set_bit()
            });
            wait_until_high(&mut self.scl);

            MODE::SclHighPeriod::delay();

            // Generate negative SCL edge

            self.usi.usicr.write(|w| {
                w
                    // Disable interrupts
                    .usisie()
                    .clear_bit()
                    .usioie()
                    .clear_bit()
                    // Set USI to two-wire mode without SCL hold on counter overflow
                    // NOTE: this is mislabelled in the rust code as "slave".
                    .usiwm()
                    .two_wire_slave()
                    // Set the clock source to be external on a positive edge
                    .usics()
                    .ext_pos()
                    // Set the counter clock source to software strobe
                    .usiclk()
                    .set_bit()
                    // Strobe the clock line
                    .usitc()
                    .set_bit()
            });

            if self.usi.usisr.read().usioif().bit_is_set() {
                break;
            }
        }

        MODE::SclLowPeriod::delay();

        // Read out bits
        let ret = self.usi.usidr.read().bits();
        // Clear data register, release SDA
        self.usi.usidr.write(|w| w.bits(0xFF));

        ret
    }
}

#[inline(always)]
fn wait_until_high<P: PinOps>(pin: &mut P) {
    loop {
        if unsafe { pin.in_get() } {
            break;
        }
    }
}