| Message ID | 20210302112123.24161-6-digetx@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add missing reset controls to NVIDIA Tegra ASoC drivers | expand |
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.
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
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!
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
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(-)