Re: [PATCH 2/6] ASoC: sunxi: sun4i-codec: Drop redundant error messages

Bui Duc Phuc <[email protected]>
Newsgroups dev.linux.lists.linux-sunxi,org.infradead.lists.linux-arm-kernel,org.kernel.vger.linux-kernel,org.kernel.vger.linux-sound
Message-ID <CAABR9nEk19N-hBm+QKv6U=7SBgkQwC2Ohb_NDz8OLEetavLLAQ@mail.gmail.com>
Hi Chen-Yu Tsai,

Thank you for your review.


> >         ret = devm_snd_dmaengine_pcm_register(&pdev->dev, NULL, 0);
>
> snd_dmaengine_pcm_register() doesn't seem to print an error when
> dma_request_chan() fails.
>
> > -       if (ret) {
> > -               dev_err(&pdev->dev, "Failed to register against DMAEngine\n");
> > +       if (ret)
> >                 return ret;
> > -       }
> >

You are right that dma_request_chan() itself has several code paths where
it doesn't print any error logs when it fails.
However, if we look at how the caller handles the return value of
dma_request_chan():

---------------------------------
chan = dma_request_chan(dev, name);
if (IS_ERR(chan)) {
                /*
                * Only report probe deferral errors, channels
                * might not be present for devices that
                * support only TX or only RX.
                */
                if (PTR_ERR(chan) == -EPROBE_DEFER)
                        return -EPROBE_DEFER;
                pcm->chan[i] = NULL;
} else {
                pcm->chan[i] = chan;
}
---------------------------------------

As shown here, the code only propagates a single error code, -EPROBE_DEFER
regardless of whatever other errors might have occurred. The reasoning behind
this is already well-documented in the comment block above.

Therefore, keeping the following error log is not particularly useful:

----------------------
ret = devm_snd_dmaengine_pcm_register(&pdev->dev, NULL, 0);
if (ret) {
        dev_err(&pdev->dev, "Failed to register against DMAEngine\n");
        return ret;
}
-----------------------

This logging statement does not make much sense here because if the returned
error is -EPROBE_DEFER , we should definitely avoid printing error messages to
the kernel log.

Let me know if this makes sense to you, or if you'd still prefer to keep
the log with a check to avoid printing on -EPROBE_DEFER.

Best regards,
Phuc
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.