linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1 0/5] Add missing reset controls to NVIDIA Tegra ASoC drivers
@ 2021-03-02 11:21 Dmitry Osipenko
  2021-03-02 11:21 ` [PATCH v1 1/5] reset: Allow devm_reset_control_array_get() to get resets in a released state Dmitry Osipenko
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Dmitry Osipenko @ 2021-03-02 11:21 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Mark Brown, Takashi Iwai,
	Jaroslav Kysela, Philipp Zabel, Paul Fertser
  Cc: alsa-devel, devicetree, linux-tegra, linux-kernel

Hi,

This series adds missing hardware reset controls to I2S and AC97 drivers.
Currently drivers happen to work properly because reset is implicitly
deasserted by tegra-clk driver, but clk driver shouldn't touch the resets
and we need to fix it because this breaks other Tegra drivers. Previously
we fixed the resets of the AHUB and HDMI codec drivers, but turned out
that we missed the I2C and AC97 drivers.

Thanks to Paul Fertser for testing the pending clk patches and finding
that audio got broken on Tegra20 AC100 netbook because of the missing I2S
reset.

Dmitry Osipenko (5):
  reset: Allow devm_reset_control_array_get() to get resets in a
    released state
  reset: Add devm_reset_control_array_get_exclusive_released()
  ASoC: tegra20: ac97: Add reset control
  ASoC: tegra20: i2s: Add reset control
  ASoC: tegra30: i2s: Add reset control

 drivers/reset/core.c           |  8 ++++++--
 include/linux/reset.h          | 20 +++++++++++++------
 sound/soc/tegra/tegra20_ac97.c | 21 ++++++++++++++++++++
 sound/soc/tegra/tegra20_ac97.h |  1 +
 sound/soc/tegra/tegra20_i2s.c  | 31 +++++++++++++++++++++++++++++
 sound/soc/tegra/tegra20_i2s.h  |  1 +
 sound/soc/tegra/tegra30_ahub.c | 14 ++++++++++---
 sound/soc/tegra/tegra30_i2s.c  | 36 +++++++++++++++++++++++++++++++++-
 sound/soc/tegra/tegra30_i2s.h  |  1 +
 9 files changed, 121 insertions(+), 12 deletions(-)

-- 
2.29.2


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v1 1/5] reset: Allow devm_reset_control_array_get() to get resets in a released state
  2021-03-02 11:21 [PATCH v1 0/5] Add missing reset controls to NVIDIA Tegra ASoC drivers Dmitry Osipenko
@ 2021-03-02 11:21 ` Dmitry Osipenko
  2021-03-02 11:21 ` [PATCH v1 2/5] reset: Add devm_reset_control_array_get_exclusive_released() Dmitry Osipenko
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Dmitry Osipenko @ 2021-03-02 11:21 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Mark Brown, Takashi Iwai,
	Jaroslav Kysela, Philipp Zabel, Paul Fertser
  Cc: alsa-devel, devicetree, linux-tegra, linux-kernel

Allow devm_reset_control_array_get() to get resets in a released state
in order to make it possible to extend reset-API with resource-managed
variants of retrieving resets array in a released state. In particular
this is needed by NVIDIA Tegra drivers.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/reset/core.c  |  8 ++++++--
 include/linux/reset.h | 14 ++++++++------
 2 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/drivers/reset/core.c b/drivers/reset/core.c
index dbf881b586d9..f36de3d3849b 100644
--- a/drivers/reset/core.c
+++ b/drivers/reset/core.c
@@ -985,6 +985,8 @@ EXPORT_SYMBOL_GPL(of_reset_control_array_get);
  * @dev: device that requests the list of reset controls
  * @shared: whether reset controls are shared or not
  * @optional: whether it is optional to get the reset controls
+ * @acquired: only one reset control may be acquired for a given controller
+ *            and ID
  *
  * The reset control array APIs are intended for a list of resets
  * that just have to be asserted or deasserted, without any
@@ -993,7 +995,8 @@ EXPORT_SYMBOL_GPL(of_reset_control_array_get);
  * Returns pointer to allocated reset_control on success or error on failure
  */
 struct reset_control *
-devm_reset_control_array_get(struct device *dev, bool shared, bool optional)
+devm_reset_control_array_get(struct device *dev, bool shared, bool optional,
+			     bool acquired)
 {
 	struct reset_control **ptr, *rstc;
 
@@ -1002,7 +1005,8 @@ devm_reset_control_array_get(struct device *dev, bool shared, bool optional)
 	if (!ptr)
 		return ERR_PTR(-ENOMEM);
 
-	rstc = of_reset_control_array_get(dev->of_node, shared, optional, true);
+	rstc = of_reset_control_array_get(dev->of_node, shared, optional,
+					  acquired);
 	if (IS_ERR_OR_NULL(rstc)) {
 		devres_free(ptr);
 		return rstc;
diff --git a/include/linux/reset.h b/include/linux/reset.h
index b9109efa2a5c..3bee086f1f06 100644
--- a/include/linux/reset.h
+++ b/include/linux/reset.h
@@ -33,7 +33,8 @@ struct reset_control *__devm_reset_control_get(struct device *dev,
 				     bool optional, bool acquired);
 
 struct reset_control *devm_reset_control_array_get(struct device *dev,
-						   bool shared, bool optional);
+						   bool shared, bool optional,
+						   bool acquired);
 struct reset_control *of_reset_control_array_get(struct device_node *np,
 						 bool shared, bool optional,
 						 bool acquired);
@@ -105,7 +106,8 @@ static inline struct reset_control *__devm_reset_control_get(
 }
 
 static inline struct reset_control *
-devm_reset_control_array_get(struct device *dev, bool shared, bool optional)
+devm_reset_control_array_get(struct device *dev, bool shared, bool optional,
+			     bool acquired)
 {
 	return optional ? NULL : ERR_PTR(-ENOTSUPP);
 }
@@ -511,25 +513,25 @@ static inline struct reset_control *devm_reset_control_get_by_index(
 static inline struct reset_control *
 devm_reset_control_array_get_exclusive(struct device *dev)
 {
-	return devm_reset_control_array_get(dev, false, false);
+	return devm_reset_control_array_get(dev, false, false, true);
 }
 
 static inline struct reset_control *
 devm_reset_control_array_get_shared(struct device *dev)
 {
-	return devm_reset_control_array_get(dev, true, false);
+	return devm_reset_control_array_get(dev, true, false, true);
 }
 
 static inline struct reset_control *
 devm_reset_control_array_get_optional_exclusive(struct device *dev)
 {
-	return devm_reset_control_array_get(dev, false, true);
+	return devm_reset_control_array_get(dev, false, true, true);
 }
 
 static inline struct reset_control *
 devm_reset_control_array_get_optional_shared(struct device *dev)
 {
-	return devm_reset_control_array_get(dev, true, true);
+	return devm_reset_control_array_get(dev, true, true, true);
 }
 
 static inline struct reset_control *
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v1 2/5] reset: Add devm_reset_control_array_get_exclusive_released()
  2021-03-02 11:21 [PATCH v1 0/5] Add missing reset controls to NVIDIA Tegra ASoC drivers Dmitry Osipenko
  2021-03-02 11:21 ` [PATCH v1 1/5] reset: Allow devm_reset_control_array_get() to get resets in a released state Dmitry Osipenko
