| 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
| ||
|
Message-Id: <20251031-i2c-dw-v2-5-90416874fcc0@bootlin.com>
Date: Fri, 31 Oct 2025 15:35:43 +0100
From: Benoît Monin <benoit.monin@...tlin.com>
To: Andi Shyti <andi.shyti@...nel.org>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>,
Jarkko Nikula <jarkko.nikula@...ux.intel.com>,
Mika Westerberg <mika.westerberg@...ux.intel.com>,
Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
Jan Dabros <jsd@...ihalf.com>,
Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
Clark Williams <clrkwllms@...nel.org>, Steven Rostedt <rostedt@...dmis.org>
Cc: Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
Gregory CLEMENT <gregory.clement@...tlin.com>,
Théo Lebrun <theo.lebrun@...tlin.com>,
Tawfik Bayouk <tawfik.bayouk@...ileye.com>,
Vladimir Kondratiev <vladimir.kondratiev@...ileye.com>,
Dmitry Guzman <dmitry.guzman@...ileye.com>, linux-i2c@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-rt-devel@...ts.linux.dev,
Benoît Monin <benoit.monin@...tlin.com>
Subject: [PATCH v2 5/5] i2c: designware: Support of controller with
IC_EMPTYFIFO_HOLD_MASTER disabled
If IC_EMPTYFIFO_HOLD_MASTER_EN parameter is 0, "Stop" and "Repeated Start"
bits in command register does not exist, thus it is impossible to send
several consecutive write messages in a single hardware batch. The
existing implementation worked with such configuration incorrectly:
all consecutive write messages are joined into a single message without
any Start/Stop or Repeated Start conditions. For example, the following
command:
i2ctransfer -y 0 w1@...5 0x00 w1@...5 0x01
does the same as
i2ctransfer -y 0 w2@...5 0x00 0x01
In i2c_dw_xfer(), we ensure that we do not have such sequence of messages
requiring a RESTART, aborting the transfer on controller that cannot
emit them explicitly.
This behavior is activated by compatible entries because the state of
the IC_EMPTYFIFO_HOLD_MASTER_EN parameter cannot be detected at runtime.
Add the compatible entry for Mobileye SoCs needing the workaround.
There is another possible problem with this controller configuration:
When the CPU is putting commands to the FIFO, this process must not be
interrupted because if FIFO buffer gets empty, the controller finishes
the I2C transaction and generates STOP condition on the bus.
If we continue writing the remainder of the message to the FIFO, the
controller will start emitting a new transaction with those data. This
turns a single a single message into multiple I2C transactions. To
ensure that we do not keep processing a message after a FIFO underrun,
checks are added in two places.
First in i2c_dw_xfer_msg() we check the raw interrupt status register to
see if a STOP condition was detected while filling the FIFO, and abort
if so. This can happen with threaded interrupt on a PREEMPT_RT kernel
if we are preempted during the processing of each bytes of the message.
Second in i2c_dw_process_transfer(), we abort if a STOP is detected
while a read or a write is in progress. This can occur when processing
a message larger than the FIFO. In that case the message is processed in
parts, and rely on the TW EMPTY interrupt to refill the FIFO when it gets
below a threshold. If servicing this interrupt is delayed for too long,
it can trigger a FIFO underrun, thus an unwanted STOP.
Originally-by: Dmitry Guzman <dmitry.guzman@...ileye.com>
Signed-off-by: Benoît Monin <benoit.monin@...tlin.com>
---
drivers/i2c/busses/i2c-designware-core.h | 1 +
drivers/i2c/busses/i2c-designware-master.c | 51 +++++++++++++++++++++++++++++
drivers/i2c/busses/i2c-designware-platdrv.c | 1 +
3 files changed, 53 insertions(+)
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index 347843b4f5dd..a31a8698e511 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -311,6 +311,7 @@ struct dw_i2c_dev {
#define ACCESS_NO_IRQ_SUSPEND BIT(1)
#define ARBITRATION_SEMAPHORE BIT(2)
#define ACCESS_POLLING BIT(3)
+#define NO_EMPTYFIFO_HOLD_MASTER BIT(4)
#define MODEL_MSCC_OCELOT BIT(8)
#define MODEL_BAIKAL_BT1 BIT(9)
diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c
index da1963d25def..329bb69485f4 100644
--- a/drivers/i2c/busses/i2c-designware-master.c
+++ b/drivers/i2c/busses/i2c-designware-master.c
@@ -463,6 +463,7 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
rx_limit = dev->rx_fifo_depth - flr;
while (buf_len > 0 && tx_limit > 0 && rx_limit > 0) {
+ unsigned int raw_stat;
u32 cmd = 0;
/*
@@ -487,6 +488,21 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
need_restart = false;
}
+ /*
+ * With threaded interrupt on a PREEMPT-RT kernel, we may
+ * be interrupted while filling the FIFO. Abort the
+ * transfer in case of a FIFO underrun on controller that
+ * emits a STOP in that case.
+ */
+ if (dev->flags & NO_EMPTYFIFO_HOLD_MASTER) {
+ regmap_read(dev->map, DW_IC_RAW_INTR_STAT,
+ &raw_stat);
+ if (raw_stat & DW_IC_INTR_STOP_DET) {
+ dev->msg_err = -EIO;
+ goto done;
+ }
+ }
+
if (msgs[dev->msg_write_idx].flags & I2C_M_RD) {
/* Avoid rx buffer overrun */
@@ -526,6 +542,7 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
dev->status &= ~STATUS_WRITE_IN_PROGRESS;
}
+done:
/*
* If i2c_msg index search is completed, we don't need TX_EMPTY
* interrupt any more.
@@ -706,6 +723,14 @@ static void i2c_dw_process_transfer(struct dw_i2c_dev *dev, unsigned int stat)
if (stat & DW_IC_INTR_TX_EMPTY)
i2c_dw_xfer_msg(dev);
+ /* Abort if we detect a STOP in the middle of a read or a write */
+ if ((stat & DW_IC_INTR_STOP_DET) &&
+ (dev->status & (STATUS_READ_IN_PROGRESS | STATUS_WRITE_IN_PROGRESS))) {
+ dev_err(dev->dev, "spurious STOP detected\n");
+ dev->rx_outstanding = 0;
+ dev->msg_err = -EIO;
+ }
+
/*
* No need to modify or disable the interrupt mask here.
* i2c_dw_xfer_msg() will take care of it according to
@@ -872,6 +897,21 @@ __i2c_dw_xfer_unlocked(struct dw_i2c_dev *dev, struct i2c_msg msgs[], int num)
return ret;
}
+/*
+ * Return true if the message needs an explicit RESTART before being sent.
+ * Without an explicit RESTART, two consecutive messages in the same direction
+ * will be merged into a single transfer.
+ * The adapter always emits a RESTART when the direction changes.
+ */
+static inline bool i2c_dw_msg_need_restart(struct i2c_msg msgs[], int idx)
+{
+ /* No need for a RESTART on the first message */
+ if (idx == 0)
+ return false;
+
+ return (msgs[idx - 1].flags & I2C_M_RD) == (msgs[idx].flags & I2C_M_RD);
+}
+
static int
i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
{
@@ -918,6 +958,17 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
goto done;
}
+ /*
+ * Make sure we don't need explicit RESTART for
+ * controllers that cannot emit them.
+ */
+ if (dev->flags & NO_EMPTYFIFO_HOLD_MASTER &&
+ i2c_dw_msg_need_restart(msg, cnt - 1)) {
+ dev_err(dev->dev, "cannot emit RESTART\n");
+ ret = -EINVAL;
+ goto done;
+ }
+
if ((msg[cnt - 1].flags & I2C_M_STOP) ||
(msg + cnt == msgs + num))
break;
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index d7d764f7554d..4aad3dc51fbc 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -346,6 +346,7 @@ static void dw_i2c_plat_remove(struct platform_device *pdev)
static const struct of_device_id dw_i2c_of_match[] = {
{ .compatible = "baikal,bt1-sys-i2c", .data = (void *)MODEL_BAIKAL_BT1 },
+ { .compatible = "mobileye,eyeq6lplus-i2c", .data = (void *)NO_EMPTYFIFO_HOLD_MASTER },
{ .compatible = "mscc,ocelot-i2c", .data = (void *)MODEL_MSCC_OCELOT },
{ .compatible = "snps,designware-i2c" },
{}
--
2.51.1
Powered by blists - more mailing lists