[PATCH RESEND] i2c: designware-platdrv: handle reset control deassert error

[PATCH RESEND] i2c: designware-platdrv: handle reset control deassert error

[Artem Shimko]

Handle the error returned by reset_control_deassert() in the probe
function to prevent continuing probe when reset deassertion fails.

Previously, reset_control_deassert() was called without checking its
return value, which could lead to probe continuing even when the
device reset wasn't properly deasserted.

The fix checks the return value and returns an error with dev_err_probe()
if reset deassertion fails, providing better error handling.

Signed-off-by: Artem Shimko <a.shimko.dev@gmail.com>
---

Hi,

Resending this patch that seems to have been overlooked.
Thanks for your time.

Previous discussion: https://lore.kernel.org/all/c863512af9a13eb92bde7e0d383d4b4c81e5ce3e.camel@pengutronix.de/T/#t

--
Regards
Artem

 drivers/i2c/busses/i2c-designware-platdrv.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 34d881572351..bfaedb851511 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -240,7 +240,9 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
 	if (IS_ERR(dev->rst))
 		return dev_err_probe(device, PTR_ERR(dev->rst), "failed to acquire reset\n");
 
-	reset_control_deassert(dev->rst);
+	ret = reset_control_deassert(dev->rst);
+	if (ret)
+		return dev_err_probe(device, ret, "Failed to deassert reset\n");
 
 	ret = i2c_dw_fw_parse_and_configure(dev);
 	if (ret)
-- 
2.43.0

Re: [PATCH RESEND] i2c: designware-platdrv: handle reset control deassert error

[Philipp Zabel]

On Di, 2025-11-11 at 10:53 +0300, Artem Shimko wrote:
> Handle the error returned by reset_control_deassert() in the probe
> function to prevent continuing probe when reset deassertion fails.
> 
> Previously, reset_control_deassert() was called without checking its
> return value, which could lead to probe continuing even when the
> device reset wasn't properly deasserted.
> 
> The fix checks the return value and returns an error with dev_err_probe()
> if reset deassertion fails, providing better error handling.
> 
> Signed-off-by: Artem Shimko <a.shimko.dev@gmail.com>

Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>

Looking at the surroundings, this driver could be simplified with
devm_reset_control_get_optional_exclusive_deasserted().

regards
Philipp

[PATCH v2 1/3] i2c: designware-platdrv: simplify reset control with devm variant

[Artem Shimko]

The current implementation uses separate calls to acquire and deassert
reset control, requiring manual error handling for the deassertion
operation. This can be simplified using the dedicated devm function that
combines both operations.

Replace devm_reset_control_get_optional_exclusive() with
devm_reset_control_get_optional_exclusive_deasserted(), which handles both
reset acquisition and deassertion in a single call. This eliminates
the need for explicit deassertion and its associated error checking while
maintaining the same functional behavior through automatic resource
management.

Suggested-by: Philipp Zabel <p.zabel@pengutronix.de>
Signed-off-by: Artem Shimko <a.shimko.dev@gmail.com>
---
Hi Philip,

That works, thank you.

I also added two more separate commits to improve the driver.

--
Regards,
Artem

ChangeLog:
  v2: Simplify reset control using devm_reset_control_get_optional_exclusive_deasserted()
    * Replace separate reset acquisition and deassertion with combined function
    * Remove explicit reset_control_deassert() call and error handling
    * Maintain same functionality with cleaner code
    * Add devm_add_action_or_reset() to fully automate reset management
    * Remove all manual reset_control_assert() calls from probe and remove
    * Streamline error handling by removing goto exit_probe and using direct cleanup

 drivers/i2c/busses/i2c-designware-platdrv.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 34d881572351..c77029e520dc 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -236,11 +236,9 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	dev->rst = devm_reset_control_get_optional_exclusive(device, NULL);
+	dev->rst = devm_reset_control_get_optional_exclusive_deasserted(device, NULL);
 	if (IS_ERR(dev->rst))
-		return dev_err_probe(device, PTR_ERR(dev->rst), "failed to acquire reset\n");
-
-	reset_control_deassert(dev->rst);
+		return PTR_ERR(dev->rst);
 
 	ret = i2c_dw_fw_parse_and_configure(dev);
 	if (ret)
-- 
2.43.0

[PATCH v2 2/3] i2c: designware-platdrv: complete reset control devm conversion

[Artem Shimko]

The driver still manually calls reset_control_assert() in error paths and
remove function. This creates inconsistent resource management and misses
the benefits of full device-managed approach.

