Clock and reset improvements for Tegra ALSA drivers

PATCH Applied Dmitry Osipenko 3 participants 14 messages Jan 20, 2021 – Jan 26, 2021

Subsystems (3)

Sound ASoC Tegra architecture support

lore

Share & tools

lore

Dmitry Osipenko

author

2021-01-20 00:33 UTC #0 lore raw

v3 cover Clock and reset improvements for Tegra ALSA drivers

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

Signature · 1 line

2.29.2

Dmitry Osipenko

author

2021-01-20 00:34 UTC #1 lore raw

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

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 <[email protected]> # Ouya T30 audio works

Tested-by Matt Merhar <[email protected]> # Ouya T30 boot-tested

Tested-by Nicolas Chauvet <[email protected]> # TK1 boot-tested

Acked-by Thierry Reding <[email protected]>

Signed-off-by Dmitry Osipenko <[email protected]>

---
 sound/pci/hda/hda_tegra.c | 68 ++++++---------------------------------
 1 file changed, 9 insertions(+), 59 deletions(-)

sound/pci/hda/hda_tegra.c +9 ~~-59~~

index 361cf2041911..a25bf7083c28 100644
@@ -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;

Signature · 1 line

2.29.2

Takashi Iwai

Applied

2021-01-26 06:34 UTC #2 lore raw

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

Quoted 10 lines · 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 <[email protected]> # Ouya T30 audio works
Tested-by Matt Merhar <[email protected]> # Ouya T30 boot-tested
Tested-by Nicolas Chauvet <[email protected]> # TK1 boot-tested
Acked-by Thierry Reding <[email protected]>
Signed-off-by Dmitry Osipenko <[email protected]>

Thanks, applied now.

Takashi

Dmitry Osipenko

author

2021-01-20 00:33 UTC #3 lore raw

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

In reply to Dmitry Osipenko

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

Tested-by Peter Geis <[email protected]> # Ouya T30 audio works

Tested-by Matt Merhar <[email protected]> # Ouya T30 boot-tested

Tested-by Nicolas Chauvet <[email protected]> # TK1 boot-tested

Signed-off-by Dmitry Osipenko <[email protected]>

---
 sound/pci/hda/hda_tegra.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

sound/pci/hda/hda_tegra.c +20 -0

index a25bf7083c28..04dcd4cdfd9e 100644
@@ -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";

Signature · 1 line

2.29.2

Takashi Iwai

2021-01-25 15:20 UTC #4 lore raw

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

Quoted 8 lines · Dmitry Osipenko

Reset hardware on RPM-resume in order to bring it into a predictable state.
Tested-by Peter Geis <[email protected]> # Ouya T30 audio works
Tested-by Matt Merhar <[email protected]> # Ouya T30 boot-tested
Tested-by Nicolas Chauvet <[email protected]> # TK1 boot-tested
Signed-off-by Dmitry Osipenko <[email protected]>

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

Quoted 66 lines

---
 sound/pci/hda/hda_tegra.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

sound/pci/hda/hda_tegra.c +20 -0

index a25bf7083c28..04dcd4cdfd9e 100644
@@ -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";

Signature · 1 line

2.29.2

Dmitry Osipenko

author

2021-01-25 15:28 UTC #5 lore raw

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

25.01.2021 18:18, Takashi Iwai пишет:

Quoted 11 lines

Quoted 8 lines · Dmitry Osipenko
Reset hardware on RPM-resume in order to bring it into a predictable state.
Tested-by Peter Geis <[email protected]> # Ouya T30 audio works
Tested-by Matt Merhar <[email protected]> # Ouya T30 boot-tested
Tested-by Nicolas Chauvet <[email protected]> # TK1 boot-tested
Signed-off-by Dmitry Osipenko <[email protected]>

Quoted 7 lines

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.

Takashi Iwai

2021-01-26 06:35 UTC #6 lore raw

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

Quoted 19 lines · Dmitry Osipenko

25.01.2021 18:18, Takashi Iwai пишет:
Quoted 17 lines
Quoted 8 lines · Dmitry Osipenko
Reset hardware on RPM-resume in order to bring it into a predictable state.
Tested-by Peter Geis <[email protected]> # Ouya T30 audio works
Tested-by Matt Merhar <[email protected]> # Ouya T30 boot-tested
Tested-by Nicolas Chauvet <[email protected]> # TK1 boot-tested
Signed-off-by Dmitry Osipenko <[email protected]>
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.

