[PATCH v3 0/6] Clock and reset improvements for Tegra ALSA drivers

[PATCH v3 0/6] Clock and reset improvements for Tegra ALSA drivers

[Dmitry Osipenko]

This series improves the handling of clock and reset controls of
NVIDA Tegra ALSA drivers. Tegra HDA and AHUB drivers aren't handling
resets properly, which needs to be fixed in order to unblock other patches
related to fixes of the reset controller driver since HDA/AHUB are bound
to fail once reset controller driver will be corrected. In particular ALSA
drivers are relying on implicit de-assertion of resets which is done by the
tegra-clk driver. It's not the business of the clk driver to touch resets
and we need to fix this because it breaks reset/clk programming sequences
of other Tegra drivers.

Changelog:

v3: - Reworked "hda/tegra: Reset hardware" and "ahub: Reset hardware properly"
      patches, they now use usleep + reset_deassert() instead of reset_reset().
      Suggested by Thierry Reding.

    - Added new patch "hda/tegra: Remove unnecessary null-check from
      hda_tegra_runtime_resume()". Suggested by Thierry Reding.

    - Replaced "ahub: Reset hardware properly" patch with "ahub: Add missing
      resets". Suggested by Thierry Reding.

    - Slightly improved commit messages.

    - Added acks from Thierry Reding.

v2: - Added regcache_sync() to the "ahub: Reset hardware properly" patch,
      which was missed by accident in v1.

    - Corrected typo in the format of the error message in "ahub: Use
      of_reset_control_array_get_exclusive()" patch by s/%p/%pe/.

Dmitry Osipenko (6):
  ALSA: hda/tegra: Use clk_bulk helpers
  ALSA: hda/tegra: Reset hardware
  ALSA: hda/tegra: Remove unnecessary null-check from
    hda_tegra_runtime_resume()
  ASoC: tegra: ahub: Add missing resets
  ASoC: tegra: ahub: Use clk_bulk helpers
  ASoC: tegra: ahub: Reset hardware properly

 sound/pci/hda/hda_tegra.c      | 90 ++++++++++++----------------------
 sound/soc/tegra/tegra30_ahub.c | 64 ++++++++++++++----------
 sound/soc/tegra/tegra30_ahub.h |  5 +-
 3 files changed, 72 insertions(+), 87 deletions(-)

-- 
2.29.2

[PATCH v3 1/6] ALSA: hda/tegra: Use clk_bulk helpers

[Dmitry Osipenko]

Use clk_bulk helpers to make code cleaner. Note that this patch changed
the order in which clocks are enabled to make code look nicer, but this
doesn't matter in terms of hardware.

Tested-by: Peter Geis <pgwipeout@gmail.com> # Ouya T30 audio works
Tested-by: Matt Merhar <mattmerhar@protonmail.com> # Ouya T30 boot-tested
Tested-by: Nicolas Chauvet <kwizart@gmail.com> # TK1 boot-tested
Acked-by: Thierry Reding <treding@nvidia.com>
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 sound/pci/hda/hda_tegra.c | 68 ++++++---------------------------------
 1 file changed, 9 insertions(+), 59 deletions(-)

diff --git a/sound/pci/hda/hda_tegra.c b/sound/pci/hda/hda_tegra.c
index 361cf2041911..a25bf7083c28 100644
--- a/sound/pci/hda/hda_tegra.c
+++ b/sound/pci/hda/hda_tegra.c
@@ -70,9 +70,8 @@
 struct hda_tegra {
 	struct azx chip;
 	struct device *dev;
-	struct clk *hda_clk;
-	struct clk *hda2codec_2x_clk;
-	struct clk *hda2hdmi_clk;
+	struct clk_bulk_data clocks[3];
+	unsigned int nclocks;
 	void __iomem *regs;
 	struct work_struct probe_work;
 };
@@ -113,36 +112,6 @@ static void hda_tegra_init(struct hda_tegra *hda)
 	writel(v, hda->regs + HDA_IPFS_INTR_MASK);
 }
 
