On Sun, 30 Sep 2007 17:54:54 +0800, Mark Zhan <rongkai.zhan@xxxxxxxxxxxxx> wrote:
> This patch makes m41t80 RTC driver also can work with the SMBus adapters,
> which doesn't i2c_transfer() method.
>
> Signed-off-by: Mark Zhan <rongkai.zhan@xxxxxxxxxxxxx>
As Jean already said, your mailer corrupted your patch.
Also, please keep in mind the 80 column rule.
> +static int m41t80_i2c_read(struct i2c_client *client, struct i2c_msg *msgs, int num)
> +{
> + int i, rc;
> + u8 dt_addr = msgs[0].buf[0];
> +
> + if (num < 2)
> + return -1;
> +
> + if (i2c_check_functionality(client->adapter, I2C_FUNC_I2C) &&
> + i2c_transfer(client->adapter, msgs, 2) < 0) {
> + dev_err(&client->dev, "read error\n");
> + return -EIO;
> + } else {
> + for (i = 0; i < msgs[1].len; i++) {
> + rc = i2c_smbus_read_byte_data(client, dt_addr + i);
> + if (rc < 0)
> + return -EIO;
> + msgs[1].buf[i] = (u8)rc;
> + }
> + }
> +
> + return 0;
> +}
You must ensure time values are consistent. The RTC might update its
time data between each I2C transfer.
It seems the original rtc_m41t81.c:m41t81_get_time() tries to solve
this issue by reading sec/min in loop, but it would not be enough.
For example, if the function was called at 23:59:59, the sec (and min)
might wrap just before reading the hour, then you might get 00:59:59.
The old m41t00 i2c driver once did it right with smbus, but current
m41t00 driver (and rtc-m41t80 driver) dropped that feature a while
ago. You can see the old code at:
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=drivers/i2c/chips/m41t00.c;hb=e931b8d8a428f87e6ea488d2fd80007bb66b3ea8
> +static int m41t80_i2c_write(struct i2c_client *client, struct i2c_msg *msg)
> +{
> + int i, rc;
> + u8 *wbuf = msg->buf;
> + u8 *buf = wbuf + 1;
> +
> + if (i2c_check_functionality(client->adapter, I2C_FUNC_I2C) &&
> + i2c_transfer(client->adapter, msg, 1) < 0) {
> + dev_err(&client->dev, "write error\n");
> + return -EIO;
> + } else {
> + for (i = 0; i < msg[0].len - 1; i++) {
> + rc = i2c_smbus_write_byte_data(client, wbuf[0]+i, buf[i]);
> + if (rc < 0)
> + return -EIO;
> + }
> + }
> +
> + return 0;
> +}
Writing to the RTC by multiple I2C transfers might have same
consistency problem. But it would not be a real problem if you wrote
the sec register first. Here is a comment in
rtc_m41t81.c:m41t81_set_time():
/*
* Note the write order matters as it ensures the correctness.
* When we write sec, 10th sec is clear. It is reasonable to
* believe we should finish writing min within a second.
*/
I think this comment is worth to import.
Other parts looks good for me.
---
Atsushi Nemoto
