Re: [PATCH v2 1/5] ASoC: add mt6351 codec driver

[KaiChieh Chuang <kaichieh.chuang@mediatek.com>]

On Wed, 2018-04-18 at 17:40 +0100, Mark Brown wrote:
Wants to clarify a few comment here, others will be update in next patch

> > +static int mt6351_codec_probe(struct snd_soc_codec *codec)
> > +{
> > +	struct mt6351_priv *priv = snd_soc_codec_get_drvdata(codec);
> > +
> > +	/* add codec controls */
> > +	snd_soc_add_codec_controls(codec,
> > +				   mt6351_snd_controls,
> > +				   ARRAY_SIZE(mt6351_snd_controls));
> > +	snd_soc_add_codec_controls(codec,
> > +				   mt6351_snd_ul_controls,
> > +				   ARRAY_SIZE(mt6351_snd_ul_controls));
> > +
> > +	mt6351_codec_init_reg(codec);
> > +
> > +	priv->ana_gain[AUDIO_ANALOG_VOLUME_HPOUTL] = 8;
> > +	priv->ana_gain[AUDIO_ANALOG_VOLUME_HPOUTR] = 8;
> > +	priv->ana_gain[AUDIO_ANALOG_VOLUME_MICAMP1] = 3;
> > +	priv->ana_gain[AUDIO_ANALOG_VOLUME_MICAMP2] = 3;
> > +
> > +	return 0;
> > +}
> 
> Can we read the configuration of the device back from the hardware?
> It's better to just use the defaults rather than set things up for a
> particular use case, that way there's a standard that can be agreed even
> if it's not good for every use case.
> 

mt6351_codec_init_reg(), regs bit in here are not govern by DAPM,
and is not optimal for low power in hardware default state.
so fixed those reg bit in optimal state by init its reg.

> > +static struct snd_soc_codec_driver mt6351_soc_codec_driver = {
> > +	.probe = mt6351_codec_probe,
> > +	.get_regmap = mt6351_get_regmap,
> 
> We're just about to remove CODEC drivers entirely and replace them with
> components - nothing else is using the get_regmap() callback.  Do you
> really need that callback, if you do we should just add it to the
> component interface?
> 
> > +	priv->regmap = dev_get_regmap(pdev->dev.parent, NULL);
> > +	if (IS_ERR(priv->regmap))
> > +		return PTR_ERR(priv->regmap);
> 
> This should be the default behaviour so I'm guessing you don't need the
> callback?

the regmap is obtain from parent dev, i assume i can use
snd_soc_component_init_regmap() as alternative for
get_regmap().callback?

Thanks