On Thu, Apr 17, 2025 at 4:12 PM Boqun Feng <boqun.f...@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-2056902943
>
> Lemme 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_ulong
> > >
> > >         crate::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?

Reply via email to