[PATCH] i2c: designware: use dev_err_probe when probing platform device

[PATCH] i2c: designware: use dev_err_probe when probing platform device

[Benoît Monin]

Add calls to dev_err_probe on error paths that can return -EPROBE_DEFER
when probing platform device. Namely when requesting the reset controller,
when probing for lock support and when requesting the clocks.

In i2c_dw_probe_master and i2c_dw_probe_slave, called by the platform
probe from i2c_dw_probe, replace the call to dev_err by dev_err_probe
when failing to acquire the IRQ.

PCI device probing already use dev_err_probe.

Signed-off-by: Benoît Monin <benoit.monin@bootlin.com>
---
I recently spend some time debugging a case where the i2c controller
never showed up. In the end it was caused by a missing reset controller
due to a typo in the device tree.

While this has nothing to do with the i2c designware driver, not having
any hint about why the device stays in deferred probe state does not
help.

The patch add dev_err_probe in the error paths that can return
-EPROBE_DEFER to aid in debugging such case.
---
 drivers/i2c/busses/i2c-designware-master.c  |  9 ++++-----
 drivers/i2c/busses/i2c-designware-platdrv.c | 11 ++++++-----
 drivers/i2c/busses/i2c-designware-slave.c   |  9 ++++-----
 3 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c
index cbd88ffa561010ff2d29086836d9119da5b8885c..c7a72c28786c2b59a249a768d43a7954119bc018 100644
--- a/drivers/i2c/busses/i2c-designware-master.c
+++ b/drivers/i2c/busses/i2c-designware-master.c
@@ -1068,11 +1068,10 @@ int i2c_dw_probe_master(struct dw_i2c_dev *dev)
 	if (!(dev->flags & ACCESS_POLLING)) {
 		ret = devm_request_irq(dev->dev, dev->irq, i2c_dw_isr,
 				       irq_flags, dev_name(dev->dev), dev);
-		if (ret) {
-			dev_err(dev->dev, "failure requesting irq %i: %d\n",
-				dev->irq, ret);
-			return ret;
-		}
+		if (ret)
+			return dev_err_probe(dev->dev, ret,
+					     "failure requesting irq %i: %d\n",
+					     dev->irq, ret);
 	}
 
 	ret = i2c_dw_init_recovery_info(dev);
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index a35e4c64a1d46f43aa2d37c0d20fbbd4bc1ff600..07efe4b529e29b52b9e4e439c4b154b467a24fda 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -238,7 +238,7 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
 
 	dev->rst = devm_reset_control_get_optional_exclusive(device, NULL);
 	if (IS_ERR(dev->rst))
-		return PTR_ERR(dev->rst);
+		return dev_err_probe(device, PTR_ERR(dev->rst), "failed to acquire reset\n");
 
 	reset_control_deassert(dev->rst);
 
@@ -247,21 +247,22 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
 		goto exit_reset;
 
 	ret = i2c_dw_probe_lock_support(dev);
-	if (ret)
+	if (ret) {
+		ret = dev_err_probe(device, ret, "failed to probe lock support\n");
 		goto exit_reset;
-
+	}
 	i2c_dw_configure(dev);
 
 	/* Optional interface clock */
 	dev->pclk = devm_clk_get_optional(device, "pclk");
 	if (IS_ERR(dev->pclk)) {
-		ret = PTR_ERR(dev->pclk);
+		ret = dev_err_probe(device, PTR_ERR(dev->pclk), "failed to acquire pclk\n");
 		goto exit_reset;
 	}
 
 	dev->clk = devm_clk_get_optional(device, NULL);
 	if (IS_ERR(dev->clk)) {
-		ret = PTR_ERR(dev->clk);
+		ret = dev_err_probe(device, PTR_ERR(dev->clk), "failed to acquire clock\n");
 		goto exit_reset;
 	}
 
diff --git a/drivers/i2c/busses/i2c-designware-slave.c b/drivers/i2c/busses/i2c-designware-slave.c
index b936a240db0a9308f005148cdf4c4f9fd512be05..6eb16b7d75a6d059c7abcead609258f9d514d012 100644
--- a/drivers/i2c/busses/i2c-designware-slave.c
+++ b/drivers/i2c/busses/i2c-designware-slave.c
@@ -266,11 +266,10 @@ int i2c_dw_probe_slave(struct dw_i2c_dev *dev)
 
 	ret = devm_request_irq(dev->dev, dev->irq, i2c_dw_isr_slave,
 			       IRQF_SHARED, dev_name(dev->dev), dev);
-	if (ret) {
-		dev_err(dev->dev, "failure requesting IRQ %i: %d\n",
-			dev->irq, ret);
-		return ret;
-	}
+	if (ret)
+		return dev_err_probe(dev->dev, ret,
+				     "failure requesting IRQ %i: %d\n",
+				     dev->irq, ret);
 
 	ret = i2c_add_numbered_adapter(adap);
 	if (ret)

---
base-commit: b320789d6883cc00ac78ce83bccbfe7ed58afcf0
change-id: 20250822-i2c-dw-dev-err-probe-eaf9bce3ef4c

Best regards,
-- 
Benoît Monin, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH] i2c: designware: use dev_err_probe when probing platform device

[Andy Shevchenko]

On Thu, Sep 04, 2025 at 10:59:24AM +0200, Benoît Monin wrote:
> Add calls to dev_err_probe on error paths that can return -EPROBE_DEFER
> when probing platform device. Namely when requesting the reset controller,
> when probing for lock support and when requesting the clocks.
> 
> In i2c_dw_probe_master and i2c_dw_probe_slave, called by the platform
> probe from i2c_dw_probe, replace the call to dev_err by dev_err_probe
> when failing to acquire the IRQ.
> 
> PCI device probing already use dev_err_probe.

What I see here is two patches:
1) conversion existing dev_err() to dev_err_probe();
2) adding messages to some of the plain return $ERR.

Can you split with respective justification for each of them?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] i2c: designware: use dev_err_probe when probing platform device

[Andy Shevchenko]

On Thu, Sep 04, 2025 at 12:47:51PM +0300, Andy Shevchenko wrote:
> On Thu, Sep 04, 2025 at 10:59:24AM +0200, Benoît Monin wrote:
> > Add calls to dev_err_probe on error paths that can return -EPROBE_DEFER

Also add () to the function names or macros with parameters, i.e.
dev_err_probe()

> > when probing platform device. Namely when requesting the reset controller,
> > when probing for lock support and when requesting the clocks.
> > 
> > In i2c_dw_probe_master and i2c_dw_probe_slave, called by the platform

i2c_dw_probe_master() and i2c_dw_probe_slave()

> > probe from i2c_dw_probe, replace the call to dev_err by dev_err_probe

i2c_dw_probe()
dev_err() by dev_err_probe()

> > when failing to acquire the IRQ.
> > 
> > PCI device probing already use dev_err_probe.

dev_err_probe()

> What I see here is two patches:
> 1) conversion existing dev_err() to dev_err_probe();
> 2) adding messages to some of the plain return $ERR.
> 
> Can you split with respective justification for each of them?

-- 
With Best Regards,
Andy Shevchenko