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
5 changed files with 41 additions and 68 deletions
Showing only changes of commit 366ff1c80e - Show all commits

View File

@ -1,34 +0,0 @@
use embedded_hal::blocking::delay::{DelayMs, DelayUs};
#[derive(Clone, Copy)]
pub struct AsmDelay {
frequency_us: u32,
frequency_ms: u32,
}
impl AsmDelay {
pub fn new(freq: u32) -> AsmDelay {
AsmDelay {
frequency_us: (freq / 1_000_000),
frequency_ms: (freq / 1_000),
}
}
}
impl<U> DelayUs<U> for AsmDelay
where
U: Into<u32>,
{
fn delay_us(&mut self, us: U) {
cortex_m::asm::delay(self.frequency_us * us.into())
}
}
impl<U> DelayMs<U> for AsmDelay
where
U: Into<u32>,
{
fn delay_ms(&mut self, ms: U) {
cortex_m::asm::delay(self.frequency_ms * ms.into())
}
}

View File

@ -83,7 +83,7 @@ use stm32f4xx_hal::{
type BoosterSpiEth = enc424j600::SpiEth<
Spi<SPI1, (PA5<Alternate<AF5>>, PA6<Alternate<AF5>>, PA7<Alternate<AF5>>)>,
PA4<Output<PushPull>>,
AsmDelay
fn(u32) -> ()
>;
pub struct NetStorage {
@ -99,6 +99,10 @@ static mut NET_STORE: NetStorage = NetStorage {
neighbor_cache: [None; 8],
};
pub fn delay_ns(time_ns: u32) {
cortex_m::asm::delay((time_ns*168_000_000)/1_000_000_000 + 1)
}
Outdated
Review

Where is this used?

Where is this used?

Removed in 5bdfd21e93.

Removed in 5bdfd21e93.
#[rtic::app(device = stm32f4xx_hal::stm32, peripherals = true, monotonic = rtic::cyccnt::CYCCNT)]
const APP: () = {
struct Resources {
@ -157,7 +161,11 @@ const APP: () = {
enc424j600::spi::interfaces::SPI_MODE,
Hertz(enc424j600::spi::interfaces::SPI_CLOCK_FREQ),
clocks);
enc424j600::SpiEth::new(spi_eth_port, spi1_nss, asm_delay)
let delay_ns_fp: fn(u32) -> () = |time_ns| {
Outdated
Review

fp?

fp?

I meant function pointer.
Changed to delay_ns.

I meant function pointer. Changed to `delay_ns`.
cortex_m::asm::delay((time_ns*21)/125 + 1)
};
enc424j600::SpiEth::new(spi_eth_port, spi1_nss, delay_ns_fp)
};
// Init controller

View File

@ -13,15 +13,13 @@ use stm32f4xx_hal::{
gpio::GpioExt,
time::U32Ext,
stm32::ITM,
delay::Delay,
spi::Spi,
time::Hertz
};
use enc424j600;
use enc424j600::EthController;
mod delay;
use delay::AsmDelay;
///
use stm32f4xx_hal::{
stm32::SPI1,
@ -33,13 +31,13 @@ use stm32f4xx_hal::{
type BoosterSpiEth = enc424j600::SpiEth<
Spi<SPI1, (PA5<Alternate<AF5>>, PA6<Alternate<AF5>>, PA7<Alternate<AF5>>)>,
PA4<Output<PushPull>>,
AsmDelay>;
fn(u32)>;
#[rtic::app(device = stm32f4xx_hal::stm32, peripherals = true, monotonic = rtic::cyccnt::CYCCNT)]
const APP: () = {
struct Resources {
spi_eth: BoosterSpiEth,
delay: AsmDelay,
delay: Delay,
itm: ITM,
}
@ -57,7 +55,7 @@ const APP: () = {
//.pclk2(64.mhz())
.require_pll48clk()
.freeze();
let mut asm_delay = AsmDelay::new(clocks.sysclk().0);
let mut delay = Delay::new(c.core.SYST, clocks);
// Init ITM
let mut itm = c.core.ITM;
@ -76,7 +74,7 @@ const APP: () = {
// Map SPISEL: see Table 1, NIC100 Manual
let mut spisel = gpioa.pa1.into_push_pull_output();
spisel.set_high().unwrap();
asm_delay.delay_ms(1_u32);
delay.delay_ms(1_u32);
spisel.set_low().unwrap();
// Create SPI1 for HAL
let mut spi_eth = {
@ -85,7 +83,11 @@ const APP: () = {
enc424j600::spi::interfaces::SPI_MODE,
Hertz(enc424j600::spi::interfaces::SPI_CLOCK_FREQ),
clocks);
enc424j600::SpiEth::new(spi_eth_port, spi1_nss, asm_delay)
let delay_ns_fp: fn(u32) -> () = |time_ns| {
cortex_m::asm::delay((time_ns*21)/125 + 1)
};
enc424j600::SpiEth::new(spi_eth_port, spi1_nss, delay_ns_fp)
};
// Init
@ -118,7 +120,7 @@ const APP: () = {
init::LateResources {
spi_eth,
delay: asm_delay,
delay,
itm,
}
}

View File

@ -4,7 +4,6 @@ pub mod spi;
use embedded_hal::{
blocking::{
spi::Transfer,
delay::DelayUs,
},
digital::v2::OutputPin,
};
@ -46,18 +45,18 @@ impl From<spi::SpiPortError> for EthControllerError {
/// Ethernet controller using SPI interface
pub struct SpiEth<SPI: Transfer<u8>,
NSS: OutputPin,
Delay: DelayUs<u16>> {
spi_port: spi::SpiPort<SPI, NSS, Delay>,
F: FnMut(u32) -> ()> {
spi_port: spi::SpiPort<SPI, NSS, F>,
rx_buf: rx::RxBuffer,
tx_buf: tx::TxBuffer
}
impl <SPI: Transfer<u8>,
NSS: OutputPin,
Delay: DelayUs<u16>> SpiEth<SPI, NSS, Delay> {
pub fn new(spi: SPI, nss: NSS, delay: Delay) -> Self {
F: FnMut(u32) -> ()> SpiEth<SPI, NSS, F> {
pub fn new(spi: SPI, nss: NSS, f: F) -> Self {
SpiEth {
spi_port: spi::SpiPort::new(spi, nss, delay),
spi_port: spi::SpiPort::new(spi, nss, f),
rx_buf: rx::RxBuffer::new(),
tx_buf: tx::TxBuffer::new()
}
@ -66,7 +65,7 @@ impl <SPI: Transfer<u8>,
impl <SPI: Transfer<u8>,
NSS: OutputPin,
Delay: DelayUs<u16>> EthController for SpiEth<SPI, NSS, Delay> {
F: FnMut(u32) -> ()> EthController for SpiEth<SPI, NSS, F> {
fn init_dev(&mut self) -> Result<(), EthControllerError> {
// Write 0x1234 to EUDAST
self.spi_port.write_reg_16b(spi::addrs::EUDAST, 0x1234)?;
@ -83,15 +82,13 @@ impl <SPI: Transfer<u8>,
// Set ETHRST (ECON2<4>) to 1
let econ2 = self.spi_port.read_reg_8b(spi::addrs::ECON2)?;
self.spi_port.write_reg_8b(spi::addrs::ECON2, 0x10 | (econ2 & 0b11101111))?;
// Wait for 25us
self.spi_port.delay_us(25_u16);
self.spi_port.delay_us(25);
// Verify that EUDAST is 0x0000
Outdated
Review

This comment is just an obvious English translation of the code. Remove.

This comment is just an obvious English translation of the code. Remove.
eudast = self.spi_port.read_reg_16b(spi::addrs::EUDAST)?;
if eudast != 0x0000 {
return Err(EthControllerError::GeneralError)
}
// Wait for 256us
self.spi_port.delay_us(256_u16);
self.spi_port.delay_us(256);
Ok(())
}

View File

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

not needed

not needed
digital::v2::OutputPin,
};
@ -53,10 +53,10 @@ pub mod addrs {
/// Note: stm32f4xx_hal::spi's pins include: SCK, MISO, MOSI
pub struct SpiPort<SPI: Transfer<u8>,
NSS: OutputPin,
Delay: DelayUs<u16>> {
F: FnMut(u32) -> ()> {
spi: SPI,
nss: NSS,
delay: Delay,
delay_ns: F,
}
pub enum SpiPortError {
@ -66,15 +66,15 @@ pub enum SpiPortError {
#[allow(unused_must_use)]
impl <SPI: Transfer<u8>,
NSS: OutputPin,
Delay: DelayUs<u16>> SpiPort<SPI, NSS, Delay> {
F: FnMut(u32) -> ()> SpiPort<SPI, NSS, F> {
// TODO: return as Result()
pub fn new(spi: SPI, mut nss: NSS, delay: Delay) -> Self {
pub fn new(spi: SPI, mut nss: NSS, f: F) -> Self {
Outdated
Review

why f and not delay_ns?

why ``f`` and not ``delay_ns``?
nss.set_high();
SpiPort {
spi,
nss,
delay
delay_ns: f,
}
}
@ -119,8 +119,8 @@ impl <SPI: Transfer<u8>,
Ok(())
}
pub fn delay_us(&mut self, duration: u16) {
self.delay.delay_us(duration)
pub fn delay_us(&mut self, duration: u32) {
(self.delay_ns)(duration * 1000)
}
// TODO: Generalise transfer functions
@ -139,17 +139,17 @@ impl <SPI: Transfer<u8>,
match self.spi.transfer(&mut buf) {
Ok(_) => {
// Disable chip select
self.delay_us(1);
(self.delay_ns)(60);
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();
self.delay_us(1);
(self.delay_ns)(30);
Ok(buf[2])
},
// TODO: Maybe too naive?
Err(_) => {
// Disable chip select
self.delay_us(1);
(self.delay_ns)(60);
self.nss.set_high();
self.delay_us(1);
(self.delay_ns)(30);
Err(SpiPortError::TransferError)
}
}