More Rust bindings for device property reads
Subsystems (3)
v5 cover More Rust bindings for device property reads
changes in v5:
* Move rust/kernel/device/mod.rs back to rust/kernel/device.rs.
* Reword some commit messages.
* Leave property_present on Device for now to make merging easier.
* Cleanup Rust platform driver sample.
* Fix conflict with alloc-next tree.
* Improve documentation and safety comments.
* Seal the traits Property and PropertyInt.
* Move more logic into the PropertyInt trait, making its methods safe.
Best regards,
Remo
Remo Senekowitsch (9):
rust: device: Create FwNode abstraction for accessing device
properties
rust: device: Enable accessing the FwNode of a Device
rust: device: Add property_present() to FwNode
rust: device: Enable printing fwnode name and path
rust: device: Introduce PropertyGuard
rust: device: Implement accessors for firmware properties
rust: device: Add child accessor and iterator
rust: device: Add property_get_reference_args
samples: rust: platform: Add property read examples
MAINTAINERS | 1 + drivers/of/unittest-data/tests-platform.dtsi | 3 + rust/helpers/helpers.c | 1 + rust/helpers/property.c | 8 + rust/kernel/device.rs | 17 + rust/kernel/device/property.rs | 578 +++++++++++++++++++ samples/rust/rust_driver_platform.rs | 60 +- 7 files changed, 667 insertions(+), 1 deletion(-) create mode 100644 rust/helpers/property.c create mode 100644 rust/kernel/device/property.rs
Signature · 1 line
2.49.0
PATCH v5 1/9 rust: device: Create FwNode abstraction for accessing device properties
Accessing device properties is currently done via methods on Device itself, using bindings to device_property_* functions. This is sufficient for the existing method property_present. However, it's not sufficient for other device properties we want to access. For example, iterating over child nodes of a device will yield a fwnode_handle. That's not a device, so it wouldn't be possible to read the properties of that child node. Thus, we need an abstraction over fwnode_handle and methods for reading its properties.
Add a struct FwNode which abstracts over the C struct fwnode_handle. Implement its reference counting analogous to other Rust abstractions over reference-counted C structs.
Subsequent patches will add functionality to access FwNode and read properties with it.
--- MAINTAINERS | 1 + rust/helpers/helpers.c | 1 + rust/helpers/property.c | 8 ++++++ rust/kernel/device.rs | 2 ++ rust/kernel/device/property.rs | 47 ++++++++++++++++++++++++++++++++++ 5 files changed, 59 insertions(+) create mode 100644 rust/helpers/property.c create mode 100644 rust/kernel/device/property.rs
MAINTAINERS +1 -0
@@ -7248,6 +7248,7 @@ F: include/linux/property.h F: include/linux/sysfs.h F: lib/kobj* F: rust/kernel/device.rs +F: rust/kernel/device/ F: rust/kernel/device_id.rs F: rust/kernel/devres.rs F: rust/kernel/driver.rs
rust/helpers/helpers.c +1 -0
@@ -25,6 +25,7 @@#include "platform.c" #include "pci.c" #include "pid_namespace.c" +#include "property.c" #include "rbtree.c" #include "rcu.c" #include "refcount.c"
rust/helpers/property.c new file +8 -0
@@ -0,0 +1,8 @@ +// SPDX-License-Identifier: GPL-2.0 + +#include <linux/property.h> + +void rust_helper_fwnode_handle_put (struct fwnode_handle *fwnode) +{ +fwnode_handle_put (fwnode); +}
rust/kernel/device.rs +2 -0
@@ -14,6 +14,8 @@#[cfg( CONFIG_PRINTK )] use crate ::c_str; +pub mod property; +/// A reference-counted device. /// /// This structure represents the Rust abstraction for a C `struct device`. This implementation
rust/kernel/device/property.rs new file +47 -0
47 lines not on this page · raw message
Signature · 1 line
2.49.0
PATCH v5 2/9 rust: device: Enable accessing the FwNode of a Device
In reply to Remo SenekowitschSubsequent patches will add methods for reading properties to FwNode. The first step to accessing these methods will be to access the "root" FwNode of a Device.
Add the method fwnode to Device.
--- rust/kernel/device.rs | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
rust/kernel/device.rs +15 -0
@@ -186,6 +186,21 @@ unsafe fn printk(&self, klevel: &[u8], msg: fmt::Arguments<'_>) { }; } +/// Obtain the [`FwNode`](property::FwNode) corresponding to the device. +pub fn fwnode (&self) ->Option <&property::FwNode > { +// SAFETY: `self` is valid. +let fwnode_handle =unsafe { bindings::__dev_fwnode (self.as_raw ()) }; +if fwnode_handle.is_null () { +return None ; + } +// SAFETY: `fwnode_handle` is valid. Its lifetime is tied to `&self`. We +// return a reference instead of an `ARef<FwNode>` because `dev_fwnode()` +// doesn't increment the refcount. It is safe to cast from a +// `struct fwnode_handle*` to a `*const FwNode` because `FwNode` is +// defined as a `#[repr(transparent)]` wrapper around `fwnode_handle`. +Some (unsafe { &*fwnode_handle.cast () }) + } +/// Checks if property is present or not. pub fn property_present (&self, name: &CStr ) ->bool {// SAFETY: By the invariant of `CStr`, `name` is null-terminated.
Signature · 1 line
2.49.0
Greg Kroah-Hartman
maintainerRe: [PATCH v5 2/9] rust: device: Enable accessing the FwNode of a Device
Quoted 16 lines · Remo Senekowitsch
Subsequent patches will add methods for reading properties to FwNode. The first step to accessing these methods will be to access the "root" FwNode of a Device.
Add the method
fwnodetoDevice.Signed-off-by Remo Senekowitsch <[email protected]>--- rust/kernel/device.rs | 15 +++++++++++++++ 1 file changed, 15 insertions(+)rust/kernel/device.rs +0
-0
rust/kernel/device.rs unsafe fn printk(&self, klevel: &[u8], msg: fmt::Arguments<'_>) { · line 192
Quoted 7 lines
rust/kernel/device.rs +4
-0@@ -186,6 +186,21 @@ unsafe fn printk(&self, klevel: &[u8], msg: fmt::Arguments<'_>) { }; } +/// Obtain the [`FwNode`](property::FwNode) corresponding to the device. +pub fn fwnode (&self) ->Option <&property::FwNode > { +// SAFETY: `self` is valid. +let fwnode_handle =unsafe { bindings::__dev_fwnode (self.as_raw ()) };
Why isn't this calling __dev_fwnode_const()? And there's no way to just use dev_fwnode() directly? Ugh, it's a macro...
thanks,
greg k-h
Re: [PATCH v5 2/9] rust: device: Enable accessing the FwNode of a Device
Quoted 20 lines · Greg Kroah-Hartman
Quoted 19 lines · Remo Senekowitsch
Subsequent patches will add methods for reading properties to FwNode. The first step to accessing these methods will be to access the "root" FwNode of a Device.
Add the method
fwnodetoDevice.Signed-off-by Remo Senekowitsch <[email protected]>--- rust/kernel/device.rs | 15 +++++++++++++++ 1 file changed, 15 insertions(+)rust/kernel/device.rs +0
-0@@ -186,6 +186,21 @@ unsafe fn printk(&self, klevel: &[u8], msg: fmt::Arguments<'_>) { }; }
Quoted 7 lines
rust/kernel/device.rs unsafe fn printk(&self, klevel: &[u8], msg: fmt::Arguments<'_>) { · line 192
Quoted 4 lines
rust/kernel/device.rs +4
-0+/// Obtain the [`FwNode`](property::FwNode) corresponding to the device. +pub fn fwnode (&self) ->Option <&property::FwNode > { +// SAFETY: `self` is valid. +let fwnode_handle =unsafe { bindings::__dev_fwnode (self.as_raw ()) };Why isn't this calling
__dev_fwnode_const()? And there's no way to just usedev_fwnode()directly? Ugh, it's a macro...
I think I had that in earlier versions of the patch series. There was a helper to call the macro from Rust. It was pointed out that this is not clearly expressing the intent - whether fwnode is mutated or not. And I think someone said the fwnode can be mutated so __dev_fwnode() is semantically correct.
PATCH v5 3/9 rust: device: Add property_present() to FwNode
In reply to Remo SenekowitschThe new FwNode abstraction will be used for accessing all device properties, so it must have the property_present method.
It's possible to duplicate the methods on the device itself, but since some of the methods on Device would have different type sigatures as the ones on FwNode, this would only lead to inconsistency and confusion. So, in the future, property_perent will be removed from Device. However, there's a user about to be merged, so the method is left to make merging easier.
--- rust/kernel/device/property.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
rust/kernel/device/property.rs +7 -1
@@ -6,7 +6,7 @@use core::ptr; -use crate ::{bindings, types::Opaque }; +use crate ::{bindings, str::CStr , types::Opaque };/// A reference-counted fwnode_handle. ///
@@ -31,6 +31,12 @@ impl FwNode {pub (crate )fn as_raw (&self) -> *mut bindings::fwnode_handle { self.0 .get () } + +/// Checks if property is present or not. +pub fn property_present (&self, name: &CStr ) ->bool { +// SAFETY: By the invariant of `CStr`, `name` is null-terminated. +unsafe { bindings::fwnode_property_present (self.as_raw ().cast_const (), name.as_char_ptr ()) } + } }// SAFETY: Instances of `FwNode` are always reference-counted.
Signature · 1 line
2.49.0
PATCH v5 4/9 rust: device: Enable printing fwnode name and path
In reply to Remo SenekowitschAdd two new public methods display_name and display_path to FwNode. They can be used by driver authors for logging purposes. In addition, they will be used by core property abstractions for automatic logging, for example when a driver attempts to read a required but missing property.
--- rust/kernel/device/property.rs | 72 ++++++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+)
rust/kernel/device/property.rs +72 -0
@@ -32,6 +32,78 @@ pub(crate) fn as_raw(&self) -> *mut bindings::fwnode_handle { self.0 .get () } +/// Returns an object that implements [`Display`](core::fmt::Display) for +/// printing the name of a node. +pub fn display_name (&self) ->impl core::fmt::Display + '_ { +struct FwNodeDisplayName <'a>(&'aFwNode ); + +impl core::fmt::Display for FwNodeDisplayName <'_> { +fn fmt (&self, f: &mut core::fmt::Formatter <'_>) -> core::fmt::Result { +// SAFETY: self is valid by its type invariant +let name =unsafe { bindings::fwnode_get_name (self.0 .as_raw ()) }; +if name.is_null () { +return Ok (()); + } +// SAFETY: fwnode_get_name returns null or a valid C string and +// name is not null +let name =unsafe {CStr ::from_char_ptr (name) }; +write! (f,"{name}" ) + } + } + +FwNodeDisplayName (self) + } + +/// Returns an object that implements [`Display`](core::fmt::Display) for +/// printing the full path of a node. +pub fn display_path (&self) ->impl core::fmt::Display + '_ { +struct FwNodeDisplayPath <'a>(&'aFwNode ); + +impl core::fmt::Display for FwNodeDisplayPath <'_> { +fn fmt (&self, f: &mut core::fmt::Formatter <'_>) -> core::fmt::Result { +// The logic here is the same as the one in lib/vsprintf.c +// (fwnode_full_name_string). + +// SAFETY: `self.0.as_raw()` is valid by its type invariant +let num_parents =unsafe { bindings::fwnode_count_parents (self.0 .as_raw ()) }; + +for depthin (0 ..=num_parents).rev () { +let fwnode =if depth ==0 { + self.0 .as_raw () + }else { +// SAFETY: `self.0.as_raw()` is valid +unsafe { bindings::fwnode_get_nth_parent (self.0 .as_raw (), depth) } + }; + +// SAFETY: fwnode is valid, it is either `self.0.as_raw()` or +// the return value of `bindings::fwnode_get_nth_parent` which +// returns a valid pointer to a fwnode_handle if the provided +// depth is within the valid range, which we know to be true. +let prefix =unsafe { bindings::fwnode_get_name_prefix (fwnode) }; +if !prefix.is_null () { +// SAFETY: fwnode_get_name_prefix returns null or a +// valid C string +let prefix =unsafe {CStr ::from_char_ptr (prefix) }; +write! (f,"{prefix}" )?; + } +write! (f,"{}" , self.0 .display_name ())?; + +if depth !=0 { +// SAFETY: `fwnode` is valid, because `depth` is +// a valid depth of a parent of `self.0.as_raw()`. +// `fwnode_get_nth_parent` increments the refcount and +// we are responsible to decrement it. +unsafe { bindings::fwnode_handle_put (fwnode) } + } + } + +Ok (()) + } + } + +FwNodeDisplayPath (self) + } +/// Checks if property is present or not. pub fn property_present (&self, name: &CStr ) ->bool {// SAFETY: By the invariant of `CStr`, `name` is null-terminated.
Signature · 1 line
2.49.0
Greg Kroah-Hartman
maintainerRe: [PATCH v5 4/9] rust: device: Enable printing fwnode name and path
Quoted 20 lines · Remo Senekowitsch
Add two new public methods
display_nameanddisplay_pathtoFwNode. They can be used by driver authors for logging purposes. In addition, they will be used by core property abstractions for automatic logging, for example when a driver attempts to read a required but missing property.Signed-off-by Remo Senekowitsch <[email protected]>--- rust/kernel/device/property.rs | 72 ++++++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+)rust/kernel/device/property.rs +1
-0@@ -32,6 +32,78 @@ pub(crate) fn as_raw(&self) -> *mut bindings::fwnode_handle { self.0 .get () } +/// Returns an object that implements [`Display`](core::fmt::Display) for
rust/kernel/device/property.rs pub(crate) fn as_raw(&self) -> *mut bindings::fwnode_handle { · line 45
Quoted 10 lines
rust/kernel/device/property.rs +10
-0+/// printing the name of a node. +pub fn display_name (&self) ->impl core::fmt::Display + '_ { +struct FwNodeDisplayName <'a>(&'aFwNode ); + +impl core::fmt::Display for FwNodeDisplayName <'_> { +fn fmt (&self, f: &mut core::fmt::Formatter <'_>) -> core::fmt::Result { +// SAFETY: self is valid by its type invariant +let name =unsafe { bindings::fwnode_get_name (self.0 .as_raw ()) }; +if name.is_null () { +return Ok (());
So if there is no name, you are returning Ok()? Are you sure that's ok to do? What will the result of the string look like then?
thanks,
greg k-h
Re: [PATCH v5 4/9] rust: device: Enable printing fwnode name and path
Quoted 20 lines · Greg Kroah-Hartman
Quoted 19 lines · Remo Senekowitsch
Add two new public methods
display_nameanddisplay_pathtoFwNode. They can be used by driver authors for logging purposes. In addition, they will be used by core property abstractions for automatic logging, for example when a driver attempts to read a required but missing property.Signed-off-by Remo Senekowitsch <[email protected]>--- rust/kernel/device/property.rs | 72 ++++++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+)rust/kernel/device/property.rs +0
-0@@ -32,6 +32,78 @@ pub(crate) fn as_raw(&self) -> *mut bindings::fwnode_handle { self.0 .get () }
Quoted 14 lines
rust/kernel/device/property.rs pub(crate) fn as_raw(&self) -> *mut bindings::fwnode_handle { · line 45
Quoted 11 lines
rust/kernel/device/property.rs +11
-0+/// Returns an object that implements [`Display`](core::fmt::Display) for +/// printing the name of a node. +pub fn display_name (&self) ->impl core::fmt::Display + '_ { +struct FwNodeDisplayName <'a>(&'aFwNode ); + +impl core::fmt::Display for FwNodeDisplayName <'_> { +fn fmt (&self, f: &mut core::fmt::Formatter <'_>) -> core::fmt::Result { +// SAFETY: self is valid by its type invariant +let name =unsafe { bindings::fwnode_get_name (self.0 .as_raw ()) }; +if name.is_null () { +return Ok (());So if there is no name, you are returning
Ok()? Are you sure that's ok to do? What will the result of the string look like then?
In that case we're not writing anything to the formatter, which is equivalent to an empty string. Ok(()) means that writing succeeded.
I assumed that a valid node would always have a name. And we're guaranteed to have a valid node. So I assumed this case would never happen and didn't think too hard about it. But even if a valid node has not name, empty string is probably the correct thing, right?
Greg Kroah-Hartman
maintainerRe: [PATCH v5 4/9] rust: device: Enable printing fwnode name and path
Quoted 36 lines · Remo Senekowitsch
Quoted 20 lines · Greg Kroah-Hartman
Quoted 19 lines · Remo Senekowitsch
Add two new public methods
display_nameanddisplay_pathtoFwNode. They can be used by driver authors for logging purposes. In addition, they will be used by core property abstractions for automatic logging, for example when a driver attempts to read a required but missing property.Signed-off-by Remo Senekowitsch <[email protected]>--- rust/kernel/device/property.rs | 72 ++++++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+)rust/kernel/device/property.rs +0
-0@@ -32,6 +32,78 @@ pub(crate) fn as_raw(&self) -> *mut bindings::fwnode_handle { self.0 .get () }Quoted 14 lines
rust/kernel/device/property.rs pub(crate) fn as_raw(&self) -> *mut bindings::fwnode_handle { · line 45
Quoted 11 lines
rust/kernel/device/property.rs +11
-0+/// Returns an object that implements [`Display`](core::fmt::Display) for +/// printing the name of a node. +pub fn display_name (&self) ->impl core::fmt::Display + '_ { +struct FwNodeDisplayName <'a>(&'aFwNode ); + +impl core::fmt::Display for FwNodeDisplayName <'_> { +fn fmt (&self, f: &mut core::fmt::Formatter <'_>) -> core::fmt::Result { +// SAFETY: self is valid by its type invariant +let name =unsafe { bindings::fwnode_get_name (self.0 .as_raw ()) }; +if name.is_null () { +return Ok (());So if there is no name, you are returning
Ok()? Are you sure that's ok to do? What will the result of the string look like then?
Quoted 7 lines
In that case we're not writing anything to the formatter, which is equivalent to an empty string.
Ok(())means that writing succeeded.I assumed that a valid node would always have a name. And we're guaranteed to have a valid node. So I assumed this case would never happen and didn't think too hard about it. But even if a valid node has not name, empty string is probably the correct thing, right?
I don't know what this "name" is used for. An empty string might not be what you want to use here, given that you could be naming something based on it, right? fwnode_get_name() is used for many things, including the detection if a name is not present at all, and if not, then the code needs to clean up and abort.
So what exactly are you going to be using this for?
thanks,
greg k-h
Re: [PATCH v5 4/9] rust: device: Enable printing fwnode name and path
Quoted 45 lines · Greg Kroah-Hartman
Quoted 36 lines · Remo Senekowitsch
Quoted 20 lines · Greg Kroah-Hartman
Quoted 19 lines · Remo Senekowitsch
Add two new public methods
display_nameanddisplay_pathtoFwNode. They can be used by driver authors for logging purposes. In addition, they will be used by core property abstractions for automatic logging, for example when a driver attempts to read a required but missing property.Signed-off-by Remo Senekowitsch <[email protected]>--- rust/kernel/device/property.rs | 72 ++++++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+)rust/kernel/device/property.rs +0
-0@@ -32,6 +32,78 @@ pub(crate) fn as_raw(&self) -> *mut bindings::fwnode_handle { self.0 .get () }Quoted 14 lines
rust/kernel/device/property.rs pub(crate) fn as_raw(&self) -> *mut bindings::fwnode_handle { · line 45
Quoted 11 lines
rust/kernel/device/property.rs +11
-0+/// Returns an object that implements [`Display`](core::fmt::Display) for +/// printing the name of a node. +pub fn display_name (&self) ->impl core::fmt::Display + '_ { +struct FwNodeDisplayName <'a>(&'aFwNode ); + +impl core::fmt::Display for FwNodeDisplayName <'_> { +fn fmt (&self, f: &mut core::fmt::Formatter <'_>) -> core::fmt::Result { +// SAFETY: self is valid by its type invariant +let name =unsafe { bindings::fwnode_get_name (self.0 .as_raw ()) }; +if name.is_null () { +return Ok (());So if there is no name, you are returning
Ok()? Are you sure that's ok to do? What will the result of the string look like then?Quoted 7 lines
In that case we're not writing anything to the formatter, which is equivalent to an empty string.
Ok(())means that writing succeeded.I assumed that a valid node would always have a name. And we're guaranteed to have a valid node. So I assumed this case would never happen and didn't think too hard about it. But even if a valid node has not name, empty string is probably the correct thing, right?
Quoted 7 lines
I don't know what this "name" is used for. An empty string might not be what you want to use here, given that you could be naming something based on it, right?
fwnode_get_name()is used for many things, including the detection if a name is not present at all, and if not, then the code needs to clean up and abort.So what exactly are you going to be using this for?
Valid question... I'm not using it for anything. I was trying to match the C API which has capabilities to print a fwnode's name and full path. I think the format specifiers are %pfwP and %pfwf if I'm reading the source correctly. It also looks to me like the C API for printing fwnode names does the same thing - print nothing if the name is null. [1] [2]
[1] https://elixir.bootlin.com/linux/v6.14.7/source/lib/vsprintf.c#L2242
[2] https://elixir.bootlin.com/linux/v6.14.7/source/lib/vsprintf.c#L711
Danilo Krummrich
maintainerRe: [PATCH v5 4/9] rust: device: Enable printing fwnode name and path
Quoted 52 lines · Remo Senekowitsch
Quoted 51 lines · Greg Kroah-Hartman
Quoted 36 lines · Remo Senekowitsch
Quoted 20 lines · Greg Kroah-Hartman
Quoted 19 lines · Remo Senekowitsch
Add two new public methods
display_nameanddisplay_pathtoFwNode. They can be used by driver authors for logging purposes. In addition, they will be used by core property abstractions for automatic logging, for example when a driver attempts to read a required but missing property.Signed-off-by Remo Senekowitsch <[email protected]>--- rust/kernel/device/property.rs | 72 ++++++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+)rust/kernel/device/property.rs +0
-0@@ -32,6 +32,78 @@ pub(crate) fn as_raw(&self) -> *mut bindings::fwnode_handle { self.0 .get () }Quoted 14 lines
rust/kernel/device/property.rs pub(crate) fn as_raw(&self) -> *mut bindings::fwnode_handle { · line 45
Quoted 11 lines
rust/kernel/device/property.rs +11
-0+/// Returns an object that implements [`Display`](core::fmt::Display) for +/// printing the name of a node. +pub fn display_name (&self) ->impl core::fmt::Display + '_ { +struct FwNodeDisplayName <'a>(&'aFwNode ); + +impl core::fmt::Display for FwNodeDisplayName <'_> { +fn fmt (&self, f: &mut core::fmt::Formatter <'_>) -> core::fmt::Result { +// SAFETY: self is valid by its type invariant +let name =unsafe { bindings::fwnode_get_name (self.0 .as_raw ()) }; +if name.is_null () { +return Ok (());So if there is no name, you are returning
Ok()? Are you sure that's ok to do? What will the result of the string look like then?Quoted 7 lines
In that case we're not writing anything to the formatter, which is equivalent to an empty string.
Ok(())means that writing succeeded.I assumed that a valid node would always have a name. And we're guaranteed to have a valid node. So I assumed this case would never happen and didn't think too hard about it. But even if a valid node has not name, empty string is probably the correct thing, right?
I don't know what this "name" is used for. An empty string might not be what you want to use here, given that you could be naming something based on it, right?
fwnode_get_name()is used for many things, including the detection if a name is not present at all, and if not, then the code needs to clean up and abort.
Quoted 3 lines
Quoted 1 line
So what exactly are you going to be using this for?
Valid question... I'm not using it for anything.
You're using this in PropertyGuard::required_by(), where you use display_path() and hence display_name() to print the node path in the error case.
So, currently, this is only used in the error case when a property of a given node that is required by a driver can't be found.
Greg Kroah-Hartman
maintainerRe: [PATCH v5 4/9] rust: device: Enable printing fwnode name and path
Quoted 55 lines · Danilo Krummrich
Quoted 54 lines · Remo Senekowitsch
Quoted 52 lines · Greg Kroah-Hartman
Quoted 36 lines · Remo Senekowitsch
Quoted 20 lines · Greg Kroah-Hartman
Quoted 19 lines · Remo Senekowitsch
Add two new public methods
display_nameanddisplay_pathtoFwNode. They can be used by driver authors for logging purposes. In addition, they will be used by core property abstractions for automatic logging, for example when a driver attempts to read a required but missing property.Signed-off-by Remo Senekowitsch <[email protected]>--- rust/kernel/device/property.rs | 72 ++++++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+)rust/kernel/device/property.rs +0
-0@@ -32,6 +32,78 @@ pub(crate) fn as_raw(&self) -> *mut bindings::fwnode_handle { self.0 .get () }Quoted 14 lines
rust/kernel/device/property.rs pub(crate) fn as_raw(&self) -> *mut bindings::fwnode_handle { · line 45
Quoted 11 lines
rust/kernel/device/property.rs +11
-0+/// Returns an object that implements [`Display`](core::fmt::Display) for +/// printing the name of a node. +pub fn display_name (&self) ->impl core::fmt::Display + '_ { +struct FwNodeDisplayName <'a>(&'aFwNode ); + +impl core::fmt::Display for FwNodeDisplayName <'_> { +fn fmt (&self, f: &mut core::fmt::Formatter <'_>) -> core::fmt::Result { +// SAFETY: self is valid by its type invariant +let name =unsafe { bindings::fwnode_get_name (self.0 .as_raw ()) }; +if name.is_null () { +return Ok (());So if there is no name, you are returning
Ok()? Are you sure that's ok to do? What will the result of the string look like then?Quoted 7 lines
In that case we're not writing anything to the formatter, which is equivalent to an empty string.
Ok(())means that writing succeeded.I assumed that a valid node would always have a name. And we're guaranteed to have a valid node. So I assumed this case would never happen and didn't think too hard about it. But even if a valid node has not name, empty string is probably the correct thing, right?
I don't know what this "name" is used for. An empty string might not be what you want to use here, given that you could be naming something based on it, right?
fwnode_get_name()is used for many things, including the detection if a name is not present at all, and if not, then the code needs to clean up and abort.So what exactly are you going to be using this for?
Quoted 7 lines
Quoted 1 line
Valid question... I'm not using it for anything.
You're using this in
PropertyGuard::required_by(), where you usedisplay_path()and hencedisplay_name()to print the node path in the error case.So, currently, this is only used in the error case when a property of a given node that is required by a driver can't be found.
And in that case, the "normal" dev_err() output should be all that is needed here, not a "pretty printer" that might fail to document it at all :)
thanks,
greg k-h
Danilo Krummrich
maintainerRe: [PATCH v5 4/9] rust: device: Enable printing fwnode name and path
Quoted 61 lines · Greg Kroah-Hartman
Quoted 60 lines · Danilo Krummrich
Quoted 52 lines · Remo Senekowitsch
Quoted 51 lines · Greg Kroah-Hartman
Quoted 36 lines · Remo Senekowitsch
Quoted 20 lines · Greg Kroah-Hartman
Quoted 19 lines · Remo Senekowitsch
Add two new public methods
display_nameanddisplay_pathtoFwNode. They can be used by driver authors for logging purposes. In addition, they will be used by core property abstractions for automatic logging, for example when a driver attempts to read a required but missing property.Signed-off-by Remo Senekowitsch <[email protected]>--- rust/kernel/device/property.rs | 72 ++++++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+)rust/kernel/device/property.rs +0
-0@@ -32,6 +32,78 @@ pub(crate) fn as_raw(&self) -> *mut bindings::fwnode_handle { self.0 .get () }Quoted 14 lines
rust/kernel/device/property.rs pub(crate) fn as_raw(&self) -> *mut bindings::fwnode_handle { · line 45
Quoted 11 lines
rust/kernel/device/property.rs +11
-0+/// Returns an object that implements [`Display`](core::fmt::Display) for +/// printing the name of a node. +pub fn display_name (&self) ->impl core::fmt::Display + '_ { +struct FwNodeDisplayName <'a>(&'aFwNode ); + +impl core::fmt::Display for FwNodeDisplayName <'_> { +fn fmt (&self, f: &mut core::fmt::Formatter <'_>) -> core::fmt::Result { +// SAFETY: self is valid by its type invariant +let name =unsafe { bindings::fwnode_get_name (self.0 .as_raw ()) }; +if name.is_null () { +return Ok (());So if there is no name, you are returning
Ok()? Are you sure that's ok to do? What will the result of the string look like then?Quoted 7 lines
In that case we're not writing anything to the formatter, which is equivalent to an empty string.
Ok(())means that writing succeeded.I assumed that a valid node would always have a name. And we're guaranteed to have a valid node. So I assumed this case would never happen and didn't think too hard about it. But even if a valid node has not name, empty string is probably the correct thing, right?
I don't know what this "name" is used for. An empty string might not be what you want to use here, given that you could be naming something based on it, right?
fwnode_get_name()is used for many things, including the detection if a name is not present at all, and if not, then the code needs to clean up and abort.Quoted 3 lines
Quoted 1 line
So what exactly are you going to be using this for?
Valid question... I'm not using it for anything.
You're using this in
PropertyGuard::required_by(), where you usedisplay_path()and hencedisplay_name()to print the node path in the error case.
Quoted 6 lines
Quoted 2 lines
So, currently, this is only used in the error case when a property of a given node that is required by a driver can't be found.
And in that case, the "normal"
dev_err()output should be all that is needed here, not a "pretty printer" that might fail to document it at all :)
That's what the code does; PropertyGuard::required_by() uses dev_err!() to print the string.
It's just that the functions from the C side that provide the strings are abstracted in an implementation of core::fmt::Display for the FwNode, which is the idiomatic way to abstract a string representations of a type.
And since Display::fmt() is fallible, we need to decide if NULL is a valid value for fwnode_get_name() to return. The interface, i.e. struct fwnode_operations, does not document this, but to me it seems that fwnode_get_name() returning a NULL pointer is an error case.
Re: [PATCH v5 4/9] rust: device: Enable printing fwnode name and path
In reply to Remo SenekowitschQuoted 2 lines · Remo Senekowitsch
+ // SAFETY: self is valid by its type invariant
Please try to be consistent with the style of the rest of the code, e.g. use Markdown in comments and end with a period (you do it in some cases, but most are missing).
Thanks!
Cheers,
Miguel
PATCH v5 5/9 rust: device: Introduce PropertyGuard
In reply to Remo SenekowitschThis abstraction is a way to force users to specify whether a property is supposed to be required or not. This allows us to move error logging of missing required properties into core, preventing a lot of boilerplate in drivers.
It will be used by upcoming methods for reading device properties.
--- rust/kernel/device/property.rs | 59 ++++++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+)
rust/kernel/device/property.rs +59 -0
@@ -123,3 +123,62 @@ unsafe fn dec_ref(obj: ptr::NonNull<Self>) {unsafe { bindings::fwnode_handle_put (obj.cast ().as_ptr ()) } } } + +/// A helper for reading device properties. +/// +/// Use [`Self::required_by`] if a missing property is considered a bug and +/// [`Self::optional`] otherwise. +/// +/// For convenience, [`Self::or`] and [`Self::or_default`] are provided. +pub struct PropertyGuard <'fwnode, 'name,T > { +/// The result of reading the property. + inner:Result <T >, +/// The fwnode of the property, used for logging in the "required" case. + fwnode: &'fwnodeFwNode , +/// The name of the property, used for logging in the "required" case. + name: &'nameCStr , +} + +impl <T >PropertyGuard <'_, '_,T > { +/// Access the property, indicating it is required. +/// +/// If the property is not present, the error is automatically logged. If a +/// missing property is not an error, use [`Self::optional`] instead. The +/// device is required to associate the log with it. +pub fn required_by (self, dev: &super ::Device ) ->Result <T > { +if self.inner.is_err () { +dev_err! ( + dev, +"{}: property '{}' is missing\n" , + self.fwnode.display_path (), + self.name + ); + } + self.inner + } + +/// Access the property, indicating it is optional. +/// +/// In contrast to [`Self::required_by`], no error message is logged if +/// the property is not present. +pub fn optional (self) ->Option <T > { + self.inner.ok () + } + +/// Access the property or the specified default value. +/// +/// Do not pass a sentinel value as default to detect a missing property. +/// Use [`Self::required_by`] or [`Self::optional`] instead. +pub fn or (self, default:T ) ->T { + self.inner.unwrap_or (default) + } +} + +impl <T :Default >PropertyGuard <'_, '_,T > { +/// Access the property or a default value. +/// +/// Use [`Self::or`] to specify a custom default value. +pub fn or_default (self) ->T { + self.inner.unwrap_or_default () + } +}
Signature · 1 line
2.49.0
PATCH v5 6/9 rust: device: Implement accessors for firmware properties
In reply to Remo SenekowitschAdd methods to FwNode for reading several firmware property types like strings, integers and arrays.
Most types are read with the generic property_read method. There are two exceptions:
* property_read_bool cannot fail, so the fallible function signature of property_read would not make sense for reading booleans.
* property_read_array_vec can fail because of a dynamic memory allocation. This error must be handled separately, leading to a different function signature than property_read.
The traits Property and PropertyInt drive the generic behavior of property_read. PropertyInt is necessary to associate specific integer types with the C functions to read them. While there is a C function to read integers of generic sizes called fwnode_property_read_int_array, it was preferred not to make this public.
--- rust/kernel/device/property.rs | 253 ++++++++++++++++++++++++++++++++- 1 file changed, 251 insertions(+), 2 deletions(-)
rust/kernel/device/property.rs +251 -2
@@ -4,9 +4,17 @@//! //! C header: [`include/linux/property.h`](srctree/include/linux/property.h) -use core::ptr; +use core::{mem::MaybeUninit , ptr}; -use crate ::{bindings, str::CStr , types::Opaque }; +use crate ::{ + alloc::KVec , + bindings, + error::{to_result,Result }, + prelude::*, + private::Sealed , + str::{CStr ,CString }, + types::Opaque , +};/// A reference-counted fwnode_handle. ///
@@ -109,6 +117,103 @@ pub fn property_present(&self, name: &CStr) -> bool {// SAFETY: By the invariant of `CStr`, `name` is null-terminated. unsafe { bindings::fwnode_property_present (self.as_raw ().cast_const (), name.as_char_ptr ()) } } + +/// Returns firmware property `name` boolean value +pub fn property_read_bool (&self, name: &CStr ) ->bool { +// SAFETY: `name` is non-null and null-terminated. `self.as_raw()` is valid +// because `self` is valid. +unsafe { bindings::fwnode_property_read_bool (self.as_raw (), name.as_char_ptr ()) } + } + +/// Returns the index of matching string `match_str` for firmware string property `name` +pub fn property_match_string (&self, name: &CStr , match_str: &CStr ) ->Result <usize > { +// SAFETY: `name` and `match_str` are non-null and null-terminated. `self.as_raw` is +// valid because `self` is valid. +let ret =unsafe { + bindings::fwnode_property_match_string ( + self.as_raw (), + name.as_char_ptr (), + match_str.as_char_ptr (), + ) + }; +to_result (ret)?; +Ok (retas usize ) + } + +/// Returns firmware property `name` integer array values in a KVec +pub fn property_read_array_vec <'fwnode, 'name,T :PropertyInt >( + &'fwnode self, + name: &'nameCStr , + len:usize , + ) ->Result <PropertyGuard <'fwnode, 'name,KVec <T >>> { +let mut val:KVec <T > =KVec ::with_capacity (len,GFP_KERNEL )?; + +let res =T ::read_array_from_fwnode_property (self, name, val.spare_capacity_mut ()); +let res =match res { +Ok (_) => { +// SAFETY: +// - `len` is equal to `val.capacity - val.len`, because +// `val.capacity` is `len` and `val.len` is zero. +// - All elements within the interval [`0`, `len`) were initialized +// by `read_array_from_fwnode_property`. +unsafe { val.inc_len (len) } +Ok (val) + } +Err (e) =>Err (e), + }; +Ok (PropertyGuard { + inner: res, + fwnode: self, + name, + }) + } + +/// Returns integer array length for firmware property `name` +pub fn property_count_elem <T :PropertyInt >(&self, name: &CStr ) ->Result <usize > { +T ::read_array_len_from_fwnode_property (self, name) + } + +/// Returns the value of firmware property `name`. +/// +/// This method is generic over the type of value to read. Informally, +/// the types that can be read are booleans, strings, unsigned integers and +/// arrays of unsigned integers. +/// +/// Reading a [`KVec`] of integers is done with the separate +/// method [`Self::property_read_array_vec`], because it takes an +/// additional `len` argument. +/// +/// When reading a boolean, this method never fails. A missing property +/// is interpreted as `false`, whereas a present property is interpreted +/// as `true`. +/// +/// For more precise documentation about what types can be read, see +/// the [implementors of Property][Property#implementors] and [its +/// implementations on foreign types][Property#foreign-impls]. +/// +/// # Examples +/// +/// ``` +/// # use kernel::{c_str, device::{Device, property::FwNode}, str::CString}; +/// fn examples(dev: &Device) -> Result { +/// let fwnode = dev.fwnode().ok_or(ENOENT)?; +/// let b: u32 = fwnode.property_read(c_str!("some-number")).required_by(dev)?; +/// if let Some(s) = fwnode.property_read::<CString>(c_str!("some-str")).optional() { +/// // ... +/// } +/// Ok(()) +/// } +/// ``` +pub fn property_read <'fwnode, 'name,T :Property >( + &'fwnode self, + name: &'nameCStr , + ) ->PropertyGuard <'fwnode, 'name,T > { +PropertyGuard { + inner:T ::read_from_fwnode_property (self, name), + fwnode: self, + name, + } + } }// SAFETY: Instances of `FwNode` are always reference-counted.
@@ -124,6 +229,150 @@ unsafe fn dec_ref(obj: ptr::NonNull<Self>) { } } +/// Implemented for types that can be read as properties. +/// +/// This is implemented for strings, integers and arrays of integers. It's used +/// to make [`FwNode::property_read`] generic over the type of property being +/// read. There are also two dedicated methods to read other types, because they +/// require more specialized function signatures: +/// - [`property_read_bool`](FwNode::property_read_bool) +/// - [`property_read_array_vec`](FwNode::property_read_array_vec) +/// +/// It must be public, because it appears in the signatures of other public +/// functions, but its methods shouldn't be used outside the kernel crate. +pub trait Property :Sized +Sealed { +/// Used to make [`FwNode::property_read`] generic. +fn read_from_fwnode_property (fwnode: &FwNode , name: &CStr ) ->Result <Self >; +} + +impl Sealed for CString {} + +impl Property for CString { +fn read_from_fwnode_property (fwnode: &FwNode , name: &CStr ) ->Result <Self > { +let mut str: *mut u8 = ptr::null_mut (); +let pstr: *mut _ = &mut str; + +// SAFETY: `name` is non-null and null-terminated. `fwnode.as_raw` is +// valid because `fwnode` is valid. +let ret =unsafe { + bindings::fwnode_property_read_string (fwnode.as_raw (), name.as_char_ptr (), pstr.cast ()) + }; +to_result (ret)?; + +// SAFETY: `pstr` is a valid pointer to a NUL-terminated C string. It +// is valid for at least as long as `fwnode`, but it's only used within +// the current function. The memory it points to is not mutated during +// that time. +let str =unsafe {CStr ::from_char_ptr (*pstr) }; +Ok (str.try_into ()?) + } +} + +/// Implemented for all integers that can be read as properties. +/// +/// This helper trait is needed on top of the existing [`Property`] +/// trait to associate the integer types of various sizes with their +/// corresponding `fwnode_property_read_*_array` functions. +/// +/// It must be public, because it appears in the signatures of other public +/// functions, but its methods shouldn't be used outside the kernel crate. +pub trait PropertyInt :Copy +Sealed { +/// Reads a property array. +fn read_array_from_fwnode_property <'a>( + fwnode: &FwNode , + name: &CStr , + out: &'amut [MaybeUninit <Self >], + ) ->Result <&'amut [Self ]>; + +/// Reads the length of a property array. +fn read_array_len_from_fwnode_property (fwnode: &FwNode , name: &CStr ) ->Result <usize >; +} +// This macro generates implementations of the traits `Property` and +// `PropertyInt` for integers of various sizes. Its input is a list +// of pairs separated by commas. The first element of the pair is the +// type of the integer, the second one is the name of its corresponding +// `fwnode_property_read_*_array` function. +macro_rules! impl_property_for_int { + ($($int:ty: $f:ident),* $(,)?) => { $( +impl Sealed for $int {} +impl <const N :usize >Sealed for [$int;N ] {} + +impl PropertyInt for $int { +fn read_array_from_fwnode_property <'a>( + fwnode: &FwNode , + name: &CStr , + out: &'amut [MaybeUninit <Self >], + ) ->Result <&'amut [Self ]> { +// SAFETY: `fwnode`, `name` and `out` are all valid by their type +// invariants. `out.len()` is a valid bound for the memory pointed +// to by `out.as_mut_ptr()`. +// CAST: It's ok to cast from `*mut MaybeUninit<$int>` to a +// `*mut $int` because they have the same memory layout. +let ret =unsafe { +bindings ::$f( + fwnode.as_raw (), + name.as_char_ptr (), + out.as_mut_ptr ().cast (), + out.len (), + ) + }; +to_result (ret)?; +// SAFETY: Transmuting from `&'a mut [MaybeUninit<Self>]` to +// `&'a mut [Self]` is sound, because the previous call to a +// `fwnode_property_read_*_array` function (which didn't fail) +// fully initialized the slice. +Ok (unsafe { core::mem::transmute (out) }) + } + +fn read_array_len_from_fwnode_property (fwnode: &FwNode , name: &CStr ) ->Result <usize > { +// SAFETY: `fwnode` and `name` are valid by their type invariants. +// It's ok to pass a null pointer to the +// `fwnode_property_read_*_array` functions if `nval` is zero. +// This will return the length of the array. +let ret =unsafe { +bindings ::$f( + fwnode.as_raw (), + name.as_char_ptr (), + ptr::null_mut (), +0 , + ) + }; +to_result (ret)?; +Ok (retas usize ) + } + } + +impl Property for $int { +fn read_from_fwnode_property (fwnode: &FwNode , name: &CStr ) ->Result <Self > { +let val: [_ ;1 ] = <[$int;1 ]>::read_from_fwnode_property (fwnode, name)?; +Ok (val[0 ]) + } + } + +impl <const N :usize >Property for [$int;N ] { +fn read_from_fwnode_property (fwnode: &FwNode , name: &CStr ) ->Result <Self > { +let mut val: [MaybeUninit <$int>;N ] = [const {MaybeUninit ::uninit () };N ]; + + <$int>::read_array_from_fwnode_property (fwnode, name, &mut val)?; + +// SAFETY: `val` is always initialized when +// fwnode_property_read_*_array is successful. +Ok (val.map (|v|unsafe { v.assume_init () })) + } + } + )* }; +} +impl_property_for_int! { +u8 : fwnode_property_read_u8_array, +u16 : fwnode_property_read_u16_array, +u32 : fwnode_property_read_u32_array, +u64 : fwnode_property_read_u64_array, +i8 : fwnode_property_read_u8_array, +i16 : fwnode_property_read_u16_array, +i32 : fwnode_property_read_u32_array, +i64 : fwnode_property_read_u64_array, +} +/// A helper for reading device properties. /// /// Use [`Self::required_by`] if a missing property is considered a bug and
Signature · 1 line
2.49.0
kernel test robot
build botRe: [PATCH v5 6/9] rust: device: Implement accessors for firmware properties
kernel test robot noticed the following build errors:
Hi Remo,
kernel test robot noticed the following build errors:
[auto build test ERROR on rust/rust-next]
[also build test ERROR on driver-core/driver-core-testing driver-core/driver-core-next driver-core/driver-core-linus robh/for-next linus/master v6.15-rc7 next-20250521] [If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Remo-Senekowitsch/rust-device-Create-FwNode-abstraction-for-accessing-device-properties/20250521-040612 base: https://github.com/Rust-for-Linux/linux rust-next patch link: https://lore.kernel.org/r/20250520200024.268655-7-remo%40buenzli.dev patch subject: [PATCH v5 6/9] rust: device: Implement accessors for firmware properties config: x86_64-rhel-9.4-rust (https://download.01.org/0day-ci/archive/20250522/[email protected]/config) compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff) rustc: rustc 1.78.0 (9b00956e5 2024-04-29) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250522/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All errors (new ones prefixed by >>):
>> error[E0432]: unresolved import `crate::private` --> rust/kernel/device/property.rs:14:5 | 14 | private::Sealed, | ^^^^^^^ | | | unresolved import | help: a similar path exists: `crate::device::private`
Signature · 2 lines
0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki
Benno Lossin
reviewerRe: [PATCH v5 6/9] rust: device: Implement accessors for firmware properties
In reply to Remo SenekowitschQuoted 20 lines · Remo Senekowitsch
Add methods to FwNode for reading several firmware property types like strings, integers and arrays.
Most types are read with the generic
property_readmethod. There are two exceptions:*
property_read_boolcannot fail, so the fallible function signature ofproperty_readwould not make sense for reading booleans.*
property_read_array_veccan fail because of a dynamic memory allocation. This error must be handled separately, leading to a different function signature thanproperty_read.The traits
PropertyandPropertyIntdrive the generic behavior ofproperty_read.PropertyIntis necessary to associate specific integer types with the C functions to read them. While there is a C function to read integers of generic sizes calledfwnode_property_read_int_array, it was preferred not to make this public.
Quoted 6 lines
Co-developed-by Rob Herring (Arm) <[email protected]>Signed-off-by Rob Herring (Arm) <[email protected]>Signed-off-by Remo Senekowitsch <[email protected]>--- rust/kernel/device/property.rs | 253 ++++++++++++++++++++++++++++++++- 1 file changed, 251 insertions(+), 2 deletions(-)
I already like this design much better than your previous version.
I probably won't have the time to review the other patches, but one thing that I notice was the lack of lists in SAFETY comments. If there are multiple justification in a single SAFETY comment, we us a bullet markdown list. (same for requirements in a # Safety section)
---
Cheers,
Benno
PATCH v5 7/9 rust: device: Add child accessor and iterator
In reply to Remo SenekowitschAllow Rust drivers to access children of a fwnode either by name or by iterating over all of them.
In C, there is the function fwnode_get_next_child_node for iteration and the macro fwnode_for_each_child_node that helps with handling the pointers. Instead of a macro, a native iterator is used in Rust such that regular for-loops can be used.
--- rust/kernel/device/property.rs | 79 +++++++++++++++++++++++++++++++++- 1 file changed, 78 insertions(+), 1 deletion(-)
rust/kernel/device/property.rs +78 -1
@@ -13,7 +13,7 @@ prelude::*, private::Sealed, str::{CStr, CString}, - types::Opaque, + types::{ARef, Opaque}, };/// A reference-counted fwnode_handle.
@@ -35,6 +35,27 @@pub struct FwNode (Opaque <bindings::fwnode_handle >);impl FwNode { +/// # Safety +/// +/// Callers must ensure that: +/// - The reference count was incremented at least once. +/// - They relinquish that increment. That is, if there is only one +/// increment, callers must not use the underlying object anymore -- it is +/// only safe to do so via the newly created `ARef<FwNode>`. +unsafe fn from_raw (raw: *mut bindings::fwnode_handle ) ->ARef <Self > { +// SAFETY: As per the safety requirements of this function: +// - `NonNull::new_unchecked`: +// - `raw` is not null. +// - `ARef::from_raw`: +// - `raw` has an incremented refcount. +// - that increment is relinquished, i.e. it won't be decremented +// elsewhere. +// CAST: It is safe to cast from a `*mut fwnode_handle` to +// `*mut FwNode`, because `FwNode` is defined as a +// `#[repr(transparent)]` wrapper around `fwnode_handle`. +unsafe {ARef ::from_raw (ptr::NonNull ::new_unchecked (raw.cast ())) } + } +/// Obtain the raw `struct fwnode_handle *`. pub (crate )fn as_raw (&self) -> *mut bindings::fwnode_handle { self.0 .get ()
@@ -214,6 +235,62 @@ pub fn property_read<'fwnode, 'name, T: Property>( name, } } + +/// Returns first matching named child node handle. +pub fn get_child_by_name (&self, name: &CStr ) ->Option <ARef <Self >> { +// SAFETY: `self` and `name` are valid by their type invariants. +let child = +unsafe { bindings::fwnode_get_named_child_node (self.as_raw (), name.as_char_ptr ()) }; +if child.is_null () { +return None ; + } +// SAFETY: +// - `fwnode_get_named_child_node` returns a pointer with its refcount +// incremented. +// - That increment is relinquished, i.e. the underlying object is not +// used anymore except via the newly created `ARef`. +Some (unsafe {Self ::from_raw (child) }) + } + +/// Returns an iterator over a node's children. +pub fn children <'a>(&'a self) ->impl Iterator <Item =ARef <FwNode >> + 'a { +let mut prev:Option <ARef <FwNode >> =None ; + + core::iter::from_fn (move || { +let prev_ptr =match prev.take () { +None => ptr::null_mut (), +Some (prev) => { +// We will pass `prev` to `fwnode_get_next_child_node`, +// which decrements its refcount, so we use +// `ARef::into_raw` to avoid decrementing the refcount +// twice. +let prev =ARef ::into_raw (prev); + prev.as_ptr ().cast () + } + }; +// SAFETY: +// - `self.as_raw()` is valid by its type invariant. +// - `prev_ptr` may be null, which is allowed and corresponds to +// getting the first child. Otherwise, `prev_ptr` is valid, as it +// is the stored return value from the previous invocation. +// - `prev_ptr` has its refount incremented. +// - The increment of `prev_ptr` is relinquished, i.e. the +// underlying object won't be used anymore. +let next =unsafe { bindings::fwnode_get_next_child_node (self.as_raw (), prev_ptr) }; +if next.is_null () { +return None ; + } +// SAFETY: +// - `next` is valid because `fwnode_get_next_child_node` returns a +// pointer with its refcount incremented. +// - That increment is relinquished, i.e. the underlying object +// won't be used anymore, except via the newly created +// `ARef<Self>`. +let next =unsafe {FwNode ::from_raw (next) }; + prev =Some (next.clone ()); +Some (next) + }) + } }// SAFETY: Instances of `FwNode` are always reference-counted.
Signature · 1 line
2.49.0
PATCH v5 8/9 rust: device: Add property_get_reference_args
In reply to Remo SenekowitschAllow Rust code to read reference args from device properties. The wrapper type FwNodeReferenceArgs allows callers to access the buffer of read args safely.
--- rust/kernel/device/property.rs | 68 ++++++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+)
rust/kernel/device/property.rs +68 -0
@@ -291,6 +291,65 @@ pub fn children<'a>(&'a self) -> impl Iterator<Item = ARef<FwNode>> + 'a {Some (next) }) } + +/// Finds a reference with arguments. +pub fn property_get_reference_args ( + &self, + prop: &CStr , + nargs:NArgs <'_>, + index:u32 , + ) ->Result <FwNodeReferenceArgs > { +let mut out_args =FwNodeReferenceArgs ::default (); + +let (nargs_prop, nargs) =match nargs { +NArgs ::Prop (nargs_prop) => (nargs_prop.as_char_ptr (),0 ), +NArgs ::N (nargs) => (ptr::null (), nargs), + }; + +// SAFETY: `self.0.get()` is valid. `prop.as_char_ptr()` is valid and +// zero-terminated. `nargs_prop` is valid and zero-terminated if `nargs` +// is zero, otherwise it is allowed to be a null-pointer. +let ret =unsafe { + bindings::fwnode_property_get_reference_args ( + self.0 .get (), + prop.as_char_ptr (), + nargs_prop, + nargs, + index, + &mut out_args.0 , + ) + }; +to_result (ret)?; + +Ok (out_args) + } +} + +/// The return value of `property_get_reference_args`. +/// +/// - [`Device::property_get_reference_args`] +/// - [`FwNode::property_get_reference_args`] +#[repr(transparent)] +#[derive( Copy , Clone , Default )] +pub struct FwNodeReferenceArgs (bindings::fwnode_reference_args ); + +impl FwNodeReferenceArgs { +/// Returns the slice of reference arguments. +pub fn as_slice (&self) -> &[u64 ] { +// SAFETY: As per the safety invariant of FwNodeReferenceArgs, `nargs` +// is the number of elements in `args` that is valid. +unsafe { core::slice::from_raw_parts (self.0 .args.as_ptr (), self.0 .nargsas usize ) } + } + +/// Returns the number of reference arguments. +pub fn len (&self) ->usize { + self.0 .nargsas usize + } + +/// Returns `true` if there are no reference arguments. +pub fn is_empty (&self) ->bool { + self.0 .nargs ==0 + } }// SAFETY: Instances of `FwNode` are always reference-counted.
@@ -450,6 +509,15 @@ fn read_from_fwnode_property(fwnode: &FwNode, name: &CStr) -> Result<Self> { i64: fwnode_property_read_u64_array, } +/// The number of arguments of a reference. +pub enum NArgs <'a> { +/// The name of the property of the reference indicating the number of +/// arguments. +Prop (&'aCStr ), +/// The known number of arguments. +N (u32 ), +} +/// A helper for reading device properties. /// /// Use [`Self::required_by`] if a missing property is considered a bug and
Signature · 1 line
2.49.0
PATCH v5 9/9 samples: rust: platform: Add property read examples
In reply to Remo SenekowitschAdd some example usage of the device property read methods for DT/ACPI/swnode properties.
--- drivers/of/unittest-data/tests-platform.dtsi | 3 + samples/rust/rust_driver_platform.rs | 60 +++++++++++++++++++- 2 files changed, 62 insertions(+), 1 deletion(-)
drivers/of/unittest-data/tests-platform.dtsi +3 -0
@@ -37,6 +37,9 @@ dev@100 { test-device@2 { compatible = "test,rust-device"; reg = <0x2>; + + test,u32-prop = <0xdeadbeef>; + test,i16-array = /bits/ 16 <1 2 (-3) (-4)>; }; };
samples/rust/rust_driver_platform.rs +59 -1
78 lines not on this page · raw message
Signature · 1 line
2.49.0