Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UsartSpi Support #562

Closed
wants to merge 19 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions avr-hal-generic/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ pub mod port;
pub mod simple_pwm;
pub mod spi;
pub mod usart;
pub mod usart_spi;
pub mod wdt;

/// Prelude containing all HAL traits
Expand Down
91 changes: 91 additions & 0 deletions avr-hal-generic/src/usart_spi.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
//! MSPIM Implimentation
use crate::spi;

// This module just impliments a macro for SpiOps, since underlyingly, the Spi type can still be used since it just needs SpiOps

pub type UsartSpi<H, USART, SCLKPIN, MOSIPIN, MISOPIN, CSPIN> =
spi::Spi<H, USART, SCLKPIN, MOSIPIN, MISOPIN, CSPIN>;

// Impliment SpiOps trait for USART
#[macro_export]
macro_rules! impl_usart_spi {
(
hal: $HAL:ty,
peripheral: $USART_SPI:ty,
register_suffix: $n:expr,
sclk: $sclkpin:ty,
mosi: $mosipin:ty,
miso: $misopin:ty,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
miso: $misopin:ty,
miso: $misopin:ty,
cs: $cspin:ty,

cs: $cspin:ty,
) => {
$crate::paste::paste! {
impl $crate::spi::SpiOps<$HAL, $sclkpin, $mosipin, $misopin, $cspin> for $USART_SPI {
fn raw_setup(&mut self, settings: &Settings) {
use $crate::hal::spi;

// Setup control registers
// We start by setting the UBBRn to 0
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't comment what code is doing, that should be obvious from reading the line below. Please comment why it is doing this — what's the purpose of setting UBBRn to 0 here?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unsure of the purpose.

But code examples in the data sheet all do so, explicitly setting it to 0, before setting the baudrate (which I will update the calculation for)

self.[<ubrr $n>].write(|w| w.bits(0));

// We have to translate the character size register into the 2 bits which are the MSB/LSB and the phase
// 5 Bit Char = MSB and 1st
// 6 Bit Char = MSB and 2nd
// 7 Bit Char = LSB and 1st
// 8 Bit Char = LSB and 2nd
self.[<ucsr $n c>].write(|w| {
w.[<umsel $n>]().spi_master();

match settings.data_order {
DataOrder::MostSignificantFirst => match settings.mode.phase {
spi::Phase::CaptureOnFirstTransition => w.[<ucsz $n>]().chr5(),
spi::Phase::CaptureOnSecondTransition => w.[<ucsz $n>]().chr6(),
},
DataOrder::LeastSignificantFirst => match settings.mode.phase {
spi::Phase::CaptureOnFirstTransition => w.[<ucsz $n>]().chr7(),
spi::Phase::CaptureOnSecondTransition => w.[<ucsz $n>]().chr8(),
},
};

match settings.mode.polarity {
spi::Polarity::IdleLow => w.[<ucpol $n>]().clear_bit(),
spi::Polarity::IdleHigh => w.[<ucpol $n>]().set_bit(),
}
});

// Enable receiver and transmitter, and also the rec interrupt.
self.[<ucsr $n b>].write(|w| w
.[<txen $n>]().set_bit()
.[<rxen $n>]().set_bit()
.[<rxcie $n>]().set_bit()
);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't enable the interrupt by default - it should always be explicitly enabled by the user through a method call, see e.g.

/// Enable the interrupt for [`Event`].
pub fn listen(&mut self, event: Event) {
self.p.raw_interrupt(event, true);
}
/// Disable the interrupt for [`Event`].
pub fn unlisten(&mut self, event: Event) {
self.p.raw_interrupt(event, false);
}

You don't need to implement this here, though — it should be part of the spi module.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you elaborate on what you mean by this?

I understand why to remove it (idk why it was there to begin with), but do you want me to add methods to change the interrupts? Or something other, because I don't understand "it should be part of the spi module"


// Set the baudrate of the UBRRn, idk what it should be set to, so for now, it'll be set to 0
self.[<ubrr $n>].write(|w| w.bits(0));
}

fn raw_release(&mut self) {
// Probably a better way to "release" the SPI interface, but from the datasheet, this is what they suggest, so ig it works
self.[<ucsr $n c>].write(|w| w.[<umsel $n>]().usart_async());
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to the USART code, you also need to disable the USART peripheral here:

fn raw_deinit(&mut self) {
// Wait for any ongoing transfer to finish.
$crate::nb::block!(self.raw_flush()).ok();
self.[<ucsr $n b>].reset();
}

(The UCSRnB reset is the important part).

With that included, you can drop your comment.


fn raw_check_iflag(&self) -> bool {
self.[<ucsr $n a>].read().[<rxc $n>]().bit_is_set()
}

fn raw_read(&self) -> u8 {
self.[<udr $n>].read().bits()
}

fn raw_write(&mut self, byte: u8) {
self.[<udr $n>].write(|w| unsafe { w.bits(byte) });
}

fn raw_transaction(&mut self, byte: u8) -> u8 {
self.raw_write(byte);
while !self.raw_check_iflag() {}
self.raw_read()
}
}
}
};
}
60 changes: 60 additions & 0 deletions mcu/atmega-hal/src/spi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,66 @@ avr_hal_generic::impl_spi! {
cs: port::PB0,
}

#[cfg(any(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// Devices where UART port can be a SPI port

feature = "atmega128a",
feature = "atmega1280",
feature = "atmega2560",
feature = "atmega32u4"
))]
pub type Usart0Spi = avr_hal_generic::usart_spi::UsartSpi<
crate::Atmega,
crate::pac::USART0,
port::PE2,
port::PE1,
port::PE0,
port::Dynamic,
>;
#[cfg(any(
feature = "atmega128a",
feature = "atmega1280",
feature = "atmega2560",
feature = "atmega32u4"
))]
avr_hal_generic::impl_usart_spi! {
hal: crate::Atmega,
peripheral: crate::pac::USART0,
register_suffix: 0,
sclk: port::PE2,
mosi: port::PE1,
miso: port::PE0,
cs: port::Dynamic,
}

#[cfg(any(
feature = "atmega168",
feature = "atmega328p",
feature = "atmega48p",
feature = "atmega8"
))]
pub type Usart0Spi = avr_hal_generic::usart_spi::UsartSpi<
crate::Atmega,
crate::pac::USART0,
port::PD4,
port::PD1,
port::PD0,
port::Dynamic,
>;
#[cfg(any(
feature = "atmega168",
feature = "atmega328p",
feature = "atmega48p",
feature = "atmega8"
))]
avr_hal_generic::impl_usart_spi! {
hal: crate::Atmega,
peripheral: crate::pac::USART0,
register_suffix: 0,
sclk: port::PD4,
mosi: port::PD1,
miso: port::PD0,
cs: port::Dynamic,
}

#[cfg(any(
feature = "atmega168",
feature = "atmega328p",
Expand Down