[PATCH V2 0/4] libgpiod: Add Rust bindings

[PATCH V2 0/4] libgpiod: Add Rust bindings

[Viresh Kumar]

Hi Bartosz,

This patch adds rust bindings for libgpiod v2.0, this is already partially
tested with the virtio rust backend I am developing, which uses these to talk to
the host kernel.

This is based of the next/post-libgpiod-2.0 branch.

I will be adding testing infrastructure later on, once other bindings are
converted to use gpiosim.

V1->V2:
- Added examples (I tested everything except gpiomon.rs, didn't have right
  hardware/mock device to test).
- Build rust bindings as part of Make, update documentation.

Thanks.

--
Viresh

Viresh Kumar (4):
  libgpiod: Generate rust FFI bindings
  libgpiod: Add rust wrappers
  rust: Add examples
  rust: Integrate building of rust bindings with make

 .gitignore                           |   5 +
 README                               |   8 +-
 TODO                                 |   8 -
 bindings/Makefile.am                 |   6 +
 bindings/rust/Cargo.toml             |  14 +
 bindings/rust/Makefile.am            |  29 ++
 bindings/rust/build.rs               |  60 ++++
 bindings/rust/examples/gpiodetect.rs |  38 +++
 bindings/rust/examples/gpiofind.rs   |  43 +++
 bindings/rust/examples/gpioget.rs    |  45 +++
 bindings/rust/examples/gpioinfo.rs   |  90 ++++++
 bindings/rust/examples/gpiomon.rs    |  73 +++++
 bindings/rust/examples/gpioset.rs    |  55 ++++
 bindings/rust/src/bindings.rs        |  16 +
 bindings/rust/src/chip.rs            | 197 ++++++++++++
 bindings/rust/src/edge_event.rs      |  78 +++++
 bindings/rust/src/event_buffer.rs    |  59 ++++
 bindings/rust/src/info_event.rs      |  70 +++++
 bindings/rust/src/lib.rs             | 268 +++++++++++++++++
 bindings/rust/src/line_config.rs     | 431 +++++++++++++++++++++++++++
 bindings/rust/src/line_info.rs       | 186 ++++++++++++
 bindings/rust/src/line_request.rs    | 217 ++++++++++++++
 bindings/rust/src/request_config.rs  | 118 ++++++++
 bindings/rust/wrapper.h              |   2 +
 configure.ac                         |  16 +
 25 files changed, 2121 insertions(+), 11 deletions(-)
 create mode 100644 bindings/rust/Cargo.toml
 create mode 100644 bindings/rust/Makefile.am
 create mode 100644 bindings/rust/build.rs
 create mode 100644 bindings/rust/examples/gpiodetect.rs
 create mode 100644 bindings/rust/examples/gpiofind.rs
 create mode 100644 bindings/rust/examples/gpioget.rs
 create mode 100644 bindings/rust/examples/gpioinfo.rs
 create mode 100644 bindings/rust/examples/gpiomon.rs
 create mode 100644 bindings/rust/examples/gpioset.rs
 create mode 100644 bindings/rust/src/bindings.rs
 create mode 100644 bindings/rust/src/chip.rs
 create mode 100644 bindings/rust/src/edge_event.rs
 create mode 100644 bindings/rust/src/event_buffer.rs
 create mode 100644 bindings/rust/src/info_event.rs
 create mode 100644 bindings/rust/src/lib.rs
 create mode 100644 bindings/rust/src/line_config.rs
 create mode 100644 bindings/rust/src/line_info.rs
 create mode 100644 bindings/rust/src/line_request.rs
 create mode 100644 bindings/rust/src/request_config.rs
 create mode 100644 bindings/rust/wrapper.h

-- 
2.31.1.272.g89b43f80a514

[PATCH V2 1/4] libgpiod: Generate rust FFI bindings

[Viresh Kumar]

Build FFI bindings for the libgpiod helpers.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 .gitignore                    |  5 +++
 bindings/rust/Cargo.toml      | 12 +++++++
 bindings/rust/build.rs        | 60 +++++++++++++++++++++++++++++++++++
 bindings/rust/src/bindings.rs | 16 ++++++++++
 bindings/rust/src/lib.rs      |  0
 bindings/rust/wrapper.h       |  2 ++
 6 files changed, 95 insertions(+)
 create mode 100644 bindings/rust/Cargo.toml
 create mode 100644 bindings/rust/build.rs
 create mode 100644 bindings/rust/src/bindings.rs
 create mode 100644 bindings/rust/src/lib.rs
 create mode 100644 bindings/rust/wrapper.h

diff --git a/.gitignore b/.gitignore
index 2d7cc7fc0758..0d3a842734bf 100644
--- a/.gitignore
+++ b/.gitignore
@@ -29,3 +29,8 @@ libtool
 *-libtool
 m4/
 stamp-h1
+
+# Added by cargo
+
+bindings/rust/target
+bindings/rust/Cargo.lock
diff --git a/bindings/rust/Cargo.toml b/bindings/rust/Cargo.toml
new file mode 100644
index 000000000000..62f3d52ddb0f
--- /dev/null
+++ b/bindings/rust/Cargo.toml
@@ -0,0 +1,12 @@
+[package]
+name = "libgpiod"
+version = "0.1.0"
+edition = "2021"
+
+# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
+
+[dependencies]
+
+[build-dependencies]
+bindgen = "0.59.1"
+cc = "1.0.46"
diff --git a/bindings/rust/build.rs b/bindings/rust/build.rs
new file mode 100644
index 000000000000..cd776332bbb9
--- /dev/null
+++ b/bindings/rust/build.rs
@@ -0,0 +1,60 @@
+extern crate bindgen;
+
+use std::env;
+use std::path::PathBuf;
+
+fn generate_bindings() {
+    // Tell cargo to invalidate the built crate whenever the wrapper changes
+    println!("cargo:rerun-if-changed=wrapper.h");
+
+    // The bindgen::Builder is the main entry point
+    // to bindgen, and lets you build up options for
+    // the resulting bindings.
+    let bindings = bindgen::Builder::default()
+        // The input header we would like to generate
+        // bindings for.
+        .header("wrapper.h")
+        // Tell cargo to invalidate the built crate whenever any of the
+        // included header files changed.
+        .parse_callbacks(Box::new(bindgen::CargoCallbacks))
+        // Finish the builder and generate the bindings.
+        .generate()
+        // Unwrap the Result and panic on failure.
+        .expect("Unable to generate bindings");
+
+    // Write the bindings to the $OUT_DIR/bindings.rs file.
+    let out_path = PathBuf::from(env::var("OUT_DIR").unwrap());
+    bindings
+        .write_to_file(out_path.join("bindings.rs"))
+        .expect("Couldn't write bindings!");
+}
+
+fn build_gpiod() {
+    // Tell Cargo that if the given file changes, to rerun this build script.
+    println!("cargo:rerun-if-changed=../../lib/");
+
+    let files = vec![
+        "../../lib/chip.c",
+        "../../lib/edge-event.c",
+        "../../lib/info-event.c",
+        "../../lib/internal.c",
+        "../../lib/line-config.c",
+        "../../lib/line-info.c",
+        "../../lib/line-request.c",
+        "../../lib/misc.c",
+        "../../lib/request-config.c",
+    ];
+
+    // Use the `cc` crate to build a C file and statically link it.
+    cc::Build::new()
+        .files(files)
+        .define("_GNU_SOURCE", None)
+        .define("GPIOD_VERSION_STR", "\"libgpio-rust\"")
+        .include("../../include")
+        .compile("gpiod");
+}
+
+fn main() {
+    generate_bindings();
+    build_gpiod();
+}
diff --git a/bindings/rust/src/bindings.rs b/bindings/rust/src/bindings.rs
new file mode 100644
index 000000000000..7d6caa7d9c11
--- /dev/null
+++ b/bindings/rust/src/bindings.rs
@@ -0,0 +1,16 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#[allow(
+    clippy::all,
+    deref_nullptr,
+    dead_code,
+    non_camel_case_types,
+    non_upper_case_globals,
+    non_snake_case,
+    improper_ctypes
+)]
+
+mod bindings_raw {
+    include!(concat!(env!("OUT_DIR"), "/bindings.rs"));
+}
+pub use bindings_raw::*;
diff --git a/bindings/rust/src/lib.rs b/bindings/rust/src/lib.rs
new file mode 100644
index 000000000000..e69de29bb2d1
diff --git a/bindings/rust/wrapper.h b/bindings/rust/wrapper.h
new file mode 100644
index 000000000000..50dc5f4db406
--- /dev/null
+++ b/bindings/rust/wrapper.h
@@ -0,0 +1,2 @@
+#include <string.h>
+#include "../../include/gpiod.h"
-- 
2.31.1.272.g89b43f80a514

[PATCH V2 2/4] libgpiod: Add rust wrappers

[Viresh Kumar]

Add rust wrappers around FFI bindings to provide a convenient interface
for the users.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 bindings/rust/Cargo.toml            |   2 +
 bindings/rust/src/chip.rs           | 197 +++++++++++++
 bindings/rust/src/edge_event.rs     |  78 +++++
 bindings/rust/src/event_buffer.rs   |  59 ++++
 bindings/rust/src/info_event.rs     |  70 +++++
 bindings/rust/src/lib.rs            | 268 +++++++++++++++++
 bindings/rust/src/line_config.rs    | 431 ++++++++++++++++++++++++++++
 bindings/rust/src/line_info.rs      | 186 ++++++++++++
 bindings/rust/src/line_request.rs   | 217 ++++++++++++++
 bindings/rust/src/request_config.rs | 118 ++++++++
 10 files changed, 1626 insertions(+)
 create mode 100644 bindings/rust/src/chip.rs
 create mode 100644 bindings/rust/src/edge_event.rs
 create mode 100644 bindings/rust/src/event_buffer.rs
 create mode 100644 bindings/rust/src/info_event.rs
 create mode 100644 bindings/rust/src/line_config.rs
 create mode 100644 bindings/rust/src/line_info.rs
 create mode 100644 bindings/rust/src/line_request.rs
 create mode 100644 bindings/rust/src/request_config.rs

diff --git a/bindings/rust/Cargo.toml b/bindings/rust/Cargo.toml
index 62f3d52ddb0f..d57dfd65ba7d 100644
--- a/bindings/rust/Cargo.toml
+++ b/bindings/rust/Cargo.toml
@@ -6,6 +6,8 @@ edition = "2021"
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
 
 [dependencies]
+thiserror = "1.0"
+vmm-sys-util = "=0.9.0"
 
 [build-dependencies]
 bindgen = "0.59.1"
