Remove cortex-m dependencies for delay #2

Merged
sb10q merged 8 commits from occheung/ENC424J600:generalize_delay into master 2021-01-25 12:35:23 +08:00
2 changed files with 27 additions and 17 deletions
Showing only changes of commit cea9f2bf57 - Show all commits

View File

@ -19,7 +19,7 @@ pub mod smoltcp_phy;
pub const RAW_FRAME_LENGTH_MAX: usize = 1518;
pub trait EthController {
fn init_dev(&mut self, delay: &mut impl DelayUs<u16>) -> Result<(), EthControllerError>;
fn init_dev(&mut self) -> Result<(), EthControllerError>;
fn init_rxbuf(&mut self) -> Result<(), EthControllerError>;
fn init_txbuf(&mut self) -> Result<(), EthControllerError>;
fn receive_next(&mut self, is_poll: bool) -> Result<rx::RxPacket, EthControllerError>;
@ -45,17 +45,19 @@ impl From<spi::SpiPortError> for EthControllerError {
/// Ethernet controller using SPI interface
pub struct SpiEth<SPI: Transfer<u8>,
NSS: OutputPin> {
spi_port: spi::SpiPort<SPI, NSS>,
NSS: OutputPin,
Delay: DelayUs<u16>> {
spi_port: spi::SpiPort<SPI, NSS, Delay>,
rx_buf: rx::RxBuffer,
tx_buf: tx::TxBuffer
}
impl <SPI: Transfer<u8>,
NSS: OutputPin> SpiEth<SPI, NSS> {
pub fn new(spi: SPI, nss: NSS) -> Self {
NSS: OutputPin,
Delay: DelayUs<u16>> SpiEth<SPI, NSS, Delay> {
pub fn new(spi: SPI, nss: NSS, delay: Delay) -> Self {
SpiEth {
spi_port: spi::SpiPort::new(spi, nss),
spi_port: spi::SpiPort::new(spi, nss, delay),
rx_buf: rx::RxBuffer::new(),
tx_buf: tx::TxBuffer::new()
}
@ -63,8 +65,8 @@ impl <SPI: Transfer<u8>,
}
impl <SPI: Transfer<u8>,
NSS: OutputPin> EthController for SpiEth<SPI, NSS> {
fn init_dev(&mut self, delay: &mut impl DelayUs<u16>) -> Result<(), EthControllerError> {
NSS: OutputPin,
Delay: DelayUs<u16>> EthController for SpiEth<SPI, NSS, Delay> {
// Write 0x1234 to EUDAST
self.spi_port.write_reg_16b(spi::addrs::EUDAST, 0x1234)?;
// Verify that EUDAST is 0x1234

View File

@ -1,5 +1,5 @@
use embedded_hal::{
blocking::spi::Transfer,
blocking::{spi::Transfer, delay::DelayUs},
Outdated
Review

not needed

not needed
digital::v2::OutputPin,
};
@ -52,9 +52,11 @@ pub mod addrs {
/// Struct for SPI I/O interface on ENC424J600
/// Note: stm32f4xx_hal::spi's pins include: SCK, MISO, MOSI
pub struct SpiPort<SPI: Transfer<u8>,
NSS: OutputPin> {
NSS: OutputPin,
Delay: DelayUs<u16>> {
spi: SPI,
nss: NSS,
delay: Delay,
}
pub enum SpiPortError {
@ -63,14 +65,16 @@ pub enum SpiPortError {
#[allow(unused_must_use)]
impl <SPI: Transfer<u8>,
NSS: OutputPin> SpiPort<SPI, NSS> {
NSS: OutputPin,
Delay: DelayUs<u16>> SpiPort<SPI, NSS, Delay> {
// TODO: return as Result()
pub fn new(spi: SPI, mut nss: NSS) -> Self {
pub fn new(spi: SPI, mut nss: NSS, delay: Delay) -> Self {
Outdated
Review

why f and not delay_ns?

why ``f`` and not ``delay_ns``?
nss.set_high();
SpiPort {
spi,
nss
nss,
delay
}
}
@ -115,6 +119,10 @@ impl <SPI: Transfer<u8>,
Ok(())
}
pub fn delay_us(&mut self, duration: u16) {
self.delay.delay_us(duration)
}
// TODO: Generalise transfer functions
// TODO: (Make data read/write as reference to array)
// Currently requires 1-byte addr, read/write data is only 1-byte
@ -131,17 +139,17 @@ impl <SPI: Transfer<u8>,
match self.spi.transfer(&mut buf) {
Ok(_) => {
// Disable chip select
cortex_m::asm::delay(10_u32);
self.delay_us(1);
Outdated
Review

Wouldn't that significantly increase the delay, and noticeably slow things down?

Wouldn't that significantly increase the delay, and noticeably slow things down?

This delay gets called when modifying register.

Sending/Receiving packets do require read/write to register. But most of the SPI transcation should be to the SRAM buffer, given a large enough packet is sent.

This delay gets called when modifying register. Sending/Receiving packets do require read/write to register. But most of the SPI transcation should be to the SRAM buffer, given a large enough packet is sent.
Outdated
Review

When Dip comes back, I'd like to know the actual reason behind this delay between CS assertion/dessertion and the actual SPI transfer. Must the delay be in nanoseconds?

When Dip comes back, I'd like to know the actual reason behind this delay between CS assertion/dessertion and the actual SPI transfer. Must the delay be in nanoseconds?

The set of delay is to comply with the SPI specification of ENC424J600 mentioned in the datasheet (p.148). However, other functions that involve SPI transaction does not have such delay. The delay could be unnecessary.

(Note: However, if the delay is removed and the code is optimized, the CS could possibly be too short for 16-bits read/write functions, as the 8-bits R/W is called back-to-back. I can only faintly recall this potential issue, please verify if that is an issue at all.)

Anyway, these delay is only called after a complete SPI transaction. It can be extended to 1 microsecond. This delay is only called a fixed number of times per packet sent.

The set of delay is to comply with the SPI specification of ENC424J600 mentioned in the [datasheet](http://ww1.microchip.com/downloads/en/devicedoc/39935b.pdf) (p.148). However, other functions that involve SPI transaction does not have such delay. The delay could be unnecessary. (Note: However, if the delay is removed and the code is optimized, the CS *could possibly* be too short for 16-bits read/write functions, as the 8-bits R/W is called back-to-back. I can only faintly recall this potential issue, please verify if that is an issue at all.) Anyway, these delay is only called after a complete SPI transaction. It can be extended to 1 microsecond. This delay is only called a fixed number of times per packet sent.
Outdated
Review

Agh! Finally found someone else complaining about a similar situation as we do: https://github.com/rust-embedded/embedded-hal/issues/264

I'm going to read through it and see what steps we can take.

@occheung Thanks for typing the feedback 😃

Agh! Finally found someone else complaining about a similar situation as we do: https://github.com/rust-embedded/embedded-hal/issues/264 I'm going to read through it and see what steps we can take. @occheung Thanks for typing the feedback 😃
self.nss.set_high();
cortex_m::asm::delay(5_u32);
self.delay_us(1);
Ok(buf[2])
},
// TODO: Maybe too naive?
Err(_) => {
// Disable chip select
cortex_m::asm::delay(10_u32);
self.delay_us(1);
self.nss.set_high();
cortex_m::asm::delay(5_u32);
self.delay_us(1);
Err(SpiPortError::TransferError)
}
}