Hi,
On Mon, Nov 11, 2019 at 02:07:46PM +0000, Adam Thomson wrote:
> On 08 November 2019 17:49, Sebastian Reichel wrote:
>
> > This adds support for most regulators of da7212 for improved
> > power management. The only thing skipped was the speaker supply,
> > which has some undocumented dependencies. It's supposed to be
> > either always-enabled or always-disabled.
> >
> > Signed-off-by: Sebastian Reichel <sebastian.reichel@xxxxxxxxxxxxx>
> > ---
> > .../devicetree/bindings/sound/da7213.txt | 4 ++
> > sound/soc/codecs/da7213.c | 72 +++++++++++++++++++
> > sound/soc/codecs/da7213.h | 2 +
> > 3 files changed, 78 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/sound/da7213.txt
> > b/Documentation/devicetree/bindings/sound/da7213.txt
> > index 759bb04e0283..cc8200b7d748 100644
> > --- a/Documentation/devicetree/bindings/sound/da7213.txt
> > +++ b/Documentation/devicetree/bindings/sound/da7213.txt
> > @@ -21,6 +21,10 @@ Optional properties:
> > - dlg,dmic-clkrate : DMIC clock frequency (Hz).
> > [<1500000>, <3000000>]
> >
> > + - VDDA-supply : Regulator phandle for Analogue power supply
> > + - VDDMIC-supply : Regulator phandle for Mic Bias
> > + - VDDIO-supply : Regulator phandle for I/O power supply
> > +
> > ======
> >
> > Example:
> > diff --git a/sound/soc/codecs/da7213.c b/sound/soc/codecs/da7213.c
> > index aff306bb58df..36e5a7c9ac33 100644
> > --- a/sound/soc/codecs/da7213.c
> > +++ b/sound/soc/codecs/da7213.c
> > @@ -19,6 +19,7 @@
> > #include <linux/module.h>
> > #include <sound/pcm.h>
> > #include <sound/pcm_params.h>
> > +#include <linux/pm_runtime.h>
> > #include <sound/soc.h>
> > #include <sound/initval.h>
> > #include <sound/tlv.h>
> > @@ -806,6 +807,12 @@ static int da7213_dai_event(struct
> > snd_soc_dapm_widget *w,
> > */
> >
> > 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.
> > + SND_SOC_DAPM_REGULATOR_SUPPLY("VDDMIC", 0, 0),
> > +
> > /*
> > * Input & Output
> > */
> > @@ -931,7 +938,16 @@ static const struct snd_soc_dapm_widget
> > da7213_dapm_widgets[] = {
> > static const struct snd_soc_dapm_route da7213_audio_map[] = {
> > /* Dest Connecting Widget source */
> >
> > + /* Main Power Supply */
> > + {"DAC Left", NULL, "VDDA"},
> > + {"DAC Right", NULL, "VDDA"},
> > + {"ADC Left", NULL, "VDDA"},
> > + {"ADC Right", NULL, "VDDA"},
> > +
> > /* Input path */
> > + {"Mic Bias 1", NULL, "VDDMIC"},
> > + {"Mic Bias 2", NULL, "VDDMIC"},
> > +
> > {"MIC1", NULL, "Mic Bias 1"},
> > {"MIC2", NULL, "Mic Bias 2"},
> >
> > @@ -1691,6 +1707,8 @@ static int da7213_probe(struct snd_soc_component
> > *component)
> > {
> > struct da7213_priv *da7213 =
> > snd_soc_component_get_drvdata(component);
> >
> > + pm_runtime_get_sync(component->dev);
> > +
> > /* Default to using ALC auto offset calibration mode. */
> > snd_soc_component_update_bits(component, DA7213_ALC_CTRL1,
> > DA7213_ALC_CALIB_MODE_MAN, 0);
> > @@ -1811,6 +1829,8 @@ static int da7213_probe(struct snd_soc_component
> > *component)
> > DA7213_DMIC_CLK_RATE_MASK, dmic_cfg);
> > }
> >
> > + pm_runtime_put_sync(component->dev);
> > +
> > /* Check if MCLK provided */
> > da7213->mclk = devm_clk_get(component->dev, "mclk");
> > if (IS_ERR(da7213->mclk)) {
> > @@ -1848,6 +1868,12 @@ static const struct regmap_config
> > da7213_regmap_config = {
> > .cache_type = REGCACHE_RBTREE,
> > };
> >
> > +static void da7213_power_off(void *data)
> > +{
> > + struct da7213_priv *da7213 = data;
> > + regulator_disable(da7213->vddio);
> > +}
> > +
> > static int da7213_i2c_probe(struct i2c_client *i2c,
> > const struct i2c_device_id *id)
> > {
> > @@ -1860,6 +1886,18 @@ static int da7213_i2c_probe(struct i2c_client *i2c,
> >
> > i2c_set_clientdata(i2c, da7213);
> >
> > + 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().
> > da7213->regmap = devm_regmap_init_i2c(i2c, &da7213_regmap_config);
> > if (IS_ERR(da7213->regmap)) {
> > ret = PTR_ERR(da7213->regmap);
> > @@ -1867,6 +1905,11 @@ static int da7213_i2c_probe(struct i2c_client *i2c,
> > return ret;
> > }
> >
> > + pm_runtime_set_autosuspend_delay(&i2c->dev, 100);
> > + pm_runtime_use_autosuspend(&i2c->dev);
> > + pm_runtime_set_active(&i2c->dev);
> > + pm_runtime_enable(&i2c->dev);
> > +
> > ret = devm_snd_soc_register_component(&i2c->dev,
> > &soc_component_dev_da7213, &da7213_dai, 1);
> > if (ret < 0) {
> > @@ -1876,6 +1919,34 @@ static int da7213_i2c_probe(struct i2c_client *i2c,
> > return ret;
> > }
> >
> > +static int __maybe_unused da7213_runtime_suspend(struct device *dev)
> > +{
> > + struct da7213_priv *da7213 = dev_get_drvdata(dev);
> > +
> > + regcache_cache_only(da7213->regmap, true);
> > + regcache_mark_dirty(da7213->regmap);
> > + regulator_disable(da7213->vddio);
> > +
> > + return 0;
> > +}
> > +
> > +static int __maybe_unused da7213_runtime_resume(struct device *dev)
> > +{
> > + struct da7213_priv *da7213 = dev_get_drvdata(dev);
> > + int ret;
> > +
> > + ret = regulator_enable(da7213->vddio);
> > + if (ret < 0)
> > + return ret;
> > + regcache_cache_only(da7213->regmap, false);
> > + regcache_sync(da7213->regmap);
> > + return 0;
> > +}
> > +
> > +static const struct dev_pm_ops da7213_pm = {
> > + SET_RUNTIME_PM_OPS(da7213_runtime_suspend,
> > da7213_runtime_resume, NULL)
> > +};
> > +
> > static const struct i2c_device_id da7213_i2c_id[] = {
> > { "da7213", 0 },
> > { }
> > @@ -1888,6 +1959,7 @@ static struct i2c_driver da7213_i2c_driver = {
> > .name = "da7213",
> > .of_match_table = of_match_ptr(da7213_of_match),
> > .acpi_match_table = ACPI_PTR(da7213_acpi_match),
> > + .pm = &da7213_pm,
> > },
> > .probe = da7213_i2c_probe,
> > .id_table = da7213_i2c_id,
> > diff --git a/sound/soc/codecs/da7213.h b/sound/soc/codecs/da7213.h
> > index 3250a3821fcc..97a250ea39e6 100644
> > --- a/sound/soc/codecs/da7213.h
> > +++ b/sound/soc/codecs/da7213.h
> > @@ -12,6 +12,7 @@
> >
> > #include <linux/clk.h>
> > #include <linux/regmap.h>
> > +#include <linux/regulator/consumer.h>
> > #include <sound/da7213.h>
> >
> > /*
> > @@ -524,6 +525,7 @@ enum da7213_sys_clk {
> > /* Codec private data */
> > struct da7213_priv {
> > struct regmap *regmap;
> > + struct regulator *vddio;
> > struct clk *mclk;
> > unsigned int mclk_rate;
> > int clk_src;
-- Sebastian
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
