Add some example usage of the device property read methods for
DT/ACPI/swnode properties.
Co-developed-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: Remo Senekowitsch <remo@buenzli.dev>
---
drivers/of/unittest-data/tests-platform.dtsi | 3 ++
samples/rust/rust_driver_platform.rs | 56 +++++++++++++++++++-
2 files changed, 58 insertions(+), 1 deletion(-)
diff --git a/drivers/of/unittest-data/tests-platform.dtsi b/drivers/of/unittest-data/tests-platform.dtsi
index 4171f43cf..50a51f38a 100644
--- a/drivers/of/unittest-data/tests-platform.dtsi
+++ b/drivers/of/unittest-data/tests-platform.dtsi
@@ -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)>;
};
};
diff --git a/samples/rust/rust_driver_platform.rs b/samples/rust/rust_driver_platform.rs
index 8120609e2..ed25a3781 100644
--- a/samples/rust/rust_driver_platform.rs
+++ b/samples/rust/rust_driver_platform.rs
@@ -2,7 +2,7 @@
//! Rust Platform driver sample.
-use kernel::{c_str, of, platform, prelude::*};
+use kernel::{c_str, of, platform, prelude::*, str::CString};
struct SampleDriver {
pdev: platform::Device,
@@ -28,6 +28,60 @@ fn probe(pdev: &mut platform::Device, info: Option<&Self::IdInfo>) -> Result<Pin
dev_info!(pdev.as_ref(), "Probed with info: '{}'.\n", info.0);
}
+ let dev = pdev.as_ref();
+ if let Ok(idx) = dev.property_match_string(c_str!("compatible"), c_str!("test,rust-device"))
+ {
+ dev_info!(pdev.as_ref(), "matched compatible string idx = {}\n", idx);
+ }
+
+ if let Ok(str) = dev
+ .property_read::<CString>(c_str!("compatible"))
+ .required()
+ {
+ dev_info!(pdev.as_ref(), "compatible string = {:?}\n", str);
+ }
+
+ let prop = dev
+ .property_read::<bool>(c_str!("test,bool-prop"))
+ .required()?;
+ dev_info!(dev, "bool prop is {}\n", prop);
+
+ if dev.property_present(c_str!("test,u32-prop")) {
+ dev_info!(dev, "'test,u32-prop' is present\n");
+ }
+
+ let prop = dev
+ .property_read::<u32>(c_str!("test,u32-optional-prop"))
+ .or(0x12);
+ dev_info!(
+ dev,
+ "'test,u32-optional-prop' is {:#x} (default = {:#x})\n",
+ prop,
+ 0x12
+ );
+
+ // Missing property without a default will print an error
+ let _ = dev
+ .property_read::<u32>(c_str!("test,u32-required-prop"))
+ .required()?;
+
+ let prop: u32 = dev.property_read(c_str!("test,u32-prop")).required()?;
+ dev_info!(dev, "'test,u32-prop' is {:#x}\n", prop);
+
+ let prop: [i16; 4] = dev.property_read(c_str!("test,i16-array")).required()?;
+ dev_info!(dev, "'test,i16-array' is {:?}\n", prop);
+ dev_info!(
+ dev,
+ "'test,i16-array' length is {}\n",
+ dev.property_count_elem::<u16>(c_str!("test,i16-array"))
+ .unwrap()
+ );
+
+ let prop: KVec<i16> = dev
+ .property_read_array_vec(c_str!("test,i16-array"), 4)?
+ .required()?;
+ dev_info!(dev, "'test,i16-array' is KVec {:?}\n", prop);
+
let drvdata = KBox::new(Self { pdev: pdev.clone() }, GFP_KERNEL)?;
Ok(drvdata.into())
--
2.49.0On Wed, Mar 26, 2025 at 06:13:49PM +0100, Remo Senekowitsch wrote:
> --- a/samples/rust/rust_driver_platform.rs
> +++ b/samples/rust/rust_driver_platform.rs
> @@ -2,7 +2,7 @@
>
> //! Rust Platform driver sample.
>
> -use kernel::{c_str, of, platform, prelude::*};
> +use kernel::{c_str, of, platform, prelude::*, str::CString};
>
> struct SampleDriver {
> pdev: platform::Device,
> @@ -28,6 +28,60 @@ fn probe(pdev: &mut platform::Device, info: Option<&Self::IdInfo>) -> Result<Pin
> dev_info!(pdev.as_ref(), "Probed with info: '{}'.\n", info.0);
> }
>
> + let dev = pdev.as_ref();
> + if let Ok(idx) = dev.property_match_string(c_str!("compatible"), c_str!("test,rust-device"))
> + {
> + dev_info!(pdev.as_ref(), "matched compatible string idx = {}\n", idx);
> + }
> +
> + if let Ok(str) = dev
> + .property_read::<CString>(c_str!("compatible"))
> + .required()
> + {
> + dev_info!(pdev.as_ref(), "compatible string = {:?}\n", str);
> + }
> +
> + let prop = dev
> + .property_read::<bool>(c_str!("test,bool-prop"))
> + .required()?;
> + dev_info!(dev, "bool prop is {}\n", prop);
> +
> + if dev.property_present(c_str!("test,u32-prop")) {
> + dev_info!(dev, "'test,u32-prop' is present\n");
> + }
> +
> + let prop = dev
> + .property_read::<u32>(c_str!("test,u32-optional-prop"))
> + .or(0x12);
> + dev_info!(
> + dev,
> + "'test,u32-optional-prop' is {:#x} (default = {:#x})\n",
> + prop,
> + 0x12
> + );
Printing the default value looks more like a test, rather than a sample.
> +
> + // Missing property without a default will print an error
> + let _ = dev
> + .property_read::<u32>(c_str!("test,u32-required-prop"))
> + .required()?;
> +
> + let prop: u32 = dev.property_read(c_str!("test,u32-prop")).required()?;
> + dev_info!(dev, "'test,u32-prop' is {:#x}\n", prop);
> +
> + let prop: [i16; 4] = dev.property_read(c_str!("test,i16-array")).required()?;
> + dev_info!(dev, "'test,i16-array' is {:?}\n", prop);
> + dev_info!(
> + dev,
> + "'test,i16-array' length is {}\n",
> + dev.property_count_elem::<u16>(c_str!("test,i16-array"))
> + .unwrap()
Please no unwrap() in the kernel, it may panic.
> + );
> +
> + let prop: KVec<i16> = dev
> + .property_read_array_vec(c_str!("test,i16-array"), 4)?
> + .required()?;
> + dev_info!(dev, "'test,i16-array' is KVec {:?}\n", prop);
> +
Please move this code to a new function, e.g.
impl SampleDriver {
fn properties_parse(dev: &device::Device)) -> Result;
}
in order to keep probe() as clean as possible. If we put too much stuff in
there, it might become too confusing to serve as a simple example showing how to
implement a platform driver in Rust.
Besides that, are we sure we really want to print everything?
On Wed, Mar 26, 2025 at 06:13:49PM +0100, Remo Senekowitsch wrote:
> Add some example usage of the device property read methods for
> DT/ACPI/swnode properties.
>
> Co-developed-by: Rob Herring (Arm) <robh@kernel.org>
> Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
> Signed-off-by: Remo Senekowitsch <remo@buenzli.dev>
> ---
> drivers/of/unittest-data/tests-platform.dtsi | 3 ++
> samples/rust/rust_driver_platform.rs | 56 +++++++++++++++++++-
> 2 files changed, 58 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/of/unittest-data/tests-platform.dtsi b/drivers/of/unittest-data/tests-platform.dtsi
> index 4171f43cf..50a51f38a 100644
> --- a/drivers/of/unittest-data/tests-platform.dtsi
> +++ b/drivers/of/unittest-data/tests-platform.dtsi
> @@ -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)>;
> };
> };
>
> diff --git a/samples/rust/rust_driver_platform.rs b/samples/rust/rust_driver_platform.rs
> index 8120609e2..ed25a3781 100644
> --- a/samples/rust/rust_driver_platform.rs
> +++ b/samples/rust/rust_driver_platform.rs
> @@ -2,7 +2,7 @@
>
> //! Rust Platform driver sample.
>
> -use kernel::{c_str, of, platform, prelude::*};
> +use kernel::{c_str, of, platform, prelude::*, str::CString};
>
> struct SampleDriver {
> pdev: platform::Device,
> @@ -28,6 +28,60 @@ fn probe(pdev: &mut platform::Device, info: Option<&Self::IdInfo>) -> Result<Pin
> dev_info!(pdev.as_ref(), "Probed with info: '{}'.\n", info.0);
> }
>
> + let dev = pdev.as_ref();
We should move this to the top and replace all the 'pdev.as_ref()' with
'dev'.
> + if let Ok(idx) = dev.property_match_string(c_str!("compatible"), c_str!("test,rust-device"))
> + {
> + dev_info!(pdev.as_ref(), "matched compatible string idx = {}\n", idx);
Like here. (Looks like this is my fault.)
> + }
> +
> + if let Ok(str) = dev
> + .property_read::<CString>(c_str!("compatible"))
> + .required()
> + {
> + dev_info!(pdev.as_ref(), "compatible string = {:?}\n", str);
> + }
> +
> + let prop = dev
> + .property_read::<bool>(c_str!("test,bool-prop"))
> + .required()?;
The 'required' is kind of odd for boolean properties. They are never
required as not present is the only way to to get false.
> + dev_info!(dev, "bool prop is {}\n", prop);
> +
> + if dev.property_present(c_str!("test,u32-prop")) {
> + dev_info!(dev, "'test,u32-prop' is present\n");
> + }
> +
> + let prop = dev
> + .property_read::<u32>(c_str!("test,u32-optional-prop"))
> + .or(0x12);
> + dev_info!(
> + dev,
> + "'test,u32-optional-prop' is {:#x} (default = {:#x})\n",
> + prop,
> + 0x12
> + );
> +
> + // Missing property without a default will print an error
> + let _ = dev
> + .property_read::<u32>(c_str!("test,u32-required-prop"))
> + .required()?;
> +
> + let prop: u32 = dev.property_read(c_str!("test,u32-prop")).required()?;
> + dev_info!(dev, "'test,u32-prop' is {:#x}\n", prop);
> +
> + let prop: [i16; 4] = dev.property_read(c_str!("test,i16-array")).required()?;
> + dev_info!(dev, "'test,i16-array' is {:?}\n", prop);
> + dev_info!(
> + dev,
> + "'test,i16-array' length is {}\n",
> + dev.property_count_elem::<u16>(c_str!("test,i16-array"))
> + .unwrap()
> + );
> +
> + let prop: KVec<i16> = dev
> + .property_read_array_vec(c_str!("test,i16-array"), 4)?
> + .required()?;
> + dev_info!(dev, "'test,i16-array' is KVec {:?}\n", prop);
> +
> let drvdata = KBox::new(Self { pdev: pdev.clone() }, GFP_KERNEL)?;
>
> Ok(drvdata.into())
> --
> 2.49.0
>
On Wed Mar 26, 2025 at 11:01 PM CET, Rob Herring wrote:
>>
>> + let prop = dev
>> + .property_read::<bool>(c_str!("test,bool-prop"))
>> + .required()?;
>
> The 'required' is kind of odd for boolean properties. They are never
> required as not present is the only way to to get false.
Agreed. I can think of a few alternatives:
* Make the trait `Property` more flexible to allow each implementor to specify
what its output type for the `read` function is, via an associated type.
I really don't like this idea, because overly generic APIs can mess with type
inference and become less ergonomic because of it.
* Use `propert_present` instead. That doesn't perfectly express the intention,
because it doesn't warn if the property is present but has a type other than
bool.
* Add an additional inherent method `property_read_bool`, which returns a plain
`bool` instead of `PropertyGuard<bool>`. Then there will be three slightly
different ways to read a bool: `property_present`, `property_read_bool` and
`property_read::<bool>`. Maybe that's confusing.
* Add `property_read_bool` and remove `impl Property for bool`. That would avoid
confusion between `property_read_bool` and `property_read::<bool>`, only the
former would work.
On Wed, Mar 26, 2025 at 5:24 PM Remo Senekowitsch <remo@buenzli.dev> wrote:
>
> On Wed Mar 26, 2025 at 11:01 PM CET, Rob Herring wrote:
> >>
> >> + let prop = dev
> >> + .property_read::<bool>(c_str!("test,bool-prop"))
> >> + .required()?;
> >
> > The 'required' is kind of odd for boolean properties. They are never
> > required as not present is the only way to to get false.
>
> Agreed. I can think of a few alternatives:
>
> * Make the trait `Property` more flexible to allow each implementor to specify
> what its output type for the `read` function is, via an associated type.
> I really don't like this idea, because overly generic APIs can mess with type
> inference and become less ergonomic because of it.
>
> * Use `propert_present` instead. That doesn't perfectly express the intention,
> because it doesn't warn if the property is present but has a type other than
> bool.
Right. I've been cleaning up the tree to use of_property_read_bool()
on bools and of_property_present() on non-bools, so don't want to go
back to 1 function. The C code now warns on a mismatch.
> * Add an additional inherent method `property_read_bool`, which returns a plain
> `bool` instead of `PropertyGuard<bool>`. Then there will be three slightly
> different ways to read a bool: `property_present`, `property_read_bool` and
> `property_read::<bool>`. Maybe that's confusing.
>
> * Add `property_read_bool` and remove `impl Property for bool`. That would avoid
> confusion between `property_read_bool` and `property_read::<bool>`, only the
> former would work.
I think I would go with this option. Easier to add another way later
than remove one.
Rob
© 2016 - 2025 Red Hat, Inc.