VSC7448 SPI driver recurses

[jamesmunns]

hubris/drv/vsc7448/src/spi.rs

Lines 35 to 96 in 6e93059

	#[inline(never)]
	fn read_core(&self, orig_addr: u32) -> Result<u32, VscError> {
	if !(0x71000000..0x72000000).contains(&orig_addr) {
	return Err(VscError::BadRegAddr(orig_addr));
	}
	// Section 5.5.2 of the VSC7448 datasheet specifies how to convert
	// a register address to a request over SPI.
	let addr = (orig_addr & 0x00FFFFFF) >> 2;
	let data: &[u8] = &addr.to_be_bytes()[1..];
	// We read back 7 + padding bytes in total:
	// - 3 bytes of address
	// - Some number of padding bytes
	// - 4 bytes of data
	const SIZE: usize = 7 + SPI_NUM_PAD_BYTES;
	let mut out = [0; SIZE];
	self.0.exchange(data, &mut out[..])?;
	let value =
	u32::from_be_bytes(out[SIZE - 4..].try_into().unwrap_lite());
	ringbuf_entry!(Trace::Read {
	addr: orig_addr,
	value
	});
	// The VSC7448 is configured to return 0x88888888 if a register is
	// read too fast. Reading takes place over SPI: we write a 3-byte
	// address, then read 4 bytes of data; based on SPI speed, we may
	// need to configure padding bytes in between the address and
	// returning data.
	//
	// This is controlled by setting DEVCPU_ORG:IF_CFGSTAT.IF_CFG in
	// init(), then by padding bytes in the `out` arrays in
	// [read] and [write].
	//
	// Therefore, we should only read "too fast" if someone has modified
	// the SPI speed without updating the padding byte, which should
	// never happen in well-behaved code.
	//
	// If we see this sentinel value, then we check
	// DEVCPU_ORG:IF_CFGSTAT.IF_STAT. If that bit is set, then the sentinel
	// value _actually_ indicates an error (and not just an unfortunate
	// coincidence).
	if value == 0x88888888 {
	// Panic immediately if we got an invalid read sentinel while
	// reading IF_CFGSTAT itself, because that means something has
	// gone very deeply wrong. This check also protects us from a
	// stack overflow.
	let if_cfgstat = DEVCPU_ORG().DEVCPU_ORG().IF_CFGSTAT();
	if orig_addr == if_cfgstat.addr {
	panic!("Invalid nested read sentinel");
	}
	// This read should _never_ fail for timing reasons because the
	// DEVCPU_ORG register block can be accessed faster than all other
	// registers (section 5.3.2 of the datasheet).
	let v = self.read(if_cfgstat)?;
	if v.if_stat() == 1 {
	return Err(VscError::InvalidRegisterRead(orig_addr));
	}
	}
	Ok(value)
	}

The current SPI driver attempts to read a register, and if that returns a certain value, we call ourselves indirectly again to read a status register.

I noticed this while improving stack analysis, we probably should hoist this check up one level to call read_core twice from read, instead of read -> read_core -> read -> ....

In practice, this will only ever "iterate" twice, but we might want to avoid it entirely to make our lives easier.