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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <AM5PR1001MB0994E162BF3AA57B69198C4480760@AM5PR1001MB0994.EURPRD10.PROD.OUTLOOK.COM>
Date:   Wed, 13 Nov 2019 10:51:26 +0000
From:   Adam Thomson <Adam.Thomson.Opensource@...semi.com>
To:     Sebastian Reichel <sebastian.reichel@...labora.com>,
        Adam Thomson <Adam.Thomson.Opensource@...semi.com>
CC:     Support Opensource <Support.Opensource@...semi.com>,
        Liam Girdwood <lgirdwood@...il.com>,
        Mark Brown <broonie@...nel.org>,
        Jaroslav Kysela <perex@...ex.cz>,
        Takashi Iwai <tiwai@...e.com>,
        "alsa-devel@...a-project.org" <alsa-devel@...a-project.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "kernel@...labora.com" <kernel@...labora.com>
Subject: RE: [PATCHv1 1/5] ASoC: da7213: Add regulator support

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 :)