All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Danilo Krummrich" <dakr@kernel.org>
To: "Daniel Almeida" <daniel.almeida@collabora.com>
Cc: <abdiel.janulgue@gmail.com>, <robin.murphy@arm.com>,
	<a.hindborg@kernel.org>, <ojeda@kernel.org>,
	<alex.gaynor@gmail.com>, <boqun.feng@gmail.com>,
	<gary@garyguo.net>, <bjorn3_gh@protonmail.com>,
	<lossin@kernel.org>, <aliceryhl@google.com>, <tmgross@umich.edu>,
	<bhelgaas@google.com>, <kwilczynski@kernel.org>,
	<gregkh@linuxfoundation.org>, <rafael@kernel.org>,
	<rust-for-linux@vger.kernel.org>, <linux-pci@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2 2/5] rust: dma: add DMA addressing capabilities
Date: Wed, 16 Jul 2025 23:13:11 +0200	[thread overview]
Message-ID: <DBDSFO1LGSYM.VFQKKLN6BX3H@kernel.org> (raw)
In-Reply-To: <DBDP0BJW9VAZ.5KRU4V4288R8@kernel.org>

On Wed Jul 16, 2025 at 8:32 PM CEST, Danilo Krummrich wrote:
> On Wed Jul 16, 2025 at 7:55 PM CEST, Danilo Krummrich wrote:
>> On Wed Jul 16, 2025 at 7:32 PM CEST, Daniel Almeida wrote:
>>> Hi Danilo,
>>>
>>>> +    #[inline]
>>>> +    pub const fn new(n: usize) -> Result<Self> {
>>>> +        Ok(Self(match n {
>>>> +            0 => 0,
>>>> +            1..=64 => u64::MAX >> (64 - n),
>>>> +            _ => return Err(EINVAL),
>>>> +        }))
>>>> +    }
>>>> +
>>>
>>> Isn’t this equivalent to genmask_u64(0..=n) ? See [0].
>>
>> Instead of the match this can use genmask_checked_u64() and convert the Option
>> to a Result, once genmask is upstream.
>>
>>> You should also get a compile-time failure if n is out of bounds by default using
>>> genmask.
>>
>> No, we can't use genmask_u64(), `n` is not guaranteed to be known at compile
>> time, so we'd need to use genmask_checked_u64().
>>
>> Of course, we could have a separate DmaMask constructor, e.g. with a const
>> generic -- not sure that's worth though.
>
> On the other hand, it doesn't hurt. Guess I will add another constructor with a
> const generic. :)

