renet
/
ENC424J600
10
1
Fork 1
Code Issues 3 Pull Requests 1 Projects Releases Wiki Activity

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
Conversation 11 Commits 8 Files Changed 4 +46 -27
4 changed files with 46 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
examples/tcp_stm32f407.rs
View File

@ -80,7 +80,9 @@ use stm32f4xx_hal::{
}; };
type BoosterSpiEth = enc424j600::SpiEth< type BoosterSpiEth = enc424j600::SpiEth<
Spi<SPI1, (PA5<Alternate<AF5>>, PA6<Alternate<AF5>>, PA7<Alternate<AF5>>)>, Spi<SPI1, (PA5<Alternate<AF5>>, PA6<Alternate<AF5>>, PA7<Alternate<AF5>>)>,
PA4<Output<PushPull>>>; PA4<Output<PushPull>>,
fn(u32) -> ()
>;
pub struct NetStorage { pub struct NetStorage {
ip_addrs: [IpCidr; 1], ip_addrs: [IpCidr; 1],
@ -153,11 +155,15 @@ const APP: () = {
enc424j600::spi::interfaces::SPI_MODE, enc424j600::spi::interfaces::SPI_MODE,
Hertz(enc424j600::spi::interfaces::SPI_CLOCK_FREQ), Hertz(enc424j600::spi::interfaces::SPI_CLOCK_FREQ),
clocks); clocks);
enc424j600::SpiEth::new(spi_eth_port, spi1_nss)
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 controller // Init controller
sb10q commented 2021-01-25 11:01:23 +08:00
Outdated
Review
Copy Link

fp?

fp?
occheung commented 2021-01-25 11:50:38 +08:00
Outdated
Review
Copy Link

I meant function pointer.
Changed to delay_ns.

I meant function pointer. Changed to `delay_ns`.
match spi_eth.init_dev(&mut delay) { match spi_eth.init_dev() {
Ok(_) => { Ok(_) => {
iprintln!(stim0, "Initializing Ethernet...") iprintln!(stim0, "Initializing Ethernet...")
} }

11
examples/tx_stm32f407.rs
View File

@ -30,7 +30,8 @@ use stm32f4xx_hal::{
}; };
type BoosterSpiEth = enc424j600::SpiEth< type BoosterSpiEth = enc424j600::SpiEth<
Spi<SPI1, (PA5<Alternate<AF5>>, PA6<Alternate<AF5>>, PA7<Alternate<AF5>>)>, Spi<SPI1, (PA5<Alternate<AF5>>, PA6<Alternate<AF5>>, PA7<Alternate<AF5>>)>,
PA4<Output<PushPull>>>; PA4<Output<PushPull>>,
fn(u32)>;
#[rtic::app(device = stm32f4xx_hal::stm32, peripherals = true, monotonic = rtic::cyccnt::CYCCNT)] #[rtic::app(device = stm32f4xx_hal::stm32, peripherals = true, monotonic = rtic::cyccnt::CYCCNT)]
const APP: () = { const APP: () = {
@ -82,11 +83,15 @@ const APP: () = {
enc424j600::spi::interfaces::SPI_MODE, enc424j600::spi::interfaces::SPI_MODE,
Hertz(enc424j600::spi::interfaces::SPI_CLOCK_FREQ), Hertz(enc424j600::spi::interfaces::SPI_CLOCK_FREQ),
clocks); clocks);
enc424j600::SpiEth::new(spi_eth_port, spi1_nss)
let delay_ns: fn(u32) -> () = |time_ns| {
cortex_m::asm::delay((time_ns*21)/125 + 1)
};
enc424j600::SpiEth::new(spi_eth_port, spi1_nss, delay_ns)
}; };
// Init // Init
match spi_eth.init_dev(&mut delay) { match spi_eth.init_dev() {
Ok(_) => { Ok(_) => {
iprintln!(stim0, "Initializing Ethernet...") iprintln!(stim0, "Initializing Ethernet...")
} }

26
src/lib.rs
View File

@ -4,7 +4,6 @@ pub mod spi;
use embedded_hal::{ use embedded_hal::{
blocking::{ blocking::{
spi::Transfer, spi::Transfer,
delay::DelayUs,
}, },
digital::v2::OutputPin, digital::v2::OutputPin,
}; };
@ -19,7 +18,7 @@ pub mod smoltcp_phy;
pub const RAW_FRAME_LENGTH_MAX: usize = 1518; pub const RAW_FRAME_LENGTH_MAX: usize = 1518;
pub trait EthController { 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_rxbuf(&mut self) -> Result<(), EthControllerError>;
fn init_txbuf(&mut self) -> Result<(), EthControllerError>; fn init_txbuf(&mut self) -> Result<(), EthControllerError>;
fn receive_next(&mut self, is_poll: bool) -> Result<rx::RxPacket, EthControllerError>; fn receive_next(&mut self, is_poll: bool) -> Result<rx::RxPacket, EthControllerError>;
@ -45,17 +44,19 @@ impl From<spi::SpiPortError> for EthControllerError {
/// Ethernet controller using SPI interface /// Ethernet controller using SPI interface
pub struct SpiEth<SPI: Transfer<u8>, pub struct SpiEth<SPI: Transfer<u8>,
NSS: OutputPin> { NSS: OutputPin,
spi_port: spi::SpiPort<SPI, NSS>, F: FnMut(u32) -> ()> {
spi_port: spi::SpiPort<SPI, NSS, F>,
rx_buf: rx::RxBuffer, rx_buf: rx::RxBuffer,
tx_buf: tx::TxBuffer tx_buf: tx::TxBuffer
} }
impl <SPI: Transfer<u8>, impl <SPI: Transfer<u8>,
NSS: OutputPin> SpiEth<SPI, NSS> { NSS: OutputPin,
pub fn new(spi: SPI, nss: NSS) -> Self { F: FnMut(u32) -> ()> SpiEth<SPI, NSS, F> {
pub fn new(spi: SPI, nss: NSS, delay_ns: F) -> Self {
SpiEth { SpiEth {
spi_port: spi::SpiPort::new(spi, nss), spi_port: spi::SpiPort::new(spi, nss, delay_ns),
rx_buf: rx::RxBuffer::new(), rx_buf: rx::RxBuffer::new(),
tx_buf: tx::TxBuffer::new() tx_buf: tx::TxBuffer::new()
} }
@ -63,8 +64,9 @@ impl <SPI: Transfer<u8>,
} }
impl <SPI: Transfer<u8>, impl <SPI: Transfer<u8>,
NSS: OutputPin> EthController for SpiEth<SPI, NSS> { NSS: OutputPin,
fn init_dev(&mut self, delay: &mut impl DelayUs<u16>) -> Result<(), EthControllerError> { F: FnMut(u32) -> ()> EthController for SpiEth<SPI, NSS, F> {
fn init_dev(&mut self) -> Result<(), EthControllerError> {
// Write 0x1234 to EUDAST // Write 0x1234 to EUDAST
self.spi_port.write_reg_16b(spi::addrs::EUDAST, 0x1234)?; self.spi_port.write_reg_16b(spi::addrs::EUDAST, 0x1234)?;
// Verify that EUDAST is 0x1234 // Verify that EUDAST is 0x1234
@ -80,15 +82,13 @@ impl <SPI: Transfer<u8>,
// Set ETHRST (ECON2<4>) to 1 // Set ETHRST (ECON2<4>) to 1
let econ2 = self.spi_port.read_reg_8b(spi::addrs::ECON2)?; let econ2 = self.spi_port.read_reg_8b(spi::addrs::ECON2)?;
self.spi_port.write_reg_8b(spi::addrs::ECON2, 0x10 | (econ2 & 0b11101111))?; self.spi_port.write_reg_8b(spi::addrs::ECON2, 0x10 | (econ2 & 0b11101111))?;
// Wait for 25us self.spi_port.delay_us(25);
delay.delay_us(25_u16);
// Verify that EUDAST is 0x0000 // Verify that EUDAST is 0x0000
sb10q commented 2021-01-22 15:09:14 +08:00
Outdated
Review
Copy Link

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

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

24
src/spi.rs
View File

@ -52,9 +52,11 @@ pub mod addrs {
/// Struct for SPI I/O interface on ENC424J600 /// Struct for SPI I/O interface on ENC424J600
/// Note: stm32f4xx_hal::spi's pins include: SCK, MISO, MOSI /// Note: stm32f4xx_hal::spi's pins include: SCK, MISO, MOSI
pub struct SpiPort<SPI: Transfer<u8>, pub struct SpiPort<SPI: Transfer<u8>,
NSS: OutputPin> { NSS: OutputPin,
F: FnMut(u32) -> ()> {
spi: SPI, spi: SPI,
nss: NSS, nss: NSS,
delay_ns: F,
} }
pub enum SpiPortError { pub enum SpiPortError {
@ -63,14 +65,16 @@ pub enum SpiPortError {
#[allow(unused_must_use)] #[allow(unused_must_use)]
impl <SPI: Transfer<u8>, impl <SPI: Transfer<u8>,
NSS: OutputPin> SpiPort<SPI, NSS> { NSS: OutputPin,
F: FnMut(u32) -> ()> SpiPort<SPI, NSS, F> {
// TODO: return as Result() // TODO: return as Result()
pub fn new(spi: SPI, mut nss: NSS) -> Self { pub fn new(spi: SPI, mut nss: NSS, delay_ns: F) -> Self {
sb10q commented 2021-01-22 18:03:04 +08:00
Outdated
Review
Copy Link

why f and not delay_ns?

why ``f`` and not ``delay_ns``?
nss.set_high(); nss.set_high();
SpiPort { SpiPort {
spi, spi,
nss nss,
delay_ns,
} }
} }
@ -115,6 +119,10 @@ impl <SPI: Transfer<u8>,
Ok(()) Ok(())
} }
pub fn delay_us(&mut self, duration: u32) {
(self.delay_ns)(duration * 1000)
}
// TODO: Generalise transfer functions // TODO: Generalise transfer functions
// TODO: (Make data read/write as reference to array) // TODO: (Make data read/write as reference to array)
// Currently requires 1-byte addr, read/write data is only 1-byte // 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) { match self.spi.transfer(&mut buf) {
Ok(_) => { Ok(_) => {
// Disable chip select // Disable chip select
cortex_m::asm::delay(10_u32); (self.delay_ns)(60);
sb10q commented 2021-01-22 15:02:53 +08:00
Outdated
Review
Copy Link

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

Wouldn't that significantly increase the delay, and noticeably slow things down?
occheung commented 2021-01-22 15:12:05 +08:00
Outdated
Review
Copy Link

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.
harry commented 2021-04-21 11:35:55 +08:00
Outdated
Review
Copy Link

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?
occheung commented 2021-04-21 12:56:05 +08:00
Outdated
Review
Copy Link

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.
harry commented 2021-04-21 17:16:51 +08:00
Outdated
Review
Copy Link

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.nss.set_high();
cortex_m::asm::delay(5_u32); (self.delay_ns)(30);
Ok(buf[2]) Ok(buf[2])
}, },
// TODO: Maybe too naive? // TODO: Maybe too naive?
Err(_) => { Err(_) => {
// Disable chip select // Disable chip select
cortex_m::asm::delay(10_u32); (self.delay_ns)(60);
self.nss.set_high(); self.nss.set_high();
cortex_m::asm::delay(5_u32); (self.delay_ns)(30);
Err(SpiPortError::TransferError) Err(SpiPortError::TransferError)
} }
} }