linux-kernel - Re: [PATCH 4/5] spi: rockchip: Use dev_err

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <6673004.tM3a2QDmDi@phil>
Date: Thu, 26 Sep 2024 11:00:42 +0200
From: Heiko Stuebner <heiko@...ech.de>
To: linux-spi@...r.kernel.org, linux-rockchip@...ts.infradead.org,
 Dragan Simic <dsimic@...jaro.org>
Cc: broonie@...nel.org, oss@...ene.moe, linux-arm-kernel@...ts.infradead.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH 4/5] spi: rockchip: Use dev_err_probe() in the probe path

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@...ene.moe>
> Signed-off-by: Dragan Simic <dsimic@...jaro.org>
> ---
>  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;
>  	}
>  
>