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.
Signed-off-by: Artem Shimko <a.shimko.dev@gmail.com>
---
drivers/i2c/busses/i2c-designware-platdrv.c | 30 +++++++--------------
1 file changed, 9 insertions(+), 21 deletions(-)
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 7be99656a67d..88e70970c72c 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -227,40 +227,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);
-
ret = i2c_dw_fw_parse_and_configure(dev);
if (ret)
- goto exit_reset;
+ return ret;
ret = i2c_dw_probe_lock_support(dev);
- if (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;
@@ -308,8 +300,6 @@ 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;
}
@@ -329,8 +319,6 @@ static void dw_i2c_plat_remove(struct platform_device *pdev)
dw_i2c_plat_pm_cleanup(dev);
i2c_dw_prepare_clk(dev, false);
-
- reset_control_assert(dev->rst);
}
static const struct of_device_id dw_i2c_of_match[] = {
--
2.43.0
Hi Artem,
On Fri, Jan 30, 2026 at 02:10: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.
>
> Signed-off-by: Artem Shimko <a.shimko.dev@gmail.com>
> ---
> drivers/i2c/busses/i2c-designware-platdrv.c | 30 +++++++--------------
> 1 file changed, 9 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
> index 7be99656a67d..88e70970c72c 100644
> --- a/drivers/i2c/busses/i2c-designware-platdrv.c
> +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
> @@ -227,40 +227,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);
> -
> ret = i2c_dw_fw_parse_and_configure(dev);
> if (ret)
> - goto exit_reset;
> + return ret;
>
> ret = i2c_dw_probe_lock_support(dev);
> - if (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");
the dev_err_probe() changes are not mentioned in the commit log.
Can you please split this patch in two, one for the
devm_reset_control and one for the dev_err_probe?
Thanks,
Andi
Hi Andi, Thank you for your review! On Wed, Feb 4, 2026 at 4:43 AM Andi Shyti <andi.shyti@kernel.org> wrote: > the dev_err_probe() changes are not mentioned in the commit log. > Can you please split this patch in two, one for the > devm_reset_control and one for the dev_err_probe? May I suggest to keep the commit as it is? From my point of view this dev_err_probe() relates to changes of the reset. Otherwise, I'll have to return the reset_control_assert() to the first commit, that's unnecessary when we use the devm_reset_control_get_optional_exclusive_deasserted(). It'll look like: 1. devm_reset_control_get_optional_exclusive() + reset_control_deassert() to devm_reset_control_get_optional_exclusive_deasserted() keeping at the same time unnecessary reset_control_assert() 2. removing reset_control_assert() 3. cleanup of exit_probe My suggestion: 1. change devm_reset_control_get_optional_exclusive() + reset_control_deassert() to devm_reset_control_get_optional_exclusive_deasserted() as well as remove unneeded reset_control_assert() 2. cleanup of exit_probe Regards, Artem
On Thu, Feb 05, 2026 at 10:56:13AM +0300, Artem Shimko wrote: > Hi Andi, > > Thank you for your review! > > On Wed, Feb 4, 2026 at 4:43 AM Andi Shyti <andi.shyti@kernel.org> wrote: > > the dev_err_probe() changes are not mentioned in the commit log. > > Can you please split this patch in two, one for the > > devm_reset_control and one for the dev_err_probe? > > May I suggest to keep the commit as it is? From my point of view this > dev_err_probe() relates to changes of the reset. Otherwise, I'll have > to return the reset_control_assert() to the first commit, that's > unnecessary when we use the > devm_reset_control_get_optional_exclusive_deasserted(). I'm sorry, it looked to me in a second look like there was one dev_err_probe() replacement independent from the patch. The patch is fine and I'm going to apply it in i2c/i2c-host-2 (it will be sent as a pull request next week). Thanks, Andi
On Thu, Feb 5, 2026 at 1:27 PM Andi Shyti <andi.shyti@kernel.org> wrote: > The patch > is fine and I'm going to apply it in i2c/i2c-host-2 (it will be > sent as a pull request next week). Great, thank you! -- Regards, Artem
On Wed, Feb 04, 2026 at 02:43:30AM +0100, Andi Shyti wrote:
> On Fri, Jan 30, 2026 at 02:10: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.
...
> > - if (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");
>
> the dev_err_probe() changes are not mentioned in the commit log.
> Can you please split this patch in two, one for the
> devm_reset_control and one for the dev_err_probe?
I believe they can go together.
But can be split as well, first we replace goto by return ret followed by
the joining the dev_err_probe(). In any case the commit message might be
improved, yes.
--
With Best Regards,
Andy Shevchenko
Hi Andy, On Wed, Feb 4, 2026 at 4:50 AM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > In any case the commit message might be > improved, yes. Got it. Thanks for the review! Regards, Artem
On Fri, Jan 30, 2026 at 02:10: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. > > Signed-off-by: Artem Shimko <a.shimko.dev@gmail.com> This is nice cleanup! Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>
© 2016 - 2026 Red Hat, Inc.