On Wed, 20 Jun 2007 00:59:26 +0900 (JST)
Atsushi Nemoto <[EMAIL PROTECTED]> wrote:
>
> This driver supports M41T80, M41T81 and M41ST85. The old m41t00
> driver supports M41T00, M41T81 and M41T85(M41ST85). While the M41T00
> chip is now supported by rtc-ds1307 driver, this driver does not
> include support for the chip.
Hi,
I'd prefer if you change the name in rtc-m41t80 and add
the names of the supported chips in the Kconfig entry.
> +/* Sets the given date and time to the real time clock. */
> +static int m41txx_set_datetime(struct i2c_client *client, struct rtc_time
> *tm)
> +{
> + 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.
> +#if defined(CONFIG_RTC_INTF_PROC) || defined(CONFIG_RTC_INTF_PROC_MODULE)
> +static int m41txx_rtc_proc(struct device *dev, struct seq_file *seq)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + struct m41txx_data *clientdata = i2c_get_clientdata(client);
> + unsigned char reg;
> +
> + if (clientdata->chip->features & M41TXX_FEATURE_BL) {
> + reg = i2c_smbus_read_byte_data(client, M41TXX_REG_FLAGS);
> + seq_printf(seq, "battery\t\t: %s\n",
> + (reg & M41TXX_FLAGS_BATT_LOW) ? "exhausted" : "ok");
> + }
> + return 0;
> +}
the proc interface will go away sooner or later, would be better to expose
the battery status with a sysfs attribute.
> +++ 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
Other than the comments above, the driver
seems fine to me.
--
Best regards,
Alessandro Zummo,
Tower Technologies - Torino, Italy
http://www.towertech.it
-
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/