-static int hda_tegra_enable_clocks(struct hda_tegra *data)
-{
-	int rc;
-
-	rc = clk_prepare_enable(data->hda_clk);
-	if (rc)
-		return rc;
-	rc = clk_prepare_enable(data->hda2codec_2x_clk);
-	if (rc)
-		goto disable_hda;
-	rc = clk_prepare_enable(data->hda2hdmi_clk);
-	if (rc)
-		goto disable_codec_2x;
-
-	return 0;
-
-disable_codec_2x:
-	clk_disable_unprepare(data->hda2codec_2x_clk);
-disable_hda:
-	clk_disable_unprepare(data->hda_clk);
-	return rc;
-}
-
-static void hda_tegra_disable_clocks(struct hda_tegra *data)
-{
-	clk_disable_unprepare(data->hda2hdmi_clk);
-	clk_disable_unprepare(data->hda2codec_2x_clk);
-	clk_disable_unprepare(data->hda_clk);
-}
-
 /*
  * power management
  */
@@ -186,7 +155,7 @@ static int __maybe_unused hda_tegra_runtime_suspend(struct device *dev)
 		azx_stop_chip(chip);
 		azx_enter_link_reset(chip);
 	}
-	hda_tegra_disable_clocks(hda);
+	clk_bulk_disable_unprepare(hda->nclocks, hda->clocks);
 
 	return 0;
 }
@@ -198,7 +167,7 @@ static int __maybe_unused hda_tegra_runtime_resume(struct device *dev)
 	struct hda_tegra *hda = container_of(chip, struct hda_tegra, chip);
 	int rc;
 
-	rc = hda_tegra_enable_clocks(hda);
+	rc = clk_bulk_prepare_enable(hda->nclocks, hda->clocks);
 	if (rc != 0)
 		return rc;
 	if (chip && chip->running) {
@@ -268,29 +237,6 @@ static int hda_tegra_init_chip(struct azx *chip, struct platform_device *pdev)
 	return 0;
 }
 
-static int hda_tegra_init_clk(struct hda_tegra *hda)
-{
-	struct device *dev = hda->dev;
-
-	hda->hda_clk = devm_clk_get(dev, "hda");
-	if (IS_ERR(hda->hda_clk)) {
-		dev_err(dev, "failed to get hda clock\n");
-		return PTR_ERR(hda->hda_clk);
-	}
-	hda->hda2codec_2x_clk = devm_clk_get(dev, "hda2codec_2x");
-	if (IS_ERR(hda->hda2codec_2x_clk)) {
-		dev_err(dev, "failed to get hda2codec_2x clock\n");
-		return PTR_ERR(hda->hda2codec_2x_clk);
-	}
-	hda->hda2hdmi_clk = devm_clk_get(dev, "hda2hdmi");
-	if (IS_ERR(hda->hda2hdmi_clk)) {
-		dev_err(dev, "failed to get hda2hdmi clock\n");
-		return PTR_ERR(hda->hda2hdmi_clk);
-	}
-
-	return 0;
-}
-
 static int hda_tegra_first_init(struct azx *chip, struct platform_device *pdev)
 {
 	struct hda_tegra *hda = container_of(chip, struct hda_tegra, chip);
@@ -495,7 +441,11 @@ static int hda_tegra_probe(struct platform_device *pdev)
 		return err;
 	}
 
-	err = hda_tegra_init_clk(hda);
+	hda->clocks[hda->nclocks++].id = "hda";
+	hda->clocks[hda->nclocks++].id = "hda2hdmi";
+	hda->clocks[hda->nclocks++].id = "hda2codec_2x";
+
+	err = devm_clk_bulk_get(&pdev->dev, hda->nclocks, hda->clocks);
 	if (err < 0)
 		goto out_free;
 
-- 
2.29.2

Re: [PATCH v3 1/6] ALSA: hda/tegra: Use clk_bulk helpers

[Takashi Iwai]

On Wed, 20 Jan 2021 01:31:49 +0100,
Dmitry Osipenko wrote:
> 
> Use clk_bulk helpers to make code cleaner. Note that this patch changed
> the order in which clocks are enabled to make code look nicer, but this
> doesn't matter in terms of hardware.
> 
> Tested-by: Peter Geis <pgwipeout@gmail.com> # Ouya T30 audio works
> Tested-by: Matt Merhar <mattmerhar@protonmail.com> # Ouya T30 boot-tested
> Tested-by: Nicolas Chauvet <kwizart@gmail.com> # TK1 boot-tested
> Acked-by: Thierry Reding <treding@nvidia.com>
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>

Thanks, applied now.


Takashi

[PATCH v3 2/6] ALSA: hda/tegra: Reset hardware

[Dmitry Osipenko]