@ 2021-03-02 11:21 ` Dmitry Osipenko
  2021-03-02 11:21 ` [PATCH v1 3/5] ASoC: tegra20: ac97: Add reset control Dmitry Osipenko
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Dmitry Osipenko @ 2021-03-02 11:21 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Mark Brown, Takashi Iwai,
	Jaroslav Kysela, Philipp Zabel, Paul Fertser
  Cc: alsa-devel, devicetree, linux-tegra, linux-kernel

Add devm_reset_control_array_get_exclusive_released() which is wanted by
NVIDIA Tegra drivers.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 include/linux/reset.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/include/linux/reset.h b/include/linux/reset.h
index 3bee086f1f06..ab240a8648ee 100644
--- a/include/linux/reset.h
+++ b/include/linux/reset.h
@@ -534,6 +534,12 @@ devm_reset_control_array_get_optional_shared(struct device *dev)
 	return devm_reset_control_array_get(dev, true, true, true);
 }
 
+static inline struct reset_control *
+devm_reset_control_array_get_exclusive_released(struct device *dev)
+{
+	return devm_reset_control_array_get(dev, false, false, false);
+}
+
 static inline struct reset_control *
 of_reset_control_array_get_exclusive(struct device_node *node)
 {
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v1 3/5] ASoC: tegra20: ac97: Add reset control
  2021-03-02 11:21 [PATCH v1 0/5] Add missing reset controls to NVIDIA Tegra ASoC drivers Dmitry Osipenko
  2021-03-02 11:21 ` [PATCH v1 1/5] reset: Allow devm_reset_control_array_get() to get resets in a released state Dmitry Osipenko
  2021-03-02 11:21 ` [PATCH v1 2/5] reset: Add devm_reset_control_array_get_exclusive_released() Dmitry Osipenko
