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

[Bui Duc Phuc]

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