diff mbox

ASoC: ux500: do not moan on deferrals

Message ID 20170120132011.1147-1-linus.walleij@linaro.org (mailing list archive)
State New, archived
Headers show

Commit Message

Linus Walleij Jan. 20, 2017, 1:20 p.m. UTC
This removes the "error" print on probe deferral, it's
unnecessary and confusing. Also cut a few debug prints we do
not need.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 sound/soc/ux500/mop500.c | 16 +---------------
 1 file changed, 1 insertion(+), 15 deletions(-)

Comments

Mark Brown Jan. 23, 2017, 6:15 p.m. UTC | #1
On Fri, Jan 20, 2017 at 02:20:11PM +0100, Linus Walleij wrote:
> This removes the "error" print on probe deferral, it's
> unnecessary and confusing. Also cut a few debug prints we do
> not need.

I know I took the other patch doing this (along with some other changes)
but in general I'm not super comfortable with just silently ignoring
probe deferral errors as it makes it hard to debug practical systems -
the device is there, the driver is there but it fails to instantiate
with no indication as to why.  The probe ordering stuff that Raphael was
working on looks like the best way of dealing with the noise that we
currently get from not even trying to order things, though there were
some other ideas like Russell's one about suppressing deferred probe
error messages until after the initial init is complete.
Linus Walleij Jan. 24, 2017, 12:49 p.m. UTC | #2
On Mon, Jan 23, 2017 at 7:15 PM, Mark Brown <broonie@kernel.org> wrote:
> On Fri, Jan 20, 2017 at 02:20:11PM +0100, Linus Walleij wrote:

>> This removes the "error" print on probe deferral, it's
>> unnecessary and confusing. Also cut a few debug prints we do
>> not need.
>
> I know I took the other patch doing this (along with some other changes)
> but in general I'm not super comfortable with just silently ignoring
> probe deferral errors as it makes it hard to debug practical systems -
> the device is there, the driver is there but it fails to instantiate
> with no indication as to why.

You're right. Let's drop this, the only improvement could I guess
be to print explicitly that things get deferred instead, unless by now
every kernel developer in the world knows what -517 means.

Yours,
Linus Walleij
Mark Brown Jan. 24, 2017, 3:45 p.m. UTC | #3
On Tue, Jan 24, 2017 at 01:49:15PM +0100, Linus Walleij wrote:
> On Mon, Jan 23, 2017 at 7:15 PM, Mark Brown <broonie@kernel.org> wrote:

> > I know I took the other patch doing this (along with some other changes)
> > but in general I'm not super comfortable with just silently ignoring
> > probe deferral errors as it makes it hard to debug practical systems -
> > the device is there, the driver is there but it fails to instantiate
> > with no indication as to why.

> You're right. Let's drop this, the only improvement could I guess
> be to print explicitly that things get deferred instead, unless by now
> every kernel developer in the world knows what -517 means.

I'm guessing so but yeah.  Actually one thing I'd really like to do at
some point is introduce an error to string conversion function and use
that more widely for printing error messages, it's trivial and makes
reading log messages *so* much more easy in general if the computer does
the decoding for you if you get -EPROBE_DEFER instead of -517.
diff mbox

Patch

diff --git a/sound/soc/ux500/mop500.c b/sound/soc/ux500/mop500.c
index ba9fc099cf67..c5a9e98ddd7f 100644
--- a/sound/soc/ux500/mop500.c
+++ b/sound/soc/ux500/mop500.c
@@ -103,8 +103,6 @@  static int mop500_probe(struct platform_device *pdev)
 	struct device_node *np = pdev->dev.of_node;
 	int ret;
 
-	dev_dbg(&pdev->dev, "%s: Enter.\n", __func__);
-
 	mop500_card.dev = &pdev->dev;
 
 	if (np) {
@@ -113,22 +111,12 @@  static int mop500_probe(struct platform_device *pdev)
 			return ret;
 	}
 
-	dev_dbg(&pdev->dev, "%s: Card %s: Set platform drvdata.\n",
-		__func__, mop500_card.name);
 	platform_set_drvdata(pdev, &mop500_card);
 
 	snd_soc_card_set_drvdata(&mop500_card, NULL);
 
-	dev_dbg(&pdev->dev, "%s: Card %s: num_links = %d\n",
-		__func__, mop500_card.name, mop500_card.num_links);
-	dev_dbg(&pdev->dev, "%s: Card %s: DAI-link 0: name = %s\n",
-		__func__, mop500_card.name, mop500_card.dai_link[0].name);
-	dev_dbg(&pdev->dev, "%s: Card %s: DAI-link 0: stream_name = %s\n",
-		__func__, mop500_card.name,
-		mop500_card.dai_link[0].stream_name);
-
 	ret = snd_soc_register_card(&mop500_card);
-	if (ret)
+	if (ret && ret != -EPROBE_DEFER)
 		dev_err(&pdev->dev,
 			"Error: snd_soc_register_card failed (%d)!\n", ret);
 
@@ -139,8 +127,6 @@  static int mop500_remove(struct platform_device *pdev)
 {
 	struct snd_soc_card *mop500_card = platform_get_drvdata(pdev);
 
-	pr_debug("%s: Enter.\n", __func__);
-
 	snd_soc_unregister_card(mop500_card);
 	mop500_ab8500_remove(mop500_card);
 	mop500_of_node_put();