On Wed, 20 Jun 2007 00:28:28 +0200, Alessandro Zummo <[EMAIL PROTECTED]> wrote:
> I'd prefer if you change the name in rtc-m41t80 and add
> the names of the supported chips in the Kconfig entry.
OK, I will do. I thought M41ST94 also can be supported but that was
wrong. From datasheets, the driver can be used for M41T8[0123],
M41ST8[457] chips.
> > + unsigned char wbuf[1 + M41TXX_REG_YEAR + 1];
>
> would be easier to understand if you have some #defines
> with the sizes you require for the various buffers.
I will do.
> the proc interface will go away sooner or later, would be better to expose
> the battery status with a sysfs attribute.
I will do.
> > +++ b/include/linux/m41txx.h
> > @@ -0,0 +1,40 @@
> > +/*
> > + * Definitions for the ST M41TXX family of i2c rtc chips.
> > + *
> > + * Based on m41t00.h by Mark A. Greer <[EMAIL PROTECTED]>
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
>
> Unless this include is required by other files, please place
> it in drivers/rtc calling it rtc-drivername.h
It is required by platform code to provide SQW setting. Also
M41TXX_DRV_NAME and M41TXX_I2C_ADDR might be useful to declare
i2c_board_info, which would be something like:
static struct m41txx_platform_data pdata = {
.sqw_freq = M41TXX_SQW_32KHZ,
};
static struct i2c_board_info binfo __initdata = {
I2C_BOARD_INFO(M41TXX_DRV_NAME, M41TXX_I2C_ADDR),
.type = "m41t80",
.platform_data = &pdata,
};
return i2c_register_board_info(0, &binfo, 1);
> Other than the comments above, the driver
> seems fine to me.
Thank you for review.
---
Atsushi Nemoto
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
