diff mbox series

[v3,5/5] spi: rockchip: Use dev_{err,warn}_probe() in the probe path

Message ID 5b6bd142dab3ab93d7039db3e2fdcfea6bee2217.1727601608.git.dsimic@manjaro.org
State Accepted
Commit e2fc05873905f2ee96b38a116ae86f45fe7d8e49
Headers show
Series Add dev_warn_probe() and improve error handling in Rockchip SPI drivers | expand

Commit Message

Dragan Simic Sept. 29, 2024, 9:21 a.m. UTC 
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.  Use the new
function dev_warn_probe() to improve error handling for the TX and RX DMA
channel requests, which are actually optional, and tweak the logged warnings
a bit to additionally describe their optional nature.

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@helene.moe>
Signed-off-by: Dragan Simic <dsimic@manjaro.org>
---
 drivers/spi/spi-rockchip.c | 28 +++++++++++++---------------
 1 file changed, 13 insertions(+), 15 deletions(-)

Comments

Hélène Vulquin Oct. 1, 2024, 9:06 a.m. UTC | #1 
On Sun Sep 29, 2024 at 11:21 AM CEST, Dragan Simic wrote:
> 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.  Use the new
> function dev_warn_probe() to improve error handling for the TX and RX DMA
> channel requests, which are actually optional, and tweak the logged warnings
> a bit to additionally describe their optional nature.
>
> 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@helene.moe>
> Signed-off-by: Dragan Simic <dsimic@manjaro.org>

I can now see appropriate messages for pending probes on Rockchip SPI
devices, instead of "(reason unknown)", which is incredibly helpful
for debugging DeviceTrees and kernel configs.

Tested on 6.11 and looks like it works as intended.

Thank you for this patch!

Tested-by: Hélène Vulquin <oss@helene.moe>
diff mbox series

Patch

diff --git a/drivers/spi/spi-rockchip.c b/drivers/spi/spi-rockchip.c
index 81f2a966c124..055cd6066466 100644
--- a/drivers/spi/spi-rockchip.c
+++ b/drivers/spi/spi-rockchip.c
@@ -773,15 +773,15 @@  static int rockchip_spi_probe(struct platform_device *pdev)
 
 	rs->apb_pclk = devm_clk_get_enabled(&pdev->dev, "apb_pclk");
 	if (IS_ERR(rs->apb_pclk)) {
-		dev_err(&pdev->dev, "Failed to get apb_pclk\n");
-		ret = PTR_ERR(rs->apb_pclk);
+		ret = dev_err_probe(&pdev->dev, PTR_ERR(rs->apb_pclk),
+				    "Failed to get apb_pclk\n");
 		goto err_put_ctlr;
 	}
 
 	rs->spiclk = devm_clk_get_enabled(&pdev->dev, "spiclk");
 	if (IS_ERR(rs->spiclk)) {
-		dev_err(&pdev->dev, "Failed to get spi_pclk\n");
-		ret = PTR_ERR(rs->spiclk);
+		ret = dev_err_probe(&pdev->dev, PTR_ERR(rs->spiclk),
+				    "Failed to get spi_pclk\n");
 		goto err_put_ctlr;
 	}
 
@@ -817,8 +817,7 @@  static int rockchip_spi_probe(struct platform_device *pdev)
 
 	rs->fifo_len = get_fifo_len(rs);
 	if (!rs->fifo_len) {
-		dev_err(&pdev->dev, "Failed to get fifo length\n");
-		ret = -EINVAL;
+		ret = dev_err_probe(&pdev->dev, -EINVAL, "Failed to get fifo length\n");
 		goto err_put_ctlr;
 	}
 
@@ -858,22 +857,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_warn_probe(rs->dev, PTR_ERR(ctlr->dma_tx),
+				     "Failed to request optional TX DMA channel\n");
+		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_warn_probe(rs->dev, PTR_ERR(ctlr->dma_rx),
+				     "Failed to request optional 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;
 	}