@ 2021-03-02 11:21 ` Dmitry Osipenko
  2021-03-02 11:21 ` [PATCH v1 4/5] ASoC: tegra20: i2s: " Dmitry Osipenko
  2021-03-02 11:21 ` [PATCH v1 5/5] ASoC: tegra30: " Dmitry Osipenko
  4 siblings, 0 replies; 9+ messages in thread
From: Dmitry Osipenko @ 2021-03-02 11:21 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Mark Brown, Takashi Iwai,
	Jaroslav Kysela, Philipp Zabel, Paul Fertser
  Cc: alsa-devel, devicetree, linux-tegra, linux-kernel

Tegra20 AC97 driver doesn't manage the AC97 controller reset, relying on
implicit deassertion of the reset by tegra-clk driver, which needs to be
fixed since this behaviour is unacceptable by other Tegra drivers. Add
explicit reset control to the Tegra20 AC97 driver.

Note that AC97 reset was always specified in Tegra20 device-tree, hence
DTB ABI changes aren't required.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 sound/soc/tegra/tegra20_ac97.c | 21 +++++++++++++++++++++
 sound/soc/tegra/tegra20_ac97.h |  1 +
 2 files changed, 22 insertions(+)

diff --git a/sound/soc/tegra/tegra20_ac97.c b/sound/soc/tegra/tegra20_ac97.c
index 06c728ae17ed..c454a34c15c4 100644
--- a/sound/soc/tegra/tegra20_ac97.c
+++ b/sound/soc/tegra/tegra20_ac97.c
@@ -21,6 +21,7 @@
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/regmap.h>
+#include <linux/reset.h>
 #include <linux/slab.h>
 #include <sound/core.h>
 #include <sound/pcm.h>
@@ -313,6 +314,12 @@ static int tegra20_ac97_platform_probe(struct platform_device *pdev)
 	}
 	dev_set_drvdata(&pdev->dev, ac97);
 
+	ac97->reset = devm_reset_control_get_exclusive(&pdev->dev, "ac97");
+	if (IS_ERR(ac97->reset)) {
+		dev_err(&pdev->dev, "Can't retrieve ac97 reset\n");
+		return PTR_ERR(ac97->reset);
+	}
+
 	ac97->clk_ac97 = devm_clk_get(&pdev->dev, NULL);
 	if (IS_ERR(ac97->clk_ac97)) {
 		dev_err(&pdev->dev, "Can't retrieve ac97 clock\n");
@@ -364,12 +371,26 @@ static int tegra20_ac97_platform_probe(struct platform_device *pdev)
 	ac97->playback_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
 	ac97->playback_dma_data.maxburst = 4;
 
+	ret = reset_control_assert(ac97->reset);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to assert AC'97 reset: %d\n", ret);
+		goto err_clk_put;
+	}
+
 	ret = clk_prepare_enable(ac97->clk_ac97);
 	if (ret) {
 		dev_err(&pdev->dev, "clk_enable failed: %d\n", ret);
 		goto err_clk_put;
 	}
 
+	usleep_range(10, 100);
+
+	ret = reset_control_deassert(ac97->reset);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to deassert AC'97 reset: %d\n", ret);
+		goto err_clk_disable_unprepare;
+	}
+
 	ret = snd_soc_set_ac97_ops(&tegra20_ac97_ops);
 	if (ret) {
 		dev_err(&pdev->dev, "Failed to set AC'97 ops: %d\n", ret);
diff --git a/sound/soc/tegra/tegra20_ac97.h b/sound/soc/tegra/tegra20_ac97.h
index e467cd1ff2ca..870ea09ff301 100644
--- a/sound/soc/tegra/tegra20_ac97.h
+++ b/sound/soc/tegra/tegra20_ac97.h
@@ -78,6 +78,7 @@ struct tegra20_ac97 {
 	struct clk *clk_ac97;
 	struct snd_dmaengine_dai_dma_data capture_dma_data;
 	struct snd_dmaengine_dai_dma_data playback_dma_data;
+	struct reset_control *reset;
 	struct regmap *regmap;
 	int reset_gpio;
 	int sync_gpio;
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v1 4/5] ASoC: tegra20: i2s: Add reset control
  2021-03-02 11:21 [PATCH v1 0/5] Add missing reset controls to NVIDIA Tegra ASoC drivers Dmitry Osipenko
                   ` (2 preceding siblings ...)
  2021-03-02 11:21 ` [PATCH v1 3/5] ASoC: tegra20: ac97: Add reset control Dmitry Osipenko
@ 2021-03-02 11:21 ` Dmitry Osipenko
  2021-03-02 11:21 ` [PATCH v1 5/5] ASoC: tegra30: " Dmitry Osipenko
  4 siblings, 0 replies; 9+ messages in thread
From: Dmitry Osipenko @ 2021-03-02 11:21 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Mark Brown, Takashi Iwai,
	Jaroslav Kysela, Philipp Zabel, Paul Fertser
  Cc: alsa-devel, devicetree, linux-tegra, linux-kernel

The I2S reset may be asserted at a boot time, in particular this is the
case on Tegra20 AC100 netbook. Tegra20 I2S driver doesn't manage the
reset control and currently it happens to work because reset is implicitly
deasserted by the tegra-clk driver when I2S clock is enabled. The I2S
permanently stays in a reset once tegra-clk is fixed to not touch the
resets, which it shouldn't be doing. Add reset control to the Tegra20
I2S driver.

Note that I2S reset was always specified in Tegra20 device-tree, hence
DTB ABI changes aren't required.

Tested-by: Paul Fertser <fercerpav@gmail.com> # T20 AC100
Reported-by: Paul Fertser <fercerpav@gmail.com>
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 sound/soc/tegra/tegra20_i2s.c | 31 +++++++++++++++++++++++++++++++
 sound/soc/tegra/tegra20_i2s.h |  1 +
 2 files changed, 32 insertions(+)

diff --git a/sound/soc/tegra/tegra20_i2s.c b/sound/soc/tegra/tegra20_i2s.c
index d7a3d046c8f8..c0af5352b483 100644
--- a/sound/soc/tegra/tegra20_i2s.c
+++ b/sound/soc/tegra/tegra20_i2s.c
@@ -22,6 +22,7 @@
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/regmap.h>
+#include <linux/reset.h>
 #include <linux/slab.h>
 #include <sound/core.h>
 #include <sound/pcm.h>
@@ -37,6 +38,8 @@ static int tegra20_i2s_runtime_suspend(struct device *dev)
 {
 	struct tegra20_i2s *i2s = dev_get_drvdata(dev);
 
+	regcache_cache_only(i2s->regmap, true);
+
 	clk_disable_unprepare(i2s->clk_i2s);
 
 	return 0;
@@ -47,13 +50,35 @@ static int tegra20_i2s_runtime_resume(struct device *dev)
 	struct tegra20_i2s *i2s = dev_get_drvdata(dev);
 	int ret;
 
+	ret = reset_control_assert(i2s->reset);
+	if (ret)
+		return ret;
+
 	ret = clk_prepare_enable(i2s->clk_i2s);
 	if (ret) {
 		dev_err(dev, "clk_enable failed: %d\n", ret);
 		return ret;
 	}
 
+	usleep_range(10, 100);
+
+	ret = reset_control_deassert(i2s->reset);
+	if (ret)
+		goto disable_clocks;
+
+	regcache_cache_only(i2s->regmap, false);
+	regcache_mark_dirty(i2s->regmap);
+
+	ret = regcache_sync(i2s->regmap);
+	if (ret)
+		goto disable_clocks;
+
 	return 0;
+
+disable_clocks:
+	clk_disable_unprepare(i2s->clk_i2s);
+
+	return ret;
 }
 
 static int tegra20_i2s_set_fmt(struct snd_soc_dai *dai,
@@ -339,6 +364,12 @@ static int tegra20_i2s_platform_probe(struct platform_device *pdev)
 	i2s->dai = tegra20_i2s_dai_template;
 	i2s->dai.name = dev_name(&pdev->dev);
 
+	i2s->reset = devm_reset_control_get_exclusive(&pdev->dev, "i2s");
+	if (IS_ERR(i2s->reset)) {
+		dev_err(&pdev->dev, "Can't retrieve i2s reset\n");
+		return PTR_ERR(i2s->reset);
+	}
+
 	i2s->clk_i2s = clk_get(&pdev->dev, NULL);
 	if (IS_ERR(i2s->clk_i2s)) {
 		dev_err(&pdev->dev, "Can't retrieve i2s clock\n");
diff --git a/sound/soc/tegra/tegra20_i2s.h b/sound/soc/tegra/tegra20_i2s.h
index 628d3ca09f42..8233e5fa2eff 100644
--- a/sound/soc/tegra/tegra20_i2s.h
+++ b/sound/soc/tegra/tegra20_i2s.h
@@ -144,6 +144,7 @@ struct tegra20_i2s {
 	struct snd_dmaengine_dai_dma_data capture_dma_data;
 	struct snd_dmaengine_dai_dma_data playback_dma_data;
 	struct regmap *regmap;
+	struct reset_control *reset;
 };
 
 #endif
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v1 5/5] ASoC: tegra30: i2s: Add reset control
  2021-03-02 11:21 [PATCH v1 0/5] Add missing reset controls to NVIDIA Tegra ASoC drivers Dmitry Osipenko
                   ` (3 preceding siblings ...)
  2021-03-02 11:21 ` [PATCH v1 4/5] ASoC: tegra20: i2s: " Dmitry Osipenko
@ 2021-03-02 11:21 ` Dmitry Osipenko
  2021-03-03  8:28   ` Dmitry Osipenko
  4 siblings, 1 reply; 9+ messages in thread
From: Dmitry Osipenko @ 2021-03-02 11:21 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Mark Brown, Takashi Iwai,
	Jaroslav Kysela, Philipp Zabel, Paul Fertser
  Cc: alsa-devel, devicetree, linux-tegra, linux-kernel

The I2S reset may be asserted at a boot time. Tegra30 I2S driver doesn't
manage the reset control and currently it happens to work because reset
is implicitly deasserted by the Tegra AHUB driver, but the reset of I2C
controller should be synchronous and I2S clock is disabled when AHUB is
reset. Add reset control to the Tegra30 I2S driver.

Note that I2S reset was always specified in Tegra30+ device-trees, hence
DTB ABI changes aren't required. Also note that AHUB touches I2S resets,
hence AHUB resets are now requested in a released state, allowing both
drivers to control the I2S resets together.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 sound/soc/tegra/tegra30_ahub.c | 14 ++++++++++---
 sound/soc/tegra/tegra30_i2s.c  | 36 +++++++++++++++++++++++++++++++++-
 sound/soc/tegra/tegra30_i2s.h  |  1 +
 3 files changed, 47 insertions(+), 4 deletions(-)

diff --git a/sound/soc/tegra/tegra30_ahub.c b/sound/soc/tegra/tegra30_ahub.c
index 9ef05ca4f6c4..1e7803819a17 100644
--- a/sound/soc/tegra/tegra30_ahub.c
+++ b/sound/soc/tegra/tegra30_ahub.c
@@ -65,13 +65,17 @@ static int tegra30_ahub_runtime_resume(struct device *dev)
 {
 	int ret;
 
-	ret = reset_control_assert(ahub->reset);
+	ret = reset_control_acquire(ahub->reset);
 	if (ret)
 		return ret;
 
+	ret = reset_control_assert(ahub->reset);
+	if (ret)
+		goto release_reset;
+
 	ret = clk_bulk_prepare_enable(ahub->nclocks, ahub->clocks);
 	if (ret)
-		return ret;
+		goto release_reset;
 
 	usleep_range(10, 100);
 
@@ -92,10 +96,14 @@ static int tegra30_ahub_runtime_resume(struct device *dev)
 	if (ret)
 		goto disable_clocks;
 
+	reset_control_release(ahub->reset);
+
 	return 0;
 
 disable_clocks:
 	clk_bulk_disable_unprepare(ahub->nclocks, ahub->clocks);
+release_reset:
+	reset_control_release(ahub->reset);
 
 	return ret;
 }
@@ -579,7 +587,7 @@ static int tegra30_ahub_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	ahub->reset = devm_reset_control_array_get_exclusive(&pdev->dev);
+	ahub->reset = devm_reset_control_array_get_exclusive_released(&pdev->dev);
 	if (IS_ERR(ahub->reset)) {
 		dev_err(&pdev->dev, "Can't get resets: %pe\n", ahub->reset);
 		return PTR_ERR(ahub->reset);
diff --git a/sound/soc/tegra/tegra30_i2s.c b/sound/soc/tegra/tegra30_i2s.c
index 6740df541508..01bff9fda784 100644
--- a/sound/soc/tegra/tegra30_i2s.c
+++ b/sound/soc/tegra/tegra30_i2s.c
@@ -23,6 +23,7 @@
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/regmap.h>
+#include <linux/reset.h>
 #include <linux/slab.h>
 #include <sound/core.h>
 #include <sound/pcm.h>
@@ -42,6 +43,7 @@ static int tegra30_i2s_runtime_suspend(struct device *dev)
 	regcache_cache_only(i2s->regmap, true);
 
 	clk_disable_unprepare(i2s->clk_i2s);
+	reset_control_release(i2s->reset);
 
 	return 0;
 }
@@ -51,15 +53,41 @@ static int tegra30_i2s_runtime_resume(struct device *dev)
 	struct tegra30_i2s *i2s = dev_get_drvdata(dev);
 	int ret;
 
+	ret = reset_control_acquire(i2s->reset);
+	if (ret)
+		return ret;
+
+	ret = reset_control_assert(i2s->reset);
+	if (ret)
+		goto release_reset;
+
 	ret = clk_prepare_enable(i2s->clk_i2s);
 	if (ret) {
 		dev_err(dev, "clk_enable failed: %d\n", ret);
-		return ret;
+		goto release_reset;
 	}
 
+	usleep_range(10, 100);
+
+	ret = reset_control_deassert(i2s->reset);
+	if (ret)
+		goto disable_clocks;
+
 	regcache_cache_only(i2s->regmap, false);
+	regcache_mark_dirty(i2s->regmap);
+
+	ret = regcache_sync(i2s->regmap);
+	if (ret)
+		goto disable_clocks;
 
 	return 0;
+
+disable_clocks:
+	clk_disable_unprepare(i2s->clk_i2s);
+release_reset:
+	reset_control_release(i2s->reset);
+
+	return ret;
 }
 
 static int tegra30_i2s_set_fmt(struct snd_soc_dai *dai,
@@ -418,6 +446,12 @@ static int tegra30_i2s_platform_probe(struct platform_device *pdev)
 	i2s->dai = tegra30_i2s_dai_template;
 	i2s->dai.name = dev_name(&pdev->dev);
 
+	i2s->reset = devm_reset_control_get_exclusive_released(&pdev->dev, "i2s");
+	if (IS_ERR(i2s->reset)) {
+		dev_err(&pdev->dev, "Can't retrieve i2s reset\n");
+		return PTR_ERR(i2s->reset);
+	}
+
 	ret = of_property_read_u32_array(pdev->dev.of_node,
 					 "nvidia,ahub-cif-ids", cif_ids,
 					 ARRAY_SIZE(cif_ids));
diff --git a/sound/soc/tegra/tegra30_i2s.h b/sound/soc/tegra/tegra30_i2s.h
index 0b1f3125a7c0..e58375ca0a59 100644
--- a/sound/soc/tegra/tegra30_i2s.h
+++ b/sound/soc/tegra/tegra30_i2s.h
@@ -235,6 +235,7 @@ struct tegra30_i2s {
 	struct snd_dmaengine_dai_dma_data playback_dma_data;
 	struct regmap *regmap;
 	struct snd_dmaengine_pcm_config dma_config;
+	struct reset_control *reset;
 };
 
 #endif
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v1 5/5] ASoC: tegra30: i2s: Add reset control
  2021-03-02 11:21 ` [PATCH v1 5/5] ASoC: tegra30: " Dmitry Osipenko
