On Mon, May 11, 2020 at 8:16 PM Tzung-Bi Shih tzungbi@google.com wrote:
(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@gmail.com 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.
Thank you for feedback. I've add initial value for invert.
+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.
Done as requested.
+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);
Done as requested.
+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.
Done as requested.
filename = "dsm_param.bin";
Either:
- initialize when declaring the variable
- remove the variable and inline into request_firmware() call
I've added initialized at declare.
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().
Done as requested.
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)
Done as requested.