diff --git a/bindings/rust/src/chip.rs b/bindings/rust/src/chip.rs
new file mode 100644
index 000000000000..144f5ef9c570
--- /dev/null
+++ b/bindings/rust/src/chip.rs
@@ -0,0 +1,197 @@
+// SPDX-License-Identifier: Apache-2.0 AND BSD-3-Clause
+//
+// Copyright 2021 Linaro Ltd. All Rights Reserved.
+//     Viresh Kumar <viresh.kumar@linaro.org>
+
+use std::os::raw::c_char;
+use std::sync::Arc;
+use std::time::Duration;
+use std::{slice, str};
+
+use vmm_sys_util::errno::Error as IoError;
+
+use super::{bindings, Error, Result};
+use crate::info_event::GpiodInfoEvent;
+use crate::line_config::GpiodLineConfig;
+use crate::line_info::GpiodLineInfo;
+use crate::line_request::GpiodLineRequest;
+use crate::request_config::GpiodRequestConfig;
+
+/// 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.
+pub(crate) struct GpiodChipInternal {
+    chip: *mut bindings::gpiod_chip,
+}
+
+impl GpiodChipInternal {
+    /// Find a GPIO chip by path.
+    pub(crate) fn open(path: &str) -> Result<Self> {
+        let chip = unsafe { bindings::gpiod_chip_open(path.as_ptr() as *const c_char) };
+        if chip.is_null() {
+            return Err(Error::OperationFailed("GpiodChip open", IoError::last()));
+        }
+
+        Ok(Self { chip })
+    }
+
+    /// Private helper, Returns gpiod_chip
+    pub(crate) fn chip(&self) -> *mut bindings::gpiod_chip {
+        self.chip
+    }
+}
+
+impl Drop for GpiodChipInternal {
+    /// Close the GPIO chip and release all associated resources.
+    fn drop(&mut self) {
+        unsafe { bindings::gpiod_chip_close(self.chip) }
+    }
+}
+
+pub struct GpiodChip {
+    ichip: Arc<GpiodChipInternal>,
+}
+
+impl GpiodChip {
+    /// Find a GPIO chip by path.
+    pub fn open(path: &str) -> Result<Self> {
+        Ok(Self {
+            ichip: Arc::new(GpiodChipInternal::open(path)?),
+        })
+    }
+
+    /// Get the GPIO chip name as represented in the kernel.
+    pub fn get_name(&self) -> Result<&str> {
+        // SAFETY: The string returned by libgpiod is guaranteed to live as long
+        // as the `struct GpiodChip`.
+        let name = unsafe { bindings::gpiod_chip_get_name(self.ichip.chip()) };
+        if name.is_null() {
+            return Err(Error::NameNotFound("GPIO chip name"));
+        }
+
+        unsafe {
+            str::from_utf8(slice::from_raw_parts(
+                name as *const u8,
+                bindings::strlen(name) as usize,
+            ))
+            .map_err(Error::InvalidString)
+        }
+    }
+
+    /// Get the GPIO chip label as represented in the kernel.
+    pub fn get_label(&self) -> Result<&str> {
+        // SAFETY: The string returned by libgpiod is guaranteed to live as long
+        // as the `struct GpiodChip`.
+        let label = unsafe { bindings::gpiod_chip_get_label(self.ichip.chip()) };
+        if label.is_null() {
+            return Err(Error::NameNotFound("GPIO chip label"));
+        }
+
+        unsafe {
+            str::from_utf8(slice::from_raw_parts(
+                label as *const u8,
+                bindings::strlen(label) as usize,
+            ))
+            .map_err(Error::InvalidString)
+        }
+    }
+
+    /// Get the path used to find this GPIO chip.
+    pub fn get_path(&self) -> Result<&str> {
+        // SAFETY: The string returned by libgpiod is guaranteed to live as long
+        // as the `struct GpiodChip`.
+        let path = unsafe { bindings::gpiod_chip_get_path(self.ichip.chip()) };
+        if path.is_null() {
+            return Err(Error::NameNotFound("GPIO chip path"));
+        }
+
+        unsafe {
+            str::from_utf8(slice::from_raw_parts(
+                path as *const u8,
+                bindings::strlen(path) as usize,
+            ))
+            .map_err(Error::InvalidString)
+        }
+    }
+
+    /// Get the number of GPIO lines exposed by this chip.
+    pub fn get_num_lines(&self) -> u32 {
+        unsafe { bindings::gpiod_chip_get_num_lines(self.ichip.chip()) }
+    }
+
+    /// Get the current snapshot of information about the line at given offset.
+    pub fn line_info(&self, offset: u32) -> Result<GpiodLineInfo> {
+        GpiodLineInfo::new(self.ichip.clone(), offset, false)
+    }
+
+    /// Get the current snapshot of information about the line at given offset
+    /// and optionally start watching it for future changes.
+    pub fn watch_line_info(&self, offset: u32) -> Result<GpiodLineInfo> {
+        GpiodLineInfo::new(self.ichip.clone(), offset, true)
+    }
+
+    /// Get the file descriptor associated with this chip.
+    ///
+    /// The returned file descriptor must not be closed by the caller, else other methods for the
+    /// `struct GpiodChip` may fail.
+    pub fn get_fd(&self) -> Result<u32> {
+        let fd = unsafe { bindings::gpiod_chip_get_fd(self.ichip.chip()) };
+
+        if fd < 0 {
+            Err(Error::OperationFailed("GpiodChip get-fd", IoError::last()))
+        } else {
+            Ok(fd as u32)
+        }
+    }
+
+    /// Wait for line status events on any of the watched lines exposed by this
+    /// chip.
+    pub fn info_event_wait(&self, timeout: Duration) -> Result<()> {
+        let ret = unsafe {
+            bindings::gpiod_chip_info_event_wait(self.ichip.chip(), timeout.as_nanos() as u64)
+        };
+
+        match ret {
+            -1 => Err(Error::OperationFailed(
+                "GpiodChip info-event-wait",
+                IoError::last(),
+            )),
+            0 => Err(Error::OperationTimedOut),
+            _ => Ok(()),
+        }
+    }
+
+    /// Read a single line status change event from this chip. If no events are
+    /// pending, this function will block.
+    pub fn info_event_read(&self) -> Result<GpiodInfoEvent> {
+        GpiodInfoEvent::new(&self.ichip.clone())
+    }
+
+    /// Map a GPIO line's name to its offset within the chip.
+    pub fn find_line(&self, name: &str) -> Result<u32> {
+        let ret = unsafe {
+            bindings::gpiod_chip_find_line(self.ichip.chip(), name.as_ptr() as *const c_char)
+        };
+
+        if ret == -1 {
+            Err(Error::OperationFailed(
+                "GpiodChip find-line",
+                IoError::last(),
+            ))
+        } else {
+            Ok(ret as u32)
+        }
+    }
+
+    /// Request a set of lines for exclusive usage.
+    pub fn request_lines(
+        &self,
+        rconfig: &GpiodRequestConfig,
+        lconfig: &GpiodLineConfig,
+    ) -> Result<GpiodLineRequest> {
+        GpiodLineRequest::new(&self.ichip.clone(), rconfig, lconfig)
+    }
+}
diff --git a/bindings/rust/src/edge_event.rs b/bindings/rust/src/edge_event.rs
new file mode 100644
index 000000000000..5fe292ac69aa
--- /dev/null
+++ b/bindings/rust/src/edge_event.rs
@@ -0,0 +1,78 @@
+// SPDX-License-Identifier: Apache-2.0 AND BSD-3-Clause
+//
+// Copyright 2021 Linaro Ltd. All Rights Reserved.
+//     Viresh Kumar <viresh.kumar@linaro.org>
+
+use std::time::Duration;
+
+use vmm_sys_util::errno::Error as IoError;
+
+use super::{bindings, EdgeEvent, Error, Result};
+use crate::event_buffer::GpiodEdgeEventBuffer;
+
+/// Line edge events handling
+///
+/// An edge event object contains information about a single line event. It
+/// contains the event type, timestamp and the offset of the line on which the
+/// event occurred as well as two sequential numbers (global for all lines
+/// associated with the parent chip and local for this line only).
+///
+/// For performance and to limit the number of memory allocations when a lot of
+/// events are being read, edge events are stored in an edge-event buffer object.
+
+pub struct GpiodEdgeEvent {
+    event: *mut bindings::gpiod_edge_event,
+}
+
+impl GpiodEdgeEvent {
+    /// Get an event stored in the buffer.
+    pub fn new(buffer: &GpiodEdgeEventBuffer, index: u64) -> Result<Self> {
+        let event = unsafe { bindings::gpiod_edge_event_buffer_get_event(buffer.buffer(), index) };
+        if event.is_null() {
+            return Err(Error::OperationFailed(
+                "GpiodEdgeEvent buffer-get-event",
+                IoError::last(),
+            ));
+        }
+
+        Ok(Self { event })
+    }
+
+    /// Get the event type.
+    pub fn get_event_type(&self) -> Result<EdgeEvent> {
+        EdgeEvent::new(unsafe { bindings::gpiod_edge_event_get_event_type(self.event) } as u32)
+    }
+
+    /// Get the timestamp of the event.
+    pub fn get_timestamp(&self) -> Duration {
+        Duration::from_nanos(unsafe { bindings::gpiod_edge_event_get_timestamp(self.event) })
+    }
+
+    /// Get the offset of the line on which the event was triggered.
+    pub fn get_line_offset(&self) -> u32 {
+        unsafe { bindings::gpiod_edge_event_get_line_offset(self.event) }
+    }
+
+    /// Get the global sequence number of this event.
+    ///
+    /// Returns sequence number of the event relative to all lines in the
+    /// associated line request.
+    pub fn get_global_seqno(&self) -> u64 {
+        unsafe { bindings::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 this line within the
+    /// lifetime of the associated line request.
+    pub fn get_line_seqno(&self) -> u64 {
+        unsafe { bindings::gpiod_edge_event_get_line_seqno(self.event) }
+    }
+}
+
+impl Drop for GpiodEdgeEvent {
+    /// Free the edge event object and release all associated resources.
+    fn drop(&mut self) {
+        unsafe { bindings::gpiod_edge_event_free(self.event) }
+    }
+}
diff --git a/bindings/rust/src/event_buffer.rs b/bindings/rust/src/event_buffer.rs
new file mode 100644
index 000000000000..fd702e91818d
--- /dev/null
+++ b/bindings/rust/src/event_buffer.rs
@@ -0,0 +1,59 @@
+// SPDX-License-Identifier: Apache-2.0 AND BSD-3-Clause
+//
+// Copyright 2021 Linaro Ltd. All Rights Reserved.
+//     Viresh Kumar <viresh.kumar@linaro.org>
+
+use vmm_sys_util::errno::Error as IoError;
+
+use super::{bindings, Error, Result};
+use crate::edge_event::GpiodEdgeEvent;
+
+/// Line edge events buffer
+pub struct GpiodEdgeEventBuffer {
+    buffer: *mut bindings::gpiod_edge_event_buffer,
+}
+
+impl GpiodEdgeEventBuffer {
+    /// 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: u32) -> Result<Self> {
+        let buffer = unsafe { bindings::gpiod_edge_event_buffer_new(capacity) };
+        if buffer.is_null() {
+            return Err(Error::OperationFailed(
+                "GpiodEdgeEventBuffer new",
+                IoError::last(),
+            ));
+        }
+
+        Ok(Self { buffer })
+    }
+
+    /// Private helper, Returns gpiod_edge_event_buffer
+    pub(crate) fn buffer(&self) -> *mut bindings::gpiod_edge_event_buffer {
+        self.buffer
+    }
+
+    /// Get the capacity of the event buffer.
+    pub fn get_capacity(&self) -> u32 {
+        unsafe { bindings::gpiod_edge_event_buffer_get_capacity(self.buffer) }
+    }
+
+    /// Read an event stored in the buffer.
+    pub fn get_event(&self, index: u64) -> Result<GpiodEdgeEvent> {
+        GpiodEdgeEvent::new(self, index)
+    }
+
+    /// Get the number of events this buffers stores.
+    pub fn num_events(&self) -> u32 {
+        unsafe { bindings::gpiod_edge_event_buffer_num_events(self.buffer) }
+    }
+}
+
+impl Drop for GpiodEdgeEventBuffer {
+    /// Free the edge event buffer and release all associated resources.
+    fn drop(&mut self) {
+        unsafe { bindings::gpiod_edge_event_buffer_free(self.buffer) };
+    }
+}
diff --git a/bindings/rust/src/info_event.rs b/bindings/rust/src/info_event.rs
new file mode 100644
index 000000000000..e0f82b31548f
--- /dev/null
+++ b/bindings/rust/src/info_event.rs
@@ -0,0 +1,70 @@
+// SPDX-License-Identifier: Apache-2.0 AND BSD-3-Clause
+//
+// Copyright 2021 Linaro Ltd. All Rights Reserved.
+//     Viresh Kumar <viresh.kumar@linaro.org>
+
+use std::convert::TryFrom;
+use std::sync::Arc;
+use std::time::Duration;
+
+use vmm_sys_util::errno::Error as IoError;
+
+use super::chip::GpiodChipInternal;
+use super::{bindings, Error, Event, Result};
+use crate::line_info::GpiodLineInfo;
+
+/// 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.
+
+pub struct GpiodInfoEvent {
+    event: *mut bindings::gpiod_info_event,
+}
+
+impl GpiodInfoEvent {
+    /// Get a single chip's line's status change event.
+    pub(crate) fn new(ichip: &Arc<GpiodChipInternal>) -> Result<Self> {
+        let event = unsafe { bindings::gpiod_chip_info_event_read(ichip.chip()) };
+        if event.is_null() {
+            return Err(Error::OperationFailed(
+                "GpiodInfoEvent event-read",
+                IoError::last(),
+            ));
+        }
+
+        Ok(Self { event })
+    }
+
+    /// Private helper, Returns gpiod_info_event
+    pub(crate) fn event(&self) -> *mut bindings::gpiod_info_event {
+        self.event
+    }
+
+    /// Get the event type of this status change event.
+    pub fn get_event_type(&self) -> Result<Event> {
+        Event::new(unsafe { bindings::gpiod_info_event_get_event_type(self.event) } as u32)
+    }
+
+    /// Get the timestamp of the event.
+    pub fn get_timestamp(&self) -> Duration {
+        Duration::from_nanos(unsafe { bindings::gpiod_info_event_get_timestamp(self.event) })
+    }
+
+    /// Get the line-info object associated with this event.
+    pub fn line_info(&self) -> Result<GpiodLineInfo> {
+        GpiodLineInfo::try_from(self)
+    }
+}
+
+impl Drop for GpiodInfoEvent {
+    /// Free the info event object and release all associated resources.
+    fn drop(&mut self) {
+        unsafe { bindings::gpiod_info_event_free(self.event) }
+    }
+}
diff --git a/bindings/rust/src/lib.rs b/bindings/rust/src/lib.rs
index e69de29bb2d1..cbdef64fdb8b 100644
--- a/bindings/rust/src/lib.rs
+++ b/bindings/rust/src/lib.rs
@@ -0,0 +1,268 @@
+// SPDX-License-Identifier: Apache-2.0 AND BSD-3-Clause
+//
+// Rust wrappers for GPIOD APIs
+//
+// Copyright 2021 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.
+
+mod bindings;
+pub mod chip;
+pub mod edge_event;
+pub mod event_buffer;
+pub mod info_event;
+pub mod line_config;
+pub mod line_info;
+pub mod line_request;
+pub mod request_config;
+
+use std::os::raw::c_char;
+use std::{slice, str};
+
+use thiserror::Error as ThisError;
+use vmm_sys_util::errno::Error as IoError;
+
+/// 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 find {0}")]
+    NameNotFound(&'static str),
+    #[error("Invalid String: {0:?}")]
+    InvalidString(str::Utf8Error),
+    #[error("Invalid {0} value: {1}")]
+    InvalidValue(&'static str, u32),
+    #[error("Operation {0} Failed: {1}")]
+    OperationFailed(&'static str, IoError),
+    #[error("Operation Timed-out")]
+    OperationTimedOut,
+}
+
+/// Direction settings.
+pub enum Direction {
+    /// Request the line(s), but didn't change current direction.
+    AsIs,
+    /// Direction is input - we're reading the state of a GPIO line.
+    Input,
+    /// Direction is output - we're driving the GPIO line.
+    Output,
+}
+
+impl Direction {
+    fn new(dir: u32) -> Result<Self> {
+        match dir {
+            bindings::GPIOD_LINE_DIRECTION_AS_IS => Ok(Direction::AsIs),
+            bindings::GPIOD_LINE_DIRECTION_INPUT => Ok(Direction::Input),
+            bindings::GPIOD_LINE_DIRECTION_OUTPUT => Ok(Direction::Output),
+            _ => Err(Error::InvalidValue("direction", dir)),
+        }
+    }
+
+    fn gpiod_direction(&self) -> u32 {
+        match self {
+            Direction::AsIs => bindings::GPIOD_LINE_DIRECTION_AS_IS,
+            Direction::Input => bindings::GPIOD_LINE_DIRECTION_INPUT,
+            Direction::Output => bindings::GPIOD_LINE_DIRECTION_OUTPUT,
+        }
+    }
+}
+
+/// Internal bias settings.
+pub enum Bias {
+    /// Don't change the bias setting when applying line config.
+    AsIs,
+    /// The internal bias state is unknown.
+    Unknown,
+    /// 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<Self> {
+        match bias {
+            bindings::GPIOD_LINE_BIAS_AS_IS => Ok(Bias::AsIs),
+            bindings::GPIOD_LINE_BIAS_UNKNOWN => Ok(Bias::Unknown),
+            bindings::GPIOD_LINE_BIAS_DISABLED => Ok(Bias::Disabled),
+            bindings::GPIOD_LINE_BIAS_PULL_UP => Ok(Bias::PullUp),
+            bindings::GPIOD_LINE_BIAS_PULL_DOWN => Ok(Bias::PullDown),
+            _ => Err(Error::InvalidValue("bias", bias)),
+        }
+    }
+
+    fn gpiod_bias(&self) -> u32 {
+        match self {
+            Bias::AsIs => bindings::GPIOD_LINE_BIAS_AS_IS,
+            Bias::Unknown => bindings::GPIOD_LINE_BIAS_UNKNOWN,
+            Bias::Disabled => bindings::GPIOD_LINE_BIAS_DISABLED,
+            Bias::PullUp => bindings::GPIOD_LINE_BIAS_PULL_UP,
+            Bias::PullDown => bindings::GPIOD_LINE_BIAS_PULL_DOWN,
+        }
+    }
+}
+
+/// Drive settings.
+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 {
+            bindings::GPIOD_LINE_DRIVE_PUSH_PULL => Ok(Drive::PushPull),
+            bindings::GPIOD_LINE_DRIVE_OPEN_DRAIN => Ok(Drive::OpenDrain),
+            bindings::GPIOD_LINE_DRIVE_OPEN_SOURCE => Ok(Drive::OpenSource),
+            _ => Err(Error::InvalidValue("drive", drive)),
+        }
+    }
+
+    fn gpiod_drive(&self) -> u32 {
+        match self {
+            Drive::PushPull => bindings::GPIOD_LINE_DRIVE_PUSH_PULL,
+            Drive::OpenDrain => bindings::GPIOD_LINE_DRIVE_OPEN_DRAIN,
+            Drive::OpenSource => bindings::GPIOD_LINE_DRIVE_OPEN_SOURCE,
+        }
+    }
+}
+
+/// Edge detection settings.
+pub enum Edge {
+    /// Line edge detection is disabled.
+    None,
+    /// Line detects rising edge events.
+    Rising,
+    /// Line detect falling edge events.
+    Falling,
+    /// Line detects both rising and falling edge events.
+    Both,
+}
+
+impl Edge {
+    fn new(edge: u32) -> Result<Self> {
+        match edge {
+            bindings::GPIOD_LINE_EDGE_NONE => Ok(Edge::None),
+            bindings::GPIOD_LINE_EDGE_RISING => Ok(Edge::Rising),
+            bindings::GPIOD_LINE_EDGE_FALLING => Ok(Edge::Falling),
+            bindings::GPIOD_LINE_EDGE_BOTH => Ok(Edge::Both),
+            _ => Err(Error::InvalidValue("edge", edge)),
+        }
+    }
+
+    fn gpiod_edge(&self) -> u32 {
+        match self {
+            Edge::None => bindings::GPIOD_LINE_EDGE_NONE,
+            Edge::Rising => bindings::GPIOD_LINE_EDGE_RISING,
+            Edge::Falling => bindings::GPIOD_LINE_EDGE_FALLING,
+            Edge::Both => bindings::GPIOD_LINE_EDGE_BOTH,
+        }
+    }
+}
+
+/// Event clock settings.
+pub enum EventClock {
+    /// Line uses the monotonic clock for edge event timestamps.
+    Monotonic,
+    /// Line uses the realtime clock for edge event timestamps.
+    Realtime,
+}
+
+impl EventClock {
+    fn new(clock: u32) -> Result<Self> {
+        match clock {
+            bindings::GPIOD_LINE_EVENT_CLOCK_MONOTONIC => Ok(EventClock::Monotonic),
+            bindings::GPIOD_LINE_EVENT_CLOCK_REALTIME => Ok(EventClock::Realtime),
+            _ => Err(Error::InvalidValue("event clock", clock)),
+        }
+    }
+
+    fn gpiod_clock(&self) -> u32 {
+        match self {
+            EventClock::Monotonic => bindings::GPIOD_LINE_EVENT_CLOCK_MONOTONIC,
+            EventClock::Realtime => bindings::GPIOD_LINE_EVENT_CLOCK_REALTIME,
+        }
+    }
+}
+
+/// Line status change event types.
+pub enum Event {
+    /// Line has been requested.
+    LineRequested,
+    /// Previously requested line has been released.
+    LineReleased,
+    /// Line configuration has changed.
+    LineConfigChanged,
+}
+
+impl Event {
+    fn new(event: u32) -> Result<Self> {
+        match event {
+            bindings::GPIOD_INFO_EVENT_LINE_REQUESTED => Ok(Event::LineRequested),
+            bindings::GPIOD_INFO_EVENT_LINE_RELEASED => Ok(Event::LineReleased),
+            bindings::GPIOD_INFO_EVENT_LINE_CONFIG_CHANGED => Ok(Event::LineConfigChanged),
+            _ => Err(Error::InvalidValue("event", event)),
+        }
+    }
+}
+
+/// Edge event types.
+pub enum EdgeEvent {
+    /// Rising edge event.
+    Rising,
+    /// Falling edge event.
+    Falling,
+}
+
+impl EdgeEvent {
+    fn new(event: u32) -> Result<Self> {
+        match event {
+            bindings::GPIOD_EDGE_EVENT_RISING_EDGE => Ok(EdgeEvent::Rising),
+            bindings::GPIOD_EDGE_EVENT_FALLING_EDGE => Ok(EdgeEvent::Falling),
+            _ => Err(Error::InvalidValue("edge event", event)),
+        }
+    }
+}
+
+/// 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 gpiod_is_gpiochip_device(path: &str) -> bool {
+    unsafe { bindings::gpiod_is_gpiochip_device(path.as_ptr() as *const c_char) }
+}
+
+/// Get the API version of the library as a human-readable string.
+pub fn gpiod_version_string() -> Result<&'static str> {
+    // SAFETY: The string returned by libgpiod is guaranteed to live forever.
+    let version = unsafe { bindings::gpiod_version_string() };
+
+    if version.is_null() {
+        return Err(Error::NameNotFound("GPIO library version"));
+    }
+
+    unsafe {
+        str::from_utf8(slice::from_raw_parts(
+            version as *const u8,
+            bindings::strlen(version) as usize,
+        ))
+        .map_err(Error::InvalidString)
+    }
+}
diff --git a/bindings/rust/src/line_config.rs b/bindings/rust/src/line_config.rs
new file mode 100644
index 000000000000..e79affe87e0c
--- /dev/null
+++ b/bindings/rust/src/line_config.rs
@@ -0,0 +1,431 @@
+// SPDX-License-Identifier: Apache-2.0 AND BSD-3-Clause
+//
+// Copyright 2021 Linaro Ltd. All Rights Reserved.
+//     Viresh Kumar <viresh.kumar@linaro.org>
+
+use std::time::Duration;
+
+use vmm_sys_util::errno::Error as IoError;
+
+use super::{bindings, Bias, Direction, Drive, Edge, Error, EventClock, Result};
+
+/// Line configuration objects.
+///
+/// The line-config object stores the configuration for lines that can be used
+/// in two cases: when making a line request and when reconfiguring a set of
+/// already requested lines. The mutators for the line request don't return
+/// errors. If the set of options is too complex to be translated into kernel
+/// uAPI structures - the error will be returned at the time of the request or
+/// reconfiguration. If an invalid value was passed to any of the getters - the
+/// default value will be silently used instead. Each option can be set
+/// globally, for a single line offset or for multiple line offsets.
+pub struct GpiodLineConfig {
+    config: *mut bindings::gpiod_line_config,
+}
+
+impl GpiodLineConfig {
+    /// Create a new line config object.
+    pub fn new() -> Result<Self> {
+        let config = unsafe { bindings::gpiod_line_config_new() };
+
+        if config.is_null() {
+            return Err(Error::OperationFailed(
+                "GpiodLineConfig new",
+                IoError::last(),
+            ));
+        }
+
+        Ok(Self { config })
+    }
+
+    /// Private helper, Returns gpiod_line_config
+    pub(crate) fn config(&self) -> *mut bindings::gpiod_line_config {
+        self.config
+    }
+
+    /// Resets the entire configuration stored in this object. This is useful if
+    /// the user wants to reuse the object without reallocating it.
+    pub fn reset(&mut self) {
+        unsafe { bindings::gpiod_line_config_reset(self.config) }
+    }
+
+    /// Set the direction for all lines.
+    pub fn set_direction(&mut self, direction: Direction) {
+        unsafe {
+            bindings::gpiod_line_config_set_direction(
+                self.config,
+                direction.gpiod_direction() as i32,
+            )
+        }
+    }
+
+    /// Set the direction for a single line at given offset.
+    pub fn set_direction_offset(&mut self, direction: Direction, offset: u32) {
+        unsafe {
+            bindings::gpiod_line_config_set_direction_offset(
+                self.config,
+                direction.gpiod_direction() as i32,
+                offset,
+            )
+        }
+    }
+
+    /// Set the direction for a subset of lines.
+    pub fn set_direction_subset(&mut self, direction: Direction, offsets: &[u32]) {
+        unsafe {
+            bindings::gpiod_line_config_set_direction_subset(
+                self.config,
+                direction.gpiod_direction() as i32,
+                offsets.len() as u32,
+                offsets.as_ptr(),
+            )
+        }
+    }
+
+    /// Get the direction of a given line.
+    ///
+    /// If an offset is used for which no config was provided, the function will
+    /// return the global default value.
+    pub fn get_direction(&self, offset: u32) -> Result<Direction> {
+        Direction::new(
+            unsafe { bindings::gpiod_line_config_get_direction(self.config, offset) } as u32,
+        )
+    }
+
+    /// Set the edge event detection for all lines.
+    pub fn set_edge_detection(&mut self, edge: Edge) {
+        unsafe {
+            bindings::gpiod_line_config_set_edge_detection(self.config, edge.gpiod_edge() as i32)
+        }
+    }
+
+    /// Set the edge event detection for a single line at given offset.
+    pub fn set_edge_detection_offset(&mut self, edge: Edge, offset: u32) {
+        unsafe {
+            bindings::gpiod_line_config_set_edge_detection_offset(
+                self.config,
+                edge.gpiod_edge() as i32,
+                offset,
+            )
+        }
+    }
+
+    /// Set the edge event detection for a subset of lines.
+    pub fn set_edge_detection_subset(&mut self, edge: Edge, offsets: &[u32]) {
+        unsafe {
+            bindings::gpiod_line_config_set_edge_detection_subset(
+                self.config,
+                edge.gpiod_edge() as i32,
+                offsets.len() as u32,
+                offsets.as_ptr(),
+            )
+        }
+    }
+
+    /// Get the edge event detection setting for a given line.
+    ///
+    /// Returns edge event detection setting that would have been used for given
+    /// offset if the config object was used in a request at the time of the
+    /// call. If an offset is used for which no config was provided, the
+    /// function will return the global default value.
+    pub fn get_edge_detection(&self, offset: u32) -> Result<Edge> {
+        Edge::new(
+            unsafe { bindings::gpiod_line_config_get_edge_detection(self.config, offset) } as u32,
+        )
+    }
+
+    /// Set the bias of all lines.
+    pub fn set_bias(&mut self, bias: Bias) {
+        unsafe { bindings::gpiod_line_config_set_bias(self.config, bias.gpiod_bias() as i32) }
+    }
+
+    /// Set the bias for a single line at given offset.
+    pub fn set_bias_offset(&mut self, bias: Bias, offset: u32) {
+        unsafe {
+            bindings::gpiod_line_config_set_bias_offset(
+                self.config,
+                bias.gpiod_bias() as i32,
+                offset,
+            )
+        }
+    }
+
+    /// Set the bias for a subset of lines.
+    pub fn set_bias_subset(&mut self, bias: Bias, offsets: &[u32]) {
+        unsafe {
+            bindings::gpiod_line_config_set_bias_subset(
+                self.config,
+                bias.gpiod_bias() as i32,
+                offsets.len() as u32,
+                offsets.as_ptr(),
+            )
+        }
+    }
+
+    /// Get the bias setting for a given line.
+    ///
+    /// Returns Bias setting that would have been used for given offset if the
+    /// config object was used in a request at the time of the call. If an
+    /// offset is used for which no config was provided, the function will
+    /// return the global default value.
+    pub fn get_bias(&self, offset: u32) -> Result<Bias> {
+        Bias::new(unsafe { bindings::gpiod_line_config_get_bias(self.config, offset) } as u32)
+    }
+
+    /// Set the drive of all lines.
+    pub fn set_drive(&mut self, drive: Drive) {
+        unsafe { bindings::gpiod_line_config_set_drive(self.config, drive.gpiod_drive() as i32) }
+    }
+
+    /// Set the drive for a single line at given offset.
+    pub fn set_drive_offset(&mut self, drive: Drive, offset: u32) {
+        unsafe {
+            bindings::gpiod_line_config_set_drive_offset(
+                self.config,
+                drive.gpiod_drive() as i32,
+                offset,
+            )
+        }
+    }
+
+    /// Set the drive for a subset of lines.
+    pub fn set_drive_subset(&mut self, drive: Drive, offsets: &[u32]) {
+        unsafe {
+            bindings::gpiod_line_config_set_drive_subset(
+                self.config,
+                drive.gpiod_drive() as i32,
+                offsets.len() as u32,
+                offsets.as_ptr(),
+            )
+        }
+    }
+
+    /// Get the drive setting for a given line.
+    ///
+    /// Returns drive setting that would have been used for given offset if the
+    /// config object was used in a request at the time of the call. If an
+    /// offset is used for which no config was provided, the function will
+    /// return the global default value.
+    pub fn get_drive(&self, offset: u32) -> Result<Drive> {
+        Drive::new(unsafe { bindings::gpiod_line_config_get_drive(self.config, offset) } as u32)
+    }
+
+    /// Set all lines as active-low.
+    pub fn set_active_low(&mut self) {
+        unsafe { bindings::gpiod_line_config_set_active_low(self.config) }
+    }
+
+    /// Set a single line as active-low.
+    pub fn set_active_low_offset(&mut self, offset: u32) {
+        unsafe { bindings::gpiod_line_config_set_active_low_offset(self.config, offset) }
+    }
+
+    /// Set a subset of lines as active-low.
+    pub fn set_active_low_subset(&mut self, offsets: &[u32]) {
+        unsafe {
+            bindings::gpiod_line_config_set_active_low_subset(
+                self.config,
+                offsets.len() as u32,
+                offsets.as_ptr(),
+            )
+        }
+    }
+
+    /// Check if the line at given offset was configured as active-low.
+    ///
+    /// Returns active-low setting that would have been used for given offset if
+    /// the config object was used in a request at the time of the call. If an
+    /// offset is used for which no config was provided, the function will
+    /// return the global default value.
+    pub fn is_active_low(&self, offset: u32) -> bool {
+        unsafe { bindings::gpiod_line_config_is_active_low(self.config, offset) }
+    }
+
+    /// Set all lines as active-high.
+    pub fn set_active_high(&mut self) {
+        unsafe { bindings::gpiod_line_config_set_active_high(self.config) }
+    }
+
+    /// Set a single line as active-high.
+    pub fn set_active_high_offset(&mut self, offset: u32) {
+        unsafe { bindings::gpiod_line_config_set_active_high_offset(self.config, offset) }
+    }
+
+    /// Set a subset of lines as active-high.
+    pub fn set_active_high_subset(&mut self, offsets: &[u32]) {
+        unsafe {
+            bindings::gpiod_line_config_set_active_high_subset(
+                self.config,
+                offsets.len() as u32,
+                offsets.as_ptr(),
+            )
+        }
+    }
+
+    /// Set the debounce period for all lines, disables debouncing if 0.
+    pub fn set_debounce_period_us(&mut self, period: Duration) {
+        unsafe {
+            bindings::gpiod_line_config_set_debounce_period_us(
+                self.config,
+                period.as_micros() as u64,
+            )
+        }
+    }
+
+    /// Set the debounce period for a single line at given offset, disables
+    /// debouncing if 0.
+    pub fn set_debounce_period_us_offset(&mut self, period: Duration, offset: u32) {
+        unsafe {
+            bindings::gpiod_line_config_set_debounce_period_us_offset(
+                self.config,
+                period.as_micros() as u64,
+                offset,
+            )
+        }
+    }
+
+    /// Set the debounce period for a subset of lines, disables debouncing if
+    /// `period` is 0.
+    pub fn set_debounce_period_us_subset(&mut self, period: Duration, offsets: &[u32]) {
+        unsafe {
+            bindings::gpiod_line_config_set_debounce_period_us_subset(
+                self.config,
+                period.as_micros() as u64,
+                offsets.len() as u32,
+                offsets.as_ptr(),
+            )
+        }
+    }
+
+    /// Get the debounce period for a given line.
+    ///
+    /// Returns debounce period that would have been used for given offset if
+    /// the config object was used in a request at the time of the call. If an
+    /// offset is used for which no config was provided, the function will
+    /// return the global default value.
+    pub fn get_debounce_period_us(&self, offset: u32) -> Result<Duration> {
+        Ok(Duration::from_micros(unsafe {
+            bindings::gpiod_line_config_get_debounce_us_period(self.config, offset)
+        }))
+    }
+
+    /// Set the event clock for all lines.
+    pub fn set_event_clock(&mut self, clock: EventClock) {
+        unsafe {
+            bindings::gpiod_line_config_set_event_clock(self.config, clock.gpiod_clock() as i32)
+        }
+    }
+
+    /// Set the event clock for a single line at given offset.
+    pub fn set_event_clock_offset(&mut self, clock: EventClock, offset: u32) {
+        unsafe {
+            bindings::gpiod_line_config_set_event_clock_offset(
+                self.config,
+                clock.gpiod_clock() as i32,
+                offset,
+            )
+        }
+    }
+
+    /// Set the event clock for a subset of lines.
+    pub fn set_event_clock_subset(&mut self, clock: EventClock, offsets: &[u32]) {
+        unsafe {
+            bindings::gpiod_line_config_set_event_clock_subset(
+                self.config,
+                clock.gpiod_clock() as i32,
+                offsets.len() as u32,
+                offsets.as_ptr(),
+            )
+        }
+    }
+
+    /// Set the output value for a single offset.
+    pub fn set_output_value(&mut self, offset: u32, value: u32) {
+        unsafe { bindings::gpiod_line_config_set_output_value(self.config, offset, value as i32) }
+    }
+
+    /// Set the output values for a set of offsets.
+    pub fn set_output_values(&mut self, offsets: &[u32], values: &[i32]) {
+        unsafe {
+            bindings::gpiod_line_config_set_output_values(
+                self.config,
+                values.len() as u32,
+                offsets.as_ptr(),
+                values.as_ptr(),
+            )
+        }
+    }
+
+    /// Get the number of lines for which the config object stores values.
+    pub fn num_output_values(&self) -> u32 {
+        unsafe { bindings::gpiod_line_config_num_output_values(self.config) }
+    }
+
+    /// Get the output value configured for a given line, 0 or 1.
+    pub fn get_output_value(&self, offset: u32) -> Result<u32> {
+        let value = unsafe { bindings::gpiod_line_config_get_output_value(self.config, offset) };
+
+        if value != 0 && value != 1 {
+            Err(Error::OperationFailed(
+                "GpiodLineConfig get-output-value",
+                IoError::last(),
+            ))
+        } else {
+            Ok(value as u32)
+        }
+    }
+
+    /// Get the output value mapping (offset, value) at given index.
+    ///
+    /// This function together with `get_num_output()` allows to iterate over
+    /// all output value mappings currently held by this object.
+    pub fn get_output_value_index(&self, index: u32) -> Result<(u32, u32)> {
+        let mut offset: u32 = 0;
+        let mut value: i32 = 0;
+
+        let ret = unsafe {
+            bindings::gpiod_line_config_get_output_value_index(
+                self.config,
+                index,
+                &mut offset,
+                &mut value,
+            )
+        };
+
+        if ret == -1 {
+            Err(Error::OperationFailed(
+                "GpiodLineConfig get-output-value-index",
+                IoError::last(),
+            ))
+        } else {
+            Ok((offset, !!(value as u32)))
+        }
+    }
+
+    /// Get all output value mappings stored in this config object.
+    ///
+    /// Each offset in the offsets vector corresponds to the value in the values
+    /// array at the same index.
+    pub fn get_output_values(&self) -> Result<(Vec<u32>, Vec<i32>)> {
+        let count = self.num_output_values() as usize;
+        let mut offset: Vec<u32> = vec![0; count];
+        let mut value: Vec<i32> = vec![0; count];
+
+        unsafe {
+            bindings::gpiod_line_config_get_output_values(
+                self.config,
+                offset.as_mut_ptr(),
+                value.as_mut_ptr(),
+            )
+        };
+
+        Ok((offset, value))
+    }
+}
+
+impl Drop for GpiodLineConfig {
+    /// Free the line config object and release all associated resources.
+    fn drop(&mut self) {
+        unsafe { bindings::gpiod_line_config_free(self.config) }
+    }
+}
diff --git a/bindings/rust/src/line_info.rs b/bindings/rust/src/line_info.rs
new file mode 100644
index 000000000000..df6dda5c6aa3
--- /dev/null
+++ b/bindings/rust/src/line_info.rs
@@ -0,0 +1,186 @@
+// SPDX-License-Identifier: Apache-2.0 AND BSD-3-Clause
+//
+// Copyright 2021 Linaro Ltd. All Rights Reserved.
+//     Viresh Kumar <viresh.kumar@linaro.org>
+
+use std::convert::TryFrom;
+use std::sync::Arc;
+use std::time::Duration;
+use std::{slice, str};
+
+use vmm_sys_util::errno::Error as IoError;
+
+use super::chip::GpiodChipInternal;
+use super::{bindings, Bias, Direction, Drive, Edge, Error, EventClock, Result};
+use crate::info_event::GpiodInfoEvent;
+
+/// Line info
+///
+/// Exposes functions for retrieving kernel information about both requested and
+/// free lines. Line info object contains an immutable snapshot of the line's
+/// state at the time when it was created.
+pub struct GpiodLineInfo {
+    info: *mut bindings::gpiod_line_info,
+    ichip: Option<Arc<GpiodChipInternal>>,
+    free: bool,
+}
+
+impl GpiodLineInfo {
+    /// Get the current snapshot of information about the line at given offset
+    /// and optionally start watching it for changes.
+    pub(crate) fn new(ichip: Arc<GpiodChipInternal>, offset: u32, watch: bool) -> Result<Self> {
+        let info = if watch {
+            unsafe { bindings::gpiod_chip_watch_line_info(ichip.chip(), offset) }
+        } else {
+            unsafe { bindings::gpiod_chip_get_line_info(ichip.chip(), offset) }
+        };
+
+        if info.is_null() {
+            return Err(Error::OperationFailed(
+                "GpiodLineInfo line-info",
+                IoError::last(),
+            ));
+        }
+
+        Ok(Self {
+            info,
+            ichip: if watch { Some(ichip) } else { None },
+            free: watch,
+        })
+    }
+
+    /// Stop watching the line
+    pub fn unwatch(&mut self) {
+        if let Some(ichip) = &self.ichip {
+            unsafe {
+                bindings::gpiod_chip_unwatch_line_info(ichip.chip(), self.get_offset());
+            }
+            self.ichip = None;
+        }
+    }
+
+    /// Get the offset of the line within the GPIO chip.
+    pub fn get_offset(&self) -> u32 {
+        unsafe { bindings::gpiod_line_info_get_offset(self.info) }
+    }
+
+    /// Get GPIO line's name.
+    pub fn get_name(&self) -> Result<&str> {
+        // SAFETY: The string returned by libgpiod is guaranteed to live as long
+        // as the `struct GpiodLineInfo`.
+        let name = unsafe { bindings::gpiod_line_info_get_name(self.info) };
+        if name.is_null() {
+            return Err(Error::NameNotFound("GPIO line's name"));
+        }
+
+        unsafe {
+            str::from_utf8(slice::from_raw_parts(
+                name as *const u8,
+                bindings::strlen(name) as usize,
+            ))
+            .map_err(Error::InvalidString)
+        }
+    }
+
+    /// 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 { bindings::gpiod_line_info_is_used(self.info) }
+    }
+
+    /// Get the GPIO line's consumer name.
+    pub fn get_consumer(&self) -> Result<&str> {
+        // SAFETY: The string returned by libgpiod is guaranteed to live as long
+        // as the `struct GpiodLineInfo`.
+        let name = unsafe { bindings::gpiod_line_info_get_consumer(self.info) };
+        if name.is_null() {
+            return Err(Error::NameNotFound("GPIO line's consumer name"));
+        }
+
+        unsafe {
+            str::from_utf8(slice::from_raw_parts(
+                name as *const u8,
+                bindings::strlen(name) as usize,
+            ))
+            .map_err(Error::InvalidString)
+        }
+    }
+
+    /// Get the GPIO line's direction.
+    pub fn get_direction(&self) -> Result<Direction> {
+        Direction::new(unsafe { bindings::gpiod_line_info_get_direction(self.info) } as u32)
+    }
+
+    /// Returns true if this line is "active-low", false otherwise.
+    pub fn is_active_low(&self) -> bool {
+        unsafe { bindings::gpiod_line_info_is_active_low(self.info) }
+    }
+
+    /// Get the GPIO line's bias setting.
+    pub fn get_bias(&self) -> Result<Bias> {
+        Bias::new(unsafe { bindings::gpiod_line_info_get_bias(self.info) } as u32)
+    }
+
+    /// Get the GPIO line's drive setting.
+    pub fn get_drive(&self) -> Result<Drive> {
+        Drive::new(unsafe { bindings::gpiod_line_info_get_drive(self.info) } as u32)
+    }
+
+    /// Get the current edge detection setting of this line.
+    pub fn get_edge_detection(&self) -> Result<Edge> {
+        Edge::new(unsafe { bindings::gpiod_line_info_get_edge_detection(self.info) } as u32)
+    }
+
+    /// Get the current event clock setting used for edge event timestamps.
+    pub fn get_event_clock(&self) -> Result<EventClock> {
+        EventClock::new(unsafe { bindings::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 { bindings::gpiod_line_info_is_debounced(self.info) }
+    }
+
+    /// Get the current debounce period.
+    pub fn get_debounce_period_us(&self) -> Duration {
+        Duration::from_micros(unsafe {
+            bindings::gpiod_line_info_get_debounce_period_us(self.info)
+        })
+    }
+}
+
+impl TryFrom<&GpiodInfoEvent> for GpiodLineInfo {
+    type Error = Error;
+
+    /// Get the Line info object associated with a event.
+    fn try_from(event: &GpiodInfoEvent) -> Result<Self> {
+        let info = unsafe { bindings::gpiod_info_event_get_line_info(event.event()) };
+        if info.is_null() {
+            return Err(Error::OperationFailed(
+                "GpiodLineInfo try-from",
+                IoError::last(),
+            ));
+        }
+
+        Ok(Self {
+            info,
+            ichip: None,
+            free: false,
+        })
+    }
+}
+
+impl Drop for GpiodLineInfo {
+    fn drop(&mut self) {
+        // We must not free the Line info object created from `struct GpiodInfoEvent` by calling
+        // libgpiod API.
+        if self.free {
+            self.unwatch();
+            unsafe { bindings::gpiod_line_info_free(self.info) }
+        }
+    }
+}
diff --git a/bindings/rust/src/line_request.rs b/bindings/rust/src/line_request.rs
new file mode 100644
index 000000000000..1b00fcd04bac
--- /dev/null
+++ b/bindings/rust/src/line_request.rs
@@ -0,0 +1,217 @@
+// SPDX-License-Identifier: Apache-2.0 AND BSD-3-Clause
+//
+// Copyright 2021 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 IoError;
+
+use super::{bindings, chip::GpiodChipInternal, Error, Result};
+use crate::event_buffer::GpiodEdgeEventBuffer;
+use crate::line_config::GpiodLineConfig;
+use crate::request_config::GpiodRequestConfig;
+
+/// Line request operations
+///
+/// Allows interaction with a set of requested lines.
+pub struct GpiodLineRequest {
+    request: *mut bindings::gpiod_line_request,
+}
+
+impl GpiodLineRequest {
+    /// Request a set of lines for exclusive usage.
+    pub(crate) fn new(
+        ichip: &Arc<GpiodChipInternal>,
+        rconfig: &GpiodRequestConfig,
+        lconfig: &GpiodLineConfig,
+    ) -> Result<Self> {
+        let request = unsafe {
+            bindings::gpiod_chip_request_lines(ichip.chip(), rconfig.config(), lconfig.config())
+        };
+
+        if request.is_null() {
+            return Err(Error::OperationFailed(
+                "GpiodLineRequest request-lines",
+                IoError::last(),
+            ));
+        }
+
+        Ok(Self { request })
+    }
+
+    /// Get the number of lines in this request.
+    pub fn get_num_lines(&self) -> u32 {
+        unsafe { bindings::gpiod_line_request_get_num_lines(self.request) }
+    }
+
+    /// Get the offsets of lines in this request.
+    pub fn get_offsets(&self) -> Vec<u32> {
+        let mut offsets = vec![0; self.get_num_lines() as usize];
+
+        unsafe { bindings::gpiod_line_request_get_offsets(self.request, offsets.as_mut_ptr()) };
+        offsets
+    }
+
+    /// Get values of all lines associated with this request.
+    pub fn get_values(&self, values: &mut Vec<i32>) -> Result<()> {
+        let ret =
+            unsafe { bindings::gpiod_line_request_get_values(self.request, values.as_mut_ptr()) };
+
+        if ret == -1 {
+            Err(Error::OperationFailed(
+                "GpiodLineRequest get-values",
+                IoError::last(),
+            ))
+        } else {
+            Ok(())
+        }
+    }
+
+    /// Get the value (0 or 1) of a single line associated with this request.
+    pub fn get_value(&self, offset: u32) -> Result<u32> {
+        let value = unsafe { bindings::gpiod_line_request_get_value(self.request, offset) };
+
+        if value != 0 && value != 1 {
+            Err(Error::OperationFailed(
+                "GpiodLineRequest get-value",
+                IoError::last(),
+            ))
+        } else {
+            Ok(value as u32)
+        }
+    }
+
+    /// Get values of a subset of lines associated with this request.
+    pub fn get_values_subset(&self, offsets: &[u32], values: &mut Vec<i32>) -> Result<()> {
+        let ret = unsafe {
+            bindings::gpiod_line_request_get_values_subset(
+                self.request,
+                offsets.len() as u32,
+                offsets.as_ptr(),
+                values.as_mut_ptr(),
+            )
+        };
+
+        if ret == -1 {
+            Err(Error::OperationFailed(
+                "GpiodLineRequest get-values-subset",
+                IoError::last(),
+            ))
+        } else {
+            Ok(())
+        }
+    }
+
+    /// Get values of all lines associated with this request.
+    pub fn set_values(&self, values: &[i32]) -> Result<()> {
+        let ret = unsafe { bindings::gpiod_line_request_set_values(self.request, values.as_ptr()) };
+
+        if ret == -1 {
+            Err(Error::OperationFailed(
+                "GpiodLineRequest set-values",
+                IoError::last(),
+            ))
+        } else {
+            Ok(())
+        }
+    }
+
+    /// Set the value of a single line associated with this request.
+    pub fn set_value(&self, offset: u32, value: i32) -> Result<()> {
+        let ret = unsafe { bindings::gpiod_line_request_set_value(self.request, offset, !!value) };
+
+        if ret == -1 {
+            Err(Error::OperationFailed(
+                "GpiodLineRequest set-value",
+                IoError::last(),
+            ))
+        } else {
+            Ok(())
+        }
+    }
+
+    /// Get values of a subset of lines associated with this request.
+    pub fn set_values_subset(&self, offsets: &[u32], values: &[i32]) -> Result<()> {
+        let ret = unsafe {
+            bindings::gpiod_line_request_set_values_subset(
+                self.request,
+                offsets.len() as u32,
+                offsets.as_ptr(),
+                values.as_ptr(),
+            )
+        };
+
+        if ret == -1 {
+            Err(Error::OperationFailed(
+                "GpiodLineRequest set-values-subset",
+                IoError::last(),
+            ))
+        } else {
+            Ok(())
+        }
+    }
+
+    /// Update the configuration of lines associated with this line request.
+    pub fn reconfigure_lines(&self, lconfig: &GpiodLineConfig) -> Result<()> {
+        let ret = unsafe {
+            bindings::gpiod_line_request_reconfigure_lines(self.request, lconfig.config())
+        };
+
+        if ret == -1 {
+            Err(Error::OperationFailed(
+                "GpiodLineRequest reconfigure-lines",
+                IoError::last(),
+            ))
+        } else {
+            Ok(())
+        }
+    }
+
+    /// Get the file descriptor associated with this line request.
+    pub fn get_fd(&self) -> u32 {
+        unsafe { bindings::gpiod_line_request_get_fd(self.request) as u32 }
+    }
+
+    /// Wait for edge events on any of the lines associated with this request.
+    pub fn edge_event_wait(&self, timeout: Duration) -> Result<()> {
+        let ret = unsafe {
+            bindings::gpiod_line_request_edge_event_wait(self.request, timeout.as_nanos() as u64)
+        };
+
+        match ret {
+            -1 => Err(Error::OperationFailed(
+                "GpiodLineRequest edge-event-wait",
+                IoError::last(),
+            )),
+            0 => Err(Error::OperationTimedOut),
+            _ => Ok(()),
+        }
+    }
+
+    /// Get a number of edge events from a line request.
+    ///
+    /// This function will block if no event was queued for this line.
+    pub fn edge_event_read(&self, buffer: &GpiodEdgeEventBuffer, max_events: u32) -> Result<u32> {
+        let ret = unsafe {
+            bindings::gpiod_line_request_edge_event_read(self.request, buffer.buffer(), max_events)
+        };
+
+        if ret == -1 {
+            Err(Error::OperationFailed(
+                "GpiodLineRequest edge-event-read",
+                IoError::last(),
+            ))
+        } else {
+            Ok(ret as u32)
+        }
+    }
+}
+
+impl Drop for GpiodLineRequest {
+    /// Release the requested lines and free all associated resources.
+    fn drop(&mut self) {
+        unsafe { bindings::gpiod_line_request_release(self.request) }
+    }
+}
diff --git a/bindings/rust/src/request_config.rs b/bindings/rust/src/request_config.rs
new file mode 100644
index 000000000000..eef7c08ff798
--- /dev/null
+++ b/bindings/rust/src/request_config.rs
@@ -0,0 +1,118 @@
+// SPDX-License-Identifier: Apache-2.0 AND BSD-3-Clause
+//
+// Copyright 2021 Linaro Ltd. All Rights Reserved.
+//     Viresh Kumar <viresh.kumar@linaro.org>
+
+use std::os::raw::c_char;
+use std::{slice, str};
+
+use vmm_sys_util::errno::Error as IoError;
+
+use super::{bindings, Error, Result};
+
+/// Request configuration objects
+///
+/// Request config object is used to pass a set of options to the kernel at the
+/// time of the line request. Similarly to the line-config - the mutators don't
+/// return error values. If the values are invalid, in general they are silently
+/// adjusted to acceptable ranges.
+pub struct GpiodRequestConfig {
+    config: *mut bindings::gpiod_request_config,
+}
+
+impl GpiodRequestConfig {
+    /// Create a new request config object.
+    pub fn new() -> Result<Self> {
+        let config = unsafe { bindings::gpiod_request_config_new() };
+        if config.is_null() {
+            return Err(Error::OperationFailed(
+                "GpiodRequestConfig new",
+                IoError::last(),
+            ));
+        }
+
+        Ok(Self { config })
+    }
+
+    /// Private helper, Returns gpiod_request_config
+    pub(crate) fn config(&self) -> *mut bindings::gpiod_request_config {
+        self.config
+    }
+
+    /// Set the consumer string.
+    ///
+    /// If the consumer string is too long, it will be truncated to the max
+    /// accepted length.
+    pub fn set_consumer(&self, consumer: &str) {
+        unsafe {
+            bindings::gpiod_request_config_set_consumer(
+                self.config,
+                consumer.as_ptr() as *const c_char,
+            )
+        }
+    }
+
+    /// Get the consumer string.
+    pub fn get_consumer(&self) -> Result<&str> {
+        // SAFETY: The string returned by libgpiod is guaranteed to live as long
+        // as the `struct GpiodRequestConfig`.
+        let consumer = unsafe { bindings::gpiod_request_config_get_consumer(self.config) };
+        if consumer.is_null() {
+            return Err(Error::OperationFailed(
+                "GpiodRequestConfig get-consumer",
+                IoError::last(),
+            ));
+        }
+
+        unsafe {
+            str::from_utf8(slice::from_raw_parts(
+                consumer as *const u8,
+                bindings::strlen(consumer) as usize,
+            ))
+            .map_err(Error::InvalidString)
+        }
+    }
+
+    /// Set line offsets for this request.
+    ///
+    /// If too many offsets were specified, the offsets above the limit accepted
+    /// by the kernel (64 lines) are silently dropped.
+    pub fn set_offsets(&self, offsets: &[u32]) {
+        unsafe {
+            bindings::gpiod_request_config_set_offsets(
+                self.config,
+                offsets.len() as u32,
+                offsets.as_ptr(),
+            )
+        }
+    }
+
+    /// Get the offsets of lines in this request config.
+    pub fn get_num_offsets(&self) -> Vec<u32> {
+        let num = unsafe { bindings::gpiod_request_config_get_num_offsets(self.config) };
+        let mut offsets = vec![0, num];
+
+        unsafe { bindings::gpiod_request_config_get_offsets(self.config, offsets.as_mut_ptr()) };
+        offsets
+    }
+
+    /// Set the size of the kernel event buffer.
+    ///
+    /// The kernel may adjust the value if it's too high. If set to 0, the
+    /// default value will be used.
+    pub fn set_event_buffer_size(&self, size: u32) {
+        unsafe { bindings::gpiod_request_config_set_event_buffer_size(self.config, size) }
+    }
+
+    /// Get the edge event buffer size from this request config.
+    pub fn get_event_buffer_size(&self) -> u32 {
+        unsafe { bindings::gpiod_request_config_get_event_buffer_size(self.config) }
+    }
+}
+
+impl Drop for GpiodRequestConfig {
+    /// Free the request config object and release all associated resources.
+    fn drop(&mut self) {
+        unsafe { bindings::gpiod_request_config_free(self.config) }
+    }
+}
-- 
2.31.1.272.g89b43f80a514

Re: [PATCH V2 2/4] libgpiod: Add rust wrappers

[Bartosz Golaszewski]

On Thu, Dec 2, 2021 at 12:23 PM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> Add rust wrappers around FFI bindings to provide a convenient interface
> for the users.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---

Hi Viresh, thanks for the hard work.

Obviously this is not something we can merge yet but it's a good base
to continue the work.

General comment on the naming convention:

The line from one of the examples: 'use libgpiod::chip::GpiodChip;'
looks like it has a lot of redundancy in it. How about calling the
crate gpiod and dropping the chip package entirely? Basically follow
what C++ and python bindings do by having `use gpiod::Chip` etc.?

[snip]

> +
> +impl GpiodChipInternal {
> +    /// Find a GPIO chip by path.
> +    pub(crate) fn open(path: &str) -> Result<Self> {
> +        let chip = unsafe { bindings::gpiod_chip_open(path.as_ptr() as *const c_char) };
> +        if chip.is_null() {
> +            return Err(Error::OperationFailed("GpiodChip open", IoError::last()));

One of my concerns last time was error handling. Does this now
translate the error codes from the underlying C code to some kind of
rust errors? Can you elaborate on how that works? I see it always
returns IoError. In my WIP C++ and Python code I try to translate the
errnos into some meaningful exceptions - for instance EINVAL ->
::std::invalid_argument etc. Can we have a similar thing here?

[snip]

> +
> +    /// Get the GPIO chip name as represented in the kernel.
> +    pub fn get_name(&self) -> Result<&str> {
> +        // SAFETY: The string returned by libgpiod is guaranteed to live as long
> +        // as the `struct GpiodChip`.
> +        let name = unsafe { bindings::gpiod_chip_get_name(self.ichip.chip()) };
> +        if name.is_null() {

This is not possible, the string can be empty at the very least but
never null. Same for label and path.

> +
> +        match ret {
> +            -1 => Err(Error::OperationFailed(
> +                "GpiodChip info-event-wait",
> +                IoError::last(),
> +            )),
> +            0 => Err(Error::OperationTimedOut),

Does it have to be an Error? It's pretty normal for an operation to
time out, no?

> +            _ => Ok(()),
> +        }
> +    }
> +
> +    /// Read a single line status change event from this chip. If no events are
> +    /// pending, this function will block.
> +    pub fn info_event_read(&self) -> Result<GpiodInfoEvent> {
> +        GpiodInfoEvent::new(&self.ichip.clone())

Would you mind explaining what the clone() function does here to ichip
and why it's needed.


> +
> +impl GpiodEdgeEvent {
> +    /// Get an event stored in the buffer.
> +    pub fn new(buffer: &GpiodEdgeEventBuffer, index: u64) -> Result<Self> {
> +        let event = unsafe { bindings::gpiod_edge_event_buffer_get_event(buffer.buffer(), index) };
> +        if event.is_null() {
> +            return Err(Error::OperationFailed(
> +                "GpiodEdgeEvent buffer-get-event",
> +                IoError::last(),
> +            ));
> +        }
> +
> +        Ok(Self { event })
> +    }
> +
> +    /// Get the event type.
> +    pub fn get_event_type(&self) -> Result<EdgeEvent> {
> +        EdgeEvent::new(unsafe { bindings::gpiod_edge_event_get_event_type(self.event) } as u32)
> +    }
> +
> +    /// Get the timestamp of the event.
> +    pub fn get_timestamp(&self) -> Duration {
> +        Duration::from_nanos(unsafe { bindings::gpiod_edge_event_get_timestamp(self.event) })
> +    }
> +
> +    /// Get the offset of the line on which the event was triggered.
> +    pub fn get_line_offset(&self) -> u32 {
> +        unsafe { bindings::gpiod_edge_event_get_line_offset(self.event) }
> +    }
> +
> +    /// Get the global sequence number of this event.
> +    ///
> +    /// Returns sequence number of the event relative to all lines in the
> +    /// associated line request.
> +    pub fn get_global_seqno(&self) -> u64 {
> +        unsafe { bindings::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 this line within the
> +    /// lifetime of the associated line request.
> +    pub fn get_line_seqno(&self) -> u64 {
> +        unsafe { bindings::gpiod_edge_event_get_line_seqno(self.event) }
> +    }
> +}

AFAICT this object will not survive the parent buffer. Take a look at
the current development version of the C++ bindings where I play with
copy constructors to ensure that.

[snip]

I'll address the config objects once we actually have the final
version in the C API.

Bart

Re: [PATCH V2 2/4] libgpiod: Add rust wrappers

[Kent Gibson]

On Thu, Dec 16, 2021 at 02:59:25PM +0100, Bartosz Golaszewski wrote:
> On Thu, Dec 2, 2021 at 12:23 PM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> >
> > Add rust wrappers around FFI bindings to provide a convenient interface
> > for the users.
> >
> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> > ---
> 
> Hi Viresh, thanks for the hard work.
> 
> Obviously this is not something we can merge yet but it's a good base
> to continue the work.
> 
> General comment on the naming convention:
> 
> The line from one of the examples: 'use libgpiod::chip::GpiodChip;'
> looks like it has a lot of redundancy in it. How about calling the
> crate gpiod and dropping the chip package entirely? Basically follow
> what C++ and python bindings do by having `use gpiod::Chip` etc.?
> 

I'm of the opinion that a rust implementation would be better targetting
the ioctls directly rather than libgpiod, as my Go library does and for
essentially the same reasons.
To test that theory, and as an exercise to learn rust, I've been writing
one, and so far I've been calling it gpiod :-|.

Since this crate targets libgpiod it would make sense for it to
remain named libgpiod.

I agree with the chip::Chip repitition point - found the same thing
myself.  OTOH chip::Info and line::Info etc work well, and in rust
you can always rename them as you import them into your namespace if it
bothers you.  So overall I'm still on the fence on this one.

> [snip]
> 
> > +
> > +impl GpiodChipInternal {
> > +    /// Find a GPIO chip by path.
> > +    pub(crate) fn open(path: &str) -> Result<Self> {
> > +        let chip = unsafe { bindings::gpiod_chip_open(path.as_ptr() as *const c_char) };
> > +        if chip.is_null() {
> > +            return Err(Error::OperationFailed("GpiodChip open", IoError::last()));
> 
> One of my concerns last time was error handling. Does this now
> translate the error codes from the underlying C code to some kind of
> rust errors? Can you elaborate on how that works? I see it always
> returns IoError. In my WIP C++ and Python code I try to translate the
> errnos into some meaningful exceptions - for instance EINVAL ->
> ::std::invalid_argument etc. Can we have a similar thing here?
> 
> [snip]
> 
> > +
> > +    /// Get the GPIO chip name as represented in the kernel.
> > +    pub fn get_name(&self) -> Result<&str> {
> > +        // SAFETY: The string returned by libgpiod is guaranteed to live as long
> > +        // as the `struct GpiodChip`.
> > +        let name = unsafe { bindings::gpiod_chip_get_name(self.ichip.chip()) };
> > +        if name.is_null() {
> 
> This is not possible, the string can be empty at the very least but
> never null. Same for label and path.
> 

On the subject of strings, are paths, names, labels, and consumers
guaranteed to be valid UTF-8?
If not then failing to convert them using UTF-8 is not an error (not
here but elsewhere in the patch).

Cheers,
Kent.

Re: [PATCH V2 2/4] libgpiod: Add rust wrappers

[Viresh Kumar]

On 17-12-21, 08:12, Kent Gibson wrote:
> I'm of the opinion that a rust implementation would be better targetting
> the ioctls directly rather than libgpiod, as my Go library does and for
> essentially the same reasons.

I remember asking Linus initially about this and I was suggested to use libgpiod
instead :)

> To test that theory, and as an exercise to learn rust, I've been writing
> one, and so far I've been calling it gpiod :-|.
> 
> Since this crate targets libgpiod it would make sense for it to
> remain named libgpiod.

I agree.

-- 
viresh

Re: [PATCH V2 2/4] libgpiod: Add rust wrappers

[Kent Gibson]

On Fri, Dec 17, 2021 at 10:41:33AM +0530, Viresh Kumar wrote:
> On 17-12-21, 08:12, Kent Gibson wrote:
> > I'm of the opinion that a rust implementation would be better targetting
> > the ioctls directly rather than libgpiod, as my Go library does and for
> > essentially the same reasons.
> 
> I remember asking Linus initially about this and I was suggested to use libgpiod
> instead :)
> 
> > To test that theory, and as an exercise to learn rust, I've been writing
> > one, and so far I've been calling it gpiod :-|.
> > 

In case you are interested, the first pass of my take on a Rust GPIO
library[1] has finally gotten to the point of being core feature complete,
and may be usable, although it is sorely lacking any integration tests.
And the documentation is still pretty light, or even non-existant for
github.
And the event interface is either polled or blocking - haven't gotten to
an async interface yet.

Anyway, there are three crates there:
 - gpiod-uapi provides a thin and safe wrapper around the ioctls and
   file reads, for both uAPI versions.
 - gpiod provides a more idiomatic abstraction, and hides the uAPI
   version being used - unless you need v2 specific features.
 - gpiodctl provides a binary that bundles all the gpio tools into one.

There are a couple of minimal example apps in the gpiod crate, in
addition to the example that gpiodctl provides.

Cheers,
Kent.

[1] https://github.com/warthog618/gpiod-rs

Re: [PATCH V2 2/4] libgpiod: Add rust wrappers

[Viresh Kumar]

On 06-01-22, 23:47, Kent Gibson wrote:
> In case you are interested, the first pass of my take on a Rust GPIO
> library[1] has finally gotten to the point of being core feature complete,
> and may be usable, although it is sorely lacking any integration tests.
> And the documentation is still pretty light, or even non-existant for
> github.
> And the event interface is either polled or blocking - haven't gotten to
> an async interface yet.
> 
> Anyway, there are three crates there:
>  - gpiod-uapi provides a thin and safe wrapper around the ioctls and
>    file reads, for both uAPI versions.
>  - gpiod provides a more idiomatic abstraction, and hides the uAPI
>    version being used - unless you need v2 specific features.
>  - gpiodctl provides a binary that bundles all the gpio tools into one.
> 
> There are a couple of minimal example apps in the gpiod crate, in
> addition to the example that gpiodctl provides.
> 
> Cheers,
> Kent.
> 
> [1] https://github.com/warthog618/gpiod-rs

Nice stuff, thanks Kent for sharing that.

I may end up using the libgpiod wrappers though, since I have already done the
development using it and it is going to be low maintenance work for me :)

But it is really nice to have raw ioctl interface too for the GPIO devices.

-- 
viresh

Re: [PATCH V2 2/4] libgpiod: Add rust wrappers

[Viresh Kumar]

On 16-12-21, 14:59, Bartosz Golaszewski wrote:
> On Thu, Dec 2, 2021 at 12:23 PM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> General comment on the naming convention:
> 
> The line from one of the examples: 'use libgpiod::chip::GpiodChip;'
> looks like it has a lot of redundancy in it. How about calling the
> crate gpiod and dropping the chip package entirely? Basically follow
> what C++ and python bindings do by having `use gpiod::Chip` etc.?

That would mean I move back all the code to lib.rs, which makes it contain too
much code. I am okay though if you want it that way.

> > +
> > +impl GpiodChipInternal {
> > +    /// Find a GPIO chip by path.
> > +    pub(crate) fn open(path: &str) -> Result<Self> {
> > +        let chip = unsafe { bindings::gpiod_chip_open(path.as_ptr() as *const c_char) };
> > +        if chip.is_null() {
> > +            return Err(Error::OperationFailed("GpiodChip open", IoError::last()));
> 
> One of my concerns last time was error handling. Does this now
> translate the error codes from the underlying C code to some kind of
> rust errors? Can you elaborate on how that works?

You can think of it like wrapping around or concatenation of errors in a way.
The above line will return this now:

Err(libgpiod::Error::OperationFailed("GpiodChip open", IoError::last())) and
will print like for EINVAL:

Operation GpiodChip open Failed: 22

The user crate can further wrap it around with their own error enums, but this
error information wouldn't be lost and won't be subject to interpretation as
well, as we return the real underlying value now. This is the right/best way to
do it I suppose in Rust, this is what I was suggested to do for the vhost-device
Rust crate I merged in rust-vmm project.

> I see it always returns IoError.

This returns the underling errno value.

> In my WIP C++ and Python code I try to translate the
> errnos into some meaningful exceptions - for instance EINVAL ->
> ::std::invalid_argument etc. Can we have a similar thing here?

I would avoid translating errors into some interpretation here and rather pass
on the real error number returned by kernel. That can be more useful in
debugging the failures later.

> > +    /// Get the GPIO chip name as represented in the kernel.
> > +    pub fn get_name(&self) -> Result<&str> {
> > +        // SAFETY: The string returned by libgpiod is guaranteed to live as long
> > +        // as the `struct GpiodChip`.
> > +        let name = unsafe { bindings::gpiod_chip_get_name(self.ichip.chip()) };
> > +        if name.is_null() {
> 
> This is not possible, the string can be empty at the very least but
> never null. Same for label and path.

Miguel, what's your take on stuff like this ? I am not sure if we should just
drop this check altogether.

> > +
> > +        match ret {
> > +            -1 => Err(Error::OperationFailed(
> > +                "GpiodChip info-event-wait",
> > +                IoError::last(),
> > +            )),
> > +            0 => Err(Error::OperationTimedOut),
> 
> Does it have to be an Error? It's pretty normal for an operation to
> time out, no?

Right, but it is still a timeout and should be represented like that. And it is
up for the caller to act based on it. For example in the gpiomon example, we
check if the returned value is OperationTimedOut or not, and we retry if it is.

> > +            _ => Ok(()),
> > +        }
> > +    }
> > +
> > +    /// Read a single line status change event from this chip. If no events are
> > +    /// pending, this function will block.
> > +    pub fn info_event_read(&self) -> Result<GpiodInfoEvent> {
> > +        GpiodInfoEvent::new(&self.ichip.clone())
> 
> Would you mind explaining what the clone() function does here to ichip
> and why it's needed.

Normally clone() is like copying of the objects, but in this case ichip is
declared as Arc (Atomic Reference Counting).

Invoking clone on Arc produces a new Arc instance, which points to the same
allocation on the heap as the source Arc, while increasing a reference count.

In rust, when the last reference of an object goes away, it is freed
automatically.

> > +impl GpiodEdgeEvent {
> > +    /// Get an event stored in the buffer.
> > +    pub fn new(buffer: &GpiodEdgeEventBuffer, index: u64) -> Result<Self> {
> > +        let event = unsafe { bindings::gpiod_edge_event_buffer_get_event(buffer.buffer(), index) };
> > +        if event.is_null() {
> > +            return Err(Error::OperationFailed(
> > +                "GpiodEdgeEvent buffer-get-event",
> > +                IoError::last(),
> > +            ));
> > +        }
> > +
> > +        Ok(Self { event })
> > +    }
> > +
> > +    /// Get the event type.
> > +    pub fn get_event_type(&self) -> Result<EdgeEvent> {
> > +        EdgeEvent::new(unsafe { bindings::gpiod_edge_event_get_event_type(self.event) } as u32)
> > +    }
> > +
> > +    /// Get the timestamp of the event.
> > +    pub fn get_timestamp(&self) -> Duration {
> > +        Duration::from_nanos(unsafe { bindings::gpiod_edge_event_get_timestamp(self.event) })
> > +    }
> > +
> > +    /// Get the offset of the line on which the event was triggered.
> > +    pub fn get_line_offset(&self) -> u32 {
> > +        unsafe { bindings::gpiod_edge_event_get_line_offset(self.event) }
> > +    }
> > +
> > +    /// Get the global sequence number of this event.
> > +    ///
> > +    /// Returns sequence number of the event relative to all lines in the
> > +    /// associated line request.
> > +    pub fn get_global_seqno(&self) -> u64 {
> > +        unsafe { bindings::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 this line within the
> > +    /// lifetime of the associated line request.
> > +    pub fn get_line_seqno(&self) -> u64 {
> > +        unsafe { bindings::gpiod_edge_event_get_line_seqno(self.event) }
> > +    }
> > +}
> 
> AFAICT this object will not survive the parent buffer. Take a look at
> the current development version of the C++ bindings where I play with
> copy constructors to ensure that.

Adding back some of the removed code as well.

+impl Drop for GpiodEdgeEvent {
+    /// Free the edge event object and release all associated resources.
+    fn drop(&mut self) {
+        unsafe { bindings::gpiod_edge_event_free(self.event) }
+    }
+}

While testing this I found a bug few days back and I wonder why
gpiod_edge_event_free() even exists. The memory for "events" is allocated with
the buffer and gpiod_edge_event_free() shouldn't try to free a part of that.
This looks buggy.

Yes I realize that the edge event shouldn't exist past the buffer itself, I will
try to fix it in a Rusty way (maybe with Arc or something else).

-- 
viresh

Re: [PATCH V2 2/4] libgpiod: Add rust wrappers

[Wedson Almeida Filho]

Some drive-by comments. I haven't reviewed the code in detail.

On Fri, 17 Dec 2021 at 05:01, Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> On 16-12-21, 14:59, Bartosz Golaszewski wrote:
> > On Thu, Dec 2, 2021 at 12:23 PM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> > General comment on the naming convention:
> >
> > The line from one of the examples: 'use libgpiod::chip::GpiodChip;'
> > looks like it has a lot of redundancy in it. How about calling the
> > crate gpiod and dropping the chip package entirely? Basically follow
> > what C++ and python bindings do by having `use gpiod::Chip` etc.?
>
> That would mean I move back all the code to lib.rs, which makes it contain too
> much code. I am okay though if you want it that way.

You can keep things in different source files/modules and then expose
them from lib.rs. For example, we have something like this in the
kernel crate:

mod types;
pub use crate::types::{bit, bits_iter, Mode, Opaque, ScopeGuard};

So these are implemented in `types.rs` but exposed directly from the
kernel crate as `kernel::bit`, `kernel::bits_iter`, etc.

> > > +
> > > +impl GpiodChipInternal {
> > > +    /// Find a GPIO chip by path.
> > > +    pub(crate) fn open(path: &str) -> Result<Self> {
> > > +        let chip = unsafe { bindings::gpiod_chip_open(path.as_ptr() as *const c_char) };

Note that Rust strings are not necessarily null-terminated, so if
`gpiod_chip_open` is expecting a null-terminated string, this isn't
always going to work. In the kernel we have null-terminated strings as
a CStr type, and they can be initialised with constants using the
`c_str!` macro. You'll probably need something similar here if you
want to avoid allocations (otherwise you can just allocate a
null-terminated copy of `path`, use it, then free it).

> > > +        if chip.is_null() {
> > > +            return Err(Error::OperationFailed("GpiodChip open", IoError::last()));
> >
> > One of my concerns last time was error handling. Does this now
> > translate the error codes from the underlying C code to some kind of
> > rust errors? Can you elaborate on how that works?
>
> You can think of it like wrapping around or concatenation of errors in a way.
> The above line will return this now:
>
> Err(libgpiod::Error::OperationFailed("GpiodChip open", IoError::last())) and
> will print like for EINVAL:
>
> Operation GpiodChip open Failed: 22

Seems ok for printing. Is there a way to get the actual error code (22
in the example above) from the error returned? (As opposed to just a
string.)

> The user crate can further wrap it around with their own error enums, but this
> error information wouldn't be lost and won't be subject to interpretation as
> well, as we return the real underlying value now. This is the right/best way to
> do it I suppose in Rust, this is what I was suggested to do for the vhost-device
> Rust crate I merged in rust-vmm project.
>
> > I see it always returns IoError.
>
> This returns the underling errno value.
>
> > In my WIP C++ and Python code I try to translate the
> > errnos into some meaningful exceptions - for instance EINVAL ->
> > ::std::invalid_argument etc. Can we have a similar thing here?
>
> I would avoid translating errors into some interpretation here and rather pass
> on the real error number returned by kernel. That can be more useful in
> debugging the failures later.
>
> > > +    /// Get the GPIO chip name as represented in the kernel.
> > > +    pub fn get_name(&self) -> Result<&str> {
> > > +        // SAFETY: The string returned by libgpiod is guaranteed to live as long
> > > +        // as the `struct GpiodChip`.
> > > +        let name = unsafe { bindings::gpiod_chip_get_name(self.ichip.chip()) };
> > > +        if name.is_null() {
> >
> > This is not possible, the string can be empty at the very least but
> > never null. Same for label and path.
>
> Miguel, what's your take on stuff like this ? I am not sure if we should just
> drop this check altogether.

I'm obviously not Miguel :) But here's my take on this:

I think it's better to keep the check even if it's not possible today.

But if you choose to remove it, then you probably want to note in the
next SAFETY comment for `from_raw_parts` (which I think you forgot to
add) that `name` is always non-null.

> > > +
> > > +        match ret {
> > > +            -1 => Err(Error::OperationFailed(
> > > +                "GpiodChip info-event-wait",
> > > +                IoError::last(),
> > > +            )),
> > > +            0 => Err(Error::OperationTimedOut),
> >
> > Does it have to be an Error? It's pretty normal for an operation to
> > time out, no?
>
> Right, but it is still a timeout and should be represented like that. And it is
> up for the caller to act based on it. For example in the gpiomon example, we
> check if the returned value is OperationTimedOut or not, and we retry if it is.
>
> > > +            _ => Ok(()),
> > > +        }
> > > +    }
> > > +
> > > +    /// Read a single line status change event from this chip. If no events are
> > > +    /// pending, this function will block.
> > > +    pub fn info_event_read(&self) -> Result<GpiodInfoEvent> {
> > > +        GpiodInfoEvent::new(&self.ichip.clone())
> >
> > Would you mind explaining what the clone() function does here to ichip
> > and why it's needed.
>
> Normally clone() is like copying of the objects, but in this case ichip is
> declared as Arc (Atomic Reference Counting).
>
> Invoking clone on Arc produces a new Arc instance, which points to the same
> allocation on the heap as the source Arc, while increasing a reference count.
>
> In rust, when the last reference of an object goes away, it is freed
> automatically.

Your description of `Arc` is accurate, but it doesn't look like the
code does what you intended. Note that you're passing the _address_ of
the cloned arc to `GpiodInfoEvent::new`, so what you're doing here is:
incrementing the refcount, calling GpiodInfoEvent::new, then
immediately decrementing the count. If you want to keep the extra
reference alive, you must transfer ownership of it (and hold on to it
for as long as you want it alive).

> > > +impl GpiodEdgeEvent {
> > > +    /// Get an event stored in the buffer.
> > > +    pub fn new(buffer: &GpiodEdgeEventBuffer, index: u64) -> Result<Self> {
> > > +        let event = unsafe { bindings::gpiod_edge_event_buffer_get_event(buffer.buffer(), index) };
> > > +        if event.is_null() {
> > > +            return Err(Error::OperationFailed(
> > > +                "GpiodEdgeEvent buffer-get-event",
> > > +                IoError::last(),
> > > +            ));
> > > +        }
> > > +
> > > +        Ok(Self { event })
> > > +    }
> > > +
> > > +    /// Get the event type.
> > > +    pub fn get_event_type(&self) -> Result<EdgeEvent> {
> > > +        EdgeEvent::new(unsafe { bindings::gpiod_edge_event_get_event_type(self.event) } as u32)
> > > +    }
> > > +
> > > +    /// Get the timestamp of the event.
> > > +    pub fn get_timestamp(&self) -> Duration {
> > > +        Duration::from_nanos(unsafe { bindings::gpiod_edge_event_get_timestamp(self.event) })
> > > +    }
> > > +
> > > +    /// Get the offset of the line on which the event was triggered.
> > > +    pub fn get_line_offset(&self) -> u32 {
> > > +        unsafe { bindings::gpiod_edge_event_get_line_offset(self.event) }
> > > +    }
> > > +
> > > +    /// Get the global sequence number of this event.
> > > +    ///
> > > +    /// Returns sequence number of the event relative to all lines in the
> > > +    /// associated line request.
> > > +    pub fn get_global_seqno(&self) -> u64 {
> > > +        unsafe { bindings::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 this line within the
> > > +    /// lifetime of the associated line request.
> > > +    pub fn get_line_seqno(&self) -> u64 {
> > > +        unsafe { bindings::gpiod_edge_event_get_line_seqno(self.event) }
> > > +    }
> > > +}
> >
> > AFAICT this object will not survive the parent buffer. Take a look at
> > the current development version of the C++ bindings where I play with
> > copy constructors to ensure that.
>
> Adding back some of the removed code as well.
>
> +impl Drop for GpiodEdgeEvent {
> +    /// Free the edge event object and release all associated resources.
> +    fn drop(&mut self) {
> +        unsafe { bindings::gpiod_edge_event_free(self.event) }
> +    }
> +}
>
> While testing this I found a bug few days back and I wonder why
> gpiod_edge_event_free() even exists. The memory for "events" is allocated with
> the buffer and gpiod_edge_event_free() shouldn't try to free a part of that.
> This looks buggy.
>
> Yes I realize that the edge event shouldn't exist past the buffer itself, I will
> try to fix it in a Rusty way (maybe with Arc or something else).
>
> --
> viresh

Re: [PATCH V2 2/4] libgpiod: Add rust wrappers

[Viresh Kumar]

On 17-12-21, 05:52, Wedson Almeida Filho wrote:
> Some drive-by comments. I haven't reviewed the code in detail.
> 
> On Fri, 17 Dec 2021 at 05:01, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> >
> > On 16-12-21, 14:59, Bartosz Golaszewski wrote:
> > > On Thu, Dec 2, 2021 at 12:23 PM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> > > General comment on the naming convention:
> > >
> > > The line from one of the examples: 'use libgpiod::chip::GpiodChip;'
> > > looks like it has a lot of redundancy in it. How about calling the
> > > crate gpiod and dropping the chip package entirely? Basically follow
> > > what C++ and python bindings do by having `use gpiod::Chip` etc.?
> >
> > That would mean I move back all the code to lib.rs, which makes it contain too
> > much code. I am okay though if you want it that way.
> 
> You can keep things in different source files/modules and then expose
> them from lib.rs. For example, we have something like this in the
> kernel crate:

Right I did that kind of thing somewhere else too. This should work better.

> mod types;
> pub use crate::types::{bit, bits_iter, Mode, Opaque, ScopeGuard};
> 
> So these are implemented in `types.rs` but exposed directly from the
> kernel crate as `kernel::bit`, `kernel::bits_iter`, etc.
> 
> > > > +
> > > > +impl GpiodChipInternal {
> > > > +    /// Find a GPIO chip by path.
> > > > +    pub(crate) fn open(path: &str) -> Result<Self> {
> > > > +        let chip = unsafe { bindings::gpiod_chip_open(path.as_ptr() as *const c_char) };
> 
> Note that Rust strings are not necessarily null-terminated, so if
> `gpiod_chip_open` is expecting a null-terminated string,

I guess it would, though my code works just fine right now and I am not sure
why, maybe by chance the next byte is zero or something :)

> this isn't
> always going to work. In the kernel we have null-terminated strings as
> a CStr type, and they can be initialised with constants using the
> `c_str!` macro. You'll probably need something similar here if you
> want to avoid allocations (otherwise you can just allocate a
> null-terminated copy of `path`, use it, then free it).

So I must do something like this then ?

    pub(crate) fn open(path: &str) -> Result<Self> {
        let chip = unsafe { bindings::gpiod_chip_open(CStr::from_ptr(path.as_ptr() as *const c_char)) };

> > > > +        if chip.is_null() {
> > > > +            return Err(Error::OperationFailed("GpiodChip open", IoError::last()));
> > >
> > > One of my concerns last time was error handling. Does this now
> > > translate the error codes from the underlying C code to some kind of
> > > rust errors? Can you elaborate on how that works?
> >
> > You can think of it like wrapping around or concatenation of errors in a way.
> > The above line will return this now:
> >
> > Err(libgpiod::Error::OperationFailed("GpiodChip open", IoError::last())) and
> > will print like for EINVAL:
> >
> > Operation GpiodChip open Failed: 22
> 
> Seems ok for printing. Is there a way to get the actual error code (22
> in the example above) from the error returned? (As opposed to just a
> string.)

The entire thing is returned to the caller,
i.e.  libgpiod::Error::OperationFailed("GpiodChip open", 22).

I think caller should be able to get the code out of it somehow, won't something
like this work ?

let errno = match error {
        libgpiod::Error::OperationFailed(msg, errno) => errno,
        _ => 0,
};

> > > > +    /// Get the GPIO chip name as represented in the kernel.
> > > > +    pub fn get_name(&self) -> Result<&str> {
> > > > +        // SAFETY: The string returned by libgpiod is guaranteed to live as long
> > > > +        // as the `struct GpiodChip`.
> > > > +        let name = unsafe { bindings::gpiod_chip_get_name(self.ichip.chip()) };
> > > > +        if name.is_null() {
> > >
> > > This is not possible, the string can be empty at the very least but
> > > never null. Same for label and path.
> >
> > Miguel, what's your take on stuff like this ? I am not sure if we should just
> > drop this check altogether.
> 
> I'm obviously not Miguel :) But here's my take on this:

You are always welcome to provide feedback, I am the newbie here :)

> I think it's better to keep the check even if it's not possible today.
>
> But if you choose to remove it, then you probably want to note in the
> next SAFETY comment for `from_raw_parts` (which I think you forgot to
> add) that `name` is always non-null.

Right.

> > > > +    /// Read a single line status change event from this chip. If no events are
> > > > +    /// pending, this function will block.
> > > > +    pub fn info_event_read(&self) -> Result<GpiodInfoEvent> {
> > > > +        GpiodInfoEvent::new(&self.ichip.clone())
> > >
> > > Would you mind explaining what the clone() function does here to ichip
> > > and why it's needed.
> >
> > Normally clone() is like copying of the objects, but in this case ichip is
> > declared as Arc (Atomic Reference Counting).
> >
> > Invoking clone on Arc produces a new Arc instance, which points to the same
> > allocation on the heap as the source Arc, while increasing a reference count.
> >
> > In rust, when the last reference of an object goes away, it is freed
> > automatically.
> 
> Your description of `Arc` is accurate, but it doesn't look like the
> code does what you intended. Note that you're passing the _address_ of
> the cloned arc to `GpiodInfoEvent::new`, so what you're doing here is:
> incrementing the refcount, calling GpiodInfoEvent::new, then
> immediately decrementing the count. If you want to keep the extra
> reference alive, you must transfer ownership of it (and hold on to it
> for as long as you want it alive).

Ahh, this instance contains a clone by mistake here. It was correctly done at
other places where I wanted the newly allocated structure to contain a reference
to underlying ichip. I have removed clone() from two places where I was passing
the reference instead.

-- 
viresh

Re: [PATCH V2 2/4] libgpiod: Add rust wrappers

[Viresh Kumar]

On 17-12-21, 10:31, Viresh Kumar wrote:
> On 16-12-21, 14:59, Bartosz Golaszewski wrote:
> > AFAICT this object will not survive the parent buffer. Take a look at
> > the current development version of the C++ bindings where I play with
> > copy constructors to ensure that.

I looked at bindings/cxx/line.cpp::make_line_event() now, is this what you were
referring to here ?

Based on what I have understood until now, I think the edge event structure can
be modified like this since it is only required to give us certain fixed values:

 pub struct EdgeEvent {
-    event: *mut bindings::gpiod_edge_event,
+    event_type: GpiodEdgeEvent,
+    timestamp: Duration,
+    line_offset: u32,
+    global_seqno: u64,
+    line_seqno: u64
 }

That way it won't depend on the struct EdgeEventBuffer anymore and can be freed
after the buffer is freed.

> Adding back some of the removed code as well.
> 
> +impl Drop for GpiodEdgeEvent {
> +    /// Free the edge event object and release all associated resources.
> +    fn drop(&mut self) {
> +        unsafe { bindings::gpiod_edge_event_free(self.event) }
> +    }
> +}
> 
> While testing this I found a bug few days back and I wonder why
> gpiod_edge_event_free() even exists. The memory for "events" is allocated with
> the buffer and gpiod_edge_event_free() shouldn't try to free a part of that.
> This looks buggy.
> 
> Yes I realize that the edge event shouldn't exist past the buffer itself, I will
> try to fix it in a Rusty way (maybe with Arc or something else).

-- 
viresh

Re: [PATCH V2 2/4] libgpiod: Add rust wrappers

[Bartosz Golaszewski]

On Fri, Dec 17, 2021 at 6:01 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>

[snip]

>
> +impl Drop for GpiodEdgeEvent {
> +    /// Free the edge event object and release all associated resources.
> +    fn drop(&mut self) {
> +        unsafe { bindings::gpiod_edge_event_free(self.event) }
> +    }
> +}
>
> While testing this I found a bug few days back and I wonder why
> gpiod_edge_event_free() even exists. The memory for "events" is allocated with
> the buffer and gpiod_edge_event_free() shouldn't try to free a part of that.
> This looks buggy.
>
> Yes I realize that the edge event shouldn't exist past the buffer itself, I will
> try to fix it in a Rusty way (maybe with Arc or something else).
>

No, it's a different story altogether. In C the buffer allocates
memory for events and when you "get" an event, you only have a pointer
to the memory space in the buffer that you must not free. But you can
"copy" an event with gpiod_edge_event_copy() which returns you a deep
copy of the event that will survive the parent and that must be freed
with gpiod_edge_event_free(). This is done so that by default we try
to limit the number of allocations (as there can be a lot of events)
unless the user decides to manually copy the event.

In C++ I used that mechanism together with the buffer's const
event_get() and event's copy assignment operator. "Getting" an event
returns a const reference to the event (still in buffer's memory) but
copying it triggers a deep copy. The memory management is of course
handled by the destructor.

This is not used in Python as speed is no longer a concern and we'd be
creating new python objects anyway. But in Rust, I think it makes
sense to reuse this mechanism.

Bart

Re: [PATCH V2 2/4] libgpiod: Add rust wrappers

[Viresh Kumar]

On 17-12-21, 10:12, Bartosz Golaszewski wrote:
> No, it's a different story altogether. In C the buffer allocates
> memory for events and when you "get" an event, you only have a pointer
> to the memory space in the buffer that you must not free. But you can
> "copy" an event with gpiod_edge_event_copy() which returns you a deep
> copy of the event that will survive the parent and that must be freed
> with gpiod_edge_event_free(). This is done so that by default we try
> to limit the number of allocations (as there can be a lot of events)
> unless the user decides to manually copy the event.
> 
> In C++ I used that mechanism together with the buffer's const
> event_get() and event's copy assignment operator. "Getting" an event
> returns a const reference to the event (still in buffer's memory) but
> copying it triggers a deep copy. The memory management is of course
> handled by the destructor.
> 
> This is not used in Python as speed is no longer a concern and we'd be
> creating new python objects anyway. But in Rust, I think it makes
> sense to reuse this mechanism.

Ahh, what about this then, it just caches all the values when the event is
requested ?

pub struct EdgeEvent {
    event_type: LineEdgeEvent,
    timestamp: Duration,
    line_offset: u32,
    global_seqno: u64,
    line_seqno: u64,
}

impl EdgeEvent {
    /// Get an event stored in the buffer.
    pub fn new(buffer: &EdgeEventBuffer, index: u64) -> Result<Self> {
        let event = unsafe { bindings::gpiod_edge_event_buffer_get_event(buffer.buffer(), index) };
        if event.is_null() {
            return Err(Error::OperationFailed(
                "Gpio EdgeEvent buffer-get-event",
                IoError::last(),
            ));
        }

        Ok(Self {
            event_type: LineEdgeEvent::new(unsafe {
                bindings::gpiod_edge_event_get_event_type(event)
            } as u32)?,

            timestamp: Duration::from_nanos(unsafe {
                bindings::gpiod_edge_event_get_timestamp(event)
            }),

            line_offset: unsafe { bindings::gpiod_edge_event_get_line_offset(event) },
            global_seqno: unsafe { bindings::gpiod_edge_event_get_global_seqno(event) },
            line_seqno: unsafe { bindings::gpiod_edge_event_get_line_seqno(event) },
        })
    }

    /// Get the event type.
    pub fn get_event_type(&self) -> LineEdgeEvent {
        self.event_type
    }

    /// Get the timestamp of the event.
    pub fn get_timestamp(&self) -> Duration {
        self.timestamp
    }

    /// Get the offset of the line on which the event was triggered.
    pub fn get_line_offset(&self) -> u32 {
        self.line_offset
    }

    /// Get the global sequence number of this event.
    ///
    /// Returns sequence number of the event relative to all lines in the
    /// associated line request.
    pub fn get_global_seqno(&self) -> u64 {
        self.global_seqno
    }

    /// Get the event sequence number specific to concerned line.
    ///
    /// Returns sequence number of the event relative to this line within the
    /// lifetime of the associated line request.
    pub fn get_line_seqno(&self) -> u64 {
        self.line_seqno
    }
}

-- 
viresh

Re: [PATCH V2 2/4] libgpiod: Add rust wrappers

[Bartosz Golaszewski]

On Fri, Dec 17, 2021 at 10:32 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> On 17-12-21, 10:12, Bartosz Golaszewski wrote:
> > No, it's a different story altogether. In C the buffer allocates
> > memory for events and when you "get" an event, you only have a pointer
> > to the memory space in the buffer that you must not free. But you can
> > "copy" an event with gpiod_edge_event_copy() which returns you a deep
> > copy of the event that will survive the parent and that must be freed
> > with gpiod_edge_event_free(). This is done so that by default we try
> > to limit the number of allocations (as there can be a lot of events)
> > unless the user decides to manually copy the event.
> >
> > In C++ I used that mechanism together with the buffer's const
> > event_get() and event's copy assignment operator. "Getting" an event
> > returns a const reference to the event (still in buffer's memory) but
> > copying it triggers a deep copy. The memory management is of course
> > handled by the destructor.
> >
> > This is not used in Python as speed is no longer a concern and we'd be
> > creating new python objects anyway. But in Rust, I think it makes
> > sense to reuse this mechanism.
>
> Ahh, what about this then, it just caches all the values when the event is
> requested ?
>
> pub struct EdgeEvent {
>     event_type: LineEdgeEvent,
>     timestamp: Duration,
>     line_offset: u32,
>     global_seqno: u64,
>     line_seqno: u64,
> }
>

Then does it make sense to make it a binding to libgpiod? :)

I don't know Rust that well to be able to come up with an idea off the
top of my head but I assume inheritance in one way or another exists
in rust? Can you have an EdgeEvent interface implemented by two
specialized structs, one that's stored in the buffer (stores a pointer
to the internal event) and one that stores the copy (and frees it when
it goes out of scope)?

Bart

Re: [PATCH V2 2/4] libgpiod: Add rust wrappers

[Viresh Kumar]

On 17-12-21, 10:43, Bartosz Golaszewski wrote:
> On Fri, Dec 17, 2021 at 10:32 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> >
> > On 17-12-21, 10:12, Bartosz Golaszewski wrote:
> > > No, it's a different story altogether. In C the buffer allocates
> > > memory for events and when you "get" an event, you only have a pointer
> > > to the memory space in the buffer that you must not free. But you can
> > > "copy" an event with gpiod_edge_event_copy() which returns you a deep
> > > copy of the event that will survive the parent and that must be freed
> > > with gpiod_edge_event_free(). This is done so that by default we try
> > > to limit the number of allocations (as there can be a lot of events)
> > > unless the user decides to manually copy the event.
> > >
> > > In C++ I used that mechanism together with the buffer's const
> > > event_get() and event's copy assignment operator. "Getting" an event
> > > returns a const reference to the event (still in buffer's memory) but
> > > copying it triggers a deep copy. The memory management is of course
> > > handled by the destructor.
> > >
> > > This is not used in Python as speed is no longer a concern and we'd be
> > > creating new python objects anyway. But in Rust, I think it makes
> > > sense to reuse this mechanism.
> >
> > Ahh, what about this then, it just caches all the values when the event is
> > requested ?
> >
> > pub struct EdgeEvent {
> >     event_type: LineEdgeEvent,
> >     timestamp: Duration,
> >     line_offset: u32,
> >     global_seqno: u64,
> >     line_seqno: u64,
> > }
> >
> 
> Then does it make sense to make it a binding to libgpiod? :)

Yes, because EdgeEvent::new() gets all these values from the internal C libgpiod
library itself. We are going to make the same copy of data, either at the C
library level, or here.

> I don't know Rust that well to be able to come up with an idea off the
> top of my head but I assume inheritance in one way or another exists
> in rust? Can you have an EdgeEvent interface implemented by two
> specialized structs, one that's stored in the buffer (stores a pointer
> to the internal event)

This is what my initial patch had, right? It isn't safe. There is no way for
libgpiod-rust to guarantee that the users don't access the EdgeEvent after the
buffer is freed and this needs to be made safer at libgpiod itself.

> and one that stores the copy (and frees it when
> it goes out of scope)?

I don't understand what we will get out of such a scheme? Is there any benefit
of getting these values via the pointer to the internal C event ?

Any user that wants to use the edge-event data, needs to call the edge-event
APIs at least once and this is what the new version does, just calls them from
new() itself.

-- 
viresh

Re: [PATCH V2 2/4] libgpiod: Add rust wrappers

[Bartosz Golaszewski]

On Fri, Dec 17, 2021 at 10:54 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> On 17-12-21, 10:43, Bartosz Golaszewski wrote:
> > On Fri, Dec 17, 2021 at 10:32 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> > >
> > > On 17-12-21, 10:12, Bartosz Golaszewski wrote:
> > > > No, it's a different story altogether. In C the buffer allocates
> > > > memory for events and when you "get" an event, you only have a pointer
> > > > to the memory space in the buffer that you must not free. But you can
> > > > "copy" an event with gpiod_edge_event_copy() which returns you a deep
> > > > copy of the event that will survive the parent and that must be freed
> > > > with gpiod_edge_event_free(). This is done so that by default we try
> > > > to limit the number of allocations (as there can be a lot of events)
> > > > unless the user decides to manually copy the event.
> > > >
> > > > In C++ I used that mechanism together with the buffer's const
> > > > event_get() and event's copy assignment operator. "Getting" an event
> > > > returns a const reference to the event (still in buffer's memory) but
> > > > copying it triggers a deep copy. The memory management is of course
> > > > handled by the destructor.
> > > >
> > > > This is not used in Python as speed is no longer a concern and we'd be
> > > > creating new python objects anyway. But in Rust, I think it makes
> > > > sense to reuse this mechanism.
> > >
> > > Ahh, what about this then, it just caches all the values when the event is
> > > requested ?
> > >
> > > pub struct EdgeEvent {
> > >     event_type: LineEdgeEvent,
> > >     timestamp: Duration,
> > >     line_offset: u32,
> > >     global_seqno: u64,
> > >     line_seqno: u64,
> > > }
> > >
> >
> > Then does it make sense to make it a binding to libgpiod? :)
>
> Yes, because EdgeEvent::new() gets all these values from the internal C libgpiod
> library itself. We are going to make the same copy of data, either at the C
> library level, or here.
>
> > I don't know Rust that well to be able to come up with an idea off the
> > top of my head but I assume inheritance in one way or another exists
> > in rust? Can you have an EdgeEvent interface implemented by two
> > specialized structs, one that's stored in the buffer (stores a pointer
> > to the internal event)
>
> This is what my initial patch had, right? It isn't safe. There is no way for
> libgpiod-rust to guarantee that the users don't access the EdgeEvent after the
> buffer is freed and this needs to be made safer at libgpiod itself.
>

I'm not sure it had the same thing. In libgpiod it's simple a matter
of definition. The docs state that a pointer retrieved using
gpiod_edge_event_buffer_get_event() must not be freed while one
obtained with gpiod_edge_event_copy() does have to be disposed of. In
C++ it's safe because either you hold a const reference to the object
stored in the event buffer or you have copied it and now it will
survive the parent meaning the user can access the event even after
the buffer was deleted.

> > and one that stores the copy (and frees it when
> > it goes out of scope)?
>
> I don't understand what we will get out of such a scheme? Is there any benefit
> of getting these values via the pointer to the internal C event ?
>

You avoid allocating memory for those objects. The memory exists in
the C buffer. In order to keep a similar level of performance you'd
need to replicate the behavior of the buffer in rust as well.

Bart

> Any user that wants to use the edge-event data, needs to call the edge-event
> APIs at least once and this is what the new version does, just calls them from
> new() itself.

Re: [PATCH V2 2/4] libgpiod: Add rust wrappers

[Viresh Kumar]

On 17-12-21, 13:02, Bartosz Golaszewski wrote:
> On Fri, Dec 17, 2021 at 10:54 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> > > I don't know Rust that well to be able to come up with an idea off the
> > > top of my head but I assume inheritance in one way or another exists
> > > in rust? Can you have an EdgeEvent interface implemented by two
> > > specialized structs, one that's stored in the buffer (stores a pointer
> > > to the internal event)
> >
> > This is what my initial patch had, right? It isn't safe. There is no way for
> > libgpiod-rust to guarantee that the users don't access the EdgeEvent after the
> > buffer is freed and this needs to be made safer at libgpiod itself.
> >
> 
> I'm not sure it had the same thing. In libgpiod it's simple a matter
> of definition. The docs state that a pointer retrieved using
> gpiod_edge_event_buffer_get_event() must not be freed while one
> obtained with gpiod_edge_event_copy() does have to be disposed of. In
> C++ it's safe because either you hold a const reference to the object
> stored in the event buffer or you have copied it and now it will
> survive the parent meaning the user can access the event even after
> the buffer was deleted.
> 
> > > and one that stores the copy (and frees it when
> > > it goes out of scope)?
> >
> > I don't understand what we will get out of such a scheme? Is there any benefit
> > of getting these values via the pointer to the internal C event ?
> >
> 
> You avoid allocating memory for those objects. The memory exists in
> the C buffer. In order to keep a similar level of performance you'd
> need to replicate the behavior of the buffer in rust as well.

I was aligned with all this, I just wanted to skip providing the interface where
we use the event without copying as that has Safety issues. I will try to
provide both the options now, lets see how it looks.

-- 
viresh

Re: [PATCH V2 2/4] libgpiod: Add rust wrappers

[Miguel Ojeda]

On Fri, Dec 17, 2021 at 6:01 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> Miguel, what's your take on stuff like this ? I am not sure if we should just
> drop this check altogether.

Given the C side documents that the pointer is valid (I think; since
others are documented as "or NULL" etc., but it is a bit implicit),
then avoiding the check is a possibility.

However, to answer this, one needs to understand the risk profile of
the project, the stability of the C API, the performance expectations
(e.g. is the call supposed to be in the hot path of clients?), etc.

When in doubt (or when there is no reason to not do the check), it is
usually better to err on the safe side, specially if you aren't close
to the maintainers of the API etc. But since you have access to
Linus/Bartosz here, it is best to discuss this with them :) And, after
all, you are already trusting the C side to some degree anyway, and it
would be good to make sure both sides understand and clearly document
the contracts of the functions so that no bindings break in the future
(Rust or otherwise).

Cheers,
Miguel

Re: [PATCH V2 2/4] libgpiod: Add rust wrappers

[Viresh Kumar]

On 17-12-21, 10:44, Miguel Ojeda wrote:
> On Fri, Dec 17, 2021 at 6:01 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> >
> > Miguel, what's your take on stuff like this ? I am not sure if we should just
> > drop this check altogether.
> 
> Given the C side documents that the pointer is valid (I think; since
> others are documented as "or NULL" etc., but it is a bit implicit),
> then avoiding the check is a possibility.
> 
> However, to answer this, one needs to understand the risk profile of
> the project, the stability of the C API, the performance expectations
> (e.g. is the call supposed to be in the hot path of clients?), etc.
> 
> When in doubt (or when there is no reason to not do the check), it is
> usually better to err on the safe side, specially if you aren't close
> to the maintainers of the API etc. But since you have access to
> Linus/Bartosz here, it is best to discuss this with them :) And, after
> all, you are already trusting the C side to some degree anyway, and it
> would be good to make sure both sides understand and clearly document
> the contracts of the functions so that no bindings break in the future
> (Rust or otherwise).

And the null-checks are dropped, with an update to SAFETY comment :)

-- 
viresh

Re: [PATCH V2 2/4] libgpiod: Add rust wrappers

[Miguel Ojeda]

On Thu, Dec 16, 2021 at 2:59 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> The line from one of the examples: 'use libgpiod::chip::GpiodChip;'
> looks like it has a lot of redundancy in it. How about calling the
> crate gpiod and dropping the chip package entirely? Basically follow
> what C++ and python bindings do by having `use gpiod::Chip` etc.?

I agree on avoiding redundancy in names where possible (in particular,
the `Gpiod` in `GpiodChip` does not buy much); however, note that
modules in Rust are used a bit differently from C++ namespaces, so I
think it would be a bad idea to drop the modules.

For instance, in Rust, one can be fairly liberal with `use`, while in
C++ one has to worry about ADL and avoid them in global scope in
headers. More importantly, Rust modules are used for encapsulation,
while in C++ namespaces not really (except for the anonymous case or
the """internal""" namespace hack in headers) and in Python privacy is
even weaker. In particular, in Rust one can give each item (including
free functions, variables, etc. not just types) inside a module
different privacy, and importantly, they get special access privileges
within their module.

So while in C++ is indeed fairly typical (though not universal, e.g.
`boost::asio::ip::tcp::socket` comes to mind) to have a flatter layout
with something like `library::Type` with privacy within the `Type` and
hiding things in different TUs; in Rust it is fairly common to have a
`library::module::Item` layout (e.g. `std::vec::Vec` from the standard
library), where one puts everything related to the "module" concept
inside that module.

In any case, one can also re-export things more conveniently for
users, so that is an option too.

> Does it have to be an Error? It's pretty normal for an operation to
> time out, no?

Hmm... what do you mean? Timing out is considered an error in many
APIs, even if it is a frequent occurrence. After all, the request of
"doing something in X time" failed (even if the underlying operation
may even succeed).

Note that errors in Rust are normal control flow, i.e. they do not
involve exceptions (if that is the concern).

Cheers,
Miguel

Re: [PATCH V2 2/4] libgpiod: Add rust wrappers

[Viresh Kumar]

On 17-12-21, 10:21, Miguel Ojeda wrote:
> In any case, one can also re-export things more conveniently for
> users, so that is an option too.

This is what I have done now, and this is how users access them:

use libgpiod::{Chip, Edge, EdgeEventBuffer, Error, LineConfig, LineEdgeEvent, RequestConfig};

-- 
viresh

[PATCH V2 3/4] rust: Add examples

[Viresh Kumar]

Add examples for the usage of the rust bindings, quite similar to the
ones in cxx bindings.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 bindings/rust/examples/gpiodetect.rs | 38 ++++++++++++
 bindings/rust/examples/gpiofind.rs   | 43 +++++++++++++
 bindings/rust/examples/gpioget.rs    | 45 ++++++++++++++
 bindings/rust/examples/gpioinfo.rs   | 90 ++++++++++++++++++++++++++++
 bindings/rust/examples/gpiomon.rs    | 73 ++++++++++++++++++++++
 bindings/rust/examples/gpioset.rs    | 55 +++++++++++++++++
 6 files changed, 344 insertions(+)
 create mode 100644 bindings/rust/examples/gpiodetect.rs
 create mode 100644 bindings/rust/examples/gpiofind.rs
 create mode 100644 bindings/rust/examples/gpioget.rs
 create mode 100644 bindings/rust/examples/gpioinfo.rs
 create mode 100644 bindings/rust/examples/gpiomon.rs
 create mode 100644 bindings/rust/examples/gpioset.rs

diff --git a/bindings/rust/examples/gpiodetect.rs b/bindings/rust/examples/gpiodetect.rs
new file mode 100644
index 000000000000..c4b2ef31469d
--- /dev/null
+++ b/bindings/rust/examples/gpiodetect.rs
@@ -0,0 +1,38 @@
+// SPDX-License-Identifier: Apache-2.0 AND BSD-3-Clause
+//
+// Copyright 2021 Linaro Ltd. All Rights Reserved.
+//     Viresh Kumar <viresh.kumar@linaro.org>
+//
+// Simplified Rust implementation of gpiodetect tool.
+
+use std::env;
+use std::fs;
+use std::path::Path;
+
+use libgpiod::chip::GpiodChip;
+use libgpiod::gpiod_is_gpiochip_device;
+
+fn main() {
+    let args: Vec<String> = env::args().collect();
+    if args.len() > 1 {
+        println!("Usage: {}", args[0]);
+        return;
+    }
+
+    for entry in fs::read_dir(Path::new("/dev")).unwrap() {
+        let pathbuf = entry.unwrap().path();
+        let path = pathbuf.to_str().unwrap();
+
+        if gpiod_is_gpiochip_device(path) {
+            let chip = GpiodChip::open(path).unwrap();
+            let ngpio = chip.get_num_lines();
+
+            println!(
+                "{} [{}] ({})",
+                chip.get_name().unwrap(),
+                chip.get_label().unwrap(),
+                ngpio
+            );
+        }
+    }
+}
diff --git a/bindings/rust/examples/gpiofind.rs b/bindings/rust/examples/gpiofind.rs
new file mode 100644
index 000000000000..5050f4a39b6b
--- /dev/null
+++ b/bindings/rust/examples/gpiofind.rs
@@ -0,0 +1,43 @@
+// SPDX-License-Identifier: Apache-2.0 AND BSD-3-Clause
+//
+// Copyright 2021 Linaro Ltd. All Rights Reserved.
+//     Viresh Kumar <viresh.kumar@linaro.org>
+//
+// Simplified Rust implementation of gpiofind tool.
+
+use std::env;
+use std::fs;
+use std::path::Path;
+
+use libgpiod::chip::GpiodChip;
+use libgpiod::gpiod_is_gpiochip_device;
+
+fn main() {
+    let args: Vec<String> = env::args().collect();
+    if args.len() != 2 {
+        println!("Usage: {} <line-name>", args[0]);
+        return;
+    }
+
+    for entry in fs::read_dir(Path::new("/dev")).unwrap() {
+        let pathbuf = entry.unwrap().path();
+        let path = pathbuf.to_str().unwrap();
+
+        if gpiod_is_gpiochip_device(path) {
+            let chip = GpiodChip::open(path).unwrap();
+
+            let offset = chip.find_line(&args[1]);
+            if offset.is_ok() {
+                println!(
+                    "Line {} found: Chip: {}, offset: {}",
+                    args[1],
+                    chip.get_name().unwrap(),
+                    offset.unwrap()
+                );
+                return;
+            }
+        }
+    }
+
+    println!("Failed to find line: {}", args[1]);
+}
diff --git a/bindings/rust/examples/gpioget.rs b/bindings/rust/examples/gpioget.rs
new file mode 100644
index 000000000000..47a51b7c56b9
--- /dev/null
+++ b/bindings/rust/examples/gpioget.rs
@@ -0,0 +1,45 @@
+// SPDX-License-Identifier: Apache-2.0 AND BSD-3-Clause
+//
+// Copyright 2021 Linaro Ltd. All Rights Reserved.
+//     Viresh Kumar <viresh.kumar@linaro.org>
+//
+// Simplified Rust implementation of gpioget tool.
+
+use std::env;
+
+use libgpiod::chip::GpiodChip;
+use libgpiod::line_config::GpiodLineConfig;
+use libgpiod::request_config::GpiodRequestConfig;
+use libgpiod::Direction;
+
+fn main() {
+    let args: Vec<String> = env::args().collect();
+    if args.len() < 3 {
+        println!("Usage: {} <chip> <line_offset0> ...", args[0]);
+        return;
+    }
+
+    let mut config = GpiodLineConfig::new().unwrap();
+    let mut offsets = Vec::<u32>::new();
+
+    for arg in &args[2..] {
+        let offset = arg.parse::<u32>().unwrap();
+
+        offsets.push(offset);
+        config.set_direction_offset(Direction::Input, offset);
+    }
+
+    let path = format!("/dev/gpiochip{}", args[1]);
+    let chip = GpiodChip::open(&path).unwrap();
+
+    let rconfig = GpiodRequestConfig::new().unwrap();
+    rconfig.set_consumer(&args[0]);
+    rconfig.set_offsets(&offsets);
+
+    let request = chip.request_lines(&rconfig, &config).unwrap();
+
+    let mut values: Vec<i32> = vec![0; offsets.len()];
+    request.get_values(&mut values).unwrap();
+
+    println!("{:?}", values);
+}
diff --git a/bindings/rust/examples/gpioinfo.rs b/bindings/rust/examples/gpioinfo.rs
new file mode 100644
index 000000000000..1b84df70e811
--- /dev/null
+++ b/bindings/rust/examples/gpioinfo.rs
@@ -0,0 +1,90 @@
+// SPDX-License-Identifier: Apache-2.0 AND BSD-3-Clause
+//
+// Copyright 2021 Linaro Ltd. All Rights Reserved.
+//     Viresh Kumar <viresh.kumar@linaro.org>
+//
+// Simplified Rust implementation of gpioinfo tool.
+
+use std::env;
+use std::fs;
+use std::path::Path;
+
+use libgpiod::chip::GpiodChip;
+use libgpiod::{gpiod_is_gpiochip_device, Direction};
+
+fn line_info(chip: &GpiodChip, offset: u32) {
+    let info = chip.line_info(offset).unwrap();
+    let off = info.get_offset();
+
+    let name = match info.get_name() {
+        Ok(name) => name,
+        _ => "unused",
+    };
+
+    let consumer = match info.get_consumer() {
+        Ok(name) => name,
+        _ => "unnamed",
+    };
+
+    let low = if info.is_active_low() {
+        "active-low"
+    } else {
+        "active-high"
+    };
+
+    let dir = match info.get_direction().unwrap() {
+        Direction::AsIs => "None",
+        Direction::Input => "Input",
+        Direction::Output => "Output",
+    };
+
+    println!(
+        "\tline {:>3}\
+              \t{:>10}\
+              \t{:>10}\
+              \t{:>6}\
+              \t{:>14}",
+        off, name, consumer, dir, low
+    );
+}
+
+fn chip_info(path: &str) {
+    if gpiod_is_gpiochip_device(path) {
+        let chip = GpiodChip::open(path).unwrap();
+        let ngpio = chip.get_num_lines();
+
+        println!("GPIO Chip name: {}", chip.get_name().unwrap());
+        println!("\tlabel: {}", chip.get_label().unwrap());
+        println!("\tpath: {}", chip.get_path().unwrap());
+        println!("\tngpio: {}\n", ngpio);
+
+        println!("\tLine information:");
+
+        for offset in 0..ngpio {
+            line_info(&chip, offset);
+        }
+        println!("\n");
+    }
+}
+
+fn main() {
+    let args: Vec<String> = env::args().collect();
+    if args.len() > 2 {
+        println!("Usage: {}", args[0]);
+        return;
+    }
+
+    if args.len() == 1 {
+        for entry in fs::read_dir(Path::new("/dev")).unwrap() {
+            let pathbuf = entry.unwrap().path();
+            let path = pathbuf.to_str().unwrap();
+
+            chip_info(path);
+        }
+    } else {
+        let index = args[1].parse::<u32>().unwrap();
+        let path = format!("/dev/gpiochip{}", index);
+
+        chip_info(&path);
+    }
+}
diff --git a/bindings/rust/examples/gpiomon.rs b/bindings/rust/examples/gpiomon.rs
new file mode 100644
index 000000000000..7e44ec0da357
--- /dev/null
+++ b/bindings/rust/examples/gpiomon.rs
@@ -0,0 +1,73 @@
+// SPDX-License-Identifier: Apache-2.0 AND BSD-3-Clause
+//
+// Copyright 2021 Linaro Ltd. All Rights Reserved.
+//     Viresh Kumar <viresh.kumar@linaro.org>
+//
+// Simplified Rust implementation of the gpiomon tool.
+
+use std::env;
+use std::time::Duration;
+
+use libgpiod::chip::GpiodChip;
+use libgpiod::event_buffer::GpiodEdgeEventBuffer;
+use libgpiod::line_config::GpiodLineConfig;
+use libgpiod::request_config::GpiodRequestConfig;
+use libgpiod::{Edge, EdgeEvent, Error};
+
+fn usage(name: &str) {
+    println!("Usage: {} <chip> <offset0> ...", name);
+}
+
+fn main() {
+    let args: Vec<String> = env::args().collect();
+    if args.len() < 3 {
+        usage(&args[0]);
+        return;
+    }
+
+    let mut config = GpiodLineConfig::new().unwrap();
+    let mut offsets = Vec::<u32>::new();
+
+    for arg in &args[2..] {
+        let offset = arg.parse::<u32>().unwrap();
+
+        offsets.push(offset);
+    }
+
+    config.set_edge_detection(Edge::Both);
+
+    let path = format!("/dev/gpiochip{}", args[1]);
+    let chip = GpiodChip::open(&path).unwrap();
+
+    let rconfig = GpiodRequestConfig::new().unwrap();
+    rconfig.set_consumer(&args[0]);
+    rconfig.set_offsets(&offsets);
+
+    let buffer = GpiodEdgeEventBuffer::new(64).unwrap();
+    let request = chip.request_lines(&rconfig, &config).unwrap();
+
+    loop {
+        match request.edge_event_wait(Duration::new(1, 0)) {
+            Err(Error::OperationTimedOut) => continue,
+            Err(x) => {
+                println!("{:?}", x);
+                return;
+            }
+            Ok(()) => (),
+        }
+
+        let count = request.edge_event_read(&buffer, 64).unwrap();
+        for i in 0..count {
+            let event = buffer.get_event(i as u64).unwrap();
+            println!(
+                "line: {} type: {}, time: {:?}",
+                event.get_line_offset(),
+                match event.get_event_type().unwrap() {
+                    EdgeEvent::Rising => "Rising",
+                    EdgeEvent::Falling => "Falling",
+                },
+                event.get_timestamp()
+            );
+        }
+    }
+}
diff --git a/bindings/rust/examples/gpioset.rs b/bindings/rust/examples/gpioset.rs
new file mode 100644
index 000000000000..956f2d754717
--- /dev/null
+++ b/bindings/rust/examples/gpioset.rs
@@ -0,0 +1,55 @@
+// SPDX-License-Identifier: Apache-2.0 AND BSD-3-Clause
+//
+// Copyright 2021 Linaro Ltd. All Rights Reserved.
+//     Viresh Kumar <viresh.kumar@linaro.org>
+//
+// Simplified Rust implementation of the gpioset tool.
+
+use std::env;
+
+use libgpiod::chip::GpiodChip;
+use libgpiod::line_config::GpiodLineConfig;
+use libgpiod::request_config::GpiodRequestConfig;
+use libgpiod::Direction;
+
+fn usage(name: &str) {
+    println!("Usage: {} <chip> <line_offset0>=<value0> ...", name);
+}
+
+fn main() {
+    let args: Vec<String> = env::args().collect();
+    if args.len() < 3 {
+        usage(&args[0]);
+        return;
+    }
+
+    let mut config = GpiodLineConfig::new().unwrap();
+    let mut offsets = Vec::<u32>::new();
+    let mut values = Vec::<i32>::new();
+
+    for arg in &args[2..] {
+        let pair: Vec<&str> = arg.split('=').collect();
+        if pair.len() != 2 {
+            usage(&args[0]);
+            return;
+        }
+
+        let offset = pair[0].parse::<u32>().unwrap();
+        let value = pair[1].parse::<u32>().unwrap();
+
+        offsets.push(offset);
+        values.push(value as i32);
+    }
+
+    config.set_direction(Direction::Output);
+    config.set_output_values(&offsets, &values);
+
+    let path = format!("/dev/gpiochip{}", args[1]);
+    let chip = GpiodChip::open(&path).unwrap();
+
+    let rconfig = GpiodRequestConfig::new().unwrap();
+    rconfig.set_consumer(&args[0]);
+    rconfig.set_offsets(&offsets);
+
+    chip.request_lines(&rconfig, &config).unwrap();
+}
-- 
2.31.1.272.g89b43f80a514

[PATCH V2 4/4] rust: Integrate building of rust bindings with make

[Viresh Kumar]

Let make build rust bindings as well.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 README                    |  8 +++++---
 TODO                      |  8 --------
 bindings/Makefile.am      |  6 ++++++
 bindings/rust/Makefile.am | 29 +++++++++++++++++++++++++++++
 configure.ac              | 16 ++++++++++++++++
 5 files changed, 56 insertions(+), 11 deletions(-)
 create mode 100644 bindings/rust/Makefile.am

diff --git a/README b/README
index 814a0f161fd2..68b5d69f9b66 100644
--- a/README
+++ b/README
@@ -119,9 +119,9 @@ TOOLS
 BINDINGS
 --------
 
-High-level, object-oriented bindings for C++ and python3 are provided. They
-can be enabled by passing --enable-bindings-cxx and --enable-bindings-python
-arguments respectively to configure.
+High-level, object-oriented bindings for C++, python3 and Rust are provided.
+They can be enabled by passing --enable-bindings-cxx, --enable-bindings-python
+and --enable-bindings-rust arguments respectively to configure.
 
 C++ bindings require C++11 support and autoconf-archive collection if building
 from git.
@@ -132,6 +132,8 @@ the PYTHON_CPPFLAGS and PYTHON_LIBS variables in order to point the build
 system to the correct locations. During native builds, the configure script
 can auto-detect the location of the development files.
 
+Rust bindings require cargo support.
+
 TESTING
 -------
 
diff --git a/TODO b/TODO
index 8bb4d8f3ad56..cf4fd7b4a962 100644
--- a/TODO
+++ b/TODO
@@ -28,14 +28,6 @@ and is partially functional.
 
 ----------
 
-* implement rust bindings
-
-With Rust gaining popularity as a low-level system's language and the
-possibility of it making its way into the linux kernel, it's probably time to
-provide Rust bindings to libgpiod as part of the project.
-
-----------
-
 * implement a simple daemon for controlling GPIOs in C together with a client
   program
 
diff --git a/bindings/Makefile.am b/bindings/Makefile.am
index 8f8c762f254f..004ae23dbc58 100644
--- a/bindings/Makefile.am
+++ b/bindings/Makefile.am
@@ -14,3 +14,9 @@ if WITH_BINDINGS_PYTHON
 SUBDIRS += python
 
 endif
+
+if WITH_BINDINGS_RUST
+
+SUBDIRS += rust
+
+endif
diff --git a/bindings/rust/Makefile.am b/bindings/rust/Makefile.am
new file mode 100644
index 000000000000..ac05692b966d
--- /dev/null
+++ b/bindings/rust/Makefile.am
@@ -0,0 +1,29 @@
+# SPDX-License-Identifier: GPL-2.0-or-later
+# SPDX-FileCopyrightText: 2021 Viresh Kumar <viresh.kumar@linaro.org>
+
+LIBGPIOD_SRC =					\
+	Cargo.toml				\
+	build.rs				\
+	src/bindings.rs				\
+	src/chip.rs				\
+	src/edge_event.rs			\
+	src/event_buffer.rs			\
+	src/info_event.rs			\
+	src/lib.rs				\
+	src/line_config.rs			\
+	src/line_info.rs			\
+	src/line_request.rs			\
+	src/request_config.rs			\
+	examples/gpiodetect.rs			\
+	examples/gpiofind.rs			\
+	examples/gpioget.rs			\
+	examples/gpioinfo.rs			\
+	examples/gpiomon.rs			\
+	examples/gpioset.rs
+
+
+all: $(LIBGPIOD_SRC)
+	cargo build --lib --examples
+
+clean:
+	cargo clean
diff --git a/configure.ac b/configure.ac
index ce6de99c1edd..e83eca2a5e14 100644
--- a/configure.ac
+++ b/configure.ac
@@ -199,6 +199,21 @@ then
 		[AC_SUBST(PYTHON_LIBS, [`$PYTHON-config --libs`])])
 fi
 
+AC_ARG_ENABLE([bindings-rust],
+	[AS_HELP_STRING([--enable-bindings-rust],[enable rust bindings [default=no]])],
+	[if test "x$enableval" = xyes; then with_bindings_rust=true; fi],
+	[with_bindings_rust=false])
+AM_CONDITIONAL([WITH_BINDINGS_RUST], [test "x$with_bindings_rust" = xtrue])
+
+if test "x$with_bindings_rust" = xtrue
+then
+	AC_CHECK_PROG([has_cargo], [cargo], [true], [false])
+	if test "x$has_cargo" = xfalse
+	then
+		AC_MSG_ERROR([cargo not found - needed for rust bindings])
+	fi
+fi
+
 AC_CHECK_PROG([has_doxygen], [doxygen], [true], [false])
 AM_CONDITIONAL([HAS_DOXYGEN], [test "x$has_doxygen" = xtrue])
 if test "x$has_doxygen" = xfalse
@@ -232,6 +247,7 @@ AC_CONFIG_FILES([Makefile
 		 bindings/python/Makefile
 		 bindings/python/examples/Makefile
 		 bindings/python/tests/Makefile
+		 bindings/rust/Makefile
 		 man/Makefile])
 
 AC_OUTPUT
-- 
2.31.1.272.g89b43f80a514

Re: [PATCH V2 0/4] libgpiod: Add Rust bindings

[Viresh Kumar]

On 02-12-21, 16:52, Viresh Kumar wrote:
> Hi Bartosz,
> 
> This patch adds rust bindings for libgpiod v2.0, this is already partially
> tested with the virtio rust backend I am developing, which uses these to talk to
> the host kernel.
> 
> This is based of the next/post-libgpiod-2.0 branch.
> 
> I will be adding testing infrastructure later on, once other bindings are
> converted to use gpiosim.

Hi Bartosz,

I can see a lots of patches related to V2 version making there way
into libgpiod, but it is a bit hard for me to follow them currently.

Just wanted to request you, if you can ping me once the API is stable
enough and most of the V2 changes you planned are merged, so I can
rebase my stuff over those and send for review.

Thanks.

-- 
viresh

Re: [PATCH V2 0/4] libgpiod: Add Rust bindings

[Bartosz Golaszewski]

On Mon, Apr 11, 2022 at 5:33 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> On 02-12-21, 16:52, Viresh Kumar wrote:
> > Hi Bartosz,
> >
> > This patch adds rust bindings for libgpiod v2.0, this is already partially
> > tested with the virtio rust backend I am developing, which uses these to talk to
> > the host kernel.
> >
> > This is based of the next/post-libgpiod-2.0 branch.
> >
> > I will be adding testing infrastructure later on, once other bindings are
> > converted to use gpiosim.
>
> Hi Bartosz,
>
> I can see a lots of patches related to V2 version making there way
> into libgpiod, but it is a bit hard for me to follow them currently.
>
> Just wanted to request you, if you can ping me once the API is stable
> enough and most of the V2 changes you planned are merged, so I can
> rebase my stuff over those and send for review.
>
> Thanks.
>
> --
> viresh

Hi Viresh!

Yes, I will! Just to give you a summary: the C API is mostly ready but
I can't guarantee it won't change more. The C++ API needs another
revision that should be posted today or tomorrow. The Python bindings
are still WIP but will be ready soon. Once those three are agreed
upon, we'll merge them into master as one (for bisectability), drop
the next branch and work from there on new features (including Rust
bindings). That's my plan at least.

Bart