@ 2021-03-03  8:28   ` Dmitry Osipenko
  2021-03-03 12:09     ` Philipp Zabel
  0 siblings, 1 reply; 9+ messages in thread
From: Dmitry Osipenko @ 2021-03-03  8:28 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Mark Brown, Takashi Iwai,
	Jaroslav Kysela, Philipp Zabel, Paul Fertser
  Cc: alsa-devel, devicetree, linux-tegra, linux-kernel

02.03.2021 14:21, Dmitry Osipenko пишет:
> The I2S reset may be asserted at a boot time. Tegra30 I2S driver doesn't
> manage the reset control and currently it happens to work because reset
> is implicitly deasserted by the Tegra AHUB driver, but the reset of I2C
> controller should be synchronous and I2S clock is disabled when AHUB is
> reset. Add reset control to the Tegra30 I2S driver.
> 
> Note that I2S reset was always specified in Tegra30+ device-trees, hence
> DTB ABI changes aren't required. Also note that AHUB touches I2S resets,
> hence AHUB resets are now requested in a released state, allowing both
> drivers to control the I2S resets together.
> 
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
>  sound/soc/tegra/tegra30_ahub.c | 14 ++++++++++---
>  sound/soc/tegra/tegra30_i2s.c  | 36 +++++++++++++++++++++++++++++++++-
>  sound/soc/tegra/tegra30_i2s.h  |  1 +
>  3 files changed, 47 insertions(+), 4 deletions(-)

