Message ID | f86049275ed165a3bf6922962b3c7e02744e5ef0.1664189248.git.viresh.kumar@linaro.org |
---|---|
State | Superseded |
Headers | show |
Series | libgpiod: Add Rust bindings | expand |
On Mon, Sep 26, 2022 at 1:08 PM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > Add rust wrapper crate, around the libpiod-sys crate added earlier, to > provide a convenient interface for the users. > > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > --- Hey Viresh! Thanks for being patient with me. :) I obviously cannot give a comprehensive review of Rust code as you know it much better than I do but there's one thing that bothers me at first glance. Please see below. [snip] > diff --git a/bindings/rust/libgpiod/src/event_buffer.rs b/bindings/rust/libgpiod/src/event_buffer.rs > new file mode 100644 > index 000000000000..e272e7aa9e9d > --- /dev/null > +++ b/bindings/rust/libgpiod/src/event_buffer.rs > @@ -0,0 +1,90 @@ > +// SPDX-License-Identifier: Apache-2.0 AND BSD-3-Clause > +// > +// Copyright 2022 Linaro Ltd. All Rights Reserved. > +// Viresh Kumar <viresh.kumar@linaro.org> > + > +use std::os::raw::c_ulong; > +use std::sync::Arc; > + > +use vmm_sys_util::errno::Error as Errno; > + > +use super::{edge, gpiod, Error, OperationType, Result}; > + > +/// Line edge events buffer > +#[derive(Clone, Debug, Eq, PartialEq)] > +pub(crate) struct BufferInternal { > + buffer: *mut gpiod::gpiod_edge_event_buffer, > +} > + > +impl BufferInternal { > + /// Create a new edge event buffer. > + /// > + /// If capacity equals 0, it will be set to a default value of 64. If > + /// capacity is larger than 1024, it will be limited to 1024. > + pub fn new(capacity: usize) -> Result<Self> { > + let buffer = unsafe { gpiod::gpiod_edge_event_buffer_new(capacity as c_ulong) }; > + if buffer.is_null() { > + return Err(Error::OperationFailed( > + OperationType::EdgeEventBufferNew, > + Errno::last(), > + )); > + } > + > + Ok(Self { buffer }) > + } > + > + /// Private helper, Returns gpiod_edge_event_buffer > + pub(crate) fn buffer(&self) -> *mut gpiod::gpiod_edge_event_buffer { > + self.buffer > + } > +} > + > +impl Drop for BufferInternal { > + /// Free the edge event buffer and release all associated resources. > + fn drop(&mut self) { > + unsafe { gpiod::gpiod_edge_event_buffer_free(self.buffer) }; > + } > +} > + > +/// Line edge events buffer > +#[derive(Clone, Debug, Eq, PartialEq)] > +pub struct Buffer { > + ibuffer: Arc<BufferInternal>, > +} > + > +impl Buffer { > + /// Create a new edge event buffer. > + /// > + /// If capacity equals 0, it will be set to a default value of 64. If > + /// capacity is larger than 1024, it will be limited to 1024. > + pub fn new(capacity: usize) -> Result<Self> { > + Ok(Self { > + ibuffer: Arc::new(BufferInternal::new(capacity)?), > + }) > + } > + > + /// Private helper, Returns gpiod_edge_event_buffer > + pub(crate) fn buffer(&self) -> *mut gpiod::gpiod_edge_event_buffer { > + self.ibuffer.buffer() > + } > + > + /// Get the capacity of the event buffer. > + pub fn capacity(&self) -> usize { > + unsafe { gpiod::gpiod_edge_event_buffer_get_capacity(self.buffer()) as usize } > + } > + > + /// Read an event stored in the buffer. > + pub fn event(&self, index: u64) -> Result<edge::Event> { > + edge::Event::new(&self.ibuffer, index) > + } In Event's new() you call gpiod_edge_event_buffer_get_event() which returns a pointer to an event stored inside the buffer. There's also the event_clone() function that calls gpiod_edge_event_copy() but I don't see it called anywhere. Should users pay attention to the lifetime of the buffer storing the event? Because IMO if the buffer goes out of scope, the program will crash attempting to access the event. In C++ the events in the buffer can only be accessed as const edge_event& so it's clear we're only holding a reference to an object existing somewhere else. Internally, the object stored in the buffer doesn't copy the edge event, only holds a C pointer to the event structure in struct gpiod_edge_event_buffer. Only upon calling edge_event& edge_event::operator=(const edge_event& other) will we trigger a copy. This way doing `for (const auto& event: buffer)` allows us to iterate over events without doing any additional allocations. Can we reproduce that behavior in Rust? For instance, the above function could return a borrowed reference and then we could have some interface to trigger the copy? Maybe do an implicit copy like in C++? I don't know if that's possible though. Bartosz > + > + /// Get the number of events the buffer contains. > + pub fn len(&self) -> usize { > + unsafe { gpiod::gpiod_edge_event_buffer_get_num_events(self.buffer()) as usize } > + } > + > + /// Check if buffer is empty. > + pub fn is_empty(&self) -> bool { > + self.len() == 0 > + } > +} [snip]
Hi Bartosz, On Mon, 26 Sept 2022 at 18:59, Bartosz Golaszewski <brgl@bgdev.pl> wrote: > Thanks for being patient with me. :) I should thank you for being patient with me :) > > + /// Read an event stored in the buffer. > > + pub fn event(&self, index: u64) -> Result<edge::Event> { > > + edge::Event::new(&self.ibuffer, index) > > + } > > In Event's new() you call gpiod_edge_event_buffer_get_event() which > returns a pointer to an event stored inside the buffer. There's also > the event_clone() function that calls gpiod_edge_event_copy() but I > don't see it called anywhere. I thought that is required if the user is concerned that the buffer will be overwritten by a new event, hence make a copy. Is that understanding correct? I didn't use it here for that reason, but it can be useful to the user expecting a lot of events. > Should users pay attention to the > lifetime of the buffer storing the event? No. > Because IMO if the buffer > goes out of scope, the program will crash attempting to access the > event. This is where Rust's memory safety comes in. If you see the design of the 'structure Event', it saves a reference to the 'struct BufferInternal'. The drop() implementation of BufferInternal calls: gpiod_edge_event_buffer_free(), but it won't get called unless all the references to it have gone out of scope. So untill the time user is still using any of the events, the buffer won't get freed. So we will never have invalid reference issue here. > In C++ the events in the buffer can only be accessed as const > edge_event& so it's clear we're only holding a reference to an object > existing somewhere else. Internally, the object stored in the buffer > doesn't copy the edge event, only holds a C pointer to the event > structure in struct gpiod_edge_event_buffer. Only upon calling > edge_event& edge_event::operator=(const edge_event& other) will we > trigger a copy. > > This way doing `for (const auto& event: buffer)` allows us to iterate > over events without doing any additional allocations. > > Can we reproduce that behavior in Rust? For instance, the above > function could return a borrowed reference and then we could have some > interface to trigger the copy? Maybe do an implicit copy like in C++? > I don't know if that's possible though. So here in Rust, you clone() normally to make a copy, but the standard clone() declaration can't have an error returned and so I had to name it event_clone(). -- Viresh
On Mon, Sep 26, 2022 at 5:27 PM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > Hi Bartosz, > > On Mon, 26 Sept 2022 at 18:59, Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > > Thanks for being patient with me. :) > > I should thank you for being patient with me :) > > > > + /// Read an event stored in the buffer. > > > + pub fn event(&self, index: u64) -> Result<edge::Event> { > > > + edge::Event::new(&self.ibuffer, index) > > > + } > > > > In Event's new() you call gpiod_edge_event_buffer_get_event() which > > returns a pointer to an event stored inside the buffer. There's also > > the event_clone() function that calls gpiod_edge_event_copy() but I > > don't see it called anywhere. > > I thought that is required if the user is concerned that the buffer will > be overwritten by a new event, hence make a copy. Is that > understanding correct? I didn't use it here for that reason, but it can > be useful to the user expecting a lot of events. > Yes, that's one of the use-cases. Copy an event from a buffer before reading more into it. > > Should users pay attention to the > > lifetime of the buffer storing the event? > > No. > > > Because IMO if the buffer > > goes out of scope, the program will crash attempting to access the > > event. > > This is where Rust's memory safety comes in. If you see the design of > the 'structure Event', it saves a reference to the 'struct BufferInternal'. > The drop() implementation of BufferInternal calls: > gpiod_edge_event_buffer_free(), but it won't get called unless all the > references to it have gone out of scope. So untill the time user is still > using any of the events, the buffer won't get freed. So we will never > have invalid reference issue here. You've got to help me out here, just looking at the code doesn't make things clear for me. Am I right to understand that each Event holds a reference to the BufferInternal object and it will only be destroyed when the last event is dropped? If we read new events into the buffer, does that create a new BufferInternal? Is that efficient? Aren't we allocating memory for each new BufferInternal at every read? > > > In C++ the events in the buffer can only be accessed as const > > edge_event& so it's clear we're only holding a reference to an object > > existing somewhere else. Internally, the object stored in the buffer > > doesn't copy the edge event, only holds a C pointer to the event > > structure in struct gpiod_edge_event_buffer. Only upon calling > > edge_event& edge_event::operator=(const edge_event& other) will we > > trigger a copy. > > > > This way doing `for (const auto& event: buffer)` allows us to iterate > > over events without doing any additional allocations. > > > > Can we reproduce that behavior in Rust? For instance, the above > > function could return a borrowed reference and then we could have some > > interface to trigger the copy? Maybe do an implicit copy like in C++? > > I don't know if that's possible though. > > So here in Rust, you clone() normally to make a copy, but the > standard clone() declaration can't have an error returned and so I had > to name it event_clone(). > Bart
On Tue, 27 Sept 2022 at 18:48, Bartosz Golaszewski <brgl@bgdev.pl> wrote: > On Mon, Sep 26, 2022 at 5:27 PM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > This is where Rust's memory safety comes in. If you see the design of > > the 'structure Event', it saves a reference to the 'struct BufferInternal'. > > The drop() implementation of BufferInternal calls: > > gpiod_edge_event_buffer_free(), but it won't get called unless all the > > references to it have gone out of scope. So untill the time user is still > > using any of the events, the buffer won't get freed. So we will never > > have invalid reference issue here. > > You've got to help me out here, just looking at the code doesn't make > things clear for me. Just to clarify, Buffer is just a wrapper over BufferInternal, you can think of them as the same entity for simplicity. > Am I right to understand that each Event holds a reference to the > BufferInternal object and it will only be destroyed when the last > event is dropped? Yes. But the buffer variable itself too holds a reference to the Buffer. Even if all the events are dropped, the buffer won't get free unless the buffer variable gets dropped too. > If we read new events into the buffer, does that > create a new BufferInternal? No. It uses the existing buffer only, that we created using Buffer::new(). > Is that efficient? Aren't we allocating > memory for each new BufferInternal at every read? No, we reuse the same buffer for all the reads. If you see the gpiomon.rs example, it shows how this is being done. The buffer (and internal buffer too) is created only once in line 42: let buffer = edge::event::Buffer::new(1)?; This allocates the memory for buffer only once, after that we continue reading from the same buffer. The BufferInternal structure is enclosed within an Arc<> instance, which is "Atomically Reference Counted", very much like kref_get/put in kernel. Unless the last reference is dropped, it stays. Hope I was able to clarify this here.
On Tue, Sep 27, 2022 at 3:57 PM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > On Tue, 27 Sept 2022 at 18:48, Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > On Mon, Sep 26, 2022 at 5:27 PM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > > > This is where Rust's memory safety comes in. If you see the design of > > > the 'structure Event', it saves a reference to the 'struct BufferInternal'. > > > The drop() implementation of BufferInternal calls: > > > gpiod_edge_event_buffer_free(), but it won't get called unless all the > > > references to it have gone out of scope. So untill the time user is still > > > using any of the events, the buffer won't get freed. So we will never > > > have invalid reference issue here. > > > > You've got to help me out here, just looking at the code doesn't make > > things clear for me. > > Just to clarify, Buffer is just a wrapper over BufferInternal, you can think > of them as the same entity for simplicity. > > > Am I right to understand that each Event holds a reference to the > > BufferInternal object and it will only be destroyed when the last > > event is dropped? > > Yes. But the buffer variable itself too holds a reference to the Buffer. > Even if all the events are dropped, the buffer won't get free unless the > buffer variable gets dropped too. > > > If we read new events into the buffer, does that > > create a new BufferInternal? > > No. It uses the existing buffer only, that we created using Buffer::new(). > So what happens if I do this: let buf = edge::event::Buffer::new(10)?; let request = chip.request_lines(&rconfig, &lconfig)?; let count = request.read_edge_events(&buffer)?; let event = buffer.event(0); let count = request.read_edge_events(&buffer)?; println!("line: {}", event.line_offset()); ? > > Is that efficient? Aren't we allocating > > memory for each new BufferInternal at every read? > > No, we reuse the same buffer for all the reads. > > If you see the gpiomon.rs example, it shows how this is being done. > The buffer (and internal buffer too) is created only once in line 42: > > let buffer = edge::event::Buffer::new(1)?; > > This allocates the memory for buffer only once, after that we continue > reading from the same buffer. > > The BufferInternal structure is enclosed within an Arc<> instance, which is > "Atomically Reference Counted", very much like kref_get/put in kernel. > > Unless the last reference is dropped, it stays. > > Hope I was able to clarify this here.
Here is what happens at each call for easier understanding: On 27-09-22, 17:25, Bartosz Golaszewski wrote: > So what happens if I do this: > > let buf = edge::event::Buffer::new(10)?; buf1 = gpiod_edge_event_buffer_new() > let request = chip.request_lines(&rconfig, &lconfig)?; gpiod_chip_request_lines() > let count = request.read_edge_events(&buffer)?; gpiod_line_request_read_edge_event(buf1) > let event = buffer.event(0); event1 = gpiod_edge_event_buffer_get_event(buf1, 0) > let count = request.read_edge_events(&buffer)?; gpiod_line_request_read_edge_event(buf1) > println!("line: {}", event.line_offset()); gpiod_edge_event_get_line_offset(event1) We will allocate a single buffer (buf1) and the event (event1) will be a reference onto its first event entry in the buffer. It will print offset value from the second read_edge_events() here instead of first, as it was just a reference to the first event. And for such a use case, the user should do event = buffer.event(0).event_clone();
On Wed, Sep 28, 2022 at 1:10 PM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > Here is what happens at each call for easier understanding: > > On 27-09-22, 17:25, Bartosz Golaszewski wrote: > > So what happens if I do this: > > > > let buf = edge::event::Buffer::new(10)?; > > buf1 = gpiod_edge_event_buffer_new() > > > let request = chip.request_lines(&rconfig, &lconfig)?; > > gpiod_chip_request_lines() > > > let count = request.read_edge_events(&buffer)?; > > gpiod_line_request_read_edge_event(buf1) > > > let event = buffer.event(0); > > event1 = gpiod_edge_event_buffer_get_event(buf1, 0) > > > let count = request.read_edge_events(&buffer)?; > > gpiod_line_request_read_edge_event(buf1) > > > println!("line: {}", event.line_offset()); > > gpiod_edge_event_get_line_offset(event1) > > > We will allocate a single buffer (buf1) and the event (event1) will be a > reference onto its first event entry in the buffer. It will print offset value > from the second read_edge_events() here instead of first, as it was just a > reference to the first event. And for such a use case, the user should do > > event = buffer.event(0).event_clone(); > > -- > viresh Ok, so this is not correct. The doc for gpiod_edge_event_buffer_get_event() says: * @return Pointer to an event stored in the buffer. The lifetime of the * event is tied to the buffer object. Users must not free the event * returned by this function. We make no guarantees that the address returned by gpiod_edge_event_buffer_get_event() will still be valid after a call to gpiod_line_request_read_edge_event(). In fact the doc for the latter says: * @note Any exising events in the buffer are overwritten. This is not an * append operation. So we're being clear that after a call to gpiod_line_request_read_edge_event(), all previously returned edge event pointers are invalid unless the objects to which they point were explicitly copied. Current implementation does indeed work like what you describe but that behavior is not "contractually" guaranteed and can change even between minor releases. In C++ if you do: const edge_event& ev = buffer.get_event(0); request.read_edge_event(buffer); std::cout << ev << std::endl; You risk a segfault. My understanding is that Rust should not allow a crash in this situation by design. Bart
On 28-09-22, 14:20, Bartosz Golaszewski wrote: > Ok, so this is not correct. The doc for > gpiod_edge_event_buffer_get_event() says: > > * @return Pointer to an event stored in the buffer. The lifetime of the > * event is tied to the buffer object. Users must not free the event > * returned by this function. > > We make no guarantees that the address returned by > gpiod_edge_event_buffer_get_event() will still be valid after a call > to gpiod_line_request_read_edge_event(). In fact the doc for the > latter says: > > * @note Any exising events in the buffer are overwritten. This is not an > * append operation. > > So we're being clear that after a call to > gpiod_line_request_read_edge_event(), all previously returned edge > event pointers are invalid unless the objects to which they point were > explicitly copied. > > Current implementation does indeed work like what you describe but > that behavior is not "contractually" guaranteed and can change even > between minor releases. > > In C++ if you do: > > const edge_event& ev = buffer.get_event(0); > request.read_edge_event(buffer); > std::cout << ev << std::endl; > > You risk a segfault. > > My understanding is that Rust should not allow a crash in this > situation by design. Hmm, so what exactly do we want to do here then ? - Don't allow events to be referenced ? i.e. make event_clone() the default behavior ? - Don't allow read_edge_event() to be called twice for a buffer ? that will be inefficient though. - Somehow guarantee that reference to all the events are dropped before issuing read_edge_event() again, else make it fail ? I am not sure how straight forward that can be though.
On Wed, Sep 28, 2022 at 5:17 PM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > On 28-09-22, 14:20, Bartosz Golaszewski wrote: > > Ok, so this is not correct. The doc for > > gpiod_edge_event_buffer_get_event() says: > > > > * @return Pointer to an event stored in the buffer. The lifetime of the > > * event is tied to the buffer object. Users must not free the event > > * returned by this function. > > > > We make no guarantees that the address returned by > > gpiod_edge_event_buffer_get_event() will still be valid after a call > > to gpiod_line_request_read_edge_event(). In fact the doc for the > > latter says: > > > > * @note Any exising events in the buffer are overwritten. This is not an > > * append operation. > > > > So we're being clear that after a call to > > gpiod_line_request_read_edge_event(), all previously returned edge > > event pointers are invalid unless the objects to which they point were > > explicitly copied. > > > > Current implementation does indeed work like what you describe but > > that behavior is not "contractually" guaranteed and can change even > > between minor releases. > > > > In C++ if you do: > > > > const edge_event& ev = buffer.get_event(0); > > request.read_edge_event(buffer); > > std::cout << ev << std::endl; > > > > You risk a segfault. > > > > My understanding is that Rust should not allow a crash in this > > situation by design. > > Hmm, so what exactly do we want to do here then ? > > - Don't allow events to be referenced ? i.e. make event_clone() the default > behavior ? > God no, that would be wasteful. > - Don't allow read_edge_event() to be called twice for a buffer ? that will be > inefficient though. > Not good either. > - Somehow guarantee that reference to all the events are dropped before issuing > read_edge_event() again, else make it fail ? I am not sure how straight > forward that can be though. In C++ the preferred way is to do buffer.get_event(0) which will return a constant reference. If you store that reference as const edge_event& ev = buffer.get_event(0) and reuse it after rereading into that buffer and the program crashes - that's on you. In most cases you should just do buffer.get_event(0).line_offset() etc. If you do: edge_event event = buffer.get_event(0); You'll copy the event and it will survive the overwriting of the buffer. I'm a Rust beginner but my understanding is that the whole idea of the language design is to *not* allow a situation where the program can crash. It should be detected at build-time. We must not rely on "contracts" defined by documentation. Is there a way to invalidate a reference in Rust? Have a small (cheap) object in the buffer which the event references and which would get dropped when reading into the buffer? Bart
On 28-09-22, 19:54, Bartosz Golaszewski wrote: > On Wed, Sep 28, 2022 at 5:17 PM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > Hmm, so what exactly do we want to do here then ? > > > > - Don't allow events to be referenced ? i.e. make event_clone() the default > > behavior ? > > > > God no, that would be wasteful. > > > - Don't allow read_edge_event() to be called twice for a buffer ? that will be > > inefficient though. > > > > Not good either. As I expected for both of them :) > > - Somehow guarantee that reference to all the events are dropped before issuing > > read_edge_event() again, else make it fail ? I am not sure how straight > > forward that can be though. > > In C++ the preferred way is to do buffer.get_event(0) which will > return a constant reference. If you store that reference as const > edge_event& ev = buffer.get_event(0) and reuse it after rereading into > that buffer and the program crashes - that's on you. In most cases you > should just do buffer.get_event(0).line_offset() etc. If you do: > > edge_event event = buffer.get_event(0); > > You'll copy the event and it will survive the overwriting of the buffer. Right, same happens here. > I'm a Rust beginner but my understanding is that the whole idea of the > language design is to *not* allow a situation where the program can > crash. It should be detected at build-time. We must not rely on > "contracts" defined by documentation. If everything was written in Rust, then this problem won't occur for sure. But in this case part of the code is available via FFI (foreign function interface) and they guarantees are a bit limited there and depend on what the FFI guarantees. > Is there a way to invalidate a reference in Rust? Have a small (cheap) > object in the buffer which the event references and which would get > dropped when reading into the buffer? I am not sure. There are locks, but then they have a cost. Miguel, any suggestions ? Bartosz, just as an FYI I am out on vacation until end of next week and won't have access to a workstation. I can still reply via Gmail (html) from my phone though.
On Thu, Sep 29, 2022 at 8:54 AM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > On 28-09-22, 19:54, Bartosz Golaszewski wrote: > > On Wed, Sep 28, 2022 at 5:17 PM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > > Hmm, so what exactly do we want to do here then ? > > > > > > - Don't allow events to be referenced ? i.e. make event_clone() the default > > > behavior ? > > > > > > > God no, that would be wasteful. > > > > > - Don't allow read_edge_event() to be called twice for a buffer ? that will be > > > inefficient though. > > > > > > > Not good either. > > As I expected for both of them :) > > > > - Somehow guarantee that reference to all the events are dropped before issuing > > > read_edge_event() again, else make it fail ? I am not sure how straight > > > forward that can be though. > > > > In C++ the preferred way is to do buffer.get_event(0) which will > > return a constant reference. If you store that reference as const > > edge_event& ev = buffer.get_event(0) and reuse it after rereading into > > that buffer and the program crashes - that's on you. In most cases you > > should just do buffer.get_event(0).line_offset() etc. If you do: > > > > edge_event event = buffer.get_event(0); > > > > You'll copy the event and it will survive the overwriting of the buffer. > > Right, same happens here. > > > I'm a Rust beginner but my understanding is that the whole idea of the > > language design is to *not* allow a situation where the program can > > crash. It should be detected at build-time. We must not rely on > > "contracts" defined by documentation. > > If everything was written in Rust, then this problem won't occur for sure. But > in this case part of the code is available via FFI (foreign function interface) > and they guarantees are a bit limited there and depend on what the FFI > guarantees. > > > Is there a way to invalidate a reference in Rust? Have a small (cheap) > > object in the buffer which the event references and which would get > > dropped when reading into the buffer? > > I am not sure. There are locks, but then they have a cost. > I'm not talking about locking, this should be left to the user of the module. Can we force-drop an object still referenced by other objects in Rust? This is what I had in mind - a small, dummy, cheap object inside the buffer that's created when reading into the buffer. Each even would reference it and then Rust would not allow us to drop it as long as there are references to it. Does it make sense? Is that possible? > Miguel, any suggestions ? > > Bartosz, just as an FYI I am out on vacation until end of next week and won't > have access to a workstation. I can still reply via Gmail (html) from my phone > though. > Nah, just take the time off and rest. BTW, I'm starting at linaro next week. :) Bart
On Thu, 29 Sept 2022 at 13:07, Bartosz Golaszewski <brgl@bgdev.pl> wrote: > I'm not talking about locking, this should be left to the user of the module. Yeah, I get that. > Can we force-drop an object still referenced by other objects in Rust? No, AFAICT. We can call drop(x) on an object, but as long as an reference to it is out there, we won't end up calling the actualy drop() routine for the object. > This is what I had in mind - a small, dummy, cheap object inside the > buffer that's created when reading into the buffer. Each even would > reference it and then Rust would not allow us to drop it as long as > there are references to it. Does it make sense? Is that possible? I don't think so. > Nah, just take the time off and rest. BTW, I'm starting at linaro next week. :) Excellent, welcome buddy :) -- Viresh
On Thu, Sep 29, 2022 at 10:58 AM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > On Thu, 29 Sept 2022 at 13:07, Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > I'm not talking about locking, this should be left to the user of the module. > > Yeah, I get that. > > > Can we force-drop an object still referenced by other objects in Rust? > > No, AFAICT. > > We can call drop(x) on an object, but as long as an reference to > it is out there, we won't end up calling the actualy drop() routine > for the object. > > > This is what I had in mind - a small, dummy, cheap object inside the > > buffer that's created when reading into the buffer. Each even would > > reference it and then Rust would not allow us to drop it as long as > > there are references to it. Does it make sense? Is that possible? > > I don't think so. Miguel: Could you give us advice on how to proceed with this? Bart > > > Nah, just take the time off and rest. BTW, I'm starting at linaro next week. :) > > Excellent, welcome buddy :) > > -- > Viresh
On Thu, Sep 29, 2022 at 09:37:40AM +0200, Bartosz Golaszewski wrote: > On Thu, Sep 29, 2022 at 8:54 AM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > > > On 28-09-22, 19:54, Bartosz Golaszewski wrote: > > > On Wed, Sep 28, 2022 at 5:17 PM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > > > Hmm, so what exactly do we want to do here then ? > > > > > > > > - Don't allow events to be referenced ? i.e. make event_clone() the default > > > > behavior ? > > > > > > > > > > God no, that would be wasteful. > > > > > > > - Don't allow read_edge_event() to be called twice for a buffer ? that will be > > > > inefficient though. > > > > > > > > > > Not good either. > > > > As I expected for both of them :) > > > > > > - Somehow guarantee that reference to all the events are dropped before issuing > > > > read_edge_event() again, else make it fail ? I am not sure how straight > > > > forward that can be though. > > > > > > In C++ the preferred way is to do buffer.get_event(0) which will > > > return a constant reference. If you store that reference as const > > > edge_event& ev = buffer.get_event(0) and reuse it after rereading into > > > that buffer and the program crashes - that's on you. In most cases you > > > should just do buffer.get_event(0).line_offset() etc. If you do: > > > > > > edge_event event = buffer.get_event(0); > > > > > > You'll copy the event and it will survive the overwriting of the buffer. > > > > Right, same happens here. > > > > > I'm a Rust beginner but my understanding is that the whole idea of the > > > language design is to *not* allow a situation where the program can > > > crash. It should be detected at build-time. We must not rely on > > > "contracts" defined by documentation. > > > > If everything was written in Rust, then this problem won't occur for sure. But > > in this case part of the code is available via FFI (foreign function interface) > > and they guarantees are a bit limited there and depend on what the FFI > > guarantees. > > > > > Is there a way to invalidate a reference in Rust? Have a small (cheap) > > > object in the buffer which the event references and which would get > > > dropped when reading into the buffer? > > > > I am not sure. There are locks, but then they have a cost. > > > > I'm not talking about locking, this should be left to the user of the module. > > Can we force-drop an object still referenced by other objects in Rust? > This is what I had in mind - a small, dummy, cheap object inside the > buffer that's created when reading into the buffer. Each even would > reference it and then Rust would not allow us to drop it as long as > there are references to it. Does it make sense? Is that possible? > No, Rust will explicitly prevent you from dropping referenced objects. But is this the sort of thing you mean: use std::process::ExitCode; #[derive(Clone)] struct Event { pub id: u8 } struct Events { b: Vec<Event> } impl Events { pub fn get(&self, idx: usize) -> Option<&Event> { self.b.get(idx) } } struct Buffer { count: u8, events: Option<Events>, } impl Buffer { pub fn read(&mut self) -> Result<&Events, ()> { self.count += 1; self.events = Some(Events{b: vec![Event{id: self.count}]}); self.events.as_ref().ok_or(()) } } fn main() -> Result<ExitCode, ()>{ let mut b = Buffer{count: 0, events:None}; let mut ee = b.read()?; let e = ee.get(0); println!("{:?}", e.unwrap().id); let cloned_e = e.unwrap().clone(); drop(e); // <-- comment out to try to create a dangling event reference ee = b.read()?; let e = ee.get(0); // <-- comment out to try to create a dangling event reference println!("{:?}", cloned_e.id); println!("{:?}", e.unwrap().id); Ok(ExitCode::from(42)) } That is a skeletal proof of concept - the small, dummy, cheap object is the Vec in Events. Is that cheap enough? You might be able to replace that with something cheaper, but Events needs to be able to pull an Event reference from somewhere (from the borrow checker's PoV) so it made this demo simpler. Comment out the two lines to try to carry e across the buffer read(). The compiler will not allow it, as e already borrows from b. In an actual implementation Event would wrap the C event, and Events.get() would get the event pointer for the Event and return that as a reference. The Event clone would call into C, rather than being derived as it is here. Key points: Buffer has to own the Events snapshot that is returned by reference by read(). The return by reference creates a borrow on the Buffer. The read() requires a &mut to prevent the Buffer being read while there are any borrows outstanding. The Events returns individual events by reference to create a borrow on the Events to prevent it (and the Buffer) being dropped or modified. The Event clone returns a concrete event that does not have a borrow of the Events or Buffer. There may well be better ways to do this, and you would really want to do some benchmarking to compare it with the immediate clone option - it may well be worse, but hopefully it at least demonstrates the semantics you are after. Cheers, Kent.
On Wed, Sep 28, 2022 at 7:54 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > I'm a Rust beginner but my understanding is that the whole idea of the > language design is to *not* allow a situation where the program can > crash. It should be detected at build-time. We must not rely on More precisely, it needs to avoid UB (which is defined similarly as in C++, and of course UB may lead to a crash if one is lucky). > Is there a way to invalidate a reference in Rust? Have a small (cheap) > object in the buffer which the event references and which would get > dropped when reading into the buffer?
On 29-09-22, 15:55, Miguel Ojeda wrote: > It looks like a container whose elements get invalidated, so > `read_edge_event` could require an exclusive reference to `buffer` in > Rust, that way you cannot keep borrows to its elements like `ev` if > you want to call it. But of course this requires tying the lifetime of > the events to that of the buffer. What about the below code changes on top of V6 ? Changes: - Removed BufferInternal. - Event contains a reference to the Buffer now, with lifetime. - read_edge_event() expects a mutable reference to buffer, to make it exclusive, i.e. disallow any previous Event references to exist at compilation itself. diff --git a/bindings/rust/libgpiod/src/edge_event.rs b/bindings/rust/libgpiod/src/edge_event.rs index ce583916a2e3..f5426459d779 100644 --- a/bindings/rust/libgpiod/src/edge_event.rs +++ b/bindings/rust/libgpiod/src/edge_event.rs @@ -3,12 +3,11 @@ // Copyright 2022 Linaro Ltd. All Rights Reserved. // Viresh Kumar <viresh.kumar@linaro.org> -use std::sync::Arc; use std::time::Duration; use vmm_sys_util::errno::Error as Errno; -use super::{edge::event::BufferInternal, gpiod, EdgeKind, Error, Offset, OperationType, Result}; +use super::{edge::event::Buffer, gpiod, EdgeKind, Error, Offset, OperationType, Result}; /// Line edge events handling /// @@ -22,15 +21,15 @@ use super::{edge::event::BufferInternal, gpiod, EdgeKind, Error, Offset, Operati /// number of events are being read. #[derive(Debug, Eq, PartialEq)] -pub struct Event { - ibuffer: Option<Arc<BufferInternal>>, +pub struct Event<'b> { + buffer: Option<&'b Buffer>, event: *mut gpiod::gpiod_edge_event, } -impl Event { +impl<'b> Event<'b> { /// Get an event stored in the buffer. - pub(crate) fn new(ibuffer: &Arc<BufferInternal>, index: u64) -> Result<Self> { - let event = unsafe { gpiod::gpiod_edge_event_buffer_get_event(ibuffer.buffer(), index) }; + pub(crate) fn new(buffer: &'b Buffer, index: u64) -> Result<Self> { + let event = unsafe { gpiod::gpiod_edge_event_buffer_get_event(buffer.buffer(), index) }; if event.is_null() { return Err(Error::OperationFailed( OperationType::EdgeEventBufferGetEvent, @@ -39,7 +38,7 @@ impl Event { } Ok(Self { - ibuffer: Some(ibuffer.clone()), + buffer: Some(buffer), event, }) } @@ -54,7 +53,7 @@ impl Event { } Ok(Self { - ibuffer: None, + buffer: None, event, }) } @@ -91,11 +90,11 @@ impl Event { } } -impl Drop for Event { +impl<'b> Drop for Event<'b> { /// Free the edge event. fn drop(&mut self) { // Free the event only if a copy is made - if self.ibuffer.is_none() { + if self.buffer.is_none() { unsafe { gpiod::gpiod_edge_event_free(self.event) }; } } diff --git a/bindings/rust/libgpiod/src/event_buffer.rs b/bindings/rust/libgpiod/src/event_buffer.rs index e272e7aa9e9d..11c8b5e1d7c9 100644 --- a/bindings/rust/libgpiod/src/event_buffer.rs +++ b/bindings/rust/libgpiod/src/event_buffer.rs @@ -4,7 +4,6 @@ // Viresh Kumar <viresh.kumar@linaro.org> use std::os::raw::c_ulong; -use std::sync::Arc; use vmm_sys_util::errno::Error as Errno; @@ -12,11 +11,11 @@ use super::{edge, gpiod, Error, OperationType, Result}; /// Line edge events buffer #[derive(Clone, Debug, Eq, PartialEq)] -pub(crate) struct BufferInternal { +pub struct Buffer { buffer: *mut gpiod::gpiod_edge_event_buffer, } -impl BufferInternal { +impl Buffer { /// Create a new edge event buffer. /// /// If capacity equals 0, it will be set to a default value of 64. If @@ -37,36 +36,6 @@ impl BufferInternal { pub(crate) fn buffer(&self) -> *mut gpiod::gpiod_edge_event_buffer { self.buffer } -} - -impl Drop for BufferInternal { - /// Free the edge event buffer and release all associated resources. - fn drop(&mut self) { - unsafe { gpiod::gpiod_edge_event_buffer_free(self.buffer) }; - } -} - -/// Line edge events buffer -#[derive(Clone, Debug, Eq, PartialEq)] -pub struct Buffer { - ibuffer: Arc<BufferInternal>, -} - -impl Buffer { - /// Create a new edge event buffer. - /// - /// If capacity equals 0, it will be set to a default value of 64. If - /// capacity is larger than 1024, it will be limited to 1024. - pub fn new(capacity: usize) -> Result<Self> { - Ok(Self { - ibuffer: Arc::new(BufferInternal::new(capacity)?), - }) - } - - /// Private helper, Returns gpiod_edge_event_buffer - pub(crate) fn buffer(&self) -> *mut gpiod::gpiod_edge_event_buffer { - self.ibuffer.buffer() - } /// Get the capacity of the event buffer. pub fn capacity(&self) -> usize { @@ -75,7 +44,7 @@ impl Buffer { /// Read an event stored in the buffer. pub fn event(&self, index: u64) -> Result<edge::Event> { - edge::Event::new(&self.ibuffer, index) + edge::Event::new(self, index) } /// Get the number of events the buffer contains. @@ -88,3 +57,10 @@ impl Buffer { self.len() == 0 } } + +impl Drop for Buffer { + /// Free the edge event buffer and release all associated resources. + fn drop(&mut self) { + unsafe { gpiod::gpiod_edge_event_buffer_free(self.buffer) }; + } +} diff --git a/bindings/rust/libgpiod/src/line_request.rs b/bindings/rust/libgpiod/src/line_request.rs index 617efaa34d58..e4ff951aef29 100644 --- a/bindings/rust/libgpiod/src/line_request.rs +++ b/bindings/rust/libgpiod/src/line_request.rs @@ -218,7 +218,7 @@ impl Request { /// Get a number of edge events from a line request. /// /// This function will block if no event was queued for the line. - pub fn read_edge_events(&self, buffer: &edge::event::Buffer) -> Result<u32> { + pub fn read_edge_events(&self, buffer: &mut edge::event::Buffer) -> Result<u32> { let ret = unsafe { gpiod::gpiod_line_request_read_edge_event( self.request, And here is an example where we get compilation error on the commented line: fn main() -> Result<()> { let mut lsettings = line::Settings::new()?; let lconfig = line::Config::new()?; let offsets = vec![1, 2]; lsettings.set_prop(&[SettingVal::EdgeDetection(Some(Edge::Both))])?; lconfig.add_line_settings(&offsets, lsettings)?; let path = "/dev/gpiochip0".to_string(); let chip = Chip::open(&path)?; let rconfig = request::Config::new()?; let mut buffer = edge::event::Buffer::new(2)?; let request = chip.request_lines(&rconfig, &lconfig)?; loop { match request.wait_edge_event(None) { Err(x) => { println!("{:?}", x); return Err(Error::InvalidArguments); } Ok(false) => { // This shouldn't happen as the call is blocking. panic!(); } Ok(true) => (), } request.read_edge_events(&mut buffer)?; let event0 = buffer.event(0)?; let event1 = buffer.event(1)?; println!("{:?}", (event0.line_offset(), event1.line_offset())); drop(event0); // This fails to compile // request.read_edge_events(&mut buffer)?; drop(event1); // This compiles fine request.read_edge_events(&mut buffer)?; } }
On Tue, Oct 11, 2022 at 09:46:51AM +0530, Viresh Kumar wrote: > On 29-09-22, 15:55, Miguel Ojeda wrote: > > It looks like a container whose elements get invalidated, so > > `read_edge_event` could require an exclusive reference to `buffer` in > > Rust, that way you cannot keep borrows to its elements like `ev` if > > you want to call it. But of course this requires tying the lifetime of > > the events to that of the buffer. > > What about the below code changes on top of V6 ? > Can you clone the event to detach it from the buffer? Or are you now forced to drop the event before reading more? Cheers, Kent.
On 11-10-22, 12:25, Kent Gibson wrote: > On Tue, Oct 11, 2022 at 09:46:51AM +0530, Viresh Kumar wrote: > > On 29-09-22, 15:55, Miguel Ojeda wrote: > > > It looks like a container whose elements get invalidated, so > > > `read_edge_event` could require an exclusive reference to `buffer` in > > > Rust, that way you cannot keep borrows to its elements like `ev` if > > > you want to call it. But of course this requires tying the lifetime of > > > the events to that of the buffer. > > > > What about the below code changes on top of V6 ? > > > > Can you clone the event to detach it from the buffer? I thought we can always do: event.event_clone() to get a copy, which won't have a reference to the Buffer. This is how it is defined: pub fn event_clone(&self) -> Result<Self> { let event = unsafe { gpiod::gpiod_edge_event_copy(self.event) }; if event.is_null() { return Err(Error::OperationFailed( OperationType::EdgeEventCopy, Errno::last(), )); } Ok(Self { buffer: None, event, }) } But when I try to do this in the earlier example: @@ -40,10 +40,12 @@ fn main() -> Result<()> { let event1 = buffer.event(1)?; println!("{:?}", (event0.line_offset(), event1.line_offset())); + let event0_copy = event0.event_clone()?; drop(event0); // This fails to compile // request.read_edge_events(&mut buffer)?; drop(event1); request.read_edge_events(&mut buffer)?; + drop(event0_copy); } } compilation fails :( error[E0502]: cannot borrow `buffer` as mutable because it is also borrowed as immutable --> libgpiod/examples/gpiobufferevent.rs:48:34 | 39 | let event0 = buffer.event(0)?; | --------------- immutable borrow occurs here ... 48 | request.read_edge_events(&mut buffer)?; | ^^^^^^^^^^^ mutable borrow occurs here 49 | drop(event0_copy); | ----------- immutable borrow later used here And I am not sure why, as the reference isn't used anymore in this case to the event0.
On 11-10-22, 10:07, Viresh Kumar wrote: > And I am not sure why, as the reference isn't used anymore in this > case to the event0. This works though: diff --git a/bindings/rust/libgpiod/examples/gpiobufferevent.rs b/bindings/rust/libgpiod/examples/gpiobufferevent.rs index 613a800584e3..e16a1a39b22c 100644 --- a/bindings/rust/libgpiod/examples/gpiobufferevent.rs +++ b/bindings/rust/libgpiod/examples/gpiobufferevent.rs @@ -40,10 +40,12 @@ fn main() -> Result<()> { let event1 = buffer.event(1)?; println!("{:?}", (event0.line_offset(), event1.line_offset())); + let event0_copy = edge::Event::event_clone(&event0)?; drop(event0); // This fails to compile // request.read_edge_events(&mut buffer)?; drop(event1); request.read_edge_events(&mut buffer)?; + drop(event0_copy); } } diff --git a/bindings/rust/libgpiod/src/edge_event.rs b/bindings/rust/libgpiod/src/edge_event.rs index f5426459d779..12c0fd73f778 100644 --- a/bindings/rust/libgpiod/src/edge_event.rs +++ b/bindings/rust/libgpiod/src/edge_event.rs @@ -43,8 +43,8 @@ impl<'b> Event<'b> { }) } - pub fn event_clone(&self) -> Result<Self> { - let event = unsafe { gpiod::gpiod_edge_event_copy(self.event) }; + pub fn event_clone(event: &Event) -> Result<Self> { + let event = unsafe { gpiod::gpiod_edge_event_copy(event.event) }; if event.is_null() { return Err(Error::OperationFailed( OperationType::EdgeEventCopy,
On 11-10-22, 09:46, Viresh Kumar wrote: > What about the below code changes on top of V6 ? > > Changes: > - Removed BufferInternal. > - Event contains a reference to the Buffer now, with lifetime. > - read_edge_event() expects a mutable reference to buffer, to make it > exclusive, i.e. disallow any previous Event references to exist at > compilation itself. Bartosz, should I send a V7 now with these changes ? I hope everything is settled ?
On Thu, Oct 13, 2022 at 8:12 AM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > On 11-10-22, 09:46, Viresh Kumar wrote: > > What about the below code changes on top of V6 ? > > > > Changes: > > - Removed BufferInternal. > > - Event contains a reference to the Buffer now, with lifetime. > > - read_edge_event() expects a mutable reference to buffer, to make it > > exclusive, i.e. disallow any previous Event references to exist at > > compilation itself. > > Bartosz, should I send a V7 now with these changes ? I hope everything > is settled ? > Maybe also add chained mutators everywhere? To be able to do settings.set_direction().set_edge() etc.? And I would still love a thorough API review from someone who actually knows rust too. :( But I will play some more with v7 so do send it. Bartosz
On 14-10-22, 11:45, Bartosz Golaszewski wrote: > Maybe also add chained mutators everywhere? To be able to do > settings.set_direction().set_edge() etc.? Based on Kent's suggestion earlier, what I have implemented is set_prop(), to which one can pass all settings and it will apply them in a loop. pub fn set_prop(&mut self, values: &[SettingVal]) -> Result<()> { for value in values { match value { SettingVal::Direction(val) => self.set_direction(*val)?, SettingVal::EdgeDetection(val) => self.set_edge_detection(*val)?, SettingVal::Bias(val) => self.set_bias(*val)?, SettingVal::Drive(val) => self.set_drive(*val)?, SettingVal::ActiveLow(val) => self.set_active_low(*val), SettingVal::DebouncePeriod(val) => self.set_debounce_period(*val), SettingVal::EventClock(val) => self.set_event_clock(*val)?, SettingVal::OutputValue(val) => self.set_output_value(*val)?, } } Ok(()) } I think that replaces the need of nested ones ? And if we want to add those later, we can always come back and add them. But I am not sure it would be required. > And I would still love a thorough API review from someone who actually > knows rust too. :( Well, Kent did a very good job earlier. I am not sure if he has extra cycles to review this once again, though not a lot has changed since last time. > But I will play some more with v7 so do send it. Great.
On Fri, Oct 14, 2022 at 11:57 AM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > On 14-10-22, 11:45, Bartosz Golaszewski wrote: > > Maybe also add chained mutators everywhere? To be able to do > > settings.set_direction().set_edge() etc.? > > Based on Kent's suggestion earlier, what I have implemented is > set_prop(), to which one can pass all settings and it will apply them > in a loop. > > pub fn set_prop(&mut self, values: &[SettingVal]) -> Result<()> { > for value in values { > match value { > SettingVal::Direction(val) => self.set_direction(*val)?, > SettingVal::EdgeDetection(val) => self.set_edge_detection(*val)?, > SettingVal::Bias(val) => self.set_bias(*val)?, > SettingVal::Drive(val) => self.set_drive(*val)?, > SettingVal::ActiveLow(val) => self.set_active_low(*val), > SettingVal::DebouncePeriod(val) => self.set_debounce_period(*val), > SettingVal::EventClock(val) => self.set_event_clock(*val)?, > SettingVal::OutputValue(val) => self.set_output_value(*val)?, > } > } > > Ok(()) > } > > I think that replaces the need of nested ones ? And if we want to add > those later, we can always come back and add them. But I am not sure > it would be required. > I cannot find Kent's comment on that - what was the reasoning behind this? > > And I would still love a thorough API review from someone who actually > > knows rust too. :( > > Well, Kent did a very good job earlier. I am not sure if he has extra > cycles to review this once again, though not a lot has changed since > last time. > Yeah sorry Kent, I forgot we're at v6 already and you did review the previous iterations. :) Bart > > But I will play some more with v7 so do send it. > > Great. > > -- > viresh
On Fri, Oct 14, 2022 at 04:25:31PM +0200, Bartosz Golaszewski wrote: > On Fri, Oct 14, 2022 at 11:57 AM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > > > On 14-10-22, 11:45, Bartosz Golaszewski wrote: > > > Maybe also add chained mutators everywhere? To be able to do > > > settings.set_direction().set_edge() etc.? > > > > Based on Kent's suggestion earlier, what I have implemented is > > set_prop(), to which one can pass all settings and it will apply them > > in a loop. > > > > pub fn set_prop(&mut self, values: &[SettingVal]) -> Result<()> { > > for value in values { > > match value { > > SettingVal::Direction(val) => self.set_direction(*val)?, > > SettingVal::EdgeDetection(val) => self.set_edge_detection(*val)?, > > SettingVal::Bias(val) => self.set_bias(*val)?, > > SettingVal::Drive(val) => self.set_drive(*val)?, > > SettingVal::ActiveLow(val) => self.set_active_low(*val), > > SettingVal::DebouncePeriod(val) => self.set_debounce_period(*val), > > SettingVal::EventClock(val) => self.set_event_clock(*val)?, > > SettingVal::OutputValue(val) => self.set_output_value(*val)?, > > } > > } > > > > Ok(()) > > } > > > > I think that replaces the need of nested ones ? And if we want to add > > those later, we can always come back and add them. But I am not sure > > it would be required. > > > > I cannot find Kent's comment on that - what was the reasoning behind this? > The comment was: Add a Setting enum that has a variant for each setting. Then you only need 3 mutators total. And the user can define configs as a list of Settings. So perhaps the mutators should accept &[Setting]. And &[offsets] rather than just offset. Similarly, gets can be consolidated into: get_prop_offset(self, offset, SettingKind) -> Result<Setting> and get_prop_default(self, SettingKind) -> Result<Setting> (simplified signatures) Prior to the line_settings change there was a veritable forest of mutators, so the idea was to consolidate them where possible, cos in Rust you can. But behind the scenes the implementation is just fanning them out again, so I'm not sure I see the benefit if that is the case. If the mutators for each field still exist they may as well be pub. And they should return Result<&mut Self> so they can be chained, as you suggest. Wrt the values param (which I would prefer was called props), the idea was that the config could be built and passed around as pure Rust objects. The set_prop() applied that list to the C line config, at the time using one of the default/offset/subset mutators. So it decoupled the settings from the scope they were to be applied to. Not such an issue now - the scope is always line. Might still be useful, might not. > > > And I would still love a thorough API review from someone who actually > > > knows rust too. :( > > > > Well, Kent did a very good job earlier. I am not sure if he has extra > > cycles to review this once again, though not a lot has changed since > > last time. > > > > Yeah sorry Kent, I forgot we're at v6 already and you did review the > previous iterations. :) > My review was of v4. Things have changed a bit since then. And I agree with your original point - it would be good to get a review from someone that actually uses Rust in anger - I only toy with it. Cheers, Kent.
On Mon, Oct 17, 2022 at 04:56:25PM +0530, Viresh Kumar wrote: > On 15-10-22, 00:06, Kent Gibson wrote: > > If the mutators for each field still exist they may as well be pub. > > > > And they should return Result<&mut Self> so they can be chained, as you > > suggest. > > > > Wrt the values param (which I would prefer was called props) > > Is this fine now ? Rebased over v7. > > diff --git a/bindings/rust/libgpiod/src/line_settings.rs b/bindings/rust/libgpiod/src/line_settings.rs > index 2c3090132ea5..be50b5b41c5a 100644 > --- a/bindings/rust/libgpiod/src/line_settings.rs > +++ b/bindings/rust/libgpiod/src/line_settings.rs > @@ -70,18 +70,18 @@ impl Settings { > } > > /// Set line prop setting. > - pub fn set_prop(&mut self, values: &[SettingVal]) -> Result<()> { > - for value in values { > - match value { > - SettingVal::Direction(val) => self.set_direction(*val)?, > - SettingVal::EdgeDetection(val) => self.set_edge_detection(*val)?, > - SettingVal::Bias(val) => self.set_bias(*val)?, > - SettingVal::Drive(val) => self.set_drive(*val)?, > - SettingVal::ActiveLow(val) => self.set_active_low(*val), > - SettingVal::DebouncePeriod(val) => self.set_debounce_period(*val), > - SettingVal::EventClock(val) => self.set_event_clock(*val)?, > - SettingVal::OutputValue(val) => self.set_output_value(*val)?, > - } > + pub fn set_prop(&mut self, props: &[SettingVal]) -> Result<()> { ^ also &mut Self Apart from that, yeah that is what I had in mind, as also noted in my forthcoming v7 review. Cheers, Kent.
diff --git a/bindings/rust/Cargo.toml b/bindings/rust/Cargo.toml index d0b3a3c88ff1..1e57ef2c0002 100644 --- a/bindings/rust/Cargo.toml +++ b/bindings/rust/Cargo.toml @@ -2,4 +2,5 @@ members = [ "libgpiod-sys", + "libgpiod" ] diff --git a/bindings/rust/libgpiod/Cargo.toml b/bindings/rust/libgpiod/Cargo.toml new file mode 100644 index 000000000000..f25242abb153 --- /dev/null +++ b/bindings/rust/libgpiod/Cargo.toml @@ -0,0 +1,13 @@ +[package] +name = "libgpiod" +version = "0.1.0" +edition = "2018" + +# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html + +[dependencies] +intmap = "2.0.0" +libc = ">=0.2.39" +libgpiod-sys = { path = "../libgpiod-sys" } +thiserror = "1.0" +vmm-sys-util = "=0.10.0" diff --git a/bindings/rust/libgpiod/src/chip.rs b/bindings/rust/libgpiod/src/chip.rs new file mode 100644 index 000000000000..4f52c3f141f4 --- /dev/null +++ b/bindings/rust/libgpiod/src/chip.rs @@ -0,0 +1,253 @@ +// SPDX-License-Identifier: Apache-2.0 AND BSD-3-Clause +// +// Copyright 2022 Linaro Ltd. All Rights Reserved. +// Viresh Kumar <viresh.kumar@linaro.org> + +use libc::strlen; +use std::os::raw::c_char; +use std::path::Path; +use std::sync::Arc; +use std::time::Duration; +use std::{slice, str}; + +use vmm_sys_util::errno::Error as Errno; + +use super::{gpiod, info, line, request, Error, Offset, OperationType, Result}; + +#[derive(Clone, Debug, Eq, PartialEq)] +pub(crate) struct Internal { + chip: *mut gpiod::gpiod_chip, +} + +impl Internal { + /// Find a chip by path. + pub(crate) fn open<P: AsRef<Path>>(path: &P) -> Result<Self> { + // Null-terminate the string + let path = path.as_ref().to_string_lossy() + "\0"; + + let chip = unsafe { gpiod::gpiod_chip_open(path.as_ptr() as *const c_char) }; + if chip.is_null() { + return Err(Error::OperationFailed( + OperationType::ChipOpen, + Errno::last(), + )); + } + + Ok(Self { chip }) + } + + /// Private helper, Returns gpiod_chip + pub(crate) fn chip(&self) -> *mut gpiod::gpiod_chip { + self.chip + } +} + +impl Drop for Internal { + /// Close the chip and release all associated resources. + fn drop(&mut self) { + unsafe { gpiod::gpiod_chip_close(self.chip) } + } +} + +/// GPIO chip +/// +/// A GPIO chip object is associated with an open file descriptor to the GPIO +/// character device. It exposes basic information about the chip and allows +/// callers to retrieve information about each line, watch lines for state +/// changes and make line requests. +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct Chip { + ichip: Arc<Internal>, + info: Info, +} + +unsafe impl Send for Chip {} +unsafe impl Sync for Chip {} + +impl Chip { + /// Find a chip by path. + pub fn open<P: AsRef<Path>>(path: &P) -> Result<Self> { + let ichip = Arc::new(Internal::open(path)?); + let info = Info::new(ichip.clone())?; + + Ok(Self { ichip, info }) + } + + /// Get the chip name as represented in the kernel. + pub fn name(&self) -> Result<&str> { + self.info.name() + } + + /// Get the chip label as represented in the kernel. + pub fn label(&self) -> Result<&str> { + self.info.label() + } + + /// Get the number of GPIO lines exposed by the chip. + pub fn num_lines(&self) -> usize { + self.info.num_lines() + } + + /// Get the path used to find the chip. + pub fn path(&self) -> Result<&str> { + // SAFETY: The string returned by libgpiod is guaranteed to live as long + // as the `struct Chip`. + let path = unsafe { gpiod::gpiod_chip_get_path(self.ichip.chip()) }; + + // SAFETY: The string is guaranteed to be valid here by the C API. + str::from_utf8(unsafe { slice::from_raw_parts(path as *const u8, strlen(path) as usize) }) + .map_err(Error::StringNotUtf8) + } + + /// Get information about the chip. + pub fn info(&self) -> Result<Info> { + Info::new(self.ichip.clone()) + } + + /// Get a snapshot of information about the line. + pub fn line_info(&self, offset: Offset) -> Result<line::Info> { + line::Info::new(self.ichip.clone(), offset) + } + + /// Get the current snapshot of information about the line at given offset and start watching + /// it for future changes. + pub fn watch_line_info(&self, offset: Offset) -> Result<line::Info> { + line::Info::new_watch(self.ichip.clone(), offset) + } + + /// Stop watching a line + pub fn unwatch(&self, offset: Offset) { + unsafe { + gpiod::gpiod_chip_unwatch_line_info(self.ichip.chip(), offset); + } + } + + /// Get the file descriptor associated with the chip. + /// + /// The returned file descriptor must not be closed by the caller, else other methods for the + /// `struct Chip` may fail. + pub fn fd(&self) -> Result<u32> { + let fd = unsafe { gpiod::gpiod_chip_get_fd(self.ichip.chip()) }; + + if fd < 0 { + Err(Error::OperationFailed( + OperationType::ChipGetFd, + Errno::last(), + )) + } else { + Ok(fd as u32) + } + } + + /// Wait for line status events on any of the watched lines on the chip. + pub fn wait_info_event(&self, timeout: Option<Duration>) -> Result<bool> { + let timeout = match timeout { + Some(x) => x.as_nanos() as i64, + // Block indefinitely + None => -1, + }; + + let ret = unsafe { gpiod::gpiod_chip_wait_info_event(self.ichip.chip(), timeout) }; + + match ret { + -1 => Err(Error::OperationFailed( + OperationType::ChipWaitInfoEvent, + Errno::last(), + )), + 0 => Ok(false), + _ => Ok(true), + } + } + + /// Read a single line status change event from the chip. If no events are + /// pending, this function will block. + pub fn read_info_event(&self) -> Result<info::Event> { + info::Event::new(&self.ichip) + } + + /// Map a GPIO line's name to its offset within the chip. + pub fn line_offset_from_name(&self, name: &str) -> Result<Offset> { + // Null-terminate the string + let name = name.to_owned() + "\0"; + + let ret = unsafe { + gpiod::gpiod_chip_get_line_offset_from_name( + self.ichip.chip(), + name.as_ptr() as *const c_char, + ) + }; + + if ret == -1 { + Err(Error::OperationFailed( + OperationType::ChipGetLine, + Errno::last(), + )) + } else { + Ok(ret as u32) + } + } + + /// Request a set of lines for exclusive usage. + pub fn request_lines( + &self, + rconfig: &request::Config, + lconfig: &line::Config, + ) -> Result<line::Request> { + line::Request::new(&self.ichip, rconfig, lconfig) + } +} + +/// GPIO chip Information +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct Info { + info: *mut gpiod::gpiod_chip_info, +} + +impl Info { + /// Find a GPIO chip by path. + pub(crate) fn new(chip: Arc<Internal>) -> Result<Self> { + let info = unsafe { gpiod::gpiod_chip_get_info(chip.chip()) }; + if info.is_null() { + return Err(Error::OperationFailed( + OperationType::ChipInfoGet, + Errno::last(), + )); + } + + Ok(Self { info }) + } + + /// Get the GPIO chip name as represented in the kernel. + pub(crate) fn name(&self) -> Result<&str> { + // SAFETY: The string returned by libgpiod is guaranteed to live as long + // as the `struct Chip`. + let name = unsafe { gpiod::gpiod_chip_info_get_name(self.info) }; + + // SAFETY: The string is guaranteed to be valid here by the C API. + str::from_utf8(unsafe { slice::from_raw_parts(name as *const u8, strlen(name) as usize) }) + .map_err(Error::StringNotUtf8) + } + + /// Get the GPIO chip label as represented in the kernel. + pub(crate) fn label(&self) -> Result<&str> { + // SAFETY: The string returned by libgpiod is guaranteed to live as long + // as the `struct Chip`. + let label = unsafe { gpiod::gpiod_chip_info_get_label(self.info) }; + + // SAFETY: The string is guaranteed to be valid here by the C API. + str::from_utf8(unsafe { slice::from_raw_parts(label as *const u8, strlen(label) as usize) }) + .map_err(Error::StringNotUtf8) + } + + /// Get the number of GPIO lines exposed by the chip. + pub(crate) fn num_lines(&self) -> usize { + unsafe { gpiod::gpiod_chip_info_get_num_lines(self.info) as usize } + } +} + +impl Drop for Info { + /// Close the GPIO chip info and release all associated resources. + fn drop(&mut self) { + unsafe { gpiod::gpiod_chip_info_free(self.info) } + } +} diff --git a/bindings/rust/libgpiod/src/edge_event.rs b/bindings/rust/libgpiod/src/edge_event.rs new file mode 100644 index 000000000000..ce583916a2e3 --- /dev/null +++ b/bindings/rust/libgpiod/src/edge_event.rs @@ -0,0 +1,102 @@ +// SPDX-License-Identifier: Apache-2.0 AND BSD-3-Clause +// +// Copyright 2022 Linaro Ltd. All Rights Reserved. +// Viresh Kumar <viresh.kumar@linaro.org> + +use std::sync::Arc; +use std::time::Duration; + +use vmm_sys_util::errno::Error as Errno; + +use super::{edge::event::BufferInternal, gpiod, EdgeKind, Error, Offset, OperationType, Result}; + +/// Line edge events handling +/// +/// An edge event object contains information about a single line edge event. +/// It contains the event type, timestamp and the offset of the line on which +/// the event occurred as well as two sequence numbers (global for all lines +/// in the associated request and local for this line only). +/// +/// Edge events are stored into an edge-event buffer object to improve +/// performance and to limit the number of memory allocations when a large +/// number of events are being read. + +#[derive(Debug, Eq, PartialEq)] +pub struct Event { + ibuffer: Option<Arc<BufferInternal>>, + event: *mut gpiod::gpiod_edge_event, +} + +impl Event { + /// Get an event stored in the buffer. + pub(crate) fn new(ibuffer: &Arc<BufferInternal>, index: u64) -> Result<Self> { + let event = unsafe { gpiod::gpiod_edge_event_buffer_get_event(ibuffer.buffer(), index) }; + if event.is_null() { + return Err(Error::OperationFailed( + OperationType::EdgeEventBufferGetEvent, + Errno::last(), + )); + } + + Ok(Self { + ibuffer: Some(ibuffer.clone()), + event, + }) + } + + pub fn event_clone(&self) -> Result<Self> { + let event = unsafe { gpiod::gpiod_edge_event_copy(self.event) }; + if event.is_null() { + return Err(Error::OperationFailed( + OperationType::EdgeEventCopy, + Errno::last(), + )); + } + + Ok(Self { + ibuffer: None, + event, + }) + } + + /// Get the event type. + pub fn event_type(&self) -> Result<EdgeKind> { + EdgeKind::new(unsafe { gpiod::gpiod_edge_event_get_event_type(self.event) } as u32) + } + + /// Get the timestamp of the event. + pub fn timestamp(&self) -> Duration { + Duration::from_nanos(unsafe { gpiod::gpiod_edge_event_get_timestamp_ns(self.event) }) + } + + /// Get the offset of the line on which the event was triggered. + pub fn line_offset(&self) -> Offset { + unsafe { gpiod::gpiod_edge_event_get_line_offset(self.event) } + } + + /// Get the global sequence number of the event. + /// + /// Returns sequence number of the event relative to all lines in the + /// associated line request. + pub fn global_seqno(&self) -> u64 { + unsafe { gpiod::gpiod_edge_event_get_global_seqno(self.event) } + } + + /// Get the event sequence number specific to concerned line. + /// + /// Returns sequence number of the event relative to the line within the + /// lifetime of the associated line request. + pub fn line_seqno(&self) -> u64 { + unsafe { gpiod::gpiod_edge_event_get_line_seqno(self.event) } + } +} + +impl Drop for Event { + /// Free the edge event. + fn drop(&mut self) { + // Free the event only if a copy is made + if self.ibuffer.is_none() { + unsafe { gpiod::gpiod_edge_event_free(self.event) }; + } + } +} diff --git a/bindings/rust/libgpiod/src/event_buffer.rs b/bindings/rust/libgpiod/src/event_buffer.rs new file mode 100644 index 000000000000..e272e7aa9e9d --- /dev/null +++ b/bindings/rust/libgpiod/src/event_buffer.rs @@ -0,0 +1,90 @@ +// SPDX-License-Identifier: Apache-2.0 AND BSD-3-Clause +// +// Copyright 2022 Linaro Ltd. All Rights Reserved. +// Viresh Kumar <viresh.kumar@linaro.org> + +use std::os::raw::c_ulong; +use std::sync::Arc; + +use vmm_sys_util::errno::Error as Errno; + +use super::{edge, gpiod, Error, OperationType, Result}; + +/// Line edge events buffer +#[derive(Clone, Debug, Eq, PartialEq)] +pub(crate) struct BufferInternal { + buffer: *mut gpiod::gpiod_edge_event_buffer, +} + +impl BufferInternal { + /// Create a new edge event buffer. + /// + /// If capacity equals 0, it will be set to a default value of 64. If + /// capacity is larger than 1024, it will be limited to 1024. + pub fn new(capacity: usize) -> Result<Self> { + let buffer = unsafe { gpiod::gpiod_edge_event_buffer_new(capacity as c_ulong) }; + if buffer.is_null() { + return Err(Error::OperationFailed( + OperationType::EdgeEventBufferNew, + Errno::last(), + )); + } + + Ok(Self { buffer }) + } + + /// Private helper, Returns gpiod_edge_event_buffer + pub(crate) fn buffer(&self) -> *mut gpiod::gpiod_edge_event_buffer { + self.buffer + } +} + +impl Drop for BufferInternal { + /// Free the edge event buffer and release all associated resources. + fn drop(&mut self) { + unsafe { gpiod::gpiod_edge_event_buffer_free(self.buffer) }; + } +} + +/// Line edge events buffer +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct Buffer { + ibuffer: Arc<BufferInternal>, +} + +impl Buffer { + /// Create a new edge event buffer. + /// + /// If capacity equals 0, it will be set to a default value of 64. If + /// capacity is larger than 1024, it will be limited to 1024. + pub fn new(capacity: usize) -> Result<Self> { + Ok(Self { + ibuffer: Arc::new(BufferInternal::new(capacity)?), + }) + } + + /// Private helper, Returns gpiod_edge_event_buffer + pub(crate) fn buffer(&self) -> *mut gpiod::gpiod_edge_event_buffer { + self.ibuffer.buffer() + } + + /// Get the capacity of the event buffer. + pub fn capacity(&self) -> usize { + unsafe { gpiod::gpiod_edge_event_buffer_get_capacity(self.buffer()) as usize } + } + + /// Read an event stored in the buffer. + pub fn event(&self, index: u64) -> Result<edge::Event> { + edge::Event::new(&self.ibuffer, index) + } + + /// Get the number of events the buffer contains. + pub fn len(&self) -> usize { + unsafe { gpiod::gpiod_edge_event_buffer_get_num_events(self.buffer()) as usize } + } + + /// Check if buffer is empty. + pub fn is_empty(&self) -> bool { + self.len() == 0 + } +} diff --git a/bindings/rust/libgpiod/src/info_event.rs b/bindings/rust/libgpiod/src/info_event.rs new file mode 100644 index 000000000000..d8be87df6679 --- /dev/null +++ b/bindings/rust/libgpiod/src/info_event.rs @@ -0,0 +1,68 @@ +// SPDX-License-Identifier: Apache-2.0 AND BSD-3-Clause +// +// Copyright 2022 Linaro Ltd. All Rights Reserved. +// Viresh Kumar <viresh.kumar@linaro.org> + +use std::sync::Arc; +use std::time::Duration; + +use vmm_sys_util::errno::Error as Errno; + +use super::{chip, gpiod, line, Error, InfoChangeKind, OperationType, Result}; + +/// Line status watch events +/// +/// Accessors for the info event objects allowing to monitor changes in GPIO +/// line state. +/// +/// Callers can be notified about changes in line's state using the interfaces +/// exposed by GPIO chips. Each info event contains information about the event +/// itself (timestamp, type) as well as a snapshot of line's state in the form +/// of a line-info object. + +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct Event { + event: *mut gpiod::gpiod_info_event, +} + +impl Event { + /// Get a single chip's line's status change event. + pub(crate) fn new(ichip: &Arc<chip::Internal>) -> Result<Self> { + let event = unsafe { gpiod::gpiod_chip_read_info_event(ichip.chip()) }; + if event.is_null() { + return Err(Error::OperationFailed( + OperationType::ChipReadInfoEvent, + Errno::last(), + )); + } + + Ok(Self { event }) + } + + /// Private helper, Returns gpiod_info_event + pub(crate) fn event(&self) -> *mut gpiod::gpiod_info_event { + self.event + } + + /// Get the event type of the status change event. + pub fn event_type(&self) -> Result<InfoChangeKind> { + InfoChangeKind::new(unsafe { gpiod::gpiod_info_event_get_event_type(self.event) } as u32) + } + + /// Get the timestamp of the event, read from the monotonic clock. + pub fn timestamp(&self) -> Duration { + Duration::from_nanos(unsafe { gpiod::gpiod_info_event_get_timestamp_ns(self.event) }) + } + + /// Get the line-info object associated with the event. + pub fn line_info(&self) -> Result<line::Info> { + line::Info::new_from_event(self) + } +} + +impl Drop for Event { + /// Free the info event object and release all associated resources. + fn drop(&mut self) { + unsafe { gpiod::gpiod_info_event_free(self.event) } + } +} diff --git a/bindings/rust/libgpiod/src/lib.rs b/bindings/rust/libgpiod/src/lib.rs new file mode 100644 index 000000000000..c5d974a9f4ea --- /dev/null +++ b/bindings/rust/libgpiod/src/lib.rs @@ -0,0 +1,471 @@ +// SPDX-License-Identifier: Apache-2.0 AND BSD-3-Clause +// +// Rust wrappers for GPIOD APIs +// +// Copyright 2022 Linaro Ltd. All Rights Reserved. +// Viresh Kumar <viresh.kumar@linaro.org> + +//! libgpiod public API +//! +//! This is the complete documentation of the public Rust API made available to +//! users of libgpiod. +//! +//! The API is logically split into several parts such as: GPIO chip & line +//! operators, GPIO events handling etc. + +/// GPIO chip related definitions. +pub mod chip; + +mod edge_event; +mod event_buffer; + +/// GPIO chip edge event related definitions. +pub mod edge { + pub use crate::edge_event::*; + + /// GPIO chip edge event buffer related definitions. + pub mod event { + pub use crate::event_buffer::*; + } +} + +mod info_event; + +/// GPIO chip info event related definitions. +pub mod info { + pub use crate::info_event::*; +} + +mod line_config; +mod line_info; +mod line_request; +mod line_settings; + +/// GPIO chip line related definitions. +pub mod line { + pub use crate::line_config::*; + pub use crate::line_info::*; + pub use crate::line_request::*; + pub use crate::line_settings::*; +} + +mod request_config; + +/// GPIO chip request related definitions. +pub mod request { + pub use crate::request_config::*; +} + +use libgpiod_sys as gpiod; + +use intmap::IntMap; +use libc::strlen; +use std::fs; +use std::os::raw::c_char; +use std::path::Path; +use std::time::Duration; +use std::{fmt, slice, str}; + +use thiserror::Error as ThisError; +use vmm_sys_util::errno::Error as Errno; + +/// Operation types, used with OperationFailed() Error. +#[derive(Copy, Clone, Debug, Eq, PartialEq)] +pub enum OperationType { + ChipOpen, + ChipGetFd, + ChipWaitInfoEvent, + ChipGetLine, + ChipGetLineInfo, + ChipInfoGet, + ChipReadInfoEvent, + ChipWatchLineInfo, + EdgeEventBufferGetEvent, + EdgeEventCopy, + EdgeEventBufferNew, + InfoEventGetLineInfo, + LineConfigNew, + LineConfigAddSettings, + LineConfigGetOffsets, + LineConfigGetSetting, + LineRequest, + LineRequestReconfigLines, + LineRequestGetVal, + LineRequestGetValSubset, + LineRequestSetVal, + LineRequestSetValSubset, + LineRequestReadEdgeEvent, + LineRequestWaitEdgeEvent, + LineSettingsNew, + LineSettingsCopy, + LineSettingsGetOutVal, + LineSettingsSetDirection, + LineSettingsSetEdgeDetection, + LineSettingsSetBias, + LineSettingsSetDrive, + LineSettingsSetActiveLow, + LineSettingsSetDebouncePeriod, + LineSettingsSetEventClock, + LineSettingsSetOutputValue, + RequestConfigNew, + RequestConfigGetConsumer, + SimBankGetVal, + SimBankNew, + SimBankSetLabel, + SimBankSetNumLines, + SimBankSetLineName, + SimBankSetPull, + SimBankHogLine, + SimCtxNew, + SimDevNew, + SimDevEnable, + SimDevDisable, +} + +impl fmt::Display for OperationType { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{:?}", self) + } +} + +/// Result of libgpiod operations. +pub type Result<T> = std::result::Result<T, Error>; + +/// Error codes for libgpiod operations. +#[derive(Copy, Clone, Debug, PartialEq, ThisError)] +pub enum Error { + #[error("Failed to get {0}")] + NullString(&'static str), + #[error("String not utf8: {0:?}")] + StringNotUtf8(str::Utf8Error), + #[error("Invalid enum {0} value: {1}")] + InvalidEnumValue(&'static str, u32), + #[error("Operation {0} Failed: {1}")] + OperationFailed(OperationType, Errno), + #[error("Invalid Arguments")] + InvalidArguments, + #[error("Std Io Error")] + IoError, +} + +/// Value settings. +#[derive(Copy, Clone, Debug, Eq, PartialEq)] +pub enum Value { + /// Active + Active, + /// Inactive + InActive, +} + +/// Maps offset to Value. +pub type ValueMap = IntMap<Value>; + +impl Value { + pub fn new(val: i32) -> Result<Self> { + match val { + 0 => Ok(Value::InActive), + 1 => Ok(Value::Active), + _ => Err(Error::InvalidEnumValue("Value", val as u32)), + } + } + + fn value(&self) -> i32 { + match self { + Value::Active => 1, + Value::InActive => 0, + } + } +} + +/// Offset type. +pub type Offset = u32; + +/// Direction settings. +#[derive(Copy, Clone, Debug, Eq, PartialEq)] +pub enum Direction { + /// Request the line(s), but don't change direction. + AsIs, + /// Direction is input - for reading the value of an externally driven GPIO line. + Input, + /// Direction is output - for driving the GPIO line. + Output, +} + +impl Direction { + fn new(dir: u32) -> Result<Self> { + match dir { + gpiod::GPIOD_LINE_DIRECTION_AS_IS => Ok(Direction::AsIs), + gpiod::GPIOD_LINE_DIRECTION_INPUT => Ok(Direction::Input), + gpiod::GPIOD_LINE_DIRECTION_OUTPUT => Ok(Direction::Output), + _ => Err(Error::InvalidEnumValue("Direction", dir)), + } + } + + fn gpiod_direction(&self) -> u32 { + match self { + Direction::AsIs => gpiod::GPIOD_LINE_DIRECTION_AS_IS, + Direction::Input => gpiod::GPIOD_LINE_DIRECTION_INPUT, + Direction::Output => gpiod::GPIOD_LINE_DIRECTION_OUTPUT, + } + } +} + +/// Internal bias settings. +#[derive(Copy, Clone, Debug, Eq, PartialEq)] +pub enum Bias { + /// The internal bias is disabled. + Disabled, + /// The internal pull-up bias is enabled. + PullUp, + /// The internal pull-down bias is enabled. + PullDown, +} + +impl Bias { + fn new(bias: u32) -> Result<Option<Self>> { + match bias { + gpiod::GPIOD_LINE_BIAS_UNKNOWN => Ok(None), + gpiod::GPIOD_LINE_BIAS_AS_IS => Ok(None), + gpiod::GPIOD_LINE_BIAS_DISABLED => Ok(Some(Bias::Disabled)), + gpiod::GPIOD_LINE_BIAS_PULL_UP => Ok(Some(Bias::PullUp)), + gpiod::GPIOD_LINE_BIAS_PULL_DOWN => Ok(Some(Bias::PullDown)), + _ => Err(Error::InvalidEnumValue("Bias", bias)), + } + } + + fn gpiod_bias(bias: Option<Bias>) -> u32 { + match bias { + None => gpiod::GPIOD_LINE_BIAS_AS_IS, + Some(bias) => match bias { + Bias::Disabled => gpiod::GPIOD_LINE_BIAS_DISABLED, + Bias::PullUp => gpiod::GPIOD_LINE_BIAS_PULL_UP, + Bias::PullDown => gpiod::GPIOD_LINE_BIAS_PULL_DOWN, + }, + } + } +} + +/// Drive settings. +#[derive(Copy, Clone, Debug, Eq, PartialEq)] +pub enum Drive { + /// Drive setting is push-pull. + PushPull, + /// Line output is open-drain. + OpenDrain, + /// Line output is open-source. + OpenSource, +} + +impl Drive { + fn new(drive: u32) -> Result<Self> { + match drive { + gpiod::GPIOD_LINE_DRIVE_PUSH_PULL => Ok(Drive::PushPull), + gpiod::GPIOD_LINE_DRIVE_OPEN_DRAIN => Ok(Drive::OpenDrain), + gpiod::GPIOD_LINE_DRIVE_OPEN_SOURCE => Ok(Drive::OpenSource), + _ => Err(Error::InvalidEnumValue("Drive", drive)), + } + } + + fn gpiod_drive(&self) -> u32 { + match self { + Drive::PushPull => gpiod::GPIOD_LINE_DRIVE_PUSH_PULL, + Drive::OpenDrain => gpiod::GPIOD_LINE_DRIVE_OPEN_DRAIN, + Drive::OpenSource => gpiod::GPIOD_LINE_DRIVE_OPEN_SOURCE, + } + } +} + +/// Edge detection settings. +#[derive(Copy, Clone, Debug, Eq, PartialEq)] +pub enum Edge { + /// Line detects rising edge events. + Rising, + /// Line detects falling edge events. + Falling, + /// Line detects both rising and falling edge events. + Both, +} + +impl Edge { + fn new(edge: u32) -> Result<Option<Self>> { + match edge { + gpiod::GPIOD_LINE_EDGE_NONE => Ok(None), + gpiod::GPIOD_LINE_EDGE_RISING => Ok(Some(Edge::Rising)), + gpiod::GPIOD_LINE_EDGE_FALLING => Ok(Some(Edge::Falling)), + gpiod::GPIOD_LINE_EDGE_BOTH => Ok(Some(Edge::Both)), + _ => Err(Error::InvalidEnumValue("Edge", edge)), + } + } + + fn gpiod_edge(edge: Option<Edge>) -> u32 { + match edge { + None => gpiod::GPIOD_LINE_EDGE_NONE, + Some(edge) => match edge { + Edge::Rising => gpiod::GPIOD_LINE_EDGE_RISING, + Edge::Falling => gpiod::GPIOD_LINE_EDGE_FALLING, + Edge::Both => gpiod::GPIOD_LINE_EDGE_BOTH, + }, + } + } +} + +/// Line setting kind. +#[derive(Copy, Clone, Debug, Eq, PartialEq)] +pub enum SettingKind { + /// Line direction. + Direction, + /// Bias. + Bias, + /// Drive. + Drive, + /// Edge detection. + EdgeDetection, + /// Active-low setting. + ActiveLow, + /// Debounce period. + DebouncePeriod, + /// Event clock type. + EventClock, + /// Output value. + OutputValue, +} + +/// Line settings. +#[derive(Copy, Clone, Debug, Eq, PartialEq)] +pub enum SettingVal { + /// Line direction. + Direction(Direction), + /// Bias. + Bias(Option<Bias>), + /// Drive. + Drive(Drive), + /// Edge detection. + EdgeDetection(Option<Edge>), + /// Active-low setting. + ActiveLow(bool), + /// Debounce period. + DebouncePeriod(Duration), + /// Event clock type. + EventClock(EventClock), + /// Output value. + OutputValue(Value), +} + +impl fmt::Display for SettingVal { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{:?}", self) + } +} + +/// Event clock settings. +#[derive(Copy, Clone, Debug, Eq, PartialEq)] +pub enum EventClock { + /// Line uses the monotonic clock for edge event timestamps. + Monotonic, + /// Line uses the realtime clock for edge event timestamps. + Realtime, + /// Line uses the hardware timestamp engine clock for edge event timestamps. + HTE, +} + +impl EventClock { + fn new(clock: u32) -> Result<Self> { + match clock { + gpiod::GPIOD_LINE_EVENT_CLOCK_MONOTONIC => Ok(EventClock::Monotonic), + gpiod::GPIOD_LINE_EVENT_CLOCK_REALTIME => Ok(EventClock::Realtime), + gpiod::GPIOD_LINE_EVENT_CLOCK_HTE => Ok(EventClock::HTE), + _ => Err(Error::InvalidEnumValue("Eventclock", clock)), + } + } + + fn gpiod_clock(&self) -> u32 { + match self { + EventClock::Monotonic => gpiod::GPIOD_LINE_EVENT_CLOCK_MONOTONIC, + EventClock::Realtime => gpiod::GPIOD_LINE_EVENT_CLOCK_REALTIME, + EventClock::HTE => gpiod::GPIOD_LINE_EVENT_CLOCK_HTE, + } + } +} + +/// Line status change event types. +#[derive(Copy, Clone, Debug, Eq, PartialEq)] +pub enum InfoChangeKind { + /// Line has been requested. + LineRequested, + /// Previously requested line has been released. + LineReleased, + /// Line configuration has changed. + LineConfigChanged, +} + +impl InfoChangeKind { + fn new(kind: u32) -> Result<Self> { + match kind { + gpiod::GPIOD_INFO_EVENT_LINE_REQUESTED => Ok(InfoChangeKind::LineRequested), + gpiod::GPIOD_INFO_EVENT_LINE_RELEASED => Ok(InfoChangeKind::LineReleased), + gpiod::GPIOD_INFO_EVENT_LINE_CONFIG_CHANGED => Ok(InfoChangeKind::LineConfigChanged), + _ => Err(Error::InvalidEnumValue("InfoChangeKind", kind)), + } + } +} + +/// Edge event types. +#[derive(Copy, Clone, Debug, Eq, PartialEq)] +pub enum EdgeKind { + /// Rising edge event. + Rising, + /// Falling edge event. + Falling, +} + +impl EdgeKind { + fn new(kind: u32) -> Result<Self> { + match kind { + gpiod::GPIOD_EDGE_EVENT_RISING_EDGE => Ok(EdgeKind::Rising), + gpiod::GPIOD_EDGE_EVENT_FALLING_EDGE => Ok(EdgeKind::Falling), + _ => Err(Error::InvalidEnumValue("EdgeEvent", kind)), + } + } +} + +/// Various libgpiod-related functions. + +/// Check if the file pointed to by path is a GPIO chip character device. +/// +/// Returns true if the file exists and is a GPIO chip character device or a +/// symbolic link to it. +pub fn is_gpiochip_device<P: AsRef<Path>>(path: &P) -> bool { + // Null-terminate the string + let path = path.as_ref().to_string_lossy() + "\0"; + + unsafe { gpiod::gpiod_is_gpiochip_device(path.as_ptr() as *const c_char) } +} + +/// Iterator for GPIO devices. +pub fn gpiochip_devices<P: AsRef<Path>>(path: &P) -> Result<Vec<chip::Chip>> { + let mut chips = Vec::new(); + + for entry in fs::read_dir(path).map_err(|_| Error::IoError)?.flatten() { + let path = entry.path(); + + if is_gpiochip_device(&path) { + chips.push(chip::Chip::open(&path)?); + } + } + + Ok(chips) +} + +/// Get the API version of the library as a human-readable string. +pub fn version_string() -> Result<&'static str> { + // SAFETY: The string returned by libgpiod is guaranteed to live forever. + let version = unsafe { gpiod::gpiod_version_string() }; + + if version.is_null() { + return Err(Error::NullString("GPIO library version")); + } + + // SAFETY: The string is guaranteed to be valid here by the C API. + str::from_utf8(unsafe { slice::from_raw_parts(version as *const u8, strlen(version) as usize) }) + .map_err(Error::StringNotUtf8) +} diff --git a/bindings/rust/libgpiod/src/line_config.rs b/bindings/rust/libgpiod/src/line_config.rs new file mode 100644 index 000000000000..ea25452fd1d6 --- /dev/null +++ b/bindings/rust/libgpiod/src/line_config.rs @@ -0,0 +1,118 @@ +// SPDX-License-Identifier: Apache-2.0 AND BSD-3-Clause +// +// Copyright 2022 Linaro Ltd. All Rights Reserved. +// Viresh Kumar <viresh.kumar@linaro.org> + +use std::os::raw::c_ulong; +use std::slice::from_raw_parts; + +use vmm_sys_util::errno::Error as Errno; + +use super::{gpiod, line::Settings, Error, Offset, OperationType, Result}; + +/// Line configuration objects. +/// +/// The line-config object contains the configuration for lines that can be +/// used in two cases: +/// - when making a line request +/// - when reconfiguring a set of already requested lines. +/// +/// A new line-config object is empty. Using it in a request will lead to an +/// error. In order to a line-config to become useful, it needs to be assigned +/// at least one offset-to-settings mapping by calling +/// ::gpiod_line_config_add_line_settings. +/// +/// When calling ::gpiod_chip_request_lines, the library will request all +/// offsets that were assigned settings in the order that they were assigned. + +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct Config { + config: *mut gpiod::gpiod_line_config, +} + +impl Config { + /// Create a new line config object. + pub fn new() -> Result<Self> { + let config = unsafe { gpiod::gpiod_line_config_new() }; + + if config.is_null() { + return Err(Error::OperationFailed( + OperationType::LineConfigNew, + Errno::last(), + )); + } + + Ok(Self { config }) + } + + /// Private helper, Returns gpiod_line_config + pub(crate) fn config(&self) -> *mut gpiod::gpiod_line_config { + self.config + } + + /// Resets the entire configuration stored in the object. This is useful if + /// the user wants to reuse the object without reallocating it. + pub fn reset(&mut self) { + unsafe { gpiod::gpiod_line_config_reset(self.config) } + } + + /// Add line settings for a set of offsets. + pub fn add_line_settings(&self, offsets: &[Offset], settings: Settings) -> Result<()> { + let ret = unsafe { + gpiod::gpiod_line_config_add_line_settings( + self.config, + offsets.as_ptr(), + offsets.len() as c_ulong, + settings.settings(), + ) + }; + + if ret == -1 { + Err(Error::OperationFailed( + OperationType::LineConfigAddSettings, + Errno::last(), + )) + } else { + Ok(()) + } + } + + /// Get line settings for offset. + pub fn line_settings(&self, offset: Offset) -> Result<Settings> { + let settings = unsafe { gpiod::gpiod_line_config_get_line_settings(self.config, offset) }; + + if settings.is_null() { + return Err(Error::OperationFailed( + OperationType::LineConfigGetSetting, + Errno::last(), + )); + } + + Ok(Settings::new_with_settings(settings)) + } + + /// Get configured offsets. + pub fn offsets(&self) -> Result<Vec<Offset>> { + let mut num: u64 = 0; + let mut offsets: *mut Offset = std::ptr::null_mut(); + + let ret = + unsafe { gpiod::gpiod_line_config_get_offsets(self.config, &mut num, &mut offsets) }; + + if ret == -1 { + Err(Error::OperationFailed( + OperationType::LineConfigGetOffsets, + Errno::last(), + )) + } else { + Ok(unsafe { from_raw_parts(offsets as *const Offset, num as usize).to_vec() }) + } + } +} + +impl Drop for Config { + /// Free the line config object and release all associated resources. + fn drop(&mut self) { + unsafe { gpiod::gpiod_line_config_free(self.config) } + } +} diff --git a/bindings/rust/libgpiod/src/line_info.rs b/bindings/rust/libgpiod/src/line_info.rs new file mode 100644 index 000000000000..9db51fc30efd --- /dev/null +++ b/bindings/rust/libgpiod/src/line_info.rs @@ -0,0 +1,180 @@ +// SPDX-License-Identifier: Apache-2.0 AND BSD-3-Clause +// +// Copyright 2022 Linaro Ltd. All Rights Reserved. +// Viresh Kumar <viresh.kumar@linaro.org> + +use libc::strlen; +use std::sync::Arc; +use std::time::Duration; +use std::{slice, str}; + +use vmm_sys_util::errno::Error as Errno; + +use super::{ + chip, gpiod, info, Bias, Direction, Drive, Edge, Error, EventClock, Offset, OperationType, + Result, +}; + +/// Line info +/// +/// Exposes functions for retrieving kernel information about both requested and +/// free lines. Line info object contains an immutable snapshot of a line's status. +/// +/// The line info contains all the publicly available information about a +/// line, which does not include the line value. The line must be requested +/// to access the line value. + +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct Info { + info: *mut gpiod::gpiod_line_info, + info_event: bool, +} + +impl Info { + /// Get a snapshot of information about the line and optionally start watching it for changes. + pub(crate) fn new(ichip: Arc<chip::Internal>, offset: Offset) -> Result<Self> { + let info = unsafe { gpiod::gpiod_chip_get_line_info(ichip.chip(), offset) }; + + if info.is_null() { + return Err(Error::OperationFailed( + OperationType::ChipGetLineInfo, + Errno::last(), + )); + } + + Ok(Self { + info, + info_event: false, + }) + } + + pub(crate) fn new_watch(ichip: Arc<chip::Internal>, offset: Offset) -> Result<Self> { + let info = unsafe { gpiod::gpiod_chip_watch_line_info(ichip.chip(), offset) }; + + if info.is_null() { + return Err(Error::OperationFailed( + OperationType::ChipWatchLineInfo, + Errno::last(), + )); + } + + Ok(Self { + info, + info_event: false, + }) + } + + /// Get the Line info object associated with an event. + pub(crate) fn new_from_event(event: &info::Event) -> Result<Self> { + let info = unsafe { gpiod::gpiod_info_event_get_line_info(event.event()) }; + + if info.is_null() { + return Err(Error::OperationFailed( + OperationType::InfoEventGetLineInfo, + Errno::last(), + )); + } + + Ok(Self { + info, + info_event: true, + }) + } + + /// Get the offset of the line within the GPIO chip. + /// + /// The offset uniquely identifies the line on the chip. The combination of the chip and offset + /// uniquely identifies the line within the system. + + pub fn offset(&self) -> Offset { + unsafe { gpiod::gpiod_line_info_get_offset(self.info) } + } + + /// Get GPIO line's name. + pub fn name(&self) -> Result<&str> { + // SAFETY: The string returned by libgpiod is guaranteed to live as long + // as the `struct Info`. + let name = unsafe { gpiod::gpiod_line_info_get_name(self.info) }; + if name.is_null() { + return Err(Error::NullString("GPIO line's name")); + } + + // SAFETY: The string is guaranteed to be valid here by the C API. + str::from_utf8(unsafe { slice::from_raw_parts(name as *const u8, strlen(name) as usize) }) + .map_err(Error::StringNotUtf8) + } + + /// Returns True if the line is in use, false otherwise. + /// + /// The user space can't know exactly why a line is busy. It may have been + /// requested by another process or hogged by the kernel. It only matters that + /// the line is used and we can't request it. + pub fn is_used(&self) -> bool { + unsafe { gpiod::gpiod_line_info_is_used(self.info) } + } + + /// Get the GPIO line's consumer name. + pub fn consumer(&self) -> Result<&str> { + // SAFETY: The string returned by libgpiod is guaranteed to live as long + // as the `struct Info`. + let name = unsafe { gpiod::gpiod_line_info_get_consumer(self.info) }; + if name.is_null() { + return Err(Error::NullString("GPIO line's consumer name")); + } + + // SAFETY: The string is guaranteed to be valid here by the C API. + str::from_utf8(unsafe { slice::from_raw_parts(name as *const u8, strlen(name) as usize) }) + .map_err(Error::StringNotUtf8) + } + + /// Get the GPIO line's direction. + pub fn direction(&self) -> Result<Direction> { + Direction::new(unsafe { gpiod::gpiod_line_info_get_direction(self.info) } as u32) + } + + /// Returns true if the line is "active-low", false otherwise. + pub fn is_active_low(&self) -> bool { + unsafe { gpiod::gpiod_line_info_is_active_low(self.info) } + } + + /// Get the GPIO line's bias setting. + pub fn bias(&self) -> Result<Option<Bias>> { + Bias::new(unsafe { gpiod::gpiod_line_info_get_bias(self.info) } as u32) + } + + /// Get the GPIO line's drive setting. + pub fn drive(&self) -> Result<Drive> { + Drive::new(unsafe { gpiod::gpiod_line_info_get_drive(self.info) } as u32) + } + + /// Get the current edge detection setting of the line. + pub fn edge_detection(&self) -> Result<Option<Edge>> { + Edge::new(unsafe { gpiod::gpiod_line_info_get_edge_detection(self.info) } as u32) + } + + /// Get the current event clock setting used for edge event timestamps. + pub fn event_clock(&self) -> Result<EventClock> { + EventClock::new(unsafe { gpiod::gpiod_line_info_get_event_clock(self.info) } as u32) + } + + /// Returns true if the line is debounced (either by hardware or by the + /// kernel software debouncer), false otherwise. + pub fn is_debounced(&self) -> bool { + unsafe { gpiod::gpiod_line_info_is_debounced(self.info) } + } + + /// Get the debounce period of the line. + pub fn debounce_period(&self) -> Duration { + Duration::from_micros(unsafe { gpiod::gpiod_line_info_get_debounce_period_us(self.info) }) + } +} + +impl Drop for Info { + fn drop(&mut self) { + // We must not free the Line info object created from `struct info::Event` by calling + // libgpiod API. + if !self.info_event { + unsafe { gpiod::gpiod_line_info_free(self.info) } + } + } +} diff --git a/bindings/rust/libgpiod/src/line_request.rs b/bindings/rust/libgpiod/src/line_request.rs new file mode 100644 index 000000000000..617efaa34d58 --- /dev/null +++ b/bindings/rust/libgpiod/src/line_request.rs @@ -0,0 +1,246 @@ +// SPDX-License-Identifier: Apache-2.0 AND BSD-3-Clause +// +// Copyright 2022 Linaro Ltd. All Rights Reserved. +// Viresh Kumar <viresh.kumar@linaro.org> + +use libc::EINVAL; +use std::os::raw::c_ulong; +use std::sync::Arc; +use std::time::Duration; + +use vmm_sys_util::errno::Error as Errno; + +use super::{ + chip, edge, gpiod, line, request, Error, Offset, OperationType, Result, Value, ValueMap, +}; + +/// Line request operations +/// +/// Allows interaction with a set of requested lines. +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct Request { + request: *mut gpiod::gpiod_line_request, +} + +impl Request { + /// Request a set of lines for exclusive usage. + pub(crate) fn new( + ichip: &Arc<chip::Internal>, + rconfig: &request::Config, + lconfig: &line::Config, + ) -> Result<Self> { + let request = unsafe { + gpiod::gpiod_chip_request_lines(ichip.chip(), rconfig.config(), lconfig.config()) + }; + + if request.is_null() { + return Err(Error::OperationFailed( + OperationType::LineRequest, + Errno::last(), + )); + } + + Ok(Self { request }) + } + + /// Get the number of lines in the request. + pub fn num_lines(&self) -> usize { + unsafe { gpiod::gpiod_line_request_get_num_lines(self.request) as usize } + } + + /// Get the offsets of lines in the request. + pub fn offsets(&self) -> Vec<Offset> { + let mut offsets = vec![0; self.num_lines() as usize]; + + unsafe { gpiod::gpiod_line_request_get_offsets(self.request, offsets.as_mut_ptr()) }; + offsets + } + + /// Get the value (0 or 1) of a single line associated with the request. + pub fn value(&self, offset: Offset) -> Result<Value> { + let value = unsafe { gpiod::gpiod_line_request_get_value(self.request, offset) }; + + if value != 0 && value != 1 { + Err(Error::OperationFailed( + OperationType::LineRequestGetVal, + Errno::last(), + )) + } else { + Value::new(value) + } + } + + /// Get values of a subset of lines associated with the request. + pub fn values_subset(&self, offsets: &[Offset]) -> Result<ValueMap> { + let mut values = vec![0; offsets.len()]; + + let ret = unsafe { + gpiod::gpiod_line_request_get_values_subset( + self.request, + offsets.len() as c_ulong, + offsets.as_ptr(), + values.as_mut_ptr(), + ) + }; + + if ret == -1 { + Err(Error::OperationFailed( + OperationType::LineRequestGetValSubset, + Errno::last(), + )) + } else { + let mut map = ValueMap::new(); + + for (i, val) in values.iter().enumerate() { + map.insert(offsets[i].into(), Value::new(*val)?); + } + + Ok(map) + } + } + + /// Get values of all lines associated with the request. + pub fn values(&self) -> Result<ValueMap> { + self.values_subset(&self.offsets()) + } + + /// Set the value of a single line associated with the request. + pub fn set_value(&self, offset: Offset, value: Value) -> Result<()> { + let ret = + unsafe { gpiod::gpiod_line_request_set_value(self.request, offset, value.value()) }; + + if ret == -1 { + Err(Error::OperationFailed( + OperationType::LineRequestSetVal, + Errno::last(), + )) + } else { + Ok(()) + } + } + + /// Get values of a subset of lines associated with the request. + pub fn set_values_subset(&self, map: ValueMap) -> Result<()> { + let mut offsets = Vec::new(); + let mut values = Vec::new(); + + for (offset, value) in map { + offsets.push(offset as u32); + values.push(value.value()); + } + + let ret = unsafe { + gpiod::gpiod_line_request_set_values_subset( + self.request, + offsets.len() as c_ulong, + offsets.as_ptr(), + values.as_ptr(), + ) + }; + + if ret == -1 { + Err(Error::OperationFailed( + OperationType::LineRequestSetValSubset, + Errno::last(), + )) + } else { + Ok(()) + } + } + + /// Get values of all lines associated with the request. + pub fn set_values(&self, values: &[Value]) -> Result<()> { + if values.len() != self.num_lines() as usize { + return Err(Error::OperationFailed( + OperationType::LineRequestSetVal, + Errno::new(EINVAL), + )); + } + + let mut new_values = Vec::new(); + for value in values { + new_values.push(value.value()); + } + + let ret = + unsafe { gpiod::gpiod_line_request_set_values(self.request, new_values.as_ptr()) }; + + if ret == -1 { + Err(Error::OperationFailed( + OperationType::LineRequestSetVal, + Errno::last(), + )) + } else { + Ok(()) + } + } + + /// Update the configuration of lines associated with the line request. + pub fn reconfigure_lines(&self, lconfig: &line::Config) -> Result<()> { + let ret = + unsafe { gpiod::gpiod_line_request_reconfigure_lines(self.request, lconfig.config()) }; + + if ret == -1 { + Err(Error::OperationFailed( + OperationType::LineRequestReconfigLines, + Errno::last(), + )) + } else { + Ok(()) + } + } + + /// Get the file descriptor associated with the line request. + pub fn fd(&self) -> u32 { + unsafe { gpiod::gpiod_line_request_get_fd(self.request) as u32 } + } + + /// Wait for edge events on any of the lines associated with the request. + pub fn wait_edge_event(&self, timeout: Option<Duration>) -> Result<bool> { + let timeout = match timeout { + Some(x) => x.as_nanos() as i64, + // Block indefinitely + None => -1, + }; + + let ret = unsafe { gpiod::gpiod_line_request_wait_edge_event(self.request, timeout) }; + + match ret { + -1 => Err(Error::OperationFailed( + OperationType::LineRequestWaitEdgeEvent, + Errno::last(), + )), + 0 => Ok(false), + _ => Ok(true), + } + } + + /// Get a number of edge events from a line request. + /// + /// This function will block if no event was queued for the line. + pub fn read_edge_events(&self, buffer: &edge::event::Buffer) -> Result<u32> { + let ret = unsafe { + gpiod::gpiod_line_request_read_edge_event( + self.request, + buffer.buffer(), + buffer.capacity() as u64, + ) + }; + + if ret == -1 { + Err(Error::OperationFailed( + OperationType::LineRequestReadEdgeEvent, + Errno::last(), + )) + } else { + Ok(ret as u32) + } + } +} + +impl Drop for Request { + /// Release the requested lines and free all associated resources. + fn drop(&mut self) { + unsafe { gpiod::gpiod_line_request_release(self.request) } + } +} diff --git a/bindings/rust/libgpiod/src/line_settings.rs b/bindings/rust/libgpiod/src/line_settings.rs new file mode 100644 index 000000000000..2c3090132ea5 --- /dev/null +++ b/bindings/rust/libgpiod/src/line_settings.rs @@ -0,0 +1,277 @@ +// SPDX-License-Identifier: Apache-2.0 AND BSD-3-Clause +// +// Copyright 2022 Linaro Ltd. All Rights Reserved. +// Viresh Kumar <viresh.kumar@linaro.org> + +use std::time::Duration; + +use vmm_sys_util::errno::Error as Errno; + +use super::{ + gpiod, Bias, Direction, Drive, Edge, Error, EventClock, OperationType, Result, SettingKind, + SettingVal, Value, +}; + +/// Line settings objects. +/// +/// Line settings object contains a set of line properties that can be used +/// when requesting lines or reconfiguring an existing request. +/// +/// Mutators in general can only fail if the new property value is invalid. The +/// return values can be safely ignored - the object remains valid even after +/// a mutator fails and simply uses the sane default appropriate for given +/// property. + +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct Settings { + settings: *mut gpiod::gpiod_line_settings, +} + +impl Settings { + /// Create a new line settings object. + pub fn new() -> Result<Self> { + let settings = unsafe { gpiod::gpiod_line_settings_new() }; + + if settings.is_null() { + return Err(Error::OperationFailed( + OperationType::LineSettingsNew, + Errno::last(), + )); + } + + Ok(Self { settings }) + } + + pub fn new_with_settings(settings: *mut gpiod::gpiod_line_settings) -> Self { + Self { settings } + } + + /// Private helper, Returns gpiod_line_settings + pub(crate) fn settings(&self) -> *mut gpiod::gpiod_line_settings { + self.settings + } + + /// Resets the line settings object to its default values. + pub fn reset(&mut self) { + unsafe { gpiod::gpiod_line_settings_reset(self.settings) } + } + + /// Makes copy of the settings object. + pub fn settings_clone(&self) -> Result<Self> { + let settings = unsafe { gpiod::gpiod_line_settings_copy(self.settings) }; + if settings.is_null() { + return Err(Error::OperationFailed( + OperationType::LineSettingsCopy, + Errno::last(), + )); + } + + Ok(Self { settings }) + } + + /// Set line prop setting. + pub fn set_prop(&mut self, values: &[SettingVal]) -> Result<()> { + for value in values { + match value { + SettingVal::Direction(val) => self.set_direction(*val)?, + SettingVal::EdgeDetection(val) => self.set_edge_detection(*val)?, + SettingVal::Bias(val) => self.set_bias(*val)?, + SettingVal::Drive(val) => self.set_drive(*val)?, + SettingVal::ActiveLow(val) => self.set_active_low(*val), + SettingVal::DebouncePeriod(val) => self.set_debounce_period(*val), + SettingVal::EventClock(val) => self.set_event_clock(*val)?, + SettingVal::OutputValue(val) => self.set_output_value(*val)?, + } + } + + Ok(()) + } + + /// Get the line prop setting. + pub fn prop(&self, property: SettingKind) -> Result<SettingVal> { + Ok(match property { + SettingKind::Direction => SettingVal::Direction(self.direction()?), + SettingKind::EdgeDetection => SettingVal::EdgeDetection(self.edge_detection()?), + SettingKind::Bias => SettingVal::Bias(self.bias()?), + SettingKind::Drive => SettingVal::Drive(self.drive()?), + SettingKind::ActiveLow => SettingVal::ActiveLow(self.active_low()), + SettingKind::DebouncePeriod => SettingVal::DebouncePeriod(self.debounce_period()?), + SettingKind::EventClock => SettingVal::EventClock(self.event_clock()?), + SettingKind::OutputValue => SettingVal::OutputValue(self.output_value()?), + }) + } + + /// Set the line direction. + fn set_direction(&mut self, direction: Direction) -> Result<()> { + let ret = unsafe { + gpiod::gpiod_line_settings_set_direction( + self.settings, + direction.gpiod_direction() as i32, + ) + }; + + if ret == -1 { + Err(Error::OperationFailed( + OperationType::LineSettingsSetDirection, + Errno::last(), + )) + } else { + Ok(()) + } + } + + /// Get the direction setting. + fn direction(&self) -> Result<Direction> { + Direction::new(unsafe { gpiod::gpiod_line_settings_get_direction(self.settings) } as u32) + } + + /// Set the edge event detection setting. + fn set_edge_detection(&mut self, edge: Option<Edge>) -> Result<()> { + let ret = unsafe { + gpiod::gpiod_line_settings_set_edge_detection( + self.settings, + Edge::gpiod_edge(edge) as i32, + ) + }; + + if ret == -1 { + Err(Error::OperationFailed( + OperationType::LineSettingsSetEdgeDetection, + Errno::last(), + )) + } else { + Ok(()) + } + } + + /// Get the edge event detection setting. + fn edge_detection(&self) -> Result<Option<Edge>> { + Edge::new(unsafe { gpiod::gpiod_line_settings_get_edge_detection(self.settings) } as u32) + } + + /// Set the bias setting. + fn set_bias(&mut self, bias: Option<Bias>) -> Result<()> { + let ret = unsafe { + gpiod::gpiod_line_settings_set_bias(self.settings, Bias::gpiod_bias(bias) as i32) + }; + + if ret == -1 { + Err(Error::OperationFailed( + OperationType::LineSettingsSetBias, + Errno::last(), + )) + } else { + Ok(()) + } + } + + /// Get the bias setting. + fn bias(&self) -> Result<Option<Bias>> { + Bias::new(unsafe { gpiod::gpiod_line_settings_get_bias(self.settings) } as u32) + } + + /// Set the drive setting. + fn set_drive(&mut self, drive: Drive) -> Result<()> { + let ret = unsafe { + gpiod::gpiod_line_settings_set_drive(self.settings, drive.gpiod_drive() as i32) + }; + + if ret == -1 { + Err(Error::OperationFailed( + OperationType::LineSettingsSetDrive, + Errno::last(), + )) + } else { + Ok(()) + } + } + + /// Get the drive setting. + fn drive(&self) -> Result<Drive> { + Drive::new(unsafe { gpiod::gpiod_line_settings_get_drive(self.settings) } as u32) + } + + /// Set active-low setting. + fn set_active_low(&mut self, active_low: bool) { + unsafe { gpiod::gpiod_line_settings_set_active_low(self.settings, active_low) } + } + + /// Check the active-low setting. + fn active_low(&self) -> bool { + unsafe { gpiod::gpiod_line_settings_get_active_low(self.settings) } + } + + /// Set the debounce period setting. + fn set_debounce_period(&mut self, period: Duration) { + unsafe { + gpiod::gpiod_line_settings_set_debounce_period_us( + self.settings, + period.as_micros() as u64, + ) + } + } + + /// Get the debounce period. + fn debounce_period(&self) -> Result<Duration> { + Ok(Duration::from_micros(unsafe { + gpiod::gpiod_line_settings_get_debounce_period_us(self.settings) + })) + } + + /// Set the event clock setting. + fn set_event_clock(&mut self, clock: EventClock) -> Result<()> { + let ret = unsafe { + gpiod::gpiod_line_settings_set_event_clock(self.settings, clock.gpiod_clock() as i32) + }; + + if ret == -1 { + Err(Error::OperationFailed( + OperationType::LineSettingsSetEventClock, + Errno::last(), + )) + } else { + Ok(()) + } + } + + /// Get the event clock setting. + fn event_clock(&self) -> Result<EventClock> { + EventClock::new(unsafe { gpiod::gpiod_line_settings_get_event_clock(self.settings) } as u32) + } + + /// Set the output value setting. + fn set_output_value(&mut self, value: Value) -> Result<()> { + let ret = + unsafe { gpiod::gpiod_line_settings_set_output_value(self.settings, value.value()) }; + + if ret == -1 { + Err(Error::OperationFailed( + OperationType::LineSettingsSetOutputValue, + Errno::last(), + )) + } else { + Ok(()) + } + } + + /// Get the output value, 0 or 1. + fn output_value(&self) -> Result<Value> { + let value = unsafe { gpiod::gpiod_line_settings_get_output_value(self.settings) }; + + if value != 0 && value != 1 { + Err(Error::OperationFailed( + OperationType::LineSettingsGetOutVal, + Errno::last(), + )) + } else { + Value::new(value) + } + } +} + +impl Drop for Settings { + /// Free the line settings object and release all associated resources. + fn drop(&mut self) { + unsafe { gpiod::gpiod_line_settings_free(self.settings) } + } +} diff --git a/bindings/rust/libgpiod/src/request_config.rs b/bindings/rust/libgpiod/src/request_config.rs new file mode 100644 index 000000000000..760d9c755c86 --- /dev/null +++ b/bindings/rust/libgpiod/src/request_config.rs @@ -0,0 +1,96 @@ +// SPDX-License-Identifier: Apache-2.0 AND BSD-3-Clause +// +// Copyright 2022 Linaro Ltd. All Rights Reserved. +// Viresh Kumar <viresh.kumar@linaro.org> + +use libc::strlen; +use std::os::raw::{c_char, c_ulong}; +use std::{slice, str}; + +use vmm_sys_util::errno::Error as Errno; + +use super::{gpiod, Error, OperationType, Result}; + +/// Request configuration objects +/// +/// Request config objects are used to pass a set of options to the kernel at +/// the time of the line request. The mutators don't return error values. If the +/// values are invalid, in general they are silently adjusted to acceptable +/// ranges. + +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct Config { + config: *mut gpiod::gpiod_request_config, +} + +impl Config { + /// Create a new request config object. + pub fn new() -> Result<Self> { + let config = unsafe { gpiod::gpiod_request_config_new() }; + if config.is_null() { + return Err(Error::OperationFailed( + OperationType::RequestConfigNew, + Errno::last(), + )); + } + + Ok(Self { config }) + } + + /// Private helper, Returns gpiod_request_config + pub(crate) fn config(&self) -> *mut gpiod::gpiod_request_config { + self.config + } + + /// Set the consumer name for the request. + /// + /// If the consumer string is too long, it will be truncated to the max + /// accepted length. + pub fn set_consumer(&self, consumer: &str) { + // Null-terminate the string + let consumer = consumer.to_owned() + "\0"; + + unsafe { + gpiod::gpiod_request_config_set_consumer( + self.config, + consumer.as_ptr() as *const c_char, + ) + } + } + + /// Get the consumer name configured in the request config. + pub fn consumer(&self) -> Result<&str> { + // SAFETY: The string returned by libgpiod is guaranteed to live as long + // as the `struct Config`. + let consumer = unsafe { gpiod::gpiod_request_config_get_consumer(self.config) }; + if consumer.is_null() { + return Err(Error::OperationFailed( + OperationType::RequestConfigGetConsumer, + Errno::last(), + )); + } + + // SAFETY: The string is guaranteed to be valid here by the C API. + str::from_utf8(unsafe { + slice::from_raw_parts(consumer as *const u8, strlen(consumer) as usize) + }) + .map_err(Error::StringNotUtf8) + } + + /// Set the size of the kernel event buffer for the request. + pub fn set_event_buffer_size(&self, size: usize) { + unsafe { gpiod::gpiod_request_config_set_event_buffer_size(self.config, size as c_ulong) } + } + + /// Get the edge event buffer size setting for the request config. + pub fn event_buffer_size(&self) -> usize { + unsafe { gpiod::gpiod_request_config_get_event_buffer_size(self.config) as usize } + } +} + +impl Drop for Config { + /// Free the request config object and release all associated resources. + fn drop(&mut self) { + unsafe { gpiod::gpiod_request_config_free(self.config) } + } +}
Add rust wrapper crate, around the libpiod-sys crate added earlier, to provide a convenient interface for the users. Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> --- bindings/rust/Cargo.toml | 1 + bindings/rust/libgpiod/Cargo.toml | 13 + bindings/rust/libgpiod/src/chip.rs | 253 ++++++++++ bindings/rust/libgpiod/src/edge_event.rs | 102 ++++ bindings/rust/libgpiod/src/event_buffer.rs | 90 ++++ bindings/rust/libgpiod/src/info_event.rs | 68 +++ bindings/rust/libgpiod/src/lib.rs | 471 +++++++++++++++++++ bindings/rust/libgpiod/src/line_config.rs | 118 +++++ bindings/rust/libgpiod/src/line_info.rs | 180 +++++++ bindings/rust/libgpiod/src/line_request.rs | 246 ++++++++++ bindings/rust/libgpiod/src/line_settings.rs | 277 +++++++++++ bindings/rust/libgpiod/src/request_config.rs | 96 ++++ 12 files changed, 1915 insertions(+) create mode 100644 bindings/rust/libgpiod/Cargo.toml create mode 100644 bindings/rust/libgpiod/src/chip.rs create mode 100644 bindings/rust/libgpiod/src/edge_event.rs create mode 100644 bindings/rust/libgpiod/src/event_buffer.rs create mode 100644 bindings/rust/libgpiod/src/info_event.rs create mode 100644 bindings/rust/libgpiod/src/lib.rs create mode 100644 bindings/rust/libgpiod/src/line_config.rs create mode 100644 bindings/rust/libgpiod/src/line_info.rs create mode 100644 bindings/rust/libgpiod/src/line_request.rs create mode 100644 bindings/rust/libgpiod/src/line_settings.rs create mode 100644 bindings/rust/libgpiod/src/request_config.rs