Re: [PATCH 4/5] spi: rockchip: Use dev_err_probe() in the probe path

[Heiko Stuebner <heiko@xxxxxxxxx>]

Am Donnerstag, 26. September 2024, 10:38:15 CEST schrieb Dragan Simic:
> Use function dev_err_probe() in the probe path instead of dev_err() where
> appropriate, to make the code a bit more uniform and compact, and to improve
> error handling for the TX and RX DMA channel requests.
> 
> Previously, deferred requests for the TX and RX DMA channels produced no
> debug messages, and the final error messages didn't include the error codes,
> which are all highly useful when debugging permanently failed DMA channel
> requests, such as when the required drivers aren't enabled.
> 
> Suggested-by: Hélene Vulquin <oss@xxxxxxxxxx>
> Signed-off-by: Dragan Simic <dsimic@xxxxxxxxxxx>
> ---
>  drivers/spi/spi-rockchip.c | 25 ++++++++++++-------------
>  1 file changed, 12 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/spi/spi-rockchip.c b/drivers/spi/spi-rockchip.c
> index 28879fed03f8..6b5c67a357bb 100644
> --- a/drivers/spi/spi-rockchip.c
> +++ b/drivers/spi/spi-rockchip.c
> @@ -853,22 +853,21 @@ static int rockchip_spi_probe(struct platform_device *pdev)
>  
>  	ctlr->dma_tx = dma_request_chan(rs->dev, "tx");
>  	if (IS_ERR(ctlr->dma_tx)) {
> -		/* Check tx to see if we need defer probing driver */
> -		if (PTR_ERR(ctlr->dma_tx) == -EPROBE_DEFER) {
> -			ret = -EPROBE_DEFER;
> +		/* Check tx to see if we need to defer driver probing */
> +		ret = dev_err_probe(rs->dev, PTR_ERR(ctlr->dma_tx),
> +				    "Failed to request TX DMA channel\n");

you're upgrading here from a warning to an error log level.
As it seems the controller may actually provide some level of functionality
even without dma, is this approriate?

Same for rx below.

Heiko

> +		if (ret == -EPROBE_DEFER)
>  			goto err_disable_pm_runtime;
> -		}
> -		dev_warn(rs->dev, "Failed to request TX DMA channel\n");
>  		ctlr->dma_tx = NULL;
>  	}
>  
>  	ctlr->dma_rx = dma_request_chan(rs->dev, "rx");
>  	if (IS_ERR(ctlr->dma_rx)) {
> -		if (PTR_ERR(ctlr->dma_rx) == -EPROBE_DEFER) {
> -			ret = -EPROBE_DEFER;
> +		/* Check rx to see if we need to defer driver probing */
> +		ret = dev_err_probe(rs->dev, PTR_ERR(ctlr->dma_rx),
> +				    "Failed to request RX DMA channel\n");
> +		if (ret == -EPROBE_DEFER)
>  			goto err_free_dma_tx;
> -		}
> -		dev_warn(rs->dev, "Failed to request RX DMA channel\n");
>  		ctlr->dma_rx = NULL;
>  	}
>  
>