...
> @@ -579,7 +587,7 @@ static int tegra30_ahub_probe(struct platform_device *pdev)
>  	if (ret)
>  		return ret;
>  
> -	ahub->reset = devm_reset_control_array_get_exclusive(&pdev->dev);
> +	ahub->reset = devm_reset_control_array_get_exclusive_released(&pdev->dev);

Thinking a bit more about this, it looks like we actually want something
like:

	devm_reset_control_array_get_exclusive_released_named()

that will request resets by given names and in a given order, similarly
to devm_clk_bulk_get(). This will be very handy for both Tegra audio and
GPU drivers. I'll prepare a v2 if there are no objections.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v1 5/5] ASoC: tegra30: i2s: Add reset control
  2021-03-03  8:28   ` Dmitry Osipenko
@ 2021-03-03 12:09     ` Philipp Zabel
  2021-03-04  9:42       ` Dmitry Osipenko
  0 siblings, 1 reply; 9+ messages in thread
From: Philipp Zabel @ 2021-03-03 12:09 UTC (permalink / raw)
  To: Dmitry Osipenko, Thierry Reding, Jonathan Hunter, Mark Brown,
	Takashi Iwai, Jaroslav Kysela, Paul Fertser
  Cc: alsa-devel, devicetree, linux-tegra, linux-kernel

Hi Dmitry,

On Wed, 2021-03-03 at 11:28 +0300, Dmitry Osipenko wrote:
> 02.03.2021 14:21, Dmitry Osipenko пишет:
> > The I2S reset may be asserted at a boot time. Tegra30 I2S driver doesn't
> > manage the reset control and currently it happens to work because reset
> > is implicitly deasserted by the Tegra AHUB driver, but the reset of I2C
> > controller should be synchronous and I2S clock is disabled when AHUB is
> > reset. Add reset control to the Tegra30 I2S driver.
> > 
> > Note that I2S reset was always specified in Tegra30+ device-trees, hence
> > DTB ABI changes aren't required. Also note that AHUB touches I2S resets,
> > hence AHUB resets are now requested in a released state, allowing both
> > drivers to control the I2S resets together.
> > 
> > Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> > ---
> >  sound/soc/tegra/tegra30_ahub.c | 14 ++++++++++---
> >  sound/soc/tegra/tegra30_i2s.c  | 36 +++++++++++++++++++++++++++++++++-
> >  sound/soc/tegra/tegra30_i2s.h  |  1 +
> >  3 files changed, 47 insertions(+), 4 deletions(-)
> 
> ...
> > @@ -579,7 +587,7 @@ static int tegra30_ahub_probe(struct platform_device *pdev)
> >  	if (ret)
> >  		return ret;
> >  
> > -	ahub->reset = devm_reset_control_array_get_exclusive(&pdev->dev);
> > +	ahub->reset = devm_reset_control_array_get_exclusive_released(&pdev->dev);
> 
> Thinking a bit more about this, it looks like we actually want something
> like:
> 
> 	devm_reset_control_array_get_exclusive_released_named()
> 
> that will request resets by given names and in a given order, similarly
> to devm_clk_bulk_get(). This will be very handy for both Tegra audio and
> GPU drivers. I'll prepare a v2 if there are no objections.

I do have an untested reset control bulk API patch that I've just never
finished, because I had no user. I'll send you an RFC, let me know if
you can build on that.

regards
Philipp

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v1 5/5] ASoC: tegra30: i2s: Add reset control
  2021-03-03 12:09     ` Philipp Zabel
