Re: [PATCH v3 04/16] rust: io: perform conversions using `AsRepr`

"Alexandre Courbot" <[email protected]>
Newsgroups dev.linux.lists.nova-gpu,dev.linux.lists.driver-core,org.freedesktop.lists.dri-devel,org.kernel.vger.linux-kernel,org.kernel.vger.linux-pci,org.kernel.vger.rust-for-linux
Message-ID <[email protected]>
On Wed Aug 19, 2026 at 8:09 PM JST, Gary Guo wrote:
> For types that are layout-compatible with an I/O capable type, we would
> want the ability to use them directly for I/O operations. E.g.
>
>     bitfield! {
>         pub struct Foo(u32) {
>             ...
>         }
>     }
>
>     #[repr(C)]
>     struct Bar {
>         foo: Foo,
>     }
>
>     let mmio: Mmio<'_, Bar> = ...;
>     io_read!(mmio, .foo)
>
> Currently this feature is available from `register!()` macro but not
> otherwise available with `io_read!`, `io_write!`. Support this by
> performing conversions to I/O primitives via the `AsRepr`/`AsReprMut`
> trait.
>
> This makes the `IoLoc::IoType` and `Register::Storage` redundant; thus
> remove them; also convert register methods to use the `read_val` and
> `write_val` instead.

Nice, the redundancy was bothering me a bit so this is clearly a better
design.

Reviewed-by: Alexandre Courbot <[email protected]>

>
> Signed-off-by: Gary Guo <[email protected]>
> ---
>  rust/kernel/bitfield.rs    |  10 ++++
>  rust/kernel/io.rs          | 135 +++++++++++++++++++++++++--------------------
>  rust/kernel/io/register.rs |  15 -----
>  rust/macros/io/register.rs |   2 -
>  4 files changed, 86 insertions(+), 76 deletions(-)
>
> diff --git a/rust/kernel/bitfield.rs b/rust/kernel/bitfield.rs
> index a0d089423f21..619c5e2189d1 100644
> --- a/rust/kernel/bitfield.rs
> +++ b/rust/kernel/bitfield.rs
> @@ -308,6 +308,7 @@ macro_rules! bitfield {
>          $(#[$attr])*
>          #[repr(transparent)]
>          #[derive(Clone, Copy, PartialEq, Eq)]
> +        #[derive($crate::prelude::FromBytes, $crate::prelude::IntoBytes)]

Do we need `FromBytes`/`IntoBytes` for every single bitfield type? I
mean that probably doesn't hurt, but if we need them for registers then
we can derive them from the register macro.

<...>
> @@ -498,10 +497,19 @@ fn try_cast<U>(self) -> Result<<Self::Backend as IoBackend>::View<'a, U>>
>      #[inline]
>      fn read_val(self) -> Self::Target
>      where
> -        Self::Backend: IoCapable<Self::Target>,
> -        Self::Target: Sized,
> +        Self::Target: AsReprMut,
> +        Self::Backend: IoCapable<<Self::Target as AsRepr>::Repr>,
>      {
> -        Self::Backend::io_read(self.as_view())
> +        let view = self.as_view();
> +        // SAFETY: `AsRepr` guarantees layout compatibility.
> +        let repr_view = unsafe {
> +            Self::Backend::project_view(
> +                view,
> +                Self::Backend::as_ptr(view).cast::<<Self::Target as AsRepr>::Repr>(),
> +            )
> +        };
> +
> +        Self::Target::from_repr(Self::Backend::io_read(repr_view))
>      }
>  
>      /// Write a value to I/O.
> @@ -520,10 +528,19 @@ fn read_val(self) -> Self::Target
>      #[inline]
>      fn write_val(self, value: Self::Target)
>      where
> -        Self::Backend: IoCapable<Self::Target>,
> -        Self::Target: Sized,
> +        Self::Target: AsRepr,
> +        Self::Backend: IoCapable<<Self::Target as AsRepr>::Repr>,
>      {
> -        Self::Backend::io_write(self.as_view(), value)
> +        let view = self.as_view();
> +        // SAFETY: `AsRepr` guarantees layout compatibility.
> +        let repr_view = unsafe {
> +            Self::Backend::project_view(
> +                view,
> +                Self::Backend::as_ptr(view).cast::<<Self::Target as AsRepr>::Repr>(),
> +            )
> +        };

Can we factorize the two `unsafe` blocks of `read_val` and `write_val`
into a common private helper?
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.