All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH for-5.8] ASoC: amd: doesn't print error log if the return value is EPROBE_DEFER
@ 2020-05-21 14:44 Hui Wang
  2020-05-21 15:43 ` Mark Brown
  0 siblings, 1 reply; 4+ messages in thread
From: Hui Wang @ 2020-05-21 14:44 UTC (permalink / raw)
  To: alsa-devel, broonie

The machine driver module and codec driver module don't have
dependency, it is possible that the machine driver is loaded ahead of
the codec driver, then the register_card() will fail and return
EPROBE_DEFER, in this case the driver should not print error log since
this is not a real failure.

For example, I saw this log from a machine with amd renoir audio:
acp_pdm_mach acp_pdm_mach.0: snd_soc_register_card(acp) failed: -517

After applying this patch, there is no error log to confuse users and
audio works after the codec driver is loaded.

Signed-off-by: Hui Wang <hui.wang@canonical.com>
---
 sound/soc/amd/acp-da7219-max98357a.c | 3 ++-
 sound/soc/amd/acp-rt5645.c           | 3 ++-
 sound/soc/amd/acp3x-rt5682-max9836.c | 3 ++-
 sound/soc/amd/renoir/acp3x-rn.c      | 7 ++++---
 4 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/sound/soc/amd/acp-da7219-max98357a.c b/sound/soc/amd/acp-da7219-max98357a.c
index 9414d7269c4f..a7e755a563e8 100644
--- a/sound/soc/amd/acp-da7219-max98357a.c
+++ b/sound/soc/amd/acp-da7219-max98357a.c
@@ -440,7 +440,8 @@ static int cz_probe(struct platform_device *pdev)
 	snd_soc_card_set_drvdata(card, machine);
 	ret = devm_snd_soc_register_card(&pdev->dev, &cz_card);
 	if (ret) {
-		dev_err(&pdev->dev,
+		if (ret != -EPROBE_DEFER)
+			dev_err(&pdev->dev,
 				"devm_snd_soc_register_card(%s) failed: %d\n",
 				cz_card.name, ret);
 		return ret;
diff --git a/sound/soc/amd/acp-rt5645.c b/sound/soc/amd/acp-rt5645.c
index 73b31f88a6b5..cffd24eae3a3 100644
--- a/sound/soc/amd/acp-rt5645.c
+++ b/sound/soc/amd/acp-rt5645.c
@@ -174,7 +174,8 @@ static int cz_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, card);
 	ret = devm_snd_soc_register_card(&pdev->dev, &cz_card);
 	if (ret) {
-		dev_err(&pdev->dev,
+		if (ret != -EPROBE_DEFER)
+			dev_err(&pdev->dev,
 				"devm_snd_soc_register_card(%s) failed: %d\n",
 				cz_card.name, ret);
 		return ret;
diff --git a/sound/soc/amd/acp3x-rt5682-max9836.c b/sound/soc/amd/acp3x-rt5682-max9836.c
index e499c00e0c66..16bcaad9ead2 100644
--- a/sound/soc/amd/acp3x-rt5682-max9836.c
+++ b/sound/soc/amd/acp3x-rt5682-max9836.c
@@ -346,7 +346,8 @@ static int acp3x_probe(struct platform_device *pdev)
 
 	ret = devm_snd_soc_register_card(&pdev->dev, &acp3x_card);
 	if (ret) {
-		dev_err(&pdev->dev,
+		if (ret != -EPROBE_DEFER)
+			dev_err(&pdev->dev,
 				"devm_snd_soc_register_card(%s) failed: %d\n",
 				acp3x_card.name, ret);
 		return ret;
diff --git a/sound/soc/amd/renoir/acp3x-rn.c b/sound/soc/amd/renoir/acp3x-rn.c
index 306134b89a82..95b616fcad30 100644
--- a/sound/soc/amd/renoir/acp3x-rn.c
+++ b/sound/soc/amd/renoir/acp3x-rn.c
@@ -54,9 +54,10 @@ static int acp_probe(struct platform_device *pdev)
 	snd_soc_card_set_drvdata(card, machine);
 	ret = devm_snd_soc_register_card(&pdev->dev, card);
 	if (ret) {
-		dev_err(&pdev->dev,
-			"snd_soc_register_card(%s) failed: %d\n",
-			acp_card.name, ret);
+		if (ret != -EPROBE_DEFER)
+			dev_err(&pdev->dev,
+				"snd_soc_register_card(%s) failed: %d\n",
+				acp_card.name, ret);
 		return ret;
 	}
 	return 0;
-- 
2.17.1


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

* Re: [PATCH for-5.8] ASoC: amd: doesn't print error log if the return value is EPROBE_DEFER
  2020-05-21 14:44 [PATCH for-5.8] ASoC: amd: doesn't print error log if the return value is EPROBE_DEFER Hui Wang
@ 2020-05-21 15:43 ` Mark Brown
  2020-05-22  7:47   ` Hui Wang
  0 siblings, 1 reply; 4+ messages in thread
From: Mark Brown @ 2020-05-21 15:43 UTC (permalink / raw)
  To: Hui Wang; +Cc: alsa-devel

[-- Attachment #1: Type: text/plain, Size: 575 bytes --]

On Thu, May 21, 2020 at 10:44:34PM +0800, Hui Wang wrote:
> The machine driver module and codec driver module don't have
> dependency, it is possible that the machine driver is loaded ahead of
> the codec driver, then the register_card() will fail and return
> EPROBE_DEFER, in this case the driver should not print error log since
> this is not a real failure.

This isn't helpful to people who are trying to figure out why the driver
isn't loading - if we silently fail then the user will struggle to
determine what the problem that causes their driver to fail to bind is.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

* Re: [PATCH for-5.8] ASoC: amd: doesn't print error log if the return value is EPROBE_DEFER
  2020-05-21 15:43 ` Mark Brown
