| 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: <aR8/Bt77UWb9G6oI@lizhi-Precision-Tower-5810>
Date: Thu, 20 Nov 2025 11:17:10 -0500
From: Frank Li <Frank.li@....com>
To: Carlos Song <carlos.song@....com>
Cc: Aisheng Dong <aisheng.dong@....com>,
"andi.shyti@...nel.org" <andi.shyti@...nel.org>,
"shawnguo@...nel.org" <shawnguo@...nel.org>,
"s.hauer@...gutronix.de" <s.hauer@...gutronix.de>,
"kernel@...gutronix.de" <kernel@...gutronix.de>,
"festevam@...il.com" <festevam@...il.com>,
"pandy.gao@....com" <pandy.gao@....com>,
"wsa@...nel.org" <wsa@...nel.org>, "vz@...ia.com" <vz@...ia.com>,
"B38611@...escale.com" <B38611@...escale.com>,
"linux-i2c@...r.kernel.org" <linux-i2c@...r.kernel.org>,
"imx@...ts.linux.dev" <imx@...ts.linux.dev>,
"linux-arm-kernel@...ts.infradead.org" <linux-arm-kernel@...ts.infradead.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] i2c: imx-lpi2c: support smbus block read feature
On Thu, Nov 20, 2025 at 03:02:26AM +0000, Carlos Song wrote:
>
>
> > -----Original Message-----
> > From: Frank Li <frank.li@....com>
> > Sent: Thursday, November 20, 2025 12:44 AM
> > To: Carlos Song <carlos.song@....com>
> > Cc: Aisheng Dong <aisheng.dong@....com>; andi.shyti@...nel.org;
> > shawnguo@...nel.org; s.hauer@...gutronix.de; kernel@...gutronix.de;
> > festevam@...il.com; pandy.gao@....com; wsa@...nel.org; vz@...ia.com;
> > B38611@...escale.com; linux-i2c@...r.kernel.org; imx@...ts.linux.dev;
> > linux-arm-kernel@...ts.infradead.org; linux-kernel@...r.kernel.org
> > Subject: Re: [PATCH] i2c: imx-lpi2c: support smbus block read feature
> >
> > On Tue, Nov 18, 2025 at 07:13:23PM +0800, Carlos Song wrote:
> > > The LPI2C controller automatically transmits a NACK on the last byte
> > > of a receive data command. It transmits the NACK unless the next
> > > command in the TXFIFO is also a receive data command. If the transmit
> > > FIFO is empty when a receive data command completes, a NACK is also
> > > automatically transmitted.
> > >
> > > Specially set read 2 bytes command initially. When the RXFIFO receives
> > > count data, controller should immediately read out this value and
> > > update MTDR register to keep the TXFIFO not empty. Set new receive
> > > data command to read other data before the 2nd byte is returned.
> > >
> > > Fixes: a55fa9d0e42e ("i2c: imx-lpi2c: add low power i2c bus driver")
> > > Signed-off-by: Carlos Song <carlos.song@....com>
> > >
> > > ---
> > > I setup an env to test this feature change:
> > >
> > > On I.MX93-EVK:
> > > LPI2C5 as slave device and use i2ctool to redesign the slave-eeprom mem:
> > >
> > > hexdump -C /sys/bus/i2c/devices/5-1064/slave-eeprom
> > >
...
> > > - 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 transmits a NACK on the last
> > byte
> > > + * of a receive data command.
> >
> > looks like transfer STOP? You get data, it should be ACK when received data.
> >
>
> According to LPI2C RM MTDR register shows:
> When controller need to read data, we should send command word first.
> Bit10| bit9 | bit8 | bit7~0
> Receive (DATA[7:0] + 1) bytes byte counter
>
> DATA[7:0] is used as a byte counter. Receive that many bytes and check each for a data match (if configured) before storing the received data in the receive FIFO.
> We can prefill the bytes count to controller, controller will auto ACK after every bytes until count is exhausted. Then controller send auto NACK.
>
Thank your for your explain. This may IC design choice or bug, look like
I2C controller should stall SCL to wait for new command?
Assume byte counter is 0, (1 bytes transfer) [0x100]
SCL: 1, 2, 3, 4, 5, 6, 7, 8, 9,
SDA: B0, B1, B2, B3, B4, B5, B6, B7, ACK.
^ Do you means here is NACK when
MTDR empty?
> > > + * command in the TXFIFO is also a receive data command. If the
> > > + /* 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;
> > > + }
If irq/schedule happen here, you may not fill MTDR in time, so MTDR maybe
empty, You just have 9 SCL time to fill new data to MTDR.
Frank
> > > +
> > > + /* 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 +648,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