Re: [PATCH 2/2] ASoC: xilinx: xlnx_spdif: Use dev_err_probe() and drop redundant error handling

[Michal Simek]

On 7/10/26 07:02, phucduc.bui@xxxxxxxxx wrote:
> From: bui duc phuc <phucduc.bui@xxxxxxxxx>
>
> Use dev_err_probe() for probe error handling where appropriate to
> simplify the code and properly handle deferred probe.
> Also remove redundant error messages when the called helper already
> reports failures, returning the error directly to avoid duplicate
> logging.
>
> Signed-off-by: bui duc phuc <phucduc.bui@xxxxxxxxx>
> ---
>   sound/soc/xilinx/xlnx_spdif.c | 30 ++++++++++--------------------
>   1 file changed, 10 insertions(+), 20 deletions(-)
>
> diff --git a/sound/soc/xilinx/xlnx_spdif.c b/sound/soc/xilinx/xlnx_spdif.c
> index 017a64ab9f1e..8f4d238be890 100644
> --- a/sound/soc/xilinx/xlnx_spdif.c
> +++ b/sound/soc/xilinx/xlnx_spdif.c
> @@ -249,21 +249,17 @@ static int xlnx_spdif_probe(struct platform_device *pdev)
>   		return -ENOMEM;
>   
>   	ctx->axi_clk = devm_clk_get_enabled(dev, "s_axi_aclk");
> -	if (IS_ERR(ctx->axi_clk)) {
> -		ret = PTR_ERR(ctx->axi_clk);
> -		dev_err(dev, "failed to get s_axi_aclk(%d)\n", ret);
> -		return ret;
> -	}
> +	if (IS_ERR(ctx->axi_clk))
> +		return dev_err_probe(dev, PTR_ERR(ctx->axi_clk), "failed to get s_axi_aclk\n");

This is pretty long line. Message should go on the next line.

>   
>   	ctx->base = devm_platform_ioremap_resource(pdev, 0);
>   	if (IS_ERR(ctx->base))
>   		return PTR_ERR(ctx->base);
>   
>   	ret = of_property_read_u32(node, "xlnx,spdif-mode", &ctx->mode);
> -	if (ret < 0) {
> -		dev_err(dev, "cannot get SPDIF mode\n");
> -		return ret;
> -	}
> +	if (ret < 0)
> +		return dev_err_probe(dev, ret, "cannot get SPDIF mode\n");
> +
>   	if (ctx->mode) {
>   		dai_drv = &xlnx_spdif_tx_dai;
>   	} else {
> @@ -274,29 +270,23 @@ static int xlnx_spdif_probe(struct platform_device *pdev)
>   		ret = devm_request_irq(dev, ret,
>   				       xlnx_spdifrx_irq_handler,
>   				       0, "XLNX_SPDIF_RX", ctx);
> -		if (ret) {
> -			dev_err(dev, "spdif rx irq request failed\n");
> -			return -ENODEV;
> -		}
> +		if (ret)
> +			return ret;

Here you are changing error value and commit message is not saying anything 
about it.

>   
>   		init_waitqueue_head(&ctx->chsts_q);
>   		dai_drv = &xlnx_spdif_rx_dai;
>   	}
>   
>   	ret = of_property_read_u32(node, "xlnx,aud_clk_i", &ctx->aclk);
> -	if (ret < 0) {
> -		dev_err(dev, "cannot get aud_clk_i value\n");
> -		return ret;
> -	}
> +	if (ret < 0)
> +		return dev_err_probe(dev, ret, "cannot get aud_clk_i value\n");
>   
>   	dev_set_drvdata(dev, ctx);
>   
>   	ret = devm_snd_soc_register_component(dev, &xlnx_spdif_component,
>   					      dai_drv, 1);
> -	if (ret) {
> -		dev_err(dev, "SPDIF component registration failed\n");
> +	if (ret)
>   		return ret;
> -	}

And this is another case. Where origin code didn't return any error which was 
wrong. That's also not described in commit message and likely this should have 
Fixed tag.

Thanks,
Michal