Reset hardware on RPM-resume in order to bring it into a predictable
state.

Tested-by: Peter Geis <pgwipeout@gmail.com> # Ouya T30 audio works
Tested-by: Matt Merhar <mattmerhar@protonmail.com> # Ouya T30 boot-tested
Tested-by: Nicolas Chauvet <kwizart@gmail.com> # TK1 boot-tested
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 sound/pci/hda/hda_tegra.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/sound/pci/hda/hda_tegra.c b/sound/pci/hda/hda_tegra.c
index a25bf7083c28..04dcd4cdfd9e 100644
--- a/sound/pci/hda/hda_tegra.c
+++ b/sound/pci/hda/hda_tegra.c
@@ -17,6 +17,7 @@
 #include <linux/moduleparam.h>
 #include <linux/mutex.h>
 #include <linux/of_device.h>
+#include <linux/reset.h>
 #include <linux/slab.h>
 #include <linux/time.h>
 #include <linux/string.h>
@@ -70,6 +71,7 @@
 struct hda_tegra {
 	struct azx chip;
 	struct device *dev;
+	struct reset_control *reset;
 	struct clk_bulk_data clocks[3];
 	unsigned int nclocks;
 	void __iomem *regs;
@@ -167,6 +169,12 @@ static int __maybe_unused hda_tegra_runtime_resume(struct device *dev)
 	struct hda_tegra *hda = container_of(chip, struct hda_tegra, chip);
 	int rc;
 
+	if (!chip->running) {
+		rc = reset_control_assert(hda->reset);
+		if (rc)
+			return rc;
+	}
+
 	rc = clk_bulk_prepare_enable(hda->nclocks, hda->clocks);
 	if (rc != 0)
 		return rc;
@@ -176,6 +184,12 @@ static int __maybe_unused hda_tegra_runtime_resume(struct device *dev)
 		/* disable controller wake up event*/
 		azx_writew(chip, WAKEEN, azx_readw(chip, WAKEEN) &
 			   ~STATESTS_INT_MASK);
+	} else {
+		usleep_range(10, 100);
+
+		rc = reset_control_deassert(hda->reset);
+		if (rc)
+			return rc;
 	}
 
 	return 0;
@@ -441,6 +455,12 @@ static int hda_tegra_probe(struct platform_device *pdev)
 		return err;
 	}
 
+	hda->reset = devm_reset_control_array_get_exclusive(&pdev->dev);
+	if (IS_ERR(hda->reset)) {
+		err = PTR_ERR(hda->reset);
+		goto out_free;
+	}
+
 	hda->clocks[hda->nclocks++].id = "hda";
 	hda->clocks[hda->nclocks++].id = "hda2hdmi";
 	hda->clocks[hda->nclocks++].id = "hda2codec_2x";
-- 
2.29.2

Re: [PATCH v3 2/6] ALSA: hda/tegra: Reset hardware

[Takashi Iwai]

On Wed, 20 Jan 2021 01:31:50 +0100,
Dmitry Osipenko wrote:
> 
> Reset hardware on RPM-resume in order to bring it into a predictable
> state.
> 
> Tested-by: Peter Geis <pgwipeout@gmail.com> # Ouya T30 audio works
> Tested-by: Matt Merhar <mattmerhar@protonmail.com> # Ouya T30 boot-tested
> Tested-by: Nicolas Chauvet <kwizart@gmail.com> # TK1 boot-tested
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>

Currently we have neither dependency nor reverse-selection of
CONFIG_RESET_CONTROLLER.  It wouldn't be a problem for builds, but
you'll get a runtime error from
devm_reset_control_array_get_exclusive() always when
CONFIG_RESET_CONTROLLER=n.

I guess it must be a corner case, but just to be sure.


thanks,

Takashi


