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

[Danilo Krummrich]

On Fri, Feb 28, 2025 at 05:35:26PM +0900, Alexandre Courbot wrote:
> On Tue Feb 25, 2025 at 1:27 AM JST, Abdiel Janulgue wrote:
> >
> > 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.
> 
> FWIW I was about to make the same comment as Andreas. The mask is set on
> a Device, it should thus be part of its implementation.

Yes, this should be Device methods. Please also add them in a separate commit.

> You can still
> keep them with the rest of the DMA API in this file by just adding an
> `impl Device` block here - since Device resides in the same crate, it is
> allowed.

Eventually, the build system will support to move rust code to the corresponding
subsystem entires in separate crates.