diff mbox series

[v2] i2c: cadence: Fix regression with bus recovery

Message ID 20221128105158.1536551-1-carsten.haitzler@foss.arm.com
State New
Headers show
Series [v2] i2c: cadence: Fix regression with bus recovery | expand

Commit Message

Carsten Haitzler Nov. 28, 2022, 10:51 a.m. UTC 
From: Carsten Haitzler <carsten.haitzler@arm.com>

Commit "i2c: cadence: Add standard bus recovery support" breaks for i2c
devices that have no pinctrl defined. There is no requirement for this
to exist in the DT. This has worked perfectly well without this before in
at least 1 real usage case on hardware (Mali Komeda DPU, Cadence i2c to
talk to a tda99xx phy). Adding the requirement to have pinctrl set up in
the device tree (or otherwise be found) is a regression where the whole
i2c device is lost entirely (in this case dropping entire devices which
then leads to the drm display stack unable to find the phy for display
output, thus having no drm display device and so on down the chain).

This converts the above commit to an enhancement if pinctrl can be found
for the i2c device, providing a timeout on read with recovery, but if not,
do what used to be done rather than a fatal loss of a device.

This restores the mentioned display devices to their working state again.

Fixes: 58b924241d0a ("i2c: cadence: Add standard bus recovery support")
Signed-off-by: Carsten Haitzler <carsten.haitzler@arm.com>
---
Note: This issue was discovered during the porting of the linux kernel
on Morello [1].

[1] https://git.morello-project.org/morello/kernel/linux
---
 drivers/i2c/busses/i2c-cadence.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

Comments

Wolfram Sang Dec. 1, 2022, 10:42 p.m. UTC | #1 
On Mon, Nov 28, 2022 at 10:51:58AM +0000, carsten.haitzler@foss.arm.com wrote:
> From: Carsten Haitzler <carsten.haitzler@arm.com>
> 
> Commit "i2c: cadence: Add standard bus recovery support" breaks for i2c
> devices that have no pinctrl defined. There is no requirement for this
> to exist in the DT. This has worked perfectly well without this before in
> at least 1 real usage case on hardware (Mali Komeda DPU, Cadence i2c to
> talk to a tda99xx phy). Adding the requirement to have pinctrl set up in
> the device tree (or otherwise be found) is a regression where the whole
> i2c device is lost entirely (in this case dropping entire devices which
> then leads to the drm display stack unable to find the phy for display
> output, thus having no drm display device and so on down the chain).
> 
> This converts the above commit to an enhancement if pinctrl can be found
> for the i2c device, providing a timeout on read with recovery, but if not,
> do what used to be done rather than a fatal loss of a device.
> 
> This restores the mentioned display devices to their working state again.
> 
> Fixes: 58b924241d0a ("i2c: cadence: Add standard bus recovery support")
> Signed-off-by: Carsten Haitzler <carsten.haitzler@arm.com>

Fixed this checkpatch check:

CHECK: Unbalanced braces around else statement
#55: FILE: drivers/i2c/busses/i2c-cadence.c:1272:
+	} else

and applied to for-current, thanks!
diff mbox series

Patch

diff --git a/drivers/i2c/busses/i2c-cadence.c b/drivers/i2c/busses/i2c-cadence.c
index fe0cd205502d..09acd2d399d5 100644
--- a/drivers/i2c/busses/i2c-cadence.c
+++ b/drivers/i2c/busses/i2c-cadence.c
@@ -852,7 +852,8 @@  static int cdns_i2c_master_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
 					 CDNS_I2C_POLL_US, CDNS_I2C_TIMEOUT_US);
 	if (ret) {
 		ret = -EAGAIN;
-		i2c_recover_bus(adap);
+		if (id->adap.bus_recovery_info)
+			i2c_recover_bus(adap);
 		goto out;
 	}
 
@@ -1263,9 +1264,13 @@  static int cdns_i2c_probe(struct platform_device *pdev)
 
 	id->rinfo.pinctrl = devm_pinctrl_get(&pdev->dev);
 	if (IS_ERR(id->rinfo.pinctrl)) {
+		int err = PTR_ERR(id->rinfo.pinctrl);
+
 		dev_info(&pdev->dev, "can't get pinctrl, bus recovery not supported\n");
-		return PTR_ERR(id->rinfo.pinctrl);
-	}
+		if (err != -ENODEV)
+			return err;
+	} else
+		id->adap.bus_recovery_info = &id->rinfo;
 
 	id->membase = devm_platform_get_and_ioremap_resource(pdev, 0, &r_mem);
 	if (IS_ERR(id->membase))
@@ -1283,7 +1288,6 @@  static int cdns_i2c_probe(struct platform_device *pdev)
 	id->adap.retries = 3;		/* Default retry value. */
 	id->adap.algo_data = id;
 	id->adap.dev.parent = &pdev->dev;
-	id->adap.bus_recovery_info = &id->rinfo;
 	init_completion(&id->xfer_done);
 	snprintf(id->adap.name, sizeof(id->adap.name),
 		 "Cadence I2C at %08lx", (unsigned long)r_mem->start);