| 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:
<VI2PR04MB11147E62979D837F193A345D1E8D5A@VI2PR04MB11147.eurprd04.prod.outlook.com>
Date: Fri, 21 Nov 2025 02:14:11 +0000
From: Carlos Song <carlos.song@....com>
To: Frank Li <frank.li@....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
> -----Original Message-----
> From: Frank Li <frank.li@....com>
> Sent: Friday, November 21, 2025 12:17 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 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?
Yes, it is.
>
> > > > + * 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
>
Hi, Frank
Yes. Your are right. That is normal case. Timeout may happen when read smbus len when system in high stress.
That is not avoidable.
> > > > +
> > > > + /* 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