Re: [PATCH v2 43/44] ASoC: codecs: wm*: use .auto_selectable_formats

Hi Charles

Thank you for your review

> > +static const u64 wm8350_selectable_formats =
> > +	SND_SOC_POSSIBLE_DAIFMT_I2S	|
> > +	SND_SOC_POSSIBLE_DAIFMT_RIGHT_J	|
> > +	SND_SOC_POSSIBLE_DAIFMT_LEFT_J	|
> > +	SND_SOC_POSSIBLE_DAIFMT_DSP_A	|
> > +	SND_SOC_POSSIBLE_DAIFMT_DSP_B	|
> > +	SND_SOC_POSSIBLE_DAIFMT_NB_NF	|
> > +	SND_SOC_POSSIBLE_DAIFMT_NB_IF	|
> > +	SND_SOC_POSSIBLE_DAIFMT_IB_NF	|
> > +	SND_SOC_POSSIBLE_DAIFMT_IB_IF;
> 
> I think we should only allow NF for the DSP modes here. Looking
> carefully at the driver code it sets the same value for DSP_A
> and DSP_B so LRCLK_INV is actually selecting between those two
> modes here. Admittedly, it should probably block then setting the
> clock stuff in the following switch, but no point compounding that
> problem here.

Oh, yes, indeed.
Thank you for pointing it.
Will fix

> > --- a/sound/soc/codecs/wm8962.c
> > +++ b/sound/soc/codecs/wm8962.c
> > @@ -3056,6 +3056,22 @@ static int wm8962_mute(struct snd_soc_dai *dai, int mute, int direction)
> >  #define WM8962_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S20_3LE |\
> >  			SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S32_LE)
> >  
> > +static const u64 wm8962_selectable_formats[] = {
> > +	/* 1st priority */
> > +	SND_SOC_POSSIBLE_DAIFMT_I2S	|
> > +	SND_SOC_POSSIBLE_DAIFMT_RIGHT_J	|
> > +	SND_SOC_POSSIBLE_DAIFMT_LEFT_J	|
> > +	SND_SOC_POSSIBLE_DAIFMT_DSP_B	|
> 
> DSP_B also only supports NF, there is a fallthrough in the
> switch.

Yes, it was my fault.
Will fix

Thank you for your help !!

Best regards
---
Kuninori Morimoto