Skip to content
Closed
Show file tree
Hide file tree
Changes from 7 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
Copy Markdown
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 {
Comment thread
CoolSlimbo marked this conversation as resolved.
fn raw_setup(&mut self, settings: &Settings) {
use $crate::hal::spi;

// Setup control registers
// We start by setting the UBBRn to 0
Copy link
Copy Markdown
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
Copy Markdown
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()
);
Comment on lines +59 to +91
Copy link
Copy Markdown
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
Copy Markdown
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());
}
Comment on lines +70 to +110
Copy link
Copy Markdown
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()
}
}
}
};
}
3 changes: 3 additions & 0 deletions mcu/atmega-hal/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,9 @@ pub mod eeprom;
#[cfg(feature = "device-selected")]
pub use eeprom::Eeprom;

#[cfg(feature = "device-selected")]
pub mod usart_spi;

pub struct Atmega;

#[cfg(any(feature = "atmega48p", feature = "atmega168", feature = "atmega328p"))]
Expand Down
6 changes: 4 additions & 2 deletions mcu/atmega-hal/src/usart.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ pub type UsartReader<USART, RX, TX, CLOCK> =
feature = "atmega328p",
feature = "atmega328pb",
feature = "atmega1284p",
feature = "atmega164pa"
feature = "atmega164pa",
feature = "atmega48p"
))]
pub type Usart0<CLOCK> = Usart<
crate::pac::USART0,
Expand All @@ -27,7 +28,8 @@ pub type Usart0<CLOCK> = Usart<
feature = "atmega328p",
feature = "atmega328pb",
feature = "atmega1284p",
feature = "atmega164pa"
feature = "atmega164pa",
feature = "atmega48p"
))]
avr_hal_generic::impl_usart_traditional! {
hal: crate::Atmega,
Expand Down
135 changes: 135 additions & 0 deletions mcu/atmega-hal/src/usart_spi.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
//! USART MSPIM implimentations
//!
//! The following list details how many USARTs and if the USARTs support MSPIM for each board choosable.
//!
//! | Board | USARTs | SPI |
//! |-------|--------|-----|
//! | `atmega48p` | 1 | Yes |
//! | `atmega164pa`| 2 | Yes |
//! | `atmega168` | 1 | Yes |
//! | `atmega328p` | 1 | Yes |
//! | `atmega328pb` | 1 | Yes |
//! | `atmega32a` | 1 | No |
//! | `atmega32u4` | 1 | Yes |
//! | `atmega2560` | 4 | Yes |
//! | `atmega128a` | 2 | No |
//! | `atmega1280` | 4 | Yes |
//! | `atmega1284p` | 2 | Yes |
//! | `atmega8` | 1 | No |

use crate::port;
use crate::spi::Settings;

#[cfg(any(feature = "atmega1280", feature = "atmega2560"))]
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 = "atmega1280", feature = "atmega2560"))]
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,
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Hm, this is quite a hack. We really don't need the cs dance for MSPIM so it would be nice to somehow wrangle the SPI module to not require a CS pin for MSPIM...

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

This was done because I impliment SpiOps to cheap out on work, and not rewrite everything.

I can either reimplement a new spiops, or, force the use of a DummyPin internally (so users don’t have to worry about it) via the DummyPin crate (I believe that’s its name)

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I would be fine with defining a UsartSpiDummyCs pin type that needs to be passed to Spi::new() to sidestep this. Pulling in an external crate for this is not a good idea, I think.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

An alternative to the Dummy CS Pin (and what I believe my original thought process was), is how it is, where it's dynamic, meaning the CS can be handled by the driver in some which way, by manual toggle instead of SPI handling it.

}

#[cfg(any(feature = "atmega1280", feature = "atmega2560"))]
pub type Usart1Spi = avr_hal_generic::usart_spi::UsartSpi<
crate::Atmega,
crate::pac::USART1,
port::PD5,
port::PD3,
port::PD2,
port::Dynamic,
>;
#[cfg(any(feature = "atmega1280", feature = "atmega2560"))]
avr_hal_generic::impl_usart_spi! {
hal: crate::Atmega,
peripheral: crate::pac::USART1,
register_suffix: 1,
sclk: port::PD5,
mosi: port::PD3,
miso: port::PD2,
cs: port::Dynamic,
}

#[cfg(any(feature = "atmega1280", feature = "atmega2560"))]
pub type Usart2Spi = avr_hal_generic::usart_spi::UsartSpi<
crate::Atmega,
crate::pac::USART2,
port::PH2,
port::PH1,
port::PH0,
port::Dynamic,
>;
#[cfg(any(feature = "atmega1280", feature = "atmega2560"))]
avr_hal_generic::impl_usart_spi! {
hal: crate::Atmega,
peripheral: crate::pac::USART2,
register_suffix: 2,
sclk: port::PH2,
mosi: port::PH1,
miso: port::PH0,
cs: port::Dynamic,
}

#[cfg(any(feature = "atmega1280", feature = "atmega2560"))]
pub type Usart3Spi = avr_hal_generic::usart_spi::UsartSpi<
crate::Atmega,
crate::pac::USART3,
port::PJ2,
port::PJ1,
port::PJ0,
port::Dynamic,
>;
#[cfg(any(feature = "atmega1280", feature = "atmega2560"))]
avr_hal_generic::impl_usart_spi! {
hal: crate::Atmega,
peripheral: crate::pac::USART3,
register_suffix: 3,
sclk: port::PJ2,
mosi: port::PJ1,
miso: port::PJ0,
cs: port::Dynamic,
}

#[cfg(any(
feature = "atmega168",
feature = "atmega328p",
feature = "atmega328pb",
feature = "atmega1284p",
feature = "atmega164pa",
feature = "atmega48p"
))]
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 = "atmega328pb",
feature = "atmega1284p",
feature = "atmega164pa",
feature = "atmega48p"
))]
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,
}