[alsa-devel] [PATCH] ASoC: fsl_asrc: add protection for the asrc of older version

[S.j. Wang]

Hi
> 
> Hi Shengjiu,
> 
> On Fri, Mar 01, 2019 at 02:32:38AM +0000, S.j. Wang wrote:
> > There is a constraint for the channel number setting on the asrc of
> > older version (e.g. imx35), the channel number should be even, odd
> > number isn't valid.
> >
> > So add protection when the asrc of older version is used.
> >
> > Signed-off-by: Shengjiu Wang <shengjiu.wang at nxp.com>
> > ---
> >  sound/soc/fsl/fsl_asrc.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/sound/soc/fsl/fsl_asrc.c b/sound/soc/fsl/fsl_asrc.c index
> > 528e8b108422..b3b3c5e15ef1 100644
> > --- a/sound/soc/fsl/fsl_asrc.c
> > +++ b/sound/soc/fsl/fsl_asrc.c
> > @@ -109,7 +109,8 @@ static int fsl_asrc_request_pair(int channels, struct
> fsl_asrc_pair *pair)
> >  	if (index == ASRC_INVALID_PAIR) {
> >  		dev_err(dev, "all pairs are busy now\n");
> >  		ret = -EBUSY;
> > -	} else if (asrc_priv->channel_avail < channels) {
> > +	} else if (asrc_priv->channel_avail < channels ||
> > +		(asrc_priv->channel_bits < 4 && channels % 2 != 0)) {
> >  		dev_err(dev, "can't afford required channels: %d\n",
> channels);
> 
> I feel it'd be better to have another else-if, since the existing error message
> is against something else.
> 
> +	} else if (asrc_priv->channel_bits < 4 && channels & 1) {
> +		/* old version of ASRC has channel_bits = 3 */
> +  		dev_err(dev, "does not support odd channel number\n");
> +  		ret = -EINVAL;
> 
> Alternatively, I feel instead of error-out at here, should we add a HW
> constraint or at least fence it off at the beginning of the hw_params()? This
> is actually nothing specific to the pair-request function but a hardware
> constraint.

How about add constraint in startup?
static int fsl_asrc_dai_startup(struct snd_pcm_substream *substream,
                               struct snd_soc_dai *dai)
{
       struct fsl_asrc *asrc_priv = snd_soc_dai_get_drvdata(dai);

       if (asrc_priv->channel_bits == 3) {
               snd_pcm_hw_constraint_step(substream->runtime, 0,
                                          SNDRV_PCM_HW_PARAM_CHANNELS, 2);
       }

       return 0;
}

Best regards
Wang shengjiu