RE: [PATCHv1 1/5] ASoC: da7213: Add regulator support

[Adam Thomson]

On 12 November 2019 15:24, Sebastian Reichel wrote:

> > >  static const struct snd_soc_dapm_widget da7213_dapm_widgets[] = {
> > > +	/*
> > > +	 * Power Supply
> > > +	 */
> > > +	SND_SOC_DAPM_REGULATOR_SUPPLY("VDDA", 0, 0),
> >
> > Having spoken with our HW team, this will cause a POR in the device so we can't
> > just enable/disable VDD_A supply. Needs to present at all times. How are you
> > verifying this?
> 
> Ok. The system, that I used for testing shared a regulator
> for VDDIO and VDDA. I suppose this needs to be moved next
> to enabling VDDIO then.

Yes, and as Mark mentioned you can use the bulk enable/disable calls.

> > > +	da7213->vddio = devm_regulator_get(&i2c->dev, "VDDIO");
> > > +	if (IS_ERR(da7213->vddio))
> > > +		return PTR_ERR(da7213->vddio);
> > > +
> > > +	ret = regulator_enable(da7213->vddio);
> > > +	if (ret < 0)
> > > +		return ret;
> > > +
> > > +	ret = devm_add_action_or_reset(&i2c->dev, da7213_power_off,
> > > da7213);
> > > +	if (ret < 0)
> > > +		return ret;
> >
> > We're seemingly leaving the VDDIO regulator enabled on failure, unless I'm
> > missing some magic somewhere?
> 
> If regulator_enable fails, the regulator is not enabled. If
> devm_add_action_or_reset
> fails, it will call da7213_power_off().

Right, didn't spot that as haven't seen that used before. Nice :)