Thanks Andi for this change !
On 12/11/2024 4:40 AM, Andi Shyti wrote:
Avoid repeating the error handling pattern:
geni_se_resources_off(&gi2c->se);
clk_disable_unprepare(gi2c->core_clk);
return;
Introduce a single 'goto' exit label for cleanup in case of
errors. While there are currently two distinct exit points, there
is no overlap in their handling, allowing both branches to
coexist cleanly.
Signed-off-by: Andi Shyti <andi.shyti@xxxxxxxxxx>
---
drivers/i2c/busses/i2c-qcom-geni.c | 30 ++++++++++++++++--------------
1 file changed, 16 insertions(+), 14 deletions(-)
diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c
index 01db24188e29..3fc85595a4aa 100644
--- a/drivers/i2c/busses/i2c-qcom-geni.c
+++ b/drivers/i2c/busses/i2c-qcom-geni.c
@@ -867,14 +867,13 @@ static int geni_i2c_probe(struct platform_device *pdev)
ret = geni_se_resources_on(&gi2c->se);
if (ret) {
- clk_disable_unprepare(gi2c->core_clk);
- return dev_err_probe(dev, ret, "Error turning on resources\n");
+ dev_err_probe(dev, ret, "Error turning on resources\n");
+ goto err_clk;
}
proto = geni_se_read_proto(&gi2c->se);
if (proto != GENI_SE_I2C) {
- geni_se_resources_off(&gi2c->se);
- clk_disable_unprepare(gi2c->core_clk);
- return dev_err_probe(dev, -ENXIO, "Invalid proto %d\n", proto);
+ dev_err_probe(dev, -ENXIO, "Invalid proto %d\n", proto);
Suggestive comment, can we make this second patch as first patch ? So
that we can have both above lines reduced in this patch.
+ goto err_off;
}
if (desc && desc->no_dma_support)
@@ -886,11 +885,8 @@ static int geni_i2c_probe(struct platform_device *pdev)
/* FIFO is disabled, so we can only use GPI DMA */
gi2c->gpi_mode = true;
ret = setup_gpi_dma(gi2c);
- if (ret) {
- geni_se_resources_off(&gi2c->se);
- clk_disable_unprepare(gi2c->core_clk);
- return ret;
- }
+ if (ret)
+ goto err_off;
dev_dbg(dev, "Using GPI DMA mode for I2C\n");
} else {
@@ -902,10 +898,8 @@ static int geni_i2c_probe(struct platform_device *pdev)
tx_depth = desc->tx_fifo_depth;
if (!tx_depth) {
- geni_se_resources_off(&gi2c->se);
- clk_disable_unprepare(gi2c->core_clk);
- return dev_err_probe(dev, -EINVAL,
- "Invalid TX FIFO depth\n");
+ dev_err_probe(dev, -EINVAL, "Invalid TX FIFO depth\n");
+ goto err_off;
}
gi2c->tx_wm = tx_depth - 1;
@@ -944,8 +938,16 @@ static int geni_i2c_probe(struct platform_device *pdev)
return 0;
return ret here ? yes, we need to initialize ret = 0.
+err_off:
can we rename as err_resources ?
+ geni_se_resources_off(&gi2c->se);
+err_clk:
+ clk_disable_unprepare(gi2c->core_clk);
+
+ return ret;
+
err_dma:
release_gpi_dma(gi2c);
+
return ret;
}
