Re: [PATCH v3 1/9] dmaengine: Support bus widths of 32 bytes and above

[Andy Shevchenko]

On Mon, Aug 31, 2026 at 12:46:38PM +0100, Nuno Sá wrote:
> The src_addr_widths and dst_addr_widths capability masks encode each
> supported width as a bit whose position equals the corresponding
> enum dma_slave_buswidth value (e.g. DMA_SLAVE_BUSWIDTH_4_BYTES sets bit
> 4). As these masks are plain u32, widths of 32 bytes and above
> (DMA_SLAVE_BUSWIDTH_32/64/128_BYTES map to bits 32, 64 and 128) cannot
> be represented at all.
> 
> Introduce bitmap-based bus width capabilities that span the full enum
> range, through a new dma_buswidth_mask_t type modeled after
> dma_cap_mask_t. To allow DMA controller drivers to be converted
> incrementally, the legacy dma_device u32 fields are kept alongside the
> new masks and the core folds a legacy-only driver's u32 into the mask
> when the device is registered, so consumers only ever have to look at
> the mask.
> 
> The new interface lives in two new headers under a new
> include/linux/dma/engine/ directory instead of growing
> linux/dmaengine.h, which is included nearly everywhere:
> 
>  - dma/engine/types.h holds enum dma_slave_buswidth and the new
>    dma_buswidth_mask_t type. Like dma_cap_mask_t, the type only needs
>    DECLARE_BITMAP();
> 
>  - dma/engine/widthmask.h holds the accessors which are based on the new
>    dma_buswidth_mask_t type. This gives us freedom to change the core
>    without affecting consumers as they only see (and should only use) the
>    new type.
> 
> Note the fold only has to happen in one direction on the producer side:
> nothing outside a controller driver reads the legacy dma_device fields,
> so a converted driver's mask is not mirrored back into them. The legacy
> dma_slave_caps fields are different, as consumers not converted yet
> still read them: dma_get_slave_caps() derives them from the mask when a
> device_caps() callback adjusted it. Both go away with the legacy fields.

...

>  F:	drivers/dma/
>  F:	include/dt-bindings/dma/
>  F:	include/linux/dma/
> +F:	include/linux/dma/engine/
>  F:	include/linux/dmaengine.h
>  F:	include/linux/of_dma.h

Unneeded, previous entry includes recursively.

...

> +/*
> + * Basic types shared by the DMA engine interfaces.
> + */
> +#ifndef LINUX_DMA_ENGINE_TYPES_H
> +#define LINUX_DMA_ENGINE_TYPES_H
> +
> +#include <linux/bitops.h>

Not yet? Perhaps next changes will use it, then they can add it.

> +#include <linux/types.h>
> +
> +/**
> + * enum dma_slave_buswidth - defines bus width of the DMA slave
> + * device, source or target buses

I would describe the _UNDEFINED case, it might require some clarification on
what behaviour is to expect with this one.

> + */

> +enum dma_slave_buswidth {
> +	DMA_SLAVE_BUSWIDTH_UNDEFINED = 0,
> +	DMA_SLAVE_BUSWIDTH_1_BYTE = 1,
> +	DMA_SLAVE_BUSWIDTH_2_BYTES = 2,
> +	DMA_SLAVE_BUSWIDTH_3_BYTES = 3,
> +	DMA_SLAVE_BUSWIDTH_4_BYTES = 4,
> +	DMA_SLAVE_BUSWIDTH_8_BYTES = 8,
> +	DMA_SLAVE_BUSWIDTH_16_BYTES = 16,
> +	DMA_SLAVE_BUSWIDTH_32_BYTES = 32,
> +	DMA_SLAVE_BUSWIDTH_64_BYTES = 64,
> +	DMA_SLAVE_BUSWIDTH_128_BYTES = 128,
> +	DMA_SLAVE_BUSWIDTH_MAX
> +};

> +/**
> + * typedef dma_buswidth_mask_t - bus width capabilities bitmap modeled after
> + * dma_cap_mask_t.
> + *
> + * Each supported bus width is represented by the bit whose position equals the
> + * corresponding enum dma_slave_buswidth value, e.g. a device supporting a bus
> + * width of 4 bytes has bit 4 set.
> + */
> +typedef struct {
> +	DECLARE_BITMAP(bits, DMA_SLAVE_BUSWIDTH_MAX);
> +} dma_buswidth_mask_t;
> +
> +#endif /* LINUX_DMA_ENGINE_TYPES_H */

...

> +/*
> + * Bus width capabilities of DMA engine devices and channels.
> + */
> +#ifndef LINUX_DMA_ENGINE_WIDTHMASK_H
> +#define LINUX_DMA_ENGINE_WIDTHMASK_H
> +
> +#include <linux/bitmap.h>
> +#include <linux/dma/engine/types.h>

+ errno.h

> +#include <linux/types.h>

I would group subsystem ones.

#include <linux/bitmap.h>
#include <linux/errno.h>
#include <linux/types.h>

#include <linux/dma/engine/types.h>

...

> +static inline enum dma_slave_buswidth
> +__dma_bus_width_min(const dma_buswidth_mask_t *mask)
> +{
> +	enum dma_slave_buswidth width = find_first_bit(mask->bits,
> +						       DMA_SLAVE_BUSWIDTH_MAX);
> +

For easier maintenance better to split the assignment.

	enum dma_slave_buswidth width;

	width = find_first_bit(mask->bits, DMA_SLAVE_BUSWIDTH_MAX);

> +	if (width == DMA_SLAVE_BUSWIDTH_MAX)
> +		return DMA_SLAVE_BUSWIDTH_UNDEFINED;
> +
> +	return width;
> +}

...

>  #ifndef LINUX_DMAENGINE_H
>  #define LINUX_DMAENGINE_H
>  
> +#include <linux/bitops.h>
>  #include <linux/device.h>

> +#include <linux/dma/engine/types.h>

Same, group them after generic linux/*.h.

>  #include <linux/err.h>
>  #include <linux/uio.h>
>  #include <linux/bug.h>

...

It's possible to split this patch to two:
- move the existing type into a new types.h header
- add support for the new API

-- 
With Best Regards,
Andy Shevchenko