| 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: <20251121110100.1909603-1-carlos.song@nxp.com>
Date: Fri, 21 Nov 2025 19:01:00 +0800
From: Carlos Song <carlos.song@....com>
To: frank.li@....com,
aisheng.dong@....com,
andi.shyti@...nel.org,
shawnguo@...nel.org,
s.hauer@...gutronix.de,
kernel@...gutronix.de,
festevam@...il.com,
vz@...ia.com,
wsa@...nel.org
Cc: linux-i2c@...r.kernel.org,
imx@...ts.linux.dev,
linux-arm-kernel@...ts.infradead.org,
linux-kernel@...r.kernel.org,
Carlos Song <carlos.song@....com>
Subject: [PATCH v2] i2c: imx-lpi2c: support smbus block read feature
The LPI2C controller automatically sends a NACK after the last byte of a
receive command unless the next command in MTDR is also a receive command.
If MTDR is empty when a receive completes, NACK is transmitted by default.
To comply with SMBus block read, start with a 2-byte read:
- The first byte is the block length. Once received, update MTDR
immediately to keep MTDR non-empty.
- Issue a new receive command for the remaining data before the second
byte arrives ensuring continuous ACK instead of NACK.
Fixes: a55fa9d0e42e ("i2c: imx-lpi2c: add low power i2c bus driver")
Signed-off-by: Carlos Song <carlos.song@....com>
---
Changes since v1:
* According to discussion with Frank, improve comment and commit log by AI.
This comment and commit log looks more clear.
---
drivers/i2c/busses/i2c-imx-lpi2c.c | 94 +++++++++++++++++++++++-------
1 file changed, 74 insertions(+), 20 deletions(-)
diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c b/drivers/i2c/busses/i2c-imx-lpi2c.c
index d882126c1778..dfacb0aec3c0 100644
--- a/drivers/i2c/busses/i2c-imx-lpi2c.c
+++ b/drivers/i2c/busses/i2c-imx-lpi2c.c
@@ -90,6 +90,7 @@
#define MRDR_RXEMPTY BIT(14)
#define MDER_TDDE BIT(0)
#define MDER_RDDE BIT(1)
+#define MSR_RDF_ASSERT(x) FIELD_GET(MSR_RDF, (x))
#define SCR_SEN BIT(0)
#define SCR_RST BIT(1)
@@ -461,7 +462,7 @@ static bool lpi2c_imx_write_txfifo(struct lpi2c_imx_struct *lpi2c_imx, bool atom
static bool lpi2c_imx_read_rxfifo(struct lpi2c_imx_struct *lpi2c_imx, bool atomic)
{
- unsigned int blocklen, remaining;
+ unsigned int remaining;
unsigned int temp, data;
do {
@@ -472,15 +473,6 @@ static bool lpi2c_imx_read_rxfifo(struct lpi2c_imx_struct *lpi2c_imx, bool atomi
lpi2c_imx->rx_buf[lpi2c_imx->delivered++] = data & 0xff;
} while (1);
- /*
- * First byte is the length of remaining packet in the SMBus block
- * data read. Add it to msgs->len.
- */
- if (lpi2c_imx->block_data) {
- blocklen = lpi2c_imx->rx_buf[0];
- lpi2c_imx->msglen += blocklen;
- }
-
remaining = lpi2c_imx->msglen - lpi2c_imx->delivered;
if (!remaining) {
@@ -493,12 +485,7 @@ static bool lpi2c_imx_read_rxfifo(struct lpi2c_imx_struct *lpi2c_imx, bool atomi
lpi2c_imx_set_rx_watermark(lpi2c_imx);
/* multiple receive commands */
- if (lpi2c_imx->block_data) {
- lpi2c_imx->block_data = 0;
- temp = remaining;
- temp |= (RECV_DATA << 8);
- writel(temp, lpi2c_imx->base + LPI2C_MTDR);
- } else if (!(lpi2c_imx->delivered & 0xff)) {
+ if (!(lpi2c_imx->delivered & 0xff)) {
temp = (remaining > CHUNK_DATA ? CHUNK_DATA : remaining) - 1;
temp |= (RECV_DATA << 8);
writel(temp, lpi2c_imx->base + LPI2C_MTDR);
@@ -536,18 +523,81 @@ static int lpi2c_imx_write_atomic(struct lpi2c_imx_struct *lpi2c_imx,
return err;
}
+static unsigned int lpi2c_SMBus_block_read_single_byte(struct lpi2c_imx_struct *lpi2c_imx)
+{
+ unsigned int val, data;
+ int ret;
+
+ ret = readl_poll_timeout(lpi2c_imx->base + LPI2C_MSR, val,
+ MSR_RDF_ASSERT(val), 1, 1000);
+ if (ret) {
+ dev_err(&lpi2c_imx->adapter.dev, "SMBus read count timeout %d\n", ret);
+ return ret;
+ }
+
+ data = readl(lpi2c_imx->base + LPI2C_MRDR);
+ lpi2c_imx->rx_buf[lpi2c_imx->delivered++] = data & 0xff;
+
+ return data;
+}
+
static void lpi2c_imx_read_init(struct lpi2c_imx_struct *lpi2c_imx,
struct i2c_msg *msgs)
{
- unsigned int temp;
+ unsigned int temp, ret, block_len;
lpi2c_imx->rx_buf = msgs->buf;
lpi2c_imx->block_data = msgs->flags & I2C_M_RECV_LEN;
lpi2c_imx_set_rx_watermark(lpi2c_imx);
- temp = msgs->len > CHUNK_DATA ? CHUNK_DATA - 1 : msgs->len - 1;
- temp |= (RECV_DATA << 8);
- writel(temp, lpi2c_imx->base + LPI2C_MTDR);
+
+ if (!lpi2c_imx->block_data) {
+ temp = msgs->len > CHUNK_DATA ? CHUNK_DATA - 1 : msgs->len - 1;
+ temp |= (RECV_DATA << 8);
+ writel(temp, lpi2c_imx->base + LPI2C_MTDR);
+ } else {
+ /*
+ * The LPI2C controller automatically sends a NACK after the last byte of a
+ * receive command, unless the next command in MTDR is also a receive command.
+ * If MTDR is empty when a receive completes, a NACK is sent by default.
+ *
+ * To comply with the SMBus block read spec, we start with a 2-byte read:
+ * The first byte in RXFIFO is the block length. Once this byte arrives, the
+ * controller immediately updates MTDR with the next read command, ensuring
+ * continuous ACK instead of NACK.
+ *
+ * The second byte is the first block data byte. Therefore, the subsequent
+ * read command should request (block_len - 1) bytes, since one data byte
+ * has already been read.
+ */
+
+ writel((RECV_DATA << 8) | 0x01, lpi2c_imx->base + LPI2C_MTDR);
+
+ /* Read the first byte as block len */
+ block_len = lpi2c_SMBus_block_read_single_byte(lpi2c_imx);
+ if (block_len < 0) {
+ dev_err(&lpi2c_imx->adapter.dev, "SMBus read data length timeout\n");
+ return;
+ }
+
+ /* Confirm SMBus transfer meets protocol */
+ if (block_len == 0 || block_len > I2C_SMBUS_BLOCK_MAX) {
+ dev_err(&lpi2c_imx->adapter.dev, "Invalid SMBus block read length\n");
+ return;
+ }
+
+ /* If just read 1 byte then read out from fifo. No need new command update */
+ if (block_len == 1) {
+ ret = lpi2c_SMBus_block_read_single_byte(lpi2c_imx);
+ if (ret < 0)
+ dev_err(&lpi2c_imx->adapter.dev, "SMBus read data timeout\n");
+ return;
+ }
+
+ /* Block read other length data need to update command again*/
+ writel((RECV_DATA << 8) | (block_len - 2), lpi2c_imx->base + LPI2C_MTDR);
+ lpi2c_imx->msglen += block_len;
+ }
}
static bool lpi2c_imx_read_chunk_atomic(struct lpi2c_imx_struct *lpi2c_imx)
@@ -599,6 +649,10 @@ static bool is_use_dma(struct lpi2c_imx_struct *lpi2c_imx, struct i2c_msg *msg)
if (pm_suspend_in_progress())
return false;
+ /* DMA is not suitable for SMBus block read */
+ if (msg->flags & I2C_M_RECV_LEN)
+ return false;
+
/*
* When the length of data is less than I2C_DMA_THRESHOLD,
* cpu mode is used directly to avoid low performance.
--
2.34.1
Powered by blists - more mailing lists