Register devm_add_action_or_reset() callback after acquiring reset control
to handle automatic assertion on probe errors and driver removal. This
eliminates all manual reset_control_assert() calls while maintaining
identical reset behavior through automatic devm resource management.

Signed-off-by: Artem Shimko <a.shimko.dev@gmail.com>
---
 drivers/i2c/busses/i2c-designware-platdrv.c | 34 ++++++++++++---------
 1 file changed, 19 insertions(+), 15 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index c77029e520dc..d334af1d7c6f 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -206,6 +206,13 @@ static void i2c_dw_remove_lock_support(struct dw_i2c_dev *dev)
 		i2c_dw_semaphore_cb_table[dev->semaphore_idx].remove(dev);
 }
 
+static void dw_i2c_plat_assert_reset(void *data)
+{
+	struct dw_i2c_dev *dev = data;
+
+	reset_control_assert(dev->rst);
+}
+
 static int dw_i2c_plat_probe(struct platform_device *pdev)
 {
 	u32 flags = (uintptr_t)device_get_match_data(&pdev->dev);
@@ -240,34 +247,34 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
 	if (IS_ERR(dev->rst))
 		return PTR_ERR(dev->rst);
 
+	ret = devm_add_action_or_reset(device, dw_i2c_plat_assert_reset, dev);
+	if (ret)
+		return ret;
+
 	ret = i2c_dw_fw_parse_and_configure(dev);
 	if (ret)
-		goto exit_reset;
+		return ret;
 
 	ret = i2c_dw_probe_lock_support(dev);
 	if (ret) {
 		ret = dev_err_probe(device, ret, "failed to probe lock support\n");
-		goto exit_reset;
+		return ret;
 	}
 
 	i2c_dw_configure(dev);
 
 	/* Optional interface clock */
 	dev->pclk = devm_clk_get_optional(device, "pclk");
-	if (IS_ERR(dev->pclk)) {
-		ret = dev_err_probe(device, PTR_ERR(dev->pclk), "failed to acquire pclk\n");
-		goto exit_reset;
-	}
+	if (IS_ERR(dev->pclk))
+		return dev_err_probe(device, PTR_ERR(dev->pclk), "failed to acquire pclk\n");
 
 	dev->clk = devm_clk_get_optional(device, NULL);
-	if (IS_ERR(dev->clk)) {
-		ret = dev_err_probe(device, PTR_ERR(dev->clk), "failed to acquire clock\n");
-		goto exit_reset;
-	}
+	if (IS_ERR(dev->clk))
+		return dev_err_probe(device, PTR_ERR(dev->clk), "failed to acquire clock\n");
 
 	ret = i2c_dw_prepare_clk(dev, true);
 	if (ret)
-		goto exit_reset;
+		return ret;
 
 	if (dev->clk) {
 		struct i2c_timings *t = &dev->timings;
@@ -315,8 +322,7 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
 exit_probe:
 	dw_i2c_plat_pm_cleanup(dev);
 	i2c_dw_prepare_clk(dev, false);
-exit_reset:
-	reset_control_assert(dev->rst);
+
 	return ret;
 }
 
@@ -338,8 +344,6 @@ static void dw_i2c_plat_remove(struct platform_device *pdev)
 	i2c_dw_prepare_clk(dev, false);
 
 	i2c_dw_remove_lock_support(dev);
-
-	reset_control_assert(dev->rst);
 }
 
 static const struct of_device_id dw_i2c_of_match[] = {
-- 
2.43.0

Re: [PATCH v2 2/3] i2c: designware-platdrv: complete reset control devm conversion

[Philipp Zabel]

On Di, 2025-11-11 at 14:45 +0300, Artem Shimko wrote:
> The driver still manually calls reset_control_assert() in error paths and
> remove function. This creates inconsistent resource management and misses
> the benefits of full device-managed approach.
> 
> Register devm_add_action_or_reset() callback after acquiring reset control
> to handle automatic assertion on probe errors and driver removal. This
> eliminates all manual reset_control_assert() calls while maintaining
> identical reset behavior through automatic devm resource management.
> 
> Signed-off-by: Artem Shimko <a.shimko.dev@gmail.com>
> ---
>  drivers/i2c/busses/i2c-designware-platdrv.c | 34 ++++++++++++---------
>  1 file changed, 19 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
> index c77029e520dc..d334af1d7c6f 100644
> --- a/drivers/i2c/busses/i2c-designware-platdrv.c
> +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
> @@ -206,6 +206,13 @@ static void i2c_dw_remove_lock_support(struct dw_i2c_dev *dev)
>  		i2c_dw_semaphore_cb_table[dev->semaphore_idx].remove(dev);
>  }
>  
> +static void dw_i2c_plat_assert_reset(void *data)
> +{
> +	struct dw_i2c_dev *dev = data;
> +
> +	reset_control_assert(dev->rst);
> +}
> +
>  static int dw_i2c_plat_probe(struct platform_device *pdev)
>  {
>  	u32 flags = (uintptr_t)device_get_match_data(&pdev->dev);
> @@ -240,34 +247,34 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
>  	if (IS_ERR(dev->rst))
>  		return PTR_ERR(dev->rst);
>  
> +	ret = devm_add_action_or_reset(device, dw_i2c_plat_assert_reset, dev);
> +	if (ret)
> +		return ret;
> +

This is already done by patch 1. Drop these hunks and squash the rest
into patch 1:

>  	ret = i2c_dw_fw_parse_and_configure(dev);
>  	if (ret)
> -		goto exit_reset;
> +		return ret;
>  
>  	ret = i2c_dw_probe_lock_support(dev);
>  	if (ret) {
>  		ret = dev_err_probe(device, ret, "failed to probe lock support\n");
> -		goto exit_reset;
> +		return ret;
>  	}
>  
>  	i2c_dw_configure(dev);
>  
>  	/* Optional interface clock */
>  	dev->pclk = devm_clk_get_optional(device, "pclk");
> -	if (IS_ERR(dev->pclk)) {
> -		ret = dev_err_probe(device, PTR_ERR(dev->pclk), "failed to acquire pclk\n");
> -		goto exit_reset;
> -	}
> +	if (IS_ERR(dev->pclk))
> +		return dev_err_probe(device, PTR_ERR(dev->pclk), "failed to acquire pclk\n");
>  
>  	dev->clk = devm_clk_get_optional(device, NULL);
> -	if (IS_ERR(dev->clk)) {
> -		ret = dev_err_probe(device, PTR_ERR(dev->clk), "failed to acquire clock\n");
> -		goto exit_reset;
> -	}
> +	if (IS_ERR(dev->clk))
> +		return dev_err_probe(device, PTR_ERR(dev->clk), "failed to acquire clock\n");
>  
>  	ret = i2c_dw_prepare_clk(dev, true);
>  	if (ret)
> -		goto exit_reset;
> +		return ret;
>  
>  	if (dev->clk) {
>  		struct i2c_timings *t = &dev->timings;
> @@ -315,8 +322,7 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
>  exit_probe:
>  	dw_i2c_plat_pm_cleanup(dev);
>  	i2c_dw_prepare_clk(dev, false);
> -exit_reset:
> -	reset_control_assert(dev->rst);
> +
>  	return ret;
>  }
>  
> @@ -338,8 +344,6 @@ static void dw_i2c_plat_remove(struct platform_device *pdev)
>  	i2c_dw_prepare_clk(dev, false);
>  
>  	i2c_dw_remove_lock_support(dev);
> -
> -	reset_control_assert(dev->rst);
>  }
>  
>  static const struct of_device_id dw_i2c_of_match[] = {

regards
Philipp

[PATCH v3] i2c: designware-platdrv: simplify reset control and error handling

[Artem Shimko]

The current reset control management uses manual deassertion and assertion
calls with goto-based error handling, which complicates the code and misses
the benefits of full device-managed resource handling.

Convert to devm_reset_control_get_optional_exclusive_deasserted() combined
with devm_add_action_or_reset() to automate reset control lifecycle. This
eliminates all manual reset_control_deassert() and reset_control_assert()
calls while maintaining the same reset behavior.

As part of this cleanup, streamline the error handling by removing goto
exit_reset and goto exit_probe labels, using direct returns with
dev_err_probe() for cleaner and more linear code flow.

Suggested-by: Philipp Zabel <p.zabel@pengutronix.de>
Signed-off-by: Artem Shimko <a.shimko.dev@gmail.com>
---

Thank you.

Done

--
Regards
Artem

v3:
  * Squash all reset control and error handling changes into single patch

 drivers/i2c/busses/i2c-designware-platdrv.c | 52 ++++++++++-----------
 1 file changed, 24 insertions(+), 28 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 34d881572351..ab15a924dad5 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -206,6 +206,13 @@ static void i2c_dw_remove_lock_support(struct dw_i2c_dev *dev)
 		i2c_dw_semaphore_cb_table[dev->semaphore_idx].remove(dev);
 }
 
