Re: [PATCH v6 12/19] ASoC: tegra: Add audio mclk configuration

From: Sowjanya Komatineni
Date: Tue Jan 07 2020 - 18:23:57 EST


On 1/7/20 2:56 PM, Dmitry Osipenko wrote:
07.01.2020 19:56, Sowjanya Komatineni ÐÐÑÐÑ:
On 1/7/20 3:14 AM, Sameer Pujar wrote:
On 1/7/2020 9:44 AM, Sowjanya Komatineni wrote:
Tegra PMC clock clk_out_1 is dedicated for audio mclk from Tegra30
through Tegra210 and currently Tegra clock driver does the initial
parent
configuration for audio mclk and keeps it enabled by default.

With the move of PMC clocks from clock driver into pmc driver,
audio clocks parent configuration can be specified through the device
tree
using assigned-clock-parents property and audio mclk control should be
taken care by the audio driver.

This patch has implementation for parent configuration when default
parent
configuration is not specified in the device tree and controls audio
mclk
enable and disable during machine startup and shutdown.

Signed-off-by: Sowjanya Komatineni <skomatineni@xxxxxxxxxx>
Minor comments, otherwise LGTM.

Thanks Sameer. Will fix.
---
 sound/soc/tegra/tegra_alc5632.c | 21 ++++++++++
 sound/soc/tegra/tegra_asoc_utils.c | 84
++++++++++++++++++++++++--------------
 sound/soc/tegra/tegra_asoc_utils.h | 2 +
 sound/soc/tegra/tegra_max98090.c | 21 ++++++++++
 sound/soc/tegra/tegra_rt5640.c | 21 ++++++++++
 sound/soc/tegra/tegra_rt5677.c | 21 ++++++++++
 sound/soc/tegra/tegra_sgtl5000.c | 21 ++++++++++
 sound/soc/tegra/tegra_wm8753.c | 21 ++++++++++
 sound/soc/tegra/tegra_wm8903.c | 21 ++++++++++
 sound/soc/tegra/trimslice.c | 21 ++++++++++
 10 files changed, 224 insertions(+), 30 deletions(-)

diff --git a/sound/soc/tegra/tegra_alc5632.c
b/sound/soc/tegra/tegra_alc5632.c
index 50a6d2ff4442..0fd10023f7a6 100644
--- a/sound/soc/tegra/tegra_alc5632.c
+++ b/sound/soc/tegra/tegra_alc5632.c
@@ -62,8 +62,29 @@ static int tegra_alc5632_asoc_hw_params(struct
snd_pcm_substream *substream,
ÂÂÂÂÂ return 0;
 }
 +static int tegra_alc5632_asoc_startup(struct snd_pcm_substream
*substream)
+{
+ÂÂÂ struct snd_soc_pcm_runtime *rtd = substream->private_data;
+ÂÂÂ struct tegra_alc5632 *machine =
snd_soc_card_get_drvdata(rtd->card);
+ÂÂÂ int ret;
+
+ÂÂÂ ret = tegra_asoc_utils_clk_enable(&machine->util_data);
+
+ÂÂÂ return ret;
+}
+
+static void tegra_alc5632_asoc_shutdown(struct snd_pcm_substream
*substream)
+{
+ÂÂÂ struct snd_soc_pcm_runtime *rtd = substream->private_data;
+ÂÂÂ struct tegra_alc5632 *machine =
snd_soc_card_get_drvdata(rtd->card);
+
+ÂÂÂ tegra_asoc_utils_clk_disable(&machine->util_data);
+}
+
 static const struct snd_soc_ops tegra_alc5632_asoc_ops = {
+ÂÂÂ .startup = tegra_alc5632_asoc_startup,
ÂÂÂÂÂ .hw_params = tegra_alc5632_asoc_hw_params,
+ÂÂÂ .shutdown = tegra_alc5632_asoc_shutdown,
 };
  static struct snd_soc_jack tegra_alc5632_hs_jack;
