From: Dmitry Osipenko <digetx@gmail.com> To: Thierry Reding <thierry.reding@gmail.com> Cc: Jonathan Hunter <jonathanh@nvidia.com>, Sameer Pujar <spujar@nvidia.com>, Peter Geis <pgwipeout@gmail.com>, Nicolas Chauvet <kwizart@gmail.com>, Takashi Iwai <tiwai@suse.com>, Jaroslav Kysela <perex@perex.cz>, alsa-devel@alsa-project.org, linux-tegra@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v1 5/5] ASoC: tegra: ahub: Reset hardware properly Date: Mon, 18 Jan 2021 03:02:38 +0300 [thread overview] Message-ID: <c2f3d149-0781-3d0a-1d68-f6820386e84d@gmail.com> (raw) In-Reply-To: <YAG4aFADwo7dh/oR@ulmo> 15.01.2021 18:44, Thierry Reding пишет: > On Tue, Jan 12, 2021 at 03:58:34PM +0300, Dmitry Osipenko wrote: >> Assert hardware reset before clocks are enabled and then de-assert it >> after clocks are enabled. This brings hardware into a predictable state >> and removes relying on implicit de-assertion of resets which is done by >> the clk driver. >> >> Tested-by: Peter Geis <pgwipeout@gmail.com> >> Tested-by: Nicolas Chauvet <kwizart@gmail.com> >> Signed-off-by: Dmitry Osipenko <digetx@gmail.com> >> --- >> sound/soc/tegra/tegra30_ahub.c | 33 ++++++++++++++++----------------- >> sound/soc/tegra/tegra30_ahub.h | 1 + >> 2 files changed, 17 insertions(+), 17 deletions(-) >> >> diff --git a/sound/soc/tegra/tegra30_ahub.c b/sound/soc/tegra/tegra30_ahub.c >> index 4dbb58f7ea36..246cf6a373a1 100644 >> --- a/sound/soc/tegra/tegra30_ahub.c >> +++ b/sound/soc/tegra/tegra30_ahub.c >> @@ -65,10 +65,20 @@ 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; >> >> + ret = reset_control_reset(ahub->reset); >> + if (ret) { >> + clk_bulk_disable_unprepare(ahub->nclocks, ahub->clocks); >> + return ret; >> + } >> + >> regcache_cache_only(ahub->regmap_apbif, false); >> regcache_cache_only(ahub->regmap_ahub, false); >> >> @@ -462,7 +472,6 @@ static int tegra30_ahub_probe(struct platform_device *pdev) >> { >> const struct of_device_id *match; >> const struct tegra30_ahub_soc_data *soc_data; >> - struct reset_control *rst; >> struct resource *res0; >> void __iomem *regs_apbif, *regs_ahub; >> int ret = 0; >> @@ -475,22 +484,6 @@ static int tegra30_ahub_probe(struct platform_device *pdev) >> return -EINVAL; >> soc_data = match->data; >> >> - /* >> - * 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. >> - */ >> - rst = of_reset_control_array_get_exclusive(pdev->dev.of_node); >> - if (IS_ERR(rst)) { >> - dev_err(&pdev->dev, "Can't get reset: %p\n", rst); >> - return PTR_ERR(rst); >> - } >> - >> - ret = reset_control_deassert(rst); >> - reset_control_put(rst); >> - if (ret) >> - return ret; >> - >> ahub = devm_kzalloc(&pdev->dev, sizeof(struct tegra30_ahub), >> GFP_KERNEL); >> if (!ahub) >> @@ -507,6 +500,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 reset: %p\n", ahub->reset); > > I didn't notice that the prior patch already introduced this, but I'd > prefer for this to either be %pe so that the symbolic error name is > printed, or %ld with PTR_ERR(ahub->reset) to format this in a more > standard way that can be more easily grepped for and parsed. This is already fixed in v2. Good catch anyways, thanks. > It also seems like the prior patch that converts this to use > of_reset_control_array_get_exclusive() is a bit pointless now. Why not > just move to this directly instead? These are two independent changes. The previous patch fixed the missing resets, this patch changes the hardware initialization logic.
WARNING: multiple messages have this Message-ID (diff)
From: Dmitry Osipenko <digetx@gmail.com> To: Thierry Reding <thierry.reding@gmail.com> Cc: alsa-devel@alsa-project.org, Nicolas Chauvet <kwizart@gmail.com>, linux-kernel@vger.kernel.org, Sameer Pujar <spujar@nvidia.com>, Takashi Iwai <tiwai@suse.com>, Peter Geis <pgwipeout@gmail.com>, linux-tegra@vger.kernel.org, Jonathan Hunter <jonathanh@nvidia.com> Subject: Re: [PATCH v1 5/5] ASoC: tegra: ahub: Reset hardware properly Date: Mon, 18 Jan 2021 03:02:38 +0300 [thread overview] Message-ID: <c2f3d149-0781-3d0a-1d68-f6820386e84d@gmail.com> (raw) In-Reply-To: <YAG4aFADwo7dh/oR@ulmo> 15.01.2021 18:44, Thierry Reding пишет: > On Tue, Jan 12, 2021 at 03:58:34PM +0300, Dmitry Osipenko wrote: >> Assert hardware reset before clocks are enabled and then de-assert it >> after clocks are enabled. This brings hardware into a predictable state >> and removes relying on implicit de-assertion of resets which is done by >> the clk driver. >> >> Tested-by: Peter Geis <pgwipeout@gmail.com> >> Tested-by: Nicolas Chauvet <kwizart@gmail.com> >> Signed-off-by: Dmitry Osipenko <digetx@gmail.com> >> --- >> sound/soc/tegra/tegra30_ahub.c | 33 ++++++++++++++++----------------- >> sound/soc/tegra/tegra30_ahub.h | 1 + >> 2 files changed, 17 insertions(+), 17 deletions(-) >> >> diff --git a/sound/soc/tegra/tegra30_ahub.c b/sound/soc/tegra/tegra30_ahub.c >> index 4dbb58f7ea36..246cf6a373a1 100644 >> --- a/sound/soc/tegra/tegra30_ahub.c >> +++ b/sound/soc/tegra/tegra30_ahub.c >> @@ -65,10 +65,20 @@ 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; >> >> + ret = reset_control_reset(ahub->reset); >> + if (ret) { >> + clk_bulk_disable_unprepare(ahub->nclocks, ahub->clocks); >> + return ret; >> + } >> + >> regcache_cache_only(ahub->regmap_apbif, false); >> regcache_cache_only(ahub->regmap_ahub, false); >> >> @@ -462,7 +472,6 @@ static int tegra30_ahub_probe(struct platform_device *pdev) >> { >> const struct of_device_id *match; >> const struct tegra30_ahub_soc_data *soc_data; >> - struct reset_control *rst; >> struct resource *res0; >> void __iomem *regs_apbif, *regs_ahub; >> int ret = 0; >> @@ -475,22 +484,6 @@ static int tegra30_ahub_probe(struct platform_device *pdev) >> return -EINVAL; >> soc_data = match->data; >> >> - /* >> - * 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. >> - */ >> - rst = of_reset_control_array_get_exclusive(pdev->dev.of_node); >> - if (IS_ERR(rst)) { >> - dev_err(&pdev->dev, "Can't get reset: %p\n", rst); >> - return PTR_ERR(rst); >> - } >> - >> - ret = reset_control_deassert(rst); >> - reset_control_put(rst); >> - if (ret) >> - return ret; >> - >> ahub = devm_kzalloc(&pdev->dev, sizeof(struct tegra30_ahub), >> GFP_KERNEL); >> if (!ahub) >> @@ -507,6 +500,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 reset: %p\n", ahub->reset); > > I didn't notice that the prior patch already introduced this, but I'd > prefer for this to either be %pe so that the symbolic error name is > printed, or %ld with PTR_ERR(ahub->reset) to format this in a more > standard way that can be more easily grepped for and parsed. This is already fixed in v2. Good catch anyways, thanks. > It also seems like the prior patch that converts this to use > of_reset_control_array_get_exclusive() is a bit pointless now. Why not > just move to this directly instead? These are two independent changes. The previous patch fixed the missing resets, this patch changes the hardware initialization logic.
next prev parent reply other threads:[~2021-01-18 0:03 UTC|newest] Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-01-12 12:58 [PATCH v1 0/5] Clock and reset improvements for Tegra ALSA drivers Dmitry Osipenko 2021-01-12 12:58 ` Dmitry Osipenko 2021-01-12 12:58 ` [PATCH v1 1/5] ALSA: hda/tegra: Use clk_bulk helpers Dmitry Osipenko 2021-01-12 12:58 ` Dmitry Osipenko 2021-01-15 15:22 ` Thierry Reding 2021-01-15 15:22 ` Thierry Reding 2021-01-17 23:31 ` Dmitry Osipenko 2021-01-17 23:31 ` Dmitry Osipenko 2021-01-19 17:31 ` Thierry Reding 2021-01-19 17:31 ` Thierry Reding 2021-01-12 12:58 ` [PATCH v1 2/5] ALSA: hda/tegra: Reset hardware Dmitry Osipenko 2021-01-12 12:58 ` Dmitry Osipenko 2021-01-15 15:35 ` Thierry Reding 2021-01-15 15:35 ` Thierry Reding 2021-01-17 23:39 ` Dmitry Osipenko 2021-01-17 23:39 ` Dmitry Osipenko 2021-01-19 17:30 ` Thierry Reding 2021-01-19 17:30 ` Thierry Reding 2021-01-12 12:58 ` [PATCH v1 3/5] ASoC: tegra: ahub: Use of_reset_control_array_get_exclusive() Dmitry Osipenko 2021-01-12 12:58 ` Dmitry Osipenko 2021-01-15 15:37 ` Thierry Reding 2021-01-15 15:37 ` Thierry Reding 2021-01-17 23:57 ` Dmitry Osipenko 2021-01-17 23:57 ` Dmitry Osipenko 2021-01-12 12:58 ` [PATCH v1 4/5] ASoC: tegra: ahub: Use clk_bulk helpers Dmitry Osipenko 2021-01-12 12:58 ` Dmitry Osipenko 2021-01-15 15:38 ` Thierry Reding 2021-01-15 15:38 ` Thierry Reding 2021-01-12 12:58 ` [PATCH v1 5/5] ASoC: tegra: ahub: Reset hardware properly Dmitry Osipenko 2021-01-12 12:58 ` Dmitry Osipenko 2021-01-15 13:02 ` Dmitry Osipenko 2021-01-15 13:02 ` Dmitry Osipenko 2021-01-15 15:44 ` Thierry Reding 2021-01-15 15:44 ` Thierry Reding 2021-01-18 0:02 ` Dmitry Osipenko [this message] 2021-01-18 0:02 ` Dmitry Osipenko 2021-01-19 17:34 ` Thierry Reding 2021-01-19 17:34 ` Thierry Reding 2021-01-15 10:18 ` [PATCH v1 0/5] Clock and reset improvements for Tegra ALSA drivers Takashi Iwai 2021-01-15 10:18 ` Takashi Iwai 2021-01-15 10:52 ` Ben Dooks 2021-01-15 10:52 ` Ben Dooks 2021-01-15 12:59 ` Dmitry Osipenko 2021-01-15 12:59 ` Dmitry Osipenko
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=c2f3d149-0781-3d0a-1d68-f6820386e84d@gmail.com \ --to=digetx@gmail.com \ --cc=alsa-devel@alsa-project.org \ --cc=jonathanh@nvidia.com \ --cc=kwizart@gmail.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-tegra@vger.kernel.org \ --cc=perex@perex.cz \ --cc=pgwipeout@gmail.com \ --cc=spujar@nvidia.com \ --cc=thierry.reding@gmail.com \ --cc=tiwai@suse.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.