Quoted 7 lines

Quoted 1 line
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

Dmitry Osipenko

author

2021-01-20 00:33 UTC #7 lore raw

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

In reply to 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 <[email protected]> # Ouya T30 audio works

Tested-by Matt Merhar <[email protected]> # Ouya T30 boot-tested

Signed-off-by Dmitry Osipenko <[email protected]>

---
 sound/pci/hda/hda_tegra.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

sound/pci/hda/hda_tegra.c +1 -1

index 04dcd4cdfd9e..6f2b743b9d75 100644
@@ -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*/

Signature · 1 line

2.29.2

Takashi Iwai

Applied

2021-01-26 06:35 UTC #8 lore raw

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

Quoted 9 lines · 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 <[email protected]> # Ouya T30 audio works
Tested-by Matt Merhar <[email protected]> # Ouya T30 boot-tested
Suggested-by Thierry Reding <[email protected]>
Signed-off-by Dmitry Osipenko <[email protected]>

Applied, thanks.

Takashi

Dmitry Osipenko

author

2021-01-20 00:33 UTC #9 lore raw

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

In reply to 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 <[email protected]> # Ouya T30 audio works

Tested-by Matt Merhar <[email protected]> # Ouya T30 boot-tested

Tested-by Dmitry Osipenko <[email protected]> # Nexus7 T30 audio works

Tested-by Nicolas Chauvet <[email protected]> # TK1 boot-tested

Signed-off-by Dmitry Osipenko <[email protected]>

---
 sound/soc/tegra/tegra30_ahub.c | 2 ++
 1 file changed, 2 insertions(+)

sound/soc/tegra/tegra30_ahub.c +2 -0

index 156e3b9d613c..8c32333cc08c 100644
@@ -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 },

Signature · 1 line

2.29.2

Dmitry Osipenko

author

2021-01-20 00:33 UTC #10 lore raw

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

In reply to Dmitry Osipenko

Use clk_bulk helpers to make code cleaner.

Tested-by Peter Geis <[email protected]> # Ouya T30 audio works

Tested-by Matt Merhar <[email protected]> # Ouya T30 boot-tested

Tested-by Dmitry Osipenko <[email protected]> # Nexus7 T30 audio works

Tested-by Nicolas Chauvet <[email protected]> # TK1 boot-tested

Acked-by Thierry Reding <[email protected]>

Signed-off-by Dmitry Osipenko <[email protected]>

---
 sound/soc/tegra/tegra30_ahub.c | 30 +++++++-----------------------
 sound/soc/tegra/tegra30_ahub.h |  4 ++--
 2 files changed, 9 insertions(+), 25 deletions(-)

sound/soc/tegra/tegra30_ahub.c +7 ~~-23~~

index 8c32333cc08c..12ca8e3ca4f6 100644
@@ -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);

sound/soc/tegra/tegra30_ahub.h +2 -2

index 6889c5f23d02..01480d7dc940 100644
@@ -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;

Signature · 1 line

2.29.2

Dmitry Osipenko

author

2021-01-20 00:33 UTC #11 lore raw

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

In reply to 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 <[email protected]> # Ouya T30 audio works

Tested-by Matt Merhar <[email protected]> # Ouya T30 boot-tested

Tested-by Dmitry Osipenko <[email protected]> # Nexus7 T30 audio works

Tested-by Nicolas Chauvet <[email protected]> # TK1 boot-tested

Signed-off-by Dmitry Osipenko <[email protected]>

---
 sound/soc/tegra/tegra30_ahub.c | 36 ++++++++++++++++++++++++++++++----
 sound/soc/tegra/tegra30_ahub.h |  1 +
 2 files changed, 33 insertions(+), 4 deletions(-)

sound/soc/tegra/tegra30_ahub.c +32 -4

index 12ca8e3ca4f6..9ef05ca4f6c4 100644
@@ -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))

sound/soc/tegra/tegra30_ahub.h +1 -0

index 01480d7dc940..3b85244f87f1 100644
@@ -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;

Signature · 1 line

2.29.2

Takashi Iwai

2021-01-25 15:19 UTC #12 lore raw

In reply to Dmitry Osipenko

Quoted 33 lines · 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/.

Quoted 8 lines

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

Quoted 9 lines

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

Signature · 1 line

2.29.2

Mark Brown

maintainer

2021-01-25 18:04 UTC #13 lore raw

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

In reply to Dmitry Osipenko

Quoted 11 lines · 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.
[...]

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