This patch adds support for the ST m41t81 and m41t85 i2c rtc chips to
the existing m41t00 driver.
Since there is no way to reliably determine what type of rtc chip is in use,
the chip type is passed in via platform_data. The i2c address and square
wave frequency are passed in via platform_data as well. To accommodate
the use of platform_data, a new header file include/linux/m41t00.h has been
added.
The m41t81 and m41t85 chips halt the updating of their time registers
while they are being accessed. They resume when a stop condition exists on
the i2c bus or when non-time related regs are accessed. To make the best use
of that facility and to make more efficient use of the i2c bus, this patch
replaces multiple i2c_smbus_xxx calls with a single i2c_transfer call.
Signed-off-by: Mark A. Greer <[email protected]>
---
drivers/i2c/chips/m41t00.c | 262 +++++++++++++++++++++++++++++++++------------
include/linux/m41t00.h | 50 ++++++++
2 files changed, 246 insertions(+), 66 deletions(-)
---
diff -Nurp linux-2.6.16-mm1-cleanup/drivers/i2c/chips/m41t00.c linux-2.6.16-mm1-newsupp/drivers/i2c/chips/m41t00.c
--- linux-2.6.16-mm1-cleanup/drivers/i2c/chips/m41t00.c 2006-03-23 16:16:35.000000000 -0700
+++ linux-2.6.16-mm1-newsupp/drivers/i2c/chips/m41t00.c 2006-03-23 18:01:26.000000000 -0700
@@ -1,5 +1,5 @@
/*
- * I2C client/driver for the ST M41T00 Real-Time Clock chip.
+ * I2C client/driver for the ST M41T00 family of i2c rtc chips.
*
* Author: Mark A. Greer <[email protected]>
*
@@ -19,22 +19,20 @@
#include <linux/i2c.h>
#include <linux/rtc.h>
#include <linux/bcd.h>
-#include <linux/mutex.h>
#include <linux/workqueue.h>
+#include <linux/platform_device.h>
+#include <linux/m41t00.h>
#include <asm/time.h>
#include <asm/rtc.h>
-#define M41T00_DRV_NAME "m41t00"
-
-static DEFINE_MUTEX(m41t00_mutex);
static struct work_struct set_rtc_time_task;
static struct i2c_driver m41t00_driver;
static struct i2c_client *save_client;
static unsigned short ignore[] = { I2C_CLIENT_END };
-static unsigned short normal_addr[] = { 0x68, I2C_CLIENT_END };
+static unsigned short normal_addr[] = { 0, I2C_CLIENT_END };
static struct i2c_client_address_data addr_data = {
.normal_i2c = normal_addr,
@@ -42,34 +40,45 @@ static struct i2c_client_address_data ad
.ignore = ignore,
};
+struct m41t00_chip_info {
+ u16 type;
+ u16 read_limit;
+ u8 sec; /* Offsets for chip regs */
+ u8 min;
+ u8 hour;
+ u8 day;
+ u8 mon;
+ u8 year;
+ u8 alarm_mon;
+ u8 alarm_hour;
+ u8 sqw;
+ u32 sqw_freq;
+};
+
+static struct m41t00_chip_info m41t00_chip_info_tbl[] = {
+ { M41T00_TYPE_M41T00, 5, 0, 1, 2, 4, 5, 6, 0, 0, 0, 0 },
+ { M41T00_TYPE_M41T81, 1, 1, 2, 3, 5, 6, 7, 0xa, 0xc, 0x13, 0 },
+ { M41T00_TYPE_M41T85, 1, 1, 2, 3, 5, 6, 7, 0xa, 0xc, 0x13, 0 },
+};
+static struct m41t00_chip_info *m41t00_chip;
+
ulong
m41t00_get_rtc_time(void)
{
s32 sec, min, hour, day, mon, year;
s32 sec1, min1, hour1, day1, mon1, year1;
- ulong limit = 10;
+ u16 reads = 0;
+ u8 buf[8], msgbuf[1] = { 0 }; /* offset into rtc's regs */
+ struct i2c_msg msgs[] = {
+ { save_client->addr, 0, 1, msgbuf },
+ { save_client->addr, I2C_M_RD, 8, buf }
+ };
sec = min = hour = day = mon = year = 0;
- sec1 = min1 = hour1 = day1 = mon1 = year1 = 0;
- mutex_lock(&m41t00_mutex);
do {
- if (((sec = i2c_smbus_read_byte_data(save_client, 0)) >= 0)
- && ((min = i2c_smbus_read_byte_data(save_client, 1))
- >= 0)
- && ((hour = i2c_smbus_read_byte_data(save_client, 2))
- >= 0)
- && ((day = i2c_smbus_read_byte_data(save_client, 4))
- >= 0)
- && ((mon = i2c_smbus_read_byte_data(save_client, 5))
- >= 0)
- && ((year = i2c_smbus_read_byte_data(save_client, 6))
- >= 0)
- && ((sec == sec1) && (min == min1) && (hour == hour1)
- && (day == day1) && (mon == mon1)
- && (year == year1)))
-
- break;
+ if (i2c_transfer(save_client->adapter, msgs, 2) < 0)
+ goto read_err;
sec1 = sec;
min1 = min;
@@ -77,21 +86,21 @@ m41t00_get_rtc_time(void)
day1 = day;
mon1 = mon;
year1 = year;
- } while (--limit > 0);
- mutex_unlock(&m41t00_mutex);
- if (limit == 0) {
- dev_warn(&save_client->dev,
- "m41t00: can't read rtc chip\n");
- sec = min = hour = day = mon = year = 0;
- }
-
- sec &= 0x7f;
- min &= 0x7f;
- hour &= 0x3f;
- day &= 0x3f;
- mon &= 0x1f;
- year &= 0xff;
+ sec = buf[m41t00_chip->sec] & 0x7f;
+ min = buf[m41t00_chip->min] & 0x7f;
+ hour = buf[m41t00_chip->hour] & 0x3f;
+ day = buf[m41t00_chip->day] & 0x3f;
+ mon = buf[m41t00_chip->mon] & 0x1f;
+ year = buf[m41t00_chip->year] & 0xff;
+ } while ((++reads < m41t00_chip->read_limit) && ((sec != sec1)
+ || (min != min1) || (hour != hour1) || (day != day1)
+ || (mon != mon1) || (year != year1)));
+
+ if ((m41t00_chip->read_limit > 1) && ((sec != sec1) || (min != min1)
+ || (hour != hour1) || (day != day1) || (mon != mon1)
+ || (year != year1)))
+ goto read_err;
sec = BCD2BIN(sec);
min = BCD2BIN(min);
@@ -105,40 +114,49 @@ m41t00_get_rtc_time(void)
year += 100;
return mktime(year, mon, day, hour, min, sec);
+
+read_err:
+ dev_err(&save_client->dev, "m41t00_get_rtc_time: Read error\n");
+ return 0;
}
static void
m41t00_set(void *arg)
{
struct rtc_time tm;
- ulong nowtime = *(ulong *)arg;
+ int nowtime = *(int *)arg;
+ s32 sec, min, hour, day, mon, year;
+ u8 wbuf[9], *buf = &wbuf[1], msgbuf[1] = { 0 };
+ struct i2c_msg msgs[] = {
+ { save_client->addr, 0, 1, msgbuf },
+ { save_client->addr, I2C_M_RD, 8, buf }
+ };
to_tm(nowtime, &tm);
tm.tm_year = (tm.tm_year - 1900) % 100;
- tm.tm_sec = BIN2BCD(tm.tm_sec);
- tm.tm_min = BIN2BCD(tm.tm_min);
- tm.tm_hour = BIN2BCD(tm.tm_hour);
- tm.tm_mon = BIN2BCD(tm.tm_mon);
- tm.tm_mday = BIN2BCD(tm.tm_mday);
- tm.tm_year = BIN2BCD(tm.tm_year);
-
- mutex_lock(&m41t00_mutex);
- if ((i2c_smbus_write_byte_data(save_client, 0, tm.tm_sec & 0x7f) < 0)
- || (i2c_smbus_write_byte_data(save_client, 1, tm.tm_min & 0x7f)
- < 0)
- || (i2c_smbus_write_byte_data(save_client, 2, tm.tm_hour & 0x7f)
- < 0)
- || (i2c_smbus_write_byte_data(save_client, 4, tm.tm_mday & 0x7f)
- < 0)
- || (i2c_smbus_write_byte_data(save_client, 5, tm.tm_mon & 0x7f)
- < 0)
- || (i2c_smbus_write_byte_data(save_client, 6, tm.tm_year & 0x7f)
- < 0))
+ sec = BIN2BCD(tm.tm_sec);
+ min = BIN2BCD(tm.tm_min);
+ hour = BIN2BCD(tm.tm_hour);
+ day = BIN2BCD(tm.tm_mday);
+ mon = BIN2BCD(tm.tm_mon);
+ year = BIN2BCD(tm.tm_year);
+
+ /* Read reg values into buf[0..7]/wbuf[1..8] */
+ if (i2c_transfer(save_client->adapter, msgs, 2) < 0) {
+ dev_err(&save_client->dev, "m41t00_set: Read error\n");
+ return;
+ }
- dev_warn(&save_client->dev,"m41t00: can't write to rtc chip\n");
+ wbuf[0] = 0; /* offset into rtc's regs */
+ buf[m41t00_chip->sec] = (buf[m41t00_chip->sec] & ~0x7f) | (sec & 0x7f);
+ buf[m41t00_chip->min] = (buf[m41t00_chip->min] & ~0x7f) | (min & 0x7f);
+ buf[m41t00_chip->hour] = (buf[m41t00_chip->hour]& ~0x3f) | (hour& 0x3f);
+ buf[m41t00_chip->day] = (buf[m41t00_chip->day] & ~0x3f) | (day & 0x3f);
+ buf[m41t00_chip->mon] = (buf[m41t00_chip->mon] & ~0x1f) | (mon & 0x1f);
- mutex_unlock(&m41t00_mutex);
+ if (i2c_master_send(save_client, wbuf, 9) < 0)
+ dev_err(&save_client->dev, "m41t00_set: Write error\n");
}
static ulong new_time;
@@ -159,6 +177,47 @@ m41t00_set_rtc_time(ulong nowtime)
/*
*****************************************************************************
*
+ * platform_data Driver Interface
+ *
+ *****************************************************************************
+ */
+static int __init
+m41t00_platform_probe(struct platform_device *pdev)
+{
+ struct m41t00_platform_data *pdata;
+ int i;
+
+ if (pdev && (pdata = pdev->dev.platform_data)) {
+ normal_addr[0] = pdata->i2c_addr;
+
+ for (i=0; i<ARRAY_SIZE(m41t00_chip_info_tbl); i++)
+ if (m41t00_chip_info_tbl[i].type == pdata->type) {
+ m41t00_chip = &m41t00_chip_info_tbl[i];
+ m41t00_chip->sqw_freq = pdata->sqw_freq;
+ return 0;
+ }
+ }
+ return -ENODEV;
+}
+
+static int __exit
+m41t00_platform_remove(struct platform_device *pdev)
+{
+ return 0;
+}
+
+static struct platform_driver m41t00_platform_driver = {
+ .probe = m41t00_platform_probe,
+ .remove = m41t00_platform_remove,
+ .driver = {
+ .owner = THIS_MODULE,
+ .name = M41T00_DRV_NAME,
+ },
+};
+
+/*
+ *****************************************************************************
+ *
* Driver Interface
*
*****************************************************************************
@@ -168,24 +227,90 @@ m41t00_probe(struct i2c_adapter *adap, i
{
struct i2c_client *client;
int rc;
+ u8 rbuf[1], wbuf[2], msgbuf[1] = { 0 }; /* offset into rtc's regs */
+ struct i2c_msg msgs[] = {
+ { 0, 0, 1, msgbuf },
+ { 0, I2C_M_RD, 1, rbuf }
+ };
client = kzalloc(sizeof(struct i2c_client), GFP_KERNEL);
if (!client)
return -ENOMEM;
- strlcpy(client->name, M41T00_DRV_NAME, I2C_NAME_SIZE);
+ strlcpy(client->name, m41t00_driver.driver.name, I2C_NAME_SIZE);
client->addr = addr;
client->adapter = adap;
client->driver = &m41t00_driver;
- if ((rc = i2c_attach_client(client)) != 0) {
- kfree(client);
- return rc;
+ if ((rc = i2c_attach_client(client)) != 0)
+ goto attach_err;
+
+ msgs[0].addr = addr;
+ msgs[1].addr = addr;
+
+ if (m41t00_chip->type != M41T00_TYPE_M41T00) {
+ /* If asked, set SQW frequency & enable */
+ if (m41t00_chip->sqw_freq) {
+ msgbuf[0] = m41t00_chip->alarm_mon;
+ if ((rc = i2c_transfer(client->adapter, msgs, 2)) < 0)
+ goto sqw_err;
+
+ wbuf[0] = m41t00_chip->alarm_mon;
+ wbuf[1] = rbuf[0] & ~0x40;
+ if ((rc = i2c_master_send(client, wbuf, 2)) < 0)
+ goto sqw_err;
+
+ wbuf[0] = m41t00_chip->sqw;
+ wbuf[1] = m41t00_chip->sqw_freq;
+ if ((rc = i2c_master_send(client, wbuf, 2)) < 0)
+ goto sqw_err;
+
+ wbuf[0] = m41t00_chip->alarm_mon;
+ wbuf[1] = rbuf[0] | 0x40;
+ if ((rc = i2c_master_send(client, wbuf, 2)) < 0)
+ goto sqw_err;
+ }
+
+ /* Make sure HT (Halt Update) bit is cleared */
+ msgbuf[0] = m41t00_chip->alarm_hour;
+ if ((rc = i2c_transfer(client->adapter, msgs, 2)) < 0)
+ goto ht_err;
+
+ if (rbuf[0] & 0x40) {
+ wbuf[0] = m41t00_chip->alarm_hour;
+ wbuf[1] = rbuf[0] & ~0x40;
+ if ((rc = i2c_master_send(client, wbuf, 2)) < 0)
+ goto ht_err;
+ }
+ }
+
+ /* Make sure ST (stop) bit is cleared */
+ msgbuf[0] = m41t00_chip->sec;
+ if ((rc = i2c_transfer(client->adapter, msgs, 2)) < 0)
+ goto st_err;
+
+ if (rbuf[0] & 0x80) {
+ wbuf[0] = m41t00_chip->sec;
+ wbuf[1] = rbuf[0] & ~0x80;
+ if ((rc = i2c_master_send(client, wbuf, 2)) < 0)
+ goto st_err;
}
INIT_WORK(&set_rtc_time_task, &m41t00_set, &new_time);
save_client = client;
return 0;
+
+st_err:
+ dev_err(&client->dev, "m41t00_probe: Can't clear ST bit\n");
+ goto attach_err;
+ht_err:
+ dev_err(&client->dev, "m41t00_probe: Can't clear HT bit\n");
+ goto attach_err;
+sqw_err:
+ dev_err(&client->dev, "m41t00_probe: Can't set SQW Frequency\n");
+attach_err:
+ kfree(client);
+ return rc;
}
static int
@@ -219,13 +344,18 @@ static struct i2c_driver m41t00_driver =
static int __init
m41t00_init(void)
{
- return i2c_add_driver(&m41t00_driver);
+ int rc;
+
+ if (!(rc = platform_driver_register(&m41t00_platform_driver)))
+ rc = i2c_add_driver(&m41t00_driver);
+ return rc;
}
static void __exit
m41t00_exit(void)
{
i2c_del_driver(&m41t00_driver);
+ platform_driver_unregister(&m41t00_platform_driver);
}
module_init(m41t00_init);
diff -Nurp linux-2.6.16-mm1-cleanup/include/linux/m41t00.h linux-2.6.16-mm1-newsupp/include/linux/m41t00.h
--- linux-2.6.16-mm1-cleanup/include/linux/m41t00.h 1969-12-31 17:00:00.000000000 -0700
+++ linux-2.6.16-mm1-newsupp/include/linux/m41t00.h 2006-03-23 17:54:54.000000000 -0700
@@ -0,0 +1,50 @@
+/*
+ * Definitions for the ST M41T00 family of i2c rtc chips.
+ *
+ * Author: Mark A. Greer <[email protected]>
+ *
+ * 2005 (c) MontaVista Software, Inc. This file is licensed under
+ * the terms of the GNU General Public License version 2. This program
+ * is licensed "as is" without any warranty of any kind, whether express
+ * or implied.
+ */
+
+#ifndef _M41T00_H
+#define _M41T00_H
+
+#define M41T00_DRV_NAME "m41t00"
+#define M41T00_I2C_ADDR 0x68
+
+#define M41T00_TYPE_M41T00 0
+#define M41T00_TYPE_M41T81 81
+#define M41T00_TYPE_M41T85 85
+
+struct m41t00_platform_data {
+ u16 type;
+ u16 i2c_addr;
+ u32 sqw_freq;
+};
+
+/* SQW output disabled, this is default value by power on*/
+#define SQW_FREQ_DISABLE (0)
+
+#define SQW_FREQ_32KHZ (1<<4) /* 32.768 KHz */
+#define SQW_FREQ_8KHZ (2<<4) /* 8.192 KHz */
+#define SQW_FREQ_4KHZ (3<<4) /* 4.096 KHz */
+#define SQW_FREQ_2KHZ (4<<4) /* 2.048 KHz */
+#define SQW_FREQ_1KHZ (5<<4) /* 1.024 KHz */
+#define SQW_FREQ_512HZ (6<<4) /* 512 Hz */
+#define SQW_FREQ_256HZ (7<<4) /* 256 Hz */
+#define SQW_FREQ_128HZ (8<<4) /* 128 Hz */
+#define SQW_FREQ_64HZ (9<<4) /* 64 Hz */
+#define SQW_FREQ_32HZ (10<<4) /* 32 Hz */
+#define SQW_FREQ_16HZ (11<<4) /* 16 Hz */
+#define SQW_FREQ_8HZ (12<<4) /* 8 Hz */
+#define SQW_FREQ_4HZ (13<<4) /* 4 Hz */
+#define SQW_FREQ_2HZ (14<<4) /* 2 Hz */
+#define SQW_FREQ_1HZ (15<<4) /* 1 Hz */
+
+extern ulong m41t00_get_rtc_time(void);
+extern int m41t00_set_rtc_time(ulong nowtime);
+
+#endif /* _M41T00_H */
"Mark A. Greer" <[email protected]> wrote:
>
> + struct i2c_msg msgs[] = {
> + { save_client->addr, 0, 1, msgbuf },
> + { save_client->addr, I2C_M_RD, 8, buf }
The
.name = value,
form would be preferred. It reduces the possibility of silent breakage if
someone changes struct i2c_msg.
Hi Andrew,
On Sun, Mar 26, 2006 at 02:58:40PM -0800, Andrew Morton wrote:
> "Mark A. Greer" <[email protected]> wrote:
> >
> > + struct i2c_msg msgs[] = {
> > + { save_client->addr, 0, 1, msgbuf },
> > + { save_client->addr, I2C_M_RD, 8, buf }
>
> The
>
> .name = value,
>
> form would be preferred. It reduces the possibility of silent breakage if
> someone changes struct i2c_msg.
Noted. Will fix in upcoming set of new patches.
Mark
Resend...
---
drivers/i2c/chips/m41t00.c | 294 ++++++++++++++++++++++++++++++++++-----------
include/linux/m41t00.h | 50 +++++++
2 files changed, 276 insertions(+), 68 deletions(-)
---
diff -Nurp linux-2.6.16-mm1-cleanup/drivers/i2c/chips/m41t00.c linux-2.6.16-mm1-newsupp/drivers/i2c/chips/m41t00.c
--- linux-2.6.16-mm1-cleanup/drivers/i2c/chips/m41t00.c 2006-03-27 16:00:35.000000000 -0700
+++ linux-2.6.16-mm1-newsupp/drivers/i2c/chips/m41t00.c 2006-03-27 16:49:35.000000000 -0700
@@ -1,5 +1,5 @@
/*
- * I2C client/driver for the ST M41T00 Real-Time Clock chip.
+ * I2C client/driver for the ST M41T00 family of i2c rtc chips.
*
* Author: Mark A. Greer <[email protected]>
*
@@ -19,21 +19,17 @@
#include <linux/i2c.h>
#include <linux/rtc.h>
#include <linux/bcd.h>
-#include <linux/mutex.h>
#include <linux/workqueue.h>
-
+#include <linux/platform_device.h>
+#include <linux/m41t00.h>
#include <asm/time.h>
#include <asm/rtc.h>
-#define M41T00_DRV_NAME "m41t00"
-
-static DEFINE_MUTEX(m41t00_mutex);
-
static struct i2c_driver m41t00_driver;
static struct i2c_client *save_client;
static unsigned short ignore[] = { I2C_CLIENT_END };
-static unsigned short normal_addr[] = { 0x68, I2C_CLIENT_END };
+static unsigned short normal_addr[] = { 0, I2C_CLIENT_END };
static struct i2c_client_address_data addr_data = {
.normal_i2c = normal_addr,
@@ -41,34 +37,55 @@ static struct i2c_client_address_data ad
.ignore = ignore,
};
+struct m41t00_chip_info {
+ u16 type;
+ u16 read_limit;
+ u8 sec; /* Offsets for chip regs */
+ u8 min;
+ u8 hour;
+ u8 day;
+ u8 mon;
+ u8 year;
+ u8 alarm_mon;
+ u8 alarm_hour;
+ u8 sqw;
+ u32 sqw_freq;
+};
+
+static struct m41t00_chip_info m41t00_chip_info_tbl[] = {
+ { M41T00_TYPE_M41T00, 5, 0, 1, 2, 4, 5, 6, 0, 0, 0, 0 },
+ { M41T00_TYPE_M41T81, 1, 1, 2, 3, 5, 6, 7, 0xa, 0xc, 0x13, 0 },
+ { M41T00_TYPE_M41T85, 1, 1, 2, 3, 5, 6, 7, 0xa, 0xc, 0x13, 0 },
+};
+static struct m41t00_chip_info *m41t00_chip;
+
ulong
m41t00_get_rtc_time(void)
{
s32 sec, min, hour, day, mon, year;
s32 sec1, min1, hour1, day1, mon1, year1;
- ulong limit = 10;
+ u16 reads = 0;
+ u8 buf[8], msgbuf[1] = { 0 }; /* offset into rtc's regs */
+ struct i2c_msg msgs[] = {
+ {
+ .addr = save_client->addr,
+ .flags = 0,
+ .len = 1,
+ .buf = msgbuf,
+ },
+ {
+ .addr = save_client->addr,
+ .flags = I2C_M_RD,
+ .len = 8,
+ .buf = buf,
+ },
+ };
sec = min = hour = day = mon = year = 0;
- sec1 = min1 = hour1 = day1 = mon1 = year1 = 0;
- mutex_lock(&m41t00_mutex);
do {
- if (((sec = i2c_smbus_read_byte_data(save_client, 0)) >= 0)
- && ((min = i2c_smbus_read_byte_data(save_client, 1))
- >= 0)
- && ((hour = i2c_smbus_read_byte_data(save_client, 2))
- >= 0)
- && ((day = i2c_smbus_read_byte_data(save_client, 4))
- >= 0)
- && ((mon = i2c_smbus_read_byte_data(save_client, 5))
- >= 0)
- && ((year = i2c_smbus_read_byte_data(save_client, 6))
- >= 0)
- && ((sec == sec1) && (min == min1) && (hour == hour1)
- && (day == day1) && (mon == mon1)
- && (year == year1)))
-
- break;
+ if (i2c_transfer(save_client->adapter, msgs, 2) < 0)
+ goto read_err;
sec1 = sec;
min1 = min;
@@ -76,21 +93,21 @@ m41t00_get_rtc_time(void)
day1 = day;
mon1 = mon;
year1 = year;
- } while (--limit > 0);
- mutex_unlock(&m41t00_mutex);
-
- if (limit == 0) {
- dev_warn(&save_client->dev,
- "m41t00: can't read rtc chip\n");
- sec = min = hour = day = mon = year = 0;
- }
- sec &= 0x7f;
- min &= 0x7f;
- hour &= 0x3f;
- day &= 0x3f;
- mon &= 0x1f;
- year &= 0xff;
+ sec = buf[m41t00_chip->sec] & 0x7f;
+ min = buf[m41t00_chip->min] & 0x7f;
+ hour = buf[m41t00_chip->hour] & 0x3f;
+ day = buf[m41t00_chip->day] & 0x3f;
+ mon = buf[m41t00_chip->mon] & 0x1f;
+ year = buf[m41t00_chip->year] & 0xff;
+ } while ((++reads < m41t00_chip->read_limit) && ((sec != sec1)
+ || (min != min1) || (hour != hour1) || (day != day1)
+ || (mon != mon1) || (year != year1)));
+
+ if ((m41t00_chip->read_limit > 1) && ((sec != sec1) || (min != min1)
+ || (hour != hour1) || (day != day1) || (mon != mon1)
+ || (year != year1)))
+ goto read_err;
sec = BCD2BIN(sec);
min = BCD2BIN(min);
@@ -104,40 +121,59 @@ m41t00_get_rtc_time(void)
year += 100;
return mktime(year, mon, day, hour, min, sec);
+
+read_err:
+ dev_err(&save_client->dev, "m41t00_get_rtc_time: Read error\n");
+ return 0;
}
static void
m41t00_set(void *arg)
{
struct rtc_time tm;
- ulong nowtime = *(ulong *)arg;
+ int nowtime = *(int *)arg;
+ s32 sec, min, hour, day, mon, year;
+ u8 wbuf[9], *buf = &wbuf[1], msgbuf[1] = { 0 };
+ struct i2c_msg msgs[] = {
+ {
+ .addr = save_client->addr,
+ .flags = 0,
+ .len = 1,
+ .buf = msgbuf,
+ },
+ {
+ .addr = save_client->addr,
+ .flags = I2C_M_RD,
+ .len = 8,
+ .buf = buf,
+ },
+ };
to_tm(nowtime, &tm);
tm.tm_year = (tm.tm_year - 1900) % 100;
- tm.tm_sec = BIN2BCD(tm.tm_sec);
- tm.tm_min = BIN2BCD(tm.tm_min);
- tm.tm_hour = BIN2BCD(tm.tm_hour);
- tm.tm_mon = BIN2BCD(tm.tm_mon);
- tm.tm_mday = BIN2BCD(tm.tm_mday);
- tm.tm_year = BIN2BCD(tm.tm_year);
-
- mutex_lock(&m41t00_mutex);
- if ((i2c_smbus_write_byte_data(save_client, 0, tm.tm_sec & 0x7f) < 0)
- || (i2c_smbus_write_byte_data(save_client, 1, tm.tm_min & 0x7f)
- < 0)
- || (i2c_smbus_write_byte_data(save_client, 2, tm.tm_hour & 0x7f)
- < 0)
- || (i2c_smbus_write_byte_data(save_client, 4, tm.tm_mday & 0x7f)
- < 0)
- || (i2c_smbus_write_byte_data(save_client, 5, tm.tm_mon & 0x7f)
- < 0)
- || (i2c_smbus_write_byte_data(save_client, 6, tm.tm_year & 0x7f)
- < 0))
+ sec = BIN2BCD(tm.tm_sec);
+ min = BIN2BCD(tm.tm_min);
+ hour = BIN2BCD(tm.tm_hour);
+ day = BIN2BCD(tm.tm_mday);
+ mon = BIN2BCD(tm.tm_mon);
+ year = BIN2BCD(tm.tm_year);
+
+ /* Read reg values into buf[0..7]/wbuf[1..8] */
+ if (i2c_transfer(save_client->adapter, msgs, 2) < 0) {
+ dev_err(&save_client->dev, "m41t00_set: Read error\n");
+ return;
+ }
- dev_warn(&save_client->dev,"m41t00: can't write to rtc chip\n");
+ wbuf[0] = 0; /* offset into rtc's regs */
+ buf[m41t00_chip->sec] = (buf[m41t00_chip->sec] & ~0x7f) | (sec & 0x7f);
+ buf[m41t00_chip->min] = (buf[m41t00_chip->min] & ~0x7f) | (min & 0x7f);
+ buf[m41t00_chip->hour] = (buf[m41t00_chip->hour]& ~0x3f) | (hour& 0x3f);
+ buf[m41t00_chip->day] = (buf[m41t00_chip->day] & ~0x3f) | (day & 0x3f);
+ buf[m41t00_chip->mon] = (buf[m41t00_chip->mon] & ~0x1f) | (mon & 0x1f);
- mutex_unlock(&m41t00_mutex);
+ if (i2c_master_send(save_client, wbuf, 9) < 0)
+ dev_err(&save_client->dev, "m41t00_set: Write error\n");
}
static ulong new_time;
@@ -160,6 +196,47 @@ m41t00_set_rtc_time(ulong nowtime)
/*
*****************************************************************************
*
+ * platform_data Driver Interface
+ *
+ *****************************************************************************
+ */
+static int __init
+m41t00_platform_probe(struct platform_device *pdev)
+{
+ struct m41t00_platform_data *pdata;
+ int i;
+
+ if (pdev && (pdata = pdev->dev.platform_data)) {
+ normal_addr[0] = pdata->i2c_addr;
+
+ for (i=0; i<ARRAY_SIZE(m41t00_chip_info_tbl); i++)
+ if (m41t00_chip_info_tbl[i].type == pdata->type) {
+ m41t00_chip = &m41t00_chip_info_tbl[i];
+ m41t00_chip->sqw_freq = pdata->sqw_freq;
+ return 0;
+ }
+ }
+ return -ENODEV;
+}
+
+static int __exit
+m41t00_platform_remove(struct platform_device *pdev)
+{
+ return 0;
+}
+
+static struct platform_driver m41t00_platform_driver = {
+ .probe = m41t00_platform_probe,
+ .remove = m41t00_platform_remove,
+ .driver = {
+ .owner = THIS_MODULE,
+ .name = M41T00_DRV_NAME,
+ },
+};
+
+/*
+ *****************************************************************************
+ *
* Driver Interface
*
*****************************************************************************
@@ -169,24 +246,100 @@ m41t00_probe(struct i2c_adapter *adap, i
{
struct i2c_client *client;
int rc;
+ u8 rbuf[1], wbuf[2], msgbuf[1] = { 0 }; /* offset into rtc's regs */
+ struct i2c_msg msgs[] = {
+ {
+ .addr = 0,
+ .flags = 0,
+ .len = 1,
+ .buf = msgbuf,
+ },
+ {
+ .addr = 0,
+ .flags = I2C_M_RD,
+ .len = 1,
+ .buf = rbuf,
+ },
+ };
client = kzalloc(sizeof(struct i2c_client), GFP_KERNEL);
if (!client)
return -ENOMEM;
- strlcpy(client->name, M41T00_DRV_NAME, I2C_NAME_SIZE);
+ strlcpy(client->name, m41t00_driver.driver.name, I2C_NAME_SIZE);
client->addr = addr;
client->adapter = adap;
client->driver = &m41t00_driver;
- if ((rc = i2c_attach_client(client)) != 0) {
- kfree(client);
- return rc;
+ if ((rc = i2c_attach_client(client)) != 0)
+ goto attach_err;
+
+ msgs[0].addr = addr;
+ msgs[1].addr = addr;
+
+ if (m41t00_chip->type != M41T00_TYPE_M41T00) {
+ /* If asked, set SQW frequency & enable */
+ if (m41t00_chip->sqw_freq) {
+ msgbuf[0] = m41t00_chip->alarm_mon;
+ if ((rc = i2c_transfer(client->adapter, msgs, 2)) < 0)
+ goto sqw_err;
+
+ wbuf[0] = m41t00_chip->alarm_mon;
+ wbuf[1] = rbuf[0] & ~0x40;
+ if ((rc = i2c_master_send(client, wbuf, 2)) < 0)
+ goto sqw_err;
+
+ wbuf[0] = m41t00_chip->sqw;
+ wbuf[1] = m41t00_chip->sqw_freq;
+ if ((rc = i2c_master_send(client, wbuf, 2)) < 0)
+ goto sqw_err;
+
+ wbuf[0] = m41t00_chip->alarm_mon;
+ wbuf[1] = rbuf[0] | 0x40;
+ if ((rc = i2c_master_send(client, wbuf, 2)) < 0)
+ goto sqw_err;
+ }
+
+ /* Make sure HT (Halt Update) bit is cleared */
+ msgbuf[0] = m41t00_chip->alarm_hour;
+ if ((rc = i2c_transfer(client->adapter, msgs, 2)) < 0)
+ goto ht_err;
+
+ if (rbuf[0] & 0x40) {
+ wbuf[0] = m41t00_chip->alarm_hour;
+ wbuf[1] = rbuf[0] & ~0x40;
+ if ((rc = i2c_master_send(client, wbuf, 2)) < 0)
+ goto ht_err;
+ }
+ }
+
+ /* Make sure ST (stop) bit is cleared */
+ msgbuf[0] = m41t00_chip->sec;
+ if ((rc = i2c_transfer(client->adapter, msgs, 2)) < 0)
+ goto st_err;
+
+ if (rbuf[0] & 0x80) {
+ wbuf[0] = m41t00_chip->sec;
+ wbuf[1] = rbuf[0] & ~0x80;
+ if ((rc = i2c_master_send(client, wbuf, 2)) < 0)
+ goto st_err;
}
m41t00_wq = create_singlethread_workqueue("m41t00");
save_client = client;
return 0;
+
+st_err:
+ dev_err(&client->dev, "m41t00_probe: Can't clear ST bit\n");
+ goto attach_err;
+ht_err:
+ dev_err(&client->dev, "m41t00_probe: Can't clear HT bit\n");
+ goto attach_err;
+sqw_err:
+ dev_err(&client->dev, "m41t00_probe: Can't set SQW Frequency\n");
+attach_err:
+ kfree(client);
+ return rc;
}
static int
@@ -219,13 +372,18 @@ static struct i2c_driver m41t00_driver =
static int __init
m41t00_init(void)
{
- return i2c_add_driver(&m41t00_driver);
+ int rc;
+
+ if (!(rc = platform_driver_register(&m41t00_platform_driver)))
+ rc = i2c_add_driver(&m41t00_driver);
+ return rc;
}
static void __exit
m41t00_exit(void)
{
i2c_del_driver(&m41t00_driver);
+ platform_driver_unregister(&m41t00_platform_driver);
}
module_init(m41t00_init);
diff -Nurp linux-2.6.16-mm1-cleanup/include/linux/m41t00.h linux-2.6.16-mm1-newsupp/include/linux/m41t00.h
--- linux-2.6.16-mm1-cleanup/include/linux/m41t00.h 1969-12-31 17:00:00.000000000 -0700
+++ linux-2.6.16-mm1-newsupp/include/linux/m41t00.h 2006-03-27 16:25:51.000000000 -0700
@@ -0,0 +1,50 @@
+/*
+ * Definitions for the ST M41T00 family of i2c rtc chips.
+ *
+ * Author: Mark A. Greer <[email protected]>
+ *
+ * 2005 (c) MontaVista Software, Inc. This file is licensed under
+ * the terms of the GNU General Public License version 2. This program
+ * is licensed "as is" without any warranty of any kind, whether express
+ * or implied.
+ */
+
+#ifndef _M41T00_H
+#define _M41T00_H
+
+#define M41T00_DRV_NAME "m41t00"
+#define M41T00_I2C_ADDR 0x68
+
+#define M41T00_TYPE_M41T00 0
+#define M41T00_TYPE_M41T81 81
+#define M41T00_TYPE_M41T85 85
+
+struct m41t00_platform_data {
+ u16 type;
+ u16 i2c_addr;
+ u32 sqw_freq;
+};
+
+/* SQW output disabled, this is default value by power on*/
+#define SQW_FREQ_DISABLE (0)
+
+#define SQW_FREQ_32KHZ (1<<4) /* 32.768 KHz */
+#define SQW_FREQ_8KHZ (2<<4) /* 8.192 KHz */
+#define SQW_FREQ_4KHZ (3<<4) /* 4.096 KHz */
+#define SQW_FREQ_2KHZ (4<<4) /* 2.048 KHz */
+#define SQW_FREQ_1KHZ (5<<4) /* 1.024 KHz */
+#define SQW_FREQ_512HZ (6<<4) /* 512 Hz */
+#define SQW_FREQ_256HZ (7<<4) /* 256 Hz */
+#define SQW_FREQ_128HZ (8<<4) /* 128 Hz */
+#define SQW_FREQ_64HZ (9<<4) /* 64 Hz */
+#define SQW_FREQ_32HZ (10<<4) /* 32 Hz */
+#define SQW_FREQ_16HZ (11<<4) /* 16 Hz */
+#define SQW_FREQ_8HZ (12<<4) /* 8 Hz */
+#define SQW_FREQ_4HZ (13<<4) /* 4 Hz */
+#define SQW_FREQ_2HZ (14<<4) /* 2 Hz */
+#define SQW_FREQ_1HZ (15<<4) /* 1 Hz */
+
+extern ulong m41t00_get_rtc_time(void);
+extern int m41t00_set_rtc_time(ulong nowtime);
+
+#endif /* _M41T00_H */
"Mark A. Greer" <[email protected]> wrote:
>
> Resend...
> ---
>
> drivers/i2c/chips/m41t00.c | 294 ++++++++++++++++++++++++++++++++++-----------
> include/linux/m41t00.h | 50 +++++++
Not sure what to make of this. It has no changelog, it doesn't apply on
top of your previous three patches:
rtc-m41t00-driver-should-use-workqueue-instead-of-tasklet.patch
rtc-m41t00-driver-cleanup.patch
rtc-add-support-for-m41t81-m41t85-chips-to-m41t00-driver.patch
and it doesn't apply when used to replace
rtc-add-support-for-m41t81-m41t85-chips-to-m41t00-driver.patch.
An incremental patch against the three above patches would be ideal...
Hi Mark, Andrew,
> Mark A. Greer wrote:
> >
> > Resend...
> > ---
> >
> > drivers/i2c/chips/m41t00.c | 294 ++++++++++++++++++++++++++++++++++-----------
> > include/linux/m41t00.h | 50 +++++++
>
Andrew Morton wrote:
> Not sure what to make of this. It has no changelog, it doesn't apply on
> top of your previous three patches:
>
> rtc-m41t00-driver-should-use-workqueue-instead-of-tasklet.patch
> rtc-m41t00-driver-cleanup.patch
> rtc-add-support-for-m41t81-m41t85-chips-to-m41t00-driver.patch
>
> and it doesn't apply when used to replace
> rtc-add-support-for-m41t81-m41t85-chips-to-m41t00-driver.patch.
Replacing works for me, after also replacing the two first patches of
the series with their new respective version. As for the changelog I
picked the one from the original third patch.
> An incremental patch against the three above patches would be ideal...
Well, the first two patches (workqueue and cleanup) look ok to me now,
so I'll send them to Greg now, together with one similar patch for the
ds1374 driver. The workqueue patches need to go to Linus soon, as they
fix a bug in these drivers, and I know that Greg has other i2c patches
waiting to go to Linus anyway. The cleanup patch can go in too, I
think, as it's simple enough.
As for the third patch, I have some more comments on it (sorry for
being slow) and it might need some tweaking, so it's probably better
(and easier) if Andrew just drops it for now, and it'll get back to him
when ready.
Mark, is it OK if this third patch adding the m41t81 and m41t85 support
spends some time in Andrew's tree and only goes in mainline for 2.6.18,
or do you need it in 2.6.17?
Thanks,
--
Jean Delvare
Hi Mark,
Some more comments, sorry for being slow.
> drivers/i2c/chips/m41t00.c | 294 ++++++++++++++++++++++++++++++++++-----------
> include/linux/m41t00.h | 50 +++++++
Who else is going to use this header file? It's rather rare to have a
hardware-specific header file under include/linux.
> diff -Nurp linux-2.6.16-mm1-cleanup/drivers/i2c/chips/m41t00.c linux-2.6.16-mm1-newsupp/drivers/i2c/chips/m41t00.c
> --- linux-2.6.16-mm1-cleanup/drivers/i2c/chips/m41t00.c 2006-03-27 16:00:35.000000000 -0700
> +++ linux-2.6.16-mm1-newsupp/drivers/i2c/chips/m41t00.c 2006-03-27 16:49:35.000000000 -0700
> (...)
> static unsigned short ignore[] = { I2C_CLIENT_END };
> -static unsigned short normal_addr[] = { 0x68, I2C_CLIENT_END };
> +static unsigned short normal_addr[] = { 0, I2C_CLIENT_END };
Bad idea. If no platform data is found, the driver will attempt to
probe I2C address 0, which is a broadcast address. I can predict
interesting results ;)
So, if you don't want the driver to do anything in the absence of
platform data, you should define normal_addr the following way:
static unsigned short normal_addr[] = { I2C_CLIENT_END, I2C_CLIENT_END };
> +struct m41t00_chip_info {
> + u16 type;
> + u16 read_limit;
> + u8 sec; /* Offsets for chip regs */
> + u8 min;
> + u8 hour;
> + u8 day;
> + u8 mon;
> + u8 year;
> + u8 alarm_mon;
> + u8 alarm_hour;
> + u8 sqw;
> + u32 sqw_freq;
> +};
u16 is probably overkill for .type and .read_limit.
> +static struct m41t00_chip_info m41t00_chip_info_tbl[] = {
> + { M41T00_TYPE_M41T00, 5, 0, 1, 2, 4, 5, 6, 0, 0, 0, 0 },
> + { M41T00_TYPE_M41T81, 1, 1, 2, 3, 5, 6, 7, 0xa, 0xc, 0x13, 0 },
> + { M41T00_TYPE_M41T85, 1, 1, 2, 3, 5, 6, 7, 0xa, 0xc, 0x13, 0 },
> +};
C99-style initialization, please? It'll take much more lines, granted,
but is also way more robust to future changes, and will be more
readable too.
May I ask why you define separate types for the M41T81 and the M41T85,
when you handle both exactly the same way?
> + sec = buf[m41t00_chip->sec] & 0x7f;
> + min = buf[m41t00_chip->min] & 0x7f;
> + hour = buf[m41t00_chip->hour] & 0x3f;
> + day = buf[m41t00_chip->day] & 0x3f;
> + mon = buf[m41t00_chip->mon] & 0x1f;
> + year = buf[m41t00_chip->year] & 0xff;
The year masking is a no-op, you could omit it.
> @@ -169,24 +246,100 @@ m41t00_probe(struct i2c_adapter *adap, i
> {
> struct i2c_client *client;
> int rc;
> + u8 rbuf[1], wbuf[2], msgbuf[1] = { 0 }; /* offset into rtc's regs */
> + struct i2c_msg msgs[] = {
> + {
> + .addr = 0,
> + .flags = 0,
> + .len = 1,
> + .buf = msgbuf,
> + },
> + {
> + .addr = 0,
> + .flags = I2C_M_RD,
> + .len = 1,
> + .buf = rbuf,
> + },
> + };
Why don't you initialize both .addr right away?
>
> client = kzalloc(sizeof(struct i2c_client), GFP_KERNEL);
> if (!client)
> return -ENOMEM;
>
> - strlcpy(client->name, M41T00_DRV_NAME, I2C_NAME_SIZE);
> + strlcpy(client->name, m41t00_driver.driver.name, I2C_NAME_SIZE);
The driver name may not be suitable here, the client name should
instead reflect what chip it is.
> + if (m41t00_chip->type != M41T00_TYPE_M41T00) {
> + /* If asked, set SQW frequency & enable */
> + if (m41t00_chip->sqw_freq) {
> + msgbuf[0] = m41t00_chip->alarm_mon;
> + if ((rc = i2c_transfer(client->adapter, msgs, 2)) < 0)
> + goto sqw_err;
You did not check whether the adapter you are attaching to supports this
kind of transaction! You need to call i2c_check_functionality, see how
other i2c chip drivers (for example ds1337) are doing. For i2c_transfer
and i2c_master_send, you want to check for I2C_FUNC_I2C.
> +
> + wbuf[0] = m41t00_chip->alarm_mon;
> + wbuf[1] = rbuf[0] & ~0x40;
> + if ((rc = i2c_master_send(client, wbuf, 2)) < 0)
> + goto sqw_err;
> +
> + wbuf[0] = m41t00_chip->sqw;
> + wbuf[1] = m41t00_chip->sqw_freq;
> + if ((rc = i2c_master_send(client, wbuf, 2)) < 0)
> + goto sqw_err;
> +
> + wbuf[0] = m41t00_chip->alarm_mon;
> + wbuf[1] = rbuf[0] | 0x40;
> + if ((rc = i2c_master_send(client, wbuf, 2)) < 0)
> + goto sqw_err;
> + }
> +
> + /* Make sure HT (Halt Update) bit is cleared */
> + msgbuf[0] = m41t00_chip->alarm_hour;
> + if ((rc = i2c_transfer(client->adapter, msgs, 2)) < 0)
> + goto ht_err;
> +
> + if (rbuf[0] & 0x40) {
> + wbuf[0] = m41t00_chip->alarm_hour;
> + wbuf[1] = rbuf[0] & ~0x40;
> + if ((rc = i2c_master_send(client, wbuf, 2)) < 0)
> + goto ht_err;
> + }
> + }
> +
> + /* Make sure ST (stop) bit is cleared */
> + msgbuf[0] = m41t00_chip->sec;
> + if ((rc = i2c_transfer(client->adapter, msgs, 2)) < 0)
> + goto st_err;
> +
> + if (rbuf[0] & 0x80) {
> + wbuf[0] = m41t00_chip->sec;
> + wbuf[1] = rbuf[0] & ~0x80;
> + if ((rc = i2c_master_send(client, wbuf, 2)) < 0)
> + goto st_err;
> }
What you do here are basically SMBus Read Byte and SMBus Write Byte
transactions. The code would be much more simple if you were using the
i2c_smbus_read_byte_data and i2c_smbus_write_byte_data functions, which
take care of all the technical details.
> +st_err:
> + dev_err(&client->dev, "m41t00_probe: Can't clear ST bit\n");
> + goto attach_err;
> +ht_err:
> + dev_err(&client->dev, "m41t00_probe: Can't clear HT bit\n");
> + goto attach_err;
> +sqw_err:
> + dev_err(&client->dev, "m41t00_probe: Can't set SQW Frequency\n");
> +attach_err:
> + kfree(client);
> + return rc;
> }
Mrghh, this isn't exactly elegant... You should be able to make it
better if you switch to i2c_smbus_read/write_byte_data as suggested.
> diff -Nurp linux-2.6.16-mm1-cleanup/include/linux/m41t00.h linux-2.6.16-mm1-newsupp/include/linux/m41t00.h
> --- linux-2.6.16-mm1-cleanup/include/linux/m41t00.h 1969-12-31 17:00:00.000000000 -0700
> +++ linux-2.6.16-mm1-newsupp/include/linux/m41t00.h 2006-03-27 16:25:51.000000000 -0700
> @@ -0,0 +1,50 @@
> +/*
> + * Definitions for the ST M41T00 family of i2c rtc chips.
> + *
> + * Author: Mark A. Greer <[email protected]>
> + *
> + * 2005 (c) MontaVista Software, Inc. This file is licensed under
We're in year 2006 now :)
> + * the terms of the GNU General Public License version 2. This program
> + * is licensed "as is" without any warranty of any kind, whether express
> + * or implied.
> + */
> +
> +#ifndef _M41T00_H
> +#define _M41T00_H
> +
> +#define M41T00_DRV_NAME "m41t00"
Why do you need to export this?
> +#define M41T00_I2C_ADDR 0x68
Not used anywhere?
> +#define M41T00_TYPE_M41T00 0
> +#define M41T00_TYPE_M41T81 81
> +#define M41T00_TYPE_M41T85 85
The i2c core has a facility for drivers supporting several types of
devices. Check for I2C_CLIENT_INSMOD_3() in your case. The advantage if
you go that way is that chip types can be set from userspace through
module parameters, which may be convenient for testing when adding
support for new platforms.
> +/* SQW output disabled, this is default value by power on*/
> +#define SQW_FREQ_DISABLE (0)
> +
> +#define SQW_FREQ_32KHZ (1<<4) /* 32.768 KHz */
> +#define SQW_FREQ_8KHZ (2<<4) /* 8.192 KHz */
> +#define SQW_FREQ_4KHZ (3<<4) /* 4.096 KHz */
> +#define SQW_FREQ_2KHZ (4<<4) /* 2.048 KHz */
> +#define SQW_FREQ_1KHZ (5<<4) /* 1.024 KHz */
> +#define SQW_FREQ_512HZ (6<<4) /* 512 Hz */
> +#define SQW_FREQ_256HZ (7<<4) /* 256 Hz */
> +#define SQW_FREQ_128HZ (8<<4) /* 128 Hz */
> +#define SQW_FREQ_64HZ (9<<4) /* 64 Hz */
> +#define SQW_FREQ_32HZ (10<<4) /* 32 Hz */
> +#define SQW_FREQ_16HZ (11<<4) /* 16 Hz */
> +#define SQW_FREQ_8HZ (12<<4) /* 8 Hz */
> +#define SQW_FREQ_4HZ (13<<4) /* 4 Hz */
> +#define SQW_FREQ_2HZ (14<<4) /* 2 Hz */
> +#define SQW_FREQ_1HZ (15<<4) /* 1 Hz */
Not used anywhere either?
> +extern ulong m41t00_get_rtc_time(void);
> +extern int m41t00_set_rtc_time(ulong nowtime);
Hopefully you won't have to export these anymore after you move to the
new RTC subsystem. But that's a different story. In the meantime,
shouldn't you have EXPORT_SYMBOL_GPL() for these functions? Else
compiling m41t00 as a module won't work.
Thanks,
--
Jean Delvare
On Tue, Mar 28, 2006 at 02:21:14PM +0200, Jean Delvare wrote:
> Hi Mark, Andrew,
>
> > Mark A. Greer wrote:
> > >
> > > Resend...
> > > ---
> > >
> > > drivers/i2c/chips/m41t00.c | 294 ++++++++++++++++++++++++++++++++++-----------
> > > include/linux/m41t00.h | 50 +++++++
> >
>
> Andrew Morton wrote:
> > Not sure what to make of this. It has no changelog,
Oops. My bad.
> > it doesn't apply on
> > top of your previous three patches:
> >
> > rtc-m41t00-driver-should-use-workqueue-instead-of-tasklet.patch
> > rtc-m41t00-driver-cleanup.patch
> > rtc-add-support-for-m41t81-m41t85-chips-to-m41t00-driver.patch
> >
> > and it doesn't apply when used to replace
> > rtc-add-support-for-m41t81-m41t85-chips-to-m41t00-driver.patch.
>
> Replacing works for me, after also replacing the two first patches of
> the series with their new respective version. As for the changelog I
> picked the one from the original third patch.
Sorry for the confusion, Andrew. The 3 patches I sent were all
replacement patches for the previous 3 patches.
> > An incremental patch against the three above patches would be ideal...
I would have done that but there wasn't a 2.6.16-mm2 patch yesterday
(that I see is there now) so I had nothing to diff against.
<snip>
> Mark, is it OK if this third patch adding the m41t81 and m41t85 support
> spends some time in Andrew's tree and only goes in mainline for 2.6.18,
> or do you need it in 2.6.17?
No, waiting is fine, Jean.
Thanks guys.
Mark
Hi Jean,
On Tue, Mar 28, 2006 at 05:54:50PM +0200, Jean Delvare wrote:
> Hi Mark,
>
> Some more comments, sorry for being slow.
You're not slow at all. No worries.
> > drivers/i2c/chips/m41t00.c | 294 ++++++++++++++++++++++++++++++++++-----------
> > include/linux/m41t00.h | 50 +++++++
>
> Who else is going to use this header file? It's rather rare to have a
> hardware-specific header file under include/linux.
The platform-specific code for any platform that uses that rtc will need
to include that header file so it can pass the platform data to the rtc
driver. I have a patch for arch/ppc/platforms/katana.c but I haven't
submitted it b/c of the ppc->powerpc merge that's going on right now.
I will get it in there once I've moved the katana code into the merged tree.
There is a lot of work before that happens, though.
> > diff -Nurp linux-2.6.16-mm1-cleanup/drivers/i2c/chips/m41t00.c linux-2.6.16-mm1-newsupp/drivers/i2c/chips/m41t00.c
> > --- linux-2.6.16-mm1-cleanup/drivers/i2c/chips/m41t00.c 2006-03-27 16:00:35.000000000 -0700
> > +++ linux-2.6.16-mm1-newsupp/drivers/i2c/chips/m41t00.c 2006-03-27 16:49:35.000000000 -0700
> > (...)
> > static unsigned short ignore[] = { I2C_CLIENT_END };
> > -static unsigned short normal_addr[] = { 0x68, I2C_CLIENT_END };
> > +static unsigned short normal_addr[] = { 0, I2C_CLIENT_END };
>
> Bad idea. If no platform data is found, the driver will attempt to
> probe I2C address 0, which is a broadcast address. I can predict
> interesting results ;)
>
> So, if you don't want the driver to do anything in the absence of
> platform data, you should define normal_addr the following way:
If there is no platform data, m41t00_platform_probe() should return
-ENODEV and make m41t00_init() causing the driver to not become active.
I've tested that, actually, and it seems to work.
> static unsigned short normal_addr[] = { I2C_CLIENT_END, I2C_CLIENT_END };
Hrm, that's a good idea. I'll do that, even though it shouldn't be
necessary.
> > +struct m41t00_chip_info {
> > + u16 type;
> > + u16 read_limit;
> > + u8 sec; /* Offsets for chip regs */
> > + u8 min;
> > + u8 hour;
> > + u8 day;
> > + u8 mon;
> > + u8 year;
> > + u8 alarm_mon;
> > + u8 alarm_hour;
> > + u8 sqw;
> > + u32 sqw_freq;
> > +};
>
> u16 is probably overkill for .type and .read_limit.
Yeah, probably. I'll change them to u8.
> > +static struct m41t00_chip_info m41t00_chip_info_tbl[] = {
> > + { M41T00_TYPE_M41T00, 5, 0, 1, 2, 4, 5, 6, 0, 0, 0, 0 },
> > + { M41T00_TYPE_M41T81, 1, 1, 2, 3, 5, 6, 7, 0xa, 0xc, 0x13, 0 },
> > + { M41T00_TYPE_M41T85, 1, 1, 2, 3, 5, 6, 7, 0xa, 0xc, 0x13, 0 },
> > +};
>
> C99-style initialization, please? It'll take much more lines, granted,
> but is also way more robust to future changes, and will be more
> readable too.
Well, the struct is right there so I figured the chances of changing the
struct and not seeing that the compile-time init code needs to be
changed too was small. I guess its better to be safe than sorry and it
does make it more readable so I'll change it.
> May I ask why you define separate types for the M41T81 and the M41T85,
> when you handle both exactly the same way?
The only reason is so that its obvious that both the t81 and t85 are
supported. If I make it M41T81 only then I can easily see someone
grep'ing around looking for M41T85, not finding it, and writing their
own driver. I don't see any harm in having both, do you?
> > + sec = buf[m41t00_chip->sec] & 0x7f;
> > + min = buf[m41t00_chip->min] & 0x7f;
> > + hour = buf[m41t00_chip->hour] & 0x3f;
> > + day = buf[m41t00_chip->day] & 0x3f;
> > + mon = buf[m41t00_chip->mon] & 0x1f;
> > + year = buf[m41t00_chip->year] & 0xff;
>
> The year masking is a no-op, you could omit it.
Yes.
> > @@ -169,24 +246,100 @@ m41t00_probe(struct i2c_adapter *adap, i
> > {
> > struct i2c_client *client;
> > int rc;
> > + u8 rbuf[1], wbuf[2], msgbuf[1] = { 0 }; /* offset into rtc's regs */
> > + struct i2c_msg msgs[] = {
> > + {
> > + .addr = 0,
> > + .flags = 0,
> > + .len = 1,
> > + .buf = msgbuf,
> > + },
> > + {
> > + .addr = 0,
> > + .flags = I2C_M_RD,
> > + .len = 1,
> > + .buf = rbuf,
> > + },
> > + };
>
> Why don't you initialize both .addr right away?
Actually, I already init the .addr field right away so I'll remove them
from them from the code above.
> >
> > client = kzalloc(sizeof(struct i2c_client), GFP_KERNEL);
> > if (!client)
> > return -ENOMEM;
> >
> > - strlcpy(client->name, M41T00_DRV_NAME, I2C_NAME_SIZE);
> > + strlcpy(client->name, m41t00_driver.driver.name, I2C_NAME_SIZE);
>
> The driver name may not be suitable here, the client name should
> instead reflect what chip it is.
Ah, good one.
> > + if (m41t00_chip->type != M41T00_TYPE_M41T00) {
> > + /* If asked, set SQW frequency & enable */
> > + if (m41t00_chip->sqw_freq) {
> > + msgbuf[0] = m41t00_chip->alarm_mon;
> > + if ((rc = i2c_transfer(client->adapter, msgs, 2)) < 0)
> > + goto sqw_err;
>
> You did not check whether the adapter you are attaching to supports this
> kind of transaction! You need to call i2c_check_functionality, see how
> other i2c chip drivers (for example ds1337) are doing. For i2c_transfer
> and i2c_master_send, you want to check for I2C_FUNC_I2C.
Okay.
> > +
> > + wbuf[0] = m41t00_chip->alarm_mon;
> > + wbuf[1] = rbuf[0] & ~0x40;
> > + if ((rc = i2c_master_send(client, wbuf, 2)) < 0)
> > + goto sqw_err;
> > +
> > + wbuf[0] = m41t00_chip->sqw;
> > + wbuf[1] = m41t00_chip->sqw_freq;
> > + if ((rc = i2c_master_send(client, wbuf, 2)) < 0)
> > + goto sqw_err;
> > +
> > + wbuf[0] = m41t00_chip->alarm_mon;
> > + wbuf[1] = rbuf[0] | 0x40;
> > + if ((rc = i2c_master_send(client, wbuf, 2)) < 0)
> > + goto sqw_err;
> > + }
> > +
> > + /* Make sure HT (Halt Update) bit is cleared */
> > + msgbuf[0] = m41t00_chip->alarm_hour;
> > + if ((rc = i2c_transfer(client->adapter, msgs, 2)) < 0)
> > + goto ht_err;
> > +
> > + if (rbuf[0] & 0x40) {
> > + wbuf[0] = m41t00_chip->alarm_hour;
> > + wbuf[1] = rbuf[0] & ~0x40;
> > + if ((rc = i2c_master_send(client, wbuf, 2)) < 0)
> > + goto ht_err;
> > + }
> > + }
> > +
> > + /* Make sure ST (stop) bit is cleared */
> > + msgbuf[0] = m41t00_chip->sec;
> > + if ((rc = i2c_transfer(client->adapter, msgs, 2)) < 0)
> > + goto st_err;
> > +
> > + if (rbuf[0] & 0x80) {
> > + wbuf[0] = m41t00_chip->sec;
> > + wbuf[1] = rbuf[0] & ~0x80;
> > + if ((rc = i2c_master_send(client, wbuf, 2)) < 0)
> > + goto st_err;
> > }
>
> What you do here are basically SMBus Read Byte and SMBus Write Byte
> transactions. The code would be much more simple if you were using the
> i2c_smbus_read_byte_data and i2c_smbus_write_byte_data functions, which
> take care of all the technical details.
That's true but I assumed that since I was using i2c_transfer
earlier, I should use it here. Is that a bad assumption?
I do see that ds1337.c uses both types.
> > +st_err:
> > + dev_err(&client->dev, "m41t00_probe: Can't clear ST bit\n");
> > + goto attach_err;
> > +ht_err:
> > + dev_err(&client->dev, "m41t00_probe: Can't clear HT bit\n");
> > + goto attach_err;
> > +sqw_err:
> > + dev_err(&client->dev, "m41t00_probe: Can't set SQW Frequency\n");
> > +attach_err:
> > + kfree(client);
> > + return rc;
> > }
>
> Mrghh, this isn't exactly elegant... You should be able to make it
> better if you switch to i2c_smbus_read/write_byte_data as suggested.
Yep.
> > diff -Nurp linux-2.6.16-mm1-cleanup/include/linux/m41t00.h linux-2.6.16-mm1-newsupp/include/linux/m41t00.h
> > --- linux-2.6.16-mm1-cleanup/include/linux/m41t00.h 1969-12-31 17:00:00.000000000 -0700
> > +++ linux-2.6.16-mm1-newsupp/include/linux/m41t00.h 2006-03-27 16:25:51.000000000 -0700
> > @@ -0,0 +1,50 @@
> > +/*
> > + * Definitions for the ST M41T00 family of i2c rtc chips.
> > + *
> > + * Author: Mark A. Greer <[email protected]>
> > + *
> > + * 2005 (c) MontaVista Software, Inc. This file is licensed under
>
> We're in year 2006 now :)
Yes, but my understanding is that I should leave the 2005 there b/c that
file was already copyrighted with that year. I could do a "2005, 2006"
if you like.
> > + * the terms of the GNU General Public License version 2. This program
> > + * is licensed "as is" without any warranty of any kind, whether express
> > + * or implied.
> > + */
> > +
> > +#ifndef _M41T00_H
> > +#define _M41T00_H
> > +
> > +#define M41T00_DRV_NAME "m41t00"
>
> Why do you need to export this?
Because the code that passes the platform_data needs to put that name
in the data so that the driver can find it. That's the value that the
driver uses to search for its platform_data. I'll attach the patch to
the platform code that I used to test it so you can see what I mean.
Its at the end of this email.
> > +#define M41T00_I2C_ADDR 0x68
>
> Not used anywhere?
Yes, in the platform code.
> > +#define M41T00_TYPE_M41T00 0
> > +#define M41T00_TYPE_M41T81 81
> > +#define M41T00_TYPE_M41T85 85
>
> The i2c core has a facility for drivers supporting several types of
> devices. Check for I2C_CLIENT_INSMOD_3() in your case. The advantage if
> you go that way is that chip types can be set from userspace through
> module parameters, which may be convenient for testing when adding
> support for new platforms.
Okay, I'll take a look.
> > +/* SQW output disabled, this is default value by power on*/
> > +#define SQW_FREQ_DISABLE (0)
> > +
> > +#define SQW_FREQ_32KHZ (1<<4) /* 32.768 KHz */
> > +#define SQW_FREQ_8KHZ (2<<4) /* 8.192 KHz */
> > +#define SQW_FREQ_4KHZ (3<<4) /* 4.096 KHz */
> > +#define SQW_FREQ_2KHZ (4<<4) /* 2.048 KHz */
> > +#define SQW_FREQ_1KHZ (5<<4) /* 1.024 KHz */
> > +#define SQW_FREQ_512HZ (6<<4) /* 512 Hz */
> > +#define SQW_FREQ_256HZ (7<<4) /* 256 Hz */
> > +#define SQW_FREQ_128HZ (8<<4) /* 128 Hz */
> > +#define SQW_FREQ_64HZ (9<<4) /* 64 Hz */
> > +#define SQW_FREQ_32HZ (10<<4) /* 32 Hz */
> > +#define SQW_FREQ_16HZ (11<<4) /* 16 Hz */
> > +#define SQW_FREQ_8HZ (12<<4) /* 8 Hz */
> > +#define SQW_FREQ_4HZ (13<<4) /* 4 Hz */
> > +#define SQW_FREQ_2HZ (14<<4) /* 2 Hz */
> > +#define SQW_FREQ_1HZ (15<<4) /* 1 Hz */
>
> Not used anywhere either?
Platform code can pass one of these values into the driver via
platform_data (if its a t81 or t85, not needed for the t00).
> > +extern ulong m41t00_get_rtc_time(void);
> > +extern int m41t00_set_rtc_time(ulong nowtime);
>
> Hopefully you won't have to export these anymore after you move to the
> new RTC subsystem. But that's a different story. In the meantime,
> shouldn't you have EXPORT_SYMBOL_GPL() for these functions? Else
> compiling m41t00 as a module won't work.
Good point. Will fix.
Below is the katana patch. Basically, it adds the platform_data
required for the m41t00 rtc which the m41t00 driver later takes out.
Note that the m41t00 doesn't need the sqw_freq value so you won't see
it being set in this patch.
I did have a couple questions above so I'll wait for your response
and then make a new patch.
Thanks for your time, Jean.
Mark
---
diff -Nurp linux-2.6.16-mm1-cleanup/arch/ppc/platforms/katana.c linux-2.6.16-mm1-newsupp/arch/ppc/platforms/katana.c
--- linux-2.6.16-mm1-cleanup/arch/ppc/platforms/katana.c 2006-03-23 16:15:22.000000000 -0700
+++ linux-2.6.16-mm1-newsupp/arch/ppc/platforms/katana.c 2006-03-23 17:37:46.000000000 -0700
@@ -28,6 +28,7 @@
#include <linux/mtd/physmap.h>
#include <linux/mv643xx.h>
#include <linux/platform_device.h>
+#include <linux/m41t00.h>
#ifdef CONFIG_BOOTIMG
#include <linux/bootimg.h>
#endif
@@ -843,8 +844,31 @@ katana_find_end_of_memory(void)
}
#if defined(CONFIG_I2C_MV64XXX) && defined(CONFIG_SENSORS_M41T00)
-extern ulong m41t00_get_rtc_time(void);
-extern int m41t00_set_rtc_time(ulong);
+static struct m41t00_platform_data katana_m41t00_pdata = {
+ .type = M41T00_TYPE_M41T00,
+ .i2c_addr = M41T00_I2C_ADDR,
+};
+
+static struct platform_device katana_m41t00_pdev = {
+ .name = M41T00_DRV_NAME,
+ .id = 0,
+ .num_resources = 0,
+ .dev = {
+ .platform_data = &katana_m41t00_pdata,
+ },
+};
+
+static int __init
+katana_add_pdata(void)
+{
+ int rc;
+
+ if ((rc = platform_device_register(&katana_m41t00_pdev)))
+ platform_device_unregister(&katana_m41t00_pdev);
+
+ return rc;
+}
+arch_initcall(katana_add_pdata);
static int __init
katana_rtc_hookup(void)
Hi Mark,
Answering your questions before I forget about them and let a week pass
again...
Anything I don't comment on, means that you were right and I agree with
you.
> > May I ask why you define separate types for the M41T81 and the M41T85,
> > when you handle both exactly the same way?
>
> The only reason is so that its obvious that both the t81 and t85 are
> supported. If I make it M41T81 only then I can easily see someone
> grep'ing around looking for M41T85, not finding it, and writing their
> own driver. I don't see any harm in having both, do you?
It wastes some memory, and you may later fix something for the M41T81
and forget to fix it for the M41T85.
If your only concern is to help grepers, you can add a clear list of
supported chips either as a comment at the top of the source file, or
as Documentation/i2c/chips/m41t00. That's what we do for hardware
monitoring chips.
No big deal anyway, so the decision is up to you.
> > What you do here are basically SMBus Read Byte and SMBus Write Byte
> > transactions. The code would be much more simple if you were using the
> > i2c_smbus_read_byte_data and i2c_smbus_write_byte_data functions, which
> > take care of all the technical details.
>
> That's true but I assumed that since I was using i2c_transfer
> earlier, I should use it here. Is that a bad assumption?
> I do see that ds1337.c uses both types.
Bad assumption, indeed. Nothing prevents you from using the smbus
functions and the i2c functions in the same driver, as long as your
call to i2c_check_functionality covers every function you use. So,
just use whatever makes your driver easier to write.
Thanks,
--
Jean Delvare
Jean,
This is a complete replacement for the patch(es) with the same subject
submitted previously. It is still against 2.6.16-mm1 but if you want
it against 2.6.16-mm2, let me know.
I kept separate entries for t81 and t85 b/c I also added a string with
the chip name. That string is copied into the client struct and is
also used as the name of the workqueue thread.
I still have several error goto's in m41t00_probe(). If I don't,
I either need to have a bunch of returns in that routine (frowned
upon) or I have to get rid of the dev_err msgs which I think are
useful. If you have a better way, let me know.
Other than that, I think I have addressed all of your concerns.
Sorry for all of the iterations.
Mark
---
This patch adds support for the ST m41t81 and m41t85 i2c rtc chips
to the existing m41t00 driver.
Since there is no way to reliably determine what type of rtc chip
is in use, the chip type is passed in via platform_data. The i2c
address and square wave frequency are passed in via platform_data
as well. To accommodate the use of platform_data, a new header
file include/linux/m41t00.h has been added.
The m41t81 and m41t85 chips halt the updating of their time registers
while they are being accessed. They resume when a stop condition
exists on the i2c bus or when non-time related regs are accessed.
To make the best use of that facility and to make more efficient
use of the i2c bus, this patch replaces multiple i2c_smbus_xxx calls
with a single i2c_transfer call.
Signed-off-by: Mark A. Greer <[email protected]>
---
drivers/i2c/chips/m41t00.c | 320 ++++++++++++++++++++++++++++++++++-----------
include/linux/m41t00.h | 50 +++++++
2 files changed, 296 insertions(+), 74 deletions(-)
---
diff -Nurp linux-2.6.16-mm1-cleanup/drivers/i2c/chips/m41t00.c linux-2.6.16-mm1-newsupp/drivers/i2c/chips/m41t00.c
--- linux-2.6.16-mm1-cleanup/drivers/i2c/chips/m41t00.c 2006-03-28 12:58:43.000000000 -0700
+++ linux-2.6.16-mm1-newsupp/drivers/i2c/chips/m41t00.c 2006-03-28 14:39:28.000000000 -0700
@@ -1,9 +1,9 @@
/*
- * I2C client/driver for the ST M41T00 Real-Time Clock chip.
+ * I2C client/driver for the ST M41T00 family of i2c rtc chips.
*
* Author: Mark A. Greer <[email protected]>
*
- * 2005 (c) MontaVista Software, Inc. This file is licensed under
+ * 2005, 2006 (c) MontaVista Software, Inc. This file is licensed under
* the terms of the GNU General Public License version 2. This program
* is licensed "as is" without any warranty of any kind, whether express
* or implied.
@@ -19,21 +19,17 @@
#include <linux/i2c.h>
#include <linux/rtc.h>
#include <linux/bcd.h>
-#include <linux/mutex.h>
#include <linux/workqueue.h>
-
+#include <linux/platform_device.h>
+#include <linux/m41t00.h>
#include <asm/time.h>
#include <asm/rtc.h>
-#define M41T00_DRV_NAME "m41t00"
-
-static DEFINE_MUTEX(m41t00_mutex);
-
static struct i2c_driver m41t00_driver;
static struct i2c_client *save_client;
static unsigned short ignore[] = { I2C_CLIENT_END };
-static unsigned short normal_addr[] = { 0x68, I2C_CLIENT_END };
+static unsigned short normal_addr[] = { I2C_CLIENT_END, I2C_CLIENT_END };
static struct i2c_client_address_data addr_data = {
.normal_i2c = normal_addr,
@@ -41,34 +37,92 @@ static struct i2c_client_address_data ad
.ignore = ignore,
};
+struct m41t00_chip_info {
+ u8 type;
+ char *name;
+ u8 read_limit;
+ u8 sec; /* Offsets for chip regs */
+ u8 min;
+ u8 hour;
+ u8 day;
+ u8 mon;
+ u8 year;
+ u8 alarm_mon;
+ u8 alarm_hour;
+ u8 sqw;
+ u8 sqw_freq;
+};
+
+static struct m41t00_chip_info m41t00_chip_info_tbl[] = {
+ {
+ .type = M41T00_TYPE_M41T00,
+ .name = "m41t00",
+ .read_limit = 5,
+ .sec = 0,
+ .min = 1,
+ .hour = 2,
+ .day = 4,
+ .mon = 5,
+ .year = 6,
+ },
+ {
+ .type = M41T00_TYPE_M41T81,
+ .name = "m41t81",
+ .read_limit = 1,
+ .sec = 1,
+ .min = 2,
+ .hour = 3,
+ .day = 5,
+ .mon = 6,
+ .year = 7,
+ .alarm_mon = 0xa,
+ .alarm_hour = 0xc,
+ .sqw = 0x13,
+ },
+ {
+ .type = M41T00_TYPE_M41T85,
+ .name = "m41t85",
+ .read_limit = 1,
+ .sec = 1,
+ .min = 2,
+ .hour = 3,
+ .day = 5,
+ .mon = 6,
+ .year = 7,
+ .alarm_mon = 0xa,
+ .alarm_hour = 0xc,
+ .sqw = 0x13,
+ },
+};
+static struct m41t00_chip_info *m41t00_chip;
+
ulong
m41t00_get_rtc_time(void)
{
s32 sec, min, hour, day, mon, year;
s32 sec1, min1, hour1, day1, mon1, year1;
- ulong limit = 10;
+ u8 reads = 0;
+ u8 buf[8], msgbuf[1] = { 0 }; /* offset into rtc's regs */
+ struct i2c_msg msgs[] = {
+ {
+ .addr = save_client->addr,
+ .flags = 0,
+ .len = 1,
+ .buf = msgbuf,
+ },
+ {
+ .addr = save_client->addr,
+ .flags = I2C_M_RD,
+ .len = 8,
+ .buf = buf,
+ },
+ };
sec = min = hour = day = mon = year = 0;
- sec1 = min1 = hour1 = day1 = mon1 = year1 = 0;
- mutex_lock(&m41t00_mutex);
do {
- if (((sec = i2c_smbus_read_byte_data(save_client, 0)) >= 0)
- && ((min = i2c_smbus_read_byte_data(save_client, 1))
- >= 0)
- && ((hour = i2c_smbus_read_byte_data(save_client, 2))
- >= 0)
- && ((day = i2c_smbus_read_byte_data(save_client, 4))
- >= 0)
- && ((mon = i2c_smbus_read_byte_data(save_client, 5))
- >= 0)
- && ((year = i2c_smbus_read_byte_data(save_client, 6))
- >= 0)
- && ((sec == sec1) && (min == min1) && (hour == hour1)
- && (day == day1) && (mon == mon1)
- && (year == year1)))
-
- break;
+ if (i2c_transfer(save_client->adapter, msgs, 2) < 0)
+ goto read_err;
sec1 = sec;
min1 = min;
@@ -76,21 +130,21 @@ m41t00_get_rtc_time(void)
day1 = day;
mon1 = mon;
year1 = year;
- } while (--limit > 0);
- mutex_unlock(&m41t00_mutex);
-
- if (limit == 0) {
- dev_warn(&save_client->dev,
- "m41t00: can't read rtc chip\n");
- sec = min = hour = day = mon = year = 0;
- }
- sec &= 0x7f;
- min &= 0x7f;
- hour &= 0x3f;
- day &= 0x3f;
- mon &= 0x1f;
- year &= 0xff;
+ sec = buf[m41t00_chip->sec] & 0x7f;
+ min = buf[m41t00_chip->min] & 0x7f;
+ hour = buf[m41t00_chip->hour] & 0x3f;
+ day = buf[m41t00_chip->day] & 0x3f;
+ mon = buf[m41t00_chip->mon] & 0x1f;
+ year = buf[m41t00_chip->year];
+ } while ((++reads < m41t00_chip->read_limit) && ((sec != sec1)
+ || (min != min1) || (hour != hour1) || (day != day1)
+ || (mon != mon1) || (year != year1)));
+
+ if ((m41t00_chip->read_limit > 1) && ((sec != sec1) || (min != min1)
+ || (hour != hour1) || (day != day1) || (mon != mon1)
+ || (year != year1)))
+ goto read_err;
sec = BCD2BIN(sec);
min = BCD2BIN(min);
@@ -104,40 +158,60 @@ m41t00_get_rtc_time(void)
year += 100;
return mktime(year, mon, day, hour, min, sec);
+
+read_err:
+ dev_err(&save_client->dev, "m41t00_get_rtc_time: Read error\n");
+ return 0;
}
+EXPORT_SYMBOL_GPL(m41t00_get_rtc_time);
static void
m41t00_set(void *arg)
{
struct rtc_time tm;
- ulong nowtime = *(ulong *)arg;
+ int nowtime = *(int *)arg;
+ s32 sec, min, hour, day, mon, year;
+ u8 wbuf[9], *buf = &wbuf[1], msgbuf[1] = { 0 };
+ struct i2c_msg msgs[] = {
+ {
+ .addr = save_client->addr,
+ .flags = 0,
+ .len = 1,
+ .buf = msgbuf,
+ },
+ {
+ .addr = save_client->addr,
+ .flags = I2C_M_RD,
+ .len = 8,
+ .buf = buf,
+ },
+ };
to_tm(nowtime, &tm);
tm.tm_year = (tm.tm_year - 1900) % 100;
- tm.tm_sec = BIN2BCD(tm.tm_sec);
- tm.tm_min = BIN2BCD(tm.tm_min);
- tm.tm_hour = BIN2BCD(tm.tm_hour);
- tm.tm_mon = BIN2BCD(tm.tm_mon);
- tm.tm_mday = BIN2BCD(tm.tm_mday);
- tm.tm_year = BIN2BCD(tm.tm_year);
-
- mutex_lock(&m41t00_mutex);
- if ((i2c_smbus_write_byte_data(save_client, 0, tm.tm_sec & 0x7f) < 0)
- || (i2c_smbus_write_byte_data(save_client, 1, tm.tm_min & 0x7f)
- < 0)
- || (i2c_smbus_write_byte_data(save_client, 2, tm.tm_hour & 0x7f)
- < 0)
- || (i2c_smbus_write_byte_data(save_client, 4, tm.tm_mday & 0x7f)
- < 0)
- || (i2c_smbus_write_byte_data(save_client, 5, tm.tm_mon & 0x7f)
- < 0)
- || (i2c_smbus_write_byte_data(save_client, 6, tm.tm_year & 0x7f)
- < 0))
+ sec = BIN2BCD(tm.tm_sec);
+ min = BIN2BCD(tm.tm_min);
+ hour = BIN2BCD(tm.tm_hour);
+ day = BIN2BCD(tm.tm_mday);
+ mon = BIN2BCD(tm.tm_mon);
+ year = BIN2BCD(tm.tm_year);
+
+ /* Read reg values into buf[0..7]/wbuf[1..8] */
+ if (i2c_transfer(save_client->adapter, msgs, 2) < 0) {
+ dev_err(&save_client->dev, "m41t00_set: Read error\n");
+ return;
+ }
- dev_warn(&save_client->dev,"m41t00: can't write to rtc chip\n");
+ wbuf[0] = 0; /* offset into rtc's regs */
+ buf[m41t00_chip->sec] = (buf[m41t00_chip->sec] & ~0x7f) | (sec & 0x7f);
+ buf[m41t00_chip->min] = (buf[m41t00_chip->min] & ~0x7f) | (min & 0x7f);
+ buf[m41t00_chip->hour] = (buf[m41t00_chip->hour]& ~0x3f) | (hour& 0x3f);
+ buf[m41t00_chip->day] = (buf[m41t00_chip->day] & ~0x3f) | (day & 0x3f);
+ buf[m41t00_chip->mon] = (buf[m41t00_chip->mon] & ~0x1f) | (mon & 0x1f);
- mutex_unlock(&m41t00_mutex);
+ if (i2c_master_send(save_client, wbuf, 9) < 0)
+ dev_err(&save_client->dev, "m41t00_set: Write error\n");
}
static ulong new_time;
@@ -156,6 +230,48 @@ m41t00_set_rtc_time(ulong nowtime)
return 0;
}
+EXPORT_SYMBOL_GPL(m41t00_set_rtc_time);
+
+/*
+ *****************************************************************************
+ *
+ * platform_data Driver Interface
+ *
+ *****************************************************************************
+ */
+static int __init
+m41t00_platform_probe(struct platform_device *pdev)
+{
+ struct m41t00_platform_data *pdata;
+ int i;
+
+ if (pdev && (pdata = pdev->dev.platform_data)) {
+ normal_addr[0] = pdata->i2c_addr;
+
+ for (i=0; i<ARRAY_SIZE(m41t00_chip_info_tbl); i++)
+ if (m41t00_chip_info_tbl[i].type == pdata->type) {
+ m41t00_chip = &m41t00_chip_info_tbl[i];
+ m41t00_chip->sqw_freq = pdata->sqw_freq;
+ return 0;
+ }
+ }
+ return -ENODEV;
+}
+
+static int __exit
+m41t00_platform_remove(struct platform_device *pdev)
+{
+ return 0;
+}
+
+static struct platform_driver m41t00_platform_driver = {
+ .probe = m41t00_platform_probe,
+ .remove = m41t00_platform_remove,
+ .driver = {
+ .owner = THIS_MODULE,
+ .name = M41T00_DRV_NAME,
+ },
+};
/*
*****************************************************************************
@@ -168,25 +284,76 @@ static int
m41t00_probe(struct i2c_adapter *adap, int addr, int kind)
{
struct i2c_client *client;
- int rc;
+ int rc = -EINVAL;
- client = kzalloc(sizeof(struct i2c_client), GFP_KERNEL);
- if (!client)
- return -ENOMEM;
+ if (!i2c_check_functionality(adap, I2C_FUNC_I2C
+ | I2C_FUNC_SMBUS_BYTE_DATA))
+ goto early_err;
+
+ if (!(client = kzalloc(sizeof(struct i2c_client), GFP_KERNEL))) {
+ rc = -ENOMEM;
+ goto early_err;
+ }
- strlcpy(client->name, M41T00_DRV_NAME, I2C_NAME_SIZE);
+ strlcpy(client->name, m41t00_chip->name, I2C_NAME_SIZE);
client->addr = addr;
client->adapter = adap;
client->driver = &m41t00_driver;
- if ((rc = i2c_attach_client(client)) != 0) {
- kfree(client);
- return rc;
+ if ((rc = i2c_attach_client(client)))
+ goto attach_err;
+
+ if (m41t00_chip->type != M41T00_TYPE_M41T00) {
+ /* If asked, disable SQW, set SQW frequency & re-enable */
+ if (m41t00_chip->sqw_freq)
+ if (((rc = i2c_smbus_read_byte_data(client,
+ m41t00_chip->alarm_mon)) < 0)
+ || ((rc = i2c_smbus_write_byte_data(client,
+ m41t00_chip->alarm_mon, rc & ~0x40)) <0)
+ || ((rc = i2c_smbus_write_byte_data(client,
+ m41t00_chip->sqw,
+ m41t00_chip->sqw_freq)) < 0)
+ || ((rc = i2c_smbus_write_byte_data(client,
+ m41t00_chip->alarm_mon, rc | 0x40)) <0))
+
+ goto sqw_err;
+
+ /* Make sure HT (Halt Update) bit is cleared */
+ if ((rc = i2c_smbus_read_byte_data(client,
+ m41t00_chip->alarm_hour)) < 0)
+ goto ht_err;
+
+ if (rc & 0x40)
+ if ((rc = i2c_smbus_write_byte_data(client,
+ m41t00_chip->alarm_hour, rc & ~0x40))<0)
+ goto ht_err;
}
- m41t00_wq = create_singlethread_workqueue("m41t00");
+ /* Make sure ST (stop) bit is cleared */
+ if ((rc = i2c_smbus_read_byte_data(client, m41t00_chip->sec)) < 0)
+ goto st_err;
+
+ if (rc & 0x80)
+ if ((rc = i2c_smbus_write_byte_data(client, m41t00_chip->sec,
+ rc & ~0x80)) < 0)
+ goto st_err;
+
+ m41t00_wq = create_singlethread_workqueue(m41t00_chip->name);
save_client = client;
return 0;
+
+st_err:
+ dev_err(&client->dev, "m41t00_probe: Can't clear ST bit\n");
+ goto attach_err;
+ht_err:
+ dev_err(&client->dev, "m41t00_probe: Can't clear HT bit\n");
+ goto attach_err;
+sqw_err:
+ dev_err(&client->dev, "m41t00_probe: Can't set SQW Frequency\n");
+attach_err:
+ kfree(client);
+early_err:
+ return rc;
}
static int
@@ -219,13 +386,18 @@ static struct i2c_driver m41t00_driver =
static int __init
m41t00_init(void)
{
- return i2c_add_driver(&m41t00_driver);
+ int rc;
+
+ if (!(rc = platform_driver_register(&m41t00_platform_driver)))
+ rc = i2c_add_driver(&m41t00_driver);
+ return rc;
}
static void __exit
m41t00_exit(void)
{
i2c_del_driver(&m41t00_driver);
+ platform_driver_unregister(&m41t00_platform_driver);
}
module_init(m41t00_init);
diff -Nurp linux-2.6.16-mm1-cleanup/include/linux/m41t00.h linux-2.6.16-mm1-newsupp/include/linux/m41t00.h
--- linux-2.6.16-mm1-cleanup/include/linux/m41t00.h 1969-12-31 17:00:00.000000000 -0700
+++ linux-2.6.16-mm1-newsupp/include/linux/m41t00.h 2006-03-28 14:36:56.000000000 -0700
@@ -0,0 +1,50 @@
+/*
+ * Definitions for the ST M41T00 family of i2c rtc chips.
+ *
+ * Author: Mark A. Greer <[email protected]>
+ *
+ * 2005 (c) MontaVista Software, Inc. This file is licensed under
+ * the terms of the GNU General Public License version 2. This program
+ * is licensed "as is" without any warranty of any kind, whether express
+ * or implied.
+ */
+
+#ifndef _M41T00_H
+#define _M41T00_H
+
+#define M41T00_DRV_NAME "m41t00"
+#define M41T00_I2C_ADDR 0x68
+
+#define M41T00_TYPE_M41T00 0
+#define M41T00_TYPE_M41T81 81
+#define M41T00_TYPE_M41T85 85
+
+struct m41t00_platform_data {
+ u8 type;
+ u8 i2c_addr;
+ u8 sqw_freq;
+};
+
+/* SQW output disabled, this is default value by power on*/
+#define SQW_FREQ_DISABLE (0)
+
+#define SQW_FREQ_32KHZ (1<<4) /* 32.768 KHz */
+#define SQW_FREQ_8KHZ (2<<4) /* 8.192 KHz */
+#define SQW_FREQ_4KHZ (3<<4) /* 4.096 KHz */
+#define SQW_FREQ_2KHZ (4<<4) /* 2.048 KHz */
+#define SQW_FREQ_1KHZ (5<<4) /* 1.024 KHz */
+#define SQW_FREQ_512HZ (6<<4) /* 512 Hz */
+#define SQW_FREQ_256HZ (7<<4) /* 256 Hz */
+#define SQW_FREQ_128HZ (8<<4) /* 128 Hz */
+#define SQW_FREQ_64HZ (9<<4) /* 64 Hz */
+#define SQW_FREQ_32HZ (10<<4) /* 32 Hz */
+#define SQW_FREQ_16HZ (11<<4) /* 16 Hz */
+#define SQW_FREQ_8HZ (12<<4) /* 8 Hz */
+#define SQW_FREQ_4HZ (13<<4) /* 4 Hz */
+#define SQW_FREQ_2HZ (14<<4) /* 2 Hz */
+#define SQW_FREQ_1HZ (15<<4) /* 1 Hz */
+
+extern ulong m41t00_get_rtc_time(void);
+extern int m41t00_set_rtc_time(ulong nowtime);
+
+#endif /* _M41T00_H */