Re: [PATCH v14 09/11] rust: dma: use `dma::Device` in `CoherentAllocation`

[Andreas Hindborg]

Abdiel Janulgue <abdiel.janulgue@xxxxxxxxx> writes:

> From: Danilo Krummrich <dakr@xxxxxxxxxx>
>
> Require a `&dyn dma::Device` in `CoherentAllocation`, such that DMA
> memory can only be allocated with devices on potentially DMA capable
> busses.
>
> Signed-off-by: Danilo Krummrich <dakr@xxxxxxxxxx>
> Signed-off-by: Abdiel Janulgue <abdiel.janulgue@xxxxxxxxx>
> ---
>  rust/kernel/dma.rs       | 16 ++++++++--------
>  samples/rust/rust_dma.rs |  2 +-
>  2 files changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/rust/kernel/dma.rs b/rust/kernel/dma.rs
> index ac3ec0042327..d0c6569cfc70 100644
> --- a/rust/kernel/dma.rs
> +++ b/rust/kernel/dma.rs
> @@ -57,10 +57,9 @@ fn dma_set_mask(&mut self, mask: u64) -> Result {
>  /// # Examples
>  ///
>  /// ```
> -/// use kernel::device::Device;
> -/// use kernel::dma::{attrs::*, CoherentAllocation};
> +/// use kernel::dma::{attrs::*, Device, CoherentAllocation};
>  ///
> -/// # fn test(dev: &Device) -> Result {
> +/// # fn test(dev: &dyn Device) -> Result {
>  /// let attribs = DMA_ATTR_FORCE_CONTIGUOUS | DMA_ATTR_NO_WARN;
>  /// let c: CoherentAllocation<u64> =
>  ///     CoherentAllocation::alloc_attrs(dev, 4, GFP_KERNEL, attribs)?;
> @@ -178,16 +177,15 @@ impl<T: AsBytes + FromBytes> CoherentAllocation<T> {
>      /// # Examples
>      ///
>      /// ```
> -    /// use kernel::device::Device;
> -    /// use kernel::dma::{attrs::*, CoherentAllocation};
> +    /// use kernel::dma::{attrs::*, Device, CoherentAllocation};
>      ///
> -    /// # fn test(dev: &Device) -> Result {
> +    /// # fn test(dev: &dyn Device) -> Result {
>      /// let c: CoherentAllocation<u64> =
>      ///     CoherentAllocation::alloc_attrs(dev, 4, GFP_KERNEL, DMA_ATTR_NO_WARN)?;
>      /// # Ok::<(), Error>(()) }
>      /// ```
>      pub fn alloc_attrs(
> -        dev: &device::Device,
> +        dev: &dyn Device,
>          count: usize,
>          gfp_flags: kernel::alloc::Flags,
>          dma_attrs: Attrs,
> @@ -197,6 +195,8 @@ pub fn alloc_attrs(
>              "It doesn't make sense for the allocated type to be a ZST"
>          );
>  
> +        let dev = dev.as_ref();
> +
>          let size = count
>              .checked_mul(core::mem::size_of::<T>())
>              .ok_or(EOVERFLOW)?;
> @@ -229,7 +229,7 @@ pub fn alloc_attrs(
>      /// Performs the same functionality as [`CoherentAllocation::alloc_attrs`], except the
>      /// `dma_attrs` is 0 by default.
>      pub fn alloc_coherent(
> -        dev: &device::Device,
> +        dev: &dyn Device,

Is it possible (and would it make sense?) to make these methods, or even
`CoherentAllocation` generic over the `D: Device` to get rid of the
dynamic dispatch:

    pub fn alloc_coherent(
        dev: &impl Device,
        count: usize,
        gfp_flags: kernel::alloc::Flags,
    ) -> Result<CoherentAllocation<T>> {


Best regards,
Andreas Hindborg