Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
To:     Tony Huang <tonyhuang.sunplus@gmail.com>,
        Li-hao Kuo <lhjeff911@gmail.com>,
        Ulf Hansson <ulf.hansson@linaro.org>,
        Arnd Bergmann <arnd@arndb.de>, linux-mmc@vger.kernel.org,
        linux-kernel@vger.kernel.org
Cc:     dan.carpenter@linaro.org, kernel-janitors@vger.kernel.org,
        error27@gmail.com, harshit.m.mogalapalli@oracle.com
Subject: [PATCH next] mmc: sunplus: Fix error handling in spmmc_drv_probe()
Date:   Mon, 31 Jul 2023 22:23:20 -0700
Message-ID: <20230801052321.1328225-1-harshit.m.mogalapalli@oracle.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

There are few issues in spmmc_drv_probe():

1. When mmc allocation fails, goto is a no-op.
2. When mmc allocation succeeds, the error paths should use goto instead
   of direct return.
3. platform_get_irq() doesn't return zero, so '<' is sufficient.

Fix the above issues by adding goto instead of direct return, also
remove NULL check in 'probe_free_host' as we changed the goto to return
when mmc_alloc_host() fails.

Fixes: 4e268fed8b18 ("mmc: Add mmc driver for Sunplus SP7021")
Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Closes: https://lore.kernel.org/all/a3829ed3-d827-4b9d-827e-9cc24a3ec3bc@moroto.mountain/
Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
---
This is based on static analysis with Smatch. Only compile tested.
---
 drivers/mmc/host/sunplus-mmc.c | 41 ++++++++++++++++++++--------------
 1 file changed, 24 insertions(+), 17 deletions(-)

diff --git a/drivers/mmc/host/sunplus-mmc.c b/drivers/mmc/host/sunplus-mmc.c
index a55a87f64d2a..21cd49be08c0 100644
--- a/drivers/mmc/host/sunplus-mmc.c
+++ b/drivers/mmc/host/sunplus-mmc.c
@@ -864,10 +864,8 @@ static int spmmc_drv_probe(struct platform_device *pdev)
 	int ret = 0;
 
 	mmc = mmc_alloc_host(sizeof(*host), &pdev->dev);
-	if (!mmc) {
-		ret = -ENOMEM;
-		goto probe_free_host;
-	}
+	if (!mmc)
+		return -ENOMEM;
 
 	host = mmc_priv(mmc);
 	host->mmc = mmc;
@@ -875,30 +873,40 @@ static int spmmc_drv_probe(struct platform_device *pdev)
 	host->dma_int_threshold = 1024;
 
 	host->base = devm_platform_get_and_ioremap_resource(pdev, 0, &res);
-	if (IS_ERR(host->base))
-		return PTR_ERR(host->base);
+	if (IS_ERR(host->base)) {
+		ret = PTR_ERR(host->base);
+		goto probe_free_host;
+	}
 
 	host->clk = devm_clk_get(&pdev->dev, NULL);
-	if (IS_ERR(host->clk))
-		return dev_err_probe(&pdev->dev, PTR_ERR(host->clk), "clk get fail\n");
+	if (IS_ERR(host->clk)) {
+		ret = dev_err_probe(&pdev->dev, PTR_ERR(host->clk), "clk get fail\n");
+		goto probe_free_host;
+	}
 
 	host->rstc = devm_reset_control_get_exclusive(&pdev->dev, NULL);
-	if (IS_ERR(host->rstc))
-		return dev_err_probe(&pdev->dev, PTR_ERR(host->rstc), "rst get fail\n");
+	if (IS_ERR(host->rstc)) {
+		ret = dev_err_probe(&pdev->dev, PTR_ERR(host->rstc), "rst get fail\n");
+		goto probe_free_host;
+	}
 
 	host->irq = platform_get_irq(pdev, 0);
-	if (host->irq <= 0)
-		return host->irq;
+	if (host->irq < 0) {
+		ret = host->irq;
+		goto probe_free_host;
+	}
 
 	ret = devm_request_threaded_irq(&pdev->dev, host->irq,
 					spmmc_irq, spmmc_func_finish_req, IRQF_SHARED,
 			NULL, host);
 	if (ret)
-		return ret;
+		goto probe_free_host;
 
 	ret = clk_prepare_enable(host->clk);
-	if (ret)
-		return dev_err_probe(&pdev->dev, ret, "failed to enable clk\n");
+	if (ret) {
+		ret = dev_err_probe(&pdev->dev, ret, "failed to enable clk\n");
+		goto probe_free_host;
+	}
 
 	ret = mmc_of_parse(mmc);
 	if (ret)
@@ -940,8 +948,7 @@ static int spmmc_drv_probe(struct platform_device *pdev)
 	clk_disable_unprepare(host->clk);
 
 probe_free_host:
-	if (mmc)
-		mmc_free_host(mmc);
+	mmc_free_host(mmc);
 
 	return ret;
 }
-- 
2.39.3