On 03/04/2016 04:15 AM, Jean Delvare wrote:
Le Thursday 03 March 2016 à 21:57 +0100, Wolfram Sang a écrit :
On Tue, Jan 19, 2016 at 11:00:48AM -0600, minyard@xxxxxxx wrote:
From: Corey Minyard <cminyard@xxxxxxxxxx>
Getting the same alert twice in a row is legal and normal,
especially on a fast device (like running in qemu). Kind of
like interrupts. So don't report duplicate alerts, and deliver
them normally.
Signed-off-by: Corey Minyard <cminyard@xxxxxxxxxx>
Looks plausible to me, but I never used SMBALERT myself. Any chance this
can cause a regression? Jean, what do you think?
I'm afraid I had a good reason to add this check back then. I'll test
with my ADM1032 evaluation board when I get back home (tomorrow at the
earliest.) Maybe my hardware was misbehaving, in which case I agree any
filtering should be done at the device driver level. But I must double
check what the SMBus specification says too.
I looked at the SMBus specification and I couldn't find anything that
would speak to this particular issue. It says it has to stop asserting
the interrupt when the ack is received on the bus, but it doesn't say
when it can re-assert the interrupts.
I will say that without this change SMBus alerts are fairly useless
with IPMI over SMBus on both real hardware and in qemu. It just
spews out these warning messages in qemu, and it prints them out
periodically on real hardware.
To give an idea of what's happening here, on IPMI over SMBus, the
IPMI controller (BMC) will signal that it needs the host to do something
using an alert. The driver does an I2C write to send a request to the
BMC to find out what it needs. The BMC performs the request then
signals with an SMBus alert that the response is ready. If the BMC is
very fast (like in the qemu case) or the host gets delayed enough before
coming back to this loop, the BMC will have the response ready and
reassert alert before the next check in the loop.
I don't see a way to fix this that handles both scenarios.
-corey
Either way the patch's subject is misleading. Should be "Don't filter
out duplicate alerts" or something like that.
---
drivers/i2c/i2c-smbus.c | 7 -------
1 file changed, 7 deletions(-)
diff --git a/drivers/i2c/i2c-smbus.c b/drivers/i2c/i2c-smbus.c
index 94765a8..cecd423 100644
--- a/drivers/i2c/i2c-smbus.c
+++ b/drivers/i2c/i2c-smbus.c
@@ -75,7 +75,6 @@ static void smbus_alert(struct work_struct *work)
{
struct i2c_smbus_alert *alert;
struct i2c_client *ara;
- unsigned short prev_addr = 0; /* Not a valid address */
alert = container_of(work, struct i2c_smbus_alert, alert);
ara = alert->ara;
@@ -99,18 +98,12 @@ static void smbus_alert(struct work_struct *work)
data.flag = status & 1;
data.addr = status >> 1;
- if (data.addr == prev_addr) {
- dev_warn(&ara->dev, "Duplicate SMBALERT# from dev "
- "0x%02x, skipping\n", data.addr);
- break;
- }
dev_dbg(&ara->dev, "SMBALERT# from dev 0x%02x, flag %d\n",
data.addr, data.flag);
/* Notify driver for the device which issued the alert */
device_for_each_child(&ara->adapter->dev, &data,
smbus_do_alert);
- prev_addr = data.addr;
}
/* We handled all alerts; re-enable level-triggered IRQs */
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
