linux-kernel - Re: [PATCH v2] i2c: Add FTDI FT4222H USB I2C adapter

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <aXH1fEkQWnekwSui@zenone.zhora.eu>
Date: Thu, 22 Jan 2026 11:37:17 +0100
From: Andi Shyti <andi.shyti@...nel.org>
To: Francesco Lavra <flavra@...libre.com>
Cc: linux-kernel@...r.kernel.org, linux-i2c@...r.kernel.org
Subject: Re: [PATCH v2] i2c: Add FTDI FT4222H USB I2C adapter

Hi Francesco,

...

> +static int ft4222_i2c_get_status(struct ft4222_i2c *ftdi)
> +{
> +	/*
> +	 * Multiple retries are needed mostly when retrieving the controller
> +	 * status after doing a write with the I2C bus operating at a low speed.
> +	 * Empirical tests conducted at 100 kHz showed that after a
> +	 * maximum-sized (512-byte) write, up to 11 retries are needed before
> +	 * the chip clears its CTRL_BUSY flag. But under certain conditions more
> +	 * retries may be needed: for example, when trying to do a write after
> +	 * disconnecting the SCL line from the I2C slave, tests showed that up
> +	 * to 64 retries are needed.
> +	 */
> +	const int max_retries = 70;
> +	int retry;
> +	u8 status;
> +
> +	for (retry = 0; retry < max_retries; retry++) {
> +		int ret = ft4222_cmd_get(ftdi, FT4222_CMD_I2C_STATUS, &status);
> +
> +		if (ret < 0)
> +			return ret;
> +		if (!(status & FT4222_I2C_CTRL_BUSY))
> +			break;

How about adding a small delay here. Does it also help to reduce
the maximum number of retries?

> +	}

...

> +	dev_dbg(&adapter->dev, "transfer with %d message(s)", num);
> +	for (i = 0; i < num; ++i) {
> +		const u8 addr = msg[i].addr;
> +		const u16 len = msg[i].len;
> +		u8 *buf = msg[i].buf;
> +		u8 flags;
> +
> +		flags = ((i == 0) ? FT4222_FLAG_START : FT4222_FLAG_RESTART);
> +		if (i == num - 1)
> +			flags |= FT4222_FLAG_STOP;

Are we setting here both STOP and RESTART at the same time.

The rest looks good to me.

Andi

> +		if (msg[i].flags & I2C_M_RD)