diff --git a/esp-hal/CHANGELOG.md b/esp-hal/CHANGELOG.md index 1492e8c633..5c492d318a 100644 --- a/esp-hal/CHANGELOG.md +++ b/esp-hal/CHANGELOG.md @@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `esp_hal::time::{Rate, Duration, Instant}` (#3083) - Async support for ADC oneshot reads for ESP32C2, ESP32C3, ESP32C6 and ESP32H2 (#2925, #3082) - `ESP_HAL_CONFIG_XTAL_FREQUENCY` configuration. For now, chips other than ESP32 and ESP32-C2 have a single option only. (#3054) +- Added more validation to UART and SPI. User can now specify the baudrate tolerance of UART config (#3074) ### Changed diff --git a/esp-hal/src/gpio/mod.rs b/esp-hal/src/gpio/mod.rs index 256a84c788..3e0773cee5 100644 --- a/esp-hal/src/gpio/mod.rs +++ b/esp-hal/src/gpio/mod.rs @@ -1262,7 +1262,6 @@ impl<'d> Output<'d> { } /// Change the configuration. - // FIXME: when https://github.com/esp-rs/esp-hal/issues/2839 is resolved, add an appropriate `# Error` entry. #[inline] pub fn apply_config(&mut self, config: &OutputConfig) { self.pin.apply_output_config(config) @@ -1451,7 +1450,6 @@ impl<'d> Input<'d> { } /// Change the configuration. - // FIXME: when https://github.com/esp-rs/esp-hal/issues/2839 is resolved, add an appropriate `# Error` entry. pub fn apply_config(&mut self, config: &InputConfig) { self.pin.apply_input_config(config) } diff --git a/esp-hal/src/lcd_cam/cam.rs b/esp-hal/src/lcd_cam/cam.rs index 8e0d0e4688..49adb391a5 100644 --- a/esp-hal/src/lcd_cam/cam.rs +++ b/esp-hal/src/lcd_cam/cam.rs @@ -166,6 +166,11 @@ impl<'d> Camera<'d> { } /// Applies the configuration to the camera interface. + /// + /// # Errors + /// + /// [`ConfigError::Clock`] will be returned if the frequency passed in + /// `Config` is too low. pub fn apply_config(&mut self, config: &Config) -> Result<(), ConfigError> { let clocks = Clocks::get(); let (i, divider) = calculate_clkm( diff --git a/esp-hal/src/lcd_cam/lcd/dpi.rs b/esp-hal/src/lcd_cam/lcd/dpi.rs index 5aa958aedf..38d6f6b7ea 100644 --- a/esp-hal/src/lcd_cam/lcd/dpi.rs +++ b/esp-hal/src/lcd_cam/lcd/dpi.rs @@ -169,6 +169,11 @@ where } /// Applies the configuration to the peripheral. + /// + /// # Errors + /// + /// [`ConfigError::Clock`] variant will be returned if the frequency passed + /// in `Config` is too low. pub fn apply_config(&mut self, config: &Config) -> Result<(), ConfigError> { let clocks = Clocks::get(); // Due to https://www.espressif.com/sites/default/files/documentation/esp32-s3_errata_en.pdf diff --git a/esp-hal/src/lcd_cam/lcd/i8080.rs b/esp-hal/src/lcd_cam/lcd/i8080.rs index 3740a8878e..af21afd0cf 100644 --- a/esp-hal/src/lcd_cam/lcd/i8080.rs +++ b/esp-hal/src/lcd_cam/lcd/i8080.rs @@ -134,6 +134,11 @@ where } /// Applies configuration. + /// + /// # Errors + /// + /// [`ConfigError::Clock`] variant will be returned if the frequency passed + /// in `Config` is too low. pub fn apply_config(&mut self, config: &Config) -> Result<(), ConfigError> { let clocks = Clocks::get(); // Due to https://www.espressif.com/sites/default/files/documentation/esp32-s3_errata_en.pdf diff --git a/esp-hal/src/spi/master.rs b/esp-hal/src/spi/master.rs index 54a9ad0bcf..0aaf40a54e 100644 --- a/esp-hal/src/spi/master.rs +++ b/esp-hal/src/spi/master.rs @@ -585,6 +585,21 @@ impl Config { fn raw_clock_reg_value(&self) -> Result { self.reg } + + fn validate(&self) -> Result<(), ConfigError> { + cfg_if::cfg_if! { + if #[cfg(esp32h2)] { + if self.frequency < Rate::from_khz(70) || self.frequency > Rate::from_mhz(48) { + return Err(ConfigError::UnsupportedFrequency); + } + } else { + if self.frequency < Rate::from_khz(70) || self.frequency > Rate::from_mhz(80) { + return Err(ConfigError::UnsupportedFrequency); + } + } + } + Ok(()) + } } #[derive(Debug)] @@ -602,7 +617,22 @@ struct SpiPinGuard { #[non_exhaustive] #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] #[cfg_attr(feature = "defmt", derive(defmt::Format))] -pub enum ConfigError {} +pub enum ConfigError { + /// The requested frequency is not supported. + UnsupportedFrequency, +} + +impl core::error::Error for ConfigError {} + +impl core::fmt::Display for ConfigError { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + match self { + ConfigError::UnsupportedFrequency => { + write!(f, "The requested frequency is not supported") + } + } + } +} /// SPI peripheral driver /// @@ -666,7 +696,10 @@ where impl<'d> Spi<'d, Blocking> { /// Constructs an SPI instance in 8bit dataframe mode. - // FIXME: when https://github.com/esp-rs/esp-hal/issues/2839 is resolved, add an appropriate `# Error` entry. + /// + /// # Errors + /// + /// See [`Spi::apply_config`]. pub fn new( spi: impl Peripheral

+ 'd, config: Config, @@ -983,7 +1016,14 @@ where } /// Change the bus configuration. - // FIXME: when https://github.com/esp-rs/esp-hal/issues/2839 is resolved, add an appropriate `# Error` entry. + /// + /// # Errors + /// + /// If frequency passed in config exceeds + #[cfg_attr(not(esp32h2), doc = " 80MHz")] + #[cfg_attr(esp32h2, doc = " 48MHz")] + /// or is below 70kHz, + /// [`ConfigError::UnsupportedFrequency`] error will be returned. pub fn apply_config(&mut self, config: &Config) -> Result<(), ConfigError> { self.driver().apply_config(config) } @@ -1071,9 +1111,10 @@ where /// /// # Errors /// - /// The corresponding error variant from [`Error`] will be returned if + /// [`Error::FifoSizeExeeded`] or [`Error::Unsupported`] will be returned if /// passed buffer is bigger than FIFO size or if buffer is empty (currently - /// unsupported). + /// unsupported). `DataMode::Single` cannot be combined with any other + /// [`DataMode`], otherwise [`Error::Unsupported`] will be returned. #[instability::unstable] pub fn half_duplex_read( &mut self, @@ -1111,7 +1152,7 @@ where /// /// # Errors /// - /// The corresponding error variant from [`Error`] will be returned if + /// [`Error::FifoSizeExeeded`] will be returned if /// passed buffer is bigger than FIFO size. #[cfg_attr( esp32, @@ -1554,7 +1595,14 @@ mod dma { } /// Change the bus configuration. - // FIXME: when https://github.com/esp-rs/esp-hal/issues/2839 is resolved, add an appropriate `# Error` entry. + /// + /// # Errors + /// + /// If frequency passed in config exceeds + #[cfg_attr(not(esp32h2), doc = " 80MHz")] + #[cfg_attr(esp32h2, doc = " 48MHz")] + /// or is below 70kHz, + /// [`ConfigError::UnsupportedFrequency`] error will be returned. #[instability::unstable] pub fn apply_config(&mut self, config: &Config) -> Result<(), ConfigError> { self.driver().apply_config(config) @@ -2017,7 +2065,14 @@ mod dma { } /// Change the bus configuration. - // FIXME: when https://github.com/esp-rs/esp-hal/issues/2839 is resolved, add an appropriate `# Error` entry. + /// + /// # Errors + /// + /// If frequency passed in config exceeds + #[cfg_attr(not(esp32h2), doc = " 80MHz")] + #[cfg_attr(esp32h2, doc = " 48MHz")] + /// or is below 70kHz, + /// [`ConfigError::UnsupportedFrequency`] error will be returned. #[instability::unstable] pub fn apply_config(&mut self, config: &Config) -> Result<(), ConfigError> { self.spi_dma.apply_config(config) @@ -3064,6 +3119,7 @@ impl Driver { } fn apply_config(&self, config: &Config) -> Result<(), ConfigError> { + config.validate()?; self.ch_bus_freq(config)?; self.set_bit_order(config.read_bit_order, config.write_bit_order); self.set_data_mode(config.mode); diff --git a/esp-hal/src/uart.rs b/esp-hal/src/uart.rs index 4d9b5e3f38..bb98b09293 100644 --- a/esp-hal/src/uart.rs +++ b/esp-hal/src/uart.rs @@ -265,6 +265,21 @@ pub enum StopBits { _2 = 3, } +/// Defines how strictly the requested baud rate must be met. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] +#[cfg_attr(feature = "defmt", derive(defmt::Format))] +#[instability::unstable] +pub enum BaudrateTolerance { + /// Accept the closest achievable baud rate without restriction. + #[default] + Closest, + /// In this setting, the deviation of only 1% from the desired baud value is + /// tolerated. + Exact, + /// Allow a certain percentage of deviation. + ErrorPercent(u8), +} + /// UART Configuration #[derive(Debug, Clone, Copy, procmacros::BuilderLite)] #[cfg_attr(feature = "defmt", derive(defmt::Format))] @@ -273,6 +288,9 @@ pub struct Config { /// The baud rate (speed) of the UART communication in bits per second /// (bps). baudrate: u32, + /// Determines how close to the desired baud rate value the driver should + /// set the baud rate. + baudrate_tolerance: BaudrateTolerance, /// Number of data bits in each frame (5, 6, 7, or 8 bits). data_bits: DataBits, /// Parity setting (None, Even, or Odd). @@ -305,6 +323,7 @@ impl Default for Config { rx: RxConfig::default(), tx: TxConfig::default(), baudrate: 115_200, + baudrate_tolerance: BaudrateTolerance::default(), data_bits: Default::default(), parity: Default::default(), stop_bits: Default::default(), @@ -349,6 +368,18 @@ impl Config { }; length } + + fn validate(&self) -> Result<(), ConfigError> { + if let BaudrateTolerance::ErrorPercent(percentage) = self.baudrate_tolerance { + assert!(percentage > 0 && percentage <= 100); + } + + // Max supported baud rate is 5Mbaud + if self.baudrate == 0 || self.baudrate > 5_000_000 { + return Err(ConfigError::UnsupportedBaudrate); + } + Ok(()) + } } /// Configuration for the AT-CMD detection functionality @@ -455,6 +486,12 @@ pub struct UartRx<'d, Dm> { #[cfg_attr(feature = "defmt", derive(defmt::Format))] #[non_exhaustive] pub enum ConfigError { + /// The requested baud rate is not achievable. + #[cfg(any(doc, feature = "unstable"))] + #[cfg_attr(docsrs, doc(cfg(feature = "unstable")))] + UnachievableBaudrate, + /// The requested baud rate is not supported. + UnsupportedBaudrate, /// The requested timeout is not supported. UnsupportedTimeout, /// The requested FIFO threshold is not supported. @@ -466,6 +503,13 @@ impl core::error::Error for ConfigError {} impl core::fmt::Display for ConfigError { fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { match self { + #[cfg(feature = "unstable")] + ConfigError::UnachievableBaudrate => { + write!(f, "The requested baud rate is not achievable") + } + ConfigError::UnsupportedBaudrate => { + write!(f, "The requested baud rate is not supported") + } ConfigError::UnsupportedTimeout => write!(f, "The requested timeout is not supported"), ConfigError::UnsupportedFifoThreshold => { write!(f, "The requested FIFO threshold is not supported") @@ -545,7 +589,6 @@ where /// Change the configuration. /// /// Note that this also changes the configuration of the RX half. - // FIXME: when https://github.com/esp-rs/esp-hal/issues/2839 is resolved, add an appropriate `# Error` entry. #[instability::unstable] pub fn apply_config(&mut self, _config: &Config) -> Result<(), ConfigError> { // Nothing to do so far. @@ -739,7 +782,22 @@ where /// Change the configuration. /// /// Note that this also changes the configuration of the TX half. - // FIXME: when https://github.com/esp-rs/esp-hal/issues/2839 is resolved, add an appropriate `# Error` entry. + /// + /// # Errors + /// + /// [`ConfigError::UnsupportedFifoThreshold`] will be returned under the + /// following conditions: + /// * if the RX FIFO threshold passed in `Config` exceeds the maximum + /// value ( + #[cfg_attr(esp32, doc = "0x7F")] + #[cfg_attr(any(esp32c6, esp32h2), doc = "0xFF")] + #[cfg_attr(any(esp32c3, esp32c2, esp32s2), doc = "0x1FF")] + #[cfg_attr(esp32h2, doc = "0x3FF")] + /// ) + /// * if the passed timeout exceeds the maximum value ( + #[cfg_attr(esp32, doc = "0x7F")] + #[cfg_attr(not(esp32), doc = "0x3FF")] + /// ). #[instability::unstable] pub fn apply_config(&mut self, config: &Config) -> Result<(), ConfigError> { self.uart @@ -974,7 +1032,10 @@ impl<'d> Uart<'d, Blocking> { /// # Ok(()) /// # } /// ``` - // FIXME: when https://github.com/esp-rs/esp-hal/issues/2839 is resolved, add an appropriate `# Error` entry. + /// + /// # Errors + /// + /// See [`Uart::apply_config`]. pub fn new( uart: impl Peripheral

+ 'd, config: Config, @@ -1165,7 +1226,17 @@ where } /// Change the configuration. - // FIXME: when https://github.com/esp-rs/esp-hal/issues/2839 is resolved, add an appropriate `# Error` entry. + /// + /// # Errors + /// + /// [`ConfigError::UnsupportedFifoThreshold`] will be returned in the cases + /// described in [`UartRx::apply_config`]. + /// [`ConfigError::UnsupportedBaudrate`] will be returned under the + /// following conditions: + /// * if baud rate passed in config exceeds 5MBaud or is equal to zero. + /// * if the user has specified in the configuration that they want + /// baudrate to correspond exactly or with some percentage of deviation + /// to the desired value, and the driver cannot reach this speed pub fn apply_config(&mut self, config: &Config) -> Result<(), ConfigError> { self.rx.apply_config(config)?; self.tx.apply_config(config)?; @@ -2177,7 +2248,8 @@ impl Info { } fn apply_config(&self, config: &Config) -> Result<(), ConfigError> { - self.change_baud(config); + config.validate()?; + self.change_baud(config)?; self.change_data_bits(config.data_bits); self.change_parity(config.parity); self.change_stop_bits(config.stop_bits); @@ -2273,6 +2345,7 @@ impl Info { /// Configures the RX-FIFO threshold /// /// # Errors + /// /// [`Err(ConfigError::UnsupportedFifoThreshold)`][ConfigError::UnsupportedFifoThreshold] if provided value exceeds maximum value /// for SOC : /// - `esp32` **0x7F** @@ -2309,7 +2382,8 @@ impl Info { /// `timeout` - the number of symbols ("bytes") to wait for before /// triggering a timeout. Pass None to disable the timeout. /// - /// # Errors + /// # Errors + /// /// [`Err(ConfigError::UnsupportedTimeout)`][ConfigError::UnsupportedTimeout] if the provided value exceeds /// the maximum value for SOC : /// - `esp32`: Symbol size is fixed to 8, do not pass a value > **0x7F**. @@ -2373,7 +2447,7 @@ impl Info { sync_regs(self.regs()); } - fn change_baud(&self, config: &Config) { + fn change_baud(&self, config: &Config) -> Result<(), ConfigError> { let clocks = Clocks::get(); let clk = match config.clock_source { ClockSource::Apb => clocks.apb_clock.as_hz(), @@ -2447,6 +2521,11 @@ impl Info { }); self.sync_regs(); + + #[cfg(feature = "unstable")] + self.verify_baudrate(clk, config)?; + + Ok(()) } fn change_data_bits(&self, data_bits: DataBits) { @@ -2503,6 +2582,57 @@ impl Info { txfifo_rst(self.regs(), true); txfifo_rst(self.regs(), false); } + + #[cfg(feature = "unstable")] + fn verify_baudrate(&self, clk: u32, config: &Config) -> Result<(), ConfigError> { + // taken from https://github.com/espressif/esp-idf/blob/c5865270b50529cd32353f588d8a917d89f3dba4/components/hal/esp32c6/include/hal/uart_ll.h#L433-L444 + // (it's different for different chips) + let clkdiv_reg = self.regs().clkdiv().read(); + let clkdiv_frag = clkdiv_reg.frag().bits() as u32; + let clkdiv = clkdiv_reg.clkdiv().bits(); + + cfg_if::cfg_if! { + if #[cfg(any(esp32, esp32s2))] { + let actual_baud = (clk << 4) / ((clkdiv << 4) | clkdiv_frag); + } else if #[cfg(any(esp32c2, esp32c3, esp32s3))] { + let sclk_div_num = self.regs().clk_conf().read().sclk_div_num().bits() as u32; + let actual_baud = (clk << 4) / ((((clkdiv as u32) << 4) | clkdiv_frag) * (sclk_div_num + 1)); + } else { // esp32c6, esp32h2 + let pcr = crate::peripherals::PCR::regs(); + let conf = if self.is_instance(unsafe { crate::peripherals::UART0::steal() }) { + pcr.uart(0).clk_conf() + } else { + pcr.uart(1).clk_conf() + }; + let sclk_div_num = conf.read().sclk_div_num().bits() as u32; + let actual_baud = (clk << 4) / ((((clkdiv as u32) << 4) | clkdiv_frag) * (sclk_div_num + 1)); + } + }; + + match config.baudrate_tolerance { + BaudrateTolerance::Exact => { + let deviation = ((config.baudrate as i32 - actual_baud as i32).unsigned_abs() + * 100) + / actual_baud; + // We tolerate deviation of 1% from the desired baud value, as it never will be + // exactly the same + if deviation > 1_u32 { + return Err(ConfigError::UnachievableBaudrate); + } + } + BaudrateTolerance::ErrorPercent(percent) => { + let deviation = ((config.baudrate as i32 - actual_baud as i32).unsigned_abs() + * 100) + / actual_baud; + if deviation > percent as u32 { + return Err(ConfigError::UnachievableBaudrate); + } + } + _ => {} + } + + Ok(()) + } } impl PartialEq for Info { diff --git a/hil-test/tests/uart.rs b/hil-test/tests/uart.rs index 4753275b29..c0620a2cae 100644 --- a/hil-test/tests/uart.rs +++ b/hil-test/tests/uart.rs @@ -43,6 +43,35 @@ mod tests { assert_eq!(byte[0], 0x42); } + #[test] + fn test_different_tolerance(mut ctx: Context) { + ctx.uart + .apply_config( + &uart::Config::default() + .with_baudrate(19_200) + .with_baudrate_tolerance(uart::BaudrateTolerance::Exact), + ) + .unwrap(); + + ctx.uart.write_bytes(&[0x42]).unwrap(); + let mut byte = [0u8; 1]; + ctx.uart.read_bytes(&mut byte).unwrap(); + assert_eq!(byte[0], 0x42); + + ctx.uart + .apply_config( + &uart::Config::default() + .with_baudrate(9600) + .with_baudrate_tolerance(uart::BaudrateTolerance::ErrorPercent(10)), + ) + .unwrap(); + + ctx.uart.write_bytes(&[0x42]).unwrap(); + let mut byte = [0u8; 1]; + ctx.uart.read_bytes(&mut byte).unwrap(); + assert_eq!(byte[0], 0x42); + } + #[test] fn test_send_receive_buffer(mut ctx: Context) { const BUF_SIZE: usize = 128; // UART_FIFO_SIZE @@ -73,6 +102,8 @@ mod tests { (9600, ClockSource::RefTick), (921_600, ClockSource::Apb), (2_000_000, ClockSource::Apb), + (3_500_000, ClockSource::Apb), + (5_000_000, ClockSource::Apb), ]; let mut byte_to_write = 0xA5;