| Message ID | 6a94249d2d69bc5c1907fea9ed80c9d7e34278aa.1665744170.git.viresh.kumar@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | libgpiod: Add Rust bindings | expand |
On Fri, Oct 14, 2022 at 04:17:20PM +0530, Viresh Kumar wrote: > Add rust wrapper crate, around the libpiod-sys crate added earlier, to > provide a convenient interface for the users. > > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > --- > bindings/rust/Cargo.toml | 1 + > bindings/rust/libgpiod/Cargo.toml | 13 + > bindings/rust/libgpiod/src/chip.rs | 253 ++++++++++ > bindings/rust/libgpiod/src/edge_event.rs | 101 ++++ > bindings/rust/libgpiod/src/event_buffer.rs | 66 +++ > bindings/rust/libgpiod/src/info_event.rs | 68 +++ > bindings/rust/libgpiod/src/lib.rs | 471 +++++++++++++++++++ > bindings/rust/libgpiod/src/line_config.rs | 118 +++++ > bindings/rust/libgpiod/src/line_info.rs | 180 +++++++ > bindings/rust/libgpiod/src/line_request.rs | 246 ++++++++++ > bindings/rust/libgpiod/src/line_settings.rs | 277 +++++++++++ > bindings/rust/libgpiod/src/request_config.rs | 96 ++++ > 12 files changed, 1890 insertions(+) > create mode 100644 bindings/rust/libgpiod/Cargo.toml > create mode 100644 bindings/rust/libgpiod/src/chip.rs > create mode 100644 bindings/rust/libgpiod/src/edge_event.rs > create mode 100644 bindings/rust/libgpiod/src/event_buffer.rs > create mode 100644 bindings/rust/libgpiod/src/info_event.rs > create mode 100644 bindings/rust/libgpiod/src/lib.rs > create mode 100644 bindings/rust/libgpiod/src/line_config.rs > create mode 100644 bindings/rust/libgpiod/src/line_info.rs > create mode 100644 bindings/rust/libgpiod/src/line_request.rs > create mode 100644 bindings/rust/libgpiod/src/line_settings.rs > create mode 100644 bindings/rust/libgpiod/src/request_config.rs > > diff --git a/bindings/rust/Cargo.toml b/bindings/rust/Cargo.toml > index d0b3a3c88ff1..1e57ef2c0002 100644 > --- a/bindings/rust/Cargo.toml > +++ b/bindings/rust/Cargo.toml > @@ -2,4 +2,5 @@ > > members = [ > "libgpiod-sys", > + "libgpiod" > ] > diff --git a/bindings/rust/libgpiod/Cargo.toml b/bindings/rust/libgpiod/Cargo.toml > new file mode 100644 > index 000000000000..f25242abb153 > --- /dev/null > +++ b/bindings/rust/libgpiod/Cargo.toml > @@ -0,0 +1,13 @@ > +[package] > +name = "libgpiod" > +version = "0.1.0" > +edition = "2018" Add license and other relevant keys as per the link. > + > +# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html > + > +[dependencies] > +intmap = "2.0.0" > +libc = ">=0.2.39" > +libgpiod-sys = { path = "../libgpiod-sys" } > +thiserror = "1.0" > +vmm-sys-util = "=0.10.0" > diff --git a/bindings/rust/libgpiod/src/chip.rs b/bindings/rust/libgpiod/src/chip.rs > new file mode 100644 > index 000000000000..4f52c3f141f4 > --- /dev/null > +++ b/bindings/rust/libgpiod/src/chip.rs > @@ -0,0 +1,253 @@ > +// SPDX-License-Identifier: Apache-2.0 AND BSD-3-Clause > +// > +// Copyright 2022 Linaro Ltd. All Rights Reserved. > +// Viresh Kumar <viresh.kumar@linaro.org> > + > +use libc::strlen; Avoid importing function names. Idiomatically, modules and types are ok, but not functions. So says the Rust book. > +use std::os::raw::c_char; > +use std::path::Path; > +use std::sync::Arc; > +use std::time::Duration; > +use std::{slice, str}; > + > +use vmm_sys_util::errno::Error as Errno; Why vmm_sys_util::errno::Error rather than, say, errno::Errno? > + > +use super::{gpiod, info, line, request, Error, Offset, OperationType, Result}; > + > +#[derive(Clone, Debug, Eq, PartialEq)] > +pub(crate) struct Internal { > + chip: *mut gpiod::gpiod_chip, > +} > + Derived implmentation of Clone on an object containing a raw pointer and a Drop that frees that pointer? That wont end well. If you have an Arc managing references to it below, why the need for Clone? Types that wrap C types that should impl Clone have a C _copy function, and require an explicit impl that calls it, not a derived. > +impl Internal { > + /// Find a chip by path. > + pub(crate) fn open<P: AsRef<Path>>(path: &P) -> Result<Self> { > + // Null-terminate the string > + let path = path.as_ref().to_string_lossy() + "\0"; > + > + let chip = unsafe { gpiod::gpiod_chip_open(path.as_ptr() as *const c_char) }; All unsafes should have a preceding SAFETY comment. > + if chip.is_null() { > + return Err(Error::OperationFailed( > + OperationType::ChipOpen, > + Errno::last(), > + )); > + } > + > + Ok(Self { chip }) > + } > + > + /// Private helper, Returns gpiod_chip > + pub(crate) fn chip(&self) -> *mut gpiod::gpiod_chip { > + self.chip > + } Just make self.chip itself pub(crate)? > +} > + > +impl Drop for Internal { > + /// Close the chip and release all associated resources. > + fn drop(&mut self) { > + unsafe { gpiod::gpiod_chip_close(self.chip) } > + } > +} > + > +/// GPIO chip > +/// > +/// A GPIO chip object is associated with an open file descriptor to the GPIO > +/// character device. It exposes basic information about the chip and allows > +/// callers to retrieve information about each line, watch lines for state > +/// changes and make line requests. > +#[derive(Clone, Debug, Eq, PartialEq)] > +pub struct Chip { > + ichip: Arc<Internal>, > + info: Info, > +} > + Doesn't info need an Arc as well? (see struct Info below). Actually, does it make sense to be able to Clone the Chip? Two threads could then watch different lines and wait_info_event() from the one chip. That isn't going to work, at least not the way you would want. > +unsafe impl Send for Chip {} > +unsafe impl Sync for Chip {} > + Send makes sense to me, but not Sync, due to the waiting for info events. Similarly for line_request::Request. At least not without some additional synchronisation/locking, and I'm not sure we want to go there. > +impl Chip { > + /// Find a chip by path. > + pub fn open<P: AsRef<Path>>(path: &P) -> Result<Self> { > + let ichip = Arc::new(Internal::open(path)?); > + let info = Info::new(ichip.clone())?; > + info is not always necessary, e.g. if the user just want to request a line. Maybe lazy load it? Assuming you want to keep a copy cached with the Chip at all. > + Ok(Self { ichip, info }) > + } > + > + /// Get the chip name as represented in the kernel. > + pub fn name(&self) -> Result<&str> { > + self.info.name() > + } > + > + /// Get the chip label as represented in the kernel. > + pub fn label(&self) -> Result<&str> { > + self.info.label() > + } > + > + /// Get the number of GPIO lines exposed by the chip. > + pub fn num_lines(&self) -> usize { > + self.info.num_lines() > + } > + > + /// Get the path used to find the chip. > + pub fn path(&self) -> Result<&str> { > + // SAFETY: The string returned by libgpiod is guaranteed to live as long > + // as the `struct Chip`. > + let path = unsafe { gpiod::gpiod_chip_get_path(self.ichip.chip()) }; > + > + // SAFETY: The string is guaranteed to be valid here by the C API. > + str::from_utf8(unsafe { slice::from_raw_parts(path as *const u8, strlen(path) as usize) }) > + .map_err(Error::StringNotUtf8) > + } > + > + /// Get information about the chip. > + pub fn info(&self) -> Result<Info> { > + Info::new(self.ichip.clone()) > + } You already wrap all the methods of Info, so why the need to return a copy as well? Do you really need both? And the functions of Info are currently pub(crate) so what could they do with it anyway? > + > + /// Get a snapshot of information about the line. > + pub fn line_info(&self, offset: Offset) -> Result<line::Info> { > + line::Info::new(self.ichip.clone(), offset) > + } > + > + /// Get the current snapshot of information about the line at given offset and start watching > + /// it for future changes. > + pub fn watch_line_info(&self, offset: Offset) -> Result<line::Info> { > + line::Info::new_watch(self.ichip.clone(), offset) > + } > + > + /// Stop watching a line > + pub fn unwatch(&self, offset: Offset) { > + unsafe { > + gpiod::gpiod_chip_unwatch_line_info(self.ichip.chip(), offset); > + } > + } > + > + /// Get the file descriptor associated with the chip. > + /// > + /// The returned file descriptor must not be closed by the caller, else other methods for the > + /// `struct Chip` may fail. > + pub fn fd(&self) -> Result<u32> { > + let fd = unsafe { gpiod::gpiod_chip_get_fd(self.ichip.chip()) }; > + > + if fd < 0 { > + Err(Error::OperationFailed( > + OperationType::ChipGetFd, > + Errno::last(), > + )) > + } else { > + Ok(fd as u32) > + } > + } > + > + /// Wait for line status events on any of the watched lines on the chip. > + pub fn wait_info_event(&self, timeout: Option<Duration>) -> Result<bool> { > + let timeout = match timeout { > + Some(x) => x.as_nanos() as i64, > + // Block indefinitely > + None => -1, > + }; > + > + let ret = unsafe { gpiod::gpiod_chip_wait_info_event(self.ichip.chip(), timeout) }; > + > + match ret { > + -1 => Err(Error::OperationFailed( > + OperationType::ChipWaitInfoEvent, > + Errno::last(), > + )), > + 0 => Ok(false), > + _ => Ok(true), > + } > + } > + > + /// Read a single line status change event from the chip. If no events are > + /// pending, this function will block. > + pub fn read_info_event(&self) -> Result<info::Event> { > + info::Event::new(&self.ichip) > + } > + > + /// Map a GPIO line's name to its offset within the chip. > + pub fn line_offset_from_name(&self, name: &str) -> Result<Offset> { > + // Null-terminate the string > + let name = name.to_owned() + "\0"; > + > + let ret = unsafe { > + gpiod::gpiod_chip_get_line_offset_from_name( > + self.ichip.chip(), > + name.as_ptr() as *const c_char, > + ) > + }; > + > + if ret == -1 { > + Err(Error::OperationFailed( > + OperationType::ChipGetLine, > + Errno::last(), > + )) The OperationType identifies the underlying C call that caused the error? So it should be ChipGetLineOffsetFromName? > + } else { > + Ok(ret as u32) > + } > + } > + > + /// Request a set of lines for exclusive usage. > + pub fn request_lines( > + &self, > + rconfig: &request::Config, > + lconfig: &line::Config, > + ) -> Result<line::Request> { > + line::Request::new(&self.ichip, rconfig, lconfig) > + } > +} > + > +/// GPIO chip Information > +#[derive(Clone, Debug, Eq, PartialEq)] > +pub struct Info { > + info: *mut gpiod::gpiod_chip_info, > +} > + Does struct Info even need to be pub? Clone/Drop wrapping a raw pointer again. > +impl Info { > + /// Find a GPIO chip by path. > + pub(crate) fn new(chip: Arc<Internal>) -> Result<Self> { > + let info = unsafe { gpiod::gpiod_chip_get_info(chip.chip()) }; > + if info.is_null() { > + return Err(Error::OperationFailed( > + OperationType::ChipInfoGet, > + Errno::last(), > + )); > + } > + > + Ok(Self { info }) > + } Do these functions need to be pub(crate)? They are only used within the module (by Chip), and are visible to that anyway. > + > + /// Get the GPIO chip name as represented in the kernel. > + pub(crate) fn name(&self) -> Result<&str> { > + // SAFETY: The string returned by libgpiod is guaranteed to live as long > + // as the `struct Chip`. > + let name = unsafe { gpiod::gpiod_chip_info_get_name(self.info) }; > + > + // SAFETY: The string is guaranteed to be valid here by the C API. > + str::from_utf8(unsafe { slice::from_raw_parts(name as *const u8, strlen(name) as usize) }) > + .map_err(Error::StringNotUtf8) > + } > + > + /// Get the GPIO chip label as represented in the kernel. > + pub(crate) fn label(&self) -> Result<&str> { > + // SAFETY: The string returned by libgpiod is guaranteed to live as long > + // as the `struct Chip`. > + let label = unsafe { gpiod::gpiod_chip_info_get_label(self.info) }; > + > + // SAFETY: The string is guaranteed to be valid here by the C API. > + str::from_utf8(unsafe { slice::from_raw_parts(label as *const u8, strlen(label) as usize) }) > + .map_err(Error::StringNotUtf8) > + } > + > + /// Get the number of GPIO lines exposed by the chip. > + pub(crate) fn num_lines(&self) -> usize { > + unsafe { gpiod::gpiod_chip_info_get_num_lines(self.info) as usize } > + } > +} > + > +impl Drop for Info { > + /// Close the GPIO chip info and release all associated resources. > + fn drop(&mut self) { > + unsafe { gpiod::gpiod_chip_info_free(self.info) } > + } > +} > diff --git a/bindings/rust/libgpiod/src/edge_event.rs b/bindings/rust/libgpiod/src/edge_event.rs > new file mode 100644 > index 000000000000..12c0fd73f778 > --- /dev/null > +++ b/bindings/rust/libgpiod/src/edge_event.rs > @@ -0,0 +1,101 @@ > +// SPDX-License-Identifier: Apache-2.0 AND BSD-3-Clause > +// > +// Copyright 2022 Linaro Ltd. All Rights Reserved. > +// Viresh Kumar <viresh.kumar@linaro.org> > + > +use std::time::Duration; > + > +use vmm_sys_util::errno::Error as Errno; > + > +use super::{edge::event::Buffer, gpiod, EdgeKind, Error, Offset, OperationType, Result}; > + > +/// Line edge events handling > +/// > +/// An edge event object contains information about a single line edge event. > +/// It contains the event type, timestamp and the offset of the line on which > +/// the event occurred as well as two sequence numbers (global for all lines > +/// in the associated request and local for this line only). > +/// > +/// Edge events are stored into an edge-event buffer object to improve > +/// performance and to limit the number of memory allocations when a large > +/// number of events are being read. > + > +#[derive(Debug, Eq, PartialEq)] > +pub struct Event<'b> { > + buffer: Option<&'b Buffer>, > + event: *mut gpiod::gpiod_edge_event, > +} > + > +impl<'b> Event<'b> { > + /// Get an event stored in the buffer. > + pub(crate) fn new(buffer: &'b Buffer, index: u64) -> Result<Self> { > + let event = unsafe { gpiod::gpiod_edge_event_buffer_get_event(buffer.buffer(), index) }; > + if event.is_null() { > + return Err(Error::OperationFailed( > + OperationType::EdgeEventBufferGetEvent, > + Errno::last(), > + )); > + } > + > + Ok(Self { > + buffer: Some(buffer), > + event, > + }) > + } > + > + pub fn event_clone(event: &Event) -> Result<Self> { > + let event = unsafe { gpiod::gpiod_edge_event_copy(event.event) }; > + if event.is_null() { > + return Err(Error::OperationFailed( > + OperationType::EdgeEventCopy, > + Errno::last(), > + )); > + } > + > + Ok(Self { > + buffer: None, > + event, > + }) > + } > + > + /// Get the event type. > + pub fn event_type(&self) -> Result<EdgeKind> { > + EdgeKind::new(unsafe { gpiod::gpiod_edge_event_get_event_type(self.event) } as u32) > + } > + > + /// Get the timestamp of the event. > + pub fn timestamp(&self) -> Duration { > + Duration::from_nanos(unsafe { gpiod::gpiod_edge_event_get_timestamp_ns(self.event) }) > + } > + > + /// Get the offset of the line on which the event was triggered. > + pub fn line_offset(&self) -> Offset { > + unsafe { gpiod::gpiod_edge_event_get_line_offset(self.event) } > + } > + > + /// Get the global sequence number of the event. > + /// > + /// Returns sequence number of the event relative to all lines in the > + /// associated line request. > + pub fn global_seqno(&self) -> u64 { > + unsafe { gpiod::gpiod_edge_event_get_global_seqno(self.event) } > + } > + > + /// Get the event sequence number specific to concerned line. > + /// > + /// Returns sequence number of the event relative to the line within the > + /// lifetime of the associated line request. > + pub fn line_seqno(&self) -> u64 { > + unsafe { gpiod::gpiod_edge_event_get_line_seqno(self.event) } > + } > +} > + > +impl<'b> Drop for Event<'b> { > + /// Free the edge event. > + fn drop(&mut self) { > + // Free the event only if a copy is made > + if self.buffer.is_none() { > + unsafe { gpiod::gpiod_edge_event_free(self.event) }; > + } > + } > +} > diff --git a/bindings/rust/libgpiod/src/event_buffer.rs b/bindings/rust/libgpiod/src/event_buffer.rs > new file mode 100644 > index 000000000000..11c8b5e1d7c9 > --- /dev/null > +++ b/bindings/rust/libgpiod/src/event_buffer.rs > @@ -0,0 +1,66 @@ > +// SPDX-License-Identifier: Apache-2.0 AND BSD-3-Clause > +// > +// Copyright 2022 Linaro Ltd. All Rights Reserved. > +// Viresh Kumar <viresh.kumar@linaro.org> > + > +use std::os::raw::c_ulong; > + > +use vmm_sys_util::errno::Error as Errno; > + > +use super::{edge, gpiod, Error, OperationType, Result}; > + > +/// Line edge events buffer > +#[derive(Clone, Debug, Eq, PartialEq)] > +pub struct Buffer { > + buffer: *mut gpiod::gpiod_edge_event_buffer, > +} Clone/Drop wrapping a raw pointer again. > + > +impl Buffer { > + /// Create a new edge event buffer. > + /// > + /// If capacity equals 0, it will be set to a default value of 64. If > + /// capacity is larger than 1024, it will be limited to 1024. > + pub fn new(capacity: usize) -> Result<Self> { > + let buffer = unsafe { gpiod::gpiod_edge_event_buffer_new(capacity as c_ulong) }; > + if buffer.is_null() { > + return Err(Error::OperationFailed( > + OperationType::EdgeEventBufferNew, > + Errno::last(), > + )); > + } > + > + Ok(Self { buffer }) > + } > + > + /// Private helper, Returns gpiod_edge_event_buffer > + pub(crate) fn buffer(&self) -> *mut gpiod::gpiod_edge_event_buffer { > + self.buffer > + } Just make self.buffer pub(crate)? Why is the idea of passing a raw pointer around making my skin crawl? Or even better, move the guts of line_request::Request::read_edge_events() into a helper function here that it can call so it doesn't need access to self.buffer. > + > + /// Get the capacity of the event buffer. > + pub fn capacity(&self) -> usize { > + unsafe { gpiod::gpiod_edge_event_buffer_get_capacity(self.buffer()) as usize } > + } > + > + /// Read an event stored in the buffer. > + pub fn event(&self, index: u64) -> Result<edge::Event> { > + edge::Event::new(self, index) > + } What is to prevent reading an event from a slot that has not yet been populated? I realise doing that is possible in C, but it should be prevented in the Rust bindings. > + > + /// Get the number of events the buffer contains. > + pub fn len(&self) -> usize { > + unsafe { gpiod::gpiod_edge_event_buffer_get_num_events(self.buffer()) as usize } > + } > + > + /// Check if buffer is empty. > + pub fn is_empty(&self) -> bool { > + self.len() == 0 > + } > +} > + > +impl Drop for Buffer { > + /// Free the edge event buffer and release all associated resources. > + fn drop(&mut self) { > + unsafe { gpiod::gpiod_edge_event_buffer_free(self.buffer) }; > + } > +} > diff --git a/bindings/rust/libgpiod/src/info_event.rs b/bindings/rust/libgpiod/src/info_event.rs > new file mode 100644 > index 000000000000..d8be87df6679 > --- /dev/null > +++ b/bindings/rust/libgpiod/src/info_event.rs > @@ -0,0 +1,68 @@ > +// SPDX-License-Identifier: Apache-2.0 AND BSD-3-Clause > +// > +// Copyright 2022 Linaro Ltd. All Rights Reserved. > +// Viresh Kumar <viresh.kumar@linaro.org> > + > +use std::sync::Arc; > +use std::time::Duration; > + > +use vmm_sys_util::errno::Error as Errno; > + > +use super::{chip, gpiod, line, Error, InfoChangeKind, OperationType, Result}; > + > +/// Line status watch events > +/// > +/// Accessors for the info event objects allowing to monitor changes in GPIO > +/// line state. > +/// > +/// Callers can be notified about changes in line's state using the interfaces > +/// exposed by GPIO chips. Each info event contains information about the event > +/// itself (timestamp, type) as well as a snapshot of line's state in the form > +/// of a line-info object. > + > +#[derive(Clone, Debug, Eq, PartialEq)] > +pub struct Event { > + event: *mut gpiod::gpiod_info_event, > +} > + Clone/Drop wrapping a raw pointer again. > +impl Event { > + /// Get a single chip's line's status change event. > + pub(crate) fn new(ichip: &Arc<chip::Internal>) -> Result<Self> { > + let event = unsafe { gpiod::gpiod_chip_read_info_event(ichip.chip()) }; > + if event.is_null() { > + return Err(Error::OperationFailed( > + OperationType::ChipReadInfoEvent, > + Errno::last(), > + )); > + } > + > + Ok(Self { event }) > + } The new() should be: pub(crate) fn new(event: *mut gpiod::gpiod_info_event) -> Self { Self {event} } The guts should be in Chip - where this new() is called. > + > + /// Private helper, Returns gpiod_info_event > + pub(crate) fn event(&self) -> *mut gpiod::gpiod_info_event { > + self.event > + } > + > + /// Get the event type of the status change event. > + pub fn event_type(&self) -> Result<InfoChangeKind> { > + InfoChangeKind::new(unsafe { gpiod::gpiod_info_event_get_event_type(self.event) } as u32) > + } > + > + /// Get the timestamp of the event, read from the monotonic clock. > + pub fn timestamp(&self) -> Duration { > + Duration::from_nanos(unsafe { gpiod::gpiod_info_event_get_timestamp_ns(self.event) }) > + } > + > + /// Get the line-info object associated with the event. > + pub fn line_info(&self) -> Result<line::Info> { > + line::Info::new_from_event(self) > + } > +} > + > +impl Drop for Event { > + /// Free the info event object and release all associated resources. > + fn drop(&mut self) { > + unsafe { gpiod::gpiod_info_event_free(self.event) } > + } > +} > diff --git a/bindings/rust/libgpiod/src/lib.rs b/bindings/rust/libgpiod/src/lib.rs > new file mode 100644 > index 000000000000..c5d974a9f4ea > --- /dev/null > +++ b/bindings/rust/libgpiod/src/lib.rs > @@ -0,0 +1,471 @@ > +// SPDX-License-Identifier: Apache-2.0 AND BSD-3-Clause > +// > +// Rust wrappers for GPIOD APIs > +// > +// Copyright 2022 Linaro Ltd. All Rights Reserved. > +// Viresh Kumar <viresh.kumar@linaro.org> > + > +//! libgpiod public API > +//! > +//! This is the complete documentation of the public Rust API made available to > +//! users of libgpiod. > +//! > +//! The API is logically split into several parts such as: GPIO chip & line > +//! operators, GPIO events handling etc. > + > +/// GPIO chip related definitions. > +pub mod chip; > + > +mod edge_event; > +mod event_buffer; > + > +/// GPIO chip edge event related definitions. ^ line > +pub mod edge { > + pub use crate::edge_event::*; > + > + /// GPIO chip edge event buffer related definitions. > + pub mod event { > + pub use crate::event_buffer::*; > + } > +} > + Not sure why event_buffer needs to be nested inside an event module. event_buffer should be in the request module. edge_event is a tougher call, either line or request. Probably the latter. > +mod info_event; > + > +/// GPIO chip info event related definitions. > +pub mod info { > + pub use crate::info_event::*; > +} > + Why is this a top level module, not part of chip? > +mod line_config; > +mod line_info; > +mod line_request; > +mod line_settings; > + > +/// GPIO chip line related definitions. > +pub mod line { > + pub use crate::line_config::*; > + pub use crate::line_info::*; > + pub use crate::line_request::*; > + pub use crate::line_settings::*; > +} The line_request should be in the request module? > + > +mod request_config; > + > +/// GPIO chip request related definitions. > +pub mod request { > + pub use crate::request_config::*; > +} > + > +use libgpiod_sys as gpiod; > + > +use intmap::IntMap; > +use libc::strlen; > +use std::fs; > +use std::os::raw::c_char; > +use std::path::Path; > +use std::time::Duration; > +use std::{fmt, slice, str}; > + > +use thiserror::Error as ThisError; > +use vmm_sys_util::errno::Error as Errno; > + > +/// Operation types, used with OperationFailed() Error. > +#[derive(Copy, Clone, Debug, Eq, PartialEq)] > +pub enum OperationType { > + ChipOpen, > + ChipGetFd, > + ChipWaitInfoEvent, > + ChipGetLine, > + ChipGetLineInfo, > + ChipInfoGet, > + ChipReadInfoEvent, > + ChipWatchLineInfo, > + EdgeEventBufferGetEvent, > + EdgeEventCopy, > + EdgeEventBufferNew, > + InfoEventGetLineInfo, > + LineConfigNew, > + LineConfigAddSettings, > + LineConfigGetOffsets, > + LineConfigGetSetting, > + LineRequest, > + LineRequestReconfigLines, > + LineRequestGetVal, > + LineRequestGetValSubset, > + LineRequestSetVal, > + LineRequestSetValSubset, > + LineRequestReadEdgeEvent, > + LineRequestWaitEdgeEvent, > + LineSettingsNew, > + LineSettingsCopy, > + LineSettingsGetOutVal, > + LineSettingsSetDirection, > + LineSettingsSetEdgeDetection, > + LineSettingsSetBias, > + LineSettingsSetDrive, > + LineSettingsSetActiveLow, > + LineSettingsSetDebouncePeriod, > + LineSettingsSetEventClock, > + LineSettingsSetOutputValue, > + RequestConfigNew, > + RequestConfigGetConsumer, > + SimBankGetVal, > + SimBankNew, > + SimBankSetLabel, > + SimBankSetNumLines, > + SimBankSetLineName, > + SimBankSetPull, > + SimBankHogLine, > + SimCtxNew, > + SimDevNew, > + SimDevEnable, > + SimDevDisable, > +} > + > +impl fmt::Display for OperationType { > + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { > + write!(f, "{:?}", self) > + } > +} > + > +/// Result of libgpiod operations. > +pub type Result<T> = std::result::Result<T, Error>; > + > +/// Error codes for libgpiod operations. > +#[derive(Copy, Clone, Debug, PartialEq, ThisError)] > +pub enum Error { > + #[error("Failed to get {0}")] > + NullString(&'static str), > + #[error("String not utf8: {0:?}")] > + StringNotUtf8(str::Utf8Error), > + #[error("Invalid enum {0} value: {1}")] > + InvalidEnumValue(&'static str, u32), > + #[error("Operation {0} Failed: {1}")] > + OperationFailed(OperationType, Errno), > + #[error("Invalid Arguments")] > + InvalidArguments, > + #[error("Std Io Error")] > + IoError, > +} > + The remaining types in this file look specific to lines and should be in the line module? > +/// Value settings. > +#[derive(Copy, Clone, Debug, Eq, PartialEq)] > +pub enum Value { > + /// Active > + Active, > + /// Inactive > + InActive, > +} > + > +/// Maps offset to Value. > +pub type ValueMap = IntMap<Value>; > + > +impl Value { > + pub fn new(val: i32) -> Result<Self> { > + match val { > + 0 => Ok(Value::InActive), > + 1 => Ok(Value::Active), > + _ => Err(Error::InvalidEnumValue("Value", val as u32)), > + } > + } Move the Ok outside the match, and return the Err immediately. Not a biggy here, but where there are more variants it makes it easier to read, if nothing else. > + > + fn value(&self) -> i32 { > + match self { > + Value::Active => 1, > + Value::InActive => 0, > + } > + } > +} > + > +/// Offset type. > +pub type Offset = u32; > + > +/// Direction settings. > +#[derive(Copy, Clone, Debug, Eq, PartialEq)] > +pub enum Direction { > + /// Request the line(s), but don't change direction. > + AsIs, > + /// Direction is input - for reading the value of an externally driven GPIO line. > + Input, > + /// Direction is output - for driving the GPIO line. > + Output, > +} > + > +impl Direction { > + fn new(dir: u32) -> Result<Self> { > + match dir { > + gpiod::GPIOD_LINE_DIRECTION_AS_IS => Ok(Direction::AsIs), > + gpiod::GPIOD_LINE_DIRECTION_INPUT => Ok(Direction::Input), > + gpiod::GPIOD_LINE_DIRECTION_OUTPUT => Ok(Direction::Output), > + _ => Err(Error::InvalidEnumValue("Direction", dir)), > + } > + } > + > + fn gpiod_direction(&self) -> u32 { > + match self { > + Direction::AsIs => gpiod::GPIOD_LINE_DIRECTION_AS_IS, > + Direction::Input => gpiod::GPIOD_LINE_DIRECTION_INPUT, > + Direction::Output => gpiod::GPIOD_LINE_DIRECTION_OUTPUT, > + } > + } > +} > + > +/// Internal bias settings. > +#[derive(Copy, Clone, Debug, Eq, PartialEq)] > +pub enum Bias { > + /// The internal bias is disabled. > + Disabled, > + /// The internal pull-up bias is enabled. > + PullUp, > + /// The internal pull-down bias is enabled. > + PullDown, > +} > + > +impl Bias { > + fn new(bias: u32) -> Result<Option<Self>> { > + match bias { > + gpiod::GPIOD_LINE_BIAS_UNKNOWN => Ok(None), > + gpiod::GPIOD_LINE_BIAS_AS_IS => Ok(None), > + gpiod::GPIOD_LINE_BIAS_DISABLED => Ok(Some(Bias::Disabled)), > + gpiod::GPIOD_LINE_BIAS_PULL_UP => Ok(Some(Bias::PullUp)), > + gpiod::GPIOD_LINE_BIAS_PULL_DOWN => Ok(Some(Bias::PullDown)), > + _ => Err(Error::InvalidEnumValue("Bias", bias)), > + } > + } > + > + fn gpiod_bias(bias: Option<Bias>) -> u32 { > + match bias { > + None => gpiod::GPIOD_LINE_BIAS_AS_IS, > + Some(bias) => match bias { > + Bias::Disabled => gpiod::GPIOD_LINE_BIAS_DISABLED, > + Bias::PullUp => gpiod::GPIOD_LINE_BIAS_PULL_UP, > + Bias::PullDown => gpiod::GPIOD_LINE_BIAS_PULL_DOWN, > + }, > + } > + } > +} > + > +/// Drive settings. > +#[derive(Copy, Clone, Debug, Eq, PartialEq)] > +pub enum Drive { > + /// Drive setting is push-pull. > + PushPull, > + /// Line output is open-drain. > + OpenDrain, > + /// Line output is open-source. > + OpenSource, > +} > + > +impl Drive { > + fn new(drive: u32) -> Result<Self> { > + match drive { > + gpiod::GPIOD_LINE_DRIVE_PUSH_PULL => Ok(Drive::PushPull), > + gpiod::GPIOD_LINE_DRIVE_OPEN_DRAIN => Ok(Drive::OpenDrain), > + gpiod::GPIOD_LINE_DRIVE_OPEN_SOURCE => Ok(Drive::OpenSource), > + _ => Err(Error::InvalidEnumValue("Drive", drive)), > + } > + } > + > + fn gpiod_drive(&self) -> u32 { > + match self { > + Drive::PushPull => gpiod::GPIOD_LINE_DRIVE_PUSH_PULL, > + Drive::OpenDrain => gpiod::GPIOD_LINE_DRIVE_OPEN_DRAIN, > + Drive::OpenSource => gpiod::GPIOD_LINE_DRIVE_OPEN_SOURCE, > + } > + } > +} > + > +/// Edge detection settings. > +#[derive(Copy, Clone, Debug, Eq, PartialEq)] > +pub enum Edge { > + /// Line detects rising edge events. > + Rising, > + /// Line detects falling edge events. > + Falling, > + /// Line detects both rising and falling edge events. > + Both, > +} > + > +impl Edge { > + fn new(edge: u32) -> Result<Option<Self>> { > + match edge { > + gpiod::GPIOD_LINE_EDGE_NONE => Ok(None), > + gpiod::GPIOD_LINE_EDGE_RISING => Ok(Some(Edge::Rising)), > + gpiod::GPIOD_LINE_EDGE_FALLING => Ok(Some(Edge::Falling)), > + gpiod::GPIOD_LINE_EDGE_BOTH => Ok(Some(Edge::Both)), > + _ => Err(Error::InvalidEnumValue("Edge", edge)), > + } > + } > + > + fn gpiod_edge(edge: Option<Edge>) -> u32 { > + match edge { > + None => gpiod::GPIOD_LINE_EDGE_NONE, > + Some(edge) => match edge { > + Edge::Rising => gpiod::GPIOD_LINE_EDGE_RISING, > + Edge::Falling => gpiod::GPIOD_LINE_EDGE_FALLING, > + Edge::Both => gpiod::GPIOD_LINE_EDGE_BOTH, > + }, > + } > + } > +} > + > +/// Line setting kind. > +#[derive(Copy, Clone, Debug, Eq, PartialEq)] > +pub enum SettingKind { > + /// Line direction. > + Direction, > + /// Bias. > + Bias, > + /// Drive. > + Drive, > + /// Edge detection. > + EdgeDetection, > + /// Active-low setting. > + ActiveLow, > + /// Debounce period. > + DebouncePeriod, > + /// Event clock type. > + EventClock, > + /// Output value. > + OutputValue, > +} > + > +/// Line settings. > +#[derive(Copy, Clone, Debug, Eq, PartialEq)] > +pub enum SettingVal { > + /// Line direction. > + Direction(Direction), > + /// Bias. > + Bias(Option<Bias>), > + /// Drive. > + Drive(Drive), > + /// Edge detection. > + EdgeDetection(Option<Edge>), > + /// Active-low setting. > + ActiveLow(bool), > + /// Debounce period. > + DebouncePeriod(Duration), > + /// Event clock type. > + EventClock(EventClock), > + /// Output value. > + OutputValue(Value), > +} > + > +impl fmt::Display for SettingVal { > + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { > + write!(f, "{:?}", self) > + } > +} > + > +/// Event clock settings. > +#[derive(Copy, Clone, Debug, Eq, PartialEq)] > +pub enum EventClock { > + /// Line uses the monotonic clock for edge event timestamps. > + Monotonic, > + /// Line uses the realtime clock for edge event timestamps. > + Realtime, > + /// Line uses the hardware timestamp engine clock for edge event timestamps. > + HTE, > +} > + > +impl EventClock { > + fn new(clock: u32) -> Result<Self> { > + match clock { > + gpiod::GPIOD_LINE_EVENT_CLOCK_MONOTONIC => Ok(EventClock::Monotonic), > + gpiod::GPIOD_LINE_EVENT_CLOCK_REALTIME => Ok(EventClock::Realtime), > + gpiod::GPIOD_LINE_EVENT_CLOCK_HTE => Ok(EventClock::HTE), > + _ => Err(Error::InvalidEnumValue("Eventclock", clock)), > + } > + } > + > + fn gpiod_clock(&self) -> u32 { > + match self { > + EventClock::Monotonic => gpiod::GPIOD_LINE_EVENT_CLOCK_MONOTONIC, > + EventClock::Realtime => gpiod::GPIOD_LINE_EVENT_CLOCK_REALTIME, > + EventClock::HTE => gpiod::GPIOD_LINE_EVENT_CLOCK_HTE, > + } > + } > +} > + > +/// Line status change event types. > +#[derive(Copy, Clone, Debug, Eq, PartialEq)] > +pub enum InfoChangeKind { > + /// Line has been requested. > + LineRequested, > + /// Previously requested line has been released. > + LineReleased, > + /// Line configuration has changed. > + LineConfigChanged, > +} > + > +impl InfoChangeKind { > + fn new(kind: u32) -> Result<Self> { > + match kind { > + gpiod::GPIOD_INFO_EVENT_LINE_REQUESTED => Ok(InfoChangeKind::LineRequested), > + gpiod::GPIOD_INFO_EVENT_LINE_RELEASED => Ok(InfoChangeKind::LineReleased), > + gpiod::GPIOD_INFO_EVENT_LINE_CONFIG_CHANGED => Ok(InfoChangeKind::LineConfigChanged), > + _ => Err(Error::InvalidEnumValue("InfoChangeKind", kind)), > + } > + } > +} > + > +/// Edge event types. > +#[derive(Copy, Clone, Debug, Eq, PartialEq)] > +pub enum EdgeKind { > + /// Rising edge event. > + Rising, > + /// Falling edge event. > + Falling, > +} > + > +impl EdgeKind { > + fn new(kind: u32) -> Result<Self> { > + match kind { > + gpiod::GPIOD_EDGE_EVENT_RISING_EDGE => Ok(EdgeKind::Rising), > + gpiod::GPIOD_EDGE_EVENT_FALLING_EDGE => Ok(EdgeKind::Falling), > + _ => Err(Error::InvalidEnumValue("EdgeEvent", kind)), > + } > + } > +} > + > +/// Various libgpiod-related functions. > + > +/// Check if the file pointed to by path is a GPIO chip character device. > +/// > +/// Returns true if the file exists and is a GPIO chip character device or a > +/// symbolic link to it. > +pub fn is_gpiochip_device<P: AsRef<Path>>(path: &P) -> bool { > + // Null-terminate the string > + let path = path.as_ref().to_string_lossy() + "\0"; > + > + unsafe { gpiod::gpiod_is_gpiochip_device(path.as_ptr() as *const c_char) } > +} > + > +/// Iterator for GPIO devices. > +pub fn gpiochip_devices<P: AsRef<Path>>(path: &P) -> Result<Vec<chip::Chip>> { > + let mut chips = Vec::new(); > + > + for entry in fs::read_dir(path).map_err(|_| Error::IoError)?.flatten() { > + let path = entry.path(); > + > + if is_gpiochip_device(&path) { > + chips.push(chip::Chip::open(&path)?); > + } > + } If there is a symlink to a device both in the path, or multiple symlinks to the same device, you will get the same chip several times in the returned Vec? Some mention of the order in which devices are returned? If you are returning a Vec then sorted might be nice, else state they are not sorted. And strictly speaking a Vec is not an iterator. It can be iterated over, but Vec.iter() is its iterator. > + > + Ok(chips) > +} > + > +/// Get the API version of the library as a human-readable string. > +pub fn version_string() -> Result<&'static str> { > + // SAFETY: The string returned by libgpiod is guaranteed to live forever. > + let version = unsafe { gpiod::gpiod_version_string() }; > + > + if version.is_null() { > + return Err(Error::NullString("GPIO library version")); > + } > + > + // SAFETY: The string is guaranteed to be valid here by the C API. > + str::from_utf8(unsafe { slice::from_raw_parts(version as *const u8, strlen(version) as usize) }) > + .map_err(Error::StringNotUtf8) > +} What if the Rust bindings version differ from the libgpiod version? > diff --git a/bindings/rust/libgpiod/src/line_config.rs b/bindings/rust/libgpiod/src/line_config.rs > new file mode 100644 > index 000000000000..ea25452fd1d6 > --- /dev/null > +++ b/bindings/rust/libgpiod/src/line_config.rs > @@ -0,0 +1,118 @@ > +// SPDX-License-Identifier: Apache-2.0 AND BSD-3-Clause > +// > +// Copyright 2022 Linaro Ltd. All Rights Reserved. > +// Viresh Kumar <viresh.kumar@linaro.org> > + > +use std::os::raw::c_ulong; > +use std::slice::from_raw_parts; > + > +use vmm_sys_util::errno::Error as Errno; > + > +use super::{gpiod, line::Settings, Error, Offset, OperationType, Result}; > + > +/// Line configuration objects. > +/// > +/// The line-config object contains the configuration for lines that can be > +/// used in two cases: > +/// - when making a line request > +/// - when reconfiguring a set of already requested lines. > +/// > +/// A new line-config object is empty. Using it in a request will lead to an > +/// error. In order to a line-config to become useful, it needs to be assigned ^ for > +/// at least one offset-to-settings mapping by calling > +/// ::gpiod_line_config_add_line_settings. > +/// > +/// When calling ::gpiod_chip_request_lines, the library will request all > +/// offsets that were assigned settings in the order that they were assigned. > + > +#[derive(Clone, Debug, Eq, PartialEq)] > +pub struct Config { > + config: *mut gpiod::gpiod_line_config, > +} > + Clone/Drop wrapping a raw pointer again. > +impl Config { > + /// Create a new line config object. > + pub fn new() -> Result<Self> { > + let config = unsafe { gpiod::gpiod_line_config_new() }; > + > + if config.is_null() { > + return Err(Error::OperationFailed( > + OperationType::LineConfigNew, > + Errno::last(), > + )); > + } > + > + Ok(Self { config }) > + } > + > + /// Private helper, Returns gpiod_line_config > + pub(crate) fn config(&self) -> *mut gpiod::gpiod_line_config { > + self.config > + } > + > + /// Resets the entire configuration stored in the object. This is useful if > + /// the user wants to reuse the object without reallocating it. > + pub fn reset(&mut self) { > + unsafe { gpiod::gpiod_line_config_reset(self.config) } > + } > + > + /// Add line settings for a set of offsets. > + pub fn add_line_settings(&self, offsets: &[Offset], settings: Settings) -> Result<()> { > + let ret = unsafe { > + gpiod::gpiod_line_config_add_line_settings( > + self.config, > + offsets.as_ptr(), > + offsets.len() as c_ulong, > + settings.settings(), > + ) > + }; > + > + if ret == -1 { > + Err(Error::OperationFailed( > + OperationType::LineConfigAddSettings, > + Errno::last(), > + )) > + } else { > + Ok(()) > + } > + } > + > + /// Get line settings for offset. > + pub fn line_settings(&self, offset: Offset) -> Result<Settings> { > + let settings = unsafe { gpiod::gpiod_line_config_get_line_settings(self.config, offset) }; > + > + if settings.is_null() { > + return Err(Error::OperationFailed( > + OperationType::LineConfigGetSetting, ^ Settings > + Errno::last(), > + )); > + } > + > + Ok(Settings::new_with_settings(settings)) > + } > + > + /// Get configured offsets. > + pub fn offsets(&self) -> Result<Vec<Offset>> { > + let mut num: u64 = 0; > + let mut offsets: *mut Offset = std::ptr::null_mut(); > + > + let ret = > + unsafe { gpiod::gpiod_line_config_get_offsets(self.config, &mut num, &mut offsets) }; > + > + if ret == -1 { > + Err(Error::OperationFailed( > + OperationType::LineConfigGetOffsets, > + Errno::last(), > + )) > + } else { > + Ok(unsafe { from_raw_parts(offsets as *const Offset, num as usize).to_vec() }) > + } > + } > +} > + > +impl Drop for Config { > + /// Free the line config object and release all associated resources. > + fn drop(&mut self) { > + unsafe { gpiod::gpiod_line_config_free(self.config) } > + } > +} > diff --git a/bindings/rust/libgpiod/src/line_info.rs b/bindings/rust/libgpiod/src/line_info.rs > new file mode 100644 > index 000000000000..9db51fc30efd > --- /dev/null > +++ b/bindings/rust/libgpiod/src/line_info.rs > @@ -0,0 +1,180 @@ > +// SPDX-License-Identifier: Apache-2.0 AND BSD-3-Clause > +// > +// Copyright 2022 Linaro Ltd. All Rights Reserved. > +// Viresh Kumar <viresh.kumar@linaro.org> > + > +use libc::strlen; > +use std::sync::Arc; > +use std::time::Duration; > +use std::{slice, str}; > + > +use vmm_sys_util::errno::Error as Errno; > + > +use super::{ > + chip, gpiod, info, Bias, Direction, Drive, Edge, Error, EventClock, Offset, OperationType, > + Result, > +}; > + > +/// Line info > +/// > +/// Exposes functions for retrieving kernel information about both requested and > +/// free lines. Line info object contains an immutable snapshot of a line's status. > +/// > +/// The line info contains all the publicly available information about a > +/// line, which does not include the line value. The line must be requested > +/// to access the line value. > + > +#[derive(Clone, Debug, Eq, PartialEq)] > +pub struct Info { > + info: *mut gpiod::gpiod_line_info, > + info_event: bool, > +} > + Clone/Drop wrapping a raw pointer again. > +impl Info { > + /// Get a snapshot of information about the line and optionally start watching it for changes. > + pub(crate) fn new(ichip: Arc<chip::Internal>, offset: Offset) -> Result<Self> { > + let info = unsafe { gpiod::gpiod_chip_get_line_info(ichip.chip(), offset) }; > + > + if info.is_null() { > + return Err(Error::OperationFailed( > + OperationType::ChipGetLineInfo, > + Errno::last(), > + )); > + } > + > + Ok(Self { > + info, > + info_event: false, > + }) > + } Move the guts into chip where this is called. This should just be a bare contructor: pub(crate) fn new(info: *mut gpiod::gpiod_line_info, free_me_seymour: bool) > + > + pub(crate) fn new_watch(ichip: Arc<chip::Internal>, offset: Offset) -> Result<Self> { > + let info = unsafe { gpiod::gpiod_chip_watch_line_info(ichip.chip(), offset) }; > + > + if info.is_null() { > + return Err(Error::OperationFailed( > + OperationType::ChipWatchLineInfo, > + Errno::last(), > + )); > + } > + > + Ok(Self { > + info, > + info_event: false, > + }) > + } Move the guts into chip where this is called, and call the new() above. > + > + /// Get the Line info object associated with an event. > + pub(crate) fn new_from_event(event: &info::Event) -> Result<Self> { > + let info = unsafe { gpiod::gpiod_info_event_get_line_info(event.event()) }; > + > + if info.is_null() { > + return Err(Error::OperationFailed( > + OperationType::InfoEventGetLineInfo, > + Errno::last(), > + )); > + } > + > + Ok(Self { > + info, > + info_event: true, > + }) > + } Move the guts into info_events where this is called, and call the new() above. > + > + /// Get the offset of the line within the GPIO chip. > + /// > + /// The offset uniquely identifies the line on the chip. The combination of the chip and offset > + /// uniquely identifies the line within the system. > + > + pub fn offset(&self) -> Offset { > + unsafe { gpiod::gpiod_line_info_get_offset(self.info) } > + } > + > + /// Get GPIO line's name. > + pub fn name(&self) -> Result<&str> { > + // SAFETY: The string returned by libgpiod is guaranteed to live as long > + // as the `struct Info`. > + let name = unsafe { gpiod::gpiod_line_info_get_name(self.info) }; > + if name.is_null() { > + return Err(Error::NullString("GPIO line's name")); > + } > + > + // SAFETY: The string is guaranteed to be valid here by the C API. > + str::from_utf8(unsafe { slice::from_raw_parts(name as *const u8, strlen(name) as usize) }) > + .map_err(Error::StringNotUtf8) > + } > + > + /// Returns True if the line is in use, false otherwise. > + /// > + /// The user space can't know exactly why a line is busy. It may have been > + /// requested by another process or hogged by the kernel. It only matters that > + /// the line is used and we can't request it. > + pub fn is_used(&self) -> bool { > + unsafe { gpiod::gpiod_line_info_is_used(self.info) } > + } > + > + /// Get the GPIO line's consumer name. > + pub fn consumer(&self) -> Result<&str> { > + // SAFETY: The string returned by libgpiod is guaranteed to live as long > + // as the `struct Info`. > + let name = unsafe { gpiod::gpiod_line_info_get_consumer(self.info) }; > + if name.is_null() { > + return Err(Error::NullString("GPIO line's consumer name")); > + } > + > + // SAFETY: The string is guaranteed to be valid here by the C API. > + str::from_utf8(unsafe { slice::from_raw_parts(name as *const u8, strlen(name) as usize) }) > + .map_err(Error::StringNotUtf8) > + } > + > + /// Get the GPIO line's direction. > + pub fn direction(&self) -> Result<Direction> { > + Direction::new(unsafe { gpiod::gpiod_line_info_get_direction(self.info) } as u32) > + } > + > + /// Returns true if the line is "active-low", false otherwise. > + pub fn is_active_low(&self) -> bool { > + unsafe { gpiod::gpiod_line_info_is_active_low(self.info) } > + } > + > + /// Get the GPIO line's bias setting. > + pub fn bias(&self) -> Result<Option<Bias>> { > + Bias::new(unsafe { gpiod::gpiod_line_info_get_bias(self.info) } as u32) > + } > + > + /// Get the GPIO line's drive setting. > + pub fn drive(&self) -> Result<Drive> { > + Drive::new(unsafe { gpiod::gpiod_line_info_get_drive(self.info) } as u32) > + } > + > + /// Get the current edge detection setting of the line. > + pub fn edge_detection(&self) -> Result<Option<Edge>> { > + Edge::new(unsafe { gpiod::gpiod_line_info_get_edge_detection(self.info) } as u32) > + } > + > + /// Get the current event clock setting used for edge event timestamps. > + pub fn event_clock(&self) -> Result<EventClock> { > + EventClock::new(unsafe { gpiod::gpiod_line_info_get_event_clock(self.info) } as u32) > + } > + > + /// Returns true if the line is debounced (either by hardware or by the > + /// kernel software debouncer), false otherwise. > + pub fn is_debounced(&self) -> bool { > + unsafe { gpiod::gpiod_line_info_is_debounced(self.info) } > + } > + > + /// Get the debounce period of the line. > + pub fn debounce_period(&self) -> Duration { > + Duration::from_micros(unsafe { gpiod::gpiod_line_info_get_debounce_period_us(self.info) }) > + } > +} > + > +impl Drop for Info { > + fn drop(&mut self) { > + // We must not free the Line info object created from `struct info::Event` by calling > + // libgpiod API. > + if !self.info_event { > + unsafe { gpiod::gpiod_line_info_free(self.info) } > + } > + } > +} > diff --git a/bindings/rust/libgpiod/src/line_request.rs b/bindings/rust/libgpiod/src/line_request.rs > new file mode 100644 > index 000000000000..e4ff951aef29 > --- /dev/null > +++ b/bindings/rust/libgpiod/src/line_request.rs > @@ -0,0 +1,246 @@ > +// SPDX-License-Identifier: Apache-2.0 AND BSD-3-Clause > +// > +// Copyright 2022 Linaro Ltd. All Rights Reserved. > +// Viresh Kumar <viresh.kumar@linaro.org> > + > +use libc::EINVAL; > +use std::os::raw::c_ulong; > +use std::sync::Arc; > +use std::time::Duration; > + > +use vmm_sys_util::errno::Error as Errno; > + > +use super::{ > + chip, edge, gpiod, line, request, Error, Offset, OperationType, Result, Value, ValueMap, > +}; > + > +/// Line request operations > +/// > +/// Allows interaction with a set of requested lines. > +#[derive(Clone, Debug, Eq, PartialEq)] > +pub struct Request { > + request: *mut gpiod::gpiod_line_request, > +} > + Clone/Drop wrapping a raw pointer again. Does it make sense to be able to clone a request? As per Chip, concerns being about reading events concurrently, as well as the freeing of the underlying resources. > +impl Request { > + /// Request a set of lines for exclusive usage. > + pub(crate) fn new( > + ichip: &Arc<chip::Internal>, > + rconfig: &request::Config, > + lconfig: &line::Config, > + ) -> Result<Self> { > + let request = unsafe { > + gpiod::gpiod_chip_request_lines(ichip.chip(), rconfig.config(), lconfig.config()) > + }; > + > + if request.is_null() { > + return Err(Error::OperationFailed( > + OperationType::LineRequest, ^ ChipRequestLines > + Errno::last(), > + )); > + } > + > + Ok(Self { request }) > + } As with other constructors, this should be a bare constructor, and the guts should be in Chip where this function is called. > + > + /// Get the number of lines in the request. > + pub fn num_lines(&self) -> usize { > + unsafe { gpiod::gpiod_line_request_get_num_lines(self.request) as usize } > + } > + > + /// Get the offsets of lines in the request. > + pub fn offsets(&self) -> Vec<Offset> { > + let mut offsets = vec![0; self.num_lines() as usize]; > + > + unsafe { gpiod::gpiod_line_request_get_offsets(self.request, offsets.as_mut_ptr()) }; > + offsets > + } > + > + /// Get the value (0 or 1) of a single line associated with the request. > + pub fn value(&self, offset: Offset) -> Result<Value> { > + let value = unsafe { gpiod::gpiod_line_request_get_value(self.request, offset) }; > + > + if value != 0 && value != 1 { > + Err(Error::OperationFailed( > + OperationType::LineRequestGetVal, > + Errno::last(), > + )) > + } else { > + Value::new(value) > + } > + } > + > + /// Get values of a subset of lines associated with the request. > + pub fn values_subset(&self, offsets: &[Offset]) -> Result<ValueMap> { > + let mut values = vec![0; offsets.len()]; > + > + let ret = unsafe { > + gpiod::gpiod_line_request_get_values_subset( > + self.request, > + offsets.len() as c_ulong, > + offsets.as_ptr(), > + values.as_mut_ptr(), > + ) > + }; > + > + if ret == -1 { > + Err(Error::OperationFailed( > + OperationType::LineRequestGetValSubset, > + Errno::last(), > + )) > + } else { > + let mut map = ValueMap::new(); > + > + for (i, val) in values.iter().enumerate() { > + map.insert(offsets[i].into(), Value::new(*val)?); > + } > + > + Ok(map) > + } > + } > + > + /// Get values of all lines associated with the request. > + pub fn values(&self) -> Result<ValueMap> { > + self.values_subset(&self.offsets()) > + } > + > + /// Set the value of a single line associated with the request. > + pub fn set_value(&self, offset: Offset, value: Value) -> Result<()> { > + let ret = > + unsafe { gpiod::gpiod_line_request_set_value(self.request, offset, value.value()) }; > + > + if ret == -1 { > + Err(Error::OperationFailed( > + OperationType::LineRequestSetVal, > + Errno::last(), > + )) > + } else { > + Ok(()) > + } > + } > + > + /// Get values of a subset of lines associated with the request. > + pub fn set_values_subset(&self, map: ValueMap) -> Result<()> { > + let mut offsets = Vec::new(); > + let mut values = Vec::new(); > + > + for (offset, value) in map { > + offsets.push(offset as u32); > + values.push(value.value()); > + } > + > + let ret = unsafe { > + gpiod::gpiod_line_request_set_values_subset( > + self.request, > + offsets.len() as c_ulong, > + offsets.as_ptr(), > + values.as_ptr(), > + ) > + }; > + > + if ret == -1 { > + Err(Error::OperationFailed( > + OperationType::LineRequestSetValSubset, > + Errno::last(), > + )) > + } else { > + Ok(()) > + } > + } > + > + /// Get values of all lines associated with the request. > + pub fn set_values(&self, values: &[Value]) -> Result<()> { > + if values.len() != self.num_lines() as usize { > + return Err(Error::OperationFailed( > + OperationType::LineRequestSetVal, > + Errno::new(EINVAL), > + )); > + } The error implies a C call failed, but that isn't the case. Make it a plain InvalidArgument? > + > + let mut new_values = Vec::new(); > + for value in values { > + new_values.push(value.value()); > + } > + > + let ret = > + unsafe { gpiod::gpiod_line_request_set_values(self.request, new_values.as_ptr()) }; > + > + if ret == -1 { > + Err(Error::OperationFailed( > + OperationType::LineRequestSetVal, > + Errno::last(), > + )) > + } else { > + Ok(()) > + } > + } > + > + /// Update the configuration of lines associated with the line request. > + pub fn reconfigure_lines(&self, lconfig: &line::Config) -> Result<()> { > + let ret = > + unsafe { gpiod::gpiod_line_request_reconfigure_lines(self.request, lconfig.config()) }; > + > + if ret == -1 { > + Err(Error::OperationFailed( > + OperationType::LineRequestReconfigLines, > + Errno::last(), > + )) > + } else { > + Ok(()) > + } > + } > + > + /// Get the file descriptor associated with the line request. > + pub fn fd(&self) -> u32 { > + unsafe { gpiod::gpiod_line_request_get_fd(self.request) as u32 } > + } impl AsRawFd? > + > + /// Wait for edge events on any of the lines associated with the request. > + pub fn wait_edge_event(&self, timeout: Option<Duration>) -> Result<bool> { > + let timeout = match timeout { > + Some(x) => x.as_nanos() as i64, > + // Block indefinitely > + None => -1, > + }; > + > + let ret = unsafe { gpiod::gpiod_line_request_wait_edge_event(self.request, timeout) }; > + > + match ret { > + -1 => Err(Error::OperationFailed( > + OperationType::LineRequestWaitEdgeEvent, > + Errno::last(), > + )), > + 0 => Ok(false), > + _ => Ok(true), > + } > + } > + > + /// Get a number of edge events from a line request. > + /// > + /// This function will block if no event was queued for the line. > + pub fn read_edge_events(&self, buffer: &mut edge::event::Buffer) -> Result<u32> { What is to stop the user requesting events between the len returned here and the end of the buffer? Why knows what state they are in. Would be better to return an object that can only access the relevant events in the buffer. I've got an alternative for the event buffer implementation - will describe that at the end. > + let ret = unsafe { > + gpiod::gpiod_line_request_read_edge_event( > + self.request, > + buffer.buffer(), > + buffer.capacity() as u64, > + ) > + }; > + > + if ret == -1 { > + Err(Error::OperationFailed( > + OperationType::LineRequestReadEdgeEvent, > + Errno::last(), > + )) > + } else { > + Ok(ret as u32) > + } > + } > +} > + > +impl Drop for Request { > + /// Release the requested lines and free all associated resources. > + fn drop(&mut self) { > + unsafe { gpiod::gpiod_line_request_release(self.request) } > + } > +} > diff --git a/bindings/rust/libgpiod/src/line_settings.rs b/bindings/rust/libgpiod/src/line_settings.rs > new file mode 100644 > index 000000000000..2c3090132ea5 > --- /dev/null > +++ b/bindings/rust/libgpiod/src/line_settings.rs > @@ -0,0 +1,277 @@ > +// SPDX-License-Identifier: Apache-2.0 AND BSD-3-Clause > +// > +// Copyright 2022 Linaro Ltd. All Rights Reserved. > +// Viresh Kumar <viresh.kumar@linaro.org> > + > +use std::time::Duration; > + > +use vmm_sys_util::errno::Error as Errno; > + > +use super::{ > + gpiod, Bias, Direction, Drive, Edge, Error, EventClock, OperationType, Result, SettingKind, > + SettingVal, Value, > +}; > + > +/// Line settings objects. > +/// > +/// Line settings object contains a set of line properties that can be used > +/// when requesting lines or reconfiguring an existing request. > +/// > +/// Mutators in general can only fail if the new property value is invalid. The > +/// return values can be safely ignored - the object remains valid even after > +/// a mutator fails and simply uses the sane default appropriate for given > +/// property. > + > +#[derive(Clone, Debug, Eq, PartialEq)] > +pub struct Settings { > + settings: *mut gpiod::gpiod_line_settings, > +} > + Clone/Drop wrapping a raw pointer again. > +impl Settings { > + /// Create a new line settings object. > + pub fn new() -> Result<Self> { > + let settings = unsafe { gpiod::gpiod_line_settings_new() }; > + > + if settings.is_null() { > + return Err(Error::OperationFailed( > + OperationType::LineSettingsNew, > + Errno::last(), > + )); > + } > + > + Ok(Self { settings }) > + } > + > + pub fn new_with_settings(settings: *mut gpiod::gpiod_line_settings) -> Self { > + Self { settings } > + } > + > + /// Private helper, Returns gpiod_line_settings > + pub(crate) fn settings(&self) -> *mut gpiod::gpiod_line_settings { > + self.settings > + } Just make self.settings pub(crate)? > + > + /// Resets the line settings object to its default values. > + pub fn reset(&mut self) { > + unsafe { gpiod::gpiod_line_settings_reset(self.settings) } > + } > + > + /// Makes copy of the settings object. > + pub fn settings_clone(&self) -> Result<Self> { > + let settings = unsafe { gpiod::gpiod_line_settings_copy(self.settings) }; > + if settings.is_null() { > + return Err(Error::OperationFailed( > + OperationType::LineSettingsCopy, > + Errno::last(), > + )); > + } > + > + Ok(Self { settings }) > + } This clone is fine - the derived one is not. > + > + /// Set line prop setting. > + pub fn set_prop(&mut self, values: &[SettingVal]) -> Result<()> { Rename values to props, particularly as values is one of the props. > + for value in values { > + match value { > + SettingVal::Direction(val) => self.set_direction(*val)?, > + SettingVal::EdgeDetection(val) => self.set_edge_detection(*val)?, > + SettingVal::Bias(val) => self.set_bias(*val)?, > + SettingVal::Drive(val) => self.set_drive(*val)?, > + SettingVal::ActiveLow(val) => self.set_active_low(*val), > + SettingVal::DebouncePeriod(val) => self.set_debounce_period(*val), > + SettingVal::EventClock(val) => self.set_event_clock(*val)?, > + SettingVal::OutputValue(val) => self.set_output_value(*val)?, > + } > + } > + > + Ok(()) > + } Make mutators -> Result<&mut Self> so they can be chained. > + > + /// Get the line prop setting. > + pub fn prop(&self, property: SettingKind) -> Result<SettingVal> { > + Ok(match property { > + SettingKind::Direction => SettingVal::Direction(self.direction()?), > + SettingKind::EdgeDetection => SettingVal::EdgeDetection(self.edge_detection()?), > + SettingKind::Bias => SettingVal::Bias(self.bias()?), > + SettingKind::Drive => SettingVal::Drive(self.drive()?), > + SettingKind::ActiveLow => SettingVal::ActiveLow(self.active_low()), > + SettingKind::DebouncePeriod => SettingVal::DebouncePeriod(self.debounce_period()?), > + SettingKind::EventClock => SettingVal::EventClock(self.event_clock()?), > + SettingKind::OutputValue => SettingVal::OutputValue(self.output_value()?), > + }) > + } > + > + /// Set the line direction. > + fn set_direction(&mut self, direction: Direction) -> Result<()> { > + let ret = unsafe { > + gpiod::gpiod_line_settings_set_direction( > + self.settings, > + direction.gpiod_direction() as i32, > + ) > + }; > + > + if ret == -1 { > + Err(Error::OperationFailed( > + OperationType::LineSettingsSetDirection, > + Errno::last(), > + )) > + } else { > + Ok(()) > + } > + } These mutators might as well be pub as well. Then the caller has the option of using the set_prop(&[settings]) or chaining mutators. Similarly the accessors. > + > + /// Get the direction setting. > + fn direction(&self) -> Result<Direction> { > + Direction::new(unsafe { gpiod::gpiod_line_settings_get_direction(self.settings) } as u32) > + } > + > + /// Set the edge event detection setting. > + fn set_edge_detection(&mut self, edge: Option<Edge>) -> Result<()> { > + let ret = unsafe { > + gpiod::gpiod_line_settings_set_edge_detection( > + self.settings, > + Edge::gpiod_edge(edge) as i32, > + ) > + }; > + > + if ret == -1 { > + Err(Error::OperationFailed( > + OperationType::LineSettingsSetEdgeDetection, > + Errno::last(), > + )) > + } else { > + Ok(()) > + } > + } > + > + /// Get the edge event detection setting. > + fn edge_detection(&self) -> Result<Option<Edge>> { > + Edge::new(unsafe { gpiod::gpiod_line_settings_get_edge_detection(self.settings) } as u32) > + } > + > + /// Set the bias setting. > + fn set_bias(&mut self, bias: Option<Bias>) -> Result<()> { > + let ret = unsafe { > + gpiod::gpiod_line_settings_set_bias(self.settings, Bias::gpiod_bias(bias) as i32) > + }; > + > + if ret == -1 { > + Err(Error::OperationFailed( > + OperationType::LineSettingsSetBias, > + Errno::last(), > + )) > + } else { > + Ok(()) > + } > + } > + > + /// Get the bias setting. > + fn bias(&self) -> Result<Option<Bias>> { > + Bias::new(unsafe { gpiod::gpiod_line_settings_get_bias(self.settings) } as u32) > + } > + > + /// Set the drive setting. > + fn set_drive(&mut self, drive: Drive) -> Result<()> { > + let ret = unsafe { > + gpiod::gpiod_line_settings_set_drive(self.settings, drive.gpiod_drive() as i32) > + }; > + > + if ret == -1 { > + Err(Error::OperationFailed( > + OperationType::LineSettingsSetDrive, > + Errno::last(), > + )) > + } else { > + Ok(()) > + } > + } > + > + /// Get the drive setting. > + fn drive(&self) -> Result<Drive> { > + Drive::new(unsafe { gpiod::gpiod_line_settings_get_drive(self.settings) } as u32) > + } > + > + /// Set active-low setting. > + fn set_active_low(&mut self, active_low: bool) { > + unsafe { gpiod::gpiod_line_settings_set_active_low(self.settings, active_low) } > + } > + > + /// Check the active-low setting. > + fn active_low(&self) -> bool { > + unsafe { gpiod::gpiod_line_settings_get_active_low(self.settings) } > + } > + > + /// Set the debounce period setting. > + fn set_debounce_period(&mut self, period: Duration) { > + unsafe { > + gpiod::gpiod_line_settings_set_debounce_period_us( > + self.settings, > + period.as_micros() as u64, > + ) > + } > + } > + > + /// Get the debounce period. > + fn debounce_period(&self) -> Result<Duration> { > + Ok(Duration::from_micros(unsafe { > + gpiod::gpiod_line_settings_get_debounce_period_us(self.settings) > + })) > + } > + > + /// Set the event clock setting. > + fn set_event_clock(&mut self, clock: EventClock) -> Result<()> { > + let ret = unsafe { > + gpiod::gpiod_line_settings_set_event_clock(self.settings, clock.gpiod_clock() as i32) > + }; > + > + if ret == -1 { > + Err(Error::OperationFailed( > + OperationType::LineSettingsSetEventClock, > + Errno::last(), > + )) > + } else { > + Ok(()) > + } > + } > + > + /// Get the event clock setting. > + fn event_clock(&self) -> Result<EventClock> { > + EventClock::new(unsafe { gpiod::gpiod_line_settings_get_event_clock(self.settings) } as u32) > + } > + > + /// Set the output value setting. > + fn set_output_value(&mut self, value: Value) -> Result<()> { > + let ret = > + unsafe { gpiod::gpiod_line_settings_set_output_value(self.settings, value.value()) }; > + > + if ret == -1 { > + Err(Error::OperationFailed( > + OperationType::LineSettingsSetOutputValue, > + Errno::last(), > + )) > + } else { > + Ok(()) > + } > + } > + > + /// Get the output value, 0 or 1. > + fn output_value(&self) -> Result<Value> { > + let value = unsafe { gpiod::gpiod_line_settings_get_output_value(self.settings) }; > + > + if value != 0 && value != 1 { > + Err(Error::OperationFailed( > + OperationType::LineSettingsGetOutVal, > + Errno::last(), > + )) > + } else { > + Value::new(value) > + } > + } > +} > + > +impl Drop for Settings { > + /// Free the line settings object and release all associated resources. > + fn drop(&mut self) { > + unsafe { gpiod::gpiod_line_settings_free(self.settings) } > + } > +} > diff --git a/bindings/rust/libgpiod/src/request_config.rs b/bindings/rust/libgpiod/src/request_config.rs > new file mode 100644 > index 000000000000..760d9c755c86 > --- /dev/null > +++ b/bindings/rust/libgpiod/src/request_config.rs > @@ -0,0 +1,96 @@ > +// SPDX-License-Identifier: Apache-2.0 AND BSD-3-Clause > +// > +// Copyright 2022 Linaro Ltd. All Rights Reserved. > +// Viresh Kumar <viresh.kumar@linaro.org> > + > +use libc::strlen; > +use std::os::raw::{c_char, c_ulong}; > +use std::{slice, str}; > + > +use vmm_sys_util::errno::Error as Errno; > + > +use super::{gpiod, Error, OperationType, Result}; > + > +/// Request configuration objects > +/// > +/// Request config objects are used to pass a set of options to the kernel at > +/// the time of the line request. The mutators don't return error values. If the > +/// values are invalid, in general they are silently adjusted to acceptable > +/// ranges. > + > +#[derive(Clone, Debug, Eq, PartialEq)] > +pub struct Config { > + config: *mut gpiod::gpiod_request_config, > +} > + And one last time... Clone/Drop wrapping a raw pointer again. > +impl Config { > + /// Create a new request config object. > + pub fn new() -> Result<Self> { > + let config = unsafe { gpiod::gpiod_request_config_new() }; > + if config.is_null() { > + return Err(Error::OperationFailed( > + OperationType::RequestConfigNew, > + Errno::last(), > + )); > + } > + > + Ok(Self { config }) > + } > + > + /// Private helper, Returns gpiod_request_config > + pub(crate) fn config(&self) -> *mut gpiod::gpiod_request_config { > + self.config > + } > + Make self.config pub(crate) instead? > + /// Set the consumer name for the request. > + /// > + /// If the consumer string is too long, it will be truncated to the max > + /// accepted length. > + pub fn set_consumer(&self, consumer: &str) { > + // Null-terminate the string > + let consumer = consumer.to_owned() + "\0"; > + > + unsafe { > + gpiod::gpiod_request_config_set_consumer( > + self.config, > + consumer.as_ptr() as *const c_char, > + ) > + } > + } > + > + /// Get the consumer name configured in the request config. > + pub fn consumer(&self) -> Result<&str> { > + // SAFETY: The string returned by libgpiod is guaranteed to live as long > + // as the `struct Config`. > + let consumer = unsafe { gpiod::gpiod_request_config_get_consumer(self.config) }; > + if consumer.is_null() { > + return Err(Error::OperationFailed( > + OperationType::RequestConfigGetConsumer, > + Errno::last(), > + )); > + } > + > + // SAFETY: The string is guaranteed to be valid here by the C API. > + str::from_utf8(unsafe { > + slice::from_raw_parts(consumer as *const u8, strlen(consumer) as usize) > + }) > + .map_err(Error::StringNotUtf8) > + } > + > + /// Set the size of the kernel event buffer for the request. > + pub fn set_event_buffer_size(&self, size: usize) { > + unsafe { gpiod::gpiod_request_config_set_event_buffer_size(self.config, size as c_ulong) } > + } > + > + /// Get the edge event buffer size setting for the request config. > + pub fn event_buffer_size(&self) -> usize { > + unsafe { gpiod::gpiod_request_config_get_event_buffer_size(self.config) as usize } > + } > +} > + > +impl Drop for Config { > + /// Free the request config object and release all associated resources. > + fn drop(&mut self) { > + unsafe { gpiod::gpiod_request_config_free(self.config) } > + } > +} > -- > 2.31.1.272.g89b43f80a514 > Overall I don't see any problems that require major rework to the scale of previous versions. My main itches that need to be scratched being dangling pointers and to a lesser degree thread safey. I find the event_buffer solution to be less safe than it could be. I've got an alternative, based on the my earlier PoC/suggestion, that returns an snaphot of events when you read events from the request into the buffer. I've put that in a branch on github [1] if you want to take a look. I went though a few variations wrt how to deal with the event cloning. Not implementing Clone on the events means they can't be stored in a Vec, and so storing them in the Buffer was problematic. My initial solution was for the event to only wrap the C pointer and have distinct event types for buffered and cloned, implemented using traits - which is bit of a pain as the user then needs to import the trait themselves to use the events :-(. Then tried a macro to add impl Event on the two event types, which looks ugly to me. Finally went with having Event be a new type of BaseEvent, which does impl Clone, so the event buffer can store them (and <cough> cast them to Event as appropriate). Slightly more evil under the hood than I would like, but provides the cleanest implementation. The Events snapshot should impl Iterator to make it easier to use, though I haven't added that yet. And the error returned by Events::event() is wrong - just cut and pasted one in there - probably should be InvalidArguments? Anyway, the end result is that it is not possible to read an invalid or uninitialised event from Rust, which was the point. It also better handles lifetimes, so it isn't necessary to explicitly drop events before re-using the buffer, and the event_clone() method can be an instance method rather than a class method (to borrow some Python terminology). I've added an events.rs to the examples to demonstrate how the changes impact usage in each case. Cheers, Kent. [1] https://github.com/warthog618/libgpiod/commits/rust_v7_mods
On Fri, Oct 14, 2022 at 04:17:20PM +0530, Viresh Kumar wrote: > Add rust wrapper crate, around the libpiod-sys crate added earlier, to > provide a convenient interface for the users. > > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> One more thing - I overlooked trying to build on 32-bit while reviewing. (in this case on a Raspberry Pi 4) Unfortunately that fails: Compiling libgpiod v0.1.0 (/home/pi/libgpiod/bindings/rust/libgpiod) error[E0308]: mismatched types --> libgpiod/src/edge_event.rs:32:88 | 32 | let event = unsafe { gpiod::gpiod_edge_event_buffer_get_event(buffer.buffer(), index) }; | ---------------------------------------- ^^^^^ expected `u32`, found `u64` | | | arguments to this function are incorrect | note: function defined here --> /home/pi/libgpiod/bindings/rust/libgpiod-sys/src/bindings.rs:1151:12 | 1151 | pub fn gpiod_edge_event_buffer_get_event( | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: you can convert a `u64` to a `u32` and panic if the converted value doesn't fit | 32 | let event = unsafe { gpiod::gpiod_edge_event_buffer_get_event(buffer.buffer(), index.try_into().unwrap()) }; | ++++++++++++++++++++ error[E0308]: mismatched types --> libgpiod/src/edge_event.rs:81:18 | 80 | pub fn global_seqno(&self) -> u64 { | --- expected `u64` because of return type 81 | unsafe { gpiod::gpiod_edge_event_get_global_seqno(self.event) } | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `u64`, found `u32` | help: you can convert a `u32` to a `u64` | 81 | unsafe { gpiod::gpiod_edge_event_get_global_seqno(self.event).into() } | +++++++ error[E0308]: mismatched types --> libgpiod/src/edge_event.rs:89:18 | 88 | pub fn line_seqno(&self) -> u64 { | --- expected `u64` because of return type 89 | unsafe { gpiod::gpiod_edge_event_get_line_seqno(self.event) } | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `u64`, found `u32` | help: you can convert a `u32` to a `u64` | 89 | unsafe { gpiod::gpiod_edge_event_get_line_seqno(self.event).into() } | +++++++ error[E0308]: mismatched types --> libgpiod/src/line_config.rs:100:72 | 100 | unsafe { gpiod::gpiod_line_config_get_offsets(self.config, &mut num, &mut offsets) }; | ------------------------------------ ^^^^^^^^ expected `u32`, found `u64` | | | arguments to this function are incorrect | = note: expected raw pointer `*mut u32` found mutable reference `&mut u64` note: function defined here --> /home/pi/libgpiod/bindings/rust/libgpiod-sys/src/bindings.rs:847:12 | 847 | pub fn gpiod_line_config_get_offsets( | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error[E0308]: mismatched types --> libgpiod/src/line_info.rs:168:40 | 168 | Duration::from_micros(unsafe { gpiod::gpiod_line_info_get_debounce_period_us(self.info) }) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `u64`, found `u32` | help: you can convert a `u32` to a `u64` | 168 | Duration::from_micros(unsafe { gpiod::gpiod_line_info_get_debounce_period_us(self.info).into() }) | +++++++ error[E0308]: mismatched types --> libgpiod/src/line_request.rs:226:17 | 223 | gpiod::gpiod_line_request_read_edge_event( | ----------------------------------------- arguments to this function are incorrect ... 226 | buffer.capacity() as u64, | ^^^^^^^^^^^^^^^^^^^^^^^^ expected `u32`, found `u64` | note: function defined here --> /home/pi/libgpiod/bindings/rust/libgpiod-sys/src/bindings.rs:1058:12 | 1058 | pub fn gpiod_line_request_read_edge_event( | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: you can convert a `u64` to a `u32` and panic if the converted value doesn't fit | 226 | (buffer.capacity() as u64).try_into().unwrap(), | + +++++++++++++++++++++ error[E0308]: mismatched types --> libgpiod/src/line_settings.rs:209:17 | 207 | gpiod::gpiod_line_settings_set_debounce_period_us( | ------------------------------------------------- arguments to this function are incorrect 208 | self.settings, 209 | period.as_micros() as u64, | ^^^^^^^^^^^^^^^^^^^^^^^^^ expected `u32`, found `u64` | note: function defined here --> /home/pi/libgpiod/bindings/rust/libgpiod-sys/src/bindings.rs:746:12 | 746 | pub fn gpiod_line_settings_set_debounce_period_us( | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: you can convert a `u64` to a `u32` and panic if the converted value doesn't fit | 209 | (period.as_micros() as u64).try_into().unwrap(), | + +++++++++++++++++++++ error[E0308]: mismatched types --> libgpiod/src/line_settings.rs:217:13 | 217 | gpiod::gpiod_line_settings_get_debounce_period_us(self.settings) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `u64`, found `u32` | help: you can convert a `u32` to a `u64` | 217 | gpiod::gpiod_line_settings_get_debounce_period_us(self.settings).into() | +++++++ For more information about this error, try `rustc --explain E0308`. error: could not compile `libgpiod` due to 8 previous errors --- An example of one of the relevant generated signatures is: pub fn gpiod_edge_event_buffer_get_event( buffer: *mut gpiod_edge_event_buffer, index: ::std::os::raw::c_ulong, ) -> *mut gpiod_edge_event; On 32-bit c_ulong maps to u32, not u64. Cheers, Kent.
Hi Kent, Thanks a lot for putting so much effort in reviewing the series, yes it makes me invest more time to the bindings, but it is totally worth it. Excellent work. I have pushed all the changed I have made so far in v8 branch in my repository in the top few commits, in case you want to have a look. On 17-10-22, 20:59, Kent Gibson wrote: > On Fri, Oct 14, 2022 at 04:17:20PM +0530, Viresh Kumar wrote: > > +use vmm_sys_util::errno::Error as Errno; > > Why vmm_sys_util::errno::Error rather than, say, errno::Errno? I used it in another project, vhost-device related to virtio, and so ended up using here. The main advantage of using this over errno is that we can do Errno::last() to get the error number of the last operation. Do you have a better alternative to that ? > > +/// GPIO chip > > +/// > > +/// A GPIO chip object is associated with an open file descriptor to the GPIO > > +/// character device. It exposes basic information about the chip and allows > > +/// callers to retrieve information about each line, watch lines for state > > +/// changes and make line requests. > > +#[derive(Clone, Debug, Eq, PartialEq)] > > +pub struct Chip { > > + ichip: Arc<Internal>, > > + info: Info, > > +} > > + > > Doesn't info need an Arc as well? (see struct Info below). Why would that be required ? Internal required it as it can be stored in two structures, Chip and Info, and should get freed after both of them are out of scope. Though Info have a single owner/user, i.e. Chip and once Chip is out of scope, Info can be freed as well. > Actually, does it make sense to be able to Clone the Chip? I have removed Clone now, since Info couldn't be cloned anymore. > Two threads could then watch different lines and > wait_info_event() from the one chip. > That isn't going to work, at least not the way you would want. > > > +unsafe impl Send for Chip {} > > +unsafe impl Sync for Chip {} > > + > > Send makes sense to me, but not Sync, due to the waiting for info events. > Similarly for line_request::Request. > At least not without some additional synchronisation/locking, and I'm not > sure we want to go there. Hmm, I added those for tests and now for the multi-threaded example I have written. And I do get what you are saying, it is tricky to share the same chip from two threads. What should I do? Remove those example/tests and remove Sync ? > > +impl Chip { > > + /// Find a chip by path. > > + pub fn open<P: AsRef<Path>>(path: &P) -> Result<Self> { > > + let ichip = Arc::new(Internal::open(path)?); > > + let info = Info::new(ichip.clone())?; > > + > > info is not always necessary, e.g. if the user just want to request a > line. Maybe lazy load it? Assuming you want to keep a copy cached with > the Chip at all. Yeah, but then I will be required to add an additional check to below routines to see if info is already available or not, and populate it if required. Or detach Info from Chip and remove below wrappers ? > > + Ok(Self { ichip, info }) > > + } > > + > > + /// Get the chip name as represented in the kernel. > > + pub fn name(&self) -> Result<&str> { > > + self.info.name() > > + } > > + > > + /// Get the chip label as represented in the kernel. > > + pub fn label(&self) -> Result<&str> { > > + self.info.label() > > + } > > + > > + /// Get the number of GPIO lines exposed by the chip. > > + pub fn num_lines(&self) -> usize { > > + self.info.num_lines() > > + } > > +impl Buffer { > > + /// Create a new edge event buffer. > > + /// > > + /// If capacity equals 0, it will be set to a default value of 64. If > > + /// capacity is larger than 1024, it will be limited to 1024. > > + pub fn new(capacity: usize) -> Result<Self> { > > + let buffer = unsafe { gpiod::gpiod_edge_event_buffer_new(capacity as c_ulong) }; > > + if buffer.is_null() { > > + return Err(Error::OperationFailed( > > + OperationType::EdgeEventBufferNew, > > + Errno::last(), > > + )); > > + } > > + > > + Ok(Self { buffer }) > > + } > > + > > + /// Private helper, Returns gpiod_edge_event_buffer > > + pub(crate) fn buffer(&self) -> *mut gpiod::gpiod_edge_event_buffer { > > + self.buffer > > + } > > Just make self.buffer pub(crate)? Why is the idea of passing a raw > pointer around making my skin crawl? > Or even better, move the guts of > line_request::Request::read_edge_events() into a helper function here > that it can call so it doesn't need access to self.buffer. But then this piece of code would need to access self.request as the API is: gpiod::gpiod_line_request_read_edge_event( self.request, buffer.buffer, buffer.capacity().try_into().unwrap(), ) It needs raw pointers to both buffer and request. > > + > > + /// Get the capacity of the event buffer. > > + pub fn capacity(&self) -> usize { > > + unsafe { gpiod::gpiod_edge_event_buffer_get_capacity(self.buffer()) as usize } > > + } > > + > > + /// Read an event stored in the buffer. > > + pub fn event(&self, index: u64) -> Result<edge::Event> { > > + edge::Event::new(self, index) > > + } > > What is to prevent reading an event from a slot that has not yet been > populated? I realise doing that is possible in C, but it should be > prevented in the Rust bindings. Good point, I have done this now: event_buffer.rs: /// Set the number of events read into the event buffer. pub fn set_event_count(&mut self, count: usize) { self.event_count = count } /// Read an event stored in the buffer. pub fn event(&self, index: usize) -> Result<request::Event> { if index >= self.event_count { Err(Error::InvalidArguments) } else { request::Event::new(self, index) } } line_request.rs: /// Get a number of edge events from a line request. /// /// This function will block if no event was queued for the line. pub fn read_edge_events(&self, buffer: &mut request::Buffer) -> Result<u32> { // SAFETY: `gpiod_line_request` is guaranteed to be valid here. let ret = unsafe { gpiod::gpiod_line_request_read_edge_event( self.request, buffer.buffer, buffer.capacity().try_into().unwrap(), ) }; if ret == -1 { Err(Error::OperationFailed( OperationType::LineRequestReadEdgeEvent, Errno::last(), )) } else { // Set count of events read in the buffer buffer.set_event_count(ret as usize); Ok(ret as u32) } } Good enough ? > Not sure why event_buffer needs to be nested inside an event module. > event_buffer should be in the request module. > > edge_event is a tougher call, either line or request. Probably the > latter. Is this okay now ? I had to rename chip.rs to chip_internal.rs in order to include info_event within it. mod chip_internal; mod info_event; pub mod chip { /// GPIO chip related definitions. pub use crate::chip_internal::*; pub mod info { /// GPIO chip info event related definitions. pub use crate::info_event::*; } } mod edge_event; mod event_buffer; mod line_request; mod request_config; /// GPIO chip request related definitions. pub mod request { pub use crate::edge_event::*; pub use crate::event_buffer::*; pub use crate::line_request::*; pub use crate::request_config::*; } mod line_config; mod line_info; mod line_settings; /// GPIO chip line related definitions. pub mod line { pub use crate::line_config::*; pub use crate::line_info::*; pub use crate::line_settings::*; } > > +/// Iterator for GPIO devices. > > +pub fn gpiochip_devices<P: AsRef<Path>>(path: &P) -> Result<Vec<chip::Chip>> { > > + let mut chips = Vec::new(); > > + > > + for entry in fs::read_dir(path).map_err(|_| Error::IoError)?.flatten() { > > + let path = entry.path(); > > + > > + if is_gpiochip_device(&path) { > > + chips.push(chip::Chip::open(&path)?); > > + } > > + } > > If there is a symlink to a device both in the path, or multiple symlinks > to the same device, you will get the same chip several times in the > returned Vec? Probably yeah. > Some mention of the order in which devices are returned? Documentation of read_dir() says: The order in which this iterator returns entries is platform and filesystem dependent. mentioned same in comment now. > If you are returning a Vec then sorted might be nice, else state they > are not sorted. What should we here then ? > > +/// Get the API version of the library as a human-readable string. > > +pub fn version_string() -> Result<&'static str> { > > + // SAFETY: The string returned by libgpiod is guaranteed to live forever. > > + let version = unsafe { gpiod::gpiod_version_string() }; > > + > > + if version.is_null() { > > + return Err(Error::NullString("GPIO library version")); > > + } > > + > > + // SAFETY: The string is guaranteed to be valid here by the C API. > > + str::from_utf8(unsafe { slice::from_raw_parts(version as *const u8, strlen(version) as usize) }) > > + .map_err(Error::StringNotUtf8) > > +} > > What if the Rust bindings version differ from the libgpiod version? Hmm, not sure which version should we return here. I think it should just be libgpiod's version. Though I am fine with whatever you and Bartosz decide. > > + /// Get a number of edge events from a line request. > > + /// > > + /// This function will block if no event was queued for the line. > > + pub fn read_edge_events(&self, buffer: &mut edge::event::Buffer) -> Result<u32> { > > What is to stop the user requesting events between the len returned > here and the end of the buffer? Why knows what state they are in. I have shared a change for the same earlier, is that enough ? > Overall I don't see any problems that require major rework to the scale > of previous versions. My main itches that need to be scratched being > dangling pointers and to a lesser degree thread safey. > > I find the event_buffer solution to be less safe than it could be. > I've got an alternative, based on the my earlier PoC/suggestion, that > returns an snaphot of events when you read events from the request into > the buffer. I've put that in a branch on github [1] if you want to take > a look. > > I went though a few variations wrt how to deal with the event cloning. > Not implementing Clone on the events means they can't be stored in a > Vec, and so storing them in the Buffer was problematic. > My initial solution was for the event to only wrap the C pointer and > have distinct event types for buffered and cloned, implemented > using traits - which is bit of a pain as the user then needs to import > the trait themselves to use the events :-(. > Then tried a macro to add impl Event on the two event types, which looks > ugly to me. > Finally went with having Event be a new type of BaseEvent, which does > impl Clone, so the event buffer can store them (and <cough> cast them > to Event as appropriate). Slightly more evil under the hood than I > would like, but provides the cleanest implementation. > > The Events snapshot should impl Iterator to make it easier to use, > though I haven't added that yet. > And the error returned by Events::event() is wrong - just cut and pasted > one in there - probably should be InvalidArguments? > > Anyway, the end result is that it is not possible to read an invalid or > uninitialised event from Rust, which was the point. > It also better handles lifetimes, so it isn't necessary to explicitly > drop events before re-using the buffer, and the event_clone() method > can be an instance method rather than a class method (to borrow some > Python terminology). > I've added an events.rs to the examples to demonstrate how the > changes impact usage in each case. I haven't looked into that in great details yet, just skimmed through it. I am not sure I understand them completely yet and I am not sure if it is worth the complexity, may be it is. I will try to get through the changes again next week though. Will it be possible to get on with the current implementation and once this is merged you send patches on top of it which can then get reviewed properly by others too, who know Rust better than me ?
On 18-10-22, 11:27, Kent Gibson wrote: > One more thing - I overlooked trying to build on 32-bit while reviewing. > (in this case on a Raspberry Pi 4) I missed running this :( cargo build --target arm-unknown-linux-gnueabi I have these changes on the top and yet I have an error which I am not sure how to fix here without special if/else code per target. diff --git a/bindings/rust/gpiosim/src/sim.rs b/bindings/rust/gpiosim/src/sim.rs index 4dd4adfa0d3f..dfa22fbec94b 100644 --- a/bindings/rust/gpiosim/src/sim.rs +++ b/bindings/rust/gpiosim/src/sim.rs @@ -186,7 +186,7 @@ impl SimBank { fn set_num_lines(&self, num: u64) -> Result<()> { // SAFETY: `gpiosim_bank` is guaranteed to be valid here. - let ret = unsafe { gpiosim_bank_set_num_lines(self.bank, num) }; + let ret = unsafe { gpiosim_bank_set_num_lines(self.bank, num.try_into().unwrap()) }; if ret == -1 { Err(Error::OperationFailed( OperationType::SimBankSetNumLines, diff --git a/bindings/rust/libgpiod/src/edge_event.rs b/bindings/rust/libgpiod/src/edge_event.rs index 0d328ebb2b03..87c5af228e2b 100644 --- a/bindings/rust/libgpiod/src/edge_event.rs +++ b/bindings/rust/libgpiod/src/edge_event.rs @@ -76,7 +76,7 @@ impl<'b> Event<'b> { /// associated line request. pub fn global_seqno(&self) -> u64 { // SAFETY: `gpiod_edge_event` is guaranteed to be valid here. - unsafe { gpiod::gpiod_edge_event_get_global_seqno(self.event) } + unsafe { gpiod::gpiod_edge_event_get_global_seqno(self.event) as u64 } } /// Get the event sequence number specific to concerned line. @@ -85,7 +85,7 @@ impl<'b> Event<'b> { /// lifetime of the associated line request. pub fn line_seqno(&self) -> u64 { // SAFETY: `gpiod_edge_event` is guaranteed to be valid here. - unsafe { gpiod::gpiod_edge_event_get_line_seqno(self.event) } + unsafe { gpiod::gpiod_edge_event_get_line_seqno(self.event) as u64 } } } diff --git a/bindings/rust/libgpiod/src/line_info.rs b/bindings/rust/libgpiod/src/line_info.rs index a458e72e0b12..aa1824cf3e80 100644 --- a/bindings/rust/libgpiod/src/line_info.rs +++ b/bindings/rust/libgpiod/src/line_info.rs @@ -144,7 +144,9 @@ impl Info { /// Get the debounce period of the line. pub fn debounce_period(&self) -> Duration { // SAFETY: `gpiod_line_info` is guaranteed to be valid here. - Duration::from_micros(unsafe { gpiod::gpiod_line_info_get_debounce_period_us(self.info) }) + Duration::from_micros(unsafe { + gpiod::gpiod_line_info_get_debounce_period_us(self.info).into() + }) } } diff --git a/bindings/rust/libgpiod/src/line_settings.rs b/bindings/rust/libgpiod/src/line_settings.rs index d422ad1ea68f..4df3efb1a93f 100644 --- a/bindings/rust/libgpiod/src/line_settings.rs +++ b/bindings/rust/libgpiod/src/line_settings.rs @@ -220,7 +220,7 @@ impl Settings { unsafe { gpiod::gpiod_line_settings_set_debounce_period_us( self.settings, - period.as_micros() as u64, + (period.as_micros() as usize).try_into().unwrap(), ); } @@ -231,7 +231,7 @@ impl Settings { pub fn debounce_period(&self) -> Result<Duration> { // SAFETY: `gpiod_line_settings` is guaranteed to be valid here. Ok(Duration::from_micros(unsafe { - gpiod::gpiod_line_settings_get_debounce_period_us(self.settings) + gpiod::gpiod_line_settings_get_debounce_period_us(self.settings).into() })) } diff --git a/bindings/rust/libgpiod/tests/chip.rs b/bindings/rust/libgpiod/tests/chip.rs index aaee910e457f..a7fdaadf4da1 100644 --- a/bindings/rust/libgpiod/tests/chip.rs +++ b/bindings/rust/libgpiod/tests/chip.rs @@ -90,7 +90,10 @@ mod chip { // Failure assert_eq!( chip.line_offset_from_name("nonexistent").unwrap_err(), - ChipError::OperationFailed(OperationType::ChipGetLineOffsetFromName, Errno::new(ENOENT)) + ChipError::OperationFailed( + OperationType::ChipGetLineOffsetFromName, + Errno::new(ENOENT), + ) ); } } $ -------------------------8<------------------------- Current failures: error[E0308]: mismatched types --> libgpiod/src/line_config.rs:105:78 | 105 | let ret = unsafe { gpiod::gpiod_line_config_get_offsets(self.config, &mut num, &mut ptr) }; | ------------------------------------ ^^^^^^^^ expected `u32`, found `u64` | | | arguments to this function are incorrect | = note: expected raw pointer `*mut u32` found mutable reference `&mut u64` note: function defined here --> /mnt/ssd/all/work/repos/virtio/rust/libgpiod/bindings/rust/libgpiod-sys/src/bindings.rs:847:12 | 847 | pub fn gpiod_line_config_get_offsets( | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ For more information about this error, try `rustc --explain E0308`. error: could not compile `libgpiod` due to previous error
On Fri, Oct 21, 2022 at 04:52:38PM +0530, Viresh Kumar wrote: > Hi Kent, > > Thanks a lot for putting so much effort in reviewing the series, yes > it makes me invest more time to the bindings, but it is totally worth > it. Excellent work. > > I have pushed all the changed I have made so far in v8 branch in my > repository in the top few commits, in case you want to have a look. > > On 17-10-22, 20:59, Kent Gibson wrote: > > On Fri, Oct 14, 2022 at 04:17:20PM +0530, Viresh Kumar wrote: > > > +use vmm_sys_util::errno::Error as Errno; > > > > Why vmm_sys_util::errno::Error rather than, say, errno::Errno? > > I used it in another project, vhost-device related to virtio, and so > ended up using here. The main advantage of using this over errno is > that we can do Errno::last() to get the error number of the last > operation. > > Do you have a better alternative to that ? > The one I mentioned is the top match for errno on crates.io[1]. That provides the same functionality, and that is all it does. All other things being equal I would go for the simplest dependency. [1] https://crates.io/crates/errno > > > +/// GPIO chip > > > +/// > > > +/// A GPIO chip object is associated with an open file descriptor to the GPIO > > > +/// character device. It exposes basic information about the chip and allows > > > +/// callers to retrieve information about each line, watch lines for state > > > +/// changes and make line requests. > > > +#[derive(Clone, Debug, Eq, PartialEq)] > > > +pub struct Chip { > > > + ichip: Arc<Internal>, > > > + info: Info, > > > +} > > > + > > > > Doesn't info need an Arc as well? (see struct Info below). > > Why would that be required ? Internal required it as it can be stored > in two structures, Chip and Info, and should get freed after both of > them are out of scope. > > Though Info have a single owner/user, i.e. Chip and once Chip is out > of scope, Info can be freed as well. > > > Actually, does it make sense to be able to Clone the Chip? > > I have removed Clone now, since Info couldn't be cloned anymore. > Exactly - if Info can't clone then you could Arc it. But if the Chip can't clone then problem solved. > > Two threads could then watch different lines and > > wait_info_event() from the one chip. > > That isn't going to work, at least not the way you would want. > > > > > +unsafe impl Send for Chip {} > > > +unsafe impl Sync for Chip {} > > > + > > > > Send makes sense to me, but not Sync, due to the waiting for info events. > > Similarly for line_request::Request. > > At least not without some additional synchronisation/locking, and I'm not > > sure we want to go there. > > Hmm, I added those for tests and now for the multi-threaded example I > have written. And I do get what you are saying, it is tricky to share > the same chip from two threads. > > What should I do? Remove those example/tests and remove Sync ? > I would remove Sync support until you are satisfied you have a working solution for it. > > > +impl Chip { > > > + /// Find a chip by path. > > > + pub fn open<P: AsRef<Path>>(path: &P) -> Result<Self> { > > > + let ichip = Arc::new(Internal::open(path)?); > > > + let info = Info::new(ichip.clone())?; > > > + > > > > info is not always necessary, e.g. if the user just want to request a > > line. Maybe lazy load it? Assuming you want to keep a copy cached with > > the Chip at all. > > Yeah, but then I will be required to add an additional check to below > routines to see if info is already available or not, and populate it > if required. > > Or detach Info from Chip and remove below wrappers ? > The usual way is to add a helper function that returns the lazy loaded object. If it hasn't been loaded yet then it does that first. And your wrapper functions use that to access the info. Or, as you suggest, just let the user get the Info and use its accessors. Either way works for me. > > > + Ok(Self { ichip, info }) > > > + } > > > + > > > + /// Get the chip name as represented in the kernel. > > > + pub fn name(&self) -> Result<&str> { > > > + self.info.name() > > > + } > > > + > > > + /// Get the chip label as represented in the kernel. > > > + pub fn label(&self) -> Result<&str> { > > > + self.info.label() > > > + } > > > + > > > + /// Get the number of GPIO lines exposed by the chip. > > > + pub fn num_lines(&self) -> usize { > > > + self.info.num_lines() > > > + } > > > > > +impl Buffer { > > > + /// Create a new edge event buffer. > > > + /// > > > + /// If capacity equals 0, it will be set to a default value of 64. If > > > + /// capacity is larger than 1024, it will be limited to 1024. > > > + pub fn new(capacity: usize) -> Result<Self> { > > > + let buffer = unsafe { gpiod::gpiod_edge_event_buffer_new(capacity as c_ulong) }; > > > + if buffer.is_null() { > > > + return Err(Error::OperationFailed( > > > + OperationType::EdgeEventBufferNew, > > > + Errno::last(), > > > + )); > > > + } > > > + > > > + Ok(Self { buffer }) > > > + } > > > + > > > + /// Private helper, Returns gpiod_edge_event_buffer > > > + pub(crate) fn buffer(&self) -> *mut gpiod::gpiod_edge_event_buffer { > > > + self.buffer > > > + } > > > > Just make self.buffer pub(crate)? Why is the idea of passing a raw > > pointer around making my skin crawl? > > Or even better, move the guts of > > line_request::Request::read_edge_events() into a helper function here > > that it can call so it doesn't need access to self.buffer. > > But then this piece of code would need to access self.request as the > API is: > > gpiod::gpiod_line_request_read_edge_event( > self.request, > buffer.buffer, > buffer.capacity().try_into().unwrap(), > ) > Yep - you pass the Request to the Buffer, rather than have the Request doing the work on the buffer. IIRC my branch includes my version of this. > It needs raw pointers to both buffer and request. > > > > + > > > + /// Get the capacity of the event buffer. > > > + pub fn capacity(&self) -> usize { > > > + unsafe { gpiod::gpiod_edge_event_buffer_get_capacity(self.buffer()) as usize } > > > + } > > > + > > > + /// Read an event stored in the buffer. > > > + pub fn event(&self, index: u64) -> Result<edge::Event> { > > > + edge::Event::new(self, index) > > > + } > > > > What is to prevent reading an event from a slot that has not yet been > > populated? I realise doing that is possible in C, but it should be > > prevented in the Rust bindings. > > Good point, I have done this now: > > event_buffer.rs: > > /// Set the number of events read into the event buffer. > pub fn set_event_count(&mut self, count: usize) { > self.event_count = count > } > pub(crate) else the user can mess with the threshold. > /// Read an event stored in the buffer. > pub fn event(&self, index: usize) -> Result<request::Event> { > if index >= self.event_count { > Err(Error::InvalidArguments) > } else { > request::Event::new(self, index) > } > } > > line_request.rs: > > /// Get a number of edge events from a line request. > /// > /// This function will block if no event was queued for the line. > pub fn read_edge_events(&self, buffer: &mut request::Buffer) -> Result<u32> { > // SAFETY: `gpiod_line_request` is guaranteed to be valid here. > let ret = unsafe { > gpiod::gpiod_line_request_read_edge_event( > self.request, > buffer.buffer, > buffer.capacity().try_into().unwrap(), > ) > }; > > if ret == -1 { > Err(Error::OperationFailed( > OperationType::LineRequestReadEdgeEvent, > Errno::last(), > )) > } else { > // Set count of events read in the buffer > buffer.set_event_count(ret as usize); > Ok(ret as u32) > } > } > > > Good enough ? > Better. > > Not sure why event_buffer needs to be nested inside an event module. > > event_buffer should be in the request module. > > > > edge_event is a tougher call, either line or request. Probably the > > latter. > > Is this okay now ? I had to rename chip.rs to chip_internal.rs in > order to include info_event within it. > > mod chip_internal; > mod info_event; > > pub mod chip { > /// GPIO chip related definitions. > pub use crate::chip_internal::*; > > pub mod info { > /// GPIO chip info event related definitions. > pub use crate::info_event::*; > } > } > Couldn't you just add those pubs to the old chip file? So you don't need to rename it to chip_internal. > mod edge_event; > mod event_buffer; > mod line_request; > mod request_config; > > /// GPIO chip request related definitions. > pub mod request { > pub use crate::edge_event::*; > pub use crate::event_buffer::*; > pub use crate::line_request::*; > pub use crate::request_config::*; > } > > mod line_config; > mod line_info; > mod line_settings; > > /// GPIO chip line related definitions. > pub mod line { > pub use crate::line_config::*; > pub use crate::line_info::*; > pub use crate::line_settings::*; > } > > > > +/// Iterator for GPIO devices. > > > +pub fn gpiochip_devices<P: AsRef<Path>>(path: &P) -> Result<Vec<chip::Chip>> { > > > + let mut chips = Vec::new(); > > > + > > > + for entry in fs::read_dir(path).map_err(|_| Error::IoError)?.flatten() { > > > + let path = entry.path(); > > > + > > > + if is_gpiochip_device(&path) { > > > + chips.push(chip::Chip::open(&path)?); > > > + } > > > + } > > > > If there is a symlink to a device both in the path, or multiple symlinks > > to the same device, you will get the same chip several times in the > > returned Vec? > > Probably yeah. > Lets say I want to implement a search for line by name, and check that the line name is unique. That will fail if there is a symlink to the relevant device, cos it will see a spurious duplicate. How would I do that then? > > Some mention of the order in which devices are returned? > > Documentation of read_dir() says: > > The order in which this iterator returns entries is platform and filesystem dependent. > > mentioned same in comment now. > > > If you are returning a Vec then sorted might be nice, else state they > > are not sorted. > > What should we here then ? >
On Thu, Oct 27, 2022 at 02:09:31PM +0200, Bartosz Golaszewski wrote: > On Fri, Oct 21, 2022 at 2:39 PM Kent Gibson <warthog618@gmail.com> wrote: > > > > On Fri, Oct 21, 2022 at 04:52:38PM +0530, Viresh Kumar wrote: > > > Hi Kent, > > > > > > > > > > > What if the Rust bindings version differ from the libgpiod version? > > > > > > Hmm, not sure which version should we return here. I think it should > > > just be libgpiod's version. Though I am fine with whatever you and > > > Bartosz decide. > > > > > > > You need both - separately. > > > > For C++ and Python, the API version stays consistent across all three > trees for simplicity. There are separate ABI numbers for libgpiod, > libgpiodcxx and libgpiosim. Rust doesn't seem to have this kind of > differentiation between ABI and API so I'd say we should have a single > API version but see below about the idea for the contrib/ directory. > My thinking was that if the Rust bindings end up being compiled into the app then it may be difficult to identify the version it was built with, as opposed to the version of libgpiod it is running against. Not sure you can always determine that from the Cargo.toml or Cargo.lock, or that those will always be available after the fact. Not a biggy if the two always remain compatible, but if a libgpiod change inadvertently triggers some fault in the Rust bindings then it could be a PITA just to determine the versions involved. > > > Will it be possible to get on with the current implementation and once > > > this is merged you send patches on top of it which can then get > > > reviewed properly by others too, who know Rust better than me ? > > > > > > > The Rust bindings are your baby, and I don't have any plans to be > > patching it. And Bart is the arbiter of what gets merged, so that > > decision is his. > > > > I was thinking about it lately and figured that - since I don't know > Rust very well and cannot reliably maintain that part myself - how > about we create a 'contrib' directory in the libgpiod tree for this > kind of "third-party" bindings? The requirements for stability and > correctness in there would be more relaxed compared to the main tree? > The problem there is you thinking that you need to be able to personally support every line of libgpiod, and that it is not a team effort. Wrt the Rust bindings, I was assuming that either Viresh would provide support, or as his work appears to be on behalf of Linaro that they would have an interest in maintaining it. Similarly, I was assuming that I would be providing support for the proposed tool changes, should they ever get merged. Though, based on the little feedback to date, I'm dubious about that ever happening. I could always split them off into a separate project, but maintaining an autoconf based setup doesn't interest me, particularly when the Go and Rust alternatives are one-liners that just work, so probably not. I was thinking that some forum for libgpiod, or more broadly GPIO user space development, would help increase community involvement, in both directions, and that might help address the Bartosz doesn't scale problem. But I don't have anything useful to add on that. The only relevant experience I have is with github, and while it would probably suffice I would explore other options first, with gitlab being the most obvious alternative. No help there, I know. Sorry for not replying sooner. I was hoping to do a respin in a more positive light, but that has eluded me and time marches on. Cheers, Kent.
On Mon, Oct 31, 2022 at 1:34 PM Kent Gibson <warthog618@gmail.com> wrote: > > On Thu, Oct 27, 2022 at 02:09:31PM +0200, Bartosz Golaszewski wrote: > > On Fri, Oct 21, 2022 at 2:39 PM Kent Gibson <warthog618@gmail.com> wrote: > > > > > > On Fri, Oct 21, 2022 at 04:52:38PM +0530, Viresh Kumar wrote: > > > > Hi Kent, > > > > > > > > > > > > > > What if the Rust bindings version differ from the libgpiod version? > > > > > > > > Hmm, not sure which version should we return here. I think it should > > > > just be libgpiod's version. Though I am fine with whatever you and > > > > Bartosz decide. > > > > > > > > > > You need both - separately. > > > > > > > For C++ and Python, the API version stays consistent across all three > > trees for simplicity. There are separate ABI numbers for libgpiod, > > libgpiodcxx and libgpiosim. Rust doesn't seem to have this kind of > > differentiation between ABI and API so I'd say we should have a single > > API version but see below about the idea for the contrib/ directory. > > > > My thinking was that if the Rust bindings end up being compiled into the > app then it may be difficult to identify the version it was built with, > as opposed to the version of libgpiod it is running against. > Not sure you can always determine that from the Cargo.toml or > Cargo.lock, or that those will always be available after the fact. > > Not a biggy if the two always remain compatible, but if a libgpiod change > inadvertently triggers some fault in the Rust bindings then it could be a > PITA just to determine the versions involved. > > > > > Will it be possible to get on with the current implementation and once > > > > this is merged you send patches on top of it which can then get > > > > reviewed properly by others too, who know Rust better than me ? > > > > > > > > > > The Rust bindings are your baby, and I don't have any plans to be > > > patching it. And Bart is the arbiter of what gets merged, so that > > > decision is his. > > > > > > > I was thinking about it lately and figured that - since I don't know > > Rust very well and cannot reliably maintain that part myself - how > > about we create a 'contrib' directory in the libgpiod tree for this > > kind of "third-party" bindings? The requirements for stability and > > correctness in there would be more relaxed compared to the main tree? > > > > The problem there is you thinking that you need to be able to personally > support every line of libgpiod, and that it is not a team effort. > > Wrt the Rust bindings, I was assuming that either Viresh would provide > support, or as his work appears to be on behalf of Linaro that they > would have an interest in maintaining it. > > Similarly, I was assuming that I would be providing support for the > proposed tool changes, should they ever get merged. Though, based on the > little feedback to date, I'm dubious about that ever happening. Hey, this is literally the next item on my TODO list. I've just merged the v2 API into master and am getting ready to dive head-first into your patches. Bart > I could always split them off into a separate project, but maintaining > an autoconf based setup doesn't interest me, particularly when the Go and > Rust alternatives are one-liners that just work, so probably not. > > I was thinking that some forum for libgpiod, or more broadly GPIO > user space development, would help increase community involvement, > in both directions, and that might help address the Bartosz doesn't > scale problem. But I don't have anything useful to add on that. > The only relevant experience I have is with github, and while it would > probably suffice I would explore other options first, with gitlab > being the most obvious alternative. No help there, I know. > > Sorry for not replying sooner. I was hoping to do a respin in a more > positive light, but that has eluded me and time marches on. > > Cheers, > Kent.
On 31-10-22, 20:33, Kent Gibson wrote: > Wrt the Rust bindings, I was assuming that either Viresh would provide > support, or as his work appears to be on behalf of Linaro that they > would have an interest in maintaining it. I will surely help in maintaining the Rust part. Not an issue.
On Wed, Nov 2, 2022 at 5:00 AM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > On 31-10-22, 20:33, Kent Gibson wrote: > > Wrt the Rust bindings, I was assuming that either Viresh would provide > > support, or as his work appears to be on behalf of Linaro that they > > would have an interest in maintaining it. > > I will surely help in maintaining the Rust part. Not an issue. > Sounds like a plan. I'm going through Kent's gpio-tools patches and rust bindings are next in line. Bart
On Wed, Nov 02, 2022 at 01:47:44PM +0100, Bartosz Golaszewski wrote: > On Wed, Nov 2, 2022 at 5:00 AM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > > > On 31-10-22, 20:33, Kent Gibson wrote: > > > Wrt the Rust bindings, I was assuming that either Viresh would provide > > > support, or as his work appears to be on behalf of Linaro that they > > > would have an interest in maintaining it. > > > > I will surely help in maintaining the Rust part. Not an issue. > > > > Sounds like a plan. I'm going through Kent's gpio-tools patches and > rust bindings are next in line. > The rust bindings might make it in before they do - you may find that they are a bit more different from the old tools than you were expecting. For my part I'm just wrapping up a quick pass over rust v8, before looking for where the python landed. Cheers, Kent.
On Wed, Nov 2, 2022 at 2:08 PM Kent Gibson <warthog618@gmail.com> wrote: > > On Wed, Nov 02, 2022 at 01:47:44PM +0100, Bartosz Golaszewski wrote: > > On Wed, Nov 2, 2022 at 5:00 AM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > > > > > On 31-10-22, 20:33, Kent Gibson wrote: > > > > Wrt the Rust bindings, I was assuming that either Viresh would provide > > > > support, or as his work appears to be on behalf of Linaro that they > > > > would have an interest in maintaining it. > > > > > > I will surely help in maintaining the Rust part. Not an issue. > > > > > > > Sounds like a plan. I'm going through Kent's gpio-tools patches and > > rust bindings are next in line. > > > > The rust bindings might make it in before they do - you may find that > they are a bit more different from the old tools than you were expecting. > Yeah, I can tell that. :) I have lots of non-functional minor things to point out - mostly coding style etc. - that I will probably just fix locally when applying. One thing that stands out though is the dependency on libedit - do you think we could make the gpioset interactive mode a configurable feature with its own configure switch that could be left out if we don't want to depend on any external libraries? Bart > For my part I'm just wrapping up a quick pass over rust v8, before > looking for where the python landed. > > Cheers, > Kent. >
On Wed, Nov 02, 2022 at 05:34:20PM +0100, Bartosz Golaszewski wrote: > On Wed, Nov 2, 2022 at 2:08 PM Kent Gibson <warthog618@gmail.com> wrote: > > > > On Wed, Nov 02, 2022 at 01:47:44PM +0100, Bartosz Golaszewski wrote: > > > On Wed, Nov 2, 2022 at 5:00 AM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > > > > > > > On 31-10-22, 20:33, Kent Gibson wrote: > > > > > Wrt the Rust bindings, I was assuming that either Viresh would provide > > > > > support, or as his work appears to be on behalf of Linaro that they > > > > > would have an interest in maintaining it. > > > > > > > > I will surely help in maintaining the Rust part. Not an issue. > > > > > > > > > > Sounds like a plan. I'm going through Kent's gpio-tools patches and > > > rust bindings are next in line. > > > > > > > The rust bindings might make it in before they do - you may find that > > they are a bit more different from the old tools than you were expecting. > > > > Yeah, I can tell that. :) > > I have lots of non-functional minor things to point out - mostly > coding style etc. - that I will probably just fix locally when > applying. > Looks like there will be a v5 for the optional interactive set, if nothing else, so let me know and I can fix them as well. > One thing that stands out though is the dependency on libedit - do you > think we could make the gpioset interactive mode a configurable > feature with its own configure switch that could be left out if we > don't want to depend on any external libraries? Well, libedit was your preferred choice. But, yeah, making the interactive mode optional is a good idea. Any preference on the name for the config flag? --enable-gpioset-interactive ? One other major thing I had been considering, and implemented in my Rust version[1], is renaming the tools, since it isn't intuitively obvious, to me anyway, what gpiodetect, gpioinfo, gpiomon, and gpiowatch do. I went with: gpiodetect -> gpiochip gpioinfo -> gpioline gpiomon -> gpioedges gpiowatch -> gpionotify where the names try to indicate the information returned, rather than how it is collected. And yeah, I got stuck with gpiowatch/gpionotify as I couldn't come up with a short meaningful name for line info changed events. Suggestions welcome. And lice is not an option. gpioget and gpioset remain unchanged as they are already as clear as can be. Would that work for you, or would you prefer to stick with the existing names? Cheers, Kent. [1] https://crates.io/crates/gpiocdev-cli
On Thu, Nov 3, 2022 at 1:38 AM Kent Gibson <warthog618@gmail.com> wrote: > > On Wed, Nov 02, 2022 at 05:34:20PM +0100, Bartosz Golaszewski wrote: > > On Wed, Nov 2, 2022 at 2:08 PM Kent Gibson <warthog618@gmail.com> wrote: > > > > > > On Wed, Nov 02, 2022 at 01:47:44PM +0100, Bartosz Golaszewski wrote: > > > > On Wed, Nov 2, 2022 at 5:00 AM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > > > > > > > > > On 31-10-22, 20:33, Kent Gibson wrote: > > > > > > Wrt the Rust bindings, I was assuming that either Viresh would provide > > > > > > support, or as his work appears to be on behalf of Linaro that they > > > > > > would have an interest in maintaining it. > > > > > > > > > > I will surely help in maintaining the Rust part. Not an issue. > > > > > > > > > > > > > Sounds like a plan. I'm going through Kent's gpio-tools patches and > > > > rust bindings are next in line. > > > > > > > > > > The rust bindings might make it in before they do - you may find that > > > they are a bit more different from the old tools than you were expecting. > > > > > > > Yeah, I can tell that. :) > > > > I have lots of non-functional minor things to point out - mostly > > coding style etc. - that I will probably just fix locally when > > applying. > > > > Looks like there will be a v5 for the optional interactive set, if > nothing else, so let me know and I can fix them as well. > > > One thing that stands out though is the dependency on libedit - do you > > think we could make the gpioset interactive mode a configurable > > feature with its own configure switch that could be left out if we > > don't want to depend on any external libraries? > > Well, libedit was your preferred choice. It's still is - it's not about the choice of the library but about keeping it possible to build a set of command-line tools with no dependencies other than libc. > But, yeah, making the interactive mode optional is a good idea. > > Any preference on the name for the config flag? > --enable-gpioset-interactive ? > Sounds great. > One other major thing I had been considering, and implemented in my Rust > version[1], is renaming the tools, since it isn't intuitively obvious, > to me anyway, what gpiodetect, gpioinfo, gpiomon, and gpiowatch do. > > I went with: > gpiodetect -> gpiochip > gpioinfo -> gpioline > gpiomon -> gpioedges > gpiowatch -> gpionotify > > where the names try to indicate the information returned, rather than > how it is collected. > And yeah, I got stuck with gpiowatch/gpionotify as I couldn't come up > with a short meaningful name for line info changed events. > Suggestions welcome. And lice is not an option. > > gpioget and gpioset remain unchanged as they are already as clear as can > be. > > Would that work for you, or would you prefer to stick with the existing > names? I don't know if it is because I'm used to the previous names but none of the first three make much sense to me. I think it's better for the names to indicate what the program is doing, not what it's returning. On the other hand - gpionotify is great, go for it! Bart > > Cheers, > Kent. > > [1] https://crates.io/crates/gpiocdev-cli
On Thu, Nov 03, 2022 at 09:35:33AM +0100, Bartosz Golaszewski wrote: > On Thu, Nov 3, 2022 at 1:38 AM Kent Gibson <warthog618@gmail.com> wrote: > > > > On Wed, Nov 02, 2022 at 05:34:20PM +0100, Bartosz Golaszewski wrote: > > > On Wed, Nov 2, 2022 at 2:08 PM Kent Gibson <warthog618@gmail.com> wrote: > > > > > > > > On Wed, Nov 02, 2022 at 01:47:44PM +0100, Bartosz Golaszewski wrote: > > > > > On Wed, Nov 2, 2022 at 5:00 AM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > > > > > > > > > > > On 31-10-22, 20:33, Kent Gibson wrote: > > > > > > > Wrt the Rust bindings, I was assuming that either Viresh would provide > > > > > > > support, or as his work appears to be on behalf of Linaro that they > > > > > > > would have an interest in maintaining it. > > > > > > > > > > > > I will surely help in maintaining the Rust part. Not an issue. > > > > > > > > > > > > > > > > Sounds like a plan. I'm going through Kent's gpio-tools patches and > > > > > rust bindings are next in line. > > > > > > > > > > > > > The rust bindings might make it in before they do - you may find that > > > > they are a bit more different from the old tools than you were expecting. > > > > > > > > > > Yeah, I can tell that. :) > > > > > > I have lots of non-functional minor things to point out - mostly > > > coding style etc. - that I will probably just fix locally when > > > applying. > > > > > > > Looks like there will be a v5 for the optional interactive set, if > > nothing else, so let me know and I can fix them as well. > > > > > One thing that stands out though is the dependency on libedit - do you > > > think we could make the gpioset interactive mode a configurable > > > feature with its own configure switch that could be left out if we > > > don't want to depend on any external libraries? > > > > Well, libedit was your preferred choice. > > It's still is - it's not about the choice of the library but about > keeping it possible to build a set of command-line tools with no > dependencies other than libc. > > > But, yeah, making the interactive mode optional is a good idea. > > > > Any preference on the name for the config flag? > > --enable-gpioset-interactive ? > > > > Sounds great. > > > One other major thing I had been considering, and implemented in my Rust > > version[1], is renaming the tools, since it isn't intuitively obvious, > > to me anyway, what gpiodetect, gpioinfo, gpiomon, and gpiowatch do. > > > > I went with: > > gpiodetect -> gpiochip > > gpioinfo -> gpioline > > gpiomon -> gpioedges > > gpiowatch -> gpionotify > > > > where the names try to indicate the information returned, rather than > > how it is collected. > > And yeah, I got stuck with gpiowatch/gpionotify as I couldn't come up > > with a short meaningful name for line info changed events. > > Suggestions welcome. And lice is not an option. > > > > gpioget and gpioset remain unchanged as they are already as clear as can > > be. > > > > Would that work for you, or would you prefer to stick with the existing > > names? > > I don't know if it is because I'm used to the previous names but none > of the first three make much sense to me. I think it's better for the > names to indicate what the program is doing, not what it's returning. > > On the other hand - gpionotify is great, go for it! > Ok, now I'm confused - only rename gpiowatch? Anyway, we probably should pick this conversation up as part of the tools review, not the Rust bindings, so let me know there. Cheers, Kent.
On Thu, Nov 3, 2022 at 1:29 PM Kent Gibson <warthog618@gmail.com> wrote: > > On Thu, Nov 03, 2022 at 09:35:33AM +0100, Bartosz Golaszewski wrote: > > On Thu, Nov 3, 2022 at 1:38 AM Kent Gibson <warthog618@gmail.com> wrote: > > > > > > On Wed, Nov 02, 2022 at 05:34:20PM +0100, Bartosz Golaszewski wrote: > > > > On Wed, Nov 2, 2022 at 2:08 PM Kent Gibson <warthog618@gmail.com> wrote: > > > > > > > > > > On Wed, Nov 02, 2022 at 01:47:44PM +0100, Bartosz Golaszewski wrote: > > > > > > On Wed, Nov 2, 2022 at 5:00 AM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > > > > > > > > > > > > > On 31-10-22, 20:33, Kent Gibson wrote: > > > > > > > > Wrt the Rust bindings, I was assuming that either Viresh would provide > > > > > > > > support, or as his work appears to be on behalf of Linaro that they > > > > > > > > would have an interest in maintaining it. > > > > > > > > > > > > > > I will surely help in maintaining the Rust part. Not an issue. > > > > > > > > > > > > > > > > > > > Sounds like a plan. I'm going through Kent's gpio-tools patches and > > > > > > rust bindings are next in line. > > > > > > > > > > > > > > > > The rust bindings might make it in before they do - you may find that > > > > > they are a bit more different from the old tools than you were expecting. > > > > > > > > > > > > > Yeah, I can tell that. :) > > > > > > > > I have lots of non-functional minor things to point out - mostly > > > > coding style etc. - that I will probably just fix locally when > > > > applying. > > > > > > > > > > Looks like there will be a v5 for the optional interactive set, if > > > nothing else, so let me know and I can fix them as well. > > > > > > > One thing that stands out though is the dependency on libedit - do you > > > > think we could make the gpioset interactive mode a configurable > > > > feature with its own configure switch that could be left out if we > > > > don't want to depend on any external libraries? > > > > > > Well, libedit was your preferred choice. > > > > It's still is - it's not about the choice of the library but about > > keeping it possible to build a set of command-line tools with no > > dependencies other than libc. > > > > > But, yeah, making the interactive mode optional is a good idea. > > > > > > Any preference on the name for the config flag? > > > --enable-gpioset-interactive ? > > > > > > > Sounds great. > > > > > One other major thing I had been considering, and implemented in my Rust > > > version[1], is renaming the tools, since it isn't intuitively obvious, > > > to me anyway, what gpiodetect, gpioinfo, gpiomon, and gpiowatch do. > > > > > > I went with: > > > gpiodetect -> gpiochip > > > gpioinfo -> gpioline > > > gpiomon -> gpioedges > > > gpiowatch -> gpionotify > > > > > > where the names try to indicate the information returned, rather than > > > how it is collected. > > > And yeah, I got stuck with gpiowatch/gpionotify as I couldn't come up > > > with a short meaningful name for line info changed events. > > > Suggestions welcome. And lice is not an option. > > > > > > gpioget and gpioset remain unchanged as they are already as clear as can > > > be. > > > > > > Would that work for you, or would you prefer to stick with the existing > > > names? > > > > I don't know if it is because I'm used to the previous names but none > > of the first three make much sense to me. I think it's better for the > > names to indicate what the program is doing, not what it's returning. > > > > On the other hand - gpionotify is great, go for it! > > > > Ok, now I'm confused - only rename gpiowatch? Yes, gpiowatch -> gpionotify. > > Anyway, we probably should pick this conversation up as part of the > tools review, not the Rust bindings, so let me know there. > Makes sense. I'm off most of this week, but I will get there, I promise. :) Bartosz
diff --git a/bindings/rust/Cargo.toml b/bindings/rust/Cargo.toml index d0b3a3c88ff1..1e57ef2c0002 100644 --- a/bindings/rust/Cargo.toml +++ b/bindings/rust/Cargo.toml @@ -2,4 +2,5 @@ members = [ "libgpiod-sys", + "libgpiod" ] diff --git a/bindings/rust/libgpiod/Cargo.toml b/bindings/rust/libgpiod/Cargo.toml new file mode 100644 index 000000000000..f25242abb153 --- /dev/null +++ b/bindings/rust/libgpiod/Cargo.toml @@ -0,0 +1,13 @@ +[package] +name = "libgpiod" +version = "0.1.0" +edition = "2018" + +# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html + +[dependencies] +intmap = "2.0.0" +libc = ">=0.2.39" +libgpiod-sys = { path = "../libgpiod-sys" } +thiserror = "1.0" +vmm-sys-util = "=0.10.0" diff --git a/bindings/rust/libgpiod/src/chip.rs b/bindings/rust/libgpiod/src/chip.rs new file mode 100644 index 000000000000..4f52c3f141f4 --- /dev/null +++ b/bindings/rust/libgpiod/src/chip.rs @@ -0,0 +1,253 @@ +// SPDX-License-Identifier: Apache-2.0 AND BSD-3-Clause +// +// Copyright 2022 Linaro Ltd. All Rights Reserved. +// Viresh Kumar <viresh.kumar@linaro.org> + +use libc::strlen; +use std::os::raw::c_char; +use std::path::Path; +use std::sync::Arc; +use std::time::Duration; +use std::{slice, str}; + +use vmm_sys_util::errno::Error as Errno; + +use super::{gpiod, info, line, request, Error, Offset, OperationType, Result}; + +#[derive(Clone, Debug, Eq, PartialEq)] +pub(crate) struct Internal { + chip: *mut gpiod::gpiod_chip, +} + +impl Internal { + /// Find a chip by path. + pub(crate) fn open<P: AsRef<Path>>(path: &P) -> Result<Self> { + // Null-terminate the string + let path = path.as_ref().to_string_lossy() + "\0"; + + let chip = unsafe { gpiod::gpiod_chip_open(path.as_ptr() as *const c_char) }; + if chip.is_null() { + return Err(Error::OperationFailed( + OperationType::ChipOpen, + Errno::last(), + )); + } + + Ok(Self { chip }) + } + + /// Private helper, Returns gpiod_chip + pub(crate) fn chip(&self) -> *mut gpiod::gpiod_chip { + self.chip + } +} + +impl Drop for Internal { + /// Close the chip and release all associated resources. + fn drop(&mut self) { + unsafe { gpiod::gpiod_chip_close(self.chip) } + } +} + +/// GPIO chip +/// +/// A GPIO chip object is associated with an open file descriptor to the GPIO +/// character device. It exposes basic information about the chip and allows +/// callers to retrieve information about each line, watch lines for state +/// changes and make line requests. +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct Chip { + ichip: Arc<Internal>, + info: Info, +} + +unsafe impl Send for Chip {} +unsafe impl Sync for Chip {} + +impl Chip { + /// Find a chip by path. + pub fn open<P: AsRef<Path>>(path: &P) -> Result<Self> { + let ichip = Arc::new(Internal::open(path)?); + let info = Info::new(ichip.clone())?; + + Ok(Self { ichip, info }) + } + + /// Get the chip name as represented in the kernel. + pub fn name(&self) -> Result<&str> { + self.info.name() + } + + /// Get the chip label as represented in the kernel. + pub fn label(&self) -> Result<&str> { + self.info.label() + } + + /// Get the number of GPIO lines exposed by the chip. + pub fn num_lines(&self) -> usize { + self.info.num_lines() + } + + /// Get the path used to find the chip. + pub fn path(&self) -> Result<&str> { + // SAFETY: The string returned by libgpiod is guaranteed to live as long + // as the `struct Chip`. + let path = unsafe { gpiod::gpiod_chip_get_path(self.ichip.chip()) }; + + // SAFETY: The string is guaranteed to be valid here by the C API. + str::from_utf8(unsafe { slice::from_raw_parts(path as *const u8, strlen(path) as usize) }) + .map_err(Error::StringNotUtf8) + } + + /// Get information about the chip. + pub fn info(&self) -> Result<Info> { + Info::new(self.ichip.clone()) + } + + /// Get a snapshot of information about the line. + pub fn line_info(&self, offset: Offset) -> Result<line::Info> { + line::Info::new(self.ichip.clone(), offset) + } + + /// Get the current snapshot of information about the line at given offset and start watching + /// it for future changes. + pub fn watch_line_info(&self, offset: Offset) -> Result<line::Info> { + line::Info::new_watch(self.ichip.clone(), offset) + } + + /// Stop watching a line + pub fn unwatch(&self, offset: Offset) { + unsafe { + gpiod::gpiod_chip_unwatch_line_info(self.ichip.chip(), offset); + } + } + + /// Get the file descriptor associated with the chip. + /// + /// The returned file descriptor must not be closed by the caller, else other methods for the + /// `struct Chip` may fail. + pub fn fd(&self) -> Result<u32> { + let fd = unsafe { gpiod::gpiod_chip_get_fd(self.ichip.chip()) }; + + if fd < 0 { + Err(Error::OperationFailed( + OperationType::ChipGetFd, + Errno::last(), + )) + } else { + Ok(fd as u32) + } + } + + /// Wait for line status events on any of the watched lines on the chip. + pub fn wait_info_event(&self, timeout: Option<Duration>) -> Result<bool> { + let timeout = match timeout { + Some(x) => x.as_nanos() as i64, + // Block indefinitely + None => -1, + }; + + let ret = unsafe { gpiod::gpiod_chip_wait_info_event(self.ichip.chip(), timeout) }; + + match ret { + -1 => Err(Error::OperationFailed( + OperationType::ChipWaitInfoEvent, + Errno::last(), + )), + 0 => Ok(false), + _ => Ok(true), + } + } + + /// Read a single line status change event from the chip. If no events are + /// pending, this function will block. + pub fn read_info_event(&self) -> Result<info::Event> { + info::Event::new(&self.ichip) + } + + /// Map a GPIO line's name to its offset within the chip. + pub fn line_offset_from_name(&self, name: &str) -> Result<Offset> { + // Null-terminate the string + let name = name.to_owned() + "\0"; + + let ret = unsafe { + gpiod::gpiod_chip_get_line_offset_from_name( + self.ichip.chip(), + name.as_ptr() as *const c_char, + ) + }; + + if ret == -1 { + Err(Error::OperationFailed( + OperationType::ChipGetLine, + Errno::last(), + )) + } else { + Ok(ret as u32) + } + } + + /// Request a set of lines for exclusive usage. + pub fn request_lines( + &self, + rconfig: &request::Config, + lconfig: &line::Config, + ) -> Result<line::Request> { + line::Request::new(&self.ichip, rconfig, lconfig) + } +} + +/// GPIO chip Information +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct Info { + info: *mut gpiod::gpiod_chip_info, +} + +impl Info { + /// Find a GPIO chip by path. + pub(crate) fn new(chip: Arc<Internal>) -> Result<Self> { + let info = unsafe { gpiod::gpiod_chip_get_info(chip.chip()) }; + if info.is_null() { + return Err(Error::OperationFailed( + OperationType::ChipInfoGet, + Errno::last(), + )); + } + + Ok(Self { info }) + } + + /// Get the GPIO chip name as represented in the kernel. + pub(crate) fn name(&self) -> Result<&str> { + // SAFETY: The string returned by libgpiod is guaranteed to live as long + // as the `struct Chip`. + let name = unsafe { gpiod::gpiod_chip_info_get_name(self.info) }; + + // SAFETY: The string is guaranteed to be valid here by the C API. + str::from_utf8(unsafe { slice::from_raw_parts(name as *const u8, strlen(name) as usize) }) + .map_err(Error::StringNotUtf8) + } + + /// Get the GPIO chip label as represented in the kernel. + pub(crate) fn label(&self) -> Result<&str> { + // SAFETY: The string returned by libgpiod is guaranteed to live as long + // as the `struct Chip`. + let label = unsafe { gpiod::gpiod_chip_info_get_label(self.info) }; + + // SAFETY: The string is guaranteed to be valid here by the C API. + str::from_utf8(unsafe { slice::from_raw_parts(label as *const u8, strlen(label) as usize) }) + .map_err(Error::StringNotUtf8) + } + + /// Get the number of GPIO lines exposed by the chip. + pub(crate) fn num_lines(&self) -> usize { + unsafe { gpiod::gpiod_chip_info_get_num_lines(self.info) as usize } + } +} + +impl Drop for Info { + /// Close the GPIO chip info and release all associated resources. + fn drop(&mut self) { + unsafe { gpiod::gpiod_chip_info_free(self.info) } + } +} diff --git a/bindings/rust/libgpiod/src/edge_event.rs b/bindings/rust/libgpiod/src/edge_event.rs new file mode 100644 index 000000000000..12c0fd73f778 --- /dev/null +++ b/bindings/rust/libgpiod/src/edge_event.rs @@ -0,0 +1,101 @@ +// SPDX-License-Identifier: Apache-2.0 AND BSD-3-Clause +// +// Copyright 2022 Linaro Ltd. All Rights Reserved. +// Viresh Kumar <viresh.kumar@linaro.org> + +use std::time::Duration; + +use vmm_sys_util::errno::Error as Errno; + +use super::{edge::event::Buffer, gpiod, EdgeKind, Error, Offset, OperationType, Result}; + +/// Line edge events handling +/// +/// An edge event object contains information about a single line edge event. +/// It contains the event type, timestamp and the offset of the line on which +/// the event occurred as well as two sequence numbers (global for all lines +/// in the associated request and local for this line only). +/// +/// Edge events are stored into an edge-event buffer object to improve +/// performance and to limit the number of memory allocations when a large +/// number of events are being read. + +#[derive(Debug, Eq, PartialEq)] +pub struct Event<'b> { + buffer: Option<&'b Buffer>, + event: *mut gpiod::gpiod_edge_event, +} + +impl<'b> Event<'b> { + /// Get an event stored in the buffer. + pub(crate) fn new(buffer: &'b Buffer, index: u64) -> Result<Self> { + let event = unsafe { gpiod::gpiod_edge_event_buffer_get_event(buffer.buffer(), index) }; + if event.is_null() { + return Err(Error::OperationFailed( + OperationType::EdgeEventBufferGetEvent, + Errno::last(), + )); + } + + Ok(Self { + buffer: Some(buffer), + event, + }) + } + + pub fn event_clone(event: &Event) -> Result<Self> { + let event = unsafe { gpiod::gpiod_edge_event_copy(event.event) }; + if event.is_null() { + return Err(Error::OperationFailed( + OperationType::EdgeEventCopy, + Errno::last(), + )); + } + + Ok(Self { + buffer: None, + event, + }) + } + + /// Get the event type. + pub fn event_type(&self) -> Result<EdgeKind> { + EdgeKind::new(unsafe { gpiod::gpiod_edge_event_get_event_type(self.event) } as u32) + } + + /// Get the timestamp of the event. + pub fn timestamp(&self) -> Duration { + Duration::from_nanos(unsafe { gpiod::gpiod_edge_event_get_timestamp_ns(self.event) }) + } + + /// Get the offset of the line on which the event was triggered. + pub fn line_offset(&self) -> Offset { + unsafe { gpiod::gpiod_edge_event_get_line_offset(self.event) } + } + + /// Get the global sequence number of the event. + /// + /// Returns sequence number of the event relative to all lines in the + /// associated line request. + pub fn global_seqno(&self) -> u64 { + unsafe { gpiod::gpiod_edge_event_get_global_seqno(self.event) } + } + + /// Get the event sequence number specific to concerned line. + /// + /// Returns sequence number of the event relative to the line within the + /// lifetime of the associated line request. + pub fn line_seqno(&self) -> u64 { + unsafe { gpiod::gpiod_edge_event_get_line_seqno(self.event) } + } +} + +impl<'b> Drop for Event<'b> { + /// Free the edge event. + fn drop(&mut self) { + // Free the event only if a copy is made + if self.buffer.is_none() { + unsafe { gpiod::gpiod_edge_event_free(self.event) }; + } + } +} diff --git a/bindings/rust/libgpiod/src/event_buffer.rs b/bindings/rust/libgpiod/src/event_buffer.rs new file mode 100644 index 000000000000..11c8b5e1d7c9 --- /dev/null +++ b/bindings/rust/libgpiod/src/event_buffer.rs @@ -0,0 +1,66 @@ +// SPDX-License-Identifier: Apache-2.0 AND BSD-3-Clause +// +// Copyright 2022 Linaro Ltd. All Rights Reserved. +// Viresh Kumar <viresh.kumar@linaro.org> + +use std::os::raw::c_ulong; + +use vmm_sys_util::errno::Error as Errno; + +use super::{edge, gpiod, Error, OperationType, Result}; + +/// Line edge events buffer +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct Buffer { + buffer: *mut gpiod::gpiod_edge_event_buffer, +} + +impl Buffer { + /// Create a new edge event buffer. + /// + /// If capacity equals 0, it will be set to a default value of 64. If + /// capacity is larger than 1024, it will be limited to 1024. + pub fn new(capacity: usize) -> Result<Self> { + let buffer = unsafe { gpiod::gpiod_edge_event_buffer_new(capacity as c_ulong) }; + if buffer.is_null() { + return Err(Error::OperationFailed( + OperationType::EdgeEventBufferNew, + Errno::last(), + )); + } + + Ok(Self { buffer }) + } + + /// Private helper, Returns gpiod_edge_event_buffer + pub(crate) fn buffer(&self) -> *mut gpiod::gpiod_edge_event_buffer { + self.buffer + } + + /// Get the capacity of the event buffer. + pub fn capacity(&self) -> usize { + unsafe { gpiod::gpiod_edge_event_buffer_get_capacity(self.buffer()) as usize } + } + + /// Read an event stored in the buffer. + pub fn event(&self, index: u64) -> Result<edge::Event> { + edge::Event::new(self, index) + } + + /// Get the number of events the buffer contains. + pub fn len(&self) -> usize { + unsafe { gpiod::gpiod_edge_event_buffer_get_num_events(self.buffer()) as usize } + } + + /// Check if buffer is empty. + pub fn is_empty(&self) -> bool { + self.len() == 0 + } +} + +impl Drop for Buffer { + /// Free the edge event buffer and release all associated resources. + fn drop(&mut self) { + unsafe { gpiod::gpiod_edge_event_buffer_free(self.buffer) }; + } +} diff --git a/bindings/rust/libgpiod/src/info_event.rs b/bindings/rust/libgpiod/src/info_event.rs new file mode 100644 index 000000000000..d8be87df6679 --- /dev/null +++ b/bindings/rust/libgpiod/src/info_event.rs @@ -0,0 +1,68 @@ +// SPDX-License-Identifier: Apache-2.0 AND BSD-3-Clause +// +// Copyright 2022 Linaro Ltd. All Rights Reserved. +// Viresh Kumar <viresh.kumar@linaro.org> + +use std::sync::Arc; +use std::time::Duration; + +use vmm_sys_util::errno::Error as Errno; + +use super::{chip, gpiod, line, Error, InfoChangeKind, OperationType, Result}; + +/// Line status watch events +/// +/// Accessors for the info event objects allowing to monitor changes in GPIO +/// line state. +/// +/// Callers can be notified about changes in line's state using the interfaces +/// exposed by GPIO chips. Each info event contains information about the event +/// itself (timestamp, type) as well as a snapshot of line's state in the form +/// of a line-info object. + +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct Event { + event: *mut gpiod::gpiod_info_event, +} + +impl Event { + /// Get a single chip's line's status change event. + pub(crate) fn new(ichip: &Arc<chip::Internal>) -> Result<Self> { + let event = unsafe { gpiod::gpiod_chip_read_info_event(ichip.chip()) }; + if event.is_null() { + return Err(Error::OperationFailed( + OperationType::ChipReadInfoEvent, + Errno::last(), + )); + } + + Ok(Self { event }) + } + + /// Private helper, Returns gpiod_info_event + pub(crate) fn event(&self) -> *mut gpiod::gpiod_info_event { + self.event + } + + /// Get the event type of the status change event. + pub fn event_type(&self) -> Result<InfoChangeKind> { + InfoChangeKind::new(unsafe { gpiod::gpiod_info_event_get_event_type(self.event) } as u32) + } + + /// Get the timestamp of the event, read from the monotonic clock. + pub fn timestamp(&self) -> Duration { + Duration::from_nanos(unsafe { gpiod::gpiod_info_event_get_timestamp_ns(self.event) }) + } + + /// Get the line-info object associated with the event. + pub fn line_info(&self) -> Result<line::Info> { + line::Info::new_from_event(self) + } +} + +impl Drop for Event { + /// Free the info event object and release all associated resources. + fn drop(&mut self) { + unsafe { gpiod::gpiod_info_event_free(self.event) } + } +} diff --git a/bindings/rust/libgpiod/src/lib.rs b/bindings/rust/libgpiod/src/lib.rs new file mode 100644 index 000000000000..c5d974a9f4ea --- /dev/null +++ b/bindings/rust/libgpiod/src/lib.rs @@ -0,0 +1,471 @@ +// SPDX-License-Identifier: Apache-2.0 AND BSD-3-Clause +// +// Rust wrappers for GPIOD APIs +// +// Copyright 2022 Linaro Ltd. All Rights Reserved. +// Viresh Kumar <viresh.kumar@linaro.org> + +//! libgpiod public API +//! +//! This is the complete documentation of the public Rust API made available to +//! users of libgpiod. +//! +//! The API is logically split into several parts such as: GPIO chip & line +//! operators, GPIO events handling etc. + +/// GPIO chip related definitions. +pub mod chip; + +mod edge_event; +mod event_buffer; + +/// GPIO chip edge event related definitions. +pub mod edge { + pub use crate::edge_event::*; + + /// GPIO chip edge event buffer related definitions. + pub mod event { + pub use crate::event_buffer::*; + } +} + +mod info_event; + +/// GPIO chip info event related definitions. +pub mod info { + pub use crate::info_event::*; +} + +mod line_config; +mod line_info; +mod line_request; +mod line_settings; + +/// GPIO chip line related definitions. +pub mod line { + pub use crate::line_config::*; + pub use crate::line_info::*; + pub use crate::line_request::*; + pub use crate::line_settings::*; +} + +mod request_config; + +/// GPIO chip request related definitions. +pub mod request { + pub use crate::request_config::*; +} + +use libgpiod_sys as gpiod; + +use intmap::IntMap; +use libc::strlen; +use std::fs; +use std::os::raw::c_char; +use std::path::Path; +use std::time::Duration; +use std::{fmt, slice, str}; + +use thiserror::Error as ThisError; +use vmm_sys_util::errno::Error as Errno; + +/// Operation types, used with OperationFailed() Error. +#[derive(Copy, Clone, Debug, Eq, PartialEq)] +pub enum OperationType { + ChipOpen, + ChipGetFd, + ChipWaitInfoEvent, + ChipGetLine, + ChipGetLineInfo, + ChipInfoGet, + ChipReadInfoEvent, + ChipWatchLineInfo, + EdgeEventBufferGetEvent, + EdgeEventCopy, + EdgeEventBufferNew, + InfoEventGetLineInfo, + LineConfigNew, + LineConfigAddSettings, + LineConfigGetOffsets, + LineConfigGetSetting, + LineRequest, + LineRequestReconfigLines, + LineRequestGetVal, + LineRequestGetValSubset, + LineRequestSetVal, + LineRequestSetValSubset, + LineRequestReadEdgeEvent, + LineRequestWaitEdgeEvent, + LineSettingsNew, + LineSettingsCopy, + LineSettingsGetOutVal, + LineSettingsSetDirection, + LineSettingsSetEdgeDetection, + LineSettingsSetBias, + LineSettingsSetDrive, + LineSettingsSetActiveLow, + LineSettingsSetDebouncePeriod, + LineSettingsSetEventClock, + LineSettingsSetOutputValue, + RequestConfigNew, + RequestConfigGetConsumer, + SimBankGetVal, + SimBankNew, + SimBankSetLabel, + SimBankSetNumLines, + SimBankSetLineName, + SimBankSetPull, + SimBankHogLine, + SimCtxNew, + SimDevNew, + SimDevEnable, + SimDevDisable, +} + +impl fmt::Display for OperationType { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{:?}", self) + } +} + +/// Result of libgpiod operations. +pub type Result<T> = std::result::Result<T, Error>; + +/// Error codes for libgpiod operations. +#[derive(Copy, Clone, Debug, PartialEq, ThisError)] +pub enum Error { + #[error("Failed to get {0}")] + NullString(&'static str), + #[error("String not utf8: {0:?}")] + StringNotUtf8(str::Utf8Error), + #[error("Invalid enum {0} value: {1}")] + InvalidEnumValue(&'static str, u32), + #[error("Operation {0} Failed: {1}")] + OperationFailed(OperationType, Errno), + #[error("Invalid Arguments")] + InvalidArguments, + #[error("Std Io Error")] + IoError, +} + +/// Value settings. +#[derive(Copy, Clone, Debug, Eq, PartialEq)] +pub enum Value { + /// Active + Active, + /// Inactive + InActive, +} + +/// Maps offset to Value. +pub type ValueMap = IntMap<Value>; + +impl Value { + pub fn new(val: i32) -> Result<Self> { + match val { + 0 => Ok(Value::InActive), + 1 => Ok(Value::Active), + _ => Err(Error::InvalidEnumValue("Value", val as u32)), + } + } + + fn value(&self) -> i32 { + match self { + Value::Active => 1, + Value::InActive => 0, + } + } +} + +/// Offset type. +pub type Offset = u32; + +/// Direction settings. +#[derive(Copy, Clone, Debug, Eq, PartialEq)] +pub enum Direction { + /// Request the line(s), but don't change direction. + AsIs, + /// Direction is input - for reading the value of an externally driven GPIO line. + Input, + /// Direction is output - for driving the GPIO line. + Output, +} + +impl Direction { + fn new(dir: u32) -> Result<Self> { + match dir { + gpiod::GPIOD_LINE_DIRECTION_AS_IS => Ok(Direction::AsIs), + gpiod::GPIOD_LINE_DIRECTION_INPUT => Ok(Direction::Input), + gpiod::GPIOD_LINE_DIRECTION_OUTPUT => Ok(Direction::Output), + _ => Err(Error::InvalidEnumValue("Direction", dir)), + } + } + + fn gpiod_direction(&self) -> u32 { + match self { + Direction::AsIs => gpiod::GPIOD_LINE_DIRECTION_AS_IS, + Direction::Input => gpiod::GPIOD_LINE_DIRECTION_INPUT, + Direction::Output => gpiod::GPIOD_LINE_DIRECTION_OUTPUT, + } + } +} + +/// Internal bias settings. +#[derive(Copy, Clone, Debug, Eq, PartialEq)] +pub enum Bias { + /// The internal bias is disabled. + Disabled, + /// The internal pull-up bias is enabled. + PullUp, + /// The internal pull-down bias is enabled. + PullDown, +} + +impl Bias { + fn new(bias: u32) -> Result<Option<Self>> { + match bias { + gpiod::GPIOD_LINE_BIAS_UNKNOWN => Ok(None), + gpiod::GPIOD_LINE_BIAS_AS_IS => Ok(None), + gpiod::GPIOD_LINE_BIAS_DISABLED => Ok(Some(Bias::Disabled)), + gpiod::GPIOD_LINE_BIAS_PULL_UP => Ok(Some(Bias::PullUp)), + gpiod::GPIOD_LINE_BIAS_PULL_DOWN => Ok(Some(Bias::PullDown)), + _ => Err(Error::InvalidEnumValue("Bias", bias)), + } + } + + fn gpiod_bias(bias: Option<Bias>) -> u32 { + match bias { + None => gpiod::GPIOD_LINE_BIAS_AS_IS, + Some(bias) => match bias { + Bias::Disabled => gpiod::GPIOD_LINE_BIAS_DISABLED, + Bias::PullUp => gpiod::GPIOD_LINE_BIAS_PULL_UP, + Bias::PullDown => gpiod::GPIOD_LINE_BIAS_PULL_DOWN, + }, + } + } +} + +/// Drive settings. +#[derive(Copy, Clone, Debug, Eq, PartialEq)] +pub enum Drive { + /// Drive setting is push-pull. + PushPull, + /// Line output is open-drain. + OpenDrain, + /// Line output is open-source. + OpenSource, +} + +impl Drive { + fn new(drive: u32) -> Result<Self> { + match drive { + gpiod::GPIOD_LINE_DRIVE_PUSH_PULL => Ok(Drive::PushPull), + gpiod::GPIOD_LINE_DRIVE_OPEN_DRAIN => Ok(Drive::OpenDrain), + gpiod::GPIOD_LINE_DRIVE_OPEN_SOURCE => Ok(Drive::OpenSource), + _ => Err(Error::InvalidEnumValue("Drive", drive)), + } + } + + fn gpiod_drive(&self) -> u32 { + match self { + Drive::PushPull => gpiod::GPIOD_LINE_DRIVE_PUSH_PULL, + Drive::OpenDrain => gpiod::GPIOD_LINE_DRIVE_OPEN_DRAIN, + Drive::OpenSource => gpiod::GPIOD_LINE_DRIVE_OPEN_SOURCE, + } + } +} + +/// Edge detection settings. +#[derive(Copy, Clone, Debug, Eq, PartialEq)] +pub enum Edge { + /// Line detects rising edge events. + Rising, + /// Line detects falling edge events. + Falling, + /// Line detects both rising and falling edge events. + Both, +} + +impl Edge { + fn new(edge: u32) -> Result<Option<Self>> { + match edge { + gpiod::GPIOD_LINE_EDGE_NONE => Ok(None), + gpiod::GPIOD_LINE_EDGE_RISING => Ok(Some(Edge::Rising)), + gpiod::GPIOD_LINE_EDGE_FALLING => Ok(Some(Edge::Falling)), + gpiod::GPIOD_LINE_EDGE_BOTH => Ok(Some(Edge::Both)), + _ => Err(Error::InvalidEnumValue("Edge", edge)), + } + } + + fn gpiod_edge(edge: Option<Edge>) -> u32 { + match edge { + None => gpiod::GPIOD_LINE_EDGE_NONE, + Some(edge) => match edge { + Edge::Rising => gpiod::GPIOD_LINE_EDGE_RISING, + Edge::Falling => gpiod::GPIOD_LINE_EDGE_FALLING, + Edge::Both => gpiod::GPIOD_LINE_EDGE_BOTH, + }, + } + } +} + +/// Line setting kind. +#[derive(Copy, Clone, Debug, Eq, PartialEq)] +pub enum SettingKind { + /// Line direction. + Direction, + /// Bias. + Bias, + /// Drive. + Drive, + /// Edge detection. + EdgeDetection, + /// Active-low setting. + ActiveLow, + /// Debounce period. + DebouncePeriod, + /// Event clock type. + EventClock, + /// Output value. + OutputValue, +} + +/// Line settings. +#[derive(Copy, Clone, Debug, Eq, PartialEq)] +pub enum SettingVal { + /// Line direction. + Direction(Direction), + /// Bias. + Bias(Option<Bias>), + /// Drive. + Drive(Drive), + /// Edge detection. + EdgeDetection(Option<Edge>), + /// Active-low setting. + ActiveLow(bool), + /// Debounce period. + DebouncePeriod(Duration), + /// Event clock type. + EventClock(EventClock), + /// Output value. + OutputValue(Value), +} + +impl fmt::Display for SettingVal { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{:?}", self) + } +} + +/// Event clock settings. +#[derive(Copy, Clone, Debug, Eq, PartialEq)] +pub enum EventClock { + /// Line uses the monotonic clock for edge event timestamps. + Monotonic, + /// Line uses the realtime clock for edge event timestamps. + Realtime, + /// Line uses the hardware timestamp engine clock for edge event timestamps. + HTE, +} + +impl EventClock { + fn new(clock: u32) -> Result<Self> { + match clock { + gpiod::GPIOD_LINE_EVENT_CLOCK_MONOTONIC => Ok(EventClock::Monotonic), + gpiod::GPIOD_LINE_EVENT_CLOCK_REALTIME => Ok(EventClock::Realtime), + gpiod::GPIOD_LINE_EVENT_CLOCK_HTE => Ok(EventClock::HTE), + _ => Err(Error::InvalidEnumValue("Eventclock", clock)), + } + } + + fn gpiod_clock(&self) -> u32 { + match self { + EventClock::Monotonic => gpiod::GPIOD_LINE_EVENT_CLOCK_MONOTONIC, + EventClock::Realtime => gpiod::GPIOD_LINE_EVENT_CLOCK_REALTIME, + EventClock::HTE => gpiod::GPIOD_LINE_EVENT_CLOCK_HTE, + } + } +} + +/// Line status change event types. +#[derive(Copy, Clone, Debug, Eq, PartialEq)] +pub enum InfoChangeKind { + /// Line has been requested. + LineRequested, + /// Previously requested line has been released. + LineReleased, + /// Line configuration has changed. + LineConfigChanged, +} + +impl InfoChangeKind { + fn new(kind: u32) -> Result<Self> { + match kind { + gpiod::GPIOD_INFO_EVENT_LINE_REQUESTED => Ok(InfoChangeKind::LineRequested), + gpiod::GPIOD_INFO_EVENT_LINE_RELEASED => Ok(InfoChangeKind::LineReleased), + gpiod::GPIOD_INFO_EVENT_LINE_CONFIG_CHANGED => Ok(InfoChangeKind::LineConfigChanged), + _ => Err(Error::InvalidEnumValue("InfoChangeKind", kind)), + } + } +} + +/// Edge event types. +#[derive(Copy, Clone, Debug, Eq, PartialEq)] +pub enum EdgeKind { + /// Rising edge event. + Rising, + /// Falling edge event. + Falling, +} + +impl EdgeKind { + fn new(kind: u32) -> Result<Self> { + match kind { + gpiod::GPIOD_EDGE_EVENT_RISING_EDGE => Ok(EdgeKind::Rising), + gpiod::GPIOD_EDGE_EVENT_FALLING_EDGE => Ok(EdgeKind::Falling), + _ => Err(Error::InvalidEnumValue("EdgeEvent", kind)), + } + } +} + +/// Various libgpiod-related functions. + +/// Check if the file pointed to by path is a GPIO chip character device. +/// +/// Returns true if the file exists and is a GPIO chip character device or a +/// symbolic link to it. +pub fn is_gpiochip_device<P: AsRef<Path>>(path: &P) -> bool { + // Null-terminate the string + let path = path.as_ref().to_string_lossy() + "\0"; + + unsafe { gpiod::gpiod_is_gpiochip_device(path.as_ptr() as *const c_char) } +} + +/// Iterator for GPIO devices. +pub fn gpiochip_devices<P: AsRef<Path>>(path: &P) -> Result<Vec<chip::Chip>> { + let mut chips = Vec::new(); + + for entry in fs::read_dir(path).map_err(|_| Error::IoError)?.flatten() { + let path = entry.path(); + + if is_gpiochip_device(&path) { + chips.push(chip::Chip::open(&path)?); + } + } + + Ok(chips) +} + +/// Get the API version of the library as a human-readable string. +pub fn version_string() -> Result<&'static str> { + // SAFETY: The string returned by libgpiod is guaranteed to live forever. + let version = unsafe { gpiod::gpiod_version_string() }; + + if version.is_null() { + return Err(Error::NullString("GPIO library version")); + } + + // SAFETY: The string is guaranteed to be valid here by the C API. + str::from_utf8(unsafe { slice::from_raw_parts(version as *const u8, strlen(version) as usize) }) + .map_err(Error::StringNotUtf8) +} diff --git a/bindings/rust/libgpiod/src/line_config.rs b/bindings/rust/libgpiod/src/line_config.rs new file mode 100644 index 000000000000..ea25452fd1d6 --- /dev/null +++ b/bindings/rust/libgpiod/src/line_config.rs @@ -0,0 +1,118 @@ +// SPDX-License-Identifier: Apache-2.0 AND BSD-3-Clause +// +// Copyright 2022 Linaro Ltd. All Rights Reserved. +// Viresh Kumar <viresh.kumar@linaro.org> + +use std::os::raw::c_ulong; +use std::slice::from_raw_parts; + +use vmm_sys_util::errno::Error as Errno; + +use super::{gpiod, line::Settings, Error, Offset, OperationType, Result}; + +/// Line configuration objects. +/// +/// The line-config object contains the configuration for lines that can be +/// used in two cases: +/// - when making a line request +/// - when reconfiguring a set of already requested lines. +/// +/// A new line-config object is empty. Using it in a request will lead to an +/// error. In order to a line-config to become useful, it needs to be assigned +/// at least one offset-to-settings mapping by calling +/// ::gpiod_line_config_add_line_settings. +/// +/// When calling ::gpiod_chip_request_lines, the library will request all +/// offsets that were assigned settings in the order that they were assigned. + +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct Config { + config: *mut gpiod::gpiod_line_config, +} + +impl Config { + /// Create a new line config object. + pub fn new() -> Result<Self> { + let config = unsafe { gpiod::gpiod_line_config_new() }; + + if config.is_null() { + return Err(Error::OperationFailed( + OperationType::LineConfigNew, + Errno::last(), + )); + } + + Ok(Self { config }) + } + + /// Private helper, Returns gpiod_line_config + pub(crate) fn config(&self) -> *mut gpiod::gpiod_line_config { + self.config + } + + /// Resets the entire configuration stored in the object. This is useful if + /// the user wants to reuse the object without reallocating it. + pub fn reset(&mut self) { + unsafe { gpiod::gpiod_line_config_reset(self.config) } + } + + /// Add line settings for a set of offsets. + pub fn add_line_settings(&self, offsets: &[Offset], settings: Settings) -> Result<()> { + let ret = unsafe { + gpiod::gpiod_line_config_add_line_settings( + self.config, + offsets.as_ptr(), + offsets.len() as c_ulong, + settings.settings(), + ) + }; + + if ret == -1 { + Err(Error::OperationFailed( + OperationType::LineConfigAddSettings, + Errno::last(), + )) + } else { + Ok(()) + } + } + + /// Get line settings for offset. + pub fn line_settings(&self, offset: Offset) -> Result<Settings> { + let settings = unsafe { gpiod::gpiod_line_config_get_line_settings(self.config, offset) }; + + if settings.is_null() { + return Err(Error::OperationFailed( + OperationType::LineConfigGetSetting, + Errno::last(), + )); + } + + Ok(Settings::new_with_settings(settings)) + } + + /// Get configured offsets. + pub fn offsets(&self) -> Result<Vec<Offset>> { + let mut num: u64 = 0; + let mut offsets: *mut Offset = std::ptr::null_mut(); + + let ret = + unsafe { gpiod::gpiod_line_config_get_offsets(self.config, &mut num, &mut offsets) }; + + if ret == -1 { + Err(Error::OperationFailed( + OperationType::LineConfigGetOffsets, + Errno::last(), + )) + } else { + Ok(unsafe { from_raw_parts(offsets as *const Offset, num as usize).to_vec() }) + } + } +} + +impl Drop for Config { + /// Free the line config object and release all associated resources. + fn drop(&mut self) { + unsafe { gpiod::gpiod_line_config_free(self.config) } + } +} diff --git a/bindings/rust/libgpiod/src/line_info.rs b/bindings/rust/libgpiod/src/line_info.rs new file mode 100644 index 000000000000..9db51fc30efd --- /dev/null +++ b/bindings/rust/libgpiod/src/line_info.rs @@ -0,0 +1,180 @@ +// SPDX-License-Identifier: Apache-2.0 AND BSD-3-Clause +// +// Copyright 2022 Linaro Ltd. All Rights Reserved. +// Viresh Kumar <viresh.kumar@linaro.org> + +use libc::strlen; +use std::sync::Arc; +use std::time::Duration; +use std::{slice, str}; + +use vmm_sys_util::errno::Error as Errno; + +use super::{ + chip, gpiod, info, Bias, Direction, Drive, Edge, Error, EventClock, Offset, OperationType, + Result, +}; + +/// Line info +/// +/// Exposes functions for retrieving kernel information about both requested and +/// free lines. Line info object contains an immutable snapshot of a line's status. +/// +/// The line info contains all the publicly available information about a +/// line, which does not include the line value. The line must be requested +/// to access the line value. + +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct Info { + info: *mut gpiod::gpiod_line_info, + info_event: bool, +} + +impl Info { + /// Get a snapshot of information about the line and optionally start watching it for changes. + pub(crate) fn new(ichip: Arc<chip::Internal>, offset: Offset) -> Result<Self> { + let info = unsafe { gpiod::gpiod_chip_get_line_info(ichip.chip(), offset) }; + + if info.is_null() { + return Err(Error::OperationFailed( + OperationType::ChipGetLineInfo, + Errno::last(), + )); + } + + Ok(Self { + info, + info_event: false, + }) + } + + pub(crate) fn new_watch(ichip: Arc<chip::Internal>, offset: Offset) -> Result<Self> { + let info = unsafe { gpiod::gpiod_chip_watch_line_info(ichip.chip(), offset) }; + + if info.is_null() { + return Err(Error::OperationFailed( + OperationType::ChipWatchLineInfo, + Errno::last(), + )); + } + + Ok(Self { + info, + info_event: false, + }) + } + + /// Get the Line info object associated with an event. + pub(crate) fn new_from_event(event: &info::Event) -> Result<Self> { + let info = unsafe { gpiod::gpiod_info_event_get_line_info(event.event()) }; + + if info.is_null() { + return Err(Error::OperationFailed( + OperationType::InfoEventGetLineInfo, + Errno::last(), + )); + } + + Ok(Self { + info, + info_event: true, + }) + } + + /// Get the offset of the line within the GPIO chip. + /// + /// The offset uniquely identifies the line on the chip. The combination of the chip and offset + /// uniquely identifies the line within the system. + + pub fn offset(&self) -> Offset { + unsafe { gpiod::gpiod_line_info_get_offset(self.info) } + } + + /// Get GPIO line's name. + pub fn name(&self) -> Result<&str> { + // SAFETY: The string returned by libgpiod is guaranteed to live as long + // as the `struct Info`. + let name = unsafe { gpiod::gpiod_line_info_get_name(self.info) }; + if name.is_null() { + return Err(Error::NullString("GPIO line's name")); + } + + // SAFETY: The string is guaranteed to be valid here by the C API. + str::from_utf8(unsafe { slice::from_raw_parts(name as *const u8, strlen(name) as usize) }) + .map_err(Error::StringNotUtf8) + } + + /// Returns True if the line is in use, false otherwise. + /// + /// The user space can't know exactly why a line is busy. It may have been + /// requested by another process or hogged by the kernel. It only matters that + /// the line is used and we can't request it. + pub fn is_used(&self) -> bool { + unsafe { gpiod::gpiod_line_info_is_used(self.info) } + } + + /// Get the GPIO line's consumer name. + pub fn consumer(&self) -> Result<&str> { + // SAFETY: The string returned by libgpiod is guaranteed to live as long + // as the `struct Info`. + let name = unsafe { gpiod::gpiod_line_info_get_consumer(self.info) }; + if name.is_null() { + return Err(Error::NullString("GPIO line's consumer name")); + } + + // SAFETY: The string is guaranteed to be valid here by the C API. + str::from_utf8(unsafe { slice::from_raw_parts(name as *const u8, strlen(name) as usize) }) + .map_err(Error::StringNotUtf8) + } + + /// Get the GPIO line's direction. + pub fn direction(&self) -> Result<Direction> { + Direction::new(unsafe { gpiod::gpiod_line_info_get_direction(self.info) } as u32) + } + + /// Returns true if the line is "active-low", false otherwise. + pub fn is_active_low(&self) -> bool { + unsafe { gpiod::gpiod_line_info_is_active_low(self.info) } + } + + /// Get the GPIO line's bias setting. + pub fn bias(&self) -> Result<Option<Bias>> { + Bias::new(unsafe { gpiod::gpiod_line_info_get_bias(self.info) } as u32) + } + + /// Get the GPIO line's drive setting. + pub fn drive(&self) -> Result<Drive> { + Drive::new(unsafe { gpiod::gpiod_line_info_get_drive(self.info) } as u32) + } + + /// Get the current edge detection setting of the line. + pub fn edge_detection(&self) -> Result<Option<Edge>> { + Edge::new(unsafe { gpiod::gpiod_line_info_get_edge_detection(self.info) } as u32) + } + + /// Get the current event clock setting used for edge event timestamps. + pub fn event_clock(&self) -> Result<EventClock> { + EventClock::new(unsafe { gpiod::gpiod_line_info_get_event_clock(self.info) } as u32) + } + + /// Returns true if the line is debounced (either by hardware or by the + /// kernel software debouncer), false otherwise. + pub fn is_debounced(&self) -> bool { + unsafe { gpiod::gpiod_line_info_is_debounced(self.info) } + } + + /// Get the debounce period of the line. + pub fn debounce_period(&self) -> Duration { + Duration::from_micros(unsafe { gpiod::gpiod_line_info_get_debounce_period_us(self.info) }) + } +} + +impl Drop for Info { + fn drop(&mut self) { + // We must not free the Line info object created from `struct info::Event` by calling + // libgpiod API. + if !self.info_event { + unsafe { gpiod::gpiod_line_info_free(self.info) } + } + } +} diff --git a/bindings/rust/libgpiod/src/line_request.rs b/bindings/rust/libgpiod/src/line_request.rs new file mode 100644 index 000000000000..e4ff951aef29 --- /dev/null +++ b/bindings/rust/libgpiod/src/line_request.rs @@ -0,0 +1,246 @@ +// SPDX-License-Identifier: Apache-2.0 AND BSD-3-Clause +// +// Copyright 2022 Linaro Ltd. All Rights Reserved. +// Viresh Kumar <viresh.kumar@linaro.org> + +use libc::EINVAL; +use std::os::raw::c_ulong; +use std::sync::Arc; +use std::time::Duration; + +use vmm_sys_util::errno::Error as Errno; + +use super::{ + chip, edge, gpiod, line, request, Error, Offset, OperationType, Result, Value, ValueMap, +}; + +/// Line request operations +/// +/// Allows interaction with a set of requested lines. +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct Request { + request: *mut gpiod::gpiod_line_request, +} + +impl Request { + /// Request a set of lines for exclusive usage. + pub(crate) fn new( + ichip: &Arc<chip::Internal>, + rconfig: &request::Config, + lconfig: &line::Config, + ) -> Result<Self> { + let request = unsafe { + gpiod::gpiod_chip_request_lines(ichip.chip(), rconfig.config(), lconfig.config()) + }; + + if request.is_null() { + return Err(Error::OperationFailed( + OperationType::LineRequest, + Errno::last(), + )); + } + + Ok(Self { request }) + } + + /// Get the number of lines in the request. + pub fn num_lines(&self) -> usize { + unsafe { gpiod::gpiod_line_request_get_num_lines(self.request) as usize } + } + + /// Get the offsets of lines in the request. + pub fn offsets(&self) -> Vec<Offset> { + let mut offsets = vec![0; self.num_lines() as usize]; + + unsafe { gpiod::gpiod_line_request_get_offsets(self.request, offsets.as_mut_ptr()) }; + offsets + } + + /// Get the value (0 or 1) of a single line associated with the request. + pub fn value(&self, offset: Offset) -> Result<Value> { + let value = unsafe { gpiod::gpiod_line_request_get_value(self.request, offset) }; + + if value != 0 && value != 1 { + Err(Error::OperationFailed( + OperationType::LineRequestGetVal, + Errno::last(), + )) + } else { + Value::new(value) + } + } + + /// Get values of a subset of lines associated with the request. + pub fn values_subset(&self, offsets: &[Offset]) -> Result<ValueMap> { + let mut values = vec![0; offsets.len()]; + + let ret = unsafe { + gpiod::gpiod_line_request_get_values_subset( + self.request, + offsets.len() as c_ulong, + offsets.as_ptr(), + values.as_mut_ptr(), + ) + }; + + if ret == -1 { + Err(Error::OperationFailed( + OperationType::LineRequestGetValSubset, + Errno::last(), + )) + } else { + let mut map = ValueMap::new(); + + for (i, val) in values.iter().enumerate() { + map.insert(offsets[i].into(), Value::new(*val)?); + } + + Ok(map) + } + } + + /// Get values of all lines associated with the request. + pub fn values(&self) -> Result<ValueMap> { + self.values_subset(&self.offsets()) + } + + /// Set the value of a single line associated with the request. + pub fn set_value(&self, offset: Offset, value: Value) -> Result<()> { + let ret = + unsafe { gpiod::gpiod_line_request_set_value(self.request, offset, value.value()) }; + + if ret == -1 { + Err(Error::OperationFailed( + OperationType::LineRequestSetVal, + Errno::last(), + )) + } else { + Ok(()) + } + } + + /// Get values of a subset of lines associated with the request. + pub fn set_values_subset(&self, map: ValueMap) -> Result<()> { + let mut offsets = Vec::new(); + let mut values = Vec::new(); + + for (offset, value) in map { + offsets.push(offset as u32); + values.push(value.value()); + } + + let ret = unsafe { + gpiod::gpiod_line_request_set_values_subset( + self.request, + offsets.len() as c_ulong, + offsets.as_ptr(), + values.as_ptr(), + ) + }; + + if ret == -1 { + Err(Error::OperationFailed( + OperationType::LineRequestSetValSubset, + Errno::last(), + )) + } else { + Ok(()) + } + } + + /// Get values of all lines associated with the request. + pub fn set_values(&self, values: &[Value]) -> Result<()> { + if values.len() != self.num_lines() as usize { + return Err(Error::OperationFailed( + OperationType::LineRequestSetVal, + Errno::new(EINVAL), + )); + } + + let mut new_values = Vec::new(); + for value in values { + new_values.push(value.value()); + } + + let ret = + unsafe { gpiod::gpiod_line_request_set_values(self.request, new_values.as_ptr()) }; + + if ret == -1 { + Err(Error::OperationFailed( + OperationType::LineRequestSetVal, + Errno::last(), + )) + } else { + Ok(()) + } + } + + /// Update the configuration of lines associated with the line request. + pub fn reconfigure_lines(&self, lconfig: &line::Config) -> Result<()> { + let ret = + unsafe { gpiod::gpiod_line_request_reconfigure_lines(self.request, lconfig.config()) }; + + if ret == -1 { + Err(Error::OperationFailed( + OperationType::LineRequestReconfigLines, + Errno::last(), + )) + } else { + Ok(()) + } + } + + /// Get the file descriptor associated with the line request. + pub fn fd(&self) -> u32 { + unsafe { gpiod::gpiod_line_request_get_fd(self.request) as u32 } + } + + /// Wait for edge events on any of the lines associated with the request. + pub fn wait_edge_event(&self, timeout: Option<Duration>) -> Result<bool> { + let timeout = match timeout { + Some(x) => x.as_nanos() as i64, + // Block indefinitely + None => -1, + }; + + let ret = unsafe { gpiod::gpiod_line_request_wait_edge_event(self.request, timeout) }; + + match ret { + -1 => Err(Error::OperationFailed( + OperationType::LineRequestWaitEdgeEvent, + Errno::last(), + )), + 0 => Ok(false), + _ => Ok(true), + } + } + + /// Get a number of edge events from a line request. + /// + /// This function will block if no event was queued for the line. + pub fn read_edge_events(&self, buffer: &mut edge::event::Buffer) -> Result<u32> { + let ret = unsafe { + gpiod::gpiod_line_request_read_edge_event( + self.request, + buffer.buffer(), + buffer.capacity() as u64, + ) + }; + + if ret == -1 { + Err(Error::OperationFailed( + OperationType::LineRequestReadEdgeEvent, + Errno::last(), + )) + } else { + Ok(ret as u32) + } + } +} + +impl Drop for Request { + /// Release the requested lines and free all associated resources. + fn drop(&mut self) { + unsafe { gpiod::gpiod_line_request_release(self.request) } + } +} diff --git a/bindings/rust/libgpiod/src/line_settings.rs b/bindings/rust/libgpiod/src/line_settings.rs new file mode 100644 index 000000000000..2c3090132ea5 --- /dev/null +++ b/bindings/rust/libgpiod/src/line_settings.rs @@ -0,0 +1,277 @@ +// SPDX-License-Identifier: Apache-2.0 AND BSD-3-Clause +// +// Copyright 2022 Linaro Ltd. All Rights Reserved. +// Viresh Kumar <viresh.kumar@linaro.org> + +use std::time::Duration; + +use vmm_sys_util::errno::Error as Errno; + +use super::{ + gpiod, Bias, Direction, Drive, Edge, Error, EventClock, OperationType, Result, SettingKind, + SettingVal, Value, +}; + +/// Line settings objects. +/// +/// Line settings object contains a set of line properties that can be used +/// when requesting lines or reconfiguring an existing request. +/// +/// Mutators in general can only fail if the new property value is invalid. The +/// return values can be safely ignored - the object remains valid even after +/// a mutator fails and simply uses the sane default appropriate for given +/// property. + +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct Settings { + settings: *mut gpiod::gpiod_line_settings, +} + +impl Settings { + /// Create a new line settings object. + pub fn new() -> Result<Self> { + let settings = unsafe { gpiod::gpiod_line_settings_new() }; + + if settings.is_null() { + return Err(Error::OperationFailed( + OperationType::LineSettingsNew, + Errno::last(), + )); + } + + Ok(Self { settings }) + } + + pub fn new_with_settings(settings: *mut gpiod::gpiod_line_settings) -> Self { + Self { settings } + } + + /// Private helper, Returns gpiod_line_settings + pub(crate) fn settings(&self) -> *mut gpiod::gpiod_line_settings { + self.settings + } + + /// Resets the line settings object to its default values. + pub fn reset(&mut self) { + unsafe { gpiod::gpiod_line_settings_reset(self.settings) } + } + + /// Makes copy of the settings object. + pub fn settings_clone(&self) -> Result<Self> { + let settings = unsafe { gpiod::gpiod_line_settings_copy(self.settings) }; + if settings.is_null() { + return Err(Error::OperationFailed( + OperationType::LineSettingsCopy, + Errno::last(), + )); + } + + Ok(Self { settings }) + } + + /// Set line prop setting. + pub fn set_prop(&mut self, values: &[SettingVal]) -> Result<()> { + for value in values { + match value { + SettingVal::Direction(val) => self.set_direction(*val)?, + SettingVal::EdgeDetection(val) => self.set_edge_detection(*val)?, + SettingVal::Bias(val) => self.set_bias(*val)?, + SettingVal::Drive(val) => self.set_drive(*val)?, + SettingVal::ActiveLow(val) => self.set_active_low(*val), + SettingVal::DebouncePeriod(val) => self.set_debounce_period(*val), + SettingVal::EventClock(val) => self.set_event_clock(*val)?, + SettingVal::OutputValue(val) => self.set_output_value(*val)?, + } + } + + Ok(()) + } + + /// Get the line prop setting. + pub fn prop(&self, property: SettingKind) -> Result<SettingVal> { + Ok(match property { + SettingKind::Direction => SettingVal::Direction(self.direction()?), + SettingKind::EdgeDetection => SettingVal::EdgeDetection(self.edge_detection()?), + SettingKind::Bias => SettingVal::Bias(self.bias()?), + SettingKind::Drive => SettingVal::Drive(self.drive()?), + SettingKind::ActiveLow => SettingVal::ActiveLow(self.active_low()), + SettingKind::DebouncePeriod => SettingVal::DebouncePeriod(self.debounce_period()?), + SettingKind::EventClock => SettingVal::EventClock(self.event_clock()?), + SettingKind::OutputValue => SettingVal::OutputValue(self.output_value()?), + }) + } + + /// Set the line direction. + fn set_direction(&mut self, direction: Direction) -> Result<()> { + let ret = unsafe { + gpiod::gpiod_line_settings_set_direction( + self.settings, + direction.gpiod_direction() as i32, + ) + }; + + if ret == -1 { + Err(Error::OperationFailed( + OperationType::LineSettingsSetDirection, + Errno::last(), + )) + } else { + Ok(()) + } + } + + /// Get the direction setting. + fn direction(&self) -> Result<Direction> { + Direction::new(unsafe { gpiod::gpiod_line_settings_get_direction(self.settings) } as u32) + } + + /// Set the edge event detection setting. + fn set_edge_detection(&mut self, edge: Option<Edge>) -> Result<()> { + let ret = unsafe { + gpiod::gpiod_line_settings_set_edge_detection( + self.settings, + Edge::gpiod_edge(edge) as i32, + ) + }; + + if ret == -1 { + Err(Error::OperationFailed( + OperationType::LineSettingsSetEdgeDetection, + Errno::last(), + )) + } else { + Ok(()) + } + } + + /// Get the edge event detection setting. + fn edge_detection(&self) -> Result<Option<Edge>> { + Edge::new(unsafe { gpiod::gpiod_line_settings_get_edge_detection(self.settings) } as u32) + } + + /// Set the bias setting. + fn set_bias(&mut self, bias: Option<Bias>) -> Result<()> { + let ret = unsafe { + gpiod::gpiod_line_settings_set_bias(self.settings, Bias::gpiod_bias(bias) as i32) + }; + + if ret == -1 { + Err(Error::OperationFailed( + OperationType::LineSettingsSetBias, + Errno::last(), + )) + } else { + Ok(()) + } + } + + /// Get the bias setting. + fn bias(&self) -> Result<Option<Bias>> { + Bias::new(unsafe { gpiod::gpiod_line_settings_get_bias(self.settings) } as u32) + } + + /// Set the drive setting. + fn set_drive(&mut self, drive: Drive) -> Result<()> { + let ret = unsafe { + gpiod::gpiod_line_settings_set_drive(self.settings, drive.gpiod_drive() as i32) + }; + + if ret == -1 { + Err(Error::OperationFailed( + OperationType::LineSettingsSetDrive, + Errno::last(), + )) + } else { + Ok(()) + } + } + + /// Get the drive setting. + fn drive(&self) -> Result<Drive> { + Drive::new(unsafe { gpiod::gpiod_line_settings_get_drive(self.settings) } as u32) + } + + /// Set active-low setting. + fn set_active_low(&mut self, active_low: bool) { + unsafe { gpiod::gpiod_line_settings_set_active_low(self.settings, active_low) } + } + + /// Check the active-low setting. + fn active_low(&self) -> bool { + unsafe { gpiod::gpiod_line_settings_get_active_low(self.settings) } + } + + /// Set the debounce period setting. + fn set_debounce_period(&mut self, period: Duration) { + unsafe { + gpiod::gpiod_line_settings_set_debounce_period_us( + self.settings, + period.as_micros() as u64, + ) + } + } + + /// Get the debounce period. + fn debounce_period(&self) -> Result<Duration> { + Ok(Duration::from_micros(unsafe { + gpiod::gpiod_line_settings_get_debounce_period_us(self.settings) + })) + } + + /// Set the event clock setting. + fn set_event_clock(&mut self, clock: EventClock) -> Result<()> { + let ret = unsafe { + gpiod::gpiod_line_settings_set_event_clock(self.settings, clock.gpiod_clock() as i32) + }; + + if ret == -1 { + Err(Error::OperationFailed( + OperationType::LineSettingsSetEventClock, + Errno::last(), + )) + } else { + Ok(()) + } + } + + /// Get the event clock setting. + fn event_clock(&self) -> Result<EventClock> { + EventClock::new(unsafe { gpiod::gpiod_line_settings_get_event_clock(self.settings) } as u32) + } + + /// Set the output value setting. + fn set_output_value(&mut self, value: Value) -> Result<()> { + let ret = + unsafe { gpiod::gpiod_line_settings_set_output_value(self.settings, value.value()) }; + + if ret == -1 { + Err(Error::OperationFailed( + OperationType::LineSettingsSetOutputValue, + Errno::last(), + )) + } else { + Ok(()) + } + } + + /// Get the output value, 0 or 1. + fn output_value(&self) -> Result<Value> { + let value = unsafe { gpiod::gpiod_line_settings_get_output_value(self.settings) }; + + if value != 0 && value != 1 { + Err(Error::OperationFailed( + OperationType::LineSettingsGetOutVal, + Errno::last(), + )) + } else { + Value::new(value) + } + } +} + +impl Drop for Settings { + /// Free the line settings object and release all associated resources. + fn drop(&mut self) { + unsafe { gpiod::gpiod_line_settings_free(self.settings) } + } +} diff --git a/bindings/rust/libgpiod/src/request_config.rs b/bindings/rust/libgpiod/src/request_config.rs new file mode 100644 index 000000000000..760d9c755c86 --- /dev/null +++ b/bindings/rust/libgpiod/src/request_config.rs @@ -0,0 +1,96 @@ +// SPDX-License-Identifier: Apache-2.0 AND BSD-3-Clause +// +// Copyright 2022 Linaro Ltd. All Rights Reserved. +// Viresh Kumar <viresh.kumar@linaro.org> + +use libc::strlen; +use std::os::raw::{c_char, c_ulong}; +use std::{slice, str}; + +use vmm_sys_util::errno::Error as Errno; + +use super::{gpiod, Error, OperationType, Result}; + +/// Request configuration objects +/// +/// Request config objects are used to pass a set of options to the kernel at +/// the time of the line request. The mutators don't return error values. If the +/// values are invalid, in general they are silently adjusted to acceptable +/// ranges. + +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct Config { + config: *mut gpiod::gpiod_request_config, +} + +impl Config { + /// Create a new request config object. + pub fn new() -> Result<Self> { + let config = unsafe { gpiod::gpiod_request_config_new() }; + if config.is_null() { + return Err(Error::OperationFailed( + OperationType::RequestConfigNew, + Errno::last(), + )); + } + + Ok(Self { config }) + } + + /// Private helper, Returns gpiod_request_config + pub(crate) fn config(&self) -> *mut gpiod::gpiod_request_config { + self.config + } + + /// Set the consumer name for the request. + /// + /// If the consumer string is too long, it will be truncated to the max + /// accepted length. + pub fn set_consumer(&self, consumer: &str) { + // Null-terminate the string + let consumer = consumer.to_owned() + "\0"; + + unsafe { + gpiod::gpiod_request_config_set_consumer( + self.config, + consumer.as_ptr() as *const c_char, + ) + } + } + + /// Get the consumer name configured in the request config. + pub fn consumer(&self) -> Result<&str> { + // SAFETY: The string returned by libgpiod is guaranteed to live as long + // as the `struct Config`. + let consumer = unsafe { gpiod::gpiod_request_config_get_consumer(self.config) }; + if consumer.is_null() { + return Err(Error::OperationFailed( + OperationType::RequestConfigGetConsumer, + Errno::last(), + )); + } + + // SAFETY: The string is guaranteed to be valid here by the C API. + str::from_utf8(unsafe { + slice::from_raw_parts(consumer as *const u8, strlen(consumer) as usize) + }) + .map_err(Error::StringNotUtf8) + } + + /// Set the size of the kernel event buffer for the request. + pub fn set_event_buffer_size(&self, size: usize) { + unsafe { gpiod::gpiod_request_config_set_event_buffer_size(self.config, size as c_ulong) } + } + + /// Get the edge event buffer size setting for the request config. + pub fn event_buffer_size(&self) -> usize { + unsafe { gpiod::gpiod_request_config_get_event_buffer_size(self.config) as usize } + } +} + +impl Drop for Config { + /// Free the request config object and release all associated resources. + fn drop(&mut self) { + unsafe { gpiod::gpiod_request_config_free(self.config) } + } +}
Add rust wrapper crate, around the libpiod-sys crate added earlier, to provide a convenient interface for the users. Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> --- bindings/rust/Cargo.toml | 1 + bindings/rust/libgpiod/Cargo.toml | 13 + bindings/rust/libgpiod/src/chip.rs | 253 ++++++++++ bindings/rust/libgpiod/src/edge_event.rs | 101 ++++ bindings/rust/libgpiod/src/event_buffer.rs | 66 +++ bindings/rust/libgpiod/src/info_event.rs | 68 +++ bindings/rust/libgpiod/src/lib.rs | 471 +++++++++++++++++++ bindings/rust/libgpiod/src/line_config.rs | 118 +++++ bindings/rust/libgpiod/src/line_info.rs | 180 +++++++ bindings/rust/libgpiod/src/line_request.rs | 246 ++++++++++ bindings/rust/libgpiod/src/line_settings.rs | 277 +++++++++++ bindings/rust/libgpiod/src/request_config.rs | 96 ++++ 12 files changed, 1890 insertions(+) create mode 100644 bindings/rust/libgpiod/Cargo.toml create mode 100644 bindings/rust/libgpiod/src/chip.rs create mode 100644 bindings/rust/libgpiod/src/edge_event.rs create mode 100644 bindings/rust/libgpiod/src/event_buffer.rs create mode 100644 bindings/rust/libgpiod/src/info_event.rs create mode 100644 bindings/rust/libgpiod/src/lib.rs create mode 100644 bindings/rust/libgpiod/src/line_config.rs create mode 100644 bindings/rust/libgpiod/src/line_info.rs create mode 100644 bindings/rust/libgpiod/src/line_request.rs create mode 100644 bindings/rust/libgpiod/src/line_settings.rs create mode 100644 bindings/rust/libgpiod/src/request_config.rs