diff --git a/sound/soc/tegra/tegra_asoc_utils.c
b/sound/soc/tegra/tegra_asoc_utils.c
index 0d2271952555..2886ae9f5a16 100644
--- a/sound/soc/tegra/tegra_asoc_utils.c
+++ b/sound/soc/tegra/tegra_asoc_utils.c
@@ -60,8 +60,6 @@ int tegra_asoc_utils_set_rate(struct
tegra_asoc_utils_data *data, int srate,
ÂÂÂÂÂ data->set_mclk = 0;
 Â clk_disable_unprepare(data->clk_cdev1);
-ÂÂÂ clk_disable_unprepare(data->clk_pll_a_out0);
-ÂÂÂ clk_disable_unprepare(data->clk_pll_a);
 Â err = clk_set_rate(data->clk_pll_a, new_baseclock);
ÂÂÂÂÂ if (err) {
@@ -77,18 +75,6 @@ int tegra_asoc_utils_set_rate(struct
tegra_asoc_utils_data *data, int srate,
 Â /* Don't set cdev1/extern1 rate; it's locked to pll_a_out0 */
 - err = clk_prepare_enable(data->clk_pll_a);
-ÂÂÂ if (err) {
-ÂÂÂÂÂÂÂ dev_err(data->dev, "Can't enable pll_a: %d\n", err);
-ÂÂÂÂÂÂÂ return err;
-ÂÂÂ }
-
-ÂÂÂ err = clk_prepare_enable(data->clk_pll_a_out0);
-ÂÂÂ if (err) {
-ÂÂÂÂÂÂÂ dev_err(data->dev, "Can't enable pll_a_out0: %d\n", err);
-ÂÂÂÂÂÂÂ return err;
-ÂÂÂ }
-
ÂÂÂÂÂ err = clk_prepare_enable(data->clk_cdev1);
ÂÂÂÂÂ if (err) {
ÂÂÂÂÂÂÂÂÂ dev_err(data->dev, "Can't enable cdev1: %d\n", err);
@@ -109,8 +95,6 @@ int tegra_asoc_utils_set_ac97_rate(struct
tegra_asoc_utils_data *data)
ÂÂÂÂÂ int err;
 Â clk_disable_unprepare(data->clk_cdev1);
-ÂÂÂ clk_disable_unprepare(data->clk_pll_a_out0);
-ÂÂÂ clk_disable_unprepare(data->clk_pll_a);
 Â /*
ÂÂÂÂÂÂ * AC97 rate is fixed at 24.576MHz and is used for both the host
@@ -130,17 +114,27 @@ int tegra_asoc_utils_set_ac97_rate(struct
tegra_asoc_utils_data *data)
 Â /* Don't set cdev1/extern1 rate; it's locked to pll_a_out0 */
 - err = clk_prepare_enable(data->clk_pll_a);
+ÂÂÂ err = clk_prepare_enable(data->clk_cdev1);
ÂÂÂÂÂ if (err) {
-ÂÂÂÂÂÂÂ dev_err(data->dev, "Can't enable pll_a: %d\n", err);
+ÂÂÂÂÂÂÂ dev_err(data->dev, "Can't enable cdev1: %d\n", err);
ÂÂÂÂÂÂÂÂÂ return err;
ÂÂÂÂÂ }
 - err = clk_prepare_enable(data->clk_pll_a_out0);
-ÂÂÂ if (err) {
-ÂÂÂÂÂÂÂ dev_err(data->dev, "Can't enable pll_a_out0: %d\n", err);
-ÂÂÂÂÂÂÂ return err;
-ÂÂÂ }
+ÂÂÂ data->set_baseclock = pll_rate;
+ÂÂÂ data->set_mclk = ac97_rate;
+
+ÂÂÂ return 0;
+}
+EXPORT_SYMBOL_GPL(tegra_asoc_utils_set_ac97_rate);
+
+void tegra_asoc_utils_clk_disable(struct tegra_asoc_utils_data *data)
+{
+ÂÂÂ clk_disable_unprepare(data->clk_cdev1);
+}
+
+int tegra_asoc_utils_clk_enable(struct tegra_asoc_utils_data *data)
+{
+ÂÂÂ int err;
 Â err = clk_prepare_enable(data->clk_cdev1);
ÂÂÂÂÂ if (err) {
@@ -148,16 +142,13 @@ int tegra_asoc_utils_set_ac97_rate(struct
tegra_asoc_utils_data *data)
ÂÂÂÂÂÂÂÂÂ return err;
ÂÂÂÂÂ }
 - data->set_baseclock = pll_rate;
-ÂÂÂ data->set_mclk = ac97_rate;
-
ÂÂÂÂÂ return 0;
 }
-EXPORT_SYMBOL_GPL(tegra_asoc_utils_set_ac97_rate);
  int tegra_asoc_utils_init(struct tegra_asoc_utils_data *data,
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ struct device *dev)
 {
+ÂÂÂ struct clk *clk_out_1, *clk_extern1;
ÂÂÂÂÂ int ret;
 Â data->dev = dev;
@@ -193,9 +184,42 @@ int tegra_asoc_utils_init(struct
tegra_asoc_utils_data *data,
ÂÂÂÂÂÂÂÂÂ return PTR_ERR(data->clk_cdev1);
ÂÂÂÂÂ }
 - ret = tegra_asoc_utils_set_rate(data, 44100, 256 * 44100);
-ÂÂÂ if (ret)
-ÂÂÂÂÂÂÂ return ret;
+ÂÂÂ /*
+ÂÂÂÂ * If clock parents are not set in DT, configure here to use
clk_out_1
+ÂÂÂÂ * as mclk and extern1 as parent for Tegra30 and higher.
+ÂÂÂÂ */
+ÂÂÂ if (!of_find_property(dev->of_node, "assigned-clock-parents",
NULL) &&
+ÂÂÂÂÂÂÂ data->soc > TEGRA_ASOC_UTILS_SOC_TEGRA20) {
+ÂÂÂÂÂÂÂ dev_err(data->dev,
As this is a fallback mechanism, use dev_info or dev_dbg instead?
dev_warn

+ÂÂÂÂÂÂÂÂÂÂÂ "Configuring clocks for a legacy device-tree\n");
I'd also add another message here saying "Please update your DT", for
clarity.
OK, Will add.