From 5d07ab2a7fa1305e429d9221716582f290b58078 Mon Sep 17 00:00:00 2001 From: Kevin Hao Date: Thu, 10 Apr 2025 14:56:09 +0800 Subject: [PATCH 1/3] spi: fsl-qspi: Fix double cleanup in probe error path Commit 40369bfe717e ("spi: fsl-qspi: use devm function instead of driver remove") introduced managed cleanup via fsl_qspi_cleanup(), but incorrectly retain manual cleanup in two scenarios: - On devm_add_action_or_reset() failure, the function automatically call fsl_qspi_cleanup(). However, the current code still jumps to err_destroy_mutex, repeating cleanup. - After the fsl_qspi_cleanup() action is added successfully, there is no need to manually perform the cleanup in the subsequent error path. However, the current code still jumps to err_destroy_mutex on spi controller failure, repeating cleanup. Skip redundant manual cleanup calls to fix these issues. Cc: stable@vger.kernel.org Fixes: 40369bfe717e ("spi: fsl-qspi: use devm function instead of driver remove") Signed-off-by: Kevin Hao Link: https://patch.msgid.link/20250410-spi-v1-1-56e867cc19cf@gmail.com Signed-off-by: Mark Brown --- drivers/spi/spi-fsl-qspi.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/drivers/spi/spi-fsl-qspi.c b/drivers/spi/spi-fsl-qspi.c index 5c59fddb32c1..2f54dc09d11b 100644 --- a/drivers/spi/spi-fsl-qspi.c +++ b/drivers/spi/spi-fsl-qspi.c @@ -949,17 +949,14 @@ static int fsl_qspi_probe(struct platform_device *pdev) ret = devm_add_action_or_reset(dev, fsl_qspi_cleanup, q); if (ret) - goto err_destroy_mutex; + goto err_put_ctrl; ret = devm_spi_register_controller(dev, ctlr); if (ret) - goto err_destroy_mutex; + goto err_put_ctrl; return 0; -err_destroy_mutex: - mutex_destroy(&q->lock); - err_disable_clk: fsl_qspi_clk_disable_unprep(q); From 82bedbfedd2fc7cd1287732879e515ceb94f8963 Mon Sep 17 00:00:00 2001 From: Kevin Hao Date: Thu, 10 Apr 2025 14:56:10 +0800 Subject: [PATCH 2/3] spi: fsl-spi: Remove redundant probe error message An error message is already emitted by the driver core function call_driver_probe() when the driver probe fails. Therefore, this redundant probe error message is removed. Signed-off-by: Kevin Hao Link: https://patch.msgid.link/20250410-spi-v1-2-56e867cc19cf@gmail.com Signed-off-by: Mark Brown --- drivers/spi/spi-fsl-qspi.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/spi/spi-fsl-qspi.c b/drivers/spi/spi-fsl-qspi.c index 2f54dc09d11b..b5ecffcaf795 100644 --- a/drivers/spi/spi-fsl-qspi.c +++ b/drivers/spi/spi-fsl-qspi.c @@ -963,7 +963,6 @@ static int fsl_qspi_probe(struct platform_device *pdev) err_put_ctrl: spi_controller_put(ctlr); - dev_err(dev, "Freescale QuadSPI probe failed\n"); return ret; } From 3f7b48efb79d91883d98dd7e33dc2a0abfa9f923 Mon Sep 17 00:00:00 2001 From: Kevin Hao Date: Thu, 10 Apr 2025 14:56:11 +0800 Subject: [PATCH 3/3] spi: fsl-qspi: Simplify probe error handling using managed API - Switch to devm_spi_alloc_host() to avoid manual spi_controller_put() calls in error paths. - Factor out the hardware disable logic into a dedicated fsl_qspi_disable() helper and register it as a managed cleanup action, removing the need to explicitly disable hardware after fsl_qspi_default_setup() failures. - Move fsl_qspi_cleanup() earlier in the probe sequence to eliminate the need for manual cleanup in the irq failure path. With these changes we can completely eliminate the messy goto labels in probe function. Signed-off-by: Kevin Hao Link: https://patch.msgid.link/20250410-spi-v1-3-56e867cc19cf@gmail.com Signed-off-by: Mark Brown --- drivers/spi/spi-fsl-qspi.c | 73 ++++++++++++++++---------------------- 1 file changed, 31 insertions(+), 42 deletions(-) diff --git a/drivers/spi/spi-fsl-qspi.c b/drivers/spi/spi-fsl-qspi.c index b5ecffcaf795..f879533535e8 100644 --- a/drivers/spi/spi-fsl-qspi.c +++ b/drivers/spi/spi-fsl-qspi.c @@ -844,13 +844,18 @@ static const struct spi_controller_mem_caps fsl_qspi_mem_caps = { .per_op_freq = true, }; -static void fsl_qspi_cleanup(void *data) +static void fsl_qspi_disable(void *data) { struct fsl_qspi *q = data; /* disable the hardware */ qspi_writel(q, QUADSPI_MCR_MDIS_MASK, q->iobase + QUADSPI_MCR); qspi_writel(q, 0x0, q->iobase + QUADSPI_RSER); +} + +static void fsl_qspi_cleanup(void *data) +{ + struct fsl_qspi *q = data; fsl_qspi_clk_disable_unprep(q); @@ -866,7 +871,7 @@ static int fsl_qspi_probe(struct platform_device *pdev) struct fsl_qspi *q; int ret; - ctlr = spi_alloc_host(&pdev->dev, sizeof(*q)); + ctlr = devm_spi_alloc_host(&pdev->dev, sizeof(*q)); if (!ctlr) return -ENOMEM; @@ -876,68 +881,60 @@ static int fsl_qspi_probe(struct platform_device *pdev) q = spi_controller_get_devdata(ctlr); q->dev = dev; q->devtype_data = of_device_get_match_data(dev); - if (!q->devtype_data) { - ret = -ENODEV; - goto err_put_ctrl; - } + if (!q->devtype_data) + return -ENODEV; platform_set_drvdata(pdev, q); /* find the resources */ q->iobase = devm_platform_ioremap_resource_byname(pdev, "QuadSPI"); - if (IS_ERR(q->iobase)) { - ret = PTR_ERR(q->iobase); - goto err_put_ctrl; - } + if (IS_ERR(q->iobase)) + return PTR_ERR(q->iobase); res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "QuadSPI-memory"); - if (!res) { - ret = -EINVAL; - goto err_put_ctrl; - } + if (!res) + return -EINVAL; q->memmap_phy = res->start; /* Since there are 4 cs, map size required is 4 times ahb_buf_size */ q->ahb_addr = devm_ioremap(dev, q->memmap_phy, (q->devtype_data->ahb_buf_size * 4)); - if (!q->ahb_addr) { - ret = -ENOMEM; - goto err_put_ctrl; - } + if (!q->ahb_addr) + return -ENOMEM; /* find the clocks */ q->clk_en = devm_clk_get(dev, "qspi_en"); - if (IS_ERR(q->clk_en)) { - ret = PTR_ERR(q->clk_en); - goto err_put_ctrl; - } + if (IS_ERR(q->clk_en)) + return PTR_ERR(q->clk_en); q->clk = devm_clk_get(dev, "qspi"); - if (IS_ERR(q->clk)) { - ret = PTR_ERR(q->clk); - goto err_put_ctrl; - } + if (IS_ERR(q->clk)) + return PTR_ERR(q->clk); + + mutex_init(&q->lock); ret = fsl_qspi_clk_prep_enable(q); if (ret) { dev_err(dev, "can not enable the clock\n"); - goto err_put_ctrl; + return ret; } + ret = devm_add_action_or_reset(dev, fsl_qspi_cleanup, q); + if (ret) + return ret; + /* find the irq */ ret = platform_get_irq(pdev, 0); if (ret < 0) - goto err_disable_clk; + return ret; ret = devm_request_irq(dev, ret, fsl_qspi_irq_handler, 0, pdev->name, q); if (ret) { dev_err(dev, "failed to request irq: %d\n", ret); - goto err_disable_clk; + return ret; } - mutex_init(&q->lock); - ctlr->bus_num = -1; ctlr->num_chipselect = 4; ctlr->mem_ops = &fsl_qspi_mem_ops; @@ -947,23 +944,15 @@ static int fsl_qspi_probe(struct platform_device *pdev) ctlr->dev.of_node = np; - ret = devm_add_action_or_reset(dev, fsl_qspi_cleanup, q); + ret = devm_add_action_or_reset(dev, fsl_qspi_disable, q); if (ret) - goto err_put_ctrl; + return ret; ret = devm_spi_register_controller(dev, ctlr); if (ret) - goto err_put_ctrl; + return ret; return 0; - -err_disable_clk: - fsl_qspi_clk_disable_unprep(q); - -err_put_ctrl: - spi_controller_put(ctlr); - - return ret; } static int fsl_qspi_suspend(struct device *dev)