ACK/Cmnt: [PATCH 1/1][SRU][Focal] UBUNTU: SAUCE: rtc: add am-1805 RTC driver

[Andrea Righi]

On Mon, May 04, 2020 at 05:53:59PM +0800, AceLan Kao wrote:
...
> +static int am1805_write(struct am1805_data *amq, u8 *buf, int len)
> +{
> +	int err = 0, i;
> +	u8 base_reg;
> +	struct i2c_msg msgs[] = {
> +		{
> +		 .addr = amq->client->addr,
> +			.flags = (amq->client->flags & I2C_M_TEN),
> +		 .len = len + 1,
> +		 .buf = buf,
> +		 },
> +	};
> +
> +	if (amq->use_smbus) {
> +		/* use SMBUS transfer protocol */
> +		base_reg = buf[0];
> +		for (i = 1; i <= len; i++) {
> +			err = i2c_smbus_write_byte_data(amq->client, base_reg,
> +							buf[i]);

It is not very elegant to pass buf and len and then read up to buf[len]
(while the am1805_read() counterpart is always accessing
buf[0 ..  len-1]), it really looks like a bug at first sight and it
took me a while to check if the callers are doing the proper job without
accessing uninitialized data, but at the end it looks correct.

Apart this little inelegance, looks good to me. :)

Acked-by: Andrea Righi <andrea.righi at canonical.com>


> +			if (err < 0) {
> +				dev_err(&amq->client->dev,
> +					"write transfer error reg 0x%02X\n",
> +					base_reg);
> +				break;
> +			}
> +			base_reg++;
> +		}
> +	} else {
> +		/* use I2C transfer protocol */
> +		err = i2c_transfer(amq->client->adapter, msgs, 1);
> +		if (err != 1) {
> +			dev_err(&amq->client->dev, "I2C write transfer error\n");
> +			err = -EIO;
> +		} else {
> +			err = 0;
> +		}
> +	}
> +	return err;
> +}