@ 2020-05-22  7:47   ` Hui Wang
  2020-05-22 11:00     ` Mark Brown
  0 siblings, 1 reply; 4+ messages in thread
From: Hui Wang @ 2020-05-22  7:47 UTC (permalink / raw)
  To: Mark Brown; +Cc: alsa-devel


On 2020/5/21 下午11:43, Mark Brown wrote:
> On Thu, May 21, 2020 at 10:44:34PM +0800, Hui Wang wrote:
>> The machine driver module and codec driver module don't have
>> dependency, it is possible that the machine driver is loaded ahead of
>> the codec driver, then the register_card() will fail and return
>> EPROBE_DEFER, in this case the driver should not print error log since
>> this is not a real failure.
> This isn't helpful to people who are trying to figure out why the driver
> isn't loading - if we silently fail then the user will struggle to
> determine what the problem that causes their driver to fail to bind is.

Yes, you are right. If the codec module is not loaded, the machine 
driver will fail silently.

There are many modules in the kernel, no other modules print the -517 
error or warning, so if this driver prints it, it really confuses users 
(according to my test, the audio works but the kernel prints this error 
with 100% chance, and within ubuntu, the error message is read color, it 
is very easily caught by users).

How about we put off the registering the machine device, this can 
guarantee everything is ready when machine driver's probe is called. I 
will send a V2 according to this idea.

Thanks,

Hui.


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

* Re: [PATCH for-5.8] ASoC: amd: doesn't print error log if the return value is EPROBE_DEFER
  2020-05-22  7:47   ` Hui Wang
@ 2020-05-22 11:00     ` Mark Brown
  0 siblings, 0 replies; 4+ messages in thread
From: Mark Brown @ 2020-05-22 11:00 UTC (permalink / raw)
  To: Hui Wang; +Cc: alsa-devel

[-- Attachment #1: Type: text/plain, Size: 671 bytes --]

On Fri, May 22, 2020 at 03:47:22PM +0800, Hui Wang wrote:

> There are many modules in the kernel, no other modules print the -517 error

This is quite simply not true.

> or warning, so if this driver prints it, it really confuses users (according
> to my test, the audio works but the kernel prints this error with 100%
> chance, and within ubuntu, the error message is read color, it is very
> easily caught by users).

> How about we put off the registering the machine device, this can guarantee
> everything is ready when machine driver's probe is called. I will send a V2
> according to this idea.

You could also lower the severity of the message when deferring.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

end of thread, other threads:[~2020-05-22 11:01 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-21 14:44 [PATCH for-5.8] ASoC: amd: doesn't print error log if the return value is EPROBE_DEFER Hui Wang
2020-05-21 15:43 ` Mark Brown
2020-05-22  7:47   ` Hui Wang
2020-05-22 11:00     ` Mark Brown

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.