mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Daniel Almeida <daniel.almeida@collabora.com>
To: Danilo Krummrich <dakr@kernel.org>
Cc: akpm@linux-foundation.org, ojeda@kernel.org,
	alex.gaynor@gmail.com, boqun.feng@gmail.com, gary@garyguo.net,
	bjorn3_gh@protonmail.com, lossin@kernel.org,
	a.hindborg@kernel.org, aliceryhl@google.com, tmgross@umich.edu,
	abdiel.janulgue@gmail.com, acourbot@nvidia.com, jgg@ziepe.ca,
	lyude@redhat.com, robin.murphy@arm.com,
	rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 2/5] rust: dma: add type alias for bindings::dma_addr_t
Date: Tue, 26 Aug 2025 14:15:04 -0300	[thread overview]
Message-ID: <CE0E1F73-69CE-4629-979E-9AE5B6AB055E@collabora.com> (raw)
In-Reply-To: <20250825132539.122412-3-dakr@kernel.org>



> On 25 Aug 2025, at 10:24, Danilo Krummrich <dakr@kernel.org> wrote:
> 
> Add a type alias for bindings::dma_addr_t (DmaAddress), such that we do
> not have to access bindings directly.
> 
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
> Suggested-by: Alice Ryhl <aliceryhl@google.com>
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> ---
> drivers/gpu/nova-core/falcon.rs |  4 ++--
> rust/kernel/dma.rs              | 18 ++++++++++++++----
> 2 files changed, 16 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/nova-core/falcon.rs b/drivers/gpu/nova-core/falcon.rs
> index 50437c67c14a..aa36ed8c04ed 100644
> --- a/drivers/gpu/nova-core/falcon.rs
> +++ b/drivers/gpu/nova-core/falcon.rs
> @@ -4,8 +4,8 @@
> 
> use core::ops::Deref;
> use hal::FalconHal;
> -use kernel::bindings;
> use kernel::device;
> +use kernel::dma::DmaAddress;
> use kernel::prelude::*;
> use kernel::time::Delta;
> use kernel::types::ARef;
> @@ -443,7 +443,7 @@ fn dma_wr<F: FalconFirmware<Target = E>>(
>                 fw.dma_handle_with_offset(load_offsets.src_start as usize)?,
>             ),
>         };
> -        if dma_start % bindings::dma_addr_t::from(DMA_LEN) > 0 {
> +        if dma_start % DmaAddress::from(DMA_LEN) > 0 {
>             dev_err!(
>                 self.dev,
>                 "DMA transfer start addresses must be a multiple of {}",
> diff --git a/rust/kernel/dma.rs b/rust/kernel/dma.rs
> index 27b25f041f32..b2a6282876da 100644
> --- a/rust/kernel/dma.rs
> +++ b/rust/kernel/dma.rs
> @@ -13,6 +13,16 @@
>     types::ARef,
> };
> 
> +/// DMA address type.
> +///
> +/// Represents a bus address used for Direct Memory Access (DMA) operations.
> +///
> +/// This is an alias of the kernel's `dma_addr_t`, which may be `u32` or `u64` depending on
> +/// `CONFIG_ARCH_DMA_ADDR_T_64BIT`.
> +///
> +/// Note that this may be `u64` even on 32-bit architectures.
> +pub type DmaAddress = bindings::dma_addr_t;
> +
> /// Trait to be implemented by DMA capable bus devices.
> ///
> /// The [`dma::Device`](Device) trait should be implemented by bus specific device representations,
> @@ -343,7 +353,7 @@ fn from(direction: DataDirection) -> Self {
> // entire `CoherentAllocation` including the allocated memory itself.
> pub struct CoherentAllocation<T: AsBytes + FromBytes> {
>     dev: ARef<device::Device>,
> -    dma_handle: bindings::dma_addr_t,
> +    dma_handle: DmaAddress,
>     count: usize,
>     cpu_addr: *mut T,
>     dma_attrs: Attrs,
> @@ -444,7 +454,7 @@ pub fn start_ptr_mut(&mut self) -> *mut T {
> 
>     /// Returns a DMA handle which may be given to the device as the DMA address base of
>     /// the region.
> -    pub fn dma_handle(&self) -> bindings::dma_addr_t {
> +    pub fn dma_handle(&self) -> DmaAddress {
>         self.dma_handle
>     }
> 
> @@ -452,13 +462,13 @@ pub fn dma_handle(&self) -> bindings::dma_addr_t {
>     /// device as the DMA address base of the region.
>     ///
>     /// Returns `EINVAL` if `offset` is not within the bounds of the allocation.
> -    pub fn dma_handle_with_offset(&self, offset: usize) -> Result<bindings::dma_addr_t> {
> +    pub fn dma_handle_with_offset(&self, offset: usize) -> Result<DmaAddress> {
>         if offset >= self.count {
>             Err(EINVAL)
>         } else {
>             // INVARIANT: The type invariant of `Self` guarantees that `size_of::<T> * count` fits
>             // into a `usize`, and `offset` is inferior to `count`.
> -            Ok(self.dma_handle + (offset * core::mem::size_of::<T>()) as bindings::dma_addr_t)
> +            Ok(self.dma_handle + (offset * core::mem::size_of::<T>()) as DmaAddress)
>         }
>     }
> 
> -- 
> 2.51.0
> 

Hmm, I wonder if this shouldn’t be its own type, instead of an alias. This
will be handy if we want to enforce that a given address is, in fact, a bus
address.

In any case, this can be a separate patch. This one is good.


Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>

  reply	other threads:[~2025-08-26 17:15 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-25 13:24 [PATCH v3 0/5] Rust infrastructure for sg_table and scatterlist Danilo Krummrich
2025-08-25 13:24 ` [PATCH v3 1/5] rust: dma: implement DataDirection Danilo Krummrich
2025-08-26 17:10   ` Daniel Almeida
2025-08-25 13:24 ` [PATCH v3 2/5] rust: dma: add type alias for bindings::dma_addr_t Danilo Krummrich
2025-08-26 17:15   ` Daniel Almeida [this message]
2025-08-26 17:33     ` Danilo Krummrich
2025-08-26 19:58       ` Daniel Almeida
2025-08-25 13:24 ` [PATCH v3 3/5] rust: scatterlist: Add abstraction for sg_table Danilo Krummrich
2025-08-26 14:16   ` Alice Ryhl
2025-08-26 14:32     ` Danilo Krummrich
2025-08-26 17:41       ` Daniel Almeida
2025-08-26 14:36   ` Alexandre Courbot
2025-08-26 15:18     ` Danilo Krummrich
2025-08-26 17:45       ` Daniel Almeida
2025-08-26 23:38       ` Danilo Krummrich
2025-08-27  8:30       ` Alexandre Courbot
2025-08-26 17:40   ` Daniel Almeida
2025-08-26 19:13     ` Danilo Krummrich
2025-08-26 20:16       ` Daniel Almeida
2025-08-26 20:27         ` Danilo Krummrich
2025-08-25 13:24 ` [PATCH v3 4/5] samples: rust: dma: add sample code for SGTable Danilo Krummrich
2025-08-26 14:38   ` Alexandre Courbot
2025-08-26 17:46   ` Daniel Almeida
2025-08-25 13:24 ` [PATCH v3 5/5] MAINTAINERS: rust: dma: add scatterlist files Danilo Krummrich
2025-08-28 10:19   ` Miguel Ojeda
2025-08-26 21:01 ` [PATCH v3 0/5] Rust infrastructure for sg_table and scatterlist Lyude Paul

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CE0E1F73-69CE-4629-979E-9AE5B6AB055E@collabora.com \
    --to=daniel.almeida@collabora.com \
    --cc=a.hindborg@kernel.org \
    --cc=abdiel.janulgue@gmail.com \
    --cc=acourbot@nvidia.com \
    --cc=akpm@linux-foundation.org \
    --cc=alex.gaynor@gmail.com \
    --cc=aliceryhl@google.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun.feng@gmail.com \
    --cc=dakr@kernel.org \
    --cc=gary@garyguo.net \
    --cc=jgg@ziepe.ca \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lossin@kernel.org \
    --cc=lyude@redhat.com \
    --cc=ojeda@kernel.org \
    --cc=robin.murphy@arm.com \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=tmgross@umich.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®