| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Fri, 16 Oct 2020 14:04:02 +0800
From: Michael Wu <michael.wu@...ics.com>
To: Jarkko Nikula <jarkko.nikula@...ux.intel.com>,
Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
Mika Westerberg <mika.westerberg@...ux.intel.com>,
<linux-i2c@...r.kernel.org>, <linux-kernel@...r.kernel.org>
CC: Morgan Chang <morgan.chang@...ics.com>,
Dean Hsiao <dean.hsiao@...ics.com>,
Paul Chen <paul.chen@...ics.com>,
Michael Wu <michael.wu@...ics.com>
Subject: [PATCH 2/2] i2c: designware: slave should do WRITE_REQUESTED before WRITE_RECEIVED
Sometime we would get the following flows when doing an i2cset:
1. No any WRITE_REQUESTED
0x1 STATUS SLAVE_ACTIVITY=0x1 : RAW_INTR_STAT=0x514 : INTR_STAT=0x4
WRITE_RECEIVED
0x1 STATUS SLAVE_ACTIVITY=0x1 : RAW_INTR_STAT=0x514 : INTR_STAT=0x4
WRITE_RECEIVED
0x1 STATUS SLAVE_ACTIVITY=0x0 : RAW_INTR_STAT=0x710 : INTR_STAT=0x200
STOP
2. WRITE_REQUESTED after WRITE_RECEIVED
0x1 STATUS SLAVE_ACTIVITY=0x1 : RAW_INTR_STAT=0x514 : INTR_STAT=0x4
WRITE_RECEIVED
0x1 STATUS SLAVE_ACTIVITY=0x0 : RAW_INTR_STAT=0x714 : INTR_STAT=0x204
WRITE_RECEIVED
WRITE_REQUESTED
Documentation/i2c/slave-interface.rst said that I2C_SLAVE_WRITE_REQUESTED,
which is mandatory, comes without any data arrived. It means
I2C_SLAVE_WRITE_REQUESTED should be in front of any
I2C_SLAVE_WRITE_RECEIVED in a write-request. Obviously the 1st log shows
no I2C_SLAVE_WRITE_REQUESTED, and 2nd log shows I2C_SLAVE_WRITE_REQUESTED
occurred after I2C_SLAVE_WRITE_RECEIVED.
I2C_SLAVE_STOP is mandatory to notify a request finish to reset the state
machine of the backend. In case 2 it never do I2C_SLAVE_STOP when STOP
condition is received. This is one of illegal issues to be fixed.
dev->status can be used to record the current machine state, especially
DesignWare I2C controller has no interrupts to notify a new write-request
coming, an IC_INTR_RX_FULL rising while dev->status isn't
STATUS_WRITE_IN_PROGRESS is notified to do an I2C_SLAVE_WRITE_REQUESTED
by checking dev->status. dev->status also helps to resolve the omitted
I2C_SLAVE_STOP in case 2.
Signed-off-by: Michael Wu <michael.wu@...ics.com>
---
drivers/i2c/busses/i2c-designware-slave.c | 73 ++++++++++++-----------
1 file changed, 39 insertions(+), 34 deletions(-)
diff --git a/drivers/i2c/busses/i2c-designware-slave.c b/drivers/i2c/busses/i2c-designware-slave.c
index 02e7c5171827..57ce1f0b403c 100644
--- a/drivers/i2c/busses/i2c-designware-slave.c
+++ b/drivers/i2c/busses/i2c-designware-slave.c
@@ -172,50 +172,55 @@ static int i2c_dw_irq_handler_slave(struct dw_i2c_dev *dev)
"%#x STATUS SLAVE_ACTIVITY=%#x : RAW_INTR_STAT=%#x : INTR_STAT=%#x\n",
enabled, slave_activity, raw_stat, stat);
+ if (stat & DW_IC_INTR_RX_FULL) {
+ if (dev->status != STATUS_WRITE_IN_PROGRESS) {
+ if (dev->status != STATUS_IDLE)
+ i2c_slave_event(dev->slave, I2C_SLAVE_STOP,
+ &val);
+
+ dev->status = STATUS_WRITE_IN_PROGRESS;
+ i2c_slave_event(dev->slave, I2C_SLAVE_WRITE_REQUESTED,
+ &val);
+ }
+
+ regmap_read(dev->map, DW_IC_DATA_CMD, &tmp);
+ val = tmp;
+ if (!i2c_slave_event(dev->slave, I2C_SLAVE_WRITE_RECEIVED,
+ &val))
+ dev_vdbg(dev->dev, "Byte %X acked!", val);
+ }
+
if (stat & DW_IC_INTR_RD_REQ) {
+ if (dev->status != STATUS_IDLE) {
+ dev->status = STATUS_IDLE;
+ i2c_slave_event(dev->slave, I2C_SLAVE_STOP, &val);
+ }
+
if (slave_activity) {
- if (stat & DW_IC_INTR_RX_FULL) {
- regmap_read(dev->map, DW_IC_DATA_CMD, &tmp);
- val = tmp;
-
- if (!i2c_slave_event(dev->slave,
- I2C_SLAVE_WRITE_RECEIVED,
- &val)) {
- dev_vdbg(dev->dev, "Byte %X acked!",
- val);
- }
- regmap_read(dev->map, DW_IC_CLR_RD_REQ, &tmp);
- } else {
- regmap_read(dev->map, DW_IC_CLR_RD_REQ, &tmp);
- regmap_read(dev->map, DW_IC_CLR_RX_UNDER, &tmp);
- }
- if (!i2c_slave_event(dev->slave,
- I2C_SLAVE_READ_REQUESTED,
- &val))
- regmap_write(dev->map, DW_IC_DATA_CMD, val);
+ regmap_read(dev->map, DW_IC_CLR_RD_REQ, &tmp);
+ regmap_read(dev->map, DW_IC_CLR_RX_UNDER, &tmp);
+
+ dev->status = STATUS_READ_IN_PROGRESS;
+ i2c_slave_event(dev->slave, I2C_SLAVE_READ_REQUESTED,
+ &val);
+ regmap_write(dev->map, DW_IC_DATA_CMD, val);
}
}
if (stat & DW_IC_INTR_RX_DONE) {
- if (!i2c_slave_event(dev->slave, I2C_SLAVE_READ_PROCESSED,
- &val))
- regmap_read(dev->map, DW_IC_CLR_RX_DONE, &tmp);
+ i2c_slave_event(dev->slave, I2C_SLAVE_READ_PROCESSED, &val);
+ regmap_read(dev->map, DW_IC_CLR_RX_DONE, &tmp);
+ dev->status = STATUS_IDLE;
i2c_slave_event(dev->slave, I2C_SLAVE_STOP, &val);
- return 1;
}
- if (stat & DW_IC_INTR_RX_FULL) {
- regmap_read(dev->map, DW_IC_DATA_CMD, &tmp);
- val = tmp;
- if (!i2c_slave_event(dev->slave, I2C_SLAVE_WRITE_RECEIVED,
- &val))
- dev_vdbg(dev->dev, "Byte %X acked!", val);
- } else
- i2c_slave_event(dev->slave, I2C_SLAVE_STOP, &val);
-
- if ((stat & DW_IC_INTR_RX_FULL) && (stat & DW_IC_INTR_STOP_DET))
- i2c_slave_event(dev->slave, I2C_SLAVE_WRITE_REQUESTED, &val);
+ if (stat & DW_IC_INTR_STOP_DET) {
+ if (dev->status != STATUS_IDLE) {
+ dev->status = STATUS_IDLE;
+ i2c_slave_event(dev->slave, I2C_SLAVE_STOP, &val);
+ }
+ }
return 1;
}
--
2.17.1
Powered by blists - more mailing lists