(The patch passed 2 round review in https://crrev.com/c/2083354) On Sun, May 10, 2020 at 4:23 PM Steve Lee <steves.lee.maxim@xxxxxxxxx> wrote: > +static int max98390_dai_set_fmt(struct snd_soc_dai *codec_dai, unsigned int fmt) > +{ > + struct snd_soc_component *component = codec_dai->component; > + struct max98390_priv *max98390 = > + snd_soc_component_get_drvdata(component); > + unsigned int mode; > + unsigned int format; > + unsigned int invert; > + > + dev_dbg(component->dev, "%s: fmt 0x%08X\n", __func__, fmt); > + > + switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) { > + case SND_SOC_DAIFMT_CBS_CFS: > + mode = MAX98390_PCM_MASTER_MODE_SLAVE; > + break; > + case SND_SOC_DAIFMT_CBM_CFM: > + max98390->master = true; > + mode = MAX98390_PCM_MASTER_MODE_MASTER; > + break; > + default: > + dev_err(component->dev, "DAI clock mode unsupported\n"); > + return -EINVAL; > + } > + > + regmap_update_bits(max98390->regmap, > + MAX98390_PCM_MASTER_MODE, > + MAX98390_PCM_MASTER_MODE_MASK, > + mode); > + > + switch (fmt & SND_SOC_DAIFMT_INV_MASK) { > + case SND_SOC_DAIFMT_NB_NF: > + break; > + case SND_SOC_DAIFMT_IB_NF: > + invert = MAX98390_PCM_MODE_CFG_PCM_BCLKEDGE; > + break; > + default: > + dev_err(component->dev, "DAI invert mode unsupported\n"); > + return -EINVAL; > + } > + > + regmap_update_bits(max98390->regmap, > + MAX98390_PCM_MODE_CFG, > + MAX98390_PCM_MODE_CFG_PCM_BCLKEDGE, > + invert); invert will be uninitialized in the SND_SOC_DAIFMT_NB_NF case. > +static int max98390_dai_hw_params(struct snd_pcm_substream *substream, > + struct snd_pcm_hw_params *params, > + struct snd_soc_dai *dai) Lack of 1 tab indent. > +static int max98390_adaptive_rdc_get(struct snd_kcontrol *kcontrol, > + struct snd_ctl_elem_value *ucontrol) > +{ > + int rdc, rdc0; > + struct snd_soc_component *component = > + snd_soc_kcontrol_component(kcontrol); > + struct max98390_priv *max98390 = > + snd_soc_component_get_drvdata(component); > + > + regmap_read(max98390->regmap, THERMAL_RDC_RD_BACK_BYTE1, &rdc); > + regmap_read(max98390->regmap, THERMAL_RDC_RD_BACK_BYTE0, &rdc0); > + rdc0 |= rdc << 8; > + ucontrol->value.integer.value[0] = rdc0; ucontrol->value.integer.value[0] = rdc0 | (rdc << 8); > +static int max98390_dsm_init(struct snd_soc_component *component) > +{ > + int ret; > + const char *filename; > + struct max98390_priv *max98390 = > + snd_soc_component_get_drvdata(component); > + const struct firmware *fw = NULL; > + char *dsm_param = NULL; Don't need to initialize fw and dsm_param in the case. > + > + filename = "dsm_param.bin"; Either: - initialize when declaring the variable - remove the variable and inline into request_firmware() call > + ret = request_firmware(&fw, filename, component->dev); > + if (ret) { > + dev_err(component->dev, > + "Failed to acquire dsm params: %d\n", ret); > + goto err; > + } > + > + dev_info(component->dev, > + "max98390: param fw size %d\n", > + fw->size); > + dsm_param = (char *)fw->data; > + dsm_param += MAX98390_DSM_PAYLOAD_OFFSET; > + regmap_bulk_write(max98390->regmap, DSM_EQ_BQ1_B0_BYTE0, > + dsm_param, > + fw->size - MAX98390_DSM_PAYLOAD_OFFSET); > + release_firmware(fw); > + regmap_write(max98390->regmap, MAX98390_R23E1_DSP_GLOBAL_EN, 0x01); > + > +err: > + return ret; > +} > +static int max98390_probe(struct snd_soc_component *component) > +{ > + struct max98390_priv *max98390 = > + snd_soc_component_get_drvdata(component); > + > + /* Update dsm bin param */ This comment makes more sense if before max98390_dsm_init(). > + regmap_write(max98390->regmap, MAX98390_SOFTWARE_RESET, 0x01); > + /* Sleep reset settle time */ > + msleep(20); > + max98390_dsm_init(component); > + /* Check Revision ID */ > + ret = regmap_read(max98390->regmap, > + MAX98390_R24FF_REV_ID, ®); > + if (ret < 0) { if (ret)