@ 2021-03-04  9:42       ` Dmitry Osipenko
  0 siblings, 0 replies; 9+ messages in thread
From: Dmitry Osipenko @ 2021-03-04  9:42 UTC (permalink / raw)
  To: Philipp Zabel, Thierry Reding, Jonathan Hunter, Mark Brown,
	Takashi Iwai, Jaroslav Kysela, Paul Fertser
  Cc: alsa-devel, devicetree, linux-tegra, linux-kernel

03.03.2021 15:09, Philipp Zabel пишет:
> Hi Dmitry,
> 
> On Wed, 2021-03-03 at 11:28 +0300, Dmitry Osipenko wrote:
>> 02.03.2021 14:21, Dmitry Osipenko пишет:
>>> The I2S reset may be asserted at a boot time. Tegra30 I2S driver doesn't
>>> manage the reset control and currently it happens to work because reset
>>> is implicitly deasserted by the Tegra AHUB driver, but the reset of I2C
>>> controller should be synchronous and I2S clock is disabled when AHUB is
>>> reset. Add reset control to the Tegra30 I2S driver.
>>>
>>> Note that I2S reset was always specified in Tegra30+ device-trees, hence
>>> DTB ABI changes aren't required. Also note that AHUB touches I2S resets,
>>> hence AHUB resets are now requested in a released state, allowing both
>>> drivers to control the I2S resets together.
>>>
>>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>>> ---
>>>  sound/soc/tegra/tegra30_ahub.c | 14 ++++++++++---
>>>  sound/soc/tegra/tegra30_i2s.c  | 36 +++++++++++++++++++++++++++++++++-
>>>  sound/soc/tegra/tegra30_i2s.h  |  1 +
>>>  3 files changed, 47 insertions(+), 4 deletions(-)
>>
>> ...
>>> @@ -579,7 +587,7 @@ static int tegra30_ahub_probe(struct platform_device *pdev)
>>>  	if (ret)
>>>  		return ret;
>>>  
>>> -	ahub->reset = devm_reset_control_array_get_exclusive(&pdev->dev);
>>> +	ahub->reset = devm_reset_control_array_get_exclusive_released(&pdev->dev);
>>
>> Thinking a bit more about this, it looks like we actually want something
>> like:
>>
>> 	devm_reset_control_array_get_exclusive_released_named()
>>
>> that will request resets by given names and in a given order, similarly
>> to devm_clk_bulk_get(). This will be very handy for both Tegra audio and
>> GPU drivers. I'll prepare a v2 if there are no objections.
> 
> I do have an untested reset control bulk API patch that I've just never
> finished, because I had no user. I'll send you an RFC, let me know if
> you can build on that.

Hello, Philipp! Thank you very much for sharing the patch, it should be
very useful!

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2021-03-04  9:43 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-02 11:21 [PATCH v1 0/5] Add missing reset controls to NVIDIA Tegra ASoC drivers Dmitry Osipenko
2021-03-02 11:21 ` [PATCH v1 1/5] reset: Allow devm_reset_control_array_get() to get resets in a released state Dmitry Osipenko
2021-03-02 11:21 ` [PATCH v1 2/5] reset: Add devm_reset_control_array_get_exclusive_released() Dmitry Osipenko
2021-03-02 11:21 ` [PATCH v1 3/5] ASoC: tegra20: ac97: Add reset control Dmitry Osipenko
2021-03-02 11:21 ` [PATCH v1 4/5] ASoC: tegra20: i2s: " Dmitry Osipenko
2021-03-02 11:21 ` [PATCH v1 5/5] ASoC: tegra30: " Dmitry Osipenko
2021-03-03  8:28   ` Dmitry Osipenko
2021-03-03 12:09     ` Philipp Zabel
2021-03-04  9:42       ` Dmitry Osipenko

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