| 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: <20251017-i2c-dw-v1-3-7b85b71c7a87@bootlin.com>
Date: Fri, 17 Oct 2025 16:59:34 +0200
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 3/3] 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 doesn't 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
consequent write messages 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
To fix it, for the controllers that behave this way, if the next message
to the same slave device has the same direction as the previous one, it
is sent to the controller only after the previous message is sent and
STOP_DET IRQ flag is raised by the controller.
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 entries of Mobileye SoCs needing the work-around and
sort the entries alphabetically.
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.
In a PREEMPT-RT kernel, interrupt handlers are by default executed in
thread and may be interrupted, which can lead to breaking an I2C message
by inserting an unwanted STOP.
To ensure proper operation on realtime kernel, use IRQF_NO_THREAD flag
when requesting IRQ.
Based on the work of 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 | 45 +++++++++++++++++++++--------
drivers/i2c/busses/i2c-designware-platdrv.c | 6 ++--
3 files changed, 38 insertions(+), 14 deletions(-)
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index 347843b4f5dd7..a31a8698e511a 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 f9a180b145da8..e5af0439ec832 100644
--- a/drivers/i2c/busses/i2c-designware-master.c
+++ b/drivers/i2c/busses/i2c-designware-master.c
@@ -443,18 +443,6 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
for (; dev->msg_write_idx < dev->msgs_num; dev->msg_write_idx++) {
u32 flags = msgs[dev->msg_write_idx].flags;
- /*
- * If target address has changed, we need to
- * reprogram the target address in the I2C
- * adapter when we are done with this transfer.
- * This can be done after STOP_DET IRQ flag is raised.
- * So, disable "TX FIFO empty" interrupt.
- */
- if (msgs[dev->msg_write_idx].addr != addr) {
- intr_mask &= ~DW_IC_INTR_TX_EMPTY;
- break;
- }
-
if (!(dev->status & STATUS_WRITE_IN_PROGRESS)) {
/* new i2c_msg */
buf = msgs[dev->msg_write_idx].buf;
@@ -470,6 +458,25 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
need_restart = true;
}
+ /*
+ * If target address has changed, we need to
+ * reprogram the target address in the I2C
+ * adapter when we are done with this transfer.
+ * This can be done after STOP_DET IRQ flag is raised.
+ * So, disable "TX FIFO empty" interrupt.
+ * Also force a stop-then-start between two messages
+ * in the same direction if we need to restart on a
+ * adapter that does not handle restart.
+ */
+ if (msgs[dev->msg_write_idx].addr != addr ||
+ ((need_restart &&
+ dev->flags & NO_EMPTYFIFO_HOLD_MASTER &&
+ ((msgs[dev->msg_write_idx].flags & I2C_M_RD) ==
+ (msgs[dev->msg_write_idx - 1].flags & I2C_M_RD))))) {
+ intr_mask &= ~DW_IC_INTR_TX_EMPTY;
+ break;
+ }
+
regmap_read(dev->map, DW_IC_TXFLR, &flr);
tx_limit = dev->tx_fifo_depth - flr;
@@ -1062,6 +1069,20 @@ int i2c_dw_probe_master(struct dw_i2c_dev *dev)
irq_flags = IRQF_SHARED | IRQF_COND_SUSPEND;
}
+ /*
+ * The first writing to TX FIFO buffer causes transmission start. If
+ * IC_EMPTYFIFO_HOLD_MASTER_EN is not set, when TX FIFO gets empty, I2C
+ * controller finishes the transaction. If writing to FIFO is
+ * interrupted, FIFO can get empty and the transaction will be finished
+ * prematurely. FIFO buffer is filled in IRQ handler, but in PREEMPT_RT
+ * kernel IRQ handler by default is executed in thread that can be
+ * preempted with another higher priority thread or an interrupt. So,
+ * IRQF_NO_THREAD flag is required in order to prevent any preemption
+ * during filling the FIFO buffer and possible data lost.
+ */
+ if (dev->flags & NO_EMPTYFIFO_HOLD_MASTER)
+ irq_flags |= IRQF_NO_THREAD;
+
ret = i2c_dw_acquire_lock(dev);
if (ret)
return ret;
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 34d881572351c..5e459175dcdb2 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -345,9 +345,11 @@ static void dw_i2c_plat_remove(struct platform_device *pdev)
}
static const struct of_device_id dw_i2c_of_match[] = {
- { .compatible = "snps,designware-i2c", },
- { .compatible = "mscc,ocelot-i2c", .data = (void *)MODEL_MSCC_OCELOT },
{ .compatible = "baikal,bt1-sys-i2c", .data = (void *)MODEL_BAIKAL_BT1 },
+ { .compatible = "mobileye,eyeq6lplus-i2c", .data = (void *)NO_EMPTYFIFO_HOLD_MASTER },
+ { .compatible = "mobileye,eyeq7h-i2c", .data = (void *)NO_EMPTYFIFO_HOLD_MASTER },
+ { .compatible = "mscc,ocelot-i2c", .data = (void *)MODEL_MSCC_OCELOT },
+ { .compatible = "snps,designware-i2c", },
{}
};
MODULE_DEVICE_TABLE(of, dw_i2c_of_match);
--
2.51.0
Powered by blists - more mailing lists