alsa-devel.alsa-project.org archive mirror
 help / color / mirror / Atom feed
From: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
To: Sebastian Reichel <sebastian.reichel@collabora.com>,
	Support Opensource <Support.Opensource@diasemi.com>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Mark Brown <broonie@kernel.org>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"alsa-devel@alsa-project.org" <alsa-devel@alsa-project.org>,
	"kernel@collabora.com" <kernel@collabora.com>,
	Takashi Iwai <tiwai@suse.com>
Subject: Re: [alsa-devel] [PATCHv1 1/5] ASoC: da7213: Add regulator support
Date: Mon, 11 Nov 2019 14:07:46 +0000	[thread overview]
Message-ID: <AM5PR1001MB09942731970692EE42BE9CB180740@AM5PR1001MB0994.EURPRD10.PROD.OUTLOOK.COM> (raw)
In-Reply-To: <20191108174843.11227-2-sebastian.reichel@collabora.com>

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@collabora.com>
> ---
>  .../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?

> +	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?

> +
>  	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;
> --
> 2.24.0.rc1

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

  reply	other threads:[~2019-11-11 14:08 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-08 17:48 [alsa-devel] [PATCHv1 0/5] ASoC: da7213: support for usage with simple-card Sebastian Reichel
2019-11-08 17:48 ` [alsa-devel] [PATCHv1 1/5] ASoC: da7213: Add regulator support Sebastian Reichel
2019-11-11 14:07   ` Adam Thomson [this message]
2019-11-12 15:24     ` Sebastian Reichel
2019-11-12 16:58       ` Mark Brown
2019-11-13 10:51       ` Adam Thomson
2019-11-08 17:48 ` [alsa-devel] [PATCHv1 2/5] ASoC: Add DA7213 audio codec as selectable option Sebastian Reichel
2019-11-08 17:48 ` [alsa-devel] [PATCHv1 3/5] ASoC: da7213: move set_sysclk to codec level Sebastian Reichel
2019-11-08 17:48 ` [alsa-devel] [PATCHv1 4/5] ASoC: da7213: move set_pll " Sebastian Reichel
2019-11-08 17:48 ` [alsa-devel] [PATCHv1 5/5] ASoC: da7213: add default clock handling Sebastian Reichel
2019-11-11 14:20   ` Adam Thomson
2019-11-12 16:29     ` Sebastian Reichel
2019-11-13 11:25       ` Adam Thomson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=AM5PR1001MB09942731970692EE42BE9CB180740@AM5PR1001MB0994.EURPRD10.PROD.OUTLOOK.COM \
    --to=adam.thomson.opensource@diasemi.com \
    --cc=Support.Opensource@diasemi.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=broonie@kernel.org \
    --cc=kernel@collabora.com \
    --cc=lgirdwood@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sebastian.reichel@collabora.com \
    --cc=tiwai@suse.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).