> ---
>  sound/pci/hda/hda_tegra.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/sound/pci/hda/hda_tegra.c b/sound/pci/hda/hda_tegra.c
> index a25bf7083c28..04dcd4cdfd9e 100644
> --- a/sound/pci/hda/hda_tegra.c
> +++ b/sound/pci/hda/hda_tegra.c
> @@ -17,6 +17,7 @@
>  #include <linux/moduleparam.h>
>  #include <linux/mutex.h>
>  #include <linux/of_device.h>
> +#include <linux/reset.h>
>  #include <linux/slab.h>
>  #include <linux/time.h>
>  #include <linux/string.h>
> @@ -70,6 +71,7 @@
>  struct hda_tegra {
>  	struct azx chip;
>  	struct device *dev;
> +	struct reset_control *reset;
>  	struct clk_bulk_data clocks[3];
>  	unsigned int nclocks;
>  	void __iomem *regs;
> @@ -167,6 +169,12 @@ static int __maybe_unused hda_tegra_runtime_resume(struct device *dev)
>  	struct hda_tegra *hda = container_of(chip, struct hda_tegra, chip);
>  	int rc;
>  
> +	if (!chip->running) {
> +		rc = reset_control_assert(hda->reset);
> +		if (rc)
> +			return rc;
> +	}
> +
>  	rc = clk_bulk_prepare_enable(hda->nclocks, hda->clocks);
>  	if (rc != 0)
>  		return rc;
> @@ -176,6 +184,12 @@ static int __maybe_unused hda_tegra_runtime_resume(struct device *dev)
>  		/* disable controller wake up event*/
>  		azx_writew(chip, WAKEEN, azx_readw(chip, WAKEEN) &
>  			   ~STATESTS_INT_MASK);
> +	} else {
> +		usleep_range(10, 100);
> +
> +		rc = reset_control_deassert(hda->reset);
> +		if (rc)
> +			return rc;
>  	}
>  
>  	return 0;
> @@ -441,6 +455,12 @@ static int hda_tegra_probe(struct platform_device *pdev)
>  		return err;
>  	}
>  
> +	hda->reset = devm_reset_control_array_get_exclusive(&pdev->dev);
> +	if (IS_ERR(hda->reset)) {
> +		err = PTR_ERR(hda->reset);
> +		goto out_free;
> +	}
> +
>  	hda->clocks[hda->nclocks++].id = "hda";
>  	hda->clocks[hda->nclocks++].id = "hda2hdmi";
>  	hda->clocks[hda->nclocks++].id = "hda2codec_2x";
> -- 
> 2.29.2
> 

Re: [PATCH v3 2/6] ALSA: hda/tegra: Reset hardware

[Dmitry Osipenko]

25.01.2021 18:18, Takashi Iwai пишет:
> On Wed, 20 Jan 2021 01:31:50 +0100,
> Dmitry Osipenko wrote:
>>
>> Reset hardware on RPM-resume in order to bring it into a predictable
>> state.
>>
>> Tested-by: Peter Geis <pgwipeout@gmail.com> # Ouya T30 audio works
>> Tested-by: Matt Merhar <mattmerhar@protonmail.com> # Ouya T30 boot-tested
>> Tested-by: Nicolas Chauvet <kwizart@gmail.com> # TK1 boot-tested
>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> 
> Currently we have neither dependency nor reverse-selection of
> CONFIG_RESET_CONTROLLER.  It wouldn't be a problem for builds, but
> you'll get a runtime error from
> devm_reset_control_array_get_exclusive() always when
> CONFIG_RESET_CONTROLLER=n.
> 
> I guess it must be a corner case, but just to be sure.

The CONFIG_RESET_CONTROLLER=y at least for ARM32 Tegra builds.

https://elixir.bootlin.com/linux/v5.11-rc5/source/arch/arm/mach-tegra/Kconfig#L15

Not sure about ARM64.

Re: [PATCH v3 2/6] ALSA: hda/tegra: Reset hardware

[Takashi Iwai]

On Mon, 25 Jan 2021 16:27:30 +0100,
Dmitry Osipenko wrote:
> 
> 25.01.2021 18:18, Takashi Iwai пишет:
> > On Wed, 20 Jan 2021 01:31:50 +0100,
> > Dmitry Osipenko wrote:
> >>
> >> Reset hardware on RPM-resume in order to bring it into a predictable
> >> state.
> >>
> >> Tested-by: Peter Geis <pgwipeout@gmail.com> # Ouya T30 audio works
> >> Tested-by: Matt Merhar <mattmerhar@protonmail.com> # Ouya T30 boot-tested
> >> Tested-by: Nicolas Chauvet <kwizart@gmail.com> # TK1 boot-tested
> >> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> > 
> > Currently we have neither dependency nor reverse-selection of
> > CONFIG_RESET_CONTROLLER.  It wouldn't be a problem for builds, but
> > you'll get a runtime error from
> > devm_reset_control_array_get_exclusive() always when
> > CONFIG_RESET_CONTROLLER=n.
> > 
> > I guess it must be a corner case, but just to be sure.
> 
> The CONFIG_RESET_CONTROLLER=y at least for ARM32 Tegra builds.
> 
> https://elixir.bootlin.com/linux/v5.11-rc5/source/arch/arm/mach-tegra/Kconfig#L15
> 
> Not sure about ARM64.

