diff options
author | Abdiel Janulgue <abdiel.janulgue@gmail.com> | 2025-06-02 11:53:12 +0300 |
---|---|---|
committer | Danilo Krummrich <dakr@kernel.org> | 2025-06-23 15:34:13 +0200 |
commit | fe58465905550576cc47cf93efeaaa6990e6c3b3 (patch) | |
tree | dd1d6f1b3bf3804678c8d05c87c69641c2c9debe | |
parent | 9863f7743339ae53a0cf80e5f229cf6d2a42a6e6 (diff) |
rust: dma: convert the read/write macros to return Result
We could do better here by having the macros return `Result`,
so that we don't have to wrap these calls in a closure for
validation which is confusing.
Co-developed-by: Andreas Hindborg <a.hindborg@kernel.org>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
Link: https://lore.kernel.org/rust-for-linux/87h63qhz4q.fsf@kernel.org/
Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>
Signed-off-by: Abdiel Janulgue <abdiel.janulgue@gmail.com>
Link: https://lore.kernel.org/r/20250602085444.1925053-3-abdiel.janulgue@gmail.com
[ Fix line length in dma_read!(). - Danilo ]
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
-rw-r--r-- | rust/kernel/dma.rs | 56 | ||||
-rw-r--r-- | samples/rust/rust_dma.rs | 28 |
2 files changed, 48 insertions, 36 deletions
diff --git a/rust/kernel/dma.rs b/rust/kernel/dma.rs index e5d1ffef1a53..38356f61c90b 100644 --- a/rust/kernel/dma.rs +++ b/rust/kernel/dma.rs @@ -328,20 +328,24 @@ unsafe impl<T: AsBytes + FromBytes + Send> Send for CoherentAllocation<T> {} #[macro_export] macro_rules! dma_read { ($dma:expr, $idx: expr, $($field:tt)*) => {{ - let item = $crate::dma::CoherentAllocation::item_from_index(&$dma, $idx)?; - // SAFETY: `item_from_index` ensures that `item` is always a valid pointer and can be - // dereferenced. The compiler also further validates the expression on whether `field` - // is a member of `item` when expanded by the macro. - unsafe { - let ptr_field = ::core::ptr::addr_of!((*item) $($field)*); - $crate::dma::CoherentAllocation::field_read(&$dma, ptr_field) - } + (|| -> ::core::result::Result<_, $crate::error::Error> { + let item = $crate::dma::CoherentAllocation::item_from_index(&$dma, $idx)?; + // SAFETY: `item_from_index` ensures that `item` is always a valid pointer and can be + // dereferenced. The compiler also further validates the expression on whether `field` + // is a member of `item` when expanded by the macro. + unsafe { + let ptr_field = ::core::ptr::addr_of!((*item) $($field)*); + ::core::result::Result::Ok( + $crate::dma::CoherentAllocation::field_read(&$dma, ptr_field) + ) + } + })() }}; ($dma:ident [ $idx:expr ] $($field:tt)* ) => { - $crate::dma_read!($dma, $idx, $($field)*); + $crate::dma_read!($dma, $idx, $($field)*) }; ($($dma:ident).* [ $idx:expr ] $($field:tt)* ) => { - $crate::dma_read!($($dma).*, $idx, $($field)*); + $crate::dma_read!($($dma).*, $idx, $($field)*) }; } @@ -368,24 +372,30 @@ macro_rules! dma_read { #[macro_export] macro_rules! dma_write { ($dma:ident [ $idx:expr ] $($field:tt)*) => {{ - $crate::dma_write!($dma, $idx, $($field)*); + $crate::dma_write!($dma, $idx, $($field)*) }}; ($($dma:ident).* [ $idx:expr ] $($field:tt)* ) => {{ - $crate::dma_write!($($dma).*, $idx, $($field)*); + $crate::dma_write!($($dma).*, $idx, $($field)*) }}; ($dma:expr, $idx: expr, = $val:expr) => { - let item = $crate::dma::CoherentAllocation::item_from_index(&$dma, $idx)?; - // SAFETY: `item_from_index` ensures that `item` is always a valid item. - unsafe { $crate::dma::CoherentAllocation::field_write(&$dma, item, $val) } + (|| -> ::core::result::Result<_, $crate::error::Error> { + let item = $crate::dma::CoherentAllocation::item_from_index(&$dma, $idx)?; + // SAFETY: `item_from_index` ensures that `item` is always a valid item. + unsafe { $crate::dma::CoherentAllocation::field_write(&$dma, item, $val) } + ::core::result::Result::Ok(()) + })() }; ($dma:expr, $idx: expr, $(.$field:ident)* = $val:expr) => { - let item = $crate::dma::CoherentAllocation::item_from_index(&$dma, $idx)?; - // SAFETY: `item_from_index` ensures that `item` is always a valid pointer and can be - // dereferenced. The compiler also further validates the expression on whether `field` - // is a member of `item` when expanded by the macro. - unsafe { - let ptr_field = ::core::ptr::addr_of_mut!((*item) $(.$field)*); - $crate::dma::CoherentAllocation::field_write(&$dma, ptr_field, $val) - } + (|| -> ::core::result::Result<_, $crate::error::Error> { + let item = $crate::dma::CoherentAllocation::item_from_index(&$dma, $idx)?; + // SAFETY: `item_from_index` ensures that `item` is always a valid pointer and can be + // dereferenced. The compiler also further validates the expression on whether `field` + // is a member of `item` when expanded by the macro. + unsafe { + let ptr_field = ::core::ptr::addr_of_mut!((*item) $(.$field)*); + $crate::dma::CoherentAllocation::field_write(&$dma, ptr_field, $val) + } + ::core::result::Result::Ok(()) + })() }; } diff --git a/samples/rust/rust_dma.rs b/samples/rust/rust_dma.rs index 874c2c964afa..9e05d5c0cdae 100644 --- a/samples/rust/rust_dma.rs +++ b/samples/rust/rust_dma.rs @@ -54,13 +54,9 @@ impl pci::Driver for DmaSampleDriver { let ca: CoherentAllocation<MyStruct> = CoherentAllocation::alloc_coherent(pdev.as_ref(), TEST_VALUES.len(), GFP_KERNEL)?; - || -> Result { - for (i, value) in TEST_VALUES.into_iter().enumerate() { - kernel::dma_write!(ca[i] = MyStruct::new(value.0, value.1)); - } - - Ok(()) - }()?; + for (i, value) in TEST_VALUES.into_iter().enumerate() { + kernel::dma_write!(ca[i] = MyStruct::new(value.0, value.1))?; + } let drvdata = KBox::new( Self { @@ -78,13 +74,19 @@ impl Drop for DmaSampleDriver { fn drop(&mut self) { dev_info!(self.pdev.as_ref(), "Unload DMA test driver.\n"); - let _ = || -> Result { - for (i, value) in TEST_VALUES.into_iter().enumerate() { - assert_eq!(kernel::dma_read!(self.ca[i].h), value.0); - assert_eq!(kernel::dma_read!(self.ca[i].b), value.1); + for (i, value) in TEST_VALUES.into_iter().enumerate() { + let val0 = kernel::dma_read!(self.ca[i].h); + let val1 = kernel::dma_read!(self.ca[i].b); + assert!(val0.is_ok()); + assert!(val1.is_ok()); + + if let Ok(val0) = val0 { + assert_eq!(val0, value.0); + } + if let Ok(val1) = val1 { + assert_eq!(val1, value.1); } - Ok(()) - }(); + } } } |