Re: [PATCH v12 2/3] rust: add dma coherent allocator abstraction.

[Abdiel Janulgue]

On 24/02/2025 16:40, Andreas Hindborg wrote:
> "Abdiel Janulgue" <abdiel.janulgue@xxxxxxxxx> writes:
>
> [...]
>
> > +/// Inform the kernel about the device's DMA addressing capabilities. This will set the mask for
> > +/// both streaming and coherent APIs together.
> > +pub fn dma_set_mask_and_coherent(dev: &Device, mask: u64) -> i32 {
> > +    // SAFETY: device pointer is guaranteed as valid by invariant on `Device`.
> > +    unsafe { bindings::dma_set_mask_and_coherent(dev.as_raw(), mask) }
> > +}
> > +
> > +/// Same as `dma_set_mask_and_coherent`, but set the mask only for streaming mappings.
> > +pub fn dma_set_mask(dev: &Device, mask: u64) -> i32 {
> > +    // SAFETY: device pointer is guaranteed as valid by invariant on `Device`.
> > +    unsafe { bindings::dma_set_mask(dev.as_raw(), mask) }
> > +}
>
> Sorry if it was asked before, I am late to the party. But would it make
> sense to put these to functions on `Device` and make them take `&self`.

Thanks for checking this. The API is about the dma addressing 
capabalities of the device, my thoughts would be to group them with the 
rest of the dma API? But either way, I don't have a strong preference. 
I'll let others comment.

Daniel, Danilo?

Regards,
Abdiel

> > +
> > +/// Possible attributes associated with a DMA mapping.
> > +///
> > +/// They can be combined with the operators `|`, `&`, and `!`.
> > +///
> > +/// Values can be used from the [`attrs`] module.
> > +#[derive(Clone, Copy, PartialEq)]
> > +#[repr(transparent)]
> > +pub struct Attrs(u32);
> > +
> > +impl Attrs {
> > +    /// Get the raw representation of this attribute.
> > +    pub(crate) fn as_raw(self) -> crate::ffi::c_ulong {
> > +        self.0 as _
> > +    }
> > +
> > +    /// Check whether `flags` is contained in `self`.
> > +    pub fn contains(self, flags: Attrs) -> bool {
> > +        (self & flags) == flags
> > +    }
> > +}
> > +
> > +impl core::ops::BitOr for Attrs {
> > +    type Output = Self;
> > +    fn bitor(self, rhs: Self) -> Self::Output {
> > +        Self(self.0 | rhs.0)
> > +    }
> > +}
> > +
> > +impl core::ops::BitAnd for Attrs {
> > +    type Output = Self;
> > +    fn bitand(self, rhs: Self) -> Self::Output {
> > +        Self(self.0 & rhs.0)
> > +    }
> > +}
> > +
> > +impl core::ops::Not for Attrs {
> > +    type Output = Self;
> > +    fn not(self) -> Self::Output {
> > +        Self(!self.0)
> > +    }
> > +}
> > +
> > +/// DMA mapping attrributes.
>
> Typo in attributes.
>
>
> Best regards,
> Andreas Hindborg