1124: Fix two SPI bugs for stm32 r=Dirbaio a=rmja

This PR fixes two bugs:
* It fixes #1095 by ensuring that pin speed is VeryHigh for all spi versions. I am on stm32f429 which seems to be spi_v1, and it also needs the VeryHigh pin speed. Otherwise bit errors on the "last bit in every byte" can happen.
* It also fixes a lifetime bug for the tx buffer when sending "write_repeated". The issue can be seen when doing spi.write where the clock byte changes during a transmission because the buffer handled to the dma must live throughout the entire transfer.

Co-authored-by: Rasmus Melchior Jacobsen <rmja@laesoe.org>
This commit is contained in:
bors[bot] 2022-12-23 15:53:59 +00:00 committed by GitHub
commit 40ef66cdfb
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 12 additions and 19 deletions

View File

@ -78,8 +78,7 @@ foreach_dma_channel! {
); );
} }
unsafe fn start_write_repeated<W: Word>(&mut self, _request: Request, repeated: W, count: usize, reg_addr: *mut W, options: TransferOptions) { unsafe fn start_write_repeated<W: Word>(&mut self, _request: Request, repeated: *const W, count: usize, reg_addr: *mut W, options: TransferOptions) {
let buf = [repeated];
low_level_api::start_transfer( low_level_api::start_transfer(
pac::$dma_peri, pac::$dma_peri,
$channel_num, $channel_num,
@ -87,7 +86,7 @@ foreach_dma_channel! {
_request, _request,
vals::Dir::FROMMEMORY, vals::Dir::FROMMEMORY,
reg_addr as *const u32, reg_addr as *const u32,
buf.as_ptr() as *mut u32, repeated as *mut u32,
count, count,
false, false,
vals::Size::from(W::bits()), vals::Size::from(W::bits()),

View File

@ -102,15 +102,14 @@ foreach_dma_channel! {
) )
} }
unsafe fn start_write_repeated<W: Word>(&mut self, request: Request, repeated: W, count: usize, reg_addr: *mut W, options: TransferOptions) { unsafe fn start_write_repeated<W: Word>(&mut self, request: Request, repeated: *const W, count: usize, reg_addr: *mut W, options: TransferOptions) {
let buf = [repeated];
low_level_api::start_transfer( low_level_api::start_transfer(
pac::$dma_peri, pac::$dma_peri,
$channel_num, $channel_num,
request, request,
vals::Dir::MEMORYTOPERIPHERAL, vals::Dir::MEMORYTOPERIPHERAL,
reg_addr as *const u32, reg_addr as *const u32,
buf.as_ptr() as *mut u32, repeated as *mut u32,
count, count,
false, false,
vals::Size::from(W::bits()), vals::Size::from(W::bits()),

View File

@ -75,15 +75,14 @@ foreach_dma_channel! {
) )
} }
unsafe fn start_write_repeated<W: Word>(&mut self, request: Request, repeated: W, count: usize, reg_addr: *mut W, options: TransferOptions) { unsafe fn start_write_repeated<W: Word>(&mut self, request: Request, repeated: *const W, count: usize, reg_addr: *mut W, options: TransferOptions) {
let buf = [repeated];
low_level_api::start_transfer( low_level_api::start_transfer(
pac::$dma_peri, pac::$dma_peri,
$channel_num, $channel_num,
request, request,
low_level_api::Dir::MemoryToPeripheral, low_level_api::Dir::MemoryToPeripheral,
reg_addr as *const u32, reg_addr as *const u32,
buf.as_ptr() as *mut u32, repeated as *mut u32,
count, count,
false, false,
W::bits(), W::bits(),

View File

@ -59,7 +59,7 @@ pub(crate) mod sealed {
unsafe fn start_write_repeated<W: super::Word>( unsafe fn start_write_repeated<W: super::Word>(
&mut self, &mut self,
request: Request, request: Request,
repeated: W, repeated: *const W,
count: usize, count: usize,
reg_addr: *mut W, reg_addr: *mut W,
options: TransferOptions, options: TransferOptions,
@ -246,7 +246,7 @@ mod transfers {
pub fn write_repeated<'a, W: Word>( pub fn write_repeated<'a, W: Word>(
channel: impl Peripheral<P = impl Channel> + 'a, channel: impl Peripheral<P = impl Channel> + 'a,
request: Request, request: Request,
repeated: W, repeated: *const W,
count: usize, count: usize,
reg_addr: *mut W, reg_addr: *mut W,
) -> impl Future<Output = ()> + 'a { ) -> impl Future<Output = ()> + 'a {

View File

@ -95,13 +95,10 @@ impl<'d, T: Instance, Tx, Rx> Spi<'d, T, Tx, Rx> {
into_ref!(peri, sck, mosi, miso); into_ref!(peri, sck, mosi, miso);
unsafe { unsafe {
sck.set_as_af(sck.af_num(), AFType::OutputPushPull); sck.set_as_af(sck.af_num(), AFType::OutputPushPull);
#[cfg(any(spi_v2, spi_v3, spi_v4))]
sck.set_speed(crate::gpio::Speed::VeryHigh); sck.set_speed(crate::gpio::Speed::VeryHigh);
mosi.set_as_af(mosi.af_num(), AFType::OutputPushPull); mosi.set_as_af(mosi.af_num(), AFType::OutputPushPull);
#[cfg(any(spi_v2, spi_v3, spi_v4))]
mosi.set_speed(crate::gpio::Speed::VeryHigh); mosi.set_speed(crate::gpio::Speed::VeryHigh);
miso.set_as_af(miso.af_num(), AFType::Input); miso.set_as_af(miso.af_num(), AFType::Input);
#[cfg(any(spi_v2, spi_v3, spi_v4))]
miso.set_speed(crate::gpio::Speed::VeryHigh); miso.set_speed(crate::gpio::Speed::VeryHigh);
} }
@ -129,10 +126,8 @@ impl<'d, T: Instance, Tx, Rx> Spi<'d, T, Tx, Rx> {
into_ref!(sck, miso); into_ref!(sck, miso);
unsafe { unsafe {
sck.set_as_af(sck.af_num(), AFType::OutputPushPull); sck.set_as_af(sck.af_num(), AFType::OutputPushPull);
#[cfg(any(spi_v2, spi_v3, spi_v4))]
sck.set_speed(crate::gpio::Speed::VeryHigh); sck.set_speed(crate::gpio::Speed::VeryHigh);
miso.set_as_af(miso.af_num(), AFType::Input); miso.set_as_af(miso.af_num(), AFType::Input);
#[cfg(any(spi_v2, spi_v3, spi_v4))]
miso.set_speed(crate::gpio::Speed::VeryHigh); miso.set_speed(crate::gpio::Speed::VeryHigh);
} }
@ -160,10 +155,8 @@ impl<'d, T: Instance, Tx, Rx> Spi<'d, T, Tx, Rx> {
into_ref!(sck, mosi); into_ref!(sck, mosi);
unsafe { unsafe {
sck.set_as_af(sck.af_num(), AFType::OutputPushPull); sck.set_as_af(sck.af_num(), AFType::OutputPushPull);
#[cfg(any(spi_v2, spi_v3, spi_v4))]
sck.set_speed(crate::gpio::Speed::VeryHigh); sck.set_speed(crate::gpio::Speed::VeryHigh);
mosi.set_as_af(mosi.af_num(), AFType::OutputPushPull); mosi.set_as_af(mosi.af_num(), AFType::OutputPushPull);
#[cfg(any(spi_v2, spi_v3, spi_v4))]
mosi.set_speed(crate::gpio::Speed::VeryHigh); mosi.set_speed(crate::gpio::Speed::VeryHigh);
} }
@ -474,7 +467,7 @@ impl<'d, T: Instance, Tx, Rx> Spi<'d, T, Tx, Rx> {
let tx_request = self.txdma.request(); let tx_request = self.txdma.request();
let tx_dst = T::REGS.tx_ptr(); let tx_dst = T::REGS.tx_ptr();
let clock_byte = 0x00u8; let clock_byte = 0x00u8;
let tx_f = crate::dma::write_repeated(&mut self.txdma, tx_request, clock_byte, clock_byte_count, tx_dst); let tx_f = crate::dma::write_repeated(&mut self.txdma, tx_request, &clock_byte, clock_byte_count, tx_dst);
unsafe { unsafe {
set_txdmaen(T::REGS, true); set_txdmaen(T::REGS, true);
@ -772,10 +765,13 @@ fn finish_dma(regs: Regs) {
#[cfg(not(any(spi_v3, spi_v4)))] #[cfg(not(any(spi_v3, spi_v4)))]
while regs.sr().read().bsy() {} while regs.sr().read().bsy() {}
// Disable the spi peripheral
regs.cr1().modify(|w| { regs.cr1().modify(|w| {
w.set_spe(false); w.set_spe(false);
}); });
// The peripheral automatically disables the DMA stream on completion without error,
// but it does not clear the RXDMAEN/TXDMAEN flag in CR2.
#[cfg(not(any(spi_v3, spi_v4)))] #[cfg(not(any(spi_v3, spi_v4)))]
regs.cr2().modify(|reg| { regs.cr2().modify(|reg| {
reg.set_txdmaen(false); reg.set_txdmaen(false);