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

[Alexandre Courbot]

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. 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.