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

[Andi Shyti <andi.shyti@kernel.org>]

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)