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

On Wed, Aug 12, 2026 at 10:08:34AM +0100, Nuno Sá wrote:
> On Tue, Aug 11, 2026 at 11:28:04PM +0530, Vinod Koul wrote:
> > On 11-08-26, 09:46, Nuno Sá wrote:
> > > On Mon, Aug 10, 2026 at 08:15:28PM +0300, Andy Shevchenko wrote:
> > > > On Mon, Aug 10, 2026 at 04:06:42PM +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. To allow DMA controller producers to be converted incrementally,
> > > > > keep the legacy dma_device u32 fields alongside the new bitmaps:
> > > > > producers using the new helpers populate the bitmap and mirror the low
> > > > > 32 bits back into the legacy field, while dma_get_slave_caps() folds a
> > > > > legacy-only producer's u32 into the returned bitmap.
> > > > > 
> > > > > Add helpers for producers and consumers so users do not need to depend
> > > > > on the bitmap layout directly. Once the remaining producers are
> > > > > converted, the legacy dma_device u32 fields can be dropped.
> > > > 
> > > > ...
> > > > 
> > > > > +++ b/include/linux/dmaengine.h
> > > > 
> > > > >  #ifndef LINUX_DMAENGINE_H
> > > > >  #define LINUX_DMAENGINE_H
> > > > >  
> > > > > +#include <linux/bitops.h>
> > > > 
> > > > Ah, this is unfortunate, this is a wrong header, the correct one is bitmap.h
> > > > and I think we may not include it here (see below on why).
> > > > 
> > > > >  #include <linux/device.h>
> > > > >  #include <linux/err.h>
> > > > >  #include <linux/uio.h>
> > > > 
> > > > ...
> > > > 
> > > > > +static inline enum dma_slave_buswidth
> > > > > +__dma_slave_caps_get_width_min(const unsigned long *bus_widths)
> > > > > +{
> > > > > +	enum dma_slave_buswidth width = find_first_bit(bus_widths,
> > > > > +						       DMA_SLAVE_BUSWIDTH_MAX);
> > > > 
> > > > This is from find.h which is internals of bitmap.h.
> > > > 
> > > > > +	if (width == DMA_SLAVE_BUSWIDTH_MAX)
> > > > > +		return DMA_SLAVE_BUSWIDTH_UNDEFINED;
> > > > > +
> > > > > +	return width;
> > > > > +}
> > > > 
> > > > The (big) problem is quite a header dependencies hell we have. All my cleanup
> > > > work of kernel.h I started on the simplest thing I wanted, id est to make
> > > > bitmap_zalloc() and similar to be static inlines. But it's impossible to achieve
> > > > (and I think that no one, except may be Ingo, see his 2000+ patch series a few
> > > >  years back, is capable of fix that at once). That's why having bitmap.h in the
> > > > kernel wide public _header_ is bad, bad idea (at least at the current state of
> > > > affairs). So, make it exported function instead and keep bitmap.h in dmaengine.c.
> > > > 
> > > > ...
> > > > 
> > > > > +/**
> > > > > + * dma_slave_caps_copy_src_widths - copy source bus width capabilities
> > > > > + * @caps: DMA slave capabilities
> > > > > + * @bus_widths: destination bitmap declared with DECLARE_DMA_BUS_WIDTHS()
> > > > > + */
> > > > > +static inline void
> > > > > +dma_slave_caps_copy_src_widths(const struct dma_slave_caps *caps,
> > > > > +			       unsigned long *bus_widths)
> > > > > +{
> > > > > +	bitmap_copy(bus_widths, caps->src_bus_widths, DMA_SLAVE_BUSWIDTH_MAX);
> > > > > +}
> > > > 
> > > > > +/**
> > > > > + * dma_slave_caps_copy_dst_widths - copy destination bus width capabilities
> > > > > + * @caps: DMA slave capabilities
> > > > > + * @bus_widths: destination bitmap declared with DECLARE_DMA_BUS_WIDTHS()
> > > > > + */
> > > > > +static inline void
> > > > > +dma_slave_caps_copy_dst_widths(const struct dma_slave_caps *caps,
> > > > > +			       unsigned long *bus_widths)
> > > > > +{
> > > > > +	bitmap_copy(bus_widths, caps->dst_bus_widths, DMA_SLAVE_BUSWIDTH_MAX);
> > > > > +}
> > > > 
> > > > As per above.
> > > > 
> > > > ...
> > > > 
> > > > Another (compromise approach) is to split the header that includes bitmap.h to
> > > > something like dmaengine-width.h, but I don't know how spread this use is. Do
> > > > we have all the users of the current dmaengine.h to use these APIs? If not,
> > > > split, if yes, then comment on this in the cover letter and perhaps that will
> > > > justify including bitmap.h in the dmaengine.h (but personally I am fully
> > > > against that).
> > > > 
> > > 
> > > Hmm I don't think all dmaengine.h will use this API. Maybe on the
> > > producer side but even in that case I don't think so. These are all tiny
> > > wrappers that make sense to be inlined but OTOH, I don't think any of
> > > these needs to be called in any fastpath. And I do understand the header
> > > pain in here but honestly, no strong feelings. So, I'll pretty much
> > > defer this to Frank or Vinod.
> > > 
> > > Frank, Vinod any preference?
> > 
> > This header is where we push stuff in, so I would try to keep it clean,
> > so I guess better to go with Andy suggestion here
> 
> Andy,
> 
> Just remembered that bitmap.h is already included in dmaengine.h anyways. bitops.h
> is because of __set/clear_bit().

So, simpler thing would be to move __dma_cap_zero() in dmaengine.c. But
all other related things are inline APIs and we would be moving this one
just because of something that live in dmaengine.h for a long time now.
We could think about some dma-caps.h but that's honestly just out of
scope for the current series and nothing I can really commit in doing.

Likely do the same for the bus_width API though I think the split
header might also work. The one place where the new API will be almost
always used is in the producer side (so drivers/dma).

But again, honestly, I'm just tempted in saying, let's leave it as-is.
bitmap.h is already part of dmaengine.h anyways.

- Nuno Sá

> 
> - Nuno Sá
> 
> > 
> > -- 
> > ~Vinod