Re: [PATCH] ASoC: codes: Add support for ES8316 producer mode

On Fri, Jul 01, 2022 at 10:05:37AM +0800, Zhu Ning wrote:

This looks basically good, a few small things:

• if ((fmt & SND_SOC_DAIFMT_CLOCK_PROVIDER_MASK) != SND_SOC_DAIFMT_CBC_CFC) {

• dev_err(component->dev, "Codec driver only supports consumer mode\n");
  

• return -EINVAL;
  

• if ((fmt & SND_SOC_DAIFMT_MASTER_MASK) == SND_SOC_DAIFMT_CBP_CFP) {

• serdata1|=ES8316_SERDATA1_MASTER;
  

  }

There's lots of small coding style things through this - for example here the { } isn't needed and

• lrck_divider = es8316->sysclk/params_rate(params);
• bclk_divider = lrck_divider/4;

There's a lot of cases without spaces around /.

#define ES8316_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S20_3LE | \

• 	SNDRV_PCM_FMTBIT_S24_LE)
  

• 	SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S32_LE)
  

This is adding S32_LE support which is fine but wasn't mentioned in the changelog, it should be a separate patch.