From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 1E04D19F47A; Thu, 15 Aug 2024 14:00:30 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1723730430; cv=none; b=LDvC406eczvbClFVi1F368zj9Qqy0s3SPOl3H7T6T4M1xfG0KS4DalFhhgtuk6GYxrHbrmi5gH6SMrTefPgsDktzdZTNXYsLfz8ZCFcRIkH4QsaEIZINaqfeXR4pbkw6pU8VkUm4a4VoMrdAY/aIJO6fsxoO/tgNLUacL8Rex5k= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1723730430; c=relaxed/simple; bh=MeFMzvQ0N0qGFc/XXdhcNAF3TrO9a9tXLSxEHBIUY6s=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=EFd4FfX74JhATavs59HB3ZrDokLFxZGsx9jx7mPLtbYMPAuCbzX8kSgv8gmsPYIxAOPbvFv0hQ3ZJeovOiQKyn6Q5dP1YieXIXsTCzkynKKtYkGAn/ivFPa7GUUSQdQLjR3o1fTMvIkBvoxUip5KRnr3VoiJZNEtLI3xMKq0HeA= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linuxfoundation.org header.i=@linuxfoundation.org header.b=aLgt7ySS; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linuxfoundation.org header.i=@linuxfoundation.org header.b="aLgt7ySS" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 80CC9C4AF0C; Thu, 15 Aug 2024 14:00:29 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1723730430; bh=MeFMzvQ0N0qGFc/XXdhcNAF3TrO9a9tXLSxEHBIUY6s=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=aLgt7ySS305bWYHItlEbv2XxrjAC/sz432g0qJkRjIhm4Bcuf8xZ2Em21xxlVkAh9 j2YE9lQPuNHeFP1PD9CWYk9Y0H7T2XZZpKvlnaTjasQNQG9ApbjY5FH5k/0IyjG+1S WSq7Mir5/HC1+Tb4P52lRWLDoGh7BeYrhpiQ+oHA= From: Greg Kroah-Hartman To: stable@vger.kernel.org Cc: Greg Kroah-Hartman , patches@lists.linux.dev, Guenter Roeck , Wolfram Sang , Sasha Levin Subject: [PATCH 5.15 406/484] i2c: smbus: Improve handling of stuck alerts Date: Thu, 15 Aug 2024 15:24:24 +0200 Message-ID: <20240815131957.133487237@linuxfoundation.org> X-Mailer: git-send-email 2.46.0 In-Reply-To: <20240815131941.255804951@linuxfoundation.org> References: <20240815131941.255804951@linuxfoundation.org> User-Agent: quilt/0.67 X-stable: review X-Patchwork-Hint: ignore Precedence: bulk X-Mailing-List: stable@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit 5.15-stable review patch. If anyone has any objections, please let me know. ------------------ From: Guenter Roeck [ 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: 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 Signed-off-by: Sasha Levin --- 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 d3d06e3b4f3b3..dcbc4b29aa729 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; -- 2.43.0