Re: [rtc-linux] [PATCH 1/2] rtc: add rtc-m41txx driver

2007-06-19 Thread Atsushi Nemoto
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/


Re: [rtc-linux] [PATCH 1/2] rtc: add rtc-m41txx driver

2007-06-19 Thread Alessandro Zummo
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/


Re: [PATCH 1/2] rtc: add rtc-m41txx driver

2007-06-19 Thread Mark A. Greer
On Wed, Jun 20, 2007 at 12:59:26AM +0900, Atsushi Nemoto wrote:
> This is a new-style i2c driver for ST M41TXX RTC chip, derived from
> works by Alexander Bigga <[EMAIL PROTECTED]> who wrote the original
> rtc-m41txx.c based on drivers/i2c/chips/m41t00.c driver.
> 
> 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.
> 
> Signed-off-by: Atsushi Nemoto <[EMAIL PROTECTED]>
> Signed-off-by: Alexander Bigga <[EMAIL PROTECTED]>

Other than addressing David's question about alarm support,

Acked-by: Mark A. Greer <[EMAIL PROTECTED]>
-
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/


[PATCH 1/2] rtc: add rtc-m41txx driver

2007-06-19 Thread Atsushi Nemoto
This is a new-style i2c driver for ST M41TXX RTC chip, derived from
works by Alexander Bigga <[EMAIL PROTECTED]> who wrote the original
rtc-m41txx.c based on drivers/i2c/chips/m41t00.c driver.

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.

Signed-off-by: Atsushi Nemoto <[EMAIL PROTECTED]>
Signed-off-by: Alexander Bigga <[EMAIL PROTECTED]>
---
 drivers/rtc/Kconfig  |   11 ++
 drivers/rtc/Makefile |1 +
 drivers/rtc/rtc-m41txx.c |  362 ++
 include/linux/m41txx.h   |   40 +
 4 files changed, 414 insertions(+), 0 deletions(-)

diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 4e4c10a..c52f3ff 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -282,6 +282,17 @@ config RTC_DRV_DS1742
  This driver can also be built as a module. If so, the module
  will be called rtc-ds1742.
 
+config RTC_DRV_M41TXX
+   tristate "ST M41Txx series RTC"
+   depends on RTC_CLASS && I2C
+   help
+ If you say Y here you will get support for the
+ ST M41TXX RTC chips series. Currently following chips are
+ supported: M41T80, M41T81 and M41ST85
+
+ This driver can also be built as a module. If so, the module
+ will be called rtc-m41txx.
+
 config RTC_DRV_M48T86
tristate "ST M48T86/Dallas DS12887"
depends on RTC_CLASS
diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
index a1afbc2..5ef1123 100644
--- a/drivers/rtc/Makefile
+++ b/drivers/rtc/Makefile
@@ -28,6 +28,7 @@ obj-$(CONFIG_RTC_DRV_PCF8583) += rtc-pcf8583.o
 obj-$(CONFIG_RTC_DRV_RS5C372)  += rtc-rs5c372.o
 obj-$(CONFIG_RTC_DRV_S3C)  += rtc-s3c.o
 obj-$(CONFIG_RTC_DRV_RS5C348)  += rtc-rs5c348.o
+obj-$(CONFIG_RTC_DRV_M41TXX)   += rtc-m41txx.o
 obj-$(CONFIG_RTC_DRV_M48T86)   += rtc-m48t86.o
 obj-$(CONFIG_RTC_DRV_DS1553)   += rtc-ds1553.o
 obj-$(CONFIG_RTC_DRV_RS5C313)  += rtc-rs5c313.o
diff --git a/drivers/rtc/rtc-m41txx.c b/drivers/rtc/rtc-m41txx.c
new file mode 100644
index 000..7f05252
--- /dev/null
+++ b/drivers/rtc/rtc-m41txx.c
@@ -0,0 +1,362 @@
+/*
+ * I2C client/driver for the ST M41TXX family of i2c rtc chips.
+ *
+ * Author: Alexander Bigga <[EMAIL PROTECTED]>
+ *
+ * Based on m41t00.c by Mark A. Greer <[EMAIL PROTECTED]>
+ *
+ * 2006 (c) mycable GmbH
+ *
+ * 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.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define M41TXX_REG_SSEC0
+#define M41TXX_REG_SEC 1
+#define M41TXX_REG_MIN 2
+#define M41TXX_REG_HOUR3
+#define M41TXX_REG_WDAY4
+#define M41TXX_REG_DAY 5
+#define M41TXX_REG_MON 6
+#define M41TXX_REG_YEAR7
+#define M41TXX_REG_ALARM_MON   0xa
+#define M41TXX_REG_ALARM_HOUR  0xc
+#define M41TXX_REG_FLAGS   0xf
+#define M41TXX_REG_SQW 0x13
+
+#define M41TXX_SEC_ST  (1 << 7)/* ST: Stop Bit */
+#define M41TXX_ALHOUR_HT   (1 << 6)/* HT: Halt Update Bit */
+#define M41TXX_FLAGS_BATT_LOW  (1 << 4)/* BL: Battery Low Bit */
+
+#define M41TXX_FEATURE_HT  (1 << 0)
+#define M41TXX_FEATURE_BL  (1 << 1)
+
+#define DRV_VERSION "0.04"
+
+struct m41txx_chip_info {
+   const char *name;
+   u8 features;
+};
+
+static const struct m41txx_chip_info m41txx_chip_info_tbl[] = {
+   { /* M41T80 */
+   .name   = "m41t80",
+   .features   = 0,
+   },
+   { /* M41T81 */
+   .name   = "m41t81",
+   .features   = M41TXX_FEATURE_HT,
+   },
+   { /* M41ST84/85/87/94 */
+   .name   = "m41t85",
+   .features   = M41TXX_FEATURE_HT | M41TXX_FEATURE_BL,
+   },
+};
+
+struct m41txx_data {
+   const struct m41txx_chip_info *chip;
+   struct rtc_device *rtc;
+};
+
+static int m41txx_get_datetime(struct i2c_client *client,
+  struct rtc_time *tm)
+{
+   u8 buf[M41TXX_REG_YEAR + 1], dt_addr[1] = { M41TXX_REG_SEC };
+   struct i2c_msg msgs[] = {
+   {
+   .addr   = client->addr,
+   .flags  = 0,
+   .len= 1,
+   .buf= dt_addr,
+   },
+   {
+   .addr   = client->addr,
+   .flags  = I2C_M_RD,
+   .len= M41TXX_REG_YEAR + 1 - M41TXX_REG_SEC,
+   .buf= buf + M41TXX_REG_SEC,
+   },
+   };
+
+   if (i2c_transfer(client->adapter, msgs, 2) < 0) {
+   dev_err(&client->dev, "read error\n");
+   return -EIO;
+   }
+
+