From 6bbc5ef8993982e1917aa07f3715a9e97e0fe97c Mon Sep 17 00:00:00 2001 From: Rene van der Meer <9930578+golemparts@users.noreply.github.com> Date: Fri, 17 May 2024 15:05:30 +0200 Subject: [PATCH] Fix support for hardware PWM on Pi 5 --- CHANGELOG.md | 4 +- src/pwm.rs | 102 ++++++++++++++++++++++++-------------------- src/pwm/sysfs.rs | 109 +++++++++++++++++++++++++++++------------------ src/system.rs | 26 +++++++++++ 4 files changed, 151 insertions(+), 90 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d412ece0..b6b5b385 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,8 +1,10 @@ # Changelog -## 0.17.2 (TBD) +## 0.18.0 (TBD) * **Gpio**: Fix `InputPin` mode not getting set correctly for uninitialized (FUNCSEL 31 (NULL)) pins on Raspberry Pi 5 (contributed by @lukeburong). +* **Pwm**: (Breaking change) Add `Error::UnknownModel`. +* **Pwm**: Fix support for hardware PWM on Raspberry Pi 5, caused by incorrect PWM chip/channel. ## 0.17.1 (January 21, 2024) diff --git a/src/pwm.rs b/src/pwm.rs index ea96d5e0..0f851b1c 100644 --- a/src/pwm.rs +++ b/src/pwm.rs @@ -76,6 +76,8 @@ mod hal; mod hal_unproven; mod sysfs; +use crate::system::DeviceInfo; + const NANOS_PER_SEC: f64 = 1_000_000_000.0; /// Errors that can occur when accessing the PWM peripheral. @@ -83,12 +85,24 @@ const NANOS_PER_SEC: f64 = 1_000_000_000.0; pub enum Error { /// I/O error. Io(io::Error), + /// Unknown model. + /// + /// The Raspberry Pi model or SoC can't be identified. Support for + /// new models is usually added shortly after they are officially + /// announced and available to the public. Make sure you're using + /// the latest release of RPPAL. + /// + /// You may also encounter this error if your Linux distribution + /// doesn't provide any of the common user-accessible system files + /// that are used to identify the model and SoC. + UnknownModel, } impl fmt::Display for Error { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match *self { Error::Io(ref err) => write!(f, "I/O error: {}", err), + Error::UnknownModel => write!(f, "Unknown Raspberry Pi model"), } } } @@ -152,7 +166,8 @@ impl fmt::Display for Polarity { /// [here]: index.html #[derive(Debug)] pub struct Pwm { - channel: Channel, + chip: u8, + channel: u8, reset_on_drop: bool, } @@ -164,9 +179,16 @@ impl Pwm { /// /// [`enable`]: #method.enable pub fn new(channel: Channel) -> Result { - sysfs::export(channel as u8)?; + // Select chip/channel based on Pi model + let device_info = DeviceInfo::new().map_err(|_| Error::UnknownModel)?; + + let chip = device_info.pwm_chip(); + let channel = device_info.pwm_channels()[channel as usize]; + + sysfs::export(chip, channel)?; let pwm = Pwm { + chip, channel, reset_on_drop: true, }; @@ -204,20 +226,10 @@ impl Pwm { polarity: Polarity, enabled: bool, ) -> Result { - sysfs::export(channel as u8)?; - - let pwm = Pwm { - channel, - reset_on_drop: true, - }; - - // Always reset "enable" to 0. The sysfs pwm interface has a bug where a previous - // export may have left "enable" as 1 after unexporting. On the next export, - // "enable" is still set to 1, even though the channel isn't enabled. - let _ = pwm.disable(); + let pwm = Pwm::new(channel)?; // Set pulse width to 0 first in case the new period is shorter than the current pulse width - let _ = sysfs::set_pulse_width(channel as u8, 0); + let _ = sysfs::set_pulse_width(pwm.chip, pwm.channel, 0); pwm.set_period(period)?; pwm.set_pulse_width(pulse_width)?; @@ -254,20 +266,10 @@ impl Pwm { polarity: Polarity, enabled: bool, ) -> Result { - sysfs::export(channel as u8)?; - - let pwm = Pwm { - channel, - reset_on_drop: true, - }; - - // Always reset "enable" to 0. The sysfs pwm interface has a bug where a previous - // export may have left "enable" as 1 after unexporting. On the next export, - // "enable" is still set to 1, even though the channel isn't enabled. - let _ = pwm.disable(); + let pwm = Pwm::new(channel)?; // Set pulse width to 0 first in case the new period is shorter than the current pulse width - let _ = sysfs::set_pulse_width(channel as u8, 0); + let _ = sysfs::set_pulse_width(pwm.chip, pwm.channel, 0); // Convert to nanoseconds let period = if frequency == 0.0 { @@ -277,8 +279,8 @@ impl Pwm { }; let pulse_width = period * duty_cycle.clamp(0.0, 1.0); - sysfs::set_period(channel as u8, period as u64)?; - sysfs::set_pulse_width(channel as u8, pulse_width as u64)?; + sysfs::set_period(pwm.chip, pwm.channel, period as u64)?; + sysfs::set_pulse_width(pwm.chip, pwm.channel, pulse_width as u64)?; pwm.set_polarity(polarity)?; if enabled { pwm.enable()?; @@ -289,7 +291,10 @@ impl Pwm { /// Returns the period. pub fn period(&self) -> Result { - Ok(Duration::from_nanos(sysfs::period(self.channel as u8)?)) + Ok(Duration::from_nanos(sysfs::period( + self.chip, + self.channel, + )?)) } /// Sets the period. @@ -299,7 +304,8 @@ impl Pwm { /// This method will fail if `period` is shorter than the current pulse width. pub fn set_period(&self, period: Duration) -> Result<()> { sysfs::set_period( - self.channel as u8, + self.chip, + self.channel, u64::from(period.subsec_nanos()) .saturating_add(period.as_secs().saturating_mul(NANOS_PER_SEC as u64)), )?; @@ -310,7 +316,8 @@ impl Pwm { /// Returns the pulse width. pub fn pulse_width(&self) -> Result { Ok(Duration::from_nanos(sysfs::pulse_width( - self.channel as u8, + self.chip, + self.channel, )?)) } @@ -322,7 +329,8 @@ impl Pwm { /// This method will fail if `pulse_width` is longer than the current period. pub fn set_pulse_width(&self, pulse_width: Duration) -> Result<()> { sysfs::set_pulse_width( - self.channel as u8, + self.chip, + self.channel, u64::from(pulse_width.subsec_nanos()) .saturating_add(pulse_width.as_secs().saturating_mul(NANOS_PER_SEC as u64)), )?; @@ -335,7 +343,7 @@ impl Pwm { /// `frequency` is a convenience method that calculates the frequency in hertz (Hz) /// based on the configured period. pub fn frequency(&self) -> Result { - let period = sysfs::period(self.channel as u8)? as f64; + let period = sysfs::period(self.chip, self.channel)? as f64; Ok(if period == 0.0 { 0.0 @@ -354,7 +362,7 @@ impl Pwm { /// `duty_cycle` is specified as a floating point value between `0.0` (0%) and `1.0` (100%). pub fn set_frequency(&self, frequency: f64, duty_cycle: f64) -> Result<()> { // Set duty cycle to 0 first in case the new period is shorter than the current duty cycle - let _ = sysfs::set_pulse_width(self.channel as u8, 0); + let _ = sysfs::set_pulse_width(self.chip, self.channel, 0); // Convert to nanoseconds let period = if frequency == 0.0 { @@ -364,8 +372,8 @@ impl Pwm { }; let pulse_width = period * duty_cycle.clamp(0.0, 1.0); - sysfs::set_period(self.channel as u8, period as u64)?; - sysfs::set_pulse_width(self.channel as u8, pulse_width as u64)?; + sysfs::set_period(self.chip, self.channel, period as u64)?; + sysfs::set_pulse_width(self.chip, self.channel, pulse_width as u64)?; Ok(()) } @@ -376,8 +384,8 @@ impl Pwm { /// floating point value between `0.0` (0%) and `1.0` (100%) based on the configured /// period and pulse width. pub fn duty_cycle(&self) -> Result { - let period = sysfs::period(self.channel as u8)? as f64; - let pulse_width = sysfs::pulse_width(self.channel as u8)? as f64; + let period = sysfs::period(self.chip, self.channel)? as f64; + let pulse_width = sysfs::pulse_width(self.chip, self.channel)? as f64; Ok(if period == 0.0 { 0.0 @@ -393,17 +401,17 @@ impl Pwm { /// /// `duty_cycle` is specified as a floating point value between `0.0` (0%) and `1.0` (100%). pub fn set_duty_cycle(&self, duty_cycle: f64) -> Result<()> { - let period = sysfs::period(self.channel as u8)? as f64; + let period = sysfs::period(self.chip, self.channel)? as f64; let pulse_width = period * duty_cycle.clamp(0.0, 1.0); - sysfs::set_pulse_width(self.channel as u8, pulse_width as u64)?; + sysfs::set_pulse_width(self.chip, self.channel, pulse_width as u64)?; Ok(()) } /// Returns the polarity. pub fn polarity(&self) -> Result { - Ok(sysfs::polarity(self.channel as u8)?) + Ok(sysfs::polarity(self.chip, self.channel)?) } /// Sets the polarity. @@ -414,26 +422,26 @@ impl Pwm { /// [`Normal`]: enum.Polarity.html#variant.Normal /// [`Inverse`]: enum.Polarity.html#variant.Inverse pub fn set_polarity(&self, polarity: Polarity) -> Result<()> { - sysfs::set_polarity(self.channel as u8, polarity)?; + sysfs::set_polarity(self.chip, self.channel, polarity)?; Ok(()) } /// Returns `true` if the PWM channel is enabled. pub fn is_enabled(&self) -> Result { - Ok(sysfs::enabled(self.channel as u8)?) + Ok(sysfs::enabled(self.chip, self.channel)?) } /// Enables the PWM channel. pub fn enable(&self) -> Result<()> { - sysfs::set_enabled(self.channel as u8, true)?; + sysfs::set_enabled(self.chip, self.channel, true)?; Ok(()) } /// Disables the PWM channel. pub fn disable(&self) -> Result<()> { - sysfs::set_enabled(self.channel as u8, false)?; + sysfs::set_enabled(self.chip, self.channel, false)?; Ok(()) } @@ -461,8 +469,8 @@ impl Pwm { impl Drop for Pwm { fn drop(&mut self) { if self.reset_on_drop { - let _ = sysfs::set_enabled(self.channel as u8, false); - let _ = sysfs::unexport(self.channel as u8); + let _ = sysfs::set_enabled(self.chip, self.channel, false); + let _ = sysfs::unexport(self.chip, self.channel); } } } diff --git a/src/pwm/sysfs.rs b/src/pwm/sysfs.rs index b37bbdc2..332323bc 100644 --- a/src/pwm/sysfs.rs +++ b/src/pwm/sysfs.rs @@ -98,10 +98,11 @@ fn check_permissions(path: &str, gid: u32) -> bool { false } -pub fn export(channel: u8) -> Result<()> { +pub fn export(chip: u8, channel: u8) -> Result<()> { // Only export if the channel isn't already exported - if !Path::new(&format!("/sys/class/pwm/pwmchip0/pwm{}", channel)).exists() { - File::create("/sys/class/pwm/pwmchip0/export")?.write_fmt(format_args!("{}", channel))?; + if !Path::new(&format!("/sys/class/pwm/pwmchip{}/pwm{}", chip, channel)).exists() { + File::create(format!("/sys/class/pwm/pwmchip{}/export", chip))? + .write_fmt(format_args!("{}", channel))?; } // If we're logged in as root or effective root, skip the permission checks @@ -126,11 +127,11 @@ pub fn export(channel: u8) -> Result<()> { }; let paths = &[ - format!("/sys/class/pwm/pwmchip0/pwm{}", channel), - format!("/sys/class/pwm/pwmchip0/pwm{}/period", channel), - format!("/sys/class/pwm/pwmchip0/pwm{}/duty_cycle", channel), - format!("/sys/class/pwm/pwmchip0/pwm{}/polarity", channel), - format!("/sys/class/pwm/pwmchip0/pwm{}/enable", channel), + format!("/sys/class/pwm/pwmchip{}/pwm{}", chip, channel), + format!("/sys/class/pwm/pwmchip{}/pwm{}/period", chip, channel), + format!("/sys/class/pwm/pwmchip{}/pwm{}/duty_cycle", chip, channel), + format!("/sys/class/pwm/pwmchip{}/pwm{}/polarity", chip, channel), + format!("/sys/class/pwm/pwmchip{}/pwm{}/enable", chip, channel), ]; let mut counter = 0; @@ -151,17 +152,21 @@ pub fn export(channel: u8) -> Result<()> { Ok(()) } -pub fn unexport(channel: u8) -> Result<()> { +pub fn unexport(chip: u8, channel: u8) -> Result<()> { // Only unexport if the channel is actually exported - if Path::new(&format!("/sys/class/pwm/pwmchip0/pwm{}", channel)).exists() { - File::create("/sys/class/pwm/pwmchip0/unexport")?.write_fmt(format_args!("{}", channel))?; + if Path::new(&format!("/sys/class/pwm/pwmchip{}/pwm{}", chip, channel)).exists() { + File::create(format!("/sys/class/pwm/pwmchip{}/unexport", chip))? + .write_fmt(format_args!("{}", channel))?; } Ok(()) } -pub fn period(channel: u8) -> Result { - let period = fs::read_to_string(format!("/sys/class/pwm/pwmchip0/pwm{}/period", channel))?; +pub fn period(chip: u8, channel: u8) -> Result { + let period = fs::read_to_string(format!( + "/sys/class/pwm/pwmchip{}/pwm{}/period", + chip, channel + ))?; if let Ok(period) = period.trim().parse() { Ok(period) } else { @@ -169,18 +174,23 @@ pub fn period(channel: u8) -> Result { } } -pub fn set_period(channel: u8, period: u64) -> Result<()> { - File::create(format!("/sys/class/pwm/pwmchip0/pwm{}/period", channel))? - .write_fmt(format_args!("{}", period))?; +pub fn set_period(chip: u8, channel: u8, period: u64) -> Result<()> { + File::create(format!( + "/sys/class/pwm/pwmchip{}/pwm{}/period", + chip, channel + ))? + .write_fmt(format_args!("{}", period))?; Ok(()) } -pub fn pulse_width(channel: u8) -> Result { +pub fn pulse_width(chip: u8, channel: u8) -> Result { // The sysfs PWM interface specifies the duty cycle in nanoseconds, which // means it's actually the pulse width. - let duty_cycle = - fs::read_to_string(format!("/sys/class/pwm/pwmchip0/pwm{}/duty_cycle", channel))?; + let duty_cycle = fs::read_to_string(format!( + "/sys/class/pwm/pwmchip{}/pwm{}/duty_cycle", + chip, channel + ))?; if let Ok(duty_cycle) = duty_cycle.trim().parse() { Ok(duty_cycle) @@ -189,17 +199,23 @@ pub fn pulse_width(channel: u8) -> Result { } } -pub fn set_pulse_width(channel: u8, pulse_width: u64) -> Result<()> { +pub fn set_pulse_width(chip: u8, channel: u8, pulse_width: u64) -> Result<()> { // The sysfs PWM interface specifies the duty cycle in nanoseconds, which // means it's actually the pulse width. - File::create(format!("/sys/class/pwm/pwmchip0/pwm{}/duty_cycle", channel))? - .write_fmt(format_args!("{}", pulse_width))?; + File::create(format!( + "/sys/class/pwm/pwmchip{}/pwm{}/duty_cycle", + chip, channel + ))? + .write_fmt(format_args!("{}", pulse_width))?; Ok(()) } -pub fn polarity(channel: u8) -> Result { - let polarity = fs::read_to_string(format!("/sys/class/pwm/pwmchip0/pwm{}/polarity", channel))?; +pub fn polarity(chip: u8, channel: u8) -> Result { + let polarity = fs::read_to_string(format!( + "/sys/class/pwm/pwmchip{}/pwm{}/polarity", + chip, channel + ))?; match polarity.trim() { "normal" => Ok(Polarity::Normal), @@ -207,20 +223,26 @@ pub fn polarity(channel: u8) -> Result { } } -pub fn set_polarity(channel: u8, polarity: Polarity) -> Result<()> { +pub fn set_polarity(chip: u8, channel: u8, polarity: Polarity) -> Result<()> { let b_polarity: &[u8] = match polarity { Polarity::Normal => b"normal", Polarity::Inverse => b"inversed", }; - File::create(format!("/sys/class/pwm/pwmchip0/pwm{}/polarity", channel))? - .write_all(b_polarity)?; + File::create(format!( + "/sys/class/pwm/pwmchip{}/pwm{}/polarity", + chip, channel + ))? + .write_all(b_polarity)?; Ok(()) } -pub fn enabled(channel: u8) -> Result { - let enabled = fs::read_to_string(format!("/sys/class/pwm/pwmchip0/pwm{}/enable", channel))?; +pub fn enabled(chip: u8, channel: u8) -> Result { + let enabled = fs::read_to_string(format!( + "/sys/class/pwm/pwmchip{}/pwm{}/enable", + chip, channel + ))?; match enabled.trim() { "0" => Ok(false), @@ -228,19 +250,22 @@ pub fn enabled(channel: u8) -> Result { } } -pub fn set_enabled(channel: u8, enabled: bool) -> Result<()> { - File::create(format!("/sys/class/pwm/pwmchip0/pwm{}/enable", channel))? - .write_fmt(format_args!("{}", enabled as u8)) - .map_err(|e| { - if e.kind() == io::ErrorKind::InvalidInput { - io::Error::new( - io::ErrorKind::InvalidInput, - "Make sure you have set either a period or frequency before enabling PWM", - ) - } else { - e - } - })?; +pub fn set_enabled(chip: u8, channel: u8, enabled: bool) -> Result<()> { + File::create(format!( + "/sys/class/pwm/pwmchip{}/pwm{}/enable", + chip, channel + ))? + .write_fmt(format_args!("{}", enabled as u8)) + .map_err(|e| { + if e.kind() == io::ErrorKind::InvalidInput { + io::Error::new( + io::ErrorKind::InvalidInput, + "Make sure you have set either a period or frequency before enabling PWM", + ) + } else { + e + } + })?; Ok(()) } diff --git a/src/system.rs b/src/system.rs index 0a170d1b..c7a4146d 100644 --- a/src/system.rs +++ b/src/system.rs @@ -331,6 +331,10 @@ pub struct DeviceInfo { gpio_lines: u8, // GPIO interface through the Broadcom SoC or a separate RP1 gpio_interface: GpioInterface, + // PWM chip # used for hardware PWM on selected GPIO pins + pwm_chip: u8, + // PWM channels used for hardware PWM on selected GPIO pins + pwm_channels: [u8; 2], } impl DeviceInfo { @@ -360,6 +364,8 @@ impl DeviceInfo { gpio_offset: GPIO_OFFSET, gpio_lines: GPIO_LINES_BCM283X, gpio_interface: GpioInterface::Bcm, + pwm_chip: 0, + pwm_channels: [0, 1], }), Model::RaspberryPi2B => Ok(DeviceInfo { model, @@ -368,6 +374,8 @@ impl DeviceInfo { gpio_offset: GPIO_OFFSET, gpio_lines: GPIO_LINES_BCM283X, gpio_interface: GpioInterface::Bcm, + pwm_chip: 0, + pwm_channels: [0, 1], }), Model::RaspberryPi3B | Model::RaspberryPiComputeModule3 | Model::RaspberryPiZero2W => { Ok(DeviceInfo { @@ -377,6 +385,8 @@ impl DeviceInfo { gpio_offset: GPIO_OFFSET, gpio_lines: GPIO_LINES_BCM283X, gpio_interface: GpioInterface::Bcm, + pwm_chip: 0, + pwm_channels: [0, 1], }) } Model::RaspberryPi3BPlus @@ -388,6 +398,8 @@ impl DeviceInfo { gpio_offset: GPIO_OFFSET, gpio_lines: GPIO_LINES_BCM283X, gpio_interface: GpioInterface::Bcm, + pwm_chip: 0, + pwm_channels: [0, 1], }), Model::RaspberryPi4B | Model::RaspberryPi400 @@ -399,6 +411,8 @@ impl DeviceInfo { gpio_offset: GPIO_OFFSET, gpio_lines: GPIO_LINES_BCM2711, gpio_interface: GpioInterface::Bcm, + pwm_chip: 0, + pwm_channels: [0, 1], }), Model::RaspberryPi5 => Ok(DeviceInfo { model, @@ -407,6 +421,8 @@ impl DeviceInfo { gpio_offset: GPIO_OFFSET_RP1, gpio_lines: GPIO_LINES_RP1, gpio_interface: GpioInterface::Rp1, + pwm_chip: 2, + pwm_channels: [2, 3], }), } } @@ -440,4 +456,14 @@ impl DeviceInfo { pub(crate) fn gpio_interface(&self) -> GpioInterface { self.gpio_interface } + + /// Returns the PWM chip # used for hardware PWM. + pub(crate) fn pwm_chip(&self) -> u8 { + self.pwm_chip + } + + /// Returns the PWM channels used for hardware PWM. + pub(crate) fn pwm_channels(&self) -> [u8; 2] { + self.pwm_channels + } }