OK, then it must be fine.
I applied now.  Thanks.


Takashi

[PATCH v3 3/6] ALSA: hda/tegra: Remove unnecessary null-check from hda_tegra_runtime_resume()

[Dmitry Osipenko]

The "chip" can't be NULL in hda_tegra_runtime_resume() because code would
crash otherwise. Let's remove the unnecessary check in order to clean up
code a tad.

Tested-by: Peter Geis <pgwipeout@gmail.com> # Ouya T30 audio works
Tested-by: Matt Merhar <mattmerhar@protonmail.com> # Ouya T30 boot-tested
Suggested-by: Thierry Reding <treding@nvidia.com>
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 sound/pci/hda/hda_tegra.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/pci/hda/hda_tegra.c b/sound/pci/hda/hda_tegra.c
index 04dcd4cdfd9e..6f2b743b9d75 100644
--- a/sound/pci/hda/hda_tegra.c
+++ b/sound/pci/hda/hda_tegra.c
@@ -178,7 +178,7 @@ static int __maybe_unused hda_tegra_runtime_resume(struct device *dev)
 	rc = clk_bulk_prepare_enable(hda->nclocks, hda->clocks);
 	if (rc != 0)
 		return rc;
-	if (chip && chip->running) {
+	if (chip->running) {
 		hda_tegra_init(hda);
 		azx_init_chip(chip, 1);
 		/* disable controller wake up event*/
-- 
2.29.2

Re: [PATCH v3 3/6] ALSA: hda/tegra: Remove unnecessary null-check from hda_tegra_runtime_resume()

[Takashi Iwai]

On Wed, 20 Jan 2021 01:31:51 +0100,
Dmitry Osipenko wrote:
> 
> The "chip" can't be NULL in hda_tegra_runtime_resume() because code would
> crash otherwise. Let's remove the unnecessary check in order to clean up
> code a tad.
> 
> Tested-by: Peter Geis <pgwipeout@gmail.com> # Ouya T30 audio works
> Tested-by: Matt Merhar <mattmerhar@protonmail.com> # Ouya T30 boot-tested
> Suggested-by: Thierry Reding <treding@nvidia.com>
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>

Applied, thanks.


Takashi

[PATCH v3 4/6] ASoC: tegra: ahub: Add missing resets

[Dmitry Osipenko]

AHUB driver misses D_AUDIO and APBIF resets. CPU hangs on trying to
access hardware if resets aren't de-asserted. This problem is currently
masked by the tegra-clk driver which implicitly de-asserts the resets when
the corresponding clocks are enabled. Soon the implicit de-assertion will
be gone from the tegra-clk driver, thus we need to fix the AHUB driver.
Add the missing resets to the driver.

Tested-by: Peter Geis <pgwipeout@gmail.com> # Ouya T30 audio works
Tested-by: Matt Merhar <mattmerhar@protonmail.com> # Ouya T30 boot-tested
Tested-by: Dmitry Osipenko <digetx@gmail.com> # Nexus7 T30 audio works
Tested-by: Nicolas Chauvet <kwizart@gmail.com> # TK1 boot-tested
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 sound/soc/tegra/tegra30_ahub.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/sound/soc/tegra/tegra30_ahub.c b/sound/soc/tegra/tegra30_ahub.c
index 156e3b9d613c..8c32333cc08c 100644
--- a/sound/soc/tegra/tegra30_ahub.c
+++ b/sound/soc/tegra/tegra30_ahub.c
@@ -337,6 +337,8 @@ static const struct {
 	const char *rst_name;
 	u32 mod_list_mask;
 } configlink_mods[] = {
+	{ "d_audio", MOD_LIST_MASK_TEGRA30_OR_LATER },
+	{ "apbif", MOD_LIST_MASK_TEGRA30_OR_LATER },
 	{ "i2s0", MOD_LIST_MASK_TEGRA30_OR_LATER },
 	{ "i2s1", MOD_LIST_MASK_TEGRA30_OR_LATER },
 	{ "i2s2", MOD_LIST_MASK_TEGRA30_OR_LATER },
-- 
2.29.2

[PATCH v3 5/6] ASoC: tegra: ahub: Use clk_bulk helpers

[Dmitry Osipenko]

Use clk_bulk helpers to make code cleaner.

Tested-by: Peter Geis <pgwipeout@gmail.com> # Ouya T30 audio works
Tested-by: Matt Merhar <mattmerhar@protonmail.com> # Ouya T30 boot-tested
Tested-by: Dmitry Osipenko <digetx@gmail.com> # Nexus7 T30 audio works
Tested-by: Nicolas Chauvet <kwizart@gmail.com> # TK1 boot-tested
Acked-by: Thierry Reding <treding@nvidia.com>
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 sound/soc/tegra/tegra30_ahub.c | 30 +++++++-----------------------
 sound/soc/tegra/tegra30_ahub.h |  4 ++--
 2 files changed, 9 insertions(+), 25 deletions(-)

diff --git a/sound/soc/tegra/tegra30_ahub.c b/sound/soc/tegra/tegra30_ahub.c
index 8c32333cc08c..12ca8e3ca4f6 100644
--- a/sound/soc/tegra/tegra30_ahub.c
+++ b/sound/soc/tegra/tegra30_ahub.c
@@ -45,8 +45,7 @@ static int tegra30_ahub_runtime_suspend(struct device *dev)
 	regcache_cache_only(ahub->regmap_apbif, true);
 	regcache_cache_only(ahub->regmap_ahub, true);
 
-	clk_disable_unprepare(ahub->clk_apbif);
-	clk_disable_unprepare(ahub->clk_d_audio);
+	clk_bulk_disable_unprepare(ahub->nclocks, ahub->clocks);
 
 	return 0;
 }
@@ -66,17 +65,9 @@ static int tegra30_ahub_runtime_resume(struct device *dev)
 {
 	int ret;
 
-	ret = clk_prepare_enable(ahub->clk_d_audio);
-	if (ret) {
-		dev_err(dev, "clk_enable d_audio failed: %d\n", ret);
+	ret = clk_bulk_prepare_enable(ahub->nclocks, ahub->clocks);
+	if (ret)
 		return ret;
-	}
-	ret = clk_prepare_enable(ahub->clk_apbif);
-	if (ret) {
-		dev_err(dev, "clk_enable apbif failed: %d\n", ret);
-		clk_disable(ahub->clk_d_audio);
-		return ret;
-	}
 
 	regcache_cache_only(ahub->regmap_apbif, false);
 	regcache_cache_only(ahub->regmap_ahub, false);
@@ -559,19 +550,12 @@ static int tegra30_ahub_probe(struct platform_device *pdev)
 	ahub->soc_data = soc_data;
 	ahub->dev = &pdev->dev;
 
-	ahub->clk_d_audio = devm_clk_get(&pdev->dev, "d_audio");
-	if (IS_ERR(ahub->clk_d_audio)) {
-		dev_err(&pdev->dev, "Can't retrieve ahub d_audio clock\n");
-		ret = PTR_ERR(ahub->clk_d_audio);
-		return ret;
-	}
+	ahub->clocks[ahub->nclocks++].id = "apbif";
+	ahub->clocks[ahub->nclocks++].id = "d_audio";
 
-	ahub->clk_apbif = devm_clk_get(&pdev->dev, "apbif");
-	if (IS_ERR(ahub->clk_apbif)) {
-		dev_err(&pdev->dev, "Can't retrieve ahub apbif clock\n");
-		ret = PTR_ERR(ahub->clk_apbif);
+	ret = devm_clk_bulk_get(&pdev->dev, ahub->nclocks, ahub->clocks);
+	if (ret)
 		return ret;
-	}
 
 	res0 = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	regs_apbif = devm_ioremap_resource(&pdev->dev, res0);
diff --git a/sound/soc/tegra/tegra30_ahub.h b/sound/soc/tegra/tegra30_ahub.h
index 6889c5f23d02..01480d7dc940 100644
--- a/sound/soc/tegra/tegra30_ahub.h
+++ b/sound/soc/tegra/tegra30_ahub.h
@@ -511,8 +511,8 @@ struct tegra30_ahub_soc_data {
 struct tegra30_ahub {
 	const struct tegra30_ahub_soc_data *soc_data;
 	struct device *dev;
-	struct clk *clk_d_audio;
-	struct clk *clk_apbif;
+	struct clk_bulk_data clocks[2];
+	unsigned int nclocks;
 	resource_size_t apbif_addr;
 	struct regmap *regmap_apbif;
 	struct regmap *regmap_ahub;
-- 
2.29.2

[PATCH v3 6/6] ASoC: tegra: ahub: Reset hardware properly

[Dmitry Osipenko]

Assert hardware resets before clocks are enabled and then de-assert them
after clocks are enabled. This brings hardware into a predictable state.

Tested-by: Peter Geis <pgwipeout@gmail.com> # Ouya T30 audio works
Tested-by: Matt Merhar <mattmerhar@protonmail.com> # Ouya T30 boot-tested
Tested-by: Dmitry Osipenko <digetx@gmail.com> # Nexus7 T30 audio works
Tested-by: Nicolas Chauvet <kwizart@gmail.com> # TK1 boot-tested
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 sound/soc/tegra/tegra30_ahub.c | 36 ++++++++++++++++++++++++++++++----
 sound/soc/tegra/tegra30_ahub.h |  1 +
 2 files changed, 33 insertions(+), 4 deletions(-)

diff --git a/sound/soc/tegra/tegra30_ahub.c b/sound/soc/tegra/tegra30_ahub.c
index 12ca8e3ca4f6..9ef05ca4f6c4 100644
--- a/sound/soc/tegra/tegra30_ahub.c
+++ b/sound/soc/tegra/tegra30_ahub.c
@@ -65,14 +65,39 @@ static int tegra30_ahub_runtime_resume(struct device *dev)
 {
 	int ret;
 
+	ret = reset_control_assert(ahub->reset);
+	if (ret)
+		return ret;
+
 	ret = clk_bulk_prepare_enable(ahub->nclocks, ahub->clocks);
 	if (ret)
 		return ret;
 
+	usleep_range(10, 100);
+
+	ret = reset_control_deassert(ahub->reset);
+	if (ret)
+		goto disable_clocks;
+
 	regcache_cache_only(ahub->regmap_apbif, false);
 	regcache_cache_only(ahub->regmap_ahub, false);
+	regcache_mark_dirty(ahub->regmap_apbif);
+	regcache_mark_dirty(ahub->regmap_ahub);
+
+	ret = regcache_sync(ahub->regmap_apbif);
+	if (ret)
+		goto disable_clocks;
+
+	ret = regcache_sync(ahub->regmap_ahub);
+	if (ret)
+		goto disable_clocks;
 
 	return 0;
+
+disable_clocks:
+	clk_bulk_disable_unprepare(ahub->nclocks, ahub->clocks);
+
+	return ret;
 }
 
 int tegra30_ahub_allocate_rx_fifo(enum tegra30_ahub_rxcif *rxcif,
@@ -519,7 +544,6 @@ static int tegra30_ahub_probe(struct platform_device *pdev)
 	/*
 	 * The AHUB hosts a register bus: the "configlink". For this to
 	 * operate correctly, all devices on this bus must be out of reset.
-	 * Ensure that here.
 	 */
 	for (i = 0; i < ARRAY_SIZE(configlink_mods); i++) {
 		if (!(configlink_mods[i].mod_list_mask &
@@ -535,10 +559,8 @@ static int tegra30_ahub_probe(struct platform_device *pdev)
 			return ret;
 		}
 
-		ret = reset_control_deassert(rst);
+		/* just check presence of the reset control in DT */
 		reset_control_put(rst);
-		if (ret)
-			return ret;
 	}
 
 	ahub = devm_kzalloc(&pdev->dev, sizeof(struct tegra30_ahub),
@@ -557,6 +579,12 @@ static int tegra30_ahub_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
+	ahub->reset = devm_reset_control_array_get_exclusive(&pdev->dev);
+	if (IS_ERR(ahub->reset)) {
+		dev_err(&pdev->dev, "Can't get resets: %pe\n", ahub->reset);
+		return PTR_ERR(ahub->reset);
+	}
+
 	res0 = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	regs_apbif = devm_ioremap_resource(&pdev->dev, res0);
 	if (IS_ERR(regs_apbif))
diff --git a/sound/soc/tegra/tegra30_ahub.h b/sound/soc/tegra/tegra30_ahub.h
index 01480d7dc940..3b85244f87f1 100644
--- a/sound/soc/tegra/tegra30_ahub.h
+++ b/sound/soc/tegra/tegra30_ahub.h
@@ -511,6 +511,7 @@ struct tegra30_ahub_soc_data {
 struct tegra30_ahub {
 	const struct tegra30_ahub_soc_data *soc_data;
 	struct device *dev;
+	struct reset_control *reset;
 	struct clk_bulk_data clocks[2];
 	unsigned int nclocks;
 	resource_size_t apbif_addr;
-- 
2.29.2

Re: [PATCH v3 0/6] Clock and reset improvements for Tegra ALSA drivers

[Takashi Iwai]

On Wed, 20 Jan 2021 01:31:48 +0100,
Dmitry Osipenko wrote:
> 
> This series improves the handling of clock and reset controls of
> NVIDA Tegra ALSA drivers. Tegra HDA and AHUB drivers aren't handling
> resets properly, which needs to be fixed in order to unblock other patches
> related to fixes of the reset controller driver since HDA/AHUB are bound
> to fail once reset controller driver will be corrected. In particular ALSA
> drivers are relying on implicit de-assertion of resets which is done by the
> tegra-clk driver. It's not the business of the clk driver to touch resets
> and we need to fix this because it breaks reset/clk programming sequences
> of other Tegra drivers.
> 
> Changelog:
> 
> v3: - Reworked "hda/tegra: Reset hardware" and "ahub: Reset hardware properly"
>       patches, they now use usleep + reset_deassert() instead of reset_reset().
>       Suggested by Thierry Reding.
> 
>     - Added new patch "hda/tegra: Remove unnecessary null-check from
>       hda_tegra_runtime_resume()". Suggested by Thierry Reding.
> 
>     - Replaced "ahub: Reset hardware properly" patch with "ahub: Add missing
>       resets". Suggested by Thierry Reding.
> 
>     - Slightly improved commit messages.
> 
>     - Added acks from Thierry Reding.
> 
> v2: - Added regcache_sync() to the "ahub: Reset hardware properly" patch,
>       which was missed by accident in v1.
> 
>     - Corrected typo in the format of the error message in "ahub: Use
>       of_reset_control_array_get_exclusive()" patch by s/%p/%pe/.
> 
> Dmitry Osipenko (6):
>   ALSA: hda/tegra: Use clk_bulk helpers
>   ALSA: hda/tegra: Reset hardware
>   ALSA: hda/tegra: Remove unnecessary null-check from
>     hda_tegra_runtime_resume()
>   ASoC: tegra: ahub: Add missing resets
>   ASoC: tegra: ahub: Use clk_bulk helpers
>   ASoC: tegra: ahub: Reset hardware properly

Mark, a half of the series are for ASoC.  Would you take those three,
or take the full patches or let me merge through my tree?


thanks,

Takashi

> 
>  sound/pci/hda/hda_tegra.c      | 90 ++++++++++++----------------------
>  sound/soc/tegra/tegra30_ahub.c | 64 ++++++++++++++----------
>  sound/soc/tegra/tegra30_ahub.h |  5 +-
>  3 files changed, 72 insertions(+), 87 deletions(-)
> 
> -- 
> 2.29.2
> 

Re: (subset) [PATCH v3 0/6] Clock and reset improvements for Tegra ALSA drivers

[Mark Brown]

On Wed, 20 Jan 2021 03:31:48 +0300, Dmitry Osipenko wrote:
> This series improves the handling of clock and reset controls of
> NVIDA Tegra ALSA drivers. Tegra HDA and AHUB drivers aren't handling
> resets properly, which needs to be fixed in order to unblock other patches
> related to fixes of the reset controller driver since HDA/AHUB are bound
> to fail once reset controller driver will be corrected. In particular ALSA
> drivers are relying on implicit de-assertion of resets which is done by the
> tegra-clk driver. It's not the business of the clk driver to touch resets
> and we need to fix this because it breaks reset/clk programming sequences
> of other Tegra drivers.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[4/6] ASoC: tegra: ahub: Add missing resets
      commit: 24a41a38dd2df065ee942221c2fae5e314770865
[5/6] ASoC: tegra: ahub: Use clk_bulk helpers
      commit: 6d8ac9b1dd2f138f4aa39008994600f561eeede8
[6/6] ASoC: tegra: ahub: Reset hardware properly
      commit: ed9ce1ed2239909c23d48c723c6549417c476246

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark