Message ID | 20240222083825.190854-1-wtli@nuvoton.com |
---|---|
Headers | show |
Series | ASoC: nau8325: Modify driver code and dtschema. | expand |
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.
Mark Brown 於 2/22/2024 11:47 PM 寫道: > 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. okay, I will make the correction to dev_dbg(). > >> +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. okay, I will revise. > >> + 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. okay, thanks.