On Fri, Apr 18, 2025 at 08:08:02AM -0400, Tamir Duberstein wrote:
On Thu, Apr 17, 2025 at 4:12 PM Boqun Feng boqun.feng@gmail.com wrote:
On Thu, Apr 17, 2025 at 03:26:14PM -0400, Tamir Duberstein wrote: [...]
Ok(()) }diff --git a/rust/kernel/device_id.rs b/rust/kernel/device_id.rs index e5859217a579..4063f09d76d9 100644 --- a/rust/kernel/device_id.rs +++ b/rust/kernel/device_id.rs @@ -82,7 +82,7 @@ impl<T: RawDeviceId, U, const N: usize> IdArray<T, U, N> { unsafe { raw_ids[i] .as_mut_ptr()
.byte_offset(T::DRIVER_DATA_OFFSET as _)
.byte_add(T::DRIVER_DATA_OFFSET) .cast::<usize>() .write(i); }diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs index f7e8f5f53622..70d12014e476 100644 --- a/rust/kernel/devres.rs +++ b/rust/kernel/devres.rs @@ -45,7 +45,7 @@ struct DevresInner<T> { /// # Example /// /// ```no_run -/// # use kernel::{bindings, c_str, device::Device, devres::Devres, io::{Io, IoRaw}}; +/// # use kernel::{bindings, c_str, device::Device, devres::Devres, ffi::c_void, io::{Io, IoRaw}}; /// # use core::ops::Deref; /// /// // See also [`pci::Bar`] for a real example. @@ -59,19 +59,19 @@ struct DevresInner<T> { /// unsafe fn new(paddr: usize) -> Result<Self>{ /// // SAFETY: By the safety requirements of this function [`paddr`, `paddr` + `SIZE`) is /// // valid for `ioremap`. -/// let addr = unsafe { bindings::ioremap(paddr as _, SIZE as _) }; +/// let addr = unsafe { bindings::ioremap(paddr as bindings::phys_addr_t, SIZE) };
/// let addr = unsafe { bindings::ioremap(bindings::phys_addr_t::from(paddr), SIZE) };
better? Or even with .into()
/// let addr = unsafe { bindings::ioremap(paddr.into(), SIZE) };
This doesn't compile because `paddr` is usize, and `bindings::phys_addr_t` is u64 (on my machine, which is aarch64).
Ok, looks like Rust yet doesn't provide From/Into between usize and u64 even if the pointer size is fixed. Latest discussion can be found at:
https://github.com/rust-lang/rust/issues/41619#issuecomment-2056902943Lemme see if we can get an issue tracking this. Thanks for taking a look.
/// if addr.is_null() { /// return Err(ENOMEM); /// } /// -/// Ok(IoMem(IoRaw::new(addr as _, SIZE)?)) +/// Ok(IoMem(IoRaw::new(addr as usize, SIZE)?)) /// } /// } /// /// impl<const SIZE: usize> Drop for IoMem<SIZE> { /// fn drop(&mut self) { /// // SAFETY: `self.0.addr()` is guaranteed to be properly mapped by `Self::new`. -/// unsafe { bindings::iounmap(self.0.addr() as _); }; +/// unsafe { bindings::iounmap(self.0.addr() as *mut c_void); }; /// } /// } ///
[...]
diff --git a/rust/kernel/dma.rs b/rust/kernel/dma.rs index 43ecf3c2e860..851a6339aa90 100644 --- a/rust/kernel/dma.rs +++ b/rust/kernel/dma.rs @@ -38,7 +38,7 @@ impl Attrs { /// Get the raw representation of this attribute. pub(crate) fn as_raw(self) -> crate::ffi::c_ulong {
self.0 as _
self.0 as crate::ffi::c_ulongcrate::ffi::c_ulong::from(self.0)maybe, a C unsigned long should always be able to hold the whole `Attr` and a lossly casting is what this function does.
This also doesn't compile: "the trait `core::convert::From<u32>` is not implemented for `usize`". Upstream has ambitions of running on 16-bit, I guess :)
They do but they also have the target_pointer_width cfg, so they can totally provide these functions. It's just they want to find a better way (like the link I post above).
Did you want me to hold off on the respin on this point, or shall I go ahead?
No need to wait. This doesn't affect your current patch. But we do need to start making some decisions about how we handle the conversions *32 => *size, *size => *64 and c*long <=> *size, they should all be lossless in the context of kernel. We have 3 options:
1. Using `as`. 2. Having our own to_*size(*32), to_*64(*size), to_*size(*64), to_c*long(*size) and to_*size(c*long) helper functions. 3. Waiting and using what Rust upstream comes up.
I'm leaning towards to 2 and then 3, because using `as` can sometimes surprise you when you change the type. Thoughts?
Regards, Boqun