diff --git a/rust/kernel/dma.rs b/rust/kernel/dma.rs
index afd3ba538e3c..ad69ef316295 100644
--- a/rust/kernel/dma.rs
+++ b/rust/kernel/dma.rs
@@ -81,25 +81,6 @@ unsafe fn dma_set_mask_and_coherent(&self, mask: DmaMask) -> Result {
 /// never exceed the bit width of `u64`.
 ///
 /// This is the Rust equivalent of the C macro `DMA_BIT_MASK()`.
-///
-/// # Examples
-///
-/// ```
-/// use kernel::dma::DmaMask;
-///
-/// let mask0 = DmaMask::new(0)?;
-/// assert_eq!(mask0.value(), 0);
-///
-/// let mask1 = DmaMask::new(1)?;
-/// assert_eq!(mask1.value(), 0b1);
-///
-/// let mask64 = DmaMask::new(64)?;
-/// assert_eq!(mask64.value(), u64::MAX);
-///
-/// let mask_overflow = DmaMask::new(100);
-/// assert!(mask_overflow.is_err());
-/// # Ok::<(), Error>(())
-/// ```
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub struct DmaMask(u64);

@@ -108,8 +89,27 @@ impl DmaMask {
     ///
     /// For `n <= 64`, sets exactly the lowest `n` bits.
     /// For `n > 64`, returns [`EINVAL`].
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// use kernel::dma::DmaMask;
+    ///
+    /// let mask0 = DmaMask::try_new(0)?;
+    /// assert_eq!(mask0.value(), 0);
+    ///
+    /// let mask1 = DmaMask::try_new(1)?;
+    /// assert_eq!(mask1.value(), 0b1);
+    ///
+    /// let mask64 = DmaMask::try_new(64)?;
+    /// assert_eq!(mask64.value(), u64::MAX);
+    ///
+    /// let mask_overflow = DmaMask::try_new(100);
+    /// assert!(mask_overflow.is_err());
+    /// # Ok::<(), Error>(())
+    /// ```
     #[inline]
-    pub const fn new(n: usize) -> Result<Self> {
+    pub const fn try_new(n: u32) -> Result<Self> {
         Ok(Self(match n {
             0 => 0,
             1..=64 => u64::MAX >> (64 - n),
@@ -117,6 +117,38 @@ pub const fn new(n: usize) -> Result<Self> {
         }))
     }

+    /// Constructs a `DmaMask` with the lowest `n` bits set to `1`.
+    ///
+    /// For `n <= 64`, sets exactly the lowest `n` bits.
+    /// For `n > 64`, results in a build error.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// use kernel::dma::DmaMask;
+    /// use kernel::bits::genmask_u64;
+    ///
+    /// let mask0 = DmaMask::new::<0>();
+    /// assert_eq!(mask0.value(), 0);
+    ///
+    /// let mask1 = DmaMask::new::<1>();
+    /// assert_eq!(mask1.value(), 0b1);
+    ///
+    /// let mask64 = DmaMask::new::<64>();
+    /// assert_eq!(mask64.value(), u64::MAX);
+    ///
+    /// // Build failure.
+    /// // let mask_overflow = DmaMask::new::<100>();
+    /// ```
+    #[inline(always)]
+    pub const fn new<const N: u32>() -> Self {
+        let Ok(mask) = Self::try_new(N) else {
+            build_error!("Invalid DMA Mask.");
+        };
+
+        mask
+    }
+
     /// Returns the underlying `u64` bitmask value.
     #[inline]
     pub const fn value(&self) -> u64 {
diff --git a/samples/rust/rust_dma.rs b/samples/rust/rust_dma.rs
index 9422ac68c139..c5e7cce68654 100644
--- a/samples/rust/rust_dma.rs
+++ b/samples/rust/rust_dma.rs
@@ -58,7 +58,7 @@ impl pci::Driver for DmaSampleDriver {
     fn probe(pdev: &pci::Device<Core>, _info: &Self::IdInfo) -> Result<Pin<KBox<Self>>> {
         dev_info!(pdev.as_ref(), "Probe DMA test driver.\n");

-        let mask = DmaMask::new(64)?;
+        let mask = DmaMask::new::<64>();

         // SAFETY: There are no concurrent calls to DMA allocation and mapping primitives.
         unsafe { pdev.dma_set_mask_and_coherent(mask)? };


  reply	other threads:[~2025-07-16 21:13 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-16 15:02 [PATCH v2 0/5] dma::Device trait and DMA mask Danilo Krummrich
2025-07-16 15:02 ` [PATCH v2 1/5] rust: dma: implement `dma::Device` trait Danilo Krummrich
2025-07-16 15:02 ` [PATCH v2 2/5] rust: dma: add DMA addressing capabilities Danilo Krummrich
2025-07-16 17:32   ` Daniel Almeida
2025-07-16 17:41     ` Daniel Almeida
2025-07-16 17:55     ` Danilo Krummrich
2025-07-16 18:32       ` Danilo Krummrich
2025-07-16 21:13         ` Danilo Krummrich [this message]
2025-07-16 22:19           ` Daniel Almeida
2025-07-16 22:32             ` Danilo Krummrich
2025-07-16 15:02 ` [PATCH v2 3/5] rust: pci: implement the `dma::Device` trait Danilo Krummrich
2025-07-16 15:02 ` [PATCH v2 4/5] rust: platform: " Danilo Krummrich
2025-07-16 15:02 ` [PATCH v2 5/5] rust: samples: dma: set DMA mask Danilo Krummrich
2025-07-16 17:28 ` [PATCH v2 0/5] dma::Device trait and " Greg KH
2025-07-16 17:34 ` Daniel Almeida
2025-07-20 14:33 ` Danilo Krummrich

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=DBDSFO1LGSYM.VFQKKLN6BX3H@kernel.org \
    --to=dakr@kernel.org \
    --cc=a.hindborg@kernel.org \
    --cc=abdiel.janulgue@gmail.com \
    --cc=alex.gaynor@gmail.com \
    --cc=aliceryhl@google.com \
    --cc=bhelgaas@google.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun.feng@gmail.com \
    --cc=daniel.almeida@collabora.com \
    --cc=gary@garyguo.net \
    --cc=gregkh@linuxfoundation.org \
    --cc=kwilczynski@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=lossin@kernel.org \
    --cc=ojeda@kernel.org \
    --cc=rafael@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.