Re: [PATCH v3 3/3] ASoC: SDCA: Add port constraints into sdca_asoc_set_constraints()

[Charles Keepax <ckeepax@opensource.cirrus.com>]

On Mon, Nov 09, 2026 at 04:16:52PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential
> issues to consider:
> - [High] ASoC: SDCA: zero word_mask in get_port_masks() when wordlengths
>   are unset
> - [Medium] ASoC: SDCA: 1-channel port_ch_mask in get_port_masks() when
>   min_ch/max_ch are 0
> --
> > +	for (i = 0; i < nprop; i++) {
> > +		if (prop[i].num != port)
> > +			continue;
> > +
> > +		if (prop[i].num_words) {
> > +			for (j = 0; j < prop[i].num_words; j++)
> > +				*word_mask |= get_word_mask(prop[i].words[j]);
> > +		} else {
> > +			for (j = prop[i].min_word; j <= prop[i].max_word; j++)
> > +				*word_mask |= get_word_mask(j);
> > +		}
> 
> [Severity: High]
> Does get_port_masks() leave *word_mask as 0 when a port does not specify
> wordlength properties?
> 
> The MIPI DisCo wordlength properties are optional, and drivers such as
> wcd9378_sdca_read_prop() leave num_words, min_word, and max_word as 0.
> When all three are 0, the else loop runs once with j = 0, get_word_mask(0)
> returns 0, and sdca_asoc_set_constraints() fails PCM startup with -EINVAL
> because constraints->width is 0.

Argh... sorry I had this in but then it got lost when I moved
over to FORMAT instead of sample bits.

> > +
> > +		/* Only handle the number of channels for constraints. */
> > +		if (prop[i].num_ch_combinations) {
> > +			for (j = 0; j < prop[i].num_ch_combinations; j++)
> > +				*ch_mask |= BIT(hweight32(prop[i].ch_combinations[j]) - 1);
> > +		} else if (prop[i].num_channels) {
> > +			*ch_mask = GENMASK(prop[i].num_channels - 1, 0);
> > +		} else {
> > +			*ch_mask = GENMASK(prop[i].max_ch - prop[i].min_ch, 0);
> > +		}
> 
> [Severity: Medium]
> As reported by Mark Brown on v2 [1], does this restrict *ch_mask to a
> single channel (GENMASK(0, 0) == 0x1) when the optional channel properties
> (num_ch_combinations, num_channels, max_ch, and min_ch) are all 0?  That
> constrains the DAI to 1 channel instead of leaving the port channel count
> unconstrained, and causes sdca_asoc_set_constraints() to fail with -EINVAL
> if the terminal cluster only supports multi-channel configurations.
> 
> [1] https://lore.kernel.org/r/179413707633.3630016.13654136728294242171.b4-review@b4

This one can't be fixed locally the problem is min_ch and max_ch
are channel indexes. So both set to zero is a totally valid
setup. I have a patch for soundwire that applies the defaults
there was probably going to send that up tomorrow. But could pull
it in to this chain if we felt strongly on it.

Thanks,
Charles