* [PATCH v2 0/2] Qcom Geni exit path cleanups
@ 2024-12-12 13:54 Andi Shyti
2024-12-12 13:54 ` [PATCH v2 1/2] i2c: qcom-geni: Use dev_err_probe in the probe function Andi Shyti
2024-12-12 13:54 ` [PATCH v2 2/2] i2c: qcom-geni: Simplify error handling in " Andi Shyti
0 siblings, 2 replies; 7+ messages in thread
From: Andi Shyti @ 2024-12-12 13:54 UTC (permalink / raw)
To: linux-arm-msm, linux-i2c; +Cc: Mukesh Kumar Savaliya, Andi Shyti
Hi,
I am submitting two trivial cleanups in this series. The first
replaces all instances of dev_err with dev_err_probe throughout
the probe function for consistency. The second improves the error
exit path by introducing a single 'goto' label for better
maintainability.
Thank you Mukesh for your reviews.
Thank you,
Andi
Changelog:
==========
v1 -> v2:
- Updated the final return statement to return 'ret' instead of
'0' for consistency. Since 'ret' already holds the value '0',
this change is purely aesthetic.
- Renamed the exit label from 'err_off' to 'err_resources' for
improved clarity and alignment with its purpose.
Andi Shyti (2):
i2c: qcom-geni: Use dev_err_probe in the probe function
i2c: qcom-geni: Simplify error handling in probe function
drivers/i2c/busses/i2c-qcom-geni.c | 55 ++++++++++++++----------------
1 file changed, 26 insertions(+), 29 deletions(-)
--
2.45.2
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 1/2] i2c: qcom-geni: Use dev_err_probe in the probe function
2024-12-12 13:54 [PATCH v2 0/2] Qcom Geni exit path cleanups Andi Shyti
@ 2024-12-12 13:54 ` Andi Shyti
2024-12-12 18:34 ` Vladimir Zapolskiy
2024-12-12 19:21 ` Mukesh Kumar Savaliya
2024-12-12 13:54 ` [PATCH v2 2/2] i2c: qcom-geni: Simplify error handling in " Andi Shyti
1 sibling, 2 replies; 7+ messages in thread
From: Andi Shyti @ 2024-12-12 13:54 UTC (permalink / raw)
To: linux-arm-msm, linux-i2c; +Cc: Mukesh Kumar Savaliya, Andi Shyti
Replace classical dev_err with dev_err_probe in the probe
function for better error reporting. Also, use dev_err_probe in
cases where the error number is clear (e.g., -EIO or -EINVAL) to
maintain consistency.
Additionally, remove redundant logging to simplify the code.
Signed-off-by: Andi Shyti <andi.shyti@kernel.org>
---
drivers/i2c/busses/i2c-qcom-geni.c | 33 +++++++++++++-----------------
1 file changed, 14 insertions(+), 19 deletions(-)
diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c
index 7a22e1f46e60..01db24188e29 100644
--- a/drivers/i2c/busses/i2c-qcom-geni.c
+++ b/drivers/i2c/busses/i2c-qcom-geni.c
@@ -823,11 +823,9 @@ static int geni_i2c_probe(struct platform_device *pdev)
return gi2c->irq;
ret = geni_i2c_clk_map_idx(gi2c);
- if (ret) {
- dev_err(dev, "Invalid clk frequency %d Hz: %d\n",
- gi2c->clk_freq_out, ret);
- return ret;
- }
+ if (ret)
+ return dev_err_probe(dev, ret, "Invalid clk frequency %d Hz\n",
+ gi2c->clk_freq_out);
gi2c->adap.algo = &geni_i2c_algo;
init_completion(&gi2c->done);
@@ -837,11 +835,10 @@ static int geni_i2c_probe(struct platform_device *pdev)
/* Keep interrupts disabled initially to allow for low-power modes */
ret = devm_request_irq(dev, gi2c->irq, geni_i2c_irq, IRQF_NO_AUTOEN,
dev_name(dev), gi2c);
- if (ret) {
- dev_err(dev, "Request_irq failed:%d: err:%d\n",
- gi2c->irq, ret);
- return ret;
- }
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "Request_irq failed: %d\n", gi2c->irq);
+
i2c_set_adapdata(&gi2c->adap, gi2c);
gi2c->adap.dev.parent = dev;
gi2c->adap.dev.of_node = dev->of_node;
@@ -870,16 +867,14 @@ static int geni_i2c_probe(struct platform_device *pdev)
ret = geni_se_resources_on(&gi2c->se);
if (ret) {
- dev_err(dev, "Error turning on resources %d\n", ret);
clk_disable_unprepare(gi2c->core_clk);
- return ret;
+ return dev_err_probe(dev, ret, "Error turning on resources\n");
}
proto = geni_se_read_proto(&gi2c->se);
if (proto != GENI_SE_I2C) {
- dev_err(dev, "Invalid proto %d\n", proto);
geni_se_resources_off(&gi2c->se);
clk_disable_unprepare(gi2c->core_clk);
- return -ENXIO;
+ return dev_err_probe(dev, -ENXIO, "Invalid proto %d\n", proto);
}
if (desc && desc->no_dma_support)
@@ -894,7 +889,7 @@ static int geni_i2c_probe(struct platform_device *pdev)
if (ret) {
geni_se_resources_off(&gi2c->se);
clk_disable_unprepare(gi2c->core_clk);
- return dev_err_probe(dev, ret, "Failed to setup GPI DMA mode\n");
+ return ret;
}
dev_dbg(dev, "Using GPI DMA mode for I2C\n");
@@ -907,10 +902,10 @@ static int geni_i2c_probe(struct platform_device *pdev)
tx_depth = desc->tx_fifo_depth;
if (!tx_depth) {
- dev_err(dev, "Invalid TX FIFO depth\n");
geni_se_resources_off(&gi2c->se);
clk_disable_unprepare(gi2c->core_clk);
- return -EINVAL;
+ return dev_err_probe(dev, -EINVAL,
+ "Invalid TX FIFO depth\n");
}
gi2c->tx_wm = tx_depth - 1;
@@ -924,7 +919,7 @@ static int geni_i2c_probe(struct platform_device *pdev)
clk_disable_unprepare(gi2c->core_clk);
ret = geni_se_resources_off(&gi2c->se);
if (ret) {
- dev_err(dev, "Error turning off resources %d\n", ret);
+ dev_err_probe(dev, ret, "Error turning off resources\n");
goto err_dma;
}
@@ -940,7 +935,7 @@ static int geni_i2c_probe(struct platform_device *pdev)
ret = i2c_add_adapter(&gi2c->adap);
if (ret) {
- dev_err(dev, "Error adding i2c adapter %d\n", ret);
+ dev_err_probe(dev, ret, "Error adding i2c adapter\n");
pm_runtime_disable(gi2c->se.dev);
goto err_dma;
}
--
2.45.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v2 2/2] i2c: qcom-geni: Simplify error handling in probe function
2024-12-12 13:54 [PATCH v2 0/2] Qcom Geni exit path cleanups Andi Shyti
2024-12-12 13:54 ` [PATCH v2 1/2] i2c: qcom-geni: Use dev_err_probe in the probe function Andi Shyti
@ 2024-12-12 13:54 ` Andi Shyti
2024-12-12 18:38 ` Vladimir Zapolskiy
1 sibling, 1 reply; 7+ messages in thread
From: Andi Shyti @ 2024-12-12 13:54 UTC (permalink / raw)
To: linux-arm-msm, linux-i2c; +Cc: Mukesh Kumar Savaliya, Andi Shyti
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@kernel.org>
---
drivers/i2c/busses/i2c-qcom-geni.c | 32 ++++++++++++++++--------------
1 file changed, 17 insertions(+), 15 deletions(-)
diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c
index 01db24188e29..88709b97f86d 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);
+ goto err_resources;
}
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_resources;
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_resources;
}
gi2c->tx_wm = tx_depth - 1;
@@ -942,10 +936,18 @@ static int geni_i2c_probe(struct platform_device *pdev)
dev_dbg(dev, "Geni-I2C adaptor successfully added\n");
- return 0;
+ return ret;
+
+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;
}
--
2.45.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/2] i2c: qcom-geni: Use dev_err_probe in the probe function
2024-12-12 13:54 ` [PATCH v2 1/2] i2c: qcom-geni: Use dev_err_probe in the probe function Andi Shyti
@ 2024-12-12 18:34 ` Vladimir Zapolskiy
2024-12-12 19:21 ` Mukesh Kumar Savaliya
1 sibling, 0 replies; 7+ messages in thread
From: Vladimir Zapolskiy @ 2024-12-12 18:34 UTC (permalink / raw)
To: Andi Shyti, linux-arm-msm, linux-i2c; +Cc: Mukesh Kumar Savaliya
On 12/12/24 15:54, Andi Shyti wrote:
> Replace classical dev_err with dev_err_probe in the probe
> function for better error reporting. Also, use dev_err_probe in
> cases where the error number is clear (e.g., -EIO or -EINVAL) to
> maintain consistency.
>
> Additionally, remove redundant logging to simplify the code.
>
> Signed-off-by: Andi Shyti <andi.shyti@kernel.org>
> ---
> drivers/i2c/busses/i2c-qcom-geni.c | 33 +++++++++++++-----------------
> 1 file changed, 14 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c
> index 7a22e1f46e60..01db24188e29 100644
> --- a/drivers/i2c/busses/i2c-qcom-geni.c
> +++ b/drivers/i2c/busses/i2c-qcom-geni.c
> @@ -823,11 +823,9 @@ static int geni_i2c_probe(struct platform_device *pdev)
> return gi2c->irq;
>
> ret = geni_i2c_clk_map_idx(gi2c);
> - if (ret) {
> - dev_err(dev, "Invalid clk frequency %d Hz: %d\n",
> - gi2c->clk_freq_out, ret);
> - return ret;
> - }
> + if (ret)
> + return dev_err_probe(dev, ret, "Invalid clk frequency %d Hz\n",
> + gi2c->clk_freq_out);
>
> gi2c->adap.algo = &geni_i2c_algo;
> init_completion(&gi2c->done);
> @@ -837,11 +835,10 @@ static int geni_i2c_probe(struct platform_device *pdev)
> /* Keep interrupts disabled initially to allow for low-power modes */
> ret = devm_request_irq(dev, gi2c->irq, geni_i2c_irq, IRQF_NO_AUTOEN,
> dev_name(dev), gi2c);
> - if (ret) {
> - dev_err(dev, "Request_irq failed:%d: err:%d\n",
> - gi2c->irq, ret);
> - return ret;
> - }
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "Request_irq failed: %d\n", gi2c->irq);
> +
> i2c_set_adapdata(&gi2c->adap, gi2c);
> gi2c->adap.dev.parent = dev;
> gi2c->adap.dev.of_node = dev->of_node;
> @@ -870,16 +867,14 @@ static int geni_i2c_probe(struct platform_device *pdev)
>
> ret = geni_se_resources_on(&gi2c->se);
> if (ret) {
> - dev_err(dev, "Error turning on resources %d\n", ret);
> clk_disable_unprepare(gi2c->core_clk);
> - return ret;
> + return dev_err_probe(dev, ret, "Error turning on resources\n");
> }
> proto = geni_se_read_proto(&gi2c->se);
> if (proto != GENI_SE_I2C) {
> - dev_err(dev, "Invalid proto %d\n", proto);
> geni_se_resources_off(&gi2c->se);
> clk_disable_unprepare(gi2c->core_clk);
> - return -ENXIO;
> + return dev_err_probe(dev, -ENXIO, "Invalid proto %d\n", proto);
> }
>
> if (desc && desc->no_dma_support)
> @@ -894,7 +889,7 @@ static int geni_i2c_probe(struct platform_device *pdev)
> if (ret) {
> geni_se_resources_off(&gi2c->se);
> clk_disable_unprepare(gi2c->core_clk);
> - return dev_err_probe(dev, ret, "Failed to setup GPI DMA mode\n");
> + return ret;
I believe the removal is intentional, since setup_gpi_dma() reports better messages.
> }
>
> dev_dbg(dev, "Using GPI DMA mode for I2C\n");
> @@ -907,10 +902,10 @@ static int geni_i2c_probe(struct platform_device *pdev)
> tx_depth = desc->tx_fifo_depth;
>
> if (!tx_depth) {
> - dev_err(dev, "Invalid TX FIFO depth\n");
> geni_se_resources_off(&gi2c->se);
> clk_disable_unprepare(gi2c->core_clk);
> - return -EINVAL;
> + return dev_err_probe(dev, -EINVAL,
> + "Invalid TX FIFO depth\n");
> }
>
> gi2c->tx_wm = tx_depth - 1;
> @@ -924,7 +919,7 @@ static int geni_i2c_probe(struct platform_device *pdev)
> clk_disable_unprepare(gi2c->core_clk);
> ret = geni_se_resources_off(&gi2c->se);
> if (ret) {
> - dev_err(dev, "Error turning off resources %d\n", ret);
> + dev_err_probe(dev, ret, "Error turning off resources\n");
> goto err_dma;
> }
>
> @@ -940,7 +935,7 @@ static int geni_i2c_probe(struct platform_device *pdev)
>
> ret = i2c_add_adapter(&gi2c->adap);
> if (ret) {
> - dev_err(dev, "Error adding i2c adapter %d\n", ret);
> + dev_err_probe(dev, ret, "Error adding i2c adapter\n");
> pm_runtime_disable(gi2c->se.dev);
> goto err_dma;
> }
Reviewed-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
--
Best wishes,
Vladimir
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] i2c: qcom-geni: Simplify error handling in probe function
2024-12-12 13:54 ` [PATCH v2 2/2] i2c: qcom-geni: Simplify error handling in " Andi Shyti
@ 2024-12-12 18:38 ` Vladimir Zapolskiy
2024-12-13 0:57 ` Andi Shyti
0 siblings, 1 reply; 7+ messages in thread
From: Vladimir Zapolskiy @ 2024-12-12 18:38 UTC (permalink / raw)
To: Andi Shyti, linux-arm-msm, linux-i2c; +Cc: Mukesh Kumar Savaliya
On 12/12/24 15:54, 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@kernel.org>
> ---
> drivers/i2c/busses/i2c-qcom-geni.c | 32 ++++++++++++++++--------------
> 1 file changed, 17 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c
> index 01db24188e29..88709b97f86d 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);
> + goto err_resources;
What's about setting ret = -ENXIO before bailing out?
> }
>
> 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_resources;
>
> 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_resources;
Same comment as above.
> }
>
> gi2c->tx_wm = tx_depth - 1;
> @@ -942,10 +936,18 @@ static int geni_i2c_probe(struct platform_device *pdev)
>
> dev_dbg(dev, "Geni-I2C adaptor successfully added\n");
>
> - return 0;
> + return ret;
> +
> +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;
> }
>
--
Best wishes,
Vladimir
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/2] i2c: qcom-geni: Use dev_err_probe in the probe function
2024-12-12 13:54 ` [PATCH v2 1/2] i2c: qcom-geni: Use dev_err_probe in the probe function Andi Shyti
2024-12-12 18:34 ` Vladimir Zapolskiy
@ 2024-12-12 19:21 ` Mukesh Kumar Savaliya
1 sibling, 0 replies; 7+ messages in thread
From: Mukesh Kumar Savaliya @ 2024-12-12 19:21 UTC (permalink / raw)
To: Andi Shyti, linux-arm-msm, linux-i2c
On 12/12/2024 7:24 PM, Andi Shyti wrote:
> Replace classical dev_err with dev_err_probe in the probe
> function for better error reporting. Also, use dev_err_probe in
> cases where the error number is clear (e.g., -EIO or -EINVAL) to
> maintain consistency.
>
> Additionally, remove redundant logging to simplify the code.
>
> Signed-off-by: Andi Shyti <andi.shyti@kernel.org>
> ---
> drivers/i2c/busses/i2c-qcom-geni.c | 33 +++++++++++++-----------------
> 1 file changed, 14 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c
> index 7a22e1f46e60..01db24188e29 100644
> --- a/drivers/i2c/busses/i2c-qcom-geni.c
> +++ b/drivers/i2c/busses/i2c-qcom-geni.c
> @@ -823,11 +823,9 @@ static int geni_i2c_probe(struct platform_device *pdev)
> return gi2c->irq;
>
> ret = geni_i2c_clk_map_idx(gi2c);
> - if (ret) {
> - dev_err(dev, "Invalid clk frequency %d Hz: %d\n",
> - gi2c->clk_freq_out, ret);
> - return ret;
> - }
> + if (ret)
> + return dev_err_probe(dev, ret, "Invalid clk frequency %d Hz\n",
> + gi2c->clk_freq_out);
>
> gi2c->adap.algo = &geni_i2c_algo;
> init_completion(&gi2c->done);
> @@ -837,11 +835,10 @@ static int geni_i2c_probe(struct platform_device *pdev)
> /* Keep interrupts disabled initially to allow for low-power modes */
> ret = devm_request_irq(dev, gi2c->irq, geni_i2c_irq, IRQF_NO_AUTOEN,
> dev_name(dev), gi2c);
> - if (ret) {
> - dev_err(dev, "Request_irq failed:%d: err:%d\n",
> - gi2c->irq, ret);
> - return ret;
> - }
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "Request_irq failed: %d\n", gi2c->irq);
> +
> i2c_set_adapdata(&gi2c->adap, gi2c);
> gi2c->adap.dev.parent = dev;
> gi2c->adap.dev.of_node = dev->of_node;
> @@ -870,16 +867,14 @@ static int geni_i2c_probe(struct platform_device *pdev)
>
> ret = geni_se_resources_on(&gi2c->se);
> if (ret) {
> - dev_err(dev, "Error turning on resources %d\n", ret);
> clk_disable_unprepare(gi2c->core_clk);
> - return ret;
> + return dev_err_probe(dev, ret, "Error turning on resources\n");
> }
> proto = geni_se_read_proto(&gi2c->se);
> if (proto != GENI_SE_I2C) {
> - dev_err(dev, "Invalid proto %d\n", proto);
> geni_se_resources_off(&gi2c->se);
> clk_disable_unprepare(gi2c->core_clk);
> - return -ENXIO;
> + return dev_err_probe(dev, -ENXIO, "Invalid proto %d\n", proto);
> }
>
> if (desc && desc->no_dma_support)
> @@ -894,7 +889,7 @@ static int geni_i2c_probe(struct platform_device *pdev)
> if (ret) {
> geni_se_resources_off(&gi2c->se);
> clk_disable_unprepare(gi2c->core_clk);
> - return dev_err_probe(dev, ret, "Failed to setup GPI DMA mode\n");
> + return ret;
> }
>
> dev_dbg(dev, "Using GPI DMA mode for I2C\n");
> @@ -907,10 +902,10 @@ static int geni_i2c_probe(struct platform_device *pdev)
> tx_depth = desc->tx_fifo_depth;
>
> if (!tx_depth) {
> - dev_err(dev, "Invalid TX FIFO depth\n");
> geni_se_resources_off(&gi2c->se);
> clk_disable_unprepare(gi2c->core_clk);
> - return -EINVAL;
> + return dev_err_probe(dev, -EINVAL,
> + "Invalid TX FIFO depth\n");
> }
>
> gi2c->tx_wm = tx_depth - 1;
> @@ -924,7 +919,7 @@ static int geni_i2c_probe(struct platform_device *pdev)
> clk_disable_unprepare(gi2c->core_clk);
> ret = geni_se_resources_off(&gi2c->se);
> if (ret) {
> - dev_err(dev, "Error turning off resources %d\n", ret);
> + dev_err_probe(dev, ret, "Error turning off resources\n");
> goto err_dma;
> }
>
> @@ -940,7 +935,7 @@ static int geni_i2c_probe(struct platform_device *pdev)
>
> ret = i2c_add_adapter(&gi2c->adap);
> if (ret) {
> - dev_err(dev, "Error adding i2c adapter %d\n", ret);
> + dev_err_probe(dev, ret, "Error adding i2c adapter\n");
> pm_runtime_disable(gi2c->se.dev);
> goto err_dma;
> }
Acked-by: Mukesh Kumar Savaliya <quic_msavaliy@quicinc.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] i2c: qcom-geni: Simplify error handling in probe function
2024-12-12 18:38 ` Vladimir Zapolskiy
@ 2024-12-13 0:57 ` Andi Shyti
0 siblings, 0 replies; 7+ messages in thread
From: Andi Shyti @ 2024-12-13 0:57 UTC (permalink / raw)
To: Vladimir Zapolskiy; +Cc: linux-arm-msm, linux-i2c, Mukesh Kumar Savaliya
> > 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);
> > + goto err_resources;
>
> What's about setting ret = -ENXIO before bailing out?
Oh yes, sorry, my bad... I snap changed this patch without
using my brain. It was better in v1 :-)
Thanks Vladimir,
Andi
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2024-12-13 0:57 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-12-12 13:54 [PATCH v2 0/2] Qcom Geni exit path cleanups Andi Shyti
2024-12-12 13:54 ` [PATCH v2 1/2] i2c: qcom-geni: Use dev_err_probe in the probe function Andi Shyti
2024-12-12 18:34 ` Vladimir Zapolskiy
2024-12-12 19:21 ` Mukesh Kumar Savaliya
2024-12-12 13:54 ` [PATCH v2 2/2] i2c: qcom-geni: Simplify error handling in " Andi Shyti
2024-12-12 18:38 ` Vladimir Zapolskiy
2024-12-13 0:57 ` Andi Shyti
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox