Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit c364d250 authored by Guenter Roeck's avatar Guenter Roeck Committed by Vegard Nossum
Browse files

i2c: smbus: Improve handling of stuck alerts



[ Upstream commit 37c526f00bc1c4f847fc800085f8f009d2e11be6 ]

The following messages were observed while testing alert functionality
on systems with multiple I2C devices on a single bus if alert was active
on more than one chip.

smbus_alert 3-000c: SMBALERT# from dev 0x0c, flag 0
smbus_alert 3-000c: no driver alert()!

and:

smbus_alert 3-000c: SMBALERT# from dev 0x28, flag 0

Once it starts, this message repeats forever at high rate. There is no
device at any of the reported addresses.

Analysis shows that this is seen if multiple devices have the alert pin
active. Apparently some devices do not support SMBus arbitration correctly.
They keep sending address bits after detecting an address collision and
handle the collision not at all or too late.
Specifically, address 0x0c is seen with ADT7461A at address 0x4c and
ADM1021 at address 0x18 if alert is active on both chips. Address 0x28 is
seen with ADT7483 at address 0x2a and ADT7461 at address 0x4c if alert is
active on both chips.

Once the system is in bad state (alert is set by more than one chip),
it often only recovers by power cycling.

To reduce the impact of this problem, abort the endless loop in
smbus_alert() if the same address is read more than once and not
handled by a driver.

Fixes: b5527a77 ("i2c: Add SMBus alert support")
Signed-off-by: default avatarGuenter Roeck <linux@roeck-us.net>
[wsa: it also fixed an interrupt storm in one of my experiments]
Tested-by: default avatarWolfram Sang <wsa+renesas@sang-engineering.com>
[wsa: rebased, moved a comment as well, improved the 'invalid' value]
Signed-off-by: default avatarWolfram Sang <wsa+renesas@sang-engineering.com>
Stable-dep-of: f6c29f710c1f ("i2c: smbus: Send alert notifications to all devices if source not found")
Signed-off-by: default avatarSasha Levin <sashal@kernel.org>
(cherry picked from commit 9540badee607a99cc07bddbd0a7d4a01fd3b9661)
Signed-off-by: default avatarVegard Nossum <vegard.nossum@oracle.com>
parent 7a346f1c
Loading
Loading
Loading
Loading
+25 −7
Original line number Diff line number Diff line
@@ -43,6 +43,7 @@ static int smbus_do_alert(struct device *dev, void *addrp)
	struct i2c_client *client = i2c_verify_client(dev);
	struct alert_data *data = addrp;
	struct i2c_driver *driver;
	int ret;

	if (!client || client->addr != data->addr)
		return 0;
@@ -56,16 +57,21 @@ static int smbus_do_alert(struct device *dev, void *addrp)
	device_lock(dev);
	if (client->dev.driver) {
		driver = to_i2c_driver(client->dev.driver);
		if (driver->alert)
		if (driver->alert) {
			/* Stop iterating after we find the device */
			driver->alert(client, data->type, data->data);
		else
			ret = -EBUSY;
		} else {
			dev_warn(&client->dev, "no driver alert()!\n");
	} else
			ret = -EOPNOTSUPP;
		}
	} else {
		dev_dbg(&client->dev, "alert with no driver\n");
		ret = -ENODEV;
	}
	device_unlock(dev);

	/* Stop iterating after we find the device */
	return -EBUSY;
	return ret;
}

/*
@@ -76,6 +82,7 @@ static void smbus_alert(struct work_struct *work)
{
	struct i2c_smbus_alert *alert;
	struct i2c_client *ara;
	unsigned short prev_addr = I2C_CLIENT_END; /* Not a valid address */

	alert = container_of(work, struct i2c_smbus_alert, alert);
	ara = alert->ara;
@@ -104,8 +111,19 @@ static void smbus_alert(struct work_struct *work)
			data.addr, data.data);

		/* Notify driver for the device which issued the alert */
		device_for_each_child(&ara->adapter->dev, &data,
		status = device_for_each_child(&ara->adapter->dev, &data,
					       smbus_do_alert);
		/*
		 * If we read the same address more than once, and the alert
		 * was not handled by a driver, it won't do any good to repeat
		 * the loop because it will never terminate.
		 * Bail out in this case.
		 * Note: This assumes that a driver with alert handler handles
		 * the alert properly and clears it if necessary.
		 */
		if (data.addr == prev_addr && status != -EBUSY)
			break;
		prev_addr = data.addr;
	}

	/* We handled all alerts; re-enable level-triggered IRQs */