Re: [PATCH v2 2/2] ASoC: nau8325: new driver

On Thu, Feb 22, 2024 at 04:38:25PM +0800, Seven Lee wrote:

A few very minor things below but basically this looks good.

+static int nau8325_clksrc_choose(struct nau8325 *nau8325,

• 		 const struct nau8325_srate_attr **srate_table,
  

• 		 int *n1_sel, int *mult_sel, int *n2_sel)
  

+{

+proc_done:

• dev_err(nau8325->dev, "nau8325->fs=%d,range=0x%x, %s, (n1,mu,n2,dmu):(%d,%d,%d), MCLK_SRC=%uHz (%d)",

• nau8325->fs, (*srate_table)->range,
  

• (*srate_table)->max ? "MAX" : "MIN",
  

• *n1_sel == CLK_PROC_BYPASS ?
  

• CLK_PROC_BYPASS : mclk_n1_div[*n1_sel].param,
  

• *mult_sel == CLK_PROC_BYPASS ?
  

• CLK_PROC_BYPASS : 1 << mclk_n3_mult[*mult_sel].param,
  

• 1 << mclk_n2_div[*n2_sel].param,
  

• (*srate_table)->mclk_src[ratio],
  

• (*srate_table)->mclk_src[ratio] / nau8325->fs);
  

This should be a dev_dbg(), dev_err() will be very noisy in normal operation.

+static int nau8325_set_fmt(struct snd_soc_dai *dai, unsigned int fmt) +{

• struct snd_soc_component *component = dai->component;
• struct nau8325 *nau8325 = snd_soc_component_get_drvdata(component);
• unsigned int ctrl1_val = 0;
• switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) {
• case SND_SOC_DAIFMT_CBS_CFS:

• break;
  

Please use the modern _CBC_CFC defines.

• ret = regmap_read(nau8325->regmap, NAU8325_R02_DEVICE_ID, &value);
• if (ret) {

• dev_err(dev, "Failed to read device id (%d)", ret);
  

• goto err;
  

• }

Probably a good idea to check that the device ID is what we expected too.