Re: [PATCH v4 08/10] ALSA: pcm_dmaengine: Use dma_slave_caps bus width helpers

[Nuno Sá]

On Tue, Sep 15, 2026 at 10:31:55AM +0300, Andy Shevchenko wrote:
> On Mon, Sep 14, 2026 at 04:39:38PM +0100, Nuno Sá wrote:
> > On Mon, Sep 14, 2026 at 11:12:09AM +0300, Andy Shevchenko wrote:
> > > On Fri, Sep 11, 2026 at 06:25:43PM +0100, Nuno Sá wrote:
> 
> ...
> 
> > > > int snd_dmaengine_pcm_refine_runtime_hwparams(
> > > >  	struct snd_pcm_hardware *hw,
> > > >  	struct dma_chan *chan)
> > > >  {
> > > > +	enum dma_slave_buswidth default_widths[] = {
> > > > +		DMA_SLAVE_BUSWIDTH_1_BYTE,
> > > > +		DMA_SLAVE_BUSWIDTH_2_BYTES,
> > > > +		DMA_SLAVE_BUSWIDTH_4_BYTES,
> > > > +	};
> > > >  	struct dma_slave_caps dma_caps;
> > > > -	u32 addr_widths = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
> > > > -			  BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) |
> > > > -			  BIT(DMA_SLAVE_BUSWIDTH_4_BYTES);
> > > 
> > > > +	dma_buswidth_mask_t bus_widths = {};
> > > 
> > > Do you really need the zeroing?
> > 
> > I think so. dma_bus_width_set_many() just does __set_bit() under the
> > hood so if we do not zero initialize I think we could be left with some
> > random bits in the mask. Unless I'm missing something.
> 
> Hmm... Since it's a recipient, I would expect it will be filled correctly
> independently on the garbage it might have had initially. So, perhaps call a
> bitmap_zero() with the maximum size in there?

Hmm don't disagree! Needs to be documented but yes, for all the users of
dma_bus_width_set_many() or dma_bus_width_set() we have so far, the expectation
is that after the call we are left exactly with what we passed.

If ever needed we can add a new (or just a flag) "change
bus_width mask" API.

- Nuno Sá
> 
> > > >  	snd_pcm_format_t i;
> > > >  	int ret = 0;
> > > 
> > > >  	if (!hw || !chan || !dma_data)
> > > >  		return -EINVAL;
> > > >  
> > > > +	ret = dma_bus_width_set_many(bus_widths, default_widths,
> > > > +				     ARRAY_SIZE(default_widths));
> > > > +	if (ret)
> > > > +		return ret;
> > > 
> > > I assume that if ret == 0, bus_widths should be set accordingly.
> 
> As mentioned here, if no error, bus_widths should be fine to use.)
> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
>