All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dmitry Osipenko <digetx@gmail.com>
To: Sameer Pujar <spujar@nvidia.com>,
	tiwai@suse.com, broonie@kernel.org, lgirdwood@gmail.com,
	robh+dt@kernel.org, thierry.reding@gmail.com, perex@perex.cz
Cc: jonathanh@nvidia.com, mkumard@nvidia.com,
	alsa-devel@alsa-project.org, devicetree@vger.kernel.org,
	linux-tegra@vger.kernel.org, linux-kernel@vger.kernel.org,
	stable@vger.kernel.org
Subject: Re: [PATCH v2 1/3] ALSA: hda/tegra: Fix Tegra194 HDA reset failure
Date: Tue, 21 Dec 2021 18:20:14 +0300	[thread overview]
Message-ID: <46acc080-56f5-f970-a9fa-3a9ece0dd2a3@gmail.com> (raw)
In-Reply-To: <f65ae56d-d289-9e3f-1c15-f0bedda3918c@nvidia.com>

21.12.2021 09:18, Sameer Pujar пишет:
> 
> 
> On 12/21/2021 6:51 AM, Dmitry Osipenko wrote:
>>
>> All stable kernels affected by this problem that don't support the bulk
>> reset API are EOL now. Please use bulk reset API like I suggested in the
>> comment to v1, it will allow us to have a cleaner and nicer code.
> 
> Agree that it would be compact and cleaner, but any specific reset
> failure in the group won't be obvious in the logs. In this case it
> failed silently. If compactness is preferred, then may be I can keep an
> error print at group level so that we see some failure context whenever
> it happens.

The group shouldn't fail ever unless device-tree is wrong. Why do you
think we should care about the case which realistically won't ever
happen? This is a bit unpractical approach.

If we really care about those error messages, then will be much more
reasonable to add them to the reset core, like clk core does it [1],
IMO. This will be a trivial change. Will you be happy with this variant?

[1]
https://elixir.bootlin.com/linux/v5.16-rc6/source/drivers/clk/clk-bulk.c#L100

diff --git a/drivers/reset/core.c b/drivers/reset/core.c
index 61e688882643..85ce0d6eeb34 100644
--- a/drivers/reset/core.c
+++ b/drivers/reset/core.c
@@ -962,6 +962,11 @@ int __reset_control_bulk_get(struct device *dev,
int num_rstcs,
 						    shared, optional, acquired);
 		if (IS_ERR(rstcs[i].rstc)) {
 			ret = PTR_ERR(rstcs[i].rstc);
+
+			if (ret != -EPROBE_DEFER)
+				dev_err(dev, "Failed to get reset '%s': %d\n",
+					rstcs[i].id, ret);
+
 			goto err;
 		}
 	}

WARNING: multiple messages have this Message-ID (diff)
From: Dmitry Osipenko <digetx@gmail.com>
To: Sameer Pujar <spujar@nvidia.com>,
	tiwai@suse.com, broonie@kernel.org, lgirdwood@gmail.com,
	robh+dt@kernel.org, thierry.reding@gmail.com, perex@perex.cz
Cc: devicetree@vger.kernel.org, alsa-devel@alsa-project.org,
	linux-kernel@vger.kernel.org, stable@vger.kernel.org,
	jonathanh@nvidia.com, linux-tegra@vger.kernel.org,
	mkumard@nvidia.com
Subject: Re: [PATCH v2 1/3] ALSA: hda/tegra: Fix Tegra194 HDA reset failure
Date: Tue, 21 Dec 2021 18:20:14 +0300	[thread overview]
Message-ID: <46acc080-56f5-f970-a9fa-3a9ece0dd2a3@gmail.com> (raw)
In-Reply-To: <f65ae56d-d289-9e3f-1c15-f0bedda3918c@nvidia.com>

21.12.2021 09:18, Sameer Pujar пишет:
> 
> 
> On 12/21/2021 6:51 AM, Dmitry Osipenko wrote:
>>
>> All stable kernels affected by this problem that don't support the bulk
>> reset API are EOL now. Please use bulk reset API like I suggested in the
>> comment to v1, it will allow us to have a cleaner and nicer code.
> 
> Agree that it would be compact and cleaner, but any specific reset
> failure in the group won't be obvious in the logs. In this case it
> failed silently. If compactness is preferred, then may be I can keep an
> error print at group level so that we see some failure context whenever
> it happens.

The group shouldn't fail ever unless device-tree is wrong. Why do you
think we should care about the case which realistically won't ever
happen? This is a bit unpractical approach.

If we really care about those error messages, then will be much more
reasonable to add them to the reset core, like clk core does it [1],
IMO. This will be a trivial change. Will you be happy with this variant?

[1]
https://elixir.bootlin.com/linux/v5.16-rc6/source/drivers/clk/clk-bulk.c#L100

diff --git a/drivers/reset/core.c b/drivers/reset/core.c
index 61e688882643..85ce0d6eeb34 100644
--- a/drivers/reset/core.c
+++ b/drivers/reset/core.c
@@ -962,6 +962,11 @@ int __reset_control_bulk_get(struct device *dev,
int num_rstcs,
 						    shared, optional, acquired);
 		if (IS_ERR(rstcs[i].rstc)) {
 			ret = PTR_ERR(rstcs[i].rstc);
+
+			if (ret != -EPROBE_DEFER)
+				dev_err(dev, "Failed to get reset '%s': %d\n",
+					rstcs[i].id, ret);
+
 			goto err;
 		}
 	}

  reply	other threads:[~2021-12-21 15:20 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-20 17:30 [PATCH v2 0/3] Fix Tegra194 HDA regression Sameer Pujar
2021-12-20 17:30 ` Sameer Pujar
2021-12-20 17:30 ` [PATCH v2 1/3] ALSA: hda/tegra: Fix Tegra194 HDA reset failure Sameer Pujar
2021-12-20 17:30   ` Sameer Pujar
2021-12-21  1:21   ` Dmitry Osipenko
2021-12-21  1:21     ` Dmitry Osipenko
2021-12-21  6:18     ` Sameer Pujar
2021-12-21  6:18       ` Sameer Pujar
2021-12-21 15:20       ` Dmitry Osipenko [this message]
2021-12-21 15:20         ` Dmitry Osipenko
2021-12-21 16:03         ` Sameer Pujar
2021-12-21 16:03           ` Sameer Pujar
2021-12-20 17:30 ` [PATCH v2 2/3] dt-bindings: sound: tegra: Update HDA resets Sameer Pujar
2021-12-20 17:30   ` Sameer Pujar
2021-12-20 17:30 ` [PATCH v2 3/3] arm64: tegra: Remove non existent Tegra194 reset Sameer Pujar
2021-12-20 17:30   ` Sameer Pujar

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=46acc080-56f5-f970-a9fa-3a9ece0dd2a3@gmail.com \
    --to=digetx@gmail.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=broonie@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=jonathanh@nvidia.com \
    --cc=lgirdwood@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-tegra@vger.kernel.org \
    --cc=mkumard@nvidia.com \
    --cc=perex@perex.cz \
    --cc=robh+dt@kernel.org \
    --cc=spujar@nvidia.com \
    --cc=stable@vger.kernel.org \
    --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: link
Be 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.