Re: [PATCH v2] rust: io: use newtype for PhysAddr

["Danilo Krummrich" <dakr@kernel.org>]

Hi Favilances,

(I assume you are Favilances; asking since the mail came from just x@2005.tr.)

On Mon May 4, 2026 at 1:14 PM CEST, x wrote:
> From: Favilances Noir <favilances@proton.me>
>
> Currently, PhysAddr is a type alias for phys_addr_t. This means
> that physical addresses can be mixed with unrelated integer values
> without Rust noticing.
>
> Use a transparent newtype for PhysAddr instead, and update the
> affected call sites to perform explicit raw conversions at the FFI
> boundary.
>
> This keeps the existing phys_addr_t representation while making
> physical addresses distinct in Rust type checking.
>
> Drop the unused Clone and Copy derives from PhysAddr, as pointed out
> in review.

This should be part of the version change log and should go below the "---"
line.

Please also consider the subsequent replies to this feedback, which point out
that this should indeed have Clone and Copy (and probably also other derives
such as PartialEq).

For instance, consider the case where you call into_raw() because you need the
raw value but then still want to do something with the PhysAddr. Without Copy
you'd need to write something like:

	let r = p.into_raw();
	let p = PhysAddr::from_raw(r); // `r` implements `Copy`.

	// Use `r` and `p`.

> Signed-off-by: Favilances Noir <favilances@proton.me>

<snip>

> diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
> index fcc7678fd9e3..999fa285a6ec 100644
> --- a/rust/kernel/io.rs
> +++ b/rust/kernel/io.rs
> @@ -21,9 +21,24 @@
>  
>  /// Physical address type.
>  ///
> -/// This is a type alias to either `u32` or `u64` depending on the config option
> -/// `CONFIG_PHYS_ADDR_T_64BIT`, and it can be a u64 even on 32-bit architectures.
> -pub type PhysAddr = bindings::phys_addr_t;
> +/// This wraps either `u32` or `u64` depending on the config option
> +/// `CONFIG_PHYS_ADDR_T_64BIT`, and it can be a `u64` even on 32-bit architectures.
> +#[repr(transparent)]
> +pub struct PhysAddr(bindings::phys_addr_t);
> +
> +impl PhysAddr {
> +    /// Creates a physical address from the raw C type.
> +    #[inline]
> +    pub const fn from_raw(value: bindings::phys_addr_t) -> Self {
> +        Self(value)
> +    }
> +
> +    /// Turns this physical address into the raw C type.
> +    #[inline]
> +    pub const fn into_raw(self) -> bindings::phys_addr_t {
> +        self.0
> +    }
> +}

I like the explicit methods, but I can also see that this might become a bit
verbose; maybe consider adding From impls as well.

I think eventually we also want {Try}From impls for usize etc., but let's wait
until we have a user.

Maybe also consider a Debug impl that prints a hex value.