+static void dw_i2c_plat_assert_reset(void *data)
+{
+	struct dw_i2c_dev *dev = data;
+
+	reset_control_assert(dev->rst);
+}
+
 static int dw_i2c_plat_probe(struct platform_device *pdev)
 {
 	u32 flags = (uintptr_t)device_get_match_data(&pdev->dev);
@@ -236,40 +243,36 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	dev->rst = devm_reset_control_get_optional_exclusive(device, NULL);
+	dev->rst = devm_reset_control_get_optional_exclusive_deasserted(device, NULL);
 	if (IS_ERR(dev->rst))
-		return dev_err_probe(device, PTR_ERR(dev->rst), "failed to acquire reset\n");
+		return PTR_ERR(dev->rst);
 
-	reset_control_deassert(dev->rst);
+	ret = devm_add_action_or_reset(device, dw_i2c_plat_assert_reset, dev);
+	if (ret)
+		return ret;
 
 	ret = i2c_dw_fw_parse_and_configure(dev);
 	if (ret)
-		goto exit_reset;
+		return ret;
 
 	ret = i2c_dw_probe_lock_support(dev);
-	if (ret) {
-		ret = dev_err_probe(device, ret, "failed to probe lock support\n");
-		goto exit_reset;
-	}
+	if (ret)
+		return dev_err_probe(device, ret, "failed to probe lock support\n");
 
 	i2c_dw_configure(dev);
 
 	/* Optional interface clock */
 	dev->pclk = devm_clk_get_optional(device, "pclk");
-	if (IS_ERR(dev->pclk)) {
-		ret = dev_err_probe(device, PTR_ERR(dev->pclk), "failed to acquire pclk\n");
-		goto exit_reset;
-	}
+	if (IS_ERR(dev->pclk))
+		return dev_err_probe(device, PTR_ERR(dev->pclk), "failed to acquire pclk\n");
 
 	dev->clk = devm_clk_get_optional(device, NULL);
-	if (IS_ERR(dev->clk)) {
-		ret = dev_err_probe(device, PTR_ERR(dev->clk), "failed to acquire clock\n");
-		goto exit_reset;
-	}
+	if (IS_ERR(dev->clk))
+		return dev_err_probe(device, PTR_ERR(dev->clk), "failed to acquire clock\n");
 
 	ret = i2c_dw_prepare_clk(dev, true);
 	if (ret)
-		goto exit_reset;
+		return ret;
 
 	if (dev->clk) {
 		struct i2c_timings *t = &dev->timings;
@@ -309,16 +312,11 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
 	pm_runtime_enable(device);
 
 	ret = i2c_dw_probe(dev);
-	if (ret)
-		goto exit_probe;
-
-	return ret;
+	if (ret) {
+		dw_i2c_plat_pm_cleanup(dev);
+		i2c_dw_prepare_clk(dev, false);
+	}
 
-exit_probe:
-	dw_i2c_plat_pm_cleanup(dev);
-	i2c_dw_prepare_clk(dev, false);
-exit_reset:
-	reset_control_assert(dev->rst);
 	return ret;
 }
 
@@ -340,8 +338,6 @@ static void dw_i2c_plat_remove(struct platform_device *pdev)
 	i2c_dw_prepare_clk(dev, false);
 
 	i2c_dw_remove_lock_support(dev);
-
-	reset_control_assert(dev->rst);
 }
 
 static const struct of_device_id dw_i2c_of_match[] = {
-- 
2.43.0

Re: [PATCH v3] i2c: designware-platdrv: simplify reset control and error handling

[Philipp Zabel]

On Di, 2025-11-11 at 17:09 +0300, Artem Shimko wrote:
> The current reset control management uses manual deassertion and assertion
> calls with goto-based error handling, which complicates the code and misses
> the benefits of full device-managed resource handling.

Unnecessary.


[...]
> combined with devm_add_action_or_reset()

No, that's not what I requested in v2.

regards
Philipp

Re: [PATCH v2 2/3] i2c: designware-platdrv: complete reset control devm conversion

[Philipp Zabel]

On Di, 2025-11-11 at 13:43 +0100, Philipp Zabel wrote:
> On Di, 2025-11-11 at 14:45 +0300, Artem Shimko wrote:
> > The driver still manually calls reset_control_assert() in error paths and
> > remove function. This creates inconsistent resource management and misses
> > the benefits of full device-managed approach.
> > 
> > Register devm_add_action_or_reset() callback after acquiring reset control
> > to handle automatic assertion on probe errors and driver removal. This
> > eliminates all manual reset_control_assert() calls while maintaining
> > identical reset behavior through automatic devm resource management.
> > 
> > Signed-off-by: Artem Shimko <a.shimko.dev@gmail.com>
> > ---
> >  drivers/i2c/busses/i2c-designware-platdrv.c | 34 ++++++++++++---------
> >  1 file changed, 19 insertions(+), 15 deletions(-)
> > 
> > diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
> > index c77029e520dc..d334af1d7c6f 100644
> > --- a/drivers/i2c/busses/i2c-designware-platdrv.c
> > +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
> > @@ -206,6 +206,13 @@ static void i2c_dw_remove_lock_support(struct dw_i2c_dev *dev)
> >  		i2c_dw_semaphore_cb_table[dev->semaphore_idx].remove(dev);
> >  }
> >  
> > +static void dw_i2c_plat_assert_reset(void *data)
> > +{
> > +	struct dw_i2c_dev *dev = data;
> > +
> > +	reset_control_assert(dev->rst);
> > +}
> > +
> >  static int dw_i2c_plat_probe(struct platform_device *pdev)
> >  {
> >  	u32 flags = (uintptr_t)device_get_match_data(&pdev->dev);
> > @@ -240,34 +247,34 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
> >  	if (IS_ERR(dev->rst))
> >  		return PTR_ERR(dev->rst);
> >  
> > +	ret = devm_add_action_or_reset(device, dw_i2c_plat_assert_reset, dev);
> > +	if (ret)
> > +		return ret;
> > +
> 
> This is already done by patch 1. Drop these hunks

See [1], _deasserted() already includes assert-on-detach.

[1] https://docs.kernel.org/driver-api/reset.html#c.devm_reset_control_get_optional_exclusive_deasserted

regards
Philipp

[PATCH v4] i2c: designware-platdrv: simplify reset control and error handling

[Artem Shimko]

The current implementation uses separate calls to acquire and deassert
reset control, requiring manual error handling for the deassertion
operation. This can be simplified using the dedicated devm function that
combines both operations.

Replace devm_reset_control_get_optional_exclusive() with
devm_reset_control_get_optional_exclusive_deasserted(), which handles both
reset acquisition and deassertion in a single call as well as
reset_control_put() which is called automatically on driver detach. This
eliminates the need for explicit deassertion and its associated error
checking while maintaining the same functional behavior through automatic
resource management.

As part of this cleanup, streamline the error handling by removing goto
exit_reset and goto exit_probe labels, using direct returns with
dev_err_probe() for cleaner and more linear code flow.

Suggested-by: Philipp Zabel <p.zabel@pengutronix.de>
Signed-off-by: Artem Shimko <a.shimko.dev@gmail.com>
---

Oh sorry Philipp, now I got it

If you have a time, could you please have a look at this version?

Thank you!

--
Regards,
Artem

 drivers/i2c/busses/i2c-designware-platdrv.c | 43 +++++++--------------
 1 file changed, 14 insertions(+), 29 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 34d881572351..147eda5f5268 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -236,40 +236,32 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	dev->rst = devm_reset_control_get_optional_exclusive(device, NULL);
+	dev->rst = devm_reset_control_get_optional_exclusive_deasserted(device, NULL);
 	if (IS_ERR(dev->rst))
-		return dev_err_probe(device, PTR_ERR(dev->rst), "failed to acquire reset\n");
-
-	reset_control_deassert(dev->rst);
+		return PTR_ERR(dev->rst);
 
 	ret = i2c_dw_fw_parse_and_configure(dev);
 	if (ret)
-		goto exit_reset;
+		return ret;
 
 	ret = i2c_dw_probe_lock_support(dev);
-	if (ret) {
-		ret = dev_err_probe(device, ret, "failed to probe lock support\n");
-		goto exit_reset;
-	}
+	if (ret)
+		return dev_err_probe(device, ret, "failed to probe lock support\n");
 
 	i2c_dw_configure(dev);
 
 	/* Optional interface clock */
 	dev->pclk = devm_clk_get_optional(device, "pclk");
-	if (IS_ERR(dev->pclk)) {
-		ret = dev_err_probe(device, PTR_ERR(dev->pclk), "failed to acquire pclk\n");
-		goto exit_reset;
-	}
+	if (IS_ERR(dev->pclk))
+		return dev_err_probe(device, PTR_ERR(dev->pclk), "failed to acquire pclk\n");
 
 	dev->clk = devm_clk_get_optional(device, NULL);
-	if (IS_ERR(dev->clk)) {
-		ret = dev_err_probe(device, PTR_ERR(dev->clk), "failed to acquire clock\n");
-		goto exit_reset;
-	}
+	if (IS_ERR(dev->clk))
+		return dev_err_probe(device, PTR_ERR(dev->clk), "failed to acquire clock\n");
 
 	ret = i2c_dw_prepare_clk(dev, true);
 	if (ret)
-		goto exit_reset;
+		return ret;
 
 	if (dev->clk) {
 		struct i2c_timings *t = &dev->timings;
@@ -309,16 +301,11 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
 	pm_runtime_enable(device);
 
 	ret = i2c_dw_probe(dev);
-	if (ret)
-		goto exit_probe;
-
-	return ret;
+	if (ret) {
+		dw_i2c_plat_pm_cleanup(dev);
+		i2c_dw_prepare_clk(dev, false);
+	}
 
-exit_probe:
-	dw_i2c_plat_pm_cleanup(dev);
-	i2c_dw_prepare_clk(dev, false);
-exit_reset:
-	reset_control_assert(dev->rst);
 	return ret;
 }
 
@@ -340,8 +327,6 @@ static void dw_i2c_plat_remove(struct platform_device *pdev)
 	i2c_dw_prepare_clk(dev, false);
 
 	i2c_dw_remove_lock_support(dev);
-
-	reset_control_assert(dev->rst);
 }
 
 static const struct of_device_id dw_i2c_of_match[] = {
-- 
2.43.0

Re: [PATCH v4] i2c: designware-platdrv: simplify reset control and error handling

[Andi Shyti]

Hi Artem,

I'm sorry, but in one thread there are three versions and I don't
really know what you are addressing. May I ask you to resend it
without linking it to any thread (please don't use in-reply-to).

On Tue, Nov 11, 2025 at 05:55:36PM +0300, Artem Shimko wrote:
> The current implementation uses separate calls to acquire and deassert
> reset control, requiring manual error handling for the deassertion
> operation. This can be simplified using the dedicated devm function that
> combines both operations.
> 
> Replace devm_reset_control_get_optional_exclusive() with
> devm_reset_control_get_optional_exclusive_deasserted(), which handles both
> reset acquisition and deassertion in a single call as well as
> reset_control_put() which is called automatically on driver detach. This
> eliminates the need for explicit deassertion and its associated error
> checking while maintaining the same functional behavior through automatic
> resource management.
> 
> As part of this cleanup, streamline the error handling by removing goto
> exit_reset and goto exit_probe labels, using direct returns with
> dev_err_probe() for cleaner and more linear code flow.

Can you please put this part in a separate patch? They are
logically unrelated.

Thanks,
Andi

[PATCH v2 3/3] i2c: designware-platdrv: streamline error handling

[Artem Shimko]

The probe function uses unnecessary goto statements and error variable
reassignments that complicate the code flow. The goto exit_probe pattern
adds indirection for simple error cleanup, while redundant error assignment
in lock support probe clutters the error handling.

Simplify the error paths by removing the goto exit_probe label and handling
PM runtime cleanup directly after i2c_dw_probe() failure. Additionally,
replace the error variable reassignment in i2c_dw_probe_lock_support() with
direct dev_err_probe() return. These changes make the error handling more
linear and readable while maintaining identical functionality.

Signed-off-by: Artem Shimko <a.shimko.dev@gmail.com>
---
 drivers/i2c/busses/i2c-designware-platdrv.c | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index d334af1d7c6f..ab15a924dad5 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -256,10 +256,8 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
 		return ret;
 
 	ret = i2c_dw_probe_lock_support(dev);
-	if (ret) {
-		ret = dev_err_probe(device, ret, "failed to probe lock support\n");
-		return ret;
-	}
+	if (ret)
+		return dev_err_probe(device, ret, "failed to probe lock support\n");
 
 	i2c_dw_configure(dev);
 
@@ -314,14 +312,10 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
 	pm_runtime_enable(device);
 
 	ret = i2c_dw_probe(dev);
-	if (ret)
-		goto exit_probe;
-
-	return ret;
-
-exit_probe:
-	dw_i2c_plat_pm_cleanup(dev);
-	i2c_dw_prepare_clk(dev, false);
+	if (ret) {
+		dw_i2c_plat_pm_cleanup(dev);
+		i2c_dw_prepare_clk(dev, false);
+	}
 
 	return ret;
 }
-- 
2.43.0