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

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

Hi Francesco,

...

> +static int ft4222_i2c_write(struct ft4222_i2c *ftdi, u8 flags, u8 slave_addr,
> +			    u8 *data, int len)
> +{
> +	struct usb_device *udev = ftdi->udev;
> +	unsigned int pipe = usb_sndbulkpipe(udev, FT4222_EP_TX);
> +	u8 *buf = ftdi->ubuf;
> +	int written = 0;
> +
> +	dev_dbg(&ftdi->adapter.dev, "write to 0x%02x, flags 0x%02x, len %d",
> +		slave_addr, flags, len);

Here and in every dev_dbg/dev_err, you are missing the "\n" at
the end of the message.

> +	buf[0] = slave_addr << 1;
> +	buf[2] = buf[3] = 0;
> +	do {
> +		int pkt_len = min(FT4222_TX_HDRLEN + len - written,
> +				  FT4222_BULK_MAXLEN);
> +		bool first_pkt = (written == 0);
> +		bool last_pkt = (written + pkt_len == FT4222_TX_HDRLEN + len);
> +		int ret, actual_len;
> +
> +		buf[1] = 0;
> +		if (first_pkt)
> +			buf[1] |= flags & FT4222_FLAG_RESTART;

Aren't we losing the _START case here?

Andi

> +		if (last_pkt)
> +			buf[1] |= flags & FT4222_FLAG_STOP;
> +		if (buf[1] == 0)
> +			buf[1] = FT4222_FLAG_NONE;
> +		memcpy(buf + FT4222_TX_HDRLEN, data + written,
> +		       pkt_len - FT4222_TX_HDRLEN);
> +		ret = usb_bulk_msg(udev, pipe, buf, pkt_len, &actual_len,
> +				   FT4222_IO_TIMEOUT);
> +		if (ret < 0)
> +			return ret;
> +		if (actual_len < pkt_len)
> +			return -EIO;
> +		ret = ft4222_i2c_get_status(ftdi);
> +		if (ret < 0)
> +			return ret;
> +		written += pkt_len - FT4222_TX_HDRLEN;
> +	} while (written < len);
> +	return 0;
> +}