From 63ac7ac799b07c18a9a621b4d20ead2b39982ef5 Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Mon, 2 Aug 2021 12:40:01 +0200 Subject: [PATCH] Mark `new`s as unsafe due to not being leak-safe. --- embassy-hal-common/src/usb/mod.rs | 5 +- embassy-stm32/src/eth/v2/mod.rs | 111 +++++++++++++++--------------- examples/stm32h7/src/bin/eth.rs | 10 +-- 3 files changed, 64 insertions(+), 62 deletions(-) diff --git a/embassy-hal-common/src/usb/mod.rs b/embassy-hal-common/src/usb/mod.rs index 0940e6b02..ae9f26075 100644 --- a/embassy-hal-common/src/usb/mod.rs +++ b/embassy-hal-common/src/usb/mod.rs @@ -60,7 +60,8 @@ where T: ClassSet, I: USBInterrupt, { - pub fn new>( + /// safety: the returned instance is not leak-safe + pub unsafe fn new>( state: &'bus mut State<'bus, B, T, I>, device: UsbDevice<'bus, B>, class_set: S, @@ -71,7 +72,7 @@ where classes: class_set.into_class_set(), _interrupt: PhantomData, }; - let mutex = unsafe { PeripheralMutex::new_unchecked(&mut state.0, initial_state, irq) }; + let mutex = PeripheralMutex::new_unchecked(&mut state.0, initial_state, irq); Self { inner: RefCell::new(mutex), } diff --git a/embassy-stm32/src/eth/v2/mod.rs b/embassy-stm32/src/eth/v2/mod.rs index 2c73e0d03..37bc9715a 100644 --- a/embassy-stm32/src/eth/v2/mod.rs +++ b/embassy-stm32/src/eth/v2/mod.rs @@ -34,7 +34,8 @@ pub struct Ethernet<'d, P: PHY, const TX: usize, const RX: usize> { } impl<'d, P: PHY, const TX: usize, const RX: usize> Ethernet<'d, P, TX, RX> { - pub fn new( + /// safety: the returned instance is not leak-safe + pub unsafe fn new( state: &'d mut State<'d, TX, RX>, peri: impl Unborrow + 'd, interrupt: impl Unborrow + 'd, @@ -55,7 +56,7 @@ impl<'d, P: PHY, const TX: usize, const RX: usize> Ethernet<'d, P, TX, RX> { // Enable the necessary Clocks // NOTE(unsafe) We have exclusive access to the registers - critical_section::with(|_| unsafe { + critical_section::with(|_| { RCC.apb4enr().modify(|w| w.set_syscfgen(true)); RCC.ahb1enr().modify(|w| { w.set_eth1macen(true); @@ -78,52 +79,52 @@ impl<'d, P: PHY, const TX: usize, const RX: usize> Ethernet<'d, P, TX, RX> { tx_en.configure(); let inner = Inner::new(peri); - let state = unsafe { PeripheralMutex::new_unchecked(&mut state.0, inner, interrupt) }; + + // NOTE(unsafe) We are ourselves not leak-safe. + let state = PeripheralMutex::new_unchecked(&mut state.0, inner, interrupt); // NOTE(unsafe) We have exclusive access to the registers - unsafe { - let dma = ETH.ethernet_dma(); - let mac = ETH.ethernet_mac(); - let mtl = ETH.ethernet_mtl(); + let dma = ETH.ethernet_dma(); + let mac = ETH.ethernet_mac(); + let mtl = ETH.ethernet_mtl(); - // Reset and wait - dma.dmamr().modify(|w| w.set_swr(true)); - while dma.dmamr().read().swr() {} + // Reset and wait + dma.dmamr().modify(|w| w.set_swr(true)); + while dma.dmamr().read().swr() {} - mac.maccr().modify(|w| { - w.set_ipg(0b000); // 96 bit times - w.set_acs(true); - w.set_fes(true); - w.set_dm(true); - // TODO: Carrier sense ? ECRSFD - }); + mac.maccr().modify(|w| { + w.set_ipg(0b000); // 96 bit times + w.set_acs(true); + w.set_fes(true); + w.set_dm(true); + // TODO: Carrier sense ? ECRSFD + }); - mac.maca0lr().write(|w| { - w.set_addrlo( - u32::from(mac_addr[0]) - | (u32::from(mac_addr[1]) << 8) - | (u32::from(mac_addr[2]) << 16) - | (u32::from(mac_addr[3]) << 24), - ) - }); - mac.maca0hr() - .modify(|w| w.set_addrhi(u16::from(mac_addr[4]) | (u16::from(mac_addr[5]) << 8))); + mac.maca0lr().write(|w| { + w.set_addrlo( + u32::from(mac_addr[0]) + | (u32::from(mac_addr[1]) << 8) + | (u32::from(mac_addr[2]) << 16) + | (u32::from(mac_addr[3]) << 24), + ) + }); + mac.maca0hr() + .modify(|w| w.set_addrhi(u16::from(mac_addr[4]) | (u16::from(mac_addr[5]) << 8))); - mac.macpfr().modify(|w| w.set_saf(true)); - mac.macqtx_fcr().modify(|w| w.set_pt(0x100)); + mac.macpfr().modify(|w| w.set_saf(true)); + mac.macqtx_fcr().modify(|w| w.set_pt(0x100)); - mtl.mtlrx_qomr().modify(|w| w.set_rsf(true)); - mtl.mtltx_qomr().modify(|w| w.set_tsf(true)); + mtl.mtlrx_qomr().modify(|w| w.set_rsf(true)); + mtl.mtltx_qomr().modify(|w| w.set_tsf(true)); - dma.dmactx_cr().modify(|w| w.set_txpbl(1)); // 32 ? - dma.dmacrx_cr().modify(|w| { - w.set_rxpbl(1); // 32 ? - w.set_rbsz(MTU as u16); - }); - } + dma.dmactx_cr().modify(|w| w.set_txpbl(1)); // 32 ? + dma.dmacrx_cr().modify(|w| { + w.set_rxpbl(1); // 32 ? + w.set_rbsz(MTU as u16); + }); // NOTE(unsafe) We got the peripheral singleton, which means that `rcc::init` was called - let hclk = unsafe { crate::rcc::get_freqs().ahb1 }; + let hclk = crate::rcc::get_freqs().ahb1; let hclk_mhz = hclk.0 / 1_000_000; // Set the MDC clock frequency in the range 1MHz - 2.5MHz @@ -165,27 +166,25 @@ impl<'d, P: PHY, const TX: usize, const RX: usize> Ethernet<'d, P, TX, RX> { fence(Ordering::SeqCst); - unsafe { - let mac = ETH.ethernet_mac(); - let mtl = ETH.ethernet_mtl(); - let dma = ETH.ethernet_dma(); + let mac = ETH.ethernet_mac(); + let mtl = ETH.ethernet_mtl(); + let dma = ETH.ethernet_dma(); - mac.maccr().modify(|w| { - w.set_re(true); - w.set_te(true); - }); - mtl.mtltx_qomr().modify(|w| w.set_ftq(true)); + mac.maccr().modify(|w| { + w.set_re(true); + w.set_te(true); + }); + mtl.mtltx_qomr().modify(|w| w.set_ftq(true)); - dma.dmactx_cr().modify(|w| w.set_st(true)); - dma.dmacrx_cr().modify(|w| w.set_sr(true)); + dma.dmactx_cr().modify(|w| w.set_st(true)); + dma.dmacrx_cr().modify(|w| w.set_sr(true)); - // Enable interrupts - dma.dmacier().modify(|w| { - w.set_nie(true); - w.set_rie(true); - w.set_tie(true); - }); - } + // Enable interrupts + dma.dmacier().modify(|w| { + w.set_nie(true); + w.set_rie(true); + w.set_tie(true); + }); }); P::phy_reset(&mut this); P::phy_init(&mut this); diff --git a/examples/stm32h7/src/bin/eth.rs b/examples/stm32h7/src/bin/eth.rs index 5cf49e82f..e49a101bf 100644 --- a/examples/stm32h7/src/bin/eth.rs +++ b/examples/stm32h7/src/bin/eth.rs @@ -135,10 +135,12 @@ fn main() -> ! { let eth_int = interrupt_take!(ETH); let mac_addr = [0x10; 6]; let state = STATE.put(State::new()); - let eth = ETH.put(Ethernet::new( - state, p.ETH, eth_int, p.PA1, p.PA2, p.PC1, p.PA7, p.PC4, p.PC5, p.PB12, p.PB13, p.PB11, - LAN8742A, mac_addr, 1, - )); + let eth = unsafe { + ETH.put(Ethernet::new( + state, p.ETH, eth_int, p.PA1, p.PA2, p.PC1, p.PA7, p.PC4, p.PC5, p.PB12, p.PB13, + p.PB11, LAN8742A, mac_addr, 1, + )) + }; let config = StaticConfigurator::new(NetConfig { address: Ipv4Cidr::new(Ipv4Address::new(192, 168, 0, 61), 24),