Re: [PATCH v2 1/9] dmaengine: Support bus widths of 32 bytes and above
Nuno Sá <[email protected]>
| Newsgroups | org.kernel.vger.dmaengine,org.kernel.vger.linux-iio |
|---|---|
| Message-ID | <anrgDskNdV7PG-T-@nsa> |
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?
- Nuno Sá
> --
> With Best Regards,
> Andy Shevchenko
>
>