From 37c526f00bc1c4f847fc800085f8f009d2e11be6 Mon Sep 17 00:00:00 2001 From: Guenter Roeck Date: Mon, 10 Jan 2022 09:28:56 -0800 Subject: [PATCH 1/6] i2c: smbus: Improve handling of stuck alerts 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: b5527a7766f0 ("i2c: Add SMBus alert support") Signed-off-by: Guenter Roeck [wsa: it also fixed an interrupt storm in one of my experiments] Tested-by: Wolfram Sang [wsa: rebased, moved a comment as well, improved the 'invalid' value] Signed-off-by: Wolfram Sang --- drivers/i2c/i2c-smbus.c | 32 +++++++++++++++++++++++++------- 1 file changed, 25 insertions(+), 7 deletions(-) diff --git a/drivers/i2c/i2c-smbus.c b/drivers/i2c/i2c-smbus.c index 7e4203df83ed..836c247e7684 100644 --- a/drivers/i2c/i2c-smbus.c +++ b/drivers/i2c/i2c-smbus.c @@ -34,6 +34,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; @@ -47,16 +48,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; } /* @@ -67,6 +73,7 @@ static irqreturn_t smbus_alert(int irq, void *d) { struct i2c_smbus_alert *alert = d; struct i2c_client *ara; + unsigned short prev_addr = I2C_CLIENT_END; /* Not a valid address */ ara = alert->ara; @@ -94,8 +101,19 @@ static irqreturn_t smbus_alert(int irq, void *d) data.addr, data.data); /* Notify driver for the device which issued the alert */ - device_for_each_child(&ara->adapter->dev, &data, - smbus_do_alert); + 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; } return IRQ_HANDLED; From f6c29f710c1ff2590109f83be3e212b86c01e0f3 Mon Sep 17 00:00:00 2001 From: Guenter Roeck Date: Tue, 30 Jul 2024 07:19:41 -0700 Subject: [PATCH 2/6] i2c: smbus: Send alert notifications to all devices if source not found If a SMBus alert is received and the originating device is not found, the reason may be that the address reported on the SMBus alert address is corrupted, for example because multiple devices asserted alert and do not correctly implement SMBus arbitration. If this happens, call alert handlers on all devices connected to the given I2C bus, in the hope that this cleans up the situation. This change reliably fixed the problem on a system with multiple devices on a single bus. Example log where the device on address 0x18 (ADM1021) and on address 0x4c (ADT7461A) both had the alert line asserted: smbus_alert 3-000c: SMBALERT# from dev 0x0c, flag 0 smbus_alert 3-000c: no driver alert()! smbus_alert 3-000c: SMBALERT# from dev 0x0c, flag 0 smbus_alert 3-000c: no driver alert()! lm90 3-0018: temp1 out of range, please check! lm90 3-0018: Disabling ALERT# lm90 3-0029: Everything OK lm90 3-002a: Everything OK lm90 3-004c: temp1 out of range, please check! lm90 3-004c: temp2 out of range, please check! lm90 3-004c: Disabling ALERT# Fixes: b5527a7766f0 ("i2c: Add SMBus alert support") Signed-off-by: Guenter Roeck [wsa: fixed a typo in the commit message] Signed-off-by: Wolfram Sang --- drivers/i2c/i2c-smbus.c | 38 +++++++++++++++++++++++++++++++++++--- 1 file changed, 35 insertions(+), 3 deletions(-) diff --git a/drivers/i2c/i2c-smbus.c b/drivers/i2c/i2c-smbus.c index 836c247e7684..8256f7aed0cf 100644 --- a/drivers/i2c/i2c-smbus.c +++ b/drivers/i2c/i2c-smbus.c @@ -65,6 +65,32 @@ static int smbus_do_alert(struct device *dev, void *addrp) return ret; } +/* Same as above, but call back all drivers with alert handler */ + +static int smbus_do_alert_force(struct device *dev, void *addrp) +{ + struct i2c_client *client = i2c_verify_client(dev); + struct alert_data *data = addrp; + struct i2c_driver *driver; + + if (!client || (client->flags & I2C_CLIENT_TEN)) + return 0; + + /* + * Drivers should either disable alerts, or provide at least + * a minimal handler. Lock so the driver won't change. + */ + device_lock(dev); + if (client->dev.driver) { + driver = to_i2c_driver(client->dev.driver); + if (driver->alert) + driver->alert(client, data->type, data->data); + } + device_unlock(dev); + + return 0; +} + /* * The alert IRQ handler needs to hand work off to a task which can issue * SMBus calls, because those sleeping calls can't be made in IRQ context. @@ -106,13 +132,19 @@ static irqreturn_t smbus_alert(int irq, void *d) /* * 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. + * the loop because it will never terminate. Try again, this + * time calling the alert handlers of all devices connected to + * the bus, and abort the loop afterwards. If this helps, we + * are all set. If it doesn't, there is nothing else we can do, + * so we might as well abort the loop. * 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) + if (data.addr == prev_addr && status != -EBUSY) { + device_for_each_child(&ara->adapter->dev, &data, + smbus_do_alert_force); break; + } prev_addr = data.addr; } From f17c06c6608ad4ecd2ccf321753fb511812d821b Mon Sep 17 00:00:00 2001 From: Richard Fitzgerald Date: Fri, 2 Aug 2024 16:22:14 +0100 Subject: [PATCH 3/6] i2c: Fix conditional for substituting empty ACPI functions Add IS_ENABLED(CONFIG_I2C) to the conditional around a bunch of ACPI functions. The conditional around these functions depended only on CONFIG_ACPI. But the functions are implemented in I2C core, so are only present if CONFIG_I2C is enabled. Signed-off-by: Richard Fitzgerald Signed-off-by: Wolfram Sang --- include/linux/i2c.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/linux/i2c.h b/include/linux/i2c.h index 07e33bbc9256..7eedd0c662da 100644 --- a/include/linux/i2c.h +++ b/include/linux/i2c.h @@ -1066,7 +1066,7 @@ static inline int of_i2c_get_board_info(struct device *dev, struct acpi_resource; struct acpi_resource_i2c_serialbus; -#if IS_ENABLED(CONFIG_ACPI) +#if IS_ENABLED(CONFIG_ACPI) && IS_ENABLED(CONFIG_I2C) bool i2c_acpi_get_i2c_resource(struct acpi_resource *ares, struct acpi_resource_i2c_serialbus **i2c); int i2c_acpi_client_count(struct acpi_device *adev); From b93d16bee557302d4e588375ececd833cc048acc Mon Sep 17 00:00:00 2001 From: Gaosheng Cui Date: Sat, 3 Aug 2024 14:10:41 +0800 Subject: [PATCH 4/6] i2c: qcom-geni: Add missing clk_disable_unprepare in geni_i2c_runtime_resume Add the missing clk_disable_unprepare() before return in geni_i2c_runtime_resume(). Fixes: 14d02fbadb5d ("i2c: qcom-geni: add desc struct to prepare support for I2C Master Hub variant") Signed-off-by: Gaosheng Cui Reviewed-by: Vladimir Zapolskiy Signed-off-by: Andi Shyti --- drivers/i2c/busses/i2c-qcom-geni.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c index 0a8b95ce35f7..78f43648e9f3 100644 --- a/drivers/i2c/busses/i2c-qcom-geni.c +++ b/drivers/i2c/busses/i2c-qcom-geni.c @@ -990,8 +990,10 @@ static int __maybe_unused geni_i2c_runtime_resume(struct device *dev) return ret; ret = geni_se_resources_on(&gi2c->se); - if (ret) + if (ret) { + clk_disable_unprepare(gi2c->core_clk); return ret; + } enable_irq(gi2c->irq); gi2c->suspended = 0; From 9ba48db9f77ce0001dbb882476fa46e092feb695 Mon Sep 17 00:00:00 2001 From: Gaosheng Cui Date: Tue, 6 Aug 2024 20:53:31 +0800 Subject: [PATCH 5/6] i2c: qcom-geni: Add missing geni_icc_disable in geni_i2c_runtime_resume Add the missing geni_icc_disable() before return in geni_i2c_runtime_resume(). Fixes: bf225ed357c6 ("i2c: i2c-qcom-geni: Add interconnect support") Signed-off-by: Gaosheng Cui Reviewed-by: Vladimir Zapolskiy Signed-off-by: Andi Shyti --- drivers/i2c/busses/i2c-qcom-geni.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c index 78f43648e9f3..365e37bba0f3 100644 --- a/drivers/i2c/busses/i2c-qcom-geni.c +++ b/drivers/i2c/busses/i2c-qcom-geni.c @@ -992,6 +992,7 @@ static int __maybe_unused geni_i2c_runtime_resume(struct device *dev) ret = geni_se_resources_on(&gi2c->se); if (ret) { clk_disable_unprepare(gi2c->core_clk); + geni_icc_disable(&gi2c->se); return ret; } From 74b0666f97f9455bc799405b7874df62fcb66bae Mon Sep 17 00:00:00 2001 From: Wolfram Sang Date: Tue, 6 Aug 2024 13:35:33 +0200 Subject: [PATCH 6/6] i2c: testunit: match HostNotify test name with docs Ensure the test has the same name in the code as it has in the docs. Signed-off-by: Wolfram Sang --- drivers/i2c/i2c-slave-testunit.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/i2c/i2c-slave-testunit.c b/drivers/i2c/i2c-slave-testunit.c index 4e03b75f9ad7..4c550306f3ec 100644 --- a/drivers/i2c/i2c-slave-testunit.c +++ b/drivers/i2c/i2c-slave-testunit.c @@ -18,7 +18,7 @@ enum testunit_cmds { TU_CMD_READ_BYTES = 1, /* save 0 for ABORT, RESET or similar */ - TU_CMD_HOST_NOTIFY, + TU_CMD_SMBUS_HOST_NOTIFY, TU_CMD_SMBUS_BLOCK_PROC_CALL, TU_NUM_CMDS }; @@ -60,7 +60,7 @@ static void i2c_slave_testunit_work(struct work_struct *work) msg.len = tu->regs[TU_REG_DATAH]; break; - case TU_CMD_HOST_NOTIFY: + case TU_CMD_SMBUS_HOST_NOTIFY: msg.addr = 0x08; msg.flags = 0; msg.len = 3;